diff --git a/reference-app/src/main/java/com/ardetrick/oryhydrareference/hydra/HydraAdminClient.java b/reference-app/src/main/java/com/ardetrick/oryhydrareference/hydra/HydraAdminClient.java index 36204c4..5f74d18 100644 --- a/reference-app/src/main/java/com/ardetrick/oryhydrareference/hydra/HydraAdminClient.java +++ b/reference-app/src/main/java/com/ardetrick/oryhydrareference/hydra/HydraAdminClient.java @@ -11,7 +11,6 @@ import org.springframework.boot.context.properties.ConfigurationProperties; import org.springframework.stereotype.Service; import sh.ory.hydra.ApiException; -import sh.ory.hydra.Configuration; import sh.ory.hydra.api.OAuth2Api; import sh.ory.hydra.model.*; @@ -20,16 +19,23 @@ @FieldDefaults(level = AccessLevel.PRIVATE, makeFinal = true) public class HydraAdminClient { - @NonNull OAuth2Api oAuth2Api; + @NonNull Properties properties; HydraAdminClient(@NonNull final HydraAdminClient.Properties properties) { - val apiClient = Configuration.getDefaultApiClient().setBasePath(properties.getBasePath()); - oAuth2Api = new OAuth2Api(apiClient); + this.properties = properties; + } + + // The base path is resolved on every call rather than captured at construction: the bean is + // built when the Spring context boots, but the configured base path may legitimately change + // afterwards (integration tests only learn Hydra's randomized port once it is running). A + // dedicated ApiClient instance also avoids mutating the SDK's global default client. + private OAuth2Api oAuth2Api() { + return new OAuth2Api(new sh.ory.hydra.ApiClient().setBasePath(properties.getBasePath())); } public List listOAuth2Clients() { try { - return oAuth2Api.listOAuth2Clients(1000L, null, null, null); + return oAuth2Api().listOAuth2Clients(1000L, null, null, null); } catch (ApiException e) { throw new RuntimeException(e); } @@ -41,7 +47,7 @@ public List listOAuth2Clients() { */ public Optional getLoginRequest(@NonNull String loginChallenge) { try { - return Optional.of(oAuth2Api.getOAuth2LoginRequest(loginChallenge)); + return Optional.of(oAuth2Api().getOAuth2LoginRequest(loginChallenge)); } catch (ApiException e) { return switch (e.getCode()) { case 410 -> Optional.empty(); // requestWasHandledResponse @@ -59,7 +65,7 @@ public OAuth2RedirectTo acceptLoginRequest( // ... acceptLoginRequest.subject(loginEmail); try { - return oAuth2Api.acceptOAuth2LoginRequest(loginChallenge, acceptLoginRequest); + return oAuth2Api().acceptOAuth2LoginRequest(loginChallenge, acceptLoginRequest); } catch (ApiException e) { switch (e.getCode()) { case 400, 401, 404, 500 -> @@ -71,7 +77,7 @@ public OAuth2RedirectTo acceptLoginRequest( public OAuth2ConsentRequest getConsentRequest(@NonNull String consentChallenge) { try { - return oAuth2Api.getOAuth2ConsentRequest(consentChallenge); + return oAuth2Api().getOAuth2ConsentRequest(consentChallenge); } catch (ApiException e) { switch (e.getCode()) { case 400, 404 -> throw new RuntimeException("code: " + e.getCode(), e); // jsonError @@ -84,8 +90,9 @@ public OAuth2RedirectTo acceptConsentRequest(@NonNull AcceptConsentRequest accep val acceptOAuth2ConsentRequest = OryHydraRequestMapper.map(acceptConsentRequest); try { - return oAuth2Api.acceptOAuth2ConsentRequest( - acceptConsentRequest.consentChallenge(), acceptOAuth2ConsentRequest); + return oAuth2Api() + .acceptOAuth2ConsentRequest( + acceptConsentRequest.consentChallenge(), acceptOAuth2ConsentRequest); } catch (ApiException e) { switch (e.getCode()) { case 404, 500 -> throw new RuntimeException("code: " + e.getCode(), e); // jsonError diff --git a/reference-app/src/test/java/com/ardetrick/oryhydrareference/OryHydraReferenceApplicationFunctionalTests.java b/reference-app/src/test/java/com/ardetrick/oryhydrareference/OryHydraReferenceApplicationFunctionalTests.java index daa7601..9f39097 100644 --- a/reference-app/src/test/java/com/ardetrick/oryhydrareference/OryHydraReferenceApplicationFunctionalTests.java +++ b/reference-app/src/test/java/com/ardetrick/oryhydrareference/OryHydraReferenceApplicationFunctionalTests.java @@ -45,10 +45,6 @@ import org.springframework.web.bind.annotation.RequestMapping; import org.springframework.web.bind.annotation.RestController; import org.springframework.web.servlet.view.RedirectView; -import sh.ory.hydra.ApiException; -import sh.ory.hydra.Configuration; -import sh.ory.hydra.api.OAuth2Api; -import sh.ory.hydra.model.OAuth2Client; @Slf4j @SpringBootTest(webEnvironment = SpringBootTest.WebEnvironment.RANDOM_PORT) @@ -57,17 +53,21 @@ ForwardingController.class, }) @TestPropertySource(properties = {"debug=true"}) +@TestInstance(TestInstance.Lifecycle.PER_CLASS) public class OryHydraReferenceApplicationFunctionalTests { - // Shared between all tests in this class. - private static Playwright playwright; - private static Browser browser; + // Shared between all tests in this class — plain instance fields: under + // @TestInstance(PER_CLASS) the single test instance lives for the whole class. + private Playwright playwright; + private Browser browser; @LocalServerPort int springBootAppPort; OryHydraContainer dockerComposeEnvironment; - OAuth2Client oAuth2Client; + String clientId; + String clientSecret; + String redirectUri; @Autowired HydraAdminClient.Properties properties; @@ -76,25 +76,26 @@ public class OryHydraReferenceApplicationFunctionalTests { @Captor ArgumentCaptor queryStringConsumerArgumentCaptor = ArgumentCaptor.forClass(String.class); + @Autowired ForwardingController forwardingController; + + // Lifecycle: the Spring context (and its random-port Tomcat) boots before JUnit creates the + // test instance; @TestInstance(PER_CLASS) reuses that single instance for the whole class, + // which is what allows @BeforeAll to be non-static — and therefore able to read the injected + // @LocalServerPort — while still running exactly once before all tests. + // + // Deliberately NOT the idiomatic @Testcontainers/@Container-on-a-static-field pattern: that + // constructs the container at class-load time, but this container's configuration needs the + // injected @LocalServerPort, which only exists after the context boots. The alternatives all + // cost more than they save (a DEFINED_PORT app breaks parallel safety; an instance @Container + // restarts per test regardless of @TestInstance). @BeforeAll - static void beforeAll() { + void startTestEnvironment() { playwright = Playwright.create(); browser = playwright.chromium().launch(); - } - - @AfterAll - static void afterAll() { - playwright.close(); - } - @BeforeEach - public void beforeEachTest() throws ApiException { - // Start Ory Hydra, passing it the port of the reference application to configure urls. - // An alternative approach would be to start the container once and re-use it across all tests. - // However, @BeforeAllTests requires the method be static but @LocalServerPort is unavailable - // statically. - // Creating the containers for each test increases test execution time but keeps all tests - // isolated. + // One Hydra container serves every test in this class — containers are the expensive + // resource. Test isolation is preserved by registering a unique OAuth2 client per test + // instead (see registerTestClient). dockerComposeEnvironment = OryHydraContainer.builder() .urlsLogin("http://localhost:" + springBootAppPort + "/login") @@ -104,54 +105,37 @@ public void beforeEachTest() throws ApiException { .build(); dockerComposeEnvironment.start(); - // A "cheat" to break a circular dependency where the reference application needs to know the - // URI of Ory Hydra - // and Ory Hydra needs to know the URI of the reference application. In a production application - // these two URIs - // should be static and well known. However, in the context of these tests the ports of both the - // reference - // application and Ory Hydra are randomized and are unknown until after the application is - // already running - // (this follows testing best practices where hard coding ports should be avoided in case the - // host machine is - // already using that port). There may be a cleaner approach out there (perhaps using Docker - // Networking?) but - // in the meantime this is a low cost and sufficient work around. - properties.setBasePath(dockerComposeEnvironment.publicBaseUriString()); - - oAuth2Client = createOAuthClient(); + // Late-bind the two Hydra endpoints now that the mapped ports exist (the circular + // port dependency: the app and Hydra each need the other's randomized URI). Two consumers, + // two different endpoints: the app's HydraAdminClient speaks the admin API, while the + // test's public proxy forwards browsers to the public authorize endpoint. + properties.setBasePath(dockerComposeEnvironment.adminBaseUriString()); + forwardingController.hydraPublicBaseUri = dockerComposeEnvironment.publicBaseUriString(); } - @AfterEach - public void afterEachTest() { - // Test containers must be stopped to avoid a port conflict error when starting them up again - // for the next test. + @AfterAll + void stopTestEnvironment() { + playwright.close(); dockerComposeEnvironment.stop(); } - private OAuth2Client createOAuthClient() throws ApiException { - val oAuth2Client = new OAuth2Client(); - oAuth2Client.clientName("test-client"); - oAuth2Client.redirectUris( - List.of("http://localhost:" + springBootAppPort + CLIENT_CALL_BACK_PATH)); - oAuth2Client.grantTypes(List.of("authorization_code", "refresh_token")); - oAuth2Client.responseTypes(List.of("code", "id_token")); - oAuth2Client.clientSecret("client-secret"); - oAuth2Client.scope(String.join(" ", "offline_access", "openid", "offline", "profile")); - - // Initialize API - val oauth2Api = - new OAuth2Api( - Configuration.getDefaultApiClient() - .setBasePath(dockerComposeEnvironment.adminBaseUriString())); - - // Create client - val oauth2Client = oauth2Api.createOAuth2Client(oAuth2Client); - - // Basic assertions on created client - assertThat(oauth2Api.getOAuth2Client(oauth2Client.getClientId())).isNotNull(); - - return oauth2Client; + @BeforeEach + public void registerTestClient() { + // A unique client per test keeps tests isolated on the shared container: Hydra remembers + // consent per subject + client, so a shared client would leak remembered consent between + // tests. Registration is a cheap upserting admin API call via the container. + clientId = "test-client-" + UUID.randomUUID(); + clientSecret = "client-secret"; + redirectUri = "http://localhost:" + springBootAppPort + CLIENT_CALL_BACK_PATH; + dockerComposeEnvironment.createOrReplaceClient( + client -> + client + .clientId(clientId) + .clientSecret(clientSecret) + .redirectUris(redirectUri) + .grantTypes("authorization_code", "refresh_token") + .responseTypes("code", "id_token") + .scope("offline_access", "openid", "offline", "profile")); } /** @@ -198,9 +182,8 @@ private URI getUriToInitiateFlow() { try { return new URIBuilder(dockerComposeEnvironment.publicBaseUriString() + "/oauth2/auth") .addParameter("response_type", "code") - .addParameter("client_id", oAuth2Client.getClientId()) - .addParameter( - "redirect_uri", Objects.requireNonNull(oAuth2Client.getRedirectUris()).get(0)) + .addParameter("client_id", clientId) + .addParameter("redirect_uri", redirectUri) .addParameter("scope", "offline_access openid offline profile") .addParameter("state", "12345678901234567890") .build(); @@ -292,14 +275,10 @@ public void completeFlowWithPartialScopeSelection() { private CodeExchangeResponse exchangeCode(String code) { val encodedParams = Map.of( - "client_id", - Objects.requireNonNull(oAuth2Client.getClientId()), - "code", - code, - "grant_type", - Objects.requireNonNull(Objects.requireNonNull(oAuth2Client.getGrantTypes()).get(0)), - "redirect_uri", - Objects.requireNonNull(oAuth2Client.getRedirectUris()).get(0)) + "client_id", clientId, + "code", code, + "grant_type", "authorization_code", + "redirect_uri", redirectUri) .entrySet() .stream() .map( @@ -318,9 +297,7 @@ private CodeExchangeResponse exchangeCode(String code) { "authorization", "Basic " + Base64.getEncoder() - .encodeToString( - (oAuth2Client.getClientId() + ":" + oAuth2Client.getClientSecret()) - .getBytes())) + .encodeToString((clientId + ":" + clientSecret).getBytes())) .POST(HttpRequest.BodyPublishers.ofString(encodedParams)) .build(); @@ -475,11 +452,14 @@ record CodeExchangeResponse( @RequestMapping("/integration-test-public-proxy") class ForwardingController { - @Autowired HydraAdminClient.Properties properties; + // Set by the test once Hydra's mapped public port is known; the proxy forwards browsers to + // Hydra's public authorize endpoint (the admin base in HydraAdminClient.Properties is a + // different endpoint for a different consumer). + String hydraPublicBaseUri; @GetMapping("oauth2/auth") public RedirectView oauth2Auth() { - val redirectView = new RedirectView(properties.getBasePath() + "/oauth2/auth"); + val redirectView = new RedirectView(hydraPublicBaseUri + "/oauth2/auth"); redirectView.setPropagateQueryParams(true); return redirectView; }