diff --git a/readme.md b/readme.md index a3625a9..0df2103 100644 --- a/readme.md +++ b/readme.md @@ -60,6 +60,14 @@ To Be Played is split into three repositories: down to USER or AUDITOR and back (`POST /api/auth/view-as`); the choice is stored per login in `refresh_tokens.active_role` and never goes above the stored role, so a demoted admin loses the rights at the next refresh. Core checks the same claim. + **Role change**: an admin sets the role of another user on the admin users page (`PUT /api/admin/users/{id}/role`). + The change is a row in `role_change` (actor, old and new role, time; shown in the "Roles" dialog) and in + `admin_audit` (`ROLE_CHANGE`), sets `users.role_changed_at` and clears "view as" of the open logins. An access + token issued before `role_changed_at` counts as not logged in (`401`) at Auth and at Core, within 30 s (cache + TTL); the frontend then refreshes and repeats the request, and the refresh reads the stored role. A role set by + SQL (the first admin) sets no `role_changed_at`; it works from the next refresh. There is never 0 admins: every + write that can remove an admin first locks the active admins (`shared/account/ActiveAdmins`, `SELECT ... FOR + UPDATE`), the actor must still be one of them, and the last admin cannot delete the own account. - **Role markers on services**: every service method a controller calls carries `@RequiresRole(Role.X)` or `@Public`, on the method or on its class (the method wins). A Spring Security method interceptor checks the active role while a request is served (scheduled jobs and `@Async` work run without a user and are not checked); @@ -68,8 +76,9 @@ To Be Played is split into three repositories: guard. ArchUnit fails the build for an unmarked method, two markers, a controller calling an `@Async` method, and a service method calling another service's method that needs more. - **Admin users page** (`/api/admin/**`, active role PLATFORM_ADMIN): search and page all users, block and unblock - (`users.enabled`; blocking ends every login and, within 30 s, the access token at Auth; Core accepts the token - until it expires, max 15 min; the login then answers `403 ACCOUNT_BLOCKED`, but only after the right password), delete (the same flag and nightly cleanup as "Delete my account"). No action on the own account + (`users.enabled`; blocking ends every login and, within 30 s, the access token at Auth and at Core; the login then + answers `403 ACCOUNT_BLOCKED`, but only after the right password), delete (the same flag and nightly cleanup as + "Delete my account"). No action on the own account or on another admin. Every action and every "view as" is a row in `admin_audit`. Accept or deny an account that waits (`users.status` PENDING; the login answers `403 ACCOUNT_PENDING` after the right password): accept makes it ACTIVE and mails "your account is active"; deny makes it DENIED, mails the user @@ -132,6 +141,8 @@ To Be Played is split into three repositories: | `GET` | `/api/admin/users?query=&status=&page=&size=` (`{content, page, size, totalElements, totalPages}`; `status` UNCONFIRMED, PENDING, ACTIVE or DENIED, empty for all; each user has `status`) | PLATFORM_ADMIN | | `GET` | `/api/admin/users/pending-count` (`{count}`: registrations that wait) | PLATFORM_ADMIN | | `PUT` | `/api/admin/users/{id}/decision` (`{accepted}`; `204`; `409 NOT_PENDING`, also for an unconfirmed request) | PLATFORM_ADMIN | +| `PUT` | `/api/admin/users/{id}/role` (`{role, expectedRole}`; `204`; `400` for an unknown role; `404 USER_NOT_FOUND`; `409 SELF_ACTION`, `ACTOR_NOT_ADMIN`, `ROLE_CHANGED` (the role is not `expectedRole`), `ALREADY_DELETED`, `TARGET_IS_BLOCKED`, `TARGET_IS_PENDING`, `TARGET_IS_UNCONFIRMED`) | PLATFORM_ADMIN | +| `GET` | `/api/admin/users/{id}/role-changes` (`{results: [{actorUserId, actorEmail, oldRole, newRole, changedAt}]}`, newest first, 50 at most; `actorEmail` null after the actor is purged) | PLATFORM_ADMIN | | `PUT` | `/api/admin/users/{id}/blocked` (`{blocked}`; `409 SELF_ACTION`, `TARGET_IS_ADMIN`, `ALREADY_DELETED`, `TARGET_IS_PENDING`) | PLATFORM_ADMIN | | `DELETE` | `/api/admin/users/{id}` (`409 SELF_ACTION`, `TARGET_IS_ADMIN`, `TARGET_IS_PENDING`, `TARGET_IS_UNCONFIRMED`) | PLATFORM_ADMIN | | `DELETE` | `/api/admin/users/{id}/unconfirmed` (hard delete of an unconfirmed request; `204`; `404`; `409 NOT_UNCONFIRMED`, `SELF_ACTION`, `TARGET_IS_ADMIN`) | PLATFORM_ADMIN | @@ -150,7 +161,7 @@ To Be Played is split into three repositories: | `POST` | `/api/admin/news` (`{titleEn, titleDe, textEn, textDe}`; `201 {id}`) | PLATFORM_ADMIN | | `PUT` | `/api/admin/news/{id}` (same body; `204`; `404 NEWS_NOT_FOUND`) | PLATFORM_ADMIN | | `DELETE` | `/api/admin/news/{id}` (`204`; `404 NEWS_NOT_FOUND`) | PLATFORM_ADMIN | -| `DELETE` | `/api/users/me` (`{password}`; `204` and both cookies cleared, `403 INVALID_PASSWORD`, `429`) | access token | +| `DELETE` | `/api/users/me` (`{password}`; `204` and both cookies cleared, `403 INVALID_PASSWORD`, `409 LAST_ADMIN` (the only active admin), `429`) | access token | | `GET` | `/api/steam-link/callback` | public, Steam's redirect back; binding cookie; redirects to the frontend settings | | `PUT` | `/api/users/settings/appearance` | access token | | `PUT` | `/api/users/settings/profile-style` (`{accent, corners, bannerAppid, backgroundAppid}`; `204`) | access token | @@ -194,9 +205,9 @@ may log in and is shown to others" uses `shared/account/ActiveUser`. A settings change is one `UserRepository.update(userId, change)`: load the user, change the model, write back the display name and the settings. A new setting needs no new repository method. Three Caffeine caches in `CacheConfig`: `users` (by userId) and `publicStats` expire 10 minutes after the write, -`activeUsers` (the filter's "blocked or deleted?" check per request) after 30 seconds. Every write evicts what it -changes. A request that read the database just before a block or delete can put the old answer back, so the short -TTL is the real limit: a blocked or deleted user keeps a valid access token for up to 30 s. +`activeUsers` (the filter's "blocked or deleted, and when did the role change?" check per request) after 30 seconds. +Every write evicts what it changes. A request that read the database just before a block or delete can put the old answer back, so the short +TTL is the real limit: a blocked or deleted user, or a token from before a role change, works for up to 30 s. `profile` and `stats` only read data. They have no entities and use `JdbcTemplate` queries in a `repo` package. `version` has only an `api` package: `GET /api/version` answers the commit and build time of the running image. `account` (delete and nightly cleanup) sets the flag with `JdbcTemplate` in a `repo` package too, and calls Core @@ -391,8 +402,8 @@ sequenceDiagram participant A as Auth participant C as Core A->>A: link / unlink / block / unblock / name change (commit) - A->>C: POST /internal/accounts/states {userId, steamId, displayName, blocked} - C->>C: blocked_user, Steam ID in user_steam_sync, display name in account_name + A->>C: POST /internal/accounts/states {userId, steamId, displayName, blocked, roleChangedAt} + C->>C: blocked_user, account_role_change, Steam ID in user_steam_sync, display name in account_name Note over A,C: nightly 04:15 A->>C: PUT /internal/accounts/states (every user) ``` diff --git a/src/main/java/auth_service/configuration/CacheConfig.java b/src/main/java/auth_service/configuration/CacheConfig.java index e821a2e..5ab9ee9 100644 --- a/src/main/java/auth_service/configuration/CacheConfig.java +++ b/src/main/java/auth_service/configuration/CacheConfig.java @@ -15,7 +15,8 @@ public class CacheConfig { // After write, not after access: an entry another path changed without an evict still ages out. static final Duration DEFAULT_TTL = Duration.ofMinutes(10); - // The real limit after a block or delete: a concurrent read can put the old answer back after the evict. + // The real limit after a block, delete or role change: a concurrent read can put the old answer back after the + // evict. static final Duration ACTIVE_USERS_TTL = Duration.ofSeconds(30); @Bean diff --git a/src/main/java/auth_service/configuration/security/JwtAuthenticationFilter.java b/src/main/java/auth_service/configuration/security/JwtAuthenticationFilter.java index 11db9a1..d536094 100644 --- a/src/main/java/auth_service/configuration/security/JwtAuthenticationFilter.java +++ b/src/main/java/auth_service/configuration/security/JwtAuthenticationFilter.java @@ -25,8 +25,8 @@ import java.util.stream.Stream; /** * Takes the login token from the access cookie or from a Bearer header (API clients and tests), the first that - * verifies. A stale cookie next to a valid header must not block the request. A blocked or deleted user counts as - * not logged in. + * verifies. A stale cookie next to a valid header must not block the request. A blocked or deleted user, and a token + * issued before the last role change of its user, count as not logged in. */ @Component @RequiredArgsConstructor @@ -48,7 +48,7 @@ public class JwtAuthenticationFilter extends OncePerRequestFilter { .flatMap(Optional::stream) .map(jwtService::parseLoginToken) .flatMap(Optional::stream) - .filter(each -> activeUserCheck.isActive(each.getIdAsLong())) + .filter(each -> activeUserCheck.state(each.getIdAsLong()).accepts(each.issuedAt())) .findFirst(); if (principal.isPresent() && SecurityContextHolder.getContext().getAuthentication() == null) { UsernamePasswordAuthenticationToken authToken = new UsernamePasswordAuthenticationToken( diff --git a/src/main/java/auth_service/configuration/security/JwtService.java b/src/main/java/auth_service/configuration/security/JwtService.java index e6f01d6..f71bcd2 100644 --- a/src/main/java/auth_service/configuration/security/JwtService.java +++ b/src/main/java/auth_service/configuration/security/JwtService.java @@ -19,6 +19,7 @@ import java.security.PrivateKey; import java.security.PublicKey; import java.security.spec.PKCS8EncodedKeySpec; import java.security.spec.X509EncodedKeySpec; +import java.time.Instant; import java.util.Base64; import java.util.Date; import java.util.List; @@ -65,15 +66,17 @@ public class JwtService implements TokenGenerator, ProfileTicketIssuer, ServiceT } @Override - public String generateToken(String userId, Long steamId, Role role) { + public String generateToken(String userId, Long steamId, Role role, Instant notBefore) { + long now = System.currentTimeMillis(); + long issuedAt = notBefore == null ? now : Math.max(now, notBefore.toEpochMilli()); var builder = Jwts.builder() .header().keyId(jwtConfig.getKid()).and() .issuer(ISSUER) .audience().add(LOGIN_AUDIENCE).and() .subject(userId) .claim(ROLE_CLAIM, role.name()) - .issuedAt(new Date(System.currentTimeMillis())) - .expiration(new Date(System.currentTimeMillis() + sessionProperties.getAccessTokenTtl().toMillis())); + .issuedAt(new Date(issuedAt)) + .expiration(new Date(now + sessionProperties.getAccessTokenTtl().toMillis())); if (steamId != null) { builder.claim("steamId", steamId); @@ -139,7 +142,8 @@ public class JwtService implements TokenGenerator, ProfileTicketIssuer, ServiceT return Optional.empty(); } return Optional.of(new UserPrincipal(userId, claims.get("steamId", Long.class), - Role.parse(claims.get(ROLE_CLAIM, String.class)))); + Role.parse(claims.get(ROLE_CLAIM, String.class)), + claims.getIssuedAt() == null ? null : claims.getIssuedAt().toInstant())); } catch (Exception _) { return Optional.empty(); } diff --git a/src/main/java/auth_service/modules/account/api/AccountExceptionController.java b/src/main/java/auth_service/modules/account/api/AccountExceptionController.java index 1cdc3d1..313a00e 100644 --- a/src/main/java/auth_service/modules/account/api/AccountExceptionController.java +++ b/src/main/java/auth_service/modules/account/api/AccountExceptionController.java @@ -1,6 +1,7 @@ package auth_service.modules.account.api; import auth_service.modules.account.exception.InvalidPasswordException; +import auth_service.modules.account.exception.LastAdminException; import auth_service.modules.account.exception.PasswordResetInvalidException; import auth_service.modules.account.exception.TooManyAttemptsException; import auth_service.shared.api.dto.response.ErrorResponse; @@ -29,6 +30,12 @@ public class AccountExceptionController { .body(new ErrorResponse("RESET_INVALID", ex.getMessage())); } + @ExceptionHandler(LastAdminException.class) + public ResponseEntity clientError(LastAdminException ex) { + return ResponseEntity.status(HttpStatus.CONFLICT).contentType(MediaType.APPLICATION_JSON) + .body(new ErrorResponse("LAST_ADMIN", ex.getMessage())); + } + @ExceptionHandler(TooManyAttemptsException.class) public ResponseEntity clientError(TooManyAttemptsException ex) { return ResponseEntity.status(HttpStatus.TOO_MANY_REQUESTS) diff --git a/src/main/java/auth_service/modules/account/exception/LastAdminException.java b/src/main/java/auth_service/modules/account/exception/LastAdminException.java new file mode 100644 index 0000000..7cd4d71 --- /dev/null +++ b/src/main/java/auth_service/modules/account/exception/LastAdminException.java @@ -0,0 +1,7 @@ +package auth_service.modules.account.exception; + +public class LastAdminException extends RuntimeException { + public LastAdminException() { + super("The last admin cannot delete the own account"); + } +} diff --git a/src/main/java/auth_service/modules/account/model/CoreAccountState.java b/src/main/java/auth_service/modules/account/model/CoreAccountState.java index 483fa7a..03316a5 100644 --- a/src/main/java/auth_service/modules/account/model/CoreAccountState.java +++ b/src/main/java/auth_service/modules/account/model/CoreAccountState.java @@ -1,7 +1,11 @@ package auth_service.modules.account.model; +import java.time.Instant; + /** * One user as Core copies it. + * + * @param roleChangedAt null when the role never changed on the admin page */ -public record CoreAccountState(long userId, Long steamId, String displayName, boolean blocked) { +public record CoreAccountState(long userId, Long steamId, String displayName, boolean blocked, Instant roleChangedAt) { } diff --git a/src/main/java/auth_service/modules/account/repo/AccountRepository.java b/src/main/java/auth_service/modules/account/repo/AccountRepository.java index 3505ae5..88b517b 100644 --- a/src/main/java/auth_service/modules/account/repo/AccountRepository.java +++ b/src/main/java/auth_service/modules/account/repo/AccountRepository.java @@ -1,6 +1,7 @@ package auth_service.modules.account.repo; import auth_service.modules.account.model.CoreAccountState; +import auth_service.shared.account.ActiveAdmins; import auth_service.shared.account.ActiveUser; import auth_service.shared.account.EmailAddress; import org.springframework.jdbc.core.JdbcTemplate; @@ -111,17 +112,26 @@ public class AccountRepository { private static final RowMapper STATE_ROW = (rs, row) -> new CoreAccountState(rs.getLong("id"), rs.getObject("steam_id", Long.class), rs.getString("display_name"), - !rs.getBoolean("enabled")); + !rs.getBoolean("enabled"), + rs.getTimestamp("role_changed_at") == null ? null : rs.getTimestamp("role_changed_at").toInstant()); public Optional findState(long userId) { - return jdbc.query("SELECT id, steam_id, display_name, enabled FROM users WHERE id = ?", STATE_ROW, userId).stream().findFirst(); + return jdbc.query("SELECT id, steam_id, display_name, enabled, role_changed_at FROM users WHERE id = ?", + STATE_ROW, userId).stream().findFirst(); } /** * Every user, also the flagged ones. */ public List findAllStates() { - return jdbc.query("SELECT id, steam_id, display_name, enabled FROM users ORDER BY id", STATE_ROW); + return jdbc.query("SELECT id, steam_id, display_name, enabled, role_changed_at FROM users ORDER BY id", STATE_ROW); + } + + /** + * Locks the active admins until the transaction ends, see {@link ActiveAdmins}. + */ + public List lockActiveAdmins() { + return jdbc.queryForList(ActiveAdmins.LOCK, Long.class); } /** diff --git a/src/main/java/auth_service/modules/account/service/AccountService.java b/src/main/java/auth_service/modules/account/service/AccountService.java index d08db21..01faea0 100644 --- a/src/main/java/auth_service/modules/account/service/AccountService.java +++ b/src/main/java/auth_service/modules/account/service/AccountService.java @@ -1,6 +1,7 @@ package auth_service.modules.account.service; import auth_service.modules.account.exception.InvalidPasswordException; +import auth_service.modules.account.exception.LastAdminException; import auth_service.modules.account.exception.TooManyAttemptsException; import auth_service.modules.account.repo.AccountRepository; import auth_service.shared.account.AccountDeleter; @@ -116,6 +117,9 @@ public class AccountService implements AccountDeleter { private boolean flagAndTellCore(long userId) { Instant now = Instant.now(); boolean flagged = Boolean.TRUE.equals(transactions.execute(status -> { + if (accountRepository.lockActiveAdmins().equals(List.of(userId))) { + throw new LastAdminException(); + } if (accountRepository.flagDeleted(userId, now) == 0) { return false; } diff --git a/src/main/java/auth_service/modules/admin/api/AdminUserController.java b/src/main/java/auth_service/modules/admin/api/AdminUserController.java index d972857..2d5d0e9 100644 --- a/src/main/java/auth_service/modules/admin/api/AdminUserController.java +++ b/src/main/java/auth_service/modules/admin/api/AdminUserController.java @@ -2,12 +2,17 @@ package auth_service.modules.admin.api; import auth_service.modules.admin.api.dto.request.BlockRequest; import auth_service.modules.admin.api.dto.request.DecisionRequest; +import auth_service.modules.admin.api.dto.request.RoleChangeRequest; import auth_service.modules.admin.api.dto.response.AdminUserPageResponse; import auth_service.modules.admin.api.dto.response.AdminUserResponse; import auth_service.modules.admin.api.dto.response.PendingCountResponse; +import auth_service.modules.admin.api.dto.response.RoleChangeListResponse; +import auth_service.modules.admin.api.dto.response.RoleChangeResponse; import auth_service.modules.admin.model.AdminUser; import auth_service.modules.admin.model.AdminUserPage; +import auth_service.modules.admin.model.RoleChange; import auth_service.modules.admin.service.AdminUserService; +import auth_service.modules.admin.service.RoleChangeService; import auth_service.shared.security.UserPrincipal; import jakarta.validation.Valid; import lombok.RequiredArgsConstructor; @@ -29,6 +34,7 @@ import org.springframework.web.bind.annotation.RestController; public class AdminUserController { private final AdminUserService adminUserService; + private final RoleChangeService roleChangeService; @GetMapping public ResponseEntity findUsers(@RequestParam(defaultValue = "") String query, @@ -59,6 +65,19 @@ public class AdminUserController { return ResponseEntity.noContent().build(); } + @PutMapping("/{userId}/role") + public ResponseEntity changeRole(@AuthenticationPrincipal UserPrincipal principal, @PathVariable long userId, + @Valid @RequestBody RoleChangeRequest request) { + roleChangeService.changeRole(principal.getIdAsLong(), userId, request.expectedRole(), request.role()); + return ResponseEntity.noContent().build(); + } + + @GetMapping("/{userId}/role-changes") + public ResponseEntity findRoleChanges(@PathVariable long userId) { + return ResponseEntity.ok(new RoleChangeListResponse(roleChangeService.findChanges(userId).stream() + .map(AdminUserController::toApi).toList())); + } + @DeleteMapping(value = "/{userId}") public ResponseEntity delete(@AuthenticationPrincipal UserPrincipal principal, @PathVariable long userId) { adminUserService.delete(principal.getIdAsLong(), userId); @@ -84,4 +103,9 @@ public class AdminUserController { user.registrationDate(), user.lastLoginDate(), user.steamLinked(), user.blocked(), user.deletedAt(), user.status()); } + + private static RoleChangeResponse toApi(RoleChange change) { + return new RoleChangeResponse(change.actorUserId(), change.actorEmail(), change.oldRole().name(), + change.newRole().name(), change.changedAt()); + } } diff --git a/src/main/java/auth_service/modules/admin/api/dto/request/RoleChangeRequest.java b/src/main/java/auth_service/modules/admin/api/dto/request/RoleChangeRequest.java new file mode 100644 index 0000000..4e4f843 --- /dev/null +++ b/src/main/java/auth_service/modules/admin/api/dto/request/RoleChangeRequest.java @@ -0,0 +1,7 @@ +package auth_service.modules.admin.api.dto.request; + +import auth_service.shared.security.Role; +import jakarta.validation.constraints.NotNull; + +public record RoleChangeRequest(@NotNull Role role, @NotNull Role expectedRole) { +} diff --git a/src/main/java/auth_service/modules/admin/api/dto/response/RoleChangeListResponse.java b/src/main/java/auth_service/modules/admin/api/dto/response/RoleChangeListResponse.java new file mode 100644 index 0000000..0eb710c --- /dev/null +++ b/src/main/java/auth_service/modules/admin/api/dto/response/RoleChangeListResponse.java @@ -0,0 +1,6 @@ +package auth_service.modules.admin.api.dto.response; + +import java.util.List; + +public record RoleChangeListResponse(List results) { +} diff --git a/src/main/java/auth_service/modules/admin/api/dto/response/RoleChangeResponse.java b/src/main/java/auth_service/modules/admin/api/dto/response/RoleChangeResponse.java new file mode 100644 index 0000000..98eb1bc --- /dev/null +++ b/src/main/java/auth_service/modules/admin/api/dto/response/RoleChangeResponse.java @@ -0,0 +1,10 @@ +package auth_service.modules.admin.api.dto.response; + +import java.time.Instant; + +/** + * @param actorEmail null after the actor is purged + */ +public record RoleChangeResponse(long actorUserId, String actorEmail, String oldRole, String newRole, + Instant changedAt) { +} diff --git a/src/main/java/auth_service/modules/admin/exception/AdminActionRefusedException.java b/src/main/java/auth_service/modules/admin/exception/AdminActionRefusedException.java index 9bd01d1..c69adcf 100644 --- a/src/main/java/auth_service/modules/admin/exception/AdminActionRefusedException.java +++ b/src/main/java/auth_service/modules/admin/exception/AdminActionRefusedException.java @@ -5,7 +5,8 @@ import lombok.Getter; @Getter public class AdminActionRefusedException extends RuntimeException { /** - * SELF_ACTION, TARGET_IS_ADMIN, ALREADY_DELETED, NOT_PENDING, TARGET_IS_PENDING, TARGET_IS_UNCONFIRMED, NOT_UNCONFIRMED or NOT_ACTIVE + * SELF_ACTION, TARGET_IS_ADMIN, ALREADY_DELETED, NOT_PENDING, TARGET_IS_PENDING, TARGET_IS_UNCONFIRMED, + * NOT_UNCONFIRMED, NOT_ACTIVE, ACTOR_NOT_ADMIN, TARGET_IS_BLOCKED or ROLE_CHANGED */ private final String code; diff --git a/src/main/java/auth_service/modules/admin/model/RoleChange.java b/src/main/java/auth_service/modules/admin/model/RoleChange.java new file mode 100644 index 0000000..9c556f4 --- /dev/null +++ b/src/main/java/auth_service/modules/admin/model/RoleChange.java @@ -0,0 +1,11 @@ +package auth_service.modules.admin.model; + +import auth_service.shared.security.Role; + +import java.time.Instant; + +/** + * @param actorEmail null after the actor is purged + */ +public record RoleChange(long actorUserId, String actorEmail, Role oldRole, Role newRole, Instant changedAt) { +} diff --git a/src/main/java/auth_service/modules/admin/model/RoleTarget.java b/src/main/java/auth_service/modules/admin/model/RoleTarget.java new file mode 100644 index 0000000..9eebcd7 --- /dev/null +++ b/src/main/java/auth_service/modules/admin/model/RoleTarget.java @@ -0,0 +1,12 @@ +package auth_service.modules.admin.model; + +import auth_service.shared.security.Role; + +import java.time.Instant; + +/** + * @param deleted flagged as deleted, or denied + * @param roleChangedAt null when the role never changed on the admin page + */ +public record RoleTarget(Role role, boolean blocked, boolean deleted, String status, Instant roleChangedAt) { +} diff --git a/src/main/java/auth_service/modules/admin/repo/RoleChangeRepository.java b/src/main/java/auth_service/modules/admin/repo/RoleChangeRepository.java new file mode 100644 index 0000000..ab2e30d --- /dev/null +++ b/src/main/java/auth_service/modules/admin/repo/RoleChangeRepository.java @@ -0,0 +1,68 @@ +package auth_service.modules.admin.repo; + +import auth_service.modules.admin.model.RoleChange; +import auth_service.modules.admin.model.RoleTarget; +import auth_service.shared.account.ActiveAdmins; +import auth_service.shared.security.Role; +import lombok.RequiredArgsConstructor; +import org.springframework.jdbc.core.JdbcTemplate; +import org.springframework.stereotype.Repository; + +import java.sql.Timestamp; +import java.time.Instant; +import java.util.List; +import java.util.Optional; + +/** + * Plain SQL on users, because the user and login modules own the entities. + */ +@Repository +@RequiredArgsConstructor +public class RoleChangeRepository { + + private final JdbcTemplate jdbc; + + /** + * Locks the active admins until the transaction ends, see {@link ActiveAdmins}. + */ + public List lockActiveAdmins() { + return jdbc.queryForList(ActiveAdmins.LOCK, Long.class); + } + + public Optional lockTarget(long userId) { + return jdbc.query(""" + SELECT role, enabled, deleted_at IS NOT NULL OR status = 'DENIED' AS deleted, status, role_changed_at + FROM users WHERE id = ? FOR UPDATE""", + (rs, row) -> new RoleTarget(Role.parse(rs.getString("role")), !rs.getBoolean("enabled"), + rs.getBoolean("deleted"), rs.getString("status"), + rs.getTimestamp("role_changed_at") == null ? null : rs.getTimestamp("role_changed_at").toInstant()), + userId).stream().findFirst(); + } + + /** + * Also clears "view as" of every open login, so the next refresh starts at the new role. + */ + public void updateRole(long userId, Role role, Instant changedAt) { + jdbc.update("UPDATE users SET role = ?, role_changed_at = ? WHERE id = ?", role.name(), + Timestamp.from(changedAt), userId); + jdbc.update("UPDATE refresh_tokens SET active_role = NULL WHERE user_id = ? AND revoked_at IS NULL", userId); + } + + public void insertChange(long actorUserId, long targetUserId, Role oldRole, Role newRole, Instant changedAt) { + jdbc.update(""" + INSERT INTO role_change (actor_user_id, target_user_id, old_role, new_role, changed_at) + VALUES (?, ?, ?, ?, ?)""", actorUserId, targetUserId, oldRole.name(), newRole.name(), + Timestamp.from(changedAt)); + } + + public List findChanges(long targetUserId, int limit) { + return jdbc.query(""" + SELECT c.actor_user_id, a.email, c.old_role, c.new_role, c.changed_at + FROM role_change c LEFT JOIN users a ON a.id = c.actor_user_id + WHERE c.target_user_id = ? ORDER BY c.changed_at DESC, c.id DESC LIMIT ?""", + (rs, row) -> new RoleChange(rs.getLong("actor_user_id"), rs.getString("email"), + Role.parse(rs.getString("old_role")), Role.parse(rs.getString("new_role")), + rs.getTimestamp("changed_at").toInstant()), + targetUserId, limit); + } +} diff --git a/src/main/java/auth_service/modules/admin/service/RoleChangeService.java b/src/main/java/auth_service/modules/admin/service/RoleChangeService.java new file mode 100644 index 0000000..fcc8a89 --- /dev/null +++ b/src/main/java/auth_service/modules/admin/service/RoleChangeService.java @@ -0,0 +1,92 @@ +package auth_service.modules.admin.service; + +import auth_service.modules.admin.exception.AdminActionRefusedException; +import auth_service.modules.admin.exception.TargetUserNotFoundException; +import auth_service.modules.admin.model.RoleChange; +import auth_service.modules.admin.model.RoleTarget; +import auth_service.modules.admin.repo.RoleChangeRepository; +import auth_service.shared.account.AccountStatePublisher; +import auth_service.shared.audit.AdminAuditLog; +import auth_service.shared.security.RequiresRole; +import auth_service.shared.security.Role; +import lombok.RequiredArgsConstructor; +import lombok.extern.slf4j.Slf4j; +import org.springframework.cache.annotation.CacheEvict; +import org.springframework.cache.annotation.Caching; +import org.springframework.stereotype.Service; +import org.springframework.transaction.annotation.Transactional; + +import java.time.Instant; +import java.time.temporal.ChronoUnit; +import java.util.List; + +@Service +@RequiresRole(Role.PLATFORM_ADMIN) +@RequiredArgsConstructor +@Slf4j +public class RoleChangeService { + + private static final int LOG_LIMIT = 50; + + private final RoleChangeRepository roleChangeRepository; + private final AdminAuditLog adminAuditLog; + private final AccountStatePublisher accountStatePublisher; + + /** + * Locks the active admins, then the target, so two admins never remove each other (see ActiveAdmins). The actor + * is never the target and must still be an admin, so at least one admin stays. + * + * @param expectedRole the role the admin saw; another current role is refused, so no change is lost + */ + @Transactional + @Caching(evict = @CacheEvict(value = {"users", "activeUsers"}, key = "#targetUserId")) + public void changeRole(long actorUserId, long targetUserId, Role expectedRole, Role newRole) { + if (actorUserId == targetUserId) { + throw new AdminActionRefusedException("SELF_ACTION", "Admins cannot change their own role"); + } + if (!roleChangeRepository.lockActiveAdmins().contains(actorUserId)) { + throw new AdminActionRefusedException("ACTOR_NOT_ADMIN", "You are no admin any more"); + } + RoleTarget target = roleChangeRepository.lockTarget(targetUserId) + .orElseThrow(() -> new TargetUserNotFoundException(targetUserId)); + checkChangeable(target); + if (target.role() != expectedRole) { + throw new AdminActionRefusedException("ROLE_CHANGED", "The role changed in the meantime"); + } + if (target.role() == newRole) { + return; + } + Instant now = Instant.now(); + // The iat is in whole seconds, so a token of the change second may be older than the change. Valid tokens + // start in the next second, and after the previous change if that was in the same second. New tokens get + // at least this iat (TokenGenerator). + Instant validFrom = now.truncatedTo(ChronoUnit.SECONDS).plusSeconds(1); + if (target.roleChangedAt() != null && !target.roleChangedAt().isBefore(validFrom)) { + validFrom = target.roleChangedAt().plusSeconds(1); + } + roleChangeRepository.updateRole(targetUserId, newRole, validFrom); + roleChangeRepository.insertChange(actorUserId, targetUserId, target.role(), newRole, now); + adminAuditLog.record(actorUserId, "ROLE_CHANGE", targetUserId, target.role() + ">" + newRole); + accountStatePublisher.publish(targetUserId); + log.info("Admin {} changed the role of user {} from {} to {}", actorUserId, targetUserId, target.role(), newRole); + } + + public List findChanges(long targetUserId) { + return roleChangeRepository.findChanges(targetUserId, LOG_LIMIT); + } + + private static void checkChangeable(RoleTarget target) { + if (target.deleted()) { + throw new AdminActionRefusedException("ALREADY_DELETED", "The account is already deleted"); + } + if (target.blocked()) { + throw new AdminActionRefusedException("TARGET_IS_BLOCKED", "Unblock the account first"); + } + if ("PENDING".equals(target.status())) { + throw new AdminActionRefusedException("TARGET_IS_PENDING", "Accept or deny the registration first"); + } + if ("UNCONFIRMED".equals(target.status())) { + throw new AdminActionRefusedException("TARGET_IS_UNCONFIRMED", "Delete the unconfirmed registration instead"); + } + } +} diff --git a/src/main/java/auth_service/modules/login/model/User.java b/src/main/java/auth_service/modules/login/model/User.java index fe9824b..35573c5 100644 --- a/src/main/java/auth_service/modules/login/model/User.java +++ b/src/main/java/auth_service/modules/login/model/User.java @@ -5,6 +5,8 @@ import lombok.Builder; import lombok.Data; import lombok.ToString; +import java.time.Instant; + @Builder @Data public class User { @@ -18,6 +20,7 @@ public class User { private boolean pending; @Builder.Default private Role role = Role.USER; + private Instant roleChangedAt; public boolean canLogin() { return enabled && !deleted && !pending; diff --git a/src/main/java/auth_service/modules/login/persistence/MinimalUserRepositoryImpl.java b/src/main/java/auth_service/modules/login/persistence/MinimalUserRepositoryImpl.java index 98dd711..4a5fa94 100644 --- a/src/main/java/auth_service/modules/login/persistence/MinimalUserRepositoryImpl.java +++ b/src/main/java/auth_service/modules/login/persistence/MinimalUserRepositoryImpl.java @@ -18,7 +18,7 @@ public class MinimalUserRepositoryImpl implements MinimalUserRepository { private static final String SELECT_USER = "SELECT id, email, password, steam_id, enabled, deleted_at IS NOT NULL OR status = 'DENIED' AS deleted, status = 'PENDING' AS pending, " - + "role FROM users "; + + "role, role_changed_at FROM users "; private static final RowMapper USER_ROW = (rs, row) -> User.builder() .id(rs.getLong("id")) @@ -29,6 +29,7 @@ public class MinimalUserRepositoryImpl implements MinimalUserRepository { .deleted(rs.getBoolean("deleted")) .pending(rs.getBoolean("pending")) .role(Role.parse(rs.getString("role"))) + .roleChangedAt(rs.getTimestamp("role_changed_at") == null ? null : rs.getTimestamp("role_changed_at").toInstant()) .build(); private final JdbcTemplate jdbc; diff --git a/src/main/java/auth_service/modules/login/service/ActiveUserService.java b/src/main/java/auth_service/modules/login/service/ActiveUserService.java index 07fe465..5b9702b 100644 --- a/src/main/java/auth_service/modules/login/service/ActiveUserService.java +++ b/src/main/java/auth_service/modules/login/service/ActiveUserService.java @@ -4,6 +4,7 @@ import auth_service.modules.login.model.User; import auth_service.modules.login.persistence.MinimalUserRepository; import auth_service.shared.security.ActiveUserCheck; import auth_service.shared.security.Public; +import auth_service.shared.security.UserTokenState; import lombok.RequiredArgsConstructor; import org.springframework.cache.annotation.Cacheable; import org.springframework.stereotype.Service; @@ -20,7 +21,9 @@ public class ActiveUserService implements ActiveUserCheck { @Override @Cacheable(value = "activeUsers", key = "#userId") - public boolean isActive(long userId) { - return minimalUserRepository.findById(userId).filter(User::canLogin).isPresent(); + public UserTokenState state(long userId) { + return minimalUserRepository.findById(userId).filter(User::canLogin) + .map(user -> new UserTokenState(true, user.getRoleChangedAt())) + .orElse(UserTokenState.INACTIVE); } } diff --git a/src/main/java/auth_service/modules/login/service/LoginService.java b/src/main/java/auth_service/modules/login/service/LoginService.java index ce3f4bc..33f313d 100644 --- a/src/main/java/auth_service/modules/login/service/LoginService.java +++ b/src/main/java/auth_service/modules/login/service/LoginService.java @@ -1,7 +1,6 @@ package auth_service.modules.login.service; import auth_service.shared.security.Public; -import auth_service.shared.security.TokenGenerator; import auth_service.modules.login.exception.AccountBlockedException; import auth_service.modules.login.exception.AccountPendingException; import auth_service.modules.login.exception.InvalidCredentialsException; @@ -23,7 +22,6 @@ import java.util.UUID; public class LoginService { private final MinimalUserRepository minimalUserRepository; private final PasswordEncoder passwordEncoder; - private final TokenGenerator tokenGenerator; private final LoginAttemptService loginAttemptService; private final RefreshTokenService refreshTokenService; private final CacheManager cacheManager; @@ -32,11 +30,10 @@ public class LoginService { private final String dummyHash; public LoginService(MinimalUserRepository minimalUserRepository, PasswordEncoder passwordEncoder, - TokenGenerator tokenGenerator, LoginAttemptService loginAttemptService, - RefreshTokenService refreshTokenService, CacheManager cacheManager) { + LoginAttemptService loginAttemptService, RefreshTokenService refreshTokenService, + CacheManager cacheManager) { this.minimalUserRepository = minimalUserRepository; this.passwordEncoder = passwordEncoder; - this.tokenGenerator = tokenGenerator; this.loginAttemptService = loginAttemptService; this.refreshTokenService = refreshTokenService; this.cacheManager = cacheManager; @@ -82,7 +79,6 @@ public class LoginService { users.evict(user.getId()); } refreshTokenService.revoke(previousRefreshToken); - String accessToken = tokenGenerator.generateToken(String.valueOf(user.getId()), user.getSteamId(), user.getRole()); - return new Session(accessToken, refreshTokenService.startFamily(user.getId(), hash)); + return refreshTokenService.startSession(user.getId(), hash); } } diff --git a/src/main/java/auth_service/modules/login/service/RefreshTokenService.java b/src/main/java/auth_service/modules/login/service/RefreshTokenService.java index de3e0bf..380a3a9 100644 --- a/src/main/java/auth_service/modules/login/service/RefreshTokenService.java +++ b/src/main/java/auth_service/modules/login/service/RefreshTokenService.java @@ -53,11 +53,16 @@ public class RefreshTokenService { * @throws InvalidCredentialsException when the password is not the one checked */ @Transactional - public String startFamily(long userId, String passwordHash) { + public Session startSession(long userId, String passwordHash) { if (!userRepository.lockPasswordHash(userId).map(current -> current.equals(passwordHash)).orElse(false)) { throw new InvalidCredentialsException("Invalid email or password"); } - return issue(userId, UUID.randomUUID(), Instant.now(), null); + // Read after the lock, so a role change or block that committed during the login is seen. + User current = userRepository.findById(userId).filter(User::canLogin) + .orElseThrow(() -> new InvalidCredentialsException("Invalid email or password")); + String accessToken = tokenGenerator.generateToken(String.valueOf(userId), current.getSteamId(), + current.getRole(), current.getRoleChangedAt()); + return new Session(accessToken, issue(userId, UUID.randomUUID(), Instant.now(), null)); } /** @@ -125,7 +130,8 @@ public class RefreshTokenService { if (switching && stored != Role.USER) { adminAuditLog.record(current.getId(), "VIEW_AS", null, requested == null ? "OWN_ROLE" : requested.name()); } - String accessToken = tokenGenerator.generateToken(String.valueOf(current.getId()), current.getSteamId(), active); + String accessToken = tokenGenerator.generateToken(String.valueOf(current.getId()), current.getSteamId(), active, + current.getRoleChangedAt()); return Optional.of(new Session(accessToken, next)); } diff --git a/src/main/java/auth_service/shared/account/ActiveAdmins.java b/src/main/java/auth_service/shared/account/ActiveAdmins.java new file mode 100644 index 0000000..7d52e6e --- /dev/null +++ b/src/main/java/auth_service/shared/account/ActiveAdmins.java @@ -0,0 +1,14 @@ +package auth_service.shared.account; + +/** + * Every write that can remove an admin runs {@link #LOCK} first in its transaction, so two of them never both see + * a second admin who is about to go. + */ +public final class ActiveAdmins { + + public static final String LOCK = "SELECT u.id FROM users u WHERE u.role = 'PLATFORM_ADMIN' AND " + + ActiveUser.of("u") + " ORDER BY u.id FOR UPDATE"; + + private ActiveAdmins() { + } +} diff --git a/src/main/java/auth_service/shared/security/ActiveUserCheck.java b/src/main/java/auth_service/shared/security/ActiveUserCheck.java index 7a480a8..d11a009 100644 --- a/src/main/java/auth_service/shared/security/ActiveUserCheck.java +++ b/src/main/java/auth_service/shared/security/ActiveUserCheck.java @@ -1,8 +1,9 @@ package auth_service.shared.security; /** - * Port for the filter: the access token of a blocked or deleted user stops working before it expires. + * Port for the filter: the access token of a blocked or deleted user, or one from before a role change, stops + * working before it expires. */ public interface ActiveUserCheck { - boolean isActive(long userId); + UserTokenState state(long userId); } diff --git a/src/main/java/auth_service/shared/security/TokenGenerator.java b/src/main/java/auth_service/shared/security/TokenGenerator.java index d0f7aa0..1b399ce 100644 --- a/src/main/java/auth_service/shared/security/TokenGenerator.java +++ b/src/main/java/auth_service/shared/security/TokenGenerator.java @@ -1,5 +1,7 @@ package auth_service.shared.security; +import java.time.Instant; + /** * Port interface for token generation. * Decouples the login service from specific token implementations (e.g. JWT). @@ -13,6 +15,16 @@ public interface TokenGenerator { * @param role The active role of the login. * @return The generated token string. */ - String generateToken(String userId, Long steamId, Role role); + default String generateToken(String userId, Long steamId, Role role) { + return generateToken(userId, steamId, role, null); + } + + /** + * Same, but the token is never issued before {@code notBefore}, so the filter's role change check accepts it + * at once (see {@link UserTokenState#accepts}). + * + * @param notBefore lower bound of the {@code iat}, null for none + */ + String generateToken(String userId, Long steamId, Role role, Instant notBefore); } diff --git a/src/main/java/auth_service/shared/security/UserPrincipal.java b/src/main/java/auth_service/shared/security/UserPrincipal.java index 1e13d4d..6b5887d 100644 --- a/src/main/java/auth_service/shared/security/UserPrincipal.java +++ b/src/main/java/auth_service/shared/security/UserPrincipal.java @@ -1,16 +1,23 @@ package auth_service.shared.security; import java.security.Principal; +import java.time.Instant; /** - * @param role the active role of this login ("view as"), never above the stored one + * @param role the active role of this login ("view as"), never above the stored one + * @param issuedAt {@code iat} of the token, null when it has none */ public record UserPrincipal( String id, Long steamId, - Role role + Role role, + Instant issuedAt ) implements Principal { + public UserPrincipal(String id, Long steamId, Role role) { + this(id, steamId, role, null); + } + public UserPrincipal(String id, Long steamId) { this(id, steamId, Role.USER); } diff --git a/src/main/java/auth_service/shared/security/UserTokenState.java b/src/main/java/auth_service/shared/security/UserTokenState.java new file mode 100644 index 0000000..98dd49e --- /dev/null +++ b/src/main/java/auth_service/shared/security/UserTokenState.java @@ -0,0 +1,20 @@ +package auth_service.shared.security; + +import java.time.Instant; + +/** + * What the filter needs to know about the user of an access token. + * + * @param roleChangedAt null when the role never changed on the admin page + */ +public record UserTokenState(boolean active, Instant roleChangedAt) { + + public static final UserTokenState INACTIVE = new UserTokenState(false, null); + + /** + * A token issued before roleChangedAt is refused. roleChangedAt is the start of the second after the change. + */ + public boolean accepts(Instant issuedAt) { + return active && (roleChangedAt == null || (issuedAt != null && !issuedAt.isBefore(roleChangedAt))); + } +} diff --git a/src/main/resources/db/migration/V32__Role_Change.sql b/src/main/resources/db/migration/V32__Role_Change.sql new file mode 100644 index 0000000..f32e0a8 --- /dev/null +++ b/src/main/resources/db/migration/V32__Role_Change.sql @@ -0,0 +1,14 @@ +-- NULL: the role never changed on the admin page. A login token issued before this time is refused. +ALTER TABLE users ADD COLUMN role_changed_at TIMESTAMPTZ; + +-- Log of role changes on the admin page. No foreign keys: the log outlives a purged user. +CREATE TABLE role_change ( + id BIGSERIAL PRIMARY KEY, + actor_user_id BIGINT NOT NULL, + target_user_id BIGINT NOT NULL, + old_role VARCHAR(16) NOT NULL CHECK (old_role IN ('USER', 'AUDITOR', 'PLATFORM_ADMIN')), + new_role VARCHAR(16) NOT NULL CHECK (new_role IN ('USER', 'AUDITOR', 'PLATFORM_ADMIN')), + changed_at TIMESTAMPTZ NOT NULL +); + +CREATE INDEX idx_role_change_target ON role_change (target_user_id, changed_at DESC); diff --git a/src/test/java/auth_service/configuration/security/JwtServiceTest.java b/src/test/java/auth_service/configuration/security/JwtServiceTest.java index 62928c6..51efb5b 100644 --- a/src/test/java/auth_service/configuration/security/JwtServiceTest.java +++ b/src/test/java/auth_service/configuration/security/JwtServiceTest.java @@ -93,8 +93,12 @@ class JwtServiceTest extends AbstractUnitTest { void parseLoginToken_ReturnsTheUser() { var token = jwtService.generateToken("0815", 123456789L, Role.USER); - assertEquals(Optional.of(new UserPrincipal("0815", 123456789L)), - jwtService.parseLoginToken(token)); + UserPrincipal principal = jwtService.parseLoginToken(token).orElseThrow(); + + assertEquals("0815", principal.id()); + assertEquals(123456789L, principal.steamId()); + assertEquals(Role.USER, principal.role()); + assertFalse(principal.issuedAt().isAfter(java.time.Instant.now())); } @Test @@ -111,6 +115,16 @@ class JwtServiceTest extends AbstractUnitTest { assertTrue(secondsLeft > 14 * 60 && secondsLeft <= 15 * 60, "expires in " + secondsLeft + "s"); } + @Test + void generateToken_IsNeverIssuedBeforeNotBefore() { + var notBefore = java.time.Instant.now().plusSeconds(60).truncatedTo(java.time.temporal.ChronoUnit.SECONDS); + + var token = jwtService.generateToken("0815", null, Role.USER, notBefore); + + var claims = Jwts.parser().verifyWith(publicKey).build().parseSignedClaims(token).getPayload(); + assertEquals(notBefore, claims.getIssuedAt().toInstant()); + } + @Test void generateProfileTicket_NamesTheUserAndExpiresSoon() { var ticket = jwtService.generateProfileTicket(42L, Role.USER, List.of(), List.of()); diff --git a/src/test/java/auth_service/modules/account/api/DeletedAccountFlowTest.java b/src/test/java/auth_service/modules/account/api/DeletedAccountFlowTest.java index e5f77cd..0ae9c83 100644 --- a/src/test/java/auth_service/modules/account/api/DeletedAccountFlowTest.java +++ b/src/test/java/auth_service/modules/account/api/DeletedAccountFlowTest.java @@ -222,4 +222,31 @@ class DeletedAccountFlowTest extends AbstractIntegrationTest { .content("{\"email\":\"" + EMAIL + "\"}")) .andExpect(jsonPath("$.code").value("EMAIL_ALREADY_IN_USE")); } + + @Test + void theOnlyAdmin_CannotDeleteTheOwnAccount() throws Exception { + jdbc.update("UPDATE users SET role = 'USER' WHERE role = 'PLATFORM_ADMIN'"); + jdbc.update("UPDATE users SET role = 'PLATFORM_ADMIN' WHERE email = ?", EMAIL); + String access = login().getCookie(AuthCookies.ACCESS_COOKIE).getValue(); + + deleteAccount(access, PASSWORD).andExpect(status().isConflict()).andExpect(jsonPath("$.code").value("LAST_ADMIN")); + + assertThat(jdbc.queryForObject("SELECT deleted_at IS NULL FROM users WHERE email = ?", Boolean.class, EMAIL)).isTrue(); + } + + @Test + void oneOfTwoAdmins_CanDeleteTheOwnAccount() throws Exception { + String otherAdmin = "delete-flow-admin@example.com"; + jdbc.update("UPDATE users SET role = 'USER' WHERE role = 'PLATFORM_ADMIN'"); + jdbc.update("UPDATE users SET role = 'PLATFORM_ADMIN' WHERE email = ?", EMAIL); + jdbc.update("DELETE FROM users WHERE email = ?", otherAdmin); + jdbc.update("INSERT INTO users (email, password, enabled, role) VALUES (?, 'x', true, 'PLATFORM_ADMIN')", otherAdmin); + String access = login().getCookie(AuthCookies.ACCESS_COOKIE).getValue(); + + try { + deleteAccount(access, PASSWORD).andExpect(status().isNoContent()); + } finally { + jdbc.update("DELETE FROM users WHERE email = ?", otherAdmin); + } + } } diff --git a/src/test/java/auth_service/modules/account/repo/AccountRepositoryTest.java b/src/test/java/auth_service/modules/account/repo/AccountRepositoryTest.java index 4ad7be2..48e6544 100644 --- a/src/test/java/auth_service/modules/account/repo/AccountRepositoryTest.java +++ b/src/test/java/auth_service/modules/account/repo/AccountRepositoryTest.java @@ -56,12 +56,28 @@ class AccountRepositoryTest extends AbstractPersistenceTest { void findStates_CarryTheSteamIdTheDisplayNameAndTheBlock() { jdbc.update("update users set enabled = false, steam_id = 76561198000000001, display_name = 'Alice' where id = ?", alice); - assertThat(repository.findState(alice)).contains(new CoreAccountState(alice, 76561198000000001L, "Alice", true)); - assertThat(repository.findAllStates()).containsExactly(new CoreAccountState(alice, 76561198000000001L, "Alice", true), - new CoreAccountState(bob, null, null, false)); + assertThat(repository.findState(alice)).contains(new CoreAccountState(alice, 76561198000000001L, "Alice", true, null)); + assertThat(repository.findAllStates()).containsExactly(new CoreAccountState(alice, 76561198000000001L, "Alice", true, null), + new CoreAccountState(bob, null, null, false, null)); assertThat(repository.findState(-1L)).isEmpty(); } + @Test + void findStates_CarryTheRoleChangeTime() { + jdbc.update("update users set role_changed_at = ? where id = ?", Timestamp.from(NOW), alice); + + assertThat(repository.findState(alice)).get().extracting(CoreAccountState::roleChangedAt).isEqualTo(NOW); + assertThat(repository.findAllStates()).extracting(CoreAccountState::roleChangedAt).containsExactly(NOW, null); + } + + @Test + void lockActiveAdmins_ListsTheActiveAdmins() { + jdbc.update("update users set role = 'PLATFORM_ADMIN' where id = ?", bob); + jdbc.update("update users set role = 'PLATFORM_ADMIN', enabled = false where id = ?", alice); + + assertThat(repository.lockActiveAdmins()).containsExactly(bob); + } + @Test void flagDeleted_SetsTheFlagAndRevokesOnlyThatUsersTokens() { assertThat(repository.flagDeleted(alice, NOW)).isEqualTo(1); diff --git a/src/test/java/auth_service/modules/account/service/AccountCleanupServiceTest.java b/src/test/java/auth_service/modules/account/service/AccountCleanupServiceTest.java index 0edc4ff..2722a9a 100644 --- a/src/test/java/auth_service/modules/account/service/AccountCleanupServiceTest.java +++ b/src/test/java/auth_service/modules/account/service/AccountCleanupServiceTest.java @@ -31,7 +31,7 @@ import static org.mockito.Mockito.when; class AccountCleanupServiceTest extends AbstractUnitTest { - private static final List STATES = List.of(new CoreAccountState(1L, 76561198000000001L, "lobo", true)); + private static final List STATES = List.of(new CoreAccountState(1L, 76561198000000001L, "lobo", true, null)); private static final Duration RETENTION = Duration.ofDays(30); diff --git a/src/test/java/auth_service/modules/account/service/AccountServiceTest.java b/src/test/java/auth_service/modules/account/service/AccountServiceTest.java index f8c96fd..0d60109 100644 --- a/src/test/java/auth_service/modules/account/service/AccountServiceTest.java +++ b/src/test/java/auth_service/modules/account/service/AccountServiceTest.java @@ -1,6 +1,7 @@ package auth_service.modules.account.service; import auth_service.modules.account.exception.InvalidPasswordException; +import auth_service.modules.account.exception.LastAdminException; import auth_service.modules.account.exception.TooManyAttemptsException; import auth_service.modules.account.repo.AccountRepository; import auth_service.shared.mail.AccountMailer; @@ -83,6 +84,25 @@ class AccountServiceTest extends AbstractUnitTest { assertThat(deletedFrom.getValue()).isBetween(earliest, latest); } + @Test + void deleteAccount_TheLastAdminIsRefusedAndNothingIsFlagged() { + when(accountRepository.lockActiveAdmins()).thenReturn(List.of(USER_ID)); + + assertThatThrownBy(() -> service.deleteAccount(USER_ID, "right")).isInstanceOf(LastAdminException.class); + + verify(accountRepository, never()).flagDeleted(anyLong(), any()); + verifyNoInteractions(accountMailer, coreAccountService); + } + + @Test + void deleteAccount_OneOfTwoAdminsIsFlagged() { + when(accountRepository.lockActiveAdmins()).thenReturn(List.of(USER_ID, 43L)); + + service.deleteAccount(USER_ID, "right"); + + verify(accountRepository).flagDeleted(eq(USER_ID), any()); + } + @Test void deleteAccount_WrongPasswordFlagsNothing() { assertThatThrownBy(() -> service.deleteAccount(USER_ID, "wrong")).isInstanceOf(InvalidPasswordException.class); diff --git a/src/test/java/auth_service/modules/account/service/CoreAccountServiceTest.java b/src/test/java/auth_service/modules/account/service/CoreAccountServiceTest.java index ea36753..55ba3d8 100644 --- a/src/test/java/auth_service/modules/account/service/CoreAccountServiceTest.java +++ b/src/test/java/auth_service/modules/account/service/CoreAccountServiceTest.java @@ -14,6 +14,7 @@ import org.springframework.web.client.RestClient; import org.springframework.web.client.RestClientException; import tools.jackson.databind.JsonNode; +import java.time.Instant; import java.util.List; import java.util.Map; @@ -71,8 +72,9 @@ class CoreAccountServiceTest extends AbstractUnitTest { expect("applyStates"); expect("replaceStates"); - service.applyStates(List.of(new CoreAccountState(42L, 76561198000000001L, "lobo", true))); - service.replaceStates(List.of(new CoreAccountState(42L, null, "lobo", false))); + service.applyStates(List.of(new CoreAccountState(42L, 76561198000000001L, "lobo", true, + Instant.parse("2026-10-04T12:00:00Z")))); + service.replaceStates(List.of(new CoreAccountState(42L, null, "lobo", false, null))); core.verify(); } diff --git a/src/test/java/auth_service/modules/account/service/CoreAccountStatePublisherTest.java b/src/test/java/auth_service/modules/account/service/CoreAccountStatePublisherTest.java index 2cf06b9..65e7c76 100644 --- a/src/test/java/auth_service/modules/account/service/CoreAccountStatePublisherTest.java +++ b/src/test/java/auth_service/modules/account/service/CoreAccountStatePublisherTest.java @@ -22,7 +22,7 @@ import static org.mockito.Mockito.when; class CoreAccountStatePublisherTest extends AbstractUnitTest { - private static final CoreAccountState STATE = new CoreAccountState(7L, 76561198000000001L, "lobo", true); + private static final CoreAccountState STATE = new CoreAccountState(7L, 76561198000000001L, "lobo", true, null); @Mock private AccountRepository accountRepository; diff --git a/src/test/java/auth_service/modules/admin/AdminLockRaceTest.java b/src/test/java/auth_service/modules/admin/AdminLockRaceTest.java new file mode 100644 index 0000000..f4408a5 --- /dev/null +++ b/src/test/java/auth_service/modules/admin/AdminLockRaceTest.java @@ -0,0 +1,144 @@ +package auth_service.modules.admin; + +import auth_service.modules.account.exception.LastAdminException; +import auth_service.modules.account.service.AccountService; +import auth_service.modules.admin.exception.AdminActionRefusedException; +import auth_service.modules.admin.service.RoleChangeService; +import auth_service.shared.account.ActiveAdmins; +import auth_service.shared.security.Role; +import auth_service.shared.security.UserPrincipal; +import auth_service.shared.test.AbstractIntegrationTest; +import org.junit.jupiter.api.AfterEach; +import org.junit.jupiter.api.BeforeEach; +import org.junit.jupiter.api.Test; +import org.springframework.beans.factory.annotation.Autowired; +import org.springframework.jdbc.core.JdbcTemplate; +import org.springframework.security.authentication.UsernamePasswordAuthenticationToken; +import org.springframework.security.core.authority.SimpleGrantedAuthority; +import org.springframework.security.core.context.SecurityContextHolder; +import org.springframework.security.crypto.password.PasswordEncoder; + +import javax.sql.DataSource; +import java.sql.Connection; +import java.sql.Statement; +import java.util.Arrays; +import java.util.concurrent.Callable; +import java.util.concurrent.ExecutionException; +import java.util.concurrent.ExecutorService; +import java.util.concurrent.Executors; +import java.util.concurrent.Future; +import java.util.concurrent.TimeUnit; + +import static org.assertj.core.api.Assertions.assertThat; +import static org.assertj.core.api.Assertions.assertThatThrownBy; + +/** + * Two admins are the only admins. The first transaction locks both and demotes one of them, not committed. The second + * write must wait on the locked admin rows and then see the new state. The order is fixed, not timed. + */ +class AdminLockRaceTest extends AbstractIntegrationTest { + + private static final String PASSWORD = "correct horse battery"; + + @Autowired + private JdbcTemplate jdbc; + + @Autowired + private DataSource dataSource; + + @Autowired + private PasswordEncoder passwordEncoder; + + @Autowired + private RoleChangeService roleChangeService; + + @Autowired + private AccountService accountService; + + private long adminA; + private long adminB; + private final ExecutorService executor = Executors.newSingleThreadExecutor(); + + private long insertAdmin(String email) { + Long id = jdbc.queryForObject("INSERT INTO users (email, password, enabled, role, display_name) " + + "VALUES (?, ?, true, 'PLATFORM_ADMIN', ?) RETURNING id", Long.class, email, + passwordEncoder.encode(PASSWORD), email.substring(0, 12)); + addSettingsRows(); + return id; + } + + @BeforeEach + void seed() { + jdbc.update("DELETE FROM users"); + adminA = insertAdmin("lock-race-a@example.com"); + adminB = insertAdmin("lock-race-b@example.com"); + } + + @AfterEach + void cleanUp() { + executor.shutdownNow(); + jdbc.update("DELETE FROM users WHERE email LIKE 'lock-race-%'"); + } + + private Future asAdmin(long userId, Callable call) { + return executor.submit(() -> { + SecurityContextHolder.getContext().setAuthentication(new UsernamePasswordAuthenticationToken( + new UserPrincipal(String.valueOf(userId), null, Role.PLATFORM_ADMIN), null, + Arrays.stream(Role.values()).map(role -> new SimpleGrantedAuthority("ROLE_" + role.name())).toList())); + try { + return call.call(); + } finally { + SecurityContextHolder.clearContext(); + } + }); + } + + private void waitForLock() throws InterruptedException { + long deadline = System.currentTimeMillis() + 10_000; + while (jdbc.queryForObject("SELECT count(*) FROM pg_stat_activity WHERE wait_event_type = 'Lock'", + Integer.class) == 0) { + assertThat(System.currentTimeMillis()).as("the second write waits for the first").isLessThan(deadline); + Thread.sleep(20); + } + } + + private Future secondWrite(long actor, Callable call) throws Exception { + try (Connection first = dataSource.getConnection(); Statement statement = first.createStatement()) { + first.setAutoCommit(false); + statement.executeQuery(ActiveAdmins.LOCK).close(); + statement.executeUpdate("UPDATE users SET role = 'USER' WHERE id = " + adminB); + Future second = asAdmin(actor, call); + waitForLock(); + Thread.sleep(300); + assertThat(second.isDone()).as("the second write still waits").isFalse(); + first.commit(); + return second; + } + } + + @Test + void adminDemotedByAnotherTransaction_CannotDemoteTheOtherAdmin() throws Exception { + Future second = secondWrite(adminB, () -> { + roleChangeService.changeRole(adminB, adminA, Role.PLATFORM_ADMIN, Role.USER); + return null; + }); + + assertThatThrownBy(() -> second.get(10, TimeUnit.SECONDS)).isInstanceOf(ExecutionException.class) + .cause().isInstanceOfSatisfying(AdminActionRefusedException.class, + e -> assertThat(e.getCode()).isEqualTo("ACTOR_NOT_ADMIN")); + assertThat(jdbc.queryForObject("SELECT role FROM users WHERE id = ?", String.class, adminA)) + .isEqualTo("PLATFORM_ADMIN"); + } + + @Test + void adminWhoBecomesTheLastAdminWhileWaiting_CannotDeleteTheAccount() throws Exception { + Future second = secondWrite(adminA, () -> { + accountService.deleteAccount(adminA, PASSWORD); + return null; + }); + + assertThatThrownBy(() -> second.get(10, TimeUnit.SECONDS)).isInstanceOf(ExecutionException.class) + .cause().isInstanceOf(LastAdminException.class); + assertThat(jdbc.queryForObject("SELECT deleted_at IS NULL FROM users WHERE id = ?", Boolean.class, adminA)).isTrue(); + } +} diff --git a/src/test/java/auth_service/modules/admin/RoleChangeLoginRaceTest.java b/src/test/java/auth_service/modules/admin/RoleChangeLoginRaceTest.java new file mode 100644 index 0000000..59e5c87 --- /dev/null +++ b/src/test/java/auth_service/modules/admin/RoleChangeLoginRaceTest.java @@ -0,0 +1,132 @@ +package auth_service.modules.admin; + +import auth_service.configuration.security.ClientHeaderFilter; +import auth_service.modules.admin.service.RoleChangeService; +import auth_service.shared.api.AuthCookies; +import auth_service.shared.security.Role; +import auth_service.shared.security.UserPrincipal; +import auth_service.shared.test.AbstractIntegrationTest; +import jakarta.servlet.http.Cookie; +import org.junit.jupiter.api.AfterEach; +import org.junit.jupiter.api.BeforeEach; +import org.junit.jupiter.api.Test; +import org.springframework.beans.factory.annotation.Autowired; +import org.springframework.http.MediaType; +import org.springframework.jdbc.core.JdbcTemplate; +import org.springframework.mock.web.MockHttpServletResponse; +import org.springframework.security.authentication.UsernamePasswordAuthenticationToken; +import org.springframework.security.core.authority.SimpleGrantedAuthority; +import org.springframework.security.core.context.SecurityContextHolder; +import org.springframework.security.crypto.password.PasswordEncoder; +import org.springframework.test.context.bean.override.mockito.MockitoSpyBean; +import org.springframework.test.web.servlet.MockMvc; + +import java.sql.Timestamp; +import java.time.Instant; +import java.util.Arrays; +import java.util.concurrent.ExecutorService; +import java.util.concurrent.Executors; +import java.util.concurrent.TimeUnit; +import java.util.concurrent.atomic.AtomicBoolean; + +import static org.assertj.core.api.Assertions.assertThat; +import static org.mockito.ArgumentMatchers.any; +import static org.mockito.Mockito.doAnswer; +import static org.springframework.test.web.servlet.request.MockMvcRequestBuilders.get; +import static org.springframework.test.web.servlet.request.MockMvcRequestBuilders.post; + +/** + * A login reads the role before its bcrypt check. A demotion that commits during that check must not leave the login + * with an access token that carries the old role and still passes the role change check. + */ +class RoleChangeLoginRaceTest extends AbstractIntegrationTest { + + private static final String PASSWORD = "correct horse battery"; + private static final String ADMIN = "login-race-admin@example.com"; + private static final String VICTIM = "login-race-victim@example.com"; + + @Autowired + private MockMvc mockMvc; + + @Autowired + private JdbcTemplate jdbc; + + @MockitoSpyBean + private PasswordEncoder passwordEncoder; + + @Autowired + private RoleChangeService roleChangeService; + + private final ExecutorService executor = Executors.newSingleThreadExecutor(); + private long adminId; + private long victimId; + + private long insertAdmin(String email) { + jdbc.update("DELETE FROM users WHERE email = ?", email); + Long id = jdbc.queryForObject("INSERT INTO users (email, password, enabled, role, display_name) " + + "VALUES (?, ?, true, 'PLATFORM_ADMIN', ?) RETURNING id", Long.class, email, + passwordEncoder.encode(PASSWORD), email.substring(0, 16)); + addSettingsRows(); + return id; + } + + @BeforeEach + void seed() { + adminId = insertAdmin(ADMIN); + victimId = insertAdmin(VICTIM); + } + + @AfterEach + void cleanUp() { + executor.shutdownNow(); + jdbc.update("DELETE FROM role_change WHERE target_user_id = ?", victimId); + jdbc.update("DELETE FROM users WHERE email LIKE 'login-race-%'"); + } + + private void demoteVictimInAnotherThread() throws Exception { + executor.submit(() -> { + SecurityContextHolder.getContext().setAuthentication(new UsernamePasswordAuthenticationToken( + new UserPrincipal(String.valueOf(adminId), null, Role.PLATFORM_ADMIN), null, + Arrays.stream(Role.values()).map(role -> new SimpleGrantedAuthority("ROLE_" + role.name())).toList())); + try { + roleChangeService.changeRole(adminId, victimId, Role.PLATFORM_ADMIN, Role.USER); + } finally { + SecurityContextHolder.clearContext(); + } + return null; + }).get(10, TimeUnit.SECONDS); + } + + @Test + void loginRacingADemotionGetsNoTokenWithTheOldRole() throws Exception { + AtomicBoolean armed = new AtomicBoolean(true); + doAnswer(invocation -> { + boolean matches = (boolean) invocation.callRealMethod(); + if (armed.getAndSet(false)) { + demoteVictimInAnotherThread(); + Instant validFrom = jdbc.queryForObject("SELECT role_changed_at FROM users WHERE id = ?", + Timestamp.class, victimId).toInstant(); + // The bcrypt check of a real login can end after the next second starts. + while (Instant.now().isBefore(validFrom.plusMillis(5))) { + Thread.sleep(5); + } + } + return matches; + }).when(passwordEncoder).matches(any(), any()); + + MockHttpServletResponse login = mockMvc.perform(post("/api/auth/login") + .header(ClientHeaderFilter.CLIENT_HEADER, "web").header("X-Forwarded-For", freshClientIp()) + .contentType(MediaType.APPLICATION_JSON) + .content("{\"email\":\"" + VICTIM + "\",\"password\":\"" + PASSWORD + "\"}")) + .andReturn().getResponse(); + assertThat(armed.get()).as("the demotion ran inside the login").isFalse(); + assertThat(jdbc.queryForObject("SELECT role FROM users WHERE id = ?", String.class, victimId)).isEqualTo("USER"); + + int adminList = login.getStatus() != 200 ? 401 : mockMvc.perform(get("/api/admin/users") + .cookie(new Cookie(AuthCookies.ACCESS_COOKIE, + login.getCookie(AuthCookies.ACCESS_COOKIE).getValue()))) + .andReturn().getResponse().getStatus(); + + assertThat(adminList).as("admin list with the token of a login that raced the demotion").isIn(401, 403); + } +} diff --git a/src/test/java/auth_service/modules/admin/api/AdminFlowTest.java b/src/test/java/auth_service/modules/admin/api/AdminFlowTest.java index 220c8fe..e2ef714 100644 --- a/src/test/java/auth_service/modules/admin/api/AdminFlowTest.java +++ b/src/test/java/auth_service/modules/admin/api/AdminFlowTest.java @@ -337,4 +337,146 @@ class AdminFlowTest extends AbstractIntegrationTest { .andExpect(jsonPath("$.code").value("TARGET_IS_ADMIN")); block(admin, 999_999_999L, true).andExpect(status().isNotFound()); } + + private ResultActions changeRole(Session session, long userId, String role, String expectedRole) throws Exception { + return mockMvc.perform(put("/api/admin/users/" + userId + "/role").header(ClientHeaderFilter.CLIENT_HEADER, "web") + .cookie(new Cookie(AuthCookies.ACCESS_COOKIE, session.access())) + .contentType(MediaType.APPLICATION_JSON) + .content("{\"role\":\"" + role + "\",\"expectedRole\":\"" + expectedRole + "\"}")); + } + + private ResultActions roleChanges(Session session, long userId) throws Exception { + return mockMvc.perform(get("/api/admin/users/" + userId + "/role-changes") + .cookie(new Cookie(AuthCookies.ACCESS_COOKIE, session.access()))); + } + + private String activeRole(Session session) throws Exception { + return com.jayway.jsonpath.JsonPath.read(mockMvc.perform(get("/api/users/me") + .cookie(new Cookie(AuthCookies.ACCESS_COOKIE, session.access()))).andReturn().getResponse() + .getContentAsString(), "$.activeRole"); + } + + @Test + void roleChange_GrantEndsTheOldTokenAndTheNewOneCarriesTheRole() throws Exception { + Session admin = Session.of(login(ADMIN)); + Session player = Session.of(login(PLAYER)); + assertThat(me(player)).isEqualTo(200); + + changeRole(admin, playerId, "AUDITOR", "USER").andExpect(status().isNoContent()); + + assertThat(me(player)).isEqualTo(401); + assertThat(activeRole(refresh(player))).isEqualTo("AUDITOR"); + roleChanges(admin, playerId).andExpect(status().isOk()) + .andExpect(jsonPath("$.results.length()").value(1)) + .andExpect(jsonPath("$.results[0].actorEmail").value(ADMIN)) + .andExpect(jsonPath("$.results[0].oldRole").value("USER")) + .andExpect(jsonPath("$.results[0].newRole").value("AUDITOR")); + assertThat(audits("ROLE_CHANGE", playerId)).isEqualTo(1); + } + + @Test + void roleChange_SameRoleWritesNothing() throws Exception { + changeRole(Session.of(login(ADMIN)), playerId, "USER", "USER").andExpect(status().isNoContent()); + + assertThat(audits("ROLE_CHANGE", playerId)).isZero(); + assertThat(jdbc.queryForObject("SELECT COUNT(*) FROM role_change WHERE target_user_id = ?", Long.class, playerId)).isZero(); + } + + @Test + void roleChange_DemotedAdminLosesTheRightsAtOnce() throws Exception { + Session admin = Session.of(login(ADMIN)); + Session other = Session.of(login(OTHER_ADMIN)); + listUsers(other).andExpect(status().isOk()); + + changeRole(admin, otherAdminId, "USER", "PLATFORM_ADMIN").andExpect(status().isNoContent()); + + listUsers(other).andExpect(status().isUnauthorized()); + listUsers(refresh(other)).andExpect(status().isForbidden()); + } + + @Test + void roleChange_AdminWhoViewsAsUserIsAuditorAfterTheDemotionToAuditor() throws Exception { + Session admin = Session.of(login(ADMIN)); + Session asUser = Session.of(viewAs(Session.of(login(OTHER_ADMIN)), "USER")); + + changeRole(admin, otherAdminId, "AUDITOR", "PLATFORM_ADMIN").andExpect(status().isNoContent()); + + assertThat(activeRole(refresh(asUser))).isEqualTo("AUDITOR"); + } + + @Test + void roleChange_RefusesOwnRoleStaleRoleBlockedTargetBadRoleAndUnknownUser() throws Exception { + Session admin = Session.of(login(ADMIN)); + + changeRole(admin, jdbc.queryForObject("SELECT id FROM users WHERE email = ?", Long.class, ADMIN), "USER", + "PLATFORM_ADMIN").andExpect(status().isConflict()).andExpect(jsonPath("$.code").value("SELF_ACTION")); + changeRole(admin, playerId, "AUDITOR", "AUDITOR").andExpect(status().isConflict()) + .andExpect(jsonPath("$.code").value("ROLE_CHANGED")); + changeRole(admin, playerId, "GOD", "USER").andExpect(status().isBadRequest()); + changeRole(admin, 999_999_999L, "AUDITOR", "USER").andExpect(status().isNotFound()); + block(admin, playerId, true).andExpect(status().isNoContent()); + changeRole(admin, playerId, "AUDITOR", "USER").andExpect(status().isConflict()) + .andExpect(jsonPath("$.code").value("TARGET_IS_BLOCKED")); + + assertThat(jdbc.queryForObject("SELECT role FROM users WHERE id = ?", String.class, playerId)).isEqualTo("USER"); + } + + @Test + void roleEndpoints_AreForAdminsOnly() throws Exception { + Session player = Session.of(login(PLAYER)); + jdbc.update("UPDATE users SET role = 'AUDITOR' WHERE email = ?", OTHER_ADMIN); + Session auditor = Session.of(login(OTHER_ADMIN)); + Session viewingAsAuditor = Session.of(viewAs(Session.of(login(ADMIN)), "AUDITOR")); + + for (Session session : new Session[]{player, auditor, viewingAsAuditor}) { + changeRole(session, playerId, "AUDITOR", "USER").andExpect(status().isForbidden()); + roleChanges(session, playerId).andExpect(status().isForbidden()); + } + mockMvc.perform(get("/api/admin/users/" + playerId + "/role-changes")).andExpect(status().isUnauthorized()); + assertThat(jdbc.queryForObject("SELECT role FROM users WHERE id = ?", String.class, playerId)).isEqualTo("USER"); + } + + @Test + void roleChange_TokenFromTheSecondOfTheDemotionLosesTheAdminRights() throws Exception { + Session admin = Session.of(login(ADMIN)); + while (System.currentTimeMillis() % 1000 > 50) { + Thread.onSpinWait(); + } + Session other = Session.of(login(OTHER_ADMIN)); + + changeRole(admin, otherAdminId, "USER", "PLATFORM_ADMIN").andExpect(status().isNoContent()); + + listUsers(other).andExpect(status().isUnauthorized()); + } + + @Test + void roleChange_RefreshInTheSecondOfTheChangeGivesAValidToken() throws Exception { + Session admin = Session.of(login(ADMIN)); + while (System.currentTimeMillis() % 1000 > 50) { + Thread.onSpinWait(); + } + Session player = Session.of(login(PLAYER)); + + changeRole(admin, playerId, "AUDITOR", "USER").andExpect(status().isNoContent()); + Session refreshed = refresh(player); + + assertThat(me(player)).isEqualTo(401); + assertThat(me(refreshed)).isEqualTo(200); + assertThat(activeRole(refreshed)).isEqualTo("AUDITOR"); + } + + @Test + void roleChange_TokenFromBetweenTwoChangesInOneSecondLosesTheRole() throws Exception { + Session admin = Session.of(login(ADMIN)); + Session player = Session.of(login(PLAYER)); + while (System.currentTimeMillis() % 1000 > 50) { + Thread.onSpinWait(); + } + + changeRole(admin, playerId, "PLATFORM_ADMIN", "USER").andExpect(status().isNoContent()); + Session promoted = refresh(player); + changeRole(admin, playerId, "USER", "PLATFORM_ADMIN").andExpect(status().isNoContent()); + + listUsers(promoted).andExpect(status().isUnauthorized()); + } } diff --git a/src/test/java/auth_service/modules/admin/repo/RoleChangeRepositoryTest.java b/src/test/java/auth_service/modules/admin/repo/RoleChangeRepositoryTest.java new file mode 100644 index 0000000..bfe2c95 --- /dev/null +++ b/src/test/java/auth_service/modules/admin/repo/RoleChangeRepositoryTest.java @@ -0,0 +1,110 @@ +package auth_service.modules.admin.repo; + +import auth_service.modules.admin.model.RoleChange; +import auth_service.modules.admin.model.RoleTarget; +import auth_service.shared.security.Role; +import auth_service.shared.test.AbstractPersistenceTest; +import org.junit.jupiter.api.BeforeEach; +import org.junit.jupiter.api.Test; +import org.springframework.beans.factory.annotation.Autowired; +import org.springframework.jdbc.core.JdbcTemplate; + +import java.sql.Timestamp; +import java.time.Instant; +import java.time.temporal.ChronoUnit; +import java.util.UUID; + +import static org.assertj.core.api.Assertions.assertThat; + +class RoleChangeRepositoryTest extends AbstractPersistenceTest { + + private static final Instant NOW = Instant.now().truncatedTo(ChronoUnit.SECONDS); + + @Autowired + private RoleChangeRepository repository; + + @Autowired + private JdbcTemplate jdbc; + + private long admin; + private long otherAdmin; + private long user; + + private long insert(String email, String role) { + return jdbc.queryForObject("INSERT INTO users (email, password, enabled, role) VALUES (?, 'x', true, ?) RETURNING id", + Long.class, email, role); + } + + private void addLogin(long userId) { + jdbc.update(""" + INSERT INTO refresh_tokens (user_id, token_hash, family_id, expires_at, created_at, active_role) + VALUES (?, ?, ?, now() + interval '30 days', now(), 'USER')""", userId, UUID.randomUUID().toString(), + UUID.randomUUID()); + } + + @BeforeEach + void seed() { + jdbc.update("DELETE FROM role_change"); + jdbc.update("DELETE FROM users"); + admin = insert("admin@example.com", "PLATFORM_ADMIN"); + otherAdmin = insert("other-admin@example.com", "PLATFORM_ADMIN"); + user = insert("user@example.com", "USER"); + } + + @Test + void lockActiveAdmins_ListsOnlyActiveAdminsByIdAndNoOtherRole() { + long blocked = insert("blocked-admin@example.com", "PLATFORM_ADMIN"); + jdbc.update("UPDATE users SET enabled = false WHERE id = ?", blocked); + long deleted = insert("deleted-admin@example.com", "PLATFORM_ADMIN"); + jdbc.update("UPDATE users SET deleted_at = now() WHERE id = ?", deleted); + long pending = insert("pending-admin@example.com", "PLATFORM_ADMIN"); + jdbc.update("UPDATE users SET status = 'PENDING' WHERE id = ?", pending); + + assertThat(repository.lockActiveAdmins()).containsExactly(admin, otherAdmin); + } + + @Test + void lockTarget_ReadsRoleBlockDeletedAndStatus() { + jdbc.update("UPDATE users SET enabled = false WHERE id = ?", user); + + assertThat(repository.lockTarget(user)).contains(new RoleTarget(Role.USER, true, false, "ACTIVE", null)); + jdbc.update("UPDATE users SET status = 'DENIED' WHERE id = ?", user); + assertThat(repository.lockTarget(user)).get().extracting(RoleTarget::deleted).isEqualTo(true); + assertThat(repository.lockTarget(-1L)).isEmpty(); + } + + @Test + void updateRole_SetsRoleAndTimeAndClearsTheActiveRoleOfOpenLoginsOnly() { + addLogin(user); + addLogin(user); + jdbc.update("UPDATE refresh_tokens SET revoked_at = now() WHERE id = (SELECT min(id) FROM refresh_tokens WHERE user_id = ?)", user); + addLogin(otherAdmin); + + repository.updateRole(user, Role.AUDITOR, NOW); + + assertThat(jdbc.queryForObject("SELECT role FROM users WHERE id = ?", String.class, user)).isEqualTo("AUDITOR"); + assertThat(jdbc.queryForObject("SELECT role_changed_at FROM users WHERE id = ?", Timestamp.class, user).toInstant()) + .isEqualTo(NOW); + assertThat(jdbc.queryForList("SELECT active_role FROM refresh_tokens WHERE user_id = ? AND revoked_at IS NULL", + String.class, user)).containsOnlyNulls().hasSize(1); + assertThat(jdbc.queryForList("SELECT active_role FROM refresh_tokens WHERE user_id = ? AND revoked_at IS NOT NULL", + String.class, user)).containsExactly("USER"); + assertThat(jdbc.queryForList("SELECT active_role FROM refresh_tokens WHERE user_id = ?", String.class, otherAdmin)) + .containsExactly("USER"); + } + + @Test + void findChanges_NewestFirstLimitedWithTheActorEmailAndNullForAGoneActor() { + repository.insertChange(admin, user, Role.USER, Role.AUDITOR, NOW.minusSeconds(20)); + repository.insertChange(otherAdmin, user, Role.AUDITOR, Role.PLATFORM_ADMIN, NOW.minusSeconds(10)); + repository.insertChange(admin, user, Role.PLATFORM_ADMIN, Role.USER, NOW); + repository.insertChange(admin, otherAdmin, Role.USER, Role.AUDITOR, NOW); + jdbc.update("DELETE FROM users WHERE id = ?", otherAdmin); + + assertThat(repository.findChanges(user, 10)).containsExactly( + new RoleChange(admin, "admin@example.com", Role.PLATFORM_ADMIN, Role.USER, NOW), + new RoleChange(otherAdmin, null, Role.AUDITOR, Role.PLATFORM_ADMIN, NOW.minusSeconds(10)), + new RoleChange(admin, "admin@example.com", Role.USER, Role.AUDITOR, NOW.minusSeconds(20))); + assertThat(repository.findChanges(user, 2)).hasSize(2); + } +} diff --git a/src/test/java/auth_service/modules/admin/service/RoleChangeServiceTest.java b/src/test/java/auth_service/modules/admin/service/RoleChangeServiceTest.java new file mode 100644 index 0000000..03b162b --- /dev/null +++ b/src/test/java/auth_service/modules/admin/service/RoleChangeServiceTest.java @@ -0,0 +1,176 @@ +package auth_service.modules.admin.service; + +import auth_service.modules.admin.exception.AdminActionRefusedException; +import auth_service.modules.admin.exception.TargetUserNotFoundException; +import auth_service.modules.admin.model.RoleChange; +import auth_service.modules.admin.model.RoleTarget; +import auth_service.modules.admin.repo.RoleChangeRepository; +import auth_service.shared.account.AccountStatePublisher; +import auth_service.shared.audit.AdminAuditLog; +import auth_service.shared.security.Role; +import auth_service.shared.test.AbstractUnitTest; +import org.junit.jupiter.api.BeforeEach; +import org.junit.jupiter.api.Test; +import org.mockito.ArgumentCaptor; +import org.mockito.InOrder; +import org.mockito.Mock; + +import java.time.Instant; +import java.time.temporal.ChronoUnit; +import java.util.List; +import java.util.Optional; + +import static org.assertj.core.api.Assertions.assertThat; +import static org.assertj.core.api.Assertions.assertThatThrownBy; +import static org.mockito.ArgumentMatchers.any; +import static org.mockito.ArgumentMatchers.anyLong; +import static org.mockito.ArgumentMatchers.eq; +import static org.mockito.Mockito.inOrder; +import static org.mockito.Mockito.lenient; +import static org.mockito.Mockito.never; +import static org.mockito.Mockito.verify; +import static org.mockito.Mockito.verifyNoInteractions; +import static org.mockito.Mockito.when; + +class RoleChangeServiceTest extends AbstractUnitTest { + + private static final long ACTOR = 1L; + private static final long TARGET = 2L; + + @Mock + private RoleChangeRepository repository; + + @Mock + private AdminAuditLog adminAuditLog; + + @Mock + private AccountStatePublisher accountStatePublisher; + + private RoleChangeService service; + + @BeforeEach + void setUp() { + service = new RoleChangeService(repository, adminAuditLog, accountStatePublisher); + lenient().when(repository.lockActiveAdmins()).thenReturn(List.of(ACTOR)); + target(Role.USER, false, false, "ACTIVE"); + } + + private void target(Role role, boolean blocked, boolean deleted, String status) { + target(role, blocked, deleted, status, null); + } + + private void target(Role role, boolean blocked, boolean deleted, String status, Instant roleChangedAt) { + lenient().when(repository.lockTarget(TARGET)) + .thenReturn(Optional.of(new RoleTarget(role, blocked, deleted, status, roleChangedAt))); + } + + private String refusal(Role expectedRole, Role newRole) { + try { + service.changeRole(ACTOR, TARGET, expectedRole, newRole); + } catch (AdminActionRefusedException e) { + return e.getCode(); + } + throw new AssertionError("not refused"); + } + + private void verifyNothingWritten() { + verify(repository, never()).updateRole(anyLong(), any(), any()); + verify(repository, never()).insertChange(anyLong(), anyLong(), any(), any(), any()); + verifyNoInteractions(adminAuditLog, accountStatePublisher); + } + + @Test + void grant_WritesTheRoleTheLogTheAuditAndTellsCore() { + service.changeRole(ACTOR, TARGET, Role.USER, Role.AUDITOR); + + Instant before = Instant.now(); + ArgumentCaptor validFrom = ArgumentCaptor.forClass(Instant.class); + verify(repository).updateRole(eq(TARGET), eq(Role.AUDITOR), validFrom.capture()); + verify(repository).insertChange(eq(ACTOR), eq(TARGET), eq(Role.USER), eq(Role.AUDITOR), any()); + assertThat(validFrom.getValue()).isEqualTo(validFrom.getValue().truncatedTo(ChronoUnit.SECONDS)) + .isAfter(before.minusSeconds(1)).isBeforeOrEqualTo(before.plusSeconds(2)); + verify(adminAuditLog).record(ACTOR, "ROLE_CHANGE", TARGET, "USER>AUDITOR"); + verify(accountStatePublisher).publish(TARGET); + } + + @Test + void secondChangeInTheSameSecond_IsValidFromAfterTheFirst() { + Instant previous = Instant.now().truncatedTo(ChronoUnit.SECONDS).plusSeconds(1); + target(Role.AUDITOR, false, false, "ACTIVE", previous); + + service.changeRole(ACTOR, TARGET, Role.AUDITOR, Role.USER); + + verify(repository).updateRole(TARGET, Role.USER, previous.plusSeconds(1)); + } + + @Test + void changeRole_LocksTheAdminsBeforeTheTarget() { + service.changeRole(ACTOR, TARGET, Role.USER, Role.AUDITOR); + + InOrder order = inOrder(repository); + order.verify(repository).lockActiveAdmins(); + order.verify(repository).lockTarget(TARGET); + order.verify(repository).updateRole(eq(TARGET), eq(Role.AUDITOR), any()); + } + + @Test + void ownRole_IsRefusedBeforeAnyLock() { + assertThatThrownBy(() -> service.changeRole(ACTOR, ACTOR, Role.PLATFORM_ADMIN, Role.USER)) + .isInstanceOfSatisfying(AdminActionRefusedException.class, e -> assertThat(e.getCode()).isEqualTo("SELF_ACTION")); + + verifyNoInteractions(repository); + verifyNothingWritten(); + } + + @Test + void actorWhoIsNoAdminAnyMore_IsRefused() { + when(repository.lockActiveAdmins()).thenReturn(List.of(9L)); + + assertThat(refusal(Role.USER, Role.AUDITOR)).isEqualTo("ACTOR_NOT_ADMIN"); + verifyNothingWritten(); + } + + @Test + void staleExpectedRole_IsRefused() { + assertThat(refusal(Role.AUDITOR, Role.PLATFORM_ADMIN)).isEqualTo("ROLE_CHANGED"); + verifyNothingWritten(); + } + + @Test + void deletedBlockedPendingAndUnconfirmedTargets_AreRefused() { + target(Role.USER, false, true, "ACTIVE"); + assertThat(refusal(Role.USER, Role.AUDITOR)).isEqualTo("ALREADY_DELETED"); + target(Role.USER, true, false, "ACTIVE"); + assertThat(refusal(Role.USER, Role.AUDITOR)).isEqualTo("TARGET_IS_BLOCKED"); + target(Role.USER, false, false, "PENDING"); + assertThat(refusal(Role.USER, Role.AUDITOR)).isEqualTo("TARGET_IS_PENDING"); + target(Role.USER, false, false, "UNCONFIRMED"); + assertThat(refusal(Role.USER, Role.AUDITOR)).isEqualTo("TARGET_IS_UNCONFIRMED"); + + verifyNothingWritten(); + } + + @Test + void unknownTarget_IsNotFound() { + when(repository.lockTarget(TARGET)).thenReturn(Optional.empty()); + + assertThatThrownBy(() -> service.changeRole(ACTOR, TARGET, Role.USER, Role.AUDITOR)) + .isInstanceOf(TargetUserNotFoundException.class); + verifyNothingWritten(); + } + + @Test + void sameRole_WritesNothing() { + service.changeRole(ACTOR, TARGET, Role.USER, Role.USER); + + verifyNothingWritten(); + } + + @Test + void findChanges_ReadsTheLog() { + List log = List.of(new RoleChange(ACTOR, "a@example.com", Role.USER, Role.AUDITOR, Instant.now())); + when(repository.findChanges(TARGET, 50)).thenReturn(log); + + assertThat(service.findChanges(TARGET)).isEqualTo(log); + } +} diff --git a/src/test/java/auth_service/modules/login/service/LoginServiceTest.java b/src/test/java/auth_service/modules/login/service/LoginServiceTest.java index 2f71196..4d7748c 100644 --- a/src/test/java/auth_service/modules/login/service/LoginServiceTest.java +++ b/src/test/java/auth_service/modules/login/service/LoginServiceTest.java @@ -1,7 +1,5 @@ package auth_service.modules.login.service; -import auth_service.shared.security.Role; -import auth_service.shared.security.TokenGenerator; import auth_service.modules.login.exception.AccountBlockedException; import auth_service.modules.login.exception.AccountPendingException; import auth_service.modules.login.exception.InvalidCredentialsException; @@ -47,9 +45,6 @@ class LoginServiceTest extends AbstractUnitTest { @Mock private PasswordEncoder passwordEncoder; - @Mock - private TokenGenerator tokenGenerator; - @Mock private LoginAttemptService loginAttemptService; @@ -65,8 +60,8 @@ class LoginServiceTest extends AbstractUnitTest { @BeforeEach void setUp() { lenient().when(passwordEncoder.encode(anyString())).thenReturn("dummy-hash"); - loginService = new LoginService(minimalUserRepository, passwordEncoder, tokenGenerator, loginAttemptService, - refreshTokenService, cacheManager); + loginService = new LoginService(minimalUserRepository, passwordEncoder, loginAttemptService, refreshTokenService, + cacheManager); user = User.builder() .email("test@example.com") .password("password") @@ -84,8 +79,7 @@ class LoginServiceTest extends AbstractUnitTest { .build(); when(minimalUserRepository.findByEmail(user.getEmail())).thenReturn(Optional.of(dbUser)); when(passwordEncoder.matches(user.getPassword(), dbUser.getPassword())).thenReturn(true); - when(tokenGenerator.generateToken("815", null, Role.USER)).thenReturn("mock-token"); - when(refreshTokenService.startFamily(815L, dbUser.getPassword())).thenReturn("mock-refresh"); + when(refreshTokenService.startSession(815L, dbUser.getPassword())).thenReturn(new Session("mock-token", "mock-refresh")); // Act var result = loginService.login(user, IP, null); @@ -176,7 +170,7 @@ class LoginServiceTest extends AbstractUnitTest { // Act & Assert assertThrows(AccountBlockedException.class, () -> loginService.login(user, IP, null)); - verify(tokenGenerator, never()).generateToken(any(), any(), any()); + verify(refreshTokenService, never()).startSession(anyLong(), any()); verify(loginAttemptService).recordSuccess("test@example.com", IP); } @@ -188,7 +182,7 @@ class LoginServiceTest extends AbstractUnitTest { when(passwordEncoder.matches(user.getPassword(), dbUser.getPassword())).thenReturn(true); assertThrows(AccountPendingException.class, () -> loginService.login(user, IP, null)); - verify(tokenGenerator, never()).generateToken(any(), any(), any()); + verify(refreshTokenService, never()).startSession(anyLong(), any()); verify(loginAttemptService).recordSuccess("test@example.com", IP); } @@ -198,7 +192,7 @@ class LoginServiceTest extends AbstractUnitTest { when(minimalUserRepository.findByEmail(user.getEmail())).thenReturn(Optional.of(dbUser)); assertThrows(InvalidCredentialsException.class, () -> loginService.login(user, IP, null)); - verify(tokenGenerator, never()).generateToken(any(), any(), any()); + verify(refreshTokenService, never()).startSession(anyLong(), any()); } @Test @@ -220,18 +214,6 @@ class LoginServiceTest extends AbstractUnitTest { assertThrows(InvalidCredentialsException.class, () -> loginService.login(user, IP, null)); } - @Test - void login_TheTokenCarriesTheStoredRole() { - var dbUser = User.builder().id(815L).email("test@example.com").password("encodedPassword").enabled(true) - .role(Role.PLATFORM_ADMIN).build(); - when(minimalUserRepository.findByEmail(user.getEmail())).thenReturn(Optional.of(dbUser)); - when(passwordEncoder.matches(user.getPassword(), dbUser.getPassword())).thenReturn(true); - - loginService.login(user, IP, null); - - verify(tokenGenerator).generateToken("815", null, Role.PLATFORM_ADMIN); - } - @Test void login_TooManyAttempts() { // Arrange @@ -252,6 +234,6 @@ class LoginServiceTest extends AbstractUnitTest { assertThrows(InvalidCredentialsException.class, () -> loginService.login(user, IP, null)); - verify(refreshTokenService, never()).startFamily(anyLong(), any()); + verify(refreshTokenService, never()).startSession(anyLong(), any()); } } diff --git a/src/test/java/auth_service/shared/security/UserTokenStateTest.java b/src/test/java/auth_service/shared/security/UserTokenStateTest.java new file mode 100644 index 0000000..b8dc9a7 --- /dev/null +++ b/src/test/java/auth_service/shared/security/UserTokenStateTest.java @@ -0,0 +1,42 @@ +package auth_service.shared.security; + +import auth_service.shared.test.AbstractUnitTest; +import org.junit.jupiter.api.Test; + +import java.time.Instant; + +import static org.junit.jupiter.api.Assertions.assertFalse; +import static org.junit.jupiter.api.Assertions.assertTrue; + +class UserTokenStateTest extends AbstractUnitTest { + + private static final Instant CHANGE = Instant.parse("2026-10-04T12:00:00Z"); + + @Test + void activeWithoutRoleChange_AcceptsAnyToken() { + assertTrue(new UserTokenState(true, null).accepts(CHANGE)); + assertTrue(new UserTokenState(true, null).accepts(null)); + } + + @Test + void inactive_RefusesEveryToken() { + assertFalse(UserTokenState.INACTIVE.accepts(CHANGE)); + assertFalse(new UserTokenState(false, CHANGE).accepts(CHANGE.plusSeconds(60))); + } + + @Test + void tokenFromBeforeTheRoleChange_IsRefused() { + assertFalse(new UserTokenState(true, CHANGE).accepts(CHANGE.minusSeconds(1))); + } + + @Test + void tokenFromTheChangeSecondOrLater_IsAccepted() { + assertTrue(new UserTokenState(true, CHANGE).accepts(CHANGE)); + assertTrue(new UserTokenState(true, CHANGE).accepts(CHANGE.plusSeconds(1))); + } + + @Test + void tokenWithoutIssueTime_IsRefusedAfterARoleChange() { + assertFalse(new UserTokenState(true, CHANGE).accepts(null)); + } +} diff --git a/src/test/resources/contract/auth-core-contract.json b/src/test/resources/contract/auth-core-contract.json index 87bbb4f..c24e2a8 100644 --- a/src/test/resources/contract/auth-core-contract.json +++ b/src/test/resources/contract/auth-core-contract.json @@ -37,12 +37,12 @@ "applyStates": { "method": "POST", "path": "/internal/accounts/states", - "request": {"accounts": [{"userId": 42, "steamId": 76561198000000001, "displayName": "lobo", "blocked": true}]} + "request": {"accounts": [{"userId": 42, "steamId": 76561198000000001, "displayName": "lobo", "blocked": true, "roleChangedAt": "2026-10-04T12:00:00Z"}]} }, "replaceStates": { "method": "PUT", "path": "/internal/accounts/states", - "request": {"accounts": [{"userId": 42, "steamId": null, "displayName": "lobo", "blocked": false}]} + "request": {"accounts": [{"userId": 42, "steamId": null, "displayName": "lobo", "blocked": false, "roleChangedAt": null}]} }, "purge": { "method": "POST",