You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@pulsar.apache.org by pe...@apache.org on 2021/11/05 12:21:35 UTC

[pulsar] 04/04: [Broker] Fix prefix setting in JWT authn and avoid multi calls for the getProperty (#12132)

This is an automated email from the ASF dual-hosted git repository.

penghui pushed a commit to branch branch-2.8
in repository https://gitbox.apache.org/repos/asf/pulsar.git

commit 916cb3b9c1e00b90fd2fab12e6572654edd6e845
Author: Zike Yang <ar...@armail.top>
AuthorDate: Fri Nov 5 14:34:00 2021 +0800

    [Broker] Fix prefix setting in JWT authn and avoid multi calls for the getProperty (#12132)
    
    Motivation
    Currently, the setting prefix for JWT authentication does not work because the code does not specify the property name for the token setting prefix.
    
    Modifications
    Add token setting prefix property name: tokenSettingPrefix.
    Avoid multi calls for the getProperty in JWT auth.
    Verifying this change
    This change is already covered by the existing test, such as testTokenSettingPrefix.
    
    (cherry picked from commit b9a250dd0c80845da703bfbd0286044251187131)
---
 .../AuthenticationProviderToken.java               | 43 ++++++++++------------
 .../AuthenticationProviderTokenTest.java           | 41 +++++++++++++++++++++
 2 files changed, 61 insertions(+), 23 deletions(-)

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 21bda4c..3801aec 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
@@ -54,7 +54,7 @@ public class AuthenticationProviderToken implements AuthenticationProvider {
     static final String HTTP_HEADER_VALUE_PREFIX = "Bearer ";
 
     // When symmetric key is configured
-    static final String CONF_TOKEN_SETTING_PREFIX = "";
+    static final String CONF_TOKEN_SETTING_PREFIX = "tokenSettingPrefix";
 
     // When symmetric key is configured
     static final String CONF_TOKEN_SECRET_KEY = "tokenSecretKey";
@@ -253,15 +253,13 @@ public class AuthenticationProviderToken implements AuthenticationProvider {
      * Try to get the validation key for tokens from several possible config options.
      */
     private Key getValidationKey(ServiceConfiguration conf) throws IOException {
-        if (conf.getProperty(confTokenSecretKeySettingName) != null
-                && StringUtils.isNotBlank((String) conf.getProperty(confTokenSecretKeySettingName))) {
-            final String validationKeyConfig = (String) conf.getProperty(confTokenSecretKeySettingName);
-            final byte[] validationKey = AuthTokenUtils.readKeyFromUrl(validationKeyConfig);
+        String tokenSecretKey = (String) conf.getProperty(confTokenSecretKeySettingName);
+        String tokenPublicKey = (String) conf.getProperty(confTokenPublicKeySettingName);
+        if (StringUtils.isNotBlank(tokenSecretKey)) {
+            final byte[] validationKey = AuthTokenUtils.readKeyFromUrl(tokenSecretKey);
             return AuthTokenUtils.decodeSecretKey(validationKey);
-        } else if (conf.getProperty(confTokenPublicKeySettingName) != null
-                && StringUtils.isNotBlank((String) conf.getProperty(confTokenPublicKeySettingName))) {
-            final String validationKeyConfig = (String) conf.getProperty(confTokenPublicKeySettingName);
-            final byte[] validationKey = AuthTokenUtils.readKeyFromUrl(validationKeyConfig);
+        } else if (StringUtils.isNotBlank(tokenPublicKey)) {
+            final byte[] validationKey = AuthTokenUtils.readKeyFromUrl(tokenPublicKey);
             return AuthTokenUtils.decodePublicKey(validationKey, publicKeyAlg);
         } else {
             throw new IOException("No secret key was provided for token authentication");
@@ -269,22 +267,21 @@ public class AuthenticationProviderToken implements AuthenticationProvider {
     }
 
     private String getTokenRoleClaim(ServiceConfiguration conf) throws IOException {
-        if (conf.getProperty(confTokenAuthClaimSettingName) != null
-                && StringUtils.isNotBlank((String) conf.getProperty(confTokenAuthClaimSettingName))) {
-            return (String) conf.getProperty(confTokenAuthClaimSettingName);
+        String tokenAuthClaim = (String) conf.getProperty(confTokenAuthClaimSettingName);
+        if (StringUtils.isNotBlank(tokenAuthClaim)) {
+            return tokenAuthClaim;
         } else {
             return Claims.SUBJECT;
         }
     }
 
     private SignatureAlgorithm getPublicKeyAlgType(ServiceConfiguration conf) throws IllegalArgumentException {
-        if (conf.getProperty(confTokenPublicAlgSettingName) != null
-                && StringUtils.isNotBlank((String) conf.getProperty(confTokenPublicAlgSettingName))) {
-            String alg = (String) conf.getProperty(confTokenPublicAlgSettingName);
+        String tokenPublicAlg = (String) conf.getProperty(confTokenPublicAlgSettingName);
+        if (StringUtils.isNotBlank(tokenPublicAlg)) {
             try {
-                return SignatureAlgorithm.forName(alg);
+                return SignatureAlgorithm.forName(tokenPublicAlg);
             } catch (SignatureException ex) {
-                throw new IllegalArgumentException("invalid algorithm provided " + alg, ex);
+                throw new IllegalArgumentException("invalid algorithm provided " + tokenPublicAlg, ex);
             }
         } else {
             return SignatureAlgorithm.RS256;
@@ -293,9 +290,9 @@ public class AuthenticationProviderToken implements AuthenticationProvider {
 
     // get Token Audience Claim from configuration, if not configured return null.
     private String getTokenAudienceClaim(ServiceConfiguration conf) throws IllegalArgumentException {
-        if (conf.getProperty(confTokenAudienceClaimSettingName) != null
-            && StringUtils.isNotBlank((String) conf.getProperty(confTokenAudienceClaimSettingName))) {
-            return (String) conf.getProperty(confTokenAudienceClaimSettingName);
+        String tokenAudienceClaim = (String) conf.getProperty(confTokenAudienceClaimSettingName);
+        if (StringUtils.isNotBlank(tokenAudienceClaim)) {
+            return tokenAudienceClaim;
         } else {
             return null;
         }
@@ -303,9 +300,9 @@ public class AuthenticationProviderToken implements AuthenticationProvider {
 
     // get Token Audience that stands for this broker from configuration, if not configured return null.
     private String getTokenAudience(ServiceConfiguration conf) throws IllegalArgumentException {
-        if (conf.getProperty(confTokenAudienceSettingName) != null
-            && StringUtils.isNotBlank((String) conf.getProperty(confTokenAudienceSettingName))) {
-            return (String) conf.getProperty(confTokenAudienceSettingName);
+        String tokenAudience = (String) conf.getProperty(confTokenAudienceSettingName);
+        if (StringUtils.isNotBlank(tokenAudience)) {
+            return tokenAudience;
         } else {
             return null;
         }
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 b05ad4c..3b58cf9 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
@@ -18,6 +18,7 @@
  */
 package org.apache.pulsar.broker.authentication;
 
+import static org.mockito.ArgumentMatchers.anyString;
 import static org.testng.Assert.assertEquals;
 import static org.testng.Assert.assertFalse;
 import static org.testng.Assert.assertNotNull;
@@ -55,6 +56,7 @@ import javax.naming.AuthenticationException;
 import org.apache.pulsar.broker.ServiceConfiguration;
 import org.apache.pulsar.broker.authentication.utils.AuthTokenUtils;
 import org.apache.pulsar.common.api.AuthData;
+import org.mockito.Mockito;
 import org.testng.annotations.Test;
 
 public class AuthenticationProviderTokenTest {
@@ -800,6 +802,45 @@ public class AuthenticationProviderTokenTest {
         provider.close();
     }
 
+    @Test
+    public void testTokenSettingPrefix() throws Exception {
+        AuthenticationProviderToken provider = new AuthenticationProviderToken();
+
+        KeyPair keyPair = Keys.keyPairFor(SignatureAlgorithm.RS256);
+        String publicKeyStr = AuthTokenUtils.encodeKeyBase64(keyPair.getPublic());
+        Properties properties = new Properties();
+        // Use public key for validation
+        properties.setProperty(AuthenticationProviderToken.CONF_TOKEN_PUBLIC_KEY, publicKeyStr);
+        ServiceConfiguration conf = new ServiceConfiguration();
+        conf.setProperties(properties);
+
+        ServiceConfiguration mockConf = Mockito.mock(ServiceConfiguration.class);
+        String prefix = "test";
+
+        Mockito.when(mockConf.getProperty(anyString()))
+                .thenAnswer(invocationOnMock ->
+                        conf.getProperty(((String) invocationOnMock.getArgument(0)).substring(prefix.length()))
+                );
+        Mockito.when(mockConf.getProperty(AuthenticationProviderToken.CONF_TOKEN_SETTING_PREFIX)).thenReturn(prefix);
+
+        provider.initialize(mockConf);
+
+        // Each property is fetched only once. Prevent multiple fetches.
+        Mockito.verify(mockConf, Mockito.times(1)).getProperty(AuthenticationProviderToken.CONF_TOKEN_SETTING_PREFIX);
+        Mockito.verify(mockConf, Mockito.times(1))
+                .getProperty(prefix + AuthenticationProviderToken.CONF_TOKEN_SECRET_KEY);
+        Mockito.verify(mockConf, Mockito.times(1))
+                .getProperty(prefix + AuthenticationProviderToken.CONF_TOKEN_PUBLIC_KEY);
+        Mockito.verify(mockConf, Mockito.times(1))
+                .getProperty(prefix + AuthenticationProviderToken.CONF_TOKEN_AUTH_CLAIM);
+        Mockito.verify(mockConf, Mockito.times(1))
+                .getProperty(prefix + AuthenticationProviderToken.CONF_TOKEN_PUBLIC_ALG);
+        Mockito.verify(mockConf, Mockito.times(1))
+                .getProperty(prefix + AuthenticationProviderToken.CONF_TOKEN_AUDIENCE_CLAIM);
+        Mockito.verify(mockConf, Mockito.times(1))
+                .getProperty(prefix + AuthenticationProviderToken.CONF_TOKEN_AUDIENCE);
+    }
+
     private static String createTokenWithAudience(Key signingKey, String audienceClaim, List<String> audience) {
         JwtBuilder builder = Jwts.builder()
                 .setSubject(SUBJECT)