Skip to content

Commit d0ddb8e

Browse files
committed
Fix error in compare policyEntry
1 parent 2accb97 commit d0ddb8e

2 files changed

Lines changed: 52 additions & 14 deletions

File tree

auth/src/main/java/org/apache/rocketmq/auth/authorization/chain/AclAuthorizationHandler.java

Lines changed: 11 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -53,7 +53,7 @@ public AclAuthorizationHandler(AuthConfig config, Supplier<?> metadataService) {
5353

5454
@Override
5555
public CompletableFuture<Void> handle(DefaultAuthorizationContext context,
56-
HandlerChain<DefaultAuthorizationContext, CompletableFuture<Void>> chain) {
56+
HandlerChain<DefaultAuthorizationContext, CompletableFuture<Void>> chain) {
5757
if (this.authorizationMetadataProvider == null) {
5858
throw new AuthorizationException("The authorizationMetadataProvider is not configured");
5959
}
@@ -112,10 +112,10 @@ private List<PolicyEntry> matchPolicyEntries(DefaultAuthorizationContext context
112112
return null;
113113
}
114114
return entries.stream()
115-
.filter(entry -> entry.isMatchResource(context.getResource()))
116-
.filter(entry -> entry.isMatchAction(context.getActions()))
117-
.filter(entry -> entry.isMatchEnvironment(Environment.of(context.getSourceIp())))
118-
.collect(Collectors.toList());
115+
.filter(entry -> entry.isMatchResource(context.getResource()))
116+
.filter(entry -> entry.isMatchAction(context.getActions()))
117+
.filter(entry -> entry.isMatchEnvironment(Environment.of(context.getSourceIp())))
118+
.collect(Collectors.toList());
119119
}
120120

121121
private int comparePolicyEntries(PolicyEntry o1, PolicyEntry o2) {
@@ -133,20 +133,17 @@ private int comparePolicyEntries(PolicyEntry o1, PolicyEntry o2) {
133133
if (r1.getResourcePattern() == ResourcePattern.PREFIXED) {
134134
String n1 = r1.getResourceName();
135135
String n2 = r2.getResourceName();
136-
compare = Integer.compare(n1.length(), n2.length());
136+
compare = -1 * Integer.compare(n1.length(), n2.length());
137137
}
138138
} else {
139-
if (r1.getResourcePattern() == ResourcePattern.LITERAL) {
140-
compare = 1;
141-
}
142139
if (r1.getResourcePattern() == ResourcePattern.LITERAL) {
143140
compare = -1;
144-
}
145-
if (r1.getResourcePattern() == ResourcePattern.PREFIXED) {
141+
} else if (r2.getResourcePattern() == ResourcePattern.LITERAL) {
146142
compare = 1;
147-
}
148-
if (r1.getResourcePattern() == ResourcePattern.PREFIXED) {
143+
} else if (r1.getResourcePattern() == ResourcePattern.PREFIXED) {
149144
compare = -1;
145+
} else if (r2.getResourcePattern() == ResourcePattern.PREFIXED) {
146+
compare = 1;
150147
}
151148
}
152149

@@ -162,6 +159,6 @@ private int comparePolicyEntries(PolicyEntry o1, PolicyEntry o2) {
162159

163160
private static void throwException(DefaultAuthorizationContext context, String detail) {
164161
throw new AuthorizationException("{} has no permission to access {} from {}, " + detail,
165-
context.getSubject().getSubjectKey(), context.getResource().getResourceKey(), context.getSourceIp());
162+
context.getSubject().getSubjectKey(), context.getResource().getResourceKey(), context.getSourceIp());
166163
}
167164
}

auth/src/test/java/org/apache/rocketmq/auth/authorization/AuthorizationEvaluatorTest.java

Lines changed: 41 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -24,6 +24,7 @@
2424
import org.apache.rocketmq.auth.authentication.manager.AuthenticationMetadataManager;
2525
import org.apache.rocketmq.auth.authentication.model.Subject;
2626
import org.apache.rocketmq.auth.authentication.model.User;
27+
import org.apache.rocketmq.auth.authorization.chain.AclAuthorizationHandler;
2728
import org.apache.rocketmq.auth.authorization.context.AuthorizationContext;
2829
import org.apache.rocketmq.auth.authorization.context.DefaultAuthorizationContext;
2930
import org.apache.rocketmq.auth.authorization.enums.Decision;
@@ -345,6 +346,46 @@ public void evaluate8() {
345346
}
346347
}
347348

349+
@Test
350+
public void evaluate9() {
351+
if (MixAll.isMac()) {
352+
return;
353+
}
354+
User user = User.of("test", "test");
355+
this.authenticationMetadataManager.createUser(user).join();
356+
357+
Acl acl0 = AuthTestHelper.buildAcl("User:test", "*", "Pub", "192.168.0.0/24", Decision.ALLOW);
358+
this.authorizationMetadataManager.createAcl(acl0).join();
359+
Acl acl1 = AuthTestHelper.buildAcl("User:test", "Topic:*", "Pub", "192.168.0.0/24", Decision.ALLOW);
360+
this.authorizationMetadataManager.createAcl(acl1).join();
361+
Acl acl2 = AuthTestHelper.buildAcl("User:test", "Topic:test*", "Pub", "192.168.0.0/24", Decision.ALLOW);
362+
this.authorizationMetadataManager.createAcl(acl2).join();
363+
Acl acl3 = AuthTestHelper.buildAcl("User:test", "Topic:test_*", "Pub", "192.168.0.0/24", Decision.DENY);
364+
this.authorizationMetadataManager.createAcl(acl3).join();
365+
Acl acl4 = AuthTestHelper.buildAcl("User:test", "Topic:test_001", "Pub", "192.168.0.0/24", Decision.DENY);
366+
this.authorizationMetadataManager.createAcl(acl4).join();
367+
368+
Assert.assertThrows(AuthorizationException.class, () -> {
369+
Subject subject = Subject.of("User:test");
370+
Resource resource = Resource.ofTopic("test_001");
371+
Action action = Action.PUB;
372+
String sourceIp = "192.168.0.1";
373+
DefaultAuthorizationContext context = DefaultAuthorizationContext.of(subject, resource, action, sourceIp);
374+
context.setRpcCode("10");
375+
this.evaluator.evaluate(Collections.singletonList(context));
376+
});
377+
378+
Assert.assertThrows(AuthorizationException.class, () -> {
379+
Subject subject = Subject.of("User:test");
380+
Resource resource = Resource.ofTopic("test_002");
381+
Action action = Action.PUB;
382+
String sourceIp = "192.168.0.1";
383+
DefaultAuthorizationContext context = DefaultAuthorizationContext.of(subject, resource, action, sourceIp);
384+
context.setRpcCode("10");
385+
this.evaluator.evaluate(Collections.singletonList(context));
386+
});
387+
}
388+
348389
private void clearAllUsers() {
349390
List<User> users = this.authenticationMetadataManager.listUser(null).join();
350391
if (CollectionUtils.isEmpty(users)) {

0 commit comments

Comments
 (0)