Skip to content

Fix error handling for http errors everywhere #496

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open

Fix error handling for http errors everywhere #496

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
23 changes: 23 additions & 0 deletions library/java/net/openid/appauth/AuthorizationException.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -125,6 +125,11 @@ public final class AuthorizationException extends Exception {
*/
public static final int TYPE_OAUTH_REGISTRATION_ERROR = 4;

/**
* Error type when returning http error code >= 400
*/
public static final int TYPE_HTTP_ERROR = 5;

@VisibleForTesting
static final String KEY_TYPE = "type";

Expand Down Expand Up @@ -499,6 +504,24 @@ public static AuthorizationException fromTemplate(
rootCause);
}

/**
* Creates an exception based on HTTP error codes returned from HttpUrlConnection
* Exception includes actual error code from HttpURLConnection.getResponseStream()
* Errors are considered anything >= 400
*/
public static AuthorizationException fromHttpError(
int responseCode,
@NonNull String responseMessage,
@NonNull String errorString) {
return new AuthorizationException(
TYPE_HTTP_ERROR,
responseCode,
responseMessage,
errorString,
null,
null);
}

/**
* Creates an exception based on one of the existing values defined in
* {@link AuthorizationRequestErrors} or {@link TokenRequestErrors}, adding information
Expand Down
21 changes: 18 additions & 3 deletions library/java/net/openid/appauth/AuthorizationService.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -450,11 +450,16 @@ protected JSONObject doInBackground(Void... voids) {
wr.write(queryData);
wr.flush();

if (conn.getResponseCode() >= HttpURLConnection.HTTP_OK
&& conn.getResponseCode() < HttpURLConnection.HTTP_MULT_CHOICE) {
if (conn.getResponseCode() < HttpURLConnection.HTTP_BAD_REQUEST) {
is = conn.getInputStream();
} else {
is = conn.getErrorStream();
String errorString = Utils.readInputStream(is);
mException = AuthorizationException.fromHttpError(
conn.getResponseCode(),
conn.getResponseMessage(),
errorString);
return null;
}
String response = Utils.readInputStream(is);
return new JSONObject(response);
Expand Down Expand Up @@ -599,7 +604,17 @@ protected JSONObject doInBackground(Void... voids) {
wr.write(postData);
wr.flush();

is = conn.getInputStream();
if (conn.getResponseCode() < HttpURLConnection.HTTP_BAD_REQUEST) {
is = conn.getInputStream();
} else {
is = conn.getErrorStream();
String errorString = Utils.readInputStream(is);
mException = AuthorizationException.fromHttpError(
conn.getResponseCode(),
conn.getResponseMessage(),
errorString);
return null;
}
String response = Utils.readInputStream(is);
return new JSONObject(response);
} catch (IOException ex) {
Expand Down
13 changes: 12 additions & 1 deletion library/java/net/openid/appauth/AuthorizationServiceConfiguration.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -329,7 +329,18 @@ protected AuthorizationServiceConfiguration doInBackground(Void... voids) {
conn.setDoInput(true);
conn.connect();

is = conn.getInputStream();
if (conn.getResponseCode() < HttpURLConnection.HTTP_BAD_REQUEST) {
is = conn.getInputStream();
} else {
is = conn.getErrorStream();
String errorString = Utils.readInputStream(is);
mException = AuthorizationException.fromHttpError(
conn.getResponseCode(),
conn.getResponseMessage(),
errorString);
return null;
}

JSONObject json = new JSONObject(Utils.readInputStream(is));

AuthorizationServiceDiscovery discovery =
Expand Down
19 changes: 19 additions & 0 deletions library/javatests/net/openid/appauth/AuthorizationServiceConfigurationTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -217,6 +217,25 @@ public void testFetchFromUrlWithoutName() throws Exception {
verify(mHttpConnection).connect();
}

@Test
public void testServiceConfigurationRequest_withBadRequest() throws Exception {
InputStream is = new ByteArrayInputStream(AuthorizationServiceTest.BAD_REQUEST_RESPONSE.getBytes());
when(mHttpConnection.getErrorStream()).thenReturn(is);
when(mHttpConnection.getResponseCode()).thenReturn(HttpURLConnection.HTTP_BAD_REQUEST);
when(mHttpConnection.getResponseMessage()).thenReturn(AuthorizationServiceTest.BAD_REQUEST_ERROR_MESSAGE);
doFetch();
mCallback.waitForCallback();
assertBadRequest(mCallback.error);
}

private void assertBadRequest(AuthorizationException error) {
assertNotNull(error);
assertEquals(AuthorizationException.TYPE_HTTP_ERROR, error.type);
assertEquals(HttpURLConnection.HTTP_BAD_REQUEST, error.code);
assertEquals(AuthorizationServiceTest.BAD_REQUEST_ERROR_MESSAGE, error.error);
assertEquals(AuthorizationServiceTest.BAD_REQUEST_RESPONSE, error.errorDescription);
}

@Test
public void testFetchFromUrl_missingArgument() throws Exception {
InputStream is = new ByteArrayInputStream(TEST_JSON_MISSING_ARGUMENT.getBytes());
Expand Down
99 changes: 51 additions & 48 deletions library/javatests/net/openid/appauth/AuthorizationServiceTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -100,14 +100,36 @@ public class AuthorizationServiceTest {
+ " \"application_type\": " + RegistrationRequest.APPLICATION_TYPE_NATIVE + "\n"
+ "}";

private static final String INVALID_GRANT_RESPONSE_JSON = "{\n"
+ " \"error\": \"invalid_grant\",\n"
+ " \"error_description\": \"invalid_grant description\"\n"
+ "}";

private static final String INVALID_GRANT_NO_DESC_RESPONSE_JSON = "{\n"
+ " \"error\": \"invalid_grant\"\n"
+ "}";
public static final String BAD_REQUEST_RESPONSE = "<!DOCTYPE html>\n" +
"<html lang=\"en\">\n" +
"<head>\n" +
" <meta charset=\"utf-8\" />\n" +
" <title>Exception caught</title>\n" +
" <style>\n" +
" body {\n" +
" background-color: #FAFAFA;\n" +
" color: #333;\n" +
" margin: 0px;\n" +
" }\n" +
"\n" +
" </style>\n" +
"\n" +
"\n" +
"</head>\n" +
"<body>\n" +
"\n" +
"<header>\n" +
" <h1>Network Error</h1>\n" +
"</header>\n" +
"<div id=\"container\">\n" +
" <h2>Bad Request</h2>\n" +
"\n" +
"</div>\n" +
"\n" +
"</body>\n" +
"</html>";

public static final String BAD_REQUEST_ERROR_MESSAGE = "Bad Request";

private static final int TEST_INVALID_GRANT_CODE = 2002;

Expand Down Expand Up @@ -342,39 +364,16 @@ public void testTokenRequest_withPostAuth() throws Exception {
}

@Test
public void testTokenRequest_withInvalidGrant() throws Exception {
public void testTokenRequest_withBadRequest() throws Exception {
ClientSecretPost csp = new ClientSecretPost(TEST_CLIENT_SECRET);
InputStream is = new ByteArrayInputStream(INVALID_GRANT_RESPONSE_JSON.getBytes());
InputStream is = new ByteArrayInputStream(BAD_REQUEST_RESPONSE.getBytes());
when(mHttpConnection.getErrorStream()).thenReturn(is);
when(mHttpConnection.getResponseCode()).thenReturn(HttpURLConnection.HTTP_BAD_REQUEST);
when(mHttpConnection.getResponseMessage()).thenReturn(BAD_REQUEST_ERROR_MESSAGE);
TokenRequest request = getTestAuthCodeExchangeRequest();
mService.performTokenRequest(request, csp, mAuthCallback);
mAuthCallback.waitForCallback();
assertInvalidGrant(mAuthCallback.error);
}

@Test
public void testTokenRequest_withInvalidGrant2() throws Exception {
ClientSecretPost csp = new ClientSecretPost(TEST_CLIENT_SECRET);
InputStream is = new ByteArrayInputStream(INVALID_GRANT_RESPONSE_JSON.getBytes());
when(mHttpConnection.getErrorStream()).thenReturn(is);
when(mHttpConnection.getResponseCode()).thenReturn(199);
TokenRequest request = getTestAuthCodeExchangeRequest();
mService.performTokenRequest(request, csp, mAuthCallback);
mAuthCallback.waitForCallback();
assertInvalidGrant(mAuthCallback.error);
}

@Test
public void testTokenRequest_withInvalidGrantWithNoDesc() throws Exception {
ClientSecretPost csp = new ClientSecretPost(TEST_CLIENT_SECRET);
InputStream is = new ByteArrayInputStream(INVALID_GRANT_NO_DESC_RESPONSE_JSON.getBytes());
when(mHttpConnection.getErrorStream()).thenReturn(is);
when(mHttpConnection.getResponseCode()).thenReturn(HttpURLConnection.HTTP_BAD_REQUEST);
TokenRequest request = getTestAuthCodeExchangeRequest();
mService.performTokenRequest(request, csp, mAuthCallback);
mAuthCallback.waitForCallback();
assertInvalidGrantWithNoDescription(mAuthCallback.error);
assertBadRequest(mAuthCallback.error);
}

@Test
Expand All @@ -400,6 +399,18 @@ public void testRegistrationRequest() throws Exception {
assertThat(postBody).isEqualTo(request.toJsonString());
}

@Test
public void testRegistrationRequest_withBadRequest() throws Exception {
InputStream is = new ByteArrayInputStream(BAD_REQUEST_RESPONSE.getBytes());
when(mHttpConnection.getErrorStream()).thenReturn(is);
when(mHttpConnection.getResponseCode()).thenReturn(HttpURLConnection.HTTP_BAD_REQUEST);
when(mHttpConnection.getResponseMessage()).thenReturn(AuthorizationServiceTest.BAD_REQUEST_ERROR_MESSAGE);
RegistrationRequest request = getTestRegistrationRequest();
mService.performRegistrationRequest(request, mRegistrationCallback);
mRegistrationCallback.waitForCallback();
assertBadRequest(mRegistrationCallback.error);
}

@Test
public void testRegistrationRequest_IoException() throws Exception {
Exception ex = new IOException();
Expand Down Expand Up @@ -453,20 +464,12 @@ private void assertTokenResponse(
assertEquals(idToken, response.idToken);
}

private void assertInvalidGrant(AuthorizationException error) {
assertNotNull(error);
assertEquals(AuthorizationException.TYPE_OAUTH_TOKEN_ERROR, error.type);
assertEquals(TEST_INVALID_GRANT_CODE, error.code);
assertEquals("invalid_grant", error.error);
assertEquals("invalid_grant description", error.errorDescription);
}

private void assertInvalidGrantWithNoDescription(AuthorizationException error) {
private void assertBadRequest(AuthorizationException error) {
assertNotNull(error);
assertEquals(AuthorizationException.TYPE_OAUTH_TOKEN_ERROR, error.type);
assertEquals(TEST_INVALID_GRANT_CODE, error.code);
assertEquals("invalid_grant", error.error);
assertNull(error.errorDescription);
assertEquals(AuthorizationException.TYPE_HTTP_ERROR, error.type);
assertEquals(HttpURLConnection.HTTP_BAD_REQUEST, error.code);
assertEquals(BAD_REQUEST_ERROR_MESSAGE, error.error);
assertEquals(BAD_REQUEST_RESPONSE, error.errorDescription);
}

private void assertRegistrationResponse(RegistrationResponse response,
Expand Down