Skip to content

[ENG-7965] Add v2 confirmation endpoint #11139

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 1 commit into
base: feature/pbs-25-10
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion api/users/serializers.py
Original file line number Diff line number Diff line change
Expand Up @@ -442,7 +442,7 @@ class Meta:
type_ = 'user_reset_password'


class ExternalLoginConfirmEmailSerializer(BaseAPISerializer):
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Refactor because it's now shared

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)
Expand Down
1 change: 1 addition & 0 deletions api/users/urls.py
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@
re_path(r'^(?P<user_id>\w+)/addons/(?P<provider>\w+)/accounts/$', views.UserAddonAccountList.as_view(), name=views.UserAddonAccountList.view_name),
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),
re_path(r'^(?P<user_id>\w+)/claim/$', views.ClaimUser.as_view(), name=views.ClaimUser.view_name),
re_path(r'^(?P<user_id>\w+)/confirm/$', views.ConfirmEmailView.as_view(), name=views.ConfirmEmailView.view_name),
re_path(r'^(?P<user_id>\w+)/draft_registrations/$', views.UserDraftRegistrations.as_view(), name=views.UserDraftRegistrations.view_name),
re_path(r'^(?P<user_id>\w+)/institutions/$', views.UserInstitutions.as_view(), name=views.UserInstitutions.view_name),
re_path(r'^(?P<user_id>\w+)/nodes/$', views.UserNodes.as_view(), name=views.UserNodes.view_name),
Expand Down
92 changes: 89 additions & 3 deletions api/users/views.py
Original file line number Diff line number Diff line change
Expand Up @@ -68,7 +68,7 @@
UserChangePasswordSerializer,
UserMessageSerializer,
ExternalLoginSerialiser,
ExternalLoginConfirmEmailSerializer,
ConfirmEmailSerializer,
)
from django.contrib.auth.models import AnonymousUser
from django.http import JsonResponse
Expand All @@ -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
Expand All @@ -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:
Expand Down Expand Up @@ -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/<user_id>/confirm/
**Body (JSON):**
{
"uid": "<osf_user_id>",
"token": "<email_verification_token>",
"destination": "<campaign-code or relative URL>"
}
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,
Expand Down Expand Up @@ -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)
Expand Down
3 changes: 2 additions & 1 deletion api_tests/base/test_views.py
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -63,6 +63,7 @@ def setUp(self):
MetricsOpenapiView,
ResetPassword,
ExternalLoginConfirmEmailView,
ConfirmEmailView,
ExternalLogin,
RegistrationCallbackView,
]
Expand Down
200 changes: 200 additions & 0 deletions api_tests/users/views/test_user_confirm.py
Original file line number Diff line number Diff line change
@@ -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 = '[email protected]'
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'
9 changes: 8 additions & 1 deletion framework/auth/oauth_scopes.py
Original file line number Diff line number Diff line change
Expand Up @@ -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'
Expand Down Expand Up @@ -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
Expand Down
Loading