From 38768819e19d195ce415e6c62dc01a75bdd0ecf7 Mon Sep 17 00:00:00 2001 From: Marek Posolda Date: Fri, 28 Nov 2025 08:51:20 +0100 Subject: [PATCH] =?UTF-8?q?Make=20sure=20that=20signature=20validation=20p?= =?UTF-8?q?ossible=20to=20configure=20for=20OIDC=20id=E2=80=A6=20(#44516)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit closes #44473 Signed-off-by: mposolda Signed-off-by: Marek Posolda Co-authored-by: Ricardo Martin --- .../admin/messages/messages_en.properties | 6 +- .../add/DiscoverySettings.tsx | 18 +++- .../admin-ui/test/identity-providers/main.ts | 2 +- .../test/identity-providers/oidc.spec.ts | 9 ++ .../keycloak-server/scripts/start-server.js | 2 +- .../JWTAuthorizationGrantConfig.java | 18 ++-- ...JWTAuthorizationGrantIdentityProvider.java | 2 +- .../broker/oidc/OIDCIdentityProvider.java | 24 ++--- .../oidc/OIDCIdentityProviderConfig.java | 17 ++++ .../IdentityProviderOidcTest.java | 91 ++++++++++++++----- ...tityProviderJWTAuthorizationGrantTest.java | 6 +- 11 files changed, 142 insertions(+), 53 deletions(-) diff --git a/js/apps/admin-ui/maven-resources/theme/keycloak.v2/admin/messages/messages_en.properties b/js/apps/admin-ui/maven-resources/theme/keycloak.v2/admin/messages/messages_en.properties index 6df73bcd736..ee1cfc7a4d0 100644 --- a/js/apps/admin-ui/maven-resources/theme/keycloak.v2/admin/messages/messages_en.properties +++ b/js/apps/admin-ui/maven-resources/theme/keycloak.v2/admin/messages/messages_en.properties @@ -530,7 +530,7 @@ secretExpiresOn=Secret expires on {{time}} searchClientByName=Search client by name loginTimeout=Login timeout attributeName=Attribute [Name] -updateError=Could not update the provider {{error}} +updateError=Could not update the provider. {{error}} importUsersHelp=If true, LDAP users will be imported into the local database and synced by the configured sync policies. If import is enabled, the username and email attributes will be stored in the local database using case-insensitivity values, in lower-case. If disabled, those attributes will be treated as case-sensitive and the values will have the same format from their corresponding LDAP entries. emptyClientProfilesInstructions=There are no profiles, select 'Create client profile' to create a new client profile policyProvider.js=Define conditions for your permissions using JavaScript. It is one of the rule-based policy types supported by Keycloak, and provides flexibility to write any policy based on the Evaluation API. @@ -2027,7 +2027,7 @@ scopePermissions.clients.view-description=Policies that decide if an administrat allowEcpFlow=Allow ECP flow rsa=rsa setPasswordConfirmText=Are you sure you want to set the password for the user {{username}}? -updateErrorIdentityProvider=Could not update the provider {{error}} +updateErrorIdentityProvider=Could not update the provider. {{error}} emptyProfiles=No client profiles configured createClientProfileError=Could not create client profile\: '{{error}}' usermodel.clientRoleMapping.clientId.tooltip=Client ID for role mappings. Just client roles of this client will be added to the token. If this is unset, client roles of all clients will be added to the token. @@ -2479,7 +2479,7 @@ targetContextAttributes=Target Context Attributes targetContextAttributesHelp=Defines the evaluation of context attributes (claims) instead of identity attributes filteredByClaim=Verify essential claim rowCancelBtnAriaLabel=Cancel edits for {{messageBundle}} -validateSignatureHelp=Enable/disable signature validation of external IDP signatures. For Federated Client Authentication and JWT Authorization Grant the signature validation must be enabled. +validateSignatureHelp=Enable/disable signature validation of external IDP tokens. When enabled, Keycloak will validate JWT tokens retrieved from the Identity provider during scenarios related to user authentication, for example when Keycloak obtains IDToken or accessToken after completion of the OIDC/OAuth2 flow with Identity provider. For Federated Client Authentication and JWT Authorization Grant, the signature validation is always used regardless of the value of this switch. searchForFlow=Search for flow verifyEmail=Verify email addressClaim.locality.label=User Attribute Name for Locality diff --git a/js/apps/admin-ui/src/identity-providers/add/DiscoverySettings.tsx b/js/apps/admin-ui/src/identity-providers/add/DiscoverySettings.tsx index 720bf100051..8f8f4e3bd47 100644 --- a/js/apps/admin-ui/src/identity-providers/add/DiscoverySettings.tsx +++ b/js/apps/admin-ui/src/identity-providers/add/DiscoverySettings.tsx @@ -35,6 +35,14 @@ const Fields = ({ readOnly, isOIDC }: DiscoverySettingsProps) => { control, name: "config.pkceEnabled", }); + const jwtAuthorizationGrantEnabled = useWatch({ + control, + name: "config.jwtAuthorizationGrantEnabled", + }); + const supportsClientAssertions = useWatch({ + control, + name: "config.supportsClientAssertions", + }); return (
@@ -93,7 +101,9 @@ const Fields = ({ readOnly, isOIDC }: DiscoverySettingsProps) => { isDisabled={readOnly} stringify /> - {validateSignature === "true" && ( + {(validateSignature === "true" || + jwtAuthorizationGrantEnabled === "true" || + supportsClientAssertions == "true") && ( <> { stringify /> {useJwks === "true" ? ( - ) : ( @@ -113,10 +125,12 @@ const Fields = ({ readOnly, isOIDC }: DiscoverySettingsProps) => { diff --git a/js/apps/admin-ui/test/identity-providers/main.ts b/js/apps/admin-ui/test/identity-providers/main.ts index 629d99feaf1..89e29d8a588 100644 --- a/js/apps/admin-ui/test/identity-providers/main.ts +++ b/js/apps/admin-ui/test/identity-providers/main.ts @@ -87,7 +87,7 @@ export async function assertInvalidUrlNotification( urlType: UrlType, ) { await expect(page.getByTestId("last-alert")).toHaveText( - `Could not update the provider The url [${urlType}${urlType.startsWith("single") ? "U" : "_u"}rl] is malformed`, + `Could not update the provider. The url [${urlType}${urlType.startsWith("single") ? "U" : "_u"}rl] is malformed`, ); } diff --git a/js/apps/admin-ui/test/identity-providers/oidc.spec.ts b/js/apps/admin-ui/test/identity-providers/oidc.spec.ts index aae89c498fb..86c4b20bd1c 100644 --- a/js/apps/admin-ui/test/identity-providers/oidc.spec.ts +++ b/js/apps/admin-ui/test/identity-providers/oidc.spec.ts @@ -67,6 +67,15 @@ test.describe.serial("OIDC identity provider test", () => { await assertPkceMethodExists(page); await clickSaveButton(page); + await assertNotificationMessage( + page, + "Could not update the provider. The 'Validating public key' is required when 'Validate signatures' enabled and 'Use JWKS URL' disabled", + ); + + await switchOn(page, "#config\\.useJwksUrl"); + await assertJwksUrlExists(page, true); + await clickSaveButton(page); + await assertNotificationMessage(page, "Provider successfully updated"); }); }); diff --git a/js/apps/keycloak-server/scripts/start-server.js b/js/apps/keycloak-server/scripts/start-server.js index 8b886d05bac..5e8b3d9d747 100755 --- a/js/apps/keycloak-server/scripts/start-server.js +++ b/js/apps/keycloak-server/scripts/start-server.js @@ -60,7 +60,7 @@ async function startServer() { path.join(SERVER_DIR, `bin/kc${SCRIPT_EXTENSION}`), [ "start-dev", - `--features="login:v2,account:v3,admin-fine-grained-authz:v2,transient-users,oid4vc-vci,organization,declarative-ui,quick-theme,spiffe,kubernetes-service-accounts,workflows"`, + `--features="transient-users,oid4vc-vci,declarative-ui,quick-theme,spiffe,kubernetes-service-accounts,workflows,client-auth-federated,jwt-authorization-grant"`, ...keycloakArgs, ], { diff --git a/services/src/main/java/org/keycloak/broker/jwtauthorizationgrant/JWTAuthorizationGrantConfig.java b/services/src/main/java/org/keycloak/broker/jwtauthorizationgrant/JWTAuthorizationGrantConfig.java index e99f7d2a98d..b38f4585a64 100644 --- a/services/src/main/java/org/keycloak/broker/jwtauthorizationgrant/JWTAuthorizationGrantConfig.java +++ b/services/src/main/java/org/keycloak/broker/jwtauthorizationgrant/JWTAuthorizationGrantConfig.java @@ -8,23 +8,27 @@ import static org.keycloak.protocol.oidc.OIDCLoginProtocol.ISSUER; public interface JWTAuthorizationGrantConfig { - public static final String JWT_AUTHORIZATION_GRANT_ENABLED = "jwtAuthorizationGrantEnabled"; + String JWT_AUTHORIZATION_GRANT_ENABLED = "jwtAuthorizationGrantEnabled"; - public static final String JWT_AUTHORIZATION_GRANT_ASSERTION_REUSE_ALLOWED = "jwtAuthorizationGrantAssertionReuseAllowed"; + String JWT_AUTHORIZATION_GRANT_ASSERTION_REUSE_ALLOWED = "jwtAuthorizationGrantAssertionReuseAllowed"; - public static final String JWT_AUTHORIZATION_GRANT_MAX_ALLOWED_ASSERTION_EXPIRATION = "jwtAuthorizationGrantMaxAllowedAssertionExpiration"; + String JWT_AUTHORIZATION_GRANT_MAX_ALLOWED_ASSERTION_EXPIRATION = "jwtAuthorizationGrantMaxAllowedAssertionExpiration"; - public static final String JWT_AUTHORIZATION_GRANT_ASSERTION_SIGNATURE_ALG = "jwtAuthorizationGrantAssertionSignatureAlg"; + String JWT_AUTHORIZATION_GRANT_ASSERTION_SIGNATURE_ALG = "jwtAuthorizationGrantAssertionSignatureAlg"; - public static final String JWT_AUTHORIZATION_GRANT_ALLOWED_CLOCK_SKEW = "jwtAuthorizationGrantAllowedClockSkew"; + String JWT_AUTHORIZATION_GRANT_ALLOWED_CLOCK_SKEW = "jwtAuthorizationGrantAllowedClockSkew"; Map getConfig(); - default boolean getJWTAuthorizationGrantEnabled() { + default boolean isJWTAuthorizationGrantEnabled() { return Boolean.parseBoolean(getConfig().getOrDefault(JWT_AUTHORIZATION_GRANT_ENABLED, "false")); } - default boolean getJWTAuthorizationGrantAssertionReuseAllowed() { + default void setJWTAuthorizationGrantEnabled(boolean jwtAuthorizationGrantEnableds) { + getConfig().put(JWT_AUTHORIZATION_GRANT_ENABLED, String.valueOf(jwtAuthorizationGrantEnableds)); + } + + default boolean isJWTAuthorizationGrantAssertionReuseAllowed() { return Boolean.parseBoolean(getConfig().getOrDefault(JWT_AUTHORIZATION_GRANT_ASSERTION_REUSE_ALLOWED, "false")); } diff --git a/services/src/main/java/org/keycloak/broker/jwtauthorizationgrant/JWTAuthorizationGrantIdentityProvider.java b/services/src/main/java/org/keycloak/broker/jwtauthorizationgrant/JWTAuthorizationGrantIdentityProvider.java index c168e203791..85edc0824fb 100644 --- a/services/src/main/java/org/keycloak/broker/jwtauthorizationgrant/JWTAuthorizationGrantIdentityProvider.java +++ b/services/src/main/java/org/keycloak/broker/jwtauthorizationgrant/JWTAuthorizationGrantIdentityProvider.java @@ -51,7 +51,7 @@ public class JWTAuthorizationGrantIdentityProvider implements JWTAuthorizationGr @Override public boolean isAssertionReuseAllowed() { - return config.getJWTAuthorizationGrantAssertionReuseAllowed(); + return config.isJWTAuthorizationGrantAssertionReuseAllowed(); } @Override diff --git a/services/src/main/java/org/keycloak/broker/oidc/OIDCIdentityProvider.java b/services/src/main/java/org/keycloak/broker/oidc/OIDCIdentityProvider.java index d11352c3836..3ccf0f5d312 100755 --- a/services/src/main/java/org/keycloak/broker/oidc/OIDCIdentityProvider.java +++ b/services/src/main/java/org/keycloak/broker/oidc/OIDCIdentityProvider.java @@ -630,7 +630,15 @@ public class OIDCIdentityProvider extends AbstractOAuth2IdentityProvider verify(v.getJws()), + FederatedJWTClientValidator validator = new FederatedJWTClientValidator(context, v -> verifySignature(v.getJws()), config.getIssuer(), config.getAllowedClockSkew(), config.isSupportsClientAssertionReuse()); if (!Profile.isFeatureEnabled(Profile.Feature.CLIENT_AUTH_FEDERATED)) { @@ -1065,24 +1073,16 @@ public class OIDCIdentityProvider extends AbstractOAuth2IdentityProvider r.sslRequired(SslRequired.EXTERNAL.name())); @@ -438,6 +418,67 @@ public class IdentityProviderOidcTest extends AbstractIdentityProviderTest { managedRealm.cleanup().add(r -> r.identityProviders().get(id).remove()); } + @Test + public void testOIDCKeysRequiredForVariousConfigs() { + String id = create(createRep("keycloak-oidc", "keycloak-oidc")); + + IdentityProviderResource resource = this.managedRealm.admin().identityProviders().get("keycloak-oidc"); + IdentityProviderRepresentation representation = resource.toRepresentation(); + OIDCIdentityProviderConfigRep oidcConfig = new OIDCIdentityProviderConfigRep(representation); + + // OIDC Keys required when "validate signature" is ON + oidcConfig.setValidateSignature(true); + try { + resource.update(representation); + fail("Not expected to update identity provider"); + } catch (Exception e) { + assertError(e, "The 'Validating public key' is required when 'Validate signatures' enabled and 'Use JWKS URL' disabled"); + } + + // OIDC Keys (set by JWKS URL) required when "validate signature" is ON + oidcConfig.setUseJwksUrl(true); + try { + resource.update(representation); + fail("JWKS URL is required when 'Validate signatures' enabled and 'Use JWKS URL' enabled"); + } catch (Exception e) { + assertError(e, "JWKS URL is required when 'Validate signatures' enabled and 'Use JWKS URL' enabled"); + } + + // OIDC Keys (set by JWKS URL) required when "authorization grant" is ON + oidcConfig.setValidateSignature(false); + oidcConfig.setJWTAuthorizationGrantEnabled(true); + try { + resource.update(representation); + fail("JWKS URL is required when 'Validate signatures' enabled and 'Use JWKS URL' enabled"); + } catch (Exception e) { + assertError(e, "JWKS URL is required when 'JWT Authorization Grant' enabled and 'Use JWKS URL' enabled"); + } + + // OIDC Keys (set by JWKS URL) required when "federated client authentication" is ON + oidcConfig.setJWTAuthorizationGrantEnabled(false); + oidcConfig.setSupportsClientAssertions(true); + try { + resource.update(representation); + fail("JWKS URL is required when 'Validate signatures' enabled and 'Use JWKS URL' enabled"); + } catch (Exception e) { + assertError(e, "JWKS URL is required when 'Supports client assertions' enabled and 'Use JWKS URL' enabled"); + } + + // Successful update when JWKS URL set + oidcConfig.setJwksUrl("https://foo"); + resource.update(representation); + + managedRealm.cleanup().add(r -> r.identityProviders().get(id).remove()); + } + + private void assertError(Exception e, String expectedError) { + assertTrue(e instanceof ClientErrorException); + Response response = ClientErrorException.class.cast(e).getResponse(); + assertEquals( Response.Status.BAD_REQUEST.getStatusCode(), response.getStatus()); + ErrorRepresentation error = ((ClientErrorException) e).getResponse().readEntity(ErrorRepresentation.class); + assertEquals(expectedError, error.getErrorMessage()); + } + @Test public void testNoExport() { String id = create(createRep("keycloak-oidc", "keycloak-oidc")); diff --git a/tests/base/src/test/java/org/keycloak/tests/oauth/OIDCIdentityProviderJWTAuthorizationGrantTest.java b/tests/base/src/test/java/org/keycloak/tests/oauth/OIDCIdentityProviderJWTAuthorizationGrantTest.java index 49c2c54d219..ed6e97cb8eb 100644 --- a/tests/base/src/test/java/org/keycloak/tests/oauth/OIDCIdentityProviderJWTAuthorizationGrantTest.java +++ b/tests/base/src/test/java/org/keycloak/tests/oauth/OIDCIdentityProviderJWTAuthorizationGrantTest.java @@ -43,9 +43,13 @@ public class OIDCIdentityProviderJWTAuthorizationGrantTest extends AbstractJWTAu rep.getConfig().put(OIDCIdentityProviderConfig.VALIDATE_SIGNATURE, "false"); }); + // Test with JWT signed by invalid key. It tests that signature validation is triggered even if "validate signature" switch on OIDC provider is false + testInvalidSignature(); + + // Test with correct signature String jwt = getIdentityProvider().encodeToken(createAuthorizationGrantToken("basic-user-id", oAuthClient.getEndpoints().getIssuer(), IDP_ISSUER)); AccessTokenResponse response = oAuthClient.jwtAuthorizationGrantRequest(jwt).send(); - assertFailure("Signature validation not enabled for issuer", response, events.poll()); + assertSuccess("test-app", response); } public static class JWTAuthorizationGrantRealmConfig extends AbstractJWTAuthorizationGrantTest.JWTAuthorizationGrantRealmConfig {