+ * Pulsar integrates with this class in the following order: + * 1. Initializing the class by calling {@link AuthenticationProvider#newAuthState(SocketAddress, SSLSession)} + * 2. Calling {@link #authenticate(AuthData)} in a loop until the result is null. + * 3. Calling {@link #getAuthRole()} and {@link #getAuthDataSource()} to use for authentication. + * 4. Calling {@link #refreshAuthentication()} when {@link #isExpired()} returns true. + * 5. GoTo step 3. */ public interface AuthenticationState { /** * After the authentication between client and broker completed, * get authentication role represent for the client. * It should throw exception if auth not complete. + * + *
Users of this interface must call {@link #authenticateAsync(AuthData)} or {@link #authenticate(AuthData)} + * before calling this method.
*/ String getAuthRole() throws AuthenticationException; @@ -71,7 +80,9 @@ default CompletableFutureUsers of this interface must call {@link #authenticateAsync(AuthData)} or {@link #authenticate(AuthData)} + * before calling this method.
+ * @return AuthenticationDataSource. */ AuthenticationDataSource getAuthDataSource(); diff --git a/pulsar-broker-common/src/main/java/org/apache/pulsar/broker/authentication/OneStageAuthenticationState.java b/pulsar-broker-common/src/main/java/org/apache/pulsar/broker/authentication/OneStageAuthenticationState.java index a355ce959562f..f09c3fb0526eb 100644 --- a/pulsar-broker-common/src/main/java/org/apache/pulsar/broker/authentication/OneStageAuthenticationState.java +++ b/pulsar-broker-common/src/main/java/org/apache/pulsar/broker/authentication/OneStageAuthenticationState.java @@ -26,16 +26,40 @@ import org.apache.pulsar.common.api.AuthData; /** - * Interface for authentication state. - * - * It tell broker whether the authentication is completed or not, - * if completed, what is the AuthRole is. + * A class to track single stage authentication. This class assumes that: + * 1. {@link #authenticate(AuthData)} is called once and then authentication is completed + * 2. Authentication does not expire, so {@link #isExpired()} always returns false. + *+ * See {@link AuthenticationState} for Pulsar's contract on how this interface is used by Pulsar. */ public class OneStageAuthenticationState implements AuthenticationState { - private final AuthenticationDataSource authenticationDataSource; - private final String authRole; + private AuthenticationDataSource authenticationDataSource; + private final SocketAddress remoteAddress; + private final SSLSession sslSession; + private final AuthenticationProvider provider; + private String authRole; + + /** + * Constructor for a {@link OneStageAuthenticationState} where there is no authentication performed on + * initialization. + * @param remoteAddress - remoteAddress associated with the {@link AuthenticationState} + * @param sslSession - sslSession associated with the {@link AuthenticationState} + * @param provider - {@link AuthenticationProvider} to use to verify {@link AuthData} + */ + public OneStageAuthenticationState(SocketAddress remoteAddress, + SSLSession sslSession, + AuthenticationProvider provider) { + this.provider = provider; + this.remoteAddress = remoteAddress; + this.sslSession = sslSession; + } + /** + * @deprecated use OneStageAuthenticationState constructor without {@link AuthData}. In order to maintain some + * backwards compatibility, this constructor validates the parameterized {@link AuthData}. + */ + @Deprecated(since = "2.12.0") public OneStageAuthenticationState(AuthData authData, SocketAddress remoteAddress, SSLSession sslSession, @@ -43,16 +67,33 @@ public OneStageAuthenticationState(AuthData authData, this.authenticationDataSource = new AuthenticationDataCommand( new String(authData.getBytes(), UTF_8), remoteAddress, sslSession); this.authRole = provider.authenticate(authenticationDataSource); + this.provider = provider; + this.remoteAddress = remoteAddress; + this.sslSession = sslSession; } + /** + * @deprecated method was only ever used for + * {@link AuthenticationProvider#newHttpAuthState(HttpServletRequest)}. That state object wasn't used other than + * to retrieve the {@link AuthenticationDataSource}, so this constructor is deprecated now. In order to maintain + * some backwards compatibility, this constructor validates the parameterized {@link AuthData}. + */ + @Deprecated(since = "2.12.0") public OneStageAuthenticationState(HttpServletRequest request, AuthenticationProvider provider) throws AuthenticationException { this.authenticationDataSource = new AuthenticationDataHttps(request); this.authRole = provider.authenticate(authenticationDataSource); + // These are not used when this constructor is invoked, set them to null. + this.provider = null; + this.remoteAddress = null; + this.sslSession = null; } @Override - public String getAuthRole() { + public String getAuthRole() throws AuthenticationException { + if (authRole == null) { + throw new AuthenticationException("Must authenticate before calling getAuthRole"); + } return authRole; } @@ -62,12 +103,15 @@ public AuthenticationDataSource getAuthDataSource() { } @Override - public AuthData authenticate(AuthData authData) { + public AuthData authenticate(AuthData authData) throws AuthenticationException { + this.authenticationDataSource = new AuthenticationDataCommand( + new String(authData.getBytes(), UTF_8), remoteAddress, sslSession); + this.authRole = provider.authenticate(authenticationDataSource); return null; } @Override public boolean isComplete() { - return true; + return authRole != null; } } diff --git a/pulsar-broker-common/src/test/java/org/apache/pulsar/broker/authentication/AuthenticationProviderBasicTest.java b/pulsar-broker-common/src/test/java/org/apache/pulsar/broker/authentication/AuthenticationProviderBasicTest.java index 723fde7083d38..e1cfd058ae25c 100644 --- a/pulsar-broker-common/src/test/java/org/apache/pulsar/broker/authentication/AuthenticationProviderBasicTest.java +++ b/pulsar-broker-common/src/test/java/org/apache/pulsar/broker/authentication/AuthenticationProviderBasicTest.java @@ -19,6 +19,7 @@ package org.apache.pulsar.broker.authentication; import static org.testng.Assert.assertEquals; +import static org.testng.Assert.assertNull; import com.google.common.io.Resources; import java.io.IOException; import java.nio.charset.StandardCharsets; @@ -39,9 +40,15 @@ public class AuthenticationProviderBasicTest { public AuthenticationProviderBasicTest() throws IOException { } + @SuppressWarnings("deprecation") private void testAuthenticate(AuthenticationProviderBasic provider) throws AuthenticationException { AuthData authData = AuthData.of("superUser2:superpassword".getBytes(StandardCharsets.UTF_8)); + // Test legacy newAuthState method, which authenticated data provider.newAuthState(authData, null, null); + // Test new newAuthState method, which requires calling authenticate method to authenticate data + AuthenticationState authState = provider.newAuthState(null, null); + AuthData challenge = authState.authenticate(authData); + assertNull(challenge, "Should not produce a challenge result"); } @Test 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 82e8a8f87ee55..14aebce895be1 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 @@ -24,6 +24,8 @@ import static org.testng.Assert.assertEquals; import static org.testng.Assert.assertFalse; import static org.testng.Assert.assertNotNull; +import static org.testng.Assert.assertNull; +import static org.testng.Assert.assertThrows; import static org.testng.Assert.assertTrue; import static org.testng.Assert.fail; import com.google.common.collect.Lists; @@ -691,6 +693,37 @@ public void testExpiringToken() throws Exception { assertEquals(brokerData, AuthData.REFRESH_AUTH_DATA); } + @SuppressWarnings("deprecation") + @Test + public void testExpiredTokenFailsOnAuthenticate() throws Exception { + SecretKey secretKey = AuthTokenUtils.createSecretKey(SignatureAlgorithm.HS256); + + @Cleanup + AuthenticationProviderToken provider = new AuthenticationProviderToken(); + + Properties properties = new Properties(); + properties.setProperty(AuthenticationProviderToken.CONF_TOKEN_SECRET_KEY, + AuthTokenUtils.encodeKeyBase64(secretKey)); + + ServiceConfiguration conf = new ServiceConfiguration(); + conf.setProperties(properties); + provider.initialize(conf); + + // Create a token that is already expired + String expiringToken = AuthTokenUtils.createToken(secretKey, SUBJECT, + Optional.of(new Date(System.currentTimeMillis() - TimeUnit.SECONDS.toMillis(3)))); + + AuthenticationState authState = provider.newAuthState(null, null); + // The call to authenticate the token is the call that fails + assertThrows(AuthenticationException.class, + () -> authState.authenticate(AuthData.of(expiringToken.getBytes()))); + + // Verify that constructing new auth fails when using legacy constructor. + assertThrows(AuthenticationException.class, + () -> provider.newAuthState(AuthData.of(expiringToken.getBytes()), null, null)); + + } + // tests for Token Audience @Test public void testRightTokenAudienceClaim() throws Exception { @@ -889,6 +922,44 @@ public void testTokenFromHttpParams() throws Exception { provider.authenticate(authState.getAuthDataSource()); } + @Test + public void testTokenStateUpdatesAuthenticationDataSource() throws Exception { + SecretKey secretKey = AuthTokenUtils.createSecretKey(SignatureAlgorithm.HS256); + + @Cleanup + AuthenticationProviderToken provider = new AuthenticationProviderToken(); + + Properties properties = new Properties(); + properties.setProperty(AuthenticationProviderToken.CONF_TOKEN_SECRET_KEY, + AuthTokenUtils.encodeKeyBase64(secretKey)); + + ServiceConfiguration conf = new ServiceConfiguration(); + conf.setProperties(properties); + provider.initialize(conf); + + AuthenticationState authState = provider.newAuthState(null, null); + + // Haven't authenticated yet, so cannot get role when using constructor with no auth data + assertThrows(AuthenticationException.class, authState::getAuthRole); + assertNull(authState.getAuthDataSource(), "Haven't created a source yet."); + + String firstToken = AuthTokenUtils.createToken(secretKey, SUBJECT, Optional.empty()); + + AuthData firstChallenge = authState.authenticate(AuthData.of(firstToken.getBytes())); + AuthenticationDataSource firstAuthDataSource = authState.getAuthDataSource(); + + assertNull(firstChallenge, "TokenAuth doesn't respond with challenges"); + assertNotNull(firstAuthDataSource, "Created authDataSource"); + + String secondToken = AuthTokenUtils.createToken(secretKey, SUBJECT, Optional.empty()); + + AuthData secondChallenge = authState.authenticate(AuthData.of(secondToken.getBytes())); + AuthenticationDataSource secondAuthDataSource = authState.getAuthDataSource(); + + assertNull(secondChallenge, "TokenAuth doesn't respond with challenges"); + assertNotNull(secondAuthDataSource, "Created authDataSource"); + } + @Test public void testTokenFromHttpHeaders() throws Exception { SecretKey secretKey = AuthTokenUtils.createSecretKey(SignatureAlgorithm.HS256); diff --git a/pulsar-broker-common/src/test/java/org/apache/pulsar/broker/authentication/OneStageAuthenticationStateTest.java b/pulsar-broker-common/src/test/java/org/apache/pulsar/broker/authentication/OneStageAuthenticationStateTest.java new file mode 100644 index 0000000000000..ba69273eba134 --- /dev/null +++ b/pulsar-broker-common/src/test/java/org/apache/pulsar/broker/authentication/OneStageAuthenticationStateTest.java @@ -0,0 +1,86 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ + +package org.apache.pulsar.broker.authentication; + +import static org.mockito.ArgumentMatchers.any; +import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.times; +import static org.mockito.Mockito.verify; +import static org.mockito.Mockito.when; +import static org.testng.Assert.assertEquals; +import static org.testng.Assert.assertNull; +import static org.testng.Assert.assertThrows; +import static org.testng.Assert.assertTrue; +import org.apache.pulsar.common.api.AuthData; +import org.testng.annotations.Test; +import javax.naming.AuthenticationException; + +// Suppress the warnings, these tests verify the behavior of deprecated methods +@SuppressWarnings("deprecation") +public class OneStageAuthenticationStateTest { + + @Test + public void verifyAuthenticateIsCalledExactlyOnce() throws Exception { + AuthenticationProvider provider = mock(AuthenticationProvider.class); + OneStageAuthenticationState authState = new OneStageAuthenticationState(null, null, provider); + authState.authenticate(AuthData.of("data".getBytes())); + verify(provider).authenticate(any()); + } + + @Test + public void verifyAuthenticateIsCalledExactlyTwice() throws Exception { + AuthenticationProvider provider = mock(AuthenticationProvider.class); + AuthData authData = AuthData.of("data".getBytes()); + OneStageAuthenticationState authState = new OneStageAuthenticationState(authData, null, null, provider); + authState.authenticate(authData); + verify(provider, times(2)).authenticate(any()); + } + + @Test + public void verifyAuthenticateSetsAuthRole() throws Exception { + String role = "my-role"; + AuthenticationProvider provider = mock(AuthenticationProvider.class); + when(provider.authenticate(any())).thenReturn(role); + OneStageAuthenticationState authState = new OneStageAuthenticationState(null, null, provider); + authState.authenticate(AuthData.INIT_AUTH_DATA); + assertEquals(authState.getAuthRole(), role, "Expect roles to match"); + assertTrue(authState.getAuthDataSource() instanceof AuthenticationDataCommand); + } + + @Test + public void verifyClassInitializationSetsAuthRole() throws Exception { + String role = "my-role"; + AuthenticationProvider provider = mock(AuthenticationProvider.class); + AuthData authData = AuthData.of("data".getBytes()); + when(provider.authenticate(any())).thenReturn(role); + OneStageAuthenticationState authState = new OneStageAuthenticationState(authData, null, null, provider); + assertEquals(authState.getAuthRole(), role, "Expect roles to match"); + assertTrue(authState.getAuthDataSource() instanceof AuthenticationDataCommand); + } + + @Test + public void verifyGetAuthRoleFailsIfCalledBeforeAuthenticate() { + AuthenticationProvider provider = mock(AuthenticationProvider.class); + OneStageAuthenticationState authState = new OneStageAuthenticationState(null, null, provider); + assertThrows(AuthenticationException.class, authState::getAuthRole); + assertNull(authState.getAuthDataSource()); + } + +} diff --git a/pulsar-broker/src/main/java/org/apache/pulsar/broker/service/ServerCnx.java b/pulsar-broker/src/main/java/org/apache/pulsar/broker/service/ServerCnx.java index 9e80e98064a15..e4fe13e0d5e19 100644 --- a/pulsar-broker/src/main/java/org/apache/pulsar/broker/service/ServerCnx.java +++ b/pulsar-broker/src/main/java/org/apache/pulsar/broker/service/ServerCnx.java @@ -902,18 +902,7 @@ protected void handleConnect(CommandConnect connect) { sslSession = ((SslHandler) sslHandler).engine().getSession(); } - authState = authenticationProvider.newAuthState(clientData, remoteAddress, sslSession); - - if (log.isDebugEnabled()) { - String role = ""; - if (authState != null && authState.isComplete()) { - role = authState.getAuthRole(); - } else { - role = "authentication incomplete or null"; - } - log.debug("[{}] Authenticate role : {}", remoteAddress, role); - } - + authState = authenticationProvider.newAuthState(remoteAddress, sslSession); state = doAuthentication(clientData, clientProtocolVersion, clientVersion); // This will fail the check if: @@ -939,10 +928,8 @@ protected void handleConnect(CommandConnect connect) { + " using auth method [%s] is not available", originalAuthMethod)); } - originalAuthState = originalAuthenticationProvider.newAuthState( - AuthData.of(connect.getOriginalAuthData().getBytes()), - remoteAddress, - sslSession); + originalAuthState = originalAuthenticationProvider.newAuthState(remoteAddress, sslSession); + originalAuthState.authenticate(AuthData.of(connect.getOriginalAuthData().getBytes())); originalAuthData = originalAuthState.getAuthDataSource(); originalPrincipal = originalAuthState.getAuthRole(); diff --git a/pulsar-broker/src/test/java/org/apache/pulsar/broker/service/ServerCnxTest.java b/pulsar-broker/src/test/java/org/apache/pulsar/broker/service/ServerCnxTest.java index 97b58d99e3d20..5f4b03957d007 100644 --- a/pulsar-broker/src/test/java/org/apache/pulsar/broker/service/ServerCnxTest.java +++ b/pulsar-broker/src/test/java/org/apache/pulsar/broker/service/ServerCnxTest.java @@ -401,7 +401,7 @@ public void testConnectCommandWithAuthenticationPositive() throws Exception { doReturn(authenticationService).when(brokerService).getAuthenticationService(); doReturn(authenticationProvider).when(authenticationService).getAuthenticationProvider(Mockito.anyString()); doReturn(authenticationState).when(authenticationProvider) - .newAuthState(Mockito.any(), Mockito.any(), Mockito.any()); + .newAuthState(Mockito.any(), Mockito.any()); doReturn(authData).when(authenticationState) .authenticate(authData); doReturn(true).when(authenticationState) diff --git a/pulsar-proxy/src/main/java/org/apache/pulsar/proxy/server/ProxyConnection.java b/pulsar-proxy/src/main/java/org/apache/pulsar/proxy/server/ProxyConnection.java index 56fbe94606981..8b1b44316ed2d 100644 --- a/pulsar-proxy/src/main/java/org/apache/pulsar/proxy/server/ProxyConnection.java +++ b/pulsar-proxy/src/main/java/org/apache/pulsar/proxy/server/ProxyConnection.java @@ -519,7 +519,7 @@ remoteAddress, protocolVersionToAdvertise, getRemoteEndpointProtocolVersion(), sslSession = ((SslHandler) sslHandler).engine().getSession(); } - authState = authenticationProvider.newAuthState(clientData, remoteAddress, sslSession); + authState = authenticationProvider.newAuthState(remoteAddress, sslSession); authenticationData = authState.getAuthDataSource(); doAuthentication(clientData); } catch (Exception e) {