You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@pulsar.apache.org by GitBox <gi...@apache.org> on 2022/02/23 07:53:50 UTC

[GitHub] [pulsar] eolivelli commented on a change in pull request #14044: [broker][authentication]Support pass http auth status

eolivelli commented on a change in pull request #14044:
URL: https://github.com/apache/pulsar/pull/14044#discussion_r812626278



##########
File path: pulsar-client/src/main/java/org/apache/pulsar/client/impl/auth/AuthenticationDataBasic.java
##########
@@ -29,10 +29,13 @@
     private static final String HTTP_HEADER_NAME = "Authorization";
     private String httpAuthToken;
     private String commandAuthToken;
+    private final Map<String, String> headers = new HashMap<>();
 
     public AuthenticationDataBasic(String userId, String password) {
         httpAuthToken = "Basic " + Base64.getEncoder().encodeToString((userId + ":" + password).getBytes());
         commandAuthToken = userId + ":" + password;
+        headers.put(HTTP_HEADER_NAME, httpAuthToken);

Review comment:
       What about using a UnmofiableMap here?

##########
File path: pulsar-client/src/main/java/org/apache/pulsar/client/impl/auth/AuthenticationDataToken.java
##########
@@ -19,19 +19,21 @@
 
 package org.apache.pulsar.client.impl.auth;
 
-import java.util.Collections;
+import java.util.HashMap;
 import java.util.Map;
 import java.util.Set;
 import java.util.function.Supplier;
 import org.apache.pulsar.client.api.AuthenticationDataProvider;
 
 public class AuthenticationDataToken implements AuthenticationDataProvider {
     public static final String HTTP_HEADER_NAME = "Authorization";
-
     private final Supplier<String> tokenSupplier;
+    private final Map<String, String> headers = new HashMap<>();
 
     public AuthenticationDataToken(Supplier<String> tokenSupplier) {
         this.tokenSupplier = tokenSupplier;
+        headers.put(PULSAR_AUTH_METHOD_NAME, AuthenticationToken.AUTH_METHOD_NAME);

Review comment:
       Unmodifiable?

##########
File path: pulsar-broker-common/src/main/java/org/apache/pulsar/broker/authentication/AuthenticationProviderToken.java
##########
@@ -320,31 +324,49 @@ private String getTokenAudience(ServiceConfiguration conf) throws IllegalArgumen
                 SocketAddress remoteAddress,
                 SSLSession sslSession) throws AuthenticationException {
             this.provider = provider;
-            this.remoteAddress = remoteAddress;
-            this.sslSession = sslSession;
-            this.authenticate(authData);
+            String token = new String(authData.getBytes(), UTF_8);
+            this.authenticationDataSource = new AuthenticationDataCommand(token, remoteAddress, sslSession);
+            this.checkExpiration(token);
+        }
+
+        TokenAuthenticationState(
+                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.authenticationDataSource = new AuthenticationDataHttps(request);
+            this.checkExpiration(token);
         }
 
         @Override
         public String getAuthRole() throws AuthenticationException {
             return provider.getPrincipal(jwt);
         }
 
+        /**
+         * Here is an explanation of why the null value is returned.
+         * pulsar-broker-common/src/main/java/org/apache/pulsar/broker/authentication/AuthenticationState.java#L49

Review comment:
       Lines change in the future, please refer to a method name, using javadoc syntax

##########
File path: pulsar-client/src/main/java/org/apache/pulsar/client/impl/auth/oauth2/AuthenticationDataOAuth2.java
##########
@@ -30,11 +30,12 @@
     public static final String HTTP_HEADER_NAME = "Authorization";
 
     private final String accessToken;
-    private final Set<Map.Entry<String, String>> headers;
+    private final Map<String, String> headers = new HashMap<>();
 
     public AuthenticationDataOAuth2(String accessToken) {
         this.accessToken = accessToken;
-        this.headers = Collections.singletonMap(HTTP_HEADER_NAME, "Bearer " + accessToken).entrySet();
+        headers.put(HTTP_HEADER_NAME, "Bearer " + accessToken);
+        headers.put(PULSAR_AUTH_METHOD_NAME, AuthenticationOAuth2.AUTH_METHOD_NAME);

Review comment:
       Unmodifiable

##########
File path: pulsar-client/src/main/java/org/apache/pulsar/client/impl/auth/AuthenticationDataTls.java
##########
@@ -42,6 +45,8 @@
     @SuppressFBWarnings(value = "SE_TRANSIENT_FIELD_NOT_RESTORED",
             justification = "Using custom serializer which Findbugs can't detect")
     private transient Supplier<ByteArrayInputStream> certStreamProvider, keyStreamProvider, trustStoreStreamProvider;
+    private final Map<String, String> headers = Collections.singletonMap(

Review comment:
       This can be static




-- 
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