fix(auth): make email-verification signup completable - #3017
Conversation
The verification email linked to /_emdash/api/auth/signup/verify — the JSON endpoint the admin SPA calls — so a person clicking it saw a raw success payload and no account. The invite flow, in the same package, already links to its admin page; signup now does the same, and the SignupPage already reads ?token= from the URL and verifies it. That alone would have redirected them to login, because /_emdash/admin/signup was not on the middleware's short list of pages an anonymous visitor may open. And the redirect to login carried only the pathname, so the token would have been lost on the way back. Both fixed: signup joins login and invite/accept as public admin pages, and the login redirect keeps the query string — that last one for any future token-bearing admin URL, not just this one. Fixes emdash-cms#3007
🦋 Changeset detectedLatest commit: bb35869 The changes in this PR will be included in the next version bump. This PR includes changesets to release 17 packages
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
@emdash-cms/admin
@emdash-cms/auth
@emdash-cms/auth-atproto
@emdash-cms/blocks
@emdash-cms/cloudflare
@emdash-cms/contentful-to-portable-text
emdash
create-emdash
@emdash-cms/gutenberg-to-portable-text
@emdash-cms/plugin-cli
@emdash-cms/plugin-types
@emdash-cms/registry-client
@emdash-cms/registry-lexicons
@emdash-cms/registry-moderation
@emdash-cms/registry-verification
@emdash-cms/sandbox-workerd
@emdash-cms/x402
@emdash-cms/plugin-ai-moderation
@emdash-cms/plugin-atproto
@emdash-cms/plugin-audit-log
@emdash-cms/plugin-color
@emdash-cms/plugin-embeds
@emdash-cms/plugin-field-kit
@emdash-cms/plugin-forms
@emdash-cms/plugin-webhook-notifier
commit: |
![emdashbot[bot]](https://avatars.githubusercontent.com/in/3255304?s=60&v=4){kind=small}
There was a problem hiding this comment.
The approach is sound. The PR fixes the three real dead‑ends for email‑verification signup: the email link now points to the existing admin signup page, that page is reachable without a session, and the login redirect keeps the token query string. These changes are additive and consistent with how the invite flow already works.
I checked the diff, the full changed files, the route injection (/_emdash/admin/[...path]), getSiteBaseUrl semantics (siteOrigin/_emdash), the admin router redirect handling, and the updated tests. One issue stands out: the unit test updated its email‑link assertion but dropped the /_emdash mount segment, so it no longer verifies the link users actually receive in production. Other than that test fixture/expectation mismatch the code changes look correct.
(The reported pnpm test output could not be verified here because this is a static review.)
| expect(sentEmails[0]!.text).toContain( | ||
| "https://example.com/_emdash/api/auth/signup/verify?token=", | ||
| ); | ||
| expect(sentEmails[0]!.text).toContain("https://example.com/admin/signup?token="); |
There was a problem hiding this comment.
[needs fixing] The updated assertion no longer verifies the real email link.
The production caller (packages/core/src/astro/routes/api/auth/signup/request.ts) passes baseUrl from getSiteBaseUrl, which returns the public origin suffixed with /_emdash (e.g. https://example.com/_emdash). The new code in packages/auth/src/signup.ts uses new URL(\${config.baseUrl}/admin/signup`), so the emitted link is https://example.com/_emdash/admin/signup?token=…`, matching the PR description and the middleware public route /_emdash/admin/signup.
However, the test still passes baseUrl: "https://example.com" (line 103) and asserts "https://example.com/admin/signup?token=". The test will pass, but it validates a URL shape that is never sent to users and could miss regressions in the mount path.
Update the fixture to pass baseUrl: "https://example.com/_emdash" and assert the production URL:
| expect(sentEmails[0]!.text).toContain("https://example.com/admin/signup?token="); | |
| expect(sentEmails[0]!.text).toContain( | |
| "https://example.com/_emdash/admin/signup?token=", | |
| ); |
github-actions
Bot
added
review/awaiting-author
2 participants
What does this PR do?
Email-verification signup dead-ended three ways, described in #3007:
/_emdash/api/auth/signup/verify— the JSON endpoint the admin SPA calls — so a person clicking it saw a raw success payload. It now links to/_emdash/admin/signup?token=…, built the same way the invite email builds its link;SignupPagealready reads?token=from the URL and verifies it./_emdash/admin/signupwasn't on the middleware's list of admin pages an anonymous visitor may open, so the page redirected to login. It now joinsloginandinvite/accept.url.pathname, dropping?token=. It now keeps the query string — worth having for any future token-bearing admin URL, not just this one.Tests: signup email link assertion updated; new middleware test covers the three public admin pages and asserts the redirect preserves the full URL. Changeset included for
@emdash-cms/authandemdash.Closes #3007
Type of change
Checklist
pnpm typecheckpassespnpm lintpassespnpm testpasses (or targeted tests for my change)pnpm formathas been runAI-generated code disclosure
Screenshots / test output