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

Merge pull request #336 from Coffey-Labs/fix/account-type-filter

Browse Source 
Filter accounts on @type, the name Stalwart uses
This commit is contained in:
jcoffey
2026-09-13 15:58:52 -07:00
committed by GitHub
parent b79db9098a 430fc2673c
commit 93c9660421
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,