Public client removal will possibly cause issues with external clients

ORY Hydra
#1

Re: https://github.com/ory/hydra/blob/master/UPGRADE.md#oauth-20-client-flag-public-has-been-removed

Version: Docker image oryd/hydra:v1.0.0-rc.6_oryOS.10

This will cause issues with clients that rely on the basic auth header vs posting the client_id in the body. Those clients will attempt to post with a valid username:null_password combo, which hydra then denies (depending on configuration) because either…

a) the client is only allowed to use “none” for token_endpoint_auth_method
b) the bcrypt password appears to be generated, and isn’t actually a blank password.

The only actual work-around I’ve been able to do for this is to manually update the bcrypt value to one thats actually a null password, but manually updating the DB for something like this won’t scale, and becomes a massive headache.

I figured I’d post here because this could be considered a bug, but I’d like some insight into the thought process behind forcing the client_id into the body.

#2

So there are several authentication modes for clients when conforming with OIDC Dynamic Client Registration:

token_endpoint_auth_method

OPTIONAL. Requested Client Authentication method for the Token Endpoint. The options are client_secret_post, client_secret_basic, client_secret_jwt, private_key_jwt, and none, as described in Section 9 of OpenID Connect Core 1.0 [OpenID.Core]. Other authentication methods MAY be defined by extensions. If omitted, the default is client_secret_basic – the HTTP Basic Authentication Scheme specified in Section 2.3.1 of OAuth 2.0 [RFC6749].

If you want to send stuff in the post body, you need to set it to client_secret_post, if it’s a public client (not confidential) you set it to none. Does that answer your question?

#3

By default, client_secret_basic is used, whether the client in question is public or otherwise.

Lots of existing clients using public clients send authorization in a basic auth header, which is to my understanding, correct per the specifications. Forcing all public clients to use “none,” which disallows posting authorization with an auth header, will break compatibility with those clients.

Setting those clients to any other auth method causes a bcrypt error, as the generated passwords for those public clients don’t match with an empty string, and appear to be randomly generated.

#4

By default , client_secret_basic is used, whether the client in question is public or otherwise.

client_secret_basic marks a client as confidential. Clients without confidentiality (e.g. for SPAs) must use none, per specification.

Lots of existing clients using public clients send authorization in a basic auth header, which is to my understanding, correct per the specifications. Forcing all public clients to use “none,” which disallows posting authorization with an auth header, will break compatibility with those clients.

Hm, specification like PKCE send this in the body. It is true however that Authorization: Basic base64encode(username): is valid (so no password sent) and is indeed not supported by fosite. However, the OIDC core spec does not touch on wether this should be sent in the body or as part of the header - but the oidc conformity tests do send it in the body and not the header.

If you can show me a reference where sending just the username in the http header is supported/required/whatever I’m happy to consider updating the logic here.

#5

From my understanding, sending in either the header (basic authorization) or in the body are both valid, and were previously supported with oauth2. “May” gets tricky…

Regarding the username:NIL posting, I know that insomnia and postman both send public (read: no password) client_ids by default (configurable) when using an authorization code flow, and I know that hydra-0.11 supported it previously. I’ve been meaning into digging into third party libraries to see how they implement it, but spare time isn’t something I’ve had a ton of lately.

#6

Ok, I think I’m open to have both possibilities - sending foo:"nil" and post parameter! Feel free to PR in fosite :slight_smile: