Fix NPE in secure-redirect-uris-enforcer during PAR authorization requests by tatsuya-kurosaka · Pull Request #54053 · keycloak/keycloak

tatsuya-kurosaka · 2026-10-16T08:43:32Z

Summary

When secure-redirect-uris-enforcer is enabled, PAR authorization requests
may not include redirect_uri in the raw authorization request parameters
during PRE_AUTHORIZATION_REQUEST.

This causes redirect URI validation to be executed with a null value and
results in a failure before the PAR request parameters are resolved.

This change skips redirect URI validation during
PRE_AUTHORIZATION_REQUEST when redirect_uri is not present.

The redirect URI is still validated later during
AUTHORIZATION_REQUEST, where the PAR request has already been resolved
and the effective redirect URI is available.

Changes

Skip redirect URI validation when redirect_uri is absent during
PRE_AUTHORIZATION_REQUEST
Add regression test for PAR authorization requests

Verification

Automated

Added integration test:
- testSecureRedirectUrisEnforcerExecutor_parAuthorizationRequest

Manual

Verified the following scenarios with secure-redirect-uris-enforcer enabled.

Valid PAR flow

Register a client with a valid redirect URI
Send a PAR request containing redirect_uri
Send an authorization request containing only:
- client_id
- request_uri
Complete login

Result:

Authorization request succeeds
Authorization code is issued
No exception is thrown

Invalid redirect URI

Register a client with a valid redirect URI
Send a PAR request containing an invalid redirect URI
(or one not permitted by the executor configuration)

Result:

Request is rejected
Redirect URI validation is still enforced

Copilot

🔵 Needs a closer look

It modifies security-sensitive redirect-URI enforcement logic where an incorrect relaxation could enable open-redirect bypasses, so a human should confirm the safety reasoning before approval.

0 open findings

What changed in this PR

This PR fixes a NullPointerException in SecureRedirectUrisEnforcerExecutor that occurred during Pushed Authorization Request (PAR) authorization flows. In a PAR flow, the authorization request sent to the authorization endpoint contains only client_id and request_uri, so redirect_uri is absent from the raw parameters processed during the PRE_AUTHORIZATION_REQUEST client-policy event. The executor previously passed this null value into the URI constructor, throwing an NPE before the request could be resolved. The fix skips the early redirect-URI validation when redirect_uri is not present, relying on the later AUTHORIZATION_REQUEST event (which runs after PAR resolution) to validate the effective redirect URI.

Changes:

Guard the PRE_AUTHORIZATION_REQUEST redirect-URI validation with a redirectUriParam != null check.
Add an integration regression test covering the end-to-end PAR authorization flow.

File	Description
services/src/main/java/org/keycloak/services/clientpolicy/executor/SecureRedirectUrisEnforcerExecutor.java	Adds a null check so redirect-URI validation is skipped when `redirect_uri` is absent during `PRE_AUTHORIZATION_REQUEST`, deferring enforcement to `AUTHORIZATION_REQUEST`.
testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/client/policies/SecureRedirectUrisEnforcerExecutorTest.java	Adds a regression test verifying a PAR authorization request with only `client_id`/`request_uri` succeeds and issues a token without an NPE.

🧠 Review effort: Balanced

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

Signed-off-by: Tatsuya Kurosaka <tatsuya.kurosaka.pf@hitachi.com>

lhanusov · 2026-10-22T13:06:50Z

Thanks for the fix! This addresses #53191 while keeping validation of the resolved redirect URI in AUTHORIZATION_REQUEST. Could you also add a negative PAR test where the resolved URI violates the executor policy? That would cover the deferred validation alongside the successful flow.

lhanusov

@tatsuya-kurosaka please check the comment.thanks

Signed-off-by: Tatsuya Kurosaka <tatsuya.kurosaka.pf@hitachi.com>

Copilot

🟢 Approval recommended

No active review issues were identified for the PAR redirect URI validation change.

No open issues 🎉

_{Review effort: Balanced}

Copilot

🟡 Changes recommended

Both new PAR regression tests were added to the deprecated Arquillian suite and should be moved to the Keycloak Test Framework using its client-policy setup helpers.

1 open issue

Move PAR regression tests to the Keycloak Test Framework · New

_{Review effort: Balanced}

+    @Test
+    public void testSecureRedirectUrisEnforcerExecutor_parAuthorizationRequest() throws Exception {


tatsuya-kurosaka · 2026-10-30T02:06:15Z

@lhanusov
Thanks for the review. I added a negative PAR test to cover the deferred validation. The test verifies that a redirect URI stored in the PAR is rejected during AUTHORIZATION_REQUEST when it violates the secure redirect URI policy.

lhanusov · 2026-11-09T13:08:25Z

@tatsuya-kurosaka
The added negative PAR test addresses my earlier request. Both new tests pass with the fix and reproduce the original NPE when the null check is reverted. Resolved redirect URI enforcement remains intact. This looks ready once functional CI passes and the commits are squashed with the issue reference.

@mposolda can you please trigger the build to have CI results? feel free to re-review, thank you 👍

Copilot

🟡 Changes recommended

When redirect_uri is omitted, the resolved destination is not validated before error redirects, allowing redirects to a registered URI forbidden by policy.

2 open issues

_{Review effort: Balanced}

                    throw invalidRedirectUri("Invalid parameter: clientId");
                }
-                if (isAuthFlowWithRedirectEnabled(client)) {
+                if (redirectUriParam != null && isAuthFlowWithRedirectEnabled(client)) {


Copilot AI balanced review requested due to automatic review settings October 16, 2026 08:43

tatsuya-kurosaka requested review from a team as code owners October 16, 2026 08:43

Copilot started reviewing on behalf of tatsuya-kurosaka October 16, 2026 08:44

View session

Copilot AI reviewed Oct 16, 2026

View reviewed changes

Fix secure-redirect-uris-enforcer for PAR requests

602c7b1

Signed-off-by: Tatsuya Kurosaka <tatsuya.kurosaka.pf@hitachi.com>

tatsuya-kurosaka force-pushed the issue-53191 branch from 3e68392 to 602c7b1 Compare October 16, 2026 10:23

eschultz-mesysteme mentioned this pull request Oct 19, 2026

secure-redirect-uris-enforcer fails with PAR requests during PRE_AUTHORIZATION_REQUEST #53191

Open

2 tasks done

lhanusov suggested changes Oct 22, 2026

View reviewed changes

Copilot AI balanced review requested due to automatic review settings October 30, 2026 01:54

Copilot started reviewing on behalf of tatsuya-kurosaka October 30, 2026 01:55

View session

Add negative PAR test for secure redirect URI validation

2b0eaff

Signed-off-by: Tatsuya Kurosaka <tatsuya.kurosaka.pf@hitachi.com>

tatsuya-kurosaka force-pushed the issue-53191 branch from 1704cfa to 2b0eaff Compare October 30, 2026 01:56

Copilot AI reviewed Oct 30, 2026

View reviewed changes

Copilot AI balanced review requested due to automatic review settings October 30, 2026 01:57

Copilot started reviewing on behalf of tatsuya-kurosaka October 30, 2026 01:57

View session

Copilot AI reviewed Oct 30, 2026

View reviewed changes

Comment thread ...test/java/org/keycloak/testsuite/client/policies/SecureRedirectUrisEnforcerExecutorTest.java

Comment on lines +708 to +709

@Test

public void testSecureRedirectUrisEnforcerExecutor_parAuthorizationRequest() throws Exception {

Merge branch 'main' into issue-53191

c99d3d0

Copilot AI balanced review requested due to automatic review settings November 12, 2026 12:08

Copilot started reviewing on behalf of tatsuya-kurosaka November 12, 2026 12:09

View session

Copilot AI reviewed Nov 12, 2026

View reviewed changes

Comment thread ...ain/java/org/keycloak/services/clientpolicy/executor/SecureRedirectUrisEnforcerExecutor.java

throw invalidRedirectUri("Invalid parameter: clientId");

}

if (isAuthFlowWithRedirectEnabled(client)) {

if (redirectUriParam != null && isAuthFlowWithRedirectEnabled(client)) {

Navigation Menu

Repository navigation

Fix NPE in secure-redirect-uris-enforcer during PAR authorization requests - #54053

Fix NPE in secure-redirect-uris-enforcer during PAR authorization requests#54053
tatsuya-kurosaka wants to merge 3 commits into
keycloak:mainfrom
tatsuya-kurosaka:issue-53191

tatsuya-kurosaka commented Oct 16, 2026

Uh oh!

Copilot AI left a comment

Uh oh!

lhanusov commented Oct 22, 2026

Uh oh!

lhanusov left a comment

Uh oh!

Copilot AI left a comment

Uh oh!

Copilot AI left a comment

Uh oh!

tatsuya-kurosaka commented Oct 30, 2026

Uh oh!

lhanusov commented Nov 9, 2026

Uh oh!

Copilot AI left a comment

Uh oh!

Reviewers

Uh oh!

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

		@Test
		public void testSecureRedirectUrisEnforcerExecutor_parAuthorizationRequest() throws Exception {

Conversation

tatsuya-kurosaka commented Oct 16, 2026

Summary

Changes

Verification

Automated

Manual

Valid PAR flow

Invalid redirect URI

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

🔵 Needs a closer look

Uh oh!

lhanusov commented Oct 22, 2026

Uh oh!

lhanusov left a comment

Choose a reason for hiding this comment

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

🟢 Approval recommended

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

🟡 Changes recommended

Uh oh!

tatsuya-kurosaka commented Oct 30, 2026

Uh oh!

lhanusov commented Nov 9, 2026

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

🟡 Changes recommended

Uh oh!

Reviewers

Uh oh!

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants