fix/plan-r1-oauth-scope
16
Commits
| Author | SHA1 | Message | Date | |
|---|---|---|---|---|
 Dmitry Verkhoturov
|
90e9a81783 |
Drop the last clause narrating an earlier draft
The paragraph correcting the coexistence claim opened by saying an earlier reading had it wrong, which is the same history narration the commit before it removed from four other places. The fact stands without it. |
||
|
Dmitry Verkhoturov
|
5045d69c20 |
Repair two sentences the previous fix broke, and finish one it missed
Verifying the last pass found that two of its own corrections introduced fresh errors. Removing a reference to an earlier draft left the Telegram sentence circular, comparing a defect to a milder form of itself with no antecedent. And the OAuth scope sentence attached its relative clause to the wrong thing, so it read as though blocking-or-partitioning were the recommended recipe; those are different facts, since the recipe drops AUTH_SAME_SITE=none and the callback cookie then carries no attribute at all. One correction had not been applied. The line saying AUTH_SAME_SITE=none adds an unpartitioned HttpOnly JWT and contributes nothing was true of the engine the cookie jar was read on and stated of all of them; Safari drops that pair outright and Firefox delivers it either way, so it is dead weight on all three for three different reasons. Smaller: a configuration survives a browser setting instead of reaching it, the Path B task list is above its cost paragraph and not below, Firefox never holds a partitioned copy instead of not needing one, and the two remaining shorthand mentions of XSS exposure now name the token exposure they mean. |
||
|
Dmitry Verkhoturov
|
d5ede91a9f |
Correct three claims a second review pass found overstated
The mechanism paragraph said the server pair being refused is what lets the widget's partitioned pair land. That is false: CHIPS makes the partition key part of a cookie's identity, so the two are different cookies and coexist, and #2218 measured all four at once on Chromium. Firefox is the exception because Total Cookie Protection files the server cookie under the embedder's partition, which makes the keys collide, and a script may not replace an HttpOnly cookie it collides with. The section now reports the outcome per engine and stops asserting a causal link the measurements do not show. PartitionedCookies was called the better answer for either path with no limits attached. Two apply. The option is global, so Service.Set puts it on the OAuth callback too, and a cookie partitioned to the popup's own top-level context is one the frame cannot see, which regresses OAuth on the permissive browsers where it works today. And HttpOnly removes bearer-token theft, not authenticated action during an XSS, since the XSRF value stays readable by design and script on the widget origin can still make requests the browser attaches the cookie to. The evidence claim was broader than any single run supports. The criterion asks for sign-in, a post and a reload; the permanent suite does all three but only on Chromium's default policy against a stack that is not the recommended recipe, and the cross-browser campaign covered sign-in and reload on the recommended recipe without posting. Both halves are now stated. Also: 70 runnable top-level tests, not 71, which counted TestMain; and three engines across four browser targets, since Safari is a WebKit browser. |
||
|
Dmitry Verkhoturov
|
ca7f55d659 |
Correct the engine claims, and stop narrating the document's own history
A review pass found the measured section had introduced a contradiction while
fixing one. It said Chromium, WebKit and Safari all refuse the server pair
because the default emits no SameSite attribute, then said two paragraphs later
that Safari blocks third-party cookies whatever the attribute says. Only the
refusal was measured on all three; the cause is the attribute on Chromium and
third-party blocking on WebKit and Safari, and the section now says which is
which.
Three more places stated a single engine's behaviour as every engine's: the
claim that the browser drops the server pair, which Firefox does not; the
Firefox session described as riding on an unpartitioned cookie, when Total
Cookie Protection stores it partitioned despite the missing attribute, which is
why it survives; and the flat claim that master satisfies the criterion, which
Firefox's block-all mode is the exception to.
OAuth off-domain is no longer called impossible outright, since an unpartitioned
cookie is still delivered on a permissive browser with AUTH_SAME_SITE=none; it
fails wherever third-party cookies are blocked or partitioned, and under the
recommended recipe everywhere. The Storage Access route is marked as reasoned
from browser policy instead of measured, because nothing in the campaign called
requestStorageAccess, and Firefox is added to the list of implementers.
Also drops four passages narrating earlier drafts of this document, which the
reader never saw, and refreshes counts that had gone stale against
|
||
|
Dmitry Verkhoturov
|
10c5b158db |
Record what the browser matrix measured, and what it changed
The R1 mechanism was reasoned from the code and checked on Chromium. Running it on two genuinely different registrable domains with real certificates, across Chromium, Firefox, WebKit and Safari 27, shows the arrangement working everywhere except Firefox's opt-in block-all, and working by two different mechanisms. Chromium, WebKit and Safari refuse the server's pair, because AUTH_SAME_SITE defaults to emitting no SameSite attribute and a cross-site cookie without one is rejected; that refusal is what lets the widget's partitioned pair land. Firefox accepts it, and since that cookie is HttpOnly the browser then forbids the widget's script from replacing it, so Firefox holds no partitioned copy and rides on an ordinary third-party cookie even with the flag on. Safari blocks third-party cookies with nothing configured, which makes AUTH_SAME_SITE=none alone already broken there rather than deprecated. It also changes what the upstream Partitioned work is worth, which this section had written off twice. Building go-pkgz/auth with the option and running the same matrix gives the same persistence with the JWT still HttpOnly: on Safari, document.cookie inside the frame returns the token under the header flag and does not under the partitioned build. The header path's XSS cost is avoidable, not inherent, so both paths are better with the upstream change even though neither depends on it. |
||
|
Dmitry Verkhoturov
|
a0680b11ea |
Bring the plan up to master, and mark two scope calls as the maintainer's
An end-to-end pass against current master found the document stale in places
where it is used as a factual base for a decision, and found two places where
this branch had overstepped.
The two overreaches first. The acceptance criteria narrowed R1 from any
configured provider to three flows, which is a product-scope decision and not
the factual correction it was presented as, especially while the Path B
constraints still say fixing #1139 must not get harder. And the criterion is
met for anonymous and email by measurement but for Telegram by inference, which
sits badly with a standing requirement being a test a proposal passes. Both are
now written as open decisions with the alternatives spelled out.
Two costs were wrong in Path B's favour and against it. The R1 line claimed a
server-rendered fragment has no client-side writer, contradicting Path B's own
task list, which retains the existing auth through htmx:configRequest; fetcher.ts
already turns X-JWT into the partitioned pair, so the upstream Partitioned work
is mandatory only for a server-set default-on solution. And the XSRF caution
rested on GET /deleteme as though it were unprotected: it sits under radmin
behind Auth, AdminOnly and matchSiteID, and the handler needs a signed token
carrying delete_me, so exempting GET removes one gate of three. The caution
stands but now asks for an audit instead of leaning on that example.
Task 2's backlog was sending someone to write tests that exist: the cross-origin
page, comments.html and its injection case, and the config surface are all
covered now. What is left is hash deep links, max_shown_comments, Telegram, the
storage-denied trigger, and a path-prefixed deployment that #2219 makes concrete.
Also refreshes the figures to
|
||
|
Dmitry Verkhoturov
|
6f7313fcd7 |
Price the Storage Access route as it really stands: not reachable today
The section sold it as a permission prompt on top of the existing flow. A browser denies requestStorageAccess outright when the embedded origin has no recent first-party interaction to grant against, and remark42 never gets one: the reader interacts on the provider's origin, and the callback returns to a document whose first statement is window.close() under ?selfClose. So the real cost is changing the first-party experience, either by having the callback collect a click before closing or by establishing interaction some other way, which is a different order of cost from the other two routes. Also states the grant correctly. It lets the frame's requests carry the cookie; it does not make it script-readable, and the JWT cookie is HttpOnly regardless, which is what the OAuth paragraph above already says. |
||
|
Dmitry Verkhoturov
|
5224c5dfcf | Drop three more banned constructions from the R1 text | ||
|
Dmitry Verkhoturov
|
3b7a7778dc |
Price the Storage Access API, and stop calling the header path cookie-free
Two more from the review. The section declared that no frontend change could bring OAuth inside the criterion, which writes off the Storage Access API: an embedded frame can ask the browser, on a user gesture, for access to its own unpartitioned first-party cookies, and both Safari and Chrome implement it. That would read the cookie the callback already set with no backend work, at the cost of a revocable permission prompt. It belongs in the comparison, and it is the only one of the three routes that is not a backend feature. And the constraint bullet still offered CHIPS or a token not relying on ambient cookies as alternatives, when the header path relies on one as soon as the page reloads. Both routes rest on the same attribute; they differ over who writes the cookie and what that costs. |
||
|
Dmitry Verkhoturov
|
be61b5873a |
Correct the R1 mechanism: the attribute carries the reload, not the writer
A reviewer pass found the central claim inverted. The text said the token never travels as a third-party cookie because fetcher.ts writes it inside the frame. It does travel as one: activeJwtToken is a module-level variable filled only from the response header, nothing reads the JWT cookie back, and getCookie is called once in the whole app for XSRF-TOKEN, so the first request after a reload sends no header and the token arrives ambiently. What spares it from blocking is the Partitioned attribute authCookieOptions sets, which is what #2214's control cookie exists to prove. Consequences elsewhere in the section. Saying the upstream Partitioned work buys no flow that does not already work was false: AUTH_SEND_JWT_HEADER ships off, so in the default configuration nothing writes a partitioned cookie at all. The OAuth handoff was described only in the shape that needs that upstream work, when answering the redemption with X-JWT needs nothing upstream. The partition-key description was a counterfactual, since a cookie with no Partitioned attribute has no partition key, and the SameSite clause did not apply to a top-level callback navigation under AUTH_SAME_SITE=none. Also: the documentation gap is two edits, since the parameters page documents the flag but still promises SameSite=Strict and a __Host- prefix that #2197 removed; #2214 adds three TLS functions and only two are table-driven; the task list still claimed R1 has no e2e coverage; the constraint bullet still described setAuthCookie's pre-#2197 behaviour; and the Path B cost line had swapped the real upstream dependency for an invented one. Wrapped to 100 columns. |
||
|
Dmitry Verkhoturov
|
6831206574 | Drop two banned constructions from the R1 text and one from Path B | ||
|
Dmitry Verkhoturov
|
8e16a7373c |
Record that email is now measured, leaving Telegram as the inference
#2214 turned its two TLS cases into tables over anonymous and email, so both are exercised in a third-party frame with the reload and again under enforced partitioning. Telegram is the only one of the three still resting on the writer keying off X-JWT and not off the provider, with #2208 as the reason it cannot be measured and go-pkgz/auth#316 as what would change that. |
||
|
Dmitry Verkhoturov
|
864cde68c8 |
Narrow the R1 coverage claim to the flow the suite measures
#2214's two TLS cases both sign in anonymously, so anonymous is the only one of the three named flows exercised in a third-party frame. Email is covered over http and never embedded, and Telegram is covered nowhere and cannot be until #2208 makes the API base URL configurable. The expectation that all three behave alike rests on the client-side writer keying off X-JWT and not off the provider, which is an inference and now reads as one. |
||
|
Dmitry Verkhoturov
|
33e93a427d |
Correct R1 in the frontend plan: scope it to the flows that can meet it
The acceptance criteria promised sign-in with any configured provider under third-party cookie blocking, which OAuth as built cannot satisfy: the callback runs in a popup, a top-level context of its own, so the cookie it sets is keyed to the auth host and the frame embedded on the other domain is a different partition that never sees it. The criteria now name email, Telegram and anonymous, and OAuth carries its own paragraph explaining why it is out and what would bring it in, which is the server-mediated one-time code the constraints below already describe. Two claims went stale alongside it. Master meets the criterion today with AUTH_SEND_JWT_HEADER on, because fetcher.ts writes the cookie from inside the frame and the token never travels as a third-party cookie, so the remaining gap for those flows is documentation and not code. The server-set cookies still lack Partitioned, but nothing depends on them surviving in a third-party frame any more, so the upstream work they were waiting on buys no flow that does not already work. |
||
 Dmitry VerkhoturovandGitHub
|
23be25d84a |
Fix seven widget defects, including the cookies the separate-domain setup needs (#2197)
* Drop the frontend workspace root and re-resolve the lockfile
`frontend/` carried a `package.json`, a `pnpm-workspace.yaml` and the lockfile
for a workspace of exactly one package. Two manifests meant two places to
declare a version, and the app pin was the one that did not win: `preact` and
`@babel/core` were each written twice, and a bump to the app manifest alone
would have been a silent no-op, since `pnpm.overrides` decides and it lived at
the root.
Everything pnpm reads now lives in `frontend/apps/remark42`: dependencies,
`packageManager`, `engines` and the overrides. `frontend/` keeps `.nvmrc`,
`.husky` and `CLAUDE.md`, none of which pnpm reads. The directory nesting
stays: every path in the repository points at `frontend/apps/remark42`,
including the published contributing docs, so moving the package up would have
rewritten 14 files to no benefit.
Moving the manifest kept the old resolutions verbatim, which left optional peer
subtrees the tree no longer reaches: `ts-node` under jest, `@swc/core` under
webpack, `vitest` under `@testing-library/jest-dom`, `tslib` under
`webpack-dev-server`. None is referenced by any config or source file here.
Re-resolving drops 137 packages and moves 59 to versions already permitted by
the ranges in the manifest, 1446 to 1308, with no direct dependency changing
version: the five that look changed differ only in their peer suffix. Every
file `pnpm build` produces is identical in size before and after.
The frontend-deps stage of the Dockerfile sets `CI=true` so the `prepare`
script skips husky, which has no git repository to install hooks into there.
* Stop markdown-only changes triggering heavy workflows, and check the documented versions
`ci-backend.yml`, `ci-build.yml` and `ci-frontend.yml` all end their path
filters with `!**.md`. The e2e workflow did not, so a change to any markdown
file under `frontend/` or `backend/` matched its `frontend/**` and `backend/**`
entries and started a docker build and the whole browser suite. The release
filter had the same hole and two of its own: it names `README.md` and `LICENSE`
on purpose, since `.goreleaser.yml` packages both, so it now excludes markdown
under `backend/` and `frontend/` only. `CLAUDE.md` and the installation page
were listed as well, and neither is packaged.
`ci-site.yml` goes on matching markdown, which is right, since the site is
built from it. It excludes `CLAUDE.md`, so a future `site/CLAUDE.md` cannot
start a site build, and `site/README.md`, which documents how to build the site
rather than being part of it.
The installation page tells a reader that a source build needs Go 1.25, Node
24+ and PNPM 10. Nothing kept those in step with `backend/go.mod`,
`engines.node`, `packageManager` and `.nvmrc`, and the drift is silent: a wrong
version in the docs builds and tests exactly as well as a right one. `.nvmrc`
is the pin with form here, having sat at 16 through the whole node 20 migration
because nothing red ever pointed at it. The check compares each stated version
against its source and holds `.nvmrc` to `engines.node`, and it fails when the
page states no version at all, so removing the claims cannot turn it into a
check that passes by comparing nothing.
Its own workflow rather than a step in an existing one, since the inputs span
the backend module, the frontend manifest and the site.
* Fix the cookie fallback page, asset path, message senders, auth teardown and cookies
Two defects with the same origin:
|
||
|
Dmitry VerkhoturovandGitHub
|
06436ff9b0 |
Migrate batch 1 components from BEM to CSS Modules (#2014)
* feat: migrate batch 1 components from BEM to CSS Modules Migrate 8 components from BEM to CSS Modules: - button (7 BEM files -> 1 module) - dropdown (7 BEM files -> 1 module) - thread (3 BEM files -> 1 module) - auth-panel (2 BEM files -> 1 module) - dropdown-item, list-comments, subscribe-by-rss, settings (from batch 0 PR #2013) Consolidates 19 BEM CSS files into 8 CSS Module files. Uses clsx for conditional class composition, replacing bem-react-helper's b() calls. Class naming follows the established convention: BEM block = .root, elements = camelCase, modifiers = camelCase. Visual regression verification on built artefacts: - remark.css: 43,779 -> 43,299 bytes (480 bytes smaller) - last-comments.css: 18,792 -> 18,776 bytes (16 bytes smaller) - remark.js: 256,709 -> 304,837 bytes (48KB larger, expected: CSS Module classname mappings now live in JS instead of plain strings) - Dark theme: pixel-identical (zero difference) - Light theme: pixel-identical (0.21% diff is the native demo page "Toggle theme" button, not any remark42 widget element) Also updates CLAUDE.md CSS guideline to reflect the migration status. * Migrate remaining BEM components to CSS Modules (final batch) Migrate the last 4 BEM components to CSS Modules, completing the migration and removing bem-react-helper from the project entirely. Components migrated: - subscribe-by-email (1 BEM CSS file -> 1 module) - comment-form + markdown-toolbar (20 BEM CSS files -> 2 modules) - comment (19 BEM CSS files -> expanded existing module) - root (10 BEM CSS files -> expanded existing module) Consolidates ~50 BEM CSS files into 4 new + 2 expanded CSS Module files. Removes bem-react-helper dependency — all components now use clsx for conditional class composition. Dead CSS cleanup during migration: - Orphaned comment-actions selectors in comment theme CSS (already migrated) - Dead BEM modifiers: comment_disabled, comment_pinned, comment_guest - Dead element: comment__user-id (CSS existed but never used in TSX) - Dead button type classes: comment-form__button_type_preview/_send - Dead mix values: auth-email-login-form__back-button, comment-form__email-dropdown Key implementation details: - comment_highlighting stays global via :global() (imperatively added by classList) - Bare .dark/.light theme class preserved on root wrapper (8+ modules depend on it) - raw-content.css kept as global utility CSS (syntax highlighting) Visual regression verification on built artefacts: - remark.css: 43,779 -> 36,106 bytes (-17.5%) - last-comments.css: 18,792 -> 13,955 bytes (-25.7%) - remark.js: 256,709 -> 253,637 bytes (-1.2%) - last-comments.js: 121,726 -> 120,795 bytes (-0.8%) - Total: 441,006 -> 424,493 bytes (-3.7%) - Screenshot comparison: pixel-identical across light/dark themes |