Compare commits
7 Commits
| Author | SHA1 | Date | |
|---|---|---|---|
| 4594bd2307 | |||
| d8c0052b54 | |||
| 4d1d94296f | |||
| c6800a6144 | |||
| 000e90a617 | |||
| 70e9465b68 | |||
| 8c3e0f9554 |
@@ -14,7 +14,29 @@ jobs:
|
||||
runs-on: ubuntu-latest
|
||||
permissions:
|
||||
contents: read
|
||||
packages: read
|
||||
steps:
|
||||
- name: Validate tag format
|
||||
run: |
|
||||
TAG="${{ inputs.tag }}"
|
||||
if ! echo "$TAG" | grep -qE '^[0-9]{4}\.[0-9]{2}\.[0-9]{2}-[a-f0-9]{7}$'; then
|
||||
echo "::error::Invalid tag format: '$TAG'. Expected format: YYYY.MM.DD-sha7 (e.g. 2026.03.28-f1b85bf)"
|
||||
exit 1
|
||||
fi
|
||||
echo "Tag format valid: $TAG"
|
||||
|
||||
- name: Verify image exists in GHCR
|
||||
env:
|
||||
GH_TOKEN: ${{ secrets.GITHUB_TOKEN }}
|
||||
run: |
|
||||
TAG="${{ inputs.tag }}"
|
||||
# Check that the API image exists — if API was pushed, web/migrate were too
|
||||
if ! gh api "/orgs/groombook/packages/container/api/versions" --jq ".[].metadata.container.tags[]" 2>/dev/null | grep -qF "$TAG"; then
|
||||
echo "::error::Image ghcr.io/groombook/api:$TAG not found in GHCR. Verify the tag was built and pushed."
|
||||
exit 1
|
||||
fi
|
||||
echo "Image verified: ghcr.io/groombook/api:$TAG exists"
|
||||
|
||||
- name: Generate infra repo token
|
||||
id: infra-token
|
||||
uses: tibdex/github-app-token@v2
|
||||
|
||||
@@ -12,6 +12,7 @@ RUN pnpm install --frozen-lockfile
|
||||
|
||||
# Build
|
||||
FROM deps AS builder
|
||||
RUN mkdir -p /home/node/.cache/node/corepack
|
||||
COPY packages/ packages/
|
||||
COPY apps/api/ apps/api/
|
||||
RUN pnpm --filter @groombook/types build && \
|
||||
|
||||
@@ -142,8 +142,8 @@ describe("auth init", () => {
|
||||
...originalEnv,
|
||||
AUTH_DISABLED: "true",
|
||||
NODE_ENV: "test",
|
||||
BETTER_AUTH_SECRET: "placeholder-for-test-only",
|
||||
};
|
||||
delete process.env.BETTER_AUTH_SECRET;
|
||||
|
||||
const { initAuth, getAuth } = await reimportAuth();
|
||||
await expect(initAuth()).resolves.toBeUndefined();
|
||||
|
||||
@@ -31,11 +31,11 @@ const BASE_APPT = {
|
||||
|
||||
// ─── Shared mock DB state ─────────────────────────────────────────────────────
|
||||
|
||||
let mockAppt: typeof BASE_APPT | null = BASE_APPT;
|
||||
let mockAppt: (typeof BASE_APPT & { confirmationToken: string }) | null = BASE_APPT as typeof BASE_APPT & { confirmationToken: string };
|
||||
let lastUpdate: Record<string, unknown> = {};
|
||||
|
||||
function resetMock() {
|
||||
mockAppt = { ...BASE_APPT };
|
||||
mockAppt = { ...BASE_APPT, confirmationToken: "valid-token-abc123" } as typeof BASE_APPT & { confirmationToken: string };
|
||||
lastUpdate = {};
|
||||
}
|
||||
|
||||
@@ -55,19 +55,39 @@ vi.mock("@groombook/db", () => {
|
||||
}),
|
||||
}),
|
||||
update: () => ({
|
||||
set: (vals: Record<string, unknown>) => ({
|
||||
where: () => {
|
||||
lastUpdate = { ...vals };
|
||||
if (mockAppt) {
|
||||
mockAppt = { ...mockAppt, ...vals } as typeof BASE_APPT;
|
||||
}
|
||||
return { returning: () => (mockAppt ? [mockAppt] : []) };
|
||||
},
|
||||
}),
|
||||
set: (vals: Record<string, unknown>) => {
|
||||
const setVals = vals;
|
||||
return {
|
||||
where: () => {
|
||||
const preUpdate = mockAppt ? { ...mockAppt } : null;
|
||||
const preStatus = preUpdate?.confirmationStatus;
|
||||
const preStart = preUpdate?.startTime;
|
||||
lastUpdate = { ...setVals };
|
||||
const whereMatched =
|
||||
preUpdate != null &&
|
||||
preStatus === "pending" &&
|
||||
preStart != null &&
|
||||
preStart > new Date();
|
||||
if (whereMatched && mockAppt) {
|
||||
mockAppt = { ...mockAppt, ...setVals } as typeof BASE_APPT & { confirmationToken: string };
|
||||
}
|
||||
return {
|
||||
returning: () => {
|
||||
if (!preUpdate) return [];
|
||||
if (preStatus !== "pending") return [];
|
||||
if (preStart && preStart <= new Date()) return [];
|
||||
return whereMatched && mockAppt ? [mockAppt] : [];
|
||||
},
|
||||
};
|
||||
},
|
||||
};
|
||||
},
|
||||
}),
|
||||
}),
|
||||
appointments,
|
||||
eq: () => ({}),
|
||||
and: (a: unknown, b: unknown, c?: unknown) => (c ? [a, b, c] : [a, b]),
|
||||
gt: () => ({}),
|
||||
};
|
||||
});
|
||||
|
||||
|
||||
@@ -362,7 +362,7 @@ describe("requireRoleOrSuperUser", () => {
|
||||
const res = await app.request("/test");
|
||||
expect(res.status).toBe(403);
|
||||
const body = await res.json();
|
||||
expect(body.error).toMatch(/super user privileges required/i);
|
||||
expect(body.error).toMatch(/role 'receptionist' is not permitted/i);
|
||||
});
|
||||
|
||||
it("blocks a non-super-user groomer from manager-only routes", async () => {
|
||||
@@ -370,7 +370,7 @@ describe("requireRoleOrSuperUser", () => {
|
||||
const res = await app.request("/test");
|
||||
expect(res.status).toBe(403);
|
||||
const body = await res.json();
|
||||
expect(body.error).toMatch(/super user privileges required/i);
|
||||
expect(body.error).toMatch(/role 'groomer' is not permitted/i);
|
||||
});
|
||||
|
||||
it("allows a manager with multiple allowed roles", async () => {
|
||||
|
||||
@@ -42,6 +42,23 @@ app.use(
|
||||
})
|
||||
);
|
||||
|
||||
// CSRF protection for state-changing requests
|
||||
app.use("/api/*", async (c, next) => {
|
||||
const method = c.req.method;
|
||||
if (["GET", "HEAD", "OPTIONS"].includes(method)) {
|
||||
await next();
|
||||
return;
|
||||
}
|
||||
const origin = c.req.header("origin");
|
||||
const trustedOrigin = process.env.CORS_ORIGIN ?? "http://localhost:5173";
|
||||
if (origin && origin !== trustedOrigin) {
|
||||
c.status(403);
|
||||
c.json({ error: "CSRF validation failed: origin mismatch" });
|
||||
return;
|
||||
}
|
||||
await next();
|
||||
});
|
||||
|
||||
// Health check (no auth required)
|
||||
app.get("/health", (c) => c.json({ status: "ok" }));
|
||||
|
||||
|
||||
+35
-11
@@ -86,10 +86,15 @@ export async function initAuth(): Promise<void> {
|
||||
// AUTH_DISABLED=true means dev/demo mode — still build Better-Auth with placeholder
|
||||
// config so auth.handler exists (middleware bypasses it anyway)
|
||||
if (process.env.AUTH_DISABLED === "true") {
|
||||
if (!BETTER_AUTH_SECRET) {
|
||||
throw new Error(
|
||||
"[FATAL] BETTER_AUTH_SECRET must be set when AUTH_DISABLED=true"
|
||||
);
|
||||
}
|
||||
console.warn("[auth] AUTH_DISABLED=true — building placeholder auth instance");
|
||||
authInstance = betterAuth({
|
||||
database: drizzleAdapter(getDb(), { provider: "pg" }),
|
||||
secret: BETTER_AUTH_SECRET ?? "placeholder-secret-do-not-use-in-prod",
|
||||
secret: BETTER_AUTH_SECRET,
|
||||
baseURL: BETTER_AUTH_URL,
|
||||
rateLimit: {
|
||||
enabled: true,
|
||||
@@ -199,20 +204,36 @@ export async function initAuth(): Promise<void> {
|
||||
return url;
|
||||
}
|
||||
};
|
||||
const validateIssuerHost = (url: string, issuerUrl: string): boolean => {
|
||||
try {
|
||||
const discovered = new URL(url);
|
||||
const expected = new URL(issuerUrl);
|
||||
return discovered.hostname === expected.hostname;
|
||||
} catch {
|
||||
return false;
|
||||
}
|
||||
};
|
||||
const authzUrl = discovery.authorization_endpoint;
|
||||
const tokenUrl = discovery.token_endpoint;
|
||||
const userInfoUrl = discovery.userinfo_endpoint;
|
||||
if (authzUrl && tokenUrl && userInfoUrl) {
|
||||
oidcConfig = {
|
||||
authorizationUrl: authzUrl,
|
||||
tokenUrl: providerConfig.internalBaseUrl
|
||||
? replaceHost(tokenUrl, providerConfig.internalBaseUrl)
|
||||
: tokenUrl,
|
||||
userInfoUrl: providerConfig.internalBaseUrl
|
||||
? replaceHost(userInfoUrl, providerConfig.internalBaseUrl)
|
||||
: userInfoUrl,
|
||||
};
|
||||
console.log("[auth] OIDC discovery successful, provider:", providerConfig.providerId);
|
||||
const validAuthz = validateIssuerHost(authzUrl, providerConfig.issuerUrl);
|
||||
const validToken = validateIssuerHost(tokenUrl, providerConfig.issuerUrl);
|
||||
const validUserInfo = validateIssuerHost(userInfoUrl, providerConfig.issuerUrl);
|
||||
if (!validAuthz || !validToken || !validUserInfo) {
|
||||
console.warn("[auth] OIDC discovery URL host mismatch — possible redirection attack, rejecting");
|
||||
} else {
|
||||
oidcConfig = {
|
||||
authorizationUrl: authzUrl,
|
||||
tokenUrl: providerConfig.internalBaseUrl
|
||||
? replaceHost(tokenUrl, providerConfig.internalBaseUrl)
|
||||
: tokenUrl,
|
||||
userInfoUrl: providerConfig.internalBaseUrl
|
||||
? replaceHost(userInfoUrl, providerConfig.internalBaseUrl)
|
||||
: userInfoUrl,
|
||||
};
|
||||
console.log("[auth] OIDC discovery successful, provider:", providerConfig.providerId);
|
||||
}
|
||||
} else {
|
||||
console.warn("[auth] OIDC discovery missing required endpoints, using discoveryUrl only");
|
||||
}
|
||||
@@ -287,6 +308,9 @@ export async function initAuth(): Promise<void> {
|
||||
enabled: true,
|
||||
maxAge: 5 * 60, // 5 minutes
|
||||
},
|
||||
cookieAttributes: {
|
||||
sameSite: "strict",
|
||||
},
|
||||
},
|
||||
trustedOrigins: [process.env.CORS_ORIGIN ?? "http://localhost:5173"],
|
||||
});
|
||||
|
||||
@@ -149,9 +149,9 @@ export function requireRoleOrSuperUser(
|
||||
}
|
||||
return c.json(
|
||||
{
|
||||
error: staffRow.isSuperUser
|
||||
? `Forbidden: role '${staffRow.role}' is not permitted`
|
||||
: "Forbidden: super user privileges required",
|
||||
error: hasAllowedRole
|
||||
? "Forbidden: super user privileges required"
|
||||
: `Forbidden: role '${staffRow.role}' is not permitted`,
|
||||
},
|
||||
403
|
||||
);
|
||||
|
||||
+38
-49
@@ -255,39 +255,37 @@ bookRouter.get("/confirm/:token", async (c) => {
|
||||
const token = c.req.param("token");
|
||||
const db = getDb();
|
||||
|
||||
// Atomic: consume token and confirm in a single query to prevent replay.
|
||||
// Only future appointments can be confirmed.
|
||||
const [appt] = await db
|
||||
.select()
|
||||
.from(appointments)
|
||||
.where(eq(appointments.confirmationToken, token))
|
||||
.limit(1);
|
||||
|
||||
if (!appt) {
|
||||
return c.redirect(`${BASE_URL()}/booking/error`);
|
||||
}
|
||||
|
||||
// Reject if appointment is in the past
|
||||
if (appt.startTime < new Date()) {
|
||||
return c.redirect(`${BASE_URL()}/booking/error`);
|
||||
}
|
||||
|
||||
// Idempotent confirm: if already confirmed, redirect to success
|
||||
if (appt.confirmationStatus === "confirmed") {
|
||||
return c.redirect(`${BASE_URL()}/booking/confirmed`);
|
||||
}
|
||||
|
||||
// Reject if already cancelled
|
||||
if (appt.confirmationStatus === "cancelled") {
|
||||
return c.redirect(`${BASE_URL()}/booking/error`);
|
||||
}
|
||||
|
||||
await db
|
||||
.update(appointments)
|
||||
.set({
|
||||
confirmationStatus: "confirmed",
|
||||
confirmedAt: new Date(),
|
||||
confirmationToken: null,
|
||||
updatedAt: new Date(),
|
||||
})
|
||||
.where(eq(appointments.id, appt.id));
|
||||
.where(
|
||||
and(
|
||||
eq(appointments.confirmationToken, token),
|
||||
eq(appointments.confirmationStatus, "pending"),
|
||||
gt(appointments.startTime, new Date())
|
||||
)
|
||||
)
|
||||
.returning();
|
||||
|
||||
if (!appt) {
|
||||
// Check status for idempotency: already-confirmed → redirect to confirmed
|
||||
const [existing] = await db
|
||||
.select({ confirmationStatus: appointments.confirmationStatus })
|
||||
.from(appointments)
|
||||
.where(eq(appointments.confirmationToken, token))
|
||||
.limit(1);
|
||||
if (existing?.confirmationStatus === "confirmed") {
|
||||
return c.redirect(`${BASE_URL()}/booking/confirmed`);
|
||||
}
|
||||
return c.redirect(`${BASE_URL()}/booking/error`);
|
||||
}
|
||||
|
||||
return c.redirect(`${BASE_URL()}/booking/confirmed`);
|
||||
});
|
||||
@@ -299,29 +297,9 @@ bookRouter.get("/cancel/:token", async (c) => {
|
||||
const token = c.req.param("token");
|
||||
const db = getDb();
|
||||
|
||||
// Atomic: consume token and cancel in a single query to prevent replay.
|
||||
// Only future appointments can be cancelled.
|
||||
const [appt] = await db
|
||||
.select()
|
||||
.from(appointments)
|
||||
.where(eq(appointments.confirmationToken, token))
|
||||
.limit(1);
|
||||
|
||||
if (!appt) {
|
||||
return c.redirect(`${BASE_URL()}/booking/error`);
|
||||
}
|
||||
|
||||
// Reject if appointment is in the past
|
||||
if (appt.startTime < new Date()) {
|
||||
return c.redirect(`${BASE_URL()}/booking/error`);
|
||||
}
|
||||
|
||||
// Reject if already cancelled (token was nullified — this path won't normally hit,
|
||||
// but guard against edge cases where token lookup still works)
|
||||
if (appt.confirmationStatus === "cancelled") {
|
||||
return c.redirect(`${BASE_URL()}/booking/error`);
|
||||
}
|
||||
|
||||
// Single-use cancellation: nullify token after use
|
||||
await db
|
||||
.update(appointments)
|
||||
.set({
|
||||
confirmationStatus: "cancelled",
|
||||
@@ -329,7 +307,18 @@ bookRouter.get("/cancel/:token", async (c) => {
|
||||
confirmationToken: null,
|
||||
updatedAt: new Date(),
|
||||
})
|
||||
.where(eq(appointments.id, appt.id));
|
||||
.where(
|
||||
and(
|
||||
eq(appointments.confirmationToken, token),
|
||||
eq(appointments.confirmationStatus, "pending"),
|
||||
gt(appointments.startTime, new Date())
|
||||
)
|
||||
)
|
||||
.returning();
|
||||
|
||||
if (!appt) {
|
||||
return c.redirect(`${BASE_URL()}/booking/error`);
|
||||
}
|
||||
|
||||
return c.redirect(`${BASE_URL()}/booking/cancelled`);
|
||||
});
|
||||
|
||||
@@ -1,5 +1,5 @@
|
||||
import { Hono } from "hono";
|
||||
import { randomBytes } from "node:crypto";
|
||||
import { randomBytes, timingSafeEqual } from "node:crypto";
|
||||
import {
|
||||
and,
|
||||
eq,
|
||||
@@ -84,7 +84,12 @@ calendarRouter.get("/:staffId.ics", async (c) => {
|
||||
.where(eq(staff.id, staffId))
|
||||
.limit(1);
|
||||
|
||||
if (!staffMember || staffMember.icalToken !== token) {
|
||||
if (
|
||||
!staffMember ||
|
||||
!staffMember.icalToken ||
|
||||
staffMember.icalToken.length !== token.length ||
|
||||
!timingSafeEqual(Buffer.from(staffMember.icalToken), Buffer.from(token))
|
||||
) {
|
||||
return c.text("Unauthorized", 401);
|
||||
}
|
||||
|
||||
|
||||
@@ -6,6 +6,25 @@ import type { AppEnv } from "../middleware/rbac.js";
|
||||
|
||||
export const setupRouter = new Hono<AppEnv>();
|
||||
|
||||
// Simple in-memory rate limiter: 10 req/min per IP for setup endpoints
|
||||
const setupRateLimitMap = new Map<string, { count: number; resetAt: number }>();
|
||||
const SETUP_RATE_LIMIT = 10;
|
||||
const SETUP_RATE_WINDOW_MS = 60 * 1000;
|
||||
|
||||
function checkSetupRateLimit(ip: string): boolean {
|
||||
const now = Date.now();
|
||||
const entry = setupRateLimitMap.get(ip);
|
||||
if (!entry || now > entry.resetAt) {
|
||||
setupRateLimitMap.set(ip, { count: 1, resetAt: now + SETUP_RATE_WINDOW_MS });
|
||||
return true;
|
||||
}
|
||||
if (entry.count >= SETUP_RATE_LIMIT) {
|
||||
return false;
|
||||
}
|
||||
entry.count++;
|
||||
return true;
|
||||
}
|
||||
|
||||
// GET /api/setup/status — public (no auth), returns whether setup is needed
|
||||
// and whether the auth provider bootstrap step should be shown
|
||||
setupRouter.get("/status", async (c) => {
|
||||
@@ -185,6 +204,11 @@ const authProviderTestSchema = z.object({
|
||||
* After setup completes, this endpoint permanently returns 403.
|
||||
*/
|
||||
setupRouter.post("/auth-provider", async (c) => {
|
||||
const ip = c.req.header("x-forwarded-for")?.split(",")[0]?.trim() ?? "unknown";
|
||||
if (!checkSetupRateLimit(ip)) {
|
||||
return c.json({ error: "Too many requests. Please try again later." }, 429);
|
||||
}
|
||||
|
||||
const db = getDb();
|
||||
|
||||
// Guard: only allow during fresh install (no super user yet)
|
||||
@@ -254,6 +278,11 @@ setupRouter.post("/auth-provider", async (c) => {
|
||||
* Only available when needsSetup is true (no super user = fresh install).
|
||||
*/
|
||||
setupRouter.post("/auth-provider/test", async (c) => {
|
||||
const ip = c.req.header("x-forwarded-for")?.split(",")[0]?.trim() ?? "unknown";
|
||||
if (!checkSetupRateLimit(ip)) {
|
||||
return c.json({ ok: false, error: "Too many requests. Please try again later." }, 429);
|
||||
}
|
||||
|
||||
const db = getDb();
|
||||
|
||||
// Guard: only allow during fresh install (no super user yet)
|
||||
|
||||
@@ -3,10 +3,22 @@ server {
|
||||
root /usr/share/nginx/html;
|
||||
index index.html;
|
||||
|
||||
# Security headers
|
||||
add_header X-Content-Type-Options "nosniff" always;
|
||||
add_header X-Frame-Options "SAMEORIGIN" always;
|
||||
add_header Referrer-Policy "strict-origin-when-cross-origin" always;
|
||||
add_header X-XSS-Protection "1; mode=block" always;
|
||||
add_header Permissions-Policy "camera=(), microphone=(), geolocation=()" always;
|
||||
|
||||
# Cache static assets
|
||||
location ~* \.(js|css|png|svg|ico|woff2)$ {
|
||||
expires 1y;
|
||||
add_header Cache-Control "public, immutable";
|
||||
add_header X-Content-Type-Options "nosniff" always;
|
||||
add_header X-Frame-Options "SAMEORIGIN" always;
|
||||
add_header Referrer-Policy "strict-origin-when-cross-origin" always;
|
||||
add_header X-XSS-Protection "1; mode=block" always;
|
||||
add_header Permissions-Policy "camera=(), microphone=(), geolocation=()" always;
|
||||
}
|
||||
|
||||
# Proxy API calls to the API service
|
||||
|
||||
Reference in New Issue
Block a user