http://codereview.appspot.com/149041/diff/3003/3004 File java/common/conf/shindig.properties (right):
http://codereview.appspot.com/149041/diff/3003/3004#newcode45 java/common/conf/shindig.properties:45: shindig.signing.ownerPageSecure=false naming convention in this file suggests "shindig.signing.owner-page-secure". But I think a better name might be "shindig.signing.enable-viewer-access-tokens" http://codereview.appspot.com/149041/diff/3003/3005 File java/gadgets/src/main/java/org/apache/shindig/gadgets/oauth/GadgetOAuthTokenStore.java (right): http://codereview.appspot.com/149041/diff/3003/3005#newcode62 java/gadgets/src/main/java/org/apache/shindig/gadgets/oauth/GadgetOAuthTokenStore.java:62: public GadgetOAuthTokenStore(OAuthStore store, GadgetSpecFactory specFactory, OAuthFetcherConfig fetcherConfig) { hmm. my initial instinct was to say fetcherConfig should be a parameter on getOAuthAccessToken, so that fetcher configs can vary per request... Does that seem reasonable? http://codereview.appspot.com/149041/diff/3003/3005#newcode116 java/gadgets/src/main/java/org/apache/shindig/gadgets/oauth/GadgetOAuthTokenStore.java:116: securityToken.getOwnerId().equals(securityToken.getViewerId())) { my goodness. maybe time to create a method for this instead? That way we can collapse these lines to "if (shouldUseToken(...)) {" http://codereview.appspot.com/149041/diff/3003/3007 File java/gadgets/src/main/java/org/apache/shindig/gadgets/oauth/OAuthRequest.java (right): http://codereview.appspot.com/149041/diff/3003/3007#newcode330 java/gadgets/src/main/java/org/apache/shindig/gadgets/oauth/OAuthRequest.java:330: "(state owner=" + stateOwner + ", pageOwner=" + pageViewer + ')'); text should be pageViewer here http://codereview.appspot.com/149041/diff/3003/3011 File java/gadgets/src/test/java/org/apache/shindig/gadgets/oauth/OAuthRequestTest.java (right): http://codereview.appspot.com/149041/diff/3003/3011#newcode422 java/gadgets/src/test/java/org/apache/shindig/gadgets/oauth/OAuthRequestTest.java:422: assertEquals(OAuthError.UNAUTHENTICATED.toString(), response.getMetadata().get("oauthError")); hrm. I hadn't expected this change. It seems like the right thing to do, but it is a change to the external makeRequest interface. I think it's OK. I can't imagine a real application depending on the old behavior. http://codereview.appspot.com/149041
