Skip to content

Commit 9394a0f

Browse files
committed
Merge tag '24.08.0' into develop
[ENG-6195] Fix admin confirmation link generation and handling #10734
2 parents 6eebef9 + 4c05c22 commit 9394a0f

File tree

5 files changed

+82
-6
lines changed

5 files changed

+82
-6
lines changed

CHANGELOG

+5
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,11 @@
22

33
We follow the CalVer (https://calver.org/) versioning scheme: YY.MINOR.MICRO.
44

5+
24.08.0 (2024-10-30)
6+
====================
7+
8+
- Fix admin confirmation link generation and handling
9+
510
24.07.0 (2024-09-19)
611
====================
712

admin/users/views.py

+13-3
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,7 @@
1616
from django.core.mail import send_mail
1717
from django.shortcuts import redirect
1818
from django.core.paginator import Paginator
19+
from django.core.exceptions import ValidationError
1920

2021
from osf.exceptions import UserStateError
2122
from osf.models.base import Guid
@@ -456,10 +457,19 @@ def get_context_data(self, **kwargs):
456457

457458
class GetUserConfirmationLink(GetUserLink):
458459
def get_link(self, user):
460+
if user.is_confirmed:
461+
return f'User {user._id} is already confirmed'
462+
463+
if user.deleted or user.is_merged:
464+
return f'User {user._id} is deleted or merged'
465+
459466
try:
460-
return user.get_confirmation_url(user.username, force=True)
461-
except KeyError as e:
462-
return str(e)
467+
confirmation_link = user.get_or_create_confirmation_url(user.username, force=True, renew=True)
468+
return confirmation_link
469+
except ValidationError:
470+
return f'Invalid email for user {user._id}'
471+
except KeyError:
472+
return 'Could not generate or refresh confirmation link'
463473

464474
def get_link_type(self):
465475
return 'User Confirmation'

admin_tests/users/test_views.py

+46-2
Original file line numberDiff line numberDiff line change
@@ -486,10 +486,15 @@ def test_get_user_confirmation_link(self):
486486
view = views.GetUserConfirmationLink()
487487
view = setup_view(view, request, guid=user._id)
488488

489+
link = view.get_link(user)
490+
491+
user.refresh_from_db()
492+
489493
user_token = list(user.email_verifications.keys())[0]
494+
490495
ideal_link_path = f'/confirm/{user._id}/{user_token}/'
491-
link = view.get_link(user)
492-
link_path = str(furl(link).path)
496+
497+
link_path = str(furl(link).path).rstrip('/') + '/'
493498

494499
assert link_path == ideal_link_path
495500

@@ -511,6 +516,45 @@ def test_get_user_confirmation_link_with_expired_token(self):
511516

512517
assert link_path == ideal_link_path
513518

519+
def test_get_user_confirmation_link_generates_new_token_if_expired(self):
520+
user = UnconfirmedUserFactory()
521+
request = RequestFactory().get('/fake_path')
522+
view = views.GetUserConfirmationLink()
523+
view = setup_view(view, request, guid=user._id)
524+
525+
old_user_token = list(user.email_verifications.keys())[0]
526+
user.email_verifications[old_user_token]['expiration'] = datetime.utcnow().replace(tzinfo=pytz.utc) - timedelta(hours=24)
527+
user.save()
528+
529+
link = view.get_link(user)
530+
user.refresh_from_db()
531+
532+
new_user_token = list(user.email_verifications.keys())[0]
533+
534+
assert new_user_token != old_user_token
535+
536+
link_path = str(furl(link).path)
537+
ideal_link_path = f'/confirm/{user._id}/{new_user_token}/'
538+
assert link_path == ideal_link_path
539+
540+
def test_get_user_confirmation_link_does_not_change_unexpired_token(self):
541+
user = UnconfirmedUserFactory()
542+
request = RequestFactory().get('/fake_path')
543+
view = views.GetUserConfirmationLink()
544+
view = setup_view(view, request, guid=user._id)
545+
546+
user_token_before = list(user.email_verifications.keys())[0]
547+
548+
user.email_verifications[user_token_before]['expiration'] = datetime.utcnow().replace(tzinfo=pytz.utc) + timedelta(hours=24)
549+
user.save()
550+
551+
with mock.patch('osf.models.user.OSFUser.get_or_create_confirmation_url') as mock_method:
552+
mock_method.return_value = user.get_confirmation_url(user.username, force=False, renew=False)
553+
554+
user_token_after = list(user.email_verifications.keys())[0]
555+
556+
assert user_token_before == user_token_after
557+
514558
def test_get_password_reset_link(self):
515559
user = UnconfirmedUserFactory()
516560
request = RequestFactory().get('/fake_path')

osf/models/user.py

+17
Original file line numberDiff line numberDiff line change
@@ -1342,6 +1342,23 @@ def get_confirmation_url(self, email,
13421342
destination = '?{}'.format(urlencode({'destination': destination})) if destination else ''
13431343
return f'{base}confirm/{external}{self._primary_key}/{token}/{destination}'
13441344

1345+
def get_or_create_confirmation_url(self, email, force=False, renew=False):
1346+
"""
1347+
Get or create a confirmation URL for the given email.
1348+
1349+
:param email: The email to generate a confirmation URL for.
1350+
:param force: Force generating a new confirmation link.
1351+
:param renew: Renew an expired token.
1352+
:raises ValidationError: If email is invalid or domain is banned.
1353+
:return: Confirmation URL for the email.
1354+
"""
1355+
try:
1356+
self.get_confirmation_token(email, force=force, renew=renew)
1357+
except KeyError:
1358+
self.add_unconfirmed_email(email)
1359+
self.save()
1360+
return self.get_confirmation_url(email)
1361+
13451362
def register(self, username, password=None, accepted_terms_of_service=None):
13461363
"""Registers the user.
13471364
"""

package.json

+1-1
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
11
{
22
"name": "OSF",
3-
"version": "24.07.0",
3+
"version": "24.08.0",
44
"description": "Facilitating Open Science",
55
"repository": "https://github.com/CenterForOpenScience/osf.io",
66
"author": "Center for Open Science",

0 commit comments

Comments
 (0)