This is an automated email from the ASF dual-hosted git repository.
rgao pushed a commit to branch branch-2.10
in repository https://gitbox.apache.org/repos/asf/pulsar.git
The following commit(s) were added to refs/heads/branch-2.10 by this push:
new 80dcea7c16e [fix][broker] Fix the reason label of authentication
metrics (#20030)
80dcea7c16e is described below
commit 80dcea7c16e7c7d120a2c76b7d1f175b64189adf
Author: ran <[email protected]>
AuthorDate: Fri May 5 08:48:59 2023 +0800
[fix][broker] Fix the reason label of authentication metrics (#20030)
(cherry picked from commit 2b515ffb389c4b4fe3cb5a9c5f3d7eb0a4c9ef99)
---
.../AuthenticationProviderAthenz.java | 20 ++++++++++++--
.../authentication/AuthenticationProvider.java | 6 +++++
.../AuthenticationProviderBasic.java | 21 ++++++++++++---
.../authentication/AuthenticationProviderList.java | 16 +++++++----
.../authentication/AuthenticationProviderTls.java | 12 +++++++--
.../AuthenticationProviderToken.java | 31 ++++++++++++++--------
.../metrics/AuthenticationMetrics.java | 16 +++++++++++
.../pulsar/broker/stats/PrometheusMetricsTest.java | 6 ++---
8 files changed, 101 insertions(+), 27 deletions(-)
diff --git
a/pulsar-broker-auth-athenz/src/main/java/org/apache/pulsar/broker/authentication/AuthenticationProviderAthenz.java
b/pulsar-broker-auth-athenz/src/main/java/org/apache/pulsar/broker/authentication/AuthenticationProviderAthenz.java
index 4a82e549623..5dfeaba9ad7 100644
---
a/pulsar-broker-auth-athenz/src/main/java/org/apache/pulsar/broker/authentication/AuthenticationProviderAthenz.java
+++
b/pulsar-broker-auth-athenz/src/main/java/org/apache/pulsar/broker/authentication/AuthenticationProviderAthenz.java
@@ -43,6 +43,15 @@ public class AuthenticationProviderAthenz implements
AuthenticationProvider {
private List<String> domainNameList = null;
private int allowedOffset = 30;
+ public enum ErrorCode {
+ UNKNOWN,
+ NO_CLIENT,
+ NO_TOKEN,
+ NO_PUBLIC_KEY,
+ DOMAIN_MISMATCH,
+ INVALID_TOKEN,
+ }
+
@Override
public void initialize(ServiceConfiguration config) throws IOException {
String domainNames;
@@ -81,11 +90,13 @@ public class AuthenticationProviderAthenz implements
AuthenticationProvider {
public String authenticate(AuthenticationDataSource authData) throws
AuthenticationException {
SocketAddress clientAddress;
String roleToken;
+ ErrorCode errorCode = ErrorCode.UNKNOWN;
try {
if (authData.hasDataFromPeer()) {
clientAddress = authData.getPeerAddress();
} else {
+ errorCode = ErrorCode.NO_CLIENT;
throw new AuthenticationException("Authentication data source
does not have a client address");
}
@@ -94,13 +105,16 @@ public class AuthenticationProviderAthenz implements
AuthenticationProvider {
} else if (authData.hasDataFromHttp()) {
roleToken =
authData.getHttpHeader(AuthZpeClient.ZPE_TOKEN_HDR);
} else {
+ errorCode = ErrorCode.NO_TOKEN;
throw new AuthenticationException("Authentication data source
does not have a role token");
}
if (roleToken == null) {
+ errorCode = ErrorCode.NO_TOKEN;
throw new AuthenticationException("Athenz token is null, can't
authenticate");
}
if (roleToken.isEmpty()) {
+ errorCode = ErrorCode.NO_TOKEN;
throw new AuthenticationException("Athenz RoleToken is empty,
Server is Using Athenz Authentication");
}
if (log.isDebugEnabled()) {
@@ -110,6 +124,7 @@ public class AuthenticationProviderAthenz implements
AuthenticationProvider {
RoleToken token = new RoleToken(roleToken);
if (!domainNameList.contains(token.getDomain())) {
+ errorCode = ErrorCode.DOMAIN_MISMATCH;
throw new AuthenticationException(
String.format("Athenz RoleToken Domain mismatch,
Expected: %s, Found: %s",
domainNameList.toString(), token.getDomain()));
@@ -120,6 +135,7 @@ public class AuthenticationProviderAthenz implements
AuthenticationProvider {
PublicKey ztsPublicKey =
AuthZpeClient.getZtsPublicKey(token.getKeyId());
if (ztsPublicKey == null) {
+ errorCode = ErrorCode.NO_PUBLIC_KEY;
throw new AuthenticationException("Unable to retrieve ZTS
Public Key");
}
@@ -128,13 +144,13 @@ public class AuthenticationProviderAthenz implements
AuthenticationProvider {
AuthenticationMetrics.authenticateSuccess(getClass().getSimpleName(),
getAuthMethodName());
return token.getPrincipal();
} else {
+ errorCode = ErrorCode.INVALID_TOKEN;
throw new AuthenticationException(
String.format("Athenz Role Token Not Authenticated
from Client: %s", clientAddress));
}
}
} catch (AuthenticationException exception) {
-
AuthenticationMetrics.authenticateFailure(getClass().getSimpleName(),
getAuthMethodName(),
- exception.getMessage());
+ incrementFailureMetric(errorCode);
throw exception;
}
}
diff --git
a/pulsar-broker-common/src/main/java/org/apache/pulsar/broker/authentication/AuthenticationProvider.java
b/pulsar-broker-common/src/main/java/org/apache/pulsar/broker/authentication/AuthenticationProvider.java
index e3c50e7a6c6..817ebb04a4a 100644
---
a/pulsar-broker-common/src/main/java/org/apache/pulsar/broker/authentication/AuthenticationProvider.java
+++
b/pulsar-broker-common/src/main/java/org/apache/pulsar/broker/authentication/AuthenticationProvider.java
@@ -26,6 +26,7 @@ import javax.net.ssl.SSLSession;
import javax.servlet.http.HttpServletRequest;
import javax.servlet.http.HttpServletResponse;
import org.apache.pulsar.broker.ServiceConfiguration;
+import org.apache.pulsar.broker.authentication.metrics.AuthenticationMetrics;
import org.apache.pulsar.common.api.AuthData;
/**
@@ -80,4 +81,9 @@ public interface AuthenticationProvider extends Closeable {
default boolean authenticateHttpRequest(HttpServletRequest request,
HttpServletResponse response) throws Exception {
throw new AuthenticationException("Not supported");
}
+
+ default void incrementFailureMetric(Enum<?> errorCode) {
+ AuthenticationMetrics.authenticateFailure(getClass().getSimpleName(),
getAuthMethodName(), errorCode);
+ }
+
}
diff --git
a/pulsar-broker-common/src/main/java/org/apache/pulsar/broker/authentication/AuthenticationProviderBasic.java
b/pulsar-broker-common/src/main/java/org/apache/pulsar/broker/authentication/AuthenticationProviderBasic.java
index 9f6bacf7298..bc8ed563d73 100644
---
a/pulsar-broker-common/src/main/java/org/apache/pulsar/broker/authentication/AuthenticationProviderBasic.java
+++
b/pulsar-broker-common/src/main/java/org/apache/pulsar/broker/authentication/AuthenticationProviderBasic.java
@@ -47,6 +47,14 @@ public class AuthenticationProviderBasic implements
AuthenticationProvider {
private static final String CONF_PULSAR_PROPERTY_KEY = "basicAuthConf";
private Map<String, String> users;
+ private enum ErrorCode {
+ UNKNOWN,
+ EMPTY_AUTH_DATA,
+ INVALID_HEADER,
+ INVALID_AUTH_DATA,
+ INVALID_TOKEN,
+ }
+
@Override
public void close() throws IOException {
// noop
@@ -105,9 +113,10 @@ public class AuthenticationProviderBasic implements
AuthenticationProvider {
String userId = authParams.getUserId();
String password = authParams.getPassword();
String msg = "Unknown user or invalid password";
-
+ ErrorCode errorCode = ErrorCode.UNKNOWN;
try {
if (users.get(userId) == null) {
+ errorCode = ErrorCode.INVALID_AUTH_DATA;
throw new AuthenticationException(msg);
}
@@ -118,15 +127,16 @@ public class AuthenticationProviderBasic implements
AuthenticationProvider {
List<String> splitEncryptedPassword =
Arrays.asList(encryptedPassword.split("\\$"));
if (splitEncryptedPassword.size() != 4 || !encryptedPassword
.equals(Md5Crypt.apr1Crypt(password.getBytes(),
splitEncryptedPassword.get(2)))) {
+ errorCode = ErrorCode.INVALID_TOKEN;
throw new AuthenticationException(msg);
}
// For crypt algorithm
} else if
(!encryptedPassword.equals(Crypt.crypt(password.getBytes(),
encryptedPassword.substring(0, 2)))) {
+ errorCode = ErrorCode.INVALID_TOKEN;
throw new AuthenticationException(msg);
}
} catch (AuthenticationException exception) {
-
AuthenticationMetrics.authenticateFailure(getClass().getSimpleName(),
getAuthMethodName(),
- exception.getMessage());
+ incrementFailureMetric(errorCode);
throw exception;
}
AuthenticationMetrics.authenticateSuccess(getClass().getSimpleName(),
getAuthMethodName());
@@ -145,24 +155,29 @@ public class AuthenticationProviderBasic implements
AuthenticationProvider {
String rawAuthToken = authData.getHttpHeader(HTTP_HEADER_NAME);
// parsing and validation
if (StringUtils.isBlank(rawAuthToken) ||
!rawAuthToken.toUpperCase().startsWith("BASIC ")) {
+ incrementFailureMetric(ErrorCode.INVALID_HEADER);
throw new AuthenticationException("Authentication token
has to be started with \"Basic \"");
}
String[] splitRawAuthToken = rawAuthToken.split(" ");
if (splitRawAuthToken.length != 2) {
+ incrementFailureMetric(ErrorCode.INVALID_HEADER);
throw new AuthenticationException("Base64 encoded token is
not found");
}
try {
authParams = new
String(Base64.getDecoder().decode(splitRawAuthToken[1]));
} catch (Exception e) {
+ incrementFailureMetric(ErrorCode.INVALID_HEADER);
throw new AuthenticationException("Base64 decoding is
failure: " + e.getMessage());
}
} else {
+ incrementFailureMetric(ErrorCode.EMPTY_AUTH_DATA);
throw new AuthenticationException("Authentication data source
does not have data");
}
String[] parsedAuthParams = authParams.split(":");
if (parsedAuthParams.length != 2) {
+ incrementFailureMetric(ErrorCode.INVALID_AUTH_DATA);
throw new AuthenticationException("Base64 decoded params are
invalid");
}
diff --git
a/pulsar-broker-common/src/main/java/org/apache/pulsar/broker/authentication/AuthenticationProviderList.java
b/pulsar-broker-common/src/main/java/org/apache/pulsar/broker/authentication/AuthenticationProviderList.java
index f455588dfeb..fce69ce35f0 100644
---
a/pulsar-broker-common/src/main/java/org/apache/pulsar/broker/authentication/AuthenticationProviderList.java
+++
b/pulsar-broker-common/src/main/java/org/apache/pulsar/broker/authentication/AuthenticationProviderList.java
@@ -43,9 +43,15 @@ public class AuthenticationProviderList implements
AuthenticationProvider {
}
+ private enum ErrorCode {
+ UNKNOWN,
+ AUTH_REQUIRED,
+ }
+
static <T, W> T applyAuthProcessor(List<W> processors, AuthProcessor<T, W>
authFunc)
throws AuthenticationException {
AuthenticationException authenticationException = null;
+ String errorCode = ErrorCode.UNKNOWN.name();
for (W ap : processors) {
try {
return authFunc.apply(ap);
@@ -55,19 +61,19 @@ public class AuthenticationProviderList implements
AuthenticationProvider {
}
// Store the exception so we can throw it later instead of a
generic one
authenticationException = ae;
+ errorCode = ap.getClass().getSimpleName() + "-INVALID-AUTH";
}
}
if (null == authenticationException) {
AuthenticationMetrics.authenticateFailure(
AuthenticationProviderList.class.getSimpleName(),
- "authentication-provider-list", "Authentication required");
+ "authentication-provider-list", ErrorCode.AUTH_REQUIRED);
throw new AuthenticationException("Authentication required");
} else {
-
AuthenticationMetrics.authenticateFailure(AuthenticationProviderList.class.getSimpleName(),
- "authentication-provider-list",
- authenticationException.getMessage() != null
- ? authenticationException.getMessage() :
"Authentication required");
+ AuthenticationMetrics.authenticateFailure(
+ AuthenticationProviderList.class.getSimpleName(),
+ "authentication-provider-list", errorCode);
throw authenticationException;
}
diff --git
a/pulsar-broker-common/src/main/java/org/apache/pulsar/broker/authentication/AuthenticationProviderTls.java
b/pulsar-broker-common/src/main/java/org/apache/pulsar/broker/authentication/AuthenticationProviderTls.java
index 4cfcad6a686..7fb93b2e623 100644
---
a/pulsar-broker-common/src/main/java/org/apache/pulsar/broker/authentication/AuthenticationProviderTls.java
+++
b/pulsar-broker-common/src/main/java/org/apache/pulsar/broker/authentication/AuthenticationProviderTls.java
@@ -27,6 +27,12 @@ import
org.apache.pulsar.broker.authentication.metrics.AuthenticationMetrics;
public class AuthenticationProviderTls implements AuthenticationProvider {
+ private enum ErrorCode {
+ UNKNOWN,
+ INVALID_CERTS,
+ INVALID_CN, // invalid common name
+ }
+
@Override
public void close() throws IOException {
// noop
@@ -45,6 +51,7 @@ public class AuthenticationProviderTls implements
AuthenticationProvider {
@Override
public String authenticate(AuthenticationDataSource authData) throws
AuthenticationException {
String commonName = null;
+ ErrorCode errorCode = ErrorCode.UNKNOWN;
try {
if (authData.hasDataFromTls()) {
/**
@@ -72,6 +79,7 @@ public class AuthenticationProviderTls implements
AuthenticationProvider {
// CN=Steve Kille,O=Isode Limited,C=GB
Certificate[] certs = authData.getTlsCertificates();
if (null == certs) {
+ errorCode = ErrorCode.INVALID_CERTS;
throw new AuthenticationException("Failed to get TLS
certificates from client");
}
String distinguishedName = ((X509Certificate)
certs[0]).getSubjectX500Principal().getName();
@@ -85,12 +93,12 @@ public class AuthenticationProviderTls implements
AuthenticationProvider {
}
if (commonName == null) {
+ errorCode = ErrorCode.INVALID_CN;
throw new AuthenticationException("Client unable to
authenticate with TLS certificate");
}
AuthenticationMetrics.authenticateSuccess(getClass().getSimpleName(),
getAuthMethodName());
} catch (AuthenticationException exception) {
-
AuthenticationMetrics.authenticateFailure(getClass().getSimpleName(),
getAuthMethodName(),
- exception.getMessage());
+ incrementFailureMetric(errorCode);
throw exception;
}
return commonName;
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 3943764f943..c3225b66a50 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
@@ -99,6 +99,12 @@ public class AuthenticationProviderToken implements
AuthenticationProvider {
private String confTokenAudienceClaimSettingName;
private String confTokenAudienceSettingName;
+ public enum ErrorCode {
+ INVALID_AUTH_DATA,
+ INVALID_TOKEN,
+ INVALID_AUDIENCES,
+ }
+
@Override
public void close() throws IOException {
// noop
@@ -145,19 +151,18 @@ public class AuthenticationProviderToken implements
AuthenticationProvider {
@Override
public String authenticate(AuthenticationDataSource authData) throws
AuthenticationException {
+ String token;
try {
// Get Token
- String token;
token = getToken(authData);
- // Parse Token by validating
- String role = getPrincipal(authenticateToken(token));
-
AuthenticationMetrics.authenticateSuccess(getClass().getSimpleName(),
getAuthMethodName());
- return role;
} catch (AuthenticationException exception) {
-
AuthenticationMetrics.authenticateFailure(getClass().getSimpleName(),
getAuthMethodName(),
- exception.getMessage());
+ incrementFailureMetric(ErrorCode.INVALID_AUTH_DATA);
throw exception;
}
+ // Parse Token by validating
+ String role = getPrincipal(authenticateToken(token));
+ AuthenticationMetrics.authenticateSuccess(getClass().getSimpleName(),
getAuthMethodName());
+ return role;
}
@Override
@@ -209,16 +214,19 @@ public class AuthenticationProviderToken implements
AuthenticationProvider {
List<String> audiences = (List<String>) object;
// audience not contains this broker, throw exception.
if (audiences.stream().noneMatch(audienceInToken ->
audienceInToken.equals(audience))) {
- throw new AuthenticationException("Audiences in token:
[" + String.join(", ", audiences)
- + "] not contains
this broker: " + audience);
+ incrementFailureMetric(ErrorCode.INVALID_AUDIENCES);
+ throw new AuthenticationException("Audiences in token:
["
+ + String.join(", ", audiences) + "] not
contains this broker: " + audience);
}
} else if (object instanceof String) {
if (!object.equals(audience)) {
- throw new AuthenticationException("Audiences in token:
[" + object
- + "] not contains
this broker: " + audience);
+ incrementFailureMetric(ErrorCode.INVALID_AUDIENCES);
+ throw new AuthenticationException(
+ "Audiences in token: [" + object + "] not
contains this broker: " + audience);
}
} else {
// should not reach here.
+ incrementFailureMetric(ErrorCode.INVALID_AUDIENCES);
throw new AuthenticationException("Audiences in token is
not in expected format: " + object);
}
}
@@ -232,6 +240,7 @@ public class AuthenticationProviderToken implements
AuthenticationProvider {
if (e instanceof ExpiredJwtException) {
expiredTokenMetrics.inc();
}
+ incrementFailureMetric(ErrorCode.INVALID_TOKEN);
throw new AuthenticationException("Failed to authentication token:
" + e.getMessage());
}
}
diff --git
a/pulsar-broker-common/src/main/java/org/apache/pulsar/broker/authentication/metrics/AuthenticationMetrics.java
b/pulsar-broker-common/src/main/java/org/apache/pulsar/broker/authentication/metrics/AuthenticationMetrics.java
index 60565db521d..e2029bcfefc 100644
---
a/pulsar-broker-common/src/main/java/org/apache/pulsar/broker/authentication/metrics/AuthenticationMetrics.java
+++
b/pulsar-broker-common/src/main/java/org/apache/pulsar/broker/authentication/metrics/AuthenticationMetrics.java
@@ -43,11 +43,27 @@ public class AuthenticationMetrics {
/**
* Log authenticate failure event to the authentication metrics.
+ *
+ * This method is deprecated due to the label "reason" is a potential
infinite value.
+ * @deprecated See {@link #authenticateFailure(String, String, Enum)} ()}
+ *
* @param providerName The short class name of the provider
* @param authMethod Authentication method name.
* @param reason Failure reason.
*/
+ @Deprecated
public static void authenticateFailure(String providerName, String
authMethod, String reason) {
authFailuresMetrics.labels(providerName, authMethod, reason).inc();
}
+
+ /**
+ * Log authenticate failure event to the authentication metrics.
+ * @param providerName The short class name of the provider
+ * @param authMethod Authentication method name.
+ * @param errorCode Error code.
+ */
+ public static void authenticateFailure(String providerName, String
authMethod, Enum<?> errorCode) {
+ authFailuresMetrics.labels(providerName, authMethod,
errorCode.name()).inc();
+ }
+
}
diff --git
a/pulsar-broker/src/test/java/org/apache/pulsar/broker/stats/PrometheusMetricsTest.java
b/pulsar-broker/src/test/java/org/apache/pulsar/broker/stats/PrometheusMetricsTest.java
index a01f5ee4edb..8aec2b3eed0 100644
---
a/pulsar-broker/src/test/java/org/apache/pulsar/broker/stats/PrometheusMetricsTest.java
+++
b/pulsar-broker/src/test/java/org/apache/pulsar/broker/stats/PrometheusMetricsTest.java
@@ -1141,15 +1141,12 @@ public class PrometheusMetricsTest extends
BrokerTestBase {
conf.setProperties(properties);
provider.initialize(conf);
- String authExceptionMessage = "";
-
try {
provider.authenticate(new AuthenticationDataSource() {
});
fail("Should have failed");
} catch (AuthenticationException e) {
// expected, no credential passed
- authExceptionMessage = e.getMessage();
}
String token = AuthTokenUtils.createToken(secretKey, "subject",
Optional.empty());
@@ -1186,7 +1183,8 @@ public class PrometheusMetricsTest extends BrokerTestBase
{
boolean haveFailed = false;
for (Metric metric : cm) {
if (Objects.equals(metric.tags.get("auth_method"), "token")
- && Objects.equals(metric.tags.get("reason"),
authExceptionMessage)
+ && Objects.equals(metric.tags.get("reason"),
+
AuthenticationProviderToken.ErrorCode.INVALID_AUTH_DATA.name())
&& Objects.equals(metric.tags.get("provider_name"),
provider.getClass().getSimpleName())) {
haveFailed = true;
}
