From d3c831f935cdc5250748ead98027c88cf82a09c0 Mon Sep 17 00:00:00 2001 From: Peter Hoffmann <954078+p-hoffmann@users.noreply.github.com> Date: Sun, 26 Apr 2026 12:34:06 +0800 Subject: [PATCH 1/2] add oidc token exchange endpoint --- .../webapi/security/authc/OidcAuthConfig.java | 179 ++++++++++++++---- .../security/authz/AuthorizationService.java | 67 +++++++ 2 files changed, 212 insertions(+), 34 deletions(-) diff --git a/src/main/java/org/ohdsi/webapi/security/authc/OidcAuthConfig.java b/src/main/java/org/ohdsi/webapi/security/authc/OidcAuthConfig.java index 83eacfbe4..d5bf1d8a3 100644 --- a/src/main/java/org/ohdsi/webapi/security/authc/OidcAuthConfig.java +++ b/src/main/java/org/ohdsi/webapi/security/authc/OidcAuthConfig.java @@ -16,6 +16,8 @@ import org.springframework.context.annotation.Bean; import org.springframework.context.annotation.Configuration; import org.springframework.core.annotation.Order; +import org.springframework.http.HttpStatus; +import org.springframework.http.ResponseEntity; import org.springframework.security.authentication.UsernamePasswordAuthenticationToken; import org.springframework.security.config.annotation.web.builders.HttpSecurity; import org.springframework.security.core.Authentication; @@ -25,7 +27,17 @@ import org.springframework.security.oauth2.client.registration.ClientRegistrations; import org.springframework.security.oauth2.client.registration.InMemoryClientRegistrationRepository; import org.springframework.security.oauth2.core.oidc.user.OidcUser; +import org.springframework.security.oauth2.jwt.Jwt; +import org.springframework.security.oauth2.jwt.JwtDecoder; +import org.springframework.security.oauth2.jwt.JwtException; +import org.springframework.security.oauth2.jwt.NimbusJwtDecoder; + +import com.nimbusds.jose.JOSEObjectType; +import com.nimbusds.jose.proc.DefaultJOSEObjectTypeVerifier; import org.springframework.security.web.SecurityFilterChain; +import org.springframework.web.bind.annotation.GetMapping; +import org.springframework.web.bind.annotation.RequestHeader; +import org.springframework.web.bind.annotation.RestController; import jakarta.servlet.http.HttpServletRequest; import jakarta.servlet.http.HttpServletResponse; @@ -102,6 +114,22 @@ public ClientRegistrationRepository oidcClientRegistrationRepository() { return new InMemoryClientRegistrationRepository(builder.build()); } + /** + * Permits {@code /user/login/openidDirect} through Spring Security so the + * controller below can validate the bearer token itself. The browser-flow + * chain at @Order(1) matches the exact path {@code /user/login/openid}, so + * it does not capture the {@code openidDirect} variant. + */ + @Bean + @Order(0) + public SecurityFilterChain oidcDirectAuthChain(HttpSecurity http) throws Exception { + httpSecurityShared.configureDefaults(http); + http + .securityMatcher("/user/login/openidDirect") + .authorizeHttpRequests(auth -> auth.anyRequest().permitAll()); + return http.build(); + } + @Bean @Order(1) public SecurityFilterChain oidcAuthChain(HttpSecurity http) throws Exception { @@ -133,11 +161,19 @@ private void handleSuccess(HttpServletRequest request, List filteredDefaults = defaultRoles.stream().filter(s -> !s.isBlank()).toList(); authorizationService.ensureUserExists(login, name, UserOrigin.OIDC, filteredDefaults); - List idpRoles = extractRoles(oidcUser.getClaims(), rolesClaim, rolesToUpperCase); + // Auto-filter: keep only role names that match an existing WebAPI sec_role. + // Logto seeds scopes whose names line up with WebAPI role names; anything that + // doesn't match (granular permissions, unknown roles) is silently dropped. + List rawRoles = extractRoles(oidcUser.getClaims(), rolesClaim, rolesToUpperCase); + List idpRoles = authorizationService.filterToExistingRoles(rawRoles); + if (rawRoles.size() != idpRoles.size()) { + log.debug("OIDC: dropped {} unknown roles from token for user {}", + rawRoles.size() - idpRoles.size(), login); + } if (!idpRoles.isEmpty()) { log.info("OIDC: Syncing roles from token for user {}: {}", login, idpRoles); - syncRoles(login, idpRoles); } + authorizationService.syncOidcRoles(login, idpRoles); List authorities = idpRoles.stream() .map(SimpleGrantedAuthority::new) @@ -149,38 +185,6 @@ private void handleSuccess(HttpServletRequest request, response.sendRedirect(appendFragmentParam(callbackUi, "token", result.jwt())); } - private void syncRoles(String login, List idpRoles) { - List currentOidcRoleNames; - try { - currentOidcRoleNames = authorizationService.getOidcOriginRoles(login); - } catch (Exception e) { - log.warn("OIDC: Could not fetch OIDC-origin roles for user {}: {}", login, e.getMessage()); - return; - } - - for (String roleName : idpRoles) { - if (!currentOidcRoleNames.contains(roleName)) { - try { - authorizationService.addUserToRole(roleName, login, UserOrigin.OIDC); - log.info("OIDC: Added role '{}' to user '{}'", roleName, login); - } catch (Exception e) { - log.warn("OIDC: Could not add role '{}' to user '{}': {}", roleName, login, e.getMessage()); - } - } - } - - for (String roleName : currentOidcRoleNames) { - if (!idpRoles.contains(roleName)) { - try { - authorizationService.removeUserFromRole(roleName, login, UserOrigin.OIDC); - log.info("OIDC: Removed role '{}' from user '{}'", roleName, login); - } catch (Exception e) { - log.warn("OIDC: Could not remove role '{}' from user '{}': {}", roleName, login, e.getMessage()); - } - } - } - } - private Set buildScopes() { Set scopes = new LinkedHashSet<>(); scopes.add("openid"); @@ -291,4 +295,111 @@ static List extractRoles(Map claims, String claimPath, b log.warn("OIDC: Claim '{}' is not a list or string: {}", claimPath, current.getClass().getName()); return List.of(); } + + /** + * Service-to-service token-exchange endpoint. Accepts a Logto/OIDC bearer + * token in the {@code Authorization} header, validates it against the + * OIDC provider's JWKS, provisions/looks up the local user, and mints a + * WebAPI session JWT. The minted JWT is returned both in a {@code Bearer} + * response header (consumed by the trex auth proxy) and in the response + * body for callers that prefer JSON. + */ + @RestController + @ConditionalOnProperty(prefix = "security.auth.oidc", name = "enabled", havingValue = "true") + public static class OpenidDirect { + + private static final Logger log = LoggerFactory.getLogger(OpenidDirect.class); + + private final AuthorizationService authorizationService; + private final LoginService loginService; + private final JwtDecoder jwtDecoder; + private final List defaultRoles; + private final String rolesClaim; + private final boolean rolesToUpperCase; + + public OpenidDirect( + AuthorizationService authorizationService, + LoginService loginService, + @Value("${security.auth.oidc.url}") String discoveryOrIssuerUrl, + @Value("${security.auth.oidc.rolesClaim:}") String rolesClaim, + @Value("${security.auth.oidc.rolesToUpperCase:true}") boolean rolesToUpperCase, + @Value("${security.defaultRoles:}") List defaultRoles) { + this.authorizationService = authorizationService; + this.loginService = loginService; + String issuer = stripDiscoverySuffixStatic(discoveryOrIssuerUrl); + log.info("OIDC direct: building JwtDecoder for issuer {}", issuer); + // Accept both `JWT` and `at+jwt` (RFC 9068) header types — Logto and other + // providers tag access tokens as `at+jwt`, which Spring's default verifier rejects. + this.jwtDecoder = NimbusJwtDecoder.withIssuerLocation(issuer) + .jwtProcessorCustomizer(processor -> processor.setJWSTypeVerifier( + new DefaultJOSEObjectTypeVerifier<>( + JOSEObjectType.JWT, + new JOSEObjectType("at+jwt"), + null))) + .build(); + this.defaultRoles = defaultRoles.stream().filter(s -> !s.isBlank()).toList(); + this.rolesClaim = rolesClaim; + this.rolesToUpperCase = rolesToUpperCase; + } + + @GetMapping("/user/login/openidDirect") + public ResponseEntity login( + @RequestHeader(value = "Authorization", required = false) String auth) { + + if (auth == null || !auth.regionMatches(true, 0, "Bearer ", 0, 7)) { + return ResponseEntity.status(HttpStatus.UNAUTHORIZED) + .body(new LoginService.Result(null, null, null, "Missing Bearer token")); + } + String token = auth.substring(7).trim(); + + Jwt jwt; + try { + jwt = jwtDecoder.decode(token); + } catch (JwtException e) { + log.warn("OIDC direct: token validation failed: {}", e.getMessage()); + return ResponseEntity.status(HttpStatus.UNAUTHORIZED) + .body(new LoginService.Result(null, null, null, "Invalid token")); + } + + String login = jwt.getSubject().toLowerCase(); + String name = firstNonBlankStatic( + jwt.getClaimAsString("name"), + jwt.getClaimAsString("email"), + login); + + log.info("OIDC direct: authenticated user sub={}", login); + + authorizationService.ensureUserExists(login, name, UserOrigin.OIDC, defaultRoles); + + List rawRoles = extractRoles(jwt.getClaims(), rolesClaim, rolesToUpperCase); + List idpRoles = authorizationService.filterToExistingRoles(rawRoles); + authorizationService.syncOidcRoles(login, idpRoles); + + List authorities = idpRoles.stream() + .map(SimpleGrantedAuthority::new) + .toList(); + Authentication wrapped = new UsernamePasswordAuthenticationToken(login, null, authorities); + LoginService.Result result = loginService.mintSession(wrapped); + + return ResponseEntity.ok() + .header("Bearer", result.jwt()) + .body(result); + } + + private static String stripDiscoverySuffixStatic(String url) { + if (url == null || url.isBlank()) { + throw new IllegalStateException("security.auth.oidc.url must be configured when OIDC is enabled"); + } + String suffix = "/.well-known/openid-configuration"; + return url.endsWith(suffix) ? url.substring(0, url.length() - suffix.length()) : url; + } + + private static String firstNonBlankStatic(String... values) { + if (values == null) return null; + for (String v : values) { + if (v != null && !v.isBlank()) return v; + } + return null; + } + } } diff --git a/src/main/java/org/ohdsi/webapi/security/authz/AuthorizationService.java b/src/main/java/org/ohdsi/webapi/security/authz/AuthorizationService.java index 7af85000e..5690a1b4e 100644 --- a/src/main/java/org/ohdsi/webapi/security/authz/AuthorizationService.java +++ b/src/main/java/org/ohdsi/webapi/security/authz/AuthorizationService.java @@ -69,6 +69,73 @@ public Iterable getUsers() { return users; } + /** + * Reconciles the OIDC-origin role assignments of {@code login} with + * {@code targetRoleNames}: roles in the target set that the user lacks are + * added with origin {@link UserOrigin#OIDC}; existing OIDC-origin roles + * absent from the target set are removed. Roles assigned via other origins + * (SYSTEM, LDAP, …) are left untouched. Per-role failures are logged and + * swallowed so a single bad row can't break the whole sync. + */ + public void syncOidcRoles(String login, List targetRoleNames) { + org.slf4j.Logger log = org.slf4j.LoggerFactory.getLogger(AuthorizationService.class); + List currentOidcRoleNames; + try { + currentOidcRoleNames = getOidcOriginRoles(login); + } catch (Exception e) { + log.warn("OIDC sync: could not fetch OIDC-origin roles for user {}: {}", login, e.getMessage()); + return; + } + for (String roleName : targetRoleNames) { + if (!currentOidcRoleNames.contains(roleName)) { + try { + addUserToRole(roleName, login, UserOrigin.OIDC); + log.info("OIDC sync: added role '{}' to user '{}'", roleName, login); + } catch (Exception e) { + log.warn("OIDC sync: could not add role '{}' to user '{}': {}", roleName, login, e.getMessage()); + } + } + } + for (String roleName : currentOidcRoleNames) { + if (!targetRoleNames.contains(roleName)) { + try { + removeUserFromRole(roleName, login, UserOrigin.OIDC); + log.info("OIDC sync: removed role '{}' from user '{}'", roleName, login); + } catch (Exception e) { + log.warn("OIDC sync: could not remove role '{}' from user '{}': {}", roleName, login, e.getMessage()); + } + } + } + } + + /** + * Returns the subset of {@code roleNames} that match an existing non-personal + * (system) role in the WebAPI {@code sec_role} table. Names not matching any + * existing role are silently dropped — used by OIDC sync so unknown IdP-side + * roles are ignored rather than auto-created. + *

+ * Matching is case-insensitive (OIDC tokens commonly upper-case role names), + * but the WebAPI canonical name is returned so downstream sync hits the + * correct {@code sec_role} row. + */ + public List filterToExistingRoles(List roleNames) { + if (roleNames == null || roleNames.isEmpty()) { + return java.util.Collections.emptyList(); + } + java.util.Map canonicalByLower = new java.util.HashMap<>(); + for (RoleEntity r : this.roleService.getRoles(false)) { + canonicalByLower.put(r.getName().toLowerCase(), r.getName()); + } + java.util.ArrayList kept = new java.util.ArrayList<>(roleNames.size()); + for (String n : roleNames) { + String canonical = canonicalByLower.get(n.toLowerCase()); + if (canonical != null) { + kept.add(canonical); + } + } + return kept; + } + public User getCurrentUser() { WebApiPrincipal principal = getAuthenticatedPrincipal(); UserEntity ue = this.userService.getUserById(principal.getUserId()); From 0298d5e00aaca5ad2e77934468d7c0361a480ac1 Mon Sep 17 00:00:00 2001 From: Peter Hoffmann <954078+p-hoffmann@users.noreply.github.com> Date: Sun, 26 Apr 2026 12:48:09 +0800 Subject: [PATCH 2/2] cleanup --- .../webapi/security/authc/OidcAuthConfig.java | 17 ----------------- 1 file changed, 17 deletions(-) diff --git a/src/main/java/org/ohdsi/webapi/security/authc/OidcAuthConfig.java b/src/main/java/org/ohdsi/webapi/security/authc/OidcAuthConfig.java index d5bf1d8a3..9aa139538 100644 --- a/src/main/java/org/ohdsi/webapi/security/authc/OidcAuthConfig.java +++ b/src/main/java/org/ohdsi/webapi/security/authc/OidcAuthConfig.java @@ -114,12 +114,6 @@ public ClientRegistrationRepository oidcClientRegistrationRepository() { return new InMemoryClientRegistrationRepository(builder.build()); } - /** - * Permits {@code /user/login/openidDirect} through Spring Security so the - * controller below can validate the bearer token itself. The browser-flow - * chain at @Order(1) matches the exact path {@code /user/login/openid}, so - * it does not capture the {@code openidDirect} variant. - */ @Bean @Order(0) public SecurityFilterChain oidcDirectAuthChain(HttpSecurity http) throws Exception { @@ -161,9 +155,6 @@ private void handleSuccess(HttpServletRequest request, List filteredDefaults = defaultRoles.stream().filter(s -> !s.isBlank()).toList(); authorizationService.ensureUserExists(login, name, UserOrigin.OIDC, filteredDefaults); - // Auto-filter: keep only role names that match an existing WebAPI sec_role. - // Logto seeds scopes whose names line up with WebAPI role names; anything that - // doesn't match (granular permissions, unknown roles) is silently dropped. List rawRoles = extractRoles(oidcUser.getClaims(), rolesClaim, rolesToUpperCase); List idpRoles = authorizationService.filterToExistingRoles(rawRoles); if (rawRoles.size() != idpRoles.size()) { @@ -296,14 +287,6 @@ static List extractRoles(Map claims, String claimPath, b return List.of(); } - /** - * Service-to-service token-exchange endpoint. Accepts a Logto/OIDC bearer - * token in the {@code Authorization} header, validates it against the - * OIDC provider's JWKS, provisions/looks up the local user, and mints a - * WebAPI session JWT. The minted JWT is returned both in a {@code Bearer} - * response header (consumed by the trex auth proxy) and in the response - * body for callers that prefer JSON. - */ @RestController @ConditionalOnProperty(prefix = "security.auth.oidc", name = "enabled", havingValue = "true") public static class OpenidDirect {