Skip to content

Commit 3a25592

Browse files
authored
[ENG-8691] Wrong server on reset password email (#11271)
* use same logic for admin and web password reset * fix tests
1 parent 2d54110 commit 3a25592

File tree

4 files changed

+49
-37
lines changed

4 files changed

+49
-37
lines changed

admin/users/views.py

Lines changed: 4 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -12,7 +12,6 @@
1212
from django.contrib.auth.mixins import PermissionRequiredMixin
1313
from django.urls import reverse
1414
from django.core.exceptions import PermissionDenied
15-
from django.core.mail import send_mail
1615
from django.shortcuts import redirect
1716
from django.core.paginator import Paginator
1817
from django.core.exceptions import ValidationError
@@ -47,7 +46,8 @@
4746
AddSystemTagForm
4847
)
4948
from admin.base.views import GuidView
50-
from website.settings import DOMAIN, OSF_SUPPORT_EMAIL
49+
from api.users.services import send_password_reset_email
50+
from website.settings import DOMAIN
5151
from django.urls import reverse_lazy
5252

5353

@@ -523,17 +523,9 @@ class ResetPasswordView(UserMixin, View):
523523
def post(self, request, *args, **kwargs):
524524
email = self.request.POST['emails']
525525
user = get_user(email)
526-
url = furl(DOMAIN)
527526

528-
user.verification_key_v2 = generate_verification_key(verification_type='password_admin')
529-
user.save()
530-
url.add(path=f'resetpassword/{user._id}/{user.verification_key_v2["token"]}')
531-
send_mail(
532-
subject='Reset OSF Password',
533-
message=f'Follow this link to reset your password: {url.url}\n Note: this link will expire in 12 hours',
534-
from_email=OSF_SUPPORT_EMAIL,
535-
recipient_list=[email]
536-
)
527+
send_password_reset_email(user, email, institutional=False, verification_type='password_admin')
528+
537529
update_admin_log(
538530
user_id=self.request.user.id,
539531
object_id=user.pk,

api/users/services.py

Lines changed: 27 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,27 @@
1+
from furl import furl
2+
from django.utils import timezone
3+
4+
from framework.auth.core import generate_verification_key
5+
from website import settings, mails
6+
7+
8+
def send_password_reset_email(user, email, verification_type='password', institutional=False, **mail_kwargs):
9+
"""Generate a password reset token, save it to the user and send the password reset email.
10+
"""
11+
# new verification key (v2)
12+
user.verification_key_v2 = generate_verification_key(verification_type=verification_type)
13+
user.email_last_sent = timezone.now()
14+
user.save()
15+
16+
reset_link = furl(settings.DOMAIN).add(path=f'resetpassword/{user._id}/{user.verification_key_v2["token"]}').url
17+
mail_template = mails.FORGOT_PASSWORD if not institutional else mails.FORGOT_PASSWORD_INSTITUTION
18+
19+
mails.send_mail(
20+
to_addr=email,
21+
mail=mail_template,
22+
reset_link=reset_link,
23+
can_change_preferences=False,
24+
**mail_kwargs,
25+
)
26+
27+
return reset_link

api/users/views.py

Lines changed: 17 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -47,6 +47,7 @@
4747
from api.registrations.serializers import RegistrationSerializer
4848
from api.resources import annotations as resource_annotations
4949

50+
from api.users.services import send_password_reset_email
5051
from api.users.permissions import (
5152
CurrentUser, ReadOnlyOrCurrentUser,
5253
ReadOnlyOrCurrentUserRelationship,
@@ -864,38 +865,30 @@ class ResetPassword(JSONAPIBaseView, generics.ListCreateAPIView):
864865
throttle_classes = (NonCookieAuthThrottle, BurstRateThrottle, RootAnonThrottle, SendEmailThrottle)
865866

866867
def get(self, request, *args, **kwargs):
868+
institutional = bool(request.query_params.get('institutional', None))
867869
email = request.query_params.get('email', None)
868870
if not email:
869871
raise ValidationError('Request must include email in query params.')
870872

871-
institutional = bool(request.query_params.get('institutional', None))
872-
mail_template = mails.FORGOT_PASSWORD if not institutional else mails.FORGOT_PASSWORD_INSTITUTION
873-
874-
status_message = language.RESET_PASSWORD_SUCCESS_STATUS_MESSAGE.format(email=email)
875-
kind = 'success'
876873
# check if the user exists
877874
user_obj = get_user(email=email)
878-
879-
if user_obj:
875+
if user_obj and user_obj.is_active:
880876
# rate limit forgot_password_post
881877
if not throttle_period_expired(user_obj.email_last_sent, settings.SEND_EMAIL_THROTTLE):
882-
status_message = 'You have recently requested to change your password. Please wait a few minutes ' \
883-
'before trying again.'
884-
kind = 'error'
885-
return Response({'message': status_message, 'kind': kind}, status=status.HTTP_429_TOO_MANY_REQUESTS)
886-
elif user_obj.is_active:
887-
# new random verification key (v2)
888-
user_obj.verification_key_v2 = generate_verification_key(verification_type='password')
889-
user_obj.email_last_sent = timezone.now()
890-
user_obj.save()
891-
reset_link = f'{settings.RESET_PASSWORD_URL}{user_obj._id}/{user_obj.verification_key_v2['token']}/'
892-
mails.send_mail(
893-
to_addr=email,
894-
mail=mail_template,
895-
reset_link=reset_link,
896-
can_change_preferences=False,
897-
)
898-
return Response(status=status.HTTP_200_OK, data={'message': status_message, 'kind': kind, 'institutional': institutional})
878+
status_message = 'You have recently requested to change your password. ' \
879+
'Please wait a few minutes before trying again.'
880+
return Response({'message': status_message, 'kind': 'error'}, status=status.HTTP_429_TOO_MANY_REQUESTS)
881+
882+
send_password_reset_email(user_obj, email, institutional=institutional)
883+
884+
return Response(
885+
status=status.HTTP_200_OK,
886+
data={
887+
'message': language.RESET_PASSWORD_SUCCESS_STATUS_MESSAGE.format(email=email),
888+
'kind': 'success',
889+
'institutional': institutional,
890+
},
891+
)
899892

900893
def post(self, request, *args, **kwargs):
901894
serializer = self.get_serializer(data=request.data)

api_tests/users/views/test_user_settings.py

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -205,7 +205,7 @@ def test_get(self, app, url, user_one):
205205
mock_send_mail.assert_called_with(
206206
to_addr=user_one.username,
207207
mail=mails.FORGOT_PASSWORD,
208-
reset_link=f'{settings.RESET_PASSWORD_URL}{user_one._id}/{user_one.verification_key_v2['token']}/',
208+
reset_link=f'{settings.DOMAIN}resetpassword/{user_one._id}/{user_one.verification_key_v2['token']}',
209209
can_change_preferences=False,
210210
)
211211

0 commit comments

Comments
 (0)