Allow SecurityRealm#getUserIdStrategy to be customizable

This way, the plugin can be aligned with the ID strategy of the configured Identity Provider.
jenkinsci · Dec 23, 2024 · 1e74b6d · 1e74b6d
1 parent 9b84856
commit 1e74b6d
Show file tree

Hide file tree

Showing 8 changed files with 163 additions and 15 deletions.
diff --git a/docs/configuration/README.md b/docs/configuration/README.md
@@ -69,11 +69,12 @@ If the JWKS endpoint is configured, JWS' signatures will be verified
 Providers have some variation in their implementation of OpenID Connect
 or some oddities they required.
 
-| field                     | format   | description                                                                                         |
-|---------------------------|----------|-----------------------------------------------------------------------------------------------------|
-| logoutFromOpenidProvider  | boolean  | Enable the logout from provider when user logout from Jenkisn.                                      |
-| sendScopesInTokenRequest  | boolean  | Some providers expects scopes to be sent in token request                                           |
-| rootURLFromRequest        | boolean  | When computing Jenkins redirect, the root url is either deduced from configured root url or request |
+| field                    | format  | description                                                                                         |
+|--------------------------|---------|-----------------------------------------------------------------------------------------------------|
+| logoutFromOpenidProvider | boolean | Enable the logout from provider when user logout from Jenkins.                                      |
+| sendScopesInTokenRequest | boolean | Some providers expects scopes to be sent in token request                                           |
+| rootURLFromRequest       | boolean | When computing Jenkins redirect, the root url is either deduced from configured root url or request |
+| customUserIdStrategy     | enum    | ID strategy that should be used for turning the user name into an ID. Default is: case-insensitive. |
 
 ### Security configuration
 
@@ -142,6 +143,7 @@ jenkins:
       rootURLFromRequest: <boolean>
       sendScopesInTokenRequest: <boolean>
       postLogoutRedirectUrl: <url>
+      customUserIdStrategy: <string:enum>
       # Security
       allowTokenAccessWithoutOicSession: <boolean>
       allowedTokenExpirationClockSkewSeconds: <integer>

diff --git a/src/main/java/org/jenkinsci/plugins/oic/OicSecurityRealm.java b/src/main/java/org/jenkinsci/plugins/oic/OicSecurityRealm.java
@@ -88,6 +88,7 @@
 import javax.servlet.http.HttpServletRequest;
 import javax.servlet.http.HttpServletResponse;
 import javax.servlet.http.HttpSession;
+import jenkins.model.IdStrategy;
 import jenkins.model.Jenkins;
 import jenkins.security.ApiTokenProperty;
 import jenkins.security.FIPS140;
@@ -133,6 +134,7 @@
 import org.springframework.security.crypto.bcrypt.BCrypt;
 import org.springframework.util.Assert;
 
+import static jenkins.model.IdStrategy.CASE_INSENSITIVE;
 import static org.apache.commons.lang.StringUtils.isNotBlank;
 
 /**
@@ -159,7 +161,19 @@ public static enum TokenAuthMethod {
         ClientAuthenticationMethod toClientAuthenticationMethod() {
             return clientAuthMethod;
         }
-    };
+    }
+
+    public enum UserIdStrategy {
+        defaultCase(CASE_INSENSITIVE),
+        caseInSensitive(CASE_INSENSITIVE),
+        caseSensitive(new IdStrategy.CaseSensitive());
+
+        final IdStrategy idStrategy;
+
+        UserIdStrategy(IdStrategy idStrategy) {
+            this.idStrategy = idStrategy;
+        }
+    }
 
     private static final String ID_TOKEN_REQUEST_ATTRIBUTE = "oic-id-token";
     private static final String NO_SECRET = "none";
@@ -192,6 +206,7 @@ ClientAuthenticationMethod toClientAuthenticationMethod() {
     @Deprecated
     private transient String userInfoServerUrl;
 
+    private transient UserIdStrategy customUserIdStrategy = UserIdStrategy.defaultCase;
     private String userNameField = "sub";
     private transient Expression<Object> userNameFieldExpr = null;
     private String tokenFieldToCheckKey = null;
@@ -323,6 +338,11 @@ public OicSecurityRealm(
         this.serverConfiguration = serverConfiguration;
     }
 
+    @Override
+    public IdStrategy getUserIdStrategy() {
+        return customUserIdStrategy == null ? UserIdStrategy.defaultCase.idStrategy : customUserIdStrategy.idStrategy;
+    }
+
     @SuppressWarnings("deprecated")
     protected Object readResolve() throws ObjectStreamException {
         // Fail if migrating to a FIPS non-compliant config
@@ -410,6 +430,10 @@ public OicServerConfiguration getServerConfiguration() {
         return serverConfiguration;
     }
 
+    public UserIdStrategy getCustomUserIdStrategy() {
+        return customUserIdStrategy;
+    }
+
     public String getUserNameField() {
         return userNameField;
     }
@@ -689,6 +713,11 @@ protected OidcClient buildOidcClient() {
         return client;
     }
 
+    @DataBoundSetter
+    public void setCustomUserIdStrategy(UserIdStrategy customUserIdStrategy) {
+        this.customUserIdStrategy = customUserIdStrategy;
+    }
+
     @DataBoundSetter
     public void setUserNameField(String userNameField) {
         this.userNameField = Util.fixNull(Util.fixEmptyAndTrim(userNameField), "sub");

diff --git a/src/main/resources/org/jenkinsci/plugins/oic/OicSecurityRealm/config.jelly b/src/main/resources/org/jenkinsci/plugins/oic/OicSecurityRealm/config.jelly
@@ -35,6 +35,12 @@
     <f:entry title="${%PostLogoutRedirectUrl}" field="postLogoutRedirectUrl" class="endSessionConfig">
       <f:textbox />
     </f:entry>
+    <f:entry title="${%IdStrategy}" field="customUserIdStrategy">
+      <f:radioBlock title="${%IdStrategyCaseInSensitive}" name="customUserIdStrategy"
+                    checked="${instance.customUserIdStrategy == null || instance.customUserIdStrategy == 'caseInSensitive'}" value="caseInSensitive" inline="true" help="${null}"/>
+      <f:radioBlock title="${%IdStrategyCaseSensitive}" name="customUserIdStrategy"
+                    checked="${instance.customUserIdStrategy == 'caseSensitive'}" value="caseSensitive" inline="true" help="${null}"/>
+    </f:entry>
 
     <f:advanced title="${%SecurityConfiguration}">
       <f:entry title="${%TokenFieldKeyToCheck}" field="tokenFieldToCheckKey">

diff --git a/src/main/resources/org/jenkinsci/plugins/oic/OicSecurityRealm/config.properties b/src/main/resources/org/jenkinsci/plugins/oic/OicSecurityRealm/config.properties
@@ -14,6 +14,9 @@ EnablePKCE=Enable Proof Key for Code Exchange (PKCE)
 FullnameFieldName=Full name field name
 Group=Group
 GroupsFieldName=Groups field name
+IdStrategy=ID Strategy
+IdStrategyCaseInSensitive=Case Insensitive Strategy
+IdStrategyCaseSensitive=Case Sensitive Strategy
 LogoutFromOpenIDProvider=Logout from OpenID Provider
 PostLogoutRedirectUrl=Post logout redirect URL
 Secret=Secret
@@ -24,5 +27,5 @@ TokenFieldValueToCheck=Token Field Value To Check
 UseRootUrlFromRequest=Use Root URL from request
 UserFields=User fields
 Username=Username
-UsernameFieldName=User name field name
+UsernameFieldName=Username field name
 WellknownConfigurationEndpoint=Well-known configuration endpoint
diff --git a/src/main/resources/org/jenkinsci/plugins/oic/OicSecurityRealm/help-customUserIdStrategy.html b/src/main/resources/org/jenkinsci/plugins/oic/OicSecurityRealm/help-customUserIdStrategy.html
@@ -0,0 +1,5 @@
+<div>
+    Required. ID strategy that should be used for turning the username into an ID. Default is: case-insensitive.
+    <br>
+    The ID strategy should match with the username case-sensitivity handling of the configured Identity Provider.
+</div>
diff --git a/src/test/java/org/jenkinsci/plugins/oic/PluginTest.java b/src/test/java/org/jenkinsci/plugins/oic/PluginTest.java
@@ -134,13 +134,30 @@ public void setUp() {
     @Test
     public void testLoginWithDefaults() throws Exception {
         mockAuthorizationRedirectsToFinishLogin();
-        mockTokenReturnsIdTokenWithGroup();
+
+        String usernameLower = "my-user";
+        String[] groupsLower = {"group-1", "group-2"};
+        String usernameUpper = usernameLower.toUpperCase();
+        String[] groupsUpper =
+                Arrays.stream(groupsLower).map(String::toUpperCase).toArray(String[]::new);
+
+        // Login user
+        mockTokenReturnsIdTokenWithGroup(usernameLower, setUpKeyValuesWithGroup(groupsLower));
         configureTestRealm(sc -> {});
+        assertEquals(
+                OicSecurityRealm.UserIdStrategy.defaultCase,
+                ((OicSecurityRealm) jenkins.getSecurityRealm()).getCustomUserIdStrategy());
+        assertEquals(
+                OicSecurityRealm.UserIdStrategy.caseInSensitive.idStrategy,
+                jenkins.getSecurityRealm().getUserIdStrategy());
+        assertEquals(
+                OicSecurityRealm.UserIdStrategy.caseInSensitive.idStrategy,
+                jenkins.getSecurityRealm().getGroupIdStrategy());
         assertAnonymous();
         browseLoginPage();
-        var user = assertTestUser();
-        assertTestUserEmail(user);
-        assertTestUserIsMemberOfTestGroups(user);
+        var userLower = assertTestUser(usernameLower);
+        assertTestUserEmail(userLower);
+        assertTestUserIsMemberOfGroups(userLower, groupsLower);
 
         verify(getRequestedFor(urlPathEqualTo("/authorization"))
                 .withQueryParam("scope", equalTo("openid email"))
@@ -151,6 +168,66 @@ public void testLoginWithDefaults() throws Exception {
             assertNotNull(((OicSecurityRealm) Jenkins.get().getSecurityRealm()).getStateAttribute(session));
             return null;
         });
+
+        // Use Default: ID Strategy CASE-INSENSITIVE
+        // Login another user with same username but in upper cases
+        mockTokenReturnsIdTokenWithGroup(usernameUpper, setUpKeyValuesWithGroup(groupsUpper));
+        browseLoginPage();
+        var userUpperCaseInsensitive = assertTestUser(usernameUpper);
+        assertEquals(
+                "With ID strategy case-insensitive, the username is to be expected in lower case",
+                usernameLower,
+                userUpperCaseInsensitive.getId());
+        assertTestUserIsMemberOfGroups(userUpperCaseInsensitive, groupsUpper);
+    }
+
+    @Test
+    public void testLoginWithDefaultsCaseSensitive() throws Exception {
+        mockAuthorizationRedirectsToFinishLogin();
+
+        String usernameLower = "my-user";
+        String[] groupsLower = {"group-1", "group-2"};
+        String usernameUpper = usernameLower.toUpperCase();
+        String[] groupsUpper =
+                Arrays.stream(groupsLower).map(String::toUpperCase).toArray(String[]::new);
+
+        // Login user
+        mockTokenReturnsIdTokenWithGroup(usernameLower, setUpKeyValuesWithGroup(groupsLower));
+        configureTestRealm(sc -> sc.setCustomUserIdStrategy(OicSecurityRealm.UserIdStrategy.caseSensitive));
+        assertEquals(
+                OicSecurityRealm.UserIdStrategy.caseSensitive,
+                ((OicSecurityRealm) jenkins.getSecurityRealm()).getCustomUserIdStrategy());
+        assertEquals(
+                OicSecurityRealm.UserIdStrategy.caseSensitive.idStrategy,
+                jenkins.getSecurityRealm().getUserIdStrategy());
+        assertEquals(
+                OicSecurityRealm.UserIdStrategy.caseSensitive.idStrategy,
+                jenkins.getSecurityRealm().getGroupIdStrategy());
+        assertAnonymous();
+        browseLoginPage();
+        var userLower = assertTestUser(usernameLower);
+        assertTestUserEmail(userLower);
+        assertTestUserIsMemberOfGroups(userLower, groupsLower);
+
+        verify(getRequestedFor(urlPathEqualTo("/authorization"))
+                .withQueryParam("scope", equalTo("openid email"))
+                .withQueryParam("nonce", matching(".+")));
+        verify(postRequestedFor(urlPathEqualTo("/token")).withRequestBody(notMatching(".*&scope=.*")));
+        webClient.executeOnServer(() -> {
+            HttpSession session = Stapler.getCurrentRequest().getSession();
+            assertNotNull(((OicSecurityRealm) Jenkins.get().getSecurityRealm()).getStateAttribute(session));
+            return null;
+        });
+
+        // Login another user with same username but in upper cases
+        mockTokenReturnsIdTokenWithGroup(usernameUpper, setUpKeyValuesWithGroup(groupsUpper));
+        browseLoginPage();
+        var userUpperCase = assertTestUser(usernameUpper);
+        assertEquals(
+                "With ID strategy case-insensitive, the username is to be expected in lower case",
+                usernameUpper,
+                userUpperCase.getId());
+        assertTestUserIsMemberOfGroups(userUpperCase, groupsUpper);
     }
 
     @Test
@@ -834,6 +911,14 @@ private static void assertTestUserEmail(User user) {
                 user.getProperty(Mailer.UserProperty.class).getAddress());
     }
 
+    private @NonNull User assertTestUser(String userName) {
+        Authentication authentication = getAuthentication();
+        assertEquals("Should be logged-in as " + userName, userName, authentication.getPrincipal());
+        User user = toUser(authentication);
+        assertEquals("Full name should be " + TEST_USER_FULL_NAME, TEST_USER_FULL_NAME, user.getFullName());
+        return user;
+    }
+
     private @NonNull User assertTestUser() {
         Authentication authentication = getAuthentication();
         assertEquals("Should be logged-in as " + TEST_USER_USERNAME, TEST_USER_USERNAME, authentication.getPrincipal());
@@ -1028,15 +1113,16 @@ private KeyPair createKeyPair() throws NoSuchAlgorithmException {
         return keyGen.generateKeyPair();
     }
 
-    private String createIdToken(PrivateKey privateKey, Map<String, Object> keyValues) throws Exception {
+    private String createIdToken(String username, PrivateKey privateKey, Map<String, Object> keyValues)
+            throws Exception {
         JsonWebSignature.Header header =
                 new JsonWebSignature.Header().setAlgorithm("RS256").setKeyId("jwks_key_id");
         long now = Clock.systemUTC().millis() / 1000;
         IdToken.Payload payload = new IdToken.Payload()
                 .setExpirationTimeSeconds(now + 60L)
                 .setIssuedAtTimeSeconds(now)
                 .setIssuer(TestRealm.ISSUER)
-                .setSubject(TEST_USER_USERNAME)
+                .setSubject(username)
                 .setAudience(Collections.singletonList(TestRealm.CLIENT_ID))
                 .setNonce("nonce");
         for (Map.Entry<String, Object> keyValue : keyValues.entrySet()) {
@@ -1302,14 +1388,29 @@ private void mockTokenReturnsIdTokenWithValues(Map<String, Object> keyValues) th
     }
 
     private void mockTokenReturnsIdTokenWithValues(Map<String, Object> keyValues, KeyPair keyPair) throws Exception {
-        mockTokenReturnsIdToken(createIdToken(keyPair.getPrivate(), keyValues));
+        mockTokenReturnsIdToken(createIdToken(TEST_USER_USERNAME, keyPair.getPrivate(), keyValues));
+    }
+
+    private void mockTokenReturnsIdTokenWithGroup(String username, Map<String, Object> keyValuesWithGroups)
+            throws Exception {
+        var keyPair = createKeyPair();
+        mockTokenReturnsIdToken(createIdToken(username, keyPair.getPrivate(), keyValuesWithGroups));
     }
 
     @SafeVarargs
     private void mockTokenReturnsIdTokenWithGroup(@CheckForNull Consumer<Map<String, String>>... tokenAcceptors)
             throws Exception {
         var keyPair = createKeyPair();
-        mockTokenReturnsIdToken(createIdToken(keyPair.getPrivate(), setUpKeyValuesWithGroup()), tokenAcceptors);
+        mockTokenReturnsIdToken(
+                createIdToken(TEST_USER_USERNAME, keyPair.getPrivate(), setUpKeyValuesWithGroup()), tokenAcceptors);
+    }
+
+    @SafeVarargs
+    private void mockTokenReturnsIdTokenWithGroup(
+            String username, @CheckForNull Consumer<Map<String, String>>... tokenAcceptors) throws Exception {
+        var keyPair = createKeyPair();
+        mockTokenReturnsIdToken(
+                createIdToken(username, keyPair.getPrivate(), setUpKeyValuesWithGroup()), tokenAcceptors);
     }
 
     private void mockTokenReturnsIdToken(@CheckForNull String idToken) {

diff --git a/src/test/resources/org/jenkinsci/plugins/oic/ConfigurationAsCode.yml b/src/test/resources/org/jenkinsci/plugins/oic/ConfigurationAsCode.yml
@@ -11,6 +11,7 @@ jenkins:
           scopes: scopes
       clientId: clientId
       clientSecret: clientSecret
+      customUserIdStrategy: caseInSensitive
       disableSslVerification: true
       emailFieldName: emailFieldName
       escapeHatchEnabled: true

diff --git a/src/test/resources/org/jenkinsci/plugins/oic/ConfigurationAsCodeExport.yml b/src/test/resources/org/jenkinsci/plugins/oic/ConfigurationAsCodeExport.yml
@@ -1,4 +1,5 @@
 clientId: "clientId"
+customUserIdStrategy: caseInSensitive
 disableSslVerification: true
 emailFieldName: "emailFieldName"
 escapeHatchEnabled: true