diff --git a/inventory/share.go b/inventory/share.go index 3f18b40c..932405bd 100644 --- a/inventory/share.go +++ b/inventory/share.go @@ -24,9 +24,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 ( @@ -229,6 +230,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 { @@ -418,7 +428,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) }) } diff --git a/inventory/share_test.go b/inventory/share_test.go new file mode 100644 index 00000000..3ce31d71 --- /dev/null +++ b/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) + } + }) + } +} diff --git a/pkg/filemanager/fs/dbfs/manage.go b/pkg/filemanager/fs/dbfs/manage.go index f0471df5..e05c590d 100644 --- a/pkg/filemanager/fs/dbfs/manage.go +++ b/pkg/filemanager/fs/dbfs/manage.go @@ -643,6 +643,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 diff --git a/pkg/filemanager/fs/dbfs/manage_direct_link_test.go b/pkg/filemanager/fs/dbfs/manage_direct_link_test.go new file mode 100644 index 00000000..485355ff --- /dev/null +++ b/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") + } + }) + } +} diff --git a/pkg/filemanager/fs/dbfs/share_navigator.go b/pkg/filemanager/fs/dbfs/share_navigator.go index c03caf83..11823e88 100644 --- a/pkg/filemanager/fs/dbfs/share_navigator.go +++ b/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,36 @@ 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 + if n.shareRoot != nil { + n.shareRoot.OwnerModel = n.owner + } + 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 { diff --git a/service/explorer/file.go b/service/explorer/file.go index e82a60c3..fe44e942 100644 --- a/service/explorer/file.go +++ b/service/explorer/file.go @@ -678,7 +678,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), ) @@ -686,8 +686,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 }