test: OIDC social-login setPassword step-up fallback E2E (#90) - #93
Merged
Conversation
Follow-up to #75. Adds the browser E2E that proves an OIDC (Keycloak) account with no passkey has POST /user/setPassword governed by allowInitialPasswordSetWithoutStepUp (403 when false, succeeds when true) rather than a permanent 401 step-up, and that a freshly logged-in OIDC user can enroll a first passkey (FACTOR_AUTHORIZATION_CODE). - New chromium-step-up-oidc Playwright project (grep @step-up-oidc), excluded from the default/step-up projects. - globalSetup/globalTeardown start a dev-mode Keycloak container (realm mounted, no external DB) when KEYCLOAK_E2E is set; reuse an already running provider, and fail fast rather than destroy a live one. - step-up-oidc.spec.ts: real Keycloak redirect login, both fallback branches (selected per app boot via STEP_UP_OIDC_ALLOW_INITIAL and a SPRING_APPLICATION_JSON override), and first-passkey enrollment. - New playwright-tests-oidc CI job mirroring playwright-tests, with a Keycloak provider (via globalSetup) and a Mailpit service so the passkey-registration notification stays local instead of hitting SES. - Docs: TESTING.md and AUTHENTICATION.md describe the new project, run commands, and the local-DB reset caveat. Claude-Session: https://claude.ai/code/session_01EJY7pA4NvY9vJt6CfDBVWt
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
🟡 Changes recommended
The new Keycloak harness has a real concurrency/ownership edge case and the new OIDC E2E spec hard-codes localhost:8080, which can break runs that override BASE_URL.
Once you've addressed the issues Copilot identified, you can request another Copilot review.
Pull request overview
Adds end-to-end coverage for the step-up fallback behavior on OIDC (Keycloak) “social-login” accounts, demonstrating that POST /user/setPassword is governed by allowInitialPasswordSetWithoutStepUp (403 vs 200 depending on the flag) instead of returning a permanent step-up 401 for passkey-less OIDC accounts. This fits the demo app’s role as the reference implementation for Spring User Framework step-up behavior and its Playwright E2E harness.
Changes:
- Add a new Chromium-only Playwright project/spec (
@step-up-oidc) that performs a real Keycloak redirect login and asserts bothallowInitialPasswordSetWithoutStepUpbranches plus first-passkey enrollment. - Add Playwright global setup/teardown + a Docker-based Keycloak lifecycle helper to provision an OIDC provider for the OIDC E2E run.
- Add a dedicated CI job to run the OIDC suite (twice, once per boot-time flag branch) and document how to run it locally.
File summaries
| File | Description |
|---|---|
| playwright/tsconfig.json | Includes new global setup/teardown TypeScript entrypoints in the Playwright TS project. |
| playwright/tests/step-up/step-up-oidc.spec.ts | New OIDC step-up fallback E2E spec (Keycloak redirect login + setPassword + first passkey enrollment). |
| playwright/src/utils/keycloak.ts | New Docker-driven Keycloak lifecycle utilities for the OIDC Playwright project. |
| playwright/playwright.config.ts | Adds globalSetup/globalTeardown hooks and the chromium-step-up-oidc project + grepInvert updates. |
| playwright/global-setup.ts | Starts Keycloak for OIDC E2E runs when KEYCLOAK_E2E is set. |
| playwright/global-teardown.ts | Stops Keycloak only when the run started it. |
| docs/TESTING.md | Documents the new OIDC Playwright project, run commands, and CI job behavior. |
| docs/AUTHENTICATION.md | Adds a reference to the OIDC fallback E2E coverage and where/how it runs. |
| .github/workflows/tests.yml | Adds playwright-tests-oidc CI job with Mailpit + Keycloak harness for OIDC step-up tests. |
Review details
- Files reviewed: 9/9 changed files
- Comments generated: 2
- Review effort level: Lite
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
Comment on lines
+84
to
+88
| export async function startKeycloak(): Promise<void> { | ||
| if (await metadataReady()) { | ||
| // Reuse an already-running provider; do not claim ownership, so teardown leaves it alone. | ||
| return; | ||
| } |
Comment on lines
+26
to
+48
| /** Which allowInitialPasswordSetWithoutStepUp branch the app under test is booted with. */ | ||
| const allowInitial = process.env.STEP_UP_OIDC_ALLOW_INITIAL === 'true'; | ||
|
|
||
| /** | ||
| * Drive the full OIDC redirect login: app login page -> Keycloak form -> back to the app authenticated. | ||
| * Uses a fresh browser context (Playwright's per-test default), so there is no Keycloak SSO cookie and | ||
| * the login form is always shown. | ||
| */ | ||
| async function loginWithKeycloak(page: Page): Promise<void> { | ||
| await page.goto('/user/login.html'); | ||
| await page.locator('a[href$="/oauth2/authorization/keycloak"]').click(); | ||
|
|
||
| // Now on the Keycloak-hosted login form (published on host port 8180). | ||
| await page.waitForURL((url) => url.port === '8180', { timeout: 30000 }); | ||
| await page.locator('#username').fill(KEYCLOAK_USER.username); | ||
| await page.locator('#password').fill(KEYCLOAK_USER.password); | ||
| await page.locator('#kc-login').click(); | ||
|
|
||
| // Back on the app, authenticated, off the login page. | ||
| await page.waitForURL((url) => url.host === 'localhost:8080' && !url.pathname.includes('/login'), { | ||
| timeout: 30000, | ||
| }); | ||
| } |
The playwright-tests-oidc job is heavier than the others (two app boots plus a Keycloak container), so gate it to pushes on main rather than running it on every pull request. Claude-Session: https://claude.ai/code/session_01EJY7pA4NvY9vJt6CfDBVWt
Comment on lines
+126
to
+236
| name: Playwright OIDC Step-Up Tests | ||
| # Heavier than the other jobs (two app boots plus a Keycloak container), so it runs only on pushes | ||
| # to main, not on every pull request. The `push` trigger is already scoped to main; the ref check | ||
| # keeps it correct if more push branches are added later. | ||
| if: github.event_name == 'push' && github.ref == 'refs/heads/main' | ||
| runs-on: ubuntu-latest | ||
| timeout-minutes: 30 | ||
| services: | ||
| mariadb: | ||
| image: mariadb:12.2 | ||
| env: | ||
| MARIADB_DATABASE: springuser | ||
| MARIADB_USER: springuser | ||
| MARIADB_PASSWORD: springuser | ||
| MARIADB_ROOT_PASSWORD: rootpassword | ||
| ports: | ||
| - 3306:3306 | ||
| options: >- | ||
| --health-cmd="healthcheck.sh --connect --innodb_initialized" | ||
| --health-interval=10s | ||
| --health-timeout=5s | ||
| --health-retries=5 | ||
| mailpit: | ||
| # The first-passkey-enrollment test registers a passkey, and user.webauthn.notifyOnRegistration | ||
| # is on by default, so the app sends a notification email. Without a relay it would attempt a real | ||
| # outbound SMTP connection to the base config's SES host on every run; catch it in Mailpit instead | ||
| # (the mail env below points spring.mail here). The test does not assert the mail; this just keeps | ||
| # the send local. | ||
| image: axllent/mailpit:v1.30.7 | ||
| env: | ||
| MP_SMTP_AUTH_ACCEPT_ANY: "1" | ||
| MP_SMTP_AUTH_ALLOW_INSECURE: "1" | ||
| ports: | ||
| - 1025:1025 | ||
| - 8025:8025 | ||
| options: >- | ||
| --health-cmd="/mailpit readyz" | ||
| --health-interval=10s | ||
| --health-timeout=5s | ||
| --health-retries=5 | ||
| env: | ||
| # The MariaDB service container replaces Spring Boot's Docker Compose integration. | ||
| SPRING_DOCKER_COMPOSE_ENABLED: "false" | ||
| # Point mail at the Mailpit service container so passkey-registration notifications stay local | ||
| # (mirrors docker-compose-keycloak.yml's SPRING_MAIL_* for the same plain-SMTP, no-auth setup). | ||
| SPRING_MAIL_HOST: localhost | ||
| SPRING_MAIL_PORT: "1025" | ||
| SPRING_MAIL_PROPERTIES_MAIL_SMTP_AUTH: "false" | ||
| SPRING_MAIL_PROPERTIES_MAIL_SMTP_STARTTLS_ENABLE: "false" | ||
| SPRING_MAIL_PROPERTIES_MAIL_SMTP_STARTTLS_REQUIRED: "false" | ||
| # Tell Playwright's globalSetup to start a Keycloak provider (via docker) for this run. | ||
| KEYCLOAK_E2E: "1" | ||
| # OIDC client + provider config for the app under test. The app runs on the runner host (started by | ||
| # Playwright's webServer), not inside a compose network, so every provider URI points at the host's | ||
| # published Keycloak port 8180 rather than the compose-network keycloak:8080. Client id/secret match | ||
| # keycloak/realm/realm-export.json (dev-only credentials committed to the repo). | ||
| DS_SPRING_USER_KEYCLOAK_CLIENT_ID: ds-spring-user-framework-demo | ||
| DS_SPRING_USER_KEYCLOAK_CLIENT_SECRET: FTp1j7sGvc4g3MFdghEX4n7RPhbu86PQ | ||
| DS_SPRING_USER_KEYCLOAK_PROVIDER_AUTHORIZATION_URI: http://localhost:8180/realms/demo/protocol/openid-connect/auth | ||
| DS_SPRING_USER_KEYCLOAK_PROVIDER_TOKEN_URI: http://localhost:8180/realms/demo/protocol/openid-connect/token | ||
| DS_SPRING_USER_KEYCLOAK_PROVIDER_USER_INFO_URI: http://localhost:8180/realms/demo/protocol/openid-connect/userinfo | ||
| DS_SPRING_USER_KEYCLOAK_PROVIDER_JWK_SET_URI: http://localhost:8180/realms/demo/protocol/openid-connect/certs | ||
| steps: | ||
| - uses: actions/checkout@v4 | ||
|
|
||
| - uses: actions/setup-java@v4 | ||
| with: | ||
| distribution: temurin | ||
| java-version: 21 | ||
|
|
||
| - uses: gradle/actions/setup-gradle@v4 | ||
|
|
||
| - name: Build application | ||
| run: ./gradlew assemble | ||
|
|
||
| - uses: actions/setup-node@v4 | ||
| with: | ||
| node-version: 20 | ||
| cache: npm | ||
| cache-dependency-path: playwright/package-lock.json | ||
|
|
||
| - name: Install Playwright dependencies | ||
| working-directory: playwright | ||
| run: | | ||
| npm ci | ||
| npx playwright install --with-deps chromium | ||
|
|
||
| # Two boots: the allowInitialPasswordSetWithoutStepUp flag is boot-time config, so each branch of | ||
| # the OIDC fallback needs its own app start. globalSetup brings Keycloak up for each run. | ||
| - name: Run E2E (OIDC step-up, initial password allowed) | ||
| working-directory: playwright | ||
| env: | ||
| APP_PROFILES: docker-keycloak,playwright-test,step-up | ||
| STEP_UP_OIDC_ALLOW_INITIAL: "true" | ||
| run: npx playwright test --project=chromium-step-up-oidc | ||
|
|
||
| - name: Run E2E (OIDC step-up, initial password denied) | ||
| working-directory: playwright | ||
| env: | ||
| APP_PROFILES: docker-keycloak,playwright-test,step-up | ||
| STEP_UP_OIDC_ALLOW_INITIAL: "false" | ||
| # Override playwright-test's flag (true) so the denial branch is exercised. | ||
| SPRING_APPLICATION_JSON: '{"user":{"security":{"allowInitialPasswordSetWithoutStepUp":false}}}' | ||
| run: npx playwright test --project=chromium-step-up-oidc | ||
|
|
||
| - name: Upload Playwright report | ||
| if: failure() | ||
| uses: actions/upload-artifact@v4 | ||
| with: | ||
| name: playwright-report-oidc | ||
| path: playwright/reports/ |
Addresses the two low-severity review findings on PR #93 (#90). - Extract setupVirtualAuthenticator, addVirtualAuthenticator, and getCredentialIds into playwright/src/utils/webauthn.ts and export them from utils/index.ts. The step-up-oidc, step-up-flow, and mfa-flow specs now import these instead of each carrying a local copy (mfa-flow previously inlined the authenticator setup). - startKeycloak now removes the container it started if waitForMetadata times out. Playwright's separate globalTeardown is not guaranteed to run when globalSetup throws, so a readiness timeout could otherwise leave the container and ownership marker behind, which the next run's "already running but not answering" guard would reject until a manual docker rm -f. Claude-Session: https://claude.ai/code/session_01EJY7pA4NvY9vJt6CfDBVWt
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Closes #90. Follow-up to #75 (WebAuthn step-up / SUF-02), which deferred this acceptance case because it needs a Keycloak-backed harness the standard Playwright setup did not have.
What this proves
An OIDC (Keycloak) social-login account has no passkey, so it can never satisfy a WEBAUTHN step-up. This E2E confirms that its initial
POST /user/setPasswordis governed byallowInitialPasswordSetWithoutStepUp— 403 when the flag is false, 200 when true — rather than the permanent401 code 6a passkey-only account gets. It also exercises that OIDC login stampsFACTOR_AUTHORIZATION_CODE, so a freshly logged-in OIDC user can enroll a first passkey.Changes
chromium-step-up-oidcPlaywright project (grep@step-up-oidc), excluded from the default and existing step-up projects.globalSetup/globalTeardownstart a dev-mode Keycloak container (thekeycloak/realmexport mounted, no external DB) whenKEYCLOAK_E2Eis set; an already-running provider is reused, and a live container is never torn down out from under a concurrent run.step-up-oidc.spec.ts: real Keycloak redirect login (demo/demo), both fallback branches, and first-passkey enrollment. The two flag branches are separate app boots selected bySTEP_UP_OIDC_ALLOW_INITIALplus aSPRING_APPLICATION_JSONoverride; each of the three tests runs exactly once across the two boots.playwright-tests-oidcCI job mirroringplaywright-tests, with the Keycloak provider (viaglobalSetup) and a Mailpit service so the passkey-registration notification stays local instead of attempting a real SES send.docs/TESTING.mdanddocs/AUTHENTICATION.mddescribe the new project, run commands, and the local-DB reset caveat (demo@example.com).The app runs on the host on
docker-keycloak,playwright-test,step-up, with theDS_SPRING_USER_KEYCLOAK_*provider URIs pointed at the host's published Keycloak port (localhost:8180) rather than the compose-networkkeycloak:8080.Testing
Ran locally against a real Keycloak (both branches, twice):
setPasswordsucceeds + first-passkey enrollment — 2 passed, 1 skipped.setPasswordreturns 403, not 401 — 1 passed, 2 skipped.Notes
keycloak.envandkeycloak/realm/realm-export.json, not a new secret.KEYCLOAK_E2E=1runs are unsupported by design (they share host port 8180); the harness fails fast with a clear message rather than corrupting state. CI runners are fresh VMs, so this does not apply there.https://claude.ai/code/session_01EJY7pA4NvY9vJt6CfDBVWt