From 080761bab12c925412bb5b193a03b0d7b2fc7210 Mon Sep 17 00:00:00 2001 From: bhumindeshpande8-spec Date: Wed, 2 Sep 2026 23:33:11 +0530 Subject: [PATCH 1/4] Build the member and organiser dashboards, and the roles behind them Adds, on top of the sign-in that already existed: roles admins/{email} gains role + active, splitting isAdmin() from isOwner(). Owners manage the roster from /admin; admins cannot appoint anyone, so a compromised organiser account cannot escalate. Both fields default permissively so rows seeded before they existed keep working. forms one collection for forms AND polls -- a poll is a form with show_tally on. Responses are keyed by uid, so one-per-member is structural rather than checked. The tally is function-written and refused to every client. sessions when the club meets. A real starts_at timestamp, which is the one date in these rules not pinned to request.time, because scheduling is the whole feature. notices categories and a reversible archive. The dashboards share one app shell (components/dashboard/Shell.tsx), which replaces the marketing chrome on /dashboard and /admin -- see ChromeGate.tsx. The profile form stopped being a gate in front of the member dashboard and became a panel inside it; it was the reason organisers reported the site "has no dashboard". 155 rules assertions against the emulator, plus a two-role browser pass over both screens. Co-Authored-By: Claude Opus 5 --- FIREBASE.md | 153 ++++- firebase.json | 10 +- firestore.rules | 537 +++++++++++++++- functions/README.md | 150 +++-- functions/github.js | 211 +++++++ functions/index.js | 331 +++++++++- functions/package.json | 4 +- functions/test-github.mjs | 193 ++++++ web/app/admin/page.tsx | 77 ++- web/app/dashboard/page.tsx | 31 + web/app/globals.css | 98 +-- web/app/join/page.tsx | 59 +- web/app/layout.tsx | 8 +- web/app/page.tsx | 22 +- web/components/ApplyForm.tsx | 470 ++++++++++++++ web/components/ChromeGate.tsx | 44 ++ web/components/CommitGraph.tsx | 72 +-- web/components/Composer.tsx | 311 ++++++++++ web/components/FormBuilder.tsx | 527 ++++++++++++++++ web/components/Icon.tsx | 130 +++- web/components/JoinGate.tsx | 620 ------------------- web/components/MemberDashboard.tsx | 239 ++++++++ web/components/Nav.tsx | 121 +++- web/components/NumbersStrip.tsx | 2 +- web/components/Portrait.tsx | 2 +- web/components/ProfileCard.tsx | 270 +++++++++ web/components/ProofPanel.tsx | 2 +- web/components/Roster.tsx | 435 +++++++++++++ web/components/Sessions.tsx | 352 +++++++++++ web/components/SignInCard.tsx | 325 ++++++++++ web/components/dashboard/Board.tsx | 132 ++++ web/components/dashboard/Contributions.tsx | 288 +++++++++ web/components/dashboard/Forms.tsx | 409 +++++++++++++ web/components/dashboard/NextSessions.tsx | 79 +++ web/components/dashboard/NextUp.tsx | 153 +++++ web/components/dashboard/Panel.tsx | 104 ++++ web/components/dashboard/Shell.tsx | 279 +++++++++ web/components/hall/Hall.tsx | 4 +- web/components/hero/Hero.tsx | 2 +- web/content/site.ts | 12 + web/content/team-editorial.ts | 52 ++ web/lib/announcements.ts | 187 ++++++ web/lib/applications.ts | 131 ++++ web/lib/auth.tsx | 59 +- web/lib/contributions.ts | 114 ++++ web/lib/firebase.ts | 87 ++- web/lib/forms.ts | 230 +++++++ web/lib/roster.ts | 177 ++++++ web/lib/security-headers.js | 24 +- web/lib/sessions.ts | 133 ++++ web/package.json | 3 +- web/scripts/assert-site.mjs | 27 +- web/scripts/e2e-auth.mjs | 60 +- web/scripts/hosting-config.mjs | 6 + web/scripts/nav-surface.mjs | 20 + web/scripts/rules-emulator.mjs | 674 ++++++++++++++++++++- web/scripts/rules.mjs | 190 +++++- web/scripts/smoke.mjs | 84 ++- web/scripts/team-roster.mjs | 220 +++++++ web/tailwind.config.ts | 80 +-- 60 files changed, 8847 insertions(+), 979 deletions(-) create mode 100644 functions/github.js create mode 100644 functions/test-github.mjs create mode 100644 web/app/dashboard/page.tsx create mode 100644 web/components/ApplyForm.tsx create mode 100644 web/components/ChromeGate.tsx create mode 100644 web/components/Composer.tsx create mode 100644 web/components/FormBuilder.tsx delete mode 100644 web/components/JoinGate.tsx create mode 100644 web/components/MemberDashboard.tsx create mode 100644 web/components/ProfileCard.tsx create mode 100644 web/components/Roster.tsx create mode 100644 web/components/Sessions.tsx create mode 100644 web/components/SignInCard.tsx create mode 100644 web/components/dashboard/Board.tsx create mode 100644 web/components/dashboard/Contributions.tsx create mode 100644 web/components/dashboard/Forms.tsx create mode 100644 web/components/dashboard/NextSessions.tsx create mode 100644 web/components/dashboard/NextUp.tsx create mode 100644 web/components/dashboard/Panel.tsx create mode 100644 web/components/dashboard/Shell.tsx create mode 100644 web/content/team-editorial.ts create mode 100644 web/lib/announcements.ts create mode 100644 web/lib/applications.ts create mode 100644 web/lib/contributions.ts create mode 100644 web/lib/forms.ts create mode 100644 web/lib/roster.ts create mode 100644 web/lib/sessions.ts create mode 100644 web/scripts/team-roster.mjs diff --git a/FIREBASE.md b/FIREBASE.md index 7893117..0f44297 100644 --- a/FIREBASE.md +++ b/FIREBASE.md @@ -38,17 +38,29 @@ npm run dev # then open http://localhost:3000/join # 5. before you change any form option later npm run rules # asserts the rules still match the form + +# 6. OPTIONAL, and only for the dashboard's GitHub panel. Needs the Blaze plan, +# because outbound network from Cloud Functions does. Everything else works +# without this. +cd .. +firebase functions:secrets:set GITHUB_TOKEN # a token with NO scopes is enough +firebase deploy --only functions ``` In the Firebase console you need, in this order: a project → a **web app** (for the config values) → **Firestore in production mode** → rules deployed → **App Check** before launch. Each is a numbered step below. +Sign-in, the profile, the organisers' page, the member dashboard and the notice board all +work with steps 1–5 alone. Step 6 is only the GitHub panel on the dashboard, and the panel +says plainly that it has not counted yet rather than showing zeroes if you skip it. + --- ## What it does -Members sign in, fill a profile once, and organisers read the roster. +Members sign in, fill a profile once, and then have a dashboard of their own. Organisers +read the roster and post to the notice board. ``` /join ──Google sign-in (@sst.scaler.com only)──▶ Firebase Auth @@ -56,12 +68,21 @@ Members sign in, fill a profile once, and organisers read the roster. ▼ profile form ──setDoc()──▶ users/{uid} one doc per member │ owner-only read/write - ▼ -/admin ──list, admins only──────────▶ breakdowns + table + CSV export + ┌───────────────────────────┴───────────────┐ + ▼ ▼ +/dashboard the member's own page /admin breakdowns + table + CSV + │ │ + ├─ announcements/{id} read by every member └─ writes announcements/{id} + └─ contributions/{uid} read by its owner + ▲ + └─ written ONLY by a Cloud Function, from the GitHub API ``` -No server, no admin SDK, no API route. The site stays a static build; everything is the -client talking to Firestore under the rules. +The site is still a static build with no API route: everything above the dotted line is +the client talking to Firestore under the rules. The one exception is +`contributions/{uid}`, which no client may write — a Cloud Function fetches those numbers +from GitHub with the Admin SDK, because a count the browser writes is a count the browser +can invent. | File | Role | |---|---| @@ -73,18 +94,41 @@ client talking to Firestore under the rules. | `web/components/JoinGate.tsx` | The four states of `/join`. | | `web/components/ProfileForm.tsx` | The form itself. | | `web/components/AdminDashboard.tsx` | The organisers' view. | +| `web/components/MemberDashboard.tsx` | The four states of `/dashboard`. | +| `web/components/ProfileCard.tsx` | The saved profile, shared by `/join` and `/dashboard`. | +| `web/components/Composer.tsx` | Where organisers write to the notice board. | +| `web/lib/announcements.ts` | The notice board's shape and its four operations. | +| `web/lib/contributions.ts` | Reads GitHub counts; never writes them. | +| `functions/github.js` | Talks to the GitHub API. Pure, and tested without a network. | | `web/scripts/rules.mjs` | Text check: rules vs the form. Runs in CI. | | `web/scripts/rules-emulator.mjs` | Executes the rules as several different users. | | `web/.env.example` | The variables, with notes. | -Three collections: +Five collections: | Collection | Who can read | Who can write | |---|---|---| | `users/{uid}` | that member, and admins | that member only, validated | | `admins/{email}` | your own row only | **nobody, from any client** | +| `announcements/{id}` | **every member** | admins only, validated | +| `contributions/{uid}` | that member, and admins | **nobody — a Cloud Function only** | | `applications/{id}` | nobody | nobody — legacy, kept sealed | +Two of those rows are worth pausing on, because they are the exceptions to the pattern +the other three follow: + +* **`announcements` is the only collection a member may list in bulk.** That is safe + because a notice holds nothing personal beyond the organiser's own byline — unlike + `users`, where every row is somebody's address and the list rule is therefore + admin-only. If a field is ever added to a notice that names a member, revisit + `allow list` in `firestore.rules` before writing the field. +* **`contributions` is write-denied to everybody, including the member it describes.** + It is a separate collection rather than a field on the profile for exactly this reason: + the profile is member-written and validated with a strict `hasOnly` list, so a function + writing merged-PR counts into it would either have to be allowed in that list — at + which point members could type their own contribution numbers — or fail validation on + every sync. + --- ## Setup @@ -201,9 +245,13 @@ cd web && npm run dev Open `/join`. You should get **"Sign in with your college account"**. Sign in with an `@sst.scaler.com` Google account, fill the profile, and save — you should land on -**"You're in the club."** with your details listed, and a new document under +**"That's you signed up."** with your details listed, and a new document under **Firestore → Data → users**, whose id is your Auth uid. +Follow **"Open my dashboard"** from there. `/dashboard` should greet you by first name +and show three panels: the notice board (empty until an organiser posts), your GitHub +activity (see below), and your saved details. + Then try it with a personal Gmail account: sign-in should refuse it and say so. If you get anything else, the troubleshooting table below names the likely cause. @@ -371,6 +419,97 @@ collection: denying it is what stops a compromised admin session appointing more --- +## The member dashboard + +`/dashboard` is what a signed-in member sees and a visitor never does. It needs nothing +beyond the sign-in setup above to work — the notice board and the profile panel run on +Firestore alone. **Only the GitHub panel needs anything extra**, and that is the rest of +this section. + +Like `/admin`, this route is **not a privilege gate**. The site is a static export, so the +HTML ships to anybody who asks for it; what refuses a stranger is `firestore.rules`. A +signed-out visitor who loads the URL gets a "sign in first" card, because every read +behind it is denied. + +### The notice board + +Nothing to configure. Deploy the rules and it works: organisers get a composer at the +foot of `/admin`, and what they post appears on every member's dashboard. + +The one thing worth knowing is that **posting is the only admin-only collection a client +writes**. Appointing admins is still console-only and deliberately so — that is the +privilege which grants privileges. Posting a notice happens weekly, is reversible, and is +often done from a phone, so routing it through the Firebase console would mean nobody +ever posts. The argument is written out in `firestore.rules` above the `announcements` +block. + +### GitHub contributions + +Two functions fill `contributions/{uid}`: + +| Function | When | What | +|---|---|---| +| `syncContributions` | 04:00 IST daily | the 100 stalest members with a handle | +| `refreshContributions` | a member presses the button | just that member, 10-minute cooldown | + +**This needs the Blaze plan.** Outbound network from Cloud Functions does, and GitHub is +outbound. The usage sits far inside the free allowance for a student club, but a billing +account has to exist on the project. That is Google's restriction on egress, not a choice +made here — the same note applies to the application email function. + +**1. Set the token.** Optional but not really: + +```bash +firebase functions:secrets:set GITHUB_TOKEN +``` + +A fine-grained personal access token with **no scopes at all** is enough. This only reads +public data; the token is for the rate limit, not for access. Without it GitHub allows 10 +searches a minute and 60 user lookups an *hour* across the whole function, so the sweep +stops after about fifteen members and the failure arrives as a 403 that reads like a +permissions problem. With it: 30 a minute and 5,000 an hour. + +**2. Deploy.** + +```bash +firebase deploy --only functions,firestore:rules +``` + +`firebase.json` gained a `functions` block for this — without one the CLI has no source +directory and `--only functions` deploys nothing while reporting success. + +**3. Check it.** The scheduled function will not have run yet, so drive the callable: +sign in, put your own GitHub handle in your details, open `/dashboard`, and press +**"Check GitHub now"**. Your merged pull requests should appear within a few seconds, and +a document should show up under **Firestore → Data → contributions**. + +If the button reports that it could not reach the sync, check in this order: + +| Symptom | Cause | +|---|---| +| `functions/internal` in the console | the CSP is missing the callable's origin — see `web/lib/security-headers.js`, and note that `firebase.json`'s copy is **generated** from it | +| `functions/not-found` | the function is deployed to a different region than `FUNCTIONS_REGION` | +| `permission-denied` | the signed-in address is off-domain, or unverified | +| counts stay at zero for a real account | the handle has a typo — the panel says so explicitly when GitHub returns 404 | + +### What the numbers mean, and do not + +* **`merged`** is GitHub's own `total_count` for `type:pr is:merged author:`, so + it is a lifetime figure and not a page length. +* **`repos`** is derived rather than returned, by counting distinct repositories across + one page of results. It is therefore **exact up to 100 merged pull requests** and an + undercount past that. If the club ever has such members, paginate in + `functions/github.js` rather than leaving the number quietly wrong. +* **The handle is not verified.** `github` is free text a member typed, so the panel + reports activity for *a handle*, not for a proven identity — and the heading on screen + says so. Verifying it would mean OAuthing GitHub as well as Google, which is a second + sign-in for a panel nobody is scored on. +* **None of it is a score.** There is no ranking, no target and no streak, and the empty + state reads "nothing yet" rather than "0". A club whose pitch is *you do not need to be + good yet* should not open with a leaderboard a first-year loses. + +--- + ## Reading submissions **Firestore → Data → `applications`.** Access is governed by who has permissions on diff --git a/firebase.json b/firebase.json index c45b7d7..4127cb5 100644 --- a/firebase.json +++ b/firebase.json @@ -2,6 +2,11 @@ "firestore": { "rules": "firestore.rules" }, + "functions": { + "source": "functions", + "codebase": "default", + "ignore": ["node_modules", ".git", "*.log", "test-*.mjs"] + }, "emulators": { "auth": { "port": 9099 @@ -9,6 +14,9 @@ "firestore": { "port": 8080 }, + "functions": { + "port": 5001 + }, "ui": { "enabled": true, "port": 4000 @@ -30,7 +38,7 @@ "headers": [ { "key": "Content-Security-Policy", - "value": "default-src 'self'; font-src 'self'; img-src 'self' data:; script-src 'self' 'unsafe-inline' https://apis.google.com https://www.google.com/recaptcha/ https://www.gstatic.com/recaptcha/; style-src 'self' 'unsafe-inline'; connect-src 'self' https://firestore.googleapis.com https://*.googleapis.com; frame-src 'self' https://www.google.com https://accounts.google.com https://osc-website-610b9.firebaseapp.com; form-action 'self'; frame-ancestors 'none'; object-src 'none'; base-uri 'self'; upgrade-insecure-requests" + "value": "default-src 'self'; font-src 'self'; img-src 'self' data:; script-src 'self' 'unsafe-inline' https://apis.google.com https://www.google.com/recaptcha/ https://www.gstatic.com/recaptcha/; style-src 'self' 'unsafe-inline'; connect-src 'self' https://firestore.googleapis.com https://*.googleapis.com https://asia-south1-osc-website-610b9.cloudfunctions.net; frame-src 'self' https://www.google.com https://accounts.google.com https://osc-website-610b9.firebaseapp.com; form-action 'self'; frame-ancestors 'none'; object-src 'none'; base-uri 'self'; upgrade-insecure-requests" }, { "key": "Strict-Transport-Security", diff --git a/firestore.rules b/firestore.rules index 62999f1..95a1a1e 100644 --- a/firestore.rules +++ b/firestore.rules @@ -7,14 +7,47 @@ rules_version = '2'; // static export with no server — so there is no back end doing checks. Everything that // protects members' data is in this file. // -// The site now has sign-in. Three collections: +// THE SITE HAS TWO INDEPENDENT FRONT DOORS, and that split matters here more than +// anywhere else in this file: // -// users/{uid} one profile per member. Readable and writable ONLY by that -// member, plus readable by admins. Contains names, college -// addresses, year, branch and hostel. -// admins/{email} membership = adminship. Nobody can write it from any client. -// applications/{id} legacy, from before sign-in. Create-only and unreadable, kept -// so the rows already there stay protected. +// APPLYING is anonymous. /join takes a form from anybody, with no account and no token, +// and that write is the single most exposed operation in this project — it is the one +// thing an unauthenticated stranger is allowed to do. Everything standing behind it is +// `isWellFormedApplication` below, plus App Check. +// +// MEMBERSHIP is authenticated and restricted to one email domain. Everything else in +// this file requires `isMember()`. +// +// Those two used to be one flow, with sign-in standing in front of the application form, +// which meant this file had no anonymous write surface at all. It has one again. If you +// are reviewing one thing here, review the create rule on applications/{id}. +// +// Five collections: +// +// users/{uid} one profile per member. Readable and writable ONLY by that +// member, plus readable by admins. Contains names, college +// addresses, year, branch and hostel. +// admins/{email} the core-team roster: the access grant AND the person's +// public billing, in one row so the two cannot drift. Owners +// write it; admins read it; members see only their own row. +// announcements/{id} the members-only notice board. EVERY member may list it; +// only admins write. The one collection here that is readable +// in bulk, because nothing personal goes in one. +// sessions/{id} when the club actually meets. Every member reads them; admins +// write them. Ordered by `starts_at`, which is why it is a real +// timestamp and not typed text. +// forms/{id} forms AND polls; a poll is a form with `show_tally` on. Every +// member reads them; admins write them. The `tally` field is +// function-written and refused to every client. +// forms/{id}/responses/{uid} +// one answer per member, keyed by uid so a second is impossible. +// Attributed: admins list them, members read only their own. +// contributions/{uid} GitHub counts for one member, written ONLY by the Cloud +// Function that fetches them. No client writes it, including +// its owner — see the note on the match block. +// applications/{id} anonymous applications from people who are NOT members yet. +// Create-only, validated on the way in, and readable by nobody +// through any client — see the match block. // // ONLY @sst.scaler.com MAY REGISTER, and this is where that is enforced. web/lib/auth.tsx // also passes the domain to Google and signs out anyone off-domain, but both of those @@ -53,14 +86,54 @@ service cloud.firestore { && request.auth.token.email.lower().matches('^[^@]+@sst[.]scaler[.]com$'); } - /** An admin is a member whose address has a document in `admins`. Looked up by - * email rather than uid so organisers can be added before they first sign in. + /** The caller's own row in `admins`, or an error if they have none — so it is only + * ever reached after an exists() guard. Factored out because three helpers below + * need it and Firestore caps a single evaluation at ten document lookups. */ + function adminDoc() { + return get(/databases/$(database)/documents/admins/$(request.auth.token.email.lower())); + } + + /** An admin is a member whose address has a row in `admins` that has not been + * retired. Looked up by email rather than uid so organisers can be added before + * they first sign in — with uid keys you would have to make somebody sign in, read + * their uid out of the Auth tab, and only then grant access, which is a poor first + * day for a club that adds people monthly. * * Lowercased on lookup because Google returns the address as the person typed it; - * the document id in `admins` must therefore always be lowercase. */ + * the document id in `admins` must therefore always be lowercase. + * + * `active` DEFAULTS TO TRUE WHEN ABSENT, and that is a compatibility decision + * rather than a lax one. Rows seeded by hand before this field existed carry only + * `added_by`, and a strict test would have silently locked out every organiser the + * club already had at the moment these rules deployed. New rows are written with + * the field by isWellFormedAdmin() below, so the default only ever applies to the + * old ones. + * + * RETIRING SOMEBODY IS `active: false`, NOT A DELETE. Deleting revokes just as + * fast but erases who ran the club last year, which is exactly what a handover + * needs. `allow delete` is therefore false on this collection. */ function isAdmin() { return isMember() - && exists(/databases/$(database)/documents/admins/$(request.auth.token.email.lower())); + && exists(/databases/$(database)/documents/admins/$(request.auth.token.email.lower())) + && adminDoc().data.get('active', true) == true; + } + + /** An owner is an admin who may also change the roster. THE ONLY PRIVILEGE THAT + * GRANTS PRIVILEGES, kept to two or three people who are seeded by hand in the + * Firebase console. + * + * The tier exists because the alternatives were both bad: console-only appointment + * means every monthly addition waits on whoever holds console access, and that + * person eventually graduates; letting any admin appoint any admin means one + * compromised college account can appoint accomplices and demote everybody else. + * This bounds the blast radius — a compromised admin cannot escalate at all, and + * there are few owners, chosen deliberately. + * + * `role` defaults to 'admin' for the same compatibility reason as `active`: an + * existing hand-seeded row must not silently become an owner. Nobody is an owner + * until somebody is explicitly made one in the console. */ + function isOwner() { + return isAdmin() && adminDoc().data.get('role', 'admin') == 'owner'; } /** Fields a member may never change after the first save. `created_at` is what @@ -142,6 +215,91 @@ service cloud.firestore { && (!('created_at' in d) || d.created_at == request.time || d.created_at == resource.data.created_at); } + /** The application shape. Mirrors web/lib/applications.ts and web/content/join.ts. + * + * THIS BACKS THE ONLY UNAUTHENTICATED WRITE IN THIS FILE, which changes what the + * validation is for. On a profile the rules are a backstop behind a signed-in member + * who has no reason to send anything strange. Here there is no identity at all, so + * this function IS the whole check: `allow create` with anything less is a public + * document store with the club's quota behind it. + * + * Three things follow from that, and none of them are optional: + * + * * `hasOnly` bounds the KEYS, so a submitter cannot append fields of their own — + * including, say, an `approved` or `isAdmin` field that a future careless rule, + * or an organiser's eye, might read as meaningful. + * * every string is length-bounded, because the form's `maxLength` is a courtesy to + * the reader and a direct SDK call never sees the form. Without these, one request + * can park a megabyte in a single field. + * * `submitted_at == request.time` pins the clock to the server, so submission order + * cannot be forged even though every other value here comes from a stranger. + * + * IT DOES NOT REQUIRE AN @sst.scaler.com ADDRESS, and that is deliberate rather than + * forgotten. Applying is asking; somebody who applies from a personal address is a + * person to email back, not a forgery to reject. The domain rule belongs on + * membership — `isMember()` — which is what actually grants anything. + * + * THE FIELD LIST IS THE PROFILE'S MINUS THE MEMBER-ONLY PARTS, and the four fields cut + * from sign-up — why, heard_from, interests, updates — are absent here too. + * `npm run rules` fails if any of them reappear in one file and not the other. */ + function isWellFormedApplication(d) { + return + d.keys().hasOnly([ + 'name', 'email', 'year_branch', 'hostel', 'github', + 'level', 'path', 'programs', 'programs_other', 'submitted_at' + ]) + && d.keys().hasAll([ + 'name', 'email', 'year_branch', 'hostel', + 'level', 'path', 'programs', 'submitted_at' + ]) + + // Required strings, bounded. + && d.name is string && d.name.size() > 0 && d.name.size() <= 120 + && d.email is string && d.email.size() > 3 && d.email.size() <= 200 + // Shape only, never deliverability — no rule can check that. This rejects the + // obviously-not-an-address, which is all the form's type="email" claims either. + && d.email.matches('^[^@\\s]+@[^@\\s]+[.][^@\\s]+$') + && d.year_branch is string && d.year_branch.size() > 0 && d.year_branch.size() <= 120 + + // Optional. Present-or-absent rather than nullable, so an absent github + // unambiguously means "not given" and the stored shape stays predictable. + && (!('github' in d) || (d.github is string && d.github.size() <= 100)) + + // Closed sets — see the drift warning in the header. These are the same three lists + // isWellFormedProfile checks and they have to stay identical: both forms render + // from one content file, so a value accepted on one screen and refused on the other + // is a bug invisible in either file alone. + && d.hostel in ['uniworld-1', 'uniworld-2'] + && d.level in ['none', 'some-git', 'merged'] + && d.path in ['build-day', 'first-contribution', 'fast-track', 'program-track'] + + // Programmes: required and non-empty, which is the form's rule too. Enforced here + // as well because a direct SDK call never sees the form, and an application with an + // empty programmes list would read as a UI bug to whoever opens it rather than as + // the forgery it is. + && d.programs is list + && d.programs.size() > 0 + && d.programs.size() <= 10 + && d.programs.hasOnly([ + 'gsoc', 'lfx', 'outreachy', 'sok', 'hacktoberfest', 'sob', + 'gssoc', 'ssoc', 'esoc', 'other' + ]) + // 'other' and its free text are a pair, checked in BOTH directions: no bare 'other' + // with nothing explaining it, and no stray text without the tick that is supposed + // to have produced it. + && (!d.programs.hasAny(['other']) + || ('programs_other' in d + && d.programs_other is string + && d.programs_other.size() > 0)) + && (!('programs_other' in d) + || (d.programs_other is string + && d.programs_other.size() <= 120 + && d.programs.hasAny(['other']))) + + // The server's clock, not the submitter's. + && d.submitted_at == request.time; + } + // ---------------------------------------------------------------- members match /users/{uid} { @@ -172,27 +330,358 @@ service cloud.firestore { // ----------------------------------------------------------------- admins + /** A roster row. Holds the access grant AND the membership half of the person's public + * billing, because + * the club was otherwise maintaining the same humans in two places — this collection + * and the hardcoded team list in web/content/club.ts — which drift silently: somebody + * who left still has access, or somebody new cannot get in but appears on the site. + * + * WHAT IS DELIBERATELY NOT HERE is the editorial: `remit` and `highlights` on the + * team page are reviewed prose with rules of their own ("a remit rather than a bio", + * "no pronouns"), and a field somebody types into an admin form at 11pm is not + * reviewed prose. Those stay in web/content/team-editorial.ts, keyed by the same + * address, and the two are merged at build time. + * + * The public half of this row reaches /team through that BUILD-TIME projection, never + * a public read. Firestore rules are per-document, not per-field, so a page reading + * this collection directly would publish every organiser's address along with their + * name — the document id IS an email. */ + function isWellFormedAdmin(d, email) { + return d.keys().hasOnly([ + 'email', 'name', 'title', 'photo', 'role', 'active', 'group', 'batch', + 'github', 'shadow_of', 'added_at', 'added_by', 'updated_at' + ]) + && d.keys().hasAll(['email', 'name', 'role', 'active', 'added_by', 'updated_at']) + // The id and the field must agree, so a row cannot grant access to one address + // while claiming to be about another. + && d.email == email + && d.name is string && d.name.size() > 0 && d.name.size() <= 120 + // `title` is the OFFICE, not a description of the person -- "President", "Repo + // Lead". It is what the team page prints above the name. + && (!('title' in d) || (d.title is string && d.title.size() <= 120)) + && (!('photo' in d) || (d.photo is string && d.photo.size() <= 300)) + // Which tier of the chart they belong to. A closed set, because the team page + // renders one section per value and an unknown one would silently drop somebody + // off the page entirely. + && (!('group' in d) || d.group in ['officer', 'lead', 'shadow']) + // Graduating batch, written as it is said out loud: "'28". + && (!('batch' in d) || (d.batch is string && d.batch.size() <= 8)) + && (!('github' in d) || (d.github is string && d.github.size() <= 100)) + // The office a shadow shadows. Free text rather than a reference, because it + // names a DESIGNATION ("Repo Lead") and not a person -- the whole point of a + // shadow is that the office outlives whoever currently holds it. + && (!('shadow_of' in d) || (d.shadow_of is string && d.shadow_of.size() <= 120)) + && d.role in ['owner', 'admin'] + && d.active is bool + && d.updated_at == request.time; + } + match /admins/{email} { - // You may read exactly your OWN row, which is how the client discovers whether to - // show the dashboard link. Scoped to the caller's address so this cannot be used - // to enumerate who the admins are. - allow get: if isMember() && request.auth.token.email.lower() == email; - allow list: if false; - // Managed by hand in the Firebase console. Denied to every client so that a - // compromised admin session cannot appoint further admins — the one privilege - // escalation this model would otherwise allow. + // Your OWN row, which is how the client discovers whether to show the organisers' + // page. Admins may also read any single row, because the roster UI needs it. + allow get: if isMember() && (request.auth.token.email.lower() == email || isAdmin()); + + // LISTABLE BY ADMINS, where it was listable by nobody. That was the right default + // while this collection was a bare access marker and the only question anyone could + // ask of it was "am I in it" — enumeration would have leaked who the organisers + // are to any member. It now carries the roster the organisers manage, so they have + // to be able to see it. Members still cannot: the clause is isAdmin(), not + // isMember(). + allow list: if isAdmin(); + + // OWNERS ONLY, and never your own row. + // + // The self-edit block is not about trust, it is about lockout: an owner who + // demotes or retires themselves cannot undo it, because the undo needs the + // privilege they just gave up. Making the roster the one thing you cannot use on + // yourself removes the whole class of accident. + // + // WHAT THIS STILL CANNOT PREVENT is the last owner being retired by another owner, + // because rules cannot count documents — there is no way to ask "how many owners + // remain". The Firebase console is the documented recovery path, and it is why + // owners are seeded there rather than here. + allow create: if isOwner() + && request.auth.token.email.lower() != email + && isWellFormedAdmin(request.resource.data, email) + // The byline is pinned to whoever is actually signed in, exactly as a notice's + // author_email is, so a grant cannot be attributed to somebody else. + && request.resource.data.added_by == request.auth.token.email + && (!('added_at' in request.resource.data) + || request.resource.data.added_at == request.time); + + allow update: if isOwner() + && request.auth.token.email.lower() != email + && isWellFormedAdmin(request.resource.data, email) + // Who granted access, and when, are history rather than fields — frozen on the + // first write for the same reason a profile's created_at is. + && request.resource.data.added_by == resource.data.added_by + && (!('added_at' in request.resource.data) + || request.resource.data.added_at == resource.data.added_at); + + // Nobody deletes a roster row from any client, including an owner. Retiring + // somebody is `active: false`, which revokes just as fast and keeps the record of + // who ran the club when. See the note on isAdmin(). + allow delete: if false; + } + + // ---------------------------------------------------------- announcements + + /** The shape of a post. Bounded for the same reason the profile is: the composer's + * maxlength is a courtesy to whoever is typing, and a direct SDK call never sees a + * form at all. */ + function isWellFormedPost(d) { + return d.keys().hasOnly([ + 'title', 'body', 'link', 'pinned', 'category', 'archived', + 'author_email', 'created_at', 'updated_at' + ]) + && d.keys().hasAll(['title', 'body', 'pinned', 'author_email', 'updated_at']) + // OPTIONAL WITH A DEFAULT, NOT REQUIRED, because notices posted before these two + // fields existed must keep working: a strict `hasAll` would make every one of + // them uneditable the moment these rules deployed, and the failure would present + // as "the board is broken" rather than as a migration nobody ran. + && (!('category' in d) || d.category in ['general', 'event', 'deadline']) + // ARCHIVED IS A HIDE, NOT A DELETE. `allow delete` stays open below for a notice + // posted by mistake, but the ordinary way to take one down is this — it keeps + // what the club said last term, which is the only record of it there is. + && (!('archived' in d) || d.archived is bool) + && d.title is string && d.title.size() > 0 && d.title.size() <= 120 + && d.body is string && d.body.size() > 0 && d.body.size() <= 2000 + && (!('link' in d) || (d.link is string && d.link.size() <= 300 && d.link.matches('^https://.*'))) + && d.pinned is bool + // The byline is pinned to the signed-in organiser, exactly as the profile's + // email is. An admin cannot post under a colleague's name. + && d.author_email == request.auth.token.email + && d.updated_at == request.time + && (!('created_at' in d) || d.created_at == request.time || d.created_at == resource.data.created_at); + } + + match /announcements/{id} { + // EVERY MEMBER READS, ONLY ORGANISERS WRITE. This is the first collection in this + // file that any member may list, and it is deliberate: a notice board that only + // its authors can read is a notice board with no readers. Nothing personal goes in + // one — no addresses, no names beyond the organiser's own byline — so a member + // reading the whole collection reveals nothing a member should not see. + allow get, list: if isMember(); + + // ADMINS WRITE FROM THE CLIENT, which is a departure from `admins` below and worth + // stating rather than leaving as an inconsistency. Adminship is still console-only, + // because that is the privilege that grants privileges. Posting a notice is not: + // it is ordinary organiser work that happens weekly, and routing it through the + // Firebase console would mean nobody ever posts. + allow create: if isAdmin() && isWellFormedPost(request.resource.data); + allow update: if isAdmin() + && isWellFormedPost(request.resource.data) + && request.resource.data.created_at == resource.data.created_at; + // Deletable, unlike a profile. A notice posted with the wrong date is a mistake + // somebody has to be able to take back, and there is no roster to wipe here. + allow delete: if isAdmin(); + } + + // ---------------------------------------------------------- contributions + + match /contributions/{uid} { + // Same get/list split, and the same reason, as users/{uid}: a member reads their + // own row, an organiser may query the collection. + allow get: if isMember() && (request.auth.uid == uid || isAdmin()); + allow list: if isAdmin(); + + // NO CLIENT WRITES THIS, INCLUDING ITS OWNER. That is the whole point of the + // collection: these counts come from GitHub via a Cloud Function using the Admin + // SDK, which bypasses these rules. If a member could write here, "merged pull + // requests" would be a self-reported number and the panel would be worthless — + // which is exactly what it would become the first time somebody edited it. allow write: if false; } - // ---------------------------------------------------- legacy applications + // ------------------------------------------------------------------- forms + // + // ONE COLLECTION FOR FORMS AND POLLS, because a poll IS a form: one question, a + // closed set of answers, and a count shown back. Building them separately would mean + // two schemas, two sets of rules, two admin screens and two places to fix the same + // bug — so `show_tally` is the whole difference between them. + // + // WHAT THE RULES CAN AND CANNOT CHECK HERE, stated plainly because the asymmetry + // looks like an oversight otherwise. Firestore rules cannot iterate a list of maps, + // so the SHAPE of `fields` is not validated element by element — an admin defining a + // form is trusted with its own structure, exactly as they are trusted with the text + // of a notice. What IS enforced is everything a member could otherwise forge: + // whose response it is, that there is at most one per person, that it answers this + // form's questions and no invented ones, and that it arrives while the form is open. + // + // `field_ids` IS A FLAT LIST ALONGSIDE `fields`, AND IT IS LOAD-BEARING. It is the + // only way a response rule can check the answer keys without iterating the nested + // field definitions — `answers.keys().hasOnly(field_ids)` is one cheap comparison. + // The admin client writes both from one source, so they cannot disagree. + + function isWellFormedForm(d) { + return d.keys().hasOnly([ + 'title', 'description', 'fields', 'field_ids', 'open', 'show_tally', + 'tally', 'closes_at', 'author_email', 'created_at', 'updated_at' + ]) + && d.keys().hasAll(['title', 'fields', 'field_ids', 'open', 'show_tally', + 'author_email', 'updated_at']) + && d.title is string && d.title.size() > 0 && d.title.size() <= 120 + && (!('description' in d) || (d.description is string && d.description.size() <= 1000)) + && d.fields is list && d.fields.size() > 0 && d.fields.size() <= 20 + && d.field_ids is list && d.field_ids.size() == d.fields.size() + && d.open is bool + && d.show_tally is bool + // The byline is pinned to the signed-in organiser, as a notice's is. + && d.author_email == request.auth.token.email + && d.updated_at == request.time; + } + + match /forms/{formId} { + // Every member reads the forms; that is the point of them. + allow get, list: if isMember(); + + // THE TALLY IS NOT WRITABLE BY ANY CLIENT, INCLUDING THE ADMIN WHO MADE THE FORM. + // It is recomputed by a Cloud Function from the responses themselves, for the same + // reason contribution counts are: a number the client supplies is a number the + // client invented, and "42 people voted for Saturday" would then be worth nothing. + // On create it must be absent; on update it must come back exactly as stored. + allow create: if isAdmin() + && isWellFormedForm(request.resource.data) + && !('tally' in request.resource.data) + && (!('created_at' in request.resource.data) + || request.resource.data.created_at == request.time); + + allow update: if isAdmin() + && isWellFormedForm(request.resource.data) + && (!('tally' in request.resource.data) + || request.resource.data.tally == resource.data.tally) + && request.resource.data.author_email == resource.data.author_email + && (!('created_at' in request.resource.data) + || request.resource.data.created_at == resource.data.created_at); + + // Deleting a form would orphan its responses, which is somebody's answer to a + // question they were asked. Closing it (`open: false`) is the reversible thing, and + // it is what the UI offers. + allow delete: if false; + + match /responses/{uid} { + // KEYED BY uid, which is what makes "one response per member" a structural + // guarantee rather than a check somebody forgets to write. A second response is + // not refused — it is impossible to express. + allow get: if isMember() && (request.auth.uid == uid || isAdmin()); + // Attributed, so organisers can see who answered and chase who has not. Members + // cannot list, so nobody can read anybody else's answers. + allow list: if isAdmin(); + + allow create, update: if isMember() + && request.auth.uid == uid + && request.resource.data.keys().hasOnly(['uid', 'email', 'name', 'answers', 'submitted_at', 'updated_at']) + && request.resource.data.keys().hasAll(['uid', 'email', 'answers', 'updated_at']) + && request.resource.data.uid == uid + // Pinned to the signed-in address, exactly as a profile's is, so a response + // cannot be filed under somebody else's name. + && request.resource.data.email == request.auth.token.email + && request.resource.data.answers is map + // Only this form's questions. Without it a member could append arbitrary keys + // to a document the organisers read and export. + && request.resource.data.answers.keys() + .hasOnly(get(/databases/$(database)/documents/forms/$(formId)).data.field_ids) + // CLOSED MEANS CLOSED. Checked here rather than only in the UI, because a + // disabled button is a rendering decision and a deadline that only the browser + // enforces is not a deadline. + && get(/databases/$(database)/documents/forms/$(formId)).data.open == true + && request.resource.data.updated_at == request.time; + + // A member may change their answer while the form is open, but not withdraw it — + // the organisers have already counted it, and a disappearing response reads as a + // bug rather than as a decision. + allow delete: if false; + } + } + + // ---------------------------------------------------------------- sessions + // + // WHAT THE NOTICE BOARD COULD NOT DO. A session is not a notice: it has a time, it + // stops being upcoming, and the question a member asks of it is "what is next" rather + // than "what was said". Posted as notices, the next session sank down the board as + // soon as anything else was posted, and nothing could sort by when it actually is. + // + // `starts_at` IS THE WHOLE POINT, so it is required and it is a real timestamp rather + // than a typed string. A club that writes "Saturday 4pm" into a text field cannot ask + // for the next three sessions, and that is the only query this collection exists to + // answer. + // + // NOT SERVER-STAMPED, unlike every other date in this file. `created_at` is + // `request.time` because a client must not be able to backdate a record; `starts_at` + // is chosen by the organiser and is frequently in the future, so the same rule would + // make it impossible to schedule anything. + + function isWellFormedSession(d) { + return d.keys().hasOnly([ + 'title', 'speaker', 'location', 'notes', 'starts_at', + 'created_by', 'created_at', 'updated_at' + ]) + && d.keys().hasAll(['title', 'starts_at', 'created_by', 'updated_at']) + && d.title is string && d.title.size() > 0 && d.title.size() <= 120 + && (!('speaker' in d) || (d.speaker is string && d.speaker.size() <= 120)) + && (!('location' in d) || (d.location is string && d.location.size() <= 120)) + && (!('notes' in d) || (d.notes is string && d.notes.size() <= 1000)) + && d.starts_at is timestamp + && d.created_by == request.auth.token.email + && d.updated_at == request.time; + } + + match /sessions/{id} { + // Every member reads them; knowing when the club meets is the point. + allow get, list: if isMember(); + + allow create: if isAdmin() + && isWellFormedSession(request.resource.data) + && (!('created_at' in request.resource.data) + || request.resource.data.created_at == request.time); + + allow update: if isAdmin() + && isWellFormedSession(request.resource.data) + && request.resource.data.created_by == resource.data.created_by + && (!('created_at' in request.resource.data) + || request.resource.data.created_at == resource.data.created_at); + + // DELETABLE, unlike a form. A session carries no member's answer — nothing is lost + // by removing one, and a cancelled session that cannot be taken off the list is a + // club telling its members to turn up to a room nobody booked. + allow delete: if isAdmin(); + } + + // ------------------------------------------------------------ applications match /applications/{id} { - // Nothing writes here any more; the profile replaced it. Reads stay denied - // because these rows hold the same personal details as a profile, and the rows - // stay immutable so the history is intact. + // NOBODY READS THIS FROM A CLIENT, INCLUDING ADMINS, and that is a decision rather + // than an omission. Every other collection here belongs to somebody who signed in and + // chose to be a member. These rows belong to people who are not members yet, who have + // no account to manage them with and no way to ask for them back — so the surface is + // the smallest one that still lets the club act on them: organisers read applications + // in the Firebase console, where access is governed by IAM and every read is logged. + // + // Stated outright rather than left to deny-by-default, so anybody loosening it has to + // delete a line that says false rather than add one that was never there. An + // organisers' view inside the site would need its own admin-only get/list AND a + // decision about how long these are kept — not a quiet edit to this line. allow read: if false; - allow create: if false; + + // IMMUTABLE ONCE SUBMITTED. An applicant has no identity to prove ownership with, so + // any rule permitting edits could only ever be "anyone may edit anyone's" — which + // would let a stranger rewrite or delete somebody else's application. allow update, delete: if false; + + // ANYONE MAY SUBMIT EXACTLY ONE WELL-FORMED APPLICATION, WITH NO ACCOUNT. This is the + // club's front door and the only unauthenticated write in this file. + // + // VALIDATED, NOT ACCEPTED: an unvalidated create-only collection is a free document + // store on somebody else's quota. The key and size limits in the helper above are the + // only thing between this rule and a script filling the project overnight. + // + // THE OTHER HALF OF PROTECTING THIS IS NOT IN THIS FILE. Rules can say what a + // document must look like; they cannot say the request came from this site rather than + // from a loop. App Check attests that — it is wired up in web/lib/firebase.ts, and + // ENFORCEMENT IS A SWITCH IN THE FIREBASE CONSOLE, which is the one part of defending + // this endpoint that reviewing this repo cannot verify. If junk applications ever + // start arriving, check that switch before changing anything here. + allow create: if isWellFormedApplication(request.resource.data); } // Everything else is closed. Deny-by-default would already do this; the explicit diff --git a/functions/README.md b/functions/README.md index 88703c4..3da64e4 100644 --- a/functions/README.md +++ b/functions/README.md @@ -1,53 +1,117 @@ -# functions/ — not deployed, on purpose - -This directory contains one Cloud Function that would email the organisers whenever -somebody applies. **It is deliberately not in use.** Applications are read in the -Firebase console instead. - -## Why it is switched off - -Cloud Functions cannot make outbound network calls on Firebase's free **Spark** plan, -and sending email is an outbound call. So this function requires the **Blaze** -(pay-as-you-go) plan, which requires a card on file — even though a student club's -volume sits inside the free allowance and the bill would in practice be zero. - -The club chose to stay on the free plan and read submissions in the console. That loses -nothing: every field of every application is stored and visible there. - -## It cannot fire by accident - -`firebase.json` does **not** declare a `functions` block, so `firebase deploy` ignores -this directory entirely. Nothing here runs, costs anything, or affects the site. The -form does not depend on it — the email was always downstream of the Firestore write, so -an applicant's submission succeeds whether or not any of this exists. - -## If you do want email later - -1. Upgrade the project to Blaze in the Firebase console. -2. Add a `functions` block to `firebase.json`: - ```json - "functions": { "source": "functions" } - ``` -3. Set the two secrets — they go to Google Secret Manager, never into this repo: - ```bash - firebase functions:secrets:set SMTP_URL # e.g. smtps://user%40gmail.com:app-password@smtp.gmail.com:465 - firebase functions:secrets:set MAIL_TO # where notifications land - ``` - URL-encode the username and password. An `@` in the username or a `/` in the - password will otherwise truncate the connection string and fail with a confusing - auth error. -4. `cd functions && npm install && cd .. && firebase deploy --only functions` +# functions/ + +Three Cloud Functions, in two unrelated jobs. **One of them is dormant and two are +live** — which half you care about depends on why you opened this directory. + +| Function | Status | What it does | +|---|---|---| +| `emailOnApplication` | **dormant** | emails the organisers when a row lands in `applications` | +| `syncContributions` | live | 04:00 IST daily, fetches members' GitHub pull requests | +| `refreshContributions` | live | the same fetch for one member, from the dashboard button | + +All three need the **Blaze** (pay-as-you-go) plan, because all three make outbound +network calls and Firebase's free **Spark** plan does not allow those. A student club's +volume sits well inside the free allowance and the bill is in practice zero, but a +billing account has to exist on the project. That is Google's restriction on egress, not +a choice made here. + +> **This directory used to say it could not fire by accident**, because `firebase.json` +> declared no `functions` block and `firebase deploy` therefore ignored it. That is no +> longer true — the block was added so the contribution sync could deploy. Everything +> here now deploys when you run `firebase deploy`, including the dormant one. + +--- + +## The GitHub sync (live) + +This is what fills the "Your open source" panel on `/dashboard`. + +### Why it is a function and not a fetch in the browser + +Two reasons, and each one alone would be enough: + +* **Rate limits.** An unauthenticated browser gets 60 GitHub requests an hour *per IP*, + which on a college network is 60 an hour for the entire club. +* **Trust.** A count the client writes is a count the client can invent. "Merged pull + requests" is the one number on that dashboard somebody has a reason to inflate, so + `contributions/{uid}` is `allow write: if false` for **every** client including its + owner, and only the Admin SDK — which bypasses the rules — writes it. + +### Setting it up + +```bash +firebase functions:secrets:set GITHUB_TOKEN # a token with NO scopes at all +firebase deploy --only functions +``` + +The token is **optional but not really**. It only ever reads public data, so no scopes +are needed — it exists purely for the rate limit. Without it GitHub allows 10 searches a +minute and 60 user lookups an *hour*, so the nightly sweep stops after about fifteen +members, and the failure arrives as a 403 that reads like a permissions problem. With it: +30 a minute and 5,000 an hour. + +### What it costs to run + +The sweep spends **two search requests and one user lookup per member**, paced one member +every five seconds, and stops at the 100 stalest rows per run. So a club of 100 is +refreshed daily and a club of 400 every four days, in a deterministic order — and the +dashboard's "checked N days ago" line is what makes that visible rather than mysterious. +If it ever needs to go faster, the cap and the gap are both named constants at the top of +`index.js`. + +The callable is rate-limited to one refresh per member per ten minutes, enforced in the +function rather than in the client, because a client-side timer is a suggestion. + +--- + +## The application email (dormant) + +It triggers on `applications/{id}`, which is the **legacy** collection from before +sign-in existed. Nothing writes there any more — the profile at `users/{uid}` replaced +it — so this function never fires today. + +It is kept rather than deleted because pointing it at `users/{uid}` is a decision about +whether organisers want an email per sign-up, not a tidy-up. If you make that decision, +change the `document` option in `index.js` and check `format.js` still matches the +profile's field list, which has lost four fields since this was written. + +To switch it on you also need its two secrets, which go to Google Secret Manager and +never into this repo: + +```bash +firebase functions:secrets:set SMTP_URL # e.g. smtps://user%40gmail.com:app-password@smtp.gmail.com:465 +firebase functions:secrets:set MAIL_TO # where notifications land +``` + +URL-encode the username and password. An `@` in the username or a `/` in the password +will otherwise truncate the connection string and fail with a confusing auth error. `SMTP_URL` is a connection string rather than a vendor SDK, so Gmail with an app password, the domain's own mailbox, SendGrid, Mailgun and Resend all work without changing the code. -## Testing it without any of that +--- + +## Testing, without Firebase, billing, a token or a deploy ```bash cd functions && npm test ``` -20 assertions against the formatter — the part with all the logic and the only part an -organiser sees. Needs no Firebase, no billing, no SMTP and no deploy. It prints a sample -email at the end so you can read what an organiser would receive. +Two suites, both pure: + +* **`test-format.mjs`** — 20 assertions against the email formatter, the part with all + the logic and the only part an organiser ever sees. It prints a sample email at the + end so you can read what would arrive. +* **`test-github.mjs`** — 33 assertions against `github.js`, driven by recorded GitHub + payloads with `fetch` stubbed out. The cases worth knowing about are the ones that are + easy to get wrong and impossible to notice: a real account with no pull requests versus + a handle that does not exist (they look almost identical from the API and the dashboard + has to word them completely differently), the distinct-repository count (derived, not + returned — an earlier version counted only the eight rows the dashboard lists and + produced a number that looked entirely plausible), and that a rate limit **throws** + rather than resolving to zeroes, because a silent zero would overwrite a real member's + real counts. + +The rules that protect what these functions write are exercised separately, against the +real emulator, by `web/scripts/rules-emulator.mjs`. diff --git a/functions/github.js b/functions/github.js new file mode 100644 index 0000000..4fde2b3 --- /dev/null +++ b/functions/github.js @@ -0,0 +1,211 @@ +// Ask GitHub what one handle has contributed, and reduce the answer to the six values +// the dashboard shows. +// +// SEPARATE FROM index.js SO IT CAN BE TESTED WITHOUT FIREBASE. Everything here is a pure +// function of a fetch response, so test-github.mjs drives it with recorded payloads and +// no emulator, no credentials and no network. The same split as format.js. +// +// THE SEARCH API, NOT THE EVENTS API, and this is the decision the whole file rests on: +// +// /users/{handle}/events is the obvious endpoint and is useless here. It returns the +// last 90 days AND at most 300 events, so a member who +// contributed steadily for two years would show a partial +// quarter. It is an activity feed, not a history. +// /search/issues answers "every PR this account has ever opened, filtered by +// state" in one request, with a total count that does not lie +// about the tail. That is exactly the question. +// +// WHAT IT COSTS, AND WHICH LIMIT IT COSTS AGAINST. GitHub meters search separately from +// everything else, and this file touches both meters: +// +// /search/issues TWO requests per member (merged, then open), against the SEARCH +// limit — 10 per minute unauthenticated, 30 with a token. This is the +// binding constraint, and the throttle in index.js is sized against it. +// /users/{handle} ONE request per member, against the CORE limit — 60 per hour +// unauthenticated, 5,000 with a token. Effectively free with a token +// and the first thing to break without one. +// +// So a token is not optional past about fifteen members, and the failure without one +// arrives as a 403 that reads like a permissions problem. +// +// `is:merged` RATHER THAN `is:closed`. A closed PR is one that was rejected or abandoned +// just as often as one that landed, and counting those as contributions would make the +// number flattering and worthless. Open ones are counted separately and labelled as +// what they are: in flight. + +/** How many recent pull requests to store per member. The dashboard shows a list, not an + * archive — and every stored row is a row in a document that is read on every dashboard + * load, so this is a page-weight decision as much as a design one. */ +const RECENT = 8; + +/** Firestore's hard limit is 1MB per document; this is nowhere near it, but a title is + * attacker-influenced in the sense that anybody can open a PR with a 2000-character + * title on a public repo. Truncated so a single hostile title cannot bloat the row. */ +const MAX_TITLE = 160; + +/** GitHub's largest allowed page size on the search API. Asked for on the merged search + * so the distinct-repository count is derived from a full page rather than from the + * handful of rows the dashboard lists — see fetchContributions. */ +const PAGE_MAX = 100; + +/** GitHub handles: alphanumerics and single hyphens, 1-39 characters, no leading or + * trailing hyphen. Validated BEFORE it goes anywhere near a URL. + * + * This is the one piece of member-supplied text this function puts into a request, and + * it arrives from a free-text field on the profile. Encoding it would be enough to make + * it safe; rejecting it outright is better, because a handle that cannot be valid can + * never be found and the request is wasted. */ +const HANDLE = /^[a-zA-Z0-9](?:[a-zA-Z0-9]|-(?=[a-zA-Z0-9])){0,38}$/; + +function isValidHandle(h) { + return typeof h === "string" && HANDLE.test(h.trim()); +} + +/** "https://github.com/owner/name/pull/12" -> "owner/name". + * + * Derived from repository_url rather than from html_url: the former is a stable API + * field ("https://api.github.com/repos/owner/name"), the latter is a web URL whose shape + * GitHub is free to change. */ +function repoOf(item) { + const u = item?.repository_url ?? ""; + const m = u.match(/\/repos\/([^/]+\/[^/]+)$/); + return m ? m[1] : "unknown"; +} + +function toPull(item, state) { + const title = String(item?.title ?? "").slice(0, MAX_TITLE); + const pull = { + title: title || "(no title)", + repo: repoOf(item), + url: String(item?.html_url ?? ""), + state, + }; + // Omitted rather than written null when absent, so the stored shape matches the + // optional field in web/lib/contributions.ts. + if (item?.pull_request?.merged_at) pull.merged_at = item.pull_request.merged_at; + else if (item?.closed_at && state === "merged") pull.merged_at = item.closed_at; + return pull; +} + +/** One search request. Returns { total, items } or throws. + * + * `fetch` is global on Node 18+, which is what functions/package.json pins via + * engines.node — so there is no HTTP dependency in this file at all. + * + * A User-Agent is REQUIRED by GitHub: without one the API returns 403 with a message + * about it, which reads like a rate limit and is not one. */ +async function search(q, token, perPage) { + const url = + "https://api.github.com/search/issues" + + `?q=${encodeURIComponent(q)}&sort=updated&order=desc&per_page=${perPage}`; + + const headers = { + Accept: "application/vnd.github+json", + "X-GitHub-Api-Version": "2022-11-28", + "User-Agent": "scaler-open-source-club", + }; + if (token) headers.Authorization = `Bearer ${token}`; + + const res = await fetch(url, { headers }); + + if (res.status === 403 || res.status === 429) { + // Distinguished from a generic failure because the caller throttles on it. GitHub + // uses 403 for rate limiting on the search API, which is why this is not simply + // "not authorised". + const err = new Error("GitHub rate limit"); + err.code = "rate-limit"; + throw err; + } + if (!res.ok) { + const err = new Error(`GitHub returned ${res.status}`); + err.code = "github-down"; + throw err; + } + + const body = await res.json(); + return { total: Number(body?.total_count ?? 0), items: body?.items ?? [] }; +} + +/** Does this account exist? Asked separately so "no such handle" is distinguishable from + * "this person has no pull requests" — which look identical through the search API, and + * which the dashboard has to word completely differently. */ +async function exists(handle, token) { + const headers = { + Accept: "application/vnd.github+json", + "X-GitHub-Api-Version": "2022-11-28", + "User-Agent": "scaler-open-source-club", + }; + if (token) headers.Authorization = `Bearer ${token}`; + const res = await fetch(`https://api.github.com/users/${encodeURIComponent(handle)}`, { + headers, + }); + if (res.status === 404) return false; + if (res.status === 403 || res.status === 429) { + const err = new Error("GitHub rate limit"); + err.code = "rate-limit"; + throw err; + } + // Anything else that is not a clean 200 is treated as "exists", so a transient blip on + // this cheap check never wipes a member's real numbers with a not_found flag. + return true; +} + +/** + * Everything the dashboard shows for one handle. + * + * Returns the exact shape stored at contributions/{uid}, minus uid and synced_at, which + * the caller adds. Throws with `code` set to 'rate-limit' or 'github-down'; a handle that + * does not exist is NOT a throw, because it is a normal answer the UI renders. + */ +async function fetchContributions(handle, token) { + const h = String(handle ?? "").trim(); + if (!isValidHandle(h)) { + const err = new Error("Not a possible GitHub handle"); + err.code = "bad-handle"; + throw err; + } + + if (!(await exists(h, token))) { + return { github: h, merged: 0, open: 0, repos: 0, recent: [], not_found: true }; + } + + // Two requests, merged first. If the second is rate-limited the first is still thrown + // away — a row with real merged counts and a zeroed `open` would be a lie that looks + // like data, and the caller's retry is cheap. + // + // PAGE_MAX ON THE MERGED SEARCH, NOT RECENT, AND THAT IS NOT A TYPO. The list only + // shows RECENT rows, so asking for eight would seem to be enough — but `repos` is + // counted from the items in this response, and eight items can name at most eight + // repositories. Sized down to RECENT, a member with thirty merged PRs across twelve + // projects would read "8 projects touched" forever, and the number would look like a + // fact rather than a truncation. One page of 100 costs exactly the same one request. + const merged = await search(`type:pr is:merged author:${h}`, token, PAGE_MAX); + const open = await search(`type:pr is:open author:${h}`, token, 1); + + const all = merged.items.map((i) => toPull(i, "merged")); + + return { + github: h, + merged: merged.total, + open: open.total, + // Distinct repositories among the merged PRs this request returned. GitHub's search + // API has no distinct-repository count, so this is derived — and it is therefore + // EXACT up to PAGE_MAX merged pull requests and an undercount past it. A second page + // would be a second request against a 30-per-minute limit, for a member who has + // merged more than a hundred patches and does not need this dashboard to tell them + // they are active. If the club ever has such members, paginate here rather than + // quietly leaving the number wrong. + repos: new Set(all.map((p) => p.repo)).size, + recent: all.slice(0, RECENT), + }; +} + +module.exports = { + fetchContributions, + isValidHandle, + repoOf, + toPull, + RECENT, + MAX_TITLE, + PAGE_MAX, +}; diff --git a/functions/index.js b/functions/index.js index 2736dd9..2377abd 100644 --- a/functions/index.js +++ b/functions/index.js @@ -1,4 +1,21 @@ -// Email the organisers whenever somebody applies. +// The club's Cloud Functions. Four of them, in three unrelated jobs: +// +// emailOnApplication emails the organisers when a row lands in `applications`. +// syncContributions nightly, fetches every member's GitHub pull requests. +// refreshContributions the same fetch for one member, on demand from the dashboard. +// tallyResponses recounts a poll whenever somebody answers it. +// +// A NOTE ON THE FIRST ONE, because it is easy to read this file and assume it still +// fires. It triggers on `applications/{id}`, which is the LEGACY collection from before +// sign-in existed — nothing writes there any more; the profile at `users/{uid}` replaced +// it. So this function is currently dormant. It is kept rather than deleted because the +// rows already in that collection are still protected by it in the rules, and because +// pointing it at `users/{uid}` is a decision about whether organisers want an email per +// sign-up, not a tidy-up. Whoever makes that decision should change the `document` below +// and check format.js still matches the profile's field list. +// +// --------------------------------------------------------------------------------- +// EMAIL THE ORGANISERS WHENEVER SOMEBODY APPLIES. // // Triggered by the document being created, NOT by the form. That ordering is the whole // design: the applicant's submit succeeds or fails on the Firestore write alone, so if @@ -103,3 +120,315 @@ exports.emailOnApplication = onDocumentCreated( } }, ); + +// --------------------------------------------------------------------------------- +// GITHUB CONTRIBUTIONS +// +// Two entry points onto the same work: a scheduled sweep that keeps every member's row +// warm, and a callable so a member who merged something an hour ago does not have to wait +// for tomorrow. +// +// WHY THIS IS A FUNCTION AND NOT A FETCH IN THE BROWSER. Both halves of the answer are in +// web/lib/contributions.ts, and both are decisive: a browser gets 60 unauthenticated +// GitHub requests an hour PER IP, which on a college network is 60 an hour for the whole +// club; and a count the client writes is a count the client can invent. The Admin SDK +// here bypasses firestore.rules, which is exactly why `contributions/{uid}` is +// `allow write: if false` for every client including its owner. +// +// CREDENTIALS. One optional secret: +// +// firebase functions:secrets:set GITHUB_TOKEN +// +// A fine-grained personal access token with NO scopes at all is enough — this only reads +// public data, and the token is for the rate limit rather than for access. Without it the +// sweep still runs and simply gets much less far before GitHub refuses; with it, the +// search limit goes from 10 requests a minute to 30. + +const { onSchedule } = require("firebase-functions/v2/scheduler"); +const { onCall, HttpsError } = require("firebase-functions/v2/https"); +const admin = require("firebase-admin"); + +const { fetchContributions, isValidHandle } = require("./github"); + +const GITHUB_TOKEN = defineSecret("GITHUB_TOKEN"); + +// Initialised once, at module load, guarded because both functions in this file share the +// process and a second initializeApp() throws. +if (!admin.apps.length) admin.initializeApp(); + +/** Collection names, which MUST match web/lib/firebase.ts. Different strings here means + * the sweep writes rows the dashboard never reads — and nothing fails, which is what + * makes it worth stating rather than inlining. `npm run rules` asserts these. */ +const USERS = "users"; +const CONTRIBUTIONS = "contributions"; +const FORMS = "forms"; +const RESPONSES = "responses"; + +/** Two search requests per member against a 30-per-minute authenticated limit means one + * member every 4.5 seconds is the ceiling. 5s leaves a little headroom. */ +const GAP_MS = 5000; + +/** How many members one scheduled run will refresh. + * + * A CAP RATHER THAN A LONGER TIMEOUT, and the cap is the honest half of the design. + * Cloud Functions v2 stops at 3,600s; at GAP_MS per member a full club sweep would + * approach that as membership grows, and a run killed mid-sweep leaves an arbitrary half + * of the club stale with nothing saying so. Instead each run takes the 100 STALEST rows, + * so the whole club is covered every ceil(n/100) days in a deterministic order. At 100 + * members that is daily; at 400 it is every four days, and the dashboard's "checked N + * days ago" line is what makes that visible rather than mysterious. */ +const MAX_PER_RUN = 100; + +/** A member may ask for a refresh this often. Enforced HERE rather than in the client, + * because a client-side timer is a suggestion — and this is the only path by which a + * signed-in member can cause outbound requests, so it is the one that needs a limit. */ +const COOLDOWN_MS = 10 * 60 * 1000; + +const sleep = (ms) => new Promise((r) => setTimeout(r, ms)); + +/** Fetch one member's contributions and store them. + * + * `synced_at` is written even for a not_found handle, and deliberately: without it a + * typo'd handle would be retried on every run forever, and the cooldown would never + * engage for the person most likely to press the button repeatedly. */ +async function syncOne(uid, handle, token) { + const data = await fetchContributions(handle, token); + await admin + .firestore() + .collection(CONTRIBUTIONS) + .doc(uid) + .set({ uid, ...data, synced_at: admin.firestore.FieldValue.serverTimestamp() }); +} + +/** Read a row back as plain JSON, for handing to a callable's caller. + * + * NEEDED BECAUSE THE WRITE ABOVE STORES A SENTINEL, not a time. Returning the object + * that was written hands the client `{_methodName: "serverTimestamp"}`, which toDate() + * cannot read and which renders as "checked never" immediately after a successful sync — + * the one moment the reader is looking straight at it. */ +async function readOne(uid) { + const snap = await admin.firestore().collection(CONTRIBUTIONS).doc(uid).get(); + if (!snap.exists) return null; + const d = snap.data(); + return { + ...d, + // A Firestore Timestamp does not survive the callable's JSON encoding as anything + // useful, so it crosses as ISO and toDate() parses the string on the other side. + synced_at: d.synced_at?.toDate?.().toISOString() ?? null, + }; +} + +exports.syncContributions = onSchedule( + { + // 04:00 IST — the middle of the night for the only people who use this. The timezone + // is stated rather than left at UTC so the schedule means what it reads. + schedule: "0 4 * * *", + timeZone: "Asia/Kolkata", + secrets: [GITHUB_TOKEN], + timeoutSeconds: 3600, + // ONE INSTANCE, ALWAYS. Two overlapping sweeps would double the request rate against + // a limit this is carefully sized under, and the throttle would mean nothing. + maxInstances: 1, + retryCount: 0, + }, + async () => { + const token = GITHUB_TOKEN.value() || ""; + if (!token) { + logger.warn( + "GITHUB_TOKEN is not set. The sweep runs against the unauthenticated limit " + + "(10 searches a minute) and will not get far. Set it with: " + + "firebase functions:secrets:set GITHUB_TOKEN", + ); + } + + const db = admin.firestore(); + // Every member who gave a handle. `github` is optional on the profile, so this is a + // subset of the roster. + const members = await db.collection(USERS).get(); + const withHandles = members.docs + .map((d) => ({ uid: d.id, handle: (d.data().github ?? "").trim() })) + .filter((m) => isValidHandle(m.handle)); + + // STALEST FIRST, so MAX_PER_RUN rotates through the club rather than refreshing the + // same hundred people every night. Never-synced rows sort first, having no timestamp. + const existing = await db.collection(CONTRIBUTIONS).get(); + const syncedAt = new Map( + existing.docs.map((d) => [d.id, d.data().synced_at?.toMillis?.() ?? 0]), + ); + withHandles.sort((a, b) => (syncedAt.get(a.uid) ?? 0) - (syncedAt.get(b.uid) ?? 0)); + + const batch = withHandles.slice(0, MAX_PER_RUN); + logger.info( + `Syncing ${batch.length} of ${withHandles.length} members with a GitHub handle.`, + ); + + let done = 0; + let failed = 0; + for (const m of batch) { + try { + await syncOne(m.uid, m.handle, token); + done++; + } catch (err) { + failed++; + if (err.code === "rate-limit") { + // STOP, DO NOT CARRY ON. Once GitHub is refusing, every further request in this + // run is also refused — continuing turns one rate limit into a hundred, and the + // remaining members are picked up tomorrow anyway because they are now the + // stalest rows. + logger.warn(`Rate limited after ${done} members. Stopping; the rest are next run.`); + break; + } + // Anything else is one member's problem, not the sweep's. Logged and skipped. + logger.error("Could not sync one member.", { + uid: m.uid, + error: err instanceof Error ? err.message : String(err), + }); + } + await sleep(GAP_MS); + } + + logger.info(`Sync finished. ${done} updated, ${failed} failed.`); + }, +); + +exports.refreshContributions = onCall( + { secrets: [GITHUB_TOKEN], timeoutSeconds: 60, maxInstances: 10 }, + async (request) => { + const auth = request.auth; + // THE UID COMES FROM THE TOKEN, NEVER FROM A PARAMETER. That is the whole reason this + // is a callable rather than an HTTP endpoint: a uid passed in the body is a uid + // somebody can change, and this would become "refresh anybody's row". + if (!auth?.uid) throw new HttpsError("unauthenticated", "Sign in first."); + + // THE SAME DOMAIN RULE AS firestore.rules, RESTATED — because the Admin SDK below + // does not go through the rules at all. Without this, a Google account on any domain + // could call this function and spend the club's GitHub rate limit. If the domain in + // web/lib/firebase.ts changes, it changes here too. + const email = String(auth.token?.email ?? "").toLowerCase(); + if (auth.token?.email_verified !== true || !/^[^@]+@sst\.scaler\.com$/.test(email)) { + throw new HttpsError("permission-denied", "Members only."); + } + + const db = admin.firestore(); + const profile = await db.collection(USERS).doc(auth.uid).get(); + const handle = (profile.data()?.github ?? "").trim(); + if (!isValidHandle(handle)) return { ok: false, reason: "no-handle" }; + + const current = await db.collection(CONTRIBUTIONS).doc(auth.uid).get(); + const last = current.data()?.synced_at?.toMillis?.() ?? 0; + // The cooldown is SKIPPED when the stored row is for a different handle. Somebody who + // has just corrected a typo should not be told to wait ten minutes to see the fix — + // that is the one moment they will certainly press the button twice. + const sameHandle = + (current.data()?.github ?? "").trim().toLowerCase() === handle.toLowerCase(); + if (sameHandle && Date.now() - last < COOLDOWN_MS) { + return { ok: false, reason: "cooldown" }; + } + + try { + await syncOne(auth.uid, handle, GITHUB_TOKEN.value() || ""); + const stored = await readOne(auth.uid); + if (stored?.not_found) return { ok: false, reason: "not-found" }; + return { ok: true, contributions: stored }; + } catch (err) { + // Logged with the distinction, returned without it: "we are being throttled" and + // "GitHub is down" are different things to whoever reads the logs and the same + // sentence to a member, who can only try again either way. + logger.error("Refresh failed.", { + uid: auth.uid, + code: err.code ?? "unknown", + error: err instanceof Error ? err.message : String(err), + }); + return { ok: false, reason: "github-down" }; + } + }, +); + +// --------------------------------------------------------------------------------- +// POLL TALLIES +// +// Recomputes forms/{formId}.tally whenever a response is written. +// +// WHY A FUNCTION AND NOT A CLIENT-SIDE COUNTER. `tally` is refused to every client by +// firestore.rules — including the admin who created the form — because a count the client +// supplies is a count the client invented. The obvious alternative, letting a member +// increment a counter as they vote, cannot be made safe in rules: a rule can check that +// the new count is the old count plus one, but it CANNOT check that the increment came +// with an actual vote, so anybody could push a number up without answering anything. +// +// IT RECOMPUTES FROM SCRATCH RATHER THAN INCREMENTING, and that is the decision worth +// defending. Incrementing is O(1) and wrong in three ordinary situations: a member +// changing their answer (decrement the old, increment the new), a retried function +// invocation (double count), and any write that lands while another is in flight. Reading +// every response and counting is O(n) per write, which for a club poll is a few hundred +// documents — and it is ALWAYS right, including after a bug, because it derives the +// answer rather than accumulating it. If the club ever runs a poll with tens of thousands +// of responses, revisit this; at that point a distributed counter is the standard answer. +// +// ONLY `choice` AND `multi` FIELDS ARE COUNTED. Tallying free text would produce one +// bucket per person, which is not a tally. + +const { onDocumentWritten } = require("firebase-functions/v2/firestore"); + +exports.tallyResponses = onDocumentWritten( + { + document: "forms/{formId}/responses/{uid}", + // ONE AT A TIME PER DEPLOYMENT. Two invocations recomputing the same form + // concurrently would both read, both count, and the slower one would write a total + // that is missing the other's response. Serialising costs latency nobody perceives on + // a club poll and removes the whole class of lost update. + maxInstances: 1, + retry: false, + timeoutSeconds: 120, + }, + async (event) => { + const formId = event.params.formId; + const db = admin.firestore(); + + const formRef = db.collection(FORMS).doc(formId); + const form = await formRef.get(); + if (!form.exists) { + logger.warn("A response was written to a form that does not exist.", { formId }); + return; + } + + const data = form.data(); + // Nothing to show, nothing to compute. A sign-up sheet is not a poll and its + // organisers read the responses directly. + if (data.show_tally !== true) return; + + const counted = (data.fields ?? []).filter( + (f) => (f.type === "choice" || f.type === "multi") && Array.isArray(f.options), + ); + if (!counted.length) return; + + const responses = await formRef.collection(RESPONSES).get(); + + // Every option starts at zero, so an option nobody picked renders as "0" rather than + // vanishing from the bars — which would make a poll look like it had fewer choices + // than it offered. + const tally = {}; + for (const f of counted) { + tally[f.id] = {}; + for (const o of f.options) tally[f.id][o] = 0; + } + + for (const doc of responses.docs) { + const answers = doc.data().answers ?? {}; + for (const f of counted) { + const a = answers[f.id]; + // An answer naming an option the form no longer offers is DROPPED rather than + // added as a new bucket: options can be edited after responses arrive, and a + // tally that grew a row for a deleted option would read as a live choice. + for (const v of Array.isArray(a) ? a : [a]) { + if (typeof v === "string" && v in tally[f.id]) tally[f.id][v] += 1; + } + } + } + + // merge:true so this touches nothing else on the form. A full set would race with an + // organiser editing the question at the same moment and silently revert their edit. + await formRef.set({ tally }, { merge: true }); + logger.info("Tally updated.", { formId, responses: responses.size }); + }, +); diff --git a/functions/package.json b/functions/package.json index 581c0eb..be010fd 100644 --- a/functions/package.json +++ b/functions/package.json @@ -1,6 +1,6 @@ { "name": "osc-functions", - "description": "Emails the organisers when somebody applies to the club", + "description": "Emails the organisers when somebody applies, and syncs members GitHub contributions", "private": true, "main": "index.js", "engines": { @@ -9,7 +9,7 @@ "scripts": { "serve": "firebase emulators:start --only functions,firestore --project demo-osc", "logs": "firebase functions:log", - "test": "node test-format.mjs" + "test": "node test-format.mjs && node test-github.mjs" }, "dependencies": { "firebase-admin": "^13.0.2", diff --git a/functions/test-github.mjs b/functions/test-github.mjs new file mode 100644 index 0000000..d11475a --- /dev/null +++ b/functions/test-github.mjs @@ -0,0 +1,193 @@ +// Drive github.js against recorded GitHub payloads. +// +// node test-github.mjs +// +// NO NETWORK, NO CREDENTIALS, NO EMULATOR — the same bargain as test-format.mjs. Every +// function under test is a pure function of a fetch response, so `fetch` is replaced with +// a stub that returns whatever the case needs. That is what lets this run in CI on a +// checkout with no Firebase project and no GitHub token. +// +// WHAT IS WORTH TESTING HERE, and it is not "does it call the right URL": +// +// * the handle validator, because it is the one place member-supplied text reaches a +// URL, and because its regex is easy to get subtly wrong (leading hyphens, doubled +// hyphens, 40 characters); +// * "no such account" versus "an account with no pull requests", because they arrive +// from GitHub looking almost identical and the dashboard has to word them completely +// differently; +// * the distinct-repository count, because it is derived rather than returned, and the +// bug it replaced — counting only the eight rows the dashboard lists — produced a +// number that looked entirely plausible; +// * that a rate limit THROWS with a code rather than resolving to zeroes, because a +// silent zero would overwrite a real member's real counts with nothing. + +import { fetchContributions, isValidHandle, repoOf, toPull } from "./github.js"; + +let pass = 0; +let fail = 0; + +function ok(label, cond, detail = "") { + if (cond) { + pass++; + console.log(` PASS ${label}`); + } else { + fail++; + console.log(` FAIL ${label}${detail ? ` ${detail}` : ""}`); + } +} + +const eq = (label, actual, expected) => + ok(label, Object.is(actual, expected), `got ${JSON.stringify(actual)}, want ${JSON.stringify(expected)}`); + +/** A search result item, in the shape /search/issues actually returns. */ +const item = (n, repo = "octo/hello") => ({ + title: `Fix the thing ${n}`, + html_url: `https://github.com/${repo}/pull/${n}`, + repository_url: `https://api.github.com/repos/${repo}`, + pull_request: { merged_at: "2026-05-01T10:00:00Z" }, +}); + +/** Replace global fetch with a router keyed on a substring of the URL. Restored by the + * caller; every case installs its own. */ +function stubFetch(routes) { + globalThis.fetch = async (url) => { + for (const [needle, res] of routes) { + if (String(url).includes(needle)) { + return typeof res === "function" ? res() : res; + } + } + throw new Error(`unstubbed fetch: ${url}`); + }; +} + +const json = (body, status = 200) => ({ + ok: status >= 200 && status < 300, + status, + json: async () => body, +}); + +const realFetch = globalThis.fetch; + +console.log("\ngithub.js\n"); + +// ------------------------------------------------------------------ handles +console.log("-- handles --"); +for (const good of ["a", "octocat", "Octo-Cat", "a1-b2-c3", "x".repeat(39)]) { + ok(`"${good}" is a possible handle`, isValidHandle(good)); +} +for (const bad of ["", "-lead", "trail-", "double--hyphen", "x".repeat(40), "has space", "has/slash", "under_score", null, 42]) { + ok(`${JSON.stringify(bad)} is refused`, !isValidHandle(bad)); +} + +// ------------------------------------------------------------------ parsing +console.log("\n-- parsing one pull request --"); +eq("repo comes from repository_url", repoOf(item(1, "torvalds/linux")), "torvalds/linux"); +eq("an unparseable repository_url degrades", repoOf({ repository_url: "nonsense" }), "unknown"); +eq("a missing title does not render as empty", toPull({}, "merged").title, "(no title)"); +ok( + "a hostile title is truncated", + toPull({ title: "x".repeat(500) }, "merged").title.length === 160, +); +ok( + "merged_at is omitted rather than null when absent", + !("merged_at" in toPull({ title: "t" }, "open")), +); + +// --------------------------------------------------------------- the fetch +console.log("\n-- a member with contributions --"); +stubFetch([ + ["/users/", json({ login: "asha" })], + [ + "is%3Amerged", + json({ + total_count: 12, + // Ten items across four distinct repositories. The list the dashboard shows is + // capped at eight, so this is the case that catches a repo count derived from the + // truncated list instead of the full page. + items: [ + item(1, "a/one"), item(2, "a/one"), item(3, "b/two"), item(4, "b/two"), + item(5, "c/three"), item(6, "c/three"), item(7, "a/one"), item(8, "a/one"), + item(9, "d/four"), item(10, "d/four"), + ], + }), + ], + ["is%3Aopen", json({ total_count: 2, items: [] })], +]); +{ + const c = await fetchContributions("asha", ""); + eq("merged is the search total, not the page length", c.merged, 12); + eq("open is counted separately", c.open, 2); + eq("repos counts DISTINCT repositories across the whole page", c.repos, 4); + eq("the stored list is capped at RECENT", c.recent.length, 8); + eq("not_found is absent for a real account", c.not_found, undefined); + eq("every stored row is labelled merged", c.recent.every((p) => p.state === "merged"), true); +} + +console.log("\n-- a real account that has contributed nothing --"); +stubFetch([ + ["/users/", json({ login: "newbie" })], + ["is%3Amerged", json({ total_count: 0, items: [] })], + ["is%3Aopen", json({ total_count: 0, items: [] })], +]); +{ + const c = await fetchContributions("newbie", ""); + eq("counts are zero", c.merged + c.open + c.repos, 0); + // THE DISTINCTION THE UI DEPENDS ON. Zeroes with not_found absent means "nothing yet", + // which the dashboard words encouragingly. Zeroes WITH not_found means "check your + // handle for a typo". Collapsing them would tell a first-year they had made a mistake. + eq("but not_found stays absent", c.not_found, undefined); +} + +console.log("\n-- a handle that does not exist --"); +stubFetch([["/users/", json({ message: "Not Found" }, 404)]]); +{ + const c = await fetchContributions("nosuchuser", ""); + eq("not_found is set", c.not_found, true); + eq("and no search was attempted", c.recent.length, 0); +} + +console.log("\n-- failures --"); +stubFetch([["/users/", json({ message: "rate limit" }, 403)]]); +{ + let code = ""; + try { + await fetchContributions("asha", ""); + } catch (e) { + code = e.code; + } + // MUST THROW, NOT RESOLVE TO ZEROES. A silent zero here overwrites a member's real + // counts with nothing, and the dashboard would show it as fact. + eq("a rate limit throws with a code", code, "rate-limit"); +} + +stubFetch([["/users/", json({}, 500)]]); +{ + // 500 is deliberately NOT treated as "no such account" — see the comment in exists(). + // A transient GitHub blip must not flag a valid handle as a typo. + stubFetch([ + ["/users/", json({}, 500)], + ["is%3Amerged", json({ total_count: 1, items: [item(1)] })], + ["is%3Aopen", json({ total_count: 0, items: [] })], + ]); + const c = await fetchContributions("asha", ""); + eq("a 500 on the user lookup does not flag a typo", c.not_found, undefined); +} + +{ + let code = ""; + try { + await fetchContributions("-not-a-handle-", ""); + } catch (e) { + code = e.code; + } + eq("an impossible handle is refused before any request", code, "bad-handle"); +} + +globalThis.fetch = realFetch; + +console.log( + fail === 0 + ? `\n ${pass} passed.\n` + : `\n ${pass} passed, ${fail} FAILED.\n`, +); +process.exit(fail === 0 ? 0 : 1); diff --git a/web/app/admin/page.tsx b/web/app/admin/page.tsx index d03f168..7685759 100644 --- a/web/app/admin/page.tsx +++ b/web/app/admin/page.tsx @@ -1,46 +1,63 @@ import type { Metadata } from "next"; import AdminDashboard from "@/components/AdminDashboard"; +import Composer from "@/components/Composer"; +import FormBuilder from "@/components/FormBuilder"; +import Roster from "@/components/Roster"; +import Sessions from "@/components/Sessions"; +import Shell from "@/components/dashboard/Shell"; -// THE ORGANISERS' DASHBOARD. Counts first, list second — see the header of -// AdminDashboard.tsx for what it is for and, more importantly, for what it is not: -// this route is not a privilege gate. The page ships to anybody who asks for it, -// because the site is a static export with no server to refuse them. What refuses -// them is the `list` rule on users/{uid} in firestore.rules, which only an address -// in the `admins` collection satisfies. A non-admin who loads this URL gets a page -// that cannot fetch anything. +// THE ORGANISERS' PAGE, in the same app shell as the member dashboard — the Stitch +// designs are one frame with different content in the middle, so the frame is shared and +// only this file's contents differ. See components/dashboard/Shell.tsx. // -// NOT IN PAGES, so it appears in neither the nav strip nor the footer's route list. -// It is reached from the finished profile on /join, and only when the signed-in -// address is an admin. That is a convenience rather than concealment — the URL is -// guessable and that is fine. +// THIS ROUTE IS NOT A PRIVILEGE GATE. The page ships to anybody who asks for it, because +// the site is a static export with no server to refuse them. What refuses them is the +// `list` rule on users/{uid} and the admin-only writes on every collection below, none of +// which any client can talk its way past. A non-admin who loads this URL gets a page whose +// every panel renders its own "not for you" state. // -// `noindex`, because a page that lists members has no business in a search index -// even though it renders nothing without an authorised session. +// THE ORDER IS BY HOW OFTEN AN ORGANISER DOES THE THING: +// +// membership the question the page is opened with — "who is in the club" +// notices weekly +// sessions weekly, and the one thing the notice board could not express +// forms every few weeks +// roster once a term, and the only one with consequences worth a scroll +// +// `noindex`, because a page that lists members has no business in a search index even +// though it renders nothing without an authorised session. export const metadata: Metadata = { title: "Organisers", - description: "Club membership, by hostel, year, branch and programme.", + description: "Club membership, sessions, notices and forms.", robots: { index: false, follow: false }, }; export default function Admin() { return ( -
-
-

Organisers only

-

- Who is in the club. -

-

- Every registered member, and the breakdowns most often asked for — by hostel, by - year, by branch, by programme. Counts are over the whole membership; the search - and filter below narrow only the list. -

- -
- + +
+
+

+ Admin dashboard +

+

+ Who is in the club, what they have been told, and what you have asked them. +

-
-
+ + + + + + + {/* THE ROSTER LAST, and that ordering is the argument. Changing who runs the club + is the rarest thing on this page and the one with the largest consequences, so + it sits where nobody reaches it by accident. It renders for every admin but is + only usable by owners — see the note in Roster.tsx for why it is shown rather + than hidden. */} + + + ); } diff --git a/web/app/dashboard/page.tsx b/web/app/dashboard/page.tsx new file mode 100644 index 0000000..9fbdeb5 --- /dev/null +++ b/web/app/dashboard/page.tsx @@ -0,0 +1,31 @@ +import type { Metadata } from "next"; +import Shell from "@/components/dashboard/Shell"; +import MemberDashboard from "@/components/MemberDashboard"; + +// THE MEMBER'S DASHBOARD — the one route on this site that is an APP rather than a page. +// +// The frame comes from components/dashboard/Shell.tsx, which the organisers' page shares; +// everything specific to a member is in MemberDashboard. See the Shell's header for why +// the site's own nav and footer are suppressed here. +// +// NOT A PRIVILEGE GATE. The site is a static export, so this markup ships to anybody who +// asks for it. What refuses a stranger is firestore.rules — users/{uid} and +// contributions/{uid} are owner-or-admin, and the board and forms require a verified +// college address. See the header of lib/auth.tsx before assuming a hidden page is safe. +// +// `noindex`, because a page whose entire content is one member's own record has no +// business in a search index even though it renders nothing without a session. + +export const metadata: Metadata = { + title: "Your dashboard", + description: "Your details, what you have merged, and what the club has pinned up.", + robots: { index: false, follow: false }, +}; + +export default function Dashboard() { + return ( + + + + ); +} diff --git a/web/app/globals.css b/web/app/globals.css index 2cb2ecd..ca2f572 100644 --- a/web/app/globals.css +++ b/web/app/globals.css @@ -498,7 +498,7 @@ body::before { blocks would be a fairground. */ .label { font-family: var(--font-label), system-ui, sans-serif; - font-size: 1rem; + font-size: 0.875rem; /* Stated, not inherited from the face — see the type note above. */ font-weight: 600; letter-spacing: 0.07em; @@ -642,7 +642,7 @@ body::before { everything beside it. The plate had room at 14px with the nav well short of its wrap point; 16px spends most of that margin, so the sm breakpoint is worth a look if a seventh link is ever added. */ - font-size: 1rem; + font-size: 0.875rem; font-weight: 500; letter-spacing: -0.005em; transition: color 180ms ease-in-out; @@ -703,7 +703,7 @@ body::before { background: rgb(var(--accent)); color: rgb(var(--bg)); font-family: var(--font-label), system-ui, sans-serif; - font-size: 0.875rem; + font-size: 0.75rem; font-weight: 700; letter-spacing: 0.1em; text-transform: uppercase; @@ -1402,7 +1402,7 @@ body::before { background: rgb(var(--accent)); color: rgb(var(--bg)); font-family: var(--font-label), system-ui, sans-serif; - font-size: 0.9375rem; + font-size: 0.8125rem; font-weight: 800; letter-spacing: 0.04em; font-variant-numeric: tabular-nums; @@ -1522,7 +1522,7 @@ body::before { background: var(--tint-soft); color: var(--tint-ink); font-family: var(--font-label), system-ui, sans-serif; - font-size: 1rem; + font-size: 0.875rem; font-weight: 800; letter-spacing: 0.02em; font-variant-numeric: tabular-nums; @@ -1546,7 +1546,7 @@ body::before { beside it, it has lowercase to be short of, and at 12px Plus Jakarta Sans's x-height put it below anything else readable here. The caps-bearing pills stay at 0.75rem for the reason given in tailwind.config.ts. */ - font-size: 0.9375rem; + font-size: 0.8125rem; font-weight: 700; letter-spacing: 0.01em; line-height: 1.2; @@ -1853,7 +1853,7 @@ body::before { border: 2px solid #000000; box-shadow: 3px 3px 0 0 #000000; font-family: var(--font-label), system-ui, sans-serif; - font-size: 0.875rem; + font-size: 0.75rem; font-weight: 700; letter-spacing: 0.02em; line-height: 1; @@ -1965,7 +1965,7 @@ body::before { background: #0f172a; color: #e2e8f0; font-family: var(--font-sans), system-ui, sans-serif; - font-size: 0.9375rem; + font-size: 0.8125rem; font-weight: 500; line-height: 1.35; letter-spacing: 0; @@ -2040,7 +2040,7 @@ body::before { background: #0f172a; color: #e2e8f0; font-family: var(--font-sans), system-ui, sans-serif; - font-size: 0.875rem; + font-size: 0.75rem; font-weight: 500; /* The captions above are uppercase and tracked out; this is prose, so it resets both — otherwise it inherits the chart's caption feel and reads as another @@ -2273,7 +2273,7 @@ body::before { margin: 0.5rem 0 0; padding: 0; list-style: none; - font-size: 0.875rem; + font-size: 0.75rem; line-height: 1.5; color: rgb(var(--haze)); } @@ -2311,7 +2311,7 @@ body::before { background: #0f172a; color: #e2e8f0; font-family: var(--font-mono), ui-monospace, monospace; - font-size: 0.8125rem; + font-size: 0.6875rem; line-height: 1.4; opacity: 0; transform: translateY(6px); @@ -2347,7 +2347,7 @@ body::before { and lands at 6.98 light / 5.63 dark, which is why .btn-primary uses it too. */ color: rgb(var(--bg)); font-family: var(--font-label), system-ui, sans-serif; - font-size: 1.125rem; + font-size: 1rem; font-weight: 700; letter-spacing: 0.02em; font-variant-numeric: tabular-nums; @@ -2375,13 +2375,13 @@ body::before { padding: 11px 21px; border-radius: 10px; font-family: var(--font-label), system-ui, sans-serif; - /* 17px/700, up from 15px/700 with the +2px pass. WATCH THE HERO PAIR AT 390px. - This step was 17px once before, under Staatliches, and moving to Plus Jakarta - Sans — which runs about a third wider at the same size — is what forced it down - to 15. The weight is unchanged and the labels are short, so the two hero buttons - should still hold one line, but this is the one control on the page where the - bump lands closest to a known wrap point rather than in open space. */ - font-size: 1.0625rem; + /* 15px/700, back down with the rest of the page after the +2px pass took it to 17. + 15 is where this step sat under Plus Jakarta Sans, which runs about a third wider + at the same size than the Staatliches it replaced — so the hero pair, the one + place on the page where this control comes near a wrap point rather than sitting + in open space, is being returned to a width that was known to hold one line at + 390px rather than moved somewhere new. */ + font-size: 0.9375rem; font-weight: 700; letter-spacing: 0.04em; text-transform: uppercase; @@ -2451,7 +2451,7 @@ body::before { .btn-compact { min-height: 44px; padding: 0 16px; - font-size: 0.9375rem; + font-size: 0.8125rem; } /* The electric-blue indicator on the secondary CTA. A right-aligned bolt in a @@ -2469,7 +2469,7 @@ body::before { border-radius: 5px; background: rgb(var(--accent)); color: rgb(var(--bg)); - font-size: 0.875rem; + font-size: 0.75rem; line-height: 1; } @@ -2846,10 +2846,11 @@ body::before { gap: 1.75rem; padding-right: 1.75rem; font-family: var(--font-label), system-ui, sans-serif; - /* Was exempt from the x-height bump the sans steps took, being capitals — see the - note in tailwind.config.ts for why that exemption does not carry over to the - +2px pass. Still clear of the 11px floor the QA sweep enforces. */ - font-size: 1rem; + /* Was exempt from the x-height bump the sans steps took, being capitals, then not + exempt from the +2px pass — see the note in tailwind.config.ts for why a uniform + instruction overrides a per-face one. That pass is reversed and this comes back + with it, still clear of the 11px floor the QA sweep enforces. */ + font-size: 0.875rem; font-weight: 800; letter-spacing: 0.12em; text-transform: uppercase; @@ -2861,7 +2862,7 @@ body::before { /* --sky rather than --accent: this is a decorative glyph at display weight, not a 13px link, so it takes the brighter step. */ color: rgb(var(--sky)); - font-size: 0.875rem; + font-size: 0.75rem; } /* Scrolls, but without a bar drawn through the nav's bottom hairline. */ @@ -3590,31 +3591,38 @@ button:focus-visible .icon-slide { /* THE COMMIT GRAPH'S TWO LANES, plotting themselves left to right. - NOT the [data-draw] rule the heading underlines use, and the reason is measured - rather than stylistic. That rule leans on `pathLength="1"` to make one dash - value serve paths of any length; this SVG also carries - `vector-effect: non-scaling-stroke`, which keeps its rails 2px at every width - and, as a side effect, makes dash lengths resolve in SCREEN PIXELS. The two - together produce a dash of "the whole path" = 304 user units read as 304px - against a lane rendered 1199px long, i.e. four dashes and three gaps in what is - supposed to be one continuous line. It rendered as a broken diagram. - - So each lane declares its own length in container-width units instead — 95cqw - for the 304-unit rail, 57cqw for the 181-unit branch, both authored at the path - in CommitGraph.tsx. Those track the rendered width of a `w-full` svg exactly, so - one dash covers one lane at every viewport, and the fallback keeps a - never-visible lane from becoming a permanently invisible one if a future call - site forgets the variable. + NOT the [data-draw] rule the heading underlines use, and not a stroke dash at + all, for a reason that is measured rather than stylistic. Every dash technique + needs to know how long the path is, and this SVG carries + `vector-effect: non-scaling-stroke` to keep its rails 2px at every width — which + takes dash lengths out of the viewBox scale and leaves them resolving against + the rendered picture instead. Whose pixels, though, is not the same answer in + every browser. `pathLength="1"` gave a dash of 304 USER units read as 304px + against a lane 1199px long: four dashes and three gaps. Sizing the dash in + container-width units instead (95cqw for the rail, 57cqw for the branch) fixed + that at 1x and broke again on a 150%-scaled Windows display, where 1077 CSS px + of dash was read as 1077 DEVICE px against a rail 1701 device px long and each + lane stopped two thirds of the way across. Both times it rendered as a broken + diagram, because both times a length had to be guessed in someone else's unit. + + So the lane is not uncovered by a dash, it is WIPED by a clip — off the path's + own bounding box, which the browser measures itself. Both lanes only ever + advance in x, so a wipe and a draw are the same picture, and there is no length + left in the effect to be wrong about. Nothing needs retuning when a path or a + breakpoint changes. + + The -6px of slack on the three edges that do not animate is because a clip + takes the FILL box: the grey rail's bounding box is a line of zero height, and + without the bleed the clip would shave off its 2px stroke and its round caps. Same 0.7s and same 0.15s step behind the block as the heading rules, so the two drawn things on the page share one rhythm. */ :root.reveal-on [data-reveal-group] .graph-lane { - stroke-dasharray: var(--lane, 100vw); - stroke-dashoffset: var(--lane, 100vw); - transition: stroke-dashoffset 0.7s ease calc(var(--reveal-delay, 0s) + 0.15s); + clip-path: inset(-6px 100% -6px -6px); + transition: clip-path 0.7s ease calc(var(--reveal-delay, 0s) + 0.15s); } :root.reveal-on [data-reveal-group].is-in .graph-lane { - stroke-dashoffset: 0; + clip-path: inset(-6px -6px -6px -6px); } /* The nodes on the commit graph, popping in along the lane the path has just diff --git a/web/app/join/page.tsx b/web/app/join/page.tsx index 327ab8d..0ead1b8 100644 --- a/web/app/join/page.tsx +++ b/web/app/join/page.tsx @@ -1,6 +1,6 @@ import type { Metadata } from "next"; import Link from "next/link"; -import JoinGate from "@/components/JoinGate"; +import ApplyForm from "@/components/ApplyForm"; import Duo from "@/components/Duo"; import Note from "@/components/fx/Note"; @@ -165,19 +165,54 @@ export default function Join() {

No prior experience

+ + {/* THE WAY BACK IN, and it is here because separating the two features + opened a hole that nothing else closes. The nav's far-end button reads + "Join" for anybody without a session and points at this page — so an + existing member on a new laptop, or after clearing their cookies, clicks + it out of habit and arrives at an application form with no sign-in + anywhere on it. Their own dashboard would be unreachable except by + typing the URL. + + BEFORE THE FORM, NOT AFTER IT. Under the fields it would be found by + somebody who had already filled them in, which is the one moment the + sentence is no longer useful — a second application is exactly what it + exists to prevent. + + Small and quiet on purpose: almost nobody reading this page is a + member, and a sign-in prompt with equal weight to the form would ask + every first-time reader to work out which of two things they are. */} +

+ Already joined?{" "} + + Sign in to your dashboard + (opens in a new tab) + {" "} + — no need to apply twice. +

- {/* WAS , THE ANONYMOUS ONE-SHOT FORM. It is now sign-in - first: register with a college Google account, then fill a profile - once that you can come back and edit. The column this sits in, the - copy beside it and the two tiles above are unchanged — the flow - changed, not the page. - - Everything about the gate is client-side, because the site is a - static export. Route-gating is therefore cosmetic and the data is - protected by firestore.rules instead. See the note at the top of - lib/auth.tsx before assuming a hidden page is a safe one. */} - + {/* THE ANONYMOUS APPLICATION FORM, and it is back here after a spell as a + sign-in gate. For a while this column held : register with + a college Google account first, then fill a profile. That put an + account requirement in front of the club's front door — a stranger + could not apply without already holding the thing that membership + grants — and it made the headline three inches to the left false at + the exact moment somebody acted on it. + + Sign-in did not go away; it stopped being this page's business. It + lives on /dashboard now (components/SignInCard.tsx), which is the one + place that genuinely needs to know who you are. This page asks, that + page identifies, and neither has to care about the other. + + The column this sits in, the copy beside it and the two tiles above + are unchanged — the flow changed, not the page. */} + diff --git a/web/app/layout.tsx b/web/app/layout.tsx index 9cf277b..c3e3b20 100644 --- a/web/app/layout.tsx +++ b/web/app/layout.tsx @@ -82,6 +82,7 @@ const mono = JetBrains_Mono({ import Nav from "@/components/Nav"; import Reveal from "@/components/Reveal"; import Footer from "@/components/Footer"; +import ChromeGate from "@/components/ChromeGate"; import { AuthProvider } from "@/lib/auth"; const SITE_URL = @@ -155,11 +156,16 @@ export default function RootLayout({ imported inside the provider, so a reader who never signs in never downloads it, and with no Firebase config the provider resolves immediately to "signed out" and renders no extra markup. */} + {/* ChromeGate hides the FOOTER on the app routes; Nav hides itself, because it is + already a client component reading the pathname. Footer is a server component + and cannot, so it is passed through as children instead. */}