ACE-5943 / REPO-4132: Rejecting a record in RM doesn't work with 6.1.0-RC4 (#323)

The reject action in RM triggers an email notification from repo. Code in EMailNotificationProvider.buildTemplateModel(...) requests for the person associated with the current authenticated user.
Potentially another bug in Repository.getPerson() (next call in the stack trace), considers the current user to be AuthenticationUtil.getRunAsUser(), instead of the fully authenticated one. This calls, down the line, for the retrieval of the person for the username System, which ends up in trying to actually create the user System (which is not permitted), hence the exception.

The most simple (and safe) fix was to add a check for username System, in the low-level method PersonServiceImpl.getPersonImpl(...), and return null or throw NoSuchPersonException.

2 tests were added, for the Person service and for the email notification generation.

(cherry picked from master commit 3b7849bbdb)

src/main/java/org/alfresco/repo/security/person/PersonServiceImpl.java

@@ -493,6 +493,15 @@ public class PersonServiceImpl extends TransactionListenerAdapter implements Per
         {
             return null;
         }
+        if (EqualsHelper.nullSafeEquals(userName, AuthenticationUtil.getSystemUserName()))
+        {
+            if (exceptionOrNull)
+            {
+                throw new NoSuchPersonException(userName);
+            }
+            return null;
+        }
+        
         final NodeRef personNode = getPersonOrNullImpl(userName);
         if (personNode == null)
         {

src/test/java/org/alfresco/repo/notification/NotificationServiceImplSystemTest.java

@@ -32,7 +32,8 @@ import java.util.Map;
 import org.alfresco.model.ContentModel;
 import org.alfresco.repo.content.MimetypeMap;
 import org.alfresco.repo.model.Repository;
-import org.alfresco.repo.security.authentication.AuthenticationUtil;
+import org.alfresco.repo.security.authentication.AuthenticationUtil;
+import org.alfresco.repo.security.authentication.AuthenticationUtil.RunAsWork;
 import org.alfresco.repo.transaction.RetryingTransactionHelper.RetryingTransactionCallback;
 import org.alfresco.service.cmr.model.FileFolderService;
 import org.alfresco.service.cmr.notification.NotificationContext;
@@ -195,28 +196,34 @@ public class NotificationServiceImplSystemTest extends BaseAlfrescoTestCase
     protected boolean useSpacesStore()
     {
         return true;
-    }
+    }
+    
+    public void testSimpleEmailNotificationSystem()
+    {
+        AuthenticationUtil.runAsSystem(new RunAsWork<Void>()
+        {
+            @Override
+            public Void doWork()
+            {
+                NotificationContext context = new NotificationContext();
+                
+                context.setFrom(FROM_EMAIL);
+                context.addTo(TO_USER1);
+                context.setSubject(SUBJECT);
+                context.setBodyTemplate(template.toString());
+                
+                Map<String, Serializable> templateArgs = new HashMap<String, Serializable>(1);
+                templateArgs.put("template", template);
+                context.setTemplateArgs(templateArgs);
+                
+                notificationService.sendNotification(EMailNotificationProvider.NAME, context);
+                
+                return null;
+            }
+        });
+    }
+    
     
-    public void testSimpleEmailNotification()
-    {
-        doTestInTransaction(new Test<Void>()
-        {
-            @Override
-            public Void run()
-            {
-                NotificationContext context = new NotificationContext();
-                
-                context.setFrom(FROM_EMAIL);
-                context.addTo(TO_USER1);
-                context.setSubject(SUBJECT);
-                context.setBody(BODY);
-                
-                notificationService.sendNotification(EMailNotificationProvider.NAME, context);
-                
-                return null;
-            }
-        });
-    }
     
     public void testTemplateEmailNotification()
     {

src/test/java/org/alfresco/repo/security/person/PersonTest.java

@@ -47,6 +47,7 @@ import org.alfresco.query.PagingRequest;
 import org.alfresco.query.PagingResults;
 import org.alfresco.repo.policy.BehaviourFilter;
 import org.alfresco.repo.security.authentication.AuthenticationUtil;
+import org.alfresco.repo.security.authentication.AuthenticationUtil.RunAsWork;
 import org.alfresco.repo.security.authentication.MutableAuthenticationDao;
 import org.alfresco.repo.transaction.AlfrescoTransactionSupport;
 import org.alfresco.repo.transaction.AlfrescoTransactionSupport.TxnReadState;
@@ -1788,4 +1789,27 @@ public class PersonTest extends TestCase
             //  expect to go here
         }
     }
+    
+    public void testBuitInSystemUser()
+    {
+
+        AuthenticationUtil.runAsSystem(new RunAsWork<Void>()
+        {
+            @Override
+            public Void doWork()
+            {
+                try
+                {
+                    NodeRef person = personService.getPerson(AuthenticationUtil.SYSTEM_USER_NAME);
+                    fail("A NoSuchPersonException should have been thrown for " + AuthenticationUtil.SYSTEM_USER_NAME +
+                            " but " + person + " was returned");
+                }
+                catch(NoSuchPersonException ignore)
+                {
+                    // This is expected for system.;
+                }
+                return null;
+            }
+        });
+    }
 }