From 2e3a680ca41602cad800fe6da900f8f6a0ce8dca Mon Sep 17 00:00:00 2001 From: Dmitry Verkhoturov Date: Sat, 4 Jul 2026 01:54:16 +0100 Subject: [PATCH] fix(deleteme): surface real avatar-store errors, tolerate only not-found Bumps go-pkgz/auth to v2.1.5, which adds avatar.ErrNotFound. deleteMeRequestCtrl's avatar removal was best-effort (log and continue on any error) because before the sentinel there was no portable way to tell an already-removed avatar from a genuine failure. It now tolerates only errors.Is(err, avatar.ErrNotFound) - keeping the repeated-request idempotency - and surfaces any other store failure as 500. --- backend/app/rest/api/admin.go | 11 +-- backend/app/rest/api/admin_test.go | 48 +++++++++++++ backend/go.mod | 2 +- backend/go.sum | 4 +- .../vendor/github.com/go-pkgz/auth/v2/auth.go | 41 ++++------- .../go-pkgz/auth/v2/avatar/avatar.go | 69 ++++++++----------- .../github.com/go-pkgz/auth/v2/avatar/bolt.go | 8 ++- .../go-pkgz/auth/v2/avatar/gridfs.go | 9 ++- .../go-pkgz/auth/v2/avatar/localfs.go | 11 ++- .../go-pkgz/auth/v2/avatar/store.go | 10 ++- .../go-pkgz/auth/v2/logger/interface.go | 2 +- .../auth/v2/middleware/user_updater.go | 2 +- .../go-pkgz/auth/v2/provider/apple.go | 4 +- .../go-pkgz/auth/v2/provider/dev_provider.go | 2 +- .../go-pkgz/auth/v2/provider/direct.go | 29 ++++++-- .../go-pkgz/auth/v2/provider/oauth1.go | 9 ++- .../go-pkgz/auth/v2/provider/oauth2.go | 5 +- .../go-pkgz/auth/v2/provider/service.go | 22 +++++- .../go-pkgz/auth/v2/provider/telegram.go | 7 +- .../go-pkgz/auth/v2/provider/verify.go | 4 +- .../github.com/go-pkgz/auth/v2/token/jwt.go | 20 +++--- .../github.com/go-pkgz/auth/v2/token/user.go | 2 +- backend/vendor/modules.txt | 2 +- 23 files changed, 203 insertions(+), 120 deletions(-) diff --git a/backend/app/rest/api/admin.go b/backend/app/rest/api/admin.go index 34e32387..10a21f5b 100644 --- a/backend/app/rest/api/admin.go +++ b/backend/app/rest/api/admin.go @@ -1,6 +1,7 @@ package api import ( + "errors" "fmt" "net/http" "path" @@ -8,6 +9,7 @@ import ( "time" "github.com/go-pkgz/auth/v2" + "github.com/go-pkgz/auth/v2/avatar" cache "github.com/go-pkgz/lcw/v2" log "github.com/go-pkgz/lgr" R "github.com/go-pkgz/rest" @@ -124,10 +126,11 @@ func (a *admin) deleteMeRequestCtrl(w http.ResponseWriter, r *http.Request) { if claims.User.Picture != "" && a.authenticator.AvatarProxy() != nil { if avatarID := avatarIDFromPicture(claims.User.Picture); avatarID != "" { - // best-effort removal: the user's data is already gone and the avatar store gives no way to tell - // an already-removed avatar from a real failure, so a missing avatar must not fail the deletion - if err = a.authenticator.AvatarProxy().Store.Remove(avatarID); err != nil { - log.Printf("[WARN] can't delete avatar for user %s on site %s: %v", claims.User.ID, audience, err) + // an already-removed avatar is fine (a repeated request stays idempotent), but a genuine + // store failure is surfaced now that avatar.ErrNotFound lets us tell the two apart + if err = a.authenticator.AvatarProxy().Store.Remove(avatarID); err != nil && !errors.Is(err, avatar.ErrNotFound) { + rest.SendErrorJSON(w, r, http.StatusInternalServerError, err, "can't delete user's avatar", rest.ErrInternal) + return } } else { log.Printf("[WARN] unexpected avatar picture %q for user %s on site %s, skipping removal", claims.User.Picture, claims.User.ID, audience) diff --git a/backend/app/rest/api/admin_test.go b/backend/app/rest/api/admin_test.go index ec733e4b..56ac5a26 100644 --- a/backend/app/rest/api/admin_test.go +++ b/backend/app/rest/api/admin_test.go @@ -797,6 +797,54 @@ func TestAdmin_DeleteMeRequestMissingAvatar(t *testing.T) { assert.EqualError(t, err, "no comments for user user3 in store", "user3 comments should be deleted") } +// a genuine (non not-found) avatar-store failure must now surface, not be silently swallowed: +// avatar.ErrNotFound lets deleteMeRequestCtrl tell an already-gone avatar from a real error +func TestAdmin_DeleteMeRequestAvatarRemoveError(t *testing.T) { + ts, srv, teardown := startupT(t) + defer teardown() + + c1 := store.Comment{Text: "test test #1", Locator: store.Locator{SiteID: "remark42", + URL: "https://radio-t.com/blah"}, User: store.User{Name: "user5 name", ID: "user5"}} + _, err := srv.DataService.Create(c1) + require.NoError(t, err) + + // put a non-empty directory where the avatar file is expected, so Store.Remove fails with a real + // error (directory not empty), not os.ErrNotExist - "pic" hashes to partition 42 + require.NoError(t, os.MkdirAll(os.TempDir()+"/ava-remark42/42/pic.image", 0o700)) + require.NoError(t, os.WriteFile(os.TempDir()+"/ava-remark42/42/pic.image/child", []byte("x"), 0o600)) + + claims := token.Claims{ + SessionOnly: true, + RegisteredClaims: jwt.RegisteredClaims{ + Audience: jwt.ClaimStrings{"remark42"}, + ID: "4567890", + Issuer: "remark42", + NotBefore: jwt.NewNumericDate(time.Now().Add(-1 * time.Minute)), + ExpiresAt: jwt.NewNumericDate(time.Now().Add(30 * time.Minute)), + }, + User: &token.User{ + ID: "user5", + Picture: "https://demo.remark42.com/api/v1/avatar/pic.image", + Attributes: map[string]any{ + "delete_me": true, + }, + }, + } + + tkn, err := srv.Authenticator.TokenService().Token(claims) + require.NoError(t, err) + + client := http.Client{} + defer client.CloseIdleConnections() + req, err := http.NewRequest(http.MethodGet, fmt.Sprintf("%s/api/v1/admin/deleteme?token=%s", ts.URL, tkn), http.NoBody) + require.NoError(t, err) + req.SetBasicAuth("admin", "password") + resp, err := client.Do(req) + require.NoError(t, err) + require.NoError(t, resp.Body.Close()) + assert.Equal(t, http.StatusInternalServerError, resp.StatusCode, "a real avatar-store failure must surface, not be swallowed") +} + func TestAvatarIDFromPicture(t *testing.T) { tbl := []struct { name string diff --git a/backend/go.mod b/backend/go.mod index 6d88e502..8d680a83 100644 --- a/backend/go.mod +++ b/backend/go.mod @@ -7,7 +7,7 @@ require ( github.com/PuerkitoBio/goquery v1.12.0 github.com/alecthomas/chroma/v2 v2.27.0 github.com/didip/tollbooth/v8 v8.0.1 - github.com/go-pkgz/auth/v2 v2.1.4 + github.com/go-pkgz/auth/v2 v2.1.5 github.com/go-pkgz/jrpc v0.4.0 github.com/go-pkgz/lcw/v2 v2.0.0 github.com/go-pkgz/lgr v0.12.3 diff --git a/backend/go.sum b/backend/go.sum index cf6ffbe3..a2fe9ae5 100644 --- a/backend/go.sum +++ b/backend/go.sum @@ -42,8 +42,8 @@ github.com/gavv/httpexpect v2.0.0+incompatible h1:1X9kcRshkSKEjNJJxX9Y9mQ5BRfbxU github.com/gavv/httpexpect v2.0.0+incompatible/go.mod h1:x+9tiU1YnrOvnB725RkpoLv1M62hOWzwo5OXotisrKc= github.com/go-oauth2/oauth2/v4 v4.5.4 h1:YjI0tmGW8oxVhn9QSBIxlr641QugWrJY5UWa6XmLcW0= github.com/go-oauth2/oauth2/v4 v4.5.4/go.mod h1:BXiOY+QZtZy2ewbsGk2B5P8TWmtz/Rf7ES5ZttQFxfQ= -github.com/go-pkgz/auth/v2 v2.1.4 h1:bCF0vMscOrShF2gelcvKPgskpwQNGCk6AQcoXOf2kbE= -github.com/go-pkgz/auth/v2 v2.1.4/go.mod h1:IvxxhJIrwd1hKqFwQgBF9i+sMTmGfzAw66wmhw1zfJc= +github.com/go-pkgz/auth/v2 v2.1.5 h1:CFL7XxRMNPga0S0YCnAnlvO61OHHEYvVEGrIZXuA98Y= +github.com/go-pkgz/auth/v2 v2.1.5/go.mod h1:IvxxhJIrwd1hKqFwQgBF9i+sMTmGfzAw66wmhw1zfJc= github.com/go-pkgz/email v0.6.0 h1:snZnXldjeF4PgKSjnx9Fa25mtOgFpAOEeWvnQvrxjLE= github.com/go-pkgz/email v0.6.0/go.mod h1:+wgi4x7S33IuCzfcCM5euN0GwQG6XvO/PBLxrNffYLI= github.com/go-pkgz/expirable-cache/v3 v3.1.0 h1:s05P851/O6QJ6Mc+7o2bh9aGtD3romB1SxDTXifdoqc= diff --git a/backend/vendor/github.com/go-pkgz/auth/v2/auth.go b/backend/vendor/github.com/go-pkgz/auth/v2/auth.go index d66c65e6..8b5fe5cf 100644 --- a/backend/vendor/github.com/go-pkgz/auth/v2/auth.go +++ b/backend/vendor/github.com/go-pkgz/auth/v2/auth.go @@ -58,7 +58,7 @@ type Opts struct { XSRFHeaderKey string // default "X-XSRF-TOKEN" XSRFIgnoreMethods []string // disable XSRF protection for the specified request methods (ex. []string{"GET", "POST")}, default empty JWTQuery string // default "token" - SendJWTHeader bool // if enabled send JWT as a header instead of cookie + SendJWTHeader bool // if enabled, also send JWT as a response header (in addition to the cookie) SameSiteCookie http.SameSite // limit cross-origin requests with SameSite cookie attribute Issuer string // optional value for iss claim, usually the application name, default "go-pkgz/auth" @@ -81,7 +81,7 @@ type Opts struct { UseGravatar bool // for email based auth (verified provider) use gravatar service AdminPasswd string // if presented, allows basic auth with user admin and given password - BasicAuthChecker middleware.BasicAuthFunc // user custom checker for basic auth, if one defined then "AdminPasswd" will ignored + BasicAuthChecker middleware.BasicAuthFunc // custom checker for basic auth; when set, AdminPasswd is bypassed entirely AudienceReader token.Audience // list of allowed aud values, default (empty) allows any AudSecrets bool // allow multiple secrets (secret per aud) Logger logger.L // logger interface, default is no logging at all @@ -240,30 +240,17 @@ func (s *Service) Handlers() (authHandler, avatarHandler http.Handler) { return withSecurityHeaders(http.HandlerFunc(ah)), withSecurityHeaders(http.HandlerFunc(s.avatarProxy.Handler)) } -// withSecurityHeaders wraps an auth response handler to apply strict CSP and nosniff -// on every response. The go-pkgz/auth package's own response surface is JSON-only -// for auth routes and images for the avatar route — no built-in HTML rendering -// anywhere — so this CSP is unconditionally safe and gives the auth origin -// defense-in-depth against any future trust-boundary regression that might emit a -// renderable body. +// withSecurityHeaders wraps a handler to set Content-Security-Policy +// "default-src 'none'; sandbox; frame-ancestors 'none'" and X-Content-Type-Options +// "nosniff" on every response. Safe to apply unconditionally because go-pkgz/auth +// only emits JSON (auth routes) and images (avatar route). // -// - Content-Security-Policy: default-src 'none'; sandbox — blocks inline scripts -// and event handlers even if a body is ever served as HTML by mistake; the -// sandbox directive additionally isolates any rendered document from this origin. -// - X-Content-Type-Options: nosniff — prevents browsers from MIME-overriding the -// declared Content-Type to a more dangerous one. -// -// The avatar Handler additionally sets Content-Disposition: inline; filename="avatar" -// inside itself, so direct callers (tests, custom mounts) still get the full header -// set without going through this wrapper. -// -// CONSUMER NOTE: custom providers added via Service.AddCustomHandler / AddProvider -// are also wrapped. If a custom provider renders HTML (login forms, JS-based flows, -// the dev_provider's login page, etc.), the strict CSP will block inline scripts and -// event handlers on those pages. Such providers should either (a) override the CSP -// for their own response by calling w.Header().Set("Content-Security-Policy", ...) -// before writing — Set replaces the wrapper's value — or (b) move any required -// scripts/styles to external files served from 'self'. +// CONSUMER NOTE: custom providers registered through AddCustomHandler / AddProvider +// are wrapped too. A provider that renders HTML (login form, JS flow, custom server +// login page, etc.) will be blocked by this CSP — default-src 'none' plus sandbox +// stop scripts, styles, forms, and images even when served from 'self'. Such +// providers must override the CSP on their own response (call w.Header().Set before +// writing — Set replaces the wrapper's value) and relax only the directives needed. func withSecurityHeaders(next http.Handler) http.Handler { return http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { w.Header().Set("Content-Security-Policy", "default-src 'none'; sandbox; frame-ancestors 'none'") @@ -517,7 +504,7 @@ func (s *Service) AddCustomHandler(p provider.Provider) { // DevAuth makes dev oauth2 server, for testing and development only! func (s *Service) DevAuth() (*provider.DevAuthServer, error) { - p, err := s.Provider("dev") // peak dev provider + p, err := s.Provider("dev") // peek dev provider if err != nil { return nil, fmt.Errorf("dev provider not registered: %w", err) } @@ -545,7 +532,7 @@ func (s *Service) TokenService() *token.Service { return s.jwtService } -// AvatarProxy returns stored in service +// AvatarProxy returns the avatar.Proxy configured on the service, or nil if no AvatarStore was set. func (s *Service) AvatarProxy() *avatar.Proxy { return s.avatarProxy } diff --git a/backend/vendor/github.com/go-pkgz/auth/v2/avatar/avatar.go b/backend/vendor/github.com/go-pkgz/auth/v2/avatar/avatar.go index 7608cb6a..aaadff93 100644 --- a/backend/vendor/github.com/go-pkgz/auth/v2/avatar/avatar.go +++ b/backend/vendor/github.com/go-pkgz/auth/v2/avatar/avatar.go @@ -28,17 +28,12 @@ import ( // http.sniffLen is 512 bytes which is how much we need to read to detect content type const sniffLen = 512 -// maxAvatarFetchSize bounds the bytes read from a remote avatar URL. 10 MiB is -// generous for any reasonable avatar (Telegram caps photo at 5 MiB; Gravatar is -// much smaller); the cap protects Proxy.Put against an upstream sending an -// unbounded body that would exhaust process memory inside resize. +// maxAvatarFetchSize caps the byte length of any avatar accepted by load, +// PutContent, or resize — i.e. the upper bound on bytes that ever reach Store.Put. const maxAvatarFetchSize = 10 << 20 -// maxAvatarPixels caps the declared pixel count of an avatar before any raster -// decode is allowed. Without this, a tiny compressed "decompression bomb" image -// declaring e.g. 65535x65535 px would force image.Decode to allocate gigabytes -// of pixel memory and OOM the auth service on a single login attempt. 16 MP -// covers any realistic avatar (~4096x4096) while keeping peak allocation bounded. +// maxAvatarPixels caps cfg.Width*cfg.Height (from image.DecodeConfig) before any +// full image.Decode runs. Defends against decompression-bomb inputs. const maxAvatarPixels = 16 * 1024 * 1024 // Proxy provides http handler for avatars from avatar.Store @@ -51,7 +46,11 @@ type Proxy struct { ResizeLimit int } -// Put stores retrieved avatar to avatar.Store. Gets image from user info. Returns proxied url +// Put fetches u.Picture, validates and optionally resizes the body, stores it via +// Store, and returns the proxied avatar URL. If u.Picture is empty, the fetch fails, +// or the upstream bytes are not a recognized image format within the configured +// dimension and size limits, Put silently falls back to a generated identicon and +// returns its proxied URL — the caller is not told upstream was rejected. func (p *Proxy) Put(u token.User, client *http.Client) (avatarURL string, err error) { genIdenticon := func(userID string) (avatarURL string, err error) { @@ -168,7 +167,9 @@ func (p *Proxy) load(url string, client *http.Client) ([]byte, error) { return body, nil } -// Handler returns token routes for given provider +// Handler serves stored avatar content by avatar id. GET only; rejects invalid ids +// (403) and stored bytes that fail the safeImgContentType sniff (415). Layered +// defense headers are set on every response via setAvatarDefenseHeaders. func (p *Proxy) Handler(w http.ResponseWriter, r *http.Request) { setAvatarDefenseHeaders(w) @@ -195,11 +196,9 @@ func (p *Proxy) Handler(w http.ResponseWriter, r *http.Request) { } }() - // io.ReadFull keeps reading until the buffer is full or EOF, so a Store - // implementation that returns a buffered reader with a small first-Read size - // won't cause DetectContentType to misclassify a real image. Short bodies - // (avatars under sniffLen) return ErrUnexpectedEOF — that's expected, we sniff - // what we got. + // ReadFull (not Read) so a Store backend that hands back a short first Read + // doesn't truncate the sniff window. Short bodies are signaled by + // ErrUnexpectedEOF and treated like EOF. buf := make([]byte, sniffLen) n, err := io.ReadFull(avReader, buf) if err != nil && err != io.EOF && err != io.ErrUnexpectedEOF { @@ -208,11 +207,9 @@ func (p *Proxy) Handler(w http.ResponseWriter, r *http.Request) { return } - // validate the bytes really are an image before declaring a content type. Even - // though Put() now refuses to store non-image content, this catches stores poisoned - // before the fix and any future regression — never trust the bytes at the store - // boundary alone, validate again at serve time. An empty body (e.g. NoOp store) - // is treated as a benign no-content case: nothing to render, no XSS surface. + // sniff buf[:n] against the safeImgContentType allowlist. This is not a full + // image decode; it catches stores poisoned before Put validated. An empty body + // (e.g. NoOp store) is treated as benign — nothing to render. var contentType string if n > 0 { var ctErr error @@ -248,20 +245,12 @@ func (p *Proxy) Handler(w http.ResponseWriter, r *http.Request) { } } -// resize validates that the input is a real image and, if needed, re-encodes it to -// fit within "limit" px on the larger side preserving aspect ratio. Returns nil for -// non-image content or for declared dimensions exceeding maxAvatarPixels so attacker -// payloads (HTML/SVG/decompression bombs) never reach the store. With limit <= 0 or -// when the image already fits, the original bytes are returned verbatim so animated -// GIFs and other multi-frame formats round-trip without being flattened to one frame. -// -// Validation uses image.DecodeConfig (cheap — declares dimensions, allocates nothing) -// before any full image.Decode, so a 100 KB compressed image declaring 65535x65535 px -// is rejected without ever materializing the raster. -// -// Callers (load, PutContent, identicon generation) must ensure body is bounded by -// maxAvatarFetchSize before calling; resize trusts the size invariant rather than -// re-buffering. An empty body or a body over the cap is refused defensively. +// resize checks body via image.DecodeConfig (format + dimensions only, no raster +// allocation), then either returns the original bytes verbatim (limit <= 0, or +// dimensions already within limit) or fully decodes and re-encodes to PNG fitting +// within limit px on the larger side. Returns nil for non-image input or for +// dimensions exceeding maxAvatarPixels. Caller must ensure body is within +// maxAvatarFetchSize; a body over the cap is also refused defensively. func (p *Proxy) resize(body []byte, limit int) io.Reader { if len(body) == 0 || int64(len(body)) > maxAvatarFetchSize { p.Logf("[WARN] avatar resize(): refusing body of size %d (cap %d)", len(body), maxAvatarFetchSize) @@ -315,13 +304,9 @@ func (p *Proxy) resize(body []byte, limit int) io.Reader { return &out } -// safeImgContentType returns the sniffed content type if the bytes look like a safe -// raster image format. The set is an explicit allowlist (PNG, JPEG, GIF, WebP, BMP, -// ICO) — no HasPrefix("image/") catch-all — so future scriptable image/* MIME types -// added by http.DetectContentType cannot silently pass. Returns an error otherwise. -// image/svg+xml is excluded because SVG can execute scripts when navigated to top -// level; image/* coverage of icon files uses both spellings http.DetectContentType -// is known to return. +// safeImgContentType returns the sniffed Content-Type for img if it is one of the +// allow-listed raster image formats (PNG, JPEG, GIF, WebP, BMP, ICO), else an error. +// SVG is excluded. func safeImgContentType(img []byte) (string, error) { ct := http.DetectContentType(img) base := ct diff --git a/backend/vendor/github.com/go-pkgz/auth/v2/avatar/bolt.go b/backend/vendor/github.com/go-pkgz/auth/v2/avatar/bolt.go index 001f214a..4e87ca69 100644 --- a/backend/vendor/github.com/go-pkgz/auth/v2/avatar/bolt.go +++ b/backend/vendor/github.com/go-pkgz/auth/v2/avatar/bolt.go @@ -11,7 +11,7 @@ import ( // BoltDB implements avatar store with bolt // using separate db (file) with "avatars" bucket to keep image bin and "metas" bucket -// to keep sha1 of picture. avatarID (base file name) used as a key for both. +// to keep a sha1 fingerprint of the stored bytes. avatarID (base file name) used as a key for both. type BoltDB struct { fileName string // full path to boltdb db *bolt.DB @@ -41,7 +41,9 @@ func NewBoltDB(fileName string, options bolt.Options) (*BoltDB, error) { return &BoltDB{db: db, fileName: fileName}, nil } -// Put avatar to bolt, key by avatarID. Trying to resize image and lso calculates sha1 of the file for ID func +// Put stores avatar bytes read from reader in the "avatars" bucket and a sha1 of the +// same bytes in the "metas" bucket, both keyed by the encoded userID with .image suffix. +// Resizing happens upstream in Proxy.resize; this layer only writes what it is given. func (b *BoltDB) Put(userID string, reader io.Reader) (avatar string, err error) { id := encodeID(userID) @@ -101,7 +103,7 @@ func (b *BoltDB) Remove(avatarID string) (err error) { return b.db.Update(func(tx *bolt.Tx) error { bkt := tx.Bucket([]byte(avatarsBktName)) if bkt.Get([]byte(avatarID)) == nil { - return fmt.Errorf("avatar key not found, %s", avatarID) + return fmt.Errorf("avatar %s not found: %w", avatarID, ErrNotFound) } if err = tx.Bucket([]byte(avatarsBktName)).Delete([]byte(avatarID)); err != nil { return fmt.Errorf("can't delete avatar object %s: %w", avatarID, err) diff --git a/backend/vendor/github.com/go-pkgz/auth/v2/avatar/gridfs.go b/backend/vendor/github.com/go-pkgz/auth/v2/avatar/gridfs.go index fc6e2fb1..db315455 100644 --- a/backend/vendor/github.com/go-pkgz/auth/v2/avatar/gridfs.go +++ b/backend/vendor/github.com/go-pkgz/auth/v2/avatar/gridfs.go @@ -27,7 +27,9 @@ type GridFS struct { timeout time.Duration } -// Put avatar to gridfs object, try to resize +// Put stores avatar bytes read from reader in GridFS, keyed by the encoded userID +// with .image suffix, and records the sha1 of those bytes in the file's metadata. +// Resizing happens upstream in Proxy.resize; this layer only writes what it is given. func (gf *GridFS) Put(userID string, reader io.Reader) (avatar string, err error) { id := encodeID(userID) bucket, err := gridfs.NewBucket(gf.db, &options.BucketOptions{Name: &gf.bucketName}) @@ -59,7 +61,8 @@ func (gf *GridFS) Get(avatar string) (reader io.ReadCloser, size int, err error) return io.NopCloser(buf), int(sz), nil } -// ID returns a fingerprint of the avatar content. Uses MD5 because gridfs provides it directly +// ID returns the sha1 fingerprint of the avatar content stored in the GridFS file's +// metadata at Put time, or an encoded fallback id when the file lookup/decode fails. func (gf *GridFS) ID(avatar string) (id string) { finfo := struct { @@ -113,7 +116,7 @@ func (gf *GridFS) Remove(avatar string) error { } return bucket.Delete(r.ID) } - return fmt.Errorf("avatar %s not found", avatar) + return fmt.Errorf("avatar %s not found: %w", avatar, ErrNotFound) } // List all avatars (ids) on gfs diff --git a/backend/vendor/github.com/go-pkgz/auth/v2/avatar/localfs.go b/backend/vendor/github.com/go-pkgz/auth/v2/avatar/localfs.go index ee4828c0..86f4b38a 100644 --- a/backend/vendor/github.com/go-pkgz/auth/v2/avatar/localfs.go +++ b/backend/vendor/github.com/go-pkgz/auth/v2/avatar/localfs.go @@ -1,6 +1,7 @@ package avatar import ( + "errors" "fmt" "hash/crc64" "io" @@ -82,7 +83,15 @@ func (fs *LocalFS) ID(avatar string) (id string) { func (fs *LocalFS) Remove(avatar string) error { location := fs.location(strings.TrimSuffix(avatar, imgSfx)) avFile := path.Join(location, avatar) - return os.Remove(avFile) + if err := os.Remove(avFile); err != nil { + if errors.Is(err, os.ErrNotExist) { + // keep the underlying os.ErrNotExist in the chain alongside ErrNotFound so callers that + // already matched errors.Is(err, os.ErrNotExist) on this store keep working + return fmt.Errorf("avatar %s not found: %w: %w", avatar, ErrNotFound, err) + } + return err + } + return nil } // List all avatars (ids) on local file system diff --git a/backend/vendor/github.com/go-pkgz/auth/v2/avatar/store.go b/backend/vendor/github.com/go-pkgz/auth/v2/avatar/store.go index cd2a3815..64a59d22 100644 --- a/backend/vendor/github.com/go-pkgz/auth/v2/avatar/store.go +++ b/backend/vendor/github.com/go-pkgz/auth/v2/avatar/store.go @@ -4,6 +4,7 @@ import ( "context" "crypto/sha1" //nolint gosec "encoding/hex" + "errors" "fmt" _ "image/gif" // initializing packages for supporting GIF _ "image/jpeg" // initializing packages for supporting JPEG. @@ -27,6 +28,10 @@ const imgSfx = ".image" var reValidAvatarID = regexp.MustCompile(`^[a-fA-F0-9]{40}\.image$`) +// ErrNotFound is returned by Store.Remove when the requested avatar does not exist, +// so callers can tell a missing avatar from a real failure with errors.Is. +var ErrNotFound = errors.New("avatar not found") + // Store defines interface to store and load avatars type Store interface { fmt.Stringer @@ -70,7 +75,10 @@ func NewStore(uri string) (Store, error) { return nil, fmt.Errorf("can't parse store url %s", uri) } -// Migrate avatars between stores +// Migrate copies avatars from src to dst, returning the number of ids enumerated +// from src and the first List error if any. Per-avatar Get/Put/Close failures are +// logged with [WARN] and skipped — they do not abort the migration or surface in +// the returned error, so the returned count is "ids attempted", not "ids stored". func Migrate(dst, src Store) (int, error) { ids, err := src.List() if err != nil { diff --git a/backend/vendor/github.com/go-pkgz/auth/v2/logger/interface.go b/backend/vendor/github.com/go-pkgz/auth/v2/logger/interface.go index 6afc5b8b..3fdba0c1 100644 --- a/backend/vendor/github.com/go-pkgz/auth/v2/logger/interface.go +++ b/backend/vendor/github.com/go-pkgz/auth/v2/logger/interface.go @@ -12,7 +12,7 @@ type L interface { // Func type is an adapter to allow the use of ordinary functions as Logger. type Func func(format string, args ...any) -// Logf calls f(id) +// Logf calls f(format, args...). func (f Func) Logf(format string, args ...any) { f(format, args...) } // NoOp logger diff --git a/backend/vendor/github.com/go-pkgz/auth/v2/middleware/user_updater.go b/backend/vendor/github.com/go-pkgz/auth/v2/middleware/user_updater.go index 1103703b..47527bc7 100644 --- a/backend/vendor/github.com/go-pkgz/auth/v2/middleware/user_updater.go +++ b/backend/vendor/github.com/go-pkgz/auth/v2/middleware/user_updater.go @@ -12,7 +12,7 @@ type UserUpdater interface { } // UserUpdFunc type is an adapter to allow the use of ordinary functions as UserUpdater. If f is a function -// with the appropriate signature, UserUpdFunc(f) is a Handler that calls f. +// with the appropriate signature, UserUpdFunc(f) is a UserUpdater that calls f. type UserUpdFunc func(user token.User) token.User // Update calls f(user) diff --git a/backend/vendor/github.com/go-pkgz/auth/v2/provider/apple.go b/backend/vendor/github.com/go-pkgz/auth/v2/provider/apple.go index aaee8748..a4fed337 100644 --- a/backend/vendor/github.com/go-pkgz/auth/v2/provider/apple.go +++ b/backend/vendor/github.com/go-pkgz/auth/v2/provider/apple.go @@ -402,7 +402,7 @@ func (ah AppleHandler) AuthHandler(w http.ResponseWriter, r *http.Request) { return } - ah.Logf("[DEBUG] user info %+v", u) + ah.Logf("[DEBUG] user info %s", userLogSummary(u)) // redirect to back url if presented in login query params if oauthClaims.Handshake != nil && oauthClaims.Handshake.From != "" { @@ -522,7 +522,7 @@ func (ah *AppleHandler) parseUserData(user *token.User, jUser string) { // catch error for log only. No need break flow if user name doesn't exist if err := json.Unmarshal([]byte(jUser), &userData); err != nil { - ah.Logf("[DEBUG] failed to parse user data %s: %v", user, err) + ah.Logf("[DEBUG] failed to parse apple user data: %v", err) user.Name = "noname_" + user.ID[6:12] // paste noname if user name failed to parse return } diff --git a/backend/vendor/github.com/go-pkgz/auth/v2/provider/dev_provider.go b/backend/vendor/github.com/go-pkgz/auth/v2/provider/dev_provider.go index 20f2322a..8179c422 100644 --- a/backend/vendor/github.com/go-pkgz/auth/v2/provider/dev_provider.go +++ b/backend/vendor/github.com/go-pkgz/auth/v2/provider/dev_provider.go @@ -167,7 +167,7 @@ func (d *DevAuthServer) Shutdown() { d.lock.Unlock() } -// NewDev makes dev oauth2 provider for admin user +// NewDev makes a dev oauth2 provider intended for local development only. func NewDev(p Params) Oauth2Handler { if p.Port == 0 { p.Port = defDevAuthPort diff --git a/backend/vendor/github.com/go-pkgz/auth/v2/provider/direct.go b/backend/vendor/github.com/go-pkgz/auth/v2/provider/direct.go index 9844989c..c8cbaf6e 100644 --- a/backend/vendor/github.com/go-pkgz/auth/v2/provider/direct.go +++ b/backend/vendor/github.com/go-pkgz/auth/v2/provider/direct.go @@ -76,24 +76,25 @@ func (p DirectHandler) Name() string { return p.ProviderName } // "aud": "bar", // } func (p DirectHandler) LoginHandler(w http.ResponseWriter, r *http.Request) { + logReq := p.scrubPasswordFromRequest(r) creds, err := p.getCredentials(w, r) if err != nil { - rest.SendErrorJSON(w, r, p.L, http.StatusBadRequest, err, "failed to parse credentials") + rest.SendErrorJSON(w, logReq, p.L, http.StatusBadRequest, err, "failed to parse credentials") return } sessOnly := r.URL.Query().Get("sess") == "1" if p.CredChecker == nil { - rest.SendErrorJSON(w, r, p.L, http.StatusInternalServerError, + rest.SendErrorJSON(w, logReq, p.L, http.StatusInternalServerError, fmt.Errorf("no credential checker"), "no credential checker") return } ok, err := p.CredChecker.Check(creds.User, creds.Password) if err != nil { - rest.SendErrorJSON(w, r, p.L, http.StatusInternalServerError, err, "failed to check user credentials") + rest.SendErrorJSON(w, logReq, p.L, http.StatusInternalServerError, err, "failed to check user credentials") return } if !ok { - rest.SendErrorJSON(w, r, p.L, http.StatusForbidden, nil, "incorrect user or password") + rest.SendErrorJSON(w, logReq, p.L, http.StatusForbidden, nil, "incorrect user or password") return } @@ -108,13 +109,13 @@ func (p DirectHandler) LoginHandler(w http.ResponseWriter, r *http.Request) { } u, err = setAvatar(p.AvatarSaver, u, &http.Client{Timeout: 5 * time.Second}) if err != nil { - rest.SendErrorJSON(w, r, p.L, http.StatusInternalServerError, err, "failed to save avatar to proxy") + rest.SendErrorJSON(w, logReq, p.L, http.StatusInternalServerError, err, "failed to save avatar to proxy") return } cid, err := randToken() if err != nil { - rest.SendErrorJSON(w, r, p.L, http.StatusInternalServerError, err, "can't make token id") + rest.SendErrorJSON(w, logReq, p.L, http.StatusInternalServerError, err, "can't make token id") return } @@ -132,12 +133,26 @@ func (p DirectHandler) LoginHandler(w http.ResponseWriter, r *http.Request) { } if _, err = p.TokenService.Set(w, claims); err != nil { - rest.SendErrorJSON(w, r, p.L, http.StatusInternalServerError, err, "failed to set token") + rest.SendErrorJSON(w, logReq, p.L, http.StatusInternalServerError, err, "failed to set token") return } rest.RenderJSON(w, claims.User) } +func (p DirectHandler) scrubPasswordFromRequest(r *http.Request) *http.Request { + if r == nil || r.URL == nil { + return r + } + if _, ok := r.URL.Query()["passwd"]; !ok { + return r + } + rc := r.Clone(r.Context()) + q := rc.URL.Query() + q.Set("passwd", "") + rc.URL.RawQuery = q.Encode() + return rc +} + // getCredentials extracts user and password from request func (p DirectHandler) getCredentials(w http.ResponseWriter, r *http.Request) (credentials, error) { diff --git a/backend/vendor/github.com/go-pkgz/auth/v2/provider/oauth1.go b/backend/vendor/github.com/go-pkgz/auth/v2/provider/oauth1.go index 6d4f5a84..47af0786 100644 --- a/backend/vendor/github.com/go-pkgz/auth/v2/provider/oauth1.go +++ b/backend/vendor/github.com/go-pkgz/auth/v2/provider/oauth1.go @@ -127,8 +127,7 @@ func (h Oauth1Handler) AuthHandler(w http.ResponseWriter, r *http.Request) { rest.SendErrorJSON(w, r, h.L, http.StatusInternalServerError, err, "failed to unmarshal user info") return } - h.Logf("[DEBUG] got raw user info %+v", jData) - + h.Logf("[DEBUG] got raw user info %s", userDataLogSummary(jData)) u := h.mapUser(jData, data) u, err = setAvatar(h.AvatarSaver, u, &http.Client{Timeout: 5 * time.Second}) if err != nil { @@ -159,7 +158,7 @@ func (h Oauth1Handler) AuthHandler(w http.ResponseWriter, r *http.Request) { return } - h.Logf("[DEBUG] user info %+v", u) + h.Logf("[DEBUG] user info %s", userLogSummary(u)) // redirect to back url if presented in login query params if oauthClaims.Handshake != nil && oauthClaims.Handshake.From != "" { @@ -190,7 +189,7 @@ func (h Oauth1Handler) makeRedirURL(path string) string { return strings.TrimSuffix(h.URL, "/") + strings.TrimSuffix(newPath, "/") + urlCallbackSuffix } -// initOauth2Handler makes oauth1 handler for given provider +// initOauth1Handler makes oauth1 handler for given provider func initOauth1Handler(p Params, service Oauth1Handler) Oauth1Handler { if p.L == nil { p.L = logger.NoOp @@ -200,7 +199,7 @@ func initOauth1Handler(p Params, service Oauth1Handler) Oauth1Handler { service.conf.ConsumerKey = p.Cid service.conf.ConsumerSecret = p.Csecret - p.Logf("[DEBUG] created %s oauth2, id=%s, redir=%s, endpoint=%s", + p.Logf("[DEBUG] created %s oauth1, id=%s, redir=%s, endpoint=%s", service.name, service.Cid, service.makeRedirURL("/{route}/"+service.name+"/"), service.conf.Endpoint) return service } diff --git a/backend/vendor/github.com/go-pkgz/auth/v2/provider/oauth2.go b/backend/vendor/github.com/go-pkgz/auth/v2/provider/oauth2.go index 14bb0bf3..e75324e9 100644 --- a/backend/vendor/github.com/go-pkgz/auth/v2/provider/oauth2.go +++ b/backend/vendor/github.com/go-pkgz/auth/v2/provider/oauth2.go @@ -202,8 +202,7 @@ func (p Oauth2Handler) AuthHandler(w http.ResponseWriter, r *http.Request) { rest.SendErrorJSON(w, r, p.L, http.StatusInternalServerError, err, "failed to unmarshal user info") return } - p.Logf("[DEBUG] got raw user info %+v", jData) - + p.Logf("[DEBUG] got raw user info %s", userDataLogSummary(jData)) u := p.mapUser(jData, data) if oauthClaims.NoAva { u.Picture = "" // reset picture on no avatar request @@ -243,7 +242,7 @@ func (p Oauth2Handler) AuthHandler(w http.ResponseWriter, r *http.Request) { p.bearerTokenHook(p.Name(), u, *tok) } - p.Logf("[DEBUG] user info %+v", u) + p.Logf("[DEBUG] user info %s", userLogSummary(u)) // redirect to back url if presented in login query params if oauthClaims.Handshake != nil && oauthClaims.Handshake.From != "" { diff --git a/backend/vendor/github.com/go-pkgz/auth/v2/provider/service.go b/backend/vendor/github.com/go-pkgz/auth/v2/provider/service.go index 4d80d466..953f62d0 100644 --- a/backend/vendor/github.com/go-pkgz/auth/v2/provider/service.go +++ b/backend/vendor/github.com/go-pkgz/auth/v2/provider/service.go @@ -6,6 +6,7 @@ import ( "fmt" "net" "net/http" + "slices" "strings" "github.com/go-pkgz/auth/v2/avatar" @@ -63,7 +64,7 @@ type Provider interface { LogoutHandler(w http.ResponseWriter, r *http.Request) } -// Handler returns auth routes for given provider +// Handler dispatches login, callback, and logout requests to the underlying provider. func (p Service) Handler(w http.ResponseWriter, r *http.Request) { if r.Method != http.MethodGet && r.Method != http.MethodPost { @@ -85,6 +86,25 @@ func (p Service) Handler(w http.ResponseWriter, r *http.Request) { w.WriteHeader(http.StatusNotFound) } +func userDataLogSummary(data UserData) string { + keys := make([]string, 0, len(data)) + for k := range data { + keys = append(keys, k) + } + slices.Sort(keys) + return fmt.Sprintf("keys=%v", keys) +} + +func userLogSummary(u token.User) string { + attrs := make([]string, 0, len(u.Attributes)) + for k := range u.Attributes { + attrs = append(attrs, k) + } + slices.Sort(attrs) + return fmt.Sprintf("id=%q name=%q picture=%t email=%t attrs=%v role=%t audience=%t", + u.ID, u.Name, u.Picture != "", u.Email != "", attrs, u.Role != "", u.Audience != "") +} + // setAvatar saves avatar and puts proxied URL to u.Picture func setAvatar(ava AvatarSaver, u token.User, client *http.Client) (token.User, error) { if ava == nil || ava == (*avatar.Proxy)(nil) { diff --git a/backend/vendor/github.com/go-pkgz/auth/v2/provider/telegram.go b/backend/vendor/github.com/go-pkgz/auth/v2/provider/telegram.go index a54f3d7d..a23de201 100644 --- a/backend/vendor/github.com/go-pkgz/auth/v2/provider/telegram.go +++ b/backend/vendor/github.com/go-pkgz/auth/v2/provider/telegram.go @@ -156,8 +156,8 @@ func (th *TelegramHandler) ProcessUpdate(ctx context.Context, textUpdate string) return nil } -// processUpdates processes a batch of updates from telegram servers -// Returns offset for subsequent calls +// processUpdates processes a batch of updates from telegram servers. Offset +// tracking for subsequent polls is handled inside TelegramAPI.GetUpdates. func (th *TelegramHandler) processUpdates(ctx context.Context, updates *telegramUpdate) { for _, update := range updates.Result { if update.Message.Chat.Type != "private" { @@ -229,7 +229,8 @@ func (th *TelegramHandler) addToken(token string, expires time.Time) error { return nil } -// checkToken verifies incoming token, returns the user address if it's confirmed and empty string otherwise +// checkToken returns the confirmed user for a known token, or an error if the +// request is missing, expired, or has not been verified yet. func (th *TelegramHandler) checkToken(token string) (*authtoken.User, error) { th.requests.RLock() authRequest, ok := th.requests.data[token] diff --git a/backend/vendor/github.com/go-pkgz/auth/v2/provider/verify.go b/backend/vendor/github.com/go-pkgz/auth/v2/provider/verify.go index 738a322f..38224260 100644 --- a/backend/vendor/github.com/go-pkgz/auth/v2/provider/verify.go +++ b/backend/vendor/github.com/go-pkgz/auth/v2/provider/verify.go @@ -161,7 +161,7 @@ func scrubTokenFromRequest(r *http.Request) *http.Request { return rc } -// Sender defines interface to send emails +// Sender defines interface to deliver a verification message (email, IM, or anything else). type Sender interface { Send(address, text string) error } @@ -380,7 +380,7 @@ func (e VerifyHandler) sendConfirmation(w http.ResponseWriter, r *http.Request) rest.RenderJSON(w, rest.JSON{"user": user, "address": address}) } -// AuthHandler doesn't do anything for direct login as it has no callbacks +// AuthHandler is a no-op for verify login — the flow has no provider callback. func (e VerifyHandler) AuthHandler(http.ResponseWriter, *http.Request) {} // LogoutHandler - GET /logout diff --git a/backend/vendor/github.com/go-pkgz/auth/v2/token/jwt.go b/backend/vendor/github.com/go-pkgz/auth/v2/token/jwt.go index 2bdc63ac..8daf4a48 100644 --- a/backend/vendor/github.com/go-pkgz/auth/v2/token/jwt.go +++ b/backend/vendor/github.com/go-pkgz/auth/v2/token/jwt.go @@ -82,7 +82,7 @@ type Opts struct { AudienceReader Audience // allowed aud values Issuer string // optional value for iss claim, usually application name AudSecrets bool // uses different secret for differed auds. important: adds pre-parsing of unverified token - SendJWTHeader bool // if enabled send JWT as a header instead of cookie + SendJWTHeader bool // if enabled, also send JWT as a response header (in addition to the cookie) SameSite http.SameSite // define a cookie attribute making it impossible for the browser to send this cookie cross-site } @@ -241,9 +241,11 @@ func (j *Service) validate(claims *Claims) error { return err } -// Set creates token cookie with xsrf cookie and put it to ResponseWriter -// accepts claims and sets expiration if none defined. permanent flag means long-living cookie, -// false makes it session only. +// Set writes the JWT cookie and the XSRF cookie to w, also emitting the JWT as a +// response header when SendJWTHeader is enabled. Expiration is filled in from +// j.TokenDuration if claims.ExpiresAt is zero. The cookie is session-only (MaxAge 0) +// when claims.SessionOnly is true or claims contain a Handshake; otherwise it lasts +// j.CookieDuration. func (j *Service) Set(w http.ResponseWriter, claims Claims) (Claims, error) { nowUnix := time.Now().Unix() @@ -286,8 +288,10 @@ func (j *Service) Set(w http.ResponseWriter, claims Claims) (Claims, error) { return claims, nil } -// Get token from url, header or cookie -// if cookie used, verify xsrf token to match +// Get token from url, header or cookie. When a user token is read from a cookie, +// XSRF protection requires the request header to match the token claim ID (the +// value Set also writes to the XSRF cookie), unless DisableXSRF is set, the request +// method is in XSRFIgnoreMethods, or the claims carry no user. func (j *Service) Get(r *http.Request) (Claims, string, error) { fromCookie := false @@ -419,7 +423,7 @@ type ClaimsUpdater interface { // with the appropriate signature, ClaimsUpdFunc(f) is a Handler that calls f. type ClaimsUpdFunc func(claims Claims) Claims -// Update calls f(id) +// Update calls f(claims). func (f ClaimsUpdFunc) Update(claims Claims) Claims { return f(claims) } @@ -434,7 +438,7 @@ type Validator interface { // with the appropriate signature, ValidatorFunc(f) is a Validator that calls f. type ValidatorFunc func(token string, claims Claims) bool -// Validate calls f(id) +// Validate calls f(token, claims). func (f ValidatorFunc) Validate(token string, claims Claims) bool { return f(token, claims) } diff --git a/backend/vendor/github.com/go-pkgz/auth/v2/token/user.go b/backend/vendor/github.com/go-pkgz/auth/v2/token/user.go index c3f73683..12c5b5d0 100644 --- a/backend/vendor/github.com/go-pkgz/auth/v2/token/user.go +++ b/backend/vendor/github.com/go-pkgz/auth/v2/token/user.go @@ -109,7 +109,7 @@ func (u *User) SetSliceAttr(key string, val []string) { func HashID(h hash.Hash, val string) string { if reValidSha.MatchString(val) { - return val // already hashed or empty + return val // already a sha1 hex digest, pass through unchanged } if _, err := io.WriteString(h, val); err != nil { diff --git a/backend/vendor/modules.txt b/backend/vendor/modules.txt index 20cbcf15..b64b867b 100644 --- a/backend/vendor/modules.txt +++ b/backend/vendor/modules.txt @@ -47,7 +47,7 @@ github.com/dlclark/regexp2/v2/syntax github.com/go-oauth2/oauth2/v4 github.com/go-oauth2/oauth2/v4/errors github.com/go-oauth2/oauth2/v4/server -# github.com/go-pkgz/auth/v2 v2.1.4 +# github.com/go-pkgz/auth/v2 v2.1.5 ## explicit; go 1.24.0 github.com/go-pkgz/auth/v2 github.com/go-pkgz/auth/v2/avatar