-
Notifications
You must be signed in to change notification settings - Fork 334
Introduce authenticate in Fluss. #577
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
1d3d21f
to
b8dfc70
Compare
2b2191b
to
ecbd770
Compare
@wuchong , I have rebased this pr into main, would you like to help review it? |
|
I'm going to review this PR, could you rebase to the main branch? |
ecbd770
to
b966db1
Compare
@wuchong , Done it |
316740d
to
345d35a
Compare
fluss-common/src/main/java/com/alibaba/fluss/security/auth/AuthenticationFactory.java
Outdated
Show resolved
Hide resolved
fluss-common/src/main/java/com/alibaba/fluss/security/auth/AuthenticationFactory.java
Outdated
Show resolved
Hide resolved
fluss-common/src/main/java/com/alibaba/fluss/security/auth/AuthenticationFactory.java
Outdated
Show resolved
Hide resolved
fluss-common/src/main/java/com/alibaba/fluss/security/auth/AuthenticationFactory.java
Outdated
Show resolved
Hide resolved
fluss-common/src/main/java/com/alibaba/fluss/security/auth/AuthenticationFactory.java
Show resolved
Hide resolved
...nk/fluss-flink-common/src/main/java/com/alibaba/fluss/flink/catalog/FlinkCatalogFactory.java
Outdated
Show resolved
Hide resolved
...link/fluss-flink-common/src/main/java/com/alibaba/fluss/flink/catalog/FlinkTableFactory.java
Outdated
Show resolved
Hide resolved
...ink/fluss-flink-common/src/test/java/com/alibaba/fluss/flink/catalog/FlinkCatalogITCase.java
Outdated
Show resolved
Hide resolved
fluss-common/src/main/java/com/alibaba/fluss/security/auth/ClientAuthenticator.java
Outdated
Show resolved
Hide resolved
fluss-rpc/src/main/java/com/alibaba/fluss/rpc/netty/server/NettyServerHandler.java
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM. I appended a commit to address the following minor comemtns.
byte[] token = authenticateRequest.getToken(); | ||
byte[] challenge; | ||
if (!authenticator.isCompleted() | ||
&& (challenge = authenticator.evaluateResponse(token)) != null) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
should evaluateResponse
first then check authenticator.isCompleted()
?
* @since 0.7 | ||
*/ | ||
public class AuthenticationFactory { | ||
private static final String SERVER_AUTHENTICATOR_PREFIX = "security."; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
not used
*/ | ||
@SuppressWarnings("unchecked") | ||
private static <T extends AuthenticationPlugin> T discoverPlugin( | ||
Configuration configuration, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
not used
final Iterator<AuthenticationPlugin> foundPlugins = pluginIteratorsSupplier.get(); | ||
while (foundPlugins.hasNext()) { | ||
AuthenticationPlugin plugin = foundPlugins.next(); | ||
if (plugin.authProtocol().equals(protocol) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We may use uppercase when developing the auth plugin(such as: RAM
, SSL
, SASL
), but we can loose this to equalsIgnoreCase
to allow users use lowercase in client side:
client.security.protocol: ram
client.security.ram.ak: xxxx
instead of having to use
client.security.protocol: RAM
client.security.RAM.ak: xxxx
fluss-rpc/src/main/java/com/alibaba/fluss/rpc/netty/server/NettyServerHandler.java
Outdated
Show resolved
Hide resolved
@loserwang1024 please check the appended commit again. |
Co-authored-by: Jark Wu <[email protected]>
Co-authored-by: Jark Wu <[email protected]>
Purpose
Linked issue: close #484 . This Pr is depended on #531 .
Brief change log
Introduce authenticate in Fluss.
Tests
com.alibaba.fluss.rpc.netty.authenticate.AuthenticationTest
API and Format
Config
server.authenticate.protocol.map
: A map defining the authentication protocol for each listener. The format is 'listenerName1:protocol1,listenerName2:protocol2', e.g., 'INTERNAL:PLAINTEXT,CLIENT:GSSAPI'. Each listener can be associated with a specific authentication protocol. Listeners not included in the map will use PLAINTEXT by default, which does not require authentication.The config of one protocol:
client.authenticate.${protocol}.xxx
server.authenticate.${protocol}.xxx
Interface
AuthenticationPlugin for client and server:
Authenticator for client and server:
Principal to identify user:
Documentation