feat(auth): one mail, one click — the approval mail carries the confirmation link (E9.5 S6)
The funnel's common ordering made a person open TWO messages: one saying their access was approved, another asking them to confirm their address. The approval mail now branches on what they actually still have to do — carry the confirmation link, point at sign-in, or say a confirmation mail follows once they register — so the usual case is one mail and one click. A mail that guesses wrong is worse than a plain one: telling someone to sign in while their address is unconfirmed contradicts the screen that will refuse them. That embedded link lives 7 days rather than the usual 24 hours. Nobody sits waiting for an approval — it can easily be read on Monday after a weekend, and an expired link spends the one-mail promise and puts them back on two. The risk is small: the token only ever confirms an address; it signs nobody in and sets no password. The most likely root cause of the 2026-09-03 report that a confirmation mail "never arrived" is here too, and it was never a delivery failure: re-registering an address that already had an unconfirmed account returned in SILENCE — same cheerful "check your inbox", no mail sent at all. It now re-sends the link. Nothing new leaks: the response and its timing are unchanged, and the public verify/resend already does exactly this for the same input. Whitelisting stays silent, and that is a decision, not an omission. The operator opens a door as a possibility — usually already agreed with the person — and a row can sit approved for weeks with nobody registering; a mail fired automatically there would be nudging someone who never asked. Telling them is its own act on its own button, `invited_at` records it so a second click is a decision rather than an accident, and the button only appears where nobody is behind the address yet: "the door is open, come in" is meaningless to someone already inside. Two things UAT caught that no test would have. Every fresh registration appeared in BOTH admin lists at once, because the stuck-sign-ups filter keyed on "unconfirmed" — the rule is now "the address may sign in, and this account still can't", which leaves an undecided request with the queue that owns it. And the invitation button was offered on every approved row, half of which are existing users. Also: the confirmation mail finally says what its link's rules are (once, 24 hours) — the reset mail always has, while this one left the reader to discover both by meeting a dead link. The card's "didn't get the email?" moves up under the form, and the three waiting messages name the spam folder, which is where a first message to a fresh recipient sometimes lands. Registration now records the language the sign-up page was in. Nothing reads it yet — the bilingual templates are S7 — but it is the only moment that choice exists anywhere the server can see, and a preference nobody recorded cannot be backfilled later. UAT on localdev: re-registering re-sent the link (one token became two, one account); approval issued a 7-day token and moved the account into the stuck list with only a re-send; whitelisting mailed nobody; the invitation recorded itself and the row then warned that sending again would write twice. Test data removed.
This commit is contained in:
@@ -7,6 +7,7 @@ import {
|
||||
Check,
|
||||
Clock,
|
||||
MailWarning,
|
||||
Mails,
|
||||
RotateCcw,
|
||||
Send,
|
||||
Shield,
|
||||
@@ -291,6 +292,12 @@ function UsersRoles({ me }: { me: Me }) {
|
||||
);
|
||||
}
|
||||
|
||||
/** Did the invitation fail because this instance has no working SMTP? */
|
||||
function inviteEmailOff(err: HttpError): boolean {
|
||||
const d = err.detailData as { code?: string; reason?: string } | null | undefined;
|
||||
return d?.code === "invite_send" && d.reason === "email_off";
|
||||
}
|
||||
|
||||
// --- Sign-ups that are still outside (roadmap UX-25) --------------------------------------------
|
||||
//
|
||||
// The gap this closes: approve an address BEFORE the person registers and their sign-up produces no
|
||||
@@ -528,6 +535,26 @@ function AdminInvites() {
|
||||
});
|
||||
if (ok) remove.mutate(i.id);
|
||||
};
|
||||
// Sending is its OWN act, never a side effect of approving an address: the operator opens a door
|
||||
// as a possibility (often already agreed in person), and a mail fired automatically would be
|
||||
// nudging someone who never asked. `invited_at` comes back on the row so a second click is a
|
||||
// decision rather than an accident.
|
||||
const invite = useMutation({
|
||||
mutationFn: (i: Invite) => api.sendInvitation(i.id),
|
||||
onSuccess: (_r, i) => {
|
||||
refresh();
|
||||
notify({ level: "success", message: t("settings.invites.invited", { email: i.email }) });
|
||||
},
|
||||
onError: (err) =>
|
||||
notify({
|
||||
level: "error",
|
||||
message:
|
||||
err instanceof HttpError && inviteEmailOff(err)
|
||||
? t("settings.invites.inviteEmailOff")
|
||||
: t("settings.invites.inviteFailed"),
|
||||
}),
|
||||
});
|
||||
|
||||
const add = useMutation({
|
||||
mutationFn: (email: string) => api.addInvite(email),
|
||||
onSuccess: (_d, email) => {
|
||||
@@ -604,6 +631,35 @@ function AdminInvites() {
|
||||
<span className={i.status === "approved" ? "text-accent" : "text-danger"}>
|
||||
{i.status}
|
||||
</span>
|
||||
{/* Only an approved row with NOBODY behind it yet: this button means "the door is
|
||||
open, come in", which is meaningless to someone already inside — and half the
|
||||
whitelist is existing users. An account that exists but is still stuck has its
|
||||
own surface above, with a re-send. The label carries the answer to "did I already
|
||||
write to them?", the question whose wrong answer means mailing a person twice. */}
|
||||
{i.status === "approved" && !i.has_account && (
|
||||
<Tooltip
|
||||
hint={
|
||||
i.invited_at
|
||||
? t("settings.invites.invitedAt", {
|
||||
time: new Date(i.invited_at).toLocaleString(),
|
||||
})
|
||||
: t("settings.invites.inviteHint")
|
||||
}
|
||||
>
|
||||
<button
|
||||
onClick={() => invite.mutate(i)}
|
||||
disabled={invite.isPending}
|
||||
className={`shrink-0 p-1 rounded-md disabled:opacity-50 transition ${
|
||||
i.invited_at
|
||||
? "text-muted/50 hover:text-fg"
|
||||
: "text-muted hover:text-accent"
|
||||
}`}
|
||||
aria-label={t("settings.invites.inviteLabel", { email: i.email })}
|
||||
>
|
||||
<Mails className="w-3.5 h-3.5" />
|
||||
</button>
|
||||
</Tooltip>
|
||||
)}
|
||||
{/* An entry with an account behind it stays read-only: deleting it would revoke that
|
||||
user's access, since is_allowed reads this row. Delete the user instead. */}
|
||||
{i.has_account ? (
|
||||
|
||||
@@ -657,6 +657,18 @@ function AuthCard({ verify }: { verify: VerifyEntry }) {
|
||||
</button>
|
||||
</form>
|
||||
|
||||
{/* A confirmation mail can be slow, filtered, or — before E9.5 S6 — never sent at all for a
|
||||
repeat sign-up. Offered right under the form rather than below the mode switches: it is
|
||||
the second thing a stuck person needs, after the password they just tried. */}
|
||||
{mode === "signin" && (
|
||||
<button
|
||||
onClick={goResend}
|
||||
className="mt-3 block w-full text-center text-xs text-muted hover:text-fg"
|
||||
>
|
||||
{t("welcome.auth.resendLink")}
|
||||
</button>
|
||||
)}
|
||||
|
||||
{/* Mode switches */}
|
||||
<div className="flex items-center justify-between mt-3 text-xs">
|
||||
{mode === "signin" ? (
|
||||
@@ -695,14 +707,6 @@ function AuthCard({ verify }: { verify: VerifyEntry }) {
|
||||
|
||||
{mode === "signin" && (
|
||||
<>
|
||||
{/* A verification email can be lost or never arrive, so offer a resend path from
|
||||
sign-in too — not only from the expired-link banner (needs a broken link to show). */}
|
||||
<button
|
||||
onClick={goResend}
|
||||
className="mt-2 block w-full text-center text-xs text-muted hover:text-fg"
|
||||
>
|
||||
{t("welcome.auth.resendLink")}
|
||||
</button>
|
||||
<div className="flex items-center gap-3 my-4 text-2xs uppercase tracking-wide text-muted">
|
||||
<span className="h-px flex-1 bg-border" />
|
||||
{t("welcome.auth.or")}
|
||||
|
||||
@@ -55,6 +55,7 @@ vi.mock("./Tabs", () => ({
|
||||
|
||||
const listMock = vi.fn();
|
||||
const deleteMock = vi.fn();
|
||||
const inviteMock = vi.fn();
|
||||
vi.mock("../lib/api", async (importOriginal) => {
|
||||
const actual = await importOriginal<typeof import("../lib/api")>();
|
||||
return {
|
||||
@@ -63,6 +64,7 @@ vi.mock("../lib/api", async (importOriginal) => {
|
||||
...actual.api,
|
||||
adminInvites: () => listMock(),
|
||||
deleteInvite: (id: number) => deleteMock(id),
|
||||
sendInvitation: (id: number) => inviteMock(id),
|
||||
// The access tab also renders the stuck-sign-ups section (E9.5 S5); nothing stuck here, so it
|
||||
// renders nothing and this file stays about the delete affordance.
|
||||
adminPendingAccess: () => Promise.resolve([]),
|
||||
@@ -83,6 +85,7 @@ const invite = (over: Partial<Record<string, unknown>> = {}) => ({
|
||||
decided_by: null,
|
||||
has_account: false,
|
||||
still_allowed_by_env: false,
|
||||
invited_at: null,
|
||||
...over,
|
||||
});
|
||||
|
||||
@@ -100,6 +103,9 @@ beforeEach(() => {
|
||||
notifyMock.mockClear();
|
||||
deleteMock.mockReset().mockResolvedValue({ deleted: 1, still_allowed_by_env: false });
|
||||
listMock.mockReset().mockResolvedValue([invite()]);
|
||||
inviteMock
|
||||
.mockReset()
|
||||
.mockResolvedValue(invite({ status: "approved", invited_at: "2026-09-05T09:00:00Z" }));
|
||||
meMock.email_configured = true;
|
||||
});
|
||||
afterEach(cleanup);
|
||||
@@ -223,3 +229,60 @@ describe("Access requests — the delete affordance", () => {
|
||||
expect(await screen.findByText("settings.invites.emailOff")).toBeTruthy();
|
||||
});
|
||||
});
|
||||
|
||||
// Whitelisting an address is opening a door as a possibility, not knocking on one — the operator has
|
||||
// usually already agreed it with the person, and a row can sit approved for weeks. So adding stays
|
||||
// silent and telling them is its own act, on its own button (E9.5 S6).
|
||||
describe("inviting someone the admin whitelisted", () => {
|
||||
// The decided rows live behind a <details>; await the summary rather than the section title, or
|
||||
// the click races the invite query that populates it.
|
||||
const open = async () => {
|
||||
renderPanel();
|
||||
fireEvent.click(await screen.findByText(/settings.invites.decided/));
|
||||
};
|
||||
|
||||
it("offers a send button on an approved row", async () => {
|
||||
listMock.mockResolvedValue([invite({ status: "approved" })]);
|
||||
await open();
|
||||
|
||||
fireEvent.click(screen.getByLabelText("settings.invites.inviteLabel:asker@example.com"));
|
||||
|
||||
await waitFor(() => expect(inviteMock).toHaveBeenCalledWith(1));
|
||||
expect(notifyMock).toHaveBeenCalledWith(
|
||||
expect.objectContaining({ message: "settings.invites.invited:asker@example.com" }),
|
||||
);
|
||||
});
|
||||
|
||||
it("offers none on a denied row, which would contradict the decision", async () => {
|
||||
listMock.mockResolvedValue([invite({ status: "denied" })]);
|
||||
await open();
|
||||
|
||||
expect(screen.queryByLabelText("settings.invites.inviteLabel:asker@example.com")).toBeNull();
|
||||
});
|
||||
|
||||
it("offers none once somebody is already behind that address", async () => {
|
||||
// "The door is open, come in" is meaningless to someone already inside, and most of a mature
|
||||
// whitelist is existing users. An account that exists but is stuck has its own row above.
|
||||
listMock.mockResolvedValue([invite({ status: "approved", has_account: true })]);
|
||||
await open();
|
||||
|
||||
expect(screen.queryByLabelText("settings.invites.inviteLabel:asker@example.com")).toBeNull();
|
||||
});
|
||||
|
||||
it("says the mail was never sent when the instance has no SMTP", async () => {
|
||||
listMock.mockResolvedValue([invite({ status: "approved" })]);
|
||||
inviteMock.mockRejectedValue(
|
||||
new HttpError(409, "ignored", { code: "invite_send", reason: "email_off" }),
|
||||
);
|
||||
await open();
|
||||
|
||||
fireEvent.click(screen.getByLabelText("settings.invites.inviteLabel:asker@example.com"));
|
||||
|
||||
// A button the admin pressed must not answer with a success toast for a mail nobody received.
|
||||
await waitFor(() =>
|
||||
expect(notifyMock).toHaveBeenCalledWith(
|
||||
expect.objectContaining({ message: "settings.invites.inviteEmailOff" }),
|
||||
),
|
||||
);
|
||||
});
|
||||
});
|
||||
|
||||
Reference in New Issue
Block a user