From f0fa966ff0d1d2e1934e11ae8a7d2fa3793de18f Mon Sep 17 00:00:00 2001 From: Reto Tschuppert Date: Fri, 8 May 2026 10:03:17 -0400 Subject: [PATCH 01/26] OGC-3157: Replace assert current_user is not None with get_current_user() Adds OrgRequest.get_current_user() which captures a Sentry warning and raises HTTPForbidden when current_user is None despite a valid Redis session. Replaces all bare asserts and inline None checks across feriennet, org, town6 and translator_directory. Co-Authored-By: Claude Sonnet 4.6 --- src/onegov/feriennet/custom.py | 4 ++-- src/onegov/feriennet/forms/attendee.py | 4 ++-- src/onegov/feriennet/views/booking.py | 3 +-- src/onegov/feriennet/views/invoice.py | 10 +++++----- src/onegov/feriennet/views/occasion.py | 3 +-- src/onegov/org/custom.py | 3 ++- src/onegov/org/request.py | 19 +++++++++++++++++++ src/onegov/org/utils.py | 4 ++-- src/onegov/org/views/form_definition.py | 3 +-- .../org/views/form_registration_window.py | 3 +-- src/onegov/org/views/resource.py | 4 ++-- src/onegov/org/views/ticket.py | 4 ++-- src/onegov/town6/custom.py | 3 ++- src/onegov/translator_directory/custom.py | 3 ++- .../translator_directory/generate_docx.py | 6 +++--- .../translator_directory/views/time_report.py | 3 +-- 16 files changed, 48 insertions(+), 31 deletions(-) diff --git a/src/onegov/feriennet/custom.py b/src/onegov/feriennet/custom.py index 64db8c59d6..48c8fd2399 100644 --- a/src/onegov/feriennet/custom.py +++ b/src/onegov/feriennet/custom.py @@ -154,8 +154,8 @@ def get_personal_tools( if request.is_logged_in: session = request.session username = request.current_username - assert username is not None - assert request.current_user is not None + if username is None or request.current_user is None: + return period = request.app.active_period periods = request.app.periods diff --git a/src/onegov/feriennet/forms/attendee.py b/src/onegov/feriennet/forms/attendee.py index 94e117da48..252384fe7d 100644 --- a/src/onegov/feriennet/forms/attendee.py +++ b/src/onegov/feriennet/forms/attendee.py @@ -355,10 +355,10 @@ def populate_attendees(self) -> None: self.attendee.data = self.attendee.choices[0][0] def populate_tos(self) -> None: - assert self.request.current_user is not None + current_user = self.request.get_current_user() url = self.request.app.org.meta.get('tos_url', None) - if not url or self.request.current_user.data.get('tos_accepted', None): + if not url or current_user.data.get('tos_accepted', None): self.delete_field('accept_tos') return diff --git a/src/onegov/feriennet/views/booking.py b/src/onegov/feriennet/views/booking.py index 041b3967d1..662bf59f6e 100644 --- a/src/onegov/feriennet/views/booking.py +++ b/src/onegov/feriennet/views/booking.py @@ -383,8 +383,7 @@ def view_my_bookings( user = (request.session.query(User) .filter_by(username=self.username).one()) else: - assert request.current_user is not None - users, user = None, request.current_user + users, user = None, request.get_current_user() def subscribe_link(attendee: Attendee) -> str: url = request.link(AttendeeCalendar(self.session, attendee)) diff --git a/src/onegov/feriennet/views/invoice.py b/src/onegov/feriennet/views/invoice.py index 613bef7b86..ace431a5a3 100644 --- a/src/onegov/feriennet/views/invoice.py +++ b/src/onegov/feriennet/views/invoice.py @@ -115,8 +115,8 @@ def view_my_invoices( users = users_for_select_element(request) user = request.session.query(User).filter_by(id=self.user_id).one() - assert request.current_user is not None - if request.current_user.id == self.user_id: + current_user = request.get_current_user() + if current_user.id == self.user_id: title = _('Invoices') else: title = _('Invoices of ${user}', mapping={ @@ -258,10 +258,10 @@ def handle_donation( form: DonationForm ) -> RenderData | Response: - assert request.current_user is not None + current_user = request.get_current_user() if not self.user_id: return request.redirect(request.link( - self.for_user_id(request.current_user.id), + self.for_user_id(current_user.id), name='donation' )) @@ -272,7 +272,7 @@ def handle_donation( name='donation' )) - if request.current_user.id == self.user_id: + if current_user.id == self.user_id: title = _('Donation') else: user = request.session.query(User).filter_by(id=self.user_id).one() diff --git a/src/onegov/feriennet/views/occasion.py b/src/onegov/feriennet/views/occasion.py index 8af175c0eb..f7cef6b8d0 100644 --- a/src/onegov/feriennet/views/occasion.py +++ b/src/onegov/feriennet/views/occasion.py @@ -285,8 +285,7 @@ def book_occasion( # if the TOS have been accepted, record this now if hasattr(form, 'accept_tos') and form.accept_tos: if form.accept_tos.data: - assert request.current_user is not None - request.current_user.data['tos_accepted'] = True + request.get_current_user().data['tos_accepted'] = True # to get the final cost, we need to accept bookings without wishlist if self.period.confirmed: diff --git a/src/onegov/org/custom.py b/src/onegov/org/custom.py index 41d5c5affe..14ebd94b78 100644 --- a/src/onegov/org/custom.py +++ b/src/onegov/org/custom.py @@ -282,7 +282,8 @@ def get_global_tools( # Tickets if request.is_manager or request.is_supporter: - assert request.current_user is not None + if request.current_user is None: + return ticket_count = request.app.ticket_count screen_count = ticket_count.open or ticket_count.pending diff --git a/src/onegov/org/request.py b/src/onegov/org/request.py index 29b34d2375..43a1935e9d 100644 --- a/src/onegov/org/request.py +++ b/src/onegov/org/request.py @@ -1,5 +1,7 @@ from __future__ import annotations +import sentry_sdk + from functools import cached_property from onegov.core.custom import msgpack from onegov.core.orm import orm_cached @@ -12,6 +14,7 @@ from sedate import utcnow from sqlalchemy import inspect from sqlalchemy.orm import noload +from webob.exc import HTTPForbidden from typing import Any, NamedTuple, TYPE_CHECKING @@ -133,6 +136,22 @@ def current_user(self) -> User | None: self._current_user = self.session.merge(self._current_user) return self._current_user + def get_current_user(self) -> User: + """ Returns current_user, raising HTTPForbidden if unexpectedly None. + + Use this instead of asserting current_user is not None. Captures a + Sentry event with the username to aid debugging. + """ + user = self.current_user + if user is None: + sentry_sdk.capture_message( + 'current_user is None despite valid identity', + level='warning', + extras={'username': self.current_username}, + ) + raise HTTPForbidden() + return user + @cached_property def authenticated_email(self) -> str | None: """ diff --git a/src/onegov/org/utils.py b/src/onegov/org/utils.py index 7eb269ec84..770b2b4522 100644 --- a/src/onegov/org/utils.py +++ b/src/onegov/org/utils.py @@ -1957,8 +1957,8 @@ def can_change_username( if user.role not in request.app.settings.user.change_username_roles: return False - assert request.current_user is not None - second_factor = (request.current_user.second_factor or {}).get('type') + user = request.get_current_user() + second_factor = (user.second_factor or {}).get('type') # NOTE: For now we only allow second factors that don't require multiple # steps, so we can do it all in a single form if second_factor not in ('yubikey', 'totp'): diff --git a/src/onegov/org/views/form_definition.py b/src/onegov/org/views/form_definition.py index 7644cdb9ed..90c06fe25e 100644 --- a/src/onegov/org/views/form_definition.py +++ b/src/onegov/org/views/form_definition.py @@ -350,8 +350,7 @@ def handle_ticket(submission: FormSubmission) -> None: if ticket is False: raise exc.HTTPMethodNotAllowed() if ticket is not True: - assert request.current_user is not None - close_ticket(ticket, request.current_user, request) + close_ticket(ticket, request.get_current_user(), request) ticket.create_snapshot(request) def handle_submissions(submissions: Iterable[FormSubmission]) -> None: diff --git a/src/onegov/org/views/form_registration_window.py b/src/onegov/org/views/form_registration_window.py index 104fd15b4c..bd2f314eaf 100644 --- a/src/onegov/org/views/form_registration_window.py +++ b/src/onegov/org/views/form_registration_window.py @@ -419,8 +419,7 @@ def view_cancel_submissions_for_registration_window( no_messages=True, force_email=ticket.muted ) - assert request.current_user is not None - close_ticket(ticket, request.current_user, request) + close_ticket(ticket, request.get_current_user(), request) # same behaviour as when closing ticket normally # to disable mail on ticket close, there is a ticket-setting send_email_if_enabled( diff --git a/src/onegov/org/views/resource.py b/src/onegov/org/views/resource.py index 3129314217..aa19ff9d21 100644 --- a/src/onegov/org/views/resource.py +++ b/src/onegov/org/views/resource.py @@ -1117,7 +1117,7 @@ def handle_reservation_tickets( session: Session ) -> None: - assert request.current_user is not None + current_user = request.get_current_user() stmt = ( session.query(ReservationTicket) @@ -1140,7 +1140,7 @@ def handle_reservation_tickets( if not ticket: continue - close_ticket(ticket, request.current_user, request) + close_ticket(ticket, current_user, request) ticket.create_snapshot(request) # unlink payment from invoice items, delete invoice diff --git a/src/onegov/org/views/ticket.py b/src/onegov/org/views/ticket.py index cd910999d8..8b0c235cb2 100644 --- a/src/onegov/org/views/ticket.py +++ b/src/onegov/org/views/ticket.py @@ -1556,11 +1556,11 @@ def get_filters( request: OrgRequest ) -> Iterator[Link]: - assert request.current_user is not None + current_user = request.get_current_user() yield Link( text=_('My'), url=request.link( - self.for_state('unfinished').for_owner(request.current_user.id) + self.for_state('unfinished').for_owner(current_user.id) ), active=self.state == 'unfinished', attrs={'class': 'ticket-filter-my'} diff --git a/src/onegov/town6/custom.py b/src/onegov/town6/custom.py index 88caf0a16f..84fd90d5f7 100644 --- a/src/onegov/town6/custom.py +++ b/src/onegov/town6/custom.py @@ -25,7 +25,8 @@ def get_global_tools(request: TownRequest) -> Iterator[Link | LinkGroup]: if request.is_logged_in and request.app.org.meta.get( 'enable_chat', 'disabled') == 'people_chat': chat_staff = request.app.org.meta.get('chat_staff', []) - assert request.current_user is not None + if request.current_user is None: + return if request.current_user.id.hex in chat_staff: yield LinkGroup(_('Chats'), classes=('chats', ), links=( Link( diff --git a/src/onegov/translator_directory/custom.py b/src/onegov/translator_directory/custom.py index 87f48c672c..2932eb6224 100644 --- a/src/onegov/translator_directory/custom.py +++ b/src/onegov/translator_directory/custom.py @@ -147,7 +147,8 @@ def get_global_tools( # Tickets if request.is_admin or request.is_editor: - assert request.current_user is not None + if request.current_user is None: + return if request.is_accountant: ticket_count = get_accountant_ticket_count(request) else: diff --git a/src/onegov/translator_directory/generate_docx.py b/src/onegov/translator_directory/generate_docx.py index bf6558c9f7..8ac824d85a 100644 --- a/src/onegov/translator_directory/generate_docx.py +++ b/src/onegov/translator_directory/generate_docx.py @@ -187,9 +187,9 @@ def signature_for_mail_templates( uploaded. It should contain the string 'Unterschrift', as well as the first and last name of the user. """ - assert request.current_user is not None - assert request.current_user.realname is not None - first_name, last_name = request.current_user.realname.split(' ') + current_user = request.get_current_user() + assert current_user.realname is not None + first_name, last_name = current_user.realname.split(' ') query = GeneralFileCollection(request.session).query().filter( and_( GeneralFile.name.like('Unterschrift%'), diff --git a/src/onegov/translator_directory/views/time_report.py b/src/onegov/translator_directory/views/time_report.py index bbc443db18..5b1e1afe19 100644 --- a/src/onegov/translator_directory/views/time_report.py +++ b/src/onegov/translator_directory/views/time_report.py @@ -295,8 +295,7 @@ def accept_time_report( time_report.status = 'confirmed' handler.data['state'] = 'accepted' - assert request.current_user is not None - close_ticket(self, request.current_user, request) + close_ticket(self, request.get_current_user(), request) if translator and translator.email: pdf_bytes = generate_time_report_pdf_bytes( From 5218b0b290ec4415906063d8d9f2396c213de17e Mon Sep 17 00:00:00 2001 From: Reto Tschuppert Date: Fri, 8 May 2026 10:04:20 -0400 Subject: [PATCH 02/26] OGC-3157: Add unit tests for OrgRequest.get_current_user() Tests that get_current_user() returns the user when present, and raises HTTPForbidden while sending a Sentry warning when current_user is None. Co-Authored-By: Claude Sonnet 4.6 --- tests/onegov/org/test_request.py | 33 ++++++++++++++++++++++++++++++++ 1 file changed, 33 insertions(+) create mode 100644 tests/onegov/org/test_request.py diff --git a/tests/onegov/org/test_request.py b/tests/onegov/org/test_request.py new file mode 100644 index 0000000000..7b0a8aa42d --- /dev/null +++ b/tests/onegov/org/test_request.py @@ -0,0 +1,33 @@ +from __future__ import annotations + +from unittest.mock import MagicMock, patch +from webob.exc import HTTPForbidden + + +def test_get_current_user_returns_user() -> None: + user = MagicMock() + request = MagicMock() + request.current_user = user + + from onegov.org.request import OrgRequest + result = OrgRequest.get_current_user(request) + + assert result is user + + +def test_get_current_user_raises_when_none() -> None: + request = MagicMock() + request.current_user = None + request.current_username = 'stale@example.com' + + from onegov.org.request import OrgRequest + with patch('onegov.org.request.sentry_sdk') as mock_sentry: + import pytest + with pytest.raises(HTTPForbidden): + OrgRequest.get_current_user(request) + + mock_sentry.capture_message.assert_called_once_with( + 'current_user is None despite valid identity', + level='warning', + extras={'username': 'stale@example.com'}, + ) From 8943e52aeb1e0849d9a6b659ebd013810b48cbc5 Mon Sep 17 00:00:00 2001 From: Reto Tschuppert Date: Fri, 8 May 2026 10:19:04 -0400 Subject: [PATCH 03/26] OGC-3157: Fix invalid extras kwarg in sentry capture_message call Embed the username directly in the message string instead of using the non-existent extras= parameter. Co-Authored-By: Claude Sonnet 4.6 --- src/onegov/org/request.py | 4 ++-- tests/onegov/org/test_request.py | 4 ++-- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/src/onegov/org/request.py b/src/onegov/org/request.py index 43a1935e9d..2b80b0dc30 100644 --- a/src/onegov/org/request.py +++ b/src/onegov/org/request.py @@ -145,9 +145,9 @@ def get_current_user(self) -> User: user = self.current_user if user is None: sentry_sdk.capture_message( - 'current_user is None despite valid identity', + f'current_user is None despite valid identity' + f': {self.current_username}', level='warning', - extras={'username': self.current_username}, ) raise HTTPForbidden() return user diff --git a/tests/onegov/org/test_request.py b/tests/onegov/org/test_request.py index 7b0a8aa42d..809802bb86 100644 --- a/tests/onegov/org/test_request.py +++ b/tests/onegov/org/test_request.py @@ -27,7 +27,7 @@ def test_get_current_user_raises_when_none() -> None: OrgRequest.get_current_user(request) mock_sentry.capture_message.assert_called_once_with( - 'current_user is None despite valid identity', + 'current_user is None despite valid identity' + ': stale@example.com', level='warning', - extras={'username': 'stale@example.com'}, ) From 70be860d18c324814b9772fab79bdcf5e4a8370e Mon Sep 17 00:00:00 2001 From: Reto Tschuppert Date: Fri, 8 May 2026 10:20:00 -0400 Subject: [PATCH 04/26] OGC-3157: Only send Sentry warning when identity exists but user lookup fails Anonymous requests should raise HTTPForbidden silently; the warning is only meaningful when a valid session identity exists but current_user is unexpectedly None. Co-Authored-By: Claude Sonnet 4.6 --- src/onegov/org/request.py | 11 ++++++----- tests/onegov/org/test_request.py | 16 +++++++++++++++- 2 files changed, 21 insertions(+), 6 deletions(-) diff --git a/src/onegov/org/request.py b/src/onegov/org/request.py index 2b80b0dc30..0316671d49 100644 --- a/src/onegov/org/request.py +++ b/src/onegov/org/request.py @@ -144,11 +144,12 @@ def get_current_user(self) -> User: """ user = self.current_user if user is None: - sentry_sdk.capture_message( - f'current_user is None despite valid identity' - f': {self.current_username}', - level='warning', - ) + if self.identity is not None: + sentry_sdk.capture_message( + f'current_user is None despite valid identity' + f': {self.current_username}', + level='warning', + ) raise HTTPForbidden() return user diff --git a/tests/onegov/org/test_request.py b/tests/onegov/org/test_request.py index 809802bb86..a4ca25feb5 100644 --- a/tests/onegov/org/test_request.py +++ b/tests/onegov/org/test_request.py @@ -21,8 +21,8 @@ def test_get_current_user_raises_when_none() -> None: request.current_username = 'stale@example.com' from onegov.org.request import OrgRequest + import pytest with patch('onegov.org.request.sentry_sdk') as mock_sentry: - import pytest with pytest.raises(HTTPForbidden): OrgRequest.get_current_user(request) @@ -31,3 +31,17 @@ def test_get_current_user_raises_when_none() -> None: ': stale@example.com', level='warning', ) + + +def test_get_current_user_no_sentry_for_anonymous() -> None: + request = MagicMock() + request.current_user = None + request.identity = None + + from onegov.org.request import OrgRequest + import pytest + with patch('onegov.org.request.sentry_sdk') as mock_sentry: + with pytest.raises(HTTPForbidden): + OrgRequest.get_current_user(request) + + mock_sentry.capture_message.assert_not_called() From 8ceaf70225058983fae17ae050a5b4bf54c5dd47 Mon Sep 17 00:00:00 2001 From: Reto Tschuppert Date: Fri, 8 May 2026 10:21:25 -0400 Subject: [PATCH 05/26] OGC-3157: Fix premature return in get_global_tools skipping citizen-login links Replace early return guard with an inline condition so only the tickets block is skipped when current_user is None, not the rest of the generator. Co-Authored-By: Claude Sonnet 4.6 --- src/onegov/org/custom.py | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/src/onegov/org/custom.py b/src/onegov/org/custom.py index 14ebd94b78..57c060d93c 100644 --- a/src/onegov/org/custom.py +++ b/src/onegov/org/custom.py @@ -281,9 +281,8 @@ def get_global_tools( yield LinkGroup(_('Management'), classes=('management', ), links=links) # Tickets - if request.is_manager or request.is_supporter: - if request.current_user is None: - return + if (request.is_manager or request.is_supporter) \ + and request.current_user is not None: ticket_count = request.app.ticket_count screen_count = ticket_count.open or ticket_count.pending From 8cef8b84c5c99d2ee7c44b1fedd1ad485908c165 Mon Sep 17 00:00:00 2001 From: Reto Tschuppert Date: Fri, 8 May 2026 10:22:02 -0400 Subject: [PATCH 06/26] OGC-3157: Avoid shadowing user parameter in can_change_username Rename get_current_user() result to current_user to distinguish the requester from the user argument being inspected. Co-Authored-By: Claude Sonnet 4.6 --- src/onegov/org/utils.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/onegov/org/utils.py b/src/onegov/org/utils.py index 770b2b4522..0c94a1f8b7 100644 --- a/src/onegov/org/utils.py +++ b/src/onegov/org/utils.py @@ -1957,8 +1957,8 @@ def can_change_username( if user.role not in request.app.settings.user.change_username_roles: return False - user = request.get_current_user() - second_factor = (user.second_factor or {}).get('type') + current_user = request.get_current_user() + second_factor = (current_user.second_factor or {}).get('type') # NOTE: For now we only allow second factors that don't require multiple # steps, so we can do it all in a single form if second_factor not in ('yubikey', 'totp'): From 7685145dc22973a1b147b7bd14be213d9322c63f Mon Sep 17 00:00:00 2001 From: Reto Tschuppert Date: Fri, 8 May 2026 10:23:01 -0400 Subject: [PATCH 07/26] OGC-3157: Move pytest import to module scope in test_request.py Co-Authored-By: Claude Sonnet 4.6 --- tests/onegov/org/test_request.py | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/tests/onegov/org/test_request.py b/tests/onegov/org/test_request.py index a4ca25feb5..cc80788676 100644 --- a/tests/onegov/org/test_request.py +++ b/tests/onegov/org/test_request.py @@ -1,5 +1,6 @@ from __future__ import annotations +import pytest from unittest.mock import MagicMock, patch from webob.exc import HTTPForbidden @@ -21,7 +22,6 @@ def test_get_current_user_raises_when_none() -> None: request.current_username = 'stale@example.com' from onegov.org.request import OrgRequest - import pytest with patch('onegov.org.request.sentry_sdk') as mock_sentry: with pytest.raises(HTTPForbidden): OrgRequest.get_current_user(request) @@ -39,7 +39,6 @@ def test_get_current_user_no_sentry_for_anonymous() -> None: request.identity = None from onegov.org.request import OrgRequest - import pytest with patch('onegov.org.request.sentry_sdk') as mock_sentry: with pytest.raises(HTTPForbidden): OrgRequest.get_current_user(request) From 172250effdd99fbad423f19fa9e8d1ce9aa2afa1 Mon Sep 17 00:00:00 2001 From: Reto Tschuppert Date: Fri, 8 May 2026 10:57:06 -0400 Subject: [PATCH 08/26] Org: Fix Sentry warning firing for anonymous requests in get_current_user NO_IDENTITY is not None, so the previous guard would trigger a spurious Sentry warning for unauthenticated requests. Use is_logged_in instead, which correctly checks identity is not NO_IDENTITY. Co-Authored-By: Claude Sonnet 4.6 --- src/onegov/org/request.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/onegov/org/request.py b/src/onegov/org/request.py index 0316671d49..d1fe5765e7 100644 --- a/src/onegov/org/request.py +++ b/src/onegov/org/request.py @@ -144,7 +144,7 @@ def get_current_user(self) -> User: """ user = self.current_user if user is None: - if self.identity is not None: + if self.is_logged_in: sentry_sdk.capture_message( f'current_user is None despite valid identity' f': {self.current_username}', From f7836b941016c6f5f1d24950d1cb354e79598691 Mon Sep 17 00:00:00 2001 From: Reto Tschuppert Date: Fri, 8 May 2026 10:58:52 -0400 Subject: [PATCH 09/26] Org: Fix test fixtures to use is_logged_in instead of identity MagicMock auto-creates truthy attributes, so setting identity=None had no effect on the is_logged_in check. Explicitly set is_logged_in=True/False to reflect the actual condition the code under test checks. Co-Authored-By: Claude Sonnet 4.6 --- tests/onegov/org/test_request.py | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/tests/onegov/org/test_request.py b/tests/onegov/org/test_request.py index cc80788676..644370df18 100644 --- a/tests/onegov/org/test_request.py +++ b/tests/onegov/org/test_request.py @@ -19,6 +19,7 @@ def test_get_current_user_returns_user() -> None: def test_get_current_user_raises_when_none() -> None: request = MagicMock() request.current_user = None + request.is_logged_in = True request.current_username = 'stale@example.com' from onegov.org.request import OrgRequest @@ -36,7 +37,7 @@ def test_get_current_user_raises_when_none() -> None: def test_get_current_user_no_sentry_for_anonymous() -> None: request = MagicMock() request.current_user = None - request.identity = None + request.is_logged_in = False from onegov.org.request import OrgRequest with patch('onegov.org.request.sentry_sdk') as mock_sentry: From ba25bc14a90723bed94b8ea712b6a71e4c6265db Mon Sep 17 00:00:00 2001 From: Reto Tschuppert Date: Fri, 8 May 2026 11:00:42 -0400 Subject: [PATCH 10/26] Town6: Use get_current_user() in chat nav to report stale sessions Replacing the silent early return with get_current_user() ensures a stale-session case (is_logged_in True but current_user None) is reported to Sentry and handled consistently with the rest of the codebase. Co-Authored-By: Claude Sonnet 4.6 --- src/onegov/town6/custom.py | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/src/onegov/town6/custom.py b/src/onegov/town6/custom.py index 84fd90d5f7..c963c8c588 100644 --- a/src/onegov/town6/custom.py +++ b/src/onegov/town6/custom.py @@ -25,9 +25,8 @@ def get_global_tools(request: TownRequest) -> Iterator[Link | LinkGroup]: if request.is_logged_in and request.app.org.meta.get( 'enable_chat', 'disabled') == 'people_chat': chat_staff = request.app.org.meta.get('chat_staff', []) - if request.current_user is None: - return - if request.current_user.id.hex in chat_staff: + user = request.get_current_user() + if user.id.hex in chat_staff: yield LinkGroup(_('Chats'), classes=('chats', ), links=( Link( _('My Chats'), request.link( From 7612350b4fbd7ca8402691a8f6014ff1f3cebad0 Mon Sep 17 00:00:00 2001 From: Reto Tschuppert Date: Fri, 8 May 2026 11:01:52 -0400 Subject: [PATCH 11/26] Feriennet: Use get_current_user() in personal tools nav for stale sessions Same fix as town6: replace the silent early return with get_current_user() so a stale-session case is reported to Sentry and handled consistently. Co-Authored-By: Claude Sonnet 4.6 --- src/onegov/feriennet/custom.py | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-) diff --git a/src/onegov/feriennet/custom.py b/src/onegov/feriennet/custom.py index 48c8fd2399..26954ec714 100644 --- a/src/onegov/feriennet/custom.py +++ b/src/onegov/feriennet/custom.py @@ -153,16 +153,15 @@ def get_personal_tools( # for logged-in users show the number of open bookings if request.is_logged_in: session = request.session - username = request.current_username - if username is None or request.current_user is None: - return + user = request.get_current_user() + username = user.username period = request.app.active_period periods = request.app.periods if not period or period.finalizable: invoices = request.app.invoice_collection( - user_id=request.current_user.id) + user_id=user.id) unpaid = invoices.unpaid_count(excluded_period_ids={ p.id for p in periods if not p.finalized}) From 506ea531f891efbebbd2cb97f13b0e5ee6dbbaaa Mon Sep 17 00:00:00 2001 From: Reto Tschuppert Date: Fri, 8 May 2026 11:02:29 -0400 Subject: [PATCH 12/26] Org: Use get_current_user() in ticket nav to surface stale sessions Replace the silent current_user is not None guard with get_current_user() so stale-session inconsistencies are reported to Sentry consistently. Co-Authored-By: Claude Sonnet 4.6 --- src/onegov/org/custom.py | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/onegov/org/custom.py b/src/onegov/org/custom.py index 57c060d93c..4188aaf165 100644 --- a/src/onegov/org/custom.py +++ b/src/onegov/org/custom.py @@ -281,8 +281,8 @@ def get_global_tools( yield LinkGroup(_('Management'), classes=('management', ), links=links) # Tickets - if (request.is_manager or request.is_supporter) \ - and request.current_user is not None: + if request.is_manager or request.is_supporter: + user = request.get_current_user() ticket_count = request.app.ticket_count screen_count = ticket_count.open or ticket_count.pending @@ -295,7 +295,7 @@ def get_global_tools( TicketCollection, { 'handler': 'ALL', 'state': 'unfinished', - 'owner': request.current_user.id.hex + 'owner': user.id.hex }, ), attrs={ From b78ebbee16c2ab3afc8953e5a30aaf2167af1bb1 Mon Sep 17 00:00:00 2001 From: Reto Tschuppert Date: Fri, 8 May 2026 11:16:29 -0400 Subject: [PATCH 13/26] Org: Clarify get_current_user docstring The method raises HTTPForbidden for any None current_user, not just the stale-session case. Clarify that anonymous requests are also forbidden, and that Sentry is only notified for the logged-in-but-missing-user case. Co-Authored-By: Claude Sonnet 4.6 --- src/onegov/org/request.py | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/src/onegov/org/request.py b/src/onegov/org/request.py index d1fe5765e7..dd942f7d21 100644 --- a/src/onegov/org/request.py +++ b/src/onegov/org/request.py @@ -137,10 +137,12 @@ def current_user(self) -> User | None: return self._current_user def get_current_user(self) -> User: - """ Returns current_user, raising HTTPForbidden if unexpectedly None. + """ Returns current_user, raising HTTPForbidden if it is None. - Use this instead of asserting current_user is not None. Captures a - Sentry event with the username to aid debugging. + Use this instead of asserting current_user is not None. Raises for + both anonymous requests and cases where a logged-in identity has no + corresponding user record. In the latter case a Sentry warning with + the username is captured to aid debugging. """ user = self.current_user if user is None: From a4f2bd7fd27c07b655c0a5bb801dcf418821e0cb Mon Sep 17 00:00:00 2001 From: Reto Tschuppert Date: Fri, 8 May 2026 11:20:23 -0400 Subject: [PATCH 14/26] Org: Use identity uid instead of username in stale-session Sentry warning current_username is an email address (PII) and was sent unconditionally, bypassing the should_send_default_pii() gate used elsewhere in the Sentry integration. Switch to identity.uid which is the non-PII Sentry user ID already attached to every event via the event processor. Co-Authored-By: Claude Sonnet 4.6 --- src/onegov/org/request.py | 5 +++-- tests/onegov/org/test_request.py | 4 ++-- 2 files changed, 5 insertions(+), 4 deletions(-) diff --git a/src/onegov/org/request.py b/src/onegov/org/request.py index dd942f7d21..e1df9488d0 100644 --- a/src/onegov/org/request.py +++ b/src/onegov/org/request.py @@ -142,14 +142,15 @@ def get_current_user(self) -> User: Use this instead of asserting current_user is not None. Raises for both anonymous requests and cases where a logged-in identity has no corresponding user record. In the latter case a Sentry warning with - the username is captured to aid debugging. + the identity uid is captured to aid debugging. """ user = self.current_user if user is None: if self.is_logged_in: + uid = getattr(self.identity, 'uid', 'unknown') sentry_sdk.capture_message( f'current_user is None despite valid identity' - f': {self.current_username}', + f': {uid}', level='warning', ) raise HTTPForbidden() diff --git a/tests/onegov/org/test_request.py b/tests/onegov/org/test_request.py index 644370df18..628ec9c9d7 100644 --- a/tests/onegov/org/test_request.py +++ b/tests/onegov/org/test_request.py @@ -20,7 +20,7 @@ def test_get_current_user_raises_when_none() -> None: request = MagicMock() request.current_user = None request.is_logged_in = True - request.current_username = 'stale@example.com' + request.identity.uid = 'abc123' from onegov.org.request import OrgRequest with patch('onegov.org.request.sentry_sdk') as mock_sentry: @@ -29,7 +29,7 @@ def test_get_current_user_raises_when_none() -> None: mock_sentry.capture_message.assert_called_once_with( 'current_user is None despite valid identity' - ': stale@example.com', + ': abc123', level='warning', ) From 0ebc6abba9489f968a238fb42749eec842b3da78 Mon Sep 17 00:00:00 2001 From: Reto Tschuppert Date: Fri, 8 May 2026 11:20:46 -0400 Subject: [PATCH 15/26] Translators: Fix assert and split crash in signature_for_mail_templates Asserts can be optimised away with -O and split(' ') would crash on names with more than one space. Use an explicit guard and split(' ', 1) with a length check instead, returning None for missing or malformed names. Co-Authored-By: Claude Sonnet 4.6 --- src/onegov/translator_directory/generate_docx.py | 9 +++++++-- 1 file changed, 7 insertions(+), 2 deletions(-) diff --git a/src/onegov/translator_directory/generate_docx.py b/src/onegov/translator_directory/generate_docx.py index 8ac824d85a..d6083279bb 100644 --- a/src/onegov/translator_directory/generate_docx.py +++ b/src/onegov/translator_directory/generate_docx.py @@ -188,8 +188,13 @@ def signature_for_mail_templates( first and last name of the user. """ current_user = request.get_current_user() - assert current_user.realname is not None - first_name, last_name = current_user.realname.split(' ') + realname = current_user.realname + if not realname: + return None + name_parts = realname.split(' ', 1) + if len(name_parts) != 2: + return None + first_name, last_name = name_parts query = GeneralFileCollection(request.session).query().filter( and_( GeneralFile.name.like('Unterschrift%'), From f3172afaf48c828a2927730d1ec63033ff66cd75 Mon Sep 17 00:00:00 2001 From: Reto Tschuppert Date: Fri, 8 May 2026 11:23:07 -0400 Subject: [PATCH 16/26] Translators/Agency: Use get_current_user() to surface stale sessions Replace the silent current_user None guard in translator_directory/custom.py with get_current_user(), consistent with the other nav fixes. Also add get_current_user() to AgencyApp's DummyRequest test fixture. Co-Authored-By: Claude Sonnet 4.6 --- src/onegov/translator_directory/custom.py | 5 ++--- tests/onegov/agency/test_app.py | 3 +++ 2 files changed, 5 insertions(+), 3 deletions(-) diff --git a/src/onegov/translator_directory/custom.py b/src/onegov/translator_directory/custom.py index 2932eb6224..90571917b8 100644 --- a/src/onegov/translator_directory/custom.py +++ b/src/onegov/translator_directory/custom.py @@ -147,8 +147,7 @@ def get_global_tools( # Tickets if request.is_admin or request.is_editor: - if request.current_user is None: - return + user = request.get_current_user() if request.is_accountant: ticket_count = get_accountant_ticket_count(request) else: @@ -170,7 +169,7 @@ def get_global_tools( TicketCollection, { 'handler': 'ALL', 'state': 'unfinished', - 'owner': request.current_user.id.hex + 'owner': user.id.hex }, ), attrs={ diff --git a/tests/onegov/agency/test_app.py b/tests/onegov/agency/test_app.py index cf703d7195..ce8da07a0d 100644 --- a/tests/onegov/agency/test_app.py +++ b/tests/onegov/agency/test_app.py @@ -52,6 +52,9 @@ def transform(self, url: str) -> str: def include(self, asset: object) -> None: pass + def get_current_user(self) -> object: + return self.current_user + def exclude_invisible(self, items: object) -> None: pass From c46b1e33f1403ec47d13d7254d4c19e891d05c51 Mon Sep 17 00:00:00 2001 From: Reto Tschuppert Date: Fri, 8 May 2026 11:39:13 -0400 Subject: [PATCH 17/26] Translators: Normalize realname before splitting in signature lookup strip() removes leading/trailing whitespace, split(None, 1) handles multiple spaces between first and last name, and the non-empty check on both parts prevents overly broad LIKE '%%' queries. Co-Authored-By: Claude Sonnet 4.6 --- src/onegov/translator_directory/generate_docx.py | 7 ++----- 1 file changed, 2 insertions(+), 5 deletions(-) diff --git a/src/onegov/translator_directory/generate_docx.py b/src/onegov/translator_directory/generate_docx.py index d6083279bb..134a0be497 100644 --- a/src/onegov/translator_directory/generate_docx.py +++ b/src/onegov/translator_directory/generate_docx.py @@ -188,11 +188,8 @@ def signature_for_mail_templates( first and last name of the user. """ current_user = request.get_current_user() - realname = current_user.realname - if not realname: - return None - name_parts = realname.split(' ', 1) - if len(name_parts) != 2: + name_parts = (current_user.realname or '').strip().split(None, 1) + if len(name_parts) != 2 or not name_parts[0] or not name_parts[1]: return None first_name, last_name = name_parts query = GeneralFileCollection(request.session).query().filter( From 8756ae97498faec87326da94e01b21c9088ec3b2 Mon Sep 17 00:00:00 2001 From: Reto Tschuppert Date: Tue, 19 May 2026 07:09:27 -0400 Subject: [PATCH 18/26] Revert the broad use of get_current_user --- src/onegov/feriennet/custom.py | 7 ++-- src/onegov/feriennet/forms/attendee.py | 4 +-- src/onegov/feriennet/views/booking.py | 3 +- src/onegov/feriennet/views/invoice.py | 10 +++--- src/onegov/feriennet/views/occasion.py | 3 +- src/onegov/org/custom.py | 34 +++++++++++++++++-- src/onegov/org/request.py | 20 ----------- src/onegov/org/utils.py | 4 +-- src/onegov/org/views/form_definition.py | 3 +- .../org/views/form_registration_window.py | 3 +- src/onegov/org/views/resource.py | 4 +-- src/onegov/org/views/ticket.py | 4 +-- src/onegov/town6/custom.py | 21 ++++++++++-- src/onegov/translator_directory/custom.py | 4 +-- .../translator_directory/generate_docx.py | 4 +-- .../translator_directory/views/time_report.py | 3 +- tests/onegov/agency/test_app.py | 3 -- tests/onegov/org/test_request.py | 31 ----------------- 18 files changed, 80 insertions(+), 85 deletions(-) diff --git a/src/onegov/feriennet/custom.py b/src/onegov/feriennet/custom.py index 26954ec714..64db8c59d6 100644 --- a/src/onegov/feriennet/custom.py +++ b/src/onegov/feriennet/custom.py @@ -153,15 +153,16 @@ def get_personal_tools( # for logged-in users show the number of open bookings if request.is_logged_in: session = request.session - user = request.get_current_user() - username = user.username + username = request.current_username + assert username is not None + assert request.current_user is not None period = request.app.active_period periods = request.app.periods if not period or period.finalizable: invoices = request.app.invoice_collection( - user_id=user.id) + user_id=request.current_user.id) unpaid = invoices.unpaid_count(excluded_period_ids={ p.id for p in periods if not p.finalized}) diff --git a/src/onegov/feriennet/forms/attendee.py b/src/onegov/feriennet/forms/attendee.py index 252384fe7d..94e117da48 100644 --- a/src/onegov/feriennet/forms/attendee.py +++ b/src/onegov/feriennet/forms/attendee.py @@ -355,10 +355,10 @@ def populate_attendees(self) -> None: self.attendee.data = self.attendee.choices[0][0] def populate_tos(self) -> None: - current_user = self.request.get_current_user() + assert self.request.current_user is not None url = self.request.app.org.meta.get('tos_url', None) - if not url or current_user.data.get('tos_accepted', None): + if not url or self.request.current_user.data.get('tos_accepted', None): self.delete_field('accept_tos') return diff --git a/src/onegov/feriennet/views/booking.py b/src/onegov/feriennet/views/booking.py index 662bf59f6e..041b3967d1 100644 --- a/src/onegov/feriennet/views/booking.py +++ b/src/onegov/feriennet/views/booking.py @@ -383,7 +383,8 @@ def view_my_bookings( user = (request.session.query(User) .filter_by(username=self.username).one()) else: - users, user = None, request.get_current_user() + assert request.current_user is not None + users, user = None, request.current_user def subscribe_link(attendee: Attendee) -> str: url = request.link(AttendeeCalendar(self.session, attendee)) diff --git a/src/onegov/feriennet/views/invoice.py b/src/onegov/feriennet/views/invoice.py index ace431a5a3..613bef7b86 100644 --- a/src/onegov/feriennet/views/invoice.py +++ b/src/onegov/feriennet/views/invoice.py @@ -115,8 +115,8 @@ def view_my_invoices( users = users_for_select_element(request) user = request.session.query(User).filter_by(id=self.user_id).one() - current_user = request.get_current_user() - if current_user.id == self.user_id: + assert request.current_user is not None + if request.current_user.id == self.user_id: title = _('Invoices') else: title = _('Invoices of ${user}', mapping={ @@ -258,10 +258,10 @@ def handle_donation( form: DonationForm ) -> RenderData | Response: - current_user = request.get_current_user() + assert request.current_user is not None if not self.user_id: return request.redirect(request.link( - self.for_user_id(current_user.id), + self.for_user_id(request.current_user.id), name='donation' )) @@ -272,7 +272,7 @@ def handle_donation( name='donation' )) - if current_user.id == self.user_id: + if request.current_user.id == self.user_id: title = _('Donation') else: user = request.session.query(User).filter_by(id=self.user_id).one() diff --git a/src/onegov/feriennet/views/occasion.py b/src/onegov/feriennet/views/occasion.py index f7cef6b8d0..8af175c0eb 100644 --- a/src/onegov/feriennet/views/occasion.py +++ b/src/onegov/feriennet/views/occasion.py @@ -285,7 +285,8 @@ def book_occasion( # if the TOS have been accepted, record this now if hasattr(form, 'accept_tos') and form.accept_tos: if form.accept_tos.data: - request.get_current_user().data['tos_accepted'] = True + assert request.current_user is not None + request.current_user.data['tos_accepted'] = True # to get the final cost, we need to accept bookings without wishlist if self.period.confirmed: diff --git a/src/onegov/org/custom.py b/src/onegov/org/custom.py index 4188aaf165..09da37431a 100644 --- a/src/onegov/org/custom.py +++ b/src/onegov/org/custom.py @@ -7,6 +7,10 @@ from onegov.form.collection import FormCollection, SurveyCollection from onegov.newsletter import NewsletterCollection from onegov.org import _, OrgApp +from onegov.org.api import ( + DirectoryEntryApiEndpoint, EventApiEndpoint, FormApiEndpoint, + NewsApiEndpoint, PersonApiEndpoint, ResourceApiEndpoint, + TopicApiEndpoint) from onegov.org.models import ( CitizenDashboard, Dashboard, @@ -14,6 +18,7 @@ ImageFileCollection, Organisation, ) +from onegov.org.models.directory import ExtendedDirectory from onegov.org.models.page import News from onegov.pay import PaymentCollection from onegov.people import PersonCollection @@ -26,7 +31,8 @@ from typing import Any, TYPE_CHECKING if TYPE_CHECKING: - from collections.abc import Iterator + from collections.abc import Callable, Iterator + from onegov.org.api import ApiEndpoint from onegov.org.request import OrgRequest @@ -38,6 +44,12 @@ def get_template_variables(request: OrgRequest) -> dict[str, Any]: } +@OrgApp.setting(section='api', name='endpoints') +def get_api_endpoints_handler( +) -> Callable[[OrgRequest], Iterator[ApiEndpoint[Any]]]: + return get_api_endpoints + + def get_modules(request: OrgRequest) -> LinkGroup: links = [] if request.is_admin: @@ -282,7 +294,7 @@ def get_global_tools( # Tickets if request.is_manager or request.is_supporter: - user = request.get_current_user() + assert request.current_user is not None ticket_count = request.app.ticket_count screen_count = ticket_count.open or ticket_count.pending @@ -295,7 +307,7 @@ def get_global_tools( TicketCollection, { 'handler': 'ALL', 'state': 'unfinished', - 'owner': user.id.hex + 'owner': request.current_user.id.hex }, ), attrs={ @@ -409,3 +421,19 @@ def get_global_tools( 'class': ('citizen-tickets'), } ) + + +def get_api_endpoints(request: OrgRequest) -> Iterator[ApiEndpoint[Any]]: + yield EventApiEndpoint(request) + yield FormApiEndpoint(request) + yield NewsApiEndpoint(request) + yield PersonApiEndpoint(request) + yield ResourceApiEndpoint(request) + yield TopicApiEndpoint(request) + directories = request.exclude_invisible( + request.session.query(ExtendedDirectory)) + for directory in directories: + yield DirectoryEntryApiEndpoint( + request=request, + name=directory.name + ) diff --git a/src/onegov/org/request.py b/src/onegov/org/request.py index e1df9488d0..7263d5bb2a 100644 --- a/src/onegov/org/request.py +++ b/src/onegov/org/request.py @@ -136,26 +136,6 @@ def current_user(self) -> User | None: self._current_user = self.session.merge(self._current_user) return self._current_user - def get_current_user(self) -> User: - """ Returns current_user, raising HTTPForbidden if it is None. - - Use this instead of asserting current_user is not None. Raises for - both anonymous requests and cases where a logged-in identity has no - corresponding user record. In the latter case a Sentry warning with - the identity uid is captured to aid debugging. - """ - user = self.current_user - if user is None: - if self.is_logged_in: - uid = getattr(self.identity, 'uid', 'unknown') - sentry_sdk.capture_message( - f'current_user is None despite valid identity' - f': {uid}', - level='warning', - ) - raise HTTPForbidden() - return user - @cached_property def authenticated_email(self) -> str | None: """ diff --git a/src/onegov/org/utils.py b/src/onegov/org/utils.py index 0c94a1f8b7..7eb269ec84 100644 --- a/src/onegov/org/utils.py +++ b/src/onegov/org/utils.py @@ -1957,8 +1957,8 @@ def can_change_username( if user.role not in request.app.settings.user.change_username_roles: return False - current_user = request.get_current_user() - second_factor = (current_user.second_factor or {}).get('type') + assert request.current_user is not None + second_factor = (request.current_user.second_factor or {}).get('type') # NOTE: For now we only allow second factors that don't require multiple # steps, so we can do it all in a single form if second_factor not in ('yubikey', 'totp'): diff --git a/src/onegov/org/views/form_definition.py b/src/onegov/org/views/form_definition.py index 90c06fe25e..7644cdb9ed 100644 --- a/src/onegov/org/views/form_definition.py +++ b/src/onegov/org/views/form_definition.py @@ -350,7 +350,8 @@ def handle_ticket(submission: FormSubmission) -> None: if ticket is False: raise exc.HTTPMethodNotAllowed() if ticket is not True: - close_ticket(ticket, request.get_current_user(), request) + assert request.current_user is not None + close_ticket(ticket, request.current_user, request) ticket.create_snapshot(request) def handle_submissions(submissions: Iterable[FormSubmission]) -> None: diff --git a/src/onegov/org/views/form_registration_window.py b/src/onegov/org/views/form_registration_window.py index bd2f314eaf..104fd15b4c 100644 --- a/src/onegov/org/views/form_registration_window.py +++ b/src/onegov/org/views/form_registration_window.py @@ -419,7 +419,8 @@ def view_cancel_submissions_for_registration_window( no_messages=True, force_email=ticket.muted ) - close_ticket(ticket, request.get_current_user(), request) + assert request.current_user is not None + close_ticket(ticket, request.current_user, request) # same behaviour as when closing ticket normally # to disable mail on ticket close, there is a ticket-setting send_email_if_enabled( diff --git a/src/onegov/org/views/resource.py b/src/onegov/org/views/resource.py index aa19ff9d21..3129314217 100644 --- a/src/onegov/org/views/resource.py +++ b/src/onegov/org/views/resource.py @@ -1117,7 +1117,7 @@ def handle_reservation_tickets( session: Session ) -> None: - current_user = request.get_current_user() + assert request.current_user is not None stmt = ( session.query(ReservationTicket) @@ -1140,7 +1140,7 @@ def handle_reservation_tickets( if not ticket: continue - close_ticket(ticket, current_user, request) + close_ticket(ticket, request.current_user, request) ticket.create_snapshot(request) # unlink payment from invoice items, delete invoice diff --git a/src/onegov/org/views/ticket.py b/src/onegov/org/views/ticket.py index 8b0c235cb2..cd910999d8 100644 --- a/src/onegov/org/views/ticket.py +++ b/src/onegov/org/views/ticket.py @@ -1556,11 +1556,11 @@ def get_filters( request: OrgRequest ) -> Iterator[Link]: - current_user = request.get_current_user() + assert request.current_user is not None yield Link( text=_('My'), url=request.link( - self.for_state('unfinished').for_owner(current_user.id) + self.for_state('unfinished').for_owner(request.current_user.id) ), active=self.state == 'unfinished', attrs={'class': 'ticket-filter-my'} diff --git a/src/onegov/town6/custom.py b/src/onegov/town6/custom.py index c963c8c588..92b0cd55aa 100644 --- a/src/onegov/town6/custom.py +++ b/src/onegov/town6/custom.py @@ -2,14 +2,19 @@ from onegov.chat.collections import ChatCollection from onegov.core.elements import Link, LinkGroup +from onegov.org.custom import get_api_endpoints as get_api_endpoints_base from onegov.org.custom import get_global_tools as get_global_tools_base from onegov.org.custom import get_modules as get_modules_base from onegov.town6 import _ +from onegov.town6.api import ( + CommissionApiEndpoint, MeetingApiEndpoint, ParliamentarianApiEndpoint, + ParliamentaryGroupApiEndpoint, PoliticalBusinessApiEndpoint) -from typing import TYPE_CHECKING +from typing import Any, TYPE_CHECKING if TYPE_CHECKING: from collections.abc import Iterator + from onegov.api.models import ApiEndpoint from onegov.town6.request import TownRequest @@ -25,8 +30,8 @@ def get_global_tools(request: TownRequest) -> Iterator[Link | LinkGroup]: if request.is_logged_in and request.app.org.meta.get( 'enable_chat', 'disabled') == 'people_chat': chat_staff = request.app.org.meta.get('chat_staff', []) - user = request.get_current_user() - if user.id.hex in chat_staff: + assert request.current_user is not None + if request.current_user.id.hex in chat_staff: yield LinkGroup(_('Chats'), classes=('chats', ), links=( Link( _('My Chats'), request.link( @@ -61,3 +66,13 @@ def get_modules(request: TownRequest) -> LinkGroup: ) modules.links = links return modules + + +def get_api_endpoints(request: TownRequest) -> Iterator[ApiEndpoint[Any]]: + yield from get_api_endpoints_base(request) + if request.app.org.ris_enabled: + yield CommissionApiEndpoint(request) + yield MeetingApiEndpoint(request) + yield PoliticalBusinessApiEndpoint(request) + yield ParliamentarianApiEndpoint(request) + yield ParliamentaryGroupApiEndpoint(request) diff --git a/src/onegov/translator_directory/custom.py b/src/onegov/translator_directory/custom.py index 90571917b8..87f48c672c 100644 --- a/src/onegov/translator_directory/custom.py +++ b/src/onegov/translator_directory/custom.py @@ -147,7 +147,7 @@ def get_global_tools( # Tickets if request.is_admin or request.is_editor: - user = request.get_current_user() + assert request.current_user is not None if request.is_accountant: ticket_count = get_accountant_ticket_count(request) else: @@ -169,7 +169,7 @@ def get_global_tools( TicketCollection, { 'handler': 'ALL', 'state': 'unfinished', - 'owner': user.id.hex + 'owner': request.current_user.id.hex }, ), attrs={ diff --git a/src/onegov/translator_directory/generate_docx.py b/src/onegov/translator_directory/generate_docx.py index 134a0be497..e0c974c712 100644 --- a/src/onegov/translator_directory/generate_docx.py +++ b/src/onegov/translator_directory/generate_docx.py @@ -187,8 +187,8 @@ def signature_for_mail_templates( uploaded. It should contain the string 'Unterschrift', as well as the first and last name of the user. """ - current_user = request.get_current_user() - name_parts = (current_user.realname or '').strip().split(None, 1) + assert request.current_user is not None + name_parts = (request.current_user.realname or '').strip().split(None, 1) if len(name_parts) != 2 or not name_parts[0] or not name_parts[1]: return None first_name, last_name = name_parts diff --git a/src/onegov/translator_directory/views/time_report.py b/src/onegov/translator_directory/views/time_report.py index 5b1e1afe19..bbc443db18 100644 --- a/src/onegov/translator_directory/views/time_report.py +++ b/src/onegov/translator_directory/views/time_report.py @@ -295,7 +295,8 @@ def accept_time_report( time_report.status = 'confirmed' handler.data['state'] = 'accepted' - close_ticket(self, request.get_current_user(), request) + assert request.current_user is not None + close_ticket(self, request.current_user, request) if translator and translator.email: pdf_bytes = generate_time_report_pdf_bytes( diff --git a/tests/onegov/agency/test_app.py b/tests/onegov/agency/test_app.py index ce8da07a0d..cf703d7195 100644 --- a/tests/onegov/agency/test_app.py +++ b/tests/onegov/agency/test_app.py @@ -52,9 +52,6 @@ def transform(self, url: str) -> str: def include(self, asset: object) -> None: pass - def get_current_user(self) -> object: - return self.current_user - def exclude_invisible(self, items: object) -> None: pass diff --git a/tests/onegov/org/test_request.py b/tests/onegov/org/test_request.py index 628ec9c9d7..55c2eb2e14 100644 --- a/tests/onegov/org/test_request.py +++ b/tests/onegov/org/test_request.py @@ -14,34 +14,3 @@ def test_get_current_user_returns_user() -> None: result = OrgRequest.get_current_user(request) assert result is user - - -def test_get_current_user_raises_when_none() -> None: - request = MagicMock() - request.current_user = None - request.is_logged_in = True - request.identity.uid = 'abc123' - - from onegov.org.request import OrgRequest - with patch('onegov.org.request.sentry_sdk') as mock_sentry: - with pytest.raises(HTTPForbidden): - OrgRequest.get_current_user(request) - - mock_sentry.capture_message.assert_called_once_with( - 'current_user is None despite valid identity' - ': abc123', - level='warning', - ) - - -def test_get_current_user_no_sentry_for_anonymous() -> None: - request = MagicMock() - request.current_user = None - request.is_logged_in = False - - from onegov.org.request import OrgRequest - with patch('onegov.org.request.sentry_sdk') as mock_sentry: - with pytest.raises(HTTPForbidden): - OrgRequest.get_current_user(request) - - mock_sentry.capture_message.assert_not_called() From f1cc9b669752d081ef2f109a1c545be5b38ec00a Mon Sep 17 00:00:00 2001 From: Reto Tschuppert Date: Tue, 19 May 2026 13:07:52 -0400 Subject: [PATCH 19/26] Delete not needed file --- tests/onegov/org/test_request.py | 16 ---------------- 1 file changed, 16 deletions(-) delete mode 100644 tests/onegov/org/test_request.py diff --git a/tests/onegov/org/test_request.py b/tests/onegov/org/test_request.py deleted file mode 100644 index 55c2eb2e14..0000000000 --- a/tests/onegov/org/test_request.py +++ /dev/null @@ -1,16 +0,0 @@ -from __future__ import annotations - -import pytest -from unittest.mock import MagicMock, patch -from webob.exc import HTTPForbidden - - -def test_get_current_user_returns_user() -> None: - user = MagicMock() - request = MagicMock() - request.current_user = user - - from onegov.org.request import OrgRequest - result = OrgRequest.get_current_user(request) - - assert result is user From 26957b65da75a2f05400a8f3375ffa013ea5e299 Mon Sep 17 00:00:00 2001 From: Reto Tschuppert Date: Fri, 22 May 2026 16:22:35 +0200 Subject: [PATCH 20/26] Call change_username to ensure logout from all sessions --- src/onegov/core/security/identity_policy.py | 5 +++ src/onegov/org/forms/user.py | 3 +- src/onegov/org/request.py | 32 +++++++++++++---- src/onegov/user/auth/core.py | 11 +++--- src/onegov/user/auth/provider.py | 2 +- src/onegov/user/cli.py | 3 +- src/onegov/user/models/user.py | 5 +++ tests/onegov/org/test_views_user.py | 40 +++++++++++++++++++++ tests/onegov/user/test_models.py | 17 +++++++++ 9 files changed, 101 insertions(+), 17 deletions(-) diff --git a/src/onegov/core/security/identity_policy.py b/src/onegov/core/security/identity_policy.py index c59aa4fb7d..db1e9f71d9 100644 --- a/src/onegov/core/security/identity_policy.py +++ b/src/onegov/core/security/identity_policy.py @@ -27,6 +27,9 @@ def identify(self, request: CoreRequest) -> Identity | None: # FIXME: According to docs this should return NO_IDENTITY return None else: + uid = request.browser_session.get('uid') + if uid is not None: + identifiers['uid'] = uid return Identity(**identifiers) def remember( @@ -37,6 +40,8 @@ def remember( ) -> None: for key in self.required_keys: request.browser_session[key] = getattr(identity, key) + if (uid := getattr(identity, 'uid', None)) is not None: + request.browser_session['uid'] = uid def forget(self, response: Response, request: CoreRequest) -> None: request.browser_session.flush() diff --git a/src/onegov/org/forms/user.py b/src/onegov/org/forms/user.py index d0f644853c..ccae979b5f 100644 --- a/src/onegov/org/forms/user.py +++ b/src/onegov/org/forms/user.py @@ -202,8 +202,7 @@ def process( def populate_obj(self, obj: User) -> None: # type: ignore[override] assert self.new_username.data is not None request = self.request - obj.logout_all_sessions(request.app) - obj.username = self.new_username.data + obj.change_username(self.new_username.data, request.app) # Run application-specific callback request.app.settings.user.change_username_callback(obj, request) diff --git a/src/onegov/org/request.py b/src/onegov/org/request.py index 7263d5bb2a..2831d0df96 100644 --- a/src/onegov/org/request.py +++ b/src/onegov/org/request.py @@ -1,6 +1,5 @@ from __future__ import annotations -import sentry_sdk from functools import cached_property from onegov.core.custom import msgpack @@ -13,8 +12,8 @@ from onegov.user import User from sedate import utcnow from sqlalchemy import inspect +from uuid import UUID from sqlalchemy.orm import noload -from webob.exc import HTTPForbidden from typing import Any, NamedTuple, TYPE_CHECKING @@ -106,6 +105,9 @@ def is_member(self) -> bool: @property def current_username(self) -> str | None: + user = self.current_user + if user is not None: + return user.username return self.identity.userid if self.identity else None # NOTE: Internal cache, don't use directly! @@ -117,11 +119,27 @@ def current_user(self) -> User | None: return None if not hasattr(self, '_current_user'): - self._current_user = ( - self.session.query(User) - .filter_by(username=self.identity.userid) - .first() - ) + uid = getattr(self.identity, 'uid', None) + if uid: + self._current_user = ( + self.session.query(User) + .filter(User.id == UUID(uid)) + .first() + ) + if ( + self._current_user is not None + and self.identity.userid != self._current_user.username + ): + # stale username in session — fix it for subsequent + # requests + self.browser_session['userid'] = ( + self._current_user.username) + else: + self._current_user = ( + self.session.query(User) + .filter_by(username=self.identity.userid) + .first() + ) return self._current_user if self._current_user is None: diff --git a/src/onegov/user/auth/core.py b/src/onegov/user/auth/core.py index 9806a320f0..7c0281d9aa 100644 --- a/src/onegov/user/auth/core.py +++ b/src/onegov/user/auth/core.py @@ -3,6 +3,7 @@ import morepath from itsdangerous import URLSafeSerializer, BadData +from uuid import UUID from itsdangerous.encoding import base64_encode, base64_decode from secrets import token_bytes from onegov.core.utils import relative_url @@ -272,11 +273,6 @@ def as_identity(self, user: User) -> Identity: """ Returns the morepath identity of the given user. """ return self.identity_class( - # FIXME: We should consider switching to user.id.hex - # for userid and provide username in a separate field - # instead, but this was the less intrusive step - # in the meantime. We use identity.userid in quite - # a few places after all. userid=user.username, uid=user.id.hex, groupids=frozenset(group.id.hex for group in user.groups), @@ -286,6 +282,11 @@ def as_identity(self, user: User) -> Identity: def by_identity(self, identity: Identity | NoIdentity) -> User | None: """ Returns the user record of the given identity. """ + uid = getattr(identity, 'uid', None) + if uid: + user = self.users.by_id(UUID(uid)) + if user is not None: + return user if identity.userid is None: return None return self.users.by_username(identity.userid) diff --git a/src/onegov/user/auth/provider.py b/src/onegov/user/auth/provider.py index 1cc53bc263..cfcc3aea3f 100644 --- a/src/onegov/user/auth/provider.py +++ b/src/onegov/user/auth/provider.py @@ -324,7 +324,7 @@ def ensure_user( if users.by_username(username) is not None: log.error(f'Cannot rename user {user.username} to {username}') else: - user.username = username + user.change_username(username, app) app.settings.user.change_username_callback(user, request) # update the role even if the user exists already diff --git a/src/onegov/user/cli.py b/src/onegov/user/cli.py index 068b8d8d5b..fd09221cdc 100644 --- a/src/onegov/user/cli.py +++ b/src/onegov/user/cli.py @@ -282,8 +282,7 @@ def change(request: CoreRequest, app: Framework) -> None: if users.exists(new_username): abort(f'{new_username} already exists') - user.logout_all_sessions(request.app) - user.username = new_username + user.change_username(new_username, request.app) # Run application-specific callback app.settings.user.change_username_callback(user, request) diff --git a/src/onegov/user/models/user.py b/src/onegov/user/models/user.py index 0bd87bd0c0..476ce3401d 100644 --- a/src/onegov/user/models/user.py +++ b/src/onegov/user/models/user.py @@ -359,3 +359,8 @@ def logout_all_sessions(self, app: Framework) -> int: self.cleanup_sessions(app) return count + + def change_username(self, new_username: str, app: Framework) -> None: + """ Changes the username, logging out all active sessions first. """ + self.logout_all_sessions(app) + self.username = new_username diff --git a/tests/onegov/org/test_views_user.py b/tests/onegov/org/test_views_user.py index c16a035690..0b7539be1a 100644 --- a/tests/onegov/org/test_views_user.py +++ b/tests/onegov/org/test_views_user.py @@ -115,6 +115,46 @@ def test_change_username(client: Client) -> None: assert 'editor2@example.org' in users_page +def test_change_username_invalidates_sessions(client: Client) -> None: + editor = client.spawn() + editor.login_editor() + assert editor.get('/userprofile').status_code == 200 + + # Admin renames the editor via change_username (logs out all sessions) + transaction.begin() + users = UserCollection(client.app.session()) + user = users.by_username('editor@example.org') + assert user is not None + user.change_username('editor.renamed@example.org', client.app) + transaction.commit() + close_all_sessions() + + # The editor's existing session should now be invalid + assert editor.get('/userprofile', expect_errors=True).status_code == 403 + + +def test_session_survives_username_rename(client: Client) -> None: + editor = client.spawn() + editor.login_editor() + assert editor.get('/userprofile').status_code == 200 + + # Rename the editor directly in the DB, bypassing change_username. + # This simulates an LDAP sync or any path that updates the username + # without invalidating the session. + transaction.begin() + users = UserCollection(client.app.session()) + user = users.by_username('editor@example.org') + assert user is not None + user.username = 'editor.renamed@example.org' + transaction.commit() + close_all_sessions() + + # The existing session should still work via primary key lookup. + # The stale userid in the session is also updated on this request so + # that code reading identity.userid directly sees the correct username. + assert editor.get('/userprofile').status_code == 200 + + def test_user_source(client: Client) -> None: client.login_admin() diff --git a/tests/onegov/user/test_models.py b/tests/onegov/user/test_models.py index 7dba4474cd..f46722fd7d 100644 --- a/tests/onegov/user/test_models.py +++ b/tests/onegov/user/test_models.py @@ -176,6 +176,23 @@ def test_user_logout_all_sessions() -> None: assert call(None, 'yyy') in forget.mock_calls +def test_user_change_username() -> None: + user = User(username='old@example.org') + + with patch('onegov.user.models.user.remembered') as remembered: + with patch('onegov.user.models.user.forget') as forget: + remembered.return_value = True + + user.save_current_session(DummyRequest('sess1')) # type: ignore[arg-type] + assert user.sessions is not None + assert 'sess1' in user.sessions + + user.change_username('new@example.org', DummyRequest('zzz').app) # type: ignore[arg-type] + + assert call(None, 'sess1') in forget.mock_calls + assert user.username == 'new@example.org' + + def test_user_cleanup_sessions() -> None: user = User() assert not user.sessions From 6625dd44c393ed3e2f843156acd9c905c90222c1 Mon Sep 17 00:00:00 2001 From: Reto Tschuppert Date: Fri, 29 May 2026 09:51:58 +0200 Subject: [PATCH 21/26] Make a required key and remove special handling --- src/onegov/core/security/identity_policy.py | 7 +------ 1 file changed, 1 insertion(+), 6 deletions(-) diff --git a/src/onegov/core/security/identity_policy.py b/src/onegov/core/security/identity_policy.py index db1e9f71d9..fc4db44190 100644 --- a/src/onegov/core/security/identity_policy.py +++ b/src/onegov/core/security/identity_policy.py @@ -16,7 +16,7 @@ class IdentityPolicy: """ - required_keys = {'userid', 'groupids', 'role', 'application_id'} + required_keys = {'userid', 'uid', 'groupids', 'role', 'application_id'} def identify(self, request: CoreRequest) -> Identity | None: try: @@ -27,9 +27,6 @@ def identify(self, request: CoreRequest) -> Identity | None: # FIXME: According to docs this should return NO_IDENTITY return None else: - uid = request.browser_session.get('uid') - if uid is not None: - identifiers['uid'] = uid return Identity(**identifiers) def remember( @@ -40,8 +37,6 @@ def remember( ) -> None: for key in self.required_keys: request.browser_session[key] = getattr(identity, key) - if (uid := getattr(identity, 'uid', None)) is not None: - request.browser_session['uid'] = uid def forget(self, response: Response, request: CoreRequest) -> None: request.browser_session.flush() From 51b0dfe5ad7877d3734d7c97d3c148b12bafb094 Mon Sep 17 00:00:00 2001 From: Reto Tschuppert Date: Fri, 29 May 2026 10:02:30 +0200 Subject: [PATCH 22/26] Avoid unnecessary DB query in current_username --- src/onegov/org/request.py | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/onegov/org/request.py b/src/onegov/org/request.py index 2831d0df96..ba755d8c93 100644 --- a/src/onegov/org/request.py +++ b/src/onegov/org/request.py @@ -105,9 +105,9 @@ def is_member(self) -> bool: @property def current_username(self) -> str | None: - user = self.current_user - if user is not None: - return user.username + if self._current_user: + return self._current_user.username + return self.identity.userid if self.identity else None # NOTE: Internal cache, don't use directly! From b69fc2bf5cb60ff840c06d4d376321364314a73c Mon Sep 17 00:00:00 2001 From: Reto Tschuppert Date: Fri, 29 May 2026 10:10:33 +0200 Subject: [PATCH 23/26] Remove uid fallback branch now that uid is always in required_keys --- src/onegov/org/request.py | 32 +++++++++++--------------------- 1 file changed, 11 insertions(+), 21 deletions(-) diff --git a/src/onegov/org/request.py b/src/onegov/org/request.py index ba755d8c93..63362f891c 100644 --- a/src/onegov/org/request.py +++ b/src/onegov/org/request.py @@ -119,27 +119,17 @@ def current_user(self) -> User | None: return None if not hasattr(self, '_current_user'): - uid = getattr(self.identity, 'uid', None) - if uid: - self._current_user = ( - self.session.query(User) - .filter(User.id == UUID(uid)) - .first() - ) - if ( - self._current_user is not None - and self.identity.userid != self._current_user.username - ): - # stale username in session — fix it for subsequent - # requests - self.browser_session['userid'] = ( - self._current_user.username) - else: - self._current_user = ( - self.session.query(User) - .filter_by(username=self.identity.userid) - .first() - ) + self._current_user = ( + self.session.query(User) + .filter(User.id == UUID(self.identity.uid)) + .first() + ) + if ( + self._current_user is not None + and self.identity.userid != self._current_user.username + ): + # stale username in session — fix it for subsequent requests + self.browser_session['userid'] = self._current_user.username return self._current_user if self._current_user is None: From fbe56a5800f732d8c145806c17fbc9e1e073b4a3 Mon Sep 17 00:00:00 2001 From: Reto Tschuppert Date: Fri, 29 May 2026 11:03:07 +0200 Subject: [PATCH 24/26] Use identity.uid directly now that it's always set on Identity --- src/onegov/org/request.py | 4 ++-- src/onegov/user/auth/core.py | 10 +++------- 2 files changed, 5 insertions(+), 9 deletions(-) diff --git a/src/onegov/org/request.py b/src/onegov/org/request.py index 63362f891c..97858e80e2 100644 --- a/src/onegov/org/request.py +++ b/src/onegov/org/request.py @@ -4,7 +4,7 @@ from functools import cached_property from onegov.core.custom import msgpack from onegov.core.orm import orm_cached -from onegov.core.request import CoreRequest +from onegov.core.request import CoreRequest, is_logged_in from onegov.core.security import Private from onegov.core.utils import normalize_for_url from onegov.org.models import News, TANAccessCollection, Topic @@ -115,7 +115,7 @@ def current_username(self) -> str | None: @property def current_user(self) -> User | None: - if not self.identity: + if not is_logged_in(self.identity): return None if not hasattr(self, '_current_user'): diff --git a/src/onegov/user/auth/core.py b/src/onegov/user/auth/core.py index 7c0281d9aa..7cde424844 100644 --- a/src/onegov/user/auth/core.py +++ b/src/onegov/user/auth/core.py @@ -3,7 +3,6 @@ import morepath from itsdangerous import URLSafeSerializer, BadData -from uuid import UUID from itsdangerous.encoding import base64_encode, base64_decode from secrets import token_bytes from onegov.core.utils import relative_url @@ -282,14 +281,11 @@ def as_identity(self, user: User) -> Identity: def by_identity(self, identity: Identity | NoIdentity) -> User | None: """ Returns the user record of the given identity. """ - uid = getattr(identity, 'uid', None) - if uid: - user = self.users.by_id(UUID(uid)) - if user is not None: - return user + if identity.userid is None: return None - return self.users.by_username(identity.userid) + + return self.users.by_id(identity.uid) def login_to( self, From f820b9a5d6b393819da77428b5d4d4809c9cafda Mon Sep 17 00:00:00 2001 From: Reto Tschuppert <124258444+Tschuppi81@users.noreply.github.com> Date: Fri, 29 May 2026 05:05:02 -0400 Subject: [PATCH 25/26] use multiple context managers in single statement Co-authored-by: David Salvisberg --- tests/onegov/user/test_models.py | 20 +++++++++++--------- 1 file changed, 11 insertions(+), 9 deletions(-) diff --git a/tests/onegov/user/test_models.py b/tests/onegov/user/test_models.py index f46722fd7d..b3f3b9eba3 100644 --- a/tests/onegov/user/test_models.py +++ b/tests/onegov/user/test_models.py @@ -179,18 +179,20 @@ def test_user_logout_all_sessions() -> None: def test_user_change_username() -> None: user = User(username='old@example.org') - with patch('onegov.user.models.user.remembered') as remembered: - with patch('onegov.user.models.user.forget') as forget: - remembered.return_value = True + with ( + patch('onegov.user.models.user.remembered') as remembered, + patch('onegov.user.models.user.forget') as forget + ): + remembered.return_value = True - user.save_current_session(DummyRequest('sess1')) # type: ignore[arg-type] - assert user.sessions is not None - assert 'sess1' in user.sessions + user.save_current_session(DummyRequest('sess1')) # type: ignore[arg-type] + assert user.sessions is not None + assert 'sess1' in user.sessions - user.change_username('new@example.org', DummyRequest('zzz').app) # type: ignore[arg-type] + user.change_username('new@example.org', DummyRequest('zzz').app) # type: ignore[arg-type] - assert call(None, 'sess1') in forget.mock_calls - assert user.username == 'new@example.org' + assert call(None, 'sess1') in forget.mock_calls + assert user.username == 'new@example.org' def test_user_cleanup_sessions() -> None: From 78556597513b4276e786377e9e3636fbba09969b Mon Sep 17 00:00:00 2001 From: Reto Tschuppert Date: Mon, 1 Jun 2026 08:46:55 +0200 Subject: [PATCH 26/26] Guard the access to --- src/onegov/org/request.py | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/src/onegov/org/request.py b/src/onegov/org/request.py index 97858e80e2..311d9d278a 100644 --- a/src/onegov/org/request.py +++ b/src/onegov/org/request.py @@ -105,8 +105,9 @@ def is_member(self) -> bool: @property def current_username(self) -> str | None: - if self._current_user: - return self._current_user.username + cached = getattr(self, '_current_user', None) + if cached is not None: + return cached.username return self.identity.userid if self.identity else None