From 6f6b31f392b9a6e866baa62472c90dd385bf24e5 Mon Sep 17 00:00:00 2001 From: Wynne Pirini Date: Thu, 17 Sep 2026 11:54:20 +1200 Subject: [PATCH 1/6] auth: archived members' MCP bearers stop validating, and cannot be refreshed MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit VerifyMCPBearer's OAuth branch never checked users.archived_at, so archiving a member — the documented offboarding path — left their connected agent bearer working, while sessions and API keys already filtered on it. The refresh grant had the same gap: it would mint fresh tokens for a dead account. Both queries now join users and require archived_at IS NULL, mirroring the API-key fallback directly below. Pinned by TestMCP_OAuthBearerRejectedWhenArchived (red-green verified against the unfixed code on both assertions) and one ARCHITECTURE §6 sentence stating the invariant. --- docs/ARCHITECTURE.md | 4 ++ internal/handler/mcp_oauth.go | 11 +++- internal/handler/mcp_oauth_authorize.go | 7 ++- internal/handler/mcp_oauth_test.go | 75 +++++++++++++++++++++++++ 4 files changed, 93 insertions(+), 4 deletions(-) diff --git a/docs/ARCHITECTURE.md b/docs/ARCHITECTURE.md index e91d5db6..b79052a5 100644 --- a/docs/ARCHITECTURE.md +++ b/docs/ARCHITECTURE.md @@ -189,6 +189,10 @@ the platform/recovery secret doesn't expose secrets. - **Offboarding = archive** (`users.archived_at`), never hard-delete — preserves bookings, event-type ownership, team links. Archived ⇒ no login, hidden from lists, skipped in routing/slots, event types deactivated. Reversible (restore). + "No login" holds on every auth path: sessions, API keys, and MCP OAuth bearers + all filter on `archived_at`, and refresh grants are refused for archived users + too — so archiving ends a member's agent access and leaves no usable + credential material behind. Archiving is blocked while the member has upcoming (primary-host) bookings; a resolve-meetings flow makes the admin reassign/cancel each first. diff --git a/internal/handler/mcp_oauth.go b/internal/handler/mcp_oauth.go index 330b9f13..bd2f1638 100644 --- a/internal/handler/mcp_oauth.go +++ b/internal/handler/mcp_oauth.go @@ -99,10 +99,15 @@ func (h *Handler) MCPCallerMiddleware(next http.Handler) http.Handler { func (h *Handler) VerifyMCPBearer(ctx context.Context, token string, _ *http.Request) (*auth.TokenInfo, error) { hash := hashAPIKey(token) - // OAuth access token? + // OAuth access token? Archived users authenticate with nothing: the session + // path (auth.go) and both API-key paths (below, and auth.go) already filter + // on users.archived_at, and without the same check here an archived member's + // connected agent bearer would keep validating after offboarding. var userID, expiresAt string - err := h.db.QueryRowContext(ctx, - `SELECT user_id, expires_at FROM oauth_access_tokens WHERE token_hash = ?`, hash). + err := h.db.QueryRowContext(ctx, ` + SELECT t.user_id, t.expires_at FROM oauth_access_tokens t + JOIN users u ON u.id = t.user_id + WHERE t.token_hash = ? AND u.archived_at IS NULL`, hash). Scan(&userID, &expiresAt) if err == nil { exp, _ := time.Parse(time.RFC3339, expiresAt) diff --git a/internal/handler/mcp_oauth_authorize.go b/internal/handler/mcp_oauth_authorize.go index 8ab139f2..a7b2e481 100644 --- a/internal/handler/mcp_oauth_authorize.go +++ b/internal/handler/mcp_oauth_authorize.go @@ -246,8 +246,13 @@ func (h *Handler) tokenRefresh(w http.ResponseWriter, r *http.Request) { return } var id, userID, storedClient, scope, resource string + // Archived users refresh nothing: their access tokens no longer validate + // (VerifyMCPBearer filters on archived_at), so minting fresh ones here + // would hand live credential material to a dead account. err := h.db.QueryRowContext(r.Context(), ` - SELECT id, user_id, client_id, scope, resource FROM oauth_access_tokens WHERE refresh_hash = ?`, + SELECT t.id, t.user_id, t.client_id, t.scope, t.resource FROM oauth_access_tokens t + JOIN users u ON u.id = t.user_id + WHERE t.refresh_hash = ? AND u.archived_at IS NULL`, hashAPIKey(refresh)).Scan(&id, &userID, &storedClient, &scope, &resource) if err != nil { writeOAuthError(w, http.StatusBadRequest, "invalid_grant", "unknown refresh token") diff --git a/internal/handler/mcp_oauth_test.go b/internal/handler/mcp_oauth_test.go index 59e5042d..a51dd3b1 100644 --- a/internal/handler/mcp_oauth_test.go +++ b/internal/handler/mcp_oauth_test.go @@ -297,3 +297,78 @@ func TestMCP_OAuthDeny(t *testing.T) { t.Errorf("deny: code=%d location=%q; want 302 with error=access_denied", rec.Code, loc) } } + +// TestMCP_OAuthBearerRejectedWhenArchived pins the offboarding half of +// "Offboarding = archive" (§6) for the MCP OAuth path. The session path and +// both API-key paths already filter on users.archived_at; the OAuth branch of +// VerifyMCPBearer did not, so an archived member's connected agent bearer kept +// validating after they were offboarded. +func TestMCP_OAuthBearerRejectedWhenArchived(t *testing.T) { + h, database, _, userID := setupWorkspaceWithDB(t) + + const rawToken = "archived-user-bearer" + const rawRefresh = "archived-user-refresh" + now := time.Now().UTC() + if _, err := database.Exec(` + INSERT INTO oauth_access_tokens (id, token_hash, refresh_hash, client_id, user_id, expires_at, created_at) + VALUES (?, ?, ?, 'client-1', ?, ?, ?)`, + "tok-archived", sha256HexForTest(rawToken), sha256HexForTest(rawRefresh), userID, + now.Add(time.Hour).Format(time.RFC3339), now.Format(time.RFC3339Nano)); err != nil { + t.Fatalf("seed token: %v", err) + } + refresh := func(t *testing.T, raw string) *httptest.ResponseRecorder { + t.Helper() + form := url.Values{"grant_type": {"refresh_token"}, "refresh_token": {raw}} + req := httptest.NewRequest(http.MethodPost, "/oauth/token", strings.NewReader(form.Encode())) + req.Header.Set("Content-Type", "application/x-www-form-urlencoded") + rec := httptest.NewRecorder() + h.TokenMCP(rec, req) + return rec + } + + // Before archiving, the bearer validates and the refresh token rotates. + // The rotated pair is kept: it is the agent's *current* credential, which + // is exactly what must die on archive (testing the pre-rotation values + // post-archive would pass vacuously — rotation already replaced them). + if info, err := h.VerifyMCPBearer(context.Background(), rawToken, nil); err != nil || info.UserID != userID { + t.Fatalf("VerifyMCPBearer before archive = %+v, %v; want UserID=%s", info, err, userID) + } + rec := refresh(t, rawRefresh) + if rec.Code != http.StatusOK { + t.Fatalf("refresh before archive = %d; want 200 — %s", rec.Code, rec.Body.String()) + } + var rotated map[string]any + if err := json.Unmarshal(rec.Body.Bytes(), &rotated); err != nil { + t.Fatalf("decode rotated pair: %v", err) + } + access, _ := rotated["access_token"].(string) + refreshTok, _ := rotated["refresh_token"].(string) + if access == "" || refreshTok == "" { + t.Fatalf("rotated pair missing tokens: %v", rotated) + } + // The rotated access token validates while the user is live — so its + // post-archive rejection below proves the archive check, not a broken + // rotation. + if info, err := h.VerifyMCPBearer(context.Background(), access, nil); err != nil || info.UserID != userID { + t.Fatalf("VerifyMCPBearer(rotated) before archive = %+v, %v; want UserID=%s", info, err, userID) + } + + // Archive the user — the documented offboarding path — and the current + // bearer must stop validating, exactly like the user's sessions and API + // keys do. + if _, err := database.Exec(`UPDATE users SET archived_at = ? WHERE id = ?`, + now.Format(time.RFC3339Nano), userID); err != nil { + t.Fatalf("archive user: %v", err) + } + if info, err := h.VerifyMCPBearer(context.Background(), access, nil); err == nil { + t.Errorf("VerifyMCPBearer after archive = %+v; want rejection — an archived member's agent bearer must not survive offboarding", info) + } + + // The refresh token must not mint fresh credentials for a dead account + // either: the rotated access token would not validate, but issuance itself + // tells the agent it is still authorized and leaves live credential + // material in the table. + if rec := refresh(t, refreshTok); rec.Code == http.StatusOK { + t.Errorf("refresh after archive issued tokens to an archived user: %s", rec.Body.String()) + } +} From 27c5265b93196030be15513f19ca47c5a723b525 Mon Sep 17 00:00:00 2001 From: Wynne Pirini Date: Thu, 17 Sep 2026 12:16:26 +1200 Subject: [PATCH 2/6] fix(reminders): name the assigned host, not the event-type owner (#48) sendReminder joined users on et.user_id, so multi-host teams got reminders naming the event-type creator instead of the booked host. Join on bookings.host_id. loadCancellationData had the same shape, so cancellation/reschedule emails use the booking host too. Pinned by TestWorker_reminderNamesAssignedHost. --- internal/handler/booking_handler.go | 15 ++++++---- internal/worker/reminder_test.go | 44 +++++++++++++++++++++++++++++ internal/worker/worker.go | 4 +-- 3 files changed, 55 insertions(+), 8 deletions(-) diff --git a/internal/handler/booking_handler.go b/internal/handler/booking_handler.go index 6b7abe35..471c81ce 100644 --- a/internal/handler/booking_handler.go +++ b/internal/handler/booking_handler.go @@ -1802,14 +1802,17 @@ func (h *Handler) loadCancellationData(ctx context.Context, b *booking.Booking) d.LocationValue = b.LocationValue d.CancellationReason = b.CancellationReason - // Event type name + slug and host name + email in one join. + // Event type name + slug and the assigned host's name + email. The host + // comes from the booking (b.HostID), not the event-type owner: with + // multi-host event types the owner rarely hosts the booking (#48). err := h.db.QueryRowContext(ctx, ` - SELECT et.name, et.slug, u.name, u.email - FROM event_types et JOIN users u ON u.id = et.user_id - WHERE et.id = ?`, b.EventTypeID). - Scan(&d.EventTypeName, &d.EventTypeSlug, &d.HostName, &d.HostEmail) + SELECT et.name, et.slug FROM event_types et WHERE et.id = ?`, b.EventTypeID). + Scan(&d.EventTypeName, &d.EventTypeSlug) if err != nil { - return d, fmt.Errorf("load event/host: %w", err) + return d, fmt.Errorf("load event: %w", err) + } + if err := h.loadHostIntoData(ctx, b.HostID, &d); err != nil { + return d, fmt.Errorf("load host: %w", err) } // Organizer attendee. diff --git a/internal/worker/reminder_test.go b/internal/worker/reminder_test.go index 4b922e6c..3e5b49d5 100644 --- a/internal/worker/reminder_test.go +++ b/internal/worker/reminder_test.go @@ -226,3 +226,47 @@ func TestWorker_reminderNotFiredBeforeRunAt(t *testing.T) { t.Errorf("job status = %q; want pending", jobStatus) } } + +// --------------------------------------------------------------------------- +// reminder.send: names the assigned host, not the event-type owner (#48) +// --------------------------------------------------------------------------- + +func TestWorker_reminderNamesAssignedHost(t *testing.T) { + database, svc := setup(t) + ctx := context.Background() + + pastRunAt := time.Now().UTC().Add(-time.Second).Format(time.RFC3339) + bookingStart := time.Now().UTC().Add(25 * time.Hour).Format(time.RFC3339) + + // Event type owned by Host One, booking assigned to Host Two. + database.ExecContext(ctx, + `INSERT INTO event_types (id, user_id, slug, name, duration_minutes) + VALUES ('et-h1','host-01','host-test','Hosted Meeting',30)`) + database.ExecContext(ctx, + `INSERT INTO bookings (id, event_type_id, host_id, start_at, end_at, status) + VALUES ('bk-h1','et-h1','host-02',?,?,'confirmed')`, bookingStart, bookingStart) + database.ExecContext(ctx, + `INSERT INTO booking_attendees (id, booking_id, name, email, iana_timezone, is_organizer) + VALUES ('att-h1','bk-h1','Alice','alice@example.com','UTC',1)`) + database.ExecContext(ctx, ` + INSERT INTO jobs (id, type, payload, run_at, status, attempts, max_attempts) + VALUES ('job-h1','reminder.send','{"booking_id":"bk-h1"}',?,'pending',0,3)`, + pastRunAt) + + m := &captureMailer{} + w := worker.New(database, svc, slog.Default(), + worker.WithMailer(m), + worker.WithHTTPClient(&http.Client{})) + w.Poll(ctx) + + if len(m.sent) != 1 { + t.Fatalf("sent %d emails; want 1", len(m.sent)) + } + msg := m.sent[0] + if !strings.Contains(msg.Text, "Host Two") { + t.Errorf("reminder body missing assigned host %q:\n%s", "Host Two", msg.Text) + } + if strings.Contains(msg.Text, "Host One") { + t.Errorf("reminder body names the event-type owner; want the assigned host:\n%s", msg.Text) + } +} diff --git a/internal/worker/worker.go b/internal/worker/worker.go index b44ed1bf..04902998 100644 --- a/internal/worker/worker.go +++ b/internal/worker/worker.go @@ -263,7 +263,7 @@ func (w *Worker) sendReminder(ctx context.Context, payload string) error { return fmt.Errorf("worker: reminder: parse payload: %w", err) } - // One query: join bookings → event_types → users (host). + // One query: join bookings → event_types → users (assigned host). // Also load notify_reminder pref and msg_reminder custom note. // Skip if booking is deleted or no longer confirmed. var d mailer.BookingData @@ -277,7 +277,7 @@ func (w *Worker) sendReminder(ctx context.Context, payload string) error { u.name, u.email, COALESCE(u.notify_reminder, 1) FROM bookings b JOIN event_types et ON et.id = b.event_type_id - JOIN users u ON u.id = et.user_id + JOIN users u ON u.id = b.host_id WHERE b.id = ?`, p.BookingID). Scan(&status, &startAt, &endAt, &locVal, &d.EventTypeName, &d.EventTypeSlug, &msgReminder, &subjReminder, From 659626dce5d421e3b0a55c4fc1dd21e10222efd7 Mon Sep 17 00:00:00 2001 From: Wynne Pirini Date: Thu, 17 Sep 2026 13:53:19 +1200 Subject: [PATCH 3/6] feat(auth): self-service password reset via emailed link (#34) The login page linked /admin/forgot-password, which never existed. Add POST /v1/auth/password/forgot (enumeration-safe, background send, skips archived and email-login-disabled accounts) and POST /v1/auth/password/reset (single-use hashed token, 1h TTL, revokes all sessions, signs straight in), mirroring the magic-link flow, plus the two public SPA pages. Pinned by nine tests in password_reset_test.go. --- frontend/src/routes/+layout.svelte | 2 + .../src/routes/forgot-password/+page.svelte | 85 ++++++ .../src/routes/reset-password/+page.svelte | 101 ++++++++ .../00058_password_reset_tokens.sql | 15 ++ internal/handler/password_reset.go | 187 ++++++++++++++ internal/handler/password_reset_test.go | 241 ++++++++++++++++++ internal/server/server.go | 2 + 7 files changed, 633 insertions(+) create mode 100644 frontend/src/routes/forgot-password/+page.svelte create mode 100644 frontend/src/routes/reset-password/+page.svelte create mode 100644 internal/db/migrations/00058_password_reset_tokens.sql create mode 100644 internal/handler/password_reset.go create mode 100644 internal/handler/password_reset_test.go diff --git a/frontend/src/routes/+layout.svelte b/frontend/src/routes/+layout.svelte index 7a34f787..856a229d 100644 --- a/frontend/src/routes/+layout.svelte +++ b/frontend/src/routes/+layout.svelte @@ -27,6 +27,8 @@ const isPublicRoute = $derived( $page.route.id === '/login' || $page.route.id === '/claim' || + $page.route.id === '/forgot-password' || + $page.route.id === '/reset-password' || $page.route.id === '/invite/[token]' ); diff --git a/frontend/src/routes/forgot-password/+page.svelte b/frontend/src/routes/forgot-password/+page.svelte new file mode 100644 index 00000000..a7e8e707 --- /dev/null +++ b/frontend/src/routes/forgot-password/+page.svelte @@ -0,0 +1,85 @@ + + +Forgot password — Calnode + +
+
+
+

Forgot your password?

+

+ Enter your account email and we'll send you a link to set a new password. +

+
+ + {#if message} +
{message}
+

+ Back to sign in +

+ {:else} +
+
+ + (error = '')} + aria-invalid={error ? 'true' : undefined} + required + /> + {#if error} +

{error}

+ {/if} +
+ +
+

+ Back to sign in +

+ {/if} +
+
diff --git a/frontend/src/routes/reset-password/+page.svelte b/frontend/src/routes/reset-password/+page.svelte new file mode 100644 index 00000000..ce9b95fe --- /dev/null +++ b/frontend/src/routes/reset-password/+page.svelte @@ -0,0 +1,101 @@ + + +Set a new password — Calnode + +
+
+
+

Set a new password

+

Choose a password of at least 8 characters.

+
+ + {#if error && !token} +
{error}
+

+ Request a new reset link +

+ {:else} +
+
+ + +
+
+ + +
+ {#if error} +
{error}
+ {/if} + +
+ {/if} +
+
diff --git a/internal/db/migrations/00058_password_reset_tokens.sql b/internal/db/migrations/00058_password_reset_tokens.sql new file mode 100644 index 00000000..d358a568 --- /dev/null +++ b/internal/db/migrations/00058_password_reset_tokens.sql @@ -0,0 +1,15 @@ +-- +goose Up +-- One-time, short-lived password-reset tokens emailed to a user. We store only the +-- SHA-256 of the token (never the raw value); single-use is enforced by used_at. +-- Mirrors magic_link_tokens (00040) with a longer TTL: a reset email sits in an +-- inbox longer than a just-requested login link. +CREATE TABLE password_reset_tokens ( + token_hash TEXT PRIMARY KEY, + user_id TEXT NOT NULL REFERENCES users(id) ON DELETE CASCADE, + expires_at TEXT NOT NULL, + used_at TEXT, + created_at TEXT NOT NULL DEFAULT (datetime('now')) +); + +-- +goose Down +DROP TABLE password_reset_tokens; diff --git a/internal/handler/password_reset.go b/internal/handler/password_reset.go new file mode 100644 index 00000000..add65c66 --- /dev/null +++ b/internal/handler/password_reset.go @@ -0,0 +1,187 @@ +package handler + +import ( + "context" + "crypto/rand" + "crypto/sha256" + "database/sql" + "encoding/hex" + "encoding/json" + "fmt" + "net/http" + "strings" + "time" + + "github.com/calnode/calnode/internal/mailer" + "golang.org/x/crypto/bcrypt" +) + +// resetLinkTTL is longer than the magic-link TTL: a reset email sits in an +// inbox longer than a just-requested login link, and the token is single-use +// and hashed at rest either way. +const resetLinkTTL = time.Hour + +// RequestPasswordReset handles POST /v1/auth/password/forgot — emails a one-time +// password-reset link to the address if it belongs to an active account with email +// login enabled. Always responds 200 with the same message (no account enumeration): +// the user lookup happens inline (a single indexed SELECT), but token generation, +// the DB write, and the email send are dispatched to a background goroutine rather +// than awaited, so a timing attacker can't distinguish "no such user" from "found +// user, still emailing" — the same pattern as RequestMagicLink. +func (h *Handler) RequestPasswordReset(w http.ResponseWriter, r *http.Request) { + r.Body = http.MaxBytesReader(w, r.Body, 4<<10) + var req struct { + Email string `json:"email"` + } + if err := json.NewDecoder(r.Body).Decode(&req); err != nil { + h.writeError(w, http.StatusBadRequest, "invalid JSON") + return + } + email := strings.TrimSpace(strings.ToLower(req.Email)) + if email != "" { + var userID string + var archivedAt sql.NullString + var emailLogin int + err := h.db.QueryRowContext(r.Context(), + `SELECT id, archived_at, email_login FROM users WHERE email = ?`, email). + Scan(&userID, &archivedAt, &emailLogin) + if err == nil && !archivedAt.Valid && emailLogin != 0 { + go h.sendPasswordReset(context.WithoutCancel(r.Context()), userID, email) + } + } + h.writeJSON(w, http.StatusOK, map[string]any{ + "ok": true, + "message": "If an account with that email exists, a password-reset link is on its way.", + }) +} + +// sendPasswordReset generates and stores a token and emails the reset link. Run in a +// background goroutine by RequestPasswordReset so its variable cost (DB write + +// mailer round-trip) never shows up in the HTTP response timing. +func (h *Handler) sendPasswordReset(ctx context.Context, userID, email string) { + rawBytes := make([]byte, 32) + if _, err := rand.Read(rawBytes); err != nil { + h.logger.ErrorContext(ctx, "password reset: rand", "error", err) + return + } + raw := hex.EncodeToString(rawBytes) + sum := sha256.Sum256([]byte(raw)) + tokenHash := hex.EncodeToString(sum[:]) + expiresAt := time.Now().UTC().Add(resetLinkTTL).Format(time.RFC3339) + + if _, err := h.db.ExecContext(ctx, + `INSERT INTO password_reset_tokens (token_hash, user_id, expires_at) VALUES (?, ?, ?)`, + tokenHash, userID, expiresAt); err != nil { + h.logger.ErrorContext(ctx, "password reset: store token", "error", err) + return + } + + link := h.baseURL + "/admin/reset-password?token=" + raw + if h.mailer != nil { + if err := h.mailer.Send(ctx, resetPasswordMessage(email, link)); err != nil { + h.logger.ErrorContext(ctx, "password reset: send email", "error", err, "user_id", userID) + } + } +} + +// ResetPassword handles POST /v1/auth/password/reset — consumes a reset token and +// sets a new password. The token proves ownership of the email address, so no session +// is required; on success every existing session is revoked and a fresh one is +// created, signing the user straight in (same assurance as VerifyMagicLink). +func (h *Handler) ResetPassword(w http.ResponseWriter, r *http.Request) { + r.Body = http.MaxBytesReader(w, r.Body, 4<<10) + var req struct { + Token string `json:"token"` + NewPassword string `json:"new_password"` + } + if err := json.NewDecoder(r.Body).Decode(&req); err != nil { + h.writeError(w, http.StatusBadRequest, "invalid JSON") + return + } + // Validate the password before touching the token so a weak-password attempt + // doesn't burn the single-use link. + if msg := validatePassword(req.NewPassword); msg != "" { + h.writeError(w, http.StatusBadRequest, msg) + return + } + if req.Token == "" { + h.writeError(w, http.StatusBadRequest, "reset link is invalid or has expired") + return + } + sum := sha256.Sum256([]byte(req.Token)) + tokenHash := hex.EncodeToString(sum[:]) + now := time.Now().UTC().Format(time.RFC3339) + + // Atomically consume: only succeeds if unused and unexpired (single-use, race-safe). + res, err := h.db.ExecContext(r.Context(), + `UPDATE password_reset_tokens SET used_at = ? WHERE token_hash = ? AND used_at IS NULL AND expires_at > ?`, + now, tokenHash, now) + if err != nil { + h.logger.ErrorContext(r.Context(), "password reset: consume", "error", err) + h.writeError(w, http.StatusInternalServerError, "internal error") + return + } + if n, _ := res.RowsAffected(); n == 0 { + h.writeError(w, http.StatusBadRequest, "reset link is invalid or has expired") + return + } + + var userID string + var archivedAt sql.NullString + var emailLogin int + if err := h.db.QueryRowContext(r.Context(), + `SELECT u.id, u.archived_at, u.email_login FROM password_reset_tokens t JOIN users u ON u.id = t.user_id WHERE t.token_hash = ?`, + tokenHash).Scan(&userID, &archivedAt, &emailLogin); err != nil { + h.logger.ErrorContext(r.Context(), "password reset: load user", "error", err) + h.writeError(w, http.StatusInternalServerError, "internal error") + return + } + // The token was emailed to this address, so saying *why* it can't be used leaks + // nothing — but an archived or email-login-disabled account must not gain access. + if archivedAt.Valid || emailLogin == 0 { + h.writeError(w, http.StatusForbidden, "password reset is not available for this account") + return + } + + newHash, err := bcrypt.GenerateFromPassword([]byte(req.NewPassword), bcrypt.DefaultCost) + if err != nil { + h.logger.ErrorContext(r.Context(), "password reset: bcrypt", "error", err) + h.writeError(w, http.StatusInternalServerError, "internal error") + return + } + if _, err := h.db.ExecContext(r.Context(), + `UPDATE users SET password_hash = ? WHERE id = ?`, string(newHash), userID); err != nil { + h.logger.ErrorContext(r.Context(), "password reset: update", "error", err) + h.writeError(w, http.StatusInternalServerError, "internal error") + return + } + // Unauthenticated request, so there is no current session to preserve — revoke + // everything (a compromised session must not survive the reset), then sign in. + if _, err := h.db.ExecContext(r.Context(), + `DELETE FROM sessions WHERE user_id = ?`, userID); err != nil { + h.logger.ErrorContext(r.Context(), "password reset: revoke sessions", "error", err, "user_id", userID) + } + if err := h.createSession(r.Context(), w, userID); err != nil { + // The password IS reset; only the convenience sign-in failed. The login + // page bounces to /admin/login without a session, so report success and + // let the user sign in with the new password. + h.logger.ErrorContext(r.Context(), "password reset: create session", "error", err) + } + h.writeJSON(w, http.StatusOK, map[string]any{"ok": true}) +} + +// resetPasswordMessage builds the reset-link email (multipart text + minimal HTML). +func resetPasswordMessage(to, link string) mailer.Message { + return mailer.Message{ + To: []string{to}, + Subject: "Reset your Calnode password", + Text: "Click the link below to set a new Calnode password. It expires in 1 hour and can be used once.\n\n" + + link + "\n\nIf you didn't request this, you can ignore this email — your password stays unchanged.", + HTML: fmt.Sprintf(`
`+ + `

Click the button below to set a new Calnode password. It expires in 1 hour and can be used once.

`+ + `

Set a new password

`+ + `

Or paste this link into your browser:
%s

`+ + `

If you didn't request this, you can ignore this email — your password stays unchanged.

`, + link, link, link), + } +} diff --git a/internal/handler/password_reset_test.go b/internal/handler/password_reset_test.go new file mode 100644 index 00000000..5ec4dc60 --- /dev/null +++ b/internal/handler/password_reset_test.go @@ -0,0 +1,241 @@ +package handler_test + +import ( + "crypto/sha256" + "database/sql" + "encoding/hex" + "net/http" + "net/http/httptest" + "strings" + "testing" + "time" + + "golang.org/x/crypto/bcrypt" +) + +func resetReq(token, newPassword string) *http.Request { + body := `{"token":"` + token + `","new_password":"` + newPassword + `"}` + return httptest.NewRequest(http.MethodPost, "/v1/auth/password/reset", strings.NewReader(body)) +} + +func seedResetToken(t *testing.T, db *sql.DB, raw, userID string, expiresAt time.Time) string { + t.Helper() + sum := sha256.Sum256([]byte(raw)) + hash := hex.EncodeToString(sum[:]) + if _, err := db.Exec(`INSERT INTO password_reset_tokens (token_hash,user_id,expires_at) VALUES (?,?,?)`, + hash, userID, expiresAt.UTC().Format(time.RFC3339)); err != nil { + t.Fatalf("seed reset token: %v", err) + } + return hash +} + +func seedPasswordUser(t *testing.T, db *sql.DB, id, email, password string, emailLogin int) { + t.Helper() + hash, _ := bcrypt.GenerateFromPassword([]byte(password), bcrypt.MinCost) + if _, err := db.Exec(`INSERT INTO users (id,email,name,iana_timezone,is_admin,email_login,password_hash) VALUES (?,?,?, 'UTC',0,?,?)`, + id, email, "Test User", emailLogin, string(hash)); err != nil { + t.Fatalf("seed user: %v", err) + } +} + +func TestPasswordReset_requestUnknownEmailGeneric(t *testing.T) { + h, db := newTestHandlerDB(t) + rec := httptest.NewRecorder() + req := httptest.NewRequest(http.MethodPost, "/v1/auth/password/forgot", + strings.NewReader(`{"email":"nobody@example.com"}`)) + h.RequestPasswordReset(rec, req) + if rec.Code != http.StatusOK { + t.Fatalf("status = %d; want 200 (generic)", rec.Code) + } + var n int + db.QueryRow(`SELECT COUNT(*) FROM password_reset_tokens`).Scan(&n) //nolint:errcheck + if n != 0 { + t.Errorf("token rows = %d; want 0 for unknown email", n) + } +} + +func TestPasswordReset_requestKnownEmailCreatesToken(t *testing.T) { + h, db := newTestHandlerDB(t) + seedPasswordUser(t, db, "u1", "known@example.com", "oldpassword", 1) + + rec := httptest.NewRecorder() + req := httptest.NewRequest(http.MethodPost, "/v1/auth/password/forgot", + strings.NewReader(`{"email":"known@example.com"}`)) + h.RequestPasswordReset(rec, req) + if rec.Code != http.StatusOK { + t.Fatalf("status = %d; want 200", rec.Code) + } + + deadline := time.Now().Add(2 * time.Second) + for time.Now().Before(deadline) { + var n int + db.QueryRow(`SELECT COUNT(*) FROM password_reset_tokens WHERE user_id = 'u1'`).Scan(&n) //nolint:errcheck + if n == 1 { + return + } + time.Sleep(5 * time.Millisecond) + } + t.Fatal("no token row for u1 after waiting (background send never completed)") +} + +func TestPasswordReset_requestSkipsArchivedAndDisabled(t *testing.T) { + h, db := newTestHandlerDB(t) + seedPasswordUser(t, db, "u1", "archived@example.com", "oldpassword", 1) + db.Exec(`UPDATE users SET archived_at = '2026-01-01T00:00:00Z' WHERE id = 'u1'`) //nolint:errcheck + seedPasswordUser(t, db, "u2", "disabled@example.com", "oldpassword", 0) + + for _, email := range []string{"archived@example.com", "disabled@example.com"} { + rec := httptest.NewRecorder() + req := httptest.NewRequest(http.MethodPost, "/v1/auth/password/forgot", + strings.NewReader(`{"email":"`+email+`"}`)) + h.RequestPasswordReset(rec, req) + if rec.Code != http.StatusOK { + t.Fatalf("%s: status = %d; want generic 200", email, rec.Code) + } + } + time.Sleep(100 * time.Millisecond) // no goroutine should have been spawned; fail loudly if one was + var n int + db.QueryRow(`SELECT COUNT(*) FROM password_reset_tokens`).Scan(&n) //nolint:errcheck + if n != 0 { + t.Errorf("token rows = %d; want 0 (archived and email-login-disabled get no email)", n) + } +} + +func TestPasswordReset_happyPath(t *testing.T) { + h, db := newTestHandlerDB(t) + seedPasswordUser(t, db, "u1", "alice@example.com", "oldpassword", 1) + seedResetToken(t, db, "rawtoken123", "u1", time.Now().Add(time.Hour)) + + rec := httptest.NewRecorder() + h.ResetPassword(rec, resetReq("rawtoken123", "newpassword1")) + if rec.Code != http.StatusOK { + t.Fatalf("reset status = %d; want 200 — %s", rec.Code, rec.Body.String()) + } + found := false + for _, c := range rec.Result().Cookies() { + if c.Name == "calnode_session" && c.Value != "" { + found = true + } + } + if !found { + t.Error("reset did not set a session cookie (user should be signed straight in)") + } + + // New password works, old one doesn't. + rec2 := httptest.NewRecorder() + h.LoginEmail(rec2, loginEmailReq("alice@example.com", "newpassword1")) + if rec2.Code != http.StatusOK { + t.Errorf("login with new password: got %d; want 200", rec2.Code) + } + rec3 := httptest.NewRecorder() + h.LoginEmail(rec3, loginEmailReq("alice@example.com", "oldpassword")) + if rec3.Code != http.StatusUnauthorized { + t.Errorf("login with old password: got %d; want 401", rec3.Code) + } +} + +func TestPasswordReset_tokenSingleUse(t *testing.T) { + h, db := newTestHandlerDB(t) + seedPasswordUser(t, db, "u1", "a@example.com", "oldpassword", 1) + seedResetToken(t, db, "oncetoken", "u1", time.Now().Add(time.Hour)) + + rec := httptest.NewRecorder() + h.ResetPassword(rec, resetReq("oncetoken", "newpassword1")) + if rec.Code != http.StatusOK { + t.Fatalf("first reset: got %d; want 200", rec.Code) + } + rec2 := httptest.NewRecorder() + h.ResetPassword(rec2, resetReq("oncetoken", "anotherpw2")) + if rec2.Code != http.StatusBadRequest { + t.Errorf("second reset with same token: got %d; want 400", rec2.Code) + } +} + +func TestPasswordReset_expiredToken(t *testing.T) { + h, db := newTestHandlerDB(t) + seedPasswordUser(t, db, "u1", "a@example.com", "oldpassword", 1) + seedResetToken(t, db, "expiredtok", "u1", time.Now().Add(-time.Minute)) + + rec := httptest.NewRecorder() + h.ResetPassword(rec, resetReq("expiredtok", "newpassword1")) + if rec.Code != http.StatusBadRequest { + t.Errorf("expired reset: got %d; want 400", rec.Code) + } +} + +func TestPasswordReset_weakPasswordDoesNotConsume(t *testing.T) { + h, db := newTestHandlerDB(t) + seedPasswordUser(t, db, "u1", "a@example.com", "oldpassword", 1) + hash := seedResetToken(t, db, "retrytoken", "u1", time.Now().Add(time.Hour)) + + rec := httptest.NewRecorder() + h.ResetPassword(rec, resetReq("retrytoken", "short")) + if rec.Code != http.StatusBadRequest { + t.Fatalf("weak password reset: got %d; want 400", rec.Code) + } + var n int + db.QueryRow(`SELECT COUNT(*) FROM password_reset_tokens WHERE token_hash = ? AND used_at IS NULL`, hash).Scan(&n) //nolint:errcheck + if n != 1 { + t.Fatalf("weak attempt consumed the token; want it still usable") + } + + // Same link works with a good password. + rec2 := httptest.NewRecorder() + h.ResetPassword(rec2, resetReq("retrytoken", "goodpassword1")) + if rec2.Code != http.StatusOK { + t.Errorf("retry with good password: got %d; want 200", rec2.Code) + } +} + +func TestPasswordReset_archivedAndDisabledForbidden(t *testing.T) { + h, db := newTestHandlerDB(t) + seedPasswordUser(t, db, "u1", "archived@example.com", "oldpassword", 1) + db.Exec(`UPDATE users SET archived_at = '2026-01-01T00:00:00Z' WHERE id = 'u1'`) //nolint:errcheck + seedResetToken(t, db, "archtok", "u1", time.Now().Add(time.Hour)) + seedPasswordUser(t, db, "u2", "disabled@example.com", "oldpassword", 0) + seedResetToken(t, db, "distok", "u2", time.Now().Add(time.Hour)) + + for _, tc := range []struct{ raw string }{{"archtok"}, {"distok"}} { + rec := httptest.NewRecorder() + h.ResetPassword(rec, resetReq(tc.raw, "newpassword1")) + if rec.Code != http.StatusForbidden { + t.Errorf("token %s: got %d; want 403", tc.raw, rec.Code) + } + for _, c := range rec.Result().Cookies() { + if c.Name == "calnode_session" { + t.Errorf("token %s: must not create a session", tc.raw) + } + } + } +} + +func TestPasswordReset_revokesOldSessions(t *testing.T) { + h, db := newTestHandlerDB(t) + seedPasswordUser(t, db, "u1", "a@example.com", "oldpassword", 1) + future := time.Now().UTC().Add(24 * time.Hour).Format(time.RFC3339) + db.Exec(`INSERT INTO sessions (id,user_id,expires_at) VALUES ('old1','u1',?),('old2','u1',?)`, future, future) //nolint:errcheck + seedResetToken(t, db, "sesstoken", "u1", time.Now().Add(time.Hour)) + + rec := httptest.NewRecorder() + h.ResetPassword(rec, resetReq("sesstoken", "newpassword1")) + if rec.Code != http.StatusOK { + t.Fatalf("reset: got %d; want 200", rec.Code) + } + var ids []string + rows, err := db.Query(`SELECT id FROM sessions WHERE user_id = 'u1'`) //nolint:errcheck + if err != nil { + t.Fatalf("list sessions: %v", err) + } + defer rows.Close() + for rows.Next() { + var id string + rows.Scan(&id) //nolint:errcheck + ids = append(ids, id) + } + if len(ids) != 1 { + t.Fatalf("sessions after reset = %v; want exactly the fresh one", ids) + } + if ids[0] == "old1" || ids[0] == "old2" { + t.Errorf("pre-reset session %q survived the reset", ids[0]) + } +} diff --git a/internal/server/server.go b/internal/server/server.go index 460b46f7..ae98ec81 100644 --- a/internal/server/server.go +++ b/internal/server/server.go @@ -279,6 +279,8 @@ func New(ctx context.Context, cfg *config.Config, db *sql.DB, logger *slog.Logge mux.HandleFunc("POST /v1/auth/login/email", loginRL(h.LoginEmail)) mux.HandleFunc("POST /v1/auth/magic-link/request", loginRL(h.RequestMagicLink)) mux.HandleFunc("GET /v1/auth/magic-link/verify", loginRL(h.VerifyMagicLink)) + mux.HandleFunc("POST /v1/auth/password/forgot", loginRL(h.RequestPasswordReset)) + mux.HandleFunc("POST /v1/auth/password/reset", loginRL(h.ResetPassword)) // OAuth login (browser sessions for admin UI). authRL := RateLimit(10, time.Minute) From 86cc414c4baacf0a51e8819c9fe1e6d420e91fd6 Mon Sep 17 00:00:00 2001 From: Wynne Pirini Date: Thu, 17 Sep 2026 14:05:53 +1200 Subject: [PATCH 4/6] fix(booking): rename honeypot off the autofill-mapped company name (#33) Browsers map a field named company to the organization autofill entry (ignoring autocomplete=off) and fill it for real humans, who are then rejected as bots with 400 invalid submission. Rename to hp_extra on the booking page and embed widget; the stale company key is now an ignored unknown field, so old embeds keep working. --- internal/handler/booking_abuse_test.go | 23 +++++++++++++++++++++-- internal/handler/booking_handler.go | 7 +++++-- internal/handler/embed.js | 4 ++-- internal/handler/templates/book.html | 8 +++++--- 4 files changed, 33 insertions(+), 9 deletions(-) diff --git a/internal/handler/booking_abuse_test.go b/internal/handler/booking_abuse_test.go index 4398b64a..228ec968 100644 --- a/internal/handler/booking_abuse_test.go +++ b/internal/handler/booking_abuse_test.go @@ -8,13 +8,13 @@ import ( "testing" ) -// TestCreateBooking_honeypotRejected: a filled honeypot ("company") field marks an +// TestCreateBooking_honeypotRejected: a filled honeypot ("hp_extra") field marks an // automated submission and is rejected without creating a booking. func TestCreateBooking_honeypotRejected(t *testing.T) { h, key, _ := setupWorkspace(t) slug, _ := seedEventTypeHTTP(t, h, key) - body := fmt.Sprintf(`{"event_type_slug":%q,"start_at":"2026-06-15T09:00:00Z","name":"Bot","email":"bot@example.com","company":"Acme Spam Co"}`, slug) + body := fmt.Sprintf(`{"event_type_slug":%q,"start_at":"2026-06-15T09:00:00Z","name":"Bot","email":"bot@example.com","hp_extra":"Acme Spam Co"}`, slug) req := httptest.NewRequest(http.MethodPost, "/v1/bookings", strings.NewReader(body)) req.Header.Set("Content-Type", "application/json") rec := httptest.NewRecorder() @@ -25,6 +25,25 @@ func TestCreateBooking_honeypotRejected(t *testing.T) { } } +// TestCreateBooking_legacyCompanyFieldIgnored: the honeypot used to be named "company", +// which browsers autofill for real humans (#33). A submission carrying the old field +// (e.g. an autofilled legacy page, or a stale embed) must be treated as a normal +// booking, not a bot. +func TestCreateBooking_legacyCompanyFieldIgnored(t *testing.T) { + h, key, _ := setupWorkspace(t) + slug, _ := seedEventTypeHTTP(t, h, key) + + body := fmt.Sprintf(`{"event_type_slug":%q,"start_at":"2026-06-15T09:00:00Z","name":"Sam","email":"sam@example.com","company":"Acme Corp"}`, slug) + req := httptest.NewRequest(http.MethodPost, "/v1/bookings", strings.NewReader(body)) + req.Header.Set("Content-Type", "application/json") + rec := httptest.NewRecorder() + h.CreateBooking(rec, req) + + if rec.Code != http.StatusCreated { + t.Errorf("booking with legacy company value: %d; want 201 — %s", rec.Code, rec.Body.String()) + } +} + // TestCreateBooking_perEmailThrottle: one email can create up to the hourly cap // across distinct slots, then is throttled with 429. func TestCreateBooking_perEmailThrottle(t *testing.T) { diff --git a/internal/handler/booking_handler.go b/internal/handler/booking_handler.go index 471c81ce..c695305e 100644 --- a/internal/handler/booking_handler.go +++ b/internal/handler/booking_handler.go @@ -649,7 +649,10 @@ func (h *Handler) CreateBooking(w http.ResponseWriter, r *http.Request) { // calnode_lang cookie, and a site owner's forced lang= override wouldn't be visible // from headers alone. See internal-docs/i18n-plan.md. Language string `json:"language"` - Company string `json:"company"` // honeypot: a hidden form field; must stay empty + // Deliberately NOT named "company": browsers map that to the organization + // autofill entry (ignoring autocomplete="off") and fill this invisible field + // for real humans, who are then rejected as bots (#33). + Honeypot string `json:"hp_extra"` Answers []struct { QuestionID string `json:"question_id"` Value string `json:"value"` @@ -662,7 +665,7 @@ func (h *Handler) CreateBooking(w http.ResponseWriter, r *http.Request) { // Honeypot: a field hidden from humans on the booking form. A non-empty value // means an automated submission — reject with a generic error. - if strings.TrimSpace(req.Company) != "" { + if strings.TrimSpace(req.Honeypot) != "" { h.logger.InfoContext(r.Context(), "booking rejected: honeypot filled") h.writeError(w, http.StatusBadRequest, "invalid submission") return diff --git a/internal/handler/embed.js b/internal/handler/embed.js index 4853ddea..f9f1c279 100644 --- a/internal/handler/embed.js +++ b/internal/handler/embed.js @@ -555,7 +555,7 @@ var back = el('button', { class: 'back-btn', html: SVG_BACK + ' ' + t(this.i18n, 'back') }); back.addEventListener('click', function () { self.state.view = 'pick'; self.render(); }); var form = el('form', { novalidate: 'novalidate' }); - var hp = el('input', { type: 'text', tabindex: '-1', autocomplete: 'off' }); + var hp = el('input', { type: 'text', name: 'hp_extra', tabindex: '-1', autocomplete: 'off' }); form.appendChild(el('div', { 'aria-hidden': 'true', style: 'position:absolute;left:-5000px;height:0;width:0;overflow:hidden;' }, [hp])); var name = el('input', { type: 'text', required: 'required', autocomplete: 'name', placeholder: t(this.i18n, 'name_placeholder') }); var email = el('input', { type: 'email', required: 'required', autocomplete: 'email', placeholder: t(this.i18n, 'email_placeholder') }); @@ -603,7 +603,7 @@ }); fetch(BASE + '/v1/bookings', { method: 'POST', headers: { 'Content-Type': 'application/json' }, - body: JSON.stringify({ event_type_slug: self.slug, start_at: slot.start, name: name.value.trim(), email: email.value.trim().toLowerCase(), timezone: TZ, language: self.locale, company: hp.value, answers: answers }), + body: JSON.stringify({ event_type_slug: self.slug, start_at: slot.start, name: name.value.trim(), email: email.value.trim().toLowerCase(), timezone: TZ, language: self.locale, hp_extra: hp.value, answers: answers }), }).then(function (r) { return r.json().then(function (data) { return { ok: r.ok, status: r.status, data: data }; }); }).then(function (res) { diff --git a/internal/handler/templates/book.html b/internal/handler/templates/book.html index e2bd96d6..4145bedb 100644 --- a/internal/handler/templates/book.html +++ b/internal/handler/templates/book.html @@ -191,9 +191,11 @@

{{.Name}}

+ it is rejected server-side. Real submissions leave it empty. The field is + deliberately NOT named "company": browsers map that to the organization + autofill entry and fill it for real humans, who are then rejected (#33). -->
@@ -682,7 +684,7 @@

{{call .T "booking_confirmed"}}

email: form.email.value.trim(), timezone: TZ, language: LOCALE, - company: form.company.value, + hp_extra: form.hp_extra.value, answers, }), }); From c22fcc29b533a74a6280ad049a216b479913d7ad Mon Sep 17 00:00:00 2001 From: Wynne Pirini Date: Thu, 17 Sep 2026 14:47:18 +1200 Subject: [PATCH 5/6] fix(caldav): discover and check every calendar, not just the first (#42) Discovery picked one VEVENT collection and ListCalendars reported only that; conflict checks never read any other collection even when selected. Now discovery returns all collections (seeded with server-side names), ListCalendars lists them live with a stored-selection fallback, and conflict checks REPORT every selected collection. The picker destination (Book) is clearable and the Check/Book headers say what they mean. --- frontend/src/routes/calendar/+page.svelte | 28 ++- internal/caldav/caldav.go | 20 +- internal/caldav/caldav_test.go | 174 +++++++++++++++++- internal/caldav/calendars.go | 101 +++++++++- internal/caldav/discovery.go | 102 +++++++--- .../db/migrations/00059_caldav_home_url.sql | 10 + 6 files changed, 376 insertions(+), 59 deletions(-) create mode 100644 internal/db/migrations/00059_caldav_home_url.sql diff --git a/frontend/src/routes/calendar/+page.svelte b/frontend/src/routes/calendar/+page.svelte index 67fec4f5..1b0f4231 100644 --- a/frontend/src/routes/calendar/+page.svelte +++ b/frontend/src/routes/calendar/+page.svelte @@ -159,12 +159,23 @@ } } - // Exclusive within this account's list. The backend clears the flag across every other + // Exclusive within this account's list, but clearable: selecting the current + // destination again removes it (plain radios can't express "none", and the + // backend accepts a save with no destination — writes then fall back to the + // account's bound calendar). The backend clears the flag across every other // account when it saves, so the UI only has to keep this list consistent. function pickDestination(chosen: CalendarPick) { pickerCals = pickerCals.map((c) => ({ ...c, is_destination: c.id === chosen.id })); } + function toggleDestination(chosen: CalendarPick) { + if (chosen.is_destination) { + pickerCals = pickerCals.map((c) => ({ ...c, is_destination: false })); + } else { + pickDestination(chosen); + } + } + async function savePicker(c: CalendarConnection) { pickerSaving = true; pickerErr = ''; @@ -319,12 +330,12 @@ {:else}

Tick the calendars to check for conflicts, and choose the one bookings are - written into. + written into (select it again to clear the choice).

Check - Book + Destination Calendar
{#each pickerCals as cal (cal.id)} @@ -333,13 +344,14 @@ - + - + pickDestination(cal)} - aria-label="Write bookings into {cal.name}" /> + onchange={() => toggleDestination(cal)} + aria-label="Write bookings into {cal.name} (select again to clear)" /> {cal.name} diff --git a/internal/caldav/caldav.go b/internal/caldav/caldav.go index cf143e75..d37e0cf0 100644 --- a/internal/caldav/caldav.go +++ b/internal/caldav/caldav.go @@ -208,9 +208,9 @@ func (c *Client) conflictConns(ctx context.Context, userID string) ([]conn, erro return nil, err } // Resolve selection + decrypt after the cursor is closed (single-connection DB pool; the - // ConflictCalendarIDs query would deadlock against an open cursor). CalDAV binds one - // calendar per connection, so the sub-calendar picker is simply an on/off toggle: an empty - // result means the user deselected it. + // ConflictCalendarIDs query would deadlock against an open cursor). One connection row + // can bind several selected collections (the picker), so every selected URL gets its + // own REPORT below; an empty result means the user deselected the whole account. var conns []conn for _, d := range data { calIDs, err := calendar.ConflictCalendarIDs(ctx, c.db, "caldav", userID, d.username, d.calURL) @@ -225,7 +225,9 @@ func (c *Client) conflictConns(ctx context.Context, userID string) ([]conn, erro c.logger.Warn("caldav: skipping connection with bad credentials", "user_id", userID, "error", err) continue } - conns = append(conns, conn{id: d.id, username: d.username, password: string(pw), calURL: d.calURL}) + for _, calURL := range calIDs { + conns = append(conns, conn{id: d.id, username: d.username, password: string(pw), calURL: calURL}) + } } return conns, nil } @@ -234,8 +236,8 @@ func (c *Client) conflictConns(ctx context.Context, userID string) ([]conn, erro // a user can connect several. On a new connection: check_conflicts=1, and it becomes the // destination only if the user has none yet. On a re-connect (row already exists for this // account): the existing check_conflicts/is_destination flags are preserved and only the -// password + calendar URL are refreshed. Mirrors gcal.saveToken. -func (c *Client) saveConnection(ctx context.Context, userID, accountEmail, password, calURL string) error { +// password + calendar URL (+ home URL) are refreshed. Mirrors gcal.saveToken. +func (c *Client) saveConnection(ctx context.Context, userID, accountEmail, password, calURL, homeURL string) error { pwEnc, err := c.encrypt([]byte(password)) if err != nil { return err @@ -261,9 +263,9 @@ func (c *Client) saveConnection(ctx context.Context, userID, accountEmail, passw if _, err := tx.ExecContext(ctx, ` INSERT INTO calendar_connections (id, user_id, provider, account_email, access_token_enc, calendar_id, - check_conflicts, is_destination, created_at) - VALUES (?, ?, 'caldav', ?, ?, ?, ?, ?, ?)`, - uid.New(), userID, accountEmail, pwEnc, calURL, checkConflicts, isDest, now); err != nil { + check_conflicts, is_destination, caldav_home_url, created_at) + VALUES (?, ?, 'caldav', ?, ?, ?, ?, ?, ?, ?)`, + uid.New(), userID, accountEmail, pwEnc, calURL, checkConflicts, isDest, homeURL, now); err != nil { return fmt.Errorf("caldav: save insert: %w", err) } return tx.Commit() diff --git a/internal/caldav/caldav_test.go b/internal/caldav/caldav_test.go index ab24ed7a..fee684fa 100644 --- a/internal/caldav/caldav_test.go +++ b/internal/caldav/caldav_test.go @@ -81,10 +81,10 @@ func TestSaveConnection_multiAccountDestinationInvariants(t *testing.T) { ctx := context.Background() seedUser(t, c.db, "u1") - if err := c.saveConnection(ctx, "u1", "a@icloud.com", "pw1", "https://x/cal/a/"); err != nil { + if err := c.saveConnection(ctx, "u1", "a@icloud.com", "pw1", "https://x/cal/a/", "https://x/home/"); err != nil { t.Fatalf("save a: %v", err) } - if err := c.saveConnection(ctx, "u1", "b@icloud.com", "pw2", "https://x/cal/b/"); err != nil { + if err := c.saveConnection(ctx, "u1", "b@icloud.com", "pw2", "https://x/cal/b/", "https://x/home/"); err != nil { t.Fatalf("save b: %v", err) } @@ -103,7 +103,7 @@ func TestSaveConnection_multiAccountDestinationInvariants(t *testing.T) { if err := svc.SetDestination(ctx, "u1", "caldav", "b@icloud.com"); err != nil { t.Fatalf("set dest b: %v", err) } - if err := c.saveConnection(ctx, "u1", "b@icloud.com", "pw2-rotated", "https://x/cal/b/"); err != nil { + if err := c.saveConnection(ctx, "u1", "b@icloud.com", "pw2-rotated", "https://x/cal/b/", "https://x/home/"); err != nil { t.Fatalf("re-save b: %v", err) } if dest := destEmail(t, c.db, "u1"); dest != "b@icloud.com" { @@ -114,15 +114,20 @@ func TestSaveConnection_multiAccountDestinationInvariants(t *testing.T) { } } -// A fake CalDAV server: answers the three discovery PROPFINDs, a calendar-query REPORT, and -// records PUT/DELETE so write-back can be asserted. -func fakeServer(t *testing.T, putBody, deletePath *string) *httptest.Server { +// A fake CalDAV server: answers the three discovery PROPFINDs, a calendar-query REPORT, +// and records PUT/DELETE so write-back can be asserted. The home exposes two VEVENT +// calendars (work, personal) plus a tasks-only collection that must be filtered out. +// REPORT targets are appended to reportPaths so multi-calendar conflict checks can be +// asserted. +func fakeServer(t *testing.T, putBody, deletePath *string, reportPaths *[]string) *httptest.Server { t.Helper() const principal = `/HTTP/1.1 200 OK/principals/user/` const home = `/principals/user/HTTP/1.1 200 OK/calendars/user/` const collections = ` /calendars/user/HTTP/1.1 200 OK /calendars/user/work/HTTP/1.1 200 OKCalendar + /calendars/user/personal/HTTP/1.1 200 OKPersonal + /calendars/user/tasks/HTTP/1.1 200 OKReminders ` // One busy event, one TRANSPARENT (ignored), one CANCELLED (ignored). const report = `/calendars/user/work/ev1.icsHTTP/1.1 200 OKBEGIN:VCALENDAR @@ -144,6 +149,14 @@ DTSTART:20260625T130000Z DTEND:20260625T140000Z STATUS:CANCELLED END:VEVENT +END:VCALENDAR` + const personalReport = `/calendars/user/personal/ev9.icsHTTP/1.1 200 OKBEGIN:VCALENDAR +BEGIN:VEVENT +UID:personal-busy@x +DTSTART:20260625T150000Z +DTEND:20260625T160000Z +SUMMARY:Personal busy +END:VEVENT END:VCALENDAR` return httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { @@ -158,8 +171,17 @@ END:VCALENDAR` ) -// Connect validates the given CalDAV credentials by discovering the user's default calendar -// collection, then stores the connection (encrypting the app password). Returns the resolved -// account email and calendar URL. The serverURL is a base (a Presets value or a user-supplied -// Nextcloud/custom URL); discovery resolves the rest. An auth failure or a server with no -// VEVENT-capable calendar is surfaced as an error so the connect form can show it. +// Connect validates the given CalDAV credentials by discovering the user's calendar +// collections, then stores the connection (encrypting the app password) bound to the +// default collection. Every discovered collection is seeded into connection_calendars +// so the calendar picker lists them all with their server-side names. Returns the +// resolved account email and calendar URL. The serverURL is a base (a Presets value or +// a user-supplied Nextcloud/custom URL); discovery resolves the rest. An auth failure +// or a server with no VEVENT-capable calendar is surfaced as an error so the connect +// form can show it. func (c *Client) Connect(ctx context.Context, userID, serverURL, username, password string) (accountEmail, calURL string, err error) { username = strings.TrimSpace(username) if username == "" || password == "" || strings.TrimSpace(serverURL) == "" { return "", "", fmt.Errorf("caldav: server URL, username and app password are all required") } - calURL, err = c.discoverCalendar(ctx, strings.TrimSpace(serverURL), username, password) + home, cals, err := c.discoverCalendars(ctx, strings.TrimSpace(serverURL), username, password) if err != nil { return "", "", err } + calURL = preferDefault(cals) accountEmail = strings.ToLower(username) - if err := c.saveConnection(ctx, userID, accountEmail, password, calURL); err != nil { + if err := c.saveConnection(ctx, userID, accountEmail, password, calURL, home); err != nil { + return "", "", err + } + if err := c.seedDiscoveredCalendars(ctx, userID, accountEmail, calURL, cals); err != nil { return "", "", err } return accountEmail, calURL, nil } -// discoverCalendar walks principal → calendar-home → calendar collections and returns the URL -// of the first VEVENT-capable calendar collection. -func (c *Client) discoverCalendar(ctx context.Context, serverURL, username, password string) (string, error) { +// preferDefault picks the bound collection out of the discovered set: a calendar +// literally named/pathed "calendar" wins when present, else the first one. Discovery +// guarantees at least one entry. +func preferDefault(cals []discoveredCalendar) string { + first := cals[0].URL + for _, cal := range cals { + if strings.EqualFold(cal.Name, "Calendar") || strings.Contains(strings.ToLower(cal.URL), "/calendar") { + return cal.URL + } + } + return first +} + +// discoverCalendars walks principal → calendar-home → calendar collections and returns +// the home URL plus every VEVENT-capable collection, with server-side display names. +func (c *Client) discoverCalendars(ctx context.Context, serverURL, username, password string) (home string, cals []discoveredCalendar, err error) { // 1. current-user-principal — try the base URL, then the RFC 5785 well-known path. principal, base, err := c.findPrincipal(ctx, serverURL, username, password) if err != nil { - return "", err + return "", nil, err } // 2. calendar-home-set on the principal. ms, homeReqURL, err := c.propfind(ctx, principal, username, password, "0", propCalendarHomeSet) if err != nil { - return "", err + return "", nil, err } - var home string for _, r := range ms.Responses { if h := r.okProp().CalendarHomeSet.Href; h != "" { home = resolveRef(homeReqURL, h) @@ -74,12 +102,11 @@ func (c *Client) discoverCalendar(ctx context.Context, serverURL, username, pass home = principal } - // 3. list calendar collections (Depth 1) and pick the first that supports VEVENT. + // 3. list calendar collections (Depth 1), keeping every VEVENT-capable one. ms, homeReqURL, err = c.propfind(ctx, home, username, password, "1", propCalendarCollections) if err != nil { - return "", err + return "", nil, err } - var firstCal string for _, r := range ms.Responses { p := r.okProp() if p.ResourceType.Calendar == nil { @@ -89,19 +116,44 @@ func (c *Client) discoverCalendar(ctx context.Context, serverURL, username, pass continue // e.g. a tasks/reminders or birthdays calendar } u := resolveRef(homeReqURL, r.Href) - if firstCal == "" { - firstCal = u - } - // Prefer a calendar literally named/pathed "calendar" as the default when present. - if strings.EqualFold(p.DisplayName, "Calendar") || strings.Contains(strings.ToLower(u), "/calendar") { - return u, nil + name := strings.TrimSpace(p.DisplayName) + if name == "" { + name = path.Base(strings.TrimSuffix(u, "/")) + if name == "" || name == "." || name == "/" { + name = "Calendar" + } } + cals = append(cals, discoveredCalendar{URL: u, Name: name}) } - if firstCal == "" { + if len(cals) == 0 { _ = base - return "", fmt.Errorf("caldav: no writable calendar found on the server") + return "", nil, fmt.Errorf("caldav: no writable calendar found on the server") + } + return home, cals, nil +} + +// seedDiscoveredCalendars records every discovered collection in connection_calendars +// so the calendar picker lists them all immediately with server-side names. The bound +// (default) collection is seeded conflict-checked, preserving the pre-picker behaviour +// where a fresh connection checks its one calendar; the rest start unchecked for the +// user to tick. Names refresh on every (re-)connect; the user's check_conflicts / +// is_destination flags are preserved (only name is overwritten on conflict) — a +// reconnect never resets the picker. +func (c *Client) seedDiscoveredCalendars(ctx context.Context, userID, accountEmail, boundCalURL string, cals []discoveredCalendar) error { + for _, cal := range cals { + check := 0 + if cal.URL == boundCalURL { + check = 1 + } + if _, err := c.db.ExecContext(ctx, ` + INSERT INTO connection_calendars (id, user_id, provider, account_email, calendar_id, name, check_conflicts, is_destination) + VALUES (?, ?, 'caldav', ?, ?, ?, ?, 0) + ON CONFLICT(user_id, provider, account_email, calendar_id) DO UPDATE SET name = excluded.name`, + uid.New(), userID, accountEmail, cal.URL, cal.Name, check); err != nil { + return fmt.Errorf("caldav: seed calendars: %w", err) + } } - return firstCal, nil + return nil } // findPrincipal resolves current-user-principal, trying the base URL first and then the diff --git a/internal/db/migrations/00059_caldav_home_url.sql b/internal/db/migrations/00059_caldav_home_url.sql new file mode 100644 index 00000000..47a51bb6 --- /dev/null +++ b/internal/db/migrations/00059_caldav_home_url.sql @@ -0,0 +1,10 @@ +-- +goose Up +-- CalDAV calendar-home URL per connection. Discovery resolves principal -> +-- calendar-home -> collections once at connect time; the home URL is needed later +-- to re-list every collection (ListCalendars) without the server base URL, which +-- is not stored. Existing connections get NULL and keep the previous behaviour +-- (the single bound calendar) until reconnected. +ALTER TABLE calendar_connections ADD COLUMN caldav_home_url TEXT; + +-- +goose Down +-- SQLite doesn't support DROP COLUMN before v3.35; leave the column in place. From d4cb4f2448de6643b44381afb7bbd935a825695c Mon Sep 17 00:00:00 2001 From: Wynne Pirini Date: Thu, 17 Sep 2026 16:13:08 +1200 Subject: [PATCH 6/6] docs(zoom): multi-member playbook and settings-page warning (#35) An unpublished Zoom app only authorizes same-account users, and Zoom refuses outsiders before Calnode is involved, so member connect failures are policy, not a bug. Document the ranked options (built-in video first) and say so on the settings page, where the old step 2 implied any team member could connect. --- docs/ZOOM.md | 47 +++++++++++++++++++ .../src/routes/settings/zoom/+page.svelte | 12 ++++- 2 files changed, 58 insertions(+), 1 deletion(-) create mode 100644 docs/ZOOM.md diff --git a/docs/ZOOM.md b/docs/ZOOM.md new file mode 100644 index 00000000..def2f247 --- /dev/null +++ b/docs/ZOOM.md @@ -0,0 +1,47 @@ +# Zoom integration + +One Zoom OAuth app per Calnode instance (entered in **Settings → Zoom**), then each +host connects their **own** Zoom account through that app from the **Calendar** page. +Bookings with a Zoom location get a real meeting link minted under the assigned host's +account. The settings page shows the exact Redirect URL and required scope +(`meeting:write`); the app can stay unpublished for a single-account team. + +## Multi-member teams: read this before inviting people + +Zoom — not Calnode — decides who may authorize your app. An **unpublished** app can +only be authorized by users on the **same Zoom account** as the app owner. A member on +a different Zoom account clicks Connect and Zoom itself refuses; no callback, token, +or error ever reaches Calnode, so there is nothing to retry or work around in settings. + +Your options, ranked honestly: + +1. **Use the video already in the box.** Built-in LiveKit needs no per-user OAuth, no + Zoom app, and no publication — see [VIDEO.md](VIDEO.md). For most teams hitting this + wall, this is the answer: it converts "Zoom won't let us" into "we don't need Zoom". +2. **Same Zoom account.** If everyone already belongs to one Zoom organization, join the + accounts and the unpublished app works for all members instantly. The price is real: + joined accounts lose independence (the admin can see usage and enforce settings) and + paid features need a license per head from the account owner. +3. **Publish the app.** Zoom Marketplace review (technical design docs, security + controls, vulnerability testing). Built for vendors shipping a product, not for a + team running its own scheduler. Only take this path if you are productizing Calnode. +4. **Stopgap: Zoom's beta share link.** Lets up to 100 external users authorize for up + to 90 days (4 weeks + extensions). A bridge, not a home — afterwards, publish or + lose access. + +What does **not** work: re-entering credentials, reinstalling, different hosting +(Docker vs Railway vs Render all behave identically), or having the member confirm +they own an active Zoom account. The refusal happens on Zoom's consent page under all +of them. + +## Note for contributors + +Per-member BYO Zoom apps (each host registers their own app) would technically work — +a member always authorizes their own app — and a PR doing it would be welcome if built +within these bounds: the instance app stays the default and fallback; per-user +credentials live entirely inside `internal/zoom` (storage + client routing) with no +changes to booking or meeting-mint flows; secrets use the existing envelope encryption. +That said, it is explicitly **not on the maintainers' roadmap**: it is a big chunk of +work that asks the least technical users to do the most technical thing (register a +Zoom Marketplace app each), while LiveKit already covers the need with zero +configuration. If you build it, build it within the bounds above. diff --git a/frontend/src/routes/settings/zoom/+page.svelte b/frontend/src/routes/settings/zoom/+page.svelte index 8a9c4328..7e3cff95 100644 --- a/frontend/src/routes/settings/zoom/+page.svelte +++ b/frontend/src/routes/settings/zoom/+page.svelte @@ -60,7 +60,10 @@ 2
Choose a General App (User-managed OAuth). You can keep it - unpublished — your own team's Zoom users can still connect. + unpublished — but only Zoom users on the same Zoom account + as the app owner can then connect. Members on other Zoom accounts are refused by Zoom + itself (see docs/ZOOM.md + for the options).
  • @@ -130,6 +133,13 @@

    {redirectURI}
    • +
    + Multi-member limit: only Zoom users on the same Zoom + account as this app's owner can connect an unpublished app — members elsewhere are + refused by Zoom before Calnode is involved. Options: + docs/ZOOM.md + (built-in video needs no Zoom app at all). +
    {/if}