diff --git a/README.md b/README.md index a7e04ee..d904f08 100644 --- a/README.md +++ b/README.md @@ -14,7 +14,7 @@ For more detail, check out the [Potok Docs](https://potok-docs.vercel.app/) - **Multiple Vaults** — Manage and sync multiple vaults independently. - **Automatic Sync** — Watches your vault folder for changes and pushes them automatically. - **Cross-Platform** — Supports Windows and Linux. macOS is untested but might work? -- **Secure Key Storage** — Encryption passwords and API keys are stored in your OS keyring (Windows Credential Manager, macOS Keychain, Linux Secret Service). +- **Secure Key Storage** — The CLI keeps encryption passwords and API keys in your OS keyring (Windows Credential Manager, macOS Keychain, Linux Secret Service). The server stores only a SHA-256 hash of each API key, never the live secret. - **Free & Open Source** — No file size limits, no file count limits, no paywalls. ## Commands @@ -120,7 +120,7 @@ Long-running process that watches for local changes and pushes them automaticall ### Sensitive data -Passwords and API keys are stored in your OS keyring under the `potok` service — never in config files. +The CLI stores passwords and API keys in your OS keyring under the `potok` service — never in config files. If an older `config.json` still has an `api_key` field, the next `potok` command moves it into the keyring and rewrites the file. | OS | Keyring backend | |---|---| @@ -136,9 +136,10 @@ Passwords and API keys are stored in your OS keyring under the `potok` service ## Security - All encryption and decryption happens locally on your device. -- The server only stores encrypted blobs — it never sees your passwords or plaintext. -- Passwords and API keys are stored in your OS keyring, not in config files. -- Encryption uses AES via `golang.org/x/crypto`. +- The server only stores encrypted blobs — it never sees your passwords or note plaintext. +- Client secrets (the live API key and vault passphrases) live in the OS keyring, not in `config.json`. +- Server-side API keys are stored as `sha256:` digests. Keys are 256-bit values from `crypto/rand`, so a fast cryptographic hash is enough to protect them at rest (the same approach used for GitHub personal access tokens). User passwords stay on bcrypt. The live key is shown once at registration and cannot be recovered from the database. +- Encryption uses AES-GCM via `golang.org/x/crypto`; passphrases are stretched with Argon2id. ## Roadmap diff --git a/internal/client/cli/commands/legacy_apikey.go b/internal/client/cli/commands/legacy_apikey.go new file mode 100644 index 0000000..254ce0c --- /dev/null +++ b/internal/client/cli/commands/legacy_apikey.go @@ -0,0 +1,32 @@ +package commands + +import ( + "errors" + "fmt" + + "github.com/mtiluk/potok/internal/client/config" + "github.com/mtiluk/potok/internal/client/secrets" +) + +// MigrateLegacyAPIKey moves a leftover config.json api_key into the OS +// keyring and rewrites the file without the secret. If the keyring already +// has an API key, the file value is discarded so the keyring stays canonical. +func MigrateLegacyAPIKey() error { + key, present, err := config.PeekLegacyAPIKey() + if err != nil || !present { + return err + } + + if key != "" { + _, getErr := secrets.Get(secrets.APIKey) + if errors.Is(getErr, secrets.ErrNotFound) { + if err := secrets.Set(secrets.APIKey, key); err != nil { + return fmt.Errorf("failed to move API key into the OS keyring: %w", err) + } + } else if getErr != nil { + return getErr + } + } + + return config.StripLegacyAPIKey() +} diff --git a/internal/client/cli/commands/legacy_apikey_test.go b/internal/client/cli/commands/legacy_apikey_test.go new file mode 100644 index 0000000..2e49a80 --- /dev/null +++ b/internal/client/cli/commands/legacy_apikey_test.go @@ -0,0 +1,113 @@ +package commands + +import ( + "encoding/json" + "errors" + "os" + "testing" + + "github.com/mtiluk/potok/internal/client/config" + "github.com/mtiluk/potok/internal/client/secrets" + "github.com/zalando/go-keyring" +) + +func TestMigrateLegacyAPIKeyMovesPlaintextIntoKeyring(t *testing.T) { + t.Setenv("POTOK_CONFIG_DIR", t.TempDir()) + keyring.MockInit() + + path, err := config.Path() + if err != nil { + t.Fatalf("Path() = %v", err) + } + if err := os.WriteFile(path, []byte(`{ + "server_url": "https://potok.example.com", + "api_key": "potok_legacy", + "vaults": [] +} +`), 0o600); err != nil { + t.Fatalf("WriteFile() = %v", err) + } + + if err := MigrateLegacyAPIKey(); err != nil { + t.Fatalf("MigrateLegacyAPIKey() = %v", err) + } + + got, err := secrets.Get(secrets.APIKey) + if err != nil { + t.Fatalf("secrets.Get() = %v", err) + } + if got != "potok_legacy" { + t.Errorf("keyring API key = %q, want potok_legacy", got) + } + + data, err := os.ReadFile(path) + if err != nil { + t.Fatalf("ReadFile() = %v", err) + } + if containsAPIKey(t, data) { + t.Errorf("config.json still has api_key:\n%s", data) + } +} + +func TestMigrateLegacyAPIKeyDoesNotOverwriteExistingKeyring(t *testing.T) { + t.Setenv("POTOK_CONFIG_DIR", t.TempDir()) + keyring.MockInit() + + if err := secrets.Set(secrets.APIKey, "already-in-keyring"); err != nil { + t.Fatalf("secrets.Set() = %v", err) + } + + path, err := config.Path() + if err != nil { + t.Fatalf("Path() = %v", err) + } + if err := os.WriteFile(path, []byte(`{ + "server_url": "https://potok.example.com", + "api_key": "should-not-win" +} +`), 0o600); err != nil { + t.Fatalf("WriteFile() = %v", err) + } + + if err := MigrateLegacyAPIKey(); err != nil { + t.Fatalf("MigrateLegacyAPIKey() = %v", err) + } + + got, err := secrets.Get(secrets.APIKey) + if err != nil { + t.Fatalf("secrets.Get() = %v", err) + } + if got != "already-in-keyring" { + t.Errorf("keyring API key = %q, want the existing value", got) + } + + data, err := os.ReadFile(path) + if err != nil { + t.Fatalf("ReadFile() = %v", err) + } + if containsAPIKey(t, data) { + t.Errorf("config.json still has api_key:\n%s", data) + } +} + +func TestMigrateLegacyAPIKeyNoopsWhenMissing(t *testing.T) { + t.Setenv("POTOK_CONFIG_DIR", t.TempDir()) + keyring.MockInit() + + if err := MigrateLegacyAPIKey(); err != nil { + t.Fatalf("MigrateLegacyAPIKey() on missing config = %v", err) + } + if _, err := secrets.Get(secrets.APIKey); !errors.Is(err, secrets.ErrNotFound) { + t.Errorf("secrets.Get() = %v, want ErrNotFound", err) + } +} + +func containsAPIKey(t *testing.T, data []byte) bool { + t.Helper() + var raw map[string]json.RawMessage + if err := json.Unmarshal(data, &raw); err != nil { + t.Fatalf("unmarshal rewritten config: %v", err) + } + _, ok := raw["api_key"] + return ok +} diff --git a/internal/client/cli/root.go b/internal/client/cli/root.go index 3b71d9e..d795458 100644 --- a/internal/client/cli/root.go +++ b/internal/client/cli/root.go @@ -16,6 +16,10 @@ func newRootCmd() *cobra.Command { SilenceUsage: true, SilenceErrors: true, + PersistentPreRunE: func(cmd *cobra.Command, _ []string) error { + return commands.MigrateLegacyAPIKey() + }, + RunE: func(cmd *cobra.Command, _ []string) error { return cmd.Help() }, diff --git a/internal/client/config/config.go b/internal/client/config/config.go index 1e5eb91..88d402e 100644 --- a/internal/client/config/config.go +++ b/internal/client/config/config.go @@ -74,6 +74,50 @@ func Load() (*Config, error) { return &cfg, nil } +// PeekLegacyAPIKey returns a leftover plaintext api_key from config.json. +// Older clients may have written the secret into the file; current clients +// keep it in the OS keyring instead. Missing files are not an error. +func PeekLegacyAPIKey() (key string, present bool, err error) { + path, err := Path() + if err != nil { + return "", false, err + } + + data, err := os.ReadFile(path) + if errors.Is(err, fs.ErrNotExist) { + return "", false, nil + } + if err != nil { + return "", false, fmt.Errorf("config: read %s: %w", path, err) + } + + var raw map[string]json.RawMessage + if err := json.Unmarshal(data, &raw); err != nil { + return "", false, fmt.Errorf("config: parse %s: %w", path, err) + } + value, ok := raw["api_key"] + if !ok { + return "", false, nil + } + if err := json.Unmarshal(value, &key); err != nil { + return "", true, fmt.Errorf("config: api_key in %s must be a string", path) + } + return strings.TrimSpace(key), true, nil +} + +// StripLegacyAPIKey rewrites config.json without an api_key field. +func StripLegacyAPIKey() error { + _, present, err := PeekLegacyAPIKey() + if err != nil || !present { + return err + } + cfg, err := Load() + if err != nil { + return err + } + return Save(cfg) +} + func Save(cfg *Config) error { if err := cfg.Validate(); err != nil { return err diff --git a/internal/client/config/config_test.go b/internal/client/config/config_test.go index 21db7ce..ebb0d6e 100644 --- a/internal/client/config/config_test.go +++ b/internal/client/config/config_test.go @@ -5,6 +5,7 @@ import ( "os" "path/filepath" "runtime" + "strings" "testing" ) @@ -193,6 +194,59 @@ func TestVaultReturnsPointer(t *testing.T) { } } +func TestPeekAndStripLegacyAPIKey(t *testing.T) { + tempConfig(t) + path, err := Path() + if err != nil { + t.Fatalf("Path() = %v", err) + } + + key, present, err := PeekLegacyAPIKey() + if err != nil || present || key != "" { + t.Fatalf("PeekLegacyAPIKey() on missing file = (%q, %v, %v)", key, present, err) + } + + if err := os.WriteFile(path, []byte(`{ + "server_url": "https://potok.example.com", + "api_key": "potok_plain_from_old_client", + "vaults": [{"name":"notes","path":"/home/user/notes"}] +} +`), 0o600); err != nil { + t.Fatalf("WriteFile() = %v", err) + } + + key, present, err = PeekLegacyAPIKey() + if err != nil { + t.Fatalf("PeekLegacyAPIKey() = %v", err) + } + if !present || key != "potok_plain_from_old_client" { + t.Fatalf("PeekLegacyAPIKey() = (%q, %v), want the leftover secret", key, present) + } + + if err := StripLegacyAPIKey(); err != nil { + t.Fatalf("StripLegacyAPIKey() = %v", err) + } + + data, err := os.ReadFile(path) + if err != nil { + t.Fatalf("ReadFile() = %v", err) + } + if strings.Contains(string(data), "api_key") || strings.Contains(string(data), "potok_plain_from_old_client") { + t.Errorf("config.json still contains the leftover API key:\n%s", data) + } + + got, err := Load() + if err != nil { + t.Fatalf("Load() after strip = %v", err) + } + if got.ServerURL != "https://potok.example.com" { + t.Errorf("ServerURL = %q after strip", got.ServerURL) + } + if len(got.Vaults) != 1 || got.Vaults[0].Name != "notes" { + t.Errorf("Vaults = %+v, want notes preserved", got.Vaults) + } +} + func TestDirPrecedence(t *testing.T) { t.Setenv("POTOK_CONFIG_DIR", "/custom/potok") t.Setenv("XDG_CONFIG_HOME", "/xdg") diff --git a/internal/server/auth/apikey.go b/internal/server/auth/apikey.go index d552677..2c3b376 100644 --- a/internal/server/auth/apikey.go +++ b/internal/server/auth/apikey.go @@ -2,13 +2,25 @@ package auth import ( "crypto/rand" + "crypto/sha256" "encoding/base64" + "encoding/hex" "fmt" + "strings" ) var Prefix = "potok_" -const keyBytes = 32 +// APIKeyHashPrefix marks a SHA-256 digest stored in users.api_key_hash. +// Generated keys are 256-bit CSPRNG values, so a fast cryptographic hash is +// enough to protect them at rest (same approach GitHub uses for PATs). +// User passwords stay on bcrypt. +const APIKeyHashPrefix = "sha256:" + +const ( + keyBytes = 32 + sha256HexLen = sha256.Size * 2 +) func GenerateAPIKey() (key string, err error) { raw := make([]byte, keyBytes) @@ -18,3 +30,17 @@ func GenerateAPIKey() (key string, err error) { key = Prefix + base64.RawURLEncoding.EncodeToString(raw) return key, nil } + +func HashAPIKey(key string) string { + sum := sha256.Sum256([]byte(key)) + return APIKeyHashPrefix + hex.EncodeToString(sum[:]) +} + +func IsHashedAPIKey(stored string) bool { + digest, ok := strings.CutPrefix(stored, APIKeyHashPrefix) + if !ok || len(digest) != sha256HexLen { + return false + } + _, err := hex.DecodeString(digest) + return err == nil +} diff --git a/internal/server/auth/apikey_test.go b/internal/server/auth/apikey_test.go index 3b7f8f8..c95f46e 100644 --- a/internal/server/auth/apikey_test.go +++ b/internal/server/auth/apikey_test.go @@ -14,4 +14,59 @@ func TestGenerateAPIKey(t *testing.T) { if !strings.HasPrefix(valid, Prefix) { t.Errorf("GenerateAPIKey() = %v, want %v", valid, Prefix+valid) } + if IsHashedAPIKey(valid) { + t.Errorf("GenerateAPIKey() = %q, looks like a stored hash", valid) + } +} + +func TestHashAPIKey(t *testing.T) { + key, err := GenerateAPIKey() + if err != nil { + t.Fatalf("GenerateAPIKey() error: %v", err) + } + + got := HashAPIKey(key) + if got == key { + t.Fatal("HashAPIKey() returned the plaintext key") + } + if !strings.HasPrefix(got, APIKeyHashPrefix) { + t.Errorf("HashAPIKey() = %q, want prefix %q", got, APIKeyHashPrefix) + } + if !IsHashedAPIKey(got) { + t.Errorf("IsHashedAPIKey(%q) = false, want true", got) + } + if HashAPIKey(key) != got { + t.Error("HashAPIKey() is not deterministic") + } + + other, err := GenerateAPIKey() + if err != nil { + t.Fatalf("GenerateAPIKey() error: %v", err) + } + if HashAPIKey(other) == got { + t.Error("HashAPIKey() collided for two distinct keys") + } +} + +func TestIsHashedAPIKey(t *testing.T) { + valid := HashAPIKey("potok_test") + tests := []struct { + name string + in string + want bool + }{ + {"hashed", valid, true}, + {"plaintext generated", Prefix + "abc", false}, + {"empty", "", false}, + {"prefix only", APIKeyHashPrefix, false}, + {"wrong length", APIKeyHashPrefix + "abcd", false}, + {"non hex", APIKeyHashPrefix + strings.Repeat("g", 64), false}, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + if got := IsHashedAPIKey(tt.in); got != tt.want { + t.Errorf("IsHashedAPIKey(%q) = %v, want %v", tt.in, got, tt.want) + } + }) + } } diff --git a/internal/server/http/http.go b/internal/server/http/http.go index 2a155a4..1f5f98a 100644 --- a/internal/server/http/http.go +++ b/internal/server/http/http.go @@ -88,8 +88,19 @@ func (h *Handler) Register(w http.ResponseWriter, r *http.Request) { return } + w.Header().Set("Content-Type", "application/json") w.WriteHeader(http.StatusCreated) - w.Write(fmt.Appendf(nil, "User created: %v", user)) + json.NewEncoder(w).Encode(struct { + ID string `json:"id"` + Email string `json:"email"` + APIKey string `json:"api_key"` + CreatedAt time.Time `json:"created_at"` + }{ + ID: user.ID, + Email: user.Email, + APIKey: user.APIKey, + CreatedAt: user.CreatedAt, + }) } func (h *Handler) CreateVault(w http.ResponseWriter, r *http.Request) { diff --git a/internal/server/http/http_test.go b/internal/server/http/http_test.go index 497ee9e..e06b532 100644 --- a/internal/server/http/http_test.go +++ b/internal/server/http/http_test.go @@ -109,6 +109,7 @@ func TestMeEndpoint(t *testing.T) { ID string `json:"id"` Email string `json:"email"` PasswordHash string `json:"password_hash"` + APIKey string `json:"api_key"` } if err := json.Unmarshal(response.Body.Bytes(), &got); err != nil { t.Fatalf("decode response: %v (body: %s)", err, response.Body.String()) @@ -120,6 +121,9 @@ func TestMeEndpoint(t *testing.T) { if got.PasswordHash != "" { t.Errorf("response leaked password_hash: %q", got.PasswordHash) } + if got.APIKey != "" { + t.Errorf("response leaked api_key: %q", got.APIKey) + } }) } } @@ -462,4 +466,48 @@ func TestRegisterEndpoint(t *testing.T) { } }) } + + t.Run("returns the api key once and does not leak the password hash", func(t *testing.T) { + s := newTestStore(t) + handler := NewHandler(s, newTestBlobStore(t)) + + req := httptest.NewRequest(http.MethodPost, "/register", strings.NewReader( + `{"email":"a@example.com","password":"hunter2"}`, + )) + req.Header.Set("Content-Type", "application/json") + rec := httptest.NewRecorder() + handler.Register(rec, req) + + if rec.Code != http.StatusCreated { + t.Fatalf("expected status code %d, got %d (body: %s)", + http.StatusCreated, rec.Code, rec.Body.String()) + } + + var got struct { + ID string `json:"id"` + Email string `json:"email"` + APIKey string `json:"api_key"` + PasswordHash string `json:"password_hash"` + } + if err := json.Unmarshal(rec.Body.Bytes(), &got); err != nil { + t.Fatalf("decode response: %v (body: %s)", err, rec.Body.String()) + } + if got.APIKey == "" { + t.Fatal("register response omitted api_key; it must be shown once") + } + if got.PasswordHash != "" { + t.Errorf("register response leaked password_hash: %q", got.PasswordHash) + } + + lookedUp, err := s.UserByAPIKey(context.Background(), got.APIKey) + if err != nil { + t.Fatalf("UserByAPIKey(returned key) = %v", err) + } + if lookedUp.Email != "a@example.com" { + t.Errorf("UserByAPIKey email = %q, want a@example.com", lookedUp.Email) + } + if lookedUp.APIKey != "" { + t.Errorf("UserByAPIKey leaked a stored key/hash: %q", lookedUp.APIKey) + } + }) } diff --git a/internal/server/store/migrate.go b/internal/server/store/migrate.go index 6838551..5bb9005 100644 --- a/internal/server/store/migrate.go +++ b/internal/server/store/migrate.go @@ -1,6 +1,7 @@ package store import ( + "context" "errors" "fmt" @@ -8,6 +9,7 @@ import ( "github.com/golang-migrate/migrate/v4/database/sqlite" "github.com/golang-migrate/migrate/v4/source/iofs" + "github.com/mtiluk/potok/internal/server/auth" "github.com/mtiluk/potok/migrations" ) @@ -37,6 +39,47 @@ func (s *Store) Migrate() error { if err := m.Up(); err != nil && !errors.Is(err, migrate.ErrNoChange) { return fmt.Errorf("store: apply migrations: %w", err) } + if err := s.hashPlaintextAPIKeys(context.Background()); err != nil { + return err + } + return nil +} + +func (s *Store) hashPlaintextAPIKeys(ctx context.Context) error { + rows, err := s.db.QueryContext(ctx, ` + SELECT id, api_key_hash + FROM users + WHERE api_key_hash IS NOT NULL AND api_key_hash != ''`) + if err != nil { + return fmt.Errorf("store: hash plaintext api keys: %w", err) + } + defer rows.Close() + + type pending struct { + id, stored string + } + var toHash []pending + for rows.Next() { + var row pending + if err := rows.Scan(&row.id, &row.stored); err != nil { + return fmt.Errorf("store: hash plaintext api keys: %w", err) + } + if !auth.IsHashedAPIKey(row.stored) { + toHash = append(toHash, row) + } + } + if err := rows.Err(); err != nil { + return fmt.Errorf("store: hash plaintext api keys: %w", err) + } + + for _, row := range toHash { + if _, err := s.db.ExecContext(ctx, ` + UPDATE users SET api_key_hash = ? WHERE id = ?`, + auth.HashAPIKey(row.stored), row.id, + ); err != nil { + return fmt.Errorf("store: hash plaintext api keys: %w", err) + } + } return nil } diff --git a/internal/server/store/store.go b/internal/server/store/store.go index 00d87ee..fb7f740 100644 --- a/internal/server/store/store.go +++ b/internal/server/store/store.go @@ -35,12 +35,14 @@ type Vault struct { } type User struct { - ID string `json:"id"` - Email string `json:"email"` - PasswordHash string `json:"password_hash"` - IsAdmin bool `json:"is_admin"` - APIKey string `json:"api_key"` - CreatedAt time.Time `json:"created_at"` + ID string `json:"id"` + Email string `json:"email"` + PasswordHash string `json:"password_hash"` + IsAdmin bool `json:"is_admin"` + // APIKey is the plaintext key, populated only by CreateUser so it can + // be shown once. Lookups leave it empty; the database stores HashAPIKey. + APIKey string `json:"-"` + CreatedAt time.Time `json:"created_at"` } type Blob struct { @@ -182,11 +184,11 @@ func (s *Store) CreateUser(ctx context.Context, email, password string) (User, e } err = s.db.QueryRowContext(ctx, ` - INSERT INTO users (id, email, password_hash, api_key) + INSERT INTO users (id, email, password_hash, api_key_hash) VALUES (?, ?, ?, ?) ON CONFLICT (email) DO NOTHING RETURNING id, email, is_admin, created_at`, - user.ID, user.Email, user.PasswordHash, key, + user.ID, user.Email, user.PasswordHash, auth.HashAPIKey(key), ).Scan(&user.ID, &user.Email, &user.IsAdmin, &user.CreatedAt) if err != nil { @@ -206,11 +208,11 @@ func (s *Store) UserByAPIKey(ctx context.Context, apiKey string) (User, error) { var user User err := s.db.QueryRowContext(ctx, ` - SELECT id, email, password_hash, is_admin, api_key, created_at + SELECT id, email, password_hash, is_admin, created_at FROM users - WHERE api_key = ?`, - apiKey, - ).Scan(&user.ID, &user.Email, &user.PasswordHash, &user.IsAdmin, &user.APIKey, &user.CreatedAt) + WHERE api_key_hash = ?`, + auth.HashAPIKey(apiKey), + ).Scan(&user.ID, &user.Email, &user.PasswordHash, &user.IsAdmin, &user.CreatedAt) if err != nil { if err == sql.ErrNoRows { diff --git a/internal/server/store/store_test.go b/internal/server/store/store_test.go index 473eca0..e7e81a0 100644 --- a/internal/server/store/store_test.go +++ b/internal/server/store/store_test.go @@ -4,7 +4,10 @@ import ( "bytes" "context" "errors" + "strings" "testing" + + "github.com/mtiluk/potok/internal/server/auth" ) func newTestStore(t *testing.T) *Store { @@ -254,3 +257,99 @@ func TestDeleteVaultScopedToUser(t *testing.T) { t.Errorf("userA's vault should still exist after userB's delete attempt, got: %v", err) } } + +func TestCreateUserStoresHashedAPIKey(t *testing.T) { + s := newTestStore(t) + user := seedUser(t, s, "a@example.com") + + if user.APIKey == "" { + t.Fatal("CreateUser() did not return the plaintext API key") + } + if !strings.HasPrefix(user.APIKey, "potok_") { + t.Errorf("CreateUser() APIKey = %q, want potok_ prefix", user.APIKey) + } + + var stored string + if err := s.db.QueryRow(`SELECT api_key_hash FROM users WHERE id = ?`, user.ID).Scan(&stored); err != nil { + t.Fatalf("read stored hash: %v", err) + } + if stored == user.APIKey { + t.Fatal("CreateUser() stored the plaintext API key") + } + if stored != auth.HashAPIKey(user.APIKey) { + t.Errorf("stored hash = %q, want %q", stored, auth.HashAPIKey(user.APIKey)) + } +} + +func TestUserByAPIKey(t *testing.T) { + s := newTestStore(t) + user := seedUser(t, s, "a@example.com") + + got, err := s.UserByAPIKey(context.Background(), user.APIKey) + if err != nil { + t.Fatalf("UserByAPIKey() error: %v", err) + } + if got.ID != user.ID { + t.Errorf("ID = %q, want %q", got.ID, user.ID) + } + if got.Email != user.Email { + t.Errorf("Email = %q, want %q", got.Email, user.Email) + } + if got.APIKey != "" { + t.Errorf("APIKey = %q, want empty (hash must not be returned)", got.APIKey) + } + + if _, err := s.UserByAPIKey(context.Background(), ""); !errors.Is(err, ErrUserNotFound) { + t.Errorf("UserByAPIKey(\"\") = %v, want ErrUserNotFound", err) + } + if _, err := s.UserByAPIKey(context.Background(), "potok_doesnotexist"); !errors.Is(err, ErrUserNotFound) { + t.Errorf("UserByAPIKey(unknown) = %v, want ErrUserNotFound", err) + } +} + +func TestMigrateHashesLegacyPlaintextAPIKeys(t *testing.T) { + s := newTestStore(t) + const plaintext = "potok_legacyplaintextkey0123456789abcd" + + if _, err := s.db.Exec(` + INSERT INTO users (id, email, password_hash, api_key_hash) + VALUES (?, ?, ?, ?)`, + "user-legacy", "legacy@example.com", "not-a-real-hash", plaintext, + ); err != nil { + t.Fatalf("insert legacy user: %v", err) + } + + if err := s.hashPlaintextAPIKeys(context.Background()); err != nil { + t.Fatalf("hashPlaintextAPIKeys() = %v", err) + } + + var stored string + if err := s.db.QueryRow(`SELECT api_key_hash FROM users WHERE id = ?`, "user-legacy").Scan(&stored); err != nil { + t.Fatalf("read stored hash: %v", err) + } + if stored == plaintext { + t.Fatal("legacy plaintext API key was left in the database") + } + if stored != auth.HashAPIKey(plaintext) { + t.Errorf("stored hash = %q, want %q", stored, auth.HashAPIKey(plaintext)) + } + + got, err := s.UserByAPIKey(context.Background(), plaintext) + if err != nil { + t.Fatalf("UserByAPIKey(legacy key) after migrate = %v", err) + } + if got.Email != "legacy@example.com" { + t.Errorf("Email = %q, want legacy@example.com", got.Email) + } + + if err := s.hashPlaintextAPIKeys(context.Background()); err != nil { + t.Fatalf("hashPlaintextAPIKeys() second run = %v", err) + } + var storedAgain string + if err := s.db.QueryRow(`SELECT api_key_hash FROM users WHERE id = ?`, "user-legacy").Scan(&storedAgain); err != nil { + t.Fatalf("read stored hash after second run: %v", err) + } + if storedAgain != stored { + t.Errorf("second migrate pass re-hashed: %q -> %q", stored, storedAgain) + } +} diff --git a/migrations/0004_hash_api_keys.down.sql b/migrations/0004_hash_api_keys.down.sql new file mode 100644 index 0000000..5a0d45c --- /dev/null +++ b/migrations/0004_hash_api_keys.down.sql @@ -0,0 +1 @@ +ALTER TABLE users RENAME COLUMN api_key_hash TO api_key; diff --git a/migrations/0004_hash_api_keys.up.sql b/migrations/0004_hash_api_keys.up.sql new file mode 100644 index 0000000..3da732e --- /dev/null +++ b/migrations/0004_hash_api_keys.up.sql @@ -0,0 +1 @@ +ALTER TABLE users RENAME COLUMN api_key TO api_key_hash;