From d2fe1e98a50aaa932fa24304c545f9f4108dd182 Mon Sep 17 00:00:00 2001 From: Matthias Dellweg Date: Mon, 3 Jun 2024 18:16:16 +0200 Subject: [PATCH] Allow to specify the type of remote authentication When configuring remote authentication (by the reverse proxy), one should be able to augment the openAPI security specification accordingly. fixes #5437 --- .github/template_gitref | 2 +- .github/workflows/scripts/before_script.sh | 6 --- .github/workflows/scripts/install.sh | 2 +- CHANGES/.TEMPLATE.rst | 47 ------------------- CHANGES/5437.feature | 2 + pulpcore/app/settings.py | 2 + pulpcore/openapi/__init__.py | 21 +++++++++ pulpcore/tests/functional/api/test_auth.py | 21 +++++++-- .../functional/api/test_openapi_schema.py | 46 +++++++++++------- requirements.txt | 2 +- staging_docs/admin/learn/settings.md | 12 ++++- template_config.yml | 1 + 12 files changed, 85 insertions(+), 79 deletions(-) delete mode 100644 CHANGES/.TEMPLATE.rst create mode 100644 CHANGES/5437.feature diff --git a/.github/template_gitref b/.github/template_gitref index bfe538dd7f..dadaa1c99c 100644 --- a/.github/template_gitref +++ b/.github/template_gitref @@ -1 +1 @@ -2021.08.26-337-g7c7a09a +2021.08.26-338-g2237db8 diff --git a/.github/workflows/scripts/before_script.sh b/.github/workflows/scripts/before_script.sh index 386e978677..1cdfe29222 100755 --- a/.github/workflows/scripts/before_script.sh +++ b/.github/workflows/scripts/before_script.sh @@ -36,12 +36,6 @@ tail -v -n +1 .ci/ansible/Containerfile cmd_prefix bash -c "echo '%wheel ALL=(ALL) NOPASSWD: ALL' > /etc/sudoers.d/nopasswd" cmd_prefix bash -c "usermod -a -G wheel pulp" -SCENARIOS=("pulp" "performance" "azure" "gcp" "s3" "generate-bindings" "lowerbounds") -if [[ " ${SCENARIOS[*]} " =~ " ${TEST} " ]]; then - # Many functional tests require these - cmd_prefix dnf install -yq lsof which -fi - if [[ "${REDIS_DISABLED:-false}" == true ]]; then cmd_prefix bash -c "s6-rc -d change redis" echo "The Redis service was disabled for $TEST" diff --git a/.github/workflows/scripts/install.sh b/.github/workflows/scripts/install.sh index b20d5eb7be..c2470bdd0e 100755 --- a/.github/workflows/scripts/install.sh +++ b/.github/workflows/scripts/install.sh @@ -126,7 +126,7 @@ if [ "$TEST" = "azure" ]; then - ./azurite:/etc/pulp\ command: "azurite-blob --blobHost 0.0.0.0 --cert /etc/pulp/azcert.pem --key /etc/pulp/azkey.pem"' vars/main.yaml sed -i -e '$a azure_test: true\ -pulp_scenario_settings: {"domain_enabled": true}\ +pulp_scenario_settings: {"domain_enabled": true, "rest_framework__default_authentication_classes": "@merge pulpcore.app.authentication.PulpRemoteUserAuthentication"}\ pulp_scenario_env: {"otel_bsp_max_export_batch_size": 1, "otel_bsp_max_queue_size": 1, "otel_exporter_otlp_endpoint": "http://localhost:4318", "otel_exporter_otlp_protocol": "http/protobuf", "otel_metric_export_interval": 800, "pulp_otel_enabled": "true"}\ ' vars/main.yaml fi diff --git a/CHANGES/.TEMPLATE.rst b/CHANGES/.TEMPLATE.rst deleted file mode 100644 index 49c2305d7b..0000000000 --- a/CHANGES/.TEMPLATE.rst +++ /dev/null @@ -1,47 +0,0 @@ -{% if render_title %} -{% if versiondata.name %} -{{ versiondata.name }} {{ versiondata.version }} ({{ versiondata.date }}) -{{ top_underline * ((versiondata.name + versiondata.version + versiondata.date)|length + 4)}} -{% else %} -{{ versiondata.version }} ({{ versiondata.date }}) -{{ top_underline * ((versiondata.version + versiondata.date)|length + 3)}} -{% endif %} -{% endif %} -{% for section, _ in sections.items() %} -{% set underline = underlines[0] %}{% if section %}{{section}} -{{ underline * section|length }}{% set underline = underlines[1] %} - -{% endif %} - -{% if sections[section] %} -{% for category, val in definitions.items() if category in sections[section]%} -{{ definitions[category]['name'] }} -{{ underline * definitions[category]['name']|length }} - -{% if definitions[category]['showcontent'] %} -{% for text, values in sections[section][category].items() %} -- {{ text }} - {{ values|join(',\n ') }} -{% endfor %} - -{% else %} -- {{ sections[section][category]['']|join(', ') }} - -{% endif %} -{% if sections[section][category]|length == 0 %} -No significant changes. - -{% else %} -{% endif %} - -{% endfor %} -{% else %} -No significant changes. - - -{% endif %} -{% endfor %} ----- - - - diff --git a/CHANGES/5437.feature b/CHANGES/5437.feature new file mode 100644 index 0000000000..d2c6eafe38 --- /dev/null +++ b/CHANGES/5437.feature @@ -0,0 +1,2 @@ +Add ability to configure the openapi schema for remote user authentication via `REMOTE_USER_OPENAPI_SECURITY_SCHEME`. +Its type defaults to "mutualTLS" for cert based authentication. diff --git a/pulpcore/app/settings.py b/pulpcore/app/settings.py index d7defa5f1f..a4e264c2a0 100644 --- a/pulpcore/app/settings.py +++ b/pulpcore/app/settings.py @@ -264,6 +264,7 @@ TMPFILE_PROTECTION_TIME = 0 REMOTE_USER_ENVIRON_NAME = "REMOTE_USER" +REMOTE_USER_OPENAPI_SECURITY_SCHEME = {"type": "mutualTLS"} AUTHENTICATION_JSON_HEADER = "" AUTHENTICATION_JSON_HEADER_JQ_FILTER = "" @@ -288,6 +289,7 @@ "COMPONENT_NO_READ_ONLY_REQUIRED": True, "GENERIC_ADDITIONAL_PROPERTIES": None, "DISABLE_ERRORS_AND_WARNINGS": not DEBUG, + "OAS_VERSION": "3.1.0", "TITLE": "Pulp 3 API", "DESCRIPTION": "Fetch, Upload, Organize, and Distribute Software Packages", "VERSION": "v3", diff --git a/pulpcore/openapi/__init__.py b/pulpcore/openapi/__init__.py index cbca50a52c..d06f0c96b0 100644 --- a/pulpcore/openapi/__init__.py +++ b/pulpcore/openapi/__init__.py @@ -499,6 +499,7 @@ def get_schema(self, request=None, public=False): paths=self.parse(request, public), components=self.registry.build(spectacular_settings.APPEND_COMPONENTS), version=self.api_version or getattr(request, "version", None), + webhooks=None, ) for hook in spectacular_settings.POSTPROCESSING_HOOKS: result = hook(result=result, generator=self, request=request, public=public) @@ -518,9 +519,29 @@ def get_schema(self, request=None, public=False): server_url = "http://localhost:24817" if not request else request.build_absolute_uri("/") result["servers"] = [{"url": server_url}] + if "bindings" in request.query_params: + # Remove all but basic auth for bindings generation + securitySchemes = { + k: v for k, v in result["components"]["securitySchemes"].items() if k == "basicAuth" + } + result["components"]["securitySchemes"] = securitySchemes + for path in result["paths"].values(): + for operation in path.values(): + security = operation.get("security") + if security: + operation["security"] = [item for item in security if "basicAuth" in item] + return normalize_result_object(result) +class PulpRemoteUserAuthenticationScheme(OpenApiAuthenticationExtension): + target_class = "pulpcore.app.authentication.PulpRemoteUserAuthentication" + name = "remoteUserAuthentication" + + def get_security_definition(self, auto_schema): + return settings.REMOTE_USER_OPENAPI_SECURITY_SCHEME + + class JSONHeaderRemoteAuthenticationScheme(OpenApiAuthenticationExtension): target_class = "pulpcore.app.authentication.JSONHeaderRemoteAuthentication" name = "json_header_remote_authentication" diff --git a/pulpcore/tests/functional/api/test_auth.py b/pulpcore/tests/functional/api/test_auth.py index 68644e5040..9a6f84ace7 100644 --- a/pulpcore/tests/functional/api/test_auth.py +++ b/pulpcore/tests/functional/api/test_auth.py @@ -14,6 +14,11 @@ @pytest.mark.parallel +@pytest.mark.skipif( + "rest_framework.authentication.BasicAuthentication" + not in settings.REST_FRAMEWORK["DEFAULT_AUTHENTICATION_CLASSES"], + reason="Test can't run unless BasicAuthentication is enabled", +) def test_base_auth_success(pulpcore_bindings, pulp_admin_user): """Perform HTTP basic authentication with valid credentials. @@ -27,6 +32,11 @@ def test_base_auth_success(pulpcore_bindings, pulp_admin_user): @pytest.mark.parallel +@pytest.mark.skipif( + "rest_framework.authentication.BasicAuthentication" + not in settings.REST_FRAMEWORK["DEFAULT_AUTHENTICATION_CLASSES"], + reason="Test can't run unless BasicAuthentication is enabled", +) def test_base_auth_failure(pulpcore_bindings, invalid_user): """Perform HTTP basic authentication with invalid credentials. @@ -44,6 +54,11 @@ def test_base_auth_failure(pulpcore_bindings, invalid_user): @pytest.mark.parallel +@pytest.mark.skipif( + "rest_framework.authentication.BasicAuthentication" + not in settings.REST_FRAMEWORK["DEFAULT_AUTHENTICATION_CLASSES"], + reason="Test can't run unless BasicAuthentication is enabled", +) def test_base_auth_required(pulpcore_bindings, anonymous_user): """Perform HTTP basic authentication with no credentials. @@ -63,7 +78,7 @@ def test_base_auth_required(pulpcore_bindings, anonymous_user): @pytest.mark.parallel @pytest.mark.skipif( "django.contrib.auth.backends.RemoteUserBackend" not in settings.AUTHENTICATION_BACKENDS - and "pulpcore.app.authentication.JSONHeaderRemoteAuthentication" + or "pulpcore.app.authentication.JSONHeaderRemoteAuthentication" not in settings.REST_FRAMEWORK["DEFAULT_AUTHENTICATION_CLASSES"], reason="Test can't run unless RemoteUserBackend and JSONHeaderRemoteAuthentication are enabled", ) @@ -86,7 +101,7 @@ def test_jq_header_remote_auth(pulpcore_bindings, anonymous_user): @pytest.mark.parallel @pytest.mark.skipif( "django.contrib.auth.backends.RemoteUserBackend" not in settings.AUTHENTICATION_BACKENDS - and "pulpcore.app.authentication.JSONHeaderRemoteAuthentication" + or "pulpcore.app.authentication.JSONHeaderRemoteAuthentication" not in settings.REST_FRAMEWORK["DEFAULT_AUTHENTICATION_CLASSES"], reason="Test can't run unless RemoteUserBackend and JSONHeaderRemoteAuthentication are enabled", ) @@ -115,7 +130,7 @@ def test_jq_header_remote_auth_denied_by_wrong_header(pulpcore_bindings, anonymo @pytest.mark.parallel @pytest.mark.skipif( "django.contrib.auth.backends.RemoteUserBackend" not in settings.AUTHENTICATION_BACKENDS - and "pulpcore.app.authentication.JSONHeaderRemoteAuthentication" + or "pulpcore.app.authentication.JSONHeaderRemoteAuthentication" not in settings.REST_FRAMEWORK["DEFAULT_AUTHENTICATION_CLASSES"], reason="Test can't run unless RemoteUserBackend and JSONHeaderRemoteAuthentication are enabled", ) diff --git a/pulpcore/tests/functional/api/test_openapi_schema.py b/pulpcore/tests/functional/api/test_openapi_schema.py index 6a40354d8a..3610642376 100644 --- a/pulpcore/tests/functional/api/test_openapi_schema.py +++ b/pulpcore/tests/functional/api/test_openapi_schema.py @@ -1,17 +1,20 @@ """Test related to the openapi schema Pulp generates.""" -import asyncio import copy import json +import os -import aiohttp +import requests import pytest import jsonschema -from drf_spectacular.validation import JSON_SCHEMA_SPEC_PATH -from jsonschema import ValidationError +from drf_spectacular import validation from collections import defaultdict +JSON_SCHEMA_SPEC_PATH = os.path.join( + os.path.dirname(validation.__file__), "openapi_3_1_schema.json" +) + @pytest.fixture(scope="session") def openapi3_schema_spec(): @@ -25,9 +28,9 @@ def openapi3_schema_spec(): def openapi3_schema_with_modified_safe_chars(openapi3_schema_spec): openapi3_schema_spec_copy = copy.deepcopy(openapi3_schema_spec) # Don't modify the original # Making OpenAPI validation to accept paths starting with / and { - properties = openapi3_schema_spec_copy["definitions"]["Paths"]["patternProperties"] - properties["^\\/|{"] = properties["^\\/"] - del properties["^\\/"] + properties = openapi3_schema_spec_copy["$defs"]["paths"]["patternProperties"] + properties["^/|{"] = properties["^/"] + del properties["^/"] return openapi3_schema_spec_copy @@ -39,19 +42,12 @@ def pulp_openapi_schema_url(pulp_api_v3_url): @pytest.fixture(scope="session") def pulp_openapi_schema(pulp_openapi_schema_url): - return asyncio.run(_download_schema(pulp_openapi_schema_url)) + return requests.get(pulp_openapi_schema_url).json() @pytest.fixture(scope="session") def pulp_openapi_schema_pk_path_set(pulp_openapi_schema_url): - url = f"{pulp_openapi_schema_url}?pk_path=1" - return asyncio.run(_download_schema(url)) - - -async def _download_schema(url): - async with aiohttp.ClientSession() as session: - async with session.get(url) as response: - return await response.json() + return requests.get(f"{pulp_openapi_schema_url}?pk_path=1").json() @pytest.mark.parallel @@ -63,7 +59,7 @@ def test_valid_with_pk_path_set(pulp_openapi_schema_pk_path_set, openapi3_schema @pytest.mark.parallel @pytest.mark.from_pulpcore_for_all_plugins def test_invalid_default_schema(pulp_openapi_schema, openapi3_schema_spec): - with pytest.raises(ValidationError): + with pytest.raises(jsonschema.ValidationError): jsonschema.validate(instance=pulp_openapi_schema, schema=openapi3_schema_spec) @@ -88,12 +84,26 @@ def test_no_dup_operation_ids(pulp_openapi_schema): assert len(dup_ids) == 0, f"Duplicate operationIds found: {dup_ids}" +@pytest.mark.parallel +def test_remote_user_auth_security_scheme(pulp_settings, pulp_openapi_schema): + if ( + "pulpcore.app.authentication.PulpRemoteUserAuthentication" + not in pulp_settings.REST_FRAMEWORK["DEFAULT_AUTHENTICATION_CLASSES"] + ): + pytest.skip("Test can't run unless PulpRemoteUserAuthentication is enabled.") + + expected_security_scheme = pulp_settings.REMOTE_USER_OPENAPI_SECURITY_SCHEME + security_schemes = pulp_openapi_schema["components"]["securitySchemes"] + + assert security_schemes["remoteUserAuthentication"] == expected_security_scheme + + @pytest.mark.parallel def test_external_auth_on_security_scheme(pulp_settings, pulp_openapi_schema): if ( "django.contrib.auth.backends.RemoteUserBackend" not in pulp_settings.AUTHENTICATION_BACKENDS - and "pulpcore.app.authentication.JSONHeaderRemoteAuthentication" + or "pulpcore.app.authentication.JSONHeaderRemoteAuthentication" not in pulp_settings.REST_FRAMEWORK["DEFAULT_AUTHENTICATION_CLASSES"] ): pytest.skip( diff --git a/requirements.txt b/requirements.txt index 306c537dbf..17d747ef92 100644 --- a/requirements.txt +++ b/requirements.txt @@ -15,7 +15,7 @@ djangorestframework>=3.14.0,<=3.15.1 djangorestframework-queryfields>=1.0,<=1.1.0 drf-access-policy>=1.1.2,<1.5.1 drf-nested-routers>=0.93.4,<=0.94.1 -drf-spectacular==0.26.5 # We monkeypatch this so we need a very narrow requirement string +drf-spectacular==0.27.2 # We monkeypatch this so we need a very narrow requirement string dynaconf>=3.1.12,<3.2.6 gunicorn>=20.1,<22.1.0 importlib-metadata>=6.0.1,<=6.0.1 # Pinned to fix opentelemetry dependency solving issues with pip diff --git a/staging_docs/admin/learn/settings.md b/staging_docs/admin/learn/settings.md index 0d95f11dca..c89bdc709f 100644 --- a/staging_docs/admin/learn/settings.md +++ b/staging_docs/admin/learn/settings.md @@ -272,8 +272,8 @@ Defaults to `30` seconds. ### REMOTE_USER_ENVIRON_NAME -The name of the WSGI environment variable to read for `webserver authentication -`. +The name of the WSGI environment variable to read for `webserver authentication `. +It is only used with the `PulpRemoteUserAuthentication` authentication class. !!! warning Configuring this has serious security implications. See the [Django warning at the end of this @@ -283,6 +283,14 @@ Defaults to `'REMOTE_USER'`. +### REMOTE_USER_OPENAPI_SECURITY_SCHEME + +A JSON object representing the security scheme advertised for the `PulpRemoteUserAuthentication` authentication class. + +Defaults to `{"type": "mutualTLS"}`, which represents x509 certificate based authentication. + + + ### ALLOWED_IMPORT_PATHS One or more real filesystem paths that Remotes with filesystem paths can import from. For example diff --git a/template_config.yml b/template_config.yml index 87c1547ae8..699c43d23e 100644 --- a/template_config.yml +++ b/template_config.yml @@ -71,6 +71,7 @@ pulp_settings: upload_protection_time: 10 pulp_settings_azure: domain_enabled: true + rest_framework__default_authentication_classes: '@merge pulpcore.app.authentication.PulpRemoteUserAuthentication' pulp_settings_gcp: null pulp_settings_s3: authentication_backends: '@merge django.contrib.auth.backends.RemoteUserBackend'