From 34dc7fdac0c85f62d82d5ec1466ead8f302b9551 Mon Sep 17 00:00:00 2001 From: Konstantin Schaper Date: Thu, 20 Aug 2020 18:57:58 +0200 Subject: [PATCH] cleanup code & add tests --- scm-ui/ui-components/src/apiclient.ts | 12 +- scm-ui/ui-components/src/errors.ts | 2 + .../ScmAtLeastOneSuccessfulStrategy.java | 2 +- .../scm/security/TokenExpiredFilter.java | 2 +- .../ScmAtLeastOneSuccessfulStrategyTest.java | 107 ++++++++++++++++++ .../scm/security/TokenExpiredFilterTest.java | 80 +++++++++++++ 6 files changed, 201 insertions(+), 4 deletions(-) create mode 100644 scm-webapp/src/test/java/sonia/scm/security/ScmAtLeastOneSuccessfulStrategyTest.java create mode 100644 scm-webapp/src/test/java/sonia/scm/security/TokenExpiredFilterTest.java diff --git a/scm-ui/ui-components/src/apiclient.ts b/scm-ui/ui-components/src/apiclient.ts index daabeeb122..a371b06571 100644 --- a/scm-ui/ui-components/src/apiclient.ts +++ b/scm-ui/ui-components/src/apiclient.ts @@ -23,7 +23,14 @@ */ import { contextPath } from "./urls"; -import { createBackendError, ForbiddenError, isBackendError, UnauthorizedError, BackendErrorContent } from "./errors"; +import { + createBackendError, + ForbiddenError, + isBackendError, + UnauthorizedError, + BackendErrorContent, + TOKEN_EXPIRED_ERROR_CODE +} from "./errors"; type SubscriptionEvent = { type: string; @@ -120,8 +127,9 @@ function handleFailure(response: Response) { if (!response.ok) { if (isBackendError(response)) { return response.json().then((content: BackendErrorContent) => { - if (content.errorCode === "DDS8D8unr1") { + if (content.errorCode === TOKEN_EXPIRED_ERROR_CODE) { window.location.replace(`${contextPath}/login`); + // Throw error because if redirect is not instantaneous, we want to display something senseful throw new UnauthorizedError("Unauthorized", 401); } else { throw createBackendError(content, response.status); diff --git a/scm-ui/ui-components/src/errors.ts b/scm-ui/ui-components/src/errors.ts index 2d4757bc72..bf1f364e67 100644 --- a/scm-ui/ui-components/src/errors.ts +++ b/scm-ui/ui-components/src/errors.ts @@ -103,3 +103,5 @@ export function createBackendError(content: BackendErrorContent, statusCode: num export function isBackendError(response: Response) { return response.headers.get("Content-Type") === "application/vnd.scmm-error+json;v=2"; } + +export const TOKEN_EXPIRED_ERROR_CODE = "DDS8D8unr1"; diff --git a/scm-webapp/src/main/java/sonia/scm/security/ScmAtLeastOneSuccessfulStrategy.java b/scm-webapp/src/main/java/sonia/scm/security/ScmAtLeastOneSuccessfulStrategy.java index 9a0931dfd2..98306e077a 100644 --- a/scm-webapp/src/main/java/sonia/scm/security/ScmAtLeastOneSuccessfulStrategy.java +++ b/scm-webapp/src/main/java/sonia/scm/security/ScmAtLeastOneSuccessfulStrategy.java @@ -38,7 +38,7 @@ import java.util.Optional; public class ScmAtLeastOneSuccessfulStrategy extends AbstractAuthenticationStrategy { - private final ThreadLocal> threadLocal = new ThreadLocal<>(); + final ThreadLocal> threadLocal = new ThreadLocal<>(); @Override public AuthenticationInfo beforeAllAttempts(Collection realms, AuthenticationToken token) throws AuthenticationException { diff --git a/scm-webapp/src/main/java/sonia/scm/security/TokenExpiredFilter.java b/scm-webapp/src/main/java/sonia/scm/security/TokenExpiredFilter.java index 759ba97e0f..1ca31a9ecf 100644 --- a/scm-webapp/src/main/java/sonia/scm/security/TokenExpiredFilter.java +++ b/scm-webapp/src/main/java/sonia/scm/security/TokenExpiredFilter.java @@ -48,7 +48,7 @@ import java.io.IOException; @Priority(Filters.PRIORITY_PRE_AUTHENTICATION) @Singleton public class TokenExpiredFilter extends HttpFilter { - private static final String TOKEN_EXPIRED_ERROR_CODE = "DDS8D8unr1"; + static final String TOKEN_EXPIRED_ERROR_CODE = "DDS8D8unr1"; private static final Logger LOG = LoggerFactory.getLogger(TokenExpiredFilter.class); private final AccessTokenCookieIssuer accessTokenCookieIssuer; diff --git a/scm-webapp/src/test/java/sonia/scm/security/ScmAtLeastOneSuccessfulStrategyTest.java b/scm-webapp/src/test/java/sonia/scm/security/ScmAtLeastOneSuccessfulStrategyTest.java new file mode 100644 index 0000000000..fde6a9c9f9 --- /dev/null +++ b/scm-webapp/src/test/java/sonia/scm/security/ScmAtLeastOneSuccessfulStrategyTest.java @@ -0,0 +1,107 @@ +/* + * MIT License + * + * Copyright (c) 2020-present Cloudogu GmbH and Contributors + * + * Permission is hereby granted, free of charge, to any person obtaining a copy + * of this software and associated documentation files (the "Software"), to deal + * in the Software without restriction, including without limitation the rights + * to use, copy, modify, merge, publish, distribute, sublicense, and/or sell + * copies of the Software, and to permit persons to whom the Software is + * furnished to do so, subject to the following conditions: + * + * The above copyright notice and this permission notice shall be included in all + * copies or substantial portions of the Software. + * + * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR + * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, + * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE + * AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER + * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, + * OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE + * SOFTWARE. + */ + +package sonia.scm.security; + +import org.apache.shiro.authc.AuthenticationException; +import org.apache.shiro.authc.AuthenticationInfo; +import org.apache.shiro.authc.AuthenticationToken; +import org.apache.shiro.authc.MergableAuthenticationInfo; +import org.apache.shiro.realm.Realm; +import org.apache.shiro.subject.PrincipalCollection; +import org.junit.Test; +import org.junit.runner.RunWith; +import org.mockito.Mock; +import org.mockito.junit.MockitoJUnitRunner; + +import java.util.ArrayList; + +import static java.util.Collections.singletonList; +import static org.assertj.core.api.Assertions.assertThat; +import static org.mockito.Mockito.when; + +@RunWith(MockitoJUnitRunner.class) +public class ScmAtLeastOneSuccessfulStrategyTest { + + @Mock + private Realm realm; + + @Mock + private AuthenticationToken token; + + @Mock + MergableAuthenticationInfo singleRealmInfo; + + @Mock + MergableAuthenticationInfo aggregateInfo; + + @Mock + TokenExpiredException tokenExpiredException; + + @Mock + AuthenticationException authenticationException; + + @Mock + PrincipalCollection principalCollection; + + @Test + public void shouldAddNonNullThrowableToList() { + final ScmAtLeastOneSuccessfulStrategy strategy = new ScmAtLeastOneSuccessfulStrategy(); + strategy.threadLocal.set(new ArrayList<>()); + + strategy.afterAttempt(realm, token, singleRealmInfo, aggregateInfo, tokenExpiredException); + + assertThat(strategy.threadLocal.get()).hasSize(1); + assertThat(strategy.threadLocal.get().get(0)).isEqualTo(tokenExpiredException); + } + + @Test(expected = TokenExpiredException.class) + public void shouldRethrowException() { + final ScmAtLeastOneSuccessfulStrategy strategy = new ScmAtLeastOneSuccessfulStrategy(); + strategy.threadLocal.set(singletonList(tokenExpiredException)); + + strategy.afterAllAttempts(token, aggregateInfo); + } + + @Test(expected = AuthenticationException.class) + public void shouldThrowGenericErrorIfNonTokenExpiredExceptionWasCaught() { + final ScmAtLeastOneSuccessfulStrategy strategy = new ScmAtLeastOneSuccessfulStrategy(); + strategy.threadLocal.set(singletonList(authenticationException)); + + strategy.afterAllAttempts(token, aggregateInfo); + } + + @Test() + public void shouldNotRethrowExceptionIfAuthenticationSuccessful() { + final ScmAtLeastOneSuccessfulStrategy strategy = new ScmAtLeastOneSuccessfulStrategy(); + strategy.threadLocal.set(singletonList(tokenExpiredException)); + when(aggregateInfo.getPrincipals()).thenReturn(principalCollection); + when(principalCollection.isEmpty()).thenReturn(false); + + final AuthenticationInfo authenticationInfo = strategy.afterAllAttempts(token, aggregateInfo); + + assertThat(authenticationInfo).isNotNull(); + } + +} diff --git a/scm-webapp/src/test/java/sonia/scm/security/TokenExpiredFilterTest.java b/scm-webapp/src/test/java/sonia/scm/security/TokenExpiredFilterTest.java new file mode 100644 index 0000000000..197ef1c3ab --- /dev/null +++ b/scm-webapp/src/test/java/sonia/scm/security/TokenExpiredFilterTest.java @@ -0,0 +1,80 @@ +/* + * MIT License + * + * Copyright (c) 2020-present Cloudogu GmbH and Contributors + * + * Permission is hereby granted, free of charge, to any person obtaining a copy + * of this software and associated documentation files (the "Software"), to deal + * in the Software without restriction, including without limitation the rights + * to use, copy, modify, merge, publish, distribute, sublicense, and/or sell + * copies of the Software, and to permit persons to whom the Software is + * furnished to do so, subject to the following conditions: + * + * The above copyright notice and this permission notice shall be included in all + * copies or substantial portions of the Software. + * + * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR + * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, + * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE + * AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER + * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, + * OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE + * SOFTWARE. + */ + +package sonia.scm.security; + +import com.fasterxml.jackson.databind.ObjectMapper; +import org.junit.Test; +import org.junit.runner.RunWith; +import org.mockito.Mock; +import org.mockito.junit.MockitoJUnitRunner; +import sonia.scm.api.v2.resources.ErrorDto; +import sonia.scm.web.VndMediaType; + +import javax.servlet.FilterChain; +import javax.servlet.ServletException; +import javax.servlet.ServletOutputStream; +import javax.servlet.http.HttpServletRequest; +import javax.servlet.http.HttpServletResponse; +import java.io.IOException; + +import static org.junit.Assert.assertEquals; +import static org.mockito.ArgumentMatchers.any; +import static org.mockito.ArgumentMatchers.argThat; +import static org.mockito.Mockito.atLeastOnce; +import static org.mockito.Mockito.doThrow; +import static org.mockito.Mockito.verify; +import static sonia.scm.security.TokenExpiredFilter.TOKEN_EXPIRED_ERROR_CODE; + +@RunWith(MockitoJUnitRunner.class) +public class TokenExpiredFilterTest { + + @Mock + private FilterChain chain; + @Mock + private HttpServletRequest request; + @Mock + private HttpServletResponse response; + @Mock + private ObjectMapper objectMapper; + @Mock + private AccessTokenCookieIssuer accessTokenCookieIssuer; + + @Test + public void shouldReturnSpecificErrorResponseAndInvalidateCookie() throws IOException, ServletException { + final TokenExpiredFilter filter = new TokenExpiredFilter(accessTokenCookieIssuer, objectMapper); + doThrow(TokenExpiredException.class).when(chain).doFilter(request, response); + + filter.doFilter(request, response, chain); + + verify(chain, atLeastOnce()).doFilter(request, response); + verify(accessTokenCookieIssuer, atLeastOnce()).invalidate(request, response); + verify(response, atLeastOnce()).setContentType(VndMediaType.ERROR_TYPE); + verify(objectMapper).writeValue((ServletOutputStream) any(), argThat((ErrorDto errorDto) -> { + assertEquals(TOKEN_EXPIRED_ERROR_CODE, errorDto.getErrorCode()); + return true; + })); + } + +}