From 1316705da5d8841bc101e1adadd0e9e1b2338e87 Mon Sep 17 00:00:00 2001 From: Martin Kanis Date: Thu, 5 Nov 2026 13:14:10 +0100 Subject: [PATCH 01/16] [CVE-2026-116468] Privilege escalation: delegated admin to realm-admin via group move (incomplete fix of CVE-2026-94000) Closes #54341 Signed-off-by: Martin Kanis --- .../resource/OrganizationGroupResource.java | 1 + .../resources/admin/GroupResource.java | 1 + .../fgap/GroupResourceTypeEvaluationTest.java | 28 +++++ .../authz/rbac/RealmAdminAccessTest.java | 101 ++++++++++++++++++ 4 files changed, 131 insertions(+) diff --git a/services/src/main/java/org/keycloak/organization/admin/resource/OrganizationGroupResource.java b/services/src/main/java/org/keycloak/organization/admin/resource/OrganizationGroupResource.java index ef5431af85fd..5183ff3f93ad 100644 --- a/services/src/main/java/org/keycloak/organization/admin/resource/OrganizationGroupResource.java +++ b/services/src/main/java/org/keycloak/organization/admin/resource/OrganizationGroupResource.java @@ -282,6 +282,7 @@ public Response addSubGroup(GroupRepresentation rep) { // Move the group if it's not already a child of this group if (!Objects.equals(child.getParentId(), group.getId())) { + GroupUtils.checkAdminGroupRoles(group, auth); session.groups().moveGroup(realm, child, group); } adminEvent.operation(OperationType.UPDATE); diff --git a/services/src/main/java/org/keycloak/services/resources/admin/GroupResource.java b/services/src/main/java/org/keycloak/services/resources/admin/GroupResource.java index 3941acd92d78..cc86b5c4e878 100755 --- a/services/src/main/java/org/keycloak/services/resources/admin/GroupResource.java +++ b/services/src/main/java/org/keycloak/services/resources/admin/GroupResource.java @@ -259,6 +259,7 @@ public Response addChild(GroupRepresentation rep) { throw ErrorResponse.error("Cannot move group '" + child.getName() + "' under itself or its descendants", Response.Status.BAD_REQUEST); } if (!Objects.equals(child.getParentId(), group.getId())) { + GroupUtils.checkAdminGroupRoles(group, auth); realm.moveGroup(child, group); } adminEvent.operation(OperationType.UPDATE); diff --git a/tests/base/src/test/java/org/keycloak/tests/admin/authz/fgap/GroupResourceTypeEvaluationTest.java b/tests/base/src/test/java/org/keycloak/tests/admin/authz/fgap/GroupResourceTypeEvaluationTest.java index 76885de7c50b..d9236d63c2c7 100644 --- a/tests/base/src/test/java/org/keycloak/tests/admin/authz/fgap/GroupResourceTypeEvaluationTest.java +++ b/tests/base/src/test/java/org/keycloak/tests/admin/authz/fgap/GroupResourceTypeEvaluationTest.java @@ -62,6 +62,7 @@ import static org.hamcrest.Matchers.hasSize; import static org.hamcrest.Matchers.instanceOf; import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertNull; import static org.junit.jupiter.api.Assertions.assertTrue; import static org.junit.jupiter.api.Assertions.fail; @@ -129,6 +130,33 @@ public void testMoveGroupRequiresManagePermissionOnChild() { } } + @Test + public void testMoveGroupRequiresPermissionToGrantAdminRoles() { + GroupRepresentation source = createGroup("source"); + GroupRepresentation destination = createGroup("destination"); + UserPolicyRepresentation policy = createUserPolicy(realm, adminPermissionsClient, "Only My Admin User Policy", + realm.admin().users().search("myadmin").get(0).getId()); + createGroupPermission(Set.of(source, destination), Set.of(VIEW, MANAGE), policy); + + // Resource permissions allow the move while the destination grants no administrative roles. + try (Response response = realmAdminClient.realm(realm.getName()).groups().group(destination.getId()).subGroup(source)) { + assertEquals(Response.Status.NO_CONTENT.getStatusCode(), response.getStatus()); + } + try (Response response = realm.admin().groups().add(source)) { + assertEquals(Response.Status.NO_CONTENT.getStatusCode(), response.getStatus()); + } + + String realmManagementId = realm.admin().clients().findByClientId(Constants.REALM_MANAGEMENT_CLIENT_ID).get(0).getId(); + RoleRepresentation realmAdminRole = realm.admin().clients().get(realmManagementId).roles().get(AdminRoles.REALM_ADMIN).toRepresentation(); + realm.admin().groups().group(destination.getId()).roles().clientLevel(realmManagementId).add(List.of(realmAdminRole)); + + // The same resource permissions do not authorize granting the destination's administrative roles. + try (Response response = realmAdminClient.realm(realm.getName()).groups().group(destination.getId()).subGroup(source)) { + assertEquals(Response.Status.FORBIDDEN.getStatusCode(), response.getStatus()); + } + assertNull(realm.admin().groups().group(source.getId()).toRepresentation().getParentId()); + } + @Test public void testCanViewUserByViewGroupMembers() { UserRepresentation myadmin = realm.admin().users().search("myadmin").get(0); diff --git a/tests/base/src/test/java/org/keycloak/tests/admin/authz/rbac/RealmAdminAccessTest.java b/tests/base/src/test/java/org/keycloak/tests/admin/authz/rbac/RealmAdminAccessTest.java index 1372f6e865f7..746e059fe881 100644 --- a/tests/base/src/test/java/org/keycloak/tests/admin/authz/rbac/RealmAdminAccessTest.java +++ b/tests/base/src/test/java/org/keycloak/tests/admin/authz/rbac/RealmAdminAccessTest.java @@ -17,6 +17,7 @@ import org.keycloak.admin.client.Keycloak; import org.keycloak.admin.client.resource.BearerAuthFilter; import org.keycloak.admin.client.resource.ClientResource; +import org.keycloak.admin.client.resource.OrganizationResource; import org.keycloak.admin.client.resource.RealmResource; import org.keycloak.admin.client.resource.RoleMappingResource; import org.keycloak.common.VerificationException; @@ -45,6 +46,8 @@ import org.keycloak.testframework.annotations.InjectKeycloakUrls; import org.keycloak.testframework.annotations.KeycloakIntegrationTest; import org.keycloak.testframework.realm.ClientBuilder; +import org.keycloak.testframework.realm.GroupBuilder; +import org.keycloak.testframework.realm.OrganizationBuilder; import org.keycloak.testframework.realm.RoleBuilder; import org.keycloak.testframework.realm.UserBuilder; import org.keycloak.testframework.server.KeycloakUrls; @@ -1752,6 +1755,104 @@ public void testManageUsersAdminCannotAddOrgMemberToAdminGroupViaOrgEndpoint() { }); } + @Test + public void testManageUsersAdminCannotMoveGroupUnderAdminGroup() { + String realmName = "test-realm"; + RealmResource testRealm = createRealm(adminClient, realmName); + UserRepresentation attacker = createUser(testRealm, "limited-admin"); + grantRealmManagementRole(testRealm, attacker.getUsername(), AdminRoles.MANAGE_USERS); + UserRepresentation realmAdmin = createUser(testRealm, "realm-admin"); + grantRealmManagementRole(testRealm, realmAdmin.getUsername(), AdminRoles.REALM_ADMIN); + + String realmManagementId = testRealm.clients().findByClientId(Constants.REALM_MANAGEMENT_CLIENT_ID).get(0).getId(); + RoleRepresentation realmAdminRole = testRealm.clients().get(realmManagementId).roles().get(AdminRoles.REALM_ADMIN).toRepresentation(); + testRealm.roles().create(RoleBuilder.create().name("admin-composite").build()); + testRealm.roles().get("admin-composite").addComposites(List.of(realmAdminRole)); + + GroupRepresentation adminGroup = GroupBuilder.create("admin-ancestor").build(); + adminGroup.setId(ApiUtil.getCreatedId(testRealm.groups().add(adminGroup))); + testRealm.groups().group(adminGroup.getId()).roles().realmLevel() + .add(List.of(testRealm.roles().get("admin-composite").toRepresentation())); + // The destination has no direct roles: its ancestor grants realm-admin through a composite. + GroupRepresentation destination = GroupBuilder.create("destination").build(); + destination.setId(ApiUtil.getCreatedId(testRealm.groups().group(adminGroup.getId()).subGroup(destination))); + GroupRepresentation ordinaryParent = GroupBuilder.create("ordinary-parent").build(); + ordinaryParent.setId(ApiUtil.getCreatedId(testRealm.groups().add(ordinaryParent))); + + GroupRepresentation source = GroupBuilder.create("source").build(); + source.setId(ApiUtil.getCreatedId(testRealm.groups().add(source))); + testRealm.users().get(attacker.getId()).joinGroup(source.getId()); + + runAs(realmName, attacker.getUsername(), client -> { + try (Response response = client.realm(realmName).groups().group(ordinaryParent.getId()).subGroup(source)) { + assertEquals(Status.NO_CONTENT.getStatusCode(), response.getStatus()); + } + try (Response response = client.realm(realmName).groups().group(destination.getId()).subGroup(source)) { + assertEquals(Status.FORBIDDEN.getStatusCode(), response.getStatus()); + } + }); + + assertEquals(ordinaryParent.getId(), testRealm.groups().group(source.getId()).toRepresentation().getParentId()); + assertFalse(testRealm.users().get(attacker.getId()).roles().clientLevel(realmManagementId).listEffective().stream() + .anyMatch(role -> AdminRoles.REALM_ADMIN.equals(role.getName()))); + + // A realm-local administrator can still move the group and grant the inherited roles. + runAs(realmName, realmAdmin.getUsername(), client -> { + try (Response response = client.realm(realmName).groups().group(destination.getId()).subGroup(source)) { + assertEquals(Status.NO_CONTENT.getStatusCode(), response.getStatus()); + } + }); + assertEquals(destination.getId(), testRealm.groups().group(source.getId()).toRepresentation().getParentId()); + assertTrue(testRealm.users().get(attacker.getId()).roles().clientLevel(realmManagementId).listEffective().stream() + .anyMatch(role -> AdminRoles.REALM_ADMIN.equals(role.getName()))); + } + + @Test + public void testDelegatedAdminCannotMoveOrganizationGroupUnderAdminGroup() { + String realmName = "test-realm"; + RealmResource testRealm = createRealm(adminClient, realmName); + RealmRepresentation rep = testRealm.toRepresentation(); + rep.setOrganizationsEnabled(true); + testRealm.update(rep); + + UserRepresentation attacker = createUser(testRealm, "limited-admin"); + grantRealmManagementRole(testRealm, attacker.getUsername(), AdminRoles.MANAGE_USERS); + grantRealmManagementRole(testRealm, attacker.getUsername(), AdminRoles.MANAGE_ORGANIZATIONS); + + String organizationId = ApiUtil.getCreatedId(testRealm.organizations().create(OrganizationBuilder.create("test-org").build())); + OrganizationResource organization = testRealm.organizations().get(organizationId); + ApiUtil.getCreatedId(organization.members().addMember(attacker.getId())); + + GroupRepresentation adminGroup = GroupBuilder.create("admin-group").build(); + adminGroup.setId(ApiUtil.getCreatedId(organization.groups().addTopLevelGroup(adminGroup))); + ClientRepresentation realmManagement = testRealm.clients().findByClientId(Constants.REALM_MANAGEMENT_CLIENT_ID).get(0); + RoleRepresentation realmAdmin = testRealm.clients().get(realmManagement.getId()).roles().get(AdminRoles.REALM_ADMIN).toRepresentation(); + organization.groups().group(adminGroup.getId()).roles().clientLevel(realmManagement.getId()).add(List.of(realmAdmin)); + + GroupRepresentation source = GroupBuilder.create("source").build(); + source.setId(ApiUtil.getCreatedId(organization.groups().addTopLevelGroup(source))); + organization.groups().group(source.getId()).addMember(attacker.getId()); + GroupRepresentation ordinaryParent = GroupBuilder.create("ordinary-parent").build(); + ordinaryParent.setId(ApiUtil.getCreatedId(organization.groups().addTopLevelGroup(ordinaryParent))); + + runAs(realmName, attacker.getUsername(), client -> { + try (Response response = client.realm(realmName).organizations().get(organizationId).groups() + .group(ordinaryParent.getId()).addSubGroup(source)) { + assertEquals(Status.NO_CONTENT.getStatusCode(), response.getStatus()); + } + try (Response response = client.realm(realmName).organizations().get(organizationId).groups() + .group(adminGroup.getId()).addSubGroup(source)) { + assertEquals(Status.FORBIDDEN.getStatusCode(), response.getStatus()); + } + }); + + assertEquals(ordinaryParent.getId(), organization.groups().group(source.getId()).toRepresentation(false).getParentId()); + try (Response response = organization.groups().group(adminGroup.getId()).addSubGroup(source)) { + assertEquals(Status.NO_CONTENT.getStatusCode(), response.getStatus()); + } + assertEquals(adminGroup.getId(), organization.groups().group(source.getId()).toRepresentation(false).getParentId()); + } + private void grantRealmManagementRole(RealmResource testRealm, GroupRepresentation group, String role) { ClientRepresentation realmMgmt = testRealm.clients() .findByClientId(Constants.REALM_MANAGEMENT_CLIENT_ID).get(0); From 56ffd6a1afbe3cbd320e4d8034df15f842ddc31e Mon Sep 17 00:00:00 2001 From: Stefan Guilhen Date: Fri, 30 Oct 2026 12:15:24 -0300 Subject: [PATCH 02/16] Document LDAP bind account permissions for recursive group retrieval strategy Closes #54366 Signed-off-by: Stefan Guilhen --- .../server_admin/topics/user-federation/ldap.adoc | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/docs/documentation/server_admin/topics/user-federation/ldap.adoc b/docs/documentation/server_admin/topics/user-federation/ldap.adoc index 04e703a686fa..bf2dfe42cb1e 100644 --- a/docs/documentation/server_admin/topics/user-federation/ldap.adoc +++ b/docs/documentation/server_admin/topics/user-federation/ldap.adoc @@ -186,6 +186,13 @@ This mapper grants a specified {project_name} role to each {project_name} user f Group Mapper:: This mapper maps LDAP groups from a branch of an LDAP tree into groups within {project_name}. This mapper also propagates user-group mappings from LDAP into user-group mappings in {project_name}. ++ +[NOTE] +==== +The `LOAD_GROUPS_BY_MEMBER_ATTRIBUTE_RECURSIVELY` value of the *User Groups Retrieve Strategy* option is available only for Microsoft Active Directory. It uses the `LDAP_MATCHING_RULE_IN_CHAIN` extension to resolve nested group memberships. Active Directory evaluates the `member` and `memberOf` attributes of the user and group objects when processing these queries, so the LDAP bind account configured for the provider must have permission to read these attributes on both the user and the group objects (e.g. `Read all properties` on the relevant user and group `OU`s). If the bind account lacks this permission, Active Directory does not return an error, but the group memberships, including the members of nested groups, might not be retrieved or displayed in {project_name}. + +The same applies to the `LOAD_ROLES_BY_MEMBER_ATTRIBUTE_RECURSIVELY` strategy of the Role Mapper. +==== [[_msad_mapper]] MSAD User Account Mapper:: From a50ba94f312898ebedcc92b9791935f86a81c8f6 Mon Sep 17 00:00:00 2001 From: akasakariko <148060537+akasakariko@users.noreply.github.com> Date: Mon, 2 Nov 2026 20:45:22 +0900 Subject: [PATCH 03/16] Reject empty SCIM group members on PUT Closes #54530 Signed-off-by: akasakariko <148060537+akasakariko@users.noreply.github.com> --- .../model/group/GroupResourceTypeProvider.java | 6 +----- .../org/keycloak/tests/scim/tck/GroupTest.java | 16 +++++++++++----- 2 files changed, 12 insertions(+), 10 deletions(-) diff --git a/scim/model/src/main/java/org/keycloak/scim/model/group/GroupResourceTypeProvider.java b/scim/model/src/main/java/org/keycloak/scim/model/group/GroupResourceTypeProvider.java index aee15e030921..d2e0056921f7 100644 --- a/scim/model/src/main/java/org/keycloak/scim/model/group/GroupResourceTypeProvider.java +++ b/scim/model/src/main/java/org/keycloak/scim/model/group/GroupResourceTypeProvider.java @@ -13,7 +13,6 @@ import java.util.ArrayList; import java.util.List; import java.util.Objects; -import java.util.Optional; import java.util.function.BiPredicate; import java.util.stream.Stream; @@ -42,7 +41,6 @@ import org.keycloak.scim.model.filter.ScimJPAPredicateEvaluator; import org.keycloak.scim.protocol.ForbiddenException; import org.keycloak.scim.resource.group.Group; -import org.keycloak.scim.resource.group.Member; import org.keycloak.scim.resource.schema.attribute.Attribute; import org.keycloak.scim.resource.spi.AbstractScimResourceTypeProvider; import org.keycloak.scim.resource.spi.MembershipChange; @@ -98,9 +96,7 @@ protected Group createResourceTypeInstance(GroupModel model, List attrib @Override public Group update(Group resource) { - List members = resource.getMembers(); - - if (!Optional.ofNullable(members).orElse(List.of()).isEmpty()) { + if (resource.getMembers() != null) { throw new ModelValidationException("Managing members on updates is not supported"); } diff --git a/scim/tests/base/src/test/java/org/keycloak/tests/scim/tck/GroupTest.java b/scim/tests/base/src/test/java/org/keycloak/tests/scim/tck/GroupTest.java index d0e562ca955a..7766a553461b 100644 --- a/scim/tests/base/src/test/java/org/keycloak/tests/scim/tck/GroupTest.java +++ b/scim/tests/base/src/test/java/org/keycloak/tests/scim/tck/GroupTest.java @@ -33,6 +33,8 @@ import org.keycloak.testframework.util.ApiUtil; import org.junit.jupiter.api.Test; +import org.junit.jupiter.params.ParameterizedTest; +import org.junit.jupiter.params.provider.ValueSource; import static org.hamcrest.MatcherAssert.assertThat; import static org.hamcrest.Matchers.equalTo; @@ -824,8 +826,9 @@ public void testGroupMembersOnCreate() { } } - @Test - public void testGroupMembersReplace() { + @ParameterizedTest + @ValueSource(booleans = {false, true}) + public void testGroupMembersReplace(boolean emptyMembers) { // create users via SCIM User userA = createScimUser(); User userB = createScimUser(); @@ -842,16 +845,19 @@ public void testGroupMembersReplace() { .add("members", userB.getId()) .build()); - // PUT update: replace members with a different set + // PUT update: replace members with a different or empty set Group toUpdate = client.groups().get(group.getId()); - toUpdate.setMembers(null); - toUpdate.addMember(userC.getId()); + toUpdate.setMembers(new ArrayList<>()); + if (!emptyMembers) { + toUpdate.addMember(userC.getId()); + } try { client.groups().update(toUpdate); fail("Should have thrown an exception"); } catch (ScimClientException e) { ErrorResponse error = e.getError(); assertNotNull(error); + assertEquals(Status.BAD_REQUEST.getStatusCode(), error.getStatusInt()); assertEquals("Managing members on updates is not supported", error.getDetail()); } } From 3acdf064261eef21b9a96814decaec8e452b4361 Mon Sep 17 00:00:00 2001 From: Peter Zaoral Date: Fri, 6 Nov 2026 14:35:55 +0100 Subject: [PATCH 04/16] Update Quarkus JUnit test dependencies (#54734) Closes: #54732 Signed-off-by: Peter Zaoral --- quarkus/deployment/pom.xml | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/quarkus/deployment/pom.xml b/quarkus/deployment/pom.xml index 5754c57415c2..83f8ffc46592 100644 --- a/quarkus/deployment/pom.xml +++ b/quarkus/deployment/pom.xml @@ -215,7 +215,7 @@ io.quarkus - quarkus-junit5-internal + quarkus-junit-internal test @@ -226,7 +226,7 @@ io.quarkus - quarkus-junit5-mockito + quarkus-junit-mockito test From baf4aaabb78314c63db4682c6acf06a0456426eb Mon Sep 17 00:00:00 2001 From: jimmychakkalakal <47317106+jimmychakkalakal@users.noreply.github.com> Date: Fri, 6 Nov 2026 13:45:55 +0000 Subject: [PATCH 05/16] Always validate post-logout redirect URIs in SecureRedirectUrisEnforcerExecutor (#53038) Closes #50982 Signed-off-by: Jimmy Chakkalakal Signed-off-by: jimmychakkalakal <47317106+jimmychakkalakal@users.noreply.github.com> Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com> --- .../topics/changes/changes-27_0_0.adoc | 32 ++ .../SecureRedirectUrisEnforcerExecutor.java | 56 +++- .../policies/AbstractClientPoliciesTest.java | 8 + ...ecureRedirectUrisEnforcerExecutorTest.java | 317 ++++++++++++++++++ ...ecureRedirectUrisEnforcerExecutorTest.java | 24 +- 5 files changed, 427 insertions(+), 10 deletions(-) create mode 100644 tests/base/src/test/java/org/keycloak/tests/client/policies/SecureRedirectUrisEnforcerExecutorTest.java diff --git a/docs/documentation/upgrading/topics/changes/changes-27_0_0.adoc b/docs/documentation/upgrading/topics/changes/changes-27_0_0.adoc index 5c0dd586476f..2d44dd5edf72 100644 --- a/docs/documentation/upgrading/topics/changes/changes-27_0_0.adoc +++ b/docs/documentation/upgrading/topics/changes/changes-27_0_0.adoc @@ -338,6 +338,38 @@ The new *OneTimeUse maximum age* setting of the SAML identity provider defaults Assertions with a `OneTimeUse` condition that are older than this are rejected. Increase the setting if users need more time to complete a login with such an assertion. +=== `secure-redirect-uris-enforcer` now always validates post-logout redirect URIs +// https://github.com/keycloak/keycloak/issues/50982 + +The `secure-redirect-uris-enforcer` client policy executor previously skipped post-logout redirect URI validation when both standard flow and implicit flow were disabled on a client. +This allowed a delegated client manager to retain an HTTP (insecure) post-logout redirect URI by temporarily disabling both flows, then re-enabling them via a partial update that omitted the `post.logout.redirect.uris` attribute. + +Post-logout redirect URIs are now always validated on every CREATE and UPDATE event, regardless of whether redirect-based flows are enabled. + +Additionally, a partial update that omits the `post.logout.redirect.uris` attribute now causes the executor to re-validate the URIs already stored in the model, rather than silently accepting the update. + +*Impact on existing clients* + +The following clients may be rejected by CREATE or UPDATE operations after upgrading: + +* Clients with both standard flow and implicit flow disabled that have redirect URIs + that violate the executor's configured rules and either no explicit + `post.logout.redirect.uris` attribute or the `{plus}` placeholder. + The absent or `{plus}` attribute inherits the client's redirect URIs, which are + now validated as post-logout redirect URIs. + +* Clients that have an explicit `post.logout.redirect.uris` value stored in their + attributes that violates the executor's configured rules — whether set before + the policy was active or retained through the partial-update bypass this fix + addresses. Any partial admin update to such a client, including routine changes + like renaming, now triggers revalidation of the stored post-logout URIs. + +Before upgrading, audit clients matched by the `secure-redirect-uris-enforcer` +policy against all of its configured URI rules. To disable post-logout redirects +on an affected client, set `post.logout.redirect.uris` to `-`; otherwise, update +its post-logout redirect URIs to satisfy those rules. Setting `-` also works +after upgrading, so an affected client can still be remediated. + === Scope mapping listings filter roles by visibility // https://github.com/keycloak/keycloak/issues/53480 diff --git a/services/src/main/java/org/keycloak/services/clientpolicy/executor/SecureRedirectUrisEnforcerExecutor.java b/services/src/main/java/org/keycloak/services/clientpolicy/executor/SecureRedirectUrisEnforcerExecutor.java index 185a18b2bd66..4e84a104f78c 100644 --- a/services/src/main/java/org/keycloak/services/clientpolicy/executor/SecureRedirectUrisEnforcerExecutor.java +++ b/services/src/main/java/org/keycloak/services/clientpolicy/executor/SecureRedirectUrisEnforcerExecutor.java @@ -23,9 +23,11 @@ import java.net.URI; import java.net.URISyntaxException; import java.net.UnknownHostException; +import java.util.ArrayList; import java.util.Collections; import java.util.HashSet; import java.util.List; +import java.util.Map; import java.util.function.Predicate; import org.keycloak.OAuth2Constants; @@ -33,6 +35,7 @@ import org.keycloak.models.ClientModel; import org.keycloak.models.KeycloakSession; import org.keycloak.protocol.oidc.OIDCAdvancedConfigWrapper; +import org.keycloak.protocol.oidc.OIDCConfigAttributes; import org.keycloak.protocol.oidc.utils.RedirectUtils; import org.keycloak.representations.idm.ClientPolicyExecutorConfigurationRepresentation; import org.keycloak.representations.idm.ClientRepresentation; @@ -215,15 +218,16 @@ public void executeOnEvent(ClientPolicyContext context) throws ClientPolicyExcep } private void verifyRedirectUris(ClientCRUDContext context) throws ClientPolicyException { - ClientRepresentation client = context.getProposedClientRepresentation(); - if (isAuthFlowWithRedirectEnabled(client)) { - List redirectUris = client.getRedirectUris(); + ClientRepresentation proposed = context.getProposedClientRepresentation(); + if (isAuthFlowWithRedirectEnabled(proposed)) { + List redirectUris = proposed.getRedirectUris(); if (redirectUris == null || redirectUris.isEmpty()) { throw invalidRedirectUri(ERR_GENERAL); } - verifyRedirectUris(client.getRootUrl(), redirectUris); - verifyPostLogoutRedirectUriUpdate(client); + verifyRedirectUris(proposed.getRootUrl(), redirectUris); } + // Always validate post-logout URIs — flows being disabled is irrelevant + verifyPostLogoutRedirectUriUpdate(proposed, context.getTargetClient()); } private static boolean isAuthFlowWithRedirectEnabled(ClientModel client) { @@ -234,13 +238,47 @@ private static boolean isAuthFlowWithRedirectEnabled(ClientRepresentation client return (client.isStandardFlowEnabled() == null || client.isStandardFlowEnabled() == Boolean.TRUE) || client.isImplicitFlowEnabled() == Boolean.TRUE; } - private void verifyPostLogoutRedirectUriUpdate(ClientRepresentation client) throws ClientPolicyException { - List postLogoutRedirectUris = OIDCAdvancedConfigWrapper.fromClientRepresentation(client).getPostLogoutRedirectUris(); - if (postLogoutRedirectUris == null || postLogoutRedirectUris.isEmpty()) { + private void verifyPostLogoutRedirectUriUpdate(ClientRepresentation client, ClientModel stored) throws ClientPolicyException { + Map attrs = client.getAttributes(); + + boolean proposedPostLogoutAttribute = attrs != null && attrs.containsKey(OIDCConfigAttributes.POST_LOGOUT_REDIRECT_URIS); + String postLogoutAttribute; + if (proposedPostLogoutAttribute) { + // The proposed representation explicitly sets post-logout URIs — validate them directly. + postLogoutAttribute = attrs.get(OIDCConfigAttributes.POST_LOGOUT_REDIRECT_URIS); + } else if (stored != null) { + // The proposed representation omits the attribute — validate whatever is already stored. + postLogoutAttribute = stored.getAttribute(OIDCConfigAttributes.POST_LOGOUT_REDIRECT_URIS); + } else { + postLogoutAttribute = null; + } + + // an unset attribute and the "+" placeholder both mean "same as the redirect uris", which are the + // ones proposed by this update, falling back to the stored ones when the update does not change them + List redirectUris = client.getRedirectUris(); + if (redirectUris == null) { + redirectUris = (stored != null) ? new ArrayList<>(stored.getRedirectUris()) : Collections.emptyList(); + } + + ClientRepresentation effective = new ClientRepresentation(); + effective.setRedirectUris(redirectUris); + if (postLogoutAttribute != null && !postLogoutAttribute.isBlank()) { + effective.setAttributes(Collections.singletonMap(OIDCConfigAttributes.POST_LOGOUT_REDIRECT_URIS, postLogoutAttribute)); + } + List postLogoutRedirectUris = OIDCAdvancedConfigWrapper.fromClientRepresentation(effective).getPostLogoutRedirectUris(); + + if (postLogoutRedirectUris == null) { + return; + } + + // blank uris are dropped before the client is persisted, so there is nothing to validate for them + postLogoutRedirectUris = postLogoutRedirectUris.stream().filter(uri -> uri != null && !uri.isBlank()).toList(); + if (postLogoutRedirectUris.isEmpty()) { return; } + String rootUrl = (client.getRootUrl() != null || stored == null) ? client.getRootUrl() : stored.getRootUrl(); logger.tracef("Verifying post-logout redirect uris. Target client: %s, Effective post-logout uris: %s", client.getClientId(), postLogoutRedirectUris); - verifyRedirectUris(client.getRootUrl(), postLogoutRedirectUris); + verifyRedirectUris(rootUrl, postLogoutRedirectUris); } void verifyRedirectUris(String rootUri, List redirectUris) throws ClientPolicyException { diff --git a/tests/base/src/test/java/org/keycloak/tests/client/policies/AbstractClientPoliciesTest.java b/tests/base/src/test/java/org/keycloak/tests/client/policies/AbstractClientPoliciesTest.java index 70f84015c720..097c039a730b 100644 --- a/tests/base/src/test/java/org/keycloak/tests/client/policies/AbstractClientPoliciesTest.java +++ b/tests/base/src/test/java/org/keycloak/tests/client/policies/AbstractClientPoliciesTest.java @@ -104,6 +104,14 @@ protected void updateClientByAdmin(ManagedRealm realm, String cId, Consumer + createClientByAdmin(realm, generateSuffixedName("client-a"), OIDCLoginProtocol.LOGIN_PROTOCOL, rep -> { + rep.setStandardFlowEnabled(Boolean.FALSE); + rep.setImplicitFlowEnabled(Boolean.FALSE); + rep.setRedirectUris(Collections.singletonList(HTTPS_CALLBACK)); + rep.getAttributes().put(OIDCConfigAttributes.POST_LOGOUT_REDIRECT_URIS, HTTP_ATTACKER_URI); + }) + ); + assertEquals("invalid_request", cpe.getMessage()); + } + + @Test + public void testUpdateExplicitHttpPostLogoutUriWithFlowsDisabledIsRejected() throws Exception { + setupSecureRedirectPolicy(); + + String cId = createClientByAdmin(realm, generateSuffixedName("client-b"), OIDCLoginProtocol.LOGIN_PROTOCOL, rep -> { + rep.setStandardFlowEnabled(Boolean.TRUE); + rep.setRedirectUris(Collections.singletonList(HTTPS_CALLBACK)); + rep.getAttributes().put(OIDCConfigAttributes.POST_LOGOUT_REDIRECT_URIS, HTTPS_SAFE_URI); + }); + + ClientPolicyException cpe = assertThrows(ClientPolicyException.class, () -> + updateClientByAdmin(realm, cId, rep -> { + rep.setStandardFlowEnabled(Boolean.FALSE); + rep.setImplicitFlowEnabled(Boolean.FALSE); + rep.getAttributes().put(OIDCConfigAttributes.POST_LOGOUT_REDIRECT_URIS, HTTP_ATTACKER_URI); + }) + ); + assertEquals("invalid_request", cpe.getMessage()); + } + + @Test + public void testPartialUpdateSettingHttpPostLogoutUriWithFlowsDisabledIsRejected() throws Exception { + setupSecureRedirectPolicy(); + + String cId = createClientByAdmin(realm, generateSuffixedName("client-c"), OIDCLoginProtocol.LOGIN_PROTOCOL, rep -> { + rep.setStandardFlowEnabled(Boolean.FALSE); + rep.setImplicitFlowEnabled(Boolean.FALSE); + rep.setRedirectUris(Collections.singletonList(HTTPS_CALLBACK)); + }); + + ClientPolicyException cpe = assertThrows(ClientPolicyException.class, () -> + updateClientByAdmin(realm, cId, rep -> + rep.getAttributes().put(OIDCConfigAttributes.POST_LOGOUT_REDIRECT_URIS, HTTP_ATTACKER_URI) + ) + ); + assertEquals("invalid_request", cpe.getMessage()); + } + + @Test + public void testPartialUpdateSettingHttpsPostLogoutUriWithFlowsDisabledIsAllowed() throws Exception { + setupSecureRedirectPolicy(); + + String cId = createClientByAdmin(realm, generateSuffixedName("client-c-pos"), OIDCLoginProtocol.LOGIN_PROTOCOL, rep -> { + rep.setStandardFlowEnabled(Boolean.FALSE); + rep.setImplicitFlowEnabled(Boolean.FALSE); + rep.setRedirectUris(Collections.singletonList(HTTPS_CALLBACK)); + }); + + updateClientByAdmin(realm, cId, rep -> + rep.getAttributes().put(OIDCConfigAttributes.POST_LOGOUT_REDIRECT_URIS, HTTPS_SAFE_URI) + ); + } + + @Test + public void testLegitimateHttpsPostLogoutUriNotBroken() throws Exception { + setupSecureRedirectPolicy(); + + String cId = createClientByAdmin(realm, generateSuffixedName("client-d"), OIDCLoginProtocol.LOGIN_PROTOCOL, rep -> { + rep.setStandardFlowEnabled(Boolean.TRUE); + rep.setRedirectUris(Collections.singletonList(HTTPS_CALLBACK)); + rep.getAttributes().put(OIDCConfigAttributes.POST_LOGOUT_REDIRECT_URIS, HTTPS_SAFE_URI); + }); + + // A full-representation update that does not touch the post-logout uris must not be rejected by the always-on revalidation. + updateClientByAdmin(realm, cId, rep -> rep.setName("updated-name")); + } + + // An absolute stored post-logout URI stays valid across ordinary updates. + @Test + public void testUpdateDoesNotDisturbValidAbsolutePostLogoutUri() throws Exception { + setupSecureRedirectPolicy(); + + String cId = createClientByAdmin(realm, generateSuffixedName("client-root-url"), OIDCLoginProtocol.LOGIN_PROTOCOL, rep -> { + rep.setStandardFlowEnabled(Boolean.TRUE); + rep.setRootUrl("https://app.example.com"); + rep.setRedirectUris(Collections.singletonList(HTTPS_CALLBACK)); + rep.getAttributes().put(OIDCConfigAttributes.POST_LOGOUT_REDIRECT_URIS, HTTPS_SAFE_URI); + }); + + // Updating basic fields shouldn't affect post-logout URIs + updateClientByAdmin(realm, cId, rep -> rep.setName("updated-root-url-client")); + updateClientByAdmin(realm, cId, rep -> rep.getAttributes().put(OIDCConfigAttributes.POST_LOGOUT_REDIRECT_URIS, "")); + } + + @Test + public void testPartialUpdateResolvesRelativeUriAgainstStoredInsecureRootUrl() throws Exception { + // Seed a relative post-logout uri under an insecure root before the policy is in place. + String cId = createClientByAdmin(realm, generateSuffixedName("client-stored-root"), OIDCLoginProtocol.LOGIN_PROTOCOL, rep -> { + rep.setStandardFlowEnabled(Boolean.TRUE); + rep.setRootUrl(HTTP_INSECURE_ROOT); + rep.setRedirectUris(Collections.singletonList(HTTPS_CALLBACK)); + rep.getAttributes().put(OIDCConfigAttributes.POST_LOGOUT_REDIRECT_URIS, RELATIVE_POST_LOGOUT); + }); + + setupSecureRedirectPolicy(); + + // Neither rootUrl nor attributes are supplied, so both come from the stored client. + ClientRepresentation partial = new ClientRepresentation(); + partial.setProtocol(OIDCLoginProtocol.LOGIN_PROTOCOL); + partial.setStandardFlowEnabled(Boolean.TRUE); + partial.setRedirectUris(List.of(HTTPS_CALLBACK)); + + ClientPolicyException ex = assertThrows(ClientPolicyException.class, () -> + updateClientByAdminPartial(realm, cId, partial) + ); + assertEquals(OAuthErrorException.INVALID_REQUEST, ex.getError()); + + assertEquals(SecureRedirectUrisEnforcerExecutor.ERR_NORMALURI, ex.getErrorDetail()); + } + + /** A rootUrl supplied by the update takes precedence over the stored one. */ + @Test + public void testPartialUpdateProposedRootUrlOverridesStoredInsecureRootUrl() throws Exception { + String cId = createClientByAdmin(realm, generateSuffixedName("client-new-root"), OIDCLoginProtocol.LOGIN_PROTOCOL, rep -> { + rep.setStandardFlowEnabled(Boolean.TRUE); + rep.setRootUrl(HTTP_INSECURE_ROOT); + rep.setRedirectUris(Collections.singletonList(HTTPS_CALLBACK)); + rep.getAttributes().put(OIDCConfigAttributes.POST_LOGOUT_REDIRECT_URIS, RELATIVE_POST_LOGOUT); + }); + + setupSecureRedirectPolicy(); + + // The update moves the client to a secure root, so the same relative uri is now safe. + ClientRepresentation partial = new ClientRepresentation(); + partial.setProtocol(OIDCLoginProtocol.LOGIN_PROTOCOL); + partial.setStandardFlowEnabled(Boolean.TRUE); + partial.setRootUrl(HTTPS_SECURE_ROOT); + partial.setRedirectUris(List.of(HTTPS_CALLBACK)); + + updateClientByAdminPartial(realm, cId, partial); + } + + @Test + public void testCreateWithNoPostLogoutUriSucceeds() throws Exception { + setupSecureRedirectPolicy(); + + createClientByAdmin(realm, generateSuffixedName("client-d2"), OIDCLoginProtocol.LOGIN_PROTOCOL, rep -> { + rep.setStandardFlowEnabled(Boolean.TRUE); + rep.setRedirectUris(Collections.singletonList(HTTPS_CALLBACK)); + rep.getAttributes().remove(OIDCConfigAttributes.POST_LOGOUT_REDIRECT_URIS); + }); + } + + /** + * Two-step partial-update bypass (issue #50982). + * + * Step 1: seed an HTTP post-logout URI into the model before the policy is active. + * Step 2: send a truly partial update — no attributes map, but safe redirectUris and + * standard flow re-enabled. Without the fix, OIDCAdvancedConfigWrapper falls + * back to clientRep.getRedirectUris() and returns the safe value, masking the + * stored HTTP URI. With the fix the executor reads the stored model and rejects. + */ + @Test + public void testPartialUpdateOmittingAttributesMasksStoredHttpPostLogoutUri() throws Exception { + // Seed a dangerous URI before the policy is in place. + String cId = createClientByAdmin(realm, generateSuffixedName("client-bypass"), OIDCLoginProtocol.LOGIN_PROTOCOL, rep -> { + rep.setStandardFlowEnabled(Boolean.FALSE); + rep.setImplicitFlowEnabled(Boolean.FALSE); + rep.setRedirectUris(Collections.singletonList(HTTPS_CALLBACK)); + rep.getAttributes().put(OIDCConfigAttributes.POST_LOGOUT_REDIRECT_URIS, HTTP_ATTACKER_URI); + }); + + // Now activate the policy. + setupSecureRedirectPolicy(); + + // Partial update: no attributes map at all (post.logout.redirect.uris absent), + // safe redirectUris provided, standard flow re-enabled. + ClientRepresentation partial = new ClientRepresentation(); + partial.setProtocol(OIDCLoginProtocol.LOGIN_PROTOCOL); + partial.setStandardFlowEnabled(Boolean.TRUE); + partial.setImplicitFlowEnabled(Boolean.FALSE); + partial.setRedirectUris(List.of(HTTPS_CALLBACK)); + // attributes intentionally null — executor must fall back to stored HTTP post-logout URI + + ClientPolicyException ex = assertThrows(ClientPolicyException.class, () -> + updateClientByAdminPartial(realm, cId, partial) + ); + assertEquals(OAuthErrorException.INVALID_REQUEST, ex.getError()); + } + + /** "+" with an HTTP redirect URI must be rejected even when flows are disabled. */ + @Test + public void testPlusPlaceholderExpandingToHttpUriIsRejected() throws Exception { + setupSecureRedirectPolicy(); + + ClientPolicyException ex = assertThrows(ClientPolicyException.class, () -> + createClientByAdmin(realm, generateSuffixedName("client-plus-http"), OIDCLoginProtocol.LOGIN_PROTOCOL, rep -> { + rep.setStandardFlowEnabled(Boolean.FALSE); + rep.setImplicitFlowEnabled(Boolean.FALSE); + rep.setRedirectUris(Collections.singletonList("http://oauth.redirect/some")); + OIDCAdvancedConfigWrapper.fromClientRepresentation(rep).setPostLogoutRedirectUris(List.of("+")); + }) + ); + assertEquals(OAuthErrorException.INVALID_REQUEST, ex.getError()); + } + + /** "+" with an HTTPS redirect URI is allowed even when flows are disabled. */ + @Test + public void testPlusPlaceholderExpandingToHttpsUriIsAllowed() throws Exception { + setupSecureRedirectPolicy(); + + createClientByAdmin(realm, generateSuffixedName("client-plus-https"), OIDCLoginProtocol.LOGIN_PROTOCOL, rep -> { + rep.setStandardFlowEnabled(Boolean.FALSE); + rep.setImplicitFlowEnabled(Boolean.FALSE); + rep.setRedirectUris(Collections.singletonList(HTTPS_CALLBACK)); + OIDCAdvancedConfigWrapper.fromClientRepresentation(rep).setPostLogoutRedirectUris(List.of("+")); + }); + } + + @Test + public void testCreateWithPlusPlaceholderAndNoRedirectUrisSucceeds() throws Exception { + setupSecureRedirectPolicy(); + + // "+" with no redirect uris to inherit expands to nothing — there is nothing to validate, and this must not blow up on a null list + createClientByAdmin(realm, generateSuffixedName("client-plus-empty"), OIDCLoginProtocol.LOGIN_PROTOCOL, rep -> { + rep.setStandardFlowEnabled(Boolean.FALSE); + rep.setImplicitFlowEnabled(Boolean.FALSE); + rep.setRedirectUris(null); + rep.getAttributes().put(OIDCConfigAttributes.POST_LOGOUT_REDIRECT_URIS, "+"); + }); + } + + @Test + public void testBlankPostLogoutAttributeInheritingHttpUriIsRejected() throws Exception { + setupSecureRedirectPolicy(); + + ClientPolicyException ex = assertThrows(ClientPolicyException.class, () -> + createClientByAdmin(realm, generateSuffixedName("client-blank-http"), OIDCLoginProtocol.LOGIN_PROTOCOL, rep -> { + rep.setStandardFlowEnabled(Boolean.FALSE); + rep.setImplicitFlowEnabled(Boolean.FALSE); + rep.setRedirectUris(Collections.singletonList("http://oauth.redirect/some")); + rep.getAttributes().put(OIDCConfigAttributes.POST_LOGOUT_REDIRECT_URIS, ""); + }) + ); + assertEquals(OAuthErrorException.INVALID_REQUEST, ex.getError()); + } + + private void setupSecureRedirectPolicy() throws Exception { + setupPolicy( + realm, + SecureRedirectUrisEnforcerExecutorFactory.PROVIDER_ID, + createSecureRedirectUrisEnforcerExecutorConfig(cfg -> {}), + AnyClientConditionFactory.PROVIDER_ID, + createAnyClientConditionConfig() + ); + } +} diff --git a/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/client/policies/SecureRedirectUrisEnforcerExecutorTest.java b/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/client/policies/SecureRedirectUrisEnforcerExecutorTest.java index 186bc2ee7d32..230380a2f66d 100644 --- a/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/client/policies/SecureRedirectUrisEnforcerExecutorTest.java +++ b/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/client/policies/SecureRedirectUrisEnforcerExecutorTest.java @@ -29,6 +29,7 @@ import org.keycloak.events.Details; import org.keycloak.events.Errors; import org.keycloak.events.EventType; +import org.keycloak.protocol.oidc.OIDCAdvancedConfigWrapper; import org.keycloak.protocol.oidc.OIDCLoginProtocol; import org.keycloak.representations.idm.ClientRepresentation; import org.keycloak.representations.idm.RealmRepresentation; @@ -83,7 +84,9 @@ public void testNotRedirectBasedFlowClient_normalUri() throws Exception { ).toString(); updatePolicies(json); - // The executor's check logic is not executed to an auth code flow or implicit flow disabled client. + // The executor's redirect uri check is not executed to an auth code flow or implicit flow disabled client. + // The post-logout redirect uri check is executed regardless of the enabled flows, so the client below + // switches the post-logout redirect uris off ("-") to keep its redirect uris out of that check. // Registration // Success - even if not setting a valid redirect uri @@ -94,6 +97,7 @@ public void testNotRedirectBasedFlowClient_normalUri() throws Exception { cId = createClientByAdmin(clientId, (ClientRepresentation clientRep) -> { clientRep.setSecret("secret"); clientRep.setRedirectUris(List.of("http://oauth.redirect/some")); // normally, a redirect url with http scheme is not allowed. + OIDCAdvancedConfigWrapper.fromClientRepresentation(clientRep).setPostLogoutRedirectUris(List.of("-")); clientRep.setStandardFlowEnabled(false); clientRep.setImplicitFlowEnabled(false); clientRep.setServiceAccountsEnabled(true); @@ -104,6 +108,23 @@ public void testNotRedirectBasedFlowClient_normalUri() throws Exception { fail(); } + // Registration + // Fail - the post-logout redirect uri is checked even though both redirect based flows are disabled + try { + createClientByAdmin(generateSuffixedName(CLIENT_NAME), (ClientRepresentation clientRep) -> { + clientRep.setSecret("secret"); + clientRep.setRedirectUris(List.of("https://oauth.redirect/some")); + OIDCAdvancedConfigWrapper.fromClientRepresentation(clientRep) + .setPostLogoutRedirectUris(List.of("http://oauth.redirect/post-logout")); + clientRep.setStandardFlowEnabled(false); + clientRep.setImplicitFlowEnabled(false); + clientRep.setServiceAccountsEnabled(true); + }); + fail("Expected to fail with an http post-logout redirect uri"); + } catch (ClientPolicyException cpe) { + assertEquals(OAuthErrorException.INVALID_REQUEST, cpe.getError()); + } + // Update // Success - even if not setting a valid redirect uri try { @@ -116,6 +137,7 @@ public void testNotRedirectBasedFlowClient_normalUri() throws Exception { } catch (ClientPolicyException cpe) { fail(); } + } @Test From ea78b1644acea15adad42efaed4591421ed214ae Mon Sep 17 00:00:00 2001 From: Nayyar Date: Fri, 6 Nov 2026 19:19:37 +0530 Subject: [PATCH 06/16] Validate ECDSA signature length before JWS verification (#53928) Closes #53927 Signed-off-by: Nayyar --- .../org/keycloak/crypto/ECDSAAlgorithm.java | 6 + .../crypto/ECDSASignatureVerifierContext.java | 4 + .../ECDSASignatureVerifierContextTest.java | 109 ++++++++++++++++++ ...yptoECDSASignatureVerifierContextTest.java | 32 +++++ ...yptoECDSASignatureVerifierContextTest.java | 22 ++++ ...1402ECDSASignatureVerifierContextTest.java | 22 ++++ .../ClientECDSASignatureVerifierContext.java | 4 + .../ServerECDSASignatureVerifierContext.java | 4 + 8 files changed, 203 insertions(+) create mode 100644 core/src/test/java/org/keycloak/crypto/ECDSASignatureVerifierContextTest.java create mode 100644 crypto/default/src/test/java/org/keycloak/crypto/def/test/DefaultCryptoECDSASignatureVerifierContextTest.java create mode 100644 crypto/elytron/src/test/java/org/keycloak/crypto/elytron/test/ElytronCryptoECDSASignatureVerifierContextTest.java create mode 100644 crypto/fips1402/src/test/java/org/keycloak/crypto/fips/test/FIPS1402ECDSASignatureVerifierContextTest.java diff --git a/core/src/main/java/org/keycloak/crypto/ECDSAAlgorithm.java b/core/src/main/java/org/keycloak/crypto/ECDSAAlgorithm.java index 8596d4a36fa4..f8ae17b8d29e 100644 --- a/core/src/main/java/org/keycloak/crypto/ECDSAAlgorithm.java +++ b/core/src/main/java/org/keycloak/crypto/ECDSAAlgorithm.java @@ -44,6 +44,12 @@ public static int getSignatureLength(String alg) { } public static byte[] concatenatedRSToASN1DER(final byte[] signature, int signLength) throws IOException { + // RFC 7518 section 3.4 fixes the JOSE ECDSA signature to exactly signLength octets. The provider + // implementations read only the first signLength bytes, so without this check a signature with + // trailing data appended would still convert and verify, making accepted signatures malleable. + if (signature.length != signLength) { + throw new IOException("Invalid ECDSA signature length: expected " + signLength + ", got " + signature.length); + } return CryptoIntegration.getProvider().getEcdsaCryptoProvider().concatenatedRSToASN1DER(signature, signLength); } diff --git a/core/src/main/java/org/keycloak/crypto/ECDSASignatureVerifierContext.java b/core/src/main/java/org/keycloak/crypto/ECDSASignatureVerifierContext.java index 177aaeb2d9a8..db353f9766d4 100644 --- a/core/src/main/java/org/keycloak/crypto/ECDSASignatureVerifierContext.java +++ b/core/src/main/java/org/keycloak/crypto/ECDSASignatureVerifierContext.java @@ -27,6 +27,10 @@ public ECDSASignatureVerifierContext(KeyWrapper key) { public boolean verify(byte[] data, byte[] signature) throws VerificationException { try { int expectedSize = ECDSAAlgorithm.getSignatureLength(getAlgorithm()); + if (signature.length != expectedSize) { + // RFC 7518 section 3.4: a wrong-length R || S is an invalid signature, not a verification error + return false; + } byte[] derSignature = ECDSAAlgorithm.concatenatedRSToASN1DER(signature, expectedSize); return super.verify(data, derSignature); } catch (Exception e) { diff --git a/core/src/test/java/org/keycloak/crypto/ECDSASignatureVerifierContextTest.java b/core/src/test/java/org/keycloak/crypto/ECDSASignatureVerifierContextTest.java new file mode 100644 index 000000000000..2fb64fa0d80a --- /dev/null +++ b/core/src/test/java/org/keycloak/crypto/ECDSASignatureVerifierContextTest.java @@ -0,0 +1,109 @@ +/* + * Copyright 2026 Red Hat, Inc. and/or its affiliates + * and other contributors as indicated by the @author tags. + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package org.keycloak.crypto; + +import java.nio.charset.StandardCharsets; +import java.security.KeyPair; +import java.security.KeyPairGenerator; +import java.security.spec.ECGenParameterSpec; +import java.util.Arrays; +import java.util.LinkedHashMap; +import java.util.Map; + +import org.keycloak.common.crypto.CryptoIntegration; +import org.keycloak.rule.CryptoInitRule; + +import org.junit.ClassRule; +import org.junit.Test; + +import static org.junit.Assert.assertEquals; +import static org.junit.Assert.assertFalse; +import static org.junit.Assert.assertTrue; + +public abstract class ECDSASignatureVerifierContextTest { + + @ClassRule + public static CryptoInitRule cryptoInitRule = new CryptoInitRule(); + + private static final Map CURVE_BY_ALGORITHM = new LinkedHashMap<>(); + + static { + CURVE_BY_ALGORITHM.put(Algorithm.ES256, "secp256r1"); + CURVE_BY_ALGORITHM.put(Algorithm.ES384, "secp384r1"); + CURVE_BY_ALGORITHM.put(Algorithm.ES512, "secp521r1"); + } + + @Test + public void verifiesExactLengthSignature() throws Exception { + for (Map.Entry entry : CURVE_BY_ALGORITHM.entrySet()) { + String algorithm = entry.getKey(); + KeyWrapper key = generateKey(algorithm, entry.getValue()); + byte[] data = "sample-signature-input".getBytes(StandardCharsets.UTF_8); + + byte[] signature = new ECDSASignatureSignerContext(key).sign(data); + + assertEquals(algorithm, ECDSAAlgorithm.getSignatureLength(algorithm), signature.length); + assertTrue(algorithm, new ECDSASignatureVerifierContext(key).verify(data, signature)); + } + } + + @Test + public void rejectsSignatureWithTrailingBytes() throws Exception { + for (Map.Entry entry : CURVE_BY_ALGORITHM.entrySet()) { + String algorithm = entry.getKey(); + KeyWrapper key = generateKey(algorithm, entry.getValue()); + byte[] data = "sample-signature-input".getBytes(StandardCharsets.UTF_8); + byte[] signature = new ECDSASignatureSignerContext(key).sign(data); + + // RFC 7518 section 3.4 fixes the JWS signature to exactly 2 * curve-size octets. A valid + // R || S with data appended must not verify, otherwise accepted signatures are malleable. + byte[] oneTrailingByte = Arrays.copyOf(signature, signature.length + 1); + assertFalse(algorithm, new ECDSASignatureVerifierContext(key).verify(data, oneTrailingByte)); + + byte[] trailingJunk = Arrays.copyOf(signature, signature.length + 17); + Arrays.fill(trailingJunk, signature.length, trailingJunk.length, (byte) 0x5a); + assertFalse(algorithm, new ECDSASignatureVerifierContext(key).verify(data, trailingJunk)); + } + } + + @Test + public void rejectsTruncatedSignature() throws Exception { + for (Map.Entry entry : CURVE_BY_ALGORITHM.entrySet()) { + String algorithm = entry.getKey(); + KeyWrapper key = generateKey(algorithm, entry.getValue()); + byte[] data = "sample-signature-input".getBytes(StandardCharsets.UTF_8); + byte[] signature = new ECDSASignatureSignerContext(key).sign(data); + + assertFalse(algorithm, new ECDSASignatureVerifierContext(key).verify(data, Arrays.copyOf(signature, signature.length - 1))); + } + } + + private KeyWrapper generateKey(String algorithm, String curve) throws Exception { + KeyPairGenerator kpg = CryptoIntegration.getProvider().getKeyPairGen(KeyType.EC); + kpg.initialize(new ECGenParameterSpec(curve)); + KeyPair keyPair = kpg.generateKeyPair(); + + KeyWrapper key = new KeyWrapper(); + key.setKid("test-kid"); + key.setAlgorithm(algorithm); + key.setType(KeyType.EC); + key.setUse(KeyUse.SIG); + key.setPrivateKey(keyPair.getPrivate()); + key.setPublicKey(keyPair.getPublic()); + return key; + } +} diff --git a/crypto/default/src/test/java/org/keycloak/crypto/def/test/DefaultCryptoECDSASignatureVerifierContextTest.java b/crypto/default/src/test/java/org/keycloak/crypto/def/test/DefaultCryptoECDSASignatureVerifierContextTest.java new file mode 100644 index 000000000000..83c1c547fe9a --- /dev/null +++ b/crypto/default/src/test/java/org/keycloak/crypto/def/test/DefaultCryptoECDSASignatureVerifierContextTest.java @@ -0,0 +1,32 @@ +/* + * Copyright 2026 Red Hat, Inc. and/or its affiliates + * and other contributors as indicated by the @author tags. + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package org.keycloak.crypto.def.test; + +import org.keycloak.common.util.Environment; +import org.keycloak.crypto.ECDSASignatureVerifierContextTest; + +import org.junit.Assume; +import org.junit.Before; + +public class DefaultCryptoECDSASignatureVerifierContextTest extends ECDSASignatureVerifierContextTest { + + @Before + public void before() { + // Run this test just if java is not in FIPS mode + Assume.assumeFalse("Java is in FIPS mode. Skipping the test.", Environment.isJavaInFipsMode()); + } +} diff --git a/crypto/elytron/src/test/java/org/keycloak/crypto/elytron/test/ElytronCryptoECDSASignatureVerifierContextTest.java b/crypto/elytron/src/test/java/org/keycloak/crypto/elytron/test/ElytronCryptoECDSASignatureVerifierContextTest.java new file mode 100644 index 000000000000..ed655b618728 --- /dev/null +++ b/crypto/elytron/src/test/java/org/keycloak/crypto/elytron/test/ElytronCryptoECDSASignatureVerifierContextTest.java @@ -0,0 +1,22 @@ +/* + * Copyright 2026 Red Hat, Inc. and/or its affiliates + * and other contributors as indicated by the @author tags. + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package org.keycloak.crypto.elytron.test; + +import org.keycloak.crypto.ECDSASignatureVerifierContextTest; + +public class ElytronCryptoECDSASignatureVerifierContextTest extends ECDSASignatureVerifierContextTest { +} diff --git a/crypto/fips1402/src/test/java/org/keycloak/crypto/fips/test/FIPS1402ECDSASignatureVerifierContextTest.java b/crypto/fips1402/src/test/java/org/keycloak/crypto/fips/test/FIPS1402ECDSASignatureVerifierContextTest.java new file mode 100644 index 000000000000..0035616da9c4 --- /dev/null +++ b/crypto/fips1402/src/test/java/org/keycloak/crypto/fips/test/FIPS1402ECDSASignatureVerifierContextTest.java @@ -0,0 +1,22 @@ +/* + * Copyright 2026 Red Hat, Inc. and/or its affiliates + * and other contributors as indicated by the @author tags. + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package org.keycloak.crypto.fips.test; + +import org.keycloak.crypto.ECDSASignatureVerifierContextTest; + +public class FIPS1402ECDSASignatureVerifierContextTest extends ECDSASignatureVerifierContextTest { +} diff --git a/services/src/main/java/org/keycloak/crypto/ClientECDSASignatureVerifierContext.java b/services/src/main/java/org/keycloak/crypto/ClientECDSASignatureVerifierContext.java index ab7e8fda363f..0956c5f131cf 100644 --- a/services/src/main/java/org/keycloak/crypto/ClientECDSASignatureVerifierContext.java +++ b/services/src/main/java/org/keycloak/crypto/ClientECDSASignatureVerifierContext.java @@ -55,6 +55,10 @@ private static KeyWrapper getKey(KeycloakSession session, ClientModel client, JW public boolean verify(byte[] data, byte[] signature) throws VerificationException { try { int expectedSize = ECDSAAlgorithm.getSignatureLength(getAlgorithm()); + if (signature.length != expectedSize) { + // RFC 7518 section 3.4: a wrong-length R || S is an invalid signature, not a verification error + return false; + } byte[] derSignature = ECDSAAlgorithm.concatenatedRSToASN1DER(signature, expectedSize); return super.verify(data, derSignature); } catch (Exception e) { diff --git a/services/src/main/java/org/keycloak/crypto/ServerECDSASignatureVerifierContext.java b/services/src/main/java/org/keycloak/crypto/ServerECDSASignatureVerifierContext.java index bfedabaad5b2..fefddb29887d 100644 --- a/services/src/main/java/org/keycloak/crypto/ServerECDSASignatureVerifierContext.java +++ b/services/src/main/java/org/keycloak/crypto/ServerECDSASignatureVerifierContext.java @@ -16,6 +16,10 @@ public ServerECDSASignatureVerifierContext(KeyWrapper key) { public boolean verify(byte[] data, byte[] signature) throws VerificationException { try { int expectedSize = ECDSAAlgorithm.getSignatureLength(getAlgorithm()); + if (signature.length != expectedSize) { + // RFC 7518 section 3.4: a wrong-length R || S is an invalid signature, not a verification error + return false; + } byte[] derSignature = ECDSAAlgorithm.concatenatedRSToASN1DER(signature, expectedSize); return super.verify(data, derSignature); } catch (Exception e) { From 77a8f12dd6fc6ae4f41d9297b8938ed932feb792 Mon Sep 17 00:00:00 2001 From: Stefan Guilhen Date: Wed, 4 Nov 2026 15:47:21 -0300 Subject: [PATCH 07/16] Fix crash and inconsistent matching in account console linked-accounts search Closes #54647 Signed-off-by: Stefan Guilhen --- .../account/LinkedAccountsResource.java | 29 ++++++++++++++----- .../LinkedAccountsRestServiceTest.java | 28 ++++++++++++++++++ 2 files changed, 49 insertions(+), 8 deletions(-) diff --git a/services/src/main/java/org/keycloak/services/resources/account/LinkedAccountsResource.java b/services/src/main/java/org/keycloak/services/resources/account/LinkedAccountsResource.java index 483cdc4af5e7..604be4b90366 100644 --- a/services/src/main/java/org/keycloak/services/resources/account/LinkedAccountsResource.java +++ b/services/src/main/java/org/keycloak/services/resources/account/LinkedAccountsResource.java @@ -154,7 +154,7 @@ public Response linkedAccounts( Map searchOptions = Map.of( IdentityProviderModel.ENABLED, "true", IdentityProviderModel.ORGANIZATION_ID, "", - IdentityProviderModel.SEARCH, search == null ? "" : search, + IdentityProviderModel.SEARCH, resolveSearch(search), IdentityProviderModel.ALIAS_NOT_IN, fedAliasesToExclude, IdentityProviderModel.SHOW_IN_ACCOUNT_CONSOLE, IdentityProviderShowInAccountConsole.ALWAYS.name()); @@ -186,20 +186,33 @@ private LinkedAccountRepresentation toLinkedAccount(IdentityProviderModel provid return rep; } + private String resolveSearch(String search) { + if (search == null) { + return ""; + } + // a lone '"' would otherwise be treated by JpaIdentityProviderStorageProvider#getAliasSearchPredicate + // as a quoted/exact search with an empty name and throw a StringIndexOutOfBoundsException; appending + // '*' keeps the same "starts with a literal quote" semantics while avoiding that crash + return "\"".equals(search) ? search + "*" : search; + } + private boolean matchesLinkedProvider(final LinkedAccountRepresentation linkedAccount, final String search) { if (search == null) { return true; - }else if (search.startsWith("\"") && search.endsWith("\"")) { + } else if (search.length() > 1 && search.startsWith("\"") && search.endsWith("\"")) { final String name = search.substring(1, search.length() - 1); return linkedAccount.getProviderAlias().equals(name) || linkedAccount.getDisplayName().equals(name); - } else if (search.startsWith("*") && search.endsWith("*")) { - final String name = search.substring(1, search.length() - 1); - return linkedAccount.getProviderAlias().contains(name) || linkedAccount.getDisplayName().contains(name); + } else if (search.length() > 1 && search.startsWith("*") && search.endsWith("*")) { + // non-exact searches are case-insensitive, matching the LIKE lower(...) semantics used by + // JpaIdentityProviderStorageProvider#getAliasSearchPredicate for the "linked=false" branch + final String name = search.substring(1, search.length() - 1).toLowerCase(Locale.ROOT); + return linkedAccount.getProviderAlias().toLowerCase(Locale.ROOT).contains(name) || linkedAccount.getDisplayName().toLowerCase(Locale.ROOT).contains(name); } else if (search.endsWith("*")) { - final String name = search.substring(0, search.length() - 1); - return linkedAccount.getProviderAlias().startsWith(name) || linkedAccount.getDisplayName().startsWith(name); + final String name = search.substring(0, search.length() - 1).toLowerCase(Locale.ROOT); + return linkedAccount.getProviderAlias().toLowerCase(Locale.ROOT).startsWith(name) || linkedAccount.getDisplayName().toLowerCase(Locale.ROOT).startsWith(name); } else { - return linkedAccount.getProviderAlias().startsWith(search) || linkedAccount.getDisplayName().startsWith(search); + final String name = search.toLowerCase(Locale.ROOT); + return linkedAccount.getProviderAlias().toLowerCase(Locale.ROOT).startsWith(name) || linkedAccount.getDisplayName().toLowerCase(Locale.ROOT).startsWith(name); } } diff --git a/tests/base/src/test/java/org/keycloak/tests/account/LinkedAccountsRestServiceTest.java b/tests/base/src/test/java/org/keycloak/tests/account/LinkedAccountsRestServiceTest.java index 80bc9beda27b..24eff3c87b46 100644 --- a/tests/base/src/test/java/org/keycloak/tests/account/LinkedAccountsRestServiceTest.java +++ b/tests/base/src/test/java/org/keycloak/tests/account/LinkedAccountsRestServiceTest.java @@ -269,6 +269,34 @@ public void testGetLinkedAccountsWithPagination() throws IOException { assertEquals(1, accounts.size()); linkedAccountAliases = accounts.stream().map(LinkedAccountRepresentation::getProviderAlias).toList(); assertThat(linkedAccountAliases, contains("myoidc")); + + // non-exact searches are case-insensitive on both branches + accounts = linkedAccountsRep("linked=true&search=GIT*"); + assertEquals(2, accounts.size()); + linkedAccountAliases = accounts.stream().map(LinkedAccountRepresentation::getProviderAlias).toList(); + assertThat(linkedAccountAliases, contains("github", "gitlab")); + + accounts = linkedAccountsRep("linked=false&search=*O*"); + assertEquals(3, accounts.size()); + linkedAccountAliases = accounts.stream().map(LinkedAccountRepresentation::getProviderAlias).toList(); + assertThat(linkedAccountAliases, contains("facebook", "microsoft", "myoidc")); + } + + @Test + public void testGetLinkedAccountsWithEdgeCaseSearchValues() throws IOException { + // a lone '*' is the "contains everything" wildcard, equivalent to no search at all + List accounts = linkedAccountsRep("linked=true&search=*"); + assertEquals(3, accounts.size()); + + accounts = linkedAccountsRep("linked=false&search=*"); + assertEquals(5, accounts.size()); + + // a lone '"' must not crash the endpoint; it matches nothing on either branch + accounts = linkedAccountsRep("linked=true&search=%22"); + assertEquals(0, accounts.size()); + + accounts = linkedAccountsRep("linked=false&search=%22"); + assertEquals(0, accounts.size()); } @Test From b1182f9d6bff5fcc2a8b4ce6a81eead5b6f0d622 Mon Sep 17 00:00:00 2001 From: Lukas Hanusovsky Date: Fri, 6 Nov 2026 13:03:35 +0100 Subject: [PATCH 08/16] Reject online refresh tokens for sessions in logout states Check user and client session logout state before refreshing tokens, preserving pending logout state and preventing idle timestamp updates. Keep offline and service-account refresh behavior unchanged. Closes #54484 Signed-off-by: Lukas Hanusovsky --- .../server_admin/topics/sessions/offline.adoc | 2 + .../keycloak/protocol/oidc/TokenManager.java | 12 +++ .../tests/oauth/RefreshTokenTest.java | 73 +++++++++++++++++++ 3 files changed, 87 insertions(+) diff --git a/docs/documentation/server_admin/topics/sessions/offline.adoc b/docs/documentation/server_admin/topics/sessions/offline.adoc index ab47f7e24bb4..9dc7e52fe1d5 100644 --- a/docs/documentation/server_admin/topics/sessions/offline.adoc +++ b/docs/documentation/server_admin/topics/sessions/offline.adoc @@ -10,6 +10,8 @@ The client application is responsible for persisting the offline token in storag The difference between a refresh token and an offline token is that an offline token never expires and is not subject to the `SSO Session Idle` timeout and `SSO Session Max` lifespan. The offline token is valid after a user logout. You must use the offline token for a refresh token action at least once per thirty days or for the value of the <<_offline-session-idle, Offline Session Idle>>. +Online refresh tokens cannot be used once logout begins for the user session or the associated client session, even if a front-channel logout flow has not finished. Offline tokens are unaffected by this restriction. + If you enable <<_offline-session-max-limited, Offline Session Max Limited>>, offline tokens expire after 60 days even if you use the offline token for a refresh token action. You can change this value, <<_offline-session-max, Offline Session Max>>, in the Admin Console. When using offline access, client idle and max timeouts can be overridden at the <<_client_advanced_settings_oidc,client level>>. The options *Client Offline Session Idle* and *Client Offline Session Max*, in the client *Advanced Settings* tab, allow you to have a shorter offline timeouts for a specific application. Note that client session values also control the refresh token expiration but they never affect the global offline user SSO session. The option *Client Offline Session Max* is only evaluated in the client if <<_offline-session-max-limited, Offline Session Max Limited>> is *Enabled* at the realm level. diff --git a/services/src/main/java/org/keycloak/protocol/oidc/TokenManager.java b/services/src/main/java/org/keycloak/protocol/oidc/TokenManager.java index 30307001049f..687188826f7e 100755 --- a/services/src/main/java/org/keycloak/protocol/oidc/TokenManager.java +++ b/services/src/main/java/org/keycloak/protocol/oidc/TokenManager.java @@ -135,6 +135,7 @@ import org.keycloak.services.util.MtlsHoKTokenUtil; import org.keycloak.services.util.UserSessionUtil; import org.keycloak.sessions.AuthenticationSessionModel; +import org.keycloak.sessions.CommonClientSessionModel; import org.keycloak.tracing.TracingAttributes; import org.keycloak.tracing.TracingProvider; import org.keycloak.util.JWKSUtils; @@ -203,6 +204,12 @@ public TokenValidation validateToken(KeycloakSession session, UriInfo uriInfo, C } else { // Find userSession regularly for online tokens userSession = session.sessions().getUserSession(realm, oldToken.getSessionState()); + // Non-browser grants can leave the state unset; logout always sets an explicit state. + UserSessionModel.State state = userSession == null ? null : userSession.getState(); + // Reject separately from expiration cleanup so front-channel logout can still finish. + if (state != null && state != UserSessionModel.State.LOGGED_IN) { + throw new OAuthErrorException(OAuthErrorException.INVALID_GRANT, "Session not active", "Session not active"); + } if (!AuthenticationManager.isSessionValid(realm, userSession)) { if (userSession != null) { UserSessionModel onlineSession = userSession; @@ -240,6 +247,11 @@ public TokenValidation validateToken(KeycloakSession session, UriInfo uriInfo, C throw new OAuthErrorException(OAuthErrorException.INVALID_GRANT, "Session doesn't have required client", "Session doesn't have required client"); } + if (!offline && (CommonClientSessionModel.Action.LOGGING_OUT.name().equals(clientSession.getAction()) + || CommonClientSessionModel.Action.LOGGED_OUT.name().equals(clientSession.getAction()))) { + throw new OAuthErrorException(OAuthErrorException.INVALID_GRANT, "Client session not active"); + } + if (!AuthenticationManager.isClientSessionValid(realm, client, userSession, clientSession)) { logger.debug("Client session not active"); UserSessionModel currentSession = userSession; diff --git a/tests/base/src/test/java/org/keycloak/tests/oauth/RefreshTokenTest.java b/tests/base/src/test/java/org/keycloak/tests/oauth/RefreshTokenTest.java index 195c981288c3..ccf23a6a4afc 100755 --- a/tests/base/src/test/java/org/keycloak/tests/oauth/RefreshTokenTest.java +++ b/tests/base/src/test/java/org/keycloak/tests/oauth/RefreshTokenTest.java @@ -43,7 +43,9 @@ import org.keycloak.jose.jws.JWSInputException; import org.keycloak.jose.jws.crypto.HashUtils; import org.keycloak.models.AccountRoles; +import org.keycloak.models.AuthenticatedClientSessionModel; import org.keycloak.models.Constants; +import org.keycloak.models.UserSessionModel; import org.keycloak.models.UserSessionProvider; import org.keycloak.models.sessions.infinispan.InfinispanUserSessionProviderFactory; import org.keycloak.models.utils.KeycloakModelUtils; @@ -60,6 +62,7 @@ import org.keycloak.representations.idm.RealmRepresentation; import org.keycloak.representations.idm.RoleRepresentation; import org.keycloak.representations.idm.UserRepresentation; +import org.keycloak.sessions.CommonClientSessionModel; import org.keycloak.testframework.annotations.InjectAdminClient; import org.keycloak.testframework.annotations.InjectEvents; import org.keycloak.testframework.annotations.InjectRealm; @@ -101,6 +104,8 @@ import org.junit.jupiter.api.Assumptions; import org.junit.jupiter.api.BeforeEach; import org.junit.jupiter.api.Test; +import org.junit.jupiter.params.ParameterizedTest; +import org.junit.jupiter.params.provider.EnumSource; import org.openqa.selenium.Cookie; import static org.keycloak.connections.infinispan.InfinispanConnectionProvider.CLIENT_SESSION_CACHE_NAME; @@ -112,7 +117,10 @@ import static org.hamcrest.Matchers.allOf; import static org.hamcrest.Matchers.greaterThan; import static org.hamcrest.Matchers.greaterThanOrEqualTo; +import static org.hamcrest.Matchers.is; import static org.hamcrest.Matchers.lessThanOrEqualTo; +import static org.hamcrest.Matchers.notNullValue; +import static org.hamcrest.Matchers.nullValue; import static org.junit.jupiter.api.Assertions.assertEquals; import static org.junit.jupiter.api.Assertions.assertFalse; import static org.junit.jupiter.api.Assertions.assertNotEquals; @@ -185,6 +193,71 @@ public static void enableRefreshTokenEvents(ManagedRealm realm) { } } + @ParameterizedTest + @EnumSource(value = UserSessionModel.State.class, names = "LOGGED_IN", mode = EnumSource.Mode.EXCLUDE) + public void rejectRefreshAfterUserLogoutBegins(UserSessionModel.State state) { + assertRefreshRejected(state, null, "Session not active"); + } + + @ParameterizedTest + @EnumSource(value = CommonClientSessionModel.Action.class, names = {"LOGGING_OUT", "LOGGED_OUT"}) + public void rejectRefreshAfterClientLogoutBegins(CommonClientSessionModel.Action action) { + assertRefreshRejected(UserSessionModel.State.LOGGED_IN, action.name(), "Client session not active"); + } + + private void assertRefreshRejected(UserSessionModel.State state, String action, String errorDescription) { + AccessTokenResponse tokens = loginForRefreshToken(); + String sessionId = oauth.parseRefreshToken(tokens.getRefreshToken()).getSessionId(); + int[] timestamps = runOnServer.fetch(session -> { + UserSessionModel userSession = session.sessions().getUserSession(session.getContext().getRealm(), sessionId); + userSession.setState(state); + AuthenticatedClientSessionModel clientSession = userSession.getAuthenticatedClientSessions().values().iterator().next(); + clientSession.setAction(action); + return new int[] { userSession.getLastSessionRefresh(), clientSession.getTimestamp() }; + }, int[].class); + + timeOffSet.set(10); + AccessTokenResponse response = oauth.doRefreshTokenRequest(tokens.getRefreshToken()); + assertThat(response.getStatusCode(), is(400)); + assertThat(response.getError(), is(OAuthErrorException.INVALID_GRANT)); + assertThat(response.getErrorDescription(), is(errorDescription)); + assertThat(response.getAccessToken(), nullValue()); + assertThat(response.getRefreshToken(), nullValue()); + + // A rejected refresh must neither extend idle validity nor remove state needed to complete SLO. + runOnServer.run(session -> { + UserSessionModel userSession = session.sessions().getUserSession(session.getContext().getRealm(), sessionId); + assertThat(userSession, notNullValue()); + assertThat(userSession.getState(), is(state)); + assertThat(userSession.getLastSessionRefresh(), is(timestamps[0])); + AuthenticatedClientSessionModel clientSession = userSession.getAuthenticatedClientSessions().values().iterator().next(); + assertThat(clientSession.getAction(), is(action)); + assertThat(clientSession.getTimestamp(), is(timestamps[1])); + }); + } + + @Test + public void offlineRefreshAfterLogout() { + try { + oauth.scope(OAuth2Constants.OFFLINE_ACCESS); + AccessTokenResponse tokens = loginForRefreshToken(); + user.admin().logout(); + + AccessTokenResponse response = oauth.doRefreshTokenRequest(tokens.getRefreshToken()); + assertThat(response.getStatusCode(), is(200)); + assertThat(response.getAccessToken(), notNullValue()); + assertThat(response.getRefreshToken(), notNullValue()); + } finally { + oauth.scope(null); + } + } + + private AccessTokenResponse loginForRefreshToken() { + AccessTokenResponse response = oauth.doPasswordGrantRequest(user.getUsername(), user.getPassword()); + assertThat(response.getStatusCode(), is(200)); + return response; + } + /** * KEYCLOAK-547 * From 6ea53a29b8db6ae64a1191279a1ac977f71ccf89 Mon Sep 17 00:00:00 2001 From: Stefan Guilhen Date: Thu, 29 Oct 2026 13:10:15 -0300 Subject: [PATCH 09/16] Reject a different brokered user on forced re-authentication before creating it Closes #54344 Signed-off-by: Stefan Guilhen --- .../IdpCreateUserIfUniqueAuthenticator.java | 22 + ...rcedReauthenticationDifferentUserTest.java | 401 ++++++++++++++++++ 2 files changed, 423 insertions(+) create mode 100644 tests/base/src/test/java/org/keycloak/tests/broker/KcOidcBrokerForcedReauthenticationDifferentUserTest.java diff --git a/services/src/main/java/org/keycloak/authentication/authenticators/broker/IdpCreateUserIfUniqueAuthenticator.java b/services/src/main/java/org/keycloak/authentication/authenticators/broker/IdpCreateUserIfUniqueAuthenticator.java index d8a402e91868..b9dbb201c9ab 100644 --- a/services/src/main/java/org/keycloak/authentication/authenticators/broker/IdpCreateUserIfUniqueAuthenticator.java +++ b/services/src/main/java/org/keycloak/authentication/authenticators/broker/IdpCreateUserIfUniqueAuthenticator.java @@ -35,8 +35,11 @@ import org.keycloak.models.RealmModel; import org.keycloak.models.UserModel; import org.keycloak.models.UserProvider; +import org.keycloak.models.UserSessionModel; import org.keycloak.models.light.LightweightUserAdapter; +import org.keycloak.services.ErrorPageException; import org.keycloak.services.ServicesLogger; +import org.keycloak.services.managers.AuthenticationManager; import org.keycloak.services.messages.Messages; import org.keycloak.userprofile.UserProfile; import org.keycloak.userprofile.UserProfileContext; @@ -97,6 +100,11 @@ protected void authenticateImpl(AuthenticationFlowContext context, SerializedBro federatedUser = new LightweightUserAdapter(session, context.getAuthenticationSession().getParentSession().getId()); federatedUser.setUsername(username); } else if (duplication == null) { + // A user that does not exist yet can never be the user of a session that is still valid (eg. on prompt=login). + // AuthenticationProcessor.attachSession() rejects that mismatch too, but only after the new user was committed + // without its identity link, which locks them out. Reject before creating it instead. + rejectIfAnotherUserHoldsTheSession(context); + logger.debugf("No duplication detected. Creating account for user '%s' and linking with identity provider '%s' .", username, broker.getAlias()); @@ -161,6 +169,20 @@ protected void authenticateImpl(AuthenticationFlowContext context, SerializedBro } } + private void rejectIfAnotherUserHoldsTheSession(AuthenticationFlowContext context) { + KeycloakSession session = context.getSession(); + RealmModel realm = context.getRealm(); + // The authentication session shares its ID with the user session it was started on top of when the browser holds an SSO session. + UserSessionModel userSession = session.sessions().getUserSession(realm, context.getAuthenticationSession().getParentSession().getId()); + if (userSession == null || userSession.getUser() == null || !AuthenticationManager.isSessionValid(realm, userSession)) { + return; + } + + context.getEvent().detail(Details.EXISTING_USER, userSession.getUser().getId()).error(Errors.DIFFERENT_USER_AUTHENTICATED); + throw new ErrorPageException(session, context.getAuthenticationSession(), Response.Status.BAD_REQUEST, + Messages.DIFFERENT_USER_AUTHENTICATED, userSession.getUser().getUsername()); + } + // Could be overriden to detect duplication based on other criterias (firstName, lastName, ...) protected ExistingUserInfo checkExistingUser(AuthenticationFlowContext context, String username, SerializedBrokeredIdentityContext serializedCtx, BrokeredIdentityContext brokerContext) { diff --git a/tests/base/src/test/java/org/keycloak/tests/broker/KcOidcBrokerForcedReauthenticationDifferentUserTest.java b/tests/base/src/test/java/org/keycloak/tests/broker/KcOidcBrokerForcedReauthenticationDifferentUserTest.java new file mode 100644 index 000000000000..5b9c909a4bbb --- /dev/null +++ b/tests/base/src/test/java/org/keycloak/tests/broker/KcOidcBrokerForcedReauthenticationDifferentUserTest.java @@ -0,0 +1,401 @@ +package org.keycloak.tests.broker; + +import java.util.HashMap; +import java.util.List; +import java.util.Map; + +import jakarta.ws.rs.core.Response; + +import org.keycloak.broker.oidc.OIDCIdentityProviderFactory; +import org.keycloak.broker.provider.IdpLinkAction; +import org.keycloak.events.Details; +import org.keycloak.events.Errors; +import org.keycloak.models.IdentityProviderModel; +import org.keycloak.protocol.oidc.OIDCLoginProtocol; +import org.keycloak.representations.idm.ComponentRepresentation; +import org.keycloak.representations.idm.CredentialRepresentation; +import org.keycloak.representations.idm.EventRepresentation; +import org.keycloak.representations.idm.IdentityProviderRepresentation; +import org.keycloak.representations.idm.UserRepresentation; +import org.keycloak.representations.idm.UserSessionRepresentation; +import org.keycloak.testframework.annotations.InjectRealm; +import org.keycloak.testframework.annotations.KeycloakIntegrationTest; +import org.keycloak.testframework.injection.LifeCycle; +import org.keycloak.testframework.oauth.OAuthClient; +import org.keycloak.testframework.oauth.annotations.InjectOAuthClient; +import org.keycloak.testframework.realm.ClientBuilder; +import org.keycloak.testframework.realm.ManagedRealm; +import org.keycloak.testframework.realm.RealmBuilder; +import org.keycloak.testframework.realm.RealmConfig; +import org.keycloak.testframework.realm.UserBuilder; +import org.keycloak.testframework.ui.annotations.InjectPage; +import org.keycloak.testframework.ui.annotations.InjectWebDriver; +import org.keycloak.testframework.ui.page.ErrorPage; +import org.keycloak.testframework.ui.page.IdpConfirmLinkPage; +import org.keycloak.testframework.ui.page.LoginPage; +import org.keycloak.testframework.ui.webdriver.ManagedWebDriver; +import org.keycloak.testframework.util.ApiUtil; +import org.keycloak.testsuite.util.AccountHelper; + +import org.junit.jupiter.api.BeforeEach; +import org.junit.jupiter.api.Test; +import org.openqa.selenium.By; + +import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertTrue; + +/** + * Forced re-authentication (eg. {@code prompt=login}) through an identity provider that would complete as a user other + * than the one holding the current session is rejected before first-broker-login creates the user. See #54344. + *

+ * The first user must already be linked to the identity provider: on a forced re-authentication the user is + * already known, so {@code IdentityProviderBean} narrows the login page to the identity providers that user + * is federated with. Without an existing link no identity provider button is rendered at all and the + * scenario cannot be reached. + */ +@KeycloakIntegrationTest +public class KcOidcBrokerForcedReauthenticationDifferentUserTest { + + private static final String IDP_ALIAS = "forced-reauth-idp"; + private static final String SECOND_IDP_ALIAS = "forced-reauth-idp-2"; + private static final String CLIENT_ID = "broker-client"; + private static final String CLIENT_SECRET = "broker-secret"; + + // Logs in through the broker first, so it ends up linked to the IdP and holding the consumer session + // that the forced re-authentication then runs against. + private static final String FIRST_USER = "first-user"; + + // Only exists at the provider, so the consumer would run first-broker-login and create it. + private static final String SECOND_USER = "second-user"; + + private static final String PASSWORD = "password"; + + @InjectRealm(ref = "provider", config = ProviderRealmConfig.class, lifecycle = LifeCycle.METHOD) + ManagedRealm providerRealm; + + @InjectRealm(ref = "consumer", config = ConsumerRealmConfig.class, lifecycle = LifeCycle.METHOD) + ManagedRealm consumerRealm; + + @InjectOAuthClient(realmRef = "consumer") + OAuthClient oauth; + + @InjectWebDriver + ManagedWebDriver driver; + + @InjectPage + LoginPage loginPage; + + @InjectPage + ErrorPage errorPage; + + @InjectPage + IdpConfirmLinkPage idpConfirmLinkPage; + + // The provider's real base URL is only known once the realm exists, so the seeded placeholder endpoints + // have to be rewritten here rather than hardcoded in the realm config (which would break whenever the + // managed server is not plain http://localhost:8080). + @BeforeEach + void configureBrokerEndpoints() { + String providerBaseUrl = providerRealm.getBaseUrl(); + IdentityProviderRepresentation idp = consumerRealm.admin().identityProviders().get(IDP_ALIAS).toRepresentation(); + Map config = idp.getConfig(); + config.put("authorizationUrl", providerBaseUrl + "/protocol/openid-connect/auth"); + config.put("tokenUrl", providerBaseUrl + "/protocol/openid-connect/token"); + config.put("userInfoUrl", providerBaseUrl + "/protocol/openid-connect/userinfo"); + config.put("jwksUrl", providerBaseUrl + "/protocol/openid-connect/certs"); + // Admin GET masks the secret, and updating the endpoints with that mask is rejected. + if (ComponentRepresentation.SECRET_VALUE.equals(config.get("clientSecret"))) { + config.put("clientSecret", CLIENT_SECRET); + } + consumerRealm.admin().identityProviders().get(IDP_ALIAS).update(idp); + } + + @Test // #54344 + public void differentUserIsRejectedAndNotCreated() { + String firstUserSessionId = loginFirstUserAndStartForcedReauthentication(); + + loginPage.clickSocial(IDP_ALIAS); + loginPage.fillLogin(SECOND_USER, PASSWORD); + loginPage.submit(); + + errorPage.assertCurrent(); + assertTrue(errorPage.getError().contains("already authenticated as different user"), + "Unexpected error: " + errorPage.getError()); + + // The user used to be committed by first-broker-login before the login was rejected, without the identity + // link, which left an account that could never log in again. + assertEquals(List.of(), consumerRealm.admin().users().search(SECOND_USER, true), + "The rejected user must not be created"); + assertEquals(List.of(firstUserSessionId), userSessionIds(FIRST_USER), + "The session of the user holding it must be left alone"); + } + + @Test // #54344 + public void rejectionIsRecordedAsDifferentUserAuthenticated() { + loginFirstUserAndStartForcedReauthentication(); + + loginPage.clickSocial(IDP_ALIAS); + loginPage.fillLogin(SECOND_USER, PASSWORD); + loginPage.submit(); + errorPage.assertCurrent(); + + List events = consumerRealm.admin().getEvents(); + EventRepresentation rejection = events.stream() + .filter(e -> Errors.DIFFERENT_USER_AUTHENTICATED.equals(e.getError())) + .findFirst() + .orElseThrow(() -> new AssertionError("No " + Errors.DIFFERENT_USER_AUTHENTICATED + " event among " + events)); + assertEquals(consumerUserId(FIRST_USER), rejection.getDetails().get(Details.EXISTING_USER), + "The event must name the user holding the session"); + } + + @Test // #54344 + public void rejectedUserCanLogInAfterSigningOut() { + loginFirstUserAndStartForcedReauthentication(); + + loginPage.clickSocial(IDP_ALIAS); + loginPage.fillLogin(SECOND_USER, PASSWORD); + loginPage.submit(); + errorPage.assertCurrent(); + + AccountHelper.logout(consumerRealm.admin(), FIRST_USER); + AccountHelper.logout(providerRealm.admin(), SECOND_USER); + + oauth.openLoginForm(); + loginPage.clickSocial(IDP_ALIAS); + loginPage.fillLogin(SECOND_USER, PASSWORD); + loginPage.submit(); + + // This is what the orphaned account used to block: the duplicate-account form that the user can never complete. + assertTrue(oauth.parseLoginResponse().isSuccess(), "The user must be able to log in after signing out"); + assertEquals(1, federatedIdentityCount(SECOND_USER), "The user must be linked to the identity provider"); + } + + @Test // #54344 + public void sameUserCanReauthenticate() { + String firstUserSessionId = loginFirstUserAndStartForcedReauthentication(); + + loginPage.clickSocial(IDP_ALIAS); + loginPage.fillLogin(FIRST_USER, PASSWORD); + loginPage.submit(); + + assertTrue(oauth.parseLoginResponse().isSuccess(), "Re-authenticating as the same user must keep working"); + assertEquals(List.of(firstUserSessionId), userSessionIds(FIRST_USER), + "Re-authenticating as the same user must keep the user session"); + } + + /** + * Linking an identity provider to the logged in user (kc_action) also runs with a user session and an identity + * that is not linked to anyone yet, which is the shape of the mismatch that is rejected. It must keep working. + */ + @Test // #54344 + public void linkingIdentityProviderToTheLoggedInUserIsNotRejected() { + String localUser = "local-user"; + UserRepresentation user = UserBuilder.create(localUser) + .password(PASSWORD) + .email(localUser + "@example.com") + .emailVerified(true) + .firstName("Local") + .lastName("User") + .build(); + try (Response response = consumerRealm.admin().users().create(user)) { + assertEquals(201, response.getStatus()); + String userId = ApiUtil.getCreatedId(response); + consumerRealm.cleanup().add(r -> r.users().get(userId).remove()); + } + createProviderUser(SECOND_USER); + + oauth.openLoginForm(); + loginPage.fillLogin(localUser, PASSWORD); + loginPage.submit(); + assertTrue(oauth.parseLoginResponse().isSuccess(), "The local user should be logged in"); + assertEquals(0, federatedIdentityCount(localUser), "The local user starts out unlinked"); + + oauth.loginForm().kcAction(IdpLinkAction.PROVIDER_ID + ":" + IDP_ALIAS).open(); + driver.findElement(By.id("kc-continue")).click(); + + loginPage.fillLogin(SECOND_USER, PASSWORD); + loginPage.submit(); + + assertTrue(oauth.parseLoginResponse().isSuccess(), "Linking the identity provider must succeed"); + assertEquals(1, federatedIdentityCount(localUser), "The identity must be linked to the logged in user"); + assertEquals(List.of(), consumerRealm.admin().users().search(SECOND_USER, true), + "Linking must not create a user for the identity"); + } + + /** + * A user that is already linked to one identity provider links a second one whose identity resolves to that very + * user (same e-mail), so first-broker-login has to send them to the link confirmation page rather than create a user. + * The identity is not linked yet, which must not be mistaken for it belonging to a different user. + */ + @Test // #54344 + public void secondIdentityProviderResolvingToTheSessionUserCanBeLinked() { + addSecondIdentityProvider(); + String firstUserSessionId = loginFirstUser(); + + // The user was created by brokering, so it has no local password to verify the account with. + CredentialRepresentation credential = new CredentialRepresentation(); + credential.setType(CredentialRepresentation.PASSWORD); + credential.setValue(PASSWORD); + credential.setTemporary(false); + consumerRealm.admin().users().get(consumerUserId(FIRST_USER)).resetPassword(credential); + + oauth.loginForm() + .prompt(OIDCLoginProtocol.PROMPT_VALUE_LOGIN) + .param("kc_idp_hint", SECOND_IDP_ALIAS) + .open(); + loginPage.fillLogin(FIRST_USER, PASSWORD); + loginPage.submit(); + + // The e-mail matches the first user, which is the user holding the session. + idpConfirmLinkPage.assertCurrent(); + idpConfirmLinkPage.clickLinkAccount(); + + // No SMTP server is configured, so the existing account is verified by re-authentication. + loginPage.fillPassword(PASSWORD); + loginPage.submit(); + + assertTrue(oauth.parseLoginResponse().isSuccess(), "Linking the second identity provider must succeed"); + assertEquals(2, federatedIdentityCount(FIRST_USER), "Both identity providers must be linked to the user"); + assertEquals(List.of(firstUserSessionId), userSessionIds(FIRST_USER), + "The user session must be kept"); + } + + /** + * Logs the first user in through the broker, then opens a forced re-authentication and stops at the + * consumer's login page. + * + * @return the ID of the first user's session. + */ + private String loginFirstUserAndStartForcedReauthentication() { + String sessionId = loginFirstUser(); + + oauth.loginForm().prompt(OIDCLoginProtocol.PROMPT_VALUE_LOGIN).open(); + loginPage.assertCurrent(); + return sessionId; + } + + /** + * Logs the first user in through the broker and drops the provider-side session. + * + * @return the ID of the first user's session. + */ + private String loginFirstUser() { + createProviderUser(FIRST_USER); + createProviderUser(SECOND_USER); + + oauth.openLoginForm(); + loginPage.clickSocial(IDP_ALIAS); + loginPage.fillLogin(FIRST_USER, PASSWORD); + loginPage.submit(); + assertTrue(oauth.parseLoginResponse().isSuccess(), "The first user should be logged in"); + + String sessionId = singleSessionId(FIRST_USER); + + // Drop only the provider-side session, so the identity provider prompts for credentials again while + // the consumer session for the first user stays alive. That live consumer session is what the + // re-authentication later collides with. + AccountHelper.logout(providerRealm.admin(), FIRST_USER); + + consumerRealm.admin().clearEvents(); + return sessionId; + } + + // Same provider as the first identity provider, under another alias, so the same provider user is not linked to it yet. + private void addSecondIdentityProvider() { + IdentityProviderRepresentation first = consumerRealm.admin().identityProviders().get(IDP_ALIAS).toRepresentation(); + Map config = new HashMap<>(first.getConfig()); + // Admin GET masks the secret. + config.put("clientSecret", CLIENT_SECRET); + + IdentityProviderRepresentation second = new IdentityProviderRepresentation(); + second.setAlias(SECOND_IDP_ALIAS); + second.setProviderId(first.getProviderId()); + second.setEnabled(true); + second.setTrustEmail(true); + second.setConfig(config); + try (Response response = consumerRealm.admin().identityProviders().create(second)) { + assertEquals(201, response.getStatus()); + } + } + + private String singleSessionId(String username) { + List sessions = userSessionIds(username); + assertEquals(1, sessions.size(), username + " should hold exactly one consumer session"); + return sessions.get(0); + } + + private String consumerUserId(String username) { + List users = consumerRealm.admin().users().search(username, true); + assertEquals(1, users.size(), "Expected exactly one user named " + username + " in the consumer realm"); + return users.get(0).getId(); + } + + private int federatedIdentityCount(String username) { + return consumerRealm.admin().users().get(consumerUserId(username)).getFederatedIdentity().size(); + } + + private List userSessionIds(String username) { + return consumerRealm.admin().users().get(consumerUserId(username)).getUserSessions().stream() + .map(UserSessionRepresentation::getId) + .toList(); + } + + private void createProviderUser(String username) { + // A complete profile keeps first-broker-login from stopping at the review-profile page, so the + // flow runs end to end without interactive steps. + UserRepresentation user = UserBuilder.create(username) + .password(PASSWORD) + .email(username + "@example.com") + .emailVerified(true) + .firstName("Test") + .lastName("User") + .build(); + try (Response response = providerRealm.admin().users().create(user)) { + assertEquals(201, response.getStatus()); + String userId = ApiUtil.getCreatedId(response); + providerRealm.cleanup().add(r -> r.users().get(userId).remove()); + } + } + + public static class ProviderRealmConfig implements RealmConfig { + + @Override + public RealmBuilder configure(RealmBuilder realm) { + return realm.clients( + ClientBuilder.create(CLIENT_ID) + .secret(CLIENT_SECRET) + .redirectUris("*") + .directAccessGrantsEnabled() + ); + } + } + + public static class ConsumerRealmConfig implements RealmConfig { + + @Override + public RealmBuilder configure(RealmBuilder realm) { + // Endpoints are placeholders; configureBrokerEndpoints() rewrites them once the provider + // realm's actual base URL is known. + String providerBase = "http://localhost:8080/realms/provider"; + + IdentityProviderRepresentation idp = new IdentityProviderRepresentation(); + idp.setAlias(IDP_ALIAS); + idp.setProviderId(OIDCIdentityProviderFactory.PROVIDER_ID); + idp.setEnabled(true); + idp.setTrustEmail(true); + idp.setConfig(Map.of( + "clientId", CLIENT_ID, + "clientSecret", CLIENT_SECRET, + "authorizationUrl", providerBase + "/protocol/openid-connect/auth", + "tokenUrl", providerBase + "/protocol/openid-connect/token", + "userInfoUrl", providerBase + "/protocol/openid-connect/userinfo", + "jwksUrl", providerBase + "/protocol/openid-connect/certs", + "defaultScope", "openid email profile", + IdentityProviderModel.SYNC_MODE, "IMPORT" + )); + + return realm.identityProviders(idp) + .eventsEnabled(true); + } + } +} From 90b3f7e6fa16bcd4073c1b513f2a71e10bbece32 Mon Sep 17 00:00:00 2001 From: Stefan Guilhen Date: Fri, 30 Oct 2026 08:48:20 -0300 Subject: [PATCH 10/16] Escape delimiter in federated and composite cache keys to prevent collisions Closes #54368 Signed-off-by: Stefan Guilhen --- .../cache/infinispan/UserCacheSession.java | 9 ++- ...nispanIdentityProviderStorageProvider.java | 9 ++- ...dpVerifyAccountLinkActionTokenHandler.java | 12 +++- .../IdentityProviderMapperTest.java | 58 +++++++++++++++++++ .../tests/broker/BrokerCacheTest.java | 50 +++++++++++++++- .../KcOidcFirstBrokerLoginLinkProofTest.java | 18 +++++- 6 files changed, 150 insertions(+), 6 deletions(-) diff --git a/model/infinispan/src/main/java/org/keycloak/models/cache/infinispan/UserCacheSession.java b/model/infinispan/src/main/java/org/keycloak/models/cache/infinispan/UserCacheSession.java index 7cb87388fe91..129f49316dfd 100755 --- a/model/infinispan/src/main/java/org/keycloak/models/cache/infinispan/UserCacheSession.java +++ b/model/infinispan/src/main/java/org/keycloak/models/cache/infinispan/UserCacheSession.java @@ -263,8 +263,15 @@ private static String getUserByFederatedIdentityCacheKey(String realmId, Federat return getUserByFederatedIdentityCacheKey(realmId, socialLink.getIdentityProvider(), socialLink.getUserId()); } + private static String escapeCacheKeyComponent(String value) { + if (value == null) { + return ""; + } + return value.replace("\\", "\\\\").replace(".", "\\."); + } + static String getUserByFederatedIdentityCacheKey(String realmId, String identityProvider, String socialUserId) { - return realmId + ".idp." + identityProvider + "." + socialUserId; + return realmId + ".idp." + escapeCacheKeyComponent(identityProvider) + "." + escapeCacheKeyComponent(socialUserId); } static String getFederatedIdentityLinksCacheKey(String userId) { diff --git a/model/infinispan/src/main/java/org/keycloak/models/cache/infinispan/idp/InfinispanIdentityProviderStorageProvider.java b/model/infinispan/src/main/java/org/keycloak/models/cache/infinispan/idp/InfinispanIdentityProviderStorageProvider.java index 54bd3b64f3d6..984fce9ce0c0 100644 --- a/model/infinispan/src/main/java/org/keycloak/models/cache/infinispan/idp/InfinispanIdentityProviderStorageProvider.java +++ b/model/infinispan/src/main/java/org/keycloak/models/cache/infinispan/idp/InfinispanIdentityProviderStorageProvider.java @@ -69,8 +69,15 @@ public static String cacheKeyIdpAlias(RealmModel realm, String alias) { return realm.getId() + "." + alias + IDP_ALIAS_KEY_SUFFIX; } + private static String escapeCacheKeyComponent(String value) { + if (value == null) { + return ""; + } + return value.replace("\\", "\\\\").replace(".", "\\."); + } + private static String cacheKeyIdpMapperAliasName(RealmModel realm, String alias, String name) { - return realm.getId() + "." + alias + IDP_ALIAS_KEY_SUFFIX + "." + name; + return realm.getId() + "." + escapeCacheKeyComponent(alias) + IDP_ALIAS_KEY_SUFFIX + "." + escapeCacheKeyComponent(name); } public static String cacheKeyOrgId(RealmModel realm, String orgId) { diff --git a/services/src/main/java/org/keycloak/authentication/actiontoken/idpverifyemail/IdpVerifyAccountLinkActionTokenHandler.java b/services/src/main/java/org/keycloak/authentication/actiontoken/idpverifyemail/IdpVerifyAccountLinkActionTokenHandler.java index 011a189d837f..939c47b4eead 100644 --- a/services/src/main/java/org/keycloak/authentication/actiontoken/idpverifyemail/IdpVerifyAccountLinkActionTokenHandler.java +++ b/services/src/main/java/org/keycloak/authentication/actiontoken/idpverifyemail/IdpVerifyAccountLinkActionTokenHandler.java @@ -16,7 +16,10 @@ */ package org.keycloak.authentication.actiontoken.idpverifyemail; +import java.nio.charset.StandardCharsets; import java.util.Map; +import java.util.stream.Collectors; +import java.util.stream.Stream; import jakarta.ws.rs.core.Response; import jakarta.ws.rs.core.UriBuilder; @@ -28,11 +31,13 @@ import org.keycloak.authentication.actiontoken.ActionTokenContext; import org.keycloak.authentication.actiontoken.TokenUtils; import org.keycloak.authentication.authenticators.broker.IdpEmailVerificationAuthenticator; +import org.keycloak.common.util.Base64Url; import org.keycloak.events.Details; import org.keycloak.events.Errors; import org.keycloak.events.EventBuilder; import org.keycloak.events.EventType; import org.keycloak.forms.login.LoginFormsProvider; +import org.keycloak.jose.jws.crypto.HashUtils; import org.keycloak.models.ClientModel; import org.keycloak.models.Constants; import org.keycloak.models.FederatedIdentityModel; @@ -210,8 +215,11 @@ public static boolean runIfUserVerified(KeycloakSession session, UserModel user, return isUserVerified; } - static String getUserVerifiedSingleObjectKey(String userId, String idpAlias, String externalId) { - return "kc.brokering.user.verified." + userId + "." + idpAlias + "." + externalId; + public static String getUserVerifiedSingleObjectKey(String userId, String idpAlias, String externalId) { + String binding = Stream.of(userId, idpAlias, externalId) + .map(value -> Base64Url.encode(value.getBytes(StandardCharsets.UTF_8))) + .collect(Collectors.joining(".")); + return "kc.brokering.user.verified." + HashUtils.sha256UrlEncodedHash(binding, StandardCharsets.UTF_8); } private Response sendLinkConfirmedAlready(KeycloakSession session, EventBuilder event, UserModel user, IdpVerifyAccountLinkActionToken token) { diff --git a/tests/base/src/test/java/org/keycloak/tests/admin/identityprovider/IdentityProviderMapperTest.java b/tests/base/src/test/java/org/keycloak/tests/admin/identityprovider/IdentityProviderMapperTest.java index 63196c75e906..d7a2e1880f72 100644 --- a/tests/base/src/test/java/org/keycloak/tests/admin/identityprovider/IdentityProviderMapperTest.java +++ b/tests/base/src/test/java/org/keycloak/tests/admin/identityprovider/IdentityProviderMapperTest.java @@ -16,6 +16,7 @@ import org.keycloak.events.admin.ResourceType; import org.keycloak.models.IdentityProviderMapperModel; import org.keycloak.models.IdentityProviderMapperSyncMode; +import org.keycloak.models.RealmModel; import org.keycloak.representations.idm.IdentityProviderMapperRepresentation; import org.keycloak.representations.idm.IdentityProviderMapperTypeRepresentation; import org.keycloak.testframework.annotations.InjectRealm; @@ -23,6 +24,8 @@ import org.keycloak.testframework.events.AdminEventAssertion; import org.keycloak.testframework.injection.LifeCycle; import org.keycloak.testframework.realm.ManagedRealm; +import org.keycloak.testframework.remote.runonserver.InjectRunOnServer; +import org.keycloak.testframework.remote.runonserver.RunOnServerClient; import org.keycloak.testframework.server.KeycloakServerConfig; import org.keycloak.testframework.server.KeycloakServerConfigBuilder; import org.keycloak.testframework.util.ApiUtil; @@ -44,6 +47,9 @@ public class IdentityProviderMapperTest extends AbstractIdentityProviderTest { @InjectRealm(lifecycle = LifeCycle.METHOD) ManagedRealm managedRealm; + @InjectRunOnServer + RunOnServerClient runOnServer; + @Test public void testMapperTypes() { @@ -334,6 +340,58 @@ public void testUpdateMapperByIdRejectsForeignIdentityProvider() { assertEquals("offline_access", providerB.getMapperById(idB).getConfig().get("role")); } + @Test + public void testIdpMapperCacheDelimiterCollision() { + String realmName = managedRealm.getName(); + + // Tuple A: alias="corp.idp.alias.part", name="mapper" + // Tuple B: alias="corp", name="part.idp.alias.mapper" + // Unescaped format: realmId + "." + alias + ".idp.alias." + name + // Without escaping, both tuples yield: realmId + ".corp.idp.alias.part.idp.alias.mapper" + create(createRep("corp.idp.alias.part", "google")); + create(createRep("corp", "google")); + + IdentityProviderResource providerA = managedRealm.admin().identityProviders().get("corp.idp.alias.part"); + IdentityProviderResource providerB = managedRealm.admin().identityProviders().get("corp"); + + addHardcodedRoleMapper(providerA, "corp.idp.alias.part", "mapper", "offline_access"); + + // Prime cache with Tuple A via getMapperByName in its own transaction + runOnServer.run(session -> { + RealmModel realm = session.realms().getRealmByName(realmName); + session.getContext().setRealm(realm); + + IdentityProviderMapperModel mapper = session.identityProviders().getMapperByName("corp.idp.alias.part", "mapper"); + Assertions.assertNotNull(mapper); + assertEquals("offline_access", mapper.getConfig().get("role")); + }); + + // Lookup Tuple B via getMapperByName must NOT return Tuple A from cache + runOnServer.run(session -> { + RealmModel realm = session.realms().getRealmByName(realmName); + session.getContext().setRealm(realm); + + IdentityProviderMapperModel mapper = session.identityProviders().getMapperByName("corp", "part.idp.alias.mapper"); + Assertions.assertNull(mapper, "Lookup for (corp, part.idp.alias.mapper) should not collide with (corp.idp.alias.part, mapper)"); + }); + + // Now create Tuple B and verify both resolve independently from cache + addHardcodedRoleMapper(providerB, "corp", "part.idp.alias.mapper", "master-realm.manage-realm"); + + runOnServer.run(session -> { + RealmModel realm = session.realms().getRealmByName(realmName); + session.getContext().setRealm(realm); + + IdentityProviderMapperModel mapperA = session.identityProviders().getMapperByName("corp.idp.alias.part", "mapper"); + IdentityProviderMapperModel mapperB = session.identityProviders().getMapperByName("corp", "part.idp.alias.mapper"); + + Assertions.assertNotNull(mapperA); + Assertions.assertNotNull(mapperB); + assertEquals("offline_access", mapperA.getConfig().get("role")); + assertEquals("master-realm.manage-realm", mapperB.getConfig().get("role")); + }); + } + private String addHardcodedRoleMapper(IdentityProviderResource provider, String alias, String name, String role) { IdentityProviderMapperRepresentation mapper = new IdentityProviderMapperRepresentation(); mapper.setIdentityProviderAlias(alias); diff --git a/tests/base/src/test/java/org/keycloak/tests/broker/BrokerCacheTest.java b/tests/base/src/test/java/org/keycloak/tests/broker/BrokerCacheTest.java index 9b417f75519c..228e222ea401 100644 --- a/tests/base/src/test/java/org/keycloak/tests/broker/BrokerCacheTest.java +++ b/tests/base/src/test/java/org/keycloak/tests/broker/BrokerCacheTest.java @@ -81,9 +81,57 @@ public void reLoginAfterIdpDeletedAndRecreatedWithSameAlias() { }); } + @Test + public void testFederatedIdentityCacheDelimiterCollision() { + String realmName = managedRealm.getName(); + + createIdentityProvider("corp.saml"); + createIdentityProvider("corp"); + + UserRepresentation user1 = new UserRepresentation(); + user1.setUsername("alice"); + String user1Id; + try (Response response = managedRealm.admin().users().create(user1)) { + user1Id = ApiUtil.getCreatedId(response); + } + + FederatedIdentityRepresentation link1 = new FederatedIdentityRepresentation(); + link1.setIdentityProvider("corp.saml"); + link1.setUserId("alice"); + link1.setUserName("alice"); + try (Response response = managedRealm.admin().users().get(user1Id).addFederatedIdentity("corp.saml", link1)) { + assertEquals(Response.Status.NO_CONTENT.getStatusCode(), response.getStatus()); + } + + // Prime cache with (corp.saml, alice) + runOnServer.run(session -> { + RealmModel realm = session.realms().getRealmByName(realmName); + session.getContext().setRealm(realm); + + FederatedIdentityModel socialLink = new FederatedIdentityModel("corp.saml", "alice", "alice"); + UserModel found = session.users().getUserByFederatedIdentity(realm, socialLink); + assertNotNull(found); + assertEquals("alice", found.getUsername()); + }); + + // Lookup (corp, saml.alice) must NOT return alice due to delimiter collision + runOnServer.run(session -> { + RealmModel realm = session.realms().getRealmByName(realmName); + session.getContext().setRealm(realm); + + FederatedIdentityModel socialLink = new FederatedIdentityModel("corp", "saml.alice", "saml.alice"); + UserModel found = session.users().getUserByFederatedIdentity(realm, socialLink); + assertNull(found, "Lookup for (corp, saml.alice) should not collide with (corp.saml, alice)"); + }); + } + private void createIdentityProvider() { + createIdentityProvider(IDP_ALIAS); + } + + private void createIdentityProvider(String alias) { IdentityProviderRepresentation idp = new IdentityProviderRepresentation(); - idp.setAlias(IDP_ALIAS); + idp.setAlias(alias); idp.setProviderId("oidc"); idp.setEnabled(true); managedRealm.admin().identityProviders().create(idp).close(); diff --git a/tests/base/src/test/java/org/keycloak/tests/broker/oidc/KcOidcFirstBrokerLoginLinkProofTest.java b/tests/base/src/test/java/org/keycloak/tests/broker/oidc/KcOidcFirstBrokerLoginLinkProofTest.java index df71fae9a20b..fc695aa652a5 100644 --- a/tests/base/src/test/java/org/keycloak/tests/broker/oidc/KcOidcFirstBrokerLoginLinkProofTest.java +++ b/tests/base/src/test/java/org/keycloak/tests/broker/oidc/KcOidcFirstBrokerLoginLinkProofTest.java @@ -284,7 +284,23 @@ private void waitForFederatedIdentity(UserResource userResource, int expectedCou private String linkProofKey(String consumerUserId) { String federatedUserId = getProviderRealm().admin().users().search(getUserLogin(), true).get(0).getId(); - return "kc.brokering.user.verified." + consumerUserId + "." + getIdpAlias() + "." + federatedUserId; + return IdpVerifyAccountLinkActionTokenHandler.getUserVerifiedSingleObjectKey(consumerUserId, getIdpAlias(), federatedUserId); + } + + @Test + public void testLinkProofKeyDelimiterCollisionCrossConsumptionFails() { + String consumerUserId = "victim-user-id"; + String key1 = IdpVerifyAccountLinkActionTokenHandler.getUserVerifiedSingleObjectKey(consumerUserId, "corp", "partner.user"); + String key2 = IdpVerifyAccountLinkActionTokenHandler.getUserVerifiedSingleObjectKey(consumerUserId, "corp.partner", "user"); + org.junit.jupiter.api.Assertions.assertNotEquals(key1, key2); + + runOnServer.run(session -> session.singleUseObjects().put(key1, 300, java.util.Map.of())); + try { + assertTrue(runOnServer.fetch(session -> session.singleUseObjects().contains(key1), Boolean.class)); + assertFalse(runOnServer.fetch(session -> session.singleUseObjects().contains(key2), Boolean.class)); + } finally { + runOnServer.run(session -> session.singleUseObjects().remove(key1)); + } } private boolean isLinkProofPresent(String consumerUserId) { From a9d7e41beff86356e1812075772fdffdddf6f515 Mon Sep 17 00:00:00 2001 From: vramik Date: Thu, 5 Nov 2026 15:00:46 +0100 Subject: [PATCH 11/16] Remove ability to set up multivalued issuer for OIDC providers Closes #54458 Signed-off-by: vramik --- .../topics/identity-broker/oidc.adoc | 1 + .../topics/changes/changes-27_0_0.adoc | 20 ++++ docs/guides/securing-apps/token-exchange.adoc | 1 + .../JpaUpdate27_0_0_OidcIssuerMigration.java | 82 ++++++++++++++ .../META-INF/jpa-changelog-27.0.0.xml | 20 ++++ .../migration/migrators/MigrateTo27_0_0.java | 1 - .../datastore/DefaultExportImportManager.java | 11 +- .../SingleIssuerIdentityProviderConfig.java | 24 ++++ .../migrators/OidcIssuerMigration.java | 107 ++++++++++++++++++ .../broker/oidc/OIDCIdentityProvider.java | 24 ++-- .../oidc/OIDCIdentityProviderConfig.java | 7 +- .../IdentityProvidersPartialImport.java | 9 +- .../IdentityProviderIssuerTest.java | 14 +++ 13 files changed, 300 insertions(+), 21 deletions(-) create mode 100644 model/jpa/src/main/java/org/keycloak/connections/jpa/updater/liquibase/custom/JpaUpdate27_0_0_OidcIssuerMigration.java create mode 100644 server-spi-private/src/main/java/org/keycloak/broker/provider/SingleIssuerIdentityProviderConfig.java create mode 100644 server-spi-private/src/main/java/org/keycloak/migration/migrators/OidcIssuerMigration.java diff --git a/docs/documentation/server_admin/topics/identity-broker/oidc.adoc b/docs/documentation/server_admin/topics/identity-broker/oidc.adoc index 8bdf4e10dfd7..68a06a34c85e 100644 --- a/docs/documentation/server_admin/topics/identity-broker/oidc.adoc +++ b/docs/documentation/server_admin/topics/identity-broker/oidc.adoc @@ -51,6 +51,7 @@ In the case of JWT signed with private key or Client secret as jwt, it is requir |Issuer |{project_name} validates issuer claims, in responses from the IDP, against this value. +Only a single issuer is supported. Configure a separate identity provider for each additional issuer. |Default Scopes |A list of OIDC scopes {project_name} sends with the authentication request. The default value is `openid`. A space separates each scope. diff --git a/docs/documentation/upgrading/topics/changes/changes-27_0_0.adoc b/docs/documentation/upgrading/topics/changes/changes-27_0_0.adoc index 2d44dd5edf72..8bbfd6a1b06f 100644 --- a/docs/documentation/upgrading/topics/changes/changes-27_0_0.adoc +++ b/docs/documentation/upgrading/topics/changes/changes-27_0_0.adoc @@ -26,6 +26,26 @@ General validation also preserves the legacy web origin literals `${authAdminUrl Trusted Hosts resolves relative logo URLs against the {project_name} frontend URL and checks that host against its allowlist. The client's root URL does not determine where the browser loads a logo. The pattern executor continues to match literal configured values, including markers and relative URLs, and respects an explicit field selection. Sector identifier URLs in pairwise protocol mappers remain subject to their separate validation. +=== OIDC identity providers accept a single issuer +// https://github.com/keycloak/keycloak/issues/54458 + +Identity providers based on the OpenID Connect provider configuration no longer accept comma-separated lists in the `Issuer` setting. +This includes the generic OpenID Connect and Keycloak OpenID Connect providers, social providers using that configuration, and custom provider subclasses. +This affects broker login and legacy external-to-internal token exchange. +Each identity provider must trust a single issuer, because federated user links identify users by provider alias and subject, without recording the issuer. +Previously, matching subject identifiers from different trusted issuers could resolve to the same local user. + +During a database upgrade, registered providers whose configurations require a single issuer are discovered through their factories. +Those with issuer lists retain the first nonempty issuer, with surrounding whitespace removed, and are disabled. +The same migration runs when importing realm JSON exported by a version before 27.0.0, including exports with no version specified. +A warning identifies each affected provider and its original issuer list. +New configurations and imports exported by 27.0.0 or later reject issuer lists. + +Before enabling a migrated provider, verify the selected issuer and review its existing federated user links. +The original issuer cannot be recovered from those links, so {project_name} cannot automatically assign them to separate providers. +Configure a separate provider alias for every additional issuer and establish links to the intended local users after verifying account ownership. +Update applications that select a provider alias, including callers of legacy external-to-internal token exchange, and grant the appropriate exchange permissions to the new providers. + === Authorization endpoint JSON errors // https://github.com/keycloak/keycloak/issues/54134 diff --git a/docs/guides/securing-apps/token-exchange.adoc b/docs/guides/securing-apps/token-exchange.adoc index fe62b89364f8..00b48990e965 100644 --- a/docs/guides/securing-apps/token-exchange.adoc +++ b/docs/guides/securing-apps/token-exchange.adoc @@ -1029,6 +1029,7 @@ The `subject_token_type` must either be `urn:ietf:params:oauth:token-type:access If the type is `urn:ietf:params:oauth:token-type:access_token` you specify the `subject_issuer` parameter and it must be the alias of the configured identity provider. If the type is `urn:ietf:params:oauth:token-type:jwt`, the provider will be matched via the `iss` (issuer) claim within the JWT which must be the alias of the provider, or a registered issuer within the provider's configuration. +Each OIDC identity provider accepts a single configured issuer. To exchange tokens from multiple issuers, configure a separate identity provider for each issuer. For validation, if the token is an access token, the provider's user info service will be invoked to validate the token. A successful call will mean that the access token is valid. If the subject token is a JWT and if the provider has signature validation enabled, that will be attempted, diff --git a/model/jpa/src/main/java/org/keycloak/connections/jpa/updater/liquibase/custom/JpaUpdate27_0_0_OidcIssuerMigration.java b/model/jpa/src/main/java/org/keycloak/connections/jpa/updater/liquibase/custom/JpaUpdate27_0_0_OidcIssuerMigration.java new file mode 100644 index 000000000000..48644b11e0b5 --- /dev/null +++ b/model/jpa/src/main/java/org/keycloak/connections/jpa/updater/liquibase/custom/JpaUpdate27_0_0_OidcIssuerMigration.java @@ -0,0 +1,82 @@ +/* + * Copyright 2026 Red Hat, Inc. and/or its affiliates + * and other contributors as indicated by the @author tags. + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package org.keycloak.connections.jpa.updater.liquibase.custom; + +import java.sql.PreparedStatement; +import java.sql.ResultSet; + +import org.keycloak.migration.migrators.OidcIssuerMigration; + +import liquibase.exception.CustomChangeException; +import liquibase.statement.core.UpdateStatement; +import liquibase.structure.core.Table; +import org.jboss.logging.Logger; + +public class JpaUpdate27_0_0_OidcIssuerMigration extends CustomKeycloakTask { + + private static final Logger LOG = Logger.getLogger(JpaUpdate27_0_0_OidcIssuerMigration.class); + + @Override + protected void generateStatementsImpl() throws CustomChangeException { + String sql = "SELECT p.INTERNAL_ID, p.PROVIDER_ID, p.PROVIDER_ALIAS, p.REALM_ID, c.VALUE FROM " + + getTableName("IDENTITY_PROVIDER") + " p JOIN " + getTableName("IDENTITY_PROVIDER_CONFIG") + + " c ON c.IDENTITY_PROVIDER_ID = p.INTERNAL_ID WHERE c.NAME = 'issuer' AND c.VALUE LIKE '%,%'"; + try (PreparedStatement ps = connection.prepareStatement(sql); ResultSet rs = ps.executeQuery()) { + while (rs.next()) { + String id = rs.getString(1); + String providerId = rs.getString(2); + String alias = rs.getString(3); + String realmId = rs.getString(4); + String issuers = rs.getString(5); + if (!OidcIssuerMigration.isMultiIssuerProvider(kcSession, providerId, issuers)) { + if (OidcIssuerMigration.getFactory(kcSession, providerId) == null) { + LOG.warnf("Identity provider '%s' in realm '%s' configures multiple issuers (%s), but its provider " + + "'%s' is not installed, so it cannot be migrated. If you install that provider later, " + + "replace the issuer list with a single issuer and configure a separate identity provider " + + "for each additional issuer.", alias, realmId, issuers, providerId); + } + continue; + } + String first = OidcIssuerMigration.firstIssuer(issuers); + if (first != null) { + statements.add(new UpdateStatement(null, null, database.correctObjectName("IDENTITY_PROVIDER_CONFIG", Table.class)) + .addNewColumnValue("VALUE", first) + .setWhereClause("IDENTITY_PROVIDER_ID=? AND NAME=?").addWhereParameter(id).addWhereParameter("issuer")); + LOG.warnf("Disabling identity provider '%s' in realm '%s' because it configured multiple issuers (%s). " + + "The same subject from different issuers could authenticate as the wrong user. Only the first issuer will be retained. " + + "Review existing federated links before enabling the provider. Configure a separate identity provider for each additional issuer; " + + "existing user links cannot be split automatically.", + alias, realmId, issuers); + } else { + LOG.warnf("Disabling identity provider '%s' in realm '%s' because its issuer value (%s) " + + "contains commas but no valid issuer. Correct the issuer before enabling the provider.", + alias, realmId, issuers); + } + statements.add(new UpdateStatement(null, null, database.correctObjectName("IDENTITY_PROVIDER", Table.class)) + .addNewColumnValue("ENABLED", false).setWhereClause("INTERNAL_ID=?").addWhereParameter(id)); + } + } catch (Exception e) { + throw new CustomChangeException(getTaskId() + ": Failed to migrate OIDC issuer lists", e); + } + confirmationMessage.append("Migrated OIDC issuer lists and disabled affected identity providers"); + } + + @Override + protected String getTaskId() { + return "27.0.0-54458-oidc-single-issuer"; + } +} diff --git a/model/jpa/src/main/resources/META-INF/jpa-changelog-27.0.0.xml b/model/jpa/src/main/resources/META-INF/jpa-changelog-27.0.0.xml index ff4141e6b806..7cc9cbf148a2 100644 --- a/model/jpa/src/main/resources/META-INF/jpa-changelog-27.0.0.xml +++ b/model/jpa/src/main/resources/META-INF/jpa-changelog-27.0.0.xml @@ -1,8 +1,28 @@ + + + + + diff --git a/model/storage-private/src/main/java/org/keycloak/migration/migrators/MigrateTo27_0_0.java b/model/storage-private/src/main/java/org/keycloak/migration/migrators/MigrateTo27_0_0.java index 0907ac0c4504..90c0fca349c5 100644 --- a/model/storage-private/src/main/java/org/keycloak/migration/migrators/MigrateTo27_0_0.java +++ b/model/storage-private/src/main/java/org/keycloak/migration/migrators/MigrateTo27_0_0.java @@ -14,7 +14,6 @@ * See the License for the specific language governing permissions and * limitations under the License. */ - package org.keycloak.migration.migrators; import org.keycloak.migration.ModelVersion; diff --git a/model/storage-private/src/main/java/org/keycloak/storage/datastore/DefaultExportImportManager.java b/model/storage-private/src/main/java/org/keycloak/storage/datastore/DefaultExportImportManager.java index 6f46d24c1aa3..15f255da4996 100644 --- a/model/storage-private/src/main/java/org/keycloak/storage/datastore/DefaultExportImportManager.java +++ b/model/storage-private/src/main/java/org/keycloak/storage/datastore/DefaultExportImportManager.java @@ -51,6 +51,7 @@ import org.keycloak.migration.ModelVersion; import org.keycloak.migration.migrators.MigrateTo8_0_0; import org.keycloak.migration.migrators.MigrationUtils; +import org.keycloak.migration.migrators.OidcIssuerMigration; import org.keycloak.models.AuthenticationExecutionModel; import org.keycloak.models.AuthenticationFlowModel; import org.keycloak.models.AuthenticatorConfigModel; @@ -216,6 +217,10 @@ public void importRealm(RealmRepresentation rep, RealmModel newRealm, Runnable u ModelVersion version = DefaultMigrationManager.getModelVersionFromRep(rep); convertDeprecatedSocialProviders(rep); + // Migrate before identity provider configurations are validated during import. + if (version == null || version.lessThan(OidcIssuerMigration.VERSION)) { + OidcIssuerMigration.migrateImport(session, rep); + } convertDeprecatedApplications(session, rep); convertDeprecatedClientTemplates(rep); @@ -650,7 +655,11 @@ private static Map createClientScopes(KeycloakSession private static void importIdentityProviders(RealmRepresentation rep, RealmModel newRealm, KeycloakSession session) { if (rep.getIdentityProviders() != null) { for (IdentityProviderRepresentation representation : rep.getIdentityProviders()) { - session.identityProviders().create(RepresentationToModel.toModel(newRealm, representation, session)); + try { + session.identityProviders().create(RepresentationToModel.toModel(newRealm, representation, session)); + } catch (IllegalArgumentException e) { + throw new ModelValidationException("Invalid configuration for identity provider '" + representation.getAlias() + "': " + e.getMessage()); + } } } } diff --git a/server-spi-private/src/main/java/org/keycloak/broker/provider/SingleIssuerIdentityProviderConfig.java b/server-spi-private/src/main/java/org/keycloak/broker/provider/SingleIssuerIdentityProviderConfig.java new file mode 100644 index 000000000000..a85fab2fa48c --- /dev/null +++ b/server-spi-private/src/main/java/org/keycloak/broker/provider/SingleIssuerIdentityProviderConfig.java @@ -0,0 +1,24 @@ +/* + * Copyright 2026 Red Hat, Inc. and/or its affiliates + * and other contributors as indicated by the @author tags. + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package org.keycloak.broker.provider; + +/** + * Identifies identity provider configurations that accept a single issuer. Config subclasses inherit + * this capability so migrations can discover affected providers through their factories. + */ +public interface SingleIssuerIdentityProviderConfig { +} diff --git a/server-spi-private/src/main/java/org/keycloak/migration/migrators/OidcIssuerMigration.java b/server-spi-private/src/main/java/org/keycloak/migration/migrators/OidcIssuerMigration.java new file mode 100644 index 000000000000..421b51b53c91 --- /dev/null +++ b/server-spi-private/src/main/java/org/keycloak/migration/migrators/OidcIssuerMigration.java @@ -0,0 +1,107 @@ +/* + * Copyright 2026 Red Hat, Inc. and/or its affiliates + * and other contributors as indicated by the @author tags. + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package org.keycloak.migration.migrators; + +import java.util.Arrays; + +import org.keycloak.broker.provider.IdentityProvider; +import org.keycloak.broker.provider.IdentityProviderFactory; +import org.keycloak.broker.provider.SingleIssuerIdentityProviderConfig; +import org.keycloak.broker.social.SocialIdentityProvider; +import org.keycloak.migration.ModelVersion; +import org.keycloak.models.IdentityProviderModel; +import org.keycloak.models.KeycloakSession; +import org.keycloak.representations.idm.RealmRepresentation; + +import org.jboss.logging.Logger; + +/** + * Migration of legacy issuer lists for providers with single-issuer configurations. Federated links do not record an issuer, + * so a migrated provider must be reviewed before it can safely be enabled again. + *

+ * This intentionally does not implement {@link Migration} and is not registered in {@code DefaultMigrationManager}. + * Identity providers are validated eagerly while a realm representation is imported (see + * {@code OIDCIdentityProviderConfig#validate}), before the standard post-import {@code migrateImport} hooks ever run, + * so a multi-issuer configuration would fail validation before any registered migrator got a chance to fix it. + * Instead, {@link #migrateImport(KeycloakSession, RealmRepresentation)} is called directly against the representation + * before identity providers are created. + */ +public final class OidcIssuerMigration { + + public static final ModelVersion VERSION = new ModelVersion("27.0.0"); + private static final Logger LOG = Logger.getLogger(OidcIssuerMigration.class); + + private OidcIssuerMigration() { + } + + /** + * Resolves the factory for the given provider id, or {@code null} when that provider is not installed. + */ + public static IdentityProviderFactory getFactory(KeycloakSession session, String providerId) { + IdentityProviderFactory factory = (IdentityProviderFactory) session.getKeycloakSessionFactory() + .getProviderFactory(IdentityProvider.class, providerId); + if (factory == null) { + factory = (IdentityProviderFactory) session.getKeycloakSessionFactory() + .getProviderFactory(SocialIdentityProvider.class, providerId); + } + return factory; + } + + public static boolean isMultiIssuerProvider(KeycloakSession session, String providerId, String issuer) { + if (issuer == null || !issuer.contains(",")) { + return false; + } + IdentityProviderFactory factory = getFactory(session, providerId); + return factory != null && factory.createConfig() instanceof SingleIssuerIdentityProviderConfig; + } + + public static String firstIssuer(String issuers) { + return Arrays.stream(issuers.split(",")) + .map(String::trim) + .filter(s -> !s.isEmpty()) + .findFirst() + .orElse(null); + } + + public static void migrateImport(KeycloakSession session, RealmRepresentation realm) { + if (realm.getIdentityProviders() == null) { + return; + } + realm.getIdentityProviders().forEach(idp -> { + if (idp.getConfig() == null) { + return; + } + String issuers = idp.getConfig().get(IdentityProviderModel.ISSUER); + if (isMultiIssuerProvider(session, idp.getProviderId(), issuers)) { + String first = firstIssuer(issuers); + if (first != null) { + idp.getConfig().put(IdentityProviderModel.ISSUER, first); + LOG.warnf("Disabled identity provider '%s' in imported realm '%s' because it configured multiple issuers (%s). " + + "The same subject from different issuers could authenticate as the wrong user. Only the first issuer was retained. " + + "Review existing federated links before enabling the provider. Configure a separate identity provider for each additional issuer; " + + "existing user links cannot be split automatically.", + idp.getAlias(), realm.getRealm(), issuers); + } else { + LOG.warnf("Disabled identity provider '%s' in imported realm '%s' because its issuer value (%s) " + + "contains commas but no valid issuer. Correct the issuer before enabling the provider.", + idp.getAlias(), realm.getRealm(), issuers); + } + idp.setEnabled(false); + } + }); + } +} 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 65e6fcf0d298..5e6b862f6abf 100755 --- a/services/src/main/java/org/keycloak/broker/oidc/OIDCIdentityProvider.java +++ b/services/src/main/java/org/keycloak/broker/oidc/OIDCIdentityProvider.java @@ -793,15 +793,11 @@ protected JsonWebToken validateToken(String encodedToken, boolean ignoreAudience throw new IdentityBrokerException("Token issued for does not match client id"); } - String trustedIssuers = getConfig().getIssuer(); + String trustedIssuer = getConfig().getIssuer(); - if (trustedIssuers != null && trustedIssuers.length() > 0) { - String[] issuers = trustedIssuers.split(","); - - for (String trustedIssuer : issuers) { - if (iss != null && iss.equals(trustedIssuer.trim())) { - return token; - } + if (trustedIssuer != null && !trustedIssuer.isEmpty()) { + if (iss != null && iss.equals(trustedIssuer.trim())) { + return token; } throw new IdentityBrokerException("Wrong issuer from token. Got: " + iss + " expected: " + getConfig().getIssuer()); @@ -832,16 +828,10 @@ public boolean isIssuer(String issuer, MultivaluedMap params) { if (requestedIssuer == null) requestedIssuer = issuer; if (requestedIssuer.equals(getConfig().getAlias())) return true; - String trustedIssuers = getConfig().getIssuer(); - - if (trustedIssuers != null && trustedIssuers.length() > 0) { - String[] issuers = trustedIssuers.split(","); + String trustedIssuer = getConfig().getIssuer(); - for (String trustedIssuer : issuers) { - if (requestedIssuer.equals(trustedIssuer.trim())) { - return true; - } - } + if (trustedIssuer != null && !trustedIssuer.isEmpty()) { + return requestedIssuer.equals(trustedIssuer.trim()); } return false; diff --git a/services/src/main/java/org/keycloak/broker/oidc/OIDCIdentityProviderConfig.java b/services/src/main/java/org/keycloak/broker/oidc/OIDCIdentityProviderConfig.java index 881ab8665bcf..4ae8751ebb60 100755 --- a/services/src/main/java/org/keycloak/broker/oidc/OIDCIdentityProviderConfig.java +++ b/services/src/main/java/org/keycloak/broker/oidc/OIDCIdentityProviderConfig.java @@ -17,6 +17,7 @@ package org.keycloak.broker.oidc; import org.keycloak.broker.jwtauthorizationgrant.JWTAuthorizationGrantConfig; +import org.keycloak.broker.provider.SingleIssuerIdentityProviderConfig; import org.keycloak.common.enums.SslRequired; import org.keycloak.models.IdentityProviderModel; import org.keycloak.models.IdentityProviderType; @@ -27,7 +28,7 @@ /** * @author Pedro Igor */ -public class OIDCIdentityProviderConfig extends OAuth2IdentityProviderConfig implements JWTAuthorizationGrantConfig, IssuerValidation { +public class OIDCIdentityProviderConfig extends OAuth2IdentityProviderConfig implements JWTAuthorizationGrantConfig, IssuerValidation, SingleIssuerIdentityProviderConfig { public static final String JWKS_URL = "jwksUrl"; @@ -166,6 +167,10 @@ public void setAllowClientIdAsAudience(boolean allowClientIdAsAudience) { @Override public void validate(RealmModel realm) { super.validate(realm); + String issuer = getIssuer(); + if (issuer != null && issuer.contains(",")) { + throw new IllegalArgumentException("Only one issuer is supported. Configure a separate identity provider for each issuer."); + } SslRequired sslRequired = realm.getSslRequired(); checkUrl(sslRequired, getJwksUrl(), "jwks_url"); checkUrl(sslRequired, getLogoutUrl(), "logout_url"); diff --git a/services/src/main/java/org/keycloak/partialimport/IdentityProvidersPartialImport.java b/services/src/main/java/org/keycloak/partialimport/IdentityProvidersPartialImport.java index 425cf2510e5c..d361cb5343f7 100644 --- a/services/src/main/java/org/keycloak/partialimport/IdentityProvidersPartialImport.java +++ b/services/src/main/java/org/keycloak/partialimport/IdentityProvidersPartialImport.java @@ -20,6 +20,7 @@ import java.util.List; import org.keycloak.models.KeycloakSession; +import org.keycloak.models.ModelValidationException; import org.keycloak.models.RealmModel; import org.keycloak.models.utils.KeycloakModelUtils; import org.keycloak.models.utils.RepresentationToModel; @@ -71,7 +72,13 @@ public void remove(RealmModel realm, KeycloakSession session, IdentityProviderRe @Override public void create(RealmModel realm, KeycloakSession session, IdentityProviderRepresentation idpRep) { idpRep.setInternalId(KeycloakModelUtils.generateId()); - session.identityProviders().create(RepresentationToModel.toModel(realm, idpRep, session)); + try { + session.identityProviders().create(RepresentationToModel.toModel(realm, idpRep, session)); + } catch (IllegalArgumentException e) { + // Partial import carries no version, so legacy configurations are not migrated here. Report the + // problem to the caller instead of failing as an internal server error. + throw new ModelValidationException("Invalid configuration for identity provider '" + getName(idpRep) + "': " + e.getMessage()); + } } } diff --git a/tests/base/src/test/java/org/keycloak/tests/admin/identityprovider/IdentityProviderIssuerTest.java b/tests/base/src/test/java/org/keycloak/tests/admin/identityprovider/IdentityProviderIssuerTest.java index 70f7e1c6619f..aab5ac9df2b4 100644 --- a/tests/base/src/test/java/org/keycloak/tests/admin/identityprovider/IdentityProviderIssuerTest.java +++ b/tests/base/src/test/java/org/keycloak/tests/admin/identityprovider/IdentityProviderIssuerTest.java @@ -38,6 +38,20 @@ @KeycloakIntegrationTest public class IdentityProviderIssuerTest extends AbstractIdentityProviderTest { + @Test + public void testMultipleIssuersNotAllowed() { + String multipleIssuers = "https://issuer-a.example,https://issuer-b.example"; + IdentityProviderRepresentation identityProvider = createRep("oidc", OIDCIdentityProviderFactory.PROVIDER_ID, + multipleIssuers, false, false); + identityProvider.getConfig().put("jwksUrl", "https://issuer-a.example/jwks"); + + try (Response response = managedRealm.admin().identityProviders().create(identityProvider)) { + Assertions.assertEquals(400, response.getStatus()); + ErrorRepresentation error = response.readEntity(ErrorRepresentation.class); + assertEquals("Only one issuer is supported. Configure a separate identity provider for each issuer.", error.getErrorMessage()); + } + } + @Test public void testCreateUpdateDuplicateIdentityProvider() { String issuer = "http://localhost:8080"; From 77d1edc568f9faf1d7c949aa7e8a253bbc60fd20 Mon Sep 17 00:00:00 2001 From: Stan Silvert Date: Fri, 6 Nov 2026 09:11:11 -0500 Subject: [PATCH 12/16] Ignore docs.redhat.com in the external links check docs.redhat.com now answers every scripted request with 403, so the External links check fails for any PR that touches the documentation. The pages still exist; the site blocks automated clients the same way Weblate and iso.org do. Fixes #54762 Signed-off-by: Stan Silvert Assisted-by: Claude Opus 5.5 --- docs/documentation/tests/src/test/resources/ignored-links | 2 ++ 1 file changed, 2 insertions(+) diff --git a/docs/documentation/tests/src/test/resources/ignored-links b/docs/documentation/tests/src/test/resources/ignored-links index 4dd7dcf24ccb..1008fcfe247e 100644 --- a/docs/documentation/tests/src/test/resources/ignored-links +++ b/docs/documentation/tests/src/test/resources/ignored-links @@ -51,3 +51,5 @@ https://www.h2database.com/* https://www.iso.org/* # Blocks automated HTTP requests from CI runners (returns 403), but loads fine in a browser. https://hosted.weblate.org/projects/keycloak/ +# Blocks automated HTTP requests from CI runners (returns 403), but loads fine in a browser. +https://docs.redhat.com/* From 74b563fdd3d6d76488f1322494642b017203f30c Mon Sep 17 00:00:00 2001 From: andymunro <48995441+andymunro@users.noreply.github.com> Date: Fri, 6 Nov 2026 10:39:41 -0500 Subject: [PATCH 13/16] Fixes link from Bootstrapping (#54728) Closes #54727 Signed-off-by: AndyMunro --- docs/guides/server/bootstrap-admin-recovery.adoc | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/docs/guides/server/bootstrap-admin-recovery.adoc b/docs/guides/server/bootstrap-admin-recovery.adoc index 678e825c6790..32dac372d0a4 100644 --- a/docs/guides/server/bootstrap-admin-recovery.adoc +++ b/docs/guides/server/bootstrap-admin-recovery.adoc @@ -12,7 +12,7 @@ A user or service admin account created using one of the methods described below == Bootstrapping a temporary admin account at {project_name} startup -{project_name} `start` and `start-dev` commands support options for bootstrapping both temporary admin users and admin service accounts. These options are standard configuration options, so they can be specified through any of the sources described in <@links.server id="configuration" anchor="_configuring_sources_for_keycloak" />, such as environment variables or CLI parameters. For instance, the following examples demonstrate how to use the `start` and `start-dev` commands with CLI parameters to bootstrap a temporary admin user and an admin service account, respectively: +{project_name} `start` and `start-dev` commands support options for bootstrapping both temporary admin users and admin service accounts. These options are standard configuration options, so they can be specified through any of the sources described in <@links.server id="configuration"/>, such as environment variables or CLI parameters. For instance, the following examples demonstrate how to use the `start` and `start-dev` commands with CLI parameters to bootstrap a temporary admin user and an admin service account, respectively: <@kc.start parameters="--bootstrap-admin-username tmpadm --bootstrap-admin-password pass"/> From 62edf998b21eb2812a71b8e4457bf098643f42d4 Mon Sep 17 00:00:00 2001 From: Andy <41118244+andy-clapson@users.noreply.github.com> Date: Fri, 6 Nov 2026 11:52:22 -0500 Subject: [PATCH 14/16] Show credentials of disabled users in admin console (#54778) Closes #54777 Signed-off-by: Andy Co-authored-by: Claude Opus 5.5 --- js/apps/admin-ui/src/user/UserCredentials.tsx | 2 +- js/apps/admin-ui/test/users/main.spec.ts | 22 +++++++++++++++++++ 2 files changed, 23 insertions(+), 1 deletion(-) diff --git a/js/apps/admin-ui/src/user/UserCredentials.tsx b/js/apps/admin-ui/src/user/UserCredentials.tsx index 59c76badb5e5..fcd15df9fa7e 100644 --- a/js/apps/admin-ui/src/user/UserCredentials.tsx +++ b/js/apps/admin-ui/src/user/UserCredentials.tsx @@ -128,7 +128,7 @@ export const UserCredentials = ({ user, setUser }: UserCredentialsProps) => { useFetch( () => { - if (user.enabled) { + if (user.enabled || !user.federationLink) { return adminClient.users.getCredentials({ id: user.id! }); } return Promise.resolve([]); diff --git a/js/apps/admin-ui/test/users/main.spec.ts b/js/apps/admin-ui/test/users/main.spec.ts index f335aa5405b5..363b8a2439de 100644 --- a/js/apps/admin-ui/test/users/main.spec.ts +++ b/js/apps/admin-ui/test/users/main.spec.ts @@ -356,6 +356,28 @@ test.describe("Existing users", () => { await assertNotificationMessage(page, "Added group membership"); await assertRowExists(page, "test-group"); }); + + test("shows credentials of a disabled user", async ({ page }) => { + await using testBed = await createTestBed({ + users: [ + { + username: existingUserName, + enabled: false, + credentials: [{ type: "password", value: "test" }], + }, + ], + }); + const user = await adminClient.findUserByUsername( + testBed.realm, + existingUserName, + ); + + await login(page, { + to: toUser({ realm: testBed.realm, id: user.id!, tab: "credentials" }), + }); + + await expect(page.getByTestId("credentialType")).toHaveText("Password"); + }); }); test.describe("User attribute search", () => { From 0bc6a6223fc9e518ff62d839d1fc1ba83dc0d58a Mon Sep 17 00:00:00 2001 From: Steven Hawkins Date: Fri, 6 Nov 2026 12:46:37 -0500 Subject: [PATCH 15/16] fix: clarifying hostname context considerations (#54681) closes: #52484 Signed-off-by: Steve Hawkins --- docs/guides/server/hostname.adoc | 4 +++- docs/guides/server/reverseproxy.adoc | 22 +++++++++++++++++----- 2 files changed, 20 insertions(+), 6 deletions(-) diff --git a/docs/guides/server/hostname.adoc b/docs/guides/server/hostname.adoc index 782d8109e25d..1955fc8288ec 100644 --- a/docs/guides/server/hostname.adoc +++ b/docs/guides/server/hostname.adoc @@ -28,7 +28,9 @@ As demonstrated in the previous example, the scheme and port are not explicitly Similarly, your reverse proxy might expose {project_name} at a different context path. It is possible to configure {project_name} to reflect that via the `hostname` and `hostname-admin` options. See the following example: -<@kc.start parameters="--hostname https://my.keycloak.org:123/auth"/> +<@kc.start parameters="--hostname https://my.keycloak.org:123/auth --proxy-headers xforwarded"/> + +NOTE: Including a path in the `hostname` option only changes the URLs that {project_name} generates. It does not change the path on which the server listens. This configuration assumes that the reverse proxy removes the context path (`/auth` in the example) from the request URI before forwarding it to {project_name}, and that `proxy-headers` is set, which is required when the path in `hostname` differs from `http-relative-path`. If your reverse proxy forwards the context path unchanged, you must instead set the `http-relative-path` option to the same value so that the server expects it. For more details, see the <@links.server id="reverseproxy" anchor="context-path-on-reverse-proxy"/> guide. == Utilizing an internal URL for communication among clients diff --git a/docs/guides/server/reverseproxy.adoc b/docs/guides/server/reverseproxy.adoc index 5ca7b941057c..d03577d6b995 100644 --- a/docs/guides/server/reverseproxy.adoc +++ b/docs/guides/server/reverseproxy.adoc @@ -371,7 +371,7 @@ The following table shows the recommended paths to expose. |=== We assume you run {project_name} on the root path `/` on your reverse proxy/gateway's public API. -If not, prefix the path with your desired one. +If not, prefix the Reverse Proxy Path with your desired one. See <> for how the context path must be handled between the proxy and {project_name}. NOTE: If you configured an `http-relative-path` on the server, proceed as follows to use discovery with RFC 8414: Configure a reverse proxy to map the `/.well-known/` path without the prefix to the path with the prefix on the server. @@ -389,14 +389,26 @@ To do that, do not expose the path `/realms/master/` to the public internet. Depending on your topology, you can combine this with serving the Admin UI and Admin API on a dedicated hostname and port. See <@links.server id="hostname" anchor="administration-console-on-a-separate-hostname" /> for details. -=== Different context path on reverse proxy +[#context-path-on-reverse-proxy] +=== Context path on reverse proxy By default, {project_name} is exposed through the root context path (`/`). -If the proxy uses a different context path than {project_name}, one of the following must be done: +If the proxy exposes {project_name} under a context path, such as `/auth`, the proxy and {project_name} must agree on whether that path is part of the request URI that reaches the server. Choose one of the following approaches. + +WARNING: A mismatch between the request URI sent by the reverse proxy and the one expected by {project_name} typically results in `404 Not Found` errors, or in redirects and links that point to the wrong location. + +==== Proxy removes the context path + +The server keeps serving requests at the root context path, and the context path is only used by {project_name} to build the URLs it returns. To configure this, do one of the following: * Use a simple hostname for the `hostname` option, `xforwarded` for the `proxy-headers` option, and have the proxy set the `X-Forwarded-Prefix` header. -* Use a full URL for the `hostname` option including the proxy context path, for example using `--hostname=https://my.keycloak.org/auth` if {project_name} is exposed through the reverse proxy on `/auth`. -* Change the context path of {project_name} itself to match the context path for the reverse proxy using the `http-relative-path` option. +* Use a full URL for the `hostname` option including the proxy context path, for example using `--hostname=https://my.keycloak.org/auth` if {project_name} is exposed through the reverse proxy on `/auth`. The `proxy-headers` option must also be set, otherwise the server logs a likely misconfiguration warning because the path in `hostname` does not match `http-relative-path`. + +In both cases, the reverse proxy must remove the context path from the request URI before forwarding the request to {project_name}. For example, a request to `/auth/realms/master` must be forwarded as `/realms/master`. Consult the documentation of your reverse proxy for how to rewrite or strip a path prefix. + +==== Proxy and server use the same context path + +Set the `http-relative-path` option on {project_name} to the same context path the proxy exposes, for example `--http-relative-path=/auth`. The server then serves requests under that path, so the reverse proxy must forward the request URI unchanged. For more details on exposing {project_name} on a different hostname or context path, including the Administration REST API and Console, see <@links.server id="hostname"/>. From d4d55541da034a55a785c09922fcd62e51cd5bde Mon Sep 17 00:00:00 2001 From: Maximilian Brodda <11014840+QuellcodeWanderer@users.noreply.github.com> Date: Fri, 6 Nov 2026 22:05:14 +0100 Subject: [PATCH 16/16] Make OIDCIdentityProvider.BROKER_NONCE_PARAM public Closes #54740 Signed-off-by: Maximilian Brodda Co-authored-by: Maximilian Brodda --- .../java/org/keycloak/broker/oidc/OIDCIdentityProvider.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) 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 5e6b862f6abf..2acfdf4b17ce 100755 --- a/services/src/main/java/org/keycloak/broker/oidc/OIDCIdentityProvider.java +++ b/services/src/main/java/org/keycloak/broker/oidc/OIDCIdentityProvider.java @@ -115,7 +115,7 @@ public class OIDCIdentityProvider extends AbstractOAuth2IdentityProvider SUPPORTED_TOKEN_TYPES = Arrays.asList(TokenUtil.TOKEN_TYPE_ID, TokenUtil.TOKEN_TYPE_BEARER, TokenUtil.TOKEN_TYPE_JWT_ACCESS_TOKEN, TokenUtil.TOKEN_TYPE_JWT_ACCESS_TOKEN_PREFIXED); public OIDCIdentityProvider(KeycloakSession session, OIDCIdentityProviderConfig config) {