-
Notifications
You must be signed in to change notification settings - Fork 208
Authorization code flow sample #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
Conversation
rayluo
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks Abhi for this attempt! It gives us better understanding in this area. As discussed off-line, let's break this work into several phases.
-
Remove the
acquire_token_silent()call in this sample (it won't work in the way we currently use it), and then re-organize the code to make it become a functionally equivalent to this accessing graph with adal sample, and then we will send out a PR to that repo. (Their sample repo has more stars and forks than ours; if you can't beat them, join them. :-) ) -
The samples in that Graph sample repo did not utilize the cache. We will revisit this, and figure out a way to demonstrate how to persist msal token cache in a web app, so that end user would not need to re-authenticate in every hour.
|
Hey Ray,
|
rayluo
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
OK. Added some minor suggestions, and then we will add this sample into our built-in samples lineup. :-)
sample/authorization-code-flow-sample/authorization_code_flow_sample.py
Outdated
Show resolved
Hide resolved
sample/authorization-code-flow-sample/authorization_code_flow_sample.py
Outdated
Show resolved
Hide resolved
sample/authorization-code-flow-sample/authorization_code_flow_sample.py
Outdated
Show resolved
Hide resolved
|
Does the wiki reference the list of samples including notes on how to run those and what they achieve? Perhaps the code should also reference that wiki page? |
|
Q: Does the wiki reference the list of samples including notes on how to run those and what they achieve? Perhaps the code should also reference that wiki page? A: Historically we do not have a wiki reference the list of samples. We have only one place referencing the list of built-in samples, and that is buried deep at THE END OF this section inside the README, because we were assuming new comers would start from that "Usage and Samples" section in README, and then naturally discover and follow that link. Now on a second thought, we should probably also have a wiki page (and now we do!) referencing to the list of samples, just to capture audiences coming from a different "entrance". But then I understand your comment was more about "how to configure/run such specific sample". That kind of information should be inside our self-contained samples. Recently we've been adding more descriptions into our existing samples, right at each configuration setting where a new dev would ask questions. That approach would also address what David mentioned yesterday that new app devs would tend to ask "client secret? what is that? where do I get one?", that sorts of questions. In that sense, we may want to add this url into our "redirect_url" setting. |
rayluo
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for your tireless effort on improving this PR! ![]()
Uh oh!
There was an error while loading. Please reload this page.