[CELEBORN-1318] Support celeborn http authentication #2440
Conversation
turboFei
force-pushed
the
http_auth
branch
2 times, most recently
from
c581317
to
c3b0e56
Compare
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #2440 +/- ##
==========================================
- Coverage 40.17% 38.75% -1.42%
==========================================
Files 218 227 +9
Lines 13742 13779 +37
Branches 1214 1209 -5
==========================================
- Hits 5520 5339 -181
- Misses 7905 8142 +237
+ Partials 317 298 -19 ☔ View full report in Codecov by Sentry. |
turboFei
force-pushed
the
http_auth
branch
2 times, most recently
from
16f83e0
to
f4cf2a8
Compare
turboFei
changed the title
turboFei
force-pushed
the
http_auth
branch
2 times, most recently
from
d6644cc
to
3dcd221
Compare
turboFei
force-pushed
the
http_auth
branch
2 times, most recently
from
ae44d1c
to
4b2dc43
Compare
|
This is PR is almost ready. cc @RexXiong @SteNicholas I have not added spnego testing, it will introduce some testing dependencies. |
|
also cc @pan3793 |
| * @throws AuthenticationException When a user is found to be invalid by the implementation | ||
| */ | ||
| @throws[AuthenticationException] | ||
| def authenticate(user: String, password: String): Unit |
There was a problem hiding this comment.
return a java.security.Principal instead. you can refer to Trino's io.trino.spi.security.HeaderAuthenticator and io.trino.spi.security.PasswordAuthenticator
| * @throws AuthenticationException When the token is found to be invalid by the implementation | ||
| */ | ||
| @throws[AuthenticationException] | ||
| def authenticate(token: String): String |
|
|
||
| package org.apache.celeborn.common.authentication | ||
|
|
||
| import javax.security.sasl.AuthenticationException |
There was a problem hiding this comment.
is this SASL related? looks like abuse
| .createWithDefault("X-Real-IP") | ||
|
|
||
| val MASTER_HTTP_AUTH_BASIC_PROVIDER_CLASS: ConfigEntry[String] = | ||
| buildConf("celeborn.master.http.auth.basic.provider.class") |
There was a problem hiding this comment.
let's remove .class and allow it to accept both the short name of the built-in implementations alias and user-provided 3-rd full class name
| buildConf("celeborn.master.http.auth.basic.provider.class") | |
| buildConf("celeborn.master.http.auth.basic.provider") |
| .doc("A comma-separated list of master http auth supported schemes." + | ||
| "<ul>" + | ||
| " <li>SPNEGO: Kerberos/GSSAPI authentication.</li>" + | ||
| " <li>BASIC: User-defined password authentication, anonymous by default.</li>" + |
There was a problem hiding this comment.
| " <li>BASIC: User-defined password authentication, anonymous by default.</li>" + | |
| " <li>BASIC: User-defined password authentication, the concreted implementation is configurable via `celeborn.master.http.auth.basic.provider`.</li>" + |
|
This PR is stale because it has been open 20 days with no activity. Remove stale label or comment or this will be closed in 10 days. |
What changes were proposed in this pull request?
Support celeborn master/worker http authentication.
Why are the changes needed?
Authentication is needed for celeborn admin APIs.
Does this PR introduce any user-facing change?
Yes, introduce authentication related config items, but does not break the current behavior.
How was this patch tested?
Added UT for BASIC and Bearer authentication.