Skip to content

Commit

Permalink
Fix subscription permission not working in reset cursor (#11132)
Browse files Browse the repository at this point in the history
### Motivation

Some `internalResetCursorXX` methods do not pass in the `subscriptionName` parameter when verifying permissions, which causes the `subscription` check to be skipped during the permission check of `AuthorizationProvider#canConsumeAsync` and leads an error validation result. This PR will fix this problem.

### Modifications

Refine the parameters of `validateTopicOperation` and supplement a relative test case.
wuzhanpeng authored Jun 28, 2021

Verified

This commit was created on GitHub.com and signed with GitHub’s verified signature. The key has expired.
1 parent 5e4cbb6 commit da66d0e
Showing 2 changed files with 10 additions and 2 deletions.
Original file line number Diff line number Diff line change
@@ -1934,7 +1934,7 @@ private void internalResetCursorForNonPartitionedTopic(AsyncResponse asyncRespon
boolean authoritative) {
try {
validateTopicOwnership(topicName, authoritative);
validateTopicOperation(topicName, TopicOperation.RESET_CURSOR);
validateTopicOperation(topicName, TopicOperation.RESET_CURSOR, subName);

log.info("[{}] [{}] Received reset cursor on subscription {} to time {}",
clientAppId(), topicName, subName, timestamp);
@@ -2157,7 +2157,7 @@ protected void internalResetCursorOnPosition(AsyncResponse asyncResponse, String
return;
} else {
validateTopicOwnership(topicName, authoritative);
validateTopicOperation(topicName, TopicOperation.RESET_CURSOR);
validateTopicOperation(topicName, TopicOperation.RESET_CURSOR, subName);

PersistentTopic topic = (PersistentTopic) getTopicReference(topicName);
if (topic == null) {
Original file line number Diff line number Diff line change
@@ -247,6 +247,14 @@ public void testSubscriberPermission() throws Exception {
// Ok
}

// reset on position
try {
sub1Admin.topics().resetCursor(topicName, subscriptionName, MessageId.earliest);
fail("should have fail with authorization exception");
} catch (org.apache.pulsar.client.admin.PulsarAdminException.NotAuthorizedException e) {
// Ok
}

// now, grant subscription-access to subscriptionRole as well
superAdmin.namespaces().grantPermissionOnSubscription(namespace, subscriptionName,
Sets.newHashSet(otherPrincipal, subscriptionRole));

0 comments on commit da66d0e

Please sign in to comment.