Skip to content

fix: security and calendar-sync hardening sweep#493

Open
jaspermayone wants to merge 3 commits into
mainfrom
jaspermayone/security-hardening-severity-sweep
Open

fix: security and calendar-sync hardening sweep#493
jaspermayone wants to merge 3 commits into
mainfrom
jaspermayone/security-hardening-severity-sweep

Conversation

@jaspermayone

Copy link
Copy Markdown
Member

Remediates the findings from a full-codebase review, in severity order. All specs pass (25, up from 7); rubocop, brakeman, and zeitwerk clean.

Critical

  • Onboard account takeover/api/user/onboard accepted a raw email and minted a non-expiring JWT for any existing account. It now requires a Google access token, verifies it with Google via GoogleTokenVerifier (audience-checked), derives the email from the verified token, enforces @wit.edu, and issues a 14-day, always-expiring JWT. Requires the matching extension change (see below).
  • Calendar-event orphaning — re-scraping one student's schedule ran destroy_all on shared meeting times, orphaning every other enrolled student's tracked Google events (and spawning duplicates). Now reconciles by content key (deletes only stale rows, only when the scrape produced data) and deletes the remote Google event when a tracking row is destroyed.

High

  • Removed Devise :registerable + signup route (bypassed the @wit.edu gate).
  • rack-attack now verifies the JWT signature before trusting user_id for the admin safelist / throttle bucket.
  • RRULE UNTIL computed in local (Eastern) zone so evening classes keep their final occurrence.
  • Creation-triggered sync routed through the concurrency-limited job; create_event_in_calendar rescues RecordNotUnique and drops the duplicate remote event.
  • catalog:refresh pre-flights the scrape and wraps delete+import+restore in a transaction; finals:reset protects current/future terms and dry-runs by default.
  • Preload rooms/building on the schedule endpoint (N+1).

Medium

  • tbd_room? string compare; course-destroy sync users captured before the enrollment cascade; room changes flagged via the join model; implemented the template disallowed-tag check; IDOR ownership check on gcal-event preference reads; university-events N+1; unique indexes + validations for the four has_one relations (migration includes conservative de-dup).

⚠️ Migration

EnforceHasOneUniqueness de-duplicates existing rows (keeping the earliest) before adding unique indexes, using raw SQL to avoid firing GoogleCalendar's remote-delete callback. Review the de-dup against production data before deploying.

Coordinated extension PR

Onboarding now expects a Google token instead of an email — see the calendar-extension branch jaspermayone/onboard-google-token-auth. Merge together. The extension's OAuth client ID must be added to the backend GOOGLE_OAUTH_CLIENT_IDS env (comma-separated; GoogleTokenVerifier also trusts the web client).

Critical
- onboard: require a Google-verified access token (GoogleTokenVerifier)
  instead of trusting a client-supplied email; enforce @wit.edu; issue a
  14-day, always-expiring JWT (JsonWebTokenService no longer mints
  non-expiring tokens)
- calendar sync: reconcile meeting times by content key instead of
  destroy_all (was orphaning every enrolled student's events on re-scrape);
  delete the remote Google event when a tracking row is destroyed

High
- remove Devise :registerable + signup route (bypassed the @wit.edu gate)
- rack_attack: verify JWT signature before trusting user_id for the admin
  safelist / throttle bucket
- RRULE UNTIL computed in local (Eastern) zone so evening classes keep
  their final occurrence
- route creation-triggered sync through the concurrency-limited job;
  rescue RecordNotUnique and drop the duplicate remote event
- catalog:refresh pre-flights the scrape and wraps delete+import+restore
  in a transaction; finals:reset protects current/future terms and dry-runs
  by default
- preload rooms/building on the schedule endpoint (N+1)

Medium
- fix tbd_room? string compare; capture course-destroy sync users before
  the enrollment cascade; flag room changes via the join model
- implement the template disallowed-tag check
- enforce IDOR ownership on gcal-event preference reads; preload term on
  university events (N+1)
- back the has_one relations with unique indexes + validations
  (migration includes conservative de-dup)

Adds specs for the token service, Google token verifier, calendar
preference uniqueness, and meeting-time reconcile.
…u gate

Onboard now keys/links the account to the verified (personal) Google email instead of enforcing @wit.edu. Auto-links returning users via an already-connected Google calendar credential; deliberately does not link by an unverified WIT email (would reintroduce the takeover).
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

migration Includes a database migration

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant