Move extracting form parameters to a helper function and introduce un…

…it test

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

@@ -41,6 +41,8 @@
 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;
@@ -191,19 +193,16 @@ public HttpResponse commenceLogin(AuthorizationCodeFlow flow) {
      * @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.
-            Map<String, String[]> parameterMap = request.getParameterMap();
-            UriComponentsBuilder queryBuilder = UriComponentsBuilder.fromHttpUrl(buf.toString());
-            for (Map.Entry<String, String[]> entry : parameterMap.entrySet()) {
-                queryBuilder.queryParam(entry.getKey(), (Object[]) entry.getValue());
-            }
-            buf = new StringBuffer(queryBuilder.build().toUriString());
+            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");
         }
@@ -226,6 +225,17 @@ public HttpResponse finishLogin(StaplerRequest request, AuthorizationCodeFlow fl
         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/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);
+    }
 }