You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@kyuubi.apache.org by "George314159 (via GitHub)" <gi...@apache.org> on 2024/08/13 04:27:22 UTC
[PR] [KYUUBI #6590] Support http bearer token authentication [kyuubi]
George314159 opened a new pull request, #6608:
URL: https://github.com/apache/kyuubi/pull/6608
# :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 #6590
## 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.
Support http bearer token authentication by adding an new interface.
## 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)
- [ x] 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! -->
- [x ] 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 #6590] Support http bearer token authentication [kyuubi]
Posted by "codecov-commenter (via GitHub)" <gi...@apache.org>.
codecov-commenter commented on PR #6608:
URL: https://github.com/apache/kyuubi/pull/6608#issuecomment-2288048081
## [Codecov](https://app.codecov.io/gh/apache/kyuubi/pull/6608?dropdown=coverage&src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) Report
Attention: Patch coverage is `0%` with `87 lines` in your changes missing coverage. Please review.
> Project coverage is 0.00%. Comparing base [(`d3e1768`)](https://app.codecov.io/gh/apache/kyuubi/commit/d3e17680f58601bf526929f82fd6f732f4d1644e?dropdown=coverage&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) to head [(`f2bbfbf`)](https://app.codecov.io/gh/apache/kyuubi/commit/f2bbfbf7e9a3a7a0737ed0aee5b267868a72f8f6?dropdown=coverage&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.
| [Files](https://app.codecov.io/gh/apache/kyuubi/pull/6608?dropdown=coverage&src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | Patch % | Lines |
|---|---|---|
| [...p/authentication/BearerAuthenticationHandler.scala](https://app.codecov.io/gh/apache/kyuubi/pull/6608?src=pr&el=tree&filepath=kyuubi-server%2Fsrc%2Fmain%2Fscala%2Forg%2Fapache%2Fkyuubi%2Fserver%2Fhttp%2Fauthentication%2FBearerAuthenticationHandler.scala&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache#diff-a3l1dWJpLXNlcnZlci9zcmMvbWFpbi9zY2FsYS9vcmcvYXBhY2hlL2t5dXViaS9zZXJ2ZXIvaHR0cC9hdXRoZW50aWNhdGlvbi9CZWFyZXJBdXRoZW50aWNhdGlvbkhhbmRsZXIuc2NhbGE=) | 0.00% | [29 Missing :warning: ](https://app.codecov.io/gh/apache/kyuubi/pull/6608?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) |
| [...authentication/AuthenticationProviderFactory.scala](https://app.codecov.io/gh/apache/kyuubi/pull/6608?src=pr&el=tree&filepath=kyuubi-common%2Fsrc%2Fmain%2Fscala%2Forg%2Fapache%2Fkyuubi%2Fservice%2Fauthentication%2FAuthenticationProviderFactory.scala&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache#diff-a3l1dWJpLWNvbW1vbi9zcmMvbWFpbi9zY2FsYS9vcmcvYXBhY2hlL2t5dXViaS9zZXJ2aWNlL2F1dGhlbnRpY2F0aW9uL0F1dGhlbnRpY2F0aW9uUHJvdmlkZXJGYWN0b3J5LnNjYWxh) | 0.00% | [15 Missing :warning: ](https://app.codecov.io/gh/apache/kyuubi/pull/6608?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) |
| [...kyuubi/service/authentication/BasicPrincipal.scala](https://app.codecov.io/gh/apache/kyuubi/pull/6608?src=pr&el=tree&filepath=kyuubi-common%2Fsrc%2Fmain%2Fscala%2Forg%2Fapache%2Fkyuubi%2Fservice%2Fauthentication%2FBasicPrincipal.scala&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache#diff-a3l1dWJpLWNvbW1vbi9zcmMvbWFpbi9zY2FsYS9vcmcvYXBhY2hlL2t5dXViaS9zZXJ2aWNlL2F1dGhlbnRpY2F0aW9uL0Jhc2ljUHJpbmNpcGFsLnNjYWxh) | 0.00% | [10 Missing :warning: ](https://app.codecov.io/gh/apache/kyuubi/pull/6608?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) |
| [...in/scala/org/apache/kyuubi/config/KyuubiConf.scala](https://app.codecov.io/gh/apache/kyuubi/pull/6608?src=pr&el=tree&filepath=kyuubi-common%2Fsrc%2Fmain%2Fscala%2Forg%2Fapache%2Fkyuubi%2Fconfig%2FKyuubiConf.scala&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache#diff-a3l1dWJpLWNvbW1vbi9zcmMvbWFpbi9zY2FsYS9vcmcvYXBhY2hlL2t5dXViaS9jb25maWcvS3l1dWJpQ29uZi5zY2FsYQ==) | 0.00% | [9 Missing :warning: ](https://app.codecov.io/gh/apache/kyuubi/pull/6608?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) |
| [...ver/http/authentication/AuthenticationFilter.scala](https://app.codecov.io/gh/apache/kyuubi/pull/6608?src=pr&el=tree&filepath=kyuubi-server%2Fsrc%2Fmain%2Fscala%2Forg%2Fapache%2Fkyuubi%2Fserver%2Fhttp%2Fauthentication%2FAuthenticationFilter.scala&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache#diff-a3l1dWJpLXNlcnZlci9zcmMvbWFpbi9zY2FsYS9vcmcvYXBhY2hlL2t5dXViaS9zZXJ2ZXIvaHR0cC9hdXRoZW50aWNhdGlvbi9BdXRoZW50aWNhdGlvbkZpbHRlci5zY2FsYQ==) | 0.00% | [9 Missing :warning: ](https://app.codecov.io/gh/apache/kyuubi/pull/6608?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) |
| [...che/kyuubi/service/authentication/Credential.scala](https://app.codecov.io/gh/apache/kyuubi/pull/6608?src=pr&el=tree&filepath=kyuubi-common%2Fsrc%2Fmain%2Fscala%2Forg%2Fapache%2Fkyuubi%2Fservice%2Fauthentication%2FCredential.scala&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache#diff-a3l1dWJpLWNvbW1vbi9zcmMvbWFpbi9zY2FsYS9vcmcvYXBhY2hlL2t5dXViaS9zZXJ2aWNlL2F1dGhlbnRpY2F0aW9uL0NyZWRlbnRpYWwuc2NhbGE=) | 0.00% | [7 Missing :warning: ](https://app.codecov.io/gh/apache/kyuubi/pull/6608?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) |
| [...apache/kyuubi/server/http/util/HttpAuthUtils.scala](https://app.codecov.io/gh/apache/kyuubi/pull/6608?src=pr&el=tree&filepath=kyuubi-server%2Fsrc%2Fmain%2Fscala%2Forg%2Fapache%2Fkyuubi%2Fserver%2Fhttp%2Futil%2FHttpAuthUtils.scala&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache#diff-a3l1dWJpLXNlcnZlci9zcmMvbWFpbi9zY2FsYS9vcmcvYXBhY2hlL2t5dXViaS9zZXJ2ZXIvaHR0cC91dGlsL0h0dHBBdXRoVXRpbHMuc2NhbGE=) | 0.00% | [4 Missing :warning: ](https://app.codecov.io/gh/apache/kyuubi/pull/6608?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) |
| [...tication/AnonymousAuthenticationProviderImpl.scala](https://app.codecov.io/gh/apache/kyuubi/pull/6608?src=pr&el=tree&filepath=kyuubi-common%2Fsrc%2Fmain%2Fscala%2Forg%2Fapache%2Fkyuubi%2Fservice%2Fauthentication%2FAnonymousAuthenticationProviderImpl.scala&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache#diff-a3l1dWJpLWNvbW1vbi9zcmMvbWFpbi9zY2FsYS9vcmcvYXBhY2hlL2t5dXViaS9zZXJ2aWNlL2F1dGhlbnRpY2F0aW9uL0Fub255bW91c0F1dGhlbnRpY2F0aW9uUHJvdmlkZXJJbXBsLnNjYWxh) | 0.00% | [2 Missing :warning: ](https://app.codecov.io/gh/apache/kyuubi/pull/6608?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) |
| [...yuubi/server/http/authentication/AuthSchemes.scala](https://app.codecov.io/gh/apache/kyuubi/pull/6608?src=pr&el=tree&filepath=kyuubi-server%2Fsrc%2Fmain%2Fscala%2Forg%2Fapache%2Fkyuubi%2Fserver%2Fhttp%2Fauthentication%2FAuthSchemes.scala&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache#diff-a3l1dWJpLXNlcnZlci9zcmMvbWFpbi9zY2FsYS9vcmcvYXBhY2hlL2t5dXViaS9zZXJ2ZXIvaHR0cC9hdXRoZW50aWNhdGlvbi9BdXRoU2NoZW1lcy5zY2FsYQ==) | 0.00% | [1 Missing :warning: ](https://app.codecov.io/gh/apache/kyuubi/pull/6608?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) |
| [...tp/authentication/BasicAuthenticationHandler.scala](https://app.codecov.io/gh/apache/kyuubi/pull/6608?src=pr&el=tree&filepath=kyuubi-server%2Fsrc%2Fmain%2Fscala%2Forg%2Fapache%2Fkyuubi%2Fserver%2Fhttp%2Fauthentication%2FBasicAuthenticationHandler.scala&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache#diff-a3l1dWJpLXNlcnZlci9zcmMvbWFpbi9zY2FsYS9vcmcvYXBhY2hlL2t5dXViaS9zZXJ2ZXIvaHR0cC9hdXRoZW50aWNhdGlvbi9CYXNpY0F1dGhlbnRpY2F0aW9uSGFuZGxlci5zY2FsYQ==) | 0.00% | [1 Missing :warning: ](https://app.codecov.io/gh/apache/kyuubi/pull/6608?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) |
<details><summary>Additional details and impacted files</summary>
```diff
@@ Coverage Diff @@
## master #6608 +/- ##
======================================
Coverage 0.00% 0.00%
======================================
Files 677 680 +3
Lines 41907 41989 +82
Branches 5721 5731 +10
======================================
- Misses 41907 41989 +82
```
</details>
[:umbrella: View full report in Codecov by Sentry](https://app.codecov.io/gh/apache/kyuubi/pull/6608?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 #6618] Support http bearer token authentication for REST protocol [kyuubi]
Posted by "pan3793 (via GitHub)" <gi...@apache.org>.
pan3793 closed pull request #6608: [KYUUBI #6618] Support http bearer token authentication for REST protocol
URL: https://github.com/apache/kyuubi/pull/6608
--
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 #6590] Support http bearer token authentication [kyuubi]
Posted by "turboFei (via GitHub)" <gi...@apache.org>.
turboFei commented on PR #6608:
URL: https://github.com/apache/kyuubi/pull/6608#issuecomment-2287954170
Please also add UT in `KyuubiRestAuthenticationSuite`.
--
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 #6590] Support http bearer token authentication [kyuubi]
Posted by "turboFei (via GitHub)" <gi...@apache.org>.
turboFei commented on code in PR #6608:
URL: https://github.com/apache/kyuubi/pull/6608#discussion_r1716055838
##########
kyuubi-common/src/main/scala/org/apache/kyuubi/config/KyuubiConf.scala:
##########
@@ -845,6 +845,23 @@ object KyuubiConf {
.stringConf
.createOptional
+ val AUTHENTICATION_CUSTOM_BASIC_CLASS: ConfigEntry[Option[String]] =
+ buildConf("kyuubi.authentication.custom.basic.class")
+ .doc("User-defined Basic authentication implementation of " +
+ "org.apache.kyuubi.service.authentication.PasswdAuthenticationProvider")
+ .version("1.6.0")
+ .serverOnly
+ .fallbackConf(AUTHENTICATION_CUSTOM_CLASS)
+
+ val AUTHENTICATION_CUSTOM_BEARER_CLASS: OptionalConfigEntry[String] =
+ buildConf("kyuubi.authentication.custom.bearer.class")
+ .doc("User-defined Token authentication implementation of " +
+ "org.apache.kyuubi.service.authentication.TokenAuthenticationProvider")
+ .version("1.6.0")
Review Comment:
1.10.0
--
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 #6590] Support http bearer token authentication [kyuubi]
Posted by "turboFei (via GitHub)" <gi...@apache.org>.
turboFei commented on code in PR #6608:
URL: https://github.com/apache/kyuubi/pull/6608#discussion_r1715627585
##########
kyuubi-common/src/main/scala/org/apache/kyuubi/service/authentication/AuthTypes.scala:
##########
@@ -20,5 +20,5 @@ package org.apache.kyuubi.service.authentication
object AuthTypes extends Enumeration {
type AuthType = Value
- val NOSASL, NONE, LDAP, JDBC, KERBEROS, CUSTOM = Value
+ val NOSASL, NONE, LDAP, JDBC, KERBEROS, CUSTOM, BEARER = Value
Review Comment:
not needed
--
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 #6590] Support http bearer token authentication [kyuubi]
Posted by "turboFei (via GitHub)" <gi...@apache.org>.
turboFei commented on code in PR #6608:
URL: https://github.com/apache/kyuubi/pull/6608#discussion_r1715630253
##########
kyuubi-server/src/main/scala/org/apache/kyuubi/server/http/authentication/AuthSchemes.scala:
##########
@@ -20,5 +20,5 @@ package org.apache.kyuubi.server.http.authentication
object AuthSchemes extends Enumeration {
type AuthScheme = Value
- val BASIC, NEGOTIATE, KYUUBI_INTERNAL = Value
+ val BASIC, NEGOTIATE, BEARER, KYUUBI_INTERNAL = Value
Review Comment:
val BASIC, BEARER, NEGOTIATE, KYUUBI_INTERNAL = Value
--
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 #6590] Support http bearer token authentication [kyuubi]
Posted by "turboFei (via GitHub)" <gi...@apache.org>.
turboFei commented on code in PR #6608:
URL: https://github.com/apache/kyuubi/pull/6608#discussion_r1715625170
##########
kyuubi-common/src/main/scala/org/apache/kyuubi/config/KyuubiConf.scala:
##########
@@ -839,7 +845,8 @@ object KyuubiConf {
val AUTHENTICATION_CUSTOM_CLASS: OptionalConfigEntry[String] =
buildConf("kyuubi.authentication.custom.class")
.doc("User-defined authentication implementation of " +
- "org.apache.kyuubi.service.authentication.PasswdAuthenticationProvider")
+ "org.apache.kyuubi.service.authentication.PasswdAuthenticationProvider " +
+ "or org.apache.kyuubi.service.authentication.TokenAuthenticationProvider")
Review Comment:
we shall involve two new config items.
`kyuubi.authentication.custom.basic.class` and `kyuubi.authentication.custom.bearer.class`
--
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 #6590] Support http bearer token authentication [kyuubi]
Posted by "turboFei (via GitHub)" <gi...@apache.org>.
turboFei commented on code in PR #6608:
URL: https://github.com/apache/kyuubi/pull/6608#discussion_r1716076392
##########
kyuubi-common/src/main/scala/org/apache/kyuubi/service/authentication/AuthenticationProviderFactory.scala:
##########
@@ -55,6 +55,32 @@ object AuthenticationProviderFactory {
case _ => throw new AuthenticationException("Not a valid authentication method")
}
+ def getBasicAuthenticationProvider(
+ method: AuthMethod,
+ conf: KyuubiConf): PasswdAuthenticationProvider = method match {
+ case AuthMethods.NONE => new AnonymousAuthenticationProviderImpl
+ case AuthMethods.LDAP => new LdapAuthenticationProviderImpl(conf)
+ case AuthMethods.JDBC => new JdbcAuthenticationProviderImpl(conf)
+ case AuthMethods.CUSTOM =>
+ val className = conf.get(KyuubiConf.AUTHENTICATION_CUSTOM_BASIC_CLASS)
+ if (className.isEmpty) {
+ throw new AuthenticationException(
+ "authentication.custom.basic.class must be set when auth method was CUSTOM.")
+ }
+ ClassUtils.createInstance(className.get, classOf[PasswdAuthenticationProvider], conf)
+ case _ => throw new AuthenticationException("Not a valid authentication method")
+ }
+
+ def getTokenAuthenticationProvider(
+ providerClass: String,
+ conf: KyuubiConf): TokenAuthenticationProvider = {
+ if (providerClass.isEmpty) {
+ throw new AuthenticationException(
+ "authentication.custom.bearer.class must be set when auth method was BEARER.")
Review Comment:
`authentication.custom.bearer.class must be set for token authentication`.
--
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 #6590] Support http bearer token authentication [kyuubi]
Posted by "turboFei (via GitHub)" <gi...@apache.org>.
turboFei commented on code in PR #6608:
URL: https://github.com/apache/kyuubi/pull/6608#discussion_r1716079858
##########
kyuubi-common/src/main/scala/org/apache/kyuubi/service/authentication/TokenAuthenticationProvider.scala:
##########
@@ -0,0 +1,36 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.kyuubi.service.authentication
+
+import javax.security.sasl.AuthenticationException
+
+trait TokenAuthenticationProvider {
+
+ /**
+ * The authenticate method is called by the Kyuubi Server authentication layer
+ * to authenticate users for their requests.
+ * If the token is to be granted, return nothing/throw nothing.
+ * When the token is to be disallowed, throw an appropriate [[AuthenticationException]].
+ *
+ * @param token The token received over the connection request
+ *
+ * @throws AuthenticationException When the token is found to be invalid by the implementation
+ */
+ @throws[AuthenticationException]
+ def authenticate(token: String): Unit
Review Comment:
Please follow the same pattern likes https://github.com/apache/celeborn/blob/main/spi/src/main/java/org/apache/celeborn/spi/authentication/TokenAuthenticationProvider.java.
--
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 #6590] Support http bearer token authentication [kyuubi]
Posted by "turboFei (via GitHub)" <gi...@apache.org>.
turboFei commented on code in PR #6608:
URL: https://github.com/apache/kyuubi/pull/6608#discussion_r1716075812
##########
kyuubi-common/src/main/scala/org/apache/kyuubi/service/authentication/AuthenticationProviderFactory.scala:
##########
@@ -55,6 +55,32 @@ object AuthenticationProviderFactory {
case _ => throw new AuthenticationException("Not a valid authentication method")
}
+ def getBasicAuthenticationProvider(
+ method: AuthMethod,
+ conf: KyuubiConf): PasswdAuthenticationProvider = method match {
+ case AuthMethods.NONE => new AnonymousAuthenticationProviderImpl
+ case AuthMethods.LDAP => new LdapAuthenticationProviderImpl(conf)
+ case AuthMethods.JDBC => new JdbcAuthenticationProviderImpl(conf)
+ case AuthMethods.CUSTOM =>
+ val className = conf.get(KyuubiConf.AUTHENTICATION_CUSTOM_BASIC_CLASS)
+ if (className.isEmpty) {
+ throw new AuthenticationException(
+ "authentication.custom.basic.class must be set when auth method was CUSTOM.")
+ }
+ ClassUtils.createInstance(className.get, classOf[PasswdAuthenticationProvider], conf)
+ case _ => throw new AuthenticationException("Not a valid authentication method")
+ }
+
+ def getTokenAuthenticationProvider(
+ providerClass: String,
+ conf: KyuubiConf): TokenAuthenticationProvider = {
+ if (providerClass.isEmpty) {
Review Comment:
if (StringUtils.isBlank(providerClass))
--
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 #6590] Support http bearer token authentication [kyuubi]
Posted by "turboFei (via GitHub)" <gi...@apache.org>.
turboFei commented on code in PR #6608:
URL: https://github.com/apache/kyuubi/pull/6608#discussion_r1715623497
##########
kyuubi-common/src/main/scala/org/apache/kyuubi/config/KyuubiConf.scala:
##########
@@ -823,6 +824,11 @@ object KyuubiConf {
" <li><code>KERBEROS</code></li>" +
" </ul>" +
" </li>" +
+ " <li>SASL/OAUTHBEARER" +
+ " <ul>" +
+ " <li><code>BEARER</code></li>" +
+ " </ul>" +
+ " </li>" +
" </ul>" +
Review Comment:
Hi @George314159 thanks for raising this PR.
Some comments, for current design:
we do not need to add new item into kyuubi.authentication, for example, now I set `kyuubi.authentication=KERBEROS,CUSTOM` in server end, for `Bearer` authentication, it belongs to `CUSTOM` authentication.
--
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 #6618] Support http bearer token authentication for REST protocol [kyuubi]
Posted by "pan3793 (via GitHub)" <gi...@apache.org>.
pan3793 commented on code in PR #6608:
URL: https://github.com/apache/kyuubi/pull/6608#discussion_r1719220391
##########
kyuubi-common/src/main/scala/org/apache/kyuubi/service/authentication/AuthenticationProviderFactory.scala:
##########
@@ -62,4 +64,30 @@ object AuthenticationProviderFactory {
new AnonymousAuthenticationProviderImpl
}
}
+
+ def getHttpBasicAuthenticationProvider(
+ method: AuthMethod,
+ conf: KyuubiConf): PasswdAuthenticationProvider = method match {
+ case AuthMethods.NONE => new AnonymousAuthenticationProviderImpl
+ case AuthMethods.LDAP => new LdapAuthenticationProviderImpl(conf)
+ case AuthMethods.JDBC => new JdbcAuthenticationProviderImpl(conf)
+ case AuthMethods.CUSTOM =>
+ val className = conf.get(KyuubiConf.AUTHENTICATION_CUSTOM_BASIC_CLASS)
+ if (className.isEmpty) {
+ throw new AuthenticationException(
+ "authentication.custom.basic.class must be set for http basic authentication.")
+ }
Review Comment:
this is caused by admin's wrong configuration, should not throw `AuthenticationException`
```
require(className.nonEmpty, "xxxxx")
```
please correct the mentioned configuration key
`authentication.custom.basic.class` => `kyuubi.authentication.custom.basic.class`
##########
kyuubi-common/src/main/scala/org/apache/kyuubi/service/authentication/AuthenticationProviderFactory.scala:
##########
@@ -62,4 +64,30 @@ object AuthenticationProviderFactory {
new AnonymousAuthenticationProviderImpl
}
}
+
+ def getHttpBasicAuthenticationProvider(
+ method: AuthMethod,
+ conf: KyuubiConf): PasswdAuthenticationProvider = method match {
+ case AuthMethods.NONE => new AnonymousAuthenticationProviderImpl
+ case AuthMethods.LDAP => new LdapAuthenticationProviderImpl(conf)
+ case AuthMethods.JDBC => new JdbcAuthenticationProviderImpl(conf)
+ case AuthMethods.CUSTOM =>
+ val className = conf.get(KyuubiConf.AUTHENTICATION_CUSTOM_BASIC_CLASS)
+ if (className.isEmpty) {
+ throw new AuthenticationException(
+ "authentication.custom.basic.class must be set for http basic authentication.")
+ }
+ ClassUtils.createInstance(className.get, classOf[PasswdAuthenticationProvider], conf)
+ case _ => throw new AuthenticationException("Not a valid authentication method")
+ }
+
+ def getHttpBearerAuthenticationProvider(
+ providerClass: String,
+ conf: KyuubiConf): TokenAuthenticationProvider = {
+ if (StringUtils.isBlank(providerClass)) {
+ throw new AuthenticationException(
+ "authentication.custom.bearer.class must be set for http bearer authentication.")
Review Comment:
ditto
--
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 #6618] Support http bearer token authentication for REST protocol [kyuubi]
Posted by "turboFei (via GitHub)" <gi...@apache.org>.
turboFei commented on code in PR #6608:
URL: https://github.com/apache/kyuubi/pull/6608#discussion_r1719274561
##########
kyuubi-common/src/test/scala/org/apache/kyuubi/service/authentication/CustomAuthenticationProviderImplSuite.scala:
##########
@@ -21,7 +21,7 @@ import javax.security.sasl.AuthenticationException
import org.apache.kyuubi.KyuubiFunSuite
import org.apache.kyuubi.config.KyuubiConf
-import org.apache.kyuubi.service.authentication.AuthenticationProviderFactory.getAuthenticationProvider
+import org.apache.kyuubi.service.authentication.AuthenticationProviderFactory.{getAuthenticationProvider, getHttpBearerAuthenticationProvider}
class CustomAuthenticationProviderImplSuite extends KyuubiFunSuite {
test("Test user defined authentication") {
Review Comment:
this UT also need to be fixed
--
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 #6618] Support http bearer token authentication for REST protocol [kyuubi]
Posted by "turboFei (via GitHub)" <gi...@apache.org>.
turboFei commented on PR #6608:
URL: https://github.com/apache/kyuubi/pull/6608#issuecomment-2292209396
also cc @pan3793
--
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 #6618] Support http bearer token authentication for REST protocol [kyuubi]
Posted by "turboFei (via GitHub)" <gi...@apache.org>.
turboFei commented on PR #6608:
URL: https://github.com/apache/kyuubi/pull/6608#issuecomment-2292204789
could you update the PR description for more details? @George314159
--
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 #6590] Support http bearer token authentication [kyuubi]
Posted by "turboFei (via GitHub)" <gi...@apache.org>.
turboFei commented on code in PR #6608:
URL: https://github.com/apache/kyuubi/pull/6608#discussion_r1716077274
##########
kyuubi-common/src/main/scala/org/apache/kyuubi/service/authentication/EngineSecureAuthenticationProviderImpl.scala:
##########
@@ -17,8 +17,12 @@
package org.apache.kyuubi.service.authentication
-class EngineSecureAuthenticationProviderImpl extends PasswdAuthenticationProvider {
+class EngineSecureAuthenticationProviderImpl extends PasswdAuthenticationProvider
+ with TokenAuthenticationProvider {
override def authenticate(user: String, password: String): Unit = {
InternalSecurityAccessor.get().authToken(password)
}
+ override def authenticate(token: String): Unit = {
+ InternalSecurityAccessor.get().authToken(token)
+ }
Review Comment:
not needed for `EngineSecureAuthenticationProviderImpl`.
EngineSecureAuthenticationProviderImpl only for `PasswdAuthenticationProvider`.
--
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 #6618] Support http bearer token authentication for REST protocol [kyuubi]
Posted by "pan3793 (via GitHub)" <gi...@apache.org>.
pan3793 commented on PR #6608:
URL: https://github.com/apache/kyuubi/pull/6608#issuecomment-2292760152
LGTM
--
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 #6618] Support http bearer token authentication for REST protocol [kyuubi]
Posted by "pan3793 (via GitHub)" <gi...@apache.org>.
pan3793 commented on PR #6608:
URL: https://github.com/apache/kyuubi/pull/6608#issuecomment-2293304928
Thanks, merged to master
--
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 #6590] Support http bearer token authentication [kyuubi]
Posted by "turboFei (via GitHub)" <gi...@apache.org>.
turboFei commented on code in PR #6608:
URL: https://github.com/apache/kyuubi/pull/6608#discussion_r1715626758
##########
kyuubi-common/src/main/scala/org/apache/kyuubi/config/KyuubiConf.scala:
##########
@@ -839,7 +845,8 @@ object KyuubiConf {
val AUTHENTICATION_CUSTOM_CLASS: OptionalConfigEntry[String] =
buildConf("kyuubi.authentication.custom.class")
.doc("User-defined authentication implementation of " +
- "org.apache.kyuubi.service.authentication.PasswdAuthenticationProvider")
+ "org.apache.kyuubi.service.authentication.PasswdAuthenticationProvider " +
+ "or org.apache.kyuubi.service.authentication.TokenAuthenticationProvider")
Review Comment:
And for
`kyuubi.authentication.custom.basic.class`, it should fallback to `AUTHENTICATION_CUSTOM_CLASS`
.fallbackConf(AUTHENTICATION_CUSTOM_CLASS)
--
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 #6618] Support http bearer token authentication for REST protocol [kyuubi]
Posted by "turboFei (via GitHub)" <gi...@apache.org>.
turboFei commented on PR #6608:
URL: https://github.com/apache/kyuubi/pull/6608#issuecomment-2292501361
For ThriftHttp protocol, https://github.com/apache/kyuubi/pull/6621
--
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 #6590] Support http bearer token authentication [kyuubi]
Posted by "George314159 (via GitHub)" <gi...@apache.org>.
George314159 commented on PR #6608:
URL: https://github.com/apache/kyuubi/pull/6608#issuecomment-2287231496
Thanks for review, updated the design based on comments.
--
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 #6590] Support http bearer token authentication [kyuubi]
Posted by "turboFei (via GitHub)" <gi...@apache.org>.
turboFei commented on code in PR #6608:
URL: https://github.com/apache/kyuubi/pull/6608#discussion_r1716055369
##########
kyuubi-common/src/main/scala/org/apache/kyuubi/config/KyuubiConf.scala:
##########
@@ -845,6 +845,23 @@ object KyuubiConf {
.stringConf
.createOptional
+ val AUTHENTICATION_CUSTOM_BASIC_CLASS: ConfigEntry[Option[String]] =
+ buildConf("kyuubi.authentication.custom.basic.class")
+ .doc("User-defined Basic authentication implementation of " +
+ "org.apache.kyuubi.service.authentication.PasswdAuthenticationProvider")
Review Comment:
add `for http basic authentication` to make it clear.
##########
kyuubi-common/src/main/scala/org/apache/kyuubi/config/KyuubiConf.scala:
##########
@@ -845,6 +845,23 @@ object KyuubiConf {
.stringConf
.createOptional
+ val AUTHENTICATION_CUSTOM_BASIC_CLASS: ConfigEntry[Option[String]] =
+ buildConf("kyuubi.authentication.custom.basic.class")
+ .doc("User-defined Basic authentication implementation of " +
+ "org.apache.kyuubi.service.authentication.PasswdAuthenticationProvider")
+ .version("1.6.0")
+ .serverOnly
+ .fallbackConf(AUTHENTICATION_CUSTOM_CLASS)
+
+ val AUTHENTICATION_CUSTOM_BEARER_CLASS: OptionalConfigEntry[String] =
+ buildConf("kyuubi.authentication.custom.bearer.class")
+ .doc("User-defined Token authentication implementation of " +
+ "org.apache.kyuubi.service.authentication.TokenAuthenticationProvider")
+ .version("1.6.0")
Review Comment:
add `for http bearer authentication` to make it clear.
--
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 #6590] Support http bearer token authentication [kyuubi]
Posted by "turboFei (via GitHub)" <gi...@apache.org>.
turboFei commented on code in PR #6608:
URL: https://github.com/apache/kyuubi/pull/6608#discussion_r1716079254
##########
kyuubi-common/src/main/scala/org/apache/kyuubi/service/authentication/TokenAuthenticationProvider.scala:
##########
@@ -0,0 +1,36 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.kyuubi.service.authentication
+
+import javax.security.sasl.AuthenticationException
+
+trait TokenAuthenticationProvider {
+
+ /**
+ * The authenticate method is called by the Kyuubi Server authentication layer
+ * to authenticate users for their requests.
+ * If the token is to be granted, return nothing/throw nothing.
+ * When the token is to be disallowed, throw an appropriate [[AuthenticationException]].
+ *
+ * @param token The token received over the connection request
+ *
+ * @throws AuthenticationException When the token is found to be invalid by the implementation
+ */
+ @throws[AuthenticationException]
+ def authenticate(token: String): Unit
Review Comment:
def authenticate(credential: TokenCredential): Principal
--
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