From 24e7cd596f631f382e7edd1c1898294f420e2598 Mon Sep 17 00:00:00 2001 From: Evan Purkhiser Date: Tue, 7 Jan 2020 22:11:07 -0800 Subject: [PATCH] fix(auth): Correct path used for SSO errors The route for sentry-organization-auth-settings was still located in the old settings path, this route is used for a redirect in the auth.helper module when there is a problem configuring SSO. This just corrects the redirect so it goes back to settings instead of 404ing. --- src/sentry/web/urls.py | 10 ++++----- .../test_organization_auth_settings.py | 22 +++++++++++++++++-- 2 files changed, 25 insertions(+), 7 deletions(-) diff --git a/src/sentry/web/urls.py b/src/sentry/web/urls.py index 1a83c7bd1e3041..500482a39686dd 100644 --- a/src/sentry/web/urls.py +++ b/src/sentry/web/urls.py @@ -448,6 +448,11 @@ react_page_view, name="sentry-organization-member-settings", ), + url( + r"^(?P[\w_-]+)/auth/$", + react_page_view, + name="sentry-organization-auth-settings", + ), url(r"^", react_page_view), ] ), @@ -509,11 +514,6 @@ react_page_view, name="sentry-organization-api-key-settings", ), - url( - r"^(?P[\w_-]+)/auth/$", - react_page_view, - name="sentry-organization-auth-settings", - ), url( r"^(?P[\w_-]+)/auth/configure/$", OrganizationAuthSettingsView.as_view(), diff --git a/tests/sentry/web/frontend/test_organization_auth_settings.py b/tests/sentry/web/frontend/test_organization_auth_settings.py index 18af7630bc7f11..b79b63128c8b94 100644 --- a/tests/sentry/web/frontend/test_organization_auth_settings.py +++ b/tests/sentry/web/frontend/test_organization_auth_settings.py @@ -4,6 +4,7 @@ from django.db import models from mock import patch +from sentry.auth.exceptions import IdentityNotValid from sentry.models import ( AuditLogEntry, AuditLogEntryEvent, @@ -94,7 +95,7 @@ def assert_require_2fa_disabled(self, user, organization, logger): "Require 2fa disabled during sso setup", extra={"organization_id": organization.id} ) - def assert_basic_flow(self, user, organization): + def assert_basic_flow(self, user, organization, expect_error=False): configure_path = reverse( "sentry-organization-auth-provider-settings", args=[organization.slug] ) @@ -107,7 +108,13 @@ def assert_basic_flow(self, user, organization): path = reverse("sentry-auth-sso") resp = self.client.post(path, {"email": user.email}) - self.assertRedirects(resp, configure_path) + settings_path = reverse("sentry-organization-auth-settings", args=[organization.slug]) + + if expect_error: + self.assertRedirects(resp, settings_path) + return + else: + self.assertRedirects(resp, configure_path) auth_provider = AuthProvider.objects.get(organization=organization, provider="dummy") auth_identity = AuthIdentity.objects.get(auth_provider=auth_provider) @@ -160,6 +167,17 @@ def test_basic_flow(self, logger): ).exists() assert not logger.info.called + @patch("sentry.auth.helper.logger") + @patch("sentry.auth.providers.dummy.DummyProvider.build_identity") + def test_basic_flow_error(self, build_identity, logger): + build_identity.side_effect = IdentityNotValid() + + user = self.create_user("bar@example.com") + organization = self.create_organization(name="foo", owner=user) + + self.login_as(user) + self.assert_basic_flow(user, organization, expect_error=True) + @patch("sentry.auth.helper.logger") def test_basic_flow__disable_require_2fa(self, logger): user = self.create_user("bar@example.com")