feat(permissions): implement fine-grained permission control#2145
feat(permissions): implement fine-grained permission control#2145Lanfei wants to merge 7 commits intoOpenListTeam:mainfrom
Conversation
0f8b0ea to
da2e92e
Compare
da2e92e to
127b445
Compare
127b445 to
d880b18
Compare
|
Hi @xrgzs @PIKACHUIM, this PR has been open for about 3 weeks now. CI is all green and there are no merge conflicts. The companion frontend PR (OpenList-Frontend#386) and docs PR (OpenList-Docs#310) are also ready. Could you help schedule a review? Happy to make any adjustments based on your feedback. Thanks! |
There was a problem hiding this comment.
Pull request overview
This PR introduces meta-level, per-user read/write allowlists and wires the new permission checks through core file operations (HTTP handlers, FTP, WebDAV), including filtering list results by read permission and expanding automated test coverage for the new permission logic.
Changes:
- Extend
MetawithReadUsers/WriteUsers(+ sub-path inheritance flags) and addcommon.CanRead/common.CanWritechecks. - Apply permission checks across file operations (list/read/write/manage) including WebDAV/FTP and list filtering.
- Add/expand tests for meta path coverage and permission combinations.
Reviewed changes
Copilot reviewed 20 out of 20 changed files in this pull request and generated 8 comments.
Show a summary per file
| File | Description |
|---|---|
internal/model/meta.go |
Adds per-user read/write allowlists to Meta (JSON-serialized). |
server/common/check.go |
Introduces CanRead/CanWrite, refactors path coverage helper(s). |
internal/fs/list.go |
Filters directory listings based on per-user read permission. |
server/handles/fsread.go |
Adjusts list response fields and write/tool availability logic. |
server/handles/fsmanage.go |
Adds parent-meta permission checks to manage operations. |
server/webdav.go |
Updates WebDAV auth/middleware context values for permission checks. |
server/webdav/webdav.go |
Adds CanAccess/CanWrite enforcement for WebDAV operations. |
server/webdav/file.go |
Adds permission gates to WebDAV copy/move helpers. |
server/middlewares/fsup.go |
Refactors upload middleware permission checks. |
server/ftp/fsup.go / server/ftp/fsmanage.go / server/ftp/fsread.go |
Adds/adjusts FTP permission checks using new helpers. |
server/handles/archive.go / server/handles/offline_download.go |
Adds meta permission checks for archive/decompress and offline download creation. |
server/common/check_test.go / internal/fs/list_test.go / server/handles/fsread_test.go |
Adds/expands tests for the new permission model and helpers. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
You can also share your feedback on Copilot code review. Take the survey.
- 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>
…ite 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>
…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>
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>
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>
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>
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>
9dc7cf7 to
814e995
Compare
|
@xrgzs @PIKACHUIM All Copilot review comments have been addressed. Could you please take a look when you have a chance? Thanks! |
|
@Lanfei Can it smoothly migrate from the old version to the new version? If not, please refine the migration code. |
|
@xrgzs Yes, migration from older versions is smooth — no manual migration patch is needed. GORM's For existing rows, In short: upgrading applies additive schema changes only, and the zero-value semantics preserve backward-compatible behavior for all existing data. |
Description / 描述
This PR implements a fine-grained, per-user read/write permission system at the meta level, allowing administrators to restrict specific users from accessing or modifying
certain paths. It also refactors existing permission check functions for clarity and adds comprehensive test coverage.
本 PR 在 meta
层面实现了细粒度的用户读/写权限系统,允许管理员限制特定用户对某些路径的访问或修改权限。同时对现有权限检查函数进行了重命名重构以提升可读性,并添加了完整的测试覆盖。
Key changes / 主要变更:
ReadUsers/WriteUsersfields to theMetamodel with sub-directory inheritance flags在
Meta模型中新增ReadUsers/WriteUsers字段,支持子目录继承标志CanReadandCanWritepermission check functions inserver/common在
server/common中实现CanRead和CanWrite权限检查函数根据用户读权限对文件列表结果进行过滤
在所有文件操作(FTP、HTTP 处理器、WebDAV)中添加权限检查
User.CanWrite()→User.CanCreateFilesOrFolders(),common.CanWrite()→common.CanWriteContentBypassUserPerms(),common.IsApply()→common.MetaCoversPath()重命名函数以提升可读性:
User.CanWrite()→User.CanCreateFilesOrFolders(),common.CanWrite()→common.CanWriteContentBypassUserPerms(),common.IsApply()→common.MetaCoversPath()Motivation and Context / 背景
The existing permission system only supports coarse-grained flags (e.g., global write permission). There was no way to restrict specific users from reading or writing to
particular paths without affecting all users. This PR addresses that gap by introducing a per-user allowlist at the meta level.
现有权限系统仅支持粗粒度的标志位(如全局写权限),无法在不影响其他用户的情况下限制特定用户对特定路径的读写访问。本 PR 通过在 meta 层面引入基于用户的白名单机制来填补这一空白。
Relates to #1328
Closes #1257
Closes #1267
Closes #346
Closes #1753
How Has This Been Tested? / 测试
TestCanRead,TestCanWrite,TestCanAccessWithReadPermissions, andTestWritePermissionCombinationsto validate the three-layer permission system新增
TestCanRead、TestCanWrite、TestCanAccessWithReadPermissions及TestWritePermissionCombinations,验证三层权限体系测试场景覆盖:nil 用户/meta、子路径继承、用户白名单及根路径限制
MetaCoversPath,CanWriteContentBypassUserPerms,getReadme,getHeader,isEncrypt, andwhetherHide为
MetaCoversPath、CanWriteContentBypassUserPerms、getReadme、getHeader、isEncrypt、whetherHide新增 43 个测试场景list_test.goandfsread_test.gofor coverage of list filtering and read handler logic新增
list_test.go和fsread_test.go,覆盖列表过滤与读取处理器逻辑Checklist / 检查清单
我已阅读 CONTRIBUTING 文档。
go fmtor prettier.我已使用
go fmt或 prettier 格式化提交的代码。我已为此 PR 添加了适当的标签(如无权限或需要的标签不存在,请在描述中说明,管理员将后续处理)。
我已在适当情况下使用"Request review"功能请求相关代码作者进行审查。
我已相应更新了相关仓库(若适用)。