Skip to content

feat(users): Enforce account suspension across all auth paths#114349

Merged
dashed merged 20 commits intomasterfrom
aleal/feat/user-suspension-backend
May 1, 2026
Merged

feat(users): Enforce account suspension across all auth paths#114349
dashed merged 20 commits intomasterfrom
aleal/feat/user-suspension-backend

Conversation

@dashed
Copy link
Copy Markdown
Member

@dashed dashed commented Apr 29, 2026

Enforce the new is_suspended field (added in #114328) across all authentication and authorization paths so that suspended users are fully locked out while their account and data are preserved.

Auth Enforcement

  • login() (utils/auth.py): Returns False for suspended users — blocks all session-based login.
  • EmailAuthBackend.authenticate() (utils/auth.py): Intentionally returns the user even if suspended, so callers (AuthenticationForm, auth_login endpoint) can show a specific "suspended" error instead of generic "invalid credentials."
  • EmailAuthBackend.user_can_authenticate() (utils/auth.py): Unchanged — returns True unconditionally (existing behavior since PR feat(django 1.10): continue to let inactive/soft-deleted users authenticate #15974 to support /reactivate/ for inactive users).
  • EmailAuthBackend.get_user() (utils/auth.py): Returns None for suspended users, invalidating existing sessions on next request.
  • AuthenticationForm.clean() (web/forms/accounts.py): Checks is_suspended after authenticate() returns — raises a "Your account has been suspended" error. This is the primary gate for both web form and API login flows.
  • SocialAuthBackend.get_user() (social_auth/backends/__init__.py): Returns None for suspended users (in addition to the existing is_active check), invalidating social-auth-backed sessions.
  • RecoverPasswordForm.clean_user() (users/web/accounts_form.py): Filters out suspended users silently (returns None, same as non-existent users) to prevent password recovery emails from being sent.
  • recover_confirm() (users/web/accounts.py): Early return for suspended users before any password reset form processing, blocking a code path that uses Django's login_user() instead of auth.login().
  • User auth tokens (api/authentication.py): Raises AuthenticationFailed("User account is suspended") for non-system tokens.
  • ViewerContext auth (api/authentication.py): Rejects suspended users with distinct "user_suspended" log reason (vs "user_inactive").
  • Signed links (utils/linksign.py): Returns None for suspended users, invalidating email confirmation links etc.
  • SSO pipeline (auth/helper.py): Checks is_suspended in _finish_login_pipeline() before entering the handler dispatch, preventing an infinite redirect loop where login() returns False and org-subdomain auto-SSO retriggers.
  • 2FA flow (web/frontend/twofactor.py): Replaces assert auth.login() with a conditional check — if a user is suspended mid-2FA flow, they get redirected to login instead of a 500 error.
  • Login API endpoint (api/endpoints/auth_login.py): Defense-in-depth is_suspended check before auth.login(). AuthenticationForm.clean() already catches suspended users (see above), but this explicit check prevents a silent success if the form validation path is ever bypassed.

Superuser API

Superusers can suspend/unsuspend users via PUT /api/0/users/{id}/ with is_suspended. The serializer:

  • Validates that superusers cannot suspend their own account.
  • Validates that Sentry App proxy users cannot be suspended (suspending a proxy user would silently break the entire integration's API token auth).
  • Refreshes the session nonce only on actual state transition (not on idempotent PUTs where the user is already suspended), preventing unnecessary session invalidation.
  • Performs the nonce refresh before super().update() so suspension and nonce invalidation happen in a single DB write.
  • Includes is_suspended in audit logging for privileged field changes.

User Serializer

isSuspended is added to the base UserSerializerResponse so all user serializations include it.

Reactivation View

Suspended users who reach /reactivate/ see a dedicated account-suspended.html template instead of the self-reactivation form, since only an admin can lift a suspension.

API Docs

isSuspended is added to user objects in API docs examples. Non-user objects (project keys, ownership rules, team roles, replay event users) that were incorrectly given the field are cleaned up.

Test Coverage

  • test_user_details.py: Suspend, unsuspend, self-suspension guard, sentry_app proxy guard, session nonce on transition only, idempotent PUT
  • test_authentication.py: Token auth and Viewer Context auth reject suspended users
  • test_auth.py: login() rejects suspended users, authenticate() intentionally returns them, get_user() returns None for suspended users
  • test_helper.py: SSO pipeline blocks suspended users with ERR_USER_SUSPENDED
  • test_linksign.py: Signed links reject suspended users
  • test_reactivate_account.py: Suspended users see suspension template
  • test_twofactor.py: Suspended user in 2FA flow gets redirect to login
  • test_auth_login.py (api): Suspended user API login attempt returns 400
  • test_auth_login.py (web): Suspended user web form login shows suspension error
  • test_accounts.py: Suspended users cannot initiate password recovery, recovery emails not sent
  • test_auth_index.py: Suspended user cannot verify password (sudo)
  • test_utils.py (social_auth): Social auth get_user() returns None for suspended users

Stacked on #114328.

@github-actions github-actions Bot added the Scope: Backend Automatically applied to PRs that change backend components label Apr 29, 2026
@dashed dashed self-assigned this Apr 29, 2026
@dashed dashed marked this pull request as ready for review April 29, 2026 19:31
@dashed dashed requested review from a team as code owners April 29, 2026 19:31
@dashed dashed force-pushed the aleal/feat/user-suspension-backend branch 2 times, most recently from df076a6 to d34f5b4 Compare April 29, 2026 19:37
Comment thread src/sentry/api/authentication.py
Comment thread src/sentry/utils/auth.py
Comment thread tests/sentry/web/frontend/test_reactivate_account.py
Comment thread src/sentry/auth/helper.py
@dashed dashed force-pushed the aleal/feat/user-suspension-migration branch from 2b740b2 to 33bcdfa Compare April 29, 2026 22:11
@dashed dashed requested review from a team as code owners April 29, 2026 22:11
@dashed dashed force-pushed the aleal/feat/user-suspension-backend branch from 682f744 to ed9790b Compare April 29, 2026 22:11
Comment thread src/sentry/utils/auth.py
Comment thread src/sentry/users/api/endpoints/user_details.py
@dashed dashed requested a review from a team as a code owner April 30, 2026 00:10
Comment thread src/sentry/api/endpoints/auth_login.py
Base automatically changed from aleal/feat/user-suspension-migration to master April 30, 2026 11:33
dashed and others added 6 commits April 30, 2026 07:42
Block suspended users from logging in via password, SSO, API tokens,
signed email links, and self-reactivation. Add is_suspended as a
writable field for superusers with automatic session invalidation
on suspend, and expose isSuspended in user API responses.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Move session nonce refresh before super().update() so suspension and
nonce invalidation happen in a single database write. Add validation
to prevent superusers from suspending their own account, which would
permanently lock them out.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Use a separate account-suspended.html template instead of reusing
reactivate-account.html with a context flag. Suspended users see a
clear message directing them to contact their administrator, with no
reactivation form visible.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Add None check for self.instance in validate_is_suspended to satisfy
mypy. Add isSuspended: False to all user objects in API docs examples
so the schema validation passes.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Add an is_suspended check in _finish_login_pipeline() to prevent a
redirect loop for SSO-authenticated suspended users. Without this,
login() returns False, _NotCompletedSecurityChecks redirects to the
login page, and org-subdomain auto-SSO triggers an infinite loop.

The fix catches suspension before entering the handler dispatch,
uses self.error() to show a clear message, and breaks the pipeline.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
dashed and others added 6 commits April 30, 2026 07:42
Add isSuspended to the Author TypedDict in release_details_types.py
since the runtime serializer (UserSerializer) emits it. Restore the
field in the release authors example to match.

Remove isSuspended from the replay example — replay endpoints use
UserResponseType which is a simpler event-user type, not a Sentry
account user.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
refresh_session_nonce() was called whenever is_suspended=True appeared
in the request payload, even if the user was already suspended. This
caused unnecessary nonce rotation on idempotent PUTs. Guard the call
with `not instance.is_suspended` so it only fires on a real state
change from active to suspended.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Cover the two auth paths that were missing suspension test coverage:
UserAuthTokenAuthentication raises AuthenticationFailed for suspended
users, and ViewerContextAuthentication returns None.
Suspending a proxy user would silently break the entire Sentry App
integration by rejecting all API token auth. Add a validation guard
in validate_is_suspended to reject this with a clear error message.
Replace assert auth.login() with a conditional check that redirects
to the login page when login fails (e.g., user suspended mid-2FA
flow). Previously this would raise AssertionError and return a 500.
Defense-in-depth: check is_suspended before calling auth.login() in
the login API endpoint. Form validation already catches suspended
users, but this explicit check prevents a silent success if the form
validation path is ever bypassed.
@dashed dashed force-pushed the aleal/feat/user-suspension-backend branch from b35c959 to 13da55a Compare April 30, 2026 11:42
…c credentials error

The suspension check in EmailAuthBackend.user_can_authenticate() was
rejecting suspended users before AuthLoginEndpoint could show the
"Your account has been suspended" message — making it dead code. Users
saw a misleading "incorrect password" error instead.

Remove the is_suspended check from user_can_authenticate() so
authenticate() returns the user, letting the endpoint's explicit
suspension check fire. auth.login() at the top of the login flow
remains the authoritative security gate for all paths.
Comment thread src/sentry/api/endpoints/auth_login.py
…se-in-depth

EmailAuthBackend.get_user() and SocialAuthBackend.get_user() are called
by Django's session middleware on every request to restore the user from
the session. Neither checked is_suspended, relying solely on session
nonce rotation to block suspended users.

Add is_suspended checks so suspended users are treated as anonymous even
if the nonce mechanism is bypassed. This is belt-and-suspenders — the
primary defense (nonce rotation on suspension) remains, and this adds a
second independent gate at the session restoration layer.
@dashed dashed requested a review from a team as a code owner April 30, 2026 12:31
Comment thread src/sentry/web/frontend/reactivate_account.py
dashed added 2 commits April 30, 2026 08:46
…ation

Cover two test gaps:
- Web form login (/auth/login/) with suspended user: view redirects
  (302) but no session is created since auth.login() returns False
- Auth index PUT (/api/0/auth/) with suspended user: session auth
  backend rejects the user, returning 401
Copy link
Copy Markdown
Contributor

@geoffg-sentry geoffg-sentry 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 I spotted a possible bypass code flow. In src/sentry/users/web/accounts.py there's a direct login_user(request, user) call that's made that users Djangos auth.login instead of our sentry.utils.auth.login. The authenticate check before it doesn't look at is_suspended today.

Comment thread src/sentry/utils/auth.py
@geoffg-sentry
Copy link
Copy Markdown
Contributor

Login API endpoint (api/endpoints/auth_login.py): Defense-in-depth is_suspended check before auth.login(). Form validation already catches suspended users with a generic error (per OWASP — don't reveal account state on login forms), but this explicit check prevents a silent success if the form validation path is ever bypassed.

This doesn't match the change. AuthenticationForm.clean() only fails when authenticate() returns None and suspended users won't reutrn that.

dashed added 2 commits April 30, 2026 11:17
Add is_suspended checks to prevent suspended users from resetting their
password or receiving password recovery emails.

- recover_confirm(): Early return with generic failure page before any
  form processing, password change, or login. Covers all three modes:
  recover, set_password, and relocate.
- RecoverPasswordForm.clean_user(): Silently filter suspended users
  (same as non-existent) to prevent recovery emails being sent.
…-depth

Add is_suspended validation in AuthenticationForm.clean() after
authenticate() succeeds. Shows a specific "Your account has been
suspended." error message instead of silently failing.

This fixes the UX gap where the web form login had no specific error
for suspended users (previously showed a 302 redirect with no session).
Copy link
Copy Markdown
Contributor

@cursor cursor Bot left a comment

Choose a reason for hiding this comment

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

Cursor Bugbot has reviewed your changes and found 2 potential issues.

Fix All in Cursor

❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.

Reviewed by Cursor Bugbot for commit ad32f06. Configure here.

Comment thread src/sentry/web/forms/accounts.py
Comment thread src/sentry/utils/auth.py
@dashed
Copy link
Copy Markdown
Member Author

dashed commented Apr 30, 2026

I think I spotted a possible bypass code flow. In src/sentry/users/web/accounts.py there's a direct login_user(request, user) call that's made that users Djangos auth.login instead of our sentry.utils.auth.login. The authenticate check before it doesn't look at is_suspended today.

@geoffg-sentry yep this is valid. this should be addressed in 88ebf04 (this PR). Fixed the primary bypass in recover_confirm() in src/sentry/users/web/accounts.py; it checks for suspension status before the login_user() call.

we also do not send the recovery email in RecoverPasswordForm.clean_user() for suspended users.

@dashed
Copy link
Copy Markdown
Member Author

dashed commented Apr 30, 2026

Login API endpoint (api/endpoints/auth_login.py): Defense-in-depth is_suspended check before auth.login(). Form validation already catches suspended users with a generic error (per OWASP — don't reveal account state on login forms), but this explicit check prevents a silent success if the form validation path is ever bypassed.

This doesn't match the change. AuthenticationForm.clean() only fails when authenticate() returns None and suspended users won't reutrn that.

AuthenticationForm.clean() now has an explicit suspension status check. this should be addressed in ad32f06

@dashed dashed requested review from a team and geoffg-sentry April 30, 2026 17:11
Copy link
Copy Markdown
Contributor

@michelletran-sentry michelletran-sentry left a comment

Choose a reason for hiding this comment

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

Generally LGTM.

@dashed dashed dismissed geoffg-sentry’s stale review May 1, 2026 16:22

Got thumbs up from Michelle.

@dashed dashed merged commit 5825bec into master May 1, 2026
81 of 82 checks passed
@dashed dashed deleted the aleal/feat/user-suspension-backend branch May 1, 2026 16:22
cleptric pushed a commit that referenced this pull request May 5, 2026
Co-authored-by: Claude Opus 4.6 <noreply@anthropic.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Scope: Backend Automatically applied to PRs that change backend components

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants