Files
steveandClaude Opus 4.8 8ef092713f
Build image / build-and-push (push) Successful in 9s
Address Gadfly review on #5: OIDC identity guard, verified-email, timeouts
Fixes from the PR #24 adversarial review (graded 23 real / 1 false positive):

Security / correctness
- LinkOIDC no longer overwrites a different stored identity: the UPDATE
  matches only when the row has no identity yet or already carries this
  exact one, so a second IdP asserting the same verified email can't
  hijack or lock out an account (returns ErrOIDCIdentityConflict). Uses
  a single UPDATE...RETURNING (also fixes the ignored-RowsAffected /
  misleading-ErrNotFound path and the round-trip).
- Provisioning now requires a verified email for BOTH linking and JIT
  creation (was: linking only), so an unverified-email identity can't
  create an account — nor become the first admin on a fresh instance,
  nor squat an email a real user later owns.
- OIDC-identity collisions surface as the dedicated ErrOIDCIdentityConflict
  instead of the email-specific ErrEmailTaken.

Robustness
- readOIDCTxCookie requires a non-empty nonce (an empty one would make the
  callback's nonce check pass vacuously).
- Callback token exchange + verify run under a 15s context timeout so a
  slow IdP can't outlast the server write timeout.
- ensure() performs discovery outside the mutex, so concurrent cold-start
  requests don't serialize behind one another's full timeout.
- setOIDCTxCookie returns its marshal error; oidcLogin aborts rather than
  redirecting to the IdP with no tx cookie.

Maintainability
- redirectAuthError helper dedups the ~dozen callback redirects (and the
  empty-code path now logs like the rest).
- Distinct login error codes (no_email / email_unverified / oidc_conflict)
  for the UI; writeServiceError maps the OIDC sentinels; shared test issuer
  const.

Tests: unverified email refused for both link and JIT; identity-overwrite
refused while the original identity keeps working.

Co-Authored-By: Claude Opus 4.8 (1M context) <[email protected]>
Claude-Session: https://claude.ai/code/session_01JdQpdYYsTgtkJBxbcpAszi
2026-07-18 17:33:25 -04:00

293 lines
9.9 KiB
Go

package service
import (
"context"
"errors"
"log/slog"
"strings"
"time"
"gitea.stevedudenhoeffer.com/steve/pansy/internal/domain"
)
// maxPasswordLen caps accepted password length. It bounds argon2 input and
// request work, and sits far above any real password.
const maxPasswordLen = 1024
// RegisterInput is the payload for local self-service signup.
type RegisterInput struct {
Email string
DisplayName string
Password string
}
// Providers reports which login methods the server offers, so the login page
// (#6) can render the right controls. OIDCLabel is only meaningful when OIDC is
// true.
type Providers struct {
Local bool `json:"local"`
OIDC bool `json:"oidc"`
OIDCLabel string `json:"oidcLabel"`
}
// OIDCIdentity is the set of claims the API layer extracts from a verified ID
// token and hands to LoginOIDC for provisioning. Issuer and Subject come from
// the verified token (not raw claims); Email/Name come from claims.
type OIDCIdentity struct {
Issuer string
Subject string
Email string
EmailVerified bool
Name string
}
// Providers returns the enabled auth methods, so the login page renders the
// right controls. OIDC is reported only when it can actually be offered (issuer,
// client ID, and a BaseURL for the redirect URI), matching the routes that
// api.New registers.
func (s *Service) Providers() Providers {
return Providers{
Local: s.cfg.LocalAuth,
OIDC: s.cfg.OIDCReady(),
OIDCLabel: s.cfg.OIDC.ButtonLabel,
}
}
// LoginOIDC provisions and returns the pansy user for a verified OIDC identity,
// issuing no session itself (the caller does). The IdP has already gated access,
// so PANSY_REGISTRATION does not apply. Resolution order:
// 1. an existing user with the same (issuer, subject) — a returning OIDC user;
// 2. else an existing user with the same email — linked to this identity (so
// one person isn't split across a local and an OIDC account);
// 3. else a new just-in-time account stamped with the identity.
//
// Steps 2 and 3 require a verified email: linking on an unverified email would
// let anyone who can assert that email at an IdP take over an account, and
// JIT-creating on one would let them squat an email a real user later owns
// (then the real user's different identity would be refused by LinkOIDC). An
// identity with no email can't be provisioned at all (email is the account key).
// Returning users (step 1) skip the email check — their identity is already
// proven.
func (s *Service) LoginOIDC(ctx context.Context, id OIDCIdentity) (*domain.User, error) {
if id.Issuer == "" || id.Subject == "" {
return nil, domain.ErrInvalidInput
}
// 1. Returning OIDC user (identity already proven; email state irrelevant).
u, err := s.store.GetUserByOIDC(ctx, id.Issuer, id.Subject)
if err == nil {
return u, nil
}
if !errors.Is(err, domain.ErrNotFound) {
return nil, err
}
// Provisioning (link or create) requires a verified email.
email := normalizeEmail(id.Email)
if email == "" {
return nil, domain.ErrOIDCNoEmail
}
if !id.EmailVerified {
return nil, domain.ErrOIDCEmailUnverified
}
// 2. Link to an existing account by email. LinkOIDC refuses to overwrite a
// different stored identity (returns ErrOIDCIdentityConflict).
existing, err := s.store.GetUserByEmail(ctx, email)
switch {
case err == nil:
return s.store.LinkOIDC(ctx, existing.ID, id.Issuer, id.Subject)
case !errors.Is(err, domain.ErrNotFound):
return nil, err
}
// 3. Just-in-time provisioning.
name := strings.TrimSpace(id.Name)
if name == "" {
name = emailLocalPart(email)
}
return s.store.CreateUser(ctx, &domain.User{
Email: email,
DisplayName: name,
OIDCIssuer: &id.Issuer,
OIDCSubject: &id.Subject,
}, true) // allowSignup: the IdP gates access, so registration policy is bypassed.
}
// Register creates a local (password) account and returns it. The first user on
// a fresh instance becomes admin and may always register (bootstrap), even when
// PANSY_REGISTRATION=closed; afterward, closed registration is enforced.
func (s *Service) Register(ctx context.Context, in RegisterInput) (*domain.User, error) {
if !s.cfg.LocalAuth {
return nil, domain.ErrLocalAuthDisabled
}
email := normalizeEmail(in.Email)
displayName := strings.TrimSpace(in.DisplayName)
if email == "" || displayName == "" || in.Password == "" || len(in.Password) > maxPasswordLen {
return nil, domain.ErrInvalidInput
}
// Cheap best-effort gate so a closed instance doesn't burn argon2 work on
// signups it will reject anyway. The authoritative, race-free gate — plus
// atomic first-user-is-admin assignment and duplicate-email detection — lives
// in store.CreateUser's single INSERT.
if !s.cfg.RegistrationOpen() {
n, err := s.store.CountUsers(ctx)
if err != nil {
return nil, err
}
if n > 0 {
return nil, domain.ErrRegistrationClosed
}
}
hash, err := hashPassword(in.Password)
if err != nil {
return nil, err
}
return s.store.CreateUser(ctx, &domain.User{
Email: email,
DisplayName: displayName,
PasswordHash: &hash,
}, s.cfg.RegistrationOpen())
}
// Login verifies an email/password pair and returns the user. Unknown-email and
// wrong-password both return domain.ErrInvalidCredentials, and both spend the
// same argon2 work, so neither the error nor the timing reveals which failed.
func (s *Service) Login(ctx context.Context, email, password string) (*domain.User, error) {
if !s.cfg.LocalAuth {
return nil, domain.ErrLocalAuthDisabled
}
if len(password) > maxPasswordLen {
// No stored password is this long; reject without spending argon2 work.
return nil, domain.ErrInvalidCredentials
}
u, err := s.store.GetUserByEmail(ctx, normalizeEmail(email))
switch {
case errors.Is(err, domain.ErrNotFound):
// Spend comparable time so timing can't distinguish a missing account.
_, _ = verifyPassword(s.dummyHash, password)
return nil, domain.ErrInvalidCredentials
case err != nil:
return nil, err
case u.PasswordHash == nil:
// OIDC-only account: no local password to check, but equalize timing.
_, _ = verifyPassword(s.dummyHash, password)
return nil, domain.ErrInvalidCredentials
}
ok, err := verifyPassword(*u.PasswordHash, password)
if err != nil {
// A stored hash we can't parse is a data problem, not a wrong password;
// surface it so a corrupt row doesn't silently lock a user out unnoticed.
slog.Error("service: malformed stored password hash", "user_id", u.ID, "error", err)
return nil, domain.ErrInvalidCredentials
}
if !ok {
return nil, domain.ErrInvalidCredentials
}
return u, nil
}
// CreateSession issues a new session for a user and returns the raw bearer token
// (to place in the cookie) and its expiry.
func (s *Service) CreateSession(ctx context.Context, userID int64) (token string, expiresAt time.Time, err error) {
raw, err := newSessionToken()
if err != nil {
return "", time.Time{}, err
}
exp := s.now().Add(sessionTTL)
if err := s.store.CreateSession(ctx, &domain.Session{
TokenHash: hashToken(raw),
UserID: userID,
ExpiresAt: formatTime(exp),
}); err != nil {
return "", time.Time{}, err
}
return raw, exp, nil
}
// ResolveSession validates a raw bearer token and returns its user and the
// session's current expiry (so the caller can slide the client cookie in
// lockstep with the server). An expired session is deleted and treated as absent
// (domain.ErrNotFound). A still-valid session has its expiry slid forward, but
// only when that moves it by more than an hour, so a busy client doesn't write
// on every request.
func (s *Service) ResolveSession(ctx context.Context, rawToken string) (*domain.User, time.Time, error) {
if rawToken == "" {
return nil, time.Time{}, domain.ErrNotFound
}
hash := hashToken(rawToken)
sess, err := s.store.GetSession(ctx, hash)
if err != nil {
return nil, time.Time{}, err
}
exp, err := parseTime(sess.ExpiresAt)
if err != nil {
// A corrupt expiry means we can't trust the session; drop it.
if delErr := s.store.DeleteSession(ctx, hash); delErr != nil {
slog.Warn("service: deleting session with corrupt expiry", "error", delErr)
}
return nil, time.Time{}, domain.ErrNotFound
}
now := s.now()
if !now.Before(exp) {
if delErr := s.store.DeleteSession(ctx, hash); delErr != nil {
slog.Warn("service: deleting expired session", "error", delErr)
}
return nil, time.Time{}, domain.ErrNotFound
}
if newExp := now.Add(sessionTTL); newExp.Sub(exp) > time.Hour {
if err := s.store.TouchSession(ctx, hash, formatTime(newExp)); err != nil {
// Non-fatal: the session is still valid at its current expiry.
slog.Warn("service: sliding session expiry", "error", err)
} else {
exp = newExp
}
}
user, err := s.store.GetUserByID(ctx, sess.UserID)
if err != nil {
return nil, time.Time{}, err
}
return user, exp, nil
}
// Logout deletes the session behind a raw bearer token. It is idempotent.
func (s *Service) Logout(ctx context.Context, rawToken string) error {
if rawToken == "" {
return nil
}
return s.store.DeleteSession(ctx, hashToken(rawToken))
}
// CleanupExpiredSessions deletes all sessions that have passed their expiry and
// returns the count. Called periodically; expired sessions are also dropped
// lazily on access by ResolveSession.
func (s *Service) CleanupExpiredSessions(ctx context.Context) (int64, error) {
return s.store.DeleteExpiredSessions(ctx, formatTime(s.now()))
}
// normalizeEmail trims and lowercases an email for consistent storage and
// lookup. (The users.email column is also NOCASE, so lookups are robust either
// way; normalizing keeps stored values tidy.)
func normalizeEmail(email string) string {
return strings.ToLower(strings.TrimSpace(email))
}
// emailLocalPart returns the portion of an email before '@', used as a fallback
// display name for JIT-provisioned OIDC users whose token carried no name.
func emailLocalPart(email string) string {
if i := strings.IndexByte(email, '@'); i > 0 {
return email[:i]
}
return email
}