[FIXED HUDSON-3459] Matrix permissions with LDAP now properly validates group names

using configured prefix and case settings; added help text about these settings.


git-svn-id: https://hudson.dev.java.net/svn/hudson/trunk/hudson/main@20744 71c3de6d-444a-0410-be80-ed276b4c234a

Originally-Committed-As: 6c2c917d91e33ffa63429848c5f6a75ea4bd773c

core/src/main/java/hudson/security/LDAPSecurityRealm.java

@@ -340,9 +340,54 @@ public class LDAPSecurityRealm extends SecurityRealm {
 
         return new SecurityComponents(
             findBean(AuthenticationManager.class, appContext),
-            new UserDetailsService() {
+            new LDAPUserDetailsService(appContext));
-                final LdapUserSearch ldapSearch = findBean(LdapUserSearch.class, appContext);
+    }
-                final LdapAuthoritiesPopulator authoritiesPopulator = findBean(LdapAuthoritiesPopulator.class, appContext);
+
+    /**
+     * Lookup a group; given input must match the configured syntax for group names
+     * in WEB-INF/security/LDAPBindSecurityRealm.groovy's authoritiesPopulator entry.
+     * The defaults are a prefix of "ROLE_" and using all uppercase.  This method will
+     * not return any data if the given name lacks the proper prefix and/or case. 
+     */
+    @Override
+    public GroupDetails loadGroupByGroupname(String groupname) throws UsernameNotFoundException, DataAccessException {
+        // Check proper syntax based on acegi configuration
+        String prefix = "";
+        boolean onlyUpperCase = false;
+        try {
+            AuthoritiesPopulatorImpl api = (AuthoritiesPopulatorImpl)
+              ((LDAPUserDetailsService)getSecurityComponents().userDetails).authoritiesPopulator;
+            prefix = api.rolePrefix;
+            onlyUpperCase = api.convertToUpperCase;
+        } catch (Exception ignore) { }
+        if (onlyUpperCase && !groupname.equals(groupname.toUpperCase()))
+            throw new UsernameNotFoundException(groupname + " should be all uppercase");
+        if (!groupname.startsWith(prefix))
+            throw new UsernameNotFoundException(groupname + " is missing prefix: " + prefix);
+        groupname = groupname.substring(prefix.length());
+
+        // TODO: obtain a DN instead so that we can obtain multiple attributes later
+        String searchBase = groupSearchBase != null ? groupSearchBase : "";
+        final Set<String> groups = (Set<String>)ldapTemplate.searchForSingleAttributeValues(searchBase, GROUP_SEARCH,
+                new String[]{groupname}, "cn");
+
+        if(groups.isEmpty())
+            throw new UsernameNotFoundException(groupname);
+
+        return new GroupDetails() {
+            public String getName() {
+                return groups.iterator().next();
+            }
+        };
+    }
+
+    public static class LDAPUserDetailsService implements UserDetailsService {
+        public final LdapUserSearch ldapSearch;
+        public final LdapAuthoritiesPopulator authoritiesPopulator;
+        LDAPUserDetailsService(WebApplicationContext appContext) {
+            ldapSearch = findBean(LdapUserSearch.class, appContext);
+            authoritiesPopulator = findBean(LdapAuthoritiesPopulator.class, appContext);
+        }
         public UserDetails loadUserByUsername(String username) throws UsernameNotFoundException, DataAccessException {
             try {
                 LdapUserDetails ldapUser = ldapSearch.searchForUser(username);
@@ -362,24 +407,6 @@ public class LDAPSecurityRealm extends SecurityRealm {
                 throw new UserMayOrMayNotExistException(e.getMessage(),e);
             }
         }
-            });
-    }
-
-    @Override
-    public GroupDetails loadGroupByGroupname(String groupname) throws UsernameNotFoundException, DataAccessException {
-        // TODO: obtain a DN instead so that we can obtain multiple attributes later
-        String searchBase = groupSearchBase != null ? groupSearchBase : "";
-        final Set<String> groups = (Set<String>)ldapTemplate.searchForSingleAttributeValues(searchBase, GROUP_SEARCH,
-                new String[]{groupname}, "cn");
-
-        if(groups.isEmpty())
-            throw new UsernameNotFoundException(groupname);
-
-        return new GroupDetails() {
-            public String getName() {
-                return groups.iterator().next();
-            }
-        };
     }
 
     /**
@@ -414,14 +441,32 @@ public class LDAPSecurityRealm extends SecurityRealm {
      * {@link LdapAuthoritiesPopulator} that adds the automatic 'authenticated' role.
      */
     public static final class AuthoritiesPopulatorImpl extends DefaultLdapAuthoritiesPopulator {
+        // Make these available (private in parent class and no get methods!)
+        String rolePrefix;
+        boolean convertToUpperCase;
         public AuthoritiesPopulatorImpl(InitialDirContextFactory initialDirContextFactory, String groupSearchBase) {
             super(initialDirContextFactory, groupSearchBase);
+            // These match the defaults in acegi 1.0.5; set again to store in non-private fields:
+            setRolePrefix("ROLE_");
+            setConvertToUpperCase(true);
         }
 
         @Override
         protected Set getAdditionalRoles(LdapUserDetails ldapUser) {
             return Collections.singleton(AUTHENTICATED_AUTHORITY);
         }
+
+        @Override
+        public void setRolePrefix(String rolePrefix) {
+            super.setRolePrefix(rolePrefix);
+            this.rolePrefix = rolePrefix;
+        }
+
+        @Override
+        public void setConvertToUpperCase(boolean convertToUpperCase) {
+            super.setConvertToUpperCase(convertToUpperCase);
+            this.convertToUpperCase = convertToUpperCase;
+        }
     }
 
     @Extension

war/resources/WEB-INF/security/LDAPBindSecurityRealm.groovy

@@ -65,6 +65,8 @@ authoritiesPopulator(AuthoritiesPopulatorImpl, initialDirContextFactory, Util.fi
     // see DefaultLdapAuthoritiesPopulator for other possible configurations
     searchSubtree = true;
     groupSearchFilter = "(| (member={0}) (uniqueMember={0}) (memberUid={1}))";
+    // rolePrefix = "ROLE_"; // Default is "ROLE_"
+    // convertToUpperCase = false; // Default is true
 }
 
 authenticationManager(ProviderManager) {