Skip to content

user deletion: delete all user sessions on deletion request#288

Open
TheHuman00 wants to merge 1 commit into
abh:mainfrom
TheHuman00:fix/delete-sessions-on-user-deletion
Open

user deletion: delete all user sessions on deletion request#288
TheHuman00 wants to merge 1 commit into
abh:mainfrom
TheHuman00:fix/delete-sessions-on-user-deletion

Conversation

@TheHuman00
Copy link
Copy Markdown

Closes #253

Delete all active sessions when a user requests an account deletion, logging them out from all devices (Manage/Account.pm)
Removes the redirect to logout in Manage::init() that was previously handling users in pending deletion (Manage.pm)

Copilot AI review requested due to automatic review settings May 16, 2026 20:33
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

Deletes all active sessions for a user when they request account deletion, so they are logged out from all devices. Also removes the now-redundant safeguard in Manage::init() that redirected users with a pending deletion to logout, since active sessions no longer exist after deletion is requested.

Changes:

  • In render_user_delete, delete all user_sessions rows for the user inside the existing transaction before committing.
  • Remove the deletion_on redirect-to-logout check from NTPPool::Control::Manage::init().

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated no comments.

File Description
lib/NTPPool/Control/Manage/Account.pm Adds delete_user_sessions call within the user-deletion transaction to invalidate all sessions.
lib/NTPPool/Control/Manage.pm Removes the special-case redirect for users with deletion_on since sessions are now actively cleared.

@abh
Copy link
Copy Markdown
Owner

abh commented May 21, 2026

A bit of a nitpick:

Strictly speaking shouldn't the Manage.pm "logout if the user is marked deleted" still be there for a few weeks while existing pending deletion users are deleted? (Or alternatively a migration to delete user sessions for users marked deleted already)?

I think it's a correct change though; how did you find it? An LLM looking for bugs? (Most of this is rewritten on a private branch that's switching the database to Postgres, so I'll merge this when I can also port the change to that branch).

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.

Delete all sessions when a user is marked for deletion

3 participants