From 44edb48771a76e1c817035e6ed05434b23a4739c Mon Sep 17 00:00:00 2001 From: Konstantin Schaper Date: Thu, 20 Aug 2020 17:44:36 +0200 Subject: [PATCH 01/18] initial implementation --- .../scm/web/filter/AuthenticationFilter.java | 11 +-- scm-ui/ui-components/src/apiclient.ts | 7 +- .../lifecycle/modules/ScmSecurityModule.java | 9 ++ .../DefaultAccessTokenCookieIssuer.java | 11 +-- .../ScmAtLeastOneSuccessfulStrategy.java | 91 +++++++++++++++++++ .../scm/security/TokenExpiredFilter.java | 90 ++++++++++++++++++ 6 files changed, 205 insertions(+), 14 deletions(-) create mode 100644 scm-webapp/src/main/java/sonia/scm/security/ScmAtLeastOneSuccessfulStrategy.java create mode 100644 scm-webapp/src/main/java/sonia/scm/security/TokenExpiredFilter.java diff --git a/scm-core/src/main/java/sonia/scm/web/filter/AuthenticationFilter.java b/scm-core/src/main/java/sonia/scm/web/filter/AuthenticationFilter.java index 04ffe12e21..a6514465fd 100644 --- a/scm-core/src/main/java/sonia/scm/web/filter/AuthenticationFilter.java +++ b/scm-core/src/main/java/sonia/scm/web/filter/AuthenticationFilter.java @@ -67,6 +67,7 @@ public class AuthenticationFilter extends HttpFilter { */ private static final String ATTRIBUTE_FAILED_AUTH = "sonia.scm.auth.failed"; + private final Set tokenGenerators; protected ScmConfiguration configuration; @@ -117,7 +118,7 @@ public class AuthenticationFilter extends HttpFilter { } /** - * Sends status code 403 back to client, if the authentication has failed. + * Sends status code 401 back to client, if the authentication has failed. * In all other cases the method will send status code 403 back to client. * * @param request servlet request @@ -209,12 +210,8 @@ public class AuthenticationFilter extends HttpFilter { subject.login(token); processChain(request, response, chain, subject); } catch (TokenExpiredException ex) { - if (logger.isTraceEnabled()) { - logger.trace("{} expired", token.getClass(), ex); - } else { - logger.debug("{} expired", token.getClass()); - } - handleUnauthorized(request, response, chain); + // Rethrow to be caught by TokenExpiredFilter + throw ex; } catch (AuthenticationException ex) { logger.warn("authentication failed", ex); handleUnauthorized(request, response, chain); diff --git a/scm-ui/ui-components/src/apiclient.ts b/scm-ui/ui-components/src/apiclient.ts index 346b02845d..daabeeb122 100644 --- a/scm-ui/ui-components/src/apiclient.ts +++ b/scm-ui/ui-components/src/apiclient.ts @@ -120,7 +120,12 @@ function handleFailure(response: Response) { if (!response.ok) { if (isBackendError(response)) { return response.json().then((content: BackendErrorContent) => { - throw createBackendError(content, response.status); + if (content.errorCode === "DDS8D8unr1") { + window.location.replace(`${contextPath}/login`); + throw new UnauthorizedError("Unauthorized", 401); + } else { + throw createBackendError(content, response.status); + } }); } else { if (response.status === 401) { diff --git a/scm-webapp/src/main/java/sonia/scm/lifecycle/modules/ScmSecurityModule.java b/scm-webapp/src/main/java/sonia/scm/lifecycle/modules/ScmSecurityModule.java index 36fd0bc363..cfa5b10f6e 100644 --- a/scm-webapp/src/main/java/sonia/scm/lifecycle/modules/ScmSecurityModule.java +++ b/scm-webapp/src/main/java/sonia/scm/lifecycle/modules/ScmSecurityModule.java @@ -28,8 +28,11 @@ package sonia.scm.lifecycle.modules; import com.google.inject.name.Names; +import org.apache.shiro.authc.Authenticator; import org.apache.shiro.authc.credential.DefaultPasswordService; import org.apache.shiro.authc.credential.PasswordService; +import org.apache.shiro.authc.pam.AuthenticationStrategy; +import org.apache.shiro.authc.pam.ModularRealmAuthenticator; import org.apache.shiro.crypto.hash.DefaultHashService; import org.apache.shiro.guice.web.ShiroWebModule; import org.apache.shiro.realm.Realm; @@ -44,6 +47,7 @@ import sonia.scm.plugin.ExtensionProcessor; import javax.servlet.ServletContext; import org.apache.shiro.mgt.RememberMeManager; import sonia.scm.security.DisabledRememberMeManager; +import sonia.scm.security.ScmAtLeastOneSuccessfulStrategy; /** * @@ -94,6 +98,11 @@ public class ScmSecurityModule extends ShiroWebModule // disable remember me cookie generation bind(RememberMeManager.class).to(DisabledRememberMeManager.class); + // bind authentication strategy + bind(ModularRealmAuthenticator.class); + bind(Authenticator.class).to(ModularRealmAuthenticator.class); + bind(AuthenticationStrategy.class).to(ScmAtLeastOneSuccessfulStrategy.class); + // bind realm for (Class realm : extensionProcessor.byExtensionPoint(Realm.class)) { diff --git a/scm-webapp/src/main/java/sonia/scm/security/DefaultAccessTokenCookieIssuer.java b/scm-webapp/src/main/java/sonia/scm/security/DefaultAccessTokenCookieIssuer.java index cea0770c01..73d9e7f3a7 100644 --- a/scm-webapp/src/main/java/sonia/scm/security/DefaultAccessTokenCookieIssuer.java +++ b/scm-webapp/src/main/java/sonia/scm/security/DefaultAccessTokenCookieIssuer.java @@ -51,6 +51,10 @@ public final class DefaultAccessTokenCookieIssuer implements AccessTokenCookieIs * the logger for DefaultAccessTokenCookieIssuer */ private static final Logger LOG = LoggerFactory.getLogger(DefaultAccessTokenCookieIssuer.class); + + private static final int DEFAULT_COOKIE_EXPIRATION_AMOUNT = 24; + private static final TimeUnit DEFAULT_COOKIE_EXPIRATION_UNIT = TimeUnit.HOURS; + private static final int DEFAULT_COOKIE_EXPIRATION = (int) TimeUnit.SECONDS.convert(DEFAULT_COOKIE_EXPIRATION_AMOUNT, DEFAULT_COOKIE_EXPIRATION_UNIT); private final ScmConfiguration configuration; @@ -75,7 +79,7 @@ public final class DefaultAccessTokenCookieIssuer implements AccessTokenCookieIs LOG.trace("create and attach cookie for access token {}", accessToken.getId()); Cookie c = new Cookie(HttpUtil.COOKIE_BEARER_AUTHENTICATION, accessToken.compact()); c.setPath(contextPath(request)); - c.setMaxAge(getMaxAge(accessToken)); + c.setMaxAge(DEFAULT_COOKIE_EXPIRATION); c.setHttpOnly(isHttpOnly()); c.setSecure(isSecure(request)); @@ -111,11 +115,6 @@ public final class DefaultAccessTokenCookieIssuer implements AccessTokenCookieIs return contextPath; } - private int getMaxAge(AccessToken accessToken){ - long maxAgeMs = accessToken.getExpiration().getTime() - new Date().getTime(); - return (int) TimeUnit.MILLISECONDS.toSeconds(maxAgeMs); - } - private boolean isSecure(HttpServletRequest request){ boolean secure = request.isSecure(); if (!secure) { diff --git a/scm-webapp/src/main/java/sonia/scm/security/ScmAtLeastOneSuccessfulStrategy.java b/scm-webapp/src/main/java/sonia/scm/security/ScmAtLeastOneSuccessfulStrategy.java new file mode 100644 index 0000000000..9a0931dfd2 --- /dev/null +++ b/scm-webapp/src/main/java/sonia/scm/security/ScmAtLeastOneSuccessfulStrategy.java @@ -0,0 +1,91 @@ +/* + * 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.pam.AbstractAuthenticationStrategy; +import org.apache.shiro.realm.Realm; +import org.apache.shiro.subject.PrincipalCollection; + +import java.util.ArrayList; +import java.util.Collection; +import java.util.List; +import java.util.Optional; + +public class ScmAtLeastOneSuccessfulStrategy extends AbstractAuthenticationStrategy { + + private final ThreadLocal> threadLocal = new ThreadLocal<>(); + + @Override + public AuthenticationInfo beforeAllAttempts(Collection realms, AuthenticationToken token) throws AuthenticationException { + this.threadLocal.set(new ArrayList<>()); + return super.beforeAllAttempts(realms, token); + } + + @Override + public AuthenticationInfo afterAttempt(Realm realm, AuthenticationToken token, AuthenticationInfo singleRealmInfo, AuthenticationInfo aggregateInfo, Throwable t) throws AuthenticationException { + if (t != null) { + this.threadLocal.get().add(t); + } + return super.afterAttempt(realm, token, singleRealmInfo, aggregateInfo, t); + } + + @Override + public AuthenticationInfo afterAllAttempts(AuthenticationToken token, AuthenticationInfo aggregate) throws AuthenticationException { + final List throwables = threadLocal.get(); + threadLocal.remove(); + if (isAuthenticationSuccessful(aggregate)) { + return aggregate; + } + Optional tokenExpiredException = findTokenExpiredException(throwables); + + if (tokenExpiredException.isPresent()) { + throw tokenExpiredException.get(); + } else { + throw createAuthenticationException(token); + } + } + + private static boolean isAuthenticationSuccessful(AuthenticationInfo aggregate) { + return aggregate != null && isNotEmpty(aggregate.getPrincipals()); + } + + private static boolean isNotEmpty(PrincipalCollection pc) { + return pc != null && !pc.isEmpty(); + } + + private static Optional findTokenExpiredException(List throwables) { + return throwables.stream().filter(t -> t instanceof TokenExpiredException).findFirst().map(t -> (TokenExpiredException) t); + } + + private static AuthenticationException createAuthenticationException(AuthenticationToken token) { + return new AuthenticationException("Authentication token of type [" + token.getClass() + "] " + + "could not be authenticated by any configured realms. Please ensure that at least one realm can " + + "authenticate these tokens."); + } + +} diff --git a/scm-webapp/src/main/java/sonia/scm/security/TokenExpiredFilter.java b/scm-webapp/src/main/java/sonia/scm/security/TokenExpiredFilter.java new file mode 100644 index 0000000000..759ba97e0f --- /dev/null +++ b/scm-webapp/src/main/java/sonia/scm/security/TokenExpiredFilter.java @@ -0,0 +1,90 @@ +/* + * 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 com.google.inject.Inject; +import com.google.inject.Singleton; +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; +import org.slf4j.MDC; +import sonia.scm.Priority; +import sonia.scm.api.v2.resources.ErrorDto; +import sonia.scm.filter.Filters; +import sonia.scm.filter.WebElement; +import sonia.scm.web.VndMediaType; +import sonia.scm.web.filter.HttpFilter; + +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; + +@WebElement("/*") +@Priority(Filters.PRIORITY_PRE_AUTHENTICATION) +@Singleton +public class TokenExpiredFilter extends HttpFilter { + private static final String TOKEN_EXPIRED_ERROR_CODE = "DDS8D8unr1"; + private static final Logger LOG = LoggerFactory.getLogger(TokenExpiredFilter.class); + + private final AccessTokenCookieIssuer accessTokenCookieIssuer; + private final ObjectMapper objectMapper; + + @Inject + public TokenExpiredFilter(AccessTokenCookieIssuer accessTokenCookieIssuer, ObjectMapper objectMapper) { + this.accessTokenCookieIssuer = accessTokenCookieIssuer; + this.objectMapper = objectMapper; + } + + @Override + protected void doFilter(HttpServletRequest request, HttpServletResponse response, FilterChain chain) throws IOException, ServletException { + try { + chain.doFilter(request, response); + } catch (TokenExpiredException ex) { + if (LOG.isTraceEnabled()) { + LOG.trace("Token expired", ex); + } else { + LOG.debug("Token expired"); + } + handleTokenExpired(request, response); + } + } + + protected void handleTokenExpired(HttpServletRequest request, + HttpServletResponse response) throws IOException { + accessTokenCookieIssuer.invalidate(request, response); + response.setStatus(HttpServletResponse.SC_UNAUTHORIZED); + response.setContentType(VndMediaType.ERROR_TYPE); + final ErrorDto errorDto = new ErrorDto(); + errorDto.setMessage("Token Expired"); + errorDto.setErrorCode(TOKEN_EXPIRED_ERROR_CODE); + errorDto.setTransactionId(MDC.get("transaction_id")); + try (ServletOutputStream stream = response.getOutputStream()) { + objectMapper.writeValue(stream, errorDto); + } + } +} From 34dc7fdac0c85f62d82d5ec1466ead8f302b9551 Mon Sep 17 00:00:00 2001 From: Konstantin Schaper Date: Thu, 20 Aug 2020 18:57:58 +0200 Subject: [PATCH 02/18] 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; + })); + } + +} From 3d0c55fd18d4044b4bcaa99a59f4916d4adf5a45 Mon Sep 17 00:00:00 2001 From: Konstantin Schaper Date: Fri, 21 Aug 2020 11:38:59 +0200 Subject: [PATCH 03/18] fix tests and adjust cookie duration --- .../sonia/scm/security/DefaultAccessTokenCookieIssuer.java | 4 ++-- .../scm/api/v2/resources/AuthenticationResourceTest.java | 1 - .../scm/security/DefaultAccessTokenCookieIssuerTest.java | 2 -- 3 files changed, 2 insertions(+), 5 deletions(-) diff --git a/scm-webapp/src/main/java/sonia/scm/security/DefaultAccessTokenCookieIssuer.java b/scm-webapp/src/main/java/sonia/scm/security/DefaultAccessTokenCookieIssuer.java index 73d9e7f3a7..2a7f6b1edd 100644 --- a/scm-webapp/src/main/java/sonia/scm/security/DefaultAccessTokenCookieIssuer.java +++ b/scm-webapp/src/main/java/sonia/scm/security/DefaultAccessTokenCookieIssuer.java @@ -52,8 +52,8 @@ public final class DefaultAccessTokenCookieIssuer implements AccessTokenCookieIs */ private static final Logger LOG = LoggerFactory.getLogger(DefaultAccessTokenCookieIssuer.class); - private static final int DEFAULT_COOKIE_EXPIRATION_AMOUNT = 24; - private static final TimeUnit DEFAULT_COOKIE_EXPIRATION_UNIT = TimeUnit.HOURS; + private static final int DEFAULT_COOKIE_EXPIRATION_AMOUNT = 365; + private static final TimeUnit DEFAULT_COOKIE_EXPIRATION_UNIT = TimeUnit.DAYS; private static final int DEFAULT_COOKIE_EXPIRATION = (int) TimeUnit.SECONDS.convert(DEFAULT_COOKIE_EXPIRATION_AMOUNT, DEFAULT_COOKIE_EXPIRATION_UNIT); private final ScmConfiguration configuration; diff --git a/scm-webapp/src/test/java/sonia/scm/api/v2/resources/AuthenticationResourceTest.java b/scm-webapp/src/test/java/sonia/scm/api/v2/resources/AuthenticationResourceTest.java index 92ab7b1209..2fe891e4cd 100644 --- a/scm-webapp/src/test/java/sonia/scm/api/v2/resources/AuthenticationResourceTest.java +++ b/scm-webapp/src/test/java/sonia/scm/api/v2/resources/AuthenticationResourceTest.java @@ -139,7 +139,6 @@ public class AuthenticationResourceTest { dispatcher.addSingletonResource(authenticationResource); AccessToken accessToken = mock(AccessToken.class); - when(accessToken.getExpiration()).thenReturn(new Date(Long.MAX_VALUE)); when(accessTokenBuilder.build()).thenReturn(accessToken); when(accessTokenBuilderFactory.create()).thenReturn(accessTokenBuilder); diff --git a/scm-webapp/src/test/java/sonia/scm/security/DefaultAccessTokenCookieIssuerTest.java b/scm-webapp/src/test/java/sonia/scm/security/DefaultAccessTokenCookieIssuerTest.java index 926022af55..1a759ad75c 100644 --- a/scm-webapp/src/test/java/sonia/scm/security/DefaultAccessTokenCookieIssuerTest.java +++ b/scm-webapp/src/test/java/sonia/scm/security/DefaultAccessTokenCookieIssuerTest.java @@ -121,8 +121,6 @@ public class DefaultAccessTokenCookieIssuerTest { } private Cookie authenticate() { - when(accessToken.getExpiration()).thenReturn(new Date()); - issuer.authenticate(request, response, accessToken); verify(response).addCookie(cookieArgumentCaptor.capture()); From a25312278bee5f9f829f97e717c208e3839a07b7 Mon Sep 17 00:00:00 2001 From: Konstantin Schaper Date: Fri, 21 Aug 2020 11:47:31 +0200 Subject: [PATCH 04/18] update changelog --- CHANGELOG.md | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 8d8fd79db9..de9cd21a47 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -4,6 +4,10 @@ All notable changes to this project will be documented in this file. The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/), and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.html). +## Unreleased +### Fixed +- JWT token timeout is now handled properly ([#1297](https://github.com/scm-manager/scm-manager/pull/1297)) + ## [2.4.0] - 2020-08-14 ### Added - Introduced merge detection for receive hooks ([#1278](https://github.com/scm-manager/scm-manager/pull/1278)) From 81d4c2f64f7d78c169a4f7077cb39d73cca06d6b Mon Sep 17 00:00:00 2001 From: Florian Scholdei Date: Mon, 24 Aug 2020 11:29:27 +0200 Subject: [PATCH 05/18] Fix text-overflow in danger zone --- .../ui-webapp/src/repos/containers/DangerZone.tsx | 13 ++++++++++++- .../ui-webapp/src/repos/containers/DeleteRepo.tsx | 10 +++++----- .../src/repos/containers/RenameRepository.tsx | 9 +++++---- 3 files changed, 22 insertions(+), 10 deletions(-) diff --git a/scm-ui/ui-webapp/src/repos/containers/DangerZone.tsx b/scm-ui/ui-webapp/src/repos/containers/DangerZone.tsx index 5fb14e4557..3844717487 100644 --- a/scm-ui/ui-webapp/src/repos/containers/DangerZone.tsx +++ b/scm-ui/ui-webapp/src/repos/containers/DangerZone.tsx @@ -36,9 +36,20 @@ type Props = { }; const DangerZoneContainer = styled.div` - padding: 1rem; + padding: 1.5rem 1rem; border: 1px solid #ff6a88; border-radius: 5px; + > .level { + flex-flow: wrap; + + .level-left { + max-width: 100%; + } + + .level-right { + margin-top: 0.75rem; + } + } > *:not(:last-child) { padding-bottom: 1.5rem; border-bottom: solid 2px whitesmoke; diff --git a/scm-ui/ui-webapp/src/repos/containers/DeleteRepo.tsx b/scm-ui/ui-webapp/src/repos/containers/DeleteRepo.tsx index 8585e98466..23f6ea10a5 100644 --- a/scm-ui/ui-webapp/src/repos/containers/DeleteRepo.tsx +++ b/scm-ui/ui-webapp/src/repos/containers/DeleteRepo.tsx @@ -26,9 +26,8 @@ import { connect } from "react-redux"; import { compose } from "redux"; import { RouteComponentProps, withRouter } from "react-router-dom"; import { WithTranslation, withTranslation } from "react-i18next"; -import { History } from "history"; import { Repository } from "@scm-manager/ui-types"; -import { confirmAlert, DeleteButton, ErrorNotification, Level, ButtonGroup } from "@scm-manager/ui-components"; +import { confirmAlert, DeleteButton, ErrorNotification, Level } from "@scm-manager/ui-components"; import { deleteRepo, getDeleteRepoFailure, isDeleteRepoPending } from "../modules/repos"; type Props = RouteComponentProps & @@ -89,10 +88,11 @@ class DeleteRepo extends React.Component { +

{t("deleteRepo.subtitle")} -

{t("deleteRepo.description")}

- +
+ {t("deleteRepo.description")} +

} right={} /> diff --git a/scm-ui/ui-webapp/src/repos/containers/RenameRepository.tsx b/scm-ui/ui-webapp/src/repos/containers/RenameRepository.tsx index ede6414029..9254774a6d 100644 --- a/scm-ui/ui-webapp/src/repos/containers/RenameRepository.tsx +++ b/scm-ui/ui-webapp/src/repos/containers/RenameRepository.tsx @@ -38,7 +38,7 @@ type Props = { }; const RenameRepository: FC = ({ repository, indexLinks }) => { - let history = useHistory(); + const history = useHistory(); const [t] = useTranslation("repos"); const [error, setError] = useState(undefined); const [loading, setLoading] = useState(false); @@ -156,10 +156,11 @@ const RenameRepository: FC = ({ repository, indexLinks }) => { /> +

{t("renameRepo.subtitle")} -

{t("renameRepo.description")}

- +
+ {t("renameRepo.description")} +

} right={