Proposed fix for missing WWW-Authenticate header

Current implementation does not include the WWW-Authenticate
header when returning a 401 for missing/invalid credentials when
attempting to access the token endpoints.

Fixes-468

Signed-off-by: Lucian Holland <lucian@patientsknowbest.com>

Author: lucian-pkb

oauth2-authorization-server/src/main/java/org/springframework/security/oauth2/server/authorization/config/annotation/web/configurers/OAuth2ClientAuthenticationConfigurer.java

@@ -79,6 +79,8 @@ public final class OAuth2ClientAuthenticationConfigurer extends AbstractOAuth2Co
 
 	private AuthenticationFailureHandler errorResponseHandler;
 
+	private String realmName = "oauth";
+
 	/**
 	 * Restrict for internal use only.
 	 * @param objectPostProcessor an {@code ObjectPostProcessor}
@@ -102,6 +104,18 @@ public OAuth2ClientAuthenticationConfigurer authenticationConverter(
 		return this;
 	}
 
+	/**
+	 * Sets the realm name for Http Basic when returning a WWW-Authenticate header on
+	 * client authentication failure.
+	 * @param realmName the Http Basic realm name
+	 * @return the {@link OAuth2ClientAuthenticationConfigurer} for further configuration
+	 */
+	public OAuth2ClientAuthenticationConfigurer realmName(String realmName) {
+		Assert.hasText(realmName, "realmName cannot be empty");
+		this.realmName = realmName;
+		return this;
+	}
+
 	/**
 	 * Sets the {@code Consumer} providing access to the {@code List} of default and
 	 * (optionally) added {@link #authenticationConverter(AuthenticationConverter)
@@ -213,7 +227,7 @@ void init(HttpSecurity httpSecurity) {
 	void configure(HttpSecurity httpSecurity) {
 		AuthenticationManager authenticationManager = httpSecurity.getSharedObject(AuthenticationManager.class);
 		OAuth2ClientAuthenticationFilter clientAuthenticationFilter = new OAuth2ClientAuthenticationFilter(
-				authenticationManager, this.requestMatcher);
+				authenticationManager, this.requestMatcher, this.realmName);
 		List<AuthenticationConverter> authenticationConverters = createDefaultAuthenticationConverters();
 		if (!this.authenticationConverters.isEmpty()) {
 			authenticationConverters.addAll(0, this.authenticationConverters);

oauth2-authorization-server/src/main/java/org/springframework/security/oauth2/server/authorization/web/OAuth2ClientAuthenticationFilter.java

@@ -24,6 +24,7 @@
 import jakarta.servlet.http.HttpServletResponse;
 
 import org.springframework.core.log.LogMessage;
+import org.springframework.http.HttpHeaders;
 import org.springframework.http.HttpStatus;
 import org.springframework.http.converter.HttpMessageConverter;
 import org.springframework.http.server.ServletServerHttpResponse;
@@ -90,24 +91,37 @@ public final class OAuth2ClientAuthenticationFilter extends OncePerRequestFilter
 
 	private final AuthenticationDetailsSource<HttpServletRequest, ?> authenticationDetailsSource = new WebAuthenticationDetailsSource();
 
+	private final String realmName;
+
 	private AuthenticationConverter authenticationConverter;
 
 	private AuthenticationSuccessHandler authenticationSuccessHandler = this::onAuthenticationSuccess;
 
 	private AuthenticationFailureHandler authenticationFailureHandler = this::onAuthenticationFailure;
 
+	/**
+	 * Internal error code used to distinguish missing authentication from invalid
+	 * authentication in order to display a WWW-Authenticate header when appropriate The
+	 * default failure handler will convert this to the spec-compliant 'invalid_client'
+	 * before returning to the caller.
+	 */
+	public static final String MISSING_CLIENT_AUTH_ERROR_CODE = "missing_client_auth";
+
 	/**
 	 * Constructs an {@code OAuth2ClientAuthenticationFilter} using the provided
 	 * parameters.
 	 * @param authenticationManager the {@link AuthenticationManager} used for
 	 * authenticating the client
 	 * @param requestMatcher the {@link RequestMatcher} used for matching against the
 	 * {@code HttpServletRequest}
+	 * @param realmName realm name to use in WWW-Authenticate header for Basic auth
 	 */
-	public OAuth2ClientAuthenticationFilter(AuthenticationManager authenticationManager,
-			RequestMatcher requestMatcher) {
+	public OAuth2ClientAuthenticationFilter(AuthenticationManager authenticationManager, RequestMatcher requestMatcher,
+			String realmName) {
+		this.realmName = realmName;
 		Assert.notNull(authenticationManager, "authenticationManager cannot be null");
 		Assert.notNull(requestMatcher, "requestMatcher cannot be null");
+		Assert.notNull(realmName, "realmName cannot be null");
 		this.authenticationManager = authenticationManager;
 		this.requestMatcher = requestMatcher;
 		// @formatter:off
@@ -140,9 +154,12 @@ protected void doFilterInternal(HttpServletRequest request, HttpServletResponse
 				validateClientIdentifier(authenticationRequest);
 				Authentication authenticationResult = this.authenticationManager.authenticate(authenticationRequest);
 				this.authenticationSuccessHandler.onAuthenticationSuccess(request, response, authenticationResult);
+				filterChain.doFilter(request, response);
+			}
+			else {
+				this.authenticationFailureHandler.onAuthenticationFailure(request, response,
+						new OAuth2AuthenticationException(MISSING_CLIENT_AUTH_ERROR_CODE));
 			}
-			filterChain.doFilter(request, response);
-
 		}
 		catch (OAuth2AuthenticationException ex) {
 			if (this.logger.isTraceEnabled()) {
@@ -204,27 +221,23 @@ private void onAuthenticationFailure(HttpServletRequest request, HttpServletResp
 
 		SecurityContextHolder.clearContext();
 
-		// TODO
-		// The authorization server MAY return an HTTP 401 (Unauthorized) status code
-		// to indicate which HTTP authentication schemes are supported.
-		// If the client attempted to authenticate via the "Authorization" request header
-		// field,
-		// the authorization server MUST respond with an HTTP 401 (Unauthorized) status
-		// code and
-		// include the "WWW-Authenticate" response header field
-		// matching the authentication scheme used by the client.
-
 		OAuth2Error error = ((OAuth2AuthenticationException) exception).getError();
 		ServletServerHttpResponse httpResponse = new ServletServerHttpResponse(response);
-		if (OAuth2ErrorCodes.INVALID_CLIENT.equals(error.getErrorCode())) {
+		String errorCode = error.getErrorCode();
+		String authHeader = request.getHeader(HttpHeaders.AUTHORIZATION);
+
+		if (MISSING_CLIENT_AUTH_ERROR_CODE.equals(errorCode) || (OAuth2ErrorCodes.INVALID_CLIENT.equals(errorCode)
+				&& authHeader != null && authHeader.trim().startsWith("Basic"))) {
+			errorCode = OAuth2ErrorCodes.INVALID_CLIENT;
 			httpResponse.setStatusCode(HttpStatus.UNAUTHORIZED);
+			httpResponse.getHeaders().set("WWW-Authenticate", "Basic realm=\"" + this.realmName + "\"");
 		}
 		else {
 			httpResponse.setStatusCode(HttpStatus.BAD_REQUEST);
 		}
 		// We don't want to reveal too much information to the caller so just return the
 		// error code
-		OAuth2Error errorResponse = new OAuth2Error(error.getErrorCode());
+		OAuth2Error errorResponse = new OAuth2Error(errorCode);
 		this.errorHttpResponseConverter.write(errorResponse, null, httpResponse);
 	}
 

oauth2-authorization-server/src/test/java/org/springframework/security/oauth2/server/authorization/web/OAuth2ClientAuthenticationFilterTests.java

@@ -26,6 +26,7 @@
 import org.junit.jupiter.api.Test;
 import org.mockito.ArgumentCaptor;
 
+import org.springframework.http.HttpHeaders;
 import org.springframework.http.HttpMethod;
 import org.springframework.http.HttpStatus;
 import org.springframework.http.converter.HttpMessageConverter;
@@ -81,7 +82,7 @@ public class OAuth2ClientAuthenticationFilterTests {
 	public void setUp() {
 		this.authenticationManager = mock(AuthenticationManager.class);
 		this.requestMatcher = new AntPathRequestMatcher(this.filterProcessesUrl, HttpMethod.POST.name());
-		this.filter = new OAuth2ClientAuthenticationFilter(this.authenticationManager, this.requestMatcher);
+		this.filter = new OAuth2ClientAuthenticationFilter(this.authenticationManager, this.requestMatcher, "realm");
 		this.authenticationConverter = mock(AuthenticationConverter.class);
 		this.filter.setAuthenticationConverter(this.authenticationConverter);
 	}
@@ -93,14 +94,14 @@ public void cleanup() {
 
 	@Test
 	public void constructorWhenAuthenticationManagerNullThenThrowIllegalArgumentException() {
-		assertThatThrownBy(() -> new OAuth2ClientAuthenticationFilter(null, this.requestMatcher))
+		assertThatThrownBy(() -> new OAuth2ClientAuthenticationFilter(null, this.requestMatcher, "realm"))
 			.isInstanceOf(IllegalArgumentException.class)
 			.hasMessage("authenticationManager cannot be null");
 	}
 
 	@Test
 	public void constructorWhenRequestMatcherNullThenThrowIllegalArgumentException() {
-		assertThatThrownBy(() -> new OAuth2ClientAuthenticationFilter(this.authenticationManager, null))
+		assertThatThrownBy(() -> new OAuth2ClientAuthenticationFilter(this.authenticationManager, null, "realm"))
 			.isInstanceOf(IllegalArgumentException.class)
 			.hasMessage("requestMatcher cannot be null");
 	}
@@ -149,8 +150,10 @@ public void doFilterWhenRequestMatchesAndEmptyCredentialsThenNotProcessed() thro
 
 		this.filter.doFilter(request, response, filterChain);
 
-		verify(filterChain).doFilter(any(HttpServletRequest.class), any(HttpServletResponse.class));
-		verifyNoInteractions(this.authenticationManager);
+		verifyNoInteractions(this.authenticationManager, filterChain);
+		assertThat(response.getStatus()).isEqualTo(HttpStatus.UNAUTHORIZED.value());
+		OAuth2Error error = readError(response);
+		assertThat(error.getErrorCode()).isEqualTo(OAuth2ErrorCodes.INVALID_CLIENT);
 	}
 
 	@Test
@@ -225,6 +228,7 @@ public void doFilterWhenRequestMatchesAndBadCredentialsThenInvalidClientError()
 
 		MockHttpServletRequest request = new MockHttpServletRequest("POST", this.filterProcessesUrl);
 		request.setServletPath(this.filterProcessesUrl);
+		request.addHeader(HttpHeaders.AUTHORIZATION, "Basic invalid-secret");
 		MockHttpServletResponse response = new MockHttpServletResponse();
 		FilterChain filterChain = mock(FilterChain.class);