Needs a test case or two in OAuthRequestTest.java. I'd suggest two test cases:
1) test approval process, then send data request with oauth state 2) test approval process, then send data request without oauth state. (This will hit the token store) I think GadgetOAuthTokenStore.java needs a change similar to what you've made in OAuthRequest.java. http://codereview.appspot.com/149041/diff/1/2 File java/common/conf/shindig.properties (right): http://codereview.appspot.com/149041/diff/1/2#newcode42 java/common/conf/shindig.properties:42: shindig.signing.ownerPageSecure=false naming convention in this file suggest "shindig.signing.owner-page-secure". I think the documentation should be "Set to true if you want to allow the use of 3-legged OAuth tokens when viewer != owner. This setting is not recommeneded for pages that allow user-controlled javascript, since that javascript could be used to make unauthorized requests on behalf of the viewer of the page." So maybe the parameter should be: shindig.signing.enable-viewer-access-tokens=false http://codereview.appspot.com/149041/diff/1/3 File java/gadgets/src/main/java/org/apache/shindig/gadgets/oauth/OAuthFetcherConfig.java (right): http://codereview.appspot.com/149041/diff/1/3#newcode39 java/gadgets/src/main/java/org/apache/shindig/gadgets/oauth/OAuthFetcherConfig.java:39: private static boolean ownerPageSecure = false; I think you should go ahead and change the constructor interface rather than using member injection. It's OK to change this interface. If people are using Guice injection, they'll get the right behavior without changing code. If people aren't using Guice injection, they'll need to make a minor change, but I think that's OK. Changing an interface to keep the code clean seems like a good trade-off to me. (Especially since this is not a wire-format change.) http://codereview.appspot.com/149041/diff/1/4 File java/gadgets/src/main/java/org/apache/shindig/gadgets/oauth/OAuthRequest.java (right): http://codereview.appspot.com/149041/diff/1/4#newcode323 java/gadgets/src/main/java/org/apache/shindig/gadgets/oauth/OAuthRequest.java:323: if (!fetcherConfig.isOwnerPageSecure() && !pageOwner.equals(pageViewer)) { I love that this config is per fetcherConfig; that means we can make it a per-container option really easily! http://codereview.appspot.com/149041
