From d023db004a914ab40a9d7caaaa40d901433a018b Mon Sep 17 00:00:00 2001 From: Wojtek Wajerowicz <115081248+wojciech-wajerowicz-sonarsource@users.noreply.github.com> Date: Tue, 20 Dec 2022 14:24:36 +0100 Subject: [PATCH] SONAR-17798 respect sonar.authenticator.ignoreStartupFailure property in LdapCredentialsAuthentication --- .cirrus.yml | 7 +- server/sonar-auth-ldap/build.gradle | 1 + .../java/org/sonar/auth/ldap/LdapRealm.java | 73 ++++++++++++----- .../org/sonar/auth/ldap/KerberosTest.java | 18 +++-- .../org/sonar/auth/ldap/LdapRealmTest.java | 79 +++++++++++++++++-- .../LdapCredentialsAuthentication.java | 22 ++---- .../LdapCredentialsAuthenticationTest.java | 12 +-- 7 files changed, 152 insertions(+), 60 deletions(-) diff --git a/.cirrus.yml b/.cirrus.yml index 5465ccf0aba..7d78f3a435e 100644 --- a/.cirrus.yml +++ b/.cirrus.yml @@ -50,6 +50,11 @@ saml_nightly_task_template: &SAML_NIGHTLY_TASK_TEMPLATE $CIRRUS_BRANCH == $BRANCH_NIGHTLY || changesInclude('server/sonar-auth-saml/src/main/java/**/*.java', 'server/sonar-auth-saml/src/main/resources/**/*', 'server/sonar-db-dao/src/main/**/SAML*.java', 'private/it-core/src/test/java/org/sonarqube/tests/saml/*.java', 'server/sonar-webserver-webapi/src/main/java/org/sonar/server/saml/**/*.java') +ldap_nightly_task_template: &LDAP_NIGHTLY_TASK_TEMPLATE + only_if: >- + $CIRRUS_BRANCH == $BRANCH_NIGHTLY || + changesInclude('server/sonar-auth-ldap/src/main/java/**/*.java', 'server/sonar-webserver-auth/src/main/java/org/sonar/server/authentication/LdapCredentialsAuthentication.java', 'private/it-core/src/test/java/org/sonarqube/tests/ldap/*.java') + docker_build_container_template: &GKE_CONTAINER_TEMPLATE dockerfile: private/docker/Dockerfile-build builder_image_project: sonarqube-team @@ -528,7 +533,7 @@ qa_saml_task: qa_ldap_task: <<: *DEFAULT_TEMPLATE <<: *BUILD_DEPENDANT_TASK_TEMPLATE - <<: *NIGHTLY_TASK_TEMPLATE + <<: *LDAP_NIGHTLY_TASK_TEMPLATE <<: *JAR_CACHE_TEMPLATE <<: *GRADLE_CACHE_TEMPLATE gke_container: diff --git a/server/sonar-auth-ldap/build.gradle b/server/sonar-auth-ldap/build.gradle index 6ef30e47a62..ff21ffe4a90 100644 --- a/server/sonar-auth-ldap/build.gradle +++ b/server/sonar-auth-ldap/build.gradle @@ -11,6 +11,7 @@ dependencies { compileOnlyApi 'com.google.code.findbugs:jsr305' compileOnlyApi 'javax.servlet:javax.servlet-api' + compileOnlyApi project(':server:sonar-process') compileOnlyApi project(':sonar-core') testImplementation 'com.tngtech.java:junit-dataprovider' diff --git a/server/sonar-auth-ldap/src/main/java/org/sonar/auth/ldap/LdapRealm.java b/server/sonar-auth-ldap/src/main/java/org/sonar/auth/ldap/LdapRealm.java index 49dbc11b4c0..7550743cec5 100644 --- a/server/sonar-auth-ldap/src/main/java/org/sonar/auth/ldap/LdapRealm.java +++ b/server/sonar-auth-ldap/src/main/java/org/sonar/auth/ldap/LdapRealm.java @@ -20,9 +20,15 @@ package org.sonar.auth.ldap; import java.util.Map; +import javax.annotation.CheckForNull; +import org.sonar.api.config.Configuration; import org.sonar.api.server.ServerSide; +import org.sonar.api.utils.log.Logger; +import org.sonar.api.utils.log.Loggers; import static org.sonar.auth.ldap.LdapSettingsManager.DEFAULT_LDAP_SERVER_KEY; +import static org.sonar.process.ProcessProperties.Property.SONAR_AUTHENTICATOR_IGNORE_STARTUP_FAILURE; +import static org.sonar.process.ProcessProperties.Property.SONAR_SECURITY_REALM; /** * @author Evgeny Mandrikov @@ -32,44 +38,71 @@ public class LdapRealm { public static final String LDAP_SECURITY_REALM = "LDAP"; public static final String DEFAULT_LDAP_IDENTITY_PROVIDER_ID = LDAP_SECURITY_REALM + "_" + DEFAULT_LDAP_SERVER_KEY; - private LdapUsersProvider usersProvider; - private LdapGroupsProvider groupsProvider; - private LdapAuthenticator authenticator; - private final LdapSettingsManager settingsManager; + private static final Logger LOG = Loggers.get(LdapRealm.class); - public LdapRealm(LdapSettingsManager settingsManager) { - this.settingsManager = settingsManager; + private final boolean isLdapAuthActivated; + private final LdapUsersProvider usersProvider; + private final LdapGroupsProvider groupsProvider; + private final LdapAuthenticator authenticator; + + public LdapRealm(LdapSettingsManager settingsManager, Configuration configuration) { + String realmName = configuration.get(SONAR_SECURITY_REALM.getKey()).orElse(null); + this.isLdapAuthActivated = LDAP_SECURITY_REALM.equals(realmName); + boolean ignoreStartupFailure = configuration.getBoolean(SONAR_AUTHENTICATOR_IGNORE_STARTUP_FAILURE.getKey()).orElse(false); + if (!isLdapAuthActivated) { + this.usersProvider = null; + this.groupsProvider = null; + this.authenticator = null; + } else { + Map contextFactories = settingsManager.getContextFactories(); + Map userMappings = settingsManager.getUserMappings(); + this.usersProvider = new DefaultLdapUsersProvider(contextFactories, userMappings); + this.authenticator = new DefaultLdapAuthenticator(contextFactories, userMappings); + this.groupsProvider = createGroupsProvider(contextFactories, userMappings, settingsManager); + testConnections(contextFactories, ignoreStartupFailure); + } } - /** - * Initializes LDAP realm and tests connection. - * - * @throws LdapException if a NamingException was thrown during test - */ - public void init() { - Map contextFactories = settingsManager.getContextFactories(); - Map userMappings = settingsManager.getUserMappings(); - usersProvider = new DefaultLdapUsersProvider(contextFactories, userMappings); - authenticator = new DefaultLdapAuthenticator(contextFactories, userMappings); + private static LdapGroupsProvider createGroupsProvider(Map contextFactories, Map userMappings, + LdapSettingsManager settingsManager) { Map groupMappings = settingsManager.getGroupMappings(); if (!groupMappings.isEmpty()) { - groupsProvider = new DefaultLdapGroupsProvider(contextFactories, userMappings, groupMappings); + return new DefaultLdapGroupsProvider(contextFactories, userMappings, groupMappings); + } else { + return null; } - for (LdapContextFactory contextFactory : contextFactories.values()) { - contextFactory.testConnection(); + } + + private static void testConnections(Map contextFactories, boolean ignoreStartupFailure) { + try { + for (LdapContextFactory contextFactory : contextFactories.values()) { + contextFactory.testConnection(); + } + } catch (RuntimeException e) { + if (ignoreStartupFailure) { + LOG.error("IGNORED - LDAP realm failed to start: " + e.getMessage()); + } else { + throw new LdapException("LDAP realm failed to start: " + e.getMessage(), e); + } } } - public LdapAuthenticator doGetAuthenticator() { + @CheckForNull + public LdapAuthenticator getAuthenticator() { return authenticator; } + @CheckForNull public LdapUsersProvider getUsersProvider() { return usersProvider; } + @CheckForNull public LdapGroupsProvider getGroupsProvider() { return groupsProvider; } + public boolean isLdapAuthActivated() { + return isLdapAuthActivated; + } } diff --git a/server/sonar-auth-ldap/src/test/java/org/sonar/auth/ldap/KerberosTest.java b/server/sonar-auth-ldap/src/test/java/org/sonar/auth/ldap/KerberosTest.java index 7eba029eef5..2de46ca6e26 100644 --- a/server/sonar-auth-ldap/src/test/java/org/sonar/auth/ldap/KerberosTest.java +++ b/server/sonar-auth-ldap/src/test/java/org/sonar/auth/ldap/KerberosTest.java @@ -25,11 +25,13 @@ import org.junit.Before; import org.junit.ClassRule; import org.junit.Test; import org.mockito.Mockito; +import org.sonar.api.config.Configuration; import org.sonar.api.config.internal.MapSettings; import org.sonar.auth.ldap.server.LdapServer; import static org.assertj.core.api.Assertions.assertThat; import static org.assertj.core.api.Assertions.assertThatThrownBy; +import static org.sonar.process.ProcessProperties.Property.SONAR_SECURITY_REALM; public class KerberosTest { @@ -46,10 +48,8 @@ public class KerberosTest { @Before public void before() { MapSettings settings = configure(); - ldapRealm = new LdapRealm(new LdapSettingsManager(settings.asConfig())); - ldapRealm.init(); - - authenticator = ldapRealm.doGetAuthenticator(); + ldapRealm = new LdapRealm(new LdapSettingsManager(settings.asConfig()), settings.asConfig()); + authenticator = ldapRealm.getAuthenticator(); } @Test @@ -86,11 +86,12 @@ public class KerberosTest { public void wrong_bind_password() { MapSettings settings = configure() .setProperty("ldap.bindPassword", "wrong_bind_password"); - LdapRealm wrongPasswordRealm = new LdapRealm(new LdapSettingsManager(settings.asConfig())); - assertThatThrownBy(wrongPasswordRealm::init) + Configuration config = settings.asConfig(); + LdapSettingsManager settingsManager = new LdapSettingsManager(config); + assertThatThrownBy(() -> new LdapRealm(settingsManager, config)) .isInstanceOf(LdapException.class) - .hasMessage("Unable to open LDAP connection"); + .hasMessage("LDAP realm failed to start: Unable to open LDAP connection"); } @@ -102,7 +103,8 @@ public class KerberosTest { .setProperty("ldap.bindPassword", "bind_password") .setProperty("ldap.user.baseDn", "ou=users,dc=example,dc=org") .setProperty("ldap.group.baseDn", "ou=groups,dc=example,dc=org") - .setProperty("ldap.group.request", "(&(objectClass=groupOfUniqueNames)(uniqueMember={dn}))"); + .setProperty("ldap.group.request", "(&(objectClass=groupOfUniqueNames)(uniqueMember={dn}))") + .setProperty(SONAR_SECURITY_REALM.getKey(), LdapRealm.LDAP_SECURITY_REALM); } } diff --git a/server/sonar-auth-ldap/src/test/java/org/sonar/auth/ldap/LdapRealmTest.java b/server/sonar-auth-ldap/src/test/java/org/sonar/auth/ldap/LdapRealmTest.java index 8ebf44cdb09..8bc3d520637 100644 --- a/server/sonar-auth-ldap/src/test/java/org/sonar/auth/ldap/LdapRealmTest.java +++ b/server/sonar-auth-ldap/src/test/java/org/sonar/auth/ldap/LdapRealmTest.java @@ -23,11 +23,14 @@ import javax.servlet.http.HttpServletRequest; import org.junit.ClassRule; import org.junit.Test; import org.mockito.Mockito; +import org.sonar.api.config.Configuration; import org.sonar.api.config.internal.MapSettings; import org.sonar.auth.ldap.server.LdapServer; import static org.assertj.core.api.Assertions.assertThat; import static org.assertj.core.api.Assertions.assertThatThrownBy; +import static org.sonar.process.ProcessProperties.Property.SONAR_AUTHENTICATOR_IGNORE_STARTUP_FAILURE; +import static org.sonar.process.ProcessProperties.Property.SONAR_SECURITY_REALM; public class LdapRealmTest { @@ -38,10 +41,11 @@ public class LdapRealmTest { public void normal() { MapSettings settings = new MapSettings() .setProperty("ldap.url", server.getUrl()) - .setProperty("ldap.user.baseDn", "cn=users"); - LdapRealm realm = new LdapRealm(new LdapSettingsManager(settings.asConfig())); - realm.init(); - assertThat(realm.doGetAuthenticator()).isInstanceOf(DefaultLdapAuthenticator.class); + .setProperty("ldap.user.baseDn", "cn=users") + .setProperty(SONAR_SECURITY_REALM.getKey(), LdapRealm.LDAP_SECURITY_REALM); + + LdapRealm realm = new LdapRealm(new LdapSettingsManager(settings.asConfig()), settings.asConfig()); + assertThat(realm.getAuthenticator()).isInstanceOf(DefaultLdapAuthenticator.class); assertThat(realm.getUsersProvider()).isInstanceOf(LdapUsersProvider.class).isInstanceOf(DefaultLdapUsersProvider.class); assertThat(realm.getGroupsProvider()).isNull(); } @@ -51,11 +55,61 @@ public class LdapRealmTest { MapSettings settings = new MapSettings() .setProperty("ldap.url", "ldap://no-such-host") .setProperty("ldap.group.baseDn", "cn=groups,dc=example,dc=org") - .setProperty("ldap.user.baseDn", "cn=users,dc=example,dc=org"); - LdapRealm realm = new LdapRealm(new LdapSettingsManager(settings.asConfig())); - assertThatThrownBy(realm::init).isInstanceOf(LdapException.class).hasMessage("Unable to open LDAP connection"); + .setProperty("ldap.user.baseDn", "cn=users,dc=example,dc=org") + .setProperty(SONAR_SECURITY_REALM.getKey(), LdapRealm.LDAP_SECURITY_REALM); + Configuration config = settings.asConfig(); + LdapSettingsManager settingsManager = new LdapSettingsManager(config); + assertThatThrownBy(() -> new LdapRealm(settingsManager, config)).isInstanceOf(LdapException.class) + .hasMessage("LDAP realm failed to start: Unable to open LDAP connection"); + } + + @Test + public void noConnection_ignore_ignoreStartupFailure_is_false() { + MapSettings settings = new MapSettings() + .setProperty("ldap.url", "ldap://no-such-host") + .setProperty("ldap.group.baseDn", "cn=groups,dc=example,dc=org") + .setProperty("ldap.user.baseDn", "cn=users,dc=example,dc=org") + .setProperty(SONAR_SECURITY_REALM.getKey(), LdapRealm.LDAP_SECURITY_REALM) + .setProperty(SONAR_AUTHENTICATOR_IGNORE_STARTUP_FAILURE.getKey(), false); + ; + Configuration config = settings.asConfig(); + LdapSettingsManager settingsManager = new LdapSettingsManager(config); + assertThatThrownBy(() -> new LdapRealm(settingsManager, config)).isInstanceOf(LdapException.class) + .hasMessage("LDAP realm failed to start: Unable to open LDAP connection"); + } + + @Test + public void noConnection_ignore_ignoreStartupFailure_is_true() { + MapSettings settings = new MapSettings() + .setProperty("ldap.url", "ldap://no-such-host") + .setProperty("ldap.group.baseDn", "cn=groups,dc=example,dc=org") + .setProperty("ldap.user.baseDn", "cn=users,dc=example,dc=org") + .setProperty(SONAR_SECURITY_REALM.getKey(), LdapRealm.LDAP_SECURITY_REALM) + .setProperty(SONAR_AUTHENTICATOR_IGNORE_STARTUP_FAILURE.getKey(), true); + + LdapRealm realm = new LdapRealm(new LdapSettingsManager(settings.asConfig()), settings.asConfig()); + verifyRealm(realm); + } + + @Test + public void should_not_activate_ldap_if_realm_is_not_set() { + MapSettings settings = new MapSettings(); + + LdapRealm realm = new LdapRealm(new LdapSettingsManager(settings.asConfig()), settings.asConfig()); + verifyDeactivatedRealm(realm); + } + + @Test + public void should_not_activate_ldap_if_realm_is_not_ldap() { + MapSettings settings = new MapSettings() + .setProperty(SONAR_SECURITY_REALM.getKey(), "not_ldap"); + + LdapRealm realm = new LdapRealm(new LdapSettingsManager(settings.asConfig()), settings.asConfig()); + verifyDeactivatedRealm(realm); + } - assertThat(realm.doGetAuthenticator()).isInstanceOf(DefaultLdapAuthenticator.class); + private static void verifyRealm(LdapRealm realm) { + assertThat(realm.getAuthenticator()).isInstanceOf(DefaultLdapAuthenticator.class); LdapUsersProvider usersProvider = realm.getUsersProvider(); assertThat(usersProvider).isInstanceOf(LdapUsersProvider.class).isInstanceOf(DefaultLdapUsersProvider.class); @@ -73,6 +127,15 @@ public class LdapRealmTest { .isInstanceOf(LdapException.class) .hasMessage("Unable to retrieve groups for user tester in server with key "); + assertThat(realm.isLdapAuthActivated()).isTrue(); + } + + private static void verifyDeactivatedRealm(LdapRealm realm) { + assertThat(realm.getAuthenticator()).isNull(); + assertThat(realm.getUsersProvider()).isNull(); + assertThat(realm.getGroupsProvider()).isNull(); + assertThat(realm.isLdapAuthActivated()).isFalse(); + } } diff --git a/server/sonar-webserver-auth/src/main/java/org/sonar/server/authentication/LdapCredentialsAuthentication.java b/server/sonar-webserver-auth/src/main/java/org/sonar/server/authentication/LdapCredentialsAuthentication.java index b41295d0562..962e63448f2 100644 --- a/server/sonar-webserver-auth/src/main/java/org/sonar/server/authentication/LdapCredentialsAuthentication.java +++ b/server/sonar-webserver-auth/src/main/java/org/sonar/server/authentication/LdapCredentialsAuthentication.java @@ -37,7 +37,6 @@ import org.sonar.auth.ldap.LdapRealm; import org.sonar.auth.ldap.LdapUserDetails; import org.sonar.auth.ldap.LdapUsersProvider; import org.sonar.db.user.UserDto; -import org.sonar.process.ProcessProperties; import org.sonar.server.authentication.event.AuthenticationEvent; import org.sonar.server.authentication.event.AuthenticationEvent.Source; import org.sonar.server.authentication.event.AuthenticationException; @@ -47,8 +46,6 @@ import static org.apache.commons.lang.StringUtils.trimToNull; public class LdapCredentialsAuthentication { - private static final String LDAP_SECURITY_REALM = "LDAP"; - private static final Logger LOG = Loggers.get(LdapCredentialsAuthentication.class); private final Configuration configuration; @@ -66,19 +63,10 @@ public class LdapCredentialsAuthentication { this.userRegistrar = userRegistrar; this.authenticationEvent = authenticationEvent; - String realmName = configuration.get(ProcessProperties.Property.SONAR_SECURITY_REALM.getKey()).orElse(null); - this.isLdapAuthActivated = LDAP_SECURITY_REALM.equals(realmName); - - if (isLdapAuthActivated) { - ldapRealm.init(); - this.ldapAuthenticator = ldapRealm.doGetAuthenticator(); - this.ldapUsersProvider = ldapRealm.getUsersProvider(); - this.ldapGroupsProvider = ldapRealm.getGroupsProvider(); - } else { - this.ldapAuthenticator = null; - this.ldapUsersProvider = null; - this.ldapGroupsProvider = null; - } + this.isLdapAuthActivated = ldapRealm.isLdapAuthActivated(); + this.ldapAuthenticator = ldapRealm.getAuthenticator(); + this.ldapUsersProvider = ldapRealm.getUsersProvider(); + this.ldapGroupsProvider = ldapRealm.getGroupsProvider(); } public Optional authenticate(Credentials credentials, HttpServletRequest request, AuthenticationEvent.Method method) { @@ -160,7 +148,7 @@ public class LdapCredentialsAuthentication { private final String key; private LdapIdentityProvider(String ldapServerKey) { - this.key = LDAP_SECURITY_REALM + "_" + ldapServerKey; + this.key = LdapRealm.LDAP_SECURITY_REALM + "_" + ldapServerKey; } @Override diff --git a/server/sonar-webserver-auth/src/test/java/org/sonar/server/authentication/LdapCredentialsAuthenticationTest.java b/server/sonar-webserver-auth/src/test/java/org/sonar/server/authentication/LdapCredentialsAuthenticationTest.java index d3c1edbaf4a..6ee4e7c724a 100644 --- a/server/sonar-webserver-auth/src/test/java/org/sonar/server/authentication/LdapCredentialsAuthenticationTest.java +++ b/server/sonar-webserver-auth/src/test/java/org/sonar/server/authentication/LdapCredentialsAuthenticationTest.java @@ -103,18 +103,21 @@ public class LdapCredentialsAuthenticationTest { @Before public void setUp() throws Exception { settings.setProperty(ProcessProperties.Property.SONAR_SECURITY_REALM.getKey(), "LDAP"); - when(ldapRealm.doGetAuthenticator()).thenReturn(ldapAuthenticator); + settings.setProperty(ProcessProperties.Property.SONAR_AUTHENTICATOR_IGNORE_STARTUP_FAILURE.getKey(), "true"); + when(ldapRealm.getAuthenticator()).thenReturn(ldapAuthenticator); when(ldapRealm.getUsersProvider()).thenReturn(ldapUsersProvider); when(ldapRealm.getGroupsProvider()).thenReturn(ldapGroupsProvider); + when(ldapRealm.isLdapAuthActivated()).thenReturn(true); underTest = new LdapCredentialsAuthentication(settings.asConfig(), userRegistrar, authenticationEvent, ldapRealm); } @Test public void authenticate_with_null_group_provider() { reset(ldapRealm); - when(ldapRealm.doGetAuthenticator()).thenReturn(ldapAuthenticator); + when(ldapRealm.getAuthenticator()).thenReturn(ldapAuthenticator); when(ldapRealm.getUsersProvider()).thenReturn(ldapUsersProvider); when(ldapRealm.getGroupsProvider()).thenReturn(null); + when(ldapRealm.isLdapAuthActivated()).thenReturn(true); underTest = new LdapCredentialsAuthentication(settings.asConfig(), userRegistrar, authenticationEvent, ldapRealm); LdapAuthenticator.Context authenticationContext = new LdapAuthenticator.Context(LOGIN, PASSWORD, request); @@ -134,7 +137,6 @@ public class LdapCredentialsAuthenticationTest { assertThat(identity.shouldSyncGroups()).isFalse(); verify(authenticationEvent).loginSuccess(request, LOGIN, Source.realm(BASIC, LDAP_SECURITY_REALM_NAME)); - verify(ldapRealm).init(); } @Test @@ -148,7 +150,6 @@ public class LdapCredentialsAuthenticationTest { assertThat(provider.getDisplay()).isNull(); assertThat(provider.isEnabled()).isTrue(); verify(authenticationEvent).loginSuccess(request, LOGIN, Source.realm(BASIC, LDAP_SECURITY_REALM_NAME)); - verify(ldapRealm).init(); } @Test @@ -265,12 +266,11 @@ public class LdapCredentialsAuthenticationTest { @Test public void return_empty_user_when_ldap_not_activated() { reset(ldapRealm); - settings.clear(); + when(ldapRealm.isLdapAuthActivated()).thenReturn(false); underTest = new LdapCredentialsAuthentication(settings.asConfig(), userRegistrar, authenticationEvent, ldapRealm); assertThat(underTest.authenticate(new Credentials(LOGIN, PASSWORD), request, BASIC)).isEmpty(); verifyNoInteractions(authenticationEvent); - verifyNoInteractions(ldapRealm); } private void executeAuthenticate(@Nullable LdapUserDetails userDetails) { -- 2.39.5