Skip to content

Commit

Permalink
Merge pull request #1208 from adorsys/fix/auth-flow-deletion-with-idp…
Browse files Browse the repository at this point in the history
…-reference

fix authentication flow error when it's referenced as an IdP
  • Loading branch information
AssahBismarkabah authored Nov 27, 2024
2 parents dbf6bdd + a50a02e commit afea1c2
Show file tree
Hide file tree
Showing 6 changed files with 196 additions and 4 deletions.
3 changes: 2 additions & 1 deletion CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,8 @@ All notable changes to this project will be documented in this file.
The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/), and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.html).

## [Unreleased]

### Fix Fails to delete authentication flow when it's referenced as an IdP [#868](https://github.com/adorsys/keycloak-config-cli/issues/868)
-
## Fixed
- otpPolicyAlgorithm ignored during import [#847](https://github.com/adorsys/keycloak-config-cli/issues/847)

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -26,11 +26,13 @@
import de.adorsys.keycloak.config.properties.ImportConfigProperties;
import de.adorsys.keycloak.config.properties.ImportConfigProperties.ImportManagedProperties.ImportManagedPropertiesValues;
import de.adorsys.keycloak.config.repository.AuthenticationFlowRepository;
import de.adorsys.keycloak.config.repository.IdentityProviderRepository;
import de.adorsys.keycloak.config.repository.RealmRepository;
import de.adorsys.keycloak.config.util.AuthenticationFlowUtil;
import de.adorsys.keycloak.config.util.CloneUtil;
import org.keycloak.representations.idm.AuthenticationExecutionInfoRepresentation;
import org.keycloak.representations.idm.AuthenticationFlowRepresentation;
import org.keycloak.representations.idm.IdentityProviderRepresentation;
import org.keycloak.representations.idm.RealmRepresentation;
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;
Expand Down Expand Up @@ -61,6 +63,7 @@ public class AuthenticationFlowsImportService {
private final ExecutionFlowsImportService executionFlowsImportService;
private final AuthenticatorConfigImportService authenticatorConfigImportService;
private final UsedAuthenticationFlowWorkaroundFactory workaroundFactory;
private final IdentityProviderRepository identityProviderRepository;

private final ImportConfigProperties importConfigProperties;

Expand All @@ -70,14 +73,16 @@ public AuthenticationFlowsImportService(
AuthenticationFlowRepository authenticationFlowRepository,
ExecutionFlowsImportService executionFlowsImportService,
AuthenticatorConfigImportService authenticatorConfigImportService, UsedAuthenticationFlowWorkaroundFactory workaroundFactory,
ImportConfigProperties importConfigProperties
ImportConfigProperties importConfigProperties,
IdentityProviderRepository identityProviderRepository
) {
this.realmRepository = realmRepository;
this.authenticationFlowRepository = authenticationFlowRepository;
this.executionFlowsImportService = executionFlowsImportService;
this.authenticatorConfigImportService = authenticatorConfigImportService;
this.workaroundFactory = workaroundFactory;
this.importConfigProperties = importConfigProperties;
this.identityProviderRepository = identityProviderRepository;
}

/**
Expand Down Expand Up @@ -330,6 +335,26 @@ private void recreateTopLevelFlow(
workaround.resetFlowIfNeeded();
}


/**
* Returns true if the flow is referenced by an identity provider in the given realm,
* either as first broker login flow or as post broker login flow.
*
* @param realmName the keycloak realm name
* @param flowAlias the alias of the flow to check
* @return true if the flow is referenced, false if not
*/

private boolean isFlowReferencedByIdP(String realmName, String flowAlias) {
List<IdentityProviderRepresentation> idps = identityProviderRepository.getAll(realmName);
Optional<IdentityProviderRepresentation> match = idps.stream()
.filter(idp ->
flowAlias.equals(idp.getFirstBrokerLoginFlowAlias())
|| flowAlias.equals(idp.getPostBrokerLoginFlowAlias())
).findAny();
return match.isPresent();
}

private void deleteTopLevelFlowsMissingInImport(
RealmImport realmImport,
List<AuthenticationFlowRepresentation> importedTopLevelFlows
Expand All @@ -345,8 +370,12 @@ private void deleteTopLevelFlowsMissingInImport(
for (AuthenticationFlowRepresentation existingTopLevelFlow : existingTopLevelFlows) {
if (topLevelFlowsToImportAliases.contains(existingTopLevelFlow.getAlias())) continue;

logger.debug("Delete authentication flow: {}", existingTopLevelFlow.getAlias());
authenticationFlowRepository.delete(realmName, existingTopLevelFlow.getId());
if (!isFlowReferencedByIdP(realmName, existingTopLevelFlow.getAlias())) {
logger.debug("Delete authentication flow: {}", existingTopLevelFlow.getAlias());
authenticationFlowRepository.delete(realmName, existingTopLevelFlow.getId());
} else {
logger.warn("Cannot delete authentication flow '{}' as it is referenced by an identity provider", existingTopLevelFlow.getAlias());
}
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -98,13 +98,40 @@ private void createOrUpdateIdentityProvider(RealmImport realmImport, IdentityPro
Optional<IdentityProviderRepresentation> maybeIdentityProvider = identityProviderRepository.search(realmName, identityProviderName);

if (maybeIdentityProvider.isPresent()) {
updateIdentityProviderFlowAliases(realmName, identityProvider);
updateIdentityProviderIfNecessary(realmName, identityProvider);
} else {
logger.debug("Create identityProvider '{}' in realm '{}'", identityProviderName, realmName);
identityProviderRepository.create(realmName, identityProvider);
}
}

public void updateIdentityProviderFlowAliases(String realmName, IdentityProviderRepresentation identityProvider) {
IdentityProviderRepresentation existingIdp = identityProviderRepository.getByAlias(realmName, identityProvider.getAlias());
if (existingIdp != null) {
boolean updated = false;
if (hasFirstBrokerLoginFlowAliasChanged(existingIdp, identityProvider)) {
existingIdp.setFirstBrokerLoginFlowAlias(identityProvider.getFirstBrokerLoginFlowAlias());
updated = true;
}
if (hasPostBrokerLoginFlowAliasChanged(existingIdp, identityProvider)) {
existingIdp.setPostBrokerLoginFlowAlias(identityProvider.getPostBrokerLoginFlowAlias());
updated = true;
}
if (updated) {
identityProviderRepository.update(realmName, existingIdp);
}
}
}

private boolean hasFirstBrokerLoginFlowAliasChanged(IdentityProviderRepresentation existingIdp, IdentityProviderRepresentation identityProvider) {
return !Objects.equals(existingIdp.getFirstBrokerLoginFlowAlias(), identityProvider.getFirstBrokerLoginFlowAlias());
}

private boolean hasPostBrokerLoginFlowAliasChanged(IdentityProviderRepresentation existingIdp, IdentityProviderRepresentation identityProvider) {
return !Objects.equals(existingIdp.getPostBrokerLoginFlowAlias(), identityProvider.getPostBrokerLoginFlowAlias());
}

private void updateIdentityProviderIfNecessary(String realmName, IdentityProviderRepresentation identityProvider) {
IdentityProviderRepresentation existingIdentityProvider = identityProviderRepository.getByAlias(realmName, identityProvider.getAlias());
IdentityProviderRepresentation patchedIdentityProvider = CloneUtil.patch(existingIdentityProvider, identityProvider);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -1201,6 +1201,46 @@ void shouldNotUpdateFlowWithAuthenticatorOnBasicFlow() throws IOException {
assertThat(thrown.getMessage(), is("Execution property authenticator 'registration-page-form' can be only set if the sub-flow 'JToken Conditional' type is 'form-flow'."));
}


@Test
@Order(66)
void shouldNotDeleteAuthenticationFlowReferencedByIdentityProvider() throws IOException {
System.setProperty("org.jboss.resteasy.client.jaxrs.internal.ClientInvocation", "DEBUG");
doImport("66a_initialize_realm_with_custom_flow.json");

RealmRepresentation realm = keycloakProvider.getInstance().realm(REALM_NAME).partialExport(true, true);
assertThat(realm.getRealm(), is(REALM_NAME));
assertThat(realm.isEnabled(), is(true));

IdentityProviderRepresentation identityProvider = identityProviderRepository.getAll(REALM_NAME).stream()
.filter(idp -> "saml-with-custom-flow".equals(idp.getAlias()))
.findFirst()
.orElse(null);

assertThat(identityProvider, is(not(nullValue())));
assertThat(identityProvider.getFirstBrokerLoginFlowAlias(), is("my custom first login flow"));
assertThat(identityProvider.getPostBrokerLoginFlowAlias(), is("my custom post login flow"));

doImport("66b_try_delete_referenced_authentication_flow.json");
RealmRepresentation updatedRealm = keycloakProvider.getInstance().realm(REALM_NAME).partialExport(true, true);

AuthenticationFlowRepresentation firstLoginFlow = getAuthenticationFlow(updatedRealm, "my custom first login flow");
assertThat(firstLoginFlow, is(not(nullValue())));
assertThat(firstLoginFlow.getAlias(), is("my custom first login flow"));

AuthenticationFlowRepresentation postLoginFlow = getAuthenticationFlow(updatedRealm, "my custom post login flow");
assertThat(postLoginFlow, is(not(nullValue())));
assertThat(postLoginFlow.getAlias(), is("my custom post login flow"));


IdentityProviderRepresentation updatedIdentityProvider = identityProviderRepository.getAll(REALM_NAME).stream()
.filter(idp -> "saml-with-custom-flow".equals(idp.getAlias()))
.findFirst()
.orElse(null);
assertThat(updatedIdentityProvider, is(not(nullValue())));
assertThat(updatedIdentityProvider.getFirstBrokerLoginFlowAlias(), is("my custom first login flow"));
assertThat(updatedIdentityProvider.getPostBrokerLoginFlowAlias(), is(nullValue()));
}
@Test
void shouldChangeSubFlowOfFirstBrokerLoginFlow() throws IOException {
doImport("init_custom_first-broker-login-flow.json");
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,54 @@
{
"enabled": true,
"realm": "realmWithFlow",
"identityProviders": [
{
"alias": "saml-with-custom-flow",
"providerId": "saml",
"enabled": true,
"updateProfileFirstLoginMode": "on",
"trustEmail": true,
"storeToken": false,
"addReadTokenRoleOnCreate": true,
"authenticateByDefault": false,
"linkOnly": false,
"firstBrokerLoginFlowAlias": "my custom first login flow",
"postBrokerLoginFlowAlias": "my custom post login flow",
"config": {}
}
],
"authenticationFlows": [
{
"alias": "my custom first login flow",
"description": "My auth first login for testing",
"providerId": "basic-flow",
"topLevel": true,
"builtIn": false,
"authenticationExecutions": [
{
"authenticator": "docker-http-basic-authenticator",
"requirement": "DISABLED",
"priority": 0,
"userSetupAllowed": false,
"autheticatorFlow": false
}
]
},
{
"alias": "my custom post login flow",
"description": "My auth post login for testing",
"providerId": "basic-flow",
"topLevel": true,
"builtIn": false,
"authenticationExecutions": [
{
"authenticator": "docker-http-basic-authenticator",
"requirement": "DISABLED",
"priority": 0,
"userSetupAllowed": false,
"autheticatorFlow": false
}
]
}
]
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,41 @@
{
"enabled": true,
"realm": "realmWithFlow",
"identityProviders": [
{
"alias": "saml-with-custom-flow",
"providerId": "saml",
"enabled": true,
"updateProfileFirstLoginMode": "on",
"trustEmail": true,
"storeToken": false,
"addReadTokenRoleOnCreate": true,
"authenticateByDefault": false,
"linkOnly": false,
"firstBrokerLoginFlowAlias": "my custom first login flow",
"postBrokerLoginFlowAlias": "",
"config": {}
}
],
"authenticationFlows": [
{
"alias": "my custom first login flow",
"description": "My auth first login for testing",
"providerId": "basic-flow",
"topLevel": true,
"builtIn": false,
"authenticationExecutions": [
{
"authenticator": "docker-http-basic-authenticator",
"requirement": "DISABLED",
"priority": 0,
"userSetupAllowed": false,
"autheticatorFlow": false
}
]
}
]
}



0 comments on commit afea1c2

Please sign in to comment.