Skip to content

Conversation

ebymatthew
Copy link

This pull request replaces #20.

After further reading reading of the spec, my understanding is client type is a property of a client and not the entire server. With #20, the entire server would be configured public or confidential. With this PR a server could be configured to handle both public and confidential client types and act appropriately.

The allowPublicClients defaults to false to preserve compatibility with previous versions.

If the option is false:

  • any request without an authentication header will be rejected.

If the option is true:

  • requests without headers will be allowed, and clientId/clientSecret will be null
  • requests with headers will be populated with the clientId and clientSecret

This approach also allows the potential for future support of the authorization_code grant type. Where you may receive a clientId but no secret: http://tools.ietf.org/html/rfc6749#section-3.2.1

…authentication header with client credentials
@domenic
Copy link
Owner

domenic commented Apr 15, 2014

Do you think we should call validateClient at all? Or should we just skip it?

@ebymatthew
Copy link
Author

I left validateClient because of the potential that you may receive only a client_id and no secret. Under the authorization_code grant type an unauthenticated client is still required to send it's id. Also, that same section says unauthenticated clients MAY send client_id in the request body. I'm reading that to mean that there is potential to support receiving id w/ no secret for the ROPC and CC flows too.

@domenic
Copy link
Owner

domenic commented Apr 15, 2014

Hmm, will have to investigate further. I don't think considerations of the authorization_code flow should impact the ROPC flow coder experience though.

@ebymatthew
Copy link
Author

I considered not adding the allowPublicClients option and expecting the hooks to handle the case where clientId or clientSecret are null. That simplifies the configuration options. If you don't want to support public clients you reject when they don't identify or give you credentials. I didn't take that approach, since it would break compatibility.

@ebymatthew
Copy link
Author

Didn't see your previous comment. I agree, especially considering that flow isn't even supported at this point.

domenic added a commit that referenced this pull request Apr 28, 2014
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

Successfully merging this pull request may close these issues.

3 participants