From 197795a854992d3f88824e4cdde34c61d8f56cf4 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=E6=9D=A8=E7=BF=8A=20SionYang?= Date: Mon, 22 Jul 2024 13:32:41 +0800 Subject: [PATCH] Fix #12395, use request context replace session depend. (#12398) --- .../auth/impl/controller/UserController.java | 11 ++-- .../impl/controller/UserControllerTest.java | 56 +++++++++---------- 2 files changed, 34 insertions(+), 33 deletions(-) diff --git a/plugin-default-impl/nacos-default-auth-plugin/src/main/java/com/alibaba/nacos/plugin/auth/impl/controller/UserController.java b/plugin-default-impl/nacos-default-auth-plugin/src/main/java/com/alibaba/nacos/plugin/auth/impl/controller/UserController.java index 554b920ba0a..ed904b83a6e 100644 --- a/plugin-default-impl/nacos-default-auth-plugin/src/main/java/com/alibaba/nacos/plugin/auth/impl/controller/UserController.java +++ b/plugin-default-impl/nacos-default-auth-plugin/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.common.utils.StringUtils; +import com.alibaba.nacos.core.context.RequestContextHolder; import com.alibaba.nacos.persistence.model.Page; import com.alibaba.nacos.plugin.auth.api.IdentityContext; import com.alibaba.nacos.plugin.auth.constant.ActionTypes; @@ -102,7 +103,11 @@ public class UserController { @Secured(resource = AuthConstants.CONSOLE_RESOURCE_NAME_PREFIX + "users", action = ActionTypes.WRITE) @PostMapping public Object createUser(@RequestParam String username, @RequestParam String password) { - + if (AuthConstants.DEFAULT_USER.equals(username)) { + return RestResultUtils.failed(HttpStatus.CONFLICT.value(), + "User `nacos` is default admin user. Please use `/nacos/v1/auth/users/admin` API to init `nacos` users. " + + "Detail see `https://nacos.io/docs/latest/manual/admin/auth/#31-%E8%AE%BE%E7%BD%AE%E7%AE%A1%E7%90%86%E5%91%98%E5%AF%86%E7%A0%81`"); + } User user = userDetailsService.getUserFromDatabase(username); if (user != null) { throw new IllegalArgumentException("user '" + username + "' already exist!"); @@ -202,8 +207,7 @@ private boolean hasPermission(String username, HttpServletRequest request) if (!authConfigs.isAuthEnabled()) { return true; } - IdentityContext identityContext = (IdentityContext) request.getSession() - .getAttribute(com.alibaba.nacos.plugin.auth.constant.Constants.Identity.IDENTITY_CONTEXT); + IdentityContext identityContext = RequestContextHolder.getContext().getAuthContext().getIdentityContext(); if (identityContext == null) { throw new HttpSessionRequiredException("session expired!"); } @@ -324,7 +328,6 @@ public RestResult updatePassword(@RequestParam(value = "oldPassword") St } } - /** * Fuzzy matching username. * diff --git a/plugin-default-impl/nacos-default-auth-plugin/src/test/java/com/alibaba/nacos/plugin/auth/impl/controller/UserControllerTest.java b/plugin-default-impl/nacos-default-auth-plugin/src/test/java/com/alibaba/nacos/plugin/auth/impl/controller/UserControllerTest.java index 3a88a98a067..391d55a0b6d 100644 --- a/plugin-default-impl/nacos-default-auth-plugin/src/test/java/com/alibaba/nacos/plugin/auth/impl/controller/UserControllerTest.java +++ b/plugin-default-impl/nacos-default-auth-plugin/src/test/java/com/alibaba/nacos/plugin/auth/impl/controller/UserControllerTest.java @@ -18,6 +18,7 @@ import com.alibaba.nacos.auth.config.AuthConfigs; import com.alibaba.nacos.common.model.RestResult; +import com.alibaba.nacos.core.context.RequestContextHolder; import com.alibaba.nacos.persistence.model.Page; import com.alibaba.nacos.plugin.auth.api.IdentityContext; import com.alibaba.nacos.plugin.auth.exception.AccessException; @@ -33,6 +34,7 @@ import com.alibaba.nacos.sys.env.EnvUtil; import com.fasterxml.jackson.databind.JsonNode; import com.fasterxml.jackson.databind.node.ObjectNode; +import org.junit.jupiter.api.AfterEach; import org.junit.jupiter.api.BeforeEach; import org.junit.jupiter.api.Test; import org.junit.jupiter.api.extension.ExtendWith; @@ -105,6 +107,12 @@ void setUp() throws Exception { AuthConstants.DEFAULT_TOKEN_EXPIRE_SECONDS.toString()); EnvUtil.setEnvironment(mockEnvironment); + RequestContextHolder.getContext().getAuthContext().setIdentityContext(new IdentityContext()); + } + + @AfterEach + public void tearDown() { + RequestContextHolder.removeContext(); } @Test @@ -123,20 +131,26 @@ void testLoginWithAuthedUser() throws AccessException, IOException { @Test void testCreateUser1() { - when(userDetailsService.getUserFromDatabase("nacos")).thenReturn(null); - RestResult result = (RestResult) userController.createUser("nacos", "test"); + when(userDetailsService.getUserFromDatabase("test")).thenReturn(null); + RestResult result = (RestResult) userController.createUser("test", "test"); assertEquals(200, result.getCode()); } @Test void testCreateUser2() { - when(userDetailsService.getUserFromDatabase("nacos")).thenReturn(new User()); + when(userDetailsService.getUserFromDatabase("test")).thenReturn(new User()); assertThrows(IllegalArgumentException.class, () -> { - userController.createUser("nacos", "test"); + userController.createUser("test", "test"); }); } + @Test + void testCreateUserNamedNacos() { + RestResult result = (RestResult) userController.createUser("nacos", "test"); + assertEquals(409, result.getCode()); + } + @Test void testCreateAdminUser1() { when(authConfigs.getNacosAuthSystemType()).thenReturn(AuthSystemTypes.NACOS.name()); @@ -221,7 +235,7 @@ void testUpdateUser2() { @Test void testUpdateUser3() throws IOException { - + RequestContextHolder.getContext().getAuthContext().setIdentityContext(null); when(authConfigs.isAuthEnabled()).thenReturn(true); MockHttpServletRequest mockHttpServletRequest = new MockHttpServletRequest(); MockHttpServletResponse mockHttpServletResponse = new MockHttpServletResponse(); @@ -234,15 +248,11 @@ void testUpdateUser3() throws IOException { @Test void testUpdateUser4() throws IOException { - + RequestContextHolder.getContext().getAuthContext().getIdentityContext() + .setParameter(AuthConstants.NACOS_USER_KEY, user); when(authConfigs.isAuthEnabled()).thenReturn(true); when(userDetailsService.getUserFromDatabase(anyString())).thenReturn(new User()); MockHttpServletRequest mockHttpServletRequest = new MockHttpServletRequest(); - IdentityContext identityContext = new IdentityContext(); - identityContext.setParameter(AuthConstants.NACOS_USER_KEY, user); - mockHttpServletRequest.getSession() - .setAttribute(com.alibaba.nacos.plugin.auth.constant.Constants.Identity.IDENTITY_CONTEXT, - identityContext); MockHttpServletResponse mockHttpServletResponse = new MockHttpServletResponse(); RestResult result = (RestResult) userController.updateUser("nacos", "test", mockHttpServletResponse, mockHttpServletRequest); @@ -252,17 +262,13 @@ void testUpdateUser4() throws IOException { @Test void testUpdateUser5() throws IOException, AccessException { - + RequestContextHolder.getContext().getAuthContext().getIdentityContext() + .setParameter(AuthConstants.NACOS_USER_KEY, null); when(authConfigs.isAuthEnabled()).thenReturn(true); when(userDetailsService.getUserFromDatabase(anyString())).thenReturn(new User()); when(authenticationManager.authenticate(any(MockHttpServletRequest.class))).thenReturn(user); MockHttpServletRequest mockHttpServletRequest = new MockHttpServletRequest(); - IdentityContext identityContext = new IdentityContext(); - identityContext.setParameter(AuthConstants.NACOS_USER_KEY, null); - mockHttpServletRequest.getSession() - .setAttribute(com.alibaba.nacos.plugin.auth.constant.Constants.Identity.IDENTITY_CONTEXT, - identityContext); MockHttpServletResponse mockHttpServletResponse = new MockHttpServletResponse(); RestResult result = (RestResult) userController.updateUser("nacos", "test", mockHttpServletResponse, mockHttpServletRequest); @@ -272,16 +278,12 @@ void testUpdateUser5() throws IOException, AccessException { @Test void testUpdateUser6() throws IOException, AccessException { - + RequestContextHolder.getContext().getAuthContext().getIdentityContext() + .setParameter(AuthConstants.NACOS_USER_KEY, null); when(authConfigs.isAuthEnabled()).thenReturn(true); when(authenticationManager.authenticate(any(MockHttpServletRequest.class))).thenReturn(null); MockHttpServletRequest mockHttpServletRequest = new MockHttpServletRequest(); - IdentityContext identityContext = new IdentityContext(); - identityContext.setParameter(AuthConstants.NACOS_USER_KEY, null); - mockHttpServletRequest.getSession() - .setAttribute(com.alibaba.nacos.plugin.auth.constant.Constants.Identity.IDENTITY_CONTEXT, - identityContext); MockHttpServletResponse mockHttpServletResponse = new MockHttpServletResponse(); Object result = userController.updateUser("nacos", "test", mockHttpServletResponse, mockHttpServletRequest); @@ -292,17 +294,13 @@ void testUpdateUser6() throws IOException, AccessException { @Test void testUpdateUser7() throws IOException, AccessException { - + RequestContextHolder.getContext().getAuthContext().getIdentityContext() + .setParameter(AuthConstants.NACOS_USER_KEY, null); when(authConfigs.isAuthEnabled()).thenReturn(true); when(authenticationManager.authenticate(any(MockHttpServletRequest.class))).thenThrow( new AccessException("test")); MockHttpServletRequest mockHttpServletRequest = new MockHttpServletRequest(); - IdentityContext identityContext = new IdentityContext(); - identityContext.setParameter(AuthConstants.NACOS_USER_KEY, null); - mockHttpServletRequest.getSession() - .setAttribute(com.alibaba.nacos.plugin.auth.constant.Constants.Identity.IDENTITY_CONTEXT, - identityContext); MockHttpServletResponse mockHttpServletResponse = new MockHttpServletResponse(); Object result = userController.updateUser("nacos", "test", mockHttpServletResponse, mockHttpServletRequest);