From 5cf093cbaf89cb45d95f153d3db0a89907b61ed7 Mon Sep 17 00:00:00 2001 From: Alan Davis Date: Thu, 17 Jan 2019 13:33:24 +0000 Subject: [PATCH] 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 3b7849bbdb8ffb4c277c5ef16ba248590ddc87cd) --- .../security/person/PersonServiceImpl.java | 9 ++++ .../NotificationServiceImplSystemTest.java | 51 +++++++++++-------- .../repo/security/person/PersonTest.java | 24 +++++++++ 3 files changed, 62 insertions(+), 22 deletions(-) diff --git a/src/main/java/org/alfresco/repo/security/person/PersonServiceImpl.java b/src/main/java/org/alfresco/repo/security/person/PersonServiceImpl.java index d3ed61652c..0c4a21bdc4 100644 --- a/src/main/java/org/alfresco/repo/security/person/PersonServiceImpl.java +++ b/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) { diff --git a/src/test/java/org/alfresco/repo/notification/NotificationServiceImplSystemTest.java b/src/test/java/org/alfresco/repo/notification/NotificationServiceImplSystemTest.java index a9be112392..446486ccb5 100644 --- a/src/test/java/org/alfresco/repo/notification/NotificationServiceImplSystemTest.java +++ b/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() + { + @Override + public Void doWork() + { + NotificationContext context = new NotificationContext(); + + context.setFrom(FROM_EMAIL); + context.addTo(TO_USER1); + context.setSubject(SUBJECT); + context.setBodyTemplate(template.toString()); + + Map templateArgs = new HashMap(1); + templateArgs.put("template", template); + context.setTemplateArgs(templateArgs); + + notificationService.sendNotification(EMailNotificationProvider.NAME, context); + + return null; + } + }); + } + - public void testSimpleEmailNotification() - { - doTestInTransaction(new Test() - { - @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() { diff --git a/src/test/java/org/alfresco/repo/security/person/PersonTest.java b/src/test/java/org/alfresco/repo/security/person/PersonTest.java index 5f04b9d9a0..e4b5d056f1 100644 --- a/src/test/java/org/alfresco/repo/security/person/PersonTest.java +++ b/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() + { + @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; + } + }); + } }