From: Simon Brandhof <simon.brandhof@sonarsource.com>
Date: Tue, 11 Dec 2018 10:51:46 +0000 (+0100)
Subject: SONARCLOUD-213 fix Quality flaws
X-Git-Tag: 7.5~18
X-Git-Url: https://source.dussan.org/?a=commitdiff_plain;h=c1caffa9fb8cabe9659d81372d82481edc00b545;p=sonarqube.git

SONARCLOUD-213 fix Quality flaws
---

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<ApplyTemplateA
   private PermissionTemplateDto template1;
   private PermissionTemplateDto template2;
 
-  private ResourceTypes resourceTypes = new ResourceTypesRule().setRootQualifiers(Qualifiers.PROJECT);
-  private PermissionService permissionService = new PermissionServiceImpl(resourceTypes);
-
   private PermissionTemplateService permissionTemplateService = new PermissionTemplateService(db.getDbClient(),
-     new TestProjectIndexers(), userSession, defaultTemplatesResolver, permissionService);
+     new TestProjectIndexers(), userSession, defaultTemplatesResolver);
 
   @Override
   protected ApplyTemplateAction buildWsAction() {
@@ -104,7 +96,7 @@ public class ApplyTemplateActionTest extends BasePermissionWsTest<ApplyTemplateA
   }
 
   @Test
-  public void apply_template_with_project_uuid() throws Exception {
+  public void apply_template_with_project_uuid() {
     loginAsAdmin(db.getDefaultOrganization());
 
     newRequest(template1.getUuid(), project.uuid(), null);
@@ -125,7 +117,7 @@ public class ApplyTemplateActionTest extends BasePermissionWsTest<ApplyTemplateA
   }
 
   @Test
-  public void apply_template_with_project_key() throws Exception {
+  public void apply_template_with_project_key() {
     loginAsAdmin(db.getDefaultOrganization());
 
     newRequest(template1.getUuid(), null, project.getDbKey());
@@ -134,7 +126,7 @@ public class ApplyTemplateActionTest extends BasePermissionWsTest<ApplyTemplateA
   }
 
   @Test
-  public void fail_when_unknown_template() throws Exception {
+  public void fail_when_unknown_template() {
     loginAsAdmin(db.getDefaultOrganization());
 
     expectedException.expect(NotFoundException.class);
@@ -144,7 +136,7 @@ public class ApplyTemplateActionTest extends BasePermissionWsTest<ApplyTemplateA
   }
 
   @Test
-  public void fail_when_unknown_project_uuid() throws Exception {
+  public void fail_when_unknown_project_uuid() {
     loginAsAdmin(db.getDefaultOrganization());
 
     expectedException.expect(NotFoundException.class);
@@ -154,7 +146,7 @@ public class ApplyTemplateActionTest extends BasePermissionWsTest<ApplyTemplateA
   }
 
   @Test
-  public void fail_when_unknown_project_key() throws Exception {
+  public void fail_when_unknown_project_key() {
     loginAsAdmin(db.getDefaultOrganization());
 
     expectedException.expect(NotFoundException.class);
@@ -164,7 +156,7 @@ public class ApplyTemplateActionTest extends BasePermissionWsTest<ApplyTemplateA
   }
 
   @Test
-  public void fail_when_template_is_not_provided() throws Exception {
+  public void fail_when_template_is_not_provided() {
     loginAsAdmin(db.getDefaultOrganization());
 
     expectedException.expect(BadRequestException.class);
@@ -173,7 +165,7 @@ public class ApplyTemplateActionTest extends BasePermissionWsTest<ApplyTemplateA
   }
 
   @Test
-  public void fail_when_project_uuid_and_key_not_provided() throws Exception {
+  public void fail_when_project_uuid_and_key_not_provided() {
     loginAsAdmin(db.getDefaultOrganization());
 
     expectedException.expect(BadRequestException.class);
@@ -183,7 +175,7 @@ public class ApplyTemplateActionTest extends BasePermissionWsTest<ApplyTemplateA
   }
 
   @Test
-  public void fail_when_not_admin_of_organization() throws Exception {
+  public void fail_when_not_admin_of_organization() {
     userSession.logIn().addPermission(ADMINISTER, "otherOrg");
 
     expectedException.expect(ForbiddenException.class);
diff --git a/server/sonar-server/src/test/java/org/sonar/server/permission/ws/template/BulkApplyTemplateActionTest.java b/server/sonar-server/src/test/java/org/sonar/server/permission/ws/template/BulkApplyTemplateActionTest.java
index 09579c8640d..e7562ae571e 100644
--- a/server/sonar-server/src/test/java/org/sonar/server/permission/ws/template/BulkApplyTemplateActionTest.java
+++ b/server/sonar-server/src/test/java/org/sonar/server/permission/ws/template/BulkApplyTemplateActionTest.java
@@ -25,12 +25,10 @@ import org.apache.commons.lang.StringUtils;
 import org.junit.Before;
 import org.junit.Test;
 import org.sonar.api.resources.Qualifiers;
-import org.sonar.api.resources.ResourceTypes;
 import org.sonar.api.server.ws.WebService.Param;
 import org.sonar.api.web.UserRole;
 import org.sonar.db.component.ComponentDto;
 import org.sonar.db.component.ComponentTesting;
-import org.sonar.db.component.ResourceTypesRule;
 import org.sonar.db.organization.OrganizationDto;
 import org.sonar.db.permission.PermissionQuery;
 import org.sonar.db.permission.template.PermissionTemplateDto;
@@ -41,8 +39,6 @@ import org.sonar.server.es.TestProjectIndexers;
 import org.sonar.server.exceptions.BadRequestException;
 import org.sonar.server.exceptions.NotFoundException;
 import org.sonar.server.l18n.I18nRule;
-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;
 
@@ -65,9 +61,6 @@ public class BulkApplyTemplateActionTest extends BasePermissionWsTest<BulkApplyT
   @org.junit.Rule
   public DefaultTemplatesResolverRule defaultTemplatesResolver = DefaultTemplatesResolverRule.withoutGovernance();
 
-  private ResourceTypes resourceTypes = new ResourceTypesRule().setRootQualifiers(Qualifiers.PROJECT);
-  private PermissionService permissionService = new PermissionServiceImpl(resourceTypes);
-
   private UserDto user1;
   private UserDto user2;
   private GroupDto group1;
@@ -80,7 +73,7 @@ public class BulkApplyTemplateActionTest extends BasePermissionWsTest<BulkApplyT
   @Override
   protected BulkApplyTemplateAction buildWsAction() {
     PermissionTemplateService permissionTemplateService = new PermissionTemplateService(db.getDbClient(),
-      projectIndexers, userSession, defaultTemplatesResolver, permissionService);
+      projectIndexers, userSession, defaultTemplatesResolver);
     return new BulkApplyTemplateAction(db.getDbClient(), userSession, permissionTemplateService, newPermissionWsSupport(), new I18nRule(), newRootResourceTypes());
   }
 
diff --git a/server/sonar-server/src/test/java/org/sonar/server/project/ws/UpdateVisibilityActionTest.java b/server/sonar-server/src/test/java/org/sonar/server/project/ws/UpdateVisibilityActionTest.java
index bd71ba074d9..b604a3d01b7 100644
--- a/server/sonar-server/src/test/java/org/sonar/server/project/ws/UpdateVisibilityActionTest.java
+++ b/server/sonar-server/src/test/java/org/sonar/server/project/ws/UpdateVisibilityActionTest.java
@@ -106,7 +106,7 @@ public class UpdateVisibilityActionTest {
   private BillingValidationsProxy billingValidations = mock(BillingValidationsProxy.class);
 
   private ProjectsWsSupport wsSupport = new ProjectsWsSupport(dbClient, TestDefaultOrganizationProvider.from(dbTester), billingValidations);
-  private UpdateVisibilityAction underTest = new UpdateVisibilityAction(dbClient, TestComponentFinder.from(dbTester), userSessionRule, projectIndexers, wsSupport, permissionService);
+  private UpdateVisibilityAction underTest = new UpdateVisibilityAction(dbClient, TestComponentFinder.from(dbTester), userSessionRule, projectIndexers, wsSupport);
   private WsActionTester ws = new WsActionTester(underTest);
 
   private final Random random = new Random();
diff --git a/sonar-plugin-api/src/main/java/org/sonar/api/web/UserRole.java b/sonar-plugin-api/src/main/java/org/sonar/api/web/UserRole.java
index 5a17fee9c9d..5ee3fa24385 100644
--- a/sonar-plugin-api/src/main/java/org/sonar/api/web/UserRole.java
+++ b/sonar-plugin-api/src/main/java/org/sonar/api/web/UserRole.java
@@ -34,6 +34,11 @@ import java.util.Set;
 @Retention(RetentionPolicy.RUNTIME)
 @Target(ElementType.TYPE)
 public @interface UserRole {
+  /**
+   * Permissions which are implicitly available for any user, any group and to group "AnyOne" on public components.
+   * @since 7.5
+   */
+  Set<String> 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<String> PUBLIC_PERMISSIONS = Collections.unmodifiableSet(new HashSet<>(Arrays.asList(UserRole.USER, UserRole.CODEVIEWER)));
-
 }