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

JSONWebTokenMiddleware always returns Anonymous User - Possible Bug & Fix #291

Closed
JSv4 opened this issue Oct 31, 2021 · 14 comments · Fixed by #320
Closed

JSONWebTokenMiddleware always returns Anonymous User - Possible Bug & Fix #291

JSv4 opened this issue Oct 31, 2021 · 14 comments · Fixed by #320

Comments

@JSv4
Copy link

JSv4 commented Oct 31, 2021

Hi Folks,

LOVE this project and its amazing contributions to the Graphene / GraphQL ecosystem in Python. I've been running into a weird bug that I think I finally traced and seems to be in your allow_any method in your JSONWebTokenMiddleware. I keep trying to authenticate with a JWT token and keep getting an error that my user is not authenticated. I had a JWT token and it was valid, so I tried to trace where this was coming from in your JSONWebTokenMiddleware with this modified code:

    def resolve(self, next, root, info, **kwargs):

        print(f"Resolve...")

        context = info.context
        token_argument = get_token_argument(context, **kwargs)

        print("Check 1")
        if jwt_settings.JWT_ALLOW_ARGUMENT and token_argument is None:

            user = self.cached_authentication.parent(info.path)
            print(f"User check 1: {user}")


            if user is not None:
                context.user = user

            elif hasattr(context, "user"):
                if hasattr(context, "session"):
                    context.user = get_user(context)
                    self.cached_authentication.insert(info.path, context.user)
                else:
                    context.user = AnonymousUser()

        print("Check 2")
        if (
            _authenticate(context) or token_argument is not None
        ) and self.authenticate_context(info, **kwargs):

            user = authenticate(request=context, **kwargs)
            print(f"User check 2: {user}")


            if user is not None:
                context.user = user

                if jwt_settings.JWT_ALLOW_ARGUMENT:
                    self.cached_authentication.insert(info.path, user)

        print("Return...")
        print(f"context user: {info.context.user}")

        return next(root, info, **kwargs)

Turns out the user keeps coming back as AnonymousUser:

gk_django       | Resolve...
gk_django       | Check 1
gk_django       | Check 2
gk_django       | Return...
gk_django       | context user: AnonymousUser

I traced the code and found your allow_any method appears to be throwing an unexpected error which is handled by the Middleware as if the User is invalid.

This line in allow_any:

field = info.schema.get_type(operation_name).fields.get(info.field_name)

Results in this error:

"'NoneType' object has no attribute 'fields'"

It looks like, when I send a query for a model called corpus, for example, the operation_name argument resolves to "Query". When I look at info.schema.types, however, Query is not one of the types. The CorpusNode name is in there, however. So, when I change the field = call to look to resolve to get_type('CorpusNode')...

field = info.schema.get_type(info.return_type.graphene_type._meta.node._meta.name).fields.get(info.field_name)

It works (with the same JWT token that said I was unauthorized before)!

It looks like others have run into similar-ish sounding problems (see recent issue #289 where someone notes there is a problem in the Middleware), so possibly this is unexpected behavior? Not sure why more people aren't running into this if it's a bug though. If it helps, here's the current list of graphene-related packages I'm using.

graphene                          2.1.9
django-graphql-jwt                0.3.4
graphene-django                   2.15.0
graphene-django-crud              1.3.2
graphene-django-extras            0.4.9
graphene-django-optimizer         0.9.1
graphene-django-plus              4.0.3
graphene-file-upload              1.3.0
graphene-permissions              1.1.4
graphene-subscriptions            1.0.2
graphql-core                      2.3.2
graphql-relay                     2.0.1
@JSv4
Copy link
Author

JSv4 commented Nov 1, 2021

I think there's a little more to this, but I haven't quite been able to figure out what's going on. I forked this repo and made the change to allow_any I suggested above, thinking that would fix the problem. It did not. When I installed from that repo instead of your package, I still kept getting messages that I wasn't authorized.

I did some more digging and it appears under the following conditions I can get everything to work:

  1. I create local copies of your backend, utils and middleware modules, and
  2. I reference my local copies of the backend and middleware in my settings.py

This fixes the issue and no change was needed to allow_any. I got confused when I was trying to bug trace because I had to create local copies to drop in the print statements, and somehow doing that is solving my issue.

At first glance, my backend.py and JSONWebTokenBackend module is identical to the one your library ships with save for the print statements I'm using. I'm having trouble figuring out why the above combination of steps appear to fix things. I'll keep digging here, but wanted to report this in the hopes it's helpful to others in the interim.

@JSv4
Copy link
Author

JSv4 commented Nov 1, 2021

Ok, this got even stranger... I've further narrowed down the behavior here:

  1. The above steps (creating local copies of my middleware) and referencing those in settings.py work but only for lists, particularly, in my implementation a DjangoFilterConnectionField. When I tried to query a single node field (graphene.Field with same model), the weird Anonymous User issued popped up again...
  2. When I also explicitly override JWT_ALLOW_ANY_HANDLER in my settings, then both DjangoFilterConnectionFields and graphene.Fields work perfectly.

It seems like something about this bug is triggered in connection with allow_any. Anyone else have any insights here?

@JSv4
Copy link
Author

JSv4 commented Nov 7, 2021

I'm still not sure why I'm seeing this behavior, but it is NOT due to the your package. I started with a fresh Django project and your package works flawlessly as documented. I haven't figured out why I'm seeing the weird behavior I documented, but I am rebuilding my project one step at a time to see if it comes up again. If I manage to isolate the cause, I'll let you folks know.

@JSv4 JSv4 closed this as completed Nov 7, 2021
@msimav
Copy link

msimav commented Nov 18, 2021

We had the same issue with graphene v3. As a workaround, we copy-pasted the allow_any function from 2f670d7 and set JWT_ALLOW_ANY_HANDLER to use that version of the allow_any function.

django==3.2.9
django-graphql-jwt==0.3.4
graphene==3.0b8
graphql-core==3.1.6
graphene-django==3.0.0b7

@JSv4
Copy link
Author

JSv4 commented Nov 18, 2021

@msimav, thanks for confirming I'm not crazy here! Maybe it's the graphene version that's causing this. I have to check what versions pip installed, but I have two graphene projects - one I originally opened this issue for and another personal project. I rebuilt the project I opened the issue for was able to use the default django-graphql-jwt configurations with no problems at all. At that point, I closed this issue. My other stack, however, still refused to return anything other than an AnonymousUser. I solved that by modifying allow_any and setting it as the JWT_ALLOW_ANY_HANDLER like so:

def allow_any(info, **kwargs):
    try:
        operation_name = get_operation_name(info.operation.operation).title()
        operation_type = info.schema.get_type(operation_name)

        if hasattr(operation_type,'fields'):

            field = operation_type.fields.get(info.field_name)

            if field is None:
                return False

        else:
            return False

        graphene_type = getattr(field.type, "graphene_type", None)

        return graphene_type is not None and issubclass(
            graphene_type, tuple(jwt_settings.JWT_ALLOW_ANY_CLASSES)
        )
    except Exception as e:
        return False

@JSv4 JSv4 reopened this Nov 18, 2021
@msimav
Copy link

msimav commented Nov 19, 2021

Today I created a new project from scratch to see if the problem is related to our codebase.
I think I have found an interesting case.

Problem

In our codebase, we named our query RootQuery.

class RootQuery(
    AQueryMixin,
    BQueryMixin,
    CQueryMixin,
    graphene.ObjectType,
):
    pass


schema = graphene.Schema(query=RootQuery, mutation=RootMutation)

But allow_any function expects a type called Query in the info.schema because operation_name is Query.

def allow_any(info, **kwargs):	 
  operation_name = get_operation_name(info.operation.operation).title()
  # operation_name == "Query"	 
  field = info.schema.get_type(operation_name).fields.get(info.field_name)	 
  # info.schema.get_type("Query") == None
  # so info.schema.get_type("Query").fields raises an error
  # It should have been info.schema.get_type("RootQuery")

Solution

Renaming RootQuery to Query fixed the error on both our codebase and the project I started from scratch.
@JSv4 Can you conform that this solution works for you, too?

@JSv4
Copy link
Author

JSv4 commented Nov 19, 2021

@msimav, I'll try to give it a shot this weekend.

@JSv4
Copy link
Author

JSv4 commented Dec 10, 2021

Finally had some time to work on this again...

So, first off, my root Query is called Query, so I don't think I have the same root cause as you, @msimav. This bug is definitely caused somehow by the operation names, however, as this is my stack trace when I tried the other fix you suggested using from #2f670d7:

| ERROR 2021-12-10 04:23:45,751 executor 11 140139008128768 An error occurred while resolving field Mutation.addTokenAnnotation
django          | Traceback (most recent call last):
django          |   File "/usr/local/lib/python3.9/site-packages/graphql/execution/executor.py", line 452, in resolve_or_error
django          |     return executor.execute(resolve_fn, source, info, **args)
django          |   File "/usr/local/lib/python3.9/site-packages/graphql/execution/executors/sync.py", line 16, in execute
django          |     return fn(*args, **kwargs)
django          |   File "/usr/local/lib/python3.9/site-packages/graphql_jwt/middleware.py", line 71, in resolve
django          |     ) and self.authenticate_context(info, **kwargs):
django          |   File "/usr/local/lib/python3.9/site-packages/graphql_jwt/middleware.py", line 46, in authenticate_context
django          |     if jwt_settings.JWT_ALLOW_ANY_HANDLER(info, **kwargs):
django          |   File "/app/config/graphql/auth/jwt.py", line 7, in allow_any
django          |     f'{info.operation.operation.name.lower()}_type',
django          | AttributeError: 'str' object has no attribute 'name'
django          | ERROR 2021-12-10 04:23:45,751 utils 11 140139008128768 Traceback (most recent call last):
django          |   File "/usr/local/lib/python3.9/site-packages/graphql/execution/executor.py", line 452, in resolve_or_error
django          |     return executor.execute(resolve_fn, source, info, **args)
django          |   File "/usr/local/lib/python3.9/site-packages/graphql/execution/executors/sync.py", line 16, in execute
django          |     return fn(*args, **kwargs)
django          |   File "/usr/local/lib/python3.9/site-packages/graphql_jwt/middleware.py", line 71, in resolve
django          |     ) and self.authenticate_context(info, **kwargs):
django          |   File "/usr/local/lib/python3.9/site-packages/graphql_jwt/middleware.py", line 46, in authenticate_context
django          |     if jwt_settings.JWT_ALLOW_ANY_HANDLER(info, **kwargs):
django          |   File "/app/config/graphql/auth/jwt.py", line 7, in allow_any
django          |     f'{info.operation.operation.name.lower()}_type',
django          | graphql.error.located_error.GraphQLLocatedError: 'str' object has no attribute 'name'

This is the error I was seeing when I first opened this issue. The fix that works for me (and which, as I said, I am a little uncomfortable with as I don't really understand the root cause here and I'm using a naive try / except) is the allow_any method I suggested above:

def allow_any(info, **kwargs):
    try:
        operation_name = get_operation_name(info.operation.operation).title()
        operation_type = info.schema.get_type(operation_name)

        if hasattr(operation_type,'fields'):

            field = operation_type.fields.get(info.field_name)

            if field is None:
                return False

        else:
            return False

        graphene_type = getattr(field.type, "graphene_type", None)

        return graphene_type is not None and issubclass(
            graphene_type, tuple(jwt_settings.JWT_ALLOW_ANY_CLASSES)
        )
    except Exception as e:
        return False

@Bernat417
Copy link

Any idea on what is the purpose of checking if the fields attribute exists in the first place?

Gizmotiretutor pushed a commit to Gizmotiretutor/django-graphql-jwt that referenced this issue Jan 18, 2022
@adamkerz
Copy link

adamkerz commented Dec 2, 2022

I don't understand what's happening, but renaming my query from RootQuery to Query solved it for me. Upgrading packages on an old project.

@Bizor1
Copy link

Bizor1 commented Jan 27, 2023

is it renaming all root queries in the files to Query that fixed it ?

@adamkerz
Copy link

Yep, RootQuery to Query and RootMutation to Mutation.

root_to_no_prefix

@alex20465
Copy link

Today I created a new project from scratch to see if the problem is related to our codebase. I think I have found an interesting case.

Problem

In our codebase, we named our query RootQuery.

class RootQuery(
    AQueryMixin,
    BQueryMixin,
    CQueryMixin,
    graphene.ObjectType,
):
    pass


schema = graphene.Schema(query=RootQuery, mutation=RootMutation)

But allow_any function expects a type called Query in the info.schema because operation_name is Query.

def allow_any(info, **kwargs):	 
  operation_name = get_operation_name(info.operation.operation).title()
  # operation_name == "Query"	 
  field = info.schema.get_type(operation_name).fields.get(info.field_name)	 
  # info.schema.get_type("Query") == None
  # so info.schema.get_type("Query").fields raises an error
  # It should have been info.schema.get_type("RootQuery")

Solution

Renaming RootQuery to Query fixed the error on both our codebase and the project I started from scratch. @JSv4 Can you conform that this solution works for you, too?

This worked for me, thanks.

@cotinus
Copy link

cotinus commented Jun 9, 2023

Today I created a new project from scratch to see if the problem is related to our codebase. I think I have found an interesting case.

Problem

In our codebase, we named our query RootQuery.

class RootQuery(
    AQueryMixin,
    BQueryMixin,
    CQueryMixin,
    graphene.ObjectType,
):
    pass


schema = graphene.Schema(query=RootQuery, mutation=RootMutation)

But allow_any function expects a type called Query in the info.schema because operation_name is Query.

def allow_any(info, **kwargs):	 
  operation_name = get_operation_name(info.operation.operation).title()
  # operation_name == "Query"	 
  field = info.schema.get_type(operation_name).fields.get(info.field_name)	 
  # info.schema.get_type("Query") == None
  # so info.schema.get_type("Query").fields raises an error
  # It should have been info.schema.get_type("RootQuery")

Solution

Renaming RootQuery to Query fixed the error on both our codebase and the project I started from scratch. @JSv4 Can you conform that this solution works for you, too?

18 months later, I lost a half day trying to work out why a code refactor had broken JWT authentication.....and it was exactly this. Great post!

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

Successfully merging a pull request may close this issue.

7 participants