Browse Source

fix(saml2): Don't break if flow from redis is None (#35121)

Alberto Leal 2 years ago
parent
commit
c86a88c0c0

+ 6 - 0
src/sentry/auth/helper.py

@@ -81,6 +81,12 @@ class AuthHelperSessionStore(PipelineSessionStore):
         super().mark_session()
         self.request.session.modified = True
 
+    def is_valid(self):
+        return super().is_valid() and self.flow in (
+            AuthHelper.FLOW_LOGIN,
+            AuthHelper.FLOW_SETUP_PROVIDER,
+        )
+
 
 @dataclass
 class AuthIdentityHandler:

+ 1 - 1
src/sentry/utils/session_store.py

@@ -92,7 +92,7 @@ class RedisSessionStore:
         self.mark_session()
 
     def is_valid(self):
-        return self.redis_key and self._client.get(self.redis_key)
+        return bool(self.redis_key and self._client.get(self.redis_key))
 
     def get_state(self):
         if not self.redis_key:

+ 1 - 1
tests/sentry/utils/test_session_store.py

@@ -42,7 +42,7 @@ class RedisSessionStoreTestCase(TestCase):
         self.store.clear()
 
     def test_uninitialized_store(self):
-        assert not self.store.is_valid()
+        assert self.store.is_valid() is False
         assert self.store.get_state() is None
         assert self.store.some_value is None
 

+ 19 - 0
tests/sentry/web/frontend/test_auth_saml2.py

@@ -10,6 +10,7 @@ from exam import fixture
 
 from sentry import audit_log
 from sentry.auth.authenticators import TotpInterface
+from sentry.auth.helper import AuthHelperSessionStore
 from sentry.auth.providers.saml2.provider import HAS_SAML2, Attributes, SAML2Provider
 from sentry.models import AuditLogEntry, AuthProvider, Organization
 from sentry.testutils import AuthProviderTestCase
@@ -120,6 +121,24 @@ class AuthSAML2Test(AuthProviderTestCase):
         assert auth.status_code == 200
         assert auth.context["existing_user"] == self.user
 
+    def test_auth_idp_initiated_invalid_flow_from_session(self):
+        original_is_valid = AuthHelperSessionStore.is_valid
+
+        def side_effect(self):
+            self.flow = None
+            assert original_is_valid(self) is False
+            return False
+
+        with mock.patch(
+            "sentry.auth.helper.AuthHelperSessionStore.is_valid",
+            side_effect=side_effect,
+            autospec=True,
+        ):
+            auth = self.accept_auth()
+
+        assert auth.status_code == 200
+        assert auth.context["existing_user"] == self.user
+
     @mock.patch("sentry.auth.helper.logger")
     def test_auth_setup(self, auth_log):
         self.auth_provider.delete()