BewareMyPower commented on a change in pull request #14044:
URL: https://github.com/apache/pulsar/pull/14044#discussion_r800574348
##########
File path:
pulsar-broker-common/src/main/java/org/apache/pulsar/broker/authentication/AuthenticationProviderToken.java
##########
@@ -363,4 +369,59 @@ public boolean isExpired() {
return expiration < System.currentTimeMillis();
}
}
+
+ private static final class TokenAuthenticationHttpState implements
AuthenticationState {
+
+ private final AuthenticationProviderToken provider;
+ private AuthenticationDataSource authenticationDataSource;
+ private Jwt<?, Claims> jwt;
+ private long expiration;
+
+ TokenAuthenticationHttpState(AuthenticationProviderToken provider,
HttpServletRequest request)
+ throws AuthenticationException {
+ this.provider = provider;
+ String httpHeaderValue = request.getHeader(HTTP_HEADER_NAME);
+ if (httpHeaderValue == null ||
!httpHeaderValue.startsWith(HTTP_HEADER_VALUE_PREFIX)) {
+ throw new AuthenticationException("Invalid HTTP Authorization
header");
+ }
+
+ // Remove prefix
+ String token =
httpHeaderValue.substring(HTTP_HEADER_VALUE_PREFIX.length());
+ this.jwt = provider.authenticateToken(token);
+ this.authenticationDataSource = new
AuthenticationDataHttps(request);
+ if (jwt.getBody().getExpiration() != null) {
+ this.expiration = jwt.getBody().getExpiration().getTime();
+ } else {
+ // Disable expiration
+ this.expiration = Long.MAX_VALUE;
+ }
+ }
+
+ @Override
+ public String getAuthRole() throws AuthenticationException {
+ return provider.getPrincipal(jwt);
+ }
+
+ @Override
+ public AuthenticationDataSource getAuthDataSource() {
+ return authenticationDataSource;
+ }
+
+ @Override
+ public AuthData authenticate(AuthData authData) throws
AuthenticationException {
+ return null;
Review comment:
Could you explain why does `authenticate` return null here? If it's
because `authenticate` could never be called, it's better to throw an exception
here and explain the reason in comments.
##########
File path:
pulsar-broker-common/src/main/java/org/apache/pulsar/broker/authentication/AuthenticationProviderToken.java
##########
@@ -363,4 +369,59 @@ public boolean isExpired() {
return expiration < System.currentTimeMillis();
}
}
+
+ private static final class TokenAuthenticationHttpState implements
AuthenticationState {
+
+ private final AuthenticationProviderToken provider;
+ private AuthenticationDataSource authenticationDataSource;
+ private Jwt<?, Claims> jwt;
+ private long expiration;
Review comment:
```suggestion
private final AuthenticationDataSource authenticationDataSource;
private final Jwt<?, Claims> jwt;
private final long expiration;
```
##########
File path:
pulsar-broker-common/src/main/java/org/apache/pulsar/broker/web/AuthenticationFilter.java
##########
@@ -76,8 +77,15 @@ public void doFilter(ServletRequest request, ServletResponse
response, FilterCha
// not sasl type, return role directly.
String role =
authenticationService.authenticateHttpRequest((HttpServletRequest) request);
request.setAttribute(AuthenticatedRoleAttributeName, role);
- request.setAttribute(AuthenticatedDataAttributeName,
- new AuthenticationDataHttps((HttpServletRequest) request));
+ String authMethodName =
httpRequest.getHeader("X-Pulsar-Auth-Method-Name");
+ if (authMethodName != null &&
authenticationService.getAuthenticationProvider(authMethodName) != null) {
+ AuthenticationState authenticationState =
authenticationService
+
.getAuthenticationProvider(authMethodName).newHttpAuthState(httpRequest);
+ request.setAttribute(AuthenticatedDataAttributeName,
authenticationState.getAuthDataSource());
Review comment:
@gaoran10 I think it's because here `request.setAttribute` accepts an
`AuthenticationDataSource` rather than a `AuthenticationDataHttps`.
--
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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
