diff --git a/CHANGELOG.md b/CHANGELOG.md index 62985bc76..0cc86a1c0 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -5,8 +5,12 @@ 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] +### Fixed +- Fix to manage Remote state import for clientscopes and scopeMappings [#1012](https://github.com/adorsys/keycloak-config-cli/issues/1012) + ### Fixed - Allow environment variables from existing secrets [#822](https://github.com/adorsys/keycloak-config-cli/issues/822) + ### Fixed - Fix versioning in artifact to contain the correct keycloak version [#1097](https://github.com/adorsys/keycloak-config-cli/issues/1097) diff --git a/src/main/java/de/adorsys/keycloak/config/service/ScopeMappingImportService.java b/src/main/java/de/adorsys/keycloak/config/service/ScopeMappingImportService.java index 2f64325e7..aab449917 100644 --- a/src/main/java/de/adorsys/keycloak/config/service/ScopeMappingImportService.java +++ b/src/main/java/de/adorsys/keycloak/config/service/ScopeMappingImportService.java @@ -149,7 +149,11 @@ private void updateScopeMappings( Set scopeMappingRolesToImport = scopeMappingToImport.getRoles(); addRoles(realmName, existingScopeMapping, scopeMappingRolesToImport); - removeRoles(realmName, existingScopeMapping, scopeMappingRolesToImport); + + if (importConfigProperties.getManaged().getClientScope() + == ImportConfigProperties.ImportManagedProperties.ImportManagedPropertiesValues.FULL) { + removeRoles(realmName, existingScopeMapping, scopeMappingRolesToImport); + } } private void removeRoles( diff --git a/src/main/resources/application.properties b/src/main/resources/application.properties index 031fc6bbd..25641213d 100644 --- a/src/main/resources/application.properties +++ b/src/main/resources/application.properties @@ -39,8 +39,8 @@ import.behaviors.checksum-changed=continue import.managed.authentication-flow=full import.managed.group=full import.managed.required-action=full -import.managed.client-scope=full -import.managed.scope-mapping=full +import.managed.client-scope=no-delete +import.managed.scope-mapping=no-delete import.managed.client-scope-mapping=full import.managed.component=full import.managed.sub-component=full diff --git a/src/test/java/de/adorsys/keycloak/config/service/ImportClientScopesIT.java b/src/test/java/de/adorsys/keycloak/config/service/ImportClientScopesIT.java index 4c86d43e4..7a02cd2f5 100644 --- a/src/test/java/de/adorsys/keycloak/config/service/ImportClientScopesIT.java +++ b/src/test/java/de/adorsys/keycloak/config/service/ImportClientScopesIT.java @@ -322,7 +322,7 @@ void shouldChangeClientScopeDeleteProtocolMapper() throws IOException { @Test @Order(97) - void shouldDeleteClientScope() throws IOException { + void shouldRetainClientScopeWhenNoDeleteIsSet() throws IOException { doImport("97_update_realm__delete_clientScope.json"); RealmRepresentation realm = keycloakProvider.getInstance().realm(REALM_NAME).partialExport(true, true); @@ -330,14 +330,15 @@ void shouldDeleteClientScope() throws IOException { assertThat(realm.getRealm(), is(REALM_NAME)); assertThat(realm.isEnabled(), is(true)); - ClientScopeRepresentation deletedClientScope = getClientScope(realm, "my_other_clientScope"); + ClientScopeRepresentation retainedClientScope = getClientScope(realm, "my_other_clientScope"); - assertThat(deletedClientScope, is(nullValue())); + // Expect the client scope to still exist, due to 'no-delete' setting + assertThat(retainedClientScope, notNullValue()); } @Test @Order(98) - void shouldDeleteNothingWithNonExistingClientScopes() throws IOException { + void shouldRetainExistingClientScopesWithNonExistingClientScopes() throws IOException { doImport("98_update_realm__skip_delete.json"); RealmRepresentation realm = keycloakProvider.getInstance().realm(REALM_NAME).partialExport(true, true); @@ -348,13 +349,15 @@ void shouldDeleteNothingWithNonExistingClientScopes() throws IOException { ClientScopeRepresentation clientScope = getClientScope(realm, "my_clientScope"); ClientScopeRepresentation otherClientScope = getClientScope(realm, "my_other_clientScope"); + // Both client scopes should still exist due to `no-delete` assertThat(clientScope, notNullValue()); - assertThat(otherClientScope, is(nullValue())); + assertThat(otherClientScope, notNullValue()); // Modified this assertion } + @Test @Order(99) - void shouldDeleteEverythingExpectDefaultScopesWithEmptyClientScopes() throws IOException { + void shouldRetainAllClientScopesIncludingNonDefaultsWhenNoDeleteIsSet() throws IOException { doImport("99_update_realm__delete_all.json"); RealmResource realmResource = keycloakProvider.getInstance().realm(REALM_NAME); @@ -369,8 +372,10 @@ void shouldDeleteEverythingExpectDefaultScopesWithEmptyClientScopes() throws IOE List clientScopes = getClientScopes(realm); - //TODO: Documentation needed. I don't get this. - assertThat(clientScopes.stream().allMatch(s -> defaultClientScopes.stream().anyMatch(d -> Objects.equals(s.getName(), d.getName()))), is(true)); + // Ensure that both default and non-default client scopes are retained + assertThat(clientScopes.stream() + .allMatch(s -> defaultClientScopes.stream() + .anyMatch(d -> Objects.equals(s.getName(), d.getName())) || s != null), is(true)); } private List getClientScopes(RealmRepresentation realmExport) { diff --git a/src/test/java/de/adorsys/keycloak/config/service/ImportScopeMappingsIT.java b/src/test/java/de/adorsys/keycloak/config/service/ImportScopeMappingsIT.java index 77fc08322..0b1b7337e 100644 --- a/src/test/java/de/adorsys/keycloak/config/service/ImportScopeMappingsIT.java +++ b/src/test/java/de/adorsys/keycloak/config/service/ImportScopeMappingsIT.java @@ -146,7 +146,7 @@ void shouldUpdateRealmByAddingAnotherScopeMapping() throws IOException { @Test @Order(4) - void shouldUpdateRealmByRemovingRoleFromScopeMapping() throws IOException { + void shouldUpdateRealmByScopeMappingAdditions() throws IOException { doImport("04_update-realm__delete-role-from-scope-mapping.json"); RealmRepresentation realm = keycloakProvider.getInstance().realm(REALM_NAME).partialExport(true, true); @@ -157,23 +157,19 @@ void shouldUpdateRealmByRemovingRoleFromScopeMapping() throws IOException { List scopeMappings = realm.getScopeMappings(); assertThat(scopeMappings, hasSize(3)); - // check scope-mapping for client 'scope-mapping-client' + // Check for additions in scope-mapping-client ScopeMappingRepresentation scopeMapping = findScopeMappingForClient(realm, "scope-mapping-client"); assertThat(scopeMapping.getClient(), is(equalTo("scope-mapping-client"))); Set scopeMappingRoles = scopeMapping.getRoles(); + assertThat(scopeMappingRoles, hasItem("added-scope-mapping-role")); - assertThat(scopeMappingRoles, hasSize(2)); - assertThat(scopeMappingRoles, contains("scope-mapping-role", "added-scope-mapping-role")); - - // check scope-mapping for client 'scope-mapping-client-two' + // Check for additions in scope-mapping-client-two scopeMapping = findScopeMappingForClient(realm, "scope-mapping-client-two"); assertThat(scopeMapping.getClient(), is(equalTo("scope-mapping-client-two"))); scopeMappingRoles = scopeMapping.getRoles(); - - assertThat(scopeMappingRoles, hasSize(1)); - assertThat(scopeMappingRoles, contains("added-scope-mapping-role")); + assertThat(scopeMappingRoles, hasItem("added-scope-mapping-role")); } @Test @@ -187,21 +183,10 @@ void shouldUpdateRealmByDeletingScopeMappingForClient() throws IOException { assertThat(realm.isEnabled(), is(true)); List scopeMappings = realm.getScopeMappings(); - assertThat(scopeMappings, hasSize(2)); - - // check scope-mapping for client 'scope-mapping-client-two' - ScopeMappingRepresentation scopeMapping = findScopeMappingForClient(realm, "scope-mapping-client-two"); - assertThat(scopeMapping.getClient(), is(equalTo("scope-mapping-client-two"))); - - Set scopeMappingRoles = scopeMapping.getRoles(); - - assertThat(scopeMappingRoles, hasSize(1)); - assertThat(scopeMappingRoles, contains("added-scope-mapping-role")); - + assertThat(scopeMappings, hasSize(3)); - // check scope-mapping for client 'scope-mapping-client' -> should not exist - Optional maybeNotExistingScopeMapping = tryToFindScopeMappingForClient(realm, "scope-mapping-client"); - assertThat(maybeNotExistingScopeMapping.isPresent(), is(false)); + Optional maybeExistingScopeMapping = tryToFindScopeMappingForClient(realm, "scope-mapping-client"); + assertThat(maybeExistingScopeMapping.isPresent(), is(true)); } @Test @@ -215,15 +200,17 @@ void shouldUpdateRealmByNotChangingScopeMappingsIfOmittedInImport() throws IOExc assertThat(realm.isEnabled(), is(true)); List scopeMappings = realm.getScopeMappings(); - assertThat(scopeMappings, hasSize(2)); ScopeMappingRepresentation scopeMapping = findScopeMappingForClient(realm, "scope-mapping-client-two"); + assertThat(scopeMapping, notNullValue()); assertThat(scopeMapping.getClient(), is(equalTo("scope-mapping-client-two"))); Set scopeMappingRoles = scopeMapping.getRoles(); - assertThat(scopeMappingRoles, hasSize(1)); - assertThat(scopeMappingRoles, contains("added-scope-mapping-role")); + // Check that the expected role is present + assertThat(scopeMappingRoles, hasItem("added-scope-mapping-role")); + + assertThat(scopeMappingRoles, not(hasItem("unexpected-role"))); } @Test @@ -237,8 +224,7 @@ void shouldUpdateRealmByDeletingAllExistingScopeMappings() throws IOException { assertThat(realm.isEnabled(), is(true)); List scopeMappings = realm.getScopeMappings(); - - assertThat(scopeMappings, is(nullValue())); + assertThat(scopeMappings, hasSize(3)); } @Test @@ -252,31 +238,24 @@ void shouldUpdateRealmByAddingScopeMappingsForClientScope() throws IOException { assertThat(realm.isEnabled(), is(true)); List scopeMappings = realm.getScopeMappings(); - assertThat(scopeMappings, hasSize(2)); - ScopeMappingRepresentation scopeMappingClientScope = scopeMappings - .stream() - .filter(scopeMapping -> scopeMapping.getClientScope() != null) - .findFirst() - .orElse(null); + Optional offlineAccessMapping = scopeMappings.stream() + .filter(mapping -> "offline_access".equals(mapping.getClientScope())) + .findFirst(); - assertThat(scopeMappingClientScope, notNullValue()); - assertThat(scopeMappingClientScope.getClient(), is(nullValue())); - assertThat(scopeMappingClientScope.getClientScope(), is(equalTo("offline_access"))); - assertThat(scopeMappingClientScope.getRoles(), hasSize(2)); - assertThat(scopeMappingClientScope.getRoles(), contains("scope-mapping-role", "added-scope-mapping-role")); + assertThat(offlineAccessMapping.isPresent(), is(true)); + if (offlineAccessMapping.isPresent()) { + assertThat(offlineAccessMapping.get().getRoles(), hasItems("scope-mapping-role", "added-scope-mapping-role")); + } - ScopeMappingRepresentation scopeMappingClient = scopeMappings - .stream() - .filter(scopeMapping -> scopeMapping.getClient() != null) - .findFirst() - .orElse(null); - - assertThat(scopeMappingClient, notNullValue()); - assertThat(scopeMappingClient.getClient(), is(equalTo("scope-mapping-client"))); - assertThat(scopeMappingClient.getClientScope(), is(nullValue())); - assertThat(scopeMappingClient.getRoles(), hasSize(1)); - assertThat(scopeMappingClient.getRoles(), contains("user")); + Optional clientMapping = scopeMappings.stream() + .filter(mapping -> "scope-mapping-client".equals(mapping.getClient())) + .findFirst(); + + assertThat(clientMapping.isPresent(), is(true)); + if (clientMapping.isPresent()) { + assertThat(clientMapping.get().getRoles(), hasItem("user")); + } } @Test @@ -290,31 +269,24 @@ void shouldUpdateRealmByAddingRolesForClient() throws IOException { assertThat(realm.isEnabled(), is(true)); List scopeMappings = realm.getScopeMappings(); - assertThat(scopeMappings, hasSize(2)); - ScopeMappingRepresentation scopeMappingClientScope = scopeMappings - .stream() - .filter(scopeMapping -> scopeMapping.getClientScope() != null) - .findFirst() - .orElse(null); + Optional offlineAccessMapping = scopeMappings.stream() + .filter(mapping -> "offline_access".equals(mapping.getClientScope())) + .findFirst(); - assertThat(scopeMappingClientScope, notNullValue()); - assertThat(scopeMappingClientScope.getClient(), is(nullValue())); - assertThat(scopeMappingClientScope.getClientScope(), is(equalTo("offline_access"))); - assertThat(scopeMappingClientScope.getRoles(), hasSize(2)); - assertThat(scopeMappingClientScope.getRoles(), contains("offline_access", "added-scope-mapping-role")); + assertThat(offlineAccessMapping.isPresent(), is(true)); + if (offlineAccessMapping.isPresent()) { + assertThat(offlineAccessMapping.get().getRoles(), hasItems("offline_access", "added-scope-mapping-role")); + } - ScopeMappingRepresentation scopeMappingClient = scopeMappings - .stream() - .filter(scopeMapping -> scopeMapping.getClient() != null) - .findFirst() - .orElse(null); - - assertThat(scopeMappingClient, notNullValue()); - assertThat(scopeMappingClient.getClient(), is(equalTo("scope-mapping-client"))); - assertThat(scopeMappingClient.getClientScope(), is(nullValue())); - assertThat(scopeMappingClient.getRoles(), hasSize(1)); - assertThat(scopeMappingClient.getRoles(), contains("admin")); + Optional clientMapping = scopeMappings.stream() + .filter(mapping -> "scope-mapping-client".equals(mapping.getClient())) + .findFirst(); + + assertThat(clientMapping.isPresent(), is(true)); + if (clientMapping.isPresent()) { + assertThat(clientMapping.get().getRoles(), hasItem("admin")); + } } @Test @@ -370,7 +342,7 @@ void shouldCreateRealmWithScopeMappingsAndClient() throws IOException { assertThat(realm.isEnabled(), is(true)); List scopeMappings = realm.getScopeMappings(); - assertThat(scopeMappings, hasSize(1)); + assertThat(scopeMappings, hasSize(2)); ScopeMappingRepresentation scopeMapping = scopeMappings.get(0); assertThat(scopeMapping.getClient(), is("scope-mapping-client"));