From fbdecdf0d3ae5cb6f2a15909b8fcb2d08316848f Mon Sep 17 00:00:00 2001 From: Raphael Frank <04.raphael.frank@gmail.com> Date: Fri, 10 Jul 2026 13:41:04 +0200 Subject: [PATCH] fix scope grants after creating new members --- .../service/KeycloakService.java | 43 +++++++++++++++- .../service/KeycloakServiceTest.java | 49 +++++++++++++++++++ 2 files changed, 91 insertions(+), 1 deletion(-) diff --git a/services/spring-member/src/main/java/tum/devoops/memberservice/service/KeycloakService.java b/services/spring-member/src/main/java/tum/devoops/memberservice/service/KeycloakService.java index a3ea3a55..785eab4c 100644 --- a/services/spring-member/src/main/java/tum/devoops/memberservice/service/KeycloakService.java +++ b/services/spring-member/src/main/java/tum/devoops/memberservice/service/KeycloakService.java @@ -15,6 +15,7 @@ import java.net.URI; import java.time.Instant; +import java.util.Arrays; import java.util.List; import java.util.UUID; @@ -23,6 +24,10 @@ public class KeycloakService { private static final long TOKEN_REFRESH_SKEW_SECONDS = 30L; + // Baseline realm role every member needs; without it, hasAnyRole('member', 'admin') + // rejects them with 403 on first login even though their Keycloak account exists. + private static final String MEMBER_ROLE_NAME = "member"; + private final RestClient restClient; private final String realm; private final String serviceAccountClientId; @@ -79,7 +84,41 @@ public UUID createUser(MemberCreate member) throws Exception { } String path = location.getPath(); - return UUID.fromString(path.substring(path.lastIndexOf("/") + 1)); + UUID userId = UUID.fromString(path.substring(path.lastIndexOf("/") + 1)); + + assignMemberRealmRole(userId); + + return userId; + } + + // Looked up via the user's own "available realm roles" endpoint rather than the + // realm-wide /roles/{role-name} resource: the org-role-sync service account only + // holds the manage-users client role (see realm-config.json), which covers a + // user's own role-mappings sub-resource but not the separate view-realm permission + // that GET /roles/{role-name} would require. + private void assignMemberRealmRole(UUID userId) { + try { + RoleRepresentation[] availableRoles = restClient.get() + .uri("/admin/realms/{realm}/users/{id}/role-mappings/realm/available", realm, userId) + .header(HttpHeaders.AUTHORIZATION, authorizationHeader()) + .retrieve() + .body(RoleRepresentation[].class); + + RoleRepresentation memberRole = Arrays.stream(availableRoles != null ? availableRoles : new RoleRepresentation[0]) + .filter(role -> MEMBER_ROLE_NAME.equals(role.name())) + .findFirst() + .orElseThrow(() -> new IllegalStateException("Keycloak realm role not found: " + MEMBER_ROLE_NAME)); + + restClient.post() + .uri("/admin/realms/{realm}/users/{id}/role-mappings/realm", realm, userId) + .header(HttpHeaders.AUTHORIZATION, authorizationHeader()) + .contentType(MediaType.APPLICATION_JSON) + .body(List.of(memberRole)) + .retrieve() + .toBodilessEntity(); + } catch (HttpClientErrorException.Forbidden e) { + throw new SecurityException("Insufficient permissions to assign the member realm role"); + } } public void updateUser(Member member) throws HttpClientErrorException { @@ -181,6 +220,8 @@ private record UserUpdateRepresentation( private record Credential(String type, String value, boolean temporary) {} + private record RoleRepresentation(String id, String name) {} + private record TokenResponse( @JsonProperty("access_token") String accessToken, @JsonProperty("expires_in") Long expiresIn diff --git a/services/spring-member/src/test/java/tum/devoops/memberservice/service/KeycloakServiceTest.java b/services/spring-member/src/test/java/tum/devoops/memberservice/service/KeycloakServiceTest.java index 0f9d1fcb..7a346b34 100644 --- a/services/spring-member/src/test/java/tum/devoops/memberservice/service/KeycloakServiceTest.java +++ b/services/spring-member/src/test/java/tum/devoops/memberservice/service/KeycloakServiceTest.java @@ -31,6 +31,7 @@ class KeycloakServiceTest { private static final String SERVICE_ACCOUNT_TOKEN = "service-account-token"; private static final String TOKEN_URI = BASE_URL + "/realms/" + REALM + "/protocol/openid-connect/token"; private static final String USERS_URI = BASE_URL + "/admin/realms/" + REALM + "/users"; + private static final String MEMBER_ROLE_ID = "member-role-id"; private MockRestServiceServer server; private KeycloakService keycloakService; @@ -68,6 +69,7 @@ void setUp() { } // Verifies that a successful creation returns the id parsed from the Location header + // and assigns the "member" realm role so the account isn't stuck at 403 on first login @Test void createUserReturnsIdFromLocationHeader() throws Exception { expectServiceAccountTokenRequest(); @@ -75,6 +77,7 @@ void createUserReturnsIdFromLocationHeader() throws Exception { .andExpect(header(HttpHeaders.AUTHORIZATION, "Bearer " + SERVICE_ACCOUNT_TOKEN)) .andRespond(withStatus(HttpStatus.CREATED) .header(HttpHeaders.LOCATION, USERS_URI + "/" + id)); + expectMemberRoleAssignment(id); UUID result = keycloakService.createUser(memberCreate); @@ -91,12 +94,45 @@ void createUserWithoutEmailReturnsId() throws Exception { .andExpect(header(HttpHeaders.AUTHORIZATION, "Bearer " + SERVICE_ACCOUNT_TOKEN)) .andRespond(withStatus(HttpStatus.CREATED) .header(HttpHeaders.LOCATION, USERS_URI + "/" + id)); + expectMemberRoleAssignment(id); UUID result = keycloakService.createUser(memberCreate); assertEquals(id, result); } + // Verifies that a 403 while assigning the member realm role is translated into a SecurityException + @Test + void createUserThrowsOnForbiddenRoleAssignment() { + expectServiceAccountTokenRequest(); + server.expect(requestTo(USERS_URI)) + .andExpect(header(HttpHeaders.AUTHORIZATION, "Bearer " + SERVICE_ACCOUNT_TOKEN)) + .andRespond(withStatus(HttpStatus.CREATED) + .header(HttpHeaders.LOCATION, USERS_URI + "/" + id)); + server.expect(requestTo(USERS_URI + "/" + id + "/role-mappings/realm/available")) + .andExpect(header(HttpHeaders.AUTHORIZATION, "Bearer " + SERVICE_ACCOUNT_TOKEN)) + .andRespond(withStatus(HttpStatus.FORBIDDEN)); + + assertThrows(SecurityException.class, () -> keycloakService.createUser(memberCreate)); + } + + // Verifies that a missing "member" realm role fails loudly instead of silently leaving the user unassigned + @Test + void createUserThrowsWhenMemberRoleNotFound() { + expectServiceAccountTokenRequest(); + server.expect(requestTo(USERS_URI)) + .andExpect(header(HttpHeaders.AUTHORIZATION, "Bearer " + SERVICE_ACCOUNT_TOKEN)) + .andRespond(withStatus(HttpStatus.CREATED) + .header(HttpHeaders.LOCATION, USERS_URI + "/" + id)); + server.expect(requestTo(USERS_URI + "/" + id + "/role-mappings/realm/available")) + .andExpect(header(HttpHeaders.AUTHORIZATION, "Bearer " + SERVICE_ACCOUNT_TOKEN)) + .andRespond(withStatus(HttpStatus.OK) + .contentType(MediaType.APPLICATION_JSON) + .body("[]")); + + assertThrows(IllegalStateException.class, () -> keycloakService.createUser(memberCreate)); + } + // Verifies that a 409 conflict is translated into an IllegalAccessException @Test void createUserThrowsOnConflict() { @@ -196,6 +232,19 @@ void deleteUserThrowsOnForbidden() { assertThrows(SecurityException.class, () -> keycloakService.deleteUser(id)); } + private void expectMemberRoleAssignment(UUID userId) { + server.expect(requestTo(USERS_URI + "/" + userId + "/role-mappings/realm/available")) + .andExpect(header(HttpHeaders.AUTHORIZATION, "Bearer " + SERVICE_ACCOUNT_TOKEN)) + .andRespond(withStatus(HttpStatus.OK) + .contentType(MediaType.APPLICATION_JSON) + .body("[{\"id\":\"" + MEMBER_ROLE_ID + "\",\"name\":\"member\"}]")); + + server.expect(requestTo(USERS_URI + "/" + userId + "/role-mappings/realm")) + .andExpect(header(HttpHeaders.AUTHORIZATION, "Bearer " + SERVICE_ACCOUNT_TOKEN)) + .andExpect(content().json("[{\"id\":\"" + MEMBER_ROLE_ID + "\",\"name\":\"member\"}]")) + .andRespond(withStatus(HttpStatus.NO_CONTENT)); + } + private void expectServiceAccountTokenRequest() { MultiValueMap form = new LinkedMultiValueMap<>(); form.add("grant_type", "client_credentials");