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

Auth directive called before building the context when calling custom mutations #632

Closed
lihaibh opened this issue Apr 1, 2019 · 5 comments · Fixed by #1258
Closed

Auth directive called before building the context when calling custom mutations #632

lihaibh opened this issue Apr 1, 2019 · 5 comments · Fixed by #1258

Comments

@lihaibh
Copy link

lihaibh commented Apr 1, 2019

Building the context of the module is essential for authentication because this is where we set up the authentication key, the current logged in user etc and without that, userId, user variables will remain undefined.

When i run accountsjs mutations like 'authenticate' i see that this building context function of AccountsModule and AccountsPasswordModule is called.
However it looks like that when i run my own mutations the function that builds the context never invoked... even though my module imports AccountsModule...

for example i have in my module this mutation:

type Mutation {
  addItems(items: [InputItems]): Boolean! @auth
}

I run 'authenticate' mutation to receive an access token and set the header like so:

"Authorization" "Bearer ...token..."

when I run my mutation, i saw that it didnt work and when i debugged the program to understand whats going on, i saw that the auth directive is called before the context function is called so token is always "undefined" and the auth-directive fails.

BTW it would be nice to publish for all packages, the source files (in Typescript) so it would be easier to debug, because i saw some packages that miss src files like:
"@accounts/server" "@accounts/graphql-api"

Do you know what might cause this?

versions:
"@accounts/database-manager": "^0.13.0",
"@accounts/graphql-api": "^0.13.0",
"@accounts/mongo": "^0.13.0",
"@accounts/password": "^0.13.0",
"@accounts/server": "^0.13.0",
"@accounts/typeorm": "^0.13.0",
"@graphql-modules/core": "^0.6.6",
"graphql": "^14.2.0",
"graphql-errors": "^2.1.0",
"graphql-import-node": "^0.0.1",
"graphql-tools": "^4.0.4",

@lihaibh
Copy link
Author

lihaibh commented May 11, 2019

any update on this matter?

@pradel
Copy link
Member

pradel commented May 17, 2019

Hey @lihaibh do you think you can provide a example that reproduce your issue, or maybe can you confirm if you can see the same behavior in the examples we have in this repo?

@bbenoist
Copy link
Contributor

Hi! I'm suffering from the same issue. As I have an architecture similar to the examples, I'll try to fix #806 so that we can reproduce the issue for @pradel 😉

@bbenoist
Copy link
Contributor

Well, it turns out that the issue can't be reproduced directly with the graphql-server-typescript, accounts-boost/mono and the accounts-boost/micro examples. I can call privateMutation and query privateField as expected : It returns Unauthorized if missing or incorrect Authorization in the headers but returns the requested values if Authorization is OK.

Which means that the problem has to be with the way @lihaibh and myself specialized the example code 🤔

@bbenoist
Copy link
Contributor

bbenoist commented Oct 11, 2019

After falling on this graphql-tools issue, I've tried to replace the graphql-tools package by graphql-tools-fork.
Once replaced, it now works with my own code (type-graphql + accounts-js microservice 🚀).

I'm not sure whether we should modify the accounts-js examples with graphql-tools-fork since graphql-tools is still supposed to be actively maintained and the examples are actually working. @pradel ?

@darkbasic darkbasic added the 1.0 label Nov 22, 2023
@darkbasic darkbasic added this to the 1.0 milestone Nov 22, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants