diff --git a/swh/web/api/views/origin_save.py b/swh/web/api/views/origin_save.py --- a/swh/web/api/views/origin_save.py +++ b/swh/web/api/views/origin_save.py @@ -5,7 +5,7 @@ from swh.web.api.apidoc import api_doc, format_docstring from swh.web.api.apiurls import api_route -from swh.web.auth.utils import SWH_AMBASSADOR_PERMISSION +from swh.web.auth.utils import privileged_user from swh.web.common.origin_save import ( create_save_origin_request, get_save_origin_requests, @@ -84,12 +84,10 @@ """ if request.method == "POST": - bypass_pending_review = request.user.is_authenticated and request.user.has_perm( - SWH_AMBASSADOR_PERMISSION - ) sor = create_save_origin_request( - visit_type, origin_url, bypass_pending_review, user_id=request.user.id + visit_type, origin_url, privileged_user(request), user_id=request.user.id ) + del sor["id"] else: sor = get_save_origin_requests(visit_type, origin_url) diff --git a/swh/web/auth/utils.py b/swh/web/auth/utils.py --- a/swh/web/auth/utils.py +++ b/swh/web/auth/utils.py @@ -70,3 +70,14 @@ The decrypted data """ return _get_fernet(password, salt).decrypt(data) + + +def privileged_user(request) -> bool: + """Determine whether a user is authenticated and is a privileged one (e.g ambassador). + This allows such user to have access to some more actions (e.g. bypass save code now + review, access to 'bundle' type...) + + """ + return request.user.is_authenticated and request.user.has_perm( + SWH_AMBASSADOR_PERMISSION + ) diff --git a/swh/web/common/origin_save.py b/swh/web/common/origin_save.py --- a/swh/web/common/origin_save.py +++ b/swh/web/common/origin_save.py @@ -113,6 +113,10 @@ # TODO: do not hardcode the task name here (T1157) _visit_type_task = {"git": "load-git", "hg": "load-hg", "svn": "load-svn"} +_visit_type_task_privileged = { + "bundle": "load-archive-files", +} + # map scheduler task status to origin save status _save_task_status = { @@ -134,23 +138,31 @@ } -def get_savable_visit_types() -> List[str]: - """ - Get the list of visit types that can be performed - through a save request. +def get_savable_visit_types(privileged_user: bool = False) -> List[str]: + """Get the list of visit types that can be performed through a save request. + + Args: + privileged_user: Flag to determine if all visit types should be returned or not. + Default to False to only list unprivileged visit types. Returns: - list: the list of saveable visit types + the list of saveable visit types + """ - return sorted(list(_visit_type_task.keys())) + task_types = list(_visit_type_task.keys()) + if privileged_user: + task_types += _visit_type_task_privileged.keys() + + return sorted(task_types) -def _check_visit_type_savable(visit_type: str) -> None: - allowed_visit_types = ", ".join(get_savable_visit_types()) - if visit_type not in _visit_type_task: +def _check_visit_type_savable(visit_type: str, privileged_user: bool = False) -> None: + visit_type_tasks = get_savable_visit_types(privileged_user) + if visit_type not in visit_type_tasks: + allowed_visit_types = ", ".join(visit_type_tasks) raise BadInputExc( - "Visit of type %s can not be saved! " - "Allowed types are the following: %s" % (visit_type, allowed_visit_types) + f"Visit of type {visit_type} can not be saved! " + f"Allowed types are the following: {allowed_visit_types}" ) @@ -340,11 +352,10 @@ def create_save_origin_request( visit_type: str, origin_url: str, - bypass_pending_review: bool = False, + privileged_user: bool = False, user_id: Optional[int] = None, ) -> SaveOriginRequestInfo: - """ - Create a loading task to save a software origin into the archive. + """Create a loading task to save a software origin into the archive. This function aims to create a software origin loading task trough the use of the swh-scheduler component. @@ -360,6 +371,9 @@ Args: visit_type: the type of visit to perform (e.g git, hg, svn, ...) origin_url: the url of the origin to save + privileged_user: Whether the user has privileged_user access to extra + functionality (e.g. bypass save code now review, access to extra visit type) + user_id: User identifier (provided when authenticated) Raises: BadInputExc: the visit type or origin url is invalid or inexistent @@ -377,12 +391,11 @@ **not created**, **not yet scheduled**, **scheduled**, **succeed** or **failed** - """ - _check_visit_type_savable(visit_type) + _check_visit_type_savable(visit_type, privileged_user) _check_origin_url_valid(origin_url) # if all checks passed so far, we can try and save the origin - save_request_status = can_save_origin(origin_url, bypass_pending_review) + save_request_status = can_save_origin(origin_url, privileged_user) task = None # if the origin save request is accepted, create a scheduler @@ -778,7 +791,8 @@ SAVE_TASK_RUNNING, ) - visit_types = get_savable_visit_types() + # for metrics, we want access to all visit types + visit_types = get_savable_visit_types(privileged_user=True) labels_set = product(request_statuses, visit_types) diff --git a/swh/web/common/swh_templatetags.py b/swh/web/common/swh_templatetags.py --- a/swh/web/common/swh_templatetags.py +++ b/swh/web/common/swh_templatetags.py @@ -1,4 +1,4 @@ -# Copyright (C) 2017-2019 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 @@ -125,15 +125,16 @@ @register.filter -def visit_type_savable(visit_type): +def visit_type_savable(visit_type: str) -> bool: """Django template filter to check if a save request can be created for a given visit type. Args: - visit_type (str): the type of visit + visit_type: the type of visit Returns: If the visit type is saveable or not + """ return visit_type in get_savable_visit_types() 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 @@ -9,6 +9,7 @@ from django.http import JsonResponse from django.shortcuts import render +from swh.web.auth.utils import privileged_user from swh.web.common.models import SaveOriginRequest from swh.web.common.origin_save import ( get_savable_visit_types, @@ -24,8 +25,11 @@ ) -def _visit_save_types_list(request): - visit_types = get_savable_visit_types() +def _visit_save_types_list(request) -> JsonResponse: + """Return the list of supported visit types as json response + + """ + visit_types = get_savable_visit_types(privileged_user(request)) return JsonResponse(visit_types, safe=False) diff --git a/swh/web/tests/common/test_origin_save.py b/swh/web/tests/common/test_origin_save.py --- a/swh/web/tests/common/test_origin_save.py +++ b/swh/web/tests/common/test_origin_save.py @@ -24,6 +24,10 @@ ) from swh.web.common.origin_save import ( _check_origin_exists, + _check_visit_type_savable, + _visit_type_task, + _visit_type_task_privileged, + get_savable_visit_types, get_save_origin_requests, get_save_origin_task_info, origin_exists, @@ -105,6 +109,34 @@ return task, task_run +@pytest.mark.parametrize( + "wrong_type,privileged_user", + [ + ("dummy", True), + ("dumb", False), + ("bundle", False), # when no privilege, this is rejected + ], +) +def test__check_visit_type_savable(wrong_type, privileged_user): + + with pytest.raises(BadInputExc, match="Allowed types"): + _check_visit_type_savable(wrong_type, privileged_user) + + # when privileged_user, the following is accepted though + _check_visit_type_savable("bundle", True) + + +def test_get_savable_visit_types(): + default_list = list(_visit_type_task.keys()) + + assert set(get_savable_visit_types()) == set(default_list) + + privileged_list = default_list.copy() + privileged_list += list(_visit_type_task_privileged.keys()) + + assert set(get_savable_visit_types(privileged_user=True)) == set(privileged_list) + + def _get_save_origin_task_info_test( mocker, task_archived=False, es_available=True, full_info=True ): 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 @@ -9,11 +9,15 @@ import pytest from swh.auth.django.utils import oidc_user_from_profile +from swh.web.auth.utils import SWH_AMBASSADOR_PERMISSION from swh.web.common.models import SaveOriginRequest from swh.web.common.origin_save import SAVE_REQUEST_ACCEPTED, SAVE_TASK_SUCCEEDED from swh.web.common.utils import reverse from swh.web.tests.utils import check_http_get_response +VISIT_TYPES = ("git", "svn", "hg") +PRIVILEGED_VISIT_TYPES = tuple(list(VISIT_TYPES) + ["bundle"]) + def test_old_save_url_redirection(client): url = reverse("browse-origin-save") @@ -23,11 +27,36 @@ assert resp["location"] == redirect_url +def test_save_types_list_default(client): + """Unprivileged listing should display default list of visit types. + + """ + url = reverse("origin-save-types-list") + resp = check_http_get_response(client, url, status_code=200) + + actual_response = resp.json() + assert set(actual_response) == set(VISIT_TYPES) + + +@pytest.mark.django_db +def test_save_types_list_privileged(client, keycloak_oidc): + """Privileged listing should display all visit types. + + """ + keycloak_oidc.realm_permissions = [SWH_AMBASSADOR_PERMISSION] + client.login(code="", code_verifier="", redirect_uri="") + + url = reverse("origin-save-types-list") + resp = check_http_get_response(client, url, status_code=200) + + actual_response = resp.json() + assert set(actual_response) == set(PRIVILEGED_VISIT_TYPES) + + @pytest.mark.django_db -def test_save_origin_requests_list(client, mocker, keycloak_oidc): - visit_types = ("git", "svn", "hg") +def test_save_origin_requests_list(client, mocker): nb_origins_per_type = 10 - for visit_type in visit_types: + for visit_type in VISIT_TYPES: for i in range(nb_origins_per_type): SaveOriginRequest.objects.create( request_date=datetime.now(tz=timezone.utc), @@ -45,7 +74,7 @@ # retrieve all save requests in 3 pages, sorted in descending order # of request creation - for i, visit_type in enumerate(reversed(visit_types)): + for i, visit_type in enumerate(reversed(VISIT_TYPES)): url = reverse( "origin-save-requests-list", url_args={"status": "all"}, @@ -65,13 +94,13 @@ ) sors = json.loads(resp.content.decode("utf-8")) assert sors["draw"] == i + 1 - assert sors["recordsFiltered"] == len(visit_types) * nb_origins_per_type - assert sors["recordsTotal"] == len(visit_types) * nb_origins_per_type + assert sors["recordsFiltered"] == len(VISIT_TYPES) * nb_origins_per_type + 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"]) # retrieve save requests filtered by visit type in a single page - for i, visit_type in enumerate(reversed(visit_types)): + for i, visit_type in enumerate(reversed(VISIT_TYPES)): url = reverse( "origin-save-requests-list", url_args={"status": "all"}, @@ -92,7 +121,7 @@ sors = json.loads(resp.content.decode("utf-8")) assert sors["draw"] == i + 1 assert sors["recordsFiltered"] == nb_origins_per_type - assert sors["recordsTotal"] == len(visit_types) * nb_origins_per_type + 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"])