diff --git a/swh/deposit/api/common.py b/swh/deposit/api/common.py --- a/swh/deposit/api/common.py +++ b/swh/deposit/api/common.py @@ -18,7 +18,7 @@ from django.urls import reverse from django.utils import timezone from rest_framework import status -from rest_framework.authentication import BaseAuthentication +from rest_framework.authentication import BaseAuthentication, BasicAuthentication from rest_framework.permissions import BasePermission, IsAuthenticated from rest_framework.request import Request from rest_framework.views import APIView @@ -160,32 +160,39 @@ if not origin_url.startswith(provider_url): raise DepositError( FORBIDDEN, - f"Cannot create origin {origin_url}, it must start with " f"{provider_url}", + f"Cannot create origin {origin_url}, it must start with {provider_url}", ) -class AuthenticatedAPIView(APIView): - """Mixin intended as a based API view to enforce the basic - authentication check - - """ - - authentication_classes: Sequence[Type[BaseAuthentication]] = ( - KeycloakBasicAuthentication, - ) - permission_classes: Sequence[Type[BasePermission]] = ( - IsAuthenticated, - HasDepositPermission, - ) - - -class APIBase(APIConfig, AuthenticatedAPIView, metaclass=ABCMeta): +class APIBase(APIConfig, APIView, metaclass=ABCMeta): """Base deposit request class sharing multiple common behaviors. """ _client: Optional[DepositClient] = None + def __init__(self): + super().__init__() + auth_provider = self.config.get("authentication_provider") + if auth_provider == "basic": + self.authentication_classes: Sequence[Type[BaseAuthentication]] = ( + BasicAuthentication, + ) + self.permission_classes: Sequence[Type[BasePermission]] = (IsAuthenticated,) + elif auth_provider == "keycloak": + self.authentication_classes: Sequence[Type[BaseAuthentication]] = ( + KeycloakBasicAuthentication, + ) + self.permission_classes: Sequence[Type[BasePermission]] = ( + IsAuthenticated, + HasDepositPermission, + ) + else: + raise ValueError( + "Configuration key 'authentication_provider' should be provided with" + f"either 'basic' or 'keycloak' value not {auth_provider!r}." + ) + def _read_headers(self, request: Request) -> ParsedRequestHeaders: """Read and unify the necessary headers from the request (those are not stored in the same location or not properly formatted). diff --git a/swh/deposit/api/private/__init__.py b/swh/deposit/api/private/__init__.py --- a/swh/deposit/api/private/__init__.py +++ b/swh/deposit/api/private/__init__.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 General Public License version 3, or any later version # See top-level LICENSE file for more information @@ -6,9 +6,9 @@ from typing import Any, Dict, List, Tuple from rest_framework.permissions import AllowAny +from rest_framework.views import APIView from swh.deposit import utils -from swh.deposit.api.common import AuthenticatedAPIView from ...config import METADATA_TYPE, APIConfig from ...models import Deposit, DepositRequest @@ -60,14 +60,16 @@ return (aggregated_metadata, raw_metadata) -class APIPrivateView(APIConfig, AuthenticatedAPIView): +class APIPrivateView(APIConfig, APIView): """Mixin intended as private api (so no authentication) based API view (for the private ones). """ - authentication_classes = () - permission_classes = (AllowAny,) + def __init__(self): + super().__init__() + self.authentication_classes = () + self.permission_classes = (AllowAny,) def checks(self, req, collection_name, deposit=None): """Override default checks implementation to allow empty collection. diff --git a/swh/deposit/api/service_document.py b/swh/deposit/api/service_document.py --- a/swh/deposit/api/service_document.py +++ b/swh/deposit/api/service_document.py @@ -12,12 +12,16 @@ APIBase, ) from swh.deposit.config import COL_IRI -from swh.deposit.models import DepositCollection +from swh.deposit.models import DepositClient, DepositCollection class ServiceDocumentAPI(APIBase): def get(self, request, *args, **kwargs): - client = request.user + if isinstance(request.user, DepositClient): + client = request.user + else: + client = DepositClient.objects.get(username=request.user) + collections = {} for col_id in client.collections: col = DepositCollection.objects.get(pk=col_id) diff --git a/swh/deposit/cli/admin.py b/swh/deposit/cli/admin.py --- a/swh/deposit/cli/admin.py +++ b/swh/deposit/cli/admin.py @@ -120,6 +120,8 @@ user.save() action_done = "created" + if password: + user.set_password(password) user.collections = [collection_.id] user.first_name = firstname user.last_name = lastname diff --git a/swh/deposit/settings/common.py b/swh/deposit/settings/common.py --- a/swh/deposit/settings/common.py +++ b/swh/deposit/settings/common.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 General Public License version 3, or any later version # See top-level LICENSE file for more information @@ -103,7 +103,6 @@ STATIC_URL = "/static/" REST_FRAMEWORK = { - "DEFAULT_AUTHENTICATION_CLASSES": ("swh.deposit.auth.KeycloakBasicAuthentication",), "EXCEPTION_HANDLER": "swh.deposit.exception.custom_exception_handler", } diff --git a/swh/deposit/settings/production.py b/swh/deposit/settings/production.py --- a/swh/deposit/settings/production.py +++ b/swh/deposit/settings/production.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 General Public License version 3, or any later version # See top-level LICENSE file for more information @@ -8,7 +8,7 @@ from swh.core import config from .common import * # noqa -from .common import ALLOWED_HOSTS +from .common import ALLOWED_HOSTS, CACHES ALLOWED_HOSTS += ["deposit.softwareheritage.org"] # Setup support for proxy headers @@ -41,7 +41,7 @@ if not conf: raise ValueError("Production: configuration %s does not exist." % (config_file,)) -for key in ("scheduler", "private"): +for key in ("scheduler", "private", "authentication_provider"): if not conf.get(key): raise ValueError( "Production: invalid configuration; missing %s config entry." % (key,) @@ -91,3 +91,20 @@ # https://docs.djangoproject.com/en/1.11/ref/settings/#std:setting-MEDIA_ROOT MEDIA_ROOT = private_conf.get("media_root") + +# Default authentication is http basic +authentication = conf["authentication_provider"] + +# With the following, we delegate the authentication mechanism to keycloak +if authentication == "keycloak": + # Optional cache server + server_cache = conf.get("cache_uri") + if server_cache: + CACHES.update( + { + "default": { + "BACKEND": "django.core.cache.backends.memcached.MemcachedCache", + "LOCATION": server_cache, + } + } + ) diff --git a/swh/deposit/settings/testing.py b/swh/deposit/settings/testing.py --- a/swh/deposit/settings/testing.py +++ b/swh/deposit/settings/testing.py @@ -40,3 +40,7 @@ FILE_UPLOAD_HANDLERS = [ "django.core.files.uploadhandler.MemoryFileUploadHandler", ] + +REST_FRAMEWORK = { + "EXCEPTION_HANDLER": "swh.deposit.exception.custom_exception_handler", +} diff --git a/swh/deposit/tests/api/test_basic_auth.py b/swh/deposit/tests/api/test_basic_auth.py new file mode 100644 --- /dev/null +++ b/swh/deposit/tests/api/test_basic_auth.py @@ -0,0 +1,32 @@ +# Copyright (C) 2017-2021 The Software Heritage developers +# See the AUTHORS file at the top-level directory of this distribution +# License: GNU General Public License version 3, or any later version +# See top-level LICENSE file for more information + +"""Module to check at least one basic authentication works. + +""" + +from django.urls import reverse_lazy as reverse +import pytest + +from swh.deposit.config import SD_IRI + +from .test_service_document import check_response + + +@pytest.fixture() +def deposit_config(common_deposit_config): + return { + **common_deposit_config, + "authentication_provider": "basic", + } + + +def test_service_document_basic(basic_authenticated_client): + """With authentication, service document list user's collection + + """ + url = reverse(SD_IRI) + response = basic_authenticated_client.get(url) + check_response(response, basic_authenticated_client.deposit_client.username) diff --git a/swh/deposit/tests/api/test_service_document.py b/swh/deposit/tests/api/test_service_document.py --- a/swh/deposit/tests/api/test_service_document.py +++ b/swh/deposit/tests/api/test_service_document.py @@ -48,7 +48,7 @@ def check_response(response, username): - assert response.status_code == status.HTTP_200_OK + assert response.status_code == status.HTTP_200_OK, f"Response: {response.content}" assert ( response.content.decode("utf-8") == """ diff --git a/swh/deposit/tests/cli/test_admin.py b/swh/deposit/tests/cli/test_admin.py --- a/swh/deposit/tests/cli/test_admin.py +++ b/swh/deposit/tests/cli/test_admin.py @@ -179,7 +179,9 @@ assert user.is_active is True second_password = user.password assert second_password is not None - assert second_password == first_password, "Password not changed (no longer used)" + # For the transition period, we can choose either basic or keycloak so we need to be + # able to still define a password (basic), so there it's updated. + assert second_password != first_password, "Password changed" assert user.domain == "domain" assert user.provider_url == "http://some-provider.org" assert user.email == "user@org.org" diff --git a/swh/deposit/tests/conftest.py b/swh/deposit/tests/conftest.py --- a/swh/deposit/tests/conftest.py +++ b/swh/deposit/tests/conftest.py @@ -58,7 +58,7 @@ TEST_USER = { "username": USERNAME, - "password": "", + "password": "pass", "email": EMAIL, "provider_url": "https://hal-test.archives-ouvertes.fr/", "domain": "archives-ouvertes.fr/", @@ -127,8 +127,8 @@ return requests_mock_datadir -@pytest.fixture() -def deposit_config(swh_scheduler_config, swh_storage_backend_config): +@pytest.fixture +def common_deposit_config(swh_scheduler_config, swh_storage_backend_config): return { "max_upload_size": 500, "extraction_dir": "/tmp/swh-deposit/test/extraction-dir", @@ -137,6 +137,14 @@ "storage": swh_storage_backend_config, "storage_metadata": swh_storage_backend_config, "swh_authority_url": "http://deposit.softwareheritage.example/", + } + + +@pytest.fixture() +def deposit_config(common_deposit_config): + return { + **common_deposit_config, + "authentication_provider": "keycloak", "keycloak": { "server_url": KEYCLOAK_SERVER_URL, "realm_name": KEYCLOAK_REALM_NAME, @@ -247,15 +255,23 @@ ) -> "DepositClient": """Create/Return the test_user "test" + For basic authentication, this will save a password. + This is not required for keycloak authentication scheme. + """ from swh.deposit.models import DepositClient user_data_d = deepcopy(user_data) user_data_d.pop("collection", None) + passwd = user_data_d.pop("password", None) user, _ = DepositClient.objects.get_or_create( # type: ignore username=user_data_d["username"], defaults={**user_data_d, "collections": [collection.id]}, ) + if passwd: + user.set_password(passwd) + user.save() + return user @@ -309,14 +325,16 @@ return anonymous_client -def _create_authenticated_client(client, user): +def _create_authenticated_client(client, user, password=None): """Return a client whose credentials will be proposed to the deposit server. This also patched the client instance to keep a reference on the associated deposit_user. """ - _token = "%s:%s" % (user.username, "irrelevant-in-test-context") + if not password: + password = "irrelevant-if-not-set" + _token = "%s:%s" % (user.username, password) token = base64.b64encode(_token.encode("utf-8")) authorization = "Basic %s" % token.decode("utf-8") client.credentials(HTTP_AUTHORIZATION=authorization) @@ -325,6 +343,13 @@ client.logout() +@pytest.fixture +def basic_authenticated_client(anonymous_client, deposit_user): + yield from _create_authenticated_client( + anonymous_client, deposit_user, password=TEST_USER["password"] + ) + + @pytest.fixture def authenticated_client(mock_keycloakopenidconnect_ok, anonymous_client, deposit_user): yield from _create_authenticated_client(anonymous_client, deposit_user) diff --git a/swh/deposit/tests/test_init.py b/swh/deposit/tests/test_init.py --- a/swh/deposit/tests/test_init.py +++ b/swh/deposit/tests/test_init.py @@ -1,10 +1,26 @@ -# Copyright (C) 2020 The Software Heritage developers +# Copyright (C) 2020-2021 The Software Heritage developers # See the AUTHORS file at the top-level directory of this distribution # License: GNU General Public License version 3, or any later version # See top-level LICENSE file for more information +import pytest + +from swh.deposit.api.common import APIBase + def test_version(): from swh.deposit import __version__ assert __version__ is not None + + +@pytest.fixture +def deposit_config(common_deposit_config): + assert "authentication_provider" not in common_deposit_config + return common_deposit_config + + +def test_api_base_misconfigured(): + """No authentication_provider key in configuration should raise""" + with pytest.raises(ValueError, match="authentication_provider"): + APIBase()