Skip to content
This repository has been archived by the owner on Aug 29, 2018. It is now read-only.

Restrict authentication by JWT #61

Closed
m0dch3n opened this issue Apr 11, 2018 · 6 comments
Closed

Restrict authentication by JWT #61

m0dch3n opened this issue Apr 11, 2018 · 6 comments

Comments

@m0dch3n
Copy link

m0dch3n commented Apr 11, 2018

If you do

app.authenticate({
  strategy: 'jwt',
  accessToken: 'current access token'
});

You get a new access token every time, with new expiresIn etc, so theoretically, you no longer need the users password to know, because you can simply refresh your token before expiration, by this method.

So if a bad guy steals one of my clients jwt, I'm no longer able to block the user account.

If I blacklist the jwt, I need a database lookup. However if the bad guy already refreshed the jwt, I need to know the whole chain, to block every jwt he generated, so I can't simply block the first stolen jwt...

So I can't blacklist him, nor will the jwt ever expire. The only solution would be, to change my servers secret, however, this would invalidate ALL my others client's session.

So should authentication by jwt not simply verify and return the original jwt, and not generate a new one everyone ?

@m0dch3n
Copy link
Author

m0dch3n commented Apr 12, 2018

Here is a hook, I currently created, to solve the problem

const {checkContext} = require('feathers-hooks-common');

module.exports = function () {
 return async context => {
   checkContext(context, 'after', ['create', 'update', 'patch'], 'jwtAuthentication')
   if (context.path === 'authentication' && context.data && context.data.strategy === 'jwt') {
     context.result.accessToken = context.data.accessToken
   }
   return context
 }
}

@daffl
Copy link
Member

daffl commented Apr 12, 2018

I was under the impression that JWT auth will just verify and return the existing token but it appears that it creates a new one, so you are correct. Your hook is definitely the way to go at the moment, I created a similar fix in feathersjs-ecosystem/authentication#664. Unfortunately it will probably have to be a major release since people probably depend on this behaviour.

I am also working on a new version of the authentication library that will be framework independent, support refresh tokens and blacklisting and not have this issue.

@m0dch3n
Copy link
Author

m0dch3n commented Apr 13, 2018

Ok, that would be great!

That would also close the issue which I referenced above.

I said also a few words about an access/refresh pattern there :

https://github.com/feathersjs/authentication/issues/22#issuecomment-380859393

@andysay
Copy link

andysay commented Apr 15, 2018

nice!

@ydeshayes
Copy link

Do the token expire date is renewed in this case?

@daffl daffl changed the title is authentication by jwt not a security issue ? Restrict authentication by JWT Aug 29, 2018
@daffl
Copy link
Member

daffl commented Aug 29, 2018

This issue was moved to feathersjs/feathers#960

@daffl daffl closed this as completed Aug 29, 2018
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

No branches or pull requests

4 participants