diff --git a/swh/auth/django/models.py b/swh/auth/django/models.py --- a/swh/auth/django/models.py +++ b/swh/auth/django/models.py @@ -1,4 +1,4 @@ -# Copyright (C) 2020-2021 The Software Heritage developers +# Copyright (C) 2020-2022 The Software Heritage developers # See the AUTHORS file at the top-level directory of this distribution # License: GNU Affero General Public License version 3, or any later version # See top-level LICENSE file for more information @@ -6,7 +6,8 @@ from datetime import datetime from typing import Any, Dict, Optional, Set -from django.contrib.auth.models import User +from django.contrib.auth.models import Group, User +from django.db.models import Q class OIDCUser(User): @@ -36,6 +37,9 @@ # User permissions permissions: Set[str] + # User groups + group_names: Set[str] + class Meta: # TODO: To redefine in subclass of this class # Forced to empty otherwise, django complains about it @@ -87,6 +91,17 @@ return any(perm.startswith(app_label) for perm in self.permissions) + @property + def groups(self): + """ + Override django.contrib.auth.models.PermissionsMixin.groups + to get groups from OIDC. + """ + search_query = Q() + for group_name in self.group_names: + search_query = search_query | Q(name=group_name) + return Group.objects.filter(search_query) + @property def oidc_profile(self) -> Dict[str, Any]: """ diff --git a/swh/auth/django/utils.py b/swh/auth/django/utils.py --- a/swh/auth/django/utils.py +++ b/swh/auth/django/utils.py @@ -1,4 +1,4 @@ -# Copyright (C) 2020-2021 The Software Heritage developers +# Copyright (C) 2020-2022 The Software Heritage developers # See the AUTHORS file at the top-level directory of this distribution # License: GNU Affero General Public License version 3, or any later version # See top-level LICENSE file for more information @@ -7,6 +7,7 @@ from typing import Any, Dict, Optional from django.conf import settings +from django.contrib.auth.models import Group from django.http import HttpRequest, QueryDict from django.urls import reverse as django_reverse @@ -43,9 +44,17 @@ email=decoded_token.get("email", ""), ) - # set is_staff user property based on groups + # process keycloak groups + group_names = set() if "groups" in decoded_token: + # set is_staff user property based on group membership user.is_staff = "/staff" in decoded_token["groups"] + for group_name in decoded_token["groups"]: + # remove leading slash added by keycloak to group name + django_group_name = group_name.lstrip("/") + # ensure a corresponding django group exist + Group.objects.get_or_create(name=django_group_name) + group_names.add(django_group_name) realm_access = decoded_token.get("realm_access", {}) permissions = realm_access.get("roles", []) @@ -58,6 +67,8 @@ # set user permissions and filter out default keycloak realm roles user.permissions = set(permissions) - {"offline_access", "uma_authorization"} + # set user groups + user.group_names = group_names # add user sub to custom User proxy model user.sub = decoded_token["sub"] diff --git a/swh/auth/tests/django/test_backends.py b/swh/auth/tests/django/test_backends.py --- a/swh/auth/tests/django/test_backends.py +++ b/swh/auth/tests/django/test_backends.py @@ -1,4 +1,4 @@ -# Copyright (C) 2020-2021 The Software Heritage developers +# Copyright (C) 2020-2022 The Software Heritage developers # See the AUTHORS file at the top-level directory of this distribution # License: GNU Affero General Public License version 3, or any later version # See top-level LICENSE file for more information @@ -18,6 +18,8 @@ from swh.auth.django.utils import oidc_profile_cache_key, reverse from swh.auth.keycloak import ExpiredSignatureError, KeycloakError +pytestmark = pytest.mark.django_db + def _authenticate_user(request_factory): request = request_factory.get(reverse("root")) @@ -40,19 +42,22 @@ assert user.last_name == decoded_token["family_name"] assert user.email == decoded_token["email"] assert user.is_staff == ("/staff" in decoded_token["groups"]) + assert {group.name for group in user.groups.all()} == { + group_name.lstrip("/") for group_name in keycloak_oidc.user_groups + } assert user.sub == decoded_token["sub"] resource_access = decoded_token.get("resource_access", {}) resource_access_client = resource_access.get(keycloak_oidc.client_id, {}) assert user.permissions == set(resource_access_client.get("roles", [])) + assert all(user.has_perm(perm) for perm in resource_access_client.get("roles", [])) -@pytest.mark.django_db def test_oidc_code_pkce_auth_backend_success(keycloak_oidc, request_factory): """ Checks successful login based on OpenID Connect with PKCE extension Django authentication backend (login from Web UI). """ - keycloak_oidc.user_groups = ["/staff"] + keycloak_oidc.user_groups = ["/staff", "/other_group"] oidc_profile = keycloak_oidc.login() user = _authenticate_user(request_factory) @@ -80,7 +85,6 @@ assert get_backends()[backend_idx].get_user(user.id) == user -@pytest.mark.django_db def test_oidc_code_pkce_auth_backend_failure(keycloak_oidc, request_factory): """ Checks failed login based on OpenID Connect with PKCE extension Django @@ -93,7 +97,6 @@ assert user is None -@pytest.mark.django_db def test_oidc_code_pkce_auth_backend_refresh_token_success( keycloak_oidc, request_factory ): @@ -118,7 +121,6 @@ assert user is not None -@pytest.mark.django_db def test_oidc_code_pkce_auth_backend_refresh_token_failure( keycloak_oidc, request_factory ): @@ -164,7 +166,6 @@ assert cache.get(cache_key) is None -@pytest.mark.django_db def test_oidc_code_pkce_auth_backend_permissions(keycloak_oidc, request_factory): """ Checks that a permission defined with OpenID Connect is correctly mapped @@ -183,7 +184,6 @@ assert not user.has_module_perms("foo") -@pytest.mark.django_db def test_drf_oidc_bearer_token_auth_backend_success(keycloak_oidc, api_request_factory): """ Checks successful login based on OpenID Connect bearer token Django REST @@ -206,7 +206,6 @@ assert hasattr(user, "access_token") and user.access_token is None -@pytest.mark.django_db def test_drf_oidc_bearer_token_auth_backend_failure(keycloak_oidc, api_request_factory): """ Checks failed login based on OpenID Connect bearer token Django REST @@ -261,7 +260,6 @@ drf_auth_backend.authenticate(request) -@pytest.mark.django_db def test_drf_oidc_bearer_token_auth_backend_permissions( keycloak_oidc, api_request_factory ): diff --git a/swh/auth/tests/django/test_utils.py b/swh/auth/tests/django/test_utils.py --- a/swh/auth/tests/django/test_utils.py +++ b/swh/auth/tests/django/test_utils.py @@ -1,4 +1,4 @@ -# Copyright (C) 2021 The Software Heritage developers +# Copyright (C) 2021-2022 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 @@ -22,8 +22,10 @@ SERVER_URL, ) +pytestmark = pytest.mark.django_db -def _check_user(user, is_staff=False, permissions=set()): + +def _check_user(user, is_staff=False, permissions=set(), groups=set()): assert user.id > 0 assert user.username == DECODED_TOKEN["preferred_username"] assert user.password == "" @@ -31,7 +33,9 @@ assert user.last_name == DECODED_TOKEN["family_name"] assert user.email == DECODED_TOKEN["email"] assert user.is_staff == is_staff + assert {group.name for group in user.groups.all()} == groups assert user.permissions == permissions + assert all(user.has_perm(perm) for perm in permissions) assert user.sub == DECODED_TOKEN["sub"] date_now = datetime.now() @@ -71,7 +75,12 @@ user = oidc_user_from_decoded_token(decoded_token, client_id=CLIENT_ID) - _check_user(user, is_staff=True, permissions={"swh.ambassador", "read-api"}) + _check_user( + user, + is_staff=True, + permissions={"swh.ambassador", "read-api"}, + groups={group_name.lstrip("/") for group_name in decoded_token["groups"]}, + ) @pytest.mark.parametrize(