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

Using email in JWT a security attack vector #23

Open
moll opened this issue Dec 8, 2015 · 3 comments
Open

Using email in JWT a security attack vector #23

moll opened this issue Dec 8, 2015 · 3 comments

Comments

@moll
Copy link

moll commented Dec 8, 2015

https://github.com/Prismatik/auth/blob/master/controllers/login.js returns a JWT keyed to the email. That's not particularly secure as it'll give you access to all future accounts that use that email address. Sessions should be tied to their related entities by id, i.e., users, so they're guaranteed to only apply to the user that created them. They should not be invalidated when a user changes her email address in the app.

@davidbanham
Copy link
Contributor

That's a good point. I'll add the ID to the token.

I feel like it would be ideal to still have the email in there. It'll maintain backwards compatibility and it seems like a useful piece of information to have around. The reason I originally keyed it off email is that I figured it's likely to have more utility to something consuming the JWT than the ID.

The downsides to leaving it in are:

  1. More bytes on something that will probably be sent across the wire on every request.
  2. Gives developers the option of mistakenly keying off the email rather than the ID and leaving this vector open.

What do you think? Do the scales of usefulness tip towards leaving it in or taking it out?

@moll
Copy link
Author

moll commented Dec 8, 2015

I'd take it out, lest you end up having to support it forever. Plus your #2. ;-)

The current setup has a client of Auth do JWT validation at their end anyways, so those, who do need more data in it, can reserialize it.

@davidbanham
Copy link
Contributor

ref #30

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

No branches or pull requests

2 participants