fix alibaba#9819 (alibaba#9850)

xzqqqcom · Jan 28, 2023 · 332dc1d · 332dc1d
1 parent 41f03fe
commit 332dc1d
Show file tree

Hide file tree

Showing 8 changed files with 54 additions and 63 deletions.
diff --git a/core/src/main/java/com/alibaba/nacos/core/auth/AuthFilter.java b/core/src/main/java/com/alibaba/nacos/core/auth/AuthFilter.java
@@ -79,8 +79,8 @@ public void doFilter(ServletRequest request, ServletResponse response, FilterCha
                 chain.doFilter(request, response);
                 return;
             }
-        } else if (StringUtils.isNotBlank(authConfigs.getServerIdentityKey()) && StringUtils
-                .isNotBlank(authConfigs.getServerIdentityValue())) {
+        } else if (StringUtils.isNotBlank(authConfigs.getServerIdentityKey()) && StringUtils.isNotBlank(
+                authConfigs.getServerIdentityValue())) {
             String serverIdentity = req.getHeader(authConfigs.getServerIdentityKey());
             if (StringUtils.isNotBlank(serverIdentity)) {
                 if (authConfigs.getServerIdentityValue().equals(serverIdentity)) {
@@ -156,9 +156,11 @@ public void doFilter(ServletRequest request, ServletResponse response, FilterCha
      * @param identityContext identity context
      */
     private void injectIdentityId(HttpServletRequest request, IdentityContext identityContext) {
-        String identityId = identityContext
-                .getParameter(com.alibaba.nacos.plugin.auth.constant.Constants.Identity.IDENTITY_ID, StringUtils.EMPTY);
+        String identityId = identityContext.getParameter(
+                com.alibaba.nacos.plugin.auth.constant.Constants.Identity.IDENTITY_ID, StringUtils.EMPTY);
         request.getSession()
                 .setAttribute(com.alibaba.nacos.plugin.auth.constant.Constants.Identity.IDENTITY_ID, identityId);
+        request.getSession().setAttribute(com.alibaba.nacos.plugin.auth.constant.Constants.Identity.IDENTITY_CONTEXT,
+                identityContext);
     }
 }
diff --git a/plugin-default-impl/src/main/java/com/alibaba/nacos/plugin/auth/impl/NacosAuthManager.java b/plugin-default-impl/src/main/java/com/alibaba/nacos/plugin/auth/impl/NacosAuthManager.java
@@ -17,7 +17,6 @@
 package com.alibaba.nacos.plugin.auth.impl;
 
 import com.alibaba.nacos.api.common.Constants;
-import com.alibaba.nacos.plugin.auth.impl.users.User;
 import com.alibaba.nacos.common.utils.StringUtils;
 import com.alibaba.nacos.core.utils.Loggers;
 import com.alibaba.nacos.plugin.auth.api.IdentityContext;
@@ -64,18 +63,14 @@ public class NacosAuthManager {
      * @return user related to this request, null if no user info is found.
      * @throws AccessException if authentication is failed
      */
-    public User login(Object request) throws AccessException {
+    public NacosUser login(Object request) throws AccessException {
         HttpServletRequest req = (HttpServletRequest) request;
         String token = resolveToken(req);
         validate0(token);
-        NacosUser user = getNacosUser(token);
-        req.getSession().setAttribute(AuthConstants.NACOS_USER_KEY, user);
-        req.getSession().setAttribute(com.alibaba.nacos.plugin.auth.constant.Constants.Identity.IDENTITY_ID,
-                user.getUserName());
-        return user;
+        return getNacosUser(token);
     }
 
-    User login(IdentityContext identityContext) throws AccessException {
+    NacosUser login(IdentityContext identityContext) throws AccessException {
         String token = resolveToken(identityContext);
         validate0(token);
         return getNacosUser(token);
@@ -88,12 +83,12 @@ User login(IdentityContext identityContext) throws AccessException {
      * @param user       user who wants to access the resource.
      * @throws AccessException if authorization is failed
      */
-    public void auth(Permission permission, User user) throws AccessException {
+    public void auth(Permission permission, NacosUser user) throws AccessException {
         if (Loggers.AUTH.isDebugEnabled()) {
             Loggers.AUTH.debug("auth permission: {}, user: {}", permission, user);
         }
 
-        if (!roleService.hasPermission(user.getUserName(), permission)) {
+        if (!roleService.hasPermission(user, permission)) {
             throw new AccessException("authorization failed!");
         }
     }

diff --git a/...default-impl/src/main/java/com/alibaba/nacos/plugin/auth/impl/NacosAuthPluginService.java b/...default-impl/src/main/java/com/alibaba/nacos/plugin/auth/impl/NacosAuthPluginService.java
@@ -17,11 +17,10 @@
 package com.alibaba.nacos.plugin.auth.impl;
 
 import com.alibaba.nacos.api.common.Constants;
-import com.alibaba.nacos.plugin.auth.api.Resource;
-import com.alibaba.nacos.plugin.auth.constant.ActionTypes;
-import com.alibaba.nacos.plugin.auth.impl.users.User;
 import com.alibaba.nacos.plugin.auth.api.IdentityContext;
 import com.alibaba.nacos.plugin.auth.api.Permission;
+import com.alibaba.nacos.plugin.auth.api.Resource;
+import com.alibaba.nacos.plugin.auth.constant.ActionTypes;
 import com.alibaba.nacos.plugin.auth.exception.AccessException;
 import com.alibaba.nacos.plugin.auth.impl.constant.AuthConstants;
 import com.alibaba.nacos.plugin.auth.impl.users.NacosUser;
@@ -40,8 +39,6 @@
 @SuppressWarnings("PMD.ServiceOrDaoClassShouldEndWithImplRule")
 public class NacosAuthPluginService implements AuthPluginService {
 
-    private static final String USER_IDENTITY_PARAM_KEY = "user";
-
     private static final List<String> IDENTITY_NAMES = new LinkedList<String>() {
         {
             add(AuthConstants.AUTHORIZATION_HEADER);
@@ -67,17 +64,18 @@ public boolean enableAuth(ActionTypes action, String type) {
     @Override
     public boolean validateIdentity(IdentityContext identityContext, Resource resource) throws AccessException {
         checkNacosAuthManager();
-        User user = nacosAuthManager.login(identityContext);
-        identityContext.setParameter(USER_IDENTITY_PARAM_KEY, user);
+        NacosUser user = nacosAuthManager.login(identityContext);
+        identityContext.setParameter(AuthConstants.NACOS_USER_KEY, user);
         identityContext.setParameter(com.alibaba.nacos.plugin.auth.constant.Constants.Identity.IDENTITY_ID,
                 user.getUserName());
         return true;
     }
 
     @Override
     public Boolean validateAuthority(IdentityContext identityContext, Permission permission) throws AccessException {
-        NacosUser user = (NacosUser) identityContext.getParameter(USER_IDENTITY_PARAM_KEY);
+        NacosUser user = (NacosUser) identityContext.getParameter(AuthConstants.NACOS_USER_KEY);
         nacosAuthManager.auth(permission, user);
+
         return true;
     }
 

diff --git a/...ault-impl/src/main/java/com/alibaba/nacos/plugin/auth/impl/controller/UserController.java b/...ault-impl/src/main/java/com/alibaba/nacos/plugin/auth/impl/controller/UserController.java
@@ -23,6 +23,7 @@
 import com.alibaba.nacos.common.model.RestResultUtils;
 import com.alibaba.nacos.common.utils.JacksonUtils;
 import com.alibaba.nacos.config.server.model.Page;
+import com.alibaba.nacos.plugin.auth.api.IdentityContext;
 import com.alibaba.nacos.plugin.auth.constant.ActionTypes;
 import com.alibaba.nacos.plugin.auth.exception.AccessException;
 import com.alibaba.nacos.plugin.auth.impl.JwtTokenManager;
@@ -154,7 +155,7 @@ public Object updateUser(@RequestParam String username, @RequestParam String new
             response.sendError(HttpServletResponse.SC_UNAUTHORIZED, "session expired!");
             return null;
         }
-
+        
         User user = userDetailsService.getUserFromDatabase(username);
         if (user == null) {
             throw new IllegalArgumentException("user " + username + " not exist!");
@@ -169,8 +170,11 @@ private boolean hasPermission(String username, HttpServletRequest request) throw
         if (!authConfigs.isAuthEnabled()) {
             return true;
         }
-        NacosUser user = (NacosUser) request.getSession().getAttribute(AuthConstants.NACOS_USER_KEY);
-        if (user == null) {
+        IdentityContext identityContext = (IdentityContext) request.getSession()
+                .getAttribute(com.alibaba.nacos.plugin.auth.constant.Constants.Identity.IDENTITY_CONTEXT);
+        NacosUser user;
+        if (identityContext == null
+                || (user = (NacosUser) identityContext.getParameter(AuthConstants.NACOS_USER_KEY)) == null) {
             throw new HttpSessionRequiredException("session expired!");
         }
         // admin
@@ -180,7 +184,7 @@ private boolean hasPermission(String username, HttpServletRequest request) throw
         // same user
         return user.getUserName().equals(username);
     }
-
+    
     /**
      * Get paged users.
      *
@@ -195,7 +199,7 @@ public Page<User> getUsers(@RequestParam int pageNo, @RequestParam int pageSize,
             @RequestParam(name = "username", required = false, defaultValue = "") String username) {
         return userDetailsService.getUsersFromDatabase(pageNo, pageSize, username);
     }
-
+    
     @GetMapping(params = "search=blur")
     @Secured(resource = AuthConstants.CONSOLE_RESOURCE_NAME_PREFIX + "users", action = ActionTypes.READ)
     public Page<User> fuzzySearchUser(@RequestParam int pageNo, @RequestParam int pageSize,
@@ -219,9 +223,9 @@ public Page<User> fuzzySearchUser(@RequestParam int pageNo, @RequestParam int pa
     public Object login(@RequestParam String username, @RequestParam String password, HttpServletResponse response,
             HttpServletRequest request) throws AccessException {
 
-        if (AuthSystemTypes.NACOS.name().equalsIgnoreCase(authConfigs.getNacosAuthSystemType()) || AuthSystemTypes.LDAP
-                .name().equalsIgnoreCase(authConfigs.getNacosAuthSystemType())) {
-            NacosUser user = (NacosUser) authManager.login(request);
+        if (AuthSystemTypes.NACOS.name().equalsIgnoreCase(authConfigs.getNacosAuthSystemType())
+                || AuthSystemTypes.LDAP.name().equalsIgnoreCase(authConfigs.getNacosAuthSystemType())) {
+            NacosUser user = authManager.login(request);
 
             response.addHeader(AuthConstants.AUTHORIZATION_HEADER, AuthConstants.TOKEN_PREFIX + user.getToken());
 

diff --git a/...ult-impl/src/main/java/com/alibaba/nacos/plugin/auth/impl/roles/NacosRoleServiceImpl.java b/...ult-impl/src/main/java/com/alibaba/nacos/plugin/auth/impl/roles/NacosRoleServiceImpl.java
@@ -19,17 +19,18 @@
 import com.alibaba.nacos.auth.config.AuthConfigs;
 import com.alibaba.nacos.common.utils.ConcurrentHashSet;
 import com.alibaba.nacos.common.utils.StringUtils;
-import com.alibaba.nacos.plugin.auth.impl.persistence.PermissionInfo;
-import com.alibaba.nacos.plugin.auth.impl.persistence.PermissionPersistService;
-import com.alibaba.nacos.plugin.auth.impl.persistence.RoleInfo;
-import com.alibaba.nacos.plugin.auth.impl.persistence.RolePersistService;
 import com.alibaba.nacos.config.server.model.Page;
 import com.alibaba.nacos.core.utils.Loggers;
 import com.alibaba.nacos.plugin.auth.api.Permission;
 import com.alibaba.nacos.plugin.auth.api.Resource;
 import com.alibaba.nacos.plugin.auth.constant.Constants;
 import com.alibaba.nacos.plugin.auth.constant.SignType;
 import com.alibaba.nacos.plugin.auth.impl.constant.AuthConstants;
+import com.alibaba.nacos.plugin.auth.impl.persistence.PermissionInfo;
+import com.alibaba.nacos.plugin.auth.impl.persistence.PermissionPersistService;
+import com.alibaba.nacos.plugin.auth.impl.persistence.RoleInfo;
+import com.alibaba.nacos.plugin.auth.impl.persistence.RolePersistService;
+import com.alibaba.nacos.plugin.auth.impl.users.NacosUser;
 import com.alibaba.nacos.plugin.auth.impl.users.NacosUserDetailsServiceImpl;
 import io.jsonwebtoken.lang.Collections;
 import org.springframework.beans.factory.annotation.Autowired;
@@ -112,24 +113,25 @@ private void reload() {
      * <p>Note if the user has many roles, this method returns true if any one role of the user has the desired
      * permission.
      *
-     * @param username   user info
+     * @param nacosUser   user info
      * @param permission permission to auth
      * @return true if granted, false otherwise
      */
-    public boolean hasPermission(String username, Permission permission) {
+    public boolean hasPermission(NacosUser nacosUser, Permission permission) {
         //update password
         if (AuthConstants.UPDATE_PASSWORD_ENTRY_POINT.equals(permission.getResource().getName())) {
             return true;
         }
 
-        List<RoleInfo> roleInfoList = getRoles(username);
+        List<RoleInfo> roleInfoList = getRoles(nacosUser.getUserName());
         if (Collections.isEmpty(roleInfoList)) {
             return false;
         }
 
         // Global admin pass:
         for (RoleInfo roleInfo : roleInfoList) {
             if (AuthConstants.GLOBAL_ADMIN_ROLE.equals(roleInfo.getRole())) {
+                nacosUser.setGlobalAdmin(true);
                 return true;
             }
         }

diff --git a/...-impl/src/test/java/com/alibaba/nacos/plugin/auth/impl/controller/UserControllerTest.java b/...-impl/src/test/java/com/alibaba/nacos/plugin/auth/impl/controller/UserControllerTest.java
@@ -26,32 +26,24 @@
 import com.alibaba.nacos.sys.env.EnvUtil;
 import com.fasterxml.jackson.databind.JsonNode;
 import io.jsonwebtoken.io.Encoders;
-import org.junit.Assert;
 import org.junit.Before;
 import org.junit.Test;
 import org.junit.runner.RunWith;
 import org.mockito.Mock;
 import org.mockito.junit.MockitoJUnitRunner;
 import org.springframework.mock.env.MockEnvironment;
-import org.springframework.mock.web.MockHttpSession;
 
 import javax.servlet.http.HttpServletRequest;
 import javax.servlet.http.HttpServletResponse;
-import java.io.IOException;
 import java.lang.reflect.Field;
 import java.nio.charset.StandardCharsets;
 
 import static org.junit.Assert.assertTrue;
-import static org.mockito.ArgumentMatchers.eq;
-import static org.mockito.Mockito.verify;
 import static org.mockito.Mockito.when;
 
 @RunWith(MockitoJUnitRunner.class)
 public class UserControllerTest {
 
-    @Mock(lenient = true)
-    JwtTokenManager jwtTokenManager;
-
     @Mock
     private HttpServletRequest request;
 
@@ -102,15 +94,6 @@ public void testLoginWithAuthedUser() throws AccessException {
         assertTrue(actualString.contains("\"globalAdmin\":true"));
     }
 
-    @Test
-    public void testSessionExpiredThrowHttpSessionRequiredException() throws IOException {
-        when(authConfigs.isAuthEnabled()).thenReturn(true);
-        when(request.getSession()).thenReturn(new MockHttpSession());
-        Object o = userController.updateUser("nacos", "qwe12345", response, request);
-        Assert.assertNull(o);
-        verify(response).sendError(eq(HttpServletResponse.SC_UNAUTHORIZED), eq("session expired!"));
-    }
-
     private void injectObject(String fieldName, Object value) throws NoSuchFieldException, IllegalAccessException {
         Field field = UserController.class.getDeclaredField(fieldName);
         field.setAccessible(true);

diff --git a/...impl/src/test/java/com/alibaba/nacos/plugin/auth/impl/roles/NacosRoleServiceImplTest.java b/...impl/src/test/java/com/alibaba/nacos/plugin/auth/impl/roles/NacosRoleServiceImplTest.java
@@ -26,6 +26,7 @@
 import com.alibaba.nacos.plugin.auth.impl.persistence.RoleInfo;
 import com.alibaba.nacos.plugin.auth.impl.persistence.RolePersistService;
 import com.alibaba.nacos.plugin.auth.impl.persistence.User;
+import com.alibaba.nacos.plugin.auth.impl.users.NacosUser;
 import com.alibaba.nacos.plugin.auth.impl.users.NacosUserDetailsServiceImpl;
 import org.junit.Assert;
 import org.junit.Before;
@@ -41,6 +42,7 @@
 
 /**
  * NacosRoleServiceImpl Test.
+ *
  * @ClassName: NacosRoleServiceImplTest
  * @Author: ChenHao26
  * @Date: 2022/8/16 17:31
@@ -73,11 +75,11 @@ public void setup() throws Exception {
         Field authConfigsFile = nacosRoleServiceClass.getDeclaredField("authConfigs");
         authConfigsFile.setAccessible(true);
         authConfigsFile.set(nacosRoleService, authConfigs);
-
+        
         Field rolePersistServiceFile = nacosRoleServiceClass.getDeclaredField("rolePersistService");
         rolePersistServiceFile.setAccessible(true);
         rolePersistServiceFile.set(nacosRoleService, rolePersistService);
-
+        
         Field userDetailsServiceField = nacosRoleServiceClass.getDeclaredField("userDetailsService");
         userDetailsServiceField.setAccessible(true);
         userDetailsServiceField.set(nacosRoleService, userDetailsService);
@@ -99,14 +101,16 @@ public void hasPermission() {
         Permission permission = new Permission();
         permission.setAction("rw");
         permission.setResource(Resource.EMPTY_RESOURCE);
-        boolean res = nacosRoleService.hasPermission("nacos", permission);
+        NacosUser nacosUser = new NacosUser();
+        nacosUser.setUserName("nacos");
+        boolean res = nacosRoleService.hasPermission(nacosUser, permission);
         Assert.assertFalse(res);
-
+        
         Permission permission2 = new Permission();
         permission2.setAction("rw");
         Resource resource = new Resource("public", "group", AuthConstants.UPDATE_PASSWORD_ENTRY_POINT, "rw", null);
         permission2.setResource(resource);
-        boolean res2 = nacosRoleService.hasPermission("nacos", permission2);
+        boolean res2 = nacosRoleService.hasPermission(nacosUser, permission2);
         Assert.assertTrue(res2);
     }
 
@@ -118,7 +122,8 @@ public void getRoles() {
 
     @Test
     public 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);
         Assert.assertEquals(roleInfoPage.getTotalCount(), 0);
     }
 
@@ -169,7 +174,7 @@ public void getPermissionsFromDatabase() {
     public void addPermission() {
         try {
             nacosRoleService.addPermission("role-admin", "", "rw");
-        }  catch (Exception e) {
+        } catch (Exception e) {
             Assert.assertTrue(e.getMessage().contains("role role-admin not found!"));
         }
     }

diff --git a/plugin/auth/src/main/java/com/alibaba/nacos/plugin/auth/constant/Constants.java b/plugin/auth/src/main/java/com/alibaba/nacos/plugin/auth/constant/Constants.java
@@ -46,16 +46,18 @@ public static class Resource {
         public static final String ANY = "*";
 
         public static final String ACTION = "action";
-
+        
         public static final String REQUEST_CLASS = "requestClass";
     }
 
     public static class Identity {
 
         public static final String IDENTITY_ID = "identity_id";
-
+        
         public static final String X_REAL_IP = "X-Real-IP";
 
         public static final String REMOTE_IP = "remote_ip";
+
+        public static final String IDENTITY_CONTEXT = "identity_context";
     }
 }