From 3591a07725139cf1c5e37bacbcf38f515d1a5c96 Mon Sep 17 00:00:00 2001 From: Marco Cucchi Date: Mon, 16 Sep 2024 16:36:02 +0200 Subject: [PATCH] show url to accept licences in PermissionDenied message --- cads_processing_api_service/auth.py | 65 +++++++++++++------------- cads_processing_api_service/clients.py | 14 ++++-- cads_processing_api_service/config.py | 12 +++++ tests/test_30_auth.py | 16 +++++-- 4 files changed, 66 insertions(+), 41 deletions(-) diff --git a/cads_processing_api_service/auth.py b/cads_processing_api_service/auth.py index 0cffefb7..cc000fc2 100644 --- a/cads_processing_api_service/auth.py +++ b/cads_processing_api_service/auth.py @@ -183,17 +183,25 @@ def get_accepted_licences(auth_header: tuple[str, str]) -> set[tuple[str, int]]: return accepted_licences -def check_licences( - required_licences: set[tuple[str, int]], accepted_licences: set[tuple[str, int]] +def verify_licences( + accepted_licences: set[tuple[str, int]] | list[tuple[str, int]], + required_licences: set[tuple[str, int]] | list[tuple[str, int]], + api_request_url: str, + process_id: str, ) -> set[tuple[str, int]]: - """Check if accepted licences satisfy required ones. + """ + Verify if all the licences required for the process submission have been accepted. Parameters ---------- - required_licences : set[tuple[str, int]] - Required licences. - accepted_licences : set[tuple[str, int]] - Accepted licences. + accepted_licences : set[tuple[str, int]] | list[tuple[str, int]], + Licences accepted by a user stored in the Extended Profiles database. + required_licences : set[tuple[str, int]] | list[tuple[str, int]], + Licences bound to the required process/dataset. + api_request_url : str + API request URL, required to generate the URL to the dataset licences page. + process_id : str + Process identifier, required to generate the URL to the dataset licences page. Returns ------- @@ -205,39 +213,30 @@ def check_licences( exceptions.PermissionDenied Raised if not all required licences have been accepted. """ + if not isinstance(accepted_licences, set): + accepted_licences = set(accepted_licences) + if not isinstance(required_licences, set): + required_licences = set(required_licences) missing_licences = required_licences - accepted_licences if not len(missing_licences) == 0: - missing_licences_detail = [ - {"id": licence[0], "revision": licence[1]} for licence in missing_licences - ] + missing_licences_message_template = ( + config.ensure_settings().missing_licences_message + ) + dataset_licences_url_template = config.ensure_settings().dataset_licences_url + parsed_api_request_url = urllib.parse.urlparse(api_request_url) + base_url = f"{parsed_api_request_url.scheme}://{parsed_api_request_url.netloc}" + dataset_licences_url = dataset_licences_url_template.format( + base_url=base_url, process_id=process_id + ) + missing_licences_message = missing_licences_message_template.format( + dataset_licences_url=dataset_licences_url + ) raise exceptions.PermissionDenied( - title="required licences not accepted", - detail=( - "required licences not accepted; " - "please accept the following licences to proceed: " - f"{missing_licences_detail}" - ), + title="required licences not accepted", detail=missing_licences_message ) return missing_licences -def validate_licences( - accepted_licences: set[tuple[str, str]], - licences: list[tuple[str, int]], -) -> None: - """Validate process execution request's payload in terms of required licences. - - Parameters - ---------- - stored_accepted_licences : set[tuple[str, str]] - Licences accepted by a user stored in the Extended Profiles database. - licences : list[tuple[str, int]] - Licences bound to the required process/dataset. - """ - required_licences = set(licences) - check_licences(required_licences, accepted_licences) # type: ignore - - def verify_if_disabled(disabled_reason: str | None, user_role: str | None) -> None: """Verify if a dataset's disabling reason grant access to the dataset for a specific user role. diff --git a/cads_processing_api_service/clients.py b/cads_processing_api_service/clients.py index c2ce5e26..f0fc489e 100644 --- a/cads_processing_api_service/clients.py +++ b/cads_processing_api_service/clients.py @@ -201,6 +201,8 @@ def post_process_execution( Parameters ---------- + api_request: fastapi.Request + API Request object. process_id : str Process identifier. execution_content : models.Execute @@ -243,15 +245,21 @@ def post_process_execution( ) as exc: raise exceptions.InvalidRequest(detail=str(exc)) from exc costs = auth.verify_cost(request_inputs, adaptor_properties, request_origin) - licences = adaptor.get_licences(request_inputs) + required_licences = adaptor.get_licences(request_inputs) if user_uid != "anonymous": accepted_licences = auth.get_accepted_licences(auth_header) - auth.validate_licences(accepted_licences, licences) + api_request_url = str(api_request.url) + _ = auth.verify_licences( + accepted_licences, required_licences, api_request_url, process_id + ) job_message = None else: job_message = config.ensure_settings().anonymous_licences_message.format( licences="; ".join( - [f"{licence[0]} (rev: {licence[1]})" for licence in licences] + [ + f"{licence[0]} (rev: {licence[1]})" + for licence in required_licences + ] ) ) job_id = str(uuid.uuid4()) diff --git a/cads_processing_api_service/config.py b/cads_processing_api_service/config.py index 34d95bb8..6016d6e7 100644 --- a/cads_processing_api_service/config.py +++ b/cads_processing_api_service/config.py @@ -42,6 +42,14 @@ "If you are using cdsapi, please upgrade to the latest version." ) + +MISSING_LICENCES_MESSAGE = ( + "Not all the required licences have been accepted; " + "please visit {dataset_licences_url} " + "to accept the required licence(s)." +) + + general_settings = None @@ -66,6 +74,10 @@ class Settings(pydantic_settings.BaseSettings): missing_dataset_title: str = "Dataset not available" anonymous_licences_message: str = ANONYMOUS_LICENCES_MESSAGE deprecation_warning_message: str = DEPRECATION_WARNING_MESSAGE + missing_licences_message: str = MISSING_LICENCES_MESSAGE + dataset_licences_url: str = ( + "{base_url}/datasets/{process_id}?tab=download#manage-licences" + ) download_nodes_config: str = "/etc/retrieve-api/download-nodes.config" diff --git a/tests/test_30_auth.py b/tests/test_30_auth.py index 03c53c98..1fde4de6 100644 --- a/tests/test_30_auth.py +++ b/tests/test_30_auth.py @@ -22,16 +22,22 @@ from cads_processing_api_service import auth, exceptions, models -def test_check_licences() -> None: - required_licences = {("licence_1", 1), ("licence_2", 2)} +def test_verify_licences() -> None: accepted_licences = {("licence_1", 1), ("licence_2", 2), ("licence_3", 3)} - missing_licences = auth.check_licences(required_licences, accepted_licences) + required_licences = {("licence_1", 1), ("licence_2", 2)} + api_request_url = "http://base_url/api/v1/processes/process_id/execution" + process_id = "process_id" + missing_licences = auth.verify_licences( + accepted_licences, required_licences, api_request_url, process_id + ) assert len(missing_licences) == 0 - required_licences = {("licence_1", 1), ("licence_2", 2)} accepted_licences = {("licence_1", 1), ("licence_2", 1)} + required_licences = {("licence_1", 1), ("licence_2", 2)} with pytest.raises(exceptions.PermissionDenied): - missing_licences = auth.check_licences(required_licences, accepted_licences) + missing_licences = auth.verify_licences( + accepted_licences, required_licences, api_request_url, process_id + ) def test_verify_permission() -> None: