France-ioi / AlgoreaBackend

Backend for the new Algorea platform
MIT License
2 stars 2 forks source link

createAccessTokenToken: Limit number of session at 10 / users to avoid session spamming #1036

Closed GeoffreyHuck closed 4 months ago

GeoffreyHuck commented 8 months ago

Related to #1022

Limits the number of sessions to 10 for a user.

When a new session is created (user logs in), we count the number of sessions. If it's more than 10, we remove the oldest one(s).

We first retrieve the list of sessions along with the most recent issued_at access token, sorted by issued_at DESC. Then we delete the oldest.

Adds to new Gherkin features:

Do we need an index?

The query is executed every time a user logs in.

Here's an explain of the request : image

sessions is filtered by user_id, which is a foreign key. access_tokens needs to sort the matching session_id by issued_at. We will never have many access tokens for one session in the database because:

  1. We'll remove the expired tokens (coming soon)
  2. We can only refresh the token every 5 min

So, no index required.

An unnecessary global variable was removed in the test package

Some lint tests were failing, which shown that we had a global variable db for the database in the test package. A refactoring was done to remove the global variable. Details are in the last commit message.

Easier to review commit by commit.

codecov[bot] commented 7 months ago

Codecov Report

All modified and coverable lines are covered by tests :white_check_mark:

Project coverage is 100.00%. Comparing base (7694762) to head (21b116c). Report is 16 commits behind head on master.

Additional details and impacted files ```diff @@ Coverage Diff @@ ## master #1036 +/- ## ========================================= Coverage 100.00% 100.00% ========================================= Files 237 237 Lines 14329 14359 +30 ========================================= + Hits 14329 14359 +30 ```

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.

smadbe commented 7 months ago

We first retrieve the list of sessions along with the most recent issued_at access token, sorted by issued_at DESC. Then we delete the oldest.

I suppose/hope you do sorted by issued_at ASC and take the first one instead :-D

I think for being future-safe (and conceptually correct), you must consider that a session might have no token at all, in which case either issued_at = 1000-01-01 or the session is always destroyed.