diff --git a/requirements-swh-server.txt b/requirements-swh-server.txt --- a/requirements-swh-server.txt +++ b/requirements-swh-server.txt @@ -2,4 +2,4 @@ swh.loader.core >= 0.0.71 swh.scheduler >= 0.7.0 swh.model >= 0.3.8 -swh.auth[django] >= 0.3.6 +swh.auth[django] >= 0.3.8 diff --git a/swh/deposit/auth.py b/swh/deposit/auth.py --- a/swh/deposit/auth.py +++ b/swh/deposit/auth.py @@ -3,7 +3,6 @@ # License: GNU General Public License version 3, or any later version # See top-level LICENSE file for more information -import json import logging from typing import Optional @@ -17,7 +16,11 @@ from swh.auth.django.models import OIDCUser from swh.auth.django.utils import oidc_user_from_profile -from swh.auth.keycloak import KeycloakError, KeycloakOpenIDConnect +from swh.auth.keycloak import ( + KeycloakError, + KeycloakOpenIDConnect, + keycloak_error_message, +) from swh.deposit.models import DepositClient from .errors import UNAUTHORIZED, make_error_response @@ -148,8 +151,7 @@ oidc_profile = self.client.login(user_id, password) except KeycloakError as e: logger.debug("KeycloakError: e: %s", e) - msg_dict = json.loads(e.error_message.decode()) - error_msg = f"{msg_dict['error']}: {msg_dict['error_description']}" + error_msg = keycloak_error_message(e) raise AuthenticationFailed(error_msg) oidc_user = oidc_user_from_profile(self.client, oidc_profile) diff --git a/swh/deposit/tests/api/test_keycloak_auth.py b/swh/deposit/tests/api/test_keycloak_auth.py new file mode 100644 --- /dev/null +++ b/swh/deposit/tests/api/test_keycloak_auth.py @@ -0,0 +1,35 @@ +# Copyright (C) 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 json + +from django.urls import reverse_lazy as reverse +import pytest + +from swh.auth.keycloak import KeycloakError +from swh.deposit.config import SD_IRI +from swh.deposit.tests.conftest import mock_keycloakopenidconnect + + +@pytest.fixture +def mock_keycloakopenidconnect_ko(mocker, keycloak_mock_auth_failure): + error = { + "error": "unknown_error", # does not help much but that can happen + } + error_message = json.dumps(error).encode() + keycloak_mock_auth_failure.login.side_effect = KeycloakError( + error_message=error_message, response_code=401 + ) + return mock_keycloakopenidconnect(mocker, keycloak_mock_auth_failure) + + +def test_keycloak_failure_service_document(unauthorized_client): + """With authentication failure without detail, exception is returned correctly + + """ + url = reverse(SD_IRI) + response = unauthorized_client.get(url) + assert response.status_code == 401 + assert b"unknown_error" in response.content 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 @@ -317,14 +317,6 @@ return mock_keycloakopenidconnect(mocker, keycloak_mock_auth_failure) -@pytest.fixture -def unauthorized_client(anonymous_client, mock_keycloakopenidconnect_ko): - """Create an unauthorized client (will see their authentication fail) - - """ - return anonymous_client - - def _create_authenticated_client(client, user, password=None): """Return a client whose credentials will be proposed to the deposit server. @@ -355,6 +347,14 @@ yield from _create_authenticated_client(anonymous_client, deposit_user) +@pytest.fixture +def unauthorized_client(mock_keycloakopenidconnect_ko, anonymous_client, deposit_user): + """Create an unauthorized client (will see their authentication fail) + + """ + yield from _create_authenticated_client(anonymous_client, deposit_user) + + @pytest.fixture def insufficient_perm_client( mocker, keycloak_mock_auth_success, anonymous_client, deposit_user