From 23bfaef4bb9482914fe3a980091496b9f379759a Mon Sep 17 00:00:00 2001 From: Denis Date: Tue, 13 Oct 2020 12:23:59 +0200 Subject: [PATCH] KEYCLOAK-15535 Account Log of user login with realm not available details when update profile --- .../java/org/keycloak/events/Details.java | 6 ++++++ .../forms/RegistrationUserCreation.java | 7 +++++-- .../requiredactions/UpdateProfile.java | 13 +++++++++++- .../resources/account/AccountFormService.java | 14 ++++++++++++- .../services/resources/account/Constants.java | 6 ++++++ .../account/AccountFormServiceTest.java | 9 +++++---- .../AppInitiatedActionUpdateProfileTest.java | 20 +++++++++++++------ .../RequiredActionMultipleActionsTest.java | 11 +++++----- .../actions/RequiredActionPriorityTest.java | 18 ++++++++++------- .../RequiredActionUpdateProfileTest.java | 18 ++++++++--------- 10 files changed, 85 insertions(+), 37 deletions(-) diff --git a/server-spi-private/src/main/java/org/keycloak/events/Details.java b/server-spi-private/src/main/java/org/keycloak/events/Details.java index 7eaedddf80..dac012cd95 100755 --- a/server-spi-private/src/main/java/org/keycloak/events/Details.java +++ b/server-spi-private/src/main/java/org/keycloak/events/Details.java @@ -37,6 +37,12 @@ public interface Details { String IDENTITY_PROVIDER_USERNAME = "identity_provider_identity"; String REGISTER_METHOD = "register_method"; String USERNAME = "username"; + String FIRST_NAME = "first_name"; + String LAST_NAME = "last_name"; + String PREVIOUS_FIRST_NAME = "previous_first_name"; + String UPDATED_FIRST_NAME = "updated_first_name"; + String PREVIOUS_LAST_NAME = "previous_last_name"; + String UPDATED_LAST_NAME = "updated_last_name"; String REMEMBER_ME = "remember_me"; String TOKEN_ID = "token_id"; String REFRESH_TOKEN_ID = "refresh_token_id"; diff --git a/services/src/main/java/org/keycloak/authentication/forms/RegistrationUserCreation.java b/services/src/main/java/org/keycloak/authentication/forms/RegistrationUserCreation.java index 9e0576a48a..85c45780a6 100755 --- a/services/src/main/java/org/keycloak/authentication/forms/RegistrationUserCreation.java +++ b/services/src/main/java/org/keycloak/authentication/forms/RegistrationUserCreation.java @@ -75,11 +75,14 @@ public class RegistrationUserCreation implements FormAction, FormActionFactory { UserProfile newProfile = result.getProfile(); String email = newProfile.getAttributes().getFirstAttribute(UserModel.EMAIL); + String username = newProfile.getAttributes().getFirstAttribute(UserModel.USERNAME); + String firstName = newProfile.getAttributes().getFirstAttribute(UserModel.FIRST_NAME); + String lastName = newProfile.getAttributes().getFirstAttribute(UserModel.LAST_NAME); context.getEvent().detail(Details.EMAIL, email); - String username = newProfile.getAttributes().getFirstAttribute(UserModel.USERNAME); - context.getEvent().detail(Details.USERNAME, username); + context.getEvent().detail(Details.FIRST_NAME, firstName); + context.getEvent().detail(Details.LAST_NAME, lastName); List errors = Validation.getFormErrorsFromValidation(result); if (context.getRealm().isRegistrationEmailAsUsername()) { diff --git a/services/src/main/java/org/keycloak/authentication/requiredactions/UpdateProfile.java b/services/src/main/java/org/keycloak/authentication/requiredactions/UpdateProfile.java index 19322a59da..7c6ba8ec9d 100644 --- a/services/src/main/java/org/keycloak/authentication/requiredactions/UpdateProfile.java +++ b/services/src/main/java/org/keycloak/authentication/requiredactions/UpdateProfile.java @@ -69,6 +69,9 @@ public class UpdateProfile implements RequiredActionProvider, RequiredActionFact event.event(EventType.UPDATE_PROFILE); MultivaluedMap formData = context.getHttpRequest().getDecodedFormParameters(); UserModel user = context.getUser(); + + String oldFirstName = user.getFirstName(); + String oldLastName = user.getLastName(); String oldEmail = user.getEmail(); UserProfileValidationResult result = forUpdateProfile(user, formData, context.getSession()).validate(); final UserProfile updatedProfile = result.getProfile(); @@ -84,11 +87,19 @@ public class UpdateProfile implements RequiredActionProvider, RequiredActionFact } String newEmail = updatedProfile.getAttributes().getFirstAttribute(UserModel.EMAIL); + String newFirstName = updatedProfile.getAttributes().getFirstAttribute(UserModel.FIRST_NAME); + String newLastName = updatedProfile.getAttributes().getFirstAttribute(UserModel.LAST_NAME); UserUpdateHelper.updateUserProfile(context.getRealm(), user, updatedProfile); + if (result.hasAttributeChanged(UserModel.FIRST_NAME)) { + event.detail(Details.PREVIOUS_FIRST_NAME, oldFirstName).detail(Details.UPDATED_FIRST_NAME, newFirstName); + } + if (result.hasAttributeChanged(UserModel.LAST_NAME)) { + event.detail(Details.PREVIOUS_LAST_NAME, oldLastName).detail(Details.UPDATED_LAST_NAME, newLastName); + } if (result.hasAttributeChanged(UserModel.EMAIL)) { user.setEmailVerified(false); - event.clone().event(EventType.UPDATE_EMAIL).detail(Details.PREVIOUS_EMAIL, oldEmail).detail(Details.UPDATED_EMAIL, newEmail).success(); + event.detail(Details.PREVIOUS_EMAIL, oldEmail).detail(Details.UPDATED_EMAIL, newEmail); } context.success(); diff --git a/services/src/main/java/org/keycloak/services/resources/account/AccountFormService.java b/services/src/main/java/org/keycloak/services/resources/account/AccountFormService.java index 120c222afd..1acc644814 100755 --- a/services/src/main/java/org/keycloak/services/resources/account/AccountFormService.java +++ b/services/src/main/java/org/keycloak/services/resources/account/AccountFormService.java @@ -364,6 +364,9 @@ public class AccountFormService extends AbstractSecuredLocalService { csrfCheck(formData); UserModel user = auth.getUser(); + + String oldFirstName = user.getFirstName(); + String oldLastName = user.getLastName(); String oldEmail = user.getEmail(); event.event(EventType.UPDATE_PROFILE).client(auth.getClient()).user(auth.getUser()); @@ -386,6 +389,9 @@ public class AccountFormService extends AbstractSecuredLocalService { UserProfile updatedProfile = result.getProfile(); String newEmail = updatedProfile.getAttributes().getFirstAttribute(UserModel.EMAIL); + String newFirstName = updatedProfile.getAttributes().getFirstAttribute(UserModel.FIRST_NAME); + String newLastName = updatedProfile.getAttributes().getFirstAttribute(UserModel.LAST_NAME); + try { // backward compatibility with old account console where attributes are not removed if missing @@ -395,9 +401,15 @@ public class AccountFormService extends AbstractSecuredLocalService { return account.setError(Response.Status.BAD_REQUEST, Messages.READ_ONLY_USER).setProfileFormData(formData).createResponse(AccountPages.ACCOUNT); } + if (result.hasAttributeChanged(UserModel.FIRST_NAME)) { + event.detail(Details.PREVIOUS_FIRST_NAME, oldFirstName).detail(Details.UPDATED_FIRST_NAME, newFirstName); + } + if (result.hasAttributeChanged(UserModel.LAST_NAME)) { + event.detail(Details.PREVIOUS_LAST_NAME, oldLastName).detail(Details.UPDATED_LAST_NAME, newLastName); + } if (result.hasAttributeChanged(UserModel.EMAIL)) { user.setEmailVerified(false); - event.clone().event(EventType.UPDATE_EMAIL).detail(Details.PREVIOUS_EMAIL, oldEmail).detail(Details.UPDATED_EMAIL, newEmail).success(); + event.detail(Details.PREVIOUS_EMAIL, oldEmail).detail(Details.UPDATED_EMAIL, newEmail); } event.success(); diff --git a/services/src/main/java/org/keycloak/services/resources/account/Constants.java b/services/src/main/java/org/keycloak/services/resources/account/Constants.java index 95b76de399..b443256f31 100644 --- a/services/src/main/java/org/keycloak/services/resources/account/Constants.java +++ b/services/src/main/java/org/keycloak/services/resources/account/Constants.java @@ -38,6 +38,12 @@ public class Constants { EXPOSED_LOG_DETAILS.add(Details.UPDATED_EMAIL); EXPOSED_LOG_DETAILS.add(Details.EMAIL); EXPOSED_LOG_DETAILS.add(Details.PREVIOUS_EMAIL); + EXPOSED_LOG_DETAILS.add(Details.FIRST_NAME); + EXPOSED_LOG_DETAILS.add(Details.LAST_NAME); + EXPOSED_LOG_DETAILS.add(Details.UPDATED_FIRST_NAME); + EXPOSED_LOG_DETAILS.add(Details.PREVIOUS_FIRST_NAME); + EXPOSED_LOG_DETAILS.add(Details.UPDATED_LAST_NAME); + EXPOSED_LOG_DETAILS.add(Details.PREVIOUS_LAST_NAME); EXPOSED_LOG_DETAILS.add(Details.USERNAME); EXPOSED_LOG_DETAILS.add(Details.REMEMBER_ME); EXPOSED_LOG_DETAILS.add(Details.REGISTER_METHOD); diff --git a/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/account/AccountFormServiceTest.java b/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/account/AccountFormServiceTest.java index 8a63dac52b..a1bc7992d5 100644 --- a/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/account/AccountFormServiceTest.java +++ b/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/account/AccountFormServiceTest.java @@ -713,8 +713,10 @@ public class AccountFormServiceTest extends AbstractTestRealmKeycloakTest { Assert.assertEquals("New last", profilePage.getLastName()); Assert.assertEquals("new@email.com", profilePage.getEmail()); - events.expectAccount(EventType.UPDATE_EMAIL).detail(Details.PREVIOUS_EMAIL, "test-user@localhost").detail(Details.UPDATED_EMAIL, "new@email.com").assertEvent(); - events.expectAccount(EventType.UPDATE_PROFILE).assertEvent(); + events.expectAccount(EventType.UPDATE_PROFILE).detail(Details.PREVIOUS_FIRST_NAME, "Tom").detail(Details.UPDATED_FIRST_NAME, "New first") + .detail(Details.PREVIOUS_LAST_NAME, "Brady").detail(Details.UPDATED_LAST_NAME, "New last") + .detail(Details.PREVIOUS_EMAIL, "test-user@localhost").detail(Details.UPDATED_EMAIL, "new@email.com") + .assertEvent(); // reset user for other tests profilePage.updateProfile("Tom", "Brady", "test-user@localhost"); @@ -755,8 +757,7 @@ public class AccountFormServiceTest extends AbstractTestRealmKeycloakTest { Assert.assertEquals("New last", profilePage.getLastName()); Assert.assertEquals("new@email.com", profilePage.getEmail()); - events.expectAccount(EventType.UPDATE_EMAIL).detail(Details.PREVIOUS_EMAIL, "test-user@localhost").detail(Details.UPDATED_EMAIL, "new@email.com").assertEvent(); - events.expectAccount(EventType.UPDATE_PROFILE).assertEvent(); + events.expectAccount(EventType.UPDATE_PROFILE).detail(Details.PREVIOUS_EMAIL, "test-user@localhost").detail(Details.UPDATED_EMAIL, "new@email.com").assertEvent(); user = userResource.toRepresentation(); assertNotNull(user.getAttributes()); diff --git a/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/actions/AppInitiatedActionUpdateProfileTest.java b/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/actions/AppInitiatedActionUpdateProfileTest.java index 7e95390d2e..c8fb1865c6 100755 --- a/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/actions/AppInitiatedActionUpdateProfileTest.java +++ b/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/actions/AppInitiatedActionUpdateProfileTest.java @@ -82,8 +82,10 @@ public class AppInitiatedActionUpdateProfileTest extends AbstractAppInitiatedAct updateProfilePage.update("New first", "New last", "new@email.com", "test-user@localhost"); - events.expectRequiredAction(EventType.UPDATE_EMAIL).detail(Details.PREVIOUS_EMAIL, "test-user@localhost").detail(Details.UPDATED_EMAIL, "new@email.com").assertEvent(); - events.expectRequiredAction(EventType.UPDATE_PROFILE).assertEvent(); + events.expectRequiredAction(EventType.UPDATE_PROFILE).detail(Details.PREVIOUS_FIRST_NAME, "Tom").detail(Details.UPDATED_FIRST_NAME, "New first") + .detail(Details.PREVIOUS_LAST_NAME, "Brady").detail(Details.UPDATED_LAST_NAME, "New last") + .detail(Details.PREVIOUS_EMAIL, "test-user@localhost").detail(Details.UPDATED_EMAIL, "new@email.com") + .assertEvent(); events.expectLogin().assertEvent(); assertKcActionStatus("success"); @@ -111,8 +113,11 @@ public class AppInitiatedActionUpdateProfileTest extends AbstractAppInitiatedAct updateProfilePage.update("New first", "New last", "new@email.com", "test-user@localhost"); events.expectLogin().assertEvent(); - events.expectRequiredAction(EventType.UPDATE_EMAIL).detail(Details.PREVIOUS_EMAIL, "test-user@localhost").detail(Details.UPDATED_EMAIL, "new@email.com").assertEvent(); - events.expectRequiredAction(EventType.UPDATE_PROFILE).assertEvent(); + events.expectRequiredAction(EventType.UPDATE_PROFILE).detail(Details.PREVIOUS_FIRST_NAME, "Tom").detail(Details.UPDATED_FIRST_NAME, "New first") + .detail(Details.PREVIOUS_LAST_NAME, "Brady").detail(Details.UPDATED_LAST_NAME, "New last") + .detail(Details.PREVIOUS_EMAIL, "test-user@localhost").detail(Details.UPDATED_EMAIL, "new@email.com") + .assertEvent(); + events.expectLogin().assertEvent(); assertKcActionStatus("success"); @@ -159,9 +164,12 @@ public class AppInitiatedActionUpdateProfileTest extends AbstractAppInitiatedAct events.expectLogin() .event(EventType.UPDATE_PROFILE) + .detail(Details.PREVIOUS_FIRST_NAME, "John") + .detail(Details.UPDATED_FIRST_NAME, "New first") + .detail(Details.PREVIOUS_LAST_NAME, "Doh") + .detail(Details.UPDATED_LAST_NAME, "New last") .detail(Details.USERNAME, "john-doh@localhost") - .user(userId) - .session(Matchers.nullValue(String.class)) + .user(userId).session(Matchers.nullValue(String.class)) .removeDetail(Details.CONSENT) .assertEvent(); diff --git a/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/actions/RequiredActionMultipleActionsTest.java b/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/actions/RequiredActionMultipleActionsTest.java index 124620a9df..196d980f51 100755 --- a/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/actions/RequiredActionMultipleActionsTest.java +++ b/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/actions/RequiredActionMultipleActionsTest.java @@ -96,17 +96,16 @@ public class RequiredActionMultipleActionsTest extends AbstractTestRealmKeycloak public String updateProfile(String codeId) { updateProfilePage.update("New first", "New last", "new@email.com", "test-user@localhost"); - AssertEvents.ExpectedEvent expectedEvent = events.expectRequiredAction(EventType.UPDATE_EMAIL) + AssertEvents.ExpectedEvent expectedEvent = events.expectRequiredAction(EventType.UPDATE_PROFILE) + .detail(Details.UPDATED_FIRST_NAME, "New first") + .detail(Details.UPDATED_LAST_NAME, "New last") .detail(Details.PREVIOUS_EMAIL, "test-user@localhost") .detail(Details.UPDATED_EMAIL, "new@email.com"); + if (codeId != null) { expectedEvent.detail(Details.CODE_ID, codeId); } - codeId = expectedEvent.assertEvent().getDetails().get(Details.CODE_ID); - events.expectRequiredAction(EventType.UPDATE_PROFILE) - .detail(Details.CODE_ID, codeId) - .assertEvent(); - return codeId; + return expectedEvent.assertEvent().getDetails().get(Details.CODE_ID); } } diff --git a/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/actions/RequiredActionPriorityTest.java b/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/actions/RequiredActionPriorityTest.java index 59b550ef94..fe8935cfe3 100644 --- a/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/actions/RequiredActionPriorityTest.java +++ b/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/actions/RequiredActionPriorityTest.java @@ -107,11 +107,13 @@ public class RequiredActionPriorityTest extends AbstractTestRealmKeycloakTest { // Finally, update profile updateProfilePage.assertCurrent(); updateProfilePage.update("New first", "New last", "new@email.com", "test-user@localhost"); - events.expectRequiredAction(EventType.UPDATE_EMAIL).detail(Details.PREVIOUS_EMAIL, "test-user@localhost") - .detail(Details.UPDATED_EMAIL, "new@email.com").assertEvent(); - events.expectRequiredAction(EventType.UPDATE_PROFILE).assertEvent(); + events.expectRequiredAction(EventType.UPDATE_PROFILE).detail(Details.UPDATED_FIRST_NAME, "New first") + .detail(Details.UPDATED_LAST_NAME, "New last") + .detail(Details.PREVIOUS_EMAIL, "test-user@localhost") + .detail(Details.UPDATED_EMAIL, "new@email.com") + .assertEvent(); - // Logined + // Logged in appPage.assertCurrent(); Assert.assertEquals(RequestType.AUTH_RESPONSE, appPage.getRequestType()); events.expectLogin().assertEvent(); @@ -139,9 +141,11 @@ public class RequiredActionPriorityTest extends AbstractTestRealmKeycloakTest { // Second, update profile updateProfilePage.assertCurrent(); updateProfilePage.update("New first", "New last", "new@email.com", "test-user@localhost"); - events.expectRequiredAction(EventType.UPDATE_EMAIL).detail(Details.PREVIOUS_EMAIL, "test-user@localhost") - .detail(Details.UPDATED_EMAIL, "new@email.com").assertEvent(); - events.expectRequiredAction(EventType.UPDATE_PROFILE).assertEvent(); + events.expectRequiredAction(EventType.UPDATE_PROFILE).detail(Details.UPDATED_FIRST_NAME, "New first") + .detail(Details.UPDATED_LAST_NAME, "New last") + .detail(Details.PREVIOUS_EMAIL, "test-user@localhost") + .detail(Details.UPDATED_EMAIL, "new@email.com") + .assertEvent(); // Finally, accept terms termsPage.assertCurrent(); diff --git a/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/actions/RequiredActionUpdateProfileTest.java b/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/actions/RequiredActionUpdateProfileTest.java index d8920ae765..f763dab418 100755 --- a/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/actions/RequiredActionUpdateProfileTest.java +++ b/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/actions/RequiredActionUpdateProfileTest.java @@ -102,9 +102,11 @@ public class RequiredActionUpdateProfileTest extends AbstractTestRealmKeycloakTe updateProfilePage.update("New first", "New last", "new@email.com", "test-user@localhost"); - events.expectRequiredAction(EventType.UPDATE_EMAIL).detail(Details.PREVIOUS_EMAIL, "test-user@localhost").detail(Details.UPDATED_EMAIL, "new@email.com").assertEvent(); - events.expectRequiredAction(EventType.UPDATE_PROFILE).assertEvent(); - + events.expectRequiredAction(EventType.UPDATE_PROFILE).detail(Details.PREVIOUS_FIRST_NAME, "Tom").detail(Details.UPDATED_FIRST_NAME, "New first") + .detail(Details.PREVIOUS_LAST_NAME, "Brady").detail(Details.UPDATED_LAST_NAME, "New last") + .detail(Details.PREVIOUS_EMAIL, "test-user@localhost").detail(Details.UPDATED_EMAIL, "new@email.com") + .detail(Details.PREVIOUS_EMAIL, "test-user@localhost").detail(Details.UPDATED_EMAIL, "new@email.com") + .assertEvent(); Assert.assertEquals(RequestType.AUTH_RESPONSE, appPage.getRequestType()); events.expectLogin().assertEvent(); @@ -129,12 +131,9 @@ public class RequiredActionUpdateProfileTest extends AbstractTestRealmKeycloakTe updateProfilePage.update("New first", "New last", "john-doh@localhost", "new"); - events.expectLogin() - .event(EventType.UPDATE_PROFILE) + events.expectLogin().event(EventType.UPDATE_PROFILE).detail(Details.UPDATED_FIRST_NAME, "New first").user(userId).session(Matchers.nullValue(String.class)).removeDetail(Details.CONSENT) + .detail(Details.UPDATED_LAST_NAME, "New last").user(userId).session(Matchers.nullValue(String.class)).removeDetail(Details.CONSENT) .detail(Details.USERNAME, "john-doh@localhost") - .user(userId) - .session(Matchers.nullValue(String.class)) - .removeDetail(Details.CONSENT) .assertEvent(); Assert.assertEquals(RequestType.AUTH_RESPONSE, appPage.getRequestType()); @@ -344,8 +343,7 @@ public class RequiredActionUpdateProfileTest extends AbstractTestRealmKeycloakTe updateProfilePage.update("New first", "New last", "new@email.com", "test-user@localhost"); - events.expectRequiredAction(EventType.UPDATE_EMAIL).detail(Details.PREVIOUS_EMAIL, "test-user@localhost").detail(Details.UPDATED_EMAIL, "new@email.com").assertEvent(); - events.expectRequiredAction(EventType.UPDATE_PROFILE).assertEvent(); + events.expectRequiredAction(EventType.UPDATE_PROFILE).detail(Details.PREVIOUS_EMAIL, "test-user@localhost").detail(Details.UPDATED_EMAIL, "new@email.com").assertEvent(); Assert.assertEquals(RequestType.AUTH_RESPONSE, appPage.getRequestType());