From a2dc290f31a4c969e02ca6b348e33fd96ef590c9 Mon Sep 17 00:00:00 2001 From: binwiederhier Date: Sun, 31 May 2026 21:25:53 -0400 Subject: [PATCH] Review --- server/config.go | 6 +++--- user/access_cache_test.go | 12 ++---------- user/manager.go | 31 ++++++++++--------------------- user/types.go | 37 ++++++++++++------------------------- 4 files changed, 27 insertions(+), 59 deletions(-) diff --git a/server/config.go b/server/config.go index a1ba4d40..b7dadddf 100644 --- a/server/config.go +++ b/server/config.go @@ -116,8 +116,8 @@ type Config struct { AuthTokens map[string][]*user.Token AuthBcryptCost int AuthStatsQueueWriterInterval time.Duration - AuthAccessCacheEnabled bool - AuthAccessCacheReloadInterval time.Duration + AuthAccessCacheEnabled bool // Enables the in-memory ACL cache (high volume servers only) + AuthAccessCacheReloadInterval time.Duration // Reload interval for access cache, relevant for ACL writes from CLI AttachmentCacheDir string AttachmentTotalSizeLimit int64 AttachmentFileSizeLimit int64 @@ -225,7 +225,7 @@ func NewConfig() *Config { AuthDefault: user.PermissionReadWrite, AuthBcryptCost: user.DefaultUserPasswordBcryptCost, AuthStatsQueueWriterInterval: user.DefaultUserStatsQueueWriterInterval, - AuthAccessCacheEnabled: user.DefaultAccessCacheEnabled, // Opt-in (e.g. ntfy.sh) via server.yml + AuthAccessCacheEnabled: user.DefaultAccessCacheEnabled, AuthAccessCacheReloadInterval: user.DefaultAccessCacheReloadInterval, AttachmentCacheDir: "", AttachmentTotalSizeLimit: DefaultAttachmentTotalSizeLimit, diff --git a/user/access_cache_test.go b/user/access_cache_test.go index 65c109cb..17fd2c2c 100644 --- a/user/access_cache_test.go +++ b/user/access_cache_test.go @@ -2,6 +2,7 @@ package user import ( "regexp" + "strings" "sync" "sync/atomic" "testing" @@ -287,7 +288,7 @@ func loadCache(t *testing.T, c *accessCache, rows []rawACLRow) { wildcards := make(map[string][]aclEntry) for _, r := range rows { e := aclEntry{length: len(r.topic), read: r.read, write: r.write} - if containsPercent(r.topic) { + if strings.Contains(r.topic, "%") { e.pattern = mustCompileLikeToRegex(t, r.topic) wildcards[r.user] = append(wildcards[r.user], e) } else { @@ -309,12 +310,3 @@ func mustCompileLikeToRegex(t *testing.T, pattern string) *regexp.Regexp { require.NoError(t, err) return r } - -func containsPercent(s string) bool { - for i := 0; i < len(s); i++ { - if s[i] == '%' { - return true - } - } - return false -} diff --git a/user/manager.go b/user/manager.go index 58fd3b2d..6cc7e8b7 100644 --- a/user/manager.go +++ b/user/manager.go @@ -38,15 +38,8 @@ const ( const ( DefaultUserStatsQueueWriterInterval = 33 * time.Second DefaultUserPasswordBcryptCost = 10 - // DefaultAccessCacheEnabled is the default for Config.AccessCacheEnabled. - // Off by default so self-hosters keep the direct-DB authorizeTopicAccess - // path; ntfy.sh opts in via server config. - DefaultAccessCacheEnabled = false - // DefaultAccessCacheReloadInterval bounds how stale the in-memory ACL snapshot - // can be relative to writes made by *other* processes (e.g. a separate `ntfy - // access` CLI invocation modifying the same database). Only honored when the - // cache is enabled. - DefaultAccessCacheReloadInterval = 60 * time.Second + DefaultAccessCacheEnabled = false + DefaultAccessCacheReloadInterval = 60 * time.Second ) var ( @@ -95,9 +88,9 @@ func newManager(d *db.DB, queries queries, config *Config) (*Manager, error) { if err := manager.maybeReloadAccessCache(); err != nil { return nil, err } - go manager.asyncAccessCacheReloader(manager.config.AccessCacheReloadInterval) + go manager.asyncAccessCacheReloadLoop(manager.config.AccessCacheReloadInterval) } - go manager.asyncQueueWriter(manager.config.QueueWriterInterval) + go manager.asyncQueueWriteLoop(manager.config.QueueWriterInterval) return manager, nil } @@ -115,12 +108,12 @@ func (a *Manager) maybeReloadAccessCache(usernames ...string) error { return a.accessCache.reload(a.db, a.queries.selectAccessCacheUsers(len(usernames)), usernames...) } -// asyncAccessCacheReloader periodically bulk-reloads the access cache so that +// asyncAccessCacheReloadLoop periodically bulk-reloads the access cache so that // writes made by other processes against the same database (most notably the // `ntfy access` CLI subcommand running while a server holds the cache) become // visible within the configured interval. This Manager's own mutations do // not depend on the poller -- they refresh affected users synchronously. -func (a *Manager) asyncAccessCacheReloader(interval time.Duration) { +func (a *Manager) asyncAccessCacheReloadLoop(interval time.Duration) { ticker := time.NewTicker(interval) defer ticker.Stop() for { @@ -213,9 +206,7 @@ func (a *Manager) RemoveUser(username string) error { if err != nil { return err } - // user_access rows are cascade-deleted along with the user (both by user_id - // and by owner_user_id). Refresh this user's own slice (now empty) and - // Everyone's slice, since reservations owned by this user landed there too. + // Reload user-specific parts of the access cache return a.maybeReloadAccessCache(username, Everyone) } @@ -253,9 +244,7 @@ func (a *Manager) MarkUserRemoved(user *User) error { if err != nil { return err } - // resetUserAccessTx deleted this user's rows AND any row owned by this user - // (typically the matching Everyone rows from their reservations). Refresh - // both slices to mirror the DB exactly. + // Reload user-specific parts of the access cache return a.maybeReloadAccessCache(user.Name, Everyone) } @@ -264,7 +253,7 @@ func (a *Manager) RemoveDeletedUsers() error { if _, err := a.db.Exec(a.queries.deleteUsersMarked, time.Now().Unix()); err != nil { return err } - // user_access rows are cascade-deleted with the users; refresh the snapshot. + // Full cache reload, because we don't know what the query affects return a.maybeReloadAccessCache() } @@ -430,7 +419,7 @@ func (a *Manager) EnqueueUserStats(userID string, stats *Stats) { a.statsQueue[userID] = stats } -func (a *Manager) asyncQueueWriter(interval time.Duration) { +func (a *Manager) asyncQueueWriteLoop(interval time.Duration) { ticker := time.NewTicker(interval) defer ticker.Stop() for { diff --git a/user/types.go b/user/types.go index 93aaa4d2..c400e48f 100644 --- a/user/types.go +++ b/user/types.go @@ -245,31 +245,18 @@ const ( // Config holds the configuration for the user Manager type Config struct { - Filename string // Database filename, e.g. "/var/lib/ntfy/user.db" (SQLite) - DatabaseURL string // Database connection string (PostgreSQL) - StartupQueries string // Queries to run on startup, e.g. to create initial users or tiers (SQLite only) - DefaultAccess Permission // Default permission if no ACL matches - ProvisionEnabled bool // Hack: Enable auto-provisioning of users and access grants, disabled for "ntfy user" commands - Users []*User // Predefined users to create on startup - Access map[string][]*Grant // Predefined access grants to create on startup (username -> []*Grant) - Tokens map[string][]*Token // Predefined users to create on startup (username -> []*Token) - QueueWriterInterval time.Duration // Interval for the async queue writer to flush stats and token updates to the database - BcryptCost int // Cost of generated passwords; lowering makes testing faster - - // AccessCacheEnabled gates the in-memory ACL cache. When false (the - // default), authorizeTopicAccess runs the direct SQL query against the - // database on every call -- mutations and authorization are unaffected by - // any cache logic. When true, the Manager keeps an in-memory snapshot of - // user_access and serves authorizeTopicAccess from it; mutations refresh - // the affected slices, and a background poller picks up cross-process - // writes at AccessCacheReloadInterval. - AccessCacheEnabled bool - - // AccessCacheReloadInterval bounds the staleness of the in-memory ACL - // cache relative to writes from other processes (e.g. `ntfy access` CLI - // against a running server). Only honored when AccessCacheEnabled is true. - // Zero falls back to DefaultAccessCacheReloadInterval. - AccessCacheReloadInterval time.Duration + Filename string // Database filename, e.g. "/var/lib/ntfy/user.db" (SQLite) + DatabaseURL string // Database connection string (PostgreSQL) + StartupQueries string // Queries to run on startup, e.g. to create initial users or tiers (SQLite only) + DefaultAccess Permission // Default permission if no ACL matches + ProvisionEnabled bool // Hack: Enable auto-provisioning of users and access grants, disabled for "ntfy user" commands + Users []*User // Predefined users to create on startup + Access map[string][]*Grant // Predefined access grants to create on startup (username -> []*Grant) + Tokens map[string][]*Token // Predefined users to create on startup (username -> []*Token) + QueueWriterInterval time.Duration // Interval for the async queue writer to flush stats and token updates to the database + BcryptCost int // Cost of generated passwords; lowering makes testing faster + AccessCacheEnabled bool // Enables the in-memory ACL cache (high volume servers only) + AccessCacheReloadInterval time.Duration // Reload interval for access cache, relevant for ACL writes from CLI } // Error constants used by the package