Skip to content

Conversation

@GeoffreyHuck
Copy link
Contributor

Add doc with the processes and new rules.

@GeoffreyHuck GeoffreyHuck requested a review from smadbe June 14, 2024 13:00
Comment on lines +58 to +64
4. We get a lock for the session.
5. We call the login-module to refresh the token, with the refresh token of the session, and get back:
- the new access token
- the new refresh token
- the expiration time of the access token
6. We store the refresh token in the session, and store the new access token.
7. We delete expired old tokens for the session.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You get a lock but you don't say when you release it.
Getting a lock for a remote access is dangerous (if you loose request, you process is killed, ...), how do you protect against that?


### A token can only be refreshed if it's not expired

If an expired token is used to refresh the token, access defined error is returned.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
If an expired token is used to refresh the token, access defined error is returned.
If an expired token is used to refresh the token, access denied error is returned.

?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

fix, it's a 401 unauthorized error


1. The first request gets the lock, and the token gets refreshed.
2. The other requests (already made before the refresh was done), are using tokens that are not the most recent anymore.
The most recent token (just refreshed) will be sent with its expiration time.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not clear, it will not be sent to the other requests, so it is not related with (2).

Co-authored-by: Damien Leroy <smadbe@users.noreply.github.com>
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

Successfully merging this pull request may close these issues.

3 participants