From c2b5bd9dc6720b2ad8e90ba1a36bf3dda7e1dd09 Mon Sep 17 00:00:00 2001 From: Assah Bismark <134822946+AssahBismarkabah@users.noreply.github.com> Date: Fri, 15 Nov 2024 12:19:00 +0100 Subject: [PATCH] Revert "Fix to update user groups" --- CHANGELOG.md | 4 ---- .../config/service/UserImportService.java | 6 +---- src/main/resources/application.properties | 2 +- .../config/service/ImportGroupsIT.java | 4 ++-- .../service/ImportUserFederationIT.java | 6 ++--- .../config/service/ImportUsersIT.java | 22 +++++++++---------- ...update_realm_update_user_remove_group.json | 3 +-- ...update_realm_update_user_add_subgroup.json | 4 +--- ...ate_realm_update_user_change_subgroup.json | 4 +--- ...ate_realm_update_user_remove_subgroup.json | 3 +-- 10 files changed, 22 insertions(+), 36 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index c958b11b8..eb63e78d4 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -6,15 +6,11 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/), ## [Unreleased] -### Fixed -- Fix to update user groups [#1132](https://github.com/adorsys/keycloak-config-cli/issues/1132) - ## Fixed - otpPolicyAlgorithm ignored during import [#847](https://github.com/adorsys/keycloak-config-cli/issues/847) ### Added - Added Navigation in the readme [#1099](https://github.com/adorsys/keycloak-config-cli/issues/1099) - ### Added - improved logging for realm retrieval errors [#1010](https://github.com/adorsys/keycloak-config-cli/issues/1010) ### Fixed diff --git a/src/main/java/de/adorsys/keycloak/config/service/UserImportService.java b/src/main/java/de/adorsys/keycloak/config/service/UserImportService.java index e31d99738..595805013 100644 --- a/src/main/java/de/adorsys/keycloak/config/service/UserImportService.java +++ b/src/main/java/de/adorsys/keycloak/config/service/UserImportService.java @@ -183,11 +183,7 @@ private void handleGroups() { .toList(); handleGroupsToBeAdded(userGroupsToUpdate, existingUserGroups); - - if (importConfigProperties.getManaged().getGroup() - == ImportConfigProperties.ImportManagedProperties.ImportManagedPropertiesValues.FULL) { - handleGroupsToBeRemoved(userGroupsToUpdate, existingUserGroups); - } + handleGroupsToBeRemoved(userGroupsToUpdate, existingUserGroups); } private void handleGroupsToBeAdded( diff --git a/src/main/resources/application.properties b/src/main/resources/application.properties index 62ad01145..031fc6bbd 100644 --- a/src/main/resources/application.properties +++ b/src/main/resources/application.properties @@ -37,7 +37,7 @@ import.behaviors.sync-user-federation=false import.behaviors.checksum-with-cache-key=true import.behaviors.checksum-changed=continue import.managed.authentication-flow=full -import.managed.group=no-delete +import.managed.group=full import.managed.required-action=full import.managed.client-scope=full import.managed.scope-mapping=full diff --git a/src/test/java/de/adorsys/keycloak/config/service/ImportGroupsIT.java b/src/test/java/de/adorsys/keycloak/config/service/ImportGroupsIT.java index f1d7a9dce..f86edfd3d 100644 --- a/src/test/java/de/adorsys/keycloak/config/service/ImportGroupsIT.java +++ b/src/test/java/de/adorsys/keycloak/config/service/ImportGroupsIT.java @@ -1689,7 +1689,7 @@ void shouldUpdateRealmUpdateGroupWithSubstringOfExistingGroupName() throws IOExc @Test @Order(78) - void shouldUpdateRealmAddedGroup() throws IOException { + void shouldUpdateRealmDeleteGroup() throws IOException { GroupRepresentation updatedGroup = tryToLoadGroup("/My Added Group").get(); assertThat(updatedGroup.getName(), Matchers.is(Matchers.equalTo("My Added Group"))); @@ -1702,7 +1702,7 @@ void shouldUpdateRealmAddedGroup() throws IOException { assertThat(realm.getRealm(), is(REALM_NAME)); - assertThat(tryToLoadGroup("/My Added Group").isPresent(), is(true)); + assertThat(tryToLoadGroup("/My Added Group").isPresent(), is(false)); assertThat(tryToLoadGroup("/My Group").isPresent(), is(true)); } diff --git a/src/test/java/de/adorsys/keycloak/config/service/ImportUserFederationIT.java b/src/test/java/de/adorsys/keycloak/config/service/ImportUserFederationIT.java index b3243f3e3..64da1e3a4 100644 --- a/src/test/java/de/adorsys/keycloak/config/service/ImportUserFederationIT.java +++ b/src/test/java/de/adorsys/keycloak/config/service/ImportUserFederationIT.java @@ -159,7 +159,7 @@ void importFederationChangeUserGroupWithReadonlyProvider() throws IOException { assertThat(user.getFirstName(), is("James")); List userGroups = getGroupsByUser(user); - assertThat(userGroups, hasSize(2)); + assertThat(userGroups, hasSize(1)); GroupRepresentation group = getGroupsByPath(userGroups, "/realm/group2"); assertThat(group, is(notNullValue())); @@ -182,7 +182,7 @@ void importFederationRemoveUserGroupWithReadonlyProvider() throws IOException { assertThat(user.getFirstName(), is("James")); List userGroups = getGroupsByUser(user); - assertThat(userGroups, hasSize(2)); + assertThat(userGroups, hasSize(0)); } @Test @@ -201,7 +201,7 @@ void importFederationUserChangeAttributeWithReadonlyProvider() throws IOExceptio assertThat(user.getFirstName(), is("James")); List userGroups = getGroupsByUser(user); - assertThat(userGroups, hasSize(2)); + assertThat(userGroups, hasSize(0)); } private List getGroupsByUser(UserRepresentation user) { diff --git a/src/test/java/de/adorsys/keycloak/config/service/ImportUsersIT.java b/src/test/java/de/adorsys/keycloak/config/service/ImportUsersIT.java index 4792423f3..4d469e1b0 100644 --- a/src/test/java/de/adorsys/keycloak/config/service/ImportUsersIT.java +++ b/src/test/java/de/adorsys/keycloak/config/service/ImportUsersIT.java @@ -400,7 +400,8 @@ void shouldUpdateRealmUpdateUserChangeGroup() throws IOException { @Test @Order(11) - void shouldUpdateRealmUpdateUserKeepExistingGroups() throws IOException { + void shouldUpdateRealmUpdateUserRemoveGroup() throws IOException { + // Create Users doImport("11_update_realm_update_user_remove_group.json"); final RealmRepresentation realm = keycloakProvider.getInstance().realm(REALM_NAME).toRepresentation(); @@ -413,10 +414,10 @@ void shouldUpdateRealmUpdateUserKeepExistingGroups() throws IOException { assertThat(user.getFirstName(), is("firstName1")); List userGroups = getGroupsByUser(user); - assertThat(userGroups, hasSize(2)); // User should still be in both groups + assertThat(userGroups, hasSize(1)); GroupRepresentation group1 = getGroupsByPath(userGroups, "/group1"); - assertThat(group1.getName(), is("group1")); + assertThat(group1, nullValue()); GroupRepresentation group2 = getGroupsByPath(userGroups, "/group2"); assertThat(group2.getName(), is("group2")); @@ -438,17 +439,16 @@ void shouldUpdateRealmUpdateUserAddSubGroup() throws IOException { assertThat(user.getFirstName(), is("firstName1")); List userGroups = getGroupsByUser(user); - assertThat(userGroups, hasSize(3)); // User should now be in all groups + assertThat(userGroups, hasSize(1)); GroupRepresentation group1 = getGroupsByPath(userGroups, "/group1/subgroup1"); assertThat(group1.getName(), is("subgroup1")); - GroupRepresentation group2 = getGroupsByPath(userGroups, "/group2"); - assertThat(group2.getName(), is("group2")); } @Test @Order(13) - void shouldUpdateRealmUpdateUserAddNewSubGroup() throws IOException { + void shouldUpdateRealmUpdateUserChangeSubGroup() throws IOException { + // Create Users doImport("13_update_realm_update_user_change_subgroup.json"); final RealmRepresentation realm = keycloakProvider.getInstance().realm(REALM_NAME).toRepresentation(); @@ -461,7 +461,7 @@ void shouldUpdateRealmUpdateUserAddNewSubGroup() throws IOException { assertThat(user.getFirstName(), is("firstName1")); List userGroups = getGroupsByUser(user); - assertThat(userGroups, hasSize(4)); // User should now be in all groups + assertThat(userGroups, hasSize(2)); GroupRepresentation group1 = getGroupsByPath(userGroups, "/group1/subgroup1"); assertThat(group1.getName(), is("subgroup1")); @@ -472,7 +472,7 @@ void shouldUpdateRealmUpdateUserAddNewSubGroup() throws IOException { @Test @Order(14) - void shouldUpdateRealmUpdateUserKeepExistingSubGroups() throws IOException { + void shouldUpdateRealmUpdateUserRemoveSubGroup() throws IOException { doImport("14_update_realm_update_user_remove_subgroup.json"); final RealmRepresentation realm = keycloakProvider.getInstance().realm(REALM_NAME).toRepresentation(); @@ -485,10 +485,10 @@ void shouldUpdateRealmUpdateUserKeepExistingSubGroups() throws IOException { assertThat(user.getFirstName(), is("firstName1")); List userGroups = getGroupsByUser(user); - assertThat(userGroups, hasSize(4)); + assertThat(userGroups, hasSize(1)); GroupRepresentation group1 = getGroupsByPath(userGroups, "/group1/subgroup1"); - assertThat(group1.getName(), is("subgroup1")); + assertThat(group1, nullValue()); GroupRepresentation group2 = getGroupsByPath(userGroups, "/group2/subgroup2"); assertThat(group2.getName(), is("subgroup2")); diff --git a/src/test/resources/import-files/users/11_update_realm_update_user_remove_group.json b/src/test/resources/import-files/users/11_update_realm_update_user_remove_group.json index 4b3110d2d..6caf4ec46 100644 --- a/src/test/resources/import-files/users/11_update_realm_update_user_remove_group.json +++ b/src/test/resources/import-files/users/11_update_realm_update_user_remove_group.json @@ -17,8 +17,7 @@ "firstName": "firstName1", "lastName": "lastName1", "groups": [ - "group2", - "group1" + "group2" ] }, { diff --git a/src/test/resources/import-files/users/12_update_realm_update_user_add_subgroup.json b/src/test/resources/import-files/users/12_update_realm_update_user_add_subgroup.json index 9480f6cb0..5e615d170 100644 --- a/src/test/resources/import-files/users/12_update_realm_update_user_add_subgroup.json +++ b/src/test/resources/import-files/users/12_update_realm_update_user_add_subgroup.json @@ -27,9 +27,7 @@ "firstName": "firstName1", "lastName": "lastName1", "groups": [ - "/group1/subgroup1", - "/group1", - "/group2" + "/group1/subgroup1" ] }, { diff --git a/src/test/resources/import-files/users/13_update_realm_update_user_change_subgroup.json b/src/test/resources/import-files/users/13_update_realm_update_user_change_subgroup.json index 212381326..660056171 100644 --- a/src/test/resources/import-files/users/13_update_realm_update_user_change_subgroup.json +++ b/src/test/resources/import-files/users/13_update_realm_update_user_change_subgroup.json @@ -28,9 +28,7 @@ "lastName": "lastName1", "groups": [ "/group1/subgroup1", - "/group2/subgroup2", - "/group1", - "/group2" + "/group2/subgroup2" ] }, { diff --git a/src/test/resources/import-files/users/14_update_realm_update_user_remove_subgroup.json b/src/test/resources/import-files/users/14_update_realm_update_user_remove_subgroup.json index 3bb5aa282..aac75846b 100644 --- a/src/test/resources/import-files/users/14_update_realm_update_user_remove_subgroup.json +++ b/src/test/resources/import-files/users/14_update_realm_update_user_remove_subgroup.json @@ -27,8 +27,7 @@ "firstName": "firstName1", "lastName": "lastName1", "groups": [ - "/group2/subgroup2", - "/group1/subgroup1" + "/group2/subgroup2" ] }, {