In case of wrong password: Cannot return null for non-nullable field ObtainJSONWebToken.token."

[124bit]

Execution of graphql_auth.mutations.ObtainJSONWebToken with wrong credentials leads to error in case if "token" is in requested fields.
If just the "success" field requested - everything works fine.

[PedroBern]

Please run pip freeze and paste the version of the packages here. When I run the same query as you, I got no errors. If you ask all fields inside only one mutation:

mutation {
  check_success: tokenAuth(
    ...
  ) {
    success,
    errors,
    token
  }
}

Does it work?

---

I don't know if the "aaa" is only for the example, but the default config of this package won't allow to create a new user with an invalid email.

[124bit]

-1. Thanks for your reply, and for this great package.

0. Here is the result of the pip freeze. The system is Ubuntu 18.
   freeze_result.txt.txt

1. No, it doesn't work with a single mutation. It fails if one of the returned fields is "token" and the password is wrong. I Made an example with two mutations just for a better explanation.

2. Yes, 'aaa' is just for example, it doesn't work in the general case.

[PedroBern]

Thanks! It still have a lot to get better, everybody is welcome to contribute!

It's probably because of django-graphql-jwt==0.3.1. I didn't have the time to check the new features, but something seems to be backward incompatible. I'm planning to catch up and support the new version, probably releasing v0.4.0 to accomplish this. You can check the progress on #23.

Please, try to run with django-graphql-jwt==0.3.0 to see if it work and let me know.

[124bit]

0-0 I don't know how it happened, I installed 0.3.0 initially. Yes, you are right, with 0.3.0 everything works fine.
Regarding contribution - I tried to do a quick fix and PR of this issue, however, I've stuck in graphql jwt internals.
Thanks

[juanzinno]

Hi!
First of all, this is an excellent project that has been very useful to me.
I write because I had this problem a few days ago. I need the latest version of JWT as I require the refresh_expires_in field which gives me the token_auth mutation, and when using it, this error appeared.
After investigating, I realized that the problem is in required = True that the JsonWebToken class of the JWT has:

class JSONWebTokenMixin:
    payload = GenericScalar(required=True)
    refresh_expires_in = graphene.Int(required=True)

    @classmethod
    def Field(cls, *args, **kwargs):
        if not jwt_settings.JWT_HIDE_TOKEN_FIELDS:
            cls._meta.fields["token"] = graphene.Field(graphene.String, required=True)

            if jwt_settings.JWT_LONG_RUNNING_REFRESH_TOKEN:
                cls._meta.fields["refresh_token"] = graphene.Field(
                    graphene.String, required=True
                )

        return super().Field(*args, **kwargs)

then I changed to:

class JSONWebTokenMixin:
    payload = GenericScalar(required=False)
    refresh_expires_in = graphene.Int(required=False)
    @classmethod
    def Field(cls, *args, **kwargs):
        if not jwt_settings.JWT_HIDE_TOKEN_FIELDS:
            cls._meta.fields["token"] = graphene.Field(graphene.String, required=False)
            if jwt_settings.JWT_LONG_RUNNING_REFRESH_TOKEN:
                cls._meta.fields["refresh_token"] = graphene.Field(
                    graphene.String, required=False,
                )

        return super().Field(*args, **kwargs)

once these fields were set to False, the api started working correctly using the versions:
JWT version django-graphql-jwt==0.3.1
AUTH version django-graphql-auth==0.3.14

I hope it helps you to solve this problem for the new version!
regards!

[PedroBern]

Hi @juanzinno, appreciate you sharing your solution, I will save it for the future :)

[rmtobin]

I also did some digging today and came to this same conclusion that this is the main blocker of updating django-graphql-jwt to 0.3.1, though I'm not yet convinced the code here is the best solution. It would require bringing the JSONWebTokenMixin, ObtainJSONWebTokenMixin, JSONWebTokenMutation, and ObtainJSONWebToken classes from the django-graphql-jwt library into this one just to be able to alter the token field and its friends because of how things are subclassed and the mro. Which then means those need to be maintained in this library and will drift over time from django-graphql-jwt as they add features, which is always a pain.

I tried to figure out a clever way to alter the required attribute of the token etc fields without having to copy over pieces of django-graphql-jwt, but they don't exactly make it easy. If someone else has ideas on how to get at the fields attribute here and change its required status before graphene has the chance to validate fields, I'd be interested to hear.

That said, looking at how django-graphql-jwt handles this case, it may actually be better to raise exceptions for this since it seems like that is what their library (and ultimately graphene) is designed to support. It would be relatively trivial to implement - the resolve_mutation method would raise instead of catch and return the class (at the expense of losing field-level errors in the data payload when obtaining a token). This would also have the benefit of being less susceptible to future changes in django-graphql-jwt.

Curious to hear your thoughts @PedroBern

[PedroBern]

Hi @rmtobin, I didn't investigate any solution, but I agree this doesn't seem to be the best/easier alternative, and take over the maintenance of these classes would be cumbersome.

If someone else has ideas on how to get at the fields attribute here and change its required status before graphene has the chance to validate fields, I'd be interested to hear.

Me too.

It would be relatively trivial to implement - the resolve_mutation method would raise instead of catch and return the class (at the expense of losing field-level errors in the data payload when obtaining a token).

Agree, this is the best solution. Appreciate you investigating and finding a way to solve it :) Would you be interested in sending a PR?