SONAR-3755 Removed useless security check

diff --git a/sonar-core/src/main/java/org/sonar/core/issue/DefaultIssueQueryResult.java b/sonar-core/src/main/java/org/sonar/core/issue/DefaultIssueQueryResult.java
index ca92f7e8d29b7fed618deaf26fbfcc8429a3be2d..4a7822c75c90ab5f3e8e2f7892358cd4eb947d08 100644 (file)
--- a/sonar-core/src/main/java/org/sonar/core/issue/DefaultIssueQueryResult.java
+++ b/sonar-core/src/main/java/org/sonar/core/issue/DefaultIssueQueryResult.java
@@ -107,7 +107,7 @@ public class DefaultIssueQueryResult implements IssueQueryResult {
     if (issues != null && !issues.isEmpty()) {
       return issues.get(0);
     }
-    throw new IllegalStateException("No issue");
+    throw new IllegalStateException("No issue found");
   }
 
   @Override

diff --git a/sonar-server/src/main/java/org/sonar/server/issue/ActionService.java b/sonar-server/src/main/java/org/sonar/server/issue/ActionService.java
index 3e55485410dd2c66e7b79d2623dc99f007c0592b..48df6ff62d79bf2ad6ec7e29dc17a0963dc62b53 100644 (file)
--- a/sonar-server/src/main/java/org/sonar/server/issue/ActionService.java
+++ b/sonar-server/src/main/java/org/sonar/server/issue/ActionService.java
@@ -68,6 +68,12 @@ public class ActionService implements ServerComponent {
     this.actions = actions;
   }
 
+  public List<Action> listAvailableActions(String issueKey) {
+    IssueQueryResult queryResult = loadIssue(issueKey);
+    final DefaultIssue issue = (DefaultIssue) queryResult.first();
+    return listAvailableActions(issue);
+  }
+
   public List<Action> listAvailableActions(final Issue issue) {
     return newArrayList(Iterables.filter(actions.list(), new Predicate<Action>() {
       @Override
@@ -77,21 +83,11 @@ public class ActionService implements ServerComponent {
     }));
   }
 
-  public List<Action> listAvailableActions(String issueKey) {
-    IssueQueryResult queryResult = loadIssue(issueKey);
-    final DefaultIssue issue = (DefaultIssue) queryResult.first();
-    return listAvailableActions(issue);
-  }
-
   public Issue execute(String issueKey, String actionKey, UserSession userSession) {
     Preconditions.checkArgument(!Strings.isNullOrEmpty(actionKey), "Missing action");
 
     IssueQueryResult queryResult = loadIssue(issueKey);
     DefaultIssue issue = (DefaultIssue) queryResult.first();
-    if (issue == null) {
-      throw new IllegalArgumentException("Issue is not found : " + issueKey);
-    }
-
     Action action = getAction(actionKey);
     if (action == null) {
       throw new IllegalArgumentException("Action is not found : " + actionKey);

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 321506a052878a2a0f8df1e249f7b164277f2e80..87533a774abb14fee5153ad40aed776da3571d16 100644 (file)
--- a/sonar-server/src/main/java/org/sonar/server/issue/InternalRubyIssueService.java
+++ b/sonar-server/src/main/java/org/sonar/server/issue/InternalRubyIssueService.java
@@ -81,11 +81,11 @@ public class InternalRubyIssueService implements ServerComponent {
   }
 
   public List<Transition> listTransitions(String issueKey) {
-    return issueService.listTransitions(issueKey, UserSession.get());
+    return issueService.listTransitions(issueKey);
   }
 
   public List<Transition> listTransitions(Issue issue) {
-    return issueService.listTransitions(issue, UserSession.get());
+    return issueService.listTransitions(issue);
   }
 
   public List<String> listStatus() {

diff --git a/sonar-server/src/main/java/org/sonar/server/issue/IssueService.java b/sonar-server/src/main/java/org/sonar/server/issue/IssueService.java
index 14ade9f87e4203c0bef404e7e61c16f15b6f2e79..331bc0ebdc4ed60a46e945b90542fe3714a9e356 100644 (file)
--- a/sonar-server/src/main/java/org/sonar/server/issue/IssueService.java
+++ b/sonar-server/src/main/java/org/sonar/server/issue/IssueService.java
@@ -92,25 +92,25 @@ public class IssueService implements ServerComponent {
    * <p/>
    * Never return null, but return an empty list if the issue does not exist.
    */
-  public List<Transition> listTransitions(String issueKey, UserSession userSession) {
-    return listTransitions(loadIssue(issueKey).first(), userSession);
+  public List<Transition> listTransitions(String issueKey) {
+    Issue issue = loadIssue(issueKey).first();
+    return listTransitions(issue);
   }
 
   /**
    * Never return null, but an empty list if the issue does not exist.
+   * No security check is done since it should already have been done to get the issue
    */
-  public List<Transition> listTransitions(@Nullable Issue issue, UserSession userSession) {
+  public List<Transition> listTransitions(@Nullable Issue issue) {
     if (issue == null) {
       return Collections.emptyList();
     }
-    checkAuthorization(userSession, issue, UserRole.USER);
     return workflow.outTransitions(issue);
   }
 
   public Issue doTransition(String issueKey, String transition, UserSession userSession) {
     IssueQueryResult queryResult = loadIssue(issueKey);
     DefaultIssue issue = (DefaultIssue) queryResult.first();
-    checkAuthorization(userSession, issue, UserRole.USER);
     IssueChangeContext context = IssueChangeContext.createUser(new Date(), userSession.login());
     if (workflow.doTransition(issue, transition, context)) {
       issueStorage.save(issue);
@@ -122,7 +122,6 @@ public class IssueService implements ServerComponent {
   public Issue assign(String issueKey, @Nullable String assignee, UserSession userSession) {
     IssueQueryResult queryResult = loadIssue(issueKey);
     DefaultIssue issue = (DefaultIssue) queryResult.first();
-    checkAuthorization(userSession, issue, UserRole.USER);
     if (assignee != null && userFinder.findByLogin(assignee) == null) {
       throw new IllegalArgumentException("Unknown user: " + assignee);
     }
@@ -140,7 +139,6 @@ public class IssueService implements ServerComponent {
     }
     IssueQueryResult queryResult = loadIssue(issueKey);
     DefaultIssue issue = (DefaultIssue) queryResult.first();
-    checkAuthorization(userSession, issue, UserRole.USER);
 
     IssueChangeContext context = IssueChangeContext.createUser(new Date(), userSession.login());
     if (issueUpdater.plan(issue, actionPlanKey, context)) {
@@ -153,7 +151,6 @@ public class IssueService implements ServerComponent {
   public Issue setSeverity(String issueKey, String severity, UserSession userSession) {
     IssueQueryResult queryResult = loadIssue(issueKey);
     DefaultIssue issue = (DefaultIssue) queryResult.first();
-    checkAuthorization(userSession, issue, UserRole.USER);
 
     IssueChangeContext context = IssueChangeContext.createUser(new Date(), userSession.login());
     if (issueUpdater.setManualSeverity(issue, severity, context)) {

diff --git a/sonar-server/src/test/java/org/sonar/server/issue/ActionServiceTest.java b/sonar-server/src/test/java/org/sonar/server/issue/ActionServiceTest.java
index 1e4831301adec0a0f172ba020cd7969e950d0e3d..9624fe12cfd43507fa7bfde852d6ad135ef7cae3 100644 (file)
--- a/sonar-server/src/test/java/org/sonar/server/issue/ActionServiceTest.java
+++ b/sonar-server/src/test/java/org/sonar/server/issue/ActionServiceTest.java
@@ -124,7 +124,7 @@ public class ActionServiceTest {
       actionService.execute("ABCD", "link-to-jira", mock(UserSession.class));
       fail();
     } catch (Exception e) {
-      assertThat(e).isInstanceOf(IllegalStateException.class).hasMessage("No issue");
+      assertThat(e).isInstanceOf(IllegalStateException.class).hasMessage("No issue found");
     }
     verifyZeroInteractions(function);
   }
@@ -187,7 +187,7 @@ public class ActionServiceTest {
       actionService.listAvailableActions("ABCD");
       fail();
     } catch (Exception e) {
-      assertThat(e).isInstanceOf(IllegalStateException.class).hasMessage("No issue");
+      assertThat(e).isInstanceOf(IllegalStateException.class).hasMessage("No issue found");
     }
   }
 

diff --git a/sonar-server/src/test/java/org/sonar/server/issue/IssueServiceTest.java b/sonar-server/src/test/java/org/sonar/server/issue/IssueServiceTest.java
index bfeec5cbfc1950b09d3fc1419c8d9f514d698f96..b529a572902623e7b860f0c11ddc726a42868973 100644 (file)
--- a/sonar-server/src/test/java/org/sonar/server/issue/IssueServiceTest.java
+++ b/sonar-server/src/test/java/org/sonar/server/issue/IssueServiceTest.java
@@ -118,29 +118,25 @@ public class IssueServiceTest {
 
   @Test
   public void should_list_transitions() {
-    grantAccess();
     List<Transition> transitions = newArrayList(transition);
     when(workflow.outTransitions(issue)).thenReturn(transitions);
 
-    List<Transition> result = issueService.listTransitions("ABCD", userSession);
+    List<Transition> result = issueService.listTransitions("ABCD");
     assertThat(result).hasSize(1);
     assertThat(result.get(0)).isEqualTo(transition);
-    verify(authorizationDao).isAuthorizedComponentId(anyLong(), anyInt(), eq(UserRole.USER));
   }
 
   @Test
   public void should_return_no_transition() {
-    grantAccess();
     when(issueQueryResult.first()).thenReturn(null);
     when(issueQueryResult.issues()).thenReturn(newArrayList((Issue) new DefaultIssue()));
 
-    assertThat(issueService.listTransitions("ABCD", userSession)).isEmpty();
+    assertThat(issueService.listTransitions("ABCD")).isEmpty();
     verifyZeroInteractions(workflow);
   }
 
   @Test
   public void should_do_transition() {
-    grantAccess();
     when(workflow.doTransition(eq(issue), eq(transition.key()), any(IssueChangeContext.class))).thenReturn(true);
 
     Issue result = issueService.doTransition("ABCD", transition.key(), userSession);
@@ -155,7 +151,6 @@ public class IssueServiceTest {
     assertThat(issueChangeContext.date()).isNotNull();
 
     verify(issueNotifications).sendChanges(eq(issue), eq(issueChangeContext), eq(issueQueryResult));
-    verify(authorizationDao).isAuthorizedComponentId(anyLong(), anyInt(), eq(UserRole.USER));
   }
 
   @Test
@@ -172,7 +167,6 @@ public class IssueServiceTest {
 
   @Test
   public void should_assign() {
-    grantAccess();
     String assignee = "perceval";
 
     when(userFinder.findByLogin(assignee)).thenReturn(new DefaultUser());
@@ -190,12 +184,10 @@ public class IssueServiceTest {
     assertThat(issueChangeContext.date()).isNotNull();
 
     verify(issueNotifications).sendChanges(eq(issue), eq(issueChangeContext), eq(issueQueryResult));
-    verify(authorizationDao).isAuthorizedComponentId(anyLong(), anyInt(), eq(UserRole.USER));
   }
 
   @Test
   public void should_not_assign() {
-    grantAccess();
     String assignee = "perceval";
 
     when(userFinder.findByLogin(assignee)).thenReturn(new DefaultUser());
@@ -210,7 +202,6 @@ public class IssueServiceTest {
 
   @Test
   public void should_fail_assign_if_assignee_not_found() {
-    grantAccess();
     String assignee = "perceval";
 
     when(userFinder.findByLogin(assignee)).thenReturn(null);
@@ -229,7 +220,6 @@ public class IssueServiceTest {
 
   @Test
   public void should_plan() {
-    grantAccess();
     String actionPlanKey = "EFGH";
 
     when(actionPlanService.findByKey(actionPlanKey, userSession)).thenReturn(new DefaultActionPlan());
@@ -247,12 +237,10 @@ public class IssueServiceTest {
     assertThat(issueChangeContext.date()).isNotNull();
 
     verify(issueNotifications).sendChanges(eq(issue), eq(issueChangeContext), eq(issueQueryResult));
-    verify(authorizationDao).isAuthorizedComponentId(anyLong(), anyInt(), eq(UserRole.USER));
   }
 
   @Test
   public void should_not_plan() {
-    grantAccess();
     String actionPlanKey = "EFGH";
 
     when(actionPlanService.findByKey(actionPlanKey, userSession)).thenReturn(new DefaultActionPlan());
@@ -267,7 +255,6 @@ public class IssueServiceTest {
 
   @Test
   public void should_fail_plan_if_action_plan_not_found() {
-    grantAccess();
     String actionPlanKey = "EFGH";
 
     when(actionPlanService.findByKey(actionPlanKey, userSession)).thenReturn(null);
@@ -285,7 +272,6 @@ public class IssueServiceTest {
 
   @Test
   public void should_set_severity() {
-    grantAccess();
     String severity = "MINOR";
     when(issueUpdater.setManualSeverity(eq(issue), eq(severity), any(IssueChangeContext.class))).thenReturn(true);
 
@@ -301,12 +287,10 @@ public class IssueServiceTest {
     assertThat(issueChangeContext.date()).isNotNull();
 
     verify(issueNotifications).sendChanges(eq(issue), eq(issueChangeContext), eq(issueQueryResult));
-    verify(authorizationDao).isAuthorizedComponentId(anyLong(), anyInt(), eq(UserRole.USER));
   }
 
   @Test
   public void should_not_set_severity() {
-    grantAccess();
     String severity = "MINOR";
     when(issueUpdater.setManualSeverity(eq(issue), eq(severity), any(IssueChangeContext.class))).thenReturn(false);