Skip to content

Feat/graph session scopes #40

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

Merged
merged 11 commits into from
Apr 22, 2020
Merged

Feat/graph session scopes #40

merged 11 commits into from
Apr 22, 2020

Conversation

jobala
Copy link
Contributor

@jobala jobala commented Apr 16, 2020

Overview

Updates GraphSession API

The API has changed so that users only has to pass an auth provider. Initially, users had to create options and auth_handler objects.

scopes = ['mail.send', 'user.read']
auth_provider = TokenCredentialAuthProvider(device_credential, scopes)

graph_session = GraphSession(auth_provider)
result = graph_session.get('/me')
print(result.json())

Closes #36

@jobala jobala requested review from MIchaelMainer and ddyett April 16, 2020 12:14
@jobala
Copy link
Contributor Author

jobala commented Apr 17, 2020

@MIchaelMainer

The following concerns have been addressed

  • Optionally set scopes on AuthProvider
  • Stopped passing AuthProviderOptions to AuthorizationHandler
  • Setting default scopes

@MIchaelMainer
Copy link
Contributor

MIchaelMainer commented Apr 20, 2020

@jobala I have the remaining concerns:

  • Setting user.read as the default scope instead of https://graph.microsoft.com/.default. Why did you make this change?
  • Provide in-depth comments to files. Especially document the args, default behaviors, expected exceptions, new/foreign concepts, etc.
  • Does this design support incremental consent?

@jobala
Copy link
Contributor Author

jobala commented Apr 21, 2020

@MIchaelMainer I am working on this PR that builds on top of this. I will add the comments in that branch.

@jobala jobala merged commit a91a347 into dev Apr 22, 2020
@samwelkanda samwelkanda deleted the feat/graph-session-scopes branch October 14, 2022 12:25
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

Successfully merging this pull request may close these issues.

Pass scopes to GraphSession through kwargs argument instead of AuthProviderOptions
2 participants