Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix(auth): add 401 response to member invite #80800

Open
wants to merge 5 commits into
base: master
Choose a base branch
from

Conversation

mifu67
Copy link
Contributor

@mifu67 mifu67 commented Nov 15, 2024

In the event that a user is logged into Sentry with an account that is different than the account invited to join an organization, we wish to prompt them to log out and try again. To do this, we introduce a 401 UNAUTHORIZED response if the requesting user's email does not match the email in the invite context.

This does not result in any changes on the user side, because the same warning message is rendered for all errors. Frontend changes will come in a subsequent PR.

@github-actions github-actions bot added the Scope: Backend Automatically applied to PRs that change backend components label Nov 15, 2024
Copy link

codecov bot commented Nov 15, 2024

❌ 22 Tests Failed:

Tests completed Failed Passed Skipped
23084 22 23062 215
View the top 3 failed tests by shortest run time
tests.sentry.api.endpoints.test_accept_organization_invite.AcceptInviteTest::test_not_needs_authentication
Stack Traces | 2.93s run time
#x1B[1m#x1B[.../api/endpoints/test_accept_organization_invite.py#x1B[0m:132: in test_not_needs_authentication
    assert resp.status_code == 200
#x1B[1m#x1B[31mE   assert 401 == 200#x1B[0m
#x1B[1m#x1B[31mE    +  where 401 = <Response status_code=401, "application/json">.status_code#x1B[0m
tests.sentry.api.endpoints.test_accept_organization_invite.AcceptInviteTest::test_multi_region_organizationmember_id
Stack Traces | 2.98s run time
#x1B[1m#x1B[.../api/endpoints/test_accept_organization_invite.py#x1B[0m:190: in test_multi_region_organizationmember_id
    assert resp.status_code == 200
#x1B[1m#x1B[31mE   assert 401 == 200#x1B[0m
#x1B[1m#x1B[31mE    +  where 401 = <Response status_code=401, "application/json">.status_code#x1B[0m
tests.sentry.users.api.endpoints.test_user_authenticator_enroll.AcceptOrganizationInviteTest::test_accept_pending_invite__totp_enroll
Stack Traces | 3.15s run time
#x1B[1m#x1B[.../api/endpoints/test_user_authenticator_enroll.py#x1B[0m:452: in test_accept_pending_invite__totp_enroll
    om = self.get_om_and_init_invite()
#x1B[1m#x1B[.../api/endpoints/test_user_authenticator_enroll.py#x1B[0m:354: in get_om_and_init_invite
    assert resp.status_code == 200
#x1B[1m#x1B[31mE   assert 401 == 200#x1B[0m
#x1B[1m#x1B[31mE    +  where 401 = <Response status_code=401, "application/json">.status_code#x1B[0m

To view more test analytics, go to the Test Analytics Dashboard
Got feedback? Let us know on Github

Copy link
Member

@cathteng cathteng left a comment

Choose a reason for hiding this comment

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

chatted in slack, but i think we should make sure we are properly fetching the invite context for the org member being invited before adding the new 401 response

Copy link
Member

@cathteng cathteng left a comment

Choose a reason for hiding this comment

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

very elegant. test pls

# render a view prompting the user to log out and try again.
if not isinstance(request.user, AnonymousUser):
if organization_member.email != request.user.email:
return self.respond_unauthorized(request.user.email)
Copy link
Member

Choose a reason for hiding this comment

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

can we test returning this 401?

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.

2 participants