diff options
| author | Paul Buetow <paul@buetow.org> | 2026-05-20 07:06:04 +0300 |
|---|---|---|
| committer | Paul Buetow <paul@buetow.org> | 2026-05-20 07:06:04 +0300 |
| commit | 190bc9ecfb2e6026d0ff831b89dcc2a39843775d (patch) | |
| tree | 79254f6005be3e2d84cb8857084a74682c8d8531 | |
| parent | 9d41de31153286103f73f6188049413a8c0d45ed (diff) | |
Promote allowedSetIDs to accessHelper (da)
browseService.ListMedia and ListSets each contained the same
"compute permitted set IDs for this non-admin user" logic. Promote
it to an exported method (h.AllowedSetIDs) on accessHelper so the
permission resolution lives in one place, then call it from both
sites in browse.go. Tests still pass via the existing service
suite — no new unit test was needed since accessHelper is exercised
through its callers.
Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
| -rw-r--r-- | player-server/internal/service/access.go | 35 | ||||
| -rw-r--r-- | player-server/internal/service/browse.go | 26 |
2 files changed, 44 insertions, 17 deletions
diff --git a/player-server/internal/service/access.go b/player-server/internal/service/access.go index e8546d6..dd85790 100644 --- a/player-server/internal/service/access.go +++ b/player-server/internal/service/access.go @@ -115,3 +115,38 @@ func (h *accessHelper) verifyRestoreAccess(ctx context.Context, mediaID, userID func (h *accessHelper) verifySetModifyAccess(ctx context.Context, setID, userID int64) error { return h.checkSetPermission(ctx, setID, userID, model.RoleOwner) } + +// AllowedSetIDs returns the set IDs the given user can read. +// +// For admin users it returns (nil, true, nil) — a nil slice with the isAdmin +// flag set, meaning "no restriction, all sets are visible". Callers should +// translate this into a repository filter that does not constrain set IDs. +// +// For non-admin users it returns the (possibly empty) list of set IDs the +// user has any permission on, with isAdmin==false. An empty non-nil slice +// means the user has no readable sets and the caller should short-circuit +// rather than querying the repository. +// +// This method exists so browse.ListMedia and progress.ListInProgress can +// share the same "what can this user see?" logic instead of reimplementing +// the admin/permission lookup separately. +func (h *accessHelper) AllowedSetIDs(ctx context.Context, userID int64) (ids []int64, isAdmin bool, err error) { + user, err := h.store.GetUserByID(ctx, userID) + if err != nil { + return nil, false, fmt.Errorf("get user: %w", err) + } + if user != nil && user.IsAdmin { + return nil, true, nil + } + + perms, err := h.store.ListPermissionsByUser(ctx, userID) + if err != nil { + return nil, false, fmt.Errorf("list permissions: %w", err) + } + + allowed := make([]int64, 0, len(perms)) + for _, p := range perms { + allowed = append(allowed, p.SetID) + } + return allowed, false, nil +} diff --git a/player-server/internal/service/browse.go b/player-server/internal/service/browse.go index be0ee2c..5786017 100644 --- a/player-server/internal/service/browse.go +++ b/player-server/internal/service/browse.go @@ -184,11 +184,6 @@ func (s *browseService) GetMediaDetail(ctx context.Context, mediaID, userID int6 } func (s *browseService) ListMedia(ctx context.Context, userID int64, filter MediaQueryFilter) ([]model.Media, error) { - user, err := s.store.GetUserByID(ctx, userID) - if err != nil { - return nil, fmt.Errorf("get user: %w", err) - } - repoFilter := repository.MediaFilter{ SetID: filter.SetID, SetIDs: filter.SetIDs, @@ -203,27 +198,24 @@ func (s *browseService) ListMedia(ctx context.Context, userID int64, filter Medi Sort: filter.Sort, Limit: filter.Limit, Offset: filter.Offset, + UserID: userID, } - if user != nil && user.IsAdmin { - repoFilter.UserID = userID - return s.store.ListMedia(ctx, repoFilter) - } - - perms, err := s.store.ListPermissionsByUser(ctx, userID) + // Delegate "what sets can this user see?" to the shared accessHelper so + // browse and progress agree on the admin/permission semantics. Admins + // get isAdmin=true and an unrestricted query; non-admins get a concrete + // (possibly empty) allow-list that we use to short-circuit empty cases. + allowed, isAdmin, err := s.helper.AllowedSetIDs(ctx, userID) if err != nil { - return nil, fmt.Errorf("list permissions: %w", err) + return nil, err } - - allowed := make([]int64, 0, len(perms)) - for _, p := range perms { - allowed = append(allowed, p.SetID) + if isAdmin { + return s.store.ListMedia(ctx, repoFilter) } if len(allowed) == 0 { return []model.Media{}, nil } repoFilter.AllowedSetIDs = allowed - repoFilter.UserID = userID return s.store.ListMedia(ctx, repoFilter) } |
