From 8e67d5b0de7d0cec19bec6c5b48a990651d7266b Mon Sep 17 00:00:00 2001 From: Simon Brandhof Date: Tue, 31 Jan 2017 18:34:44 +0100 Subject: [PATCH] SONAR-8716 drop UserSession#checkComponentPermission(String,String) --- .../java/org/sonar/ce/user/CeUserSession.java | 5 -- .../org/sonar/server/batch/IssuesAction.java | 13 ++-- .../server/duplication/ws/ShowAction.java | 5 +- .../server/user/AbstractUserSession.java | 8 --- .../server/user/ThreadLocalUserSession.java | 6 -- .../org/sonar/server/user/UserSession.java | 8 --- .../sonar/server/batch/IssuesActionTest.java | 22 ++++--- .../sonar/server/tester/UserSessionRule.java | 6 -- .../server/user/ServerUserSessionTest.java | 61 ------------------- 9 files changed, 18 insertions(+), 116 deletions(-) diff --git a/server/sonar-ce/src/main/java/org/sonar/ce/user/CeUserSession.java b/server/sonar-ce/src/main/java/org/sonar/ce/user/CeUserSession.java index 0e15807fe29..b2ed1cfc861 100644 --- a/server/sonar-ce/src/main/java/org/sonar/ce/user/CeUserSession.java +++ b/server/sonar-ce/src/main/java/org/sonar/ce/user/CeUserSession.java @@ -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(); diff --git a/server/sonar-server/src/main/java/org/sonar/server/batch/IssuesAction.java b/server/sonar-server/src/main/java/org/sonar/server/batch/IssuesAction.java index 4573ded8dde..ae5a5faede1 100644 --- a/server/sonar-server/src/main/java/org/sonar/server/batch/IssuesAction.java +++ b/server/sonar-server/src/main/java/org/sonar/server/batch/IssuesAction.java @@ -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 keysByUUid = keysByUUid(session, component); ScannerInput.ServerIssue.Builder issueBuilder = ScannerInput.ServerIssue.newBuilder(); for (Iterator issueDocIterator = issueIndex.selectIssuesForBatch(component); issueDocIterator.hasNext();) { handleIssue(issueDocIterator.next(), issueBuilder, keysByUUid, response.stream().output()); } - } finally { - MyBatis.closeQuietly(session); } } diff --git a/server/sonar-server/src/main/java/org/sonar/server/duplication/ws/ShowAction.java b/server/sonar-server/src/main/java/org/sonar/server/duplication/ws/ShowAction.java index 3f0aa8f9ade..a1947958968 100644 --- a/server/sonar-server/src/main/java/org/sonar/server/duplication/ws/ShowAction.java +++ b/server/sonar-server/src/main/java/org/sonar/server/duplication/ws/ShowAction.java @@ -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 blocks = parser.parse(component, duplications, dbSession); duplicationsJsonWriter.write(blocks, json, dbSession); json.endObject().close(); - } finally { - dbClient.closeSession(dbSession); } } diff --git a/server/sonar-server/src/main/java/org/sonar/server/user/AbstractUserSession.java b/server/sonar-server/src/main/java/org/sonar/server/user/AbstractUserSession.java index 76335c8b83d..ce1f8dfd526 100644 --- a/server/sonar-server/src/main/java/org/sonar/server/user/AbstractUserSession.java +++ b/server/sonar-server/src/main/java/org/sonar/server/user/AbstractUserSession.java @@ -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)) { diff --git a/server/sonar-server/src/main/java/org/sonar/server/user/ThreadLocalUserSession.java b/server/sonar-server/src/main/java/org/sonar/server/user/ThreadLocalUserSession.java index 9692239f592..34607091b75 100644 --- a/server/sonar-server/src/main/java/org/sonar/server/user/ThreadLocalUserSession.java +++ b/server/sonar-server/src/main/java/org/sonar/server/user/ThreadLocalUserSession.java @@ -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); diff --git a/server/sonar-server/src/main/java/org/sonar/server/user/UserSession.java b/server/sonar-server/src/main/java/org/sonar/server/user/UserSession.java index 9ccd1d380f4..772293bfe6b 100644 --- a/server/sonar-server/src/main/java/org/sonar/server/user/UserSession.java +++ b/server/sonar-server/src/main/java/org/sonar/server/user/UserSession.java @@ -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 diff --git a/server/sonar-server/src/test/java/org/sonar/server/batch/IssuesActionTest.java b/server/sonar-server/src/test/java/org/sonar/server/batch/IssuesActionTest.java index b7087ef9e90..a25f4948c74 100644 --- a/server/sonar-server/src/test/java/org/sonar/server/batch/IssuesActionTest.java +++ b/server/sonar-server/src/test/java/org/sonar/server/batch/IssuesActionTest.java @@ -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()); } } diff --git a/server/sonar-server/src/test/java/org/sonar/server/tester/UserSessionRule.java b/server/sonar-server/src/test/java/org/sonar/server/tester/UserSessionRule.java index 2941eabb8fc..6a6333bec99 100644 --- a/server/sonar-server/src/test/java/org/sonar/server/tester/UserSessionRule.java +++ b/server/sonar-server/src/test/java/org/sonar/server/tester/UserSessionRule.java @@ -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); diff --git a/server/sonar-server/src/test/java/org/sonar/server/user/ServerUserSessionTest.java b/server/sonar-server/src/test/java/org/sonar/server/user/ServerUserSessionTest.java index a357d6925a5..963568ff6f6 100644 --- a/server/sonar-server/src/test/java/org/sonar/server/user/ServerUserSessionTest.java +++ b/server/sonar-server/src/test/java/org/sonar/server/user/ServerUserSessionTest.java @@ -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); -- 2.39.5