Skip to content

Commit

Permalink
address comment
Browse files Browse the repository at this point in the history
  • Loading branch information
@Technoboy-
Technoboy- committed Oct 10, 2024
1 parent d18afa6 commit 153d6ca
Show file tree
Hide file tree
Showing 2 changed files with 77 additions and 69 deletions.
119 changes: 66 additions & 53 deletions ...mon/src/main/java/org/apache/pulsar/broker/authorization/PulsarAuthorizationProvider.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,7 @@
import java.util.Map;
import java.util.Set;
import java.util.concurrent.CompletableFuture;
import java.util.stream.Stream;
import javax.ws.rs.core.Response;
import org.apache.pulsar.broker.PulsarServerException;
import org.apache.pulsar.broker.ServiceConfiguration;
Expand Down Expand Up @@ -255,65 +256,77 @@ public CompletableFuture<Void> grantPermissionAsync(TopicName topicName, Set<Aut
}

public CompletableFuture<Void> grantPermissionAsync(List<GrantTopicPermissionOptions> options) {
return getPoliciesReadOnlyAsync().thenCompose(readonly -> {
if (readonly) {
if (log.isDebugEnabled()) {
log.debug("Policies are read-only. Broker cannot do read-write operations");
}
throw new IllegalStateException("policies are in readonly mode");
}
TopicName topicName = TopicName.get(options.get(0).getTopic());
return pulsarResources.getNamespaceResources()
.setPoliciesAsync(topicName.getNamespaceObject(), policies -> {
options.stream().forEach(o -> {
final String topicUri = TopicName.get(o.getTopic()).toString();
policies.auth_policies.getTopicAuthentication()
.computeIfAbsent(topicUri, __ -> new HashMap<>())
.put(o.getRole(), o.getActions());
});
return policies;
}).whenComplete((__, ex) -> {
if (ex != null) {
log.error("Failed to grant permissions for {}", options);
} else {
log.info("Successfully granted access for {}", options);
return checkNamespace(options.stream().map(o -> TopicName.get(o.getTopic()).getNamespace()))
.thenCompose(__ -> getPoliciesReadOnlyAsync())
.thenCompose(readonly -> {
if (readonly) {
if (log.isDebugEnabled()) {
log.debug("Policies are read-only. Broker cannot do read-write operations");
}
});
});
throw new IllegalStateException("policies are in readonly mode");
}
TopicName topicName = TopicName.get(options.get(0).getTopic());
return pulsarResources.getNamespaceResources()
.setPoliciesAsync(topicName.getNamespaceObject(), policies -> {
options.stream().forEach(o -> {
final String topicUri = TopicName.get(o.getTopic()).toString();
policies.auth_policies.getTopicAuthentication()
.computeIfAbsent(topicUri, __ -> new HashMap<>())
.put(o.getRole(), o.getActions());
});
return policies;
}).whenComplete((__, ex) -> {
if (ex != null) {
log.error("Failed to grant permissions for {}", options);
} else {
log.info("Successfully granted access for {}", options);
}
});
});
}

@Override
public CompletableFuture<Void> revokePermissionAsync(List<RevokeTopicPermissionOptions> options) {
return getPoliciesReadOnlyAsync().thenCompose(readonly -> {
if (readonly) {
if (log.isDebugEnabled()) {
log.debug("Policies are read-only. Broker cannot do read-write operations");
}
throw new IllegalStateException("policies are in readonly mode");
}
TopicName topicName = TopicName.get(options.get(0).getTopic());
return pulsarResources.getNamespaceResources()
.setPoliciesAsync(topicName.getNamespaceObject(), policies -> {
options.stream().forEach(o -> {
final String topicUri = TopicName.get(o.getTopic()).toString();
policies.auth_policies.getTopicAuthentication()
.computeIfPresent(topicUri, (topicNameUri, roles) -> {
roles.remove(o.getRole());
if (roles.isEmpty()) {
return null;
}
return roles;
});
});
return policies;
}).whenComplete((__, ex) -> {
if (ex != null) {
log.error("Failed to revoke permissions for {}", options, ex);
} else {
log.info("Successfully revoke permissions for {}", options);
return checkNamespace(options.stream().map(o -> TopicName.get(o.getTopic()).getNamespace()))
.thenCompose(__ -> getPoliciesReadOnlyAsync())
.thenCompose(readonly -> {
if (readonly) {
if (log.isDebugEnabled()) {
log.debug("Policies are read-only. Broker cannot do read-write operations");
}
});
});
throw new IllegalStateException("policies are in readonly mode");
}
TopicName topicName = TopicName.get(options.get(0).getTopic());
return pulsarResources.getNamespaceResources()
.setPoliciesAsync(topicName.getNamespaceObject(), policies -> {
options.stream().forEach(o -> {
final String topicUri = TopicName.get(o.getTopic()).toString();
policies.auth_policies.getTopicAuthentication()
.computeIfPresent(topicUri, (topicNameUri, roles) -> {
roles.remove(o.getRole());
if (roles.isEmpty()) {
return null;
}
return roles;
});
});
return policies;
}).whenComplete((__, ex) -> {
if (ex != null) {
log.error("Failed to revoke permissions for {}", options, ex);
} else {
log.info("Successfully revoke permissions for {}", options);
}
});
});
}

private CompletableFuture<Void> checkNamespace(Stream<String> namespaces) {
boolean sameNamespace = namespaces.distinct().count() == 1;
if (!sameNamespace) {
throw new IllegalArgumentException("The namespace should be the same");
}
return CompletableFuture.completedFuture(null);
}

@Override
Expand Down
27 changes: 11 additions & 16 deletions pulsar-broker/src/main/java/org/apache/pulsar/broker/admin/impl/NamespacesBase.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -618,13 +618,9 @@ protected CompletableFuture<Void> internalGrantPermissionOnNamespaceAsync(String

protected CompletableFuture<Void> internalGrantPermissionOnTopicsAsync(List<GrantTopicPermissionOptions> options) {
return checkNamespace(options.stream().map(o -> TopicName.get(o.getTopic()).getNamespace()))
.thenCompose(theSameNamespace -> {
if (!theSameNamespace) {
throw new RestException(Status.BAD_REQUEST, "The namespace should be the same");
}

return validateAdminAccessForTenantAsync(TopicName.get(options.get(0).getTopic()).getTenant());
}).thenCompose(__ -> internalCheckTopicExists(options.stream().map(o -> TopicName.get(o.getTopic()))))
.thenCompose(__ -> validateAdminAccessForTenantAsync(
TopicName.get(options.get(0).getTopic()).getTenant())
).thenCompose(__ -> internalCheckTopicExists(options.stream().map(o -> TopicName.get(o.getTopic()))))
.thenCompose(__ -> getAuthorizationService().grantPermissionAsync(options))
.thenAccept(unused -> log.info("[{}] Successfully granted access for {}", clientAppId(), options))
.exceptionally(ex -> {
Expand Down Expand Up @@ -652,13 +648,9 @@ protected CompletableFuture<Void> internalGrantPermissionOnTopicsAsync(List<Gran
protected CompletableFuture<Void> internalRevokePermissionOnTopicsAsync(
List<RevokeTopicPermissionOptions> options) {
return checkNamespace(options.stream().map(o -> TopicName.get(o.getTopic()).getNamespace()))
.thenCompose(theSameNamespace -> {
if (!theSameNamespace) {
throw new RestException(Status.BAD_REQUEST, "The namespace should be the same");
}

return validateAdminAccessForTenantAsync(TopicName.get(options.get(0).getTopic()).getTenant());
}).thenCompose(__ -> internalCheckTopicExists(options.stream().map(o -> TopicName.get(o.getTopic()))))
.thenCompose(__ -> validateAdminAccessForTenantAsync(
TopicName.get(options.get(0).getTopic()).getTenant()))
.thenCompose(__ -> internalCheckTopicExists(options.stream().map(o -> TopicName.get(o.getTopic()))))
.thenCompose(__ -> getAuthorizationService().revokePermissionAsync(options))
.thenAccept(unused -> log.info("[{}] Successfully revoke access for {}", clientAppId(), options))
.exceptionally(ex -> {
Expand All @@ -683,9 +675,12 @@ protected CompletableFuture<Void> internalRevokePermissionOnTopicsAsync(
});
}

private CompletableFuture<Boolean> checkNamespace(Stream<String> namespaces) {
private CompletableFuture<Void> checkNamespace(Stream<String> namespaces) {
boolean sameNamespace = namespaces.distinct().count() == 1;
return CompletableFuture.completedFuture(sameNamespace);
if (!sameNamespace) {
throw new RestException(Status.BAD_REQUEST, "The namespace should be the same");
}
return CompletableFuture.completedFuture(null);
}

private CompletableFuture<Void> internalCheckTopicExists(Stream<TopicName> topicNameStream) {
Expand Down

0 comments on commit 153d6ca

Please sign in to comment.