Repository navigation
Security review fixes: dev stack bind, login throttle, GDPR erasure, practice isolation - #545
Merged
Merged
Conversation
docker-compose.yml runs django with network_mode: host and bound gunicorn to 0.0.0.0:8000 with DEBUG on, so the dev stack (which mounts the real data directories) was reachable from the local network over plain HTTP. Bind 127.0.0.1 like the postgres service and the prod compose default, and add a guardrail test. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
AuthenticationForm normalizes the username with NFKC before the account lookup, but the lockout key only lowercased the raw input. Fullwidth and other compatibility spellings therefore reached the same account under a fresh counter, defeating the 5-attempt limit. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
- Anonymise the inquiry the client was converted from and delete their imported calendar events; both only SET_NULL their link and kept the client's details after the erasure. - Delete GebueH service records before sessions (PROTECT), which made the erasure roll back for those clients. - Send the notification email only after the transaction has committed. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
PracticeScoped{List,Update,Delete}View only filtered by practice when the
model's queryset had for_current_practice, and silently returned every row
otherwise. MarketingPeriod has a practice FK but a plain manager, so its
edit and delete views resolved any practice's row by pk.
Give MarketingPeriod the PracticeScopedManager, make the mixins raise
ImproperlyConfigured for a practice-FK model without one, and add a
guardrail test over every scoped view.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
get_active_token/get_service(None) returned the newest active token of any practice, and save_token(practice=None) deactivated every practice's token. A signed-in user without a practice could therefore read another practice's calendar, rewrite its calendar_id, or break its sync. None now selects only the practice-less fallback token, and the guardrail no longer exempts google_calendar.py. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
CalendarEventParser fell back to every client of every practice, and none of the callers passed a scoped list. A client code in practice A's calendar (anyone can send an invite) therefore matched practice B's client: the fetch command booked sessions onto it, and the approval queue showed it with B's draft invoices. - clients is now a required argument; callers pass practice_clients(). - Importing, the quick action and the billing and approval views ignore a stored match that belongs to another practice. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
PendingCalendarEvent.google_event_id was unique across all practices and fetch_calendar_events looked rows up by event id alone. When two practices' calendars returned the same event id, the second practice's sync got the first practice's row back and its cancel/reschedule transitions rewrote that practice's Session. The key is now (practice, google_event_id), and every lookup in the command includes the practice. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fixes from a source review of the views, settings, uploads and calendar import. Each commit is a self-contained fix with a regression test; every new test fails on the previous code.
Live issues
docker-compose.ymlran gunicorn on0.0.0.0:8000withnetwork_mode: hostand DEBUG on, so it was reachable from the local network over plain HTTP. It binds127.0.0.1now, like Postgres and prod's default, and a guardrail test checks it.UsernameFieldis, so compatibility spellings of a username each got a fresh counter.GDPR erasure
SET_NULLtheir link, so they kept the client's details.Leistungserfassungrows are deleted before sessions. They'rePROTECT, so the erasure used to roll back.Practice isolation (latent with one practice)
PracticeScoped{List,Update,Delete}Viewfail closed: a practice-FK model without the scoped manager raisesImproperlyConfigured. MarketingPeriod gets the manager; it could be edited and deleted across practices. A guardrail test walks every scoped view.practice_clients()). Imports, the quick action, billing and the approval queue ignore stored matches from another practice.get_active_token/save_tokenwithpractice=Nonemean the practice-less token, not every practice.google_calendar.pyis no longer exempt from the fail-open guardrail.PendingCalendarEventis unique on(practice, google_event_id)(migration0009), and every lookup infetch_calendar_eventsincludes the practice.Upgrading: migration
0009runs on start; existing rows already satisfy the new constraint. Restart the dev stack so gunicorn rebinds.Testing:
./dev.py test --fastpasses (2055 Django tests plus the JS tests);makemigrations --checkand./dev.py lintare clean.🤖 Generated with Claude Code