Removing special treatment for single-server case as suggested in jen…

…kinsci#49 (comment)
jglick · Sep 28, 2020 · c48c887 · c48c887
1 parent 0621ae2
commit c48c887
Show file tree

Hide file tree

Showing 2 changed files with 7 additions and 44 deletions.
diff --git a/src/main/java/hudson/security/LDAPSecurityRealm.java b/src/main/java/hudson/security/LDAPSecurityRealm.java
@@ -659,10 +659,8 @@ public String getMailAddressAttributeName() {
 
     @CheckForNull @Restricted(NoExternalUse.class)
     public LDAPConfiguration getConfigurationFor(LdapUserDetails d) {
-        if (d instanceof DelegatedLdapUserDetails && ((DelegatedLdapUserDetails) d).getConfigurationId() != null) {
+        if (d instanceof DelegatedLdapUserDetails) {
             return getConfigurationFor(((DelegatedLdapUserDetails) d).getConfigurationId());
-        } else if (hasConfiguration() && configurations.size() == 1) {
-            return configurations.get(0);
         } else {
             return null;
         }
@@ -681,9 +679,6 @@ public LDAPConfiguration getConfigurationFor(String configurationId) {
                     return configuration;
                 }
             }
-            if (configurations.size() == 1) {
-                return configurations.get(0);
-            }
         }
         LOGGER.log(Level.FINE, "Unable to find configuration for {0}", configurationId);
         return null;
@@ -739,7 +734,6 @@ private static String getProviderUrl(String server, String rootDN) {
 
     @Override @Nonnull
     public SecurityComponents createSecurityComponents() {
-        if (configurations.size() > 1) {
             DelegateLDAPUserDetailsService details = new DelegateLDAPUserDetailsService();
             LDAPAuthenticationManager manager = new LDAPAuthenticationManager(details);
             for (LDAPConfiguration conf : configurations) {
@@ -748,16 +742,6 @@ public SecurityComponents createSecurityComponents() {
                 details.addDelegate(new LDAPUserDetailsService(appContext.ldapUserSearch, appContext.ldapAuthoritiesPopulator, conf.getGroupMembershipStrategy(), conf.getId()));
             }
             return new SecurityComponents(manager, details);
-        } else {
-            final LDAPConfiguration conf = configurations.get(0);
-            LDAPConfiguration.ApplicationContext appContext = conf.createApplicationContext(this);
-            final LDAPAuthenticationManager manager = new LDAPAuthenticationManager();
-            manager.addDelegate(appContext.authenticationManager, "", appContext.ldapUserSearch);
-            return new SecurityComponents(
-                    manager,
-                    new LDAPUserDetailsService(appContext.ldapUserSearch, appContext.ldapAuthoritiesPopulator, conf.getGroupMembershipStrategy(), null)
-            );
-        }
     }
 
     /**
@@ -987,10 +971,6 @@ private class LDAPAuthenticationManager implements AuthenticationManager {
         private final List<ManagerEntry> delegates = new ArrayList<>();;
         private final DelegateLDAPUserDetailsService detailsService;
 
-        private LDAPAuthenticationManager() {
-            detailsService = null;
-        }
-
         private LDAPAuthenticationManager(DelegateLDAPUserDetailsService detailsService) {
             this.detailsService = detailsService;
         }
@@ -1002,21 +982,13 @@ private void addDelegate(AuthenticationManager delegate, String configurationId,
         @Override
         public Authentication authenticate(Authentication authentication) throws AuthenticationException {
         try (SetContextClassLoader sccl = new SetContextClassLoader()) {
-            if (delegates.size() == 1) {
-                try {
-                    return updateUserDetails(delegates.get(0).delegate.authenticate(authentication), delegates.get(0).ldapUserSearch);
-                } catch (AuthenticationServiceException e) {
-                    LOGGER.log(Level.WARNING, "Failed communication with ldap server.", e);
-                    throw e;
-                }
-            }
             AuthenticationException lastException = null;
             for (ManagerEntry delegate : delegates) {
                 try {
                     Authentication a = delegate.delegate.authenticate(authentication);
                     Object principal = a.getPrincipal();
                     if (principal instanceof LdapUserDetails && !(principal instanceof DelegatedLdapUserDetails)) {
-                        principal = new DelegatedLdapUserDetails((LdapUserDetails) principal, delegate.configurationId);
+                        principal = new DelegatedLdapUserDetails((LdapUserDetails) principal, delegate.configurationId, null);
                     }
                     return updateUserDetails(new DelegatedLdapAuthentication(a, principal, delegate.configurationId), delegate.ldapUserSearch);
                 } catch (BadCredentialsException e) {
@@ -1123,16 +1095,12 @@ public String getConfigurationId() {
     static class DelegatedLdapUserDetails implements LdapUserDetails, Serializable {
         private static final long serialVersionUID = 1L;
         private final LdapUserDetails userDetails;
-        @CheckForNull
+        @Nonnull
         private final String configurationId;
         @CheckForNull
         private final Attributes attributes;
 
-        public DelegatedLdapUserDetails(@Nonnull LdapUserDetails userDetails, @CheckForNull String configurationId) {
-            this(userDetails, configurationId, null);
-        }
-
-        public DelegatedLdapUserDetails(@Nonnull LdapUserDetails userDetails, @CheckForNull String configurationId, @CheckForNull Attributes attributes) {
+        DelegatedLdapUserDetails(@Nonnull LdapUserDetails userDetails, @Nonnull String configurationId, @CheckForNull Attributes attributes) {
             this.userDetails = userDetails;
             this.configurationId = configurationId;
             this.attributes = attributes;
@@ -1182,7 +1150,7 @@ public LdapUserDetails getUserDetails() {
             return userDetails;
         }
 
-        @CheckForNull
+        @Nonnull
         public String getConfigurationId() {
             return configurationId;
         }
@@ -1283,11 +1251,6 @@ public static class LDAPUserDetailsService implements UserDetailsService {
          */
         private final LRUMap attributesCache = new LRUMap(32);
 
-        @Deprecated
-        LDAPUserDetailsService(LdapUserSearch ldapSearch, LdapAuthoritiesPopulator authoritiesPopulator) {
-            this(ldapSearch, authoritiesPopulator, null, null);
-        }
-
         LDAPUserDetailsService(LdapUserSearch ldapSearch, LdapAuthoritiesPopulator authoritiesPopulator, LDAPGroupMembershipStrategy groupMembershipStrategy, String configurationId) {
             this.ldapSearch = ldapSearch;
             this.authoritiesPopulator = authoritiesPopulator;
@@ -1345,7 +1308,7 @@ public DelegatedLdapUserDetails loadUserByUsername(String username) throws Usern
                             user.addAuthority(extraAuthority);
                         }
                     }
-                DelegatedLdapUserDetails ldapUserDetails = new DelegatedLdapUserDetails(user.createUserDetails(), StringUtils.isNotEmpty(configurationId) ? configurationId : null, v);
+                DelegatedLdapUserDetails ldapUserDetails = new DelegatedLdapUserDetails(user.createUserDetails(), configurationId, v);
                 if (securityRealm instanceof LDAPSecurityRealm
                         && (securityRealm.getSecurityComponents().userDetails2 == this
                             || (securityRealm.getSecurityComponents().userDetails2 instanceof DelegateLDAPUserDetailsService

diff --git a/src/test/java/hudson/security/LDAPSecurityRealmTest.java b/src/test/java/hudson/security/LDAPSecurityRealmTest.java
@@ -71,7 +71,7 @@ public void attributesCache() {
                 } catch (InvalidNameException x) {
                     throw new UsernameNotFoundException(x.toString(), x);
                 }
-            }, (userData, username) -> Collections.emptySet());
+            }, (userData, username) -> Collections.emptySet(), null, "irrelevant");
         LDAPSecurityRealm.DelegatedLdapUserDetails d1 = s.loadUserByUsername("me");
         LDAPSecurityRealm.DelegatedLdapUserDetails d2 = s.loadUserByUsername("you");
         LDAPSecurityRealm.DelegatedLdapUserDetails d3 = s.loadUserByUsername("me");