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): handle uniqueness of ApiAuthorization per org #81160

Merged
merged 2 commits into from
Nov 22, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 3 additions & 1 deletion src/sentry/web/frontend/oauth_authorize.py
Original file line number Diff line number Diff line change
Expand Up @@ -171,7 +171,9 @@ def get(self, request: HttpRequest, **kwargs) -> HttpResponseBase:
if not request.user.is_authenticated:
return super().get(request, application=application)

if not force_prompt:
# If the application expects org level access, we need to prompt the user to choose which
# organization they want to give access to every time. We should not presume the user intention
if not (force_prompt or application.requires_org_level_access):
try:
existing_auth = ApiAuthorization.objects.get(
user_id=request.user.id, application=application
Expand Down
65 changes: 65 additions & 0 deletions tests/sentry/web/frontend/test_oauth_authorize.py
Original file line number Diff line number Diff line change
Expand Up @@ -358,6 +358,7 @@ def setUp(self):
super().setUp()
self.owner = self.create_user(email="[email protected]")
self.create_member(user=self.owner, organization=self.organization, role="owner")
self.another_organization = self.create_organization(owner=self.owner)
self.application = ApiApplication.objects.create(
owner=self.user,
redirect_uris="https://example.com",
Expand Down Expand Up @@ -419,3 +420,67 @@ def test_exceed_scope(self):

assert resp.status_code == 302
assert resp["Location"] == "https://example.com?error=invalid_scope&state=foo"

def test_second_time(self):
self.login_as(self.owner)

# before hitting the authorize endpoint we expect that ApiAuthorization does not exist
before_apiauth = ApiAuthorization.objects.filter(
user=self.owner, application=self.application
)
assert before_apiauth.exists() is False

# The first time the app hits the endpoint for the user, it is expected that
# 1. User sees the view to choose an organization
# 2. ApiAuthorization is created with the selected organization
resp = self.client.get(
f"{self.path}?response_type=code&client_id={self.application.client_id}&scope=org:read&state=foo"
)

assert resp.status_code == 200
self.assertTemplateUsed("sentry/oauth-authorize.html")
assert resp.context["application"] == self.application

resp = self.client.post(
self.path, {"op": "approve", "selected_organization_id": self.organization.id}
)

grant = ApiGrant.objects.get(user=self.owner)
assert grant.redirect_uri == self.application.get_default_redirect_uri()
# There is only one ApiAuthorization for this user and app which is related to the right organization
api_auth = ApiAuthorization.objects.get(user=self.owner, application=self.application)
assert api_auth.organization_id == self.organization.id

# The second time the app hits the endpoint for the user, it is expected that
# 1. User still sees the view to choose an organization
# 2. ApiAuthorization is not created again if the user chooses the same organization
resp = self.client.get(
f"{self.path}?response_type=code&client_id={self.application.client_id}&scope=org:read&state=foo"
)
assert resp.status_code == 200
self.assertTemplateUsed("sentry/oauth-authorize.html")
assert resp.context["application"] == self.application
resp = self.client.post(
self.path, {"op": "approve", "selected_organization_id": self.organization.id}
)
same_api_auth = ApiAuthorization.objects.get(user=self.owner, application=self.application)
assert api_auth.id == same_api_auth.id

# The other time the app hits the endpoint for the user, it is expected that
# 1. User still sees the view to choose an organization
# 2. New ApiAuthorization is created again if the user chooses another organization
resp = self.client.get(
f"{self.path}?response_type=code&client_id={self.application.client_id}&scope=org:read&state=foo"
)
assert resp.status_code == 200
self.assertTemplateUsed("sentry/oauth-authorize.html")
assert resp.context["application"] == self.application
resp = self.client.post(
self.path, {"op": "approve", "selected_organization_id": self.another_organization.id}
)
another_api_auth = ApiAuthorization.objects.get(
user=self.owner,
application=self.application,
organization_id=self.another_organization.id,
)
assert api_auth.id != another_api_auth.id
Loading