Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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
Support for multiple tokens in native auth flows #2082
Support for multiple tokens in native auth flows #2082
Changes from 23 commits
cf1f61b
7e788d8
b9aa09e
50e6c35
4ef097b
19130be
0ebf50a
7d46e01
5f601a0
443b48e
c897940
4f59452
6957c82
90e31d6
1818e11
1fe903c
681ed64
678a810
db37b21
7663b69
267edff
c3a2589
3b985e4
ba70fff
1ba1bfd
File filter
Filter by extension
Conversations
Jump to
There are no files selected for viewing
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.
As discussed in the technical design document review, the documentation in comments needs to be updated to explain the difference between these two methods.
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.
Comments updated
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.
We should actually be using
commandResult.correlationid
here, for the unlikely case that the API has returned a different correlation ID. That field might be null, so we'd need to updateGetAccessTokenError
. And that improvement would have to be applied to more places.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.
This change can be done when
commandResult is ServiceException
ifcommandResult is Exception
then we will use the old correlationId.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.
Feel free to move this into a separate PR. It's not blocking.