REPO-1882: Create a group or a person with an id that contains backslash does not throw an error

Added an char array containing illegall characters (/,\,\n,\r) to Groups and People API's, and adapted the checking of the previous illlegal character (/), to verify all of them.
      Added two tests (one for group and one for people), creating using invalid id's.

src/main/java/org/alfresco/rest/api/impl/GroupsImpl.java

@@ -86,6 +86,7 @@ public class GroupsImpl implements Groups
     private static final String DISPLAY_NAME = "displayName";
     private static final String AUTHORITY_NAME = "authorityName";
     private static final String ERR_MSG_MODIFY_FIXED_AUTHORITY = "Trying to modify a fixed authority";
+    private static final char[] illegalCharacters = {'/', '\\', '\r', '\n'};
 
     private final static Map<String, String> SORT_PARAMS_TO_NAMES;
     static
@@ -933,17 +934,21 @@ public class GroupsImpl implements Groups
 
         if (!isUpdate)
         {
-            if (group.getId() == null || group.getId().isEmpty())
+            String groupId = group.getId();
+            if (groupId == null || groupId.isEmpty())
             {
                 throw new InvalidArgumentException("groupId is null or empty");
             }
 
-            if (group.getId().indexOf('/') != -1)
+            for (char illegalCharacter : illegalCharacters)
             {
-                throw new IllegalArgumentException("groupId contains characters that are not permitted.");
+                if (groupId.indexOf(illegalCharacter) != -1)
+                {
+                    throw new IllegalArgumentException("groupId contains characters that are not permitted: "+groupId.charAt(groupId.indexOf(illegalCharacter)));
+                }
             }
 
-            if (groupAuthorityExists(group.getId()))
+            if (groupAuthorityExists(groupId))
             {
                 throw new ConstraintViolatedException("Group '" + group.getId() + "' already exists.");
             }

src/main/java/org/alfresco/rest/api/impl/PeopleImpl.java

@@ -97,14 +97,15 @@ public class PeopleImpl implements People
             NamespaceService.SYSTEM_MODEL_1_0_URI,
             "http://www.alfresco.org/model/user/1.0",
             NamespaceService.CONTENT_MODEL_1_0_URI);
-	private static final List<QName> EXCLUDED_ASPECTS = Arrays.asList();
+    private static final List<QName> EXCLUDED_ASPECTS = Arrays.asList();
-	private static final List<QName> EXCLUDED_PROPS = 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
     };
+    private static final char[] illegalCharacters = {'/', '\\', '\r', '\n'};
 
     protected Nodes nodes;
 	protected Sites sites;
@@ -544,10 +545,10 @@ public class PeopleImpl implements People
         return person;
     }
 
-	@Override
+    @Override
-	public Person create(Person person)
+    public Person create(Person person)
-	{
+    {
-		validateCreatePersonData(person);
+        validateCreatePersonData(person);
 
         if (! isAdminAuthority())
         {
@@ -556,15 +557,15 @@ public class PeopleImpl implements People
             throw new PermissionDeniedException();
         }
 
-		// Unfortunately PersonService.createPerson(...) only throws an AlfrescoRuntimeException
+        // Unfortunately PersonService.createPerson(...) only throws an AlfrescoRuntimeException
-		// rather than a more specific exception and does not use a message ID either, so there's
+        // rather than a more specific exception and does not use a message ID either, so there's
-		// no sensible way to know that it was thrown due to the user already existing - hence this check here.
+        // no sensible way to know that it was thrown due to the user already existing - hence this check here.
-		if (personService.personExists(person.getUserName()))
+        if (personService.personExists(person.getUserName()))
-		{
+        {
-			throw new ConstraintViolatedException("Person '"+person.getUserName()+"' already exists.");
+            throw new ConstraintViolatedException("Person '" + person.getUserName() + "' already exists.");
-		}
+        }
 
-		// set enabled default value true
+        // set enabled default value true
         if (person.isEnabled() == null)
         {
             person.setEnabled(true);
@@ -627,17 +628,17 @@ public class PeopleImpl implements People
         });
     }
 
-	private void validateCreatePersonData(Person person)
+    private void validateCreatePersonData(Person person)
-	{
+    {
-	    // Mandatory field checks first
+        // Mandatory field checks first
-		checkRequiredField("id", person.getUserName());
+        checkRequiredField("id", person.getUserName());
-		checkRequiredField("firstName", person.getFirstName());
+        checkRequiredField("firstName", person.getFirstName());
-		checkRequiredField("email", person.getEmail());
+        checkRequiredField("email", person.getEmail());
-		checkRequiredField("password", person.getPassword());
+        checkRequiredField("password", person.getPassword());
 
         validateUsername(person.getUserName());
         validateNamespaces(person.getAspectNames(), person.getProperties());
-	}
+    }
 
     private void validateUsername(String username)
     {
@@ -646,9 +647,12 @@ public class PeopleImpl implements People
             throw new InvalidArgumentException("Username exceeds max length of " + USERNAME_MAXLENGTH + " characters.");
         }
 
-        if (username.indexOf('/') != -1)
+        for (char illegalCharacter : illegalCharacters)
         {
-            throw new IllegalArgumentException("Username contains characters that are not permitted.");
+            if (username.indexOf(illegalCharacter) != -1)
+            {
+                throw new IllegalArgumentException("Username contains characters that are not permitted: "+username.charAt(username.indexOf(illegalCharacter)));
+            }
         }
 
         for (String prefix : RESERVED_AUTHORITY_PREFIXES)

src/test/java/org/alfresco/rest/api/tests/GroupsTest.java

@@ -1438,6 +1438,15 @@ public class GroupsTest extends AbstractSingleNetworkSiteTest
             groupsProxy.createGroup(group, null, HttpServletResponse.SC_CONFLICT);
         }
 
+        // Create Group with an id that contains "\" should return an error.
+        {
+            setRequestContext(networkOne.getId(), networkAdmin, DEFAULT_ADMIN_PWD);
+
+            Group group = new Group();
+            group.setId("te\\st");
+            groupsProxy.createGroup(group, null, HttpServletResponse.SC_BAD_REQUEST);
+        }
+
         // Create subgroup with invalid parent.
         {
             setRequestContext(networkOne.getId(), networkAdmin, DEFAULT_ADMIN_PWD);

src/test/java/org/alfresco/rest/api/tests/TestPeople.java

@@ -608,7 +608,26 @@ public class TestPeople extends AbstractBaseApiTest
             people.create(person, 400);
         }
     }
-    
+
+    /**
+     * Created for REPO-1882, '\\', '\r' and '\n' marked as invalid character for personId
+     */
+    @Test
+    public void testCreatePersonWithInvalidCharacter () throws Exception
+    {
+        publicApiClient.setRequestContext(new RequestContext(account1.getId(), account1Admin, "admin"));
+
+        Person person = new Person();
+        String personId = UUID.randomUUID().toString()+"\\";
+        person.setUserName(personId);
+        person.setFirstName("Joe");
+        person.setEmail(personId+"@"+account1.getId());
+        person.setEnabled(true);
+        person.setPassword("password123");
+
+        people.create(person, 400);
+    }
+
     @Test
     public void testGetPerson_withCustomProps() throws PublicApiException
     {