diff --git a/modules/gitea/client.ts b/modules/gitea/client.ts index d510b10..06cbb7d 100644 --- a/modules/gitea/client.ts +++ b/modules/gitea/client.ts @@ -390,7 +390,7 @@ export class GiteaAdmin { } private async findTeam(org: string, team: string): Promise { - const res = await this.request(`/orgs/${encodeURIComponent(org)}/teams`); + const res = await this.request(`/orgs/${encodeURIComponent(org)}/teams?limit=50`); if (res.status !== 200) return null; const match = (res.body as any[] | null)?.find((t) => t?.name === team); return match ? Number(match.id) : null; @@ -414,7 +414,9 @@ export class GiteaAdmin { const patch = await this.request(`/admin/users/${encodeURIComponent(username)}`, { method: "PATCH", // login_name is required by the admin edit endpoint; for a local user it is the username. - body: JSON.stringify({ login_name: username, password, must_change_password: false }), + // active and prohibit_login: a deactivated or login-prohibited user is refused like a wrong + // password, so the provisioner's check reports it lost; applying again must undo both. + body: JSON.stringify({ login_name: username, password, must_change_password: false, active: true, prohibit_login: false }), }); if (patch.status === 200) return; GiteaAdmin.fail(`/admin/users/${username}`, patch); @@ -445,7 +447,7 @@ export class GiteaAdmin { }); if (me.status === 401 || me.status === 403) return false; if (me.status !== 200) throw new Error(`Gitea GET /user as ${username}: ${me.status}`); - const teams = await this.request(`/orgs/${encodeURIComponent(org)}/teams`); + const teams = await this.request(`/orgs/${encodeURIComponent(org)}/teams?limit=50`); if (teams.status === 404) return false; if (teams.status !== 200) GiteaAdmin.fail(`/orgs/${org}/teams`, teams); const found = (teams.body as { id: number; name: string }[]).find((t) => t.name === team); diff --git a/modules/mailu/client.ts b/modules/mailu/client.ts index e1d1291..08c54b4 100644 --- a/modules/mailu/client.ts +++ b/modules/mailu/client.ts @@ -127,7 +127,9 @@ export class MailuClient { } async changePassword(email: string, password: string): Promise { - await this.api("PATCH", `/user/${encodeURIComponent(email)}`, { raw_password: password }); + // enabled: a disabled mailbox is what the provisioner's check reports as lost, so applying the + // mesh's password again also enables it; otherwise the two would disagree for ever. + await this.api("PATCH", `/user/${encodeURIComponent(email)}`, { raw_password: password, enabled: true }); } async deleteUser(email: string): Promise { @@ -135,34 +137,24 @@ export class MailuClient { } /** - * Whether a mailbox exists, is enabled, and accepts exactly this password. Read-only. Existence - * from the admin API; the password from `doveadm auth test` in the imap container, which is how - * the mail server itself authenticates, and which exits 77 for a refused login. The password - * reaches doveadm through the exec's environment, never the host's argv. An unreachable API or - * container rejects (novox/hq issue 120). + * Whether a mailbox exists and is enabled. Read-only, through the admin API. + * + * **The password is not checked.** Mailu authenticates in its admin service, behind the front; + * the imap server's own password database accepts any password from Mailu's subnet, so asking it + * (`doveadm auth test`) proves nothing, or refuses everyone. A lost or disabled mailbox is caught; + * a password changed by hand is not (novox/hq issue 120). */ - async holdsUser(email: string, password: string): Promise { + async holdsUser(email: string): Promise { const res = await fetch(`${this.baseUrl}/user/${encodeURIComponent(email)}`, { headers: { Authorization: this.apiKey, Accept: "application/json" }, }); if (res.status === 404) return false; if (!res.ok) throw new Error(`Mailu API GET /user/${email}: ${res.status} ${await res.text()}`); const user = (await res.json()) as { enabled?: boolean }; - if (user.enabled === false) return false; - try { - await run( - "docker", - ["exec", "-e", "MESH_USER", "-e", "MESH_PW", this.imapContainer, - "sh", "-c", 'doveadm auth test "$MESH_USER" "$MESH_PW"'], - { env: { ...process.env, MESH_USER: email, MESH_PW: password }, timeout: 30_000 }, - ); - return true; - } catch (err) { - if ((err as { code?: number }).code === 77) return false; - throw err; - } + return user.enabled !== false; } + async listAliases(): Promise { const aliases = await this.api("GET", "/alias"); return (aliases ?? []).map((a) => ({ diff --git a/modules/mailu/provisioner/index.ts b/modules/mailu/provisioner/index.ts index 27ecb37..41232bb 100644 --- a/modules/mailu/provisioner/index.ts +++ b/modules/mailu/provisioner/index.ts @@ -65,6 +65,6 @@ runProvisioner("smtp", { // Asked every minute by the harness: whether the backend still holds this consumer exactly as // the mesh gave it, so a login lost behind the provisioner's back is made again (novox/hq issue 120). async holds(p: Provision): Promise { - return mailu.holdsUser(addressOf(p), p.password); + return mailu.holdsUser(addressOf(p)); }, }); diff --git a/modules/mongodb/client.ts b/modules/mongodb/client.ts index f0fb4de..ae8c666 100644 --- a/modules/mongodb/client.ts +++ b/modules/mongodb/client.ts @@ -115,21 +115,23 @@ print(EJSON.stringify({ ok: 1 })); * authentication failure or a missing role; an unreachable server rejects (novox/hq issue 120). */ async canAuthenticateAs(database: string, user: string, password: string): Promise { - const uri = - `mongodb://${encodeURIComponent(user)}:${encodeURIComponent(password)}@${this.conn.host}:${this.conn.port}` + - `/${encodeURIComponent(database)}?authSource=${encodeURIComponent(database)}&serverSelectionTimeoutMS=10000`; + // Connected without credentials, then authenticated inside the eval from the environment, so + // the consumer's password is neither on argv nor in the message of a failed command. + const uri = `mongodb://${this.conn.host}:${this.conn.port}/?serverSelectionTimeoutMS=10000`; + const js = + "const t = db.getSiblingDB(process.env.MESH_HOLDS_DB);" + + "t.auth(process.env.MESH_HOLDS_USER, process.env.MESH_HOLDS_PW);" + + "print(EJSON.stringify(t.runCommand({ connectionStatus: 1 }).authInfo.authenticatedUserRoles))"; let stdout: string; try { - ({ stdout } = await run( - "mongosh", - [uri, "--quiet", "--eval", - "print(EJSON.stringify(db.runCommand({ connectionStatus: 1 }).authInfo.authenticatedUserRoles))"], - { timeout: 30_000 }, - )); + ({ stdout } = await run("mongosh", [uri, "--quiet", "--eval", js], { + env: { ...process.env, MESH_HOLDS_DB: database, MESH_HOLDS_USER: user, MESH_HOLDS_PW: password }, + timeout: 30_000, + })); } catch (err) { const text = `${(err as { stderr?: string }).stderr ?? ""}${(err as { stdout?: string }).stdout ?? ""}`; if (/Authentication failed|AuthenticationFailed/i.test(text)) return false; - throw err; + throw new Error(`mongosh could not check ${user}: ${text.trim().slice(0, 500) || String((err as Error).message).split("\n")[0]}`); } const roles = JSON.parse(stdout.trim()) as { role: string; db: string }[]; return roles.some((r) => r.role === "dbOwner" && r.db === database); diff --git a/modules/mosquitto/client.ts b/modules/mosquitto/client.ts index 08044ba..d6fb10e 100644 --- a/modules/mosquitto/client.ts +++ b/modules/mosquitto/client.ts @@ -88,9 +88,20 @@ export class MosquittoClient { "-u", this.conn.adminUser, "-P", this.conn.adminPassword, ]; - const { stdout, stderr } = await run("mosquitto_ctrl", [...base, "dynsec", ...args], { - maxBuffer: 16 << 20, - }); + let stdout: string; + let stderr: string; + try { + ({ stdout, stderr } = await run("mosquitto_ctrl", [...base, "dynsec", ...args], { + maxBuffer: 16 << 20, + timeout: 30_000, + })); + } catch (err) { + // A failed run's message repeats its argv, the admin password (-P) included; say what failed + // without it. + const e = err as { code?: unknown; signal?: unknown; stderr?: string; stdout?: string }; + const detail = `${e.stderr ?? ""}${e.stdout ?? ""}`.trim().slice(0, 500); + throw new Error(`mosquitto_ctrl dynsec ${args[0] ?? ""} could not run (${e.code ?? e.signal ?? "error"}): ${detail}`); + } const failure = ctlError(`${stdout}\n${stderr}`); if (failure) { throw new Error(`mosquitto_ctrl dynsec ${args[0] ?? ""} failed: ${failure}`); @@ -141,6 +152,11 @@ export class MosquittoClient { if (await this.clientExists(username)) { await this.ctl("setClientPassword", username, password); + // A disabled client is refused like a wrong password, so the check the provisioner runs + // reports it lost; applying again must enable it, or the two would disagree for ever. + if (/Disabled:\s*true/i.test(await this.ctl("getClient", username))) { + await this.ctl("enableClient", username); + } } else { await this.ctl("createClient", username, "-p", password); } @@ -174,7 +190,16 @@ export class MosquittoClient { const code = await mqttConnack(this.conn.host, this.conn.port, username, password); if (code === 4 || code === 5) return false; if (code !== 0) throw new Error(`mosquitto refused ${username} with CONNACK ${code}`); - return this.clientHasRole(username, username); + // The role, asked directly: only "not found" means absent. Any other failure to ask rejects, + // unlike clientHasRole, which reads every failure as "no role". + let out: string; + try { + out = await this.ctl("getClient", username); + } catch (err) { + if (/not\s*found|does not exist|no such/i.test(String(err))) return false; + throw err; + } + return new RegExp(`(^|\\s)${escapeRegExp(username)}\\s+\\(priority`, "m").test(out); } /** Remove a client and the per-client role created for it, idempotently. */ diff --git a/modules/mssql/client.ts b/modules/mssql/client.ts index 6a0ac07..e1802a4 100644 --- a/modules/mssql/client.ts +++ b/modules/mssql/client.ts @@ -75,14 +75,18 @@ export class MssqlClient { * prints (split across output lines for a large result, and reassembled here) is parsed. An * empty result yields no output at all — an empty array. */ - async query(select: string, database = "master"): Promise[]> { + async query( + select: string, + database = "master", + variables: Record = {}, + ): Promise[]> { const wrapped = `SET NOCOUNT ON;\n${stripTrailingSemis(select)}\nFOR JSON PATH, INCLUDE_NULL_VALUES;`; - const stdout = await this.sqlcmd(wrapped, database); + const stdout = await this.sqlcmd(wrapped, database, variables); return parseJsonRows(stdout); } /** The one execution boundary: invoke `sqlcmd` and return its concatenated stdout. */ - private async sqlcmd(sql: string, database: string): Promise { + private async sqlcmd(sql: string, database: string, variables: Record = {}): Promise { // `-h -1` drops the column-header rule; `-y 0`/`-Y 0` lift the display-width cap so a long // JSON document is not truncated; `-W` trims trailing whitespace so the JSON chunks rejoin // cleanly. sqlcmd from the mssql-tools ships in the runtime container, the way `psql` ships @@ -101,7 +105,9 @@ export class MssqlClient { "-W", "-Q", sql, ], - { env: { ...process.env, SQLCMDPASSWORD: this.conn.password }, maxBuffer: 16 << 20 }, + // `variables` reach sqlcmd as environment variables, which it substitutes as `$(NAME)` scripting + // variables: a value that must not appear on argv, or in the message of a failed command. + { env: { ...process.env, ...variables, SQLCMDPASSWORD: this.conn.password }, maxBuffer: 16 << 20 }, ); return stdout; } @@ -121,6 +127,9 @@ export class MssqlClient { ); } else { await this.exec(`ALTER LOGIN ${ident(login)} WITH PASSWORD = ${literal(password)}`); + // A disabled login is refused like a wrong password; the check the provisioner runs reports it + // lost, so applying again must enable it or the two would disagree for ever. + await this.exec(`ALTER LOGIN ${ident(login)} ENABLE`); } const dbs = await this.query( @@ -138,6 +147,10 @@ export class MssqlClient { ); if (users.length === 0) { await this.exec(`CREATE USER ${ident(login)} FOR LOGIN ${ident(login)}`, database); + } else { + // Re-point an existing user at the login. A database restored from elsewhere keeps its user + // under the old login's SID, orphaned; this maps it back, and is a no-op when it already is. + await this.exec(`ALTER USER ${ident(login)} WITH LOGIN = ${ident(login)}`, database); } await this.exec(`ALTER ROLE db_owner ADD MEMBER ${ident(login)}`, database); } @@ -148,14 +161,24 @@ export class MssqlClient { * nothing logs in and no failed-login is recorded (novox/hq issue 120). */ async holdsLogin(database: string, login: string, password: string): Promise { + // The password reaches sqlcmd as a scripting variable from the environment, never inside the + // query text, so it is neither on argv nor in the message of a failed command. It is the mesh's + // minted value, which carries no quote. const server = await this.query( `SELECT CAST(CASE WHEN EXISTS (SELECT 1 FROM sys.sql_logins WHERE name = ${literal(login)} ` + - `AND is_disabled = 0 AND PWDCOMPARE(${literal(password)}, password_hash) = 1) ` + + `AND is_disabled = 0 AND PWDCOMPARE(N'$(MESHHOLDSPW)', password_hash) = 1) ` + `AND DB_ID(${literal(database)}) IS NOT NULL THEN 1 ELSE 0 END AS int) AS ok`, + "master", + { MESHHOLDSPW: password }, ); if (Number(server[0]?.ok) !== 1) return false; + // The user must be this login's, by SID, and a db_owner. A user orphaned by a restore has the + // right name and the wrong SID, and cannot be reached through the login. const owner = await this.query( - `SELECT CAST(IS_ROLEMEMBER('db_owner', ${literal(login)}) AS int) AS ok`, + `SELECT CAST(CASE WHEN EXISTS (SELECT 1 FROM sys.database_principals dp ` + + `JOIN sys.server_principals sp ON dp.sid = sp.sid ` + + `WHERE dp.name = ${literal(login)} AND sp.name = ${literal(login)}) ` + + `AND IS_ROLEMEMBER('db_owner', ${literal(login)}) = 1 THEN 1 ELSE 0 END AS int) AS ok`, database, ); return Number(owner[0]?.ok) === 1; diff --git a/modules/postgres/client.ts b/modules/postgres/client.ts index f16af8b..ee4d6a9 100644 --- a/modules/postgres/client.ts +++ b/modules/postgres/client.ts @@ -88,9 +88,11 @@ export class PostgresClient { async createDatabaseAndRole(database: string, role: string, password: string): Promise { const roles = await this.query("SELECT 1 FROM pg_roles WHERE rolname = " + literal(role)); if (roles.rows.length === 0) { - await this.query(`CREATE ROLE ${ident(role)} WITH LOGIN PASSWORD ${literal(password)}`); + await this.query(`CREATE ROLE ${ident(role)} WITH LOGIN PASSWORD ${literal(password)} VALID UNTIL 'infinity'`); } else { - await this.query(`ALTER ROLE ${ident(role)} WITH LOGIN PASSWORD ${literal(password)}`); + // VALID UNTIL 'infinity': a password that expired is refused like a wrong one, so the check the + // provisioner runs would report it lost, and only clearing the expiry makes applying it again work. + await this.query(`ALTER ROLE ${ident(role)} WITH LOGIN PASSWORD ${literal(password)} VALID UNTIL 'infinity'`); } const dbs = await this.query("SELECT 1 FROM pg_database WHERE datname = " + literal(database)); if (dbs.rows.length === 0) {