From e0005cebcf4b895d069003a2fa5596f36dc154c6 Mon Sep 17 00:00:00 2001 From: Cristian Turlica Date: Thu, 3 Nov 2016 10:17:58 +0000 Subject: [PATCH] REPO-1506: Update Person - implement - added implementation for update personService - added tests git-svn-id: https://svn.alfresco.com/repos/alfresco-enterprise/alfresco/BRANCHES/DEV/5.2.N/root@132117 c4b6b30b-aa2e-2d43-bbcb-ca4b014f7261 --- source/java/org/alfresco/rest/api/People.java | 9 +- .../alfresco/rest/api/impl/PeopleImpl.java | 97 +++--- .../org/alfresco/rest/api/model/Person.java | 4 + .../rest/api/people/PeopleEntityResource.java | 46 ++- .../alfresco/rest/api/tests/TestPeople.java | 288 +++++++++++++++++- 5 files changed, 392 insertions(+), 52 deletions(-) diff --git a/source/java/org/alfresco/rest/api/People.java b/source/java/org/alfresco/rest/api/People.java index 430bf63568..68fd9cbd03 100644 --- a/source/java/org/alfresco/rest/api/People.java +++ b/source/java/org/alfresco/rest/api/People.java @@ -52,5 +52,12 @@ public interface People */ Person create(PersonUpdate person); - //Person updatePerson(String personId, Person person); + /** + * Update the given person's details. + * + * @param personId The identifier of a person. + * @param person The person details. + * @return The updated person details. + */ + Person update(String personId, Person person); } diff --git a/source/java/org/alfresco/rest/api/impl/PeopleImpl.java b/source/java/org/alfresco/rest/api/impl/PeopleImpl.java index 08b8df2dbe..8ceb6b83c7 100644 --- a/source/java/org/alfresco/rest/api/impl/PeopleImpl.java +++ b/source/java/org/alfresco/rest/api/impl/PeopleImpl.java @@ -36,6 +36,7 @@ import org.alfresco.rest.api.model.PersonUpdate; import org.alfresco.rest.framework.core.exceptions.ConstraintViolatedException; import org.alfresco.rest.framework.core.exceptions.EntityNotFoundException; import org.alfresco.rest.framework.core.exceptions.InvalidArgumentException; +import org.alfresco.rest.framework.resource.parameters.Parameters; import org.alfresco.service.cmr.repository.*; import org.alfresco.service.cmr.security.AuthenticationService; import org.alfresco.service.cmr.security.MutableAuthenticationService; @@ -300,26 +301,38 @@ public class PeopleImpl implements People mas.createAuthentication(person.getUserName(), person.getPassword().toCharArray()); mas.setAuthenticationEnabled(person.getUserName(), person.isEnabled()); NodeRef nodeRef = personService.createPerson(props); - - // Write the contents of PersonUpdate.getDescription() text to a content file - // and store the content URL in ContentModel.PROP_PERSONDESC - if (person.getDescription() != null) - { - AuthenticationUtil.runAsSystem(new RunAsWork() - { - @Override - public Void doWork() throws Exception - { - ContentWriter writer = contentService.getWriter(nodeRef, ContentModel.PROP_PERSONDESC, true); - writer.putContent(person.getDescription()); - return null; - } - }); - } - // Return a fresh retrieval - return getPerson(person.getUserName()); - } + // Write the contents of PersonUpdate.getDescription() text to a content file + // and store the content URL in ContentModel.PROP_PERSONDESC + if (person.getDescription() != null) + { + savePersonDescription(person.getDescription(), nodeRef); + } + + // Return a fresh retrieval + return getPerson(person.getUserName()); + } + + /** + * Write the description to a content file and store the content URL in + * ContentModel.PROP_PERSONDESC + * + * @param description + * @param nodeRef + */ + private void savePersonDescription(final String description, final NodeRef nodeRef) + { + AuthenticationUtil.runAsSystem(new RunAsWork() + { + @Override + public Void doWork() throws Exception + { + ContentWriter writer = contentService.getWriter(nodeRef, ContentModel.PROP_PERSONDESC, true); + writer.putContent(description); + return null; + } + }); + } private void validateCreatePersonData(PersonUpdate person) { @@ -337,28 +350,38 @@ public class PeopleImpl implements People throw new InvalidArgumentException("Field '"+fieldName+"' is null, but is required."); } } - - /** - public Person updatePerson(String personId, final Person person) + public Person update(String personId, final Person person) { - personId = validatePerson(personId); + MutableAuthenticationService mutableAuthenticationService = (MutableAuthenticationService) authenticationService; - final Map properties = toProperties(person); + final String personIdToUpdate = validatePerson(personId); + final Map properties = person.toProperties(); - final String pId = personId; - AuthenticationUtil.runAsSystem(new RunAsWork() - { - @Override - public Void doWork() throws Exception - { - personService.setPersonProperties(pId, properties, false); - return null; - } - - }); + 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()); + } - return getPerson(personId); + if (person.isEnabled() != null) + { + mutableAuthenticationService.setAuthenticationEnabled(personIdToUpdate, person.isEnabled()); + } + + if (person.getDescription() != null) + { + // Remove person description from saved properties + properties.remove(ContentModel.PROP_PERSONDESC); + + // Custom save for person description. + NodeRef personNodeRef = personService.getPerson(personIdToUpdate, false); + savePersonDescription(person.getDescription(), personNodeRef); + } + + personService.setPersonProperties(personIdToUpdate, properties, false); + + return getPerson(personId); } - */ } diff --git a/source/java/org/alfresco/rest/api/model/Person.java b/source/java/org/alfresco/rest/api/model/Person.java index d7430ac3e1..1355ad182b 100644 --- a/source/java/org/alfresco/rest/api/model/Person.java +++ b/source/java/org/alfresco/rest/api/model/Person.java @@ -359,6 +359,10 @@ public class Person addToMap(properties, ContentModel.PROP_SIZE_QUOTA, getQuota()); addToMap(properties, ContentModel.PROP_SIZE_CURRENT, getQuotaUsed()); addToMap(properties, ContentModel.PROP_PERSONDESC, getDescription()); + addToMap(properties, ContentModel.PROP_ENABLED, isEnabled()); + + Boolean isEmailNotificationsEnabled = isEmailNotificationsEnabled(); + addToMap(properties, ContentModel.PROP_EMAIL_FEED_DISABLED, (isEmailNotificationsEnabled == null ? null : !isEmailNotificationsEnabled.booleanValue())); } } diff --git a/source/java/org/alfresco/rest/api/people/PeopleEntityResource.java b/source/java/org/alfresco/rest/api/people/PeopleEntityResource.java index a0e3b1c32d..f9c85279e6 100644 --- a/source/java/org/alfresco/rest/api/people/PeopleEntityResource.java +++ b/source/java/org/alfresco/rest/api/people/PeopleEntityResource.java @@ -50,7 +50,7 @@ import java.util.List; * @author Gethin James */ @EntityResource(name="people", title = "People") -public class PeopleEntityResource implements EntityResourceAction.ReadById, EntityResourceAction.Create, InitializingBean +public class PeopleEntityResource implements EntityResourceAction.ReadById, EntityResourceAction.Create, EntityResourceAction.Update, InitializingBean { private static Log logger = LogFactory.getLog(PeopleEntityResource.class); @@ -133,4 +133,48 @@ public class PeopleEntityResource implements EntityResourceAction.ReadById accountsIt; private TestNetwork account1; private TestNetwork account2; + private TestNetwork account3; private Iterator account1PersonIt; private Iterator account2PersonIt; + private Iterator account3PersonIt; private String account1Admin; private String account2Admin; + private String account3Admin; - @Before - public void setUp() throws Exception - { - people = publicApiClient.people(); - accountsIt = getTestFixture().getNetworksIt(); - account1 = accountsIt.next(); - account2 = accountsIt.next(); - account1Admin = "admin@"+account1.getId(); - account2Admin = "admin@"+account2.getId(); - account1PersonIt = account1.getPersonIds().iterator(); - account2PersonIt = account2.getPersonIds().iterator(); - } + @Before + public void setUp() throws Exception + { + people = publicApiClient.people(); + accountsIt = getTestFixture().getNetworksIt(); + account1 = accountsIt.next(); + account2 = accountsIt.next(); + account3 = createNetwork("account3"); + account1Admin = "admin@" + account1.getId(); + account2Admin = "admin@" + account2.getId(); + account3Admin = "admin@" + account3.getId(); + account1PersonIt = account1.getPersonIds().iterator(); + account2PersonIt = account2.getPersonIds().iterator(); + + account3.createUser(); + account3PersonIt = account3.getPersonIds().iterator(); + } + + private TestNetwork createNetwork(String networkPrefix) + { + TestNetwork network = getRepoService().createNetwork(networkPrefix + GUID.generate(), true); + network.create(); + + return network; + } @Test public void testPeople() throws Exception @@ -442,4 +467,241 @@ public class TestPeople extends EnterpriseTestApi return personJson; } } + + @Test + public void testUpdatePersonAuthenticationFailed() throws PublicApiException + { + final String personId = account2PersonIt.next(); + + publicApiClient.setRequestContext(new RequestContext(account1.getId(), personId)); + + 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 testUpdatePersonNonexistentPerson() throws PublicApiException + { + final String personId = "non-existent"; + publicApiClient.setRequestContext(new RequestContext(account3.getId(), account3Admin, "admin")); + + people.update("people", personId, null, null, "{\n" + " \"firstName\": \"Updated firstName\"\n" + "}", null, "Expected 404 response when updating " + personId, 404); + } + + @Test + public void testUpdatePersonUsingPartialUpdate() throws PublicApiException + { + final String personId = account3PersonIt.next(); + + publicApiClient.setRequestContext(new RequestContext(account3.getId(), account3Admin, "admin")); + + String updatedFirstName = "Updated firstName"; + + HttpResponse response = people.update("people", personId, null, null, "{\n" + " \"firstName\": \"" + updatedFirstName + "\"\n" + "}", null, + "Expected 200 response when updating " + personId, 200); + + Person updatedPerson = Person.parsePerson((JSONObject) response.getJsonResponse().get("entry")); + + assertEquals(updatedFirstName, updatedPerson.getFirstName()); + } + + @Test + public void testUpdatePersonWithRestrictedResponseFields() throws PublicApiException + { + final String personId = account3PersonIt.next(); + + publicApiClient.setRequestContext(new RequestContext(account3.getId(), account3Admin, "admin")); + + String updatedFirstName = "Updated firstName"; + + Map params = new HashMap<>(); + params.put("fields", "id,firstName"); + + HttpResponse response = people.update("people", personId, null, null, "{\n" + " \"firstName\": \"" + updatedFirstName + "\"\n" + "}", params, + "Expected 200 response when updating " + personId, 200); + + Person updatedPerson = Person.parsePerson((JSONObject) response.getJsonResponse().get("entry")); + + assertNotNull(updatedPerson.getId()); + assertEquals(updatedFirstName, updatedPerson.getFirstName()); + assertNull(updatedPerson.getEmail()); + } + + @Test + public void testUpdatePersonUpdate() throws Exception + { + final String personId = account3PersonIt.next(); + + publicApiClient.setRequestContext(new RequestContext(account3.getId(), account3Admin, "admin")); + + String firstName = "updatedFirstName"; + String lastName = "updatedLastName"; + String description = "updatedDescription"; + String email = "updated@example.com"; + String skypeId = "updated.skype.id"; + String googleId = "googleId"; + String instantMessageId = "updated.user@example.com"; + String jobTitle = "updatedJobTitle"; + String location = "updatedLocation"; + + Company company = new Company("updatedOrganization", "updatedAddress1", "updatedAddress2", "updatedAddress3", "updatedPostcode", "updatedTelephone", "updatedFax", "updatedEmail"); + + String mobile = "mobile"; + String telephone = "telephone"; + String userStatus = "userStatus"; + Boolean enabled = true; + Boolean emailNotificationsEnabled = false; + + Map params = new HashMap<>(); + params.put("fields", "id,firstName,lastName,description,avatarId,email,skypeId,googleId,instantMessageId,jobTitle,location,mobile,telephone,userStatus,emailNotificationsEnabled,enabled,company"); + + HttpResponse response = people.update("people", personId, null, null, + "{\n" + + " \"firstName\": \"" + firstName + "\",\n" + + " \"lastName\": \"" + lastName + "\",\n" + + " \"description\": \"" + description + "\",\n" + + " \"email\": \"" + email + "\",\n" + + " \"skypeId\": \"" + skypeId + "\",\n" + + " \"googleId\": \"" + googleId + "\",\n" + + " \"instantMessageId\": \"" + instantMessageId + "\",\n" + + " \"jobTitle\": \"" + jobTitle + "\",\n" + + " \"location\": \"" + location + "\",\n" + + + " \"company\": {\n" + + " \"organization\": \"" + company.getOrganization() + "\",\n" + + " \"address1\": \"" + company.getAddress1() + "\",\n" + + " \"address2\": \"" + company.getAddress2() + "\",\n" + + " \"address3\": \"" + company.getAddress3() + "\",\n" + + " \"postcode\": \"" + company.getPostcode() + "\",\n" + + " \"telephone\": \"" + company.getTelephone() + "\",\n" + + " \"fax\": \"" + company.getFax() + "\",\n" + + " \"email\": \"" + company.getEmail() + "\"\n" + + " },\n" + + + " \"mobile\": \"" + mobile + "\",\n" + + " \"telephone\": \"" + telephone + "\",\n" + + " \"userStatus\": \"" + userStatus + "\",\n" + + " \"emailNotificationsEnabled\": \"" + emailNotificationsEnabled + "\",\n" + + " \"enabled\": \"" + enabled + "\"\n" + + + "}", params, + "Expected 200 response when updating " + personId, 200); + + Person updatedPerson = Person.parsePerson((JSONObject) response.getJsonResponse().get("entry")); + + assertNotNull(updatedPerson.getId()); + assertEquals(firstName, updatedPerson.getFirstName()); + assertEquals(lastName, updatedPerson.getLastName()); + assertEquals(description, updatedPerson.getDescription()); + assertEquals(email, updatedPerson.getEmail()); + assertEquals(skypeId, updatedPerson.getSkypeId()); + assertEquals(googleId, updatedPerson.getGoogleId()); + assertEquals(instantMessageId, updatedPerson.getInstantMessageId()); + assertEquals(jobTitle, updatedPerson.getJobTitle()); + assertEquals(location, updatedPerson.getLocation()); + + assertNotNull(updatedPerson.getCompany()); + company.expected(updatedPerson.getCompany()); + + assertEquals(mobile, updatedPerson.getMobile()); + assertEquals(telephone, updatedPerson.getTelephone()); + assertEquals(userStatus, updatedPerson.getUserStatus()); + assertEquals(emailNotificationsEnabled, updatedPerson.isEmailNotificationsEnabled()); + assertEquals(enabled, updatedPerson.isEnabled()); + } + + @Test + public void testUpdatePersonEnabledNonAdminNotAllowed() throws PublicApiException + { + final String personId = account3PersonIt.next(); + publicApiClient.setRequestContext(new RequestContext(account3.getId(), personId)); + + people.update("people", personId, null, null, "{\n" + " \"enabled\": \"false\"\n" + "}", null, "Expected 403 response when updating " + personId, 403); + } + + @Test + public void testUpdatePersonEnabled() throws PublicApiException + { + final String personId = account3PersonIt.next(); + publicApiClient.setRequestContext(new RequestContext(account3.getId(), account3Admin, "admin")); + + Boolean enabled = false; + + Map params = new HashMap<>(); + params.put("fields", "enabled"); + + HttpResponse response = people.update("people", personId, null, null, "{\n" + " \"enabled\": \"" + enabled + "\"\n" + "}", params, + "Expected 200 response when updating " + personId, 200); + + Person updatedPerson = Person.parsePerson((JSONObject) response.getJsonResponse().get("entry")); + + assertEquals(enabled, updatedPerson.isEnabled()); + } + + @Test + public void testUpdatePersonPasswordNonAdminNotAllowed() throws PublicApiException + { + final String personId = account3PersonIt.next(); + publicApiClient.setRequestContext(new RequestContext(account3.getId(), personId)); + + people.update("people", personId, null, null, "{\n" + " \"password\": \"newPassword\"\n" + "}", null, "Expected 403 response when updating " + personId, 403); + } + + @Test + public void testUpdatePersonPassword() throws PublicApiException + { + final String personId = account3PersonIt.next(); + final String networkId = account3.getId(); + + publicApiClient.setRequestContext(new RequestContext(networkId, account3Admin, "admin")); + + String invalidPassword = "invalidPassword"; + String updatedPassword = "newPassword"; + + people.update("people", personId, null, null, "{\n" + " \"password\": \"" + updatedPassword + "\"\n" + "}", null, + "Expected 200 response when updating " + personId, 200); + + publicApiClient.setRequestContext(new RequestContext(networkId, personId, invalidPassword)); + try + { + this.people.getPerson(personId); + fail(""); + } + catch (PublicApiException e) + { + assertEquals(HttpStatus.SC_UNAUTHORIZED, e.getHttpResponse().getStatusCode()); + } + + publicApiClient.setRequestContext(new RequestContext(networkId, personId, updatedPassword)); + this.people.getPerson(personId); + } + + @Test + public void testUpdatePersonWithNotUpdatableFields() throws PublicApiException + { + final String personId = account3PersonIt.next(); + + publicApiClient.setRequestContext(new RequestContext(account3.getId(), account3Admin, "admin")); + + List> notUpdatableFields = new ArrayList<>(); + notUpdatableFields.add(new Pair("userName", "userName")); + notUpdatableFields.add(new Pair("avatarId", "avatarId")); + notUpdatableFields.add(new Pair("statusUpdatedAt", "statusUpdatedAt")); + notUpdatableFields.add(new Pair("quota", "quota")); + notUpdatableFields.add(new Pair("quotaUsed", "quotaUsed")); + + for (Pair notUpdatableField : notUpdatableFields) + { + people.update("people", personId, null, null, "{\n" + "\"" + notUpdatableField.getFirst() + "\": \"" + notUpdatableField.getSecond() + "\"\n" + "}", null, + "Expected 400 response when updating " + personId, 400); + } + } }