From d0239ba20960937dc995c4920f5b578b6ff6c8e4 Mon Sep 17 00:00:00 2001 From: Michael Mayer Date: Mon, 17 Mar 2025 16:12:47 +0100 Subject: [PATCH] UX: Improve quota calculation and checks in config and api #4266 Signed-off-by: Michael Mayer --- internal/api/import.go | 2 +- internal/api/oidc_redirect.go | 2 +- internal/api/users_update.go | 15 +++++++- internal/api/users_upload.go | 2 +- internal/auth/acl/const.go | 2 +- internal/auth/acl/role.go | 13 +++++++ internal/config/config_usage.go | 55 ++++++++++++++++++++-------- internal/config/config_usage_test.go | 33 +++++++++++++---- internal/entity/auth_user.go | 9 +---- internal/server/webdav.go | 2 +- internal/workers/sync_download.go | 2 +- 11 files changed, 98 insertions(+), 39 deletions(-) diff --git a/internal/api/import.go b/internal/api/import.go index 7561b4463..a6242578e 100644 --- a/internal/api/import.go +++ b/internal/api/import.go @@ -56,7 +56,7 @@ func StartImport(router *gin.RouterGroup) { } // Abort if there is not enough free storage to import new files. - if conf.FilesQuotaExceeded() { + if conf.FilesQuotaReached() { event.AuditErr([]string{ClientIP(c), "session %s", "import files", "insufficient storage"}, s.RefID) Abort(c, http.StatusInsufficientStorage, i18n.ErrInsufficientStorage) return diff --git a/internal/api/oidc_redirect.go b/internal/api/oidc_redirect.go index dcaf6049e..3f1777eed 100644 --- a/internal/api/oidc_redirect.go +++ b/internal/api/oidc_redirect.go @@ -225,7 +225,7 @@ func OIDCRedirect(router *gin.RouterGroup) { } else if err = avatar.SetUserImageURL(user, avatarUrl, entity.SrcOIDC, conf.ThumbCachePath()); err != nil { event.AuditWarn([]string{clientIp, "create session", "oidc", userName, "failed to set avatar image", err.Error()}) } - } else if conf.UsersQuotaExceeded() { + } else if conf.UsersQuotaReached(conf.OIDCRole()) { userName = oidcUser.Username() event.AuditWarn([]string{clientIp, "create session", "oidc", "create user", userName, authn.ErrUsersQuotaExceeded.Error()}) event.LoginError(clientIp, "oidc", userName, userAgent, authn.ErrUsersQuotaExceeded.Error()) diff --git a/internal/api/users_update.go b/internal/api/users_update.go index 3a31fc101..68c41ddee 100644 --- a/internal/api/users_update.go +++ b/internal/api/users_update.go @@ -7,9 +7,11 @@ import ( "github.com/gin-gonic/gin" "github.com/photoprism/photoprism/internal/auth/acl" + "github.com/photoprism/photoprism/internal/config" "github.com/photoprism/photoprism/internal/entity" "github.com/photoprism/photoprism/internal/event" "github.com/photoprism/photoprism/internal/photoprism/get" + "github.com/photoprism/photoprism/pkg/authn" "github.com/photoprism/photoprism/pkg/clean" "github.com/photoprism/photoprism/pkg/i18n" ) @@ -61,10 +63,17 @@ func UpdateUser(router *gin.RouterGroup) { return } - // Check if the session user is has user management privileges. + // Check if the session user has user management privileges. isAdmin := acl.Rules.AllowAll(acl.ResourceUsers, s.UserRole(), acl.Permissions{acl.AccessAll, acl.ActionManage}) privilegeLevelChange := isAdmin && m.PrivilegeLevelChange(f) + // Check if the user account quota has been exceeded. + if f.UserRole != "" && m.UserRole != f.UserRole && !conf.UsersQuotaReached(acl.ParseRole(m.UserRole)) && conf.UsersQuotaReached(acl.ParseRole(f.UserRole)) { + event.AuditErr([]string{ClientIP(c), "session %s", "users", m.UserName, "update", authn.ErrUsersQuotaExceeded.Error()}, s.RefID) + AbortQuotaExceeded(c) + return + } + // Get user from session. u := s.User() @@ -91,6 +100,10 @@ func UpdateUser(router *gin.RouterGroup) { // Flush session cache. if isAdmin { entity.FlushSessionCache() + if f.UserRole != "" { + config.FlushUsageCache() + UpdateClientConfig() + } } else { s.ClearCache() } diff --git a/internal/api/users_upload.go b/internal/api/users_upload.go index a5d39a323..c533f053d 100644 --- a/internal/api/users_upload.go +++ b/internal/api/users_upload.go @@ -53,7 +53,7 @@ func UploadUserFiles(router *gin.RouterGroup) { } // Abort if there is not enough free storage to upload new files. - if conf.FilesQuotaExceeded() { + if conf.FilesQuotaReached() { event.AuditErr([]string{ClientIP(c), "session %s", "upload files", "insufficient storage"}, s.RefID) Abort(c, http.StatusInsufficientStorage, i18n.ErrInsufficientStorage) return diff --git a/internal/auth/acl/const.go b/internal/auth/acl/const.go index 866ded1fe..365c5fc38 100644 --- a/internal/auth/acl/const.go +++ b/internal/auth/acl/const.go @@ -3,10 +3,10 @@ package acl // Roles that can be granted Permissions to use a Resource. const ( RoleDefault Role = "default" + RoleAdmin Role = "admin" RoleUser Role = "user" RoleViewer Role = "viewer" RoleGuest Role = "guest" - RoleAdmin Role = "admin" RoleVisitor Role = "visitor" RoleClient Role = "client" RoleNone Role = "" diff --git a/internal/auth/acl/role.go b/internal/auth/acl/role.go index 803c048e1..2c109534f 100644 --- a/internal/auth/acl/role.go +++ b/internal/auth/acl/role.go @@ -3,6 +3,7 @@ package acl import ( "strings" + "github.com/photoprism/photoprism/pkg/clean" "github.com/photoprism/photoprism/pkg/txt" ) @@ -47,3 +48,15 @@ func (r Role) Valid(s string) bool { func (r Role) Invalid(s string) bool { return !r.Valid(s) } + +// ParseRole returns the account role matching the specified string. +func ParseRole(s string) Role { + s = clean.Role(s) + + switch s { + case "", "0", "false", "nil", "null", "nan": + return RoleNone + default: + return UserRoles[s] + } +} diff --git a/internal/config/config_usage.go b/internal/config/config_usage.go index 6c5a9a44b..2b4a04100 100644 --- a/internal/config/config_usage.go +++ b/internal/config/config_usage.go @@ -6,6 +6,7 @@ import ( gc "github.com/patrickmn/go-cache" + "github.com/photoprism/photoprism/internal/auth/acl" "github.com/photoprism/photoprism/internal/entity/query" "github.com/photoprism/photoprism/pkg/fs" "github.com/photoprism/photoprism/pkg/fs/duf" @@ -73,21 +74,20 @@ func (c *Config) Usage() Usage { info.FilesUsedPct = 1 } - if info.FilesUsedPct > 100 { - info.FilesUsedPct = 100 - } - info.FilesFreePct = 100 - info.FilesUsedPct + if info.FilesFreePct < 0 { + info.FilesFreePct = 0 + } + if usersTotal := c.UsersQuota(); usersTotal > 0 { usersUsed := query.CountUsers(true, true, nil, []string{"guest"}) info.UsersUsedPct = int(math.Floor(float64(usersUsed) / float64(usersTotal) * 100)) - - if info.UsersUsedPct > 100 { - info.UsersUsedPct = 100 - } - info.UsersFreePct = 100 - info.UsersUsedPct + + if info.UsersFreePct < 0 { + info.UsersFreePct = 0 + } } usageCache.SetDefault(originalsPath, info) @@ -109,7 +109,7 @@ func (c *Config) FilesQuota() uint64 { return c.options.FilesQuota } -// FilesQuotaBytes returns the maximum aggregated size of all indexed files in bytes, or 0 if no limit exists. +// FilesQuotaBytes returns the maximum aggregated size of all indexed files in bytes, or 0 if unlimited. func (c *Config) FilesQuotaBytes() uint64 { if c.options.FilesQuota <= 0 { return 0 @@ -118,9 +118,18 @@ func (c *Config) FilesQuotaBytes() uint64 { return c.options.FilesQuota * fs.GB } -// FilesQuotaExceeded checks whether the filesystem usage has been reached or exceeded. -func (c *Config) FilesQuotaExceeded() bool { - return c.Usage().FilesUsedPct >= 100 +// FilesQuotaReached checks whether the filesystem usage has been reached or exceeded. +func (c *Config) FilesQuotaReached() bool { + return c.FilesQuotaExceeded(99) +} + +// FilesQuotaExceeded checks if the filesystem quota specified in percent has been exceeded. +func (c *Config) FilesQuotaExceeded(usedPct int) bool { + if c.options.FilesQuota <= 0 { + return false + } + + return c.Usage().FilesUsedPct > usedPct } // UsersQuota returns the maximum number of user accounts without guests, or 0 if unlimited. @@ -132,7 +141,21 @@ func (c *Config) UsersQuota() int { return c.options.UsersQuota } -// UsersQuotaExceeded checks whether the maximum number of user accounts has been reached or exceeded. -func (c *Config) UsersQuotaExceeded() bool { - return c.Usage().UsersUsedPct >= 100 +// UsersQuotaReached checks whether the maximum number of user accounts has been reached or exceeded. +func (c *Config) UsersQuotaReached(role acl.Role) bool { + return c.UsersQuotaExceeded(99, role) +} + +// UsersQuotaExceeded checks whether the number of user accounts specified in percent has been exceeded. +func (c *Config) UsersQuotaExceeded(usedPct int, role acl.Role) bool { + if c.options.UsersQuota <= 0 { + return false + } + + switch role { + case acl.RoleNone, acl.RoleGuest, acl.RoleVisitor, acl.RoleClient: + return false + default: + return c.Usage().UsersUsedPct > usedPct + } } diff --git a/internal/config/config_usage_test.go b/internal/config/config_usage_test.go index d5fa2bc4f..6e4b2ec01 100644 --- a/internal/config/config_usage_test.go +++ b/internal/config/config_usage_test.go @@ -5,6 +5,7 @@ import ( "github.com/stretchr/testify/assert" + "github.com/photoprism/photoprism/internal/auth/acl" "github.com/photoprism/photoprism/pkg/fs" "github.com/photoprism/photoprism/pkg/fs/duf" ) @@ -59,36 +60,52 @@ func TestConfig_Quota(t *testing.T) { c.options.UsersQuota = 0 } -func TestConfig_FilesQuotaExceeded(t *testing.T) { +func TestConfig_FilesQuotaReached(t *testing.T) { c := TestConfig() FlushUsageCache() - assert.False(t, c.FilesQuotaExceeded()) + assert.False(t, c.FilesQuotaReached()) + assert.False(t, c.FilesQuotaExceeded(-1)) + assert.False(t, c.FilesQuotaExceeded(99)) + assert.False(t, c.FilesQuotaExceeded(99)) c.options.FilesQuota = uint64(1) FlushUsageCache() - assert.True(t, c.FilesQuotaExceeded()) + assert.True(t, c.FilesQuotaReached()) + assert.True(t, c.FilesQuotaExceeded(-1)) + assert.True(t, c.FilesQuotaExceeded(99)) + assert.True(t, c.FilesQuotaExceeded(100)) c.options.FilesQuota = uint64(5) FlushUsageCache() - assert.False(t, c.FilesQuotaExceeded()) + assert.False(t, c.FilesQuotaReached()) c.options.FilesQuota = uint64(0) } -func TestConfig_UsersQuotaExceeded(t *testing.T) { +func TestConfig_UsersQuotaReached(t *testing.T) { c := TestConfig() FlushUsageCache() - assert.False(t, c.UsersQuotaExceeded()) + assert.False(t, c.UsersQuotaReached(acl.RoleUser)) c.options.UsersQuota = 1 FlushUsageCache() - assert.True(t, c.UsersQuotaExceeded()) + assert.True(t, c.UsersQuotaExceeded(99, acl.RoleAdmin)) + assert.True(t, c.UsersQuotaExceeded(100, acl.RoleAdmin)) + assert.True(t, c.UsersQuotaReached(acl.RoleAdmin)) + assert.True(t, c.UsersQuotaReached(acl.RoleUser)) + assert.False(t, c.UsersQuotaReached(acl.RoleNone)) + assert.False(t, c.UsersQuotaReached(acl.RoleGuest)) + assert.False(t, c.UsersQuotaReached(acl.RoleVisitor)) c.options.UsersQuota = 100000 FlushUsageCache() - assert.False(t, c.UsersQuotaExceeded()) + assert.False(t, c.UsersQuotaReached(acl.RoleAdmin)) + assert.False(t, c.UsersQuotaReached(acl.RoleUser)) + assert.False(t, c.UsersQuotaReached(acl.RoleNone)) + assert.False(t, c.UsersQuotaReached(acl.RoleGuest)) + assert.False(t, c.UsersQuotaReached(acl.RoleVisitor)) c.options.UsersQuota = 0 } diff --git a/internal/entity/auth_user.go b/internal/entity/auth_user.go index c75284710..a89fb9ef9 100644 --- a/internal/entity/auth_user.go +++ b/internal/entity/auth_user.go @@ -752,14 +752,7 @@ func (m *User) FullName() string { // SetRole sets the user role specified as string. func (m *User) SetRole(role string) *User { - role = clean.Role(role) - - switch role { - case "", "0", "false", "nil", "null", "nan": - m.UserRole = acl.RoleNone.String() - default: - m.UserRole = acl.UserRoles[role].String() - } + m.UserRole = acl.ParseRole(role).String() return m } diff --git a/internal/server/webdav.go b/internal/server/webdav.go index a90cafa8c..ba76e6251 100644 --- a/internal/server/webdav.go +++ b/internal/server/webdav.go @@ -95,7 +95,7 @@ func WebDAV(dir string, router *gin.RouterGroup, conf *config.Config) { // is not enough free storage to upload new files. switch c.Request.Method { case MethodPut, MethodPost, MethodPatch, MethodCopy: - if conf.FilesQuotaExceeded() { + if conf.FilesQuotaReached() { c.AbortWithStatus(http.StatusInsufficientStorage) return } diff --git a/internal/workers/sync_download.go b/internal/workers/sync_download.go index ce1a41619..1d236792c 100644 --- a/internal/workers/sync_download.go +++ b/internal/workers/sync_download.go @@ -105,7 +105,7 @@ func (w *Sync) download(a entity.Service) (complete bool, err error) { done := make(map[string]bool) for _, files := range relatedFiles { - if w.conf.FilesQuotaExceeded() { + if w.conf.FilesQuotaReached() { log.Warnf("sync: skipped downloading files from %s due to insufficient storage", clean.Log(a.AccName)) return false, nil }