fix(backend): require owner/admin to read or modify object ACLs
GET/PUT /acl/:object_type/:object_id performed no authorization check, so any authenticated user could read the permission list of, or grant themselves view/edit on, any file/tag/category/pool. ACLService now resolves the object's owner and rejects callers who are neither the owner nor an admin. SetPermissions also wraps its delete+insert replace in a single transaction so a partial failure can no longer wipe permissions. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
This commit is contained in:
@@ -77,7 +77,7 @@ func main() {
|
|||||||
cfg.JWTAccessTTL,
|
cfg.JWTAccessTTL,
|
||||||
cfg.JWTRefreshTTL,
|
cfg.JWTRefreshTTL,
|
||||||
)
|
)
|
||||||
aclSvc := service.NewACLService(aclRepo)
|
aclSvc := service.NewACLService(aclRepo, fileRepo, tagRepo, categoryRepo, poolRepo, transactor)
|
||||||
auditSvc := service.NewAuditService(auditRepo)
|
auditSvc := service.NewAuditService(auditRepo)
|
||||||
tagSvc := service.NewTagService(tagRepo, tagRuleRepo, aclSvc, auditSvc, transactor)
|
tagSvc := service.NewTagService(tagRepo, tagRuleRepo, aclSvc, auditSvc, transactor)
|
||||||
categorySvc := service.NewCategoryService(categoryRepo, tagRepo, aclSvc, auditSvc)
|
categorySvc := service.NewCategoryService(categoryRepo, tagRepo, aclSvc, auditSvc)
|
||||||
|
|||||||
@@ -80,7 +80,8 @@ func (h *ACLHandler) GetPermissions(c *gin.Context) {
|
|||||||
return
|
return
|
||||||
}
|
}
|
||||||
|
|
||||||
perms, err := h.aclSvc.GetPermissions(c.Request.Context(), objectTypeID, objectID)
|
userID, isAdmin, _ := domain.UserFromContext(c.Request.Context())
|
||||||
|
perms, err := h.aclSvc.GetPermissions(c.Request.Context(), userID, isAdmin, objectTypeID, objectID)
|
||||||
if err != nil {
|
if err != nil {
|
||||||
respondError(c, err)
|
respondError(c, err)
|
||||||
return
|
return
|
||||||
@@ -124,13 +125,14 @@ func (h *ACLHandler) SetPermissions(c *gin.Context) {
|
|||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
if err := h.aclSvc.SetPermissions(c.Request.Context(), objectTypeID, objectID, perms); err != nil {
|
userID, isAdmin, _ := domain.UserFromContext(c.Request.Context())
|
||||||
|
if err := h.aclSvc.SetPermissions(c.Request.Context(), userID, isAdmin, objectTypeID, objectID, perms); err != nil {
|
||||||
respondError(c, err)
|
respondError(c, err)
|
||||||
return
|
return
|
||||||
}
|
}
|
||||||
|
|
||||||
// Re-read to return the stored permissions (with UserName denormalized).
|
// Re-read to return the stored permissions (with UserName denormalized).
|
||||||
stored, err := h.aclSvc.GetPermissions(c.Request.Context(), objectTypeID, objectID)
|
stored, err := h.aclSvc.GetPermissions(c.Request.Context(), userID, isAdmin, objectTypeID, objectID)
|
||||||
if err != nil {
|
if err != nil {
|
||||||
respondError(c, err)
|
respondError(c, err)
|
||||||
return
|
return
|
||||||
|
|||||||
@@ -125,7 +125,7 @@ func setupSuite(t *testing.T) *harness {
|
|||||||
|
|
||||||
// --- Services ------------------------------------------------------------
|
// --- Services ------------------------------------------------------------
|
||||||
authSvc := service.NewAuthService(userRepo, sessionRepo, "test-secret", 15*time.Minute, 720*time.Hour)
|
authSvc := service.NewAuthService(userRepo, sessionRepo, "test-secret", 15*time.Minute, 720*time.Hour)
|
||||||
aclSvc := service.NewACLService(aclRepo)
|
aclSvc := service.NewACLService(aclRepo, fileRepo, tagRepo, categoryRepo, poolRepo, transactor)
|
||||||
auditSvc := service.NewAuditService(auditRepo)
|
auditSvc := service.NewAuditService(auditRepo)
|
||||||
tagSvc := service.NewTagService(tagRepo, tagRuleRepo, aclSvc, auditSvc, transactor)
|
tagSvc := service.NewTagService(tagRepo, tagRuleRepo, aclSvc, auditSvc, transactor)
|
||||||
categorySvc := service.NewCategoryService(categoryRepo, tagRepo, aclSvc, auditSvc)
|
categorySvc := service.NewCategoryService(categoryRepo, tagRepo, aclSvc, auditSvc)
|
||||||
|
|||||||
@@ -12,11 +12,32 @@ import (
|
|||||||
|
|
||||||
// ACLService handles access control checks and permission management.
|
// ACLService handles access control checks and permission management.
|
||||||
type ACLService struct {
|
type ACLService struct {
|
||||||
aclRepo port.ACLRepo
|
aclRepo port.ACLRepo
|
||||||
|
files port.FileRepo
|
||||||
|
tags port.TagRepo
|
||||||
|
categories port.CategoryRepo
|
||||||
|
pools port.PoolRepo
|
||||||
|
tx port.Transactor
|
||||||
}
|
}
|
||||||
|
|
||||||
func NewACLService(aclRepo port.ACLRepo) *ACLService {
|
// NewACLService creates an ACLService. The object repositories are used to
|
||||||
return &ACLService{aclRepo: aclRepo}
|
// resolve an object's owner when authorizing permission management.
|
||||||
|
func NewACLService(
|
||||||
|
aclRepo port.ACLRepo,
|
||||||
|
files port.FileRepo,
|
||||||
|
tags port.TagRepo,
|
||||||
|
categories port.CategoryRepo,
|
||||||
|
pools port.PoolRepo,
|
||||||
|
tx port.Transactor,
|
||||||
|
) *ACLService {
|
||||||
|
return &ACLService{
|
||||||
|
aclRepo: aclRepo,
|
||||||
|
files: files,
|
||||||
|
tags: tags,
|
||||||
|
categories: categories,
|
||||||
|
pools: pools,
|
||||||
|
tx: tx,
|
||||||
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
// CanView returns true if the user may view the object.
|
// CanView returns true if the user may view the object.
|
||||||
@@ -70,12 +91,86 @@ func (s *ACLService) CanEdit(
|
|||||||
return perm.CanEdit, nil
|
return perm.CanEdit, nil
|
||||||
}
|
}
|
||||||
|
|
||||||
// GetPermissions returns all explicit ACL entries for an object.
|
// GetPermissions returns all explicit ACL entries for an object. Only the
|
||||||
func (s *ACLService) GetPermissions(ctx context.Context, objectTypeID int16, objectID uuid.UUID) ([]domain.Permission, error) {
|
// object's owner or an admin may inspect its permission list.
|
||||||
|
func (s *ACLService) GetPermissions(
|
||||||
|
ctx context.Context,
|
||||||
|
userID int16, isAdmin bool,
|
||||||
|
objectTypeID int16, objectID uuid.UUID,
|
||||||
|
) ([]domain.Permission, error) {
|
||||||
|
if err := s.authorizeManage(ctx, userID, isAdmin, objectTypeID, objectID); err != nil {
|
||||||
|
return nil, err
|
||||||
|
}
|
||||||
return s.aclRepo.List(ctx, objectTypeID, objectID)
|
return s.aclRepo.List(ctx, objectTypeID, objectID)
|
||||||
}
|
}
|
||||||
|
|
||||||
// SetPermissions replaces all ACL entries for an object (full replace semantics).
|
// SetPermissions replaces all ACL entries for an object (full replace semantics).
|
||||||
func (s *ACLService) SetPermissions(ctx context.Context, objectTypeID int16, objectID uuid.UUID, perms []domain.Permission) error {
|
// Only the object's owner or an admin may change its permissions. The replace is
|
||||||
return s.aclRepo.Set(ctx, objectTypeID, objectID, perms)
|
// performed atomically inside a single transaction.
|
||||||
|
func (s *ACLService) SetPermissions(
|
||||||
|
ctx context.Context,
|
||||||
|
userID int16, isAdmin bool,
|
||||||
|
objectTypeID int16, objectID uuid.UUID,
|
||||||
|
perms []domain.Permission,
|
||||||
|
) error {
|
||||||
|
if err := s.authorizeManage(ctx, userID, isAdmin, objectTypeID, objectID); err != nil {
|
||||||
|
return err
|
||||||
|
}
|
||||||
|
return s.tx.WithTx(ctx, func(ctx context.Context) error {
|
||||||
|
return s.aclRepo.Set(ctx, objectTypeID, objectID, perms)
|
||||||
|
})
|
||||||
|
}
|
||||||
|
|
||||||
|
// authorizeManage returns nil if the user may manage the object's ACL
|
||||||
|
// (admin or owner), ErrForbidden otherwise, or a propagated lookup error
|
||||||
|
// (including ErrNotFound when the object does not exist).
|
||||||
|
func (s *ACLService) authorizeManage(
|
||||||
|
ctx context.Context,
|
||||||
|
userID int16, isAdmin bool,
|
||||||
|
objectTypeID int16, objectID uuid.UUID,
|
||||||
|
) error {
|
||||||
|
if isAdmin {
|
||||||
|
return nil
|
||||||
|
}
|
||||||
|
owner, err := s.objectOwner(ctx, objectTypeID, objectID)
|
||||||
|
if err != nil {
|
||||||
|
return err
|
||||||
|
}
|
||||||
|
if owner != userID {
|
||||||
|
return domain.ErrForbidden
|
||||||
|
}
|
||||||
|
return nil
|
||||||
|
}
|
||||||
|
|
||||||
|
// objectOwner resolves the creator ID of the object identified by
|
||||||
|
// (objectTypeID, objectID). Returns ErrNotFound if the object does not exist.
|
||||||
|
func (s *ACLService) objectOwner(ctx context.Context, objectTypeID int16, objectID uuid.UUID) (int16, error) {
|
||||||
|
switch objectTypeID {
|
||||||
|
case fileObjectTypeID:
|
||||||
|
obj, err := s.files.GetByID(ctx, objectID)
|
||||||
|
if err != nil {
|
||||||
|
return 0, err
|
||||||
|
}
|
||||||
|
return obj.CreatorID, nil
|
||||||
|
case tagObjectTypeID:
|
||||||
|
obj, err := s.tags.GetByID(ctx, objectID)
|
||||||
|
if err != nil {
|
||||||
|
return 0, err
|
||||||
|
}
|
||||||
|
return obj.CreatorID, nil
|
||||||
|
case categoryObjectTypeID:
|
||||||
|
obj, err := s.categories.GetByID(ctx, objectID)
|
||||||
|
if err != nil {
|
||||||
|
return 0, err
|
||||||
|
}
|
||||||
|
return obj.CreatorID, nil
|
||||||
|
case poolObjectTypeID:
|
||||||
|
obj, err := s.pools.GetByID(ctx, objectID)
|
||||||
|
if err != nil {
|
||||||
|
return 0, err
|
||||||
|
}
|
||||||
|
return obj.CreatorID, nil
|
||||||
|
default:
|
||||||
|
return 0, domain.ErrValidation
|
||||||
|
}
|
||||||
}
|
}
|
||||||
|
|||||||
Reference in New Issue
Block a user