diff --git a/admin/base/urls.py b/admin/base/urls.py index c63c9d20fbd..85946577196 100644 --- a/admin/base/urls.py +++ b/admin/base/urls.py @@ -1,62 +1,70 @@ from django.conf.urls import include, url from django.contrib import admin -from admin.base.settings import ADMIN_BASE, DEBUG +from admin.base.settings import ADMIN_BASE, DEBUG, ENABLE_LOGIN_FORM from admin.base import views base_pattern = '^{}'.format(ADMIN_BASE) +admin_urls = [ + url(r'^$', views.home, name='home'), + url(r'^admin/', admin.site.urls), + url(r'^asset_files/', include('admin.asset_files.urls', namespace='asset_files')), + url(r'^banners/', include('admin.banners.urls', namespace='banners')), + url(r'^brands/', include('admin.brands.urls', namespace='brands')), + url(r'^spam/', include('admin.spam.urls', namespace='spam')), + url(r'^institutions/', include('admin.institutions.urls', namespace='institutions')), + url(r'^entitlements/', include('admin.entitlements.urls', namespace='entitlements')), + url(r'^quota_recalc/', include('admin.quota_recalc.urls', namespace='quota_recalc')), + url(r'^preprint_providers/', include('admin.preprint_providers.urls', namespace='preprint_providers')), + url(r'^collection_providers/', include('admin.collection_providers.urls', namespace='collection_providers')), + url(r'^registration_providers/', include('admin.registration_providers.urls', namespace='registration_providers')), + url(r'^account/', include('admin.common_auth.urls', namespace='auth')), +] + +if ENABLE_LOGIN_FORM: + # password recovery/reset only makes sense while password-based login itself is enabled + admin_urls.append(url(r'^password/', include('password_reset.urls'))) + +admin_urls += [ + url(r'^nodes/', include('admin.nodes.urls', namespace='nodes')), + url(r'^preprints/', include('admin.preprints.urls', namespace='preprints')), + url(r'^subjects/', include('admin.subjects.urls', namespace='subjects')), + url(r'^users/', include('admin.users.urls', namespace='users')), + url(r'^user-emails/', include('admin.user_emails.urls', namespace='user-emails')), + url(r'^maintenance/', include('admin.maintenance.urls', namespace='maintenance')), + url(r'^meetings/', include('admin.meetings.urls', + namespace='meetings')), + url(r'^metrics/', include('admin.metrics.urls', + namespace='metrics')), + url(r'^desk/', include('admin.desk.urls', + namespace='desk')), + url(r'^osf_groups/', include('admin.osf_groups.urls', namespace='osf_groups')), + url(r'^management/', include('admin.management.urls', namespace='management')), + url(r'^announcement/', include('admin.rdm_announcement.urls', namespace='announcement')), + url(r'^addons/', include('admin.rdm_addons.urls', namespace='addons')), + url(r'^oauth/', include('admin.rdm_addons.oauth.urls', namespace='oauth')), + url(r'^statistics/', include('admin.rdm_statistics.urls', namespace='statistics')), + url(r'^timestampadd/', include('admin.rdm_timestampadd.urls', namespace='timestampadd')), + url(r'^keymanagement/', include('admin.rdm_keymanagement.urls', namespace='keymanagement')), + url(r'^timestampsettings/', include('admin.rdm_timestampsettings.urls', namespace='timestampsettings')), + url(r'^custom_storage_location/', include('admin.rdm_custom_storage_location.urls', namespace='custom_storage_location')), + url(r'^institutional_storage_quota_control/', include('admin.institutional_storage_quota_control.urls', + namespace='institutional_storage_quota_control')), + url(r'^metadata/', include('admin.rdm_metadata.urls', namespace='metadata')), + url(r'^user_identification_information/', + include('admin.user_identification_information.urls', namespace='user_identification_information')), + url(r'^user_identification_information_admin/', + include('admin.user_identification_information_admin.urls', namespace='user_identification_information_admin')), + url(r'^project_limit_number/', include('admin.project_limit_number.urls', namespace='project_limit_number')), + url(r'^rdm_workflow/', include('admin.rdm_workflow.urls', namespace='rdm_workflow')), + url(r'^loa/', include('admin.loa.urls', namespace='loa')), +] + urlpatterns = [ ### ADMIN ### url( base_pattern, - include([ - url(r'^$', views.home, name='home'), - url(r'^admin/', admin.site.urls), - url(r'^asset_files/', include('admin.asset_files.urls', namespace='asset_files')), - url(r'^banners/', include('admin.banners.urls', namespace='banners')), - url(r'^brands/', include('admin.brands.urls', namespace='brands')), - url(r'^spam/', include('admin.spam.urls', namespace='spam')), - url(r'^institutions/', include('admin.institutions.urls', namespace='institutions')), - url(r'^entitlements/', include('admin.entitlements.urls', namespace='entitlements')), - url(r'^quota_recalc/', include('admin.quota_recalc.urls', namespace='quota_recalc')), - url(r'^preprint_providers/', include('admin.preprint_providers.urls', namespace='preprint_providers')), - url(r'^collection_providers/', include('admin.collection_providers.urls', namespace='collection_providers')), - url(r'^registration_providers/', include('admin.registration_providers.urls', namespace='registration_providers')), - url(r'^account/', include('admin.common_auth.urls', namespace='auth')), - url(r'^password/', include('password_reset.urls')), - url(r'^nodes/', include('admin.nodes.urls', namespace='nodes')), - url(r'^preprints/', include('admin.preprints.urls', namespace='preprints')), - url(r'^subjects/', include('admin.subjects.urls', namespace='subjects')), - url(r'^users/', include('admin.users.urls', namespace='users')), - url(r'^user-emails/', include('admin.user_emails.urls', namespace='user-emails')), - url(r'^maintenance/', include('admin.maintenance.urls', namespace='maintenance')), - url(r'^meetings/', include('admin.meetings.urls', - namespace='meetings')), - url(r'^metrics/', include('admin.metrics.urls', - namespace='metrics')), - url(r'^desk/', include('admin.desk.urls', - namespace='desk')), - url(r'^osf_groups/', include('admin.osf_groups.urls', namespace='osf_groups')), - url(r'^management/', include('admin.management.urls', namespace='management')), - url(r'^announcement/', include('admin.rdm_announcement.urls', namespace='announcement')), - url(r'^addons/', include('admin.rdm_addons.urls', namespace='addons')), - url(r'^oauth/', include('admin.rdm_addons.oauth.urls', namespace='oauth')), - url(r'^statistics/', include('admin.rdm_statistics.urls', namespace='statistics')), - url(r'^timestampadd/', include('admin.rdm_timestampadd.urls', namespace='timestampadd')), - url(r'^keymanagement/', include('admin.rdm_keymanagement.urls', namespace='keymanagement')), - url(r'^timestampsettings/', include('admin.rdm_timestampsettings.urls', namespace='timestampsettings')), - url(r'^custom_storage_location/', include('admin.rdm_custom_storage_location.urls', namespace='custom_storage_location')), - url(r'^institutional_storage_quota_control/', include('admin.institutional_storage_quota_control.urls', - namespace='institutional_storage_quota_control')), - url(r'^metadata/', include('admin.rdm_metadata.urls', namespace='metadata')), - url(r'^user_identification_information/', - include('admin.user_identification_information.urls', namespace='user_identification_information')), - url(r'^user_identification_information_admin/', - include('admin.user_identification_information_admin.urls', namespace='user_identification_information_admin')), - url(r'^project_limit_number/', include('admin.project_limit_number.urls', namespace='project_limit_number')), - url(r'^rdm_workflow/', include('admin.rdm_workflow.urls', namespace='rdm_workflow')), - url(r'^loa/', include('admin.loa.urls', namespace='loa')), - ]), + include(admin_urls), ), ] diff --git a/admin/base/utils.py b/admin/base/utils.py index 6ee8780be71..4f063bbbea3 100644 --- a/admin/base/utils.py +++ b/admin/base/utils.py @@ -1,6 +1,8 @@ """ Utility functions and classes """ +from functools import wraps + from osf.models import Subject, NodeLicense, Brand from django.core.exceptions import ValidationError, PermissionDenied @@ -35,6 +37,23 @@ def osf_staff_check(user): return user.is_authenticated and user.is_staff +def osf_superuser_check(user): + return user.is_authenticated and user.is_superuser + + +def superuser_required(view_func): + """Function-view decorator that returns 403 (PermissionDenied) instead of + redirecting to login, unlike django.contrib.auth.decorators.user_passes_test.""" + + @wraps(view_func) + def wrapped_view(request, *args, **kwargs): + if not osf_superuser_check(request.user): + raise PermissionDenied + return view_func(request, *args, **kwargs) + + return wrapped_view + + def get_subject_rules(subjects_selected): """ Take a list of subjects, and parse them into rules consistent with preprpint provider diff --git a/admin/collection_providers/views.py b/admin/collection_providers/views.py index cafb9476f9e..4cfd366d990 100644 --- a/admin/collection_providers/views.py +++ b/admin/collection_providers/views.py @@ -4,7 +4,7 @@ from django.core import serializers from django.core.exceptions import ValidationError from django.urls import reverse_lazy -from django.shortcuts import redirect +from django.shortcuts import redirect, get_object_or_404 from django.views.generic import View, CreateView, ListView, DetailView, UpdateView, DeleteView, TemplateView from django.contrib import messages from django.contrib.auth.mixins import PermissionRequiredMixin @@ -228,12 +228,14 @@ def get_object(self, queryset=None): return CollectionProvider.objects.get(id=self.kwargs['collection_provider_id']) -class CannotDeleteProvider(TemplateView): +class CannotDeleteProvider(PermissionRequiredMixin, TemplateView): + permission_required = 'osf.delete_collectionprovider' + raise_exception = True template_name = 'collection_providers/cannot_delete.html' def get_context_data(self, **kwargs): context = super(CannotDeleteProvider, self).get_context_data(**kwargs) - context['provider'] = CollectionProvider.objects.get(id=self.kwargs['collection_provider_id']) + context['provider'] = get_object_or_404(CollectionProvider, id=self.kwargs['collection_provider_id']) return context diff --git a/admin/common_auth/views.py b/admin/common_auth/views.py index d2dc3993090..dd03b30dedb 100644 --- a/admin/common_auth/views.py +++ b/admin/common_auth/views.py @@ -39,10 +39,12 @@ def dispatch(self, request, *args, **kwargs): return super(LoginView, self).dispatch(request, *args, **kwargs) def form_valid(self, form): - user = authenticate( - username=form.cleaned_data.get('email').strip(), - password=form.cleaned_data.get('password').strip() - ) + user = None + if ENABLE_LOGIN_FORM: + user = authenticate( + username=form.cleaned_data.get('email').strip(), + password=form.cleaned_data.get('password').strip() + ) if user is not None: login(self.request, user) else: diff --git a/admin/management/views.py b/admin/management/views.py index fffd1bbef4f..b1522991ad2 100644 --- a/admin/management/views.py +++ b/admin/management/views.py @@ -1,15 +1,29 @@ +from django.contrib.auth.mixins import UserPassesTestMixin from django.views.generic import TemplateView, View from osf.management.commands.manage_switch_flags import manage_waffle from django.core.urlresolvers import reverse from django.shortcuts import redirect -class ManagementCommands(TemplateView): +from admin.rdm.utils import RdmPermissionMixin + +class ManagementCommands(RdmPermissionMixin, UserPassesTestMixin, TemplateView): """ Basic form to trigger various management commands """ template_name = 'management/commands.html' object_type = 'management' + raise_exception = True + + def test_func(self): + """check user permissions""" + return self.is_super_admin + + +class WaffleFlag(RdmPermissionMixin, UserPassesTestMixin, View): + raise_exception = True -class WaffleFlag(View): + def test_func(self): + """check user permissions""" + return self.is_super_admin def post(self, request, *args, **kwargs): manage_waffle() diff --git a/admin/preprint_providers/views.py b/admin/preprint_providers/views.py index 159a0a409f5..8d2ecbf6d29 100644 --- a/admin/preprint_providers/views.py +++ b/admin/preprint_providers/views.py @@ -14,7 +14,7 @@ from django.contrib import messages from django.contrib.auth.mixins import PermissionRequiredMixin from django.forms.models import model_to_dict -from django.shortcuts import redirect, render +from django.shortcuts import redirect, render, get_object_or_404 from admin.base import settings from admin.base.forms import ImportFileForm @@ -319,12 +319,14 @@ def get_object(self, queryset=None): return PreprintProvider.objects.get(id=self.kwargs['preprint_provider_id']) -class CannotDeleteProvider(TemplateView): +class CannotDeleteProvider(PermissionRequiredMixin, TemplateView): + permission_required = 'osf.delete_preprintprovider' + raise_exception = True template_name = 'preprint_providers/cannot_delete.html' def get_context_data(self, **kwargs): context = super(CannotDeleteProvider, self).get_context_data(**kwargs) - context['provider'] = PreprintProvider.objects.get(id=self.kwargs['preprint_provider_id']) + context['provider'] = get_object_or_404(PreprintProvider, id=self.kwargs['preprint_provider_id']) return context diff --git a/admin/quota_recalc/views.py b/admin/quota_recalc/views.py index 29484c3d559..540e1acef0d 100644 --- a/admin/quota_recalc/views.py +++ b/admin/quota_recalc/views.py @@ -1,6 +1,7 @@ from django.http import JsonResponse from django.db import transaction, IntegrityError +from admin.base.utils import superuser_required from addons.osfstorage.models import Region from api.base import settings as api_settings from osf.models import OSFUser, UserQuota, Node @@ -60,6 +61,7 @@ def calculate_quota(user): user_quota.used = used user_quota.save() +@superuser_required def all_users(request, **kwargs): c = 0 for osf_user in OSFUser.objects.exclude(deleted__isnull=False): @@ -70,6 +72,7 @@ def all_users(request, **kwargs): 'message': str(c) + ' users\' quota successfully recalculated!' }) +@superuser_required def user(request, guid, **kwargs): user = OSFUser.load(guid) if user is None: diff --git a/admin/rdm_custom_storage_location/cron.sh b/admin/rdm_custom_storage_location/cron.sh index 10cb90c4250..81c65448528 100644 --- a/admin/rdm_custom_storage_location/cron.sh +++ b/admin/rdm_custom_storage_location/cron.sh @@ -1 +1 @@ -*/30 * * * * curl http://localhost:8001/custom_storage_location/external_acc_update/d610ef95f0b0f5868f13919b8ed64070b9acb9c19b8da9f2c514ed938203ec3e236c9cad4f4146bdf22b4e79cf0d92f6d4f4c996d236c6b0ee79a1336b26afb7/ | jq -c . +*/30 * * * * TOKEN=$(cd /code && python3 manage.py generate_signed_cron_token | tail -n1) && [ -n "$TOKEN" ] && curl "http://localhost:8001/custom_storage_location/external_acc_update/$TOKEN/" | jq -c . diff --git a/admin/rdm_custom_storage_location/urls.py b/admin/rdm_custom_storage_location/urls.py index 6a62d690294..c51f34c148e 100644 --- a/admin/rdm_custom_storage_location/urls.py +++ b/admin/rdm_custom_storage_location/urls.py @@ -3,7 +3,7 @@ urlpatterns = [ - url(r'^external_acc_update/(?P-?\w+)/$', views.external_acc_update, name='external_acc_update'), + url(r'^external_acc_update/(?P\d+)/(?P[0-9a-fA-F]+)/$', views.external_acc_update, name='external_acc_update'), url(r'^institutional_storage/$', views.InstitutionalStorageView.as_view(), name='institutional_storage'), url(r'^institutional_storage/institutions/$', views.InstitutionalStorageListView.as_view(), name='institutional_storage_institutions'), url(r'^institutional_storage/institutions/(?P[0-9]+)/$', views.InstitutionalStorageView.as_view(), name='institutional_storage_list'), diff --git a/admin/rdm_custom_storage_location/views.py b/admin/rdm_custom_storage_location/views.py index dff08d2b3ec..33dd5824844 100644 --- a/admin/rdm_custom_storage_location/views.py +++ b/admin/rdm_custom_storage_location/views.py @@ -5,7 +5,6 @@ from django.views.generic import TemplateView, View, ListView import codecs import json -import hashlib from rest_framework import status as http_status from mimetypes import MimeTypes import os @@ -19,13 +18,13 @@ from osf.models import Institution, OSFUser from osf.models.external import ExternalAccountTemporary from scripts import refresh_addon_tokens +from framework.auth import cron_signed_url from website import settings as osf_settings from distutils.util import strtobool from admin.base import settings logger = logging.getLogger(__name__) -SITE_KEY = 'rdm_custom_storage_location' INSTITUTION_NOT_FOUND_MESSAGE = 'Institution does not exist' class InstitutionalStorageBaseView(RdmPermissionMixin, UserPassesTestMixin): @@ -522,8 +521,8 @@ def post(self, request, *args, **kwargs): 'message': 'Garbage data removed!!' }, status=http_status.HTTP_200_OK) -def external_acc_update(request, access_token): - if hashlib.sha512(SITE_KEY.encode('utf-8')).hexdigest() != access_token.lower(): +def external_acc_update(request, ts, signature): + if not cron_signed_url.verify_signed_params(ts, signature): return HttpResponse( json.dumps({'state': 'fail', 'error': 'access forbidden'}), content_type='application/json', diff --git a/admin/rdm_statistics/cron.sh b/admin/rdm_statistics/cron.sh index 62a80709268..a3d434e7808 100644 --- a/admin/rdm_statistics/cron.sh +++ b/admin/rdm_statistics/cron.sh @@ -1 +1 @@ -0 2 * * 1 curl http://localhost:8001/statistics/gather/2A85563B2B0F7D3168199F475365F57DA1D56E4BB2CE2B7044EB058AE5E287637E7C636A772682D92C8D6B1830B9A97C5A5DC3DE7016C60BDE4BAA7CC3B38AEB/ | jq -c . +0 2 * * 1 TOKEN=$(cd /code && python3 manage.py generate_signed_cron_token | tail -n1) && [ -n "$TOKEN" ] && curl "http://localhost:8001/statistics/gather/$TOKEN/" | jq -c . diff --git a/admin/rdm_statistics/urls.py b/admin/rdm_statistics/urls.py index 5775b6bf71e..443de572baf 100644 --- a/admin/rdm_statistics/urls.py +++ b/admin/rdm_statistics/urls.py @@ -8,7 +8,7 @@ url(r'^index$', views.IndexView.as_view(), name='index'), url(r'^(?P-?[0-9]+)/graph/(?P\w+)_(?P\w+)\.(\w+)$', views.ImageView.as_view(), name='graph'), - url(r'^gather/(?P-?\w+)/$', views.GatherView.as_view(), name='gather'), + url(r'^gather/(?P\d+)/(?P[0-9a-fA-F]+)/$', views.GatherView.as_view(), name='gather'), url(r'^report/(?P-?[0-9]+)/$', views.create_pdf, name='report'), url(r'^csv/(?P-?[0-9]+)/$', views.create_csv, name='csv'), url(r'^mail/(?P-?[0-9]+)/$', views.SendView.as_view(), name='mail'), diff --git a/admin/rdm_statistics/views.py b/admin/rdm_statistics/views.py index d4522cf1a9c..20f8dc23017 100644 --- a/admin/rdm_statistics/views.py +++ b/admin/rdm_statistics/views.py @@ -12,7 +12,6 @@ import csv import pandas as pd import numpy as np -import hashlib from django.apps import apps from django.views.generic import TemplateView, View @@ -41,8 +40,10 @@ import seaborn as sns import pdfkit from admin.base import settings +from admin.base.utils import superuser_required from admin.rdm.utils import RdmPermissionMixin, get_dummy_institution from admin.rdm_addons import utils +from framework.auth import cron_signed_url import logging logger = logging.getLogger(__name__) @@ -51,7 +52,6 @@ STATISTICS_IMAGE_HEIGHT = 4 RECURSIVE_LIMIT = 10000 WB_MAX_RETRY = 3 -SITE_KEY = 'rdm_statistics' class InstitutionListViewStat(RdmPermissionMixin, UserPassesTestMixin, TemplateView): """institlutions list view for statistics""" @@ -113,8 +113,6 @@ def get_context_data(self, **kwargs): ctx['current_date'] = current_date ctx['user'] = user ctx['provider_data_array'] = provider_data_array - digest = hashlib.sha512(SITE_KEY.encode('utf-8')).hexdigest() - ctx['token'] = digest.upper() return ctx @@ -447,9 +445,10 @@ class GatherView(TemplateView): raise_exception = True def get(self, request, *args, **kwargs): - # simple authentication - access_token = self.kwargs.get('access_token') - if not simple_auth(access_token): + # signed-URL authentication + ts = self.kwargs.get('ts') + signature = self.kwargs.get('signature') + if not cron_signed_url.verify_signed_params(ts, signature): response_hash = {'state': 'fail', 'error': 'access forbidden'} response_json = json.dumps(response_hash) response = HttpResponse(response_json, content_type='application/json') @@ -484,7 +483,7 @@ def get(self, request, *args, **kwargs): response_json = json.dumps(self.stat_list) response = HttpResponse(response_json, content_type='application/json') # statistics mail send - send_stat_mail(request) + send_stat_mail_core(request) except Exception as err: response_hash = {'state': 'fail', 'error': str(err)} response_json = json.dumps(response_hash) @@ -589,15 +588,15 @@ def count_project_files(self, node_id, provider, path, cookies): pass self.count_project_files(provider=provider, node_id=node_id, path='/' + path, cookies=cookies) -def simple_auth(access_token): - digest = hashlib.sha512(SITE_KEY.encode('utf-8')).hexdigest() - if digest == access_token.lower(): - return True - else: - return False -def send_stat_mail(request, **kwargs): - """send statistics information email""" +def send_stat_mail_core(request, **kwargs): + """Send statistics information email. + + No authorization check here by design - this is the shared logic called + both by the send_stat_mail view (protected below) and internally by + GatherView.get(), which runs unauthenticated over localhost from + cron and is authorized separately via its own access token. + """ current_date = get_current_date() all_institutions = Institution.objects.order_by('id').all() all_staff_users = OSFUser.objects.filter(is_staff=True) @@ -630,6 +629,11 @@ def send_stat_mail(request, **kwargs): response = HttpResponse(response_json, content_type='application/json') return response +@superuser_required +def send_stat_mail(request, **kwargs): + """URL-exposed view for /statistics/test/mail/ - Integrated Admin only.""" + return send_stat_mail_core(request, **kwargs) + def send_error_mail(err): """send error email""" current_date = get_current_date() @@ -776,14 +780,18 @@ def approximate_size(size, a_kilobyte_is_1024_bytes=True): ### views or funcs for development and test ############################################ -class IndexView(TemplateView): +class IndexView(RdmPermissionMixin, UserPassesTestMixin, TemplateView): """index view of statistics module.""" template_name = 'rdm_statistics/index.html' raise_exception = True + def test_func(self): + """check user permissions""" + return self.is_authenticated and (self.is_super_admin or self.is_admin) + def find_bookmark_collection(self, user): collection = apps.get_model('osf.Collection') - return collection.objects.get(creator=user, is_deleted=False, is_bookmark_collection=True) + return collection.objects.get(creator=user, deleted__isnull=True, is_bookmark_collection=True) def get(self, request, *args, **kwargs): user = self.request.user diff --git a/admin/registration_providers/views.py b/admin/registration_providers/views.py index 2466bfae63a..c576ed7db16 100644 --- a/admin/registration_providers/views.py +++ b/admin/registration_providers/views.py @@ -6,7 +6,7 @@ from django.core.exceptions import ValidationError from django.core.management import call_command from django.urls import reverse_lazy -from django.shortcuts import redirect +from django.shortcuts import redirect, get_object_or_404 from django.views.generic import View, CreateView, ListView, DetailView, UpdateView, DeleteView, TemplateView from django.contrib.auth.mixins import PermissionRequiredMixin from django.contrib import messages @@ -204,12 +204,14 @@ def get_context_data(self, *args, **kwargs): return super(DeleteRegistrationProvider, self).get_context_data(*args, **kwargs) -class CannotDeleteProvider(TemplateView): +class CannotDeleteProvider(PermissionRequiredMixin, TemplateView): + permission_required = 'osf.delete_registrationprovider' + raise_exception = True template_name = 'registration_providers/cannot_delete.html' def get_context_data(self, **kwargs): context = super(CannotDeleteProvider, self).get_context_data(**kwargs) - context['provider'] = RegistrationProvider.objects.get(id=self.kwargs['registration_provider_id']) + context['provider'] = get_object_or_404(RegistrationProvider, id=self.kwargs['registration_provider_id']) return context @@ -376,7 +378,7 @@ def get(self, request, *args, **kwargs): return redirect(reverse_lazy('registration_providers:detail', kwargs={'registration_provider_id': provider.id})) -class ChangeSchema(TemplateView): +class ChangeSchema(PermissionRequiredMixin, TemplateView): permission_required = 'osf.change_registrationprovider' template_name = 'registration_providers/change_schema.html' diff --git a/admin_tests/collection_providers/test_views.py b/admin_tests/collection_providers/test_views.py index ef054a2778f..1dd19e84801 100644 --- a/admin_tests/collection_providers/test_views.py +++ b/admin_tests/collection_providers/test_views.py @@ -1,6 +1,9 @@ import pytest from django.test import RequestFactory +from django.contrib.auth.models import AnonymousUser, Permission +from django.core.exceptions import PermissionDenied +from django.http import Http404 from osf_tests.factories import ( AuthUserFactory, @@ -93,3 +96,45 @@ def view(self, req, provider): view = setup_view(view, req) view.kwargs = {'collection_provider_id': provider.id} return view + + +@pytest.mark.urls('admin.base.urls') +class TestCannotDeleteProviderPermission: + """CannotDeleteProvider must be restricted to users with 'osf.delete_collectionprovider'.""" + + @pytest.fixture() + def provider(self): + return CollectionProviderFactory() + + def test_denied_for_general_user(self, req, provider): + with pytest.raises(PermissionDenied): + views.CannotDeleteProvider.as_view()(req, collection_provider_id=provider.id) + + def test_denied_for_anonymous(self, provider): + req = RequestFactory().get('/fake_path') + req.user = AnonymousUser() + with pytest.raises(PermissionDenied): + views.CannotDeleteProvider.as_view()(req, collection_provider_id=provider.id) + + def test_allowed_for_user_with_permission(self, req, user, provider): + permission = Permission.objects.get(codename='delete_collectionprovider') + user.user_permissions.add(permission) + user.save() + + res = views.CannotDeleteProvider.as_view()(req, collection_provider_id=provider.id) + assert res.status_code == 200 + + def test_allowed_for_superuser(self, req, user, provider): + user.is_superuser = True + user.save() + + res = views.CannotDeleteProvider.as_view()(req, collection_provider_id=provider.id) + assert res.status_code == 200 + + def test_404_for_missing_provider(self, req, user): + permission = Permission.objects.get(codename='delete_collectionprovider') + user.user_permissions.add(permission) + user.save() + + with pytest.raises(Http404): + views.CannotDeleteProvider.as_view()(req, collection_provider_id=999999) diff --git a/admin_tests/common_auth/test_views.py b/admin_tests/common_auth/test_views.py index 5bc23647714..89bc21614f7 100644 --- a/admin_tests/common_auth/test_views.py +++ b/admin_tests/common_auth/test_views.py @@ -5,14 +5,15 @@ from django.http import Http404 from django.urls import reverse from django.contrib.auth import REDIRECT_FIELD_NAME +from django.contrib.messages.storage.fallback import FallbackStorage from tests.base import AdminTestCase from osf_tests.factories import AuthUserFactory, InstitutionFactory from admin_tests.utilities import setup_form_view, setup_view from osf.models.user import OSFUser -from admin.common_auth.views import RegisterUser, ShibLoginView -from admin.common_auth.forms import UserRegistrationForm +from admin.common_auth.views import RegisterUser, ShibLoginView, LoginView +from admin.common_auth.forms import UserRegistrationForm, LoginForm class TestRegisterUser(AdminTestCase): @@ -356,3 +357,47 @@ def test_new_user_missing_displayname_header_raises_key_error(self, mock_use_epp view = setup_view(ShibLoginView(), request) with nt.assert_raises(KeyError): view.dispatch(request) + + +class TestLoginViewFormValid(AdminTestCase): + """LoginView.form_valid must skip authenticate() entirely when + ENABLE_LOGIN_FORM is False, so password-based login stays disabled even via + a direct POST to /account/login/.""" + + def setUp(self): + super(TestLoginViewFormValid, self).setUp() + self.view = LoginView() + self.request = RequestFactory().post('/account/login/') + # django.contrib.messages has a bug which effects unittests + # more info here -> https://code.djangoproject.com/ticket/17971 + setattr(self.request, 'session', 'session') + setattr(self.request, '_messages', FallbackStorage(self.request)) + self.form = LoginForm(data={'email': 'user@example.com', 'password': 'secret'}) + nt.assert_true(self.form.is_valid()) + self.view = setup_form_view(self.view, self.request, self.form) + + @mock.patch('admin.common_auth.views.ENABLE_LOGIN_FORM', False) + @mock.patch('admin.common_auth.views.authenticate') + def test_authenticate_skipped_when_login_form_disabled(self, mock_authenticate): + response = self.view.form_valid(self.form) + nt.assert_false(mock_authenticate.called) + nt.assert_equal(response.status_code, 302) + nt.assert_true(response.url.startswith('/account/login')) + + @mock.patch('admin.common_auth.views.ENABLE_LOGIN_FORM', True) + @mock.patch('admin.common_auth.views.authenticate', return_value=None) + def test_authenticate_called_and_denied_when_login_form_enabled(self, mock_authenticate): + response = self.view.form_valid(self.form) + nt.assert_true(mock_authenticate.called) + nt.assert_equal(response.status_code, 302) + nt.assert_true(response.url.startswith('/account/login')) + + @mock.patch('admin.common_auth.views.ENABLE_LOGIN_FORM', True) + @mock.patch('admin.common_auth.views.login') + @mock.patch('admin.common_auth.views.authenticate') + def test_authenticate_called_and_login_succeeds_when_login_form_enabled(self, mock_authenticate, mock_login): + mock_authenticate.return_value = AuthUserFactory() + response = self.view.form_valid(self.form) + nt.assert_true(mock_authenticate.called) + nt.assert_true(mock_login.called) + nt.assert_equal(response.status_code, 302) diff --git a/admin_tests/management/__init__.py b/admin_tests/management/__init__.py new file mode 100644 index 00000000000..e69de29bb2d diff --git a/admin_tests/management/test_views.py b/admin_tests/management/test_views.py new file mode 100644 index 00000000000..8e93b66af0f --- /dev/null +++ b/admin_tests/management/test_views.py @@ -0,0 +1,70 @@ +# -*- coding: utf-8 -*- +from nose import tools as nt +import mock +from django.test import RequestFactory +from django.contrib.auth.models import AnonymousUser +from django.core.exceptions import PermissionDenied + +from admin.management import views +from osf_tests.factories import AuthUserFactory +from tests.base import AdminTestCase + + +class TestManagementCommandsPermission(AdminTestCase): + """ManagementCommands must be restricted to Integrated Admin (is_super_admin).""" + + def setUp(self): + super(TestManagementCommandsPermission, self).setUp() + self.superuser = AuthUserFactory() + self.superuser.is_superuser = True + self.superuser.save() + self.general_user = AuthUserFactory() + + def test_get_denied_for_general_user(self): + request = RequestFactory().get('/fake_path') + request.user = self.general_user + with nt.assert_raises(PermissionDenied): + views.ManagementCommands.as_view()(request) + + def test_get_denied_for_anonymous(self): + request = RequestFactory().get('/fake_path') + request.user = AnonymousUser() + with nt.assert_raises(PermissionDenied): + views.ManagementCommands.as_view()(request) + + def test_get_allowed_for_superuser(self): + request = RequestFactory().get('/fake_path') + request.user = self.superuser + response = views.ManagementCommands.as_view()(request) + nt.assert_equal(response.status_code, 200) + + +class TestWaffleFlagPermission(AdminTestCase): + """WaffleFlag must be restricted to Integrated Admin (is_super_admin).""" + + def setUp(self): + super(TestWaffleFlagPermission, self).setUp() + self.superuser = AuthUserFactory() + self.superuser.is_superuser = True + self.superuser.save() + self.general_user = AuthUserFactory() + + def test_post_denied_for_general_user(self): + request = RequestFactory().post('/fake_path') + request.user = self.general_user + with nt.assert_raises(PermissionDenied): + views.WaffleFlag.as_view()(request) + + def test_post_denied_for_anonymous(self): + request = RequestFactory().post('/fake_path') + request.user = AnonymousUser() + with nt.assert_raises(PermissionDenied): + views.WaffleFlag.as_view()(request) + + @mock.patch('admin.management.views.manage_waffle') + def test_post_allowed_for_superuser(self, mock_manage_waffle): + request = RequestFactory().post('/fake_path') + request.user = self.superuser + response = views.WaffleFlag.as_view()(request) + nt.assert_equal(response.status_code, 302) + nt.assert_true(mock_manage_waffle.called) diff --git a/admin_tests/preprint_providers/test_views.py b/admin_tests/preprint_providers/test_views.py index 66403b89536..204107770a3 100644 --- a/admin_tests/preprint_providers/test_views.py +++ b/admin_tests/preprint_providers/test_views.py @@ -6,6 +6,9 @@ import responses from nose import tools as nt from django.test import RequestFactory +from django.contrib.auth.models import AnonymousUser, Permission +from django.core.exceptions import PermissionDenied +from django.http import Http404 from django.core.files.uploadedfile import InMemoryUploadedFile from scripts.update_taxonomies import update_taxonomies @@ -369,6 +372,48 @@ def test_cannot_get_if_preprints_present(self, req, view, preprint, provider_wit assert redirect.url == '/preprint_providers/{}/cannot_delete/'.format(provider_with_preprint.id) assert redirect.status_code == 302 +@pytest.mark.urls('admin.base.urls') +class TestCannotDeleteProviderPermission: + """CannotDeleteProvider must be restricted to users with 'osf.delete_preprintprovider'.""" + + @pytest.fixture() + def provider(self): + return PreprintProviderFactory() + + def test_denied_for_general_user(self, req, provider): + with pytest.raises(PermissionDenied): + views.CannotDeleteProvider.as_view()(req, preprint_provider_id=provider.id) + + def test_denied_for_anonymous(self, provider): + req = RequestFactory().get('/fake_path') + req.user = AnonymousUser() + with pytest.raises(PermissionDenied): + views.CannotDeleteProvider.as_view()(req, preprint_provider_id=provider.id) + + def test_allowed_for_user_with_permission(self, req, user, provider): + permission = Permission.objects.get(codename='delete_preprintprovider') + user.user_permissions.add(permission) + user.save() + + res = views.CannotDeleteProvider.as_view()(req, preprint_provider_id=provider.id) + assert res.status_code == 200 + + def test_allowed_for_superuser(self, req, user, provider): + user.is_superuser = True + user.save() + + res = views.CannotDeleteProvider.as_view()(req, preprint_provider_id=provider.id) + assert res.status_code == 200 + + def test_404_for_missing_provider(self, req, user): + permission = Permission.objects.get(codename='delete_preprintprovider') + user.user_permissions.add(permission) + user.save() + + with pytest.raises(Http404): + views.CannotDeleteProvider.as_view()(req, preprint_provider_id=999999) + + class TestProcessCustomTaxonomy(ProcessCustomTaxonomyMixinBase): @pytest.fixture(autouse=True) diff --git a/admin_tests/quota_recalc/test_views.py b/admin_tests/quota_recalc/test_views.py index cbaa876e015..1263d179b26 100644 --- a/admin_tests/quota_recalc/test_views.py +++ b/admin_tests/quota_recalc/test_views.py @@ -1,5 +1,7 @@ # -*- coding: utf-8 -*- from django.test import RequestFactory +from django.contrib.auth.models import AnonymousUser +from django.core.exceptions import PermissionDenied import json import mock from nose import tools as nt @@ -12,9 +14,16 @@ class TestQuotaRecalcView(AdminTestCase): - @staticmethod - def get_request(view, **kwargs): - return view(RequestFactory().get('/fake_path'), **kwargs) + def setUp(self): + super(TestQuotaRecalcView, self).setUp() + self.superuser = AuthUserFactory() + self.superuser.is_superuser = True + self.superuser.save() + + def get_request(self, view, **kwargs): + request = RequestFactory().get('/fake_path') + request.user = self.superuser + return view(request, **kwargs) @mock.patch('admin.quota_recalc.views.used_quota') def test_user_create_userquota_record(self, mock_usedquota): @@ -85,7 +94,59 @@ def test_users_create_userquota_record(self, mock_usedquota): res_json3 = json.loads(response3.content) nt.assert_equal(response3.status_code, 200) nt.assert_equal(res_json3['status'], 'OK') - nt.assert_true('2' in res_json3['message']) + nt.assert_true('3' in res_json3['message']) + + +class TestQuotaRecalcPermission(AdminTestCase): + """all_users/user must be restricted to Integrated Admin (is_superuser).""" + + def setUp(self): + super(TestQuotaRecalcPermission, self).setUp() + self.general_user = AuthUserFactory() + self.superuser = AuthUserFactory() + self.superuser.is_superuser = True + self.superuser.save() + self.target_user = AuthUserFactory() + + def test_all_users_denied_for_general_user(self): + request = RequestFactory().get('/fake_path') + request.user = self.general_user + with nt.assert_raises(PermissionDenied): + views.all_users(request) + + def test_all_users_denied_for_anonymous(self): + request = RequestFactory().get('/fake_path') + request.user = AnonymousUser() + with nt.assert_raises(PermissionDenied): + views.all_users(request) + + @mock.patch('admin.quota_recalc.views.used_quota') + def test_all_users_allowed_for_superuser(self, mock_usedquota): + mock_usedquota.return_value = 0 + request = RequestFactory().get('/fake_path') + request.user = self.superuser + response = views.all_users(request) + nt.assert_equal(response.status_code, 200) + + def test_user_denied_for_general_user(self): + request = RequestFactory().get('/fake_path') + request.user = self.general_user + with nt.assert_raises(PermissionDenied): + views.user(request, guid=self.target_user._id) + + def test_user_denied_for_anonymous(self): + request = RequestFactory().get('/fake_path') + request.user = AnonymousUser() + with nt.assert_raises(PermissionDenied): + views.user(request, guid=self.target_user._id) + + @mock.patch('admin.quota_recalc.views.used_quota') + def test_user_allowed_for_superuser(self, mock_usedquota): + mock_usedquota.return_value = 0 + request = RequestFactory().get('/fake_path') + request.user = self.superuser + response = views.user(request, guid=self.target_user._id) + nt.assert_equal(response.status_code, 200) class TestCalculateQuota(AdminTestCase): diff --git a/admin_tests/rdm_statistics/test_views.py b/admin_tests/rdm_statistics/test_views.py index 670cfcc31d7..181c64d51ce 100644 --- a/admin_tests/rdm_statistics/test_views.py +++ b/admin_tests/rdm_statistics/test_views.py @@ -1,10 +1,13 @@ # -*- coding: utf-8 -*- from nose import tools as nt from django.test import RequestFactory +from django.contrib.auth.models import AnonymousUser +from django.core.exceptions import PermissionDenied from tests.base import AdminTestCase from osf_tests.factories import ( AuthUserFactory, + BookmarkCollectionFactory, InstitutionFactory, ProjectFactory ) @@ -14,6 +17,7 @@ from osf.models.user import Institution from admin.rdm_statistics import views +from framework.auth import cron_signed_url from mock import patch import datetime @@ -179,7 +183,6 @@ def test_get_context_data(self, **kwargs): nt.assert_true('current_date' in ctx) nt.assert_true('user' in ctx) nt.assert_true('provider_data_array' in ctx) - nt.assert_true('token' in ctx) class TestImageView(AdminTestCase): """test ImageView""" @@ -350,9 +353,6 @@ def tearDown(self): self.user.delete() self.institution1.delete() -def test_simple_auth(): - access_key_hexa = '2a85563b2b0f7d3168199f475365f57da1d56e4bb2ce2b7044eb058ae5e287637e7c636a772682d92c8d6b1830b9a97c5a5dc3de7016c60bde4baa7cc3b38aeb' - nt.assert_true(views.simple_auth(access_key_hexa)) def test_get_start_date(): end_date = datetime.datetime.now() @@ -408,7 +408,8 @@ def setUp(self): self.request = RequestFactory().get('/fake_path') self.view = views.GatherView() self.view = setup_user_view(self.view, self.request, user=self.user) - self.view.kwargs = {'institution_id': self.institution1.id, 'access_token': '2A85563B2B0F7D3168199F475365F57DA1D56E4BB2CE2B7044EB058AE5E287637E7C636A772682D92C8D6B1830B9A97C5A5DC3DE7016C60BDE4BAA7CC3B38AEB'.lower()} + ts, signature = cron_signed_url.generate_signed_params() + self.view.kwargs = {'institution_id': self.institution1.id, 'ts': ts, 'signature': signature} def tearDown(self): super(TestGatherView, self).tearDown() @@ -426,8 +427,14 @@ def test_get(self, *args, **kwargs): # metadata addon is now enabled by default, so we have 3 providers nt.assert_equal(len(resp), 3) + def test_get_forbidden_with_invalid_signature(self): + # GatherView.get() reads ts/signature from self.kwargs, set in setUp() + self.view.kwargs['signature'] = 'deadbeef' + resp = json.loads(self.view.get(self.request).content) + nt.assert_equal(resp['state'], 'fail') + def test_send_stat_mail(self, *args, **kwargs): - nt.assert_equal(views.send_stat_mail(self.request).status_code, 200) + nt.assert_equal(views.send_stat_mail_core(self.request).status_code, 200) def test_send_error_mail(self, *args, **kwargs): ret = views.send_error_mail(Exception()) @@ -505,3 +512,80 @@ def test_get_graphs(self, mock_sessionget): provider='osfstorage' ) nt.assert_equal(result['content-type'], 'image/png') + + +class TestSendStatMailPermission(AdminTestCase): + """send_stat_mail (No.86, /statistics/test/mail/) must be restricted to Integrated Admin (is_superuser).""" + + def setUp(self): + super(TestSendStatMailPermission, self).setUp() + self.general_user = AuthUserFactory() + self.superuser = AuthUserFactory() + self.superuser.is_superuser = True + self.superuser.save() + + def test_send_stat_mail_denied_for_general_user(self): + request = RequestFactory().get('/fake_path') + request.user = self.general_user + with nt.assert_raises(PermissionDenied): + views.send_stat_mail(request) + + def test_send_stat_mail_denied_for_anonymous(self): + request = RequestFactory().get('/fake_path') + request.user = AnonymousUser() + with nt.assert_raises(PermissionDenied): + views.send_stat_mail(request) + + def test_send_stat_mail_allowed_for_superuser(self): + request = RequestFactory().get('/fake_path') + request.user = self.superuser + response = views.send_stat_mail(request) + nt.assert_equal(response.status_code, 200) + + +class TestIndexViewPermission(AdminTestCase): + """IndexView must be restricted to Integrated Admin or Institutional Admin.""" + + def setUp(self): + super(TestIndexViewPermission, self).setUp() + self.superuser = AuthUserFactory() + self.superuser.is_superuser = True + self.superuser.save() + self.institution_admin = AuthUserFactory() + self.institution_admin.is_staff = True + self.institution_admin.save() + self.general_user = AuthUserFactory() + + def test_denied_for_general_user(self): + request = RequestFactory().get('/fake_path') + request.user = self.general_user + with nt.assert_raises(PermissionDenied): + views.IndexView.as_view()(request) + + def test_denied_for_anonymous(self): + request = RequestFactory().get('/fake_path') + request.user = AnonymousUser() + with nt.assert_raises(PermissionDenied): + views.IndexView.as_view()(request) + + def test_allowed_for_super_admin(self): + request = RequestFactory().get('/fake_path') + request.user = self.superuser + view = views.IndexView() + view.request = request + nt.assert_true(view.test_func()) + + def test_allowed_for_institution_admin(self): + request = RequestFactory().get('/fake_path') + request.user = self.institution_admin + view = views.IndexView() + view.request = request + nt.assert_true(view.test_func()) + + def test_find_bookmark_collection(self): + """Collection has no 'is_deleted' field; filtering on it raised FieldError.""" + # conftest mocks out new_bookmark_collection for speed, so the bookmark + # collection normally created with the user has to be added here. + bookmark_collection = BookmarkCollectionFactory(creator=self.superuser) + view = views.IndexView() + nt.assert_equal(view.find_bookmark_collection(self.superuser), bookmark_collection) diff --git a/admin_tests/registration_providers/test_views.py b/admin_tests/registration_providers/test_views.py index 9025c46b68c..e2bc66a9f1d 100644 --- a/admin_tests/registration_providers/test_views.py +++ b/admin_tests/registration_providers/test_views.py @@ -2,6 +2,9 @@ import pytest from django.test import RequestFactory +from django.contrib.auth.models import AnonymousUser, Permission +from django.core.exceptions import PermissionDenied +from django.http import Http404 from osf_tests.factories import ( AuthUserFactory, @@ -207,3 +210,79 @@ def test_post(self, view, req, schema, provider): res = view.post(req) assert res.status_code == 302 assert provider.schemas.get(id=schema_id) + + +@pytest.mark.urls('admin.base.urls') +class TestChangeSchemaPermission: + """ChangeSchema must be restricted to users with 'osf.change_registrationprovider'.""" + + @pytest.fixture() + def provider(self): + return RegistrationProviderFactory() + + def test_get_denied_for_general_user(self, req, provider): + with pytest.raises(PermissionDenied): + views.ChangeSchema.as_view()(req, registration_provider_id=provider.id) + + def test_get_denied_for_anonymous(self, provider): + req = RequestFactory().get('/fake_path') + req.user = AnonymousUser() + with pytest.raises(PermissionDenied): + views.ChangeSchema.as_view()(req, registration_provider_id=provider.id) + + def test_get_allowed_for_user_with_permission(self, req, user, provider): + permission = Permission.objects.get(codename='change_registrationprovider') + user.user_permissions.add(permission) + user.save() + + res = views.ChangeSchema.as_view()(req, registration_provider_id=provider.id) + assert res.status_code == 200 + + def test_get_allowed_for_superuser(self, req, user, provider): + user.is_superuser = True + user.save() + + res = views.ChangeSchema.as_view()(req, registration_provider_id=provider.id) + assert res.status_code == 200 + + +@pytest.mark.urls('admin.base.urls') +class TestCannotDeleteProviderPermission: + """CannotDeleteProvider must be restricted to users with 'osf.delete_registrationprovider'.""" + + @pytest.fixture() + def provider(self): + return RegistrationProviderFactory() + + def test_denied_for_general_user(self, req, provider): + with pytest.raises(PermissionDenied): + views.CannotDeleteProvider.as_view()(req, registration_provider_id=provider.id) + + def test_denied_for_anonymous(self, provider): + req = RequestFactory().get('/fake_path') + req.user = AnonymousUser() + with pytest.raises(PermissionDenied): + views.CannotDeleteProvider.as_view()(req, registration_provider_id=provider.id) + + def test_allowed_for_user_with_permission(self, req, user, provider): + permission = Permission.objects.get(codename='delete_registrationprovider') + user.user_permissions.add(permission) + user.save() + + res = views.CannotDeleteProvider.as_view()(req, registration_provider_id=provider.id) + assert res.status_code == 200 + + def test_allowed_for_superuser(self, req, user, provider): + user.is_superuser = True + user.save() + + res = views.CannotDeleteProvider.as_view()(req, registration_provider_id=provider.id) + assert res.status_code == 200 + + def test_404_for_missing_provider(self, req, user): + permission = Permission.objects.get(codename='delete_registrationprovider') + user.user_permissions.add(permission) + user.save() + + with pytest.raises(Http404): + views.CannotDeleteProvider.as_view()(req, registration_provider_id=999999) diff --git a/framework/auth/cron_signed_url.py b/framework/auth/cron_signed_url.py new file mode 100644 index 00000000000..868e82cf526 --- /dev/null +++ b/framework/auth/cron_signed_url.py @@ -0,0 +1,62 @@ +# -*- coding: utf-8 -*- +"""HMAC-signed, time-limited URL auth for cron-triggered admin endpoints. + +Used by No.1 (admin/rdm_custom_storage_location/views.py:external_acc_update) +and No.82 (admin/rdm_statistics/views.py:GatherView), which are called by +cron.sh scripts with no interactive user/session. A signed URL looks like +`...///`, where `ts` is a microsecond-resolution Unix +timestamp and `signature` is HMAC(secret, ts). + +Security comes from two independent checks, both required: + 1. authenticity - signature must match HMAC(CRON_SIGNED_URL_SECRET, ts) + 2. freshness - ts must be within CRON_SIGNED_URL_TTL_SECONDS of "now" + +Timestamp precision (microseconds) does not itself add security - it is a +customer requirement, not a defense mechanism. Both checks above are what +actually make an intercepted URL become useless after CRON_SIGNED_URL_TTL_SECONDS. +""" +import hmac +import time + +from website import settings + + +def _sign(ts): + return hmac.new( + key=settings.CRON_SIGNED_URL_SECRET.encode('utf-8'), + msg=str(ts).encode('utf-8'), + digestmod=settings.CRON_SIGNED_URL_HMAC_ALGORITHM, + ).hexdigest() + + +def generate_signed_params(): + """Returns (ts, signature) as strings, ready to drop into a URL path. + + ts is a microsecond-resolution Unix timestamp (str), per customer + requirement (see module docstring). + """ + ts = str(int(time.time() * 1000000)) + return ts, _sign(ts) + + +def verify_signed_params(ts, signature, ttl_seconds=None): + """Verifies a (ts, signature) pair produced by generate_signed_params(). + + Returns True only if signature matches ts AND ts is within ttl_seconds + of now (in either direction). Returns False for any malformed input + instead of raising. + """ + if ttl_seconds is None: + ttl_seconds = settings.CRON_SIGNED_URL_TTL_SECONDS + if not ts or not signature: + return False + try: + ts_us = int(ts) + except (TypeError, ValueError): + return False + expected_signature = _sign(ts) + if not hmac.compare_digest(expected_signature, signature.lower()): + return False + now_us = int(time.time() * 1000000) + ttl_us = int(ttl_seconds * 1000000) + return abs(now_us - ts_us) <= ttl_us diff --git a/osf/management/commands/generate_signed_cron_token.py b/osf/management/commands/generate_signed_cron_token.py new file mode 100644 index 00000000000..98c09cab48f --- /dev/null +++ b/osf/management/commands/generate_signed_cron_token.py @@ -0,0 +1,19 @@ +# -*- coding: utf-8 -*- +from django.core.management.base import BaseCommand + +from framework.auth.cron_signed_url import generate_signed_params + + +class Command(BaseCommand): + """Prints "/" for building a B(2) signed cron URL. + + Used by admin/rdm_statistics/cron.sh (No.82) and + admin/rdm_custom_storage_location/cron.sh (No.1) to mint a fresh + signed URL segment immediately before each curl call. See + framework.auth.cron_signed_url for the verification side. + """ + help = 'Print "/" for a B(2) signed cron URL.' + + def handle(self, *args, **options): + ts, signature = generate_signed_params() + self.stdout.write('{}/{}'.format(ts, signature)) diff --git a/osf_tests/management_commands/test_generate_signed_cron_token.py b/osf_tests/management_commands/test_generate_signed_cron_token.py new file mode 100644 index 00000000000..821d616c75c --- /dev/null +++ b/osf_tests/management_commands/test_generate_signed_cron_token.py @@ -0,0 +1,25 @@ +# -*- coding: utf-8 -*- +import io +import pytest +from django.core.management import call_command + +from framework.auth import cron_signed_url + + +@pytest.mark.django_db +class TestGenerateSignedCronToken: + + def test_prints_ts_slash_signature(self): + out = io.StringIO() + call_command('generate_signed_cron_token', stdout=out) + output = out.getvalue().strip() + assert output.count('/') == 1 + ts, signature = output.split('/') + assert ts.isdigit() + assert len(signature) > 0 + + def test_printed_pair_verifies_successfully(self): + out = io.StringIO() + call_command('generate_signed_cron_token', stdout=out) + ts, signature = out.getvalue().strip().split('/') + assert cron_signed_url.verify_signed_params(ts, signature) diff --git a/tests/test_cron_signed_url.py b/tests/test_cron_signed_url.py new file mode 100644 index 00000000000..d77169a13ce --- /dev/null +++ b/tests/test_cron_signed_url.py @@ -0,0 +1,62 @@ +# -*- coding: utf-8 -*- +import time +import mock +from nose.tools import * # noqa (PEP8 asserts) + +from framework.auth import cron_signed_url + + +class TestCronSignedUrl: + + def setup_method(self, method): + self.secret_patch = mock.patch('website.settings.CRON_SIGNED_URL_SECRET', 'test-secret') + self.ttl_patch = mock.patch('website.settings.CRON_SIGNED_URL_TTL_SECONDS', 100) + self.secret_patch.start() + self.ttl_patch.start() + + def teardown_method(self, method): + self.secret_patch.stop() + self.ttl_patch.stop() + + def test_generate_then_verify_succeeds(self): + ts, signature = cron_signed_url.generate_signed_params() + assert_true(cron_signed_url.verify_signed_params(ts, signature)) + + def test_ts_is_microsecond_precision(self): + before_us = int(time.time() * 1000000) + ts, _ = cron_signed_url.generate_signed_params() + after_us = int(time.time() * 1000000) + assert_true(before_us <= int(ts) <= after_us) + + def test_tampered_signature_is_rejected(self): + ts, signature = cron_signed_url.generate_signed_params() + flipped_char = '0' if signature[0] != '0' else '1' + tampered = flipped_char + signature[1:] + assert_false(cron_signed_url.verify_signed_params(ts, tampered)) + + def test_tampered_timestamp_is_rejected(self): + ts, signature = cron_signed_url.generate_signed_params() + tampered_ts = str(int(ts) + 1) + assert_false(cron_signed_url.verify_signed_params(tampered_ts, signature)) + + def test_expired_timestamp_is_rejected(self): + with mock.patch('time.time', return_value=time.time() - 200): + ts, signature = cron_signed_url.generate_signed_params() + assert_false(cron_signed_url.verify_signed_params(ts, signature)) + + def test_future_timestamp_beyond_ttl_is_rejected(self): + with mock.patch('time.time', return_value=time.time() + 200): + ts, signature = cron_signed_url.generate_signed_params() + assert_false(cron_signed_url.verify_signed_params(ts, signature)) + + def test_non_numeric_timestamp_is_rejected(self): + assert_false(cron_signed_url.verify_signed_params('not-a-number', 'deadbeef')) + + def test_missing_signature_is_rejected(self): + ts, _ = cron_signed_url.generate_signed_params() + assert_false(cron_signed_url.verify_signed_params(ts, '')) + + def test_wrong_secret_is_rejected(self): + ts, signature = cron_signed_url.generate_signed_params() + with mock.patch('website.settings.CRON_SIGNED_URL_SECRET', 'a-different-secret'): + assert_false(cron_signed_url.verify_signed_params(ts, signature)) diff --git a/tests/test_oauth.py b/tests/test_oauth.py index 74ce6195df1..af4002d5bf7 100644 --- a/tests/test_oauth.py +++ b/tests/test_oauth.py @@ -38,6 +38,7 @@ ) from django.test import RequestFactory from admin.rdm_custom_storage_location import views as customstoragelocation_update +from framework.auth import cron_signed_url SILENT_LOGGERS = ['oauthlib', 'requests_oauthlib'] @@ -1050,11 +1051,12 @@ def test_callback_with_institution(self, mock_drive_refresh): assert_equal(account.oauth_key, 'mock_access_token') assert_equal(account.provider_id, 'mock_provider_id') req = RequestFactory().get('http://localhost:8001/customstoragelocation/external_acc_update/') - res = customstoragelocation_update.external_acc_update(req,access_token='d610ef95f0b0f5868f13919b8ed64070b9acb9c19b8da9f2c514ed938203ec3e236c9cad4f4146bdf22b4e79cf0d92f6d4f4c996d236c6b0ee79a1336b26afb7') - assert_equal(res.status_code,200) + ts, signature = cron_signed_url.generate_signed_params() + res = customstoragelocation_update.external_acc_update(req, ts=ts, signature=signature) + assert_equal(res.status_code, 200) assert_equal(res.content.decode(), 'Done') - res = customstoragelocation_update.external_acc_update(req,access_token='b610ef95f0b0f5868f13919b8ed64070b9acb9c19b8da9f2c514ed938203ec3e236c9cad4f4146bdf22b4e79cf0d92f6d4f4c996d236c6b0ee79a1336b26afb7') - assert_equal(res.status_code,200) + res = customstoragelocation_update.external_acc_update(req, ts=ts, signature='deadbeef') + assert_equal(res.status_code, 200) assert_not_equal(res.content.decode(), 'Done') @responses.activate diff --git a/website/settings/__init__.py b/website/settings/__init__.py index 38da961d50b..e533b3a4619 100644 --- a/website/settings/__init__.py +++ b/website/settings/__init__.py @@ -21,7 +21,7 @@ if not DEV_MODE: from . import local from . import defaults - for setting in ('WATERBUTLER_JWE_SECRET', 'WATERBUTLER_JWE_SALT', 'WATERBUTLER_JWT_SECRET', 'JWT_SECRET', 'DEFAULT_HMAC_SECRET', 'POPULAR_LINKS_NODE', 'NEW_AND_NOTEWORTHY_LINKS_NODE', 'SENSITIVE_DATA_SALT', 'SENSITIVE_DATA_SECRET'): + for setting in ('WATERBUTLER_JWE_SECRET', 'WATERBUTLER_JWE_SALT', 'WATERBUTLER_JWT_SECRET', 'JWT_SECRET', 'DEFAULT_HMAC_SECRET', 'CRON_SIGNED_URL_SECRET', 'POPULAR_LINKS_NODE', 'NEW_AND_NOTEWORTHY_LINKS_NODE', 'SENSITIVE_DATA_SALT', 'SENSITIVE_DATA_SECRET'): assert getattr(local, setting, None) and getattr(local, setting, None) != getattr(defaults, setting, None), '{} must be specified in local.py when DEV_MODE is False'.format(setting) diff --git a/website/settings/defaults.py b/website/settings/defaults.py index 644f84ba689..1b57ba7516f 100644 --- a/website/settings/defaults.py +++ b/website/settings/defaults.py @@ -328,6 +328,13 @@ def parent_dir(path): DEFAULT_HMAC_SECRET = 'changeme' DEFAULT_HMAC_ALGORITHM = hashlib.sha256 + +# signed-URL auth for cron-triggered admin endpoints (No.1 external_acc_update, +# No.82 GatherView). Deliberately separate from DEFAULT_HMAC_SECRET (WaterButler<->OSF) +# so a leak of one does not affect the other. +CRON_SIGNED_URL_SECRET = 'changeme' +CRON_SIGNED_URL_HMAC_ALGORITHM = hashlib.sha256 +CRON_SIGNED_URL_TTL_SECONDS = 100 WATERBUTLER_URL = 'http://localhost:7777' WATERBUTLER_INTERNAL_URL = WATERBUTLER_URL