Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Allow to specify the type of remote authentication #5438

Draft
wants to merge 1 commit into
base: main
Choose a base branch
from
Draft
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion .github/template_gitref
Original file line number Diff line number Diff line change
@@ -1 +1 @@
2021.08.26-337-g7c7a09a
2021.08.26-338-g2237db8
6 changes: 0 additions & 6 deletions .github/workflows/scripts/before_script.sh
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down
2 changes: 1 addition & 1 deletion .github/workflows/scripts/install.sh
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
47 changes: 0 additions & 47 deletions CHANGES/.TEMPLATE.rst

This file was deleted.

2 changes: 2 additions & 0 deletions CHANGES/5437.feature
Original file line number Diff line number Diff line change
@@ -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.
2 changes: 2 additions & 0 deletions pulpcore/app/settings.py
Original file line number Diff line number Diff line change
Expand Up @@ -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 = ""
Expand All @@ -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",
Expand Down
21 changes: 21 additions & 0 deletions pulpcore/openapi/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand All @@ -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"
Expand Down
21 changes: 18 additions & 3 deletions pulpcore/tests/functional/api/test_auth.py
Original file line number Diff line number Diff line change
Expand Up @@ -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.

Expand All @@ -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.

Expand All @@ -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.

Expand All @@ -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",
)
Expand All @@ -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",
)
Expand Down Expand Up @@ -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",
)
Expand Down
46 changes: 28 additions & 18 deletions pulpcore/tests/functional/api/test_openapi_schema.py
Original file line number Diff line number Diff line change
@@ -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():
Expand All @@ -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

Expand All @@ -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
Expand All @@ -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)


Expand All @@ -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(
Expand Down
2 changes: 1 addition & 1 deletion requirements.txt
Original file line number Diff line number Diff line change
Expand Up @@ -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
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Does this PR need 0.26.5 specifically? or is this an incidental change? If the latter - including in this PR makes the PR way harder to backport.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's worse than that. Seems like we need openapi 3.1 instead of 3.0.3 and now we need to coordinate a lot of other things including the openapi-generator.
But on the one hand we'll probably not backport this feature, and otoh i split this version bump into another PR already.

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
Expand Down
12 changes: 10 additions & 2 deletions staging_docs/admin/learn/settings.md
Original file line number Diff line number Diff line change
Expand Up @@ -272,8 +272,8 @@ Defaults to `30` seconds.

### REMOTE_USER_ENVIRON_NAME

The name of the WSGI environment variable to read for `webserver authentication
<webserver-authentication>`.
The name of the WSGI environment variable to read for `webserver authentication <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
Expand All @@ -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
Expand Down
1 change: 1 addition & 0 deletions template_config.yml
Original file line number Diff line number Diff line change
Expand Up @@ -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'
Expand Down
Loading