From 2a6d917d63bdb5cc05e8b97549fb948fc5277d43 Mon Sep 17 00:00:00 2001 From: Julien Lancelot Date: Wed, 17 Jul 2013 11:25:46 +0200 Subject: [PATCH] SONAR-2474 Only issue filter owner can change filter shared parameter --- .../server/issue/IssueFilterService.java | 8 +++++ .../views/issues/_filter_shared_form.html.erb | 8 +++-- .../server/issue/IssueFilterServiceTest.java | 31 +++++++++++++++++-- 3 files changed, 42 insertions(+), 5 deletions(-) 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 e46717848cb..622607aa66c 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 @@ -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 82a26226f27..d595611f70e 100644 --- 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 @@ <% end %>
- - /> + <% if !@filter || @filter.user == current_user.login %> + + /> + <% else %> + + <% end %>
\ 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 eab2f15d8dc..49a5cbd5da2 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 @@ -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)); } -- 2.39.5