You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@kyuubi.apache.org by "beryllw (via GitHub)" <gi...@apache.org> on 2024/04/04 04:27:33 UTC
[PR] [KYUUBI #4079] Web UI support Basic authN [kyuubi]
beryllw opened a new pull request, #6258:
URL: https://github.com/apache/kyuubi/pull/6258
# :mag: Description
## Issue References 🔗
<!-- Append the issue number after #. If there is no issue for you to link create one or -->
<!-- If there are no issues to link, please provide details here. -->
This pull request fixes #6079
## Describe Your Solution 🔧
Please include a summary of the change and which issue is fixed. Please also include relevant motivation and context. List any dependencies that are required for this change.
## Types of changes :bookmark:
<!--- What types of changes does your code introduce? Put an `x` in all the boxes that apply: -->
- [ ] Bugfix (non-breaking change which fixes an issue)
- [ ] New feature (non-breaking change which adds functionality)
- [ ] Breaking change (fix or feature that would cause existing functionality to change)
## Test Plan 🧪
#### Behavior Without This Pull Request :coffin:
#### Behavior With This Pull Request :tada:
#### Related Unit Tests
---
# Checklist 📝
<!--- Go over all the following points, and put an `x` in all the boxes that apply. -->
<!--- If you're unsure about any of these, don't hesitate to ask. We're here to help! -->
- [ ] This patch was not authored or co-authored using [Generative Tooling](https://www.apache.org/legal/generative-tooling.html)
**Be nice. Be informative.**
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@kyuubi.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@kyuubi.apache.org
For additional commands, e-mail: notifications-help@kyuubi.apache.org
Re: [PR] [KYUUBI #6079] Web UI support Basic authN [kyuubi]
Posted by "beryllw (via GitHub)" <gi...@apache.org>.
beryllw commented on PR #6258:
URL: https://github.com/apache/kyuubi/pull/6258#issuecomment-2044461179
> seems a explicitly logout button is required, otherwise user can not switch login user
https://stackoverflow.com/questions/233507/how-to-log-out-user-from-web-site-using-basic-authentication
> What you have to do is have the user click a logout link, and send a ‘401 Unauthorized’ in response, using the same realm and at the same URL folder level as the normal 401 you send requesting a login.
Simple implementation is that when the user clicks the logout button, the server returns 401.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@kyuubi.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@kyuubi.apache.org
For additional commands, e-mail: notifications-help@kyuubi.apache.org
Re: [PR] [KYUUBI #6079] Web UI support Basic authN [kyuubi]
Posted by "pan3793 (via GitHub)" <gi...@apache.org>.
pan3793 commented on PR #6258:
URL: https://github.com/apache/kyuubi/pull/6258#issuecomment-2046469003
> Simple implementation is that when the user clicks the logout button, the server returns 401.
We can use a specific endpoint for that, so other endpoints behavior won't be affected
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@kyuubi.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@kyuubi.apache.org
For additional commands, e-mail: notifications-help@kyuubi.apache.org
Re: [PR] [KYUUBI #4079] Web UI support Basic authN [kyuubi]
Posted by "codecov-commenter (via GitHub)" <gi...@apache.org>.
codecov-commenter commented on PR #6258:
URL: https://github.com/apache/kyuubi/pull/6258#issuecomment-2041320351
## [Codecov](https://app.codecov.io/gh/apache/kyuubi/pull/6258?dropdown=coverage&src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) Report
All modified and coverable lines are covered by tests :white_check_mark:
> Project coverage is 58.38%. Comparing base [(`c8a40d9`)](https://app.codecov.io/gh/apache/kyuubi/commit/c8a40d97eb4af53f329a6833f216b5da83c12863?dropdown=coverage&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) to head [(`f3f7f3d`)](https://app.codecov.io/gh/apache/kyuubi/pull/6258?dropdown=coverage&src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache).
> Report is 2 commits behind head on master.
<details><summary>Additional details and impacted files</summary>
```diff
@@ Coverage Diff @@
## master #6258 +/- ##
============================================
- Coverage 58.42% 58.38% -0.04%
Complexity 24 24
============================================
Files 651 651
Lines 39513 39522 +9
Branches 5441 5443 +2
============================================
- Hits 23086 23076 -10
- Misses 13954 13963 +9
- Partials 2473 2483 +10
```
</details>
[:umbrella: View full report in Codecov by Sentry](https://app.codecov.io/gh/apache/kyuubi/pull/6258?dropdown=coverage&src=pr&el=continue&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache).
:loudspeaker: Have feedback on the report? [Share it here](https://about.codecov.io/codecov-pr-comment-feedback/?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache).
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@kyuubi.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@kyuubi.apache.org
For additional commands, e-mail: notifications-help@kyuubi.apache.org
Re: [PR] [KYUUBI #6079] Web UI support Basic authN [kyuubi]
Posted by "pan3793 (via GitHub)" <gi...@apache.org>.
pan3793 closed pull request #6258: [KYUUBI #6079] Web UI support Basic authN
URL: https://github.com/apache/kyuubi/pull/6258
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@kyuubi.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@kyuubi.apache.org
For additional commands, e-mail: notifications-help@kyuubi.apache.org
Re: [PR] [KYUUBI #6079] Web UI support Basic authN [kyuubi]
Posted by "pan3793 (via GitHub)" <gi...@apache.org>.
pan3793 commented on PR #6258:
URL: https://github.com/apache/kyuubi/pull/6258#issuecomment-2044278381
seems a explicitly logout button is required, otherwise user can not switch login user
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@kyuubi.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@kyuubi.apache.org
For additional commands, e-mail: notifications-help@kyuubi.apache.org
Re: [PR] [KYUUBI #6079] Web UI support Basic authN [kyuubi]
Posted by "beryllw (via GitHub)" <gi...@apache.org>.
beryllw commented on PR #6258:
URL: https://github.com/apache/kyuubi/pull/6258#issuecomment-2046958582
> Simple implementation is that when the user clicks the logout button, the server returns 401.
Login users cannot be displayed in this way.Implemented on the frontend code instead.


--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@kyuubi.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@kyuubi.apache.org
For additional commands, e-mail: notifications-help@kyuubi.apache.org
Re: [PR] [KYUUBI #6079] Web UI support Basic authN [kyuubi]
Posted by "beryllw (via GitHub)" <gi...@apache.org>.
beryllw commented on PR #6258:
URL: https://github.com/apache/kyuubi/pull/6258#issuecomment-2044191363
@pan3793 cc
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@kyuubi.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@kyuubi.apache.org
For additional commands, e-mail: notifications-help@kyuubi.apache.org
Re: [PR] [KYUUBI #4079] Web UI support Basic authN [kyuubi]
Posted by "pan3793 (via GitHub)" <gi...@apache.org>.
pan3793 commented on PR #6258:
URL: https://github.com/apache/kyuubi/pull/6258#issuecomment-2041825963
found 2 issues:
- if user input the wrong username/password, no way to correct
- only SQL editor works
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@kyuubi.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@kyuubi.apache.org
For additional commands, e-mail: notifications-help@kyuubi.apache.org
Re: [PR] [KYUUBI #6079] Web UI support Basic authN [kyuubi]
Posted by "pan3793 (via GitHub)" <gi...@apache.org>.
pan3793 commented on PR #6258:
URL: https://github.com/apache/kyuubi/pull/6258#issuecomment-2041993829
> Other api return http status code 405? Because the current user is not an administrator.
Oh, I see ...
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@kyuubi.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@kyuubi.apache.org
For additional commands, e-mail: notifications-help@kyuubi.apache.org
Re: [PR] [KYUUBI #4079] Web UI support Basic authN [kyuubi]
Posted by "pan3793 (via GitHub)" <gi...@apache.org>.
pan3793 commented on PR #6258:
URL: https://github.com/apache/kyuubi/pull/6258#issuecomment-2041754594
> If user login success, other http request will auto attach a basic auth header.
interesting, let me verify it locally
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@kyuubi.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@kyuubi.apache.org
For additional commands, e-mail: notifications-help@kyuubi.apache.org
Re: [PR] [KYUUBI #6079] Web UI support Basic authN [kyuubi]
Posted by "pan3793 (via GitHub)" <gi...@apache.org>.
pan3793 commented on PR #6258:
URL: https://github.com/apache/kyuubi/pull/6258#issuecomment-2049478303
Thanks, merged to master/1.9.1
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@kyuubi.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@kyuubi.apache.org
For additional commands, e-mail: notifications-help@kyuubi.apache.org
Re: [PR] [KYUUBI #4079] Web UI support Basic authN [kyuubi]
Posted by "beryllw (via GitHub)" <gi...@apache.org>.
beryllw commented on PR #6258:
URL: https://github.com/apache/kyuubi/pull/6258#issuecomment-2041282068
> the frontend code needs to be modified so that all http request will attach a basic auth header
frontend code means web-ui code?
If user login success, other http request will auto attach a basic auth header.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@kyuubi.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@kyuubi.apache.org
For additional commands, e-mail: notifications-help@kyuubi.apache.org
Re: [PR] [KYUUBI #4079] Web UI support Basic authN [kyuubi]
Posted by "pan3793 (via GitHub)" <gi...@apache.org>.
pan3793 commented on PR #6258:
URL: https://github.com/apache/kyuubi/pull/6258#issuecomment-2038047357
the frontend code needs to be modified so that all http request will attach a basic auth header
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@kyuubi.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@kyuubi.apache.org
For additional commands, e-mail: notifications-help@kyuubi.apache.org
Re: [PR] [KYUUBI #6079] Web UI support Basic authN [kyuubi]
Posted by "beryllw (via GitHub)" <gi...@apache.org>.
beryllw commented on PR #6258:
URL: https://github.com/apache/kyuubi/pull/6258#issuecomment-2041942609
> * if user input the wrong username/password, no way to correct
Fixing this will change the current authentication logic, i will fix it soon.
> * only SQL editor works
Other api return http status code 405? Because the current user is not an administrator.
```
org.apache.kyuubi.server.api.v1.AdminResource#sessions
if (!fe.isAdministrator(userName)) {
throw new NotAllowedException(
s"$userName is not allowed to list all live sessions")
}
```
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@kyuubi.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@kyuubi.apache.org
For additional commands, e-mail: notifications-help@kyuubi.apache.org