Skip to content

Conversation

Luke-Oldenburg
Copy link
Contributor

@Luke-Oldenburg Luke-Oldenburg commented Sep 5, 2025

Summary of the problem

Closes #7258
Supersedes #7303 and #7408
Screenshot From 2025-09-04 21-51-22

Describe your changes

@Luke-Oldenburg Luke-Oldenburg marked this pull request as ready for review September 5, 2025 01:51
@Luke-Oldenburg Luke-Oldenburg requested review from a team as code owners September 5, 2025 01:51
Copy link
Member

@sampoder sampoder left a comment

Choose a reason for hiding this comment

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

I think we should be setting expiration_at and not bringing back peacefully expired. See #8950

@Luke-Oldenburg
Copy link
Contributor Author

The job malted wrote wouldn't have worked well because of the timing of expiration so I refactored the logic to signed_in_user in SessionsHelper.

Copy link
Member

@sampoder sampoder left a comment

Choose a reason for hiding this comment

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

lgtm! @davidcornu can you take a quick look?

Co-authored-by: Sam Poder <[email protected]>
Luke-Oldenburg and others added 3 commits September 22, 2025 20:03
Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
Copy link
Member

@davidcornu davidcornu left a comment

Choose a reason for hiding this comment

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

Save for #11596 (comment), LGTM.

@sampoder sampoder merged commit ab1d1c4 into main Sep 23, 2025
13 checks passed
@sampoder sampoder deleted the lro-inactivity-logout branch September 23, 2025 00:34
Copy link
Member

@garyhtou garyhtou left a comment

Choose a reason for hiding this comment

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

Question: Does this mean that someone would keep a session forever alive? I feel like we should at least force users to sign in every X period (maybe at least every months; or 2 weeks).

@Luke-Oldenburg
Copy link
Contributor Author

Question: Does this mean that someone would keep a session forever alive? I feel like we should at least force users to sign in every X period (maybe at least every months; or 2 weeks).

#11596 (comment)

@garyhtou
Copy link
Member

Thanks for surfacing that comment. I do strongly believe that there must be a hard time limit on sessions. We shouldn't allow sessions to exist forever.

@Luke-Oldenburg
Copy link
Contributor Author

I would agree with that.

@garyhtou
Copy link
Member

@sampoder, did you have any further thoughts here?

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.

[User Session] Log out after X days of inactivity

4 participants