fix: match admin share paths by owner scope (#5992)

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, "/")

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

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)

share/storage_test.go

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