diff --git a/.clj-kondo/config.edn b/.clj-kondo/config.edn index f0070b8dd..6f60baba9 100644 --- a/.clj-kondo/config.edn +++ b/.clj-kondo/config.edn @@ -126,6 +126,11 @@ (orcpub.routes.folder-test/with-conn) (orcpub.email-change-test/with-conn) (orcpub.registration-rollback-test/with-conn) + (orcpub.routes.public-reads-test/with-conn) + (orcpub.routes.item-save-test/with-conn) + (orcpub.routes.share-test/with-conn) + (orcpub.routes.party-test/with-conn) + (orcpub.heartbeat-test/with-conn) (user/with-db)]}} ;; native/cljs and web/cljs are separate source roots; kondo doesn't know ;; about them so ns names appear to mismatch their file paths. diff --git a/.gitignore b/.gitignore index e79411267..c423e1ebb 100644 --- a/.gitignore +++ b/.gitignore @@ -54,7 +54,10 @@ deploy/homebrew/* branch-sync-plan.md dev/test-accounts.edn -# E2E test artifacts +# E2E test artifacts. Unanchored, because a leading slash pins the pattern to the +# repo root: /node_modules/ and /e2e/node_modules/ both missed the real one at +# scripts/e2e/node_modules, and 179 files of it went into a commit. +node_modules/ /e2e/node_modules/ /scripts/e2e/node_modules /e2e/playwright-report/ diff --git a/CHANGELOG.md b/CHANGELOG.md index 5bc404516..f0819d30e 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -11,7 +11,7 @@ House style and the branch → release fold: [`docs/branch-changelog.template.md ### Highlights -You can now move and copy homebrew between sources, turn content off without deleting it, and see in one place what needs fixing, with imports and exports that no longer spawn silent duplicates or false warnings. Characters that used to blank-screen on a bad load now recover in place, and printable spell cards and card backs read cleanly in black and white. +You can now move and copy homebrew between sources, turn content off without deleting it, and see in one place what needs fixing, with imports and exports that no longer spawn silent duplicates or false warnings. Characters that used to blank-screen on a bad load now recover in place, and printable spell cards and card backs read cleanly in black and white. Signing up, signing in and recovering an account have been rebuilt, with breach-screened passwords, a live strength meter, and reset links that can only set a new password. ### Fixed @@ -1172,6 +1172,384 @@ one 10px gutter, and the header's tab menus open on screen again. - **The security-policy docs match the server** — there is no report-only mode; development sends no policy, everything else enforces it (`4e972cf7`, `c9fd5a5b`). +### integration-local (accounts and the September hotfixes) + +**Added** + +- **New homebrew keys carry their source's tag** — two authors' "Stone Elf" become + `:stone-elf-trcs` and `:stone-elf-kbtx` instead of colliding, and a source can set its own + tag from My Content. The builder also gains a key row to change a key on purpose + (`ed4f5a4c`, merged in `01180480`). The duplicate-save gating that came with it was reverted + (`65502f3d`) because it could let a save overwrite another library's item. +- **A check for homebrew no repair knows about** — a browser probe breaks one race on + conversion and one only when drawn, and checks the app, the notice, the set-aside, + Restore and import (`986c25a4`). + +- **A check that every fix is saved** — a browser probe applies each repair through + the app, reloads and exports, so a fix that only lives on screen fails a test + instead of reverting on refresh (`c275c101`). + +- **A character brings its custom items from the server** — whoever can open a character now sees the + custom items it has equipped, as they are now. Share links no longer carry items, so they are + shorter; homebrew still travels in the link, and older links still open (`397aea6a`). + +- **Short share links** — a character's owner gets a link of about 85 characters that stays the + same and always shows the character's current homebrew, which the server keeps and checks; New link + revokes every earlier link, nothing is stored until the owner presses Share link, the limits are + config settings, and anyone else's link still carries the homebrew itself (`5ef7939b`, `56723035`, + `79b79f9d`, `0eda1015`). + +- **A party keeps a shared character's homebrew** — a character added to a party from its share link + shows its homebrew on the party page, until its owner makes a new link (`f328ca60`). +- **Unused share links expire** — a share link nobody opens for 180 days of the server running is + deleted with its share data; the character stays, the owner can share again, and + `ORCPUB_SHARE_PRUNE_DAYS` sets the window, 30 days at least. A request without the link's token does + not count as use, and time the server was off does not count against a link (`c5532564`, `4b17550d`, + `8b7e011a`). +- **Stop sharing** — a character's owner can stop sharing it at any time: every link made before stops + showing its custom content, and nothing is stored again until Share link is pressed (`124c1c62`). +- **The owner is told when a share link expired** — the character page and list show the date beside + Share link on every visit, until the owner shares again or dismisses the note; the note keeps only the + character and the date (`124c1c62`). +- **The server records when it was running** — an hourly heartbeat kept in the database notes when the + server was off, and scheduled jobs run after it; share link pruning is the first (`124c1c62`). + +- **The password rules judge length and shape, not character classes** — a minimum of 12 + characters, with repeats, runs and too few distinct characters refused, and no rule about + symbols or capitals. NIST has told everyone to stop imposing composition rules, which push + people to Password1! and no further. The validator is shared between the browser and the + server so neither can disagree with the other (`e8496511`, `696a962e`, `29783af5`). + +- **A strength meter that asks the validator instead of guessing beside it** — four rungs at + 12, 16, 22 and 28 characters, and it refuses to show a verdict the server would contradict. + The username and email go into the judgement, so a password made of them does not score + (`29783af5`). + +- **Breach screening against the Pwned Passwords corpus** — only the first five characters of + the password's SHA-1 leave the server, so the password itself is never sent. A password is + refused only when it appears 1000 or more times in the corpus, and a service that cannot + answer never blocks anyone (`09ac513c`, `729458ec`, `1cf25582`). + +- **A notice when one account is signed into from several places** — with the time, the + browser and where from, held to one message per account per cooldown so a person with a + phone and a laptop is not mailed all day (`073d542c`, `18d1deb2`, `416a3dce`). + +- **A browser suite for signing up, verifying, signing in and recovering an account** — + `scripts/e2e/auth-flows.js` runs twenty-two checks on flows that had no coverage. + `scripts/e2e/lib/mail-sink.js` is a small SMTP server that catches the emails, and `run.sh` + starts it, so the suite follows the same links a person would click. It confirms that wrong + guesses give identical answers and that a reset link works only once (`991e5ff9`, + `b50602f1`). + +**Fixed** + +- **A password reset link no longer signs you in** — opening it used to start an hour-long session without changing the password, and the owner was never told; now it only lets you set a new password, once, and is checked when you submit (`f8f270dd`). +- **Signing out everywhere cannot be undone by the hourly refresh** — a refresh that read the + database just before a reset or sign-out-everywhere committed replaced the register with that + older read, and the withdrawn sessions worked again for up to an hour (`530837f4`). +- **A password is judged as it is saved** — signup and reset checked it as typed but stored it + trimmed, so `" pomegranate "` passed the 12-character minimum and was saved at 11 (`91806337`). +- **Moving a class or race keeps its subclasses and subraces** — a move into a source that already + had that key renamed the item but left its dependents on the old key, where they joined the + other source's item. They now follow it, in a bulk move too, and the old key is recorded + (`aab74709`, `8f481078`). +- **Blank icons show again** — the Edit button on character and item pages, New link and three + import-log lines used Font Awesome 4 names the app does not serve, so on a phone they were empty + buttons (`f6354d71`). +- **A mistyped `ORCPUB_SHARE_PRUNE_DAYS` is called out at boot** — it showed as set while the default + was in use (`d108cd46`). +- **Broken keys and card lists in homebrew are repaired** — an entry whose key was not + a keyword, or whose traits or options held something other than cards, crashed the + import or later the export; it now imports with what can be kept (`d1c234eb`). +- **Startup only reads homebrew** — the character options autosave needs are built on + the first save instead of on every page load, and a library that cannot be loaded is + set aside intact with a download, so the app still starts (`986c25a4`). +- **Homebrew that still breaks is set aside, not left to break the page** — an entry + that fails to build is set aside with a notice that names it and links to My Content, + and a page that fails on homebrew checks it and loads again on its own (`986c25a4`). + +- **Bad stored homebrew no longer stops the app from starting** — a race whose key + was text left every page on the loading spinner. A failed startup step no longer + stops the app mounting, and sorting keys no longer breaks on mixed types + (`ae80c93f`). + +- **A character is repaired on refresh too, and stays repaired** — the repair is + saved back to the builder's draft, so it happens once instead of on every load, + and its notice shows after the page navigates instead of being cleared by it + (`d17ea6b3`, `f0e267dc`). +- **Entries set aside on load leave the stored library** — they were set aside in + memory only and came back on the next refresh (`47f6e886`). +- **The save banner's export link exports the source as it is now** — it exported + the copy it remembered from when the banner appeared, dropping later edits + (`0db360ed`). +- **Damaged homebrew is repaired on import and on load** — a section stored as + text or as a list comes back with everything in it, and a file whose whole + content was stored as text imports instead of crashing. What nothing can read is + set aside with Export raw and Discard (`117af073`, `71e37588`). +- **A partial import shows as a warning** — it arrived in the green success card. + Each line is now marked imported, repaired or skipped, and counts read "1 item" + (`246d3ef2`). +- **A nameless item reads "Unnamed Spell" in the missing-fields dialog** — with its + key beneath, instead of a bare ":no-name" (`5ae5bbbf`). +- **The import-log button no longer covers header menus** — it stays off the page + while the log is empty and draws under an open menu (`74d8b241`). +- **The save sparkle stays on the save button** — it drifted into the gap beside + it (`7ad28714`). +- **A library stored as text loads again** — it loaded nothing (`4ac618e2`). +- **A stored value that isn't a library is handled once** — it was copied aside + again on every load (`4ac618e2`). +- **Save anyway in the selection builder keeps the selection** — a name like + "9 Lives" was saved under a key the next load set aside (`155abb4c`). +- **Renames chosen for existing items are kept when nothing incoming imports** — + they were dropped along with the incoming entries (`544cd342`). +- **An entry with no source takes the name of the source it sits in** — unless its + source field was there but blank, it was skipped on import or set aside on load. + Only an entry with no name to take goes to Default Option Source (`34472aeb`). +- **An import with an entry that isn't a map no longer crashes** — the entry is + skipped and listed in the import log (`c79a28e7`). + +- **The email-support button on a character that will not load sends the report again** — clicking + it threw after the login change, so no report went out (`a53520ed`). +- **A login the server has stopped accepting now logs you out** — the app kept showing you signed in + while every request failed, until you logged in again or reloaded (`ffa050ea`). +- **Share links no longer carry item ids or your username** — each custom item went into the link as + stored, with its database ids and its owner; links made before this drop both when opened + (`852bc8f7`). +- **Only its owner can read a custom item by its id** — anyone with the id could fetch the item and its + owner's username; everyone else now gets "not found" (`904f7be2`). +- **A character page never shows an email address** — characters saved during nine days in May 2017 + named their owner by the email used to log in (`904f7be2`). +- **Logins, API calls and PDF downloads go to the server that served the page** — every + http://localhost page sent them to port 8890, which broke the Docker setup opened at + http://localhost and a server on any other local port (`f9f50e55`). + +- **Copy link shares the character it sits beside** — on the character page and in the character list + it carried the homebrew of whatever character was open in the builder (`3ece5ac9`). +- **Pasted images and video are left out of shared homebrew** — a data: URI in any text field is + emptied before sharing and again when a link is opened (`806d1a23`). + +- **Only characters can be added to a party** — adding checked nothing, so any id could be added + (`f328ca60`). + +- **The mobile header no longer crowds itself** — on a phone the logo is capped, a + full-width child no longer measures wider than the bar it sits in, the import log + panel cannot exceed the screen, and a child that still outruns the bar is clipped + rather than scrolling the whole page sideways (`c3988d92`). + +- **Login no longer says whether a username exists** — every failure answers the same way, and + the per-address throttle is consulted BEFORE the credentials rather than computed and thrown + away, so a spray is turned back without first being told whether it guessed a real account + (`360709a8`, `80121fce`). + +- **Password reset no longer says whether an address is registered** — it answers 200 either + way, including once throttled, because an endpoint that changes its answer under a limit has + simply moved the oracle (`1d8d7be8`). + +- **Reset keys are stored as a digest and expire** — the table holds a SHA-256 of the key + rather than the key itself, and it is good for two hours. An unknown key used to fall through + to signing a token for username nil (`072786ca`). + +- **Registration is capped per host** — ten an hour, counted only once a form was otherwise + going to succeed, so a signup that was failing anyway is not held against the address. What + the limits turn away is counted and summarised hourly, so the numbers can be tuned against + something (`8788b764`, `3134d972`, `49b37009`). + +- **A margin utility no longer makes 194 places bold** — `.m-b-10` sat in a Garden selector + group that read as a descendant chain and was not one, so a margin class carried + `font-weight: bold` across the app. Measured rather than guessed: 194 uses in source, 11 + rendering on the logged-out pages, 7 computing a different weight, 5 actually looking + different (`4ed59b1f`). + +- **The legal links printed twice on the register page** — the consent line links both + documents and the footer printed the same pair under it. The footer keeps the copyright there + and drops its copies (`d5feb650`). + +- **The gryphon panel tiled** — no `background-repeat` and no `background-size`, so once the + form column outgrew the image a second half-cropped gryphon drew below the first. It is a + Garden class now instead of an inline style map (`d5feb650`). + +- **The email help link was one word in the middle of a sentence** — "whitelist" as the whole + clickable target, which is a small target and names no destination when a screen reader reads + it alone. Both instances link a phrase now, and the login copy no longer reads "Didn't receive + validation the email?" or advise resetting a password to fix a missing validation email + (`d5feb650`). + +- **Sign out everywhere** — an account can shut every session from My Account, including the + current one. It does not ask for the password, so someone who fears their account is open on + a device they cannot reach can close it from the session they hold. Signing out never + requires changing the password (`3c4b147d`). + +- **Resetting a password takes back the sessions that existed before it** — every session from + before a reset used to keep working. Tokens now record when they were minted, and one minted + before the password changed is refused. Signing in again works normally on any number of + devices. The list of withdrawals is rebuilt from the database at boot, so a restart does not + bring old sessions back (`c9de41dd`). + +- **A changed password now tells the account it happened** — nothing was sent. Six + outbound mails existed and none of them fired on a credential change, so a takeover was + silent until the owner tried to log in, and then all they learned was that they could + not. The notice says when and from what browser, and links the reset PAGE rather than + carrying a working credential (`5142b5b5`). + +- **An email change tells the address that currently owns the account** — the verification + goes to the NEW address, which is the one place the owner cannot read if it was not + them, so the party losing the account was the only one never told (`5142b5b5`). + +- **Moving an account to another address takes the password, not just a session** — it + asked for nothing but a session, so a borrowed laptop or a reset link followed and left + open was enough to walk off with an account. Checked before the address is even looked + at, so an unauthenticated caller cannot use it to learn which addresses are taken + (`5142b5b5`). + +- **scripts/e2e/run.sh rebuilds a stale bundle instead of testing it** — the server compiles + from source at every boot, but the bundle and stylesheet on disk were never rebuilt, so a + suite could pass or fail against old code. `run.sh` now names what is newer and rebuilds the + kind of bundle already there. `E2E_SKIP_BUILD=1` keeps the existing files and says so loudly + (`5142b5b5`). + +- **Both places a password is made share the area, not just its parts** — the registration and + reset pages each assembled their own password pair, meter and checks. The reset form had no + meter and judged a password against no username, so a password could look fine and then be + refused by the server for matching the username. `password-fields` and + `registration/password-pair-faults` now serve both pages, and the reset page reads the + username from a cookie it sets (`5334fc70`, `f8f270dd`). + +- **What the reset page's "not your name" check does and does not know** — it judges a + password against the username only, read from a cookie the reset page sets. The server also + checks the email's local part. Matching that on the page would mean putting the address in a + readable cookie, so the server's refusal explains itself on submit. The chip fires only on the + whole username, never a prefix (`d0719721`, `f8f270dd`). + +- **All nine auth pages share a body, not just a shell** — four pages hand-assembled the same + container, one nested it wrongly and got a double gutter, and the login fields were 350px + wide where every other page's fill the column. `auth-form-page` takes a heading, a lede, + fields and a tail, so the nine pages now come in two shapes and fixes land once instead of + per page (`55cb4d4e`). + +- **The pages that only announce an outcome get a layout of their own** — registration + complete, password changed, unsubscribed and check-your-email had the heading centred and + everything under it flush left with no gutter. They now centre in the space the card has, + and the one thing to do looks like a button rather than a stray link (`42756378`). + +- **The login page's other ways in are one group with one rhythm** — the three separate + blocks, with `
` gaps pushing each answer away from its question, are now one group where + a question and its answer share a line. The help line sits below a rule because it is not + another way in, and the LOGIN button is full width and aligned to the fields (`42756378`). + +- **The last three auth pages join the shared heading** — `verify-failed`, + `send-password-reset-page` and `password-reset-page` still drew their own heading with no + amber rule and no form gutter, and they are the pages someone locked out actually sees. The + two reset pages are now titled "Reset your password" (the one that mails a link) and "Choose + a new password" (the one that takes it). + +- **Password reset told nobody why it would not go** — an ordinary password left SUBMIT + dimmed and the page silent. The rules gated the form while their reasons were suppressed: + `:messages password-messages` had been commented out since the dual-build era, which was + survivable at a minimum of eight and is not at twelve. The messages show, and the button + is never dimmed (`0dd4545c`). + +- **The server's reason never left the server** — the reset endpoint answered + `{:status 400 :message "..."}`, but `:message` is not a Pedestal response key, so the + reply was a 400 with an empty body and the page showed a generic apology. Field-keyed + messages in `:body` now, the shape registration already used (`991e5ff9`). + +- **Two verdicts that disagreed** — a password the breach corpus refused was drawn as a red + field error directly above a meter reporting UNCOMMON, in green, about the same string. + The corpus verdict is the meter's: its own key, the fail colours, a Too common badge and + the reasoning on a line under the bar (`b50602f1`). + +- **The auth card's text was set solid** — the CSS reset sets `body{line-height:1}`, and a + unitless line-height is inherited as a NUMBER, so every element that did not set its own + rendered at its own font size and descenders ran into the next line. Eleven elements + measured under a 1.25 ratio, nine at exactly 1.00 (`b50602f1`). + +- **The gryphon panel tiled, and the legal links printed twice** — no `background-repeat` + and no `background-size`, so once the form column outgrew the image a second cropped + gryphon drew below it; and the consent line and the footer each linked the same two + documents (`d5feb650`). + +- **The email help link was one word mid-sentence** — "whitelist" as the whole clickable + target, in two places, one of which also read "Didn't receive validation the email?" and + advised resetting a password to fix a missing validation email (`d5feb650`). + +**Changed** + +- **A character's sharing is one line under its title** — a status (Not shared, Shared, or Link expired + and the date) with its actions as text buttons: Share link until a link exists, then Copy link, New + link and Stop sharing. It replaces the share buttons in the page header, which split into extra rows + on a phone (`d2e01052`, `38365ce3`). +- **The character list row shares with one Copy link button** — it copies the link, or makes the share + and copies it when the character has none; the status, New link and Stop sharing stay on the + character page (`e388e24b`). +- **Share wording** — the startup log's share settings speak of share data, the Share link button no + longer describes server storage, and What's New describes short links, New link and Stop sharing + (`124c1c62`, `50ccee78`). +- **A party forgets a character's link when its share ends** — New link, Stop sharing, expiry and + deleting the character delete the token the party saved; the character stays in the party + (`d1080896`). +- **The share settings are documented** in `docs/ENVIRONMENT.md` (`6dbf9161`). +- **Browser probes share one way to find Chromium** — they find Playwright's + current install without setting four variables by hand, and the runner reports + a missing browser once as a skip (`47d3ed8f`). +- **The PDF check is now a named suite, `export-character-pdf.js`** — it hides the What's New + panel that had been taking its clicks (`60adac5f`). +- **run.sh lists the e2e suites and runs them on any machine** — it honours E2E_PORT and finds + Chromium the same way the probes do (`60adac5f`). +- **run.sh says when it turns CSP off for a development bundle** (`60adac5f`). +- **A browser check that a character shows its owner's items to others** — it opens a seeded character + logged out, as another account and as the owner, and checks the item reaches the sheet each time; + run.sh now waits for the test accounts to be seeded before a suite starts (`c3f656cb`). +- **Named Garden classes replace 28 inline style maps on the registration, login and password + pages** — 49 class definitions were taken from the stalled `refactor/garden-inline-styles` + branch without merging it. Two classes that were generated from value lists emitted no CSS + before and are generated here. The password meter keeps an inline width because that width + is the measurement. Nothing renders differently (`887293f5`, `a104089a`). +- **The nine auth pages share one heading** — `registration-page` was always the shared shell, + but six pages carried their own copy of the same orange drop-shadowed heading. The shell owns + it now, as ink with an amber rule, so it changes in one place (`dab92f84`). + +- **The auth fields are notched outlines with a real label** — the label rides the input's + border rather than sitting in a placeholder that vanishes the moment somebody types, so the + field never stops saying what it is (`5a8d7500`). + +- **Errors read in GOV.UK order** — the label lifts out of the notch, the message sits under it + and the input follows, with a rail down the group, a 1px border and a soft tint instead of a + heavy outline plus a boxed message. A summary above the form counts fields rather than + messages, and each line focuses its field (`8ee1dd8a`, `e59bfa6a`). + +- **A password can be revealed, and the confirm box retires when it is** — two boxes while it + is masked, one while it is not, and `display: none` rather than dimmed so a submit cannot + fail pointing at a field nobody can see (`93faf389`). + +- **The email is confirmed, not the password, and a mistyped domain is offered a fix** — a + mistyped password is recoverable; a mistyped address makes a dead account holding the username + its owner wanted and mails a stranger on the way. Within two edits of a known domain, a + correction is offered under the field (`0a64172c`). +- **The last three auth pages join the shared heading** — `verify-failed`, + `send-password-reset-page` and `password-reset-page` still drew their own bold heading + with no rule and no form gutter, so three of the nine shipped unredesigned, and they are + the three somebody locked out of their account sees. The two reset pages would both have + read "reset password"; the one that mails a link is "Reset your password" and the one + that takes the new password is "Choose a new password" (`82ea472d`). + +- **Every password rule is visible as its own chip, and the meter says what to do next** — + five chips under the bar, one per rule, each showing whether this password satisfies it + before anything is pressed. Below the bar, a line points forward at every rung except the + top, so a password that is already good enough is not told off. + +- **The reset form gets the meter and the reveal** — it refused by exactly the same rules + as registration while showing no meter, offering no reveal and demanding a confirmation + it never retired. The meter was a block inside `register-form` reading the registration + form directly; it is a component now, used by both (`05bac5ed`). + +- **A password is refused for the corpus only when it is egregiously common** — 1000 + appearances, not one. The count was already returned and both call sites flattened it to + a boolean. Reset also judges the password against the username, which registration did + and it did not (`1cf25582`, `991e5ff9`). + +- **Five dead rules removed** — the four `password-strength-*` from the meter this replaced + and `success-header`, all with zero uses outside garden (`b50602f1`). + ### refactor/picks-namespace **Added** diff --git a/dev/e2e_boot.clj b/dev/e2e_boot.clj index 8020b3823..f410e45ce 100644 --- a/dev/e2e_boot.clj +++ b/dev/e2e_boot.clj @@ -12,6 +12,9 @@ (:require [com.stuartsierra.component :as component] [datomic.api :as d] [orcpub.system :as s] + [orcpub.routes :as routes] + [orcpub.entity.strict :as se] + [orcpub.dnd.e5.character :as char5e] [user :as dev])) (defn- item @@ -24,8 +27,35 @@ :orcpub.dnd.e5.magic-items/rarity :rare :orcpub.dnd.e5.magic-items/description (str nm " — seeded by e2e-boot.")}) +(defn- fighter-carrying + "A level 1 fighter with one of its owner's custom items equipped, so a browser check can open a + character whose sheet needs an item from another account." + [character-name item-key] + {::se/summary {::char5e/character-name character-name + ::char5e/classes [{::char5e/class-name "Fighter" ::char5e/level 1}]} + ::se/selections [{::se/key :ability-scores + ::se/option {::se/key :standard-scores + ::se/map-value {::char5e/str 15 ::char5e/dex 14 ::char5e/con 13 + ::char5e/int 12 ::char5e/wis 10 ::char5e/cha 8}}} + {::se/key :class + ::se/options [{::se/key :fighter + ::se/selections [{::se/key :levels + ::se/options [{::se/key :level-1}]}]}]} + {::se/key :other-magic-items + ::se/options [{::se/key item-key}]}]}) + +(defn- knowing-language + "The same fighter, also knowing a homebrew language. The browser suite puts that language in the + owner's local library, which is where homebrew lives; the server never has it." + [character-name item-key language-key] + (update (fighter-carrying character-name item-key) ::se/selections conj + {::se/key :languages ::se/options [{::se/key language-key}]})) + (defn -main [& _] - (let [sys (component/start (s/system :dev)) + ;; The :dev system pins port 8890; run.sh passes E2E_PORT through as PORT. + (let [port (some-> (System/getenv "PORT") Integer/parseInt) + sys (component/start (cond-> (s/system :dev) + port (assoc-in [:service-map :io.pedestal.http/port] port))) conn (get-in sys [:conn :conn])] ;; TWO accounts, each with its OWN items. One account cannot show isolation: ;; a bug that returned every user's items would look identical to correct @@ -43,6 +73,23 @@ (item "kaylee" "Kaylee Seeded Beta") (item "kaylee" "Kaylee Seeded Gamma") (item "zoe" "Zoe Seeded Only")]) + ;; Printed so a check can open the character without logging in. + (let [{:keys [status body]} (routes/do-save-character (d/db conn) conn + (fighter-carrying "Bree Tinker" :kaylee-seeded-alpha) + {:user "kaylee"})] + (println "E2E-CHARACTER" status (:db/id body))) + (let [{:keys [status body]} (routes/do-save-character (d/db conn) conn + (knowing-language "Wren Holloway" :kaylee-seeded-beta :e2e-cant) + {:user "kaylee"})] + (println "E2E-HOMEBREW-CHARACTER" status (:db/id body))) + ;; A character whose last share link expired, as prune! leaves it: a note of the character and the date. + ;; Ten days ago, well inside the window, so the heartbeat's first prune keeps the note. + (let [{:keys [status body]} (routes/do-save-character (d/db conn) conn + (knowing-language "Ash Marlow" :kaylee-seeded-gamma :e2e-cant) + {:user "kaylee"})] + @(d/transact conn [{:orcpub.share-expiry/character (:db/id body) + :orcpub.share-expiry/on (java.util.Date. (- (System/currentTimeMillis) (* 10 24 60 60 1000)))}]) + (println "E2E-EXPIRED-CHARACTER" status (:db/id body))) (println "E2E-READY") (flush) @(promise))) diff --git a/docs/CUSTOM_ITEMS_INVESTIGATION.md b/docs/CUSTOM_ITEMS_INVESTIGATION.md deleted file mode 100644 index 57814d8dc..000000000 --- a/docs/CUSTOM_ITEMS_INVESTIGATION.md +++ /dev/null @@ -1,1086 +0,0 @@ -# Custom Items "Disappearing" — Branch Investigation Log - -Working notes for Orcpub/orcpub#669 and related reports of server-persisted -custom magic items appearing blank, missing, or stale on the character sheet, -in the item list, and in PDF output. - -Originally written on `claude/fix-custom-items-disappearing-DW8rb`, which was -re-homed as `fix/filtered-list-staleness` to be mergeable. - -## STATUS: merged into integration - -These are the working notes as they stood during the investigation, kept for the -reasoning behind P1-P5. They are a historical record, not a plan: where they -describe work as pending, or route the Playwright spec to `testing/develop`, that -is what was expected at the time and not what happened. The spec never became -runnable and did not land. - -What actually verified the fix is on `agents/develop` under `docs/kb/`: -`filtered-list-staleness.md` (the defect), `plan-669-merge-verification.md` (the -plan and its results), and `handoff-669-final-pass.md`. The regression net that -did land is `test/cljs/orcpub/dnd/e5/filtered_list_reactivity_test.cljs`. - -Commits on branch (oldest to newest, all behind `d42e05d` base): - -| Commit | Type | Summary | -|---------|------|---------| -| `1248e8e` | test | equipment-subs tests (guard gap + filter reactivity regression) | -| `0024e0b` | fix | **P1** — filtered-items/filtered-spells reactive (#669 root cause fix) | -| `910fd53` | refactor | **P2** — get-auth-token to event_utils, auth path consolidation | -| `dbc8657` | fix | **P3** — 401 observability breadcrumb on custom-items | -| `73ce135` | refactor | **P4** — orphaned ::mi5e/remote-item chain commented with explainer | -| `d9f4863` | refactor | **P5** — reg-api-sub HOF, 5 API-backed subs migrated | -| `0553b9c` | test | **P5-guard** — :user compound on-401 pinned via pure-fn extraction | -| `cbc63e4` | test | Playwright spec staged for cherry-pick to testing/develop | - -All code changes verified via `lein fig:test` (cljs) and `lein test` (cljc + -clj) contracts — tests land with the code that makes them green. The -Playwright spec is an inert staged file on the fix branch; it activates on -testing/develop. - -### Scope boundaries - -**Fixed:** -- #669 root cause: `::char5e/filtered-items` / `::char5e/filtered-spells` - snapshot-to-db pattern that froze the item/spell list after any - filter-box interaction, breaking refresh after create/edit/delete. -- Latent `::mi5e/remote-item` stale-`:user` guard bug (inside the P4 - discarded form, so inert today but correct if restored). -- Scattered inline `(:token (:user-data @app-db))` guards — consolidated - via `get-auth-token` (canonical SSOT) and the `reg-api-sub` HOF. -- `auth-headers` DRY cleanup — now delegates to `get-auth-token`. -- Test coverage gap: no `::mi5e/custom-items` guard tests previously. - -**Not fixed / out of scope (logged as follow-ups below):** -- `db[:user]` near-dead storage cleanup -- `db[:user-data][:user-data]` double-nesting fix -- Cross-user item sharing (P4 preserves the groundwork for this) - -### Final plan vs. executed plan - -The plan locked during iteration matches what was executed with two -corrections from user pushback mid-flight: - -1. `logged-in?` predicate was dropped — `get-auth-token` is used directly - as the guard. The wrapper would have been a thin `(some? ...)` that - didn't clear the "helper needs a legitimate reason to exist" bar. - Strong docstring on `get-auth-token` captures the intent and the - promotion trigger if the definition of "logged in" ever grows. - -2. The `reg-api-sub` HOF was extracted into a new dedicated cljs-only - namespace `orcpub.dnd.e5.api-subs` rather than inlined in `subs.cljs` - or shoehorned into `event_utils.cljc`. The new namespace keeps - cljs-specific imports (cljs-http, core.async, reagent.ratom, - reg-sub-raw) isolated from the shared .cljc module and avoids - creating a new `equipment_subs → subs` dependency edge. - -### Key decisions captured for agents/develop KB reconciliation - -These are the non-obvious points that should propagate back to the KB -when the work merges: - -1. **Canonical auth path and dual use** — `event_utils.cljc` now owns - the `[:user-data :token]` path inside `get-auth-token` with a - docstring that documents the retrieval-vs-guard dual use and the - promotion trigger to a dedicated `logged-in?` predicate. The - `env-and-auth.md` KB entry is the right home for a brief update - noting this function exists and what its contract is. - -2. **reg-api-sub HOF** — new pattern worth its own KB entry describing - the anti-pattern it replaces (10 lines of duplicated boilerplate - across 5 API-backed subs) and the opts contract. Suggested KB path: - `docs/kb/reframe-subscription-patterns.md` could be extended with - a "reg-api-sub helper" section, or a new `docs/kb/api-sub-helper.md`. - -3. **Filter-sub anti-pattern** — `reg-filtered-sub` in `subs.cljs` - documents the snapshot-to-db anti-pattern it replaces. This wasn't - caught by the phase-1 subscribe-outside-reactive-context refactor - because that refactor's scope was warnings, not cache staleness. - Worth a new KB entry `docs/kb/filter-sub-anti-pattern.md` that - cross-links to `re-frame-subscribe-refactor.md` to explain why - the existing phase-1 docs don't cover it. - -4. **`db[:user]` dead storage + `db[:user-data][:user-data]` nesting** — - both are existing tech debt the investigation surfaced but didn't - fix. Follow-ups below have enough detail to pick up directly. - -5. **`::mi5e/remote-item` chain intent and restore path** — P4's - block comment documents the full chain, why it's commented, and a - 5-step restore checklist. Item sharing is on the roadmap; the - restorer should not have to reconstruct any of this. - -**This is branch-specific investigation state.** Reusable findings get -reconciled back to `agents/develop` KB at the end. See -[Reconciliation plan](#reconciliation-plan-for-agentsdevelop) at the bottom. - -## Scope clarification from repo owner - -- **In scope**: server-persisted custom items (Item Builder → `POST /items` - → Datomic → `GET /items`). These are *not* exportable/importable — they - live only in Datomic, owned by `::mi5e/owner = username`. -- **Out of scope**: orcbrew export ("M" validation errors). Tracked - separately. -- **Out of scope**: plugin-defined magic items. The spec allows them under - `::plugin`'s open content-keyword but no subscription wires them into - item maps. If orcbrew content with magic items ever worked, the wiring - is gone today. Separate, much larger concern. - -## Existing KB — relevant entries on `agents/develop` - -**I should have read these first. Lesson learned.** The KB already covers -most of what I was re-deriving. Entries consulted for this investigation: - -| KB file | Coverage | -|---|---| -| `docs/kb/reframe-subscription-patterns.md` | `reg-sub-raw` HTTP pattern; auth guard placement; `:user-data` canonical token path; loading counter as int; "stale data after login" gotcha verbatim | -| `docs/kb/re-frame-subscribe-refactor.md` | Phase 1 (12 subscribe-outside-reactive fixes), including `::char5e/filter-items` and `::char5e/filter-spells` scoped to subscribe warnings, NOT cache staleness | -| `docs/kb/subscribe-refactor-phase2.md` | Phase 2 fixes including `equipment_subs.cljs::mi5e/item` conversion to `reg-sub-raw` — does NOT address the `::mi5e/remote-item` `:user` guard typo | -| `docs/kb/env-and-auth.md` | Canonical token path `[:user-data :token]`; historical `:user` sub bug fix in `subs.cljs`. **Does not claim `::mi5e/remote-item` was fixed** — my assumption it had been was wrong | -| `docs/kb/http-fx-patterns.md` | `:http` effect handler contract — consulted to confirm dispatch-vector semantics | - -## Auth chain map (full) - -**db shape:** -``` -db -├── :user-data ← real auth state -│ ├── :token ← JWT string from login response -│ ├── :user-data ← nested literal same key — actual user record -│ │ ├── :username -│ │ ├── :email -│ │ ├── :pending-email -│ │ ├── :send-updates? -│ │ └── ... -│ └── :theme ← persists across logout -│ -└── :user ← NEAR-DEAD STORAGE (see below) - └── :following ← only attribute ever populated -``` - -### Why the double-nested `:user-data` exists - -Server `:login-success` response body is `{:token "..." :user-data {:username :email ...}}`. -Client merges the whole body into `db[:user-data]`: -```clojure -(update db :user-data merge (-> response :body)) -``` -Result: `db[:user-data] = {:token "..." :user-data {...}}`. - -**This is fixable.** Unwrap at the merge site — flatten into `:token` -and `:profile` (or similar) at the same level, or use explicit -`assoc-in`. Touches login-success, set-user-data, clear-login, the -user-data subs (subs.cljs:253-270), views.cljs:1525 lifecycle read, -local-store interceptor, and the localStorage format (migration needed -for existing sessions). **Not in scope for this patch.** Logged as a -follow-up cleanup at the bottom of this file. - -### Why `db[:user]` exists (and why it's near-dead) - -`db[:user]` is read/written by: -- `events.cljs:1110 :follow-user` — `(update (:user db) :following conj username)` → `:set-user` -- `events.cljs:1205 :unfollow-user` — same shape -- `events.cljs:1622 :set-user` — `(assoc db :user user-data)` — only caller is above -- `subs.cljs:416 :user` reg-sub-raw — fetches `/user`, but **on-success is `(fn [])` — a no-op**. Response discarded. -- `subs.cljs:429` — reaction reads `(get @app-db :user [])` -- `subs.cljs:432 :following-users` — derived `(set (:following user))` - -**So `db[:user]` is only populated by the follow/unfollow flow, starting -from nil ghost state.** The `:user` reg-sub-raw fetches `/user` but -throws the response away. Whatever data the `/user` endpoint returns -(profile, following list, etc.) is never installed. The `:following` -list gets locally assembled via `:follow-user` dispatches that do -`(update nil :following conj username)` → `{:following (...)}`. - -**Implication for `::mi5e/remote-item`'s broken guard:** -```clojure -(when (and (:user @app-db) (:token (:user @app-db))) ...) -``` -`(:user @app-db)` returns the ghost `{:following ...}` map or nil. -`(:token ...)` has **never** been true because `db[:user]` has never -contained a `:token`. The guard was not "regressed" — it was wrong from -inception in 45ef969 (the commit that introduced the guards). `a0e20a8` -fixed the typo on the sibling `::mi5e/custom-items` sub but missed this -one. - -**This is not `:user` vs `:user-data` as a var-renaming collision — they -are two genuinely separate db keys.** But `db[:user]` is near-dead -storage that could be collapsed into `db[:user-data]` alongside the real -user data. Logged as a follow-up cleanup at the bottom of this file. - -**Canonical paths in current code:** -- `(-> db :user-data :token)` → JWT -- `(-> db :user-data :user-data :username)` → username (double traversal, see above) -- `(-> db :user-data :theme)` → theme (survives logout) -- `(-> db :user :following)` → follow list (ghost-state storage) - -**All auth-related functions and call sites in the codebase:** - -| Location | Shape | Role | -|---|---|---| -| `events.cljs:1992 get-auth-token` | `(-> db :user-data :token)` | Retrieves token (2 callers inject to `:http :auth-token`) | -| `event_utils.cljc:29 auth-headers` | `(let [token (-> db :user-data :token)] ...)` | Builds `Authorization` header; re-reads path inline instead of calling `get-auth-token` (DRY miss) | -| `events.cljs:430 authorization-headers` | `(def ... event-utils/auth-headers)` | Alias used by ~25 callers in events.cljs | -| 7× inline `(when (:token (:user-data @app-db)) ...)` | Guard | `::mi5e/custom-items`, `::mi5e/remote-item` (BROKEN — uses `:user`), `:verify-user-session`, `::char5e/characters`, `::party5e/parties`, `:user`, `::folder5e/folders` | -| `events.cljs:1594 :set-user-data` | `(update db :user-data merge ...)` | State mutation | -| `events.cljs:1599 :clear-login` | `(update db :user-data dissoc :user-data :token)` | Removes both nested user-data AND token; theme survives | -| `events.cljs:1795 :login-success` | `(update db :user-data merge (-> response :body))` | Installs token + nested user-data atomically | -| `subs.cljs:247-270` user-data subs | `(-> db :user-data :user-data :username)` etc. | User data field readers | -| `subs.cljs:424 :user sub on-401` | Same action as `:clear-login`, inlined | Only sub that clears login state on 401 | -| `events.cljs:1110,1205` `:follow-user`/`:unfollow-user` | `(update (:user db) :following ...)` | The ONLY live writers of `db[:user]` | -| `events.cljs:1622 :set-user` | `(assoc db :user ...)` | Setter (only called from follow/unfollow) | -| `subs.cljs:416 :user` reg-sub-raw | Fetches `/user`, on-success is `(fn [])` | Response discarded — reg-sub-raw is effectively fire-and-forget | - -**State transitions — verified:** - -- `:login-success` installs token + nested user-data atomically. No state - with one but not the other. -- `:clear-login` (and `:user` sub's on-401 handler) removes both atomically. -- Page reload: `user->local-store-interceptor` persists `(:user-data db)` - after every mutation; `:initialize-db` rehydrates. - -**Conclusion**: "has token" and "logged in" are currently equivalent. -`db[:user]` is not part of the login decision — it's ghost state fed by -the follow/unfollow flow. The auth check is always against -`db[:user-data][:token]`. - -## Architecture map (kept for branch convenience) - -### Client ⇄ server (all confirmed symmetric) -``` -POST /api/dnd/e5/items → routes.clj:968 save-item → save-entity (create/update by :db/id) -GET /api/dnd/e5/items → routes.clj:1004 item-list → query [?e ::mi5e/owner ?username] -DEL /api/dnd/e5/items/:id → routes.clj:983 delete-item -``` -Owner field `::mi5e/owner`, populated from `(:user identity)` which always -carries username (JWT only encodes username — see `routes.clj:230 create-token`). - -### Client subscription chain -``` -(reg-sub-raw ::mi5e/custom-items ...) equipment_subs.cljs:33 - guard: (when (:token (:user-data @app-db))) ; fixed by a0e20a8 - effect: GET /items, dispatch ::mi/set-custom-items - read: (get @app-db ::mi5e/custom-items []) - ↓ -::mi5e/expanded-custom-items equipment_subs.cljs:54 - ↓ (expand-magic-items: weapon/armor subtype expansion + add-key) -::char5e/sorted-items equipment_subs.cljs:71 = expanded + SRD delay - ↓ -::mi5e/magic-weapons / magic-armor / other-magic-items reg-sub, plain - ↓ -::mi5e/magic-weapon-map / magic-armor-map / other-magic-items-map (map-by-key-or-id) - ↓ -::mi5e/all-magic-items-map (merge of 3 maps + static) -``` - -### Template chain (what fills inventory dropdowns) -``` -::mi5e/magic-weapon-options (via magic-item-options xform) - :<- [::mi5e/magic-weapons] - ↓ -::char5e/template-selections equipment_subs.cljs:289 - ↓ -::char5e/template - ↓ -:built-template subs.cljs:300 — propagates reactively to the dropdown -``` - -### Character build (debounced — affects stats, not dropdown options) -``` -:built-character subs.cljs:342 - reg-sub-raw with debounced-build-sub (500ms leading+trailing) -``` - -## Bugs — status table (live-updated) - -| # | Bug | File:line | Status | Confirmed by | -|---|---|---|---|---| -| 1 | `::char5e/filtered-items` snapshot-to-db staleness | subs.cljs:956 + events.cljs:2379 | **✅ confirmed** | Static reading of filter-items event handler + sub fallback logic | -| 2 | `::char5e/filtered-spells` same shape | subs.cljs:946 + events.cljs:2367 | **✅ confirmed** | Identical pattern to #1 | -| 3 | `::mi5e/remote-item` guard uses stale `:user` key | equipment_subs.cljs:253 | **✅ confirmed** + **orphaned** | Broken from inception in 45ef969; also the sub has zero live subscribers. Resolution via P4: comment out the whole chain, fix the guard inside the discarded form so future restorer gets it right. | -| 4 | Reg-sub-raw reaction cached at `[]` after guest→login transition | equipment_subs.cljs:33 | **Known/documented** in `reframe-subscription-patterns.md`. Current workaround: navigate away and back. Whether to fix is a design decision, not a clear bug. | -| 5 | Silent `:on-401 (fn [])` swallow with no retry | equipment_subs.cljs:44 | **Intentional** per auth-cleanup work. Changing to `:route-to-login` risks login loops. Adding console.warn is safe. | -| 6 | PDF first-click lazy-load race | views.cljs:8065 | **Speculative**. Only affects users who land on character list and click print before `GET /items` returns. No code change proposed without repro. | - -## Retracted hypotheses - -Listed here so I don't re-discover them. - -- **`change-inventory-item-quantity` strips `::char-equip5e/name`** (events.cljs:1385) — *retracted*. `::char-equip5e/name` is only set on user-typed custom items under `::entity/values`. Magic items under `::entity/options` never have this key set. `select-keys` dropping it is a no-op. -- **Load-time race between `:initialize-db` rehydration and first subscribe** — *weakened*. `web/cljs/orcpub/core.cljs:27` calls `dispatch-sync :initialize-db` synchronously before `rdc/render` at line 123. User-data is hydrated before any subscription runs. The race only exists in the narrow guest→login case (see bug #4). -- **Plan to replace `reg-sub-raw` with plain `reg-sub` + eager fetch on login** — *rejected*. Trades away lazy loading (pages that don't need items don't pay the fetch cost) for a speculative recovery path. Lazy load is a legitimate UX feature. -- **Introduce new `logged-in?` helper** — *superseded*. `get-auth-token` at `events.cljs:1992` already exists and is the correct function; it just needs to live in `event_utils.cljc` so the sub files can use it. - -## Confirmed non-issues - -- `:initialize-db` IS dispatched synchronously before render. Common path is not racing. -- Save/load round-trip is symmetric: both use `(:user identity)`, both serialize via `d/pull '[*]`. -- `item-save-success` correctly upserts by `:db/id` into `::mi/custom-items`. No duplication. -- `from-internal-item` preserves `:db/id` in its `select-keys`, so edits go to `update-entity`. -- No `@(subscribe [::mi5e/custom-items])` callers anywhere. All consumers are chained subs or non-reactive `@re-frame.db/app-db` reads (`options.cljc:1167`, `:1746` — the latter is the canonical pattern for non-reactive contexts per KB Pattern 5). - -### What the existing `subs_test.cljs` tests ACTUALLY cover - -**Correction**: earlier I said existing tests are "most of" what P5 -needs. User pushed back and asked me to verify. Audit result: - -| Coverage | Subs | -|---|---| -| Guard skips fetch when no token | characters, parties, :user, folders | -| `login-optional?` doesn't bypass guard | characters, parties | -| Stale `:user` key doesn't accidentally pass | :user | -| Reaction returns default when db empty | all 4 | -| Loading counter not incremented on skip | characters, parties, folders | - -**Not covered**: -- On-success dispatches `set-event` with body -- On-401 dispatches correct event(s) (silent / default / conditional / - compound) -- On-500 dispatches correct event(s) -- Reaction reads cached value once db populated -- `::mi5e/custom-items` at all — coverage gap in current code - -**Implication for P5**: existing tests catch **guard regressions** -post-refactor. They do NOT catch **response-handling regressions**. -The `:user` sub's compound on-401 is the highest-risk refactor in P5 -and is **not** currently covered. For real P5 verification of -response-handling equivalence, need new tests that stub `:http` and -assert on dispatch paths. KB confirms `reg-fx` stubbing works without -`re-frame-test`. - -### DB mocking concern — not needed for guard tests - -User raised: "lein-based custom-items test is complicated by the need -for a DB unless one can be mocked." Verified: existing `subs_test.cljs` -avoids the DB question entirely by only testing guard behavior. From -the test file comment (line 58-59): - -> The go block will fire and try HTTP (which will fail in test env), -> but the reaction should still return [] since no data is cached yet. - -Tests accept silent HTTP failure and only assert on guard-skip / -guard-pass behavior + reaction defaults. No DB needed. - -For P1 filter-reactivity tests: no HTTP or DB needed either — just -`reset!` app-db with test data in `::mi/custom-items`, dispatch -filter events, assert sub output. - -For P5 response-handling tests: DB not needed, but **HTTP stubbing -is needed** via `(re-frame.core/reg-fx :http (fn [_] ...))` to -return controlled responses and assert the dispatch paths. More work -than the guard-only pattern but achievable without `re-frame-test`. - -### Branch restriction — confirmed - -System prompt: "DEVELOP all your changes on the designated branch -above. NEVER push to a different branch without explicit permission." -Designated branch: `claude/fix-custom-items-disappearing-DW8rb`. - -**Cannot push to `testing/develop`.** Playwright spec must be -staged on the fix branch for later cherry-pick. - -**Staging strategy**: put the Playwright spec at its target path -(`e2e/scenarios/custom-items.spec.ts`) on the fix branch. File won't -run without `e2e/` infrastructure (package.json, playwright.config, -fixtures) but it's inert text until it lands on `testing/develop`. -Header comment documents the transfer path: - -```typescript -/** - * STAGED FOR testing/develop - * - * This spec depends on e2e/ infrastructure that only exists on - * testing/develop. To activate: cherry-pick this file onto - * testing/develop and run Playwright there. - */ -``` - -Transfer = one cherry-pick or file copy. - -## Test strategy - -**Correction from earlier in the investigation**: I was hand-wringing -about P5's refactor risk under "no e2e means no verification." That -was wrong — both `lein test`/`lein fig:test` and Playwright exist. - -### Available infrastructure - -- **`lein test`** (JVM, Clojure): runs `test/clj/` + `test/cljc/`. - 123 tests + 332 assertions per `docs/kb/testing-infrastructure.md` - on `agents/develop`. -- **`lein fig:test`** (CLJS → browser): compiles `test/cljs/` tests; - runs interactively in browser (no headless runner configured). -- **Playwright E2E**: lives on `testing/develop` branch. Full - `e2e/` directory with `playwright.config.ts`, fixture utilities - (`waitForAppReady`, `setupConsoleCapture`), scenario files, - agent-friendly JSON reporter, and local + Codespace runner - scripts. **No existing spec for item-builder / custom-items** — - would need to be added. -- **Existing `test/cljs/orcpub/dnd/e5/subs_test.cljs`**: 11 tests - covering the token-guard behavior for 4 of the 5 API subs I'm - touching (`::char5e/characters`, `::party5e/parties`, `:user`, - `::folder5e/folders`). **Missing**: `::mi5e/custom-items` tests - (lives in `equipment_subs.cljs`, no corresponding test file). -- **re-frame.test NOT available** per the KB — testing uses - `dispatch-sync` + `reset! app-db` + direct deref assertions. - -### Per-patch test coverage to add - -- **P1** (filter HOF): new regression tests proving - `filtered-items`/`filtered-spells` recompute when - `::mi/custom-items` / `::mi/plugins` (for spells) change, plus - tests that the filter event no longer writes a snapshot to db. - New file `test/cljs/orcpub/dnd/e5/equipment_subs_test.cljs` or - extend `subs_test.cljs`. -- **P2** (`get-auth-token` consolidation): existing `subs_test.cljs` - tests already cover guard behavior for 4 of 5 sites — they'll - continue passing post-refactor as built-in regression checks. - Add `::mi5e/custom-items` guard tests (fills a pre-existing - coverage gap). Add a cljc test for `get-auth-token` itself in - `test/cljc/orcpub/dnd/e5/event_utils_test.cljc`. -- **P3** (console.warn observability): minimal test surface. One - test asserting 401 path doesn't dispatch `:route-to-login` - (preserving current silent behavior). -- **P4** (comment out remote-item): nothing to test — discarded - forms aren't evaluated, so no subs are registered. Could assert - `(rf/subscribe [::mi5e/remote-item 1])` doesn't find a registered - sub, but that's just asserting discarded code is discarded. Skip. -- **P5** (`reg-api-sub` HOF): the critical test surface. - - Existing subs_test.cljs covers 4 of 5 post-migration sites. - - Add tests for the `:user` sub's compound `on-401` (dispatches - `:set-user-data` AND conditionally `:route-to-login`) — the - trickiest variant. - - Add a test for the HOF itself: register a throwaway sub via - `reg-api-sub`, manipulate `app-db`, assert correct response - shape. Tests the abstraction, not just its applications. - -### E2E coverage for #669 (the user-facing symptom) - -Playwright spec in `e2e/scenarios/custom-items.spec.ts` on -`testing/develop`: - -- `item list refreshes after creating a new custom item` -- `item list refreshes after editing a custom item` -- `item list refreshes after deleting a custom item` -- `filter-then-create still shows new item` - -These are the user-visible regressions from #669. Unit tests prove -the data layer; Playwright proves the UI actually re-renders. - -### Branch strategy for tests (pending user decision) - -Three options: -- (A) Pull e2e infrastructure from `testing/develop` onto - `claude/fix-custom-items-disappearing-DW8rb`. Bundles everything - in one branch; cross-cuts are harder to review. -- (B) cljs unit tests on fix branch, e2e spec on `testing/develop` - as a separate commit after the fix merges. Gap between merge and - e2e coverage. -- (C) Both — cljs unit tests land with the fix on the fix branch; - Playwright spec lands on `testing/develop` after merge. Each - branch stays focused. Cleanest story. - -Leaning (C). - -### Revised commit sequence (assuming option C) - -1. `test: add equipment-subs test file covering custom-items guard + filter reactivity` -2. `fix: make filtered-items/filtered-spells reactive (P1)` — regression tests from step 1 now pass -3. `refactor: move get-auth-token to event_utils, consolidate inline guards (P2)` -4. `fix: log 401 on custom-items fetch for session-stale diagnosis (P3)` -5. `refactor: comment out orphaned remote-item chain with explainer (P4)` -6. `refactor: extract reg-api-sub HOF, migrate 5 API subs (P5)` — existing subs_test.cljs + new custom-items tests verify equivalence -7. (On `testing/develop` after merge) `test: add e2e spec for custom items create/edit/delete refresh` - -Each commit independently revertable. Each with tests at the -appropriate layer (unit for data-layer changes, e2e for -user-visible symptom). - -## Planned fix — current state - -Five patches, all on `claude/fix-custom-items-disappearing-DW8rb`. - -### Patch P1: `::char5e/filtered-items` + `::char5e/filtered-spells` via HOF - -**Files**: `subs.cljs`, `events.cljs` - -Introduce a HOF `reg-filtered-sub` that takes a sorted-sub vector, a -filter-text-sub vector, a filter function, and a min-length. Both subs -use it. The HOF's docstring is the canonical anti-pattern documentation: -storing filtered results in db breaks reactivity when the underlying -list changes. - -Drop the snapshot write from both `::char5e/filter-items` and -`::char5e/filter-spells` events — they only store the filter text from -here on. - -**Non-obvious benefit (to document in-code + KB)**: per-keystroke filtering -becomes strictly cheaper. The current event-handler path calls -`compute-sorted-items`/`compute-sorted-spells` from scratch every keystroke, -bypassing re-frame's sub cache. Reactive path memoizes the upstream chain -and only re-runs the filter step per keystroke. Counterintuitive unless -you know re-frame memoization rules. - -### Patch P2: Move `get-auth-token` to event_utils, use it as the guard - -**Files**: `event_utils.cljc`, `events.cljs`, `equipment_subs.cljs`, `subs.cljs` - -**Motivation**: the 7 inline guards of the form -`(when (:token (:user-data @app-db)) ...)` are scattered across three -files, and one (`::mi5e/remote-item`, bug #3) is misspelled as `:user` -instead of `:user-data` from inception in 45ef969 (Aug 2025). Verified -by exhaustive grep: - -- Only writer of `db[:user]` is `:set-user` at `events.cljs:1624` -- Only callers of `:set-user` are `:follow-user` / `:unfollow-user`, - both passing `{:following [...]}` shapes — never `:token` -- No macro touches `db[:user]` -- `user->local-store-interceptor` persists `:user-data` only -- `:initialize-db` hydrates `:user-data` only -- `:user` reg-sub-raw's on-success is `(fn [])` — response discarded -- Grep across src/cljs, src/cljc, test/, web/ confirms no other writers - -So `db[:user]` has never contained `:token`. Guard has been false 100% -of the time since the commit that introduced it. - -**Additional finding**: `::mi5e/remote-item` has ZERO live callers. -Only in-tree reference is at `equipment_subs.cljs:272`, inside a `#_` -reader-discard at lines 268-273 (the commented-out `::mi5e/item` -dispatcher). The broken guard has zero observable effect today — -nothing subscribes to this sub. Fix is hygiene (clean up latent bug -before someone un-comments or adds a caller), not active bleeding. - -### Why `get-auth-token` directly (not a wrapper) — SSOT via docstring - -Considered adding a `logged-in?` predicate as `(some? (get-auth-token db))`. -Dropped: the wrapper is a one-liner that doesn't do anything the -underlying function doesn't already do. `(when (get-auth-token db) ...)` -is idiomatic Clojure — nil is falsy, retrieval-as-predicate is standard. - -**SSOT analysis** — two concerns, one function: - -- **Path SSOT** ("where is the token stored?"): `get-auth-token` already - captures `(-> db :user-data :token)` in one place. Move it to - `event_utils.cljc` and refactor `auth-headers` to call it and you've - got one canonical location for the path. -- **Check SSOT** ("what does it mean to be logged in?"): today the - check is `(some? (get-auth-token db))`, used at all 7 guard sites - via `when`. This is **SSOT by convention** — uniform call-site - pattern, not code-enforced. A wrapper would upgrade to code-SSOT, - but the check is currently trivial enough that convention-SSOT is - sufficient. - -**What would change the calculus**: if the logged follow-up cleanups -(`db[:user]` dead storage, `db[:user-data][:user-data]` double-nesting) -ever land, the definition of "logged in" could naturally become a -compound check (`token present AND profile map present AND not stale`). -At that point, inlining the compound check at call sites would violate -SSOT — that's when `logged-in?` would start earning its keep. - -**Decision**: no wrapper today. Strong docstring on `get-auth-token` in -`event_utils.cljc` that documents: - -1. The canonical path (`(-> db :user-data :token)`) -2. The dual use (retrieval for HTTP; predicate under `when` for guards) -3. The promotion trigger — if a compound check becomes needed, promote - the guard usage to a dedicated `logged-in?` predicate at that point; - don't inline the additional logic at call sites. - -The docstring IS the SSOT for the convention. Greppers who land on -`get-auth-token` see the rules immediately. - -**Steps**: - -1. Move `get-auth-token` from `events.cljs:1992` to `event_utils.cljc`. - The one live caller (`events.cljs:2166 reset-password`) gets its - import updated; events.cljs:2000 is commented out and ignored. -2. Refactor `auth-headers` in event_utils to call `get-auth-token` - internally — eliminates the duplicated path read. -3. Replace the 7 inline guards with `(get-auth-token @app-db)`: - - `equipment_subs.cljs:37` `::mi5e/custom-items` - - `equipment_subs.cljs:253` `::mi5e/remote-item` ← fixes bug #3 as side effect - - `events.cljs:1610` `:verify-user-session` - - `subs.cljs:389` `::char5e/characters` - - `subs.cljs:404` `::party5e/parties` - - `subs.cljs:419` `:user` - - `subs.cljs:452` `::folder5e/folders` -4. Add `get-auth-token` to the `:refer` lists in `equipment_subs.cljs`, - `subs.cljs`, `events.cljs` as needed. - -**Blast radius** (measured, not estimated): - -- Function moves/adds: `get-auth-token` move (1 live caller to update), - `auth-headers` internal refactor (0 callers affected — signature - unchanged). -- Guard replacements: 7 line-level changes. -- `:refer` updates: 2-3 files. -- **Tests/web references**: ZERO. Confirmed by grep across `test/` and - `web/`. -- **Downstream consumers affected**: ZERO. All changes are internal - function moves + behavior-preserving substitutions. The only - behavior change is `::mi5e/remote-item` (dead code today). -- **Total diff**: ~15 changed lines, ~5 added lines. - -**Non-obvious benefits (to comment in-code + KB)**: - -- **Typo class eliminated**: misspelled `get-auth-token` is an - unresolved-symbol compile error. Misspelled `:user-data` is silent - nil — the exact failure mode of the `::mi5e/remote-item` bug. - Routing all seven decisions through one function makes this class - impossible. -- **DRY**: `auth-headers` currently duplicates the token path read. - After the refactor, one canonical function holds the path. - -### Patch P3: `:on-401` console.warn observability - -**Files**: `equipment_subs.cljs` - -Change the two `:on-401 (fn [])` no-ops on `::mi5e/custom-items` and -`::mi5e/remote-item` to log a one-line console warning like: -``` -"custom-items: 401 fetch rejected; session may be stale" -``` - -**No behavior change**: no dispatch, no state mutation, no UX change. -**Value**: future debugging has a greppable breadcrumb. Currently users -reporting "items missing" give us nothing to correlate with logs. - -**NOT** changing to `:route-to-login` — KB `reframe-subscription-patterns.md` -and the breaking/ work explicitly avoid that because of login-loop risk. - -### Follow-up cleanups (logged but not in scope) - -These were discovered during the auth-chain audit and the pattern -smell analysis. They are worth fixing in their own patches. **Do not -bundle with the #669 fix** — they touch login flow, local-store -migration, or broad refactors with independent test surfaces. - -1. **`db[:user]` dead-storage cleanup**. The `:user` reg-sub-raw fetches - `/user` but its on-success is `(fn [])` — the response is discarded. - The only writers are follow/unfollow events that build ghost state - from nil. `:following` could live under `db[:user-data][:user-data]` - alongside the real user data. Candidate cleanup: wire the `:user` - sub's on-success to dispatch `[:set-user (:body response)]`, OR - collapse `db[:user]` entirely and move `:following` into the real - user-data path. Touches: subs.cljs:416-429, events.cljs:1110,1205,1622, - anywhere `:following-users` is used. - -2. **`db[:user-data][:user-data]` double-nesting fix**. Caused by - `:login-success` doing `(update db :user-data merge (-> response :body))` - with a body shaped `{:token ... :user-data {...}}`. Fix at the merge - site — unwrap token and profile into sibling keys (e.g., `:token` + - `:profile`), or assoc-in explicitly. Touches login-success, - set-user-data, clear-login, all user-data field subs - (subs.cljs:253-270), views.cljs:1525, local-store interceptor, and - needs a migration for existing localStorage entries that carry the - old shape. - -3. **`reg-api-sub` template extraction**. Five reg-sub-raw API-backed - subs (`::mi5e/custom-items`, `::mi5e/remote-item`, `::char5e/characters`, - `::party5e/parties`, `::folder5e/folders`, plus `:user` which is - similar) all follow the same ~10-line boilerplate: auth guard, - loading counter increment, `http/get` with `auth-headers`, loading - decrement, `handle-api-response` with success dispatch + 401 - handler + context, wrapped in `ra/make-reaction` reading a db key. - - Only ~5 elements vary: sub-key, URL, success event, on-401 - behavior, db-key for the reaction read. Each site could collapse - from ~14 lines to ~5 lines of configuration. - - **Why it's a separate patch, not bundled**: - - - Subs have subtle variations that fight a simple HOF. `::mi5e/remote-item` - stores to a keyed nested map and dispatches a merge event instead - of replacing at a top-level key. `:user` has a custom on-401 that - mutates login state. `::char5e/characters` / `::party5e/parties` - have conditional on-401 gated on a `login-optional?` query arg. - A fully-generic HOF needs 6-8 parameters and savings per site - shrink as the parameter count grows. - - The right abstraction may not be a HOF. Candidates: a `reg-api-sub` - HOF, a `defapi-sub` macro, or a smaller helper that wraps just - the "go + fetch + handle-response + loading counter" block and - leaves `reg-sub-raw` and `ra/make-reaction` explicit at call - sites. Picking between those is its own design pass. - - The 5 sites aren't going anywhere. Parking this is cheap; doing - it right later is not blocked by anything. - - **When to revisit**: after the #669 fix ships, either as a dedicated - pattern-extraction session or piggybacked on whoever touches these - files next for another reason. Worth a KB entry in `docs/kb/` on - `agents/develop` documenting the decision tree between HOF / macro - / helper before the refactor starts. - -### Patch P4: Document-and-comment the orphaned `::mi5e/remote-item` chain - -**Files**: `equipment_subs.cljs`, `events.cljs` - -The `::mi5e/remote-item` machinery is groundwork for cross-user item -viewing (item sharing). The bulk `GET /items` endpoint only returns -items the current user owns; the server has `GET /items/:id` which -returns any item by db-id regardless of owner, and this client-side -chain was meant to consume it. The live `views/item-page` at -`views.cljs:3874` punts on this — it subscribes to `::mi/custom-item` -directly, which only works for items you already own. - -**Status confirmed orphaned**: -- `::mi5e/remote-items` (plural) reg-sub: registered, zero subscribers -- `::mi5e/remote-item` (singular) reg-sub-raw: registered, zero subscribers -- `::mi/add-remote-item` event: registered, only dispatched from inside - the unsubscribed reg-sub-raw -- `::mi5e/item` dispatcher: already `#_` reader-discard, labeled - "Groundwork... restore when needed" - -**Item sharing is on the roadmap but not at the top** (per repo owner). -The groundwork should be preserved, not deleted. - -**Action**: wrap all four orphaned forms in `#_` reader-discards with -a block-comment header explaining what they do, the chain, why they're -commented out, and a concrete restore checklist. **Inside** the -commented `::mi5e/remote-item`, fix the guard from `(:user @app-db)` -to `(get-auth-token @app-db)` so the future restorer doesn't hit the -same typo that broke it originally. - -**Block-comment contents (for equipment_subs.cljs above the #_ block)**: - -``` -ORPHANED: Cross-user item detail fetch — commented out, kept for reference. - -PURPOSE: Groundwork for viewing magic items owned by OTHER users -(item sharing feature, roadmap, not yet prioritized). - -Why this exists: bulk GET /items returns only items where -::mi5e/owner = current username. Server also has GET /items/:id -(routes.clj:976 get-item) which returns ANY item by db-id. -This chain was intended as the client-side consumer for the -by-id endpoint. The live views/item-page (views.cljs:3874) -bypasses it by subscribing to ::mi/custom-item directly, so -visiting /items/ for an item you don't own silently falls -back to "not found." - -CHAIN (when restored): - views/item-page calls (subscribe [::mi5e/item item-key]) - ↓ ::mi5e/item dispatcher (below) - ├── int key: (subscribe [::mi5e/remote-item id]) - │ ↓ fires GET /api/dnd/e5/items/:id with auth headers - │ ↓ dispatches [::mi5e/add-remote-item (:body response)] - │ ↓ stores under db[::mi5e/remote-items][id] - │ ↓ reaction reads that path - └── kw key: (get mi5e/all-equipment-map key) via ra/make-reaction - -COMMENTED OUT because: -- Half-alive: registered in signal graph but no live subscribers. -- Broken from inception (45ef969 Aug 2025): auth guard was - (:token (:user @app-db)) but token lives at [:user-data :token]. - Guard has been false 100% of the time, so the fetch never fired - even if a subscriber had existed. Fixed below so future restorer - doesn't hit the same trap. -- Leaving registered with no consumers was confusing to auditors - (re-discovered repeatedly). - -TO RESTORE when item sharing is implemented: -1. Uncomment the four forms below and the ::mi/add-remote-item - event in events.cljs (~line 2673). -2. Update views/item-page (views.cljs:3874) to subscribe to - [::mi5e/item key] instead of [::mi/custom-item item-key], so - numeric id keys route through the remote fetch. -3. Consider whether the remote fetch should be an explicit event - on route-mount rather than a reg-sub-raw side effect — the - modern pattern. See docs/kb/reframe-subscription-patterns.md - on agents/develop. -4. Product decisions needed: can viewers edit items they don't - own? Favorite? Clone? Decide before wiring the UI. -5. Add a KB entry to docs/kb/ on agents/develop documenting the - cross-user item fetch chain once it's wired and working. -``` - -**Back-reference comment** at events.cljs:2673-2676 (where -`::mi/add-remote-item` lives) pointing at the explainer: - -``` -;; See equipment_subs.cljs ::mi5e/remote-item block-comment — -;; this event is part of the orphaned cross-user item fetch -;; chain. Kept as groundwork, commented out together with the -;; rest of the chain. Do not remove in isolation. -``` - -**Blast radius**: three #_ insertions, one block comment header, -one back-reference comment. ~60 lines total touched (mostly comment -text, not code). Zero behavior change. The `::mi5e/remote-item` -guard fix is inside the discarded form so it's inert today. - -### Dropped from plan - -- **Patch 4 (`:login-success` dispatches fetch)** — the KB documents the - stale-reaction-after-login behavior as known with a "navigate away and - back" workaround. Adding a login-success dispatch would change that - from accepted behavior to a fix. Not a clear call; deferred pending - explicit user decision. -- **PDF first-click lazy-load race** — speculative without repro. -- **Plugin → magic-items pipeline** — different, larger problem. - -## Reconciliation plan for `agents/develop` - -At the end of this investigation, reconcile findings into the KB per -project convention (docs/kb is the OrcPub Agent Knowledge Base). - -### New KB entries to add - -1. **`docs/kb/filter-sub-anti-pattern.md`** — document the snapshot-to-db - anti-pattern, the `reg-filtered-sub` HOF fix, and why phase-1 subscribe - refactor (`re-frame-subscribe-refactor.md`) preserved the anti-pattern - (its scope was warnings, not cache staleness). Cross-link from the - existing subscribe-pattern docs. - -### KB entries to update - -1. **`docs/kb/env-and-auth.md`** — explicit mention of `::mi5e/remote-item` - as the remaining typo instance after `a0e20a8`, or mark it as fixed - once the patch lands. Currently the KB is silent on this sub and a - casual reader might assume all siblings were fixed together. - -2. **`docs/kb/reframe-subscription-patterns.md`** — add `get-auth-token` - as the canonical guard function (not just the inline pattern). Cross- - link to `env-and-auth.md`. - -3. **`docs/kb/http-fx-patterns.md`** — brief note on silent `:on-401` - swallow as an intentional pattern to avoid login loops, with the - caveat that adding console.warn for diagnostics is zero-risk. - -### Process lesson to capture - -Add to `DOC-CONVENTIONS.md` or a new onboarding section: **"Read the -docs/kb/ index on agents/develop before investigating domain issues."** -The KB is already comprehensive; investigations that skip it end up -re-deriving documented patterns and risk missing nuances. - -## Session activity log — execution phase - -- **P1 regression tests** (`1248e8e`): new `equipment_subs_test.cljs` - with 10 deftests covering custom-items guard behavior (gap fill), - filtered-items reactive recomputation, filtered-spells parallel - cases, and the simulated-save-refresh flow that matches the - reporter scenario verbatim. These fail against the pre-P1 code. - -- **P1 code fix** (`0024e0b`): extracted `reg-filtered-sub` HOF in - `subs.cljs` with the anti-pattern docstring. Migrated both - filtered-items and filtered-spells subs. Simplified the - `::char5e/filter-items` and `::char5e/filter-spells` event - handlers to only store filter text. Dropped 4 dead local aliases - in events.cljs (compute-sorted-spells, compute-sorted-items, - filter-spells, filter-items) — kept filter-by-name-xform because - `search-results` still uses it. Updated existing events_test.cljs - filter tests to assert via the reactive sub rather than the dead - db snapshot. - -- **P2 auth consolidation** (`910fd53`): moved `get-auth-token` from - `events.cljs` to `event_utils.cljc` with a comprehensive docstring - declaring it the SSOT for the `[:user-data :token]` path and for - the logged-in check. Refactored `auth-headers` to call it - internally. Updated `:verify-user-session` and `:delete-character` - to use `event-utils/get-auth-token`. Added - `get-auth-token-test` in `event_utils_test.cljc`. - -- **P3 observability** (`dbc8657`): one-line change replacing the - silent `(fn [])` `:on-401` on `::mi5e/custom-items` with a - `console.warn`. Surrounding comment explains why the silent path - exists (login-loop risk) so future maintainers don't "fix" it by - switching to `:route-to-login`. - -- **P4 orphaned chain** (`73ce135`): wrapped three forms in `#_` - reader-discards, added a block-comment header with the full - restore checklist, fixed the guard inside the discarded form so - the next restorer inherits the correct `get-auth-token` check. - Cross-referenced from `events.cljs` where - `::mi/add-remote-item` is also commented. - -- **P5 HOF extraction** (`d9f4863`): created - `src/cljs/orcpub/dnd/e5/api_subs.cljs` with the `reg-api-sub` HOF. - Migrated 5 API-backed subs: `::char5e/characters`, - `::party5e/parties`, `::folder5e/folders`, `:user` (compound - on-401 preserved), and `::mi5e/custom-items` (inside the existing - `if js/window.location` compile-time split). Removed unused - `auth-headers` from subs.cljs :refer list. Added `[re-frame.db]` - import to subs.cljs for the `:user` on-401 callback's - `@re-frame.db/app-db` reference. - -- **P5 test pinning** (`0553b9c`): extracted - `user-sub-on-401-actions` as a pure fn and `user-sub-on-401` as - its side-effecting wrapper. Added 5 deftests pinning the - compound behavior: required?-true produces both dispatches, - required?-false/nil/absent suppresses the route-to-login, theme - survives the login clear, empty user-data defensive case, and - the query-v destructuring contract. - -- **Playwright spec staging** (`cbc63e4`): created - `e2e/scenarios/custom-items.spec.ts` with four scenarios covering - the user-facing #669 regression surface. Header comment documents - the transfer path (single-file cherry-pick to `testing/develop`). - The spec is inert on the fix branch — no playwright infrastructure - there — but activates the moment it lands on testing/develop. - -## Session activity log — investigation phase - -Appended live as the investigation proceeds. - -- **Initial reading**: static analysis of equipment_subs.cljs, events.cljs, - routes.clj; git history of custom-items hotfixes. -- **First (over-broad) plan**: proposed converting reg-sub-raw to plain - reg-sub + eager fetch on login. Rejected by repo owner — lazy loading - has UX value. -- **Retraction**: `change-inventory-item-quantity` name-stripping - hypothesis withdrawn after re-reading `::char-equip5e/name` call sites. -- **Narrowed scope**: scope limited to server-persisted custom items per - repo owner. Orcbrew export is separate. -- **Found filtered-items bug**: user prompt about item list not - refreshing → confirmed in static reading of `::char5e/filter-items` - event + `::char5e/filtered-items` sub. -- **Found dropdown/PDF chain distinction**: user prompt led to tracing - `:built-template` (reactive, immediate) vs. `:built-character` - (debounced) vs. PDF `plugin-data` capture (snapshot at render). -- **Discovered agents/develop KB**: user corrected "agents/develop is a - branch, not a path." Fetched branch, read 5 relevant KB entries. - Reconciled findings — filtered-items bug is genuinely new, remote-item - typo is real, most other analysis was duplicative. -- **Discovered get-auth-token already exists**: user prompt led to - finding events.cljs:1992 — first-pass revision of Patch P2 to reuse - it as the guard predicate. -- **Deeper auth-chain audit** (earlier this session): user pushed back - on the "just reuse `get-auth-token`" plan and asked whether - `logged-in?` is more semantic. Exhaustive grep of auth-related sites - revealed 7 inline guards (one with the `:user` typo from - `::mi5e/remote-item`), plus `get-auth-token`, `auth-headers`, - `authorization-headers` alias, login/logout state mutations. - Initial revision: add `logged-in?` alongside `get-auth-token` as - separate predicate, with extensibility and semantic intent arguments. - -- **`logged-in?` final drop** (earlier, this session): user asked - "so `logged-in? db` is just going to swap out `get-auth-token db`?" - — correct instinct. The wrapper is a one-liner `(some? ...)` that - doesn't add any check the underlying function doesn't already - provide. The "grep role separation" and "reading clarity" arguments - are real but tiny and don't clear the "legitimate reason to exist" - bar. Dropped. - -- **Test-strategy correction** (latest session): user called out - that I was hand-wringing about "no e2e means no verification" when - both `lein fig:test` (cljs) and Playwright (`testing/develop`) are - available. Existing `test/cljs/orcpub/dnd/e5/subs_test.cljs` - already covers the token-guard behavior for 4 of the 5 API subs - I'm refactoring — those tests are built-in regression checks for - P2/P5. Need to add: `::mi5e/custom-items` guard coverage (fills a - pre-existing gap), filter-reactivity regression tests for P1, - compound on-401 test for the `:user` sub variant in P5, and a - test for the `reg-api-sub` HOF itself. E2E for #669's user-facing - symptom goes to `testing/develop` as a separate follow-up commit. - Full test strategy documented above (see "Test strategy" section). - -- **`::mi5e/remote-item` scope decision** (earlier, this session): user asked - whether the sub is orphaned and what it's supposed to do. Verified - all four pieces (remote-items plural, remote-item singular, - add-remote-item event, #_'d ::mi5e/item dispatcher) — registered - but zero live subscribers. Purpose: groundwork for cross-user item - viewing (item sharing roadmap feature, not top priority). Server - endpoint GET /items/:id exists and returns any item by id; client - consumer was half-written. Live item-page bypasses it via - ::mi/custom-item, so cross-user URLs silently fail. - - Decision: **Option C-plus** — fix the guard AND comment out the - whole chain with a block-comment explainer. Preserves groundwork, - removes the confusing "registered but no consumers" state, - documents the chain and restore checklist so future implementer - (or agent re-auditing the code) has full context without needing - to reconstruct it. Became new Patch P4. - -- **Pattern smell — reg-sub-raw API template** (earlier, this session): user - owned their "noodle around when I see the same thing repeated" - instinct and asked whether there's a deeper template underneath the - `(some? (get-auth-token db))` idiom. Traced the five API-backed - reg-sub-raw subs and confirmed: yes, they all follow the same - ~10-line boilerplate (guard, loading counter, http/get with - auth-headers, handle-api-response, reaction wrapping). Extracting - a `reg-api-sub` helper would collapse each site from ~14 lines to - ~5 lines. BUT: the 5 subs have enough subtle variation - (`::mi5e/remote-item` uses a nested db key and a merge-dispatch, - `:user` has a custom on-401 that mutates login state, characters/ - parties have conditional on-401 via `login-optional?`) that a clean - HOF needs 6-8 parameters and the right abstraction might be a - macro, not a HOF. Decision: log as follow-up #3, do not bundle - with #669. The noodle was finding a real pattern, just bigger than - the `(some? ...)` layer and too much to execute mid-patch. - -- **SSOT follow-up** (earlier, this session): user asked whether - `(some? (get-auth-token db))` can be made SSOT. Clarified two - concepts: PATH SSOT (already handled by `get-auth-token` — it owns - `(-> db :user-data :token)`) vs CHECK SSOT ("what does 'logged in' - mean?", currently SSOT-by-convention at 7 uniform call sites). A - `logged-in?` wrapper would upgrade convention-SSOT to code-SSOT but - is marginal today because the check is trivial. Landed on: add a - strong docstring to `get-auth-token` that explicitly documents the - dual use and the promotion trigger (if compound checks become - needed after the `db[:user]` / double-nesting follow-ups, promote - to `logged-in?` at that moment). The docstring IS the SSOT for the - convention — no code added, rules stay visible to anyone grepping - the function. - -- **Pragmatic reasoning correction** (earlier this session): user pushed - back on both earlier positions — (a) don't roll over on pushback - without reasoning, (b) verify the `:user` vs `:user-data` claim - rigorously because macros/interceptors could be hiding things, (c) - quantify blast radius. Did the work: - - 1. **`logged-in?` decision (superseded above)**: re-weighed honestly - with grep-role-separation and reading-clarity arguments. User - subsequently correctly observed the wrapper was still thin enough - to drop. - - 2. **Guard-broken claim verified**: grep for all writers of `db[:user]` - found only `:set-user` at events.cljs:1624, called only from - follow/unfollow with `{:following ...}` shapes. No macros touching - db[:user]. `user->local-store-interceptor` persists `:user-data` - only. `:initialize-db` hydrates `:user-data` only. `:user` - reg-sub-raw's on-success is `(fn [])` — response discarded. - VERIFIED: `db[:user]` has never contained `:token`. - - BUT ALSO DISCOVERED: `::mi5e/remote-item` has zero live callers. - Only in-tree reference is inside a `#_` reader-discard at - `equipment_subs.cljs:272`. The broken guard has zero observable - effect today. Fix is hygiene, not bleeding. - - 3. **Blast radius measured**: function moves touch 1 live caller - (reset-password). 7 guard call sites replaced. 2-3 `:refer` - updates. Zero test/ or web/ references. Zero downstream - consumers affected. Total ~20 changed lines. - - Earlier "artificial-complexity" entry below remains valid for the - retraction of the extensibility argument — that retraction stands. - What changed is: I over-corrected to dropping `logged-in?` entirely, - and the user pushed back on THAT. Restored with honest reasoning. - -- **Artificial-complexity correction** (earlier session): user called out - (a) the extensibility argument as speculative — I couldn't name a - concrete scenario where "logged in" would grow beyond "has token"; - (b) my "note the double traversal — bad upstream naming, but it's - the reality" shrug as non-KISS resignation; (c) not finishing the - `:user` vs `:user-data` trace before labeling them "separate keys." - - Corrections: - 1. Dropped `logged-in?`. Revised Patch P2 to move `get-auth-token` to - event_utils.cljc and use it directly as the guard at all 7 sites. - No new predicate. KISS. - 2. Finished tracing `db[:user]`: it's NOT a rename collision with - `:user-data`, they are separate top-level keys. But `db[:user]` is - NEAR-DEAD storage — the `:user` reg-sub-raw fetches `/user` but - its on-success is `(fn [])`, discarding the response. The only - writers are `:follow-user`/`:unfollow-user` which build - `{:following ...}` from nil ghost state. The `::mi5e/remote-item` - guard has been broken since inception (45ef969), not regressed - from a rename — it was checking for `:token` under a key that - has NEVER contained a token. - 3. Flagged `db[:user-data][:user-data]` double-nesting as fixable - (unwrap at `:login-success` merge site) rather than immutable. - Added as a follow-up cleanup, not in current scope. - 4. Flagged `db[:user]` dead-storage as a follow-up cleanup. - - Updated auth chain map to reflect `db[:user]` as a separate branch - with its (near-dead) status and actual writers. This is the state - that should reconcile back to `agents/develop` KB — the existing - `env-and-auth.md` doesn't capture any of it. diff --git a/docs/ENVIRONMENT.md b/docs/ENVIRONMENT.md index cc5be5fc3..dcba517e8 100644 --- a/docs/ENVIRONMENT.md +++ b/docs/ENVIRONMENT.md @@ -52,6 +52,21 @@ See [PDF-EXPORT-CAPACITY.md](PDF-EXPORT-CAPACITY.md) for what these cost and how A value that is present but not a positive integer is reported at boot and the default is used, so a typo does not take the server down or silently mean zero. +### Share links + +A character's owner can share it with a short link. The server keeps one copy of that character's share +data, checked when it is uploaded. + +| Variable | Default | Description | +|---|---|---| +| `ORCPUB_SHARE_MAX_UPLOAD_KB` | `64` | Largest compressed share data one character's link keeps. Over it, the owner's link carries the data itself instead. A packed level 20 character measured 12 to 25 KB. | +| `ORCPUB_SHARE_MAX_TEXT_KB` | `256` | Largest that share data may be once unpacked. This also bounds what a viewer's browser unpacks. | +| `ORCPUB_SHARE_MAX_ACCOUNT_KB` | `1024` | Share data one account keeps across all its characters. | +| `ORCPUB_SHARE_PRUNE_DAYS` | `180` | Days a share link may go unused, counting only time the server is running, before the link and its share data are deleted. The character is never deleted, and its owner's page says the link expired. `0` keeps links forever; `1` to `29` count as `30`, so a typo cannot delete links a day old. | + +The size limits reach the browser in response headers, so changing one needs no rebuild. Pruning runs +hourly, after the server's heartbeat, which records when the server was off so that time never counts. + ### What the server prints at boot Every start prints what it resolved, so you never have to guess whether a change was @@ -140,6 +155,19 @@ the table in half. Plain ASCII, no colour: it is read in aggregators as often as |----------|---------|-------------| | `CSP_POLICY` | `strict` | Content Security Policy mode: `strict`, `permissive`, or `none` | | `DEV_MODE` | `"true"` (in :dev profile) | When true, **no CSP header is sent at all** — that is what lets Figwheel's `ws://localhost:3449` through. Must be the string `"true"` (case-insensitive); any other value, including `"1"`, `"yes"` or empty, is false. | +| `ORCPUB_PWNED_CHECK` | `on` | Screen new passwords against the Have I Been Pwned corpus. Accepts `on`/`off`, `true`/`false`, `1`/`0`, `yes`/`no`. | + +**About the breach check.** Signup and password reset send the first five characters of +the password's SHA-1 to `api.pwnedpasswords.com` and score the reply locally, so the +password itself never leaves the server and the size of the reply is padded. A password +is refused only at **1000 or more** appearances: the corpus measures how common a +password is, not whether this person was breached, and refusing on a single appearance +turns that measure into a veto. Below the line the strength meter says so while somebody +is still typing. + +Set `ORCPUB_PWNED_CHECK=off` as the kill switch. The check already fails open on a +timeout, so this is for the case where the service is not down but slow enough to be felt +on every signup. Turning it off changes nothing else. CSP modes: - **strict** — nonce-based CSP with `strict-dynamic`, always as an enforcing `Content-Security-Policy` header. **There is no Report-Only mode**; no code has ever emitted `Content-Security-Policy-Report-Only`. Dev mode does not soften the policy, it skips it. @@ -185,6 +213,15 @@ See `docker/transactor.properties.template` for the full transactor configuratio | `ORCPUB_ENV` | — | Set to `dev` to enable `add-test-user` in user.clj | | `FIGWHEEL_PORT` | `3449` | Figwheel WebSocket port for frontend hot-reload. Read by `scripts/common.sh`. | | `FIGWHEEL_CONNECT_URL` | *(auto-detected)* | Figwheel WebSocket URL override for remote environments (Gitpod, tunnels). Auto-detected for GitHub Codespaces. Example: `wss://my-remote-host:3449/figwheel-connect` | +| `E2E_MAIL_PORT` | `2525` | Port for the browser suites' throwaway SMTP server. `scripts/e2e/run.sh` starts it and points `EMAIL_SERVER_URL`/`EMAIL_SERVER_PORT` at it. | + +**Why the suites need a mail server.** The verification key and the password-reset key +exist only inside an email, and registration fails outright when no mail can be sent -- +the verification mail goes out inside the same try as the transaction. So signup, +verification and reset could not be driven end to end at all, and every suite worked +around it by starting from a user seeded straight into Datomic. `scripts/e2e/lib/mail-sink.js` +is a dependency-free SMTP server that accepts anything and hands the message back, which +is what lets `auth-flows.js` follow the link a real person would click. ### Branding, Social Links & Integrations diff --git a/docs/ORCBREW_FILE_VALIDATION.md b/docs/ORCBREW_FILE_VALIDATION.md index 75577eb02..6dcdd5515 100644 --- a/docs/ORCBREW_FILE_VALIDATION.md +++ b/docs/ORCBREW_FILE_VALIDATION.md @@ -127,8 +127,8 @@ The system automatically fixes these common corruption patterns: | `disabled? nil` | `disabled? false` | | `nil nil,` | (removed) | | `:field-name nil` | (removed) | -| `option-pack ""` | `option-pack "Default Option Source"` | -| Empty plugin name `""` | `"Default Option Source"` | +| No `:option-pack`, or `""` or `nil` | The name of the source the entry is filed under; in a single-source file, the one source the other entries name; otherwise `"Default Option Source"` | +| Empty plugin name `""` | `"Unnamed Content"` (numbered when there are several) | **This happens automatically** - you don't need to do anything! diff --git a/resources/public/image/ATTRIBUTION.md b/resources/public/image/ATTRIBUTION.md index 526bf1ae6..958e4f911 100644 --- a/resources/public/image/ATTRIBUTION.md +++ b/resources/public/image/ATTRIBUTION.md @@ -4,7 +4,7 @@ Most of the icons in this directory come from **[game-icons.net](https://game-ic and are used under the **[Creative Commons Attribution 3.0 Unported](https://creativecommons.org/licenses/by/3.0/)** licence, which requires that the authors be credited wherever the icons appear. -The four vendored for the PDF spell cards: +The five vendored for the PDF spell cards: | Icon | Author | |---|---| diff --git a/scripts/e2e/api-sub-loaders.js b/scripts/e2e/api-sub-loaders.js index e5fd604f1..04686af1f 100644 --- a/scripts/e2e/api-sub-loaders.js +++ b/scripts/e2e/api-sub-loaders.js @@ -85,7 +85,7 @@ const check = (n, ok, d) => { console.log(` ${ok ? 'PASS' : 'FAIL'} ${n}${d ? } }); - // Log in (helper shape lifted from fix/custom-item-classification:scripts/e2e/run.js). + // Log in. await page.goto(`${BASE}/pages/login-page`, { waitUntil: 'domcontentloaded' }); await page.waitForSelector('input'); await page.locator('input').nth(0).fill(USER); diff --git a/scripts/e2e/auth-flows.js b/scripts/e2e/auth-flows.js new file mode 100644 index 000000000..c652cb2d7 --- /dev/null +++ b/scripts/e2e/auth-flows.js @@ -0,0 +1,491 @@ +// Needs: dev bundle (reads the app's internals, which a production bundle compiles away). +// Needs: fresh server (creates accounts and spends sign-in throttles that other suites' logins would hit). +// Every state of every flow where an account is made, verified, entered or recovered. +// +// ./scripts/e2e/run.sh auth-flows.js +// +// These flows had NO coverage, and no way to get any: the verification key and the reset +// key exist only inside an email, registration itself fails when no mail can be sent, and +// every other suite sidesteps all of it by starting from a user e2e-boot seeded straight +// into Datomic. So nothing here was ever exercised, and a reset form that refused every +// ordinary password in silence reached a merge. +// +// run.sh starts scripts/e2e/lib/mail-sink.js and points the app at it, which is what makes +// the keys readable. Without E2E_MAIL_PORT this suite skips rather than pretending to pass. +const { chromium } = require('playwright'); +const { BASE, EXECUTABLE, newContext, checker } = require('./lib'); +const { startMailSink } = require('./lib/mail-sink'); + +const PORT = Number(process.env.E2E_MAIL_PORT || 0); +const GOOD = 'brass lantern rope'; +// 14 characters, passes every local rule, and the corpus has seen it thousands of +// times -- so ONLY the server can refuse it. It is the one case that proves the +// server's reason reaches the page. +const COMMON = 'letmeinletmein'; +const SHORT = 'Sw0rdfish'; + +const uniq = () => 'e2e' + Date.now().toString(36) + Math.floor(Math.random() * 1e4); +const text = (page, sel) => page.locator(sel).innerText().catch(() => ''); +const cardText = page => text(page, '.registration-content'); +const heading = page => text(page, '.auth-heading'); +const authInputs = page => page.locator('.auth-form input'); +const press = (page, label) => page.locator('.auth-tail button', { hasText: label }).click(); + +// The link in a captured mail, with quoted-printable soft breaks undone. +const linkIn = (mail, re) => (mail.raw.replace(/=\r\n/g, '').match(re) || [])[0]; + +async function registerAndVerify(page, sink, user, email, password) { + // Clear FIRST. waitFor returns a matching message already held, so with mail + // from an earlier account still in the sink this returned that one and + // re-verified somebody else -- leaving the new account uncreated and the + // failure surfacing three steps later as reset mail that never arrived. + sink.clear(); + await page.goto(`${BASE}/pages/register-page`, { waitUntil: 'networkidle' }); + await page.waitForSelector('.auth-form input'); + const b = authInputs(page); + await b.nth(0).fill(user); await b.nth(1).fill(email); await b.nth(2).fill(email); + await b.nth(3).fill(password); await b.nth(4).fill(password); + await press(page, 'JOIN'); + const mail = await sink.waitFor(new RegExp(email.replace('.', '\\.'), 'i'), 20000); + const key = (mail.raw.replace(/=\r\n/g, '').match(/verify\?key=([A-Za-z0-9-]+)/) || [])[1]; + await page.goto(`${BASE}/verify?key=${key}`, { waitUntil: 'networkidle' }); + await page.waitForTimeout(800); + return key; +} + +// A signed-in session sends the register page somewhere else, so a block that +// registers after one that logs in silently registers nobody -- which showed up +// three steps later as reset mail that never arrived. +async function signOut(ctx, page) { + await ctx.clearCookies(); + await page.goto(`${BASE}/pages/login-page`, { waitUntil: 'domcontentloaded' }); + await page.evaluate(() => { try { localStorage.removeItem('token'); } catch (e) {} }); +} + +// null when no reset mail arrives, rather than throwing: a limit that stops the +// mail is a RESULT this suite asserts about, not a crash. +async function resetLinkFor(page, sink, email) { + sink.clear(); + await page.goto(`${BASE}/pages/send-password-reset-page`, { waitUntil: 'networkidle' }); + await page.waitForSelector('.auth-form input'); + await authInputs(page).first().fill(email); + await press(page, 'SUBMIT'); + const mail = await sink.waitFor(/reset/i, 20000).catch(() => null); + if (!mail) return null; + return linkIn(mail, /https?:\/\/[^\s"'<>]*reset-password[^\s"'<>]*/); +} + +(async () => { + if (!PORT) { + console.log('SKIP auth-flows.js needs E2E_MAIL_PORT; run it through scripts/e2e/run.sh'); + process.exit(0); + } + const sink = await startMailSink({ port: PORT }); + const browser = await chromium.launch({ executablePath: EXECUTABLE }); + const ctx = await newContext(browser); + const page = await ctx.newPage(); + const { check, failures } = checker(); + const errors = []; + page.on('pageerror', e => errors.push(String(e))); + + const user = uniq(), email = `${user}@example.com`; + + console.log('registration'); + await registerAndVerify(page, sink, user, email, GOOD); + check((await heading(page)).toLowerCase().includes('registration is complete'), + 'a new account registers and verifies', await heading(page)); + + // The same address and name a second time must be refused, per field. + await page.goto(`${BASE}/pages/register-page`, { waitUntil: 'networkidle' }); + await page.waitForSelector('.auth-form input'); + let b = authInputs(page); + await b.nth(0).fill(user); await b.nth(1).fill(email); await b.nth(2).fill(email); + await b.nth(3).fill(GOOD); await b.nth(4).fill(GOOD); + await press(page, 'JOIN'); + await page.waitForTimeout(2500); + const taken = await cardText(page); + check(/taken|already/i.test(taken), 'a username and email already in use are refused', + taken.replace(/\n/g, ' | ').slice(0, 120)); + + // Short password: the rules must say what is wrong, not just refuse. + await page.goto(`${BASE}/pages/register-page`, { waitUntil: 'networkidle' }); + await page.waitForSelector('.auth-form input'); + b = authInputs(page); + await b.nth(0).fill(uniq()); const e2 = uniq() + '@example.com'; + await b.nth(1).fill(e2); await b.nth(2).fill(e2); + await b.nth(3).fill(SHORT); await b.nth(4).fill(SHORT); + await press(page, 'JOIN'); + await page.waitForTimeout(1200); + check(/at least 12 characters/i.test(await cardText(page)), + 'a short password is refused WITH its reason'); + + // The chips and the tip. Five rules, each visible as its own chip before + // anything is pressed, and a line pointing forward once nothing is wrong. + await page.goto(`${BASE}/pages/register-page`, { waitUntil: 'networkidle' }); + await page.waitForSelector('.auth-form input'); + b = authInputs(page); + await b.nth(0).fill('kaylee'); + const chipState = async () => { + await page.waitForTimeout(400); + return page.locator('.pw-chip').evaluateAll(els => Object.fromEntries( + els.map(e => [e.textContent.trim(), e.classList.contains('is-ok')]))); + }; + await b.nth(3).fill('aaaaaaaaaaaa'); + let c = await chipState(); + check(c['12 or more'] === true && c['no triples'] === false && c['some variety'] === false, + 'twelve of one letter: length passes, triples and variety do not', JSON.stringify(c)); + await b.nth(3).fill('abcdefghijkl'); + c = await chipState(); + check(c['no key runs'] === false, 'an alphabet run fails its own chip', JSON.stringify(c)); + await b.nth(3).fill('kayleekaylee12'); + c = await chipState(); + check(c['not your name'] === false, 'a password holding the username fails its own chip', + JSON.stringify(c)); + await b.nth(3).fill('brass lantern rope candle bell'); + c = await chipState(); + check(Object.values(c).every(Boolean) && Object.keys(c).length === 5, + 'a real passphrase satisfies all five', JSON.stringify(c)); + const tip = await text(page, '.pw-note'); + check(tip.length > 0, 'and the meter says what to do next rather than only what is wrong', tip); + + console.log('login'); + await page.goto(`${BASE}/pages/login-page`, { waitUntil: 'networkidle' }); + await page.waitForSelector('input'); + await page.locator('input').nth(0).fill(user); + await page.locator('input').nth(1).fill('not the password'); + await page.locator('button.form-button').click(); + await page.waitForTimeout(2500); + const wrongPass = await cardText(page); + + await page.goto(`${BASE}/pages/login-page`, { waitUntil: 'networkidle' }); + await page.waitForSelector('input'); + await page.locator('input').nth(0).fill('nobody' + uniq()); + await page.locator('input').nth(1).fill('not the password'); + await page.locator('button.form-button').click(); + await page.waitForTimeout(2500); + const noSuchUser = await cardText(page); + + // The oracle: a wrong password and a username that does not exist must be + // indistinguishable, or the login form is a membership test. + check(wrongPass === noSuchUser, + 'a wrong password and an unknown username answer identically', + wrongPass === noSuchUser ? '' : `"${wrongPass.slice(0,60)}" vs "${noSuchUser.slice(0,60)}"`); + + console.log('password reset'); + const link = await resetLinkFor(page, sink, email); + check(!!link, 'a reset link is emailed for a known address', link || 'no link in the mail'); + + // An address nobody registered must answer the same way and send nothing. + sink.clear(); + await page.goto(`${BASE}/pages/send-password-reset-page`, { waitUntil: 'networkidle' }); + await page.waitForSelector('.auth-form input'); + await authInputs(page).first().fill(uniq() + '@example.com'); + await press(page, 'SUBMIT'); + await page.waitForTimeout(2500); + check((await heading(page)).toLowerCase().includes('check your email'), + 'an unknown address is answered the same way'); + check(sink.messages.length === 0, 'and no mail is sent for it', + sink.messages.length ? `${sink.messages.length} sent` : ''); + + await page.goto(link, { waitUntil: 'networkidle' }); + await page.waitForSelector('.auth-form input'); + check((await heading(page)).toLowerCase().includes('choose a new password'), + 'the link opens the reset form'); + check(await page.locator('.pw-slot').count() === 4, + 'the strength meter is on the reset form too'); + + // Short: refused by a rule the browser knows, so it must say so without a round trip. + let r = authInputs(page); + await r.nth(0).fill(SHORT); await r.nth(1).fill(SHORT); + await press(page, 'SUBMIT'); + await page.waitForTimeout(800); + check(/at least 12 characters/i.test(await cardText(page)), + 'a short new password says what is wrong'); + + // Common: passes every local rule, so ONLY the server can refuse it. This is the + // case that was answering 400 with an empty body and a generic apology. + r = authInputs(page); + await r.nth(0).fill(COMMON); await r.nth(1).fill(COMMON); + await press(page, 'SUBMIT'); + await page.waitForTimeout(3500); + const common = await cardText(page); + check(/too common/i.test(common), + "the server's reason for a common password reaches the page", + common.replace(/\n/g, ' | ').slice(0, 140)); + + // Mismatch, while masked. + r = authInputs(page); + await r.nth(0).fill(GOOD + ' one'); await r.nth(1).fill(GOOD + ' two'); + await press(page, 'SUBMIT'); + await page.waitForTimeout(800); + check(/do not match/i.test(await cardText(page)), 'a mismatched confirmation is caught'); + + // And the whole point: a good password goes through and then LETS YOU IN. + const fresh = GOOD + ' candle'; + r = authInputs(page); + await r.nth(0).fill(fresh); await r.nth(1).fill(fresh); + await press(page, 'SUBMIT'); + await page.waitForTimeout(3500); + check(/password/i.test(await heading(page)) === false || /changed|reset/i.test(await cardText(page)), + 'the reset completes', (await heading(page))); + + await page.goto(`${BASE}/pages/login-page`, { waitUntil: 'networkidle' }); + await page.waitForSelector('input'); + await page.locator('input').nth(0).fill(user); + await page.locator('input').nth(1).fill(fresh); + await page.locator('button.form-button').click(); + await page.waitForTimeout(3500); + check(!page.url().includes('login-page'), 'and the new password logs in', page.url().replace(BASE, '')); + + // The link is one use only. + await page.goto(link, { waitUntil: 'networkidle' }); + await page.waitForTimeout(1500); + const reused = (await cardText(page)).toLowerCase(); + check(/no longer works|already been used|expired/.test(reused), + 'a reset link cannot be used twice', reused.replace(/\n/g, ' | ').slice(0, 120)); + + // ---- the limits, none of which had ever been exercised ---- + console.log('limits'); + + // Five failures against one username inside the window, then it is turned away + // whatever is typed -- including, on the sixth, the RIGHT password. + const victim = uniq(), vEmail = victim + '@example.com'; + await signOut(ctx, page); + await registerAndVerify(page, sink, victim, vEmail, GOOD); + for (let i = 0; i < 5; i++) { + await page.goto(`${BASE}/pages/login-page`, { waitUntil: 'networkidle' }); + await page.waitForSelector('input'); + await page.locator('input').nth(0).fill(victim); + await page.locator('input').nth(1).fill('wrong one ' + i); + await page.locator('button.form-button').click(); + await page.waitForTimeout(1200); + } + // A sixth WRONG guess is turned away by the limit rather than by the lookup. + await page.goto(`${BASE}/pages/login-page`, { waitUntil: 'networkidle' }); + await page.waitForSelector('input'); + await page.locator('input').nth(0).fill(victim); + await page.locator('input').nth(1).fill('wrong one 5'); + await page.locator('button.form-button').click(); + await page.waitForTimeout(2500); + const throttled = await cardText(page); + check(page.url().includes('login-page') && /too many|attempt/i.test(throttled), + 'a sixth wrong guess at one account is turned away by the limit', + throttled.replace(/\n/g, ' | ').slice(0, 110)); + + // And the owner still gets in. Locking the account on five wrong guesses would + // hand anybody a way to lock anybody out, so the limit deliberately sits on the + // FAILURE path only -- this is the assertion that keeps it there. + await page.goto(`${BASE}/pages/login-page`, { waitUntil: 'networkidle' }); + await page.waitForSelector('input'); + await page.locator('input').nth(0).fill(victim); + await page.locator('input').nth(1).fill(GOOD); + await page.locator('button.form-button').click(); + await page.waitForTimeout(3500); + check(!page.url().includes('login-page'), + 'and the real password still gets the owner in, so nobody can lock them out', + page.url().replace(BASE, '')); + + // Three reset requests for one address inside the hour; the fourth sends no mail + // but must still answer identically, or the limit is itself an oracle. + const r2 = uniq(), r2Email = r2 + '@example.com'; + await signOut(ctx, page); + await registerAndVerify(page, sink, r2, r2Email, GOOD); + check((await heading(page)).toLowerCase().includes('registration is complete'), + 'a second account registers after a session was signed in', await heading(page)); + const sent = []; + for (let i = 0; i < 3; i++) sent.push(Boolean(await resetLinkFor(page, sink, r2Email))); + check(sent.every(Boolean), `all ${sent.length} reset requests inside the limit are sent`, + `sent = [${sent.join(', ')}]`); + sink.clear(); + await page.goto(`${BASE}/pages/send-password-reset-page`, { waitUntil: 'networkidle' }); + await page.waitForSelector('.auth-form input'); + await authInputs(page).first().fill(r2Email); + await press(page, 'SUBMIT'); + await page.waitForTimeout(2500); + check((await heading(page)).toLowerCase().includes('check your email'), + 'a throttled reset still answers the same way'); + check(sink.messages.length === 0, 'and sends no fourth mail in the hour', + sink.messages.length ? `${sink.messages.length} sent` : ''); + + // ---- the card at phone width ---- + const phone = await ctx.newPage(); + await phone.setViewportSize({ width: 390, height: 844 }); + await phone.goto(`${BASE}/pages/register-page`, { waitUntil: 'networkidle' }); + await phone.waitForTimeout(1200); + const docW = await phone.evaluate(() => document.documentElement.scrollWidth); + check(docW <= 392, 'the register card does not scroll sideways on a phone', `scrollWidth ${docW}`); + const solid = await phone.evaluate(() => { + const root = document.querySelector('.registration-content'); + let n = 0; + for (const el of root.querySelectorAll('*')) { + if (![...el.childNodes].some(x => x.nodeType === 3 && x.textContent.trim())) continue; + const st = getComputedStyle(el); + // The three deliberate ones: .notch and .lift are single-line label chips + // positioned to the pixel, and .auth-heading is display type at 1.2. + if (el.classList.contains('notch') || el.classList.contains('lift') + || el.classList.contains('auth-heading')) continue; + if (parseFloat(st.lineHeight) / parseFloat(st.fontSize) < 1.25) n++; + } + return n; + }); + check(solid === 0, 'no text on the card is set solid', solid ? `${solid} elements under 1.25` : ''); + await phone.close(); + + // ---- the notices, and who is allowed to move an account ---- + console.log('notices'); + + const nUser = uniq(), nEmail = nUser + '@example.com'; + await signOut(ctx, page); + await registerAndVerify(page, sink, nUser, nEmail, GOOD); + + // A completed reset must tell the account that it happened. + const nLink = await resetLinkFor(page, sink, nEmail); + await page.goto(nLink, { waitUntil: 'networkidle' }); + await page.waitForSelector('.auth-form input'); + sink.clear(); + const np = GOOD + ' kettle'; + let nb = authInputs(page); + await nb.nth(0).fill(np); await nb.nth(1).fill(np); + await press(page, 'SUBMIT'); + await page.waitForTimeout(3000); + let notice = await sink.waitFor(/password changed/i, 15000).catch(() => null); + check(Boolean(notice), 'a completed password reset tells the account it happened', + notice ? notice.headers.subject : 'no mail arrived'); + if (notice) { + check(notice.headers.to.includes(nEmail), 'and it goes to the account address', notice.headers.to); + check(!/key=|token=/.test(notice.raw), + 'and carries no working credential, so it does not teach people to click one'); + } + + // Moving the account to another address needs the password, not just a session. + await page.goto(`${BASE}/pages/login-page`, { waitUntil: 'networkidle' }); + await page.waitForSelector('input'); + await page.locator('input').nth(0).fill(nUser); + await page.locator('input').nth(1).fill(np); + await page.locator('button.form-button').click(); + await page.waitForTimeout(3500); + + // Driven through the form a person uses, not a hand-rolled request: the + // endpoint takes transit over PUT and getting either wrong looks like a pass. + const moveTo = uniq() + '@example.com'; + const attempt = async (pw) => { + await page.goto(`${BASE}/pages/my-account`, { waitUntil: 'networkidle' }); + await page.waitForTimeout(1500); + const change = page.locator('button', { hasText: /^Change$|Change email|Change again/ }).first(); + if (await change.count()) await change.click().catch(() => {}); + await page.waitForTimeout(500); + await page.locator('input[placeholder="New email address"]').fill(moveTo); + await page.locator('input[placeholder="Confirm new email address"]').fill(moveTo); + await page.locator('input[placeholder="Your current password"]').fill(pw); + await page.locator('button.form-button', { hasText: 'Save' }).first().click(); + await page.waitForTimeout(3000); + return page.locator('.registration-content, body').first().innerText(); + }; + + sink.clear(); + const wrongText = await attempt('not the password'); + check(/password is not right/i.test(wrongText), + 'a session alone can no longer move the account to another address', + wrongText.replace(/\n/g, ' | ').slice(0, 110)); + check(sink.messages.length === 0, 'and nothing is emailed for a refused attempt', + `${sink.messages.length} sent`); + + sink.clear(); + await attempt(np); + const toOld = await sink.waitFor(new RegExp(nEmail.replace('.', '\\.'), 'i'), 15000).catch(() => null); + check(Boolean(toOld), 'the real password moves it, and the address LOSING the account is told', + toOld ? toOld.headers.subject : 'never told'); + if (toOld) check(toOld.raw.includes(moveTo), 'and told where it is going'); + + // ---- a reset takes back the sessions that existed before it ---- + console.log('withdrawal'); + + const wUser = uniq(), wEmail = wUser + '@example.com'; + await signOut(ctx, page); + await registerAndVerify(page, sink, wUser, wEmail, GOOD); + + // Sign in and keep the token, standing in for another device still logged in. + await page.goto(`${BASE}/pages/login-page`, { waitUntil: 'networkidle' }); + await page.waitForSelector('input'); + await page.locator('input').nth(0).fill(wUser); + await page.locator('input').nth(1).fill(GOOD); + await page.locator('button.form-button').click(); + await page.waitForTimeout(3500); + const otherDevice = await page.evaluate(() => { + const c = window.cljs.core; + return c.get_in(window.re_frame.db.app_db.state, + c.PersistentVector.fromArray([c.keyword(null, 'user-data'), c.keyword(null, 'token')], true)); + }); + const useToken = (t) => page.evaluate(async ([b, tok]) => + (await fetch(b + '/user', { headers: { Authorization: `Token ${tok}` } })).status, [BASE, t]); + check(await useToken(otherDevice) === 200, 'a signed-in session works before the reset'); + + // Reset the password from somewhere else entirely. + await signOut(ctx, page); + const wLink = await resetLinkFor(page, sink, wEmail); + await page.goto(wLink, { waitUntil: 'networkidle' }); + await page.waitForSelector('.auth-form input'); + const wNew = GOOD + ' lantern'; + const wb = authInputs(page); + await wb.nth(0).fill(wNew); await wb.nth(1).fill(wNew); + await press(page, 'SUBMIT'); + await page.waitForTimeout(3500); + + check(await useToken(otherDevice) === 401, + 'and is refused afterwards, so a reset takes the account back', + `HTTP ${await useToken(otherDevice)}`); + + // And signing in again works normally -- this withdraws tokens, it does not + // limit how many places somebody may be signed in. + await page.goto(`${BASE}/pages/login-page`, { waitUntil: 'networkidle' }); + await page.waitForSelector('input'); + await page.locator('input').nth(0).fill(wUser); + await page.locator('input').nth(1).fill(wNew); + await page.locator('button.form-button').click(); + await page.waitForTimeout(3500); + const reissued = await page.evaluate(() => { + const c = window.cljs.core; + return c.get_in(window.re_frame.db.app_db.state, + c.PersistentVector.fromArray([c.keyword(null, 'user-data'), c.keyword(null, 'token')], true)); + }); + check(Boolean(reissued) && await useToken(reissued) === 200, + 'while a fresh sign-in works, on as many devices as you like'); + + // Sign out everywhere, without changing the password. + await page.goto(`${BASE}/pages/login-page`, { waitUntil: 'networkidle' }); + await page.waitForSelector('input'); + await page.locator('input').nth(0).fill(wUser); + await page.locator('input').nth(1).fill(wNew); + await page.locator('button.form-button').click(); + await page.waitForTimeout(3500); + const deviceA = await page.evaluate(() => { + const c = window.cljs.core; + return c.get_in(window.re_frame.db.app_db.state, + c.PersistentVector.fromArray([c.keyword(null, 'user-data'), c.keyword(null, 'token')], true)); + }); + const bye = await page.evaluate(async ([b, t]) => + (await fetch(b + '/user/sessions', { method: 'DELETE', headers: { Authorization: `Token ${t}` } })).status, + [BASE, deviceA]); + check(bye === 200, 'sign out everywhere answers', `HTTP ${bye}`); + check(await useToken(deviceA) === 401, + 'and the session that asked for it is signed out too', `HTTP ${await useToken(deviceA)}`); + check(await useToken(reissued) === 401, 'along with every other session'); + + // The password did NOT change: it still signs in. + await page.goto(`${BASE}/pages/login-page`, { waitUntil: 'networkidle' }); + await page.waitForSelector('input'); + await page.locator('input').nth(0).fill(wUser); + await page.locator('input').nth(1).fill(wNew); + await page.locator('button.form-button').click(); + await page.waitForTimeout(3500); + check(!page.url().includes('login-page'), + 'and the password still works, because signing out is not changing it', + page.url().replace(BASE, '')); + + check(errors.length === 0, 'no page errors anywhere in the run', errors.slice(0, 3).join(' | ')); + + await browser.close(); + await sink.stop(); + console.log(failures() ? `\n${failures()} FAILED` : '\nall passed'); + process.exit(failures() ? 1 : 0); +})(); diff --git a/scripts/e2e/run.js b/scripts/e2e/export-character-pdf.js similarity index 97% rename from scripts/e2e/run.js rename to scripts/e2e/export-character-pdf.js index a71f738e3..3b4830f21 100644 --- a/scripts/e2e/run.js +++ b/scripts/e2e/export-character-pdf.js @@ -1,6 +1,6 @@ // Browser end-to-end check of the PDF export. // -// ./scripts/e2e/run.sh +// ./scripts/e2e/run.sh export-character-pdf.js // // Builds characters through the real builder and exports each one, so the whole // path runs: pdf_spec assembling the field map in the browser, the form POST, @@ -345,6 +345,14 @@ if (require.main !== module) return; const browser = await chromium.launch({ executablePath: EXECUTABLE }); const context = await browser.newContext({ acceptDownloads: true }); + // The cookie banner and the What's New panel (since 2026-09-06) sit over the builder and take + // every click. Mark both seen before any page loads, as api-sub-loaders.js does. + await context.addInitScript(() => { + try { + localStorage.setItem('orcpub:no-cookie-banner', '1'); + localStorage.setItem('whats-new-seen', JSON.stringify('summer-patch-2026')); + } catch (e) {} + }); for (const scenario of SCENARIOS) { console.log(`\n${scenario.name}: ${scenario.classes.map(c => c.join(' ')).join(' / ')}`); diff --git a/scripts/e2e/lib.js b/scripts/e2e/lib.js new file mode 100644 index 000000000..c28f16ac5 --- /dev/null +++ b/scripts/e2e/lib.js @@ -0,0 +1,100 @@ +// Helpers the scripts/e2e suites share: a browser context with the overlays out of the way, logging in, +// watching share requests, pressing and reading the share buttons, and the homebrew the share suites seed. +const BASE = process.env.E2E_BASE || 'http://localhost:8890'; +const EXECUTABLE = process.env.E2E_CHROMIUM || undefined; + +// The homebrew language e2e-boot's E2E_HOMEBREW_CHARACTER_ID character knows. The server never has it; +// a suite puts it in the owner's local library. +const HOMEBREW = { + source: 'E2E Tongues', + language: 'E2E Cant', + library: description => + `{"E2E Tongues" {:orcpub.dnd.e5/languages {:e2e-cant {:name "E2E Cant" :key :e2e-cant :option-pack "E2E Tongues" :description "${description}"}}}}`, +}; + +// A context whose pages skip the cookie banner and What's New panel, keep what Copy link writes in +// window.__copied, and start with `library` in local storage when given. +async function newContext(browser, library) { + const ctx = await browser.newContext(); + await ctx.addInitScript(lib => { + try { + localStorage.setItem('orcpub:no-cookie-banner', '1'); + localStorage.setItem('whats-new-seen', JSON.stringify('summer-patch-2026')); + if (lib && !localStorage.getItem('plugins')) localStorage.setItem('plugins', lib); + } catch (e) {} + Object.defineProperty(navigator, 'clipboard', { + configurable: true, + value: { writeText: text => { window.__copied = text; return Promise.resolve(); } }, + }); + }, library || null); + return ctx; +} + +async function login(page, user, pass) { + await page.goto(`${BASE}/pages/login-page`, { waitUntil: 'domcontentloaded' }); + await page.waitForSelector('input'); + await page.locator('input').nth(0).fill(user); + await page.locator('input').nth(1).fill(pass); + await page.locator('button.form-button').click(); + await page.waitForTimeout(3500); +} + +// Every request to a share route this page makes, as "METHOD status". +function watchShares(page) { + const seen = []; + page.on('response', r => { + if (new URL(r.url()).pathname.includes('/shares/')) seen.push(`${r.request().method()} ${r.status()}`); + }); + return seen; +} + +async function waitForButton(page, text) { + await page.waitForFunction( + t => [...document.querySelectorAll('button')].some(b => b.textContent.includes(t) && !b.disabled), + text, { timeout: 60000 }); +} + +async function copyLink(page) { + await waitForButton(page, 'Copy link'); + await page.evaluate(() => { window.__copied = ''; }); + await page.locator('button', { hasText: 'Copy link' }).first().click(); + await page.waitForTimeout(300); + return page.evaluate(() => window.__copied || ''); +} + +// The sources in the page's shared-homebrew overlay, and the seeded language's description there. +async function sharedHomebrew(page) { + return page.evaluate(src => { + const c = window.cljs.core; + const shared = c.get(window.re_frame.db.app_db.state, c.keyword(null, 'shared-plugins')); + const path = c.PersistentVector.fromArray( + [src, c.keyword('orcpub.dnd.e5', 'languages'), c.keyword(null, 'e2e-cant'), c.keyword(null, 'description')], true); + return { + sources: shared ? c.clj__GT_js(c.vec(c.keys(shared))) : [], + description: shared ? c.get_in(shared, path) : null, + }; + }, HOMEBREW.source); +} + +// The status a signed-in page gets from character `id`'s share-token route: 200 when it is shared, 404 +// when not, 410 when its last link expired and the owner has not acted on that. +async function tokenStatus(page, id) { + return page.evaluate(async url => { + const c = window.cljs.core; + const login = c.get_in(window.re_frame.db.app_db.state, + c.PersistentVector.fromArray([c.keyword(null, 'user-data'), c.keyword(null, 'token')], true)); + return (await fetch(url, { headers: { Authorization: `Token ${login}` } })).status; + }, `${BASE}/dnd/5e/characters/${id}/share-token`); +} + +function checker() { + let failures = 0; + const check = (ok, label, detail) => { + if (!ok) failures++; + console.log(` ${ok ? 'PASS' : 'FAIL'} ${label}${detail ? ' -- ' + detail : ''}`); + }; + return { check, failures: () => failures }; +} + +module.exports = { BASE, EXECUTABLE, HOMEBREW, newContext, login, watchShares, waitForButton, copyLink, + sharedHomebrew, tokenStatus, checker }; diff --git a/scripts/e2e/lib/mail-sink.js b/scripts/e2e/lib/mail-sink.js new file mode 100644 index 000000000..cbeb32a45 --- /dev/null +++ b/scripts/e2e/lib/mail-sink.js @@ -0,0 +1,120 @@ +// A throwaway SMTP server that accepts anything and writes what it was given to +// a JSON file. Dependency-free on purpose: it is under a hundred lines of the +// protocol and adding a package to run the tests is how test setups rot. +// +// It exists because the e2e harness had no mail server, so registration, +// verification and password reset could not be driven end to end AT ALL -- the +// verification key and the reset key only ever exist inside an email. That is +// why those flows were never covered, and why a broken reset form reached a +// merge. Point the app at this and the keys become readable: +// +// EMAIL_SERVER_URL=127.0.0.1 EMAIL_SERVER_PORT=2525 +// +// Usage: +// const sink = await startMailSink({ port: 2525, file: '/tmp/mail.json' }); +// ... drive the app ... +// const msg = await sink.waitFor(/verification/i); +// sink.stop(); + +const net = require('net'); +const fs = require('fs'); + +function parseMessage(raw) { + const split = raw.indexOf('\r\n\r\n'); + const head = split === -1 ? raw : raw.slice(0, split); + const body = split === -1 ? '' : raw.slice(split + 4); + const headers = {}; + // Unfold continuation lines before splitting on the colon. + for (const line of head.replace(/\r\n[ \t]+/g, ' ').split('\r\n')) { + const i = line.indexOf(':'); + if (i > 0) headers[line.slice(0, i).toLowerCase().trim()] = line.slice(i + 1).trim(); + } + return { headers, body, raw }; +} + +function startMailSink({ port = 2525, file = null } = {}) { + const messages = []; + const waiters = []; + + const deliver = (msg) => { + messages.push(msg); + if (file) fs.writeFileSync(file, JSON.stringify(messages, null, 2)); + for (let i = waiters.length - 1; i >= 0; i--) { + if (waiters[i].match(msg)) { waiters[i].resolve(msg); waiters.splice(i, 1); } + } + }; + + const server = net.createServer((sock) => { + let buf = ''; + let inData = false; + let data = ''; + const say = (s) => sock.write(s + '\r\n'); + say('220 mail-sink ready'); + + sock.on('data', (chunk) => { + buf += chunk.toString('utf8'); + let nl; + while ((nl = buf.indexOf('\r\n')) !== -1) { + const line = buf.slice(0, nl); + buf = buf.slice(nl + 2); + + if (inData) { + // A lone dot ends the message; a leading dot on any other line is stuffed. + if (line === '.') { + inData = false; + deliver(parseMessage(data)); + data = ''; + say('250 2.0.0 Ok: queued'); + } else { + data += (line.startsWith('..') ? line.slice(1) : line) + '\r\n'; + } + continue; + } + + const verb = line.split(/\s+/)[0].toUpperCase(); + switch (verb) { + case 'EHLO': + // No STARTTLS and no AUTH advertised: the client then sends neither, + // which is what keeps this short. + say('250-mail-sink'); + say('250 8BITMIME'); + break; + case 'HELO': say('250 mail-sink'); break; + case 'MAIL': + case 'RCPT': say('250 2.1.0 Ok'); break; + case 'AUTH': say('235 2.7.0 Accepted'); break; + case 'RSET': say('250 2.0.0 Ok'); break; + case 'NOOP': say('250 2.0.0 Ok'); break; + case 'DATA': inData = true; say('354 End data with .'); break; + case 'QUIT': say('221 2.0.0 Bye'); sock.end(); break; + default: say('250 2.0.0 Ok'); break; + } + } + }); + sock.on('error', () => {}); + }); + + return new Promise((resolve, reject) => { + server.once('error', reject); + server.listen(port, '127.0.0.1', () => resolve({ + port, + messages, + clear: () => { messages.length = 0; if (file) fs.writeFileSync(file, '[]'); }, + // Resolves with the first message already held or next delivered that matches. + waitFor: (re, timeoutMs = 15000) => new Promise((res, rej) => { + const match = (m) => re.test(m.raw); + const already = messages.find(match); + if (already) return res(already); + const w = { match, resolve: res }; + waiters.push(w); + setTimeout(() => { + const i = waiters.indexOf(w); + if (i !== -1) { waiters.splice(i, 1); rej(new Error(`no mail matching ${re} in ${timeoutMs}ms`)); } + }, timeoutMs); + }), + stop: () => new Promise((res) => server.close(res)), + })); + }); +} + +module.exports = { startMailSink }; diff --git a/scripts/e2e/move-homebrew-keeps-dependents.js b/scripts/e2e/move-homebrew-keeps-dependents.js new file mode 100644 index 000000000..1a651e528 --- /dev/null +++ b/scripts/e2e/move-homebrew-keeps-dependents.js @@ -0,0 +1,98 @@ +// Needs: dev bundle (reads the app's internals, which a production bundle compiles away). +// Does moving a class into a source that already has its key take the class's subclasses with it? +// +// ./scripts/e2e/run.sh move-homebrew-keeps-dependents.js +// +// Two sources each hold a class keyed :e2e-artificer, each with a subclass of its own. Moving Alpha's +// class into Beta through My Content's Move / copy bar has to rename it, since Beta already holds that +// key. Alpha's subclass must follow the renamed class, Beta's must stay on Beta's class, and the moved +// class must remember its old key. Before the fix (Greptile, PR #34) the rename left Alpha's subclass on +// :e2e-artificer, where it attached to Beta's class. The rules themselves are in orcbrew_validation_test. +// Needs no account: My Content is the browser's local library. +const { chromium } = require('playwright'); +const { BASE, EXECUTABLE, newContext, checker } = require('./lib'); + +const ALPHA = 'E2E Alpha'; +const BETA = 'E2E Beta'; +const LIBRARY = + `{"${ALPHA}" {:orcpub.dnd.e5/classes {:e2e-artificer {:name "E2E Artificer" :key :e2e-artificer :option-pack "${ALPHA}"}}` + + ` :orcpub.dnd.e5/subclasses {:e2e-alchemist {:name "E2E Alchemist" :key :e2e-alchemist :class :e2e-artificer :option-pack "${ALPHA}"}}}` + + ` "${BETA}" {:orcpub.dnd.e5/classes {:e2e-artificer {:name "E2E Artificer" :key :e2e-artificer :option-pack "${BETA}"}}` + + ` :orcpub.dnd.e5/subclasses {:e2e-armorer {:name "E2E Armorer" :key :e2e-armorer :class :e2e-artificer :option-pack "${BETA}"}}}}`; + +// Every class and subclass in the page's library, as {source, type, key, name, class, formerKeys}. +const library = page => page.evaluate(() => { + const c = window.cljs.core; + const kw = (ns, n) => c.keyword(ns, n); + const plugins = c.get(window.re_frame.db.app_db.state, kw(null, 'plugins')); + const rows = []; + c.run_BANG_(entry => { + const source = c.key(entry), plugin = c.val(entry); + for (const type of ['classes', 'subclasses']) { + c.run_BANG_(e => { const k = c.key(e), item = c.val(e); rows.push({ + source, type, key: c.name(k), + name: c.get(item, kw(null, 'name')), + class: (cls => cls ? c.name(cls) : null)(c.get(item, kw(null, 'class'))), + formerKeys: c.clj__GT_js(c.map(c.name, c.get(item, kw(null, 'former-keys')))), + }); }, c.get(plugin, kw('orcpub.dnd.e5', type))); + } + }, plugins); + return rows; +}); + +(async () => { + const { check, failures } = checker(); + const browser = await chromium.launch({ executablePath: EXECUTABLE }); + const ctx = await newContext(browser, LIBRARY); + const page = await ctx.newPage(); + const errors = []; + page.on('pageerror', e => errors.push(String(e))); + + await page.goto(`${BASE}/dnd/5e/my-content`, { waitUntil: 'networkidle', timeout: 120000 }); + await page.waitForTimeout(2000); + const before = await library(page); + check(before.filter(r => r.type === 'classes').length === 2, 'both sources load their class', JSON.stringify(before)); + + await page.locator('button.b-move').click(); + // Sources and the content types inside them render collapsed, each with its own "expand" link. + const row = page.locator('.mc-selrow', { hasText: 'E2E Artificer' }).first(); + for (let i = 0; i < 10 && !(await row.isVisible().catch(() => false)); i++) { + const expand = page.locator('span.underline', { hasText: /^expand$/ }).first(); + if (!(await expand.isVisible().catch(() => false))) break; + await expand.click(); + await page.waitForTimeout(300); + } + await page.locator('.mc-selrow', { hasText: 'E2E Artificer' }).first().click(); + await page.locator('select').filter({ has: page.locator(`option[value="${BETA}"]`) }).first().selectOption(BETA); + await page.locator('button', { hasText: 'Move here' }).click(); + await page.waitForTimeout(1500); + + const after = await library(page); + const classes = after.filter(r => r.type === 'classes'); + const moved = classes.find(r => r.source === BETA && r.key !== 'e2e-artificer'); + const betaOwn = classes.find(r => r.source === BETA && r.key === 'e2e-artificer'); + const alchemist = after.find(r => r.key === 'e2e-alchemist'); + const armorer = after.find(r => r.key === 'e2e-armorer'); + + check(!classes.some(r => r.source === ALPHA), 'the class left Alpha', JSON.stringify(classes)); + check(!!moved, 'it arrived in Beta under a new key', JSON.stringify(classes)); + check(!!betaOwn && betaOwn.name === 'E2E Artificer', "Beta's own class is untouched", JSON.stringify(betaOwn)); + check(!!moved && alchemist && alchemist.class === moved.key, + "Alpha's subclass follows the moved class", JSON.stringify({ alchemist, moved: moved && moved.key })); + check(!!armorer && armorer.class === 'e2e-artificer', "Beta's subclass stays on Beta's class", JSON.stringify(armorer)); + check(!!moved && (moved.formerKeys || []).includes('e2e-artificer'), + 'the moved class remembers its old key', JSON.stringify(moved)); + + // The move persists like any edit: a reload reads it back from local storage. + await page.reload({ waitUntil: 'networkidle' }); + await page.waitForTimeout(2000); + const reloaded = await library(page); + const alchemistAgain = reloaded.find(r => r.key === 'e2e-alchemist'); + check(!!moved && alchemistAgain && alchemistAgain.class === moved.key, 'and it survives a reload', + JSON.stringify(alchemistAgain)); + check(errors.length === 0, 'no uncaught page errors', errors.join(' | ')); + + await browser.close(); + console.log(failures() ? `\nFAILURES: ${failures()}` : '\nall checks passed'); + process.exit(failures() ? 1 : 0); +})(); diff --git a/scripts/e2e/package-lock.json b/scripts/e2e/package-lock.json new file mode 100644 index 000000000..5cb5da3f7 --- /dev/null +++ b/scripts/e2e/package-lock.json @@ -0,0 +1,40 @@ +{ + "name": "orcpub-e2e", + "lockfileVersion": 3, + "requires": true, + "packages": { + "": { + "name": "orcpub-e2e", + "dependencies": { + "playwright": "^1.62.0" + } + }, + "node_modules/playwright": { + "version": "1.63.0", + "resolved": "https://registry.npmjs.org/playwright/-/playwright-1.63.0.tgz", + "integrity": "sha512-+7ziBLidS4NaNCdt57SUDT+wYmmd5fmiQejUic/kb+YsYSCPyOOE9sebzMjNmQrsnNpDJqd4WHvV/8lfKfUDUg==", + "license": "Apache-2.0", + "dependencies": { + "playwright-core": "1.63.0" + }, + "bin": { + "playwright": "cli.js" + }, + "engines": { + "node": ">=20" + } + }, + "node_modules/playwright-core": { + "version": "1.63.0", + "resolved": "https://registry.npmjs.org/playwright-core/-/playwright-core-1.63.0.tgz", + "integrity": "sha512-rYCsBF/M5HjUch52bbtVONEFjv6Xu8sm8h72dNlR5bzIE1fvC/bxgspzkjSfU+MweEMmPM8KJebG6nnyxo5mCg==", + "license": "Apache-2.0", + "bin": { + "playwright-core": "cli.js" + }, + "engines": { + "node": ">=20" + } + } + } +} diff --git a/scripts/e2e/party-keeps-shared-homebrew.js b/scripts/e2e/party-keeps-shared-homebrew.js new file mode 100644 index 000000000..462c9f2b0 --- /dev/null +++ b/scripts/e2e/party-keeps-shared-homebrew.js @@ -0,0 +1,92 @@ +// Needs: dev bundle (reads the app's internals, which a production bundle compiles away). +// Does a character added to a party from a share link keep showing its homebrew on the party page? +// +// ./scripts/e2e/run.sh party-keeps-shared-homebrew.js +// +// kaylee shares the seeded character that knows a homebrew language (E2E_HOMEBREW_CHARACTER_ID). zoe, +// signed in and without that homebrew, opens the link and presses Add to Party, which makes a new party. +// On zoe's parties page the party must list the share token for that character, and opening its row must +// load the homebrew. After kaylee presses New link, zoe's party must list no token and opening the row +// must load nothing. That only characters can join a party is covered by party_test.clj. +const { chromium } = require('playwright'); +const { BASE, EXECUTABLE, HOMEBREW, newContext, login, waitForButton, copyLink, sharedHomebrew, + checker } = require('./lib'); + +const ID = process.env.E2E_HOMEBREW_CHARACTER_ID; +const NAME = 'Wren Holloway'; + +// The share token zoe's parties list for the character, from the parties the page loaded. +const listedToken = page => page.evaluate(id => { + const c = window.cljs.core; + const parties = c.get(window.re_frame.db.app_db.state, c.keyword('orcpub.dnd.e5.character', 'parties')); + const tokens = []; + c.run_BANG_(party => c.run_BANG_(ch => { + if (c.get(ch, c.keyword(null, 'db/id')) === parseInt(id) || c.get(ch, c.keyword('db', 'id')) === parseInt(id)) { + tokens.push(c.get(ch, c.keyword('orcpub.party-share', 'token')) || null); + } + }, c.get(party, c.keyword('orcpub.dnd.e5.party', 'character-ids'))), parties || c.PersistentVector.EMPTY); + return tokens; +}, ID); + +async function openPartyRow(page) { + await page.goto(`${BASE}/pages/dnd/5e/parties`, { waitUntil: 'networkidle', timeout: 120000 }); + await page.waitForTimeout(2000); + const tokens = await listedToken(page); + await page.locator(`text=${NAME}`).first().click(); + await page.waitForTimeout(4000); + return { tokens, ...(await sharedHomebrew(page)) }; +} + +(async () => { + if (!ID) { + console.error('No E2E_HOMEBREW_CHARACTER_ID: run this through scripts/e2e/run.sh, which reads it from e2e-boot.'); + process.exit(2); + } + const { check, failures } = checker(); + const browser = await chromium.launch({ executablePath: EXECUTABLE }); + + console.log('kaylee shares her character:'); + const kaylee = await newContext(browser, HOMEBREW.library('Spoken only by test suites.')); + const owner = await kaylee.newPage(); + owner.on('dialog', d => d.accept()); + await login(owner, 'kaylee', 'serenity99'); + await owner.goto(`${BASE}/pages/dnd/5e/characters/${ID}`, { waitUntil: 'networkidle', timeout: 120000 }); + await waitForButton(owner, 'Share link'); + await owner.locator('button', { hasText: 'Share link' }).first().click(); + const link = await copyLink(owner); + check(/#s=[A-Za-z0-9_-]{22}$/.test(link), 'a short link', link); + + console.log('\nzoe opens it and adds the character to a party:'); + const zoe = await newContext(browser, null); + const page = await zoe.newPage(); + const errors = []; + page.on('pageerror', e => errors.push(e.message)); + await login(page, 'zoe', 'washburne7'); + await page.goto(link, { waitUntil: 'networkidle', timeout: 120000 }); + await page.waitForTimeout(4000); + check((await sharedHomebrew(page)).sources.includes(HOMEBREW.source), 'the link loaded the homebrew'); + const add = page.locator('button.form-button', { hasText: /^ADD$/ }); + check(await add.count() > 0, 'the page offers Add to Party'); + await add.first().click(); + await page.waitForTimeout(2500); + + console.log('\nzoe opens the party page:'); + const kept = await openPartyRow(page); + check(kept.tokens.length === 1 && kept.tokens[0] === link.split('#s=')[1], 'the party keeps the link\'s token', JSON.stringify(kept.tokens)); + check(kept.sources.includes(HOMEBREW.source), 'opening the row loads the homebrew', JSON.stringify(kept.sources)); + + console.log('\nkaylee presses New link, and zoe opens the party page again:'); + await copyLink(owner); + await owner.locator('button', { hasText: 'New link' }).first().click(); + await owner.waitForTimeout(3000); + const revoked = await openPartyRow(page); + check(revoked.tokens.length === 1 && revoked.tokens[0] === null, 'the party lists no token', JSON.stringify(revoked.tokens)); + check(revoked.sources.length === 0, 'opening the row loads nothing', JSON.stringify(revoked.sources)); + check(errors.length === 0, 'no uncaught page errors on zoe\'s pages', errors.join(' | ')); + + await kaylee.close(); + await zoe.close(); + await browser.close(); + console.log(failures() ? `\nFAILURES: ${failures()}` : '\nall checks passed'); + process.exit(failures() ? 1 : 0); +})(); diff --git a/scripts/e2e/run-all.sh b/scripts/e2e/run-all.sh index bd5b36d0a..42bac838f 100755 --- a/scripts/e2e/run-all.sh +++ b/scripts/e2e/run-all.sh @@ -35,6 +35,11 @@ done # 0 or a negative count would leave the scheduler waiting for a job slot no job will ever free. [[ "$JOBS" =~ ^[1-9][0-9]*$ ]] || { echo "run-all.sh: --jobs needs a whole number of 1 or more, got '$JOBS'"; exit 2; } +# One run at a time: a second one shares the first's server and database, and both report results +# that are neither's. The lock is released when this shell exits, however it exits. +exec 9>"/tmp/e2e-run-all-${E2E_PORT:-8890}.lock" +flock -n 9 || { echo "run-all.sh: another run is already using :${E2E_PORT:-8890}; wait for it or stop it"; exit 2; } + LOGDIR="$(mktemp -d /tmp/e2e-all.XXXX)" RESULTS="$LOGDIR/results" : > "$RESULTS" @@ -86,7 +91,7 @@ fail_all() { # ... # Peak memory: parallel tests are bounded by it, and a run that gets close slows or fails for # reasons that are not the app. -( while :; do free -m | awk '/^Mem:/ {print $3, $2}'; sleep 2; done ) > "$LOGDIR/memory" & +( while :; do free -m | awk '/^Mem:/ {print $3, $2}'; sleep 2; done ) > "$LOGDIR/memory" 9>&- & MEMPID=$! SHARED_UP=0 # One exit path for an interrupted run: the sampler and a shared server both go. diff --git a/scripts/e2e/run.sh b/scripts/e2e/run.sh index 2f1a50636..80832e4ed 100755 --- a/scripts/e2e/run.sh +++ b/scripts/e2e/run.sh @@ -4,7 +4,7 @@ # ./scripts/e2e/run.sh --describe prints: # ./scripts/e2e/run.sh --build-only builds the bundle and stylesheet the suite needs, and # stops; run-all.sh does this while a server boots -# ./scripts/e2e/run.sh [suite] [args...] a file in scripts/e2e/ (default run.js), or a path +# ./scripts/e2e/run.sh [args...] a file in scripts/e2e/, or a path # such as test/browser/boot_rescue_e2e.js; any args # are passed to the suite # @@ -40,7 +40,15 @@ SHEET=resources/public/css/compiled/styles.css DESCRIBE=""; BUILD_ONLY="" [ "${1:-}" = --describe ] && { DESCRIBE=1; shift; } [ "${1:-}" = --build-only ] && { BUILD_ONLY=1; shift; } -SUITE_ARG="${1:-run.js}" +# No default suite: with none named, list them. Checked before anything builds or boots. +if [ $# -eq 0 ]; then + echo "usage: ./scripts/e2e/run.sh [args...]"; echo "suites:" + for f in scripts/e2e/*.js test/browser/*_e2e.js; do + case "$f" in */lib.js|*/boot-check.js) ;; *) echo " $f" ;; esac + done + exit 2 +fi +SUITE_ARG="$1" [ $# -gt 0 ] && shift SUITE_ARGS=("$@") case "$SUITE_ARG" in */*) SUITE="$SUITE_ARG" ;; *) SUITE="scripts/e2e/$SUITE_ARG" ;; esac @@ -168,6 +176,14 @@ else trap './scripts/e2e/server.sh stop' EXIT fi +# The seeded characters some suites open, from the server's log (dev/e2e_boot.clj prints them). +SERVER_LOG="${E2E_LOG:-/tmp/e2e-server-${PORT}.log}" +seeded() { grep -o "$1 200 [0-9]*" "$SERVER_LOG" 2>/dev/null | awk '{print $3}' | tail -1; } +export E2E_CHARACTER_ID="$(seeded E2E-CHARACTER)" +export E2E_HOMEBREW_CHARACTER_ID="$(seeded E2E-HOMEBREW-CHARACTER)" +export E2E_EXPIRED_CHARACTER_ID="$(seeded E2E-EXPIRED-CHARACTER)" +export E2E_MAIL_PORT="${E2E_MAIL_PORT:-2525}" + # --- the app must start before anything is judged ------------------------------------------ node scripts/e2e/boot-check.js "$BASE" || fail "the app did not start in a browser (reason above)" diff --git a/scripts/e2e/server.sh b/scripts/e2e/server.sh index 7c06fc84b..31e260284 100755 --- a/scripts/e2e/server.sh +++ b/scripts/e2e/server.sh @@ -11,6 +11,8 @@ # dev/e2e_boot.clj starts the server AND seeds the user in one process. set -uo pipefail cd "$(dirname "$0")/../.." +# run-all.sh's lock is fd 9; the JVM below outlives this script and must not keep it held. +exec 9>&- PORT="${E2E_PORT:-8890}" LOG="${E2E_LOG:-/tmp/e2e-server-${PORT}.log}" @@ -38,6 +40,11 @@ case "${1:-}" in exit 1 fi echo "Starting server on :${PORT} (profiles: $PROFILES)..." + # Mail goes to scripts/e2e/lib/mail-sink.js, which auth-flows.js runs on E2E_MAIL_PORT: without + # a mail server registration fails outright, and the verify and reset keys exist only in mail. + EMAIL_SERVER_URL=127.0.0.1 \ + EMAIL_SERVER_PORT="${E2E_MAIL_PORT:-2525}" \ + EMAIL_FROM_ADDRESS="${EMAIL_FROM_ADDRESS:-e2e@orcpub.invalid}" \ DATOMIC_URL="datomic:mem://orcpub-e2e" \ ORCPUB_ENV=dev \ SIGNATURE="${SIGNATURE:-e2e-test-signature}" \ @@ -51,6 +58,9 @@ case "${1:-}" in if ! curl -sf -o /dev/null "$BASE/"; then tail -30 "$LOG"; stop; echo "server.sh: the server never came up (log above)"; exit 1 fi + # It answers before e2e-boot has finished seeding the users and characters. + for _ in $(seq 1 60); do grep -q E2E-READY "$LOG" && break; sleep 1; done + grep -q E2E-READY "$LOG" || { stop; echo "server.sh: the server never finished seeding (no E2E-READY in $LOG)"; exit 1; } if grep -q BindException "$LOG" 2>/dev/null; then stop; echo "server.sh: port ${PORT} was already in use, so this would have tested a stale server"; exit 1 fi diff --git a/scripts/e2e/share-from-character-list.js b/scripts/e2e/share-from-character-list.js new file mode 100644 index 000000000..c5244dc55 --- /dev/null +++ b/scripts/e2e/share-from-character-list.js @@ -0,0 +1,68 @@ +// Needs: dev bundle (reads the app's internals, which a production bundle compiles away). +// Needs: fresh server (shares and unshares the seeded homebrew character another share suite expects unshared). +// Can a character be shared from the character list, where there is no room for the share line? +// +// ./scripts/e2e/run.sh share-from-character-list.js +// +// kaylee's homebrew character (E2E_HOMEBREW_CHARACTER_ID) has never been shared. Its row in the character list +// has one Copy link button: a single press must make the share, send the homebrew to the server, copy a short +// link and say the character is now shared. Pressing it again copies the same link, and the character page +// then shows the character as shared, with that link. +const { chromium } = require('playwright'); +const { BASE, EXECUTABLE, HOMEBREW, newContext, login, watchShares, waitForButton, copyLink, tokenStatus, + checker } = require('./lib'); + +const ID = process.env.E2E_HOMEBREW_CHARACTER_ID; + +(async () => { + if (!ID) { + console.error('No E2E_HOMEBREW_CHARACTER_ID: run this through scripts/e2e/run.sh, which reads it from e2e-boot.'); + process.exit(2); + } + const { check, failures } = checker(); + const browser = await chromium.launch({ executablePath: EXECUTABLE }); + const ctx = await newContext(browser, HOMEBREW.library('Spoken only by test suites.')); + const page = await ctx.newPage(); + const errors = []; + page.on('pageerror', e => errors.push(e.message)); + const requests = watchShares(page); + await login(page, 'kaylee', 'serenity99'); + + console.log('kaylee opens the character list:'); + await page.goto(`${BASE}/pages/dnd/5e/characters`, { waitUntil: 'networkidle', timeout: 120000 }); + check(await tokenStatus(page, ID) === 404, 'the character starts unshared'); + await page.evaluate(id => { + const c = window.cljs.core; + window.re_frame.core.dispatch(c.PersistentVector.fromArray( + [c.keyword(null, 'toggle-character-expanded'), parseInt(id)], true)); + }, ID); + // The message is a toast that closes itself, so watch for it from before the press. + const noticed = page.waitForFunction(() => document.body && document.body.innerText.includes('now shared'), null, + { timeout: 30000 }).then(() => true, () => false); + await waitForButton(page, 'Copy link'); + await page.evaluate(() => { window.__copied = ''; }); + await page.locator('button', { hasText: 'Copy link' }).first().click(); + await page.waitForFunction(() => window.__copied, null, { timeout: 30000 }).catch(() => {}); + const link = await page.evaluate(() => window.__copied || ''); + check(/#s=[A-Za-z0-9_-]{22}$/.test(link), 'one press makes a short link and copies it', link); + check(requests.includes('PUT 200'), 'the homebrew went to the server', requests.join(', ')); + check(await noticed, 'the page says the character is now shared'); + check(await tokenStatus(page, ID) === 200, 'the server has the share'); + + console.log('\nshe presses it again:'); + await page.waitForTimeout(2500); + const again = await copyLink(page); + check(again === link, 'the same link is copied', again); + + console.log('\nshe opens the character page:'); + await page.goto(`${BASE}/pages/dnd/5e/characters/${ID}`, { waitUntil: 'networkidle', timeout: 120000 }); + await waitForButton(page, 'Stop sharing').catch(() => {}); + check(await page.evaluate(() => [...document.querySelectorAll('.share-pill')].some(p => p.textContent.trim() === 'Shared')), + 'the page shows it as shared'); + check(await copyLink(page) === link, 'with the same link'); + check(errors.length === 0, 'no uncaught page errors', errors.join(' | ')); + + await browser.close(); + console.log(failures() ? `\nFAILURES: ${failures()}` : '\nall checks passed'); + process.exit(failures() ? 1 : 0); +})(); diff --git a/scripts/e2e/share-link-carries-homebrew.js b/scripts/e2e/share-link-carries-homebrew.js new file mode 100644 index 000000000..930fc68d5 --- /dev/null +++ b/scripts/e2e/share-link-carries-homebrew.js @@ -0,0 +1,150 @@ +// Needs: dev bundle (reads the app's internals, which a production bundle compiles away). +// Needs: fresh server (shares and unshares the seeded homebrew character another share suite expects unshared). +// Does a short share link carry a character's homebrew to someone who does not have it, and keep +// carrying the current homebrew? +// +// ./scripts/e2e/run.sh share-link-carries-homebrew.js +// +// e2e-boot seeds a character for kaylee that knows a homebrew language, and run.sh passes its id on as +// E2E_HOMEBREW_CHARACTER_ID. kaylee's browser gets that language in its local library, where an import +// would leave it. Signed in, her character page must store nothing until she presses Share link, which +// sends the homebrew to the server and makes a link that carries only a token; the character list must +// then give the same link. The link is opened logged out and as zoe, neither of whom has the homebrew, +// and both must load it; a wrong token must load nothing and say so. Then kaylee changes the language's +// description and opens her character again: the link must stay the same and show the new description. +// Then she presses New link: the old link loads nothing and the new one works. Last, she presses Stop +// sharing: that link loads nothing too, and opening her character again stores nothing. +const { chromium } = require('playwright'); +const { BASE, EXECUTABLE, HOMEBREW, newContext, login, watchShares, waitForButton, copyLink, + sharedHomebrew, tokenStatus, checker } = require('./lib'); + +const ID = process.env.E2E_HOMEBREW_CHARACTER_ID; +const FIRST = 'Spoken only by test suites.'; +const SECOND = 'Spoken only by test suites, and now with verbs.'; + +// kaylee, signed in, on her character's page, with the language described as `description`. +async function ownerSession(browser, description) { + const ctx = await newContext(browser, HOMEBREW.library(description)); + const page = await ctx.newPage(); + page.on('dialog', d => d.accept()); + const requests = watchShares(page); + await login(page, 'kaylee', 'serenity99'); + await page.goto(`${BASE}/pages/dnd/5e/characters/${ID}`, { waitUntil: 'networkidle', timeout: 120000 }); + return { ctx, page, requests }; +} + +async function open(browser, link, who) { + const ctx = await newContext(browser, null); + const page = await ctx.newPage(); + const errors = []; + page.on('pageerror', e => errors.push(e.message)); + const requests = watchShares(page); + if (who) await login(page, who.user, who.pass); + await page.goto(link, { waitUntil: 'domcontentloaded', timeout: 120000 }); + // A failure notice is a toast that closes itself, so watch for it from the start. + const noticed = page.waitForFunction(() => document.body && document.body.innerText.includes('could not be loaded'), + null, { timeout: 15000 }).then(() => 1, () => 0); + await page.waitForLoadState('networkidle', { timeout: 120000 }); + await page.waitForTimeout(5000); + const loaded = await sharedHomebrew(page); + const banner = await page.locator('text=Shared with 1 homebrew piece').count(); + const failedNotice = await noticed; + await ctx.close(); + return { ...loaded, banner, failedNotice, requests, errors }; +} + +(async () => { + if (!ID) { + console.error('No E2E_HOMEBREW_CHARACTER_ID: run this through scripts/e2e/run.sh, which reads it from e2e-boot.'); + process.exit(2); + } + const { check, failures } = checker(); + const browser = await chromium.launch({ executablePath: EXECUTABLE }); + + console.log('kaylee makes the link:'); + const first = await ownerSession(browser, FIRST); + const beforeSharing = await tokenStatus(first.page, ID); + check(beforeSharing === 404 && first.requests.length === 0, 'nothing is stored before Share link is pressed', + `token route ${beforeSharing}; ${first.requests.join(', ') || 'no uploads'}`); + await waitForButton(first.page, 'Share link'); + await first.page.locator('button', { hasText: 'Share link' }).first().click(); + const link = await copyLink(first.page); + const match = link.match(/#s=([A-Za-z0-9_-]{22})$/); + check(Boolean(match), 'the link carries a share token', link); + check(link.length < 120, 'the link is short', `${link.length} characters`); + check(first.requests.includes('PUT 200'), 'the homebrew went to the server', first.requests.join(', ')); + + // The character list's Copy link is the same button, which once packed whatever character was open in + // the builder; in this fresh browser that is none, so its link must match the character page's. + console.log('\nkaylee copies it from the character list:'); + await first.page.goto(`${BASE}/pages/dnd/5e/characters`, { waitUntil: 'networkidle', timeout: 120000 }); + await first.page.evaluate(id => { + const c = window.cljs.core; + window.re_frame.core.dispatch(c.PersistentVector.fromArray( + [c.keyword(null, 'toggle-character-expanded'), parseInt(id)], true)); + }, ID); + let listLink; + try { listLink = await copyLink(first.page); } catch (e) { listLink = `(no Copy link in the row: ${e.message.split('\n')[0]})`; } + check(Boolean(match) && listLink === link, "the list row's link is the character page's link", listLink); + await first.ctx.close(); + + if (match) { + for (const who of [null, { user: 'zoe', pass: 'washburne7' }]) { + console.log(`\n${who ? who.user : 'logged out'} opens it:`); + const r = await open(browser, link, who); + check(r.sources.includes(HOMEBREW.source), 'the homebrew loaded', JSON.stringify(r.sources)); + check(r.description === FIRST, 'with its description', r.description); + check(r.banner > 0, 'the page says what was shared'); + check(!r.requests.some(x => x.startsWith('PUT')), 'nothing was uploaded', r.requests.join(', ')); + check(r.errors.length === 0, 'no uncaught page errors', r.errors.join(' | ')); + } + + console.log('\nthe same link with a wrong token:'); + const wrong = await open(browser, link.replace(/#s=[A-Za-z0-9_-]{22}$/, '#s=' + 'A'.repeat(22)), null); + check(wrong.sources.length === 0, 'loads nothing', JSON.stringify(wrong.sources)); + check(wrong.failedNotice > 0, 'says the homebrew could not be loaded'); + + console.log('\nkaylee changes the description and opens her character again:'); + const second = await ownerSession(browser, SECOND); + check(await copyLink(second.page) === link, 'the link stays the same'); + check(second.requests.includes('PUT 200'), 'the changed homebrew went to the server', second.requests.join(', ')); + await second.ctx.close(); + const updated = await open(browser, link, null); + check(updated.description === SECOND, 'the same link shows the new description', updated.description); + + console.log('\nkaylee makes a new link:'); + const third = await ownerSession(browser, SECOND); + await copyLink(third.page); + await third.page.locator('button', { hasText: 'New link' }).first().click(); + let fresh = link; + for (let i = 0; i < 20 && fresh === link; i++) { await third.page.waitForTimeout(1000); fresh = await copyLink(third.page); } + check(/#s=[A-Za-z0-9_-]{22}$/.test(fresh) && fresh !== link, 'New link gives a different link', fresh); + await third.ctx.close(); + const old = await open(browser, link, null); + check(old.sources.length === 0 && old.failedNotice > 0, 'the old link loads nothing, and says so', JSON.stringify(old.sources)); + const renewed = await open(browser, fresh, null); + check(renewed.description === SECOND, 'the new link loads the homebrew', renewed.description); + + console.log('\nkaylee stops sharing:'); + const fourth = await ownerSession(browser, SECOND); + await waitForButton(fourth.page, 'Stop sharing'); + await fourth.page.locator('button', { hasText: 'Stop sharing' }).first().click(); + await waitForButton(fourth.page, 'Share link'); + check(await tokenStatus(fourth.page, ID) === 404, 'the page offers Share link again, and the server keeps no share'); + await fourth.ctx.close(); + const stopped = await open(browser, fresh, null); + check(stopped.sources.length === 0 && stopped.failedNotice > 0, 'the last link loads nothing, and says so', + JSON.stringify(stopped.sources)); + const fifth = await ownerSession(browser, SECOND); + await waitForButton(fifth.page, 'Share link'); + await fifth.page.waitForTimeout(2000); + const afterStopping = await tokenStatus(fifth.page, ID); + check(!fifth.requests.some(x => x.startsWith('PUT')) && afterStopping === 404, + 'opening the character again stores nothing', `token route ${afterStopping}; ${fifth.requests.join(', ') || 'no uploads'}`); + await fifth.ctx.close(); + } + + await browser.close(); + console.log(failures() ? `\nFAILURES: ${failures()}` : '\nall checks passed'); + process.exit(failures() ? 1 : 0); +})(); diff --git a/scripts/e2e/share-link-expired-note.js b/scripts/e2e/share-link-expired-note.js new file mode 100644 index 000000000..a3ce0710d --- /dev/null +++ b/scripts/e2e/share-link-expired-note.js @@ -0,0 +1,84 @@ +// Needs: dev bundle (reads the app's internals, which a production bundle compiles away). +// Does the owner's page say that a share link expired, and keep saying so until she acts on it? +// +// ./scripts/e2e/run.sh share-link-expired-note.js +// +// e2e-boot seeds a character for kaylee that knows the homebrew language and whose last share link expired, +// as the pruner leaves it: a note of the character and the date. run.sh passes its id on as +// E2E_EXPIRED_CHARACTER_ID. The share line's status must say the link expired, with Share link beside it, on +// her character page and again after a reload. The character list row has only its Copy link button, so +// the status stays on the page. Dismiss must remove it for good without storing a share. +const { chromium } = require('playwright'); +const { BASE, EXECUTABLE, HOMEBREW, newContext, login, watchShares, waitForButton, tokenStatus, + checker } = require('./lib'); + +const ID = process.env.E2E_EXPIRED_CHARACTER_ID; +const NOTE = 'Link expired'; + +// The status text once it shows, or null when it does not within 30 seconds. innerText is what is on +// screen, so a hidden status does not count; the comparison ignores case in case a container sets +// capitals. +async function note(page) { + const shown = await page.waitForFunction(t => document.body && document.body.innerText.toLowerCase().includes(t), + NOTE.toLowerCase(), { timeout: 30000 }).then(() => true, () => false); + if (!shown) return null; + return page.evaluate(t => [...document.querySelectorAll('span')].map(s => s.textContent).find(x => x.includes(t)), NOTE); +} + +const noteShowing = page => page.evaluate(t => document.body.innerText.toLowerCase().includes(t), NOTE.toLowerCase()); + +(async () => { + if (!ID) { + console.error('No E2E_EXPIRED_CHARACTER_ID: run this through scripts/e2e/run.sh, which reads it from e2e-boot.'); + process.exit(2); + } + const { check, failures } = checker(); + const browser = await chromium.launch({ executablePath: EXECUTABLE }); + const ctx = await newContext(browser, HOMEBREW.library('Spoken only by test suites.')); + const page = await ctx.newPage(); + const errors = []; + page.on('pageerror', e => errors.push(e.message)); + const requests = watchShares(page); + await login(page, 'kaylee', 'serenity99'); + const characterPage = `${BASE}/pages/dnd/5e/characters/${ID}`; + + console.log('kaylee opens the character:'); + await page.goto(characterPage, { waitUntil: 'networkidle', timeout: 120000 }); + const text = await note(page); + check(Boolean(text), 'the page says the link expired', text); + check(/\b20\d\d\b/.test(text || ''), 'with the date', text); + await waitForButton(page, 'Share link'); + check(await tokenStatus(page, ID) === 410, 'Share link is offered beside it, and the server still has the note'); + + console.log('\nshe reloads and does nothing:'); + await page.reload({ waitUntil: 'networkidle', timeout: 120000 }); + check(Boolean(await note(page)), 'the note is still there'); + + console.log('\nshe opens the character list:'); + await page.goto(`${BASE}/pages/dnd/5e/characters`, { waitUntil: 'networkidle', timeout: 120000 }); + await page.evaluate(id => { + const c = window.cljs.core; + window.re_frame.core.dispatch(c.PersistentVector.fromArray( + [c.keyword(null, 'toggle-character-expanded'), parseInt(id)], true)); + }, ID); + await waitForButton(page, 'Copy link'); + check(!(await noteShowing(page)), "the character's row has its Copy link button and leaves the status to the page"); + + console.log('\nshe dismisses it:'); + await page.goto(characterPage, { waitUntil: 'networkidle', timeout: 120000 }); + await waitForButton(page, 'Dismiss'); + await page.locator('button', { hasText: 'Dismiss' }).first().click(); + await page.waitForFunction(t => !document.body.innerText.includes(t), NOTE, { timeout: 15000 }).catch(() => {}); + check(!(await noteShowing(page)), 'the note goes'); + await page.reload({ waitUntil: 'networkidle', timeout: 120000 }); + await waitForButton(page, 'Share link'); + await page.waitForTimeout(2000); + check(!(await noteShowing(page)), 'and stays gone after a reload'); + check(await tokenStatus(page, ID) === 404, 'the server keeps no share and no note'); + check(!requests.some(x => x.startsWith('PUT')), 'nothing was stored', requests.join(', ') || 'no uploads'); + check(errors.length === 0, 'no uncaught page errors', errors.join(' | ')); + + await browser.close(); + console.log(failures() ? `\nFAILURES: ${failures()}` : '\nall checks passed'); + process.exit(failures() ? 1 : 0); +})(); diff --git a/scripts/e2e/shared-character-shows-its-items.js b/scripts/e2e/shared-character-shows-its-items.js new file mode 100644 index 000000000..e7af3e9ff --- /dev/null +++ b/scripts/e2e/shared-character-shows-its-items.js @@ -0,0 +1,108 @@ +// Needs: dev bundle (reads the app's internals, which a production bundle compiles away). +// Does a character that uses its owner's custom items show them to someone else? +// +// ./scripts/e2e/run.sh shared-character-shows-its-items.js +// +// e2e-boot seeds a fighter owned by kaylee with one of her custom items equipped and prints its +// id; run.sh passes that on as E2E_CHARACTER_ID. The page is opened three times: logged out, as +// zoe, and as kaylee. The server sends the equipped item with the character, so each viewer should +// see it in the Equipment tab's Other Magic Items table. For logged-out and zoe it comes from the +// copy kept for that character; kaylee's comes from her own list, and no copy is kept for her. +const { chromium } = require('playwright'); + +const BASE = process.env.E2E_BASE || 'http://localhost:8890'; +const ID = process.env.E2E_CHARACTER_ID; +const EXECUTABLE = process.env.E2E_CHROMIUM || undefined; +const ITEM = 'Kaylee Seeded Alpha'; + +const kept = (page, id) => page.evaluate(id => { + const c = window.cljs.core, s = window.re_frame.db.app_db.state; + const nameKw = c.keyword('orcpub.dnd.e5.magic-items', 'name'); + const names = items => (items == null ? [] : c.clj__GT_js(c.vec(c.map(i => c.get(i, nameKw), items)))); + const overlay = c.deref(window.re_frame.core.subscribe( + c.PersistentVector.fromArray([c.keyword('orcpub.dnd.e5.magic-items', 'shared-custom-items')], true))); + return { + keptForThisCharacter: names(c.get(c.get(s, c.keyword(null, 'character-custom-items')), parseInt(id))), + overlay: names(overlay), + }; +}, id); + +async function view(browser, who) { + const ctx = await browser.newContext(); + // The cookie banner and What's New panel would take the clicks. + await ctx.addInitScript(() => { + try { + localStorage.setItem('orcpub:no-cookie-banner', '1'); + localStorage.setItem('whats-new-seen', JSON.stringify('summer-patch-2026')); + } catch (e) {} + }); + const page = await ctx.newPage(); + const errors = []; + page.on('pageerror', e => errors.push(e.message)); + let read = null; + page.on('response', async r => { + if (new URL(r.url()).pathname !== `/dnd/5e/characters/${ID}`) return; + try { + const body = await r.text(); + read = { status: r.status(), carriesItem: body.includes(ITEM), carriesEmail: body.includes('@example.com') }; + } catch (e) {} + }); + if (who) { + await page.goto(`${BASE}/pages/login-page`, { waitUntil: 'domcontentloaded' }); + await page.waitForSelector('input'); + await page.locator('input').nth(0).fill(who.user); + await page.locator('input').nth(1).fill(who.pass); + await page.locator('button.form-button').click(); + await page.waitForTimeout(3500); + } + await page.goto(`${BASE}/pages/dnd/5e/characters/${ID}`, { waitUntil: 'networkidle', timeout: 120000 }); + await page.waitForTimeout(4000); + const state = await kept(page, ID); + // The tab labels are lowercase text styled as capitals; switch the way the tab's click does. + await page.evaluate(() => { + const c = window.cljs.core; + window.re_frame.core.dispatch(c.PersistentVector.fromArray( + [c.keyword('orcpub.dnd.e5.character', 'set-selected-display-tab'), 'equipment'], true)); + }); + await page.waitForTimeout(2000); + const table = page.locator('tbody.other-magic-items'); + const tableText = (await table.count()) ? (await table.first().innerText()).replace(/\s+/g, ' ') : ''; + await ctx.close(); + return { name: who ? who.user : 'logged out', read, tableText, errors, ...state }; +} + +(async () => { + if (!ID) { + console.error('No E2E_CHARACTER_ID: run this through scripts/e2e/run.sh, which reads it from e2e-boot.'); + process.exit(2); + } + const browser = await chromium.launch({ executablePath: EXECUTABLE }); + const results = [ + await view(browser, null), + await view(browser, { user: 'zoe', pass: 'washburne7' }), + await view(browser, { user: 'kaylee', pass: 'serenity99' }), + ]; + await browser.close(); + + let failures = 0; + const check = (ok, label, detail) => { + if (!ok) failures++; + console.log(` ${ok ? 'PASS' : 'FAIL'} ${label}${detail ? ' -- ' + detail : ''}`); + }; + for (const r of results) { + console.log(`\n${r.name}:`); + check(r.read && r.read.status === 200, 'the character read succeeded'); + check(r.read && r.read.carriesItem, 'the character read carried the equipped item'); + check(r.read && !r.read.carriesEmail, 'the character read carried no email address'); + check(r.tableText.includes(ITEM), "the item is in the sheet's Other Magic Items table", r.tableText); + check(r.errors.length === 0, 'no uncaught page errors', r.errors.join(' | ')); + if (r.name === 'kaylee') { + check(r.keptForThisCharacter.length === 0, 'her own character keeps no server copy', JSON.stringify(r.keptForThisCharacter)); + } else { + check(JSON.stringify(r.keptForThisCharacter) === JSON.stringify([ITEM]), 'the item is kept for this character', JSON.stringify(r.keptForThisCharacter)); + check(r.overlay.includes(ITEM), "the item is in the sheet's overlay", JSON.stringify(r.overlay)); + } + } + console.log(failures ? `\nFAILURES: ${failures}` : '\nall checks passed'); + process.exit(failures ? 1 : 0); +})(); diff --git a/scripts/e2e/template-on-open.js b/scripts/e2e/template-on-open.js new file mode 100644 index 000000000..bca022372 --- /dev/null +++ b/scripts/e2e/template-on-open.js @@ -0,0 +1,105 @@ +// Needs: dev bundle (reads the app's internals, which a production bundle compiles away). +// When is the character template built, and what does that cost? +// +// ./scripts/e2e/run.sh template-on-open.js +// +// The template cache feeds the save, and the heal of a renamed pick waits for it (offered-keys). +// Pages with no character must not build it. Opening a character must start it, so the heal lands +// without a save, and must not make the builder noticeably slower. Prints timings so a change can +// be compared against the run before it. +const { chromium } = require('playwright'); +const { BASE, EXECUTABLE, checker } = require('./lib'); + +const OLD_KEY = 'half-elf-phb'; +const NEW_KEY = 'half-elf-ua'; +const PLUGINS = `{"Heal Pak" {:orcpub.dnd.e5/races {:${NEW_KEY} {:key :${NEW_KEY} :name "Half-Elf (UA)" :option-pack "Heal Pak" :former-key :${OLD_KEY}}}}}`; +const CACHE = '[:orcpub.dnd.e5.autosave-fx/cached-template]'; +const OFFERED = '[:orcpub.dnd.e5.content-reconciliation/offered-keys]'; +const RACE = '[:character :orcpub.entity/options :race :orcpub.entity/key]'; + +async function context(browser) { + const ctx = await browser.newContext({ viewport: { width: 1280, height: 900 } }); + await ctx.addInitScript(plugins => { + try { + localStorage.setItem('plugins', plugins); + localStorage.setItem('orcpub:no-cookie-banner', '1'); + localStorage.setItem('whats-new-seen', '"summer-patch-2026"'); + } catch (e) {} + window.__blocked = 0; + try { + new PerformanceObserver(l => l.getEntries().forEach(e => { window.__blocked += e.duration; })) + .observe({ type: 'longtask', buffered: true }); + } catch (e) {} + }, PLUGINS); + return ctx; +} + +const appReady = page => page.waitForFunction(() => { + try { return cljs.core.contains_QMARK_(cljs.core.deref(re_frame.db.app_db), cljs.reader.read_string(':plugins')); } + catch (e) { return false; } +}, null, { timeout: 60000 }); + +const isSet = (page, p) => page.evaluate(p => { + const v = cljs.core.get_in(cljs.core.deref(re_frame.db.app_db), cljs.reader.read_string(p)); + return v != null; +}, p); + +const dbAt = (page, p) => page.evaluate(p => + cljs.core.pr_str(cljs.core.get_in(cljs.core.deref(re_frame.db.app_db), cljs.reader.read_string(p))), p); + +async function until(page, fn, ms) { + const t0 = Date.now(); + while (Date.now() - t0 < ms) { if (await fn()) return Date.now() - t0; await page.waitForTimeout(100); } + return null; +} + +(async () => { + const { check, failures } = checker(); + const browser = await chromium.launch({ executablePath: EXECUTABLE }); + + // Pages with no character build nothing. + for (const [label, url] of [['login', '/pages/login-page'], ['character list', '/pages/dnd/5e/characters']]) { + const ctx = await context(browser); + const page = await ctx.newPage(); + await page.goto(`${BASE}${url}`, { waitUntil: 'domcontentloaded' }); + await appReady(page); + await page.waitForTimeout(4000); + check(!(await isSet(page, CACHE)) && !(await isSet(page, OFFERED)), `${label}: no template built`); + console.log(` ${label}: main thread blocked ${Math.round(await page.evaluate(() => window.__blocked))} ms`); + await ctx.close(); + } + + // Opening a character with a renamed pick: when does it heal, and what does the builder cost? + const runs = []; + for (let i = 0; i < 3; i++) { + const ctx = await context(browser); + const page = await ctx.newPage(); + const t0 = Date.now(); + await page.goto(`${BASE}/pages/dnd/5e/character-builder`, { waitUntil: 'domcontentloaded' }); + await appReady(page); + await page.waitForFunction(() => /Ability Scores/.test(document.body.innerText), null, { timeout: 60000 }); + const builderMs = Date.now() - t0; + const blockedBefore = await page.evaluate(() => window.__blocked); + await page.evaluate(oldKey => { + const rd = cljs.reader.read_string; + const db = cljs.core.deref(re_frame.db.app_db); + const broken = cljs.core.assoc_in(cljs.core.get(db, rd(':character')), + rd('[:orcpub.entity/options :race]'), rd(`{:orcpub.entity/key :${oldKey}}`)); + re_frame.core.dispatch_sync(cljs.core.conj(rd('[:orcpub.dnd.e5.character/open-character]'), broken)); + }, OLD_KEY); + const healMs = await until(page, async () => (await dbAt(page, RACE)) === `:${NEW_KEY}`, 15000); + const cacheMs = await until(page, () => isSet(page, CACHE), 1000); + await page.waitForTimeout(500); + const openBlocked = Math.round((await page.evaluate(() => window.__blocked)) - blockedBefore); + runs.push({ builderMs, healMs, openBlocked }); + console.log(` run ${i + 1}: builder ready ${builderMs} ms, heal after open ${healMs === null ? 'NONE in 15s' : healMs + ' ms'}, ` + + `cache ${cacheMs === null ? 'absent' : 'present'}, main thread blocked after open ${openBlocked} ms`); + await ctx.close(); + } + check(runs.every(r => r.healMs !== null), 'opening a character heals its renamed pick without a save', + runs.map(r => r.healMs).join(', ')); + + await browser.close(); + console.log(failures() ? `\nFAILURES: ${failures()}` : '\nall checks passed'); + process.exit(failures() ? 1 : 0); +})(); diff --git a/scripts/test/probe-baseline.json b/scripts/test/probe-baseline.json index 19a7403dc..e937ff790 100644 --- a/scripts/test/probe-baseline.json +++ b/scripts/test/probe-baseline.json @@ -1,11 +1,11 @@ { "_comment": "Per probe: `checks` = assertions it should RUN (pass or fail); a probe running fewer has quietly stopped asserting, which is what a renamed control inside an `if (await x.count())` guard does. `seconds` = observed runtime, used to size its timeout. Observed with ORCBREW_PACK set. Regenerate with UPDATE_BASELINE=1. export_busy_retry_e2e.js was observed under `lein e2e-server-busy`; BUSY_SERVER=1 runs it and skips the ordinary-server probes, which the busy profile would fail for reasons of its own.", "boot_rescue_e2e.js": { - "checks": 16, + "checks": 17, "seconds": 120 }, "builder_card_export_e2e.js": { - "checks": 11, + "checks": 13, "seconds": 150 }, "character_image_capture_e2e.js": { @@ -24,10 +24,18 @@ "checks": 9, "seconds": 41 }, + "fixes_persist_e2e.js": { + "checks": 35, + "seconds": 34 + }, "header_menus_e2e.js": { - "checks": 22, + "checks": 30, "seconds": 50 }, + "homebrew_safety_net_e2e.js": { + "checks": 16, + "seconds": 60 + }, "notification_flows_e2e.js": { "checks": 9, "seconds": 4 @@ -63,5 +71,9 @@ "whats_new_e2e.js": { "checks": 18, "seconds": 30 + }, + "character_heal_e2e.js": { + "checks": 13, + "seconds": 25 } } diff --git a/scripts/test/run-browser-probes.js b/scripts/test/run-browser-probes.js index da444d404..427f5dbf1 100755 --- a/scripts/test/run-browser-probes.js +++ b/scripts/test/run-browser-probes.js @@ -26,6 +26,7 @@ const { spawn } = require('child_process'); const fs = require('fs'), path = require('path'); const http = require('http'); +const { resolveBrowser } = require('../../test/browser/lib/find-chrome'); const ROOT = path.resolve(__dirname, '../..'); const SERVER = 'http://localhost:8890'; @@ -54,12 +55,15 @@ const BASELINE = path.join(__dirname, 'probe-baseline.json'); // needsPack: imports a homebrew library and asserts against its content. const PROBES = [ { file: 'character_image_capture_e2e.js', needs: 'server' }, + { file: 'character_heal_e2e.js', needs: 'server' }, { file: 'class_handlers_functional_e2e.js', needs: 'server', needsPack: true }, { file: 'equipment_add_functional_e2e.js', needs: 'server', needsPack: true }, { file: 'export_busy_retry_e2e.js', needs: 'busy-server' }, { file: 'boot_rescue_e2e.js', needs: 'server', suppress: false }, { file: 'builder_card_export_e2e.js', needs: 'server', suppress: false }, + { file: 'fixes_persist_e2e.js', needs: 'server' }, { file: 'header_menus_e2e.js', needs: 'server' }, + { file: 'homebrew_safety_net_e2e.js', needs: 'server' }, { file: 'overlay_reachability_e2e.js', needs: 'server' }, { file: 'notification_flows_e2e.js', needs: 'standalone' }, { file: 'notifications_acceptance_e2e.js', needs: 'standalone' }, @@ -150,6 +154,10 @@ function run(probe, pack, budgetMs) { } console.log(serverUp ? ' up' : ` giving up after ${waitS}s`); } + // Every probe launches Chromium. Without one they all fail the same way, which reads + // as thirty regressions; it is one missing install, so say that once and skip. + const browser = resolveBrowser(); + console.log(browser ? `browser: ${browser}` : 'browser: none found'); const pack = process.env.ORCBREW_PACK; const only = (process.env.ONLY || '').split(',').filter(Boolean); // 3 by default, measured: 421s wall against 976s sequential, 2.3x, with every probe @@ -162,6 +170,7 @@ function run(probe, pack, budgetMs) { const skipped = []; const skip = (p, why) => { skipped.push({ ...p, why }); }; queue = queue.filter(p => { + if (!browser) { skip(p, 'no Chromium — set CHROME to one, or run `npx playwright install chromium`'); return false; } if (p.needs === 'server' && !serverUp) { skip(p, `no server at ${SERVER} — run \`lein e2e-server\``); return false; } // Not merely unnecessary: this profile is the whole point of the probe, and against the // ordinary server the busy page never appears and every check fails. diff --git a/scripts/test/run-cljs-tests.js b/scripts/test/run-cljs-tests.js index 9bb0883fd..c150948ee 100644 --- a/scripts/test/run-cljs-tests.js +++ b/scripts/test/run-cljs-tests.js @@ -10,24 +10,12 @@ // cljs.test prints to the console. const http = require('http'), fs = require('fs'), path = require('path'); const { chromium } = require('playwright'); +const { findChrome } = require('../../test/browser/lib/find-chrome'); const ROOT = path.resolve(process.argv[2] || 'target/test'); const MIME = { '.js': 'application/javascript', '.html': 'text/html', '.map': 'application/json', '.json': 'application/json' }; -function findChrome() { - const base = process.env.PLAYWRIGHT_BROWSERS_PATH || '/opt/pw-browsers'; - try { - const dir = fs.readdirSync(base) - .filter(d => d.startsWith('chromium-') && !d.includes('headless')).sort().pop(); - if (dir) { - const p = path.join(base, dir, 'chrome-linux', 'chrome'); - if (fs.existsSync(p)) return p; - } - } catch (_) {} - return undefined; -} - // cljs-test-display, which the auto-testing main reports through, renders into // this element and asserts it exists. const HTML = ` diff --git a/src/clj/orcpub/config.clj b/src/clj/orcpub/config.clj index c4556ee56..f11b9825e 100644 --- a/src/clj/orcpub/config.clj +++ b/src/clj/orcpub/config.clj @@ -224,6 +224,50 @@ "APP_SOCIAL_BLUESKY" "APP_SOCIAL_DISCORD" "APP_SOCIAL_FACEBOOK" "APP_SOCIAL_PATREON" "APP_SOCIAL_REDDIT" "APP_SOCIAL_TWITTER"]) +(defn get-share-max-upload-kb + "Largest compressed homebrew one short share link keeps, in KB, from ORCPUB_SHARE_MAX_UPLOAD_KB. + + Measured 2026-09-13 on the MegaPak: a packed level 20 character's homebrew compresses to 12 to 25 KB, + and a wizard holding every subclass and spell in it to 40 KB. Over the cap, the owner's link carries + the homebrew itself instead." + [] + (positive-int-env ["ORCPUB_SHARE_MAX_UPLOAD_KB"] 64)) + +(defn get-share-max-text-kb + "Largest that homebrew may be unpacked, in KB, from ORCPUB_SHARE_MAX_TEXT_KB. The same measurement: + 34 to 112 KB for a packed level 20 character, 175 KB for the everything wizard. It also bounds what a + viewer's browser will unpack." + [] + (positive-int-env ["ORCPUB_SHARE_MAX_TEXT_KB"] 256)) + +(defn get-share-max-account-kb + "Shared homebrew one account keeps across all its characters, in KB, from ORCPUB_SHARE_MAX_ACCOUNT_KB. + 1024 is about 40 wizard-sized shares; a share over it carries the homebrew in its link instead." + [] + (positive-int-env ["ORCPUB_SHARE_MAX_ACCOUNT_KB"] 1024)) + +(defn get-share-prune-days + "Days a character's share link may go unused while the server runs before the link and its share data + are deleted, from ORCPUB_SHARE_PRUNE_DAYS; 0 keeps them. The character itself is never deleted. 180 + leaves room for a campaign that pauses for a season. 1 to 29 count as 30, so a typo cannot delete + links a day old." + [] + (let [n (some-> (env-raw "ORCPUB_SHARE_PRUNE_DAYS") str/trim parse-long)] + (cond + (or (nil? n) (neg? n)) 180 + (zero? n) 0 + :else (max n 30)))) + +(defn pwned-check-enabled? + "Whether a new password is screened against Have I Been Pwned, from + ORCPUB_PWNED_CHECK; on by default. + GOTCHA: fails open on a timeout, so a slow or down HIBP service never blocks + signup. Set the env var to off/false/0/no to disable the check entirely. + See account-flows.md." + [] + (let [raw (some-> (env-raw "ORCPUB_PWNED_CHECK") str/trim (.toLowerCase Locale/ROOT))] + (not (contains? #{"off" "false" "0" "no"} raw)))) + (def settings "Everything the boot banner reports, in print order. `:secret?`: the VALUE IS NEVER PRINTED, only whether it is present. @@ -258,12 +302,29 @@ {:group "capacity" :var "ORCPUB_PDF_MAX_CASTER_SECTIONS" :get #(get-pdf-max-caster-sections) :note "spellcasting sections one sheet may grow to"} {:group "capacity" :var "ORCPUB_PDF_MAX_CARDS" :get #(get-pdf-max-cards) - :note "cards of each kind per export; 9 to a sheet, so 22 sheets"}])) + :note "cards of each kind per export; 9 to a sheet, so 22 sheets"} + {:group "capacity" :var "ORCPUB_SHARE_MAX_UPLOAD_KB" :get #(get-share-max-upload-kb) + :note "compressed share data one share link keeps"} + {:group "capacity" :var "ORCPUB_SHARE_MAX_TEXT_KB" :get #(get-share-max-text-kb) + :note "that share data unpacked"} + {:group "capacity" :var "ORCPUB_SHARE_MAX_ACCOUNT_KB" :get #(get-share-max-account-kb) + :note "share data one account keeps in all"} + {:group "runtime" :var "ORCPUB_PWNED_CHECK" :get #(if (pwned-check-enabled?) "on" "off") + :accepts #(contains? #{"on" "off" "true" "false" "1" "0" "yes" "no"} (.toLowerCase (str/trim %) Locale/ROOT)) + :expects "on or off" + :note "screens a new password against known breaches; fails open, off disables the call"} + {:group "retention" :var "ORCPUB_SHARE_PRUNE_DAYS" :get #(get-share-prune-days) + :accepts #(some-> % str/trim parse-long (>= 0)) :expects "a whole number of days, 0 or more" + :note "days unused before a character's share link and share data are deleted; 0 keeps them, at least 30"}])) (def ^:private tunables "Kept for the capacity rows' typo detection: only these parse as integers." (filter #(= "capacity" (:group %)) settings)) +(defn- positive-int-text? [raw] + (let [n (try (Integer/parseInt (str/trim raw)) (catch NumberFormatException _ nil))] + (boolean (and n (pos? n))))) + (defn report "What each setting resolved to, and whether that came from the environment. @@ -271,17 +332,16 @@ change was picked up, ignored as a typo, or never set. Secret values are dropped here, not at print time, so nothing downstream can leak one by accident." [] - (for [{env-var :var lookup :get :keys [secret? redact? critical? note group fix]} settings] - (let [raw (env-raw env-var) - integer-setting? (some? (some #(= env-var (:var %)) tunables)) - parsed (when (and raw integer-setting?) - (try (Integer/parseInt (str/trim raw)) - (catch NumberFormatException _ nil))) - ignored? (boolean (and raw integer-setting? (not (and parsed (pos? parsed))))) - value (cond secret? nil - lookup (lookup) - :else raw)] - {:var env-var :group group :note note :critical? critical? :fix fix + (for [{env-var :var getter :get :keys [secret? redact? critical? note group fix accepts expects]} settings] + (let [raw (env-raw env-var) + tunable? (some? (some #(= env-var (:var %)) tunables)) + ;; A row with its own :accepts (the prune window takes 0) is checked by that instead. + accepts (or accepts (when tunable? positive-int-text?)) + ignored? (boolean (and raw accepts (not (accepts raw)))) + value (cond secret? nil + getter (getter) + :else raw)] + {:var env-var :group group :note note :critical? critical? :fix fix :expects expects :value (if (and value redact?) (redact-secrets value) value) :secret? secret? :raw (when-not secret? raw) @@ -330,9 +390,9 @@ (format " %s %s of %s set" (apply str (repeat w " ")) brand (count branding-vars))] (when-let [bad (seq (filter :ignored? rows))] (cons rule - (for [{env-var :var :keys [raw]} bad] - (format " (!) %s=%s was ignored: not a positive integer. The default above is in use." - env-var raw)))) + (for [{env-var :var raw :raw expects :expects} bad] + (format " (!) %s=%s was ignored: not %s. The default above is in use." + env-var raw (or expects "a positive integer"))))) ;; A missing SIGNATURE means every login fails. That is not a row to be spotted. ;; Say what broke AND how to fix it. A boot line that names a symptom and leaves the ;; remedy to be guessed just moves the work. diff --git a/src/clj/orcpub/datomic.clj b/src/clj/orcpub/datomic.clj index da9a23e8f..573efb931 100644 --- a/src/clj/orcpub/datomic.clj +++ b/src/clj/orcpub/datomic.clj @@ -57,3 +57,9 @@ (defn new-datomic [uri] ;; The real uri, not a redacted one -- this is what d/connect is given. (map->DatomicComponent {:uri uri})) + +(defn cas-failed? + "Whether a transaction failed because a :db/cas found a value other than the one it expected." + [e] + (boolean (some #(= :db.error/cas-failed (:db/error (ex-data %))) + (take-while some? (iterate #(.getCause ^Throwable %) e))))) diff --git a/src/clj/orcpub/db/schema.clj b/src/clj/orcpub/db/schema.clj index 07b4cefb6..7e9f3a0d5 100644 --- a/src/clj/orcpub/db/schema.clj +++ b/src/clj/orcpub/db/schema.clj @@ -147,6 +147,12 @@ {:db/ident :orcpub.user/password-reset :db/valueType :db.type/instant :db/cardinality :db.cardinality/one} + ;; When the account holder last signed every session out. Separate from + ;; password-reset because signing out everywhere must not require changing + ;; the password, and the withdrawal register takes the LATER of the two. + {:db/ident :orcpub.user/sessions-withdrawn + :db/valueType :db.type/instant + :db/cardinality :db.cardinality/one} {:db/ident :orcpub.user/password-reset-key :db/valueType :db.type/string :db/cardinality :db.cardinality/one} @@ -328,7 +334,19 @@ :db/cardinality :db.cardinality/one} {:db/ident ::party5e/character-ids :db/valueType :db.type/ref - :db/cardinality :db.cardinality/many}]) + :db/cardinality :db.cardinality/many} + ;; The share token a character was added with, when it came from a share link, so the party page + ;; can load that character's homebrew. + {:db/ident ::party5e/shared-tokens + :db/valueType :db.type/ref + :db/cardinality :db.cardinality/many + :db/isComponent true} + {:db/ident :orcpub.party-share/character + :db/valueType :db.type/long + :db/cardinality :db.cardinality/one} + {:db/ident :orcpub.party-share/token + :db/valueType :db.type/string + :db/cardinality :db.cardinality/one}]) (def folder-schema [{:db/ident ::folder5e/owner @@ -409,6 +427,37 @@ ::weapon5e/reach? ::weapon5e/ammunition?]))) +(def share-schema + "Shared homebrew, see orcpub.routes.share: one record per character, holding the homebrew its owner + last shared, compressed, and the token a share link must carry to load it." + [{:db/ident :orcpub.share/character :db/valueType :db.type/long :db/cardinality :db.cardinality/one + :db/unique :db.unique/identity} + {:db/ident :orcpub.share/token :db/valueType :db.type/string :db/cardinality :db.cardinality/one + :db/noHistory true} + {:db/ident :orcpub.share/digest :db/valueType :db.type/string :db/cardinality :db.cardinality/one + :db/noHistory true} + {:db/ident :orcpub.share/owner :db/valueType :db.type/string :db/cardinality :db.cardinality/one + :db/index true} + {:db/ident :orcpub.share/bundle :db/valueType :db.type/bytes :db/cardinality :db.cardinality/one + :db/noHistory true} + {:db/ident :orcpub.share/size :db/valueType :db.type/long :db/cardinality :db.cardinality/one} + ;; When the share was last used, recorded at most daily; see orcpub.routes.share/prune!. + {:db/ident :orcpub.share/used :db/valueType :db.type/instant :db/cardinality :db.cardinality/one + :db/noHistory true} + ;; A share that expired unused, kept as its character and the date so the owner's page can say so; see + ;; orcpub.routes.share/prune!. + {:db/ident :orcpub.share-expiry/character :db/valueType :db.type/long :db/cardinality :db.cardinality/one + :db/unique :db.unique/identity} + {:db/ident :orcpub.share-expiry/on :db/valueType :db.type/instant :db/cardinality :db.cardinality/one}]) + +(def heartbeat-schema + "When the server was running, see orcpub.heartbeat: the last beat, on the entity :orcpub.heartbeat/clock, + and each gap between beats long enough to mean the server was off." + [{:db/ident :orcpub.heartbeat/beat :db/valueType :db.type/instant :db/cardinality :db.cardinality/one + :db/noHistory true} + {:db/ident :orcpub.outage/from :db/valueType :db.type/instant :db/cardinality :db.cardinality/one} + {:db/ident :orcpub.outage/to :db/valueType :db.type/instant :db/cardinality :db.cardinality/one}]) + (def all-schemas (concat user-schema @@ -420,4 +469,6 @@ party-schema folder-schema magic-item-schema - weapon-schema)) + weapon-schema + share-schema + heartbeat-schema)) diff --git a/src/clj/orcpub/email.clj b/src/clj/orcpub/email.clj index 7c7a23da8..81dd271ec 100644 --- a/src/clj/orcpub/email.clj +++ b/src/clj/orcpub/email.clj @@ -220,6 +220,176 @@ :username username} e))))) +(defn describe-browser + "A user agent in the words a person would use. Deliberately coarse: this goes + in front of somebody deciding whether an attempt was their own, and \"Chrome on + Windows\" answers that where the raw header does not. Order matters -- Edge and + Opera both claim to be Chrome, and Chrome claims to be Safari." + [ua] + (if (s/blank? ua) + "an unrecognised browser" + (let [browser (cond + (re-find #"(?i)edg/" ua) "Edge" + (re-find #"(?i)opr/|opera" ua) "Opera" + (re-find #"(?i)firefox/" ua) "Firefox" + (re-find #"(?i)chrome/|crios/" ua) "Chrome" + (re-find #"(?i)safari/" ua) "Safari" + :else nil) + platform (cond + (re-find #"(?i)iphone|ipad|ipod" ua) "iOS" + (re-find #"(?i)android" ua) "Android" + (re-find #"(?i)windows" ua) "Windows" + (re-find #"(?i)mac os x|macintosh" ua) "a Mac" + (re-find #"(?i)linux" ua) "Linux" + :else nil)] + (cond + (and browser platform) (str browser " on " platform) + browser browser + platform (str "a browser on " platform) + :else "an unrecognised browser")))) + +(defn- attempt-time [] + (.format (java.time.format.DateTimeFormatter/ofPattern "d MMMM yyyy 'at' HH:mm 'UTC'") + (java.time.ZonedDateTime/now java.time.ZoneOffset/UTC))) + +(defn sign-in-attempts-email-html [first-and-last-name reset-url browser when-str] + (into + [:div + (str "Dear " (if (seq first-and-last-name) first-and-last-name (str branding/app-name " User")) ",") + [:br] + [:br] + "Someone tried to sign in to your account several times just now, from more than one place at once." + [:br] + [:br] + (str "The last of those attempts was on " when-str ", from " browser ".") + [:br] + [:br] + "Usually this is nothing. A phone and a laptop both signed out at the same time will do it, as will a password you half remember. Nobody got in, nothing has changed, and your account is not locked." + [:br] + [:br] + "If none of it was you, pick a new password when you get a moment. You can start that here:" + [:br] + [:br] + [:a {:href reset-url} reset-url] + [:br] + [:br] + "Sincerely," + [:br] + [:br] + (str "The " branding/email-sender-name)] + (social-links-footer))) + +(defn sign-in-attempts-email [first-and-last-name reset-url browser when-str] + [{:type "text/html" + :content (str (hiccup/html (sign-in-attempts-email-html + first-and-last-name reset-url browser when-str)))}]) + +(defn send-sign-in-attempts-email + "Tells an account holder that failed sign-ins arrived from several places at once. + GOTCHA: never throws and leaks nothing (success, failure, timing) to the + response -- sent on behalf of someone not signed in, possibly an attacker. + Links to the reset PAGE, never a token, to avoid training people to click + mailed credentials. See account-flows.md." + [base-url {:keys [email first-and-last-name user-agent]}] + (try + (postal/send-message (email-cfg) + {:from (str branding/email-sender-name " <" (emailfrom) ">") + :to email + :subject (str branding/app-name " sign-in attempts") + :body (sign-in-attempts-email + first-and-last-name + (str base-url (routes/path-for routes/send-password-reset-page-route)) + (describe-browser user-agent) + (attempt-time))}) + (catch Exception e + (println "ERROR: could not send a sign-in attempts notice:" (.getMessage e))))) + +(defn password-changed-email-html [first-and-last-name reset-url browser when-str] + (into + [:div + (str "Dear " (if (seq first-and-last-name) first-and-last-name (str branding/app-name " User")) ",") + [:br] [:br] + "The password on your account was just changed." + [:br] [:br] + (str "That was on " when-str ", from " browser ".") + [:br] [:br] + "If it was you, there is nothing to do." + [:br] [:br] + "If it was not, somebody else can sign in as you right now. Change the password back straight away -- you will need access to this mailbox, which is how the change was made in the first place:" + [:br] [:br] + [:a {:href reset-url} reset-url] + [:br] [:br] + "Sincerely," + [:br] [:br] + (str "The " branding/email-sender-name)] + (social-links-footer))) + +(defn password-changed-email [first-and-last-name reset-url browser when-str] + [{:type "text/html" + :content (str (hiccup/html (password-changed-email-html + first-and-last-name reset-url browser when-str)))}]) + +(defn send-password-changed-email + "Tells an account holder their password has just been changed. + GOTCHA: the only signal that makes an unauthorised reset visible; never + throws since the reset is already committed. Same no-token reset-page link + as the sign-in notice. See account-flows.md." + [base-url {:keys [email first-and-last-name user-agent]}] + (try + (postal/send-message (email-cfg) + {:from (str branding/email-sender-name " <" (emailfrom) ">") + :to email + :subject (str branding/app-name " password changed") + :body (password-changed-email + first-and-last-name + (str base-url (routes/path-for routes/send-password-reset-page-route)) + (describe-browser user-agent) + (attempt-time))}) + (catch Exception e + (println "ERROR: could not send a password-changed notice:" (.getMessage e))))) + +(defn email-change-notice-html [first-and-last-name new-email when-str] + (into + [:div + (str "Dear " (if (seq first-and-last-name) first-and-last-name (str branding/app-name " User")) ",") + [:br] [:br] + (str "Somebody asked us to move your account to " new-email ".") + [:br] [:br] + (str "That was on " when-str ". Nothing has moved yet -- the new address has to confirm first.") + [:br] [:br] + "If it was you, carry on: the confirmation is waiting in that other mailbox." + [:br] [:br] + "If it was not, somebody is signed in to your account. Change your password now, from this address, while it still reaches you:" + [:br] [:br] + "Sincerely," + [:br] [:br] + (str "The " branding/email-sender-name)] + (social-links-footer))) + +(defn email-change-notice [first-and-last-name new-email when-str] + [{:type "text/html" + :content (str (hiccup/html (email-change-notice-html + first-and-last-name new-email when-str)))}]) + +(defn send-email-change-notice + "Tells the address that currently OWNS the account that somebody is moving it. + GOTCHA: goes to the new address, unreachable by the current owner if this + wasn't them -- the only notice the losing address gets. Never throws: the + change is already accepted, so a failure here must not report an error for it. + See account-flows.md." + [base-url {:keys [email first-and-last-name new-email]}] + (try + (postal/send-message (email-cfg) + {:from (str branding/email-sender-name " <" (emailfrom) ">") + :to email + :subject (str branding/app-name " email change requested") + :body (email-change-notice + first-and-last-name + new-email + (attempt-time))}) + (catch Exception e + (println "ERROR: could not send an email-change notice:" (.getMessage e))))) + (defn unsubscribe-url "Build a full unsubscribe URL from a base-url and a pre-signed JWT token. Token is generated by routes/unsubscribe-token to avoid circular deps." diff --git a/src/clj/orcpub/heartbeat.clj b/src/clj/orcpub/heartbeat.clj new file mode 100644 index 000000000..cf22cc453 --- /dev/null +++ b/src/clj/orcpub/heartbeat.clj @@ -0,0 +1,78 @@ +;; When the server was running, kept in the database: an hourly beat, and each gap between beats long +;; enough to mean the server was off or its clock jumped ahead. Scheduled jobs run on the same tick, after +;; the beat, so a job that measures time (share link pruning) can leave out the time the server was off. +(ns orcpub.heartbeat + (:require [com.stuartsierra.component :as component] + [datomic.api :as d]) + (:import [java.util.concurrent Executors ScheduledExecutorService ThreadFactory TimeUnit])) + +(defn now [] (java.util.Date.)) + +(def ^:private hour-ms (* 60 60 1000)) + +(def ^:private gap-ms + "Beats come hourly; a longer gap than this means the server was off or its clock jumped ahead." + (+ hour-ms (* 10 60 1000))) + +(defn last-beat + "When the server last recorded a beat, or nil." + [db] + (d/q '[:find ?t . :where [?e :db/ident :orcpub.heartbeat/clock] [?e :orcpub.heartbeat/beat ?t]] db)) + +(defn beat! + "Records that the server is running, and records the time since the last beat as an outage when the gap + is longer than hourly beats explain. A clock set back records no outage." + [conn] + (let [at (now) + previous ^java.util.Date (last-beat (d/db conn))] + @(d/transact conn (cond-> [{:db/ident :orcpub.heartbeat/clock :orcpub.heartbeat/beat at}] + (and previous (> (- (.getTime at) (.getTime previous)) gap-ms)) + (conj {:orcpub.outage/from previous :orcpub.outage/to at}))))) + +(defn outages + "Every recorded outage, as [from to] dates." + [db] + (d/q '[:find ?from ?to :where [?o :orcpub.outage/from ?from] [?o :orcpub.outage/to ?to]] db)) + +(defn running-ms + "Milliseconds from `from` to `to` that the server was running, going by `off`, the [from to] pairs + outages returns. Negative when `from` is later than `to`, as after a clock set back." + [off ^java.util.Date from ^java.util.Date to] + (let [a (.getTime from) + b (.getTime to)] + (- b a (reduce + (for [[^java.util.Date start ^java.util.Date end] off] + (max 0 (- (min b (.getTime end)) (max a (.getTime start))))))))) + +(defn tick + "One heartbeat: the beat, then each job. Jobs wait for the next tick when the beat cannot be recorded, + since an unrecorded outage would count against whatever they measure; one job failing does not stop + the others." + [conn jobs] + (when (try (beat! conn) true + (catch Throwable e + (println "WARNING: the heartbeat was not recorded, so scheduled jobs wait an hour:" (.getMessage e)) + false)) + (doseq [[job-name job] jobs] + (try (job conn) + (catch Throwable e + (println "WARNING:" job-name "failed and runs again in an hour:" (.getMessage e))))))) + +(defrecord Heartbeat [conn jobs ^ScheduledExecutorService executor] + component/Lifecycle + (start [this] + (if executor + this + (let [ex (Executors/newSingleThreadScheduledExecutor + (reify ThreadFactory + (newThread [_ r] (doto (Thread. ^Runnable r "heartbeat") (.setDaemon true)))))] + (.scheduleAtFixedRate ex ^Runnable (fn [] (tick (:conn conn) jobs)) 1 60 TimeUnit/MINUTES) + (assoc this :executor ex)))) + (stop [this] + (when executor (.shutdownNow executor)) + (assoc this :executor nil))) + +(defn new-heartbeat + "A heartbeat that runs `jobs`, a map of a name for the log to (fn [conn]), a minute after the server + starts and then hourly, each time after the beat." + [jobs] + (map->Heartbeat {:jobs jobs})) diff --git a/src/clj/orcpub/index.clj b/src/clj/orcpub/index.clj index fa57ab9ee..73b56c00d 100644 --- a/src/clj/orcpub/index.clj +++ b/src/clj/orcpub/index.clj @@ -6,6 +6,7 @@ [orcpub.dnd.e5.views-2 :as views-2] [orcpub.favicon :as fi] [orcpub.fork.integrations :as integrations] + [orcpub.loading-spinner :as spinner] [orcpub.env :as env])) (def homebrew-url @@ -123,7 +124,8 @@ description image fb-type - nonce]} + nonce + spinner-kind]} & [splash?]] (html5 {:lang :en} @@ -227,6 +229,7 @@ table { html { min-height: 100%; }"] + [:style spinner/css] [:title title] (integrations/head-tags nonce) (script-tag {:nonce nonce} @@ -236,9 +239,7 @@ html { [:div#app (if splash? (views-2/splash-page) - [:div.h-full {:style "display:flex;justify-content:space-around"} - [:img {:src "/image/spiral.gif" - :style "height:200px;width:200px;margin-top:200px"}]])] + (spinner/markup (or spinner-kind (spinner/pick nil))))] ;; Homebrew rescue, a dead-man's switch: present by default, removed by the app once it has ;; rendered, so any boot failure (broken bundle, CLJS error, crashing homebrew) leaves it up. ;; It depends on nothing the app owns: server-rendered markup, inline styles (styles.css may diff --git a/src/clj/orcpub/loading_spinner.clj b/src/clj/orcpub/loading_spinner.clj new file mode 100644 index 000000000..6c285714a --- /dev/null +++ b/src/clj/orcpub/loading_spinner.clj @@ -0,0 +1,134 @@ +(ns orcpub.loading-spinner + "The spinner the server page shows until the app mounts: a spiral, a tumbling d20 or a spell + circle, picked at random per load, or by `?spinner=` for testing. Inline markup and CSS, so it + shows before any stylesheet or script loads. + GOTCHA: animate only transform and opacity, each moving piece its own HTML element. Anything + else repaints on the main thread and freezes while the app is building, when it is on screen." + (:require [clojure.string :as str])) + +(def kinds [:spiral :d20 :spell]) + +(defn- n + "`x` to one decimal. Locale/ROOT: a default locale can write 100,5 and break the SVG." + [x] + (String/format java.util.Locale/ROOT "%.1f" (object-array [(double x)]))) + +(def ^:private spiral-path + (let [steps 218] + (str "M" + (str/join " L" + (for [i (range (inc steps)) + :let [th (Math/toRadians (* 7.0 i)) + r (+ 5 (* 79.0 (/ i steps)))]] + (str (n (+ 100 (* r (Math/cos th)))) " " (n (+ 100 (* r (Math/sin th)))))))))) + +(defn- spiral [] + (str "")) + +;; d20: the 12 vertices of an icosahedron, its 20 faces found as vertex triples one edge apart, and +;; for each face the matrix3d that lays a flat 60px triangle onto it, front facing outward. +(defn- v- [a b] (mapv - a b)) +(defn- v+ [a b] (mapv + a b)) +(defn- v* [a k] (mapv #(* % k) a)) +(defn- dot [a b] (reduce + (map * a b))) +(defn- cross [[a1 a2 a3] [b1 b2 b3]] [(- (* a2 b3) (* a3 b2)) (- (* a3 b1) (* a1 b3)) (- (* a1 b2) (* a2 b1))]) +(defn- unit [a] (v* a (/ 1 (Math/sqrt (dot a a))))) +(defn- dist [a b] (Math/sqrt (dot (v- a b) (v- a b)))) + +(def ^:private d20-faces + (let [p (/ (+ 1 (Math/sqrt 5)) 2) + verts (vec (for [a [-1 1] b [-1 1] v [[0 a (* b p)] [a (* b p) 0] [(* b p) 0 a]]] v)) + faces (filter (fn [[i j k]] (every? #(< (Math/abs (- (apply dist (map verts %)) 2.0)) 1e-6) + [[i j] [j k] [i k]])) + (for [i (range 12) j (range (inc i) 12) k (range (inc j) 12)] [i j k])) + side 60.0 + h (* side (/ (Math/sqrt 3) 2)) + light (unit [-0.5 -0.8 0.6])] + (for [f faces + :let [[a b c] (map #(v* (verts %) (/ side 2)) f) + centre (v* (v+ (v+ a b) c) (/ 1 3.0)) + y (unit (v- c (v* (v+ a b) 0.5))) + [a b] (if (neg? (dot (cross (unit (v- b a)) (v* y -1)) centre)) [b a] [a b]) + x (unit (v- b a)) + z (cross x (v* y -1)) + t (v+ a (v* y h)) + lit (+ 34 (int (* 30 (max 0 (dot (unit centre) light)))))]] + {:matrix (str/join "," (map #(String/format java.util.Locale/ROOT "%.4f" (object-array [(double %)])) + (concat x [0] (v* y -1) [0] z [0] t [1]))) + :lit lit}))) + +(defn- d20 [] + (str "
" + (apply str (for [{:keys [matrix lit]} d20-faces] + (str ""))) + "
")) + +;; Spell circle: twelve made-up runes, each three strokes between points of a 3x3 grid, around a ring. +(def ^:private rune-points [[-4 -6] [4 -6] [-4 0] [4 0] [-4 6] [4 6] [0 -6] [0 6] [0 0]]) + +(def ^:private runes + (apply str + (for [i (range 12) + s (range 3) + :let [th (/ (* 2 Math/PI i) 12) + cx (+ 100 (* 76 (Math/sin th))) + cy (- 100 (* 76 (Math/cos th))) + rot (fn [[x y]] [(+ cx (- (* x (Math/cos th)) (* y (Math/sin th)))) + (+ cy (* x (Math/sin th)) (* y (Math/cos th)))]) + [x1 y1] (rot (rune-points (mod (+ (* 5 i) (* 2 s)) 9))) + [x2 y2] (rot (rune-points (mod (+ (* 5 i) (* 2 s) 1 (* 3 s)) 9)))]] + (str "M" (n x1) " " (n y1) "L" (n x2) " " (n y2))))) + +(def ^:private hexagram + (let [pt #(let [a (/ (* 2 Math/PI %) 6)] (str (n (+ 100 (* 56 (Math/sin a)))) " " (n (- 100 (* 56 (Math/cos a))))))] + (str "M" (str/join "L" (map pt [0 2 4])) "Z" + "M" (str/join "L" (map pt [1 3 5])) "Z"))) + +(defn- spell [] + (str "
" + "" + "" + "" + "" + "" + "" + "" + "
")) + +(def css + "Styles for all three, for the page head." + (str ".ls{display:flex;justify-content:center;padding-top:180px}" + ".ls-box{width:200px;height:200px;display:grid;place-items:center}" + "@keyframes ls-turn{to{transform:rotate(360deg)}}" + ".ls-spiral{width:200px;height:200px;animation:ls-turn 1.6s linear infinite}" + ".ls-d20-stage{width:160px;height:160px;perspective:520px;display:grid;place-items:center;transform:scale(1.3)}" + ".ls-d20{position:relative;width:0;height:0;transform-style:preserve-3d;" + "animation:ls-roll 3.2s cubic-bezier(.45,.05,.35,1) infinite}" + ".ls-d20 i{position:absolute;left:0;top:0;width:60px;height:51.96px;transform-origin:0 0;" + "backface-visibility:hidden;clip-path:polygon(0 100%,100% 100%,50% 0);background:hsl(30 70% calc(var(--l) - 12%))}" + ".ls-d20 i::before{content:'';position:absolute;inset:0;clip-path:polygon(7% 95%,93% 95%,50% 8%);" + "background:hsl(38 96% var(--l))}" + "@keyframes ls-roll{to{transform:rotateX(360deg) rotateY(720deg) rotateZ(360deg)}}" + ".ls-spell{position:relative;width:180px;height:180px}" + ".ls-spell svg{position:absolute;inset:0;width:100%;height:100%}" + ".ls-spell-outer{animation:ls-turn 9s linear infinite}" + ".ls-spell-inner{animation:ls-turn 6s linear infinite reverse}" + ".ls-spell-core{position:absolute;inset:38%;border-radius:50%;" + "background:radial-gradient(#f5b942,transparent 70%);animation:ls-pulse 1.6s ease-in-out infinite}" + "@keyframes ls-pulse{0%,100%{transform:scale(.55);opacity:.45}50%{transform:scale(1);opacity:1}}" + "@media (prefers-reduced-motion:reduce){.ls-spiral,.ls-d20,.ls-spell-outer,.ls-spell-inner," + ".ls-spell-core{animation-duration:8s}}")) + +(defn pick + "The spinner kind for a request: `requested` (a ?spinner= value) when it names one, else random." + [requested] + (or (some #{(keyword requested)} kinds) (rand-nth kinds))) + +(defn markup + "The placeholder markup for spinner `kind`, tagged data-spinner with its name." + [kind] + (str "
" + (case kind :spiral (spiral) :d20 (d20) :spell (spell)) + "
")) diff --git a/src/clj/orcpub/pwned.clj b/src/clj/orcpub/pwned.clj new file mode 100644 index 000000000..fed13cd98 --- /dev/null +++ b/src/clj/orcpub/pwned.clj @@ -0,0 +1,119 @@ +(ns orcpub.pwned + "Screens a password against Have I Been Pwned without ever sending it. + GOTCHA: only the first five hex chars of the local SHA-1 hash leave this + process (k-anonymity range query); a listener who breaks TLS learns at most + 1-in-~800 candidate hashes, never the password. Every failure path returns + :unknown rather than throwing or blocking. See account-flows.md." + (:require [clj-http.client :as client] + [clojure.string :as s] + [orcpub.config :as config]) + (:import (java.security MessageDigest))) + +;; Counted since boot for audit: fail-open must not mean fail-open BLIND. A +;; permanently broken lookup -- unset env var, firewall, a CDN rejecting our +;; User-Agent -- is otherwise indistinguishable from a working one: registration +;; succeeds either way and nobody learns their password is common. +(def ^:private tally (atom {:asked 0 :common 0 :clear 0 :unavailable 0})) + +(defn stats [] @tally) + +(defn take-stats! + "The counts since the last time anyone asked, and resets them. Used by the + hourly summary, which reports a period rather than a running total." + [] + (first (reset-vals! tally {:asked 0 :common 0 :clear 0 :unavailable 0}))) + +(defn summary-line + "A line for the log, or nil when nothing happened and there is nothing to say." + [{:keys [asked common clear unavailable]}] + (when (pos? (or asked 0)) + (format "pwned: %d checked, %d common, %d clear, %d unavailable" asked common clear unavailable))) + +(def ^:private endpoint "https://api.pwnedpasswords.com/range/") + +;; Short enough that a stalled third party cannot hold a signup open. Registration +;; already does real work after this; seconds here are seconds a person waits. +(def ^:private timeout-ms 2500) + +(defn- sha1-hex [^String value] + (let [digest (.digest (MessageDigest/getInstance "SHA-1") + (.getBytes value "UTF-8"))] + (s/upper-case (apply str (map #(format "%02x" %) digest))))) + +(defn- parse-suffixes + "The body is SUFFIX:COUNT per line. Returns a map of suffix to count." + [body] + (into {} + (keep (fn [line] + (let [[suffix count-str] (s/split (s/trim line) #":" 2)] + (when (and suffix count-str) + (when-let [n (try (Long/parseLong (s/trim count-str)) + (catch Exception _ nil))] + [(s/upper-case suffix) n]))))) + (s/split-lines (or body "")))) + +;; A sustained outage would otherwise write a line per signup. One a minute is +;; enough to see it in a log without burying everything else. +(def ^:private log-window-ms 60000) +(def ^:private last-logged (atom 0)) + +(defn- note-unavailable! [reason] + (swap! tally update :unavailable inc) + (let [now (System/currentTimeMillis) + [before _] (swap-vals! last-logged #(if (> (- now %) log-window-ms) now %))] + (when (> (- now before) log-window-ms) + ;; The reason carries no password material: not the password, not its hash, + ;; not the prefix, not the account it belonged to. + (println "pwned: lookup unavailable -" reason))) + :unknown) + +(defn check + "How many known breaches this password appears in. + + Returns a count (0 means not found), or :unknown when the service could not + answer. :unknown is not a failure to handle -- it means the caller learned + nothing and should carry on." + [password] + (if (or (s/blank? password) (not (config/pwned-check-enabled?))) + :unknown + (try + (swap! tally update :asked inc) + (let [digest (sha1-hex password) + prefix (subs digest 0 5) + suffix (subs digest 5) + ;; Add-Padding makes every response a similar size, so the length of + ;; the reply does not hint at how common the password is. + {:keys [status body]} (client/get (str endpoint prefix) + {:headers {"Add-Padding" "true" + "User-Agent" "orcpub-password-check"} + :socket-timeout timeout-ms + :connection-timeout timeout-ms + :throw-exceptions false + ;; The CDN sets a cookie we neither want nor + ;; can parse; without this every call logs a + ;; warning about it on the way past. + :cookie-policy :none + ;; A redirect off this host would be + ;; somewhere we never agreed to talk to. + :redirect-strategy :none})] + (if (= 200 status) + (let [n (get (parse-suffixes body) suffix 0)] + (swap! tally update (if (pos? n) :common :clear) inc) + n) + (note-unavailable! (str "HTTP " status)))) + (catch Exception e + ;; The class and message describe the transport, never the password. + (note-unavailable! (.. e getClass getSimpleName)))))) + +(defn breached? + "True only when the service positively said this password is known. An + unreachable service is not a breached password." + [password] + (let [result (check password)] + (and (number? result) (pos? result)))) + +(defn summary-job + "Heartbeat job: one line an hour, and only when something was checked." + [_conn] + (when-let [line (summary-line (take-stats!))] + (println line))) diff --git a/src/clj/orcpub/routes.clj b/src/clj/orcpub/routes.clj index c0a01ecd6..7059df250 100644 --- a/src/clj/orcpub/routes.clj +++ b/src/clj/orcpub/routes.clj @@ -26,6 +26,7 @@ [orcpub.dnd.e5.spells :as spells] [orcpub.dnd.e5.spell-annotations :as spell-annotations] [orcpub.dnd.e5.magic-items :as mi5e] + [orcpub.dnd.e5.share-bundle :as sb] [orcpub.dnd.e5.template :as t5e] [datomic.api :as d] [bidi.bidi :as bidi] @@ -35,9 +36,11 @@ [orcpub.privacy :as privacy] [orcpub.config :as config] [orcpub.email :as email] + [orcpub.loading-spinner :as spinner] [orcpub.index :refer [index-page]] [orcpub.pdf :as pdf] [orcpub.registration :as registration] + [orcpub.pwned :as pwned] [orcpub.entity.strict :as se] [orcpub.entity :as entity] [orcpub.security :as security] @@ -46,6 +49,8 @@ [orcpub.fork.user-data :as user-data] [orcpub.routes.party :as party] [orcpub.routes.folder :as folder] + [orcpub.routes.share :as share] + [orcpub.datomic :as odb] [hiccup.page :as page] [hiccup2.core :as h] [orcpub.env :as env] @@ -202,16 +207,79 @@ (try (let [request (:request context) updated-request (authentication-request request backend) - username (get-in updated-request [:identity :user])] - (if (and (:identity updated-request) - username) - (assoc context :request (assoc updated-request :username username)) - (terminate-request context 401 "Unauthorized"))) + username (get-in updated-request [:identity :user]) + minted (get-in updated-request [:identity :minted])] + (cond + (not (and (:identity updated-request) username)) + (terminate-request context 401 "Unauthorized") + + ;; Changing a password takes back the sessions that existed + ;; before it. The same 401 as any other refusal: which of + ;; the two reasons it was is not the caller's business. + (security/token-withdrawn? username minted) + (terminate-request context 401 "Unauthorized") + + :else + (assoc context :request (assoc updated-request :username username)))) (catch Exception e (terminate-request context 401 (str "Authentication failed: " (.getMessage e)))))))})) +(defn withdraw-sessions + "Signs every session for this account out, including the one asking. + + Deliberately does NOT require the password: somebody who thinks their account + is open somewhere they cannot reach should be able to close it from the + session they already hold, and they have already proved they hold it. It + grants an attacker with a session nothing -- they can sign themselves out." + [{:keys [conn db identity] :as _request}] + (let [username (:user identity) + {:keys [:db/id]} (when username (first-user-by db username-query username))] + (if-not id + {:status 400 :body {:error :user-not-found}} + (let [now (java.util.Date.)] + @(d/transact conn [{:db/id id :orcpub.user/sessions-withdrawn now}]) + (security/note-password-changed! username (.getTime now)) + {:status 200 :body {:withdrawn true}})))) + +(def ^:private recent-password-changes-query + ;; Either reason a token is withdrawn. The caller keeps the LATER per account, + ;; so signing out everywhere and then resetting -- or the reverse -- does not + ;; let the earlier of the two reinstate anything. + '[:find ?username ?changed + :in $ ?since + :where + (or [?e :orcpub.user/password-reset ?changed] + [?e :orcpub.user/sessions-withdrawn ?changed]) + [(> ?changed ?since)] + [?e :orcpub.user/username ?username]]) + +(defn refresh-token-withdrawals! + "Rebuilds the in-memory register of passwords that moved recently, from the + database, where the durable record lives. + GOTCHA: memory-only, so a restart silently reinstates every token it was + holding, looking exactly like working; only changes inside the token + lifetime matter, since older ones already expired. See account-flows.md." + [db] + (let [since (java.util.Date. (- (System/currentTimeMillis) + (* auth/token-lifetime-hours 60 60 1000))) + rows (d/q recent-password-changes-query db since)] + (security/absorb-password-changes! + (reduce (fn [m [username ^java.util.Date changed]] + (update m username (fnil max 0) (.getTime changed))) + {} + rows) + (.getTime since)) + (count rows))) + +(defn withdrawal-refresh-job + "Heartbeat job: re-read the register from the database. Also repairs it if a + note was ever lost, since the database is the record and this is the cache." + [conn] + (refresh-token-withdrawals! (d/db conn)) + nil) + (defn party-owner [db id] (d/q '[:find ?owner . :in $ ?id @@ -274,7 +342,11 @@ {:error error-key})}) (defn create-token [username exp] + ;; :minted, not :iat. buddy validates the registered claims it recognises, and + ;; a standard iat is seconds where this is millis -- naming it apart keeps the + ;; library out of it entirely. (jwt/sign {:user username + :minted (System/currentTimeMillis) :exp exp} (signature-or-throw))) @@ -298,14 +370,32 @@ (some? (:orcpub.user/dark-button-text? user)) (assoc :dark-button-text? (:orcpub.user/dark-button-text? user)))) -(defn bad-credentials-response [db username ip] +(defn base-url [{:keys [scheme headers]}] + (str (or (headers "x-forwarded-proto") (name scheme)) "://" (headers "host"))) + +(defn bad-credentials-response [db username ip request] (security/add-failed-login-attempt! username ip) (if (security/too-many-attempts-for-username? username) - (login-error errors/too-many-attempts) + (do (security/note-refusal! :login-username) + (login-error errors/too-many-attempts)) (let [user-for-username (find-user-by-username-or-email db username)] - (login-error (if (:db/id user-for-username) - errors/bad-credentials - errors/no-account))))) + ;; Several addresses failing against ONE account inside a minute is also what + ;; a person with a phone, a laptop and a tablet looks like -- so it notifies + ;; the owner rather than locking them out. Runs on another thread, and its + ;; outcome never reaches the response: failing logins must not learn a thing. + (when (and (:db/id user-for-username) + (security/multiple-ip-attempts-to-same-account? username) + (security/claim-sign-in-notice! username)) + (future + (email/send-sign-in-attempts-email + (base-url request) + {:email (:orcpub.user/email user-for-username) + :first-and-last-name (:orcpub.user/first-and-last-name user-for-username) + :user-agent (get (:headers request) "user-agent")}))) + ;; One answer for both. Telling somebody the username does not exist made + ;; the login form the same membership test the reset endpoint was, and a + ;; cheaper one, since it needs no mail to be sent. + (login-error errors/bad-credentials)))) (defn create-login-response [db conn user id & [headers]] (let [token (create-token (:orcpub.user/username user) @@ -325,6 +415,15 @@ (cond (s/blank? raw-username) (login-error errors/username-required) (s/blank? raw-password) (login-error errors/password-required) + + ;; Checked before the credentials, not on the failure path: stuffing ends on + ;; the one account it guesses right, so a check that only runs after a failed + ;; lookup would never see that attempt. Trips on five DISTINCT usernames + ;; failing from this address inside a minute -- one person on several devices can't, a spray across accounts can. + (security/multiple-account-access? remote-addr) + (do (security/note-refusal! :login-spray) + (login-error errors/too-many-attempts)) + :else (let [username (s/trim raw-username) password (s/trim raw-password) {:keys [:orcpub.user/verified? @@ -334,17 +433,14 @@ unverified? (not verified?) expired? (and verification-sent (verification-expired? verification-sent))] (cond - (nil? id) (bad-credentials-response db username remote-addr) + (nil? id) (bad-credentials-response db username remote-addr request) (and unverified? expired?) (login-error errors/unverified-expired) unverified? (login-error errors/unverified {:email email}) :else (create-login-response db conn user id)))))) -(defn login [{:keys [json-params db] :as request}] - (try - (let [resp (login-response request)] - resp) - (catch Throwable e (prn "E" e) (throw e)))) +(defn login [request] + (login-response request)) (defn user-for-email [db email] @@ -357,9 +453,6 @@ (s/lower-case email))] user)) -(defn base-url [{:keys [scheme headers]}] - (str (or (headers "x-forwarded-proto") (name scheme)) "://" (headers "host"))) - (defn send-verification-email [request params verification-key] (email/send-verification-email (base-url request) @@ -470,7 +563,43 @@ {:error :verification-email-failed} e))))))) -(defn register [{:keys [json-params db conn] :as request}] + +(defn- breach-message + "Why this reads as a strength verdict, not a security warning: the corpus + measures commonness, not a breach or an attacker targeting this person. + GOTCHA: speaks like the strength meter under the \"Too common\" badge, so it + must not repeat what the badge already said. See account-flows.md." + [_n] + "A few words strung together are harder to guess and easier to remember.") + +(def ^:private breach-refusal-threshold + "How many corpus appearances make a password common enough to refuse. + GOTCHA: below this line it's advisory only, surfaced by the strength meter + while typing; at or above, refusal at submit is reserved for the egregious + case -- one appearance is an old leak, four figures ships in every cracking + wordlist. See account-flows.md." + 1000) + +(defn- breach-errors + "A validation map for an egregiously common password, or nil. + + Only a positive answer counts, and only one at or above the threshold: an + unreachable service must read as no objection, and nor must a handful of + appearances." + [password] + (let [result (pwned/check password)] + (when (and (number? result) (>= result breach-refusal-threshold)) + ;; Its OWN key, not :password: a strength verdict, not a rule fault, so it + ;; belongs to the meter -- the only thing on the page judging password quality. + ;; Keyed with the rule faults instead, it would render as a red field error + ;; sitting right above a meter reporting UNCOMMON in green: two disagreeing verdicts. + {:password-common [(breach-message result)]}))) + +(def ^:private registration-throttled-message + (str "Too many accounts have been created from this connection in the last hour. " + "Try again a little later, or email us if you are stuck.")) + +(defn register [{:keys [json-params db conn remote-addr] :as request}] (let [{:keys [username email password send-updates?]} json-params username (when username (s/trim username)) email (when email (s/lower-case (s/trim email))) @@ -479,25 +608,35 @@ json-params (seq (d/q email-query db email)) (seq (d/q username-query db username))) - now (java.util.Date.)] - (try - (if (seq validation) - {:status 400 - :body validation} - (do-verification - request - json-params - conn - (merge - {:orcpub.user/email email - :orcpub.user/username username - :orcpub.user/password (hashers/encrypt password) - :orcpub.user/send-updates? send-updates? - :orcpub.user/created now} - (when auth/record-last-login-at-registration? - {:orcpub.user/last-login now}) - (user-data/registration-defaults)))) - (catch Throwable e (prn e) (throw e))))) + now (java.util.Date.) + ;; Checked only once the form is otherwise valid: no reason to ask a third + ;; party about a password attached to a malformed signup. + validation (if (seq validation) + validation + (or (breach-errors password) validation)) + ;; Checked last, so a form that was going to be rejected anyway is not + ;; counted against the host. :general rather than a field key: nobody + ;; can edit their way past a rate limit, so it must not sit in the map + ;; that disables the button. + validation (if (or (seq validation) (security/registration-allowed? remote-addr)) + validation + (assoc validation :general [registration-throttled-message]))] + (if (seq validation) + {:status 400 + :body validation} + (do-verification + request + json-params + conn + (merge + {:orcpub.user/email email + :orcpub.user/username username + :orcpub.user/password (hashers/encrypt password) + :orcpub.user/send-updates? send-updates? + :orcpub.user/created now} + (when auth/record-last-login-at-registration? + {:orcpub.user/last-login now}) + (user-data/registration-defaults)))))) (def user-for-verification-key-query '[:find ?e @@ -633,13 +772,29 @@ (assoc :dark-button-text? (:orcpub.user/dark-button-text? updated-user)))})) {:status 400 :body {:error "User not found"}}))) +(defn hash-reset-key + "What goes in the database. The emailed key is the secret; storing it verbatim + made anyone who can read the user table able to complete a reset on any + account with one outstanding. Only the digest is kept, so a stolen table + yields nothing that can be mailed back in." + [key] + (->> (.getBytes ^String key "UTF-8") + (.digest (java.security.MessageDigest/getInstance "SHA-256")) + (map #(format "%02x" %)) + (apply str))) + +;; Two hours. One is the usual choice and is a one-line change here; two leaves +;; room for mail that takes a while to arrive and for somebody who reads it on +;; the way home, on a site whose worst case is a character sheet. +(def password-reset-valid-hours 2) + (defn do-send-password-reset [user-id email conn request] (let [key (str (java.util.UUID/randomUUID))] (try @(d/transact conn [{:db/id user-id - :orcpub.user/password-reset-key key + :orcpub.user/password-reset-key (hash-reset-key key) :orcpub.user/password-reset-sent (java.util.Date.)}]) (email/send-reset-email (base-url request) @@ -655,49 +810,121 @@ e)))))) (defn password-reset-expired? [password-reset-sent] - (and password-reset-sent (before? (instant password-reset-sent) (-> 24 hours ago)))) + (and password-reset-sent + (before? (instant password-reset-sent) (-> password-reset-valid-hours hours ago)))) (defn password-already-reset? [password-reset password-reset-sent] (and password-reset (before? (instant password-reset-sent) (instant password-reset)))) -(defn send-password-reset [{:keys [query-params db conn scheme headers] :as request}] - (try - (let [email (:email query-params) - {:keys [:orcpub.user/password-reset-sent - :orcpub.user/password-reset - :db/id] :as user} (user-for-email db email) - expired? (password-reset-expired? password-reset-sent) - already-reset? (password-already-reset? password-reset password-reset-sent)] - (if id +(defn send-password-reset [{:keys [query-params db conn remote-addr] :as request}] + (let [email (:email query-params) + {:keys [:db/id]} (user-for-email db email)] + ;; The answer is identical whether or not that address has an account: + ;; confirmed-valid addresses are precisely what a stuffing run needs. + ;; The per-address limit is checked AFTER the lookup but never changes the + ;; response -- a limit that visibly altered it would itself be a membership oracle, which the uniform 200 closes. + (when (and id (security/reset-email-allowed? email remote-addr)) + (try (do-send-password-reset id email conn request) - {:status 400 :body {:error :no-account}})) - (catch Throwable e (prn e) (throw e)))) + (catch Exception e + ;; Swallowed on purpose. Letting this escape would restore the oracle + ;; in a subtler form -- only a real account can fail to be emailed, + ;; so an error response would mark the address as registered. + (println "ERROR: password reset for a known address could not be sent:" + (.getMessage e))))) + {:status 200})) + +(def reset-link-spent-message + "This reset link has expired or has already been used. Request a new one from the login page.") -(defn do-password-reset [conn user-id password] +(defn do-password-reset [conn user-id password & [request user]] (try @(d/transact conn - [{:db/id user-id - :orcpub.user/password (hashers/encrypt (s/trim password)) - :orcpub.user/password-reset (java.util.Date.) - :orcpub.user/verified? true}]) + (cond-> [{:db/id user-id + :orcpub.user/password (hashers/encrypt (s/trim password)) + :orcpub.user/verified? true} + ;; Conditional on the reset this request read (nil: none yet), so of two submits of + ;; one link racing, the second fails instead of also setting a password. + [:db/cas user-id :orcpub.user/password-reset + (:orcpub.user/password-reset user) (java.util.Date.)]] + ;; The link is spent with the password it set, in the same transaction. + (:orcpub.user/password-reset-key user) + (conj [:db/retract user-id :orcpub.user/password-reset-key + (:orcpub.user/password-reset-key user)]))) + ;; Before anything else that can fail: every token minted before now is no + ;; longer good for this account, including the one this very request is + ;; using. That is the point -- a reset is how somebody takes their account + ;; back, and leaving the intruder's session alive would defeat it. + (when-let [username (:orcpub.user/username user)] + (security/note-password-changed! username)) + ;; After the transact, on another thread, and its outcome never reaches the + ;; response: the password HAS changed by now, and a mail failure must not + ;; report an error for something that already happened -- somebody would + ;; simply do it again. + (when (and request (:orcpub.user/email user)) + (future + (email/send-password-changed-email + (base-url request) + {:email (:orcpub.user/email user) + :first-and-last-name (:orcpub.user/first-and-last-name user) + :user-agent (get (:headers request) "user-agent")}))) {:status 200} (catch Exception e - (println "ERROR: Failed to reset password for user" user-id ":" (.getMessage e)) - (throw (ex-info "Unable to reset password. Please try again or contact support." - {:error :password-update-failed - :user-id user-id} - e))))) - -(defn reset-password [{:keys [json-params db conn cookies identity] :as request}] + (if (odb/cas-failed? e) + {:status 400 :body {:general [reset-link-spent-message]}} + (do (println "ERROR: Failed to reset password for user" user-id ":" (.getMessage e)) + (throw (ex-info "Unable to reset password. Please try again or contact support." + {:error :password-update-failed + :user-id user-id} + e))))))) + +(declare user-by-password-reset-key-query) + +(defn reset-password + "Set a new password with the key from a reset link. The key is the only authority: opening + the link grants no session, and the key is checked here, at submit, then retired." + [{:keys [json-params db conn] :as request}] (try (let [{:keys [password verify-password]} json-params - username (:user identity) - {:keys [:db/id] :as user} (first-user-by db username-query username)] + ;; Trimmed before anything judges it, because do-password-reset stores it + ;; trimmed and the corpus lookup hashes it: an untrimmed check asks about + ;; a different string from the one saved. + password (some-> password s/trim) + verify-password (some-> verify-password s/trim) + reset-key (:key json-params) + {:keys [:db/id :orcpub.user/username :orcpub.user/email + :orcpub.user/password-reset-sent :orcpub.user/password-reset] :as user} + (when-not (s/blank? reset-key) + (first-user-by db user-by-password-reset-key-query (hash-reset-key reset-key))) + usable? (and id + (not (password-reset-expired? password-reset-sent)) + (not (password-already-reset? password-reset password-reset-sent))) + ;; The username is known here, even though this page never shows it, + ;; so a password that IS the username can be refused the way + ;; registration already refuses one. Without the context this was the + ;; one rule the two paths did not share. + context {:username username :email email} + rule-errors (registration/validate-password password context) + ;; Asked once. Only reached when the password is otherwise acceptable, + ;; so a rejected reset never costs a call. + breached (when (and usable? (= password verify-password) (empty? rule-errors)) + (first (:password-common (breach-errors password))))] + ;; Field-keyed, in :body, the same shape registration answers with. + ;; :message is not a Pedestal response key: these came back as a 400 with + ;; an EMPTY body, so the reasons never left the server and the page had + ;; nothing to show but a generic apology. (cond - (not= password verify-password) {:status 400 :message "Passwords do not match"} - (seq (registration/validate-password password)) {:status 400 :message "New password is invalid"} - :else (do-password-reset conn id password))) + ;; Missing, unknown, expired and used are one answer: this link no longer works. + (not usable?) + {:status 400 :body {:general [reset-link-spent-message]}} + + (not= password verify-password) + {:status 400 :body {:verify-password ["Passwords do not match"]}} + + (seq rule-errors) {:status 400 :body rule-errors} + breached {:status 400 :body {:password-common [breached]}} + :else (do-password-reset conn id password request user))) (catch Throwable t (prn t) (throw t)))) (def font-sizes @@ -918,19 +1145,45 @@ (swap! probed-images #(-> % (prune-probes now) (assoc url {:at now :outcome outcome}))) outcome)))) +(def ^:private probe-max-concurrency + "Outbound fetches the probe endpoint may have running at once. This endpoint + needs no login and answers one URL per request, so without a bound a caller + can hand it many distinct not-yet-cached addresses and park a Jetty worker + on someone else's slow host per request. Small on purpose: this is one + request's worth of curiosity about one picture, not sheet generation." + 8) + +(def ^:private probe-slots + (delay (java.util.concurrent.Semaphore. probe-max-concurrency true))) + (defn image-probe - "Whether this server can fetch the picture at the posted `:url`, asked by the builder before - it exports a picture the browser could not read. Answers 200 with a reason name: ok, - blocked-address, the fetch's failure reason, or unknown. The fetched bytes are kept for the - export that follows. - GOTCHA: it needs no login, so it must never return the picture; that would make it an - open proxy." - [{:keys [transit-params]}] + "Whether this server can fetch the picture at the posted `:url`, asked by the builder before it exports one the browser couldn't read. + Answers 200 with a reason: ok, blocked-address, the fetch's failure reason, unknown, or rate-limited; fetched bytes are kept for the export that follows. + A cache hit always answers; a miss is gated by a per-host hourly limit and the concurrency bound, since misses are what let an unauthenticated caller park Jetty workers on hosts of their choosing -- both refusals surface as rate-limited. + + GOTCHA: no login is required, so this must never return the picture itself; that would make it an open proxy." + [{:keys [transit-params remote-addr]}] (let [url (:url transit-params) - reason (if-not (well-formed-image-url? url) + cached (get @probed-images url) + reason (cond + (not (well-formed-image-url? url)) :blocked-address - (let [{:keys [image reason]} (probed-outcome url)] - (if image :ok (or reason :unknown))))] + + cached + (let [{:keys [image reason]} (:outcome cached)] + (if image :ok (or reason :unknown))) + + (not (security/image-probe-allowed? remote-addr)) + :rate-limited + + (not (.tryAcquire ^java.util.concurrent.Semaphore @probe-slots)) + :rate-limited + + :else + (try + (let [{:keys [image reason]} (probed-outcome url)] + (if image :ok (or reason :unknown))) + (finally (.release ^java.util.concurrent.Semaphore @probe-slots))))] ;; The HOST only, never the URL: an image address can carry a signed query ;; string. This is how the genuinely unreachable set gets measured rather than ;; guessed at. @@ -1267,14 +1520,15 @@ (merge response {:status 200 - :headers {"Content-Type" "text/html" } + :headers (merge {"Content-Type" "text/html"} (:headers response)) :body (index-page {:url (str "http://" host uri) :title (or title default-title) :description (or description default-description) :image (or image-url (default-image-url host)) - :nonce csp-nonce} + :nonce csp-nonce + :spinner-kind (spinner/pick (get-in request [:query-params :spinner]))} (= "/" uri))}))) (defn default-index-page [request & [response]] @@ -1283,23 +1537,32 @@ (defn index [{:keys [headers scheme uri server-name] :as request} & [response]] (default-index-page request response)) -(defn reset-password-page [{:keys [query-params db conn] :as req}] - (if-let [key (:key query-params)] - (let [{:keys [:db/id - :orcpub.user/username - :orcpub.user/password-reset-key - :orcpub.user/password-reset-sent - :orcpub.user/password-reset] :as user} - (first-user-by db user-by-password-reset-key-query key) - expired? (password-reset-expired? password-reset-sent) - already-reset? (password-already-reset? password-reset password-reset-sent)] - (cond - expired? (redirect route-map/password-reset-expired-route) - already-reset? (redirect route-map/password-reset-used-route) - :else (let [token (create-token username (-> 1 hours from-now))] - (index req {:cookies {"token" token}})))) - {:status 400 - :body "Key is required"})) +(defn reset-password-page [{:keys [query-params db] :as req}] + (let [key (:key query-params) + {:keys [:db/id + :orcpub.user/username + :orcpub.user/password-reset-sent + :orcpub.user/password-reset]} + ;; The link carries the key; the table holds only its digest. + (when-not (s/blank? key) + (first-user-by db user-by-password-reset-key-query (hash-reset-key key))) + expired? (password-reset-expired? password-reset-sent) + already-reset? (password-already-reset? password-reset password-reset-sent)] + (cond + ;; Covers both no key at all and a key matching nobody, so neither bare-strings + ;; a "Key is required" error nor falls through to :else and signs a token for + ;; a nil username. A mangled link and an expired one are the same event to the + ;; reader -- it doesn't work -- so both land on the page offering to send a new one. + (nil? id) (redirect route-map/password-reset-expired-route) + expired? (redirect route-map/password-reset-expired-route) + already-reset? (redirect route-map/password-reset-used-route) + ;; No session: the key in the URL is what the form submits. The username cookie grants + ;; nothing; it lets the meter flag a password equal to it. no-referrer keeps the key out of + ;; the Referer of anything this page loads. + :else (index req {:headers {"Referrer-Policy" "no-referrer"} + :cookies {"reset-username" {:value username :path "/" + :max-age (* password-reset-valid-hours 3600) + :same-site :strict}}})))) (defn check-field [query value db] {:status 200 @@ -1515,9 +1778,12 @@ {:status 200 :body result}))) -(defn get-item [{:keys [db] {:keys [:id]} :path-params}] +(defn get-item + "A custom item, for its owner only. Anyone else gets the 404 a missing item gets, so an id + says nothing about whether it exists." + [{:keys [db username] {:keys [:id]} :path-params}] (let [item (d/pull db '[*] id)] - (if (::mi5e/owner item) + (if (and (::mi5e/owner item) (= username (::mi5e/owner item))) {:status 200 :body item} {:status 404}))) @@ -1630,17 +1896,46 @@ (errors/with-db-error-handling :character-deletion-failed {:character-id parsed-id} "Unable to delete character. Please try again or contact support." - @(d/transact conn [[:db/retractEntity parsed-id]]) + @(d/transact conn (cons [:db/retractEntity parsed-id] + (share/retractions-for-character db parsed-id))) {:status 200}) {:status 400 :body problems}) {:status 401 :body "You do not own this character"}))) +(defn- public-owner + "The owner as a character read shows it: the username, or nil. For nine days in May 2017 the + login token carried whatever the user typed (e3cdf5f to cf170fc), so characters saved then name + their owner by email address; this read needs no login, so the address must not go out." + [db owner] + (if-let [user (find-user-by-username-or-email db owner)] + (:orcpub.user/username user) + (when-not (s/includes? owner "@") owner))) + +(defn- equipped-custom-items + "The owner's custom items this character has equipped, without database ids or owner. Whoever may + read the character sees these, because the sheet is drawn from them; the owner's other items stay + out. Any failure sends none rather than failing the character read." + [db username character] + (try + (sb/used-custom-items (entity/from-strict character) + (d/q '[:find [(pull ?e [*]) ...] :in $ ?owner :where [?e ::mi5e/owner ?owner]] + db username) + #(mi5e/expand-magic-items [%])) + (catch Exception e + (println "WARNING: custom items not attached to character" (:db/id character) ":" (.getMessage e)) + nil))) + (defn get-character-for-id [db id] (let [{:keys [::se/owner] :as character} (d/pull db '[*] id) problems [] #_(dnd-e5-char-type-problems character)] (if (or (not owner) (seq problems)) {:status 400 :body problems} - {:status 200 :body character}))) + (let [shown (public-owner db owner) + items (when shown (equipped-custom-items db shown character))] + {:status 200 :body (cond-> (if shown + (assoc character ::se/owner shown) + (dissoc character ::se/owner)) + (seq items) (assoc ::char5e/custom-items items))})))) (defn character-summary-for-id [db id] ;; Fixed: bare destructuring outside let silently returned nil @@ -1727,6 +2022,7 @@ (try ;; Client sends {:new-email "..."} (confirm-email is validated client-side only) (let [new-email (s/lower-case (s/trim (str (:new-email transit-params)))) + current-password (:current-password transit-params) username (:user identity)] (if (nil? username) {:status 400 :body {:error :user-not-found}} @@ -1738,6 +2034,13 @@ (nil? id) {:status 400 :body {:error :user-not-found}} + ;; Re-authentication: moving an account to another address takes it away + ;; from whoever holds this one, requiring more than just a session -- a + ;; borrowed or forgotten session alone must not be enough to walk off with + ;; the account. The password is compared, never read: lookup-user does the hash check. + (nil? (:db/id (lookup-user db username (str current-password)))) + {:status 400 :body {:error :bad-credentials}} + (registration/bad-email? new-email) {:status 400 :body {:error :invalid-email}} @@ -1780,6 +2083,16 @@ (send-email-change-verification request {:email new-email :username username} verification-key) + ;; Tells the address that currently owns the account. Verification goes to + ;; the NEW address, unreachable by the owner if this wasn't them, so without + ;; this notice the losing party is never told. Runs on another thread: the + ;; change is already accepted, so a failure here must not report an error. + (future + (email/send-email-change-notice + (base-url request) + {:email email + :first-and-last-name (:orcpub.user/first-and-last-name user) + :new-email new-email})) {:status 200 :body {:pending-email new-email}} (catch Throwable e (errors/log-error "ERROR:" (str "Email send failed, rolling back pending state: " (.getMessage e))) @@ -1838,6 +2151,11 @@ [route-map/login-page-route] [route-map/verify-sent-route] [route-map/password-reset-sent-route] + ;; Was the only one of these missing. The client routes here after a reset, + ;; so it renders inside a session -- but a refresh, a back button or a + ;; bookmark asked the server for it and got "Not Found", on the one page + ;; whose whole job is to confirm the password was changed. + [route-map/password-reset-success-route] [route-map/password-reset-expired-route] [route-map/password-reset-used-route] [route-map/verify-failed-route] @@ -1941,6 +2259,8 @@ :delete `delete-user}] [(route-map/path-for route-map/user-email-route) ^:interceptors [check-auth] {:put `request-email-change}] + [(route-map/path-for route-map/user-sessions-route) ^:interceptors [check-auth] + {:delete `withdraw-sessions}] [(route-map/path-for route-map/follow-user-route :user ":user") ^:interceptors [check-auth] {:post `follow-user :delete `unfollow-user}] @@ -1950,9 +2270,8 @@ {:post `save-item :get `item-list}] [(route-map/path-for route-map/dnd-e5-item-route :id ":id") ^:interceptors [check-auth parse-id] - {:delete `delete-item}] - [(route-map/path-for route-map/dnd-e5-item-route :id ":id") ^:interceptors [parse-id] - {:get `get-item}] + {:get `get-item + :delete `delete-item}] ;; Characters [(route-map/path-for route-map/dnd-e5-char-list-route) ^:interceptors [check-auth] @@ -1966,6 +2285,15 @@ {:delete `delete-character}] [(route-map/path-for route-map/dnd-e5-char-route :id ":id") {:get `get-character}] + [(route-map/path-for route-map/dnd-e5-char-share-route :id ":id" :token ":token") ^:interceptors [parse-id] + {:get `share/get-share}] + [(route-map/path-for route-map/dnd-e5-char-share-route :id ":id" :token ":token") ^:interceptors [check-auth parse-id] + {:put `share/put-share}] + [(route-map/path-for route-map/dnd-e5-char-share-token-route :id ":id") ^:interceptors [check-auth parse-id] + {:get `share/get-token + :put `share/create-token + :post `share/new-token + :delete `share/stop-sharing}] [(route-map/path-for route-map/dnd-e5-char-page-route :id ":id") ^:interceptors [parse-id] {:get `character-page}] @@ -2003,7 +2331,7 @@ {:get `re-verify}] [(route-map/path-for route-map/unsubscribe-route) {:get `unsubscribe}] - [(route-map/path-for route-map/reset-password-route) ^:interceptors [ring/cookies check-auth] + [(route-map/path-for route-map/reset-password-route) ^:interceptors [ring/cookies] {:post `reset-password}] [(route-map/path-for route-map/reset-password-page-route) ^:interceptors [ring/cookies] {:get `reset-password-page}] diff --git a/src/clj/orcpub/routes/party.clj b/src/clj/orcpub/routes/party.clj index 4f7c5781b..200523069 100644 --- a/src/clj/orcpub/routes/party.clj +++ b/src/clj/orcpub/routes/party.clj @@ -6,22 +6,45 @@ [datomic.api :as d] [orcpub.dnd.e5.party :as party] [orcpub.entity.strict :as se] - [orcpub.errors :as errors])) + [orcpub.errors :as errors] + [orcpub.routes.share :as share])) + +(defn- character? + "Whether id names a character. A party holds only characters: an item, a folder or another party + would load as a broken row." + [db id] + (boolean (and (integer? id) (::se/owner (d/pull db [::se/owner] id))))) + +(defn- token-entry + "The component entity remembering the share token a character was added with, or nil when the token is + not that character's current one." + [db character-id token] + (when (share/token-current? db character-id token) + {:orcpub.party-share/character character-id :orcpub.party-share/token token})) (defn create-party "Creates a party from :transit-params, owned by the authenticated user. Returns 200 with the created party. Throws ExceptionInfo :party-creation-failed on database failure." [{:keys [db conn identity] party :transit-params}] - (let [username (:user identity)] - (errors/with-db-error-handling :party-creation-failed - {:username username} - "Unable to create party. Please try again or contact support." - (let [result @(d/transact conn [(assoc party ::party/owner username)]) - new-id (-> result :tempids first val)] - {:status 200 :body (d/pull (d/db conn) '[*] new-id)})))) + (let [username (:user identity) + ids (::party/character-ids party) + tokens (keep (fn [{:keys [:orcpub.party-share/character :orcpub.party-share/token]}] + (when (contains? (set ids) character) (token-entry db character token))) + (::party/shared-tokens party))] + (if-not (every? #(character? db %) ids) + {:status 400 :body {:error :not-a-character}} + (errors/with-db-error-handling :party-creation-failed + {:username username} + "Unable to create party. Please try again or contact support." + (let [result @(d/transact conn [(cond-> (-> (select-keys party [::party/name ::party/character-ids]) + (assoc ::party/owner username :db/id "new-party")) + (seq tokens) (assoc ::party/shared-tokens tokens))]) + new-id (get-in result [:tempids "new-party"])] + {:status 200 :body (d/pull (d/db conn) '[*] new-id)}))))) -(def pull-party [:db/id ::party/name {::party/character-ids [:db/id ::se/owner ::se/summary]}]) +(def pull-party [:db/id ::party/name {::party/character-ids [:db/id ::se/owner ::se/summary]} + {::party/shared-tokens [:orcpub.party-share/character :orcpub.party-share/token]}]) (defn parties [{:keys [db identity]}] (let [username (:user identity) @@ -32,16 +55,23 @@ username) mapped (map (fn [[party]] - (update - party - ::party/character-ids - (fn [chars] - (map - (fn [{:keys [:db/id ::se/owner ::se/summary]}] - (assoc summary - :db/id id - ::se/owner owner)) - chars)))) + ;; Only tokens that still work: a revoked or pruned share leaves the row without homebrew. + (let [tokens (into {} + (keep (fn [{:keys [:orcpub.party-share/character :orcpub.party-share/token]}] + (when (share/token-current? db character token) [character token]))) + (::party/shared-tokens party))] + (-> party + (dissoc ::party/shared-tokens) + (update + ::party/character-ids + (fn [chars] + (map + (fn [{:keys [:db/id ::se/owner ::se/summary]}] + (cond-> (assoc summary + :db/id id + ::se/owner owner) + (tokens id) (assoc :orcpub.party-share/token (tokens id)))) + chars)))))) result)] {:status 200 :body mapped})) @@ -61,20 +91,32 @@ {:status 200 :body (d/pull (d/db conn) pull-party id)})) -(defn add-character [{:keys [db conn identity] - character-id :transit-params - {:keys [id]} :path-params}] - (try - (let [{:keys [db-after]} @(d/transact conn [{:db/id id - ::party/character-ids character-id}])] - {:status 200 :body (d/pull db-after '[*] id)}) +(defn add-character + "Adds a character to a party. The body is the character id, or {:character-id id :share-token token} + when the character was opened from a share link; a current token is kept with the entry so the party + page can load that character's homebrew. Anything that is not a character is refused." + [{:keys [db conn identity] + params :transit-params + {:keys [id]} :path-params}] + (let [[character-id token] (if (map? params) [(:character-id params) (:share-token params)] [params nil]) + previous (d/q '[:find [?t ...] :in $ ?party ?c + :where [?party ::party/shared-tokens ?t] [?t :orcpub.party-share/character ?c]] + db id character-id) + entry (token-entry db character-id token)] + (if-not (character? db character-id) + {:status 400 :body {:error :not-a-character}} + (try + @(d/transact conn (concat (when entry (map (fn [t] [:db/retractEntity t]) previous)) + [(cond-> {:db/id id ::party/character-ids character-id} + entry (assoc ::party/shared-tokens [entry]))])) + {:status 200 :body (d/pull (d/db conn) '[*] id)} (catch Exception e (println "ERROR: Failed to add character" character-id "to party" id ":" (.getMessage e)) (throw (ex-info "Unable to add character to party. Please try again or contact support." {:error :party-add-character-failed :party-id id :character-id character-id} - e))))) + e))))))) (defn remove-character "Removes path-param :character-id from the party at :id. @@ -89,8 +131,12 @@ (errors/with-db-error-handling :party-remove-character-failed {:party-id id :character-id char-id} "Unable to remove character from party. Please try again or contact support." - (let [{:keys [db-after]} @(d/transact conn [[:db/retract id ::party/character-ids char-id]])] - {:status 200 :body (d/pull db-after '[*] id)})))) + @(d/transact conn (cons [:db/retract id ::party/character-ids char-id] + (map (fn [t] [:db/retractEntity t]) + (d/q '[:find [?t ...] :in $ ?party ?c + :where [?party ::party/shared-tokens ?t] [?t :orcpub.party-share/character ?c]] + db id char-id)))) + {:status 200 :body (d/pull (d/db conn) '[*] id)}))) (defn delete-party [{:keys [db conn identity] {:keys [id]} :path-params}] diff --git a/src/clj/orcpub/routes/share.clj b/src/clj/orcpub/routes/share.clj new file mode 100644 index 000000000..055afd381 --- /dev/null +++ b/src/clj/orcpub/routes/share.clj @@ -0,0 +1,278 @@ +;; Shared homebrew: what a character's short share link loads. +(ns orcpub.routes.share + "A character's owner shares its homebrew by link, /characters/#s=; + one copy per character, replaced on re-share, visible/replaceable only by the owner. + GOTCHA: an upload must unpack under a size cap and be exactly homebrew + (share-bundle/whitelist-shared); limits come from config (ORCPUB_SHARE_MAX_*_KB). + Idle past ORCPUB_SHARE_PRUNE_DAYS is pruned hourly (token+data, never the character). + See share-links.md." + (:require [clojure.edn :as edn] + [datomic.api :as d] + [orcpub.config :as config] + [orcpub.datomic :as odb] + [orcpub.heartbeat :as heartbeat] + [orcpub.dnd.e5.share-bundle :as sb] + [orcpub.entity.strict :as se]) + (:import [java.security MessageDigest SecureRandom] + [java.util Arrays Base64] + [java.util.zip GZIPInputStream] + [java.io ByteArrayInputStream InputStream])) + +;; The caps, from config; the measurements behind their defaults sit with the getters. +(defn max-upload-bytes [] (* 1024 (config/get-share-max-upload-kb))) +(defn max-text-bytes [] (* 1024 (config/get-share-max-text-kb))) +(defn max-bytes-per-owner [] (* 1024 (config/get-share-max-account-kb))) + +(defn- cap-headers [] + {"X-Share-Max-Upload-Bytes" (str (max-upload-bytes)) + "X-Share-Max-Text-Bytes" (str (max-text-bytes))}) + +(defn- read-capped + "Up to cap bytes from in, or ::too-large when there are more. Reads at most one byte past the cap." + [^InputStream in cap] + (let [buf (byte-array (inc cap)) + n (loop [off 0] + (if (= off (alength buf)) + off + (let [got (.read in buf off (- (alength buf) off))] + (if (neg? got) off (recur (+ off got))))))] + (if (> n cap) ::too-large (Arrays/copyOf buf (int n))))) + +(defn- digest ^String [^bytes b] + (.encodeToString (.withoutPadding (Base64/getUrlEncoder)) (.digest (MessageDigest/getInstance "SHA-256") b))) + +(defn- upload-problem + "Why a compressed upload cannot be stored, or nil when it can: ::too-large unpacked, ::unreadable as + data, ::empty of homebrew, or ::not-homebrew when the whitelist would drop or change any of it." + [^bytes upload] + (let [text (try (with-open [z (GZIPInputStream. (ByteArrayInputStream. upload))] + (read-capped z (max-text-bytes))) + (catch Exception _ ::unreadable))] + (if (keyword? text) + text + (let [data (try (edn/read-string (String. ^bytes text "UTF-8")) (catch Exception _ ::unreadable)) + kept (when-not (= ::unreadable data) (:plugins (sb/whitelist-shared data)))] + (cond + (= ::unreadable data) ::unreadable + (empty? kept) ::empty + (not= data kept) ::not-homebrew + :else nil))))) + +(defn- owns-character? + "Whether username owns the character. A character saved in May 2017 may name its owner by email." + [db username character-id] + (boolean + (when username + (let [owner (::se/owner (d/pull db [::se/owner] character-id)) + email (d/q '[:find ?email . :in $ ?username + :where [?u :orcpub.user/username ?username] [?u :orcpub.user/email ?email]] + db username)] + (and owner (or (= owner username) (and email (= owner email)))))))) + +(defn- share-of [db character-id] + (when-let [e (d/q '[:find ?e . :in $ ?c :where [?e :orcpub.share/character ?c]] db character-id)] + (d/pull db [:db/id :orcpub.share/token :orcpub.share/bundle :orcpub.share/digest :orcpub.share/size + :orcpub.share/used] e))) + +(defn now [] (java.util.Date.)) + +(def ^:private day-ms (* 24 60 60 1000)) + +(defn- touch! + "Records use of a share, at most once a day." + [conn share] + (let [at (now) + used ^java.util.Date (:orcpub.share/used share)] + (when (or (nil? used) (> (- (.getTime at) (.getTime used)) day-ms)) + @(d/transact conn [{:db/id (:db/id share) :orcpub.share/used at}])))) + +(defn token-current? + "Whether token is the character's current share token." + [db character-id token] + (let [share (share-of db character-id)] + (boolean (and share (string? token) (= token (:orcpub.share/token share)))))) + +(defn- expiry-of + "When the character's last share link expired unused, while the owner has neither shared again nor + dismissed the note." + [db character-id] + (d/q '[:find ?on . :in $ ?c :where [?e :orcpub.share-expiry/character ?c] [?e :orcpub.share-expiry/on ?on]] + db character-id)) + +(defn- expiry-retractions [db character-id] + (map (fn [e] [:db/retractEntity e]) + (d/q '[:find [?e ...] :in $ ?c :where [?e :orcpub.share-expiry/character ?c]] db character-id))) + +(defn- party-token-retractions + "The tokens parties saved for the character, which load nothing once its share is gone. The parties + keep the character." + [db character-id] + (map (fn [e] [:db/retractEntity e]) + (d/q '[:find [?t ...] :in $ ?c :where [?t :orcpub.party-share/character ?c]] db character-id))) + +(defn prune! + "Expires the shares unused for longer than the prune window: each is deleted, leaving a note of its + character and the date for the owner's page, and a note still there after another window is deleted + too. A day more is allowed because use is recorded at most daily, a share with no recorded use counts + from when its record was made, and time the server was off does not count. Only share records, notes + and the tokens parties saved go, never a character or its place in a party. Returns + {:expired n :forgotten n}." + [conn] + (let [db (d/db conn) + at (now) + days (config/get-share-prune-days) + outages (heartbeat/outages db) + unused? (fn [since] (> (heartbeat/running-ms outages since at) (* (inc days) day-ms))) + stale (when (pos? days) + (vec (for [[e c made] (d/q '[:find ?e ?c ?made + :where [?e :orcpub.share/character ?c ?tx] [?tx :db/txInstant ?made]] + db) + :when (unused? (or (:orcpub.share/used (d/entity db e)) made))] + [e c]))) + expiring (set (map second stale)) + forgotten (when (pos? days) + (vec (for [[e c on] (d/q '[:find ?e ?c ?on + :where [?e :orcpub.share-expiry/character ?c] [?e :orcpub.share-expiry/on ?on]] + db) + :when (and (unused? on) (not (expiring c)))] + e)))] + (when (or (seq stale) (seq forgotten)) + @(d/transact conn (concat (mapcat (fn [[e c]] (concat [[:db/retractEntity e] + {:orcpub.share-expiry/character c :orcpub.share-expiry/on at}] + (party-token-retractions db c))) + stale) + (map (fn [e] [:db/retractEntity e]) forgotten)))) + {:expired (count stale) :forgotten (count forgotten)})) + +(defn prune-job + "prune! for the heartbeat, saying in the log what it removed." + [conn] + (let [{:keys [expired forgotten]} (prune! conn)] + (when (pos? (+ expired forgotten)) + (println "Share links:" expired "expired unused," forgotten "expiry notes nobody dismissed removed")))) + +(defn- random-token [] + (let [b (byte-array 16)] + (.nextBytes (SecureRandom.) b) + (.encodeToString (.withoutPadding (Base64/getUrlEncoder)) b))) + +(defn- text [s] {:status 200 :headers (merge {"Content-Type" "text/plain; charset=utf-8"} (cap-headers)) :body s}) + +(defn retractions-for-character + "Transaction data that deletes a character's share, any note that its last link expired, and the tokens + parties saved for it, for Stop sharing, New link, and when the character goes." + [db character-id] + (concat (map (fn [e] [:db/retractEntity e]) + (d/q '[:find [?e ...] :in $ ?c :where [?e :orcpub.share/character ?c]] db character-id)) + (expiry-retractions db character-id) + (party-token-retractions db character-id))) + +(defn get-token + "The character's share token, for its owner. Otherwise 404, or 410 with the date when the character's + last share link expired unused and the owner has neither shared again nor dismissed the note. Nothing + is made here: a character's page asks this on every view, and only Share link creates a share." + [{:keys [db conn identity] {:keys [id]} :path-params}] + (let [owner? (owns-character? db (:user identity) id) + share (when owner? (share-of db id)) + on (when (and owner? (not share)) (expiry-of db id))] + (cond + share (do (touch! conn share) + (text (:orcpub.share/token share))) + on {:status 410 :headers {"Content-Type" "text/plain; charset=utf-8"} + :body (str (.toInstant ^java.util.Date on))} + :else {:status 404}))) + +(defn create-token + "Shares the character: makes its token if it has none, for its owner, and returns it. Share link does + this; until then nothing about the character is stored. Sharing again clears the note that an earlier + link expired." + [{:keys [db conn identity] {:keys [id]} :path-params}] + (let [username (:user identity) + share (share-of db id)] + (cond + (not (owns-character? db username id)) {:status 404} + share (do (touch! conn share) + (text (:orcpub.share/token share))) + :else (do @(d/transact conn (concat [{:orcpub.share/character id :orcpub.share/owner username + :orcpub.share/token (random-token) :orcpub.share/used (now)}] + (expiry-retractions db id))) + ;; Read back: had two first requests raced, both callers get the token that stuck. + (text (:orcpub.share/token (share-of (d/db conn) id))))))) + +(defn new-token + "Replaces the character's share token and deletes its shared homebrew, so every link made before loads + nothing. For the owner only." + [{:keys [db conn identity] {:keys [id]} :path-params}] + (let [username (:user identity)] + (if-not (owns-character? db username id) + {:status 404} + (let [token (random-token)] + ;; Two transactions: deleting the record and making its replacement in one would name the same + ;; unique character twice. + @(d/transact conn (retractions-for-character db id)) + @(d/transact conn [{:orcpub.share/character id :orcpub.share/owner username :orcpub.share/token token + :orcpub.share/used (now)}]) + (text token))))) + +(defn stop-sharing + "Deletes the character's share, so every link made before loads nothing and nothing is stored until + Share link is pressed again, and the note that an earlier link expired. For the owner only." + [{:keys [db conn identity] {:keys [id]} :path-params}] + (if-not (owns-character? db (:user identity) id) + {:status 404} + (do (when-let [tx (seq (retractions-for-character db id))] + @(d/transact conn tx)) + {:status 204}))) + +(defn get-share + "The character's shared homebrew, compressed, to anyone whose link carries its current token. Only that + request counts as use: one without the token writes nothing, so it cannot keep a share from expiring." + [{:keys [db conn] {:keys [id token]} :path-params}] + (let [share (share-of db id) + stored (:orcpub.share/bundle share)] + (if (and stored (string? token) (= token (:orcpub.share/token share))) + (do (touch! conn share) + {:status 200 :headers (merge {"Content-Type" "application/octet-stream"} (cap-headers)) + :body (ByteArrayInputStream. ^bytes stored)}) + {:status 404}))) + +(defn put-share + "Replaces the homebrew a character shares, for its owner, when the path carries its current token. The + body is the compressed homebrew, stored exactly as sent once upload-problem finds nothing wrong. An + upload identical to the stored one is not checked or written again." + [{:keys [db conn identity body] {:keys [id token]} :path-params}] + (let [username (:user identity) + share (when (owns-character? db username id) (share-of db id)) + current? (and share (= token (:orcpub.share/token share)))] + (when current? (touch! conn share)) + (if-not current? + {:status 404} + (let [upload (when body (read-capped body (max-upload-bytes)))] + (cond + (= ::too-large upload) {:status 413 :body {:error :share-too-large}} + (nil? upload) {:status 400 :body {:error :share-unreadable}} + (= (digest upload) (:orcpub.share/digest share)) {:status 200 :body {:token token}} + :else + (case (upload-problem upload) + ::too-large {:status 413 :body {:error :share-too-large}} + ::unreadable {:status 400 :body {:error :share-unreadable}} + ::empty {:status 400 :body {:error :share-empty}} + ::not-homebrew {:status 400 :body {:error :share-not-homebrew}} + nil + (let [used (or (ffirst (d/q '[:find (sum ?size) :with ?e :in $ ?owner + :where [?e :orcpub.share/owner ?owner] [?e :orcpub.share/size ?size]] + db username)) + 0)] + (if (> (+ (- used (or (:orcpub.share/size share) 0)) (alength ^bytes upload)) (max-bytes-per-owner)) + {:status 413 :body {:error :share-quota}} + ;; Conditional on the token it was checked against: a New link or Stop sharing that + ;; lands first deletes this share, and the write must not land on the deleted record. + (try @(d/transact conn [[:db/cas (:db/id share) :orcpub.share/token token token] + {:db/id (:db/id share) + :orcpub.share/used (now) + :orcpub.share/bundle upload + :orcpub.share/digest (digest upload) + :orcpub.share/size (alength ^bytes upload)}]) + {:status 200 :body {:token token}} + (catch Exception e + (if (odb/cas-failed? e) {:status 404} (throw e)))))))))))) diff --git a/src/clj/orcpub/security.clj b/src/clj/orcpub/security.clj index e8798e807..850744561 100644 --- a/src/clj/orcpub/security.clj +++ b/src/clj/orcpub/security.clj @@ -1,7 +1,7 @@ (ns orcpub.security "Login attempt tracking and rate limiting. See orcpub.time for date/time utilities." - (:require [orcpub.time :as time :refer [minutes ago]])) + (:require [orcpub.time :as time :refer [minutes hours ago]])) (defn compare-dates [attempt-1 attempt-2] (compare (:date attempt-1) (:date attempt-2))) @@ -89,10 +89,231 @@ count (>= 3))) -;; NOTE unused: nothing calls this. Its sibling too-many-attempts-for-username? -;; is wired into the login handler in routes.clj; this one is not, so a fault in -;; it fails open silently and no test of the wiring would catch it. +;; Three addresses failing against ONE account inside the window. That is what a +;; credential-stuffing run looks like, and it is also what somebody with a phone, +;; a laptop and a tablet looks like -- so bad-credentials-response in routes.clj +;; uses it to TELL the account holder rather than to lock anyone out. (defn multiple-ip-attempts-to-same-account? [username] (multiple-ip-attempts-to-same-account-aux username @failed-login-attempts-by-username)) + + +;; --------------------------------------------------------------------------- +;; Windowed limits for the endpoints that cost something to serve: the two that +;; SEND MAIL spend our sending reputation on a message a stranger chose, not us. +;; Neither limit may change the response -- an endpoint that answers differently +;; once throttled is an oracle for whether a limit was reached, which on reset is the membership test already closed. +;; --------------------------------------------------------------------------- + +;; A limit nobody counts is a limit nobody can tune: numbers like ten signups +;; an hour from one household are guesses, validated only by how often they bite. +;; Steady refusals across many hosts means the number is too low and hurting real +;; people; a sudden spike on one host means it's doing its job. +(def ^:private refusals (atom {})) + +(defn note-refusal! + "Records a refusal against a limit, and WHO it was. The count alone cannot + tell one host refused twenty times from twenty hosts refused once each -- + the first is somebody hammering, the second is a spread, and only the second + suggests a pool of addresses working through the same list." + ([what] (note-refusal! what nil)) + ([what who] + (swap! refusals update what + (fn [{:keys [n who-set] :or {n 0 who-set #{}}}] + {:n (inc n) :who-set (cond-> who-set who (conj who))})) + nil)) + +(defn take-refusals! [] + (first (reset-vals! refusals {}))) + +(defn refusal-summary + "A line for the log, or nil on an hour when nothing was turned away. Each + limit reports how many refusals and across how many distinct sources, which + is the difference between one bad actor and a pool of them." + [counts] + (when (seq counts) + (str "limits: " + (->> counts + (sort-by (comp - :n val)) + (map (fn [[what {:keys [n who-set]}]] + (str (name what) " " n + (when (> (count who-set) 1) + (str " from " (count who-set) " sources"))))) + (interpose ", ") + (apply str))))) + +;; --------------------------------------------------------------------------- +;; Tokens withdrawn by a password change +;; +;; Holds the moment each account's password last moved; a token minted before +;; that is refused. IN MEMORY, not a database read, because check-auth runs on +;; every request doing pure signature verification, and only holds accounts whose password moved within the token lifetime -- older entries have already expired on their own. +;; --------------------------------------------------------------------------- + +(def ^:private password-changes (atom {})) + +(defn note-password-changed! + "Records that `username`'s password moved, withdrawing every token minted + before that instant. Defaults to now." + ([username] (note-password-changed! username (System/currentTimeMillis))) + ([username at-millis] + (swap! password-changes assoc username at-millis) + at-millis)) + +(defn token-withdrawn? + "Whether a token minted at `minted-millis` has been withdrawn for `username`. + + A token carrying no minted stamp counts as older than any change: it predates + this mechanism, and the only accounts in the map are ones whose password moved + recently enough for the question to still matter." + [username minted-millis] + (boolean + (when-let [changed-at (get @password-changes username)] + (< (or minted-millis 0) changed-at)))) + +(defn forget-password-changes-before! + "Drops entries older than `cutoff-millis`. Every token they could withdraw has + expired by then, so keeping them only grows the map." + [cutoff-millis] + (swap! password-changes + (fn [m] (into {} (remove (fn [[_ at]] (< at cutoff-millis))) m))) + nil) + +(defn absorb-password-changes! + "Folds `username->millis`, read from the database, into the map, keeping the + later instant per account, then drops entries older than `cutoff-millis`. + GOTCHA: merges rather than replaces -- the database read is a snapshot, and + a reset or sign-out-everywhere committing after the read would be dropped + by a replace, reinstating the tokens it withdrew until the next hourly + refresh. See account-flows.md." + [username->millis cutoff-millis] + (swap! password-changes + (fn [m] + (into {} + (remove (fn [[_ at]] (< at cutoff-millis))) + (merge-with max m (into {} username->millis))))) + nil) + +(defn restore-password-changes! + "Repopulates the map, for boot. + + Without this a restart quietly reinstates every token the map was holding, and + nothing would notice -- the failure is silent and looks exactly like working, + which is why it has a test of its own." + [username->millis] + (reset! password-changes (into {} username->millis)) + nil) + +(defn password-changes-snapshot + "The map as it stands. For the boot summary and for tests." + [] + @password-changes) + +(def ^:private sightings (atom {})) + +(defn- prune [m cutoff] + (into {} + (keep (fn [[k ts]] + (let [live (remove #(.isBefore % cutoff) ts)] + (when (seq live) [k (vec live)])))) + m)) + +(defn under-limit? + "True while `k` has been seen fewer than `limit` times inside `window`, and + false once it has hit the limit. The sighting is recorded in the same swap + that answers, so simultaneous callers cannot both be told yes on the last + slot. `k` is any value -- callers namespace their own, e.g. [:reset email]." + [k limit window] + (let [cutoff (ago window) + [before _] (swap-vals! sightings + (fn [m] + (let [m (prune m cutoff)] + (update m k #(conj (vec %) (time/now)))))) + seen (count (remove #(.isBefore % cutoff) (get before k))) + allowed (< seen limit)] + ;; The key's first element names the limit, so every caller is counted + ;; without having to remember to say so. + (when-not allowed + (note-refusal! (if (vector? k) (first k) k) + (when (vector? k) (second k)))) + allowed)) + +(defn claim-once? + "True the first time `k` is claimed inside `window`, false until it lapses." + [k window] + (under-limit? k 1 window)) + +;; One sign-in notice per account per day. An attacker who can trigger the +;; condition can trigger it on a loop, and without this the warning would be a +;; way to mail-bomb any account whose username is public. +(def notice-cooldown-hours 24) + +(defn claim-sign-in-notice! [username] + (claim-once? [:sign-in-notice username] (hours notice-cooldown-hours))) + +;; Three reset mails an hour to one address leaves room for "it never arrived, +;; send it again" and takes the bombing ceiling off unlimited. The per-address +;; limit is the one that protects a PERSON; the per-host limit is the one that +;; stops somebody walking a list of addresses through the same endpoint. +(def reset-per-address-hourly 3) +(def reset-per-host-hourly 10) + +(defn reset-email-allowed? [email ip] + (and (under-limit? [:reset-address email] reset-per-address-hourly (hours 1)) + (under-limit? [:reset-host ip] reset-per-host-hourly (hours 1)))) + +;; What this defends is NOT the account table -- an unverified account cannot log +;; in, so bulk signups sit inert. What signup CAN do is send mail to an address +;; the filler doesn't own, same as the reset endpoint. Sized to real use (eight +;; people at one game night, plus retries and typos), not to the attack: a bulk run wants thousands, so any sane number stops it. +(def registrations-per-host-hourly 10) + +(defn registration-allowed? [ip] + (under-limit? [:register ip] registrations-per-host-hourly (hours 1))) + +;; The probe has no login, so IP is the only handle on who is asking. A real +;; character builder fires it once per image field, well under this; a script +;; walking a URL list to keep the fetch pool occupied is what it stops. +(def image-probe-per-host-hourly 300) + +(defn image-probe-allowed? [ip] + (under-limit? [:image-probe-host ip] image-probe-per-host-hourly (hours 1))) + +(defn busiest + "The heaviest single user of each limit in the window, whether or not it was + ever refused. A host that stops at ten signups an hour, every hour, never + trips anything and so never appears in the refusals at all -- this is the + only place it shows up." + [] + (->> @sightings + (reduce (fn [acc [k ts]] + (let [limit (if (vector? k) (first k) k)] + (update acc limit (fnil max 0) (count ts)))) + {}))) + +(defn busiest-summary + "A line naming any limit whose heaviest user is at least at `floor` of it -- + the quiet version of abuse, which sits just inside every number it is given." + [highs floor] + (let [caps {:register registrations-per-host-hourly + :reset-address reset-per-address-hourly + :reset-host reset-per-host-hourly + :image-probe-host image-probe-per-host-hourly} + near (for [[what n] highs + :let [cap (get caps what)] + :when (and cap (>= n (* floor cap)))] + (str (name what) " " n "/" cap))] + (when (seq near) + (str "limits, busiest source: " (apply str (interpose ", " near)))))) + +(defn summary-job + "Heartbeat job: at most two lines an hour, and silent when there is nothing + to say. The first is what was turned away and how widely; the second is the + heaviest user of any limit that is sitting at 80% or more of it." + [_conn] + (let [highs (busiest)] + (when-let [line (refusal-summary (take-refusals!))] + (println line)) + (when-let [line (busiest-summary highs 0.8)] + (println line)))) diff --git a/src/clj/orcpub/styles/core.clj b/src/clj/orcpub/styles/core.clj index cec5a3c12..46de0366c 100644 --- a/src/clj/orcpub/styles/core.clj +++ b/src/clj/orcpub/styles/core.clj @@ -17,6 +17,13 @@ (def green "#70a800") (def cyan "#47eaf8") ; import log, conflict rename option (def purple "#8b7ec8") ; conflict skip option +;; Field-error red (GOV.UK's, not the site's own wine-toned red): 4.86:1 as text on the white +;; card, 4.52:1 as a border on its own tint -- never run as text ON that tint, which only clears 4.52:1 (AA-scrape). +;; Ink/muted on the white auth card: 8.9:1 and 4.6:1 respectively (counterparts to muted-on-dark). +(def text-color-light "#4b545e") +(def muted-on-light "#6b7681") +(def error-red "#d4351c") +(def error-tint "#fdf5f6") (def warning-yellow "#ffd21a") ; attention severity: unresolved conflicts, missing fields (def broken-red "#e5637a") ; broken severity: invalid / unexportable data @@ -55,7 +62,14 @@ (px-prop :margin-top :m-t - (concat (range 0 10) [21] (range 10 30 5)))) + ;; 100 drops a success message clear of the registration panel's heading. + (concat (range 0 10) [21] (range 10 30 5) [100]))) + +(def min-widths + (px-prop + :min-width + :min-w + [53 120 160])) (def widths (px-prop @@ -120,6 +134,10 @@ [:.flex-basis-50-p {:flex-basis "50%"}] + ;; Space between the lines of a wrapping row whose buttons carry no vertical margin. + [:.row-gap-5 + {:row-gap "5px"}] + [:.i {:font-style :italic}] @@ -684,6 +702,376 @@ {:background-color "rgba(72,72,72,0.2)"}] [:.bg-lighter {:background-color "rgba(0,0,0,0.15)"}] + + ;; The gryphon panel needs background-repeat:no-repeat and background-size:cover -- + ;; without them, the moment the panel grows taller than the image (as on the + ;; register form, the tallest page, with its confirm fields) the browser tiles + ;; it by default: a second gryphon appears below the first, half cropped. + [:.registration-image + {:background-image "url(/image/login-side.jpg)" + :background-repeat :no-repeat + :background-size :cover + :background-position "center" + :background-clip :content-box + :width "350px" + :min-height "600px"}] + ;; The auth card is white whatever theme the app is in, so anything inside it needs + ;; the light-ground colours: .red otherwise resolves to red-on-dark (tuned for + ;; near-black) and washes out to about 2.6:1 on white. .app.light-theme already + ;; does this but that selector never matches here, hence the explicit override. + [:.registration-content + ;; GOTCHA: body{line-height:1} is UNITLESS, so each descendant multiplies 1 by + ;; its OWN font-size, not an inherited pixel value -- unset here, text renders + ;; SOLID, descenders hitting the next line's ascenders. Set once on the card for + ;; a readable default; boxes with tuned geometry pin their own below. + {:line-height "1.5"} + [:.red {:color red} + [:a :a:visited {:color red}]] + + ;; LIGHT-GROUND VARIANTS: .field-notice, .message and .callout default to dark-app + ;; tones (rgba(255,255,255,0.06) ground, red-on-dark text, color:white), which + ;; would put white text on white if dropped onto this card unchanged. Re-toned + ;; here rather than forked, so the auth pages reuse the same components as everywhere else. + [:.field-notice + {:background-color :transparent + :border-left :none + :padding "0" + :margin "0 2px 8px" + :font-size "13px" + :line-height "19px" + ;; The words are INK. Only the mark and the label carry the colour, because + ;; a wall of red text is most of what makes a form feel like it is shouting. + :color text-color-light + :text-align :left} + ;; An empty notice is not a notice. The slot is always rendered so a live + ;; check and a submitted one cannot lay the field out differently, which + ;; means every healthy field carried a bare "!" until this. + [:&:empty {:display :none}] + [:&.is-error:before + {:content "\"!\"" + :flex "0 0 auto" + :font-weight :bold + :color error-red}] + [:&.is-note {:color muted-on-light + :margin "7px 2px 0"}]] + ;; GOTCHA: flex-basis ZERO, not auto or the dark-app notice's 260px -- those + ;; size by content, so a long message (e.g. "Too common. A few words...") + ;; wraps as a whole block, stranding the "!" above. Zero lets it wrap INSIDE + ;; the remaining space beside the mark; min-width must go with it or a long unbroken word restores the floor. + [:.field-notice-what {:flex "1 1 0" + :min-width "0"}] + [:.field-notice-action {:color "#c98700" + :font-size "13px"}] + [:.field-notice.is-error {:color text-color-light}] + [:.field-notice.is-warning {:color text-color-light}] + + [:.message {:color text-color-light}] + [:.message.tone-error {:background-color "rgba(212, 53, 28, 0.08)" + :border-color "rgba(212, 53, 28, 0.45)"}] + [:.message.tone-warning {:background-color "rgba(240, 161, 0, 0.10)" + :border-color "rgba(240, 161, 0, 0.5)"}] + [:.message.tone-success {:background-color "rgba(74, 112, 0, 0.09)" + :border-color "rgba(74, 112, 0, 0.45)"}] + [:.tone-error [:.message-icon {:color error-red}]] + [:.tone-success [:.message-icon {:color "#4a7000"}]] + + [:.bg-warning {:background-color "rgba(240, 161, 0, 0.10)"}] + [:.bg-note {:background-color "rgba(0, 0, 0, 0.04)"}] + + ;; GOTCHA: the notch is driven by :placeholder-shown, not a class -- a class + ;; desyncs the moment someone types and tabs away, dropping the label back over + ;; their text (reads as the field clearing itself); every input needs + ;; placeholder=" " for the selector to match. Field owns its own alignment, not the page's text-align:center wrapper. + [:.field {:position :static + :margin-bottom "22px" + :text-align :left}] + ;; The heading. Ink with an amber rule under it, replacing an orange word + ;; carrying a drop shadow -- which six pages each held their own copy of. + ;; The rule carries the brand colour so the heading does not have to shout + ;; it, and ink reads at full contrast on the card where the orange did not. + [:.auth-heading {:margin "0" + :font-size "25px" + :font-weight :bold + :letter-spacing "0.055em" + :text-transform :uppercase + :color "#23282e" + :text-align :center + :line-height "1.2"}] + [:.auth-rule {:width "54px" + :height "3px" + :background-color orange + :border-radius "2px" + :margin "10px auto 8px"}] + ;; Everything below the fields -- submit, "already have an account", consent -- + ;; centres itself; a page-level text-align:center wrapper is NOT used, since + ;; that leaves a non-centred element hard against the left edge. .auth-summary uses + ;; .bg-note's neutral ground with an error-colour rail rather than a tinted box, since the fields below are already tinted and two pink areas stacked would crowd. + [:.auth-summary {:border-left (str "3px solid " error-red) + :background-color "rgba(0, 0, 0, 0.035)" + :border-radius "3px" + :padding "12px 14px" + :margin "0 0 22px" + :text-align :left}] + [:.auth-summary-title {:font-weight :bold + :color error-red + :font-size "14px" + :margin-bottom "6px"}] + [:.auth-summary-list {:margin "0" + :padding-left "18px" + :font-size "13.5px" + :color text-color-light} + [:li {:margin-bottom "3px"}] + [:a {:color error-red + :text-decoration :underline + :text-underline-offset "2px" + :cursor :pointer}]] + + [:.auth-tail {:text-align :center + :padding "0 16px" + :box-sizing :border-box + :font-weight :normal + :font-size "14px" + :color text-color-light}] + ;; Full width, as the design pass settled: at 174px it read as one option + ;; among the links around it rather than the thing the page is for. + [:.join-button {:width "100%" + :height "48px" + :font-size "15px" + :font-weight "700" + :letter-spacing "0.12em" + :margin-top "4px"}] + ;; text-wrap:balance avoids a single orphaned word on the second line of centred + ;; text in this 435px column (e.g. "off." or "Privacy Policy." alone) -- without + ;; it, browsers fall back to a ragged wrap. The card is a fixed 600px and these + ;; pages hold exactly three lines, so centring the block in the space below the heading settles it against the full-height image beside it. + [:.auth-outcome {:display :flex + :flex-direction :column + :align-items :center + :justify-content :center + :text-align :center + :min-height "340px" + :padding "0 16px"}] + ;; The one thing in an outcome sentence worth picking out -- the address the + ;; mail went to. Ink and bold, because red is a fault everywhere else here. + [:.auth-emphasis {:font-weight :bold + :color "#23282e" + :word-break :break-word}] + [:.auth-outcome-line {:color text-color-light + :font-size "15px" + :max-width "34ch" + :text-wrap :balance}] + [:.auth-outcome-onward {:margin-top "26px" + :width "100%" + :max-width "260px"}] + + ;; The other ways in. A question and the thing that answers it on ONE line, + ;; with one rhythm down the group -- these were three separate blocks, two + ;; of them holding a pair of
that pushed the answer three lines from + ;; its question. + [:.auth-alts {:display :flex + :flex-direction :column + :gap "10px"}] + [:.auth-alt {:display :flex + :justify-content :center + :align-items :baseline + :flex-wrap :wrap + :gap "8px"}] + ;; Set apart from the pair above it: not another way in, but what to do when + ;; the way in never arrived. + [:.auth-help {:margin-top "22px" + :padding-top "18px" + :border-top "1px solid #ebe7e1" + :font-size "13.5px" + :color muted-on-light + :text-align :center}] + + + [:.auth-fineprint {:margin-top "16px" + :font-size "12.5px" + :color muted-on-light + :text-align :center + :text-wrap :balance + :padding "0 16px"}] + [:.auth-lede {:margin "0 0 22px" + :text-align :center + :text-wrap :balance + :color text-color-light + :font-size "14.5px"}] + + ;; The gutter base-input used to carry as .p-l-10.p-r-10 on every field. + ;; It belongs to the form, not to each field -- a field that indents itself + ;; cannot be put anywhere else -- and the wrong-state rail needs room to sit + ;; in, which is why it is 16px rather than 10. + [:.auth-form {:padding "0 16px" + :box-sizing :border-box}] + [:.field-box {:position :relative}] + ;; border-box, because there is no global one: width:100% plus 13px of + ;; padding and a border made every field wider than the 435px column it + ;; sits in, and it ran out over the gryphon. + [:.field [:input {:width "100%" + :box-sizing :border-box + :height "50px" + :padding "14px 13px 0" + :font-size "15px" + :color text-color-light + :background-color :white + :border "1px solid #d9dee3" + :border-radius "3px" + :color-scheme :light + :outline :none + :transition "border-color 0.15s, box-shadow 0.15s, background-color 0.15s"}]] + [:.field [:.notch {:position :absolute + ;; Pinned: this is positioned to the pixel against the + ;; input's border and the card's line-height would move it. + :line-height "1" + :left "11px" + :top "15px" + :padding "0 4px" + :pointer-events :none + :color muted-on-light + :font-size "15px" + :background-color :white + :transition "top 0.14s, font-size 0.14s, color 0.14s, letter-spacing 0.14s"}]] + [:.field ["input:not(:placeholder-shown) + .notch" "input:focus + .notch" + {:top "-9px" + :font-size "11px" + :letter-spacing "0.08em" + :text-transform :uppercase + :color text-color-light}]] + [:.field ["input:focus" {:border-color orange + :box-shadow "0 0 0 3px rgba(240, 161, 0, 0.20)"}]] + [:.field ["input:focus + .notch" {:color "#c98700"}]] + + ;; GOTCHA: one container, not four -- tinting the input AND doubling its border + ;; AND boxing the message AND railing the box all at once looks pinched. The + ;; field visibly changes shape as a signal before the word is read; label + ;; leaves the notch for a plain line, GOV.UK order: explanation before the box to retype. + [:.field [:.lift {:display :none + :line-height "1" + :margin "0 0 5px" + :font-size "11px" + :letter-spacing "0.08em" + :text-transform :uppercase + :color error-red}]] + [:.field.is-wrong {:padding-left "13px" + :border-left (str "3px solid " error-red) + :margin-left "-16px"}] + [:.field.is-wrong [:.lift {:display :block}]] + [:.field.is-wrong [:.notch {:display :none}]] + [:.field.is-wrong [:input {:border-color error-red + :background-color error-tint}]] + [:.field.is-wrong ["input:focus" {:box-shadow "0 0 0 3px rgba(212, 53, 28, 0.18)"}]] + + ;; The reveal. Inside the field so it cannot be read as a control of its + ;; own; the input reserves room rather than letting text run under it. + [:.peek {:position :absolute + :right "6px" + :top "8px" + :height "34px" + :padding "0 11px" + :border :none + :background :none + :color muted-on-light + :font-size "11.5px" + :font-weight :bold + :letter-spacing "0.11em" + :text-transform :uppercase + :cursor :pointer + :border-radius "3px"}] + [:.peek:hover {:color text-color-light + :background-color "#eef1f4"}] + ;; A keyword cannot carry the parens of :has(), so this selector is a string. + [".field-box:has(.peek) input" {:padding-right "78px"}]] + + ;; Password meter: 4 slots, one fill sweeping through (continuous movement, + ;; countable milestones); min length lands on the first slot's edge so the gap + ;; marks it, no separate notch needed. Fill WIDTH alone is set inline (it IS the + ;; measurement). LIGHT-ground colours only -- *-on-dark washes out on this white card -- and fill/label colours stay separate families since Legendary's fill is a gradient (wrong behind text via currentColor). + [:.pw-slots {:display :flex + :gap "4px" + :height "10px"}] + [:.pw-slot {:flex "1" + :border-radius "2px" + :overflow :hidden + :background-color "rgba(0,0,0,0.12)"}] + [:.pw-fill {:height "100%" + :width "0" + :transition "width 0.22s cubic-bezier(0.4,0,0.2,1), background-color 0.2s"}] + + ;; A broken rule takes every slot to the failure colour. The fill still + ;; reports the real length, so the bar never claims progress it has not made. + [:.pw-fill-fail {:background-color red}] + [:.pw-fill-0 {:background-color "#5d8c00"}] + [:.pw-fill-1 {:background-color "#2f6fb5"}] + [:.pw-fill-2 {:background-color purple}] + [:.pw-fill-3 {:background-image "linear-gradient(100deg,#b06d08,#e8b24a 42%,#ffe08a 52%,#e8b24a 62%,#b06d08)"}] + + [:.pw-name-fail {:color red}] + [:.pw-name-0 {:color "#4a7000"}] + [:.pw-name-1 {:color "#2f6fb5"}] + [:.pw-name-2 {:color "#6f61b0"}] + [:.pw-name-3 {:color "#a2650c"}] + + [:.pw-verdict {:display :flex + :justify-content :space-between + :align-items :center + :gap "10px" + :min-height "24px" + :margin-top "9px"}] + ;; One chip per rule. Outlined and quiet: five of these have to sit under a + ;; field without reading as five warnings, so the satisfied state is a thin + ;; green line and the broken one a thin red line, neither of them filled. + [:.pw-chips {:display :flex + :flex-wrap :wrap + :gap "5px" + :margin-top "10px"}] + [:.pw-chip {:font-size "11.5px" + :line-height "16px" + :padding "3px 9px" + :border-radius "11px" + :border "1px solid #e3e7eb" + :color muted-on-light + :background-color "#fbfcfd"}] + [:.pw-chip.is-ok {:border-color "#c2dd8e" + :color "#4a7000" + :background-color "#f4f9ea"}] + [:.pw-chip.is-bad {:border-color "#f0b9c4" + :color error-red + :background-color error-tint}] + + ;; The line under the bar, in both its readings: why a password was refused, + ;; and -- the rest of the time -- what buys the next rung. Ink rather than + ;; red: the bar and the word above it already carry the colour, and this is + ;; advice on writing a better password, not a second alarm. + [:.pw-note {:font-size "12.5px" + :line-height "18px" + :color text-color-light + :text-align :left + :margin-top "9px" + :max-width "38ch"}] + [:.pw-note.is-tip {:color muted-on-light}] + [:.pw-tier-name {:font-size "13px" + :font-weight :bold + :letter-spacing "0.08em" + :text-transform :uppercase}] + [:.pw-next {:font-size "12px" + :color "#6b7681" + :font-variant-numeric "tabular-nums"}] + + ;; The JOIN button's dimensions. They were four inline declarations on the + ;; element, alongside a class that dimmed it to 50% whenever the form was + ;; not ready -- which is gone: a control that looks dead reads as a broken + ;; site rather than an unfinished form, and it cannot say why it will not go. + [:.join-button {:height "40px" + :width "174px" + :font-size "16px" + :font-weight "600"}] + + [:.registration-notice + {:color red + :font-size "14px" + :line-height "1.5"}] + [:.bg-orange {:background-color orange}] [:.bg-red @@ -757,6 +1145,14 @@ [:.tone-warning [:.message-icon {:color orange}]] [:.tone-error [:.message-icon {:color red-on-dark}]] + ;; Per-line marks on a partial import: imported, repaired, skipped. + [:.message-mark + {:width "16px" + :margin-right "6px" + :text-align :center}] + [:.message-mark.mark-done :.message-mark.mark-repaired {:color "#8cc63f"}] + [:.message-mark.mark-skipped {:color orange}] + [:.message-body {:flex "1 1 auto" :min-width 0}] @@ -1213,15 +1609,6 @@ {:width "785px" :min-height "600px"}] - [:.login-form-inputs - {:max-width "350px" - :margin-left :auto - :margin-right :auto - :margin-top "50px"} - [:input - {:width "100%" - :box-sizing :border-box}]] - [:.registration-input {:min-width "438px"}] @@ -1565,6 +1952,51 @@ [:.lib-badge-benign {:background-color "#33658A" :color "#ffffff"}] [:.lib-badge-compat {:background-color "#8a5a00" :color "#ffffff"}]]) + ;; ── Share line ─────────────────────────────────────────────────────────── + ;; A character's sharing line: a status pill (label, never tappable) then actions as text + ;; buttons, 40px tall for a thumb target. Before a link exists it's the pill + Share link; + ;; after, Copy link / New link / Stop sharing. Light-theme colours are under .app.light-theme below. + [:.share-line + {:display :flex + :flex-wrap :wrap + :align-items :center + :column-gap "18px" + :text-transform :none}] + [:.share-pill + {:display :inline-flex + :align-items :center + :font-size "12px" + :font-weight 600 + :line-height 1.4 + :padding "3px 10px" + :border-radius "999px" + :white-space :nowrap}] + [:.share-pill-neutral {:background-color "rgba(255,255,255,0.08)" :color "rgba(255,255,255,0.75)"}] + [:.share-pill-shared {:background-color "rgba(110,168,220,0.18)" :color "#9ec7ea"}] + [:.share-pill-expired {:background-color "rgba(217,165,32,0.20)" :color "#e5c169"}] + [:.share-action + {:display :inline-flex + :align-items :center + :gap "6px" + :min-height "40px" + :padding 0 + :border :none + :background :none + :cursor :pointer + :font-family "inherit" + :font-size "13px" + :font-weight 600 + :color orange + :white-space :nowrap} + [:.fa {:font-size "14px"}]] + ;; Only where a pointer hovers: a phone keeps :hover after a tap, leaving the action underlined. + (at-media {:hover "hover"} [:.share-action:hover {:text-decoration :underline}]) + [:.share-action:focus-visible {:outline (str "2px solid " orange) :outline-offset "2px" :border-radius "3px"}] + [:.share-action:disabled {:opacity 0.6 :cursor :default :text-decoration :none}] + [:.share-action-danger {:color "#ef8592"}] + [:.share-action-quiet {:color "rgba(255,255,255,0.7)"}] + (at-media xs-query [:.share-line {:column-gap "16px"}]) + [:.roll-button {:color :white :min-width "68px" @@ -1718,15 +2150,22 @@ [:100% {:box-shadow "0 0 0 0 rgba(255,210,26,0.0)"}]) [:.save-healed {:animation "save-healed-glint 2.6s ease-in-out infinite" - :position :relative}] + :position :relative + ;; Above its neighbours. The header buttons are siblings in a row, so the + ;; next one along paints over anything this draws outside its own box -- + ;; which swallowed most of the sparkle before this was here. + :z-index 1}] ;; The sparkle itself. A pseudo-element rather than markup so the button's ;; hiccup stays a plain button, and pointer-events none so it can never eat ;; the click it is advertising. [:.save-healed:after {:content "'\\2726'" :position :absolute - :top "-4px" - :right "-3px" + ;; Inset, not hung off the corner. Sitting outside the button put it in the + ;; gap between Save and the next button, where it read as belonging to + ;; neither and was half covered by whichever one painted later. + :top "3px" + :right "4px" :font-size "11px" :line-height 1 :color "rgba(255,210,26,0.95)" @@ -1958,6 +2397,15 @@ {:background-color "rgba(180,120,0,0.16)" :color "#8a5a00"} [:.lib-dot {:background-color "#b47800"}]] + ;; Share line: the same meanings on a light ground, with the light theme's blue for actions. + [:.share-pill-neutral {:background-color "rgba(0,0,0,0.06)" :color "#4a4a4a"}] + [:.share-pill-shared {:background-color "rgba(51,101,138,0.14)" :color "#2b567a"}] + [:.share-pill-expired {:background-color "rgba(180,120,0,0.16)" :color "#8a5a00"}] + [:.share-action {:color "#33658A"}] + [:.share-action:focus-visible {:outline-color "#33658A"}] + [:.share-action-danger {:color "#9a031e"}] + [:.share-action-quiet {:color "#4a4a4a"}] + [:.builder-option-dropdown (merge {:border "1px solid #282828" @@ -1983,9 +2431,13 @@ {:background-image "linear-gradient(to right, #d35730, #eda41e)" :padding ".5em 2em"}] - [:.modal-container :.m-b-10, - :.modal-container :.link-button - {:font-weight "bold"}] + ;; GOTCHA: [:.modal-container :.m-b-10, :.modal-container :.link-button {...}] looks nested but + ;; garden treats every keyword before the map as a SEPARATE selector, compiling to + ;; .modal-container, .m-b-10, .modal-container, .link-button -- putting font-weight:bold on + ;; .m-b-10, a margin utility used 194 times across the app. Nest the vectors instead, as below. + [:.modal-container + [:.m-b-10 {:font-weight "bold"}] + [:.link-button {:font-weight "bold"}]] [:.modal-container :.link-button {:color "#f7c257" @@ -2115,6 +2567,47 @@ {:color "rgba(255,255,255,0.5)" :margin-top "8px"}] + ;; A nameless item in the missing-fields dialog: placeholder title, key beneath. + [:.issue-item-heading + {:margin-bottom "4px"}] + + [:.issue-item-key + {:font-family "monospace" + :font-size "12px" + :color "rgba(255,255,255,0.5)"}] + + ;; The z-index sits under an open header tab's 200, so a header menu long enough + ;; to reach this corner draws over the button; at 900 the button covered the + ;; menu's last item on a laptop screen. Still above the sticky button row (100). + ;; The log panel the button opens is at 950. + [:.import-log-button + {:position :fixed + :bottom "20px" + :right "20px" + :width "40px" + :height "40px" + :border-radius "50%" + :background "#2c3445" + :color :white + :box-shadow "0 2px 6px 0 rgba(0,0,0,0.5)" + :z-index 150 + :transition "all 0.2s ease"} + [:&.open {:background orange}] + [:.fa {:font-size "14px"}]] + + [:.import-log-badge + {:position :absolute + :top "-4px" + :right "-4px" + :background "#d94b20" + :color :white + :font-size "10px" + :font-weight :bold + :min-width "16px" + :height "16px" + :border-radius "8px" + :padding "0 4px"}] + ;; Conflict card — an unresolved conflict is an attention item, so its rail and ;; key carry the same warning-yellow used by the health card and library tint. [:.conflict-item @@ -2412,12 +2905,22 @@ :margin-top "6px"}] [:.whats-new-close + ;; A real