From ca402507bd82768981bbf15497a1b6d250854b5b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Micha=C5=82=20Pasternak?= Date: Sat, 24 Oct 2026 12:11:53 +0200 Subject: [PATCH 1/4] =?UTF-8?q?fix(security):=20wymagany=20konkretny=20ten?= =?UTF-8?q?ant=20i=20ostro=C5=BCniejsze=20wi=C4=85zanie=20kont=20Microsoft?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - logowanie Office 365 wymaga MICROSOFT_AUTH_TENANT_ID wskazującego konkretny katalog Entra ID (GUID lub domena); puste/common/organizations/consumers -> ImproperlyConfigured przy starcie, - nowy backend bpp.auth.microsoft.BppMicrosoftAuthenticationBackend (zamiast backendu z pakietu) sprawdza claimy iss/tid tokenu względem skonfigurowanego katalogu, - nowe konto Microsoft nie jest automatycznie wiązane po e-mailu z istniejącym kontem staff/superuser lub z hasłem lokalnym, chyba że MICROSOFT_AUTH_LINK_BY_EMAIL=True (domyślnie False); konta już powiązane logują się bez zmian, - dawna ścieżka backendu nadal rozpoznawana jako zewnętrzna (polityka haseł), - dokumentacja w docs/administrator/zaawansowane.md, testy, newsfragment. Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_01UJ23Rb1c5UtxHTfAke9ENQ --- docs/administrator/zaawansowane.md | 42 +++ src/bpp/auth/__init__.py | 0 src/bpp/auth/microsoft.py | 61 ++++ src/bpp/auth/microsoft_conf.py | 92 ++++++ .../sec-microsoft-tenant.bugfix.rst | 1 + src/bpp/tests/test_auth_microsoft.py | 271 ++++++++++++++++++ src/django_bpp/external_auth.py | 6 +- src/django_bpp/settings/base.py | 13 +- src/django_bpp/views.py | 5 + 9 files changed, 488 insertions(+), 3 deletions(-) create mode 100644 src/bpp/auth/__init__.py create mode 100644 src/bpp/auth/microsoft.py create mode 100644 src/bpp/auth/microsoft_conf.py create mode 100644 src/bpp/newsfragments/sec-microsoft-tenant.bugfix.rst create mode 100644 src/bpp/tests/test_auth_microsoft.py diff --git a/docs/administrator/zaawansowane.md b/docs/administrator/zaawansowane.md index 0839c023f..7a37cfdf4 100644 --- a/docs/administrator/zaawansowane.md +++ b/docs/administrator/zaawansowane.md @@ -193,3 +193,45 @@ inline. Jeżeli serwer WWW (np. nginx) sam ustawia nagłówek CSP, wyłącz nagłówek z aplikacji: `DJANGO_BPP_CSP_ENABLED=false`. + +## Logowanie przez Microsoft (Office 365 / Entra ID) + +Logowanie kontem Microsoft wymaga opcjonalnego pakietu `django_microsoft_auth` +(`uv sync --extra office365`) i włącza się po ustawieniu w pliku `.env` +zmiennych `MICROSOFT_AUTH_CLIENT_ID` oraz `MICROSOFT_AUTH_CLIENT_SECRET`. + +| Zmienna | Opis | +|---------|------| +| `MICROSOFT_AUTH_CLIENT_ID` | Identyfikator aplikacji zarejestrowanej w Entra ID. | +| `MICROSOFT_AUTH_CLIENT_SECRET` | Sekret tej aplikacji. | +| `MICROSOFT_AUTH_TENANT_ID` | **Wymagany.** Identyfikator katalogu (GUID) albo jego domena, np. `uczelnia.onmicrosoft.com`. | +| `MICROSOFT_AUTH_LINK_BY_EMAIL` | Domyślnie `False`. Patrz niżej. | +| `MICROSOFT_AUTH_EXTRA_SCOPES` | Dodatkowe zakresy OAuth (opcjonalnie). | + +### Wymagany konkretny katalog (tenant) + +Jeżeli ustawiono `MICROSOFT_AUTH_CLIENT_ID`, a `MICROSOFT_AUTH_TENANT_ID` jest +pusty albo ma wartość `common`, `organizations` lub `consumers`, serwer BPP +**nie wystartuje** (błąd `ImproperlyConfigured`). BPP przyjmuje wyłącznie +tokeny wystawione przez skonfigurowany katalog — sprawdzane są claimy `iss` +i `tid` tokenu. Identyfikator katalogu znajdziesz w portalu Azure: +*Microsoft Entra ID → Przegląd → Identyfikator dzierżawy*. + +### Wiązanie kont Microsoft z istniejącymi kontami BPP + +Przy pierwszym logowaniu kontem Microsoft BPP szuka użytkownika o tym samym +adresie e-mail. Domyślnie (`MICROSOFT_AUTH_LINK_BY_EMAIL=False`) **nie** +powiąże automatycznie konta Microsoft z istniejącym użytkownikiem, który: + +- ma uprawnienia administracyjne (`is_staff` lub `is_superuser`), albo +- ma ustawione lokalne hasło do BPP. + +Takie logowanie zostanie odrzucone. Administrator może powiązać konto ręcznie +w module administracyjnym (*Microsoft accounts*), albo — jeżeli adresy e-mail +w katalogu są w pełni zaufane — ustawić `MICROSOFT_AUTH_LINK_BY_EMAIL=True`. +Zwykłe konta bez hasła lokalnego oraz nowi użytkownicy są wiązani/zakładani +automatycznie, jak dotychczas. + +Konta Microsoft **już powiązane** z użytkownikiem BPP logują się bez zmian. +Po aktualizacji użytkownicy zalogowani przez Microsoft muszą jednorazowo +zalogować się ponownie (zmieniła się nazwa backendu uwierzytelniania). diff --git a/src/bpp/auth/__init__.py b/src/bpp/auth/__init__.py new file mode 100644 index 000000000..e69de29bb diff --git a/src/bpp/auth/microsoft.py b/src/bpp/auth/microsoft.py new file mode 100644 index 000000000..48fd120e9 --- /dev/null +++ b/src/bpp/auth/microsoft.py @@ -0,0 +1,61 @@ +"""Backend logowania Microsoft (Office 365) z dodatkowymi zabezpieczeniami. + +Rozszerza ``microsoft_auth.backends.MicrosoftAuthenticationBackend``: + +* przyjmuje wyłącznie tokeny wystawione przez skonfigurowany katalog Entra ID + (claimy ``iss``/``tid``), +* nie wiąże automatycznie nowego konta Microsoft z istniejącym kontem + z uprawnieniami lub z lokalnym hasłem, chyba że + ``MICROSOFT_AUTH_LINK_BY_EMAIL = True``. + +Konta Microsoft już powiązane z użytkownikiem BPP logują się bez zmian. +""" + +import logging + +from django.conf import settings +from django.contrib.auth import get_user_model +from microsoft_auth.backends import MicrosoftAuthenticationBackend + +from bpp.auth.microsoft_conf import claims_match_tenant, may_link_existing_users + +logger = logging.getLogger(__name__) + + +class BppMicrosoftAuthenticationBackend(MicrosoftAuthenticationBackend): + def _expected_issuer(self): + openid_config = self.microsoft.openid_config or {} + return openid_config.get("issuer") + + def _authenticate_microsoft_user(self): + claims = self.microsoft.get_claims() + if claims is None: + return None + + tenant_id = getattr(settings, "MICROSOFT_AUTH_TENANT_ID", None) + if not claims_match_tenant(claims, tenant_id, self._expected_issuer()): + logger.warning( + "Odrzucono logowanie Microsoft: token spoza skonfigurowanego " + "katalogu (tid=%r)", + claims.get("tid"), + ) + return None + + return self._get_user_from_microsoft(claims) + + def _verify_microsoft_user(self, microsoft_user, data): + if microsoft_user.user is None: + email = data.get("email") + if email: + matching = get_user_model().objects.filter(email=email) + link_by_email = getattr(settings, "MICROSOFT_AUTH_LINK_BY_EMAIL", False) + if not may_link_existing_users(matching, link_by_email): + logger.warning( + "Odmowa automatycznego powiązania konta Microsoft %r " + "z istniejącym użytkownikiem BPP (MICROSOFT_AUTH_LINK_" + "BY_EMAIL wyłączone). Powiąż konto ręcznie w adminie.", + microsoft_user.microsoft_id, + ) + return None + + return super()._verify_microsoft_user(microsoft_user, data) diff --git a/src/bpp/auth/microsoft_conf.py b/src/bpp/auth/microsoft_conf.py new file mode 100644 index 000000000..1ca9eba50 --- /dev/null +++ b/src/bpp/auth/microsoft_conf.py @@ -0,0 +1,92 @@ +"""Reguły bezpieczeństwa logowania przez Microsoft (Office 365). + +Moduł celowo NIE importuje ``microsoft_auth`` ani modeli Django — jest czytany +z ``settings/base.py`` zanim Django w pełni wstanie, a funkcje decyzyjne są +testowalne bez zainstalowanej aplikacji ``microsoft_auth``. +""" + +import re + +from django.core.exceptions import ImproperlyConfigured + +# Wartości „tenant" Entra ID, które NIE wskazują jednej organizacji: +# akceptują konta z dowolnego katalogu (lub konta prywatne Microsoft). +MULTI_TENANT_VALUES = frozenset({"common", "organizations", "consumers"}) + +_GUID_RE = re.compile(r"^[0-9a-f]{8}-[0-9a-f]{4}-[0-9a-f]{4}-[0-9a-f]{4}-[0-9a-f]{12}$") + + +def is_guid(value): + return bool(value) and bool(_GUID_RE.match(str(value).strip().lower())) + + +def validate_microsoft_tenant_id(tenant_id): + """Zwróć znormalizowany ``tenant_id`` albo rzuć ``ImproperlyConfigured``. + + Logowanie Microsoft wymaga wskazania konkretnej organizacji (GUID katalogu + albo jego domena, np. ``uczelnia.onmicrosoft.com``). + """ + value = (tenant_id or "").strip() + if not value or value.lower() in MULTI_TENANT_VALUES: + raise ImproperlyConfigured( + "Zdefiniowano MICROSOFT_AUTH_CLIENT_ID, ale MICROSOFT_AUTH_TENANT_ID " + f"jest pusty lub ogólny ({value!r}). Ustaw MICROSOFT_AUTH_TENANT_ID " + "na identyfikator (GUID) lub domenę katalogu Entra ID uczelni — " + "wartości 'common', 'organizations' i 'consumers' są niedozwolone." + ) + return value + + +def claims_match_tenant(claims, tenant_id, expected_issuer): + """Czy claimy ``id_token`` pochodzą ze skonfigurowanego katalogu? + + ``expected_issuer`` to ``issuer`` z dokumentu OpenID discovery pobranego + dla skonfigurowanego tenanta (dla konkretnego katalogu zawiera jego GUID). + """ + if not claims or not tenant_id or not expected_issuer: + return False + if "{" in expected_issuer: + # Szablon issuera ({tenantid}) — discovery dla endpointu wielotenantowego. + return False + + tid = str(claims.get("tid") or "").strip().lower() + iss = str(claims.get("iss") or "").strip() + if not tid or not iss: + return False + + if iss != expected_issuer: + return False + if tid not in iss.lower(): + return False + if is_guid(tenant_id) and tid != tenant_id.strip().lower(): + return False + return True + + +def user_is_protected_from_email_linking(user): + """Czy istniejącego użytkownika NIE wolno automatycznie powiązać po e-mailu. + + Chronione są konta z uprawnieniami administracyjnymi oraz konta posiadające + lokalne hasło. + """ + if user.is_staff or user.is_superuser: + return True + if user.password and user.has_usable_password(): + return True + return False + + +def may_link_existing_users(users, link_by_email): + """Czy wolno powiązać nowe konto Microsoft z (jedynym) użytkownikiem z listy. + + Pusta lista — nie ma kogo wiązać (zostanie założone nowe konto), więc + ``True``. Więcej niż jeden pasujący użytkownik — odmowa. + """ + users = list(users) + if not users: + return True + if len(users) > 1: + return False + if link_by_email: + return True + return not user_is_protected_from_email_linking(users[0]) diff --git a/src/bpp/newsfragments/sec-microsoft-tenant.bugfix.rst b/src/bpp/newsfragments/sec-microsoft-tenant.bugfix.rst new file mode 100644 index 000000000..43fd6bfa1 --- /dev/null +++ b/src/bpp/newsfragments/sec-microsoft-tenant.bugfix.rst @@ -0,0 +1 @@ +Wzmocniono zabezpieczenia logowania przez Microsoft (Office 365): wymagane jest wskazanie konkretnego katalogu Entra ID (``MICROSOFT_AUTH_TENANT_ID``), weryfikowane są claimy ``iss``/``tid`` tokenu, a nowe konto Microsoft nie jest automatycznie wiązane po adresie e-mail z istniejącym kontem administracyjnym lub posiadającym hasło lokalne (chyba że ``MICROSOFT_AUTH_LINK_BY_EMAIL=True``). diff --git a/src/bpp/tests/test_auth_microsoft.py b/src/bpp/tests/test_auth_microsoft.py new file mode 100644 index 000000000..bc480ab63 --- /dev/null +++ b/src/bpp/tests/test_auth_microsoft.py @@ -0,0 +1,271 @@ +"""Testy logowania Microsoft (Office 365): tenant i wiązanie kont po e-mailu.""" + +from types import SimpleNamespace +from unittest import mock + +import pytest +from django.contrib.auth import get_user_model +from django.core.exceptions import ImproperlyConfigured +from django.test import override_settings +from model_bakery import baker + +from bpp.auth.microsoft_conf import ( + claims_match_tenant, + may_link_existing_users, + validate_microsoft_tenant_id, +) +from django_bpp.external_auth import EXTERNAL_AUTH_BACKENDS, MICROSOFT_BACKEND + +TENANT = "11111111-2222-3333-4444-555555555555" +OTHER_TENANT = "99999999-8888-7777-6666-555555555555" +ISSUER = f"https://login.microsoftonline.com/{TENANT}/v2.0" +OTHER_ISSUER = f"https://login.microsoftonline.com/{OTHER_TENANT}/v2.0" + + +def _claims(tid=TENANT, iss=ISSUER, **extra): + data = { + "sub": "ms-sub-1", + "tid": tid, + "iss": iss, + "email": "jan@example.com", + "preferred_username": "jan@example.com", + "name": "Jan Kowalski", + } + data.update(extra) + return data + + +# --- konfiguracja --------------------------------------------------------- + + +@pytest.mark.parametrize( + "value", [None, "", " ", "common", "Common", "organizations", "consumers"] +) +def test_validate_tenant_odrzuca_puste_i_wielotenantowe(value): + with pytest.raises(ImproperlyConfigured): + validate_microsoft_tenant_id(value) + + +@pytest.mark.parametrize("value", [TENANT, "uczelnia.onmicrosoft.com"]) +def test_validate_tenant_akceptuje_konkretny_katalog(value): + assert validate_microsoft_tenant_id(f" {value} ") == value + + +def test_backend_bpp_rozpoznawany_jako_zewnetrzny(): + assert MICROSOFT_BACKEND in EXTERNAL_AUTH_BACKENDS + + +# --- claimy tid / iss ----------------------------------------------------- + + +def test_claims_zgodny_tenant_guid(): + assert claims_match_tenant(_claims(), TENANT, ISSUER) + + +def test_claims_tenant_jako_domena_weryfikowany_po_issuerze(): + assert claims_match_tenant(_claims(), "uczelnia.onmicrosoft.com", ISSUER) + + +def test_claims_inny_tid_odrzucony(): + assert not claims_match_tenant( + _claims(tid=OTHER_TENANT, iss=OTHER_ISSUER), TENANT, ISSUER + ) + + +def test_claims_inny_tid_odrzucony_dla_tenanta_domenowego(): + assert not claims_match_tenant( + _claims(tid=OTHER_TENANT, iss=OTHER_ISSUER), + "uczelnia.onmicrosoft.com", + ISSUER, + ) + + +def test_claims_tid_niespojny_z_iss_odrzucony(): + assert not claims_match_tenant(_claims(tid=OTHER_TENANT), TENANT, ISSUER) + + +@pytest.mark.parametrize("brak", ["tid", "iss"]) +def test_claims_bez_tid_lub_iss_odrzucone(brak): + claims = _claims() + del claims[brak] + assert not claims_match_tenant(claims, TENANT, ISSUER) + + +def test_claims_issuer_szablonowy_odrzucony(): + template = "https://login.microsoftonline.com/{tenantid}/v2.0" + assert not claims_match_tenant(_claims(), TENANT, template) + + +def test_claims_brak_expected_issuer_odrzucony(): + assert not claims_match_tenant(_claims(), TENANT, None) + + +# --- decyzja o wiązaniu po e-mailu ----------------------------------------- + + +@pytest.mark.django_db +def test_link_brak_istniejacego_usera_dozwolony(): + assert may_link_existing_users([], link_by_email=False) + + +@pytest.mark.django_db +@pytest.mark.parametrize( + "attrs", + [{"is_superuser": True}, {"is_staff": True}], +) +def test_link_konto_uprzywilejowane_zablokowane(attrs): + user = baker.make(get_user_model(), password="", **attrs) + assert not may_link_existing_users([user], link_by_email=False) + + +@pytest.mark.django_db +def test_link_konto_z_haslem_lokalnym_zablokowane(): + user = baker.make(get_user_model()) + user.set_password("haslo-lokalne-123") + user.save() + assert not may_link_existing_users([user], link_by_email=False) + + +@pytest.mark.django_db +def test_link_zwykle_konto_bez_hasla_dozwolone(): + bez_hasla = baker.make(get_user_model(), password="") + nieuzywalne = baker.make(get_user_model()) + nieuzywalne.set_unusable_password() + nieuzywalne.save() + assert may_link_existing_users([bez_hasla], link_by_email=False) + assert may_link_existing_users([nieuzywalne], link_by_email=False) + + +@pytest.mark.django_db +def test_link_wlaczony_flaga_pozwala_na_konto_uprzywilejowane(): + user = baker.make(get_user_model(), is_superuser=True, password="") + assert may_link_existing_users([user], link_by_email=True) + + +@pytest.mark.django_db +def test_link_wielu_pasujacych_userow_zablokowane(): + users = baker.make(get_user_model(), password="", _quantity=2) + assert not may_link_existing_users(users, link_by_email=True) + + +# --- backend (wymaga zainstalowanego microsoft_auth) ------------------------ + + +def _backend(claims, issuer=ISSUER): + from bpp.auth.microsoft import BppMicrosoftAuthenticationBackend + + backend = BppMicrosoftAuthenticationBackend() + backend.microsoft = SimpleNamespace( + get_claims=lambda: claims, openid_config={"issuer": issuer} + ) + return backend + + +@pytest.mark.uruchom_tylko_z_microsoft_auth +@pytest.mark.django_db +@override_settings(MICROSOFT_AUTH_TENANT_ID=TENANT) +def test_backend_token_z_innego_tenanta_brak_logowania(): + from microsoft_auth.models import MicrosoftAccount + + user = baker.make(get_user_model(), email="jan@example.com", password="") + baker.make(MicrosoftAccount, microsoft_id="ms-sub-1", user=user) + + backend = _backend(_claims(tid=OTHER_TENANT, iss=OTHER_ISSUER)) + assert backend._authenticate_microsoft_user() is None + + +@pytest.mark.uruchom_tylko_z_microsoft_auth +@pytest.mark.django_db +@override_settings(MICROSOFT_AUTH_TENANT_ID=TENANT) +def test_backend_token_z_innego_tenanta_nie_zaklada_konta(): + from microsoft_auth.models import MicrosoftAccount + + backend = _backend(_claims(tid=OTHER_TENANT, iss=OTHER_ISSUER, sub="obcy")) + assert backend._authenticate_microsoft_user() is None + assert not MicrosoftAccount.objects.filter(microsoft_id="obcy").exists() + + +@pytest.mark.uruchom_tylko_z_microsoft_auth +@pytest.mark.django_db +@override_settings(MICROSOFT_AUTH_TENANT_ID=TENANT, MICROSOFT_AUTH_LINK_BY_EMAIL=False) +def test_backend_nowe_konto_ms_nie_wiaze_sie_z_superuserem(): + from microsoft_auth.models import MicrosoftAccount + + admin = baker.make( + get_user_model(), email="jan@example.com", is_superuser=True, password="" + ) + + backend = _backend(_claims()) + assert backend._authenticate_microsoft_user() is None + + admin.refresh_from_db() + assert not MicrosoftAccount.objects.filter(user=admin).exists() + assert get_user_model().objects.filter(email="jan@example.com").count() == 1 + + +@pytest.mark.uruchom_tylko_z_microsoft_auth +@pytest.mark.django_db +@override_settings(MICROSOFT_AUTH_TENANT_ID=TENANT, MICROSOFT_AUTH_LINK_BY_EMAIL=True) +def test_backend_flaga_link_by_email_wiaze_z_superuserem(): + from microsoft_auth.models import MicrosoftAccount + + admin = baker.make( + get_user_model(), email="jan@example.com", is_superuser=True, password="" + ) + + backend = _backend(_claims()) + assert backend._authenticate_microsoft_user() == admin + assert MicrosoftAccount.objects.get(microsoft_id="ms-sub-1").user == admin + + +@pytest.mark.uruchom_tylko_z_microsoft_auth +@pytest.mark.django_db +@override_settings(MICROSOFT_AUTH_TENANT_ID=TENANT, MICROSOFT_AUTH_LINK_BY_EMAIL=False) +def test_backend_istniejace_powiazane_konto_loguje_sie(): + from microsoft_auth.models import MicrosoftAccount + + admin = baker.make(get_user_model(), email="jan@example.com", is_superuser=True) + admin.set_password("haslo-lokalne-123") + admin.save() + baker.make(MicrosoftAccount, microsoft_id="ms-sub-1", user=admin) + + backend = _backend(_claims()) + assert backend._authenticate_microsoft_user() == admin + + +@pytest.mark.uruchom_tylko_z_microsoft_auth +@pytest.mark.django_db +@override_settings(MICROSOFT_AUTH_TENANT_ID=TENANT) +def test_backend_nowy_uzytkownik_jest_zakladany(): + backend = _backend(_claims(email="nowy@example.com", sub="ms-sub-nowy")) + user = backend._authenticate_microsoft_user() + assert user is not None + assert user.email == "nowy@example.com" + + +@pytest.mark.uruchom_tylko_z_microsoft_auth +@pytest.mark.django_db +@override_settings(MICROSOFT_AUTH_TENANT_ID=TENANT) +def test_backend_authenticate_pelny_przeplyw(rf): + from microsoft_auth.models import MicrosoftAccount + + from bpp.auth.microsoft import BppMicrosoftAuthenticationBackend + + user = baker.make(get_user_model(), email="jan@example.com", password="") + baker.make(MicrosoftAccount, microsoft_id="ms-sub-1", user=user) + + def _client(claims): + client = mock.MagicMock() + client.fetch_token.return_value = {"access_token": "x", "scope": ["openid"]} + client.valid_scopes.return_value = True + client.get_claims.return_value = claims + client.openid_config = {"issuer": ISSUER} + return client + + request = rf.get("/microsoft/auth-callback/") + with mock.patch("microsoft_auth.backends.MicrosoftClient") as klasa: + klasa.return_value = _client(_claims()) + assert BppMicrosoftAuthenticationBackend().authenticate(request, "kod") == user + + klasa.return_value = _client(_claims(tid=OTHER_TENANT, iss=OTHER_ISSUER)) + assert BppMicrosoftAuthenticationBackend().authenticate(request, "kod") is None diff --git a/src/django_bpp/external_auth.py b/src/django_bpp/external_auth.py index e2ae7e055..6a4cb08a2 100644 --- a/src/django_bpp/external_auth.py +++ b/src/django_bpp/external_auth.py @@ -5,12 +5,16 @@ nie ma password_policies w INSTALLED_APPS). """ -MICROSOFT_BACKEND = "microsoft_auth.backends.MicrosoftAuthenticationBackend" +MICROSOFT_BACKEND = "bpp.auth.microsoft.BppMicrosoftAuthenticationBackend" +# Ścieżka backendu z pakietu django-microsoft-auth, używana przed wprowadzeniem +# BppMicrosoftAuthenticationBackend — nadal rozpoznawana jako zewnętrzna. +MICROSOFT_LEGACY_BACKEND = "microsoft_auth.backends.MicrosoftAuthenticationBackend" ORCID_BACKEND = "orcid_integration.backends.OrcidAuthenticationBackend" OIDC_BACKEND = "oidc_integration.backends.BppOIDCBackend" EXTERNAL_AUTH_BACKENDS = { MICROSOFT_BACKEND, + MICROSOFT_LEGACY_BACKEND, ORCID_BACKEND, OIDC_BACKEND, } diff --git a/src/django_bpp/settings/base.py b/src/django_bpp/settings/base.py index 87027dfb6..4175697c7 100644 --- a/src/django_bpp/settings/base.py +++ b/src/django_bpp/settings/base.py @@ -13,6 +13,7 @@ from celery.schedules import crontab from django.core.exceptions import ImproperlyConfigured +from bpp.auth.microsoft_conf import validate_microsoft_tenant_id from bpp.util import slugify_function from django_bpp.channels_prefix import get_channels_prefix from django_bpp.rollbar_filters import zbuduj_exception_level_filters @@ -62,6 +63,7 @@ def int_or_none(v): MICROSOFT_AUTH_CLIENT_SECRET=(str, None), MICROSOFT_AUTH_TENANT_ID=(str, None), MICROSOFT_AUTH_EXTRA_SCOPES=(str, ""), + MICROSOFT_AUTH_LINK_BY_EMAIL=(bool, False), # # Email # @@ -1375,7 +1377,14 @@ def can_login_as(request, target_user): ) ) from e MICROSOFT_AUTH_CLIENT_SECRET = env("MICROSOFT_AUTH_CLIENT_SECRET") - MICROSOFT_AUTH_TENANT_ID = env("MICROSOFT_AUTH_TENANT_ID") + # Fail-closed: wymagany konkretny katalog Entra ID (GUID lub domena); + # "common"/"organizations"/"consumers" albo brak wartości -> błąd konfiguracji. + MICROSOFT_AUTH_TENANT_ID = validate_microsoft_tenant_id( + env("MICROSOFT_AUTH_TENANT_ID") + ) + # Automatyczne wiązanie nowego konta Microsoft z istniejącym kontem BPP + # z uprawnieniami / hasłem lokalnym po samym e-mailu — domyślnie wyłączone. + MICROSOFT_AUTH_LINK_BY_EMAIL = env("MICROSOFT_AUTH_LINK_BY_EMAIL") MICROSOFT_AUTH_LOGIN_TYPE = "ma" MICROSOFT_AUTH_EXTRA_SCOPES = env("MICROSOFT_AUTH_EXTRA_SCOPES") @@ -1392,7 +1401,7 @@ def can_login_as(request, target_user): # to avoid unnecessary network traffic on every page load AUTHENTICATION_BACKENDS = [ - "microsoft_auth.backends.MicrosoftAuthenticationBackend", + "bpp.auth.microsoft.BppMicrosoftAuthenticationBackend", "django.contrib.auth.backends.ModelBackend", ] diff --git a/src/django_bpp/views.py b/src/django_bpp/views.py index 9190bbc75..c9b6fc5ff 100644 --- a/src/django_bpp/views.py +++ b/src/django_bpp/views.py @@ -15,6 +15,7 @@ from django_bpp.external_auth import ( EXTERNAL_AUTH_BACKENDS, MICROSOFT_BACKEND, + MICROSOFT_LEGACY_BACKEND, OIDC_BACKEND, ORCID_BACKEND, ) @@ -194,6 +195,10 @@ def _get_post_logout_redirect_uri(self, request): "Microsoft", "https://myaccount.microsoft.com/", ), + MICROSOFT_LEGACY_BACKEND: ( + "Microsoft", + "https://myaccount.microsoft.com/", + ), ORCID_BACKEND: ( "ORCID", "https://orcid.org/my-orcid", From 42a6faafe94c2fb04968e866f27e77911868b857 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Micha=C5=82=20Pasternak?= Date: Sat, 24 Oct 2026 12:28:40 +0200 Subject: [PATCH 2/4] =?UTF-8?q?fix(security):=20w=C5=82asna=20logika=20wi?= =?UTF-8?q?=C4=85zania=20kont=20Microsoft,=20od=C5=9Bwie=C5=BCanie=20disco?= =?UTF-8?q?very?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - nowe konto Microsoft bez (lub z pustym) adresem e-mail nie jest ani wiązane, ani zakładane, - wyszukanie istniejącego użytkownika bez rozróżniania wielkości liter; więcej niż jedno trafienie -> odmowa; backend nie deleguje już wiązania/zakładania do pakietu, - gdy zbuforowany dokument OpenID discovery nie odpowiada skonfigurowanemu katalogowi, cache jest jednorazowo odświeżany, - testy i doprecyzowana dokumentacja. Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_01UJ23Rb1c5UtxHTfAke9ENQ --- docs/administrator/zaawansowane.md | 4 +- src/bpp/auth/microsoft.py | 125 ++++++++++++++++++++++----- src/bpp/tests/test_auth_microsoft.py | 73 ++++++++++++++++ 3 files changed, 181 insertions(+), 21 deletions(-) diff --git a/docs/administrator/zaawansowane.md b/docs/administrator/zaawansowane.md index 7a37cfdf4..724b2fb4c 100644 --- a/docs/administrator/zaawansowane.md +++ b/docs/administrator/zaawansowane.md @@ -220,7 +220,9 @@ i `tid` tokenu. Identyfikator katalogu znajdziesz w portalu Azure: ### Wiązanie kont Microsoft z istniejącymi kontami BPP Przy pierwszym logowaniu kontem Microsoft BPP szuka użytkownika o tym samym -adresie e-mail. Domyślnie (`MICROSOFT_AUTH_LINK_BY_EMAIL=False`) **nie** +adresie e-mail (bez rozróżniania wielkości liter). Token bez adresu e-mail +oraz adres pasujący do więcej niż jednego użytkownika powodują odrzucenie +logowania. Domyślnie (`MICROSOFT_AUTH_LINK_BY_EMAIL=False`) **nie** powiąże automatycznie konta Microsoft z istniejącym użytkownikiem, który: - ma uprawnienia administracyjne (`is_staff` lub `is_superuser`), albo diff --git a/src/bpp/auth/microsoft.py b/src/bpp/auth/microsoft.py index 48fd120e9..3f7d45fa0 100644 --- a/src/bpp/auth/microsoft.py +++ b/src/bpp/auth/microsoft.py @@ -15,17 +15,38 @@ from django.conf import settings from django.contrib.auth import get_user_model +from django.core.cache import cache from microsoft_auth.backends import MicrosoftAuthenticationBackend +from microsoft_auth.conf import CACHE_KEY_JWKS, CACHE_KEY_OPENID -from bpp.auth.microsoft_conf import claims_match_tenant, may_link_existing_users +from bpp.auth.microsoft_conf import ( + claims_match_tenant, + is_guid, + may_link_existing_users, +) logger = logging.getLogger(__name__) class BppMicrosoftAuthenticationBackend(MicrosoftAuthenticationBackend): - def _expected_issuer(self): - openid_config = self.microsoft.openid_config or {} - return openid_config.get("issuer") + def _expected_issuer(self, tenant_id): + issuer = (self.microsoft.openid_config or {}).get("issuer") or "" + if self._issuer_is_stale(issuer, tenant_id): + # Dokument discovery jest cache'owany pod kluczem niezależnym od + # tenanta — po zmianie MICROSOFT_AUTH_TENANT_ID w cache może leżeć + # wersja dla innego endpointu. Odśwież jednorazowo. + cache.delete(CACHE_KEY_OPENID) + cache.delete(CACHE_KEY_JWKS) + issuer = (self.microsoft.openid_config or {}).get("issuer") or "" + return issuer + + @staticmethod + def _issuer_is_stale(issuer, tenant_id): + if not issuer or "{" in issuer: + return True + if is_guid(tenant_id) and tenant_id.strip().lower() not in issuer.lower(): + return True + return False def _authenticate_microsoft_user(self): claims = self.microsoft.get_claims() @@ -33,7 +54,8 @@ def _authenticate_microsoft_user(self): return None tenant_id = getattr(settings, "MICROSOFT_AUTH_TENANT_ID", None) - if not claims_match_tenant(claims, tenant_id, self._expected_issuer()): + expected_issuer = self._expected_issuer(tenant_id) + if not claims_match_tenant(claims, tenant_id, expected_issuer): logger.warning( "Odrzucono logowanie Microsoft: token spoza skonfigurowanego " "katalogu (tid=%r)", @@ -43,19 +65,82 @@ def _authenticate_microsoft_user(self): return self._get_user_from_microsoft(claims) + @staticmethod + def _split_name(fullname): + first_name, last_name = "", "" + if fullname: + try: + # Format „Nazwisko, Imię" + last_name, first_name = fullname.split(", ") + except ValueError: + try: + first_name, last_name = fullname.split(" ", 1) + except ValueError: + first_name = fullname + return first_name, last_name + def _verify_microsoft_user(self, microsoft_user, data): - if microsoft_user.user is None: - email = data.get("email") - if email: - matching = get_user_model().objects.filter(email=email) - link_by_email = getattr(settings, "MICROSOFT_AUTH_LINK_BY_EMAIL", False) - if not may_link_existing_users(matching, link_by_email): - logger.warning( - "Odmowa automatycznego powiązania konta Microsoft %r " - "z istniejącym użytkownikiem BPP (MICROSOFT_AUTH_LINK_" - "BY_EMAIL wyłączone). Powiąż konto ręcznie w adminie.", - microsoft_user.microsoft_id, - ) - return None - - return super()._verify_microsoft_user(microsoft_user, data) + """Zwróć użytkownika BPP dla konta Microsoft (fail-closed). + + Konto już powiązane — bez zmian. Nowe konto Microsoft: wymagany + niepusty e-mail; wyszukanie bez rozróżniania wielkości liter; więcej + niż jedno trafienie, konto z uprawnieniami lub z hasłem lokalnym + (bez ``MICROSOFT_AUTH_LINK_BY_EMAIL``) — odmowa. + """ + if microsoft_user.user is not None: + return microsoft_user.user + + email = (data.get("email") or "").strip() + if not email: + logger.warning( + "Odrzucono logowanie Microsoft %r: token bez adresu e-mail.", + microsoft_user.microsoft_id, + ) + return None + + User = get_user_model() + matching = list(User.objects.filter(email__iexact=email)[:2]) + link_by_email = getattr(settings, "MICROSOFT_AUTH_LINK_BY_EMAIL", False) + if not may_link_existing_users(matching, link_by_email): + logger.warning( + "Odmowa automatycznego powiązania konta Microsoft %r " + "z istniejącym użytkownikiem BPP. Powiąż konto ręcznie " + "w adminie lub ustaw MICROSOFT_AUTH_LINK_BY_EMAIL.", + microsoft_user.microsoft_id, + ) + return None + + first_name, last_name = self._split_name(data.get("name")) + + if matching: + user = matching[0] + if user.first_name == "" and user.last_name == "": + user.first_name = first_name + user.last_name = last_name + user.save() + else: + username = (data.get("preferred_username") or email)[:150] + user = User( + username=username, + first_name=first_name, + last_name=last_name, + email=email, + ) + user.save() + + existing_account = self._get_existing_microsoft_account(user) + if existing_account is not None: + if self.config.MICROSOFT_AUTH_AUTO_REPLACE_ACCOUNTS: + existing_account.user = None + existing_account.save() + else: + logger.warning( + "Użytkownik %r ma już powiązane konto Microsoft, a " + "MICROSOFT_AUTH_AUTO_REPLACE_ACCOUNTS jest wyłączone.", + user.pk, + ) + return None + + microsoft_user.user = user + microsoft_user.save() + return user diff --git a/src/bpp/tests/test_auth_microsoft.py b/src/bpp/tests/test_auth_microsoft.py index bc480ab63..e15d4a7e4 100644 --- a/src/bpp/tests/test_auth_microsoft.py +++ b/src/bpp/tests/test_auth_microsoft.py @@ -269,3 +269,76 @@ def _client(claims): klasa.return_value = _client(_claims(tid=OTHER_TENANT, iss=OTHER_ISSUER)) assert BppMicrosoftAuthenticationBackend().authenticate(request, "kod") is None + + +@pytest.mark.uruchom_tylko_z_microsoft_auth +@pytest.mark.django_db +@override_settings(MICROSOFT_AUTH_TENANT_ID=TENANT) +@pytest.mark.parametrize("email", ["", " ", None]) +def test_backend_pusty_email_nie_wiaze_z_kontem_bez_emaila(email): + from microsoft_auth.models import MicrosoftAccount + + admin = baker.make(get_user_model(), email="", is_superuser=True, password="") + baker.make(get_user_model(), email="", password="") + liczba = get_user_model().objects.count() + + backend = _backend(_claims(email=email)) + assert backend._authenticate_microsoft_user() is None + assert not MicrosoftAccount.objects.filter(user=admin).exists() + assert get_user_model().objects.count() == liczba + + +@pytest.mark.uruchom_tylko_z_microsoft_auth +@pytest.mark.django_db +@override_settings(MICROSOFT_AUTH_TENANT_ID=TENANT, MICROSOFT_AUTH_LINK_BY_EMAIL=False) +def test_backend_email_innej_wielkosci_liter_nie_wiaze_z_superuserem(): + from microsoft_auth.models import MicrosoftAccount + + admin = baker.make( + get_user_model(), email="jan@example.com", is_superuser=True, password="" + ) + backend = _backend(_claims(email="Jan@Example.COM")) + assert backend._authenticate_microsoft_user() is None + assert not MicrosoftAccount.objects.filter(user=admin).exists() + assert get_user_model().objects.filter(email__iexact="jan@example.com").count() == 1 + + +@pytest.mark.uruchom_tylko_z_microsoft_auth +@pytest.mark.django_db +@override_settings(MICROSOFT_AUTH_TENANT_ID=TENANT) +def test_backend_zwykle_konto_wiazane_bez_wzgledu_na_wielkosc_liter(): + user = baker.make(get_user_model(), email="jan@example.com", password="") + backend = _backend(_claims(email="JAN@example.com")) + assert backend._authenticate_microsoft_user() == user + + +@pytest.mark.uruchom_tylko_z_microsoft_auth +@pytest.mark.django_db +@override_settings(MICROSOFT_AUTH_TENANT_ID=TENANT) +def test_backend_odswieza_nieaktualny_dokument_discovery(): + from microsoft_auth.models import MicrosoftAccount + + from bpp.auth.microsoft import BppMicrosoftAuthenticationBackend + + user = baker.make(get_user_model(), email="jan@example.com", password="") + baker.make(MicrosoftAccount, microsoft_id="ms-sub-1", user=user) + + class _Client: + def __init__(self): + self.wywolania = 0 + + def get_claims(self): + return _claims() + + @property + def openid_config(self): + self.wywolania += 1 + if self.wywolania == 1: + return {"issuer": "https://login.microsoftonline.com/{tenantid}/v2.0"} + return {"issuer": ISSUER} + + backend = BppMicrosoftAuthenticationBackend() + backend.microsoft = _Client() + with mock.patch("bpp.auth.microsoft.cache") as cache: + assert backend._authenticate_microsoft_user() == user + assert cache.delete.called From 4ce6391d6020eef1eae0b09151a374180ea4ee42 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Micha=C5=82=20Pasternak?= Date: Sat, 24 Oct 2026 22:30:30 +0200 Subject: [PATCH 3/4] =?UTF-8?q?fix(security):=20brak=20konkretnego=20tenan?= =?UTF-8?q?ta=20Microsoft=20jako=20ostrze=C5=BCenie,=20nie=20b=C5=82=C4=85?= =?UTF-8?q?d=20startu?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Pusty lub ogólny MICROSOFT_AUTH_TENANT_ID (common/organizations/consumers) nie blokuje już startu. Zamiast ImproperlyConfigured: system check bpp.W201 i komunikat dla superusera po zalogowaniu z podpowiedzią (GUID lub domena katalogu). Backend w trybie wielokatalogowym pomija weryfikację iss/tid, ale zachowuje ostrożne wiązanie kont. Przy konkretnym tenancie - pełna weryfikacja jak dotąd. Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_01UJ23Rb1c5UtxHTfAke9ENQ --- docs/administrator/zaawansowane.md | 27 ++- src/bpp/apps.py | 6 +- src/bpp/auth/microsoft.py | 16 +- src/bpp/auth/microsoft_conf.py | 84 ++++++-- src/bpp/checks.py | 21 ++ .../sec-microsoft-tenant.bugfix.rst | 2 +- src/bpp/tests/test_auth_microsoft.py | 190 +++++++++++++++++- src/bpp/tests/test_konta_bez_uczelni.py | 6 +- src/django_bpp/settings/base.py | 9 +- 9 files changed, 323 insertions(+), 38 deletions(-) diff --git a/docs/administrator/zaawansowane.md b/docs/administrator/zaawansowane.md index 724b2fb4c..9a3589efc 100644 --- a/docs/administrator/zaawansowane.md +++ b/docs/administrator/zaawansowane.md @@ -204,19 +204,32 @@ zmiennych `MICROSOFT_AUTH_CLIENT_ID` oraz `MICROSOFT_AUTH_CLIENT_SECRET`. |---------|------| | `MICROSOFT_AUTH_CLIENT_ID` | Identyfikator aplikacji zarejestrowanej w Entra ID. | | `MICROSOFT_AUTH_CLIENT_SECRET` | Sekret tej aplikacji. | -| `MICROSOFT_AUTH_TENANT_ID` | **Wymagany.** Identyfikator katalogu (GUID) albo jego domena, np. `uczelnia.onmicrosoft.com`. | +| `MICROSOFT_AUTH_TENANT_ID` | **Zalecany.** Identyfikator katalogu (GUID) albo jego domena, np. `uczelnia.onmicrosoft.com`. | | `MICROSOFT_AUTH_LINK_BY_EMAIL` | Domyślnie `False`. Patrz niżej. | | `MICROSOFT_AUTH_EXTRA_SCOPES` | Dodatkowe zakresy OAuth (opcjonalnie). | -### Wymagany konkretny katalog (tenant) +### Zalecany konkretny katalog (tenant) -Jeżeli ustawiono `MICROSOFT_AUTH_CLIENT_ID`, a `MICROSOFT_AUTH_TENANT_ID` jest -pusty albo ma wartość `common`, `organizations` lub `consumers`, serwer BPP -**nie wystartuje** (błąd `ImproperlyConfigured`). BPP przyjmuje wyłącznie -tokeny wystawione przez skonfigurowany katalog — sprawdzane są claimy `iss` -i `tid` tokenu. Identyfikator katalogu znajdziesz w portalu Azure: +Zalecamy ustawienie `MICROSOFT_AUTH_TENANT_ID` na katalog uczelni. Wtedy BPP +przyjmuje wyłącznie tokeny wystawione przez ten katalog i sprawdza claimy +`iss` i `tid` tokenu. Identyfikator katalogu znajdziesz w portalu Azure: *Microsoft Entra ID → Przegląd → Identyfikator dzierżawy*. +Jeżeli ustawiono `MICROSOFT_AUTH_CLIENT_ID`, a `MICROSOFT_AUTH_TENANT_ID` jest +pusty albo ma wartość `common`, `organizations` lub `consumers`, BPP +**uruchomi się normalnie**, ale logowanie działa w trybie wielokatalogowym: + +- przez logowanie Microsoft przejdą konta z dowolnego katalogu Entra ID + (dla `common` i `consumers` także prywatne konta Microsoft), o ile + rejestracja aplikacji w Entra ID na to pozwala; BPP nie sprawdza, z którego + katalogu pochodzi token, +- zasady wiązania kont opisane niżej obowiązują bez zmian, +- `manage.py check` zgłasza ostrzeżenie `bpp.W201`, a superuser po każdym + zalogowaniu widzi komunikat z tą samą podpowiedzią. + +Aby usunąć ostrzeżenie, ustaw `MICROSOFT_AUTH_TENANT_ID` (GUID albo domena +katalogu) i zrestartuj serwer aplikacji. + ### Wiązanie kont Microsoft z istniejącymi kontami BPP Przy pierwszym logowaniu kontem Microsoft BPP szuka użytkownika o tym samym diff --git a/src/bpp/apps.py b/src/bpp/apps.py index 4ab170470..9f489abe7 100644 --- a/src/bpp/apps.py +++ b/src/bpp/apps.py @@ -11,12 +11,17 @@ def ready(self): from django.db.models.signals import post_migrate, post_save from . import checks # noqa: F401 + from .auth.microsoft_conf import ostrzez_superusera_o_tenancie_microsoft from .konta_bez_uczelni import ostrzez_superusera_po_zalogowaniu user_logged_in.connect( ostrzez_superusera_po_zalogowaniu, dispatch_uid="bpp.ostrzez_superusera_po_zalogowaniu", ) + user_logged_in.connect( + ostrzez_superusera_o_tenancie_microsoft, + dispatch_uid="bpp.ostrzez_superusera_o_tenancie_microsoft", + ) # Only register post_migrate signal when dbtemplates is available # (auth_server uses minimal INSTALLED_APPS without dbtemplates) @@ -34,7 +39,6 @@ def ready(self): dispatch_uid="bpp.invalidate_uczelnia_cache_on_article_change", ) - self._podepnij_inwalidacje_cache_publicznego() # Ensure BppUserAdmin takes precedence over microsoft_auth's UserAdmin diff --git a/src/bpp/auth/microsoft.py b/src/bpp/auth/microsoft.py index 3f7d45fa0..07c791284 100644 --- a/src/bpp/auth/microsoft.py +++ b/src/bpp/auth/microsoft.py @@ -2,8 +2,13 @@ Rozszerza ``microsoft_auth.backends.MicrosoftAuthenticationBackend``: -* przyjmuje wyłącznie tokeny wystawione przez skonfigurowany katalog Entra ID - (claimy ``iss``/``tid``), +* przy ustawionym konkretnym katalogu Entra ID (GUID lub domena) przyjmuje + wyłącznie tokeny wystawione przez ten katalog (claimy ``iss``/``tid``); + bez konkretnego katalogu (``common``/``organizations``/``consumers`` lub + pusty) działa w trybie wielokatalogowym, jak biblioteka — tę konfigurację + zgłasza system check ``bpp.W201``, +* nie wiąże nowego konta Microsoft z kontem BPP, gdy token nie ma e-maila + albo e-mail pasuje do więcej niż jednego konta, * nie wiąże automatycznie nowego konta Microsoft z istniejącym kontem z uprawnieniami lub z lokalnym hasłem, chyba że ``MICROSOFT_AUTH_LINK_BY_EMAIL = True``. @@ -22,6 +27,7 @@ from bpp.auth.microsoft_conf import ( claims_match_tenant, is_guid, + is_multi_tenant, may_link_existing_users, ) @@ -54,6 +60,12 @@ def _authenticate_microsoft_user(self): return None tenant_id = getattr(settings, "MICROSOFT_AUTH_TENANT_ID", None) + if is_multi_tenant(tenant_id): + # Tryb wielokatalogowy: nie ma katalogu, względem którego można + # sprawdzić iss/tid (podpis i audience sprawdza biblioteka). + # Ostrożne wiązanie kont w _verify_microsoft_user obowiązuje dalej. + return self._get_user_from_microsoft(claims) + expected_issuer = self._expected_issuer(tenant_id) if not claims_match_tenant(claims, tenant_id, expected_issuer): logger.warning( diff --git a/src/bpp/auth/microsoft_conf.py b/src/bpp/auth/microsoft_conf.py index 1ca9eba50..c52ad8eeb 100644 --- a/src/bpp/auth/microsoft_conf.py +++ b/src/bpp/auth/microsoft_conf.py @@ -5,9 +5,10 @@ testowalne bez zainstalowanej aplikacji ``microsoft_auth``. """ +import logging import re -from django.core.exceptions import ImproperlyConfigured +logger = logging.getLogger(__name__) # Wartości „tenant" Entra ID, które NIE wskazują jednej organizacji: # akceptują konta z dowolnego katalogu (lub konta prywatne Microsoft). @@ -20,21 +21,80 @@ def is_guid(value): return bool(value) and bool(_GUID_RE.match(str(value).strip().lower())) -def validate_microsoft_tenant_id(tenant_id): - """Zwróć znormalizowany ``tenant_id`` albo rzuć ``ImproperlyConfigured``. +#: ID system checka ostrzegającego o braku konkretnego katalogu (tenanta). +W_TENANT_ID = "bpp.W201" - Logowanie Microsoft wymaga wskazania konkretnej organizacji (GUID katalogu - albo jego domena, np. ``uczelnia.onmicrosoft.com``). +#: Wartość używana przez ``django_microsoft_auth``, gdy tenant nie jest podany. +DEFAULT_TENANT_ID = "common" + + +def normalize_microsoft_tenant_id(tenant_id): + """Zwróć znormalizowany ``tenant_id``; pusty -> ``"common"``. + + Brak konkretnego katalogu nie blokuje startu — logowanie działa wtedy + w trybie wielokatalogowym, a operator dostaje ostrzeżenie + (system check ``bpp.W201`` i komunikat dla superusera po zalogowaniu). """ value = (tenant_id or "").strip() - if not value or value.lower() in MULTI_TENANT_VALUES: - raise ImproperlyConfigured( - "Zdefiniowano MICROSOFT_AUTH_CLIENT_ID, ale MICROSOFT_AUTH_TENANT_ID " - f"jest pusty lub ogólny ({value!r}). Ustaw MICROSOFT_AUTH_TENANT_ID " - "na identyfikator (GUID) lub domenę katalogu Entra ID uczelni — " - "wartości 'common', 'organizations' i 'consumers' są niedozwolone." + return value or DEFAULT_TENANT_ID + + +def is_multi_tenant(tenant_id): + """Czy ``tenant_id`` NIE wskazuje jednego, konkretnego katalogu?""" + value = (tenant_id or "").strip().lower() + return not value or value in MULTI_TENANT_VALUES + + +def microsoft_tenant_warning(client_id, tenant_id): + """Treść ostrzeżenia o braku konkretnego tenanta albo ``None``. + + Ostrzeżenie dotyczy wyłącznie instalacji z włączonym logowaniem Microsoft + (ustawione ``MICROSOFT_AUTH_CLIENT_ID``). + """ + if not client_id or not is_multi_tenant(tenant_id): + return None + shown = (tenant_id or "").strip() or "(pusty)" + return ( + "Logowanie przez Microsoft działa w trybie wielokatalogowym: " + f"MICROSOFT_AUTH_TENANT_ID = {shown}. Konta Microsoft z dowolnego " + "katalogu Entra ID mogą przejść przez logowanie, a BPP nie sprawdza, " + "z którego katalogu pochodzi token." + ) + + +#: Podpowiedź dołączana do ostrzeżenia (system check i komunikat po logowaniu). +TENANT_HINT = ( + "Ustaw MICROSOFT_AUTH_TENANT_ID na identyfikator katalogu uczelni (GUID, " + "portal Azure: Microsoft Entra ID -> Przegląd -> Identyfikator dzierżawy) " + "albo jego domenę, np. uczelnia.onmicrosoft.com, i zrestartuj serwer." +) + + +def ostrzez_superusera_o_tenancie_microsoft(sender, request, user, **kwargs): + """Handler ``user_logged_in``: superuser dostaje komunikat, gdy logowanie + Microsoft jest włączone bez konkretnego katalogu (tenanta). + + Nigdy nie przerywa logowania — błędy są logowane. + """ + if request is None or not getattr(user, "is_superuser", False): + return + + try: + from django.conf import settings + from django.contrib import messages + + tresc = microsoft_tenant_warning( + getattr(settings, "MICROSOFT_AUTH_CLIENT_ID", None), + getattr(settings, "MICROSOFT_AUTH_TENANT_ID", None), + ) + if tresc: + messages.warning(request, f"{tresc} {TENANT_HINT}", fail_silently=True) + except Exception: + # Komunikat jest informacyjny — żaden błąd nie może zablokować + # logowania. Pełny traceback trafia do logów. + logger.exception( + "Nieoczekiwany błąd przy sprawdzaniu konfiguracji tenanta Microsoft." ) - return value def claims_match_tenant(claims, tenant_id, expected_issuer): diff --git a/src/bpp/checks.py b/src/bpp/checks.py index 95dcca3ff..1d62b3cb8 100644 --- a/src/bpp/checks.py +++ b/src/bpp/checks.py @@ -4,6 +4,7 @@ wielouczelniana). - ``bpp.W101``–``bpp.W104``: szyfrowanie sekretów integracji (EncryptedTextField). +- ``bpp.W201``: logowanie Microsoft bez konkretnego katalogu (tenanta). Wszystkie są ostrzeżeniami (Warning) — nie blokują startu, ``migrate`` ani buildu. Checki zaglądające do bazy mają tag ``Tags.database`` i działają @@ -165,3 +166,23 @@ def sekrety_w_bazie_check(app_configs, databases=None, **kwargs): ) ) return wynik + + +@checks.register(checks.Tags.security) +def microsoft_tenant_check(app_configs, **kwargs): + """Ostrzeż, gdy logowanie Microsoft nie wskazuje konkretnego katalogu.""" + from django.conf import settings + + from bpp.auth.microsoft_conf import ( + TENANT_HINT, + W_TENANT_ID, + microsoft_tenant_warning, + ) + + tresc = microsoft_tenant_warning( + getattr(settings, "MICROSOFT_AUTH_CLIENT_ID", None), + getattr(settings, "MICROSOFT_AUTH_TENANT_ID", None), + ) + if not tresc: + return [] + return [checks.Warning(tresc, hint=TENANT_HINT, id=W_TENANT_ID)] diff --git a/src/bpp/newsfragments/sec-microsoft-tenant.bugfix.rst b/src/bpp/newsfragments/sec-microsoft-tenant.bugfix.rst index 43fd6bfa1..bdc8a366a 100644 --- a/src/bpp/newsfragments/sec-microsoft-tenant.bugfix.rst +++ b/src/bpp/newsfragments/sec-microsoft-tenant.bugfix.rst @@ -1 +1 @@ -Wzmocniono zabezpieczenia logowania przez Microsoft (Office 365): wymagane jest wskazanie konkretnego katalogu Entra ID (``MICROSOFT_AUTH_TENANT_ID``), weryfikowane są claimy ``iss``/``tid`` tokenu, a nowe konto Microsoft nie jest automatycznie wiązane po adresie e-mail z istniejącym kontem administracyjnym lub posiadającym hasło lokalne (chyba że ``MICROSOFT_AUTH_LINK_BY_EMAIL=True``). +Wzmocniono zabezpieczenia logowania przez Microsoft (Office 365): przy ustawionym konkretnym katalogu Entra ID (``MICROSOFT_AUTH_TENANT_ID``) weryfikowane są claimy ``iss``/``tid`` tokenu, brak konkretnego katalogu (pusty, ``common``, ``organizations``, ``consumers``) zgłaszany jest ostrzeżeniem ``bpp.W201`` i komunikatem dla superusera po zalogowaniu, a nowe konto Microsoft nie jest automatycznie wiązane po adresie e-mail z istniejącym kontem administracyjnym lub posiadającym hasło lokalne (chyba że ``MICROSOFT_AUTH_LINK_BY_EMAIL=True``). diff --git a/src/bpp/tests/test_auth_microsoft.py b/src/bpp/tests/test_auth_microsoft.py index e15d4a7e4..0da5550fc 100644 --- a/src/bpp/tests/test_auth_microsoft.py +++ b/src/bpp/tests/test_auth_microsoft.py @@ -5,15 +5,19 @@ import pytest from django.contrib.auth import get_user_model -from django.core.exceptions import ImproperlyConfigured +from django.contrib.auth.signals import user_logged_in +from django.contrib.messages import get_messages +from django.core import checks from django.test import override_settings from model_bakery import baker from bpp.auth.microsoft_conf import ( + W_TENANT_ID, claims_match_tenant, may_link_existing_users, - validate_microsoft_tenant_id, + normalize_microsoft_tenant_id, ) +from bpp.checks import microsoft_tenant_check from django_bpp.external_auth import EXTERNAL_AUTH_BACKENDS, MICROSOFT_BACKEND TENANT = "11111111-2222-3333-4444-555555555555" @@ -38,17 +42,124 @@ def _claims(tid=TENANT, iss=ISSUER, **extra): # --- konfiguracja --------------------------------------------------------- -@pytest.mark.parametrize( - "value", [None, "", " ", "common", "Common", "organizations", "consumers"] +HASLO = "poprawne-haslo-testowe-123" +OSTRZEZENIE = "trybie wielokatalogowym" +WIELOKATALOGOWE = ["", "common", "Common", "organizations", "consumers"] + + +@pytest.mark.parametrize("value", [None, "", " "]) +def test_normalize_tenant_pusty_to_common(value): + assert normalize_microsoft_tenant_id(value) == "common" + + +@pytest.mark.parametrize("value", [TENANT, "uczelnia.onmicrosoft.com", "common"]) +def test_normalize_tenant_zachowuje_wartosc(value): + assert normalize_microsoft_tenant_id(f" {value} ") == value + + +@pytest.mark.parametrize("tenant", WIELOKATALOGOWE) +def test_check_ostrzega_przy_braku_konkretnego_tenanta(tenant): + with override_settings( + MICROSOFT_AUTH_CLIENT_ID="client-id", MICROSOFT_AUTH_TENANT_ID=tenant + ): + wynik = microsoft_tenant_check(None) + assert len(wynik) == 1 + assert wynik[0].id == W_TENANT_ID == "bpp.W201" + assert isinstance(wynik[0], checks.Warning) + assert "MICROSOFT_AUTH_TENANT_ID" in wynik[0].hint + assert "onmicrosoft.com" in wynik[0].hint + + +@pytest.mark.parametrize("tenant", [TENANT, "uczelnia.onmicrosoft.com"]) +def test_check_brak_ostrzezenia_dla_konkretnego_tenanta(tenant): + with override_settings( + MICROSOFT_AUTH_CLIENT_ID="client-id", MICROSOFT_AUTH_TENANT_ID=tenant + ): + assert microsoft_tenant_check(None) == [] + + +def test_check_brak_ostrzezenia_bez_logowania_microsoft(): + with override_settings(MICROSOFT_AUTH_CLIENT_ID="", MICROSOFT_AUTH_TENANT_ID=""): + assert microsoft_tenant_check(None) == [] + + +def test_check_zarejestrowany(): + assert microsoft_tenant_check in checks.registry.registry.get_checks() + + +# --- komunikat dla superusera po zalogowaniu -------------------------------- + + +def _zaloguj(client, username): + return client.post( + "/admin/login/", + {"username": username, "password": HASLO, "next": "/admin/"}, + follow=True, + ) + + +def _komunikaty(response): + return [str(m) for m in get_messages(response.wsgi_request)] + + +@pytest.mark.django_db +@override_settings( + MICROSOFT_AUTH_CLIENT_ID="client-id", MICROSOFT_AUTH_TENANT_ID="common" +) +def test_superuser_po_zalogowaniu_dostaje_ostrzezenie(client): + get_user_model().objects.create_superuser( + username="szef", password=HASLO, email="szef@example.com" + ) + komunikaty = _komunikaty(_zaloguj(client, "szef")) + assert any(OSTRZEZENIE in m and "onmicrosoft.com" in m for m in komunikaty), ( + komunikaty + ) + + +@pytest.mark.django_db +@override_settings( + MICROSOFT_AUTH_CLIENT_ID="client-id", MICROSOFT_AUTH_TENANT_ID="common" +) +def test_zwykly_staff_nie_dostaje_ostrzezenia(client): + user = baker.make(get_user_model(), username="staff1", is_staff=True) + user.set_password(HASLO) + user.save() + komunikaty = _komunikaty(_zaloguj(client, "staff1")) + assert not any(OSTRZEZENIE in m for m in komunikaty) + + +@pytest.mark.django_db +@override_settings( + MICROSOFT_AUTH_CLIENT_ID="client-id", MICROSOFT_AUTH_TENANT_ID=TENANT +) +def test_superuser_konkretny_tenant_brak_ostrzezenia(client): + get_user_model().objects.create_superuser( + username="szef", password=HASLO, email="szef@example.com" + ) + komunikaty = _komunikaty(_zaloguj(client, "szef")) + assert not any(OSTRZEZENIE in m for m in komunikaty) + + +@pytest.mark.django_db +@override_settings( + MICROSOFT_AUTH_CLIENT_ID="client-id", MICROSOFT_AUTH_TENANT_ID="common" ) -def test_validate_tenant_odrzuca_puste_i_wielotenantowe(value): - with pytest.raises(ImproperlyConfigured): - validate_microsoft_tenant_id(value) +def test_blad_przy_ostrzezeniu_nie_blokuje_logowania(client, caplog): + get_user_model().objects.create_superuser( + username="szef", password=HASLO, email="szef@example.com" + ) + with mock.patch( + "bpp.auth.microsoft_conf.microsoft_tenant_warning", + side_effect=RuntimeError("awaria"), + ): + response = _zaloguj(client, "szef") + assert response.wsgi_request.user.is_authenticated + assert "tenanta Microsoft" in caplog.text -@pytest.mark.parametrize("value", [TENANT, "uczelnia.onmicrosoft.com"]) -def test_validate_tenant_akceptuje_konkretny_katalog(value): - assert validate_microsoft_tenant_id(f" {value} ") == value +def test_handler_podpiety_pod_user_logged_in(): + klucze = [klucz for klucz, *_ in user_logged_in.receivers] + assert any(k[0] == "bpp.ostrzez_superusera_o_tenancie_microsoft" for k in klucze) def test_backend_bpp_rozpoznawany_jako_zewnetrzny(): @@ -342,3 +453,62 @@ def openid_config(self): with mock.patch("bpp.auth.microsoft.cache") as cache: assert backend._authenticate_microsoft_user() == user assert cache.delete.called + + +# --- backend w trybie wielokatalogowym (brak konkretnego tenanta) ----------- + + +@pytest.mark.uruchom_tylko_z_microsoft_auth +@pytest.mark.django_db +@pytest.mark.parametrize("tenant", ["common", "organizations"]) +def test_backend_tryb_wielokatalogowy_loguje_powiazane_konto(tenant): + from microsoft_auth.models import MicrosoftAccount + + user = baker.make(get_user_model(), email="jan@example.com", password="") + baker.make(MicrosoftAccount, microsoft_id="ms-sub-1", user=user) + + backend = _backend( + _claims(tid=OTHER_TENANT, iss=OTHER_ISSUER), + issuer="https://login.microsoftonline.com/{tenantid}/v2.0", + ) + with override_settings(MICROSOFT_AUTH_TENANT_ID=tenant): + assert backend._authenticate_microsoft_user() == user + + +@pytest.mark.uruchom_tylko_z_microsoft_auth +@pytest.mark.django_db +@override_settings( + MICROSOFT_AUTH_TENANT_ID="common", MICROSOFT_AUTH_LINK_BY_EMAIL=False +) +@pytest.mark.parametrize("attrs", [{"is_staff": True}, {"is_superuser": True}]) +def test_backend_tryb_wielokatalogowy_nie_wiaze_konta_staff(attrs): + from microsoft_auth.models import MicrosoftAccount + + konto = baker.make(get_user_model(), email="jan@example.com", password="", **attrs) + backend = _backend(_claims(tid=OTHER_TENANT, iss=OTHER_ISSUER)) + assert backend._authenticate_microsoft_user() is None + assert not MicrosoftAccount.objects.filter(user=konto).exists() + + +@pytest.mark.uruchom_tylko_z_microsoft_auth +@pytest.mark.django_db +@override_settings( + MICROSOFT_AUTH_TENANT_ID="common", MICROSOFT_AUTH_LINK_BY_EMAIL=False +) +def test_backend_tryb_wielokatalogowy_nie_wiaze_konta_z_haslem(): + konto = baker.make(get_user_model(), email="jan@example.com") + konto.set_password(HASLO) + konto.save() + backend = _backend(_claims()) + assert backend._authenticate_microsoft_user() is None + + +@pytest.mark.uruchom_tylko_z_microsoft_auth +@pytest.mark.django_db +@override_settings(MICROSOFT_AUTH_TENANT_ID="common") +def test_backend_tryb_wielokatalogowy_odmawia_przy_pustym_lub_wieloznacznym_emailu(): + baker.make(get_user_model(), email="jan@example.com", password="", _quantity=2) + liczba = get_user_model().objects.count() + assert _backend(_claims(email=""))._authenticate_microsoft_user() is None + assert _backend(_claims(sub="ms-2"))._authenticate_microsoft_user() is None + assert get_user_model().objects.count() == liczba diff --git a/src/bpp/tests/test_konta_bez_uczelni.py b/src/bpp/tests/test_konta_bez_uczelni.py index 0c55302e6..0cc5c25f3 100644 --- a/src/bpp/tests/test_konta_bez_uczelni.py +++ b/src/bpp/tests/test_konta_bez_uczelni.py @@ -89,7 +89,11 @@ def test_komunikat_przechodzi_przez_storage_jako_safestring( CookieStorage.cookie_name ].value - wiadomosci = list(CookieStorage(request)) + # Filtr po treści: inne handlery user_logged_in (np. ostrzeżenie + # bpp.W201 o konfiguracji Microsoft) mogą dodać własne komunikaty. + wiadomosci = [ + m for m in CookieStorage(request) if "Pokaż te konta" in str(m.message) + ] assert len(wiadomosci) == 1 assert isinstance(wiadomosci[0].message, SafeData) diff --git a/src/django_bpp/settings/base.py b/src/django_bpp/settings/base.py index 4175697c7..b7db36d48 100644 --- a/src/django_bpp/settings/base.py +++ b/src/django_bpp/settings/base.py @@ -13,7 +13,7 @@ from celery.schedules import crontab from django.core.exceptions import ImproperlyConfigured -from bpp.auth.microsoft_conf import validate_microsoft_tenant_id +from bpp.auth.microsoft_conf import normalize_microsoft_tenant_id from bpp.util import slugify_function from django_bpp.channels_prefix import get_channels_prefix from django_bpp.rollbar_filters import zbuduj_exception_level_filters @@ -1377,9 +1377,10 @@ def can_login_as(request, target_user): ) ) from e MICROSOFT_AUTH_CLIENT_SECRET = env("MICROSOFT_AUTH_CLIENT_SECRET") - # Fail-closed: wymagany konkretny katalog Entra ID (GUID lub domena); - # "common"/"organizations"/"consumers" albo brak wartości -> błąd konfiguracji. - MICROSOFT_AUTH_TENANT_ID = validate_microsoft_tenant_id( + # Zalecany konkretny katalog Entra ID (GUID lub domena). Pusty -> + # "common" (tryb wielokatalogowy): start nie jest blokowany, ale system + # check bpp.W201 i komunikat dla superusera ostrzegają o konfiguracji. + MICROSOFT_AUTH_TENANT_ID = normalize_microsoft_tenant_id( env("MICROSOFT_AUTH_TENANT_ID") ) # Automatyczne wiązanie nowego konta Microsoft z istniejącym kontem BPP From 5503435b8712815fbe30fe82fbba8ea78abb73f1 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Micha=C5=82=20Pasternak?= Date: Sat, 24 Oct 2026 22:45:37 +0200 Subject: [PATCH 4/4] =?UTF-8?q?fix(security):=20tryb=20wielokatalogowy=20M?= =?UTF-8?q?icrosoft=20tylko=20dla=20kont=20ju=C5=BC=20powi=C4=85zanych?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Bez konkretnego MICROSOFT_AUTH_TENANT_ID logują się wyłącznie konta Microsoft już powiązane z kontem BPP: bez zakładania nowych kont i bez wiązania po adresie e-mail (także dla kont bez uprawnień), z neutralnym komunikatem dla użytkownika. Jawny opt-in MICROSOFT_AUTH_MULTI_TENANT_ALLOW_LINKING=True przywraca ostrożne reguły wiązania. Treść bpp.W201 i komunikatu dla superusera opisuje ten tryb. Przy konkretnym tenancie zachowanie bez zmian. Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_01UJ23Rb1c5UtxHTfAke9ENQ --- docs/administrator/zaawansowane.md | 23 +++- src/bpp/auth/microsoft.py | 52 +++++++- src/bpp/auth/microsoft_conf.py | 26 +++- src/bpp/checks.py | 1 + .../sec-microsoft-tenant.bugfix.rst | 2 +- src/bpp/tests/test_auth_microsoft.py | 123 +++++++++++++++--- src/django_bpp/settings/base.py | 6 + 7 files changed, 197 insertions(+), 36 deletions(-) diff --git a/docs/administrator/zaawansowane.md b/docs/administrator/zaawansowane.md index 9a3589efc..b31c0da25 100644 --- a/docs/administrator/zaawansowane.md +++ b/docs/administrator/zaawansowane.md @@ -206,6 +206,7 @@ zmiennych `MICROSOFT_AUTH_CLIENT_ID` oraz `MICROSOFT_AUTH_CLIENT_SECRET`. | `MICROSOFT_AUTH_CLIENT_SECRET` | Sekret tej aplikacji. | | `MICROSOFT_AUTH_TENANT_ID` | **Zalecany.** Identyfikator katalogu (GUID) albo jego domena, np. `uczelnia.onmicrosoft.com`. | | `MICROSOFT_AUTH_LINK_BY_EMAIL` | Domyślnie `False`. Patrz niżej. | +| `MICROSOFT_AUTH_MULTI_TENANT_ALLOW_LINKING` | Domyślnie `False`. Dotyczy tylko trybu bez konkretnego katalogu. Patrz niżej. | | `MICROSOFT_AUTH_EXTRA_SCOPES` | Dodatkowe zakresy OAuth (opcjonalnie). | ### Zalecany konkretny katalog (tenant) @@ -223,12 +224,23 @@ pusty albo ma wartość `common`, `organizations` lub `consumers`, BPP (dla `common` i `consumers` także prywatne konta Microsoft), o ile rejestracja aplikacji w Entra ID na to pozwala; BPP nie sprawdza, z którego katalogu pochodzi token, -- zasady wiązania kont opisane niżej obowiązują bez zmian, +- logują się **wyłącznie konta Microsoft już powiązane** z kontem BPP; + BPP nie zakłada nowych kont i nie wiąże kont po adresie e-mail (także + zwykłych kont bez uprawnień), bo w tym trybie adresu e-mail z tokenu nie da + się zweryfikować; użytkownik niepowiązanego konta zobaczy komunikat + „Konto Microsoft nie jest powiązane z kontem w BPP — skontaktuj się + z administratorem”, a administrator może powiązać konto ręcznie w module + administracyjnym (*Microsoft accounts*), +- `MICROSOFT_AUTH_MULTI_TENANT_ALLOW_LINKING=True` przywraca w tym trybie + zakładanie nowych kont i wiązanie po e-mailu według zasad opisanych niżej + (konta z uprawnieniami lub hasłem lokalnym nadal nie są wiązane + automatycznie) — używaj tylko świadomie, - `manage.py check` zgłasza ostrzeżenie `bpp.W201`, a superuser po każdym zalogowaniu widzi komunikat z tą samą podpowiedzią. -Aby usunąć ostrzeżenie, ustaw `MICROSOFT_AUTH_TENANT_ID` (GUID albo domena -katalogu) i zrestartuj serwer aplikacji. +Aby usunąć ostrzeżenie i przywrócić zwykłe zakładanie oraz wiązanie kont, +ustaw `MICROSOFT_AUTH_TENANT_ID` (GUID albo domena katalogu) i zrestartuj +serwer aplikacji. ### Wiązanie kont Microsoft z istniejącymi kontami BPP @@ -244,8 +256,9 @@ powiąże automatycznie konta Microsoft z istniejącym użytkownikiem, który: Takie logowanie zostanie odrzucone. Administrator może powiązać konto ręcznie w module administracyjnym (*Microsoft accounts*), albo — jeżeli adresy e-mail w katalogu są w pełni zaufane — ustawić `MICROSOFT_AUTH_LINK_BY_EMAIL=True`. -Zwykłe konta bez hasła lokalnego oraz nowi użytkownicy są wiązani/zakładani -automatycznie, jak dotychczas. +Przy skonfigurowanym konkretnym katalogu zwykłe konta bez hasła lokalnego oraz +nowi użytkownicy są wiązani/zakładani automatycznie, jak dotychczas (w trybie +wielokatalogowym — patrz wyżej). Konta Microsoft **już powiązane** z użytkownikiem BPP logują się bez zmian. Po aktualizacji użytkownicy zalogowani przez Microsoft muszą jednorazowo diff --git a/src/bpp/auth/microsoft.py b/src/bpp/auth/microsoft.py index 07c791284..bd28e9ade 100644 --- a/src/bpp/auth/microsoft.py +++ b/src/bpp/auth/microsoft.py @@ -5,8 +5,10 @@ * przy ustawionym konkretnym katalogu Entra ID (GUID lub domena) przyjmuje wyłącznie tokeny wystawione przez ten katalog (claimy ``iss``/``tid``); bez konkretnego katalogu (``common``/``organizations``/``consumers`` lub - pusty) działa w trybie wielokatalogowym, jak biblioteka — tę konfigurację - zgłasza system check ``bpp.W201``, + pusty) działa w trybie wielokatalogowym — tę konfigurację zgłasza system + check ``bpp.W201``; w tym trybie logują się wyłącznie konta Microsoft już + powiązane z kontem BPP (bez wiązania po e-mailu i bez zakładania nowych + kont), chyba że ``MICROSOFT_AUTH_MULTI_TENANT_ALLOW_LINKING = True``, * nie wiąże nowego konta Microsoft z kontem BPP, gdy token nie ma e-maila albo e-mail pasuje do więcej niż jednego konta, * nie wiąże automatycznie nowego konta Microsoft z istniejącym kontem @@ -19,6 +21,7 @@ import logging from django.conf import settings +from django.contrib import messages from django.contrib.auth import get_user_model from django.core.cache import cache from microsoft_auth.backends import MicrosoftAuthenticationBackend @@ -33,8 +36,42 @@ logger = logging.getLogger(__name__) +#: Komunikat dla użytkownika, gdy konto Microsoft nie jest powiązane z BPP. +KOMUNIKAT_NIEPOWIAZANE = ( + "Konto Microsoft nie jest powiązane z kontem w BPP — skontaktuj się " + "z administratorem." +) + class BppMicrosoftAuthenticationBackend(MicrosoftAuthenticationBackend): + def authenticate(self, request, code=None): + self._request = request + return super().authenticate(request, code=code) + + def _odmowa_niepowiazane(self): + request = getattr(self, "_request", None) + if request is not None: + messages.error(request, KOMUNIKAT_NIEPOWIAZANE, fail_silently=True) + return None + + def _get_linked_user_only(self, claims): + """Tryb wielokatalogowy bez opt-in: tylko konta już powiązane.""" + from microsoft_auth.models import MicrosoftAccount + + account = ( + MicrosoftAccount.objects.filter(microsoft_id=claims.get("sub")) + .select_related("user") + .first() + ) + if account is not None and account.user is not None: + return account.user + logger.warning( + "Odrzucono logowanie Microsoft (tryb wielokatalogowy): konto " + "%r nie jest powiązane z użytkownikiem BPP.", + claims.get("sub"), + ) + return self._odmowa_niepowiazane() + def _expected_issuer(self, tenant_id): issuer = (self.microsoft.openid_config or {}).get("issuer") or "" if self._issuer_is_stale(issuer, tenant_id): @@ -63,7 +100,12 @@ def _authenticate_microsoft_user(self): if is_multi_tenant(tenant_id): # Tryb wielokatalogowy: nie ma katalogu, względem którego można # sprawdzić iss/tid (podpis i audience sprawdza biblioteka). - # Ostrożne wiązanie kont w _verify_microsoft_user obowiązuje dalej. + # Adres e-mail w tokenie nie jest wtedy wiarygodny — bez jawnego + # opt-in logują się wyłącznie konta już powiązane. + if not getattr( + settings, "MICROSOFT_AUTH_MULTI_TENANT_ALLOW_LINKING", False + ): + return self._get_linked_user_only(claims) return self._get_user_from_microsoft(claims) expected_issuer = self._expected_issuer(tenant_id) @@ -108,7 +150,7 @@ def _verify_microsoft_user(self, microsoft_user, data): "Odrzucono logowanie Microsoft %r: token bez adresu e-mail.", microsoft_user.microsoft_id, ) - return None + return self._odmowa_niepowiazane() User = get_user_model() matching = list(User.objects.filter(email__iexact=email)[:2]) @@ -120,7 +162,7 @@ def _verify_microsoft_user(self, microsoft_user, data): "w adminie lub ustaw MICROSOFT_AUTH_LINK_BY_EMAIL.", microsoft_user.microsoft_id, ) - return None + return self._odmowa_niepowiazane() first_name, last_name = self._split_name(data.get("name")) diff --git a/src/bpp/auth/microsoft_conf.py b/src/bpp/auth/microsoft_conf.py index c52ad8eeb..5ac74a0ab 100644 --- a/src/bpp/auth/microsoft_conf.py +++ b/src/bpp/auth/microsoft_conf.py @@ -45,7 +45,7 @@ def is_multi_tenant(tenant_id): return not value or value in MULTI_TENANT_VALUES -def microsoft_tenant_warning(client_id, tenant_id): +def microsoft_tenant_warning(client_id, tenant_id, allow_linking=False): """Treść ostrzeżenia o braku konkretnego tenanta albo ``None``. Ostrzeżenie dotyczy wyłącznie instalacji z włączonym logowaniem Microsoft @@ -54,11 +54,20 @@ def microsoft_tenant_warning(client_id, tenant_id): if not client_id or not is_multi_tenant(tenant_id): return None shown = (tenant_id or "").strip() or "(pusty)" - return ( - "Logowanie przez Microsoft działa w trybie wielokatalogowym: " - f"MICROSOFT_AUTH_TENANT_ID = {shown}. Konta Microsoft z dowolnego " - "katalogu Entra ID mogą przejść przez logowanie, a BPP nie sprawdza, " - "z którego katalogu pochodzi token." + tresc = ( + "Logowanie przez Microsoft działa w trybie wielokatalogowym " + f"(MICROSOFT_AUTH_TENANT_ID = {shown}): BPP nie sprawdza, z którego " + "katalogu Entra ID pochodzi token." + ) + if allow_linking: + return tresc + ( + " Włączono MICROSOFT_AUTH_MULTI_TENANT_ALLOW_LINKING — nowe konta " + "Microsoft są zakładane i wiązane po adresie e-mail, którego " + "w tym trybie nie da się zweryfikować." + ) + return tresc + ( + " Logują się wyłącznie konta Microsoft już powiązane z kontem BPP; " + "zakładanie nowych kont i wiązanie po adresie e-mail są wyłączone." ) @@ -66,7 +75,9 @@ def microsoft_tenant_warning(client_id, tenant_id): TENANT_HINT = ( "Ustaw MICROSOFT_AUTH_TENANT_ID na identyfikator katalogu uczelni (GUID, " "portal Azure: Microsoft Entra ID -> Przegląd -> Identyfikator dzierżawy) " - "albo jego domenę, np. uczelnia.onmicrosoft.com, i zrestartuj serwer." + "albo jego domenę, np. uczelnia.onmicrosoft.com, i zrestartuj serwer — " + "wtedy działa pełna weryfikacja katalogu, a zakładanie i wiązanie kont " + "wracają do zwykłych reguł." ) @@ -86,6 +97,7 @@ def ostrzez_superusera_o_tenancie_microsoft(sender, request, user, **kwargs): tresc = microsoft_tenant_warning( getattr(settings, "MICROSOFT_AUTH_CLIENT_ID", None), getattr(settings, "MICROSOFT_AUTH_TENANT_ID", None), + getattr(settings, "MICROSOFT_AUTH_MULTI_TENANT_ALLOW_LINKING", False), ) if tresc: messages.warning(request, f"{tresc} {TENANT_HINT}", fail_silently=True) diff --git a/src/bpp/checks.py b/src/bpp/checks.py index 1d62b3cb8..c77590bbc 100644 --- a/src/bpp/checks.py +++ b/src/bpp/checks.py @@ -182,6 +182,7 @@ def microsoft_tenant_check(app_configs, **kwargs): tresc = microsoft_tenant_warning( getattr(settings, "MICROSOFT_AUTH_CLIENT_ID", None), getattr(settings, "MICROSOFT_AUTH_TENANT_ID", None), + getattr(settings, "MICROSOFT_AUTH_MULTI_TENANT_ALLOW_LINKING", False), ) if not tresc: return [] diff --git a/src/bpp/newsfragments/sec-microsoft-tenant.bugfix.rst b/src/bpp/newsfragments/sec-microsoft-tenant.bugfix.rst index bdc8a366a..508d02082 100644 --- a/src/bpp/newsfragments/sec-microsoft-tenant.bugfix.rst +++ b/src/bpp/newsfragments/sec-microsoft-tenant.bugfix.rst @@ -1 +1 @@ -Wzmocniono zabezpieczenia logowania przez Microsoft (Office 365): przy ustawionym konkretnym katalogu Entra ID (``MICROSOFT_AUTH_TENANT_ID``) weryfikowane są claimy ``iss``/``tid`` tokenu, brak konkretnego katalogu (pusty, ``common``, ``organizations``, ``consumers``) zgłaszany jest ostrzeżeniem ``bpp.W201`` i komunikatem dla superusera po zalogowaniu, a nowe konto Microsoft nie jest automatycznie wiązane po adresie e-mail z istniejącym kontem administracyjnym lub posiadającym hasło lokalne (chyba że ``MICROSOFT_AUTH_LINK_BY_EMAIL=True``). +Wzmocniono zabezpieczenia logowania przez Microsoft (Office 365): przy ustawionym konkretnym katalogu Entra ID (``MICROSOFT_AUTH_TENANT_ID``) weryfikowane są claimy ``iss``/``tid`` tokenu; bez konkretnego katalogu (pusty, ``common``, ``organizations``, ``consumers``) serwer startuje, ale zgłaszane jest ostrzeżenie ``bpp.W201`` i komunikat dla superusera po zalogowaniu, a logować mogą się wyłącznie konta Microsoft już powiązane z kontem BPP (bez zakładania nowych kont i wiązania po e-mailu, chyba że ``MICROSOFT_AUTH_MULTI_TENANT_ALLOW_LINKING=True``); nowe konto Microsoft nie jest automatycznie wiązane po adresie e-mail z istniejącym kontem administracyjnym lub posiadającym hasło lokalne (chyba że ``MICROSOFT_AUTH_LINK_BY_EMAIL=True``). diff --git a/src/bpp/tests/test_auth_microsoft.py b/src/bpp/tests/test_auth_microsoft.py index 0da5550fc..6ec7e70a3 100644 --- a/src/bpp/tests/test_auth_microsoft.py +++ b/src/bpp/tests/test_auth_microsoft.py @@ -68,6 +68,18 @@ def test_check_ostrzega_przy_braku_konkretnego_tenanta(tenant): assert isinstance(wynik[0], checks.Warning) assert "MICROSOFT_AUTH_TENANT_ID" in wynik[0].hint assert "onmicrosoft.com" in wynik[0].hint + assert "wiązanie po adresie e-mail są wyłączone" in wynik[0].msg + + +def test_check_opt_in_wspomina_o_wlaczonym_wiazaniu(): + with override_settings( + MICROSOFT_AUTH_CLIENT_ID="client-id", + MICROSOFT_AUTH_TENANT_ID="common", + MICROSOFT_AUTH_MULTI_TENANT_ALLOW_LINKING=True, + ): + wynik = microsoft_tenant_check(None) + assert [w.id for w in wynik] == ["bpp.W201"] + assert "MICROSOFT_AUTH_MULTI_TENANT_ALLOW_LINKING" in wynik[0].msg @pytest.mark.parametrize("tenant", [TENANT, "uczelnia.onmicrosoft.com"]) @@ -83,6 +95,21 @@ def test_check_brak_ostrzezenia_bez_logowania_microsoft(): assert microsoft_tenant_check(None) == [] +@pytest.mark.uruchom_tylko_z_microsoft_auth +def test_check_z_rzeczywistych_ustawien(): + # Bez override_settings: ustawienia z env (MICROSOFT_AUTH_TENANT_ID) + # po normalizacji w settings/base.py. + from django.conf import settings + + from bpp.auth.microsoft_conf import is_multi_tenant + + wynik = microsoft_tenant_check(None) + if is_multi_tenant(settings.MICROSOFT_AUTH_TENANT_ID): + assert [w.id for w in wynik] == ["bpp.W201"] + else: + assert wynik == [] + + def test_check_zarejestrowany(): assert microsoft_tenant_check in checks.registry.registry.get_checks() @@ -111,9 +138,10 @@ def test_superuser_po_zalogowaniu_dostaje_ostrzezenie(client): username="szef", password=HASLO, email="szef@example.com" ) komunikaty = _komunikaty(_zaloguj(client, "szef")) - assert any(OSTRZEZENIE in m and "onmicrosoft.com" in m for m in komunikaty), ( - komunikaty - ) + assert any( + OSTRZEZENIE in m and "onmicrosoft.com" in m and "są wyłączone" in m + for m in komunikaty + ), komunikaty @pytest.mark.django_db @@ -457,6 +485,8 @@ def openid_config(self): # --- backend w trybie wielokatalogowym (brak konkretnego tenanta) ----------- +TEMPLATE_ISSUER = "https://login.microsoftonline.com/{tenantid}/v2.0" + @pytest.mark.uruchom_tylko_z_microsoft_auth @pytest.mark.django_db @@ -468,8 +498,7 @@ def test_backend_tryb_wielokatalogowy_loguje_powiazane_konto(tenant): baker.make(MicrosoftAccount, microsoft_id="ms-sub-1", user=user) backend = _backend( - _claims(tid=OTHER_TENANT, iss=OTHER_ISSUER), - issuer="https://login.microsoftonline.com/{tenantid}/v2.0", + _claims(tid=OTHER_TENANT, iss=OTHER_ISSUER), issuer=TEMPLATE_ISSUER ) with override_settings(MICROSOFT_AUTH_TENANT_ID=tenant): assert backend._authenticate_microsoft_user() == user @@ -477,36 +506,94 @@ def test_backend_tryb_wielokatalogowy_loguje_powiazane_konto(tenant): @pytest.mark.uruchom_tylko_z_microsoft_auth @pytest.mark.django_db -@override_settings( - MICROSOFT_AUTH_TENANT_ID="common", MICROSOFT_AUTH_LINK_BY_EMAIL=False +@override_settings(MICROSOFT_AUTH_TENANT_ID="common", MICROSOFT_AUTH_LINK_BY_EMAIL=True) +@pytest.mark.parametrize( + "attrs", + [{}, {"is_staff": True}, {"is_superuser": True}], + ids=["zwykle", "staff", "superuser"], ) -@pytest.mark.parametrize("attrs", [{"is_staff": True}, {"is_superuser": True}]) -def test_backend_tryb_wielokatalogowy_nie_wiaze_konta_staff(attrs): +def test_backend_tryb_wielokatalogowy_nie_wiaze_po_emailu(attrs): from microsoft_auth.models import MicrosoftAccount konto = baker.make(get_user_model(), email="jan@example.com", password="", **attrs) + liczba = get_user_model().objects.count() + backend = _backend(_claims(tid=OTHER_TENANT, iss=OTHER_ISSUER)) assert backend._authenticate_microsoft_user() is None assert not MicrosoftAccount.objects.filter(user=konto).exists() + assert not MicrosoftAccount.objects.filter(microsoft_id="ms-sub-1").exists() + assert get_user_model().objects.count() == liczba @pytest.mark.uruchom_tylko_z_microsoft_auth @pytest.mark.django_db -@override_settings( - MICROSOFT_AUTH_TENANT_ID="common", MICROSOFT_AUTH_LINK_BY_EMAIL=False -) -def test_backend_tryb_wielokatalogowy_nie_wiaze_konta_z_haslem(): - konto = baker.make(get_user_model(), email="jan@example.com") - konto.set_password(HASLO) - konto.save() - backend = _backend(_claims()) +@override_settings(MICROSOFT_AUTH_TENANT_ID="common", MICROSOFT_AUTH_AUTO_CREATE=True) +def test_backend_tryb_wielokatalogowy_nie_zaklada_nowego_konta(): + from microsoft_auth.models import MicrosoftAccount + + liczba = get_user_model().objects.count() + backend = _backend(_claims(email="nowy@example.com", sub="ms-sub-nowy")) assert backend._authenticate_microsoft_user() is None + assert get_user_model().objects.count() == liczba + assert not MicrosoftAccount.objects.filter(microsoft_id="ms-sub-nowy").exists() @pytest.mark.uruchom_tylko_z_microsoft_auth @pytest.mark.django_db @override_settings(MICROSOFT_AUTH_TENANT_ID="common") -def test_backend_tryb_wielokatalogowy_odmawia_przy_pustym_lub_wieloznacznym_emailu(): +def test_backend_tryb_wielokatalogowy_odmowa_z_neutralnym_komunikatem(rf): + from django.contrib.messages.storage.fallback import FallbackStorage + + from bpp.auth.microsoft import KOMUNIKAT_NIEPOWIAZANE + + request = rf.get("/microsoft/auth-callback/") + request.session = {} + request._messages = FallbackStorage(request) + + backend = _backend(_claims(email="nowy@example.com", sub="ms-sub-nowy")) + backend._request = request + assert backend._authenticate_microsoft_user() is None + assert [str(m) for m in request._messages] == [KOMUNIKAT_NIEPOWIAZANE] + + +@pytest.mark.uruchom_tylko_z_microsoft_auth +@pytest.mark.django_db +@override_settings( + MICROSOFT_AUTH_TENANT_ID="common", MICROSOFT_AUTH_MULTI_TENANT_ALLOW_LINKING=True +) +def test_backend_opt_in_zaklada_nowe_konto_i_wiaze_zwykle(): + nowy = _backend( + _claims(email="nowy@example.com", sub="ms-sub-nowy") + )._authenticate_microsoft_user() + assert nowy is not None + assert nowy.email == "nowy@example.com" + + zwykle = baker.make(get_user_model(), email="jan@example.com", password="") + assert _backend(_claims())._authenticate_microsoft_user() == zwykle + + +@pytest.mark.uruchom_tylko_z_microsoft_auth +@pytest.mark.django_db +@override_settings( + MICROSOFT_AUTH_TENANT_ID="common", + MICROSOFT_AUTH_MULTI_TENANT_ALLOW_LINKING=True, + MICROSOFT_AUTH_LINK_BY_EMAIL=False, +) +@pytest.mark.parametrize("attrs", [{"is_staff": True}, {"is_superuser": True}]) +def test_backend_opt_in_nadal_nie_wiaze_konta_staff(attrs): + from microsoft_auth.models import MicrosoftAccount + + konto = baker.make(get_user_model(), email="jan@example.com", password="", **attrs) + assert _backend(_claims())._authenticate_microsoft_user() is None + assert not MicrosoftAccount.objects.filter(user=konto).exists() + + +@pytest.mark.uruchom_tylko_z_microsoft_auth +@pytest.mark.django_db +@override_settings( + MICROSOFT_AUTH_TENANT_ID="common", MICROSOFT_AUTH_MULTI_TENANT_ALLOW_LINKING=True +) +def test_backend_opt_in_odmawia_przy_pustym_lub_wieloznacznym_emailu(): baker.make(get_user_model(), email="jan@example.com", password="", _quantity=2) liczba = get_user_model().objects.count() assert _backend(_claims(email=""))._authenticate_microsoft_user() is None diff --git a/src/django_bpp/settings/base.py b/src/django_bpp/settings/base.py index b7db36d48..ee18e804e 100644 --- a/src/django_bpp/settings/base.py +++ b/src/django_bpp/settings/base.py @@ -64,6 +64,7 @@ def int_or_none(v): MICROSOFT_AUTH_TENANT_ID=(str, None), MICROSOFT_AUTH_EXTRA_SCOPES=(str, ""), MICROSOFT_AUTH_LINK_BY_EMAIL=(bool, False), + MICROSOFT_AUTH_MULTI_TENANT_ALLOW_LINKING=(bool, False), # # Email # @@ -1386,6 +1387,11 @@ def can_login_as(request, target_user): # Automatyczne wiązanie nowego konta Microsoft z istniejącym kontem BPP # z uprawnieniami / hasłem lokalnym po samym e-mailu — domyślnie wyłączone. MICROSOFT_AUTH_LINK_BY_EMAIL = env("MICROSOFT_AUTH_LINK_BY_EMAIL") + # Tryb wielokatalogowy: domyślnie logują się wyłącznie konta Microsoft już + # powiązane z BPP (bez wiązania po e-mailu i zakładania nowych kont). + MICROSOFT_AUTH_MULTI_TENANT_ALLOW_LINKING = env( + "MICROSOFT_AUTH_MULTI_TENANT_ALLOW_LINKING" + ) MICROSOFT_AUTH_LOGIN_TYPE = "ma" MICROSOFT_AUTH_EXTRA_SCOPES = env("MICROSOFT_AUTH_EXTRA_SCOPES")