Skip to content

Commit

Permalink
[Broker] Fix prefix setting in JWT authn and avoid multi calls for th…
Browse files Browse the repository at this point in the history
…e getProperty (apache#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 b9a250d)
  • Loading branch information
RobertIndie authored and codelipenghui committed Nov 5, 2021
1 parent ea40033 commit 916cb3b
Show file tree
Hide file tree
Showing 2 changed files with 61 additions and 23 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -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";
Expand Down Expand Up @@ -253,38 +253,35 @@ private String getPrincipal(Jwt<?, Claims> jwt) {
* 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");
}
}

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;
Expand All @@ -293,19 +290,19 @@ private SignatureAlgorithm getPublicKeyAlgType(ServiceConfiguration conf) throws

// 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;
}
}

// 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;
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -55,6 +56,7 @@
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 {
Expand Down Expand Up @@ -800,6 +802,45 @@ public String getCommandData() {
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)
Expand Down

0 comments on commit 916cb3b

Please sign in to comment.