diff --git a/openedx/core/djangoapps/user_api/management/commands/bulk_rehash_retired_usernames.py b/openedx/core/djangoapps/user_api/management/commands/bulk_rehash_retired_usernames.py new file mode 100644 index 0000000000000000000000000000000000000000..cc1ecf1653a47685dcad053efcd0e05406403c5e --- /dev/null +++ b/openedx/core/djangoapps/user_api/management/commands/bulk_rehash_retired_usernames.py @@ -0,0 +1,117 @@ +""" +One-off script to rehash all retired usernames in UserRetirementStatus and auth_user. +Background: We discovered that all prior retired usernames were generated based on +the exact capitalization of the original username, despite the fact that +usernames are considered case insensitive in practice. This led to the +possibility of users registering accounts with effectively retired usernames just +by changing the capitalization of the username, because the different +capitalization would hash to a different digest. +Solution: Rehash all usernames using the normalized-case (lowercase) +original usernames rather than the possibly mixed-case ones. This management +command likely cannot be re-used in the future because eventually we will need +to clean out the UserRetirementStatus table. +""" +from __future__ import print_function + +from django.conf import settings +from django.db import transaction +from django.core.management.base import BaseCommand +from six import text_type + +from lms.lib import comment_client +from openedx.core.djangoapps.user_api.models import UserRetirementStatus +from user_util import user_util + + +class Command(BaseCommand): + """ + Implementation of the bulk_rehash_retired_usernames command. + """ + def add_arguments(self, parser): + parser.add_argument( + '--dry_run', + action='store_true', + help='Print proposed changes, but take no action.' + ) + + def handle(self, *args, **options): + """ + Execute the command. + """ + dry_run = options['dry_run'] + retirements = UserRetirementStatus.objects.all().select_related('user') + + failed_retirements = [] + for retirement in retirements: + original_username = retirement.original_username + old_retired_username = retirement.retired_username + new_retired_username = user_util.get_retired_username( + original_username, + settings.RETIRED_USER_SALTS, + settings.RETIRED_USERNAME_FMT + ) + # Sanity check: + if retirement.user.username != old_retired_username: + print( + 'WARNING: Skipping UserRetirementStatus ID {} / User ID {} because the user does not appear to ' + 'have a retired username: {} != {}.'.format( + retirement.id, + retirement.user.id, + retirement.user.username, + old_retired_username + ) + ) + # If the original username was already normalized (or all lowercase), the old and new hashes would + # match: + elif old_retired_username == new_retired_username: + print( + 'Skipping UserRetirementStatus ID {} / User ID {} because the hash would not change.'.format( + retirement.id, + retirement.user.id, + ) + ) + # Found an username to update + else: + print( + 'Updating UserRetirementStatus ID {} / User ID {} ' + 'to rehash their retired username: {} -> {}'.format( + retirement.id, + retirement.user.id, + old_retired_username, + new_retired_username + ) + ) + if not dry_run: + try: + # Update the forums first, that way if it fails the user can + # be re-run. It does not need to be in the same transaction, + # as the local db updates and can be slow, so keeping it + # outside to cut down on potential deadlocks. + cc_user = comment_client.User.from_django_user(retirement.user) + cc_user.retire(new_retired_username) + + # Update and save both the user table and retirement queue table: + with transaction.atomic(): + retirement.user.username = new_retired_username + retirement.user.save() + retirement.retired_username = new_retired_username + retirement.save() + except Exception as exc: # pylint: disable=broad-except + print( + 'UserRetirementStatus ID {} User ID {} failed rename'.format( + retirement.id, retirement.user.id + ) + ) + print(text_type(exc)) + failed_retirements.append(retirement) + + if failed_retirements: + print('------------------------------------------------------------') + print( + 'FAILED! {} retirements failed to rehash. Retirement IDs:\n{}'.format( + len(failed_retirements), + '\n'.join([text_type(r.id) for r in failed_retirements]) + ) + ) + else: + print('Success! {} retirements examined.'.format(len(retirements))) diff --git a/openedx/core/djangoapps/user_api/management/tests/test_bulk_rehash_retired_usernames.py b/openedx/core/djangoapps/user_api/management/tests/test_bulk_rehash_retired_usernames.py new file mode 100644 index 0000000000000000000000000000000000000000..6164a7f25c955c3743ecfb0e7938aa91aa0b66a4 --- /dev/null +++ b/openedx/core/djangoapps/user_api/management/tests/test_bulk_rehash_retired_usernames.py @@ -0,0 +1,140 @@ +""" +Test the bulk_rehash_retired_usernames management command +""" +from mock import call, patch +import pytest + +from django.conf import settings +from django.core.management import call_command +from user_util.user_util import get_retired_username + +from openedx.core.djangoapps.user_api.accounts.tests.retirement_helpers import RetirementTestCase, fake_retirement +from openedx.core.djangoapps.user_api.models import UserRetirementStatus +from openedx.core.djangolib.testing.utils import skip_unless_lms +from student.tests.factories import UserFactory + +pytestmark = pytest.mark.django_db + + +def _setup_users(): + """ + Creates and returns test users in the different states of needing rehash: + - Skipped: has not yet been retired + - Faked: has been fake-retired, but the retired username does not require updating + - Needing rehash: has been fake-retired and name changed so it triggers a hash update + """ + # When we loop through creating users, take additional action on these + user_indexes_to_be_fake_retired = (2, 4, 6, 8, 10) + user_indexes_to_be_rehashed = (4, 6) + + users_skipped = [] + users_faked = [] + users_needing_rehash = [] + retirements = {} + + # Create some test users with retirements + for i in range(1, 11): + user = UserFactory() + retirement = UserRetirementStatus.create_retirement(user) + retirements[user.id] = retirement + + if i in user_indexes_to_be_fake_retired: + fake_retirement(user) + + if i in user_indexes_to_be_rehashed: + # In order to need a rehash user.username must be the same as + # retirement.retired_username and NOT the same as the hash + # generated when the script is run. So we force that here. + retirement.retired_username = retirement.retired_username.upper() + user.username = retirement.retired_username + retirement.save() + user.save() + users_needing_rehash.append(user) + else: + users_faked.append(user) + else: + users_skipped.append(user) + return users_skipped, users_faked, users_needing_rehash, retirements + + +@skip_unless_lms +@patch('lms.lib.comment_client.User.retire') +def test_successful_rehash(retire_user_forums, capsys): + """ + Run the command with users of all different hash statuses, expect success + """ + RetirementTestCase.setup_states() + users_skipped, users_faked, users_needing_rehash, retirements = _setup_users() + + call_command('bulk_rehash_retired_usernames') + output = capsys.readouterr().out + + # Make sure forums was called the correct number of times + assert retire_user_forums.call_count == 2 + + for user in users_skipped: + assert "User ID {} because the user does not appear to have a retired username:".format(user.id) in output + + for user in users_faked: + assert "User ID {} because the hash would not change.".format(user.id) in output + + expected_username_calls = [] + for user in users_needing_rehash: + retirement = retirements[user.id] + user.refresh_from_db() + retirement.refresh_from_db() + new_retired_username = get_retired_username( + retirement.original_username, + settings.RETIRED_USER_SALTS, + settings.RETIRED_USERNAME_FMT + ) + expected_username_calls.append(call(new_retired_username)) + + assert "User ID {} to rehash their retired username".format(user.id) in output + assert new_retired_username == user.username + assert new_retired_username == retirement.retired_username + + retire_user_forums.assert_has_calls(expected_username_calls) + + +@skip_unless_lms +@patch('lms.lib.comment_client.User.retire') +def test_forums_failed(retire_user_forums, capsys): + """ + Run the command with users of all different hash statuses, expect success + """ + RetirementTestCase.setup_states() + users_skipped, users_faked, users_needing_rehash, retirements = _setup_users() + retire_user_forums.side_effect = Exception('something bad happened with forums') + + call_command('bulk_rehash_retired_usernames') + output = capsys.readouterr().out + + # Make sure forums was called the correct number of times + assert retire_user_forums.call_count == 2 + + for user in users_skipped: + assert "User ID {} because the user does not appear to have a retired username:".format(user.id) in output + + for user in users_faked: + assert "User ID {} because the hash would not change.".format(user.id) in output + + expected_username_calls = [] + for user in users_needing_rehash: + retirement = retirements[user.id] + user.refresh_from_db() + retirement.refresh_from_db() + new_retired_username = get_retired_username( + retirement.original_username, + settings.RETIRED_USER_SALTS, + settings.RETIRED_USERNAME_FMT + ) + expected_username_calls.append(call(new_retired_username)) + + assert "User ID {} to rehash their retired username".format(user.id) in output + # Confirm that the usernames are *not* updated, due to the forums error + assert new_retired_username != user.username + assert new_retired_username != retirement.retired_username + + assert "FAILED! 2 retirements failed to rehash. Retirement IDs:" in output + retire_user_forums.assert_has_calls(expected_username_calls)