Skip to content

Commit c856700

Browse files
authored
[ENG-10586] BE: Ensure only one ORCID ID is availalble in the UserModel.external_identity attribute (#11661)
* If a user has an existing ORCID ID, and then associates a new ORCID ID, the existing ORCID ID should be removed/overwritten * add test to check if orcid is overwritten * update literal from 'orcid' to 'ORCID' * update tests * v1 looks to be used for orcid auth (give the hint with comment)
1 parent c26712d commit c856700

File tree

3 files changed

+31
-6
lines changed

3 files changed

+31
-6
lines changed

api/users/views.py

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -755,7 +755,11 @@ def post(self, request, *args, **kwargs):
755755
# 1. update user oauth, with pending status
756756
external_identity[external_id_provider][external_id] = 'LINK'
757757
if external_id_provider in user.external_identity:
758-
user.external_identity[external_id_provider].update(external_identity[external_id_provider])
758+
# v1 looks to be used because of /confirm/external/ usage for auth but add orcid external identity rewrite updates for v2 as well
759+
if external_id_provider == settings.EXTERNAL_IDENTITY_PROFILE.get('OrcidProfile'):
760+
user.external_identity[external_id_provider] = external_identity[external_id_provider]
761+
else:
762+
user.external_identity[external_id_provider].update(external_identity[external_id_provider])
759763
else:
760764
user.external_identity.update(external_identity)
761765
if not user.accepted_terms_of_service and accepted_terms_of_service:
@@ -1153,7 +1157,6 @@ class ConfirmEmailView(generics.CreateAPIView):
11531157

11541158
def _process_external_identity(self, user, external_identity, service_url):
11551159
"""Handle all external_identity logic, including task enqueueing and url updates."""
1156-
11571160
provider = next(iter(external_identity))
11581161
if provider not in user.external_identity:
11591162
raise ValidationError('External-ID provider mismatch.')

api_tests/users/views/test_user_external_login.py

Lines changed: 21 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -48,7 +48,7 @@ def csrf_token(self):
4848
@pytest.fixture()
4949
def session_data(self):
5050
session = SessionStore()
51-
session['auth_user_external_id_provider'] = 'orcid'
51+
session['auth_user_external_id_provider'] = 'ORCID'
5252
session['auth_user_external_id'] = '1234-1234-1234-1234'
5353
session['auth_user_fullname'] = 'external login'
5454
session['auth_user_external_first_login'] = True
@@ -62,7 +62,7 @@ def test_external_login(self, app, payload, url, session_data, csrf_token):
6262
with capture_notifications():
6363
res = app.post_json_api(url, payload, headers={'X-CSRFToken': csrf_token})
6464
assert res.status_code == 200
65-
assert res.json == {'external_id_provider': 'orcid', 'auth_user_fullname': 'external login'}
65+
assert res.json == {'external_id_provider': 'ORCID', 'auth_user_fullname': 'external login'}
6666
assert not OSFUser.objects.get(username='freddie@mercury.com').is_confirmed
6767

6868
def test_invalid_payload(self, app, url, session_data, csrf_token):
@@ -84,6 +84,24 @@ def test_existing_user(self, app, payload, url, user_one, session_data, csrf_tok
8484
with capture_notifications():
8585
res = app.post_json_api(url, payload, headers={'X-CSRFToken': csrf_token})
8686
assert res.status_code == 200
87-
assert res.json == {'external_id_provider': 'orcid', 'auth_user_fullname': 'external login'}
87+
assert res.json == {'external_id_provider': 'ORCID', 'auth_user_fullname': 'external login'}
8888
user_one.reload()
8989
assert user_one.username in user_one.unconfirmed_emails
90+
91+
def test_existing_user_orcid_overwrites(self, app, payload, url, user_one, session_data, csrf_token):
92+
user_one.external_identity = {
93+
'ORCID': {
94+
'0000-0000-0000-0000': 'LINK',
95+
}
96+
}
97+
user_one.save()
98+
app.set_cookie(CSRF_COOKIE_NAME, csrf_token)
99+
app.set_cookie(settings.COOKIE_NAME, str(session_data))
100+
assert user_one.external_identity['ORCID'] == {'0000-0000-0000-0000': 'LINK'}
101+
assert '0000-0000-0000-0000' in user_one.external_identity['ORCID']
102+
payload['data']['attributes']['email'] = user_one.username
103+
with capture_notifications():
104+
res = app.post_json_api(url, payload, headers={'X-CSRFToken': csrf_token})
105+
assert res.status_code == 200
106+
user_one.reload()
107+
assert user_one.external_identity['ORCID'] == {'1234-1234-1234-1234': 'LINK'}

framework/auth/views.py

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1096,7 +1096,11 @@ def external_login_email_post():
10961096
# 1. update user oauth, with pending status
10971097
external_identity[external_id_provider][external_id] = 'LINK'
10981098
if external_id_provider in user.external_identity:
1099-
user.external_identity[external_id_provider].update(external_identity[external_id_provider])
1099+
# v1 looks to be used because of /confirm/external/ usage for auth but add orcid external identity rewrite updates for v2 as well
1100+
if external_id_provider == settings.EXTERNAL_IDENTITY_PROFILE.get('OrcidProfile'):
1101+
user.external_identity[external_id_provider] = external_identity[external_id_provider]
1102+
else:
1103+
user.external_identity[external_id_provider].update(external_identity[external_id_provider])
11001104
else:
11011105
user.external_identity.update(external_identity)
11021106
if not user.accepted_terms_of_service and form.accepted_terms_of_service.data:

0 commit comments

Comments
 (0)