diff --git a/CHANGELOG b/CHANGELOG index 205d47de13c..94705b38a79 100644 --- a/CHANGELOG +++ b/CHANGELOG @@ -2,6 +2,11 @@ We follow the CalVer (https://calver.org/) versioning scheme: YY.MINOR.MICRO. +24.08.0 (2024-10-30) +==================== + +- Fix admin confirmation link generation and handling + 24.07.0 (2024-09-19) ==================== diff --git a/admin/users/views.py b/admin/users/views.py index 69bfa821c5c..1e6d6e3b09a 100644 --- a/admin/users/views.py +++ b/admin/users/views.py @@ -16,6 +16,7 @@ from django.core.mail import send_mail from django.shortcuts import redirect from django.core.paginator import Paginator +from django.core.exceptions import ValidationError from osf.exceptions import UserStateError from osf.models.base import Guid @@ -456,10 +457,19 @@ def get_context_data(self, **kwargs): class GetUserConfirmationLink(GetUserLink): def get_link(self, user): + if user.is_confirmed: + return f'User {user._id} is already confirmed' + + if user.deleted or user.is_merged: + return f'User {user._id} is deleted or merged' + try: - return user.get_confirmation_url(user.username, force=True) - except KeyError as e: - return str(e) + confirmation_link = user.get_or_create_confirmation_url(user.username, force=True, renew=True) + return confirmation_link + except ValidationError: + return f'Invalid email for user {user._id}' + except KeyError: + return 'Could not generate or refresh confirmation link' def get_link_type(self): return 'User Confirmation' diff --git a/admin_tests/users/test_views.py b/admin_tests/users/test_views.py index 80da9721651..cd51459e134 100644 --- a/admin_tests/users/test_views.py +++ b/admin_tests/users/test_views.py @@ -486,10 +486,15 @@ def test_get_user_confirmation_link(self): view = views.GetUserConfirmationLink() view = setup_view(view, request, guid=user._id) + link = view.get_link(user) + + user.refresh_from_db() + user_token = list(user.email_verifications.keys())[0] + ideal_link_path = f'/confirm/{user._id}/{user_token}/' - link = view.get_link(user) - link_path = str(furl(link).path) + + link_path = str(furl(link).path).rstrip('/') + '/' assert link_path == ideal_link_path @@ -511,6 +516,45 @@ def test_get_user_confirmation_link_with_expired_token(self): assert link_path == ideal_link_path + def test_get_user_confirmation_link_generates_new_token_if_expired(self): + user = UnconfirmedUserFactory() + request = RequestFactory().get('/fake_path') + view = views.GetUserConfirmationLink() + view = setup_view(view, request, guid=user._id) + + old_user_token = list(user.email_verifications.keys())[0] + user.email_verifications[old_user_token]['expiration'] = datetime.utcnow().replace(tzinfo=pytz.utc) - timedelta(hours=24) + user.save() + + link = view.get_link(user) + user.refresh_from_db() + + new_user_token = list(user.email_verifications.keys())[0] + + assert new_user_token != old_user_token + + link_path = str(furl(link).path) + ideal_link_path = f'/confirm/{user._id}/{new_user_token}/' + assert link_path == ideal_link_path + + def test_get_user_confirmation_link_does_not_change_unexpired_token(self): + user = UnconfirmedUserFactory() + request = RequestFactory().get('/fake_path') + view = views.GetUserConfirmationLink() + view = setup_view(view, request, guid=user._id) + + user_token_before = list(user.email_verifications.keys())[0] + + user.email_verifications[user_token_before]['expiration'] = datetime.utcnow().replace(tzinfo=pytz.utc) + timedelta(hours=24) + user.save() + + with mock.patch('osf.models.user.OSFUser.get_or_create_confirmation_url') as mock_method: + mock_method.return_value = user.get_confirmation_url(user.username, force=False, renew=False) + + user_token_after = list(user.email_verifications.keys())[0] + + assert user_token_before == user_token_after + def test_get_password_reset_link(self): user = UnconfirmedUserFactory() request = RequestFactory().get('/fake_path') diff --git a/osf/models/user.py b/osf/models/user.py index 29e10efa991..d0783c208aa 100644 --- a/osf/models/user.py +++ b/osf/models/user.py @@ -1342,6 +1342,23 @@ def get_confirmation_url(self, email, destination = '?{}'.format(urlencode({'destination': destination})) if destination else '' return f'{base}confirm/{external}{self._primary_key}/{token}/{destination}' + def get_or_create_confirmation_url(self, email, force=False, renew=False): + """ + Get or create a confirmation URL for the given email. + + :param email: The email to generate a confirmation URL for. + :param force: Force generating a new confirmation link. + :param renew: Renew an expired token. + :raises ValidationError: If email is invalid or domain is banned. + :return: Confirmation URL for the email. + """ + try: + self.get_confirmation_token(email, force=force, renew=renew) + except KeyError: + self.add_unconfirmed_email(email) + self.save() + return self.get_confirmation_url(email) + def register(self, username, password=None, accepted_terms_of_service=None): """Registers the user. """ diff --git a/package.json b/package.json index be5c3b44a30..8b0edd12961 100644 --- a/package.json +++ b/package.json @@ -1,6 +1,6 @@ { "name": "OSF", - "version": "24.07.0", + "version": "24.08.0", "description": "Facilitating Open Science", "repository": "https://github.com/CenterForOpenScience/osf.io", "author": "Center for Open Science",