From b44db32bf76fddb109d03f53e3a80a52f79dd094 Mon Sep 17 00:00:00 2001 From: Kristian Wind Date: Sun, 16 Aug 2026 23:00:28 +0200 Subject: [PATCH] feat(auth): Argon2id passwords, opaque tokens, TOTP and recovery codes MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Password hashes are self-describing PHC strings, so the cost parameters can be raised later without locking anyone out: NeedsRehash reports which hashes to upgrade at the next login, when the plaintext is briefly at hand. Session, invite, reset and API tokens are stored only as SHA-256 hashes; the plaintext exists once, in the link or the reply. Unsalted is correct here and only here — the input is 256 bits of randomness that has to be found by value in a WHERE clause. feat(store): project roles with viewer/editor/owner and explicit denial Roles are strictly ordered so every check is "at least X". Access to a task is access to its project: being the assignee or the author confers nothing, or removing someone from a project would leave them holding tasks inside it. Checks take a Querier so they can run in the same transaction as the write they guard. Co-Authored-By: Claude Opus 5 --- go.mod | 3 + go.sum | 10 + internal/auth/auth_test.go | 282 ++++++++++++++++++++++++++++ internal/auth/password.go | 131 +++++++++++++ internal/auth/token.go | 57 ++++++ internal/auth/totp.go | 136 ++++++++++++++ internal/store/access.go | 141 ++++++++++++++ internal/store/access_test.go | 334 ++++++++++++++++++++++++++++++++++ 8 files changed, 1094 insertions(+) create mode 100644 internal/auth/auth_test.go create mode 100644 internal/auth/password.go create mode 100644 internal/auth/token.go create mode 100644 internal/auth/totp.go create mode 100644 internal/store/access.go create mode 100644 internal/store/access_test.go diff --git a/go.mod b/go.mod index a23fca0..fb7141a 100644 --- a/go.mod +++ b/go.mod @@ -5,11 +5,14 @@ go 1.26.3 require modernc.org/sqlite v1.56.0 require ( + github.com/boombuler/barcode v1.0.1-0.20190219062509-6c824513bacc // indirect github.com/dustin/go-humanize v1.0.1 // indirect github.com/google/uuid v1.6.0 // indirect github.com/mattn/go-isatty v0.0.24 // indirect github.com/ncruces/go-strftime v1.0.0 // indirect + github.com/pquerna/otp v1.5.0 // indirect github.com/remyoudompheng/bigfft v0.0.0-20230129092748-24d4a6f8daec // indirect + golang.org/x/crypto v0.55.0 // indirect golang.org/x/sys v0.47.0 // indirect modernc.org/libc v1.74.4 // indirect modernc.org/mathutil v1.7.1 // indirect diff --git a/go.sum b/go.sum index 1932692..ab554cc 100644 --- a/go.sum +++ b/go.sum @@ -1,3 +1,6 @@ +github.com/boombuler/barcode v1.0.1-0.20190219062509-6c824513bacc h1:biVzkmvwrH8WK8raXaxBx6fRVTlJILwEwQGL1I/ByEI= +github.com/boombuler/barcode v1.0.1-0.20190219062509-6c824513bacc/go.mod h1:paBWMcWSl3LHKBqUq+rly7CNSldXjb2rDl3JlRe0mD8= +github.com/davecgh/go-spew v1.1.0/go.mod h1:J7Y8YcW2NihsgmVo/mv3lAwl/skON4iLHjSsI+c5H38= github.com/dustin/go-humanize v1.0.1 h1:GzkhY7T5VNhEkwH0PVJgjz+fX1rhBrR7pRT3mDkpeCY= github.com/dustin/go-humanize v1.0.1/go.mod h1:Mu1zIs6XwVuF/gI1OepvI0qD18qycQx+mFykh5fBlto= github.com/google/pprof v0.0.0-20260802141513-ef3492d7dac3 h1:LMLX+LgTNWpfvCBdFebv6EsYotImrt/Ppc5cXIriCSo= @@ -10,8 +13,15 @@ github.com/mattn/go-isatty v0.0.24 h1:tGZZoVgT/KiqK1c8ocVLeDS8BSWMRd47J3Lbz7vsRe github.com/mattn/go-isatty v0.0.24/go.mod h1:nMCL3Zebbrt45jsMDgnfIwz6ydEQApk5oEI3HqDio6A= github.com/ncruces/go-strftime v1.0.0 h1:HMFp8mLCTPp341M/ZnA4qaf7ZlsbTc+miZjCLOFAw7w= github.com/ncruces/go-strftime v1.0.0/go.mod h1:Fwc5htZGVVkseilnfgOVb9mKy6w1naJmn9CehxcKcls= +github.com/pmezard/go-difflib v1.0.0/go.mod h1:iKH77koFhYxTK1pcRnkKkqfTogsbg7gZNVY4sRDYZ/4= +github.com/pquerna/otp v1.5.0 h1:NMMR+WrmaqXU4EzdGJEE1aUUI0AMRzsp96fFFWNPwxs= +github.com/pquerna/otp v1.5.0/go.mod h1:dkJfzwRKNiegxyNb54X/3fLwhCynbMspSyWKnvi1AEg= github.com/remyoudompheng/bigfft v0.0.0-20230129092748-24d4a6f8daec h1:W09IVJc94icq4NjY3clb7Lk8O1qJ8BdBEF8z0ibU0rE= github.com/remyoudompheng/bigfft v0.0.0-20230129092748-24d4a6f8daec/go.mod h1:qqbHyh8v60DhA7CoWK5oRCqLrMHRGoxYCSS9EjAz6Eo= +github.com/stretchr/objx v0.1.0/go.mod h1:HFkY916IF+rwdDfMAkV7OtwuqBVzrE8GR6GFx+wExME= +github.com/stretchr/testify v1.3.0/go.mod h1:M5WIy9Dh21IEIfnGCwXGc5bZfKNJtfHm1UVUgZn+9EI= +golang.org/x/crypto v0.55.0 h1:+KWHjbgOaAQ66dh/YlkZKHlz9ZUlq61AFirAR9ntP8M= +golang.org/x/crypto v0.55.0/go.mod h1:uq0V9dE/fzQuJtbnL+2EhWOE63vo164FY8xqEnV9xis= golang.org/x/mod v0.37.0 h1:vF1DjpVEshcIqoEaauuHebaLk1O1forxjxBaVn884JQ= golang.org/x/mod v0.37.0/go.mod h1:m8S8VeM9r4dzDwjrKO0a1sZP3YjeMamRRlD+fmR2Q/0= golang.org/x/sync v0.21.0 h1:HLII4xRRTtCRkxYp4HNFF0Js/Og6q2i++KXbg0gHCwM= diff --git a/internal/auth/auth_test.go b/internal/auth/auth_test.go new file mode 100644 index 0000000..ba58142 --- /dev/null +++ b/internal/auth/auth_test.go @@ -0,0 +1,282 @@ +package auth + +import ( + "strings" + "testing" + "time" + + "github.com/pquerna/otp" + "github.com/pquerna/otp/totp" +) + +func TestPasswordRoundTrip(t *testing.T) { + const password = "kaffe & rugbrød, tak!" + + hash, err := HashPassword(password) + if err != nil { + t.Fatalf("hash: %v", err) + } + if err := VerifyPassword(hash, password); err != nil { + t.Errorf("verify with the right password: %v", err) + } + if err := VerifyPassword(hash, password+"x"); err != ErrMismatch { + t.Errorf("verify with a wrong password: %v, want ErrMismatch", err) + } +} + +// The salt is per-hash, so the same password never produces the same stored value. +// Without that, identical hashes in a leaked table tell an attacker which accounts +// share a password. +func TestSamePasswordHashesDifferently(t *testing.T) { + a, err := HashPassword("hemmeligt") + if err != nil { + t.Fatal(err) + } + b, err := HashPassword("hemmeligt") + if err != nil { + t.Fatal(err) + } + if a == b { + t.Error("two hashes of one password are identical; the salt is not random") + } + // Both must still verify. + for _, h := range []string{a, b} { + if err := VerifyPassword(h, "hemmeligt"); err != nil { + t.Errorf("verify: %v", err) + } + } +} + +func TestPasswordHashFormat(t *testing.T) { + hash, err := HashPassword("x") + if err != nil { + t.Fatal(err) + } + if !strings.HasPrefix(hash, "$argon2id$v=19$m=19456,t=2,p=1$") { + t.Errorf("hash = %q; the parameters must be recorded in the string", hash) + } + if n := len(strings.Split(hash, "$")); n != 6 { + t.Errorf("hash has %d $-separated fields, want 6", n) + } +} + +// A stored hash that has been truncated, edited or produced by something else must +// fail closed. The one thing it must never do is verify. +func TestMalformedHashNeverVerifies(t *testing.T) { + good, err := HashPassword("hemmeligt") + if err != nil { + t.Fatal(err) + } + + broken := []string{ + "", + "not a hash", + "$argon2id$v=19$m=19456,t=2,p=1$onlyfourfields", + "$argon2i$v=19$m=19456,t=2,p=1$c2FsdHNhbHRzYWx0$aGFzaA", // wrong variant + "$argon2id$v=13$m=19456,t=2,p=1$c2FsdHNhbHRzYWx0$aGFzaA", // wrong version + "$argon2id$v=19$m=0,t=0,p=0$c2FsdHNhbHRzYWx0$aGFzaA", // zero parameters + "$argon2id$v=19$m=19456,t=2,p=1$!!!not-base64!!!$aGFzaA", + "$argon2id$v=19$m=19456,t=2,p=1$c2FsdHNhbHRzYWx0$", // empty digest + good[:len(good)-1], // digest truncated by one character + good[:len(good)-8], + } + for _, h := range broken { + t.Run(h, func(t *testing.T) { + if err := VerifyPassword(h, "hemmeligt"); err == nil { + t.Error("a malformed hash verified") + } + }) + } +} + +func TestNeedsRehash(t *testing.T) { + current, err := HashPassword("x") + if err != nil { + t.Fatal(err) + } + if NeedsRehash(current) { + t.Error("a hash made with the current parameters wants rehashing") + } + + // A hash from when the parameters were weaker should be upgraded on next login. + weak := encodeHash("x", []byte("0123456789abcdef"), 8*1024, 1, 1) + if !NeedsRehash(weak) { + t.Error("a hash with weaker parameters was not flagged for rehashing") + } + // It must still verify in the meantime, or the upgrade locks people out. + if err := VerifyPassword(weak, "x"); err != nil { + t.Errorf("an old-parameter hash no longer verifies: %v", err) + } + + if !NeedsRehash("garbage") { + t.Error("an unreadable hash should be replaced") + } +} + +func TestTokensAreUniqueAndOpaque(t *testing.T) { + seen := map[string]bool{} + for i := 0; i < 1000; i++ { + tok, err := NewToken() + if err != nil { + t.Fatal(err) + } + if seen[tok] { + t.Fatal("NewToken returned a duplicate") + } + seen[tok] = true + + if len(tok) < 40 { + t.Fatalf("token %q is shorter than 256 bits of randomness", tok) + } + // URL-safe: these end up in emailed links. + if strings.ContainsAny(tok, "+/= ") { + t.Fatalf("token %q contains characters that need escaping in a URL", tok) + } + } +} + +func TestHashTokenIsStableAndDistinct(t *testing.T) { + a, err := NewToken() + if err != nil { + t.Fatal(err) + } + b, err := NewToken() + if err != nil { + t.Fatal(err) + } + + if HashToken(a) != HashToken(a) { + t.Error("HashToken is not deterministic; a stored token could never be found again") + } + if HashToken(a) == HashToken(b) { + t.Error("two different tokens hash the same") + } + if strings.Contains(HashToken(a), a) { + t.Error("the hash contains the token it is meant to protect") + } +} + +func TestAPITokenIsRecognisable(t *testing.T) { + tok, prefix, err := NewAPIToken() + if err != nil { + t.Fatal(err) + } + if !IsAPIToken(tok) { + t.Errorf("token %q is not recognised as an API token", tok) + } + if !strings.HasPrefix(tok, prefix) { + t.Errorf("prefix %q does not match token %q", prefix, tok) + } + if IsAPIToken("some-session-id") { + t.Error("a session id was mistaken for an API token") + } +} + +func TestTOTPRoundTrip(t *testing.T) { + secret, uri, err := NewTOTPSecret("verdande", "kristian@example.com") + if err != nil { + t.Fatalf("generate: %v", err) + } + if _, err := DecodeTOTPSecret(secret); err != nil { + t.Errorf("secret %q is not valid base32: %v", secret, err) + } + if !strings.HasPrefix(uri, "otpauth://totp/") { + t.Errorf("uri = %q", uri) + } + + now := time.Date(2026, 3, 10, 10, 0, 0, 0, time.UTC) + code := codeAt(t, secret, now) + + if err := VerifyTOTP(secret, code, now); err != nil { + t.Errorf("the current code was rejected: %v", err) + } + // Apps show the code in two groups; people paste it that way. + if err := VerifyTOTP(secret, code[:3]+" "+code[3:], now); err != nil { + t.Errorf("a code with a space in it was rejected: %v", err) + } + if err := VerifyTOTP(secret, "000000", now); err == nil { + t.Error("a wrong code was accepted") + } + if err := VerifyTOTP("", code, now); err != ErrNoTOTPSecret { + t.Errorf("verifying against an empty secret: %v, want ErrNoTOTPSecret", err) + } +} + +// One step of tolerance either way covers a code entered as it rolls over and a +// phone whose clock drifts. Two steps out must fail, or a stolen code lives too long. +func TestTOTPWindow(t *testing.T) { + secret, _, err := NewTOTPSecret("verdande", "k@example.com") + if err != nil { + t.Fatal(err) + } + now := time.Date(2026, 3, 10, 10, 0, 0, 0, time.UTC) + code := codeAt(t, secret, now) + + for _, delta := range []time.Duration{-30 * time.Second, 0, 30 * time.Second} { + if err := VerifyTOTP(secret, code, now.Add(delta)); err != nil { + t.Errorf("code rejected at %v offset: %v", delta, err) + } + } + for _, delta := range []time.Duration{-90 * time.Second, 90 * time.Second} { + if err := VerifyTOTP(secret, code, now.Add(delta)); err == nil { + t.Errorf("code still accepted %v away from when it was issued", delta) + } + } +} + +func TestRecoveryCodes(t *testing.T) { + codes, hashes, err := NewRecoveryCodes() + if err != nil { + t.Fatal(err) + } + if len(codes) != len(hashes) || len(codes) == 0 { + t.Fatalf("got %d codes and %d hashes", len(codes), len(hashes)) + } + + seen := map[string]bool{} + for _, c := range codes { + if seen[c] { + t.Error("duplicate recovery code") + } + seen[c] = true + } + + // Every code must match its own entry, and nothing else. + for i, c := range codes { + if got := MatchRecoveryCode(hashes, c); got != i { + t.Errorf("code %d matched index %d", i, got) + } + } + if got := MatchRecoveryCode(hashes, "AAAAA-AAAAA"); got != -1 { + t.Errorf("an invented code matched index %d", got) + } + + // Transcription must be forgiven: read off a printout, typed back in. + c := codes[0] + for _, variant := range []string{ + strings.ToLower(c), + strings.ReplaceAll(c, "-", ""), + " " + c + " ", + strings.ToLower(strings.ReplaceAll(c, "-", " ")), + } { + if got := MatchRecoveryCode(hashes, variant); got != 0 { + t.Errorf("variant %q matched index %d, want 0", variant, got) + } + } +} + +// codeAt produces the code an authenticator app would display at that moment, +// using the same settings VerifyTOTP checks against. +func codeAt(t *testing.T, secret string, now time.Time) string { + t.Helper() + code, err := totp.GenerateCodeCustom(secret, now, totp.ValidateOpts{ + Period: totpPeriod, + Skew: totpSkew, + Digits: totpDigits, + Algorithm: otp.AlgorithmSHA1, + }) + if err != nil { + t.Fatalf("generate code: %v", err) + } + return code +} diff --git a/internal/auth/password.go b/internal/auth/password.go new file mode 100644 index 0000000..13a05c9 --- /dev/null +++ b/internal/auth/password.go @@ -0,0 +1,131 @@ +// Package auth holds password hashing, opaque tokens and TOTP — the parts of +// verdande's own user system where getting the details wrong is not recoverable +// by fixing them later. +package auth + +import ( + "crypto/rand" + "crypto/subtle" + "encoding/base64" + "errors" + "fmt" + "strings" + + "golang.org/x/crypto/argon2" +) + +// Argon2id parameters, at OWASP's current recommendation for interactive logins: +// 19 MiB of memory, two passes, no parallelism. +// +// Memory is the knob that matters — it is what makes a GPU attack expensive — but +// it is also charged per concurrent login, and verdande runs on somebody's homelab +// next to a game server. 19 MiB × a handful of simultaneous logins is affordable +// there in a way that the 64 MiB variant is not. +// +// These are the parameters for *new* hashes only. Every hash records the parameters +// it was made with, so raising these does not lock anyone out: existing passwords +// keep verifying against their own settings, and NeedsRehash reports which ones +// should be upgraded the next time their owner logs in and the plaintext is at hand. +const ( + argonMemory = 19 * 1024 // KiB + argonTime = 2 + argonThreads = 1 + argonKeyLen = 32 + saltLen = 16 +) + +var ( + ErrInvalidHash = errors.New("auth: password hash is not in the expected format") + ErrMismatch = errors.New("auth: password does not match") +) + +// HashPassword returns a PHC-format string that carries the algorithm, its +// parameters and the salt alongside the digest, so it is self-describing: +// +// $argon2id$v=19$m=19456,t=2,p=1$$ +func HashPassword(password string) (string, error) { + salt := make([]byte, saltLen) + if _, err := rand.Read(salt); err != nil { + return "", fmt.Errorf("auth: read salt: %w", err) + } + return encodeHash(password, salt, argonMemory, argonTime, argonThreads), nil +} + +// VerifyPassword reports whether password produced encoded. +// +// It returns ErrMismatch for a wrong password and ErrInvalidHash for a stored value +// that cannot be read at all. Callers must not tell those two apart to the user: +// both mean "you are not logged in", and distinguishing them leaks which accounts +// exist. +func VerifyPassword(encoded, password string) error { + params, salt, want, err := decodeHash(encoded) + if err != nil { + return err + } + got := argon2.IDKey([]byte(password), salt, params.time, params.memory, params.threads, uint32(len(want))) + // Constant time: a byte-by-byte comparison that returns early leaks how much of + // a guess was right, one request at a time. + if subtle.ConstantTimeCompare(got, want) != 1 { + return ErrMismatch + } + return nil +} + +// NeedsRehash reports whether a stored hash was made with weaker parameters than +// the ones in force now. Verifying a login is the only moment the plaintext exists, +// so it is the only moment an upgrade is possible. +func NeedsRehash(encoded string) bool { + params, _, _, err := decodeHash(encoded) + if err != nil { + return true + } + return params.memory < argonMemory || params.time < argonTime +} + +type argonParams struct { + memory uint32 + time uint32 + threads uint8 +} + +func encodeHash(password string, salt []byte, memory, time uint32, threads uint8) string { + sum := argon2.IDKey([]byte(password), salt, time, memory, threads, argonKeyLen) + b64 := base64.RawStdEncoding + return fmt.Sprintf("$argon2id$v=%d$m=%d,t=%d,p=%d$%s$%s", + argon2.Version, memory, time, threads, b64.EncodeToString(salt), b64.EncodeToString(sum)) +} + +func decodeHash(encoded string) (argonParams, []byte, []byte, error) { + parts := strings.Split(encoded, "$") + if len(parts) != 6 || parts[0] != "" || parts[1] != "argon2id" { + return argonParams{}, nil, nil, ErrInvalidHash + } + + var version int + if _, err := fmt.Sscanf(parts[2], "v=%d", &version); err != nil { + return argonParams{}, nil, nil, ErrInvalidHash + } + if version != argon2.Version { + // A hash from a different Argon2 version cannot be checked by this build. + return argonParams{}, nil, nil, ErrInvalidHash + } + + var p argonParams + if _, err := fmt.Sscanf(parts[3], "m=%d,t=%d,p=%d", &p.memory, &p.time, &p.threads); err != nil { + return argonParams{}, nil, nil, ErrInvalidHash + } + if p.memory == 0 || p.time == 0 || p.threads == 0 { + return argonParams{}, nil, nil, ErrInvalidHash + } + + b64 := base64.RawStdEncoding + salt, err := b64.DecodeString(parts[4]) + if err != nil { + return argonParams{}, nil, nil, ErrInvalidHash + } + sum, err := b64.DecodeString(parts[5]) + if err != nil || len(sum) == 0 { + return argonParams{}, nil, nil, ErrInvalidHash + } + return p, salt, sum, nil +} diff --git a/internal/auth/token.go b/internal/auth/token.go new file mode 100644 index 0000000..5f05582 --- /dev/null +++ b/internal/auth/token.go @@ -0,0 +1,57 @@ +package auth + +import ( + "crypto/rand" + "crypto/sha256" + "encoding/base64" + "encoding/hex" + "fmt" + "strings" +) + +// tokenBytes is 32 bytes — 256 bits — of randomness. These tokens sit in emailed +// links and in scripts' config files, where they may live for a long time and be +// guessed at without rate limiting, so they are sized to be unguessable rather than +// convenient. +const tokenBytes = 32 + +// NewToken returns a URL-safe random token. This is the only time the plaintext +// exists: store HashToken(t), put t in the link, and never write it down anywhere +// else. A database dump then contains nothing an attacker can present. +func NewToken() (string, error) { + b := make([]byte, tokenBytes) + if _, err := rand.Read(b); err != nil { + return "", fmt.Errorf("auth: read random: %w", err) + } + return base64.RawURLEncoding.EncodeToString(b), nil +} + +// HashToken is the lookup key for a token. +// +// SHA-256 with no salt, deliberately — unlike a password, this has to be *found* by +// its value in a WHERE clause, which a per-row salt makes impossible. That is safe +// here only because the input is 256 bits of uniform randomness: there is no +// dictionary to run against it and no work factor worth adding. +func HashToken(token string) string { + sum := sha256.Sum256([]byte(token)) + return hex.EncodeToString(sum[:]) +} + +// apiTokenPrefix marks verdande's own tokens in logs and config files, and lets +// secret scanners recognise one that has been committed by accident. +const apiTokenPrefix = "vrd_" + +// NewAPIToken returns a personal API token and the short prefix shown in the UI so +// somebody can tell two of their tokens apart without being shown either in full. +func NewAPIToken() (token, prefix string, err error) { + raw, err := NewToken() + if err != nil { + return "", "", err + } + token = apiTokenPrefix + raw + return token, token[:len(apiTokenPrefix)+6], nil +} + +// IsAPIToken reports whether a credential looks like one of verdande's API tokens +// rather than a session id, so the two are never checked against the wrong table. +func IsAPIToken(s string) bool { return strings.HasPrefix(s, apiTokenPrefix) } diff --git a/internal/auth/totp.go b/internal/auth/totp.go new file mode 100644 index 0000000..d68e4cd --- /dev/null +++ b/internal/auth/totp.go @@ -0,0 +1,136 @@ +package auth + +import ( + "crypto/rand" + "encoding/base32" + "errors" + "fmt" + "net/url" + "strings" + "time" + + "github.com/pquerna/otp" + "github.com/pquerna/otp/totp" +) + +// TOTP settings: RFC 6238 defaults, which is what every authenticator app assumes +// when a QR code does not say otherwise. +const ( + totpDigits = otp.DigitsSix + totpPeriod = 30 // seconds + // A one-step window either side, so a code entered as it rolls over — or on a + // phone whose clock is a few seconds out — is still accepted. Wider than this + // starts meaningfully extending the life of a stolen code. + totpSkew = 1 +) + +var ( + ErrInvalidCode = errors.New("auth: verification code is not valid") + ErrNoTOTPSecret = errors.New("auth: two-factor authentication is not set up") + recoveryCodeLen = 10 + recoveryCodeSets = 8 +) + +// NewTOTPSecret creates a secret for a user and the otpauth:// URI to render as a +// QR code. Issuer and account are what the authenticator app shows in its list, so +// they have to identify this verdande instance rather than just say "verdande" — +// somebody self-hosting two of them needs to tell the entries apart. +func NewTOTPSecret(issuer, account string) (secret, uri string, err error) { + key, err := totp.Generate(totp.GenerateOpts{ + Issuer: issuer, + AccountName: account, + Period: totpPeriod, + Digits: totpDigits, + Algorithm: otp.AlgorithmSHA1, // what authenticator apps actually implement + }) + if err != nil { + return "", "", fmt.Errorf("auth: generate totp secret: %w", err) + } + return key.Secret(), key.URL(), nil +} + +// TOTPURI rebuilds the otpauth:// URI for an existing secret, for showing the QR +// code again before enrolment is confirmed. +func TOTPURI(secret, issuer, account string) string { + v := url.Values{} + v.Set("secret", secret) + v.Set("issuer", issuer) + v.Set("algorithm", "SHA1") + v.Set("digits", "6") + v.Set("period", fmt.Sprint(totpPeriod)) + return "otpauth://totp/" + url.PathEscape(issuer+":"+account) + "?" + v.Encode() +} + +// VerifyTOTP checks a code against a secret at the given moment. +func VerifyTOTP(secret, code string, now time.Time) error { + if secret == "" { + return ErrNoTOTPSecret + } + // Authenticator apps display codes in groups ("123 456"); people paste what + // they see. + code = strings.ReplaceAll(strings.TrimSpace(code), " ", "") + + ok, err := totp.ValidateCustom(code, secret, now, totp.ValidateOpts{ + Period: totpPeriod, + Skew: totpSkew, + Digits: totpDigits, + Algorithm: otp.AlgorithmSHA1, + }) + if err != nil || !ok { + return ErrInvalidCode + } + return nil +} + +// NewRecoveryCodes returns single-use codes for someone who has lost their phone, +// and the hashes to store. Without these, losing a device means losing the account, +// and the only fix left is editing the database by hand. +// +// The plaintext is returned once, to be shown once. Only the hashes are stored. +func NewRecoveryCodes() (codes []string, hashes []string, err error) { + // Crockford-ish base32 without padding: unambiguous when read off a printout + // and typed back in. + const alphabet = "ABCDEFGHJKLMNPQRSTUVWXYZ23456789" + for i := 0; i < recoveryCodeSets; i++ { + b := make([]byte, recoveryCodeLen) + if _, err := rand.Read(b); err != nil { + return nil, nil, fmt.Errorf("auth: read random: %w", err) + } + var sb strings.Builder + for j, v := range b { + if j == recoveryCodeLen/2 { + sb.WriteByte('-') + } + sb.WriteByte(alphabet[int(v)%len(alphabet)]) + } + code := sb.String() + codes = append(codes, code) + hashes = append(hashes, HashToken(normalizeRecoveryCode(code))) + } + return codes, hashes, nil +} + +// MatchRecoveryCode finds which stored hash a typed code corresponds to, or -1. +// The caller must delete that hash: a recovery code works exactly once. +func MatchRecoveryCode(hashes []string, code string) int { + want := HashToken(normalizeRecoveryCode(code)) + for i, h := range hashes { + if h == want { + return i + } + } + return -1 +} + +// normalizeRecoveryCode forgives how the code was transcribed — case, the dash, +// surrounding space — so a correct code is never rejected for its formatting. +func normalizeRecoveryCode(code string) string { + r := strings.NewReplacer("-", "", " ", "", "\t", "") + return strings.ToUpper(r.Replace(strings.TrimSpace(code))) +} + +// DecodeTOTPSecret is used by tests and by the enrolment check to confirm a secret +// is well-formed base32 before it is written to the database. +func DecodeTOTPSecret(secret string) ([]byte, error) { + return base32.StdEncoding.WithPadding(base32.NoPadding).DecodeString(strings.ToUpper(secret)) +} diff --git a/internal/store/access.go b/internal/store/access.go new file mode 100644 index 0000000..9e1b7b2 --- /dev/null +++ b/internal/store/access.go @@ -0,0 +1,141 @@ +package store + +import ( + "context" + "database/sql" + "errors" +) + +// Role is a member's standing in one project. +// +// Roles are deliberately few and strictly ordered — viewer < editor < owner — so +// every permission question reduces to "is this role at least X". A capability +// model with independent flags reads as more flexible and is, in practice, the +// thing that produces a viewer who can somehow delete a section. +type Role string + +const ( + RoleOwner Role = "owner" + RoleEditor Role = "editor" + RoleViewer Role = "viewer" +) + +var roleRank = map[Role]int{RoleViewer: 1, RoleEditor: 2, RoleOwner: 3} + +// Valid reports whether r is a role verdande knows. Anything else — including the +// empty string — is not a weaker role, it is no membership at all. +func (r Role) Valid() bool { _, ok := roleRank[r]; return ok } + +// AtLeast is the single comparison every permission check goes through. +func (r Role) AtLeast(min Role) bool { + have, ok := roleRank[r] + if !ok { + return false + } + want, ok := roleRank[min] + return ok && have >= want +} + +// CanView reports whether the role may read a project and its tasks. +func (r Role) CanView() bool { return r.AtLeast(RoleViewer) } + +// CanEdit reports whether the role may change tasks: create, complete, reschedule, +// comment, attach files. This is the boundary a viewer must never cross. +func (r Role) CanEdit() bool { return r.AtLeast(RoleEditor) } + +// CanManage reports whether the role may change the project itself — rename it, +// invite and remove people, archive or delete it. Owner only. +func (r Role) CanManage() bool { return r.AtLeast(RoleOwner) } + +// ErrNoAccess means the user is not a member of the project. +// +// It is returned rather than a distinct "not found" whenever someone asks about a +// project they cannot see, so that probing ids cannot be used to learn which +// projects exist. +var ErrNoAccess = errors.New("store: no access to this project") + +// ProjectRole returns the user's role in a project. +// +// The owner is authoritative even without a project_members row, so a project can +// never end up with nobody able to administer it — a state that is unrecoverable +// from inside the app. +func ProjectRole(ctx context.Context, q Querier, projectID, userID string) (Role, error) { + var ownerID string + var deletedAt sql.NullInt64 + err := q.QueryRowContext(ctx, + `SELECT owner_id, deleted_at FROM projects WHERE id = ?`, projectID).Scan(&ownerID, &deletedAt) + if errors.Is(err, sql.ErrNoRows) { + return "", ErrNoAccess + } + if err != nil { + return "", err + } + // A project in the trash is not accessible; restoring it is a separate action + // against a separate endpoint. + if deletedAt.Valid { + return "", ErrNoAccess + } + if ownerID == userID { + return RoleOwner, nil + } + + var role Role + err = q.QueryRowContext(ctx, + `SELECT role FROM project_members WHERE project_id = ? AND user_id = ?`, + projectID, userID).Scan(&role) + if errors.Is(err, sql.ErrNoRows) { + return "", ErrNoAccess + } + if err != nil { + return "", err + } + if !role.Valid() { + // A role the database somehow holds but this build does not understand is + // treated as no access, never as a guess at what it might have meant. + return "", ErrNoAccess + } + return role, nil +} + +// RequireProjectRole checks a project against a minimum role in one call, which is +// what handlers actually want. It returns the role so a handler can also make finer +// distinctions afterwards without a second query. +func RequireProjectRole(ctx context.Context, q Querier, projectID, userID string, min Role) (Role, error) { + role, err := ProjectRole(ctx, q, projectID, userID) + if err != nil { + return "", err + } + if !role.AtLeast(min) { + return role, ErrNoAccess + } + return role, nil +} + +// TaskRole resolves access to a task through the project that holds it. Tasks have +// no permissions of their own: an assignee is not thereby an editor, and the person +// who created a task has no standing in a project they were later removed from. +func TaskRole(ctx context.Context, q Querier, taskID, userID string) (Role, error) { + var projectID string + var deletedAt sql.NullInt64 + err := q.QueryRowContext(ctx, + `SELECT project_id, deleted_at FROM tasks WHERE id = ?`, taskID).Scan(&projectID, &deletedAt) + if errors.Is(err, sql.ErrNoRows) { + return "", ErrNoAccess + } + if err != nil { + return "", err + } + if deletedAt.Valid { + return "", ErrNoAccess + } + return ProjectRole(ctx, q, projectID, userID) +} + +// Querier is the read surface shared by *sql.DB and *sql.Tx, so a permission check +// can run inside the same transaction as the write it is guarding. Checking access +// on the connection while mutating in a transaction leaves a window in which a +// membership can be revoked between the two. +type Querier interface { + QueryRowContext(ctx context.Context, query string, args ...any) *sql.Row + QueryContext(ctx context.Context, query string, args ...any) (*sql.Rows, error) +} diff --git a/internal/store/access_test.go b/internal/store/access_test.go new file mode 100644 index 0000000..5584266 --- /dev/null +++ b/internal/store/access_test.go @@ -0,0 +1,334 @@ +package store + +import ( + "context" + "database/sql" + "errors" + "testing" +) + +// The permission matrix, stated once. Everything below checks against this. +func TestRoleCapabilities(t *testing.T) { + cases := []struct { + role Role + view, edit, manage, ok bool + }{ + {RoleOwner, true, true, true, true}, + {RoleEditor, true, true, false, true}, + {RoleViewer, true, false, false, true}, + // Anything that is not one of the three is not a lesser role — it is none. + {Role(""), false, false, false, false}, + {Role("admin"), false, false, false, false}, + {Role("OWNER"), false, false, false, false}, + {Role("editor "), false, false, false, false}, + } + for _, tc := range cases { + t.Run(string(tc.role), func(t *testing.T) { + if got := tc.role.Valid(); got != tc.ok { + t.Errorf("Valid() = %v, want %v", got, tc.ok) + } + if got := tc.role.CanView(); got != tc.view { + t.Errorf("CanView() = %v, want %v", got, tc.view) + } + if got := tc.role.CanEdit(); got != tc.edit { + t.Errorf("CanEdit() = %v, want %v", got, tc.edit) + } + if got := tc.role.CanManage(); got != tc.manage { + t.Errorf("CanManage() = %v, want %v", got, tc.manage) + } + }) + } +} + +func TestRoleOrdering(t *testing.T) { + if !RoleOwner.AtLeast(RoleEditor) || !RoleOwner.AtLeast(RoleViewer) { + t.Error("owner does not outrank editor and viewer") + } + if !RoleEditor.AtLeast(RoleViewer) { + t.Error("editor does not outrank viewer") + } + if RoleViewer.AtLeast(RoleEditor) || RoleEditor.AtLeast(RoleOwner) { + t.Error("a lesser role satisfied a greater minimum") + } + if RoleOwner.AtLeast(Role("nonsense")) { + t.Error("an unknown minimum was satisfied; it must never be") + } +} + +// seedAccess builds one shared project with a member of each role, plus an outsider. +func seedAccess(t *testing.T, db *DB) { + t.Helper() + stmts := []string{ + `INSERT INTO users (id, email, name, password_hash, created_at, updated_at) VALUES + ('owner', 'owner@example.com', 'Owner', 'x', 0, 0), + ('editor', 'editor@example.com', 'Editor', 'x', 0, 0), + ('viewer', 'viewer@example.com', 'Viewer', 'x', 0, 0), + ('outsider','outsider@example.com','Outsider','x', 0, 0)`, + + `INSERT INTO projects (id, name, owner_id, created_at, updated_at) VALUES + ('shared', 'Delt projekt', 'owner', 0, 0), + ('private', 'Privat projekt', 'owner', 0, 0)`, + + `INSERT INTO project_members (project_id, user_id, role, added_at) VALUES + ('shared', 'editor', 'editor', 0), + ('shared', 'viewer', 'viewer', 0)`, + + `INSERT INTO tasks (id, project_id, content, created_by, created_at, updated_at) VALUES + ('task-shared', 'shared', 'delt opgave', 'owner', 0, 0), + ('task-private', 'private', 'privat opgave', 'owner', 0, 0)`, + } + for _, s := range stmts { + if _, err := db.Exec(s); err != nil { + t.Fatalf("seed: %v", err) + } + } +} + +func TestProjectRole(t *testing.T) { + db := openTest(t) + seedAccess(t, db) + ctx := context.Background() + + cases := []struct { + user, project string + want Role + wantErr bool + }{ + // The owner is the owner without needing a project_members row. + {"owner", "shared", RoleOwner, false}, + {"editor", "shared", RoleEditor, false}, + {"viewer", "shared", RoleViewer, false}, + {"owner", "private", RoleOwner, false}, + + // An external user sees only what has been shared with them. + {"outsider", "shared", "", true}, + {"outsider", "private", "", true}, + {"editor", "private", "", true}, + {"viewer", "private", "", true}, + + // An id that does not exist is indistinguishable from one you cannot see. + {"owner", "no-such-project", "", true}, + {"no-such-user", "shared", "", true}, + } + + for _, tc := range cases { + t.Run(tc.user+"/"+tc.project, func(t *testing.T) { + got, err := ProjectRole(ctx, db, tc.project, tc.user) + if tc.wantErr { + if !errors.Is(err, ErrNoAccess) { + t.Fatalf("err = %v, want ErrNoAccess", err) + } + return + } + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + if got != tc.want { + t.Errorf("role = %q, want %q", got, tc.want) + } + }) + } +} + +// The central promise of the sharing model: a viewer can read and cannot write. +func TestViewerCannotMutate(t *testing.T) { + db := openTest(t) + seedAccess(t, db) + ctx := context.Background() + + if _, err := RequireProjectRole(ctx, db, "shared", "viewer", RoleViewer); err != nil { + t.Errorf("viewer cannot view the project they were given: %v", err) + } + if _, err := RequireProjectRole(ctx, db, "shared", "viewer", RoleEditor); !errors.Is(err, ErrNoAccess) { + t.Errorf("viewer passed an editor check: %v", err) + } + if _, err := RequireProjectRole(ctx, db, "shared", "viewer", RoleOwner); !errors.Is(err, ErrNoAccess) { + t.Errorf("viewer passed an owner check: %v", err) + } +} + +func TestEditorCannotManageTheProject(t *testing.T) { + db := openTest(t) + seedAccess(t, db) + ctx := context.Background() + + if _, err := RequireProjectRole(ctx, db, "shared", "editor", RoleEditor); err != nil { + t.Errorf("editor cannot edit: %v", err) + } + // Renaming, inviting, deleting: owner only. + if _, err := RequireProjectRole(ctx, db, "shared", "editor", RoleOwner); !errors.Is(err, ErrNoAccess) { + t.Errorf("editor passed an owner check: %v", err) + } +} + +// An external user must see nothing but the projects actually shared with them. +func TestExternalUserSeesOnlySharedProjects(t *testing.T) { + db := openTest(t) + seedAccess(t, db) + ctx := context.Background() + + for _, project := range []string{"shared", "private"} { + if _, err := ProjectRole(ctx, db, project, "outsider"); !errors.Is(err, ErrNoAccess) { + t.Errorf("outsider reached project %q: %v", project, err) + } + } + + // Once invited, and only then. + _, err := db.Exec(`INSERT INTO project_members (project_id, user_id, role, added_at) + VALUES ('shared', 'outsider', 'viewer', 0)`) + if err != nil { + t.Fatal(err) + } + if role, err := ProjectRole(ctx, db, "shared", "outsider"); err != nil || role != RoleViewer { + t.Errorf("after being invited: role %q, err %v", role, err) + } + if _, err := ProjectRole(ctx, db, "private", "outsider"); !errors.Is(err, ErrNoAccess) { + t.Error("being invited to one project granted access to another") + } +} + +// Access to a task is access to its project, and nothing else confers it. +func TestTaskAccessFollowsItsProject(t *testing.T) { + db := openTest(t) + seedAccess(t, db) + ctx := context.Background() + + if role, err := TaskRole(ctx, db, "task-shared", "viewer"); err != nil || role != RoleViewer { + t.Errorf("viewer on a task in a shared project: role %q, err %v", role, err) + } + if _, err := TaskRole(ctx, db, "task-private", "editor"); !errors.Is(err, ErrNoAccess) { + t.Error("a task in an unshared project was reachable") + } + if _, err := TaskRole(ctx, db, "no-such-task", "owner"); !errors.Is(err, ErrNoAccess) { + t.Error("a task that does not exist did not report as inaccessible") + } +} + +// Being assigned a task, or having created one, is not a form of membership. +// Otherwise removing somebody from a project would leave them holding the tasks. +func TestAssigneeAndAuthorGetNoStanding(t *testing.T) { + db := openTest(t) + seedAccess(t, db) + ctx := context.Background() + + _, err := db.Exec(`INSERT INTO tasks (id, project_id, content, assignee_id, created_by, created_at, updated_at) + VALUES ('assigned', 'private', 'til outsider', 'outsider', 'outsider', 0, 0)`) + if err != nil { + t.Fatal(err) + } + + if _, err := TaskRole(ctx, db, "assigned", "outsider"); !errors.Is(err, ErrNoAccess) { + t.Error("being the assignee and author granted access to the project") + } +} + +// A project in the trash is not readable until it is explicitly restored. +func TestTrashedProjectIsNotAccessible(t *testing.T) { + db := openTest(t) + seedAccess(t, db) + ctx := context.Background() + + if _, err := db.Exec(`UPDATE projects SET deleted_at = 1 WHERE id = 'shared'`); err != nil { + t.Fatal(err) + } + + for _, user := range []string{"owner", "editor", "viewer"} { + if _, err := ProjectRole(ctx, db, "shared", user); !errors.Is(err, ErrNoAccess) { + t.Errorf("%s still reached a trashed project: %v", user, err) + } + } + if _, err := TaskRole(ctx, db, "task-shared", "owner"); !errors.Is(err, ErrNoAccess) { + t.Error("a task in a trashed project was still reachable") + } +} + +func TestTrashedTaskIsNotAccessible(t *testing.T) { + db := openTest(t) + seedAccess(t, db) + ctx := context.Background() + + if _, err := db.Exec(`UPDATE tasks SET deleted_at = 1 WHERE id = 'task-shared'`); err != nil { + t.Fatal(err) + } + if _, err := TaskRole(ctx, db, "task-shared", "owner"); !errors.Is(err, ErrNoAccess) { + t.Error("a trashed task was still reachable") + } +} + +// A role string the database holds but this build does not understand must fail +// closed. Reading it as "some access" is how a future migration becomes a breach. +func TestUnknownStoredRoleDeniesAccess(t *testing.T) { + db := openTest(t) + seedAccess(t, db) + ctx := context.Background() + + // The CHECK constraint refuses this on the way in, which is the first line of + // defence; write it in behind the constraint's back to test the second. + if _, err := db.Exec(`PRAGMA ignore_check_constraints = ON`); err != nil { + t.Fatal(err) + } + _, err := db.Exec(`INSERT INTO project_members (project_id, user_id, role, added_at) + VALUES ('private', 'outsider', 'superuser', 0)`) + if err != nil { + t.Skipf("could not write an invalid role to test the fallback: %v", err) + } + + if _, err := ProjectRole(ctx, db, "private", "outsider"); !errors.Is(err, ErrNoAccess) { + t.Errorf("an unrecognised role granted access: %v", err) + } +} + +// Permission checks must be able to run inside the transaction that does the write. +// Checking on the connection and then writing in a transaction leaves a window in +// which a membership can be revoked between the two — so ProjectRole takes a +// Querier, which both *sql.DB and *sql.Tx satisfy. +func TestPermissionCheckWorksInsideATransaction(t *testing.T) { + db := openTest(t) + seedAccess(t, db) + ctx := context.Background() + + err := db.Tx(ctx, func(tx *sql.Tx) error { + role, err := RequireProjectRole(ctx, tx, "shared", "editor", RoleEditor) + if err != nil { + return err + } + if role != RoleEditor { + t.Errorf("role in transaction = %q, want editor", role) + } + _, err = tx.ExecContext(ctx, + `INSERT INTO tasks (id, project_id, content, created_by, created_at, updated_at) + VALUES ('in-tx', 'shared', 'skrevet i transaktion', 'editor', 0, 0)`) + return err + }) + if err != nil { + t.Fatalf("transaction: %v", err) + } + + var n int + if err := db.QueryRow(`SELECT count(*) FROM tasks WHERE id = 'in-tx'`).Scan(&n); err != nil { + t.Fatal(err) + } + if n != 1 { + t.Error("the write inside the checked transaction did not land") + } + + // And a denied check inside a transaction rolls the whole thing back. + err = db.Tx(ctx, func(tx *sql.Tx) error { + if _, err := tx.ExecContext(ctx, + `INSERT INTO tasks (id, project_id, content, created_by, created_at, updated_at) + VALUES ('rolled-back', 'shared', 'skal rulles tilbage', 'viewer', 0, 0)`); err != nil { + return err + } + _, err := RequireProjectRole(ctx, tx, "shared", "viewer", RoleEditor) + return err + }) + if !errors.Is(err, ErrNoAccess) { + t.Fatalf("err = %v, want ErrNoAccess", err) + } + if err := db.QueryRow(`SELECT count(*) FROM tasks WHERE id = 'rolled-back'`).Scan(&n); err != nil { + t.Fatal(err) + } + if n != 0 { + t.Error("a write survived a transaction that failed its permission check") + } +}