SONAR-2474 When sharing a filter, the owner (and not the current user or previous owner) should have the sharing permission

5 files changed, 55 insertions, 32 deletions

diff --git a/sonar-server/src/main/java/org/sonar/server/issue/InternalRubyIssueService.java b/sonar-server/src/main/java/org/sonar/server/issue/InternalRubyIssueService.java
index b96f8658b4c..6b092d4a65f 100644
--- a/sonar-server/src/main/java/org/sonar/server/issue/InternalRubyIssueService.java
+++ b/sonar-server/src/main/java/org/sonar/server/issue/InternalRubyIssueService.java
@@ -412,7 +412,7 @@ public class InternalRubyIssueService implements ServerComponent {
   }
 
   public boolean canUserShareIssueFilter(){
-    return issueFilterService.canShareFilter(UserSession.get());
+    return issueFilterService.canShareFilter(UserSession.get().login());
   }
 
   public String serializeFilterQuery(Map<String, Object> filterQuery) {
diff --git a/sonar-server/src/main/java/org/sonar/server/issue/IssueFilterService.java b/sonar-server/src/main/java/org/sonar/server/issue/IssueFilterService.java
index a9ae87ab0d3..cff4e165ce9 100644
--- a/sonar-server/src/main/java/org/sonar/server/issue/IssueFilterService.java
+++ b/sonar-server/src/main/java/org/sonar/server/issue/IssueFilterService.java
@@ -93,19 +93,19 @@ public class IssueFilterService implements ServerComponent {
   public DefaultIssueFilter save(DefaultIssueFilter issueFilter, UserSession userSession) {
     String user = getLoggedLogin(userSession);
     issueFilter.setUser(user);
-    validateFilter(issueFilter, userSession);
+    validateFilter(issueFilter);
     return insertIssueFilter(issueFilter, user);
   }
 
   public DefaultIssueFilter update(DefaultIssueFilter issueFilter, UserSession userSession) {
     String login = getLoggedLogin(userSession);
     IssueFilterDto existingFilterDto = findIssueFilterDto(issueFilter.id(), login);
-    verifyCurrentUserCanModifyFilter(existingFilterDto.toIssueFilter(), userSession);
+    verifyCurrentUserCanModifyFilter(existingFilterDto.toIssueFilter(), login);
     verifyCurrentUserCanChangeFilterSharingFilter(issueFilter, existingFilterDto, login);
     if (!existingFilterDto.getUserLogin().equals(issueFilter.user())) {
-      verifyCurrentUserCanChangeFilterOwnership(userSession);
+      verifyCurrentUserCanChangeFilterOwnership(login);
     }
-    validateFilter(issueFilter, userSession);
+    validateFilter(issueFilter);
     deleteOtherFavoriteFiltersIfFilterBecomeUnshared(existingFilterDto, issueFilter);
     filterDao.update(IssueFilterDto.toIssueFilter(issueFilter));
     return issueFilter;
@@ -124,7 +124,7 @@ public class IssueFilterService implements ServerComponent {
   public DefaultIssueFilter updateFilterQuery(Long issueFilterId, Map<String, Object> filterQuery, UserSession userSession) {
     String login = getLoggedLogin(userSession);
     IssueFilterDto issueFilterDto = findIssueFilterDto(issueFilterId, login);
-    verifyCurrentUserCanModifyFilter(issueFilterDto.toIssueFilter(), userSession);
+    verifyCurrentUserCanModifyFilter(issueFilterDto.toIssueFilter(), login);
 
     DefaultIssueFilter issueFilter = issueFilterDto.toIssueFilter();
     issueFilter.setData(serializeFilterQuery(filterQuery));
@@ -135,7 +135,7 @@ public class IssueFilterService implements ServerComponent {
   public void delete(Long issueFilterId, UserSession userSession) {
     String login = getLoggedLogin(userSession);
     IssueFilterDto issueFilterDto = findIssueFilterDto(issueFilterId, login);
-    verifyCurrentUserCanModifyFilter(issueFilterDto.toIssueFilter(), userSession);
+    verifyCurrentUserCanModifyFilter(issueFilterDto.toIssueFilter(), login);
 
     deleteFavouriteIssueFilters(issueFilterDto);
     filterDao.delete(issueFilterId);
@@ -148,7 +148,7 @@ public class IssueFilterService implements ServerComponent {
     issueFilterDtoToCopy.setShared(false);
     issueFilter.setUser(login);
     issueFilter.setData(issueFilterDtoToCopy.getData());
-    validateFilter(issueFilter, userSession);
+    validateFilter(issueFilter);
     return insertIssueFilter(issueFilter, login);
   }
 
@@ -200,8 +200,8 @@ public class IssueFilterService implements ServerComponent {
     return issueFilterDto;
   }
 
-  public boolean canShareFilter(UserSession userSession){
-    return userSession.hasGlobalPermission(Permission.DASHBOARD_SHARING);
+  public boolean canShareFilter(String user){
+    return authorizationDao.selectGlobalPermissions(user).contains(Permission.DASHBOARD_SHARING.key());
   }
 
   String getLoggedLogin(UserSession userSession) {
@@ -218,8 +218,8 @@ public class IssueFilterService implements ServerComponent {
     }
   }
 
-  private void verifyCurrentUserCanModifyFilter(DefaultIssueFilter issueFilter, UserSession userSession) {
-    if (!issueFilter.user().equals(userSession.login()) && !isAdmin(userSession)) {
+  private void verifyCurrentUserCanModifyFilter(DefaultIssueFilter issueFilter, String user) {
+    if (!issueFilter.user().equals(user) && !isAdmin(user)) {
       throw new ForbiddenException("User is not authorized to modify this filter");
     }
   }
@@ -230,19 +230,19 @@ public class IssueFilterService implements ServerComponent {
     }
   }
 
-  private void verifyCurrentUserCanChangeFilterOwnership(UserSession userSession) {
-    if (!isAdmin(userSession)) {
+  private void verifyCurrentUserCanChangeFilterOwnership(String user) {
+    if (!isAdmin(user)) {
       throw new ForbiddenException("User is not authorized to change the owner of this filter");
     }
   }
 
-  private void verifyCurrentUserCanShareFilter(DefaultIssueFilter issueFilter, UserSession userSession) {
-    if (issueFilter.shared() && !canShareFilter(userSession)) {
-      throw new ForbiddenException("User is not authorized to share this filter");
+  private void verifyCurrentUserCanShareFilter(DefaultIssueFilter issueFilter, String user) {
+    if (issueFilter.shared() && !canShareFilter(user)) {
+      throw new ForbiddenException("User cannot own this filter because of insufficient rights");
     }
   }
 
-  private void validateFilter(final DefaultIssueFilter issueFilter, UserSession userSession) {
+  private void validateFilter(final DefaultIssueFilter issueFilter) {
     List<IssueFilterDto> userFilters = selectUserIssueFilters(issueFilter.user());
     IssueFilterDto userFilterSameName = findFilterWithSameName(userFilters, issueFilter.name());
     if (userFilterSameName != null && !userFilterSameName.getId().equals(issueFilter.id())) {
@@ -254,7 +254,7 @@ public class IssueFilterService implements ServerComponent {
       if (sharedFilterWithSameName != null && !sharedFilterWithSameName.getId().equals(issueFilter.id())) {
         throw new BadRequestException("Other users already share filters with the same name");
       }
-      verifyCurrentUserCanShareFilter(issueFilter, userSession);
+      verifyCurrentUserCanShareFilter(issueFilter, issueFilter.user());
     }
   }
 
@@ -321,8 +321,8 @@ public class IssueFilterService implements ServerComponent {
     }));
   }
 
-  private boolean isAdmin(UserSession userSession) {
-    return userSession.hasGlobalPermission(Permission.SYSTEM_ADMIN);
+  private boolean isAdmin(String user) {
+    return authorizationDao.selectGlobalPermissions(user).contains(Permission.SYSTEM_ADMIN.key());
   }
 
   private IssueFilterResult createIssueFilterResult(IssueQueryResult issueQueryResult, IssueQuery issueQuery) {
diff --git a/sonar-server/src/main/webapp/WEB-INF/app/models/measure_filter.rb b/sonar-server/src/main/webapp/WEB-INF/app/models/measure_filter.rb
index e0e8e519dad..379c90aa4bc 100644
--- a/sonar-server/src/main/webapp/WEB-INF/app/models/measure_filter.rb
+++ b/sonar-server/src/main/webapp/WEB-INF/app/models/measure_filter.rb
@@ -321,8 +321,14 @@ class MeasureFilter < ActiveRecord::Base
         count = MeasureFilter.count('id', :conditions => ['name=? and shared=? and (user_id is null or user_id<>?)', name, true, user_id])
       end
       errors.add_to_base('Other users already share filters with the same name') if count>0
+
+      # Verify filter owner has sharing permission
+      if user && !user.has_role?(:shareDashboard)
+        errors.add(:user, "cannot own this filter because of insufficient rights")
+      end
     elsif system?
       errors.add_to_base("System filters can't be unshared")
     end
   end
+
 end
\ No newline at end of file
diff --git a/sonar-server/src/test/java/org/sonar/server/issue/InternalRubyIssueServiceTest.java b/sonar-server/src/test/java/org/sonar/server/issue/InternalRubyIssueServiceTest.java
index 82849fe0371..f2a7c891e50 100644
--- a/sonar-server/src/test/java/org/sonar/server/issue/InternalRubyIssueServiceTest.java
+++ b/sonar-server/src/test/java/org/sonar/server/issue/InternalRubyIssueServiceTest.java
@@ -541,7 +541,7 @@ public class InternalRubyIssueServiceTest {
   @Test
   public void should_check_if_user_can_share_issue_filter(){
     service.canUserShareIssueFilter();
-    verify(issueFilterService).canShareFilter(any(UserSession.class));
+    verify(issueFilterService).canShareFilter(anyString());
   }
 
   @Test
diff --git a/sonar-server/src/test/java/org/sonar/server/issue/IssueFilterServiceTest.java b/sonar-server/src/test/java/org/sonar/server/issue/IssueFilterServiceTest.java
index 16f5948ae1b..a2cc7bd783c 100644
--- a/sonar-server/src/test/java/org/sonar/server/issue/IssueFilterServiceTest.java
+++ b/sonar-server/src/test/java/org/sonar/server/issue/IssueFilterServiceTest.java
@@ -242,7 +242,7 @@ public class IssueFilterServiceTest {
 
   @Test
   public void should_have_permission_to_share_filter() {
-    UserSession userSession = MockUserSession.create().setLogin("john").setPermissions(Permission.DASHBOARD_SHARING);
+    when(authorizationDao.selectGlobalPermissions("john")).thenReturn(newArrayList(Permission.DASHBOARD_SHARING.key()));
     when(issueFilterDao.selectById(1L)).thenReturn(new IssueFilterDto().setId(1L).setName("My Filter").setShared(false).setUserLogin("john"));
 
     DefaultIssueFilter result = service.update(new DefaultIssueFilter().setId(1L).setName("My Filter").setShared(true).setUser("john"), userSession);
@@ -253,14 +253,14 @@ public class IssueFilterServiceTest {
 
   @Test
   public void should_not_share_filter_if_no_permission() {
-    UserSession userSession = MockUserSession.create().setLogin("john").setPermissions();
+    when(authorizationDao.selectGlobalPermissions("john")).thenReturn(Collections.<String>emptyList());
     when(issueFilterDao.selectById(1L)).thenReturn(new IssueFilterDto().setId(1L).setName("My Filter").setShared(false).setUserLogin("john"));
 
     try {
       service.update(new DefaultIssueFilter().setId(1L).setName("My Filter").setShared(true).setUser("john"), userSession);
       fail();
     } catch (Exception e) {
-      assertThat(e).isInstanceOf(ForbiddenException.class).hasMessage("User is not authorized to share this filter");
+      assertThat(e).isInstanceOf(ForbiddenException.class).hasMessage("User cannot own this filter because of insufficient rights");
     }
     verify(issueFilterDao, never()).update(any(IssueFilterDto.class));
   }
@@ -268,7 +268,7 @@ public class IssueFilterServiceTest {
   @Test
   public void should_not_update_sharing_if_not_owner() {
     // John is admin and want to change arthur filter sharing
-    UserSession userSession = MockUserSession.create().setLogin("john").setPermissions(Permission.SYSTEM_ADMIN);
+    when(authorizationDao.selectGlobalPermissions("john")).thenReturn(newArrayList(Permission.SYSTEM_ADMIN.key()));
     when(issueFilterDao.selectById(1L)).thenReturn(new IssueFilterDto().setId(1L).setName("Arthur Filter").setShared(true).setUserLogin("arthur"));
 
     try {
@@ -308,11 +308,12 @@ public class IssueFilterServiceTest {
   }
 
   @Test
-  public void should_update_other_shared_filter_if_admin() {
-    UserSession userSession = MockUserSession.create().setLogin("john").setPermissions(Permission.SYSTEM_ADMIN, Permission.DASHBOARD_SHARING);
+  public void should_update_other_shared_filter_if_admin_and_if_filter_owner_has_sharing_permission() {
+    when(authorizationDao.selectGlobalPermissions("john")).thenReturn(newArrayList(Permission.SYSTEM_ADMIN.key()));
+    when(authorizationDao.selectGlobalPermissions("arthur")).thenReturn(newArrayList(Permission.DASHBOARD_SHARING.key()));
     when(issueFilterDao.selectById(1L)).thenReturn(new IssueFilterDto().setId(1L).setName("My Old Filter").setDescription("Old description").setUserLogin("arthur").setShared(true));
 
-    DefaultIssueFilter result = service.update(new DefaultIssueFilter().setId(1L).setName("My New Filter").setDescription("New description").setShared(true), userSession);
+    DefaultIssueFilter result = service.update(new DefaultIssueFilter().setId(1L).setName("My New Filter").setDescription("New description").setShared(true).setUser("arthur"), userSession);
     assertThat(result.name()).isEqualTo("My New Filter");
     assertThat(result.description()).isEqualTo("New description");
 
@@ -320,6 +321,21 @@ public class IssueFilterServiceTest {
   }
 
   @Test
+  public void should_not_update_other_shared_filter_if_admin_and_if_filter_owner_has_no_sharing_permission() {
+    when(authorizationDao.selectGlobalPermissions("john")).thenReturn(newArrayList(Permission.SYSTEM_ADMIN.key()));
+    when(authorizationDao.selectGlobalPermissions("arthur")).thenReturn(Collections.<String>emptyList());
+    when(issueFilterDao.selectById(1L)).thenReturn(new IssueFilterDto().setId(1L).setName("My Old Filter").setDescription("Old description").setUserLogin("arthur").setShared(true));
+
+    try {
+      service.update(new DefaultIssueFilter().setId(1L).setName("My New Filter").setDescription("New description").setShared(true).setUser("arthur"), userSession);
+      fail();
+    } catch (Exception e) {
+      assertThat(e).isInstanceOf(ForbiddenException.class).hasMessage("User cannot own this filter because of insufficient rights");
+    }
+    verify(issueFilterDao, never()).update(any(IssueFilterDto.class));
+  }
+
+  @Test
   public void should_not_update_if_filter_not_found() {
     when(issueFilterDao.selectById(1L)).thenReturn(null);
 
@@ -376,11 +392,12 @@ public class IssueFilterServiceTest {
 
   @Test
   public void should_change_shared_filter_ownership_when_admin() throws Exception {
-    String currentUser = "dave.loper";
     IssueFilterDto sharedFilter = new IssueFilterDto().setId(1L).setName("My filter").setUserLogin("former.owner").setShared(true);
     IssueFilterDto expectedDto = new IssueFilterDto().setName("My filter").setUserLogin("new.owner").setShared(true);
 
-    UserSession userSession = MockUserSession.create().setLogin(currentUser).setPermissions(Permission.SYSTEM_ADMIN, Permission.DASHBOARD_SHARING);
+    // New owner should have sharing perm in order to own the filter
+    when(authorizationDao.selectGlobalPermissions("new.owner")).thenReturn(newArrayList(Permission.DASHBOARD_SHARING.key()));
+    when(authorizationDao.selectGlobalPermissions("john")).thenReturn(newArrayList(Permission.SYSTEM_ADMIN.key()));
 
     when(issueFilterDao.selectById(1L)).thenReturn(sharedFilter);
     when(issueFilterDao.selectSharedFilters()).thenReturn(Lists.newArrayList(sharedFilter));
@@ -445,7 +462,7 @@ public class IssueFilterServiceTest {
 
   @Test
   public void should_delete_shared_filter_if_user_is_admin() {
-    UserSession userSession = MockUserSession.create().setLogin("john").setPermissions(Permission.SYSTEM_ADMIN);
+    when(authorizationDao.selectGlobalPermissions("john")).thenReturn(newArrayList(Permission.SYSTEM_ADMIN.key()));
     when(issueFilterDao.selectById(1L)).thenReturn(new IssueFilterDto().setId(1L).setName("My Issues").setUserLogin("arthur").setShared(true));
 
     service.delete(1L, userSession);