client_secret_basic authentication failures should return challenge

[jgrandja]

As per section 3.2.3.1. Error Response:

"invalid_client": Client authentication failed (e.g., unknown client, no client authentication included, or unsupported authentication method). 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.

We should respond with the required authentication scheme when a client fails authentication.

[octopusthu]

Hi @jgrandja , the latest version of OAuth 2.1 is draft-ietf-oauth-v2-1-04, so perhaps you could update the link address in the original post to this: 3.2.3.1. Error Response

[sgiannino]

Hi @jgrandja i can work on it, in which version is it planned? Thank you!

[jgrandja]

Thanks for your interest @Enkosz. This enhancement hasn't been planned for a specific release since it's lower priority.

However, if you would like to work on it we can schedule it whenever it is done.

[symposion]

@jgrandja You've closed my ticket #1873 as a duplicate of this one, but this ticket is a low-priority enhancement. I'd like to argue that the current behaviour is a bug - since it's in direct violation of both the HTTP spec and the OAuth spec; and it's not entirely insignificant since it means that many http clients, including at least Apache Commons and the .NET default http client - will, with their default settings, fail to authenticate for a client_credentials grant using http basic. This is a very common use case with very common libraries and it's actually quite non-obvious what's wrong at first glance. After upgrading from the old @EnableAuthorizationServer implementation to the new Spring Authorization Server this caused several of our customers to experience outages of their live services due to the (spec-incompliant) change in behaviour.

I'm happy to submit a PR - I believe the fix is fairly trivial - if you agree that this is reasonably important.

[jgrandja]

@symposion The WWW-Authenticate response header was never implemented when client basic authn was implemented so that's why this was labeled as an enhancement. However, after reviewing the spec, I agree this is a bug since this MUST be implemented. I've changed it to a bug and scheduled it for next release.

Would you be able to submit a PR for this fix?

[symposion]

@jgrandja I'd be happy to have a stab at implementing this, it doesn't seem especially complicated. Before I do, I'd like to understand why this behaviour was changed from the original implementation in the old AuthorizationServerSecurityConfigurer. In that previous auth server implementation, the default behaviour was to use BasicAuthenticationEntryPoint with a default realm name of oauth2/client . It seems like when re-implementing a deliberate choice was made to use HttpStatusEntryPoint instead... was there a reason for that? Or did it just get lost in the move?

Related: is there any reason why we can't assume that it's a valid default setup to always issue a challenge with the Basic scheme? The spec says that the server should respond with the same scheme as any specified in the Authorization header of the request, but AFAICT out of the box SAS doesn't support anything other than Basic for this type of authentication (obviously it support other modes of auth such as JWT or POST parameters but these don't use the Authorization header)

Providing that this is configurable - and it should always be possible for users to supply their own AuthenticationEntryPoint - I think this seems like a reasonable default. If users extend the server to support other schemes for the Authorization header when authenticating clients, then obviously they'll have to customise the AuthenticationEntryPoint

[jgrandja]

@symposion

I'd like to understand why this behaviour was changed from the original implementation in the old AuthorizationServerSecurityConfigurer

Spring Authorization Server was a complete re-write and therefore you can't rely on the same behaviour as the legacy framework. As already mentioned, the WWW-Authenticate response header was not implemented when client_secret_basic was implemented.

is there any reason why we can't assume that it's a valid default setup to always issue a challenge with the Basic scheme?

To be clear, the WWW-Authenticate response header should only be returned when the client_secret_basic authentication method is used.

[symposion]

To be clear, the WWW-Authenticate response header should only be returned when the client_secret_basic authentication method is used.

I remain concerned that this isn't correct. Regardless of what the OAuth2 spec has to say on the subject, the HTTP spec is pretty unambiguous that this header is mandatory for all 401 responses. https://datatracker.ietf.org/doc/html/rfc7235#section-3.1 I know that Spring Security has chosen to bend this rule for certain specific cases - particularly for cases where the requesting client is an XMLHttpRequest to avoid creating problems for JS applications - and we could choose to do the same here; but omitting it otherwise when returning a 401 seems wrong.

[jgrandja]

Regardless of what the OAuth2 spec has to say on the subject

Spring Authorization Server is responsible for implementing features as outlined in the various specs. For anything off spec, we provide extension points (OAuth2ClientAuthenticationFilter.setAuthenticationSuccessHandler()) that consuming applications can customize to meet their requirements.

If there are other client authentication methods, other than client_secret_basic, that require the WWW-Authenticate response header, then please provide a reference to the spec so I can review it. As far as I know, it has only been specified for client_secret_basic method.

[symposion]

If there are other client authentication methods, other than client_secret_basic, that require the WWW-Authenticate response header, then please provide a reference to the spec so I can review it. As far as I know, it has only been specified for client_secret_basic method.

Sorry, maybe I wasn't clear; it's not that I'm saying there are other authentication methods to consider; it's that the HTTP spec itself is very clear that no matter what a WWW-Authenticate header is mandatory for all responses with a status code of 401. As such, it would seem to make sense to ensure that wherever the code sets a 401 status code on a response, it is also setting this header unless there's a specific reason to ignore the HTTP spec (e.g. that the client is explicitly a JS client using XMLHttpRequest).

[jgrandja]

@symposion I'm completely following you, but again, unless it's specified in one of the OAuth2 specs, it's off spec and must be implemented in the consuming application as a customization. The OAuth2 specs take precedence over the HTTP spec.

I'm open to adding the WWW-Authenticate to other client authentication methods but it needs to be specified in the related spec.

We know for client_secret_basic, it would be WWW-Authenticate: Basic.

But what would the WWW-Authenticate header be for each of the other client authentication methods?

• client_secret_post
• client_secret_jwt
• private_key_jwt
• tls_client_auth
• self_signed_tls_client_auth

If you're able to identify this in any of the related specs, then please provide a link to the reference and we'll consider adding it as part of this PR.

[symposion]

@jgrandja I guess where I'm not quite on the same page yet is that I don't see the OAuth2 spec contradicting anything in the HTTP spec. The only thing the OAuth2 spec says is that if the client sent an Authorization header, the WWW-Authenticate header must much the same scheme. Since Spring Auth Server only currently supports one authentication method that uses the Auth header -> client_basic, this is trivial.

But in the case that a client attempts to authenticate with some other approach, I see nothing in the OAuth2 spec that justifies returning a 401 without a WWW-Authenticate header. I grant you that it's not obvious what such a header should contain. I think an argument could be made that it would always be appropriate to return the "Basic" scheme, since the server does at least support that, even if it also supports other modes of authentication.

Alternatively, the OAuth2 spec wording regarding invalid_client errors is: The authorization server MAY return an HTTP 401 (Unauthorized) status code to indicate which HTTP authentication schemes are supported. This implies to me that the other option would simply to be return a 400 instead if one of this authentication approaches fails.

All things considered, I think the most compliant approach here would be:

• No credentials provided at all -> 401 with WWW-Authenticate: Basic
• client_secret_basic provided, but invalid -> 401 with WWW-Authenticate: Basic
• Some other authentication provided, but invalid -> 400

But I guess the only issue here could be if clients using other authentication schemes are already relying on getting a 401 in case of authentication failure

[jgrandja]

No credentials provided at all -> 401 with WWW-Authenticate: Basic

We cannot default to WWW-Authenticate: Basic because if client authentication was customized and client_secret_basic was disabled (this configuration is possible) then it would be an invalid response. Furthermore, client_secret_basic is not a recommended best practice as outlined in OAuth 2.0 Security Best Current Practice:

It is RECOMMENDED to use asymmetric (public-key based) methods for client authentication such as mTLS or private_key_jwt.

Some other authentication provided, but invalid -> 400

This would likely break existing applications since they would be expecting 401. Not to mention, returning 400 is not a valid status code for unauthenticated requests.

client_secret_basic provided, but invalid -> 401 with WWW-Authenticate: Basic

At this point in our discussion, this is the only change that makes sense to me.

[symposion]

As concerns the response in the case of invalid alternative authentication methods: ok fine; I'm still not entirely convinced this is the intention of the specs, but you're the maintainer and I'll follow your lead on this.

As concerns the "no credentials" case, I think we need to do better here, somehow. The reason I came across this issue at all is because of the way several major HTTP clients - including the default .Net client and (I believe) Apache HttpComponents handle authentication by default. If you use their specific support for configuring Authentication like so:

var wb = new WebClient();
wb.Credentials = new NetworkCredential(clientId, clientSecret);

then this will fail in a rather opaque fashion unless you happen to know about the pre-emptive authentication setting. By default, these clients omit the "Authorization" header on the initial request, despite credentials having been provided explicitly. They will only submit the credentials using HTTP Basic in the Authorization header on receipt of a 401 with a correctly formatted WWW-Authenticate header.

Your suggestion means that, out of the box, Spring Authorization Server will not work as expected with some of the most common http clients in Java and .Net. I think that's a bad compromise and, from a personal point of view, it removes my justification to my employer for contributing here, because it's not going to fix our problem.

That said, I understand that it's not a valid thing to do to respond with a WWW-Authenticate: Basic challenge if the server doesn't support http basic, and we don't have control over what the user might customise. I can think of some reasonable compromises here:

1. Configure the WWW-Authenticate header for responses to token requests lacking authentication only if we're using the vanilla config. If setAuthenticationConverter is called we disable this behaviour on the basis that it might no longer be valid
2. We take an approach closer to what the main Spring-Security project has done- they have a dedicated Configurer for HttpBasic that is responsible for setting up both the handling for the authentication and the error response that includes the WWW-Authenticate header, to ensure that you don't end up with one without the other. That doesn't map totally straightforwardly to how Spring Auth Server is set up right now, but I don't see it being so hard to achieve something equivalent.

(1) is obviously easier but less helpful in users wanting to engage in customisation of the authentication methods. But I do think that some kind of solution is required here; this is not a theoretical problem, it affects major real-world client libraries.

[symposion]

I'd also like to add that this is a behaviour change vs the old authorization server config in previous versions of Spring Security. I understand that Spring Authorization Server is technically a different product, but in reality anyone who was using this functionality and is trying to upgrade to newer Spring Security (which is obviously essential for CVE patches) is going to need to use Spring Authorization Server. And if they have clients who use one of these very common libraries to do a client credentials grant, those clients are very likely going to break unless you override the default authorization server configuration. We found this out the hard way with live client integrations breaking. I don't think we'll be the only ones who experience this.

[jgrandja]

it means that many http clients, including at least Apache Commons and the .NET default http client - will, with their default settings, fail to authenticate for a client_credentials grant using http basic.

I'm still puzzled why this issue is popping up now since the WWW-Authenticate header hasn't been implemented since Jan 2020 as indicated by the TODO in this commit. Then I just realized that using straight-up HTTP client libs, instead of OAuth2 client libs, seems to be the difference.

it means that many http clients, including at least Apache Commons and the .NET default http client - will, with their default settings, fail to authenticate for a client_credentials grant using http basic.

I'm curious why you're using an HTTP client to perform OAuth2 client flows? There are many OAuth2 Client libs out there that are typically used for OAuth2. I suspect most of our users are using an OAuth2 Client lib with Spring Authorization Server or this specific issue would have been raised when we first implemented client authentication back in June 2020.

Are you able to switch to using an OAuth2 Client lib instead? I suspect the issue with the client_credentials grant would resolve.

[symposion]

@jgrandja Unfortunately this is not entirely under our control. We provide a large online service with many APIs, and we have customers that integrate with those. For our own work, of course, we use dedicated tools rather than writing this stuff for ourselves - primarily WebClient and its integration with the rest of the Spring Security architecture - and we strongly encourage our customers to do the same. But sadly, they don't always listen to us and think "how hard can it be". Then when things break/change it can be extremely hard to persuade them to update, especially when there's a fairly strong argument to be made that the change is in violation of the http spec.

[jgrandja]

we strongly encourage our customers to do the same. But sadly, they don't always listen to us and think "how hard can it be"

Understood.

Then when things break/change it can be extremely hard to persuade them to update

Agreed and I imagine this will be quite challenging on your end to convince your clients.

when there's a fairly strong argument to be made that the change is in violation of the http spec.

We're still not on the same page here. Spring Authorization Server is NOT an HTTP Server so the HTTP spec cannot take precedence over the OAuth2 spec. I reviewed 3.2.3.1. Error Response again:

"invalid_client": Client authentication failed (e.g., unknown client, no client authentication included, or unsupported authentication method). 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.

Note the bold text. This further validates that the only change required is to return WWW-Authenticate: Basic when client_secret_basic authentication method is used, given this is the matching authentication scheme used by the client and client_secret_basic is the only client authentication method that supports the Authorization request header.

Having said that, I do want to help you to get through these migration issues you are experiencing. To expedite your current situation and to simply "get things working", I can provide you a customization for OAuth2ClientAuthenticationFilter based on your requirements if you like? I would just need to understand what the expected behaviour is. At least this will get you by in the meantime until we get on the same page with the acceptable fix. What do you think?

[symposion]

We're still not on the same page here. Spring Authorization Server is NOT an HTTP Server so the HTTP spec cannot take precedence over the OAuth2 spec. I reviewed 3.2.3.1. Error Response again:

"invalid_client": Client authentication failed (e.g., unknown client, no client authentication included, or unsupported authentication method). 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.

Note the bold text. This further validates that the only change required is to return WWW-Authenticate: Basic when client_secret_basic authentication method is used, given this is the matching authentication scheme used by the client and client_secret_basic is the only client authentication method that supports the Authorization request header.

At the risk of flogging a dead horse, I still read this differently to you. I'll try and explain one last time, but if that fails then I guess just have to leave this one.

Breaking it down:

1. Spring Authorization Server absolutely is a HTTP server. OAuth2 is a higher level protocol that operates on top of HTTP as a transport layer. The OAuth2 Spec makes a normative reference to the HTTP spec and says

This specification is designed for use with HTTP ([RFC9110]). The use of OAuth over any protocol other than HTTP is out of scope.

Designing a spec that requires implementers to violate an explicit provision of the underlying, normative transport spec would be very unusual and incredibly bad practice, and certainly something I'd expect to see called out explicitly and justified.

2. The section you quote is a subsection of the broader section on error handling. The initial text sets the default behaviour:

The authorization server responds with an HTTP 400 (Bad Request) status code (unless specified otherwise) and includes the following parameters with the response:

So the expected behaviour is: return a 400 for all errors unless otherwise specified.

2. In the case of the "invalid_client" error code, the spec provides for an optional variation on the above - the server MAY return a 401. Interestingly, it is specifically stated that the purpose of doing so would be to indicate which HTTP authentication schemes are supported - and the only specified mechanism by which the server can do this is via the WWW-Authenticate header AFAIK. Up until this point, there is nothing at all that suggests that the OAuth2 Spec intends for implementers to do anything contrary to the underlying HTTP spec in regards to 401 responses, and the "... to indicate..." subclause strongly implies to me that they very much intend to pick up the default HTTP behaviour here.

3. The clincher here for both us is that final sentence:

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.

I agree here that in practice, for Spring Auth Server, which doesn't support any other Authn schemes that use the Authorization header other than client_secret_basic, then the only situation in which this sentence applies is when a client has submitted credentials using http basic, and in that situation the server must respond with a header including that same scheme. Where I disagree is the implication that you additionally seem to be drawing, which is that this somehow entails that there are no other circumstances in which the WWW-Authenticate header is required - that this is an exhaustive specification of the usage of this header rather than a specific restriction of its use when a particular condition holds. That seems to me to be a very odd reading of the language here. If a spec is built fundamentally on another one - and OAuth2 relies absolutely on the HTTP spec to be in any way useful - then I think overriding the underlying spec is something that surely has to be explicit rather than just a failure to restate.

The fully explicitly version of my reading of this section would be something like:

In the case of the "invalid_client" error, the server is free to return a 401 instead of 400 in order to indicate to the client what HTTP authentication schemes it supports [using the mandatory WWW-Authenticate header as specified in the HTTP spec in regards to 401 responses]. If the client included an Authorization header field in the original request, then returning a 401 becomes mandatory, and _in this case_ this specification further constrains the structure of the mandatory WWW-Authenticate header: it must reference the same HTTP authentication scheme original requested by the client rather than any other authentication schemes that the server may support

Anyway, that's as completely clear and explicit as I think I can possibly be. At the end of the day, if you don't agree, then I'm happy to shut the PR and we'll patch at this end to make it behave how we think it should.

Having said that, I do want to help you to get through these migration issues you are experiencing. To expedite your current situation and to simply "get things working", I can provide you a customization for OAuth2ClientAuthenticationFilter based on your requirements if you like? I would just need to understand what the expected behaviour is. At least this will get you by in the meantime until we get on the same page with the acceptable fix. What do you think?

Thank you for the offer, that's kind. We already have a workaround that we can apply - we know our way around Spring Security and Spring Auth Server reasonably well now and I don't have a problem fixing this at our end. My motivation for arguing the case here is partly altruistic - I don't want to see other people caught out by this same issue; and partly selfish, in that any such customization is of course a potential source of headaches if and when Spring Auth Server updates and our customisations don't fit with the new version (which is of course how we ended up here in the first place)

Thank you for your time in talking through this - I appreciate you taking the time to discuss it.

[jgrandja]

@symposion I asked @sjohnr for a 2nd opinion on this issue and he also feels that it would be helpful for the client if we implemented the optional behaviour:

The authorization server MAY return an HTTP 401 (Unauthorized) status code to indicate which HTTP authentication schemes are supported

I believe this is the only sticking point at the moment?

Let me restate the changes to resolve this issue.

1. If client authentication fails using the client_secret_basic method then return 401 WWW-Authenticate: Basic.
2. If no client authentication was included then return the default 401 WWW-Authenticate: Basic.

Can you re-confirm if these are the changes you are looking for?

[symposion]

@jgrandja

Yes, this would fix the problem.

I was talking this over with my colleagues earlier and it would seem like maybe the best option in this case would be to provide a customisation hook for OAuth2ClientAuthenticationFilter so that the behaviour in onAuthenticationFailure can be controlled. I think the behaviour you described in your last message would be a reasonable default given the client_secret_basic is normally configured out of the box, but having a clear extension point to allow this to be overridden in the case that someone chose to alter the supported authentication methods would be a good idea.

I'm very happy to have a stab at implementing this if you agree.

[jgrandja]

@symposion

Yes, this would fix the problem.

Excellent. As far as 2. goes (no client auth) there might be some side effects with the new default but we'll work through it and figure it out.

to provide a customisation hook for OAuth2ClientAuthenticationFilter so that the behaviour in onAuthenticationFailure can be controlled

This is already possible. See Configuring Client Authentication and the clientAuthentication.errorResponseHandler(), which is an AuthenticationFailureHandler.

I'm very happy to have a stab at implementing this if you agree.

That would be great. Thanks.

[jgrandja]

Reverted via c624d0a908af0b2a9314021e1377332ed3370661

[jgrandja]

This issue was transferred from spring-projects/spring-authorization-server (see spring-authorization-server#2195)