Fix 2.6 regression on group field and deprecate Jackson parser (fix #236

)
jenkinsci · Mar 19, 2024 · 85274cd · 85274cd
1 parent 503b9a7
commit 85274cd
Show file tree

Hide file tree

Showing 5 changed files with 38 additions and 37 deletions.
diff --git a/pom.xml b/pom.xml
@@ -23,7 +23,7 @@
 
   <scm>
     <connection>scm:git:ssh://github.com/${gitHubRepo}.git</connection>
-    <developerConnection>scm:git:ssh://[email protected]/${gitHubRepo}.git</developerConnection>
+    <developerConnection>scm:git:[email protected]/${gitHubRepo}.git</developerConnection>
     <url>https://github.com/${gitHubRepo}</url>
     <tag>oic-auth-3.0</tag>
   </scm>
@@ -58,7 +58,7 @@
     </dependency>
     <dependency>
       <groupId>com.google.http-client</groupId>
-      <artifactId>google-http-client-jackson2</artifactId>
+      <artifactId>google-http-client-gson</artifactId>
       <version>1.44.1</version>
     </dependency>
     <dependency>

diff --git a/src/main/java/org/jenkinsci/plugins/oic/OicSecurityRealm.java b/src/main/java/org/jenkinsci/plugins/oic/OicSecurityRealm.java
@@ -23,7 +23,7 @@
 */
 package org.jenkinsci.plugins.oic;
 
-import com.fasterxml.jackson.core.JsonParseException;
+import com.google.gson.JsonParseException;
 import com.google.api.client.auth.oauth2.AuthorizationCodeFlow;
 import com.google.api.client.auth.oauth2.AuthorizationCodeTokenRequest;
 import com.google.api.client.auth.oauth2.BearerToken;
@@ -41,9 +41,8 @@
 import com.google.api.client.http.HttpTransport;
 import com.google.api.client.http.javanet.NetHttpTransport;
 import com.google.api.client.json.GenericJson;
-import com.google.api.client.json.JsonFactory;
 import com.google.api.client.json.JsonObjectParser;
-import com.google.api.client.json.jackson2.JacksonFactory;
+import com.google.api.client.json.gson.GsonFactory;
 import com.google.api.client.json.webtoken.JsonWebSignature;
 import com.google.api.client.util.ArrayMap;
 import com.google.api.client.util.Data;
@@ -123,12 +122,10 @@ public class OicSecurityRealm extends SecurityRealm {
     private static final Logger LOGGER = Logger.getLogger(OicSecurityRealm.class.getName());
     public static enum TokenAuthMethod { client_secret_basic, client_secret_post };
 
-    private static final JsonFactory JSON_FACTORY = new JacksonFactory();
     private static final String ID_TOKEN_REQUEST_ATTRIBUTE = "oic-id-token";
     private static final String STATE_REQUEST_ATTRIBUTE = "oic-state";
     private static final String NO_SECRET = "none";
 
-
     private final String clientId;
     private final Secret clientSecret;
     private String wellKnownOpenIDConfigurationUrl = null;
@@ -142,8 +139,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 transient String simpleGroupsFieldName = null;
+    private transient String nestedGroupFieldName = null;
     private String scopes = null;
     private final boolean disableSslVerification;
     private boolean logoutFromOpenidProvider = true;
@@ -191,6 +188,9 @@ public static enum TokenAuthMethod { client_secret_basic, client_secret_post };
     private transient String endSessionUrl;
 
     private transient HttpTransport httpTransport;
+
+    /** Random generator needed for robust random wait
+     */
     private static final Random RANDOM = new Random();
 
     /**
@@ -227,11 +227,11 @@ public OicSecurityRealm(String clientId, String clientSecret, String wellKnownOp
             this.wellKnownOpenIDConfigurationUrl = null;  // Remove the autoconfig URL
         }
 
-        this.tokenFieldToCheckKey = Util.fixEmpty(tokenFieldToCheckKey);
-        this.tokenFieldToCheckValue = Util.fixEmpty(tokenFieldToCheckValue);
-        this.userNameField = Util.fixEmpty(userNameField) == null ? "sub" : userNameField;
-        this.fullNameFieldName = Util.fixEmpty(fullNameFieldName);
-        this.emailFieldName = Util.fixEmpty(emailFieldName);
+        this.setTokenFieldToCheckKey(Util.fixEmpty(tokenFieldToCheckKey));
+        this.setTokenFieldToCheckValue(Util.fixEmpty(tokenFieldToCheckValue));
+        this.setUserNameField(Util.fixEmpty(userNameField) == null ? "sub" : userNameField);
+        this.setFullNameFieldName(Util.fixEmpty(fullNameFieldName));
+        this.setEmailFieldName(Util.fixEmpty(emailFieldName));
         this.setGroupsFieldName(Util.fixEmpty(groupsFieldName));
         this.logoutFromOpenidProvider = Util.fixNull(logoutFromOpenidProvider, Boolean.TRUE);
         this.postLogoutRedirectUrl = postLogoutRedirectUrl;
@@ -276,6 +276,17 @@ protected Object readResolve() {
                 LOGGER.log(Level.SEVERE, "Can't set endSessionEndpoint from old value", e);
             }
         }
+
+        // backward compatibility with wrong groupsFieldName split
+        if(Strings.isNullOrEmpty(this.groupsFieldName) && !Strings.isNullOrEmpty(this.simpleGroupsFieldName)) {
+            String originalGroupFieldName = this.simpleGroupsFieldName;
+            if(!Strings.isNullOrEmpty(this.nestedGroupFieldName)) {
+                originalGroupFieldName += "[]." + this.nestedGroupFieldName;
+            }
+            this.setGroupsFieldName(originalGroupFieldName);
+        } else {
+            this.setGroupsFieldName(this.groupsFieldName);
+        }
         return this;
     }
 
@@ -435,7 +446,7 @@ private void loadWellKnownOpenIDConfigurationUrl() {
                 .createRequestFactory().buildGetRequest(new GenericUrl(url));
 
             com.google.api.client.http.HttpResponse response = request.execute();
-            WellKnownOpenIDConfigurationResponse config = OicSecurityRealm.JSON_FACTORY
+            WellKnownOpenIDConfigurationResponse config = GsonFactory.getDefaultInstance()
                 .fromInputStream(response.getContent(), Charset.defaultCharset(),
                         WellKnownOpenIDConfigurationResponse.class);
 
@@ -716,7 +727,7 @@ protected AuthorizationCodeFlow buildAuthorizationCodeFlow() {
         AuthorizationCodeFlow.Builder builder = new AuthorizationCodeFlow.Builder(
                 tokenAccessMethod,
                 httpTransport,
-                JSON_FACTORY,
+                GsonFactory.getDefaultInstance(),
                 new GenericUrl(tokenServerUrl),
                 authInterceptor,
                 clientId,
@@ -1220,7 +1231,7 @@ public FormValidation doCheckWellKnownOpenIDConfigurationUrl(@QueryParameter Str
 
                 // Try to parse the response. If it's not valid, a JsonParseException will be thrown indicating
                 // that it's not a valid JSON describing an OpenID Connect endpoint
-                WellKnownOpenIDConfigurationResponse config = OicSecurityRealm.JSON_FACTORY
+                WellKnownOpenIDConfigurationResponse config = GsonFactory.getDefaultInstance()
                         .fromInputStream(response.getContent(), Charset.defaultCharset(),
                                 WellKnownOpenIDConfigurationResponse.class);
                 if(config.getAuthorizationEndpoint() == null || config.getTokenEndpoint() == null) {

diff --git a/src/test/java/org/jenkinsci/plugins/oic/OicTokenResponseTest.java b/src/test/java/org/jenkinsci/plugins/oic/OicTokenResponseTest.java
@@ -1,7 +1,6 @@
 package org.jenkinsci.plugins.oic;
 
-import com.google.api.client.json.JsonFactory;
-import com.google.api.client.json.jackson2.JacksonFactory;
+import com.google.api.client.json.gson.GsonFactory;
 import java.io.IOException;
 import org.junit.Test;
 
@@ -31,8 +30,7 @@ public class OicTokenResponseTest {
 
     @Test
     public void parseLongLiteral() throws IOException {
-        JsonFactory jsonFactory = new JacksonFactory();
-        OicTokenResponse response = jsonFactory.fromString(JSON_WITH_LONG_LITERAL, OicTokenResponse.class);
+        OicTokenResponse response = GsonFactory.getDefaultInstance().fromString(JSON_WITH_LONG_LITERAL, OicTokenResponse.class);
         assertEquals("2YotnFZFEjr1zCsicMWpAA", response.getAccessToken());
         assertEquals("example", response.getTokenType());
         assertEquals(3600L, response.getExpiresInSeconds().longValue());
@@ -42,8 +40,7 @@ public void parseLongLiteral() throws IOException {
 
     @Test
     public void parseStringWithLong() throws IOException {
-        JsonFactory jsonFactory = new JacksonFactory();
-        OicTokenResponse response = jsonFactory.fromString(JSON_WITH_LONG_AS_STRING, OicTokenResponse.class);
+        OicTokenResponse response = GsonFactory.getDefaultInstance().fromString(JSON_WITH_LONG_AS_STRING, OicTokenResponse.class);
         assertEquals("2YotnFZFEjr1zCsicMWpAA", response.getAccessToken());
         assertEquals("example", response.getTokenType());
         assertEquals(3600L, response.getExpiresInSeconds().longValue());
@@ -81,8 +78,7 @@ public void testSetters() throws IOException {
 
     @Test
     public void parseAbsent() throws IOException {
-        JsonFactory jsonFactory = new JacksonFactory();
-        OicTokenResponse response = jsonFactory.fromString(JSON_WITH_ABSENT, OicTokenResponse.class);
+        OicTokenResponse response = GsonFactory.getDefaultInstance().fromString(JSON_WITH_ABSENT, OicTokenResponse.class);
         assertEquals("2YotnFZFEjr1zCsicMWpAA", response.getAccessToken());
         assertEquals("example", response.getTokenType());
         assertEquals(null, response.getExpiresInSeconds());

diff --git a/src/test/java/org/jenkinsci/plugins/oic/PluginTest.java b/src/test/java/org/jenkinsci/plugins/oic/PluginTest.java
@@ -3,8 +3,7 @@
 import com.github.tomakehurst.wiremock.core.WireMockConfiguration;
 import com.github.tomakehurst.wiremock.junit.WireMockRule;
 import com.google.api.client.auth.openidconnect.IdToken;
-import com.google.api.client.json.JsonFactory;
-import com.google.api.client.json.jackson2.JacksonFactory;
+import com.google.api.client.json.gson.GsonFactory;
 import com.google.api.client.json.webtoken.JsonWebSignature;
 import com.google.api.client.json.webtoken.JsonWebToken;
 import com.google.api.client.util.ArrayMap;
@@ -59,8 +58,6 @@
  */
 @Url("https://jenkins.io/blog/2018/01/13/jep-200/")
 public class PluginTest {
-    private static final JsonFactory JSON_FACTORY = new JacksonFactory();
-
     private static final String TEST_USER_USERNAME = "testUser";
     private static final String TEST_USER_EMAIL_ADDRESS = "[email protected]";
     private static final String TEST_USER_FULL_NAME = "Oic Test User";
@@ -1164,7 +1161,7 @@ private String createIdToken(PrivateKey privateKey, Map<String, Object> keyValue
             payload.set(keyValue.getKey(), keyValue.getValue());
         }
 
-        return JsonWebSignature.signUsingRsaSha256(privateKey, JSON_FACTORY, header, payload);
+        return JsonWebSignature.signUsingRsaSha256(privateKey, GsonFactory.getDefaultInstance(), header, payload);
     }
 
     private String createUserInfoJWT(PrivateKey privateKey, String userInfo) throws Exception {
@@ -1177,7 +1174,7 @@ private String createUserInfoJWT(PrivateKey privateKey, String userInfo) throws
             payload.set(keyValue.getKey(), keyValue.getValue().getAsString());
         }
 
-        return JsonWebSignature.signUsingRsaSha256(privateKey, JSON_FACTORY, header, payload);
+        return JsonWebSignature.signUsingRsaSha256(privateKey, GsonFactory.getDefaultInstance(), header, payload);
     }
 
     /**

diff --git a/src/test/java/org/jenkinsci/plugins/oic/WellKnownOpenIDConfigurationResponseTest.java b/src/test/java/org/jenkinsci/plugins/oic/WellKnownOpenIDConfigurationResponseTest.java
@@ -1,7 +1,6 @@
 package org.jenkinsci.plugins.oic;
 
-import com.google.api.client.json.JsonFactory;
-import com.google.api.client.json.jackson2.JacksonFactory;
+import com.google.api.client.json.gson.GsonFactory;
 import java.io.IOException;
 import org.junit.Test;
 
@@ -13,8 +12,6 @@
 
 public class WellKnownOpenIDConfigurationResponseTest {
 
-    private final JsonFactory jsonFactory = new JacksonFactory();
-
     private static final String JSON_FROM_GOOGLE = "{"
            + " \"issuer\": \"https://accounts.google.com\","
            + " \"authorization_endpoint\": \"https://accounts.google.com/o/oauth2/v2/auth\","
@@ -69,7 +66,7 @@ public class WellKnownOpenIDConfigurationResponseTest {
 
     @Test
     public void parseExplicitKeys() throws IOException {
-        WellKnownOpenIDConfigurationResponse response = jsonFactory.fromString(JSON_FROM_GOOGLE, WellKnownOpenIDConfigurationResponse.class);
+        WellKnownOpenIDConfigurationResponse response = GsonFactory.getDefaultInstance().fromString(JSON_FROM_GOOGLE, WellKnownOpenIDConfigurationResponse.class);
 
         assertThat(response.getAuthorizationEndpoint(), is("https://accounts.google.com/o/oauth2/v2/auth"));
         assertThat(response.getTokenEndpoint(), is("https://www.googleapis.com/oauth2/v4/token"));
@@ -81,7 +78,7 @@ public void parseExplicitKeys() throws IOException {
 
     @Test
     public void parseWellKnownKeys() throws IOException {
-        WellKnownOpenIDConfigurationResponse response = jsonFactory.fromString(JSON_FROM_GOOGLE, WellKnownOpenIDConfigurationResponse.class);
+        WellKnownOpenIDConfigurationResponse response = GsonFactory.getDefaultInstance().fromString(JSON_FROM_GOOGLE, WellKnownOpenIDConfigurationResponse.class);
         assertThat(response.getKnownKeys().keySet(), containsInAnyOrder(
             "authorization_endpoint",
             "token_endpoint",