Skip to content
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.

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

Improved error handling for NodeRequestor #120

Open
pixtron opened this issue Jul 31, 2019 · 5 comments
Open

Improved error handling for NodeRequestor #120

pixtron opened this issue Jul 31, 2019 · 5 comments
Assignees
Labels

Comments

@pixtron
Copy link

pixtron commented Jul 31, 2019

Expected Behavior

Describe expected behavior

Failing requests to /token endpoint (status code 400) should reject with the full error returned by the client in the body of the request. The body contains information for the reason of a failing request (eg. refresh_token expired, client authentication not successful)

Describe the problem

Currently the requestors rejects with new AppAuthError(statusMessage) (FetchRequestor rejects with new AppAuthError(statusCode, statusMessage)).
As the app does not receive the error response (see RFC 6749 section 5.2) it can't handle accordingly.

Actual Behavior

NodeRequestor rejects with Bad Request only.

Steps to reproduce the behavior

Issue a Token Request with an invalid authorization code:

const requestor = new NodeRequestor();
const tokenHandler = new BaseTokenRequestHandler(requestor);
const request = new TokenRequest({
  client_id: idpConfig.clientId,
  redirect_uri: idpConfig.redirectUri,
  grant_type: GRANT_TYPE_AUTHORIZATION_CODE,
  code: 'INVALID CODE',
  refresh_token: undefined,
  extras: extras
});

tokenHandler.performTokenRequest(serviceConfiguration, request)
  .then(response => {})
  .catch(err => {
    //err is {message:'Bad Request'},
    //err should be {message: 'Bad Request', code: 400, body: { error: 'invalid_grant', error_description: 'Malformed auth code.' }}
  });

Environment

  • AppAuth-JS version: 1.2.4
  • AppAuth-JS Environment: Node (also applicable for Browser in JQueryRequestor and FetchRequestor )
@tikurahul
Copy link
Collaborator

Yes. I should get to this soon. TypeScript 3.6 is out too. So will do this as a fast-follow.

@PeteMac88
Copy link

Whats the status here?

@pommelinho
Copy link

pommelinho commented Dec 10, 2021

For me this looks good. My project also relies on openid/AppAuth-JS. And i want to handle different token request errors separatly.

Can we merge the PR??

@tikurahul ?

@alexsanderp
Copy link

Whats the status here?

@paulinang
Copy link

@tikurahul can we merge this please?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

6 participants