Allow data containing groups from SSO server to be a List of Maps in …

…addition to a List of Strings. (#198)

* Allow data containing groups from SSO server to be a List of Maps in
addition to a List of Strings. In cases where groups is a List of Maps, give option to specify which key in the Map holds the group name, defaulting to a key "name"

* fix checkstyle errors for ImportOrderCheck and spaces at end of line

* Allow groupsFieldName to be specified as 'groups[].name' to specify a nested group field name for case where SSO server returns groups as an array of maps.

* updates for comments in PR #198

* fix newline at end of file for checkstyle

src/main/java/org/jenkinsci/plugins/oic/OicSecurityRealm.java

@@ -44,6 +44,7 @@
 import com.google.api.client.json.JsonObjectParser;
 import com.google.api.client.json.jackson2.JacksonFactory;
 import com.google.api.client.json.webtoken.JsonWebSignature;
+import com.google.api.client.util.ArrayMap;
 import com.google.api.client.util.Data;
 import com.google.common.base.Strings;
 import edu.umd.cs.findbugs.annotations.SuppressFBWarnings;
@@ -136,6 +137,8 @@ public static enum TokenAuthMethod { client_secret_basic, client_secret_post };
     private String fullNameFieldName = null;
     private String emailFieldName = null;
     private String groupsFieldName = null;
+    private String simpleGroupsFieldName = null;
+    private String nestedGroupFieldName = null;
     private String scopes = null;
     private final boolean disableSslVerification;
     private boolean logoutFromOpenidProvider = true;
@@ -232,7 +235,7 @@ public OicSecurityRealm(String clientId, String clientSecret, String wellKnownOp
         this.userNameField = Util.fixEmpty(userNameField) == null ? "sub" : userNameField;
         this.fullNameFieldName = Util.fixEmpty(fullNameFieldName);
         this.emailFieldName = Util.fixEmpty(emailFieldName);
-        this.groupsFieldName = Util.fixEmpty(groupsFieldName);
+        this.setGroupsFieldName(Util.fixEmpty(groupsFieldName));
         this.logoutFromOpenidProvider = Util.fixNull(logoutFromOpenidProvider, Boolean.TRUE);
         this.postLogoutRedirectUrl = postLogoutRedirectUrl;
         this.escapeHatchEnabled = Util.fixNull(escapeHatchEnabled, Boolean.FALSE);
@@ -486,6 +489,22 @@ public void setEmailFieldName(String emailFieldName) {
     @DataBoundSetter
     public void setGroupsFieldName(String groupsFieldName) {
         this.groupsFieldName = Util.fixEmpty(groupsFieldName);
+        // if groupsFieldName contains []., then groupsFieldName
+        // is first portion, and nestedGroupFieldName is
+        // second portion
+        // split on "[]." and only split on first occurrence
+        if (this.groupsFieldName != null) {
+            String[] parts = this.groupsFieldName.split("\\[\\]\\.", 2);
+            this.simpleGroupsFieldName = Util.fixEmpty(parts[0]);
+            this.nestedGroupFieldName = parts.length > 1 ? Util.fixEmpty(parts[1]) : null;
+            if (this.groupsFieldName.split("\\[\\]\\.").length > 2) {
+                LOGGER.warning("nestedGroupFieldName contains more than one []., this is not supported");
+            }
+            LOGGER.fine(
+                    "in setGroupsFieldName,  groupsFieldName is " + this.groupsFieldName + " simpleGroupsFieldName is "
+                            + this.simpleGroupsFieldName + " nestedGroupFieldName is " + this.nestedGroupFieldName);
+        }
+
     }
 
     // Not a DataBoundSetter - set in constructor
@@ -836,30 +855,29 @@ private String determineStringField(String fieldName, IdToken idToken, GenericJs
     private List<GrantedAuthority> determineAuthorities(IdToken idToken, GenericJson userInfo) {
         List<GrantedAuthority> grantedAuthorities = new ArrayList<>();
         grantedAuthorities.add(SecurityRealm.AUTHENTICATED_AUTHORITY2);
-
-        if (isNotBlank(groupsFieldName)) {
-            if (!Strings.isNullOrEmpty(userInfoServerUrl) && containsField(userInfo, groupsFieldName)) {
-                LOGGER.fine("UserInfo contains group field name: " + groupsFieldName + " with value class:" + getField(userInfo, groupsFieldName).getClass());
-                List<String> groupNames = ensureString(getField(userInfo, groupsFieldName));
+        if (isNotBlank(simpleGroupsFieldName)) {
+            if (!Strings.isNullOrEmpty(userInfoServerUrl) && containsField(userInfo, simpleGroupsFieldName)) {
+                LOGGER.fine("UserInfo contains group field name: " + simpleGroupsFieldName + " with value class:" + getField(userInfo, simpleGroupsFieldName).getClass());
+                List<String> groupNames = ensureString(getField(userInfo, simpleGroupsFieldName));
                 if(groupNames.isEmpty()){
-                    LOGGER.warning("UserInfo does not contains groups in " + groupsFieldName);
+                    LOGGER.warning("UserInfo does not contains groups in " + simpleGroupsFieldName);
                 } else {
                     LOGGER.fine("Number of groups in groupNames: " + groupNames.size());
                 }
                 for (String groupName : groupNames) {
                     LOGGER.fine("Adding group from UserInfo: " + groupName);
                     grantedAuthorities.add(new SimpleGrantedAuthority(groupName));
                 }
-            } else if (containsField(idToken.getPayload(), groupsFieldName)) {
-                LOGGER.fine("idToken contains group field name: " + groupsFieldName + " with value class:" + getField(idToken.getPayload(), groupsFieldName).getClass());
-                List<String> groupNames = ensureString(getField(idToken.getPayload(), groupsFieldName));
+            } else if (containsField(idToken.getPayload(), simpleGroupsFieldName)) {
+                LOGGER.fine("idToken contains group field name: " + simpleGroupsFieldName + " with value class:" + getField(idToken.getPayload(), simpleGroupsFieldName).getClass());
+                List<String> groupNames = ensureString(getField(idToken.getPayload(), simpleGroupsFieldName));
                 LOGGER.fine("Number of groups in groupNames: " + groupNames.size());
                 for (String groupName : groupNames) {
                     LOGGER.fine("Adding group from idToken: " + groupName);
                     grantedAuthorities.add(new SimpleGrantedAuthority(groupName));
                 }
             } else {
-                LOGGER.warning("idToken and userInfo did not contain group field name: " + groupsFieldName);
+                LOGGER.warning("idToken and userInfo did not contain group field name: " + simpleGroupsFieldName);
             }
         } else {
             LOGGER.fine("Not adding groups because groupsFieldName is not set. groupsFieldName=" + groupsFieldName);
@@ -887,6 +905,21 @@ private List<String> ensureString(Object field) {
                 }
             }
             return result;
+        } else if (field instanceof ArrayList) {
+            List<String> result = new ArrayList<>();
+            List<Object> groups = (List<Object>) field;
+            for (Object group : groups) {
+              if (group instanceof String) {
+                result.add(group.toString());
+              } else if (group instanceof ArrayMap) {
+                // if its a Map, we use the nestedGroupFieldName to grab the groups
+                Map<String, String> groupMap = (Map<String, String>) group;
+                if (nestedGroupFieldName != null &&  groupMap.keySet().contains(nestedGroupFieldName)) {
+                    result.add(groupMap.get(nestedGroupFieldName));
+                }
+              }
+            }
+            return result;
         } else {
             try {
                 return (List<String>) field;
@@ -1243,5 +1276,21 @@ public FormValidation doCheckPostLogoutRedirectUrl(@QueryParameter String postLo
 
             return FormValidation.ok();
         }
+
+        @RequirePOST
+        // method to check groupsFieldName matches the required format
+        // can contain the substring "[]." at most once.
+        // e.g. "groups", "groups[].name" are valid
+        // groups[].name[].id is not valid
+        public FormValidation doCheckGroupsFieldName(@QueryParameter String groupsFieldName) {
+            Jenkins.get().checkPermission(Jenkins.ADMINISTER);
+            if (Util.fixEmptyAndTrim(groupsFieldName) == null) {
+                return FormValidation.ok();
+            }
+            if (groupsFieldName.split("\\[\\]\\.").length > 2) {
+                return FormValidation.error(Messages.OicSecurityRealm_InvalidGroupsFieldName());
+            }
+            return FormValidation.ok();
+        }
     }
 }

src/main/resources/org/jenkinsci/plugins/oic/Messages.properties

@@ -16,3 +16,4 @@ OicSecurityRealm.UsingDefaultUsername = Using ''sub''.
 OicSecurityRealm.UsingDefaultScopes = Using ''openid email''.
 OicSecurityRealm.RUSureOpenIdNotInScope = Are you sure you don''t want to include ''openid'' as an scope?
 OicSecurityRealm.EndSessionURLKeyRequired = End Session URL Key is required.
+OicSecurityRealm.InvalidGroupsFieldName = Invalid groups field name - may only contain letters, numbers, and underscores and one instance of [].

src/main/resources/org/jenkinsci/plugins/oic/OicSecurityRealm/help-groupsFieldName.html

@@ -2,4 +2,7 @@
     Not required. If the field exists in the token and is an array of strings, then each string is added as a group.
     This allows groups based authorization in Jenkins. The SSO server will need to add the field with the list of groups in 
     the token. For example in Keycloak, this can be done with a 'Group Membership' mapper in the configuration of the client.
-</div>
+
+    If the SSO server adds the groups as an array of maps instead, then specify the group field as "groups[].name" where "groups"
+    is the field containing the array of maps, and "name" it the name of the key in the map that holds the group name.
+</div>

src/test/java/org/jenkinsci/plugins/oic/PluginTest.java

@@ -7,20 +7,24 @@
 import com.google.api.client.json.jackson2.JacksonFactory;
 import com.google.api.client.json.webtoken.JsonWebSignature;
 import com.google.api.client.json.webtoken.JsonWebToken;
+import com.google.api.client.util.ArrayMap;
 import com.google.gson.JsonElement;
 import hudson.model.User;
 import hudson.tasks.Mailer;
 import java.security.KeyPair;
 import java.security.KeyPairGenerator;
 import java.security.NoSuchAlgorithmException;
 import java.security.PrivateKey;
+import java.util.ArrayList;
 import java.util.Collections;
 import java.util.HashMap;
+import java.util.List;
 import java.util.Map;
 import java.util.concurrent.Callable;
 import jenkins.model.Jenkins;
 import org.acegisecurity.Authentication;
 import org.junit.Before;
+import org.junit.BeforeClass;
 import org.junit.Rule;
 import org.junit.Test;
 import org.jvnet.hudson.test.JenkinsRule;
@@ -61,6 +65,7 @@ public class PluginTest {
     private static final String TEST_USER_EMAIL_ADDRESS = "test@jenkins.oic";
     private static final String TEST_USER_FULL_NAME = "Oic Test User";
     private static final String[] TEST_USER_GROUPS = new String[]{"group1", "group2"};
+    private static final List<Map<String,String>> TEST_USER_GROUPS_MAP = new ArrayList<>();
     private static final String OPENID_CONNECT_USER_PROPERTY = "OpenID Connect user property";
 
     @Rule public WireMockRule wireMockRule = new WireMockRule(new WireMockConfiguration().dynamicPort(),true);
@@ -69,6 +74,12 @@ public class PluginTest {
     private JenkinsRule.WebClient webClient;
     private Jenkins jenkins;
 
+    @BeforeClass
+    public static void oneTimeSetUp() {
+        TEST_USER_GROUPS_MAP.add(ArrayMap.<String,String>of("id", "id1", "name", "group1" ));
+        TEST_USER_GROUPS_MAP.add(ArrayMap.<String,String>of("id", "id2", "name", "group2" ));
+    }
+
     @Before
     public void setUp() {
         jenkins = jenkinsRule.getInstance();
@@ -241,6 +252,56 @@ public void setUp() {
                 .withQueryParam("nonce", absent()));
     }
 
+    @Test public void testLoginUsingUserInfoEndpointWithGroupsMap() throws Exception {
+        wireMockRule.resetAll();
+
+        KeyPair keyPair = createKeyPair();
+
+        wireMockRule.stubFor(get(urlPathEqualTo("/authorization")).willReturn(
+                aResponse()
+                        .withStatus(302)
+                        .withHeader("Content-Type", "text/html; charset=utf-8")
+                        .withHeader("Location", jenkins.getRootUrl()+"securityRealm/finishLogin?state=state&code=code")
+                        .withBody("")
+        ));
+        wireMockRule.stubFor(post(urlPathEqualTo("/token")).willReturn(
+                aResponse()
+                        .withHeader("Content-Type", "application/json")
+                        .withBody("{" +
+                                "\"id_token\": \""+createIdToken(keyPair.getPrivate(),Collections.<String,Object>emptyMap())+"\"," +
+                                "\"access_token\":\"AcCeSs_ToKeN\"," +
+                                "\"token_type\":\"example\"," +
+                                "\"expires_in\":3600," +
+                                "\"refresh_token\":\"ReFrEsH_ToKeN\"," +
+                                "\"example_parameter\":\"example_value\"" +
+                                "}")
+        ));
+        wireMockRule.stubFor(get(urlPathEqualTo("/userinfo")).willReturn(
+                aResponse()
+                    .withHeader("Content-Type", "application/json")
+                    .withBody("{\n" +
+                            "   \"sub\": \""+TEST_USER_USERNAME+"\",\n" +
+                            "   \""+FULL_NAME_FIELD+"\": \""+TEST_USER_FULL_NAME+"\",\n" +
+                            "   \""+EMAIL_FIELD+"\": \""+TEST_USER_EMAIL_ADDRESS+"\",\n" +
+                            "   \""+GROUPS_FIELD+"\": "+toJsonArray(TEST_USER_GROUPS_MAP)+"\n" +
+                            "  }")
+        ));
+
+        System.out.println("jsonarray : " + toJsonArray(TEST_USER_GROUPS_MAP ));
+        jenkins.setSecurityRealm(new TestRealm(wireMockRule, "http://localhost:" + wireMockRule.port() + "/userinfo", "email", "groups[].name"));
+        assertEquals("Shouldn't be authenticated", getAuthentication().getPrincipal(), Jenkins.ANONYMOUS.getPrincipal());
+
+        webClient.goTo(jenkins.getSecurityRealm().getLoginUrl());
+
+        Authentication authentication = getAuthentication();
+        assertEquals("Should be logged-in as "+ TEST_USER_USERNAME, authentication.getPrincipal(), TEST_USER_USERNAME);
+        User user = User.get(String.valueOf(authentication.getPrincipal()));
+        assertEquals("Full name should be "+TEST_USER_FULL_NAME, TEST_USER_FULL_NAME, user.getFullName());
+        assertEquals("Email should be "+ TEST_USER_EMAIL_ADDRESS, TEST_USER_EMAIL_ADDRESS, user.getProperty(Mailer.UserProperty.class).getAddress());
+        assertTrue("User should be part of group "+ TEST_USER_GROUPS_MAP.get(0).get("name"), user.getAuthorities().contains(TEST_USER_GROUPS_MAP.get(0).get("name")));
+        assertTrue("User should be part of group "+ TEST_USER_GROUPS_MAP.get(1).get("name"), user.getAuthorities().contains(TEST_USER_GROUPS_MAP.get(1).get("name")));
+    }
+
     @Test public void testLoginWithMinimalConfiguration() throws Exception {
         KeyPair keyPair = createKeyPair();
 
@@ -1063,6 +1124,28 @@ private String toJsonArray(String[] array) {
         return builder.toString();
     }
 
+    private String toJsonArray(List<Map<String,String>> list) {
+        StringBuilder builder = new StringBuilder();
+        builder.append("[");
+
+        for(Map<String,String> entry : list) {
+            builder.append("{");
+            for ( String key :entry.keySet() ) {
+                builder.append("\"").append(key).append("\": ");
+                builder.append("\"").append(entry.get(key)).append("\",");
+            }
+            if(builder.lastIndexOf(",") != -1) {
+                builder.deleteCharAt(builder.length()-1);
+            }
+            builder.append("},");
+        }
+        if(builder.lastIndexOf(",") != -1) {
+            builder.deleteCharAt(builder.length()-1);
+        }
+        builder.append("]");
+        return builder.toString();
+    }
+
     private KeyPair createKeyPair() throws NoSuchAlgorithmException {
         KeyPairGenerator keyGen = KeyPairGenerator.getInstance("RSA");
         keyGen.initialize(2048);