Adds user_secret validation for auth_token and refresh_token #80
Conversation
…uth_token, so that the user_secret in the token is the same as the in the DB. If a user refreshes the user_secret, it therefore invalidates all tokens.
… in/out of the stateful auth_token.
|
I just added |
… folder won't be commited.
For now, if we provide proper hooks/filters, folks can use them to override these types of settings. Some sort of backend option page I think will need a larger discussion on how to best approach that. Do we want the main WPGraphQL plugin to add a I'm hesitant to add admin pages for these plugins as it creates a lot of maintenance overhead, etc. |
If we go this way, we can probably go to a single-token flow then? 🤔 The 2-token approach was because the access token couldn't be revoked or otherwise invalidated, but was short lived, so the 2nd token allowed to refresh, but do nothing else. If the auth-token now has the user_secret as well, the 2 tokens are the same and there's not really any need for both types of tokens, or a refresh flow at all, eh? |
| @@ -156,15 +176,17 @@ protected static function get_signed_token( $user, $cap_check = true ) { | |||
| 'exp' => self::get_token_expiration(), | |||
| 'data' => [ | |||
| 'user' => [ | |||
| 'id' => $user->data->ID, | |||
| 'id' => $user->data->ID, | |||
| 'user_secret' => $secret, | |||
There was a problem hiding this comment.
At the moment, this is the one thing that makes a refresh token different than an access token. Having this in both tokens makes the tokens identical.
| @@ -11,6 +11,8 @@ class Auth { | |||
| protected static $issued; | |||
| protected static $expiration; | |||
| protected static $is_refresh_token = false; | |||
| protected static $refresh_token_valid_days = 30; | |||
There was a problem hiding this comment.
I think 30 days is probably a fine default instead of a year. If you're not using an application for more than 30 days, the application asking you to login again seems reasonable.
| throw new \Exception( __( 'The User Secret does not match for this user', 'wp-graphql-jwt-authentication' ) ); | ||
| } | ||
| } else if (self::$auth_token_is_stateful || self::$is_refresh_token) { | ||
| throw new \Exception( __( 'The User Secret is missing in the token.', 'wp-graphql-jwt-authentication' ) ); |
There was a problem hiding this comment.
Same comment as above. If the secret exists for both auth tokens and refresh tokens, the tokens are the same 🤔
Related issue: #79
So I tried myself on some php today and implemented the validation for user_secrets.
graphql_jwt_auth_secretmeta data.If we want to keep the statelessness of auth_tokens, we could make this optional, so you could opt in, to have the user_secret in auth_tokens?
I'd like to give that option actually, so the users can decide themselves. Also we could let them override the expiry time days. What would be the best option to give that choice? A backend option_page or just a define in config.php?
Well it is not the most beautiful code and I haven't written tests for it, but @jasonbahl said the code will likely be refactored and we just wanna get things working for now.