diff --git a/api/users/serializers.py b/api/users/serializers.py index e7e306f9194..72299a234ff 100644 --- a/api/users/serializers.py +++ b/api/users/serializers.py @@ -442,7 +442,7 @@ class Meta: type_ = 'user_reset_password' -class ExternalLoginConfirmEmailSerializer(BaseAPISerializer): +class ConfirmEmailSerializer(BaseAPISerializer): uid = ser.CharField(write_only=True, required=True) destination = ser.CharField(write_only=True, required=True) token = ser.CharField(write_only=True, required=True) diff --git a/api/users/urls.py b/api/users/urls.py index bd7eaa6b3cb..74c91c53074 100644 --- a/api/users/urls.py +++ b/api/users/urls.py @@ -14,6 +14,7 @@ re_path(r'^(?P\w+)/addons/(?P\w+)/accounts/$', views.UserAddonAccountList.as_view(), name=views.UserAddonAccountList.view_name), re_path(r'^(?P\w+)/addons/(?P\w+)/accounts/(?P\w+)/$', views.UserAddonAccountDetail.as_view(), name=views.UserAddonAccountDetail.view_name), re_path(r'^(?P\w+)/claim/$', views.ClaimUser.as_view(), name=views.ClaimUser.view_name), + re_path(r'^(?P\w+)/confirm/$', views.ConfirmEmailView.as_view(), name=views.ConfirmEmailView.view_name), re_path(r'^(?P\w+)/draft_registrations/$', views.UserDraftRegistrations.as_view(), name=views.UserDraftRegistrations.view_name), re_path(r'^(?P\w+)/institutions/$', views.UserInstitutions.as_view(), name=views.UserInstitutions.view_name), re_path(r'^(?P\w+)/nodes/$', views.UserNodes.as_view(), name=views.UserNodes.view_name), diff --git a/api/users/views.py b/api/users/views.py index 4da2f5102d2..b983b142ff2 100644 --- a/api/users/views.py +++ b/api/users/views.py @@ -68,7 +68,7 @@ UserChangePasswordSerializer, UserMessageSerializer, ExternalLoginSerialiser, - ExternalLoginConfirmEmailSerializer, + ConfirmEmailSerializer, ) from django.contrib.auth.models import AnonymousUser from django.http import JsonResponse @@ -80,6 +80,7 @@ from framework.auth.oauth_scopes import CoreScopes, normalize_scopes from framework.auth.exceptions import ChangePasswordError from framework.celery_tasks.handlers import enqueue_task +from django.shortcuts import redirect from framework.utils import throttle_period_expired from framework.sessions.utils import remove_sessions_for_user from framework.exceptions import PermissionsError, HTTPError @@ -105,7 +106,7 @@ from website import mails, settings, language from website.project.views.contributor import send_claim_email, send_claim_registered_email from website.util.metrics import CampaignClaimedTags, CampaignSourceTags -from framework.auth import exceptions +from framework.auth import exceptions, cas class UserMixin: @@ -1060,6 +1061,91 @@ def post(self, request, *args, **kwargs): return Response(status=status.HTTP_204_NO_CONTENT) +class ConfirmEmailView(generics.CreateAPIView): + """ + Confirm an e-mail address created during *first-time* OAuth login. + + **URL:** POST /v2/users//confirm/ + + **Body (JSON):** + { + "uid": "", + "token": "", + "destination": "" + } + + On success the endpoint redirects (HTTP 302) to CAS with a + one-time verification key, exactly like the original Flask view. + """ + permission_classes = ( + base_permissions.TokenHasScope, + ) + required_read_scopes = [CoreScopes.USERS_CONFIRM] + required_write_scopes = [CoreScopes.USERS_CONFIRM] + + view_category = 'users' + view_name = 'confirm-user' + + serializer_class = ConfirmEmailSerializer + + def post(self, request, *args, **kwargs): + serializer = self.serializer_class(data=request.data) + serializer.is_valid(raise_exception=True) + + uid = serializer.validated_data['uid'] + token = serializer.validated_data['token'] + + user = OSFUser.load(uid) + if not user: + raise ValidationError('User not found.') + + verification = user.email_verifications.get(token) + if not verification: + raise ValidationError('Invalid or expired token.') + + provider = next(iter(verification['external_identity'])) + provider_id = next(iter(verification['external_identity'][provider])) + + if provider not in user.external_identity: + raise ValidationError('External-ID provider mismatch.') + + external_status = user.external_identity[provider][provider_id] + ensure_external_identity_uniqueness(provider, provider_id, user) + + email = verification['email'] + if not user.is_registered: + user.register(email) + + user.emails.get_or_create(address=email.lower()) + user.external_identity[provider][provider_id] = 'VERIFIED' + user.date_last_logged_in = timezone.now() + + del user.email_verifications[token] + user.verification_key = generate_verification_key() + user.save() + + service_url = request.build_absolute_uri() + if external_status == 'CREATE': + service_url += '&' + urlencode({'new': 'true'}) + elif external_status == 'LINK': + mails.send_mail( + user=user, + to_addr=user.username, + mail=mails.EXTERNAL_LOGIN_LINK_SUCCESS, + external_id_provider=provider, + can_change_preferences=False, + ) + + enqueue_task(update_affiliation_for_orcid_sso_users.s(user._id, provider_id)) + + return redirect( + cas.get_login_url( + service_url, + username=user.username, + verification_key=user.verification_key, + ), + ) + class UserEmailsList(JSONAPIBaseView, generics.ListAPIView, generics.CreateAPIView, UserMixin, ListFilterMixin): permission_classes = ( drf_permissions.IsAuthenticatedOrReadOnly, @@ -1212,7 +1298,7 @@ class ExternalLoginConfirmEmailView(generics.CreateAPIView): permission_classes = ( drf_permissions.AllowAny, ) - serializer_class = ExternalLoginConfirmEmailSerializer + serializer_class = ConfirmEmailSerializer view_category = 'users' view_name = 'external-login-confirm-email' throttle_classes = (NonCookieAuthThrottle, BurstRateThrottle, RootAnonThrottle) diff --git a/api_tests/base/test_views.py b/api_tests/base/test_views.py index b09df8d753c..b553c5c7ef0 100644 --- a/api_tests/base/test_views.py +++ b/api_tests/base/test_views.py @@ -17,7 +17,7 @@ CountedAuthUsageView, MetricsOpenapiView, ) -from api.users.views import ClaimUser, ResetPassword, ExternalLoginConfirmEmailView, ExternalLogin +from api.users.views import ClaimUser, ResetPassword, ExternalLoginConfirmEmailView, ExternalLogin, ConfirmEmailView from api.registrations.views import RegistrationCallbackView from api.wb.views import MoveFileMetadataView, CopyFileMetadataView from rest_framework.permissions import IsAuthenticatedOrReadOnly, IsAuthenticated @@ -63,6 +63,7 @@ def setUp(self): MetricsOpenapiView, ResetPassword, ExternalLoginConfirmEmailView, + ConfirmEmailView, ExternalLogin, RegistrationCallbackView, ] diff --git a/api_tests/users/views/test_user_confirm.py b/api_tests/users/views/test_user_confirm.py new file mode 100644 index 00000000000..9b4303c63b4 --- /dev/null +++ b/api_tests/users/views/test_user_confirm.py @@ -0,0 +1,200 @@ +import pytest +from unittest import mock + +from api.base.settings.defaults import API_BASE +from osf_tests.factories import AuthUserFactory + + +@pytest.mark.django_db +class TestConfirmEmail: + + @pytest.fixture() + def user(self): + return AuthUserFactory() + + @pytest.fixture() + def user_with_email_verification(self, user): + email = 'HurtsSoGood@eagles.burds.com' + external_identity = { + 'ORCID': { + '0002-0001-0001-0001': 'CREATE', + } + } + token = user.add_unconfirmed_email(email, external_identity=external_identity) + user.external_identity.update(external_identity) + user.save() + return user, token, email + + @pytest.fixture() + def confirm_url(self, user_with_email_verification): + user, _, _ = user_with_email_verification + return f'/{API_BASE}users/{user._id}/confirm/' + + def test_get_not_allowed(self, app, confirm_url): + res = app.get(confirm_url, expect_errors=True) + assert res.status_code == 405 + + def test_post_missing_fields(self, app, confirm_url, user_with_email_verification): + user, _, _ = user_with_email_verification + res = app.post_json_api( + confirm_url, + {'data': {'attributes': {}}}, + expect_errors=True, + auth=user.auth + ) + assert res.status_code == 400 + assert res.json['errors'] == [ + { + 'source': { + 'pointer': '/data/attributes/uid' + }, + 'detail': 'This field is required.' + }, + { + 'source': { + 'pointer': '/data/attributes/destination' + }, + 'detail': 'This field is required.' + }, + { + 'source': { + 'pointer': '/data/attributes/token' + }, + 'detail': 'This field is required.' + } + ] + + def test_post_user_not_found(self, app, user_with_email_verification): + user, _, _ = user_with_email_verification + fake_user_id = 'abcd1' + res = app.post_json_api( + f'/{API_BASE}users/{fake_user_id}/confirm/', + { + 'data': { + 'attributes': { + 'uid': fake_user_id, + 'token': 'doesnotmatter', + 'destination': 'doesnotmatter', + } + } + }, + expect_errors=True + ) + assert res.status_code == 400 + assert 'user not found' in res.json['errors'][0]['detail'].lower() + + def test_post_invalid_token(self, app, confirm_url, user_with_email_verification): + user, _, _ = user_with_email_verification + res = app.post_json_api( + confirm_url, + { + 'data': { + 'attributes': { + 'uid': user._id, + 'token': 'badtoken', + 'destination': 'doesnotmatter', + } + } + }, + expect_errors=True + ) + assert res.status_code == 400 + assert res.json['errors'] == [{'detail': 'Invalid or expired token.'}] + + def test_post_provider_mismatch(self, app, confirm_url, user_with_email_verification): + user, token, _ = user_with_email_verification + user.external_identity = {} # clear the valid provider + user.save() + + res = app.post_json_api( + confirm_url, + { + 'data': { + 'attributes': { + 'uid': user._id, + 'token': token, + 'destination': 'doesnotmatter', + } + } + }, + expect_errors=True + ) + assert res.status_code == 400 + assert 'provider mismatch' in res.json['errors'][0]['detail'].lower() + + @mock.patch('website.mails.send_mail') + def test_post_success_create(self, mock_send_mail, app, confirm_url, user_with_email_verification): + user, token, email = user_with_email_verification + user.is_registered = False + user.save() + res = app.post_json_api( + confirm_url, + { + 'data': { + 'attributes': { + 'uid': user._id, + 'token': token, + 'destination': 'doesnotmatter', + } + } + }, + expect_errors=True + ) + assert res.status_code == 302 + import urllib.parse + + # Extract and decode the service parameter + location = res.headers['Location'] + parsed_url = urllib.parse.urlparse(location) + query = urllib.parse.parse_qs(parsed_url.query) + service = query.get('service', [None])[0] + + assert service is not None + decoded_service = urllib.parse.unquote(service) + assert '&new=true' in decoded_service + + assert not mock_send_mail.called + + user.reload() + assert user.is_registered + assert token not in user.email_verifications + assert user.external_identity == {'ORCID': {'0002-0001-0001-0001': 'VERIFIED'}} + assert user.emails.filter(address=email.lower()).exists() + + @mock.patch('website.mails.send_mail') + def test_post_success_link(self, mock_send_mail, app, confirm_url, user_with_email_verification): + import urllib.parse + + user, token, email = user_with_email_verification + user.external_identity['ORCID']['0000-0000-0000-0000'] = 'LINK' + user.save() + + res = app.post_json_api( + confirm_url, + { + 'data': { + 'attributes': { + 'uid': user._id, + 'token': token, + 'destination': 'doesnotmatter' + } + } + }, + expect_errors=True + ) + assert res.status_code == 302 + + # Decode the redirect URL and check that &new=true is NOT present + location = res.headers['Location'] + parsed_url = urllib.parse.urlparse(location) + query = urllib.parse.parse_qs(parsed_url.query) + service = query.get('service', [None])[0] + + assert service is not None + decoded_service = urllib.parse.unquote(service) + assert '&new=true' not in decoded_service + + assert mock_send_mail.called + + user.reload() + assert user.external_identity['ORCID']['0000-0000-0000-0000'] == 'VERIFIED' diff --git a/framework/auth/oauth_scopes.py b/framework/auth/oauth_scopes.py index 796023228df..fcbd45802ff 100644 --- a/framework/auth/oauth_scopes.py +++ b/framework/auth/oauth_scopes.py @@ -32,6 +32,7 @@ class CoreScopes: USERS_MESSAGE_READ_EMAIL = 'users_message_read_email' USERS_MESSAGE_WRITE_EMAIL = 'users_message_write_email' USERS_CREATE = 'users_create' + USERS_CONFIRM = 'users_confirm' USER_SETTINGS_READ = 'user.settings_read' USER_SETTINGS_WRITE = 'user.settings_write' @@ -214,7 +215,13 @@ class ComposedScopes: # Users collection USERS_READ = (CoreScopes.USERS_READ, CoreScopes.SUBSCRIPTIONS_READ, CoreScopes.ALERTS_READ, CoreScopes.USER_SETTINGS_READ) - USERS_WRITE = USERS_READ + (CoreScopes.USERS_WRITE, CoreScopes.SUBSCRIPTIONS_WRITE, CoreScopes.ALERTS_WRITE, CoreScopes.USER_SETTINGS_WRITE) + USERS_WRITE = USERS_READ + ( + CoreScopes.USERS_WRITE, + CoreScopes.USERS_CONFIRM, + CoreScopes.SUBSCRIPTIONS_WRITE, + CoreScopes.ALERTS_WRITE, + CoreScopes.USER_SETTINGS_WRITE + ) USERS_CREATE = USERS_READ + (CoreScopes.USERS_CREATE, ) # User extensions