[GitHub] jai1 commented on issue #1002: Making Pulsar Proxy more secure
jai1 commented on issue #1002: Making Pulsar Proxy more secure URL: https://github.com/apache/incubator-pulsar/pull/1002#issuecomment-357870142 retest this please This is an automated message from the Apache Git Service. To respond to the message, please log on GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
[GitHub] massakam closed issue #1061: Consumers in failover subscription stops consuming after restart
massakam closed issue #1061: Consumers in failover subscription stops consuming after restart URL: https://github.com/apache/incubator-pulsar/issues/1061 This is an automated message from the Apache Git Service. To respond to the message, please log on GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
[GitHub] massakam closed pull request #1062: Issue 1061: Consumers in failover subscription stops consuming after restart
massakam closed pull request #1062: Issue 1061: Consumers in failover
subscription stops consuming after restart
URL: https://github.com/apache/incubator-pulsar/pull/1062
This is a PR merged from a forked repository.
As GitHub hides the original diff on merge, it is displayed below for
the sake of provenance:
As this is a foreign pull request (from a fork), the diff is supplied
below (as it won't show otherwise due to GitHub magic):
diff --git
a/pulsar-broker/src/main/java/org/apache/pulsar/broker/service/persistent/PersistentDispatcherSingleActiveConsumer.java
b/pulsar-broker/src/main/java/org/apache/pulsar/broker/service/persistent/PersistentDispatcherSingleActiveConsumer.java
index 68addac27..4e3fd00b5 100644
---
a/pulsar-broker/src/main/java/org/apache/pulsar/broker/service/persistent/PersistentDispatcherSingleActiveConsumer.java
+++
b/pulsar-broker/src/main/java/org/apache/pulsar/broker/service/persistent/PersistentDispatcherSingleActiveConsumer.java
@@ -238,6 +238,12 @@ public void redeliverUnacknowledgedMessages(Consumer
consumer, List 0) {
This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
With regards,
Apache Git Services
[GitHub] jai1 commented on a change in pull request #1002: Making Pulsar Proxy more secure
jai1 commented on a change in pull request #1002: Making Pulsar Proxy more
secure
URL: https://github.com/apache/incubator-pulsar/pull/1002#discussion_r161654082
##
File path:
pulsar-broker-common/src/main/java/org/apache/pulsar/broker/authorization/AuthorizationManager.java
##
@@ -70,6 +70,19 @@ public boolean canProduce(DestinationName destination,
String role) throws Excep
}
}
+/**
+ * Check if the specified role has permission to access the destination
via a proxy
+ *
+ * @param destination
+ *the fully qualified destination name associated with the
destination.
+ * @param role
+ *the app id used to receive messages from the destination.
+ */
+public CompletableFuture canProxyAsync(DestinationName
destination, String role) {
+return checkAuthorization(destination, role, AuthAction.proxy);
Review comment:
I don't think passing proxy as a flag will work here
What we want to achieve is restrict the proxy machines (authRole) access to
certain topics so that if the proxy is compromised - not all topics can be
accessed via the proxy.
Besides creating a separate AuthAction the only other way I see that this
will be possible is that per topic along with client role we also add proxy
role with AuthAction produce/consume and change the ServerCnx logic to
authenticate authRole and originalClientRole both for AuthAction
produce/consume each time a producer or consumer is created.
Advt:- finer granularity in produce and consume
DisAdvt:- More messy code and a bit of an overkill
Anyways - let me know what you think about this,
This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
With regards,
Apache Git Services
[GitHub] hrsakai closed pull request #820: Added setting for anonymous user role
hrsakai closed pull request #820: Added setting for anonymous user role
URL: https://github.com/apache/incubator-pulsar/pull/820
This is a PR merged from a forked repository.
As GitHub hides the original diff on merge, it is displayed below for
the sake of provenance:
As this is a foreign pull request (from a fork), the diff is supplied
below (as it won't show otherwise due to GitHub magic):
diff --git a/conf/broker.conf b/conf/broker.conf
index af05b567c..5b8e90380 100644
--- a/conf/broker.conf
+++ b/conf/broker.conf
@@ -210,6 +210,9 @@ brokerClientAuthenticationParameters=
# Supported Athenz provider domain names(comma separated) for authentication
athenzDomainNames=
+# When this parameter is not empty, unauthenticated users perform as
anonymousUserRole
+anonymousUserRole=
+
### --- BookKeeper Client --- ###
# Authentication plugin to use when connecting to bookies
diff --git a/conf/standalone.conf b/conf/standalone.conf
index 797b07db4..47a3b60e4 100644
--- a/conf/standalone.conf
+++ b/conf/standalone.conf
@@ -176,6 +176,9 @@ brokerClientAuthenticationParameters=
# Supported Athenz provider domain names(comma separated) for authentication
athenzDomainNames=
+# When this parameter is not empty, unauthenticated users perform as
anonymousUserRole
+anonymousUserRole=
+
### --- BookKeeper Client --- ###
# Authentication plugin to use when connecting to bookies
diff --git a/conf/websocket.conf b/conf/websocket.conf
index de043d5c7..cf5135d9e 100644
--- a/conf/websocket.conf
+++ b/conf/websocket.conf
@@ -72,6 +72,9 @@ superUserRoles=
brokerClientAuthenticationPlugin=
brokerClientAuthenticationParameters=
+# When this parameter is not empty, unauthenticated users perform as
anonymousUserRole
+anonymousUserRole=
+
### --- TLS --- ###
# Enable TLS
diff --git
a/pulsar-broker-common/src/main/java/org/apache/pulsar/broker/ServiceConfiguration.java
b/pulsar-broker-common/src/main/java/org/apache/pulsar/broker/ServiceConfiguration.java
index a638fd4fb..2bf177396 100644
---
a/pulsar-broker-common/src/main/java/org/apache/pulsar/broker/ServiceConfiguration.java
+++
b/pulsar-broker-common/src/main/java/org/apache/pulsar/broker/ServiceConfiguration.java
@@ -201,6 +201,9 @@
private String brokerClientAuthenticationPlugin =
"org.apache.pulsar.client.impl.auth.AuthenticationDisabled";
private String brokerClientAuthenticationParameters = "";
+// When this parameter is not empty, unauthenticated users perform as
anonymousUserRole
+private String anonymousUserRole = null;
+
/ --- BookKeeper Client --- /
// Authentication plugin to use when connecting to bookies
private String bookkeeperClientAuthenticationPlugin;
@@ -796,6 +799,14 @@ public void setBrokerClientAuthenticationParameters(String
brokerClientAuthentic
this.brokerClientAuthenticationParameters =
brokerClientAuthenticationParameters;
}
+public String getAnonymousUserRole() {
+return anonymousUserRole;
+}
+
+public void setAnonymousUserRole(String anonymousUserRole) {
+this.anonymousUserRole = anonymousUserRole;
+}
+
public String getBookkeeperClientAuthenticationPlugin() {
return bookkeeperClientAuthenticationPlugin;
}
diff --git
a/pulsar-broker-common/src/main/java/org/apache/pulsar/broker/authentication/AuthenticationService.java
b/pulsar-broker-common/src/main/java/org/apache/pulsar/broker/authentication/AuthenticationService.java
index d94e97c59..930f3d2e5 100644
---
a/pulsar-broker-common/src/main/java/org/apache/pulsar/broker/authentication/AuthenticationService.java
+++
b/pulsar-broker-common/src/main/java/org/apache/pulsar/broker/authentication/AuthenticationService.java
@@ -25,6 +25,7 @@
import javax.naming.AuthenticationException;
import javax.servlet.http.HttpServletRequest;
+import org.apache.commons.lang3.StringUtils;
import org.apache.pulsar.broker.PulsarServerException;
import org.apache.pulsar.broker.ServiceConfiguration;
import org.slf4j.Logger;
@@ -38,10 +39,12 @@
*/
public class AuthenticationService implements Closeable {
private static final Logger LOG =
LoggerFactory.getLogger(AuthenticationService.class);
+private final String anonymousUserRole;
private final Map providers =
Maps.newHashMap();
public AuthenticationService(ServiceConfiguration conf) throws
PulsarServerException {
+anonymousUserRole = conf.getAnonymousUserRole();
if (conf.isAuthenticationEnabled()) {
try {
AuthenticationProvider provider;
@@ -71,6 +74,9 @@ public String authenticate(AuthenticationDataSource authData,
String authMethodN
if (provider != null) {
return provider.authenticate(authData);
} else {
+if (StringUtils.isNotBlank(anonymousUserRole)) {
+return anonymousUserRole;
+}
throw new
[GitHub] jai1 commented on a change in pull request #1002: Making Pulsar Proxy more secure
jai1 commented on a change in pull request #1002: Making Pulsar Proxy more
secure
URL: https://github.com/apache/incubator-pulsar/pull/1002#discussion_r161607734
##
File path: pulsar-common/src/main/proto/PulsarApi.proto
##
@@ -186,6 +187,7 @@ message CommandSubscribe {
message CommandPartitionedTopicMetadata {
required string topic= 1;
required uint64 request_id = 2;
+optional string original_principal = 3;
Review comment:
As per the current proxy logic, we use one connection per broker and run all
lookups through this connection. If I use originalPrincipal that broker
received on handleConnect() - I will need to create two (lookup and getMeta)
new connections per lookup.
This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
With regards,
Apache Git Services
[GitHub] jai1 commented on a change in pull request #1002: Making Pulsar Proxy more secure
jai1 commented on a change in pull request #1002: Making Pulsar Proxy more
secure
URL: https://github.com/apache/incubator-pulsar/pull/1002#discussion_r161607640
##
File path:
pulsar-broker/src/main/java/org/apache/pulsar/broker/service/ServerCnx.java
##
@@ -174,77 +174,115 @@ public void exceptionCaught(ChannelHandlerContext ctx,
Throwable cause) throws E
@Override
protected void handleLookup(CommandLookupTopic lookup) {
final long requestId = lookup.getRequestId();
-final String topic = lookup.getTopic();
+final String topicName = lookup.getTopic();
if (log.isDebugEnabled()) {
-log.debug("[{}] Received Lookup from {} for {}", topic,
remoteAddress, requestId);
-}
-final Semaphore lookupSemaphore = service.getLookupRequestSemaphore();
-if (lookupSemaphore.tryAcquire()) {
-lookupDestinationAsync(getBrokerService().pulsar(),
DestinationName.get(topic), lookup.getAuthoritative(),
-getRole(), lookup.getRequestId()).handle((lookupResponse,
ex) -> {
-if (ex == null) {
-ctx.writeAndFlush(lookupResponse);
-} else {
-// it should never happen
-log.warn("[{}] lookup failed with error {}, {}",
remoteAddress, topic, ex.getMessage(), ex);
-ctx.writeAndFlush(
-
newLookupErrorResponse(ServerError.ServiceNotReady, ex.getMessage(),
requestId));
-}
-lookupSemaphore.release();
-return null;
-});
-} else {
-if (log.isDebugEnabled()) {
-log.debug("[{}] Failed lookup due to too many lookup-requests
{}", remoteAddress, topic);
-}
-
ctx.writeAndFlush(newLookupErrorResponse(ServerError.TooManyRequests,
-"Failed due to too many pending lookup requests",
requestId));
+log.debug("[{}] Received Lookup from {} for {}", topicName,
remoteAddress, requestId);
}
-
+final String proxyClientAuthRole = lookup.hasOriginalPrincipal() ?
lookup.getOriginalPrincipal() : this.proxyClientAuthRole;
Review comment:
As per the current proxy logic, we use one connection per broker and run all
lookups through this connection. If I use originalPrincipal that broker
received on handleConnect() - I will need to create two (lookup and getMeta)
new connections per lookup.
This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
With regards,
Apache Git Services
[GitHub] merlimat commented on issue #1065: Issue 1060: changes make MessageId Comparable
merlimat commented on issue #1065: Issue 1060: changes make MessageId Comparable URL: https://github.com/apache/incubator-pulsar/pull/1065#issuecomment-357737104 retest this please This is an automated message from the Apache Git Service. To respond to the message, please log on GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
[GitHub] merlimat commented on issue #1056: DoubleByteBuf fix for Netty > 4.1.12
merlimat commented on issue #1056: DoubleByteBuf fix for Netty > 4.1.12 URL: https://github.com/apache/incubator-pulsar/pull/1056#issuecomment-357730220 > can you merge this? I don't have permissions There are some tests failing (otherwise you should be able to merge). I think they're not related to the `DoubleByteBuf` change, though they might be releated to the netty upgrade to 4.1.19 This is an automated message from the Apache Git Service. To respond to the message, please log on GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
[GitHub] sschepens commented on issue #1056: DoubleByteBuf fix for Netty > 4.1.12
sschepens commented on issue #1056: DoubleByteBuf fix for Netty > 4.1.12 URL: https://github.com/apache/incubator-pulsar/pull/1056#issuecomment-357724800 @merlimat can you merge this? I don't have permissions This is an automated message from the Apache Git Service. To respond to the message, please log on GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
[GitHub] zhaijack opened a new pull request #1066: Issue 937: add CommandGetLastMessageId to make reader know the end of topic
zhaijack opened a new pull request #1066: Issue 937: add CommandGetLastMessageId to make reader know the end of topic URL: https://github.com/apache/incubator-pulsar/pull/1066 This is the first version to collect suggestion to achieve it. ### Motivation We have recently introduced the concept of topic "Reader" as an alternative to the traditional pub-sub consumer abstraction. A common followup request has been to have a way to identify when the reader has reached the last published entry on the topic. There is no currently direct way to achieve that and using readNext(timeout) doesn't help because in case the client is not connected to broker, it doesn't mean that there are no more messages to read. There are a few workaround that are not easy or desirable (eg: terminating the topic, or using HTTP admin API to check the backlog on the reader). Since this is a common theme, we should have a good way to handle this. ### Modifications - add CommandGetLastMessageId in PulsarApi.proto; - implementation CommandGetLastMessageId; - add a testcase in TopicReaderTest ### Result A new command CommandGetLastMessageId added in binary protocol This is an automated message from the Apache Git Service. To respond to the message, please log on GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
[GitHub] massakam closed pull request #899: Add subscription auth mode by prefix
massakam closed pull request #899: Add subscription auth mode by prefix
URL: https://github.com/apache/incubator-pulsar/pull/899
This is a PR merged from a forked repository.
As GitHub hides the original diff on merge, it is displayed below for
the sake of provenance:
As this is a foreign pull request (from a fork), the diff is supplied
below (as it won't show otherwise due to GitHub magic):
diff --git
a/pulsar-broker-common/src/main/java/org/apache/pulsar/broker/authorization/AuthorizationManager.java
b/pulsar-broker-common/src/main/java/org/apache/pulsar/broker/authorization/AuthorizationManager.java
index e33e2a5a6..9fa31ccfd 100644
---
a/pulsar-broker-common/src/main/java/org/apache/pulsar/broker/authorization/AuthorizationManager.java
+++
b/pulsar-broker-common/src/main/java/org/apache/pulsar/broker/authorization/AuthorizationManager.java
@@ -22,8 +22,10 @@
import java.util.Set;
import java.util.concurrent.CompletableFuture;
import static java.util.concurrent.TimeUnit.SECONDS;
+import static org.apache.commons.lang3.StringUtils.isNotBlank;
import static org.apache.pulsar.zookeeper.ZooKeeperCache.cacheTimeOutInSec;
+import org.apache.pulsar.broker.PulsarServerException;
import org.apache.pulsar.broker.ServiceConfiguration;
import org.apache.pulsar.broker.cache.ConfigurationCacheService;
import org.apache.pulsar.common.naming.DestinationName;
@@ -78,14 +80,52 @@ public boolean canProduce(DestinationName destination,
String role) throws Excep
*the fully qualified destination name associated with the
destination.
* @param role
*the app id used to receive messages from the destination.
+ * @param subscription
+ *the subscription name defined by the client
*/
-public CompletableFuture canConsumeAsync(DestinationName
destination, String role) {
-return checkAuthorization(destination, role, AuthAction.consume);
+public CompletableFuture canConsumeAsync(DestinationName
destination, String role, String subscription) {
+CompletableFuture permissionFuture = new
CompletableFuture<>();
+try {
+configCache.policiesCache().getAsync(POLICY_ROOT +
destination.getNamespace()).thenAccept(policies -> {
+if (!policies.isPresent()) {
+if (log.isDebugEnabled()) {
+log.debug("Policies node couldn't be found for
destination : {}", destination);
+}
+} else {
+if (isNotBlank(subscription)) {
+switch (policies.get().subscription_auth_mode) {
+case Prefix:
+if (!subscription.startsWith(role)) {
+PulsarServerException ex = new
PulsarServerException(
+String.format("Failed to create
consumer - The subscription name needs to be prefixed by the authentication
role, like %s- for destination: %s", role, destination));
+permissionFuture.completeExceptionally(ex);
+return;
+}
+break;
+default:
+break;
+}
+}
+}
+checkAuthorization(destination, role,
AuthAction.consume).thenAccept(isAuthorized -> {
+permissionFuture.complete(isAuthorized);
+});
+}).exceptionally(ex -> {
+log.warn("Client with Role - {} failed to get permissions for
destination - {}", role, destination,
+ex);
+permissionFuture.completeExceptionally(ex);
+return null;
+});
+} catch (Exception e) {
+log.warn("Client with Role - {} failed to get permissions for
destination - {}", role, destination, e);
+permissionFuture.completeExceptionally(e);
+}
+return permissionFuture;
}
-public boolean canConsume(DestinationName destination, String role) throws
Exception {
+public boolean canConsume(DestinationName destination, String role, String
subscription) throws Exception {
try {
-return canConsumeAsync(destination, role).get(cacheTimeOutInSec,
SECONDS);
+return canConsumeAsync(destination, role,
subscription).get(cacheTimeOutInSec, SECONDS);
} catch (InterruptedException e) {
log.warn("Time-out {} sec while checking authorization on {} ",
cacheTimeOutInSec, destination);
throw e;
@@ -107,7 +147,7 @@ public boolean canConsume(DestinationName destination,
String role) throws Excep
* @throws Exception
*/
public boolean canLookup(DestinationName destination, String role) throws
Exception {
-return canProduce(destination, role) ||
[GitHub] ivankelly commented on issue #1044: Compact algo
ivankelly commented on issue #1044: Compact algo URL: https://github.com/apache/incubator-pulsar/pull/1044#issuecomment-357644735 Based on #1039, only the second commit should be reviewed here This is an automated message from the Apache Git Service. To respond to the message, please log on GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
[GitHub] ivankelly commented on issue #1044: Compact algo
ivankelly commented on issue #1044: Compact algo URL: https://github.com/apache/incubator-pulsar/pull/1044#issuecomment-357644453 @merlimat @sijie could I get a review on this? This is an automated message from the Apache Git Service. To respond to the message, please log on GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
[GitHub] ivankelly commented on issue #1039: Serialization and Deserialization for RawMessage
ivankelly commented on issue #1039: Serialization and Deserialization for RawMessage URL: https://github.com/apache/incubator-pulsar/pull/1039#issuecomment-357644390 @sijie @merlimat ping This is an automated message from the Apache Git Service. To respond to the message, please log on GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
