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
AuthN Config V2 -> HotReload Aware Authentication settings in dev mode. #2414
base: main
Are you sure you want to change the base?
AuthN Config V2 -> HotReload Aware Authentication settings in dev mode. #2414
Changes from 6 commits
1923b58
ddb315f
8e33fef
4d37450
b3ea16c
7bd8a05
b130102
51b7693
caeb23a
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.
nit: rename to indicate this function also handles signaling the change token. Currently, it says send event notification only.
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 setup of httpContext.User is the only required change for hot reload in this file,right? Is my understanding correct that the rest of the changes in this file are good to have but not really relevant to hot reload?
I dont mind the changes but trying to understand how the value of scheme affects hot reload..
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 need all the code in this file and on lines that precede this code. The following line is important because we must pass scheme to authenticateAsync to ensure the proper authentication handler is used. Scheme is determined using the runtime config. If the provider changes in runtime config, DAB must resolve that change and ensure the request is authenticated using the hotreloaded scheme.
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.
should we make these constants and use case-insensitive matching?