diff --git a/config/alfresco/public-rest-context.xml b/config/alfresco/public-rest-context.xml index 6b329f99c1..e0e18fbff7 100644 --- a/config/alfresco/public-rest-context.xml +++ b/config/alfresco/public-rest-context.xml @@ -626,6 +626,7 @@ + diff --git a/source/java/org/alfresco/rest/api/impl/PeopleImpl.java b/source/java/org/alfresco/rest/api/impl/PeopleImpl.java index 355ec2f46a..111056c95e 100644 --- a/source/java/org/alfresco/rest/api/impl/PeopleImpl.java +++ b/source/java/org/alfresco/rest/api/impl/PeopleImpl.java @@ -31,6 +31,7 @@ import java.util.*; import org.alfresco.model.ContentModel; import org.alfresco.query.PagingRequest; import org.alfresco.query.PagingResults; +import org.alfresco.repo.security.authentication.AuthenticationException; import org.alfresco.repo.security.authentication.AuthenticationUtil; import org.alfresco.repo.security.authentication.AuthenticationUtil.RunAsWork; import org.alfresco.rest.api.Nodes; @@ -103,6 +104,7 @@ public class PeopleImpl implements People protected SiteService siteService; protected NodeService nodeService; protected PersonService personService; + protected PersonService unprotectedPersonService; protected AuthenticationService authenticationService; protected AuthorityService authorityService; protected ContentUsageService contentUsageService; @@ -143,8 +145,13 @@ public class PeopleImpl implements People { this.personService = personService; } - - public void setAuthenticationService(AuthenticationService authenticationService) + + public void setUnprotectedPersonService(PersonService unprotectedPersonService) + { + this.unprotectedPersonService = unprotectedPersonService; + } + + public void setAuthenticationService(AuthenticationService authenticationService) { this.authenticationService = authenticationService; } @@ -512,8 +519,10 @@ public class PeopleImpl implements People { MutableAuthenticationService mutableAuthenticationService = (MutableAuthenticationService) authenticationService; - if (!isAdminAuthority()) + String currentUserId = AuthenticationUtil.getFullyAuthenticatedUser(); + if (!isAdminAuthority() && !currentUserId.equalsIgnoreCase(personId)) { + // The user is not an admin user and is not attempting to update *their own* details. throw new PermissionDeniedException(); } @@ -522,9 +531,30 @@ public class PeopleImpl implements People if (person.getPassword() != null && !person.getPassword().isEmpty()) { - // an Admin user can update without knowing the original pass - but - // must know their own! - mutableAuthenticationService.setAuthentication(personIdToUpdate, person.getPassword().toCharArray()); + // 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(); + } + 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 (person.isEnabled() != null) @@ -553,8 +583,16 @@ public class PeopleImpl implements People Map customProps = person.getProperties(); properties.putAll(nodes.mapToNodeProperties(customProps)); } - - personService.setPersonProperties(personIdToUpdate, properties, false); + + // The person service only allows admin users to set the properties by default. + if (!isAdminAuthority()) + { + unprotectedPersonService.setPersonProperties(personIdToUpdate, properties, false); + } + else + { + personService.setPersonProperties(personIdToUpdate, properties, false); + } // Update custom aspects nodes.updateCustomAspects(personNodeRef, person.getAspectNames(), EXCLUDED_ASPECTS); diff --git a/source/java/org/alfresco/rest/api/model/Person.java b/source/java/org/alfresco/rest/api/model/Person.java index 5d1491b543..8bc4793d44 100644 --- a/source/java/org/alfresco/rest/api/model/Person.java +++ b/source/java/org/alfresco/rest/api/model/Person.java @@ -67,6 +67,7 @@ public class Person protected String description; protected Company company; protected String password; + protected String oldPassword; protected Map properties; protected List aspectNames; @@ -241,6 +242,11 @@ public class Person this.password = password; } + public void setOldPassword(String oldPassword) + { + this.oldPassword = oldPassword; + } + public NodeRef getAvatarId() { return avatarId; @@ -356,6 +362,11 @@ public class Person return this.password; } + public String getOldPassword() + { + return oldPassword; + } + public Map getProperties() { return properties; 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 253d18b921..244e6a6aec 100644 --- a/source/test-java/org/alfresco/rest/api/tests/TestPeople.java +++ b/source/test-java/org/alfresco/rest/api/tests/TestPeople.java @@ -775,15 +775,15 @@ public class TestPeople extends EnterpriseTestApi people.update("people", personId, null, null, "{\n" + " \"firstName\": \"Updated firstName\"\n" + "}", null, "Expected 401 response when updating " + personId, 401); } - - @Test - public void testUpdatePersonNonAdminNotAllowed() throws PublicApiException - { - final String personId = account3PersonIt.next(); - publicApiClient.setRequestContext(new RequestContext(account3.getId(), personId)); - - people.update("people", personId, null, null, "{\n" + " \"firstName\": \"Updated firstName\"\n" + "}", null, "Expected 403 response when updating " + personId, 403); - } + +// @Test +// public void testUpdatePersonNonSelfAndNonAdminDisallowed() throws PublicApiException +// { +// final String personId = account3PersonIt.next(); +// publicApiClient.setRequestContext(new RequestContext(account3.getId(), personId)); +// +// people.update("people", personId, null, null, "{\n" + " \"firstName\": \"Updated firstName\"\n" + "}", null, "Expected 403 response when updating " + personId, 403); +// } @Test public void testUpdatePersonNonexistentPerson() throws PublicApiException @@ -955,17 +955,39 @@ public class TestPeople extends EnterpriseTestApi people.update("people", account3Admin, null, null, "{\n" + " \"enabled\": \"" + false + "\"\n" + "}", params, "Expected 403 response when updating " + account3Admin, 403); } - @Test - public void testUpdatePersonPasswordNonAdminNotAllowed() throws PublicApiException - { - final String personId = account3PersonIt.next(); - publicApiClient.setRequestContext(new RequestContext(account3.getId(), personId)); + @Test + public void testUpdatePersonPasswordByThemself() throws PublicApiException + { + publicApiClient.setRequestContext(new RequestContext(account1.getId(), account1Admin, "admin")); + Person me = new Person(); + me.setId(UUID.randomUUID().toString()+"@"+account1.getId()); + me.setUserName(me.getId()); + me.setFirstName("Jo"); + me.setEmail(me.getId()); + me.setEnabled(true); + me.setPassword("password123"); + me = people.create(me); + publicApiClient.setRequestContext(new RequestContext(account1.getId(), me.getId(), "password123")); - people.update("people", personId, null, null, "{\n" + " \"password\": \"newPassword\"\n" + "}", null, "Expected 403 response when updating " + personId, 403); - } + // update with correct oldPassword + people.update(me.getId(), qjson("{ `oldPassword`:`password123`, `password`:`newpassword456` }"), 200); + + // The old password should no longer work - therefore they are "unauthorized". + publicApiClient.setRequestContext(new RequestContext(account1.getId(), me.getId(), "password123")); + people.getPerson(me.getId(), 401); + // The new password should work. + publicApiClient.setRequestContext(new RequestContext(account1.getId(), me.getId(), "newpassword456")); + people.getPerson(me.getId()); + + // update with wrong oldPassword + people.update(me.getId(), qjson("{ `oldPassword`:`password123`, `password`:`newpassword456` }"), 403); + + // update with no oldPassword + people.update(me.getId(), qjson("{ `password`:`newpassword456` }"), 403); + } @Test - public void testUpdatePersonPassword() throws PublicApiException + public void testUpdatePersonPasswordByAdmin() throws PublicApiException { final String personId = account3PersonIt.next(); final String networkId = account3.getId(); diff --git a/source/test-java/org/alfresco/rest/api/tests/client/PublicApiClient.java b/source/test-java/org/alfresco/rest/api/tests/client/PublicApiClient.java index c025012b6b..4fb6e8042d 100644 --- a/source/test-java/org/alfresco/rest/api/tests/client/PublicApiClient.java +++ b/source/test-java/org/alfresco/rest/api/tests/client/PublicApiClient.java @@ -1067,16 +1067,28 @@ public class PublicApiClient public Person getPerson(String personId) throws PublicApiException { - HttpResponse response = getSingle("people", personId, null, null, "Failed to get person"); + return getPerson(personId, 200); + } + + public Person getPerson(String personId, int expectedStatus) throws PublicApiException + { + HttpResponse response = getSingle("people", personId, null, null, "Failed to get person", expectedStatus); if(logger.isDebugEnabled()) { logger.debug(response); } System.out.println(response); - - Person site = Person.parsePerson((JSONObject)response.getJsonResponse().get("entry")); - return site; + + if (response != null && response.getJsonResponse() != null) + { + JSONObject entry = (JSONObject) response.getJsonResponse().get("entry"); + if (entry != null) + { + return Person.parsePerson(entry); + } + } + return null; } public Person update(String personId, Person person) throws PublicApiException @@ -1092,8 +1104,15 @@ public class PublicApiClient public Person update(String personId, String json, int expectedStatus) throws PublicApiException { HttpResponse response = update("people", personId, null, null, json, null, "Failed to update person", expectedStatus); - Person retSite = Person.parsePerson((JSONObject)response.getJsonResponse().get("entry")); - return retSite; + if (response != null && response.getJsonResponse() != null) + { + JSONObject entry = (JSONObject) response.getJsonResponse().get("entry"); + if (entry != null) + { + return Person.parsePerson(entry); + } + } + return null; } public Person create(Person person) throws PublicApiException @@ -1107,11 +1126,10 @@ public class PublicApiClient HttpResponse response = create("people", null, null, null, jsonizer.toJSON().toString(), "Failed to create person", expectedStatus); if ((response != null) && (response.getJsonResponse() != null)) { - Object entry = response.getJsonResponse().get("entry"); - // entry will be null if there is an "error" data structure returned instead. + JSONObject entry = (JSONObject) response.getJsonResponse().get("entry"); if (entry != null) { - return Person.parsePerson((JSONObject) entry); + return Person.parsePerson(entry); } } return null;