diff --git a/source/java/org/alfresco/rest/api/impl/PeopleImpl.java b/source/java/org/alfresco/rest/api/impl/PeopleImpl.java index 0f3576387e..3b8d5bf43f 100644 --- a/source/java/org/alfresco/rest/api/impl/PeopleImpl.java +++ b/source/java/org/alfresco/rest/api/impl/PeopleImpl.java @@ -530,64 +530,25 @@ public class PeopleImpl implements People public Person update(String personId, final Person person) { - MutableAuthenticationService mutableAuthenticationService = (MutableAuthenticationService) authenticationService; + boolean isAdmin = isAdminAuthority(); validateUpdatePersonData(person); boolean isAdmin = isAdminAuthority(); String currentUserId = AuthenticationUtil.getFullyAuthenticatedUser(); - if (!isAdminAuthority() && !currentUserId.equalsIgnoreCase(personId)) + if (!isAdmin && !currentUserId.equalsIgnoreCase(personId)) { // The user is not an admin user and is not attempting to update *their own* details. throw new PermissionDeniedException(); } - if (!isAdminAuthority() && person.getOldPassword() != null && person.getPassword() == null) - { - throw new IllegalArgumentException("To change password, both 'oldPassword' and 'password' fields are required."); - } final String personIdToUpdate = validatePerson(personId); final Map properties = person.toProperties(); - if (person.getPassword() != null && !person.getPassword().isEmpty()) - { - // An admin user can update without knowing the original pass - but must know their own! - char[] newPassword = person.getPassword().toCharArray(); - if (isAdminAuthority()) - { - mutableAuthenticationService.setAuthentication(personIdToUpdate, newPassword); - } - else - { - // Non-admin users can update their own password, but must provide their current password. - if (person.getOldPassword() == null) - { - throw new PermissionDeniedException("Existing password is required, but missing (field 'oldPassword')."); - } - char[] oldPassword = person.getOldPassword().toCharArray(); - try - { - mutableAuthenticationService.updateAuthentication(personIdToUpdate, oldPassword, newPassword); - } - catch (AuthenticationException e) - { - // TODO: add to exception mappings/handler - throw new PermissionDeniedException("Incorrect existing password."); - } - } - } + // if requested, update password + updatePassword(isAdmin, personIdToUpdate, person); - if (person.isEnabled() != null) - { - if (isAdminAuthority(personIdToUpdate)) - { - throw new PermissionDeniedException("Admin authority cannot be disabled."); - } - - // note: if current user is not an admin then permission denied exception is thrown - mutableAuthenticationService.setAuthenticationEnabled(personIdToUpdate, person.isEnabled()); - } NodeRef personNodeRef = personService.getPerson(personIdToUpdate, false); if (person.wasSet(Person.PROP_PERSON_DESCRIPTION)) @@ -623,6 +584,66 @@ public class PeopleImpl implements People return getPerson(personId); } + private void updatePassword(boolean isAdmin, String personIdToUpdate, Person person) + { + MutableAuthenticationService mutableAuthenticationService = (MutableAuthenticationService) authenticationService; + + boolean isOldPassword = person.wasSet(Person.PROP_PERSON_OLDPASSWORD); + boolean isPassword = person.wasSet(Person.PROP_PERSON_PASSWORD); + + if (isPassword || isOldPassword) + { + if (isOldPassword && ((person.getOldPassword() == null) || (person.getOldPassword().isEmpty()))) + { + throw new IllegalArgumentException("'oldPassword' field cannot be empty."); + } + + if (!isPassword || (person.getPassword() == null) || (person.getPassword().isEmpty())) + { + throw new IllegalArgumentException("password' field cannot be empty."); + } + + char[] newPassword = person.getPassword().toCharArray(); + + if (!isAdmin) + { + // Non-admin users can update their own password, but must provide their current password. + if (!isOldPassword) + { + throw new IllegalArgumentException("To change password, both 'oldPassword' and 'password' fields are required."); + } + + char[] oldPassword = person.getOldPassword().toCharArray(); + try + { + mutableAuthenticationService.updateAuthentication(personIdToUpdate, oldPassword, newPassword); + } + catch (AuthenticationException e) + { + throw new PermissionDeniedException("Incorrect password."); + } + } + else + { + // An admin user can update without knowing the original pass - but must know their own! + // note: is it reasonable to silently ignore oldPassword if supplied ? + + mutableAuthenticationService.setAuthentication(personIdToUpdate, newPassword); + } + } + + if (person.isEnabled() != null) + { + if (isAdminAuthority(personIdToUpdate)) + { + throw new PermissionDeniedException("Admin authority cannot be disabled."); + } + + mutableAuthenticationService.setAuthenticationEnabled(personIdToUpdate, person.isEnabled()); + } + + } + private void validateUpdatePersonData(Person person) { if (person.wasSet(ContentModel.PROP_FIRSTNAME)) diff --git a/source/test-java/org/alfresco/rest/api/tests/TestPeople.java b/source/test-java/org/alfresco/rest/api/tests/TestPeople.java index 1191eccb75..e3b49da6b3 100644 --- a/source/test-java/org/alfresco/rest/api/tests/TestPeople.java +++ b/source/test-java/org/alfresco/rest/api/tests/TestPeople.java @@ -1163,11 +1163,14 @@ public class TestPeople extends EnterpriseTestApi people.update(me.getId(), qjson("{ `oldPassword`:`password123`, `password`:`newpassword456` }"), 403); // update with no oldPassword - people.update(me.getId(), qjson("{ `password`:`newpassword456` }"), 403); + people.update(me.getId(), qjson("{ `password`:`newpassword456` }"), 400); + people.update(me.getId(), qjson("{ `oldPassword`:``, `password`:`newpassword456` }"), 400); + people.update(me.getId(), qjson("{ `oldPassword`:null, `password`:`newpassword456` }"), 400); - // update with no password - people.update(me.getId(), qjson("{ `oldPassword`:`newpassword456`, `password`:`` }"), 400); + // update with no new password people.update(me.getId(), qjson("{ `oldPassword`:`newpassword456` }"), 400); + people.update(me.getId(), qjson("{ `oldPassword`:`newpassword456`, `password`:`` }"), 400); + people.update(me.getId(), qjson("{ `oldPassword`:`newpassword456`, `password`:null }"), 400); } @Test @@ -1197,6 +1200,30 @@ public class TestPeople extends EnterpriseTestApi publicApiClient.setRequestContext(new RequestContext(networkId, personId, updatedPassword)); this.people.getPerson(personId); + + publicApiClient.setRequestContext(new RequestContext(networkId, account3Admin, "admin")); + + // update with another new password but note that oldPassword is ignored (even if sent by admin) + String updatedPassword2 = "newPassword2"; + people.update(personId, qjson("{ `password`:`" + updatedPassword2 + "`, `oldPassword`:`rubbish` }"), 200); + + publicApiClient.setRequestContext(new RequestContext(networkId, personId, updatedPassword)); + try + { + this.people.getPerson(personId); + fail(""); + } + catch (PublicApiException e) + { + assertEquals(HttpStatus.SC_UNAUTHORIZED, e.getHttpResponse().getStatusCode()); + } + + publicApiClient.setRequestContext(new RequestContext(networkId, personId, updatedPassword2)); + this.people.getPerson(personId); + + // -ve: update with no new password + people.update(personId, qjson("{ `password`:`` }"), 400); + people.update(personId, qjson("{ `password`:null }"), 400); } @Test