You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@pulsar.apache.org by "nodece (via GitHub)" <gi...@apache.org> on 2023/01/30 08:46:13 UTC
[GitHub] [pulsar] nodece opened a new pull request, #19359: [fix][authentication] Make AuthenticationProviderBasic refreshes data and role
nodece opened a new pull request, #19359:
URL: https://github.com/apache/pulsar/pull/19359
### Motivation
`AuthenticationProviderBasic` defaults to use the `OneStageAuthenticationState`, which can only refresh one authentication data and role. Once the client sends the different authentication data to the broker, this auth state doesn't work.
### Modifications
- Override the `newAuthState` to using `BasicAuthenticationState` instead of `OneStageAuthenticationState`
- Override the `authenticateAsync` to simplify default implementation
### Verifying this change
- [x] Make sure that the change passes the CI checks.
Added test
### Documentation
<!-- DO NOT REMOVE THIS SECTION. CHECK THE PROPER BOX ONLY. -->
- [ ] `doc` <!-- Your PR contains doc changes. Please attach the local preview screenshots (run `sh start.sh` at `pulsar/site2/website`) to your PR description, or else your PR might not get merged. -->
- [ ] `doc-required` <!-- Your PR changes impact docs and you will update later -->
- [x] `doc-not-needed` <!-- Your PR changes do not impact docs -->
- [ ] `doc-complete` <!-- Docs have been already added -->
--
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: commits-unsubscribe@pulsar.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [pulsar] nodece commented on a diff in pull request #19359: [fix][authentication] Make AuthenticationProviderBasic refreshes data and role
Posted by "nodece (via GitHub)" <gi...@apache.org>.
nodece commented on code in PR #19359:
URL: https://github.com/apache/pulsar/pull/19359#discussion_r1093971890
##########
pulsar-broker-common/src/main/java/org/apache/pulsar/broker/authentication/AuthenticationProviderBasic.java:
##########
@@ -177,4 +196,55 @@ public String getPassword() {
return password;
}
}
+
+ private static class BasicAuthenticationState implements AuthenticationState {
+
+ private final AuthenticationProviderBasic authenticationProvider;
+ private final SocketAddress remoteAddress;
+ private final SSLSession sslSession;
+ private String role;
+ private AuthenticationDataSource authenticationDataSource;
+
+ public BasicAuthenticationState(SocketAddress remoteAddress, SSLSession sslSession,
+ AuthenticationProviderBasic authenticationProvider) {
+ this.remoteAddress = remoteAddress;
+ this.sslSession = sslSession;
+ this.authenticationProvider = authenticationProvider;
+ }
+
+ @Override
+ public String getAuthRole() throws AuthenticationException {
+ if (authenticationDataSource == null) {
+ throw new AuthenticationException("Must authenticate before calling getAuthRole");
+ }
+
+ return role;
+ }
+
+ @Override
+ public AuthData authenticate(AuthData authData) throws AuthenticationException {
+ AuthenticationDataSource dataSource =
+ new AuthenticationDataCommand(new String(authData.getBytes(), StandardCharsets.UTF_8),
+ remoteAddress, sslSession);
+ role = authenticationProvider.authenticationBasic(dataSource);
+ authenticationDataSource = dataSource;
+
+ return null;
+ }
+
+ @Override
+ public AuthenticationDataSource getAuthDataSource() {
+ return authenticationDataSource;
+ }
+
+ @Override
+ public boolean isComplete() {
+ return authenticationDataSource != null;
+ }
+
+ @Override
+ public boolean isExpired() {
+ return false;
+ }
Review Comment:
Good explanation!
--
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: commits-unsubscribe@pulsar.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [pulsar] michaeljmarshall commented on a diff in pull request #19359: [fix][authentication] Make AuthenticationProviderBasic refreshes data and role
Posted by "michaeljmarshall (via GitHub)" <gi...@apache.org>.
michaeljmarshall commented on code in PR #19359:
URL: https://github.com/apache/pulsar/pull/19359#discussion_r1092821526
##########
pulsar-broker-common/src/main/java/org/apache/pulsar/broker/authentication/AuthenticationProviderBasic.java:
##########
@@ -177,4 +196,55 @@ public String getPassword() {
return password;
}
}
+
+ private static class BasicAuthenticationState implements AuthenticationState {
+
+ private final AuthenticationProviderBasic authenticationProvider;
+ private final SocketAddress remoteAddress;
+ private final SSLSession sslSession;
+ private String role;
+ private AuthenticationDataSource authenticationDataSource;
+
+ public BasicAuthenticationState(SocketAddress remoteAddress, SSLSession sslSession,
+ AuthenticationProviderBasic authenticationProvider) {
+ this.remoteAddress = remoteAddress;
+ this.sslSession = sslSession;
+ this.authenticationProvider = authenticationProvider;
+ }
+
+ @Override
+ public String getAuthRole() throws AuthenticationException {
+ if (authenticationDataSource == null) {
+ throw new AuthenticationException("Must authenticate before calling getAuthRole");
+ }
+
+ return role;
+ }
+
+ @Override
+ public AuthData authenticate(AuthData authData) throws AuthenticationException {
+ AuthenticationDataSource dataSource =
+ new AuthenticationDataCommand(new String(authData.getBytes(), StandardCharsets.UTF_8),
+ remoteAddress, sslSession);
+ role = authenticationProvider.authenticationBasic(dataSource);
+ authenticationDataSource = dataSource;
+
+ return null;
+ }
+
+ @Override
+ public AuthenticationDataSource getAuthDataSource() {
+ return authenticationDataSource;
+ }
+
+ @Override
+ public boolean isComplete() {
+ return authenticationDataSource != null;
+ }
+
+ @Override
+ public boolean isExpired() {
+ return false;
+ }
Review Comment:
The fact that the auth state is never expired and that the authentication happens in a single stag indicates to me that there isn't any state to keep track of.
--
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: commits-unsubscribe@pulsar.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [pulsar] nodece closed pull request #19359: [fix][authentication] Make AuthenticationProviderBasic refreshes data and role
Posted by "nodece (via GitHub)" <gi...@apache.org>.
nodece closed pull request #19359: [fix][authentication] Make AuthenticationProviderBasic refreshes data and role
URL: https://github.com/apache/pulsar/pull/19359
--
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: commits-unsubscribe@pulsar.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org