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

ForbiddenError does not return 403 status code #1512

Closed
curry684 opened this issue Aug 9, 2018 · 3 comments
Closed

ForbiddenError does not return 403 status code #1512

curry684 opened this issue Aug 9, 2018 · 3 comments

Comments

@curry684
Copy link

curry684 commented Aug 9, 2018

Simplified code:

const {ApolloServer, ForbiddenError} = require('apollo-server-koa');

new ApolloServer({
          typeDefs,
          resolvers,
          context: async ({ctx, connection}) => {
              const user = connection ? connection.context.user : ctx.state.user;
              if (!user.scopes || !user.scopes.includes('api:read')) {
                  throw new ForbiddenError('Your token does not have required scope "api:read"');
              }
              return {user};
          },
});

This correctly fails the request:

{
  "error": {
    "errors": [
      {
        "message": "Context creation failed: Your token does not have required scope \"api:read\"",
        "extensions": {
          "code": "FORBIDDEN",
          "exception": {
            "stacktrace": [
              "ForbiddenError: Context creation failed: Your token does not have required scope \"api:read\"",
              "    at ApolloServer.context (...)",
....

However it does so with HTTP response code 400, instead of the 403 I was expecting. Now in other threads I've seen the argument that "this is not the GraphQL way to rely on HTTP response codes". However, I'm failing at the context creation level, not the resolver level. So it is a total request failure, otherwise it would also give a 200 with an errors array.

The information is also important to me on the client side. When parsing the JWT in Passport before the request is handed to Apollo, I throw a 401 if the JWT is expired, as that is correct HTTP semantic for "your request failed now, but you may retry it with a fresh token". While a missing OAuth scope simply means "your request will never work until you change something elsewhere". So I should be able to differentiate between those response codes on the server so I can act correctly on them elsewhere.

@trevorblades trevorblades added the 🚧👷‍♀️👷‍♂️🚧 in triage Issue currently being triaged label Jul 8, 2019
@trevorblades
Copy link
Contributor

trevorblades commented Jul 8, 2019

This is by design, as noted by this comment:

// For errors that are not internal, such as authentication, we
// should provide a 400 response
if (
e.extensions &&
e.extensions.code &&
e.extensions.code !== 'INTERNAL_SERVER_ERROR'
) {
return throwHttpGraphQLError(400, [e], options);
} else {
return throwHttpGraphQLError(500, [e], options);
}

Currently we return a 500 for internal server errors, and 400 for everything else. Given the code in this function, it would be possible to account for other GraphQL error codes (FORBIDDEN for example), and return a specific status code for each.

I'm going to close this issue now, but feel free to submit a PR with this change if you still need it!

@hwillson
Copy link
Member

hwillson commented Jul 8, 2019

Just to add to the above, please also see #1709 (comment) for an example that shows how you can use didEncounterErrors to customize the response HTTP status code.

@abernix abernix removed 🚧👷‍♀️👷‍♂️🚧 in triage Issue currently being triaged labels Jul 9, 2019
@willrust
Copy link

willrust commented Feb 17, 2020

What would be the best approach on the client side for differentiating between what would have been a 403 or 401, if it's all a 400?

Edit:
Nevermind. Iterate over errors, checking the code value of each. Full Stack Error Handling with GraphQL and Apollo

@github-actions github-actions bot locked as resolved and limited conversation to collaborators Apr 20, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

No branches or pull requests

5 participants