fix: revalidate share and direct links after permission changes

Revalidate existing share links against the owner's current status and group permissions.
Reject redirected direct links when the owner's current group disables direct links.
Refresh restored share navigator state before serving cached paths.
Prevent client-side redirect caching and add regression coverage for restricted owners.

Co-authored-by: Codex <codex@openai.com>
pull/3524/head
Darren Yu 1 month ago
parent cec2b55e1e
commit e18785a9ca
No known key found for this signature in database
GPG Key ID: 2D69AA5646405984

@ -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)
})
}

@ -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)
}
})
}
}

@ -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

@ -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")
}
})
}
}

@ -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 {

@ -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
}

Loading…
Cancel
Save