From c1caffa9fb8cabe9659d81372d82481edc00b545 Mon Sep 17 00:00:00 2001 From: Simon Brandhof Date: Tue, 11 Dec 2018 11:51:46 +0100 Subject: [PATCH] SONARCLOUD-213 fix Quality flaws --- .../authentication/OAuth2ContextFactory.java | 2 +- .../authentication/RequestAuthenticator.java | 6 +++-- .../permission/GroupPermissionChanger.java | 10 +++---- .../permission/PermissionTemplateService.java | 6 ++--- .../permission/UserPermissionChanger.java | 10 +++---- .../project/ws/UpdateVisibilityAction.java | 5 +--- .../PermissionTemplateServiceTest.java | 2 +- .../ws/template/ApplyTemplateActionTest.java | 26 +++++++------------ .../template/BulkApplyTemplateActionTest.java | 9 +------ .../ws/UpdateVisibilityActionTest.java | 2 +- .../main/java/org/sonar/api/web/UserRole.java | 11 ++++---- 11 files changed, 35 insertions(+), 54 deletions(-) diff --git a/server/sonar-server/src/main/java/org/sonar/server/authentication/OAuth2ContextFactory.java b/server/sonar-server/src/main/java/org/sonar/server/authentication/OAuth2ContextFactory.java index 1fd9118349f..11b000e633f 100644 --- a/server/sonar-server/src/main/java/org/sonar/server/authentication/OAuth2ContextFactory.java +++ b/server/sonar-server/src/main/java/org/sonar/server/authentication/OAuth2ContextFactory.java @@ -49,7 +49,7 @@ public class OAuth2ContextFactory { private final OAuth2AuthenticationParameters oAuthParameters; public OAuth2ContextFactory(ThreadLocalUserSession threadLocalUserSession, UserRegistrar userRegistrar, Server server, - OAuthCsrfVerifier csrfVerifier, JwtHttpHandler jwtHttpHandler, UserSessionFactory userSessionFactory, OAuth2AuthenticationParameters oAuthParameters) { + OAuthCsrfVerifier csrfVerifier, JwtHttpHandler jwtHttpHandler, UserSessionFactory userSessionFactory, OAuth2AuthenticationParameters oAuthParameters) { this.threadLocalUserSession = threadLocalUserSession; this.userRegistrar = userRegistrar; this.server = server; diff --git a/server/sonar-server/src/main/java/org/sonar/server/authentication/RequestAuthenticator.java b/server/sonar-server/src/main/java/org/sonar/server/authentication/RequestAuthenticator.java index 5f74851796a..a70b4e74cd1 100644 --- a/server/sonar-server/src/main/java/org/sonar/server/authentication/RequestAuthenticator.java +++ b/server/sonar-server/src/main/java/org/sonar/server/authentication/RequestAuthenticator.java @@ -22,12 +22,14 @@ package org.sonar.server.authentication; import javax.servlet.http.HttpServletRequest; import javax.servlet.http.HttpServletResponse; import org.sonar.api.server.ServerSide; -import org.sonar.server.authentication.event.AuthenticationException; import org.sonar.server.user.UserSession; @ServerSide public interface RequestAuthenticator { - UserSession authenticate(HttpServletRequest request, HttpServletResponse response) throws AuthenticationException; + /** + * @throws org.sonar.server.authentication.event.AuthenticationException if user is not authenticated + */ + UserSession authenticate(HttpServletRequest request, HttpServletResponse response); } diff --git a/server/sonar-server/src/main/java/org/sonar/server/permission/GroupPermissionChanger.java b/server/sonar-server/src/main/java/org/sonar/server/permission/GroupPermissionChanger.java index c5c7b0d189c..f3396095398 100644 --- a/server/sonar-server/src/main/java/org/sonar/server/permission/GroupPermissionChanger.java +++ b/server/sonar-server/src/main/java/org/sonar/server/permission/GroupPermissionChanger.java @@ -55,18 +55,18 @@ public class GroupPermissionChanger { } } - private boolean isImplicitlyAlreadyDone(GroupPermissionChange change) { + private static boolean isImplicitlyAlreadyDone(GroupPermissionChange change) { return change.getProjectId() .map(projectId -> isImplicitlyAlreadyDone(projectId, change)) .orElse(false); } - private boolean isImplicitlyAlreadyDone(ProjectId projectId, GroupPermissionChange change) { + private static boolean isImplicitlyAlreadyDone(ProjectId projectId, GroupPermissionChange change) { return isAttemptToAddPublicPermissionToPublicComponent(change, projectId) || isAttemptToRemovePermissionFromAnyoneOnPrivateComponent(change, projectId); } - private boolean isAttemptToAddPublicPermissionToPublicComponent(GroupPermissionChange change, ProjectId projectId) { + private static boolean isAttemptToAddPublicPermissionToPublicComponent(GroupPermissionChange change, ProjectId projectId) { return !projectId.isPrivate() && change.getOperation() == ADD && PUBLIC_PERMISSIONS.contains(change.getPermission()); @@ -78,7 +78,7 @@ public class GroupPermissionChanger { && change.getGroupIdOrAnyone().isAnyone(); } - private void ensureConsistencyWithVisibility(GroupPermissionChange change) { + private static void ensureConsistencyWithVisibility(GroupPermissionChange change) { change.getProjectId() .ifPresent(projectId -> { checkRequest( @@ -96,7 +96,7 @@ public class GroupPermissionChanger { && change.getGroupIdOrAnyone().isAnyone(); } - private boolean isAttemptToRemovePublicPermissionFromPublicComponent(GroupPermissionChange change, ProjectId projectId) { + private static boolean isAttemptToRemovePublicPermissionFromPublicComponent(GroupPermissionChange change, ProjectId projectId) { return !projectId.isPrivate() && change.getOperation() == REMOVE && PUBLIC_PERMISSIONS.contains(change.getPermission()); diff --git a/server/sonar-server/src/main/java/org/sonar/server/permission/PermissionTemplateService.java b/server/sonar-server/src/main/java/org/sonar/server/permission/PermissionTemplateService.java index 7824a71b2cc..4d5abb65a19 100644 --- a/server/sonar-server/src/main/java/org/sonar/server/permission/PermissionTemplateService.java +++ b/server/sonar-server/src/main/java/org/sonar/server/permission/PermissionTemplateService.java @@ -60,15 +60,13 @@ public class PermissionTemplateService { private final ProjectIndexers projectIndexers; private final UserSession userSession; private final DefaultTemplatesResolver defaultTemplatesResolver; - private final PermissionService permissionService; public PermissionTemplateService(DbClient dbClient, ProjectIndexers projectIndexers, UserSession userSession, - DefaultTemplatesResolver defaultTemplatesResolver, PermissionService permissionService) { + DefaultTemplatesResolver defaultTemplatesResolver) { this.dbClient = dbClient; this.projectIndexers = projectIndexers; this.userSession = userSession; this.defaultTemplatesResolver = defaultTemplatesResolver; - this.permissionService = permissionService; } public boolean wouldUserHaveScanPermissionWithDefaultTemplate(DbSession dbSession, @@ -170,7 +168,7 @@ public class PermissionTemplateService { } } - private boolean permissionValidForProject(ComponentDto project, String permission) { + private static boolean permissionValidForProject(ComponentDto project, String permission) { return project.isPrivate() || !PUBLIC_PERMISSIONS.contains(permission); } diff --git a/server/sonar-server/src/main/java/org/sonar/server/permission/UserPermissionChanger.java b/server/sonar-server/src/main/java/org/sonar/server/permission/UserPermissionChanger.java index 45e7775f6f4..08d5ce882d6 100644 --- a/server/sonar-server/src/main/java/org/sonar/server/permission/UserPermissionChanger.java +++ b/server/sonar-server/src/main/java/org/sonar/server/permission/UserPermissionChanger.java @@ -57,30 +57,30 @@ public class UserPermissionChanger { } } - private boolean isImplicitlyAlreadyDone(UserPermissionChange change) { + private static boolean isImplicitlyAlreadyDone(UserPermissionChange change) { return change.getProjectId() .map(projectId -> isImplicitlyAlreadyDone(projectId, change)) .orElse(false); } - private boolean isImplicitlyAlreadyDone(ProjectId projectId, UserPermissionChange change) { + private static boolean isImplicitlyAlreadyDone(ProjectId projectId, UserPermissionChange change) { return isAttemptToAddPublicPermissionToPublicComponent(change, projectId); } - private boolean isAttemptToAddPublicPermissionToPublicComponent(UserPermissionChange change, ProjectId projectId) { + private static boolean isAttemptToAddPublicPermissionToPublicComponent(UserPermissionChange change, ProjectId projectId) { return !projectId.isPrivate() && change.getOperation() == ADD && PUBLIC_PERMISSIONS.contains(change.getPermission()); } - private void ensureConsistencyWithVisibility(UserPermissionChange change) { + private static void ensureConsistencyWithVisibility(UserPermissionChange change) { change.getProjectId() .ifPresent(projectId -> checkRequest( !isAttemptToRemovePublicPermissionFromPublicComponent(change, projectId), "Permission %s can't be removed from a public component", change.getPermission())); } - private boolean isAttemptToRemovePublicPermissionFromPublicComponent(UserPermissionChange change, ProjectId projectId) { + private static boolean isAttemptToRemovePublicPermissionFromPublicComponent(UserPermissionChange change, ProjectId projectId) { return !projectId.isPrivate() && change.getOperation() == REMOVE && PUBLIC_PERMISSIONS.contains(change.getPermission()); diff --git a/server/sonar-server/src/main/java/org/sonar/server/project/ws/UpdateVisibilityAction.java b/server/sonar-server/src/main/java/org/sonar/server/project/ws/UpdateVisibilityAction.java index 473c393e597..822f233bf77 100644 --- a/server/sonar-server/src/main/java/org/sonar/server/project/ws/UpdateVisibilityAction.java +++ b/server/sonar-server/src/main/java/org/sonar/server/project/ws/UpdateVisibilityAction.java @@ -37,7 +37,6 @@ import org.sonar.db.permission.UserPermissionDto; import org.sonar.server.component.ComponentFinder; import org.sonar.server.es.ProjectIndexer; import org.sonar.server.es.ProjectIndexers; -import org.sonar.server.permission.PermissionService; import org.sonar.server.project.Visibility; import org.sonar.server.user.UserSession; import org.sonarqube.ws.client.project.ProjectsWsParameters; @@ -58,16 +57,14 @@ public class UpdateVisibilityAction implements ProjectsWsAction { private final UserSession userSession; private final ProjectIndexers projectIndexers; private final ProjectsWsSupport projectsWsSupport; - private final PermissionService permissionService; public UpdateVisibilityAction(DbClient dbClient, ComponentFinder componentFinder, UserSession userSession, - ProjectIndexers projectIndexers, ProjectsWsSupport projectsWsSupport, PermissionService permissionService) { + ProjectIndexers projectIndexers, ProjectsWsSupport projectsWsSupport) { this.dbClient = dbClient; this.componentFinder = componentFinder; this.userSession = userSession; this.projectIndexers = projectIndexers; this.projectsWsSupport = projectsWsSupport; - this.permissionService = permissionService; } public void define(WebService.NewController context) { diff --git a/server/sonar-server/src/test/java/org/sonar/server/permission/PermissionTemplateServiceTest.java b/server/sonar-server/src/test/java/org/sonar/server/permission/PermissionTemplateServiceTest.java index 43a91834f33..d0006042b95 100644 --- a/server/sonar-server/src/test/java/org/sonar/server/permission/PermissionTemplateServiceTest.java +++ b/server/sonar-server/src/test/java/org/sonar/server/permission/PermissionTemplateServiceTest.java @@ -66,7 +66,7 @@ public class PermissionTemplateServiceTest { private DbSession session = dbTester.getSession(); private ProjectIndexers projectIndexers = new TestProjectIndexers(); - private PermissionTemplateService underTest = new PermissionTemplateService(dbTester.getDbClient(), projectIndexers, userSession, defaultTemplatesResolver, permissionService); + private PermissionTemplateService underTest = new PermissionTemplateService(dbTester.getDbClient(), projectIndexers, userSession, defaultTemplatesResolver); @Test public void apply_does_not_insert_permission_to_group_AnyOne_when_applying_template_on_private_project() { diff --git a/server/sonar-server/src/test/java/org/sonar/server/permission/ws/template/ApplyTemplateActionTest.java b/server/sonar-server/src/test/java/org/sonar/server/permission/ws/template/ApplyTemplateActionTest.java index 3c8347fad14..9ef1ecd5b53 100644 --- a/server/sonar-server/src/test/java/org/sonar/server/permission/ws/template/ApplyTemplateActionTest.java +++ b/server/sonar-server/src/test/java/org/sonar/server/permission/ws/template/ApplyTemplateActionTest.java @@ -24,11 +24,8 @@ import javax.annotation.Nullable; import org.junit.Before; import org.junit.Rule; import org.junit.Test; -import org.sonar.api.resources.Qualifiers; -import org.sonar.api.resources.ResourceTypes; import org.sonar.api.web.UserRole; import org.sonar.db.component.ComponentDto; -import org.sonar.db.component.ResourceTypesRule; import org.sonar.db.permission.PermissionQuery; import org.sonar.db.permission.template.PermissionTemplateDto; import org.sonar.db.user.GroupDto; @@ -37,8 +34,6 @@ import org.sonar.server.es.TestProjectIndexers; import org.sonar.server.exceptions.BadRequestException; import org.sonar.server.exceptions.ForbiddenException; import org.sonar.server.exceptions.NotFoundException; -import org.sonar.server.permission.PermissionService; -import org.sonar.server.permission.PermissionServiceImpl; import org.sonar.server.permission.PermissionTemplateService; import org.sonar.server.permission.ws.BasePermissionWsTest; import org.sonar.server.ws.TestRequest; @@ -63,11 +58,8 @@ public class ApplyTemplateActionTest extends BasePermissionWsTest PUBLIC_PERMISSIONS = Collections.unmodifiableSet(new HashSet<>(Arrays.asList(UserRole.USER, UserRole.CODEVIEWER))); /** * @deprecated use the constant USER since 1.12. @@ -58,10 +63,4 @@ public @interface UserRole { String[] value() default {}; - /** - * Permissions which are implicitly available for any user, any group and to group "AnyOne" on public components. - * @since 7.5 - */ - Set PUBLIC_PERMISSIONS = Collections.unmodifiableSet(new HashSet<>(Arrays.asList(UserRole.USER, UserRole.CODEVIEWER))); - } -- 2.39.5