acesstoken: credentials are cleared when using 'authorization: Basic'

[bbigras]

Bug report

Creds are set with credentials.authenticate() (334) but are cleared by credentials.unauthenticated (343).

oxide-auth/oxide-auth/src/code_grant/accesstoken.rs

Lines 332 to 345 in bd2ac63

	let mut credentials = Credentials::None;
	if let Some((client_id, auth)) = &authorization {
	credentials.authenticate(client_id.as_ref(), auth.as_ref());
	}
	if let Some(client_id) = &client_id {
	match &client_secret {
	Some(auth) if request.allow_credentials_in_body() => {
	credentials.authenticate(client_id.as_ref(), auth.as_ref().as_bytes())
	}
	// Ignore parameter if not allowed.
	Some(_) | None => credentials.unauthenticated(client_id.as_ref()),
	}
	}

Reproduction

Steps to reproduce the behavior:

1. POST /token with authorization: Basic and request.extension("client_secret") as None.

Expected behaviour

Common sense dictates, that the creds should be used. Unless maybe the specs says otherwise.

---

• A pull request (does not yet exist)

[HeroicKatora]

Note: the code path is only executed if a client_id is provided as a body parameter. This is also optional. The RFC says:

Alternatively, the authorization server MAY support including the
client credentials in the request-body using the following
parameters:

client_id […]

client_secret
REQUIRED. The client secret. The client MAY omit the
parameter if the client secret is an empty string.

Including the client credentials in the request-body using the two
parameters is NOT RECOMMENDED and SHOULD be limited to clients unable
to directly utilize the HTTP Basic authentication scheme (or other
password-based HTTP authentication schemes).

This is to be revised in OAuth 2.1 to

"client_secret": REQUIRED. The client secret.

In order to avoid confusion where client_id in body conflicts with the Basic authorization, we reject the authentication when any portion of it is present but it can not be validated (no matter if due to being disabled, or incorrect).
Note: would be more consistent if client_secret without client_id were simiarly rejected.

[bbigras]

Thanks!