Author: beaton
Date: Thu Jul 10 17:24:55 2008
New Revision: 675815
URL: http://svn.apache.org/viewvc?rev=675815&view=rev
Log:
Improve OAuth error handling. We treat all 401s as "ask the user for
permission" and anything else between 400 and 499 inclusive as "go away."
Added:
incubator/shindig/trunk/java/gadgets/src/test/java/org/apache/shindig/gadgets/oauth/OAuthProtocolExceptionTest.java
(with props)
Modified:
incubator/shindig/trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/oauth/OAuthFetcher.java
incubator/shindig/trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/oauth/OAuthProtocolException.java
incubator/shindig/trunk/java/gadgets/src/test/java/org/apache/shindig/gadgets/oauth/FakeOAuthServiceProvider.java
incubator/shindig/trunk/java/gadgets/src/test/java/org/apache/shindig/gadgets/oauth/OAuthFetcherTest.java
Modified:
incubator/shindig/trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/oauth/OAuthFetcher.java
URL:
http://svn.apache.org/viewvc/incubator/shindig/trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/oauth/OAuthFetcher.java?rev=675815&r1=675814&r2=675815&view=diff
==============================================================================
---
incubator/shindig/trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/oauth/OAuthFetcher.java
(original)
+++
incubator/shindig/trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/oauth/OAuthFetcher.java
Thu Jul 10 17:24:55 2008
@@ -280,7 +280,6 @@
return buildNonDataResponse();
}
- // TODO(beaton) test case
private boolean handleProtocolException(
OAuthProtocolException pe, int attempts) throws OAuthStoreException {
if (pe.startFromScratch()) {
@@ -495,13 +494,11 @@
HttpRequest.DEFAULT_OPTIONS);
HttpResponse response = nextFetcher.fetch(oauthRequest);
+ checkForProtocolProblem(response);
OAuthMessage reply = new OAuthMessage(null, null, null);
reply.addParameters(OAuth.decodeForm(response.getResponseAsString()));
reply = parseAuthHeader(reply, response);
- if (reply.getParameter(OAuthProblemException.OAUTH_PROBLEM) != null) {
- throw new OAuthProtocolException(reply);
- }
return reply;
}
@@ -684,14 +681,7 @@
HttpResponse response = nextFetcher.fetch(oauthHttpRequest);
- // TODO is there a better way to detect an SP error?
- int statusCode = response.getHttpStatusCode();
- if (statusCode >= 400 && statusCode < 500) {
- OAuthMessage message = parseAuthHeader(null, response);
- if (message.getParameter(OAuthProblemException.OAUTH_PROBLEM) != null)
{
- throw new OAuthProtocolException(message);
- }
- }
+ checkForProtocolProblem(response);
// Track metadata on the response
addResponseMetadata(response);
@@ -706,6 +696,20 @@
throw new GadgetException(GadgetException.Code.INTERNAL_SERVER_ERROR, e);
}
}
+
+ private void checkForProtocolProblem(HttpResponse response)
+ throws OAuthProtocolException, IOException {
+ int status = response.getHttpStatusCode();
+ if (status >= 400 && status < 500) {
+ OAuthMessage message = parseAuthHeader(null, response);
+ if (message.getParameter(OAuthProblemException.OAUTH_PROBLEM) != null) {
+ // SP reported extended error information
+ throw new OAuthProtocolException(message);
+ }
+ // No extended information, guess based on HTTP response code.
+ throw new OAuthProtocolException(status);
+ }
+ }
/**
* Extracts only those parameters from an OAuthMessage that are
OAuth-related.
Modified:
incubator/shindig/trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/oauth/OAuthProtocolException.java
URL:
http://svn.apache.org/viewvc/incubator/shindig/trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/oauth/OAuthProtocolException.java?rev=675815&r1=675814&r2=675815&view=diff
==============================================================================
---
incubator/shindig/trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/oauth/OAuthProtocolException.java
(original)
+++
incubator/shindig/trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/oauth/OAuthProtocolException.java
Thu Jul 10 17:24:55 2008
@@ -44,8 +44,6 @@
*
* TODO: add a third category to cope with reauthorization per the
ScalableOAuth
* extension.
- *
- * TODO(beaton) test case
*/
class OAuthProtocolException extends Exception {
@@ -108,6 +106,23 @@
}
/**
+ * Handle OAuth protocol errors for SPs that don't support the problem
+ * reporting extension
+ * @param status HTTP status code, assumed to be between 400 and 499
inclusive
+ */
+ public OAuthProtocolException(int status) {
+ problemCode = Integer.toString(status);
+ problemText = null;
+ if (status == 401) {
+ startFromScratch = true;
+ canRetry = true;
+ } else {
+ startFromScratch = true;
+ canRetry = false;
+ }
+ }
+
+ /**
* @return true if we've gotten confused to the point where we should give
* up and ask the user for approval again.
*/
Modified:
incubator/shindig/trunk/java/gadgets/src/test/java/org/apache/shindig/gadgets/oauth/FakeOAuthServiceProvider.java
URL:
http://svn.apache.org/viewvc/incubator/shindig/trunk/java/gadgets/src/test/java/org/apache/shindig/gadgets/oauth/FakeOAuthServiceProvider.java?rev=675815&r1=675814&r2=675815&view=diff
==============================================================================
---
incubator/shindig/trunk/java/gadgets/src/test/java/org/apache/shindig/gadgets/oauth/FakeOAuthServiceProvider.java
(original)
+++
incubator/shindig/trunk/java/gadgets/src/test/java/org/apache/shindig/gadgets/oauth/FakeOAuthServiceProvider.java
Thu Jul 10 17:24:55 2008
@@ -35,7 +35,6 @@
import java.util.ArrayList;
import java.util.Arrays;
import java.util.HashMap;
-import java.util.Iterator;
import java.util.List;
import java.util.Map;
@@ -110,6 +109,7 @@
private final OAuthConsumer consumer;
private boolean throttled;
+ private boolean vagueErrors;
private int requestTokenCount = 0;
@@ -124,6 +124,11 @@
null, CONSUMER_KEY, CONSUMER_SECRET, provider);
tokenState = new HashMap<String, TokenState>();
validator = new SimpleOAuthValidator();
+ vagueErrors = false;
+ }
+
+ public void setVagueErrors(boolean vagueErrors) {
+ this.vagueErrors = vagueErrors;
}
@SuppressWarnings("unused")
@@ -177,6 +182,13 @@
private HttpResponse makeOAuthProblemReport(String code, String text)
throws IOException {
+ if (vagueErrors) {
+ int rc = 401;
+ if ("consumer_key_unknown".equals(code)) {
+ rc = 403;
+ }
+ return new HttpResponse(rc, null, null);
+ }
OAuthMessage msg = new OAuthMessage(null, null, null);
msg.addParameter("oauth_problem", code);
msg.addParameter("oauth_problem_advice", text);
Modified:
incubator/shindig/trunk/java/gadgets/src/test/java/org/apache/shindig/gadgets/oauth/OAuthFetcherTest.java
URL:
http://svn.apache.org/viewvc/incubator/shindig/trunk/java/gadgets/src/test/java/org/apache/shindig/gadgets/oauth/OAuthFetcherTest.java?rev=675815&r1=675814&r2=675815&view=diff
==============================================================================
---
incubator/shindig/trunk/java/gadgets/src/test/java/org/apache/shindig/gadgets/oauth/OAuthFetcherTest.java
(original)
+++
incubator/shindig/trunk/java/gadgets/src/test/java/org/apache/shindig/gadgets/oauth/OAuthFetcherTest.java
Thu Jul 10 17:24:55 2008
@@ -242,6 +242,60 @@
response = fetcher.fetch(request);
assertEquals("User data is reapproved", response.getResponseAsString());
}
+
+
+ @Test
+ public void testError401() throws Exception {
+ HttpFetcher fetcher;
+ HttpRequest request;
+ HttpResponse response;
+
+ serviceProvider.setVagueErrors(true);
+
+ fetcher = getFetcher(
+ getSecurityToken("owner", "owner"),
+ new OAuthRequestParams(SERVICE_NAME, null, null, false));
+ request = new HttpRequest(
+ new URI(FakeOAuthServiceProvider.RESOURCE_URL));
+ response = fetcher.fetch(request);
+ String clientState = response.getMetadata().get("oauthState");
+ assertNotNull(clientState);
+ String approvalUrl = response.getMetadata().get("oauthApprovalUrl");
+ assertNotNull(approvalUrl);
+
+ serviceProvider.browserVisit(approvalUrl + "&user_data=hello-oauth");
+
+ fetcher = getFetcher(
+ getSecurityToken("owner", "owner"),
+ new OAuthRequestParams(SERVICE_NAME, null, clientState, false));
+ request = new HttpRequest(
+ new URI(FakeOAuthServiceProvider.RESOURCE_URL));
+ response = fetcher.fetch(request);
+ assertEquals("User data is hello-oauth", response.getResponseAsString());
+
+ serviceProvider.revokeAllAccessTokens();
+
+ fetcher = getFetcher(
+ getSecurityToken("owner", "owner"),
+ new OAuthRequestParams(SERVICE_NAME, null, clientState, false));
+ request = new HttpRequest(
+ new URI(FakeOAuthServiceProvider.RESOURCE_URL));
+ response = fetcher.fetch(request);
+
+ clientState = response.getMetadata().get("oauthState");
+ assertNotNull(clientState);
+ approvalUrl = response.getMetadata().get("oauthApprovalUrl");
+ assertNotNull(approvalUrl);
+
+ serviceProvider.browserVisit(approvalUrl + "&user_data=reapproved");
+ fetcher = getFetcher(
+ getSecurityToken("owner", "owner"),
+ new OAuthRequestParams(SERVICE_NAME, null, clientState, false));
+ request = new HttpRequest(
+ new URI(FakeOAuthServiceProvider.RESOURCE_URL));
+ response = fetcher.fetch(request);
+ assertEquals("User data is reapproved", response.getResponseAsString());
+ }
@Test
public void testUnknownConsumerKey() throws Exception {
@@ -262,6 +316,26 @@
"invalid consumer: garbage_key",
metadata.get("oauthErrorText"));
}
+
+ @Test
+ public void testError403() throws Exception {
+ HttpFetcher fetcher;
+ HttpRequest request;
+ HttpResponse response;
+
+ serviceProvider.setVagueErrors(true);
+
+ fetcher = getFetcher(
+ getNokeySecurityToken("owner", "owner"),
+ new OAuthRequestParams(SERVICE_NAME_NO_KEY, null, null, false));
+ request = new HttpRequest(
+ new URI(FakeOAuthServiceProvider.RESOURCE_URL));
+ response = fetcher.fetch(request);
+ Map<String, String> metadata = response.getMetadata();
+ assertNotNull(metadata);
+ assertEquals("403", metadata.get("oauthError"));
+ assertNull(metadata.get("oauthErrorText"));
+ }
@Test
public void testConsumerThrottled() throws Exception {
Added:
incubator/shindig/trunk/java/gadgets/src/test/java/org/apache/shindig/gadgets/oauth/OAuthProtocolExceptionTest.java
URL:
http://svn.apache.org/viewvc/incubator/shindig/trunk/java/gadgets/src/test/java/org/apache/shindig/gadgets/oauth/OAuthProtocolExceptionTest.java?rev=675815&view=auto
==============================================================================
---
incubator/shindig/trunk/java/gadgets/src/test/java/org/apache/shindig/gadgets/oauth/OAuthProtocolExceptionTest.java
(added)
+++
incubator/shindig/trunk/java/gadgets/src/test/java/org/apache/shindig/gadgets/oauth/OAuthProtocolExceptionTest.java
Thu Jul 10 17:24:55 2008
@@ -0,0 +1,48 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations under the License.
+ */
+package org.apache.shindig.gadgets.oauth;
+
+import static org.junit.Assert.assertEquals;
+import static org.junit.Assert.assertFalse;
+import static org.junit.Assert.assertTrue;
+
+import java.util.Map;
+
+import net.oauth.OAuthMessage;
+
+import org.apache.shindig.gadgets.http.HttpResponse;
+import org.junit.Test;
+
+/**
+ * Test cases for OAuthProtocolException
+ */
+public class OAuthProtocolExceptionTest {
+
+ @Test
+ public void testProblemReportingExtension() throws Exception {
+ OAuthMessage m = new OAuthMessage(null, null, null);
+ m.addParameter("oauth_problem", "consumer_key_refused");
+ m.addParameter("oauth_problem_advice", "stuff it");
+ OAuthProtocolException e = new OAuthProtocolException(m);
+ assertFalse(e.canRetry());
+ HttpResponse r = e.getResponseForGadget();
+ Map<String, String> metadata = r.getMetadata();
+ assertEquals("stuff it", metadata.get("oauthErrorText"));
+ assertEquals("consumer_key_refused", metadata.get("oauthError"));
+ }
+}
Propchange:
incubator/shindig/trunk/java/gadgets/src/test/java/org/apache/shindig/gadgets/oauth/OAuthProtocolExceptionTest.java
------------------------------------------------------------------------------
svn:eol-style = native
