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

Ship it!


This looks good to me, however if we're going to go this route I wouldn't mind 
seeing the BlobCrypterSecurityTokenCodec changed to always expect to get the 
actual token key from ContainerConfig -- and then making the code that parses 
the container.js file smart enough to be able to resolve res://key-file or 
file:///path/to/key-file automatically (or if not prefixed to take the value 
from container.js as is).  I think this would work well for this particular 
case and would be a nice addition to the ContainerConfig code.

It's pretty straight forward to add that support to ContainerConfig and I've 
got a patch that does that which I could post if people are interested -- I'd 
need to add some additional unit tests around the enhancements to the 
ContainerConfig loader but other than that it should be good to go.

Or you could commit this as is and I could post that as a follow on patch at a 
later date and if people like it we could move to it.  Either way is fine with 
me.

- Jesse


On 2011-11-01 18:12:28, Stanton Sievers wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/2648/
> -----------------------------------------------------------
> 
> (Updated 2011-11-01 18:12:28)
> 
> 
> Review request for shindig, Ryan Baxter and Dan Dumont.
> 
> 
> Summary
> -------
> 
> This patch is to illustrate the minimal set of changes needed to allow the 
> BlobCrypterSecurityTokenCodec to have a key provided by the config instead of 
> needing a key file to be provided.
> 
> 
> This addresses bug SHINDIG-1636.
>     https://issues.apache.org/jira/browse/SHINDIG-1636
> 
> 
> Diffs
> -----
> 
>   
> http://svn.apache.org/repos/asf/shindig/trunk/java/common/src/test/java/org/apache/shindig/auth/BlobCrypterSecurityTokenCodecTest.java
>  1195457 
>   http://svn.apache.org/repos/asf/shindig/trunk/config/container.js 1195457 
>   
> http://svn.apache.org/repos/asf/shindig/trunk/java/common/src/main/java/org/apache/shindig/auth/BlobCrypterSecurityTokenCodec.java
>  1195457 
> 
> Diff: https://reviews.apache.org/r/2648/diff
> 
> 
> Testing
> -------
> 
> Updated existing JUnits and added one more.
> 
> 
> Thanks,
> 
> Stanton
> 
>

Reply via email to