Skip to content

Commit ff60361

Browse files
rollback cognito if dynamo fails to update user record
1 parent 6f08651 commit ff60361

File tree

2 files changed

+123
-6
lines changed

2 files changed

+123
-6
lines changed

backend/compact-connect/lambdas/python/provider-data-v1/handlers/provider_users.py

Lines changed: 43 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -381,11 +381,48 @@ def _post_provider_email_verify(event: dict, context: LambdaContext): # noqa: A
381381
raise
382382

383383
# Update the provider record with new email and clear verification data
384-
config.data_client.complete_provider_email_update(
385-
compact=compact,
386-
provider_id=provider_id,
387-
new_email_address=new_email,
388-
)
384+
try:
385+
config.data_client.complete_provider_email_update(
386+
compact=compact,
387+
provider_id=provider_id,
388+
new_email_address=new_email,
389+
)
390+
except Exception as dynamo_error:
391+
# If DynamoDB update fails, we need to roll back the Cognito change
392+
logger.error(
393+
'DynamoDB update failed after Cognito update succeeded, rolling back Cognito changes',
394+
compact=compact,
395+
provider_id=provider_id,
396+
error=str(dynamo_error),
397+
)
398+
try:
399+
# Roll back the Cognito email change
400+
config.cognito_client.admin_update_user_attributes(
401+
UserPoolId=config.provider_user_pool_id,
402+
Username=new_email, # Current username is now the new email
403+
UserAttributes=[
404+
{'Name': 'email', 'Value': current_email},
405+
{'Name': 'email_verified', 'Value': 'true'},
406+
],
407+
)
408+
logger.info(
409+
'Successfully rolled back Cognito email change',
410+
compact=compact,
411+
provider_id=provider_id,
412+
)
413+
except ClientError as rollback_error:
414+
logger.error(
415+
'Failed to roll back Cognito email change - user may be in inconsistent state',
416+
compact=compact,
417+
provider_id=provider_id,
418+
original_error=str(dynamo_error),
419+
rollback_error=str(rollback_error.response),
420+
)
421+
422+
# Raise internal exception to trigger alert
423+
raise CCInternalException(
424+
'Failed to complete email update - system may be in inconsistent state'
425+
) from dynamo_error
389426

390427
# Send notification to old email address
391428
try:
@@ -417,4 +454,4 @@ def _post_provider_email_verify(event: dict, context: LambdaContext): # noqa: A
417454
provider_id=provider_id,
418455
error=str(e),
419456
)
420-
raise CCInternalException('Failed to verify email address') from e
457+
raise CCInternalException('Failed to complete email update. Please try again later.') from e

backend/compact-connect/lambdas/python/provider-data-v1/tests/function/test_handlers/test_provider_users_email.py

Lines changed: 80 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,7 @@
44
from datetime import datetime
55
from unittest.mock import patch
66

7+
from cc_common.exceptions import CCInternalException
78
from common_test.test_constants import DEFAULT_COMPACT, DEFAULT_PROVIDER_ID
89
from moto import mock_aws
910

@@ -562,3 +563,82 @@ def test_endpoint_returns_400_if_new_email_becomes_unavailable_during_verificati
562563

563564
# Original email should remain unchanged
564565
self.assertEqual(TEST_OLD_EMAIL, stored_provider_data.compactConnectRegisteredEmailAddress)
566+
567+
568+
@mock_aws
569+
@patch('cc_common.config._Config.current_standard_datetime', datetime.fromisoformat('2024-11-08T23:59:59+00:00'))
570+
class TestPostProviderUsersEmailVerifyRollback(TstFunction):
571+
"""
572+
Separate test class for rollback functionality to avoid cached property issues with data_client mocking.
573+
"""
574+
575+
def _when_testing_provider_user_event_with_custom_claims(
576+
self, code=TEST_VERIFICATION_CODE, expiration=TEST_TOKEN_EXPIRATION
577+
):
578+
self.test_data_generator.put_default_provider_record_in_provider_table(
579+
value_overrides={
580+
'compactConnectRegisteredEmailAddress': TEST_OLD_EMAIL,
581+
'pendingEmailAddress': TEST_NEW_EMAIL,
582+
'emailVerificationCode': '1234',
583+
'emailVerificationExpiry': expiration,
584+
}
585+
)
586+
587+
event = self.test_data_generator.generate_test_api_event()
588+
event['httpMethod'] = 'POST'
589+
event['resource'] = '/v1/provider-users/me/email/verify'
590+
event['requestContext']['authorizer']['claims']['custom:providerId'] = DEFAULT_PROVIDER_ID
591+
event['requestContext']['authorizer']['claims']['custom:compact'] = DEFAULT_COMPACT
592+
event['body'] = json.dumps({'verificationCode': code})
593+
594+
return event
595+
596+
@patch('cc_common.config._Config.email_service_client')
597+
@patch('handlers.provider_users.config.data_client.complete_provider_email_update')
598+
def test_endpoint_rolls_back_cognito_change_when_dynamo_update_fails(
599+
self, mock_complete_email_update, mock_email_service_client
600+
):
601+
from handlers.provider_users import provider_users_api_handler
602+
603+
# First create the old email user in Cognito so we can update it
604+
self.config.cognito_client.admin_create_user(
605+
UserPoolId=self.config.provider_user_pool_id,
606+
Username=TEST_OLD_EMAIL,
607+
UserAttributes=[
608+
{'Name': 'email', 'Value': TEST_OLD_EMAIL},
609+
{'Name': 'email_verified', 'Value': 'true'},
610+
{'Name': 'custom:compact', 'Value': DEFAULT_COMPACT},
611+
{'Name': 'custom:providerId', 'Value': DEFAULT_PROVIDER_ID},
612+
],
613+
MessageAction='SUPPRESS',
614+
)
615+
616+
# Mock the DynamoDB update method to fail
617+
mock_complete_email_update.side_effect = Exception('DynamoDB connection error')
618+
619+
event = self._when_testing_provider_user_event_with_custom_claims()
620+
621+
# Should raise CCInternalException to trigger an alert
622+
with self.assertRaises(CCInternalException) as context:
623+
provider_users_api_handler(event, self.mock_context)
624+
625+
# Verify the exception message
626+
self.assertEqual('Failed to complete email update. Please try again later.', str(context.exception))
627+
628+
# Verify the Cognito user's email was rolled back to the original email
629+
cognito_users = self.config.cognito_client.list_users(
630+
UserPoolId=self.config.provider_user_pool_id, Filter=f'email = "{TEST_OLD_EMAIL}"'
631+
)
632+
633+
self.assertEqual(1, len(cognito_users['Users']))
634+
user_attributes = {attr['Name']: attr['Value'] for attr in cognito_users['Users'][0]['Attributes']}
635+
636+
# Verify the email was rolled back
637+
self.assertEqual(TEST_OLD_EMAIL, user_attributes['email'])
638+
self.assertEqual('true', user_attributes['email_verified'])
639+
640+
# Verify no user exists with the new email address (rollback succeeded)
641+
new_email_users = self.config.cognito_client.list_users(
642+
UserPoolId=self.config.provider_user_pool_id, Filter=f'email = "{TEST_NEW_EMAIL}"'
643+
)
644+
self.assertEqual(0, len(new_email_users['Users']))

0 commit comments

Comments
 (0)