coffey-labs/ihasmail
1
0
Fork 0
Code Issues Pull Requests 1 Actions Packages Projects Releases 2 Wiki Activity

Filter accounts on @type, the name Stalwart uses

Browse Source 
The Accounts list sent x:Account/query with {"type": "User"}, and a live
0.16 server refuses it: "unsupportedFilter - type". The registry keys a
filter by the property's name on the object, which for the discriminator is
@type, so the whole list failed to load. {"@type": "User"} is accepted.

The mock took the wrong name without complaint, which is how it shipped. It
now refuses any filter name the real server does not index for that object,
answering the way Stalwart does.
This commit is contained in:
jcoffey-dev
2026-09-13 15:56:16 -07:00
parent b79db9098a
commit 430fc2673c
4 changed files with 45 additions and 14 deletions
Download Patch File Download Diff File Expand all files Collapse all files
server/src/mock/directory.test.ts
+11 -4
View File
@@ -21,7 +21,7 @@ test("an ordinary user is refused the directory outright", () => {
test("helpdesk may read and edit but not create or delete", () => {
const dir = make("helpdesk");
const { ids } = dir.handlers["x:Account/query"]!({ filter: { type: "User" } }) as { ids: string[] };
const { ids } = dir.handlers["x:Account/query"]!({ filter: { "@type": "User" } }) as { ids: string[] };
assert.ok(ids.length > 20);
assert.throws(() => dir.handlers["x:Account/set"]!({ create: { n: { name: "x", domainId: "d1" } } }), (e: Refused) => e.type === "forbidden");
assert.throws(() => dir.handlers["x:Account/set"]!({ destroy: [ids[0]] }), (e: Refused) => e.type === "forbidden");
@@ -29,11 +29,11 @@ test("helpdesk may read and edit but not create or delete", () => {
test("queries page, count and match text the way the client asks", () => {
const dir = make("admin");
const all = dir.handlers["x:Account/query"]!({ filter: { type: "User" }, calculateTotal: true }) as { ids: string[]; total: number };
const page = dir.handlers["x:Account/query"]!({ filter: { type: "User" }, position: 10, limit: 5, calculateTotal: true }) as { ids: string[]; total: number };
const all = dir.handlers["x:Account/query"]!({ filter: { "@type": "User" }, calculateTotal: true }) as { ids: string[]; total: number };
const page = dir.handlers["x:Account/query"]!({ filter: { "@type": "User" }, position: 10, limit: 5, calculateTotal: true }) as { ids: string[]; total: number };
assert.equal(page.total, all.total);
assert.deepEqual(page.ids, all.ids.slice(10, 15));
const ada = dir.handlers["x:Account/query"]!({ filter: { type: "User", text: "lovelace" } }) as { ids: string[] };
const ada = dir.handlers["x:Account/query"]!({ filter: { "@type": "User", text: "lovelace" } }) as { ids: string[] };
assert.equal(ada.ids.length, 1);
assert.throws(() => dir.handlers["x:Account/query"]!({ filter: { operator: "OR", conditions: [] } }), (e: Refused) => e.type === "unsupportedFilter");
});
@@ -93,3 +93,10 @@ test("a domain's zone file is computed on read, with long keys split as the serv
assert.match(zone, /IN MX 10 /);
assert.match(zone, /_domainkey\.example\.com\. IN TXT \(\n {4}"/);
});
test("a filter on a name the registry does not index is refused, as the live server refuses it", () => {
const dir = make("admin");
// Seen on a live 0.16 server: "x:Account/query: unsupportedFilter - type".
assert.throws(() => dir.handlers["x:Account/query"]!({ filter: { type: "User" } }), (e: Refused) => e.type === "unsupportedFilter" && e.message === "type");
assert.doesNotThrow(() => dir.handlers["x:Account/query"]!({ filter: { "@type": "Group", domainId: "d1", text: "x" } }));
});
server/src/mock/directory.ts
+15 -6
View File
@@ -197,10 +197,19 @@ export function createDirectory(opts: Options) {
return { accountId: opts.accountId, state: "1", list: found.map((x) => view(x, a.properties)), notFound: ids ? ids.filter((id) => !list.some((x) => x.id === id)) : [] };
};
const query = (list: () => Obj[], perm: string, match: (o: Obj, filter: Obj) => boolean) => (a: Obj) => {
/**
* A query, filtered only on what the real server indexes for that object.
* Any other name is refused the way Stalwart refuses it -- `unsupportedFilter`
* with the name as the whole description -- because a mock that took
* `{"type": "User"}` let exactly that ship, and the live server answers it
* with "unsupportedFilter - type".
*/
const query = (list: () => Obj[], perm: string, filterable: string[], match: (o: Obj, filter: Obj) => boolean) => (a: Obj) => {
demand(perm);
const filter = (a.filter as Obj | undefined) ?? {};
if ("operator" in filter) throw opts.fail("unsupportedFilter", "Only AND is supported in filters");
const unknown = Object.keys(filter).find((k) => !filterable.includes(k));
if (unknown) throw opts.fail("unsupportedFilter", unknown);
// Stalwart's default order is newest first, by id.
const rows = list().filter((o) => match(o, filter)).sort((x, y) => String(y.id).localeCompare(String(x.id), undefined, { numeric: true }));
const position = Math.max(0, Number(a.position ?? 0));
@@ -245,8 +254,8 @@ export function createDirectory(opts: Options) {
const handlers: Record<string, (a: Obj) => Obj> = {
"x:Account/get": get(accounts, "sysAccountGet"),
"x:Account/query": query(() => accounts, "sysAccountQuery", (o, f) =>
(f.type === undefined || o["@type"] === f.type) && (f.domainId === undefined || o.domainId === f.domainId) && matchText(o, f.text) && matchText(o, f.name)),
"x:Account/query": query(() => accounts, "sysAccountQuery", ["text", "@type", "domainId", "externalId", "memberGroupIds", "memberTenantId", "name"], (o, f) =>
(f["@type"] === undefined || o["@type"] === f["@type"]) && (f.domainId === undefined || o.domainId === f.domainId) && matchText(o, f.text) && matchText(o, f.name)),
"x:Account/set": (a) => {
const created: Obj = {};
const notCreated: Obj = {};
@@ -319,7 +328,7 @@ export function createDirectory(opts: Options) {
return { accountId: opts.accountId, oldState: "1", newState: "2", created, updated, destroyed, ...(Object.keys(notCreated).length ? { notCreated } : {}), ...(Object.keys(notUpdated).length ? { notUpdated } : {}), ...(Object.keys(notDestroyed).length ? { notDestroyed } : {}) };
},
"x:Domain/get": get(domains, "sysDomainGet"),
"x:Domain/query": query(() => domains, "sysDomainQuery", (o, f) => matchText(o, f.text) && matchText(o, f.name)),
"x:Domain/query": query(() => domains, "sysDomainQuery", ["text", "aliases", "memberTenantId", "name"], (o, f) => matchText(o, f.text) && matchText(o, f.name)),
"x:Domain/set": (a) => {
const created: Obj = {};
const notCreated: Obj = {};
@@ -366,7 +375,7 @@ export function createDirectory(opts: Options) {
return { accountId: opts.accountId, oldState: "1", newState: "2", created, updated, destroyed, ...(Object.keys(notCreated).length ? { notCreated } : {}), ...(Object.keys(notUpdated).length ? { notUpdated } : {}), ...(Object.keys(notDestroyed).length ? { notDestroyed } : {}) };
},
"x:DkimSignature/get": get(dkimKeys, "sysDkimSignatureGet"),
"x:DkimSignature/query": query(() => dkimKeys, "sysDkimSignatureQuery", (o, f) => f.domainId === undefined || o.domainId === f.domainId),
"x:DkimSignature/query": query(() => dkimKeys, "sysDkimSignatureQuery", ["domainId", "memberTenantId"], (o, f) => f.domainId === undefined || o.domainId === f.domainId),
"x:DkimSignature/set": (a) => {
const destroyed: string[] = [];
for (const id of (a.destroy as string[]) ?? []) {
@@ -382,7 +391,7 @@ export function createDirectory(opts: Options) {
return { accountId: opts.accountId, state: "1", list: ((a.ids as string[]) ?? ["ns1"]).filter((id) => id === "ns1").map((id) => ({ id, "@type": "Cloudflare", description: "Cloudflare (main zone)" })), notFound: [] };
},
"x:Role/get": get(roles, "sysRoleGet"),
"x:Role/query": query(() => roles, "sysRoleQuery", (o, f) => matchText(o, f.description)),
"x:Role/query": query(() => roles, "sysRoleQuery", ["text", "description", "memberTenantId"], (o, f) => matchText(o, f.description)),
};
return { handlers, permissions: [...permissions], accounts };
web/src/lib/__tests__/adminDirectory.test.ts
+14 -2
View File
@@ -1,5 +1,6 @@
import { describe, expect, it } from "vitest";
import { aliasList, describeDirectoryError, DirectoryError, hasPassword, passwordPatch, quotasWithDisk } from "@/lib/adminDirectory";
import { describe, expect, it, vi } from "vitest";
import { client } from "@/jmap/client";
import { aliasList, describeDirectoryError, DirectoryError, hasPassword, passwordPatch, queryAccounts, quotasWithDisk } from "@/lib/adminDirectory";
describe("setting a password", () => {
it("writes into the existing password credential, keeping its place", () => {
@@ -43,3 +44,14 @@ describe("explaining a refusal", () => {
expect(describeDirectoryError({ type: "forbidden", message: "x:Account/set: forbidden" })).toMatch(/refused/);
});
});
describe("the account query", () => {
it("filters on @type, the property's name on the object", async () => {
// A live 0.16 server answers a plain `type` with "unsupportedFilter - type"
// and fails the whole list, which is how this was found.
const call = vi.spyOn(client, "call").mockResolvedValue({ ids: [], total: 0 });
await queryAccounts({ type: "User", text: " ada ", position: 50, limit: 50 });
expect(call).toHaveBeenCalledWith("x:Account/query", { filter: { "@type": "User", text: "ada" }, position: 50, limit: 50, calculateTotal: true });
call.mockRestore();
});
});
web/src/lib/adminDirectory.ts
+5 -2
View File
@@ -19,7 +19,8 @@ import type { PermissionsMode, RoleDef, UserRoles } from "@/lib/adminAccess";
* `emailAddress` and `usedDiskQuota` are computed by the server.
* - Secrets read back masked. A new password is written to the existing
* password credential, so its id -- which OAuth tokens are tied to -- stays.
* - Filters are AND only, and the default order is newest first.
* - Filters are AND only, keyed by property name as it appears on the object
* (`@type`, not `type`), and the default order is newest first.
*
* Query and get are two requests rather than one with a result reference.
* Whether the registry methods resolve back-references has not been checked on
@@ -89,7 +90,9 @@ interface QueryResult {
}
export async function queryAccounts(opts: { type: "User" | "Group"; text?: string; position?: number; limit?: number }): Promise<{ ids: string[]; total: number }> {
const filter: Record<string, unknown> = { type: opts.type };
// The registry names the discriminator `@type`, as it is on the object. A
// plain `type` is not a property it knows and fails the whole query.
const filter: Record<string, unknown> = { "@type": opts.type };
if (opts.text?.trim()) filter.text = opts.text.trim();
const res = await client.call<QueryResult>("x:Account/query", {
filter,