fix: security and calendar-sync hardening sweep#493
Open
jaspermayone wants to merge 3 commits into
Open
Conversation
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).
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.
Remediates the findings from a full-codebase review, in severity order. All specs pass (25, up from 7); rubocop, brakeman, and zeitwerk clean.
Critical
/api/user/onboardaccepted a raw email and minted a non-expiring JWT for any existing account. It now requires a Google access token, verifies it with Google viaGoogleTokenVerifier(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).destroy_allon 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
:registerable+ signup route (bypassed the@wit.edugate).user_idfor the admin safelist / throttle bucket.RRULE UNTILcomputed in local (Eastern) zone so evening classes keep their final occurrence.create_event_in_calendarrescuesRecordNotUniqueand drops the duplicate remote event.catalog:refreshpre-flights the scrape and wraps delete+import+restore in a transaction;finals:resetprotects current/future terms and dry-runs by default.rooms/buildingon 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 fourhas_onerelations (migration includes conservative de-dup).EnforceHasOneUniquenessde-duplicates existing rows (keeping the earliest) before adding unique indexes, using raw SQL to avoid firingGoogleCalendar'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-extensionbranchjaspermayone/onboard-google-token-auth. Merge together. The extension's OAuth client ID must be added to the backendGOOGLE_OAUTH_CLIENT_IDSenv (comma-separated;GoogleTokenVerifieralso trusts the web client).