Merge pull request #39 from abhishekbiyala/master

Fix for issue# 31
hkamel · May 1, 2018 · 8d97fc5 · 8d97fc5
2 parents e3eb259 + 07106d5
commit 8d97fc5
Show file tree

Hide file tree

Showing 7 changed files with 119 additions and 27 deletions.
diff --git a/pom.xml b/pom.xml
@@ -76,6 +76,11 @@
             <version>3.0.2</version>
             <scope>provided</scope>
         </dependency>
+        <dependency>
+		    <groupId>org.apache.commons</groupId>
+		    <artifactId>commons-lang3</artifactId>
+		    <version>3.7</version>
+		</dependency>
 
         <!-- Test dependencies -->
         <dependency>
@@ -93,7 +98,7 @@
         <dependency>
             <groupId>org.mockito</groupId>
             <artifactId>mockito-core</artifactId>
-            <version>1.10.19</version>
+            <version>2.15.0</version>
             <scope>test</scope>
             <exclusions>
                 <exclusion>

diff --git a/src/main/java/org/almrangers/auth/aad/AadIdentityProvider.java b/src/main/java/org/almrangers/auth/aad/AadIdentityProvider.java
@@ -32,6 +32,7 @@
 import com.microsoft.aad.adal4j.ClientCredential;
 import com.microsoft.aad.adal4j.UserInfo;
 import java.net.HttpURLConnection;
+import java.net.MalformedURLException;
 import java.net.URI;
 import java.net.URL;
 import java.util.HashSet;
@@ -40,6 +41,8 @@
 import java.util.concurrent.Executors;
 import java.util.concurrent.Future;
 import javax.servlet.http.HttpServletRequest;
+
+import org.apache.commons.lang3.StringUtils;
 import org.json.JSONArray;
 import org.json.JSONObject;
 import org.sonar.api.server.ServerSide;
@@ -154,32 +157,45 @@ private String getLogin(UserInfo aadUser) {
     }
   }
 
-  private Set<String> getUserGroupsMembership(String accessToken, String userId) {
-    Set<String> userGroups = new HashSet<>();
+  URL getUrl(String userId, String nextPage) throws MalformedURLException {
+	  String url =  String.format(GROUPS_REQUEST_FORMAT, settings.tenantId(), userId);
+	  // Append odata query parameters for subsequent pages
+	if (null != nextPage) {
+		url += "&" + nextPage;
+	}
+	return new URL(url);
+  }
+
+  public Set<String> getUserGroupsMembership(String accessToken, String userId) {
+	Set<String> userGroups = new HashSet<>();
+	String nextPage = null;
     try {
-      URL url = new URL(String.format(GROUPS_REQUEST_FORMAT, settings.tenantId(), userId));
-      HttpURLConnection connection = (HttpURLConnection) url.openConnection();
-      connection.setRequestProperty("api-version", "1.6");
-      connection.setRequestProperty("Authorization", accessToken);
-      connection.setRequestProperty("Accept", "application/json;odata=minimalmetadata");
-      String goodRespStr = HttpClientHelper.getResponseStringFromConn(connection, true);
-      int responseCode = connection.getResponseCode();
-      JSONObject response = HttpClientHelper.processGoodRespStr(responseCode, goodRespStr);
-      JSONArray groups;
-      groups = JSONHelper.fetchDirectoryObjectJSONArray(response);
-      AadGroup group;
-      for (int i = 0; i < groups.length(); i++) {
-        JSONObject thisUserJSONObject = groups.optJSONObject(i);
-        group = new AadGroup();
-        JSONHelper.convertJSONObjectToDirectoryObject(thisUserJSONObject, group);
-        userGroups.add(group.getDisplayName());
-      }
+      do {
+    	  URL url = getUrl(userId, nextPage);
+	      HttpURLConnection connection = (HttpURLConnection) url.openConnection();
+	      connection.setRequestProperty("api-version", "1.6");
+	      connection.setRequestProperty("Authorization", accessToken);
+	      connection.setRequestProperty("Accept", "application/json;odata=minimalmetadata");
+	      String goodRespStr = HttpClientHelper.getResponseStringFromConn(connection, true);
+	      int responseCode = connection.getResponseCode();
+	      JSONObject response = HttpClientHelper.processGoodRespStr(responseCode, goodRespStr);
+	      JSONArray groups;
+	      groups = JSONHelper.fetchDirectoryObjectJSONArray(response);      
+	      AadGroup group;
+	      for (int i = 0; i < groups.length(); i++) {
+	        JSONObject thisUserJSONObject = groups.optJSONObject(i);
+	        group = new AadGroup();
+	        JSONHelper.convertJSONObjectToDirectoryObject(thisUserJSONObject, group);
+	        userGroups.add(group.getDisplayName());
+	      }
+	      nextPage = JSONHelper.fetchNextPageLink(response);
+      } while (StringUtils.isNotEmpty(nextPage));
     } catch (Exception e) {
       LOGGER.error(e.toString());
     }
     return userGroups;
   }
-
+  
   private String generateUniqueLogin(UserInfo aadUser) {
     return String.format("%s@%s", aadUser.getDisplayableId(), getKey());
   }

diff --git a/src/main/java/org/almrangers/auth/aad/JSONHelper.java b/src/main/java/org/almrangers/auth/aad/JSONHelper.java
@@ -20,6 +20,7 @@
 package org.almrangers.auth.aad;
 
 import java.lang.reflect.Field;
+import org.apache.commons.lang3.StringUtils;
 import org.apache.commons.lang3.text.WordUtils;
 import org.json.JSONArray;
 import org.json.JSONObject;
@@ -42,6 +43,16 @@ public static JSONArray fetchDirectoryObjectJSONArray(JSONObject jsonObject) {
     return jsonObject.optJSONObject("responseMsg").optJSONArray("value");
   }
 
+  /**
+   * This method parses a JSON field out of a json object
+   *
+   * @param jsonObject The JSON String that holds the collection.
+   * @return next page link 
+   * @throws Exception
+   */
+  public static String fetchNextPageLink(JSONObject jsonObject) {
+    return jsonObject.optJSONObject("responseMsg").has("odata.nextLink") ? StringUtils.substringAfterLast(jsonObject.optJSONObject("responseMsg").get("odata.nextLink").toString(), "memberOf?") : null;
+  }
 
   /**
    * This is a generic method that copies the simple attribute values from an

diff --git a/src/main/resources/mockito-extensions/org.mockito.plugins.MockMaker b/src/main/resources/mockito-extensions/org.mockito.plugins.MockMaker
@@ -0,0 +1 @@
+mock-maker-inline
diff --git a/src/test/java/org/almrangers/auth/aad/AadIdentityProviderTest.java b/src/test/java/org/almrangers/auth/aad/AadIdentityProviderTest.java
@@ -26,25 +26,70 @@
  */
 package org.almrangers.auth.aad;
 
+import static org.assertj.core.api.Assertions.assertThat;
+import static org.junit.Assert.assertEquals;
+import static org.mockito.Mockito.doReturn;
+import static org.mockito.Mockito.mock;
+import static org.mockito.Mockito.spy;
+import static org.mockito.Mockito.verify;
+import static org.mockito.Mockito.when;
+
+import java.io.IOException;
+import java.net.HttpURLConnection;
+import java.net.URL;
+import java.util.HashSet;
+
 import org.junit.Rule;
 import org.junit.Test;
 import org.junit.rules.ExpectedException;
 import org.sonar.api.config.Settings;
 import org.sonar.api.config.internal.MapSettings;
 import org.sonar.api.server.authentication.OAuth2IdentityProvider;
 
-import static org.assertj.core.api.Assertions.assertThat;
-import static org.mockito.Mockito.mock;
-import static org.mockito.Mockito.verify;
-import static org.mockito.Mockito.when;
-
 public class AadIdentityProviderTest {
 
   @Rule
   public ExpectedException thrown = ExpectedException.none();
   Settings settings = new MapSettings();
   AadSettings aadSettings = new AadSettings(settings);
-  AadIdentityProvider underTest = new AadIdentityProvider(aadSettings);
+  AadIdentityProvider underTest = spy(new AadIdentityProvider(aadSettings));
+
+  @Test
+  public void shouldHandleGetMembershipsPagination() throws IOException {
+
+	URL mockUrl = mock(URL.class);
+	HttpURLConnection mockConnection = mock(HttpURLConnection.class);
+
+	doReturn(mockUrl)
+	  .when(underTest)
+	  .getUrl("userId", null);
+
+	doReturn (mockConnection)
+	  .when(mockUrl)
+	  .openConnection();
+
+	doReturn(ClassLoader.class.getResourceAsStream("/get-members-page1.json"))
+	  .when(mockConnection)
+	  .getInputStream();
+
+
+	URL mockUrl2 = mock(URL.class);
+	HttpURLConnection mockConnection2 = mock(HttpURLConnection.class);
+
+	doReturn(mockUrl2)
+	  .when(underTest)
+	  .getUrl("userId", "$skiptoken=X'1234567890'");
+
+	doReturn (mockConnection2)
+	  .when(mockUrl2)
+	  .openConnection();
+
+	doReturn(ClassLoader.class.getResourceAsStream("/get-members-page2.json"))
+	  .when(mockConnection2)
+	  .getInputStream();
+
+	assertEquals(5, underTest.getUserGroupsMembership("token", "userId").size());
+  }
 
   @Test
   public void check_fields() {

diff --git a/src/test/resources/get-members-page1.json b/src/test/resources/get-members-page1.json
@@ -0,0 +1,8 @@
+{
+  "value": [
+  	{ "displayName": "Group1" },
+  	{ "displayName": "Group2" },
+  	{ "displayName": "Group3" }
+  ],
+  "odata.nextLink": "directoryObjects/$/Microsoft.DirectoryServices.User/userId/memberOf?$skiptoken=X'1234567890'"
+}
diff --git a/src/test/resources/get-members-page2.json b/src/test/resources/get-members-page2.json
@@ -0,0 +1,6 @@
+{
+  "value": [
+  	{ "displayName": "Group4" },
+  	{ "displayName": "Group5" }
+  ]
+}