From c0532282210a52fa2c973770f58c481ae81b550b Mon Sep 17 00:00:00 2001 From: Konstantin Schaper Date: Fri, 28 Aug 2020 13:26:19 +0200 Subject: [PATCH] Fix token expired exception being displayed in browser --- .../scm/web/filter/AuthenticationFilter.java | 7 +++++- ...otocolServletAuthenticationFilterBase.java | 12 ++++++++++ ...olServletAuthenticationFilterBaseTest.java | 23 ++++++++++++++++++- 3 files changed, 40 insertions(+), 2 deletions(-) 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 a6514465fd..30e8df9fe7 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 @@ -166,6 +166,11 @@ public class AuthenticationFilter extends HttpFilter { HttpUtil.sendUnauthorized(request, response, configuration.getRealmDescription()); } + protected void handleTokenExpiredException(HttpServletRequest request, HttpServletResponse response, + FilterChain chain, TokenExpiredException tokenExpiredException) throws IOException, ServletException { + throw tokenExpiredException; + } + /** * Iterates all {@link WebTokenGenerator} and creates an * {@link AuthenticationToken} from the given request. @@ -211,7 +216,7 @@ public class AuthenticationFilter extends HttpFilter { processChain(request, response, chain, subject); } catch (TokenExpiredException ex) { // Rethrow to be caught by TokenExpiredFilter - throw ex; + handleTokenExpiredException(request, response, chain, ex); } catch (AuthenticationException ex) { logger.warn("authentication failed", ex); handleUnauthorized(request, response, chain); diff --git a/scm-core/src/main/java/sonia/scm/web/filter/HttpProtocolServletAuthenticationFilterBase.java b/scm-core/src/main/java/sonia/scm/web/filter/HttpProtocolServletAuthenticationFilterBase.java index 08f845740c..e6716f0013 100644 --- a/scm-core/src/main/java/sonia/scm/web/filter/HttpProtocolServletAuthenticationFilterBase.java +++ b/scm-core/src/main/java/sonia/scm/web/filter/HttpProtocolServletAuthenticationFilterBase.java @@ -25,6 +25,7 @@ package sonia.scm.web.filter; import sonia.scm.config.ScmConfiguration; +import sonia.scm.security.TokenExpiredException; import sonia.scm.util.HttpUtil; import sonia.scm.web.UserAgent; import sonia.scm.web.UserAgentParser; @@ -59,4 +60,15 @@ public class HttpProtocolServletAuthenticationFilterBase extends AuthenticationF HttpUtil.sendUnauthorized(request, response); } } + + @Override + protected void handleTokenExpiredException(HttpServletRequest request, HttpServletResponse response, FilterChain chain, TokenExpiredException tokenExpiredException) throws IOException, ServletException { + UserAgent userAgent = userAgentParser.parse(request); + if (userAgent.isBrowser()) { + // we can proceed the filter chain because the HttpProtocolServlet will render the ui if the client is a browser + chain.doFilter(request, response); + } else { + super.handleTokenExpiredException(request, response, chain, tokenExpiredException); + } + } } diff --git a/scm-core/src/test/java/sonia/scm/web/filter/HttpProtocolServletAuthenticationFilterBaseTest.java b/scm-core/src/test/java/sonia/scm/web/filter/HttpProtocolServletAuthenticationFilterBaseTest.java index daa2ad448b..2c49458547 100644 --- a/scm-core/src/test/java/sonia/scm/web/filter/HttpProtocolServletAuthenticationFilterBaseTest.java +++ b/scm-core/src/test/java/sonia/scm/web/filter/HttpProtocolServletAuthenticationFilterBaseTest.java @@ -21,7 +21,7 @@ * OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE * SOFTWARE. */ - + package sonia.scm.web.filter; import org.junit.jupiter.api.BeforeEach; @@ -30,6 +30,7 @@ import org.junit.jupiter.api.extension.ExtendWith; import org.mockito.Mock; import org.mockito.junit.jupiter.MockitoExtension; import sonia.scm.config.ScmConfiguration; +import sonia.scm.security.TokenExpiredException; import sonia.scm.util.HttpUtil; import sonia.scm.web.UserAgent; import sonia.scm.web.UserAgentParser; @@ -43,6 +44,7 @@ import java.io.IOException; import java.util.Collections; import java.util.Set; +import static org.junit.jupiter.api.Assertions.assertThrows; import static org.mockito.Mockito.verify; import static org.mockito.Mockito.when; @@ -93,4 +95,23 @@ class HttpProtocolServletAuthenticationFilterBaseTest { verify(filterChain).doFilter(request, response); } + @Test + void shouldIgnoreTokenExpiredExceptionForBrowserCall() throws IOException, ServletException { + when(userAgentParser.parse(request)).thenReturn(browser); + + authenticationFilter.handleTokenExpiredException(request, response, filterChain, new TokenExpiredException("Nothing ever expired so much")); + + verify(filterChain).doFilter(request, response); + } + + @Test + void shouldRethrowTokenExpiredExceptionForApiCall() { + when(userAgentParser.parse(request)).thenReturn(nonBrowser); + + final TokenExpiredException tokenExpiredException = new TokenExpiredException("Nothing ever expired so much"); + + assertThrows(TokenExpiredException.class, + () -> authenticationFilter.handleTokenExpiredException(request, response, filterChain, tokenExpiredException)); + } + }