Port clubsite to Rails - #183
Draft
russellporter wants to merge 16 commits into
Draft
Conversation
…imezone bugs - New CI workflow runs the test suite against MariaDB plus a Brakeman scan; the container image push now requires both to pass - Remove the unused Webpacker setup and empty scaffold controllers, helpers, and tests - Scope User#max_privilege_level to the user (previously it returned the highest access level held by anyone, letting any signed-in user pass the redactor upload check) - Fix the broken privileges.maps settings lookup in RedactorPolicy - Stop Event#to_ics and #to_fullcalendar from mutating the global time zone - Enforce CSRF with exceptions; IOF result uploads keep an explicit skip since external tools authenticate over HTTP Basic - Add real model/policy tests and fixtures (two clubs, all privilege tiers) - Add user.merge privilege level to settings
Serve club domains from this app alongside the apex site, in preparation for folding the CakePHP Clubsite in: - Requests on non-apex hosts route to a new Clubsite:: namespace via a domain constraint; a catch-all keeps club domains out of apex routes - Clubsite::BaseController resolves the club from the Host header (301s redirect_domain, 404s unknown hosts), runs requests in the club's timezone, and selects the club's layout - Port the default/other/embed/printable layouts, menu highlighting, club branding resources (header image, logo, custom CSS), and per-series color CSS - Fold clubsite frontend assets into Sprockets: vendored libraries, the wjr modules converted from AMD to plain scripts (RequireJS dropped), and five clubsite bundles; apex bundles unchanged - Per-club robots.txt honoring club visibility; apex robots.txt moves to a controller since static files are served for every host - IOF XML, event feed, maps, and redactor API routes become reachable on club domains as well as the apex - Google Maps API key moves from source to configuration - Production host allowlisting shifts to nginx (generated per-club vhosts plus a default_server); development supports *.localhost club domains and seeds a demo club - Integration tests for tenancy, layout selection, redirects, robots
Pages and content blocks: - Club home, contact, resources, export, and admin hub pages; dynamic Resources pages with in-place jEditable editing (now CSRF-protected) - Content blocks with auto-seeded defaults and the legacy wrapper markup so the in-place editor keeps working - The admin hub is authorized server-side (level 90); the old app only hid the link - Page templates are dispatched from a fixed allowlist instead of rendering request params as template paths Events: - Calendar, event listing, and event detail pages including courses, results, live results, organizers, registration section markup, OpenGraph tags, and Flickr photos - Event .xml requests 301 to the IOF result list; custom_url events redirect non-editors; upcoming/past/ongoing/for_series scopes port the legacy date-window semantics Maps, courses, and results: - Map list/detail/report pages, map file downloads with the Dropbox direct-download rewrite, and course/result pages - Uploaded media served through MediaStore, which only builds paths from integer ids and per-type allowlisted thumbnail sizes Shared: media and geocoding helpers (Nominatim now sent a User-Agent and cached through Rails.cache), time/OpenGraph/link helpers, default media images, and fixtures. Organizer-editable event and course descriptions are sanitized on output; webmaster-authored content blocks and pages render as stored. 86 tests green, Brakeman clean.
Sign-in stays on the apex domain; club domains receive the session through a 60-second signed token bound to the destination host, checked by the club-side consume endpoint. This will replace the shared cross_app_sessions table once the PHP app is retired (the legacy flow is untouched until then). - Apex /sso/authorize validates the requested host against the clubs table, routes through the sign-in form when needed, and only forwards relative return paths; /sso/logout ends the apex session - Club /users/login, /users/logout, /users/logoutComplete and other legacy URL shapes are preserved; register/profile URLs 301 to the apex - Devise hardening: remember_for drops from 20 years to 1 year, paranoid mode on, rate limiting on sign-in, password reset, and token consume - Integration tests cover the full handoff, expiry, host binding, open redirect rejection, and both-session sign-out
- Club settings (with Facebook page and Juicer feed URL-to-id parsing moved into the Club model), series, roles (including the JSON list the organizer picker uses), map standards, memberships, and officials - Privileges management with the level cap enforced server-side: users can only see, grant, and revoke groups at or below their own level, only within their club, and never global groups. The old app's grant endpoint had no authorization check at all; it now requires level 90 - Club branding uploads (header image, logo, custom CSS) with extension allowlists, overwrite-by-key semantics, and jpg thumbnails generated through image_processing/MiniMagick argument APIs instead of interpolated shell commands; ImageMagick and Ghostscript added to the image and CI - Thin Pundit policies per surface; all admin pages authorized server-side; all mutations POST with CSRF - 69 new integration tests; full suite 155 green, Brakeman clean
Events: - Event editor with the Knockout course and organizer editors (JSON blob contracts preserved, organizers replaced wholesale per save, dates assembled in the club's timezone), event delete at level 90 - Results editor with the legacy status/score semantics (blank status becomes ok, all-zero times become NULL) and the course-belongs-to- event check enforced before any write - Planner (now requiring the planning privilege it was always meant to have), printable entries, course map upload pages, live results visibility toggle Registration and users: - Course registration/unregistration with the legacy permission rules: anyone signed in may register others; unregistering someone else requires having registered them; duplicates are silent no-ops - Name-only fake users for registering people without accounts (now CSRF-protected), users autocomplete (now requiring sign-in), user merge as a transactional tool, duplicate finder Maps: - Map editor with image upload, generated thumbnails, and the random 60x60 banner crop folded into the upload path; marker-drag location updates now require the map edit privilege (they had no check) All remaining GET mutations are POST with CSRF; media pipelines go through MiniMagick argument APIs. Redactor loads from public/redactor when mounted at deploy time. 233 tests green, Brakeman clean.
form_with on a persisted record submits with the PATCH method override, but the legacy-shaped edit endpoints only accepted POST, so the edit forms for clubs, maps, series, roles, map standards, memberships, and officials 404ed in the browser (tests had been posting directly). Regression tests now submit the way browsers do.
- Capybara + selenium driving headless Chromium (installed in the image and CI); the system chromedriver is used explicitly since Selenium Manager hangs in the container, and --disable-gpu is required for Chrome to start at all - All hostnames resolve to the test server so the multi-domain club fixtures work in a real browser; club lookup falls back to the portless host since the test server picks a random port - Every system test fails on unexpected browser console errors, the widest net for JS wiring regressions; expected third-party noise is ignored - Club pages now fetch the event and result feeds same-origin instead of via the absolute apex URL (they are served by this app now), which the console tripwire caught - Separate CI job; the image push depends on it
Covers the P0 flows from the browser test plan in a real headless browser: the SSO round trip, the Knockout event and results editors, course registration including register-others, in-place jEditable editing, map editing with image upload, the calendar, the event lists, and club settings/privileges admin. 26 tests, each also failing on unexpected browser console errors. Bugs the tests exposed, fixed here: - The Register Others panel was dropped during the port; restored on the event page while registration is open - Pages with a rich-text area crashed all page JS when the deploy-time Redactor library is absent; degrades to a plain textarea now - The person picker dropped or resurfaced stale autocomplete responses arriving out of order (also broken in the legacy app) - The event list's showing-events-after/before headers never rendered due to an inverted null check (also broken in the legacy app) - Fixture events silently had no classification (explicit-id fixtures referenced by label) - The SSO endpoints now tolerate a port in the return host (as any non-standard-port deployment sends), with a strict host format check
Completes the automatable scope of the browser test plan: live results toggle and feed rendering, the registrant comment modal, printable entries, series admin with the color picker, branding uploads and their layout effects, user merge and the duplicates screen, .embed iframe variants, the remaining admin CRUD through real forms, and the password reset mail flow. 42 system tests total. Bugs the tests exposed, fixed here: - A misnamed callback action (update_live vs update_live_result_list) broke every request to the results controller wherever Rails raises on missing callback actions, including the live results feed, and left live result uploads without their authentication gate - The live results header bound a nonexistent creationDate property, so the results-produced-on date never rendered
- Block cross-tenant privilege escalation through user merge: a club admin can no longer merge a global-admin account (and inherit its cross-club rights or delete it). Adds User#global_admin?/#global_privilege_level. - Gate the globally shared map standards and roles behind a real global-admin privilege instead of any per-club level-100 administrator. - Scope course unregistration to the current course so a registration made on one course can't be used to unregister someone elsewhere; require a bounded, non-blank name for fake-user creation. - Restrict redactor uploads to an allowlisted set of extensions (stops stored XSS via .html/.svg), independent of the spoofable client content type. - Sanitize admin-authored rich text (pages, content blocks, series info) and escape club-supplied fields on the apex homepage. - Validate Map#file_url, neutralize CSV formula injection in the participation report, raise the minimum password length to 8, replace the committed Devise secret sample with a placeholder, and delete the dead update_result_list endpoint. - Add regression tests covering each of the above.
activerecord-session_store serialized session data with Marshal, so any database-write foothold (restored backup, compromised replica, a future SQLi) could be escalated to RCE via Marshal.load on the next request. The DB store was only there for the legacy cross-app setup, but the CakePHP app uses file-based sessions and never reads Rails' sessions table (the bridge rides on the separate cross_app_sessions token table), so the store can be removed. - Switch to the encrypted :cookie_store (secure + httponly via force_ssl). - Remove the activerecord-session_store gem, drop the sessions table, and remove the now-unused sessions:trim cron job. Deploy note: the session cookie key changes, so users are signed out once.
The system tests fail on CI but pass locally, and the failure assertion alone (wrong host after the SSO handoff) does not show which redirect branch was taken. Save the final URL and page HTML next to the automatic failure screenshot and upload the folder as a workflow artifact.
The CI failures all shared one shape: after signing in on the apex domain, the next visit to a club domain was silently dropped and the test asserted against the apex page. The sign-in submit goes through rails-ujs, so the driver's click does not wait for the resulting navigation; sign_in_through_browser now waits for the signed-in page before returning. CI's Chrome 151 loses that race consistently while the Docker image's Chromium 131 happens to win it. Also de-flake the registrant comment tests: Bootstrap's modal fade animation moves the dialog while the click position is computed, so the Save click could land on the backdrop and dismiss the modal without submitting. Disable animations in system tests and wait on the saved comment instead of on a header that is already visible under the modal. The failure-page capture now runs in before_teardown, while the page is still live, instead of after Capybara has reset the session.
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
1 participant
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.
No description provided.