From 7fe1374edbb803a286d33d448e3c1e243640bf11 Mon Sep 17 00:00:00 2001 From: Alan Davis Date: Thu, 18 Sep 2014 17:17:42 +0000 Subject: [PATCH] Merged HEAD-BUG-FIX (5.0/Cloud) to HEAD (5.0/Cloud) 83943: Merged V4.2-BUG-FIX (4.2.4) to HEAD-BUG-FIX (5.0/Cloud) 82504: Merged V4.1-BUG-FIX (4.1.10) to V4.2-BUG-FIX (4.2.4) 81668: Merged DEV to V4.1-BUG-FIX (4.1.10) 80985 : MNT-12082 : Consumers can add comments using URL parameters, although they are not allowed to do so - Fixed the issue. Implemented a test to simulate the issue. 82513: Fix build failure after: Merged DEV to V4.1-BUG-FIX (4.1.10) 80985 : MNT-12082 : Consumers can add comments using URL parameters, although they are not allowed to do so - Fixed the issue. Implemented a test to simulate the issue. 83418: Add further hacks to the CommentsApiTest, which needs to be refactored to use RetryingTransactionHelper. 83679: MNT-12082 : Consumers can add comments using URL parameters, although they are not allowed to do so - Fixed build failure git-svn-id: https://svn.alfresco.com/repos/alfresco-enterprise/alfresco/HEAD/root@84601 c4b6b30b-aa2e-2d43-bbcb-ca4b014f7261 --- .../alfresco/repository/blogs/blogpost.lib.js | 2 + .../web/scripts/comments/CommentsPost.java | 9 +- .../repo/web/scripts/WebScriptTestSuite.java | 54 +++--- .../web/scripts/comment/CommentsApiTest.java | 177 ++++++++++++++++-- 4 files changed, 196 insertions(+), 46 deletions(-) diff --git a/config/alfresco/templates/webscripts/org/alfresco/repository/blogs/blogpost.lib.js b/config/alfresco/templates/webscripts/org/alfresco/repository/blogs/blogpost.lib.js index bb97754290..3fd174e74d 100644 --- a/config/alfresco/templates/webscripts/org/alfresco/repository/blogs/blogpost.lib.js +++ b/config/alfresco/templates/webscripts/org/alfresco/repository/blogs/blogpost.lib.js @@ -16,6 +16,8 @@ function setOrUpdateReleasedAndUpdatedDates(node) // only set if was previously draft - as only the owner/admin can do this if (!node.inheritsPermissions()) { + // MNT-12082 + node.removePermission("All", node.getOwner()); node.setInheritsPermissions(true); } diff --git a/source/java/org/alfresco/repo/web/scripts/comments/CommentsPost.java b/source/java/org/alfresco/repo/web/scripts/comments/CommentsPost.java index 7719fd7759..5194d5a836 100644 --- a/source/java/org/alfresco/repo/web/scripts/comments/CommentsPost.java +++ b/source/java/org/alfresco/repo/web/scripts/comments/CommentsPost.java @@ -1,5 +1,5 @@ /* - * Copyright (C) 2005-2013 Alfresco Software Limited. + * Copyright (C) 2005-2014 Alfresco Software Limited. * * This file is part of Alfresco * @@ -318,13 +318,17 @@ public class CommentsPost extends DeclarativeWebScript { public NodeRef doWork() throws Exception { NodeRef commentsFolder = null; + AuthenticationUtil.pushAuthentication(); // ALF-5240: turn off auditing round the discussion node creation to prevent // the source document from being modified by the first user leaving a comment behaviourFilter.disableBehaviour(nodeRef, ContentModel.ASPECT_AUDITABLE); try - { + { + // MNT-12082: set System user for creating forumFolder and commentsFolder nodes + AuthenticationUtil.setFullyAuthenticatedUser(AuthenticationUtil.getSystemUserName()); + nodeService.addAspect(nodeRef, QName.createQName(NamespaceService.FORUMS_MODEL_1_0_URI, "discussable"), null); nodeService.addAspect(nodeRef, QName.createQName(NamespaceService.FORUMS_MODEL_1_0_URI, "commentsRollup"), null); List assocs = nodeService.getChildAssocs(nodeRef, QName.createQName(NamespaceService.FORUMS_MODEL_1_0_URI, "discussion"), RegexQNamePattern.MATCH_ALL); @@ -344,6 +348,7 @@ public class CommentsPost extends DeclarativeWebScript { } finally { + AuthenticationUtil.popAuthentication(); behaviourFilter.enableBehaviour(nodeRef, ContentModel.ASPECT_AUDITABLE); } diff --git a/source/test-java/org/alfresco/repo/web/scripts/WebScriptTestSuite.java b/source/test-java/org/alfresco/repo/web/scripts/WebScriptTestSuite.java index c178a6d6e6..2c8f456314 100644 --- a/source/test-java/org/alfresco/repo/web/scripts/WebScriptTestSuite.java +++ b/source/test-java/org/alfresco/repo/web/scripts/WebScriptTestSuite.java @@ -1,5 +1,5 @@ /* - * Copyright (C) 2005-2012 Alfresco Software Limited. + * Copyright (C) 2005-2014 Alfresco Software Limited. * * This file is part of Alfresco * @@ -26,6 +26,7 @@ import org.alfresco.repo.web.scripts.activities.feed.control.FeedControlTest; import org.alfresco.repo.web.scripts.admin.AdminWebScriptTest; import org.alfresco.repo.web.scripts.audit.AuditWebScriptTest; import org.alfresco.repo.web.scripts.blogs.BlogServiceTest; +import org.alfresco.repo.web.scripts.comment.CommentsApiTest; import org.alfresco.repo.web.scripts.dictionary.DictionaryRestApiTest; import org.alfresco.repo.web.scripts.discussion.DiscussionRestApiTest; import org.alfresco.repo.web.scripts.facet.FacetRestApiTest; @@ -68,35 +69,36 @@ public class WebScriptTestSuite extends TestSuite suite.addTestSuite( QuickShareRestApiTest.class ); suite.addTestSuite( AdminWebScriptTest.class ); suite.addTestSuite( AuditWebScriptTest.class ); - suite.addTestSuite( BlogServiceTest.class ); - suite.addTestSuite( DictionaryRestApiTest.class ); - suite.addTestSuite( DiscussionRestApiTest.class ); - suite.addTestSuite( FeedControlTest.class ); - suite.addTestSuite( FormRestApiGet_Test.class ); - suite.addTestSuite( FormRestApiJsonPost_Test.class ); - suite.addTestSuite( GroupsTest.class ); - suite.addTestSuite( InvitationWebScriptTest.class ); - suite.addTestSuite( InviteServiceTest.class ); - suite.addTestSuite( LoginTest.class ); - suite.addTestSuite( PersonSearchTest.class ); - suite.addTestSuite( PersonServiceTest.class ); - suite.addTestSuite( PreferenceServiceTest.class ); + suite.addTestSuite( BlogServiceTest.class ); + suite.addTestSuite( DictionaryRestApiTest.class ); + suite.addTestSuite( DiscussionRestApiTest.class ); + suite.addTestSuite( FeedControlTest.class ); + suite.addTestSuite( FormRestApiGet_Test.class ); + suite.addTestSuite( FormRestApiJsonPost_Test.class ); + suite.addTestSuite( GroupsTest.class ); + suite.addTestSuite( InvitationWebScriptTest.class ); + suite.addTestSuite( InviteServiceTest.class ); + suite.addTestSuite( LoginTest.class ); + suite.addTestSuite( PersonSearchTest.class ); + suite.addTestSuite( PersonServiceTest.class ); + suite.addTestSuite( PreferenceServiceTest.class ); suite.addTestSuite( RatingRestApiTest.class ); - suite.addTestSuite( ReplicationRestApiTest.class ); - suite.addTestSuite( RepositoryContainerTest.class ); - suite.addTestSuite( RuleServiceTest.class ); - suite.addTestSuite( RunningActionRestApiTest.class ); - suite.addTestSuite( SiteServiceTest.class ); - suite.addTestSuite( TaggingServiceTest.class ); - suite.addTestSuite( ThumbnailServiceTest.class ); - suite.addTestSuite( TransferWebScriptTest.class ); - suite.addTestSuite( WorkflowModelBuilderTest.class ); - suite.addTestSuite( ActivitiWorkflowRestApiTest.class ); - suite.addTestSuite( JBPMWorkflowRestApiTest.class ); + suite.addTestSuite( ReplicationRestApiTest.class ); + suite.addTestSuite( RepositoryContainerTest.class ); + suite.addTestSuite( RuleServiceTest.class ); + suite.addTestSuite( RunningActionRestApiTest.class ); + suite.addTestSuite( SiteServiceTest.class ); + suite.addTestSuite( TaggingServiceTest.class ); + suite.addTestSuite( ThumbnailServiceTest.class ); + suite.addTestSuite( TransferWebScriptTest.class ); + suite.addTestSuite( WorkflowModelBuilderTest.class ); + suite.addTestSuite( ActivitiWorkflowRestApiTest.class ); + suite.addTestSuite( JBPMWorkflowRestApiTest.class ); suite.addTestSuite( PublishingRestApiTest.class ); suite.addTestSuite( SOLRWebScriptTest.class ); suite.addTestSuite( SubscriptionServiceRestApiTest.class ); suite.addTestSuite( FacetRestApiTest.class ); + suite.addTestSuite( CommentsApiTest.class ); // This uses a slightly different context // As such, we can't run it in the same suite as the others, @@ -105,4 +107,4 @@ public class WebScriptTestSuite extends TestSuite return suite; } -} +} diff --git a/source/test-java/org/alfresco/repo/web/scripts/comment/CommentsApiTest.java b/source/test-java/org/alfresco/repo/web/scripts/comment/CommentsApiTest.java index 1c4d5f2863..910918a8a8 100644 --- a/source/test-java/org/alfresco/repo/web/scripts/comment/CommentsApiTest.java +++ b/source/test-java/org/alfresco/repo/web/scripts/comment/CommentsApiTest.java @@ -1,3 +1,21 @@ +/* + * Copyright (C) 2005-2014 Alfresco Software Limited. + * + * This file is part of Alfresco + * + * Alfresco is free software: you can redistribute it and/or modify + * it under the terms of the GNU Lesser General Public License as published by + * the Free Software Foundation, either version 3 of the License, or + * (at your option) any later version. + * + * Alfresco is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU Lesser General Public License for more details. + * + * You should have received a copy of the GNU Lesser General Public License + * along with Alfresco. If not, see . + */ package org.alfresco.repo.web.scripts.comment; import java.text.MessageFormat; @@ -7,13 +25,15 @@ import javax.transaction.UserTransaction; import org.alfresco.error.AlfrescoRuntimeException; import org.alfresco.model.ContentModel; +import org.alfresco.repo.node.archive.NodeArchiveService; import org.alfresco.repo.security.authentication.AuthenticationComponent; import org.alfresco.repo.security.authentication.AuthenticationUtil; import org.alfresco.repo.security.permissions.PermissionReference; import org.alfresco.repo.security.permissions.PermissionServiceSPI; import org.alfresco.repo.security.permissions.impl.ModelDAO; -import org.alfresco.repo.security.authentication.MutableAuthenticationDao; import org.alfresco.repo.security.permissions.impl.SimplePermissionEntry; +import org.alfresco.repo.security.authentication.MutableAuthenticationDao; +import org.alfresco.repo.site.SiteModel; import org.alfresco.repo.web.scripts.BaseWebScriptTest; import org.alfresco.service.cmr.model.FileFolderService; import org.alfresco.service.cmr.repository.NodeRef; @@ -24,19 +44,31 @@ import org.alfresco.service.cmr.security.AccessStatus; import org.alfresco.service.cmr.security.MutableAuthenticationService; import org.alfresco.service.cmr.security.PermissionService; import org.alfresco.service.cmr.security.PersonService; +import org.alfresco.service.cmr.site.SiteInfo; +import org.alfresco.service.cmr.site.SiteService; +import org.alfresco.service.cmr.site.SiteVisibility; import org.alfresco.service.cmr.version.VersionService; import org.alfresco.service.namespace.NamespaceService; +import org.alfresco.service.namespace.QName; import org.alfresco.service.transaction.TransactionService; import org.alfresco.util.PropertyMap; import org.springframework.context.ApplicationContext; +import org.springframework.extensions.webscripts.Status; import org.springframework.extensions.webscripts.TestWebScriptServer.PostRequest; import org.springframework.extensions.webscripts.TestWebScriptServer.Response; +/** + * TODO: Fix the loose transaction handling. + */ public class CommentsApiTest extends BaseWebScriptTest { private static final String URL_POST_COMMENT = "api/node/{0}/{1}/{2}/comments"; - private static final String JSON = "application/json"; + private static final String SITE_SHORT_NAME = "SomeTestSiteShortName"; + private static final String USER_ONE = "SomeTestUserOne"; + private static final String USER_TWO = "SomeTestUserTwo"; + + private String requestBodyJson = "{\"title\" : \"Test Title\", \"content\" : \"Test Comment\"}"; private FileFolderService fileFolderService; private TransactionService transactionService; @@ -49,18 +81,19 @@ public class CommentsApiTest extends BaseWebScriptTest private AuthenticationComponent authenticationComponent; protected PermissionServiceSPI permissionService; protected ModelDAO permissionModelDAO; - private MutableAuthenticationDao authenticationDAO; private NodeRef rootNodeRef; private NodeRef companyHomeNodeRef; private NodeRef nodeRef; + private NodeRef sitePage; private static final String USER_TEST = "UserTest"; private UserTransaction txn; private String USER2 = "user2"; - + private SiteService siteService; + private NodeArchiveService nodeArchiveService; @Override protected void setUp() throws Exception @@ -75,11 +108,13 @@ public class CommentsApiTest extends BaseWebScriptTest namespaceService = (NamespaceService)appContext.getBean("namespaceService"); versionService = (VersionService)appContext.getBean("versionService"); personService = (PersonService)getServer().getApplicationContext().getBean("PersonService"); - authenticationDAO = (MutableAuthenticationDao) appContext.getBean("authenticationDao"); authenticationService = (MutableAuthenticationService)getServer().getApplicationContext().getBean("AuthenticationService"); authenticationComponent = (AuthenticationComponent)getServer().getApplicationContext().getBean("authenticationComponent"); permissionService = (PermissionServiceSPI) getServer().getApplicationContext().getBean("permissionService"); permissionModelDAO = (ModelDAO) getServer().getApplicationContext().getBean("permissionsModelDAO"); + siteService = (SiteService)getServer().getApplicationContext().getBean("SiteService"); + personService = (PersonService)getServer().getApplicationContext().getBean("PersonService"); + nodeArchiveService = (NodeArchiveService)getServer().getApplicationContext().getBean("nodeArchiveService"); AuthenticationUtil.setFullyAuthenticatedUser(AuthenticationUtil.getAdminUserName()); @@ -105,22 +140,42 @@ public class CommentsApiTest extends BaseWebScriptTest versionService.ensureVersioningEnabled(nodeRef, null); nodeService.setProperty(nodeRef, ContentModel.PROP_AUTO_VERSION_PROPS, true); - if (authenticationDAO.userExists(USER2)) - { - authenticationService.deleteAuthentication(USER2); - } - authenticationService.createAuthentication(USER2, USER2.toCharArray()); - + createUser(USER2); createUser(USER_TEST); txn.commit(); AuthenticationUtil.clearCurrentSecurityContext(); + + // MNT-12082 + // Authenticate as admin + AuthenticationUtil.setAdminUserAsFullyAuthenticatedUser(); + + // Create test site + // - only create the site if it doesn't already exist + SiteInfo siteInfo = siteService.getSite(SITE_SHORT_NAME); + if (siteInfo == null) + { + siteInfo = siteService.createSite("SomeTestSite", SITE_SHORT_NAME, "SiteTitle", "SiteDescription", SiteVisibility.PUBLIC); + } + + txn = transactionService.getUserTransaction(); + txn.begin(); + + // Create users + createUser(USER_ONE, SiteModel.SITE_CONSUMER); + createUser(USER_TWO, SiteModel.SITE_CONTRIBUTOR); + + // Create site page + sitePage = nodeService.createNode(siteInfo.getNodeRef(), + ContentModel.ASSOC_CONTAINS, + QName.createQName(NamespaceService.CONTENT_MODEL_PREFIX, "test"), + ContentModel.TYPE_CONTENT).getChildRef(); + + txn.commit(); + } - /* (non-Javadoc) - * @see junit.framework.TestCase#tearDown() - */ @Override protected void tearDown() throws Exception { @@ -132,12 +187,29 @@ public class CommentsApiTest extends BaseWebScriptTest // delete the discussions users if(personService.personExists(USER_TEST)) { + // delete invite site personService.deletePerson(USER_TEST); + + // delete the users } if (authenticationService.authenticationExists(USER_TEST)) { authenticationService.deleteAuthentication(USER_TEST); } + + AuthenticationUtil.setAdminUserAsFullyAuthenticatedUser(); + + SiteInfo siteInfo = this.siteService.getSite(SITE_SHORT_NAME); + if (siteInfo != null) + { + // delete invite site + siteService.deleteSite(SITE_SHORT_NAME); + nodeArchiveService.purgeArchivedNode(nodeArchiveService.getArchivedNode(siteInfo.getNodeRef())); + } + + // delete the users + deleteUser(USER_ONE); + deleteUser(USER_TWO); } private void addComment(NodeRef nodeRef, String user, int status) throws Exception @@ -151,14 +223,23 @@ public class CommentsApiTest extends BaseWebScriptTest AuthenticationUtil.setFullyAuthenticatedUser(user); StringBuilder body = new StringBuilder("{"); - body.append("'title' : 'Test Title', "); - body.append("'content' : 'Test Comment'"); + body.append("\"title\" : \"Test Title\", "); + body.append("\"content\" : \"Test Comment\""); body.append("}"); response = sendRequest(new PostRequest(MessageFormat.format(URL_POST_COMMENT, new Object[] {nodeRef.getStoreRef().getProtocol(), nodeRef.getStoreRef().getIdentifier(), nodeRef.getId()}), body.toString(), JSON), status); assertEquals(status, response.getStatus()); - txn.commit(); + // Normally, webscripts are in their own transaction. The test infrastructure here forces us to have a transaction + // around the calls. if the WebScript fails, then we should rollback. + if (response.getStatus() == 500) + { + txn.rollback(); + } + else + { + txn.commit(); + } } private String getCurrentVersion(NodeRef nodeRef) throws Exception @@ -195,9 +276,13 @@ public class CommentsApiTest extends BaseWebScriptTest //Contributor should be able to add comments addComment(contentForUserContributor, USER_TEST, 200); + + txn.commit(); // Hack. Internally, the addComment starts and rolls back the next txn. //Consumer shouldn't be able to add comments see MNT-9883 addComment(contentForUserConsumer, USER_TEST, 500); + txn = transactionService.getUserTransaction(); + txn.begin(); nodeService.deleteNode(contentForUserContributor); nodeService.deleteNode(contentForUserConsumer); @@ -236,7 +321,6 @@ public class CommentsApiTest extends BaseWebScriptTest /** * MNT-9771 - * @throws Exception */ public void testCommentDoesNotChangeModifier() throws Exception { @@ -247,4 +331,61 @@ public class CommentsApiTest extends BaseWebScriptTest assertEquals(modifierBefore, modifierAfter); } + /** + * MNT-12082 + */ + public void testConsumerCanNotComment() throws Exception + { + // Authenticate as consumer + authenticationService.authenticate(USER_ONE, USER_ONE.toCharArray()); + + String uri = MessageFormat.format(URL_POST_COMMENT, new Object[] {sitePage.getStoreRef().getProtocol(), sitePage.getStoreRef().getIdentifier(), sitePage.getId()}); + Response response = sendRequest(new PostRequest(uri, requestBodyJson, JSON), + Status.STATUS_INTERNAL_SERVER_ERROR); + assertEquals(Status.STATUS_INTERNAL_SERVER_ERROR, response.getStatus()); + } + + /** + * MNT-12082 + */ + public void testContributorCanComment() throws Exception + { + // Authenticate as contributor + authenticationService.authenticate(USER_TWO, USER_TWO.toCharArray()); + + String uri = MessageFormat.format(URL_POST_COMMENT, new Object[] {sitePage.getStoreRef().getProtocol(), sitePage.getStoreRef().getIdentifier(), sitePage.getId()}); + Response response = sendRequest(new PostRequest(uri, requestBodyJson, JSON), Status.STATUS_OK); + assertEquals(Status.STATUS_OK, response.getStatus()); + } + + private void createUser(String userName, String role) + { + // if user with given user name doesn't already exist then create user + if (authenticationService.authenticationExists(userName) == false) + { + authenticationService.createAuthentication(userName, userName.toCharArray()); + + // create person properties + PropertyMap personProps = new PropertyMap(); + personProps.put(ContentModel.PROP_USERNAME, userName); + personProps.put(ContentModel.PROP_FIRSTNAME, "FirstName123"); + personProps.put(ContentModel.PROP_LASTNAME, "LastName123"); + personProps.put(ContentModel.PROP_EMAIL, "FirstName123.LastName123@email.com"); + personProps.put(ContentModel.PROP_JOBTITLE, "JobTitle123"); + personProps.put(ContentModel.PROP_JOBTITLE, "Organisation123"); + + personService.createPerson(personProps); + } + + siteService.setMembership(SITE_SHORT_NAME, userName, role); + } + + private void deleteUser(String user) + { + personService.deletePerson(user); + if (authenticationService.authenticationExists(user)) + { + authenticationService.deleteAuthentication(user); + } + } }