-
Notifications
You must be signed in to change notification settings - Fork 203
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
Add support for acquiring a token with a pre-signed JWT #271
Conversation
JWT. Useful for where the signing takes place externally for example using Azure Key Vault (AKV). AKV sample included.
Nice to meet you, again, David! :-) We will definitely take a look into this. |
prod |
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.
Thank you, David, for your patience, and your comprehensive work on this PR! It is rare to see a contributor PR with this level of test coverage. You are awesome!
We added some comments below. If you can make some adjustment, that would be great.
PR tile is malformed. |
…e in both if and else"
Thanks @jiasli for the review, too! @lochiiconnectivity , I added some changes mainly to the docs, as yet another PR to your feature branch. Please help proofread and merge that PR when done. We will take care of this PR after that. |
Co-authored-by: Ray Luo <rayluo.mba@gmail.com>
Co-authored-by: Ray Luo <rayluo.mba@gmail.com>
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.
Feature-wise, this is completed. Approving now.
Somehow github reports a conflict which prevents my merge attempt. @lochiiconnectivity would you mind fixing this by a rebase-to-dev or merge-from-dev? Never mind. David might not be able to see the merge conflict from github ui. I can try a manual fix locally.
I've resolved the merge conflict by hand, no need for rebase - it was minor. |
Thanks David for driving this to the finish line! It will be included in our next release. At that time we will leave another note in this PR, so you will receive a notification. Meanwhile, relax and be proud. :-) |
Hi Dave (@lochiiconnectivity ), I mean to reply your email today but somehow my response won't be sent. Just letting you know that we are still working on another feature which was also scheduled in the next release but took longer than expected. Don't worry. Our next release will automatically include all the features currently in dev branch. Meanwhile, there is a workaround for you. You can use the following line to install the "cutting-edge" MSAL which includes your PR. Later when MSAL 1.13 is released, you simply switch that line back to the normal "pip install msal", without any code changes in your project.
Thanks again for your patience and understanding. |
Thanks, but we're waiting to publish software tied to 1.13 (as in msal=1.13 in requirements.txt), we can't do this without a published module , so we want you to publish 1.13 as soon as possible please. |
Hi Dave (@lochiiconnectivity ), thanks for your patience. MSAL 1.13 has been released just now! Also thank you very much for your contribution! |
JWT.
Useful for where the signing takes place externally for example using
Azure Key Vault (AKV).
AKV sample included.
This is the MSAL version of This pull