Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

[ISSUES #8417] throw a HttpSessionRequiredException when sessions expired #8419

Merged
merged 5 commits into from
May 23, 2022
Merged

[ISSUES #8417] throw a HttpSessionRequiredException when sessions expired #8419

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
5203788
[ISSUE #8417] throw a HttpSessionRequiredException when sessions expired
onewe May 20, 2022
02fc0ed
[ISSUE #8417] add unit test
onewe May 20, 2022
6c5cdd7
[ISSUE #8417] repackage
onewe May 20, 2022
9c005b5
[ISSUE #8417] retry ci
onewe May 23, 2022
f86e444
[ISSUE #8417] retry ci
onewe May 23, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion console-ui/src/utils/request.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -89,7 +89,7 @@ const request = () => {

if (
[401, 403].includes(status) &&
['unknown user!', 'token invalid!', 'token expired!', 'authorization failed!'].includes(
['unknown user!', 'token invalid!', 'token expired!', 'session expired!'].includes(
message
)
) {
Expand Down
4 changes: 2 additions & 2 deletions console/src/main/resources/static/css/main.css
View file
Open in desktop

Large diffs are not rendered by default.

4 changes: 2 additions & 2 deletions console/src/main/resources/static/index.html
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,7 @@
<link rel="stylesheet" type="text/css" href="console-ui/public/css/icon.css">
<link rel="stylesheet" type="text/css" href="console-ui/public/css/font-awesome.css">
<!-- 第三方css结束 -->
<link href="./css/main.css?1cbbea1b0db3eec7912c" rel="stylesheet"></head>
<link href="./css/main.css?fc5d786f5bd050c52a07" rel="stylesheet"></head>

<body>
<div id="root" style="overflow:hidden"></div>
Expand All @@ -56,6 +56,6 @@
<script src="console-ui/public/js/merge.js"></script>
<script src="console-ui/public/js/loader.js"></script>
<!-- 第三方js结束 -->
<script type="text/javascript" src="./js/main.js?1cbbea1b0db3eec7912c"></script></body>
<script type="text/javascript" src="./js/main.js?fc5d786f5bd050c52a07"></script></body>

</html>
27 changes: 14 additions & 13 deletions console/src/main/resources/static/js/main.js
View file
Open in desktop

Large diffs are not rendered by default.

22 changes: 13 additions & 9 deletions ...ault-impl/src/main/java/com/alibaba/nacos/plugin/auth/impl/controller/UserController.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -43,6 +43,7 @@
import org.springframework.security.core.Authentication;
import org.springframework.security.core.context.SecurityContextHolder;
import org.springframework.security.core.userdetails.UserDetails;
import org.springframework.web.HttpSessionRequiredException;
import org.springframework.web.bind.annotation.DeleteMapping;
import org.springframework.web.bind.annotation.GetMapping;
import org.springframework.web.bind.annotation.PostMapping;
Expand All @@ -55,7 +56,6 @@
import javax.servlet.http.HttpServletResponse;
import java.io.IOException;
import java.util.List;
import java.util.Objects;

/**
* User related methods entry.
Expand Down Expand Up @@ -144,11 +144,16 @@ public Object deleteUser(@RequestParam String username) {
public Object updateUser(@RequestParam String username, @RequestParam String newPassword,
HttpServletResponse response, HttpServletRequest request) throws IOException {
// admin or same user
if (!hasPermission(username, request)) {
response.sendError(HttpServletResponse.SC_FORBIDDEN, "authorization failed!");
try {
if (!hasPermission(username, request)) {
response.sendError(HttpServletResponse.SC_FORBIDDEN, "authorization failed!");
return null;
}
} catch (HttpSessionRequiredException e) {
response.sendError(HttpServletResponse.SC_UNAUTHORIZED, "session expired!");
return null;
}

User user = userDetailsService.getUserFromDatabase(username);
if (user == null) {
throw new IllegalArgumentException("user " + username + " not exist!");
Expand All @@ -159,15 +164,14 @@ public Object updateUser(@RequestParam String username, @RequestParam String new
return RestResultUtils.success("update user ok!");
}

private boolean hasPermission(String username, HttpServletRequest request) {
private boolean hasPermission(String username, HttpServletRequest request) throws HttpSessionRequiredException {
if (!authConfigs.isAuthEnabled()) {
return true;
}
if (Objects.isNull(request.getSession().getAttribute(AuthConstants.NACOS_USER_KEY))) {
return false;
}

NacosUser user = (NacosUser) request.getSession().getAttribute(AuthConstants.NACOS_USER_KEY);
if (user == null) {
throw new HttpSessionRequiredException("session expired!");
}
// admin
if (user.isGlobalAdmin()) {
return true;
Expand Down
14 changes: 14 additions & 0 deletions ...-impl/src/test/java/com/alibaba/nacos/plugin/auth/impl/controller/UserControllerTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -24,20 +24,25 @@
import com.alibaba.nacos.plugin.auth.impl.constant.AuthSystemTypes;
import com.alibaba.nacos.plugin.auth.impl.users.NacosUser;
import com.fasterxml.jackson.databind.JsonNode;
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.web.MockHttpSession;

import javax.servlet.http.HttpServletRequest;
import javax.servlet.http.HttpServletResponse;
import java.io.IOException;
import java.lang.reflect.Field;
import java.util.Properties;

import static org.hamcrest.CoreMatchers.instanceOf;
import static org.junit.Assert.assertThat;
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)
Expand Down Expand Up @@ -93,6 +98,15 @@ 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);
Expand Down