Skip to content

Fix sign-in issues from the review of #38 - #41

Merged
ashutosh091 merged 4 commits into
erphq:previewfrom
dikshantgoyal011:site/sign-in-review-fixes
Nov 13, 2026
Merged

ashutosh091 merged 4 commits into
erphq:previewfrom
dikshantgoyal011:site/sign-in-review-fixes

Conversation

@dikshantgoyal011

Copy link
Copy Markdown

What this fixes

Fixes from a code review of #38 (sign-in and the waitlist), now on preview. Each commit is one area:

d013905 Sign-in limits and the request body (api)

  • IPv6. The per-network limits (30 codes and 60 checks an hour) counted each IPv6 address on its own. A host usually holds a whole /64, so it could ask for codes from new addresses without ever reaching the limit, for example to email-bomb addresses through our Resend account. An IPv6 address is now counted by its /64.
  • Codes not sent no longer use up the address's limit. If the email failed, or another request won the one-a-minute slot, the request still counted toward the address's 5 an hour, so a few failed sends could lock someone out for an hour. Those requests now give their count back.
  • Request bodies are capped while they are read. A body sent without a Content-Length was read in full before its size was checked. It's now read in chunks and refused past 4 KB.
  • No more flaky rate-limit tests. The service counts in fixed hourly and daily windows on the real clock, so a test that started just before the hour could fail. Those tests now wait for the next window when the current one is about to end.

f4441a2 The admin page shows each account's real status (api, site)

  • With SIGNUP_MODE set to "open", the admin page listed old waitlist accounts as Waiting, with Approve buttons, although the console already lets them in. They now show as approved. Their stored status is kept, so they wait again if the waitlist comes back.
  • Approving an account that's already approved (two admins at once, or a second click) said the approval email could not be sent. It now answers alreadyApproved, and the page says "…was already approved, so no email was sent."

157687d A failed session check no longer signs people out (site)

  • When the service couldn't be asked who is signed in (a network blip, or the service briefly down), the site treated it as signed out: /app, /account and /waitlist sent the reader to /login. Those pages now show "We couldn't check your sign-in." with Try again. "Check again" on the waitlist says the service can't be reached and stays on the page.

d02a0c2 Copy, a comment and the dev setup (site, api)

  • /signup said you'd open the console straight away. It now says new accounts join the waitlist.
  • The note in site/src/constants/links.ts on bringing back the Tally form named a check that Sign in with an emailed code, with a waitlist for the beta #38 had removed. It now says to restore that check too.
  • api/.dev.vars.example allowed http://localhost:5173, where sign-in can't work, because the SameSite=Strict cookie lives on 127.0.0.1. The example and api/README.md now say to use 127.0.0.1.

After merging

The api/ changes in the first two commits take effect only once the Worker is redeployed (npm --prefix api run deploy, by someone with Cloudflare access). The site changes deploy with the preview workflow as usual.

Testing

  • npm --prefix api run check: 57 tests, all passing. A new test covers IPv6 networks; others check that failed sends give their count back, the open-mode listing, and the already-approved answer. With the fixes undone, those tests fail.
  • npm --prefix site run check passes at every commit, and docs/ matches a rebuild.
  • In Edge against a local service:
    • sign-in: 25 checks
    • waitlist: 25 checks
    • the fixes: 15 checks, covering the service going down on /app, /account and /waitlist, Try again before and after it comes back, two admins approving the same account (one email, no false failure), and the sign-up copy
  • The production build with its CSP, served as GitHub Pages serves it: 18 checks from the landing page's Join the waitlist through sign-up, approval and the console, with no CSP violations or console errors.

🤖 Generated with Claude Code

dikshant-deskera and others added 4 commits November 12, 2026 17:38
From a review of erphq#38:

- The limits per network count an IPv6 address by its /64. A host is
  usually given a whole /64, so counting each address on its own let one
  host ask for codes from new addresses without ever reaching the limit.
- The per-address limits count codes sent. A request that sends none,
  because another one won the minute or the email failed, gives its
  count back, so a failed send no longer uses up the address's five an
  hour.
- The request body is read a chunk at a time and refused once it passes
  4 KB, instead of being read in full first when it comes without a
  Content-Length.
- The tests that count requests in an hour or a day wait for the next
  window when the current one is about to end, so they can't fail by
  running across a window boundary.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
From a review of erphq#38:

- While SIGNUP_MODE is "open", nobody waits, so the admin page lists
  every account as approved instead of offering to approve accounts the
  console already lets in. The stored status is kept, so they wait again
  if the waitlist comes back.
- Approving an account that is already approved (two admins at once, or
  a second click) answers alreadyApproved, and the page says so instead
  of reporting that the approval email could not be sent.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
From a review of erphq#38: when the sign-in service couldn't be asked who is
signed in (a network blip, the service briefly down), the site took it
as being signed out. /app, /account and /waitlist sent the reader to
/login, though their session was likely fine.

The session now has a fourth state, "unavailable". Those pages show
"We couldn't check your sign-in." with Try again, which carries on once
the service answers. Check again on the waitlist page says the service
can't be reached and stays, instead of signing the reader out. Sign-in
and sign-up still show their form.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
From a review of erphq#38:

- /signup promised to open the console straight away, but a new account
  lands on the waitlist. It now says new accounts join the waitlist.
- The note in constants/links.ts on bringing the Tally form back named
  a check in vite.config.ts that c5ea719 removed. It now says to put
  that check back too.
- api/.dev.vars.example allowed http://localhost:5173, where sign-in
  can't work: the session cookie is SameSite=Strict on 127.0.0.1, and
  localhost is another site to it. The example and api/README.md now
  say to open the site at 127.0.0.1.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@ashutosh091
ashutosh091 merged commit f9f25e9 into erphq:preview Nov 13, 2026
3 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants