fix: session refresh works as intended now #5330
Open
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Fixes #5324
What's fixed
/session/refreshfailed with expired sessions given, but the frontend only attempted to refresh after a failed fetch. The route fails because it attempts to get the current user from the auth token, but this fails because the session has expired.refresh_expiresDB column would be reset to 60 days, not retain the timestamp from the original session, making this functionality completely useless./session/refreshaccepted any authorization token, not justmra_session tokens, and would issue a new session.How's it fixed
struct SessionBuilderhas two new fields,expires: Option<DateTime<Utc>>andrefresh_expires.None, database defaults are usedget_user_record_from_bearer_tokento ignore session expiryget_user_from_bearer_tokenfunction that transformsDBUser->Userroutes/internal/session.rs/issue_sessionfunction now acceptsrefresh_expires: Option<DateTime<Utc>>as a new parameter, feeds it into theSessionBuildersession/refreshroute now feeds in the expired session'srefresh_expiressession/refreshroute from accepting Authorization tokens that are notmra_Questions to you
In an attempt to not duplicate the defaults set by the database table migration
(https://github.com/modrinth/code/blob/main/apps/labrinth/migrations/20230628180115_kill-ory.sql#L32-L33), a slightly over-complicated match expression was used to only insert those values when they are
Some.https://github.com/isXander/modrinth/blob/76c63148152b7591b7c93688175bed8e8da4b8aa/apps/labrinth/src/database/models/session_item.rs#L66-L111
If instead you do not mind some duplication, this would make this function significantly simpler.