Add support for Microsoft AD FS

docs/configuration/ADFS.md

@@ -0,0 +1,26 @@
+# ADFS Provider
+
+[ADFS][1] can be used as as an OpenID Connect identity provider.
+
+## Provider configuration
+
+[This][2] stack overflow step though is a great resource, followed by [This IBM resource][3] for granting the correct permissions.
+
+Where the IBM resource adds 2 individual permissions, 3 are needed and can be performed in one command - e.g.
+`Set-AdfsApplicationPermission -TargetIdentifier fe56f061-c689-45e8-af8d-b8fdf5d1e60f -AddScope 'openid','aza','allatclaims'`
+
+Extra claims (for example users display name) can be added using a similar approach to the groups.
+
+## Plugin configuration
+
+ADFS provides a well known configuration endpoint which can be used for automating endpoint configuration.
+It also supports PKCE verification for additional security.
+
+### User information
+
+Without any extra claims, the user field should be set to `upn`
+
+
+[1]: https://learn.microsoft.com/en-us/windows-server/identity/ad-fs/ad-fs-overview
+[2]: https://stackoverflow.com/questions/55494354/user-groups-as-claims-through-openid-connect-over-adfs/55570286#55570286
+[3]: https://community.ibm.com/community/user/security/blogs/laurent-lapiquionne1/2020/07/21/how-to-configure-igi-service-center-to-authent

docs/configuration/README.md

@@ -9,6 +9,7 @@ There are specifics instructions for well known providers:
 
 * [Google Provider](GOOGLE.md)
 * [Gitlab Provider](GITLAB.md)
+* [Microsoft AD FS](ADFS.md)
 
 This page contains the reference of plugin's configuration.
 

pom.xml

@@ -112,6 +112,16 @@
       <version>4.5.14</version>
       <scope>test</scope>
     </dependency>
+    <dependency>
+      <groupId>org.mockito</groupId>
+      <artifactId>mockito-core</artifactId>
+      <scope>test</scope>
+    </dependency>
+    <dependency>
+      <groupId>org.mockito</groupId>
+      <artifactId>mockito-junit-jupiter</artifactId>
+      <scope>test</scope>
+    </dependency>
   </dependencies>
 
   <repositories>
@@ -129,6 +139,36 @@
   </pluginRepositories>
 
   <build>
+    <pluginManagement>
+      <plugins>
+        <!--This plugin's configuration is used to store Eclipse m2e settings only. It has no influence on the Maven build itself.-->
+        <plugin>
+          <groupId>org.eclipse.m2e</groupId>
+          <artifactId>lifecycle-mapping</artifactId>
+          <version>1.0.0</version>
+          <configuration>
+            <lifecycleMappingMetadata>
+              <pluginExecutions>
+                <pluginExecution>
+                  <pluginExecutionFilter>
+                    <!-- https://github.com/eclipse-m2e/m2e-core/issues/1291 -->
+                    <groupId>org.apache.maven.plugins</groupId>
+                    <artifactId>maven-checkstyle-plugin</artifactId>
+                    <versionRange>[3.4.0,)</versionRange>
+                    <goals>
+                      <goal>check</goal>
+                    </goals>
+                  </pluginExecutionFilter>
+                  <action>
+                    <ignore />
+                  </action>
+                </pluginExecution>
+              </pluginExecutions>
+            </lifecycleMappingMetadata>
+          </configuration>
+        </plugin>
+      </plugins>
+    </pluginManagement>
     <plugins>
       <plugin>
         <artifactId>maven-release-plugin</artifactId>

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

@@ -0,0 +1,34 @@
+package org.jenkinsci.plugins.oic;
+
+import hudson.Extension;
+import hudson.security.SecurityRealm;
+import hudson.security.csrf.CrumbExclusion;
+import java.io.IOException;
+import javax.servlet.FilterChain;
+import javax.servlet.ServletException;
+import javax.servlet.http.HttpServletRequest;
+import javax.servlet.http.HttpServletResponse;
+import jenkins.model.Jenkins;
+
+/**
+ * Crumb exclusion to allow POSTing to {@link OicSecurityRealm#doFinishLogin(org.kohsuke.stapler.StaplerRequest)}
+ */
+@Extension
+public class OicCrumbExclusion extends CrumbExclusion {
+
+    @Override
+    public boolean process(HttpServletRequest request, HttpServletResponse response, FilterChain chain)
+            throws IOException, ServletException {
+        Jenkins j = Jenkins.getInstanceOrNull();
+        if (j != null) {
+            SecurityRealm sr = j.getSecurityRealm();
+            if (sr instanceof OicSecurityRealm) {
+                if ("/securityRealm/finishLogin".equals(request.getPathInfo())) {
+                    chain.doFilter(request, response);
+                    return true;
+                }
+            }
+        }
+        return false;
+    }
+}

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

@@ -38,12 +38,16 @@
 import java.security.MessageDigest;
 import java.security.NoSuchAlgorithmException;
 import java.security.SecureRandom;
+import java.util.Map;
 import java.util.UUID;
 import javax.servlet.http.HttpSession;
+import org.kohsuke.accmod.Restricted;
+import org.kohsuke.accmod.restrictions.DoNotUse;
 import org.kohsuke.stapler.HttpRedirect;
 import org.kohsuke.stapler.HttpResponse;
 import org.kohsuke.stapler.Stapler;
 import org.kohsuke.stapler.StaplerRequest;
+import org.springframework.web.util.UriComponentsBuilder;
 
 import static org.jenkinsci.plugins.oic.OicSecurityRealm.ensureStateAttribute;
 
@@ -189,11 +193,16 @@
      * @return an {@link HttpResponse}
      */
     public HttpResponse finishLogin(StaplerRequest request, AuthorizationCodeFlow flow) throws IOException {
-        StringBuffer buf = request.getRequestURL();
+        final String requestURL;
         if (request.getQueryString() != null) {
+            StringBuffer buf = request.getRequestURL();
             buf.append('?').append(request.getQueryString());
+            requestURL = buf.toString();
+        } else {
+            // some providers ADFS! post data using a form rather than the queryString.
+            requestURL = convertFormToQueryParameters(request);
         }
-        AuthorizationCodeResponseUrl responseUrl = new AuthorizationCodeResponseUrl(buf.toString());
+        AuthorizationCodeResponseUrl responseUrl = new AuthorizationCodeResponseUrl(requestURL);
         if (!state.equals(responseUrl.getState())) {
             return new Failure("State is invalid");
         }
@@ -216,6 +225,17 @@
         return onSuccess(code, flow);
     }
 
+    @VisibleForTesting
+    @Restricted(DoNotUse.class)
+    protected static String convertFormToQueryParameters(StaplerRequest request) {
+        Map<String, String[]> parameterMap = request.getParameterMap();
+        UriComponentsBuilder queryBuilder =
+                UriComponentsBuilder.fromHttpUrl(request.getRequestURL().toString());
+        for (Map.Entry<String, String[]> entry : parameterMap.entrySet()) {
+            queryBuilder.queryParam(entry.getKey(), (Object[]) entry.getValue());
+        }
+        return queryBuilder.build().toUriString();
+    }
     /**
      * Where was the user trying to navigate to when they had to login?
      *

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

@@ -0,0 +1,101 @@
+package org.jenkinsci.plugins.oic;
+
+import javax.servlet.FilterChain;
+import javax.servlet.http.HttpServletRequest;
+import javax.servlet.http.HttpServletResponse;
+import jenkins.model.Jenkins;
+import org.junit.jupiter.api.Test;
+import org.junit.jupiter.api.extension.ExtendWith;
+import org.mockito.Mock;
+import org.mockito.MockedStatic;
+import org.mockito.Mockito;
+import org.mockito.junit.jupiter.MockitoExtension;
+
+import static org.junit.Assert.assertFalse;
+import static org.junit.Assert.assertTrue;
+import static org.mockito.Mockito.lenient;
+import static org.mockito.Mockito.times;
+import static org.mockito.Mockito.when;
+
+@ExtendWith(MockitoExtension.class)
+class OicCrumbExclusionTest {
+
+    @Mock
+    Jenkins jenkins;
+
+    @Mock
+    MockedStatic<Jenkins> staticJenkins;
+
+    @Mock
+    OicSecurityRealm oicSecurityRealm;
+
+    @Mock
+    FilterChain chain;
+
+    @Mock
+    HttpServletRequest request;
+
+    @Mock
+    HttpServletResponse response;
+
+    private void withJenkins() {
+        staticJenkins.when(Jenkins::getInstanceOrNull).thenReturn(jenkins);
+    }
+
+    private void withRequestPath(String path) {
+        lenient().when(request.getPathInfo()).thenReturn(path);
+    }
+
+    private void withOicSecurityRealm() {
+        when(jenkins.getSecurityRealm()).thenReturn(oicSecurityRealm);
+    }
+
+    @Test
+    void exclusion_applies_when_realm_is_OIC_and_path_is_finishLogin() throws Exception {
+        withJenkins();
+        withRequestPath("/securityRealm/finishLogin");
+        withOicSecurityRealm();
+
+        OicCrumbExclusion oicCrumbExclusion = new OicCrumbExclusion();
+        assertTrue("path should be excluded", oicCrumbExclusion.process(request, response, chain));
+        Mockito.verify(chain, times(1)).doFilter(request, response);
+    }
+
+    @Test
+    void exclusion_does_not_apply_when_realm_is_OIC_and_path_is_not_finishLogin() throws Exception {
+        withJenkins();
+        withRequestPath("/securityRealm/anything");
+        withOicSecurityRealm();
+
+        OicCrumbExclusion oicCrumbExclusion = new OicCrumbExclusion();
+        assertFalse("path should not be excluded", oicCrumbExclusion.process(request, response, chain));
+        Mockito.verify(chain, times(0)).doFilter(request, response);
+    }
+
+    @Test
+    void exclusion_does_not_apply_when_realm_is_not_OIC_and_path_is_finishLogin() throws Exception {
+        withJenkins();
+        withRequestPath("/securityRealm/finishLogin");
+
+        OicCrumbExclusion oicCrumbExclusion = new OicCrumbExclusion();
+        assertFalse("path should not be excluded", oicCrumbExclusion.process(request, response, chain));
+        Mockito.verify(chain, times(0)).doFilter(request, response);
+    }
+
+    @Test
+    void exclusion_does_not_apply_when_realm_is_not_OIC_and_path_is_not_finishLogin() throws Exception {
+        withJenkins();
+        withRequestPath("/securityRealm/anything");
+
+        OicCrumbExclusion oicCrumbExclusion = new OicCrumbExclusion();
+        assertFalse("path should not be excluded", oicCrumbExclusion.process(request, response, chain));
+        Mockito.verify(chain, times(0)).doFilter(request, response);
+    }
+
+    @Test
+    void exclusion_does_not_apply_when_jenkins_is_not_set() throws Exception {
+        OicCrumbExclusion oicCrumbExclusion = new OicCrumbExclusion();
+        assertFalse("path should not be excluded", oicCrumbExclusion.process(request, response, chain));
+        Mockito.verify(chain, times(0)).doFilter(request, response);
+    }
+}

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

@@ -2,15 +2,20 @@
 
 import com.google.api.client.auth.oauth2.AuthorizationCodeFlow;
 import java.io.IOException;
+import java.util.SortedMap;
+import java.util.TreeMap;
 import jenkins.model.Jenkins;
-import org.junit.Before;
 import org.junit.Rule;
 import org.junit.Test;
 import org.jvnet.hudson.test.JenkinsRule;
+import org.jvnet.hudson.test.WithoutJenkins;
 import org.kohsuke.stapler.HttpResponse;
+import org.kohsuke.stapler.StaplerRequest;
 
 import static org.junit.Assert.assertEquals;
 import static org.junit.Assert.assertNotEquals;
+import static org.mockito.Mockito.mock;
+import static org.mockito.Mockito.when;
 
 public class OicSessionTest {
 
@@ -21,7 +26,6 @@ public class OicSessionTest {
 
     private static final String from = "fromAddy";
 
-    @Before
     public void init() throws IOException {
         TestRealm realm = new TestRealm.Builder("http://localhost/")
                 .WithMinimalDefaults().WithScopes("openid").build();
@@ -44,12 +48,30 @@ private String buildOAuthRedirectUrl() throws NullPointerException {
     }
 
     @Test
-    public void getFrom() {
+    public void getFrom() throws Exception {
+        init();
         assertEquals(from, session.getFrom());
     }
 
     @Test
-    public void getState() {
+    public void getState() throws Exception {
+        init();
         assertNotEquals("", session.getState());
     }
+
+    @Test
+    @WithoutJenkins
+    public void testFormToQueryParameters() {
+        StaplerRequest sr = mock(StaplerRequest.class);
+        when(sr.getRequestURL())
+                .thenReturn(new StringBuffer("http://domain.invalid/jenkins/securityRealm/finishLogin"));
+        SortedMap<String, String[]> parametersMap = new TreeMap<>();
+        parametersMap.put("param1", new String[] {"p1k1"});
+        parametersMap.put("param2", new String[] {"p2k1", "p2k2"});
+        when(sr.getParameterMap()).thenReturn(parametersMap);
+        String converted = OicSession.convertFormToQueryParameters(sr);
+        assertEquals(
+                "http://domain.invalid/jenkins/securityRealm/finishLogin?param1=p1k1&param2=p2k1&param2=p2k2",
+                converted);
+    }
 }