[
https://issues.apache.org/jira/browse/SHINDIG-1636?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13126705#comment-13126705
]
[email protected] commented on SHINDIG-1636:
--------------------------------------------------------
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/2362/#review2552
-----------------------------------------------------------
Thinking through this a bit I'm feeling like this strategy might be a little
too specific and heavyweight IMO to achieve the higher level goal of
generically loading a bit of container configuration from an arbitrary
location. I'm wondering if you can't take a slightly higher level approach and
come out with a more generically applicable solution in the end.
For the sake of simplicity (so I don't have to continue to refer to "an
arbitrary location") let's assume the arbitrary location is a database -- so
right now you're just looking to load the security token key from a database,
but what if in the future you want to load 10 other container config properties
from the database too? One way you might accomplish that now would be to
provide a custom implementation of the ContainerConfig interface which extends
the default JsonContainerConfig implementation -- and in your custom
implementation maybe you load those 10 properties from your database at
initialization time and use those values to overwrite the values pulled from
container.js.
Of course that doesn't help you now with the token encryption key because the
config currently returns the location of the key file -- but if the config was
changed to return the actual key value instead then you could override that
value with the approach I outlined above.
You wouldn't want to break existing users who expect to be able to load from a
file or the classpath though or make them hard code their key into container.js
-- so I think to work around that we make the machinery that loads the data
from container.js smart enough to recognize the res:// and file:// prefixes,
and for those cases it could use the Shindig ResourceLoader to load the actual
value from a file.
So putting that all together I'm thinking we could change the token key
configuration in container.js from this:
"gadgets.securityTokenKeyFile" : "/path/to/key/file.txt",
to something like this:
"gadgets.securityTokenKey" : "file://path/to/key/file.txt",
make the loader smart enough to resolve file:// and res:// into actual values,
change the security token Java code to expect to pull the actual key value from
ContainerConfig instead of a key file location, and finally, you could
implement and Guice inject the custom ContainerConfig I was talking about which
would load the key value from the database to override the value read from
container.js.
Thinking through what would be required to make all this work really doesn't
seem like it would be too difficult and I think you end up with something
cleaner and simpler to deal with -- plus you're already all setup then to pull
other container config values from the database when you need to.
What do you think?
- Jesse
On 2011-10-13 14:19:32, Stanton Sievers wrote:
bq.
bq. -----------------------------------------------------------
bq. This is an automatically generated e-mail. To reply, visit:
bq. https://reviews.apache.org/r/2362/
bq. -----------------------------------------------------------
bq.
bq. (Updated 2011-10-13 14:19:32)
bq.
bq.
bq. Review request for Ryan Baxter, Dan Dumont and Jesse Ciancetta.
bq.
bq.
bq. Summary
bq. -------
bq.
bq. Currently,
org.apache.shindig.auth.BlobCrypterSecurityTokenCodec.loadContainers(ContainerConfig,
Collection<String>, Map<String, BlobCrypter>, Map<String, String>) reads an
encryption key from a keyfile to instantiate the BlobCrypter. The keyfile is
defined in the container.js. An improvement to this behavior would be to
provide an injectable KeyProvider class that can return the key. This would
allow the key to reside anywhere instead of in a static keyfile.
bq.
bq. Initial review to Dan, Ryan, and Jesse. Once we've decided that this
seems like a rational approach, I'll add the dev list.
bq.
bq.
bq. This addresses bug SHINDIG-1636.
bq. https://issues.apache.org/jira/browse/SHINDIG-1636
bq.
bq.
bq. Diffs
bq. -----
bq.
bq.
http://svn.apache.org/repos/asf/shindig/trunk/java/common/src/main/java/org/apache/shindig/auth/BlobCrypterSecurityTokenCodec.java
1182570
bq.
http://svn.apache.org/repos/asf/shindig/trunk/java/common/src/main/java/org/apache/shindig/auth/DefaultSecurityTokenCodec.java
1182570
bq.
http://svn.apache.org/repos/asf/shindig/trunk/java/common/src/main/java/org/apache/shindig/auth/KeyFileKeyProvider.java
PRE-CREATION
bq.
http://svn.apache.org/repos/asf/shindig/trunk/java/common/src/main/java/org/apache/shindig/auth/KeyProvider.java
PRE-CREATION
bq.
http://svn.apache.org/repos/asf/shindig/trunk/java/common/src/main/java/org/apache/shindig/common/crypto/BasicBlobCrypter.java
1182570
bq.
http://svn.apache.org/repos/asf/shindig/trunk/java/common/src/test/java/org/apache/shindig/auth/BlobCrypterSecurityTokenCodecTest.java
1182570
bq.
http://svn.apache.org/repos/asf/shindig/trunk/java/common/src/test/java/org/apache/shindig/auth/DefaultSecurityTokenCodecTest.java
1182570
bq.
http://svn.apache.org/repos/asf/shindig/trunk/java/common/src/test/java/org/apache/shindig/auth/KeyFileKeyProviderTest.java
PRE-CREATION
bq.
bq. Diff: https://reviews.apache.org/r/2362/diff
bq.
bq.
bq. Testing
bq. -------
bq.
bq. Updated and ran existing JUnits.
bq. Created new JUnits for the new KeyFileKeyProvider.
bq. Performed manual functional tests with encrypted security tokens in the
sample common container.
bq.
bq.
bq. Thanks,
bq.
bq. Stanton
bq.
bq.
> Create a KeyProvider to provide an encryption key to the SecurityToken
> workflow
> -------------------------------------------------------------------------------
>
> Key: SHINDIG-1636
> URL: https://issues.apache.org/jira/browse/SHINDIG-1636
> Project: Shindig
> Issue Type: Improvement
> Components: Java
> Reporter: Stanton Sievers
> Original Estimate: 48h
> Remaining Estimate: 48h
>
> Currently,
> org.apache.shindig.auth.BlobCrypterSecurityTokenCodec.loadContainers(ContainerConfig,
> Collection<String>, Map<String, BlobCrypter>, Map<String, String>) reads an
> encryption key from a keyfile to instantiate the BlobCrypter. The keyfile is
> defined in the container.js. An improvement to this behavior would be to
> provide an injectable KeyProvider class that can return the key. This would
> allow the key to reside anywhere instead of in a static keyfile.
--
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
