From 8573ea5fb210660600ca9fa162a2dc307dd7c5b0 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Martin=20Barto=C5=A1?= Date: Thu, 8 Apr 2021 16:06:06 +0200 Subject: [PATCH] KEYCLOAK-17690 Add missing test case for user email update --- .../keycloak/testsuite/admin/UserTest.java | 226 +++++++++++------- 1 file changed, 139 insertions(+), 87 deletions(-) diff --git a/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/admin/UserTest.java b/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/admin/UserTest.java index 7483244851..08d832c62d 100755 --- a/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/admin/UserTest.java +++ b/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/admin/UserTest.java @@ -153,10 +153,10 @@ public class UserTest extends AbstractAdminTest { @After public void after() { - realm.identityProviders().findAll().stream() + realm.identityProviders().findAll() .forEach(ip -> realm.identityProviders().get(ip.getAlias()).remove()); - realm.groups().groups().stream() + realm.groups().groups() .forEach(g -> realm.groups().group(g.getId()).remove()); } @@ -179,9 +179,10 @@ public class UserTest extends AbstractAdminTest { } private String createUser(UserRepresentation userRep, boolean assertAdminEvent) { - Response response = realm.users().create(userRep); - String createdId = ApiUtil.getCreatedId(response); - response.close(); + final String createdId; + try (Response response = realm.users().create(userRep)) { + createdId = ApiUtil.getCreatedId(response); + } if (assertAdminEvent) { assertAdminEvents.assertEvent(realmId, OperationType.CREATE, AdminEventPaths.userResourcePath(createdId), userRep, @@ -236,15 +237,14 @@ public class UserTest extends AbstractAdminTest { UserRepresentation user = new UserRepresentation(); user.setUsername("user1"); - Response response = realm.users().create(user); - assertEquals(409, response.getStatus()); - assertAdminEvents.assertEmpty(); + try (Response response = realm.users().create(user)) { + assertEquals(409, response.getStatus()); + assertAdminEvents.assertEmpty(); - // Just to show how to retrieve underlying error message - ErrorRepresentation error = response.readEntity(ErrorRepresentation.class); - Assert.assertEquals("User exists with same username", error.getErrorMessage()); - - response.close(); + // Just to show how to retrieve underlying error message + ErrorRepresentation error = response.readEntity(ErrorRepresentation.class); + Assert.assertEquals("User exists with same username", error.getErrorMessage()); + } } @Test @@ -254,14 +254,14 @@ public class UserTest extends AbstractAdminTest { UserRepresentation user = new UserRepresentation(); user.setUsername("user2"); user.setEmail("user1@localhost"); - Response response = realm.users().create(user); - assertEquals(409, response.getStatus()); - assertAdminEvents.assertEmpty(); - ErrorRepresentation error = response.readEntity(ErrorRepresentation.class); - Assert.assertEquals("User exists with same email", error.getErrorMessage()); + try (Response response = realm.users().create(user)) { + assertEquals(409, response.getStatus()); + assertAdminEvents.assertEmpty(); - response.close(); + ErrorRepresentation error = response.readEntity(ErrorRepresentation.class); + Assert.assertEquals("User exists with same email", error.getErrorMessage()); + } } //KEYCLOAK-14611 @@ -286,11 +286,14 @@ public class UserTest extends AbstractAdminTest { //Create a third user with the same email user.setUsername("user3"); - Response response = realm.users().create(user); - assertEquals(409, response.getStatus()); - ErrorRepresentation error = response.readEntity(ErrorRepresentation.class); - Assert.assertEquals("User exists with same email", error.getErrorMessage()); - response.close(); + assertAdminEvents.clear(); + + try (Response response = realm.users().create(user)) { + assertEquals(409, response.getStatus()); + ErrorRepresentation error = response.readEntity(ErrorRepresentation.class); + Assert.assertEquals("User exists with same email", error.getErrorMessage()); + assertAdminEvents.assertEmpty(); + } } @Test @@ -443,9 +446,11 @@ public class UserTest extends AbstractAdminTest { UserRepresentation user = new UserRepresentation(); user.setUsername("User1"); - Response response = realm.users().create(user); - assertEquals(409, response.getStatus()); - response.close(); + + try (Response response = realm.users().create(user)) { + assertEquals(409, response.getStatus()); + assertAdminEvents.assertEmpty(); + } } @Test @@ -454,9 +459,11 @@ public class UserTest extends AbstractAdminTest { UserRepresentation user = new UserRepresentation(); user.setUsername("USER1"); - Response response = realm.users().create(user); - assertEquals(409, response.getStatus()); - response.close(); + + try (Response response = realm.users().create(user)) { + assertEquals(409, response.getStatus()); + assertAdminEvents.assertEmpty(); + } } @Test @@ -466,9 +473,11 @@ public class UserTest extends AbstractAdminTest { UserRepresentation user = new UserRepresentation(); user.setUsername("user2"); user.setEmail("User1@localhost"); - Response response = realm.users().create(user); - assertEquals(409, response.getStatus()); - response.close(); + + try (Response response = realm.users().create(user)) { + assertEquals(409, response.getStatus()); + assertAdminEvents.assertEmpty(); + } } @Test @@ -478,9 +487,11 @@ public class UserTest extends AbstractAdminTest { UserRepresentation user = new UserRepresentation(); user.setUsername("user2"); user.setEmail("user1@LOCALHOST"); - Response response = realm.users().create(user); - assertEquals(409, response.getStatus()); - response.close(); + + try (Response response = realm.users().create(user)) { + assertEquals(409, response.getStatus()); + assertAdminEvents.assertEmpty(); + } } @Test @@ -490,12 +501,11 @@ public class UserTest extends AbstractAdminTest { UserRepresentation user = new UserRepresentation(); user.setUsername("user2"); user.setEmail("user1@localhost"); - Response response = realm.users().create(user); - assertEquals(409, response.getStatus()); - response.close(); - - assertAdminEvents.assertEmpty(); + try (Response response = realm.users().create(user)) { + assertEquals(409, response.getStatus()); + assertAdminEvents.assertEmpty(); + } } // KEYCLOAK-7015 @@ -535,11 +545,13 @@ public class UserTest extends AbstractAdminTest { public void createUserWithoutUsername() { UserRepresentation user = new UserRepresentation(); user.setEmail("user1@localhost"); - Response response = realm.users().create(user); - assertEquals(400, response.getStatus()); - ErrorRepresentation error = response.readEntity(ErrorRepresentation.class); - Assert.assertEquals("User name is missing", error.getErrorMessage()); - response.close(); + + try (Response response = realm.users().create(user)) { + assertEquals(400, response.getStatus()); + ErrorRepresentation error = response.readEntity(ErrorRepresentation.class); + Assert.assertEquals("User name is missing", error.getErrorMessage()); + assertAdminEvents.assertEmpty(); + } } @Test @@ -559,11 +571,13 @@ public class UserTest extends AbstractAdminTest { UserRepresentation user = new UserRepresentation(); user.setUsername(""); user.setEmail("user2@localhost"); - Response response = realm.users().create(user); - assertEquals(400, response.getStatus()); - ErrorRepresentation error = response.readEntity(ErrorRepresentation.class); - Assert.assertEquals("User name is missing", error.getErrorMessage()); - response.close(); + + try (Response response = realm.users().create(user)) { + assertEquals(400, response.getStatus()); + ErrorRepresentation error = response.readEntity(ErrorRepresentation.class); + Assert.assertEquals("User name is missing", error.getErrorMessage()); + assertAdminEvents.assertEmpty(); + } } @Test @@ -578,14 +592,17 @@ public class UserTest extends AbstractAdminTest { CredentialRepresentation rawPassword = new CredentialRepresentation(); rawPassword.setValue("ABCD"); rawPassword.setType(CredentialRepresentation.PASSWORD); - user.setCredentials(Arrays.asList(rawPassword)); - Response response = realm.users().create(user); - assertEquals(400, response.getStatus()); - ErrorRepresentation error = response.readEntity(ErrorRepresentation.class); - Assert.assertEquals("Password policy not met", error.getErrorMessage()); - rep.setPasswordPolicy(passwordPolicy); - realm.update(rep); - response.close(); + user.setCredentials(Collections.singletonList(rawPassword)); + assertAdminEvents.clear(); + + try (Response response = realm.users().create(user)) { + assertEquals(400, response.getStatus()); + ErrorRepresentation error = response.readEntity(ErrorRepresentation.class); + Assert.assertEquals("Password policy not met", error.getErrorMessage()); + rep.setPasswordPolicy(passwordPolicy); + assertAdminEvents.assertEmpty(); + realm.update(rep); + } } private List createUsers() { @@ -1075,17 +1092,17 @@ public class UserTest extends AbstractAdminTest { @Test public void delete() { String userId = createUser(); - Response response = realm.users().delete(userId); - assertEquals(204, response.getStatus()); - response.close(); + try (Response response = realm.users().delete(userId)) { + assertEquals(204, response.getStatus()); + } assertAdminEvents.assertEvent(realmId, OperationType.DELETE, AdminEventPaths.userResourcePath(userId), ResourceType.USER); } @Test public void deleteNonExistent() { - Response response = realm.users().delete("does-not-exist"); - assertEquals(404, response.getStatus()); - response.close(); + try (Response response = realm.users().delete("does-not-exist")) { + assertEquals(404, response.getStatus()); + } assertAdminEvents.assertEmpty(); } @@ -1285,6 +1302,7 @@ public class UserTest extends AbstractAdminTest { Assert.fail("Not supposed to successfully update user"); } catch (BadRequestException bre) { // Expected + assertAdminEvents.assertEmpty(); } // The same test as before, but with the case-sensitivity used @@ -1295,6 +1313,7 @@ public class UserTest extends AbstractAdminTest { Assert.fail("Not supposed to successfully update user"); } catch (BadRequestException bre) { // Expected + assertAdminEvents.assertEmpty(); } // Attribute "deniedSomeAdmin" was denied for administrator @@ -1305,6 +1324,7 @@ public class UserTest extends AbstractAdminTest { Assert.fail("Not supposed to successfully update user"); } catch (BadRequestException bre) { // Expected + assertAdminEvents.assertEmpty(); } // usercertificate and saml attribute are allowed by admin @@ -1352,6 +1372,7 @@ public class UserTest extends AbstractAdminTest { ErrorRepresentation error = e.getResponse().readEntity(ErrorRepresentation.class); Assert.assertEquals("User email missing", error.getErrorMessage()); + assertAdminEvents.assertEmpty(); } try { userRep = user.toRepresentation(); @@ -1366,6 +1387,7 @@ public class UserTest extends AbstractAdminTest { ErrorRepresentation error = e.getResponse().readEntity(ErrorRepresentation.class); Assert.assertEquals("User is disabled", error.getErrorMessage()); + assertAdminEvents.assertEmpty(); } try { userRep.setEnabled(true); @@ -1378,6 +1400,7 @@ public class UserTest extends AbstractAdminTest { ErrorRepresentation error = e.getResponse().readEntity(ErrorRepresentation.class); Assert.assertEquals("Client doesn't exist", error.getErrorMessage()); + assertAdminEvents.assertEmpty(); } } @@ -1416,7 +1439,7 @@ public class UserTest extends AbstractAdminTest { driver.navigate().to(link); proceedPage.assertCurrent(); - Assert.assertThat(proceedPage.getInfo(), Matchers.containsString("Update Password")); + assertThat(proceedPage.getInfo(), Matchers.containsString("Update Password")); proceedPage.clickProceedLink(); passwordUpdatePage.assertCurrent(); @@ -1476,7 +1499,7 @@ public class UserTest extends AbstractAdminTest { driver.navigate().to(link); proceedPage.assertCurrent(); - Assert.assertThat(proceedPage.getInfo(), Matchers.containsString("Update Password")); + assertThat(proceedPage.getInfo(), Matchers.containsString("Update Password")); proceedPage.clickProceedLink(); passwordUpdatePage.assertCurrent(); @@ -1516,7 +1539,7 @@ public class UserTest extends AbstractAdminTest { driver.navigate().to(link); proceedPage.assertCurrent(); - Assert.assertThat(proceedPage.getInfo(), Matchers.containsString("Update Password")); + assertThat(proceedPage.getInfo(), Matchers.containsString("Update Password")); proceedPage.clickProceedLink(); passwordUpdatePage.assertCurrent(); @@ -1562,7 +1585,7 @@ public class UserTest extends AbstractAdminTest { driver.navigate().to(link); proceedPage.assertCurrent(); - Assert.assertThat(proceedPage.getInfo(), Matchers.containsString("Update Password")); + assertThat(proceedPage.getInfo(), Matchers.containsString("Update Password")); proceedPage.clickProceedLink(); passwordUpdatePage.assertCurrent(); @@ -1604,7 +1627,7 @@ public class UserTest extends AbstractAdminTest { driver.navigate().to(link); proceedPage.assertCurrent(); - Assert.assertThat(proceedPage.getInfo(), Matchers.containsString("Update Password")); + assertThat(proceedPage.getInfo(), Matchers.containsString("Update Password")); proceedPage.clickProceedLink(); passwordUpdatePage.assertCurrent(); @@ -1614,7 +1637,7 @@ public class UserTest extends AbstractAdminTest { driver.navigate().to(link); proceedPage.assertCurrent(); - Assert.assertThat(proceedPage.getInfo(), Matchers.containsString("Update Password")); + assertThat(proceedPage.getInfo(), Matchers.containsString("Update Password")); proceedPage.clickProceedLink(); passwordUpdatePage.assertCurrent(); @@ -1718,7 +1741,7 @@ public class UserTest extends AbstractAdminTest { driver.navigate().to(link); proceedPage.assertCurrent(); - Assert.assertThat(proceedPage.getInfo(), Matchers.containsString("Update Password")); + assertThat(proceedPage.getInfo(), Matchers.containsString("Update Password")); proceedPage.clickProceedLink(); passwordUpdatePage.assertCurrent(); @@ -1782,7 +1805,7 @@ public class UserTest extends AbstractAdminTest { driver.navigate().to(link); proceedPage.assertCurrent(); - Assert.assertThat(proceedPage.getInfo(), Matchers.containsString("Update Password")); + assertThat(proceedPage.getInfo(), Matchers.containsString("Update Password")); proceedPage.clickProceedLink(); passwordUpdatePage.assertCurrent(); @@ -1867,7 +1890,7 @@ public class UserTest extends AbstractAdminTest { driver.navigate().to(link); proceedPage.assertCurrent(); - Assert.assertThat(proceedPage.getInfo(), Matchers.containsString("Update Password")); + assertThat(proceedPage.getInfo(), Matchers.containsString("Update Password")); proceedPage.clickProceedLink(); passwordUpdatePage.assertCurrent(); @@ -1918,6 +1941,7 @@ public class UserTest extends AbstractAdminTest { ErrorRepresentation error = e.getResponse().readEntity(ErrorRepresentation.class); Assert.assertEquals("User is disabled", error.getErrorMessage()); + assertAdminEvents.assertEmpty(); } try { userRep.setEnabled(true); @@ -1930,6 +1954,7 @@ public class UserTest extends AbstractAdminTest { ErrorRepresentation error = e.getResponse().readEntity(ErrorRepresentation.class); Assert.assertEquals("Client doesn't exist", error.getErrorMessage()); + assertAdminEvents.assertEmpty(); } user.sendVerifyEmail(); @@ -1942,7 +1967,7 @@ public class UserTest extends AbstractAdminTest { driver.navigate().to(link); proceedPage.assertCurrent(); - Assert.assertThat(proceedPage.getInfo(), Matchers.containsString("Verify Email")); + assertThat(proceedPage.getInfo(), Matchers.containsString("Verify Email")); proceedPage.clickProceedLink(); Assert.assertEquals("Your account has been updated.", infoPage.getInfo()); @@ -1950,7 +1975,7 @@ public class UserTest extends AbstractAdminTest { driver.navigate().to(link); // It should be possible to use the same action token multiple times proceedPage.assertCurrent(); - Assert.assertThat(proceedPage.getInfo(), Matchers.containsString("Verify Email")); + assertThat(proceedPage.getInfo(), Matchers.containsString("Verify Email")); proceedPage.clickProceedLink(); Assert.assertEquals("Your account has been updated.", infoPage.getInfo()); } @@ -2022,6 +2047,30 @@ public class UserTest extends AbstractAdminTest { switchRegistrationEmailAsUsername(false); } + @Test + public void updateUserWithExistingEmail() { + final String userId = createUser(); + assertNotNull(userId); + assertNotNull(createUser("user2", "user2@localhost")); + + UserResource user = realm.users().get(userId); + UserRepresentation userRep = user.toRepresentation(); + assertNotNull(userRep); + userRep.setEmail("user2@localhost"); + + try { + updateUser(user, userRep); + fail("Expected failure - Email conflict"); + } catch (ClientErrorException e) { + assertNotNull(e.getResponse()); + assertThat(e.getResponse().getStatus(), is(409)); + + ErrorRepresentation error = e.getResponse().readEntity(ErrorRepresentation.class); + Assert.assertEquals("User exists with same username or email", error.getErrorMessage()); + assertAdminEvents.assertEmpty(); + } + } + @Test public void updateUserWithNewUsernameNotPossible() { String id = createUser(); @@ -2050,6 +2099,7 @@ public class UserTest extends AbstractAdminTest { fail("Expected failure"); } catch (ClientErrorException e) { assertEquals(404, e.getResponse().getStatus()); + assertAdminEvents.assertEmpty(); } finally { switchEditUsernameAllowedOn(false); } @@ -2212,10 +2262,10 @@ public class UserTest extends AbstractAdminTest { realm.roles().create(RoleBuilder.create().name("realm-child").build()); realm.roles().get("realm-composite").addComposites(Collections.singletonList(realm.roles().get("realm-child").toRepresentation())); - - Response response = realm.clients().create(ClientBuilder.create().clientId("myclient").build()); - String clientUuid = ApiUtil.getCreatedId(response); - response.close(); + final String clientUuid; + try (Response response = realm.clients().create(ClientBuilder.create().clientId("myclient").build())) { + clientUuid = ApiUtil.getCreatedId(response); + } RoleRepresentation clientCompositeRole = RoleBuilder.create().name("client-composite").singleAttribute("attribute1", "value1").build(); @@ -2226,9 +2276,10 @@ public class UserTest extends AbstractAdminTest { realm.clients().get(clientUuid).roles().create(RoleBuilder.create().name("client-child").build()); realm.clients().get(clientUuid).roles().get("client-composite").addComposites(Collections.singletonList(realm.clients().get(clientUuid).roles().get("client-child").toRepresentation())); - response = realm.users().create(UserBuilder.create().username("myuser").build()); - String userId = ApiUtil.getCreatedId(response); - response.close(); + final String userId; + try (Response response = realm.users().create(UserBuilder.create().username("myuser").build())) { + userId = ApiUtil.getCreatedId(response); + } // Admin events for creating role, client or user tested already in other places assertAdminEvents.clear(); @@ -2744,10 +2795,11 @@ public class UserTest extends AbstractAdminTest { } private GroupRepresentation createGroup(RealmResource realm, GroupRepresentation group) { - Response response = realm.groups().add(group); - String groupId = ApiUtil.getCreatedId(response); - getCleanup().addGroupId(groupId); - response.close(); + final String groupId; + try (Response response = realm.groups().add(group)) { + groupId = ApiUtil.getCreatedId(response); + getCleanup().addGroupId(groupId); + } assertAdminEvents.assertEvent(realmId, OperationType.CREATE, AdminEventPaths.groupPath(groupId), group, ResourceType.GROUP);