REPO-1813: 500 Response Code when creating a person without id

Now returns 400. Also fixed to return 400 when user id starts with GROUP_ or ROLE_

git-svn-id: https://svn.alfresco.com/repos/alfresco-enterprise/alfresco/BRANCHES/DEV/5.2.N/root@134714 c4b6b30b-aa2e-2d43-bbcb-ca4b014f7261

source/java/org/alfresco/rest/api/impl/PeopleImpl.java

@@ -50,11 +50,7 @@ import org.alfresco.service.cmr.repository.ContentService;
 import org.alfresco.service.cmr.repository.ContentWriter;
 import org.alfresco.service.cmr.repository.NodeRef;
 import org.alfresco.service.cmr.repository.NodeService;
-import org.alfresco.service.cmr.security.AuthenticationService;
+import org.alfresco.service.cmr.security.*;
-import org.alfresco.service.cmr.security.AuthorityService;
-import org.alfresco.service.cmr.security.MutableAuthenticationService;
-import org.alfresco.service.cmr.security.NoSuchPersonException;
-import org.alfresco.service.cmr.security.PersonService;
 import org.alfresco.service.cmr.site.SiteService;
 import org.alfresco.service.cmr.thumbnail.ThumbnailService;
 import org.alfresco.service.cmr.usage.ContentUsageService;
@@ -87,6 +83,11 @@ public class PeopleImpl implements People
 	private static final List<QName> EXCLUDED_ASPECTS = Arrays.asList();
 	private static final List<QName> EXCLUDED_PROPS = Arrays.asList();
     private static final int USERNAME_MAXLENGTH = 100;
+    private static final String[] RESERVED_AUTHORITY_PREFIXES =
+    {
+            PermissionService.GROUP_PREFIX,
+            PermissionService.ROLE_PREFIX
+    };
     protected Nodes nodes;
 	protected Sites sites;
 
@@ -512,12 +513,14 @@ public class PeopleImpl implements People
 
 	private void validateCreatePersonData(Person person)
 	{
-        validateUsername(person.getUserName());
+	    // Mandatory field checks first
-        validateNamespaces(person.getAspectNames(), person.getProperties());
 		checkRequiredField("id", person.getUserName());
 		checkRequiredField("firstName", person.getFirstName());
 		checkRequiredField("email", person.getEmail());
 		checkRequiredField("password", person.getPassword());
+
+        validateUsername(person.getUserName());
+        validateNamespaces(person.getAspectNames(), person.getProperties());
 	}
 
     private void validateUsername(String username)
@@ -531,6 +534,14 @@ public class PeopleImpl implements People
         {
             throw new IllegalArgumentException("Username contains characters that are not permitted.");
         }
+
+        for (String prefix : RESERVED_AUTHORITY_PREFIXES)
+        {
+            if (username.toUpperCase().startsWith(prefix))
+            {
+                throw new IllegalArgumentException("Username cannot start with the reserved prefix '"+prefix+"'.");
+            }
+        }
     }
 
     private void validateNamespaces(List<String> aspectNames, Map<String, Object> properties)

source/test-java/org/alfresco/rest/api/tests/TestPeople.java

@@ -300,6 +300,19 @@ public class TestPeople extends EnterpriseTestApi
         person.setUserName("myUser/Name@" + account1.getId());
         people.create(person, 400);
 
+        // check for reserved authority prefixes
+        person.setUserName("GROUP_EVERYONE");
+        people.create(person, 400);
+
+        person.setUserName("GROUP_mygroup");
+        people.create(person, 400);
+
+        person.setUserName("ROLE_ANYTHING");
+        people.create(person, 400);
+
+        // lower case
+        person.setUserName("role_whatever");
+        people.create(person, 400);
     }
 
     @Test
@@ -456,10 +469,14 @@ public class TestPeople extends EnterpriseTestApi
 
         // -ve: not enough fields!
         {
-            // Create a person with no fields set.
+            // Create a person with no fields other than user ID set.
             Person person = new Person();
             person.setUserName("joe.bloggs.2@"+account1.getId());
             people.create(person, 400);
+
+            // Missing ID
+            person.setUserName(null);
+            people.create(person, 400);
         }
     }