[ 
https://issues.apache.org/jira/browse/SHINDIG-1732?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13271991#comment-13271991
 ] 

[email protected] commented on SHINDIG-1732:
--------------------------------------------------------


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

Ship it!


Committed revision 1336461

Please close the review.

- Stanton


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


                
> Restrictive OAuth2Client (endpoint whitelisting)
> ------------------------------------------------
>
>                 Key: SHINDIG-1732
>                 URL: https://issues.apache.org/jira/browse/SHINDIG-1732
>             Project: Shindig
>          Issue Type: Improvement
>          Components: Java
>    Affects Versions: 2.5.0-beta1
>            Reporter: Adam Clarke
>             Fix For: 2.5.0
>
>         Attachments: 1732_20120504.patch
>
>
> I have received comments asking for improvements in the OAuth2 Consumer to 
> help administrators with token security.
> One of these comments I plan on submitting a patch for is to allow 
> OAuth2Clients to be defined in a way that restricts where it will send OAuth2 
> tokens.
> One area of concern is already addressed with the "allowModuleOverride" 
> setting.
> 1) Adminstrator creates an OAuth2Client and binds Gadget X to it.
> 2) Gadget X changes it's <ModulePrefs> to a new authorization and/or token 
> url endpoint
> 3) If allowModuleOverride==true the OAuth2 Consumer will honor the 
> ModulePrefs and initiate the dance wherever Gadget X indicates.
> 4) Especially when using "client_credentials" this may be dangerous because 
> the admin's client_id/client_secret have been sent to a potentially untrusted 
> endpoint.
> 5) Today adminstrator can set allowModuleOverride=false on the OAuth2Client 
> and be assured the dance will only be initiated on the URLs they've setup, 
> ignoring the gadgets preferences.
> Setting allowModuleOverride is sufficient to protect the client_id and 
> client_secret.  However, Gadget X can still issue a makeRequest to any URL it 
> wants and the OAuth2 Consumer will send the access_token.
> This change will allow the administrator to define the valid servers/domains 
> for an OAuth2Client and prevent the Consumer from sending tokens anywhere 
> outside of the specified locations.  This was particularly important for 
> clients using "client_credentials" or other flows where compromised 
> access_tokens can open up large amounts of data on a server.
> Any suggestions on how to effectively implement the token whitelist are 
> appreciated...

--
This message is automatically generated by JIRA.
If you think it was sent incorrectly, please contact your JIRA administrators: 
https://issues.apache.org/jira/secure/ContactAdministrators!default.jspa
For more information on JIRA, see: http://www.atlassian.com/software/jira

        

Reply via email to