You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@pulsar.apache.org by GitBox <gi...@apache.org> on 2020/04/22 21:17:41 UTC
[GitHub] [pulsar] KannarFr opened a new pull request #6799: optional auth method name header in http authentication
KannarFr opened a new pull request #6799:
URL: https://github.com/apache/pulsar/pull/6799
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [pulsar] KannarFr commented on a change in pull request #6799: optional auth method name header in http authentication
Posted by GitBox <gi...@apache.org>.
KannarFr commented on a change in pull request #6799:
URL: https://github.com/apache/pulsar/pull/6799#discussion_r626173851
##########
File path: pulsar-broker-common/src/main/java/org/apache/pulsar/broker/authentication/AuthenticationService.java
##########
@@ -99,18 +99,38 @@ public String authenticate(AuthenticationDataSource authData, String authMethodN
}
public String authenticateHttpRequest(HttpServletRequest request) throws AuthenticationException {
- // Try to validate with any configured provider
AuthenticationException authenticationException = null;
AuthenticationDataSource authData = new AuthenticationDataHttps(request);
- for (AuthenticationProvider provider : providers.values()) {
+ String authMethodName = request.getHeader("X-Pulsar-Auth-Method-Name");
+
+ if (authMethodName != null) {
+ AuthenticationProvider providerToUse = providers
+ .values()
+ .parallelStream()
+ .filter(provider -> provider.getAuthMethodName().equals(authMethodName))
+ .findAny()
+ .orElseThrow(() -> new AuthenticationException(String.format("Unsupported header X-Pulsar-Auth-Method-Name [%s].", authMethodName)));
Review comment:
applied in 4265e40231558ce839bc429f811a4e59f2a6e8d1.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [pulsar] eolivelli merged pull request #6799: optional auth method name header in http authentication
Posted by GitBox <gi...@apache.org>.
eolivelli merged pull request #6799:
URL: https://github.com/apache/pulsar/pull/6799
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [pulsar] merlimat commented on a change in pull request #6799: optional auth method name header in http authentication
Posted by GitBox <gi...@apache.org>.
merlimat commented on a change in pull request #6799:
URL: https://github.com/apache/pulsar/pull/6799#discussion_r626097618
##########
File path: pulsar-broker-common/src/main/java/org/apache/pulsar/broker/authentication/AuthenticationService.java
##########
@@ -99,18 +99,38 @@ public String authenticate(AuthenticationDataSource authData, String authMethodN
}
public String authenticateHttpRequest(HttpServletRequest request) throws AuthenticationException {
- // Try to validate with any configured provider
AuthenticationException authenticationException = null;
AuthenticationDataSource authData = new AuthenticationDataHttps(request);
- for (AuthenticationProvider provider : providers.values()) {
+ String authMethodName = request.getHeader("Auth-Method-Name");
+
+ if (authMethodName != null) {
+ AuthenticationProvider providerToUse = providers
+ .values()
+ .parallelStream()
+ .filter(provider -> provider.getAuthMethodName().equals(authMethodName))
+ .findAny()
Review comment:
`AuthenticationService` should build a map in the constructor with all the available provider indexed by name
##########
File path: pulsar-client/src/test/java/org/apache/pulsar/client/impl/schema/AvroSchemaTest.java
##########
@@ -20,39 +20,36 @@
import static java.nio.charset.StandardCharsets.UTF_8;
import static org.apache.pulsar.client.impl.schema.SchemaTestUtils.FOO_FIELDS;
-import static org.apache.pulsar.client.impl.schema.SchemaTestUtils.SCHEMA_AVRO_NOT_ALLOW_NULL;
import static org.apache.pulsar.client.impl.schema.SchemaTestUtils.SCHEMA_AVRO_ALLOW_NULL;
+import static org.apache.pulsar.client.impl.schema.SchemaTestUtils.SCHEMA_AVRO_NOT_ALLOW_NULL;
Review comment:
These test changes look unrelated to the auth change.
##########
File path: pulsar-broker-common/src/main/java/org/apache/pulsar/broker/authentication/AuthenticationService.java
##########
@@ -99,18 +99,38 @@ public String authenticate(AuthenticationDataSource authData, String authMethodN
}
public String authenticateHttpRequest(HttpServletRequest request) throws AuthenticationException {
- // Try to validate with any configured provider
AuthenticationException authenticationException = null;
AuthenticationDataSource authData = new AuthenticationDataHttps(request);
- for (AuthenticationProvider provider : providers.values()) {
+ String authMethodName = request.getHeader("Auth-Method-Name");
Review comment:
Since this is a non-standard header, we should use a prefixed name, like `X-Pulsar-Auth-Method-Name` or `X-Pulsar-Auth-Method`
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [pulsar] merlimat commented on a change in pull request #6799: optional auth method name header in http authentication
Posted by GitBox <gi...@apache.org>.
merlimat commented on a change in pull request #6799:
URL: https://github.com/apache/pulsar/pull/6799#discussion_r626169169
##########
File path: pulsar-broker-common/src/main/java/org/apache/pulsar/broker/authentication/AuthenticationService.java
##########
@@ -99,18 +99,38 @@ public String authenticate(AuthenticationDataSource authData, String authMethodN
}
public String authenticateHttpRequest(HttpServletRequest request) throws AuthenticationException {
- // Try to validate with any configured provider
AuthenticationException authenticationException = null;
AuthenticationDataSource authData = new AuthenticationDataHttps(request);
- for (AuthenticationProvider provider : providers.values()) {
+ String authMethodName = request.getHeader("X-Pulsar-Auth-Method-Name");
+
+ if (authMethodName != null) {
+ AuthenticationProvider providerToUse = providers
+ .values()
+ .parallelStream()
+ .filter(provider -> provider.getAuthMethodName().equals(authMethodName))
+ .findAny()
+ .orElseThrow(() -> new AuthenticationException(String.format("Unsupported header X-Pulsar-Auth-Method-Name [%s].", authMethodName)));
Review comment:
```suggestion
AuthenticationProvider providerToUse = providers.get(authMethodName);
if (providerToUse == null) {
throw new AuthenticationException(
String.format("Unsupported authentication method: [%s].", authMethodName);
}
```
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [pulsar] KannarFr commented on pull request #6799: optional auth method name header in http authentication
Posted by GitBox <gi...@apache.org>.
KannarFr commented on pull request #6799:
URL: https://github.com/apache/pulsar/pull/6799#issuecomment-833455775
flaky tests?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [pulsar] codelipenghui commented on pull request #6799: optional auth method name header in http authentication
Posted by GitBox <gi...@apache.org>.
codelipenghui commented on pull request #6799:
URL: https://github.com/apache/pulsar/pull/6799#issuecomment-639948216
@KannarFr Could you please rebase to the master branch?
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [pulsar] codelipenghui commented on pull request #6799: optional auth method name header in http authentication
Posted by GitBox <gi...@apache.org>.
codelipenghui commented on pull request #6799:
URL: https://github.com/apache/pulsar/pull/6799#issuecomment-640405760
move to 2.7.0 first.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [pulsar] KannarFr commented on a change in pull request #6799: optional auth method name header in http authentication
Posted by GitBox <gi...@apache.org>.
KannarFr commented on a change in pull request #6799:
URL: https://github.com/apache/pulsar/pull/6799#discussion_r626154888
##########
File path: pulsar-broker-common/src/main/java/org/apache/pulsar/broker/authentication/AuthenticationService.java
##########
@@ -99,18 +99,38 @@ public String authenticate(AuthenticationDataSource authData, String authMethodN
}
public String authenticateHttpRequest(HttpServletRequest request) throws AuthenticationException {
- // Try to validate with any configured provider
AuthenticationException authenticationException = null;
AuthenticationDataSource authData = new AuthenticationDataHttps(request);
- for (AuthenticationProvider provider : providers.values()) {
+ String authMethodName = request.getHeader("Auth-Method-Name");
Review comment:
Done in e0c4876550e68905879e8440fa13a3e5226cb782.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [pulsar] sijie commented on pull request #6799: optional auth method name header in http authentication
Posted by GitBox <gi...@apache.org>.
sijie commented on pull request #6799:
URL: https://github.com/apache/pulsar/pull/6799#issuecomment-625081756
/pulsarbot run-failure-checks
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [pulsar] KannarFr commented on pull request #6799: optional auth method name header in http authentication
Posted by GitBox <gi...@apache.org>.
KannarFr commented on pull request #6799:
URL: https://github.com/apache/pulsar/pull/6799#issuecomment-832320323
@merlimat I added test 8694c4b4da510c9f8a094d37e0f2459a365d2356.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [pulsar] merlimat commented on a change in pull request #6799: optional auth method name header in http authentication
Posted by GitBox <gi...@apache.org>.
merlimat commented on a change in pull request #6799:
URL: https://github.com/apache/pulsar/pull/6799#discussion_r626156224
##########
File path: pulsar-broker-common/src/main/java/org/apache/pulsar/broker/authentication/AuthenticationService.java
##########
@@ -99,18 +99,38 @@ public String authenticate(AuthenticationDataSource authData, String authMethodN
}
public String authenticateHttpRequest(HttpServletRequest request) throws AuthenticationException {
- // Try to validate with any configured provider
AuthenticationException authenticationException = null;
AuthenticationDataSource authData = new AuthenticationDataHttps(request);
- for (AuthenticationProvider provider : providers.values()) {
+ String authMethodName = request.getHeader("Auth-Method-Name");
+
+ if (authMethodName != null) {
+ AuthenticationProvider providerToUse = providers
+ .values()
+ .parallelStream()
+ .filter(provider -> provider.getAuthMethodName().equals(authMethodName))
+ .findAny()
Review comment:
Ok, so we should be able to just do `providers.get(authMethodName)` instead of iterating over the items of the map.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [pulsar] jiazhai commented on pull request #6799: optional auth method name header in http authentication
Posted by GitBox <gi...@apache.org>.
jiazhai commented on pull request #6799:
URL: https://github.com/apache/pulsar/pull/6799#issuecomment-626622225
/pulsarbot run-failure-checks
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [pulsar] KannarFr commented on a change in pull request #6799: optional auth method name header in http authentication
Posted by GitBox <gi...@apache.org>.
KannarFr commented on a change in pull request #6799:
URL: https://github.com/apache/pulsar/pull/6799#discussion_r626154752
##########
File path: pulsar-broker-common/src/main/java/org/apache/pulsar/broker/authentication/AuthenticationService.java
##########
@@ -99,18 +99,38 @@ public String authenticate(AuthenticationDataSource authData, String authMethodN
}
public String authenticateHttpRequest(HttpServletRequest request) throws AuthenticationException {
- // Try to validate with any configured provider
AuthenticationException authenticationException = null;
AuthenticationDataSource authData = new AuthenticationDataHttps(request);
- for (AuthenticationProvider provider : providers.values()) {
+ String authMethodName = request.getHeader("Auth-Method-Name");
+
+ if (authMethodName != null) {
+ AuthenticationProvider providerToUse = providers
+ .values()
+ .parallelStream()
+ .filter(provider -> provider.getAuthMethodName().equals(authMethodName))
+ .findAny()
Review comment:
`providers` is already a Map built in the constructor. Not sure to understand your statement?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [pulsar] KannarFr commented on a change in pull request #6799: optional auth method name header in http authentication
Posted by GitBox <gi...@apache.org>.
KannarFr commented on a change in pull request #6799:
URL: https://github.com/apache/pulsar/pull/6799#discussion_r626154888
##########
File path: pulsar-broker-common/src/main/java/org/apache/pulsar/broker/authentication/AuthenticationService.java
##########
@@ -99,18 +99,38 @@ public String authenticate(AuthenticationDataSource authData, String authMethodN
}
public String authenticateHttpRequest(HttpServletRequest request) throws AuthenticationException {
- // Try to validate with any configured provider
AuthenticationException authenticationException = null;
AuthenticationDataSource authData = new AuthenticationDataHttps(request);
- for (AuthenticationProvider provider : providers.values()) {
+ String authMethodName = request.getHeader("Auth-Method-Name");
Review comment:
Done in f97ccc37bbe02814434a3a1fd54e9f964fadd13c.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [pulsar] sijie commented on a change in pull request #6799: optional auth method name header in http authentication
Posted by GitBox <gi...@apache.org>.
sijie commented on a change in pull request #6799:
URL: https://github.com/apache/pulsar/pull/6799#discussion_r413589283
##########
File path: pulsar-broker-common/src/main/java/org/apache/pulsar/broker/authentication/AuthenticationService.java
##########
@@ -83,13 +83,29 @@ public String authenticate(AuthenticationDataSource authData, String authMethodN
}
public String authenticateHttpRequest(HttpServletRequest request) throws AuthenticationException {
- // Try to validate with any configured provider
AuthenticationDataSource authData = new AuthenticationDataHttps(request);
- for (AuthenticationProvider provider : providers.values()) {
+ String authMethodName = request.getHeader("Auth-Method-Name");
+
+ if (authMethodName != null) {
+ AuthenticationProvider providerToUse = providers
+ .values()
+ .parallelStream()
+ .filter(provider -> provider.getAuthMethodName().equals(authMethodName))
+ .findAny()
+ .orElseThrow(() -> new AuthenticationException(String.format("Unsupported header Auth-Method-Name [%s].", authMethodName)));
+
try {
- return provider.authenticate(authData);
+ return providerToUse.authenticate(authData);
} catch (AuthenticationException e) {
- // Ignore the exception because we don't know which authentication method is expected here.
+ throw e;
+ }
+ } else {
+ for (AuthenticationProvider provider : providers.values()) {
+ try {
+ return provider.authenticate(authData);
+ } catch (AuthenticationException e) {
+ // Ignore the exception because we don't know which authentication method is expected here.
+ }
}
Review comment:
If none of the providers is able to authenticate the provided `authData`, shouldn't we throw any exception here?
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [pulsar] KannarFr commented on pull request #6799: optional auth method name header in http authentication
Posted by GitBox <gi...@apache.org>.
KannarFr commented on pull request #6799:
URL: https://github.com/apache/pulsar/pull/6799#issuecomment-619683210
rebased
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [pulsar] KannarFr commented on pull request #6799: optional auth method name header in http authentication
Posted by GitBox <gi...@apache.org>.
KannarFr commented on pull request #6799:
URL: https://github.com/apache/pulsar/pull/6799#issuecomment-640057445
@codelipenghui done.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [pulsar] KannarFr commented on a change in pull request #6799: optional auth method name header in http authentication
Posted by GitBox <gi...@apache.org>.
KannarFr commented on a change in pull request #6799:
URL: https://github.com/apache/pulsar/pull/6799#discussion_r626156609
##########
File path: pulsar-broker-common/src/main/java/org/apache/pulsar/broker/authentication/AuthenticationService.java
##########
@@ -99,18 +99,38 @@ public String authenticate(AuthenticationDataSource authData, String authMethodN
}
public String authenticateHttpRequest(HttpServletRequest request) throws AuthenticationException {
- // Try to validate with any configured provider
AuthenticationException authenticationException = null;
AuthenticationDataSource authData = new AuthenticationDataHttps(request);
- for (AuthenticationProvider provider : providers.values()) {
+ String authMethodName = request.getHeader("Auth-Method-Name");
+
+ if (authMethodName != null) {
+ AuthenticationProvider providerToUse = providers
+ .values()
+ .parallelStream()
+ .filter(provider -> provider.getAuthMethodName().equals(authMethodName))
+ .findAny()
Review comment:
Oh ok, so try catch.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [pulsar] sijie commented on pull request #6799: optional auth method name header in http authentication
Posted by GitBox <gi...@apache.org>.
sijie commented on pull request #6799:
URL: https://github.com/apache/pulsar/pull/6799#issuecomment-619803681
/pulsarbot run-failure-checks
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [pulsar] sijie commented on pull request #6799: optional auth method name header in http authentication
Posted by GitBox <gi...@apache.org>.
sijie commented on pull request #6799:
URL: https://github.com/apache/pulsar/pull/6799#issuecomment-626077893
/pulsarbot run-failure-checks
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [pulsar] codelipenghui commented on pull request #6799: optional auth method name header in http authentication
Posted by GitBox <gi...@apache.org>.
codelipenghui commented on pull request #6799:
URL: https://github.com/apache/pulsar/pull/6799#issuecomment-640131490
/pulsarbot run-failure-checks
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [pulsar] sijie commented on pull request #6799: optional auth method name header in http authentication
Posted by GitBox <gi...@apache.org>.
sijie commented on pull request #6799:
URL: https://github.com/apache/pulsar/pull/6799#issuecomment-633845920
/pulsarbot run-failure-checks
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [pulsar] codelipenghui commented on pull request #6799: optional auth method name header in http authentication
Posted by GitBox <gi...@apache.org>.
codelipenghui commented on pull request #6799:
URL: https://github.com/apache/pulsar/pull/6799#issuecomment-638614662
@KannarFr Could you please rebase to the master branch?
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [pulsar] jiazhai commented on pull request #6799: optional auth method name header in http authentication
Posted by GitBox <gi...@apache.org>.
jiazhai commented on pull request #6799:
URL: https://github.com/apache/pulsar/pull/6799#issuecomment-626522337
/pulsarbot run-failure-checks
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [pulsar] KannarFr commented on a change in pull request #6799: optional auth method name header in http authentication
Posted by GitBox <gi...@apache.org>.
KannarFr commented on a change in pull request #6799:
URL: https://github.com/apache/pulsar/pull/6799#discussion_r626174138
##########
File path: pulsar-broker-common/src/main/java/org/apache/pulsar/broker/authentication/AuthenticationService.java
##########
@@ -99,18 +99,38 @@ public String authenticate(AuthenticationDataSource authData, String authMethodN
}
public String authenticateHttpRequest(HttpServletRequest request) throws AuthenticationException {
- // Try to validate with any configured provider
AuthenticationException authenticationException = null;
AuthenticationDataSource authData = new AuthenticationDataHttps(request);
- for (AuthenticationProvider provider : providers.values()) {
+ String authMethodName = request.getHeader("Auth-Method-Name");
+
+ if (authMethodName != null) {
+ AuthenticationProvider providerToUse = providers
+ .values()
+ .parallelStream()
+ .filter(provider -> provider.getAuthMethodName().equals(authMethodName))
+ .findAny()
Review comment:
Thanks for the suggestion I thought get from Map could throws Exception is not find :D.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [pulsar] KannarFr commented on a change in pull request #6799: optional auth method name header in http authentication
Posted by GitBox <gi...@apache.org>.
KannarFr commented on a change in pull request #6799:
URL: https://github.com/apache/pulsar/pull/6799#discussion_r626154232
##########
File path: pulsar-client/src/test/java/org/apache/pulsar/client/impl/schema/AvroSchemaTest.java
##########
@@ -20,39 +20,36 @@
import static java.nio.charset.StandardCharsets.UTF_8;
import static org.apache.pulsar.client.impl.schema.SchemaTestUtils.FOO_FIELDS;
-import static org.apache.pulsar.client.impl.schema.SchemaTestUtils.SCHEMA_AVRO_NOT_ALLOW_NULL;
import static org.apache.pulsar.client.impl.schema.SchemaTestUtils.SCHEMA_AVRO_ALLOW_NULL;
+import static org.apache.pulsar.client.impl.schema.SchemaTestUtils.SCHEMA_AVRO_NOT_ALLOW_NULL;
Review comment:
fixed
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [pulsar] KannarFr commented on pull request #6799: optional auth method name header in http authentication
Posted by GitBox <gi...@apache.org>.
KannarFr commented on pull request #6799:
URL: https://github.com/apache/pulsar/pull/6799#issuecomment-838265406
Can we merge it?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [pulsar] codelipenghui commented on pull request #6799: optional auth method name header in http authentication
Posted by GitBox <gi...@apache.org>.
codelipenghui commented on pull request #6799:
URL: https://github.com/apache/pulsar/pull/6799#issuecomment-640139520
/pulsarbot run-failure-checks
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [pulsar] codelipenghui commented on pull request #6799: optional auth method name header in http authentication
Posted by GitBox <gi...@apache.org>.
codelipenghui commented on pull request #6799:
URL: https://github.com/apache/pulsar/pull/6799#issuecomment-640143842
@KannarFr The failed CI tests are related to this change, could you please take a look? Or we can move it to 2.6.1/2.7.0
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [pulsar] KannarFr commented on a change in pull request #6799: optional auth method name header in http authentication
Posted by GitBox <gi...@apache.org>.
KannarFr commented on a change in pull request #6799:
URL: https://github.com/apache/pulsar/pull/6799#discussion_r414239613
##########
File path: pulsar-broker-common/src/main/java/org/apache/pulsar/broker/authentication/AuthenticationService.java
##########
@@ -83,13 +83,29 @@ public String authenticate(AuthenticationDataSource authData, String authMethodN
}
public String authenticateHttpRequest(HttpServletRequest request) throws AuthenticationException {
- // Try to validate with any configured provider
AuthenticationDataSource authData = new AuthenticationDataHttps(request);
- for (AuthenticationProvider provider : providers.values()) {
+ String authMethodName = request.getHeader("Auth-Method-Name");
+
+ if (authMethodName != null) {
+ AuthenticationProvider providerToUse = providers
+ .values()
+ .parallelStream()
+ .filter(provider -> provider.getAuthMethodName().equals(authMethodName))
+ .findAny()
+ .orElseThrow(() -> new AuthenticationException(String.format("Unsupported header Auth-Method-Name [%s].", authMethodName)));
+
try {
- return provider.authenticate(authData);
+ return providerToUse.authenticate(authData);
} catch (AuthenticationException e) {
- // Ignore the exception because we don't know which authentication method is expected here.
+ throw e;
+ }
+ } else {
+ for (AuthenticationProvider provider : providers.values()) {
+ try {
+ return provider.authenticate(authData);
+ } catch (AuthenticationException e) {
+ // Ignore the exception because we don't know which authentication method is expected here.
+ }
}
Review comment:
Already done here: https://github.com/apache/pulsar/blob/562231c7faf88fd1fed1ce4ac2dc7b5084a93896/pulsar-broker-common/src/main/java/org/apache/pulsar/broker/authentication/AuthenticationService.java#L118, right?
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org