SONAR-2474 Only issue filter owner can change filter shared parameter

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 e46717848cb2c60746827392b94cce3cc847bc0b..622607aa66c4ad6fde5b73c8ddce950e42211884 100644 (file)
--- a/sonar-server/src/main/java/org/sonar/server/issue/IssueFilterService.java
+++ b/sonar-server/src/main/java/org/sonar/server/issue/IssueFilterService.java
@@ -43,6 +43,7 @@ import org.sonar.server.exceptions.UnauthorizedException;
 import org.sonar.server.user.UserSession;
 
 import javax.annotation.CheckForNull;
+
 import java.util.List;
 import java.util.Map;
 
@@ -100,6 +101,7 @@ public class IssueFilterService implements ServerComponent {
     String login = getLoggedLogin(userSession);
     IssueFilterDto existingFilterDto = findIssueFilterDto(issueFilter.id(), login);
     verifyCurrentUserCanModifyFilter(existingFilterDto.toIssueFilter(), login);
+    verifyCurrentUserCanChangeFilterSharingFilter(issueFilter, existingFilterDto, login);
     if (!existingFilterDto.getUserLogin().equals(issueFilter.user())) {
       verifyCurrentUserCanChangeFilterOwnership(login);
     }
@@ -216,6 +218,12 @@ public class IssueFilterService implements ServerComponent {
     }
   }
 
+  private void verifyCurrentUserCanChangeFilterSharingFilter(DefaultIssueFilter issueFilter, IssueFilterDto existingFilterDto, String login) {
+    if (existingFilterDto.isShared() != issueFilter.shared() && !existingFilterDto.getUserLogin().equals(login)) {
+      throw new ForbiddenException("Only owner of a filter can change sharing");
+    }
+  }
+
   private void verifyCurrentUserCanChangeFilterOwnership(String user) {
     if (!isAdmin(user)) {
       throw new ForbiddenException("User is not authorized to change the owner of this filter");

diff --git a/sonar-server/src/main/webapp/WEB-INF/app/views/issues/_filter_shared_form.html.erb b/sonar-server/src/main/webapp/WEB-INF/app/views/issues/_filter_shared_form.html.erb
index 82a26226f270c1af50d920b067218293716a6290..d595611f70e7cd36d7c75e191b052fa91fcb4f65 100644 (file)
--- a/sonar-server/src/main/webapp/WEB-INF/app/views/issues/_filter_shared_form.html.erb
+++ b/sonar-server/src/main/webapp/WEB-INF/app/views/issues/_filter_shared_form.html.erb
@@ -21,7 +21,11 @@
     <input id="user" name="user" type="hidden" value="<%= h(@filter.user) if @filter -%>"/>
   <% end %>
   <div class="modal-field">
-    <label for="shared"><%= message('issue_filter.form.share') -%></label>
-    <input id="shared" name="shared" type="checkbox" value="true" <%= 'checked' if (@filter && @filter.shared) -%>/>
+    <% if !@filter || @filter.user == current_user.login %>
+      <label for="shared"><%= message('issue_filter.form.share') -%></label>
+      <input id="shared" name="shared" type="checkbox" value="true" <%= 'checked' if (@filter && @filter.shared) -%>/>
+    <% else %>
+      <input id="shared" name="shared" type="hidden" value="<%= @filter.shared if @filter -%>"/>
+    <% end %>
   </div>
 </div>
\ No newline at end of file

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 eab2f15d8dc90561f846747ded69d85c7c8471a7..49a5cbd5da273033d3c2e3f8f5014e12b43c223e 100644 (file)
--- a/sonar-server/src/test/java/org/sonar/server/issue/IssueFilterServiceTest.java
+++ b/sonar-server/src/test/java/org/sonar/server/issue/IssueFilterServiceTest.java
@@ -243,6 +243,31 @@ public class IssueFilterServiceTest {
     verify(issueFilterDao).update(any(IssueFilterDto.class));
   }
 
+  @Test
+  public void should_update_sharing() {
+    when(issueFilterDao.selectById(1L)).thenReturn(new IssueFilterDto().setId(1L).setName("My Filter").setShared(true).setUserLogin("john"));
+
+    DefaultIssueFilter result = service.update(new DefaultIssueFilter().setId(1L).setName("My Filter").setShared(false).setUser("john"), userSession);
+    assertThat(result.shared()).isFalse();
+
+    verify(issueFilterDao).update(any(IssueFilterDto.class));
+  }
+
+  @Test
+  public void should_not_update_sharing_if_not_owner() {
+    // John is admin and want to change arthur filter sharing
+    when(issueFilterDao.selectById(1L)).thenReturn(new IssueFilterDto().setId(1L).setName("Arthur Filter").setShared(true).setUserLogin("arthur"));
+    when(authorizationDao.selectGlobalPermissions("john")).thenReturn(newArrayList(Permission.SYSTEM_ADMIN.key()));
+
+    try {
+      service.update(new DefaultIssueFilter().setId(1L).setName("Arthur Filter").setShared(false).setUser("john"), userSession);
+      fail();
+    } catch (Exception e) {
+      assertThat(e).isInstanceOf(ForbiddenException.class).hasMessage("Only owner of a filter can change sharing");
+    }
+    verify(issueFilterDao, never()).update(any(IssueFilterDto.class));
+  }
+
   @Test
   public void should_update_own_user_filter_without_changing_anything() {
     IssueFilterDto dto = new IssueFilterDto().setId(1L).setName("My Filter").setUserLogin("john");
@@ -273,11 +298,11 @@ public class IssueFilterServiceTest {
   @Test
   public void should_update_other_shared_filter_if_admin() {
     when(authorizationDao.selectGlobalPermissions("john")).thenReturn(newArrayList(Permission.SYSTEM_ADMIN.key()));
-    when(issueFilterDao.selectById(1L)).thenReturn(new IssueFilterDto().setId(1L).setName("My Old Filter").setUserLogin("arthur").setShared(true));
+    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"), userSession);
+    DefaultIssueFilter result = service.update(new DefaultIssueFilter().setId(1L).setName("My New Filter").setDescription("New description").setShared(true), userSession);
     assertThat(result.name()).isEqualTo("My New Filter");
-    assertThat(result.shared()).isFalse();
+    assertThat(result.description()).isEqualTo("New description");
 
     verify(issueFilterDao).update(any(IssueFilterDto.class));
   }