diff --git a/assets/src/bundles/save/index.js b/assets/src/bundles/save/index.js --- a/assets/src/bundles/save/index.js +++ b/assets/src/bundles/save/index.js @@ -5,20 +5,16 @@ * See top-level LICENSE file for more information */ -import {handleFetchError, csrfPost, isGitRepoUrl, htmlAlert, removeUrlFragment} from 'utils/functions'; +import {handleFetchError, isGitRepoUrl, htmlAlert, removeUrlFragment} from 'utils/functions'; import {swhSpinnerSrc} from 'utils/constants'; let saveRequestsTable; function originSaveRequest(originType, originUrl, acceptedCallback, pendingCallback, errorCallback) { - let addSaveOriginRequestUrl = Urls.origin_save_request(originType, originUrl); - let headers = { - 'Accept': 'application/json', - 'Content-Type': 'application/json' - }; + let addSaveOriginRequestUrl = Urls.api_1_save_origin(originType, originUrl); $('.swh-processing-save-request').css('display', 'block'); - csrfPost(addSaveOriginRequestUrl, headers) + fetch(addSaveOriginRequestUrl, {method: 'POST'}) .then(handleFetchError) .then(response => response.json()) .then(data => { diff --git a/cypress/integration/origin-save.spec.js b/cypress/integration/origin-save.spec.js --- a/cypress/integration/origin-save.spec.js +++ b/cypress/integration/origin-save.spec.js @@ -94,7 +94,7 @@ before(function() { url = this.Urls.origin_save(); origin = this.origin[0]; - this.originSaveUrl = this.Urls.origin_save_request(origin.type, origin.url); + this.originSaveUrl = this.Urls.api_1_save_origin(origin.type, origin.url); }); beforeEach(function() { @@ -139,7 +139,7 @@ it('should validate gitlab subproject url', function() { const gitlabSubProjectUrl = 'https://gitlab.com/user/project/sub/'; - const originSaveUrl = this.Urls.origin_save_request('git', gitlabSubProjectUrl); + const originSaveUrl = this.Urls.api_1_save_origin('git', gitlabSubProjectUrl); stubSaveRequest({requestUrl: originSaveUrl, saveRequestStatus: 'accepted', @@ -155,7 +155,7 @@ it('should validate project url with _ in username', function() { const gitlabSubProjectUrl = 'https://gitlab.com/user_name/project.git'; - const originSaveUrl = this.Urls.origin_save_request('git', gitlabSubProjectUrl); + const originSaveUrl = this.Urls.api_1_save_origin('git', gitlabSubProjectUrl); stubSaveRequest({requestUrl: originSaveUrl, saveRequestStatus: 'accepted', @@ -357,7 +357,7 @@ const badVisitType = 'hg'; const goodVisitType = 'git'; cy.intercept('/save/requests/list/**', {fixture: 'origin-save'}); - stubSaveRequest({requestUrl: this.Urls.origin_save_request(badVisitType, originUrl), + stubSaveRequest({requestUrl: this.Urls.api_1_save_origin(badVisitType, originUrl), visitType: badVisitType, saveRequestStatus: 'accepted', originUrl: originUrl, diff --git a/swh/web/common/utils.py b/swh/web/common/utils.py --- a/swh/web/common/utils.py +++ b/swh/web/common/utils.py @@ -1,4 +1,4 @@ -# Copyright (C) 2017-2020 The Software Heritage developers +# Copyright (C) 2017-2021 The Software Heritage developers # See the AUTHORS file at the top-level directory of this distribution # License: GNU Affero General Public License version 3, or any later version # See top-level LICENSE file for more information @@ -18,7 +18,6 @@ from django.http import HttpRequest, QueryDict from django.urls import reverse as django_reverse -from rest_framework.authentication import SessionAuthentication from swh.web.common.exc import BadInputExc from swh.web.common.typing import QueryParameters @@ -277,18 +276,6 @@ } -class EnforceCSRFAuthentication(SessionAuthentication): - """ - Helper class to enforce CSRF validation on a DRF view - when a user is not authenticated. - """ - - def authenticate(self, request): - user = getattr(request._request, "user", None) - self.enforce_csrf(request) - return (user, None) - - def resolve_branch_alias( snapshot: Dict[str, Any], branch: Optional[Dict[str, Any]] ) -> Optional[Dict[str, Any]]: diff --git a/swh/web/misc/origin_save.py b/swh/web/misc/origin_save.py --- a/swh/web/misc/origin_save.py +++ b/swh/web/misc/origin_save.py @@ -8,17 +8,12 @@ from django.db.models import Q from django.http import JsonResponse from django.shortcuts import render -from rest_framework.decorators import api_view, authentication_classes -from swh.web.api.throttling import throttle_scope -from swh.web.common.exc import ForbiddenExc from swh.web.common.models import SaveOriginRequest from swh.web.common.origin_save import ( - create_save_origin_request, get_savable_visit_types, get_save_origin_task_info, ) -from swh.web.common.utils import EnforceCSRFAuthentication def _origin_save_view(request): @@ -29,24 +24,6 @@ ) -@api_view(["POST"]) -@authentication_classes((EnforceCSRFAuthentication,)) -@throttle_scope("swh_save_origin") -def _origin_save_request(request, visit_type, origin_url): - """ - This view is called through AJAX from the save code now form of swh-web. - We use DRF here as we want to rate limit the number of submitted requests - per user to avoid being possibly flooded by bots. - """ - try: - response = create_save_origin_request(visit_type, origin_url) - return JsonResponse(response) - except ForbiddenExc as exc: - return JsonResponse({"detail": str(exc)}, status=403) - except Exception as exc: - return JsonResponse({"detail": str(exc)}, status=500) - - def _visit_save_types_list(request): visit_types = get_savable_visit_types() return JsonResponse(visit_types, safe=False) @@ -102,11 +79,6 @@ urlpatterns = [ url(r"^save/$", _origin_save_view, name="origin-save"), - url( - r"^save/(?P.+)/url/(?P.+)/$", - _origin_save_request, - name="origin-save-request", - ), url(r"^save/types/list/$", _visit_save_types_list, name="origin-save-types-list"), url( r"^save/requests/list/(?P.+)/$", diff --git a/swh/web/tests/api/views/test_origin_save.py b/swh/web/tests/api/views/test_origin_save.py --- a/swh/web/tests/api/views/test_origin_save.py +++ b/swh/web/tests/api/views/test_origin_save.py @@ -25,7 +25,12 @@ SaveUnauthorizedOrigin, ) from swh.web.common.utils import reverse -from swh.web.tests.utils import check_api_get_responses, check_api_post_responses +from swh.web.settings.tests import save_origin_rate_post +from swh.web.tests.utils import ( + check_api_get_responses, + check_api_post_response, + check_api_post_responses, +) pytestmark = pytest.mark.django_db @@ -374,3 +379,51 @@ ) % unknown_origin_url, } + + +_visit_type = "git" +_origin_url = "https://github.com/python/cpython" + + +def test_save_requests_rate_limit(api_client, mocker): + create_save_origin_request = mocker.patch( + "swh.web.api.views.origin_save.create_save_origin_request" + ) + + def _save_request_dict(*args, **kwargs): + return { + "id": 1, + "visit_type": _visit_type, + "origin_url": _origin_url, + "save_request_date": datetime.now().isoformat(), + "save_request_status": SAVE_REQUEST_ACCEPTED, + "save_task_status": SAVE_TASK_NOT_YET_SCHEDULED, + "visit_date": None, + "visit_status": None, + } + + create_save_origin_request.side_effect = _save_request_dict + + url = reverse( + "api-1-save-origin", + url_args={"visit_type": _visit_type, "origin_url": _origin_url}, + ) + + for _ in range(save_origin_rate_post): + check_api_post_response(api_client, url, status_code=200) + + check_api_post_response(api_client, url, status_code=429) + + +def test_save_request_form_server_error(api_client, mocker): + create_save_origin_request = mocker.patch( + "swh.web.api.views.origin_save.create_save_origin_request" + ) + create_save_origin_request.side_effect = Exception("Server error") + + url = reverse( + "api-1-save-origin", + url_args={"visit_type": _visit_type, "origin_url": _origin_url}, + ) + + check_api_post_responses(api_client, url, status_code=500) diff --git a/swh/web/tests/misc/test_origin_save.py b/swh/web/tests/misc/test_origin_save.py --- a/swh/web/tests/misc/test_origin_save.py +++ b/swh/web/tests/misc/test_origin_save.py @@ -1,4 +1,4 @@ -# Copyright (C) 2019 The Software Heritage developers +# Copyright (C) 2019-2021 The Software Heritage developers # See the AUTHORS file at the top-level directory of this distribution # License: GNU Affero General Public License version 3, or any later version # See top-level LICENSE file for more information @@ -8,79 +8,10 @@ import pytest -from django.test import Client - from swh.web.common.models import SaveOriginRequest -from swh.web.common.origin_save import ( - SAVE_REQUEST_ACCEPTED, - SAVE_TASK_NOT_YET_SCHEDULED, - SAVE_TASK_SUCCEEDED, -) +from swh.web.common.origin_save import SAVE_REQUEST_ACCEPTED, SAVE_TASK_SUCCEEDED from swh.web.common.utils import reverse -from swh.web.settings.tests import save_origin_rate_post -from swh.web.tests.utils import ( - check_api_post_response, - check_http_get_response, - check_http_post_response, -) - -visit_type = "git" -origin = {"url": "https://github.com/python/cpython"} - - -@pytest.fixture -def client(): - return Client(enforce_csrf_checks=True) - - -def test_save_request_form_csrf_token(client, mocker): - mock_create_save_origin_request = mocker.patch( - "swh.web.misc.origin_save.create_save_origin_request" - ) - _mock_create_save_origin_request(mock_create_save_origin_request) - - url = reverse( - "origin-save-request", - url_args={"visit_type": visit_type, "origin_url": origin["url"]}, - ) - - check_http_post_response(client, url, status_code=403) - - data = _get_csrf_token(client, reverse("origin-save")) - check_api_post_response(client, url, data=data, status_code=200) - - -def test_save_request_form_rate_limit(client, mocker): - mock_create_save_origin_request = mocker.patch( - "swh.web.misc.origin_save.create_save_origin_request" - ) - _mock_create_save_origin_request(mock_create_save_origin_request) - - url = reverse( - "origin-save-request", - url_args={"visit_type": visit_type, "origin_url": origin["url"]}, - ) - - data = _get_csrf_token(client, reverse("origin-save")) - for _ in range(save_origin_rate_post): - check_api_post_response(client, url, data=data, status_code=200) - - check_api_post_response(client, url, data=data, status_code=429) - - -def test_save_request_form_server_error(client, mocker): - mock_create_save_origin_request = mocker.patch( - "swh.web.misc.origin_save.create_save_origin_request" - ) - mock_create_save_origin_request.side_effect = Exception("Server error") - - url = reverse( - "origin-save-request", - url_args={"visit_type": visit_type, "origin_url": origin["url"]}, - ) - - data = _get_csrf_token(client, reverse("origin-save")) - check_api_post_response(client, url, data=data, status_code=500) +from swh.web.tests.utils import check_http_get_response def test_old_save_url_redirection(client): @@ -163,20 +94,3 @@ assert sors["recordsTotal"] == len(visit_types) * nb_origins_per_type assert len(sors["data"]) == nb_origins_per_type assert all(d["visit_type"] == visit_type for d in sors["data"]) - - -def _get_csrf_token(client, url): - resp = client.get(url) - return {"csrfmiddlewaretoken": resp.cookies["csrftoken"].value} - - -def _mock_create_save_origin_request(mock): - expected_data = { - "visit_type": visit_type, - "origin_url": origin["url"], - "save_request_date": datetime.now().isoformat(), - "save_request_status": SAVE_REQUEST_ACCEPTED, - "save_task_status": SAVE_TASK_NOT_YET_SCHEDULED, - "visit_date": None, - } - mock.return_value = expected_data