InternalRestClient respects kyuubi.engine.security.enabled to add HTTP auth header
#5566
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
pan3793
changed the title
kyuubi.engine.security.enabled to add HTTP auth header
turboFei
approved these changes
Oct 31, 2023
pan3793
added a commit
that referenced
this pull request
Oct 31, 2023
…abled` to add HTTP auth header ### _Why are the changes needed?_ `kyuubi.engine.security.enabled` aims to control whether enabled security mechanism internal communication, but the current implementation is not symmetrical, the auth generator ignores the conf and always produces the auth header, but the auth header handler is only activated when conf is enabled, that causes authentication failure when `kyuubi.engine.security.enabled=false`(default value) ### _How was this patch tested?_ - [ ] Add some test cases that check the changes thoroughly including negative and positive cases if possible - [ ] Add screenshots for manual tests if appropriate - [x] [Run test](https://kyuubi.readthedocs.io/en/master/contributing/code/testing.html#running-tests) locally before make a pull request ### _Was this patch authored or co-authored using generative AI tooling?_ No. Closes #5566 from pan3793/none-auth. Closes #5566 d42a4c3 [Cheng Pan] Revert "Extract AnonymousAuthenticationHandler from BasicAuthenticationHandler" b544343 [Cheng Pan] Extract AnonymousAuthenticationHandler from BasicAuthenticationHandler 75c4b7d [Cheng Pan] InternalRestClient respects `kyuubi.engine.security.enabled` to add HTTP auth header Authored-by: Cheng Pan <chengpan@apache.org> Signed-off-by: Cheng Pan <chengpan@apache.org> (cherry picked from commit 5f53073) Signed-off-by: Cheng Pan <chengpan@apache.org>
|
Merged to master/1.8 |
pan3793
added a commit
to pan3793/kyuubi
that referenced
this pull request
Nov 1, 2023
…ialized only when `kyuubi.engine.security.enabled` is true
3 tasks
pan3793
added a commit
that referenced
this pull request
Nov 1, 2023
…d only when `kyuubi.engine.security.enabled` is true ### _Why are the changes needed?_ Internal REST client should work when `kyuubi.engine.security.enabled` is `true`/`false`. The changes in #5566 is not sufficient. `KyuubiRestAuthenticationSuite` covers the `true` case `BatchesV1ResourceSuite` and `BatchesV2ResourceSuite` cover the `false` case ### _How was this patch tested?_ - [ ] Add some test cases that check the changes thoroughly including negative and positive cases if possible - [ ] Add screenshots for manual tests if appropriate - [x] [Run test](https://kyuubi.readthedocs.io/en/master/contributing/code/testing.html#running-tests) locally before make a pull request ### _Was this patch authored or co-authored using generative AI tooling?_ No. Closes #5601 from pan3793/5566-followup. Closes #5566 abb7106 [Cheng Pan] test 3f9e735 [Cheng Pan] [KYUUBI #5566][FOLLOWUP] Check InternalSecurityAccessor is initialized only when `kyuubi.engine.security.enabled` is true Authored-by: Cheng Pan <chengpan@apache.org> Signed-off-by: Cheng Pan <chengpan@apache.org>
pan3793
added a commit
that referenced
this pull request
Nov 1, 2023
…d only when `kyuubi.engine.security.enabled` is true ### _Why are the changes needed?_ Internal REST client should work when `kyuubi.engine.security.enabled` is `true`/`false`. The changes in #5566 is not sufficient. `KyuubiRestAuthenticationSuite` covers the `true` case `BatchesV1ResourceSuite` and `BatchesV2ResourceSuite` cover the `false` case ### _How was this patch tested?_ - [ ] Add some test cases that check the changes thoroughly including negative and positive cases if possible - [ ] Add screenshots for manual tests if appropriate - [x] [Run test](https://kyuubi.readthedocs.io/en/master/contributing/code/testing.html#running-tests) locally before make a pull request ### _Was this patch authored or co-authored using generative AI tooling?_ No. Closes #5601 from pan3793/5566-followup. Closes #5566 abb7106 [Cheng Pan] test 3f9e735 [Cheng Pan] [KYUUBI #5566][FOLLOWUP] Check InternalSecurityAccessor is initialized only when `kyuubi.engine.security.enabled` is true Authored-by: Cheng Pan <chengpan@apache.org> Signed-off-by: Cheng Pan <chengpan@apache.org> (cherry picked from commit 28fb0a7) Signed-off-by: Cheng Pan <chengpan@apache.org>
YesOrNo828
pushed a commit
to YesOrNo828/kyuubi
that referenced
this pull request
Nov 6, 2023
…ialized only when `kyuubi.engine.security.enabled` is true ### _Why are the changes needed?_ Internal REST client should work when `kyuubi.engine.security.enabled` is `true`/`false`. The changes in apache#5566 is not sufficient. `KyuubiRestAuthenticationSuite` covers the `true` case `BatchesV1ResourceSuite` and `BatchesV2ResourceSuite` cover the `false` case ### _How was this patch tested?_ - [ ] Add some test cases that check the changes thoroughly including negative and positive cases if possible - [ ] Add screenshots for manual tests if appropriate - [x] [Run test](https://kyuubi.readthedocs.io/en/master/contributing/code/testing.html#running-tests) locally before make a pull request ### _Was this patch authored or co-authored using generative AI tooling?_ No. Closes apache#5601 from pan3793/5566-followup. Closes apache#5566 abb7106 [Cheng Pan] test 3f9e735 [Cheng Pan] [KYUUBI apache#5566][FOLLOWUP] Check InternalSecurityAccessor is initialized only when `kyuubi.engine.security.enabled` is true Authored-by: Cheng Pan <chengpan@apache.org> Signed-off-by: Cheng Pan <chengpan@apache.org>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
Why are the changes needed?
kyuubi.engine.security.enabledaims to control whether enabled security mechanism internal communication, but the current implementation is not symmetrical, the auth generator ignores the conf and always produces the auth header, but the auth header handler is only activated when conf is enabled, that causes authentication failure whenkyuubi.engine.security.enabled=false(default value)How was this patch tested?
Add some test cases that check the changes thoroughly including negative and positive cases if possible
Add screenshots for manual tests if appropriate
Run test locally before make a pull request
Was this patch authored or co-authored using generative AI tooling?
No.