Review

server/config.go

@@ -116,8 +116,8 @@ type Config struct {
 	AuthTokens                           map[string][]*user.Token
 	AuthBcryptCost                       int
 	AuthStatsQueueWriterInterval         time.Duration
-	AuthAccessCacheEnabled               bool
+	AuthAccessCacheEnabled               bool          // Enables the in-memory ACL cache (high volume servers only)
-	AuthAccessCacheReloadInterval        time.Duration
+	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,

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
-}

user/manager.go

@@ -38,15 +38,8 @@ const (
 const (
 	DefaultUserStatsQueueWriterInterval = 33 * time.Second
 	DefaultUserPasswordBcryptCost       = 10
-	// DefaultAccessCacheEnabled is the default for Config.AccessCacheEnabled.
+	DefaultAccessCacheEnabled           = false
-	// Off by default so self-hosters keep the direct-DB authorizeTopicAccess
+	DefaultAccessCacheReloadInterval    = 60 * time.Second
-	// 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
 )
 
 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
+	// Reload user-specific parts of the access cache
-	// 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.
 	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
+	// Reload user-specific parts of the access cache
-	// (typically the matching Everyone rows from their reservations). Refresh
-	// both slices to mirror the DB exactly.
 	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 {

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)
+	Filename                  string              // Database filename, e.g. "/var/lib/ntfy/user.db" (SQLite)
-	DatabaseURL         string              // Database connection string (PostgreSQL)
+	DatabaseURL               string              // Database connection string (PostgreSQL)
-	StartupQueries      string              // Queries to run on startup, e.g. to create initial users or tiers (SQLite only)
+	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
+	DefaultAccess             Permission          // Default permission if no ACL matches
-	ProvisionEnabled    bool                // Hack: Enable auto-provisioning of users and access grants, disabled for "ntfy user" commands
+	ProvisionEnabled          bool                // Hack: Enable auto-provisioning of users and access grants, disabled for "ntfy user" commands
-	Users               []*User             // Predefined users to create on startup
+	Users                     []*User             // Predefined users to create on startup
-	Access              map[string][]*Grant // Predefined access grants to create on startup (username -> []*Grant)
+	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)
+	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
+	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
+	BcryptCost                int                 // Cost of generated passwords; lowering makes testing faster
-
+	AccessCacheEnabled        bool                // Enables the in-memory ACL cache (high volume servers only)
-	// AccessCacheEnabled gates the in-memory ACL cache. When false (the
+	AccessCacheReloadInterval time.Duration       // Reload interval for access cache, relevant for ACL writes from CLI
-	// 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
 }
 
 // Error constants used by the package