From 9aa7e68a3f835ec48826003e891daf3ffb55a6b9 Mon Sep 17 00:00:00 2001 From: Benjamin Dauvergne Date: Tue, 30 Nov 2021 16:40:02 +0100 Subject: [PATCH 3/3] provisionning: add synchronous provisionning to DRF authentication (#59135) --- hobo/provisionning/utils.py | 72 +++++++++++- hobo/rest_authentication.py | 22 ++-- tests_authentic/test_rest_authentication.py | 15 ++- tests_multitenant/conftest.py | 8 +- tests_multitenant/test_provisionning.py | 120 +++++++++++++++++++- tests_multitenant/test_settings.py | 1 + tests_multitenant/test_uwsgidecorators.py | 5 +- 7 files changed, 217 insertions(+), 26 deletions(-) diff --git a/hobo/provisionning/utils.py b/hobo/provisionning/utils.py index a5c6ce2..8fade64 100644 --- a/hobo/provisionning/utils.py +++ b/hobo/provisionning/utils.py @@ -16,13 +16,18 @@ import hashlib import logging +from urllib.parse import quote, urljoin +import requests +from django.conf import settings from django.contrib.auth import get_user_model from django.contrib.auth.models import Group from django.db import IntegrityError +from django.db.models import Q from django.db.transaction import atomic from mellon.models import Issuer +from hobo import signature from hobo.agent.common.models import Role from hobo.multitenant.utils import provision_user_groups @@ -51,6 +56,11 @@ def user_str(user): return s +def get_idp_service(): + idp_services = list((settings.KNOWN_SERVICES or {}).get('authentic', {}).values()) + return (idp_services or [None])[0] + + def get_issuer(entity_id): issuer, created = Issuer.objects.get_or_create(entity_id=entity_id) return issuer @@ -69,12 +79,7 @@ def provision_user(entity_id, o, tries=0): } User = get_user_model() - try: - user = User.objects.get( - saml_identifiers__name_id=o['uuid'], saml_identifiers__issuer__entity_id=entity_id - ) - except User.DoesNotExist: - user = User() + user = get_user_from_name_id(name_id=o['uuid'], entity_id=entity_id) or User() for key in attributes: if getattr(user, key) != attributes[key]: @@ -263,3 +268,58 @@ class NotificationProcessing: except TryAgain: continue break + + +def get_user_from_name_id(name_id, entity_id=None, raise_on_missing=False): + User = get_user_model() + try: + user = User.objects.get( + saml_identifiers__name_id=name_id, saml_identifiers__issuer__entity_id=entity_id + ) + except User.DoesNotExist: + try: + user = User.objects.get(Q(username=name_id[:30]) | Q(username=name_id[:150])) + except User.DoesNotExist: + user = None + if not user: + if raise_on_missing: + raise User.DoesNotExist + return user + + +class ProvisionningTemporaryError(RuntimeError): + pass + + +def get_or_create_user_from_name_id(name_id, raise_on_missing=False): + User = get_user_model() + + user = get_user_from_name_id(name_id=name_id) + if user: + return user + + idp_service = get_idp_service() + if not idp_service: + if raise_on_missing: + raise User.DoesNotExist('no idp service defined') + return None + + entity_id = idp_service['saml-idp-metadata-url'] + issuer = get_issuer(entity_id) + users_api_url = urljoin(idp_service['url'], 'api/users/%s/' % quote(name_id, safe='')) + try: + response = requests.get(signature.sign_url(users_api_url, idp_service['secret']), timeout=5) + response.raise_for_status() + except requests.HTTPError as e: + if e.response.status_code == 404: + if raise_on_missing: + raise User.DoesNotExist + return None + if raise_on_missing: + raise ProvisionningTemporaryError(str(e) or repr(e)) + return None + except requests.RequestException as e: + if raise_on_missing: + raise ProvisionningTemporaryError(str(e) or repr(e)) + return None + return provision_user(issuer.entity_id, o=response.json()) diff --git a/hobo/rest_authentication.py b/hobo/rest_authentication.py index b7476b1..cd28148 100644 --- a/hobo/rest_authentication.py +++ b/hobo/rest_authentication.py @@ -9,6 +9,7 @@ from django.utils.module_loading import import_string from rest_framework import authentication, exceptions, status from hobo import signature +from hobo.provisionning.utils import ProvisionningTemporaryError, get_or_create_user_from_name_id from hobo.requests_wrapper import Requests try: @@ -81,10 +82,15 @@ class APIClientUser: class PublikAuthenticationFailed(exceptions.APIException): status_code = status.HTTP_401_UNAUTHORIZED - default_code = 'invalid-signature' - def __init__(self, code): - self.detail = {'err': 1, 'err_desc': code} + def __init__(self, code, description=None): + self.detail = {'err': 1, 'err_class': code, 'err_desc': code} + if description: + self.detail['err_desc'] = description + + +class PublikAuthenticationTemporaryFailure(PublikAuthenticationFailed): + status_code = status.HTTP_500_INTERNAL_SERVER_ERROR class PublikAuthentication(authentication.BaseAuthentication): @@ -110,8 +116,10 @@ class PublikAuthentication(authentication.BaseAuthentication): elif UserSAMLIdentifier: try: - return UserSAMLIdentifier.objects.get(name_id=name_id).user - except UserSAMLIdentifier.DoesNotExist: + return get_or_create_user_from_name_id(name_id, raise_on_missing=True) + except ProvisionningTemporaryError as e: + raise PublikAuthenticationTemporaryFailure('idp-not-reachable', str(e)) + except User.DoesNotExist: raise PublikAuthenticationFailed('user-not-found') else: raise PublikAuthenticationFailed('no-usable-model') @@ -123,7 +131,6 @@ class PublikAuthentication(authentication.BaseAuthentication): pass if hasattr(settings, 'HOBO_ANONYMOUS_SERVICE_USER_CLASS'): klass = import_string(settings.HOBO_ANONYMOUS_SERVICE_USER_CLASS) - self.logger.info('anonymous signature validated') return klass() raise PublikAuthenticationFailed('no-user-for-orig') @@ -149,9 +156,8 @@ class PublikAuthentication(authentication.BaseAuthentication): ), 'signature.check_url should never return False with raise_on_error' except signature.SignatureError as e: self.logger.warning('publik rest-framework-authentication failed: %s', e) - raise PublikAuthenticationFailed(str(e)) + raise PublikAuthenticationFailed('invalid-signature', str(e)) user = self.resolve_user(request) - self.logger.info('user authenticated with signature %s', user) return (user, None) diff --git a/tests_authentic/test_rest_authentication.py b/tests_authentic/test_rest_authentication.py index 7a7ded6..af35d68 100644 --- a/tests_authentic/test_rest_authentication.py +++ b/tests_authentic/test_rest_authentication.py @@ -145,7 +145,8 @@ def test_response(rf, settings, tenant): response = view(request) assert response.status_code == 401 - assert response.data == {'err': 1, 'err_desc': 'user-not-found'} + assert response.data['err'] == 1 + assert response.data['err_desc'] == 'user-not-found' # Service authentication, wrong timestamp request = rf.get( @@ -154,10 +155,11 @@ def test_response(rf, settings, tenant): response = view(request) assert response.status_code == 401 - assert response.data == { - 'err': 1, - 'err_desc': "invalid timestamp, time data 'xxx' does not match format '%Y-%m-%dT%H:%M:%SZ'", - } + assert response.data['err'] == 1 + assert ( + response.data['err_desc'] + == "invalid timestamp, time data 'xxx' does not match format '%Y-%m-%dT%H:%M:%SZ'" + ) # Service authentication request = rf.get(signature.sign_url('/?orig=zzz', secret_key)) @@ -169,4 +171,5 @@ def test_response(rf, settings, tenant): del settings.HOBO_ANONYMOUS_SERVICE_USER_CLASS response = view(request) assert response.status_code == 401 - assert response.data == {'err': 1, 'err_desc': 'no-user-for-orig'} + assert response.data['err'] == 1 + assert response.data['err_desc'] == 'no-user-for-orig' diff --git a/tests_multitenant/conftest.py b/tests_multitenant/conftest.py index 53b5e8e..4a2550e 100644 --- a/tests_multitenant/conftest.py +++ b/tests_multitenant/conftest.py @@ -1,4 +1,5 @@ import pytest +from tenant_schemas.utils import tenant_context @pytest.fixture @@ -68,6 +69,7 @@ def make_tenant(tmp_path, transactional_db, settings, request): 'service-id': 'authentic', 'base_url': 'http://other.example.net', 'legacy_urls': [{'base_url': 'http://olda2.example.net'}], + 'saml-idp-metadata-url': 'https://other.example.net/idp/saml2/metadata', }, { 'slug': 'another', @@ -82,6 +84,7 @@ def make_tenant(tmp_path, transactional_db, settings, request): 'service-id': 'combo', 'template_name': '...portal-user...', 'base_url': 'http://portal-user.example.net', + 'secret_key': 'abcdefg', }, ], }, @@ -116,7 +119,10 @@ def tenants(make_tenant): @pytest.fixture def tenant(make_tenant): - return make_tenant('tenant.example.net') + tenant = make_tenant('tenant.example.net') + + with tenant_context(tenant): + yield tenant @pytest.fixture diff --git a/tests_multitenant/test_provisionning.py b/tests_multitenant/test_provisionning.py index dedd72f..6b76a2e 100644 --- a/tests_multitenant/test_provisionning.py +++ b/tests_multitenant/test_provisionning.py @@ -1,4 +1,20 @@ -from hobo.provisionning.utils import NotificationProcessing +import json + +import pytest +import requests +from httmock import HTTMock, urlmatch +from mellon.models import Issuer +from rest_framework import permissions +from rest_framework.response import Response +from rest_framework.views import APIView + +from hobo import rest_authentication, signature +from hobo.provisionning.utils import ( + NotificationProcessing, + ProvisionningTemporaryError, + get_or_create_user_from_name_id, + get_user_from_name_id, +) def test_truncate_role_name(): @@ -20,3 +36,105 @@ def test_truncate_role_name(): assert len(truncated) == max_length assert truncated not in seen seen.add(truncated) + + +@urlmatch() +def request_exception(url, request): + raise requests.ConnectionError + + +NAME_ID = '1234' * 8 + + +@urlmatch(path='/api/users/') +def user_payload(url, request): + return { + 'status_code': 200, + 'content': json.dumps( + { + 'uuid': NAME_ID, + 'first_name': 'John', + 'last_name': 'Doe', + 'email': 'john.doe@example.net', + 'is_superuser': False, + 'is_active': True, + } + ), + } + + +@urlmatch(path='/api/users/') +def error_404(url, request): + return { + 'status_code': 404, + } + + +def test_get_or_create_user_from_name_id(tenant, django_user_model): + Issuer.objects.create(entity_id='https://idp.examle.net/idp/saml2/metadata') + + assert get_user_from_name_id(NAME_ID) is None + with pytest.raises(django_user_model.DoesNotExist): + get_user_from_name_id(NAME_ID, raise_on_missing=True) + + with HTTMock(request_exception): + assert get_or_create_user_from_name_id(NAME_ID) is None + with pytest.raises(ProvisionningTemporaryError): + get_or_create_user_from_name_id(NAME_ID, raise_on_missing=True) + + with HTTMock(error_404): + assert get_or_create_user_from_name_id(NAME_ID) is None + with pytest.raises(django_user_model.DoesNotExist): + get_or_create_user_from_name_id(NAME_ID, raise_on_missing=True) + + with HTTMock(user_payload): + user = get_or_create_user_from_name_id(NAME_ID) + assert user is not None + assert user.first_name == 'John' + assert user.username == NAME_ID + assert user.saml_identifiers.get().name_id == NAME_ID + assert django_user_model.objects.count() == 1 + + +class DummyAPIView(APIView): + authentication_classes = (rest_authentication.PublikAuthentication,) + permission_classes = (permissions.IsAuthenticated,) + + def get(self, request, format=None): + return Response({'err': 0}) + + +def test_rest_authentication_provisionning_ok(tenant, settings, rf, django_user_model): + key = settings.KNOWN_SERVICES['combo']['another2']['secret'] + request = rf.get(signature.sign_url('/api/misc/?param1=2&NameID=%s&orig=portal-user.example.net', key)) + + view = DummyAPIView.as_view() + + with HTTMock(user_payload): + response = view(request) + assert response.status_code == 200 + assert response.data == {'err': 0} + user = django_user_model.objects.get() + assert user.first_name == 'John' + assert user.username == NAME_ID + assert user.saml_identifiers.get().name_id == NAME_ID + + +def test_rest_authentication_provisionning_nok(tenant, settings, rf): + key = settings.KNOWN_SERVICES['combo']['another2']['secret'] + request = rf.get(signature.sign_url('/api/misc/?param1=2&NameID=abcd&orig=portal-user.example.net', key)) + + view = DummyAPIView.as_view() + + with HTTMock(request_exception): + response = view(request) + assert response.status_code == 500 + assert response.data['err'] == 1 + assert response.data['err_class'] == 'idp-not-reachable' + assert 'ConnectionError' in response.data['err_desc'] + + with HTTMock(error_404): + response = view(request) + assert response.status_code == 401 + assert response.data['err'] == 1 + assert response.data['err_class'] == 'user-not-found' diff --git a/tests_multitenant/test_settings.py b/tests_multitenant/test_settings.py index 004ed18..437655f 100644 --- a/tests_multitenant/test_settings.py +++ b/tests_multitenant/test_settings.py @@ -205,6 +205,7 @@ def test_known_services(tenants, settings): 'saml-sp-metadata-url', 'provisionning-url', 'secondary', + 'saml-idp-metadata-url', } == authentic_other_keys assert ( settings.KNOWN_SERVICES['authentic']['other']['url'] == hobo_json['services'][2]['base_url'] diff --git a/tests_multitenant/test_uwsgidecorators.py b/tests_multitenant/test_uwsgidecorators.py index 67fb28c..ca1043a 100644 --- a/tests_multitenant/test_uwsgidecorators.py +++ b/tests_multitenant/test_uwsgidecorators.py @@ -44,14 +44,11 @@ def test_mocked_uwsgi(uwsgi): def test_mocked_uwsgi_tenant(uwsgi, tenant): - from tenant_schemas.utils import tenant_context - @hobo.multitenant.uwsgidecorators.spool def function(a, b): pass - with tenant_context(tenant): - function.spool(1, 2) + function.spool(1, 2) assert set(uwsgi.spool.call_args[1].keys()) == {'body', 'tenant', 'name'} assert pickle.loads(uwsgi.spool.call_args[1]['body']) == {'args': (1, 2), 'kwargs': {}} -- 2.37.2