chore(staff): Cleanup u2f state in finally again (#67246)

Author: schew2381

src/sentry/auth/authenticators/u2f.py

@@ -327,22 +327,8 @@ def validate_response(self, request: HttpRequest, challenge, response):
             sentry_sdk.capture_exception(err)
             return False
         finally:
-            staff_state = request.session.get("staff_webauthn_authentication_state")
-            default_state = request.session.get("webauthn_authentication_state")
-            logger.info(
-                "State in finally",
-                extra={
-                    "user": request.user.id,
-                    "state_timestamp": (
-                        datetime.fromtimestamp(default_state[1]) if default_state else 0
-                    ),
-                    "staff_timestamp": (
-                        datetime.fromtimestamp(staff_state[1]) if staff_state else 0
-                    ),
-                },
-            )
             # Cleanup the U2F state from the session
-            # request.session.pop("webauthn_authentication_state", None)
-            # request.session.pop("staff_webauthn_authentication_state", None)
-            # request.session.pop("staff_auth_flow", None)
+            request.session.pop("webauthn_authentication_state", None)
+            request.session.pop("staff_webauthn_authentication_state", None)
+            request.session.pop("staff_auth_flow", None)
         return True

tests/sentry/auth/authenticators/test_u2f.py

@@ -126,6 +126,7 @@ def test_validate_response_normal_state(self):
         assert self.u2f.validate_response(self.request, None, self.response)
         _, kwargs = mock_state.call_args
         assert kwargs.get("state") == "normal state"
+        assert "webauthn_authentication_state" not in self.request.session
 
     @freeze_time(CURRENT_TIME)
     def test_validate_response_staff_state_valid_timestamp(self):
@@ -139,6 +140,7 @@ def test_validate_response_staff_state_valid_timestamp(self):
         assert self.u2f.validate_response(self.request, None, self.response)
         _, kwargs = mock_state.call_args
         assert kwargs.get("state") == "staff state"
+        assert "staff_webauthn_authentication_state" not in self.request.session
 
     @freeze_time(CURRENT_TIME)
     def test_validate_response_staff_state_invalid_timestamp(self):
@@ -153,13 +155,15 @@ def test_validate_response_staff_state_invalid_timestamp(self):
         assert self.u2f.validate_response(self.request, None, self.response)
         _, kwargs = mock_state.call_args
         assert kwargs.get("state") == "non-staff state"
+        assert "webauthn_authentication_state" not in self.request.session
 
         # Test timestamp too far in the future
         self.request.session["webauthn_authentication_state"] = ("non-staff state", 5)
         self.request.session["staff_auth_flow"] = self.INVALID_FUTURE_TIMESTAMP
         assert self.u2f.validate_response(self.request, None, self.response)
         _, kwargs = mock_state.call_args
         assert kwargs.get("state") == "non-staff state"
+        assert "webauthn_authentication_state" not in self.request.session
 
     @freeze_time(CURRENT_TIME)
     def test_validate_response_failing_still_clears_all_states(self):
@@ -175,3 +179,6 @@ def test_validate_response_failing_still_clears_all_states(self):
             self.u2f.validate_response(self.request, None, self.response)
         _, kwargs = mock_state.call_args
         assert kwargs.get("state") == "staff state"
+        assert "webauthn_authentication_state" not in self.request.session
+        assert "staff_webauthn_authentication_state" not in self.request.session
+        assert "staff_auth_flow" not in self.request.session