Skip to content

Fix automatic logout of deleted users#2006

Open
Oaphi wants to merge 1 commit intodevelopfrom
0valt/591/deletion-logout-fix
Open

Fix automatic logout of deleted users#2006
Oaphi wants to merge 1 commit intodevelopfrom
0valt/591/deletion-logout-fix

Conversation

@Oaphi
Copy link
Member

@Oaphi Oaphi commented Mar 4, 2026

closes #591

To test the change:

  1. Log in as any user (important note: the "Stay signed in on this device" option must be checked);
  2. While logged in, mark the user as deleted (the easiest way is to set deleted to true for that user directly in the database);
  3. Reload the page - the header should now look as if you were logged out;
  4. Click on the "sign in" button;
  5. Observe being redirected to the sign in page with the danger notice (incorrectly) saying "You are already signed in";
  6. Checkout the PR, reload the page, and click on the "sign in" button again;
  7. Observe being redirected to the sign in page with the danger notice (correctly) saying:
    • "Invalid email or password" if the user is marked as deleted network-wide;
    • "Your profile on this community has been deleted" if only the user's community user is marked as deleted;

logout helper from Warden::Test::Helpers doesn't actually perform the
logout (so as testing lifecycle callbacks can decide when it happens)
@Oaphi Oaphi requested review from ArtOfCode- and cellio March 4, 2026 12:28
@codecov
Copy link

codecov bot commented Mar 4, 2026

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 80.72%. Comparing base (f8f56f5) to head (b42817d).

Additional details and impacted files
Components Coverage Δ
controllers 75.82% <ø> (ø)
helpers 85.06% <100.00%> (-0.02%) ⬇️
jobs 66.88% <ø> (ø)
models 93.01% <ø> (ø)
tasks 61.11% <ø> (ø)

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@cellio
Copy link
Member

cellio commented Mar 5, 2026

On develop and following the setup instructions, I didn't get the notice at step 5 so I couldn't repro the problem before proceeding. But when I tried to log in it failed, and (as admin) if I try to visit the profile page I get a Rails error because of a data error presumably because this isn't really how to delete users.

The specific change I made in the db was: update users set deleted=true where id=61; (where 61 is one of my cannon-fodder users, nothing else special about it). Is that how I was supposed to do it?

@Oaphi
Copy link
Member Author

Oaphi commented Mar 5, 2026

I get a Rails error because of a data error

Just in case we have one more bug on our hands - what's the exact error you get?

Copy link
Member

@cellio cellio left a comment

Choose a reason for hiding this comment

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

Couldn't repro yesterday's behavior, weird. I'm sorry; it was something about a nil or null that seemed like it might have been about a time value (deleted_at?), but I didn't record it since I expected to be able to repro on demand, oops.

This behaves correctly for me on the branch. However, develop also behaves correctly for me, so I'm not sure what that means. If Art is happy then I'm fine with merging.

@Oaphi
Copy link
Member Author

Oaphi commented Mar 6, 2026

Couldn't repro yesterday's behavior, weird.

My apologies, I forgot to include the part that actually makes the import reproducible - the remember me checkbox must be set when logging in. Please do try again - this is a critical path, so I would like to be extra sure the fix works as expected (and that I've correctly identified the root problem).

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.

You are already signed in shouldn't show for deleted user

3 participants