Skip to content

Commit 0343e5f

Browse files
committed
upgrade user confirmation to v2
1 parent 9f366cd commit 0343e5f

File tree

5 files changed

+299
-5
lines changed

5 files changed

+299
-5
lines changed

api/users/serializers.py

+1-1
Original file line numberDiff line numberDiff line change
@@ -442,7 +442,7 @@ class Meta:
442442
type_ = 'user_reset_password'
443443

444444

445-
class ExternalLoginConfirmEmailSerializer(BaseAPISerializer):
445+
class ConfirmEmailSerializer(BaseAPISerializer):
446446
uid = ser.CharField(write_only=True, required=True)
447447
destination = ser.CharField(write_only=True, required=True)
448448
token = ser.CharField(write_only=True, required=True)

api/users/urls.py

+1
Original file line numberDiff line numberDiff line change
@@ -14,6 +14,7 @@
1414
re_path(r'^(?P<user_id>\w+)/addons/(?P<provider>\w+)/accounts/$', views.UserAddonAccountList.as_view(), name=views.UserAddonAccountList.view_name),
1515
re_path(r'^(?P<user_id>\w+)/addons/(?P<provider>\w+)/accounts/(?P<account_id>\w+)/$', views.UserAddonAccountDetail.as_view(), name=views.UserAddonAccountDetail.view_name),
1616
re_path(r'^(?P<user_id>\w+)/claim/$', views.ClaimUser.as_view(), name=views.ClaimUser.view_name),
17+
re_path(r'^(?P<user_id>\w+)/confirm/$', views.ConfirmEmailView.as_view(), name=views.ConfirmEmailView.view_name),
1718
re_path(r'^(?P<user_id>\w+)/draft_registrations/$', views.UserDraftRegistrations.as_view(), name=views.UserDraftRegistrations.view_name),
1819
re_path(r'^(?P<user_id>\w+)/institutions/$', views.UserInstitutions.as_view(), name=views.UserInstitutions.view_name),
1920
re_path(r'^(?P<user_id>\w+)/nodes/$', views.UserNodes.as_view(), name=views.UserNodes.view_name),

api/users/views.py

+89-3
Original file line numberDiff line numberDiff line change
@@ -68,7 +68,7 @@
6868
UserChangePasswordSerializer,
6969
UserMessageSerializer,
7070
ExternalLoginSerialiser,
71-
ExternalLoginConfirmEmailSerializer,
71+
ConfirmEmailSerializer,
7272
)
7373
from django.contrib.auth.models import AnonymousUser
7474
from django.http import JsonResponse
@@ -80,6 +80,7 @@
8080
from framework.auth.oauth_scopes import CoreScopes, normalize_scopes
8181
from framework.auth.exceptions import ChangePasswordError
8282
from framework.celery_tasks.handlers import enqueue_task
83+
from django.shortcuts import redirect
8384
from framework.utils import throttle_period_expired
8485
from framework.sessions.utils import remove_sessions_for_user
8586
from framework.exceptions import PermissionsError, HTTPError
@@ -105,7 +106,7 @@
105106
from website import mails, settings, language
106107
from website.project.views.contributor import send_claim_email, send_claim_registered_email
107108
from website.util.metrics import CampaignClaimedTags, CampaignSourceTags
108-
from framework.auth import exceptions
109+
from framework.auth import exceptions, cas
109110

110111

111112
class UserMixin:
@@ -1060,6 +1061,91 @@ def post(self, request, *args, **kwargs):
10601061
return Response(status=status.HTTP_204_NO_CONTENT)
10611062

10621063

1064+
class ConfirmEmailView(generics.CreateAPIView):
1065+
"""
1066+
Confirm an e-mail address created during *first-time* OAuth login.
1067+
1068+
**URL:** POST /v2/users/<user_id>/confirm/
1069+
1070+
**Body (JSON):**
1071+
{
1072+
"uid": "<osf_user_id>",
1073+
"token": "<email_verification_token>",
1074+
"destination": "<campaign-code or relative URL>"
1075+
}
1076+
1077+
On success the endpoint redirects (HTTP 302) to CAS with a
1078+
one-time verification key, exactly like the original Flask view.
1079+
"""
1080+
permission_classes = (
1081+
base_permissions.TokenHasScope,
1082+
)
1083+
required_read_scopes = [CoreScopes.USERS_CONFIRM]
1084+
required_write_scopes = [CoreScopes.USERS_CONFIRM]
1085+
1086+
view_category = 'users'
1087+
view_name = 'confirm-user'
1088+
1089+
serializer_class = ConfirmEmailSerializer
1090+
1091+
def post(self, request, *args, **kwargs):
1092+
serializer = self.serializer_class(data=request.data)
1093+
serializer.is_valid(raise_exception=True)
1094+
1095+
uid = serializer.validated_data['uid']
1096+
token = serializer.validated_data['token']
1097+
1098+
user = OSFUser.load(uid)
1099+
if not user:
1100+
raise ValidationError('User not found.')
1101+
1102+
verification = user.email_verifications.get(token)
1103+
if not verification:
1104+
raise ValidationError('Invalid or expired token.')
1105+
1106+
provider = next(iter(verification['external_identity']))
1107+
provider_id = next(iter(verification['external_identity'][provider]))
1108+
1109+
if provider not in user.external_identity:
1110+
raise ValidationError('External-ID provider mismatch.')
1111+
1112+
external_status = user.external_identity[provider][provider_id]
1113+
ensure_external_identity_uniqueness(provider, provider_id, user)
1114+
1115+
email = verification['email']
1116+
if not user.is_registered:
1117+
user.register(email)
1118+
1119+
user.emails.get_or_create(address=email.lower())
1120+
user.external_identity[provider][provider_id] = 'VERIFIED'
1121+
user.date_last_logged_in = timezone.now()
1122+
1123+
del user.email_verifications[token]
1124+
user.verification_key = generate_verification_key()
1125+
user.save()
1126+
1127+
service_url = request.build_absolute_uri()
1128+
if external_status == 'CREATE':
1129+
service_url += '&' + urlencode({'new': 'true'})
1130+
elif external_status == 'LINK':
1131+
mails.send_mail(
1132+
user=user,
1133+
to_addr=user.username,
1134+
mail=mails.EXTERNAL_LOGIN_LINK_SUCCESS,
1135+
external_id_provider=provider,
1136+
can_change_preferences=False,
1137+
)
1138+
1139+
enqueue_task(update_affiliation_for_orcid_sso_users.s(user._id, provider_id))
1140+
1141+
return redirect(
1142+
cas.get_login_url(
1143+
service_url,
1144+
username=user.username,
1145+
verification_key=user.verification_key,
1146+
),
1147+
)
1148+
10631149
class UserEmailsList(JSONAPIBaseView, generics.ListAPIView, generics.CreateAPIView, UserMixin, ListFilterMixin):
10641150
permission_classes = (
10651151
drf_permissions.IsAuthenticatedOrReadOnly,
@@ -1212,7 +1298,7 @@ class ExternalLoginConfirmEmailView(generics.CreateAPIView):
12121298
permission_classes = (
12131299
drf_permissions.AllowAny,
12141300
)
1215-
serializer_class = ExternalLoginConfirmEmailSerializer
1301+
serializer_class = ConfirmEmailSerializer
12161302
view_category = 'users'
12171303
view_name = 'external-login-confirm-email'
12181304
throttle_classes = (NonCookieAuthThrottle, BurstRateThrottle, RootAnonThrottle)
+200
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,200 @@
1+
import pytest
2+
from unittest import mock
3+
4+
from api.base.settings.defaults import API_BASE
5+
from osf_tests.factories import AuthUserFactory
6+
7+
8+
@pytest.mark.django_db
9+
class TestConfirmEmail:
10+
11+
@pytest.fixture()
12+
def user(self):
13+
return AuthUserFactory()
14+
15+
@pytest.fixture()
16+
def user_with_email_verification(self, user):
17+
18+
external_identity = {
19+
'ORCID': {
20+
'0002-0001-0001-0001': 'CREATE',
21+
}
22+
}
23+
token = user.add_unconfirmed_email(email, external_identity=external_identity)
24+
user.external_identity.update(external_identity)
25+
user.save()
26+
return user, token, email
27+
28+
@pytest.fixture()
29+
def confirm_url(self, user_with_email_verification):
30+
user, _, _ = user_with_email_verification
31+
return f'/{API_BASE}users/{user._id}/confirm/'
32+
33+
def test_get_not_allowed(self, app, confirm_url):
34+
res = app.get(confirm_url, expect_errors=True)
35+
assert res.status_code == 405
36+
37+
def test_post_missing_fields(self, app, confirm_url, user_with_email_verification):
38+
user, _, _ = user_with_email_verification
39+
res = app.post_json_api(
40+
confirm_url,
41+
{'data': {'attributes': {}}},
42+
expect_errors=True,
43+
auth=user.auth
44+
)
45+
assert res.status_code == 400
46+
assert res.json['errors'] == [
47+
{
48+
'source': {
49+
'pointer': '/data/attributes/uid'
50+
},
51+
'detail': 'This field is required.'
52+
},
53+
{
54+
'source': {
55+
'pointer': '/data/attributes/destination'
56+
},
57+
'detail': 'This field is required.'
58+
},
59+
{
60+
'source': {
61+
'pointer': '/data/attributes/token'
62+
},
63+
'detail': 'This field is required.'
64+
}
65+
]
66+
67+
def test_post_user_not_found(self, app, user_with_email_verification):
68+
user, _, _ = user_with_email_verification
69+
fake_user_id = 'abcd1'
70+
res = app.post_json_api(
71+
f'/{API_BASE}users/{fake_user_id}/confirm/',
72+
{
73+
'data': {
74+
'attributes': {
75+
'uid': fake_user_id,
76+
'token': 'doesnotmatter',
77+
'destination': 'doesnotmatter',
78+
}
79+
}
80+
},
81+
expect_errors=True
82+
)
83+
assert res.status_code == 400
84+
assert 'user not found' in res.json['errors'][0]['detail'].lower()
85+
86+
def test_post_invalid_token(self, app, confirm_url, user_with_email_verification):
87+
user, _, _ = user_with_email_verification
88+
res = app.post_json_api(
89+
confirm_url,
90+
{
91+
'data': {
92+
'attributes': {
93+
'uid': user._id,
94+
'token': 'badtoken',
95+
'destination': 'doesnotmatter',
96+
}
97+
}
98+
},
99+
expect_errors=True
100+
)
101+
assert res.status_code == 400
102+
assert res.json['errors'] == [{'detail': 'Invalid or expired token.'}]
103+
104+
def test_post_provider_mismatch(self, app, confirm_url, user_with_email_verification):
105+
user, token, _ = user_with_email_verification
106+
user.external_identity = {} # clear the valid provider
107+
user.save()
108+
109+
res = app.post_json_api(
110+
confirm_url,
111+
{
112+
'data': {
113+
'attributes': {
114+
'uid': user._id,
115+
'token': token,
116+
'destination': 'doesnotmatter',
117+
}
118+
}
119+
},
120+
expect_errors=True
121+
)
122+
assert res.status_code == 400
123+
assert 'provider mismatch' in res.json['errors'][0]['detail'].lower()
124+
125+
@mock.patch('website.mails.send_mail')
126+
def test_post_success_create(self, mock_send_mail, app, confirm_url, user_with_email_verification):
127+
user, token, email = user_with_email_verification
128+
user.is_registered = False
129+
user.save()
130+
res = app.post_json_api(
131+
confirm_url,
132+
{
133+
'data': {
134+
'attributes': {
135+
'uid': user._id,
136+
'token': token,
137+
'destination': 'doesnotmatter',
138+
}
139+
}
140+
},
141+
expect_errors=True
142+
)
143+
assert res.status_code == 302
144+
import urllib.parse
145+
146+
# Extract and decode the service parameter
147+
location = res.headers['Location']
148+
parsed_url = urllib.parse.urlparse(location)
149+
query = urllib.parse.parse_qs(parsed_url.query)
150+
service = query.get('service', [None])[0]
151+
152+
assert service is not None
153+
decoded_service = urllib.parse.unquote(service)
154+
assert '&new=true' in decoded_service
155+
156+
assert not mock_send_mail.called
157+
158+
user.reload()
159+
assert user.is_registered
160+
assert token not in user.email_verifications
161+
assert user.external_identity == {'ORCID': {'0002-0001-0001-0001': 'VERIFIED'}}
162+
assert user.emails.filter(address=email.lower()).exists()
163+
164+
@mock.patch('website.mails.send_mail')
165+
def test_post_success_link(self, mock_send_mail, app, confirm_url, user_with_email_verification):
166+
import urllib.parse
167+
168+
user, token, email = user_with_email_verification
169+
user.external_identity['ORCID']['0000-0000-0000-0000'] = 'LINK'
170+
user.save()
171+
172+
res = app.post_json_api(
173+
confirm_url,
174+
{
175+
'data': {
176+
'attributes': {
177+
'uid': user._id,
178+
'token': token,
179+
'destination': 'doesnotmatter'
180+
}
181+
}
182+
},
183+
expect_errors=True
184+
)
185+
assert res.status_code == 302
186+
187+
# Decode the redirect URL and check that &new=true is NOT present
188+
location = res.headers['Location']
189+
parsed_url = urllib.parse.urlparse(location)
190+
query = urllib.parse.parse_qs(parsed_url.query)
191+
service = query.get('service', [None])[0]
192+
193+
assert service is not None
194+
decoded_service = urllib.parse.unquote(service)
195+
assert '&new=true' not in decoded_service
196+
197+
assert mock_send_mail.called
198+
199+
user.reload()
200+
assert user.external_identity['ORCID']['0000-0000-0000-0000'] == 'VERIFIED'

framework/auth/oauth_scopes.py

+8-1
Original file line numberDiff line numberDiff line change
@@ -32,6 +32,7 @@ class CoreScopes:
3232
USERS_MESSAGE_READ_EMAIL = 'users_message_read_email'
3333
USERS_MESSAGE_WRITE_EMAIL = 'users_message_write_email'
3434
USERS_CREATE = 'users_create'
35+
USERS_CONFIRM = 'users_confirm'
3536

3637
USER_SETTINGS_READ = 'user.settings_read'
3738
USER_SETTINGS_WRITE = 'user.settings_write'
@@ -214,7 +215,13 @@ class ComposedScopes:
214215

215216
# Users collection
216217
USERS_READ = (CoreScopes.USERS_READ, CoreScopes.SUBSCRIPTIONS_READ, CoreScopes.ALERTS_READ, CoreScopes.USER_SETTINGS_READ)
217-
USERS_WRITE = USERS_READ + (CoreScopes.USERS_WRITE, CoreScopes.SUBSCRIPTIONS_WRITE, CoreScopes.ALERTS_WRITE, CoreScopes.USER_SETTINGS_WRITE)
218+
USERS_WRITE = USERS_READ + (
219+
CoreScopes.USERS_WRITE,
220+
CoreScopes.USERS_CONFIRM,
221+
CoreScopes.SUBSCRIPTIONS_WRITE,
222+
CoreScopes.ALERTS_WRITE,
223+
CoreScopes.USER_SETTINGS_WRITE
224+
)
218225
USERS_CREATE = USERS_READ + (CoreScopes.USERS_CREATE, )
219226

220227
# User extensions

0 commit comments

Comments
 (0)