From c711b44ab5873bd602605552b05b04a1a26b255c Mon Sep 17 00:00:00 2001 From: Antoine Vigneau Date: Thu, 9 Mar 2023 18:43:53 +0100 Subject: [PATCH] SONAR-18655 Prevent account creation at login when the instance is managed --- .../authentication/UserRegistrarImpl.java | 18 +++++++- .../authentication/UserRegistration.java | 15 +++++-- .../HttpHeadersAuthenticationTest.java | 3 +- .../authentication/UserRegistrarImplTest.java | 43 ++++++++++++++++++- 4 files changed, 72 insertions(+), 7 deletions(-) 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 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); + } + } -- 2.39.5