Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Grant Type Password Requires a client_id #71

Closed
jeffsawatzky opened this issue Mar 11, 2019 · 5 comments
Closed

Grant Type Password Requires a client_id #71

jeffsawatzky opened this issue Mar 11, 2019 · 5 comments

Comments

@jeffsawatzky
Copy link

As per doorkeeper-gem/doorkeeper#296, client_id is optional for the password grant type, however doorkeeper-openid_connect seems to assume that it will always be there.

@toupeira
Copy link
Member

@Niltz thanks, can you give some more details on the errors you're seeing? I'm not sure if this is related to this change, as it seems that's the only file where we deal with client.

@natashad
Copy link

@toupeira The issue seems to be in the code to get the audience for the id token. https://github.com/doorkeeper-gem/doorkeeper-openid_connect/blob/master/lib/doorkeeper/openid_connect/id_token.rb#L49. If there's no client_id passed in, there's no application so @access_token.application.uid this fails with a NoMethodError (undefined method 'uid' for nil:NilClass)

FATAL
FATAL   NoMethodError (undefined method `uid' for nil:NilClass):
FATAL
FATAL   doorkeeper-openid_connect (1.6.0) lib/doorkeeper/openid_connect/id_token.rb:49:in `audience'
doorkeeper-openid_connect (1.6.0) lib/doorkeeper/openid_connect/id_token.rb:19:in `claims'
doorkeeper-openid_connect (1.6.0) lib/doorkeeper/openid_connect/id_token.rb:28:in `as_json'
doorkeeper-openid_connect (1.6.0) lib/doorkeeper/openid_connect/id_token.rb:32:in `as_jws_token'
doorkeeper-openid_connect (1.6.0) lib/doorkeeper/openid_connect/oauth/token_response.rb:12:in `body'
doorkeeper (5.0.0) app/controllers/doorkeeper/tokens_controller.rb:6:in `create'
app/controllers/tokens_controller.rb:11:in `create'
actionpack (5.2.2) lib/abstract_controller/base.rb:194:in `process_action'
actionpack (5.2.2) lib/action_controller/metal/instrumentation.rb:34:in `block in process_action'

@alduro
Copy link

alduro commented Apr 22, 2019

I'm running exactly into this issue. Is this still being maintained ?

@toupeira
Copy link
Member

toupeira commented Jun 7, 2019

@natashad thanks for looking into this! Still not sure how we should fix this though, according to the spec the client_id MUST be returned:

REQUIRED. Audience(s) that this ID Token is intended for. It MUST contain the OAuth 2.0 client_id of the Relying Party as an audience value.

But looking at this article it seems it's also acceptable to return the username sent by the client as client_id.

Unfortunately AFAICT we don't have access to that in Doorkeeper, since the user has to implement resource_owner_from_credentials and we only get the final resource owner object.

I wonder if it's also okay to return resource_owner.id? Does anybody have access to another OIDC provider which supports password grant, and can report what they return for client_id?

Is this still being maintained ?

@alduro barely, it's mostly just me ;-) Any help is welcome!

@toupeira
Copy link
Member

toupeira commented Feb 7, 2020

The problem with the audience claim should now be fixed with #99 released in 1.7.1, feel free to reopen if you're still experiencing issues!

@toupeira toupeira closed this as completed Feb 7, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

4 participants