diff --git a/source/java/org/alfresco/rest/api/impl/PeopleImpl.java b/source/java/org/alfresco/rest/api/impl/PeopleImpl.java index 787132cdf4..46b84d5a03 100644 --- a/source/java/org/alfresco/rest/api/impl/PeopleImpl.java +++ b/source/java/org/alfresco/rest/api/impl/PeopleImpl.java @@ -526,10 +526,18 @@ public class PeopleImpl implements People { throw new InvalidArgumentException("Field '"+fieldName+"' is null, but is required."); } + + // belts-and-braces - note: should not see empty string (since converted to null via custom json deserializer) + if ((fieldValue instanceof String) && ((String)fieldValue).isEmpty()) + { + throw new InvalidArgumentException("Field '"+fieldName+"' is empty, but is required."); + } } public Person update(String personId, final Person person) { + validateUpdatePersonData(person); + boolean isAdmin = isAdminAuthority(); String currentUserId = AuthenticationUtil.getFullyAuthenticatedUser(); @@ -545,6 +553,18 @@ public class PeopleImpl implements People // 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 mutableAuthenticationService = (MutableAuthenticationService) authenticationService; + mutableAuthenticationService.setAuthenticationEnabled(personIdToUpdate, person.isEnabled()); + } + NodeRef personNodeRef = personService.getPerson(personIdToUpdate, false); if (person.wasSet(Person.PROP_PERSON_DESCRIPTION)) { @@ -579,6 +599,29 @@ public class PeopleImpl implements People return getPerson(personId); } + private void validateUpdatePersonData(Person person) + { + if (person.wasSet(ContentModel.PROP_FIRSTNAME)) + { + checkRequiredField("firstName", person.getFirstName()); + } + + if (person.wasSet(ContentModel.PROP_EMAIL)) + { + checkRequiredField("email", person.getEmail()); + } + + if (person.wasSet(ContentModel.PROP_ENABLED) && (person.isEnabled() == null)) + { + throw new IllegalArgumentException("'enabled' field cannot be empty."); + } + + if (person.wasSet(ContentModel.PROP_EMAIL_FEED_DISABLED) && (person.isEmailNotificationsEnabled() == null)) + { + throw new IllegalArgumentException("'emailNotificationsEnabled' field cannot be empty."); + } + } + private void updatePassword(boolean isAdmin, String personIdToUpdate, Person person) { MutableAuthenticationService mutableAuthenticationService = (MutableAuthenticationService) authenticationService; @@ -626,17 +669,6 @@ public class PeopleImpl implements People 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 boolean isAdminAuthority() diff --git a/source/java/org/alfresco/rest/api/model/Person.java b/source/java/org/alfresco/rest/api/model/Person.java index 30c8a23415..82c39562cf 100644 --- a/source/java/org/alfresco/rest/api/model/Person.java +++ b/source/java/org/alfresco/rest/api/model/Person.java @@ -493,50 +493,62 @@ public class Person if (wasSet(PROP_PERSON_COMPANY)) { Company company = getCompany(); + + int setCount = 0; if (company != null) { if (company.wasSet(ContentModel.PROP_ORGANIZATION)) { + setCount++; properties.put(ContentModel.PROP_ORGANIZATION, company.getOrganization()); } - + if (company.wasSet(ContentModel.PROP_COMPANYADDRESS1)) { + setCount++; properties.put(ContentModel.PROP_COMPANYADDRESS1, company.getAddress1()); } - + if (company.wasSet(ContentModel.PROP_COMPANYADDRESS2)) { + setCount++; properties.put(ContentModel.PROP_COMPANYADDRESS2, company.getAddress2()); } - + if (company.wasSet(ContentModel.PROP_COMPANYADDRESS3)) { + setCount++; properties.put(ContentModel.PROP_COMPANYADDRESS3, company.getAddress3()); } if (company.wasSet(ContentModel.PROP_COMPANYPOSTCODE)) { + setCount++; properties.put(ContentModel.PROP_COMPANYPOSTCODE, company.getPostcode()); } if (company.wasSet(ContentModel.PROP_COMPANYTELEPHONE)) { + setCount++; properties.put(ContentModel.PROP_COMPANYTELEPHONE, company.getTelephone()); } - + if (company.wasSet(ContentModel.PROP_COMPANYFAX)) { + setCount++; properties.put(ContentModel.PROP_COMPANYFAX, company.getFax()); } if (company.wasSet(ContentModel.PROP_COMPANYEMAIL)) { + setCount++; properties.put(ContentModel.PROP_COMPANYEMAIL, company.getEmail()); } } - else + + if (setCount == 0) { + // company was null or {} (no individual properties set) properties.put(ContentModel.PROP_ORGANIZATION, null); properties.put(ContentModel.PROP_COMPANYADDRESS1, null); properties.put(ContentModel.PROP_COMPANYADDRESS2, null); 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 e7688f3b1c..68aca5e24b 100644 --- a/source/test-java/org/alfresco/rest/api/tests/TestPeople.java +++ b/source/test-java/org/alfresco/rest/api/tests/TestPeople.java @@ -852,6 +852,11 @@ public class TestPeople extends EnterpriseTestApi // TODO: temp fix, set back to orig firstName publicApiClient.setRequestContext(new RequestContext(account1.getId(), account1Admin, "admin")); people.update(personId, qjson("{ `firstName`:`Bill` }"), 200); + + // -ve test: check that required/mandatory/non-null fields cannot be unset (or empty string) + people.update("people", personId, null, null, qjson("{ `firstName`:`` }"), null, "Expected 400 response when updating " + personId, 400); + people.update("people", personId, null, null, qjson("{ `email`:`` }"), null, "Expected 400 response when updating " + personId, 400); + people.update("people", personId, null, null, qjson("{ `emailNotificationsEnabled`:`` }"), null, "Expected 400 response when updating " + personId, 400); } @Test @@ -996,7 +1001,6 @@ public class TestPeople extends EnterpriseTestApi + " \"location\":null,\n" + " \"company\": {\n" - + " \"organization\":null,\n" + " \"address1\":null,\n" + " \"address2\":null,\n" + " \"address3\":null,\n" @@ -1024,7 +1028,8 @@ public class TestPeople extends EnterpriseTestApi assertNull(updatedPerson.getLocation()); assertNotNull(updatedPerson.getCompany()); - assertNull(updatedPerson.getCompany().getOrganization()); + assertNotNull(updatedPerson.getCompany().getOrganization()); + assertNull(updatedPerson.getCompany().getAddress1()); assertNull(updatedPerson.getCompany().getAddress2()); assertNull(updatedPerson.getCompany().getAddress3()); @@ -1037,6 +1042,19 @@ public class TestPeople extends EnterpriseTestApi assertNull(updatedPerson.getTelephone()); assertNull(updatedPerson.getUserStatus()); + // test ability to unset company fields as a whole + response = people.update("people", personId, null, null, + "{\n" + + " \"company\": {} \n" + + "}", params, + "Expected 200 response when updating " + personId, 200); + + updatedPerson = Person.parsePerson((JSONObject) response.getJsonResponse().get("entry")); + + // note: empty company object is returned for backwards compatibility (with pre-existing getPerson API <= 5.1) + assertNotNull(updatedPerson.getCompany()); + assertNull(updatedPerson.getCompany().getOrganization()); + // set at least one company field String updatedOrgName = "another org"; @@ -1064,7 +1082,6 @@ public class TestPeople extends EnterpriseTestApi // note: empty company object is returned for backwards compatibility (with pre-existing getPerson API <= 5.1) assertNotNull(updatedPerson.getCompany()); - assertNull(updatedPerson.getCompany().getOrganization()); } @@ -1104,6 +1121,10 @@ public class TestPeople extends EnterpriseTestApi assertEquals(enabled, updatedPerson.isEnabled()); } + // -ve test: enabled flag cannot be null/empty + people.update("people", personId, null, null, qjson("{ `enabled`: null }"), null, "Expected 400 response when updating " + personId, 400); + people.update("people", personId, null, null, qjson("{ `enabled`: `` }"), null, "Expected 400 response when updating " + personId, 400); + // Use non-admin user's own credentials publicApiClient.setRequestContext(new RequestContext(account3.getId(), personId, "password"));