Skip to content

Commit 419eeb1

Browse files
authored
Use next link from group membership request (Azure#14305)
* Use next link from group membership request loadUser only gets the first 100 groups a user belongs to rather than using the odata.nextLink in the response to get the full list of groups. Fix this by checking if the response contains an odata.nextLink and then build the URL appropriately for the configured API version. For V1, odata.nextLink contains a skip token which we extract. More information here: https://docs.microsoft.com/en-us/previous-versions/azure/ad/graph/api/users-operations#get-a-users-manager-- For V2, odata.nextLink can be used directly as the URL for the request as specified here: https://docs.microsoft.com/en-us/graph/paging fix Azure#14222 * Remove jackson datatype dep Change the type of the odata.nextLink from Optional<String> to String * Remove unused Jdk8Module import Co-authored-by: Philippe Partarrieu <[email protected]>
1 parent c329352 commit 419eeb1

File tree

8 files changed

+122
-35
lines changed

8 files changed

+122
-35
lines changed

sdk/spring/azure-spring-boot-samples/azure-spring-boot-sample-active-directory/src/main/java/com/microsoft/azure/aad/controller/TodoListController.java

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -93,7 +93,7 @@ public ResponseEntity<String> deleteTodoItem(@PathVariable("id") int id,
9393
final UserPrincipal current = (UserPrincipal) authToken.getPrincipal();
9494

9595
if (current.isMemberOf(
96-
new UserGroup("xxxxxxxx-xxxx-xxxx-xxxx-xxxxxxxxxxxx", "group1"))) {
96+
new UserGroup("xxxxxxxx-xxxx-xxxx-xxxx-xxxxxxxxxxxx", "Group", "group1"))) {
9797
final List<TodoItem> find = todoList.stream().filter(i -> i.getID() == id).collect(Collectors.toList());
9898
if (!find.isEmpty()) {
9999
todoList.remove(todoList.indexOf(find.get(0)));

sdk/spring/azure-spring-boot/src/main/java/com/microsoft/azure/spring/autoconfigure/aad/AzureADGraphClient.java

Lines changed: 36 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -3,7 +3,6 @@
33

44
package com.microsoft.azure.spring.autoconfigure.aad;
55

6-
import com.fasterxml.jackson.databind.JsonNode;
76
import com.fasterxml.jackson.databind.ObjectMapper;
87
import com.microsoft.aad.msal4j.ClientCredentialFactory;
98
import com.microsoft.aad.msal4j.ConfidentialClientApplication;
@@ -40,7 +39,6 @@
4039
import java.util.concurrent.ExecutorService;
4140
import java.util.concurrent.Executors;
4241
import java.util.stream.Collectors;
43-
import java.util.stream.StreamSupport;
4442

4543
/**
4644
* Microsoft Graph client encapsulation.
@@ -77,8 +75,8 @@ private void initAADMicrosoftGraphApiBool(String endpointEnv) {
7775
this.aadMicrosoftGraphApiBool = endpointEnv.contains(V2_VERSION_ENV_FLAG);
7876
}
7977

80-
private String getUserMembershipsV1(String accessToken) throws IOException {
81-
final URL url = new URL(serviceEndpoints.getAadMembershipRestUri());
78+
private String getUserMemberships(String accessToken, String odataNextLink) throws IOException {
79+
final URL url = buildUrl(odataNextLink);
8280
final HttpURLConnection conn = (HttpURLConnection) url.openConnection();
8381
// Set the appropriate header fields in the request header.
8482

@@ -103,6 +101,26 @@ private String getUserMembershipsV1(String accessToken) throws IOException {
103101
}
104102
}
105103

104+
private String getSkipTokenFromLink(String odataNextLink) {
105+
String[] parts = odataNextLink.split("/memberOf\\?");
106+
return parts[1];
107+
}
108+
109+
private URL buildUrl(String odataNextLink) throws MalformedURLException {
110+
URL url;
111+
if (odataNextLink != null) {
112+
if (this.aadMicrosoftGraphApiBool) {
113+
url = new URL(odataNextLink);
114+
} else {
115+
String skipToken = getSkipTokenFromLink(odataNextLink);
116+
url = new URL(serviceEndpoints.getAadMembershipRestUri() + "&" + skipToken);
117+
}
118+
} else {
119+
url = new URL(serviceEndpoints.getAadMembershipRestUri());
120+
}
121+
return url;
122+
}
123+
106124
private static String getResponseStringFromConn(HttpURLConnection conn) throws IOException {
107125

108126
try (BufferedReader reader = new BufferedReader(
@@ -121,37 +139,34 @@ public List<UserGroup> getGroups(String graphApiToken) throws IOException {
121139
}
122140

123141
private List<UserGroup> loadUserGroups(String graphApiToken) throws IOException {
124-
final String responseInJson = getUserMembershipsV1(graphApiToken);
142+
String responseInJson = getUserMemberships(graphApiToken, null);
125143
final List<UserGroup> lUserGroups = new ArrayList<>();
126144
final ObjectMapper objectMapper = JacksonObjectMapperFactory.getInstance();
127-
final JsonNode rootNode = objectMapper.readValue(responseInJson, JsonNode.class);
128-
final JsonNode valuesNode = rootNode.get("value");
129-
130-
if (valuesNode != null) {
131-
lUserGroups
132-
.addAll(StreamSupport.stream(valuesNode.spliterator(), false).filter(this::isMatchingUserGroupKey)
133-
.map(node -> {
134-
final String objectID = node.
135-
get(aadAuthenticationProperties.getUserGroup().getObjectIDKey()).asText();
136-
final String displayName = node.get("displayName").asText();
137-
return new UserGroup(objectID, displayName);
138-
}).collect(Collectors.toList()));
145+
UserGroups groupsFromJson = objectMapper.readValue(responseInJson, UserGroups.class);
139146

147+
if (groupsFromJson.getValue() != null) {
148+
lUserGroups.addAll(groupsFromJson.getValue().stream().filter(this::isMatchingUserGroupKey)
149+
.collect(Collectors.toList()));
150+
}
151+
while (groupsFromJson.getOdataNextLink() != null) {
152+
responseInJson = getUserMemberships(graphApiToken, groupsFromJson.getOdataNextLink());
153+
groupsFromJson = objectMapper.readValue(responseInJson, UserGroups.class);
154+
lUserGroups.addAll(groupsFromJson.getValue().stream().filter(this::isMatchingUserGroupKey)
155+
.collect(Collectors.toList()));
140156
}
141157

142158
return lUserGroups;
143159
}
144160

145161
/**
146-
* Checks that the JSON Node is a valid User Group to extract User Groups from
162+
* Checks that the UserGroup has a Group object type.
147163
*
148164
* @param node - json node to look for a key/value to equate against the
149165
* {@link AADAuthenticationProperties.UserGroupProperties}
150166
* @return true if the json node contains the correct key, and expected value to identify a user group.
151167
*/
152-
private boolean isMatchingUserGroupKey(final JsonNode node) {
153-
return node.get(aadAuthenticationProperties.getUserGroup().getKey()).asText()
154-
.equals(aadAuthenticationProperties.getUserGroup().getValue());
168+
private boolean isMatchingUserGroupKey(final UserGroup group) {
169+
return group.getObjectType().equals(aadAuthenticationProperties.getUserGroup().getValue());
155170
}
156171

157172
public Set<GrantedAuthority> getGrantedAuthorities(String graphApiToken) throws IOException {

sdk/spring/azure-spring-boot/src/main/java/com/microsoft/azure/spring/autoconfigure/aad/UserGroup.java

Lines changed: 20 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -6,21 +6,37 @@
66
import java.io.Serializable;
77
import java.util.Objects;
88

9+
import com.fasterxml.jackson.annotation.JsonAlias;
10+
import com.fasterxml.jackson.annotation.JsonCreator;
11+
import com.fasterxml.jackson.annotation.JsonIgnoreProperties;
12+
import com.fasterxml.jackson.annotation.JsonProperty;
13+
14+
@JsonIgnoreProperties(ignoreUnknown = true)
915
public class UserGroup implements Serializable {
1016
private static final long serialVersionUID = 9064197572478554735L;
1117

1218
private String objectID;
19+
private String objectType;
1320
private String displayName;
1421

15-
public UserGroup(String objectID, String displayName) {
22+
@JsonCreator
23+
public UserGroup(
24+
@JsonProperty("objectId") @JsonAlias("id") String objectID,
25+
@JsonProperty("objectType") @JsonAlias("@odata.type") String objectType,
26+
@JsonProperty("displayName") String displayName) {
1627
this.objectID = objectID;
28+
this.objectType = objectType;
1729
this.displayName = displayName;
1830
}
1931

2032
public String getDisplayName() {
2133
return displayName;
2234
}
2335

36+
public String getObjectType() {
37+
return objectType;
38+
}
39+
2440
public String getObjectID() {
2541
return objectID;
2642
}
@@ -35,11 +51,12 @@ public boolean equals(Object o) {
3551
}
3652
final UserGroup group = (UserGroup) o;
3753
return this.getDisplayName().equals(group.getDisplayName())
38-
&& this.getObjectID().equals(group.getObjectID());
54+
&& this.getObjectID().equals(group.getObjectID())
55+
&& this.getObjectType().equals(group.getObjectType());
3956
}
4057

4158
@Override
4259
public int hashCode() {
43-
return Objects.hash(objectID, displayName);
60+
return Objects.hash(objectID, objectType, displayName);
4461
}
4562
}
Lines changed: 52 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,52 @@
1+
// Copyright (c) Microsoft Corporation. All rights reserved.
2+
// Licensed under the MIT License.
3+
4+
package com.microsoft.azure.spring.autoconfigure.aad;
5+
6+
import java.util.List;
7+
import java.util.Objects;
8+
9+
import com.fasterxml.jackson.annotation.JsonCreator;
10+
import com.fasterxml.jackson.annotation.JsonIgnoreProperties;
11+
import com.fasterxml.jackson.annotation.JsonProperty;
12+
13+
@JsonIgnoreProperties(ignoreUnknown = true)
14+
public class UserGroups {
15+
16+
private String odataNextLink;
17+
private List<UserGroup> value;
18+
19+
@JsonCreator
20+
public UserGroups(
21+
@JsonProperty("odata.nextLink") String odataNextLink,
22+
@JsonProperty("value") List<UserGroup> value) {
23+
this.odataNextLink = odataNextLink;
24+
this.value = value;
25+
}
26+
27+
public String getOdataNextLink() {
28+
return odataNextLink;
29+
}
30+
31+
public List<UserGroup> getValue() {
32+
return value;
33+
}
34+
35+
@Override
36+
public boolean equals(Object o) {
37+
if (o == this) {
38+
return true;
39+
}
40+
if (!(o instanceof UserGroups)) {
41+
return false;
42+
}
43+
final UserGroups groups = (UserGroups) o;
44+
return this.getOdataNextLink().equals(groups.getOdataNextLink())
45+
&& this.getValue().equals(groups.getValue());
46+
}
47+
48+
@Override
49+
public int hashCode() {
50+
return Objects.hash(odataNextLink, value);
51+
}
52+
}

sdk/spring/azure-spring-boot/src/test/java/com/microsoft/azure/spring/autoconfigure/aad/AzureADGraphClientTest.java

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -41,7 +41,7 @@ public void setup() {
4141
public void testConvertGroupToGrantedAuthorities() {
4242

4343
final List<UserGroup> userGroups = Collections.singletonList(
44-
new UserGroup("testId", "Test_Group"));
44+
new UserGroup("testId", "Group", "Test_Group"));
4545

4646
final Set<GrantedAuthority> authorities = adGraphClient.convertGroupsToGrantedAuthorities(userGroups);
4747
assertThat(authorities).hasSize(1).extracting(GrantedAuthority::getAuthority)
@@ -51,8 +51,8 @@ public void testConvertGroupToGrantedAuthorities() {
5151
@Test
5252
public void testConvertGroupToGrantedAuthoritiesUsingAllowedGroups() {
5353
final List<UserGroup> userGroups = Arrays
54-
.asList(new UserGroup("testId", "Test_Group"),
55-
new UserGroup("testId", "Another_Group"));
54+
.asList(new UserGroup("testId", "Group", "Test_Group"),
55+
new UserGroup("testId", "Group", "Another_Group"));
5656
aadAuthProps.getUserGroup().getAllowedGroups().add("Another_Group");
5757
final Set<GrantedAuthority> authorities = adGraphClient.convertGroupsToGrantedAuthorities(userGroups);
5858
assertThat(authorities).hasSize(2).extracting(GrantedAuthority::getAuthority)

sdk/spring/azure-spring-boot/src/test/java/com/microsoft/azure/spring/autoconfigure/aad/UserGroupTest.java

Lines changed: 8 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -7,27 +7,32 @@
77
import org.junit.Test;
88

99
public class UserGroupTest {
10-
private static final UserGroup GROUP_1 = new UserGroup("12345", "test");
10+
private static final UserGroup GROUP_1 = new UserGroup("12345", "Group", "test");
1111

1212
@Test
1313
public void getDisplayName() {
1414
Assert.assertEquals("test", GROUP_1.getDisplayName());
1515
}
1616

17+
@Test
18+
public void getObjectType() {
19+
Assert.assertEquals("Group", GROUP_1.getObjectType());
20+
}
21+
1722
@Test
1823
public void getObjectID() {
1924
Assert.assertEquals("12345", GROUP_1.getObjectID());
2025
}
2126

2227
@Test
2328
public void equals() {
24-
final UserGroup group2 = new UserGroup("12345", "test");
29+
final UserGroup group2 = new UserGroup("12345", "Group", "test");
2530
Assert.assertEquals(GROUP_1, group2);
2631
}
2732

2833
@Test
2934
public void hashCodeTest() {
30-
final UserGroup group2 = new UserGroup("12345", "test");
35+
final UserGroup group2 = new UserGroup("12345", "Group", "test");
3136
Assert.assertEquals(GROUP_1.hashCode(), group2.hashCode());
3237
}
3338
}

sdk/spring/azure-spring-boot/src/test/resources/aad/azure-ad-graph-user-groups.json

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -60,6 +60,5 @@
6060
"provisioningErrors": [],
6161
"proxyAddresses": [],
6262
"securityEnabled": true
63-
}],
64-
"odata.nextLink": "directoryObjects/$/Microsoft.DirectoryServices.User/12345678-2898-434a-a370-8ec974c2fb57/memberOf?$skiptoken=X'445370740700010000000000000000100000009D29CBA7B45D854A84FF7F9B636BD9DC000000000000000000000017312E322E3834302E3131333535362E312E342E3233333100000000'"
63+
}]
6564
}

sdk/spring/azure-spring-boot/src/test/resources/aad/microsoft-graph-user-groups.json

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -75,6 +75,5 @@
7575
"securityEnabled": true,
7676
"visibility": null,
7777
"onPremisesProvisioningErrors": []
78-
}],
79-
"odata.nextLink": "directoryObjects/$/Microsoft.DirectoryServices.User/12345678-2898-434a-a370-8ec974c2fb57/memberOf?$skiptoken=X'445370740700010000000000000000100000009D29CBA7B45D854A84FF7F9B636BD9DC000000000000000000000017312E322E3834302E3131333535362E312E342E3233333100000000'"
78+
}]
8079
}

0 commit comments

Comments
 (0)