diff --git a/core/src/main/java/org/keycloak/representations/idm/IdentityProviderRepresentation.java b/core/src/main/java/org/keycloak/representations/idm/IdentityProviderRepresentation.java index 9ca5a0d23b..5d6c658614 100755 --- a/core/src/main/java/org/keycloak/representations/idm/IdentityProviderRepresentation.java +++ b/core/src/main/java/org/keycloak/representations/idm/IdentityProviderRepresentation.java @@ -41,7 +41,7 @@ public class IdentityProviderRepresentation { *
  • missing - update profile page is presented for users with missing some of mandatory user profile fields *
  • off - update profile page is newer shown after first login * - * + * * @see #UPFLM_ON * @see #UPFLM_MISSING * @see #UPFLM_OFF @@ -54,8 +54,10 @@ public class IdentityProviderRepresentation { protected boolean addReadTokenRoleOnCreate; protected boolean authenticateByDefault; protected boolean linkOnly; + protected boolean hideOnLogin; protected String firstBrokerLoginFlowAlias; protected String postBrokerLoginFlowAlias; + protected String organizationId; protected Map config = new HashMap<>(); public String getInternalId() { @@ -106,10 +108,18 @@ public class IdentityProviderRepresentation { this.linkOnly = linkOnly; } + public boolean isHideOnLogin() { + return this.hideOnLogin; + } + + public void setHideOnLogin(boolean hideOnLogin) { + this.hideOnLogin = hideOnLogin; + } + /** - * + * * Deprecated because replaced by {@link #updateProfileFirstLoginMode}. Kept here to allow import of old realms. - * + * * @deprecated {@link #setUpdateProfileFirstLoginMode(String)} */ @Deprecated @@ -194,4 +204,12 @@ public class IdentityProviderRepresentation { this.displayName = displayName; } + public String getOrganizationId() { + return this.organizationId; + } + + public void setOrganizationId(String organizationId) { + this.organizationId = organizationId; + } + } diff --git a/docs/documentation/upgrading/topics/changes/changes-26_0_0.adoc b/docs/documentation/upgrading/topics/changes/changes-26_0_0.adoc index d7f507213c..edac8284c4 100644 --- a/docs/documentation/upgrading/topics/changes/changes-26_0_0.adoc +++ b/docs/documentation/upgrading/topics/changes/changes-26_0_0.adoc @@ -147,3 +147,11 @@ If you wish to disable placeholder replacement for the `import` command, add the You can still override automatic detection by specifying the `https-key-store-type` and `https-trust-store-type` explicitly. The same applies to the management interface and its `https-management-key-store-type`. Restrictions for the FIPS strict mode stay unchanged. NOTE: The `+spi-truststore-file-*+` options and the truststore related options `+https-trust-store-*+` are deprecated, we strongly recommend to use System Truststore. For more details refer to the relevant https://www.keycloak.org/server/keycloak-truststore[guide]. + += Improving performance for selection of identity providers + +New indexes were added to the `IDENTITY_PROVIDER` table to improve the performance of queries that fetch the IDPs associated with an organization, and fetch IDPs that are available for login (those that are `enabled`, not `link_only`, not marked as `hide_on_login`). + +If the table currently contains more than 300.000 entries, +{project_name} will skip the creation of the indexes by default during the automatic schema migration, and will instead log the SQL statements +on the console during migration. In this case, the statements must be run manually in the DB after {project_name}'s startup. diff --git a/js/apps/admin-ui/src/identity-providers/IdentityProvidersSection.tsx b/js/apps/admin-ui/src/identity-providers/IdentityProvidersSection.tsx index c83adc512d..8f6185005e 100644 --- a/js/apps/admin-ui/src/identity-providers/IdentityProvidersSection.tsx +++ b/js/apps/admin-ui/src/identity-providers/IdentityProvidersSection.tsx @@ -76,7 +76,7 @@ const OrganizationLink = (identityProvider: IdentityProviderRepresentation) => { const { t } = useTranslation(); const { realm } = useRealm(); - if (!identityProvider.config?.["kc.org"]) { + if (!identityProvider?.organizationId) { return "—"; } @@ -85,7 +85,7 @@ const OrganizationLink = (identityProvider: IdentityProviderRepresentation) => { key={identityProvider.providerId} to={toEditOrganization({ realm, - id: identityProvider.config["kc.org"], + id: identityProvider.organizationId, tab: "identityProviders", })} > @@ -299,7 +299,7 @@ export default function IdentityProvidersSection() { cellFormatters: [upperCaseFormatter()], }, { - name: "config['kc.org']", + name: "organizationId", displayKey: "linkedOrganization", cellRenderer: OrganizationLink, }, diff --git a/js/apps/admin-ui/src/identity-providers/add/AdvancedSettings.tsx b/js/apps/admin-ui/src/identity-providers/add/AdvancedSettings.tsx index 668b31f001..3f3332f5bc 100644 --- a/js/apps/admin-ui/src/identity-providers/add/AdvancedSettings.tsx +++ b/js/apps/admin-ui/src/identity-providers/add/AdvancedSettings.tsx @@ -150,7 +150,11 @@ export const AdvancedSettings = ({ isOIDC, isSAML }: AdvancedSettingsProps) => { label="accountLinkingOnly" fieldType="boolean" /> - + {(!isSAML || isOIDC) && ( diff --git a/js/libs/keycloak-admin-client/src/defs/identityProviderRepresentation.ts b/js/libs/keycloak-admin-client/src/defs/identityProviderRepresentation.ts index 7e8ba1dca6..cef88107f4 100644 --- a/js/libs/keycloak-admin-client/src/defs/identityProviderRepresentation.ts +++ b/js/libs/keycloak-admin-client/src/defs/identityProviderRepresentation.ts @@ -11,8 +11,10 @@ export default interface IdentityProviderRepresentation { firstBrokerLoginFlowAlias?: string; internalId?: string; linkOnly?: boolean; + hideOnLogin?: boolean; postBrokerLoginFlowAlias?: string; providerId?: string; storeToken?: boolean; trustEmail?: boolean; + organizationId?: string; } diff --git a/model/infinispan/src/main/java/org/keycloak/models/cache/infinispan/idp/InfinispanIDPProvider.java b/model/infinispan/src/main/java/org/keycloak/models/cache/infinispan/idp/InfinispanIDPProvider.java index c976ed5cba..32faf4ffff 100644 --- a/model/infinispan/src/main/java/org/keycloak/models/cache/infinispan/idp/InfinispanIDPProvider.java +++ b/model/infinispan/src/main/java/org/keycloak/models/cache/infinispan/idp/InfinispanIDPProvider.java @@ -143,7 +143,7 @@ public class InfinispanIDPProvider implements IDPProvider { } @Override - public Stream getAllStream(Map attrs, Integer first, Integer max) { + public Stream getAllStream(Map attrs, Integer first, Integer max) { return idpDelegate.getAllStream(attrs, first, max); } diff --git a/model/infinispan/src/main/java/org/keycloak/models/cache/infinispan/organization/InfinispanOrganizationProviderFactory.java b/model/infinispan/src/main/java/org/keycloak/models/cache/infinispan/organization/InfinispanOrganizationProviderFactory.java index 8750df6b3d..646db79c6a 100644 --- a/model/infinispan/src/main/java/org/keycloak/models/cache/infinispan/organization/InfinispanOrganizationProviderFactory.java +++ b/model/infinispan/src/main/java/org/keycloak/models/cache/infinispan/organization/InfinispanOrganizationProviderFactory.java @@ -58,10 +58,10 @@ public class InfinispanOrganizationProviderFactory implements OrganizationProvid } private void registerOrganizationInvalidation(KeycloakSession session, IdentityProviderModel idp) { - if (idp.getConfig().get(OrganizationModel.ORGANIZATION_ATTRIBUTE) != null) { + if (idp.getOrganizationId() != null) { InfinispanOrganizationProvider orgProvider = (InfinispanOrganizationProvider) session.getProvider(OrganizationProvider.class, getId()); if (orgProvider != null) { - OrganizationModel organization = orgProvider.getById(idp.getConfig().get(OrganizationModel.ORGANIZATION_ATTRIBUTE)); + OrganizationModel organization = orgProvider.getById(idp.getOrganizationId()); orgProvider.registerOrganizationInvalidation(organization); } } diff --git a/model/jpa/src/main/java/org/keycloak/connections/jpa/updater/liquibase/custom/JpaUpdate26_0_0_IdentityProviderAttributesMigration.java b/model/jpa/src/main/java/org/keycloak/connections/jpa/updater/liquibase/custom/JpaUpdate26_0_0_IdentityProviderAttributesMigration.java new file mode 100644 index 0000000000..4398e11193 --- /dev/null +++ b/model/jpa/src/main/java/org/keycloak/connections/jpa/updater/liquibase/custom/JpaUpdate26_0_0_IdentityProviderAttributesMigration.java @@ -0,0 +1,85 @@ +/* + * 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.connections.jpa.updater.liquibase.custom; + +import java.sql.PreparedStatement; +import java.sql.ResultSet; + +import liquibase.exception.CustomChangeException; +import liquibase.statement.core.DeleteStatement; +import liquibase.statement.core.UpdateStatement; +import liquibase.structure.core.Table; +import org.keycloak.models.IdentityProviderModel; +import org.keycloak.models.OrganizationModel; + +/** + * Custom SQL change to migrate the organization ID and the hide on login page config from the IDP config table to the + * IDP table. + */ +public class JpaUpdate26_0_0_IdentityProviderAttributesMigration extends CustomKeycloakTask { + + @Override + protected void generateStatementsImpl() throws CustomChangeException { + + // move the organization id from the config to the IDP. + try (PreparedStatement ps = connection.prepareStatement("SELECT c.IDENTITY_PROVIDER_ID, c.VALUE" + + " FROM " + getTableName("IDENTITY_PROVIDER_CONFIG") + " c WHERE c.NAME = '" + OrganizationModel.ORGANIZATION_ATTRIBUTE + "'"); + ResultSet resultSet = ps.executeQuery() + ) { + while (resultSet.next()) { + String id = resultSet.getString(1); + String value = resultSet.getString(2); + statements.add(new UpdateStatement(null, null, database.correctObjectName("IDENTITY_PROVIDER", Table.class)) + .addNewColumnValue("ORGANIZATION_ID", value) + .setWhereClause("INTERNAL_ID=?") + .addWhereParameter(id)); + } + statements.add(new DeleteStatement(null, null, database.correctObjectName("IDENTITY_PROVIDER_CONFIG", Table.class)) + .setWhere("NAME=?") + .addWhereParameter(OrganizationModel.ORGANIZATION_ATTRIBUTE)); + + } catch (Exception e) { + throw new CustomChangeException(getTaskId() + ": Exception when updating data from previous version", e); + } + + // move hide on login page from the config to the IDP. + try (PreparedStatement ps = connection.prepareStatement("SELECT c.IDENTITY_PROVIDER_ID, c.VALUE" + + " FROM " + getTableName("IDENTITY_PROVIDER_CONFIG") + " c WHERE c.NAME = '" + IdentityProviderModel.LEGACY_HIDE_ON_LOGIN_ATTR + "'"); + ResultSet resultSet = ps.executeQuery() + ) { + while (resultSet.next()) { + String id = resultSet.getString(1); + String value = resultSet.getString(2); + statements.add(new UpdateStatement(null, null, database.correctObjectName("IDENTITY_PROVIDER", Table.class)) + .addNewColumnValue("HIDE_ON_LOGIN", Boolean.parseBoolean(value)) + .setWhereClause("INTERNAL_ID=?") + .addWhereParameter(id)); + } + statements.add(new DeleteStatement(null, null, database.correctObjectName("IDENTITY_PROVIDER_CONFIG", Table.class)) + .setWhere("NAME=?") + .addWhereParameter(IdentityProviderModel.LEGACY_HIDE_ON_LOGIN_ATTR)); + + } catch (Exception e) { + throw new CustomChangeException(getTaskId() + ": Exception when updating data from previous version", e); + } + } + + @Override + protected String getTaskId() { + return "Migrate kc.org and hideOnLoginPage from the IDP config to the IDP itself"; + } +} diff --git a/model/jpa/src/main/java/org/keycloak/models/jpa/JpaIDPProvider.java b/model/jpa/src/main/java/org/keycloak/models/jpa/JpaIDPProvider.java index 79564d4aeb..eff7b38814 100644 --- a/model/jpa/src/main/java/org/keycloak/models/jpa/JpaIDPProvider.java +++ b/model/jpa/src/main/java/org/keycloak/models/jpa/JpaIDPProvider.java @@ -31,7 +31,6 @@ import jakarta.persistence.criteria.CriteriaQuery; import jakarta.persistence.criteria.MapJoin; import jakarta.persistence.criteria.Predicate; import jakarta.persistence.criteria.Root; -import org.apache.commons.lang3.StringUtils; import org.hibernate.Session; import org.jboss.logging.Logger; import org.keycloak.broker.provider.IdentityProvider; @@ -51,6 +50,9 @@ import static org.keycloak.models.IdentityProviderModel.ALIAS; import static org.keycloak.models.IdentityProviderModel.AUTHENTICATE_BY_DEFAULT; import static org.keycloak.models.IdentityProviderModel.ENABLED; import static org.keycloak.models.IdentityProviderModel.FIRST_BROKER_LOGIN_FLOW_ID; +import static org.keycloak.models.IdentityProviderModel.HIDE_ON_LOGIN; +import static org.keycloak.models.IdentityProviderModel.LINK_ONLY; +import static org.keycloak.models.IdentityProviderModel.ORGANIZATION_ID; import static org.keycloak.models.IdentityProviderModel.POST_BROKER_LOGIN_FLOW_ID; import static org.keycloak.models.jpa.PaginationUtils.paginateQuery; import static org.keycloak.utils.StreamsUtil.closing; @@ -92,8 +94,10 @@ public class JpaIDPProvider implements IDPProvider { entity.setAuthenticateByDefault(identityProvider.isAuthenticateByDefault()); entity.setFirstBrokerLoginFlowId(identityProvider.getFirstBrokerLoginFlowId()); entity.setPostBrokerLoginFlowId(identityProvider.getPostBrokerLoginFlowId()); + entity.setOrganizationId(identityProvider.getOrganizationId()); entity.setConfig(identityProvider.getConfig()); entity.setLinkOnly(identityProvider.isLinkOnly()); + entity.setHideOnLogin(identityProvider.isHideOnLogin()); em.persist(entity); // flush so that constraint violations are flagged and converted into model exception now rather than at the end of the tx. em.flush(); @@ -113,10 +117,12 @@ public class JpaIDPProvider implements IDPProvider { entity.setAuthenticateByDefault(identityProvider.isAuthenticateByDefault()); entity.setFirstBrokerLoginFlowId(identityProvider.getFirstBrokerLoginFlowId()); entity.setPostBrokerLoginFlowId(identityProvider.getPostBrokerLoginFlowId()); + entity.setOrganizationId(identityProvider.getOrganizationId()); entity.setAddReadTokenRoleOnCreate(identityProvider.isAddReadTokenRoleOnCreate()); entity.setStoreToken(identityProvider.isStoreToken()); entity.setConfig(identityProvider.getConfig()); entity.setLinkOnly(identityProvider.isLinkOnly()); + entity.setHideOnLogin(identityProvider.isHideOnLogin()); // flush so that constraint violations are flagged and converted into model exception now rather than at the end of the tx. em.flush(); @@ -148,7 +154,7 @@ public class JpaIDPProvider implements IDPProvider { IdentityProviderEntity entity = this.getEntityByAlias(alias); if (entity != null) { - //call toModel(entity) now as after em.remove(entity) and the flush it might throw LazyInitializationException + //call toModel(entity) now as after em.remove(entity) and the flush it might throw LazyInitializationException //when accessing the config of the entity (entity.getConfig()) withing the toModel(entity) IdentityProviderModel model = toModel(entity); @@ -218,7 +224,7 @@ public class JpaIDPProvider implements IDPProvider { } @Override - public Stream getAllStream(Map attrs, Integer first, Integer max) { + public Stream getAllStream(Map attrs, Integer first, Integer max) { CriteriaBuilder builder = em.getCriteriaBuilder(); CriteriaQuery query = builder.createQuery(IdentityProviderEntity.class); Root idp = query.from(IdentityProviderEntity.class); @@ -227,19 +233,27 @@ public class JpaIDPProvider implements IDPProvider { predicates.add(builder.equal(idp.get("realmId"), getRealm().getId())); if (attrs != null) { - for (Map.Entry entry : attrs.entrySet()) { + for (Map.Entry entry : attrs.entrySet()) { String key = entry.getKey(); - String value = entry.getValue(); + Object value = entry.getValue(); if (StringUtil.isBlank(key)) { continue; } switch(key) { + case AUTHENTICATE_BY_DEFAULT: case ENABLED: - case AUTHENTICATE_BY_DEFAULT: { - predicates.add(builder.equal(idp.get(key), Boolean.valueOf(value))); + case HIDE_ON_LOGIN: + case LINK_ONLY: { + if (Boolean.parseBoolean(value.toString())) { + predicates.add(builder.isTrue(idp.get(key))); + } else { + predicates.add(builder.isFalse(idp.get(key))); + } break; - } case FIRST_BROKER_LOGIN_FLOW_ID: { - if (StringUtils.isBlank(value)) { + } + case FIRST_BROKER_LOGIN_FLOW_ID: + case ORGANIZATION_ID: { + if (value == null || value.toString().isEmpty()) { predicates.add(builder.isNull(idp.get(key))); } else { predicates.add(builder.equal(idp.get(key), value)); @@ -377,10 +391,12 @@ public class JpaIDPProvider implements IDPProvider { identityProviderModel.setConfig(config); identityProviderModel.setEnabled(entity.isEnabled()); identityProviderModel.setLinkOnly(entity.isLinkOnly()); + identityProviderModel.setHideOnLogin(entity.isHideOnLogin()); identityProviderModel.setTrustEmail(entity.isTrustEmail()); identityProviderModel.setAuthenticateByDefault(entity.isAuthenticateByDefault()); identityProviderModel.setFirstBrokerLoginFlowId(entity.getFirstBrokerLoginFlowId()); identityProviderModel.setPostBrokerLoginFlowId(entity.getPostBrokerLoginFlowId()); + identityProviderModel.setOrganizationId(entity.getOrganizationId()); identityProviderModel.setStoreToken(entity.isStoreToken()); identityProviderModel.setAddReadTokenRoleOnCreate(entity.isAddReadTokenRoleOnCreate()); diff --git a/model/jpa/src/main/java/org/keycloak/models/jpa/entities/IdentityProviderEntity.java b/model/jpa/src/main/java/org/keycloak/models/jpa/entities/IdentityProviderEntity.java index 4793aa3fd4..640ff4e30c 100755 --- a/model/jpa/src/main/java/org/keycloak/models/jpa/entities/IdentityProviderEntity.java +++ b/model/jpa/src/main/java/org/keycloak/models/jpa/entities/IdentityProviderEntity.java @@ -65,6 +65,9 @@ public class IdentityProviderEntity { @Column(name="LINK_ONLY") private boolean linkOnly; + @Column(name="HIDE_ON_LOGIN") + private boolean hideOnLogin; + @Column(name="ADD_TOKEN_ROLE") protected boolean addReadTokenRoleOnCreate; @@ -77,6 +80,9 @@ public class IdentityProviderEntity { @Column(name="POST_BROKER_LOGIN_FLOW_ID") private String postBrokerLoginFlowId; + @Column(name="ORGANIZATION_ID") + private String organizationId; + @ElementCollection @MapKeyColumn(name="NAME") @Column(name="VALUE", columnDefinition = "TEXT") @@ -163,6 +169,22 @@ public class IdentityProviderEntity { this.postBrokerLoginFlowId = postBrokerLoginFlowId; } + public String getOrganizationId() { + return this.organizationId; + } + + public void setOrganizationId(String organizationId) { + this.organizationId = organizationId; + } + + public boolean isHideOnLogin() { + return this.hideOnLogin; + } + + public void setHideOnLogin(boolean hideOnLogin) { + this.hideOnLogin = hideOnLogin; + } + public Map getConfig() { return this.config; } diff --git a/model/jpa/src/main/resources/META-INF/jpa-changelog-26.0.0.xml b/model/jpa/src/main/resources/META-INF/jpa-changelog-26.0.0.xml index ccf78d1e2b..55a6f7e770 100644 --- a/model/jpa/src/main/resources/META-INF/jpa-changelog-26.0.0.xml +++ b/model/jpa/src/main/resources/META-INF/jpa-changelog-26.0.0.xml @@ -73,4 +73,23 @@ + + + + + + + + + + + + + + + + + + + diff --git a/server-spi-private/src/main/java/org/keycloak/models/utils/ModelToRepresentation.java b/server-spi-private/src/main/java/org/keycloak/models/utils/ModelToRepresentation.java index dde2c2a9a2..2a72bf718b 100755 --- a/server-spi-private/src/main/java/org/keycloak/models/utils/ModelToRepresentation.java +++ b/server-spi-private/src/main/java/org/keycloak/models/utils/ModelToRepresentation.java @@ -819,6 +819,7 @@ public class ModelToRepresentation { IdentityProviderRepresentation providerRep = toBriefRepresentation(realm, identityProviderModel); providerRep.setLinkOnly(identityProviderModel.isLinkOnly()); + providerRep.setHideOnLogin(identityProviderModel.isHideOnLogin()); providerRep.setStoreToken(identityProviderModel.isStoreToken()); providerRep.setTrustEmail(identityProviderModel.isTrustEmail()); providerRep.setAuthenticateByDefault(identityProviderModel.isAuthenticateByDefault()); @@ -849,8 +850,8 @@ public class ModelToRepresentation { providerRep.setPostBrokerLoginFlowAlias(flow.getAlias()); } - if (export) { - providerRep.getConfig().remove(OrganizationModel.ORGANIZATION_ATTRIBUTE); + if (!export) { + providerRep.setOrganizationId(identityProviderModel.getOrganizationId()); } return providerRep; diff --git a/server-spi-private/src/main/java/org/keycloak/models/utils/RepresentationToModel.java b/server-spi-private/src/main/java/org/keycloak/models/utils/RepresentationToModel.java index ed51c4763e..6f614857ea 100755 --- a/server-spi-private/src/main/java/org/keycloak/models/utils/RepresentationToModel.java +++ b/server-spi-private/src/main/java/org/keycloak/models/utils/RepresentationToModel.java @@ -78,7 +78,6 @@ import org.keycloak.models.FederatedIdentityModel; import org.keycloak.models.GroupModel; import org.keycloak.models.IdentityProviderMapperModel; import org.keycloak.models.IdentityProviderModel; -import org.keycloak.models.KeycloakContext; import org.keycloak.models.KeycloakSession; import org.keycloak.models.ModelException; import org.keycloak.models.OrganizationDomainModel; @@ -867,15 +866,20 @@ public class RepresentationToModel { identityProviderModel.setProviderId(representation.getProviderId()); identityProviderModel.setEnabled(representation.isEnabled()); identityProviderModel.setLinkOnly(representation.isLinkOnly()); + identityProviderModel.setHideOnLogin(representation.isHideOnLogin()); + // check if the legacy hide on login attribute is present. + String hideOnLoginAttr = representation.getConfig().remove(IdentityProviderModel.LEGACY_HIDE_ON_LOGIN_ATTR); + if (hideOnLoginAttr != null) identityProviderModel.setHideOnLogin(Boolean.parseBoolean(hideOnLoginAttr)); identityProviderModel.setTrustEmail(representation.isTrustEmail()); identityProviderModel.setAuthenticateByDefault(representation.isAuthenticateByDefault()); identityProviderModel.setStoreToken(representation.isStoreToken()); identityProviderModel.setAddReadTokenRoleOnCreate(representation.isAddReadTokenRoleOnCreate()); - updateOrganizationBroker(realm, representation, session); + updateOrganizationBroker(representation, session); + identityProviderModel.setOrganizationId(representation.getOrganizationId()); identityProviderModel.setConfig(removeEmptyString(representation.getConfig())); String flowAlias = representation.getFirstBrokerLoginFlowAlias(); - if (flowAlias == null || flowAlias.trim().length() == 0) { + if (flowAlias == null || flowAlias.trim().isEmpty()) { identityProviderModel.setFirstBrokerLoginFlowId(null); } else { AuthenticationFlowModel flowModel = realm.getFlowByAlias(flowAlias); @@ -886,7 +890,7 @@ public class RepresentationToModel { } flowAlias = representation.getPostBrokerLoginFlowAlias(); - if (flowAlias == null || flowAlias.trim().length() == 0) { + if (flowAlias == null || flowAlias.trim().isEmpty()) { identityProviderModel.setPostBrokerLoginFlowId(null); } else { AuthenticationFlowModel flowModel = realm.getFlowByAlias(flowAlias); @@ -1639,21 +1643,22 @@ public class RepresentationToModel { return toModel(representation, authorization, client); } - private static void updateOrganizationBroker(RealmModel realm, IdentityProviderRepresentation representation, KeycloakSession session) { + private static void updateOrganizationBroker(IdentityProviderRepresentation representation, KeycloakSession session) { if (!Profile.isFeatureEnabled(Feature.ORGANIZATION)) { return; } IdentityProviderModel existing = Optional.ofNullable(session.identityProviders().getByAlias(representation.getAlias())) .orElse(session.identityProviders().getById(representation.getInternalId())); - String orgId = existing == null ? representation.getConfig().get(OrganizationModel.ORGANIZATION_ATTRIBUTE) : existing.getOrganizationId(); + String repOrgId = representation.getOrganizationId() != null ? representation.getOrganizationId() : + representation.getConfig().remove(OrganizationModel.ORGANIZATION_ATTRIBUTE); + String orgId = existing != null ? existing.getOrganizationId() : repOrgId; if (orgId != null) { OrganizationProvider provider = session.getProvider(OrganizationProvider.class); OrganizationModel org = provider.getById(orgId); - String newOrgId = representation.getConfig().get(OrganizationModel.ORGANIZATION_ATTRIBUTE); - if (org == null || (newOrgId != null && provider.getById(newOrgId) == null)) { + if (org == null || (repOrgId != null && provider.getById(repOrgId) == null)) { throw new IllegalArgumentException("Organization associated with broker does not exist"); } @@ -1666,7 +1671,7 @@ public class RepresentationToModel { } // make sure the link to an organization does not change - representation.getConfig().put(OrganizationModel.ORGANIZATION_ATTRIBUTE, orgId); + representation.setOrganizationId(orgId); } } } diff --git a/server-spi/src/main/java/org/keycloak/models/IDPProvider.java b/server-spi/src/main/java/org/keycloak/models/IDPProvider.java index e3d5937851..f6906b0311 100644 --- a/server-spi/src/main/java/org/keycloak/models/IDPProvider.java +++ b/server-spi/src/main/java/org/keycloak/models/IDPProvider.java @@ -16,6 +16,7 @@ */ package org.keycloak.models; +import java.util.LinkedHashMap; import java.util.Map; import java.util.stream.Stream; @@ -118,7 +119,7 @@ public interface IDPProvider extends Provider { * @param max the maximum number of results to be returned. Ignored if negative or {@code null}. * @return a non-null stream of {@link IdentityProviderModel}s that match the search criteria. */ - Stream getAllStream(Map attrs, Integer first, Integer max); + Stream getAllStream(Map attrs, Integer first, Integer max); /** * Returns all identity providers associated with the organization with the provided id. @@ -129,7 +130,7 @@ public interface IDPProvider extends Provider { * @return a non-null stream of {@link IdentityProviderModel}s that match the search criteria. */ default Stream getByOrganization(String orgId, Integer first, Integer max) { - return getAllStream(Map.of(OrganizationModel.ORGANIZATION_ATTRIBUTE, orgId), first, max); + return getAllStream(Map.of(IdentityProviderModel.ORGANIZATION_ID, orgId), first, max); } /** @@ -147,6 +148,51 @@ public interface IDPProvider extends Provider { */ Stream getByFlow(String flowId, String search, Integer first, Integer max); + /** + * Returns all identity providers available for login, according to the specified mode. An IDP can be used for login + * if it is enabled, is not a link-only IDP, and is not configured to be hidden on login page. + *

    + * The mode parameter may narrow the list of IDPs that are available. {@code FETCH_MODE.REALM_ONLY} fetches only realm-level + * IDPs (i.e. those not associated with any org). {@code FETCH_MODE.ORG_ONLY} will work together with the {@code organizationId} + * parameter. If the latter is set, only the IDPs associated with that org will be returned. Otherwise, the method returns + * the IDPs associated with any org. {@code FETCH_MODE.ALL} combines both approaches, returning both the realm-level + * IDPs with those associated with organizations (or a specific organization as per the {@code organizationId} param). + * + * @param mode the fetch mode to be used. Can be {@code REALM_ONLY}, {@code ORG_ONLY}, or {@code ALL}. + * @param organizationId an optional organization ID. If present and the mode is not {@code REALM_ONLY}, the param indicates + * that only IDPs associated with the specified organization are to be returned. + * @return a non-null stream of {@link IdentityProviderModel}s that are suitable for being displayed in the login pages. + */ + default Stream getForLogin(FETCH_MODE mode, String organizationId) { + Stream result = Stream.of(); + if (mode == FETCH_MODE.REALM_ONLY || mode == FETCH_MODE.ALL) { + // fetch all realm-only IDPs - i.e. those not associated with orgs. + Map searchOptions = getBasicSearchOptionsForLogin(); + searchOptions.put(IdentityProviderModel.ORGANIZATION_ID, null); + result = Stream.concat(result, getAllStream(searchOptions, null, null)); + } + if (mode == FETCH_MODE.ORG_ONLY || mode == FETCH_MODE.ALL) { + // fetch IDPs associated with organizations. + Map searchOptions = getBasicSearchOptionsForLogin(); + if (organizationId != null) { + // we want the IDPs associated with a specific org. + searchOptions.put(IdentityProviderModel.ORGANIZATION_ID, organizationId); + } + searchOptions.put(OrganizationModel.BROKER_PUBLIC, "true"); + result = Stream.concat(result, getAllStream(searchOptions, null, null)); + } + return result; + } + + private static Map getBasicSearchOptionsForLogin() { + Map searchOptions = new LinkedHashMap<>(); + searchOptions.put(IdentityProviderModel.ENABLED, "true"); + searchOptions.put(IdentityProviderModel.LINK_ONLY, "false"); + searchOptions.put(IdentityProviderModel.HIDE_ON_LOGIN, "false"); + return searchOptions; + } + + /** * Returns the number of IDPs in the realm. * @@ -163,4 +209,6 @@ public interface IDPProvider extends Provider { default boolean isIdentityFederationEnabled() { return count() > 0; } + + enum FETCH_MODE {REALM_ONLY, ORG_ONLY, ALL} } diff --git a/server-spi/src/main/java/org/keycloak/models/IdentityProviderModel.java b/server-spi/src/main/java/org/keycloak/models/IdentityProviderModel.java index ddd8e54bfa..da6041d34f 100755 --- a/server-spi/src/main/java/org/keycloak/models/IdentityProviderModel.java +++ b/server-spi/src/main/java/org/keycloak/models/IdentityProviderModel.java @@ -41,9 +41,12 @@ public class IdentityProviderModel implements Serializable { public static final String ENABLED = "enabled"; public static final String FILTERED_BY_CLAIMS = "filteredByClaim"; public static final String FIRST_BROKER_LOGIN_FLOW_ID = "firstBrokerLoginFlowId"; - public static final String HIDE_ON_LOGIN = "hideOnLoginPage"; + public static final String HIDE_ON_LOGIN = "hideOnLogin"; + public static final String LEGACY_HIDE_ON_LOGIN_ATTR = "hideOnLoginPage"; + public static final String LINK_ONLY = "linkOnly"; public static final String LOGIN_HINT = "loginHint"; public static final String METADATA_DESCRIPTOR_URL = "metadataDescriptorUrl"; + public static final String ORGANIZATION_ID = "organizationId"; public static final String PASS_MAX_AGE = "passMaxAge"; public static final String POST_BROKER_LOGIN_FLOW_ID = "postBrokerLoginFlowId"; public static final String SYNC_MODE = "syncMode"; @@ -80,11 +83,13 @@ public class IdentityProviderModel implements Serializable { private String postBrokerLoginFlowId; + private String organizationId; + private String displayName; private String displayIconClasses; - private IdentityProviderSyncMode syncMode; + private boolean hideOnLogin; /** *

    A map containing the configuration and properties for a specific identity provider instance and implementation. The items @@ -110,7 +115,9 @@ public class IdentityProviderModel implements Serializable { this.addReadTokenRoleOnCreate = model.addReadTokenRoleOnCreate; this.firstBrokerLoginFlowId = model.getFirstBrokerLoginFlowId(); this.postBrokerLoginFlowId = model.getPostBrokerLoginFlowId(); + this.organizationId = model.getOrganizationId(); this.displayIconClasses = model.getDisplayIconClasses(); + this.hideOnLogin = model.isHideOnLogin(); } } @@ -225,11 +232,11 @@ public class IdentityProviderModel implements Serializable { } public String getOrganizationId() { - return getConfig().get(OrganizationModel.ORGANIZATION_ATTRIBUTE); + return this.organizationId; } public void setOrganizationId(String organizationId) { - getConfig().put(OrganizationModel.ORGANIZATION_ATTRIBUTE, organizationId); + this.organizationId = organizationId; } /** @@ -268,11 +275,11 @@ public class IdentityProviderModel implements Serializable { public boolean isHideOnLogin() { - return Boolean.valueOf(getConfig().get(HIDE_ON_LOGIN)); + return this.hideOnLogin; } public void setHideOnLogin(boolean hideOnLogin) { - getConfig().put(HIDE_ON_LOGIN, String.valueOf(hideOnLogin)); + this.hideOnLogin = hideOnLogin; } /** diff --git a/services/src/main/java/org/keycloak/authentication/authenticators/browser/UsernameForm.java b/services/src/main/java/org/keycloak/authentication/authenticators/browser/UsernameForm.java index 635db3157a..160ff86339 100755 --- a/services/src/main/java/org/keycloak/authentication/authenticators/browser/UsernameForm.java +++ b/services/src/main/java/org/keycloak/authentication/authenticators/browser/UsernameForm.java @@ -17,16 +17,20 @@ package org.keycloak.authentication.authenticators.browser; -import java.util.stream.Stream; +import java.util.Objects; import org.keycloak.authentication.AuthenticationFlowContext; +import org.keycloak.authentication.authenticators.broker.AbstractIdpAuthenticator; +import org.keycloak.authentication.authenticators.broker.util.SerializedBrokeredIdentityContext; import org.keycloak.forms.login.LoginFormsProvider; -import org.keycloak.forms.login.freemarker.LoginFormsUtil; import org.keycloak.models.IdentityProviderModel; +import org.keycloak.models.KeycloakSession; +import org.keycloak.models.UserModel; import org.keycloak.services.messages.Messages; import jakarta.ws.rs.core.MultivaluedMap; import jakarta.ws.rs.core.Response; +import org.keycloak.sessions.AuthenticationSessionModel; public final class UsernameForm extends UsernamePasswordForm { @@ -34,9 +38,7 @@ public final class UsernameForm extends UsernamePasswordForm { public void authenticate(AuthenticationFlowContext context) { if (context.getUser() != null) { // We can skip the form when user is re-authenticating. Unless current user has some IDP set, so he can re-authenticate with that IDP - Stream identityProviders = LoginFormsUtil - .filterIdentityProviders(context.getRealm().getIdentityProvidersStream(), context.getSession(), context); - if (identityProviders.findAny().isEmpty()) { + if (!this.contextUserHasFederatedIDPs(context)) { context.success(); return; } @@ -69,4 +71,28 @@ public final class UsernameForm extends UsernamePasswordForm { return Messages.INVALID_USERNAME_OR_EMAIL; return Messages.INVALID_USERNAME; } + + /** + * Checks if the context user, if it has been set, is currently linked to any IDPs they could use to authenticate. + * If the auth session has an existing IDP in the brokered context, it is filtered out. + * + * @param context a reference to the {@link AuthenticationFlowContext} + * @return {@code true} if the context user has federated IDPs that can be used for authentication; {@code false} otherwise. + */ + private boolean contextUserHasFederatedIDPs(AuthenticationFlowContext context) { + KeycloakSession session = context.getSession(); + UserModel user = context.getUser(); + if (user == null) { + return false; + } + AuthenticationSessionModel authSession = context.getAuthenticationSession(); + SerializedBrokeredIdentityContext serializedCtx = SerializedBrokeredIdentityContext.readFromAuthenticationSession(authSession, AbstractIdpAuthenticator.BROKERED_CONTEXT_NOTE); + final IdentityProviderModel existingIdp = (serializedCtx == null) ? null : serializedCtx.deserialize(session, authSession).getIdpConfig(); + + return session.users().getFederatedIdentitiesStream(session.getContext().getRealm(), user) + .map(fedIdentity -> session.identityProviders().getByAlias(fedIdentity.getIdentityProvider())) + .filter(Objects::nonNull) + .anyMatch(idpModel -> existingIdp == null || !Objects.equals(existingIdp.getAlias(), idpModel.getAlias())); + + } } diff --git a/services/src/main/java/org/keycloak/broker/saml/SAMLIdentityProviderFactory.java b/services/src/main/java/org/keycloak/broker/saml/SAMLIdentityProviderFactory.java index 66d7b500cb..cd919920b7 100755 --- a/services/src/main/java/org/keycloak/broker/saml/SAMLIdentityProviderFactory.java +++ b/services/src/main/java/org/keycloak/broker/saml/SAMLIdentityProviderFactory.java @@ -160,7 +160,7 @@ public class SAMLIdentityProviderFactory extends AbstractIdentityProviderFactory for (AttributeType attribute : entityType.getExtensions().getEntityAttributes().getAttribute()) { if (MACEDIR_ENTITY_CATEGORY.equals(attribute.getName()) && attribute.getAttributeValue().contains(REFEDS_HIDE_FROM_DISCOVERY)) { - samlIdentityProviderConfig.setHideOnLogin(true); + samlIdentityProviderConfig.getConfig().put(IdentityProviderModel.LEGACY_HIDE_ON_LOGIN_ATTR, Boolean.TRUE.toString()); } } diff --git a/services/src/main/java/org/keycloak/forms/login/freemarker/FreeMarkerLoginFormsProvider.java b/services/src/main/java/org/keycloak/forms/login/freemarker/FreeMarkerLoginFormsProvider.java index 4f1d60bd64..3902965cdf 100755 --- a/services/src/main/java/org/keycloak/forms/login/freemarker/FreeMarkerLoginFormsProvider.java +++ b/services/src/main/java/org/keycloak/forms/login/freemarker/FreeMarkerLoginFormsProvider.java @@ -62,7 +62,6 @@ import org.keycloak.forms.login.freemarker.model.VerifyProfileBean; import org.keycloak.forms.login.freemarker.model.X509ConfirmBean; import org.keycloak.models.ClientModel; import org.keycloak.models.Constants; -import org.keycloak.models.IdentityProviderModel; import org.keycloak.models.KeycloakSession; import org.keycloak.models.OrganizationModel; import org.keycloak.models.RealmModel; @@ -101,7 +100,6 @@ import java.util.Objects; import java.util.Optional; import java.util.Properties; import java.util.function.Function; -import java.util.stream.Stream; import static org.keycloak.models.UserModel.RequiredAction.UPDATE_PASSWORD; import static org.keycloak.organization.utils.Organizations.resolveOrganization; @@ -480,12 +478,10 @@ public class FreeMarkerLoginFormsProvider implements LoginFormsProvider { if (realm != null) { attributes.put("realm", new RealmBean(realm)); - Stream identityProviders = LoginFormsUtil - .filterIdentityProvidersForTheme(realm.getIdentityProvidersStream(), session, context); - IdentityProviderBean idpBean = new IdentityProviderBean(realm, session, identityProviders, baseUriWithCodeAndClientId); + IdentityProviderBean idpBean = new IdentityProviderBean(session, realm, baseUriWithCodeAndClientId, context); - if (Profile.isFeatureEnabled(Feature.ORGANIZATION)) { - idpBean = new OrganizationAwareIdentityProviderBean(idpBean, session); + if (Profile.isFeatureEnabled(Feature.ORGANIZATION) && realm.isOrganizationsEnabled()) { + idpBean = new OrganizationAwareIdentityProviderBean(idpBean); } attributes.put("social", idpBean); diff --git a/services/src/main/java/org/keycloak/forms/login/freemarker/LoginFormsUtil.java b/services/src/main/java/org/keycloak/forms/login/freemarker/LoginFormsUtil.java deleted file mode 100755 index 24aaacd677..0000000000 --- a/services/src/main/java/org/keycloak/forms/login/freemarker/LoginFormsUtil.java +++ /dev/null @@ -1,115 +0,0 @@ -/* - * Copyright 2016 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.forms.login.freemarker; - -import org.keycloak.authentication.AuthenticationFlowContext; -import org.keycloak.authentication.AuthenticationProcessor; -import org.keycloak.authentication.authenticators.broker.AbstractIdpAuthenticator; -import org.keycloak.authentication.authenticators.broker.util.SerializedBrokeredIdentityContext; -import org.keycloak.common.Profile; -import org.keycloak.common.Profile.Feature; -import org.keycloak.models.FederatedIdentityModel; -import org.keycloak.models.IdentityProviderModel; -import org.keycloak.models.KeycloakSession; -import org.keycloak.models.UserModel; -import org.keycloak.services.resources.LoginActionsService; -import org.keycloak.sessions.AuthenticationSessionModel; - -import java.util.Objects; -import java.util.Set; -import java.util.stream.Collectors; -import java.util.stream.Stream; - -/** - * Various util methods, so the logic is not hardcoded in freemarker beans - * - * @author Marek Posolda - */ -public class LoginFormsUtil { - - public static Stream filterIdentityProvidersForTheme(Stream providers, KeycloakSession session, AuthenticationFlowContext context) { - if (context != null) { - AuthenticationSessionModel authSession = context.getAuthenticationSession(); - String currentFlowPath = authSession.getAuthNote(AuthenticationProcessor.CURRENT_FLOW_PATH); - UserModel currentUser = context.getUser(); - // Fixing #14173 - // If the current user is not null, then it's a re-auth, and we should filter the possible options with the pre-14173 logic - // If the current user is null, then it's one of the following cases: - // - either connecting a new IdP to the user's account. - // - in this case the currentUser is null AND the current flow is the FIRST_BROKER_LOGIN_PATH - // - so we should filter out the one they just used for login, as they need to re-auth themselves with an already linked IdP account - // - or we're on the Login page - // - in this case the current user is null AND the current flow is NOT the FIRST_BROKER_LOGIN_PATH - // - so we should show all the possible IdPs to the user trying to log in (this is the bug in #14173) - // - so we're skipping this branch, and returning everything at the end of the method - if (currentUser != null || Objects.equals(LoginActionsService.FIRST_BROKER_LOGIN_PATH, currentFlowPath)) { - return filterIdentityProviders(providers, session, context); - } - } - return filterOrganizationBrokers(providers); - } - - public static Stream filterIdentityProviders(Stream providers, KeycloakSession session, AuthenticationFlowContext context) { - - if (context != null) { - AuthenticationSessionModel authSession = context.getAuthenticationSession(); - SerializedBrokeredIdentityContext serializedCtx = SerializedBrokeredIdentityContext.readFromAuthenticationSession(authSession, AbstractIdpAuthenticator.BROKERED_CONTEXT_NOTE); - - final IdentityProviderModel existingIdp = (serializedCtx == null) ? null : serializedCtx.deserialize(session, authSession).getIdpConfig(); - - final Set federatedIdentities; - UserModel user = context.getUser(); - if (user != null) { - federatedIdentities = session.users().getFederatedIdentitiesStream(session.getContext().getRealm(), user) - .map(FederatedIdentityModel::getIdentityProvider) - .collect(Collectors.toSet()); - } else { - federatedIdentities = Set.of(); - } - - return filterOrganizationBrokers(providers) - .filter(p -> { // Filter current IDP during first-broker-login flow. Re-authentication with the "linked" broker should not be possible - if (existingIdp == null) return true; - return !Objects.equals(p.getAlias(), existingIdp.getAlias()); - }) - .filter(idp -> { - // user not established in authentication session, he can choose to authenticate using any broker - if (user == null) { - return true; - } - - if (federatedIdentities.isEmpty()) { - // user established but not linked to any broker, he can choose to authenticate using any organization broker - return idp.getOrganizationId() != null; - } - - // user established, we show just providers already linked to this user - return federatedIdentities.contains(idp.getAlias()); - }); - } - - return filterOrganizationBrokers(providers); - } - - private static Stream filterOrganizationBrokers(Stream providers) { - if (!Profile.isFeatureEnabled(Feature.ORGANIZATION)) { - providers = providers.filter(identityProviderModel -> identityProviderModel.getOrganizationId() == null); - } - return providers; - } -} diff --git a/services/src/main/java/org/keycloak/forms/login/freemarker/model/IdentityProviderBean.java b/services/src/main/java/org/keycloak/forms/login/freemarker/model/IdentityProviderBean.java index f689a5d5f1..357d9ad188 100755 --- a/services/src/main/java/org/keycloak/forms/login/freemarker/model/IdentityProviderBean.java +++ b/services/src/main/java/org/keycloak/forms/login/freemarker/model/IdentityProviderBean.java @@ -16,22 +16,34 @@ */ package org.keycloak.forms.login.freemarker.model; +import org.keycloak.authentication.AuthenticationFlowContext; +import org.keycloak.authentication.AuthenticationProcessor; +import org.keycloak.authentication.authenticators.broker.AbstractIdpAuthenticator; +import org.keycloak.authentication.authenticators.broker.util.SerializedBrokeredIdentityContext; +import org.keycloak.common.Profile; +import org.keycloak.models.FederatedIdentityModel; +import org.keycloak.models.IDPProvider; import org.keycloak.models.IdentityProviderModel; import org.keycloak.models.KeycloakSession; import org.keycloak.models.OrderedModel; import org.keycloak.models.RealmModel; +import org.keycloak.models.UserModel; import org.keycloak.models.utils.KeycloakModelUtils; import org.keycloak.services.Urls; +import org.keycloak.services.resources.LoginActionsService; +import org.keycloak.sessions.AuthenticationSessionModel; import org.keycloak.theme.Theme; import java.io.IOException; import java.net.URI; -import java.util.ArrayList; +import java.util.HashSet; import java.util.List; -import java.util.Map; +import java.util.Objects; import java.util.Optional; import java.util.Properties; -import java.util.stream.Stream; +import java.util.Set; +import java.util.function.Predicate; +import java.util.stream.Collectors; /** * @author Stian Thorgersen @@ -42,44 +54,63 @@ public class IdentityProviderBean { public static OrderedModel.OrderedModelComparator IDP_COMPARATOR_INSTANCE = new OrderedModel.OrderedModelComparator<>(); private static final String ICON_THEME_PREFIX = "kcLogoIdP-"; - private boolean displaySocial; - private List providers; - private RealmModel realm; - private final KeycloakSession session; + protected AuthenticationFlowContext context; + protected List providers; + protected KeycloakSession session; + protected RealmModel realm; + protected URI baseURI; - public IdentityProviderBean() { - this.session = null; - } - - public IdentityProviderBean(RealmModel realm, KeycloakSession session, Stream identityProviders, URI baseURI) { - this.realm = realm; + public IdentityProviderBean(KeycloakSession session, RealmModel realm, URI baseURI, AuthenticationFlowContext context) { this.session = session; - - List orderedList = new ArrayList<>(); - - identityProviders.forEach(identityProvider -> { - if (identityProvider.isEnabled() && !identityProvider.isLinkOnly()) { - addIdentityProvider(orderedList, realm, baseURI, identityProvider); - } - }); - - if (!orderedList.isEmpty()) { - orderedList.sort(IDP_COMPARATOR_INSTANCE); - providers = orderedList; - displaySocial = true; - } + this.realm = realm; + this.baseURI = baseURI; + this.context = context; } - private void addIdentityProvider(List orderedSet, RealmModel realm, URI baseURI, IdentityProviderModel identityProvider) { + public List getProviders() { + if (this.providers == null) { + String existingIDP = this.getExistingIDP(session, context); + Set federatedIdentities = this.getFederatedIdentities(session, realm, context); + if (federatedIdentities != null) { + this.providers = getFederatedIdentityProviders(federatedIdentities, existingIDP); + } else { + this.providers = searchForIdentityProviders(existingIDP); + } + } + return this.providers; + } + + public KeycloakSession getSession() { + return this.session; + } + + public RealmModel getRealm() { + return this.realm; + } + + public URI getBaseURI() { + return this.baseURI; + } + + public AuthenticationFlowContext getFlowContext() { + return this.context; + } + + /** + * Creates an {@link IdentityProvider} instance from the specified {@link IdentityProviderModel}. + * + * @param realm a reference to the realm. + * @param baseURI the base URI. + * @param identityProvider the {@link IdentityProviderModel} from which the freemarker {@link IdentityProvider} is + * to be built. + * @return the constructed {@link IdentityProvider}. + */ + protected IdentityProvider createIdentityProvider(RealmModel realm, URI baseURI, IdentityProviderModel identityProvider) { String loginUrl = Urls.identityProviderAuthnRequest(baseURI, identityProvider.getAlias(), realm.getName()).toString(); String displayName = KeycloakModelUtils.getIdentityProviderDisplayName(session, identityProvider); - Map config = identityProvider.getConfig(); - boolean hideOnLoginPage = config != null && Boolean.parseBoolean(config.get("hideOnLoginPage")); - if (!hideOnLoginPage) { - orderedSet.add(new IdentityProvider(identityProvider.getAlias(), - displayName, identityProvider.getProviderId(), loginUrl, - config != null ? config.get("guiOrder") : null, getLoginIconClasses(identityProvider))); - } + return new IdentityProvider(identityProvider.getAlias(), + displayName, identityProvider.getProviderId(), loginUrl, + identityProvider.getConfig().get("guiOrder"), getLoginIconClasses(identityProvider)); } // Get icon classes defined in properties of current theme with key 'kcLogoIdP-{alias}' @@ -97,12 +128,121 @@ public class IdentityProviderBean { return ""; } - public List getProviders() { - return providers; + private String getLogoIconClass(IdentityProviderModel identityProvider, Properties themeProperties) throws IOException { + String iconClass = themeProperties.getProperty(ICON_THEME_PREFIX + identityProvider.getAlias()); + + if (iconClass == null) { + return themeProperties.getProperty(ICON_THEME_PREFIX + identityProvider.getProviderId()); + } + + return iconClass; } - public boolean isDisplayInfo() { - return realm.isRegistrationAllowed() || displaySocial; + /** + * Checks if an IDP is being connected to the user's account. In this case the currentUser is {@code null} and the current flow + * is the {@code FIRST_BROKER_LOGIN_PATH}, so we should retrieve the IDP they used for login and filter it out of the list + * of IDPs that are available for login. (GHI #14173). + * + * @param session a reference to the {@link KeycloakSession}. + * @param context a reference to the {@link AuthenticationFlowContext}. + * @return the alias of the IDP used for login before linking a new IDP to the user's account (if any). + */ + protected String getExistingIDP(KeycloakSession session, AuthenticationFlowContext context) { + + String existingIDPAlias = null; + if (context != null) { + AuthenticationSessionModel authSession = context.getAuthenticationSession(); + String currentFlowPath = authSession.getAuthNote(AuthenticationProcessor.CURRENT_FLOW_PATH); + UserModel currentUser = context.getUser(); + + if (currentUser == null && Objects.equals(LoginActionsService.FIRST_BROKER_LOGIN_PATH, currentFlowPath)) { + SerializedBrokeredIdentityContext serializedCtx = SerializedBrokeredIdentityContext.readFromAuthenticationSession(authSession, AbstractIdpAuthenticator.BROKERED_CONTEXT_NOTE); + final IdentityProviderModel existingIdp = (serializedCtx == null) ? null : serializedCtx.deserialize(session, authSession).getIdpConfig(); + if (existingIdp != null) { + existingIDPAlias = existingIdp.getAlias(); + } + } + } + return existingIDPAlias; + } + + /** + * Returns the list of IDPs associated with the user's federated identities, if any. In case these IDPs exist, the login + * page should show only the IDPs already linked to the user. Returning {@code null} indicates that all public enabled IDPs + * should be available. + *

    + * Returning an empty set essentially narrows the list of available IDPs to zero, so no IDPs will be shown for login. + * + * @param session a reference to the {@link KeycloakSession}. + * @param realm a reference to the realm. + * @param context a reference to the {@link AuthenticationFlowContext}. + * @return a {@link Set} containing the aliases of the IDPs that should be available for login. An empty set indicates + * that no IDPs should be available. + */ + protected Set getFederatedIdentities(KeycloakSession session, RealmModel realm, AuthenticationFlowContext context) { + Set result = null; + if (context != null) { + UserModel currentUser = context.getUser(); + if (currentUser != null) { + Set federatedIdentities = session.users().getFederatedIdentitiesStream(session.getContext().getRealm(), currentUser) + .map(FederatedIdentityModel::getIdentityProvider) + .collect(Collectors.toSet()); + + if (!federatedIdentities.isEmpty() || organizationsDisabled(realm)) + // if orgs are enabled, we don't want to return an empty set - we want the organization IDPs to be shown if those are available. + result = new HashSet<>(federatedIdentities); + + } + } + return result; + } + + /** + * Builds and returns a list of {@link IdentityProvider} instances from the specified set of federated IDPs. The IDPs + * must be enabled, not link-only, and not set to be hidden on login page. If any IDP has an alias that matches the + * {@code existingIDP} parameter, it must be filtered out. + * + * @param federatedProviders a {@link Set} containing the aliases of the federated IDPs that should be considered for login. + * @param existingIDP the alias of the IDP that must be filtered out from the result (used when linking a new IDP to a user's account). + * @return a {@link List} containing the constructed {@link IdentityProvider}s. + */ + protected List getFederatedIdentityProviders(Set federatedProviders, String existingIDP) { + return federatedProviders.stream() + .filter(alias -> !Objects.equals(existingIDP, alias)) + .map(alias -> session.identityProviders().getByAlias(alias)) + .filter(federatedProviderPredicate()) + .map(idp -> createIdentityProvider(this.realm, this.baseURI, idp)) + .sorted(IDP_COMPARATOR_INSTANCE).toList(); + } + + /** + * Returns a predicate that can filter out IDPs associated with the current user's federated identities before those + * are converted into {@link IdentityProvider}s. Subclasses may use this as a way to further refine the IDPs that are + * to be returned. + * + * @return the custom {@link Predicate} used as a last filter before conversion into {@link IdentityProvider} + */ + protected Predicate federatedProviderPredicate() { + return idp -> Objects.nonNull(idp) && idp.isEnabled() && !idp.isLinkOnly() && !idp.isHideOnLogin(); + } + + /** + * Builds and returns a list of {@link IdentityProvider} instances that will be available for login. This method goes + * to the {@link IDPProvider} to fetch the IDPs that can be used for login (enabled, not link-only and not set to be + * hidden on login page). + * + * @param existingIDP the alias of the IDP that must be filtered out from the result (used when linking a new IDP to a user's account). + * @return a {@link List} containing the constructed {@link IdentityProvider}s. + */ + protected List searchForIdentityProviders(String existingIDP) { + return session.identityProviders().getForLogin(IDPProvider.FETCH_MODE.REALM_ONLY, null) + .filter(idp -> !Objects.equals(existingIDP, idp.getAlias())) + .map(idp -> createIdentityProvider(this.realm, this.baseURI, idp)) + .sorted(IDP_COMPARATOR_INSTANCE).toList(); + } + + private static boolean organizationsDisabled(RealmModel realm) { + return !Profile.isFeatureEnabled(Profile.Feature.ORGANIZATION) || !realm.isOrganizationsEnabled(); } public static class IdentityProvider implements OrderedModel { @@ -153,13 +293,5 @@ public class IdentityProviderBean { } } - private String getLogoIconClass(IdentityProviderModel identityProvider, Properties themeProperties) throws IOException { - String iconClass = themeProperties.getProperty(ICON_THEME_PREFIX + identityProvider.getAlias()); - if (iconClass == null) { - return themeProperties.getProperty(ICON_THEME_PREFIX + identityProvider.getProviderId()); - } - - return iconClass; - } } 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 5774699c6a..3ce59fdf8f 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 @@ -180,7 +180,7 @@ public class OrganizationAuthenticator extends IdentityProviderAuthenticator { .setAttributeMapper(attributes -> { if (hasPublicBrokers(organization)) { attributes.computeIfPresent("social", - (key, bean) -> new OrganizationAwareIdentityProviderBean((IdentityProviderBean) bean, session, true) + (key, bean) -> new OrganizationAwareIdentityProviderBean((IdentityProviderBean) bean, true) ); // do not show the self-registration link if there are public brokers available from the organization to force the user to register using a broker attributes.computeIfPresent("realm", @@ -188,7 +188,7 @@ public class OrganizationAuthenticator extends IdentityProviderAuthenticator { ); } else { attributes.computeIfPresent("social", - (key, bean) -> new OrganizationAwareIdentityProviderBean((IdentityProviderBean) bean, session, false, true) + (key, bean) -> new OrganizationAwareIdentityProviderBean((IdentityProviderBean) bean, false, true) ); } @@ -208,7 +208,7 @@ public class OrganizationAuthenticator extends IdentityProviderAuthenticator { LoginFormsProvider form = context.form() .setAttributeMapper(attributes -> { attributes.computeIfPresent("social", - (key, bean) -> new OrganizationAwareIdentityProviderBean((IdentityProviderBean) bean, session, false, true) + (key, bean) -> new OrganizationAwareIdentityProviderBean((IdentityProviderBean) bean, false, true) ); attributes.computeIfPresent("auth", (key, bean) -> new OrganizationAwareAuthenticationContextBean((AuthenticationContextBean) bean, false) diff --git a/services/src/main/java/org/keycloak/organization/forms/login/freemarker/model/OrganizationAwareIdentityProviderBean.java b/services/src/main/java/org/keycloak/organization/forms/login/freemarker/model/OrganizationAwareIdentityProviderBean.java index 07189790f6..f351382a33 100644 --- a/services/src/main/java/org/keycloak/organization/forms/login/freemarker/model/OrganizationAwareIdentityProviderBean.java +++ b/services/src/main/java/org/keycloak/organization/forms/login/freemarker/model/OrganizationAwareIdentityProviderBean.java @@ -18,73 +18,88 @@ package org.keycloak.organization.forms.login.freemarker.model; import java.util.List; -import java.util.Optional; +import java.util.Objects; import java.util.function.Predicate; import org.keycloak.forms.login.freemarker.model.IdentityProviderBean; +import org.keycloak.models.IDPProvider; import org.keycloak.models.IdentityProviderModel; -import org.keycloak.models.KeycloakSession; import org.keycloak.models.OrganizationModel; -import org.keycloak.models.RealmModel; import org.keycloak.organization.utils.Organizations; public class OrganizationAwareIdentityProviderBean extends IdentityProviderBean { - private final KeycloakSession session; - private final List providers; + private final OrganizationModel organization; + private final boolean onlyRealmBrokers; + private final boolean onlyOrganizationBrokers; - public OrganizationAwareIdentityProviderBean(IdentityProviderBean delegate, KeycloakSession session, boolean onlyOrganizationBrokers) { - this(delegate, session, onlyOrganizationBrokers, false); + public OrganizationAwareIdentityProviderBean(IdentityProviderBean delegate) { + this(delegate, false); } - public OrganizationAwareIdentityProviderBean(IdentityProviderBean delegate, KeycloakSession session, boolean onlyOrganizationBrokers, boolean onlyRealmBrokers) { - this.session = session; + public OrganizationAwareIdentityProviderBean(IdentityProviderBean delegate, boolean onlyOrganizationBrokers) { + this(delegate, onlyOrganizationBrokers, false); + } + + public OrganizationAwareIdentityProviderBean(IdentityProviderBean delegate, boolean onlyOrganizationBrokers, boolean onlyRealmBrokers) { + super(delegate.getSession(), delegate.getRealm(), delegate.getBaseURI(), delegate.getFlowContext()); + this.organization = Organizations.resolveOrganization(super.session); + this.onlyRealmBrokers = onlyRealmBrokers; + this.onlyOrganizationBrokers = onlyOrganizationBrokers; + } + + @Override + protected List searchForIdentityProviders(String existingIDP) { if (onlyRealmBrokers) { - providers = Optional.ofNullable(delegate.getProviders()).orElse(List.of()).stream() - .filter(Predicate.not(this::isPublicOrganizationBroker)) - .toList(); - } else if (onlyOrganizationBrokers) { - providers = Optional.ofNullable(delegate.getProviders()).orElse(List.of()).stream() - .filter(this::isPublicOrganizationBroker) - .toList(); - } else { - providers = Optional.ofNullable(delegate.getProviders()).orElse(List.of()).stream() - .filter(p -> isRealmBroker(p) || isPublicOrganizationBroker(p)) - .toList(); + // we only want the realm-level IDPs - i.e. those not associated with any orgs. + return session.identityProviders().getForLogin(IDPProvider.FETCH_MODE.REALM_ONLY, null) + .filter(idp -> !Objects.equals(existingIDP, idp.getAlias())) + .map(idp -> createIdentityProvider(this.realm, this.baseURI, idp)) + .sorted(IDP_COMPARATOR_INSTANCE).toList(); } - } - public OrganizationAwareIdentityProviderBean(IdentityProviderBean delegate, KeycloakSession session) { - this(delegate, session, false); + if (onlyOrganizationBrokers) { + // we already have the organization, just fetch the organization's public enabled IDPs. + if (this.organization != null) { + return organization.getIdentityProviders() + .filter(idp -> idp.isEnabled() && !idp.isLinkOnly() && !idp.isHideOnLogin() + && Boolean.parseBoolean(idp.getConfig().get(OrganizationModel.BROKER_PUBLIC))) + .map(idp -> createIdentityProvider(super.realm, super.baseURI, idp)) + .toList(); + } + // we don't have a specific organization - fetch public enabled IDPs linked to any org. + return session.identityProviders().getForLogin(IDPProvider.FETCH_MODE.ORG_ONLY, null) + .filter(idp -> !Objects.equals(existingIDP, idp.getAlias())) + .map(idp -> createIdentityProvider(this.realm, this.baseURI, idp)) + .sorted(IDP_COMPARATOR_INSTANCE).toList(); + } + return session.identityProviders().getForLogin(IDPProvider.FETCH_MODE.ALL, this.organization != null ? this.organization.getId() : null) + .filter(idp -> !Objects.equals(existingIDP, idp.getAlias())) + .map(idp -> createIdentityProvider(this.realm, this.baseURI, idp)) + .sorted(IDP_COMPARATOR_INSTANCE).toList(); } @Override - public List getProviders() { - return providers; + protected Predicate federatedProviderPredicate() { + // use the predicate from the superclass combined with the organization filter. + return super.federatedProviderPredicate().and(idp -> { + if (onlyRealmBrokers) { + return idp.getOrganizationId() == null; + } else if (onlyOrganizationBrokers) { + return isPublicOrganizationBroker(idp); + } else { + return idp.getOrganizationId() == null || isPublicOrganizationBroker(idp); + } + }); } - @Override - public boolean isDisplayInfo() { - return !providers.isEmpty(); - } + private boolean isPublicOrganizationBroker(IdentityProviderModel idp) { - private boolean isPublicOrganizationBroker(IdentityProvider idp) { - IdentityProviderModel model = session.identityProviders().getByAlias(idp.getAlias()); - - if (model.getOrganizationId() == null) { + if (idp.getOrganizationId() == null) { return false; } - - OrganizationModel organization = Organizations.resolveOrganization(session); - - if (organization != null && !organization.getId().equals(model.getOrganizationId())) { + if (organization != null && !Objects.equals(organization.getId(),idp.getOrganizationId())) { return false; } - - return Boolean.parseBoolean(model.getConfig().getOrDefault(OrganizationModel.BROKER_PUBLIC, Boolean.FALSE.toString())); - } - - private boolean isRealmBroker(IdentityProvider idp) { - IdentityProviderModel model = session.identityProviders().getByAlias(idp.getAlias()); - return model.getOrganizationId() == null; + return Boolean.parseBoolean(idp.getConfig().getOrDefault(OrganizationModel.BROKER_PUBLIC, Boolean.FALSE.toString())); } } diff --git a/services/src/main/java/org/keycloak/services/resources/admin/IdentityProviderResource.java b/services/src/main/java/org/keycloak/services/resources/admin/IdentityProviderResource.java index 3d93e46d14..eb4780afe9 100644 --- a/services/src/main/java/org/keycloak/services/resources/admin/IdentityProviderResource.java +++ b/services/src/main/java/org/keycloak/services/resources/admin/IdentityProviderResource.java @@ -197,6 +197,8 @@ public class IdentityProviderResource { } session.identityProviders().update(updated); + // update in case of legacy hide on login attr was used. + providerRep.setHideOnLogin(updated.isHideOnLogin()); if (oldProviderAlias != null && !oldProviderAlias.equals(newProviderAlias)) { diff --git a/services/src/main/java/org/keycloak/services/resources/admin/IdentityProvidersResource.java b/services/src/main/java/org/keycloak/services/resources/admin/IdentityProvidersResource.java index 651bb49ea8..2fb91d5645 100755 --- a/services/src/main/java/org/keycloak/services/resources/admin/IdentityProvidersResource.java +++ b/services/src/main/java/org/keycloak/services/resources/admin/IdentityProvidersResource.java @@ -215,6 +215,7 @@ public class IdentityProvidersResource { session.identityProviders().create(identityProvider); representation.setInternalId(identityProvider.getInternalId()); + representation.setHideOnLogin(identityProvider.isHideOnLogin()); // update in case of legacy hide on login attr was used. adminEvent.operation(OperationType.CREATE).resourcePath(session.getContext().getUri(), identityProvider.getAlias()) .representation(StripSecretsUtils.stripSecrets(session, representation)).success(); diff --git a/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/admin/IdentityProviderTest.java b/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/admin/IdentityProviderTest.java index f7879efbcb..874d793f21 100755 --- a/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/admin/IdentityProviderTest.java +++ b/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/admin/IdentityProviderTest.java @@ -47,6 +47,7 @@ import java.nio.file.Paths; import java.security.PublicKey; import java.security.cert.CertificateEncodingException; import java.security.cert.X509Certificate; +import java.util.ArrayList; import java.util.Arrays; import java.util.Base64; import java.util.Collections; @@ -587,6 +588,8 @@ public class IdentityProviderTest extends AbstractAdminTest { String secret = idpRep.getConfig() != null ? idpRep.getConfig().get("clientSecret") : null; idpRep = StripSecretsUtils.stripSecrets(null, idpRep); + // if legacy hide on login page attribute was used, the attr will be removed when converted to model + idpRep.setHideOnLogin(Boolean.parseBoolean(idpRep.getConfig().remove(IdentityProviderModel.LEGACY_HIDE_ON_LOGIN_ATTR))); assertAdminEvents.assertEvent(realmId, OperationType.CREATE, AdminEventPaths.identityProviderPath(idpRep.getAlias()), idpRep, ResourceType.IDENTITY_PROVIDER); @@ -1044,6 +1047,7 @@ public class IdentityProviderTest extends AbstractAdminTest { Assert.assertEquals("alias", expected.getAlias(), actual.getAlias()); Assert.assertEquals("providerId", expected.getProviderId(), actual.getProviderId()); Assert.assertEquals("enabled", expected.isEnabled(), actual.isEnabled()); + Assert.assertEquals("hideOnLogin", expected.isHideOnLogin(), actual.isHideOnLogin()); Assert.assertEquals("firstBrokerLoginFlowAlias", expected.getFirstBrokerLoginFlowAlias(), actual.getFirstBrokerLoginFlowAlias()); Assert.assertEquals("config", expected.getConfig(), actual.getConfig()); } @@ -1055,32 +1059,36 @@ public class IdentityProviderTest extends AbstractAdminTest { Assert.assertEquals("alias", "saml", idp.getAlias()); Assert.assertEquals("providerId", "saml", idp.getProviderId()); Assert.assertEquals("enabled",enabled, idp.isEnabled()); + Assert.assertTrue("hideOnLogin", idp.isHideOnLogin()); Assert.assertNull("firstBrokerLoginFlowAlias", idp.getFirstBrokerLoginFlowAlias()); - assertSamlConfig(idp.getConfig(), postBindingResponse); + assertSamlConfig(idp.getConfig(), postBindingResponse, false); } - private void assertSamlConfig(Map config, boolean postBindingResponse) { + private void assertSamlConfig(Map config, boolean postBindingResponse, boolean hasHideOnLoginPage) { // import endpoint simply converts IDPSSODescriptor into key value pairs. // check that saml-idp-metadata.xml was properly converted into key value pairs //System.out.println(config); - assertThat(config.keySet(), containsInAnyOrder( - "syncMode", - "validateSignature", - "singleLogoutServiceUrl", - "postBindingLogout", - "postBindingResponse", - "artifactBindingResponse", - "postBindingAuthnRequest", - "singleSignOnServiceUrl", - "artifactResolutionServiceUrl", - "wantAuthnRequestsSigned", - "nameIDPolicyFormat", - "signingCertificate", - "addExtensionsElementWithKeyInfo", - "loginHint", - "hideOnLoginPage", - "idpEntityId" + List configKeys = new ArrayList<>(List.of( + "syncMode", + "validateSignature", + "singleLogoutServiceUrl", + "postBindingLogout", + "postBindingResponse", + "artifactBindingResponse", + "postBindingAuthnRequest", + "singleSignOnServiceUrl", + "artifactResolutionServiceUrl", + "wantAuthnRequestsSigned", + "nameIDPolicyFormat", + "signingCertificate", + "addExtensionsElementWithKeyInfo", + "loginHint", + "idpEntityId" )); + if (hasHideOnLoginPage) { + configKeys.add("hideOnLoginPage"); + } + assertThat(config.keySet(), containsInAnyOrder(configKeys.toArray())); assertThat(config, hasEntry("validateSignature", "true")); assertThat(config, hasEntry("singleLogoutServiceUrl", "http://localhost:8080/auth/realms/master/protocol/saml")); assertThat(config, hasEntry("artifactResolutionServiceUrl", "http://localhost:8080/auth/realms/master/protocol/saml/resolve")); @@ -1091,9 +1099,11 @@ public class IdentityProviderTest extends AbstractAdminTest { assertThat(config, hasEntry("wantAuthnRequestsSigned", "true")); assertThat(config, hasEntry("addExtensionsElementWithKeyInfo", "false")); assertThat(config, hasEntry("nameIDPolicyFormat", "urn:oasis:names:tc:SAML:2.0:nameid-format:persistent")); - assertThat(config, hasEntry("hideOnLoginPage", "true")); assertThat(config, hasEntry("idpEntityId", "http://localhost:8080/auth/realms/master")); assertThat(config, hasEntry(is("signingCertificate"), notNullValue())); + if (hasHideOnLoginPage) { + assertThat(config, hasEntry("hideOnLoginPage", "true")); + } } private void assertSamlImport(Map config, String expectedSigningCertificates, boolean enabled, boolean postBindingResponse) { @@ -1101,7 +1111,7 @@ public class IdentityProviderTest extends AbstractAdminTest { boolean enabledFromMetadata = Boolean.valueOf(config.get(SAMLIdentityProviderConfig.ENABLED_FROM_METADATA)); config.remove(SAMLIdentityProviderConfig.ENABLED_FROM_METADATA); Assert.assertEquals(enabledFromMetadata,enabled); - assertSamlConfig(config, postBindingResponse); + assertSamlConfig(config, postBindingResponse, true); assertThat(config, hasEntry("signingCertificate", expectedSigningCertificates)); } diff --git a/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/broker/KcOidcBrokerHiddenIdpHintTest.java b/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/broker/KcOidcBrokerHiddenIdpHintTest.java index 444ad73c6a..f9ddec86cf 100644 --- a/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/broker/KcOidcBrokerHiddenIdpHintTest.java +++ b/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/broker/KcOidcBrokerHiddenIdpHintTest.java @@ -30,7 +30,7 @@ import static org.keycloak.testsuite.broker.BrokerTestTools.createIdentityProvid /** * Migrated from old testsuite. Previous version by Pedro Igor. - * + * * @author Stan Silvert ssilvert@redhat.com (C) 2019 Red Hat Inc. * @author pedroigor */ @@ -40,16 +40,16 @@ public class KcOidcBrokerHiddenIdpHintTest extends AbstractInitializedBaseBroker protected BrokerConfiguration getBrokerConfiguration() { return new KcOidcHiddenBrokerConfiguration(); } - + private class KcOidcHiddenBrokerConfiguration extends KcOidcBrokerConfiguration { - + @Override public IdentityProviderRepresentation setUpIdentityProvider(IdentityProviderSyncMode syncMode) { IdentityProviderRepresentation idp = createIdentityProvider(IDP_OIDC_ALIAS, IDP_OIDC_PROVIDER_ID); Map config = idp.getConfig(); applyDefaultConfiguration(config, syncMode); - config.put("hideOnLoginPage", "true"); + idp.setHideOnLogin(true); return idp; } } @@ -68,9 +68,9 @@ public class KcOidcBrokerHiddenIdpHintTest extends AbstractInitializedBaseBroker log.debug("Logging in"); loginPage.login(bc.getUserLogin(), bc.getUserPassword()); - + // authenticated and redirected to app Assert.assertTrue(driver.getCurrentUrl().contains("/auth/realms/" + bc.consumerRealmName() + "/")); } - -} \ No newline at end of file + +} diff --git a/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/organization/broker/OrganizationIdentityProviderTest.java b/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/organization/broker/OrganizationIdentityProviderTest.java index 037728adaf..90c7c522e3 100644 --- a/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/organization/broker/OrganizationIdentityProviderTest.java +++ b/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/organization/broker/OrganizationIdentityProviderTest.java @@ -56,12 +56,12 @@ public class OrganizationIdentityProviderTest extends AbstractOrganizationTest { IdentityProviderRepresentation expected = orgIdPResource.toRepresentation(); // organization link set - Assert.assertEquals(expected.getConfig().get(OrganizationModel.ORGANIZATION_ATTRIBUTE), organization.getId()); + Assert.assertEquals(expected.getOrganizationId(), organization.getId()); IdentityProviderResource idpResource = testRealm().identityProviders().get(expected.getAlias()); IdentityProviderRepresentation actual = idpResource.toRepresentation(); - Assert.assertEquals(actual.getConfig().get(OrganizationModel.ORGANIZATION_ATTRIBUTE), organization.getId()); - actual.getConfig().put(OrganizationModel.ORGANIZATION_ATTRIBUTE, "somethingelse"); + Assert.assertEquals(actual.getOrganizationId(), organization.getId()); + actual.setOrganizationId("somethingelse"); try { idpResource.update(actual); Assert.fail("Should fail because it maps to an invalid org"); @@ -69,19 +69,19 @@ public class OrganizationIdentityProviderTest extends AbstractOrganizationTest { } OrganizationRepresentation secondOrg = createOrganization("secondorg"); - actual.getConfig().put(OrganizationModel.ORGANIZATION_ATTRIBUTE, secondOrg.getId()); + actual.setOrganizationId(secondOrg.getId()); idpResource.update(actual); actual = idpResource.toRepresentation(); - Assert.assertEquals(actual.getConfig().get(OrganizationModel.ORGANIZATION_ATTRIBUTE), organization.getId()); + Assert.assertEquals(actual.getOrganizationId(), organization.getId()); actual = idpResource.toRepresentation(); // the link to the organization should not change - Assert.assertEquals(actual.getConfig().get(OrganizationModel.ORGANIZATION_ATTRIBUTE), organization.getId()); - actual.getConfig().remove(OrganizationModel.ORGANIZATION_ATTRIBUTE); + Assert.assertEquals(actual.getOrganizationId(), organization.getId()); + actual.setOrganizationId(null); idpResource.update(actual); actual = idpResource.toRepresentation(); // the link to the organization should not change - Assert.assertEquals(actual.getConfig().get(OrganizationModel.ORGANIZATION_ATTRIBUTE), organization.getId()); + Assert.assertEquals(actual.getOrganizationId(), organization.getId()); String domain = actual.getConfig().get(ORGANIZATION_DOMAIN_ATTRIBUTE); @@ -112,18 +112,19 @@ public class OrganizationIdentityProviderTest extends AbstractOrganizationTest { .identityProviders().get(bc.getIDPAlias()).toRepresentation(); //remove Org related stuff from the template - idpTemplate.getConfig().remove(OrganizationModel.ORGANIZATION_ATTRIBUTE); + idpTemplate.setOrganizationId(null); idpTemplate.getConfig().remove(OrganizationModel.ORGANIZATION_DOMAIN_ATTRIBUTE); + idpTemplate.getConfig().remove(OrganizationModel.BROKER_PUBLIC); idpTemplate.getConfig().remove(OrganizationModel.IdentityProviderRedirectMode.EMAIL_MATCH.getKey()); for (int i = 0; i < 5; i++) { idpTemplate.setAlias("idp-" + i); idpTemplate.setInternalId(null); try (Response response = testRealm().identityProviders().create(idpTemplate)) { - assertThat("Falied to create idp-" + i, response.getStatus(), equalTo(Status.CREATED.getStatusCode())); + assertThat("Failed to create idp-" + i, response.getStatus(), equalTo(Status.CREATED.getStatusCode())); } try (Response response = organization.identityProviders().addIdentityProvider(idpTemplate.getAlias())) { - assertThat("Falied to add idp-" + i, response.getStatus(), equalTo(Status.NO_CONTENT.getStatusCode())); + assertThat("Failed to add idp-" + i, response.getStatus(), equalTo(Status.NO_CONTENT.getStatusCode())); } } @@ -191,7 +192,7 @@ public class OrganizationIdentityProviderTest extends AbstractOrganizationTest { // broker not removed from realm IdentityProviderRepresentation idpRep = testRealm().identityProviders().get(bc.getIDPAlias()).toRepresentation(); // broker no longer linked to the org - Assert.assertNull(idpRep.getConfig().get(OrganizationModel.ORGANIZATION_ATTRIBUTE)); + Assert.assertNull(idpRep.getOrganizationId()); Assert.assertNull(idpRep.getConfig().get(ORGANIZATION_DOMAIN_ATTRIBUTE)); Assert.assertNull(idpRep.getConfig().get(BROKER_PUBLIC)); } diff --git a/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/organization/exportimport/OrganizationExportTest.java b/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/organization/exportimport/OrganizationExportTest.java index dc95f3fe9b..98aff2d510 100644 --- a/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/organization/exportimport/OrganizationExportTest.java +++ b/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/organization/exportimport/OrganizationExportTest.java @@ -27,8 +27,8 @@ import java.util.ArrayList; import java.util.HashMap; import java.util.List; import java.util.Map; +import java.util.Objects; import java.util.Optional; -import java.util.function.Predicate; import jakarta.ws.rs.core.Response; import org.hamcrest.Matchers; @@ -194,7 +194,7 @@ public class OrganizationExportTest extends AbstractOrganizationTest { RealmRepresentation export = testRealm().partialExport(exportGroupsAndRoles, exportClients); assertTrue(Optional.ofNullable(export.getGroups()).orElse(List.of()).stream().noneMatch(g -> g.getAttributes().containsKey(OrganizationModel.ORGANIZATION_ATTRIBUTE))); assertTrue(Optional.ofNullable(export.getOrganizations()).orElse(List.of()).isEmpty()); - assertTrue(Optional.ofNullable(export.getIdentityProviders()).orElse(List.of()).stream().noneMatch(g -> g.getConfig().containsKey(OrganizationModel.ORGANIZATION_ATTRIBUTE))); + assertTrue(Optional.ofNullable(export.getIdentityProviders()).orElse(List.of()).stream().noneMatch(idp -> Objects.nonNull(idp.getOrganizationId()))); PartialImportRepresentation rep = new PartialImportRepresentation(); rep.setUsers(export.getUsers()); rep.setClients(export.getClients()); diff --git a/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/util/IdentityProviderBuilder.java b/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/util/IdentityProviderBuilder.java index b125c10e1a..bcce239960 100644 --- a/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/util/IdentityProviderBuilder.java +++ b/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/util/IdentityProviderBuilder.java @@ -50,9 +50,9 @@ public class IdentityProviderBuilder { rep.setDisplayName(displayName); return this; } - + public IdentityProviderBuilder hideOnLoginPage() { - setAttribute("hideOnLoginPage", "true"); + rep.setHideOnLogin(true); return this; }