http://codereview.appspot.com/28075/diff/23/1015 File ../trunk/java/common/src/main/java/org/apache/shindig/auth/AuthenticationServletFilter.java (right):
http://codereview.appspot.com/28075/diff/23/1015#newcode124 Line 124: final HttpServletRequest wrapped; don't need this variable http://codereview.appspot.com/28075/diff/23/1010 File ../trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/oauth/OAuthCommandLine.java (right): http://codereview.appspot.com/28075/diff/23/1010#newcode58 Line 58: * --bodySigning hash|legacy add |none http://codereview.appspot.com/28075/diff/23/1010#newcode79 Line 79: Boolean.valueOf(params.get("--legacyBodySigning")); Dead code. http://codereview.appspot.com/28075/diff/23/1010#newcode107 Line 107: bodySigningEnum = BodySigning.valueOf(paramLocation); paramLocation? should be bodySigning. http://codereview.appspot.com/28075/diff/23/1014 File ../trunk/java/social-api/src/main/java/org/apache/shindig/social/core/oauth/OAuthAuthenticationHandler.java (right): http://codereview.appspot.com/28075/diff/23/1014#newcode57 Line 57: private OAuthDataStore store; these can be final http://codereview.appspot.com/28075/diff/23/1014#newcode88 Line 88: if (allowLegacyBodySigning && !request.getContentType().contains(OAuth.FORM_ENCODED)) { Comment about meaning of allowLegacyBodySigning would be good here, or somewhere. http://codereview.appspot.com/28075/diff/23/1014#newcode101 Line 101: OAuthEntry entry = null; This function is kind of long, could it be broken up a bit? http://codereview.appspot.com/28075/diff/23/1014#newcode106 Line 106: throw new InvalidAuthenticationException("No oauth entry for token", null); It's weird this isn't a checked exception. http://codereview.appspot.com/28075/diff/23/1014#newcode115 Line 115: OAuthConsumer authConsumer = store.getConsumer(consumerKey); Should die a horrible and verbose death if authConsumer is null. http://codereview.appspot.com/28075/diff/23/1014#newcode134 Line 134: I suspect people are going to need to customize the latter parts of this function, might be good to split it out into a protected method SecurityToken getTokenFromVerifiedRequest(OAuthMessage msg, OAuthEntry entry); http://codereview.appspot.com/28075/diff/23/1018 File ../trunk/java/social-api/src/test/java/org/apache/shindig/social/core/oauth/FakeOAuthRequest.java (right): http://codereview.appspot.com/28075/diff/23/1018#newcode61 Line 61: String url; private static final http://codereview.appspot.com/28075/diff/23/1018#newcode154 Line 154: static String getAuthorizationHeader(List<Map.Entry<String, String>> oauthParams) { these methods can be private. http://codereview.appspot.com/28075/diff/23/1011 File ../trunk/java/social-api/src/test/java/org/apache/shindig/social/core/oauth/OAuthAuthenticationHanderTest.java (right): http://codereview.appspot.com/28075/diff/23/1011#newcode43 Line 43: * Verify behavior of 3-legged OAuth handler These tests look great. Should change comment to mention that it handles both 2-legged and 3-legged. http://codereview.appspot.com/28075
