From e38f1283eae2c5325f0bd71c793ca518c35886b0 Mon Sep 17 00:00:00 2001 From: Saurya Velagapudi Date: Wed, 4 Feb 2026 17:58:44 -0800 Subject: [PATCH] feat(recaptcha): add user_id and email to assessment log (#12749) Co-authored-by: openhands --- enterprise/server/auth/recaptcha_service.py | 4 ++ enterprise/server/routes/auth.py | 1 + .../tests/unit/test_recaptcha_service.py | 58 +++++++++++++++++++ 3 files changed, 63 insertions(+) diff --git a/enterprise/server/auth/recaptcha_service.py b/enterprise/server/auth/recaptcha_service.py index b839e87868..3047aa754f 100644 --- a/enterprise/server/auth/recaptcha_service.py +++ b/enterprise/server/auth/recaptcha_service.py @@ -64,6 +64,7 @@ class RecaptchaService: user_ip: str, user_agent: str, email: str | None = None, + user_id: str | None = None, ) -> AssessmentResult: """Create a reCAPTCHA Enterprise assessment. @@ -73,6 +74,7 @@ class RecaptchaService: user_ip: The user's IP address. user_agent: The user's browser user agent. email: Optional email for Account Defender hashing. + user_id: Optional Keycloak user ID for logging correlation. Returns: AssessmentResult with score, validity, and allowed status. @@ -143,6 +145,8 @@ class RecaptchaService: 'has_suspicious_labels': has_suspicious_labels, 'allowed': allowed, 'user_ip': user_ip, + 'user_id': user_id, + 'email': email, }, ) diff --git a/enterprise/server/routes/auth.py b/enterprise/server/routes/auth.py index 13ca39ff69..a31a2042f4 100644 --- a/enterprise/server/routes/auth.py +++ b/enterprise/server/routes/auth.py @@ -219,6 +219,7 @@ async def keycloak_callback( user_ip=user_ip, user_agent=user_agent, email=email, + user_id=user_id, ) if not result.allowed: diff --git a/enterprise/tests/unit/test_recaptcha_service.py b/enterprise/tests/unit/test_recaptcha_service.py index 4758d4ff48..11ace835c4 100644 --- a/enterprise/tests/unit/test_recaptcha_service.py +++ b/enterprise/tests/unit/test_recaptcha_service.py @@ -290,6 +290,64 @@ class TestRecaptchaServiceCreateAssessment: assert call_kwargs[1]['extra']['action_valid'] is True assert call_kwargs[1]['extra']['user_ip'] == '192.168.1.1' + def test_should_log_user_id_and_email_when_provided( + self, recaptcha_service, mock_gcp_client + ): + """Test that user_id and email are included in log when provided.""" + # Arrange + mock_response = MagicMock() + mock_response.name = 'projects/test-project/assessments/xyz123' + mock_response.token_properties.valid = True + mock_response.token_properties.action = 'LOGIN' + mock_response.risk_analysis.score = 0.9 + mock_response.risk_analysis.reasons = [] + mock_gcp_client.create_assessment.return_value = mock_response + + with patch('server.auth.recaptcha_service.logger') as mock_logger: + # Act + recaptcha_service.create_assessment( + token='test-token', + action='LOGIN', + user_ip='192.168.1.1', + user_agent='Mozilla/5.0', + email='test@example.com', + user_id='keycloak-user-123', + ) + + # Assert + mock_logger.info.assert_called_once() + call_kwargs = mock_logger.info.call_args + assert call_kwargs[1]['extra']['user_id'] == 'keycloak-user-123' + assert call_kwargs[1]['extra']['email'] == 'test@example.com' + + def test_should_log_none_for_user_id_and_email_when_not_provided( + self, recaptcha_service, mock_gcp_client + ): + """Test that user_id and email are logged as None when not provided.""" + # Arrange + mock_response = MagicMock() + mock_response.name = 'projects/test-project/assessments/abc456' + mock_response.token_properties.valid = True + mock_response.token_properties.action = 'LOGIN' + mock_response.risk_analysis.score = 0.9 + mock_response.risk_analysis.reasons = [] + mock_gcp_client.create_assessment.return_value = mock_response + + with patch('server.auth.recaptcha_service.logger') as mock_logger: + # Act + recaptcha_service.create_assessment( + token='test-token', + action='LOGIN', + user_ip='192.168.1.1', + user_agent='Mozilla/5.0', + ) + + # Assert + mock_logger.info.assert_called_once() + call_kwargs = mock_logger.info.call_args + assert call_kwargs[1]['extra']['user_id'] is None + assert call_kwargs[1]['extra']['email'] is None + def test_should_raise_exception_when_gcp_client_fails( self, recaptcha_service, mock_gcp_client ):