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 2018/12/21 23:10:02 UTC

[GitHub] merlimat closed pull request #3239: [Pulsar-Broker-Common] Refactor AuthenticationProviderToken

merlimat closed pull request #3239: [Pulsar-Broker-Common] Refactor AuthenticationProviderToken
URL: https://github.com/apache/pulsar/pull/3239
 
 
   

This is a PR merged from a forked repository.
As GitHub hides the original diff on merge, it is displayed below for
the sake of provenance:

As this is a foreign pull request (from a fork), the diff is supplied
below (as it won't show otherwise due to GitHub magic):

diff --git a/pulsar-broker-common/src/main/java/org/apache/pulsar/broker/authentication/AuthenticationProviderToken.java b/pulsar-broker-common/src/main/java/org/apache/pulsar/broker/authentication/AuthenticationProviderToken.java
index 4ed88684ae..7e155c3a7f 100644
--- a/pulsar-broker-common/src/main/java/org/apache/pulsar/broker/authentication/AuthenticationProviderToken.java
+++ b/pulsar-broker-common/src/main/java/org/apache/pulsar/broker/authentication/AuthenticationProviderToken.java
@@ -34,7 +34,7 @@
 
 public class AuthenticationProviderToken implements AuthenticationProvider {
 
-    public final static String HTTP_HEADER_NAME = "Authorization";
+    final static String HTTP_HEADER_NAME = "Authorization";
     final static String HTTP_HEADER_VALUE_PREFIX = "Bearer ";
 
     // When simmetric key is configured
@@ -43,6 +43,8 @@
     // When public/private key pair is configured
     final static String CONF_TOKEN_PUBLIC_KEY = "tokenPublicKey";
 
+    final static String TOKEN = "token";
+
     private Key validationKey;
 
     @Override
@@ -57,33 +59,47 @@ public void initialize(ServiceConfiguration config) throws IOException {
 
     @Override
     public String getAuthMethodName() {
-        return "token";
+        return TOKEN;
     }
 
     @Override
     public String authenticate(AuthenticationDataSource authData) throws AuthenticationException {
-        String token = null;
+        // Get Token
+        String token = getToken(authData);
+
+        // Parse Token by validating
+        return parseToken(token);
+    }
 
+    private String getToken(AuthenticationDataSource authData) throws AuthenticationException {
         if (authData.hasDataFromCommand()) {
             // Authenticate Pulsar binary connection
-            token = authData.getCommandData();
+            return authData.getCommandData();
         } else if (authData.hasDataFromHttp()) {
             // Authentication HTTP request. The format here should be compliant to RFC-6750
-            // (https://tools.ietf.org/html/rfc6750#section-2.1). Eg:
-            //
-            // Authorization: Bearer xxxxxxxxxxxxx
+            // (https://tools.ietf.org/html/rfc6750#section-2.1). Eg: Authorization: Bearer xxxxxxxxxxxxx
             String httpHeaderValue = authData.getHttpHeader(HTTP_HEADER_NAME);
             if (httpHeaderValue == null || !httpHeaderValue.startsWith(HTTP_HEADER_VALUE_PREFIX)) {
                 throw new AuthenticationException("Invalid HTTP Authorization header");
             }
 
             // Remove prefix
-            token = httpHeaderValue.substring(HTTP_HEADER_VALUE_PREFIX.length());
+            String token = httpHeaderValue.substring(HTTP_HEADER_VALUE_PREFIX.length());
+            return validateToken(token);
         } else {
             throw new AuthenticationException("No token credentials passed");
         }
+    }
 
-        // Validate the token
+    private String validateToken(final String token) throws AuthenticationException {
+        if(StringUtils.isNotBlank(token)) {
+            return token;
+        } else {
+            throw new AuthenticationException("Blank token found");
+        }
+    }
+
+    private String parseToken(final String token) throws AuthenticationException {
         try {
             @SuppressWarnings("unchecked")
             Jwt<?, Claims> jwt = Jwts.parser()
@@ -99,28 +115,19 @@ public String authenticate(AuthenticationDataSource authData) throws Authenticat
     /**
      * Try to get the validation key for tokens from several possible config options.
      */
-    private static Key getValidationKey(ServiceConfiguration conf) throws IOException {
-        final boolean isPublicKey;
-        final String validationKeyConfig;
-
+    private Key getValidationKey(ServiceConfiguration conf) throws IOException {
         if (conf.getProperty(CONF_TOKEN_SECRET_KEY) != null
-                && !StringUtils.isBlank((String) conf.getProperty(CONF_TOKEN_SECRET_KEY))) {
-            isPublicKey = false;
-            validationKeyConfig = (String) conf.getProperty(CONF_TOKEN_SECRET_KEY);
+                && StringUtils.isNotBlank((String) conf.getProperty(CONF_TOKEN_SECRET_KEY))) {
+            final String validationKeyConfig = (String) conf.getProperty(CONF_TOKEN_SECRET_KEY);
+            final byte[] validationKey = AuthTokenUtils.readKeyFromUrl(validationKeyConfig);
+            return AuthTokenUtils.decodeSecretKey(validationKey);
         } else if (conf.getProperty(CONF_TOKEN_PUBLIC_KEY) != null
-                && !StringUtils.isBlank((String) conf.getProperty(CONF_TOKEN_PUBLIC_KEY))) {
-            isPublicKey = true;
-            validationKeyConfig = (String) conf.getProperty(CONF_TOKEN_PUBLIC_KEY);
-        } else {
-            throw new IOException("No secret key was provided for token authentication");
-        }
-
-        byte[] validationKey = AuthTokenUtils.readKeyFromUrl(validationKeyConfig);
-
-        if (isPublicKey) {
+                && StringUtils.isNotBlank((String) conf.getProperty(CONF_TOKEN_PUBLIC_KEY))) {
+            final String validationKeyConfig = (String) conf.getProperty(CONF_TOKEN_PUBLIC_KEY);
+            final byte[] validationKey = AuthTokenUtils.readKeyFromUrl(validationKeyConfig);
             return AuthTokenUtils.decodePublicKey(validationKey);
         } else {
-            return AuthTokenUtils.decodeSecretKey(validationKey);
+            throw new IOException("No secret key was provided for token authentication");
         }
     }
 }
diff --git a/pulsar-broker-common/src/test/java/org/apache/pulsar/broker/authentication/AuthenticationProviderTokenTest.java b/pulsar-broker-common/src/test/java/org/apache/pulsar/broker/authentication/AuthenticationProviderTokenTest.java
index 078a7b625a..36b14c6f3a 100644
--- a/pulsar-broker-common/src/test/java/org/apache/pulsar/broker/authentication/AuthenticationProviderTokenTest.java
+++ b/pulsar-broker-common/src/test/java/org/apache/pulsar/broker/authentication/AuthenticationProviderTokenTest.java
@@ -19,6 +19,7 @@
 package org.apache.pulsar.broker.authentication;
 
 import static org.testng.Assert.assertEquals;
+import static org.testng.Assert.assertNotNull;
 import static org.testng.Assert.fail;
 
 import io.jsonwebtoken.Claims;
@@ -48,6 +49,8 @@
 
 public class AuthenticationProviderTokenTest {
 
+    private static final String SUBJECT = "my-test-subject";
+
     @Test
     public void testInvalidInitialize() throws Exception {
         AuthenticationProviderToken provider = new AuthenticationProviderToken();
@@ -57,17 +60,18 @@ public void testInvalidInitialize() throws Exception {
             fail("should have failed");
         } catch (IOException e) {
             // Expected, secret key was not defined
+        } finally {
+            // currently, will not close any resource
+            provider.close();
         }
-
-        provider.close();
     }
 
     @Test
-    public void testSerializeSecretKey() throws Exception {
+    public void testSerializeSecretKey() {
         SecretKey secretKey = AuthTokenUtils.createSecretKey(SignatureAlgorithm.HS256);
 
         String token = Jwts.builder()
-                .setSubject("my-test-subject")
+                .setSubject(SUBJECT)
                 .signWith(secretKey)
                 .compact();
 
@@ -76,7 +80,9 @@ public void testSerializeSecretKey() throws Exception {
                 .setSigningKey(AuthTokenUtils.decodeSecretKey(secretKey.getEncoded()))
                 .parse(token);
 
-        System.out.println("Subject: " + jwt.getBody().getSubject());
+        assertNotNull(jwt);
+        assertNotNull(jwt.getBody());
+        assertEquals(jwt.getBody().getSubject(), SUBJECT);
     }
 
     @Test
@@ -87,7 +93,7 @@ public void testSerializeKeyPair() throws Exception {
         String publicKey = AuthTokenUtils.encodeKeyBase64(keyPair.getPublic());
 
         String token = AuthTokenUtils.createToken(AuthTokenUtils.decodePrivateKey(Decoders.BASE64.decode(privateKey)),
-                "my-test-subject",
+                SUBJECT,
                 Optional.empty());
 
         @SuppressWarnings("unchecked")
@@ -95,7 +101,9 @@ public void testSerializeKeyPair() throws Exception {
                 .setSigningKey(AuthTokenUtils.decodePublicKey(Decoders.BASE64.decode(publicKey)))
                 .parse(token);
 
-        System.out.println("Subject: " + jwt.getBody().getSubject());
+        assertNotNull(jwt);
+        assertNotNull(jwt.getBody());
+        assertEquals(jwt.getBody().getSubject(), SUBJECT);
     }
 
     @Test
@@ -103,7 +111,7 @@ public void testAuthSecretKey() throws Exception {
         SecretKey secretKey = AuthTokenUtils.createSecretKey(SignatureAlgorithm.HS256);
 
         AuthenticationProviderToken provider = new AuthenticationProviderToken();
-        assertEquals(provider.getAuthMethodName(), "token");
+        assertEquals(provider.getAuthMethodName(), AuthenticationProviderToken.TOKEN);
 
         Properties properties = new Properties();
         properties.setProperty(AuthenticationProviderToken.CONF_TOKEN_SECRET_KEY,
@@ -121,7 +129,7 @@ public void testAuthSecretKey() throws Exception {
             // expected, no credential passed
         }
 
-        String token = AuthTokenUtils.createToken(secretKey, "my-test-subject", Optional.empty());
+        String token = AuthTokenUtils.createToken(secretKey, SUBJECT, Optional.empty());
 
         // Pulsar protocol auth
         String subject = provider.authenticate(new AuthenticationDataSource() {
@@ -135,7 +143,7 @@ public String getCommandData() {
                 return token;
             }
         });
-        assertEquals(subject, "my-test-subject");
+        assertEquals(subject, SUBJECT);
 
         // HTTP protocol auth
         provider.authenticate(new AuthenticationDataSource() {
@@ -146,17 +154,17 @@ public boolean hasDataFromHttp() {
 
             @Override
             public String getHttpHeader(String name) {
-                if (name.equals("Authorization")) {
-                    return "Bearer " + token;
+                if (name.equals(AuthenticationProviderToken.HTTP_HEADER_NAME)) {
+                    return AuthenticationProviderToken.HTTP_HEADER_VALUE_PREFIX + token;
                 } else {
                     throw new IllegalArgumentException("Wrong HTTP header");
                 }
             }
         });
-        assertEquals(subject, "my-test-subject");
+        assertEquals(subject, SUBJECT);
 
         // Expired token. This should be rejected by the authentication provider
-        String expiredToken = AuthTokenUtils.createToken(secretKey, "my-test-subject",
+        String expiredToken = AuthTokenUtils.createToken(secretKey, SUBJECT,
                 Optional.of(new Date(System.currentTimeMillis() - TimeUnit.HOURS.toMillis(1))));
 
         // Pulsar protocol auth
@@ -184,7 +192,7 @@ public String getCommandData() {
     public void testAuthSecretKeyFromFile() throws Exception {
         SecretKey secretKey = AuthTokenUtils.createSecretKey(SignatureAlgorithm.HS256);
 
-        File secretKeyFile = File.createTempFile("pular-test-secret-key-", ".key");
+        File secretKeyFile = File.createTempFile("pulsar-test-secret-key-", ".key");
         secretKeyFile.deleteOnExit();
         Files.write(Paths.get(secretKeyFile.toString()), secretKey.getEncoded());
 
@@ -197,7 +205,7 @@ public void testAuthSecretKeyFromFile() throws Exception {
         conf.setProperties(properties);
         provider.initialize(conf);
 
-        String token = AuthTokenUtils.createToken(secretKey, "my-test-subject", Optional.empty());
+        String token = AuthTokenUtils.createToken(secretKey, SUBJECT, Optional.empty());
 
         // Pulsar protocol auth
         String subject = provider.authenticate(new AuthenticationDataSource() {
@@ -211,7 +219,7 @@ public String getCommandData() {
                 return token;
             }
         });
-        assertEquals(subject, "my-test-subject");
+        assertEquals(subject, SUBJECT);
         provider.close();
     }
 
@@ -229,7 +237,7 @@ public void testAuthSecretKeyFromDataBase64() throws Exception {
         conf.setProperties(properties);
         provider.initialize(conf);
 
-        String token = AuthTokenUtils.createToken(secretKey, "my-test-subject", Optional.empty());
+        String token = AuthTokenUtils.createToken(secretKey, SUBJECT, Optional.empty());
 
         // Pulsar protocol auth
         String subject = provider.authenticate(new AuthenticationDataSource() {
@@ -243,7 +251,7 @@ public String getCommandData() {
                 return token;
             }
         });
-        assertEquals(subject, "my-test-subject");
+        assertEquals(subject, SUBJECT);
         provider.close();
     }
 
@@ -266,7 +274,7 @@ public void testAuthSecretKeyPair() throws Exception {
 
         // Use private key to generate token
         PrivateKey privateKey = AuthTokenUtils.decodePrivateKey(Decoders.BASE64.decode(privateKeyStr));
-        String token = AuthTokenUtils.createToken(privateKey, "my-test-subject", Optional.empty());
+        String token = AuthTokenUtils.createToken(privateKey, SUBJECT, Optional.empty());
 
         // Pulsar protocol auth
         String subject = provider.authenticate(new AuthenticationDataSource() {
@@ -280,8 +288,122 @@ public String getCommandData() {
                 return token;
             }
         });
-        assertEquals(subject, "my-test-subject");
+        assertEquals(subject, SUBJECT);
 
         provider.close();
     }
+
+    @Test(expectedExceptions = AuthenticationException.class)
+    public void testAuthenticateWhenNoJwtPassed() throws AuthenticationException {
+        AuthenticationProviderToken provider = new AuthenticationProviderToken();
+        provider.authenticate(new AuthenticationDataSource() {
+            @Override
+            public boolean hasDataFromCommand() {
+                return false;
+            }
+
+            @Override
+            public boolean hasDataFromHttp() {
+                return false;
+            }
+        });
+    }
+
+    @Test(expectedExceptions = AuthenticationException.class)
+    public void testAuthenticateWhenAuthorizationHeaderNotExist() throws AuthenticationException {
+        AuthenticationProviderToken provider = new AuthenticationProviderToken();
+        provider.authenticate(new AuthenticationDataSource() {
+            @Override
+            public String getHttpHeader(String name) {
+                return null;
+            }
+
+            @Override
+            public boolean hasDataFromHttp() {
+                return true;
+            }
+        });
+    }
+
+    @Test(expectedExceptions = AuthenticationException.class)
+    public void testAuthenticateWhenAuthHeaderValuePrefixIsInvalid() throws AuthenticationException {
+        AuthenticationProviderToken provider = new AuthenticationProviderToken();
+        provider.authenticate(new AuthenticationDataSource() {
+            @Override
+            public String getHttpHeader(String name) {
+                return "MyBearer ";
+            }
+
+            @Override
+            public boolean hasDataFromHttp() {
+                return true;
+            }
+        });
+    }
+
+    @Test(expectedExceptions = AuthenticationException.class)
+    public void testAuthenticateWhenJwtIsBlank() throws AuthenticationException {
+        AuthenticationProviderToken provider = new AuthenticationProviderToken();
+        provider.authenticate(new AuthenticationDataSource() {
+            @Override
+            public String getHttpHeader(String name) {
+                return AuthenticationProviderToken.HTTP_HEADER_VALUE_PREFIX + "      ";
+            }
+
+            @Override
+            public boolean hasDataFromHttp() {
+                return true;
+            }
+        });
+    }
+
+    @Test(expectedExceptions = AuthenticationException.class)
+    public void testAuthenticateWhenInvalidTokenIsPassed() throws AuthenticationException, IOException {
+        SecretKey secretKey = AuthTokenUtils.createSecretKey(SignatureAlgorithm.HS256);
+
+        Properties properties = new Properties();
+        properties.setProperty(AuthenticationProviderToken.CONF_TOKEN_SECRET_KEY,
+                AuthTokenUtils.encodeKeyBase64(secretKey));
+
+        ServiceConfiguration conf = new ServiceConfiguration();
+        conf.setProperties(properties);
+
+        AuthenticationProviderToken provider = new AuthenticationProviderToken();
+        provider.initialize(conf);
+        provider.authenticate(new AuthenticationDataSource() {
+            @Override
+            public String getHttpHeader(String name) {
+                return AuthenticationProviderToken.HTTP_HEADER_VALUE_PREFIX + "invalid_token";
+            }
+
+            @Override
+            public boolean hasDataFromHttp() {
+                return true;
+            }
+        });
+    }
+
+    @Test(expectedExceptions = IOException.class)
+    public void testValidationKeyWhenBlankSecretKeyIsPassed() throws IOException {
+        Properties properties = new Properties();
+        properties.setProperty(AuthenticationProviderToken.CONF_TOKEN_SECRET_KEY, "   ");
+
+        ServiceConfiguration conf = new ServiceConfiguration();
+        conf.setProperties(properties);
+
+        AuthenticationProviderToken provider = new AuthenticationProviderToken();
+        provider.initialize(conf);
+    }
+
+    @Test(expectedExceptions = IOException.class)
+    public void testValidationKeyWhenBlankPublicKeyIsPassed() throws IOException {
+        Properties properties = new Properties();
+        properties.setProperty(AuthenticationProviderToken.CONF_TOKEN_PUBLIC_KEY, "   ");
+
+        ServiceConfiguration conf = new ServiceConfiguration();
+        conf.setProperties(properties);
+
+        AuthenticationProviderToken provider = new AuthenticationProviderToken();
+        provider.initialize(conf);
+    }
 }


 

----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services