Skip to content

Commit 3b2e674

Browse files
committed
[KYUUBI #5990] Always take the first declared SASL/PLAIN auth type
# 🔍 Description ## Issue References 🔗 #5185 changed the type of `kyuubi.authentication` from `Seq`(ordered) to `Set`(unordered), which break the assumption of ``` Note that: for SASL authentication, KERBEROS and PLAIN auth types are supported at the same time, and only the first specified PLAIN auth type is valid. ``` ## Describe Your Solution 🔧 Restore the type to Seq ## Types of changes 🔖 - [x] 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 🧪 UT is updated --- # Checklist 📝 - [x] This patch was not authored or co-authored using [Generative Tooling](https://www.apache.org/legal/generative-tooling.html) **Be nice. Be informative.** Closes #5990 from pan3793/auth-plain. Closes #5990 acae25f [Cheng Pan] fix doc cef7dba [Cheng Pan] fix 87b370f [Cheng Pan] Always take the first declared SASL/PLAIN auth type Authored-by: Cheng Pan <chengpan@apache.org> Signed-off-by: Cheng Pan <chengpan@apache.org>
1 parent 7b38889 commit 3b2e674

File tree

12 files changed

+29
-25
lines changed

12 files changed

+29
-25
lines changed

docs/configuration/settings.md

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -33,7 +33,7 @@ You can configure the Kyuubi properties in `$KYUUBI_HOME/conf/kyuubi-defaults.co
3333

3434
| Key | Default | Meaning | Type | Since |
3535
|-----------------------------------------------|-------------------|--------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------|--------|-------|
36-
| kyuubi.authentication | NONE | A comma-separated list of client authentication types.<ul> <li>NOSASL: raw transport.</li> <li>NONE: no authentication check.</li> <li>KERBEROS: Kerberos/GSSAPI authentication.</li> <li>CUSTOM: User-defined authentication.</li> <li>JDBC: JDBC query authentication.</li> <li>LDAP: Lightweight Directory Access Protocol authentication.</li></ul>The following tree describes the catalog of each option.<ul> <li><code>NOSASL</code></li> <li>SASL <ul> <li>SASL/PLAIN</li> <ul> <li><code>NONE</code></li> <li><code>LDAP</code></li> <li><code>JDBC</code></li> <li><code>CUSTOM</code></li> </ul> <li>SASL/GSSAPI <ul> <li><code>KERBEROS</code></li> </ul> </li> </ul> </li></ul> Note that: for SASL authentication, KERBEROS and PLAIN auth types are supported at the same time, and only the first specified PLAIN auth type is valid. | set | 1.0.0 |
36+
| kyuubi.authentication | NONE | A comma-separated list of client authentication types.<ul> <li>NOSASL: raw transport.</li> <li>NONE: no authentication check.</li> <li>KERBEROS: Kerberos/GSSAPI authentication.</li> <li>CUSTOM: User-defined authentication.</li> <li>JDBC: JDBC query authentication.</li> <li>LDAP: Lightweight Directory Access Protocol authentication.</li></ul>The following tree describes the catalog of each option.<ul> <li><code>NOSASL</code></li> <li>SASL <ul> <li>SASL/PLAIN</li> <ul> <li><code>NONE</code></li> <li><code>LDAP</code></li> <li><code>JDBC</code></li> <li><code>CUSTOM</code></li> </ul> <li>SASL/GSSAPI <ul> <li><code>KERBEROS</code></li> </ul> </li> </ul> </li></ul> Note that: for SASL authentication, KERBEROS and PLAIN auth types are supported at the same time, and only the first specified PLAIN auth type is valid. | seq | 1.0.0 |
3737
| kyuubi.authentication.custom.class | &lt;undefined&gt; | User-defined authentication implementation of org.apache.kyuubi.service.authentication.PasswdAuthenticationProvider | string | 1.3.0 |
3838
| kyuubi.authentication.jdbc.driver.class | &lt;undefined&gt; | Driver class name for JDBC Authentication Provider. | string | 1.6.0 |
3939
| kyuubi.authentication.jdbc.password | &lt;undefined&gt; | Database password for JDBC Authentication Provider. | string | 1.6.0 |

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

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -757,7 +757,7 @@ object KyuubiConf {
757757
.stringConf
758758
.createWithDefault("X-Real-IP")
759759

760-
val AUTHENTICATION_METHOD: ConfigEntry[Set[String]] = buildConf("kyuubi.authentication")
760+
val AUTHENTICATION_METHOD: ConfigEntry[Seq[String]] = buildConf("kyuubi.authentication")
761761
.doc("A comma-separated list of client authentication types." +
762762
"<ul>" +
763763
" <li>NOSASL: raw transport.</li>" +
@@ -793,9 +793,9 @@ object KyuubiConf {
793793
.serverOnly
794794
.stringConf
795795
.transformToUpperCase
796-
.toSet()
796+
.toSequence()
797797
.checkValues(AuthTypes)
798-
.createWithDefault(Set(AuthTypes.NONE.toString))
798+
.createWithDefault(Seq(AuthTypes.NONE.toString))
799799

800800
val AUTHENTICATION_CUSTOM_CLASS: OptionalConfigEntry[String] =
801801
buildConf("kyuubi.authentication.custom.class")

kyuubi-common/src/main/scala/org/apache/kyuubi/service/authentication/KyuubiAuthenticationFactory.scala

Lines changed: 12 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -37,11 +37,15 @@ import org.apache.kyuubi.shaded.thrift.transport.{TSaslServerTransport, TTranspo
3737

3838
class KyuubiAuthenticationFactory(conf: KyuubiConf, isServer: Boolean = true) extends Logging {
3939

40-
val authTypes: Set[AuthType] = conf.get(AUTHENTICATION_METHOD).map(AuthTypes.withName)
41-
val noSaslEnabled: Boolean = authTypes == Set(NOSASL)
40+
val authTypes: Seq[AuthType] = conf.get(AUTHENTICATION_METHOD).map(AuthTypes.withName)
41+
val saslDisabled: Boolean = authTypes == Seq(NOSASL)
4242
val kerberosEnabled: Boolean = authTypes.contains(KERBEROS)
43-
private val plainAuthTypeOpt = authTypes.filterNot(_.equals(KERBEROS))
44-
.filterNot(_.equals(NOSASL)).headOption
43+
44+
// take the first declared SASL/PLAIN auth type
45+
private val effectivePlainAuthType = authTypes.find {
46+
case NOSASL | KERBEROS => false
47+
case _ => true
48+
}
4549

4650
private val hadoopAuthServer: Option[HadoopThriftAuthBridgeServer] = {
4751
if (kerberosEnabled) {
@@ -70,7 +74,7 @@ class KyuubiAuthenticationFactory(conf: KyuubiConf, isServer: Boolean = true) ex
7074
}
7175

7276
def getTTransportFactory: TTransportFactory = {
73-
if (noSaslEnabled) {
77+
if (saslDisabled) {
7478
new TTransportFactory()
7579
} else {
7680
var transportFactory: TSaslServerTransport.Factory = null
@@ -87,7 +91,7 @@ class KyuubiAuthenticationFactory(conf: KyuubiConf, isServer: Boolean = true) ex
8791
case _ =>
8892
}
8993

90-
plainAuthTypeOpt match {
94+
effectivePlainAuthType match {
9195
case Some(plainAuthType) =>
9296
transportFactory = PlainSASLHelper.getTransportFactory(
9397
plainAuthType.toString,
@@ -152,8 +156,8 @@ object KyuubiAuthenticationFactory extends Logging {
152156
}
153157
}
154158

155-
def getValidPasswordAuthMethod(authTypes: Set[AuthType]): AuthMethod = {
156-
if (authTypes == Set(NOSASL)) AuthMethods.NONE
159+
def getValidPasswordAuthMethod(authTypes: Seq[AuthType]): AuthMethod = {
160+
if (authTypes == Seq(NOSASL)) AuthMethods.NONE
157161
else if (authTypes.contains(NONE)) AuthMethods.NONE
158162
else if (authTypes.contains(LDAP)) AuthMethods.LDAP
159163
else if (authTypes.contains(JDBC)) AuthMethods.JDBC

kyuubi-common/src/test/scala/org/apache/kyuubi/service/authentication/KyuubiAuthenticationFactorySuite.scala

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -56,33 +56,33 @@ class KyuubiAuthenticationFactorySuite extends KyuubiFunSuite {
5656
}
5757

5858
test("AuthType Other") {
59-
val conf = KyuubiConf().set(KyuubiConf.AUTHENTICATION_METHOD, Set("INVALID"))
59+
val conf = KyuubiConf().set(KyuubiConf.AUTHENTICATION_METHOD, Seq("INVALID"))
6060
interceptEquals[IllegalArgumentException] { new KyuubiAuthenticationFactory(conf) }(
6161
"The value of kyuubi.authentication should be one of" +
6262
" NOSASL, NONE, LDAP, JDBC, KERBEROS, CUSTOM, but was INVALID")
6363
}
6464

6565
test("AuthType LDAP") {
66-
val conf = KyuubiConf().set(KyuubiConf.AUTHENTICATION_METHOD, Set("LDAP"))
66+
val conf = KyuubiConf().set(KyuubiConf.AUTHENTICATION_METHOD, Seq("LDAP"))
6767
val authFactory = new KyuubiAuthenticationFactory(conf)
6868
authFactory.getTTransportFactory
6969
assert(Security.getProviders.exists(_.isInstanceOf[SaslPlainProvider]))
7070
}
7171

7272
test("AuthType KERBEROS w/o keytab/principal") {
73-
val conf = KyuubiConf().set(KyuubiConf.AUTHENTICATION_METHOD, Set("KERBEROS"))
73+
val conf = KyuubiConf().set(KyuubiConf.AUTHENTICATION_METHOD, Seq("KERBEROS"))
7474

7575
val factory = new KyuubiAuthenticationFactory(conf)
7676
val e = intercept[LoginException](factory.getTTransportFactory)
7777
assert(e.getMessage startsWith "Kerberos principal should have 3 parts")
7878
}
7979

8080
test("AuthType is NOSASL if only NOSASL is specified") {
81-
val conf = KyuubiConf().set(KyuubiConf.AUTHENTICATION_METHOD, Set("NOSASL"))
81+
val conf = KyuubiConf().set(KyuubiConf.AUTHENTICATION_METHOD, Seq("NOSASL"))
8282
var factory = new KyuubiAuthenticationFactory(conf)
8383
!factory.getTTransportFactory.isInstanceOf[TSaslServerTransport.Factory]
8484

85-
conf.set(KyuubiConf.AUTHENTICATION_METHOD, Set("NOSASL", "NONE"))
85+
conf.set(KyuubiConf.AUTHENTICATION_METHOD, Seq("NOSASL", "NONE"))
8686
factory = new KyuubiAuthenticationFactory(conf)
8787
factory.getTTransportFactory.isInstanceOf[TSaslServerTransport.Factory]
8888
}

kyuubi-server/src/main/scala/org/apache/kyuubi/server/http/ThriftHttpServlet.scala

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -136,7 +136,7 @@ class ThriftHttpServlet(
136136
} else SessionManager.setForwardedAddresses(List.empty[String])
137137

138138
// Generate new cookie and add it to the response
139-
if (requireNewCookie && !authFactory.noSaslEnabled) {
139+
if (requireNewCookie && !authFactory.saslDisabled) {
140140
val cookieToken = HttpAuthUtils.createCookieToken(clientUserName)
141141
val hs2Cookie = createCookie(signer.signCookie(cookieToken))
142142
if (isHttpOnlyCookie) response.setHeader("SET-COOKIE", getHttpOnlyCookieHeader(hs2Cookie))

kyuubi-server/src/test/scala/org/apache/kyuubi/RestClientTestHelper.scala

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -48,7 +48,7 @@ trait RestClientTestHelper extends RestFrontendTestHelper with KerberizedTestHel
4848
UserGroupInformation.setConfiguration(config)
4949
assert(UserGroupInformation.isSecurityEnabled)
5050

51-
val conf = KyuubiConf().set(KyuubiConf.AUTHENTICATION_METHOD, Set("KERBEROS", "LDAP", "CUSTOM"))
51+
val conf = KyuubiConf().set(KyuubiConf.AUTHENTICATION_METHOD, Seq("KERBEROS", "LDAP", "CUSTOM"))
5252
.set(KyuubiConf.SERVER_KEYTAB.key, testKeytab)
5353
.set(KyuubiConf.SERVER_PRINCIPAL, testPrincipal)
5454
.set(KyuubiConf.SERVER_SPNEGO_KEYTAB, testKeytab)

kyuubi-server/src/test/scala/org/apache/kyuubi/operation/KyuubiOperationKerberosAndPlainAuthSuite.scala

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -64,7 +64,7 @@ class KyuubiOperationKerberosAndPlainAuthSuite extends WithKyuubiServer with Ker
6464
assert(UserGroupInformation.isSecurityEnabled)
6565

6666
KyuubiConf()
67-
.set(KyuubiConf.AUTHENTICATION_METHOD, Set("KERBEROS", "LDAP", "CUSTOM"))
67+
.set(KyuubiConf.AUTHENTICATION_METHOD, Seq("KERBEROS", "LDAP", "CUSTOM"))
6868
.set(KyuubiConf.SERVER_KEYTAB, testKeytab)
6969
.set(KyuubiConf.SERVER_PRINCIPAL, testPrincipal)
7070
.set(KyuubiConf.AUTHENTICATION_LDAP_URL, ldapUrl)

kyuubi-server/src/test/scala/org/apache/kyuubi/operation/thrift/http/KyuubiOperationThriftHttpKerberosAndPlainAuthSuite.scala

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -49,7 +49,7 @@ class KyuubiOperationThriftHttpKerberosAndPlainAuthSuite
4949
UserGroupInformation.setConfiguration(config)
5050
assert(UserGroupInformation.isSecurityEnabled)
5151

52-
KyuubiConf().set(KyuubiConf.AUTHENTICATION_METHOD, Set("KERBEROS", "LDAP", "CUSTOM"))
52+
KyuubiConf().set(KyuubiConf.AUTHENTICATION_METHOD, Seq("KERBEROS", "LDAP", "CUSTOM"))
5353
.set(KyuubiConf.SERVER_KEYTAB, testKeytab)
5454
.set(KyuubiConf.SERVER_PRINCIPAL, testPrincipal)
5555
.set(KyuubiConf.AUTHENTICATION_LDAP_URL, ldapUrl)

kyuubi-server/src/test/scala/org/apache/kyuubi/server/api/v1/AdminResourceSuite.scala

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -51,7 +51,7 @@ class AdminResourceSuite extends KyuubiFunSuite with RestFrontendTestHelper {
5151
private val engineMgr = new KyuubiApplicationManager()
5252

5353
override protected lazy val conf: KyuubiConf = KyuubiConf()
54-
.set(AUTHENTICATION_METHOD, Set("CUSTOM"))
54+
.set(AUTHENTICATION_METHOD, Seq("CUSTOM"))
5555
.set(AUTHENTICATION_CUSTOM_CLASS, classOf[AnonymousAuthenticationProviderImpl].getName)
5656
.set(SERVER_ADMINISTRATORS, Set("admin001"))
5757
.set(ENGINE_IDLE_TIMEOUT, Duration.ofMinutes(3).toMillis)

kyuubi-server/src/test/scala/org/apache/kyuubi/server/api/v1/BatchesResourceSuite.scala

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -85,7 +85,7 @@ abstract class BatchesResourceSuiteBase extends KyuubiFunSuite
8585
override protected lazy val conf: KyuubiConf = {
8686
val testResourceDir = Paths.get(sparkBatchTestResource.get).getParent
8787
val kyuubiConf = KyuubiConf()
88-
.set(AUTHENTICATION_METHOD, Set("CUSTOM"))
88+
.set(AUTHENTICATION_METHOD, Seq("CUSTOM"))
8989
.set(AUTHENTICATION_CUSTOM_CLASS, classOf[AnonymousAuthenticationProviderImpl].getName)
9090
.set(SERVER_ADMINISTRATORS, Set("admin"))
9191
.set(BATCH_IMPL_VERSION, batchVersion)

0 commit comments

Comments
 (0)