KEYCLOAK-12278 Default first broker login flow is broken after migration (#6556)
This commit is contained in:
parent
56d53b191a
commit
fa453e9c0c
4 changed files with 201 additions and 13 deletions
|
@ -47,6 +47,7 @@ import org.keycloak.migration.migrators.MigrateTo4_2_0;
|
|||
import org.keycloak.migration.migrators.MigrateTo4_6_0;
|
||||
import org.keycloak.migration.migrators.MigrateTo6_0_0;
|
||||
import org.keycloak.migration.migrators.MigrateTo8_0_0;
|
||||
import org.keycloak.migration.migrators.MigrateTo8_0_2;
|
||||
import org.keycloak.migration.migrators.MigrateTo9_0_0;
|
||||
import org.keycloak.migration.migrators.Migration;
|
||||
import org.keycloak.models.KeycloakSession;
|
||||
|
@ -86,6 +87,7 @@ public class MigrationModelManager {
|
|||
new MigrateTo4_6_0(),
|
||||
new MigrateTo6_0_0(),
|
||||
new MigrateTo8_0_0(),
|
||||
new MigrateTo8_0_2(),
|
||||
new MigrateTo9_0_0()
|
||||
};
|
||||
|
||||
|
|
|
@ -0,0 +1,135 @@
|
|||
/*
|
||||
* Copyright 2019 Red Hat, Inc. and/or its affiliates
|
||||
* and other contributors as indicated by the @author tags.
|
||||
*
|
||||
* Licensed under the Apache License, Version 2.0 (the "License");
|
||||
* you may not use this file except in compliance with the License.
|
||||
* You may obtain a copy of the License at
|
||||
*
|
||||
* http://www.apache.org/licenses/LICENSE-2.0
|
||||
*
|
||||
* Unless required by applicable law or agreed to in writing, software
|
||||
* distributed under the License is distributed on an "AS IS" BASIS,
|
||||
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
|
||||
* See the License for the specific language governing permissions and
|
||||
* limitations under the License.
|
||||
*
|
||||
*/
|
||||
|
||||
package org.keycloak.migration.migrators;
|
||||
|
||||
import java.util.LinkedList;
|
||||
|
||||
import org.jboss.logging.Logger;
|
||||
import org.keycloak.migration.ModelVersion;
|
||||
import org.keycloak.models.AuthenticationExecutionModel;
|
||||
import org.keycloak.models.AuthenticationFlowModel;
|
||||
import org.keycloak.models.KeycloakSession;
|
||||
import org.keycloak.models.RealmModel;
|
||||
import org.keycloak.representations.idm.RealmRepresentation;
|
||||
|
||||
/**
|
||||
* @author <a href="mailto:mposolda@redhat.com">Marek Posolda</a>
|
||||
*/
|
||||
public class MigrateTo8_0_2 implements Migration {
|
||||
|
||||
public static final ModelVersion VERSION = new ModelVersion("8.0.2");
|
||||
|
||||
private static final Logger LOG = Logger.getLogger(MigrateTo8_0_2.class);
|
||||
|
||||
@Override
|
||||
public ModelVersion getVersion() {
|
||||
return VERSION;
|
||||
}
|
||||
|
||||
@Override
|
||||
public void migrate(KeycloakSession session) {
|
||||
session.realms().getRealms().forEach(this::migrateAuthenticationFlowsWithAlternativeRequirements);
|
||||
}
|
||||
|
||||
@Override
|
||||
public void migrateImport(KeycloakSession session, RealmModel realm, RealmRepresentation rep, boolean skipUserDependent) {
|
||||
migrateAuthenticationFlowsWithAlternativeRequirements(realm);
|
||||
}
|
||||
|
||||
|
||||
protected void migrateAuthenticationFlowsWithAlternativeRequirements(RealmModel realm) {
|
||||
for (AuthenticationFlowModel flow : realm.getAuthenticationFlows()) {
|
||||
|
||||
boolean alternativeFound = false;
|
||||
boolean requiredFound = false;
|
||||
for (AuthenticationExecutionModel execution : realm.getAuthenticationExecutions(flow.getId())) {
|
||||
switch (execution.getRequirement()) {
|
||||
case REQUIRED:
|
||||
case CONDITIONAL:
|
||||
requiredFound = true;
|
||||
break;
|
||||
case ALTERNATIVE:
|
||||
alternativeFound = true;
|
||||
break;
|
||||
}
|
||||
}
|
||||
|
||||
// This flow contains some REQUIRED and ALTERNATIVE at the same level. We will migrate ALTERNATIVES to separate subflows
|
||||
// to try to preserve same behaviour as in previous versions
|
||||
if (requiredFound && alternativeFound) {
|
||||
|
||||
// Suffix used just to avoid name conflicts
|
||||
int suffix = 0;
|
||||
LinkedList<AuthenticationExecutionModel> alternativesToMigrate = new LinkedList<>();
|
||||
for (AuthenticationExecutionModel execution : realm.getAuthenticationExecutions(flow.getId())) {
|
||||
if (AuthenticationExecutionModel.Requirement.ALTERNATIVE.equals(execution.getRequirement())) {
|
||||
alternativesToMigrate.add(execution);
|
||||
}
|
||||
|
||||
// If we have some REQUIRED then ALTERNATIVE and then REQUIRED/CONDITIONAL, we migrate the alternatives to the new subflow.
|
||||
if (AuthenticationExecutionModel.Requirement.REQUIRED.equals(execution.getRequirement()) ||
|
||||
AuthenticationExecutionModel.Requirement.CONDITIONAL.equals(execution.getRequirement())) {
|
||||
if (!alternativesToMigrate.isEmpty()) {
|
||||
migrateAlternatives(realm, flow, alternativesToMigrate, suffix);
|
||||
suffix += 1;
|
||||
alternativesToMigrate.clear();
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
if (!alternativesToMigrate.isEmpty()) {
|
||||
migrateAlternatives(realm, flow, alternativesToMigrate, suffix);
|
||||
}
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
|
||||
private void migrateAlternatives(RealmModel realm, AuthenticationFlowModel parentFlow,
|
||||
LinkedList<AuthenticationExecutionModel> alternativesToMigrate, int suffix) {
|
||||
LOG.debugf("Migrating %d ALTERNATIVE executions in the flow '%s' of realm '%s' to separate subflow", alternativesToMigrate.size(),
|
||||
parentFlow.getAlias(), realm.getName());
|
||||
|
||||
AuthenticationFlowModel newFlow = new AuthenticationFlowModel();
|
||||
newFlow.setTopLevel(false);
|
||||
newFlow.setBuiltIn(parentFlow.isBuiltIn());
|
||||
newFlow.setAlias(parentFlow.getAlias() + " - Alternatives - " + suffix);
|
||||
newFlow.setDescription("Subflow of " + parentFlow.getAlias() + " with alternative executions");
|
||||
newFlow.setProviderId("basic-flow");
|
||||
newFlow = realm.addAuthenticationFlow(newFlow);
|
||||
|
||||
AuthenticationExecutionModel execution = new AuthenticationExecutionModel();
|
||||
execution.setParentFlow(parentFlow.getId());
|
||||
execution.setRequirement(AuthenticationExecutionModel.Requirement.REQUIRED);
|
||||
execution.setFlowId(newFlow.getId());
|
||||
// Use same priority as the first ALTERNATIVE as new execution will defacto replace it in the parent flow
|
||||
execution.setPriority(alternativesToMigrate.getFirst().getPriority());
|
||||
execution.setAuthenticatorFlow(true);
|
||||
realm.addAuthenticatorExecution(execution);
|
||||
|
||||
int priority = 0;
|
||||
for (AuthenticationExecutionModel ex : alternativesToMigrate) {
|
||||
priority += 10;
|
||||
ex.setParentFlow(newFlow.getId());
|
||||
ex.setPriority(priority);
|
||||
realm.updateAuthenticatorExecution(ex);
|
||||
}
|
||||
}
|
||||
|
||||
}
|
|
@ -347,7 +347,7 @@ Run the test (Update according to your DB connection, versions etc):
|
|||
|
||||
mvn -B -f testsuite/integration-arquillian/pom.xml \
|
||||
clean install \
|
||||
-Pjpa,auth-server-wildfly,db-mysql,auth-server-migration \
|
||||
-Pjpa,auth-server-wildfly,db-mariadb,auth-server-migration \
|
||||
-Dauth.server.jboss.startup.timeout=900 \
|
||||
-Dtest=MigrationTest \
|
||||
-Dmigration.mode=auto \
|
||||
|
@ -355,10 +355,7 @@ Run the test (Update according to your DB connection, versions etc):
|
|||
-Dprevious.product.unpacked.folder.name=keycloak-$OLD_KEYCLOAK_VERSION \
|
||||
-Dmigration.import.file.name=migration-realm-$OLD_KEYCLOAK_VERSION.json \
|
||||
-Dauth.server.ssl.required=false \
|
||||
-Djdbc.mvn.version.legacy=5.1.38 \
|
||||
-Djdbc.mvn.groupId=mysql \
|
||||
-Djdbc.mvn.artifactId=mysql-connector-java \
|
||||
-Djdbc.mvn.version=8.0.12
|
||||
-Djdbc.mvn.version.legacy=2.2.4
|
||||
|
||||
|
||||
For the available versions of old keycloak server, you can take a look to [this directory](tests/base/src/test/resources/migration-test) .
|
||||
|
|
|
@ -16,25 +16,27 @@
|
|||
*/
|
||||
package org.keycloak.testsuite.migration;
|
||||
|
||||
import org.apache.commons.io.IOUtils;
|
||||
import org.apache.http.impl.client.CloseableHttpClient;
|
||||
import org.apache.http.impl.client.HttpClientBuilder;
|
||||
import org.hamcrest.Matchers;
|
||||
import org.keycloak.admin.client.resource.ClientResource;
|
||||
import org.keycloak.admin.client.resource.ClientsResource;
|
||||
import org.keycloak.admin.client.resource.RealmResource;
|
||||
import org.keycloak.admin.client.resource.RoleResource;
|
||||
import org.keycloak.authentication.authenticators.broker.IdpConfirmLinkAuthenticatorFactory;
|
||||
import org.keycloak.authentication.authenticators.broker.IdpCreateUserIfUniqueAuthenticatorFactory;
|
||||
import org.keycloak.authentication.authenticators.broker.IdpEmailVerificationAuthenticatorFactory;
|
||||
import org.keycloak.authentication.authenticators.broker.IdpReviewProfileAuthenticatorFactory;
|
||||
import org.keycloak.authentication.authenticators.broker.IdpUsernamePasswordFormFactory;
|
||||
import org.keycloak.authentication.authenticators.browser.OTPFormAuthenticatorFactory;
|
||||
import org.keycloak.authentication.authenticators.conditional.ConditionalUserConfiguredAuthenticatorFactory;
|
||||
import org.keycloak.broker.provider.util.SimpleHttp;
|
||||
import org.keycloak.common.constants.KerberosConstants;
|
||||
import org.keycloak.component.PrioritizedComponentModel;
|
||||
import org.keycloak.keys.KeyProvider;
|
||||
import org.keycloak.models.AccountRoles;
|
||||
import org.keycloak.models.AdminRoles;
|
||||
import org.keycloak.models.AuthenticationExecutionModel;
|
||||
import org.keycloak.models.ClientModel;
|
||||
import org.keycloak.models.Constants;
|
||||
import org.keycloak.models.LDAPConstants;
|
||||
import org.keycloak.models.RealmModel;
|
||||
import org.keycloak.models.UserModel;
|
||||
import org.keycloak.models.utils.DefaultAuthenticationFlows;
|
||||
import org.keycloak.models.utils.TimeBasedOTP;
|
||||
|
@ -64,10 +66,8 @@ import org.keycloak.testsuite.util.OAuthClient;
|
|||
|
||||
import java.io.IOException;
|
||||
import java.net.URI;
|
||||
import java.nio.charset.StandardCharsets;
|
||||
import java.util.ArrayList;
|
||||
import java.util.Arrays;
|
||||
import java.util.Collections;
|
||||
import java.util.HashSet;
|
||||
import java.util.List;
|
||||
import java.util.Map;
|
||||
|
@ -80,7 +80,6 @@ import static org.hamcrest.Matchers.containsInAnyOrder;
|
|||
import static org.hamcrest.Matchers.equalTo;
|
||||
import static org.hamcrest.Matchers.is;
|
||||
|
||||
import static org.junit.Assert.assertArrayEquals;
|
||||
import static org.junit.Assert.assertEquals;
|
||||
import static org.junit.Assert.assertFalse;
|
||||
import static org.junit.Assert.assertNotNull;
|
||||
|
@ -277,6 +276,8 @@ public abstract class AbstractMigrationTest extends AbstractKeycloakTest {
|
|||
testAccountConsoleClient(masterRealm);
|
||||
testAccountConsoleClient(migrationRealm);
|
||||
testAlwaysDisplayInConsole();
|
||||
testFirstBrokerLoginFlowMigrated(masterRealm);
|
||||
testFirstBrokerLoginFlowMigrated(migrationRealm);
|
||||
}
|
||||
|
||||
private void testAdminClientUrls(RealmResource realm) {
|
||||
|
@ -324,6 +325,59 @@ public abstract class AbstractMigrationTest extends AbstractKeycloakTest {
|
|||
assertEquals("oidc-audience-resolve-mapper", mappers.get(0).getProtocolMapper());
|
||||
}
|
||||
|
||||
private void testFirstBrokerLoginFlowMigrated(RealmResource realm) {
|
||||
log.infof("Test that firstBrokerLogin flow was migrated in new realm '%s'", realm.toRepresentation().getRealm());
|
||||
|
||||
List<AuthenticationExecutionInfoRepresentation> authExecutions = realm.flows().getExecutions(DefaultAuthenticationFlows.FIRST_BROKER_LOGIN_FLOW);
|
||||
|
||||
testAuthenticationExecution(authExecutions.get(0), null,
|
||||
IdpReviewProfileAuthenticatorFactory.PROVIDER_ID, AuthenticationExecutionModel.Requirement.REQUIRED, 0, 0);
|
||||
|
||||
testAuthenticationExecution(authExecutions.get(1), true,
|
||||
null, AuthenticationExecutionModel.Requirement.REQUIRED, 0, 1);
|
||||
|
||||
testAuthenticationExecution(authExecutions.get(2), null,
|
||||
IdpCreateUserIfUniqueAuthenticatorFactory.PROVIDER_ID, AuthenticationExecutionModel.Requirement.ALTERNATIVE, 1, 0);
|
||||
|
||||
testAuthenticationExecution(authExecutions.get(3), true,
|
||||
null, AuthenticationExecutionModel.Requirement.ALTERNATIVE, 1, 1);
|
||||
|
||||
testAuthenticationExecution(authExecutions.get(4), null,
|
||||
IdpConfirmLinkAuthenticatorFactory.PROVIDER_ID, AuthenticationExecutionModel.Requirement.REQUIRED, 2, 0);
|
||||
|
||||
testAuthenticationExecution(authExecutions.get(5), true,
|
||||
null, AuthenticationExecutionModel.Requirement.REQUIRED, 2, 1);
|
||||
|
||||
testAuthenticationExecution(authExecutions.get(6), null,
|
||||
IdpEmailVerificationAuthenticatorFactory.PROVIDER_ID, AuthenticationExecutionModel.Requirement.ALTERNATIVE, 3, 0);
|
||||
|
||||
testAuthenticationExecution(authExecutions.get(7), true,
|
||||
null, AuthenticationExecutionModel.Requirement.ALTERNATIVE, 3, 1);
|
||||
|
||||
testAuthenticationExecution(authExecutions.get(8), null,
|
||||
IdpUsernamePasswordFormFactory.PROVIDER_ID, AuthenticationExecutionModel.Requirement.REQUIRED, 4, 0);
|
||||
|
||||
testAuthenticationExecution(authExecutions.get(9), true,
|
||||
null, AuthenticationExecutionModel.Requirement.CONDITIONAL, 4, 1);
|
||||
|
||||
// There won't be a requirement in the future, so this test would need to change
|
||||
testAuthenticationExecution(authExecutions.get(10), null,
|
||||
ConditionalUserConfiguredAuthenticatorFactory.PROVIDER_ID, AuthenticationExecutionModel.Requirement.REQUIRED, 5, 0);
|
||||
|
||||
testAuthenticationExecution(authExecutions.get(11), null,
|
||||
OTPFormAuthenticatorFactory.PROVIDER_ID, AuthenticationExecutionModel.Requirement.REQUIRED, 5, 1);
|
||||
}
|
||||
|
||||
|
||||
private void testAuthenticationExecution(AuthenticationExecutionInfoRepresentation execution, Boolean expectedAuthenticationFlow, String expectedProviderId,
|
||||
AuthenticationExecutionModel.Requirement expectedRequirement, int expectedLevel, int expectedIndex) {
|
||||
Assert.assertEquals(execution.getAuthenticationFlow(), expectedAuthenticationFlow);
|
||||
Assert.assertEquals(execution.getProviderId(), expectedProviderId);
|
||||
Assert.assertEquals(execution.getRequirement(), expectedRequirement.toString());
|
||||
Assert.assertEquals(execution.getLevel(), expectedLevel);
|
||||
Assert.assertEquals(execution.getIndex(), expectedIndex);
|
||||
}
|
||||
|
||||
private void testDecisionStrategySetOnResourceServer() {
|
||||
ClientsResource clients = migrationRealm.clients();
|
||||
ClientRepresentation clientRepresentation = clients.findByClientId("authz-servlet").get(0);
|
||||
|
|
Loading…
Reference in a new issue