oauth2: support disabling redirects for AJAX requests#32655
oauth2: support disabling redirects for AJAX requests#32655mattklein123 merged 6 commits intoenvoyproxy:mainfrom
Conversation
Added new parameter `ajax_request_matcher` to optionally not allow OAuth2 authorization redirect when all tokens are expired. Such redirect usually redirects the user to a login page (in authorization code flow) and this behavior is not desired in Ajax requests. Fixes envoyproxy#30102 Signed-off-by: Samuel Valis <samuel.valis@innovatrics.com>
|
Hi @samo1, welcome and thank you for your contribution. We will try to review your Pull Request as quickly as possible. In the meantime, please take a look at the contribution guidelines if you have not done so already. |
|
CC @envoyproxy/api-shepherds: Your approval is needed for changes made to |
Signed-off-by: Samuel Valis <samuel.valis@innovatrics.com>
mattklein123
left a comment
There was a problem hiding this comment.
One question, thanks.
/wait-any
| // Requests that match any of the provided matchers will immediately return unauthorized response | ||
| // when tokens are not valid (instead of redirecting to authorization server as usual). | ||
| // This behavior is useful for AJAX and asset requests to avoid the OAuth flood. | ||
| repeated config.route.v3.HeaderMatcher ajax_request_matcher = 14; |
There was a problem hiding this comment.
I don't know much about this stuff but are there other cases where it would be desirable to have this behavior? I.e., is this AJAX specific? Would it be better to just call this immediate_unauthorized_response_matcher or something like that?
There was a problem hiding this comment.
Thanks for the suggestion. You are right that there might be other cases where this would be desirable. I have changed the name and description to describe the functionality better, I hope.
Signed-off-by: Samuel Valis <samuel.valis@innovatrics.com>
mattklein123
left a comment
There was a problem hiding this comment.
LGTM with small comment nit. Can you merge main?
/wait
|
|
||
| // Any request that matches any of the provided matchers won't be redirected to OAuth server when tokens are not valid. | ||
| // Automatic access token refresh will be performed for these requests, if enabled. | ||
| // This behavior cam be useful for AJAX requests. |
There was a problem hiding this comment.
| // This behavior cam be useful for AJAX requests. | |
| // This behavior can be useful for AJAX requests. |
# Conflicts: # changelogs/current.yaml Signed-off-by: Samuel Valis <samuel.valis@innovatrics.com>
# Conflicts: # changelogs/current.yaml Signed-off-by: Samuel Valis <samuel.valis@innovatrics.com>
* origin: mobile: Clean up Java and Kotlin code (envoyproxy#32773) [mobile] Increase stream response buffer watermark size to 2MB (envoyproxy#32754) oauth2: support disabling redirects for AJAX requests (envoyproxy#32655) http2: Switch the value of envoy.reloadable_features.http2_use_oghttp2 to false (envoyproxy#32751) envoy: reverting unnecessary exception E-M macros (envoyproxy#32745) dfp: reverting 31433 (envoyproxy#32743) opentelemetrytracer: Allow sampler to set variant type span attribute… (envoyproxy#32681) mobile: Java code cleanup (envoyproxy#32747) mobile: fix quic_test_server_test (envoyproxy#32755) Common TunnelResponseHeadersOrTrailers for UDP & TCP tunneling (envoyproxy#32619)
Commit Message: oauth2: support disabling redirects for AJAX requests
Additional Description: Added new parameter
ajax_request_matcherto optionally deny OAuth2 authorization redirect when all tokens are expired. It usually redirects the user to a login page (in authorization code flow) and this behavior is not desired in Ajax requests.Risk Level: Low
Testing: Unit tests
Docs Changes: Yes
Release Notes: Yes
Platform Specific Features:
Fixes #30102