alibaba · KomachiSion · Oct 17, 2024 · Oct 17, 2024
diff --git a/auth/src/main/java/com/alibaba/nacos/auth/AbstractProtocolAuthService.java b/auth/src/main/java/com/alibaba/nacos/auth/AbstractProtocolAuthService.java
@@ -29,6 +29,7 @@
 import com.alibaba.nacos.plugin.auth.spi.server.AuthPluginService;
 
 import java.util.Optional;
+import java.util.Properties;
 
 /**
  * Abstract protocol auth service.
@@ -84,7 +85,11 @@ public boolean validateAuthority(IdentityContext identityContext, Permission per
      * @return resource
      */
     protected Resource parseSpecifiedResource(Secured secured) {
-        return new Resource(null, null, secured.resource(), SignType.SPECIFIED, null);
+        Properties properties = new Properties();
+        for (String each : secured.tags()) {
+            properties.put(each, each);
+        }
+        return new Resource(null, null, secured.resource(), SignType.SPECIFIED, properties);
     }
 
     /**

diff --git a/auth/src/test/java/com/alibaba/nacos/auth/GrpcProtocolAuthServiceTest.java b/auth/src/test/java/com/alibaba/nacos/auth/GrpcProtocolAuthServiceTest.java
@@ -21,6 +21,7 @@
 import com.alibaba.nacos.auth.annotation.Secured;
 import com.alibaba.nacos.auth.config.AuthConfigs;
 import com.alibaba.nacos.auth.mock.MockAuthPluginService;
+import com.alibaba.nacos.auth.mock.MockResourceParser;
 import com.alibaba.nacos.plugin.auth.api.IdentityContext;
 import com.alibaba.nacos.plugin.auth.api.Permission;
 import com.alibaba.nacos.plugin.auth.api.Resource;
@@ -85,7 +86,8 @@ void testParseResourceWithSpecifiedResource() throws NoSuchMethodException {
         assertEquals(SignType.SPECIFIED, actual.getType());
         assertNull(actual.getNamespaceId());
         assertNull(actual.getGroup());
-        assertNull(actual.getProperties());
+        assertNotNull(actual.getProperties());
+        assertTrue(actual.getProperties().isEmpty());
     }
 
     @Test
@@ -96,6 +98,14 @@ void testParseResourceWithNonExistType() throws NoSuchMethodException {
         assertEquals(Resource.EMPTY_RESOURCE, actual);
     }
 
+    @Test
+    @Secured(signType = "non-exist", parser = MockResourceParser.class)
+    void testParseResourceWithNonExistTypeException() throws NoSuchMethodException {
+        Secured secured = getMethodSecure("testParseResourceWithNonExistTypeException");
+        Resource actual = protocolAuthService.parseResource(namingRequest, secured);
+        assertEquals(Resource.EMPTY_RESOURCE, actual);
+    }
+
     @Test
     @Secured()
     void testParseResourceWithNamingType() throws NoSuchMethodException {
@@ -152,6 +162,22 @@ void testValidateAuthorityWithPlugin() throws AccessException {
                 new Permission(Resource.EMPTY_RESOURCE, "")));
     }
 
+    @Test
+    @Secured(signType = SignType.CONFIG)
+    void testEnabledAuthWithPlugin() throws NoSuchMethodException {
+        Mockito.when(authConfigs.getNacosAuthSystemType()).thenReturn(MockAuthPluginService.TEST_PLUGIN);
+        Secured secured = getMethodSecure("testEnabledAuthWithPlugin");
+        assertTrue(protocolAuthService.enableAuth(secured));
+    }
+
+    @Test
+    @Secured(signType = SignType.CONFIG)
+    void testEnabledAuthWithoutPlugin() throws NoSuchMethodException {
+        Mockito.when(authConfigs.getNacosAuthSystemType()).thenReturn("non-exist-plugin");
+        Secured secured = getMethodSecure("testEnabledAuthWithoutPlugin");
+        assertFalse(protocolAuthService.enableAuth(secured));
+    }
+
     private Secured getMethodSecure(String methodName) throws NoSuchMethodException {
         Method method = GrpcProtocolAuthServiceTest.class.getDeclaredMethod(methodName);
         return method.getAnnotation(Secured.class);

diff --git a/auth/src/test/java/com/alibaba/nacos/auth/HttpProtocolAuthServiceTest.java b/auth/src/test/java/com/alibaba/nacos/auth/HttpProtocolAuthServiceTest.java
@@ -21,6 +21,7 @@
 import com.alibaba.nacos.auth.annotation.Secured;
 import com.alibaba.nacos.auth.config.AuthConfigs;
 import com.alibaba.nacos.auth.mock.MockAuthPluginService;
+import com.alibaba.nacos.auth.mock.MockResourceParser;
 import com.alibaba.nacos.plugin.auth.api.IdentityContext;
 import com.alibaba.nacos.plugin.auth.api.Permission;
 import com.alibaba.nacos.plugin.auth.api.Resource;
@@ -56,12 +57,12 @@ class HttpProtocolAuthServiceTest {
     @Mock
     private HttpServletRequest request;
 
-    private HttpProtocolAuthService httpProtocolAuthService;
+    private HttpProtocolAuthService protocolAuthService;
 
     @BeforeEach
     void setUp() throws Exception {
-        httpProtocolAuthService = new HttpProtocolAuthService(authConfigs);
-        httpProtocolAuthService.initialize();
+        protocolAuthService = new HttpProtocolAuthService(authConfigs);
+        protocolAuthService.initialize();
         Mockito.when(request.getParameter(eq(CommonParams.NAMESPACE_ID))).thenReturn("testNNs");
         Mockito.when(request.getParameter(eq(CommonParams.GROUP_NAME))).thenReturn("testNG");
         Mockito.when(request.getParameter(eq(CommonParams.SERVICE_NAME))).thenReturn("testS");
@@ -71,30 +72,40 @@ void setUp() throws Exception {
     }
 
     @Test
-    @Secured(resource = "testResource")
+    @Secured(resource = "testResource", tags = {"testTag"})
     void testParseResourceWithSpecifiedResource() throws NoSuchMethodException {
         Secured secured = getMethodSecure("testParseResourceWithSpecifiedResource");
-        Resource actual = httpProtocolAuthService.parseResource(request, secured);
+        Resource actual = protocolAuthService.parseResource(request, secured);
         assertEquals("testResource", actual.getName());
         assertEquals(SignType.SPECIFIED, actual.getType());
         assertNull(actual.getNamespaceId());
         assertNull(actual.getGroup());
-        assertNull(actual.getProperties());
+        assertNotNull(actual.getProperties());
+        assertEquals(1, actual.getProperties().size());
+        assertEquals("testTag", actual.getProperties().get("testTag"));
     }
 
     @Test
     @Secured(signType = "non-exist")
     void testParseResourceWithNonExistType() throws NoSuchMethodException {
         Secured secured = getMethodSecure("testParseResourceWithNonExistType");
-        Resource actual = httpProtocolAuthService.parseResource(request, secured);
+        Resource actual = protocolAuthService.parseResource(request, secured);
+        assertEquals(Resource.EMPTY_RESOURCE, actual);
+    }
+
+    @Test
+    @Secured(signType = "non-exist", parser = MockResourceParser.class)
+    void testParseResourceWithNonExistTypeException() throws NoSuchMethodException {
+        Secured secured = getMethodSecure("testParseResourceWithNonExistTypeException");
+        Resource actual = protocolAuthService.parseResource(request, secured);
         assertEquals(Resource.EMPTY_RESOURCE, actual);
     }
 
     @Test
     @Secured()
     void testParseResourceWithNamingType() throws NoSuchMethodException {
         Secured secured = getMethodSecure("testParseResourceWithNamingType");
-        Resource actual = httpProtocolAuthService.parseResource(request, secured);
+        Resource actual = protocolAuthService.parseResource(request, secured);
         assertEquals(SignType.NAMING, actual.getType());
         assertEquals("testS", actual.getName());
         assertEquals("testNNs", actual.getNamespaceId());
@@ -106,7 +117,7 @@ void testParseResourceWithNamingType() throws NoSuchMethodException {
     @Secured(signType = SignType.CONFIG)
     void testParseResourceWithConfigType() throws NoSuchMethodException {
         Secured secured = getMethodSecure("testParseResourceWithConfigType");
-        Resource actual = httpProtocolAuthService.parseResource(request, secured);
+        Resource actual = protocolAuthService.parseResource(request, secured);
         assertEquals(SignType.CONFIG, actual.getType());
         assertEquals("testD", actual.getName());
         assertEquals("testNNs", actual.getNamespaceId());
@@ -116,36 +127,52 @@ void testParseResourceWithConfigType() throws NoSuchMethodException {
 
     @Test
     void testParseIdentity() {
-        IdentityContext actual = httpProtocolAuthService.parseIdentity(request);
+        IdentityContext actual = protocolAuthService.parseIdentity(request);
         assertNotNull(actual);
     }
 
     @Test
     void testValidateIdentityWithoutPlugin() throws AccessException {
         IdentityContext identityContext = new IdentityContext();
-        assertTrue(httpProtocolAuthService.validateIdentity(identityContext, Resource.EMPTY_RESOURCE));
+        assertTrue(protocolAuthService.validateIdentity(identityContext, Resource.EMPTY_RESOURCE));
     }
 
     @Test
     void testValidateIdentityWithPlugin() throws AccessException {
         Mockito.when(authConfigs.getNacosAuthSystemType()).thenReturn(MockAuthPluginService.TEST_PLUGIN);
         IdentityContext identityContext = new IdentityContext();
-        assertFalse(httpProtocolAuthService.validateIdentity(identityContext, Resource.EMPTY_RESOURCE));
+        assertFalse(protocolAuthService.validateIdentity(identityContext, Resource.EMPTY_RESOURCE));
     }
 
     @Test
     void testValidateAuthorityWithoutPlugin() throws AccessException {
-        assertTrue(httpProtocolAuthService.validateAuthority(new IdentityContext(),
+        assertTrue(protocolAuthService.validateAuthority(new IdentityContext(),
                 new Permission(Resource.EMPTY_RESOURCE, "")));
     }
 
     @Test
     void testValidateAuthorityWithPlugin() throws AccessException {
         Mockito.when(authConfigs.getNacosAuthSystemType()).thenReturn(MockAuthPluginService.TEST_PLUGIN);
-        assertFalse(httpProtocolAuthService.validateAuthority(new IdentityContext(),
+        assertFalse(protocolAuthService.validateAuthority(new IdentityContext(),
                 new Permission(Resource.EMPTY_RESOURCE, "")));
     }
 
+    @Test
+    @Secured(signType = SignType.CONFIG)
+    void testEnabledAuthWithPlugin() throws NoSuchMethodException {
+        Mockito.when(authConfigs.getNacosAuthSystemType()).thenReturn(MockAuthPluginService.TEST_PLUGIN);
+        Secured secured = getMethodSecure("testEnabledAuthWithPlugin");
+        assertTrue(protocolAuthService.enableAuth(secured));
+    }
+
+    @Test
+    @Secured(signType = SignType.CONFIG)
+    void testEnabledAuthWithoutPlugin() throws NoSuchMethodException {
+        Mockito.when(authConfigs.getNacosAuthSystemType()).thenReturn("non-exist-plugin");
+        Secured secured = getMethodSecure("testEnabledAuthWithoutPlugin");
+        assertFalse(protocolAuthService.enableAuth(secured));
+    }
+
     private Secured getMethodSecure(String methodName) throws NoSuchMethodException {
         Method method = HttpProtocolAuthServiceTest.class.getDeclaredMethod(methodName);
         return method.getAnnotation(Secured.class);

diff --git a/auth/src/test/java/com/alibaba/nacos/auth/mock/MockResourceParser.java b/auth/src/test/java/com/alibaba/nacos/auth/mock/MockResourceParser.java
@@ -0,0 +1,30 @@
+/*
+ * Copyright 1999-2023 Alibaba Group Holding Ltd.
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ *      http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package com.alibaba.nacos.auth.mock;
+
+import com.alibaba.nacos.api.exception.runtime.NacosRuntimeException;
+import com.alibaba.nacos.auth.annotation.Secured;
+import com.alibaba.nacos.auth.parser.ResourceParser;
+import com.alibaba.nacos.plugin.auth.api.Resource;
+
+public class MockResourceParser implements ResourceParser<Object> {
+
+    @Override
+    public Resource parse(Object request, Secured secured) {
+        throw new NacosRuntimeException(500);
+    }
+}
diff --git a/auth/src/test/java/com/alibaba/nacos/auth/parser/grpc/ConfigGrpcResourceParserTest.java b/auth/src/test/java/com/alibaba/nacos/auth/parser/grpc/ConfigGrpcResourceParserTest.java
@@ -18,6 +18,7 @@
 
 import com.alibaba.nacos.api.common.Constants;
 import com.alibaba.nacos.api.config.remote.request.ConfigBatchListenRequest;
+import com.alibaba.nacos.api.config.remote.request.ConfigChangeNotifyRequest;
 import com.alibaba.nacos.api.config.remote.request.ConfigPublishRequest;
 import com.alibaba.nacos.api.remote.request.Request;
 import com.alibaba.nacos.auth.annotation.Secured;
@@ -98,6 +99,23 @@ void testParseWithConfigBatchListenRequest() throws NoSuchMethodException {
         assertEquals(StringUtils.EMPTY, actual.getGroup());
         assertEquals(StringUtils.EMPTY, actual.getName());
         assertEquals(Constants.Config.CONFIG_MODULE, actual.getType());
+        request.getConfigListenContexts().clear();
+        actual = resourceParser.parse(request, secured);
+        assertEquals(StringUtils.EMPTY, actual.getNamespaceId());
+        assertEquals(StringUtils.EMPTY, actual.getGroup());
+        assertEquals(StringUtils.EMPTY, actual.getName());
+        assertEquals(Constants.Config.CONFIG_MODULE, actual.getType());
+    }
+
+    @Test
+    @Secured(signType = Constants.Config.CONFIG_MODULE)
+    void testParseWithReflectionRequest() throws NoSuchMethodException {
+        Secured secured = getMethodSecure();
+        Request request = ConfigChangeNotifyRequest.build("rTestD", "rTestG", "rTestNs");
+        Resource actual = resourceParser.parse(request, secured);
+        assertEquals("rTestNs", actual.getNamespaceId());
+        assertEquals("rTestG", actual.getGroup());
+        assertEquals("rTestD", actual.getName());
     }
 
     private Request mockConfigRequest(String tenant, String group, String dataId) {

diff --git a/...-auth-plugin/src/main/java/com/alibaba/nacos/plugin/auth/impl/NacosAuthPluginService.java b/...-auth-plugin/src/main/java/com/alibaba/nacos/plugin/auth/impl/NacosAuthPluginService.java
@@ -99,7 +99,6 @@ private String resolveToken(IdentityContext identityContext) {
     public Boolean validateAuthority(IdentityContext identityContext, Permission permission) throws AccessException {
         NacosUser user = (NacosUser) identityContext.getParameter(AuthConstants.NACOS_USER_KEY);
         authenticationManager.authorize(permission, user);
-
         return true;
     }
 

diff --git a/...th-plugin/src/main/java/com/alibaba/nacos/plugin/auth/impl/controller/UserController.java b/...th-plugin/src/main/java/com/alibaba/nacos/plugin/auth/impl/controller/UserController.java
@@ -175,7 +175,8 @@ public Object deleteUser(@RequestParam String username) {
      * @since 1.2.0
      */
     @PutMapping
-    @Secured(resource = AuthConstants.UPDATE_PASSWORD_ENTRY_POINT, action = ActionTypes.WRITE)
+    @Secured(resource = AuthConstants.UPDATE_PASSWORD_ENTRY_POINT, action = ActionTypes.WRITE, tags = {
+            AuthConstants.UPDATE_PASSWORD_ENTRY_POINT})
     public Object updateUser(@RequestParam String username, @RequestParam String newPassword,
             HttpServletResponse response, HttpServletRequest request) throws IOException {
         // admin or same user

diff --git a/...h-plugin/src/main/java/com/alibaba/nacos/plugin/auth/impl/roles/NacosRoleServiceImpl.java b/...h-plugin/src/main/java/com/alibaba/nacos/plugin/auth/impl/roles/NacosRoleServiceImpl.java
@@ -41,6 +41,7 @@
 import java.util.HashSet;
 import java.util.List;
 import java.util.Map;
+import java.util.Properties;
 import java.util.Set;
 import java.util.concurrent.ConcurrentHashMap;
 import java.util.regex.Pattern;
@@ -120,8 +121,7 @@ private void reload() {
      * @return true if granted, false otherwise
      */
     public boolean hasPermission(NacosUser nacosUser, Permission permission) {
-        //update password
-        if (AuthConstants.UPDATE_PASSWORD_ENTRY_POINT.equals(permission.getResource().getName())) {
+        if (isUpdatePasswordPermission(permission)) {
             return true;
         }
 
@@ -161,6 +161,14 @@ public boolean hasPermission(NacosUser nacosUser, Permission permission) {
         return false;
     }
 
+    /**
+     * If API is update user password, don't do permission check, because there is permission check in API logic.
+     */
+    private boolean isUpdatePasswordPermission(Permission permission) {
+        Properties properties = permission.getResource().getProperties();
+        return null != properties && properties.contains(AuthConstants.UPDATE_PASSWORD_ENTRY_POINT);
+    }
+
     public List<RoleInfo> getRoles(String username) {
         List<RoleInfo> roleInfoList = roleInfoMap.get(username);
         if (!authConfigs.isCachingEnabled() || roleInfoList == null) {

diff --git a/...ugin/src/test/java/com/alibaba/nacos/plugin/auth/impl/roles/NacosRoleServiceImplTest.java b/...ugin/src/test/java/com/alibaba/nacos/plugin/auth/impl/roles/NacosRoleServiceImplTest.java
@@ -38,6 +38,7 @@
 import java.lang.reflect.Method;
 import java.util.Collections;
 import java.util.List;
+import java.util.Properties;
 
 import static org.junit.jupiter.api.Assertions.assertEquals;
 import static org.junit.jupiter.api.Assertions.assertFalse;
@@ -113,10 +114,14 @@ void hasPermission() {
 
         Permission permission2 = new Permission();
         permission2.setAction("rw");
-        Resource resource = new Resource("public", "group", AuthConstants.UPDATE_PASSWORD_ENTRY_POINT, "rw", null);
+        Resource resource = new Resource("public", "group", AuthConstants.UPDATE_PASSWORD_ENTRY_POINT, "rw",
+                new Properties());
         permission2.setResource(resource);
         boolean res2 = nacosRoleService.hasPermission(nacosUser, permission2);
-        assertTrue(res2);
+        assertFalse(res2);
+        resource.getProperties().put(AuthConstants.UPDATE_PASSWORD_ENTRY_POINT, AuthConstants.UPDATE_PASSWORD_ENTRY_POINT);
+        boolean res3 = nacosRoleService.hasPermission(nacosUser, permission2);
+        assertTrue(res3);
     }
 
     @Test
@@ -127,7 +132,8 @@ void getRoles() {
 
     @Test
     void getRolesFromDatabase() {
-        Page<RoleInfo> roleInfoPage = nacosRoleService.getRolesFromDatabase("nacos", "ROLE_ADMIN", 1, Integer.MAX_VALUE);
+        Page<RoleInfo> roleInfoPage = nacosRoleService.getRolesFromDatabase("nacos", "ROLE_ADMIN", 1,
+                Integer.MAX_VALUE);
         assertEquals(0, roleInfoPage.getTotalCount());
     }
 
@@ -141,8 +147,8 @@ void getPermissions() {
 
     @Test
     void getPermissionsByRoleFromDatabase() {
-        Page<PermissionInfo> permissionsByRoleFromDatabase = nacosRoleService.getPermissionsByRoleFromDatabase("role-admin", 1,
-                Integer.MAX_VALUE);
+        Page<PermissionInfo> permissionsByRoleFromDatabase = nacosRoleService.getPermissionsByRoleFromDatabase(
+                "role-admin", 1, Integer.MAX_VALUE);
         assertNull(permissionsByRoleFromDatabase);
     }
 
@@ -169,7 +175,8 @@ void deleteRole() {
 
     @Test
     void getPermissionsFromDatabase() {
-        Page<PermissionInfo> permissionsFromDatabase = nacosRoleService.getPermissionsFromDatabase("role-admin", 1, Integer.MAX_VALUE);
+        Page<PermissionInfo> permissionsFromDatabase = nacosRoleService.getPermissionsFromDatabase("role-admin", 1,
+                Integer.MAX_VALUE);
         assertEquals(0, permissionsFromDatabase.getTotalCount());
     }