Support JWT / Refresh tokens

[timkelty]

Side note: This looks great! I was just about to start a project where I would have had to do nearly the same thing. Thanks!

Did you/have you considered using JWT at all?

As I understand it, the downside of doing things how you are is the client is forced to store the access token, likely insecurely (eg localstorage). So you're left with either long expiry (less secure, hard to invalidate) or short ones (user keeps getting logged out).

With JWT, this is usually mitigated by making the JWT expiry super short, and refreshing it with a "refresh token", as outlined here: https://hasura.io/blog/best-practices-of-using-jwt-with-graphql/#silent_refresh

JWT also gives you the opportunity to get back some data about the user (e.g. member group), should you want to do something with permissions, but non-gql based (e.g. routing).

[jamesedmonston]

@timkelty Thanks for the link! I'll have a read through that article this evening 👍🏻

I could potentially look into adding some settings to let you decide how the tokens are handled:

1. Returned in response (as it is currently)
2. Sent as a secure SameSite cookie – browser support seems pretty good for this now: https://caniuse.com/same-site-cookie-attribute
3. Proper JWT tokens

[timkelty]

SameSite cookies are referenced in that article – but also setting it in an httpOnly cookie is a good start. I think an "easy" addition, short of JWT, would be to return the token in an httpOnly (and/or SameSite).

Let me know what you think!

[jamesedmonston]

@timkelty yeah that sounds like a great 'quick win'! I'll see if I can get around to adding that in this evening, then explore JWT possibilities over the coming days.

Thanks!

[timkelty]

hooray! happy to help with JWT/refresh if/when you tackle that.

[jamesedmonston]

@timkelty https://github.com/jamesedmonston/graphql-authentication/commit/98437b77845506eb92ed5965ccbf8390e333fab6 👀

Going to run through some testing before pushing a release.

Would you still expect the token expiry setting to be taken into account if using the cookies, or just if you're using the less secure method?

[jamesedmonston]

@timkelty I've slightly altered the logic: https://github.com/jamesedmonston/graphql-authentication/commit/b8e0bed195afd7cb799198b7d6ee9582505b096a

I originally omitted the tokens from the register/authenticate mutation responses if the cookie setting was enabled, but realised that they'll still be useful for putting into iOS/Android Keychain for use in native apps.

[jamesedmonston]

@timkelty https://github.com/jamesedmonston/graphql-authentication/releases/tag/1.1.0

[timkelty]

Wow, what service! I already have some projects lined up that could use this.

Thanks a ton @jamesedmonston!

For my immediate use, I think I may have to use JWT, for other reasons (specifically, frameworks like Nuxt Auth come with native support for JWT/refresh tokens)

Before I dig into the codebase, do you think modifying this plugin (adding JWT) makes sense, or do you think that would be best as a separate module/plugin?

[jamesedmonston]

@timkelty Yeah, I definitely think it's within the scope of the plugin to add it. Seems a shame to miss an opportunity for additional use-cases. I see it as (hopefully) ending up with various authentication methods, including social sign-in etc, too.

[timkelty]

@jamesedmonston awesome. I'm probably going to work on JWT integration this weekend. If you can point me to points in the codebase (only glanced through it so far) you think are the right places for implementing this, that might help.

Feel free to hit me up on the Craft Discord (timkelty) or email if you want to discuss.

[jamesedmonston]

@timkelty Really appreciate that! I can generate a couple licence keys for you if you'd like (or Beerpay/similar)?

Had a read through that article you sent over – interesting stuff! It mentions only returning the refresh token as an HTTP-Only cookie; do you think it'd be worth adding a setting that also returns it in the mutation response, for use in environments where cookies aren't available (e.g. native apps)?

Had a quick Google for Yii JWT Token libraries and saw this: https://github.com/sizeg/yii2-jwt – maybe it could be of some use?

Do you have any thoughts on the best place to store the refresh tokens? I was thinking they could potentially be stored in a custom channel – we could then add a Refresh Tokens section for admins to manually invalidate/remove them if necessary.

[timkelty]

do you think it'd be worth adding a setting that also returns it in the mutation response, for use in environments where cookies aren't available (e.g. native apps)?

That seems reasonable. I also think maybe just putting events in the right place with some examples of how to override things might be good too, since everyone's implementation could be different.

Had a quick Google for Yii JWT Token libraries and saw this: https://github.com/sizeg/yii2-jwt – maybe it could be of some use?

Hmm…I wonder what that would give us vs using lcobucci/jwt directly. Will have to look.

Do you have any thoughts on the best place to store the refresh tokens

I was planning on using Craft's Tokens (https://docs.craftcms.com/api/v3/craft-services-tokens.html#public-methods), the same kind it uses for password resets, etc. Pretty sure you can tag those so you could show them in the admin specifically.

[jamesedmonston]

That seems reasonable. I also think maybe just putting events in the right place with some examples of how to override things might be good too, since everyone's implementation could be different.

Yeah that's a good idea, the docs definitely need fleshing out and making a little easier to understand, generally (will sort soon).

I was planning on using Craft's Tokens (https://docs.craftcms.com/api/v3/craft-services-tokens.html#public-methods), the same kind it uses for password resets, etc. Pretty sure you can tag those so you could show them in the admin specifically.

Oo, neat. Yeah that sounds great 👍🏻

[jamesedmonston]

@timkelty @daltonrooney I've just pushed the first lot of work for the JWT integration: https://github.com/jamesedmonston/graphql-authentication/commit/3c93a4071b0de10f433c752531f7ba9c712c1370

Passing a JWT into the Authorization header as Bearer will fail as Craft is doing its own validation. You have to pass it in as Authorization: JWT ${token} at the moment – I'm not sure if there's anything I can do to get around this, but I'll have another look. Can you see any issues with it being passed through as JWT?

The following information is currently passed into the JWT payload:

{
  "iss": "https://plugins.localhost/",
  "iat": 1606698926,
  "exp": 1606699826,
  "userId": 21,
  "fullName": "James Edmonston",
  "accessToken": "FdwPLuZQHIzaV2aEI8qYLFHqd1PtMQuU",
  "schema": "User"
}

You can ignore the accessToken field – that's just used for grabbing the GQL token to use inside Craft. Is there anything else you think should be in there?

To get started, go into the plugin settings and change the Token Type field to JWT and save (you may need to save twice to get the JWT Secret Key field to save – will fix tomorrow!).

After that's set, your authentication mutations (including social!) will start returning the JWT, and setting a refresh token cookie.

You can refresh the JWT by calling the refreshToken mutation. It uses the aforementioned refresh token cookie. You can also pass the refresh token directly into the mutation arguments (useful for native apps).

I'm sure there'll be a few bugs as this is just the first pass, and I know there's a few things I need to tidy up.

@timkelty I ended up creating a custom refresh token section in the admin that uses its own database table etc. as I couldn't think of a nice way of actually tying the Craft tokens to users:

Cheers

[timkelty]

@jamesedmonston this looks great!

I was hacking on some of the same stuff last night, but this looks more integrated into the plugin.

Is there anything else you think should be in there?

• As we're never going to meet everyone's needs, I definitely think we need an event to control the payload and the verification data.
• If we want to match the JWT spec, I think userId should be sub: https://tools.ietf.org/html/rfc7519#section-4.1.2
• I know @daltonrooney specifically wants user group info in here, so he can do things conditionally based on permissions:

                ->withClaim('sub', $user->id)
                ->withClaim('admin', $user->admin)
                ->withClaim('groups', array_column($user->getGroups(), 'id'))

That said, could be fairly specific to the app, and maybe should be done via event

I'm also wondering if issuedBy should be Craft::$app->id…might make things easier testing between environments.

[timkelty]

So, I think we'd want 2 events:

• one for TokenService::EVENT_CREATE_JWT that has the token builder and user as an arg, so you can add your own claims.
• one that passes the JWT config (in getHeaderToken), so you can customize the validation.

[jamesedmonston]

@timkelty Appreciate your feedback!

• Good point on moving the user ID into sub.
• I definitely like the idea of adding events to modify the payload/verification – as you've said, we'll never get the right fields for everybody's needs otherwise.
• Updating issuedBy as you said makes sense to me.

I'll try and take a stab at these tweaks this evening 👍🏻

[timkelty]

It looks like ->relatedTo('user123') is how the builder wants your to set sub.

@jamesedmonston It doesn't look like you're setting any validation constraints explicitly…do you know if we need to explicitly set Lcobucci\JWT\Validation\Constraint\ValidAt, Lcobucci\JWT\Validation\Constraint\RelatedTo, etc. to our assertions?

[jamesedmonston]

@timkelty It's currently only validating based on the secret key being the same:

https://github.com/jamesedmonston/graphql-authentication/blob/develop/src/services/TokenService.php#L145-L163

Specifically: https://github.com/jamesedmonston/graphql-authentication/blob/develop/src/services/TokenService.php#L152-L153

[jamesedmonston]

@timkelty I've made a few updates to this:

• Added email, groups, and admin to JWT payload
• Moved userId into sub
• Updated issuedBy to use Craft::$app->id (fallback to cpUrl, just in case!)
• Expired access & refresh tokens are now removed automatically
• Refresh tokens are now consumed (deleted) when calling the refreshToken mutation

Didn't get time to look into adding events for the payload/verification modification unfortunately – I'll try and sort that tomorrow evening.

[timkelty]

Looks great!

Didn't get time to look into adding events for the payload/verification modification unfortunately

let me know if you need any help with that!

[jamesedmonston]

@timkelty @daltonrooney I've pushed the events for modifying the JWT at creation time, and the validators used to verify it!

An example module:

<?php

namespace modules;

use craft\base\Plugin;
use jamesedmonston\graphqlauthentication\events\JwtCreateEvent;
use jamesedmonston\graphqlauthentication\events\JwtValidateEvent;
use jamesedmonston\graphqlauthentication\services\TokenService;
use Lcobucci\JWT\Validation\Constraint\IssuedBy;
use yii\base\Event;

class ModifyJwt extends Plugin
{
    public function init()
    {
        parent::init();

        Event::on(
            TokenService::class,
            TokenService::EVENT_BEFORE_CREATE_JWT,
            [$this, 'addJwtClaims']
        );

        Event::on(
            TokenService::class,
            TokenService::EVENT_BEFORE_VALIDATE_JWT,
            [$this, 'validateJwt']
        );
    }

    public function addJwtClaims(JwtCreateEvent $event)
    {
        $builder = $event->builder;
        $user = $event->user;

        $builder->withClaim('customClaim', 'customValue');
    }

    public function validateJwt(JwtValidateEvent $event)
    {
        $config = $event->config;
        $validator = new IssuedBy('Custom Validator');
        $config->setValidationConstraints($validator);
    }
}

[jamesedmonston]

Going to pre-emptively close this as I think everything has been taken care of. We can track any bugs in new issues. Cheers

[timkelty]

@jamesedmonston you're killing it!