chore(issues): Prevent assigning issues to deactivated users#115668
Conversation
github-actions
Bot
added
the
Scope: Backend
|
makes sense! just a question for my edification:
how does a user get deactivated? |
only superusers can hard delete accounts. soft deletes (deactivation) is the default where user_is_active is set to false, but the account itself is still in the db |
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
![sentry[bot]](https://avatars.githubusercontent.com/in/12637?s=60&v=4){kind=small}
cvxluo
left a comment
cvxluo
left a comment
There was a problem hiding this comment.
looks good, some minor questions
| user = Factories.create_user() | ||
| org = Factories.create_organization(owner=user) | ||
|
|
||
| OrganizationMember.objects.filter(organization=org, user_id=user.id).update( |
There was a problem hiding this comment.
nit: why not do user.is_active = False, user.save() instead of filtering and updating?
| @@ -950,6 +951,37 @@ def test_post_delete_sets_ownership_version(self) -> None: | |||
| assert version is not None | |||
| assert isinstance(version, float) | |||
|
|
|||
| def test_handle_auto_assignment_skips_deactivated_user(self) -> None: | |||
| self.ownership = ProjectOwnership.objects.create( | |||
There was a problem hiding this comment.
nit, why assign these to self here? do other tests do that here?
| if OrganizationMember.objects.filter( | ||
| organization_id=group.project.organization_id, | ||
| user_id=owner.id, | ||
| user_is_active=False, | ||
| ).exists(): | ||
| logger.info("handle_auto_assignment.skip_deactivated_user", extra=logging_extra) | ||
| return |
There was a problem hiding this comment.
why do we filter by inactive owners rather than checking owner.is_active?
| from sentry.users.services.user import RpcUser | ||
|
|
||
| if isinstance(assigned_to, (UserModel, RpcUser)): | ||
| if OrganizationMember.objects.filter( |
There was a problem hiding this comment.
similar comment to https://github.com/getsentry/sentry/pull/115668/changes#r3260585645
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes and found 1 potential issue.
❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.
Reviewed by Cursor Bugbot for commit 426eef7. Configure here.
| if isinstance(assigned_to, (UserModel, RpcUser)) and assigned_to.is_active is False: | ||
| return {"new_assignment": False, "updated_assignment": False} |
There was a problem hiding this comment.
Bug: GroupAssignee.objects.assign() now silently fails for deactivated users, but the integration sync path in sync.py doesn't check the return value, leading to inconsistent assignment states.
Severity: MEDIUM
Suggested Fix
The caller, _handle_assign in sync.py, should check the return value of GroupAssignee.objects.assign(). If the assignment was not successful (e.g., new_assignment is False), the group should not be appended to the groups_assigned list. This will ensure that a partial sync failure is correctly reported when attempting to assign a deactivated user.
Prompt for AI Agent
Review the code at the location below. A potential bug has been identified by an AI
agent. Verify if this is a real issue. If it is, propose a fix; if not, explain why it's
not valid.
Location: src/sentry/models/groupassignee.py#L141-L142
Potential issue: The integration sync path in `_handle_assign` calls
`GroupAssignee.objects.assign()` but does not check its return value. With the new
change, if `assign()` is called with a deactivated user, it silently returns without
performing the assignment. However, the caller in `sync.py` unconditionally adds the
group to `groups_assigned`, incorrectly reporting a successful assignment. This prevents
a `lifecycle.record_halt` from being triggered, which should signal a partial sync
failure. Consequently, the external tool (e.g., Jira, GitHub) and Sentry will have
inconsistent assignee states, with the external tool showing an assignment that doesn't
exist in Sentry.
There was a problem hiding this comment.
preexisting issue, will create a follow-up ticket for this
There was a problem hiding this comment.
4 participants
Addresses ISWF-2214
Prevents assigning issues to deactivated users for both auto-assignments and manual ones.