Repository navigation
fix: PAR authorization request supports registration via prompt=create - #50183
Conversation
There was a problem hiding this comment.
Pull request overview
This PR fixes handling of OpenID Connect prompt=create when the authorization request parameters are delivered via Pushed Authorization Requests (PAR), ensuring the registration page is shown as it is for non-PAR flows.
Changes:
- Update OIDC
AuthorizationEndpointto considerprompteven when not present in the browser request (PAR scenario). - Add a PAR test that uses
prompt=createand verifies the registration page opens. - Extend the test PAR request helper to allow setting
prompt.
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
tests/utils-shared/src/main/java/org/keycloak/testsuite/util/oauth/ParRequest.java |
Adds a prompt() helper to include prompt in PAR requests for tests. |
tests/base/src/test/java/org/keycloak/tests/oid4vc/issuance/signing/OID4VCAuthorizationCodeFlowWithPARTest.java |
Adds coverage verifying prompt=create over PAR opens the registration flow. |
services/src/main/java/org/keycloak/protocol/oidc/endpoints/AuthorizationEndpoint.java |
Adjusts prompt=create handling so PAR-delivered prompt can trigger registration. |
| String promptValue = Optional | ||
| .ofNullable(params.getFirst(OAuth2Constants.PROMPT)) | ||
| .orElseGet(() -> session.getContext().getAuthenticationSession().getClientNote(OAuth2Constants.PROMPT)); | ||
|
|
||
| if (OIDCLoginProtocol.PROMPT_VALUE_CREATE.equals(promptValue)) { |
There was a problem hiding this comment.
Original code had this raw equality check, so I'd only touch this if someone from Keycloak project asks for it.
| public ParRequest prompt(String prompt) { | ||
| parameter(OIDCLoginProtocol.PROMPT_PARAM, prompt); | ||
| return this; | ||
| } |
7e230d6 to
f8fb528
Compare
Unreported flaky test detectedIf the flaky tests below are affected by the changes, please review and update the changes accordingly. Otherwise, a maintainer should report the flaky te 8000 sts prior to merging the PR. org.keycloak.testsuite.forms.BruteForceTest#testGrantMissingOtpKeycloak CI - Forms IT (chrome) |
There was a problem hiding this comment.
@vilmosnagy Thanks! I hope your PR can be merged once tests are OK. Tried to re-run the failing UI tests, which likely failed due the other reason that your PR.
Thanks @mposolda. To be honest, I didn't even check the admin ui e2e test failure -> it failed a bunch of times in CI unrelated to my changes on the few PRs I had the past month (and after a re-run sometimes it become green, sometimes it turned red again with a simple rebase). It seems to be unrelated again: If re-running wont fix this, pls ping me and I'm going to check more deeply. |
|
🤔 I see that it fails for the 3rd time - let me check this evening. |
f8fb528 to
d42fc53
Compare
| registerPage.register("Vilmos", "Szabó-Nagy", "vilmos@email", "vnagy", "AppleTree123"); | ||
| testRealm.cleanup().add(r -> r.users().search("vnagy").stream().findFirst().ifPresent(u -> r.users().delete(u.getId()))); |
|
@mposolda I rebased my PR, and js tests seems to be fixed here: https://github.com/keycloak/keycloak/actions/runs/27948119713 Applied the code changes from |
Closes keycloak#49255 Based on keycloak#47244 Signed-off-by: Vilmos Nagy <me@vnagy.eu>
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com> Signed-off-by: Nagy Vilmos <me@vnagy.eu>
70335a2 to
6faa332
Compare
|
@vilmosnagy I did remote rebase of your branch and CI is running. Hopefully it is OK and your PR can be merged. |
|
Thanks for the quick review & merge! |
…te` (keycloak#50183) Closes keycloak#49255 Signed-off-by: Vilmos Nagy <me@vnagy.eu>
Closes #49255
Based on #47244