Add configuration option for groupNameAttribute to use fields other than CN as group lookup.

README.md

@@ -1,10 +1,10 @@
-Jenkins Reverse Proxy Authentication and Authorisation Plugin
+# Jenkins Reverse Proxy Authentication and Authorisation Plugin
 
-The Reverse Proxy Plugin providers developers the ability to have easy and simple Authentication and Authorisation using SSO techniques. The plugin expects that the user to have Jenkins authenticated agains will be informed via a HHTP header field.
+The Reverse proxy plugin providers developers the ability to have easy and simple authentication and authorisation using SSO techniques. The plugin authenticates the user in Jenkins via a HTTP header field.
 
-When it comes to Authorisation, the plugin has been extended in order to offer two flavours to developers: HTTP header containing LDAP groups; or LDAP discovery. When one of the mentioned favlours is used, the developer can have Jenkins configured to use Role Based Matrix Authorisation, that will read the groups that were fed to the Reverse Proxy plugin.
+When it comes to authorisation, the offers two options to developers: HTTP header containing LDAP groups or LDAP discovery. When one of the mentioned options is used, the developer can have Jenkins configured to use role based matrix authorisation, that will read the groups that were configured in the Reverse Proxy plugin.
 
-The default values for the HTTP header fields are:
+## The default values for the HTTP header fields are:
 
 1. Header User Name: X-Forwarded-User
 2. Header Groups Name: X-Forwarded-Groups
@@ -14,7 +14,8 @@ The LDAP options can be displayed via the Advanced... button, located on the rig
 
 If no LDAP information is given, the default used will be the HEADER fields. However, if both are configured, the LDAP has priority over the HTTP header.
 
-If the username is not forwaded to Jenkins, the user will be authenticated as ANONYMOUS. When LDAP groups are sent via the HTTP header, there is no check if the username exists in the LDAP directory, so protect your proxy in order to avoid HTTP Header injection. Once an username is informed, the user will be authenticated. If no groups are returned from the LDAP search, the user will still be authenticated, but no other grants will be given.
+If the username is not forwarded to Jenkins, the user will be authenticated as ANONYMOUS. When LDAP groups are sent via the HTTP header, there is no check if the username exists in the LDAP directory, so protect your proxy in order to avoid HTTP Header injection. Once an username is informed, the user will be authenticated. If no groups are returned from the LDAP search, the user will still be authenticated, but no other grants will be given.
 
 However, once the LDAP is properly configured instead of groups on the HTTP header, there is guarantee that only the groups of a given user will be returned. There is no possibility to get groups injected via the header.
 
+See the fields in [ReverseProxySecurityRealm.java](https://github.com/jenkinsci/reverse-proxy-auth-plugin/blob/master/src/main/java/org/jenkinsci/plugins/reverse_proxy_auth/ReverseProxySecurityRealm.java) for details about the available options. 

src/main/java/org/jenkinsci/plugins/reverse_proxy_auth/ReverseProxySecurityRealm.java

@@ -213,6 +213,13 @@ public class ReverseProxySecurityRealm extends SecurityRealm {
       */
      public final String groupMembershipFilter;
 
+     /**
+      * Attribute that should be used instead of CN as name to match a users group name to the groupSearchFilter name.
+      * When {@link #groupSearchFilter} is set to search for a field other than CN e.g. <code>GroupDisplayName={0}</code> 
+      * here you can configure that this (<code>GroupDisplayName</code>) or another field should be used when looking for a users groups.
+      */
+     public String groupNameAttribute;
+
 	/**
 	 * If non-null, we use this and {@link #managerPassword}
 	 * when binding to LDAP.
@@ -260,7 +267,8 @@ public class ReverseProxySecurityRealm extends SecurityRealm {
 
 	@DataBoundConstructor
 	public ReverseProxySecurityRealm(String forwardedUser, String headerGroups, String headerGroupsDelimiter, String server, String rootDN, boolean inhibitInferRootDN,
-			String userSearchBase, String userSearch, String groupSearchBase, String groupSearchFilter, String groupMembershipFilter, String managerDN, String managerPassword, Integer updateInterval, boolean disableLdapEmailResolver, String displayNameLdapAttribute, String emailAddressLdapAttribute) {
+			String userSearchBase, String userSearch, String groupSearchBase, String groupSearchFilter, String groupMembershipFilter, String groupNameAttribute, String managerDN, String managerPassword, 
+			Integer updateInterval, boolean disableLdapEmailResolver, String displayNameLdapAttribute, String emailAddressLdapAttribute) {
 
 		this.forwardedUser = fixEmptyAndTrim(forwardedUser);
 
@@ -270,7 +278,6 @@ public ReverseProxySecurityRealm(String forwardedUser, String headerGroups, Stri
 		} else {
 			this.headerGroupsDelimiter = "|";
 		}
-		//
 		this.server = fixEmptyAndTrim(server);
 		this.managerDN = fixEmpty(managerDN);
 		this.managerPassword = Scrambler.scramble(fixEmpty(managerPassword));
@@ -289,6 +296,7 @@ public ReverseProxySecurityRealm(String forwardedUser, String headerGroups, Stri
 		this.groupSearchBase = fixEmptyAndTrim(groupSearchBase);
 		this.groupSearchFilter = fixEmptyAndTrim(groupSearchFilter);
 		this.groupMembershipFilter = fixEmptyAndTrim(groupMembershipFilter);
+		this.groupNameAttribute = fixEmptyAndTrim(groupNameAttribute);
 
 		this.updateInterval = (updateInterval == null || updateInterval <= 0) ? CHECK_INTERVAL : updateInterval;
 
@@ -339,6 +347,14 @@ public String getGroupMembershipFilter() {
 		return groupMembershipFilter;
 	}
 
+	public String getGroupNameAttribute() {
+		return groupNameAttribute;
+	}
+
+	public void setGroupNameAttribute(String groupNameAttribute) {
+		this.groupNameAttribute = groupNameAttribute;
+	}
+
 	public String getDisplayNameLdapAttribute() {
 		return displayNameLdapAttribute;
 	}
@@ -444,12 +460,10 @@ public void doFilter(ServletRequest request,
 				String userFromHeader = null;
 
 				Authentication auth = Hudson.ANONYMOUS;
-				if ((forwardedUser != null
-					 && (userFromHeader = r.getHeader(forwardedUser)) != null)
-					 || userFromApiToken != null) {
-					//LOGGER.log(Level.INFO, "USER LOGGED IN: {0}", userFromHeader);
-				        if (userFromHeader == null && userFromApiToken != null) {
-					        userFromHeader = userFromApiToken;
+				if ((forwardedUser != null && (userFromHeader = r.getHeader(forwardedUser)) != null) || userFromApiToken != null) {
+					LOGGER.log(Level.FINE, "USER LOGGED IN: {0}", userFromHeader);
+			        if (userFromHeader == null && userFromApiToken != null) {
+				        userFromHeader = userFromApiToken;
 					}
 
 					if (getLDAPURL() != null) {
@@ -556,10 +570,15 @@ public SecurityComponents createSecurityComponents() {
 		} else {
 			ldapTemplate = new LdapTemplate(findBean(InitialDirContextFactory.class, appContext));
 
+			if (groupMembershipFilter != null || groupNameAttribute != null) {
+				ProxyLDAPAuthoritiesPopulator authoritiesPopulator = findBean(ProxyLDAPAuthoritiesPopulator.class, appContext);
 		        if (groupMembershipFilter != null) {
-			        ProxyLDAPAuthoritiesPopulator authoritiesPopulator = findBean(ProxyLDAPAuthoritiesPopulator.class, appContext);
 			        authoritiesPopulator.setGroupSearchFilter(groupMembershipFilter);
 		        }
+		        if (groupNameAttribute != null) {					
+		        	authoritiesPopulator.setGroupRoleAttribute(groupNameAttribute);
+				}
+			}
 
 			return new SecurityComponents(findBean(AuthenticationManager.class, appContext), new ProxyLDAPUserDetailsService(this, appContext));
 		}
@@ -585,7 +604,7 @@ public LdapUserDetails updateLdapUserDetails(LdapUserDetails d) {
             LOGGER.log(Level.FINEST, "getAttributes is null");
         } else {
             hudson.model.User u = hudson.model.User.get(d.getUsername());
-            if (!StringUtils.isBlank(displayNameLdapAttribute)){
+            if (!StringUtils.isBlank(displayNameLdapAttribute)) {
                 LOGGER.log(Level.FINEST, "Getting user details from LDAP attributes");
                 try {
                     Attribute attribute = d.getAttributes().get(displayNameLdapAttribute);
@@ -669,16 +688,17 @@ public FormValidation doServerCheck(
 				@QueryParameter final String managerDN,
 				@QueryParameter final String managerPassword) {
 
-			if(!Jenkins.getInstance().hasPermission(Jenkins.ADMINISTER))
+			if (!Jenkins.getInstance().hasPermission(Jenkins.ADMINISTER)) {
 				return FormValidation.ok();
+			}
 
 			try {
 				Hashtable<String,String> props = new Hashtable<String,String>();
-				if(managerDN!=null && managerDN.trim().length() > 0  && !"undefined".equals(managerDN)) {
-					props.put(Context.SECURITY_PRINCIPAL,managerDN);
+				if (managerDN != null && managerDN.trim().length() > 0 && !"undefined".equals(managerDN)) {
+					props.put(Context.SECURITY_PRINCIPAL, managerDN);
 				}
-				if(managerPassword!=null && managerPassword.trim().length() > 0 && !"undefined".equals(managerPassword)) {
-					props.put(Context.SECURITY_CREDENTIALS,managerPassword);
+				if (managerPassword!=null && managerPassword.trim().length() > 0 && !"undefined".equals(managerPassword)) {
+					props.put(Context.SECURITY_CREDENTIALS, managerPassword);
 				}
 
 				props.put(Context.INITIAL_CONTEXT_FACTORY, "com.sun.jndi.ldap.LdapCtxFactory");
@@ -769,7 +789,7 @@ private GrantedAuthority[] retrieveAuthoritiesIfNecessary(final String userFromH
 
 					authorityUpdateCache.put(userFromHeader, current);
 
-					LOGGER.log(Level.INFO, "Authorities for user "+userFromHeader+" have been updated.");
+					LOGGER.log(Level.INFO, "Authorities for user " + userFromHeader + " have been updated.");
 				}
 			} else {
 				if (authorityUpdateCache == null) {

src/main/java/org/jenkinsci/plugins/reverse_proxy_auth/auth/ReverseProxyAuthoritiesPopulatorImpl.java

@@ -44,8 +44,7 @@ public ReverseProxyAuthoritiesPopulatorImpl(
 	}
 
 	@Override
-	protected Set<GrantedAuthority> getAdditionalRoles(
-			ReverseProxyUserDetails proxyUser) {
+	protected Set<GrantedAuthority> getAdditionalRoles(ReverseProxyUserDetails proxyUser) {
 		return Collections.singleton(SecurityRealm.AUTHENTICATED_AUTHORITY);
 	}
 
@@ -71,8 +70,7 @@ public void setConvertToUpperCase(boolean convertToUpperCase) {
 	@Override
 	public Set<GrantedAuthority> getGroupMembershipRoles(String username) {
 
-		Set<GrantedAuthority> names = super
-				.getGroupMembershipRoles(username);
+		Set<GrantedAuthority> names = super.getGroupMembershipRoles(username);
 
 		Set<GrantedAuthority> groupRoles = new HashSet<GrantedAuthority>(names.size() * 2);
 		groupRoles.addAll(names);

src/main/java/org/jenkinsci/plugins/reverse_proxy_auth/service/ProxyLDAPAuthoritiesPopulator.java

@@ -55,8 +55,8 @@ public void setConvertToUpperCase(boolean convertToUpperCase) {
 	 */
 	@Override
 	@SuppressWarnings("unchecked")
-	public Set getGroupMembershipRoles(String userDn, String username) {
-		Set<GrantedAuthority> names = super.getGroupMembershipRoles(userDn,username);
+	public Set<GrantedAuthority> getGroupMembershipRoles(String userDn, String username) {
+		Set<GrantedAuthority> names = super.getGroupMembershipRoles(userDn, username);
 
 		Set<GrantedAuthority> r = new HashSet<GrantedAuthority>(names.size()*2);
 		r.addAll(names);

src/main/java/org/jenkinsci/plugins/reverse_proxy_auth/service/ProxyLDAPUserDetailsService.java

@@ -59,7 +59,7 @@ public LdapUserDetails loadUserByUsername(String username) throws UsernameNotFou
 
 					// intern attributes
 					Attributes v = ldapUser.getAttributes();
-					if (v instanceof BasicAttributes) {// BasicAttributes.equals is what makes the interning possible
+					if (v instanceof BasicAttributes) { // BasicAttributes.equals is what makes the interning possible
 						synchronized (attributesCache) {
 							Attributes vv = (Attributes)attributesCache.get(v);
 							if (vv==null)   attributesCache.put(v,vv=v);

src/main/resources/org/jenkinsci/plugins/reverse_proxy_auth/ReverseProxySecurityRealm/config.jelly

@@ -56,6 +56,9 @@ THE SOFTWARE.
     <f:entry title="${%Group membership filter}" >
       <f:textbox name="ldap.groupMembershipFilter" value="${instance.groupMembershipFilter}" />
     </f:entry>
+    <f:entry title="${%Group name attribute instead of CN}" >
+      <f:textbox name="ldap.groupNameAttribute" value="${instance.groupNameAttribute}" />
+    </f:entry>
     <f:entry title="${%Manager DN}" >
       <f:textbox name="managerDN" value="${instance.managerDN}" autocomplete="off"
          checkUrl="'${rootURL}/securityRealms/LDAPSecurityRealm/serverCheck?field=managerDN&amp;server='+encodeURIComponent(this.form.elements['ldap.server'].value)+'&amp;managerDN='+encodeURIComponent(this.value)+'&amp;managerPassword='+encodeURIComponent(this.form.elements['ldap.managerPassword'].value)"