Use next link from group membership request

[ppartarr]

loadUsers() 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 #14222

[azuresdkci]

Can one of the admins verify this patch?

[ppartarr]

the check-enforcer has been running for over 20h. Some CI jobs are failing because the spring/pom.xml is dependent on com.microsoft.azure:azure-servicebus-jms:0.0.4 which is in turn dependent on org.apache.geronimo.specs:geronimo-jms_2.0_spec:LATEST which is why the maven enforcer is complaining about non-release dependency versions. I didn't add those dependencies in my PR... How can I work around this?

[conniey]

@ppartarr Oh, Check-enforcer only completes when all the other checks pass. It's a bit deceiving in its behaviour.

At the moment, it won't complete because some of those java-spring-ci tests are failing.

[saragluna]

@ppartarr

This repo uses a mechanism to centralize the version for dependencies. If a new dependency is being added then it should also be in the external_dependency.txt file. That's why the Analyze failed on the version check step.

Also, there were some AAD related unit tests failing.

[ppartarr]

@saragluna thanks for the info. It seems like the dependency I added is already in the external_depency.txt in master: https://github.com/Azure/azure-sdk-for-java/blob/master/eng/versioning/external_dependencies.txt#L8

My mistake was that I had x-version-update instead of x-include-update in the <bannedDependencies> of the azure-spring-boot/pom.xml

I'm still working on resolving the Jackson deserialisation issue that's causing the tests to fail 😃

[zhoufenqin]

@ppartarr I have a concern that if the organization has thousands of groups, then the loadUserGroups() will cost a lot of time. whether we can let the user choose to search all groups or not?

[ppartarr]

@zhoufenqin loadUserGroups() doesn't retrieve all the AD groups but simply the groups that the user is a member of. Arbitrarily omitting groups from loadUserGroups() isn't how the azure-spring-boot library should behave as it will break applications depending on group membership information. This decision shouldn't be left to the user. See bug #14222 for more information

Aside: if an organisation assigns their AD users to thousands of groups they probably aren't using group nesting correctly. Nevertheless, from my experience, it isn't uncommon for users to be a part of hundreds of groups.

[chenrujun]

/azp run java - spring - ci

[azure-pipelines]

Azure Pipelines successfully started running 1 pipeline(s).

[ppartarr]

/azp run java - spring - ci

[azure-pipelines]

Commenter does not have sufficient privileges for PR 14305 in repo Azure/azure-sdk-for-java

[ppartarr]

@chenrujun can you have a look at the failing tests please? It says that the TodoListController requires 2 parameters but this is clearly not the case from the changes in my pr: failing TodoListController line & UserGroup constructor

[chenrujun]

Hi, @ppartarr .
Please rebase to latest master branch to solve this problem.

[chenrujun]

/azp run java - spring - ci

[azure-pipelines]

Azure Pipelines successfully started running 1 pipeline(s).

[ppartarr]

@chenrujun all tests are passing, are you okay to merge this branch in?

[chenrujun]

LGTM. Thank you for your effort.