[KYUUBI #6041] RESTful API supports isolated authentication configuration #6042

beryllw · 2024-02-04T09:05:02Z

🔍 Description

Issue References 🔗

This pull request fixes #6041

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 🔖

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

Behavior With This Pull Request 🎉

Related Unit Tests

Checklist 📝

This patch was not authored or co-authored using Generative Tooling

Be nice. Be informative.

pan3793 · 2024-02-04T09:41:27Z

kyuubi-common/src/main/scala/org/apache/kyuubi/config/KyuubiConf.scala

@@ -797,6 +797,14 @@ object KyuubiConf {
    .checkValues(AuthTypes)
    .createWithDefault(Seq(AuthTypes.NONE.toString))

+  val RESTFUL_AUTHENTICATION: ConfigEntry[Boolean] =
+    buildConf("kyuubi.restful.security.enabled")


such a flag is too crude

agree. Now I just turn off RESTful API authentication. Is there anything else I need to consider or implement?

THRIFT-HTTP and RESTful are different things, your change affects both.

We should make the configuration intuitive

kyuubi.authentication= // fallback to `kyuubi.authentication` if not configure kyuubi.frontend.mysql.authentication= kyuubi.frontend.rest.authentication=

agree 👍. Isolated authentication method configuration is better.

…figuration

beryllw · 2024-02-05T08:08:13Z

docs/configuration/settings.md

@@ -246,6 +246,7 @@ You can configure the Kyuubi properties in `$KYUUBI_HOME/conf/kyuubi-defaults.co
 | kyuubi.frontend.mysql.worker.keepalive.time            | PT1M               | Time(ms) that an idle async thread of the command execution thread pool will wait for a new task to arrive before terminating in MySQL frontend service                                                                                                                                                                                                                                                                                                                                                                     | duration | 1.4.0 |
 | kyuubi.frontend.protocols                              | THRIFT_BINARY,REST | A comma-separated list for all frontend protocols <ul> <li>THRIFT_BINARY - HiveServer2 compatible thrift binary protocol.</li> <li>THRIFT_HTTP - HiveServer2 compatible thrift http protocol.</li> <li>REST - Kyuubi defined REST API(experimental).</li>  <li>MYSQL - MySQL compatible text protocol(experimental).</li>  <li>TRINO - Trino compatible http protocol(experimental).</li> </ul>                                                                                                                             | seq      | 1.4.0 |
 | kyuubi.frontend.proxy.http.client.ip.header            | X-Real-IP          | The HTTP header to record the real client IP address. If your server is behind a load balancer or other proxy, the server will see this load balancer or proxy IP address as the client IP address, to get around this common issue, most load balancers or proxies offer the ability to record the real remote IP address in an HTTP header that will be added to the request for other devices to use. Note that, because the header value can be specified to any IP address, so it will not be used for authentication. | string   | 1.6.0 |
+| kyuubi.frontend.rest.authentication                    | NONE               | A comma-separated list of client authentication types. It fallback to `kyuubi.authentication` if not configure.                                                                                                                                                                                                                                                                                                                                                                                                             | seq      | 1.9.0 |


mysql and other forntend protocols would implement in another PR.

beryllw · 2024-02-06T09:21:16Z

@pan3793 @turboFei cc

beryllw · 2024-02-20T12:23:31Z

Look like MYSQL, TRINO, THRIFT_BINARY(KyuubiTBinaryFrontendService) and THRIFT_HTTP(KyuubiTHttpFrontendService) are not required to support isolated authentication configuration at this time, so I only support RESTful API.

Not sure if I've missed anything, could you help me review this code? @pan3793 @turboFei cc

codecov-commenter · 2024-02-20T13:21:28Z

Codecov Report

Attention: Patch coverage is 92.50000% with 3 lines in your changes are missing coverage. Please review.

Project coverage is 61.08%. Comparing base (a2179cc) to head (8ab6c16).
Report is 21 commits behind head on master.

Files	Patch %	Lines
...authentication/AuthenticationProviderFactory.scala	80.00%	0 Missing and 1 partial ⚠️
...che/kyuubi/server/KyuubiTHttpFrontendService.scala	50.00%	0 Missing and 1 partial ⚠️
...ver/http/authentication/AuthenticationFilter.scala	80.00%	0 Missing and 1 partial ⚠️

Additional details and impacted files

@@            Coverage Diff            @@
##             master    #6042   +/-   ##
=========================================
  Coverage     61.07%   61.08%           
  Complexity       23       23           
=========================================
  Files           623      623           
  Lines         37160    37231   +71     
  Branches       5038     5042    +4     
=========================================
+ Hits          22696    22742   +46     
- Misses        12014    12019    +5     
- Partials       2450     2470   +20

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

pan3793 · 2024-02-20T15:07:59Z

kyuubi-common/src/main/scala/org/apache/kyuubi/config/KyuubiConf.scala

+      .version("1.9.0")
+      .serverOnly
+      .fallbackConf(AUTHENTICATION_METHOD)
+
  val AUTHENTICATION_CUSTOM_CLASS: OptionalConfigEntry[String] =
    buildConf("kyuubi.authentication.custom.class")


should we make it isolated too?

beryllw · 2024-02-21T10:24:00Z

AUTHENTICATION_METHOD and AUTHENTICATION_CUSTOM_CLASS related to FrontendProtocol, i only support rest api isolated authentication for this time. Other FrontendProtocol also can support, if necessary we can implement it.

beryllw · 2024-02-22T10:36:22Z

@pan3793 @turboFei cc

pan3793 · 2024-02-22T13:17:03Z

Have a quick look, overall LGTM.

As many community users request to use Web UI while enabling security on Thrift Binary protocol, would you mind doing a manual test to make sure everything goes well in a such case?

Additionally, if you are interested in the Web UI tech stack, we may want basic authN support for Web UI - that means, when LDAP/JDBC/CUSTOM auth method is configured, pop something like below to allow user login via username and password

beryllw · 2024-02-23T12:29:22Z

As many community users request to use Web UI while enabling security on Thrift Binary protocol, would you mind doing a manual test to make sure everything goes well in a such case?

backported to kyuubi 1.8.0, with following configuration, rest api/v1 and thrift binary authentication test OK.

kyuubi.authentication   KERBEROS
kyuubi.frontend.rest.authentication     NONE

kyuubi.authentication   KERBEROS
kyuubi.frontend.rest.authentication     JDBC

Is there anything else need to do?

github-actions bot added module:server module:common labels Feb 4, 2024

pan3793 reviewed Feb 4, 2024

View reviewed changes

github-actions bot added the kind:documentation Documentation is a feature! label Feb 4, 2024

[KYUUBI apache#6041] RESTful API supports isolated authentication con…

b899d7c

…figuration

beryllw force-pushed the kyuubi_6041 branch from 017894d to b899d7c Compare February 5, 2024 08:01

beryllw commented Feb 5, 2024

View reviewed changes

beryllw requested a review from pan3793 February 5, 2024 08:09

beryllw changed the title ~~[KYUUBI #6041] RESTful API supports isolated authentication eable configuration~~ [KYUUBI #6041] RESTful API supports isolated authentication configuration Feb 6, 2024

fix test

8a877af

pan3793 reviewed Feb 20, 2024

View reviewed changes

support rest api user defined authentication

30bc564

beryllw force-pushed the kyuubi_6041 branch from dc9857a to 30bc564 Compare February 22, 2024 07:50

wangjunbo added 5 commits February 22, 2024 16:03

Remove unnecessary code changes

711049c

Remove unnecessary code changes

4e05065

clean code

dee5341

clean code

8359674

remove unused import

9ac664e

beryllw requested a review from pan3793 February 22, 2024 09:51

add more test

0cfe358

beryllw mentioned this pull request Feb 23, 2024

[TASK][MEDIUM] Web UI support Basic authN #6079

Closed

4 tasks

fix test

8ab6c16

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[KYUUBI #6041] RESTful API supports isolated authentication configuration #6042

[KYUUBI #6041] RESTful API supports isolated authentication configuration #6042

beryllw commented Feb 4, 2024

pan3793 Feb 4, 2024

beryllw Feb 4, 2024

pan3793 Feb 4, 2024 •

edited

Loading

beryllw Feb 5, 2024

beryllw Feb 5, 2024

beryllw commented Feb 6, 2024

beryllw commented Feb 20, 2024

codecov-commenter commented Feb 20, 2024 •

edited

Loading

pan3793 Feb 20, 2024 •

edited

Loading

beryllw Feb 21, 2024 •

edited

Loading

beryllw commented Feb 21, 2024 •

edited

Loading

beryllw commented Feb 22, 2024

pan3793 commented Feb 22, 2024 •

edited

Loading

beryllw commented Feb 23, 2024 •

edited

Loading

[KYUUBI #6041] RESTful API supports isolated authentication configuration #6042

Are you sure you want to change the base?

[KYUUBI #6041] RESTful API supports isolated authentication configuration #6042

Conversation

beryllw commented Feb 4, 2024

🔍 Description

Issue References 🔗

Describe Your Solution 🔧

Types of changes 🔖

Test Plan 🧪

Behavior Without This Pull Request ⚰️

Behavior With This Pull Request 🎉

Related Unit Tests

Checklist 📝

pan3793 Feb 4, 2024

Choose a reason for hiding this comment

beryllw Feb 4, 2024

Choose a reason for hiding this comment

pan3793 Feb 4, 2024 • edited Loading

Choose a reason for hiding this comment

beryllw Feb 5, 2024

Choose a reason for hiding this comment

beryllw Feb 5, 2024

Choose a reason for hiding this comment

beryllw commented Feb 6, 2024

beryllw commented Feb 20, 2024

codecov-commenter commented Feb 20, 2024 • edited Loading

Codecov Report

pan3793 Feb 20, 2024 • edited Loading

Choose a reason for hiding this comment

beryllw Feb 21, 2024 • edited Loading

Choose a reason for hiding this comment

beryllw commented Feb 21, 2024 • edited Loading

beryllw commented Feb 22, 2024

pan3793 commented Feb 22, 2024 • edited Loading

beryllw commented Feb 23, 2024 • edited Loading

pan3793 Feb 4, 2024 •

edited

Loading

codecov-commenter commented Feb 20, 2024 •

edited

Loading

pan3793 Feb 20, 2024 •

edited

Loading

beryllw Feb 21, 2024 •

edited

Loading

beryllw commented Feb 21, 2024 •

edited

Loading

pan3793 commented Feb 22, 2024 •

edited

Loading

beryllw commented Feb 23, 2024 •

edited

Loading