fix: honor shared credentials on shared hosts (#1730)

* fix: honor shared credentials on shared hosts and in personal SSH auth

* fix: snapshot shared SSH auth from a credential shared with the host owner
This commit is contained in:
Jonathan Rehuel Lewerissa authored and GitHub committed 2026-10-29 12:37:48 -05:00
1 parent 3173f45e7e
commit ffe757f91d
6 files changed
+133 -23

No files matched your search

@@ -8,7 +8,7 @@ const state = vi.hoisted(() => ({
isShared: true, isShared: true,
isAdminBypass: false, isAdminBypass: false,
}, },
credentialOwned: true, credentialUsable: true,
credentialId: 7 as number | null, credentialId: 7 as number | null,
writes: [] as Array<{ protocol: string; credentialId: number | null }>, writes: [] as Array<{ protocol: string; credentialId: number | null }>,
auditCalls: [] as Array<Record<string, unknown>>, auditCalls: [] as Array<Record<string, unknown>>,
@@ -70,11 +70,12 @@ vi.mock("../../../utils/permission-manager.js", () => ({
}, },
})); }));
vi.mock("../../../hosts/usable-credential.js", () => ({
findUsableCredential: async (credentialId: number) =>
state.credentialUsable ? { id: credentialId } : null,
}));
vi.mock("../../../database/repositories/factory.js", () => ({ vi.mock("../../../database/repositories/factory.js", () => ({
createCurrentCredentialRepository: () => ({
findByIdForUser: async () =>
state.credentialOwned ? { id: state.credentialId } : null,
}),
createCurrentSharedHostAuthOverrideRepository: () => ({ createCurrentSharedHostAuthOverrideRepository: () => ({
findCredentialId: async () => state.credentialId, findCredentialId: async () => state.credentialId,
setCredential: async ( setCredential: async (
@@ -206,7 +207,7 @@ describe("shared host authentication override routes", () => {
isShared: true, isShared: true,
isAdminBypass: false, isAdminBypass: false,
}; };
state.credentialOwned = true; state.credentialUsable = true;
state.credentialId = 7; state.credentialId = 7;
state.writes = []; state.writes = [];
state.auditCalls = []; state.auditCalls = [];
@@ -254,7 +255,7 @@ describe("shared host authentication override routes", () => {
const invalidResponse = await invoke("put", { credentialId: 0 }); const invalidResponse = await invoke("put", { credentialId: 0 });
expect(invalidResponse.status).toBe(400); expect(invalidResponse.status).toBe(400);
state.credentialOwned = false; state.credentialUsable = false;
const foreignResponse = await invoke("put", { credentialId: 99 }); const foreignResponse = await invoke("put", { credentialId: 99 });
expect(foreignResponse.status).toBe(404); expect(foreignResponse.status).toBe(404);
@@ -6,6 +6,7 @@ const state = vi.hoisted(() => ({
isAdminBypass: false, isAdminBypass: false,
overrideCredentialId: null as number | null, overrideCredentialId: null as number | null,
credentials: new Map<string, Record<string, unknown>>(), credentials: new Map<string, Record<string, unknown>>(),
sharedCredentials: new Map<string, Record<string, unknown>>(),
auditCalls: [] as Record<string, unknown>[], auditCalls: [] as Record<string, unknown>[],
sharedSecret: null as Record<string, unknown> | null, sharedSecret: null as Record<string, unknown> | null,
activeHostAccessId: null as number | null, activeHostAccessId: null as number | null,
@@ -37,6 +38,13 @@ vi.mock("../../database/repositories/factory.js", () => ({
}), }),
})); }));
vi.mock("../../hosts/usable-credential.js", () => ({
findUsableCredential: async (credentialId: number, userId: string) =>
state.credentials.get(`${credentialId}:${userId}`) ??
state.sharedCredentials.get(`${credentialId}:${userId}`) ??
null,
}));
vi.mock("../../utils/audit-logger.js", () => ({ vi.mock("../../utils/audit-logger.js", () => ({
logAudit: async (params: Record<string, unknown>) => { logAudit: async (params: Record<string, unknown>) => {
state.auditCalls.push(params); state.auditCalls.push(params);
@@ -125,6 +133,7 @@ beforeEach(() => {
state.isAdminBypass = false; state.isAdminBypass = false;
state.overrideCredentialId = null; state.overrideCredentialId = null;
state.credentials.clear(); state.credentials.clear();
state.sharedCredentials.clear();
state.auditCalls = []; state.auditCalls = [];
state.sharedSecret = null; state.sharedSecret = null;
state.activeHostAccessId = null; state.activeHostAccessId = null;
@@ -376,6 +385,61 @@ describe("resolveHostById", () => {
expect(host.username).toBe("shared-login"); expect(host.username).toBe("shared-login");
}); });
it("uses a credential shared with the recipient as their personal override", async () => {
state.overrideCredentialId = 7;
state.sharedCredentials.set("7:recipient", {
id: 7,
username: "shared-cred-user",
password: "shared-cred-pass",
privateKey: null,
key: null,
keyPassword: null,
keyType: null,
});
const host = (await resolveHostById(42, "recipient")) as Record<
string,
unknown
>;
expect(host.username).toBe("shared-cred-user");
expect(host.password).toBe("shared-cred-pass");
});
it("uses the host's credential when it is also shared with the recipient", async () => {
state.host = baseHost({
authType: "credential",
password: null,
credentialId: 1,
});
state.sharedCredentials.set("1:recipient", {
id: 1,
username: "deploy",
password: null,
privateKey: "OWNER-KEY-COPY",
key: null,
keyPassword: null,
keyType: "ssh-ed25519",
});
const host = (await resolveHostById(42, "recipient")) as Record<
string,
unknown
>;
expect(host.authType).toBe("key");
expect(host.key).toBe("OWNER-KEY-COPY");
expect(host.username).toBe("deploy");
});
it("denies a recipient when the host's credential is not shared with them", async () => {
state.host = baseHost({
authType: "credential",
password: null,
credentialId: 1,
});
expect(await resolveHostById(42, "recipient")).toBeNull();
});
it("denies a non-owner when a secret-bearing host has no personal credential", async () => { it("denies a non-owner when a secret-bearing host has no personal credential", async () => {
expect(await resolveHostById(42, "recipient")).toBeNull(); expect(await resolveHostById(42, "recipient")).toBeNull();
}); });
@@ -17,8 +17,14 @@ const state = vi.hoisted(() => ({
})); }));
vi.mock("../../hosts/usable-credential.js", () => ({ vi.mock("../../hosts/usable-credential.js", () => ({
findUsableCredential: async (credentialId: number) => findUsableCredential: async (credentialId: number, userId: string) => {
state.credentials.get(credentialId) ?? null, const credential = state.credentials.get(credentialId);
if (!credential) return null;
const sharedWith = (credential.sharedWith as string[] | undefined) ?? [];
return credential.userId === userId || sharedWith.includes(userId)
? credential
: null;
},
})); }));
vi.mock("../../database/repositories/factory.js", () => ({ vi.mock("../../database/repositories/factory.js", () => ({
@@ -29,8 +35,10 @@ vi.mock("../../database/repositories/factory.js", () => ({
findHostById: async (hostId: number) => state.hosts.get(hostId) ?? null, findHostById: async (hostId: number) => state.hosts.get(hostId) ?? null,
findHostOwnerId: async (hostId: number) => findHostOwnerId: async (hostId: number) =>
(state.hosts.get(hostId)?.userId as string) ?? null, (state.hosts.get(hostId)?.userId as string) ?? null,
findCredentialByIdForUser: async (credentialId: number) => findCredentialByIdForUser: async (credentialId: number, userId: string) =>
state.credentials.get(credentialId) ?? null, state.credentials.get(credentialId)?.userId === userId
? state.credentials.get(credentialId)
: null,
}), }),
createCurrentSharedHostSecretsRepository: () => ({ createCurrentSharedHostSecretsRepository: () => ({
upsert: async (row: Record<string, unknown>) => { upsert: async (row: Record<string, unknown>) => {
@@ -260,6 +268,39 @@ describe("SharedHostSecretsManager", () => {
}); });
}); });
it("snapshots SSH auth from a credential shared with the host owner", async () => {
state.credentials.set(123, {
id: 123,
userId: "cred-owner",
sharedWith: ["owner"],
username: "cred-user",
authType: "password",
password: "cred-pass",
privateKey: null,
key: null,
keyPassword: null,
keyType: null,
});
state.hosts.set(
42,
baseHost({
authType: "credential",
credentialId: 123,
shareSshAuth: true,
password: null,
}),
);
await manager.snapshotForUser(7, 42, "target", "owner");
expect(state.secretRows.map((row) => row.protocol)).toEqual(["ssh"]);
expect(await manager.getSecretForUser(42, "target", "ssh")).toMatchObject({
username: "cred-user",
authType: "password",
password: "cred-pass",
});
});
it("snapshots a credential-backed protocol login from the credential", async () => { it("snapshots a credential-backed protocol login from the credential", async () => {
state.credentials.set(55, { state.credentials.set(55, {
id: 55, id: 55,
@@ -1,10 +1,10 @@
import type { AuthOverrideProtocol } from "../../types/auth-protocols.js"; import type { AuthOverrideProtocol } from "../../types/auth-protocols.js";
import { isAuthOverrideProtocol } from "../hosts/protocol-auth/registry.js"; import { isAuthOverrideProtocol } from "../hosts/protocol-auth/registry.js";
import { import {
createCurrentCredentialRepository,
createCurrentSharedHostAuthOverrideRepository, createCurrentSharedHostAuthOverrideRepository,
createCurrentUserRepository, createCurrentUserRepository,
} from "../database/repositories/factory.js"; } from "../database/repositories/factory.js";
import { findUsableCredential } from "../hosts/usable-credential.js";
import { logAudit } from "./audit-logger.js"; import { logAudit } from "./audit-logger.js";
import { PermissionManager } from "./permission-manager.js"; import { PermissionManager } from "./permission-manager.js";
@@ -60,11 +60,7 @@ export class SharedHostAuthOverrideService {
await this.requireSharedHostAccess(hostId, userId); await this.requireSharedHostAccess(hostId, userId);
if (credentialId !== null) { if (credentialId !== null) {
const credential = const credential = await findUsableCredential(credentialId, userId);
await createCurrentCredentialRepository().findByIdForUser(
userId,
credentialId,
);
if (!credential) { if (!credential) {
throw new SharedHostAuthOverrideServiceError( throw new SharedHostAuthOverrideServiceError(
"Credential not found", "Credential not found",
+13 -4
View File
@@ -12,6 +12,7 @@ import type {
} from "../database/repositories/host-resolution-repository.js"; } from "../database/repositories/host-resolution-repository.js";
import { getSshAuthProvider } from "../hosts/connect/auth-provider-registry.js"; import { getSshAuthProvider } from "../hosts/connect/auth-provider-registry.js";
import { ensureCoreSshAuthProviders } from "../hosts/connect/core-providers.js"; import { ensureCoreSshAuthProviders } from "../hosts/connect/core-providers.js";
import { findUsableCredential } from "../hosts/usable-credential.js";
import { import {
SharedHostSecretsManager, SharedHostSecretsManager,
type SharedSecretData, type SharedSecretData,
@@ -80,10 +81,7 @@ export async function resolveRecipientSharedHostAuthentication(
} }
if (overrideCredentialId) { if (overrideCredentialId) {
const credential = await repository.findCredentialByIdForUser( const credential = await findUsableCredential(overrideCredentialId, userId);
overrideCredentialId,
userId,
);
if (credential) { if (credential) {
return { return {
source: "personal-override", source: "personal-override",
@@ -145,6 +143,17 @@ export async function resolveRecipientSharedHostAuthentication(
} }
} }
if (protocol === "ssh" && host.credentialId) {
const credential = await findUsableCredential(host.credentialId, userId);
if (credential) {
return {
source: "personal-override",
credentialId: host.credentialId,
credential,
};
}
}
return requiresPersonalHostAuthentication(host, protocol) return requiresPersonalHostAuthentication(host, protocol)
? { source: "required" } ? { source: "required" }
: { source: "secretless" }; : { source: "secretless" };
@@ -381,12 +381,11 @@ class SharedHostSecretsManager {
host: HostResolutionHostRecord, host: HostResolutionHostRecord,
ownerId: string, ownerId: string,
): Promise<ProtocolSnapshot[]> { ): Promise<ProtocolSnapshot[]> {
const repository = createCurrentHostResolutionRepository();
const snapshots: ProtocolSnapshot[] = []; const snapshots: ProtocolSnapshot[] = [];
if (sshEnabled(host) && host.shareSshAuth) { if (sshEnabled(host) && host.shareSshAuth) {
if (host.credentialId) { if (host.credentialId) {
const credential = await repository.findCredentialByIdForUser( const credential = await findUsableCredential(
host.credentialId, host.credentialId,
ownerId, ownerId,
); );