fix(rbac): allow source-tenant Admin+ to manage their cross-tenant shares

Plan 3 of #1303 lifts share ownership to the tenant level, but
RemoveShare / UpdateSharePermission still gated on the original sharer
user. If that user left the tenant or moved on, the source tenant lost
the ability to revoke its own share — only the target org's admin
could clean up, which is governance theatre rather than ownership.

Extend the authz envelope on KB-share and agent-share mutations:

  (1) original sharer (unchanged);
  (2) source-tenant Admin+ — new, encodes "ownership lives in the
      tenant" by reading callerTenantRole from ctx;
  (3) target-org admin (unchanged).

Factored out as kbShareService.callerCanManageShare so Update and
Remove cannot drift apart.
This commit is contained in:
wizardchen
2026-05-15 22:52:19 +08:00
committed by lyingbug
parent eda6b1011a
commit f6684f226b
2 changed files with 56 additions and 18 deletions

View File

@@ -154,7 +154,8 @@ func (s *agentShareService) ShareAgent(ctx context.Context, agentID string, orgI
}
// RemoveShare removes an agent share.
// Same authz envelope as KB-share remove: original sharer or org admin.
// Same authz envelope as KB-share remove (see kbshare.callerCanManageShare):
// original sharer, OR source-tenant Admin+, OR target-org admin.
func (s *agentShareService) RemoveShare(ctx context.Context, shareID string, userID string, tenantID uint64) error {
share, err := s.shareRepo.GetByID(ctx, shareID)
if err != nil {
@@ -163,11 +164,18 @@ func (s *agentShareService) RemoveShare(ctx context.Context, shareID string, use
}
return err
}
// (1) Original sharer.
if share.SharedByUserID == userID {
return s.shareRepo.Delete(ctx, shareID)
}
tm, err := s.orgRepo.GetTenantMember(ctx, share.OrganizationID, tenantID)
if err == nil && tm.Role == types.OrgRoleAdmin {
// (2) Source-tenant Admin+ — Plan 3 ownership is tenant-level.
if tenantID != 0 && tenantID == share.SourceTenantID {
if types.TenantRoleFromContext(ctx).HasPermission(types.TenantRoleAdmin) {
return s.shareRepo.Delete(ctx, shareID)
}
}
// (3) Org admin in the target org (governance / sharer-left repair).
if tm, err := s.orgRepo.GetTenantMember(ctx, share.OrganizationID, tenantID); err == nil && tm.Role == types.OrgRoleAdmin {
return s.shareRepo.Delete(ctx, shareID)
}
return ErrAgentSharePermission

View File

@@ -139,9 +139,16 @@ func (s *kbShareService) ShareKnowledgeBase(ctx context.Context, kbID string, or
}
// UpdateSharePermission updates a share's permission.
// Allowed if (1) the caller is the original sharer (same user id), or
// (2) the caller's tenant is admin in the target org. The latter lets
// org admins repair shares when the original sharer leaves.
// Allowed if any one of:
//
// (1) the caller is the original sharer (same user id);
// (2) the caller's tenant IS the source tenant and the caller is
// Admin+ in their tenant — Plan 3 says ownership of a shared
// resource is tenant-level, so any Admin in the source tenant
// can manage what their tenant has shared, even if the original
// sharer user has left or moved tenants;
// (3) the caller's tenant is admin in the target org. The latter
// lets org admins repair shares when the original sharer leaves.
func (s *kbShareService) UpdateSharePermission(ctx context.Context, shareID string, permission types.OrgMemberRole, userID string, tenantID uint64) error {
share, err := s.shareRepo.GetByID(ctx, shareID)
if err != nil {
@@ -151,11 +158,8 @@ func (s *kbShareService) UpdateSharePermission(ctx context.Context, shareID stri
return err
}
if share.SharedByUserID != userID {
tm, err := s.orgRepo.GetTenantMember(ctx, share.OrganizationID, tenantID)
if err != nil || tm.Role != types.OrgRoleAdmin {
return ErrSharePermissionDenied
}
if !s.callerCanManageShare(ctx, share.SharedByUserID, share.SourceTenantID, share.OrganizationID, userID, tenantID) {
return ErrSharePermissionDenied
}
if !permission.IsValid() {
@@ -169,7 +173,7 @@ func (s *kbShareService) UpdateSharePermission(ctx context.Context, shareID stri
}
// RemoveShare removes a share.
// Same authz envelope as UpdateSharePermission: original sharer or org admin.
// Same authz envelope as UpdateSharePermission — see callerCanManageShare.
func (s *kbShareService) RemoveShare(ctx context.Context, shareID string, userID string, tenantID uint64) error {
share, err := s.shareRepo.GetByID(ctx, shareID)
if err != nil {
@@ -179,18 +183,44 @@ func (s *kbShareService) RemoveShare(ctx context.Context, shareID string, userID
return err
}
if share.SharedByUserID == userID {
return s.shareRepo.Delete(ctx, shareID)
}
tm, err := s.orgRepo.GetTenantMember(ctx, share.OrganizationID, tenantID)
if err == nil && tm.Role == types.OrgRoleAdmin {
if s.callerCanManageShare(ctx, share.SharedByUserID, share.SourceTenantID, share.OrganizationID, userID, tenantID) {
return s.shareRepo.Delete(ctx, shareID)
}
return ErrSharePermissionDenied
}
// callerCanManageShare encapsulates the "who can mutate this share" rule
// reused by Update/RemoveShare. See UpdateSharePermission's doc for the
// three accepted shapes. callerTenantRole is read from ctx so callers
// don't need to thread it explicitly; missing role defaults to Viewer
// (fail-closed) via TenantRoleFromContext.
func (s *kbShareService) callerCanManageShare(
ctx context.Context,
shareSharedByUserID string,
shareSourceTenantID uint64,
shareOrgID string,
callerUserID string,
callerTenantID uint64,
) bool {
// (1) Original sharer.
if shareSharedByUserID == callerUserID {
return true
}
// (2) Source-tenant Admin+ — Plan 3 ownership is tenant-level.
if callerTenantID != 0 && callerTenantID == shareSourceTenantID {
role := types.TenantRoleFromContext(ctx)
if role.HasPermission(types.TenantRoleAdmin) {
return true
}
}
// (3) Org admin in the target org (governance / sharer-left repair).
if tm, err := s.orgRepo.GetTenantMember(ctx, shareOrgID, callerTenantID); err == nil && tm.Role == types.OrgRoleAdmin {
return true
}
return false
}
// ListSharesByKnowledgeBase lists shares for a knowledge base; caller's tenant must own the KB.
func (s *kbShareService) ListSharesByKnowledgeBase(ctx context.Context, kbID string, tenantID uint64) ([]*types.KnowledgeBaseShare, error) {
kb, err := s.kbRepo.GetKnowledgeBaseByID(ctx, kbID)