shindig-issues mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "jiraposter@reviews.apache.org (Commented) (JIRA)" <j...@apache.org>
Subject [jira] [Commented] (SHINDIG-1645) Refactor security token implementation to utilize existing APIs for expires and clean up code.
Date Wed, 19 Oct 2011 21:05:11 GMT

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

jiraposter@reviews.apache.org commented on SHINDIG-1645:
--------------------------------------------------------


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


Just some nitpicks.  I'm good with the changes.  

I wish I knew if there was some special motivation in timestamping the blobs as the means
of enforcing expiration.  If anybody has a problem with changing this, now would be a good
time to speak up. :)

Matt


http://svn.apache.org/repos/asf/shindig/trunk/java/common/src/main/java/org/apache/shindig/auth/BlobCrypterSecurityTokenCodec.java
<https://reviews.apache.org/r/2396/#comment6035>

    Import flux.  Eclipse likes to reorder the imports when you organize them.  Adds unnecessary
changes.



http://svn.apache.org/repos/asf/shindig/trunk/java/common/src/test/java/org/apache/shindig/auth/BlobCrypterSecurityTokenTest.java
<https://reviews.apache.org/r/2396/#comment6036>

    Import flux



http://svn.apache.org/repos/asf/shindig/trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/servlet/GadgetsHandlerService.java
<https://reviews.apache.org/r/2396/#comment6033>

    Unused import



http://svn.apache.org/repos/asf/shindig/trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/servlet/GadgetsHandlerService.java
<https://reviews.apache.org/r/2396/#comment6034>

    Unused import


- Matt


On 2011-10-19 19:46:39, Dan Dumont wrote:
bq.  
bq.  -----------------------------------------------------------
bq.  This is an automatically generated e-mail. To reply, visit:
bq.  https://reviews.apache.org/r/2396/
bq.  -----------------------------------------------------------
bq.  
bq.  (Updated 2011-10-19 19:46:39)
bq.  
bq.  
bq.  Review request for shindig, Matt Marum, Ryan Baxter, Eric Woods, li xu, Jesse Ciancetta,
and Stanton Sievers.
bq.  
bq.  
bq.  Summary
bq.  -------
bq.  
bq.  Long diffs... but before I make any more progress, I want to make sure that everyone
agrees that this cleanup is sound.
bq.  
bq.  Major things of note:
bq.  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.
bq.  
bq.  Expiration setting and checking has been moved into the token.  
bq.  Codecs are now responsible for enforcing token expirations, not crypters.
bq.  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.
bq.  
bq.  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...
bq.  
bq.  I'll make a jira before this gets submitted, as I'm not sure how close to the final design
this will be.
bq.  
bq.  
bq.  This addresses bug SHINDIG-1645.
bq.      https://issues.apache.org/jira/browse/SHINDIG-1645
bq.  
bq.  
bq.  Diffs
bq.  -----
bq.  
bq.    http://svn.apache.org/repos/asf/shindig/trunk/java/common/src/main/java/org/apache/shindig/auth/AbstractSecurityToken.java
1184753 
bq.    http://svn.apache.org/repos/asf/shindig/trunk/java/common/src/main/java/org/apache/shindig/auth/AnonymousSecurityToken.java
1184753 
bq.    http://svn.apache.org/repos/asf/shindig/trunk/java/common/src/main/java/org/apache/shindig/auth/BasicSecurityToken.java
1184753 
bq.    http://svn.apache.org/repos/asf/shindig/trunk/java/common/src/main/java/org/apache/shindig/auth/BasicSecurityTokenCodec.java
1184753 
bq.    http://svn.apache.org/repos/asf/shindig/trunk/java/common/src/main/java/org/apache/shindig/auth/BlobCrypterSecurityToken.java
1184753 
bq.    http://svn.apache.org/repos/asf/shindig/trunk/java/common/src/main/java/org/apache/shindig/auth/BlobCrypterSecurityTokenCodec.java
1184753 
bq.    http://svn.apache.org/repos/asf/shindig/trunk/java/common/src/main/java/org/apache/shindig/auth/DefaultSecurityTokenCodec.java
1184753 
bq.    http://svn.apache.org/repos/asf/shindig/trunk/java/common/src/main/java/org/apache/shindig/auth/SecurityToken.java
1184753 
bq.    http://svn.apache.org/repos/asf/shindig/trunk/java/common/src/main/java/org/apache/shindig/auth/SecurityTokenCodec.java
1184753 
bq.    http://svn.apache.org/repos/asf/shindig/trunk/java/common/src/main/java/org/apache/shindig/common/crypto/BasicBlobCrypter.java
1184753 
bq.    http://svn.apache.org/repos/asf/shindig/trunk/java/common/src/main/java/org/apache/shindig/common/crypto/BlobCrypter.java
1184753 
bq.    http://svn.apache.org/repos/asf/shindig/trunk/java/common/src/main/java/org/apache/shindig/common/crypto/BlobExpiredException.java
1184753 
bq.    http://svn.apache.org/repos/asf/shindig/trunk/java/common/src/test/java/org/apache/shindig/auth/BasicSecurityTokenCodecTest.java
1184753 
bq.    http://svn.apache.org/repos/asf/shindig/trunk/java/common/src/test/java/org/apache/shindig/auth/BlobCrypterSecurityTokenCodecTest.java
1184753 
bq.    http://svn.apache.org/repos/asf/shindig/trunk/java/common/src/test/java/org/apache/shindig/auth/BlobCrypterSecurityTokenTest.java
1184753 
bq.    http://svn.apache.org/repos/asf/shindig/trunk/java/common/src/test/java/org/apache/shindig/auth/DefaultSecurityTokenCodecTest.java
1184753 
bq.    http://svn.apache.org/repos/asf/shindig/trunk/java/common/src/test/java/org/apache/shindig/auth/UrlParameterAuthenticationHandlerTest.java
1184753 
bq.    http://svn.apache.org/repos/asf/shindig/trunk/java/common/src/test/java/org/apache/shindig/common/crypto/BlobCrypterTest.java
1184753 
bq.    http://svn.apache.org/repos/asf/shindig/trunk/java/common/src/test/java/org/apache/shindig/common/testing/FakeGadgetToken.java
1184753 
bq.    http://svn.apache.org/repos/asf/shindig/trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/config/ShindigAuthConfigContributor.java
1184753 
bq.    http://svn.apache.org/repos/asf/shindig/trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/oauth/OAuthCallbackState.java
1184753 
bq.    http://svn.apache.org/repos/asf/shindig/trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/oauth/OAuthCallbackStateToken.java
PRE-CREATION 
bq.    http://svn.apache.org/repos/asf/shindig/trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/oauth/OAuthClientState.java
1184753 
bq.    http://svn.apache.org/repos/asf/shindig/trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/servlet/GadgetsHandlerService.java
1184753 
bq.    http://svn.apache.org/repos/asf/shindig/trunk/java/gadgets/src/test/java/org/apache/shindig/gadgets/oauth/OAuthClientStateTest.java
1184753 
bq.    http://svn.apache.org/repos/asf/shindig/trunk/java/gadgets/src/test/java/org/apache/shindig/gadgets/oauth2/MockUtils.java
1184753 
bq.    http://svn.apache.org/repos/asf/shindig/trunk/java/gadgets/src/test/java/org/apache/shindig/gadgets/servlet/GadgetsHandlerServiceTest.java
1184753 
bq.    http://svn.apache.org/repos/asf/shindig/trunk/java/gadgets/src/test/java/org/apache/shindig/gadgets/servlet/GadgetsHandlerTest.java
1184753 
bq.    http://svn.apache.org/repos/asf/shindig/trunk/java/social-api/src/main/java/org/apache/shindig/social/core/oauth/OAuthSecurityToken.java
1184753 
bq.  
bq.  Diff: https://reviews.apache.org/r/2396/diff
bq.  
bq.  
bq.  Testing
bq.  -------
bq.  
bq.  Tests have been updated with the interface changes and all currently pass.
bq.  
bq.  Tests that tested crypter expiration/timestamp enforcement have been removed.
bq.  
bq.  
bq.  Thanks,
bq.  
bq.  Dan
bq.  
bq.


                
> Refactor security token implementation to utilize existing APIs for expires and clean
up code.
> ----------------------------------------------------------------------------------------------
>
>                 Key: SHINDIG-1645
>                 URL: https://issues.apache.org/jira/browse/SHINDIG-1645
>             Project: Shindig
>          Issue Type: Improvement
>            Reporter: Dan Dumont
>


--
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

        

Mime
View raw message