Skip to content

add a clientType option for ROPC flows to allow for public client types#20

Closed
ebymatthew wants to merge 2 commits intodomenic:masterfrom
matteby:addClientTypePublicOption
Closed

add a clientType option for ROPC flows to allow for public client types#20
ebymatthew wants to merge 2 commits intodomenic:masterfrom
matteby:addClientTypePublicOption

Conversation

@ebymatthew
Copy link
Copy Markdown

I'm working on a client application where confidentiality of the client credentials cannot be maintained and therefore falls under the OAuth2 definition of a public client type. I've found restify-oauth2 very helpful and would love to see this change integrated (or some form of support for public clients). Below is a summary of the relevant OAuth2 sections and a summary of my changes...

OAuth2 describes two types of clients public and confidential.

The CC flow must only be used by confidential client types.

The ROPC flow allows either public or confidential client types.

With this pull request, I've added an additional option clientType that can be passed to restifyOAuth2.ropc. Valid values are public or confidential. It defaults to confidential to preserve compatibility with previous versions.

@domenic
Copy link
Copy Markdown
Owner

domenic commented Apr 14, 2014

Just define your validateClient hook to always call back with true... I don't get it.

@ebymatthew
Copy link
Copy Markdown
Author

Yes, that was my first thought too.

The Ember client side OAuth2 library that I'm using does not send an authorization header which causes a error in validateGrantTokenRequest. Client side frameworks can't protect the secret, so I think that's why OAuth2 allows for public clients. It didn't make much sense to submit a patch to the Ember library.

I guess the other reason to implement this is because it's in the OAuth2 spec. That would allow it to play nicely with any other client libraries that also have the public option.

@domenic
Copy link
Copy Markdown
Owner

domenic commented Apr 14, 2014

I'm still having trouble understanding. If you don't submit an Authorization header, you're not following the OAuth2 spec. Right?

@ebymatthew
Copy link
Copy Markdown
Author

I don't believe so... http://tools.ietf.org/html/rfc6749#section-2.1 describes the two client types. If every client had to authenticate why would they make a distinction between public and confidential clients?

http://tools.ietf.org/html/rfc6749#section-4.3.2 which describes the access token request for ROPC only requires the client to authenticate if it was confidential:

If the client type is confidential or the client was issued client
credentials (or assigned other authentication requirements), the
client MUST authenticate with the authorization server as described
in Section 3.2.1.

@domenic
Copy link
Copy Markdown
Owner

domenic commented Apr 14, 2014

It sounds like if the client is public, it shouldn't authenticate with the authorization server at all, and so it shouldn't hit the token endpoint at all.

@ebymatthew
Copy link
Copy Markdown
Author

The ultimate goal is not just to authenticate the client... it's also to authenticate the resource owner. The end user's credentials are still protected with this setup since they go straight from the browser to the token endpoint. You need access to the token endpoint to validate the resource owner.

@domenic
Copy link
Copy Markdown
Owner

domenic commented Apr 14, 2014

I think I am starting to see. Fascinating. I never noticed this aspect of the spec before.

Unfortunately I've tied up a lot of my time this weekend with Restify–OAuth2 already; I'm probably an hour away from finishing scope support per #17. That'll be the end of my time budget for this weekend :(.

I'll look into incorporating these changes, maybe some time this week, or more realistically next weekend. Sorry for the delay...

In the meantime, once scope support drops (in an hour or so), you could help by rebasing on top of that. (Not merging.)

@ebymatthew
Copy link
Copy Markdown
Author

Awesome... thanks for sticking with me. No hurry on my part. I'll aim to rebase it tomorrow.

@ebymatthew
Copy link
Copy Markdown
Author

I submitted PR #21 to replace this PR. I believe it to be a better approach to handling public vs. confidential clients.

@ebymatthew ebymatthew closed this Apr 15, 2014
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