SONAR-19515 Let the user remove a non-local user, in case the user is provided by...

diff --git a/server/sonar-server-common/src/main/java/org/sonar/server/management/DelegatingManagedInstanceService.java b/server/sonar-server-common/src/main/java/org/sonar/server/management/DelegatingManagedInstanceService.java
index f4db6c5516aa68fcb6f243f8c0ba24d1e84ffb09..98ca712eb608eb605081ff52920ffd333c745b73 100644 (file)
--- a/server/sonar-server-common/src/main/java/org/sonar/server/management/DelegatingManagedInstanceService.java
+++ b/server/sonar-server-common/src/main/java/org/sonar/server/management/DelegatingManagedInstanceService.java
@@ -82,6 +82,13 @@ public class DelegatingManagedInstanceService implements ManagedInstanceService
       .orElseThrow(() -> NOT_MANAGED_INSTANCE_EXCEPTION);
   }
 
+  @Override
+  public boolean isUserManaged(DbSession dbSession, String login) {
+    return findManagedInstanceService()
+      .map(managedInstanceService -> managedInstanceService.isUserManaged(dbSession, login))
+      .orElse(false);
+  }
+
   private Optional<ManagedInstanceService> findManagedInstanceService() {
     Set<ManagedInstanceService> managedInstanceServices = delegates.stream()
       .filter(ManagedInstanceService::isInstanceExternallyManaged)

diff --git a/server/sonar-server-common/src/main/java/org/sonar/server/management/ManagedInstanceService.java b/server/sonar-server-common/src/main/java/org/sonar/server/management/ManagedInstanceService.java
index 2580f55dc4d95f16c3fa3dcefee8bbe15a97f0ab..dcce08c7d0a18600d4f2366a0672db628d05a2a1 100644 (file)
--- a/server/sonar-server-common/src/main/java/org/sonar/server/management/ManagedInstanceService.java
+++ b/server/sonar-server-common/src/main/java/org/sonar/server/management/ManagedInstanceService.java
@@ -38,4 +38,6 @@ public interface ManagedInstanceService {
   String getManagedUsersSqlFilter(boolean filterByManaged);
 
   String getManagedGroupsSqlFilter(boolean filterByManaged);
+
+  boolean isUserManaged(DbSession dbSession, String login);
 }

diff --git a/server/sonar-server-common/src/test/java/org/sonar/server/management/DelegatingManagedInstanceServiceTest.java b/server/sonar-server-common/src/test/java/org/sonar/server/management/DelegatingManagedInstanceServiceTest.java
index 5f3b35dac9397ef81a8313e09c14d3dccb806ff2..f2ecf25fd4fc5a327e63c8a25936b8d49ede1eac 100644 (file)
--- a/server/sonar-server-common/src/test/java/org/sonar/server/management/DelegatingManagedInstanceServiceTest.java
+++ b/server/sonar-server-common/src/test/java/org/sonar/server/management/DelegatingManagedInstanceServiceTest.java
@@ -111,6 +111,20 @@ public class DelegatingManagedInstanceServiceTest {
     assertThat(groupUuidToManaged).containsExactlyInAnyOrderEntriesOf(Map.of("a", false, "b", false));
   }
 
+  @Test
+  public void isUserManaged_delegatesToRightService_andPropagateAnswer() {
+    DelegatingManagedInstanceService managedInstanceService = new DelegatingManagedInstanceService(Set.of(new NeverManagedInstanceService(), new AlwaysManagedInstanceService()));
+
+    assertThat(managedInstanceService.isUserManaged(dbSession, "login")).isTrue();
+  }
+
+  @Test
+  public void isUserManaged_whenNoDelegates_returnsFalse() {
+    DelegatingManagedInstanceService managedInstanceService = new DelegatingManagedInstanceService(Set.of());
+
+    assertThat(managedInstanceService.isUserManaged(dbSession, "login")).isFalse();
+  }
+
   @Test
   public void getGroupUuidToManaged_delegatesToRightService_andPropagateAnswer() {
     Set<String> groupUuids = Set.of("a", "b");
@@ -217,6 +231,11 @@ public class DelegatingManagedInstanceServiceTest {
     public String getManagedGroupsSqlFilter(boolean filterByManaged) {
       return null;
     }
+
+    @Override
+    public boolean isUserManaged(DbSession dbSession, String login) {
+      return false;
+    }
   }
 
   private static class AlwaysManagedInstanceService implements ManagedInstanceService {
@@ -250,6 +269,11 @@ public class DelegatingManagedInstanceServiceTest {
     public String getManagedGroupsSqlFilter(boolean filterByManaged) {
       return "any filter";
     }
+
+    @Override
+    public boolean isUserManaged(DbSession dbSession, String login) {
+      return true;
+    }
   }
 
 }

diff --git a/server/sonar-webserver-webapi/src/it/java/org/sonar/server/user/ws/DeactivateActionIT.java b/server/sonar-webserver-webapi/src/it/java/org/sonar/server/user/ws/DeactivateActionIT.java
index 9832254f153f99f1845275e339d1d883a61c96f0..a715f4cea4744d1e885fadfa5ee5249163c73b14 100644 (file)
--- a/server/sonar-webserver-webapi/src/it/java/org/sonar/server/user/ws/DeactivateActionIT.java
+++ b/server/sonar-webserver-webapi/src/it/java/org/sonar/server/user/ws/DeactivateActionIT.java
@@ -48,7 +48,7 @@ import org.sonar.server.exceptions.BadRequestException;
 import org.sonar.server.exceptions.ForbiddenException;
 import org.sonar.server.exceptions.NotFoundException;
 import org.sonar.server.exceptions.UnauthorizedException;
-import org.sonar.server.management.ManagedInstanceChecker;
+import org.sonar.server.management.ManagedInstanceService;
 import org.sonar.server.tester.UserSessionRule;
 import org.sonar.server.user.ExternalIdentity;
 import org.sonar.server.ws.TestRequest;
@@ -57,11 +57,12 @@ import org.sonar.server.ws.WsActionTester;
 
 import static java.util.Collections.singletonList;
 import static org.assertj.core.api.Assertions.assertThat;
+import static org.assertj.core.api.Assertions.assertThatExceptionOfType;
 import static org.assertj.core.api.Assertions.assertThatThrownBy;
-import static org.mockito.Mockito.doThrow;
+import static org.mockito.ArgumentMatchers.any;
+import static org.mockito.ArgumentMatchers.eq;
 import static org.mockito.Mockito.mock;
-import static org.mockito.Mockito.never;
-import static org.mockito.Mockito.verify;
+import static org.mockito.Mockito.when;
 import static org.sonar.db.property.PropertyTesting.newUserPropertyDto;
 import static org.sonar.test.JsonAssert.assertJson;
 
@@ -78,8 +79,10 @@ public class DeactivateActionIT {
   private final DbSession dbSession = db.getSession();
   private final UserAnonymizer userAnonymizer = new UserAnonymizer(db.getDbClient(), () -> "anonymized");
   private final UserDeactivator userDeactivator = new UserDeactivator(dbClient, userAnonymizer);
-  private final ManagedInstanceChecker managedInstanceChecker = mock(ManagedInstanceChecker.class);
-  private final WsActionTester ws = new WsActionTester(new DeactivateAction(dbClient, userSession, new UserJsonWriter(userSession), userDeactivator, managedInstanceChecker));
+  private final ManagedInstanceService managedInstanceService = mock(ManagedInstanceService.class);
+
+  private final WsActionTester ws = new WsActionTester(new DeactivateAction(dbClient, userSession, new UserJsonWriter(userSession), userDeactivator, managedInstanceService
+  ));
 
   @Test
   public void deactivate_user_and_delete_their_related_data() {
@@ -432,15 +435,14 @@ public class DeactivateActionIT {
 
   @Test
   public void handle_whenUserManagedAndInstanceManaged_shouldThrowBadRequestException() {
-    BadRequestException badRequestException = BadRequestException.create("message");
-    doThrow(badRequestException).when(managedInstanceChecker).throwIfInstanceIsManaged();
-
     createAdminUser();
     logInAsSystemAdministrator();
-    UserDto user = db.users().insertUser(u -> u.setLocal(false));
+    UserDto user = db.users().insertUser();
+    when(managedInstanceService.isUserManaged(any(), eq(user.getLogin()))).thenReturn(true);
 
-    assertThatThrownBy(() -> deactivate(user.getLogin()))
-      .isEqualTo(badRequestException);
+    assertThatExceptionOfType(BadRequestException.class)
+      .isThrownBy(() -> deactivate(user.getLogin()))
+      .withMessage("Operation not allowed when the instance is externally managed.");
   }
 
   @Test
@@ -451,7 +453,6 @@ public class DeactivateActionIT {
     assertThatThrownBy(() -> deactivate(login))
       .isInstanceOf(UnauthorizedException.class)
       .hasMessage("Authentication is required");
-    verify(managedInstanceChecker, never()).throwIfInstanceIsManaged();
   }
 
   private void logInAsSystemAdministrator() {

diff --git a/server/sonar-webserver-webapi/src/main/java/org/sonar/server/user/ws/DeactivateAction.java b/server/sonar-webserver-webapi/src/main/java/org/sonar/server/user/ws/DeactivateAction.java
index b5426ae77c4060c78a8c31a684d210e25b0e10e3..7c31f90118715f206ed35e05b97fe228759ae857 100644 (file)
--- a/server/sonar-webserver-webapi/src/main/java/org/sonar/server/user/ws/DeactivateAction.java
+++ b/server/sonar-webserver-webapi/src/main/java/org/sonar/server/user/ws/DeactivateAction.java
@@ -29,7 +29,7 @@ import org.sonar.api.utils.text.JsonWriter;
 import org.sonar.db.DbClient;
 import org.sonar.db.DbSession;
 import org.sonar.db.user.UserDto;
-import org.sonar.server.management.ManagedInstanceChecker;
+import org.sonar.server.management.ManagedInstanceService;
 import org.sonar.server.user.UserSession;
 
 import static java.util.Collections.singletonList;
@@ -45,15 +45,15 @@ public class DeactivateAction implements UsersWsAction {
   private final UserSession userSession;
   private final UserJsonWriter userWriter;
   private final UserDeactivator userDeactivator;
-  private final ManagedInstanceChecker managedInstanceChecker;
+  private final ManagedInstanceService managedInstanceService;
 
   public DeactivateAction(DbClient dbClient, UserSession userSession, UserJsonWriter userWriter,
-    UserDeactivator userDeactivator, ManagedInstanceChecker managedInstanceChecker) {
+    UserDeactivator userDeactivator, ManagedInstanceService managedInstanceService) {
     this.dbClient = dbClient;
     this.userSession = userSession;
     this.userWriter = userWriter;
     this.userDeactivator = userDeactivator;
-    this.managedInstanceChecker = managedInstanceChecker;
+    this.managedInstanceService = managedInstanceService;
   }
 
   @Override
@@ -94,10 +94,7 @@ public class DeactivateAction implements UsersWsAction {
   }
 
   private void preventManagedUserDeactivationIfManagedInstance(DbSession dbSession, String login) {
-    UserDto userDto = dbClient.userDao().selectByLogin(dbSession, login);
-    if (userDto != null && !userDto.isLocal()) {
-      managedInstanceChecker.throwIfInstanceIsManaged();
-    }
+    checkRequest(!managedInstanceService.isUserManaged(dbSession, login), "Operation not allowed when the instance is externally managed.");
   }
 
   private void writeResponse(Response response, String login) {