Missing required parameter client_id returning invalid_client

[StefRoo]

Which version of Duende IdentityServer are you using?
v4.

Which version of .NET are you using?
.NET 6

Describe the bug
A third party pointed out to us that upon sending us a request on our token endpoint in the flow described in the OAuth2.0 specifications in chapter 4.1, the Authorization Code Grant, without a client_id configured, we return an error-response with the error-code set to 'invalid_client'. We have seen in the codebase of IdentityServer that this is default behaviour, as can be seen in the ClientSecretValidator:

if (parsedSecret == null)
        {
            await RaiseFailureEventAsync("unknown", "No client id found");

            _logger.LogError("No client identifier found");
            return fail;
        }

And in the TokenEndpoint:

if (clientResult.Client == null)
       {
           return Error(OidcConstants.TokenErrors.InvalidClient);
       }

To Reproduce
Send a request to the TokenEndpoint with no client_id configured.

Expected behavior
In our use case, section 4.1 of the OAuth2.0 specification, the client_id is a required parameter. As RFC 6749 section 5.2 states, if a required parameter is missing in a request, an error-response with the error-code set to 'invalid_request' should be returned. We would therefore expect the default TokenEndpoint implementation to return an 'invalid_request' instead of an 'invalid_client' when the entire parameter is missing, and an 'invalid_client' when the content of the parameter is missing or empty.

We would like to hear your thoughts on this issue.

[brockallen]

We'll have a look, thanks.

[user1336]

@brockallen if you agree to this issue we can create a PR.

[brockallen]

@leastprivilege to review. the issue here is that our abstraction over validating client credentials doesn't indicate this specific missing param as a special case.

[brockallen]

@StefRoo, i understand the desire to be as spec compliant as possible, but is there a real/practical reason that the error code being invalid_request vs. invalid_client really matters? I'm thinking that in either case, a client would know there's an error and need to consult the logs to determine the real issue. I can't see how the distinction matters in a real runtime environment (but that's why I'm asking).

[user1336]

@brockallen @StefRoo , It might be very semantic but the client could interpret invalid_request as that they should probably alter a parameter in the request. On the other hand they could interpret invalid_client as authentication fails so that they will investigate the authentication process.
Either way, the client would still be denied authorization.

The reason why we insist for this change is that the third party audits our product for compliancy to the OAuth specification and at this point we have a non-conformity. To mitigate this non-conformity we had to override the whole Token endpoint, so extensibility was quite unpractical. Even if we don't want to make a change to the validation of the client credentials perhaps we could provide for better extensibility.

[brockallen]

Ok, I looked again and there is a fairly small change to accommodate that specific error condition. PR is submitted.

[user1336]

@brockallen Amazing, looks good! Thanks