Project-Books / book-project

Book tracker web app for book lovers
https://project-books.github.io/
GNU General Public License v3.0
476 stars 456 forks source link

Issue 757: Blacklist tokens #956

Closed mslowiak closed 2 years ago

mslowiak commented 2 years ago

Summary of change

Blacklisting tokens based on:

Related issue

Closes #757

Pull request checklist

Please keep this checklist in & ensure you have done the following:

For any of the optional checkboxes (e.g. the screenshots one), still check it if it does not apply.

knjk04 commented 2 years ago

@mslowiak Thanks for working on this! I'll review this at some point this week

knjk04 commented 2 years ago

@mslowiak I've taken a very look over this and noticed you're not creating a table to store blacklisted/already used tokens. Is there a particular reason for this?

mslowiak commented 2 years ago

@mslowiak I've taken a very look over this and noticed you're not creating a table to store blacklisted/already used tokens. Is there a particular reason for this?

@knjk04 Yes, the initial thought was to save every token in a separate table... However, this approach saves some time! If you think about tokens, there is a refresh token and an access token. To deactivate the token after logging out/ changing the password you would need to store the refresh token and access token. When refresh token is not a problem because it is usually long-lived, the access token is short lived, and saving it will hit db every time that refresh token is exchanged to the access token.

As a conclusion to that, I decided to keep the timestamp of the last password change date. At every authenticated request we just query this table - if there is no user it means that he was deleted and the token is no longer valid. When the user exists we just validate if the issue date is after the last password change date.

sonarcloud[bot] commented 2 years ago

SonarCloud Quality Gate failed.    Quality Gate failed

Bug A 0 Bugs
Vulnerability A 0 Vulnerabilities
Security Hotspot E 1 Security Hotspot
Code Smell A 13 Code Smells

54.5% 54.5% Coverage
0.0% 0.0% Duplication

knjk04 commented 2 years ago

Hi @mslowiak, apologies for the delay. Something has come at home, but I'll review your comment as soon as I can

mslowiak commented 2 years ago

@knjk04 how do you do ;)

stale[bot] commented 2 years ago

Is this still relevant? If so, what is blocking it? Is there anything you can do to help move it forward?

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs.

mslowiak commented 1 year ago

@knjk04