From: Antoine Vigneau <antoine.vigneau@sonarsource.com>
Date: Thu, 9 Mar 2023 17:43:53 +0000 (+0100)
Subject: SONAR-18655 Prevent account creation at login when the instance is managed
X-Git-Tag: 10.0.0.68432~98
X-Git-Url: https://source.dussan.org/?a=commitdiff_plain;h=c711b44ab5873bd602605552b05b04a1a26b255c;p=sonarqube.git

SONAR-18655 Prevent account creation at login when the instance is managed
---

diff --git a/server/sonar-webserver-auth/src/main/java/org/sonar/server/authentication/UserRegistrarImpl.java b/server/sonar-webserver-auth/src/main/java/org/sonar/server/authentication/UserRegistrarImpl.java
index 9ecd8f38437..8ac82b36844 100644
--- a/server/sonar-webserver-auth/src/main/java/org/sonar/server/authentication/UserRegistrarImpl.java
+++ b/server/sonar-webserver-auth/src/main/java/org/sonar/server/authentication/UserRegistrarImpl.java
@@ -42,6 +42,7 @@ import org.sonar.db.user.UserDto;
 import org.sonar.db.user.UserGroupDto;
 import org.sonar.server.authentication.event.AuthenticationEvent.Source;
 import org.sonar.server.authentication.event.AuthenticationException;
+import org.sonar.server.management.ManagedInstanceService;
 import org.sonar.server.user.ExternalIdentity;
 import org.sonar.server.user.NewUser;
 import org.sonar.server.user.UpdateUser;
@@ -64,11 +65,14 @@ public class UserRegistrarImpl implements UserRegistrar {
   private final DbClient dbClient;
   private final UserUpdater userUpdater;
   private final DefaultGroupFinder defaultGroupFinder;
+  private final ManagedInstanceService managedInstanceService;
 
-  public UserRegistrarImpl(DbClient dbClient, UserUpdater userUpdater, DefaultGroupFinder defaultGroupFinder) {
+  public UserRegistrarImpl(DbClient dbClient, UserUpdater userUpdater, DefaultGroupFinder defaultGroupFinder,
+    ManagedInstanceService managedInstanceService) {
     this.dbClient = dbClient;
     this.userUpdater = userUpdater;
     this.defaultGroupFinder = defaultGroupFinder;
+    this.managedInstanceService = managedInstanceService;
   }
 
   @Override
@@ -159,6 +163,7 @@ public class UserRegistrarImpl implements UserRegistrar {
   }
 
   private UserDto registerNewUser(DbSession dbSession, @Nullable UserDto disabledUser, UserRegistration authenticatorParameters) {
+    blockUnmanagedUserCreationOnManagedInstance(authenticatorParameters);
     Optional<UserDto> otherUserToIndex = detectEmailUpdate(dbSession, authenticatorParameters, disabledUser != null ? disabledUser.getUuid() : null);
     NewUser newUser = createNewUser(authenticatorParameters);
     if (disabledUser == null) {
@@ -167,6 +172,17 @@ public class UserRegistrarImpl implements UserRegistrar {
     return userUpdater.reactivateAndCommit(dbSession, disabledUser, newUser, beforeCommit(dbSession, authenticatorParameters), toArray(otherUserToIndex));
   }
 
+  private void blockUnmanagedUserCreationOnManagedInstance(UserRegistration userRegistration) {
+    if (managedInstanceService.isInstanceExternallyManaged() && !userRegistration.managed()) {
+      throw AuthenticationException.newBuilder()
+        .setMessage("No account found for this user. As the instance is managed, make sure to provision the user from your IDP.")
+        .setPublicMessage("You have no account on SonarQube. Please make sure with your administrator that your account is provisioned.")
+        .setLogin(userRegistration.getUserIdentity().getProviderLogin())
+        .setSource(userRegistration.getSource())
+        .build();
+    }
+  }
+
   private UserDto registerExistingUser(DbSession dbSession, UserDto userDto, UserRegistration authenticatorParameters) {
     UpdateUser update = new UpdateUser()
       .setEmail(authenticatorParameters.getUserIdentity().getEmail())
diff --git a/server/sonar-webserver-auth/src/main/java/org/sonar/server/authentication/UserRegistration.java b/server/sonar-webserver-auth/src/main/java/org/sonar/server/authentication/UserRegistration.java
index a3bbc60bf16..576125644ee 100644
--- a/server/sonar-webserver-auth/src/main/java/org/sonar/server/authentication/UserRegistration.java
+++ b/server/sonar-webserver-auth/src/main/java/org/sonar/server/authentication/UserRegistration.java
@@ -19,9 +19,6 @@
  */
 package org.sonar.server.authentication;
 
-import java.util.Set;
-import javax.annotation.CheckForNull;
-import javax.annotation.Nullable;
 import org.sonar.api.server.authentication.IdentityProvider;
 import org.sonar.api.server.authentication.UserIdentity;
 import org.sonar.server.authentication.event.AuthenticationEvent;
@@ -33,11 +30,13 @@ public class UserRegistration {
   private final UserIdentity userIdentity;
   private final IdentityProvider provider;
   private final AuthenticationEvent.Source source;
+  private final boolean managed;
 
   UserRegistration(Builder builder) {
     this.userIdentity = builder.userIdentity;
     this.provider = builder.provider;
     this.source = builder.source;
+    this.managed = builder.managed;
   }
 
   public UserIdentity getUserIdentity() {
@@ -52,6 +51,10 @@ public class UserRegistration {
     return source;
   }
 
+  public boolean managed() {
+    return managed;
+  }
+
   public static UserRegistration.Builder builder() {
     return new Builder();
   }
@@ -60,6 +63,7 @@ public class UserRegistration {
     private UserIdentity userIdentity;
     private IdentityProvider provider;
     private AuthenticationEvent.Source source;
+    private boolean managed = false;
 
     public Builder setUserIdentity(UserIdentity userIdentity) {
       this.userIdentity = userIdentity;
@@ -76,6 +80,11 @@ public class UserRegistration {
       return this;
     }
 
+    public Builder setManaged(boolean managed) {
+      this.managed = managed;
+      return this;
+    }
+
     public UserRegistration build() {
       requireNonNull(userIdentity, "userIdentity must be set");
       requireNonNull(provider, "identityProvider must be set");
diff --git a/server/sonar-webserver-auth/src/test/java/org/sonar/server/authentication/HttpHeadersAuthenticationTest.java b/server/sonar-webserver-auth/src/test/java/org/sonar/server/authentication/HttpHeadersAuthenticationTest.java
index 056092f1be3..4e8a3d82fac 100644
--- a/server/sonar-webserver-auth/src/test/java/org/sonar/server/authentication/HttpHeadersAuthenticationTest.java
+++ b/server/sonar-webserver-auth/src/test/java/org/sonar/server/authentication/HttpHeadersAuthenticationTest.java
@@ -41,6 +41,7 @@ import org.sonar.server.authentication.event.AuthenticationEvent;
 import org.sonar.server.authentication.event.AuthenticationEvent.Source;
 import org.sonar.server.authentication.event.AuthenticationException;
 import org.sonar.server.es.EsTester;
+import org.sonar.server.management.ManagedInstanceService;
 import org.sonar.server.user.NewUserNotifier;
 import org.sonar.server.user.UserUpdater;
 import org.sonar.server.user.index.UserIndexer;
@@ -97,7 +98,7 @@ public class HttpHeadersAuthenticationTest {
   private final DefaultGroupFinder defaultGroupFinder = new DefaultGroupFinder(db.getDbClient());
   private final UserRegistrarImpl userIdentityAuthenticator = new UserRegistrarImpl(db.getDbClient(),
     new UserUpdater(mock(NewUserNotifier.class), db.getDbClient(), userIndexer, defaultGroupFinder, settings.asConfig(), mock(AuditPersister.class), localAuthentication),
-    defaultGroupFinder);
+    defaultGroupFinder, mock(ManagedInstanceService.class));
   private final HttpServletResponse response = mock(HttpServletResponse.class);
   private final JwtHttpHandler jwtHttpHandler = mock(JwtHttpHandler.class);
   private final AuthenticationEvent authenticationEvent = mock(AuthenticationEvent.class);
diff --git a/server/sonar-webserver-auth/src/test/java/org/sonar/server/authentication/UserRegistrarImplTest.java b/server/sonar-webserver-auth/src/test/java/org/sonar/server/authentication/UserRegistrarImplTest.java
index 920f3595997..070f9a6b330 100644
--- a/server/sonar-webserver-auth/src/test/java/org/sonar/server/authentication/UserRegistrarImplTest.java
+++ b/server/sonar-webserver-auth/src/test/java/org/sonar/server/authentication/UserRegistrarImplTest.java
@@ -39,6 +39,7 @@ import org.sonar.server.authentication.event.AuthenticationEvent;
 import org.sonar.server.authentication.event.AuthenticationEvent.Source;
 import org.sonar.server.authentication.event.AuthenticationException;
 import org.sonar.server.es.EsTester;
+import org.sonar.server.management.ManagedInstanceService;
 import org.sonar.server.user.NewUserNotifier;
 import org.sonar.server.user.UserUpdater;
 import org.sonar.server.user.index.UserIndexer;
@@ -51,6 +52,7 @@ import static org.mockito.ArgumentMatchers.any;
 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.server.authentication.UserRegistrarImpl.GITHUB_PROVIDER;
 import static org.sonar.server.authentication.UserRegistrarImpl.GITLAB_PROVIDER;
 import static org.sonar.server.authentication.UserRegistrarImpl.LDAP_PROVIDER_PREFIX;
@@ -88,9 +90,10 @@ public class UserRegistrarImplTest {
   private final CredentialsLocalAuthentication localAuthentication = new CredentialsLocalAuthentication(db.getDbClient(), settings.asConfig());
   private final DefaultGroupFinder groupFinder = new DefaultGroupFinder(db.getDbClient());
   private final AuditPersister auditPersister = mock(AuditPersister.class);
+  private final ManagedInstanceService managedInstanceService = mock(ManagedInstanceService.class);
   private final UserUpdater userUpdater = new UserUpdater(mock(NewUserNotifier.class), db.getDbClient(), userIndexer, groupFinder, settings.asConfig(), auditPersister, localAuthentication);
 
-  private final UserRegistrarImpl underTest = new UserRegistrarImpl(db.getDbClient(), userUpdater, groupFinder);
+  private final UserRegistrarImpl underTest = new UserRegistrarImpl(db.getDbClient(), userUpdater, groupFinder, managedInstanceService);
   private GroupDto defaultGroup;
 
   @Before
@@ -234,6 +237,37 @@ public class UserRegistrarImplTest {
       .hasFieldOrPropertyWithValue("publicMessage", "'github' users are not allowed to sign up");
   }
 
+  @Test
+  public void register_whenNewUnmanagedUserAndManagedInstance_shouldThrow() {
+    when(managedInstanceService.isInstanceExternallyManaged()).thenReturn(true);
+
+    TestIdentityProvider identityProvider = composeIdentityProvider("saml", "Okta", true, true);
+    Source source = realm(AuthenticationEvent.Method.FORM, identityProvider.getName());
+    UserRegistration registration = composeUserRegistration(USER_IDENTITY, identityProvider, source);
+
+    assertThatThrownBy(() -> underTest.register(registration))
+      .isInstanceOf(AuthenticationException.class)
+      .hasMessage("No account found for this user. As the instance is managed, make sure to provision the user from your IDP.")
+      .hasFieldOrPropertyWithValue("source", source)
+      .hasFieldOrPropertyWithValue("login", USER_IDENTITY.getProviderLogin());
+  }
+
+  @Test
+  public void register_whenNewManagedUserAndManagedInstance_shouldCreateAndReturnUser() {
+    when(managedInstanceService.isInstanceExternallyManaged()).thenReturn(true);
+
+    TestIdentityProvider identityProvider = composeIdentityProvider("saml", "Okta", true, true);
+    Source source = realm(AuthenticationEvent.Method.FORM, identityProvider.getName());
+    UserRegistration registration = composeUserRegistration(USER_IDENTITY, identityProvider, source, true);
+
+    UserDto userDto = underTest.register(registration);
+
+    assertThat(userDto.getExternalId()).isEqualTo(USER_IDENTITY.getProviderId());
+    assertThat(userDto.getExternalLogin()).isEqualTo(USER_IDENTITY.getProviderLogin());
+    assertThat(userDto.getName()).isEqualTo(USER_IDENTITY.getName());
+    assertThat(userDto.getEmail()).isEqualTo(USER_IDENTITY.getEmail());
+  }
+
   @Test
   public void authenticate_existing_user_doesnt_change_group_membership() {
     UserDto user = db.users().insertUser(u -> u.setExternalIdentityProvider(GH_IDENTITY_PROVIDER.getKey()));
@@ -594,12 +628,17 @@ public class UserRegistrarImplTest {
       .setAllowsUsersToSignUp(allowsUsersToSignUp);
   }
 
-  private static UserRegistration composeUserRegistration(UserIdentity userIdentity, IdentityProvider identityProvider, Source source) {
+  private static UserRegistration composeUserRegistration(UserIdentity userIdentity, IdentityProvider identityProvider, Source source, Boolean managed) {
     return UserRegistration.builder()
       .setUserIdentity(userIdentity)
       .setProvider(identityProvider)
       .setSource(source)
+      .setManaged(managed)
       .build();
   }
 
+  private static UserRegistration composeUserRegistration(UserIdentity userIdentity, IdentityProvider identityProvider, Source source) {
+    return composeUserRegistration(userIdentity, identityProvider, source, false);
+  }
+
 }