Changeset View
Standalone View
swh/deposit/auth.py
# Copyright (C) 2017 The Software Heritage developers | # Copyright (C) 2017-2021 The Software Heritage developers | ||||
# See the AUTHORS file at the top-level directory of this distribution | # See the AUTHORS file at the top-level directory of this distribution | ||||
# License: GNU General Public License version 3, or any later version | # License: GNU General Public License version 3, or any later version | ||||
# See top-level LICENSE file for more information | # See top-level LICENSE file for more information | ||||
import logging | |||||
from typing import Optional | |||||
from django.core.cache import cache | |||||
from django.utils import timezone | |||||
from rest_framework import status | from rest_framework import status | ||||
from rest_framework.authentication import BasicAuthentication | |||||
from rest_framework.exceptions import AuthenticationFailed | |||||
from rest_framework.permissions import BasePermission | |||||
from sentry_sdk import capture_exception | |||||
from swh.auth.django.models import OIDCUser | |||||
from swh.auth.django.utils import oidc_user_from_profile | |||||
from swh.auth.keycloak import KeycloakOpenIDConnect | |||||
from swh.deposit.models import DepositClient | |||||
from .errors import UNAUTHORIZED, make_error_response | from .errors import UNAUTHORIZED, make_error_response | ||||
logger = logging.getLogger(__name__) | |||||
OIDC_DEPOSIT_CLIENT_ID = "swh-deposit" | |||||
DEPOSIT_PERMISSION = "swh.deposit.api" | |||||
def convert_response(request, content): | def convert_response(request, content): | ||||
"""Convert response from drf's basic authentication mechanism to a | """Convert response from drf's basic authentication mechanism to a | ||||
swh-deposit one. | swh-deposit one. | ||||
Args: | Args: | ||||
request (Request): Use to build the response | request (Request): Use to build the response | ||||
content (bytes): The drf's answer | content (bytes): The drf's answer | ||||
Show All 38 Lines | def __call__(self, request): | ||||
response = self.get_response(request) | response = self.get_response(request) | ||||
if response.status_code is status.HTTP_401_UNAUTHORIZED: | if response.status_code is status.HTTP_401_UNAUTHORIZED: | ||||
content_type = response._headers.get("content-type") | content_type = response._headers.get("content-type") | ||||
if content_type == ("Content-Type", "application/json"): | if content_type == ("Content-Type", "application/json"): | ||||
return convert_response(request, response.content) | return convert_response(request, response.content) | ||||
return response | return response | ||||
anlambert: As next step to plug Keycloak authentication in `swh-deposit` will be to decode access tokens… | |||||
Done Inline Actionsright, thanks. ardumont: right, thanks. | |||||
class HasDepositPermission(BasePermission): | |||||
"""Allows access to authenticated users with the DEPOSIT_PERMISSION. | |||||
""" | |||||
def has_permission(self, request, view): | |||||
assert isinstance(request.user, DepositClient) | |||||
return request.user.oidc_user.has_perm(DEPOSIT_PERMISSION) | |||||
class KeycloakBasicAuthentication(BasicAuthentication): | |||||
"""Keycloack authentication against username/password. | |||||
Deposit users will continue sending `Basic authentication` queries to the deposit | |||||
server. Transparently, the deposit server will stop authenticate itself the users. | |||||
It will delegate the authentication queries to the keycloak instance. | |||||
Technically, reuses :class:`rest_framework.BasicAuthentication` and overrides the | |||||
func:`authenticate_credentials` method to discuss with keycloak. | |||||
As an implementation detail, this also uses the django cache mechanism to avoid too | |||||
many authentication request to keycloak. | |||||
""" | |||||
_client: Optional[KeycloakOpenIDConnect] = None | |||||
@property | |||||
def client(self): | |||||
if self._client is None: | |||||
self._client = KeycloakOpenIDConnect.from_configfile( | |||||
client_id=OIDC_DEPOSIT_CLIENT_ID | |||||
) | |||||
return self._client | |||||
def get_user(self, user_id: str) -> Optional[OIDCUser]: | |||||
"""Retrieve user from cache if any. | |||||
""" | |||||
oidc_profile = cache.get(f"oidc_user_{user_id}") | |||||
if oidc_profile: | |||||
try: | |||||
Done Inline Actionsoffline_access scope is not required for the deposit use case, you can remove it. anlambert: `offline_access` scope is not required for the deposit use case, you can remove it. | |||||
Done Inline Actionshow could that have worked...? ardumont: how could that have worked...?
It's used below but it's not an oidc_profile which is returned… | |||||
return oidc_user_from_profile(self.client, oidc_profile) | |||||
except Exception as e: | |||||
capture_exception(e) | |||||
return None | |||||
def authenticate_credentials(self, user_id, password, request): | |||||
"""Authenticate the user_id/password against keycloak. | |||||
Raises: | |||||
AuthenticationFailed in case of authentication failure | |||||
Returns: | |||||
Tuple of deposit_client, None. | |||||
""" | |||||
oidc_user = self.get_user(user_id) | |||||
ttl: Optional[int] = None | |||||
if not oidc_user: | |||||
try: | |||||
oidc_profile = self.client.login(user_id, password) | |||||
except Exception as e: | |||||
Done Inline ActionsAgreed, authentication backend should only login a user. anlambert: Agreed, authentication backend should only login a user. | |||||
raise AuthenticationFailed(e) | |||||
oidc_user = oidc_user_from_profile(self.client, oidc_profile) | |||||
ttl = int( | |||||
Done Inline Actionsfrom swh.web.auth.utils import oidc_user_from_profile oidc_user = oidc_user_from_profile(oidc_profile) anlambert: ```
lang=python
from swh.web.auth.utils import oidc_user_from_profile
oidc_user =… | |||||
Done Inline Actionscorrected code oidc_user = oidc_user_from_profile(self.client(), oidc_profile) anlambert: corrected code
```
oidc_user = oidc_user_from_profile(self.client(), oidc_profile)
``` | |||||
oidc_user.refresh_expires_at.timestamp() - timezone.now().timestamp() | |||||
) | |||||
Done Inline ActionsYou should rather use: deposit_client, created = DepositClient.objects.get_or_create(username=user_id) because at this point, the user has been successfully authenticated with Keycloak so You should never have to create user manually through the CLI now this is delegated anlambert: You should rather use:
```lang=python
deposit_client, created = DepositClient.objects. | |||||
Done Inline Actionskeycloak has, for now, only the responsibility to log the deposit users. We still need other information on the deposit client though (provider_url, ardumont: keycloak has, for now, only the responsibility to log the deposit users.
We still need other… | |||||
# Making sure the associated deposit client is correctly configured in backend | |||||
try: | |||||
Done Inline Actionsfrom django.utils import timezone ttl = int(oidc_user.refresh_expires_at.timestamp() - timezone.now().timestamp()) anlambert: ```lang=python
from django.utils import timezone
ttl = int(oidc_user.refresh_expires_at. | |||||
deposit_client = DepositClient.objects.get(username=user_id) | |||||
except DepositClient.DoesNotExist: | |||||
raise AuthenticationFailed(f"Unknown user {user_id}") | |||||
if not deposit_client.is_active: | |||||
Not Done Inline Actionsnitpick, you could drop the exceptions. prefix by adapting the imports anlambert: nitpick, you could drop the `exceptions.` prefix by adapting the imports | |||||
raise AuthenticationFailed(f"Deactivated user {user_id}") | |||||
deposit_client.oidc_user = oidc_user | |||||
if ttl: | |||||
# cache the oidc_profile user while it's valid | |||||
cache.set( | |||||
f"oidc_user_{user_id}", oidc_profile, timeout=max(0, ttl), | |||||
) | |||||
return (deposit_client, None) | |||||
Done Inline Actionss/funn/:func:/ anlambert: s/funn/:func:/ |
As next step to plug Keycloak authentication in swh-deposit will be to decode access tokens returned by keycloak in order to get user permissions, you will have to use the python-keycloak utility module.
All authentication flows are supported by the library so I guess you could start using it in that diff and simplify the KeycloakOpenIDConnectDirectGrant class implementation instead of using raw calls to requests library.