You are viewing a plain text version of this content. The canonical link for it is here.

Posted to dev@shindig.apache.org by Eric Woods <wo...@gmail.com> on 2011/09/16 23:05:04 UTC

Review Request: OAuth 2.0 service provider implementation in Apache Shindig.

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

Review request for shindig.


Summary
-------

OAuth 2.0 service provider implementation in Apache Shindig.

Documentation wiki: http://docs.opensocial.org/display/OSD/OAuth+2.0+Service+Provider+Implementation+in+Apache+Shindig

JIRA issue: https://issues.apache.org/jira/browse/SHINDIG-1623


Diffs
-----

  http://svn.apache.org/repos/asf/shindig/trunk/content/sampledata/canonicaldb.json 1171761 
  http://svn.apache.org/repos/asf/shindig/trunk/java/server/src/main/webapp/WEB-INF/web.xml 1171761 
  http://svn.apache.org/repos/asf/shindig/trunk/java/social-api/src/main/java/org/apache/shindig/social/core/oauth/AuthenticationHandlerProvider.java 1171761 
  http://svn.apache.org/repos/asf/shindig/trunk/java/social-api/src/main/java/org/apache/shindig/social/core/oauth2/OAuth2AuthenticationHandler.java PRE-CREATION 
  http://svn.apache.org/repos/asf/shindig/trunk/java/social-api/src/main/java/org/apache/shindig/social/core/oauth2/OAuth2AuthorizationHandler.java PRE-CREATION 
  http://svn.apache.org/repos/asf/shindig/trunk/java/social-api/src/main/java/org/apache/shindig/social/core/oauth2/OAuth2Client.java PRE-CREATION 
  http://svn.apache.org/repos/asf/shindig/trunk/java/social-api/src/main/java/org/apache/shindig/social/core/oauth2/OAuth2Code.java PRE-CREATION 
  http://svn.apache.org/repos/asf/shindig/trunk/java/social-api/src/main/java/org/apache/shindig/social/core/oauth2/OAuth2DataService.java PRE-CREATION 
  http://svn.apache.org/repos/asf/shindig/trunk/java/social-api/src/main/java/org/apache/shindig/social/core/oauth2/OAuth2DataServiceImpl.java PRE-CREATION 
  http://svn.apache.org/repos/asf/shindig/trunk/java/social-api/src/main/java/org/apache/shindig/social/core/oauth2/OAuth2Exception.java PRE-CREATION 
  http://svn.apache.org/repos/asf/shindig/trunk/java/social-api/src/main/java/org/apache/shindig/social/core/oauth2/OAuth2Filter.java PRE-CREATION 
  http://svn.apache.org/repos/asf/shindig/trunk/java/social-api/src/main/java/org/apache/shindig/social/core/oauth2/OAuth2NormalizedRequest.java PRE-CREATION 
  http://svn.apache.org/repos/asf/shindig/trunk/java/social-api/src/main/java/org/apache/shindig/social/core/oauth2/OAuth2NormalizedResponse.java PRE-CREATION 
  http://svn.apache.org/repos/asf/shindig/trunk/java/social-api/src/main/java/org/apache/shindig/social/core/oauth2/OAuth2Service.java PRE-CREATION 
  http://svn.apache.org/repos/asf/shindig/trunk/java/social-api/src/main/java/org/apache/shindig/social/core/oauth2/OAuth2ServiceImpl.java PRE-CREATION 
  http://svn.apache.org/repos/asf/shindig/trunk/java/social-api/src/main/java/org/apache/shindig/social/core/oauth2/OAuth2Servlet.java PRE-CREATION 
  http://svn.apache.org/repos/asf/shindig/trunk/java/social-api/src/main/java/org/apache/shindig/social/core/oauth2/OAuth2TokenHandler.java PRE-CREATION 
  http://svn.apache.org/repos/asf/shindig/trunk/java/social-api/src/main/java/org/apache/shindig/social/core/oauth2/OAuth2Types.java PRE-CREATION 
  http://svn.apache.org/repos/asf/shindig/trunk/java/social-api/src/main/java/org/apache/shindig/social/core/oauth2/OAuth2Utils.java PRE-CREATION 
  http://svn.apache.org/repos/asf/shindig/trunk/java/social-api/src/main/java/org/apache/shindig/social/core/oauth2/validators/AccessTokenRequestValidator.java PRE-CREATION 
  http://svn.apache.org/repos/asf/shindig/trunk/java/social-api/src/main/java/org/apache/shindig/social/core/oauth2/validators/AuthCodeGrantValidator.java PRE-CREATION 
  http://svn.apache.org/repos/asf/shindig/trunk/java/social-api/src/main/java/org/apache/shindig/social/core/oauth2/validators/AuthorizationCodeRequestValidator.java PRE-CREATION 
  http://svn.apache.org/repos/asf/shindig/trunk/java/social-api/src/main/java/org/apache/shindig/social/core/oauth2/validators/ClientCredentialsGrantValidator.java PRE-CREATION 
  http://svn.apache.org/repos/asf/shindig/trunk/java/social-api/src/main/java/org/apache/shindig/social/core/oauth2/validators/DefaultResourceRequestValidator.java PRE-CREATION 
  http://svn.apache.org/repos/asf/shindig/trunk/java/social-api/src/main/java/org/apache/shindig/social/core/oauth2/validators/OAuth2GrantValidator.java PRE-CREATION 
  http://svn.apache.org/repos/asf/shindig/trunk/java/social-api/src/main/java/org/apache/shindig/social/core/oauth2/validators/OAuth2ProtectedResourceValidator.java PRE-CREATION 
  http://svn.apache.org/repos/asf/shindig/trunk/java/social-api/src/main/java/org/apache/shindig/social/core/oauth2/validators/OAuth2RequestValidator.java PRE-CREATION 
  http://svn.apache.org/repos/asf/shindig/trunk/java/social-api/src/main/java/org/apache/shindig/social/sample/SampleModule.java 1171761 
  http://svn.apache.org/repos/asf/shindig/trunk/java/social-api/src/test/java/org/apache/shindig/social/SocialApiTestsGuiceModule.java 1171761 
  http://svn.apache.org/repos/asf/shindig/trunk/java/social-api/src/test/java/org/apache/shindig/social/core/config/SocialApiGuiceModuleTest.java 1171761 
  http://svn.apache.org/repos/asf/shindig/trunk/java/social-api/src/test/java/org/apache/shindig/social/core/oauth/AuthenticationProviderHandlerTest.java 1171761 
  http://svn.apache.org/repos/asf/shindig/trunk/java/social-api/src/test/java/org/apache/shindig/social/core/oauth/MockServletOutputStream.java PRE-CREATION 
  http://svn.apache.org/repos/asf/shindig/trunk/java/social-api/src/test/java/org/apache/shindig/social/core/oauth/OAuth2AuthCodeFlowTest.java PRE-CREATION 
  http://svn.apache.org/repos/asf/shindig/trunk/java/social-api/src/test/java/org/apache/shindig/social/core/oauth/OAuth2AuthenticationHandlerTest.java PRE-CREATION 
  http://svn.apache.org/repos/asf/shindig/trunk/java/social-api/src/test/java/org/apache/shindig/social/core/oauth/OAuth2ClientCredentialFlowTest.java PRE-CREATION 
  http://svn.apache.org/repos/asf/shindig/trunk/java/social-api/src/test/java/org/apache/shindig/social/core/oauth/OAuth2ImplicitFlowTest.java PRE-CREATION 
  http://svn.apache.org/repos/asf/shindig/trunk/java/social-api/src/test/java/org/apache/shindig/social/dataservice/integration/AbstractLargeRestfulTests.java 1171761 

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


Testing
-------

Yes, JUnits executed with maven build.


Thanks,

Eric

Re: Review Request: OAuth 2.0 service provider implementation in Apache Shindig.

Posted by Eric Woods <wo...@gmail.com>.


> On 2011-09-29 17:51:35, Ryan Baxter wrote:
> > This is my quick review.  I am not expert on OAuth2 so this was purely from a code perspective.  It would be great is people who already have oauth2 implementations could review this code.  Most of my comments are below, but I have some general ones as well.
> > 
> > 1.  Throughout the code there are TODOs with actions items, which seem to indicate there may be some gaps in the code.  How complete is this code?
> > 
> > 2.  I see tests for the different flows but I don't see unit tests for many of new classes.

The remaining TODOs are for future considerations, not incorrect functionality or gaps in the supported flows.  For example, scoping is not currently supported.  Neither are client registration services.  These are candidates to implement in the future, but are not currently being developed.


> On 2011-09-29 17:51:35, Ryan Baxter wrote:
> > http://svn.apache.org/repos/asf/shindig/trunk/java/social-api/src/main/java/org/apache/shindig/social/core/oauth2/OAuth2AuthenticationHandler.java, line 52
> > <https://reviews.apache.org/r/1940/diff/2/?file=45163#file45163line52>
> >
> >     Log the exception

Done.


> On 2011-09-29 17:51:35, Ryan Baxter wrote:
> > http://svn.apache.org/repos/asf/shindig/trunk/java/social-api/src/main/java/org/apache/shindig/social/core/oauth2/OAuth2AuthorizationHandler.java, line 31
> > <https://reviews.apache.org/r/1940/diff/2/?file=45164#file45164line31>
> >
> >     Add javadoc

Done.


> On 2011-09-29 17:51:35, Ryan Baxter wrote:
> > http://svn.apache.org/repos/asf/shindig/trunk/java/social-api/src/main/java/org/apache/shindig/social/core/oauth2/OAuth2AuthorizationHandler.java, line 93
> > <https://reviews.apache.org/r/1940/diff/2/?file=45164#file45164line93>
> >
> >     if we should never get here should we throw and exception that the calling code can handle instead of returning null

Changed to correctly throw OAuth2Exception for receiving unsupported response type.


> On 2011-09-29 17:51:35, Ryan Baxter wrote:
> > http://svn.apache.org/repos/asf/shindig/trunk/java/social-api/src/main/java/org/apache/shindig/social/core/oauth2/OAuth2Code.java, line 29
> > <https://reviews.apache.org/r/1940/diff/2/?file=45166#file45166line29>
> >
> >     Add javadoc

Done.


> On 2011-09-29 17:51:35, Ryan Baxter wrote:
> > http://svn.apache.org/repos/asf/shindig/trunk/java/social-api/src/main/java/org/apache/shindig/social/core/oauth2/OAuth2DataService.java, line 28
> > <https://reviews.apache.org/r/1940/diff/2/?file=45167#file45167line28>
> >
> >     make sure you add @param and @returns

Done.


> On 2011-09-29 17:51:35, Ryan Baxter wrote:
> > http://svn.apache.org/repos/asf/shindig/trunk/java/social-api/src/main/java/org/apache/shindig/social/core/oauth2/OAuth2DataServiceImpl.java, line 56
> > <https://reviews.apache.org/r/1940/diff/2/?file=45168#file45168line56>
> >
> >     use the Lists API

Done.


> On 2011-09-29 17:51:35, Ryan Baxter wrote:
> > http://svn.apache.org/repos/asf/shindig/trunk/java/social-api/src/main/java/org/apache/shindig/social/core/oauth2/OAuth2DataServiceImpl.java, line 57
> > <https://reviews.apache.org/r/1940/diff/2/?file=45168#file45168line57>
> >
> >     Use the Maps API

Done.


> On 2011-09-29 17:51:35, Ryan Baxter wrote:
> > http://svn.apache.org/repos/asf/shindig/trunk/java/social-api/src/main/java/org/apache/shindig/social/core/oauth2/OAuth2DataServiceImpl.java, line 58
> > <https://reviews.apache.org/r/1940/diff/2/?file=45168#file45168line58>
> >
> >     Use the Maps API

Done.


> On 2011-09-29 17:51:35, Ryan Baxter wrote:
> > http://svn.apache.org/repos/asf/shindig/trunk/java/social-api/src/main/java/org/apache/shindig/social/core/oauth2/OAuth2DataServiceImpl.java, line 64
> > <https://reviews.apache.org/r/1940/diff/2/?file=45168#file45168line64>
> >
> >     Could getId return null?

Under no circumstances should a client not have an ID.


> On 2011-09-29 17:51:35, Ryan Baxter wrote:
> > http://svn.apache.org/repos/asf/shindig/trunk/java/social-api/src/main/java/org/apache/shindig/social/core/oauth2/OAuth2DataServiceImpl.java, line 87
> > <https://reviews.apache.org/r/1940/diff/2/?file=45168#file45168line87>
> >
> >     Use the Lists API

Done.


> On 2011-09-29 17:51:35, Ryan Baxter wrote:
> > http://svn.apache.org/repos/asf/shindig/trunk/java/social-api/src/main/java/org/apache/shindig/social/core/oauth2/OAuth2DataServiceImpl.java, line 97
> > <https://reviews.apache.org/r/1940/diff/2/?file=45168#file45168line97>
> >
> >     Could getValue be null?

An OAuth2Code should always have a value; it's not optional.


> On 2011-09-29 17:51:35, Ryan Baxter wrote:
> > http://svn.apache.org/repos/asf/shindig/trunk/java/social-api/src/main/java/org/apache/shindig/social/core/oauth2/OAuth2DataServiceImpl.java, line 110
> > <https://reviews.apache.org/r/1940/diff/2/?file=45168#file45168line110>
> >
> >     Could getValue be null?

An OAuth2Code should always have a value; it's not optional.


> On 2011-09-29 17:51:35, Ryan Baxter wrote:
> > http://svn.apache.org/repos/asf/shindig/trunk/java/social-api/src/main/java/org/apache/shindig/social/core/oauth2/OAuth2DataServiceImpl.java, line 122
> > <https://reviews.apache.org/r/1940/diff/2/?file=45168#file45168line122>
> >
> >     Use the Lists API

Done.


> On 2011-09-29 17:51:35, Ryan Baxter wrote:
> > http://svn.apache.org/repos/asf/shindig/trunk/java/social-api/src/main/java/org/apache/shindig/social/core/oauth2/OAuth2Exception.java, line 23
> > <https://reviews.apache.org/r/1940/diff/2/?file=45169#file45169line23>
> >
> >     Add javadoc

Done.


> On 2011-09-29 17:51:35, Ryan Baxter wrote:
> > http://svn.apache.org/repos/asf/shindig/trunk/java/social-api/src/main/java/org/apache/shindig/social/core/oauth2/OAuth2Filter.java, line 48
> > <https://reviews.apache.org/r/1940/diff/2/?file=45170#file45170line48>
> >
> >     remove the @author

Silly Matt - Done.


> On 2011-09-29 17:51:35, Ryan Baxter wrote:
> > http://svn.apache.org/repos/asf/shindig/trunk/java/social-api/src/main/java/org/apache/shindig/social/core/oauth2/OAuth2NormalizedRequest.java, line 173
> > <https://reviews.apache.org/r/1940/diff/2/?file=45171#file45171line173>
> >
> >     Should the startsWith be case sensitive?

Great catch.  HTTP header names are case-insensitive.  Fixed.


> On 2011-09-29 17:51:35, Ryan Baxter wrote:
> > http://svn.apache.org/repos/asf/shindig/trunk/java/social-api/src/main/java/org/apache/shindig/social/core/oauth2/OAuth2NormalizedRequest.java, line 186
> > <https://reviews.apache.org/r/1940/diff/2/?file=45171#file45171line186>
> >
> >     Same thing should this be case sensitive?

Fixed.


> On 2011-09-29 17:51:35, Ryan Baxter wrote:
> > http://svn.apache.org/repos/asf/shindig/trunk/java/social-api/src/main/java/org/apache/shindig/social/core/oauth2/OAuth2DataServiceImpl.java, line 132
> > <https://reviews.apache.org/r/1940/diff/2/?file=45168#file45168line132>
> >
> >     Could getValue be null?

An OAuth2Code should always have a value; it's not optional.


> On 2011-09-29 17:51:35, Ryan Baxter wrote:
> > http://svn.apache.org/repos/asf/shindig/trunk/java/social-api/src/main/java/org/apache/shindig/social/core/oauth2/OAuth2NormalizedRequest.java, line 211
> > <https://reviews.apache.org/r/1940/diff/2/?file=45171#file45171line211>
> >
> >     Log the exception

Done.


> On 2011-09-29 17:51:35, Ryan Baxter wrote:
> > http://svn.apache.org/repos/asf/shindig/trunk/java/social-api/src/main/java/org/apache/shindig/social/core/oauth2/OAuth2NormalizedRequest.java, line 224
> > <https://reviews.apache.org/r/1940/diff/2/?file=45171#file45171line224>
> >
> >     Should we be hardcoding localhost:8080 here?

The localhost:8080 is only used to build a valid URL from which parameters may be extracted.


> On 2011-09-29 17:51:35, Ryan Baxter wrote:
> > http://svn.apache.org/repos/asf/shindig/trunk/java/social-api/src/main/java/org/apache/shindig/social/core/oauth2/OAuth2NormalizedRequest.java, line 252
> > <https://reviews.apache.org/r/1940/diff/2/?file=45171#file45171line252>
> >
> >     Log the exception

Done.


> On 2011-09-29 17:51:35, Ryan Baxter wrote:
> > http://svn.apache.org/repos/asf/shindig/trunk/java/social-api/src/main/java/org/apache/shindig/social/core/oauth2/OAuth2NormalizedRequest.java, line 253
> > <https://reviews.apache.org/r/1940/diff/2/?file=45171#file45171line253>
> >
> >     I think you need a finally here to make sure the InputStream is closed

Done.


> On 2011-09-29 17:51:35, Ryan Baxter wrote:
> > http://svn.apache.org/repos/asf/shindig/trunk/java/social-api/src/main/java/org/apache/shindig/social/core/oauth2/OAuth2NormalizedResponse.java, line 36
> > <https://reviews.apache.org/r/1940/diff/2/?file=45172#file45172line36>
> >
> >     Use the Maps API

Done.


> On 2011-09-29 17:51:35, Ryan Baxter wrote:
> > http://svn.apache.org/repos/asf/shindig/trunk/java/social-api/src/main/java/org/apache/shindig/social/core/oauth2/OAuth2NormalizedResponse.java, line 37
> > <https://reviews.apache.org/r/1940/diff/2/?file=45172#file45172line37>
> >
> >     Use the Maps API

Done.


> On 2011-09-29 17:51:35, Ryan Baxter wrote:
> > http://svn.apache.org/repos/asf/shindig/trunk/java/social-api/src/main/java/org/apache/shindig/social/core/oauth2/OAuth2NormalizedResponse.java, line 80
> > <https://reviews.apache.org/r/1940/diff/2/?file=45172#file45172line80>
> >
> >     Might make sense to make these strings private static final since you are using them more than once in the code

I'm fine with that.  Added private static final strings.


> On 2011-09-29 17:51:35, Ryan Baxter wrote:
> > http://svn.apache.org/repos/asf/shindig/trunk/java/social-api/src/main/java/org/apache/shindig/social/core/oauth2/OAuth2ServiceImpl.java, line 46
> > <https://reviews.apache.org/r/1940/diff/2/?file=45174#file45174line46>
> >
> >     Should these be configurable?  I can imagine people wanting the configure the expiration times.

Added 3 properties to shindig.properties for configuration:
shindig.oauth2.authCodeExpiration=300000
shindig.oauth2.accessTokenExpiration=18000000
shindig.oauth2.refreshTokenExpiration=432000000


> On 2011-09-29 17:51:35, Ryan Baxter wrote:
> > http://svn.apache.org/repos/asf/shindig/trunk/java/social-api/src/main/java/org/apache/shindig/social/core/oauth2/OAuth2Servlet.java, line 96
> > <https://reviews.apache.org/r/1940/diff/2/?file=45175#file45175line96>
> >
> >     Log the exception and you probably want a finally clause here to make sure the PrintWriter is cleaned up properly

Done.


> On 2011-09-29 17:51:35, Ryan Baxter wrote:
> > http://svn.apache.org/repos/asf/shindig/trunk/java/social-api/src/main/java/org/apache/shindig/social/core/oauth2/OAuth2TokenHandler.java, line 32
> > <https://reviews.apache.org/r/1940/diff/2/?file=45176#file45176line32>
> >
> >     Add Javadoc

Done.


> On 2011-09-29 17:51:35, Ryan Baxter wrote:
> > http://svn.apache.org/repos/asf/shindig/trunk/java/social-api/src/main/java/org/apache/shindig/social/core/oauth2/OAuth2TokenHandler.java, line 62
> > <https://reviews.apache.org/r/1940/diff/2/?file=45176#file45176line62>
> >
> >     Log the exception

Catching the exception is an expected flow, no different from the normal flow.  If an exception is thrown, the NormalizedResponse is returned to the user as it would a valid and successful request.


> On 2011-09-29 17:51:35, Ryan Baxter wrote:
> > http://svn.apache.org/repos/asf/shindig/trunk/java/social-api/src/main/java/org/apache/shindig/social/core/oauth2/OAuth2Utils.java, line 37
> > <https://reviews.apache.org/r/1940/diff/2/?file=45178#file45178line37>
> >
> >     Add javadoc to all methods

Fixed.  Well actually, the undocumented methods were carry-overs and were removed.


> On 2011-09-29 17:51:35, Ryan Baxter wrote:
> > http://svn.apache.org/repos/asf/shindig/trunk/java/social-api/src/main/java/org/apache/shindig/social/core/oauth2/OAuth2Utils.java, line 67
> > <https://reviews.apache.org/r/1940/diff/2/?file=45178#file45178line67>
> >
> >     Log the exception

Method removed.


> On 2011-09-29 17:51:35, Ryan Baxter wrote:
> > http://svn.apache.org/repos/asf/shindig/trunk/java/social-api/src/main/java/org/apache/shindig/social/core/oauth2/validators/OAuth2ProtectedResourceValidator.java, line 23
> > <https://reviews.apache.org/r/1940/diff/2/?file=45185#file45185line23>
> >
> >     Add javadoc

Done.


> On 2011-09-29 17:51:35, Ryan Baxter wrote:
> > http://svn.apache.org/repos/asf/shindig/trunk/java/social-api/src/main/java/org/apache/shindig/social/core/oauth2/validators/OAuth2RequestValidator.java, line 23
> > <https://reviews.apache.org/r/1940/diff/2/?file=45186#file45186line23>
> >
> >     Add Javadoc

Done.


- Eric


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


On 2011-10-10 20:41:02, Eric Woods wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/1940/
> -----------------------------------------------------------
> 
> (Updated 2011-10-10 20:41:02)
> 
> 
> Review request for shindig.
> 
> 
> Summary
> -------
> 
> OAuth 2.0 service provider implementation in Apache Shindig.
> 
> Documentation wiki: http://docs.opensocial.org/display/OSD/OAuth+2.0+Service+Provider+Implementation+in+Apache+Shindig
> 
> JIRA issue: https://issues.apache.org/jira/browse/SHINDIG-1623
> 
> 
> Diffs
> -----
> 
>   http://svn.apache.org/repos/asf/shindig/trunk/content/sampledata/canonicaldb.json 1181203 
>   http://svn.apache.org/repos/asf/shindig/trunk/java/common/conf/shindig.properties 1181203 
>   http://svn.apache.org/repos/asf/shindig/trunk/java/server/src/main/webapp/WEB-INF/web.xml 1181203 
>   http://svn.apache.org/repos/asf/shindig/trunk/java/social-api/src/main/java/org/apache/shindig/social/core/oauth/AuthenticationHandlerProvider.java 1181203 
>   http://svn.apache.org/repos/asf/shindig/trunk/java/social-api/src/main/java/org/apache/shindig/social/core/oauth2/OAuth2AuthenticationHandler.java PRE-CREATION 
>   http://svn.apache.org/repos/asf/shindig/trunk/java/social-api/src/main/java/org/apache/shindig/social/core/oauth2/OAuth2AuthorizationHandler.java PRE-CREATION 
>   http://svn.apache.org/repos/asf/shindig/trunk/java/social-api/src/main/java/org/apache/shindig/social/core/oauth2/OAuth2Client.java PRE-CREATION 
>   http://svn.apache.org/repos/asf/shindig/trunk/java/social-api/src/main/java/org/apache/shindig/social/core/oauth2/OAuth2Code.java PRE-CREATION 
>   http://svn.apache.org/repos/asf/shindig/trunk/java/social-api/src/main/java/org/apache/shindig/social/core/oauth2/OAuth2DataService.java PRE-CREATION 
>   http://svn.apache.org/repos/asf/shindig/trunk/java/social-api/src/main/java/org/apache/shindig/social/core/oauth2/OAuth2DataServiceImpl.java PRE-CREATION 
>   http://svn.apache.org/repos/asf/shindig/trunk/java/social-api/src/main/java/org/apache/shindig/social/core/oauth2/OAuth2Exception.java PRE-CREATION 
>   http://svn.apache.org/repos/asf/shindig/trunk/java/social-api/src/main/java/org/apache/shindig/social/core/oauth2/OAuth2NormalizedRequest.java PRE-CREATION 
>   http://svn.apache.org/repos/asf/shindig/trunk/java/social-api/src/main/java/org/apache/shindig/social/core/oauth2/OAuth2NormalizedResponse.java PRE-CREATION 
>   http://svn.apache.org/repos/asf/shindig/trunk/java/social-api/src/main/java/org/apache/shindig/social/core/oauth2/OAuth2Service.java PRE-CREATION 
>   http://svn.apache.org/repos/asf/shindig/trunk/java/social-api/src/main/java/org/apache/shindig/social/core/oauth2/OAuth2ServiceImpl.java PRE-CREATION 
>   http://svn.apache.org/repos/asf/shindig/trunk/java/social-api/src/main/java/org/apache/shindig/social/core/oauth2/OAuth2Servlet.java PRE-CREATION 
>   http://svn.apache.org/repos/asf/shindig/trunk/java/social-api/src/main/java/org/apache/shindig/social/core/oauth2/OAuth2TokenHandler.java PRE-CREATION 
>   http://svn.apache.org/repos/asf/shindig/trunk/java/social-api/src/main/java/org/apache/shindig/social/core/oauth2/OAuth2Types.java PRE-CREATION 
>   http://svn.apache.org/repos/asf/shindig/trunk/java/social-api/src/main/java/org/apache/shindig/social/core/oauth2/OAuth2Utils.java PRE-CREATION 
>   http://svn.apache.org/repos/asf/shindig/trunk/java/social-api/src/main/java/org/apache/shindig/social/core/oauth2/validators/AccessTokenRequestValidator.java PRE-CREATION 
>   http://svn.apache.org/repos/asf/shindig/trunk/java/social-api/src/main/java/org/apache/shindig/social/core/oauth2/validators/AuthCodeGrantValidator.java PRE-CREATION 
>   http://svn.apache.org/repos/asf/shindig/trunk/java/social-api/src/main/java/org/apache/shindig/social/core/oauth2/validators/AuthorizationCodeRequestValidator.java PRE-CREATION 
>   http://svn.apache.org/repos/asf/shindig/trunk/java/social-api/src/main/java/org/apache/shindig/social/core/oauth2/validators/ClientCredentialsGrantValidator.java PRE-CREATION 
>   http://svn.apache.org/repos/asf/shindig/trunk/java/social-api/src/main/java/org/apache/shindig/social/core/oauth2/validators/DefaultResourceRequestValidator.java PRE-CREATION 
>   http://svn.apache.org/repos/asf/shindig/trunk/java/social-api/src/main/java/org/apache/shindig/social/core/oauth2/validators/OAuth2GrantValidator.java PRE-CREATION 
>   http://svn.apache.org/repos/asf/shindig/trunk/java/social-api/src/main/java/org/apache/shindig/social/core/oauth2/validators/OAuth2ProtectedResourceValidator.java PRE-CREATION 
>   http://svn.apache.org/repos/asf/shindig/trunk/java/social-api/src/main/java/org/apache/shindig/social/core/oauth2/validators/OAuth2RequestValidator.java PRE-CREATION 
>   http://svn.apache.org/repos/asf/shindig/trunk/java/social-api/src/main/java/org/apache/shindig/social/sample/SampleModule.java 1181203 
>   http://svn.apache.org/repos/asf/shindig/trunk/java/social-api/src/test/java/org/apache/shindig/social/SocialApiTestsGuiceModule.java 1181203 
>   http://svn.apache.org/repos/asf/shindig/trunk/java/social-api/src/test/java/org/apache/shindig/social/core/config/SocialApiGuiceModuleTest.java 1181203 
>   http://svn.apache.org/repos/asf/shindig/trunk/java/social-api/src/test/java/org/apache/shindig/social/core/oauth/AuthenticationProviderHandlerTest.java 1181203 
>   http://svn.apache.org/repos/asf/shindig/trunk/java/social-api/src/test/java/org/apache/shindig/social/core/oauth/MockServletOutputStream.java PRE-CREATION 
>   http://svn.apache.org/repos/asf/shindig/trunk/java/social-api/src/test/java/org/apache/shindig/social/core/oauth/OAuth2AuthCodeFlowTest.java PRE-CREATION 
>   http://svn.apache.org/repos/asf/shindig/trunk/java/social-api/src/test/java/org/apache/shindig/social/core/oauth/OAuth2AuthenticationHandlerTest.java PRE-CREATION 
>   http://svn.apache.org/repos/asf/shindig/trunk/java/social-api/src/test/java/org/apache/shindig/social/core/oauth/OAuth2ClientCredentialFlowTest.java PRE-CREATION 
>   http://svn.apache.org/repos/asf/shindig/trunk/java/social-api/src/test/java/org/apache/shindig/social/core/oauth/OAuth2ImplicitFlowTest.java PRE-CREATION 
>   http://svn.apache.org/repos/asf/shindig/trunk/java/social-api/src/test/java/org/apache/shindig/social/core/oauth/OAuthAuthenticationHanderTest.java 1181203 
>   http://svn.apache.org/repos/asf/shindig/trunk/java/social-api/src/test/java/org/apache/shindig/social/dataservice/integration/AbstractLargeRestfulTests.java 1181203 
> 
> Diff: https://reviews.apache.org/r/1940/diff
> 
> 
> Testing
> -------
> 
> Yes, JUnits executed with maven build.
> 
> 
> Thanks,
> 
> Eric
> 
>

Re: Review Request: OAuth 2.0 service provider implementation in Apache Shindig.

Posted by Ryan Baxter <rb...@gmail.com>.

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


This is my quick review.  I am not expert on OAuth2 so this was purely from a code perspective.  It would be great is people who already have oauth2 implementations could review this code.  Most of my comments are below, but I have some general ones as well.

1.  Throughout the code there are TODOs with actions items, which seem to indicate there may be some gaps in the code.  How complete is this code?

2.  I see tests for the different flows but I don't see unit tests for many of new classes.


http://svn.apache.org/repos/asf/shindig/trunk/java/social-api/src/main/java/org/apache/shindig/social/core/oauth2/OAuth2AuthenticationHandler.java
<https://reviews.apache.org/r/1940/#comment4997>

    Log the exception



http://svn.apache.org/repos/asf/shindig/trunk/java/social-api/src/main/java/org/apache/shindig/social/core/oauth2/OAuth2AuthorizationHandler.java
<https://reviews.apache.org/r/1940/#comment4998>

    Add javadoc



http://svn.apache.org/repos/asf/shindig/trunk/java/social-api/src/main/java/org/apache/shindig/social/core/oauth2/OAuth2AuthorizationHandler.java
<https://reviews.apache.org/r/1940/#comment4999>

    if we should never get here should we throw and exception that the calling code can handle instead of returning null



http://svn.apache.org/repos/asf/shindig/trunk/java/social-api/src/main/java/org/apache/shindig/social/core/oauth2/OAuth2Code.java
<https://reviews.apache.org/r/1940/#comment5000>

    Add javadoc



http://svn.apache.org/repos/asf/shindig/trunk/java/social-api/src/main/java/org/apache/shindig/social/core/oauth2/OAuth2DataService.java
<https://reviews.apache.org/r/1940/#comment5001>

    make sure you add @param and @returns



http://svn.apache.org/repos/asf/shindig/trunk/java/social-api/src/main/java/org/apache/shindig/social/core/oauth2/OAuth2DataServiceImpl.java
<https://reviews.apache.org/r/1940/#comment5002>

    use the Lists API



http://svn.apache.org/repos/asf/shindig/trunk/java/social-api/src/main/java/org/apache/shindig/social/core/oauth2/OAuth2DataServiceImpl.java
<https://reviews.apache.org/r/1940/#comment5003>

    Use the Maps API



http://svn.apache.org/repos/asf/shindig/trunk/java/social-api/src/main/java/org/apache/shindig/social/core/oauth2/OAuth2DataServiceImpl.java
<https://reviews.apache.org/r/1940/#comment5004>

    Use the Maps API



http://svn.apache.org/repos/asf/shindig/trunk/java/social-api/src/main/java/org/apache/shindig/social/core/oauth2/OAuth2DataServiceImpl.java
<https://reviews.apache.org/r/1940/#comment5005>

    Could getId return null?



http://svn.apache.org/repos/asf/shindig/trunk/java/social-api/src/main/java/org/apache/shindig/social/core/oauth2/OAuth2DataServiceImpl.java
<https://reviews.apache.org/r/1940/#comment5006>

    Could getValue be null?



http://svn.apache.org/repos/asf/shindig/trunk/java/social-api/src/main/java/org/apache/shindig/social/core/oauth2/OAuth2DataServiceImpl.java
<https://reviews.apache.org/r/1940/#comment5007>

    Use the Lists API



http://svn.apache.org/repos/asf/shindig/trunk/java/social-api/src/main/java/org/apache/shindig/social/core/oauth2/OAuth2DataServiceImpl.java
<https://reviews.apache.org/r/1940/#comment5008>

    Could getValue be null?



http://svn.apache.org/repos/asf/shindig/trunk/java/social-api/src/main/java/org/apache/shindig/social/core/oauth2/OAuth2DataServiceImpl.java
<https://reviews.apache.org/r/1940/#comment5009>

    Could getValue be null?



http://svn.apache.org/repos/asf/shindig/trunk/java/social-api/src/main/java/org/apache/shindig/social/core/oauth2/OAuth2DataServiceImpl.java
<https://reviews.apache.org/r/1940/#comment5010>

    Use the Lists API



http://svn.apache.org/repos/asf/shindig/trunk/java/social-api/src/main/java/org/apache/shindig/social/core/oauth2/OAuth2DataServiceImpl.java
<https://reviews.apache.org/r/1940/#comment5011>

    Could getValue be null?



http://svn.apache.org/repos/asf/shindig/trunk/java/social-api/src/main/java/org/apache/shindig/social/core/oauth2/OAuth2Exception.java
<https://reviews.apache.org/r/1940/#comment5012>

    Add javadoc



http://svn.apache.org/repos/asf/shindig/trunk/java/social-api/src/main/java/org/apache/shindig/social/core/oauth2/OAuth2Filter.java
<https://reviews.apache.org/r/1940/#comment5013>

    remove the @author



http://svn.apache.org/repos/asf/shindig/trunk/java/social-api/src/main/java/org/apache/shindig/social/core/oauth2/OAuth2NormalizedRequest.java
<https://reviews.apache.org/r/1940/#comment5014>

    Should the startsWith be case sensitive?



http://svn.apache.org/repos/asf/shindig/trunk/java/social-api/src/main/java/org/apache/shindig/social/core/oauth2/OAuth2NormalizedRequest.java
<https://reviews.apache.org/r/1940/#comment5015>

    Same thing should this be case sensitive?



http://svn.apache.org/repos/asf/shindig/trunk/java/social-api/src/main/java/org/apache/shindig/social/core/oauth2/OAuth2NormalizedRequest.java
<https://reviews.apache.org/r/1940/#comment5016>

    Log the exception



http://svn.apache.org/repos/asf/shindig/trunk/java/social-api/src/main/java/org/apache/shindig/social/core/oauth2/OAuth2NormalizedRequest.java
<https://reviews.apache.org/r/1940/#comment5017>

    Should we be hardcoding localhost:8080 here?



http://svn.apache.org/repos/asf/shindig/trunk/java/social-api/src/main/java/org/apache/shindig/social/core/oauth2/OAuth2NormalizedRequest.java
<https://reviews.apache.org/r/1940/#comment5018>

    Log the exception



http://svn.apache.org/repos/asf/shindig/trunk/java/social-api/src/main/java/org/apache/shindig/social/core/oauth2/OAuth2NormalizedRequest.java
<https://reviews.apache.org/r/1940/#comment5019>

    I think you need a finally here to make sure the InputStream is closed



http://svn.apache.org/repos/asf/shindig/trunk/java/social-api/src/main/java/org/apache/shindig/social/core/oauth2/OAuth2NormalizedResponse.java
<https://reviews.apache.org/r/1940/#comment5020>

    Use the Maps API



http://svn.apache.org/repos/asf/shindig/trunk/java/social-api/src/main/java/org/apache/shindig/social/core/oauth2/OAuth2NormalizedResponse.java
<https://reviews.apache.org/r/1940/#comment5021>

    Use the Maps API



http://svn.apache.org/repos/asf/shindig/trunk/java/social-api/src/main/java/org/apache/shindig/social/core/oauth2/OAuth2NormalizedResponse.java
<https://reviews.apache.org/r/1940/#comment5022>

    Might make sense to make these strings private static final since you are using them more than once in the code



http://svn.apache.org/repos/asf/shindig/trunk/java/social-api/src/main/java/org/apache/shindig/social/core/oauth2/OAuth2Service.java
<https://reviews.apache.org/r/1940/#comment5024>

    Add @param and @returns to the javadoc



http://svn.apache.org/repos/asf/shindig/trunk/java/social-api/src/main/java/org/apache/shindig/social/core/oauth2/OAuth2ServiceImpl.java
<https://reviews.apache.org/r/1940/#comment5025>

    Should these be configurable?  I can imagine people wanting the configure the expiration times.



http://svn.apache.org/repos/asf/shindig/trunk/java/social-api/src/main/java/org/apache/shindig/social/core/oauth2/OAuth2Servlet.java
<https://reviews.apache.org/r/1940/#comment5027>

    Log the exception and you probably want a finally clause here to make sure the PrintWriter is cleaned up properly



http://svn.apache.org/repos/asf/shindig/trunk/java/social-api/src/main/java/org/apache/shindig/social/core/oauth2/OAuth2TokenHandler.java
<https://reviews.apache.org/r/1940/#comment5028>

    Add Javadoc



http://svn.apache.org/repos/asf/shindig/trunk/java/social-api/src/main/java/org/apache/shindig/social/core/oauth2/OAuth2TokenHandler.java
<https://reviews.apache.org/r/1940/#comment5029>

    Log the exception



http://svn.apache.org/repos/asf/shindig/trunk/java/social-api/src/main/java/org/apache/shindig/social/core/oauth2/OAuth2Utils.java
<https://reviews.apache.org/r/1940/#comment5030>

    Add javadoc to all methods



http://svn.apache.org/repos/asf/shindig/trunk/java/social-api/src/main/java/org/apache/shindig/social/core/oauth2/OAuth2Utils.java
<https://reviews.apache.org/r/1940/#comment5031>

    Log the exception



http://svn.apache.org/repos/asf/shindig/trunk/java/social-api/src/main/java/org/apache/shindig/social/core/oauth2/validators/OAuth2ProtectedResourceValidator.java
<https://reviews.apache.org/r/1940/#comment5032>

    Add javadoc



http://svn.apache.org/repos/asf/shindig/trunk/java/social-api/src/main/java/org/apache/shindig/social/core/oauth2/validators/OAuth2RequestValidator.java
<https://reviews.apache.org/r/1940/#comment5033>

    Add Javadoc


- Ryan


On 2011-09-22 19:17:13, Eric Woods wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/1940/
> -----------------------------------------------------------
> 
> (Updated 2011-09-22 19:17:13)
> 
> 
> Review request for shindig.
> 
> 
> Summary
> -------
> 
> OAuth 2.0 service provider implementation in Apache Shindig.
> 
> Documentation wiki: http://docs.opensocial.org/display/OSD/OAuth+2.0+Service+Provider+Implementation+in+Apache+Shindig
> 
> JIRA issue: https://issues.apache.org/jira/browse/SHINDIG-1623
> 
> 
> Diffs
> -----
> 
>   http://svn.apache.org/repos/asf/shindig/trunk/java/social-api/src/main/java/org/apache/shindig/social/core/oauth2/OAuth2AuthenticationHandler.java PRE-CREATION 
>   http://svn.apache.org/repos/asf/shindig/trunk/java/social-api/src/main/java/org/apache/shindig/social/core/oauth2/OAuth2AuthorizationHandler.java PRE-CREATION 
>   http://svn.apache.org/repos/asf/shindig/trunk/java/social-api/src/main/java/org/apache/shindig/social/core/oauth2/OAuth2Client.java PRE-CREATION 
>   http://svn.apache.org/repos/asf/shindig/trunk/java/social-api/src/main/java/org/apache/shindig/social/core/oauth2/OAuth2Code.java PRE-CREATION 
>   http://svn.apache.org/repos/asf/shindig/trunk/java/social-api/src/main/java/org/apache/shindig/social/core/oauth2/OAuth2DataService.java PRE-CREATION 
>   http://svn.apache.org/repos/asf/shindig/trunk/java/social-api/src/main/java/org/apache/shindig/social/core/oauth2/OAuth2DataServiceImpl.java PRE-CREATION 
>   http://svn.apache.org/repos/asf/shindig/trunk/java/social-api/src/main/java/org/apache/shindig/social/core/oauth2/OAuth2Exception.java PRE-CREATION 
>   http://svn.apache.org/repos/asf/shindig/trunk/java/social-api/src/main/java/org/apache/shindig/social/core/oauth2/OAuth2Filter.java PRE-CREATION 
>   http://svn.apache.org/repos/asf/shindig/trunk/java/social-api/src/main/java/org/apache/shindig/social/core/oauth2/OAuth2NormalizedRequest.java PRE-CREATION 
>   http://svn.apache.org/repos/asf/shindig/trunk/java/social-api/src/main/java/org/apache/shindig/social/core/oauth2/OAuth2NormalizedResponse.java PRE-CREATION 
>   http://svn.apache.org/repos/asf/shindig/trunk/java/social-api/src/main/java/org/apache/shindig/social/core/oauth2/OAuth2Service.java PRE-CREATION 
>   http://svn.apache.org/repos/asf/shindig/trunk/java/social-api/src/main/java/org/apache/shindig/social/core/oauth2/OAuth2ServiceImpl.java PRE-CREATION 
>   http://svn.apache.org/repos/asf/shindig/trunk/java/social-api/src/main/java/org/apache/shindig/social/core/oauth2/OAuth2Servlet.java PRE-CREATION 
>   http://svn.apache.org/repos/asf/shindig/trunk/java/social-api/src/main/java/org/apache/shindig/social/core/oauth2/OAuth2TokenHandler.java PRE-CREATION 
>   http://svn.apache.org/repos/asf/shindig/trunk/java/social-api/src/main/java/org/apache/shindig/social/core/oauth2/OAuth2Types.java PRE-CREATION 
>   http://svn.apache.org/repos/asf/shindig/trunk/java/social-api/src/main/java/org/apache/shindig/social/core/oauth2/OAuth2Utils.java PRE-CREATION 
>   http://svn.apache.org/repos/asf/shindig/trunk/java/social-api/src/main/java/org/apache/shindig/social/core/oauth2/validators/AccessTokenRequestValidator.java PRE-CREATION 
>   http://svn.apache.org/repos/asf/shindig/trunk/java/social-api/src/main/java/org/apache/shindig/social/core/oauth2/validators/AuthCodeGrantValidator.java PRE-CREATION 
>   http://svn.apache.org/repos/asf/shindig/trunk/java/social-api/src/main/java/org/apache/shindig/social/core/oauth2/validators/AuthorizationCodeRequestValidator.java PRE-CREATION 
>   http://svn.apache.org/repos/asf/shindig/trunk/java/social-api/src/main/java/org/apache/shindig/social/core/oauth2/validators/ClientCredentialsGrantValidator.java PRE-CREATION 
>   http://svn.apache.org/repos/asf/shindig/trunk/java/social-api/src/main/java/org/apache/shindig/social/core/oauth2/validators/DefaultResourceRequestValidator.java PRE-CREATION 
>   http://svn.apache.org/repos/asf/shindig/trunk/java/social-api/src/main/java/org/apache/shindig/social/core/oauth2/validators/OAuth2GrantValidator.java PRE-CREATION 
>   http://svn.apache.org/repos/asf/shindig/trunk/java/social-api/src/main/java/org/apache/shindig/social/core/oauth2/validators/OAuth2ProtectedResourceValidator.java PRE-CREATION 
>   http://svn.apache.org/repos/asf/shindig/trunk/java/social-api/src/main/java/org/apache/shindig/social/core/oauth2/validators/OAuth2RequestValidator.java PRE-CREATION 
>   http://svn.apache.org/repos/asf/shindig/trunk/java/social-api/src/main/java/org/apache/shindig/social/sample/SampleModule.java 1174269 
>   http://svn.apache.org/repos/asf/shindig/trunk/java/social-api/src/test/java/org/apache/shindig/social/SocialApiTestsGuiceModule.java 1174269 
>   http://svn.apache.org/repos/asf/shindig/trunk/java/social-api/src/test/java/org/apache/shindig/social/core/config/SocialApiGuiceModuleTest.java 1174269 
>   http://svn.apache.org/repos/asf/shindig/trunk/java/social-api/src/test/java/org/apache/shindig/social/core/oauth/AuthenticationProviderHandlerTest.java 1174269 
>   http://svn.apache.org/repos/asf/shindig/trunk/java/social-api/src/test/java/org/apache/shindig/social/core/oauth/MockServletOutputStream.java PRE-CREATION 
>   http://svn.apache.org/repos/asf/shindig/trunk/java/social-api/src/test/java/org/apache/shindig/social/core/oauth/OAuth2AuthCodeFlowTest.java PRE-CREATION 
>   http://svn.apache.org/repos/asf/shindig/trunk/java/social-api/src/test/java/org/apache/shindig/social/core/oauth/OAuth2AuthenticationHandlerTest.java PRE-CREATION 
>   http://svn.apache.org/repos/asf/shindig/trunk/java/social-api/src/test/java/org/apache/shindig/social/core/oauth/OAuth2ClientCredentialFlowTest.java PRE-CREATION 
>   http://svn.apache.org/repos/asf/shindig/trunk/java/social-api/src/test/java/org/apache/shindig/social/core/oauth/OAuth2ImplicitFlowTest.java PRE-CREATION 
>   http://svn.apache.org/repos/asf/shindig/trunk/java/social-api/src/test/java/org/apache/shindig/social/core/oauth/OAuthAuthenticationHanderTest.java 1174269 
>   http://svn.apache.org/repos/asf/shindig/trunk/java/social-api/src/test/java/org/apache/shindig/social/dataservice/integration/AbstractLargeRestfulTests.java 1174269 
>   http://svn.apache.org/repos/asf/shindig/trunk/content/sampledata/canonicaldb.json 1174269 
>   http://svn.apache.org/repos/asf/shindig/trunk/java/server/src/main/webapp/WEB-INF/web.xml 1174269 
>   http://svn.apache.org/repos/asf/shindig/trunk/java/social-api/src/main/java/org/apache/shindig/social/core/oauth/AuthenticationHandlerProvider.java 1174269 
> 
> Diff: https://reviews.apache.org/r/1940/diff
> 
> 
> Testing
> -------
> 
> Yes, JUnits executed with maven build.
> 
> 
> Thanks,
> 
> Eric
> 
>

Re: Review Request: OAuth 2.0 service provider implementation in Apache Shindig.

Posted by Eric Woods <wo...@gmail.com>.


> On 2011-09-29 23:36:00, Henry Saputra wrote:
> > http://svn.apache.org/repos/asf/shindig/trunk/content/sampledata/canonicaldb.json, line 639
> > <https://reviews.apache.org/r/1940/diff/2/?file=45160#file45160line639>
> >
> >     Tabs instead of spaces?

Fixed.


> On 2011-09-29 23:36:00, Henry Saputra wrote:
> > http://svn.apache.org/repos/asf/shindig/trunk/java/social-api/src/main/java/org/apache/shindig/social/core/oauth/AuthenticationHandlerProvider.java, line 38
> > <https://reviews.apache.org/r/1940/diff/2/?file=45162#file45162line38>
> >
> >     Most auth for OpenSocial will be for gadget security token, its better to move this after OAuth 1.0 handler.

Done.


> On 2011-09-29 23:36:00, Henry Saputra wrote:
> > http://svn.apache.org/repos/asf/shindig/trunk/java/social-api/src/main/java/org/apache/shindig/social/core/oauth2/OAuth2AuthenticationHandler.java, line 28
> > <https://reviews.apache.org/r/1940/diff/2/?file=45163#file45163line28>
> >
> >     Add class header comment what is this class for?

Done.


- Eric


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


On 2011-09-22 19:17:13, Eric Woods wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/1940/
> -----------------------------------------------------------
> 
> (Updated 2011-09-22 19:17:13)
> 
> 
> Review request for shindig.
> 
> 
> Summary
> -------
> 
> OAuth 2.0 service provider implementation in Apache Shindig.
> 
> Documentation wiki: http://docs.opensocial.org/display/OSD/OAuth+2.0+Service+Provider+Implementation+in+Apache+Shindig
> 
> JIRA issue: https://issues.apache.org/jira/browse/SHINDIG-1623
> 
> 
> Diffs
> -----
> 
>   http://svn.apache.org/repos/asf/shindig/trunk/java/social-api/src/main/java/org/apache/shindig/social/core/oauth2/OAuth2AuthenticationHandler.java PRE-CREATION 
>   http://svn.apache.org/repos/asf/shindig/trunk/java/social-api/src/main/java/org/apache/shindig/social/core/oauth2/OAuth2AuthorizationHandler.java PRE-CREATION 
>   http://svn.apache.org/repos/asf/shindig/trunk/java/social-api/src/main/java/org/apache/shindig/social/core/oauth2/OAuth2Client.java PRE-CREATION 
>   http://svn.apache.org/repos/asf/shindig/trunk/java/social-api/src/main/java/org/apache/shindig/social/core/oauth2/OAuth2Code.java PRE-CREATION 
>   http://svn.apache.org/repos/asf/shindig/trunk/java/social-api/src/main/java/org/apache/shindig/social/core/oauth2/OAuth2DataService.java PRE-CREATION 
>   http://svn.apache.org/repos/asf/shindig/trunk/java/social-api/src/main/java/org/apache/shindig/social/core/oauth2/OAuth2DataServiceImpl.java PRE-CREATION 
>   http://svn.apache.org/repos/asf/shindig/trunk/java/social-api/src/main/java/org/apache/shindig/social/core/oauth2/OAuth2Exception.java PRE-CREATION 
>   http://svn.apache.org/repos/asf/shindig/trunk/java/social-api/src/main/java/org/apache/shindig/social/core/oauth2/OAuth2Filter.java PRE-CREATION 
>   http://svn.apache.org/repos/asf/shindig/trunk/java/social-api/src/main/java/org/apache/shindig/social/core/oauth2/OAuth2NormalizedRequest.java PRE-CREATION 
>   http://svn.apache.org/repos/asf/shindig/trunk/java/social-api/src/main/java/org/apache/shindig/social/core/oauth2/OAuth2NormalizedResponse.java PRE-CREATION 
>   http://svn.apache.org/repos/asf/shindig/trunk/java/social-api/src/main/java/org/apache/shindig/social/core/oauth2/OAuth2Service.java PRE-CREATION 
>   http://svn.apache.org/repos/asf/shindig/trunk/java/social-api/src/main/java/org/apache/shindig/social/core/oauth2/OAuth2ServiceImpl.java PRE-CREATION 
>   http://svn.apache.org/repos/asf/shindig/trunk/java/social-api/src/main/java/org/apache/shindig/social/core/oauth2/OAuth2Servlet.java PRE-CREATION 
>   http://svn.apache.org/repos/asf/shindig/trunk/java/social-api/src/main/java/org/apache/shindig/social/core/oauth2/OAuth2TokenHandler.java PRE-CREATION 
>   http://svn.apache.org/repos/asf/shindig/trunk/java/social-api/src/main/java/org/apache/shindig/social/core/oauth2/OAuth2Types.java PRE-CREATION 
>   http://svn.apache.org/repos/asf/shindig/trunk/java/social-api/src/main/java/org/apache/shindig/social/core/oauth2/OAuth2Utils.java PRE-CREATION 
>   http://svn.apache.org/repos/asf/shindig/trunk/java/social-api/src/main/java/org/apache/shindig/social/core/oauth2/validators/AccessTokenRequestValidator.java PRE-CREATION 
>   http://svn.apache.org/repos/asf/shindig/trunk/java/social-api/src/main/java/org/apache/shindig/social/core/oauth2/validators/AuthCodeGrantValidator.java PRE-CREATION 
>   http://svn.apache.org/repos/asf/shindig/trunk/java/social-api/src/main/java/org/apache/shindig/social/core/oauth2/validators/AuthorizationCodeRequestValidator.java PRE-CREATION 
>   http://svn.apache.org/repos/asf/shindig/trunk/java/social-api/src/main/java/org/apache/shindig/social/core/oauth2/validators/ClientCredentialsGrantValidator.java PRE-CREATION 
>   http://svn.apache.org/repos/asf/shindig/trunk/java/social-api/src/main/java/org/apache/shindig/social/core/oauth2/validators/DefaultResourceRequestValidator.java PRE-CREATION 
>   http://svn.apache.org/repos/asf/shindig/trunk/java/social-api/src/main/java/org/apache/shindig/social/core/oauth2/validators/OAuth2GrantValidator.java PRE-CREATION 
>   http://svn.apache.org/repos/asf/shindig/trunk/java/social-api/src/main/java/org/apache/shindig/social/core/oauth2/validators/OAuth2ProtectedResourceValidator.java PRE-CREATION 
>   http://svn.apache.org/repos/asf/shindig/trunk/java/social-api/src/main/java/org/apache/shindig/social/core/oauth2/validators/OAuth2RequestValidator.java PRE-CREATION 
>   http://svn.apache.org/repos/asf/shindig/trunk/java/social-api/src/main/java/org/apache/shindig/social/sample/SampleModule.java 1174269 
>   http://svn.apache.org/repos/asf/shindig/trunk/java/social-api/src/test/java/org/apache/shindig/social/SocialApiTestsGuiceModule.java 1174269 
>   http://svn.apache.org/repos/asf/shindig/trunk/java/social-api/src/test/java/org/apache/shindig/social/core/config/SocialApiGuiceModuleTest.java 1174269 
>   http://svn.apache.org/repos/asf/shindig/trunk/java/social-api/src/test/java/org/apache/shindig/social/core/oauth/AuthenticationProviderHandlerTest.java 1174269 
>   http://svn.apache.org/repos/asf/shindig/trunk/java/social-api/src/test/java/org/apache/shindig/social/core/oauth/MockServletOutputStream.java PRE-CREATION 
>   http://svn.apache.org/repos/asf/shindig/trunk/java/social-api/src/test/java/org/apache/shindig/social/core/oauth/OAuth2AuthCodeFlowTest.java PRE-CREATION 
>   http://svn.apache.org/repos/asf/shindig/trunk/java/social-api/src/test/java/org/apache/shindig/social/core/oauth/OAuth2AuthenticationHandlerTest.java PRE-CREATION 
>   http://svn.apache.org/repos/asf/shindig/trunk/java/social-api/src/test/java/org/apache/shindig/social/core/oauth/OAuth2ClientCredentialFlowTest.java PRE-CREATION 
>   http://svn.apache.org/repos/asf/shindig/trunk/java/social-api/src/test/java/org/apache/shindig/social/core/oauth/OAuth2ImplicitFlowTest.java PRE-CREATION 
>   http://svn.apache.org/repos/asf/shindig/trunk/java/social-api/src/test/java/org/apache/shindig/social/core/oauth/OAuthAuthenticationHanderTest.java 1174269 
>   http://svn.apache.org/repos/asf/shindig/trunk/java/social-api/src/test/java/org/apache/shindig/social/dataservice/integration/AbstractLargeRestfulTests.java 1174269 
>   http://svn.apache.org/repos/asf/shindig/trunk/content/sampledata/canonicaldb.json 1174269 
>   http://svn.apache.org/repos/asf/shindig/trunk/java/server/src/main/webapp/WEB-INF/web.xml 1174269 
>   http://svn.apache.org/repos/asf/shindig/trunk/java/social-api/src/main/java/org/apache/shindig/social/core/oauth/AuthenticationHandlerProvider.java 1174269 
> 
> Diff: https://reviews.apache.org/r/1940/diff
> 
> 
> Testing
> -------
> 
> Yes, JUnits executed with maven build.
> 
> 
> Thanks,
> 
> Eric
> 
>

Re: Review Request: OAuth 2.0 service provider implementation in Apache Shindig.

Posted by Henry Saputra <hs...@apache.org>.

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


So much code =) Just initial review.


http://svn.apache.org/repos/asf/shindig/trunk/content/sampledata/canonicaldb.json
<https://reviews.apache.org/r/1940/#comment5040>

    Tabs instead of spaces?



http://svn.apache.org/repos/asf/shindig/trunk/java/social-api/src/main/java/org/apache/shindig/social/core/oauth/AuthenticationHandlerProvider.java
<https://reviews.apache.org/r/1940/#comment5043>

    Most auth for OpenSocial will be for gadget security token, its better to move this after OAuth 1.0 handler.



http://svn.apache.org/repos/asf/shindig/trunk/java/social-api/src/main/java/org/apache/shindig/social/core/oauth2/OAuth2AuthenticationHandler.java
<https://reviews.apache.org/r/1940/#comment5044>

    Add class header comment what is this class for?


- Henry


On 2011-09-22 19:17:13, Eric Woods wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/1940/
> -----------------------------------------------------------
> 
> (Updated 2011-09-22 19:17:13)
> 
> 
> Review request for shindig.
> 
> 
> Summary
> -------
> 
> OAuth 2.0 service provider implementation in Apache Shindig.
> 
> Documentation wiki: http://docs.opensocial.org/display/OSD/OAuth+2.0+Service+Provider+Implementation+in+Apache+Shindig
> 
> JIRA issue: https://issues.apache.org/jira/browse/SHINDIG-1623
> 
> 
> Diffs
> -----
> 
>   http://svn.apache.org/repos/asf/shindig/trunk/java/social-api/src/main/java/org/apache/shindig/social/core/oauth2/OAuth2AuthenticationHandler.java PRE-CREATION 
>   http://svn.apache.org/repos/asf/shindig/trunk/java/social-api/src/main/java/org/apache/shindig/social/core/oauth2/OAuth2AuthorizationHandler.java PRE-CREATION 
>   http://svn.apache.org/repos/asf/shindig/trunk/java/social-api/src/main/java/org/apache/shindig/social/core/oauth2/OAuth2Client.java PRE-CREATION 
>   http://svn.apache.org/repos/asf/shindig/trunk/java/social-api/src/main/java/org/apache/shindig/social/core/oauth2/OAuth2Code.java PRE-CREATION 
>   http://svn.apache.org/repos/asf/shindig/trunk/java/social-api/src/main/java/org/apache/shindig/social/core/oauth2/OAuth2DataService.java PRE-CREATION 
>   http://svn.apache.org/repos/asf/shindig/trunk/java/social-api/src/main/java/org/apache/shindig/social/core/oauth2/OAuth2DataServiceImpl.java PRE-CREATION 
>   http://svn.apache.org/repos/asf/shindig/trunk/java/social-api/src/main/java/org/apache/shindig/social/core/oauth2/OAuth2Exception.java PRE-CREATION 
>   http://svn.apache.org/repos/asf/shindig/trunk/java/social-api/src/main/java/org/apache/shindig/social/core/oauth2/OAuth2Filter.java PRE-CREATION 
>   http://svn.apache.org/repos/asf/shindig/trunk/java/social-api/src/main/java/org/apache/shindig/social/core/oauth2/OAuth2NormalizedRequest.java PRE-CREATION 
>   http://svn.apache.org/repos/asf/shindig/trunk/java/social-api/src/main/java/org/apache/shindig/social/core/oauth2/OAuth2NormalizedResponse.java PRE-CREATION 
>   http://svn.apache.org/repos/asf/shindig/trunk/java/social-api/src/main/java/org/apache/shindig/social/core/oauth2/OAuth2Service.java PRE-CREATION 
>   http://svn.apache.org/repos/asf/shindig/trunk/java/social-api/src/main/java/org/apache/shindig/social/core/oauth2/OAuth2ServiceImpl.java PRE-CREATION 
>   http://svn.apache.org/repos/asf/shindig/trunk/java/social-api/src/main/java/org/apache/shindig/social/core/oauth2/OAuth2Servlet.java PRE-CREATION 
>   http://svn.apache.org/repos/asf/shindig/trunk/java/social-api/src/main/java/org/apache/shindig/social/core/oauth2/OAuth2TokenHandler.java PRE-CREATION 
>   http://svn.apache.org/repos/asf/shindig/trunk/java/social-api/src/main/java/org/apache/shindig/social/core/oauth2/OAuth2Types.java PRE-CREATION 
>   http://svn.apache.org/repos/asf/shindig/trunk/java/social-api/src/main/java/org/apache/shindig/social/core/oauth2/OAuth2Utils.java PRE-CREATION 
>   http://svn.apache.org/repos/asf/shindig/trunk/java/social-api/src/main/java/org/apache/shindig/social/core/oauth2/validators/AccessTokenRequestValidator.java PRE-CREATION 
>   http://svn.apache.org/repos/asf/shindig/trunk/java/social-api/src/main/java/org/apache/shindig/social/core/oauth2/validators/AuthCodeGrantValidator.java PRE-CREATION 
>   http://svn.apache.org/repos/asf/shindig/trunk/java/social-api/src/main/java/org/apache/shindig/social/core/oauth2/validators/AuthorizationCodeRequestValidator.java PRE-CREATION 
>   http://svn.apache.org/repos/asf/shindig/trunk/java/social-api/src/main/java/org/apache/shindig/social/core/oauth2/validators/ClientCredentialsGrantValidator.java PRE-CREATION 
>   http://svn.apache.org/repos/asf/shindig/trunk/java/social-api/src/main/java/org/apache/shindig/social/core/oauth2/validators/DefaultResourceRequestValidator.java PRE-CREATION 
>   http://svn.apache.org/repos/asf/shindig/trunk/java/social-api/src/main/java/org/apache/shindig/social/core/oauth2/validators/OAuth2GrantValidator.java PRE-CREATION 
>   http://svn.apache.org/repos/asf/shindig/trunk/java/social-api/src/main/java/org/apache/shindig/social/core/oauth2/validators/OAuth2ProtectedResourceValidator.java PRE-CREATION 
>   http://svn.apache.org/repos/asf/shindig/trunk/java/social-api/src/main/java/org/apache/shindig/social/core/oauth2/validators/OAuth2RequestValidator.java PRE-CREATION 
>   http://svn.apache.org/repos/asf/shindig/trunk/java/social-api/src/main/java/org/apache/shindig/social/sample/SampleModule.java 1174269 
>   http://svn.apache.org/repos/asf/shindig/trunk/java/social-api/src/test/java/org/apache/shindig/social/SocialApiTestsGuiceModule.java 1174269 
>   http://svn.apache.org/repos/asf/shindig/trunk/java/social-api/src/test/java/org/apache/shindig/social/core/config/SocialApiGuiceModuleTest.java 1174269 
>   http://svn.apache.org/repos/asf/shindig/trunk/java/social-api/src/test/java/org/apache/shindig/social/core/oauth/AuthenticationProviderHandlerTest.java 1174269 
>   http://svn.apache.org/repos/asf/shindig/trunk/java/social-api/src/test/java/org/apache/shindig/social/core/oauth/MockServletOutputStream.java PRE-CREATION 
>   http://svn.apache.org/repos/asf/shindig/trunk/java/social-api/src/test/java/org/apache/shindig/social/core/oauth/OAuth2AuthCodeFlowTest.java PRE-CREATION 
>   http://svn.apache.org/repos/asf/shindig/trunk/java/social-api/src/test/java/org/apache/shindig/social/core/oauth/OAuth2AuthenticationHandlerTest.java PRE-CREATION 
>   http://svn.apache.org/repos/asf/shindig/trunk/java/social-api/src/test/java/org/apache/shindig/social/core/oauth/OAuth2ClientCredentialFlowTest.java PRE-CREATION 
>   http://svn.apache.org/repos/asf/shindig/trunk/java/social-api/src/test/java/org/apache/shindig/social/core/oauth/OAuth2ImplicitFlowTest.java PRE-CREATION 
>   http://svn.apache.org/repos/asf/shindig/trunk/java/social-api/src/test/java/org/apache/shindig/social/core/oauth/OAuthAuthenticationHanderTest.java 1174269 
>   http://svn.apache.org/repos/asf/shindig/trunk/java/social-api/src/test/java/org/apache/shindig/social/dataservice/integration/AbstractLargeRestfulTests.java 1174269 
>   http://svn.apache.org/repos/asf/shindig/trunk/content/sampledata/canonicaldb.json 1174269 
>   http://svn.apache.org/repos/asf/shindig/trunk/java/server/src/main/webapp/WEB-INF/web.xml 1174269 
>   http://svn.apache.org/repos/asf/shindig/trunk/java/social-api/src/main/java/org/apache/shindig/social/core/oauth/AuthenticationHandlerProvider.java 1174269 
> 
> Diff: https://reviews.apache.org/r/1940/diff
> 
> 
> Testing
> -------
> 
> Yes, JUnits executed with maven build.
> 
> 
> Thanks,
> 
> Eric
> 
>

Re: Review Request: OAuth 2.0 service provider implementation in Apache Shindig.

Posted by Eric Woods <wo...@gmail.com>.

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

Ship it!


Patch applied!

- Eric


On 2011-10-10 20:41:02, Eric Woods wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/1940/
> -----------------------------------------------------------
> 
> (Updated 2011-10-10 20:41:02)
> 
> 
> Review request for shindig.
> 
> 
> Summary
> -------
> 
> OAuth 2.0 service provider implementation in Apache Shindig.
> 
> Documentation wiki: http://docs.opensocial.org/display/OSD/OAuth+2.0+Service+Provider+Implementation+in+Apache+Shindig
> 
> JIRA issue: https://issues.apache.org/jira/browse/SHINDIG-1623
> 
> 
> Diffs
> -----
> 
>   http://svn.apache.org/repos/asf/shindig/trunk/content/sampledata/canonicaldb.json 1181203 
>   http://svn.apache.org/repos/asf/shindig/trunk/java/common/conf/shindig.properties 1181203 
>   http://svn.apache.org/repos/asf/shindig/trunk/java/server/src/main/webapp/WEB-INF/web.xml 1181203 
>   http://svn.apache.org/repos/asf/shindig/trunk/java/social-api/src/main/java/org/apache/shindig/social/core/oauth/AuthenticationHandlerProvider.java 1181203 
>   http://svn.apache.org/repos/asf/shindig/trunk/java/social-api/src/main/java/org/apache/shindig/social/core/oauth2/OAuth2AuthenticationHandler.java PRE-CREATION 
>   http://svn.apache.org/repos/asf/shindig/trunk/java/social-api/src/main/java/org/apache/shindig/social/core/oauth2/OAuth2AuthorizationHandler.java PRE-CREATION 
>   http://svn.apache.org/repos/asf/shindig/trunk/java/social-api/src/main/java/org/apache/shindig/social/core/oauth2/OAuth2Client.java PRE-CREATION 
>   http://svn.apache.org/repos/asf/shindig/trunk/java/social-api/src/main/java/org/apache/shindig/social/core/oauth2/OAuth2Code.java PRE-CREATION 
>   http://svn.apache.org/repos/asf/shindig/trunk/java/social-api/src/main/java/org/apache/shindig/social/core/oauth2/OAuth2DataService.java PRE-CREATION 
>   http://svn.apache.org/repos/asf/shindig/trunk/java/social-api/src/main/java/org/apache/shindig/social/core/oauth2/OAuth2DataServiceImpl.java PRE-CREATION 
>   http://svn.apache.org/repos/asf/shindig/trunk/java/social-api/src/main/java/org/apache/shindig/social/core/oauth2/OAuth2Exception.java PRE-CREATION 
>   http://svn.apache.org/repos/asf/shindig/trunk/java/social-api/src/main/java/org/apache/shindig/social/core/oauth2/OAuth2NormalizedRequest.java PRE-CREATION 
>   http://svn.apache.org/repos/asf/shindig/trunk/java/social-api/src/main/java/org/apache/shindig/social/core/oauth2/OAuth2NormalizedResponse.java PRE-CREATION 
>   http://svn.apache.org/repos/asf/shindig/trunk/java/social-api/src/main/java/org/apache/shindig/social/core/oauth2/OAuth2Service.java PRE-CREATION 
>   http://svn.apache.org/repos/asf/shindig/trunk/java/social-api/src/main/java/org/apache/shindig/social/core/oauth2/OAuth2ServiceImpl.java PRE-CREATION 
>   http://svn.apache.org/repos/asf/shindig/trunk/java/social-api/src/main/java/org/apache/shindig/social/core/oauth2/OAuth2Servlet.java PRE-CREATION 
>   http://svn.apache.org/repos/asf/shindig/trunk/java/social-api/src/main/java/org/apache/shindig/social/core/oauth2/OAuth2TokenHandler.java PRE-CREATION 
>   http://svn.apache.org/repos/asf/shindig/trunk/java/social-api/src/main/java/org/apache/shindig/social/core/oauth2/OAuth2Types.java PRE-CREATION 
>   http://svn.apache.org/repos/asf/shindig/trunk/java/social-api/src/main/java/org/apache/shindig/social/core/oauth2/OAuth2Utils.java PRE-CREATION 
>   http://svn.apache.org/repos/asf/shindig/trunk/java/social-api/src/main/java/org/apache/shindig/social/core/oauth2/validators/AccessTokenRequestValidator.java PRE-CREATION 
>   http://svn.apache.org/repos/asf/shindig/trunk/java/social-api/src/main/java/org/apache/shindig/social/core/oauth2/validators/AuthCodeGrantValidator.java PRE-CREATION 
>   http://svn.apache.org/repos/asf/shindig/trunk/java/social-api/src/main/java/org/apache/shindig/social/core/oauth2/validators/AuthorizationCodeRequestValidator.java PRE-CREATION 
>   http://svn.apache.org/repos/asf/shindig/trunk/java/social-api/src/main/java/org/apache/shindig/social/core/oauth2/validators/ClientCredentialsGrantValidator.java PRE-CREATION 
>   http://svn.apache.org/repos/asf/shindig/trunk/java/social-api/src/main/java/org/apache/shindig/social/core/oauth2/validators/DefaultResourceRequestValidator.java PRE-CREATION 
>   http://svn.apache.org/repos/asf/shindig/trunk/java/social-api/src/main/java/org/apache/shindig/social/core/oauth2/validators/OAuth2GrantValidator.java PRE-CREATION 
>   http://svn.apache.org/repos/asf/shindig/trunk/java/social-api/src/main/java/org/apache/shindig/social/core/oauth2/validators/OAuth2ProtectedResourceValidator.java PRE-CREATION 
>   http://svn.apache.org/repos/asf/shindig/trunk/java/social-api/src/main/java/org/apache/shindig/social/core/oauth2/validators/OAuth2RequestValidator.java PRE-CREATION 
>   http://svn.apache.org/repos/asf/shindig/trunk/java/social-api/src/main/java/org/apache/shindig/social/sample/SampleModule.java 1181203 
>   http://svn.apache.org/repos/asf/shindig/trunk/java/social-api/src/test/java/org/apache/shindig/social/SocialApiTestsGuiceModule.java 1181203 
>   http://svn.apache.org/repos/asf/shindig/trunk/java/social-api/src/test/java/org/apache/shindig/social/core/config/SocialApiGuiceModuleTest.java 1181203 
>   http://svn.apache.org/repos/asf/shindig/trunk/java/social-api/src/test/java/org/apache/shindig/social/core/oauth/AuthenticationProviderHandlerTest.java 1181203 
>   http://svn.apache.org/repos/asf/shindig/trunk/java/social-api/src/test/java/org/apache/shindig/social/core/oauth/MockServletOutputStream.java PRE-CREATION 
>   http://svn.apache.org/repos/asf/shindig/trunk/java/social-api/src/test/java/org/apache/shindig/social/core/oauth/OAuth2AuthCodeFlowTest.java PRE-CREATION 
>   http://svn.apache.org/repos/asf/shindig/trunk/java/social-api/src/test/java/org/apache/shindig/social/core/oauth/OAuth2AuthenticationHandlerTest.java PRE-CREATION 
>   http://svn.apache.org/repos/asf/shindig/trunk/java/social-api/src/test/java/org/apache/shindig/social/core/oauth/OAuth2ClientCredentialFlowTest.java PRE-CREATION 
>   http://svn.apache.org/repos/asf/shindig/trunk/java/social-api/src/test/java/org/apache/shindig/social/core/oauth/OAuth2ImplicitFlowTest.java PRE-CREATION 
>   http://svn.apache.org/repos/asf/shindig/trunk/java/social-api/src/test/java/org/apache/shindig/social/core/oauth/OAuthAuthenticationHanderTest.java 1181203 
>   http://svn.apache.org/repos/asf/shindig/trunk/java/social-api/src/test/java/org/apache/shindig/social/dataservice/integration/AbstractLargeRestfulTests.java 1181203 
> 
> Diff: https://reviews.apache.org/r/1940/diff
> 
> 
> Testing
> -------
> 
> Yes, JUnits executed with maven build.
> 
> 
> Thanks,
> 
> Eric
> 
>

Re: Review Request: OAuth 2.0 service provider implementation in Apache Shindig.

Posted by dd...@apache.org.

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

(Updated June 26, 2012, 6:46 p.m.)


Review request for shindig.


Description
-------

OAuth 2.0 service provider implementation in Apache Shindig.

Documentation wiki: http://docs.opensocial.org/display/OSD/OAuth+2.0+Service+Provider+Implementation+in+Apache+Shindig

JIRA issue: https://issues.apache.org/jira/browse/SHINDIG-1623


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


Diffs
-----

  http://svn.apache.org/repos/asf/shindig/trunk/content/sampledata/canonicaldb.json 1181203 
  http://svn.apache.org/repos/asf/shindig/trunk/java/common/conf/shindig.properties 1181203 
  http://svn.apache.org/repos/asf/shindig/trunk/java/server/src/main/webapp/WEB-INF/web.xml 1181203 
  http://svn.apache.org/repos/asf/shindig/trunk/java/social-api/src/main/java/org/apache/shindig/social/core/oauth/AuthenticationHandlerProvider.java 1181203 
  http://svn.apache.org/repos/asf/shindig/trunk/java/social-api/src/main/java/org/apache/shindig/social/core/oauth2/OAuth2AuthenticationHandler.java PRE-CREATION 
  http://svn.apache.org/repos/asf/shindig/trunk/java/social-api/src/main/java/org/apache/shindig/social/core/oauth2/OAuth2AuthorizationHandler.java PRE-CREATION 
  http://svn.apache.org/repos/asf/shindig/trunk/java/social-api/src/main/java/org/apache/shindig/social/core/oauth2/OAuth2Client.java PRE-CREATION 
  http://svn.apache.org/repos/asf/shindig/trunk/java/social-api/src/main/java/org/apache/shindig/social/core/oauth2/OAuth2Code.java PRE-CREATION 
  http://svn.apache.org/repos/asf/shindig/trunk/java/social-api/src/main/java/org/apache/shindig/social/core/oauth2/OAuth2DataService.java PRE-CREATION 
  http://svn.apache.org/repos/asf/shindig/trunk/java/social-api/src/main/java/org/apache/shindig/social/core/oauth2/OAuth2DataServiceImpl.java PRE-CREATION 
  http://svn.apache.org/repos/asf/shindig/trunk/java/social-api/src/main/java/org/apache/shindig/social/core/oauth2/OAuth2Exception.java PRE-CREATION 
  http://svn.apache.org/repos/asf/shindig/trunk/java/social-api/src/main/java/org/apache/shindig/social/core/oauth2/OAuth2NormalizedRequest.java PRE-CREATION 
  http://svn.apache.org/repos/asf/shindig/trunk/java/social-api/src/main/java/org/apache/shindig/social/core/oauth2/OAuth2NormalizedResponse.java PRE-CREATION 
  http://svn.apache.org/repos/asf/shindig/trunk/java/social-api/src/main/java/org/apache/shindig/social/core/oauth2/OAuth2Service.java PRE-CREATION 
  http://svn.apache.org/repos/asf/shindig/trunk/java/social-api/src/main/java/org/apache/shindig/social/core/oauth2/OAuth2ServiceImpl.java PRE-CREATION 
  http://svn.apache.org/repos/asf/shindig/trunk/java/social-api/src/main/java/org/apache/shindig/social/core/oauth2/OAuth2Servlet.java PRE-CREATION 
  http://svn.apache.org/repos/asf/shindig/trunk/java/social-api/src/main/java/org/apache/shindig/social/core/oauth2/OAuth2TokenHandler.java PRE-CREATION 
  http://svn.apache.org/repos/asf/shindig/trunk/java/social-api/src/main/java/org/apache/shindig/social/core/oauth2/OAuth2Types.java PRE-CREATION 
  http://svn.apache.org/repos/asf/shindig/trunk/java/social-api/src/main/java/org/apache/shindig/social/core/oauth2/OAuth2Utils.java PRE-CREATION 
  http://svn.apache.org/repos/asf/shindig/trunk/java/social-api/src/main/java/org/apache/shindig/social/core/oauth2/validators/AccessTokenRequestValidator.java PRE-CREATION 
  http://svn.apache.org/repos/asf/shindig/trunk/java/social-api/src/main/java/org/apache/shindig/social/core/oauth2/validators/AuthCodeGrantValidator.java PRE-CREATION 
  http://svn.apache.org/repos/asf/shindig/trunk/java/social-api/src/main/java/org/apache/shindig/social/core/oauth2/validators/AuthorizationCodeRequestValidator.java PRE-CREATION 
  http://svn.apache.org/repos/asf/shindig/trunk/java/social-api/src/main/java/org/apache/shindig/social/core/oauth2/validators/ClientCredentialsGrantValidator.java PRE-CREATION 
  http://svn.apache.org/repos/asf/shindig/trunk/java/social-api/src/main/java/org/apache/shindig/social/core/oauth2/validators/DefaultResourceRequestValidator.java PRE-CREATION 
  http://svn.apache.org/repos/asf/shindig/trunk/java/social-api/src/main/java/org/apache/shindig/social/core/oauth2/validators/OAuth2GrantValidator.java PRE-CREATION 
  http://svn.apache.org/repos/asf/shindig/trunk/java/social-api/src/main/java/org/apache/shindig/social/core/oauth2/validators/OAuth2ProtectedResourceValidator.java PRE-CREATION 
  http://svn.apache.org/repos/asf/shindig/trunk/java/social-api/src/main/java/org/apache/shindig/social/core/oauth2/validators/OAuth2RequestValidator.java PRE-CREATION 
  http://svn.apache.org/repos/asf/shindig/trunk/java/social-api/src/main/java/org/apache/shindig/social/sample/SampleModule.java 1181203 
  http://svn.apache.org/repos/asf/shindig/trunk/java/social-api/src/test/java/org/apache/shindig/social/SocialApiTestsGuiceModule.java 1181203 
  http://svn.apache.org/repos/asf/shindig/trunk/java/social-api/src/test/java/org/apache/shindig/social/core/config/SocialApiGuiceModuleTest.java 1181203 
  http://svn.apache.org/repos/asf/shindig/trunk/java/social-api/src/test/java/org/apache/shindig/social/core/oauth/AuthenticationProviderHandlerTest.java 1181203 
  http://svn.apache.org/repos/asf/shindig/trunk/java/social-api/src/test/java/org/apache/shindig/social/core/oauth/MockServletOutputStream.java PRE-CREATION 
  http://svn.apache.org/repos/asf/shindig/trunk/java/social-api/src/test/java/org/apache/shindig/social/core/oauth/OAuth2AuthCodeFlowTest.java PRE-CREATION 
  http://svn.apache.org/repos/asf/shindig/trunk/java/social-api/src/test/java/org/apache/shindig/social/core/oauth/OAuth2AuthenticationHandlerTest.java PRE-CREATION 
  http://svn.apache.org/repos/asf/shindig/trunk/java/social-api/src/test/java/org/apache/shindig/social/core/oauth/OAuth2ClientCredentialFlowTest.java PRE-CREATION 
  http://svn.apache.org/repos/asf/shindig/trunk/java/social-api/src/test/java/org/apache/shindig/social/core/oauth/OAuth2ImplicitFlowTest.java PRE-CREATION 
  http://svn.apache.org/repos/asf/shindig/trunk/java/social-api/src/test/java/org/apache/shindig/social/core/oauth/OAuthAuthenticationHanderTest.java 1181203 
  http://svn.apache.org/repos/asf/shindig/trunk/java/social-api/src/test/java/org/apache/shindig/social/dataservice/integration/AbstractLargeRestfulTests.java 1181203 

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


Testing
-------

Yes, JUnits executed with maven build.


Thanks,

Eric Woods

Re: Review Request: OAuth 2.0 service provider implementation in Apache Shindig.

Posted by Eric Woods <wo...@gmail.com>.

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

(Updated 2011-10-10 20:41:02.373211)


Review request for shindig.


Changes
-------

Updated to address Ryan Baxter's and Henry Saputra's feedback.  Additional feedback welcome.

This code has seasoned pretty well.  I'll give it a few more days and commit unless I receive additional items.  Thanks.


Summary
-------

OAuth 2.0 service provider implementation in Apache Shindig.

Documentation wiki: http://docs.opensocial.org/display/OSD/OAuth+2.0+Service+Provider+Implementation+in+Apache+Shindig

JIRA issue: https://issues.apache.org/jira/browse/SHINDIG-1623


Diffs (updated)
-----

  http://svn.apache.org/repos/asf/shindig/trunk/content/sampledata/canonicaldb.json 1181203 
  http://svn.apache.org/repos/asf/shindig/trunk/java/common/conf/shindig.properties 1181203 
  http://svn.apache.org/repos/asf/shindig/trunk/java/server/src/main/webapp/WEB-INF/web.xml 1181203 
  http://svn.apache.org/repos/asf/shindig/trunk/java/social-api/src/main/java/org/apache/shindig/social/core/oauth/AuthenticationHandlerProvider.java 1181203 
  http://svn.apache.org/repos/asf/shindig/trunk/java/social-api/src/main/java/org/apache/shindig/social/core/oauth2/OAuth2AuthenticationHandler.java PRE-CREATION 
  http://svn.apache.org/repos/asf/shindig/trunk/java/social-api/src/main/java/org/apache/shindig/social/core/oauth2/OAuth2AuthorizationHandler.java PRE-CREATION 
  http://svn.apache.org/repos/asf/shindig/trunk/java/social-api/src/main/java/org/apache/shindig/social/core/oauth2/OAuth2Client.java PRE-CREATION 
  http://svn.apache.org/repos/asf/shindig/trunk/java/social-api/src/main/java/org/apache/shindig/social/core/oauth2/OAuth2Code.java PRE-CREATION 
  http://svn.apache.org/repos/asf/shindig/trunk/java/social-api/src/main/java/org/apache/shindig/social/core/oauth2/OAuth2DataService.java PRE-CREATION 
  http://svn.apache.org/repos/asf/shindig/trunk/java/social-api/src/main/java/org/apache/shindig/social/core/oauth2/OAuth2DataServiceImpl.java PRE-CREATION 
  http://svn.apache.org/repos/asf/shindig/trunk/java/social-api/src/main/java/org/apache/shindig/social/core/oauth2/OAuth2Exception.java PRE-CREATION 
  http://svn.apache.org/repos/asf/shindig/trunk/java/social-api/src/main/java/org/apache/shindig/social/core/oauth2/OAuth2NormalizedRequest.java PRE-CREATION 
  http://svn.apache.org/repos/asf/shindig/trunk/java/social-api/src/main/java/org/apache/shindig/social/core/oauth2/OAuth2NormalizedResponse.java PRE-CREATION 
  http://svn.apache.org/repos/asf/shindig/trunk/java/social-api/src/main/java/org/apache/shindig/social/core/oauth2/OAuth2Service.java PRE-CREATION 
  http://svn.apache.org/repos/asf/shindig/trunk/java/social-api/src/main/java/org/apache/shindig/social/core/oauth2/OAuth2ServiceImpl.java PRE-CREATION 
  http://svn.apache.org/repos/asf/shindig/trunk/java/social-api/src/main/java/org/apache/shindig/social/core/oauth2/OAuth2Servlet.java PRE-CREATION 
  http://svn.apache.org/repos/asf/shindig/trunk/java/social-api/src/main/java/org/apache/shindig/social/core/oauth2/OAuth2TokenHandler.java PRE-CREATION 
  http://svn.apache.org/repos/asf/shindig/trunk/java/social-api/src/main/java/org/apache/shindig/social/core/oauth2/OAuth2Types.java PRE-CREATION 
  http://svn.apache.org/repos/asf/shindig/trunk/java/social-api/src/main/java/org/apache/shindig/social/core/oauth2/OAuth2Utils.java PRE-CREATION 
  http://svn.apache.org/repos/asf/shindig/trunk/java/social-api/src/main/java/org/apache/shindig/social/core/oauth2/validators/AccessTokenRequestValidator.java PRE-CREATION 
  http://svn.apache.org/repos/asf/shindig/trunk/java/social-api/src/main/java/org/apache/shindig/social/core/oauth2/validators/AuthCodeGrantValidator.java PRE-CREATION 
  http://svn.apache.org/repos/asf/shindig/trunk/java/social-api/src/main/java/org/apache/shindig/social/core/oauth2/validators/AuthorizationCodeRequestValidator.java PRE-CREATION 
  http://svn.apache.org/repos/asf/shindig/trunk/java/social-api/src/main/java/org/apache/shindig/social/core/oauth2/validators/ClientCredentialsGrantValidator.java PRE-CREATION 
  http://svn.apache.org/repos/asf/shindig/trunk/java/social-api/src/main/java/org/apache/shindig/social/core/oauth2/validators/DefaultResourceRequestValidator.java PRE-CREATION 
  http://svn.apache.org/repos/asf/shindig/trunk/java/social-api/src/main/java/org/apache/shindig/social/core/oauth2/validators/OAuth2GrantValidator.java PRE-CREATION 
  http://svn.apache.org/repos/asf/shindig/trunk/java/social-api/src/main/java/org/apache/shindig/social/core/oauth2/validators/OAuth2ProtectedResourceValidator.java PRE-CREATION 
  http://svn.apache.org/repos/asf/shindig/trunk/java/social-api/src/main/java/org/apache/shindig/social/core/oauth2/validators/OAuth2RequestValidator.java PRE-CREATION 
  http://svn.apache.org/repos/asf/shindig/trunk/java/social-api/src/main/java/org/apache/shindig/social/sample/SampleModule.java 1181203 
  http://svn.apache.org/repos/asf/shindig/trunk/java/social-api/src/test/java/org/apache/shindig/social/SocialApiTestsGuiceModule.java 1181203 
  http://svn.apache.org/repos/asf/shindig/trunk/java/social-api/src/test/java/org/apache/shindig/social/core/config/SocialApiGuiceModuleTest.java 1181203 
  http://svn.apache.org/repos/asf/shindig/trunk/java/social-api/src/test/java/org/apache/shindig/social/core/oauth/AuthenticationProviderHandlerTest.java 1181203 
  http://svn.apache.org/repos/asf/shindig/trunk/java/social-api/src/test/java/org/apache/shindig/social/core/oauth/MockServletOutputStream.java PRE-CREATION 
  http://svn.apache.org/repos/asf/shindig/trunk/java/social-api/src/test/java/org/apache/shindig/social/core/oauth/OAuth2AuthCodeFlowTest.java PRE-CREATION 
  http://svn.apache.org/repos/asf/shindig/trunk/java/social-api/src/test/java/org/apache/shindig/social/core/oauth/OAuth2AuthenticationHandlerTest.java PRE-CREATION 
  http://svn.apache.org/repos/asf/shindig/trunk/java/social-api/src/test/java/org/apache/shindig/social/core/oauth/OAuth2ClientCredentialFlowTest.java PRE-CREATION 
  http://svn.apache.org/repos/asf/shindig/trunk/java/social-api/src/test/java/org/apache/shindig/social/core/oauth/OAuth2ImplicitFlowTest.java PRE-CREATION 
  http://svn.apache.org/repos/asf/shindig/trunk/java/social-api/src/test/java/org/apache/shindig/social/core/oauth/OAuthAuthenticationHanderTest.java 1181203 
  http://svn.apache.org/repos/asf/shindig/trunk/java/social-api/src/test/java/org/apache/shindig/social/dataservice/integration/AbstractLargeRestfulTests.java 1181203 

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


Testing
-------

Yes, JUnits executed with maven build.


Thanks,

Eric

Re: Review Request: OAuth 2.0 service provider implementation in Apache Shindig.

Posted by Eric Woods <wo...@gmail.com>.

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

(Updated 2011-09-22 19:17:13.697848)


Review request for shindig.


Changes
-------

Updated patch to support OAuth 2.0 service provider based on feedback.  Updates include:
- General code formatting to conform with Shindig conventions.
- TODOs identified by name.
- Utilities now use UriBuilder where applicable.
- Apache license stamp added to remaining files.
- Additional tests for token revokation.
- Use of printStackTrace() removed.

All tests passing.


Summary
-------

OAuth 2.0 service provider implementation in Apache Shindig.

Documentation wiki: http://docs.opensocial.org/display/OSD/OAuth+2.0+Service+Provider+Implementation+in+Apache+Shindig

JIRA issue: https://issues.apache.org/jira/browse/SHINDIG-1623


Diffs (updated)
-----

  http://svn.apache.org/repos/asf/shindig/trunk/java/social-api/src/main/java/org/apache/shindig/social/core/oauth2/OAuth2AuthenticationHandler.java PRE-CREATION 
  http://svn.apache.org/repos/asf/shindig/trunk/java/social-api/src/main/java/org/apache/shindig/social/core/oauth2/OAuth2AuthorizationHandler.java PRE-CREATION 
  http://svn.apache.org/repos/asf/shindig/trunk/java/social-api/src/main/java/org/apache/shindig/social/core/oauth2/OAuth2Client.java PRE-CREATION 
  http://svn.apache.org/repos/asf/shindig/trunk/java/social-api/src/main/java/org/apache/shindig/social/core/oauth2/OAuth2Code.java PRE-CREATION 
  http://svn.apache.org/repos/asf/shindig/trunk/java/social-api/src/main/java/org/apache/shindig/social/core/oauth2/OAuth2DataService.java PRE-CREATION 
  http://svn.apache.org/repos/asf/shindig/trunk/java/social-api/src/main/java/org/apache/shindig/social/core/oauth2/OAuth2DataServiceImpl.java PRE-CREATION 
  http://svn.apache.org/repos/asf/shindig/trunk/java/social-api/src/main/java/org/apache/shindig/social/core/oauth2/OAuth2Exception.java PRE-CREATION 
  http://svn.apache.org/repos/asf/shindig/trunk/java/social-api/src/main/java/org/apache/shindig/social/core/oauth2/OAuth2Filter.java PRE-CREATION 
  http://svn.apache.org/repos/asf/shindig/trunk/java/social-api/src/main/java/org/apache/shindig/social/core/oauth2/OAuth2NormalizedRequest.java PRE-CREATION 
  http://svn.apache.org/repos/asf/shindig/trunk/java/social-api/src/main/java/org/apache/shindig/social/core/oauth2/OAuth2NormalizedResponse.java PRE-CREATION 
  http://svn.apache.org/repos/asf/shindig/trunk/java/social-api/src/main/java/org/apache/shindig/social/core/oauth2/OAuth2Service.java PRE-CREATION 
  http://svn.apache.org/repos/asf/shindig/trunk/java/social-api/src/main/java/org/apache/shindig/social/core/oauth2/OAuth2ServiceImpl.java PRE-CREATION 
  http://svn.apache.org/repos/asf/shindig/trunk/java/social-api/src/main/java/org/apache/shindig/social/core/oauth2/OAuth2Servlet.java PRE-CREATION 
  http://svn.apache.org/repos/asf/shindig/trunk/java/social-api/src/main/java/org/apache/shindig/social/core/oauth2/OAuth2TokenHandler.java PRE-CREATION 
  http://svn.apache.org/repos/asf/shindig/trunk/java/social-api/src/main/java/org/apache/shindig/social/core/oauth2/OAuth2Types.java PRE-CREATION 
  http://svn.apache.org/repos/asf/shindig/trunk/java/social-api/src/main/java/org/apache/shindig/social/core/oauth2/OAuth2Utils.java PRE-CREATION 
  http://svn.apache.org/repos/asf/shindig/trunk/java/social-api/src/main/java/org/apache/shindig/social/core/oauth2/validators/AccessTokenRequestValidator.java PRE-CREATION 
  http://svn.apache.org/repos/asf/shindig/trunk/java/social-api/src/main/java/org/apache/shindig/social/core/oauth2/validators/AuthCodeGrantValidator.java PRE-CREATION 
  http://svn.apache.org/repos/asf/shindig/trunk/java/social-api/src/main/java/org/apache/shindig/social/core/oauth2/validators/AuthorizationCodeRequestValidator.java PRE-CREATION 
  http://svn.apache.org/repos/asf/shindig/trunk/java/social-api/src/main/java/org/apache/shindig/social/core/oauth2/validators/ClientCredentialsGrantValidator.java PRE-CREATION 
  http://svn.apache.org/repos/asf/shindig/trunk/java/social-api/src/main/java/org/apache/shindig/social/core/oauth2/validators/DefaultResourceRequestValidator.java PRE-CREATION 
  http://svn.apache.org/repos/asf/shindig/trunk/java/social-api/src/main/java/org/apache/shindig/social/core/oauth2/validators/OAuth2GrantValidator.java PRE-CREATION 
  http://svn.apache.org/repos/asf/shindig/trunk/java/social-api/src/main/java/org/apache/shindig/social/core/oauth2/validators/OAuth2ProtectedResourceValidator.java PRE-CREATION 
  http://svn.apache.org/repos/asf/shindig/trunk/java/social-api/src/main/java/org/apache/shindig/social/core/oauth2/validators/OAuth2RequestValidator.java PRE-CREATION 
  http://svn.apache.org/repos/asf/shindig/trunk/java/social-api/src/main/java/org/apache/shindig/social/sample/SampleModule.java 1174269 
  http://svn.apache.org/repos/asf/shindig/trunk/java/social-api/src/test/java/org/apache/shindig/social/SocialApiTestsGuiceModule.java 1174269 
  http://svn.apache.org/repos/asf/shindig/trunk/java/social-api/src/test/java/org/apache/shindig/social/core/config/SocialApiGuiceModuleTest.java 1174269 
  http://svn.apache.org/repos/asf/shindig/trunk/java/social-api/src/test/java/org/apache/shindig/social/core/oauth/AuthenticationProviderHandlerTest.java 1174269 
  http://svn.apache.org/repos/asf/shindig/trunk/java/social-api/src/test/java/org/apache/shindig/social/core/oauth/MockServletOutputStream.java PRE-CREATION 
  http://svn.apache.org/repos/asf/shindig/trunk/java/social-api/src/test/java/org/apache/shindig/social/core/oauth/OAuth2AuthCodeFlowTest.java PRE-CREATION 
  http://svn.apache.org/repos/asf/shindig/trunk/java/social-api/src/test/java/org/apache/shindig/social/core/oauth/OAuth2AuthenticationHandlerTest.java PRE-CREATION 
  http://svn.apache.org/repos/asf/shindig/trunk/java/social-api/src/test/java/org/apache/shindig/social/core/oauth/OAuth2ClientCredentialFlowTest.java PRE-CREATION 
  http://svn.apache.org/repos/asf/shindig/trunk/java/social-api/src/test/java/org/apache/shindig/social/core/oauth/OAuth2ImplicitFlowTest.java PRE-CREATION 
  http://svn.apache.org/repos/asf/shindig/trunk/java/social-api/src/test/java/org/apache/shindig/social/core/oauth/OAuthAuthenticationHanderTest.java 1174269 
  http://svn.apache.org/repos/asf/shindig/trunk/java/social-api/src/test/java/org/apache/shindig/social/dataservice/integration/AbstractLargeRestfulTests.java 1174269 
  http://svn.apache.org/repos/asf/shindig/trunk/content/sampledata/canonicaldb.json 1174269 
  http://svn.apache.org/repos/asf/shindig/trunk/java/server/src/main/webapp/WEB-INF/web.xml 1174269 
  http://svn.apache.org/repos/asf/shindig/trunk/java/social-api/src/main/java/org/apache/shindig/social/core/oauth/AuthenticationHandlerProvider.java 1174269 

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


Testing
-------

Yes, JUnits executed with maven build.


Thanks,

Eric

Re: Review Request: OAuth 2.0 service provider implementation in Apache Shindig.

Posted by Matt Marum <mg...@us.ibm.com>.


> On 2011-09-20 09:01:22, Paul Lindner wrote:
> > wow, lots of code here.  Only skimmed.  In terms of style there's lots of tabs and trailing white space.
> > 
> > For function please consider looking at what's coming in shiro 1.2:
> > 
> > https://issues.apache.org/jira/browse/SHIRO-119
> > 
> > and check out Amber (which is incubating.. still...)
> >

I checked the Shiro patch and it looks like they are pulling in Apache Amber for their OAuth support.  We had been working under the assumption that Apache Amber was a non-starter for a number of reasons (including the fact it is still incubating).


- Matt


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


On 2011-09-16 21:05:04, Eric Woods wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/1940/
> -----------------------------------------------------------
> 
> (Updated 2011-09-16 21:05:04)
> 
> 
> Review request for shindig.
> 
> 
> Summary
> -------
> 
> OAuth 2.0 service provider implementation in Apache Shindig.
> 
> Documentation wiki: http://docs.opensocial.org/display/OSD/OAuth+2.0+Service+Provider+Implementation+in+Apache+Shindig
> 
> JIRA issue: https://issues.apache.org/jira/browse/SHINDIG-1623
> 
> 
> Diffs
> -----
> 
>   http://svn.apache.org/repos/asf/shindig/trunk/content/sampledata/canonicaldb.json 1171761 
>   http://svn.apache.org/repos/asf/shindig/trunk/java/server/src/main/webapp/WEB-INF/web.xml 1171761 
>   http://svn.apache.org/repos/asf/shindig/trunk/java/social-api/src/main/java/org/apache/shindig/social/core/oauth/AuthenticationHandlerProvider.java 1171761 
>   http://svn.apache.org/repos/asf/shindig/trunk/java/social-api/src/main/java/org/apache/shindig/social/core/oauth2/OAuth2AuthenticationHandler.java PRE-CREATION 
>   http://svn.apache.org/repos/asf/shindig/trunk/java/social-api/src/main/java/org/apache/shindig/social/core/oauth2/OAuth2AuthorizationHandler.java PRE-CREATION 
>   http://svn.apache.org/repos/asf/shindig/trunk/java/social-api/src/main/java/org/apache/shindig/social/core/oauth2/OAuth2Client.java PRE-CREATION 
>   http://svn.apache.org/repos/asf/shindig/trunk/java/social-api/src/main/java/org/apache/shindig/social/core/oauth2/OAuth2Code.java PRE-CREATION 
>   http://svn.apache.org/repos/asf/shindig/trunk/java/social-api/src/main/java/org/apache/shindig/social/core/oauth2/OAuth2DataService.java PRE-CREATION 
>   http://svn.apache.org/repos/asf/shindig/trunk/java/social-api/src/main/java/org/apache/shindig/social/core/oauth2/OAuth2DataServiceImpl.java PRE-CREATION 
>   http://svn.apache.org/repos/asf/shindig/trunk/java/social-api/src/main/java/org/apache/shindig/social/core/oauth2/OAuth2Exception.java PRE-CREATION 
>   http://svn.apache.org/repos/asf/shindig/trunk/java/social-api/src/main/java/org/apache/shindig/social/core/oauth2/OAuth2Filter.java PRE-CREATION 
>   http://svn.apache.org/repos/asf/shindig/trunk/java/social-api/src/main/java/org/apache/shindig/social/core/oauth2/OAuth2NormalizedRequest.java PRE-CREATION 
>   http://svn.apache.org/repos/asf/shindig/trunk/java/social-api/src/main/java/org/apache/shindig/social/core/oauth2/OAuth2NormalizedResponse.java PRE-CREATION 
>   http://svn.apache.org/repos/asf/shindig/trunk/java/social-api/src/main/java/org/apache/shindig/social/core/oauth2/OAuth2Service.java PRE-CREATION 
>   http://svn.apache.org/repos/asf/shindig/trunk/java/social-api/src/main/java/org/apache/shindig/social/core/oauth2/OAuth2ServiceImpl.java PRE-CREATION 
>   http://svn.apache.org/repos/asf/shindig/trunk/java/social-api/src/main/java/org/apache/shindig/social/core/oauth2/OAuth2Servlet.java PRE-CREATION 
>   http://svn.apache.org/repos/asf/shindig/trunk/java/social-api/src/main/java/org/apache/shindig/social/core/oauth2/OAuth2TokenHandler.java PRE-CREATION 
>   http://svn.apache.org/repos/asf/shindig/trunk/java/social-api/src/main/java/org/apache/shindig/social/core/oauth2/OAuth2Types.java PRE-CREATION 
>   http://svn.apache.org/repos/asf/shindig/trunk/java/social-api/src/main/java/org/apache/shindig/social/core/oauth2/OAuth2Utils.java PRE-CREATION 
>   http://svn.apache.org/repos/asf/shindig/trunk/java/social-api/src/main/java/org/apache/shindig/social/core/oauth2/validators/AccessTokenRequestValidator.java PRE-CREATION 
>   http://svn.apache.org/repos/asf/shindig/trunk/java/social-api/src/main/java/org/apache/shindig/social/core/oauth2/validators/AuthCodeGrantValidator.java PRE-CREATION 
>   http://svn.apache.org/repos/asf/shindig/trunk/java/social-api/src/main/java/org/apache/shindig/social/core/oauth2/validators/AuthorizationCodeRequestValidator.java PRE-CREATION 
>   http://svn.apache.org/repos/asf/shindig/trunk/java/social-api/src/main/java/org/apache/shindig/social/core/oauth2/validators/ClientCredentialsGrantValidator.java PRE-CREATION 
>   http://svn.apache.org/repos/asf/shindig/trunk/java/social-api/src/main/java/org/apache/shindig/social/core/oauth2/validators/DefaultResourceRequestValidator.java PRE-CREATION 
>   http://svn.apache.org/repos/asf/shindig/trunk/java/social-api/src/main/java/org/apache/shindig/social/core/oauth2/validators/OAuth2GrantValidator.java PRE-CREATION 
>   http://svn.apache.org/repos/asf/shindig/trunk/java/social-api/src/main/java/org/apache/shindig/social/core/oauth2/validators/OAuth2ProtectedResourceValidator.java PRE-CREATION 
>   http://svn.apache.org/repos/asf/shindig/trunk/java/social-api/src/main/java/org/apache/shindig/social/core/oauth2/validators/OAuth2RequestValidator.java PRE-CREATION 
>   http://svn.apache.org/repos/asf/shindig/trunk/java/social-api/src/main/java/org/apache/shindig/social/sample/SampleModule.java 1171761 
>   http://svn.apache.org/repos/asf/shindig/trunk/java/social-api/src/test/java/org/apache/shindig/social/SocialApiTestsGuiceModule.java 1171761 
>   http://svn.apache.org/repos/asf/shindig/trunk/java/social-api/src/test/java/org/apache/shindig/social/core/config/SocialApiGuiceModuleTest.java 1171761 
>   http://svn.apache.org/repos/asf/shindig/trunk/java/social-api/src/test/java/org/apache/shindig/social/core/oauth/AuthenticationProviderHandlerTest.java 1171761 
>   http://svn.apache.org/repos/asf/shindig/trunk/java/social-api/src/test/java/org/apache/shindig/social/core/oauth/MockServletOutputStream.java PRE-CREATION 
>   http://svn.apache.org/repos/asf/shindig/trunk/java/social-api/src/test/java/org/apache/shindig/social/core/oauth/OAuth2AuthCodeFlowTest.java PRE-CREATION 
>   http://svn.apache.org/repos/asf/shindig/trunk/java/social-api/src/test/java/org/apache/shindig/social/core/oauth/OAuth2AuthenticationHandlerTest.java PRE-CREATION 
>   http://svn.apache.org/repos/asf/shindig/trunk/java/social-api/src/test/java/org/apache/shindig/social/core/oauth/OAuth2ClientCredentialFlowTest.java PRE-CREATION 
>   http://svn.apache.org/repos/asf/shindig/trunk/java/social-api/src/test/java/org/apache/shindig/social/core/oauth/OAuth2ImplicitFlowTest.java PRE-CREATION 
>   http://svn.apache.org/repos/asf/shindig/trunk/java/social-api/src/test/java/org/apache/shindig/social/dataservice/integration/AbstractLargeRestfulTests.java 1171761 
> 
> Diff: https://reviews.apache.org/r/1940/diff
> 
> 
> Testing
> -------
> 
> Yes, JUnits executed with maven build.
> 
> 
> Thanks,
> 
> Eric
> 
>

Re: Review Request: OAuth 2.0 service provider implementation in Apache Shindig.

Posted by Paul Lindner <li...@inuus.com>.

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


wow, lots of code here.  Only skimmed.  In terms of style there's lots of tabs and trailing white space.

For function please consider looking at what's coming in shiro 1.2:

https://issues.apache.org/jira/browse/SHIRO-119

and check out Amber (which is incubating.. still...)



http://svn.apache.org/repos/asf/shindig/trunk/java/social-api/src/main/java/org/apache/shindig/social/core/oauth2/OAuth2AuthorizationHandler.java
<https://reviews.apache.org/r/1940/#comment4448>

    please format as TODO(name): so we know who wrote the todo.



http://svn.apache.org/repos/asf/shindig/trunk/java/social-api/src/main/java/org/apache/shindig/social/core/oauth2/OAuth2Servlet.java
<https://reviews.apache.org/r/1940/#comment4449>

    use a logger here.



http://svn.apache.org/repos/asf/shindig/trunk/java/social-api/src/main/java/org/apache/shindig/social/core/oauth2/OAuth2Servlet.java
<https://reviews.apache.org/r/1940/#comment4450>

    space after for



http://svn.apache.org/repos/asf/shindig/trunk/java/social-api/src/main/java/org/apache/shindig/social/core/oauth2/OAuth2Utils.java
<https://reviews.apache.org/r/1940/#comment4451>

    consider replacing callers to this and buildUrl with the UriBuilder class.


- Paul


On 2011-09-16 21:05:04, Eric Woods wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/1940/
> -----------------------------------------------------------
> 
> (Updated 2011-09-16 21:05:04)
> 
> 
> Review request for shindig.
> 
> 
> Summary
> -------
> 
> OAuth 2.0 service provider implementation in Apache Shindig.
> 
> Documentation wiki: http://docs.opensocial.org/display/OSD/OAuth+2.0+Service+Provider+Implementation+in+Apache+Shindig
> 
> JIRA issue: https://issues.apache.org/jira/browse/SHINDIG-1623
> 
> 
> Diffs
> -----
> 
>   http://svn.apache.org/repos/asf/shindig/trunk/content/sampledata/canonicaldb.json 1171761 
>   http://svn.apache.org/repos/asf/shindig/trunk/java/server/src/main/webapp/WEB-INF/web.xml 1171761 
>   http://svn.apache.org/repos/asf/shindig/trunk/java/social-api/src/main/java/org/apache/shindig/social/core/oauth/AuthenticationHandlerProvider.java 1171761 
>   http://svn.apache.org/repos/asf/shindig/trunk/java/social-api/src/main/java/org/apache/shindig/social/core/oauth2/OAuth2AuthenticationHandler.java PRE-CREATION 
>   http://svn.apache.org/repos/asf/shindig/trunk/java/social-api/src/main/java/org/apache/shindig/social/core/oauth2/OAuth2AuthorizationHandler.java PRE-CREATION 
>   http://svn.apache.org/repos/asf/shindig/trunk/java/social-api/src/main/java/org/apache/shindig/social/core/oauth2/OAuth2Client.java PRE-CREATION 
>   http://svn.apache.org/repos/asf/shindig/trunk/java/social-api/src/main/java/org/apache/shindig/social/core/oauth2/OAuth2Code.java PRE-CREATION 
>   http://svn.apache.org/repos/asf/shindig/trunk/java/social-api/src/main/java/org/apache/shindig/social/core/oauth2/OAuth2DataService.java PRE-CREATION 
>   http://svn.apache.org/repos/asf/shindig/trunk/java/social-api/src/main/java/org/apache/shindig/social/core/oauth2/OAuth2DataServiceImpl.java PRE-CREATION 
>   http://svn.apache.org/repos/asf/shindig/trunk/java/social-api/src/main/java/org/apache/shindig/social/core/oauth2/OAuth2Exception.java PRE-CREATION 
>   http://svn.apache.org/repos/asf/shindig/trunk/java/social-api/src/main/java/org/apache/shindig/social/core/oauth2/OAuth2Filter.java PRE-CREATION 
>   http://svn.apache.org/repos/asf/shindig/trunk/java/social-api/src/main/java/org/apache/shindig/social/core/oauth2/OAuth2NormalizedRequest.java PRE-CREATION 
>   http://svn.apache.org/repos/asf/shindig/trunk/java/social-api/src/main/java/org/apache/shindig/social/core/oauth2/OAuth2NormalizedResponse.java PRE-CREATION 
>   http://svn.apache.org/repos/asf/shindig/trunk/java/social-api/src/main/java/org/apache/shindig/social/core/oauth2/OAuth2Service.java PRE-CREATION 
>   http://svn.apache.org/repos/asf/shindig/trunk/java/social-api/src/main/java/org/apache/shindig/social/core/oauth2/OAuth2ServiceImpl.java PRE-CREATION 
>   http://svn.apache.org/repos/asf/shindig/trunk/java/social-api/src/main/java/org/apache/shindig/social/core/oauth2/OAuth2Servlet.java PRE-CREATION 
>   http://svn.apache.org/repos/asf/shindig/trunk/java/social-api/src/main/java/org/apache/shindig/social/core/oauth2/OAuth2TokenHandler.java PRE-CREATION 
>   http://svn.apache.org/repos/asf/shindig/trunk/java/social-api/src/main/java/org/apache/shindig/social/core/oauth2/OAuth2Types.java PRE-CREATION 
>   http://svn.apache.org/repos/asf/shindig/trunk/java/social-api/src/main/java/org/apache/shindig/social/core/oauth2/OAuth2Utils.java PRE-CREATION 
>   http://svn.apache.org/repos/asf/shindig/trunk/java/social-api/src/main/java/org/apache/shindig/social/core/oauth2/validators/AccessTokenRequestValidator.java PRE-CREATION 
>   http://svn.apache.org/repos/asf/shindig/trunk/java/social-api/src/main/java/org/apache/shindig/social/core/oauth2/validators/AuthCodeGrantValidator.java PRE-CREATION 
>   http://svn.apache.org/repos/asf/shindig/trunk/java/social-api/src/main/java/org/apache/shindig/social/core/oauth2/validators/AuthorizationCodeRequestValidator.java PRE-CREATION 
>   http://svn.apache.org/repos/asf/shindig/trunk/java/social-api/src/main/java/org/apache/shindig/social/core/oauth2/validators/ClientCredentialsGrantValidator.java PRE-CREATION 
>   http://svn.apache.org/repos/asf/shindig/trunk/java/social-api/src/main/java/org/apache/shindig/social/core/oauth2/validators/DefaultResourceRequestValidator.java PRE-CREATION 
>   http://svn.apache.org/repos/asf/shindig/trunk/java/social-api/src/main/java/org/apache/shindig/social/core/oauth2/validators/OAuth2GrantValidator.java PRE-CREATION 
>   http://svn.apache.org/repos/asf/shindig/trunk/java/social-api/src/main/java/org/apache/shindig/social/core/oauth2/validators/OAuth2ProtectedResourceValidator.java PRE-CREATION 
>   http://svn.apache.org/repos/asf/shindig/trunk/java/social-api/src/main/java/org/apache/shindig/social/core/oauth2/validators/OAuth2RequestValidator.java PRE-CREATION 
>   http://svn.apache.org/repos/asf/shindig/trunk/java/social-api/src/main/java/org/apache/shindig/social/sample/SampleModule.java 1171761 
>   http://svn.apache.org/repos/asf/shindig/trunk/java/social-api/src/test/java/org/apache/shindig/social/SocialApiTestsGuiceModule.java 1171761 
>   http://svn.apache.org/repos/asf/shindig/trunk/java/social-api/src/test/java/org/apache/shindig/social/core/config/SocialApiGuiceModuleTest.java 1171761 
>   http://svn.apache.org/repos/asf/shindig/trunk/java/social-api/src/test/java/org/apache/shindig/social/core/oauth/AuthenticationProviderHandlerTest.java 1171761 
>   http://svn.apache.org/repos/asf/shindig/trunk/java/social-api/src/test/java/org/apache/shindig/social/core/oauth/MockServletOutputStream.java PRE-CREATION 
>   http://svn.apache.org/repos/asf/shindig/trunk/java/social-api/src/test/java/org/apache/shindig/social/core/oauth/OAuth2AuthCodeFlowTest.java PRE-CREATION 
>   http://svn.apache.org/repos/asf/shindig/trunk/java/social-api/src/test/java/org/apache/shindig/social/core/oauth/OAuth2AuthenticationHandlerTest.java PRE-CREATION 
>   http://svn.apache.org/repos/asf/shindig/trunk/java/social-api/src/test/java/org/apache/shindig/social/core/oauth/OAuth2ClientCredentialFlowTest.java PRE-CREATION 
>   http://svn.apache.org/repos/asf/shindig/trunk/java/social-api/src/test/java/org/apache/shindig/social/core/oauth/OAuth2ImplicitFlowTest.java PRE-CREATION 
>   http://svn.apache.org/repos/asf/shindig/trunk/java/social-api/src/test/java/org/apache/shindig/social/dataservice/integration/AbstractLargeRestfulTests.java 1171761 
> 
> Diff: https://reviews.apache.org/r/1940/diff
> 
> 
> Testing
> -------
> 
> Yes, JUnits executed with maven build.
> 
> 
> Thanks,
> 
> Eric
> 
>