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

Automatically adding user to graphql resolver context #421

Closed
elie222 opened this issue Jul 30, 2018 · 9 comments
Closed

Automatically adding user to graphql resolver context #421

elie222 opened this issue Jul 30, 2018 · 9 comments

Comments

@elie222
Copy link
Contributor

elie222 commented Jul 30, 2018

I see the code has authenticated as shown here:

changePassword: authenticated(accountsServer, changePassword(accountsServer)),

Would it make sense to just add authenticated to the context and have the user object available for every request made to the server?

Right now I have this in my code:

const server = new ApolloServer({
  typeDefs: [typeDefs as any, accountsGraphQL.typeDefs],
  resolvers: merge(accountsGraphQL.resolvers, resolvers),
  context: async ({ req }) => {
    const ctx = accountsContext(req)
    const { authToken } = ctx

    let user

    try {
      user = await accountsServer.resumeSession(authToken as string)
    } catch (error) {}

    return {
      ...ctx,
      user,
      userId: user && user._id,
    }
  },
})

Seems something like this would be useful for all users of the graphql-api package. Would it make sense to have accountsContext also return the user object if it exists.

@TimMikeladze
Copy link
Member

This sounds like a good improvement to me.

@elie222
Copy link
Contributor Author

elie222 commented Aug 1, 2018

Is resume session as in the code above the best way to do this?

@TimMikeladze
Copy link
Member

I think so. After this change do you think the getUser query is still needed? Or perhaps it could be modified to return context.user if it exists?

@elie222
Copy link
Contributor Author

elie222 commented Aug 1, 2018 via email

@TimMikeladze
Copy link
Member

Yeah sorry I worded that incorrectly. I meant to say that it should be modified to use context.user if it exists instead of doing Accounts.resumeSession.

@pradel
Copy link
Member

pradel commented Aug 2, 2018

@elie222 was wondering does throwing inside the context function create a formatted graphql error?

@lorensr
Copy link
Contributor

lorensr commented Aug 4, 2018

AFAICT, the current behavior is that whenever there's a valid access token stored on the client, it's included in a header, and accountsContext() provides a context.user object to all of my resolvers.

@elie222
Copy link
Contributor Author

elie222 commented Aug 4, 2018 via email

@pradel
Copy link
Member

pradel commented Aug 8, 2018

I just submitted a pull request for that #437

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

No branches or pull requests

4 participants