From 3b40b3bfd66d8527d746bfe7328a91aec123fb5c Mon Sep 17 00:00:00 2001 From: Peter Hoffmann <954078+p-hoffmann@users.noreply.github.com> Date: Sun, 30 Aug 2026 08:00:44 +0800 Subject: [PATCH 1/2] Allow OIDC metadata to be read from an internal address An issuer served through a gateway is often unreachable under that name from inside the deployment, which leaves the client registration unable to build and takes the whole context down with it. security.auth.oidc.internalUrl supplies an address that does resolve; discovery and the signing keys are read there while the issuer, and the endpoints a browser is sent to, stay public. Blank keeps the previous behaviour of resolving everything from the issuer. --- .../webapi/security/authc/OidcAuthConfig.java | 132 ++++++++++++++++-- .../authc/OidcAuthConfigInternalUrlTest.java | 126 +++++++++++++++++ 2 files changed, 247 insertions(+), 11 deletions(-) create mode 100644 src/test/java/org/ohdsi/webapi/security/authc/OidcAuthConfigInternalUrlTest.java 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 0d1cdb8e9..681b54a90 100644 --- a/src/main/java/org/ohdsi/webapi/security/authc/OidcAuthConfig.java +++ b/src/main/java/org/ohdsi/webapi/security/authc/OidcAuthConfig.java @@ -2,6 +2,10 @@ import java.io.IOException; import java.net.URI; +import java.net.http.HttpClient; +import java.net.http.HttpRequest; +import java.net.http.HttpResponse; +import java.time.Duration; import java.util.ArrayList; import java.util.LinkedHashSet; import java.util.List; @@ -26,8 +30,11 @@ 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.JwtValidators; import org.springframework.security.oauth2.jwt.NimbusJwtDecoder; +import com.fasterxml.jackson.core.type.TypeReference; +import com.fasterxml.jackson.databind.ObjectMapper; import com.nimbusds.jose.JOSEObjectType; import com.nimbusds.jose.proc.DefaultJOSEObjectTypeVerifier; import org.springframework.security.web.SecurityFilterChain; @@ -62,6 +69,15 @@ public class OidcAuthConfig { @Value("${security.auth.oidc.externalUrl:}") private String externalUrl; + // Base URL that serves the same provider as the issuer but is reachable from + // inside the deployment. The issuer a provider advertises has to stay the one + // its tokens carry, which in a split-horizon deployment is a public address the + // server itself cannot resolve; discovery and JWKS are then fetched here while + // the public issuer remains what tokens are validated against. Blank keeps the + // plain behaviour of resolving everything from the issuer. + @Value("${security.auth.oidc.internalUrl:}") + private String internalUrl; + @Value("${security.auth.oidc.extraScopes:}") private String extraScopes; @@ -96,9 +112,18 @@ public ClientRegistrationRepository oidcClientRegistrationRepository() { return registrationId -> null; } String issuer = stripDiscoverySuffix(discoveryOrIssuerUrl); - log.info("OIDC: Discovering provider metadata from issuer {}", issuer); - - ClientRegistration.Builder builder = ClientRegistrations.fromIssuerLocation(issuer) + String internalBase = normaliseBase(internalUrl); + + ClientRegistration.Builder builder; + if (internalBase == null) { + log.info("OIDC: Discovering provider metadata from issuer {}", issuer); + builder = ClientRegistrations.fromIssuerLocation(issuer); + } else { + log.info("OIDC: Discovering provider metadata for issuer {} via internal base {}", issuer, internalBase); + builder = ClientRegistrations.fromOidcConfiguration( + internalConfiguration(issuer, internalBase)); + } + builder .registrationId(REGISTRATION_ID) .clientId(clientId) .clientSecret(clientSecret) @@ -210,6 +235,68 @@ private static String appendFragmentParam(String url, String key, String value) return base + "#" + fragment + separator + key + "=" + value; } + // Fetches the discovery document from the internally reachable base and points + // the endpoints the server itself calls back at that base. The issuer and the + // endpoints a browser is sent to are deliberately left on their public + // addresses: rewriting those would put an unreachable host in front of the user + // and break issuer validation. + private static Map internalConfiguration(String issuer, String internalBase) { + Map configuration = fetchConfiguration(internalBase + DISCOVERY_SUFFIX); + for (String endpoint : List.of("token_endpoint", "jwks_uri", "userinfo_endpoint")) { + Object value = configuration.get(endpoint); + if (value instanceof String url) { + configuration.put(endpoint, toInternal(url, issuer, internalBase)); + } + } + return configuration; + } + + private static Map fetchConfiguration(String url) { + try { + HttpResponse response = HttpClient.newBuilder() + // Ask for HTTP/1.1 outright. The default attempts an HTTP/2 upgrade, + // which a provider that only speaks 1.1 answers with a 502 rather than + // by declining the upgrade -- discovery then fails against a provider + // that is serving the document perfectly well. + .version(HttpClient.Version.HTTP_1_1) + .connectTimeout(Duration.ofSeconds(10)) + .build() + .send(HttpRequest.newBuilder(URI.create(url)) + .timeout(Duration.ofSeconds(10)) + .GET() + .build(), + HttpResponse.BodyHandlers.ofString()); + if (response.statusCode() != 200) { + throw new IllegalStateException( + "OIDC discovery at " + url + " returned HTTP " + response.statusCode()); + } + return new ObjectMapper().readValue(response.body(), new TypeReference>() {}); + } catch (IOException e) { + throw new IllegalStateException("Could not read OIDC discovery from " + url, e); + } catch (InterruptedException e) { + Thread.currentThread().interrupt(); + throw new IllegalStateException("Interrupted reading OIDC discovery from " + url, e); + } + } + + private static String toInternal(String url, String issuer, String internalBase) { + return url.startsWith(issuer) ? internalBase + url.substring(issuer.length()) : url; + } + + private static String normaliseBase(String url) { + if (url == null || url.isBlank()) { + return null; + } + String base = url.trim(); + if (base.endsWith(DISCOVERY_SUFFIX)) { + base = base.substring(0, base.length() - DISCOVERY_SUFFIX.length()); + } + while (base.endsWith("/")) { + base = base.substring(0, base.length() - 1); + } + return base; + } + private String stripDiscoverySuffix(String url) { if (url == null || url.isBlank()) { throw new IllegalStateException("security.auth.oidc.url must be configured when OIDC is enabled"); @@ -299,6 +386,7 @@ public OpenidDirect( org.ohdsi.webapi.security.authc.mapper.OidcGroupToRoleMapper oidcGroupToRoleMapper, @Value("${security.auth.oidc.enabled:false}") boolean enabled, @Value("${security.auth.oidc.url}") String discoveryOrIssuerUrl, + @Value("${security.auth.oidc.internalUrl:}") String internalUrl, @Value("${security.auth.oidc.rolesClaim:}") String rolesClaim, @Value("${security.auth.oidc.rolesToUpperCase:true}") boolean rolesToUpperCase) { this.loginService = loginService; @@ -311,16 +399,38 @@ public OpenidDirect( return; } String issuer = stripDiscoverySuffixStatic(discoveryOrIssuerUrl); - log.info("OIDC direct: building JwtDecoder for issuer {}", issuer); + String internalBase = normaliseBase(internalUrl); // 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(); + NimbusJwtDecoder decoder; + if (internalBase == null) { + log.info("OIDC direct: building JwtDecoder for issuer {}", issuer); + decoder = NimbusJwtDecoder.withIssuerLocation(issuer) + .jwtProcessorCustomizer(processor -> processor.setJWSTypeVerifier( + new DefaultJOSEObjectTypeVerifier<>( + JOSEObjectType.JWT, + new JOSEObjectType("at+jwt"), + null))) + .build(); + } else { + // Same split-horizon concession as the registration repository: the keys + // are fetched over the internal base, but tokens are still only accepted + // when they carry the public issuer. + Object jwks = internalConfiguration(issuer, internalBase).get("jwks_uri"); + if (!(jwks instanceof String jwkSetUri)) { + throw new IllegalStateException("OIDC discovery at " + internalBase + " advertised no jwks_uri"); + } + log.info("OIDC direct: building JwtDecoder for issuer {} with keys from {}", issuer, jwkSetUri); + decoder = NimbusJwtDecoder.withJwkSetUri(jwkSetUri) + .jwtProcessorCustomizer(processor -> processor.setJWSTypeVerifier( + new DefaultJOSEObjectTypeVerifier<>( + JOSEObjectType.JWT, + new JOSEObjectType("at+jwt"), + null))) + .build(); + decoder.setJwtValidator(JwtValidators.createDefaultWithIssuer(issuer)); + } + this.jwtDecoder = decoder; } @GetMapping("/user/login/openidDirect") diff --git a/src/test/java/org/ohdsi/webapi/security/authc/OidcAuthConfigInternalUrlTest.java b/src/test/java/org/ohdsi/webapi/security/authc/OidcAuthConfigInternalUrlTest.java new file mode 100644 index 000000000..a5ae579e4 --- /dev/null +++ b/src/test/java/org/ohdsi/webapi/security/authc/OidcAuthConfigInternalUrlTest.java @@ -0,0 +1,126 @@ +package org.ohdsi.webapi.security.authc; + +import static org.junit.Assert.assertEquals; +import static org.junit.Assert.assertNotNull; +import static org.junit.Assert.assertTrue; + +import com.sun.net.httpserver.HttpServer; +import java.io.OutputStream; +import java.net.InetSocketAddress; +import java.nio.charset.StandardCharsets; +import org.junit.After; +import org.junit.Before; +import org.junit.Test; +import org.springframework.security.oauth2.client.registration.ClientRegistration; +import org.springframework.security.oauth2.client.registration.ClientRegistrationRepository; +import org.springframework.test.util.ReflectionTestUtils; + +/** + * A provider whose issuer is a public address the server itself cannot resolve is + * the normal case behind a gateway. These cover that shape: metadata is read over + * the internal base, the endpoints the server calls follow it there, and both the + * issuer and the endpoint a browser is sent to stay public. + */ +public class OidcAuthConfigInternalUrlTest { + + private static final String PUBLIC_ISSUER = "https://gateway.invalid:41100/trex/oidc"; + + private HttpServer server; + private String internalBase; + + @Before + public void startProvider() throws Exception { + server = HttpServer.create(new InetSocketAddress("127.0.0.1", 0), 0); + internalBase = "http://127.0.0.1:" + server.getAddress().getPort() + "/trex/oidc"; + server.createContext("/trex/oidc/.well-known/openid-configuration", exchange -> { + byte[] body = ("{" + + "\"issuer\":\"" + PUBLIC_ISSUER + "\"," + + "\"authorization_endpoint\":\"" + PUBLIC_ISSUER + "/authorize\"," + + "\"token_endpoint\":\"" + PUBLIC_ISSUER + "/token\"," + + "\"userinfo_endpoint\":\"" + PUBLIC_ISSUER + "/userinfo\"," + + "\"jwks_uri\":\"" + PUBLIC_ISSUER + "/.well-known/jwks.json\"," + + "\"end_session_endpoint\":\"" + PUBLIC_ISSUER + "/session/end\"," + + "\"response_types_supported\":[\"code\"]," + + "\"grant_types_supported\":[\"authorization_code\"]," + + "\"subject_types_supported\":[\"public\"]," + + "\"id_token_signing_alg_values_supported\":[\"RS256\"]" + + "}").getBytes(StandardCharsets.UTF_8); + exchange.getResponseHeaders().add("Content-Type", "application/json"); + exchange.sendResponseHeaders(200, body.length); + try (OutputStream out = exchange.getResponseBody()) { + out.write(body); + } + }); + server.start(); + } + + @After + public void stopProvider() { + server.stop(0); + } + + private ClientRegistration buildRegistration(String externalUrl) { + OidcAuthConfig config = new OidcAuthConfig(null, null, null); + ReflectionTestUtils.setField(config, "oidcRuntimeEnabled", true); + ReflectionTestUtils.setField(config, "clientId", "d2e-webapi"); + ReflectionTestUtils.setField(config, "clientSecret", "secret"); + ReflectionTestUtils.setField(config, "callbackApi", "https://gateway.invalid:41100/WebAPI/user/oauth/callback"); + ReflectionTestUtils.setField(config, "callbackUi", "https://gateway.invalid:41100/atlas"); + ReflectionTestUtils.setField(config, "extraScopes", ""); + ReflectionTestUtils.setField(config, "discoveryOrIssuerUrl", PUBLIC_ISSUER); + ReflectionTestUtils.setField(config, "internalUrl", internalBase); + ReflectionTestUtils.setField(config, "externalUrl", externalUrl); + + ClientRegistrationRepository repository = config.oidcClientRegistrationRepository(); + ClientRegistration registration = repository.findByRegistrationId("openid"); + assertNotNull("registration was not built", registration); + return registration; + } + + @Test + public void readsMetadataOverTheInternalBaseWithoutTouchingTheIssuerHost() { + ClientRegistration registration = buildRegistration(""); + + // The issuer is what tokens carry, so it has to survive verbatim even though + // nothing was ever fetched from it. + assertEquals(PUBLIC_ISSUER, registration.getProviderDetails().getIssuerUri()); + } + + @Test + public void movesServerSideEndpointsOntoTheInternalBase() { + ClientRegistration.ProviderDetails details = buildRegistration("").getProviderDetails(); + + assertEquals(internalBase + "/token", details.getTokenUri()); + assertEquals(internalBase + "/.well-known/jwks.json", details.getJwkSetUri()); + assertEquals(internalBase + "/userinfo", details.getUserInfoEndpoint().getUri()); + } + + @Test + public void keepsTheBrowserFacingEndpointPublic() { + ClientRegistration.ProviderDetails details = buildRegistration("").getProviderDetails(); + + assertTrue( + "a browser must not be sent to an internal address, but got " + details.getAuthorizationUri(), + details.getAuthorizationUri().startsWith(PUBLIC_ISSUER)); + } + + @Test + public void leavesTheEndSessionEndpointPublic() { + ClientRegistration.ProviderDetails details = buildRegistration("").getProviderDetails(); + + // Logout is a browser redirect like authorization, so it must not be moved + // onto an address only the server can reach. + assertEquals( + PUBLIC_ISSUER + "/session/end", + details.getConfigurationMetadata().get("end_session_endpoint")); + } + + @Test + public void stillRewritesTheBrowserFacingEndpointOntoTheExternalUrl() { + String externalUrl = "https://public.example/trex/oidc"; + + ClientRegistration.ProviderDetails details = buildRegistration(externalUrl).getProviderDetails(); + + assertEquals(externalUrl + "/authorize", details.getAuthorizationUri()); + } +} From 8ad33124ef136c03a96a41f2fb73c9800317e856 Mon Sep 17 00:00:00 2001 From: Peter Hoffmann <954078+p-hoffmann@users.noreply.github.com> Date: Sun, 30 Aug 2026 08:15:17 +0800 Subject: [PATCH 2/2] Assert the Google redirect URI as the resolver actually emits it Two assertions described behaviour that cannot occur. The redirect URI is carried in a query parameter, where ":" and "/" need no escaping, so it appears verbatim rather than percent-encoded. The other named a server port that disagreed with a portless Host header, which a servlet container cannot produce, and the port then surfaced in the redirect URI. The resolver's output was correct in both cases. --- .../security/authc/GoogleAuthConfigTest.java | 28 +++++++++++-------- 1 file changed, 17 insertions(+), 11 deletions(-) diff --git a/src/test/java/org/ohdsi/webapi/security/authc/GoogleAuthConfigTest.java b/src/test/java/org/ohdsi/webapi/security/authc/GoogleAuthConfigTest.java index 7c8c194c5..cda67ad36 100644 --- a/src/test/java/org/ohdsi/webapi/security/authc/GoogleAuthConfigTest.java +++ b/src/test/java/org/ohdsi/webapi/security/authc/GoogleAuthConfigTest.java @@ -25,24 +25,26 @@ public class GoogleAuthConfigTest { public void googleRedirectUriUsesHttpsWhenForwardedHeadersAreProcessed() throws Exception { OAuth2AuthorizationRequest authorizationRequest = resolveAuthorizationRequest(true); + String expectedRedirectUri = "https://atlas-preview.ohdsi.org/WebAPI/user/oauth/callback/google"; + assertNotNull(authorizationRequest); - assertEquals( - "https://atlas-preview.ohdsi.org/WebAPI/user/oauth/callback/google", - authorizationRequest.getRedirectUri()); - assertTrue(authorizationRequest.getAuthorizationRequestUri().contains( - "redirect_uri=https%3A%2F%2Fatlas-preview.ohdsi.org%2FWebAPI%2Fuser%2Foauth%2Fcallback%2Fgoogle")); + assertEquals(expectedRedirectUri, authorizationRequest.getRedirectUri()); + // ":" and "/" are legal in a query value, so the parameter carries the URI + // as-is rather than percent-encoded. + assertTrue(authorizationRequest.getAuthorizationRequestUri() + .contains("redirect_uri=" + expectedRedirectUri)); } @Test public void googleRedirectUriFallsBackToHttpWhenForwardedHeadersAreIgnored() throws Exception { OAuth2AuthorizationRequest authorizationRequest = resolveAuthorizationRequest(false); + String expectedRedirectUri = "http://atlas-preview.ohdsi.org/WebAPI/user/oauth/callback/google"; + assertNotNull(authorizationRequest); - assertEquals( - "http://atlas-preview.ohdsi.org/WebAPI/user/oauth/callback/google", - authorizationRequest.getRedirectUri()); - assertTrue(authorizationRequest.getAuthorizationRequestUri().contains( - "redirect_uri=http%3A%2F%2Fatlas-preview.ohdsi.org%2FWebAPI%2Fuser%2Foauth%2Fcallback%2Fgoogle")); + assertEquals(expectedRedirectUri, authorizationRequest.getRedirectUri()); + assertTrue(authorizationRequest.getAuthorizationRequestUri() + .contains("redirect_uri=" + expectedRedirectUri)); } private OAuth2AuthorizationRequest resolveAuthorizationRequest(boolean applyForwardedHeaderFilter) throws Exception { @@ -52,7 +54,11 @@ private OAuth2AuthorizationRequest resolveAuthorizationRequest(boolean applyForw request.setRequestURI("/WebAPI/user/login/google"); request.setScheme("http"); request.setServerName("atlas-preview.ohdsi.org"); - request.setServerPort(8080); + // A servlet container derives these from the Host header, so a portless Host + // means the default port for the scheme. Naming a different one here would + // describe a request no container can produce, and the port would then show + // up in the redirect URI. + request.setServerPort(80); request.setSecure(false); request.addHeader("Host", "atlas-preview.ohdsi.org"); request.addHeader("X-Forwarded-Proto", "https");