Skip to content
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
7 changes: 7 additions & 0 deletions admin_tests/users/test_views.py
Original file line number Diff line number Diff line change
Expand Up @@ -113,6 +113,13 @@ def test_correct_view_permissions(self):
class TestGDPRDeleteUser(AdminTestCase):
def setUp(self):
self.user = UserFactory()
self.user.external_identity_tokens = {
'ORCID': {'fake-orcid-id': {'access_token': 'fake-orcid-token'}},
}
self.user.save()
post_patcher = mock.patch('osf.models.user.requests.post', return_value=mock.Mock(status_code=200))
self.mock_post = post_patcher.start()
self.addCleanup(post_patcher.stop)
self.request = RequestFactory().post('/fake_path')
self.view = views.UserGDPRDeleteView
self.view = setup_log_view(self.view, self.request, guid=self.user._id)
Expand Down
9 changes: 8 additions & 1 deletion api_tests/users/views/test_user_detail.py
Original file line number Diff line number Diff line change
Expand Up @@ -1216,8 +1216,15 @@ def test_requesting_deactivated_user_returns_410_response_and_meta_info(
res.json['errors'][0]['meta']['profile_image']).netloc == 'secure.gravatar.com'
assert res.json['errors'][0]['detail'] == 'The requested user is no longer available.'

@mock.patch('osf.models.user.requests.post')
def test_gdpr_deleted_user_returns_404_and_no_meta_info(
self, app, user_one, user_two):
self, mock_post, app, user_one, user_two):
mock_post.return_value = mock.Mock(status_code=200)
user_one.external_identity_tokens = {
'ORCID': {'fake-orcid-id': {'access_token': 'fake-orcid-token'}},
}
user_one.save()

url = f'/{API_BASE}users/{user_one._id}/'
res = app.get(url, auth=user_two.auth, expect_errors=False)
assert res.status_code == 200
Expand Down
1 change: 1 addition & 0 deletions framework/auth/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -60,6 +60,7 @@ def external_first_login_authenticate(user_dict, response):
data = {
'auth_user_external_id_provider': user_dict['external_id_provider'],
'auth_user_external_id': user_dict['external_id'],
'auth_user_external_id_access_token': user_dict['external_id_access_token'],
'auth_user_fullname': user_dict['fullname'],
'auth_user_external_first_login': True,
'service_url': user_dict['service_url'],
Expand Down
15 changes: 15 additions & 0 deletions framework/auth/cas.py
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@
from lxml import etree
import requests


from framework.auth import authenticate, external_first_login_authenticate
from framework.auth.core import get_user, generate_verification_key
from framework.auth.utils import print_cas_log, LogLevel
Expand Down Expand Up @@ -253,6 +254,13 @@ def get_profile_url():

return get_client().get_profile_url()

def save_orcid_access_token_to_user(user, orcid_id: str, access_token: str):
if orcid_id and access_token:
provider = settings.EXTERNAL_IDENTITY_PROFILE['OrcidProfile']
user.external_identity_tokens.setdefault(provider, {})[orcid_id] = {
'access_token': access_token,
}
return

def make_response_from_ticket(ticket, service_url):
"""
Expand Down Expand Up @@ -297,7 +305,13 @@ def make_response_from_ticket(ticket, service_url):
# this extra step will guarantee that 2FA are enforced
# current CAS session created by external login must be cleared first before authentication
if external_credential:
access_token = cas_resp.attributes.get('orcidAccessToken', None)
user.verification_key = generate_verification_key()
save_orcid_access_token_to_user(
user,
external_credential['id'],
access_token,
)
user.save()
print_cas_log(
f'CAS response - redirect existing external IdP login to verification key login: user=[{user._id}]',
Expand Down Expand Up @@ -325,6 +339,7 @@ def make_response_from_ticket(ticket, service_url):
user = {
'external_id_provider': external_credential['provider'],
'external_id': external_credential['id'],
'external_id_access_token': cas_resp.attributes.get('orcidAccessToken', None),
'fullname': fullname,
'service_url': service_furl.url,
}
Expand Down
1 change: 1 addition & 0 deletions framework/auth/tasks.py
Original file line number Diff line number Diff line change
Expand Up @@ -54,6 +54,7 @@ def update_affiliation_for_orcid_sso_users(user_id, orcid_id):
logger.error(error_message)
sentry.log_message(error_message)
return

institution = check_institution_affiliation(orcid_id)
if institution:
logger.info(f'Eligible institution affiliation has been found for ORCiD SSO user: '
Expand Down
3 changes: 3 additions & 0 deletions framework/auth/views.py
Original file line number Diff line number Diff line change
Expand Up @@ -1050,6 +1050,7 @@ def external_login_email_post():

external_id_provider = session.get('auth_user_external_id_provider', None)
external_id = session.get('auth_user_external_id', None)
external_id_access_token = session.get('auth_user_external_id_access_token', None)
fullname = session.get('auth_user_fullname', None) or form.name.data
service_url = session.get('service_url', None)

Expand Down Expand Up @@ -1131,6 +1132,7 @@ def external_login_email_post():
campaign=None,
accepted_terms_of_service=accepted_terms_of_service
)
cas.save_orcid_access_token_to_user(user, external_id, external_id_access_token)
user.save()
# 3. send confirmation email
send_confirm_email_async(
Expand Down Expand Up @@ -1174,6 +1176,7 @@ def clear_external_first_login_anonymous_session_data(session):
session.pop('auth_user_external_first_login')
session.pop('auth_user_external_id_provider')
session.pop('auth_user_external_id')
session.pop('auth_user_external_id_access_token')
session.pop('auth_user_fullname')
session.pop('service_url')
session['post_request_removal'] = True
Expand Down
18 changes: 18 additions & 0 deletions osf/migrations/0053_osfuser_orcid_token_tracking.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,18 @@
from django.db import migrations
import osf.utils.datetime_aware_jsonfield
import osf.utils.fields


class Migration(migrations.Migration):

dependencies = [
('osf', '0052_downloadevent_download_channel'),
]

operations = [
migrations.AddField(
model_name='osfuser',
name='external_identity_tokens',
field=osf.utils.datetime_aware_jsonfield.DateTimeAwareJSONField(blank=True, default=dict, encoder=osf.utils.datetime_aware_jsonfield.DateTimeAwareJSONEncoder),
),
]
77 changes: 62 additions & 15 deletions osf/models/user.py
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@
# OSF imports
import itsdangerous
import pytz
import requests
from dirtyfields import DirtyFieldsMixin

from django.conf import settings
Expand All @@ -20,7 +21,7 @@
from django.contrib.auth.models import PermissionsMixin
from django.core.exceptions import FieldDoesNotExist
from django.dispatch import receiver
from django.db import models
from django.db import models, transaction
from django.db.models import Count, Exists, OuterRef
from django.db.models.signals import post_save
from django.utils import timezone
Expand Down Expand Up @@ -319,6 +320,16 @@ class OSFUser(DirtyFieldsMixin, GuidMixin, BaseModel, AbstractBaseUser, Permissi
# },
# ...
# }
external_identity_tokens = DateTimeAwareJSONField(default=dict, blank=True)
# Format: {
# <external_id_provider>: {
# <external_id>: {
# "access_token" : <token>,
# }
# ...
# },
# ...
# }

# Employment history
jobs = DateTimeAwareJSONField(default=list, blank=True, validators=[validate_history_item])
Expand Down Expand Up @@ -817,7 +828,12 @@ def merge_user(self, user):
self.external_identity[service] = {
service_id: status
}

token_entry = user.external_identity_tokens.get(service, {}).get(service_id)
if token_entry:
self.external_identity_tokens.setdefault(service, {})[service_id] = token_entry
user.external_identity = {}
user.external_identity_tokens = {}

# FOREIGN FIELDS
self.external_accounts.add(*user.external_accounts.values_list('pk', flat=True))
Expand Down Expand Up @@ -2063,21 +2079,22 @@ def gdpr_delete(self):
"""
Complies with GDPR guidelines by disabling the account and removing identifying information.
"""
self._validate_and_remove_resource_for_gdpr_delete(
self.nodes.all(),
hard_delete=False
)
self._validate_and_remove_resource_for_gdpr_delete(
self.preprints.all(),
hard_delete=False
)
self._validate_and_remove_resource_for_gdpr_delete(
self.draft_registrations.all(),
hard_delete=True
)
with transaction.atomic():
self._validate_and_remove_resource_for_gdpr_delete(
self.nodes.all(),
hard_delete=False
)
self._validate_and_remove_resource_for_gdpr_delete(
self.preprints.all(),
hard_delete=False
)
self._validate_and_remove_resource_for_gdpr_delete(
self.draft_registrations.all(),
hard_delete=True
)

# Finally delete the user's info.
self._clear_identifying_information()
# Finally delete the user's info.
self._clear_identifying_information()

def _validate_and_remove_resource_for_gdpr_delete(self, resources, hard_delete):
"""
Expand Down Expand Up @@ -2137,6 +2154,34 @@ def _clear_identifying_information(self):
'''
This method ensures a user's info is deleted during a GDPR delete
'''
# A user has at most one ORCID identity, so there is at most one token entry to revoke.
orcid_id, token_entry = next(iter(self.external_identity_tokens.get('ORCID', {}).items()), (None, None))
orcid_token = token_entry.get('access_token') if token_entry else None

if not (orcid_id and orcid_token):
raise UserStateError('User do not have connected ORCID')
else:
try:
response = requests.post(
website_settings.ORCID_OAUTH_REVOKE_URL,
data={
'client_id': website_settings.ORCID_OAUTH_CLIENT_ID,
'client_secret': website_settings.ORCID_OAUTH_CLIENT_SECRET,
'token': orcid_token,
},
timeout=website_settings.ORCID_OAUTH_REVOKE_REQUEST_TIMEOUT,
)
response.raise_for_status()
except requests.exceptions.RequestException as e:
sentry.log_message(
f'[GDPR delete] Failed to revoke ORCID token for user {self._id} ORCID id {orcid_id}: {e}',
level=logging.ERROR,
)
sentry.log_exception(e)
raise UserStateError(
'Fail to revoke ORCID\'s service could not be reached'
)

# This doesn't remove identifying info, but ensures other users can't see the deleted user's profile etc.
self.deactivate_account()

Expand Down Expand Up @@ -2172,7 +2217,9 @@ def _clear_identifying_information(self):
account.profile_url = None
account.save()
self.external_accounts.clear()

self.external_identity = {}
self.external_identity_tokens = {}
self.deleted = timezone.now()

@property
Expand Down
26 changes: 26 additions & 0 deletions osf_tests/test_merging_users.py
Original file line number Diff line number Diff line change
Expand Up @@ -56,6 +56,7 @@ def _add_unregistered_user(self):
self.project_with_unreg_contrib.save()

@pytest.mark.enable_enqueue_task
@pytest.mark.enable_search
@mock.patch('website.mailchimp_utils.get_mailchimp_api')
def test_merge(self, mock_get_mailchimp_api):
def is_mrm_field(value):
Expand Down Expand Up @@ -273,6 +274,31 @@ def test_merge_preserves_external_identity(self):
assert linking_user.external_identity == {}
assert no_provider_user.external_identity == {'ORCID': {'1234-1234-1234-1234': 'VERIFIED', '4321-4321-4321-4321': 'VERIFIED'}}

def test_merge_transfers_external_identity_tokens(self):
surviving_user = UserFactory(
external_identity={'ORCID': {'1234-1234-1234-1234': 'VERIFIED'}},
)
merged_user = UserFactory(
external_identity={'ORCID': {'1234-1234-1234-1234': 'VERIFIED', '4321-4321-4321-4321': 'VERIFIED'}},
external_identity_tokens={
'ORCID': {
'1234-1234-1234-1234': {'access_token': 'token-1234'},
'4321-4321-4321-4321': {'access_token': 'token-4321'},
},
},
)

with override_flag(ENABLE_GV, active=True):
surviving_user.merge_user(merged_user)

assert surviving_user.external_identity_tokens == {
'ORCID': {
'1234-1234-1234-1234': {'access_token': 'token-1234'},
'4321-4321-4321-4321': {'access_token': 'token-4321'},
},
}
assert merged_user.external_identity_tokens == {}

def test_merge_unregistered(self):
# test only those behaviors that are not tested with unconfirmed users
self._add_unregistered_user()
Expand Down
Loading
Loading