OAuth 2.0 draft30 update #644

Merged
merged 5 commits into from Dec 23, 2012

Conversation

Projects
None yet
7 participants
@FantomJAC
Member

FantomJAC commented Aug 22, 2012

These codes support OAuth 2.0 latest specification draft30 for Restlet framework.
The implementation considered that meets minimum requirement to draft30.
I modified 'heavily' latest version 2.2-SNAPSHOT.
I've also support AuthLogin pages which is actually not supported in latest version(2.2-SNAPSHOT).
This includes bug fix for issue #614.

FantomJAC added some commits Jul 27, 2012

OAuth extension
- BugFix: ValidationServerResource response with no error contents when the request is unauthenticated.
- BugFix: Setting ClientStoreImpl with no constructor arguments through ClientStoreFactory#setClientStoreImpl causes NullPointerException at ClientStoreFactory#getInstance.
- Fix the Issue #614.
OAuth 2.0 draft30 implementation.
- Heavily modified version of original OAuth extension from 2.2-SNAPSHOT.
- Support 'Bearer' authentication.
- Move 'Memory' implementations to org.restlet.ext.oauth.internal.memory.
@stoffeg

This comment has been minimized.

Show comment Hide comment
@stoffeg

stoffeg Aug 22, 2012

Member

Hi Shotaro san!

Give me a day or two to review. You seemed to have put in a lot of effort so I need to just make sure everything looks good. Have also some manual tests to FB, Google and MS.

Have you by any chance ran the automated tests or any other tests just to know the cross compatibility?

Regards Stoffe

Member

stoffeg commented Aug 22, 2012

Hi Shotaro san!

Give me a day or two to review. You seemed to have put in a lot of effort so I need to just make sure everything looks good. Have also some manual tests to FB, Google and MS.

Have you by any chance ran the automated tests or any other tests just to know the cross compatibility?

Regards Stoffe

@jlouvel

This comment has been minimized.

Show comment Hide comment
@jlouvel

jlouvel Aug 22, 2012

Owner

Hi Shotaro,

This looks like an awesome contribution indeed, thanks for your efforts!

In addition to Kristoffer handling of technical aspects (code review, actual merge, etc.), I'd like us to get started with the legal side of it to allow us to merge your contribution. Could you look at this page and it is good for you return us a signed JCA at the email address mentioned in the PDF?
http://www.restlet.org/community/contribute

Let me know if you have any question.

Best regards,
Jerome

Owner

jlouvel commented Aug 22, 2012

Hi Shotaro,

This looks like an awesome contribution indeed, thanks for your efforts!

In addition to Kristoffer handling of technical aspects (code review, actual merge, etc.), I'd like us to get started with the legal side of it to allow us to merge your contribution. Could you look at this page and it is good for you return us a signed JCA at the email address mentioned in the PDF?
http://www.restlet.org/community/contribute

Let me know if you have any question.

Best regards,
Jerome

@FantomJAC

This comment has been minimized.

Show comment Hide comment
@FantomJAC

FantomJAC Aug 22, 2012

Member

Hi Stoffe-san

Actually I have not write the automated tests yet. I know we need it, so I will provide those tests soon.
I've also noticed that OAuth 2.0 draft31 has been released, seems few changes are applied to draft30.
I'll continue work on those updates.

Regards,
Shotaro

Member

FantomJAC commented Aug 22, 2012

Hi Stoffe-san

Actually I have not write the automated tests yet. I know we need it, so I will provide those tests soon.
I've also noticed that OAuth 2.0 draft31 has been released, seems few changes are applied to draft30.
I'll continue work on those updates.

Regards,
Shotaro

@FantomJAC

This comment has been minimized.

Show comment Hide comment
@FantomJAC

FantomJAC Aug 22, 2012

Member

Hi Jerome-san

Thanks, I'll send you singned JCA as soon as possible.

Regards,
Shotaro

Member

FantomJAC commented Aug 22, 2012

Hi Jerome-san

Thanks, I'll send you singned JCA as soon as possible.

Regards,
Shotaro

Fixed several bugs (draft30)
- BugFix: Unnecessary duplicated scope is added, when approved scopes contains the same granted scopes.
- BugFix: Paramter "token_type", "expires_in", and "scope" is missing in Access Token Response. (Implicit Grant)
- BugFix: Use the fragment component to send error during Implicit Grant. (4.2.2.1.)
- BugFix: AuthPageServerResource does not catch Throwable, causes HTTP 500 internal server error.
- BugFix: Cache-Control header field with a value of "no-store" is not included in token error response. (5.2)
- BugFix: Parameter "scope" is missing in Access Token Response. (Auth code flow)
- BugFix: Parameter "scope" is not implemented. (Password flow and Refresh flow)
@msvens

This comment has been minimized.

Show comment Hide comment
@msvens

msvens Aug 31, 2012

Member

Hi Shataro-San, I will try to run some testing during the weekend as a complement. Concerning the tests, just build on the ones that are in the test package. Thanks much for the help

cheers,
Martin

Member

msvens commented Aug 31, 2012

Hi Shataro-San, I will try to run some testing during the weekend as a complement. Concerning the tests, just build on the ones that are in the test package. Thanks much for the help

cheers,
Martin

@FantomJAC

This comment has been minimized.

Show comment Hide comment
@FantomJAC

FantomJAC Aug 31, 2012

Member

Hi Jerome-san
I've sent the signed JCA today.

Regards,
Shotaro

Member

FantomJAC commented Aug 31, 2012

Hi Jerome-san
I've sent the signed JCA today.

Regards,
Shotaro

@jlouvel

This comment has been minimized.

Show comment Hide comment
@jlouvel

jlouvel Aug 31, 2012

Owner

Hi Shotaro,

Thanks for sending over your signed JCA. We are all set from this point of view!

Cheers,
Jerome

Owner

jlouvel commented Aug 31, 2012

Hi Shotaro,

Thanks for sending over your signed JCA. We are all set from this point of view!

Cheers,
Jerome

@ghost ghost assigned stoffeg Aug 31, 2012

@lauterry

This comment has been minimized.

Show comment Hide comment
@lauterry

lauterry Sep 2, 2012

Hi

I've noticed that OAuthAuthorizer class is missing in the draft 30 version of Oauth Extension provided byFantomJAC .

Why ?

What should I put before my protected resource now instead of a OAuthAuthorizer ?

Best regards

lauterry commented Sep 2, 2012

Hi

I've noticed that OAuthAuthorizer class is missing in the draft 30 version of Oauth Extension provided byFantomJAC .

Why ?

What should I put before my protected resource now instead of a OAuthAuthorizer ?

Best regards

@FantomJAC

This comment has been minimized.

Show comment Hide comment
@FantomJAC

FantomJAC Sep 3, 2012

Member

Hi

Use TokenVerifier with ChallengeAuthenticator to verify the provided tokens.

OAuthAuthorizer has following issues.

  • OAuthAuthorizer overrides RoleAuthorizer#authorize, breaks RoleAuthorizer's original concept. RoleAuthorizer should verify/authorize only against the user's authenticated roles.
  • OAuthAuthorizer actually handles ChallengeRequests. ChallengeAuthenticator should handle these requests.
  • OAuthAuthorizer sets an access token as an User's secret, which makes no sense.
  • Resource server(User application) never knows the actual user's granted roles.

I simplified the OAuth resource protection architecture. TokenVerifier does not perform more than "verifing".

  • TokenVerifier(which located at the resource server) send request to TokenAuthServerResource(which usually located at the authorization server).
  • TokenAuthServerResource perform "verify" against the provided token, then response with "owner" and "scope" if the token is valid.
  • TokenVerifier set these information to the current ClientInfo. (User and Roles)
  • Leave the user application to decide usage of "owner" and "scope". Simplist way is using RoleAuthorizer.

Here is an example to protect resources using TokenVerifier.

@Override
public synchronized Restlet createInboundRoot() {
    Router router = new Router(getContext());

    router.attach("/status", ServerResource.class);

    RoleAuthorizer roleAuthorizer = new RoleAuthorizer();
    roleAuthorizer.setAuthorizedRoles(Scopes.toRoles("status"));
    roleAuthorizer.setNext(router);

    ChallengeAuthenticator bearerAuthenticator =
            new ChallengeAuthenticator(getContext(),
                TokenVerifier.HTTP_BEARER, "Application");
    bearerAuthenticator.setVerifier(
            new TokenVerifier(new Reference("yourTokenAuthURI")));
    bearerAuthenticator.setNext(roleAuthorizer);

    return bearerAuthenticator;
}

Regards,
Shotaro

Member

FantomJAC commented Sep 3, 2012

Hi

Use TokenVerifier with ChallengeAuthenticator to verify the provided tokens.

OAuthAuthorizer has following issues.

  • OAuthAuthorizer overrides RoleAuthorizer#authorize, breaks RoleAuthorizer's original concept. RoleAuthorizer should verify/authorize only against the user's authenticated roles.
  • OAuthAuthorizer actually handles ChallengeRequests. ChallengeAuthenticator should handle these requests.
  • OAuthAuthorizer sets an access token as an User's secret, which makes no sense.
  • Resource server(User application) never knows the actual user's granted roles.

I simplified the OAuth resource protection architecture. TokenVerifier does not perform more than "verifing".

  • TokenVerifier(which located at the resource server) send request to TokenAuthServerResource(which usually located at the authorization server).
  • TokenAuthServerResource perform "verify" against the provided token, then response with "owner" and "scope" if the token is valid.
  • TokenVerifier set these information to the current ClientInfo. (User and Roles)
  • Leave the user application to decide usage of "owner" and "scope". Simplist way is using RoleAuthorizer.

Here is an example to protect resources using TokenVerifier.

@Override
public synchronized Restlet createInboundRoot() {
    Router router = new Router(getContext());

    router.attach("/status", ServerResource.class);

    RoleAuthorizer roleAuthorizer = new RoleAuthorizer();
    roleAuthorizer.setAuthorizedRoles(Scopes.toRoles("status"));
    roleAuthorizer.setNext(router);

    ChallengeAuthenticator bearerAuthenticator =
            new ChallengeAuthenticator(getContext(),
                TokenVerifier.HTTP_BEARER, "Application");
    bearerAuthenticator.setVerifier(
            new TokenVerifier(new Reference("yourTokenAuthURI")));
    bearerAuthenticator.setNext(roleAuthorizer);

    return bearerAuthenticator;
}

Regards,
Shotaro

@jlouvel

This comment has been minimized.

Show comment Hide comment
@jlouvel

jlouvel Sep 3, 2012

Owner

Thanks Shotaro for the detailed comment. The decoupling of OAuthAuthorizer looks like a sound refactoring to me.

Best regards,
Jerome

Owner

jlouvel commented Sep 3, 2012

Thanks Shotaro for the detailed comment. The decoupling of OAuthAuthorizer looks like a sound refactoring to me.

Best regards,
Jerome

@lauterry

This comment has been minimized.

Show comment Hide comment
@lauterry

lauterry Sep 3, 2012

Hi thank you for the reply.

I encountered the following error :

Challenge scheme HTTP_BEARER not supported by the Restlet engine.

I'm using restlet 2.1 M7

Best regards

lauterry commented Sep 3, 2012

Hi thank you for the reply.

I encountered the following error :

Challenge scheme HTTP_BEARER not supported by the Restlet engine.

I'm using restlet 2.1 M7

Best regards

@FantomJAC

This comment has been minimized.

Show comment Hide comment
@FantomJAC

FantomJAC Sep 3, 2012

Member

Hi,
The message will appear because Helper is not registered to the Engine.
Actually, Bearer auth doesn't require Helper class(use only for raw value), therefore you can ignore the message.
I will put Helper class for MAC auth(which is alternative token usage) for future commit.
Regards,
Shotaro

Member

FantomJAC commented Sep 3, 2012

Hi,
The message will appear because Helper is not registered to the Engine.
Actually, Bearer auth doesn't require Helper class(use only for raw value), therefore you can ignore the message.
I will put Helper class for MAC auth(which is alternative token usage) for future commit.
Regards,
Shotaro

@msvens

This comment has been minimized.

Show comment Hide comment
@msvens

msvens Sep 3, 2012

Member

Agree, very good explanation as to the refactoring (and good design!). What we need to do is to basically provide an upgrade guide so that it is easy for people to migrate. I have not had time to verification yet but one thing is Facebook that I know (last I checked) did only support an old version of the spec - at least before they were waiting to the spec to be finalized. Shotaro, have you verified facebook. What we had was The OAuthAuthenticatorV2 for this, but I think you have removed that in your branch, right?

cheers,
martin

Member

msvens commented Sep 3, 2012

Agree, very good explanation as to the refactoring (and good design!). What we need to do is to basically provide an upgrade guide so that it is easy for people to migrate. I have not had time to verification yet but one thing is Facebook that I know (last I checked) did only support an old version of the spec - at least before they were waiting to the spec to be finalized. Shotaro, have you verified facebook. What we had was The OAuthAuthenticatorV2 for this, but I think you have removed that in your branch, right?

cheers,
martin

@lauterry

This comment has been minimized.

Show comment Hide comment
@lauterry

lauterry Sep 3, 2012

Hi

I agree that an upgraded guide is needed. Unfortunalty, I try to implements OAUTH2.0 draft v30 for a POC but I haven't much time to do it So I intend to keep the draft 10.

One question : Is the OauthProxy class ready to work with your Oauth extension draft v30 ? Does it need to be refined ?

About OauthProxy, please correct me if i'm wrong but I think the case where the access_token is provided is not handledin the OauthProxy , isn't it ?

lauterry commented Sep 3, 2012

Hi

I agree that an upgraded guide is needed. Unfortunalty, I try to implements OAUTH2.0 draft v30 for a POC but I haven't much time to do it So I intend to keep the draft 10.

One question : Is the OauthProxy class ready to work with your Oauth extension draft v30 ? Does it need to be refined ?

About OauthProxy, please correct me if i'm wrong but I think the case where the access_token is provided is not handledin the OauthProxy , isn't it ?

@msvens

This comment has been minimized.

Show comment Hide comment
@msvens

msvens Sep 3, 2012

Member

Now we are talking about the "old" extension right. In that case you are right. You will have to set the clientInfo (i.e the user) yourself. What I typically do is to have a filter that bypasses the OAuthProxy if the token is provided and access the resource directly. If you are not using that kind of solution you end up generating new tokens for every request

Member

msvens commented Sep 3, 2012

Now we are talking about the "old" extension right. In that case you are right. You will have to set the clientInfo (i.e the user) yourself. What I typically do is to have a filter that bypasses the OAuthProxy if the token is provided and access the resource directly. If you are not using that kind of solution you end up generating new tokens for every request

@msvens

This comment has been minimized.

Show comment Hide comment
@msvens

msvens Sep 3, 2012

Member

About your other comment, we are verifying that everything works fine with the new v30 code. I am still digging into the code but from the looks it is still the old one so I guess it needs updating (if at all it will be used)

Member

msvens commented Sep 3, 2012

About your other comment, we are verifying that everything works fine with the new v30 code. I am still digging into the code but from the looks it is still the old one so I guess it needs updating (if at all it will be used)

@lauterry

This comment has been minimized.

Show comment Hide comment
@lauterry

lauterry Sep 3, 2012

Hi msvens

Thank you for your reply.

Indeed i was ending up generating new tokens for every request and thought It was not the correct solution.

About OAuthProxy, it would be great to update it as well so it can work with the v30 code.

Thus, we can developer Oauth provider with Restlet but also Oauth Client.

Please look at my issue #648 . Maybe this issue is still in the v30 code (maybe not).
Please tell me how can I catch the error code 403 or 401 when I attempt to request a protected resource with an invalid access token. What I have is a response with code 200 without an entity.

Thanks a lot

Best regards

lauterry commented Sep 3, 2012

Hi msvens

Thank you for your reply.

Indeed i was ending up generating new tokens for every request and thought It was not the correct solution.

About OAuthProxy, it would be great to update it as well so it can work with the v30 code.

Thus, we can developer Oauth provider with Restlet but also Oauth Client.

Please look at my issue #648 . Maybe this issue is still in the v30 code (maybe not).
Please tell me how can I catch the error code 403 or 401 when I attempt to request a protected resource with an invalid access token. What I have is a response with code 200 without an entity.

Thanks a lot

Best regards

@msvens

This comment has been minimized.

Show comment Hide comment
@msvens

msvens Sep 30, 2012

Member

Shotaro-san, just wanted to check the status with you. Have you had time to verify the new extension against google and facebook (I have not). As soon as we verify those I think we should merge to 2.2. We will also need to work on a migration guide and a solid example for using the extension

cheers,
martin

Member

msvens commented Sep 30, 2012

Shotaro-san, just wanted to check the status with you. Have you had time to verify the new extension against google and facebook (I have not). As soon as we verify those I think we should merge to 2.2. We will also need to work on a migration guide and a solid example for using the extension

cheers,
martin

@FantomJAC

This comment has been minimized.

Show comment Hide comment
@FantomJAC

FantomJAC Oct 2, 2012

Member

Hi,
I'm still working on the tests and the client-side.
(I was busy with another projects last month...)
For server-side, I've almost done the example code. I'll create the repository for that, within a week.
Regards,
Shotaro

Member

FantomJAC commented Oct 2, 2012

Hi,
I'm still working on the tests and the client-side.
(I was busy with another projects last month...)
For server-side, I've almost done the example code. I'll create the repository for that, within a week.
Regards,
Shotaro

@jlouvel

This comment has been minimized.

Show comment Hide comment
@jlouvel

jlouvel Oct 2, 2012

Owner

Hi Shotaro,

That sounds great! For the example, feel free to make a pull request for 'org.restlet.example.ext.oauth' if it can fit in a packages tree.

Cheers,
Jerome

Owner

jlouvel commented Oct 2, 2012

Hi Shotaro,

That sounds great! For the example, feel free to make a pull request for 'org.restlet.example.ext.oauth' if it can fit in a packages tree.

Cheers,
Jerome

@msvens

This comment has been minimized.

Show comment Hide comment
@msvens

msvens Oct 5, 2012

Member

Agree with Jerome and I will not touch the current code for now

Member

msvens commented Oct 5, 2012

Agree with Jerome and I will not touch the current code for now

@FantomJAC

This comment has been minimized.

Show comment Hide comment
@FantomJAC

FantomJAC Oct 10, 2012

Member

Hi
I've just added the server-side example and a bug fix.
The example uses MongoDB Java driver, you need to add the dependency for it while compile.

Member

FantomJAC commented Oct 10, 2012

Hi
I've just added the server-side example and a bug fix.
The example uses MongoDB Java driver, you need to add the dependency for it while compile.

@msvens

This comment has been minimized.

Show comment Hide comment
@msvens

msvens Oct 16, 2012

Member

That is great. And I just saw that OAuth2 is finally approved as an IETF spec

Member

msvens commented Oct 16, 2012

That is great. And I just saw that OAuth2 is finally approved as an IETF spec

@jlouvel

This comment has been minimized.

Show comment Hide comment
@jlouvel

jlouvel Oct 17, 2012

Owner

Excellent! At this point, based on Shorato's latest commits, I would suggest that we move forward with the integration of his pull request into the master branch. Then to list the remaining tasks in the issue tracker (doc update, etc.). Martin, Stoffe, does that work for you?

Owner

jlouvel commented Oct 17, 2012

Excellent! At this point, based on Shorato's latest commits, I would suggest that we move forward with the integration of his pull request into the master branch. Then to list the remaining tasks in the issue tracker (doc update, etc.). Martin, Stoffe, does that work for you?

@msvens

This comment has been minimized.

Show comment Hide comment
@msvens

msvens Oct 18, 2012

Member

This works fine, lets make sure that Shaorato's version conforms to the final spec (which I think it does). Easist is if Shorato makes this check. I also realized the other day that the equals/hashCode methods are actually important for Role since we are storing those in maps. Just to be sure, there are no plans on re-introducing those in Role?

Member

msvens commented Oct 18, 2012

This works fine, lets make sure that Shaorato's version conforms to the final spec (which I think it does). Easist is if Shorato makes this check. I also realized the other day that the equals/hashCode methods are actually important for Role since we are storing those in maps. Just to be sure, there are no plans on re-introducing those in Role?

@thboileau thboileau merged commit 3f72276 into restlet:master Dec 23, 2012

@thboileau

This comment has been minimized.

Show comment Hide comment
@thboileau

thboileau Dec 23, 2012

Owner

Thanks a lot Shaorato for the contribution, it will be part of 2.2m1 release.

I've added the mongo db dependency (I will try to remove it latter?), "turned off" the test cases that did not compile.

Owner

thboileau commented Dec 23, 2012

Thanks a lot Shaorato for the contribution, it will be part of 2.2m1 release.

I've added the mongo db dependency (I will try to remove it latter?), "turned off" the test cases that did not compile.

@jlouvel

This comment has been minimized.

Show comment Hide comment
@jlouvel

jlouvel Dec 23, 2012

Owner

Moved remaining tasks to issue #665 as we can't reopen a pull request.

Owner

jlouvel commented Dec 23, 2012

Moved remaining tasks to issue #665 as we can't reopen a pull request.

@FantomJAC

This comment has been minimized.

Show comment Hide comment
@FantomJAC

FantomJAC Dec 24, 2012

Member

Thanks for the merge, Restlet team.
I'll keep working on the remaining issues.

Member

FantomJAC commented Dec 24, 2012

Thanks for the merge, Restlet team.
I'll keep working on the remaining issues.

@jlouvel

This comment has been minimized.

Show comment Hide comment
@jlouvel

jlouvel Jan 19, 2013

Owner

FYI, I have just restore the Role.equals and hashcode method into 2.2/master branch. See issue #689 for details.

Owner

jlouvel commented Jan 19, 2013

FYI, I have just restore the Role.equals and hashcode method into 2.2/master branch. See issue #689 for details.

@Tembrel

This comment has been minimized.

Show comment Hide comment
@Tembrel

Tembrel Jan 19, 2013

Member

I'm not seeing that on the master in GitHub -- have you not pushed it yet?

Member

Tembrel commented Jan 19, 2013

I'm not seeing that on the master in GitHub -- have you not pushed it yet?

@jlouvel

This comment has been minimized.

Show comment Hide comment
@jlouvel

jlouvel Jan 19, 2013

Owner

The series of commits were pushed and start with this one:
00f45c4

Owner

jlouvel commented Jan 19, 2013

The series of commits were pushed and start with this one:
00f45c4

@jlouvel

This comment has been minimized.

Show comment Hide comment
@jlouvel

jlouvel Sep 19, 2013

Owner

Hello Shorato, just letting you know that we finally released RF 2.2 M5 which includes your latest changes to OAuth extensnion. http://blog.restlet.com/2013/05/02/how-much-rest-should-your-web-api-get/

What are the next steps on your side?

Owner

jlouvel commented Sep 19, 2013

Hello Shorato, just letting you know that we finally released RF 2.2 M5 which includes your latest changes to OAuth extensnion. http://blog.restlet.com/2013/05/02/how-much-rest-should-your-web-api-get/

What are the next steps on your side?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment