diff --git a/reference-app/src/main/java/com/ardetrick/oryhydrareference/consent/ConsentForm.java b/reference-app/src/main/java/com/ardetrick/oryhydrareference/consent/ConsentForm.java index 3d792b2..b0884d2 100644 --- a/reference-app/src/main/java/com/ardetrick/oryhydrareference/consent/ConsentForm.java +++ b/reference-app/src/main/java/com/ardetrick/oryhydrareference/consent/ConsentForm.java @@ -7,8 +7,9 @@ public record ConsentForm( String consentChallenge, Boolean remember, List scopes, String deny) { /** - * This field is implemented in HTML as a checkbox. When a checkbox element is not checked in a - * form it is not submitted in the form at all. This provides a useful null safe getter. + * Unchecked HTML checkboxes are omitted from the form post entirely — there is no {@code + * remember=false} — so Spring binds this as {@code null} when the box is unchecked. The boxed + * {@code Boolean} and this getter exist to absorb that. */ public boolean isRemember() { if (remember == null) { diff --git a/reference-app/src/main/java/com/ardetrick/oryhydrareference/hydra/AcceptLoginRequest.java b/reference-app/src/main/java/com/ardetrick/oryhydrareference/hydra/AcceptLoginRequest.java new file mode 100644 index 0000000..4251e8c --- /dev/null +++ b/reference-app/src/main/java/com/ardetrick/oryhydrareference/hydra/AcceptLoginRequest.java @@ -0,0 +1,8 @@ +package com.ardetrick.oryhydrareference.hydra; + +import lombok.Builder; +import lombok.NonNull; + +@Builder +public record AcceptLoginRequest( + @NonNull String loginChallenge, @NonNull String subject, boolean remember) {} 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 73a61fb..2d950d8 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 @@ -58,14 +58,12 @@ public Optional getLoginRequest(@NonNull String loginChallen } } - public OAuth2RedirectTo acceptLoginRequest( - @NonNull String loginChallenge, @NonNull String loginEmail) { - val acceptLoginRequest = new AcceptOAuth2LoginRequest(); - // Subject is an alias for user ID. A subject can be a random string, a UUID, an email address, - // ... - acceptLoginRequest.subject(loginEmail); + public OAuth2RedirectTo acceptLoginRequest(@NonNull AcceptLoginRequest acceptLoginRequest) { + val acceptOAuth2LoginRequest = OryHydraRequestMapper.map(acceptLoginRequest); + try { - return oAuth2Api().acceptOAuth2LoginRequest(loginChallenge, acceptLoginRequest); + return oAuth2Api() + .acceptOAuth2LoginRequest(acceptLoginRequest.loginChallenge(), acceptOAuth2LoginRequest); } catch (ApiException e) { switch (e.getCode()) { case 400, 401, 404, 500 -> diff --git a/reference-app/src/main/java/com/ardetrick/oryhydrareference/hydra/OryHydraRequestMapper.java b/reference-app/src/main/java/com/ardetrick/oryhydrareference/hydra/OryHydraRequestMapper.java index 8037a77..18665e8 100644 --- a/reference-app/src/main/java/com/ardetrick/oryhydrareference/hydra/OryHydraRequestMapper.java +++ b/reference-app/src/main/java/com/ardetrick/oryhydrareference/hydra/OryHydraRequestMapper.java @@ -3,12 +3,24 @@ import lombok.NonNull; import sh.ory.hydra.model.AcceptOAuth2ConsentRequest; import sh.ory.hydra.model.AcceptOAuth2ConsentRequestSession; +import sh.ory.hydra.model.AcceptOAuth2LoginRequest; import sh.ory.hydra.model.RejectOAuth2Request; public class OryHydraRequestMapper { private static final Long DEFAULT_SESSION_EXPIRATION_IN_SECONDS = 3600L; + public static AcceptOAuth2LoginRequest map(@NonNull final AcceptLoginRequest acceptLoginRequest) { + // The subject becomes the `sub` claim in every token Hydra issues — it is the stable, unique + // identifier for the end user. Any string works, but it should never change or be reassigned. + // This demo uses the login email for readability; a production login app should prefer an + // immutable internal user ID (emails change and get recycled). + return new AcceptOAuth2LoginRequest() + .subject(acceptLoginRequest.subject()) + .remember(acceptLoginRequest.remember()) + .rememberFor(DEFAULT_SESSION_EXPIRATION_IN_SECONDS); + } + public static AcceptOAuth2ConsentRequest map( @NonNull final AcceptConsentRequest acceptConsentRequest) { return new AcceptOAuth2ConsentRequest() diff --git a/reference-app/src/main/java/com/ardetrick/oryhydrareference/login/LoginForm.java b/reference-app/src/main/java/com/ardetrick/oryhydrareference/login/LoginForm.java index 740968b..ae16e09 100644 --- a/reference-app/src/main/java/com/ardetrick/oryhydrareference/login/LoginForm.java +++ b/reference-app/src/main/java/com/ardetrick/oryhydrareference/login/LoginForm.java @@ -1,3 +1,17 @@ package com.ardetrick.oryhydrareference.login; -public record LoginForm(String loginEmail, String loginPassword, String loginChallenge) {} +public record LoginForm( + String loginEmail, String loginPassword, String loginChallenge, Boolean remember) { + + /** + * Unchecked HTML checkboxes are omitted from the form post entirely — there is no {@code + * remember=false} — so Spring binds this as {@code null} when the box is unchecked. The boxed + * {@code Boolean} and this getter exist to absorb that. + */ + public boolean isRemember() { + if (remember == null) { + return false; + } + return remember; + } +} diff --git a/reference-app/src/main/java/com/ardetrick/oryhydrareference/login/LoginService.java b/reference-app/src/main/java/com/ardetrick/oryhydrareference/login/LoginService.java index d911f88..090f3bb 100644 --- a/reference-app/src/main/java/com/ardetrick/oryhydrareference/login/LoginService.java +++ b/reference-app/src/main/java/com/ardetrick/oryhydrareference/login/LoginService.java @@ -1,5 +1,6 @@ package com.ardetrick.oryhydrareference.login; +import com.ardetrick.oryhydrareference.hydra.AcceptLoginRequest; import com.ardetrick.oryhydrareference.hydra.HydraAdminClient; import com.ardetrick.oryhydrareference.login.LoginResult.LoginAcceptedFollowRedirect; import com.ardetrick.oryhydrareference.login.LoginResult.LoginDeniedInvalidCredentials; @@ -31,8 +32,21 @@ public LoginResult processInitialLoginRequest(@NonNull String loginChallenge) { val loginRequest = maybeLoginRequest.get(); if (loginRequest.getSkip()) { - hydraAdminClient.acceptLoginRequest(loginChallenge, loginRequest.getSubject()); - return new LoginAcceptedFollowRedirect(loginRequest.getRequestUrl()); + // skip=true means Hydra recognized an existing remembered login session (only a prior + // accept with remember=true creates one), so accept without showing the login UI. + // remember=false does not end that session — it declines to extend it, matching Ory's + // reference implementation; sliding-session behavior is available via + // extend_session_lifespan=true instead. Follow the accept response's redirect: + // redirecting back to the original request URL would replay the authorization request + // and loop straight back here. + val completedRequest = + hydraAdminClient.acceptLoginRequest( + AcceptLoginRequest.builder() + .loginChallenge(loginChallenge) + .subject(loginRequest.getSubject()) + .remember(false) + .build()); + return new LoginAcceptedFollowRedirect(completedRequest.getRedirectTo()); } return new LoginNotSkippableDisplayLoginUI(loginChallenge); @@ -51,7 +65,12 @@ public LoginResult processSubmittedLoginRequest( } val completedRequest = - hydraAdminClient.acceptLoginRequest(loginChallenge, loginForm.loginEmail()); + hydraAdminClient.acceptLoginRequest( + AcceptLoginRequest.builder() + .loginChallenge(loginChallenge) + .subject(loginForm.loginEmail()) + .remember(loginForm.isRemember()) + .build()); return new LoginAcceptedFollowRedirect(completedRequest.getRedirectTo()); } diff --git a/reference-app/src/main/resources/templates/login.ftlh b/reference-app/src/main/resources/templates/login.ftlh index 8aa93b2..a2a31df 100644 --- a/reference-app/src/main/resources/templates/login.ftlh +++ b/reference-app/src/main/resources/templates/login.ftlh @@ -13,6 +13,9 @@
+ +
+ 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 eeb7dcb..a8724a4 100644 --- a/reference-app/src/test/java/com/ardetrick/oryhydrareference/OryHydraReferenceApplicationFunctionalTests.java +++ b/reference-app/src/test/java/com/ardetrick/oryhydrareference/OryHydraReferenceApplicationFunctionalTests.java @@ -464,6 +464,52 @@ public void denyConsentRedirectsToClientWithAccessDeniedError() { assertThat(queryString).contains("error=access_denied"); assertThat(queryString).doesNotContain("code="); } + + @Test + public void skipLoginScreenOnSecondFlowWhenLoginRememberMeIsUsed() { + val screenshotPathProducer = + ScreenshotPathProducer.builder() + .testName("skipLoginScreenOnSecondFlowWhenLoginRememberMeIsUsed") + .build(); + + val page = browser.newPage(); + + page.navigate(getUriToInitiateFlow().toString()); + page.screenshot(screenshotPathProducer.screenshotOptionsForStepName("initial-load")); + + page.locator("input[name=loginEmail]").fill("foo@bar.com"); + page.locator("input[name=loginPassword]").fill("password"); + page.locator("input[id=remember]").check(); + + page.screenshot(screenshotPathProducer.screenshotOptionsForStepName("after-check-remember")); + + page.locator("input[name=submit]").click(); + + page.waitForLoadState(); + page.screenshot(screenshotPathProducer.screenshotOptionsForStepName("after-login-submit")); + + // Uncheck the consent-side remember so the second flow isolates the login skip: consent must + // be asked again, proving the login screen alone was skipped. + page.locator("input[id=remember]").uncheck(); + + page.locator("input[id=accept]").click(); + + page.waitForLoadState(); + page.screenshot(screenshotPathProducer.screenshotOptionsForStepName("after-consent-submit")); + + val code = getCodeFromCallbackCaptor(); + exchangeCode(code); + + page.navigate(getUriToInitiateFlow().toString()); + + page.waitForLoadState(); + page.screenshot( + screenshotPathProducer.screenshotOptionsForStepName("initial-load-second-time")); + + // The login screen is skipped — no credentials were entered this time — but the consent + // screen still appears because its remember was unchecked. + assertThat(page.url()).contains("/consent"); + } } @JsonNaming(PropertyNamingStrategies.SnakeCaseStrategy.class)