From 1a5408636e9f7ce9245d61c1ca2ce9e921a26d30 Mon Sep 17 00:00:00 2001 From: Nick Burch Date: Mon, 3 Oct 2011 10:21:23 +0000 Subject: [PATCH] ALF-2404 Correct the site membership role listing for user with all-numeric usernames git-svn-id: https://svn.alfresco.com/repos/alfresco-enterprise/alfresco/HEAD/root@30903 c4b6b30b-aa2e-2d43-bbcb-ca4b014f7261 --- .../site/membership/memberships.get.js | 74 --------- .../web-scripts-application-context.xml | 19 ++- .../scripts/site/AbstractSiteWebScript.java | 77 +++++++++ .../repo/web/scripts/site/SiteExportGet.java | 11 +- .../web/scripts/site/SiteMembershipsGet.java | 150 ++++++++++++++++++ .../web/scripts/site/SiteServiceTest.java | 52 ++++++ 6 files changed, 299 insertions(+), 84 deletions(-) delete mode 100644 config/alfresco/templates/webscripts/org/alfresco/repository/site/membership/memberships.get.js create mode 100644 source/java/org/alfresco/repo/web/scripts/site/AbstractSiteWebScript.java create mode 100644 source/java/org/alfresco/repo/web/scripts/site/SiteMembershipsGet.java diff --git a/config/alfresco/templates/webscripts/org/alfresco/repository/site/membership/memberships.get.js b/config/alfresco/templates/webscripts/org/alfresco/repository/site/membership/memberships.get.js deleted file mode 100644 index 459abf3e80..0000000000 --- a/config/alfresco/templates/webscripts/org/alfresco/repository/site/membership/memberships.get.js +++ /dev/null @@ -1,74 +0,0 @@ -/** - * Implementation of list memberships - */ - -function main() -{ - // Get the site id - var shortName = url.extension.split("/")[0]; - var site = siteService.getSite(shortName); - - // get the filters - var nameFilter = args["nf"]; - var roleFilter = args["rf"]; - var authorityType = args["authorityType"]; - var sizeString = args["size"]; - var collapseGroups = false; - - if (authorityType != null) - { - if (authorityType.match("USER|GROUP") == null) - { - status.setCode(status.STATUS_BAD_REQUEST, "The 'authorityType' argument must be either USER or GROUP."); - return; - } - if (authorityType == "USER") - { - collapseGroups = true; - } - } - - var sizeSearch = 0; - if(sizeString != null) - { - sizeSearch = parseInt(sizeString); - } - - // Get the filtered memberships - // Comes back as a Map containing the full authority name and role - var memberships = site.listMembers(nameFilter, roleFilter, sizeSearch, collapseGroups); - - // Get a list of all the users resolved to person nodes - var authorities = Array(memberships.length); - var roles = {}; - - for (userName in memberships) - { - var membershipRole = memberships[userName]; - if (userName.match("^GROUP_")) - { - if (authorityType == null || authorityType == "GROUP") - { - // make sure the keys are strings - as usernames may be all numbers! - authorities['_' + userName] = groups.getGroupForFullAuthorityName(userName); - roles['_' + userName] = membershipRole; - } - } - else - { - if (authorityType == null || authorityType == "USER") - { - // make sure the keys are strings - as usernames may be all numbers! - authorities['_' + userName] = people.getPerson(userName); - roles['_' + userName] = membershipRole; - } - } - } - - // Pass the information to the template - model.site = site; - model.roles = roles; - model.authorities = authorities; -} - -main(); \ No newline at end of file diff --git a/config/alfresco/web-scripts-application-context.xml b/config/alfresco/web-scripts-application-context.xml index c23de12a7a..734a15d6ec 100644 --- a/config/alfresco/web-scripts-application-context.xml +++ b/config/alfresco/web-scripts-application-context.xml @@ -712,15 +712,30 @@ + + + + + + + + + + + + - + - + + diff --git a/source/java/org/alfresco/repo/web/scripts/site/AbstractSiteWebScript.java b/source/java/org/alfresco/repo/web/scripts/site/AbstractSiteWebScript.java new file mode 100644 index 0000000000..14d153d67e --- /dev/null +++ b/source/java/org/alfresco/repo/web/scripts/site/AbstractSiteWebScript.java @@ -0,0 +1,77 @@ +/* + * Copyright (C) 2005-2011 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.site; + +import java.util.Map; + +import org.alfresco.service.cmr.security.AuthorityService; +import org.alfresco.service.cmr.site.SiteInfo; +import org.alfresco.service.cmr.site.SiteService; +import org.springframework.extensions.webscripts.Cache; +import org.springframework.extensions.webscripts.DeclarativeWebScript; +import org.springframework.extensions.webscripts.Status; +import org.springframework.extensions.webscripts.WebScriptException; +import org.springframework.extensions.webscripts.WebScriptRequest; + +/** + * @author Nick Burch + * @since 4.0 + */ +public abstract class AbstractSiteWebScript extends DeclarativeWebScript +{ + protected SiteService siteService; + protected AuthorityService authorityService; + + public void setSiteService(SiteService siteService) + { + this.siteService = siteService; + } + + public void setAuthorityService(AuthorityService authorityService) + { + this.authorityService = authorityService; + } + + + protected static String buildSiteGroup(SiteInfo site) + { + return "GROUP_site_" + site.getShortName(); + } + + @Override + protected Map executeImpl(WebScriptRequest req, + Status status, Cache cache) + { + // Grab the site + String siteName = + req.getServiceMatch().getTemplateVars().get("shortname"); + SiteInfo site = siteService.getSite(siteName); + if (site == null) + { + throw new WebScriptException( + Status.STATUS_NOT_FOUND, + "No Site found with that short name"); + } + + // Process + return executeImpl(site, req, status, cache); + } + protected abstract Map executeImpl(SiteInfo site, + WebScriptRequest req, Status status, Cache cache); +} \ No newline at end of file diff --git a/source/java/org/alfresco/repo/web/scripts/site/SiteExportGet.java b/source/java/org/alfresco/repo/web/scripts/site/SiteExportGet.java index 63d589bad1..4de38f7d24 100644 --- a/source/java/org/alfresco/repo/web/scripts/site/SiteExportGet.java +++ b/source/java/org/alfresco/repo/web/scripts/site/SiteExportGet.java @@ -167,7 +167,7 @@ public class SiteExportGet extends AbstractWebScript protected void doPeopleACPExport(SiteInfo site, CloseIgnoringOutputStream writeTo) throws IOException { // Find the root group - String siteGroup = buildSiteGroup(site); + String siteGroup = AbstractSiteWebScript.buildSiteGroup(site); // Now get all people in the child groups Set siteUsers = authorityService.getContainedAuthorities( @@ -205,7 +205,7 @@ public class SiteExportGet extends AbstractWebScript protected void doGroupExport(SiteInfo site, CloseIgnoringOutputStream writeTo) throws IOException { // Find the root group - String siteGroup = buildSiteGroup(site); + String siteGroup = AbstractSiteWebScript.buildSiteGroup(site); // Get all the child groups of the site (but not children of them) Set siteGroups = authorityService.getContainedAuthorities( @@ -265,7 +265,7 @@ public class SiteExportGet extends AbstractWebScript List exportNodes = new ArrayList(); // Identify all the users - String siteGroup = buildSiteGroup(site); + String siteGroup = AbstractSiteWebScript.buildSiteGroup(site); Set siteUsers = authorityService.getContainedAuthorities( AuthorityType.USER, siteGroup, false); @@ -302,11 +302,6 @@ public class SiteExportGet extends AbstractWebScript exporterService.exportView(handler, parameters, null); } - protected String buildSiteGroup(SiteInfo site) - { - return "GROUP_site_" + site.getShortName(); - } - protected static class CloseIgnoringOutputStream extends FilterOutputStream { public CloseIgnoringOutputStream(OutputStream out) diff --git a/source/java/org/alfresco/repo/web/scripts/site/SiteMembershipsGet.java b/source/java/org/alfresco/repo/web/scripts/site/SiteMembershipsGet.java new file mode 100644 index 0000000000..803e06e4b1 --- /dev/null +++ b/source/java/org/alfresco/repo/web/scripts/site/SiteMembershipsGet.java @@ -0,0 +1,150 @@ +/* + * Copyright (C) 2005-2011 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.site; + +import java.util.HashMap; +import java.util.LinkedHashMap; +import java.util.Map; + +import org.alfresco.repo.security.authority.script.ScriptAuthorityService; +import org.alfresco.service.cmr.security.PersonService; +import org.alfresco.service.cmr.site.SiteInfo; +import org.springframework.extensions.webscripts.Cache; +import org.springframework.extensions.webscripts.Status; +import org.springframework.extensions.webscripts.WebScriptException; +import org.springframework.extensions.webscripts.WebScriptRequest; + +/** + * Lists the members of a site, optionally filtering by name, role + * or authority type. + * + * Based on the old memberships.get.js webscript controller + * + * @author Nick Burch + * @since 4.0 + */ +public class SiteMembershipsGet extends AbstractSiteWebScript +{ + private PersonService personService; + private ScriptAuthorityService scriptAuthorityService; + + public void setPersonService(PersonService personService) + { + this.personService = personService; + } + + public void setScriptAuthorityService(ScriptAuthorityService scriptAuthorityService) + { + this.scriptAuthorityService = scriptAuthorityService; + } + + @Override + protected Map executeImpl(SiteInfo site, + WebScriptRequest req, Status status, Cache cache) + { + // Grab out filters + String nameFilter = req.getParameter("nf"); + String roleFilter = req.getParameter("rf"); + String authorityType = req.getParameter("authorityType"); + String sizeS = req.getParameter("size"); + boolean collapseGroups = false; + + // Sanity check the types + if(authorityType != null) + { + if("USER".equals(authorityType)) + { + collapseGroups = true; + } + else if("GROUP".equals(authorityType)) + { + // No extra settings needed + } + else + { + throw new WebScriptException(Status.STATUS_BAD_REQUEST, + "The Authority must be one of USER or GROUP"); + } + } + + // Figure out what to limit to, if anythign + int limit = 0; + if(sizeS != null) + { + try + { + limit = Integer.parseInt(sizeS); + } + catch(NumberFormatException e) + { + throw new WebScriptException(Status.STATUS_BAD_REQUEST, + "Invalid size specified"); + } + } + + + // Fetch the membership details of the site + Map members = this.siteService.listMembers( + site.getShortName(), nameFilter, roleFilter, limit, collapseGroups); + + + // Process it ready for FreeMarker + // Note that as usernames may be all numbers, we need to + // prefix them with an underscore otherwise FTL can get confused! + Map authorities = new HashMap(members.size()); + Map roles = new LinkedHashMap(members.size()); + for(String authorityName : members.keySet()) + { + String role = members.get(authorityName); + String ftlSafeName = "_" + authorityName; + + if(authorityName.startsWith("GROUP_")) + { + if(authorityType == null || authorityType.equals("GROUP")) + { + // Record the details + authorities.put( + ftlSafeName, + scriptAuthorityService.getGroupForFullAuthorityName(authorityName) + ); + roles.put(ftlSafeName, role); + } + } + else + { + if(authorityType == null || authorityType.equals("USER")) + { + // Record the details + authorities.put( + ftlSafeName, + personService.getPerson(authorityName) + ); + roles.put(ftlSafeName, role); + } + } + } + + // Pass the details to freemarker + Map model = new HashMap(); + model.put("site", site); + model.put("roles", roles); + model.put("authorities", authorities); + return model; + } +} \ No newline at end of file diff --git a/source/java/org/alfresco/repo/web/scripts/site/SiteServiceTest.java b/source/java/org/alfresco/repo/web/scripts/site/SiteServiceTest.java index 2b7f29d328..3412879c03 100644 --- a/source/java/org/alfresco/repo/web/scripts/site/SiteServiceTest.java +++ b/source/java/org/alfresco/repo/web/scripts/site/SiteServiceTest.java @@ -70,6 +70,7 @@ public class SiteServiceTest extends BaseWebScriptTest private static final String USER_ONE = "SiteTestOne"; private static final String USER_TWO = "SiteTestTwo"; private static final String USER_THREE = "SiteTestThree"; + private static final String USER_NUMERIC = "1234567890"; private static final String URL_SITES = "/api/sites"; private static final String URL_SITES_QUERY = URL_SITES + "/query"; @@ -96,6 +97,7 @@ public class SiteServiceTest extends BaseWebScriptTest createUser(USER_ONE); createUser(USER_TWO); createUser(USER_THREE); + createUser(USER_NUMERIC); // Do tests as user one this.authenticationComponent.setCurrentUser(USER_ONE); @@ -117,6 +119,14 @@ public class SiteServiceTest extends BaseWebScriptTest this.personService.createPerson(ppOne); } } + private void deleteUser(String username) + { + this.personService.deletePerson(username); + if(this.authenticationService.authenticationExists(username)) + { + this.authenticationService.deleteAuthentication(username); + } + } @Override protected void tearDown() throws Exception @@ -132,6 +142,12 @@ public class SiteServiceTest extends BaseWebScriptTest // Clear the list this.createdSites.clear(); + + // Clear the users + deleteUser(USER_ONE); + deleteUser(USER_TWO); + deleteUser(USER_THREE); + deleteUser(USER_NUMERIC); } public void testCreateSite() throws Exception @@ -350,6 +366,42 @@ public class SiteServiceTest extends BaseWebScriptTest JSONArray result2 = new JSONArray(response.getContentAsString()); assertNotNull(result2); assertEquals(2, result2.length()); + + + // Add another user, with a fully numeric username + membership = new JSONObject(); + membership.put("role", SiteModel.SITE_CONTRIBUTOR); + person = new JSONObject(); + person.put("userName", USER_NUMERIC); + membership.put("person", person); + response = sendRequest(new PostRequest(URL_SITES + "/" + shortName + URL_MEMBERSHIPS, membership.toString(), "application/json"), 200); + + // Check the details are correct for one user + membership = new JSONObject(response.getContentAsString()); + assertEquals(SiteModel.SITE_CONTRIBUTOR, membership.get("role")); + assertEquals(USER_NUMERIC, membership.getJSONObject("authority").get("userName")); + + + // Check the membership list + response = sendRequest(new GetRequest(URL_SITES + "/" + shortName + URL_MEMBERSHIPS), 200); + String json = response.getContentAsString(); + JSONArray result3 = new JSONArray(json); + assertNotNull(result3); + assertEquals(3, result3.length()); + + // Check the everyone has the correct membership + // (The webscript returns the users in order to make testing easier) + membership = result3.getJSONObject(0); + assertEquals(SiteModel.SITE_MANAGER, membership.get("role")); + assertEquals(USER_ONE, membership.getJSONObject("authority").get("userName")); + + membership = result3.getJSONObject(1); + assertEquals(SiteModel.SITE_CONSUMER, membership.get("role")); + assertEquals(USER_TWO, membership.getJSONObject("authority").get("userName")); + + membership = result3.getJSONObject(2); + assertEquals(SiteModel.SITE_CONTRIBUTOR, membership.get("role")); + assertEquals(USER_NUMERIC, membership.getJSONObject("authority").get("userName")); } public void testGetMembership() throws Exception