From 56251b15bc1b3d7f94537e89cdaf1d1128f68256 Mon Sep 17 00:00:00 2001 From: Jean-Baptiste Vilain Date: Wed, 10 Jul 2013 11:26:23 +0200 Subject: [PATCH] SONAR-4465 Reworked the ResourcePermission implementation to rely on the permission templates to get the default permissions for a resource --- .../resource/DefaultResourcePermissions.java | 64 ++++++++++++++++++- .../org/sonar/core/user/PermissionDao.java | 11 ++++ .../core/user/PermissionTemplateDto.java | 4 ++ .../core/user/PermissionTemplateMapper.java | 2 + .../core/user/PermissionTemplateMapper.xml | 7 ++ .../DefaultResourcePermissionsTest.java | 57 ++++++++--------- .../sonar/core/user/PermissionDaoTest.java | 12 ++++ .../grantDefaultRoles-result.xml | 19 ++++-- .../grantDefaultRoles.xml | 13 ++++ .../grantDefaultRolesProject-result.xml | 42 ++++++++++++ .../grantDefaultRolesProject.xml | 33 ++++++++++ ...grantDefaultRoles_unknown_group-result.xml | 8 ++- .../grantDefaultRoles_unknown_group.xml | 7 ++ .../grantDefaultRoles_users-result.xml | 11 +++- .../grantDefaultRoles_users.xml | 7 ++ .../selectPermissionTemplate.xml | 3 + 16 files changed, 257 insertions(+), 43 deletions(-) create mode 100644 sonar-core/src/test/resources/org/sonar/core/resource/DefaultResourcePermissionsTest/grantDefaultRolesProject-result.xml create mode 100644 sonar-core/src/test/resources/org/sonar/core/resource/DefaultResourcePermissionsTest/grantDefaultRolesProject.xml diff --git a/sonar-core/src/main/java/org/sonar/core/resource/DefaultResourcePermissions.java b/sonar-core/src/main/java/org/sonar/core/resource/DefaultResourcePermissions.java index 7a7e17b14e3..35506cd04ef 100644 --- a/sonar-core/src/main/java/org/sonar/core/resource/DefaultResourcePermissions.java +++ b/sonar-core/src/main/java/org/sonar/core/resource/DefaultResourcePermissions.java @@ -19,6 +19,7 @@ */ package org.sonar.core.resource; +import org.apache.commons.lang.StringUtils; import org.apache.ibatis.session.SqlSession; import org.sonar.api.ServerExtension; import org.sonar.api.config.Settings; @@ -30,6 +31,9 @@ import org.sonar.api.web.UserRole; import org.sonar.core.persistence.MyBatis; import org.sonar.core.user.*; +import java.util.ArrayList; +import java.util.List; + /** * @since 3.2 */ @@ -39,12 +43,14 @@ public class DefaultResourcePermissions implements ResourcePermissions, TaskExte private final MyBatis myBatis; private final RoleDao roleDao; private final UserDao userDao; + private final PermissionDao permissionDao; - public DefaultResourcePermissions(Settings settings, MyBatis myBatis, RoleDao roleDao, UserDao userDao) { + public DefaultResourcePermissions(Settings settings, MyBatis myBatis, RoleDao roleDao, UserDao userDao, PermissionDao permissionDao) { this.settings = settings; this.myBatis = myBatis; this.roleDao = roleDao; this.userDao = userDao; + this.permissionDao = permissionDao; } public boolean hasRoles(Resource resource) { @@ -123,7 +129,9 @@ public class DefaultResourcePermissions implements ResourcePermissions, TaskExte } private void grantDefaultRoles(Resource resource, String role, SqlSession session) { - String[] groupNames = settings.getStringArrayBySeparator("sonar.role." + role + "." + resource.getQualifier() + ".defaultGroups", ","); + PermissionTemplateDto applicablePermissionTemplate = getPermissionTemplate(resource.getQualifier()); + + List groupNames = getEligibleGroups(role, applicablePermissionTemplate); for (String groupName : groupNames) { GroupRoleDto groupRole = new GroupRoleDto().setRole(role).setResourceId(Long.valueOf(resource.getId())); if (DefaultGroups.isAnyone(groupName)) { @@ -136,7 +144,7 @@ public class DefaultResourcePermissions implements ResourcePermissions, TaskExte } } - String[] logins = settings.getStringArrayBySeparator("sonar.role." + role + "." + resource.getQualifier() + ".defaultUsers", ","); + List logins = getEligibleUsers(role, applicablePermissionTemplate); for (String login : logins) { UserDto user = userDao.selectActiveUserByLogin(login, session); if (user != null) { @@ -145,4 +153,54 @@ public class DefaultResourcePermissions implements ResourcePermissions, TaskExte } } } + + private List getEligibleGroups(String role, PermissionTemplateDto permissionTemplate) { + List eligibleGroups = new ArrayList(); + if(permissionTemplate.getGroupsPermissions() != null) { + for (PermissionTemplateGroupDto groupPermission : permissionTemplate.getGroupsPermissions()) { + if(role.equals(groupPermission.getPermission())) { + String groupName = groupPermission.getGroupName() != null ? groupPermission.getGroupName() : DefaultGroups.ANYONE; + eligibleGroups.add(groupName); + } + } + } + return eligibleGroups; + } + + private List getEligibleUsers(String role, PermissionTemplateDto permissionTemplate) { + List eligibleUsers = new ArrayList(); + if(permissionTemplate.getUsersPermissions() != null) { + for (PermissionTemplateUserDto userPermission : permissionTemplate.getUsersPermissions()) { + if(role.equals(userPermission.getPermission())) { + eligibleUsers.add(userPermission.getUserLogin()); + } + } + } + return eligibleUsers; + } + + private PermissionTemplateDto getPermissionTemplate(String qualifier) { + String qualifierTemplateId = settings.getString("sonar.permission.template." + qualifier + ".default"); + if(!StringUtils.isBlank(qualifierTemplateId)) { + return getTemplateWithPermissions(qualifierTemplateId); + } + + String defaultTemplateId = settings.getString("sonar.permission.template.default"); + if(StringUtils.isBlank(defaultTemplateId)) { + throw new RuntimeException("At least one default permission template should be defined"); + } + return getTemplateWithPermissions(defaultTemplateId); + } + + private PermissionTemplateDto getTemplateWithPermissions(String templateId) { + PermissionTemplateDto permissionTemplateDto = permissionDao.selectTemplateById(Long.parseLong(templateId)); + if(permissionTemplateDto == null) { + throw new RuntimeException("Could not retrieve permission template with id " + templateId); + } + PermissionTemplateDto templateWithPermissions = permissionDao.selectPermissionTemplate(permissionTemplateDto.getName()); + if(templateWithPermissions == null) { + throw new RuntimeException("Could not retrieve permissions for template with id " + templateId); + } + return templateWithPermissions; + } } diff --git a/sonar-core/src/main/java/org/sonar/core/user/PermissionDao.java b/sonar-core/src/main/java/org/sonar/core/user/PermissionDao.java index 2d1e499c6fc..0043f4339d5 100644 --- a/sonar-core/src/main/java/org/sonar/core/user/PermissionDao.java +++ b/sonar-core/src/main/java/org/sonar/core/user/PermissionDao.java @@ -49,6 +49,17 @@ public class PermissionDao implements TaskExtension, ServerExtension { } } + @CheckForNull + public PermissionTemplateDto selectTemplateById(Long templateId) { + SqlSession session = myBatis.openSession(); + try { + PermissionTemplateMapper mapper = session.getMapper(PermissionTemplateMapper.class); + return mapper.selectById(templateId); + } finally { + MyBatis.closeQuietly(session); + } + } + @CheckForNull public PermissionTemplateDto selectPermissionTemplate(String templateName) { PermissionTemplateDto permissionTemplate = null; diff --git a/sonar-core/src/main/java/org/sonar/core/user/PermissionTemplateDto.java b/sonar-core/src/main/java/org/sonar/core/user/PermissionTemplateDto.java index fb235c99e8b..3a2bc1b9095 100644 --- a/sonar-core/src/main/java/org/sonar/core/user/PermissionTemplateDto.java +++ b/sonar-core/src/main/java/org/sonar/core/user/PermissionTemplateDto.java @@ -20,6 +20,7 @@ package org.sonar.core.user; +import javax.annotation.CheckForNull; import javax.annotation.Nullable; import java.util.Date; import java.util.List; @@ -52,6 +53,7 @@ public class PermissionTemplateDto { return this; } + @CheckForNull public String getDescription() { return description; } @@ -61,6 +63,7 @@ public class PermissionTemplateDto { return this; } + @CheckForNull public List getUsersPermissions() { return usersPermissions; } @@ -70,6 +73,7 @@ public class PermissionTemplateDto { return this; } + @CheckForNull public List getGroupsPermissions() { return groupsPermissions; } diff --git a/sonar-core/src/main/java/org/sonar/core/user/PermissionTemplateMapper.java b/sonar-core/src/main/java/org/sonar/core/user/PermissionTemplateMapper.java index 124492f07a5..8f00c507968 100644 --- a/sonar-core/src/main/java/org/sonar/core/user/PermissionTemplateMapper.java +++ b/sonar-core/src/main/java/org/sonar/core/user/PermissionTemplateMapper.java @@ -37,6 +37,8 @@ public interface PermissionTemplateMapper { PermissionTemplateDto selectByName(String templateName); + PermissionTemplateDto selectById(Long templateId); + PermissionTemplateDto selectTemplateUsersPermissions(String templateName); PermissionTemplateDto selectTemplateGroupsPermissions(String templateName); diff --git a/sonar-core/src/main/resources/org/sonar/core/user/PermissionTemplateMapper.xml b/sonar-core/src/main/resources/org/sonar/core/user/PermissionTemplateMapper.xml index 9e40370798a..de84f3485bd 100644 --- a/sonar-core/src/main/resources/org/sonar/core/user/PermissionTemplateMapper.xml +++ b/sonar-core/src/main/resources/org/sonar/core/user/PermissionTemplateMapper.xml @@ -59,6 +59,12 @@ + + diff --git a/sonar-core/src/test/java/org/sonar/core/resource/DefaultResourcePermissionsTest.java b/sonar-core/src/test/java/org/sonar/core/resource/DefaultResourcePermissionsTest.java index df51c2356b9..ab69bc5eaa5 100644 --- a/sonar-core/src/test/java/org/sonar/core/resource/DefaultResourcePermissionsTest.java +++ b/sonar-core/src/test/java/org/sonar/core/resource/DefaultResourcePermissionsTest.java @@ -19,12 +19,14 @@ */ package org.sonar.core.resource; +import org.junit.Before; import org.junit.Test; import org.sonar.api.config.Settings; import org.sonar.api.resources.Project; import org.sonar.api.resources.Resource; import org.sonar.api.security.DefaultGroups; import org.sonar.core.persistence.AbstractDaoTestCase; +import org.sonar.core.user.PermissionDao; import org.sonar.core.user.RoleDao; import org.sonar.core.user.UserDao; @@ -33,13 +35,20 @@ import static org.fest.assertions.Assertions.assertThat; public class DefaultResourcePermissionsTest extends AbstractDaoTestCase { private Resource project = new Project("project").setId(123); + private Settings settings; + private DefaultResourcePermissions permissions; + + @Before + public void initResourcePermissions() { + settings = new Settings(); + permissions = new DefaultResourcePermissions(settings, getMyBatis(), + new RoleDao(getMyBatis()), new UserDao(getMyBatis()), new PermissionDao(getMyBatis())); + } @Test public void grantGroupRole() { setupData("grantGroupRole"); - DefaultResourcePermissions permissions = new DefaultResourcePermissions(new Settings(), getMyBatis(), - new RoleDao(getMyBatis()), new UserDao(getMyBatis())); permissions.grantGroupRole(project, "sonar-administrators", "admin"); // do not insert duplicated rows @@ -52,8 +61,6 @@ public class DefaultResourcePermissionsTest extends AbstractDaoTestCase { public void grantGroupRole_anyone() { setupData("grantGroupRole_anyone"); - DefaultResourcePermissions permissions = new DefaultResourcePermissions(new Settings(), getMyBatis(), - new RoleDao(getMyBatis()), new UserDao(getMyBatis())); permissions.grantGroupRole(project, DefaultGroups.ANYONE, "admin"); checkTables("grantGroupRole_anyone", "group_roles"); @@ -63,8 +70,6 @@ public class DefaultResourcePermissionsTest extends AbstractDaoTestCase { public void grantGroupRole_ignore_if_group_not_found() { setupData("grantGroupRole_ignore_if_group_not_found"); - DefaultResourcePermissions permissions = new DefaultResourcePermissions(new Settings(), getMyBatis(), - new RoleDao(getMyBatis()), new UserDao(getMyBatis())); permissions.grantGroupRole(project, "not_found", "admin"); checkTables("grantGroupRole_ignore_if_group_not_found", "group_roles"); @@ -74,8 +79,6 @@ public class DefaultResourcePermissionsTest extends AbstractDaoTestCase { public void grantGroupRole_ignore_if_not_persisted() { setupData("grantGroupRole_ignore_if_not_persisted"); - DefaultResourcePermissions permissions = new DefaultResourcePermissions(new Settings(), getMyBatis(), - new RoleDao(getMyBatis()), new UserDao(getMyBatis())); Project resourceWithoutId = new Project(""); permissions.grantGroupRole(resourceWithoutId, "sonar-users", "admin"); @@ -86,8 +89,6 @@ public class DefaultResourcePermissionsTest extends AbstractDaoTestCase { public void grantUserRole() { setupData("grantUserRole"); - DefaultResourcePermissions permissions = new DefaultResourcePermissions(new Settings(), getMyBatis(), - new RoleDao(getMyBatis()), new UserDao(getMyBatis())); permissions.grantUserRole(project, "marius", "admin"); // do not insert duplicated rows @@ -97,32 +98,33 @@ public class DefaultResourcePermissionsTest extends AbstractDaoTestCase { } @Test - public void grantDefaultRoles() { + public void grantDefaultRoles_qualifier_independent() { setupData("grantDefaultRoles"); - Settings settings = new Settings(); - settings.setProperty("sonar.role.admin.TRK.defaultGroups", "sonar-administrators"); - settings.setProperty("sonar.role.admin.TRK.defaultUsers", ""); - settings.setProperty("sonar.role.user.TRK.defaultGroups", "Anyone,sonar-users"); - settings.setProperty("sonar.role.user.TRK.defaultUsers", ""); - settings.setProperty("sonar.role.codeviewer.TRK.defaultGroups", "Anyone,sonar-users"); - settings.setProperty("sonar.role.codeviewer.TRK.defaultUsers", ""); - DefaultResourcePermissions permissions = new DefaultResourcePermissions(settings, getMyBatis(), - new RoleDao(getMyBatis()), new UserDao(getMyBatis())); + settings.setProperty("sonar.permission.template.default", "1"); permissions.grantDefaultRoles(project); checkTables("grantDefaultRoles", "user_roles", "group_roles"); } + @Test + public void grantDefaultRoles_qualifier_specific() { + setupData("grantDefaultRolesProject"); + + settings.setProperty("sonar.permission.template.default", "1"); + settings.setProperty("sonar.permission.template.TRK.default", "2"); + + permissions.grantDefaultRoles(project); + + checkTables("grantDefaultRolesProject", "user_roles", "group_roles"); + } + @Test public void grantDefaultRoles_unknown_group() { setupData("grantDefaultRoles_unknown_group"); - Settings settings = new Settings(); - settings.setProperty("sonar.role.admin.TRK.defaultGroups", "sonar-administrators,unknown"); - DefaultResourcePermissions permissions = new DefaultResourcePermissions(settings, getMyBatis(), - new RoleDao(getMyBatis()), new UserDao(getMyBatis())); + settings.setProperty("sonar.permission.template.TRK.default", "1"); permissions.grantDefaultRoles(project); checkTables("grantDefaultRoles_unknown_group", "group_roles"); @@ -132,10 +134,7 @@ public class DefaultResourcePermissionsTest extends AbstractDaoTestCase { public void grantDefaultRoles_users() { setupData("grantDefaultRoles_users"); - Settings settings = new Settings(); - settings.setProperty("sonar.role.admin.TRK.defaultUsers", "marius,disabled,notfound"); - DefaultResourcePermissions permissions = new DefaultResourcePermissions(settings, getMyBatis(), - new RoleDao(getMyBatis()), new UserDao(getMyBatis())); + settings.setProperty("sonar.permission.template.TRK.default", "1"); permissions.grantDefaultRoles(project); checkTables("grantDefaultRoles_users", "user_roles"); @@ -144,8 +143,6 @@ public class DefaultResourcePermissionsTest extends AbstractDaoTestCase { @Test public void hasRoles() { setupData("hasRoles"); - DefaultResourcePermissions permissions = new DefaultResourcePermissions(new Settings(), getMyBatis(), - new RoleDao(getMyBatis()), new UserDao(getMyBatis())); // no groups and at least one user assertThat(permissions.hasRoles(new Project("only_users").setId(1))).isTrue(); diff --git a/sonar-core/src/test/java/org/sonar/core/user/PermissionDaoTest.java b/sonar-core/src/test/java/org/sonar/core/user/PermissionDaoTest.java index 023fe4d405e..6f99afabffc 100644 --- a/sonar-core/src/test/java/org/sonar/core/user/PermissionDaoTest.java +++ b/sonar-core/src/test/java/org/sonar/core/user/PermissionDaoTest.java @@ -89,6 +89,18 @@ public class PermissionDaoTest extends AbstractDaoTestCase { assertThat(permissionTemplate.getDescription()).isEqualTo("my description"); } + @Test + public void should_select_permission_template_by_id() throws Exception { + setupData("selectPermissionTemplate"); + + PermissionTemplateDto permissionTemplate = permissionDao.selectTemplateById(1L); + + assertThat(permissionTemplate).isNotNull(); + assertThat(permissionTemplate.getId()).isEqualTo(1L); + assertThat(permissionTemplate.getName()).isEqualTo("my template"); + assertThat(permissionTemplate.getDescription()).isEqualTo("my description"); + } + @Test public void should_select_all_permission_templates() throws Exception { setupData("selectAllPermissionTemplates"); diff --git a/sonar-core/src/test/resources/org/sonar/core/resource/DefaultResourcePermissionsTest/grantDefaultRoles-result.xml b/sonar-core/src/test/resources/org/sonar/core/resource/DefaultResourcePermissionsTest/grantDefaultRoles-result.xml index f4154d15024..88a68afdb6b 100644 --- a/sonar-core/src/test/resources/org/sonar/core/resource/DefaultResourcePermissionsTest/grantDefaultRoles-result.xml +++ b/sonar-core/src/test/resources/org/sonar/core/resource/DefaultResourcePermissionsTest/grantDefaultRoles-result.xml @@ -12,9 +12,20 @@ new rows : sonar-administrators (admin), sonar-users (user & codeviewer), Anyone (user & codeviewer), --> - - - - + + + + + + + + + + + + + + + \ No newline at end of file diff --git a/sonar-core/src/test/resources/org/sonar/core/resource/DefaultResourcePermissionsTest/grantDefaultRoles.xml b/sonar-core/src/test/resources/org/sonar/core/resource/DefaultResourcePermissionsTest/grantDefaultRoles.xml index 21a4dda2ccf..fea38a879d8 100644 --- a/sonar-core/src/test/resources/org/sonar/core/resource/DefaultResourcePermissionsTest/grantDefaultRoles.xml +++ b/sonar-core/src/test/resources/org/sonar/core/resource/DefaultResourcePermissionsTest/grantDefaultRoles.xml @@ -7,4 +7,17 @@ + + + + + + + + + + + + + \ No newline at end of file diff --git a/sonar-core/src/test/resources/org/sonar/core/resource/DefaultResourcePermissionsTest/grantDefaultRolesProject-result.xml b/sonar-core/src/test/resources/org/sonar/core/resource/DefaultResourcePermissionsTest/grantDefaultRolesProject-result.xml new file mode 100644 index 00000000000..c4afa99de89 --- /dev/null +++ b/sonar-core/src/test/resources/org/sonar/core/resource/DefaultResourcePermissionsTest/grantDefaultRolesProject-result.xml @@ -0,0 +1,42 @@ + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + \ No newline at end of file diff --git a/sonar-core/src/test/resources/org/sonar/core/resource/DefaultResourcePermissionsTest/grantDefaultRolesProject.xml b/sonar-core/src/test/resources/org/sonar/core/resource/DefaultResourcePermissionsTest/grantDefaultRolesProject.xml new file mode 100644 index 00000000000..3ef5580c464 --- /dev/null +++ b/sonar-core/src/test/resources/org/sonar/core/resource/DefaultResourcePermissionsTest/grantDefaultRolesProject.xml @@ -0,0 +1,33 @@ + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + \ No newline at end of file diff --git a/sonar-core/src/test/resources/org/sonar/core/resource/DefaultResourcePermissionsTest/grantDefaultRoles_unknown_group-result.xml b/sonar-core/src/test/resources/org/sonar/core/resource/DefaultResourcePermissionsTest/grantDefaultRoles_unknown_group-result.xml index ef56a12934c..8158b940535 100644 --- a/sonar-core/src/test/resources/org/sonar/core/resource/DefaultResourcePermissionsTest/grantDefaultRoles_unknown_group-result.xml +++ b/sonar-core/src/test/resources/org/sonar/core/resource/DefaultResourcePermissionsTest/grantDefaultRoles_unknown_group-result.xml @@ -8,9 +8,11 @@ - + + + + + \ No newline at end of file diff --git a/sonar-core/src/test/resources/org/sonar/core/resource/DefaultResourcePermissionsTest/grantDefaultRoles_unknown_group.xml b/sonar-core/src/test/resources/org/sonar/core/resource/DefaultResourcePermissionsTest/grantDefaultRoles_unknown_group.xml index 21a4dda2ccf..0c4dfd1ab47 100644 --- a/sonar-core/src/test/resources/org/sonar/core/resource/DefaultResourcePermissionsTest/grantDefaultRoles_unknown_group.xml +++ b/sonar-core/src/test/resources/org/sonar/core/resource/DefaultResourcePermissionsTest/grantDefaultRoles_unknown_group.xml @@ -7,4 +7,11 @@ + + + + + + + \ No newline at end of file diff --git a/sonar-core/src/test/resources/org/sonar/core/resource/DefaultResourcePermissionsTest/grantDefaultRoles_users-result.xml b/sonar-core/src/test/resources/org/sonar/core/resource/DefaultResourcePermissionsTest/grantDefaultRoles_users-result.xml index caff65f9e21..75c11a9834c 100644 --- a/sonar-core/src/test/resources/org/sonar/core/resource/DefaultResourcePermissionsTest/grantDefaultRoles_users-result.xml +++ b/sonar-core/src/test/resources/org/sonar/core/resource/DefaultResourcePermissionsTest/grantDefaultRoles_users-result.xml @@ -9,8 +9,13 @@ - + + + + + + + + \ No newline at end of file diff --git a/sonar-core/src/test/resources/org/sonar/core/resource/DefaultResourcePermissionsTest/grantDefaultRoles_users.xml b/sonar-core/src/test/resources/org/sonar/core/resource/DefaultResourcePermissionsTest/grantDefaultRoles_users.xml index 7c5f6c5d347..3f492a44512 100644 --- a/sonar-core/src/test/resources/org/sonar/core/resource/DefaultResourcePermissionsTest/grantDefaultRoles_users.xml +++ b/sonar-core/src/test/resources/org/sonar/core/resource/DefaultResourcePermissionsTest/grantDefaultRoles_users.xml @@ -8,4 +8,11 @@ + + + + + + + \ No newline at end of file diff --git a/sonar-core/src/test/resources/org/sonar/core/user/PermissionDaoTest/selectPermissionTemplate.xml b/sonar-core/src/test/resources/org/sonar/core/user/PermissionDaoTest/selectPermissionTemplate.xml index dec6ec2ed4f..f869265575c 100644 --- a/sonar-core/src/test/resources/org/sonar/core/user/PermissionDaoTest/selectPermissionTemplate.xml +++ b/sonar-core/src/test/resources/org/sonar/core/user/PermissionDaoTest/selectPermissionTemplate.xml @@ -10,7 +10,10 @@ + + + -- 2.39.5