From 662086c8ba957e1a7f9a8d426919e7b845ee0c5c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Micha=C5=82=20Pasternak?= Date: Fri, 24 Jul 2026 23:37:23 +0200 Subject: [PATCH 1/3] =?UTF-8?q?fix(zglos):=20podpowied=C5=BA=20jednostki?= =?UTF-8?q?=20zn=C3=B3w=20dzia=C5=82a=20dla=20nie-redaktor=C3=B3w=20(403?= =?UTF-8?q?=E2=86=92200)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Endpoint /bpp/api/ostatnia-jednostka-i-dyscyplina/ dostał w e892142ff bramkę WprowadzanieDanychRequiredMixin, choć niczego nie mutuje — czyta Autor/Autor_Dyscyplina i zwraca JSON. Konsumuje go autorform_dependant.js, ładowany także do PUBLICZNEGO formularza zglos_publikacje, więc każdy zgłaszający bez roli redaktora dostawał 403 i tracił podpowiedź jednostki oraz dyscypliny — po cichu, bo to AJAX. Wracamy do LoginRequiredMixin (stan sprzed e892142ff): anonim nadal dostaje redirect na login, co pokrywa istniejący test test_api_endpoints_require_login. W Rollbarze objawiało się to ~19 osobnymi itemami PermissionDenied (m.in. #1532, #1495, #1486) — po jednym na wystąpienie. Co-Authored-By: Claude Opus 5 (1M context) Claude-Session: https://claude.ai/code/session_01NcAqeqyqBzNEkkVnhpHDaH --- ...+zglos-podpowiedz-jednostki-403.bugfix.rst | 6 ++++ src/bpp/tests/test_views/test_api.py | 28 +++++++++++++++++++ src/bpp/views/api/__init__.py | 11 +++++++- 3 files changed, 44 insertions(+), 1 deletion(-) create mode 100644 src/bpp/newsfragments/+zglos-podpowiedz-jednostki-403.bugfix.rst diff --git a/src/bpp/newsfragments/+zglos-podpowiedz-jednostki-403.bugfix.rst b/src/bpp/newsfragments/+zglos-podpowiedz-jednostki-403.bugfix.rst new file mode 100644 index 000000000..a58bfd116 --- /dev/null +++ b/src/bpp/newsfragments/+zglos-podpowiedz-jednostki-403.bugfix.rst @@ -0,0 +1,6 @@ +Formularz „Zgłoś publikację" znów podpowiada jednostkę i dyscyplinę autora. +Endpoint ``/bpp/api/ostatnia-jednostka-i-dyscyplina/`` — który tylko czyta dane +i niczego nie zapisuje — trafił przez pomyłkę pod bramkę uprawnień +redaktorskich, więc każdy zgłaszający bez roli redaktora dostawał w tle błąd +403 i tracił podpowiedź (po cichu, bo to zapytanie AJAX). Wymagane pozostaje +samo zalogowanie. diff --git a/src/bpp/tests/test_views/test_api.py b/src/bpp/tests/test_views/test_api.py index 66646c897..7fbd1bceb 100644 --- a/src/bpp/tests/test_views/test_api.py +++ b/src/bpp/tests/test_views/test_api.py @@ -3,9 +3,11 @@ import pytest from django.urls import reverse +from model_bakery import baker from bpp.models import Autor_Dyscyplina, Typ_Odpowiedzialnosci from bpp.models.zrodlo import Punktacja_Zrodla +from bpp.permissions import moze_wprowadzac_dane from bpp.tests.util import CURRENT_YEAR, any_autor, any_habilitacja, any_zrodlo from bpp.views.api import ( OstatniaJednostkaIDyscyplinaView, @@ -420,6 +422,32 @@ def test_api_endpoints_require_login(client, url_name, url_kwargs, post_data): ) +@pytest.mark.django_db +def test_ostatnia_jednostka_dostepna_bez_uprawnien_redaktorskich( + client, autor, jednostka +): + """Podpowiadanie jednostki działa dla ZALOGOWANEGO usera bez uprawnień + redaktorskich. + + Regresja (Rollbar #1532 i ~19 bliźniaczych itemów): endpoint dostał + ``WprowadzanieDanychRequiredMixin``, choć niczego nie mutuje — tylko czyta. + Konsumuje go ``autorform_dependant.js`` ładowany do PUBLICZNEGO formularza + ``zglos_publikacje``, więc każdy zgłaszający bez roli redaktora dostawał + 403 i tracił podpowiedź jednostki (po cichu — to AJAX). + """ + jednostka.dodaj_autora(autor) + + user = baker.make("bpp.BppUser", is_staff=False, is_superuser=False) + assert not moze_wprowadzac_dane(user) + client.force_login(user) + + url = reverse("bpp:api_ostatnia_jednostka_i_dyscyplina") + response = client.post(url, data={"autor_id": autor.pk, "rok": CURRENT_YEAR}) + + assert response.status_code == 200 + assert json.loads(response.content)["jednostka_id"] == jednostka.pk + + @pytest.mark.django_db def test_upload_punktacja_zrodla_anon_does_not_write(client): """Najtwardszy regression test: anonim NIE może utworzyć Punktacja_Zrodla.""" diff --git a/src/bpp/views/api/__init__.py b/src/bpp/views/api/__init__.py index c9af13b48..39a582513 100644 --- a/src/bpp/views/api/__init__.py +++ b/src/bpp/views/api/__init__.py @@ -1,5 +1,6 @@ from decimal import Decimal, InvalidOperation +from django.contrib.auth.mixins import LoginRequiredMixin from django.db import models, transaction from django.http import JsonResponse from django.http.response import HttpResponseNotFound @@ -154,9 +155,17 @@ def ostatnia_dyscyplina(request, a, rok): return ad.dyscyplina_naukowa or ad.subdyscyplina_naukowa -class OstatniaJednostkaIDyscyplinaView(WprowadzanieDanychRequiredMixin, View): +class OstatniaJednostkaIDyscyplinaView(LoginRequiredMixin, View): """Zwraca jako JSON ostatnią jednostkę danego autora oraz ewentualnie jego dyscyplinę naukową, w sytuacji gdy jest ona jedna i określona na dany rok. + + ŚWIADOMIE ``LoginRequiredMixin``, a NIE + ``WprowadzanieDanychRequiredMixin``: widok niczego nie mutuje — czyta + ``Autor``/``Autor_Dyscyplina`` i zwraca JSON. Konsumuje go + ``autorform_dependant.js``, ładowany także do PUBLICZNEGO formularza + ``zglos_publikacje`` (patrz ``zglos_publikacje.forms``), więc bramka + redaktorska odcinała zwykłych zgłaszających od podpowiedzi jednostki + i dyscypliny — po cichu, bo to AJAX. """ def post(self, request, *args, **kw): From 4b7278650dc3ff280ae5a693432b3e4d5a011944 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Micha=C5=82=20Pasternak?= Date: Sat, 25 Jul 2026 00:07:50 +0200 Subject: [PATCH 2/3] =?UTF-8?q?fix(zglos):=20domknij=20self-review=20?= =?UTF-8?q?=E2=80=94=20test-lustro=20i=20uczciwszy=20newsfragment?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - Newsfragment obiecywał, że formularz "znów podpowiada" — dla ANONIMA to nieprawda: LoginRequiredMixin nadal daje mu 302, tak samo jak przed e892142ff. Formularz zgłoszeń przepuszcza niezalogowanych, więc to realna grupa. Doprecyzowane w newsfragmencie i w docstringu widoku, wraz z powodem, dla którego nie luzujemy tego teraz (endpoint jest oraklem istnienia autora dla całej przestrzeni PK, bez rate-limitu). - Nowy test-lustro: pozostałe trzy widoki API (rok habilitacji, punktacja źródła, upload punktacji) NADAL dają 403 zalogowanemu bez uprawnień redaktorskich. Bez tego nic nie broni przed przyszłym "skoro tamten odblokowaliśmy, odblokujmy wszystkie". Co-Authored-By: Claude Opus 5 (1M context) Claude-Session: https://claude.ai/code/session_01NcAqeqyqBzNEkkVnhpHDaH --- ...+zglos-podpowiedz-jednostki-403.bugfix.rst | 13 +++---- src/bpp/tests/test_views/test_api.py | 35 +++++++++++++++++++ src/bpp/views/api/__init__.py | 5 +++ 3 files changed, 47 insertions(+), 6 deletions(-) diff --git a/src/bpp/newsfragments/+zglos-podpowiedz-jednostki-403.bugfix.rst b/src/bpp/newsfragments/+zglos-podpowiedz-jednostki-403.bugfix.rst index a58bfd116..c36732f30 100644 --- a/src/bpp/newsfragments/+zglos-podpowiedz-jednostki-403.bugfix.rst +++ b/src/bpp/newsfragments/+zglos-podpowiedz-jednostki-403.bugfix.rst @@ -1,6 +1,7 @@ -Formularz „Zgłoś publikację" znów podpowiada jednostkę i dyscyplinę autora. -Endpoint ``/bpp/api/ostatnia-jednostka-i-dyscyplina/`` — który tylko czyta dane -i niczego nie zapisuje — trafił przez pomyłkę pod bramkę uprawnień -redaktorskich, więc każdy zgłaszający bez roli redaktora dostawał w tle błąd -403 i tracił podpowiedź (po cichu, bo to zapytanie AJAX). Wymagane pozostaje -samo zalogowanie. +Formularz „Zgłoś publikację" znów podpowiada jednostkę i dyscyplinę autora +zalogowanym użytkownikom bez uprawnień redaktorskich. Endpoint, z którego +korzysta ta podpowiedź — tylko czytający dane, niczego nie zapisujący — +trafił przez pomyłkę pod bramkę uprawnień redaktorskich, więc taki +użytkownik dostawał w tle błąd i tracił podpowiedź (po cichu, bo to +zapytanie w tle). Dla niezalogowanych podpowiedź nadal nie działa — tak jak +wcześniej. diff --git a/src/bpp/tests/test_views/test_api.py b/src/bpp/tests/test_views/test_api.py index 7fbd1bceb..388f56a27 100644 --- a/src/bpp/tests/test_views/test_api.py +++ b/src/bpp/tests/test_views/test_api.py @@ -448,6 +448,41 @@ def test_ostatnia_jednostka_dostepna_bez_uprawnien_redaktorskich( assert json.loads(response.content)["jednostka_id"] == jednostka.pk +@pytest.mark.django_db +@pytest.mark.parametrize( + "url_name,url_kwargs,post_data", + [ + ("bpp:api_rok_habilitacji", {}, {"autor_pk": 1}), + ("bpp:api_punktacja_zrodla", {"zrodlo_id": 1, "rok": CURRENT_YEAR}, {}), + ( + "bpp:api_upload_punktacja_zrodla", + {"zrodlo_id": 1, "rok": CURRENT_YEAR}, + {"impact_factor": "50.0"}, + ), + ], +) +def test_pozostale_api_nadal_wymagaja_uprawnien_redaktorskich( + client, url_name, url_kwargs, post_data +): + """Lustro poprzedniego testu: poluzowanie dotyczy JEDNEGO widoku. + + Bez tego nic nie broni przed przyszłym „skoro tamten odblokowaliśmy, to + odblokujmy wszystkie" — a te trzy albo mutują dane + (``UploadPunktacjaZrodlaView``), albo wystawiają dane redakcyjne + konsumowane wyłącznie przez JS admina. + """ + user = baker.make("bpp.BppUser", is_staff=False, is_superuser=False) + assert not moze_wprowadzac_dane(user) + client.force_login(user) + + response = client.post(reverse(url_name, kwargs=url_kwargs), data=post_data) + + assert response.status_code == 403, ( + f"{url_name} przepuszcza zalogowanego bez uprawnień redaktorskich " + f"(status={response.status_code})" + ) + + @pytest.mark.django_db def test_upload_punktacja_zrodla_anon_does_not_write(client): """Najtwardszy regression test: anonim NIE może utworzyć Punktacja_Zrodla.""" diff --git a/src/bpp/views/api/__init__.py b/src/bpp/views/api/__init__.py index 39a582513..8aa10f404 100644 --- a/src/bpp/views/api/__init__.py +++ b/src/bpp/views/api/__init__.py @@ -166,6 +166,11 @@ class OstatniaJednostkaIDyscyplinaView(LoginRequiredMixin, View): ``zglos_publikacje`` (patrz ``zglos_publikacje.forms``), więc bramka redaktorska odcinała zwykłych zgłaszających od podpowiedzi jednostki i dyscypliny — po cichu, bo to AJAX. + + Anonim NADAL dostaje 302 na login (kontrakt ``LoginRequiredMixin``), więc + dla niezalogowanych zgłaszających podpowiedź nie działa — tak samo jak + przed ``e892142ff``. Poluzowanie tego to osobna decyzja: endpoint jest + oraklem istnienia autora dla całej przestrzeni PK i nie ma rate-limitu. """ def post(self, request, *args, **kw): From 8e973cdd4b5ea05388f683af1e99e8e5709ade5e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Micha=C5=82=20Pasternak?= Date: Sat, 25 Jul 2026 00:57:32 +0200 Subject: [PATCH 3/3] =?UTF-8?q?fix(zglos):=20rozdziel=20listy=20tras=20w?= =?UTF-8?q?=20test=5Fauthz=5Fviews=20(CI=20z=C5=82apa=C5=82o=20realny=20ko?= =?UTF-8?q?nflikt)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit CI wywaliło test_api_zwykly_user_403[bpp:api_ostatnia_jednostka_i_dyscyplina] — istniejący test w src/bpp/tests/test_authz_views.py asertował 403 dla tego endpointu, a to jest dokładnie zachowanie, które ten PR zmienia. Przegapiłem ten plik, bo lokalnie uruchamiałem testy celowane (test_api.py, test_permissions.py, zglos_publikacje), a nie całe src/bpp/tests. API_ROUTES było wspólną listą dla dwóch testów o różnych kontraktach. Rozdzielone: - API_ROUTES_REDAKCYJNE — zalogowany bez uprawnień → 403 (trzy widoki mutujące/redakcyjne), - API_ROUTES = powyższe + ostatnia_jednostka — anonim → 302 na login. Ten kontrakt się NIE zmienia i nadal jest pilnowany. Komentarz przy liście wyjaśnia, dlaczego ostatnia_jednostka jest w drugiej, a nie w pierwszej, i wskazuje test pilnujący jej dostępności (200). Zweryfikowane pełnym przebiegiem src/bpp/tests: 2850 passed, 2 skipped. Co-Authored-By: Claude Opus 5 (1M context) Claude-Session: https://claude.ai/code/session_01NcAqeqyqBzNEkkVnhpHDaH --- src/bpp/tests/test_authz_views.py | 15 +++++++++++++-- 1 file changed, 13 insertions(+), 2 deletions(-) diff --git a/src/bpp/tests/test_authz_views.py b/src/bpp/tests/test_authz_views.py index 92a1d2b04..a08dfe8e5 100644 --- a/src/bpp/tests/test_authz_views.py +++ b/src/bpp/tests/test_authz_views.py @@ -64,16 +64,27 @@ def test_toz_anonim_przekierowuje(client): # --- API punktacji / habilitacji / jednostki ----------------------------- -API_ROUTES = [ +# Endpointy REDAKCYJNE: zalogowany bez uprawnień do wprowadzania danych → 403. +API_ROUTES_REDAKCYJNE = [ ("bpp:api_rok_habilitacji", {}), ("bpp:api_punktacja_zrodla", {"zrodlo_id": 1, "rok": 2020}), ("bpp:api_upload_punktacja_zrodla", {"zrodlo_id": 1, "rok": 2020}), +] + +# Wszystkie endpointy API wymagające ZALOGOWANIA (anonim → 302 na login). +# `api_ostatnia_jednostka_i_dyscyplina` jest tutaj, ale ŚWIADOMIE nie ma go na +# liście redakcyjnej wyżej: to widok tylko-do-odczytu, konsumowany przez +# `autorform_dependant.js` w PUBLICZNYM formularzu `zglos_publikacje`, więc +# zwykły zalogowany użytkownik musi dostać 200, a nie 403. Pilnuje tego +# `test_ostatnia_jednostka_dostepna_bez_uprawnien_redaktorskich` +# w `src/bpp/tests/test_views/test_api.py`. +API_ROUTES = API_ROUTES_REDAKCYJNE + [ ("bpp:api_ostatnia_jednostka_i_dyscyplina", {}), ] @pytest.mark.django_db -@pytest.mark.parametrize("name,kwargs", API_ROUTES) +@pytest.mark.parametrize("name,kwargs", API_ROUTES_REDAKCYJNE) def test_api_zwykly_user_403(client, zwykly_user, name, kwargs): client.force_login(zwykly_user) url = reverse(name, kwargs=kwargs)