Fix/eng 11829 2

admin_tests/users/test_views.py

@@ -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)

api_tests/users/views/test_user_detail.py

@@ -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

framework/auth/cas.py

@@ -8,6 +8,9 @@
 from lxml import etree
 import requests
 
+import logging
+
+from framework import sentry
 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
@@ -253,6 +256,22 @@ def get_profile_url():
 
     return get_client().get_profile_url()
 
+def save_orcid_access_token_to_user(user, orcid_id: str, access_token: str):
+    sentry.log_message(
+        f'CAS response ORCID attributes: user=[{user._id}], orcidId=[{orcid_id}], '
+        f'orcidAccessToken=[{"present" if access_token else "missing"}]',
+        level=logging.WARNING,
+    )
+    if orcid_id and access_token:
+        provider = settings.EXTERNAL_IDENTITY_PROFILE['OrcidProfile']
+        user.external_identity_tokens.setdefault(provider, {})[orcid_id] = {
+            'access_token': access_token,
+        }
+        sentry.log_message(
+            f'ORCID token stored on external_identity_tokens: user=[{user._id}], '
+            f'provider_id=[{orcid_id}], access_token=[{"present" if access_token else "missing"}]',
+            level=logging.INFO,
+        )
 
 def make_response_from_ticket(ticket, service_url):
     """
@@ -297,7 +316,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}]',
@@ -325,6 +350,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,
             }

framework/auth/tasks.py

@@ -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: '

osf/migrations/0053_osfuser_orcid_token_tracking.py

@@ -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),
+        ),
+    ]

osf/models/user.py

@@ -12,6 +12,7 @@
 # OSF imports
 import itsdangerous
 import pytz
+import requests
 from dirtyfields import DirtyFieldsMixin
 
 from django.conf import settings
@@ -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])
@@ -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))
@@ -2137,6 +2153,50 @@ 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
+        sentry.log_message(
+            f'[GDPR delete; _clear_identifying_information] user={self._id}: '
+            f'{"found" if orcid_token else "no"} ORCID token to revoke',
+            level=logging.INFO,
+        )
+        if not (orcid_id and orcid_token):
+            raise UserStateError(
+                'User do not have connected ORCID'
+            )
+        else:
+            sentry.log_message(
+                f'[GDPR delete] user={self._id}: revoking ORCID id={orcid_id} '
+                f'via {website_settings.ORCID_OAUTH_REVOKE_URL}',
+                level=logging.INFO,
+            )
+            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,
+                )
+                sentry.log_message(
+                    f'[GDPR delete] user={self._id}: ORCID id={orcid_id} revoked, '
+                    f'status_code={response.status_code}, response_text={response.text}',
+                    level=logging.INFO,
+                )
+                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()
 
@@ -2172,7 +2232,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

osf_tests/test_merging_users.py

@@ -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):
@@ -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()

osf_tests/test_user.py

@@ -12,6 +12,7 @@
 from unittest import mock
 import itsdangerous
 import pytest
+import requests
 from importlib import import_module
 from waffle.testutils import override_flag
 
@@ -2124,7 +2125,13 @@ class TestUserGdprDelete:
 
     @pytest.fixture()
     def user(self):
-        return AuthUserFactory()
+        user = AuthUserFactory()
+        user.external_identity_tokens = {
+            'ORCID': {'fake-orcid-id': {'access_token': 'fake-orcid-token'}},
+        }
+        user.save()
+        with mock.patch('osf.models.user.requests.post', return_value=mock.Mock(status_code=200)):
+            yield user
 
     @pytest.fixture()
     def project_with_two_admins(self, user):
@@ -2224,11 +2231,18 @@ def test_gdpr_delete_triggers_share_update_for_public_shared_preprints(
 
         assert mock_update_search.called
 
-    def test_can_gdpr_delete(self, user):
+    @mock.patch('osf.models.user.requests.post')
+    def test_can_gdpr_delete(self, mock_post, user):
+        mock_post.return_value = mock.Mock(status_code=200)
+        user.external_identity = {'ORCID': {'fake-orcid-id': 'VERIFIED'}}
+        user.external_identity_tokens = {
+            'ORCID': {'fake-orcid-id': {'access_token': 'fake-orcid-token'}},
+        }
+        user.save()
+
         user.social = ['fake social']
         user.schools = ['fake schools']
         user.jobs = ['fake jobs']
-        user.external_identity = ['fake external identity']
         user.external_accounts.add(ExternalAccountFactory())
 
         user.gdpr_delete()
@@ -2239,10 +2253,100 @@ def test_can_gdpr_delete(self, user):
         assert user.schools == []
         assert user.jobs == []
         assert user.external_identity == {}
+        assert user.external_identity_tokens == {}
         assert not user.emails.exists()
         assert not user.external_accounts.exists()
         assert user.is_disabled
         assert user.deleted is not None
+        mock_post.assert_called_once()
+        assert mock_post.call_args.kwargs['data']['token'] == 'fake-orcid-token'
+
+    @mock.patch('osf.models.user.requests.post')
+    def test_gdpr_delete_no_orcid_token_no_revoke_call(self, mock_post, user):
+        mock_post.return_value = mock.Mock(status_code=200)
+        user.external_identity_tokens = {
+            'ORCID': {'fake-orcid-id': {'access_token': 'fake-orcid-token'}},
+        }
+        user.save()
+
+        user.gdpr_delete()
+
+        mock_post.assert_called_once()
+        assert mock_post.call_args.kwargs['data']['token'] == 'fake-orcid-token'
+
+    @mock.patch('osf.models.user.requests.post')
+    def test_gdpr_delete_orcid_revoke_failure_blocks_delete(self, mock_post, user):
+        mock_post.side_effect = requests.exceptions.ConnectionError('boom')
+        user.external_identity_tokens = {
+            'ORCID': {'fake-orcid-id': {'access_token': 'fake-orcid-token'}},
+        }
+        user.save()
+
+        with pytest.raises(UserStateError):
+            user.gdpr_delete()
+
+        mock_post.assert_called_once()
+        assert user.deleted is None
+        assert not user.is_disabled
+        assert user.external_identity_tokens == {
+            'ORCID': {'fake-orcid-id': {'access_token': 'fake-orcid-token'}},
+        }
+
+    @mock.patch('osf.models.user.requests.post')
+    def test_gdpr_delete_orcid_revoke_invalid_client_credentials_blocks_delete(self, mock_post, user):
+        mock_response = mock.Mock(status_code=401, text='invalid_client')
+        mock_response.raise_for_status.side_effect = requests.exceptions.HTTPError('401 Client Error')
+        mock_post.return_value = mock_response
+        user.external_identity_tokens = {
+            'ORCID': {'fake-orcid-id': {'access_token': 'fake-orcid-token'}},
+        }
+        user.save()
+
+        with pytest.raises(UserStateError):
+            user.gdpr_delete()
+
+        mock_post.assert_called_once()
+        assert mock_post.call_args.kwargs['data']['token'] == 'fake-orcid-token'
+        assert user.deleted is None
+        assert not user.is_disabled
+        assert user.external_identity_tokens == {
+            'ORCID': {'fake-orcid-id': {'access_token': 'fake-orcid-token'}},
+        }
+
+    @mock.patch('osf.models.user.requests.post')
+    def test_gdpr_delete_orcid_revoke_invalid_token_blocks_delete(self, mock_post, user):
+        mock_response = mock.Mock(status_code=400, text='invalid_token')
+        mock_response.raise_for_status.side_effect = requests.exceptions.HTTPError('400 Client Error')
+        mock_post.return_value = mock_response
+        user.external_identity_tokens = {
+            'ORCID': {'fake-orcid-id': {'access_token': 'bad-orcid-token'}},
+        }
+        user.save()
+
+        with pytest.raises(UserStateError):
+            user.gdpr_delete()
+
+        mock_post.assert_called_once()
+        assert mock_post.call_args.kwargs['data']['token'] == 'bad-orcid-token'
+        assert user.deleted is None
+        assert not user.is_disabled
+        assert user.external_identity_tokens == {
+            'ORCID': {'fake-orcid-id': {'access_token': 'bad-orcid-token'}},
+        }
+
+    @mock.patch('osf.models.user.requests.post')
+    def test_gdpr_delete_orcid_empty_token_no_revoke_call(self, mock_post, user):
+        mock_post.return_value = mock.Mock(status_code=200)
+        user.external_identity_tokens = {
+            'ORCID': {'fake-orcid-id': {'access_token': 'fake-orcid-token'}},
+        }
+        user.save()
+
+        user.gdpr_delete()
+
+        mock_post.assert_called_once()
+        assert user.deleted is not None
+        assert user.is_disabled
 
     def test_can_gdpr_delete_personal_nodes(self, user):