From e30c32cb02e1f02f8bb862e1550f05a75253a6da Mon Sep 17 00:00:00 2001 From: Jialei Date: Fri, 5 Jan 2024 16:01:42 +0800 Subject: [PATCH] chore(controller): prevent access to deleted projects (#3112) --- .../security/JwtTokenFilter.java | 25 +++++++++++++++---- .../security/JwtTokenFilterTest.java | 20 +++++++++++++++ 2 files changed, 40 insertions(+), 5 deletions(-) diff --git a/server/controller/src/main/java/ai/starwhale/mlops/configuration/security/JwtTokenFilter.java b/server/controller/src/main/java/ai/starwhale/mlops/configuration/security/JwtTokenFilter.java index 918b3e247f..9046f4635d 100644 --- a/server/controller/src/main/java/ai/starwhale/mlops/configuration/security/JwtTokenFilter.java +++ b/server/controller/src/main/java/ai/starwhale/mlops/configuration/security/JwtTokenFilter.java @@ -26,6 +26,8 @@ import ai.starwhale.mlops.domain.user.bo.Role; import ai.starwhale.mlops.domain.user.bo.User; import ai.starwhale.mlops.exception.StarwhaleException; +import ai.starwhale.mlops.exception.SwNotFoundException; +import ai.starwhale.mlops.exception.SwNotFoundException.ResourceType; import ai.starwhale.mlops.exception.SwValidationException; import io.jsonwebtoken.Claims; import java.io.IOException; @@ -66,9 +68,13 @@ public JwtTokenFilter(JwtTokenUtil jwtTokenUtil, UserService userService, Projec } boolean allowAnonymous(HttpServletRequest request) { - var projects = getProjects(request); - // only for public project - return projects.stream().allMatch(p -> p.getPrivacy() == Project.Privacy.PUBLIC); + try { + var projects = getProjects(request); + // only for public project + return projects.stream().allMatch(p -> p.getPrivacy() == Project.Privacy.PUBLIC); + } catch (SwNotFoundException e) { + return false; + } } @Override @@ -120,6 +126,9 @@ protected void doFilterInternal(HttpServletRequest httpServletRequest, Set projects = getProjects(httpServletRequest); Set rolesOfUser = userService.getProjectsRolesOfUser(user, projects); roles.addAll(rolesOfUser); + } catch (SwNotFoundException e) { + error(httpServletResponse, HttpStatus.NOT_FOUND.value(), Code.validationException, e.getMessage()); + return; } catch (StarwhaleException e) { logger.error(e.getMessage()); } @@ -133,12 +142,18 @@ protected void doFilterInternal(HttpServletRequest httpServletRequest, } @NotNull - private Set getProjects(HttpServletRequest httpServletRequest) { + private Set getProjects(HttpServletRequest httpServletRequest) throws SwNotFoundException { @SuppressWarnings("unchecked") Set projects = ((Set) httpServletRequest .getAttribute(ProjectDetectionFilter.ATTRIBUTE_PROJECT)) .stream() - .map(projectService::findProject) + .map((String projectUrl) -> { + var p = projectService.findProject(projectUrl); + if (p.isDeleted()) { + throw new SwNotFoundException(ResourceType.PROJECT, "Project is deleted"); + } + return p; + }) .collect(Collectors.toSet()); return projects; } diff --git a/server/controller/src/test/java/ai/starwhale/mlops/configuration/security/JwtTokenFilterTest.java b/server/controller/src/test/java/ai/starwhale/mlops/configuration/security/JwtTokenFilterTest.java index b8719f3539..1e3368c60c 100644 --- a/server/controller/src/test/java/ai/starwhale/mlops/configuration/security/JwtTokenFilterTest.java +++ b/server/controller/src/test/java/ai/starwhale/mlops/configuration/security/JwtTokenFilterTest.java @@ -16,6 +16,7 @@ package ai.starwhale.mlops.configuration.security; +import static org.mockito.ArgumentMatchers.any; import static org.mockito.Mockito.doThrow; import static org.mockito.Mockito.mock; import static org.mockito.Mockito.mockStatic; @@ -28,6 +29,7 @@ import ai.starwhale.mlops.domain.project.ProjectService; import ai.starwhale.mlops.domain.project.bo.Project; import ai.starwhale.mlops.domain.user.UserService; +import ai.starwhale.mlops.domain.user.bo.User; import ai.starwhale.mlops.exception.SwValidationException; import ai.starwhale.mlops.exception.SwValidationException.ValidSubject; import io.jsonwebtoken.impl.DefaultClaims; @@ -65,6 +67,7 @@ public void setup() { when(jwtTokenUtil.parseJwt("y")).thenThrow(new SwValidationException(ValidSubject.USER)); DefaultClaims claims = new DefaultClaims(Map.of("taskId", "x")); when(jwtTokenUtil.parseJwt("a")).thenReturn(claims); + userService = mock(UserService.class); JwtClaimValidator jwtClaimValidator = mock(JwtClaimValidator.class); doThrow(SwValidationException.class).when(jwtClaimValidator).validClaims(claims); jwtClaimValidators = List.of(jwtClaimValidator); @@ -119,4 +122,21 @@ public void testAnonymous(Project.Privacy privacy, int errorTimes) throws Servle "Not logged in."), times(errorTimes)); } + @Test + public void testDeletedProject() throws ServletException, IOException { + when(projectService.findProject("deleted")) + .thenReturn(Project.builder().isDeleted(true).build()); + HttpServletRequest request = mock(HttpServletRequest.class); + when(request.getHeader("Authorization")).thenReturn("Bearer a"); + when(request.getAttribute("PROJECT")).thenReturn(Set.of("deleted")); + HttpServletResponse response = mock(HttpServletResponse.class); + FilterChain filterchain = mock(FilterChain.class); + when(jwtTokenUtil.getUsername(any())).thenReturn("foo"); + when(userService.loadUserByUsername("foo")).thenReturn(mock(User.class)); + jwtTokenFilter = new JwtTokenFilter(jwtTokenUtil, userService, projectService, List.of()); + jwtTokenFilter.doFilterInternal(request, response, filterchain); + httpUtilMockedStatic.verify( + () -> HttpUtil.error(response, HttpStatus.NOT_FOUND.value(), Code.validationException, + "Resource is not found Project\nProject is deleted"), times(1)); + } }