From 0a801ae95c097cea4b927abaa5ec67e7abe3a449 Mon Sep 17 00:00:00 2001 From: Pedro Igor Date: Mon, 3 Jun 2024 18:22:19 -0300 Subject: [PATCH] Prevent users to unlink from their home identity provider when they are a managed member Closes #30092 Signed-off-by: Pedro Igor Co-authored-by: Vlasta Ramik --- .../browser/OrganizationAuthenticator.java | 9 +- .../organization/utils/Organizations.java | 11 +- .../OrganizationMemberValidator.java | 35 ++++-- .../keycloak/services/messages/Messages.java | 2 + .../account/LinkedAccountsResource.java | 55 +++++---- .../admin/OrganizationAccountTest.java | 105 ++++++++++++++++++ .../account/messages/messages_en.properties | 1 + 7 files changed, 174 insertions(+), 44 deletions(-) create mode 100755 testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/organization/admin/OrganizationAccountTest.java diff --git a/services/src/main/java/org/keycloak/organization/authentication/authenticators/browser/OrganizationAuthenticator.java b/services/src/main/java/org/keycloak/organization/authentication/authenticators/browser/OrganizationAuthenticator.java index a5b174a73b2f..5fd4106cf2fc 100644 --- a/services/src/main/java/org/keycloak/organization/authentication/authenticators/browser/OrganizationAuthenticator.java +++ b/services/src/main/java/org/keycloak/organization/authentication/authenticators/browser/OrganizationAuthenticator.java @@ -21,6 +21,7 @@ import static org.keycloak.organization.utils.Organizations.resolveBroker; import java.util.List; + import jakarta.ws.rs.core.MultivaluedMap; import org.keycloak.authentication.AuthenticationFlowContext; import org.keycloak.authentication.AuthenticationFlowError; @@ -86,14 +87,14 @@ public void action(AuthenticationFlowContext context) { return; } - IdentityProviderModel broker = resolveBroker(session, user); + List broker = resolveBroker(session, user); - if (broker == null) { + if (broker.isEmpty()) { // not a managed member, continue with the regular flow context.attempted(); - } else { + } else if (broker.size() == 1) { // user is a managed member and associated with a broker, redirect automatically - redirect(context, broker.getAlias(), user.getEmail()); + redirect(context, broker.get(0).getAlias(), user.getEmail()); } return; diff --git a/services/src/main/java/org/keycloak/organization/utils/Organizations.java b/services/src/main/java/org/keycloak/organization/utils/Organizations.java index efe7b9bdf037..c4fbd710d4ad 100644 --- a/services/src/main/java/org/keycloak/organization/utils/Organizations.java +++ b/services/src/main/java/org/keycloak/organization/utils/Organizations.java @@ -66,19 +66,18 @@ public static boolean canManageOrganizationGroup(KeycloakSession session, GroupM return true; } - public static IdentityProviderModel resolveBroker(KeycloakSession session, UserModel user) { + public static List resolveBroker(KeycloakSession session, UserModel user) { OrganizationProvider provider = session.getProvider(OrganizationProvider.class); RealmModel realm = session.getContext().getRealm(); OrganizationModel organization = provider.getByMember(user); if (organization == null || !organization.isEnabled()) { - return null; + return List.of(); } if (provider.isManagedMember(organization, user)) { - // user is a managed member, try to resolve the origin broker and redirect automatically List organizationBrokers = organization.getIdentityProviders().toList(); - List brokers = session.users().getFederatedIdentitiesStream(realm, user) + return session.users().getFederatedIdentitiesStream(realm, user) .map(f -> { IdentityProviderModel broker = realm.getIdentityProviderByAlias(f.getIdentityProvider()); @@ -95,11 +94,9 @@ public static IdentityProviderModel resolveBroker(KeycloakSession session, UserM return null; }).filter(Objects::nonNull) .toList(); - - return brokers.size() == 1 ? brokers.get(0) : null; } - return null; + return List.of(); } public static Consumer removeGroup(KeycloakSession session, RealmModel realm) { diff --git a/services/src/main/java/org/keycloak/organization/validator/OrganizationMemberValidator.java b/services/src/main/java/org/keycloak/organization/validator/OrganizationMemberValidator.java index d74ba3fa86e1..957137ac0a3f 100644 --- a/services/src/main/java/org/keycloak/organization/validator/OrganizationMemberValidator.java +++ b/services/src/main/java/org/keycloak/organization/validator/OrganizationMemberValidator.java @@ -21,7 +21,11 @@ import static org.keycloak.organization.utils.Organizations.resolveBroker; import static org.keycloak.validate.BuiltinValidators.emailValidator; +import java.util.Collections; +import java.util.HashSet; import java.util.List; +import java.util.Set; +import java.util.stream.Collectors; import org.keycloak.Config.Scope; import org.keycloak.broker.provider.BrokeredIdentityContext; @@ -92,7 +96,7 @@ private void validateEmailDomain(String email, String inputHint, ValidationConte AttributeContext attributeContext = upContext.getAttributeContext(); UserModel user = attributeContext.getUser(); String emailDomain = email.substring(email.indexOf('@') + 1); - List expectedDomains = organization.getDomains().map(OrganizationDomainModel::getName).toList(); + Set expectedDomains = organization.getDomains().map(OrganizationDomainModel::getName).collect(Collectors.toSet()); if (expectedDomains.isEmpty()) { // no domain to check @@ -116,24 +120,33 @@ private void validateEmailDomain(String email, String inputHint, ValidationConte context.addError(new ValidationError(ID, inputHint, "Email domain does not match any domain from the organization")); } - private static List resolveExpectedDomainsForManagedUser(ValidationContext context, UserModel user) { - IdentityProviderModel broker = resolveBroker(context.getSession(), user); + private static Set resolveExpectedDomainsForManagedUser(ValidationContext context, UserModel user) { + List brokers = resolveBroker(context.getSession(), user); - if (broker == null) { - return List.of(); + if (brokers.isEmpty()) { + return Set.of(); + } + + Set domains = new HashSet<>(); + + for (IdentityProviderModel broker : brokers) { + String domain = broker.getConfig().get(OrganizationModel.ORGANIZATION_DOMAIN_ATTRIBUTE); + + if (domain != null) { + domains.add(domain); + } } - String domain = broker.getConfig().get(OrganizationModel.ORGANIZATION_DOMAIN_ATTRIBUTE); - return ofNullable(domain).map(List::of).orElse(List.of()); + return Collections.unmodifiableSet(domains); } - private static List resolveExpectedDomainsWhenReviewingFederatedUserProfile(OrganizationModel organization, AttributeContext attributeContext) { + private static Set resolveExpectedDomainsWhenReviewingFederatedUserProfile(OrganizationModel organization, AttributeContext attributeContext) { // validating in the context of the brokering flow KeycloakSession session = attributeContext.getSession(); BrokeredIdentityContext brokerContext = (BrokeredIdentityContext) session.getAttribute(BrokeredIdentityContext.class.getName()); if (brokerContext == null) { - return List.of(); + return Set.of(); } String alias = brokerContext.getIdpConfig().getAlias(); @@ -144,12 +157,12 @@ private static List resolveExpectedDomainsWhenReviewingFederatedUserProf if (broker == null) { // the broker the user is authenticating is not linked to the organization - return List.of(); + return Set.of(); } // expect the email domain to match the domain set to the broker or none if not set String brokerDomain = broker.getConfig().get(OrganizationModel.ORGANIZATION_DOMAIN_ATTRIBUTE); - return ofNullable(brokerDomain).map(List::of).orElse(List.of()); + return ofNullable(brokerDomain).map(Set::of).orElse(Set.of()); } private OrganizationModel resolveOrganization(ValidationContext context, KeycloakSession session) { diff --git a/services/src/main/java/org/keycloak/services/messages/Messages.java b/services/src/main/java/org/keycloak/services/messages/Messages.java index 62ecc4558d94..aa01a637b76b 100755 --- a/services/src/main/java/org/keycloak/services/messages/Messages.java +++ b/services/src/main/java/org/keycloak/services/messages/Messages.java @@ -245,6 +245,8 @@ public class Messages { public static final String FEDERATED_IDENTITY_REMOVING_LAST_PROVIDER = "federatedIdentityRemovingLastProviderMessage"; + public static final String FEDERATED_IDENTITY_BOUND_ORGANIZATION = "federatedIdentityBoundOrganization"; + public static final String IDENTITY_PROVIDER_REDIRECT_ERROR = "identityProviderRedirectErrorMessage"; public static final String IDENTITY_PROVIDER_REMOVED = "identityProviderRemovedMessage"; 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 215c073baa24..b85727e75740 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 @@ -38,6 +38,8 @@ import jakarta.ws.rs.core.Response; import jakarta.ws.rs.core.UriBuilder; import org.jboss.logging.Logger; +import org.keycloak.common.Profile; +import org.keycloak.common.Profile.Feature; import org.keycloak.http.HttpRequest; import org.keycloak.broker.social.SocialIdentityProvider; import org.keycloak.common.util.Base64Url; @@ -52,6 +54,7 @@ import org.keycloak.models.UserModel; import org.keycloak.models.credential.PasswordCredentialModel; import org.keycloak.models.utils.KeycloakModelUtils; +import org.keycloak.organization.utils.Organizations; import org.keycloak.provider.ProviderFactory; import org.keycloak.representations.account.AccountLinkUriRepresentation; import org.keycloak.representations.account.LinkedAccountRepresentation; @@ -72,7 +75,7 @@ */ public class LinkedAccountsResource { private static final Logger logger = Logger.getLogger(LinkedAccountsResource.class); - + private final KeycloakSession session; private final HttpRequest request; private final EventBuilder event; @@ -80,10 +83,10 @@ public class LinkedAccountsResource { private final RealmModel realm; private final Auth auth; - public LinkedAccountsResource(KeycloakSession session, - HttpRequest request, + public LinkedAccountsResource(KeycloakSession session, + HttpRequest request, Auth auth, - EventBuilder event, + EventBuilder event, UserModel user) { this.session = session; this.request = request; @@ -92,7 +95,7 @@ public LinkedAccountsResource(KeycloakSession session, this.user = user; realm = session.getContext().getRealm(); } - + @GET @Path("/") @Produces(MediaType.APPLICATION_JSON) @@ -101,7 +104,7 @@ public Response linkedAccounts() { SortedSet linkedAccounts = getLinkedAccounts(this.session, this.realm, this.user); return Cors.builder().auth().allowedOrigins(auth.getToken()).add(Response.ok(linkedAccounts)); } - + private Set findSocialIds() { return session.getKeycloakSessionFactory().getProviderFactoriesStream(SocialIdentityProvider.class) .map(ProviderFactory::getId) @@ -141,7 +144,7 @@ private FederatedIdentityModel getIdentity(Stream identi return identities.filter(model -> Objects.equals(model.getIdentityProvider(), providerAlias)) .findFirst().orElse(null); } - + @GET @Path("/{providerAlias}") @Produces(MediaType.APPLICATION_JSON) @@ -149,11 +152,11 @@ private FederatedIdentityModel getIdentity(Stream identi public Response buildLinkedAccountURI(@PathParam("providerAlias") String providerAlias, @QueryParam("redirectUri") String redirectUri) { auth.require(AccountRoles.MANAGE_ACCOUNT); - + if (redirectUri == null) { ErrorResponse.error(Messages.INVALID_REDIRECT_URI, Response.Status.BAD_REQUEST); } - + String errorMessage = checkCommonPreconditions(providerAlias); if (errorMessage != null) { throw ErrorResponse.error(errorMessage, Response.Status.BAD_REQUEST); @@ -161,7 +164,7 @@ public Response buildLinkedAccountURI(@PathParam("providerAlias") String provide if (auth.getSession() == null) { throw ErrorResponse.error(Messages.SESSION_NOT_ACTIVE, Response.Status.BAD_REQUEST); } - + try { String nonce = UUID.randomUUID().toString(); MessageDigest md = MessageDigest.getInstance("SHA-256"); @@ -177,35 +180,43 @@ public Response buildLinkedAccountURI(@PathParam("providerAlias") String provide .queryParam("client_id", ACCOUNT_CONSOLE_CLIENT_ID) .queryParam("redirect_uri", redirectUri) .build(); - + AccountLinkUriRepresentation rep = new AccountLinkUriRepresentation(); rep.setAccountLinkUri(linkUri); rep.setHash(hash); rep.setNonce(nonce); - + return Cors.builder().auth().allowedOrigins(auth.getToken()).add(Response.ok(rep)); } catch (Exception spe) { spe.printStackTrace(); throw ErrorResponse.error(Messages.FAILED_TO_PROCESS_RESPONSE, Response.Status.INTERNAL_SERVER_ERROR); } } - + @DELETE @Path("/{providerAlias}") @Produces(MediaType.APPLICATION_JSON) public Response removeLinkedAccount(@PathParam("providerAlias") String providerAlias) { auth.require(AccountRoles.MANAGE_ACCOUNT); - + String errorMessage = checkCommonPreconditions(providerAlias); if (errorMessage != null) { throw ErrorResponse.error(errorMessage, Response.Status.BAD_REQUEST); } - + FederatedIdentityModel link = session.users().getFederatedIdentity(realm, user, providerAlias); if (link == null) { throw ErrorResponse.error(translateErrorMessage(Messages.FEDERATED_IDENTITY_NOT_ACTIVE), Response.Status.BAD_REQUEST); } + if (Profile.isFeatureEnabled(Feature.ORGANIZATION)) { + if (Organizations.resolveBroker(session, user).stream() + .map(IdentityProviderModel::getAlias) + .anyMatch(providerAlias::equals)) { + throw ErrorResponse.error(translateErrorMessage(Messages.FEDERATED_IDENTITY_BOUND_ORGANIZATION), Response.Status.BAD_REQUEST); + } + } + // Removing last social provider is not possible if you don't have other possibility to authenticate if (!(session.users().getFederatedIdentitiesStream(realm, user).count() > 1 || user.getFederationLink() != null || isPasswordSet())) { throw ErrorResponse.error(translateErrorMessage(Messages.FEDERATED_IDENTITY_REMOVING_LAST_PROVIDER), Response.Status.BAD_REQUEST); @@ -223,22 +234,22 @@ public Response removeLinkedAccount(@PathParam("providerAlias") String providerA return Cors.builder().auth().allowedOrigins(auth.getToken()).add(Response.noContent()); } - + private String checkCommonPreconditions(String providerAlias) { auth.require(AccountRoles.MANAGE_ACCOUNT); - + if (Validation.isEmpty(providerAlias)) { return Messages.MISSING_IDENTITY_PROVIDER; } - + if (!isValidProvider(providerAlias)) { return Messages.IDENTITY_PROVIDER_NOT_FOUND; } - + if (!user.isEnabled()) { return Messages.ACCOUNT_DISABLED; } - + return null; } @@ -249,11 +260,11 @@ private String translateErrorMessage(String errorCode) { return errorCode; } } - + private boolean isPasswordSet() { return user.credentialManager().isConfiguredFor(PasswordCredentialModel.TYPE); } - + private boolean isValidProvider(String providerAlias) { return realm.getIdentityProvidersStream().anyMatch(model -> Objects.equals(model.getAlias(), providerAlias)); } diff --git a/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/organization/admin/OrganizationAccountTest.java b/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/organization/admin/OrganizationAccountTest.java new file mode 100755 index 000000000000..d019e4c940d2 --- /dev/null +++ b/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/organization/admin/OrganizationAccountTest.java @@ -0,0 +1,105 @@ +/* + * Copyright 2024 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.testsuite.organization.admin; + +import java.io.IOException; +import java.util.SortedSet; + +import com.fasterxml.jackson.core.type.TypeReference; +import jakarta.ws.rs.core.Response.Status; +import org.apache.http.impl.client.CloseableHttpClient; +import org.apache.http.impl.client.HttpClientBuilder; +import org.junit.After; +import org.junit.Assert; +import org.junit.Before; +import org.junit.Rule; +import org.junit.Test; +import org.keycloak.admin.client.resource.OrganizationResource; +import org.keycloak.broker.provider.util.SimpleHttp.Response; +import org.keycloak.common.Profile.Feature; +import org.keycloak.representations.account.LinkedAccountRepresentation; +import org.keycloak.representations.idm.ErrorRepresentation; +import org.keycloak.representations.idm.UserRepresentation; +import org.keycloak.testsuite.admin.ApiUtil; +import org.keycloak.testsuite.arquillian.annotation.EnableFeature; +import org.keycloak.testsuite.broker.util.SimpleHttpDefault; +import org.keycloak.testsuite.util.TokenUtil; + +@EnableFeature(Feature.ORGANIZATION) +public class OrganizationAccountTest extends AbstractOrganizationTest { + + @Rule + public TokenUtil tokenUtil = new TokenUtil(bc.getUserEmail(), bc.getUserPassword()); + + private CloseableHttpClient client; + + @Before + public void before() { + client = HttpClientBuilder.create().build(); + } + + @After + public void after() { + try { + client.close(); + } catch (IOException e) { + throw new RuntimeException(e); + } + } + + @Test + public void testFailUnlinkIdentityProvider() throws IOException { + // federate user + OrganizationResource organization = testRealm().organizations().get(createOrganization().getId()); + assertBrokerRegistration(organization, bc.getUserEmail()); + // reset password to obtain a token and access the account api + UserRepresentation user = ApiUtil.findUserByUsername(realmsResouce().realm(bc.consumerRealmName()), bc.getUserLogin()); + ApiUtil.resetUserPassword(realmsResouce().realm(bc.consumerRealmName()).users().get(user.getId()), bc.getUserPassword(), false); + + LinkedAccountRepresentation link = findLinkedAccount(bc.getIDPAlias()); + Assert.assertNotNull(link); + try (Response response = SimpleHttpDefault.doDelete(getAccountUrl("linked-accounts/" + link.getProviderAlias()), client).auth(tokenUtil.getToken()).acceptJson().asResponse()) { + Assert.assertEquals(Status.BAD_REQUEST.getStatusCode(), response.getStatus()); + ErrorRepresentation error = response.asJson(ErrorRepresentation.class); + Assert.assertEquals("You cannot remove the link to an identity provider associated with an organization.", error.getErrorMessage()); + } + + // broker no longer linked to the organization + organization.identityProviders().get(bc.getIDPAlias()).delete().close(); + try (Response response = SimpleHttpDefault.doDelete(getAccountUrl("linked-accounts/" + link.getProviderAlias()), client).auth(tokenUtil.getToken()).acceptJson().asResponse()) { + Assert.assertEquals(Status.NO_CONTENT.getStatusCode(), response.getStatus()); + } + } + + private SortedSet linkedAccountsRep() throws IOException { + return SimpleHttpDefault.doGet(getAccountUrl("linked-accounts"), client).auth(tokenUtil.getToken()) + .asJson(new TypeReference<>() {}); + } + + private String getAccountUrl(String resource) { + return suiteContext.getAuthServerInfo().getContextRoot().toString() + "/auth/realms/test/account" + (resource != null ? "/" + resource : ""); + } + + private LinkedAccountRepresentation findLinkedAccount(String providerAlias) throws IOException { + for (LinkedAccountRepresentation account : linkedAccountsRep()) { + if (account.getProviderAlias().equals(providerAlias)) return account; + } + + return null; + } +} diff --git a/themes/src/main/resources/theme/base/account/messages/messages_en.properties b/themes/src/main/resources/theme/base/account/messages/messages_en.properties index fb6d3cb0e40b..bd4f3e66f759 100755 --- a/themes/src/main/resources/theme/base/account/messages/messages_en.properties +++ b/themes/src/main/resources/theme/base/account/messages/messages_en.properties @@ -213,6 +213,7 @@ invalidFederatedIdentityActionMessage=Invalid or missing action. identityProviderNotFoundMessage=Specified identity provider not found. federatedIdentityLinkNotActiveMessage=This identity is not active anymore. federatedIdentityRemovingLastProviderMessage=You can''t remove last federated identity as you don''t have a password. +federatedIdentityBoundOrganization=You cannot remove the link to an identity provider associated with an organization. identityProviderRedirectErrorMessage=Failed to redirect to identity provider. identityProviderRemovedMessage=Identity provider removed successfully. identityProviderAlreadyLinkedMessage=Federated identity returned by {0} is already linked to another user.