[
https://issues.apache.org/jira/browse/SHINDIG-1732?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13266800#comment-13266800
]
[email protected] commented on SHINDIG-1732:
--------------------------------------------------------
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/4947/#review7480
-----------------------------------------------------------
http://svn.apache.org/repos/asf/shindig/trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/oauth2/BasicOAuth2Store.java
<https://reviews.apache.org/r/4947/#comment16551>
The call sequence here appears to be BasicOAuth2Store.removeToken(
OAuth2Token ) that disaggregates into the removeToken( token parts ), where we
do a cache lookup by parts to find a token (that we already had) so that we can
do a remove by OAuth2Token. Seems like this could be streamlined with a remove
method on the cache by a token's component parts (since the MapCache
disassembles the token into its component parts to build the key)
http://svn.apache.org/repos/asf/shindig/trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/oauth2/OAuth2CallbackState.java
<https://reviews.apache.org/r/4947/#comment16553>
If there is no crypter, then the stateBlob won't be parsed. If there is no
intent to handle a null crypter in the class, then it seems like that should be
an exception condition. Seems like no crypter might be useful in some debug
conditions
http://svn.apache.org/repos/asf/shindig/trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/oauth2/OAuth2CallbackState.java
<https://reviews.apache.org/r/4947/#comment16552>
There is no toString in the superclasses, so if there is no crypter, the
string will be unusable
http://svn.apache.org/repos/asf/shindig/trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/oauth2/persistence/OAuth2Client.java
<https://reviews.apache.org/r/4947/#comment16550>
This changes the toString method result. I didn't see a corresponding
change in this patch to the
org.apache.shindig.gadgets.oauth2.persistence.OAuth2ClientTest class for test
method testOAuth2Client_1
- BrianLillie
On 2012-05-01 15:12:43, Adam Clarke wrote:
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-01 15:12:43)
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
1332636
bq.
http://svn.apache.org/repos/asf/shindig/trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/oauth2/BasicOAuth2Accessor.java
1332636
bq.
http://svn.apache.org/repos/asf/shindig/trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/oauth2/BasicOAuth2Request.java
1332636
bq.
http://svn.apache.org/repos/asf/shindig/trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/oauth2/BasicOAuth2Store.java
1332636
bq.
http://svn.apache.org/repos/asf/shindig/trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/oauth2/OAuth2Accessor.java
1332636
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
1332636
bq.
http://svn.apache.org/repos/asf/shindig/trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/oauth2/OAuth2FetcherConfig.java
1332636
bq.
http://svn.apache.org/repos/asf/shindig/trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/oauth2/OAuth2Message.java
1332636
bq.
http://svn.apache.org/repos/asf/shindig/trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/oauth2/OAuth2Module.java
1332636
bq.
http://svn.apache.org/repos/asf/shindig/trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/oauth2/OAuth2Store.java
1332636
bq.
http://svn.apache.org/repos/asf/shindig/trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/oauth2/OAuth2Token.java
1332636
bq.
http://svn.apache.org/repos/asf/shindig/trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/oauth2/handler/CodeAuthorizationResponseHandler.java
1332636
bq.
http://svn.apache.org/repos/asf/shindig/trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/oauth2/handler/CodeGrantTypeHandler.java
1332636
bq.
http://svn.apache.org/repos/asf/shindig/trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/oauth2/handler/OAuth2HandlerModule.java
1332636
bq.
http://svn.apache.org/repos/asf/shindig/trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/oauth2/handler/TokenAuthorizationResponseHandler.java
1332636
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
1332636
bq.
http://svn.apache.org/repos/asf/shindig/trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/oauth2/persistence/OAuth2Client.java
1332636
bq.
http://svn.apache.org/repos/asf/shindig/trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/oauth2/persistence/OAuth2Persister.java
1332636
bq.
http://svn.apache.org/repos/asf/shindig/trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/oauth2/persistence/OAuth2TokenPersistence.java
1332636
bq.
http://svn.apache.org/repos/asf/shindig/trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/oauth2/persistence/sample/InMemoryCache.java
1332636
bq.
http://svn.apache.org/repos/asf/shindig/trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/oauth2/persistence/sample/JSONOAuth2Persister.java
1332636
bq.
http://svn.apache.org/repos/asf/shindig/trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/servlet/MakeRequestHandler.java
1332636
bq.
http://svn.apache.org/repos/asf/shindig/trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/servlet/OAuth2CallbackServlet.java
1332636
bq.
http://svn.apache.org/repos/asf/shindig/trunk/java/gadgets/src/test/java/org/apache/shindig/gadgets/oauth2/MockUtils.java
1332636
bq.
http://svn.apache.org/repos/asf/shindig/trunk/java/gadgets/src/test/java/org/apache/shindig/gadgets/oauth2/handler/CodeGrantTypeHandlerTest.java
1332636
bq.
http://svn.apache.org/repos/asf/shindig/trunk/java/gadgets/src/test/java/org/apache/shindig/gadgets/oauth2/handler/TokenAuthorizationResponseHandlerTest.java
1332636
bq.
http://svn.apache.org/repos/asf/shindig/trunk/java/gadgets/src/test/java/org/apache/shindig/gadgets/oauth2/persistence/sample/InMemoryCacheTest.java
1332636
bq.
http://svn.apache.org/repos/asf/shindig/trunk/java/gadgets/src/test/java/org/apache/shindig/gadgets/oauth2/persistence/sample/JSONOAuth2PersisterTest.java
1332636
bq.
http://svn.apache.org/repos/asf/shindig/trunk/java/gadgets/src/test/java/org/apache/shindig/gadgets/servlet/MakeRequestHandlerTest.java
1332636
bq.
http://svn.apache.org/repos/asf/shindig/trunk/java/gadgets/src/test/resources/org/apache/shindig/gadgets/oauth2/oauth2_test.json
1332636
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-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
