]> source.dussan.org Git - sonarqube.git/commitdiff
SONAR-8716 drop UserSession#checkComponentPermission(String,String)
authorSimon Brandhof <simon.brandhof@sonarsource.com>
Tue, 31 Jan 2017 17:34:44 +0000 (18:34 +0100)
committerSimon Brandhof <simon.brandhof@sonarsource.com>
Wed, 1 Feb 2017 16:11:52 +0000 (17:11 +0100)
server/sonar-ce/src/main/java/org/sonar/ce/user/CeUserSession.java
server/sonar-server/src/main/java/org/sonar/server/batch/IssuesAction.java
server/sonar-server/src/main/java/org/sonar/server/duplication/ws/ShowAction.java
server/sonar-server/src/main/java/org/sonar/server/user/AbstractUserSession.java
server/sonar-server/src/main/java/org/sonar/server/user/ThreadLocalUserSession.java
server/sonar-server/src/main/java/org/sonar/server/user/UserSession.java
server/sonar-server/src/test/java/org/sonar/server/batch/IssuesActionTest.java
server/sonar-server/src/test/java/org/sonar/server/tester/UserSessionRule.java
server/sonar-server/src/test/java/org/sonar/server/user/ServerUserSessionTest.java

index 0e15807fe2998530954ac8d5011c492967503e68..b2ed1cfc86104560e5ca433149003995e67d2240 100644 (file)
@@ -111,11 +111,6 @@ public class CeUserSession implements UserSession {
     return notImplemented();
   }
 
-  @Override
-  public UserSession checkComponentPermission(String projectPermission, String componentKey) {
-    return notImplemented();
-  }
-
   @Override
   public UserSession checkComponentUuidPermission(String permission, String componentUuid) {
     return notImplemented();
index 4573ded8dde8de552107c62b01363c78f55e47ee..ae5a5faede12da3810a8663b8960b3ac813ac794 100644 (file)
@@ -30,7 +30,6 @@ import org.sonar.api.server.ws.Response;
 import org.sonar.api.server.ws.WebService;
 import org.sonar.db.DbClient;
 import org.sonar.db.DbSession;
-import org.sonar.db.MyBatis;
 import org.sonar.db.component.ComponentDto;
 import org.sonar.scanner.protocol.input.ScannerInput;
 import org.sonar.server.component.ComponentFinder;
@@ -77,21 +76,19 @@ public class IssuesAction implements BatchWsAction {
 
   @Override
   public void handle(Request request, Response response) throws Exception {
-    String componentKey = request.mandatoryParam(PARAM_KEY);
-    userSession.checkComponentPermission(USER, componentKey);
-
     response.stream().setMediaType(MediaTypes.PROTOBUF);
-    DbSession session = dbClient.openSession(false);
-    try {
+
+    try (DbSession session = dbClient.openSession(false)) {
+      String componentKey = request.mandatoryParam(PARAM_KEY);
       ComponentDto component = componentFinder.getByKey(session, componentKey);
+      userSession.checkComponentPermission(USER, component);
+
       Map<String, String> keysByUUid = keysByUUid(session, component);
 
       ScannerInput.ServerIssue.Builder issueBuilder = ScannerInput.ServerIssue.newBuilder();
       for (Iterator<IssueDoc> issueDocIterator = issueIndex.selectIssuesForBatch(component); issueDocIterator.hasNext();) {
         handleIssue(issueDocIterator.next(), issueBuilder, keysByUUid, response.stream().output());
       }
-    } finally {
-      MyBatis.closeQuietly(session);
     }
   }
 
index 3f0aa8f9adec792221496a52f351324a6aabcdb9..a194795896871188e71bb2511d92af69181b4dd2 100644 (file)
@@ -77,8 +77,7 @@ public class ShowAction implements RequestHandler {
 
   @Override
   public void handle(Request request, Response response) {
-    DbSession dbSession = dbClient.openSession(false);
-    try {
+    try (DbSession dbSession = dbClient.openSession(false)) {
       ComponentDto component = componentFinder.getByUuidOrKey(dbSession, request.param("uuid"), request.param("key"), UUID_AND_KEY);
       userSession.checkComponentPermission(UserRole.CODEVIEWER, component);
       JsonWriter json = response.newJsonWriter().beginObject();
@@ -86,8 +85,6 @@ public class ShowAction implements RequestHandler {
       List<DuplicationsParser.Block> blocks = parser.parse(component, duplications, dbSession);
       duplicationsJsonWriter.write(blocks, json, dbSession);
       json.endObject().close();
-    } finally {
-      dbClient.closeSession(dbSession);
     }
   }
 
index 76335c8b83d2aca394073c563a84ef5e4c5e876b..ce1f8dfd5267cdf7883e09e91d911e4a10b8db2b 100644 (file)
@@ -85,14 +85,6 @@ public abstract class AbstractUserSession implements UserSession {
     return this;
   }
 
-  @Override
-  public UserSession checkComponentPermission(String projectPermission, String componentKey) {
-    if (!hasComponentPermission(projectPermission, componentKey)) {
-      throw new ForbiddenException(INSUFFICIENT_PRIVILEGES_MESSAGE);
-    }
-    return this;
-  }
-
   @Override
   public UserSession checkComponentUuidPermission(String permission, String componentUuid) {
     if (!hasComponentUuidPermission(permission, componentUuid)) {
index 9692239f59216298dfa88e0a8cb96cc3baa2bd50..34607091b7554292c776da0cc1da4eb5360b31e0 100644 (file)
@@ -126,12 +126,6 @@ public class ThreadLocalUserSession implements UserSession {
     return this;
   }
 
-  @Override
-  public UserSession checkComponentPermission(String projectPermission, String componentKey) {
-    get().checkComponentPermission(projectPermission, componentKey);
-    return this;
-  }
-
   @Override
   public UserSession checkComponentUuidPermission(String permission, String componentUuid) {
     get().checkComponentUuidPermission(permission, componentUuid);
index 9ccd1d380f4f5d8c68096b1ee317c187e46f2f8c..772293bfe6b5141663c93cfb62d478ce1965fdad 100644 (file)
@@ -122,14 +122,6 @@ public interface UserSession {
    */
   UserSession checkComponentPermission(String projectPermission, ComponentDto component);
 
-  /**
-   * Ensures that permission is granted to user on the specified component, otherwise throws
-   * a {@link org.sonar.server.exceptions.ForbiddenException}.
-   * If the component doesn't exist and the user doesn't have the global permission,
-   * throws a {@link org.sonar.server.exceptions.ForbiddenException}.
-   */
-  UserSession checkComponentPermission(String projectPermission, String componentKey);
-
   /**
    * Ensures that permission is granted to user, otherwise throws a {@link org.sonar.server.exceptions.ForbiddenException}.
    * If the component doesn't exist and the user doesn't have the permission, throws
index b7087ef9e900512c160979792a3257ee1962e128..a25f4948c74014b00e07582f2c9cee51d5a31de2 100644 (file)
@@ -107,7 +107,7 @@ public class IssuesActionTest {
       .setChecksum(null)
       .setAssignee(null));
 
-    addBrowsePermissionOnComponent(PROJECT_KEY);
+    addBrowsePermissionOnComponent(project);
     WsTester.TestRequest request = tester.newGetRequest("batch", "issues").setParam("key", PROJECT_KEY);
 
     ServerIssue serverIssue = ServerIssue.parseDelimitedFrom(new ByteArrayInputStream(request.execute().output()));
@@ -146,7 +146,7 @@ public class IssuesActionTest {
       .setChecksum("123456")
       .setAssignee("john"));
 
-    addBrowsePermissionOnComponent(PROJECT_KEY);
+    addBrowsePermissionOnComponent(project);
     WsTester.TestRequest request = tester.newGetRequest("batch", "issues").setParam("key", PROJECT_KEY);
 
     ServerIssue serverIssue = ServerIssue.parseDelimitedFrom(new ByteArrayInputStream(request.execute().output()));
@@ -184,7 +184,7 @@ public class IssuesActionTest {
       .setChecksum("123456")
       .setAssignee("john"));
 
-    addBrowsePermissionOnComponent(PROJECT_KEY);
+    addBrowsePermissionOnComponent(project);
     WsTester.TestRequest request = tester.newGetRequest("batch", "issues").setParam("key", PROJECT_KEY);
 
     ServerIssue serverIssue = ServerIssue.parseDelimitedFrom(new ByteArrayInputStream(request.execute().output()));
@@ -222,7 +222,7 @@ public class IssuesActionTest {
       .setChecksum("123456")
       .setAssignee("john"));
 
-    addBrowsePermissionOnComponent(FILE_KEY);
+    addBrowsePermissionOnComponent(project);
     WsTester.TestRequest request = tester.newGetRequest("batch", "issues").setParam("key", FILE_KEY);
 
     ServerIssue serverIssue = ServerIssue.parseDelimitedFrom(new ByteArrayInputStream(request.execute().output()));
@@ -260,7 +260,7 @@ public class IssuesActionTest {
       .setChecksum("123456")
       .setAssignee("john"));
 
-    addBrowsePermissionOnComponent(MODULE_KEY);
+    addBrowsePermissionOnComponent(project);
     WsTester.TestRequest request = tester.newGetRequest("batch", "issues").setParam("key", MODULE_KEY);
 
     ServerIssue previousIssue = ServerIssue.parseDelimitedFrom(new ByteArrayInputStream(request.execute().output()));
@@ -299,7 +299,7 @@ public class IssuesActionTest {
       .setChecksum("123456")
       .setAssignee("john"));
 
-    addBrowsePermissionOnComponent(PROJECT_KEY);
+    addBrowsePermissionOnComponent(project);
     WsTester.TestRequest request = tester.newGetRequest("batch", "issues").setParam("key", PROJECT_KEY);
 
     ServerIssue serverIssue = ServerIssue.parseDelimitedFrom(new ByteArrayInputStream(request.execute().output()));
@@ -310,10 +310,12 @@ public class IssuesActionTest {
 
   @Test
   public void fail_without_browse_permission_on_file() throws Exception {
-    addBrowsePermissionOnComponent(PROJECT_KEY);
+    ComponentDto project = db.components().insertProject();
+    ComponentDto file = db.components().insertComponent(ComponentTesting.newFileDto(project));
 
     thrown.expect(ForbiddenException.class);
-    tester.newGetRequest("batch", "issues").setParam("key", "Other component key").execute();
+
+    tester.newGetRequest("batch", "issues").setParam("key", file.key()).execute();
   }
 
   private void indexIssues(IssueDoc... issues) {
@@ -329,7 +331,7 @@ public class IssuesActionTest {
     authorizationIndexerTester.allow(access);
   }
 
-  private void addBrowsePermissionOnComponent(String componentKey) {
-    userSessionRule.addComponentPermission(UserRole.USER, PROJECT_KEY, componentKey);
+  private void addBrowsePermissionOnComponent(ComponentDto project) {
+    userSessionRule.addProjectUuidPermissions(UserRole.USER, project.uuid());
   }
 }
index 2941eabb8fc4a360d740fa4ec079d92aed9e9b21..6a6333bec99e3106b0c1dda2d33051cdae9d156e 100644 (file)
@@ -343,12 +343,6 @@ public class UserSessionRule implements TestRule, UserSession {
     return this;
   }
 
-  @Override
-  public UserSession checkComponentPermission(String projectPermission, String componentKey) {
-    currentUserSession.checkComponentPermission(projectPermission, componentKey);
-    return this;
-  }
-
   @Override
   public UserSession checkComponentUuidPermission(String permission, String componentUuid) {
     currentUserSession.checkComponentUuidPermission(permission, componentUuid);
index a357d6925a528307c60ce6306dfd01618272d8fe..963568ff6f6296beb92e191eb477b50c6de92ab5 100644 (file)
@@ -229,67 +229,6 @@ public class ServerUserSessionTest {
     assertThat(session.hasComponentUuidPermission(UserRole.ADMIN, FILE_UUID)).isFalse();
   }
 
-  @Test
-  public void checkComponentPermission_succeeds_if_user_has_permission_for_specified_key_in_db() {
-    addProjectPermissions(project, UserRole.USER);
-    UserSession session = newUserSession(userDto);
-
-    session.checkComponentPermission(UserRole.USER, FILE_KEY);
-  }
-
-  @Test
-  public void checkComponentPermission_succeeds_if_user_has_global_permission_in_db() {
-    addGlobalPermissions(UserRole.USER);
-    UserSession session = newUserSession(userDto);
-
-    session.checkComponentPermission(UserRole.USER, FILE_KEY);
-  }
-
-  @Test
-  public void checkComponentPermission_succeeds_when_flag_is_true_on_UserDto_no_matter_if_user_has_permission_for_specified_key_in_db() {
-    UserSession underTest = newUserSession(ROOT_USER_DTO);
-
-    assertThat(underTest.checkComponentPermission(UserRole.USER, FILE_KEY)).isSameAs(underTest);
-    assertThat(underTest.checkComponentPermission(UserRole.CODEVIEWER, FILE_KEY)).isSameAs(underTest);
-    assertThat(underTest.checkComponentPermission("whatever", "who cares?")).isSameAs(underTest);
-  }
-
-  @Test
-  public void checkComponentPermission_throws_FE_when_user_has_not_permission_for_specified_key_in_db() {
-    ComponentDto project2 = db.components().insertComponent(ComponentTesting.newProjectDto(db.organizations().insert()));
-    ComponentDto file2 = db.components().insertComponent(ComponentTesting.newFileDto(project2, null));
-    addProjectPermissions(project, UserRole.USER);
-    UserSession session = newUserSession(userDto);
-
-    expectInsufficientPrivilegesForbiddenException();
-
-    session.checkComponentPermission(UserRole.USER, file2.getKey());
-  }
-
-  @Test
-  public void checkComponentPermission_throws_FE_when_project_does_not_exist_in_db() {
-    addProjectPermissions(project, UserRole.USER);
-    UserSession session = newUserSession(userDto);
-
-    expectInsufficientPrivilegesForbiddenException();
-
-    session.checkComponentPermission(UserRole.USER, "another");
-  }
-
-  @Test
-  public void checkComponentPermission_fails_with_FE_when_project_of_specified_uuid_can_not_be_found() {
-    ComponentDto project2 = db.components().insertComponent(ComponentTesting.newProjectDto(db.organizations().insert()));
-    ComponentDto file2 = db.components().insertComponent(ComponentTesting.newFileDto(project2, null)
-      // Simulate file is linked to an invalid project
-      .setProjectUuid("INVALID"));
-    addProjectPermissions(project, UserRole.USER);
-    UserSession session = newUserSession(userDto);
-
-    expectInsufficientPrivilegesForbiddenException();
-
-    session.checkComponentPermission(UserRole.USER, file2.getKey());
-  }
-
   @Test
   public void checkComponentUuidPermission_succeeds_if_user_has_permission_for_specified_uuid_in_db() {
     UserSession underTest = newUserSession(ROOT_USER_DTO);