Skip to content

Commit 035c91a

Browse files
majialoongmajialoong
authored
[ISSUE #9763] Fix invalid user disable status check in authorization (#9764)
1 parent 08b62b9 commit 035c91a

File tree

2 files changed

+165
-3
lines changed

2 files changed

+165
-3
lines changed

‎auth/src/main/java/org/apache/rocketmq/auth/authorization/chain/UserAuthorizationHandler.java‎

Lines changed: 2 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -21,7 +21,6 @@
2121
import org.apache.rocketmq.auth.authentication.enums.SubjectType;
2222
import org.apache.rocketmq.auth.authentication.enums.UserStatus;
2323
import org.apache.rocketmq.auth.authentication.enums.UserType;
24-
import org.apache.rocketmq.auth.authentication.exception.AuthenticationException;
2524
import org.apache.rocketmq.auth.authentication.factory.AuthenticationFactory;
2625
import org.apache.rocketmq.auth.authentication.model.Subject;
2726
import org.apache.rocketmq.auth.authentication.model.User;
@@ -62,8 +61,8 @@ private CompletableFuture<User> getUser(Subject subject) {
6261
if (result == null) {
6362
throw new AuthorizationException("User:{} not found.", user.getUsername());
6463
}
65-
if (user.getUserStatus() == UserStatus.DISABLE) {
66-
throw new AuthenticationException("User:{} is disabled.", user.getUsername());
64+
if (result.getUserStatus() == UserStatus.DISABLE) {
65+
throw new AuthorizationException("User:{} is disabled.", result.getUsername());
6766
}
6867
return result;
6968
});

‎auth/src/test/java/org/apache/rocketmq/auth/authorization/chain/UserAuthorizationHandlerTest.java‎

Lines changed: 163 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,163 @@
1+
/*
2+
* Licensed to the Apache Software Foundation (ASF) under one or more
3+
* contributor license agreements. See the NOTICE file distributed with
4+
* this work for additional information regarding copyright ownership.
5+
* The ASF licenses this file to You under the Apache License, Version 2.0
6+
* (the "License"); you may not use this file except in compliance with
7+
* the License. You may obtain a copy of the License at
8+
*
9+
* http://www.apache.org/licenses/LICENSE-2.0
10+
*
11+
* Unless required by applicable law or agreed to in writing, software
12+
* distributed under the License is distributed on an "AS IS" BASIS,
13+
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
14+
* See the License for the specific language governing permissions and
15+
* limitations under the License.
16+
*/
17+
package org.apache.rocketmq.auth.authorization.chain;
18+
19+
import java.util.List;
20+
import java.util.concurrent.CompletableFuture;
21+
22+
import org.apache.commons.collections.CollectionUtils;
23+
import org.apache.rocketmq.auth.authentication.enums.UserStatus;
24+
import org.apache.rocketmq.auth.authentication.enums.UserType;
25+
import org.apache.rocketmq.auth.authentication.factory.AuthenticationFactory;
26+
import org.apache.rocketmq.auth.authentication.manager.AuthenticationMetadataManager;
27+
import org.apache.rocketmq.auth.authentication.model.Subject;
28+
import org.apache.rocketmq.auth.authentication.model.User;
29+
import org.apache.rocketmq.auth.authorization.context.DefaultAuthorizationContext;
30+
import org.apache.rocketmq.auth.authorization.exception.AuthorizationException;
31+
import org.apache.rocketmq.auth.config.AuthConfig;
32+
import org.apache.rocketmq.auth.helper.AuthTestHelper;
33+
import org.apache.rocketmq.common.MixAll;
34+
import org.apache.rocketmq.common.action.Action;
35+
import org.apache.rocketmq.common.chain.HandlerChain;
36+
import org.apache.rocketmq.auth.authorization.model.Resource;
37+
import org.junit.After;
38+
import org.junit.Assert;
39+
import org.junit.Before;
40+
import org.junit.Test;
41+
42+
import static org.mockito.ArgumentMatchers.any;
43+
import static org.mockito.Mockito.mock;
44+
import static org.mockito.Mockito.never;
45+
import static org.mockito.Mockito.times;
46+
import static org.mockito.Mockito.verify;
47+
import static org.mockito.Mockito.when;
48+
49+
50+
public class UserAuthorizationHandlerTest {
51+
52+
private AuthConfig authConfig;
53+
private AuthenticationMetadataManager authenticationMetadataManager;
54+
private UserAuthorizationHandler handler;
55+
private HandlerChain<DefaultAuthorizationContext, CompletableFuture<Void>> nextChain;
56+
57+
@Before
58+
public void setUp() {
59+
if (MixAll.isMac()) {
60+
return;
61+
}
62+
this.authConfig = AuthTestHelper.createDefaultConfig();
63+
this.authenticationMetadataManager = AuthenticationFactory.getMetadataManager(this.authConfig);
64+
this.handler = new UserAuthorizationHandler(this.authConfig, null);
65+
this.nextChain = mock(HandlerChain.class);
66+
clearAllUsers();
67+
}
68+
69+
@After
70+
public void tearDown() {
71+
if (MixAll.isMac()) {
72+
return;
73+
}
74+
clearAllUsers();
75+
this.authenticationMetadataManager.shutdown();
76+
}
77+
78+
@Test
79+
public void testUserNotFoundThrows() {
80+
if (MixAll.isMac()) {
81+
return;
82+
}
83+
User noSuchUser = User.of("no_such_user", "pwd");
84+
DefaultAuthorizationContext ctx = buildContext(noSuchUser, Resource.ofTopic("t1"), Action.SUB, "127.0.0.1");
85+
86+
AuthorizationException authorizationException = Assert.assertThrows(AuthorizationException.class, () -> {
87+
try {
88+
handler.handle(ctx, nextChain).join();
89+
} catch (Exception e) {
90+
AuthTestHelper.handleException(e);
91+
}
92+
});
93+
Assert.assertEquals("User:no_such_user not found.", authorizationException.getMessage());
94+
}
95+
96+
@Test
97+
public void testUserDisabledThrows() {
98+
if (MixAll.isMac()) {
99+
return;
100+
}
101+
User user = User.of("disabled", "pwd");
102+
authenticationMetadataManager.createUser(user).join();
103+
User saved = authenticationMetadataManager.getUser("disabled").join();
104+
saved.setUserStatus(UserStatus.DISABLE);
105+
authenticationMetadataManager.updateUser(saved).join();
106+
107+
DefaultAuthorizationContext ctx = buildContext(user, Resource.ofTopic("t1"), Action.SUB, "127.0.0.1");
108+
109+
AuthorizationException authorizationException = Assert.assertThrows(AuthorizationException.class, () -> {
110+
try {
111+
handler.handle(ctx, nextChain).join();
112+
} catch (Exception e) {
113+
AuthTestHelper.handleException(e);
114+
}
115+
});
116+
117+
Assert.assertEquals("User:disabled is disabled.", authorizationException.getMessage());
118+
verify(nextChain, never()).handle(any());
119+
}
120+
121+
@Test
122+
public void testSuperUserBypassNextChain() {
123+
if (MixAll.isMac()) {
124+
return;
125+
}
126+
User superUser = User.of("super", "pwd", UserType.SUPER);
127+
authenticationMetadataManager.createUser(superUser).join();
128+
129+
DefaultAuthorizationContext ctx = buildContext(superUser, Resource.ofTopic("t1"), Action.SUB, "127.0.0.1");
130+
131+
handler.handle(ctx, nextChain).join();
132+
// super user should bypass the next chain
133+
verify(nextChain, never()).handle(any());
134+
}
135+
136+
@Test
137+
public void testNormalUserGoesToNextChain() {
138+
if (MixAll.isMac()) {
139+
return;
140+
}
141+
User normalUser = User.of("normal", "pwd", UserType.NORMAL);
142+
authenticationMetadataManager.createUser(normalUser).join();
143+
144+
DefaultAuthorizationContext ctx = buildContext(normalUser, Resource.ofTopic("t1"), Action.SUB, "127.0.0.1");
145+
146+
when(nextChain.handle(any())).thenReturn(CompletableFuture.completedFuture(null));
147+
handler.handle(ctx, nextChain).join();
148+
// normal user should go to the next chain
149+
verify(nextChain, times(1)).handle(any());
150+
}
151+
152+
private DefaultAuthorizationContext buildContext(Subject subject, Resource resource, Action action, String sourceIp) {
153+
return DefaultAuthorizationContext.of(subject, resource, action, sourceIp);
154+
}
155+
156+
private void clearAllUsers() {
157+
List<User> users = this.authenticationMetadataManager.listUser(null).join();
158+
if (CollectionUtils.isEmpty(users)) {
159+
return;
160+
}
161+
users.forEach(user -> this.authenticationMetadataManager.deleteUser(user.getUsername()).join());
162+
}
163+
}

0 commit comments

Comments
 (0)