From 18906f227d1cddd285f97185acd5878e70515bcd Mon Sep 17 00:00:00 2001 From: John Tordoff Date: Wed, 23 Jan 2019 16:51:03 -0500 Subject: [PATCH 01/42] remove unused EZID code --- osf_tests/factories.py | 19 +--- scripts/create_fakes.py | 2 - tests/identifiers/test_ezid.py | 134 ------------------------ website/identifiers/clients/__init__.py | 1 - website/identifiers/clients/ezid.py | 80 -------------- website/settings/defaults.py | 4 - 6 files changed, 2 insertions(+), 238 deletions(-) delete mode 100644 tests/identifiers/test_ezid.py delete mode 100644 website/identifiers/clients/ezid.py diff --git a/osf_tests/factories.py b/osf_tests/factories.py index 452ee464840..0a0f02f0aef 100644 --- a/osf_tests/factories.py +++ b/osf_tests/factories.py @@ -21,7 +21,6 @@ from website.notifications.constants import NOTIFICATION_TYPES from osf.utils import permissions from website.archiver import ARCHIVER_SUCCESS -from website.identifiers.utils import parse_identifiers from website.settings import FAKE_EMAIL_NAME, FAKE_EMAIL_DOMAIN from framework.auth.core import Auth @@ -591,23 +590,9 @@ def _create(cls, target_class, *args, **kwargs): def sync_set_identifiers(preprint): - from website.identifiers.clients import EzidClient from website import settings - client = preprint.get_doi_client() - - if isinstance(client, EzidClient): - doi_value = settings.DOI_FORMAT.format(prefix=settings.EZID_DOI_NAMESPACE, guid=preprint._id) - ark_value = '{ark}osf.io/{guid}'.format(ark=settings.EZID_ARK_NAMESPACE, guid=preprint._id) - return_value = {'success': '{} | {}'.format(doi_value, ark_value)} - else: - return_value = {'doi': settings.DOI_FORMAT.format(prefix=preprint.provider.doi_prefix, guid=preprint._id)} - - doi_client_return_value = { - 'response': return_value, - 'already_exists': False - } - id_dict = parse_identifiers(doi_client_return_value) - preprint.set_identifier_values(doi=id_dict['doi']) + doi = settings.DOI_FORMAT.format(prefix=preprint.provider.doi_prefix, guid=preprint._id) + preprint.set_identifier_values(doi=doi) class PreprintFactory(DjangoModelFactory): diff --git a/scripts/create_fakes.py b/scripts/create_fakes.py index f010da5a61d..9ec3cb6498b 100644 --- a/scripts/create_fakes.py +++ b/scripts/create_fakes.py @@ -311,8 +311,6 @@ def create_fake_project(creator, n_users, privacy, n_components, name, n_tags, p if not provider: provider = PreprintProviderFactory(name=fake.science_word()) privacy = 'public' - mock_change_identifier = mock.patch('website.identifiers.client.EzidClient.update_identifier') - mock_change_identifier.start() mock_change_identifier_preprints = mock.patch('website.identifiers.client.CrossRefClient.update_identifier') mock_change_identifier_preprints.start() project = PreprintFactory(title=project_title, description=fake.science_paragraph(), creator=creator, provider=provider) diff --git a/tests/identifiers/test_ezid.py b/tests/identifiers/test_ezid.py deleted file mode 100644 index 8ef0f1f3bb8..00000000000 --- a/tests/identifiers/test_ezid.py +++ /dev/null @@ -1,134 +0,0 @@ -import furl -import mock -import pytest -import responses -from waffle.testutils import override_switch -from nose.tools import * # noqa - -from tests.base import OsfTestCase -from tests.test_addons import assert_urls_equal -from osf_tests.factories import AuthUserFactory, RegistrationFactory - -from website import settings -from website.app import init_addons -from website.identifiers.utils import to_anvl -from website.identifiers.clients import EzidClient - - -@pytest.mark.django_db -class TestEZIDClient(OsfTestCase): - - def setUp(self): - super(TestEZIDClient, self).setUp() - self.user = AuthUserFactory() - self.registration = RegistrationFactory(creator=self.user, is_public=True) - self.client = EzidClient(base_url='https://test.ezid.osf.io', prefix=settings.EZID_DOI_NAMESPACE.replace('doi:', '')) - - @override_switch('ezid', active=True) - @responses.activate - def test_create_identifiers_not_exists_ezid(self): - guid = self.registration._id - url = furl.furl(self.client.base_url) - doi = settings.DOI_FORMAT.format(prefix=settings.EZID_DOI_NAMESPACE, guid=guid).replace('doi:', '') - url.path.segments += ['id', doi] - responses.add( - responses.Response( - responses.PUT, - url.url, - body=to_anvl({ - 'success': '{doi}osf.io/{ident} | {ark}osf.io/{ident}'.format( - doi=settings.EZID_DOI_NAMESPACE, - ark=settings.EZID_ARK_NAMESPACE, - ident=guid, - ), - }), - status=201, - ) - ) - with mock.patch('osf.models.Registration.get_doi_client') as mock_get_doi: - mock_get_doi.return_value = self.client - res = self.app.post( - self.registration.api_url_for('node_identifiers_post'), - auth=self.user.auth, - ) - self.registration.reload() - assert_equal( - res.json['doi'], - self.registration.get_identifier_value('doi') - ) - - assert_equal(res.status_code, 201) - - @override_switch('ezid', active=True) - @responses.activate - def test_create_identifiers_exists_ezid(self): - guid = self.registration._id - doi = settings.DOI_FORMAT.format(prefix=settings.EZID_DOI_NAMESPACE, guid=guid).replace('doi:', '') - url = furl.furl(self.client.base_url) - url.path.segments += ['id', doi] - responses.add( - responses.Response( - responses.PUT, - url.url, - body='identifier already exists', - status=400, - ) - ) - responses.add( - responses.Response( - responses.GET, - url.url, - body=to_anvl({ - 'success': doi, - }), - status=200, - ) - ) - with mock.patch('osf.models.Registration.get_doi_client') as mock_get_doi: - mock_get_doi.return_value = self.client - res = self.app.post( - self.registration.api_url_for('node_identifiers_post'), - auth=self.user.auth, - ) - self.registration.reload() - assert_equal( - res.json['doi'], - self.registration.get_identifier_value('doi') - ) - assert_equal(res.status_code, 201) - - @responses.activate - def test_get_by_identifier(self): - self.registration.set_identifier_value('doi', 'FK424601') - self.registration.set_identifier_value('ark', 'fk224601') - res_doi = self.app.get( - self.registration.web_url_for( - 'get_referent_by_identifier', - category='doi', - value=self.registration.get_identifier_value('doi'), - ), - ) - assert_equal(res_doi.status_code, 302) - assert_urls_equal(res_doi.headers['Location'], self.registration.absolute_url) - res_ark = self.app.get( - self.registration.web_url_for( - 'get_referent_by_identifier', - category='ark', - value=self.registration.get_identifier_value('ark'), - ), - ) - assert_equal(res_ark.status_code, 302) - assert_urls_equal(res_ark.headers['Location'], self.registration.absolute_url) - - @responses.activate - def test_get_by_identifier_not_found(self): - self.registration.set_identifier_value('doi', 'FK424601') - res = self.app.get( - self.registration.web_url_for( - 'get_referent_by_identifier', - category='doi', - value='fakedoi', - ), - expect_errors=True, - ) - assert_equal(res.status_code, 404) diff --git a/website/identifiers/clients/__init__.py b/website/identifiers/clients/__init__.py index 60f1d0ec489..6dd707891fc 100644 --- a/website/identifiers/clients/__init__.py +++ b/website/identifiers/clients/__init__.py @@ -1,3 +1,2 @@ from .crossref import CrossRefClient, ECSArXivCrossRefClient # noqa from .datacite import DataCiteClient # noqa -from .ezid import EzidClient # noqa diff --git a/website/identifiers/clients/ezid.py b/website/identifiers/clients/ezid.py deleted file mode 100644 index fe9c38655a4..00000000000 --- a/website/identifiers/clients/ezid.py +++ /dev/null @@ -1,80 +0,0 @@ -# -*- coding: utf-8 -*- -import logging -import furl -import requests - -from website import settings -from website.identifiers import utils -from website.util.client import BaseClient -from website.identifiers.clients import DataCiteClient, exceptions - -logger = logging.getLogger(__name__) - - -class EzidClient(BaseClient, DataCiteClient): - """Inherits _make_request from BaseClient""" - - def _build_url(self, *segments, **query): - url = furl.furl(self.base_url) - url.path.segments.extend(segments) - url.args.update(query) - return url.url - - @property - def _default_headers(self): - return {'Content-Type': 'text/plain; charset=UTF-8'} - - def build_doi(self, object): - return settings.DOI_FORMAT.format(prefix=self.prefix, guid=object._id) - - def get_identifier(self, identifier): - resp = self._make_request( - 'GET', - self._build_url('id', identifier), - expects=(200, ), - ) - return utils.from_anvl(resp.content.strip('\n')) - - def create_identifier(self, object, category): - if category in ['doi', 'ark']: - metadata = self.build_metadata(object) - doi = self.build_doi(object) - resp = requests.request( - 'PUT', - self._build_url('id', doi), - data=utils.to_anvl(metadata or {}), - ) - if resp.status_code != 201: - if 'identifier already exists' in resp.content: - raise exceptions.IdentifierAlreadyExists() - else: - raise exceptions.ClientResponseError(resp) - resp = utils.from_anvl(resp.content) - return dict( - [each.strip('/') for each in pair.strip().split(':')] - for pair in resp['success'].split('|') - ) - else: - raise NotImplementedError('Create identifier method is not supported for category {}'.format(category)) - - def update_identifier(self, object, category): - metadata = self.build_metadata(object) - status = self.get_status(object) - metadata['_status'] = status - identifier = self.build_doi(object) - resp = self._make_request( - 'POST', - self._build_url('id', identifier), - data=utils.to_anvl(metadata or {}), - expects=(200, ), - ) - return utils.from_anvl(resp.content) - - def get_status(self, object): - from osf.models import Preprint - - if isinstance(object, Preprint): - status = 'public' if object.verified_publishable else 'unavailable' - else: - status = 'public' if object.is_public or not object.is_deleted else 'unavailable' - return status diff --git a/website/settings/defaults.py b/website/settings/defaults.py index dd81999d400..881ef4393a7 100644 --- a/website/settings/defaults.py +++ b/website/settings/defaults.py @@ -310,10 +310,6 @@ def parent_dir(path): # General Format for DOIs DOI_FORMAT = '{prefix}/osf.io/{guid}' -# ezid -EZID_DOI_NAMESPACE = 'doi:10.5072' -EZID_ARK_NAMESPACE = 'ark:99999' - # datacite DATACITE_USERNAME = None DATACITE_PASSWORD = None From 023fd4b784bfde3169c2bbea2805be4697ae8272 Mon Sep 17 00:00:00 2001 From: John Tordoff Date: Thu, 24 Jan 2019 08:59:47 -0500 Subject: [PATCH 02/42] stop passing status through the update_or_create_preprint_idientifiers --- osf/models/identifiers.py | 4 ++-- osf/models/node.py | 3 +-- website/identifiers/clients/crossref.py | 10 +++++----- website/identifiers/clients/datacite.py | 2 +- website/identifiers/listeners.py | 2 +- website/identifiers/tasks.py | 4 ++-- website/preprints/tasks.py | 3 +-- 7 files changed, 13 insertions(+), 15 deletions(-) diff --git a/osf/models/identifiers.py b/osf/models/identifiers.py index e6149452353..5d871a74c0d 100644 --- a/osf/models/identifiers.py +++ b/osf/models/identifiers.py @@ -51,10 +51,10 @@ def request_identifier(self, category): if client: return client.create_identifier(self, category) - def request_identifier_update(self, category, status=None): + def request_identifier_update(self, category): client = self.get_doi_client() if client: - return client.update_identifier(self, category, status=status) + return client.update_identifier(self, category) def get_identifier(self, category): """Returns None of no identifier matches""" diff --git a/osf/models/node.py b/osf/models/node.py index 4d9c06501d0..72fb817c5e4 100644 --- a/osf/models/node.py +++ b/osf/models/node.py @@ -1186,8 +1186,7 @@ def set_privacy(self, permissions, auth=None, log=True, save=True, meeting_creat # Update existing identifiers if self.get_identifier('doi'): - doi_status = 'unavailable' if permissions == 'private' else 'public' - enqueue_task(update_doi_metadata_on_change.s(self._id, status=doi_status)) + enqueue_task(update_doi_metadata_on_change.s(self._id)) if log: action = NodeLog.MADE_PUBLIC if permissions == 'public' else NodeLog.MADE_PRIVATE diff --git a/website/identifiers/clients/crossref.py b/website/identifiers/clients/crossref.py index a1d65e98496..ad19794f6ad 100644 --- a/website/identifiers/clients/crossref.py +++ b/website/identifiers/clients/crossref.py @@ -214,9 +214,9 @@ def _build_url(self, **query): url.args.update(query) return url.url - def create_identifier(self, preprint, category, status=None, include_relation=True): - if status is None: - status = self.get_status(preprint) + def create_identifier(self, preprint, category, include_relation=True): + status = self.get_status(preprint) + if category == 'doi': metadata = self.build_metadata(preprint, status, include_relation) doi = self.build_doi(preprint) @@ -241,8 +241,8 @@ def create_identifier(self, preprint, category, status=None, include_relation=Tr else: raise NotImplementedError() - def update_identifier(self, preprint, category, status=None): - return self.create_identifier(preprint, category, status) + def update_identifier(self, preprint, category): + return self.create_identifier(preprint, category) def get_status(self, preprint): return 'public' if preprint.verified_publishable and not preprint.is_retracted else 'unavailable' diff --git a/website/identifiers/clients/datacite.py b/website/identifiers/clients/datacite.py index c0bd4570278..d8aa5d05da7 100644 --- a/website/identifiers/clients/datacite.py +++ b/website/identifiers/clients/datacite.py @@ -85,7 +85,7 @@ def create_identifier(self, node, category): else: raise NotImplementedError('Creating an identifier with category {} is not supported'.format(category)) - def update_identifier(self, node, category, status=None): + def update_identifier(self, node, category): if not node.is_public or node.is_deleted: if category == 'doi': doi = self.build_doi(node) diff --git a/website/identifiers/listeners.py b/website/identifiers/listeners.py index 6d3307d7dde..b0acb6efb06 100644 --- a/website/identifiers/listeners.py +++ b/website/identifiers/listeners.py @@ -7,4 +7,4 @@ def update_status_on_delete(node): from website.identifiers.tasks import update_doi_metadata_on_change if node.get_identifier('doi'): - enqueue_task(update_doi_metadata_on_change.s(node._id, status='unavailable')) + enqueue_task(update_doi_metadata_on_change.s(node._id)) diff --git a/website/identifiers/tasks.py b/website/identifiers/tasks.py index 1c17ed199bf..b6f5cade2f4 100644 --- a/website/identifiers/tasks.py +++ b/website/identifiers/tasks.py @@ -4,8 +4,8 @@ @celery_app.task(ignore_results=True) -def update_doi_metadata_on_change(target_guid, status): +def update_doi_metadata_on_change(target_guid): Guid = apps.get_model('osf.Guid') target_object = Guid.load(target_guid).referent if target_object.get_identifier('doi'): - target_object.request_identifier_update(category='doi', status=status) + target_object.request_identifier_update(category='doi') diff --git a/website/preprints/tasks.py b/website/preprints/tasks.py index 1b8a52f2217..debb5871a02 100644 --- a/website/preprints/tasks.py +++ b/website/preprints/tasks.py @@ -48,9 +48,8 @@ def should_update_preprint_identifiers(preprint, old_subjects, saved_fields): ) def update_or_create_preprint_identifiers(preprint): - status = 'public' if preprint.verified_publishable and not preprint.is_retracted else 'unavailable' try: - preprint.request_identifier_update(category='doi', status=status) + preprint.request_identifier_update(category='doi') except HTTPError as err: sentry.log_exception() sentry.log_message(err.args[0]) From c494dd9785ba3db0625dabc93cbc784f581438da Mon Sep 17 00:00:00 2001 From: John Tordoff Date: Thu, 24 Jan 2019 09:09:12 -0500 Subject: [PATCH 03/42] remove unused metadata utils and tests --- api_tests/nodes/views/test_node_detail.py | 3 +- tests/identifiers/test_datacite.py | 3 - tests/identifiers/test_identifiers.py | 117 +------------------- website/identifiers/clients/crossref.py | 2 +- website/identifiers/metadata.py | 127 ---------------------- website/identifiers/utils.py | 75 +++---------- 6 files changed, 16 insertions(+), 311 deletions(-) delete mode 100644 website/identifiers/metadata.py diff --git a/api_tests/nodes/views/test_node_detail.py b/api_tests/nodes/views/test_node_detail.py index fda3788bbab..9e7697cdb6a 100644 --- a/api_tests/nodes/views/test_node_detail.py +++ b/api_tests/nodes/views/test_node_detail.py @@ -1150,8 +1150,7 @@ def test_set_node_private_updates_doi( assert res.status_code == 200 project_public.reload() assert not project_public.is_public - mock_update_doi_metadata.assert_called_with( - project_public._id, status='unavailable') + mock_update_doi_metadata.assert_called_with(project_public._id) @pytest.mark.enable_enqueue_task @mock.patch('website.preprints.tasks.update_or_enqueue_on_preprint_updated') diff --git a/tests/identifiers/test_datacite.py b/tests/identifiers/test_datacite.py index c1d3ae0643d..161f6b364fe 100644 --- a/tests/identifiers/test_datacite.py +++ b/tests/identifiers/test_datacite.py @@ -11,10 +11,7 @@ from framework.auth import Auth from website import settings -from website.app import init_addons from website.identifiers.clients import DataCiteClient -from website.identifiers.clients.datacite import DataCiteMDSClient -from website.identifiers import metadata from website.identifiers.utils import request_identifiers from tests.base import OsfTestCase diff --git a/tests/identifiers/test_identifiers.py b/tests/identifiers/test_identifiers.py index 5b47f0e95f2..edf4339dc11 100644 --- a/tests/identifiers/test_identifiers.py +++ b/tests/identifiers/test_identifiers.py @@ -2,130 +2,15 @@ from nose.tools import * # noqa from django.db import IntegrityError -from waffle.testutils import override_switch from osf_tests.factories import ( - SubjectFactory, - AuthUserFactory, - PreprintFactory, IdentifierFactory, RegistrationFactory, - PreprintProviderFactory ) from tests.base import OsfTestCase -import lxml.etree - -from website import settings -from website.identifiers import metadata -from osf.models import Identifier, Subject, NodeLicense - - -class TestMetadataGeneration(OsfTestCase): - - def setUp(self): - OsfTestCase.setUp(self) - self.visible_contrib = AuthUserFactory() - visible_contrib2 = AuthUserFactory(given_name=u'ヽ༼ ಠ益ಠ ༽ﾉ', family_name=u'ლ(´◉❥◉｀ლ)') - self.invisible_contrib = AuthUserFactory() - self.node = RegistrationFactory(is_public=True) - self.identifier = Identifier(referent=self.node, category='catid', value='cat:7') - self.node.add_contributor(self.visible_contrib, visible=True) - self.node.add_contributor(self.invisible_contrib, visible=False) - self.node.add_contributor(visible_contrib2, visible=True) - self.node.save() - - # This test is not used as datacite is currently used for nodes, leaving here for future reference - def test_datacite_metadata_for_preprint_has_correct_structure(self): - provider = PreprintProviderFactory() - license = NodeLicense.objects.get(name='CC-By Attribution 4.0 International') - license_details = { - 'id': license.license_id, - 'year': '2017', - 'copyrightHolders': ['Jeff Hardy', 'Matt Hardy'] - } - preprint = PreprintFactory(provider=provider, project=self.node, is_published=True, license_details=license_details) - metadata_xml = metadata.datacite_metadata_for_preprint(preprint, doi=preprint.get_identifier('doi').value, pretty_print=True) - - root = lxml.etree.fromstring(metadata_xml) - xsi_location = '{http://www.w3.org/2001/XMLSchema-instance}schemaLocation' - expected_location = 'http://datacite.org/schema/kernel-4 http://schema.datacite.org/meta/kernel-4/metadata.xsd' - assert root.attrib[xsi_location] == expected_location - - identifier = root.find('{%s}identifier' % metadata.NAMESPACE) - assert identifier.attrib['identifierType'] == 'DOI' - assert identifier.text == preprint.get_identifier('doi').value - - creators = root.find('{%s}creators' % metadata.NAMESPACE) - assert len(creators.getchildren()) == len(preprint.visible_contributors) - - subjects = root.find('{%s}subjects' % metadata.NAMESPACE) - assert subjects.getchildren() - - publisher = root.find('{%s}publisher' % metadata.NAMESPACE) - assert publisher.text == provider.name - - pub_year = root.find('{%s}publicationYear' % metadata.NAMESPACE) - assert pub_year.text == str(preprint.date_published.year) - - dates = root.find('{%s}dates' % metadata.NAMESPACE).getchildren()[0] - assert dates.text == preprint.modified.isoformat() - assert dates.attrib['dateType'] == 'Updated' - - alternate_identifier = root.find('{%s}alternateIdentifiers' % metadata.NAMESPACE).getchildren()[0] - assert alternate_identifier.text == settings.DOMAIN + preprint._id - assert alternate_identifier.attrib['alternateIdentifierType'] == 'URL' - - descriptions = root.find('{%s}descriptions' % metadata.NAMESPACE).getchildren()[0] - assert descriptions.text == preprint.description - - rights = root.find('{%s}rightsList' % metadata.NAMESPACE).getchildren()[0] - assert rights.text == preprint.license.name - - # This test is not used as datacite is currently used for nodes, leaving here for future reference - def test_datacite_format_creators_for_preprint(self): - preprint = PreprintFactory(project=self.node, is_published=True) - - verified_user = AuthUserFactory(external_identity={'ORCID': {'1234-1234-1234-1234': 'VERIFIED'}}) - linked_user = AuthUserFactory(external_identity={'ORCID': {'1234-nope-1234-nope': 'LINK'}}) - preprint.add_contributor(verified_user, visible=True) - preprint.add_contributor(linked_user, visible=True) - preprint.save() - - formatted_creators = metadata.format_creators(preprint) - - contributors_with_orcids = 0 - guid_identifiers = [] - for creator_xml in formatted_creators: - assert creator_xml.find('creatorName').text != u'{}, {}'.format(self.invisible_contrib.family_name, self.invisible_contrib.given_name) - - name_identifiers = creator_xml.findall('nameIdentifier') - - for name_identifier in name_identifiers: - if name_identifier.attrib['nameIdentifierScheme'] == 'ORCID': - assert name_identifier.attrib['schemeURI'] == 'http://orcid.org/' - contributors_with_orcids += 1 - else: - guid_identifiers.append(name_identifier.text) - assert name_identifier.attrib['nameIdentifierScheme'] == 'OSF' - assert name_identifier.attrib['schemeURI'] == settings.DOMAIN - - assert contributors_with_orcids >= 1 - assert len(formatted_creators) == len(self.node.visible_contributors) - assert sorted(guid_identifiers) == sorted([contrib.absolute_url for contrib in preprint.visible_contributors]) - - # This test is not used as datacite is currently used for nodes, leaving here for future reference - def test_datacite_format_subjects_for_preprint(self): - subject = SubjectFactory() - subject_1 = SubjectFactory(parent=subject) - subject_2 = SubjectFactory(parent=subject) - - subjects = [[subject._id, subject_1._id], [subject._id, subject_2._id]] - preprint = PreprintFactory(subjects=subjects, project=self.node, is_published=True) - - formatted_subjects = metadata.format_subjects(preprint) - assert len(formatted_subjects) == Subject.objects.all().count() +from osf.models import Identifier class TestIdentifierModel(OsfTestCase): diff --git a/website/identifiers/clients/crossref.py b/website/identifiers/clients/crossref.py index ad19794f6ad..da8bc00bb59 100644 --- a/website/identifiers/clients/crossref.py +++ b/website/identifiers/clients/crossref.py @@ -8,7 +8,7 @@ from django.db.models import QuerySet from framework.auth.utils import impute_names -from website.identifiers.metadata import remove_control_characters +from website.identifiers.utils import remove_control_characters from website.identifiers.clients.base import AbstractIdentifierClient from website import settings diff --git a/website/identifiers/metadata.py b/website/identifiers/metadata.py deleted file mode 100644 index 2bfa4623321..00000000000 --- a/website/identifiers/metadata.py +++ /dev/null @@ -1,127 +0,0 @@ -# -*- coding: utf-8 -*- -import unicodedata -import lxml.etree -import lxml.builder - -from website import settings - -NAMESPACE = 'http://datacite.org/schema/kernel-4' -XSI = 'http://www.w3.org/2001/XMLSchema-instance' -SCHEMA_LOCATION = 'http://datacite.org/schema/kernel-4 http://schema.datacite.org/meta/kernel-4/metadata.xsd' -E = lxml.builder.ElementMaker(nsmap={ - None: NAMESPACE, - 'xsi': XSI}, -) - -CREATOR = E.creator -CREATOR_NAME = E.creatorName -SUBJECT_SCHEME = 'bepress Digital Commons Three-Tiered Taxonomy' - -# From https://stackoverflow.com/a/19016117 -# lxml does not accept strings with control characters -def remove_control_characters(s): - return ''.join(ch for ch in s if unicodedata.category(ch)[0] != 'C') - -# This function is not OSF-specific -def datacite_metadata(doi, title, creators, publisher, publication_year, pretty_print=False): - """Return the formatted datacite metadata XML as a string. - - :param str doi - :param str title - :param list creators: List of creator names, formatted like 'Shakespeare, William' - :param str publisher: Publisher name. - :param int publication_year - :param bool pretty_print - """ - creators = [CREATOR(CREATOR_NAME(each)) for each in creators] - root = E.resource( - E.resourceType('Project', resourceTypeGeneral='Text'), - E.identifier(doi, identifierType='DOI'), - E.creators(*creators), - E.titles(E.title(remove_control_characters(title))), - E.publisher(publisher), - E.publicationYear(str(publication_year)), - ) - # set xsi:schemaLocation - root.attrib['{%s}schemaLocation' % XSI] = SCHEMA_LOCATION - return lxml.etree.tostring(root, pretty_print=pretty_print) - - -def format_contributor(contributor): - return remove_control_characters(u'{}, {}'.format(contributor.family_name, contributor.given_name)) - - -# This function is OSF specific. -def datacite_metadata_for_node(node, doi, pretty_print=False): - """Return the datacite metadata XML document for a given node as a string. - - :param Node node - :param str doi - """ - creators = [format_contributor(each) for each in node.visible_contributors] - return datacite_metadata( - doi=doi, - title=node.title, - creators=creators, - publisher='Open Science Framework', - publication_year=getattr(node.registered_date or node.created, 'year'), - pretty_print=pretty_print - ) - - -def format_creators(preprint): - creators = [] - for contributor in preprint.visible_contributors: - creator = CREATOR(E.creatorName(format_contributor(contributor))) - creator.append(E.givenName(remove_control_characters(contributor.given_name))) - creator.append(E.familyName(remove_control_characters(contributor.family_name))) - creator.append(E.nameIdentifier(contributor.absolute_url, nameIdentifierScheme='OSF', schemeURI=settings.DOMAIN)) - - # contributor.external_identity = {'ORCID': {'1234-1234-1234-1234': 'VERIFIED'}} - if contributor.external_identity.get('ORCID'): - verified = contributor.external_identity['ORCID'].values()[0] == 'VERIFIED' - if verified: - creator.append(E.nameIdentifier(contributor.external_identity['ORCID'].keys()[0], nameIdentifierScheme='ORCID', schemeURI='http://orcid.org/')) - - creators.append(creator) - - return creators - - -def format_subjects(preprint): - return [E.subject(subject, subjectScheme=SUBJECT_SCHEME) for subject in preprint.subjects.values_list('text', flat=True)] - - -# This function is OSF specific. -def datacite_metadata_for_preprint(preprint, doi, pretty_print=False): - """Return the datacite metadata XML document for a given preprint as a string. - - :param preprint -- the preprint - :param str doi - """ - # NOTE: If you change *ANYTHING* here be 100% certain that the - # changes you make are also made to the SHARE serialization code. - # If the data sent out is not EXCATLY the same all the data will get jumbled up in SHARE. - # And then search results will be wrong and broken. And it will be your fault. And you'll have caused many sleepless nights. - # Don't be that person. - root = E.resource( - E.resourceType('Preprint', resourceTypeGeneral='Text'), - E.identifier(doi, identifierType='DOI'), - E.subjects(*format_subjects(preprint)), - E.creators(*format_creators(preprint)), - E.titles(E.title(remove_control_characters(preprint.title))), - E.publisher(preprint.provider.name), - E.publicationYear(str(getattr(preprint.date_published, 'year'))), - E.dates(E.date(preprint.modified.isoformat(), dateType='Updated')), - E.alternateIdentifiers(E.alternateIdentifier(settings.DOMAIN + preprint._id, alternateIdentifierType='URL')), - E.descriptions(E.description(remove_control_characters(preprint.description), descriptionType='Abstract')), - ) - - if preprint.license: - root.append(E.rightsList(E.rights(preprint.license.name))) - - if preprint.article_doi: - root.append(E.relatedIdentifiers(E.relatedIdentifier(settings.DOI_URL_PREFIX + preprint.article_doi, relatedIdentifierType='URL', relationType='IsPreviousVersionOf'))), - # set xsi:schemaLocation - root.attrib['{%s}schemaLocation' % XSI] = SCHEMA_LOCATION - return lxml.etree.tostring(root, pretty_print=pretty_print) diff --git a/website/identifiers/utils.py b/website/identifiers/utils.py index 458c667dc97..c0f75408d84 100644 --- a/website/identifiers/utils.py +++ b/website/identifiers/utils.py @@ -2,9 +2,9 @@ import re import logging +import unicodedata from framework.exceptions import HTTPError -from website import settings logger = logging.getLogger(__name__) @@ -28,33 +28,6 @@ def unescape(value): return re.sub(r'%[0-9A-Fa-f]{2}', decode, value) -def to_anvl(data): - if isinstance(data, dict): - return FIELD_SEPARATOR.join( - PAIR_SEPARATOR.join([escape(key), escape(to_anvl(value))]) - for key, value in data.items() - ) - return data - - -def _field_from_anvl(raw): - key, value = raw.split(PAIR_SEPARATOR) - return [unescape(key), from_anvl(unescape(value))] - - -def from_anvl(data): - if PAIR_SEPARATOR in data: - return dict([ - _field_from_anvl(pair) - for pair in data.split(FIELD_SEPARATOR) - ]) - return data - - -def merge_dicts(*dicts): - return dict(sum((each.items() for each in dicts), [])) - - def request_identifiers(target_object): """Request identifiers for the target object using the appropriate client. @@ -74,43 +47,17 @@ def request_identifiers(target_object): if not client: return doi = client.build_doi(target_object) - already_exists = False - only_doi = True try: identifiers = target_object.request_identifier(category='doi') - except exceptions.IdentifierAlreadyExists as error: + except exceptions.IdentifierAlreadyExists: identifiers = client.get_identifier(doi) - already_exists = True - only_doi = False except exceptions.ClientResponseError as error: raise HTTPError(error.response.status_code) return { - 'doi': identifiers.get('doi'), - 'already_exists': already_exists, - 'only_doi': only_doi + 'doi': identifiers['doi'] } -def parse_identifiers(doi_client_response): - """ - Note: ARKs include a leading slash. This is stripped here to avoid multiple - consecutive slashes in internal URLs (e.g. /ids/ark//). Frontend code - that build ARK URLs is responsible for adding the leading slash. - Moved from website/project/views/register.py for use by other modules - """ - resp = doi_client_response['response'] - exists = doi_client_response.get('already_exists', None) - if exists: - doi = resp['success'] - suffix = doi.strip(settings.EZID_DOI_NAMESPACE) - return { - 'doi': doi.replace('doi:', ''), - 'ark': '{0}{1}'.format(settings.EZID_ARK_NAMESPACE.replace('ark:', ''), suffix), - } - else: - return {'doi': resp['doi']} - - def get_or_create_identifiers(target_object): """ Note: ARKs include a leading slash. This is stripped here to avoid multiple @@ -118,14 +65,18 @@ def get_or_create_identifiers(target_object): that build ARK URLs is responsible for adding the leading slash. Moved from website/project/views/register.py for use by other modules """ - response_dict = request_identifiers(target_object) - ark = target_object.get_identifier(category='ark') - doi = response_dict['doi'] + doi = request_identifiers(target_object)['doi'] if not doi: client = target_object.get_doi_client() doi = client.build_doi(target_object) - response = {'doi': doi} + + ark = target_object.get_identifier(category='ark') if ark: - response['ark'] = ark.value + return {'doi': doi, 'ark': ark} + + return {'doi': doi} - return response +# From https://stackoverflow.com/a/19016117 +# lxml does not accept strings with control characters +def remove_control_characters(s): + return ''.join(ch for ch in s if unicodedata.category(ch)[0] != 'C') From ffddfec663478c9c393a321020baf9201ec92cee Mon Sep 17 00:00:00 2001 From: John Tordoff Date: Thu, 13 Jun 2019 14:44:50 -0400 Subject: [PATCH 04/42] add periodic task that deactivates requested accounts with tests --- api/users/serializers.py | 12 +--- .../users/views/test_user_settings_detail.py | 7 --- .../0162_osfuser_contacted_deactivation.py | 20 +++++++ osf/models/user.py | 12 ++++ scripts/periodic/__init__.py | 0 .../periodic/deactivate_requested_accounts.py | 57 ++++++++++++++++++ .../test_deactivate_requested_accounts.py | 60 +++++++++++++++++++ tests/test_views.py | 9 --- website/mails/mails.py | 2 + website/profile/views.py | 14 ----- website/settings/defaults.py | 5 ++ .../request_deactivation_complete.html.mako | 19 ++++++ 12 files changed, 176 insertions(+), 41 deletions(-) create mode 100644 osf/migrations/0162_osfuser_contacted_deactivation.py create mode 100644 scripts/periodic/__init__.py create mode 100644 scripts/periodic/deactivate_requested_accounts.py create mode 100644 scripts/tests/test_deactivate_requested_accounts.py create mode 100644 website/templates/emails/request_deactivation_complete.html.mako diff --git a/api/users/serializers.py b/api/users/serializers.py index afa1515c797..d76cdc41a2e 100644 --- a/api/users/serializers.py +++ b/api/users/serializers.py @@ -20,9 +20,8 @@ from osf.exceptions import ValidationValueError, ValidationError, BlacklistedEmailError from osf.models import OSFUser, QuickFilesNode, Preprint from osf.utils.requests import string_type_request_headers -from website.settings import MAILCHIMP_GENERAL_LIST, OSF_HELP_LIST, CONFIRM_REGISTRATIONS_BY_EMAIL, OSF_SUPPORT_EMAIL +from website.settings import MAILCHIMP_GENERAL_LIST, OSF_HELP_LIST, CONFIRM_REGISTRATIONS_BY_EMAIL from osf.models.provider import AbstractProviderGroupObjectPermission -from website import mails from website.profile.views import update_osf_help_mails_subscription, update_mailchimp_subscription from api.nodes.serializers import NodeSerializer, RegionRelationshipField from api.base.schemas.utils import validate_user_json, from_json @@ -522,17 +521,8 @@ def verify_two_factor(self, instance, value, two_factor_addon): def request_deactivation(self, instance, requested_deactivation): if instance.requested_deactivation != requested_deactivation: - if requested_deactivation: - mails.send_mail( - to_addr=OSF_SUPPORT_EMAIL, - mail=mails.REQUEST_DEACTIVATION, - user=instance, - can_change_preferences=False, - ) - instance.email_last_sent = timezone.now() instance.requested_deactivation = requested_deactivation instance.save() - return def to_representation(self, instance): """ diff --git a/api_tests/users/views/test_user_settings_detail.py b/api_tests/users/views/test_user_settings_detail.py index e4dd06d46d9..d0433037a41 100644 --- a/api_tests/users/views/test_user_settings_detail.py +++ b/api_tests/users/views/test_user_settings_detail.py @@ -250,31 +250,24 @@ def test_patch_requested_deactivation(self, mock_mail, app, user_one, user_two, assert res.status_code == 403 # Logged in, request to deactivate - assert user_one.email_last_sent is None assert user_one.requested_deactivation is False res = app.patch_json_api(url, payload, auth=user_one.auth) assert res.status_code == 200 user_one.reload() - assert user_one.email_last_sent is not None assert user_one.requested_deactivation is True - assert mock_mail.call_count == 1 # Logged in, deactivation already requested res = app.patch_json_api(url, payload, auth=user_one.auth) assert res.status_code == 200 user_one.reload() - assert user_one.email_last_sent is not None assert user_one.requested_deactivation is True - assert mock_mail.call_count == 1 # Logged in, request to cancel deactivate request payload['data']['attributes']['deactivation_requested'] = False res = app.patch_json_api(url, payload, auth=user_one.auth) assert res.status_code == 200 user_one.reload() - assert user_one.email_last_sent is not None assert user_one.requested_deactivation is False - assert mock_mail.call_count == 1 @mock.patch('framework.auth.views.mails.send_mail') def test_patch_invalid_type(self, mock_mail, app, user_one, url, payload): diff --git a/osf/migrations/0162_osfuser_contacted_deactivation.py b/osf/migrations/0162_osfuser_contacted_deactivation.py new file mode 100644 index 00000000000..e8db0a83d3f --- /dev/null +++ b/osf/migrations/0162_osfuser_contacted_deactivation.py @@ -0,0 +1,20 @@ +# -*- coding: utf-8 -*- +# Generated by Django 1.11.15 on 2019-06-13 15:32 +from __future__ import unicode_literals + +from django.db import migrations, models + + +class Migration(migrations.Migration): + + dependencies = [ + ('osf', '0161_add_spam_fields_to_user'), + ] + + operations = [ + migrations.AddField( + model_name='osfuser', + name='contacted_deactivation', + field=models.BooleanField(default=False), + ), + ] diff --git a/osf/models/user.py b/osf/models/user.py index bef51f60e46..a5ec1795a20 100644 --- a/osf/models/user.py +++ b/osf/models/user.py @@ -369,6 +369,9 @@ class OSFUser(DirtyFieldsMixin, GuidMixin, BaseModel, AbstractBaseUser, Permissi # whether the user has requested to deactivate their account requested_deactivation = models.BooleanField(default=False) + # whether the user has who requested deactivation has been contacted + contacted_deactivation = models.BooleanField(default=False) + affiliated_institutions = models.ManyToManyField('Institution', blank=True) notifications_configured = DateTimeAwareJSONField(default=dict, blank=True) @@ -1705,6 +1708,15 @@ def gdpr_delete(self): self.external_identity = {} self.deleted = timezone.now() + @property + def has_resources(self): + # TODO: Update once quickfolders in merged + + nodes = self.nodes.exclude(type='osf.quickfilesnode').exists() + quickfiles = self.nodes.get(type='osf.quickfilesnode').files.exists() + + return nodes or quickfiles or self.preprints.exists() + class Meta: # custom permissions for use in the OSF Admin App permissions = ( diff --git a/scripts/periodic/__init__.py b/scripts/periodic/__init__.py new file mode 100644 index 00000000000..e69de29bb2d diff --git a/scripts/periodic/deactivate_requested_accounts.py b/scripts/periodic/deactivate_requested_accounts.py new file mode 100644 index 00000000000..64b43ad199e --- /dev/null +++ b/scripts/periodic/deactivate_requested_accounts.py @@ -0,0 +1,57 @@ +""" +This script runs nightly and emails users who want to delete there account with info on how to do so. Users who don't +have any content can be automatically deleted. +""" +import sys +import logging + +from website import mails +from django.utils import timezone + +from framework.celery_tasks import app as celery_app +from website.app import setup_django +setup_django() +from osf.models import OSFUser +from website.settings import OSF_SUPPORT_EMAIL, OSF_CONTACT_EMAIL + +from scripts.utils import add_file_logger + +logger = logging.getLogger(__name__) +logging.basicConfig(level=logging.INFO) + + +def deactivate_requested_accounts(dry_run=True): + users = OSFUser.objects.filter(requested_deactivation=True, contacted_deactivation=False, date_disabled__isnull=True) + + for user in users: + if user.has_resources: + mails.send_mail( + to_addr=OSF_SUPPORT_EMAIL, + mail=mails.REQUEST_DEACTIVATION, + user=user, + can_change_preferences=False, + ) + else: + user.disable_account() + mails.send_mail( + to_addr=user.username, + mail=mails.REQUEST_DEACTIVATION_COMPLETE, + user=user, + contact_email=OSF_CONTACT_EMAIL, + can_change_preferences=False, + ) + + user.contacted_deactivation = True + user.email_last_sent = timezone.now() + user.save() + + +@celery_app.task(name='scripts.periodic.deactivate_requested_accounts') +def run_main(dry_run=True): + if not dry_run: + add_file_logger(logger, __file__) + deactivate_requested_accounts(dry_run=dry_run) + + +if __name__ == '__main__': + run_main(dry_run='--dry' in sys.argv) diff --git a/scripts/tests/test_deactivate_requested_accounts.py b/scripts/tests/test_deactivate_requested_accounts.py new file mode 100644 index 00000000000..197c093d67a --- /dev/null +++ b/scripts/tests/test_deactivate_requested_accounts.py @@ -0,0 +1,60 @@ +# -*- coding: utf-8 -*- + +import pytest +import mock + +from nose.tools import * # noqa + +from osf_tests.factories import ProjectFactory, AuthUserFactory + +from scripts.periodic.deactivate_requested_accounts import deactivate_requested_accounts + +from website import mails, settings + +@pytest.mark.django_db +@pytest.mark.enable_quickfiles_creation +class TestDeactivateRequestedAccount: + + @pytest.fixture() + def user_requested_deactivation(self): + user = AuthUserFactory(requested_deactivation=True) + user.requested_deactivation = True + user.save() + return user + + @pytest.fixture() + def user_requested_deactivation_with_node(self): + user = AuthUserFactory(requested_deactivation=True) + node = ProjectFactory(creator=user) + node.save() + user.save() + return user + + @mock.patch('scripts.periodic.deactivate_requested_accounts.mails.send_mail') + def test_deactivate_user_with_no_content(self, mock_mail, user_requested_deactivation): + + deactivate_requested_accounts(dry_run=False) + user_requested_deactivation.reload() + + assert user_requested_deactivation.requested_deactivation + assert user_requested_deactivation.contacted_deactivation + assert user_requested_deactivation.is_disabled + mock_mail.assert_called_with(can_change_preferences=False, + mail=mails.REQUEST_DEACTIVATION_COMPLETE, + to_addr=user_requested_deactivation.username, + contact_email=settings.OSF_CONTACT_EMAIL, + user=user_requested_deactivation) + + @mock.patch('scripts.periodic.deactivate_requested_accounts.mails.send_mail') + def test_deactivate_user_with_content(self, mock_mail, user_requested_deactivation_with_node): + + deactivate_requested_accounts(dry_run=False) + user_requested_deactivation_with_node.reload() + + assert user_requested_deactivation_with_node.requested_deactivation + assert not user_requested_deactivation_with_node.is_disabled + mock_mail.assert_called_with(can_change_preferences=False, + mail=mails.REQUEST_DEACTIVATION, + to_addr=settings.OSF_SUPPORT_EMAIL, + user=user_requested_deactivation_with_node) + diff --git a/tests/test_views.py b/tests/test_views.py index 4597150a402..e107b952cca 100644 --- a/tests/test_views.py +++ b/tests/test_views.py @@ -1784,15 +1784,6 @@ def test_user_cannot_request_account_export_before_throttle_expires(self, send_m assert_equal(res.status_code, 400) assert_equal(send_mail.call_count, 1) - @mock.patch('framework.auth.views.mails.send_mail') - def test_user_cannot_request_account_deactivation_before_throttle_expires(self, send_mail): - url = api_url_for('request_deactivation') - self.app.post(url, auth=self.user.auth) - assert_true(send_mail.called) - res = self.app.post(url, auth=self.user.auth, expect_errors=True) - assert_equal(res.status_code, 400) - assert_equal(send_mail.call_count, 1) - def test_get_unconfirmed_emails_exclude_external_identity(self): external_identity = { 'service': { diff --git a/website/mails/mails.py b/website/mails/mails.py index abca57312b2..8439348f1c9 100644 --- a/website/mails/mails.py +++ b/website/mails/mails.py @@ -251,6 +251,8 @@ def get_english_article(word): REQUEST_EXPORT = Mail('support_request', subject='[via OSF] Export Request') REQUEST_DEACTIVATION = Mail('support_request', subject='[via OSF] Deactivation Request') +REQUEST_DEACTIVATION_COMPLETE = Mail('request_deactivation_complete', subject='[via OSF] Deactivation Request') + SPAM_USER_BANNED = Mail('spam_user_banned', subject='[OSF] Account flagged as spam') CONFERENCE_SUBMITTED = Mail( diff --git a/website/profile/views.py b/website/profile/views.py index 79c10f18954..933b68396a1 100644 --- a/website/profile/views.py +++ b/website/profile/views.py @@ -815,20 +815,6 @@ def request_export(auth): @must_be_logged_in def request_deactivation(auth): user = auth.user - if not throttle_period_expired(user.email_last_sent, settings.SEND_EMAIL_THROTTLE): - raise HTTPError(http.BAD_REQUEST, - data={ - 'message_long': 'Too many requests. Please wait a while before sending another account deactivation request.', - 'error_type': 'throttle_error' - }) - - mails.send_mail( - to_addr=settings.OSF_SUPPORT_EMAIL, - mail=mails.REQUEST_DEACTIVATION, - user=auth.user, - can_change_preferences=False, - ) - user.email_last_sent = timezone.now() user.requested_deactivation = True user.save() return {'message': 'Sent account deactivation request'} diff --git a/website/settings/defaults.py b/website/settings/defaults.py index cdf93a3ae87..afaa3a62de3 100644 --- a/website/settings/defaults.py +++ b/website/settings/defaults.py @@ -472,6 +472,7 @@ class CeleryConfig: 'scripts.generate_sitemap', 'scripts.premigrate_created_modified', 'scripts.add_missing_identifiers_to_preprints', + 'scripts.periodic.deactivate_requested_accounts', ) # Modules that need metrics and release requirements @@ -578,6 +579,10 @@ class CeleryConfig: 'task': 'scripts.generate_sitemap', 'schedule': crontab(minute=0, hour=5), # Daily 12:00 a.m. }, + 'deactivate_requested_accounts': { + 'task': 'scripts.periodic.deactivate_requested_accounts', + 'schedule': crontab(minute=0, hour=5), # Daily 12:00 a.m. + }, } # Tasks that need metrics and release requirements diff --git a/website/templates/emails/request_deactivation_complete.html.mako b/website/templates/emails/request_deactivation_complete.html.mako new file mode 100644 index 00000000000..6ef726186e3 --- /dev/null +++ b/website/templates/emails/request_deactivation_complete.html.mako @@ -0,0 +1,19 @@ +<%inherit file="notify_base.mako" /> + +<%def name="content()"> + + + Hi ${user.given_name}, +
+
+ + Your OSF account has been deactivated. You will not show up in search, nor will a profile be visible for you. + If you try to log in, you will receive an error message that your account has been disabled. If, in the future, + you would like to create an account with this email address, you can do so by emailing us at ${contact_email}. + +
+
+ Sincerely, + The OSF Team + + \ No newline at end of file From 1db904bf48fac1da995596114ff52055d19d41ee Mon Sep 17 00:00:00 2001 From: John Tordoff Date: Tue, 9 Jul 2019 15:39:33 -0400 Subject: [PATCH 05/42] hack together fix to send redirect url to akismet --- addons/forward/tests/test_views.py | 41 ++++++++++++++++++++++++++++-- addons/forward/views/config.py | 1 + api/nodes/serializers.py | 7 +++-- osf/models/mixins.py | 3 ++- osf/models/node.py | 1 + 5 files changed, 48 insertions(+), 5 deletions(-) diff --git a/addons/forward/tests/test_views.py b/addons/forward/tests/test_views.py index 11907155367..435c47759d3 100644 --- a/addons/forward/tests/test_views.py +++ b/addons/forward/tests/test_views.py @@ -1,16 +1,21 @@ +import mock import pytest from nose.tools import assert_equal from addons.forward.tests.utils import ForwardAddonTestCase from tests.base import OsfTestCase +from website import settings +from tests.json_api_test_app import JSONAPITestApp pytestmark = pytest.mark.django_db -class TestForwardLogs(ForwardAddonTestCase, OsfTestCase): +class TestForward(ForwardAddonTestCase, OsfTestCase): + + django_app = JSONAPITestApp() def setUp(self): - super(TestForwardLogs, self).setUp() + super(TestForward, self).setUp() self.app.authenticate(*self.user.auth) def test_change_url_log_added(self): @@ -27,6 +32,38 @@ def test_change_url_log_added(self): log_count + 1 ) + @mock.patch.object(settings, 'SPAM_CHECK_ENABLED', True) + @mock.patch('osf.models.node.Node.do_check_spam') + def test_change_url_check_spam_v1(self, mock_check_spam): + self.project.is_public = True + self.project.save() + self.app.put_json(self.project.api_url_for('forward_config_put'), {'url': 'http://possiblyspam.com'}) + + assert mock_check_spam.called + data, _ = mock_check_spam.call_args + author, author_email, content, request_headers = data + + assert author == self.user.fullname + assert author_email == self.user.username + assert content == 'http://possiblyspam.com' + + @mock.patch.object(settings, 'SPAM_CHECK_ENABLED', True) + @mock.patch('osf.models.node.Node.do_check_spam') + def test_change_url_check_spam_v2(self, mock_check_spam): + self.project.is_public = True + self.project.save() + self.django_app.put_json_api('/v2/nodes/{}/addons/forward/'.format(self.project._id), + {'data': {'attributes': {'url': 'http://possiblyspam.com'}}}, + auth=self.user.auth) + + assert mock_check_spam.called + data, _ = mock_check_spam.call_args + author, author_email, content, request_headers = data + + assert author == self.user.fullname + assert author_email == self.user.username + assert content == 'http://possiblyspam.com' + def test_change_timeout_log_not_added(self): log_count = self.project.logs.count() self.app.put_json( diff --git a/addons/forward/views/config.py b/addons/forward/views/config.py index 8c9f45972ed..5b59eed77fa 100644 --- a/addons/forward/views/config.py +++ b/addons/forward/views/config.py @@ -60,5 +60,6 @@ def forward_config_put(auth, node_addon, **kwargs): auth=auth, save=True, ) + node_addon.owner.check_spam(auth.user, {'addons_forward_node_settings__url'}, request.headers) return {} diff --git a/api/nodes/serializers.py b/api/nodes/serializers.py index 8cfd596572e..ac5c26084ca 100644 --- a/api/nodes/serializers.py +++ b/api/nodes/serializers.py @@ -46,7 +46,7 @@ from website.project.metadata.utils import is_prereg_admin_not_project_admin from website.project.model import NodeUpdateError from osf.utils import permissions as osf_permissions - +from osf.utils.requests import get_headers_from_request class RegistrationProviderRelationshipField(RelationshipField): def get_object(self, _id): @@ -890,7 +890,9 @@ def create(self, validated_data): class ForwardNodeAddonSettingsSerializer(NodeAddonSettingsSerializerBase): def update(self, instance, validated_data): - auth = Auth(self.context['request'].user) + request = self.context['request'] + user = request.user + auth = Auth(user) set_url = 'url' in validated_data set_label = 'label' in validated_data @@ -935,6 +937,7 @@ def update(self, instance, validated_data): auth=auth, save=True, ) + instance.owner.check_spam(user, {'addons_forward_node_settings__url'}, get_headers_from_request(request)) return instance diff --git a/osf/models/mixins.py b/osf/models/mixins.py index 8d85260b660..95f5cf98076 100644 --- a/osf/models/mixins.py +++ b/osf/models/mixins.py @@ -1593,7 +1593,8 @@ def _get_spam_content(self, saved_fields): spam_fields = self.get_spam_fields(saved_fields) content = [] for field in spam_fields: - content.append((getattr(self, field, None) or '').encode('utf-8')) + values = list(self.__class__.objects.filter(id=self.id).values_list(field, flat=True)) + content.append((' '.join(values) or '').encode('utf-8')) if self.all_tags.exists(): content.extend([name.encode('utf-8') for name in self.all_tags.values_list('name', flat=True)]) if not content: diff --git a/osf/models/node.py b/osf/models/node.py index 307a79f4636..096bcc6e00b 100644 --- a/osf/models/node.py +++ b/osf/models/node.py @@ -282,6 +282,7 @@ class AbstractNode(DirtyFieldsMixin, TypedModel, AddonModelMixin, IdentifierMixi SPAM_CHECK_FIELDS = { 'title', 'description', + 'addons_forward_node_settings__url' # the often spammed redirect URL } # Fields that are writable by Node.update From 2703303dae09581eb6aca7a3e6f339ffaaa6603f Mon Sep 17 00:00:00 2001 From: John Tordoff Date: Wed, 24 Jul 2019 10:33:32 -0400 Subject: [PATCH 06/42] change email subject line for user deactivation --- website/mails/mails.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/website/mails/mails.py b/website/mails/mails.py index 8439348f1c9..ba88c20fbda 100644 --- a/website/mails/mails.py +++ b/website/mails/mails.py @@ -251,7 +251,7 @@ def get_english_article(word): REQUEST_EXPORT = Mail('support_request', subject='[via OSF] Export Request') REQUEST_DEACTIVATION = Mail('support_request', subject='[via OSF] Deactivation Request') -REQUEST_DEACTIVATION_COMPLETE = Mail('request_deactivation_complete', subject='[via OSF] Deactivation Request') +REQUEST_DEACTIVATION_COMPLETE = Mail('request_deactivation_complete', subject='[via OSF] OSF account deactivated') SPAM_USER_BANNED = Mail('spam_user_banned', subject='[OSF] Account flagged as spam') From fda9bd1c040d003781c6f2c0bbdce27660573287 Mon Sep 17 00:00:00 2001 From: John Tordoff Date: Wed, 24 Jul 2019 10:36:12 -0400 Subject: [PATCH 07/42] fix and comment has resource to accomodate for deleted resources --- osf/models/user.py | 15 ++++++++++++--- 1 file changed, 12 insertions(+), 3 deletions(-) diff --git a/osf/models/user.py b/osf/models/user.py index a5ec1795a20..d88247c8776 100644 --- a/osf/models/user.py +++ b/osf/models/user.py @@ -369,7 +369,8 @@ class OSFUser(DirtyFieldsMixin, GuidMixin, BaseModel, AbstractBaseUser, Permissi # whether the user has requested to deactivate their account requested_deactivation = models.BooleanField(default=False) - # whether the user has who requested deactivation has been contacted + # whether the user has who requested deactivation has been contacted about their pending request. This is reset when + # requests are canceled contacted_deactivation = models.BooleanField(default=False) affiliated_institutions = models.ManyToManyField('Institution', blank=True) @@ -1710,12 +1711,20 @@ def gdpr_delete(self): @property def has_resources(self): + """ + This is meant to determine if a user has any resources, nodes, preprints etc that might impede their deactivation. + If a user only has no resources or only deleted resources this will return false and they can safely be deactivated + otherwise they must delete or transfer their outstanding resources. + + :return bool: does the user have any active node, preprints, groups, quickfiles etc? + """ # TODO: Update once quickfolders in merged - nodes = self.nodes.exclude(type='osf.quickfilesnode').exists() + nodes = self.nodes.exclude(type='osf.quickfilesnode').filter(deleted__isnull=True).exists() quickfiles = self.nodes.get(type='osf.quickfilesnode').files.exists() + groups = self.osf_groups.exists(_contributors=self, ever_public=True, deleted__isnull=True) - return nodes or quickfiles or self.preprints.exists() + return groups or nodes or quickfiles or self.preprints.exists() class Meta: # custom permissions for use in the OSF Admin App From dc117b20b9fb2e9dc69f29c18ac8ffeb73e05490 Mon Sep 17 00:00:00 2001 From: John Tordoff Date: Wed, 24 Jul 2019 10:37:24 -0400 Subject: [PATCH 08/42] reset contacted_deactivation to be false if user cancels deactivation --- website/profile/views.py | 1 + 1 file changed, 1 insertion(+) diff --git a/website/profile/views.py b/website/profile/views.py index 933b68396a1..0bfc0412cc4 100644 --- a/website/profile/views.py +++ b/website/profile/views.py @@ -823,5 +823,6 @@ def request_deactivation(auth): def cancel_request_deactivation(auth): user = auth.user user.requested_deactivation = False + user.contacted_deactivation = False # In case we've already contacted them once. user.save() return {'message': 'You have canceled your deactivation request'} From f325f95eb04c2c56f941cccf4284c218dc01e580 Mon Sep 17 00:00:00 2001 From: John Tordoff Date: Wed, 24 Jul 2019 10:58:36 -0400 Subject: [PATCH 09/42] make user deactivation celery script into management command, add docstrings --- .../deactivate_requested_accounts.py | 46 +++++++++++++------ ...=> 0174_osfuser_contacted_deactivation.py} | 2 +- scripts/periodic/__init__.py | 0 website/settings/defaults.py | 4 +- 4 files changed, 35 insertions(+), 17 deletions(-) rename {scripts/periodic => osf/management/commands}/deactivate_requested_accounts.py (53%) rename osf/migrations/{0162_osfuser_contacted_deactivation.py => 0174_osfuser_contacted_deactivation.py} (89%) delete mode 100644 scripts/periodic/__init__.py diff --git a/scripts/periodic/deactivate_requested_accounts.py b/osf/management/commands/deactivate_requested_accounts.py similarity index 53% rename from scripts/periodic/deactivate_requested_accounts.py rename to osf/management/commands/deactivate_requested_accounts.py index 64b43ad199e..2da79e302ed 100644 --- a/scripts/periodic/deactivate_requested_accounts.py +++ b/osf/management/commands/deactivate_requested_accounts.py @@ -1,8 +1,3 @@ -""" -This script runs nightly and emails users who want to delete there account with info on how to do so. Users who don't -have any content can be automatically deleted. -""" -import sys import logging from website import mails @@ -13,8 +8,7 @@ setup_django() from osf.models import OSFUser from website.settings import OSF_SUPPORT_EMAIL, OSF_CONTACT_EMAIL - -from scripts.utils import add_file_logger +from django.core.management.base import BaseCommand logger = logging.getLogger(__name__) logging.basicConfig(level=logging.INFO) @@ -43,15 +37,39 @@ def deactivate_requested_accounts(dry_run=True): user.contacted_deactivation = True user.email_last_sent = timezone.now() - user.save() + if not dry_run: + user.save() + + if dry_run: + logger.info('Dry run complete') + +@celery_app.task(name='management.commands.check_crossref_dois') +def main(dry_run=False): + """ + This task runs nightly and emails users who want to delete there account with info on how to do so. Users who don't + have any content can be automatically deleted. + """ -@celery_app.task(name='scripts.periodic.deactivate_requested_accounts') -def run_main(dry_run=True): - if not dry_run: - add_file_logger(logger, __file__) deactivate_requested_accounts(dry_run=dry_run) -if __name__ == '__main__': - run_main(dry_run='--dry' in sys.argv) +class Command(BaseCommand): + help = ''' + If there are any users who want to be deactivated we will either: immediately deactivate, or if they have active + resources (undeleted nodes, preprints etc) we contact admin to guide the user through the deactivation process. + ''' + + def add_arguments(self, parser): + super(Command, self).add_arguments(parser) + parser.add_argument( + '--dry', + action='store_true', + dest='dry_run', + help='Dry run', + ) + + # Management command handler + def handle(self, *args, **options): + dry_run = options.get('dry_run', True) + main(dry_run=dry_run) diff --git a/osf/migrations/0162_osfuser_contacted_deactivation.py b/osf/migrations/0174_osfuser_contacted_deactivation.py similarity index 89% rename from osf/migrations/0162_osfuser_contacted_deactivation.py rename to osf/migrations/0174_osfuser_contacted_deactivation.py index e8db0a83d3f..4c887fef0be 100644 --- a/osf/migrations/0162_osfuser_contacted_deactivation.py +++ b/osf/migrations/0174_osfuser_contacted_deactivation.py @@ -8,7 +8,7 @@ class Migration(migrations.Migration): dependencies = [ - ('osf', '0161_add_spam_fields_to_user'), + ('osf', '0173_ensure_schemas'), ] operations = [ diff --git a/scripts/periodic/__init__.py b/scripts/periodic/__init__.py deleted file mode 100644 index e69de29bb2d..00000000000 diff --git a/website/settings/defaults.py b/website/settings/defaults.py index e8bbd326f2d..419881bdc47 100644 --- a/website/settings/defaults.py +++ b/website/settings/defaults.py @@ -478,7 +478,7 @@ class CeleryConfig: 'scripts.generate_sitemap', 'scripts.premigrate_created_modified', 'scripts.add_missing_identifiers_to_preprints', - 'scripts.periodic.deactivate_requested_accounts', + 'management.commands.deactivate_requested_accounts', ) # Modules that need metrics and release requirements @@ -595,7 +595,7 @@ class CeleryConfig: 'schedule': crontab(minute=0, hour=5), # Daily 12:00 a.m. }, 'deactivate_requested_accounts': { - 'task': 'scripts.periodic.deactivate_requested_accounts', + 'task': 'management.commands.deactivate_requested_accounts', 'schedule': crontab(minute=0, hour=5), # Daily 12:00 a.m. }, } From 197c1bd8b285ec1be72d2c9972ae0672dd2b422c Mon Sep 17 00:00:00 2001 From: John Tordoff Date: Wed, 24 Jul 2019 12:03:19 -0400 Subject: [PATCH 10/42] fix has_resources more --- osf/models/user.py | 12 +++++++++--- 1 file changed, 9 insertions(+), 3 deletions(-) diff --git a/osf/models/user.py b/osf/models/user.py index dd55d2da41e..ad15d10cfa3 100644 --- a/osf/models/user.py +++ b/osf/models/user.py @@ -1798,13 +1798,19 @@ def has_resources(self): :return bool: does the user have any active node, preprints, groups, quickfiles etc? """ + from osf.models import Preprint + # TODO: Update once quickfolders in merged - nodes = self.nodes.exclude(type='osf.quickfilesnode').filter(deleted__isnull=True).exists() + nodes = self.nodes.exclude(type='osf.quickfilesnode', is_deleted=True).exists() quickfiles = self.nodes.get(type='osf.quickfilesnode').files.exists() - groups = self.osf_groups.exists(_contributors=self, ever_public=True, deleted__isnull=True) + groups = self.osf_groups.exists() + preprints = Preprint.objects.filter(_contributors=self, + ever_public=True, + deleted__isnull=True, + withdrawal_justification__isnull=False) - return groups or nodes or quickfiles or self.preprints.exists() + return groups or nodes or quickfiles or preprints class Meta: # custom permissions for use in the OSF Admin App From 51081b7836825cc86e56836c2d0d4d3fc0261412 Mon Sep 17 00:00:00 2001 From: John Tordoff Date: Wed, 24 Jul 2019 12:13:59 -0400 Subject: [PATCH 11/42] fix for travis --- .travis.yml | 1 + 1 file changed, 1 insertion(+) diff --git a/.travis.yml b/.travis.yml index 9eb7d369dd4..ef28085b46f 100644 --- a/.travis.yml +++ b/.travis.yml @@ -6,6 +6,7 @@ python: - "2.7" sudo: false +dist: trusty # TODO: uncomment when https://github.com/travis-ci/travis-ci/issues/8836 is resolved # addons: From 99b8e18c3d63290999ac6741eb95a2e5505f1e06 Mon Sep 17 00:00:00 2001 From: John Tordoff Date: Wed, 24 Jul 2019 12:50:12 -0400 Subject: [PATCH 12/42] seperate v1 and v2 tests --- addons/forward/tests/test_views.py | 46 ++++++++++-------------------- api/addons/forward/test_views.py | 36 +++++++++++++++++++++++ 2 files changed, 51 insertions(+), 31 deletions(-) create mode 100644 api/addons/forward/test_views.py diff --git a/addons/forward/tests/test_views.py b/addons/forward/tests/test_views.py index 435c47759d3..67e7696e5fa 100644 --- a/addons/forward/tests/test_views.py +++ b/addons/forward/tests/test_views.py @@ -32,29 +32,26 @@ def test_change_url_log_added(self): log_count + 1 ) - @mock.patch.object(settings, 'SPAM_CHECK_ENABLED', True) - @mock.patch('osf.models.node.Node.do_check_spam') - def test_change_url_check_spam_v1(self, mock_check_spam): - self.project.is_public = True - self.project.save() - self.app.put_json(self.project.api_url_for('forward_config_put'), {'url': 'http://possiblyspam.com'}) - - assert mock_check_spam.called - data, _ = mock_check_spam.call_args - author, author_email, content, request_headers = data - - assert author == self.user.fullname - assert author_email == self.user.username - assert content == 'http://possiblyspam.com' + def test_change_timeout_log_not_added(self): + log_count = self.project.logs.count() + self.app.put_json( + self.project.api_url_for('forward_config_put'), + dict( + url=self.node_settings.url, + ), + ) + self.project.reload() + assert_equal( + self.project.logs.count(), + log_count + ) @mock.patch.object(settings, 'SPAM_CHECK_ENABLED', True) @mock.patch('osf.models.node.Node.do_check_spam') - def test_change_url_check_spam_v2(self, mock_check_spam): + def test_change_url_check_spam(self, mock_check_spam): self.project.is_public = True self.project.save() - self.django_app.put_json_api('/v2/nodes/{}/addons/forward/'.format(self.project._id), - {'data': {'attributes': {'url': 'http://possiblyspam.com'}}}, - auth=self.user.auth) + self.app.put_json(self.project.api_url_for('forward_config_put'), {'url': 'http://possiblyspam.com'}) assert mock_check_spam.called data, _ = mock_check_spam.call_args @@ -64,16 +61,3 @@ def test_change_url_check_spam_v2(self, mock_check_spam): assert author_email == self.user.username assert content == 'http://possiblyspam.com' - def test_change_timeout_log_not_added(self): - log_count = self.project.logs.count() - self.app.put_json( - self.project.api_url_for('forward_config_put'), - dict( - url=self.node_settings.url, - ), - ) - self.project.reload() - assert_equal( - self.project.logs.count(), - log_count - ) diff --git a/api/addons/forward/test_views.py b/api/addons/forward/test_views.py new file mode 100644 index 00000000000..703357a2064 --- /dev/null +++ b/api/addons/forward/test_views.py @@ -0,0 +1,36 @@ +import mock +import pytest + +from addons.forward.tests.utils import ForwardAddonTestCase +from tests.base import OsfTestCase +from website import settings +from tests.json_api_test_app import JSONAPITestApp + +pytestmark = pytest.mark.django_db + +class TestForward(ForwardAddonTestCase, OsfTestCase): + + django_app = JSONAPITestApp() + + def setUp(self): + super(TestForward, self).setUp() + self.app.authenticate(*self.user.auth) + + @mock.patch.object(settings, 'SPAM_CHECK_ENABLED', True) + @mock.patch('osf.models.node.Node.do_check_spam') + def test_change_url_check_spam(self, mock_check_spam): + self.project.is_public = True + self.project.save() + self.django_app.put_json_api( + '/v2/nodes/{}/addons/forward/'.format(self.project._id), + {'data': {'attributes': {'url': 'http://possiblyspam.com'}}}, + auth=self.user.auth, + ) + + assert mock_check_spam.called + data, _ = mock_check_spam.call_args + author, author_email, content, request_headers = data + + assert author == self.user.fullname + assert author_email == self.user.username + assert content == 'http://possiblyspam.com' From a6bd6ac6796a9db6b5f6ec0240bc601cc58980ee Mon Sep 17 00:00:00 2001 From: John Tordoff Date: Wed, 24 Jul 2019 13:17:02 -0400 Subject: [PATCH 13/42] exclude null values from spam content look up --- osf/models/mixins.py | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/osf/models/mixins.py b/osf/models/mixins.py index 95f5cf98076..e3941b59c00 100644 --- a/osf/models/mixins.py +++ b/osf/models/mixins.py @@ -1593,8 +1593,10 @@ def _get_spam_content(self, saved_fields): spam_fields = self.get_spam_fields(saved_fields) content = [] for field in spam_fields: - values = list(self.__class__.objects.filter(id=self.id).values_list(field, flat=True)) - content.append((' '.join(values) or '').encode('utf-8')) + exclude_null = {field + '__isnull': False} + values = list(self.__class__.objects.filter(id=self.id, **exclude_null).values_list(field, flat=True)) + if values: + content.append((' '.join(values) or '').encode('utf-8')) if self.all_tags.exists(): content.extend([name.encode('utf-8') for name in self.all_tags.values_list('name', flat=True)]) if not content: From a310456f31b526c06a099f07193c77de23635753 Mon Sep 17 00:00:00 2001 From: John Tordoff Date: Wed, 24 Jul 2019 13:41:56 -0400 Subject: [PATCH 14/42] add for travis to pass --- .travis.yml | 1 + 1 file changed, 1 insertion(+) diff --git a/.travis.yml b/.travis.yml index 9eb7d369dd4..ef28085b46f 100644 --- a/.travis.yml +++ b/.travis.yml @@ -6,6 +6,7 @@ python: - "2.7" sudo: false +dist: trusty # TODO: uncomment when https://github.com/travis-ci/travis-ci/issues/8836 is resolved # addons: From 847a93f038d0e03880ad6051e59e1a808575bd12 Mon Sep 17 00:00:00 2001 From: John Tordoff Date: Wed, 24 Jul 2019 15:21:19 -0400 Subject: [PATCH 15/42] add functionality for node setting Forward route --- api/addons/forward/test_views.py | 34 ++++++++++++++++++++++++++++++++ api/nodes/serializers.py | 3 +++ 2 files changed, 37 insertions(+) diff --git a/api/addons/forward/test_views.py b/api/addons/forward/test_views.py index 703357a2064..f3190b447c3 100644 --- a/api/addons/forward/test_views.py +++ b/api/addons/forward/test_views.py @@ -9,6 +9,10 @@ pytestmark = pytest.mark.django_db class TestForward(ForwardAddonTestCase, OsfTestCase): + """ + Forward (the redirect url has two v2 routes, one is addon based `/v2/nodes/{}/addons/forward/` one is node settings + based `/v2/nodes/{}/settings/` they both need to be checked for spam each time they are used to modify a redirect url. + """ django_app = JSONAPITestApp() @@ -34,3 +38,33 @@ def test_change_url_check_spam(self, mock_check_spam): assert author == self.user.fullname assert author_email == self.user.username assert content == 'http://possiblyspam.com' + + @mock.patch.object(settings, 'SPAM_CHECK_ENABLED', True) + @mock.patch('osf.models.node.Node.do_check_spam') + def test_change_url_check_spam_node_settings(self, mock_check_spam): + self.project.is_public = True + self.project.save() + + payload = { + 'data': { + 'type': 'node-settings', + 'attributes': { + 'access_requests_enabled': False, + 'redirect_link_url': 'http://possiblyspam.com', + }, + }, + } + + self.django_app.put_json_api( + '/v2/nodes/{}/settings/'.format(self.project._id), + payload, + auth=self.user.auth, + ) + + assert mock_check_spam.called + data, _ = mock_check_spam.call_args + author, author_email, content, request_headers = data + + assert author == self.user.fullname + assert author_email == self.user.username + assert content == 'http://possiblyspam.com' diff --git a/api/nodes/serializers.py b/api/nodes/serializers.py index ac5c26084ca..d27be4c1611 100644 --- a/api/nodes/serializers.py +++ b/api/nodes/serializers.py @@ -1777,6 +1777,9 @@ def update_forward_fields(self, obj, validated_data, auth): if save_forward: forward_addon.save() + request = self.context['request'] + forward_addon.owner.check_spam(request.user, {'addons_forward_node_settings__url'}, get_headers_from_request(request)) + def enable_or_disable_addon(self, obj, should_enable, addon_name, auth): """ Returns addon, if exists, otherwise returns None From 19f048aa90019df2739f9d942dca153ad84a6f68 Mon Sep 17 00:00:00 2001 From: John Tordoff Date: Wed, 24 Jul 2019 15:35:09 -0400 Subject: [PATCH 16/42] deregister as well as deactivate them --- osf/management/commands/deactivate_requested_accounts.py | 1 + 1 file changed, 1 insertion(+) diff --git a/osf/management/commands/deactivate_requested_accounts.py b/osf/management/commands/deactivate_requested_accounts.py index 2da79e302ed..19602e21b72 100644 --- a/osf/management/commands/deactivate_requested_accounts.py +++ b/osf/management/commands/deactivate_requested_accounts.py @@ -27,6 +27,7 @@ def deactivate_requested_accounts(dry_run=True): ) else: user.disable_account() + user.is_registered = False mails.send_mail( to_addr=user.username, mail=mails.REQUEST_DEACTIVATION_COMPLETE, From fef1f7d0d3f3bc0a9adf4d945e32b9c0860dbf0a Mon Sep 17 00:00:00 2001 From: John Tordoff Date: Thu, 25 Jul 2019 15:12:05 -0400 Subject: [PATCH 17/42] fix docstring for exid clean up --- website/identifiers/utils.py | 6 +----- 1 file changed, 1 insertion(+), 5 deletions(-) diff --git a/website/identifiers/utils.py b/website/identifiers/utils.py index c0f75408d84..f97cfa041e8 100644 --- a/website/identifiers/utils.py +++ b/website/identifiers/utils.py @@ -32,11 +32,7 @@ def request_identifiers(target_object): """Request identifiers for the target object using the appropriate client. :param target_object: object to request identifiers for - :return: dict with keys relating to the status of the identifier - response - response from the DOI client - already_exists - the DOI has already been registered with a client - only_doi - boolean; only include the DOI (and not the ARK) identifier - when processing this response in get_or_create_identifiers + :return: dict with DOI """ from website.identifiers.clients import exceptions From 1bcb96d6358d650597a5bd659038bf6c2d066fce Mon Sep 17 00:00:00 2001 From: John Tordoff Date: Fri, 26 Jul 2019 14:33:49 -0400 Subject: [PATCH 18/42] .get identifiers instead of lookup --- website/identifiers/utils.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/website/identifiers/utils.py b/website/identifiers/utils.py index f97cfa041e8..987e3a99cbb 100644 --- a/website/identifiers/utils.py +++ b/website/identifiers/utils.py @@ -50,7 +50,7 @@ def request_identifiers(target_object): except exceptions.ClientResponseError as error: raise HTTPError(error.response.status_code) return { - 'doi': identifiers['doi'] + 'doi': identifiers.get('doi') } From 7bbac65e4c21acabc07f9af95879b66875826927 Mon Sep 17 00:00:00 2001 From: John Tordoff Date: Mon, 29 Jul 2019 09:44:23 -0400 Subject: [PATCH 19/42] de-randomize testmon value --- api_tests/nodes/views/test_node_wiki_list.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/api_tests/nodes/views/test_node_wiki_list.py b/api_tests/nodes/views/test_node_wiki_list.py index 7167aef45b7..27c3074813b 100644 --- a/api_tests/nodes/views/test_node_wiki_list.py +++ b/api_tests/nodes/views/test_node_wiki_list.py @@ -338,7 +338,7 @@ def test_create_public_wiki_page(self, app, user_write_contributor, url_node_pub assert res.json['data']['attributes']['name'] == page_name def test_create_public_wiki_page_with_content(self, app, user_write_contributor, url_node_public, project_public): - page_name = fake.word() + page_name = 'using random variables in tests can sometimes expose Testmon problems!' payload = create_wiki_payload(page_name) payload['data']['attributes']['content'] = 'my first wiki page' res = app.post_json_api(url_node_public, payload, auth=user_write_contributor.auth) From 647c1dcfa2dd199a01483f191016482a13fcb3ee Mon Sep 17 00:00:00 2001 From: John Tordoff Date: Wed, 31 Jul 2019 09:53:44 -0400 Subject: [PATCH 20/42] add more docs and remove line for askismet redirect link --- api/addons/forward/test_views.py | 3 --- osf/models/mixins.py | 8 ++++++++ 2 files changed, 8 insertions(+), 3 deletions(-) diff --git a/api/addons/forward/test_views.py b/api/addons/forward/test_views.py index f3190b447c3..95f30a16aba 100644 --- a/api/addons/forward/test_views.py +++ b/api/addons/forward/test_views.py @@ -4,7 +4,6 @@ from addons.forward.tests.utils import ForwardAddonTestCase from tests.base import OsfTestCase from website import settings -from tests.json_api_test_app import JSONAPITestApp pytestmark = pytest.mark.django_db @@ -14,8 +13,6 @@ class TestForward(ForwardAddonTestCase, OsfTestCase): based `/v2/nodes/{}/settings/` they both need to be checked for spam each time they are used to modify a redirect url. """ - django_app = JSONAPITestApp() - def setUp(self): super(TestForward, self).setUp() self.app.authenticate(*self.user.auth) diff --git a/osf/models/mixins.py b/osf/models/mixins.py index e3941b59c00..25de3b9dead 100644 --- a/osf/models/mixins.py +++ b/osf/models/mixins.py @@ -1590,6 +1590,14 @@ def confirm_spam(self, save=False): self.save() def _get_spam_content(self, saved_fields): + """ + This function retrieves retrieves strings of potential spam from various DB fields. Also here we can follow + django's typical ORM query structure for example we can grab the redirect link of a node by giving a saved + field of {'addons_forward_node_settings__url'}. + + :param saved_fields: set + :return: str + """ spam_fields = self.get_spam_fields(saved_fields) content = [] for field in spam_fields: From 92bca4aefcfb2922ea75166123b37691af8e4616 Mon Sep 17 00:00:00 2001 From: John Tordoff Date: Wed, 31 Jul 2019 10:08:05 -0400 Subject: [PATCH 21/42] remove travis trusty dist --- .travis.yml | 1 - 1 file changed, 1 deletion(-) diff --git a/.travis.yml b/.travis.yml index 251ed135e7a..9c6d46cb7dd 100644 --- a/.travis.yml +++ b/.travis.yml @@ -6,7 +6,6 @@ python: - "2.7" dist: trusty sudo: false -dist: trusty # TODO: uncomment when https://github.com/travis-ci/travis-ci/issues/8836 is resolved # addons: From cccf63f29aa6358095b38f78b92aa6ca2f2e7133 Mon Sep 17 00:00:00 2001 From: John Tordoff Date: Wed, 31 Jul 2019 12:17:00 -0400 Subject: [PATCH 22/42] validate url for NodeAddonSettingsSerializerBase --- api/addons/forward/test_views.py | 11 +++++++++++ api/nodes/serializers.py | 13 +++++++++---- 2 files changed, 20 insertions(+), 4 deletions(-) diff --git a/api/addons/forward/test_views.py b/api/addons/forward/test_views.py index f3190b447c3..5e3d3dbd7bd 100644 --- a/api/addons/forward/test_views.py +++ b/api/addons/forward/test_views.py @@ -68,3 +68,14 @@ def test_change_url_check_spam_node_settings(self, mock_check_spam): assert author == self.user.fullname assert author_email == self.user.username assert content == 'http://possiblyspam.com' + + def test_invalid_url(self): + res = self.django_app.put_json_api( + '/v2/nodes/{}/addons/forward/'.format(self.project._id), + {'data': {'attributes': {'url': 'bad url'}}}, + auth=self.user.auth, expect_errors=True, + ) + assert res.status_code == 400 + error = res.json['errors'][0] + + assert error['detail'] == 'Enter a valid URL.' diff --git a/api/nodes/serializers.py b/api/nodes/serializers.py index 141a56d6fc1..359957e64e9 100644 --- a/api/nodes/serializers.py +++ b/api/nodes/serializers.py @@ -863,7 +863,7 @@ class Meta: # Forward-specific label = ser.CharField(required=False, allow_blank=True) - url = ser.CharField(required=False, allow_blank=True) + url = ser.URLField(required=False, allow_blank=True) links = LinksField({ 'self': 'get_absolute_url', @@ -920,8 +920,10 @@ def update(self, instance, validated_data): instance.url = url instance.label = label url_changed = True - - instance.save() + try: + instance.save() + except ValidationError as e: + raise exceptions.ValidationError(detail=str(e)) if url_changed: # add log here because forward architecture isn't great @@ -1774,7 +1776,10 @@ def update_forward_fields(self, obj, validated_data, auth): save_forward = True if save_forward: - forward_addon.save() + try: + forward_addon.save() + except ValidationError as e: + raise exceptions.ValidationError(detail=str(e)) request = self.context['request'] forward_addon.owner.check_spam(request.user, {'addons_forward_node_settings__url'}, get_headers_from_request(request)) From d5549c35e1cc170cf70e0ff72761956fe9231371 Mon Sep 17 00:00:00 2001 From: John Tordoff Date: Wed, 31 Jul 2019 13:06:58 -0400 Subject: [PATCH 23/42] move check for spam into nade addon model --- addons/forward/models.py | 13 +++++++++++++ addons/forward/views/config.py | 3 +-- api/nodes/serializers.py | 12 ++++-------- 3 files changed, 18 insertions(+), 10 deletions(-) diff --git a/addons/forward/models.py b/addons/forward/models.py index 6cf0d8e8544..8cc518f7887 100644 --- a/addons/forward/models.py +++ b/addons/forward/models.py @@ -1,10 +1,12 @@ # -*- coding: utf-8 -*- +from osf.utils.requests import get_request_and_user_id, get_headers_from_request from addons.base.models import BaseNodeSettings from dirtyfields import DirtyFieldsMixin from django.db import models from osf.exceptions import ValidationValueError from osf.models.validators import validate_no_html +from osf.models import OSFUser class NodeSettings(DirtyFieldsMixin, BaseNodeSettings): @@ -33,6 +35,17 @@ def after_register(self, node, registration, user, save=True): return clone, None + def save(self, request=None, *args, **kwargs): + super(NodeSettings, self).save(*args, **kwargs) + if request: + if not hasattr(request, 'user'): # TODO: remove when Flask is removed + _, user_id = get_request_and_user_id() + user = OSFUser.load(user_id) + else: + user = request.user + + self.owner.check_spam(user, {'addons_forward_node_settings__url'}, get_headers_from_request(request)) + def clean(self): if self.url and self.owner._id in self.url: raise ValidationValueError('Circular URL') diff --git a/addons/forward/views/config.py b/addons/forward/views/config.py index 5b59eed77fa..e3e21872e84 100644 --- a/addons/forward/views/config.py +++ b/addons/forward/views/config.py @@ -44,7 +44,7 @@ def forward_config_put(auth, node_addon, **kwargs): # Save settings and get changed fields; crash if validation fails try: dirty_fields = node_addon.get_dirty_fields() - node_addon.save() + node_addon.save(request=request) except ValidationValueError: raise HTTPError(http.BAD_REQUEST) @@ -60,6 +60,5 @@ def forward_config_put(auth, node_addon, **kwargs): auth=auth, save=True, ) - node_addon.owner.check_spam(auth.user, {'addons_forward_node_settings__url'}, request.headers) return {} diff --git a/api/nodes/serializers.py b/api/nodes/serializers.py index 359957e64e9..e0855f16c86 100644 --- a/api/nodes/serializers.py +++ b/api/nodes/serializers.py @@ -45,7 +45,7 @@ from website.project.metadata.utils import is_prereg_admin_not_project_admin from website.project.model import NodeUpdateError from osf.utils import permissions as osf_permissions -from osf.utils.requests import get_headers_from_request + class RegistrationProviderRelationshipField(RelationshipField): def get_object(self, _id): @@ -920,8 +920,9 @@ def update(self, instance, validated_data): instance.url = url instance.label = label url_changed = True + try: - instance.save() + instance.save(request=request) except ValidationError as e: raise exceptions.ValidationError(detail=str(e)) @@ -938,8 +939,6 @@ def update(self, instance, validated_data): auth=auth, save=True, ) - instance.owner.check_spam(user, {'addons_forward_node_settings__url'}, get_headers_from_request(request)) - return instance @@ -1777,13 +1776,10 @@ def update_forward_fields(self, obj, validated_data, auth): if save_forward: try: - forward_addon.save() + forward_addon.save(request=self.context['request']) except ValidationError as e: raise exceptions.ValidationError(detail=str(e)) - request = self.context['request'] - forward_addon.owner.check_spam(request.user, {'addons_forward_node_settings__url'}, get_headers_from_request(request)) - def enable_or_disable_addon(self, obj, should_enable, addon_name, auth): """ Returns addon, if exists, otherwise returns None From f7d1ebe4255b39ef66194d2d891a88f37880249c Mon Sep 17 00:00:00 2001 From: John Tordoff Date: Thu, 1 Aug 2019 09:40:22 -0400 Subject: [PATCH 24/42] put emails being sent in a dry run conditional --- .../commands/deactivate_requested_accounts.py | 30 +++++++++++-------- 1 file changed, 17 insertions(+), 13 deletions(-) diff --git a/osf/management/commands/deactivate_requested_accounts.py b/osf/management/commands/deactivate_requested_accounts.py index 19602e21b72..bcf00afe04f 100644 --- a/osf/management/commands/deactivate_requested_accounts.py +++ b/osf/management/commands/deactivate_requested_accounts.py @@ -19,22 +19,26 @@ def deactivate_requested_accounts(dry_run=True): for user in users: if user.has_resources: - mails.send_mail( - to_addr=OSF_SUPPORT_EMAIL, - mail=mails.REQUEST_DEACTIVATION, - user=user, - can_change_preferences=False, - ) + logger.info('User {} was contacted for about deactivation.'.format(user._id)) + if not dry_run: + mails.send_mail( + to_addr=OSF_SUPPORT_EMAIL, + mail=mails.REQUEST_DEACTIVATION, + user=user, + can_change_preferences=False, + ) else: user.disable_account() user.is_registered = False - mails.send_mail( - to_addr=user.username, - mail=mails.REQUEST_DEACTIVATION_COMPLETE, - user=user, - contact_email=OSF_CONTACT_EMAIL, - can_change_preferences=False, - ) + logger.info('User {} was disabled'.format(user._id)) + if not dry_run: + mails.send_mail( + to_addr=user.username, + mail=mails.REQUEST_DEACTIVATION_COMPLETE, + user=user, + contact_email=OSF_CONTACT_EMAIL, + can_change_preferences=False, + ) user.contacted_deactivation = True user.email_last_sent = timezone.now() From c1dffdb63d87967309ade66dc7140307a57b0509 Mon Sep 17 00:00:00 2001 From: John Tordoff Date: Thu, 1 Aug 2019 09:57:25 -0400 Subject: [PATCH 25/42] v2 user requested contact deactivation to false --- api/users/serializers.py | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/api/users/serializers.py b/api/users/serializers.py index 943d99e6b93..50b3147fbb4 100644 --- a/api/users/serializers.py +++ b/api/users/serializers.py @@ -525,9 +525,13 @@ def verify_two_factor(self, instance, value, two_factor_addon): two_factor_addon.save() def request_deactivation(self, instance, requested_deactivation): - if instance.requested_deactivation != requested_deactivation: + if not requested_deactivation: + instance.contacted_deactivation = False + elif instance.requested_deactivation != requested_deactivation: instance.requested_deactivation = requested_deactivation - instance.save() + else: + return + instance.save() def to_representation(self, instance): """ From 297eedeef54cc999895cc8e6ce1935305bb36e8a Mon Sep 17 00:00:00 2001 From: John Tordoff Date: Thu, 1 Aug 2019 10:11:10 -0400 Subject: [PATCH 26/42] remove withdrawl justification filter for has resource --- osf/models/user.py | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-) diff --git a/osf/models/user.py b/osf/models/user.py index ad15d10cfa3..b2d871b47ca 100644 --- a/osf/models/user.py +++ b/osf/models/user.py @@ -7,6 +7,8 @@ from copy import deepcopy from os.path import splitext +from past.builtins import basestring + from flask import Request as FlaskRequest from framework import analytics from guardian.shortcuts import get_perms @@ -1805,10 +1807,7 @@ def has_resources(self): nodes = self.nodes.exclude(type='osf.quickfilesnode', is_deleted=True).exists() quickfiles = self.nodes.get(type='osf.quickfilesnode').files.exists() groups = self.osf_groups.exists() - preprints = Preprint.objects.filter(_contributors=self, - ever_public=True, - deleted__isnull=True, - withdrawal_justification__isnull=False) + preprints = Preprint.objects.filter(_contributors=self, ever_public=True, deleted__isnull=True).exists() return groups or nodes or quickfiles or preprints From 4684f95027100a93455475f114a97193b5dbd165 Mon Sep 17 00:00:00 2001 From: John Tordoff Date: Thu, 1 Aug 2019 10:16:47 -0400 Subject: [PATCH 27/42] fix merge migrations --- ...d_deactivation.py => 0175_osfuser_contacted_deactivation.py} | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) rename osf/migrations/{0174_osfuser_contacted_deactivation.py => 0175_osfuser_contacted_deactivation.py} (86%) diff --git a/osf/migrations/0174_osfuser_contacted_deactivation.py b/osf/migrations/0175_osfuser_contacted_deactivation.py similarity index 86% rename from osf/migrations/0174_osfuser_contacted_deactivation.py rename to osf/migrations/0175_osfuser_contacted_deactivation.py index 4c887fef0be..c5049dfaf49 100644 --- a/osf/migrations/0174_osfuser_contacted_deactivation.py +++ b/osf/migrations/0175_osfuser_contacted_deactivation.py @@ -8,7 +8,7 @@ class Migration(migrations.Migration): dependencies = [ - ('osf', '0173_ensure_schemas'), + ('osf', '0174_add_ab_testing_home_page_version_b_flag'), ] operations = [ From 7bbb8589b17096bf2ff428bec2e39020f20e65bc Mon Sep 17 00:00:00 2001 From: John Tordoff Date: Thu, 1 Aug 2019 10:23:59 -0400 Subject: [PATCH 28/42] fix import typos for automatic account deactivation --- osf/management/commands/deactivate_requested_accounts.py | 2 +- website/settings/defaults.py | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/osf/management/commands/deactivate_requested_accounts.py b/osf/management/commands/deactivate_requested_accounts.py index bcf00afe04f..83f3e9a8afb 100644 --- a/osf/management/commands/deactivate_requested_accounts.py +++ b/osf/management/commands/deactivate_requested_accounts.py @@ -49,7 +49,7 @@ def deactivate_requested_accounts(dry_run=True): logger.info('Dry run complete') -@celery_app.task(name='management.commands.check_crossref_dois') +@celery_app.task(name='management.commands.deactivate_requested_accounts') def main(dry_run=False): """ This task runs nightly and emails users who want to delete there account with info on how to do so. Users who don't diff --git a/website/settings/defaults.py b/website/settings/defaults.py index 819dbfa3aea..86bf9c60faa 100644 --- a/website/settings/defaults.py +++ b/website/settings/defaults.py @@ -486,7 +486,7 @@ class CeleryConfig: 'scripts.generate_sitemap', 'scripts.premigrate_created_modified', 'scripts.add_missing_identifiers_to_preprints', - 'management.commands.deactivate_requested_accounts', + 'osf.management.commands.deactivate_requested_accounts', ) # Modules that need metrics and release requirements From 635c462eee26c5019bfbb19c7697a1c3d4a18ade Mon Sep 17 00:00:00 2001 From: John Tordoff Date: Thu, 1 Aug 2019 10:25:48 -0400 Subject: [PATCH 29/42] fix query for nodes of has_resources --- osf/models/user.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/osf/models/user.py b/osf/models/user.py index b2d871b47ca..9969b6ae514 100644 --- a/osf/models/user.py +++ b/osf/models/user.py @@ -1804,7 +1804,7 @@ def has_resources(self): # TODO: Update once quickfolders in merged - nodes = self.nodes.exclude(type='osf.quickfilesnode', is_deleted=True).exists() + nodes = self.nodes.exclude(type='osf.quickfilesnode').filter(is_deleted=True) quickfiles = self.nodes.get(type='osf.quickfilesnode').files.exists() groups = self.osf_groups.exists() preprints = Preprint.objects.filter(_contributors=self, ever_public=True, deleted__isnull=True).exists() From 05a21cba771d5e9fb13663e5bcc73584b9f612db Mon Sep 17 00:00:00 2001 From: John Tordoff Date: Thu, 1 Aug 2019 11:16:43 -0400 Subject: [PATCH 30/42] reset contacted_deactivation for v2 account deactivation --- api/users/serializers.py | 7 ++++--- api_tests/users/views/test_user_settings_detail.py | 10 ++++++++++ 2 files changed, 14 insertions(+), 3 deletions(-) diff --git a/api/users/serializers.py b/api/users/serializers.py index 50b3147fbb4..bdd433b06cd 100644 --- a/api/users/serializers.py +++ b/api/users/serializers.py @@ -427,6 +427,7 @@ class UserSettingsSerializer(JSONAPISerializer): subscribe_osf_general_email = ser.SerializerMethodField() subscribe_osf_help_email = ser.SerializerMethodField() deactivation_requested = ser.BooleanField(source='requested_deactivation', required=False) + contacted_deactivation = ser.BooleanField(required=False) secret = ser.SerializerMethodField(read_only=True) def to_representation(self, instance): @@ -527,10 +528,10 @@ def verify_two_factor(self, instance, value, two_factor_addon): def request_deactivation(self, instance, requested_deactivation): if not requested_deactivation: instance.contacted_deactivation = False - elif instance.requested_deactivation != requested_deactivation: + + if instance.requested_deactivation != requested_deactivation: instance.requested_deactivation = requested_deactivation - else: - return + instance.save() def to_representation(self, instance): diff --git a/api_tests/users/views/test_user_settings_detail.py b/api_tests/users/views/test_user_settings_detail.py index d0433037a41..873c1ab737d 100644 --- a/api_tests/users/views/test_user_settings_detail.py +++ b/api_tests/users/views/test_user_settings_detail.py @@ -269,6 +269,16 @@ def test_patch_requested_deactivation(self, mock_mail, app, user_one, user_two, user_one.reload() assert user_one.requested_deactivation is False + # cancel deactivate request after contacted reset deactivation contacted bool + user_one.contacted_deactivation = True + user_one.save() + payload['data']['attributes']['deactivation_requested'] = False + res = app.patch_json_api(url, payload, auth=user_one.auth) + assert res.status_code == 200 + user_one.reload() + assert user_one.requested_deactivation is False + assert user_one.contacted_deactivation is False + @mock.patch('framework.auth.views.mails.send_mail') def test_patch_invalid_type(self, mock_mail, app, user_one, url, payload): assert user_one.email_last_sent is None From 375f4a6d120bd501416e000398c48832770c0e23 Mon Sep 17 00:00:00 2001 From: John Tordoff Date: Fri, 2 Aug 2019 10:51:41 -0400 Subject: [PATCH 31/42] wake up Travis, your cache is cleared From 4dacb01c740b1486c7fa86a324b2798252665ec2 Mon Sep 17 00:00:00 2001 From: John Tordoff Date: Wed, 7 Aug 2019 09:43:46 -0400 Subject: [PATCH 32/42] move conditional for automatic account deactivation --- api/users/serializers.py | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-) diff --git a/api/users/serializers.py b/api/users/serializers.py index bdd433b06cd..aecf0bcfd88 100644 --- a/api/users/serializers.py +++ b/api/users/serializers.py @@ -526,13 +526,12 @@ def verify_two_factor(self, instance, value, two_factor_addon): two_factor_addon.save() def request_deactivation(self, instance, requested_deactivation): - if not requested_deactivation: - instance.contacted_deactivation = False if instance.requested_deactivation != requested_deactivation: instance.requested_deactivation = requested_deactivation - - instance.save() + if not requested_deactivation: + instance.contacted_deactivation = False + instance.save() def to_representation(self, instance): """ From 55c984436eded4f5f3d5013189daebc931bec29e Mon Sep 17 00:00:00 2001 From: John Tordoff Date: Wed, 7 Aug 2019 09:46:40 -0400 Subject: [PATCH 33/42] fix oops on exists for automatic account deactivation --- osf/models/user.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/osf/models/user.py b/osf/models/user.py index 9969b6ae514..4fb19761bd1 100644 --- a/osf/models/user.py +++ b/osf/models/user.py @@ -1804,7 +1804,7 @@ def has_resources(self): # TODO: Update once quickfolders in merged - nodes = self.nodes.exclude(type='osf.quickfilesnode').filter(is_deleted=True) + nodes = self.nodes.exclude(type='osf.quickfilesnode').filter(is_deleted=True).exists() quickfiles = self.nodes.get(type='osf.quickfilesnode').files.exists() groups = self.osf_groups.exists() preprints = Preprint.objects.filter(_contributors=self, ever_public=True, deleted__isnull=True).exists() From 60bab810a7841d66237098cd6e6d9f82158e723e Mon Sep 17 00:00:00 2001 From: John Tordoff Date: Wed, 7 Aug 2019 09:49:53 -0400 Subject: [PATCH 34/42] fix logging language for account deactivation scripts --- osf/management/commands/deactivate_requested_accounts.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/osf/management/commands/deactivate_requested_accounts.py b/osf/management/commands/deactivate_requested_accounts.py index 83f3e9a8afb..2a6f77ef7f4 100644 --- a/osf/management/commands/deactivate_requested_accounts.py +++ b/osf/management/commands/deactivate_requested_accounts.py @@ -19,7 +19,7 @@ def deactivate_requested_accounts(dry_run=True): for user in users: if user.has_resources: - logger.info('User {} was contacted for about deactivation.'.format(user._id)) + logger.info('OSF support is being emailed about deactivating the account of user {}.'.format(user._id)) if not dry_run: mails.send_mail( to_addr=OSF_SUPPORT_EMAIL, From c0be24e0470832118a7f43454c0c657371520196 Mon Sep 17 00:00:00 2001 From: John Tordoff Date: Wed, 7 Aug 2019 09:55:05 -0400 Subject: [PATCH 35/42] catch extraneous trusty for auto account deactivation --- .travis.yml | 1 - 1 file changed, 1 deletion(-) diff --git a/.travis.yml b/.travis.yml index 251ed135e7a..9c6d46cb7dd 100644 --- a/.travis.yml +++ b/.travis.yml @@ -6,7 +6,6 @@ python: - "2.7" dist: trusty sudo: false -dist: trusty # TODO: uncomment when https://github.com/travis-ci/travis-ci/issues/8836 is resolved # addons: From 3e762ba02baad3147c7751d31e4926f4cc8d73a2 Mon Sep 17 00:00:00 2001 From: John Tordoff Date: Wed, 7 Aug 2019 10:10:27 -0400 Subject: [PATCH 36/42] fix tests and imports for moving account deactivation script to management command --- osf/models/user.py | 2 +- scripts/tests/test_deactivate_requested_accounts.py | 6 +++--- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/osf/models/user.py b/osf/models/user.py index 4fb19761bd1..ea84c093293 100644 --- a/osf/models/user.py +++ b/osf/models/user.py @@ -1804,7 +1804,7 @@ def has_resources(self): # TODO: Update once quickfolders in merged - nodes = self.nodes.exclude(type='osf.quickfilesnode').filter(is_deleted=True).exists() + nodes = self.nodes.exclude(type='osf.quickfilesnode').exclude(is_deleted=True).exists() quickfiles = self.nodes.get(type='osf.quickfilesnode').files.exists() groups = self.osf_groups.exists() preprints = Preprint.objects.filter(_contributors=self, ever_public=True, deleted__isnull=True).exists() diff --git a/scripts/tests/test_deactivate_requested_accounts.py b/scripts/tests/test_deactivate_requested_accounts.py index 197c093d67a..300f9ea5570 100644 --- a/scripts/tests/test_deactivate_requested_accounts.py +++ b/scripts/tests/test_deactivate_requested_accounts.py @@ -7,7 +7,7 @@ from osf_tests.factories import ProjectFactory, AuthUserFactory -from scripts.periodic.deactivate_requested_accounts import deactivate_requested_accounts +from osf.management.commands.deactivate_requested_accounts import deactivate_requested_accounts from website import mails, settings @@ -30,7 +30,7 @@ def user_requested_deactivation_with_node(self): user.save() return user - @mock.patch('scripts.periodic.deactivate_requested_accounts.mails.send_mail') + @mock.patch('osf.management.commands.deactivate_requested_accounts.mails.send_mail') def test_deactivate_user_with_no_content(self, mock_mail, user_requested_deactivation): deactivate_requested_accounts(dry_run=False) @@ -45,7 +45,7 @@ def test_deactivate_user_with_no_content(self, mock_mail, user_requested_deactiv contact_email=settings.OSF_CONTACT_EMAIL, user=user_requested_deactivation) - @mock.patch('scripts.periodic.deactivate_requested_accounts.mails.send_mail') + @mock.patch('osf.management.commands.deactivate_requested_accounts.mails.send_mail') def test_deactivate_user_with_content(self, mock_mail, user_requested_deactivation_with_node): deactivate_requested_accounts(dry_run=False) From 5588d13dc5f7292851347014fa79ddf7395fcbd9 Mon Sep 17 00:00:00 2001 From: John Tordoff Date: Wed, 7 Aug 2019 10:11:01 -0400 Subject: [PATCH 37/42] make contacted_deactivation serializer field read only --- api/users/serializers.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/api/users/serializers.py b/api/users/serializers.py index aecf0bcfd88..a7beacd096f 100644 --- a/api/users/serializers.py +++ b/api/users/serializers.py @@ -427,7 +427,7 @@ class UserSettingsSerializer(JSONAPISerializer): subscribe_osf_general_email = ser.SerializerMethodField() subscribe_osf_help_email = ser.SerializerMethodField() deactivation_requested = ser.BooleanField(source='requested_deactivation', required=False) - contacted_deactivation = ser.BooleanField(required=False) + contacted_deactivation = ser.BooleanField(required=False, read_only=True) secret = ser.SerializerMethodField(read_only=True) def to_representation(self, instance): From c09bf8f454a163a328d9a5d653d177cbd8dad4b3 Mon Sep 17 00:00:00 2001 From: John Tordoff Date: Wed, 7 Aug 2019 10:58:07 -0400 Subject: [PATCH 38/42] remove impossible test case for automatic account deactivation --- api_tests/users/views/test_user_settings_detail.py | 10 ---------- 1 file changed, 10 deletions(-) diff --git a/api_tests/users/views/test_user_settings_detail.py b/api_tests/users/views/test_user_settings_detail.py index 873c1ab737d..d0433037a41 100644 --- a/api_tests/users/views/test_user_settings_detail.py +++ b/api_tests/users/views/test_user_settings_detail.py @@ -269,16 +269,6 @@ def test_patch_requested_deactivation(self, mock_mail, app, user_one, user_two, user_one.reload() assert user_one.requested_deactivation is False - # cancel deactivate request after contacted reset deactivation contacted bool - user_one.contacted_deactivation = True - user_one.save() - payload['data']['attributes']['deactivation_requested'] = False - res = app.patch_json_api(url, payload, auth=user_one.auth) - assert res.status_code == 200 - user_one.reload() - assert user_one.requested_deactivation is False - assert user_one.contacted_deactivation is False - @mock.patch('framework.auth.views.mails.send_mail') def test_patch_invalid_type(self, mock_mail, app, user_one, url, payload): assert user_one.email_last_sent is None From 7e359eedef76c966a1ca5541cdba28bc7f677ebf Mon Sep 17 00:00:00 2001 From: Dawn Pattison Date: Wed, 28 Aug 2019 14:08:01 -0500 Subject: [PATCH 39/42] Rebase migration. --- ...d_deactivation.py => 0181_osfuser_contacted_deactivation.py} | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) rename osf/migrations/{0175_osfuser_contacted_deactivation.py => 0181_osfuser_contacted_deactivation.py} (86%) diff --git a/osf/migrations/0175_osfuser_contacted_deactivation.py b/osf/migrations/0181_osfuser_contacted_deactivation.py similarity index 86% rename from osf/migrations/0175_osfuser_contacted_deactivation.py rename to osf/migrations/0181_osfuser_contacted_deactivation.py index c5049dfaf49..046f2fe968b 100644 --- a/osf/migrations/0175_osfuser_contacted_deactivation.py +++ b/osf/migrations/0181_osfuser_contacted_deactivation.py @@ -8,7 +8,7 @@ class Migration(migrations.Migration): dependencies = [ - ('osf', '0174_add_ab_testing_home_page_version_b_flag'), + ('osf', '0180_finalize_token_scopes_mig'), ] operations = [ From 6464054e07bb6a8a91a2e20abf85e2548741ecf8 Mon Sep 17 00:00:00 2001 From: Dawn Pattison Date: Wed, 4 Sep 2019 13:23:54 -0500 Subject: [PATCH 40/42] Only disable a user's account/mark as not registered if not a dry_run - in management command to deactivate requested accounts. --- osf/management/commands/deactivate_requested_accounts.py | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/osf/management/commands/deactivate_requested_accounts.py b/osf/management/commands/deactivate_requested_accounts.py index 2a6f77ef7f4..25ef179985b 100644 --- a/osf/management/commands/deactivate_requested_accounts.py +++ b/osf/management/commands/deactivate_requested_accounts.py @@ -28,10 +28,10 @@ def deactivate_requested_accounts(dry_run=True): can_change_preferences=False, ) else: - user.disable_account() - user.is_registered = False - logger.info('User {} was disabled'.format(user._id)) + logger.info('Disabling user {}.'.format(user._id)) if not dry_run: + user.disable_account() + user.is_registered = False mails.send_mail( to_addr=user.username, mail=mails.REQUEST_DEACTIVATION_COMPLETE, @@ -55,7 +55,8 @@ def main(dry_run=False): This task runs nightly and emails users who want to delete there account with info on how to do so. Users who don't have any content can be automatically deleted. """ - + if dry_run: + logger.info('This is a dry run; no changes will be saved, and no emails will be sent.') deactivate_requested_accounts(dry_run=dry_run) From 4b2f96a7e11645e4ef3db07a503fe530299637c6 Mon Sep 17 00:00:00 2001 From: Dawn Pattison Date: Wed, 4 Sep 2019 13:25:10 -0500 Subject: [PATCH 41/42] Remove redefinition of basestring. --- osf/models/user.py | 2 -- 1 file changed, 2 deletions(-) diff --git a/osf/models/user.py b/osf/models/user.py index b659a637774..9bce511537d 100644 --- a/osf/models/user.py +++ b/osf/models/user.py @@ -7,8 +7,6 @@ from copy import deepcopy from os.path import splitext -from past.builtins import basestring - from flask import Request as FlaskRequest from framework import analytics from guardian.shortcuts import get_perms From 82dbb969005654113aab0c8cc511fdcfdc1209b6 Mon Sep 17 00:00:00 2001 From: Dawn Pattison Date: Thu, 5 Sep 2019 16:17:03 -0500 Subject: [PATCH 42/42] Bump version and update changelog. --- CHANGELOG | 6 ++++++ package.json | 2 +- 2 files changed, 7 insertions(+), 1 deletion(-) diff --git a/CHANGELOG b/CHANGELOG index 791c594bb10..c0eadf5b56f 100644 --- a/CHANGELOG +++ b/CHANGELOG @@ -2,6 +2,12 @@ We follow the CalVer (https://calver.org/) versioning scheme: YY.MINOR.MICRO. +19.25.0 (2019-09-05) +=================== +- Automate account deactivation if users have no content +- Clean up EZID workflow +- Check redirect URL's for spam + 19.24.0 (2019-08-27) =================== - APIv2: Allow creating a node with a license attached on creation diff --git a/package.json b/package.json index c5ea698507d..c5f42c3ccf2 100644 --- a/package.json +++ b/package.json @@ -1,6 +1,6 @@ { "name": "OSF", - "version": "19.24.0", + "version": "19.25.0", "description": "Facilitating Open Science", "repository": "https://github.com/CenterForOpenScience/osf.io", "author": "Center for Open Science",