]> source.dussan.org Git - sonarqube.git/commitdiff
SONARCLOUD-213 fix Quality flaws
authorSimon Brandhof <simon.brandhof@sonarsource.com>
Tue, 11 Dec 2018 10:51:46 +0000 (11:51 +0100)
committerSonarTech <sonartech@sonarsource.com>
Wed, 12 Dec 2018 19:21:03 +0000 (20:21 +0100)
server/sonar-server/src/main/java/org/sonar/server/authentication/OAuth2ContextFactory.java
server/sonar-server/src/main/java/org/sonar/server/authentication/RequestAuthenticator.java
server/sonar-server/src/main/java/org/sonar/server/permission/GroupPermissionChanger.java
server/sonar-server/src/main/java/org/sonar/server/permission/PermissionTemplateService.java
server/sonar-server/src/main/java/org/sonar/server/permission/UserPermissionChanger.java
server/sonar-server/src/main/java/org/sonar/server/project/ws/UpdateVisibilityAction.java
server/sonar-server/src/test/java/org/sonar/server/permission/PermissionTemplateServiceTest.java
server/sonar-server/src/test/java/org/sonar/server/permission/ws/template/ApplyTemplateActionTest.java
server/sonar-server/src/test/java/org/sonar/server/permission/ws/template/BulkApplyTemplateActionTest.java
server/sonar-server/src/test/java/org/sonar/server/project/ws/UpdateVisibilityActionTest.java
sonar-plugin-api/src/main/java/org/sonar/api/web/UserRole.java

index 1fd9118349f549035b180448ad547060f1a4bc08..11b000e633f25a66d46079af3008db1e559a3b44 100644 (file)
@@ -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;
index 5f74851796a6f845ea890a746f9eccc7a0177c45..a70b4e74cd102a5762d012b890483c70b9c4fc34 100644 (file)
@@ -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);
 
 }
index c5c7b0d189cb8965fe925410bc4ff825f106a860..f3396095398ce513b47eb22d98bdfd6eeebcee55 100644 (file)
@@ -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());
index 7824a71b2ccd015d44dc433f581b29aeb7126ef6..4d5abb65a191b715e72a237ddee04ae5f0edfd6e 100644 (file)
@@ -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);
   }
 
index 45e7775f6f4db822fbf80582f4b2923188b80892..08d5ce882d6c2496d0a89a70651ad1216d88cee4 100644 (file)
@@ -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());
index 473c393e597de1116a76adf171bf11b5f45f6566..822f233bf77ac3b9b112dac6fc0acc1d1b93aaee 100644 (file)
@@ -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) {
index 43a91834f33ed76d4bf334e2d5fb2c693adb23f8..d0006042b952384a860970e208d685fe7bebd461 100644 (file)
@@ -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() {
index 3c8347fad14ad50d6da59db3aa6cbecf378877d0..9ef1ecd5b5378189defbf0da4089b9ac92b3f9c6 100644 (file)
@@ -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);
index 09579c8640d537e3944b1165b29142a0601e6df2..e7562ae571e8a26d155fbc5ad4697420ca8c76c0 100644 (file)
@@ -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());
   }
 
index bd71ba074d9dc01799c58b7580b1062a4d297525..b604a3d01b72e43577554313423742abdbd39886 100644 (file)
@@ -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();
index 5a17fee9c9d14a3f5f057ba939f4301726d86d30..5ee3fa243855d5d5a2abfed872db906f3bda206a 100644 (file)
@@ -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)));
-
 }