Merge e4a0e888b5 into 1c3fe4da3c

inventory/share.go

@@ -25,9 +25,10 @@ type (
 )
 
 var (
-	ErrShareLinkExpired  = fmt.Errorf("share link expired")
-	ErrOwnerInactive     = fmt.Errorf("owner is inactive")
-	ErrSourceFileInvalid = fmt.Errorf("source file is deleted")
+	ErrShareLinkExpired   = fmt.Errorf("share link expired")
+	ErrOwnerInactive      = fmt.Errorf("owner is inactive")
+	ErrOwnerShareDisabled = fmt.Errorf("owner is not allowed to share files")
+	ErrSourceFileInvalid  = fmt.Errorf("source file is deleted")
 )
 
 type (
@@ -237,6 +238,15 @@ func IsValidShare(share *ent.Share) error {
 		return ErrOwnerInactive
 	}
 
+	// Creating and accessing share links are governed by the owner's current
+	// group. This makes existing links unavailable as soon as the permission is
+	// revoked, instead of only preventing the creation of new links.
+	ownerGroup, err := owner.Edges.GroupOrErr()
+	if err != nil || ownerGroup.Permissions == nil ||
+		!ownerGroup.Permissions.Enabled(int(types.GroupPermissionShare)) {
+		return ErrOwnerShareDisabled
+	}
+
 	// Check source file status
 	file, err := share.Edges.FileOrErr()
 	if err != nil || file.FileChildren == 0 || file.OwnerID != owner.ID {
@@ -426,7 +436,8 @@ func withShareEagerLoading(ctx context.Context, q *ent.ShareQuery) *ent.ShareQue
 	}
 	if v, ok := ctx.Value(LoadShareUser{}).(bool); ok && v {
 		q.WithUser(func(q *ent.UserQuery) {
-			withUserEagerLoading(ctx, q)
+			userCtx := context.WithValue(ctx, LoadUserGroup{}, true)
+			withUserEagerLoading(userCtx, q)
 		})
 	}
 

inventory/share_test.go

@@ -0,0 +1,44 @@
+package inventory
+
+import (
+	"testing"
+
+	"github.com/cloudreve/Cloudreve/v4/ent"
+	entuser "github.com/cloudreve/Cloudreve/v4/ent/user"
+	"github.com/cloudreve/Cloudreve/v4/inventory/types"
+	"github.com/cloudreve/Cloudreve/v4/pkg/boolset"
+)
+
+func TestIsValidShareChecksCurrentOwnerAccess(t *testing.T) {
+	tests := []struct {
+		name     string
+		status   entuser.Status
+		canShare bool
+		wantErr  bool
+	}{
+		{name: "active owner with share permission", status: entuser.StatusActive, canShare: true},
+		{name: "active owner without share permission", status: entuser.StatusActive, wantErr: true},
+		{name: "manually banned owner", status: entuser.StatusManualBanned, canShare: true, wantErr: true},
+		{name: "system banned owner", status: entuser.StatusSysBanned, canShare: true, wantErr: true},
+		{name: "inactive owner", status: entuser.StatusInactive, canShare: true, wantErr: true},
+	}
+
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			permissions := &boolset.BooleanSet{}
+			boolset.Set(types.GroupPermissionShare, tt.canShare, permissions)
+			group := &ent.Group{Permissions: permissions}
+			owner := &ent.User{ID: 1, Status: tt.status}
+			owner.SetGroup(group)
+			file := &ent.File{OwnerID: owner.ID, FileChildren: 1}
+			share := &ent.Share{}
+			share.SetUser(owner)
+			share.SetFile(file)
+
+			err := IsValidShare(share)
+			if (err != nil) != tt.wantErr {
+				t.Fatalf("IsValidShare() error = %v, wantErr %v", err, tt.wantErr)
+			}
+		})
+	}
+}

pkg/filemanager/fs/dbfs/manage.go

@@ -698,6 +698,17 @@ func (f *DBFS) GetFileFromDirectLink(ctx context.Context, dl *ent.DirectLink) (f
 		return nil, fs.ErrDirectLinkInvalid.WithError(fmt.Errorf("file owner is not active"))
 	}
 
+	// Revalidate the owner's current direct-link permission so existing links
+	// are revoked when direct links are disabled for the current group, whether
+	// by changing the group setting or moving the owner to another group.
+	group, err := owner.Edges.GroupOrErr()
+	if err != nil {
+		return nil, fs.ErrDirectLinkInvalid.WithError(fmt.Errorf("file owner group is unavailable: %w", err))
+	}
+	if group.Settings == nil || group.Settings.SourceBatchSize <= 0 {
+		return nil, fs.ErrDirectLinkInvalid.WithError(fmt.Errorf("file owner is not allowed to create direct links"))
+	}
+
 	file := newFile(nil, fileModel)
 
 	// Traverse to the root file

pkg/filemanager/fs/dbfs/manage_direct_link_test.go

@@ -0,0 +1,38 @@
+package dbfs
+
+import (
+	"context"
+	"testing"
+
+	"github.com/cloudreve/Cloudreve/v4/ent"
+	entuser "github.com/cloudreve/Cloudreve/v4/ent/user"
+	"github.com/cloudreve/Cloudreve/v4/inventory/types"
+)
+
+func TestGetFileFromDirectLinkRejectsRestrictedOwner(t *testing.T) {
+	tests := []struct {
+		name      string
+		status    entuser.Status
+		batchSize int
+	}{
+		{name: "active owner without direct link permission", status: entuser.StatusActive},
+		{name: "manually banned owner", status: entuser.StatusManualBanned, batchSize: 1},
+		{name: "system banned owner", status: entuser.StatusSysBanned, batchSize: 1},
+		{name: "inactive owner", status: entuser.StatusInactive, batchSize: 1},
+	}
+
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			owner := &ent.User{Status: tt.status}
+			owner.SetGroup(&ent.Group{Settings: &types.GroupSetting{SourceBatchSize: tt.batchSize}})
+			file := &ent.File{}
+			file.SetOwner(owner)
+			link := &ent.DirectLink{}
+			link.SetFile(file)
+
+			if _, err := (&DBFS{}).GetFileFromDirectLink(context.Background(), link); err == nil {
+				t.Fatal("GetFileFromDirectLink() error = nil, want restricted owner to be rejected")
+			}
+		})
+	}
+}

pkg/filemanager/fs/dbfs/share_navigator.go

@@ -57,6 +57,7 @@ type (
 		share           *ent.Share
 		owner           *ent.User
 		disableRecycle  bool
+		restoredState   bool
 		persist         func()
 	}
 
@@ -90,6 +91,7 @@ func (n *shareNavigator) RestoreState(s State) error {
 		n.singleFileShare = state.SingleFileShare
 		n.share = state.Share
 		n.owner = state.Owner
+		n.restoredState = true
 		return nil
 	}
 
@@ -179,6 +181,57 @@ func (n *shareNavigator) Root(ctx context.Context, path *fs.URI) (*File, error)
 }
 
 func (n *shareNavigator) To(ctx context.Context, path *fs.URI) (*File, error) {
+	// Navigator state contains the share owner's status and group. Revalidate it
+	// after a cache restore so a ban or group permission change takes effect
+	// immediately instead of being bypassed for ContextHintTTL.
+	if n.restoredState {
+		shareCtx := context.WithValue(ctx, inventory.LoadShareUser{}, true)
+		shareCtx = context.WithValue(shareCtx, inventory.LoadShareFile{}, true)
+		share, err := n.shareClient.GetByHashID(shareCtx, path.ID(hashid.EncodeUserID(n.hasher, n.user.ID)))
+		if err != nil {
+			return nil, ErrShareNotFound.WithError(err)
+		}
+
+		if err := inventory.IsValidShare(share); err != nil {
+			return nil, ErrShareNotFound.WithError(err)
+		}
+
+		if share.Password != "" && share.Password != path.Password() {
+			return nil, ErrShareIncorrectPassword
+		}
+
+		n.share = share
+		n.owner = share.Edges.User
+
+		// Root() is skipped when using restored state, so re-check requester permissions
+		// to ensure group permission changes take effect immediately.
+		if n.user.ID != n.owner.ID && !n.user.Edges.Group.Permissions.Enabled(int(types.GroupPermissionShareDownload)) {
+			if inventory.IsAnonymousUser(n.user) {
+				return nil, serializer.NewError(
+					serializer.CodeAnonymouseAccessDenied,
+					fmt.Sprintf("You don't have permission to access share links"),
+					nil,
+				)
+			}
+
+			return nil, serializer.NewError(
+				serializer.CodeNoPermissionErr,
+				fmt.Sprintf("You don't have permission to access share links"),
+				nil,
+			)
+		}
+
+		if n.shareRoot != nil {
+			n.shareRoot.OwnerModel = n.owner
+			n.shareRoot.disableView = (share.Props == nil || !share.Props.ShareView) && n.user.ID != n.owner.ID
+			n.shareRoot.CapabilitiesBs = n.Capabilities(false).Capability
+		}
+		if n.ownerRoot != nil {
+			n.ownerRoot.OwnerModel = n.owner
+		}
+		n.restoredState = false
+	}
+
 	if n.shareRoot == nil {
 		root, err := n.Root(ctx, path)
 		if err != nil {

pkg/filemanager/fs/dbfs/share_navigator_test.go

@@ -0,0 +1,105 @@
+package dbfs
+
+import (
+	"context"
+	"testing"
+
+	"github.com/cloudreve/Cloudreve/v4/ent"
+	entuser "github.com/cloudreve/Cloudreve/v4/ent/user"
+	"github.com/cloudreve/Cloudreve/v4/inventory"
+	"github.com/cloudreve/Cloudreve/v4/inventory/types"
+	"github.com/cloudreve/Cloudreve/v4/pkg/boolset"
+	"github.com/cloudreve/Cloudreve/v4/pkg/filemanager/fs"
+	"github.com/cloudreve/Cloudreve/v4/pkg/hashid"
+	"github.com/cloudreve/Cloudreve/v4/pkg/setting"
+)
+
+type restoredStateShareClient struct {
+	inventory.ShareClient
+	share *ent.Share
+}
+
+func (c *restoredStateShareClient) GetByHashID(context.Context, string) (*ent.Share, error) {
+	return c.share, nil
+}
+
+func TestShareNavigatorRestoredStateRevalidatesRequester(t *testing.T) {
+	ownerPermissions := &boolset.BooleanSet{}
+	boolset.Set(types.GroupPermissionShare, true, ownerPermissions)
+	owner := &ent.User{ID: 1, Status: entuser.StatusActive}
+	owner.SetGroup(&ent.Group{Permissions: ownerPermissions})
+
+	file := &ent.File{OwnerID: owner.ID, FileChildren: 1}
+	share := &ent.Share{ID: 1, Props: &types.ShareProps{ShareView: true}}
+	share.SetUser(owner)
+	share.SetFile(file)
+
+	hasher, err := hashid.New("restored-state-test")
+	if err != nil {
+		t.Fatalf("hashid.New() error = %v", err)
+	}
+	path, err := fs.NewUriFromString(fs.NewShareUri(hashid.EncodeShareID(hasher, share.ID), ""))
+	if err != nil {
+		t.Fatalf("fs.NewUriFromString() error = %v", err)
+	}
+
+	tests := []struct {
+		name        string
+		canDownload bool
+		wantErr     bool
+	}{
+		{name: "permission revoked", wantErr: true},
+		{name: "permission retained", canDownload: true},
+	}
+
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			requesterPermissions := &boolset.BooleanSet{}
+			boolset.Set(types.GroupPermissionShareDownload, tt.canDownload, requesterPermissions)
+			requester := &ent.User{ID: 2, Status: entuser.StatusActive}
+			requester.SetGroup(&ent.Group{Permissions: requesterPermissions})
+
+			cachedOwner := &ent.User{ID: owner.ID, Status: entuser.StatusActive}
+			root := newFile(nil, file)
+			t.Cleanup(root.Recycle)
+			root.OwnerModel = cachedOwner
+			root.disableView = true
+			root.CapabilitiesBs = &boolset.BooleanSet{0xff}
+
+			navigator := NewShareNavigator(
+				requester,
+				nil,
+				&restoredStateShareClient{share: share},
+				nil,
+				&setting.DBFS{},
+				hasher,
+			).(*shareNavigator)
+			if err := navigator.RestoreState(shareNavigatorState{
+				ShareRoot: root,
+				OwnerRoot: root,
+				Share:     share,
+				Owner:     cachedOwner,
+			}); err != nil {
+				t.Fatalf("RestoreState() error = %v", err)
+			}
+
+			_, err := navigator.To(context.Background(), path)
+			if (err != nil) != tt.wantErr {
+				t.Fatalf("To() error = %v, wantErr %v", err, tt.wantErr)
+			}
+			if tt.wantErr {
+				return
+			}
+
+			if root.OwnerModel != owner {
+				t.Fatal("To() did not refresh the cached owner")
+			}
+			if root.disableView {
+				t.Fatal("To() did not refresh the cached share view setting")
+			}
+			if root.CapabilitiesBs != shareNavigatorCapability {
+				t.Fatal("To() did not refresh the cached navigator capabilities")
+			}
+		})
+	}
+}

service/explorer/file.go

@@ -692,7 +692,7 @@ func RedirectDirectLink(c *gin.Context, name string, download bool) error {
 
 	// Request entity URL
 	expire := time.Now().Add(settings.EntityUrlValidDuration(c))
-	res, earliestExpire, err := m.GetUrlForRedirectedDirectLink(c, dl,
+	res, _, err := m.GetUrlForRedirectedDirectLink(c, dl,
 		fs.WithUrlExpire(&expire),
 		fs.WithIsDownload(download),
 	)
@@ -700,8 +700,8 @@ func RedirectDirectLink(c *gin.Context, name string, download bool) error {
 		return err
 	}
 
+	c.Header("Cache-Control", "no-store")
 	c.Redirect(http.StatusFound, res)
-	c.Header("Cache-Control", fmt.Sprintf("public, max-age=%d", int(earliestExpire.Sub(time.Now()).Seconds())))
 	return nil
 }