Auth0 renewAuth fails to propagate http status code to AuthenticationException in onFailure

[dcostinett]

Description

I’m working on an Android app and getting an unexpected response from the renewAuth API call when I try to refresh a token. The process works fine up until the token gets invalidated on the back-end. When I get a response with the http response code 403 Forbidden with body:

{“error”:“invalid_grant”,“error_description”:“Unknown or invalid refresh token.”}

what I want to do is check the AuthenticationException object in the callback and I expect that that error object will have the statusCode set to 403 to match the http response status code, but the value it holds is 0:

Prerequisites

[x] I have checked the documentation for this library [Add a link].

[x] I have checked the Auth0 Community for related posts.
I posted this issue there too.

[x] I have checked for related or duplicate Issues and PRs.

[x] I have read the Auth0 general contribution guidelines.

[x] I have read the Auth0 Code of Conduct.

Environment

Please provide the following:

• Version of Auth0.Android used: 1.15.1
• Version of Android being used: API 21 to API 28 (Android 5.0 Lollipop to Q). App built targetting API 28, using AndroidX.
• Additional packages that might be affecting your instance: Retrofit 2.4.0, OkHttp 3.14.1

Reproduction

With a valid Auth0 refresh token we call the renewAuth SDK method and get a 403 http status code

response from the service which fires the onFailure method of the callback. Examine the AuthenticationException object's statusCode property and I expect to see the status code there match the http status code. But it appears the AuthenticationException(String, String) constructor is called rather than the AuthenticationException(

Please include:

• Specific devices affected
  Galaxy S9, Pixel 2 XL

• Log files or stacktraces (redact/remove sensitive information)

• Snippet of the code you're running (redact/remove sensitive information)
  override fun refreshToken(): Single {
  return Single.create {
  authClient.renewAuth(profileService.getRefreshToken())
  .start(object : BaseCallback<Credentials, AuthenticationException> {
  override fun onSuccess(payload: Credentials?) {
  payload?.accessToken?.let { token ->
  profileService.updateAuthToken(token)
  }
  payload?.refreshToken?.let { token ->
  profileService.updateRefreshToken(token)
  }
  it.onSuccess(true)
  }

                    override fun onFailure(error: AuthenticationException?) {
                        // return false if we get a 403 or a 401...
                        if (error?.statusCode == CODE_ERROR_FORBIDDEN || error?.statusCode == CODE_ERROR_UNAUTHORIZED) {
                            if (!BuildConfig.DEBUG) {
                                NewRelic.recordHandledException(error)
                            }
                            Log.e(this::class.tag, "Error code = ${error.code}, description = ${error.description}")
                            it.onSuccess(false)
                        } else {
                            // ... or accessDenied, otherwise, we don't want to clear the token
                            it.onSuccess(error?.isAccessDenied != true)
                        }
                    }
                })
    }.retryWhen {
        retryHandler(it, 3, 2) { ex, i ->
            if (!BuildConfig.DEBUG) {
                NewRelic.recordCustomEvent("Authorization", "RefreshTokenFailure:", mapOf(Pair(i.toString(), ex.message)))
            }
        }
    }
  

  }

• Screenshots, if helpful

[lbalmaceda]

Thanks for filling the details. Yes.. in fact is using the Map constructor as it's reading the response body as JSON. In this case, a request to renew a token that fails means either that the request failed due to networks reasons (e.g. cause of the exception is Auth0Exception) or the refresh token was revoked. For the last case, other than re-logging the users, there's no alternative path..
Can I ask why you need to check the status code? It's not clear to me after reading the snippet you shared.

[dcostinett]

Thanks for the quick response!

I specifically want to distinguish between network issues and an explicit revocation of the token -- 401/403. When I get the 401/403 I want to log out the user and ask them to log back in, but I want to avoid doing that for a simple network blip with the mobile device. At the point where I'm looking at the AuthenticationException I don't have easy access to the http status code other than from the AuthenticationException itself.

[lbalmaceda]

After re-checking the classes, any network error should trigger a call to onFailure (or throw the exception in case of a sync call). The parseUnsuccessfulResponse method will only be called on those requests that succeed but have a status code other than 2xx.

I think the solution here is to create a different error type in that method so that you can easily check that scenario by calling something similar to exception.isNetworkError(). There's no need to dig deeper in the status codes or manually parsing the error description. I'll add a task to review this in the next 2 weeks.

[lbalmaceda]

@dcostinett I've proposed a solution. Feel free to try it and leave feedback 🌮 🎉

[dcostinett]

Hi Ibalmaceda, sorry about the delay here... I am getting a call to the onFailure method with the AuthorizationException getting passed in with the statusCode value set to 0. Are you suggesting that I should get a call to the parseUnsuccessfulResponse method as well? The method here is the implementation of Auth0's callback to the start method on the renewAuth call. I'm not sure how I would implement a new error type. Can you please elaborate?
From what I see in the BaseRequest, parseUnsuccessfulResponse gets passed the Response object from OkHttp and creates the AuthenticationException from the response body. I think that the code() from the OkHttp Response should be added to the AuthenticationException's stringPaylod before it gets created. That way the statusCode could get pulled out in the map version of the AuthenticationException's constructor.

BaseResquest's parseUnsuccessfulResponse() gets this small change:
protected U parseUnsuccessfulResponse(Response response) {
String stringPayload = null;
ResponseBody body = response.body();
try {
stringPayload = body.string();
Type mapType = new TypeToken<Map<String, Object>>() {
}.getType();
Map<String, Object> mapPayload = gson.fromJson(stringPayload, mapType);
// add following line
mapPayload.put("statusCode", response.code())
return errorBuilder.from(mapPayload);
} catch (JsonSyntaxException e) {
return errorBuilder.from(stringPayload, response.code());
} catch (IOException e) {
final Auth0Exception auth0Exception = new Auth0Exception("Error parsing the server response", e);
return errorBuilder.from("Request to " + url.toString() + " failed", auth0Exception);
} finally {
closeStream(body);
}
}

And the AuthenticationException constructor that takes a map gets this:
this.statusCode = Integer.parseInt(values.get("statusCode").toString());
(But use a constant for STATUS_CODE_KEY, etc. like you do for ERROR_KEY, etc.)

[lbalmaceda]

Please checkout the branch from the PR linked to this issue. Once you do, if you use that code in your app you'll now be able to distinguish a network error from an API/server error.

Please use isNetworkError() on the AuthenticationException that you receive in order to tell this.

if (error.isNetworkError()){
  //handle network error differently 
}

As mentioned before, there's no need to manually check the HTTP status code.

[dcostinett]

That seems like it will be better than parsing the string in the error “code”. Thanks, Dan

…

On May 30, 2019, at 1:53 PM, Luciano Balmaceda ***@***.***> wrote: Please checkout the branch from the PR linked to this issue. Once you do, if you use that code in your app you'll now be able to distinguish a network error from an API/server error. Please use isNetworkError() on the AuthenticationException that you receive in order to tell this. if (error.isNetworkError()){ //handle network error differently } As mentioned before, there's no need to manually check the HTTP status code. — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#234?email_source=notifications&email_token=AASXCDQKIMA2QVPAIYNN5SLPYA5FNA5CNFSM4HPRDNCKYY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGODWTPUWA#issuecomment-497482328>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AASXCDQSZ4IO6TNMPI3KZJTPYA5FNANCNFSM4HPRDNCA>.