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

Allow HTTP401 from "context creation failed" (schema authorization) #2960

Closed
elnygren opened this issue Jul 1, 2019 · 7 comments
Closed

Comments

@elnygren
Copy link

elnygren commented Jul 1, 2019

Problem

Apollo docs suggest doing "schema authorization" in the context function. However, when you throw an AuthenticationError in the context function, the HTTP status is set to 400 and haven't found a way to customise this behavior. I'd like a 401 in this situation.

The relevant code can be found from:

if (typeof options.context === 'function') {
try {
(options.context as () => never)();
} catch (e) {
e.message = `Context creation failed: ${e.message}`;
// 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);
}
}
}

Solution?

Plugins?

Allow a similar solution as described in #1709 (comment) (requestDidStart + didEncounterErrors)

Customise apollo-server-express behavior?

It seems I should be able to customise behavior at the express<->Apollo level, here:

(error: HttpQueryError) => {
if ('HttpQueryError' !== error.name) {
return next(error);
}
if (error.headers) {
for (const [name, value] of Object.entries(error.headers)) {
res.setHeader(name, value);
}
}
res.statusCode = error.statusCode;
res.write(error.message);
res.end();
},
);

Should full schema auth happen elsewhere?

Different web servers can probably check for auth token / cookie before routing to POST /graphql

Hacky solution for Express

/*
Not included:
- jwt middleware adds req.jwtToken and req.jwtPayload
- apolloServer = new ApolloServer(...)

Note: the order of the three calls here matters.
*/

// before any apollo code, check JWT 
app.post('/graphql', (req, res, next) => {
  if (!req.jwtToken || !req.jwtPayload || !req.jwtPayload.roles.includes('user')) {
    throw new AuthenticationError('unauthorized') // Apollo's error but we could also use our own
  }
  next()
})

apolloServer.applyMiddleware({ app, path: '/graphql' })

// error middleware that handles AuthenticationError by manually constructing a graphql error
app.use((err, req, res, next) => {
  if (err instanceof AuthenticationError) {
    res.status(401).send({
      data: null,
      errors: [
        {
          message: err.message,
          locations: err.locations || [],
          extensions: err.extensions || [],
          path: err.path || [],
        },
      ],
    })
  } else {
    next(err)
  }
})
@abernix
Copy link
Member

abernix commented Aug 26, 2019

Please see my suggestion in #1709 (comment), which should now be possible thanks to #2719.

@abernix abernix closed this as completed Aug 26, 2019
@Karnich
Copy link

Karnich commented Oct 25, 2019

@abernix as fare as i can tell, if you throw the error during context creation you don't enter the apollo server pipeline and there fore you can't catch the error in didEncounterErrors

@loganbentley
Copy link

@abernix yeah, I'm running into this as well. It appears that if you throw the error during context creation it does not enter the apollo server pipeline

@Yunoo
Copy link

Yunoo commented Dec 5, 2019

@abernix Same problem here. Errors thrown during the context creation do not hit didEncounterErrors at all.

@DavidFlorin
Copy link

Same problem here. If we do the authorization during the context creation, didEncounterErrors is not called

@lewispham
Copy link

I think the best solution is to make all errors thrown in context creation fall through formatError. The current behavior is a bug imho.

@abernix
Copy link
Member

abernix commented Dec 31, 2020

Linking this to: #3223

@apollographql apollographql locked and limited conversation to collaborators Dec 31, 2020
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

7 participants