Skip to content
This repository has been archived by the owner on Jul 3, 2020. It is now read-only.

Resource Owner Password Credentials Grant #21

Open
ashihaby opened this issue Sep 10, 2013 · 12 comments
Open

Resource Owner Password Credentials Grant #21

ashihaby opened this issue Sep 10, 2013 · 12 comments

Comments

@ashihaby
Copy link
Contributor

Hi,
Do you have a support for the grant type of password which was described in this section of the protocol http://tools.ietf.org/html/draft-ietf-oauth-v2-31#section-4.3 ?

@gvanderploeg
Copy link
Contributor

Hi.
At the moment, no. Apis does not support the Resource Owner credentials grant type.
In our primary use case this is actually not feasible because the type of user authentication is unknown/transparent to the Authorization server.
But further discussion or a implementation suggestion / pull request is welcome of course.

@ashihaby
Copy link
Contributor Author

OK I will try to implement that and send you a pull request.
Thanks.

@gvanderploeg
Copy link
Contributor

Ashihaby,
as you might have seen, user authentication is not handled by the authorization server itself, but delegated to the so called authentication module.
I suggest you first have a look there.
Do you already have in mind how to let the authentication module handle user credentials? See AbstractAuthenticator.java.
We would be most interested to think along with you on this.

@ashihaby
Copy link
Contributor Author

gvanderploeg,
I have started to add the support of User Password Credentials and this is what i did till now
ashihaby@f8b35f1
I have implemented the full path without validating the password, the next step will be vaidating the password through an Authenticator before sending the response.

@ashihaby
Copy link
Contributor Author

BTW I have renamed UserPassCredentials Class to ClientCredentials, I think this name is better as you use it only for Client Authentication.

@gvanderploeg
Copy link
Contributor

Hi,

How are you faring on this issue? It seems that commits for this issue were mixed into the same pull request as #32, so I did not include these in the merge.
So if you completed the implementation by now (including the authenticator support) please open a new pull request.

And by the way: please put the commits for this issue in a separate branch, without other commits. Otherwise the merge could probably get just as painful as the one for #32...
Thanks!

@sfitts
Copy link
Contributor

sfitts commented May 6, 2014

Is there an update on this? I need this grant type as well so if it isn't ready yet I'll have to roll my own version.

@gvanderploeg
Copy link
Contributor

To me it looks like the feature is not complete (without the authenticator support), and there is no proper pull request (or a branch with only the correct commits).
And it looks like @ashihaby has definitely forked from ours (not having pulled in our changes for a long time), so I wonder whether she is willing to feed this feature upstream?

@sfitts
Copy link
Contributor

sfitts commented May 7, 2014

Thanks -- I figured since it had been 8 months since any progress it probably hadn't progressed, but wanted to confirm before I proceeded on my own.

@sfitts
Copy link
Contributor

sfitts commented May 7, 2014

A note on PR #47 -- there is likely more work that could (and arguably should) be done on this. For starters the code which adds the resource owner to the auth server model could be moved into its own package (though I've had issues with splitting data models across packages in the past). Also, I didn't update the Selenium tests to add one for the new grant type and this should certainly be done. In fact I didn't run the Selenium tests at all yet, so I could have broken them.

That said -- this appears to be working pretty well and I'm likely to go back to some other things for a bit.

@FredFrancisIT
Copy link

Hi all !!
Thanks to sfitts, I manage to build my Oauth server with the Resource Owner grant. IT's seems to work Ok so far. But now I have a quick question, so in order for it to work we need to add the resourceowner table to the db, and they will act as Users. But my issue is that I'd like to introduce a "role" to my resourceowner. So I just want to confirm they won't be "design" issue doing that ? And second make sure I understood, I'll have to modify my "authenticator" on the autorization server to add the roles to the authenticatedprincipal ?
Can you confirm ?

Thanks a lot to all the people involved in that project, it make things easier !!

Cheers

@thiagozf
Copy link

PRs #106 and #107 enhance @sfitts's work. Seems to be working fine. Can this issue be closed?

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

No branches or pull requests

5 participants