Merge branch 'release/19.25.0'

Author: pattisdr

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

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

addons/forward/tests/test_views.py

@@ -1,16 +1,18 @@
+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
 
 pytestmark = pytest.mark.django_db
 
-class TestForwardLogs(ForwardAddonTestCase, OsfTestCase):
+class TestForward(ForwardAddonTestCase, OsfTestCase):
 
     def setUp(self):
-        super(TestForwardLogs, self).setUp()
+        super(TestForward, self).setUp()
         self.app.authenticate(*self.user.auth)
 
     def test_change_url_log_added(self):
@@ -40,3 +42,19 @@ def test_change_timeout_log_not_added(self):
             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(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'
+

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)
 

api/addons/forward/test_views.py

@@ -0,0 +1,81 @@
+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):
+    """
+    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()
+
+    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'
+
+    @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'
+
+    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.'

api/nodes/serializers.py

@@ -897,7 +897,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',
@@ -923,7 +923,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
 
@@ -953,7 +955,10 @@ def update(self, instance, validated_data):
             instance.label = label
             url_changed = True
 
-        instance.save()
+        try:
+            instance.save(request=request)
+        except ValidationError as e:
+            raise exceptions.ValidationError(detail=str(e))
 
         if url_changed:
             # add log here because forward architecture isn't great
@@ -968,7 +973,6 @@ def update(self, instance, validated_data):
                 auth=auth,
                 save=True,
             )
-
         return instance
 
 
@@ -1805,7 +1809,10 @@ def update_forward_fields(self, obj, validated_data, auth):
             save_forward = True
 
         if save_forward:
-            forward_addon.save()
+            try:
+                forward_addon.save(request=self.context['request'])
+            except ValidationError as e:
+                raise exceptions.ValidationError(detail=str(e))
 
     def enable_or_disable_addon(self, obj, should_enable, addon_name, auth):
         """

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
@@ -428,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, read_only=True)
     secret = ser.SerializerMethodField(read_only=True)
 
     def to_representation(self, instance):
@@ -526,18 +526,12 @@ 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 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
+            if not requested_deactivation:
+                instance.contacted_deactivation = False
             instance.save()
-        return
 
     def to_representation(self, instance):
         """

api_tests/nodes/views/test_node_detail.py

@@ -1348,8 +1348,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')

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)

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