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