> On 2011-10-19 21:05:16, Matt Marum wrote: > > http://svn.apache.org/repos/asf/shindig/trunk/java/common/src/main/java/org/apache/shindig/auth/BlobCrypterSecurityTokenCodec.java, > > line 22 > > <https://reviews.apache.org/r/2396/diff/6/?file=51209#file51209line22> > > > > Import flux. Eclipse likes to reorder the imports when you organize > > them. Adds unnecessary changes. > > Dan Dumont wrote: > Thanks for the review! Does eclipse randomly reorder them or are they > reordered the same way generally for everyone? > If it's somewhat standard, I may just leave it this way. I constantly > tell eclipse to organize imports as I change things, and usually ignore the > flux in reviews.
I use Eclipse and do the same thing. I've been dinged in previous reviews so I'm just spreading the love. :) Eclipse has preferences that affects the order of imports when you organize them. However, not everybody working on Shindig is using Eclipse. - Matt ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/2396/#review2676 ----------------------------------------------------------- On 2011-10-20 12:56:57, Dan Dumont wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/2396/ > ----------------------------------------------------------- > > (Updated 2011-10-20 12:56:57) > > > Review request for shindig, Matt Marum, Ryan Baxter, Eric Woods, li xu, Jesse > Ciancetta, and Stanton Sievers. > > > 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 > ----- > > > 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 > >
