Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Handle groupmemberships over SAML/JWT limit by following and parsing Graph URI results #174

Open
jmartens opened this issue Aug 11, 2021 · 12 comments

Comments

@jmartens
Copy link

jmartens commented Aug 11, 2021

When a large number of memberships is supplied in the auth token the individual groups are not provided anymore. Instead a Graph URI is provided. By following that you should be able to retrieve all group memberships for the user.

AFAICT the limit is 150 for SAML requests and 200 for JWT requests.

A workaround on the Azure side is to configure your application for specific groups only and set the groups claim to only return groups assigned to this application (as can be seen in the screenshot here), but it would be nice if this could be handled by this framework as well by following the Graph URI provided and parsing its results.

Fund with Polar
@jmartens jmartens changed the title Handle groupmemberships over SAML/JWT by following graph URI Handle groupmemberships over SAML/JWT limit by following and parsing Graph URI results Aug 11, 2021
@JonasKs
Copy link
Member

JonasKs commented Aug 11, 2021

Hi, this seems like a good suggestion indeed.
PR welcome. 😊

@jmartens
Copy link
Author

Not sure if I can find the time, but will see what I can do. Already have a hunch on where to add it in the code, not sure on the implementation details though.

Any preference on how to run rest requests and process the results? Are there helpers or classes in this package you prefer to be used?

@jmartens
Copy link
Author

jmartens commented Aug 11, 2021

Already have a hunch on where to add it in the code

I am guessing an additional leaf in this if else clause is the proper place to check for and retrieve the Graph Api url:

if settings.GROUPS_CLAIM in claims:
claim_groups = claims[settings.GROUPS_CLAIM]
if not isinstance(claim_groups, list):
claim_groups = [claim_groups, ]
else:
logger.debug("The configured groups claim '%s' was not found in the access token",
settings.GROUPS_CLAIM)
claim_groups = []

@JonasKs
Copy link
Member

JonasKs commented Aug 11, 2021

Yeah, I'd say there too. The graph API has to be optional, though. 😊 It'll add some overhead, so should be opt in.

@JonasKs
Copy link
Member

JonasKs commented Aug 11, 2021

I've never worked on a Azure AD Django project, so i'm not you man to implement this.

Currently writing an Azure AD plug-in for FastAPI, but it won't be as comprehensive as this package.

@jmartens
Copy link
Author

It'll add some overhead, so should be opt in.

OK, will see if and how I can create a settings parameter for it.

@jmartens
Copy link
Author

The payload of a claim is explained here and this is the specific part for the so-called Groups overage claim

@jmartens
Copy link
Author

This SO post might also hold some vital clues to get it working as apparently the application needs for Directory.Read.All permissions to successfully request the group memberships using the returned Graph URI.

@jmartens
Copy link
Author

jmartens commented Aug 12, 2021

To get the URL for the claim I think this should work:

try:
  claim_name = claims['_claim_names'][settings.GROUPS_CLAIM]
  claim_source = claims['_claim_sources'][claim_name]
  claim_url = claim_source['endpoint']
except KeyError as e:
  logger.error(f"Key not found: {e}")

@JonasKs
Copy link
Member

JonasKs commented Aug 27, 2021

This is also related to #10. Both of you want a Graph integration.

@stefanoostwegel
Copy link

Has anyone got an update regarding this issue?
Would love to see the graph integration work as we have far to many users with to many groups.
Now we have to administer azure AD and Django, in stead of only django.

@JonasKs
Copy link
Member

JonasKs commented Jul 5, 2022

Not much update to give, other than a PR is welcome.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants