-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/2396/
-----------------------------------------------------------
(Updated 2011-10-20 12:56:57.693241)
Review request for shindig, Matt Marum, Ryan Baxter, Eric Woods, li xu, Jesse
Ciancetta, and Stanton Sievers.
Changes
-------
Addressing Matt's review
Summary
-------
Long diffs... but before I make any more progress, I want to make sure that
everyone agrees that this cleanup is sound.
Major things of note:
SecurityToken interface has token expiration built into it, yet crypters were
timestamping maps before encoding them and throwing exceptions during decrypt
if the timestamp was too old. This bypassed the meaning of the expiration
values in the tokens. It also seemed like the crypter was a terrible place to
be doing this. Now, all tokens that claim support for the expire key will now
have actual expire times instead of timestamps tacked on to the tokens.
Expiration setting and checking has been moved into the token.
Codecs are now responsible for enforcing token expirations, not crypters.
AbstractSecurityToken has gotten a major face-lift. It now includes an Enum
for various popular token keys and many utility methods for handling token to
Map and Map to token conversions.
Some OAuth code changes were necessary as they depended on former crypter token
timestamping. For the most part, I tried to leave the code alone because I'm
not familiar with it. This has meant that an awkward, but functional
implementation of an OAuthClientState is actually an extention of
AbstractSecurityToken... It seemed to almost make sense for all the work it
was doing...
I'll make a jira before this gets submitted, as I'm not sure how close to the
final design this will be.
This addresses bug SHINDIG-1645.
https://issues.apache.org/jira/browse/SHINDIG-1645
Diffs (updated)
-----
http://svn.apache.org/repos/asf/shindig/trunk/java/common/src/main/java/org/apache/shindig/auth/AbstractSecurityToken.java
1184753
http://svn.apache.org/repos/asf/shindig/trunk/java/common/src/main/java/org/apache/shindig/auth/AnonymousSecurityToken.java
1184753
http://svn.apache.org/repos/asf/shindig/trunk/java/common/src/main/java/org/apache/shindig/auth/BasicSecurityToken.java
1184753
http://svn.apache.org/repos/asf/shindig/trunk/java/common/src/main/java/org/apache/shindig/auth/BasicSecurityTokenCodec.java
1184753
http://svn.apache.org/repos/asf/shindig/trunk/java/common/src/main/java/org/apache/shindig/auth/BlobCrypterSecurityToken.java
1184753
http://svn.apache.org/repos/asf/shindig/trunk/java/common/src/main/java/org/apache/shindig/auth/BlobCrypterSecurityTokenCodec.java
1184753
http://svn.apache.org/repos/asf/shindig/trunk/java/common/src/main/java/org/apache/shindig/auth/DefaultSecurityTokenCodec.java
1184753
http://svn.apache.org/repos/asf/shindig/trunk/java/common/src/main/java/org/apache/shindig/auth/SecurityToken.java
1184753
http://svn.apache.org/repos/asf/shindig/trunk/java/common/src/main/java/org/apache/shindig/auth/SecurityTokenCodec.java
1184753
http://svn.apache.org/repos/asf/shindig/trunk/java/common/src/main/java/org/apache/shindig/common/crypto/BasicBlobCrypter.java
1184753
http://svn.apache.org/repos/asf/shindig/trunk/java/common/src/main/java/org/apache/shindig/common/crypto/BlobCrypter.java
1184753
http://svn.apache.org/repos/asf/shindig/trunk/java/common/src/main/java/org/apache/shindig/common/crypto/BlobExpiredException.java
1184753
http://svn.apache.org/repos/asf/shindig/trunk/java/common/src/test/java/org/apache/shindig/auth/BasicSecurityTokenCodecTest.java
1184753
http://svn.apache.org/repos/asf/shindig/trunk/java/common/src/test/java/org/apache/shindig/auth/BlobCrypterSecurityTokenCodecTest.java
1184753
http://svn.apache.org/repos/asf/shindig/trunk/java/common/src/test/java/org/apache/shindig/auth/BlobCrypterSecurityTokenTest.java
1184753
http://svn.apache.org/repos/asf/shindig/trunk/java/common/src/test/java/org/apache/shindig/auth/DefaultSecurityTokenCodecTest.java
1184753
http://svn.apache.org/repos/asf/shindig/trunk/java/common/src/test/java/org/apache/shindig/auth/UrlParameterAuthenticationHandlerTest.java
1184753
http://svn.apache.org/repos/asf/shindig/trunk/java/common/src/test/java/org/apache/shindig/common/crypto/BlobCrypterTest.java
1184753
http://svn.apache.org/repos/asf/shindig/trunk/java/common/src/test/java/org/apache/shindig/common/testing/FakeGadgetToken.java
1184753
http://svn.apache.org/repos/asf/shindig/trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/config/ShindigAuthConfigContributor.java
1184753
http://svn.apache.org/repos/asf/shindig/trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/oauth/OAuthCallbackState.java
1184753
http://svn.apache.org/repos/asf/shindig/trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/oauth/OAuthCallbackStateToken.java
PRE-CREATION
http://svn.apache.org/repos/asf/shindig/trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/oauth/OAuthClientState.java
1184753
http://svn.apache.org/repos/asf/shindig/trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/servlet/GadgetsHandlerService.java
1184753
http://svn.apache.org/repos/asf/shindig/trunk/java/gadgets/src/test/java/org/apache/shindig/gadgets/oauth/OAuthClientStateTest.java
1184753
http://svn.apache.org/repos/asf/shindig/trunk/java/gadgets/src/test/java/org/apache/shindig/gadgets/oauth2/MockUtils.java
1184753
http://svn.apache.org/repos/asf/shindig/trunk/java/gadgets/src/test/java/org/apache/shindig/gadgets/servlet/GadgetsHandlerServiceTest.java
1184753
http://svn.apache.org/repos/asf/shindig/trunk/java/gadgets/src/test/java/org/apache/shindig/gadgets/servlet/GadgetsHandlerTest.java
1184753
http://svn.apache.org/repos/asf/shindig/trunk/java/social-api/src/main/java/org/apache/shindig/social/core/oauth/OAuthSecurityToken.java
1184753
Diff: https://reviews.apache.org/r/2396/diff
Testing
-------
Tests have been updated with the interface changes and all currently pass.
Tests that tested crypter expiration/timestamp enforcement have been removed.
Thanks,
Dan
