thomas4019 / expressa

API creation middleware with an admin interface
MIT License
439 stars 27 forks source link

Token Expire on Password Change #185

Closed kane-mason closed 2 years ago

kane-mason commented 2 years ago

Attempt to fix #184

This is one way to resolve this issue. Store the timestamp of when the password was last updated in the token. This way, when password is changed, old tokens will no longer be valid, and those users will need to log in again. This includes logged in user.

Another way is to store session information in a collection. Im not opposed to this but figured it was a more honerous solution which could also be exploited if not careful. My initial solution seems simpler but possibly less robust.

Im keen to hear your thoughts @thomas4019 ?

thomas4019 commented 2 years ago

I like this solution. Some feedback: 1) It would be good to add a test to show that this works and ensure it doesn't get broken accidentally 2) Rename updated_password on the User model to something like password_last_updated_at. I find the first name confusing until I read through the code carefully and your description.

kane-mason commented 2 years ago

@thomas4019 i added a unit test let me know what you think.

I was thinking of being able to enable this feature behind a setting but am battling to come up with a name.. Does it fall under the jwt_ setting space?

thomas4019 commented 2 years ago

@thomas4019 i added a unit test let me know what you think.

I was thinking of being able to enable this feature behind a setting but am battling to come up with a name.. Does it fall under the jwt_ setting space?

Your unit test looks great, nice addition! Hmm, I do think it would fall under jwt. How about jwt_expire_on_password_change?

kane-mason commented 2 years ago

@thomas4019 i added a unit test let me know what you think. I was thinking of being able to enable this feature behind a setting but am battling to come up with a name.. Does it fall under the jwt_ setting space?

Your unit test looks great, nice addition! Hmm, I do think it would fall under jwt. How about jwt_expire_on_password_change?

I like that, will implement.