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.
   ![phoSgHuZKz](https://github.com/apache/kyuubi/assets/25627922/c207be75-d0e3-4479-871a-eab299f8c3a2)
   ![mssFmLoEiD](https://github.com/apache/kyuubi/assets/25627922/e9b2e02e-f7b0-478a-ae13-47454e251710)
   


-- 
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