You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@shindig.apache.org by Stanton Sievers <si...@gmail.com> on 2011/11/01 18:16:16 UTC
Re: Review Request: Allow the blob crypter encryption key to be provided
explicitly in the config
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/2648/
-----------------------------------------------------------
(Updated 2011-11-01 17:16:16.418233)
Review request for shindig, Ryan Baxter and Dan Dumont.
Changes
-------
Adding the dev list.
Summary (updated)
-------
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
-------
None right now. Updated JUnits so they'd pass
Thanks,
Stanton
Re: Review Request: Allow the blob crypter encryption key to be provided
explicitly in the config
Posted by Jesse Ciancetta <jc...@mitre.org>.
> On 2011-11-01 19:00:41, Jesse Ciancetta wrote:
> > 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.
>
> Stanton Sievers wrote:
> That was something I was thinking we could tackle later. I'm just trying to identify the minimal amount of changes that will unblock my use case and then we can focus on making it better.
Sounds good -- I'll try to post a follow on patch to implement the enhancements to ContainerConfig shortly after this one goes in.
- Jesse
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/2648/#review2993
-----------------------------------------------------------
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
>
>
Re: Review Request: Allow the blob crypter encryption key to be provided
explicitly in the config
Posted by Stanton Sievers <si...@gmail.com>.
> On 2011-11-01 19:00:41, Jesse Ciancetta wrote:
> > 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.
That was something I was thinking we could tackle later. I'm just trying to identify the minimal amount of changes that will unblock my use case and then we can focus on making it better.
- Stanton
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/2648/#review2993
-----------------------------------------------------------
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
>
>
Re: Review Request: Allow the blob crypter encryption key to be provided
explicitly in the config
Posted by Jesse Ciancetta <jc...@mitre.org>.
-----------------------------------------------------------
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
>
>
Re: Review Request: Allow the blob crypter encryption key to be provided
explicitly in the config
Posted by Dan Dumont <dd...@us.ibm.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/2648/#review3012
-----------------------------------------------------------
Ship it!
LGTM!
- Dan
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
>
>
Re: Review Request: Allow the blob crypter encryption key to be provided
explicitly in the config
Posted by Ryan Baxter <rb...@gmail.com>.
> On 2011-11-02 00:28:45, Ryan Baxter wrote:
> > One small nit, otherwise LGTM
Committed revision 1196683.
- Ryan
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/2648/#review3003
-----------------------------------------------------------
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
>
>
Re: Review Request: Allow the blob crypter encryption key to be provided
explicitly in the config
Posted by Ryan Baxter <rb...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/2648/#review3003
-----------------------------------------------------------
Ship it!
One small nit, otherwise LGTM
http://svn.apache.org/repos/asf/shindig/trunk/java/common/src/main/java/org/apache/shindig/auth/BlobCrypterSecurityTokenCodec.java
<https://reviews.apache.org/r/2648/#comment6724>
small nit, your could put the cypters.put call outside of the if else block
- Ryan
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
>
>
Re: Review Request: Allow the blob crypter encryption key to be provided
explicitly in the config
Posted by Stanton Sievers <si...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/2648/
-----------------------------------------------------------
(Updated 2011-11-01 18:12:28.326279)
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)
-------
Updated existing JUnits and added one more.
Thanks,
Stanton