invalidate existing sessions on password change

This commit is contained in:
Athou
2026-11-02 09:43:37 +01:00
parent ca4642f7c9
commit 0c280c4dc4
16 changed files with 370 additions and 26 deletions

View File

@@ -10,7 +10,10 @@ import io.quarkus.security.UnauthorizedException;
import jakarta.annotation.Priority;
import jakarta.validation.ValidationException;
import jakarta.ws.rs.core.HttpHeaders;
import jakarta.ws.rs.core.MediaType;
import jakarta.ws.rs.core.NewCookie;
import jakarta.ws.rs.core.UriInfo;
import jakarta.ws.rs.ext.Provider;
import lombok.RequiredArgsConstructor;
@@ -49,8 +52,19 @@ public class ExceptionMappers {
@ServerExceptionMapper(AuthenticationFailedException.class)
public RestResponse<AuthenticationFailed> authenticationFailed(
AuthenticationFailedException e) {
AuthenticationFailedException e, HttpHeaders headers, UriInfo uriInfo) {
NewCookie logoutCookie = cookieService.buildLogoutCookie();
// a browser navigating with an invalidated session would otherwise display the error
// instead of the page, retry the request now that the cookie is removed
if (headers.getAcceptableMediaTypes().stream()
.anyMatch(m -> MediaType.TEXT_HTML_TYPE.isCompatible(m) && !m.isWildcardType())) {
return ResponseBuilder.<AuthenticationFailed>create(Status.TEMPORARY_REDIRECT)
.location(uriInfo.getRequestUri())
.cookie(logoutCookie)
.build();
}
return ResponseBuilder.create(Status.UNAUTHORIZED, new AuthenticationFailed(e.getMessage()))
.cookie(logoutCookie)
.build();

View File

@@ -42,6 +42,9 @@ public class User extends AbstractModel {
@Column(nullable = false)
private boolean disabled;
@Column(nullable = false)
private int sessionVersion;
@Column private Instant lastLogin;
@Column private Instant created;

View File

@@ -155,6 +155,17 @@ public class UserService {
userDAO.delete(user);
}
public void changePassword(User user, String password) {
user.setPassword(encryptionService.getEncryptedPassword(password, user.getSalt()));
userDAO.merge(user);
invalidateSessions(user);
}
public void invalidateSessions(User user) {
user.setSessionVersion(user.getSessionVersion() + 1);
}
public String generateApiKey(User user) {
byte[] key =
encryptionService.getEncryptedPassword(

View File

@@ -6,7 +6,6 @@ import com.commafeed.backend.dao.UserRoleDAO;
import com.commafeed.backend.model.User;
import com.commafeed.backend.model.UserRole;
import com.commafeed.backend.model.UserRole.Role;
import com.commafeed.backend.service.PasswordEncryptionService;
import com.commafeed.backend.service.UserService;
import com.commafeed.frontend.model.UserModel;
import com.commafeed.frontend.model.request.AdminSaveUserRequest;
@@ -56,7 +55,6 @@ public class AdminREST {
private final UserDAO userDAO;
private final UserRoleDAO userRoleDAO;
private final UserService userService;
private final PasswordEncryptionService encryptionService;
private final MetricRegistry metrics;
@Path("/user/save")
@@ -102,10 +100,15 @@ public class AdminREST {
User u = userDAO.findById(id);
u.setName(req.getName());
if (StringUtils.isNotBlank(req.getPassword())) {
u.setPassword(
encryptionService.getEncryptedPassword(req.getPassword(), u.getSalt()));
userService.changePassword(u, req.getPassword());
if (StringUtils.isNotBlank(u.getApiKey())) {
u.setApiKey(userService.generateApiKey(u));
}
}
u.setEmail(req.getEmail());
if (!u.isDisabled() && !req.isEnabled()) {
userService.invalidateSessions(u);
}
u.setDisabled(!req.isEnabled());
Set<Role> roles = userRoleDAO.findRoles(u);

View File

@@ -21,7 +21,6 @@ import com.commafeed.backend.model.UserSettings.ReadingMode;
import com.commafeed.backend.model.UserSettings.ReadingOrder;
import com.commafeed.backend.model.UserSettings.ScrollMode;
import com.commafeed.backend.service.MailService;
import com.commafeed.backend.service.PasswordEncryptionService;
import com.commafeed.backend.service.PushNotificationService;
import com.commafeed.backend.service.UserService;
import com.commafeed.backend.service.db.DatabaseStartupService;
@@ -86,7 +85,6 @@ public class UserREST {
private final UserRoleDAO userRoleDAO;
private final UserSettingsDAO userSettingsDAO;
private final UserService userService;
private final PasswordEncryptionService encryptionService;
private final DatabaseStartupService databaseStartupService;
private final MailService mailService;
private final CommaFeedConfiguration config;
@@ -316,10 +314,7 @@ public class UserREST {
}
if (StringUtils.isNotBlank(request.getNewPassword())) {
byte[] password =
encryptionService.getEncryptedPassword(
request.getNewPassword(), user.getSalt());
user.setPassword(password);
userService.changePassword(user, request.getNewPassword());
user.setApiKey(userService.generateApiKey(user));
}
@@ -455,8 +450,7 @@ public class UserREST {
return Response.status(Status.UNAUTHORIZED).entity("Token expired.").build();
}
byte[] encryptedPassword = encryptionService.getEncryptedPassword(password, user.getSalt());
user.setPassword(encryptedPassword);
userService.changePassword(user, password);
if (StringUtils.isNotBlank(user.getApiKey())) {
user.setApiKey(userService.generateApiKey(user));
}

View File

@@ -21,11 +21,11 @@ public class AuthenticationContext {
return null;
}
String userId = securityIdentity.getPrincipal().getName();
if (userId == null) {
String name = securityIdentity.getPrincipal().getName();
if (name == null) {
return null;
}
return userDAO.findById(Long.valueOf(userId));
return userDAO.findById(SessionPrincipal.parse(name).userId());
}
}

View File

@@ -0,0 +1,27 @@
package com.commafeed.security;
import com.commafeed.backend.model.User;
/** Value of the security principal, which is also what ends up in the encrypted session cookie. */
public record SessionPrincipal(long userId, int sessionVersion) {
private static final String SEPARATOR = ":";
public static SessionPrincipal of(User user) {
return new SessionPrincipal(user.getId(), user.getSessionVersion());
}
public static SessionPrincipal parse(String name) {
// cookies created before session versions were introduced only contain the user id
if (!name.contains(SEPARATOR)) {
return new SessionPrincipal(Long.parseLong(name), 0);
}
String[] parts = name.split(SEPARATOR, 2);
return new SessionPrincipal(Long.parseLong(parts[0]), Integer.parseInt(parts[1]));
}
public String toName() {
return userId + SEPARATOR + sessionVersion;
}
}

View File

@@ -4,6 +4,7 @@ import com.commafeed.backend.dao.UnitOfWork;
import com.commafeed.backend.model.User;
import com.commafeed.backend.model.UserRole.Role;
import com.commafeed.backend.service.UserService;
import com.commafeed.security.SessionPrincipal;
import io.quarkus.security.AuthenticationFailedException;
import io.quarkus.security.identity.AuthenticationRequestContext;
@@ -47,7 +48,7 @@ public class DatabaseApiKeyIdentityProvider
Set<Role> roles = unitOfWork.call(() -> userService.getRoles(user.get()));
return QuarkusSecurityIdentity.builder()
.setPrincipal(new QuarkusPrincipal(String.valueOf(user.get().getId())))
.setPrincipal(new QuarkusPrincipal(SessionPrincipal.of(user.get()).toName()))
.addRoles(roles.stream().map(Enum::name).collect(Collectors.toSet()))
.build();
});

View File

@@ -6,6 +6,7 @@ import com.commafeed.backend.model.UserRole.Role;
import com.commafeed.backend.service.UserService;
import com.commafeed.frontend.exception.CommaFeedApplicationException;
import com.commafeed.frontend.exception.CommaFeedExceptionType;
import com.commafeed.security.SessionPrincipal;
import io.quarkus.security.identity.AuthenticationRequestContext;
import io.quarkus.security.identity.IdentityProvider;
@@ -52,7 +53,7 @@ public class DatabaseUsernamePasswordIdentityProvider
Set<Role> roles = unitOfWork.call(() -> userService.getRoles(user.get()));
return QuarkusSecurityIdentity.builder()
.setPrincipal(new QuarkusPrincipal(String.valueOf(user.get().getId())))
.setPrincipal(new QuarkusPrincipal(SessionPrincipal.of(user.get()).toName()))
.addRoles(roles.stream().map(Enum::name).collect(Collectors.toSet()))
.build();
});

View File

@@ -6,6 +6,7 @@ import com.commafeed.backend.model.User;
import com.commafeed.backend.model.UserRole.Role;
import com.commafeed.backend.service.UserService;
import com.commafeed.backend.service.internal.PostLoginActivities;
import com.commafeed.security.SessionPrincipal;
import io.quarkus.security.AuthenticationFailedException;
import io.quarkus.security.identity.AuthenticationRequestContext;
@@ -41,11 +42,17 @@ public class TrustedIdentityProvider implements IdentityProvider<TrustedAuthenti
public Uni<SecurityIdentity> authenticate(
TrustedAuthenticationRequest request, AuthenticationRequestContext context) {
return context.runBlocking(() -> {
Long userId = Long.valueOf(request.getPrincipal());
User user = unitOfWork.call(() -> userDAO.findById(userId));
SessionPrincipal principal = SessionPrincipal.parse(request.getPrincipal());
User user = unitOfWork.call(() -> userDAO.findById(principal.userId()));
if (user == null) {
throw new AuthenticationFailedException("user not found");
}
if (user.isDisabled()) {
throw new AuthenticationFailedException("user disabled");
}
if (user.getSessionVersion() != principal.sessionVersion()) {
throw new AuthenticationFailedException("session invalidated");
}
// execute post login activities manually because we didn't call login() since
// we received
@@ -54,7 +61,7 @@ public class TrustedIdentityProvider implements IdentityProvider<TrustedAuthenti
Set<Role> roles = unitOfWork.call(() -> userService.getRoles(user));
return QuarkusSecurityIdentity.builder()
.setPrincipal(new QuarkusPrincipal(String.valueOf(userId)))
.setPrincipal(new QuarkusPrincipal(principal.toName()))
.addRoles(roles.stream().map(Enum::name).collect(Collectors.toSet()))
.build();
});

View File

@@ -0,0 +1,14 @@
<?xml version="1.0" encoding="UTF-8" standalone="no"?>
<databaseChangeLog xmlns="http://www.liquibase.org/xml/ns/dbchangelog"
xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance"
xsi:schemaLocation="http://www.liquibase.org/xml/ns/dbchangelog https://www.liquibase.org/xml/ns/dbchangelog/dbchangelog-latest.xsd">
<changeSet id="user-session-version" author="athou">
<addColumn tableName="USERS">
<column name="sessionVersion" type="INT" valueNumeric="0" defaultValueNumeric="0">
<constraints nullable="false" />
</column>
</addColumn>
</changeSet>
</databaseChangeLog>

View File

@@ -39,5 +39,6 @@
<include file="changelogs/db.changelog-5.12.xml" />
<include file="changelogs/db.changelog-7.0.xml" />
<include file="changelogs/db.changelog-7.2.xml" />
<include file="changelogs/db.changelog-7.5.xml" />
</databaseChangeLog>

View File

@@ -109,15 +109,15 @@ public abstract class BaseIT {
}
protected List<HttpCookie> login() {
return login(TestConstants.ADMIN_USERNAME, TestConstants.ADMIN_PASSWORD);
}
protected List<HttpCookie> login(String userName, String password) {
List<Header> setCookieHeaders =
RestAssured.given()
.auth()
.none()
.formParams(
"j_username",
TestConstants.ADMIN_USERNAME,
"j_password",
TestConstants.ADMIN_PASSWORD)
.formParams("j_username", userName, "j_password", password)
.post("j_security_check")
.then()
.statusCode(HttpStatus.SC_OK)

View File

@@ -6,6 +6,7 @@ import com.commafeed.TestConstants;
import com.commafeed.frontend.exception.CommaFeedExceptionType;
import com.commafeed.frontend.model.Entries;
import com.commafeed.frontend.model.UserModel;
import com.commafeed.frontend.model.request.AdminSaveUserRequest;
import com.commafeed.frontend.model.request.MarkRequest;
import com.commafeed.frontend.model.request.ProfileModificationRequest;
import com.commafeed.frontend.model.request.SubscribeRequest;
@@ -17,6 +18,7 @@ import io.restassured.http.ContentType;
import jakarta.ws.rs.core.HttpHeaders;
import org.apache.hc.core5.http.HttpStatus;
import org.hamcrest.Matchers;
import org.junit.jupiter.api.Assertions;
import org.junit.jupiter.api.BeforeEach;
import org.junit.jupiter.api.Test;
@@ -28,6 +30,10 @@ import java.util.stream.Collectors;
@QuarkusTest
class SecurityIT extends BaseIT {
private static final String USER_NAME = "session-user";
private static final String USER_PASSWORD = "MyPassword123!";
private static final String NEW_USER_PASSWORD = "MyNewPassword123!";
@BeforeEach
void setup() {
initialSetup(TestConstants.ADMIN_USERNAME, TestConstants.ADMIN_PASSWORD);
@@ -184,4 +190,232 @@ class SecurityIT extends BaseIT {
.then()
.statusCode(HttpStatus.SC_UNAUTHORIZED);
}
@Test
void selfPasswordChangeInvalidatesSessions() {
createUser(false);
String sessionA = session(USER_NAME, USER_PASSWORD);
String sessionB = session(USER_NAME, USER_PASSWORD);
String adminSession = session(TestConstants.ADMIN_USERNAME, TestConstants.ADMIN_PASSWORD);
ProfileModificationRequest req = new ProfileModificationRequest();
req.setCurrentPassword(USER_PASSWORD);
req.setNewPassword(NEW_USER_PASSWORD);
RestAssured.given()
.header(HttpHeaders.COOKIE, sessionA)
.body(req)
.contentType(ContentType.JSON)
.post("rest/user/profile")
.then()
.statusCode(HttpStatus.SC_OK);
assertSessionStatus(sessionA, HttpStatus.SC_UNAUTHORIZED);
assertSessionStatus(sessionB, HttpStatus.SC_UNAUTHORIZED);
assertSessionStatus(adminSession, HttpStatus.SC_OK);
assertSessionStatus(session(USER_NAME, NEW_USER_PASSWORD), HttpStatus.SC_OK);
}
@Test
void profileChangeWithoutNewPasswordKeepsSessions() {
createUser(false);
String session = session(USER_NAME, USER_PASSWORD);
ProfileModificationRequest req = new ProfileModificationRequest();
req.setCurrentPassword(USER_PASSWORD);
req.setEmail("session-user@commafeed.com");
RestAssured.given()
.header(HttpHeaders.COOKIE, session)
.body(req)
.contentType(ContentType.JSON)
.post("rest/user/profile")
.then()
.statusCode(HttpStatus.SC_OK);
assertSessionStatus(session, HttpStatus.SC_OK);
}
@Test
void adminPasswordChangeInvalidatesSessionsAndRotatesApiKey() {
Long id = createUser(false);
String session = session(USER_NAME, USER_PASSWORD);
ProfileModificationRequest apiKeyReq = new ProfileModificationRequest();
apiKeyReq.setCurrentPassword(USER_PASSWORD);
apiKeyReq.setNewApiKey(true);
RestAssured.given()
.header(HttpHeaders.COOKIE, session)
.body(apiKeyReq)
.contentType(ContentType.JSON)
.post("rest/user/profile")
.then()
.statusCode(HttpStatus.SC_OK);
String apiKey = profile(session).getApiKey();
Assertions.assertNotNull(apiKey);
AdminSaveUserRequest req = saveUserRequest(false);
req.setId(id);
req.setPassword(NEW_USER_PASSWORD);
saveUser(req);
assertSessionStatus(session, HttpStatus.SC_UNAUTHORIZED);
String newSession = session(USER_NAME, NEW_USER_PASSWORD);
assertSessionStatus(newSession, HttpStatus.SC_OK);
Assertions.assertNotEquals(apiKey, profile(newSession).getApiKey());
}
@Test
void adminPasswordChangeDoesNotCreateApiKey() {
Long id = createUser(false);
AdminSaveUserRequest req = saveUserRequest(false);
req.setId(id);
req.setPassword(NEW_USER_PASSWORD);
saveUser(req);
Assertions.assertNull(profile(session(USER_NAME, NEW_USER_PASSWORD)).getApiKey());
}
@Test
void disablingUserInvalidatesSessions() {
Long id = createUser(false);
String session = session(USER_NAME, USER_PASSWORD);
assertSessionStatus(session, HttpStatus.SC_OK);
AdminSaveUserRequest req = saveUserRequest(false);
req.setId(id);
req.setEnabled(false);
saveUser(req);
assertSessionStatus(session, HttpStatus.SC_UNAUTHORIZED);
// sessions opened before the user was disabled stay invalid after enabling it again
req.setEnabled(true);
saveUser(req);
assertSessionStatus(session, HttpStatus.SC_UNAUTHORIZED);
assertSessionStatus(session(USER_NAME, USER_PASSWORD), HttpStatus.SC_OK);
}
@Test
void savingUserWithoutChangesKeepsSessions() {
Long id = createUser(false);
String session = session(USER_NAME, USER_PASSWORD);
AdminSaveUserRequest req = saveUserRequest(false);
req.setId(id);
saveUser(req);
assertSessionStatus(session, HttpStatus.SC_OK);
}
@Test
void disablingAdminRevokesAdminAccess() {
Long id = createUser(true);
String session = session(USER_NAME, USER_PASSWORD);
RestAssured.given()
.header(HttpHeaders.COOKIE, session)
.get("rest/admin/metrics")
.then()
.statusCode(HttpStatus.SC_OK);
AdminSaveUserRequest req = saveUserRequest(true);
req.setId(id);
req.setEnabled(false);
saveUser(req);
RestAssured.given()
.header(HttpHeaders.COOKIE, session)
.get("rest/admin/metrics")
.then()
.statusCode(HttpStatus.SC_UNAUTHORIZED);
}
@Test
void invalidatedSessionRemovesCookie() {
Long id = createUser(false);
String session = session(USER_NAME, USER_PASSWORD);
AdminSaveUserRequest req = saveUserRequest(false);
req.setId(id);
req.setEnabled(false);
saveUser(req);
// api calls receive an error
String apiSetCookie =
RestAssured.given()
.header(HttpHeaders.COOKIE, session)
.get("rest/user/profile")
.then()
.statusCode(HttpStatus.SC_UNAUTHORIZED)
.extract()
.header(HttpHeaders.SET_COOKIE);
Assertions.assertTrue(apiSetCookie.contains("Max-Age=0"));
// browsers are redirected to the page they asked for, this time without the cookie
String browserSetCookie =
RestAssured.given()
.redirects()
.follow(false)
.header(HttpHeaders.COOKIE, session)
.header(HttpHeaders.ACCEPT, "text/html,application/xhtml+xml,*/*;q=0.8")
.get("next?category=all")
.then()
.statusCode(HttpStatus.SC_TEMPORARY_REDIRECT)
.header(HttpHeaders.LOCATION, Matchers.endsWith("/next?category=all"))
.extract()
.header(HttpHeaders.SET_COOKIE);
Assertions.assertTrue(browserSetCookie.contains("Max-Age=0"));
}
private Long createUser(boolean admin) {
AdminSaveUserRequest req = saveUserRequest(admin);
req.setPassword(USER_PASSWORD);
return saveUser(req);
}
private AdminSaveUserRequest saveUserRequest(boolean admin) {
AdminSaveUserRequest req = new AdminSaveUserRequest();
req.setName(USER_NAME);
req.setEnabled(true);
req.setAdmin(admin);
return req;
}
private Long saveUser(AdminSaveUserRequest req) {
return RestAssured.given()
.auth()
.preemptive()
.basic(TestConstants.ADMIN_USERNAME, TestConstants.ADMIN_PASSWORD)
.body(req)
.contentType(ContentType.JSON)
.post("rest/admin/user/save")
.then()
.statusCode(HttpStatus.SC_OK)
.extract()
.as(Long.class);
}
private String session(String userName, String password) {
return login(userName, password).stream()
.map(HttpCookie::toString)
.collect(Collectors.joining(";"));
}
private UserModel profile(String session) {
return RestAssured.given()
.header(HttpHeaders.COOKIE, session)
.get("rest/user/profile")
.then()
.statusCode(HttpStatus.SC_OK)
.extract()
.as(UserModel.class);
}
private void assertSessionStatus(String session, int status) {
RestAssured.given()
.header(HttpHeaders.COOKIE, session)
.get("rest/user/profile")
.then()
.statusCode(status);
}
}

View File

@@ -13,6 +13,7 @@ import io.restassured.http.ContentType;
import io.vertx.ext.mail.MailMessage;
import jakarta.inject.Inject;
import jakarta.ws.rs.core.HttpHeaders;
import org.jsoup.Jsoup;
import org.jsoup.nodes.Element;
@@ -21,9 +22,11 @@ import org.junit.jupiter.api.Assertions;
import org.junit.jupiter.api.BeforeEach;
import org.junit.jupiter.api.Test;
import java.net.HttpCookie;
import java.net.URLDecoder;
import java.nio.charset.StandardCharsets;
import java.util.List;
import java.util.stream.Collectors;
@QuarkusTest
class UserIT extends BaseIT {
@@ -47,6 +50,9 @@ class UserIT extends BaseIT {
@Test
void resetPassword() {
String session =
login().stream().map(HttpCookie::toString).collect(Collectors.joining(";"));
PasswordResetRequest req = new PasswordResetRequest();
req.setEmail("admin@commafeed.com");
RestAssured.given()
@@ -99,6 +105,15 @@ class UserIT extends BaseIT {
.then()
.statusCode(200);
// sessions opened before the reset are no longer valid
RestAssured.given()
.auth()
.none()
.header(HttpHeaders.COOKIE, session)
.get("rest/user/settings")
.then()
.statusCode(401);
RestAssured.authentication =
RestAssured.preemptive().basic(TestConstants.ADMIN_USERNAME, newPassword);
RestAssured.given().get("rest/user/settings").then().statusCode(200);

View File

@@ -0,0 +1,19 @@
package com.commafeed.security;
import org.junit.jupiter.api.Assertions;
import org.junit.jupiter.api.Test;
class SessionPrincipalTest {
@Test
void roundTrip() {
SessionPrincipal principal = new SessionPrincipal(12, 3);
Assertions.assertEquals("12:3", principal.toName());
Assertions.assertEquals(principal, SessionPrincipal.parse(principal.toName()));
}
@Test
void cookieWithoutSessionVersion() {
Assertions.assertEquals(new SessionPrincipal(12, 0), SessionPrincipal.parse("12"));
}
}