This is an automated email from the ASF dual-hosted git repository.
rgao pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/pulsar.git
The following commit(s) were added to refs/heads/master by this push:
new 2b515ffb389 [fix][broker] Fix the reason label of authentication
metrics (#20030)
2b515ffb389 is described below
commit 2b515ffb389c4b4fe3cb5a9c5f3d7eb0a4c9ef99
Author: ran <r...@streamnative.io>
AuthorDate: Fri May 5 08:48:59 2023 +0800
[fix][broker] Fix the reason label of authentication metrics (#20030)
---
.../AuthenticationProviderAthenz.java | 20 ++++++++++++--
.../oidc/AuthenticationProviderOpenID.java | 2 +-
.../authentication/AuthenticationProvider.java | 5 ++++
.../AuthenticationProviderBasic.java | 21 ++++++++++++---
.../authentication/AuthenticationProviderList.java | 20 +++++++++-----
.../authentication/AuthenticationProviderTls.java | 12 +++++++--
.../AuthenticationProviderToken.java | 31 ++++++++++++++--------
.../metrics/AuthenticationMetrics.java | 16 +++++++++++
.../pulsar/broker/stats/PrometheusMetricsTest.java | 6 ++---
9 files changed, 103 insertions(+), 30 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 2e062b87a83..652a922b9a5 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-auth-oidc/src/main/java/org/apache/pulsar/broker/authentication/oidc/AuthenticationProviderOpenID.java
b/pulsar-broker-auth-oidc/src/main/java/org/apache/pulsar/broker/authentication/oidc/AuthenticationProviderOpenID.java
index ae4774b6f6b..00ec09bd181 100644
---
a/pulsar-broker-auth-oidc/src/main/java/org/apache/pulsar/broker/authentication/oidc/AuthenticationProviderOpenID.java
+++
b/pulsar-broker-auth-oidc/src/main/java/org/apache/pulsar/broker/authentication/oidc/AuthenticationProviderOpenID.java
@@ -447,7 +447,7 @@ public class AuthenticationProviderOpenID implements
AuthenticationProvider {
}
static void incrementFailureMetric(AuthenticationExceptionCode code) {
- AuthenticationMetrics.authenticateFailure(SIMPLE_NAME, "token",
code.toString());
+ AuthenticationMetrics.authenticateFailure(SIMPLE_NAME,
AUTH_METHOD_NAME, code);
}
/**
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 109259537a4..7862a35b5e8 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
@@ -30,6 +30,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;
import org.apache.pulsar.common.util.FutureUtil;
@@ -143,6 +144,10 @@ public interface AuthenticationProvider extends Closeable {
}
}
+ default void incrementFailureMetric(Enum<?> errorCode) {
+ AuthenticationMetrics.authenticateFailure(getClass().getSimpleName(),
getAuthMethodName(), errorCode);
+ }
+
/**
* Set response, according to passed in request.
* and return whether we should do following chain.doFilter or not.
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 3c4759fec7d..ca5150c9bdb 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
@@ -46,6 +46,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
@@ -104,9 +112,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);
}
@@ -117,15 +126,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());
@@ -144,24 +154,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 16d6f9859a0..663a6253f44 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
@@ -44,9 +44,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);
@@ -56,19 +62,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;
}
@@ -129,7 +135,7 @@ public class AuthenticationProviderList implements
AuthenticationProvider {
previousException = new
AuthenticationException("Authentication required");
}
AuthenticationMetrics.authenticateFailure(AuthenticationProviderList.class.getSimpleName(),
- "authentication-provider-list", "Authentication
required");
+ "authentication-provider-list",
ErrorCode.AUTH_REQUIRED);
authChallengeFuture.completeExceptionally(previousException);
return;
}
@@ -235,7 +241,7 @@ public class AuthenticationProviderList implements
AuthenticationProvider {
previousException = new
AuthenticationException("Authentication required");
}
AuthenticationMetrics.authenticateFailure(AuthenticationProviderList.class.getSimpleName(),
- "authentication-provider-list", "Authentication required");
+ "authentication-provider-list", ErrorCode.AUTH_REQUIRED);
roleFuture.completeExceptionally(previousException);
return;
}
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 47e5316bce9..a4c44121b4b 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 67e1f39a389..f8992b21ff4 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
@@ -106,6 +106,12 @@ public class AuthenticationProviderToken implements
AuthenticationProvider {
private String confTokenAudienceSettingName;
private String confTokenAllowedClockSkewSecondsSettingName;
+ public enum ErrorCode {
+ INVALID_AUTH_DATA,
+ INVALID_TOKEN,
+ INVALID_AUDIENCES,
+ }
+
@Override
public void close() throws IOException {
// noop
@@ -158,19 +164,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
@@ -241,16 +246,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);
}
}
@@ -264,6 +272,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 0a095235f3c..5faaccbe157 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 bf141e10aa1..7a23e56fe0b 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
@@ -1379,15 +1379,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());
@@ -1424,7 +1421,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;
}
