From e328844a1baab74510e90edaf4ce1234cd59f14e Mon Sep 17 00:00:00 2001 From: xarmian Date: Fri, 17 Apr 2026 03:03:49 +0000 Subject: [PATCH 1/8] fix: resolve five open bugs (BUG-585, BUG-586, BUG-588, BUG-589, BUG-590) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit BUG-585 — Code-block copy no longer includes ``` fences Editor.svelte: ProseMirror plugin overrides copy/cut when the selection is inside a code_block node and writes raw textBetween to the clipboard. NodeView for non-mermaid code blocks now shows a hover "Copy" button that uses the existing copyToClipboard() util (with execCommand fallback). BUG-586 — Wiki-link picker matches on item ref Editor.svelte: getFilteredLinks() now also matches formatItemRef(item), so typing [[DOC-535]] finds items by their issue ID. Picker dropdown shows the ref as a badge; {#each} key switched to doc.id so duplicate titles across collections don't collide. BUG-588 — Can unlink OAuth provider when password is configured Adds a password_set column to track whether a user has a usable password vs. the random placeholder hash given to OAuth users. CreateUser sets it true, UpdateUser sets it true when a password is provided, and ValidatePassword auto-upgrades it on any successful email/password login (which transparently upgrades pre-existing users who linked OAuth after signing up with a real password — the OAuth placeholder hash cannot match user-supplied plaintext, so this is safe). handleOAuthUnlink now permits removing the last provider when user.HasPassword() is true. BUG-589 — Pre-auth pages render standalone +layout.svelte: isAuthPage now also matches /forgot-password and /reset-password/* so those pages don't inherit the authenticated sidebar/topbar layout. BUG-590 — Search no longer crashes with null results store.Search() returned a nil Results slice on no-match queries, which Go marshals as JSON null; CommandPalette then crashed on results.length. Backend now normalizes nil to []SearchResult{} before returning. CommandPalette also coalesces resp.results ?? [] on the initial search and loadMore paths as belt-and-suspenders hardening. --- internal/models/user.go | 9 ++ internal/server/handlers_cloud.go | 15 +- .../migrations/043_user_password_set.sql | 13 ++ .../pgmigrations/023_user_password_set.sql | 13 ++ internal/store/search.go | 7 + internal/store/users.go | 25 ++- web/src/lib/components/editor/Editor.svelte | 145 +++++++++++++++++- .../components/search/CommandPalette.svelte | 8 +- web/src/routes/+layout.svelte | 9 +- 9 files changed, 223 insertions(+), 21 deletions(-) create mode 100644 internal/store/migrations/043_user_password_set.sql create mode 100644 internal/store/pgmigrations/023_user_password_set.sql diff --git a/internal/models/user.go b/internal/models/user.go index b090a162..1a5be778 100644 --- a/internal/models/user.go +++ b/internal/models/user.go @@ -22,6 +22,7 @@ type User struct { StripeCustomerID string `json:"-"` // Never serialized PlanOverrides string `json:"plan_overrides,omitempty"` // JSON overrides for per-user limits OAuthProviders string `json:"-"` // JSON array of linked providers, e.g. ["github","google"] + PasswordSet bool `json:"password_set"` // True if the user explicitly set a password (vs. OAuth placeholder hash) DisabledAt string `json:"disabled_at,omitempty"` // Non-empty = account disabled LastActiveAt string `json:"last_active_at,omitempty"` // Last authenticated API request CreatedAt time.Time `json:"created_at"` @@ -55,6 +56,14 @@ func (u *User) HasOAuthProvider(provider string) bool { return false } +// HasPassword returns true if the user has explicitly set a password that +// they can sign in with. OAuth-only users have a random placeholder hash +// stored in PasswordHash which can't actually be used to log in, so this +// bit is tracked separately from PasswordHash being non-empty. +func (u *User) HasPassword() bool { + return u.PasswordSet +} + // UserCreate is the input for registering a new user. type UserCreate struct { Email string `json:"email"` diff --git a/internal/server/handlers_cloud.go b/internal/server/handlers_cloud.go index edb40919..d3bb8884 100644 --- a/internal/server/handlers_cloud.go +++ b/internal/server/handlers_cloud.go @@ -385,14 +385,11 @@ func (s *Server) handleOAuthUnlink(w http.ResponseWriter, r *http.Request) { return } - // Ensure user won't be locked out: they must have another linked - // provider remaining. All users have a password hash (OAuth users get - // a random one), so we can't distinguish "has usable password" from - // "has unusable random hash". Requiring another provider is the safe - // default. Users who set a real password via the reset flow can unlink - // their last provider since they'll still have password-based login. - // TODO: track whether the user has explicitly set a password to allow - // unlinking the last provider in that case. + // Ensure user won't be locked out after unlinking. They must retain + // at least one usable sign-in method: either another linked OAuth + // provider, or an explicitly-set password. OAuth-only users have a + // random placeholder hash in password_hash that can't actually be + // used to log in, which is why we track password_set separately. providers := user.GetOAuthProviders() hasOtherProvider := false for _, p := range providers { @@ -401,7 +398,7 @@ func (s *Server) handleOAuthUnlink(w http.ResponseWriter, r *http.Request) { break } } - if !hasOtherProvider { + if !hasOtherProvider && !user.HasPassword() { writeError(w, http.StatusBadRequest, "bad_request", "Cannot unlink your only sign-in method. Link another provider or set a password first.") return diff --git a/internal/store/migrations/043_user_password_set.sql b/internal/store/migrations/043_user_password_set.sql new file mode 100644 index 00000000..4c6cdff6 --- /dev/null +++ b/internal/store/migrations/043_user_password_set.sql @@ -0,0 +1,13 @@ +-- Track whether the user has explicitly set a password (vs. the random +-- placeholder hash given to OAuth users in CreateOAuthUser). Used by the +-- OAuth unlink flow to decide whether the user will still have a way to +-- sign in after removing their last linked provider. +ALTER TABLE users ADD COLUMN password_set INTEGER NOT NULL DEFAULT 0; + +-- Backfill: any user with no linked OAuth providers must have registered +-- via email/password, so they have a usable password. +UPDATE users +SET password_set = 1 +WHERE oauth_providers IS NULL + OR oauth_providers = '' + OR oauth_providers = '[]'; diff --git a/internal/store/pgmigrations/023_user_password_set.sql b/internal/store/pgmigrations/023_user_password_set.sql new file mode 100644 index 00000000..1958d078 --- /dev/null +++ b/internal/store/pgmigrations/023_user_password_set.sql @@ -0,0 +1,13 @@ +-- Track whether the user has explicitly set a password (vs. the random +-- placeholder hash given to OAuth users in CreateOAuthUser). Used by the +-- OAuth unlink flow to decide whether the user will still have a way to +-- sign in after removing their last linked provider. +ALTER TABLE users ADD COLUMN IF NOT EXISTS password_set BOOLEAN NOT NULL DEFAULT FALSE; + +-- Backfill: any user with no linked OAuth providers must have registered +-- via email/password, so they have a usable password. +UPDATE users +SET password_set = TRUE +WHERE oauth_providers IS NULL + OR oauth_providers = '' + OR oauth_providers = '[]'; diff --git a/internal/store/search.go b/internal/store/search.go index e44cd7ea..f1377def 100644 --- a/internal/store/search.go +++ b/internal/store/search.go @@ -442,6 +442,13 @@ func (s *Store) Search(params SearchParams) (*SearchResponse, error) { total = len(results) } + // Normalize nil → empty slice so JSON always serializes `results` as + // `[]` not `null`. Frontend consumers (CommandPalette) read .length + // without a null check. + if results == nil { + results = []SearchResult{} + } + return &SearchResponse{Results: results, Total: total, Limit: params.Limit, Offset: params.Offset, Facets: facets}, nil } diff --git a/internal/store/users.go b/internal/store/users.go index a99b89dd..fb67f16c 100644 --- a/internal/store/users.go +++ b/internal/store/users.go @@ -21,7 +21,7 @@ var usernameCleanRe = regexp.MustCompile(`[^a-z0-9-]+`) const bcryptCost = 12 // user SELECT columns — used by all user queries. -const userColumns = `id, email, username, name, password_hash, role, avatar_url, totp_secret, totp_enabled, recovery_codes, plan, plan_expires_at, stripe_customer_id, plan_overrides, oauth_providers, disabled_at, last_active_at, created_at, updated_at` +const userColumns = `id, email, username, name, password_hash, role, avatar_url, totp_secret, totp_enabled, recovery_codes, plan, plan_expires_at, stripe_customer_id, plan_overrides, oauth_providers, password_set, disabled_at, last_active_at, created_at, updated_at` // scanUser scans a user row into a User struct. // Note: does NOT decrypt the TOTP secret — call store.decryptUserTOTP() after @@ -35,6 +35,7 @@ func scanUser(row interface{ Scan(...interface{}) error }) (*models.User, error) &u.ID, &u.Email, &u.Username, &u.Name, &u.PasswordHash, &u.Role, &u.AvatarURL, &u.TOTPSecret, &u.TOTPEnabled, &u.RecoveryCodes, &u.Plan, &u.PlanExpiresAt, &u.StripeCustomerID, &u.PlanOverrides, &u.OAuthProviders, + &u.PasswordSet, &disabledAt, &lastActiveAt, &createdAt, &updatedAt, ) if disabledAt.Valid { @@ -84,9 +85,9 @@ func (s *Store) CreateUser(input models.UserCreate) (*models.User, error) { ts := now() _, err = s.db.Exec(s.q(` - INSERT INTO users (id, email, username, name, password_hash, role, created_at, updated_at) - VALUES (?, ?, ?, ?, ?, ?, ?, ?) - `), id, strings.ToLower(strings.TrimSpace(input.Email)), strings.TrimSpace(input.Username), strings.TrimSpace(input.Name), string(hash), role, ts, ts) + INSERT INTO users (id, email, username, name, password_hash, role, password_set, created_at, updated_at) + VALUES (?, ?, ?, ?, ?, ?, ?, ?, ?) + `), id, strings.ToLower(strings.TrimSpace(input.Email)), strings.TrimSpace(input.Username), strings.TrimSpace(input.Name), string(hash), role, true, ts, ts) if err != nil { return nil, fmt.Errorf("insert user: %w", err) } @@ -155,6 +156,10 @@ func (s *Store) UpdateUser(id string, input models.UserUpdate) (*models.User, er } sets = append(sets, "password_hash = ?") args = append(args, string(hash)) + // Explicit password change — mark the user as having a usable password + // (clears the OAuth placeholder-hash state set by CreateOAuthUser). + sets = append(sets, "password_set = ?") + args = append(args, true) } if input.AvatarURL != nil { sets = append(sets, "avatar_url = ?") @@ -197,6 +202,18 @@ func (s *Store) ValidatePassword(email, password string) (*models.User, error) { return nil, nil // wrong password — not an error } + // A successful bcrypt compare with a user-supplied plaintext proves the + // stored hash is usable for real sign-ins (the random 64-byte placeholder + // set by CreateOAuthUser cannot be guessed). Auto-upgrade password_set so + // users who pre-date the password_set column — or who linked OAuth after + // signing up with email/password — don't get trapped in the OAuth-unlink + // check. Failure here is non-fatal: login succeeds regardless. + if !u.PasswordSet { + if _, err := s.db.Exec(s.q(`UPDATE users SET password_set = ? WHERE id = ?`), true, u.ID); err == nil { + u.PasswordSet = true + } + } + return u, nil } diff --git a/web/src/lib/components/editor/Editor.svelte b/web/src/lib/components/editor/Editor.svelte index 83d82762..a1615fe1 100644 --- a/web/src/lib/components/editor/Editor.svelte +++ b/web/src/lib/components/editor/Editor.svelte @@ -1,6 +1,7 @@