Read OIDC metadata from an internal address, and assert the Google redirect URI as emitted

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;
@@ -27,8 +31,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;
@@ -63,6 +70,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("openid")
         .clientId(clientId)
         .clientSecret(clientSecret)
@@ -196,6 +221,68 @@ private Set<String> buildScopes() {
     return scopes;
   }
 
+  // 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<String, Object> internalConfiguration(String issuer, String internalBase) {
+    Map<String, Object> 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<String, Object> fetchConfiguration(String url) {
+    try {
+      HttpResponse<String> 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<Map<String, Object>>() {});
+    } 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");
@@ -287,6 +374,7 @@ public OpenidDirect(
         org.ohdsi.webapi.security.authz.mapping.ExternalRoleMapService externalRoleMapService,
         @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;
@@ -300,16 +388,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")

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");

src/test/java/org/ohdsi/webapi/security/authc/OidcAuthConfigInternalUrlTest.java

@@ -0,0 +1,125 @@
+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, null);
+    ReflectionTestUtils.setField(config, "oidcRuntimeEnabled", true);
+    ReflectionTestUtils.setField(config, "clientId", "d2e-webapi");
+    ReflectionTestUtils.setField(config, "clientSecret", "secret");
+    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());
+  }
+}