fix: address onboarding review feedback
This commit is contained in:
1 parent
07770e862c
commit
d89bf25e21
72 files changed
+7783
-280
No files matched your search
@@ -49,7 +49,12 @@ export const getOnboardingClaimTokenFromCookieHeader = (cookieHeader: string | n
|
||||
const name = part.slice(0, separator).trim();
|
||||
if (name !== onboardingClaimCookieName) continue;
|
||||
const value = part.slice(separator + 1).trim();
|
||||
return value || undefined;
|
||||
if (!value) return undefined;
|
||||
try {
|
||||
return decodeURIComponent(value);
|
||||
} catch {
|
||||
return undefined;
|
||||
}
|
||||
}
|
||||
return undefined;
|
||||
};
|
||||
|
||||
@@ -19,8 +19,6 @@ const defaultIcon = "https://cdn.jsdelivr.net/gh/homarr-labs/dashboard-icons@mas
|
||||
|
||||
export const appRouter = createTRPCRouter({
|
||||
getPaginated: protectedProcedure
|
||||
.input(paginatedSchema)
|
||||
.output(z.object({ items: z.array(selectAppSchema), totalCount: z.number() }))
|
||||
.meta({
|
||||
openapi: {
|
||||
method: "GET",
|
||||
@@ -34,6 +32,8 @@ export const appRouter = createTRPCRouter({
|
||||
"List apps with pagination. OPTIONAL: search (string to filter by name), pageSize (number, default 10), page (number, default 1). All fields are optional — call with no arguments to get the first page",
|
||||
},
|
||||
})
|
||||
.input(paginatedSchema)
|
||||
.output(z.object({ items: z.array(selectAppSchema), totalCount: z.number() }))
|
||||
.query(async ({ input, ctx }) => {
|
||||
const whereQuery = input.search ? like(apps.name, `%${input.search.trim()}%`) : undefined;
|
||||
const totalCount = await ctx.db.$count(apps, whereQuery);
|
||||
|
||||
@@ -242,7 +242,6 @@ export const boardRouter = createTRPCRouter({
|
||||
id: true,
|
||||
name: true,
|
||||
image: true,
|
||||
email: true,
|
||||
},
|
||||
},
|
||||
userPermissions: {
|
||||
|
||||
@@ -7,7 +7,6 @@ export const nextOnboardingStepAsync = async (db: Database) => {
|
||||
const { current } = await getOnboardingOrFallbackAsync(db);
|
||||
if (current !== "start") return;
|
||||
const nextStep = await getNextOnboardingStepAsync(db);
|
||||
if (!nextStep) return;
|
||||
|
||||
await db.update(onboarding).set({ previousStep: current, step: nextStep });
|
||||
};
|
||||
|
||||
@@ -8,6 +8,8 @@ import { z } from "zod/v4";
|
||||
|
||||
import { createId } from "@homarr/common";
|
||||
import { encryptSecret } from "@homarr/common/server";
|
||||
import { createLogger } from "@homarr/core/infrastructure/logs";
|
||||
import { ErrorWithMetadata } from "@homarr/core/infrastructure/logs/error";
|
||||
import { handleTransactionsAsync, eq, inArray, like, or } from "@homarr/db";
|
||||
import { getServerSettingByKeyAsync } from "@homarr/db/queries";
|
||||
import {
|
||||
@@ -50,6 +52,8 @@ import { MissingSecretError, testConnectionAsync } from "../integration/integrat
|
||||
import { getOnboardingOrFallbackAsync, nextOnboardingStepAsync } from "./onboard-queries";
|
||||
import { probeRuntimeCapabilitiesAsync } from "./capability-probes";
|
||||
|
||||
const logger = createLogger({ module: "onboardRouter" });
|
||||
|
||||
interface PortInfo {
|
||||
IP?: string;
|
||||
PublicPort?: number;
|
||||
@@ -159,7 +163,7 @@ export const onboardRouter = createTRPCRouter({
|
||||
const empty = { integrations: [] as DiscoveredIntegration[], apps: [] as DiscoveredApp[] };
|
||||
|
||||
try {
|
||||
const { listDiscoveredContainersAsync, dockerLabels } = await import("@homarr/docker");
|
||||
const { createDockerSourceId, listDiscoveredContainersAsync, dockerLabels } = await import("@homarr/docker");
|
||||
const discovery = await listDiscoveredContainersAsync();
|
||||
const successfulHosts = discovery.hosts.filter((host) => host.status === "success");
|
||||
const unavailableHosts = discovery.hosts.filter((host) => host.status === "unavailable");
|
||||
@@ -169,14 +173,15 @@ export const onboardRouter = createTRPCRouter({
|
||||
.map((container) => ({ ...container, host: hostResult.host })),
|
||||
);
|
||||
const labeledContainerKeys = new Set(
|
||||
discovery.services.map((service) => `${service.host}:${service.containerId}`),
|
||||
discovery.services.map((service) => createDockerSourceId(service.host, service.containerId)),
|
||||
);
|
||||
const imageCandidates = containers.filter(
|
||||
(container) => !labeledContainerKeys.has(`${container.host}:${container.Id}`),
|
||||
);
|
||||
const likeQueries = imageCandidates.map((container) =>
|
||||
like(icons.name, `%${extractContainerImageName(container.Image)}%`),
|
||||
(container) => !labeledContainerKeys.has(createDockerSourceId(container.host, container.Id)),
|
||||
);
|
||||
const candidateImageNames = [
|
||||
...new Set(imageCandidates.map((container) => extractContainerImageName(container.Image)).filter(Boolean)),
|
||||
].slice(0, 64);
|
||||
const likeQueries = candidateImageNames.map((imageName) => like(icons.name, `%${imageName}%`));
|
||||
const dbIcons = likeQueries.length > 0 ? await ctx.db.query.icons.findMany({ where: or(...likeQueries) }) : [];
|
||||
const cdnIconUrl = (slug: string) =>
|
||||
`https://cdn.jsdelivr.net/gh/homarr-labs/dashboard-icons@master/svg/${slug}.svg`;
|
||||
@@ -237,7 +242,7 @@ export const onboardRouter = createTRPCRouter({
|
||||
const iconUrl = dbIcon ?? cdnIconUrl(imageName.toLowerCase());
|
||||
const { url: suggestedUrl, publishedPort } = buildSuggestedUrl(container.Ports, container.host);
|
||||
const kind = matchIntegrationKindFromContainer({ image: container.Image, name: containerName });
|
||||
const sourceId = `docker:${container.host}:${container.Id}`;
|
||||
const sourceId = createDockerSourceId(container.host, container.Id);
|
||||
|
||||
if (kind) {
|
||||
discoveredIntegrations.push({
|
||||
@@ -281,6 +286,7 @@ export const onboardRouter = createTRPCRouter({
|
||||
|
||||
return { status, hosts, integrations: discoveredIntegrations, apps: discoveredApps };
|
||||
} catch (error) {
|
||||
logger.warn(new ErrorWithMetadata("Docker discovery failed during onboarding", {}, { cause: error }));
|
||||
const reason = error instanceof Error ? error.message : "Docker discovery failed";
|
||||
return {
|
||||
status: "unavailable" as const,
|
||||
@@ -306,6 +312,9 @@ export const onboardRouter = createTRPCRouter({
|
||||
: boardList.length === 1
|
||||
? boardList[0]
|
||||
: undefined;
|
||||
if (input.board.id !== undefined && !existingTargetBoard) {
|
||||
throw new TRPCError({ code: "NOT_FOUND", message: "The selected board no longer exists." });
|
||||
}
|
||||
const pendingBoardRow =
|
||||
boardList.length === 0 && input.board.id === undefined
|
||||
? {
|
||||
@@ -701,6 +710,48 @@ export const onboardRouter = createTRPCRouter({
|
||||
return id;
|
||||
};
|
||||
|
||||
const getDockerWidgetPlan = (service: (typeof selectedDockerSources)[number]) => {
|
||||
const widgetKind = service.widgetKind;
|
||||
if (!widgetKind || !input.selectedWidgetKinds.includes(widgetKind)) return null;
|
||||
const integration = service.integrationKind
|
||||
? selectedIntegrations.find(
|
||||
(candidate) => candidate.id === stableOnboardingId("integration", service.sourceId),
|
||||
)
|
||||
: undefined;
|
||||
const issue = getWidgetIntegrationIssue(widgetKind, integration ? [integration.kind] : []);
|
||||
if (issue) {
|
||||
if (issue.code !== "integration-limit") {
|
||||
skippedWidgets.push({
|
||||
sourceId: service.sourceId,
|
||||
widgetKind,
|
||||
code: issue.code,
|
||||
integrationKind: integration?.kind,
|
||||
});
|
||||
}
|
||||
return null;
|
||||
}
|
||||
const config = getDefaultWidgetConfig(widgetKind);
|
||||
return config.skip ? null : { widgetKind, integration, config };
|
||||
};
|
||||
|
||||
const linkExistingDockerWidget = (
|
||||
widgetKind: WidgetKind,
|
||||
integration: (typeof selectedIntegrations)[number] | undefined,
|
||||
) => {
|
||||
const existing = existingItemsByKind.get(widgetKind);
|
||||
if (!existing) return false;
|
||||
if (!integration) return true;
|
||||
const linkedIds = new Set([
|
||||
...existing.integrations.map((row) => row.integrationId),
|
||||
...newIntegrationItems.filter((row) => row.itemId === existing.id).map((row) => row.integrationId),
|
||||
]);
|
||||
if (linkedIds.has(integration.id)) return true;
|
||||
const limit = widgetIntegrationLimits[widgetKind] ?? Number.POSITIVE_INFINITY;
|
||||
if (linkedIds.size >= limit) return false;
|
||||
newIntegrationItems.push({ itemId: existing.id, integrationId: integration.id });
|
||||
return true;
|
||||
};
|
||||
|
||||
const selectedIntegrationIdsByWidget = new Map<WidgetKind, string[]>();
|
||||
const labeledIntegrationSourceById = new Map(
|
||||
selectedDockerSources
|
||||
@@ -780,48 +831,15 @@ export const onboardRouter = createTRPCRouter({
|
||||
if (app && !existingAppIds.has(app.id)) {
|
||||
placeNewItem("app", SuperJSON.stringify({ appId: app.id, openInNewTab: true, showTitle: true }), 1, 1);
|
||||
}
|
||||
if (!service.widgetKind || !input.selectedWidgetKinds.includes(service.widgetKind)) continue;
|
||||
const integration = service.integrationKind
|
||||
? selectedIntegrations.find(
|
||||
(candidate) => candidate.id === stableOnboardingId("integration", service.sourceId),
|
||||
)
|
||||
: undefined;
|
||||
const issue = getWidgetIntegrationIssue(service.widgetKind, integration ? [integration.kind] : []);
|
||||
if (issue) {
|
||||
if (issue.code !== "integration-limit") {
|
||||
skippedWidgets.push({
|
||||
sourceId: service.sourceId,
|
||||
widgetKind: service.widgetKind,
|
||||
code: issue.code,
|
||||
integrationKind: integration?.kind,
|
||||
});
|
||||
}
|
||||
continue;
|
||||
}
|
||||
const existing = existingItemsByKind.get(service.widgetKind);
|
||||
if (existing && integration) {
|
||||
const linkedIds = new Set([
|
||||
...existing.integrations.map((row) => row.integrationId),
|
||||
...newIntegrationItems.filter((row) => row.itemId === existing.id).map((row) => row.integrationId),
|
||||
]);
|
||||
const limit = widgetIntegrationLimits[service.widgetKind] ?? Number.POSITIVE_INFINITY;
|
||||
if (!linkedIds.has(integration.id) && linkedIds.size < limit) {
|
||||
newIntegrationItems.push({ itemId: existing.id, integrationId: integration.id });
|
||||
continue;
|
||||
}
|
||||
if (linkedIds.has(integration.id)) continue;
|
||||
} else if (existing) {
|
||||
continue;
|
||||
}
|
||||
const config = getDefaultWidgetConfig(service.widgetKind);
|
||||
if (config.skip) continue;
|
||||
const plan = getDockerWidgetPlan(service);
|
||||
if (!plan || linkExistingDockerWidget(plan.widgetKind, plan.integration)) continue;
|
||||
const itemId = placeNewItem(
|
||||
service.widgetKind,
|
||||
config.options ? SuperJSON.stringify(config.options) : emptySuperJSON,
|
||||
config.width,
|
||||
config.height,
|
||||
plan.widgetKind,
|
||||
plan.config.options ? SuperJSON.stringify(plan.config.options) : emptySuperJSON,
|
||||
plan.config.width,
|
||||
plan.config.height,
|
||||
);
|
||||
if (integration) newIntegrationItems.push({ itemId, integrationId: integration.id });
|
||||
if (plan.integration) newIntegrationItems.push({ itemId, integrationId: plan.integration.id });
|
||||
}
|
||||
|
||||
const groupedSourcesByName = Map.groupBy(
|
||||
@@ -857,49 +875,14 @@ export const onboardRouter = createTRPCRouter({
|
||||
});
|
||||
}
|
||||
|
||||
if (!service.widgetKind || !input.selectedWidgetKinds.includes(service.widgetKind)) continue;
|
||||
const integration = service.integrationKind
|
||||
? selectedIntegrations.find(
|
||||
(candidate) => candidate.id === stableOnboardingId("integration", service.sourceId),
|
||||
)
|
||||
: undefined;
|
||||
const issue = getWidgetIntegrationIssue(service.widgetKind, integration ? [integration.kind] : []);
|
||||
if (issue) {
|
||||
if (issue.code !== "integration-limit") {
|
||||
skippedWidgets.push({
|
||||
sourceId: service.sourceId,
|
||||
widgetKind: service.widgetKind,
|
||||
code: issue.code,
|
||||
integrationKind: integration?.kind,
|
||||
});
|
||||
}
|
||||
continue;
|
||||
}
|
||||
|
||||
const existing = existingItemsByKind.get(service.widgetKind);
|
||||
if (existing && integration) {
|
||||
const linkedIds = new Set([
|
||||
...existing.integrations.map((row) => row.integrationId),
|
||||
...newIntegrationItems.filter((row) => row.itemId === existing.id).map((row) => row.integrationId),
|
||||
]);
|
||||
const limit = widgetIntegrationLimits[service.widgetKind] ?? Number.POSITIVE_INFINITY;
|
||||
if (!linkedIds.has(integration.id) && linkedIds.size < limit) {
|
||||
newIntegrationItems.push({ itemId: existing.id, integrationId: integration.id });
|
||||
continue;
|
||||
}
|
||||
if (linkedIds.has(integration.id)) continue;
|
||||
} else if (existing) {
|
||||
continue;
|
||||
}
|
||||
|
||||
const config = getDefaultWidgetConfig(service.widgetKind);
|
||||
if (config.skip) continue;
|
||||
const plan = getDockerWidgetPlan(service);
|
||||
if (!plan || linkExistingDockerWidget(plan.widgetKind, plan.integration)) continue;
|
||||
groupedItemPlans.push({
|
||||
kind: service.widgetKind,
|
||||
options: config.options ? SuperJSON.stringify(config.options) : emptySuperJSON,
|
||||
width: config.width,
|
||||
height: config.height,
|
||||
integrationId: integration?.id,
|
||||
kind: plan.widgetKind,
|
||||
options: plan.config.options ? SuperJSON.stringify(plan.config.options) : emptySuperJSON,
|
||||
width: plan.config.width,
|
||||
height: plan.config.height,
|
||||
integrationId: plan.integration?.id,
|
||||
});
|
||||
}
|
||||
if (groupedItemPlans.length === 0) continue;
|
||||
@@ -982,10 +965,12 @@ export const onboardRouter = createTRPCRouter({
|
||||
}
|
||||
}
|
||||
|
||||
const currentAppearance = await getServerSettingByKeyAsync(ctx.db, "appearance");
|
||||
const currentCulture = await getServerSettingByKeyAsync(ctx.db, "culture");
|
||||
const currentAnalytics = await getServerSettingByKeyAsync(ctx.db, "analytics");
|
||||
const currentBoardSettings = await getServerSettingByKeyAsync(ctx.db, "board");
|
||||
const [currentAppearance, currentCulture, currentAnalytics, currentBoardSettings] = await Promise.all([
|
||||
getServerSettingByKeyAsync(ctx.db, "appearance"),
|
||||
getServerSettingByKeyAsync(ctx.db, "culture"),
|
||||
getServerSettingByKeyAsync(ctx.db, "analytics"),
|
||||
getServerSettingByKeyAsync(ctx.db, "board"),
|
||||
]);
|
||||
const settingValues = {
|
||||
appearance: SuperJSON.stringify({ ...currentAppearance, defaultColorScheme: input.server.defaultColorScheme }),
|
||||
culture: SuperJSON.stringify({ ...currentCulture, defaultLocale: input.server.defaultLocale }),
|
||||
|
||||
@@ -288,6 +288,20 @@ describe("getAllBoards should return all boards accessable to the current user",
|
||||
});
|
||||
|
||||
describe("getManageOverview", () => {
|
||||
test("does not expose a public board creator email to anonymous callers", async () => {
|
||||
const db = createDb();
|
||||
const caller = boardRouter.createCaller({ db, deviceType: undefined, session: null });
|
||||
const creatorId = createId();
|
||||
|
||||
await db.insert(users).values({ id: creatorId, name: "Board creator", email: "private@example.com" });
|
||||
await db.insert(boards).values({ id: createId(), name: "public-overview", creatorId, isPublic: true });
|
||||
|
||||
const [board] = await caller.getManageOverview();
|
||||
|
||||
expect(board?.creator).toEqual({ id: creatorId, name: "Board creator", image: null });
|
||||
expect(board?.creator).not.toHaveProperty("email");
|
||||
});
|
||||
|
||||
test("returns the same accessible board set as getAllBoards", async () => {
|
||||
const db = createDb();
|
||||
const caller = boardRouter.createCaller({ db, deviceType: undefined, session: defaultSession });
|
||||
@@ -433,6 +447,8 @@ describe("getManageOverview", () => {
|
||||
const result = await caller.getManageOverview();
|
||||
|
||||
expect(result).toHaveLength(1);
|
||||
expect(result[0]?.creator).toEqual({ id: defaultCreatorId, name: null, image: null });
|
||||
expect(result[0]?.creator).not.toHaveProperty("email");
|
||||
expect(result[0]).toMatchObject({
|
||||
id: boardId,
|
||||
isHome: true,
|
||||
|
||||
@@ -43,6 +43,8 @@ vi.mock("@homarr/common", async (importOriginal) => ({
|
||||
createId: createIdMock,
|
||||
}));
|
||||
vi.mock("@homarr/docker", () => ({
|
||||
createDockerSourceId: (host: string, externalId: string) =>
|
||||
`docker:${encodeURIComponent(host)}:${encodeURIComponent(externalId)}`,
|
||||
dockerLabels: { hide: "homarr.hide" },
|
||||
listDiscoveredContainersAsync,
|
||||
}));
|
||||
@@ -267,6 +269,20 @@ describe("onboard.completeSetup", () => {
|
||||
expect(await db.query.onboarding.findFirst()).toMatchObject({ step: "setup", previousStep: "group" });
|
||||
});
|
||||
|
||||
it("rejects an explicit board id that no longer exists", async () => {
|
||||
const db = createDb();
|
||||
const claimToken = await seedOnboardingAsync(db);
|
||||
await seedBoardAsync(db, { boardId: "target" });
|
||||
|
||||
await expect(createCaller(db, claimToken).completeSetup(completionInput("missing"))).rejects.toMatchObject({
|
||||
code: "NOT_FOUND",
|
||||
message: "The selected board no longer exists.",
|
||||
});
|
||||
|
||||
expect(await db.query.boards.findFirst({ where: eq(boards.id, "target") })).toMatchObject({ name: "target" });
|
||||
expect(await db.query.onboarding.findFirst()).toMatchObject({ step: "setup", previousStep: "group" });
|
||||
});
|
||||
|
||||
it("rejects a board rename that only differs by case from another board", async () => {
|
||||
const db = createDb();
|
||||
const claimToken = await seedOnboardingAsync(db);
|
||||
@@ -899,6 +915,7 @@ describe("onboard.completeSetup", () => {
|
||||
for (const layoutId of ["target-base", "target-mobile"]) {
|
||||
const positions = groupedLayouts.filter((layout) => layout.layoutId === layoutId);
|
||||
const containerLayout = containerLayouts.find((layout) => layout.layoutId === layoutId);
|
||||
expect(positions.length).toBeGreaterThan(0);
|
||||
expect(containerLayout).toBeDefined();
|
||||
expect(Math.max(...positions.map((position) => position.yOffset + position.height))).toBeLessThanOrEqual(
|
||||
containerLayout?.height ?? 0,
|
||||
|
||||
@@ -77,6 +77,8 @@ describe("onboarding claim", () => {
|
||||
expect(
|
||||
getOnboardingClaimTokenFromCookieHeader(`theme=dark; ${onboardingClaimCookieName}=secret-token; other=x`),
|
||||
).toBe("secret-token");
|
||||
expect(getOnboardingClaimTokenFromCookieHeader(`${onboardingClaimCookieName}=secret%2Dtoken`)).toBe("secret-token");
|
||||
expect(getOnboardingClaimTokenFromCookieHeader(`${onboardingClaimCookieName}=%E0%A4%A`)).toBeUndefined();
|
||||
expect(getOnboardingClaimTokenFromCookieHeader(`${onboardingClaimCookieName}-copy=wrong`)).toBeUndefined();
|
||||
});
|
||||
});
|
||||
@@ -70,6 +70,7 @@ export const userRouter = createTRPCRouter({
|
||||
ownerId: userId,
|
||||
position: maxPosition + 1,
|
||||
};
|
||||
const nextStep = isProviderEnabled("ldap") || isProviderEnabled("oidc") ? "group" : "setup";
|
||||
|
||||
await handleTransactionsAsync(ctx.db, {
|
||||
async handleAsync(db, schema) {
|
||||
@@ -78,7 +79,6 @@ export const userRouter = createTRPCRouter({
|
||||
await transaction.insert(schema.groups).values(groupRow);
|
||||
await transaction.insert(schema.groupPermissions).values({ groupId, permission: "admin" });
|
||||
await transaction.insert(schema.groupMembers).values({ groupId, userId });
|
||||
const nextStep = isProviderEnabled("ldap") || isProviderEnabled("oidc") ? "group" : "setup";
|
||||
await transaction.update(schema.onboarding).set({ previousStep: "user", step: nextStep });
|
||||
});
|
||||
},
|
||||
@@ -88,7 +88,6 @@ export const userRouter = createTRPCRouter({
|
||||
transaction.insert(groups).values(groupRow).run();
|
||||
transaction.insert(groupPermissions).values({ groupId, permission: "admin" }).run();
|
||||
transaction.insert(groupMembers).values({ groupId, userId }).run();
|
||||
const nextStep = isProviderEnabled("ldap") || isProviderEnabled("oidc") ? "group" : "setup";
|
||||
transaction.update(onboarding).set({ previousStep: "user", step: nextStep }).run();
|
||||
});
|
||||
},
|
||||
|
||||
Reference in new issue
Block a user