Skip to content

Commit

Permalink
Skip secure redirect validation if redirect URI-dependent flows are n…
Browse files Browse the repository at this point in the history
…ot used

This commit adjusts the secure redirect validation logic to bypass the check
when neither the standard flow nor the implicit flow is used. This ensures
that the validation only occurs for authentication flows that rely on
redirect URIs, preventing unnecessary validation for flows that do not
require redirect URIs.

Additionally, the logic for handling Update and Register operations has been
unified to ensure consistency. Empty URLs are now no longer allowed during
Update operations, as permitting them is not meaningful for flows
relying on redirect URIs.

Closes keycloak#33734

Signed-off-by: Sven-Torben Janus <[email protected]>
  • Loading branch information
sventorben committed Oct 11, 2024
1 parent 25da859 commit be17570
Showing 1 changed file with 27 additions and 17 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -142,7 +142,7 @@ public void setAllowWildcardContextPath(boolean allowWildcardContextPath) {
public List<String> getAllowPermittedDomains() {
return allowPermittedDomains;
}

public void setAllowPermittedDomains(List<String> permittedDomains) {
this.allowPermittedDomains = permittedDomains;
}
Expand Down Expand Up @@ -173,26 +173,14 @@ public void executeOnEvent(ClientPolicyContext context) throws ClientPolicyExcep
switch (context.getEvent()) {
case REGISTER:
if (context instanceof AdminClientRegisterContext || context instanceof DynamicClientRegisterContext) {
ClientRepresentation client = ((ClientCRUDContext)context).getProposedClientRepresentation();
List<String> redirectUris = client.getRedirectUris();
if (redirectUris == null || redirectUris.isEmpty()) {
throw invalidRedirectUri(ERR_GENERAL);
}
verifyRedirectUris(client.getRootUrl(), redirectUris);
verifyPostLogoutRedirectUriUpdate(client);
verifyRedirectUris((ClientCRUDContext) context);
} else {
throw invalidRedirectUri(ERR_GENERAL);
}
return;
case UPDATE:
if (context instanceof AdminClientUpdateContext || context instanceof DynamicClientUpdateContext) {
ClientRepresentation client = ((ClientCRUDContext)context).getProposedClientRepresentation();
List<String> redirectUris = client.getRedirectUris();
if (redirectUris == null || redirectUris.isEmpty()) {
return;
}
verifyRedirectUris(client.getRootUrl(), redirectUris);
verifyPostLogoutRedirectUriUpdate(client);
verifyRedirectUris((ClientCRUDContext) context);
} else {
throw invalidRedirectUri(ERR_GENERAL);
}
Expand All @@ -205,12 +193,34 @@ public void executeOnEvent(ClientPolicyContext context) throws ClientPolicyExcep
if (client == null) {
throw invalidRedirectUri("Invalid parameter: clientId");
}
verifyRedirectUri(redirectUriParam, true);
if (isAuthFlowWithRedirectEnabled(client)) {
verifyRedirectUri(redirectUriParam, true);
}
return;
default:
}
}

private void verifyRedirectUris(ClientCRUDContext context) throws ClientPolicyException {
ClientRepresentation client = context.getProposedClientRepresentation();
if (isAuthFlowWithRedirectEnabled(client)) {
List<String> redirectUris = client.getRedirectUris();
if (redirectUris == null || redirectUris.isEmpty()) {
throw invalidRedirectUri(ERR_GENERAL);
}
verifyRedirectUris(client.getRootUrl(), redirectUris);
verifyPostLogoutRedirectUriUpdate(client);
}
}

private boolean isAuthFlowWithRedirectEnabled(ClientModel client) {
return client.isStandardFlowEnabled() || client.isImplicitFlowEnabled();
}

private static boolean isAuthFlowWithRedirectEnabled(ClientRepresentation client) {
return client.isStandardFlowEnabled() == Boolean.TRUE || client.isImplicitFlowEnabled() == Boolean.TRUE;
}

private void verifyPostLogoutRedirectUriUpdate(ClientRepresentation client) throws ClientPolicyException {
List<String> postLogoutRedirectUris = OIDCAdvancedConfigWrapper.fromClientRepresentation(client).getPostLogoutRedirectUris();
if (postLogoutRedirectUris == null || postLogoutRedirectUris.isEmpty()) {
Expand Down Expand Up @@ -240,7 +250,7 @@ void verifyRedirectUri(String redirectUri, boolean isRedirectUriParam) throws Cl
logger.debugv("URISyntaxException - input = {0}, errMessage = {1], errReason = {2}, redirectUri = {3}", e.getInput(), e.getMessage(), e.getReason(), redirectUri);
throw invalidRedirectUri(ERR_GENERAL);
}

validation.validate();
}

Expand Down

0 comments on commit be17570

Please sign in to comment.