From deedb74828b3ea196df38b4d1487fb501e3966cb Mon Sep 17 00:00:00 2001 From: Athena Moghaddam <132939361+sentaur-athena@users.noreply.github.com> Date: Mon, 11 Nov 2024 10:38:25 -0800 Subject: [PATCH] fix(oauth): Show error if user has no org (#80141) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit This flow is org scoped oauth so the user entering this flow must be a member of at least one organization. Before this would break the UI but now we show a proper error. Screenshot 2024-11-01 at 11 23 14 AM --- src/sentry/web/frontend/oauth_authorize.py | 8 +++++++ .../web/frontend/test_oauth_authorize.py | 22 ++++++++++++++++--- 2 files changed, 27 insertions(+), 3 deletions(-) diff --git a/src/sentry/web/frontend/oauth_authorize.py b/src/sentry/web/frontend/oauth_authorize.py index 691600344a98d..8617bdbd5a465 100644 --- a/src/sentry/web/frontend/oauth_authorize.py +++ b/src/sentry/web/frontend/oauth_authorize.py @@ -211,6 +211,14 @@ def get(self, request: HttpRequest, **kwargs) -> HttpResponseBase: if application.requires_org_level_access: organization_options = user_service.get_organizations(user_id=request.user.id) + if not organization_options: + return self.respond( + "sentry/oauth-error.html", + { + "error": "This authorization flow is only available for users who are members of an organization." + }, + status=400, + ) else: # If application is not org level we should not show organizations to choose from at all organization_options = [] diff --git a/tests/sentry/web/frontend/test_oauth_authorize.py b/tests/sentry/web/frontend/test_oauth_authorize.py index f509d84eab2d0..15f78aee47a67 100644 --- a/tests/sentry/web/frontend/test_oauth_authorize.py +++ b/tests/sentry/web/frontend/test_oauth_authorize.py @@ -356,6 +356,8 @@ def path(self): def setUp(self): super().setUp() + self.owner = self.create_user(email="admin@test.com") + self.create_member(user=self.owner, organization=self.organization, role="owner") self.application = ApiApplication.objects.create( owner=self.user, redirect_uris="https://example.com", @@ -363,8 +365,22 @@ def setUp(self): scopes=["org:read", "project:read"], ) + def test_no_orgs(self): + # If the user has no organizations, this oauth flow should not be possible + user = self.create_user(email="user1@test.com") + self.login_as(user) + resp = self.client.get( + f"{self.path}?response_type=code&client_id={self.application.client_id}&scope=org%3write&state=foo" + ) + assert resp.status_code == 400 + self.assertTemplateUsed("sentry/oauth-error.html") + assert ( + resp.context["error"] + == "This authorization flow is only available for users who are members of an organization." + ) + def test_rich_params(self): - self.login_as(self.user) + self.login_as(self.owner) # Putting scope in the query string to show that this will be overridden by the scopes that are stored on the application model resp = self.client.get( @@ -379,7 +395,7 @@ def test_rich_params(self): self.path, {"op": "approve", "selected_organization_id": self.organization.id} ) - grant = ApiGrant.objects.get(user=self.user) + grant = ApiGrant.objects.get(user=self.owner) assert grant.redirect_uri == self.application.get_default_redirect_uri() assert grant.application == self.application assert grant.get_scopes() == ["org:read", "project:read"] @@ -394,4 +410,4 @@ def test_rich_params(self): f"state=foo&code={grant.code}" ) - assert not ApiToken.objects.filter(user=self.user).exists() + assert not ApiToken.objects.filter(user=self.owner).exists()