From aca63cb67934c83e7b9e96542183d9269fa2add0 Mon Sep 17 00:00:00 2001 From: Davy Steegen Date: Tue, 27 Mar 2018 20:27:33 +0200 Subject: [PATCH] Send HTTP status code 401 to the response when no token could be obtained. The previous implementation resulted in status code 500 as every exception thrown in a ZuulFilter implementation is treated as failure by FilterProcessor. --- .../oauth2/proxy/OAuth2TokenRelayFilter.java | 16 +++++++------ .../proxy/OAuth2TokenRelayFilterTests.java | 24 +++++++++---------- 2 files changed, 21 insertions(+), 19 deletions(-) diff --git a/spring-cloud-security/src/main/java/org/springframework/cloud/security/oauth2/proxy/OAuth2TokenRelayFilter.java b/spring-cloud-security/src/main/java/org/springframework/cloud/security/oauth2/proxy/OAuth2TokenRelayFilter.java index e00d3ece..b5217300 100644 --- a/spring-cloud-security/src/main/java/org/springframework/cloud/security/oauth2/proxy/OAuth2TokenRelayFilter.java +++ b/spring-cloud-security/src/main/java/org/springframework/cloud/security/oauth2/proxy/OAuth2TokenRelayFilter.java @@ -1,15 +1,16 @@ package org.springframework.cloud.security.oauth2.proxy; +import java.io.IOException; import java.util.HashMap; import java.util.Map; import javax.servlet.http.HttpServletResponse; import org.springframework.cloud.security.oauth2.proxy.ProxyAuthenticationProperties.Route; -import org.springframework.security.authentication.BadCredentialsException; import org.springframework.security.core.Authentication; import org.springframework.security.core.context.SecurityContextHolder; import org.springframework.security.oauth2.client.OAuth2RestOperations; +import org.springframework.security.oauth2.client.resource.UserRedirectRequiredException; import org.springframework.security.oauth2.provider.OAuth2Authentication; import org.springframework.security.oauth2.provider.authentication.OAuth2AuthenticationDetails; @@ -21,6 +22,7 @@ * can detect the token as part of the currently authenticated principal. * * @author Dave Syer + * @author Davy Steegen * */ public class OAuth2TokenRelayFilter extends ZuulFilter { @@ -91,12 +93,12 @@ private String getAccessToken(RequestContext ctx) { try { value = restTemplate.getAccessToken().getValue(); } - catch (Exception e) { - // Quite possibly a UserRedirectRequiredException, but the caller - // probably doesn't know how to handle it, otherwise they wouldn't be - // using this filter, so we rethrow as an authentication exception - ctx.set("error.status_code", HttpServletResponse.SC_UNAUTHORIZED); - throw new BadCredentialsException("Cannot obtain valid access token"); + catch (UserRedirectRequiredException urre) { + try { + ctx.getResponse().sendError(HttpServletResponse.SC_UNAUTHORIZED); + } catch (IOException e) { + throw new RuntimeException("Unable to send error to the response", e); + } } } } diff --git a/spring-cloud-security/src/test/java/org/springframework/cloud/security/oauth2/proxy/OAuth2TokenRelayFilterTests.java b/spring-cloud-security/src/test/java/org/springframework/cloud/security/oauth2/proxy/OAuth2TokenRelayFilterTests.java index fdf7247d..504d2085 100644 --- a/spring-cloud-security/src/test/java/org/springframework/cloud/security/oauth2/proxy/OAuth2TokenRelayFilterTests.java +++ b/spring-cloud-security/src/test/java/org/springframework/cloud/security/oauth2/proxy/OAuth2TokenRelayFilterTests.java @@ -20,18 +20,22 @@ import static org.junit.Assert.assertFalse; import static org.junit.Assert.assertNotNull; import static org.junit.Assert.assertTrue; -import static org.junit.Assert.fail; + +import java.util.HashMap; + +import javax.servlet.http.HttpServletResponse; import org.junit.After; import org.junit.Before; import org.junit.Test; import org.mockito.Mockito; import org.springframework.mock.web.MockHttpServletRequest; -import org.springframework.security.authentication.BadCredentialsException; +import org.springframework.mock.web.MockHttpServletResponse; import org.springframework.security.authentication.UsernamePasswordAuthenticationToken; import org.springframework.security.core.Authentication; import org.springframework.security.core.context.SecurityContextHolder; import org.springframework.security.oauth2.client.OAuth2RestOperations; +import org.springframework.security.oauth2.client.resource.UserRedirectRequiredException; import org.springframework.security.oauth2.client.token.grant.code.AuthorizationCodeResourceDetails; import org.springframework.security.oauth2.common.DefaultOAuth2AccessToken; import org.springframework.security.oauth2.provider.AuthorizationRequest; @@ -62,6 +66,9 @@ public void init() { httpRequest.setAttribute(OAuth2AuthenticationDetails.ACCESS_TOKEN_TYPE, "bearer"); httpRequest.setAttribute(OAuth2AuthenticationDetails.ACCESS_TOKEN_VALUE, "FOO"); auth.setDetails(new OAuth2AuthenticationDetails(httpRequest)); + + RequestContext.testSetCurrentContext(new RequestContext()); + RequestContext.getCurrentContext().setResponse(new MockHttpServletResponse()); } @After @@ -117,20 +124,13 @@ public void unauthorizedWithRestTemplate() { AuthorizationCodeResourceDetails resource = new AuthorizationCodeResourceDetails(); resource.setClientId("client"); Mockito.when(restTemplate.getResource()).thenReturn(resource); - Mockito.when(restTemplate.getAccessToken()).thenThrow(new RuntimeException()); + Mockito.when(restTemplate.getAccessToken()).thenThrow(new UserRedirectRequiredException("http://login", new HashMap())); filter.setRestTemplate(restTemplate); assertNotNull(RequestContext.getCurrentContext()); SecurityContextHolder.getContext().setAuthentication(auth); assertTrue(filter.shouldFilter()); - try { - filter.run(); - fail("Expected BadCredentialsException"); - } - catch (BadCredentialsException e) { - assertEquals(401, - RequestContext.getCurrentContext().get("error.status_code")); - - } + filter.run(); + assertEquals(RequestContext.getCurrentContext().getResponse().getStatus(), HttpServletResponse.SC_UNAUTHORIZED); } }