Skip to content

Commit d85f084

Browse files
LanfeiclaudePIKACHUIM
authored
feat(permissions): implement fine-grained permission control (#2145)
* refactor(permission): rename permission check functions for clarity - User.CanWrite() → User.CanCreateFilesOrFolders() - common.CanWrite() → common.CanWriteContentBypassUserPerms() - common.IsApply() → common.MetaCoversPath() Improves code readability by making function names more descriptive. The new MetaCoversPath name clearly indicates it checks if a meta rule covers a specific path. It better conveys that it's a query function rather than an action, and the applyToSubFolder parameter is more explicit than applySub. Also adds comprehensive test coverage: - 10 tests for MetaCoversPath core logic - 6 tests for CanWriteContent UserPerms - 7 tests for getReadme - 5 tests for getHeader - 6 tests for isEncrypt - 9 tests for whetherHide Total: 43 test scenarios covering all path matching and permission inheritance logic. Tests verify both normal behavior and bug fixes for Readme/Header information leakage and write permission bypass. Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com> * feat(permission): implement fine-grained user permissions for read/write operations Add per-user read and write permission controls at the meta level to enable more granular access control beyond the existing permission flags. Key changes: - Add ReadUsers/WriteUsers fields to Meta model with sub-directory inheritance flags - Implement CanRead and CanWrite permission check functions in server/common - Filter file list results based on user read permissions - Add permission checks across all file operations (FTP, HTTP handlers, WebDAV) - Simplify error handling pattern for MetaNotFound errors throughout codebase This allows administrators to restrict specific users from accessing or modifying certain paths, providing finer control over file system permissions. Note: Batch and recursive operations (FsMove, FsCopy, FsRemove, FsRecursiveMove, FsBatchRename, FsRegexRename) currently check parent directory permissions only. Individual item permission checks are not performed for performance reasons. Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com> * test(permission): add comprehensive tests for CanRead, CanWrite, and combined permission checks Add TestCanRead, TestCanWrite, TestCanAccessWithReadPermissions, and TestWritePermissionCombinations to validate the three-layer permission system including nil user/meta, sub-path inheritance, user whitelists, and root-level restrictions. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> * fix(webdav): use safe type assertion for MetaPassKey to prevent panic Bearer-token and OPTIONS auth paths do not set MetaPassKey in context, causing a panic when handlers perform a forced type assertion on nil. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> * fix(permission): treat nil user as system context in CanRead/CanWrite Previously, CanRead/CanWrite returned false for nil user, causing filterReadableObjs to return an empty list when fs.List is called from internal contexts without a user (e.g. context.Background()). A nil user represents an internal/system call and should bypass per-user restrictions, consistent with how whetherHide already handles nil user. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> * fix(fsmanage): prevent path traversal in FsRemove The previous check only skipped names that resolved to "/", but did not prevent traversal to sibling directories (e.g. "../secret"), which could bypass the CanWrite permission check that is only applied to req.Dir. Replace with a post-join prefix check to ensure each resolved path stays within reqPath. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> * fix(webdav): align MetaPassKey behavior with FTP auth logic For guest users, the WebDAV password input serves as the meta folder password (consistent with FTP anonymous/guest handling). For authenticated users, MetaPassKey is set to empty string since their login password is not the meta folder password. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> * fix(permission): require write auth for fs list refresh * refactor(permission): use MetaCoversPath in CanRead/CanWrite for consistency Replace inline `(Sub || meta.Path == path)` logic with MetaCoversPath, consistent with CanWriteContentBypassUserPerms. Also fix a copy-paste error in the CanWrite comment (read → write). Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> --------- Co-authored-by: Claude Sonnet 4.5 <noreply@anthropic.com> Co-authored-by: Pikachu Ren <40362270+PIKACHUIM@users.noreply.github.com>
1 parent d598ef7 commit d85f084

File tree

20 files changed

+1850
-190
lines changed

20 files changed

+1850
-190
lines changed

‎internal/fs/list.go‎

Lines changed: 27 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -2,13 +2,14 @@ package fs
22

33
import (
44
"context"
5-
65
"github.com/OpenListTeam/OpenList/v4/internal/conf"
6+
"github.com/OpenListTeam/OpenList/v4/internal/errs"
77
"github.com/OpenListTeam/OpenList/v4/internal/model"
88
"github.com/OpenListTeam/OpenList/v4/internal/op"
9-
"github.com/OpenListTeam/OpenList/v4/pkg/utils"
9+
"github.com/OpenListTeam/OpenList/v4/server/common"
1010
"github.com/pkg/errors"
1111
log "github.com/sirupsen/logrus"
12+
"path"
1213
)
1314

1415
// List files
@@ -43,7 +44,29 @@ func list(ctx context.Context, path string, args *ListArgs) ([]model.Obj, error)
4344
om.InitHideReg(meta.Hide)
4445
}
4546
objs := om.Merge(_objs, virtualFiles...)
46-
return objs, nil
47+
objs, err = filterReadableObjs(objs, user, path, meta)
48+
return objs, err
49+
}
50+
51+
func filterReadableObjs(objs []model.Obj, user *model.User, reqPath string, parentMeta *model.Meta) ([]model.Obj, error) {
52+
var result []model.Obj
53+
for _, obj := range objs {
54+
var meta *model.Meta
55+
objPath := path.Join(reqPath, obj.GetName())
56+
if obj.IsDir() {
57+
var err error
58+
meta, err = op.GetNearestMeta(objPath)
59+
if err != nil && !errors.Is(errors.Cause(err), errs.MetaNotFound) {
60+
return result, err
61+
}
62+
} else {
63+
meta = parentMeta
64+
}
65+
if common.CanRead(user, meta, objPath) {
66+
result = append(result, obj)
67+
}
68+
}
69+
return result, nil
4770
}
4871

4972
func whetherHide(user *model.User, meta *model.Meta, path string) bool {
@@ -60,7 +83,7 @@ func whetherHide(user *model.User, meta *model.Meta, path string) bool {
6083
return false
6184
}
6285
// if meta doesn't apply to sub_folder, don't hide
63-
if !utils.PathEqual(meta.Path, path) && !meta.HSub {
86+
if !common.MetaCoversPath(meta.Path, path, meta.HSub) {
6487
return false
6588
}
6689
// if is guest, hide

‎internal/fs/list_test.go‎

Lines changed: 151 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,151 @@
1+
package fs
2+
3+
import (
4+
"testing"
5+
6+
"github.com/OpenListTeam/OpenList/v4/internal/model"
7+
)
8+
9+
func TestWhetherHide(t *testing.T) {
10+
tests := []struct {
11+
name string
12+
user *model.User
13+
meta *model.Meta
14+
path string
15+
want bool
16+
reason string
17+
}{
18+
{
19+
name: "nil user",
20+
user: nil,
21+
meta: &model.Meta{
22+
Path: "/folder",
23+
Hide: "secret",
24+
HSub: true,
25+
},
26+
path: "/folder",
27+
want: false,
28+
reason: "nil user (treated as admin) should not hide",
29+
},
30+
{
31+
name: "user with can_see_hides permission",
32+
user: &model.User{
33+
Role: model.GENERAL,
34+
Permission: 1, // bit 0 set = can see hides
35+
},
36+
meta: &model.Meta{
37+
Path: "/folder",
38+
Hide: "secret",
39+
HSub: true,
40+
},
41+
path: "/folder",
42+
want: false,
43+
reason: "user with can_see_hides permission should not hide",
44+
},
45+
{
46+
name: "nil meta",
47+
user: &model.User{
48+
Role: model.GUEST,
49+
},
50+
meta: nil,
51+
path: "/folder",
52+
want: false,
53+
reason: "nil meta should not hide",
54+
},
55+
{
56+
name: "empty hide string",
57+
user: &model.User{
58+
Role: model.GUEST,
59+
},
60+
meta: &model.Meta{
61+
Path: "/folder",
62+
Hide: "",
63+
HSub: true,
64+
},
65+
path: "/folder",
66+
want: false,
67+
reason: "empty hide string should not hide",
68+
},
69+
{
70+
name: "exact path match with HSub=false",
71+
user: &model.User{
72+
Role: model.GUEST,
73+
},
74+
meta: &model.Meta{
75+
Path: "/folder",
76+
Hide: "secret",
77+
HSub: false,
78+
},
79+
path: "/folder",
80+
want: true,
81+
reason: "exact path match should hide for guest",
82+
},
83+
{
84+
name: "sub path with HSub=true",
85+
user: &model.User{
86+
Role: model.GUEST,
87+
},
88+
meta: &model.Meta{
89+
Path: "/folder",
90+
Hide: "secret",
91+
HSub: true,
92+
},
93+
path: "/folder/subfolder",
94+
want: true,
95+
reason: "sub path with HSub=true should hide for guest",
96+
},
97+
{
98+
name: "sub path with HSub=false",
99+
user: &model.User{
100+
Role: model.GUEST,
101+
},
102+
meta: &model.Meta{
103+
Path: "/folder",
104+
Hide: "secret",
105+
HSub: false,
106+
},
107+
path: "/folder/subfolder",
108+
want: false,
109+
reason: "sub path with HSub=false should not hide",
110+
},
111+
{
112+
name: "non-sub path with HSub=true",
113+
user: &model.User{
114+
Role: model.GUEST,
115+
},
116+
meta: &model.Meta{
117+
Path: "/folder",
118+
Hide: "secret",
119+
HSub: true,
120+
},
121+
path: "/other",
122+
want: false,
123+
reason: "non-sub path should not hide even with HSub=true",
124+
},
125+
{
126+
name: "user without can_see_hides permission",
127+
user: &model.User{
128+
Role: model.GENERAL,
129+
Permission: 0, // bit 0 not set = cannot see hides
130+
},
131+
meta: &model.Meta{
132+
Path: "/folder",
133+
Hide: "secret",
134+
HSub: true,
135+
},
136+
path: "/folder",
137+
want: true,
138+
reason: "user without can_see_hides permission should hide",
139+
},
140+
}
141+
142+
for _, tt := range tests {
143+
t.Run(tt.name, func(t *testing.T) {
144+
got := whetherHide(tt.user, tt.meta, tt.path)
145+
if got != tt.want {
146+
t.Errorf("whetherHide() = %v, want %v\nReason: %s",
147+
got, tt.want, tt.reason)
148+
}
149+
})
150+
}
151+
}

‎internal/model/meta.go‎

Lines changed: 16 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -1,16 +1,20 @@
11
package model
22

33
type Meta struct {
4-
ID uint `json:"id" gorm:"primaryKey"`
5-
Path string `json:"path" gorm:"unique" binding:"required"`
6-
Password string `json:"password"`
7-
PSub bool `json:"p_sub"`
8-
Write bool `json:"write"`
9-
WSub bool `json:"w_sub"`
10-
Hide string `json:"hide"`
11-
HSub bool `json:"h_sub"`
12-
Readme string `json:"readme"`
13-
RSub bool `json:"r_sub"`
14-
Header string `json:"header"`
15-
HeaderSub bool `json:"header_sub"`
4+
ID uint `json:"id" gorm:"primaryKey"`
5+
Path string `json:"path" gorm:"unique" binding:"required"`
6+
ReadUsers []uint `json:"read_users" gorm:"serializer:json"`
7+
ReadUsersSub bool `json:"read_users_sub"`
8+
WriteUsers []uint `json:"write_users" gorm:"serializer:json"`
9+
WriteUsersSub bool `json:"write_users_sub"`
10+
Password string `json:"password"`
11+
PSub bool `json:"p_sub"`
12+
Write bool `json:"write"`
13+
WSub bool `json:"w_sub"`
14+
Hide string `json:"hide"`
15+
HSub bool `json:"h_sub"`
16+
Readme string `json:"readme"`
17+
RSub bool `json:"r_sub"`
18+
Header string `json:"header"`
19+
HeaderSub bool `json:"header_sub"`
1620
}

‎internal/model/user.go‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -123,12 +123,12 @@ func (u *User) CanAddOfflineDownloadTasks() bool {
123123
return CanAddOfflineDownloadTasks(u.Permission)
124124
}
125125

126-
func CanWrite(permission int32) bool {
126+
func CanWriteContent(permission int32) bool {
127127
return (permission>>3)&1 == 1
128128
}
129129

130-
func (u *User) CanWrite() bool {
131-
return CanWrite(u.Permission)
130+
func (u *User) CanWriteContent() bool {
131+
return CanWriteContent(u.Permission)
132132
}
133133

134134
func CanRename(permission int32) bool {

‎server/common/check.go‎

Lines changed: 34 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,7 @@ package common
22

33
import (
44
"path"
5+
"slices"
56
"strings"
67

78
"github.com/OpenListTeam/OpenList/v4/internal/conf"
@@ -17,31 +18,49 @@ func IsStorageSignEnabled(rawPath string) bool {
1718
return storage != nil && storage.GetStorage().EnableSign
1819
}
1920

20-
func CanWrite(meta *model.Meta, path string) bool {
21-
if meta == nil || !meta.Write {
21+
func CanRead(user *model.User, meta *model.Meta, path string) bool {
22+
// nil user is treated as internal/system context and bypasses per-user read restrictions
23+
if user == nil {
24+
return true
25+
}
26+
if meta != nil && len(meta.ReadUsers) > 0 && !slices.Contains(meta.ReadUsers, user.ID) && MetaCoversPath(meta.Path, path, meta.ReadUsersSub) {
2227
return false
2328
}
24-
return meta.WSub || meta.Path == path
29+
return true
2530
}
2631

27-
func IsApply(metaPath, reqPath string, applySub bool) bool {
28-
if utils.PathEqual(metaPath, reqPath) {
32+
func CanWrite(user *model.User, meta *model.Meta, path string) bool {
33+
// nil user is treated as internal/system context and bypasses per-user write restrictions
34+
if user == nil {
2935
return true
3036
}
31-
return utils.IsSubPath(metaPath, reqPath) && applySub
37+
if meta != nil && len(meta.WriteUsers) > 0 && !slices.Contains(meta.WriteUsers, user.ID) && MetaCoversPath(meta.Path, path, meta.WriteUsersSub) {
38+
return false
39+
}
40+
return true
41+
}
42+
43+
func CanWriteContentBypassUserPerms(meta *model.Meta, path string) bool {
44+
if meta == nil || !meta.Write {
45+
return false
46+
}
47+
return MetaCoversPath(meta.Path, path, meta.WSub)
3248
}
3349

3450
func CanAccess(user *model.User, meta *model.Meta, reqPath string, password string) bool {
3551
// if the reqPath is in hide (only can check the nearest meta) and user can't see hides, can't access
3652
if meta != nil && !user.CanSeeHides() && meta.Hide != "" &&
37-
IsApply(meta.Path, path.Dir(reqPath), meta.HSub) { // the meta should apply to the parent of current path
53+
MetaCoversPath(meta.Path, path.Dir(reqPath), meta.HSub) { // the meta should apply to the parent of current path
3854
for _, hide := range strings.Split(meta.Hide, "\n") {
3955
re := regexp2.MustCompile(hide, regexp2.None)
4056
if isMatch, _ := re.MatchString(path.Base(reqPath)); isMatch {
4157
return false
4258
}
4359
}
4460
}
61+
if !CanRead(user, meta, reqPath) {
62+
return false
63+
}
4564
// if is not guest and can access without password
4665
if user.CanAccessWithoutPassword() {
4766
return true
@@ -51,13 +70,20 @@ func CanAccess(user *model.User, meta *model.Meta, reqPath string, password stri
5170
return true
5271
}
5372
// if meta doesn't apply to sub_folder, can access
54-
if !utils.PathEqual(meta.Path, reqPath) && !meta.PSub {
73+
if !MetaCoversPath(meta.Path, reqPath, meta.PSub) {
5574
return true
5675
}
5776
// validate password
5877
return meta.Password == password
5978
}
6079

80+
func MetaCoversPath(metaPath, reqPath string, applyToSubFolder bool) bool {
81+
if utils.PathEqual(metaPath, reqPath) {
82+
return true
83+
}
84+
return utils.IsSubPath(metaPath, reqPath) && applyToSubFolder
85+
}
86+
6187
// ShouldProxy TODO need optimize
6288
// when should be proxy?
6389
// 1. config.MustProxy()

0 commit comments

Comments
 (0)