You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@shindig.apache.org by Adam Clarke <cl...@gmail.com> on 2012/05/01 17:12:43 UTC

Re: Review Request: Jira 1732 and lots of other fixes/enhancements

-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/4947/
-----------------------------------------------------------

(Updated 2012-05-01 15:12:43.842304)


Review request for shindig, Ryan Baxter and Brian Lillie.


Summary
-------

When the original OAuth2 Consumer was added the patch was quite large and it was tough to get a  comprehensive review.  It was expected that there would be a couple of revisions to address problems.

This patch contains the changes for Jira 173.  Due to time constraints it also contains a number of other fixes/enhancements found in internal OAuth2 Consumer reviews and testing.

Some of these changes are interface changes/additions/deletions and could effect custom consumer extensions.  (Probably unlikely because nobody has complained about them yet.)


Patch Includes:
0) More standard formatting and checkstyle in modified files.

1) Jira 1732 for restricted OAuth2 endpoints.

2) Rework of OAuth2Cache and the default InMemoryCache.  Tried to get too cute with the original which made it very hard to implement.  New version is much easier to implement with your own Maps.

3) OAuth2 State encryption/decryption.  Wasn't absolutely necessary for OAuth2, but added it for consistency with OAuth1 and for the peace-of-mind of security types.

4) BasicOAuth2Request no longer sends expired tokens.  The previous impl relied on the OAuth2 Service Provider returning 401 when a expired token was used.  This led to a nasty user experience when a service provider violated the spec and returned something other than 401.  OAuth2 Consumer no longer sends expired tokens and reacts as if the server returned the 401.

5) OAuth2Persister had an unnecessary method to create a token.  This is now handled in the OAuth2Store.

6) Caching and OAuth2Store.init() fixes for better behavior in a clustered environment.

7) Properly handle the URL %scheme% for the redirect uri (aka callback url)


This addresses bug SHINDIG-1732.
    https://issues.apache.org/jira/browse/SHINDIG-1732


Diffs
-----

  http://svn.apache.org/repos/asf/shindig/trunk/java/common/conf/shindig.properties 1332636 
  http://svn.apache.org/repos/asf/shindig/trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/oauth2/BasicOAuth2Accessor.java 1332636 
  http://svn.apache.org/repos/asf/shindig/trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/oauth2/BasicOAuth2Request.java 1332636 
  http://svn.apache.org/repos/asf/shindig/trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/oauth2/BasicOAuth2Store.java 1332636 
  http://svn.apache.org/repos/asf/shindig/trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/oauth2/OAuth2Accessor.java 1332636 
  http://svn.apache.org/repos/asf/shindig/trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/oauth2/OAuth2CallbackState.java PRE-CREATION 
  http://svn.apache.org/repos/asf/shindig/trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/oauth2/OAuth2CallbackStateToken.java PRE-CREATION 
  http://svn.apache.org/repos/asf/shindig/trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/oauth2/OAuth2Error.java 1332636 
  http://svn.apache.org/repos/asf/shindig/trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/oauth2/OAuth2FetcherConfig.java 1332636 
  http://svn.apache.org/repos/asf/shindig/trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/oauth2/OAuth2Message.java 1332636 
  http://svn.apache.org/repos/asf/shindig/trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/oauth2/OAuth2Module.java 1332636 
  http://svn.apache.org/repos/asf/shindig/trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/oauth2/OAuth2Store.java 1332636 
  http://svn.apache.org/repos/asf/shindig/trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/oauth2/OAuth2Token.java 1332636 
  http://svn.apache.org/repos/asf/shindig/trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/oauth2/handler/CodeAuthorizationResponseHandler.java 1332636 
  http://svn.apache.org/repos/asf/shindig/trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/oauth2/handler/CodeGrantTypeHandler.java 1332636 
  http://svn.apache.org/repos/asf/shindig/trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/oauth2/handler/OAuth2HandlerModule.java 1332636 
  http://svn.apache.org/repos/asf/shindig/trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/oauth2/handler/TokenAuthorizationResponseHandler.java 1332636 
  http://svn.apache.org/repos/asf/shindig/trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/oauth2/persistence/MapCache.java PRE-CREATION 
  http://svn.apache.org/repos/asf/shindig/trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/oauth2/persistence/OAuth2Cache.java 1332636 
  http://svn.apache.org/repos/asf/shindig/trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/oauth2/persistence/OAuth2Client.java 1332636 
  http://svn.apache.org/repos/asf/shindig/trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/oauth2/persistence/OAuth2Persister.java 1332636 
  http://svn.apache.org/repos/asf/shindig/trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/oauth2/persistence/OAuth2TokenPersistence.java 1332636 
  http://svn.apache.org/repos/asf/shindig/trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/oauth2/persistence/sample/InMemoryCache.java 1332636 
  http://svn.apache.org/repos/asf/shindig/trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/oauth2/persistence/sample/JSONOAuth2Persister.java 1332636 
  http://svn.apache.org/repos/asf/shindig/trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/servlet/MakeRequestHandler.java 1332636 
  http://svn.apache.org/repos/asf/shindig/trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/servlet/OAuth2CallbackServlet.java 1332636 
  http://svn.apache.org/repos/asf/shindig/trunk/java/gadgets/src/test/java/org/apache/shindig/gadgets/oauth2/MockUtils.java 1332636 
  http://svn.apache.org/repos/asf/shindig/trunk/java/gadgets/src/test/java/org/apache/shindig/gadgets/oauth2/handler/CodeGrantTypeHandlerTest.java 1332636 
  http://svn.apache.org/repos/asf/shindig/trunk/java/gadgets/src/test/java/org/apache/shindig/gadgets/oauth2/handler/TokenAuthorizationResponseHandlerTest.java 1332636 
  http://svn.apache.org/repos/asf/shindig/trunk/java/gadgets/src/test/java/org/apache/shindig/gadgets/oauth2/persistence/sample/InMemoryCacheTest.java 1332636 
  http://svn.apache.org/repos/asf/shindig/trunk/java/gadgets/src/test/java/org/apache/shindig/gadgets/oauth2/persistence/sample/JSONOAuth2PersisterTest.java 1332636 
  http://svn.apache.org/repos/asf/shindig/trunk/java/gadgets/src/test/java/org/apache/shindig/gadgets/servlet/MakeRequestHandlerTest.java 1332636 
  http://svn.apache.org/repos/asf/shindig/trunk/java/gadgets/src/test/resources/org/apache/shindig/gadgets/oauth2/oauth2_test.json 1332636 

Diff: https://reviews.apache.org/r/4947/diff


Testing
-------

All test cases pass.


Thanks,

Adam


Re: Review Request: Jira 1732 and lots of other fixes/enhancements

Posted by br...@us.ibm.com.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/4947/#review7650
-----------------------------------------------------------

Ship it!


Don't see any other issues at this time.

- BrianLillie


On 2012-05-04 14:31:05, Adam Clarke wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/4947/
> -----------------------------------------------------------
> 
> (Updated 2012-05-04 14:31:05)
> 
> 
> Review request for shindig, Ryan Baxter and Brian Lillie.
> 
> 
> Summary
> -------
> 
> When the original OAuth2 Consumer was added the patch was quite large and it was tough to get a  comprehensive review.  It was expected that there would be a couple of revisions to address problems.
> 
> This patch contains the changes for Jira 173.  Due to time constraints it also contains a number of other fixes/enhancements found in internal OAuth2 Consumer reviews and testing.
> 
> Some of these changes are interface changes/additions/deletions and could effect custom consumer extensions.  (Probably unlikely because nobody has complained about them yet.)
> 
> 
> Patch Includes:
> 0) More standard formatting and checkstyle in modified files.
> 
> 1) Jira 1732 for restricted OAuth2 endpoints.
> 
> 2) Rework of OAuth2Cache and the default InMemoryCache.  Tried to get too cute with the original which made it very hard to implement.  New version is much easier to implement with your own Maps.
> 
> 3) OAuth2 State encryption/decryption.  Wasn't absolutely necessary for OAuth2, but added it for consistency with OAuth1 and for the peace-of-mind of security types.
> 
> 4) BasicOAuth2Request no longer sends expired tokens.  The previous impl relied on the OAuth2 Service Provider returning 401 when a expired token was used.  This led to a nasty user experience when a service provider violated the spec and returned something other than 401.  OAuth2 Consumer no longer sends expired tokens and reacts as if the server returned the 401.
> 
> 5) OAuth2Persister had an unnecessary method to create a token.  This is now handled in the OAuth2Store.
> 
> 6) Caching and OAuth2Store.init() fixes for better behavior in a clustered environment.
> 
> 7) Properly handle the URL %scheme% for the redirect uri (aka callback url)
> 
> 
> This addresses bug SHINDIG-1732.
>     https://issues.apache.org/jira/browse/SHINDIG-1732
> 
> 
> Diffs
> -----
> 
>   http://svn.apache.org/repos/asf/shindig/trunk/java/common/conf/shindig.properties 1333970 
>   http://svn.apache.org/repos/asf/shindig/trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/oauth2/BasicOAuth2Accessor.java 1333970 
>   http://svn.apache.org/repos/asf/shindig/trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/oauth2/BasicOAuth2Request.java 1333970 
>   http://svn.apache.org/repos/asf/shindig/trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/oauth2/BasicOAuth2Store.java 1333970 
>   http://svn.apache.org/repos/asf/shindig/trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/oauth2/OAuth2Accessor.java 1333970 
>   http://svn.apache.org/repos/asf/shindig/trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/oauth2/OAuth2CallbackState.java PRE-CREATION 
>   http://svn.apache.org/repos/asf/shindig/trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/oauth2/OAuth2CallbackStateToken.java PRE-CREATION 
>   http://svn.apache.org/repos/asf/shindig/trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/oauth2/OAuth2Error.java 1333970 
>   http://svn.apache.org/repos/asf/shindig/trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/oauth2/OAuth2FetcherConfig.java 1333970 
>   http://svn.apache.org/repos/asf/shindig/trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/oauth2/OAuth2Message.java 1333970 
>   http://svn.apache.org/repos/asf/shindig/trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/oauth2/OAuth2Module.java 1333970 
>   http://svn.apache.org/repos/asf/shindig/trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/oauth2/OAuth2Store.java 1333970 
>   http://svn.apache.org/repos/asf/shindig/trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/oauth2/OAuth2Token.java 1333970 
>   http://svn.apache.org/repos/asf/shindig/trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/oauth2/handler/CodeAuthorizationResponseHandler.java 1333970 
>   http://svn.apache.org/repos/asf/shindig/trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/oauth2/handler/CodeGrantTypeHandler.java 1333970 
>   http://svn.apache.org/repos/asf/shindig/trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/oauth2/handler/OAuth2HandlerModule.java 1333970 
>   http://svn.apache.org/repos/asf/shindig/trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/oauth2/handler/TokenAuthorizationResponseHandler.java 1333970 
>   http://svn.apache.org/repos/asf/shindig/trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/oauth2/persistence/MapCache.java PRE-CREATION 
>   http://svn.apache.org/repos/asf/shindig/trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/oauth2/persistence/OAuth2Cache.java 1333970 
>   http://svn.apache.org/repos/asf/shindig/trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/oauth2/persistence/OAuth2Client.java 1333970 
>   http://svn.apache.org/repos/asf/shindig/trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/oauth2/persistence/OAuth2Persister.java 1333970 
>   http://svn.apache.org/repos/asf/shindig/trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/oauth2/persistence/OAuth2TokenPersistence.java 1333970 
>   http://svn.apache.org/repos/asf/shindig/trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/oauth2/persistence/sample/InMemoryCache.java 1333970 
>   http://svn.apache.org/repos/asf/shindig/trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/oauth2/persistence/sample/JSONOAuth2Persister.java 1333970 
>   http://svn.apache.org/repos/asf/shindig/trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/servlet/MakeRequestHandler.java 1333970 
>   http://svn.apache.org/repos/asf/shindig/trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/servlet/OAuth2CallbackServlet.java 1333970 
>   http://svn.apache.org/repos/asf/shindig/trunk/java/gadgets/src/test/java/org/apache/shindig/gadgets/oauth2/MockUtils.java 1333970 
>   http://svn.apache.org/repos/asf/shindig/trunk/java/gadgets/src/test/java/org/apache/shindig/gadgets/oauth2/handler/CodeGrantTypeHandlerTest.java 1333970 
>   http://svn.apache.org/repos/asf/shindig/trunk/java/gadgets/src/test/java/org/apache/shindig/gadgets/oauth2/handler/TokenAuthorizationResponseHandlerTest.java 1333970 
>   http://svn.apache.org/repos/asf/shindig/trunk/java/gadgets/src/test/java/org/apache/shindig/gadgets/oauth2/persistence/OAuth2ClientTest.java 1333970 
>   http://svn.apache.org/repos/asf/shindig/trunk/java/gadgets/src/test/java/org/apache/shindig/gadgets/oauth2/persistence/sample/InMemoryCacheTest.java 1333970 
>   http://svn.apache.org/repos/asf/shindig/trunk/java/gadgets/src/test/java/org/apache/shindig/gadgets/oauth2/persistence/sample/JSONOAuth2PersisterTest.java 1333970 
>   http://svn.apache.org/repos/asf/shindig/trunk/java/gadgets/src/test/java/org/apache/shindig/gadgets/servlet/MakeRequestHandlerTest.java 1333970 
>   http://svn.apache.org/repos/asf/shindig/trunk/java/gadgets/src/test/resources/org/apache/shindig/gadgets/oauth2/oauth2_test.json 1333970 
> 
> Diff: https://reviews.apache.org/r/4947/diff
> 
> 
> Testing
> -------
> 
> All test cases pass.
> 
> 
> Thanks,
> 
> Adam
> 
>


Re: Review Request: Jira 1732 and lots of other fixes/enhancements

Posted by A Clarke <cl...@gmail.com>.
OAuth2 was added into 2.5.0

Sorry for the migration pains Doug.  Unfortunately the interfaces we needed
to change were broken too badly to make backwards compatible.  On the
bright side the new ones actually work and will be easier to maintain in
the future.

On Mon, Jun 4, 2012 at 2:46 PM, Henry Saputra <he...@gmail.com>wrote:

> Thanks Doug, we will be more careful about making API changes.
>
> Is OAuth2 support is in Shindig 2.0 or is it released for 2.5 release?
> If its in 2.0 we may need to document the API changes in the UPGRADING
> file.
>
> - Henry
>
> On Mon, Jun 4, 2012 at 11:39 AM, daviesd <da...@oclc.org> wrote:
> >>>
> > Some of these changes are interface changes/additions/deletions and could
> > effect custom consumer extensions.  (Probably unlikely because nobody has
> > complained about them yet.)
> > <<
> >
> > Ugh... I should have been keeping up with this.  Yes the interface
> changes
> > affect us big time moving from beta1 to beta2.  So chalk up one
> complaint.
> > :)
> >
> > doug
> >
> >
> > On 5/9/12 8:49 PM, "Stanton Sievers" <si...@gmail.com> wrote:
> >
> >>
> >>
> >> -----------------------------------------------------------
> >> This is an automatically generated e-mail. To reply, visit:
> >> https://reviews.apache.org/r/4947/#review7754
> >> -----------------------------------------------------------
> >>
> >> Ship it!
> >>
> >>
> >> Committed revision 1336461
> >>
> >> Please close the review.
> >>
> >> - Stanton
> >>
> >>
> >> On 2012-05-04 14:31:05, Adam Clarke wrote:
> >>> >
> >>> > -----------------------------------------------------------
> >>> > This is an automatically generated e-mail. To reply, visit:
> >>> > https://reviews.apache.org/r/4947/
> >>> > -----------------------------------------------------------
> >>> >
> >>> > (Updated 2012-05-04 14:31:05)
> >>> >
> >>> >
> >>> > Review request for shindig, Ryan Baxter and Brian Lillie.
> >>> >
> >>> >
> >>> > Summary
> >>> > -------
> >>> >
> >>> > When the original OAuth2 Consumer was added the patch was quite
> large and
> >>> it was tough to get a  comprehensive review.  It was expected that
> there
> >>> would be a couple of revisions to address problems.
> >>> >
> >>> > This patch contains the changes for Jira 173.  Due to time
> constraints it
> >>> also contains a number of other fixes/enhancements found in internal
> OAuth2
> >>> Consumer reviews and testing.
> >>> >
> >>> > Some of these changes are interface changes/additions/deletions and
> could
> >>> effect custom consumer extensions.  (Probably unlikely because nobody
> has
> >>> complained about them yet.)
> >>> >
> >>> >
> >>> > Patch Includes:
> >>> > 0) More standard formatting and checkstyle in modified files.
> >>> >
> >>> > 1) Jira 1732 for restricted OAuth2 endpoints.
> >>> >
> >>> > 2) Rework of OAuth2Cache and the default InMemoryCache.  Tried to
> get too
> >>> cute with the original which made it very hard to implement.  New
> version is
> >>> much easier to implement with your own Maps.
> >>> >
> >>> > 3) OAuth2 State encryption/decryption.  Wasn't absolutely necessary
> for
> >>> OAuth2, but added it for consistency with OAuth1 and for the
> peace-of-mind of
> >>> security types.
> >>> >
> >>> > 4) BasicOAuth2Request no longer sends expired tokens.  The previous
> impl
> >>> relied on the OAuth2 Service Provider returning 401 when a expired
> token was
> >>> used.  This led to a nasty user experience when a service provider
> violated
> >>> the spec and returned something other than 401.  OAuth2 Consumer no
> longer
> >>> sends expired tokens and reacts as if the server returned the 401.
> >>> >
> >>> > 5) OAuth2Persister had an unnecessary method to create a token.
>  This is
> >>> now handled in the OAuth2Store.
> >>> >
> >>> > 6) Caching and OAuth2Store.init() fixes for better behavior in a
> clustered
> >>> environment.
> >>> >
> >>> > 7) Properly handle the URL %scheme% for the redirect uri (aka
> callback url)
> >>> >
> >>> >
> >>> > This addresses bug SHINDIG-1732.
> >>> >     https://issues.apache.org/jira/browse/SHINDIG-1732
> >>> >
> >>> >
> >>> > Diffs
> >>> > -----
> >>> >
> >>> >
> >>>
> http://svn.apache.org/repos/asf/shindig/trunk/java/common/conf/shindig.proper
> >>> ties 1333970
> >>> >
> >>>
> http://svn.apache.org/repos/asf/shindig/trunk/java/gadgets/src/main/java/org/
> >>> apache/shindig/gadgets/oauth2/BasicOAuth2Accessor.java 1333970
> >>> >
> >>>
> http://svn.apache.org/repos/asf/shindig/trunk/java/gadgets/src/main/java/org/
> >>> apache/shindig/gadgets/oauth2/BasicOAuth2Request.java 1333970
> >>> >
> >>>
> http://svn.apache.org/repos/asf/shindig/trunk/java/gadgets/src/main/java/org/
> >>> apache/shindig/gadgets/oauth2/BasicOAuth2Store.java 1333970
> >>> >
> >>>
> http://svn.apache.org/repos/asf/shindig/trunk/java/gadgets/src/main/java/org/
> >>> apache/shindig/gadgets/oauth2/OAuth2Accessor.java 1333970
> >>> >
> >>>
> http://svn.apache.org/repos/asf/shindig/trunk/java/gadgets/src/main/java/org/
> >>> apache/shindig/gadgets/oauth2/OAuth2CallbackState.java PRE-CREATION
> >>> >
> >>>
> http://svn.apache.org/repos/asf/shindig/trunk/java/gadgets/src/main/java/org/
> >>> apache/shindig/gadgets/oauth2/OAuth2CallbackStateToken.java
> PRE-CREATION
> >>> >
> >>>
> http://svn.apache.org/repos/asf/shindig/trunk/java/gadgets/src/main/java/org/
> >>> apache/shindig/gadgets/oauth2/OAuth2Error.java 1333970
> >>> >
> >>>
> http://svn.apache.org/repos/asf/shindig/trunk/java/gadgets/src/main/java/org/
> >>> apache/shindig/gadgets/oauth2/OAuth2FetcherConfig.java 1333970
> >>> >
> >>>
> http://svn.apache.org/repos/asf/shindig/trunk/java/gadgets/src/main/java/org/
> >>> apache/shindig/gadgets/oauth2/OAuth2Message.java 1333970
> >>> >
> >>>
> http://svn.apache.org/repos/asf/shindig/trunk/java/gadgets/src/main/java/org/
> >>> apache/shindig/gadgets/oauth2/OAuth2Module.java 1333970
> >>> >
> >>>
> http://svn.apache.org/repos/asf/shindig/trunk/java/gadgets/src/main/java/org/
> >>> apache/shindig/gadgets/oauth2/OAuth2Store.java 1333970
> >>> >
> >>>
> http://svn.apache.org/repos/asf/shindig/trunk/java/gadgets/src/main/java/org/
> >>> apache/shindig/gadgets/oauth2/OAuth2Token.java 1333970
> >>> >
> >>>
> http://svn.apache.org/repos/asf/shindig/trunk/java/gadgets/src/main/java/org/
> >>>
> apache/shindig/gadgets/oauth2/handler/CodeAuthorizationResponseHandler.java
> >>> 1333970
> >>> >
> >>>
> http://svn.apache.org/repos/asf/shindig/trunk/java/gadgets/src/main/java/org/
> >>> apache/shindig/gadgets/oauth2/handler/CodeGrantTypeHandler.java 1333970
> >>> >
> >>>
> http://svn.apache.org/repos/asf/shindig/trunk/java/gadgets/src/main/java/org/
> >>> apache/shindig/gadgets/oauth2/handler/OAuth2HandlerModule.java 1333970
> >>> >
> >>>
> http://svn.apache.org/repos/asf/shindig/trunk/java/gadgets/src/main/java/org/
> >>>
> apache/shindig/gadgets/oauth2/handler/TokenAuthorizationResponseHandler.java
> >>> 1333970
> >>> >
> >>>
> http://svn.apache.org/repos/asf/shindig/trunk/java/gadgets/src/main/java/org/
> >>> apache/shindig/gadgets/oauth2/persistence/MapCache.java PRE-CREATION
> >>> >
> >>>
> http://svn.apache.org/repos/asf/shindig/trunk/java/gadgets/src/main/java/org/
> >>> apache/shindig/gadgets/oauth2/persistence/OAuth2Cache.java 1333970
> >>> >
> >>>
> http://svn.apache.org/repos/asf/shindig/trunk/java/gadgets/src/main/java/org/
> >>> apache/shindig/gadgets/oauth2/persistence/OAuth2Client.java 1333970
> >>> >
> >>>
> http://svn.apache.org/repos/asf/shindig/trunk/java/gadgets/src/main/java/org/
> >>> apache/shindig/gadgets/oauth2/persistence/OAuth2Persister.java 1333970
> >>> >
> >>>
> http://svn.apache.org/repos/asf/shindig/trunk/java/gadgets/src/main/java/org/
> >>> apache/shindig/gadgets/oauth2/persistence/OAuth2TokenPersistence.java
> 1333970
> >>> >
> >>>
> http://svn.apache.org/repos/asf/shindig/trunk/java/gadgets/src/main/java/org/
> >>> apache/shindig/gadgets/oauth2/persistence/sample/InMemoryCache.java
> 1333970
> >>> >
> >>>
> http://svn.apache.org/repos/asf/shindig/trunk/java/gadgets/src/main/java/org/
> >>>
> apache/shindig/gadgets/oauth2/persistence/sample/JSONOAuth2Persister.java
> >>> 1333970
> >>> >
> >>>
> http://svn.apache.org/repos/asf/shindig/trunk/java/gadgets/src/main/java/org/
> >>> apache/shindig/gadgets/servlet/MakeRequestHandler.java 1333970
> >>> >
> >>>
> http://svn.apache.org/repos/asf/shindig/trunk/java/gadgets/src/main/java/org/
> >>> apache/shindig/gadgets/servlet/OAuth2CallbackServlet.java 1333970
> >>> >
> >>>
> http://svn.apache.org/repos/asf/shindig/trunk/java/gadgets/src/test/java/org/
> >>> apache/shindig/gadgets/oauth2/MockUtils.java 1333970
> >>> >
> >>>
> http://svn.apache.org/repos/asf/shindig/trunk/java/gadgets/src/test/java/org/
> >>> apache/shindig/gadgets/oauth2/handler/CodeGrantTypeHandlerTest.java
> 1333970
> >>> >
> >>>
> http://svn.apache.org/repos/asf/shindig/trunk/java/gadgets/src/test/java/org/
> >>>
> apache/shindig/gadgets/oauth2/handler/TokenAuthorizationResponseHandlerTest.j
> >>> ava 1333970
> >>> >
> >>>
> http://svn.apache.org/repos/asf/shindig/trunk/java/gadgets/src/test/java/org/
> >>> apache/shindig/gadgets/oauth2/persistence/OAuth2ClientTest.java 1333970
> >>> >
> >>>
> http://svn.apache.org/repos/asf/shindig/trunk/java/gadgets/src/test/java/org/
> >>> apache/shindig/gadgets/oauth2/persistence/sample/InMemoryCacheTest.java
> >>> 1333970
> >>> >
> >>>
> http://svn.apache.org/repos/asf/shindig/trunk/java/gadgets/src/test/java/org/
> >>>
> apache/shindig/gadgets/oauth2/persistence/sample/JSONOAuth2PersisterTest.java
> >>> 1333970
> >>> >
> >>>
> http://svn.apache.org/repos/asf/shindig/trunk/java/gadgets/src/test/java/org/
> >>> apache/shindig/gadgets/servlet/MakeRequestHandlerTest.java 1333970
> >>> >
> >>>
> http://svn.apache.org/repos/asf/shindig/trunk/java/gadgets/src/test/resources
> >>> /org/apache/shindig/gadgets/oauth2/oauth2_test.json 1333970
> >>> >
> >>> > Diff: https://reviews.apache.org/r/4947/diff
> >>> >
> >>> >
> >>> > Testing
> >>> > -------
> >>> >
> >>> > All test cases pass.
> >>> >
> >>> >
> >>> > Thanks,
> >>> >
> >>> > Adam
> >>> >
> >>> >
> >>
> >>
> >
>

Re: Review Request: Jira 1732 and lots of other fixes/enhancements

Posted by Henry Saputra <he...@gmail.com>.
Thanks Doug, we will be more careful about making API changes.

Is OAuth2 support is in Shindig 2.0 or is it released for 2.5 release?
If its in 2.0 we may need to document the API changes in the UPGRADING
file.

- Henry

On Mon, Jun 4, 2012 at 11:39 AM, daviesd <da...@oclc.org> wrote:
>>>
> Some of these changes are interface changes/additions/deletions and could
> effect custom consumer extensions.  (Probably unlikely because nobody has
> complained about them yet.)
> <<
>
> Ugh... I should have been keeping up with this.  Yes the interface changes
> affect us big time moving from beta1 to beta2.  So chalk up one complaint.
> :)
>
> doug
>
>
> On 5/9/12 8:49 PM, "Stanton Sievers" <si...@gmail.com> wrote:
>
>>
>>
>> -----------------------------------------------------------
>> This is an automatically generated e-mail. To reply, visit:
>> https://reviews.apache.org/r/4947/#review7754
>> -----------------------------------------------------------
>>
>> Ship it!
>>
>>
>> Committed revision 1336461
>>
>> Please close the review.
>>
>> - Stanton
>>
>>
>> On 2012-05-04 14:31:05, Adam Clarke wrote:
>>> >
>>> > -----------------------------------------------------------
>>> > This is an automatically generated e-mail. To reply, visit:
>>> > https://reviews.apache.org/r/4947/
>>> > -----------------------------------------------------------
>>> >
>>> > (Updated 2012-05-04 14:31:05)
>>> >
>>> >
>>> > Review request for shindig, Ryan Baxter and Brian Lillie.
>>> >
>>> >
>>> > Summary
>>> > -------
>>> >
>>> > When the original OAuth2 Consumer was added the patch was quite large and
>>> it was tough to get a  comprehensive review.  It was expected that there
>>> would be a couple of revisions to address problems.
>>> >
>>> > This patch contains the changes for Jira 173.  Due to time constraints it
>>> also contains a number of other fixes/enhancements found in internal OAuth2
>>> Consumer reviews and testing.
>>> >
>>> > Some of these changes are interface changes/additions/deletions and could
>>> effect custom consumer extensions.  (Probably unlikely because nobody has
>>> complained about them yet.)
>>> >
>>> >
>>> > Patch Includes:
>>> > 0) More standard formatting and checkstyle in modified files.
>>> >
>>> > 1) Jira 1732 for restricted OAuth2 endpoints.
>>> >
>>> > 2) Rework of OAuth2Cache and the default InMemoryCache.  Tried to get too
>>> cute with the original which made it very hard to implement.  New version is
>>> much easier to implement with your own Maps.
>>> >
>>> > 3) OAuth2 State encryption/decryption.  Wasn't absolutely necessary for
>>> OAuth2, but added it for consistency with OAuth1 and for the peace-of-mind of
>>> security types.
>>> >
>>> > 4) BasicOAuth2Request no longer sends expired tokens.  The previous impl
>>> relied on the OAuth2 Service Provider returning 401 when a expired token was
>>> used.  This led to a nasty user experience when a service provider violated
>>> the spec and returned something other than 401.  OAuth2 Consumer no longer
>>> sends expired tokens and reacts as if the server returned the 401.
>>> >
>>> > 5) OAuth2Persister had an unnecessary method to create a token.  This is
>>> now handled in the OAuth2Store.
>>> >
>>> > 6) Caching and OAuth2Store.init() fixes for better behavior in a clustered
>>> environment.
>>> >
>>> > 7) Properly handle the URL %scheme% for the redirect uri (aka callback url)
>>> >
>>> >
>>> > This addresses bug SHINDIG-1732.
>>> >     https://issues.apache.org/jira/browse/SHINDIG-1732
>>> >
>>> >
>>> > Diffs
>>> > -----
>>> >
>>> >
>>> http://svn.apache.org/repos/asf/shindig/trunk/java/common/conf/shindig.proper
>>> ties 1333970
>>> >
>>> http://svn.apache.org/repos/asf/shindig/trunk/java/gadgets/src/main/java/org/
>>> apache/shindig/gadgets/oauth2/BasicOAuth2Accessor.java 1333970
>>> >
>>> http://svn.apache.org/repos/asf/shindig/trunk/java/gadgets/src/main/java/org/
>>> apache/shindig/gadgets/oauth2/BasicOAuth2Request.java 1333970
>>> >
>>> http://svn.apache.org/repos/asf/shindig/trunk/java/gadgets/src/main/java/org/
>>> apache/shindig/gadgets/oauth2/BasicOAuth2Store.java 1333970
>>> >
>>> http://svn.apache.org/repos/asf/shindig/trunk/java/gadgets/src/main/java/org/
>>> apache/shindig/gadgets/oauth2/OAuth2Accessor.java 1333970
>>> >
>>> http://svn.apache.org/repos/asf/shindig/trunk/java/gadgets/src/main/java/org/
>>> apache/shindig/gadgets/oauth2/OAuth2CallbackState.java PRE-CREATION
>>> >
>>> http://svn.apache.org/repos/asf/shindig/trunk/java/gadgets/src/main/java/org/
>>> apache/shindig/gadgets/oauth2/OAuth2CallbackStateToken.java PRE-CREATION
>>> >
>>> http://svn.apache.org/repos/asf/shindig/trunk/java/gadgets/src/main/java/org/
>>> apache/shindig/gadgets/oauth2/OAuth2Error.java 1333970
>>> >
>>> http://svn.apache.org/repos/asf/shindig/trunk/java/gadgets/src/main/java/org/
>>> apache/shindig/gadgets/oauth2/OAuth2FetcherConfig.java 1333970
>>> >
>>> http://svn.apache.org/repos/asf/shindig/trunk/java/gadgets/src/main/java/org/
>>> apache/shindig/gadgets/oauth2/OAuth2Message.java 1333970
>>> >
>>> http://svn.apache.org/repos/asf/shindig/trunk/java/gadgets/src/main/java/org/
>>> apache/shindig/gadgets/oauth2/OAuth2Module.java 1333970
>>> >
>>> http://svn.apache.org/repos/asf/shindig/trunk/java/gadgets/src/main/java/org/
>>> apache/shindig/gadgets/oauth2/OAuth2Store.java 1333970
>>> >
>>> http://svn.apache.org/repos/asf/shindig/trunk/java/gadgets/src/main/java/org/
>>> apache/shindig/gadgets/oauth2/OAuth2Token.java 1333970
>>> >
>>> http://svn.apache.org/repos/asf/shindig/trunk/java/gadgets/src/main/java/org/
>>> apache/shindig/gadgets/oauth2/handler/CodeAuthorizationResponseHandler.java
>>> 1333970
>>> >
>>> http://svn.apache.org/repos/asf/shindig/trunk/java/gadgets/src/main/java/org/
>>> apache/shindig/gadgets/oauth2/handler/CodeGrantTypeHandler.java 1333970
>>> >
>>> http://svn.apache.org/repos/asf/shindig/trunk/java/gadgets/src/main/java/org/
>>> apache/shindig/gadgets/oauth2/handler/OAuth2HandlerModule.java 1333970
>>> >
>>> http://svn.apache.org/repos/asf/shindig/trunk/java/gadgets/src/main/java/org/
>>> apache/shindig/gadgets/oauth2/handler/TokenAuthorizationResponseHandler.java
>>> 1333970
>>> >
>>> http://svn.apache.org/repos/asf/shindig/trunk/java/gadgets/src/main/java/org/
>>> apache/shindig/gadgets/oauth2/persistence/MapCache.java PRE-CREATION
>>> >
>>> http://svn.apache.org/repos/asf/shindig/trunk/java/gadgets/src/main/java/org/
>>> apache/shindig/gadgets/oauth2/persistence/OAuth2Cache.java 1333970
>>> >
>>> http://svn.apache.org/repos/asf/shindig/trunk/java/gadgets/src/main/java/org/
>>> apache/shindig/gadgets/oauth2/persistence/OAuth2Client.java 1333970
>>> >
>>> http://svn.apache.org/repos/asf/shindig/trunk/java/gadgets/src/main/java/org/
>>> apache/shindig/gadgets/oauth2/persistence/OAuth2Persister.java 1333970
>>> >
>>> http://svn.apache.org/repos/asf/shindig/trunk/java/gadgets/src/main/java/org/
>>> apache/shindig/gadgets/oauth2/persistence/OAuth2TokenPersistence.java 1333970
>>> >
>>> http://svn.apache.org/repos/asf/shindig/trunk/java/gadgets/src/main/java/org/
>>> apache/shindig/gadgets/oauth2/persistence/sample/InMemoryCache.java 1333970
>>> >
>>> http://svn.apache.org/repos/asf/shindig/trunk/java/gadgets/src/main/java/org/
>>> apache/shindig/gadgets/oauth2/persistence/sample/JSONOAuth2Persister.java
>>> 1333970
>>> >
>>> http://svn.apache.org/repos/asf/shindig/trunk/java/gadgets/src/main/java/org/
>>> apache/shindig/gadgets/servlet/MakeRequestHandler.java 1333970
>>> >
>>> http://svn.apache.org/repos/asf/shindig/trunk/java/gadgets/src/main/java/org/
>>> apache/shindig/gadgets/servlet/OAuth2CallbackServlet.java 1333970
>>> >
>>> http://svn.apache.org/repos/asf/shindig/trunk/java/gadgets/src/test/java/org/
>>> apache/shindig/gadgets/oauth2/MockUtils.java 1333970
>>> >
>>> http://svn.apache.org/repos/asf/shindig/trunk/java/gadgets/src/test/java/org/
>>> apache/shindig/gadgets/oauth2/handler/CodeGrantTypeHandlerTest.java 1333970
>>> >
>>> http://svn.apache.org/repos/asf/shindig/trunk/java/gadgets/src/test/java/org/
>>> apache/shindig/gadgets/oauth2/handler/TokenAuthorizationResponseHandlerTest.j
>>> ava 1333970
>>> >
>>> http://svn.apache.org/repos/asf/shindig/trunk/java/gadgets/src/test/java/org/
>>> apache/shindig/gadgets/oauth2/persistence/OAuth2ClientTest.java 1333970
>>> >
>>> http://svn.apache.org/repos/asf/shindig/trunk/java/gadgets/src/test/java/org/
>>> apache/shindig/gadgets/oauth2/persistence/sample/InMemoryCacheTest.java
>>> 1333970
>>> >
>>> http://svn.apache.org/repos/asf/shindig/trunk/java/gadgets/src/test/java/org/
>>> apache/shindig/gadgets/oauth2/persistence/sample/JSONOAuth2PersisterTest.java
>>> 1333970
>>> >
>>> http://svn.apache.org/repos/asf/shindig/trunk/java/gadgets/src/test/java/org/
>>> apache/shindig/gadgets/servlet/MakeRequestHandlerTest.java 1333970
>>> >
>>> http://svn.apache.org/repos/asf/shindig/trunk/java/gadgets/src/test/resources
>>> /org/apache/shindig/gadgets/oauth2/oauth2_test.json 1333970
>>> >
>>> > Diff: https://reviews.apache.org/r/4947/diff
>>> >
>>> >
>>> > Testing
>>> > -------
>>> >
>>> > All test cases pass.
>>> >
>>> >
>>> > Thanks,
>>> >
>>> > Adam
>>> >
>>> >
>>
>>
>

Re: Review Request: Jira 1732 and lots of other fixes/enhancements

Posted by daviesd <da...@oclc.org>.
>>
Some of these changes are interface changes/additions/deletions and could
effect custom consumer extensions.  (Probably unlikely because nobody has
complained about them yet.)
<<

Ugh... I should have been keeping up with this.  Yes the interface changes
affect us big time moving from beta1 to beta2.  So chalk up one complaint.
:)

doug


On 5/9/12 8:49 PM, "Stanton Sievers" <si...@gmail.com> wrote:

> 
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/4947/#review7754
> -----------------------------------------------------------
> 
> Ship it!
> 
> 
> Committed revision 1336461
> 
> Please close the review.
> 
> - Stanton
> 
> 
> On 2012-05-04 14:31:05, Adam Clarke wrote:
>> >
>> > -----------------------------------------------------------
>> > This is an automatically generated e-mail. To reply, visit:
>> > https://reviews.apache.org/r/4947/
>> > -----------------------------------------------------------
>> >
>> > (Updated 2012-05-04 14:31:05)
>> >
>> >
>> > Review request for shindig, Ryan Baxter and Brian Lillie.
>> >
>> >
>> > Summary
>> > -------
>> >
>> > When the original OAuth2 Consumer was added the patch was quite large and
>> it was tough to get a  comprehensive review.  It was expected that there
>> would be a couple of revisions to address problems.
>> >
>> > This patch contains the changes for Jira 173.  Due to time constraints it
>> also contains a number of other fixes/enhancements found in internal OAuth2
>> Consumer reviews and testing.
>> >
>> > Some of these changes are interface changes/additions/deletions and could
>> effect custom consumer extensions.  (Probably unlikely because nobody has
>> complained about them yet.)
>> >
>> >
>> > Patch Includes:
>> > 0) More standard formatting and checkstyle in modified files.
>> >
>> > 1) Jira 1732 for restricted OAuth2 endpoints.
>> >
>> > 2) Rework of OAuth2Cache and the default InMemoryCache.  Tried to get too
>> cute with the original which made it very hard to implement.  New version is
>> much easier to implement with your own Maps.
>> >
>> > 3) OAuth2 State encryption/decryption.  Wasn't absolutely necessary for
>> OAuth2, but added it for consistency with OAuth1 and for the peace-of-mind of
>> security types.
>> >
>> > 4) BasicOAuth2Request no longer sends expired tokens.  The previous impl
>> relied on the OAuth2 Service Provider returning 401 when a expired token was
>> used.  This led to a nasty user experience when a service provider violated
>> the spec and returned something other than 401.  OAuth2 Consumer no longer
>> sends expired tokens and reacts as if the server returned the 401.
>> >
>> > 5) OAuth2Persister had an unnecessary method to create a token.  This is
>> now handled in the OAuth2Store.
>> >
>> > 6) Caching and OAuth2Store.init() fixes for better behavior in a clustered
>> environment.
>> >
>> > 7) Properly handle the URL %scheme% for the redirect uri (aka callback url)
>> >
>> >
>> > This addresses bug SHINDIG-1732.
>> >     https://issues.apache.org/jira/browse/SHINDIG-1732
>> >
>> >
>> > Diffs
>> > -----
>> >
>> >   
>> http://svn.apache.org/repos/asf/shindig/trunk/java/common/conf/shindig.proper
>> ties 1333970
>> >   
>> http://svn.apache.org/repos/asf/shindig/trunk/java/gadgets/src/main/java/org/
>> apache/shindig/gadgets/oauth2/BasicOAuth2Accessor.java 1333970
>> >   
>> http://svn.apache.org/repos/asf/shindig/trunk/java/gadgets/src/main/java/org/
>> apache/shindig/gadgets/oauth2/BasicOAuth2Request.java 1333970
>> >   
>> http://svn.apache.org/repos/asf/shindig/trunk/java/gadgets/src/main/java/org/
>> apache/shindig/gadgets/oauth2/BasicOAuth2Store.java 1333970
>> >   
>> http://svn.apache.org/repos/asf/shindig/trunk/java/gadgets/src/main/java/org/
>> apache/shindig/gadgets/oauth2/OAuth2Accessor.java 1333970
>> >   
>> http://svn.apache.org/repos/asf/shindig/trunk/java/gadgets/src/main/java/org/
>> apache/shindig/gadgets/oauth2/OAuth2CallbackState.java PRE-CREATION
>> >   
>> http://svn.apache.org/repos/asf/shindig/trunk/java/gadgets/src/main/java/org/
>> apache/shindig/gadgets/oauth2/OAuth2CallbackStateToken.java PRE-CREATION
>> >   
>> http://svn.apache.org/repos/asf/shindig/trunk/java/gadgets/src/main/java/org/
>> apache/shindig/gadgets/oauth2/OAuth2Error.java 1333970
>> >   
>> http://svn.apache.org/repos/asf/shindig/trunk/java/gadgets/src/main/java/org/
>> apache/shindig/gadgets/oauth2/OAuth2FetcherConfig.java 1333970
>> >   
>> http://svn.apache.org/repos/asf/shindig/trunk/java/gadgets/src/main/java/org/
>> apache/shindig/gadgets/oauth2/OAuth2Message.java 1333970
>> >   
>> http://svn.apache.org/repos/asf/shindig/trunk/java/gadgets/src/main/java/org/
>> apache/shindig/gadgets/oauth2/OAuth2Module.java 1333970
>> >   
>> http://svn.apache.org/repos/asf/shindig/trunk/java/gadgets/src/main/java/org/
>> apache/shindig/gadgets/oauth2/OAuth2Store.java 1333970
>> >   
>> http://svn.apache.org/repos/asf/shindig/trunk/java/gadgets/src/main/java/org/
>> apache/shindig/gadgets/oauth2/OAuth2Token.java 1333970
>> >   
>> http://svn.apache.org/repos/asf/shindig/trunk/java/gadgets/src/main/java/org/
>> apache/shindig/gadgets/oauth2/handler/CodeAuthorizationResponseHandler.java
>> 1333970
>> >   
>> http://svn.apache.org/repos/asf/shindig/trunk/java/gadgets/src/main/java/org/
>> apache/shindig/gadgets/oauth2/handler/CodeGrantTypeHandler.java 1333970
>> >   
>> http://svn.apache.org/repos/asf/shindig/trunk/java/gadgets/src/main/java/org/
>> apache/shindig/gadgets/oauth2/handler/OAuth2HandlerModule.java 1333970
>> >   
>> http://svn.apache.org/repos/asf/shindig/trunk/java/gadgets/src/main/java/org/
>> apache/shindig/gadgets/oauth2/handler/TokenAuthorizationResponseHandler.java
>> 1333970
>> >   
>> http://svn.apache.org/repos/asf/shindig/trunk/java/gadgets/src/main/java/org/
>> apache/shindig/gadgets/oauth2/persistence/MapCache.java PRE-CREATION
>> >   
>> http://svn.apache.org/repos/asf/shindig/trunk/java/gadgets/src/main/java/org/
>> apache/shindig/gadgets/oauth2/persistence/OAuth2Cache.java 1333970
>> >   
>> http://svn.apache.org/repos/asf/shindig/trunk/java/gadgets/src/main/java/org/
>> apache/shindig/gadgets/oauth2/persistence/OAuth2Client.java 1333970
>> >   
>> http://svn.apache.org/repos/asf/shindig/trunk/java/gadgets/src/main/java/org/
>> apache/shindig/gadgets/oauth2/persistence/OAuth2Persister.java 1333970
>> >   
>> http://svn.apache.org/repos/asf/shindig/trunk/java/gadgets/src/main/java/org/
>> apache/shindig/gadgets/oauth2/persistence/OAuth2TokenPersistence.java 1333970
>> >   
>> http://svn.apache.org/repos/asf/shindig/trunk/java/gadgets/src/main/java/org/
>> apache/shindig/gadgets/oauth2/persistence/sample/InMemoryCache.java 1333970
>> >   
>> http://svn.apache.org/repos/asf/shindig/trunk/java/gadgets/src/main/java/org/
>> apache/shindig/gadgets/oauth2/persistence/sample/JSONOAuth2Persister.java
>> 1333970
>> >   
>> http://svn.apache.org/repos/asf/shindig/trunk/java/gadgets/src/main/java/org/
>> apache/shindig/gadgets/servlet/MakeRequestHandler.java 1333970
>> >   
>> http://svn.apache.org/repos/asf/shindig/trunk/java/gadgets/src/main/java/org/
>> apache/shindig/gadgets/servlet/OAuth2CallbackServlet.java 1333970
>> >   
>> http://svn.apache.org/repos/asf/shindig/trunk/java/gadgets/src/test/java/org/
>> apache/shindig/gadgets/oauth2/MockUtils.java 1333970
>> >   
>> http://svn.apache.org/repos/asf/shindig/trunk/java/gadgets/src/test/java/org/
>> apache/shindig/gadgets/oauth2/handler/CodeGrantTypeHandlerTest.java 1333970
>> >   
>> http://svn.apache.org/repos/asf/shindig/trunk/java/gadgets/src/test/java/org/
>> apache/shindig/gadgets/oauth2/handler/TokenAuthorizationResponseHandlerTest.j
>> ava 1333970
>> >   
>> http://svn.apache.org/repos/asf/shindig/trunk/java/gadgets/src/test/java/org/
>> apache/shindig/gadgets/oauth2/persistence/OAuth2ClientTest.java 1333970
>> >   
>> http://svn.apache.org/repos/asf/shindig/trunk/java/gadgets/src/test/java/org/
>> apache/shindig/gadgets/oauth2/persistence/sample/InMemoryCacheTest.java
>> 1333970
>> >   
>> http://svn.apache.org/repos/asf/shindig/trunk/java/gadgets/src/test/java/org/
>> apache/shindig/gadgets/oauth2/persistence/sample/JSONOAuth2PersisterTest.java
>> 1333970
>> >   
>> http://svn.apache.org/repos/asf/shindig/trunk/java/gadgets/src/test/java/org/
>> apache/shindig/gadgets/servlet/MakeRequestHandlerTest.java 1333970
>> >   
>> http://svn.apache.org/repos/asf/shindig/trunk/java/gadgets/src/test/resources
>> /org/apache/shindig/gadgets/oauth2/oauth2_test.json 1333970
>> >
>> > Diff: https://reviews.apache.org/r/4947/diff
>> >
>> >
>> > Testing
>> > -------
>> >
>> > All test cases pass.
>> >
>> >
>> > Thanks,
>> >
>> > Adam
>> >
>> >
> 
> 


Re: Review Request: Jira 1732 and lots of other fixes/enhancements

Posted by Stanton Sievers <si...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/4947/#review7754
-----------------------------------------------------------

Ship it!


Committed revision 1336461

Please close the review.

- Stanton


On 2012-05-04 14:31:05, Adam Clarke wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/4947/
> -----------------------------------------------------------
> 
> (Updated 2012-05-04 14:31:05)
> 
> 
> Review request for shindig, Ryan Baxter and Brian Lillie.
> 
> 
> Summary
> -------
> 
> When the original OAuth2 Consumer was added the patch was quite large and it was tough to get a  comprehensive review.  It was expected that there would be a couple of revisions to address problems.
> 
> This patch contains the changes for Jira 173.  Due to time constraints it also contains a number of other fixes/enhancements found in internal OAuth2 Consumer reviews and testing.
> 
> Some of these changes are interface changes/additions/deletions and could effect custom consumer extensions.  (Probably unlikely because nobody has complained about them yet.)
> 
> 
> Patch Includes:
> 0) More standard formatting and checkstyle in modified files.
> 
> 1) Jira 1732 for restricted OAuth2 endpoints.
> 
> 2) Rework of OAuth2Cache and the default InMemoryCache.  Tried to get too cute with the original which made it very hard to implement.  New version is much easier to implement with your own Maps.
> 
> 3) OAuth2 State encryption/decryption.  Wasn't absolutely necessary for OAuth2, but added it for consistency with OAuth1 and for the peace-of-mind of security types.
> 
> 4) BasicOAuth2Request no longer sends expired tokens.  The previous impl relied on the OAuth2 Service Provider returning 401 when a expired token was used.  This led to a nasty user experience when a service provider violated the spec and returned something other than 401.  OAuth2 Consumer no longer sends expired tokens and reacts as if the server returned the 401.
> 
> 5) OAuth2Persister had an unnecessary method to create a token.  This is now handled in the OAuth2Store.
> 
> 6) Caching and OAuth2Store.init() fixes for better behavior in a clustered environment.
> 
> 7) Properly handle the URL %scheme% for the redirect uri (aka callback url)
> 
> 
> This addresses bug SHINDIG-1732.
>     https://issues.apache.org/jira/browse/SHINDIG-1732
> 
> 
> Diffs
> -----
> 
>   http://svn.apache.org/repos/asf/shindig/trunk/java/common/conf/shindig.properties 1333970 
>   http://svn.apache.org/repos/asf/shindig/trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/oauth2/BasicOAuth2Accessor.java 1333970 
>   http://svn.apache.org/repos/asf/shindig/trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/oauth2/BasicOAuth2Request.java 1333970 
>   http://svn.apache.org/repos/asf/shindig/trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/oauth2/BasicOAuth2Store.java 1333970 
>   http://svn.apache.org/repos/asf/shindig/trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/oauth2/OAuth2Accessor.java 1333970 
>   http://svn.apache.org/repos/asf/shindig/trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/oauth2/OAuth2CallbackState.java PRE-CREATION 
>   http://svn.apache.org/repos/asf/shindig/trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/oauth2/OAuth2CallbackStateToken.java PRE-CREATION 
>   http://svn.apache.org/repos/asf/shindig/trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/oauth2/OAuth2Error.java 1333970 
>   http://svn.apache.org/repos/asf/shindig/trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/oauth2/OAuth2FetcherConfig.java 1333970 
>   http://svn.apache.org/repos/asf/shindig/trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/oauth2/OAuth2Message.java 1333970 
>   http://svn.apache.org/repos/asf/shindig/trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/oauth2/OAuth2Module.java 1333970 
>   http://svn.apache.org/repos/asf/shindig/trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/oauth2/OAuth2Store.java 1333970 
>   http://svn.apache.org/repos/asf/shindig/trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/oauth2/OAuth2Token.java 1333970 
>   http://svn.apache.org/repos/asf/shindig/trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/oauth2/handler/CodeAuthorizationResponseHandler.java 1333970 
>   http://svn.apache.org/repos/asf/shindig/trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/oauth2/handler/CodeGrantTypeHandler.java 1333970 
>   http://svn.apache.org/repos/asf/shindig/trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/oauth2/handler/OAuth2HandlerModule.java 1333970 
>   http://svn.apache.org/repos/asf/shindig/trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/oauth2/handler/TokenAuthorizationResponseHandler.java 1333970 
>   http://svn.apache.org/repos/asf/shindig/trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/oauth2/persistence/MapCache.java PRE-CREATION 
>   http://svn.apache.org/repos/asf/shindig/trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/oauth2/persistence/OAuth2Cache.java 1333970 
>   http://svn.apache.org/repos/asf/shindig/trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/oauth2/persistence/OAuth2Client.java 1333970 
>   http://svn.apache.org/repos/asf/shindig/trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/oauth2/persistence/OAuth2Persister.java 1333970 
>   http://svn.apache.org/repos/asf/shindig/trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/oauth2/persistence/OAuth2TokenPersistence.java 1333970 
>   http://svn.apache.org/repos/asf/shindig/trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/oauth2/persistence/sample/InMemoryCache.java 1333970 
>   http://svn.apache.org/repos/asf/shindig/trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/oauth2/persistence/sample/JSONOAuth2Persister.java 1333970 
>   http://svn.apache.org/repos/asf/shindig/trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/servlet/MakeRequestHandler.java 1333970 
>   http://svn.apache.org/repos/asf/shindig/trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/servlet/OAuth2CallbackServlet.java 1333970 
>   http://svn.apache.org/repos/asf/shindig/trunk/java/gadgets/src/test/java/org/apache/shindig/gadgets/oauth2/MockUtils.java 1333970 
>   http://svn.apache.org/repos/asf/shindig/trunk/java/gadgets/src/test/java/org/apache/shindig/gadgets/oauth2/handler/CodeGrantTypeHandlerTest.java 1333970 
>   http://svn.apache.org/repos/asf/shindig/trunk/java/gadgets/src/test/java/org/apache/shindig/gadgets/oauth2/handler/TokenAuthorizationResponseHandlerTest.java 1333970 
>   http://svn.apache.org/repos/asf/shindig/trunk/java/gadgets/src/test/java/org/apache/shindig/gadgets/oauth2/persistence/OAuth2ClientTest.java 1333970 
>   http://svn.apache.org/repos/asf/shindig/trunk/java/gadgets/src/test/java/org/apache/shindig/gadgets/oauth2/persistence/sample/InMemoryCacheTest.java 1333970 
>   http://svn.apache.org/repos/asf/shindig/trunk/java/gadgets/src/test/java/org/apache/shindig/gadgets/oauth2/persistence/sample/JSONOAuth2PersisterTest.java 1333970 
>   http://svn.apache.org/repos/asf/shindig/trunk/java/gadgets/src/test/java/org/apache/shindig/gadgets/servlet/MakeRequestHandlerTest.java 1333970 
>   http://svn.apache.org/repos/asf/shindig/trunk/java/gadgets/src/test/resources/org/apache/shindig/gadgets/oauth2/oauth2_test.json 1333970 
> 
> Diff: https://reviews.apache.org/r/4947/diff
> 
> 
> Testing
> -------
> 
> All test cases pass.
> 
> 
> Thanks,
> 
> Adam
> 
>


Re: Review Request: Jira 1732 and lots of other fixes/enhancements

Posted by Stanton Sievers <si...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/4947/#review7688
-----------------------------------------------------------

Ship it!


If there are no other objections, I will commit this within the next 24 hours or so.

- Stanton


On 2012-05-04 14:31:05, Adam Clarke wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/4947/
> -----------------------------------------------------------
> 
> (Updated 2012-05-04 14:31:05)
> 
> 
> Review request for shindig, Ryan Baxter and Brian Lillie.
> 
> 
> Summary
> -------
> 
> When the original OAuth2 Consumer was added the patch was quite large and it was tough to get a  comprehensive review.  It was expected that there would be a couple of revisions to address problems.
> 
> This patch contains the changes for Jira 173.  Due to time constraints it also contains a number of other fixes/enhancements found in internal OAuth2 Consumer reviews and testing.
> 
> Some of these changes are interface changes/additions/deletions and could effect custom consumer extensions.  (Probably unlikely because nobody has complained about them yet.)
> 
> 
> Patch Includes:
> 0) More standard formatting and checkstyle in modified files.
> 
> 1) Jira 1732 for restricted OAuth2 endpoints.
> 
> 2) Rework of OAuth2Cache and the default InMemoryCache.  Tried to get too cute with the original which made it very hard to implement.  New version is much easier to implement with your own Maps.
> 
> 3) OAuth2 State encryption/decryption.  Wasn't absolutely necessary for OAuth2, but added it for consistency with OAuth1 and for the peace-of-mind of security types.
> 
> 4) BasicOAuth2Request no longer sends expired tokens.  The previous impl relied on the OAuth2 Service Provider returning 401 when a expired token was used.  This led to a nasty user experience when a service provider violated the spec and returned something other than 401.  OAuth2 Consumer no longer sends expired tokens and reacts as if the server returned the 401.
> 
> 5) OAuth2Persister had an unnecessary method to create a token.  This is now handled in the OAuth2Store.
> 
> 6) Caching and OAuth2Store.init() fixes for better behavior in a clustered environment.
> 
> 7) Properly handle the URL %scheme% for the redirect uri (aka callback url)
> 
> 
> This addresses bug SHINDIG-1732.
>     https://issues.apache.org/jira/browse/SHINDIG-1732
> 
> 
> Diffs
> -----
> 
>   http://svn.apache.org/repos/asf/shindig/trunk/java/common/conf/shindig.properties 1333970 
>   http://svn.apache.org/repos/asf/shindig/trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/oauth2/BasicOAuth2Accessor.java 1333970 
>   http://svn.apache.org/repos/asf/shindig/trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/oauth2/BasicOAuth2Request.java 1333970 
>   http://svn.apache.org/repos/asf/shindig/trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/oauth2/BasicOAuth2Store.java 1333970 
>   http://svn.apache.org/repos/asf/shindig/trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/oauth2/OAuth2Accessor.java 1333970 
>   http://svn.apache.org/repos/asf/shindig/trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/oauth2/OAuth2CallbackState.java PRE-CREATION 
>   http://svn.apache.org/repos/asf/shindig/trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/oauth2/OAuth2CallbackStateToken.java PRE-CREATION 
>   http://svn.apache.org/repos/asf/shindig/trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/oauth2/OAuth2Error.java 1333970 
>   http://svn.apache.org/repos/asf/shindig/trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/oauth2/OAuth2FetcherConfig.java 1333970 
>   http://svn.apache.org/repos/asf/shindig/trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/oauth2/OAuth2Message.java 1333970 
>   http://svn.apache.org/repos/asf/shindig/trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/oauth2/OAuth2Module.java 1333970 
>   http://svn.apache.org/repos/asf/shindig/trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/oauth2/OAuth2Store.java 1333970 
>   http://svn.apache.org/repos/asf/shindig/trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/oauth2/OAuth2Token.java 1333970 
>   http://svn.apache.org/repos/asf/shindig/trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/oauth2/handler/CodeAuthorizationResponseHandler.java 1333970 
>   http://svn.apache.org/repos/asf/shindig/trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/oauth2/handler/CodeGrantTypeHandler.java 1333970 
>   http://svn.apache.org/repos/asf/shindig/trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/oauth2/handler/OAuth2HandlerModule.java 1333970 
>   http://svn.apache.org/repos/asf/shindig/trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/oauth2/handler/TokenAuthorizationResponseHandler.java 1333970 
>   http://svn.apache.org/repos/asf/shindig/trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/oauth2/persistence/MapCache.java PRE-CREATION 
>   http://svn.apache.org/repos/asf/shindig/trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/oauth2/persistence/OAuth2Cache.java 1333970 
>   http://svn.apache.org/repos/asf/shindig/trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/oauth2/persistence/OAuth2Client.java 1333970 
>   http://svn.apache.org/repos/asf/shindig/trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/oauth2/persistence/OAuth2Persister.java 1333970 
>   http://svn.apache.org/repos/asf/shindig/trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/oauth2/persistence/OAuth2TokenPersistence.java 1333970 
>   http://svn.apache.org/repos/asf/shindig/trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/oauth2/persistence/sample/InMemoryCache.java 1333970 
>   http://svn.apache.org/repos/asf/shindig/trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/oauth2/persistence/sample/JSONOAuth2Persister.java 1333970 
>   http://svn.apache.org/repos/asf/shindig/trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/servlet/MakeRequestHandler.java 1333970 
>   http://svn.apache.org/repos/asf/shindig/trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/servlet/OAuth2CallbackServlet.java 1333970 
>   http://svn.apache.org/repos/asf/shindig/trunk/java/gadgets/src/test/java/org/apache/shindig/gadgets/oauth2/MockUtils.java 1333970 
>   http://svn.apache.org/repos/asf/shindig/trunk/java/gadgets/src/test/java/org/apache/shindig/gadgets/oauth2/handler/CodeGrantTypeHandlerTest.java 1333970 
>   http://svn.apache.org/repos/asf/shindig/trunk/java/gadgets/src/test/java/org/apache/shindig/gadgets/oauth2/handler/TokenAuthorizationResponseHandlerTest.java 1333970 
>   http://svn.apache.org/repos/asf/shindig/trunk/java/gadgets/src/test/java/org/apache/shindig/gadgets/oauth2/persistence/OAuth2ClientTest.java 1333970 
>   http://svn.apache.org/repos/asf/shindig/trunk/java/gadgets/src/test/java/org/apache/shindig/gadgets/oauth2/persistence/sample/InMemoryCacheTest.java 1333970 
>   http://svn.apache.org/repos/asf/shindig/trunk/java/gadgets/src/test/java/org/apache/shindig/gadgets/oauth2/persistence/sample/JSONOAuth2PersisterTest.java 1333970 
>   http://svn.apache.org/repos/asf/shindig/trunk/java/gadgets/src/test/java/org/apache/shindig/gadgets/servlet/MakeRequestHandlerTest.java 1333970 
>   http://svn.apache.org/repos/asf/shindig/trunk/java/gadgets/src/test/resources/org/apache/shindig/gadgets/oauth2/oauth2_test.json 1333970 
> 
> Diff: https://reviews.apache.org/r/4947/diff
> 
> 
> Testing
> -------
> 
> All test cases pass.
> 
> 
> Thanks,
> 
> Adam
> 
>


Re: Review Request: Jira 1732 and lots of other fixes/enhancements

Posted by Adam Clarke <cl...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/4947/
-----------------------------------------------------------

(Updated 2012-05-04 14:31:05.271804)


Review request for shindig, Ryan Baxter and Brian Lillie.


Changes
-------

Addresses the testcase failure and refresh token issue.


Summary
-------

When the original OAuth2 Consumer was added the patch was quite large and it was tough to get a  comprehensive review.  It was expected that there would be a couple of revisions to address problems.

This patch contains the changes for Jira 173.  Due to time constraints it also contains a number of other fixes/enhancements found in internal OAuth2 Consumer reviews and testing.

Some of these changes are interface changes/additions/deletions and could effect custom consumer extensions.  (Probably unlikely because nobody has complained about them yet.)


Patch Includes:
0) More standard formatting and checkstyle in modified files.

1) Jira 1732 for restricted OAuth2 endpoints.

2) Rework of OAuth2Cache and the default InMemoryCache.  Tried to get too cute with the original which made it very hard to implement.  New version is much easier to implement with your own Maps.

3) OAuth2 State encryption/decryption.  Wasn't absolutely necessary for OAuth2, but added it for consistency with OAuth1 and for the peace-of-mind of security types.

4) BasicOAuth2Request no longer sends expired tokens.  The previous impl relied on the OAuth2 Service Provider returning 401 when a expired token was used.  This led to a nasty user experience when a service provider violated the spec and returned something other than 401.  OAuth2 Consumer no longer sends expired tokens and reacts as if the server returned the 401.

5) OAuth2Persister had an unnecessary method to create a token.  This is now handled in the OAuth2Store.

6) Caching and OAuth2Store.init() fixes for better behavior in a clustered environment.

7) Properly handle the URL %scheme% for the redirect uri (aka callback url)


This addresses bug SHINDIG-1732.
    https://issues.apache.org/jira/browse/SHINDIG-1732


Diffs (updated)
-----

  http://svn.apache.org/repos/asf/shindig/trunk/java/common/conf/shindig.properties 1333970 
  http://svn.apache.org/repos/asf/shindig/trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/oauth2/BasicOAuth2Accessor.java 1333970 
  http://svn.apache.org/repos/asf/shindig/trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/oauth2/BasicOAuth2Request.java 1333970 
  http://svn.apache.org/repos/asf/shindig/trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/oauth2/BasicOAuth2Store.java 1333970 
  http://svn.apache.org/repos/asf/shindig/trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/oauth2/OAuth2Accessor.java 1333970 
  http://svn.apache.org/repos/asf/shindig/trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/oauth2/OAuth2CallbackState.java PRE-CREATION 
  http://svn.apache.org/repos/asf/shindig/trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/oauth2/OAuth2CallbackStateToken.java PRE-CREATION 
  http://svn.apache.org/repos/asf/shindig/trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/oauth2/OAuth2Error.java 1333970 
  http://svn.apache.org/repos/asf/shindig/trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/oauth2/OAuth2FetcherConfig.java 1333970 
  http://svn.apache.org/repos/asf/shindig/trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/oauth2/OAuth2Message.java 1333970 
  http://svn.apache.org/repos/asf/shindig/trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/oauth2/OAuth2Module.java 1333970 
  http://svn.apache.org/repos/asf/shindig/trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/oauth2/OAuth2Store.java 1333970 
  http://svn.apache.org/repos/asf/shindig/trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/oauth2/OAuth2Token.java 1333970 
  http://svn.apache.org/repos/asf/shindig/trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/oauth2/handler/CodeAuthorizationResponseHandler.java 1333970 
  http://svn.apache.org/repos/asf/shindig/trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/oauth2/handler/CodeGrantTypeHandler.java 1333970 
  http://svn.apache.org/repos/asf/shindig/trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/oauth2/handler/OAuth2HandlerModule.java 1333970 
  http://svn.apache.org/repos/asf/shindig/trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/oauth2/handler/TokenAuthorizationResponseHandler.java 1333970 
  http://svn.apache.org/repos/asf/shindig/trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/oauth2/persistence/MapCache.java PRE-CREATION 
  http://svn.apache.org/repos/asf/shindig/trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/oauth2/persistence/OAuth2Cache.java 1333970 
  http://svn.apache.org/repos/asf/shindig/trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/oauth2/persistence/OAuth2Client.java 1333970 
  http://svn.apache.org/repos/asf/shindig/trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/oauth2/persistence/OAuth2Persister.java 1333970 
  http://svn.apache.org/repos/asf/shindig/trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/oauth2/persistence/OAuth2TokenPersistence.java 1333970 
  http://svn.apache.org/repos/asf/shindig/trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/oauth2/persistence/sample/InMemoryCache.java 1333970 
  http://svn.apache.org/repos/asf/shindig/trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/oauth2/persistence/sample/JSONOAuth2Persister.java 1333970 
  http://svn.apache.org/repos/asf/shindig/trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/servlet/MakeRequestHandler.java 1333970 
  http://svn.apache.org/repos/asf/shindig/trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/servlet/OAuth2CallbackServlet.java 1333970 
  http://svn.apache.org/repos/asf/shindig/trunk/java/gadgets/src/test/java/org/apache/shindig/gadgets/oauth2/MockUtils.java 1333970 
  http://svn.apache.org/repos/asf/shindig/trunk/java/gadgets/src/test/java/org/apache/shindig/gadgets/oauth2/handler/CodeGrantTypeHandlerTest.java 1333970 
  http://svn.apache.org/repos/asf/shindig/trunk/java/gadgets/src/test/java/org/apache/shindig/gadgets/oauth2/handler/TokenAuthorizationResponseHandlerTest.java 1333970 
  http://svn.apache.org/repos/asf/shindig/trunk/java/gadgets/src/test/java/org/apache/shindig/gadgets/oauth2/persistence/OAuth2ClientTest.java 1333970 
  http://svn.apache.org/repos/asf/shindig/trunk/java/gadgets/src/test/java/org/apache/shindig/gadgets/oauth2/persistence/sample/InMemoryCacheTest.java 1333970 
  http://svn.apache.org/repos/asf/shindig/trunk/java/gadgets/src/test/java/org/apache/shindig/gadgets/oauth2/persistence/sample/JSONOAuth2PersisterTest.java 1333970 
  http://svn.apache.org/repos/asf/shindig/trunk/java/gadgets/src/test/java/org/apache/shindig/gadgets/servlet/MakeRequestHandlerTest.java 1333970 
  http://svn.apache.org/repos/asf/shindig/trunk/java/gadgets/src/test/resources/org/apache/shindig/gadgets/oauth2/oauth2_test.json 1333970 

Diff: https://reviews.apache.org/r/4947/diff


Testing
-------

All test cases pass.


Thanks,

Adam


Re: Review Request: Jira 1732 and lots of other fixes/enhancements

Posted by br...@us.ibm.com.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/4947/#review7480
-----------------------------------------------------------



http://svn.apache.org/repos/asf/shindig/trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/oauth2/BasicOAuth2Store.java
<https://reviews.apache.org/r/4947/#comment16551>

    The call sequence here appears to be BasicOAuth2Store.removeToken( OAuth2Token ) that disaggregates into the removeToken( token parts ), where we do a cache lookup by parts to find a token (that we already had) so that we can do a remove by OAuth2Token.  Seems like this could be streamlined with a remove method on the cache by a token's component parts (since the MapCache disassembles the token into its component parts to build the key)



http://svn.apache.org/repos/asf/shindig/trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/oauth2/OAuth2CallbackState.java
<https://reviews.apache.org/r/4947/#comment16553>

    If there is no crypter, then the stateBlob won't be parsed.  If there is no intent to handle a null crypter in the class, then it seems like that should be an exception condition.   Seems like no crypter might be useful in some debug conditions



http://svn.apache.org/repos/asf/shindig/trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/oauth2/OAuth2CallbackState.java
<https://reviews.apache.org/r/4947/#comment16552>

    There is no toString in the superclasses, so if there is no crypter, the string will be unusable



http://svn.apache.org/repos/asf/shindig/trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/oauth2/persistence/OAuth2Client.java
<https://reviews.apache.org/r/4947/#comment16550>

    This changes the toString method result.   I didn't see a corresponding change in this patch to the org.apache.shindig.gadgets.oauth2.persistence.OAuth2ClientTest class for test method testOAuth2Client_1


- BrianLillie


On 2012-05-01 15:12:43, Adam Clarke wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/4947/
> -----------------------------------------------------------
> 
> (Updated 2012-05-01 15:12:43)
> 
> 
> Review request for shindig, Ryan Baxter and Brian Lillie.
> 
> 
> Summary
> -------
> 
> When the original OAuth2 Consumer was added the patch was quite large and it was tough to get a  comprehensive review.  It was expected that there would be a couple of revisions to address problems.
> 
> This patch contains the changes for Jira 173.  Due to time constraints it also contains a number of other fixes/enhancements found in internal OAuth2 Consumer reviews and testing.
> 
> Some of these changes are interface changes/additions/deletions and could effect custom consumer extensions.  (Probably unlikely because nobody has complained about them yet.)
> 
> 
> Patch Includes:
> 0) More standard formatting and checkstyle in modified files.
> 
> 1) Jira 1732 for restricted OAuth2 endpoints.
> 
> 2) Rework of OAuth2Cache and the default InMemoryCache.  Tried to get too cute with the original which made it very hard to implement.  New version is much easier to implement with your own Maps.
> 
> 3) OAuth2 State encryption/decryption.  Wasn't absolutely necessary for OAuth2, but added it for consistency with OAuth1 and for the peace-of-mind of security types.
> 
> 4) BasicOAuth2Request no longer sends expired tokens.  The previous impl relied on the OAuth2 Service Provider returning 401 when a expired token was used.  This led to a nasty user experience when a service provider violated the spec and returned something other than 401.  OAuth2 Consumer no longer sends expired tokens and reacts as if the server returned the 401.
> 
> 5) OAuth2Persister had an unnecessary method to create a token.  This is now handled in the OAuth2Store.
> 
> 6) Caching and OAuth2Store.init() fixes for better behavior in a clustered environment.
> 
> 7) Properly handle the URL %scheme% for the redirect uri (aka callback url)
> 
> 
> This addresses bug SHINDIG-1732.
>     https://issues.apache.org/jira/browse/SHINDIG-1732
> 
> 
> Diffs
> -----
> 
>   http://svn.apache.org/repos/asf/shindig/trunk/java/common/conf/shindig.properties 1332636 
>   http://svn.apache.org/repos/asf/shindig/trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/oauth2/BasicOAuth2Accessor.java 1332636 
>   http://svn.apache.org/repos/asf/shindig/trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/oauth2/BasicOAuth2Request.java 1332636 
>   http://svn.apache.org/repos/asf/shindig/trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/oauth2/BasicOAuth2Store.java 1332636 
>   http://svn.apache.org/repos/asf/shindig/trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/oauth2/OAuth2Accessor.java 1332636 
>   http://svn.apache.org/repos/asf/shindig/trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/oauth2/OAuth2CallbackState.java PRE-CREATION 
>   http://svn.apache.org/repos/asf/shindig/trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/oauth2/OAuth2CallbackStateToken.java PRE-CREATION 
>   http://svn.apache.org/repos/asf/shindig/trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/oauth2/OAuth2Error.java 1332636 
>   http://svn.apache.org/repos/asf/shindig/trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/oauth2/OAuth2FetcherConfig.java 1332636 
>   http://svn.apache.org/repos/asf/shindig/trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/oauth2/OAuth2Message.java 1332636 
>   http://svn.apache.org/repos/asf/shindig/trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/oauth2/OAuth2Module.java 1332636 
>   http://svn.apache.org/repos/asf/shindig/trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/oauth2/OAuth2Store.java 1332636 
>   http://svn.apache.org/repos/asf/shindig/trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/oauth2/OAuth2Token.java 1332636 
>   http://svn.apache.org/repos/asf/shindig/trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/oauth2/handler/CodeAuthorizationResponseHandler.java 1332636 
>   http://svn.apache.org/repos/asf/shindig/trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/oauth2/handler/CodeGrantTypeHandler.java 1332636 
>   http://svn.apache.org/repos/asf/shindig/trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/oauth2/handler/OAuth2HandlerModule.java 1332636 
>   http://svn.apache.org/repos/asf/shindig/trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/oauth2/handler/TokenAuthorizationResponseHandler.java 1332636 
>   http://svn.apache.org/repos/asf/shindig/trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/oauth2/persistence/MapCache.java PRE-CREATION 
>   http://svn.apache.org/repos/asf/shindig/trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/oauth2/persistence/OAuth2Cache.java 1332636 
>   http://svn.apache.org/repos/asf/shindig/trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/oauth2/persistence/OAuth2Client.java 1332636 
>   http://svn.apache.org/repos/asf/shindig/trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/oauth2/persistence/OAuth2Persister.java 1332636 
>   http://svn.apache.org/repos/asf/shindig/trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/oauth2/persistence/OAuth2TokenPersistence.java 1332636 
>   http://svn.apache.org/repos/asf/shindig/trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/oauth2/persistence/sample/InMemoryCache.java 1332636 
>   http://svn.apache.org/repos/asf/shindig/trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/oauth2/persistence/sample/JSONOAuth2Persister.java 1332636 
>   http://svn.apache.org/repos/asf/shindig/trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/servlet/MakeRequestHandler.java 1332636 
>   http://svn.apache.org/repos/asf/shindig/trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/servlet/OAuth2CallbackServlet.java 1332636 
>   http://svn.apache.org/repos/asf/shindig/trunk/java/gadgets/src/test/java/org/apache/shindig/gadgets/oauth2/MockUtils.java 1332636 
>   http://svn.apache.org/repos/asf/shindig/trunk/java/gadgets/src/test/java/org/apache/shindig/gadgets/oauth2/handler/CodeGrantTypeHandlerTest.java 1332636 
>   http://svn.apache.org/repos/asf/shindig/trunk/java/gadgets/src/test/java/org/apache/shindig/gadgets/oauth2/handler/TokenAuthorizationResponseHandlerTest.java 1332636 
>   http://svn.apache.org/repos/asf/shindig/trunk/java/gadgets/src/test/java/org/apache/shindig/gadgets/oauth2/persistence/sample/InMemoryCacheTest.java 1332636 
>   http://svn.apache.org/repos/asf/shindig/trunk/java/gadgets/src/test/java/org/apache/shindig/gadgets/oauth2/persistence/sample/JSONOAuth2PersisterTest.java 1332636 
>   http://svn.apache.org/repos/asf/shindig/trunk/java/gadgets/src/test/java/org/apache/shindig/gadgets/servlet/MakeRequestHandlerTest.java 1332636 
>   http://svn.apache.org/repos/asf/shindig/trunk/java/gadgets/src/test/resources/org/apache/shindig/gadgets/oauth2/oauth2_test.json 1332636 
> 
> Diff: https://reviews.apache.org/r/4947/diff
> 
> 
> Testing
> -------
> 
> All test cases pass.
> 
> 
> Thanks,
> 
> Adam
> 
>