From bb12e603a3e7b4f04babc3e3a2f9d916d97b419e Mon Sep 17 00:00:00 2001 From: Lucas Rayan Date: Mon, 24 Aug 2026 11:02:02 -0300 Subject: [PATCH] fix(admin): dominio trocado ao criar, apelidos escondidos e quota a zero Criar "lucas@cienciaembarcada.com.br" criava "lucas@lucasrguerra.dev.br". O formulario partia o endereco e deitava fora a metade do dominio, e nao enviava domain_id nenhum -- o servidor caia entao no dominio do admin com sessao iniciada. Passa a ser parte local mais um dominio escolhido de uma lista: a metade que decide o endereco deixa de poder ser descartada em silencio. Apelidos de outros dominios estavam escondidos, e a culpa era minha: procurava so no dominio onde a morada do proprio utilizador vive. Uma caixa recebe postmaster, abuse, dmarc e tlsrpt de varios dominios ao mesmo tempo, por isso o painel mostrava quatro onde entregavam oito. Ha um teste que falha com a consulta antiga. Os apelidos passam a poder ser criados e removidos dentro do modal. O dominio sai do endereco escrito, para nao repetir a armadilha do formulario de cima. USER, DOMAIN_ADMIN e SUPER_ADMIN eram constantes impressas no ecra, logo ficavam em ingles em qualquer idioma. Um papel desconhecido continua a ser mostrado tal como esta guardado, porque quem o ve precisa de saber o que a base de dados tem. A quota estava certa e mal mostrada. used_bytes vale 140771 -- os 137,5 KB reais -- mas a pagina arredondava para megabytes inteiros, portanto qualquer coisa abaixo de 512 KB lia-se "0 / 2 MB" e a barra ficava vazia, calculada sobre os megabytes ja arredondados. Passa a formatar a partir dos bytes. Co-Authored-By: Claude Opus 5 --- services/auth/src/repository.ts | 21 ++- services/auth/src/router.test.ts | 20 ++ services/webmail/messages/en.json | 9 +- services/webmail/messages/es.json | 11 +- services/webmail/messages/pt-BR.json | 9 +- .../src/app/(admin)/admin/mailboxes/page.tsx | 174 +++++++++++++++--- services/webmail/src/lib/formatBytes.test.ts | 43 +++++ services/webmail/src/lib/formatBytes.ts | 38 ++++ services/webmail/src/lib/roleLabel.ts | 18 ++ 9 files changed, 308 insertions(+), 35 deletions(-) create mode 100644 services/webmail/src/lib/formatBytes.test.ts create mode 100644 services/webmail/src/lib/formatBytes.ts create mode 100644 services/webmail/src/lib/roleLabel.ts diff --git a/services/auth/src/repository.ts b/services/auth/src/repository.ts index ffc3a20..a1e229b 100644 --- a/services/auth/src/repository.ts +++ b/services/auth/src/repository.ts @@ -896,24 +896,31 @@ export async function listAliasesForMailbox( mailboxId: string, ): Promise { const { sql, params } = scopeClause(scope, "m.domain_id"); - const owner = await queryOne<{ email_address: string; domain_id: string }>( - `SELECT m.email_address, m.domain_id FROM mailboxes m WHERE m.id = $${params.length + 1} AND ${sql}`, + const owner = await queryOne<{ email_address: string }>( + `SELECT m.email_address FROM mailboxes m WHERE m.id = $${params.length + 1} AND ${sql}`, [...params, mailboxId], ); // Out of scope and non-existent are answered the same way, so the console // of one domain cannot probe for ids belonging to another. if (!owner) return null; + // Every domain in the caller's scope, not only the one the user's own + // address sits in. One mailbox commonly receives postmaster, abuse, dmarc + // and tlsrpt for several domains at once, and scoping this to the owner's + // domain hid all the others - the panel showed four aliases where eight + // were delivering. + const { sql: aliasSql, params: aliasParams } = scopeClause(scope, "a.domain_id"); return query( - `SELECT a.id, a.source_address, a.destination_addresses, a.is_catch_all, a.is_active - FROM aliases a - WHERE a.domain_id = $1 + `SELECT a.id, a.source_address, a.destination_addresses, a.is_catch_all, a.is_active, + d.name AS domain_name + FROM aliases a JOIN domains d ON d.id = a.domain_id + WHERE ${aliasSql} AND EXISTS ( SELECT 1 FROM unnest(a.destination_addresses) AS dest - WHERE lower(dest) = lower($2) + WHERE lower(dest) = lower($${aliasParams.length + 1}) ) ORDER BY a.source_address`, - [owner.domain_id, owner.email_address], + [...aliasParams, owner.email_address], ); } diff --git a/services/auth/src/router.test.ts b/services/auth/src/router.test.ts index ed782eb..5ece9d3 100644 --- a/services/auth/src/router.test.ts +++ b/services/auth/src/router.test.ts @@ -826,6 +826,26 @@ describeDb("auth API against a real database", () => { expect(res.status).toBe(404); }); + // One mailbox commonly receives postmaster, abuse, dmarc and tlsrpt for + // several domains at once. Scoping the lookup to the domain the user's own + // address sits in hid every alias belonging to the others. + it("lists aliases from every domain, not only the user's own", async () => { + const mine = `cross-domain-alias@pagedtest-${PAGED_DOMAIN}.invalid`; + await query( + `INSERT INTO aliases (domain_id, source_address, destination_addresses) + VALUES ($1, $2, ARRAY[$3]) ON CONFLICT DO NOTHING`, + [PAGED_DOMAIN, mine, userEmail()], + ); + + const res = await get(`/api/v1/admin/mailboxes/${USER_ID}/aliases`, admin); + expect(res.status).toBe(200); + const sources = (res.body as unknown as { source_address: string }[]).map((a) => a.source_address); + + // USER_ID lives in DOMAIN_ID; this alias lives in PAGED_DOMAIN and + // delivers to them all the same. + expect(sources).toContain(mine); + }); + it("answers 404 for a user that does not exist rather than leaking the difference", async () => { const res = await get(`/api/v1/admin/mailboxes/44444444-4444-4444-4444-444444444444/aliases`, admin); expect(res.status).toBe(404); diff --git a/services/webmail/messages/en.json b/services/webmail/messages/en.json index 6374dec..e9dd2a0 100644 --- a/services/webmail/messages/en.json +++ b/services/webmail/messages/en.json @@ -104,7 +104,9 @@ "serverChecks": "Server", "userChecks": "User", "runChecks": "Run", - "checksRun": "{count} checks" + "checksRun": "{count} checks", + "pickADomain": "Pick a domain.", + "aliasSource": "Alias address" }, "auth": { "adminLoginTitle": "Admin console", @@ -413,5 +415,10 @@ "queueHasFrozen": "Frozen messages in the queue", "usersOverQuota": "Users over their quota", "noUsersOverQuota": "No user is over quota" + }, + "roles": { + "user": "User", + "domainAdmin": "Domain admin", + "superAdmin": "Super admin" } } diff --git a/services/webmail/messages/es.json b/services/webmail/messages/es.json index 71ccab6..fb6bd02 100644 --- a/services/webmail/messages/es.json +++ b/services/webmail/messages/es.json @@ -1,6 +1,6 @@ { "admin": { - "addAlias": "Añadir apelido", + "addAlias": "Anadir alias", "aliasTarget": "Dirección de destino", "aliasesTitle": "Alias", "auditLog": "Registro de auditoría", @@ -104,7 +104,9 @@ "serverChecks": "Servidor", "userChecks": "Usuario", "runChecks": "Ejecutar", - "checksRun": "{count} comprobaciones" + "checksRun": "{count} comprobaciones", + "pickADomain": "Elija un dominio.", + "aliasSource": "Direccion del alias" }, "auth": { "adminLoginTitle": "Consola de administración", @@ -413,5 +415,10 @@ "queueHasFrozen": "Hay mensajes congelados en la cola", "usersOverQuota": "Usuarios con la cuota excedida", "noUsersOverQuota": "Ningun usuario con la cuota excedida" + }, + "roles": { + "user": "Usuario", + "domainAdmin": "Admin del dominio", + "superAdmin": "Super admin" } } diff --git a/services/webmail/messages/pt-BR.json b/services/webmail/messages/pt-BR.json index 6a8139e..a440b42 100644 --- a/services/webmail/messages/pt-BR.json +++ b/services/webmail/messages/pt-BR.json @@ -104,7 +104,9 @@ "serverChecks": "Servidor", "userChecks": "Utilizador", "runChecks": "Correr", - "checksRun": "{count} verificacoes" + "checksRun": "{count} verificacoes", + "pickADomain": "Escolha um dominio.", + "aliasSource": "Endereco do apelido" }, "auth": { "adminLoginTitle": "Console de administração", @@ -413,5 +415,10 @@ "queueHasFrozen": "Ha mensagens congeladas na fila", "usersOverQuota": "Utilizadores com a quota excedida", "noUsersOverQuota": "Nenhum utilizador com a quota excedida" + }, + "roles": { + "user": "Utilizador", + "domainAdmin": "Admin do dominio", + "superAdmin": "Super admin" } } diff --git a/services/webmail/src/app/(admin)/admin/mailboxes/page.tsx b/services/webmail/src/app/(admin)/admin/mailboxes/page.tsx index f8bf2a4..aa89d96 100644 --- a/services/webmail/src/app/(admin)/admin/mailboxes/page.tsx +++ b/services/webmail/src/app/(admin)/admin/mailboxes/page.tsx @@ -22,6 +22,8 @@ import { useTranslations } from "../../../../i18n/provider"; import { Badge } from "../../../../components/ui/Badge"; import { Pagination } from "../../../../components/ui/Pagination"; import { useDebounced } from "../../../../lib/useDebounced"; +import { formatBytes, usageRatio } from "../../../../lib/formatBytes"; +import { roleLabel } from "../../../../lib/roleLabel"; import { initialsFor } from "../../../../lib/initials"; import { Button } from "../../../../components/ui/Button"; @@ -29,8 +31,8 @@ interface Mailbox { id: string; email: string; role: string; - storageUsedMb: number; - quotaMb: number; + usedBytes: number; + quotaBytes: number; mfaEnabled: boolean; locked: boolean; locale: string; @@ -76,6 +78,8 @@ export default function AdminMailboxesPage() { const [activeTab, setActiveTab] = useState<"mailboxes" | "aliases" | "csv">("mailboxes"); const [mailboxes, setMailboxes] = useState([]); const [aliases, setAliases] = useState([]); + const [domains, setDomains] = useState<{ id: string; name: string }[]>([]); + const [newDomainId, setNewDomainId] = useState(""); const [mfaPolicy, setMfaPolicy] = useState<"optional" | "required_admins" | "required_all">("required_admins"); // Create mailbox state @@ -111,6 +115,8 @@ export default function AdminMailboxesPage() { const [editLocale, setEditLocale] = useState(""); const [editAliases, setEditAliases] = useState(null); const [saving, setSaving] = useState(false); + const [newAliasForUser, setNewAliasForUser] = useState(""); + const [aliasBusy, setAliasBusy] = useState(false); const load = useCallback(async () => { try { @@ -125,6 +131,14 @@ export default function AdminMailboxesPage() { ), fetch("/api/v1/admin/aliases").then((r) => (r.ok ? r.json() : [])), ]); + + // The address to create is local part plus a chosen domain. Without the + // list there is nothing to choose from, and the server falls back to the + // signed-in admin's own domain. + const dom = await fetch("/api/v1/admin/domains").then((r) => (r.ok ? r.json() : [])); + const domainRows = (dom as { id: string; name: string }[]) ?? []; + setDomains(domainRows); + setNewDomainId((prev) => (domainRows.some((d) => d.id === prev) ? prev : (domainRows[0]?.id ?? ""))); const users = mb as PagedUsers; setPageInfo({ total: users.total ?? 0, @@ -137,8 +151,10 @@ export default function AdminMailboxesPage() { id: m.id, email: m.email_address, role: m.role, - storageUsedMb: Math.round(Number(m.used_bytes ?? 0) / 1048576), - quotaMb: Math.round(Number(m.quota_bytes ?? 0) / 1048576), + // Kept as bytes. Rounding to whole megabytes here is what made a + // mailbox holding 137.5 KB read as "0 / 2 MB" with an empty bar. + usedBytes: Number(m.used_bytes ?? 0), + quotaBytes: Number(m.quota_bytes ?? 0), mfaEnabled: Boolean(m.mfa_enrolled), locked: !m.is_active, locale: m.locale ?? "", @@ -170,11 +186,20 @@ export default function AdminMailboxesPage() { const handleCreateMailbox = async (e: React.FormEvent) => { e.preventDefault(); setError(null); - const localPart = newEmail.split("@")[0]; + // The domain half used to be split off and thrown away, and no domain_id + // was sent - so the server fell back to the signed-in admin's own domain + // and "lucas@cienciaembarcada.com.br" was created as + // "lucas@lucasrguerra.dev.br", silently. + const localPart = newEmail.split("@")[0].trim(); + if (!newDomainId) { + setError(t("admin.pickADomain")); + return; + } const res = await fetch("/api/v1/admin/mailboxes", { method: "POST", headers: { "Content-Type": "application/json" }, body: JSON.stringify({ + domain_id: newDomainId, local_part: localPart, password: newPassword, role: newRole, @@ -219,7 +244,7 @@ export default function AdminMailboxesPage() { const openUser = async (mb: Mailbox) => { setEditing(mb); setEditRole(mb.role); - setEditQuotaMb(mb.quotaMb); + setEditQuotaMb(Math.round(mb.quotaBytes / 1048576)); setEditLocale(mb.locale); setEditAliases(null); try { @@ -238,6 +263,54 @@ export default function AdminMailboxesPage() { } }; + /** + * Adds an alias that delivers to the user being edited. + * + * The domain is taken from the address typed, so an alias for a second + * domain does not silently land in the admin's own - the same trap the + * mailbox form fell into. + */ + const addAliasForUser = async () => { + if (!editing || !newAliasForUser.trim()) return; + setAliasBusy(true); + setError(null); + try { + const source = newAliasForUser.trim().toLowerCase(); + const domainName = source.split("@")[1] ?? ""; + const domain = domains.find((d) => d.name.toLowerCase() === domainName); + if (!domain) { + setError(t("admin.pickADomain")); + return; + } + const res = await fetch("/api/v1/admin/aliases", { + method: "POST", + headers: { "Content-Type": "application/json" }, + body: JSON.stringify({ domain_id: domain.id, source, destinations: [editing.email] }), + }); + const data = await res.json().catch(() => ({})); + if (!res.ok) throw new Error(data.message); + setNewAliasForUser(""); + await openUser(editing); + await load(); + } catch (err) { + setError(err instanceof Error && err.message ? err.message : t("errors.serverError")); + } finally { + setAliasBusy(false); + } + }; + + const removeAliasForUser = async (id: string) => { + if (!editing) return; + setAliasBusy(true); + try { + await fetch(`/api/v1/admin/aliases/${encodeURIComponent(id)}`, { method: "DELETE" }); + await openUser(editing); + await load(); + } finally { + setAliasBusy(false); + } + }; + const saveUser = async () => { if (!editing) return; setSaving(true); @@ -438,14 +511,33 @@ export default function AdminMailboxesPage() { setNewEmail(e.target.value)} - placeholder="nova.conta@domain.com" + placeholder="nova.conta" required className={input} /> + {/* The domain is chosen, not typed. Typing a full address let + the domain half be discarded without anyone noticing. */} +
+ + +
@@ -515,9 +607,9 @@ export default function AdminMailboxesPage() { className="rounded-[10px] border border-white/[0.14] bg-dark-panel px-3 py-2 text-[13px] text-slate-100" > - - - + + +
@@ -700,12 +791,47 @@ export default function AdminMailboxesPage() { ) : (
    {editAliases.map((a) => ( -
  • - {a.aliasAddress} +
  • + + {a.aliasAddress} + +
    • ))}
)} + +
+ setNewAliasForUser(e.target.value)} + placeholder={t("admin.aliasSource")} + aria-label={t("admin.aliasSource")} + className={`${input} min-w-[200px] flex-1`} + /> + +
diff --git a/services/webmail/src/lib/formatBytes.test.ts b/services/webmail/src/lib/formatBytes.test.ts new file mode 100644 index 0000000..c24224b --- /dev/null +++ b/services/webmail/src/lib/formatBytes.test.ts @@ -0,0 +1,43 @@ +import { describe, it, expect } from "vitest"; +import { formatBytes, usageRatio } from "./formatBytes"; + +describe("formatBytes", () => { + // The case from the console: 140771 bytes rendered as "0 / 2 MB". + it("shows a mailbox smaller than a megabyte as itself", () => { + expect(formatBytes(140771)).toBe("137.5 KB"); + }); + + it("climbs through the units", () => { + expect(formatBytes(0)).toBe("0 B"); + expect(formatBytes(512)).toBe("512 B"); + expect(formatBytes(1024)).toBe("1.0 KB"); + expect(formatBytes(2097152)).toBe("2.0 MB"); + expect(formatBytes(5_368_709_120)).toBe("5.0 GB"); + }); + + it("keeps one decimal at every size, so the reading stays comparable", () => { + expect(formatBytes(157_286_400)).toBe("150.0 MB"); + }); + + it.each([-1, Number.NaN, Number.POSITIVE_INFINITY])("survives %s", (value) => { + expect(typeof formatBytes(value)).toBe("string"); + }); +}); + +describe("usageRatio", () => { + it("measures from the raw bytes, not from rounded megabytes", () => { + // Rounding first gave 0/2 = 0%, and the bar stayed empty on a mailbox + // that was really 6.7% full. + expect(usageRatio(140771, 2097152)).toBeCloseTo(0.0671, 3); + }); + + // Zero means unlimited; dividing by it reported every unlimited mailbox as + // over its limit. + it("never calls an unlimited mailbox full", () => { + expect(usageRatio(5_000_000, 0)).toBe(0); + }); + + it("stops at full rather than overflowing the bar", () => { + expect(usageRatio(3_000_000, 2_000_000)).toBe(1); + }); +}); diff --git a/services/webmail/src/lib/formatBytes.ts b/services/webmail/src/lib/formatBytes.ts new file mode 100644 index 0000000..4e26318 --- /dev/null +++ b/services/webmail/src/lib/formatBytes.ts @@ -0,0 +1,38 @@ +/** + * A byte count as a person reads it. + * + * The admin list rounded bytes to whole megabytes, so a mailbox holding + * 137.5 KB displayed as "0 / 2 MB" and its usage bar sat empty - the figure + * was right in the database and lost on the way to the screen. + */ +export function formatBytes(bytes: number): string { + const value = Number(bytes); + if (!Number.isFinite(value) || value <= 0) return "0 B"; + + const units = ["B", "KB", "MB", "GB", "TB"]; + let n = value; + let unit = 0; + while (n >= 1024 && unit < units.length - 1) { + n /= 1024; + unit++; + } + + // Bytes are whole things; above that always one decimal, which is what + // tells 137.5 KB from 137 KB. Dropping it on larger numbers would have hid + // exactly the digit that made the reading recognisable. + const rendered = unit === 0 ? String(Math.round(n)) : n.toFixed(1); + return `${rendered} ${units[unit]}`; +} + +/** + * How full a mailbox is, as a percentage, computed from the raw byte counts. + * + * A quota of zero means unlimited, so it is never full - dividing by it would + * report every unlimited mailbox as over its limit. + */ +export function usageRatio(usedBytes: number, quotaBytes: number): number { + const used = Number(usedBytes); + const quota = Number(quotaBytes); + if (!Number.isFinite(used) || !Number.isFinite(quota) || quota <= 0 || used <= 0) return 0; + return Math.min(used / quota, 1); +} diff --git a/services/webmail/src/lib/roleLabel.ts b/services/webmail/src/lib/roleLabel.ts new file mode 100644 index 0000000..ca5db6c --- /dev/null +++ b/services/webmail/src/lib/roleLabel.ts @@ -0,0 +1,18 @@ +/** + * A role as a person reads it. + * + * The console printed the stored constants - USER, DOMAIN_ADMIN, SUPER_ADMIN - + * straight onto the screen, so those three words stayed in English whatever + * the interface was set to. + */ +export function roleLabel(t: (key: string) => string, role: string): string { + const known: Record = { + USER: "roles.user", + DOMAIN_ADMIN: "roles.domainAdmin", + SUPER_ADMIN: "roles.superAdmin", + }; + const key = known[String(role).toUpperCase()]; + // An unknown role is shown as stored rather than as a missing-translation + // marker: whoever sees it needs to know what the database actually holds. + return key ? t(key) : String(role); +}