From 43a404ca69bf25553bfbbb2b446f0f53077c6302 Mon Sep 17 00:00:00 2001 From: JinHyuk Sung <163989462+sjh9714@users.noreply.github.com> Date: Sat, 27 Jun 2026 14:43:22 +0900 Subject: [PATCH] fix: match admin share paths by owner scope (#5992) --- http/share.go | 30 +++++++++++- http/share_test.go | 110 ++++++++++++++++++++++++++++++++++++++++++ share/storage.go | 17 ------- share/storage_test.go | 85 -------------------------------- 4 files changed, 139 insertions(+), 103 deletions(-) create mode 100644 http/share_test.go delete mode 100644 share/storage_test.go diff --git a/http/share.go b/http/share.go index 7cd08faf..3d05fe05 100644 --- a/http/share.go +++ b/http/share.go @@ -7,6 +7,7 @@ import ( "errors" "fmt" "net/http" + "path/filepath" "sort" "strconv" "strings" @@ -14,6 +15,7 @@ import ( fberrors "github.com/filebrowser/filebrowser/v2/errors" "github.com/filebrowser/filebrowser/v2/share" + "github.com/filebrowser/filebrowser/v2/users" "golang.org/x/crypto/bcrypt" ) @@ -61,7 +63,7 @@ var shareGetsHandler = withPermShare(func(w http.ResponseWriter, r *http.Request err error ) if d.user.Perm.Admin { - s, err = d.store.Share.GetsByPath(r.URL.Path) + s, err = getSharesForAdminPath(d, r.URL.Path) } else { s, err = d.store.Share.Gets(r.URL.Path, d.user.ID) } @@ -76,6 +78,32 @@ var shareGetsHandler = withPermShare(func(w http.ResponseWriter, r *http.Request return renderJSON(w, r, s) }) +func getSharesForAdminPath(d *data, path string) ([]*share.Link, error) { + links, err := d.store.Share.All() + if err != nil { + return nil, err + } + + adminPath := filepath.Clean(d.user.FullPath(path)) + owners := make(map[uint]*users.User) + filtered := make([]*share.Link, 0, len(links)) + for _, link := range links { + owner, ok := owners[link.UserID] + if !ok { + owner, err = d.store.Users.Get(d.server.Root, d.server.FollowExternalSymlinks, link.UserID) + if err != nil && !errors.Is(err, fberrors.ErrNotExist) { + return nil, err + } + owners[link.UserID] = owner // owner is nil on ErrNotExist + } + if owner != nil && filepath.Clean(owner.FullPath(link.Path)) == adminPath { + filtered = append(filtered, link) + } + } + + return filtered, nil +} + var shareDeleteHandler = withPermShare(func(_ http.ResponseWriter, r *http.Request, d *data) (int, error) { hash := strings.TrimSuffix(r.URL.Path, "/") hash = strings.TrimPrefix(hash, "/") diff --git a/http/share_test.go b/http/share_test.go new file mode 100644 index 00000000..8029acc5 --- /dev/null +++ b/http/share_test.go @@ -0,0 +1,110 @@ +package fbhttp + +import ( + "encoding/json" + "net/http" + "net/http/httptest" + "os" + "path/filepath" + "testing" + "time" + + "github.com/asdine/storm/v3" + "github.com/golang-jwt/jwt/v5" + + "github.com/filebrowser/filebrowser/v2/settings" + "github.com/filebrowser/filebrowser/v2/share" + "github.com/filebrowser/filebrowser/v2/storage/bolt" + "github.com/filebrowser/filebrowser/v2/users" +) + +func TestAdminShareGetsHandlerMatchesOwnerScope(t *testing.T) { + t.Parallel() + + root := t.TempDir() + ownerScope := filepath.Join(root, "owner") + if err := os.MkdirAll(ownerScope, 0o755); err != nil { + t.Fatal(err) + } + if err := os.WriteFile(filepath.Join(ownerScope, "file.txt"), []byte("shared"), 0o600); err != nil { + t.Fatal(err) + } + + db, err := storm.Open(filepath.Join(t.TempDir(), "db")) + if err != nil { + t.Fatalf("failed to open db: %v", err) + } + t.Cleanup(func() { _ = db.Close() }) + + st, err := bolt.NewStorage(db) + if err != nil { + t.Fatalf("failed to get storage: %v", err) + } + + owner := &users.User{ + Username: "owner", + Password: "pw", + Scope: "/owner", + Perm: users.Permissions{Share: true, Download: true}, + } + if err := st.Users.Save(owner); err != nil { + t.Fatalf("failed to save owner: %v", err) + } + + adminPerm := users.Permissions{Admin: true, Share: true, Download: true} + admin := &users.User{ + Username: "admin", + Password: "pw", + Scope: "/", + Perm: adminPerm, + } + if err := st.Users.Save(admin); err != nil { + t.Fatalf("failed to save admin: %v", err) + } + + if err := st.Share.Save(&share.Link{Hash: "h", UserID: owner.ID, Path: "/file.txt"}); err != nil { + t.Fatalf("failed to save share: %v", err) + } + key := []byte("test-signing-key") + if err := st.Settings.Save(&settings.Settings{Key: key}); err != nil { + t.Fatalf("failed to save settings: %v", err) + } + + req, err := http.NewRequest(http.MethodGet, "/owner/file.txt", http.NoBody) + if err != nil { + t.Fatalf("failed to construct request: %v", err) + } + req.Header.Set("X-Auth", signShareTestToken(t, admin.ID, admin.Username, adminPerm, key)) + + rec := httptest.NewRecorder() + handle(shareGetsHandler, "", st, &settings.Server{Root: root}).ServeHTTP(rec, req) + + if rec.Code != http.StatusOK { + t.Fatalf("expected status 200, got %d: %s", rec.Code, rec.Body.String()) + } + + var links []*share.Link + if err := json.Unmarshal(rec.Body.Bytes(), &links); err != nil { + t.Fatalf("failed to decode response: %v", err) + } + if len(links) != 1 || links[0].Hash != "h" { + t.Fatalf("expected admin to see owner share h, got %#v", links) + } +} + +func signShareTestToken(t *testing.T, id uint, username string, perm users.Permissions, key []byte) string { + t.Helper() + + claims := &authToken{ + User: userInfo{ID: id, Username: username, Perm: perm}, + RegisteredClaims: jwt.RegisteredClaims{ + IssuedAt: jwt.NewNumericDate(time.Now().Add(-time.Minute)), + ExpiresAt: jwt.NewNumericDate(time.Now().Add(time.Hour)), + }, + } + signed, err := jwt.NewWithClaims(jwt.SigningMethodHS256, claims).SignedString(key) + if err != nil { + t.Fatalf("failed to sign token: %v", err) + } + return signed +} diff --git a/share/storage.go b/share/storage.go index 684ab018..3b73ef35 100644 --- a/share/storage.go +++ b/share/storage.go @@ -110,23 +110,6 @@ func (s *Storage) Gets(path string, id uint) ([]*Link, error) { return links, nil } -// GetsByPath returns all non-expired links for a path. -func (s *Storage) GetsByPath(path string) ([]*Link, error) { - links, err := s.All() - if err != nil { - return nil, err - } - - filtered := make([]*Link, 0, len(links)) - for _, link := range links { - if link.Path == path { - filtered = append(filtered, link) - } - } - - return filtered, nil -} - // Save wraps a StorageBackend.Save func (s *Storage) Save(l *Link) error { return s.back.Save(l) diff --git a/share/storage_test.go b/share/storage_test.go deleted file mode 100644 index dd78fe71..00000000 --- a/share/storage_test.go +++ /dev/null @@ -1,85 +0,0 @@ -package share - -import ( - "reflect" - "testing" -) - -type fakeBackend struct { - links []*Link -} - -func (f fakeBackend) All() ([]*Link, error) { - return f.links, nil -} - -func (f fakeBackend) FindByUserID(id uint) ([]*Link, error) { - var links []*Link - for _, link := range f.links { - if link.UserID == id { - links = append(links, link) - } - } - return links, nil -} - -func (f fakeBackend) GetByHash(hash string) (*Link, error) { - for _, link := range f.links { - if link.Hash == hash { - return link, nil - } - } - return nil, nil -} - -func (f fakeBackend) GetPermanent(path string, id uint) (*Link, error) { - for _, link := range f.links { - if link.Path == path && link.UserID == id && link.Expire == 0 { - return link, nil - } - } - return nil, nil -} - -func (f fakeBackend) Gets(path string, id uint) ([]*Link, error) { - var links []*Link - for _, link := range f.links { - if link.Path == path && link.UserID == id { - links = append(links, link) - } - } - return links, nil -} - -func (f fakeBackend) Save(_ *Link) error { - return nil -} - -func (f fakeBackend) Delete(_ string) error { - return nil -} - -func (f fakeBackend) DeleteWithPathPrefix(_ string, _ uint) error { - return nil -} - -func TestGetsByPathReturnsLinksFromAllUsers(t *testing.T) { - t.Parallel() - - expected := []*Link{ - {Hash: "a", Path: "/file.txt", UserID: 1}, - {Hash: "b", Path: "/file.txt", UserID: 2}, - } - store := NewStorage(fakeBackend{ - links: append(expected, &Link{Hash: "c", Path: "/other.txt", UserID: 3}), - }) - - links, err := store.GetsByPath("/file.txt") - if err != nil { - t.Fatalf("GetsByPath returned error: %v", err) - } - - if !reflect.DeepEqual(links, expected) { - t.Fatalf("GetsByPath returned %#v, want %#v", links, expected) - } -}