From cd5749dff8bbc2c16f99b7a127651a9d8e0da694 Mon Sep 17 00:00:00 2001 From: Henrique Dias Date: Sat, 25 Jul 2026 08:09:48 +0200 Subject: [PATCH] test(auth): cover per-user scope isolation for proxy and hook provisioning Unit tests for Settings.CreateUserHome (derives a per-user home, rejects a colliding scope, preserves an explicit scope) plus proxy and hook regression tests asserting that with CreateUserDir enabled two provisioned users receive distinct home directories instead of the server root. Refs GHSA-j7jh-37pf-mf8h, GHSA-j2fc-28fx-hc8q --- auth/hook_test.go | 68 ++++++++++++++++++++++++++++++++++++++++ auth/proxy_test.go | 43 +++++++++++++++++++++++++ settings/dir_test.go | 74 ++++++++++++++++++++++++++++++++++++++++++++ 3 files changed, 185 insertions(+) create mode 100644 settings/dir_test.go diff --git a/auth/hook_test.go b/auth/hook_test.go index 4b0112b1..10819759 100644 --- a/auth/hook_test.go +++ b/auth/hook_test.go @@ -5,6 +5,9 @@ import ( "path/filepath" "runtime" "testing" + + "github.com/filebrowser/filebrowser/v2/settings" + "github.com/filebrowser/filebrowser/v2/users" ) // writeHookScript writes a POSIX shell script to a temp file and returns its @@ -86,3 +89,68 @@ fi t.Fatalf("expected action %q, got %q", "auth", action) } } + +// newHookAuth builds a HookAuth for a freshly provisioned user with the given +// parsed hook fields (hook.action=auth is added automatically). +func newHookAuth(store *mockUserStore, s *settings.Settings, srv *settings.Server, username string, fields map[string]string) *HookAuth { + fields["hook.action"] = "auth" + return &HookAuth{ + Users: store, + Settings: s, + Server: srv, + Cred: hookCred{Username: username, Password: "a-strong-password"}, + Fields: hookFields{Values: fields}, + } +} + +// With CreateUserDir enabled and no explicit scope from the hook, a provisioned +// hook user must receive its own home directory rather than the server root. +func TestHookSaveUserCreateUserDirIsolatesScope(t *testing.T) { + t.Parallel() + + store := &mockUserStore{users: make(map[string]*users.User)} + srv := &settings.Server{Root: t.TempDir()} + s := &settings.Settings{ + Key: []byte("key"), + CreateUserDir: true, + UserHomeBasePath: "/users", + Defaults: settings.UserDefaults{ + Scope: ".", + Perm: users.Permissions{Create: true}, + }, + } + + u, err := newHookAuth(store, s, srv, "alice", map[string]string{}).SaveUser() + if err != nil { + t.Fatalf("SaveUser error: %v", err) + } + if u.Scope != "/users/alice" { + t.Errorf("hook user without explicit scope: expected /users/alice, got %q", u.Scope) + } +} + +// A scope explicitly returned by the hook takes precedence over the automatic +// per-user home directory derivation. +func TestHookSaveUserRespectsExplicitScope(t *testing.T) { + t.Parallel() + + store := &mockUserStore{users: make(map[string]*users.User)} + srv := &settings.Server{Root: t.TempDir()} + s := &settings.Settings{ + Key: []byte("key"), + CreateUserDir: true, + UserHomeBasePath: "/users", + Defaults: settings.UserDefaults{ + Scope: ".", + Perm: users.Permissions{Create: true}, + }, + } + + u, err := newHookAuth(store, s, srv, "teamlead", map[string]string{"user.scope": "/shared/team"}).SaveUser() + if err != nil { + t.Fatalf("SaveUser error: %v", err) + } + if u.Scope != "/shared/team" { + t.Errorf("explicit hook scope should win, got %q", u.Scope) + } +} diff --git a/auth/proxy_test.go b/auth/proxy_test.go index 9b9ef3a7..233e72c8 100644 --- a/auth/proxy_test.go +++ b/auth/proxy_test.go @@ -78,3 +78,46 @@ func TestProxyAuthCreateUserRestrictsDefaults(t *testing.T) { t.Error("auto-provisioned proxy user should retain Create permission from defaults") } } + +// With CreateUserDir enabled, two distinct proxy-authenticated users must each +// receive their own home directory instead of both inheriting the server root. +func TestProxyAuthCreateUserDirIsolatesScope(t *testing.T) { + t.Parallel() + + store := &mockUserStore{users: make(map[string]*users.User)} + srv := &settings.Server{Root: t.TempDir()} + s := &settings.Settings{ + Key: []byte("key"), + AuthMethod: MethodProxyAuth, + CreateUserDir: true, + UserHomeBasePath: "/users", + Defaults: settings.UserDefaults{ + Scope: ".", + Perm: users.Permissions{Create: true}, + }, + } + + auth := ProxyAuth{Header: "X-Remote-User"} + provision := func(name string) *users.User { + req, _ := http.NewRequest(http.MethodGet, "/", http.NoBody) + req.Header.Set("X-Remote-User", name) + u, err := auth.Auth(req, store, s, srv) + if err != nil { + t.Fatalf("Auth(%q) error: %v", name, err) + } + return u + } + + alice := provision("alice") + bob := provision("bob") + + if alice.Scope == "/" || bob.Scope == "/" { + t.Fatalf("provisioned users inherited the server root: alice=%q bob=%q", alice.Scope, bob.Scope) + } + if alice.Scope == bob.Scope { + t.Fatalf("distinct users must get distinct scopes, both got %q", alice.Scope) + } + if alice.Scope != "/users/alice" { + t.Errorf("expected /users/alice, got %q", alice.Scope) + } +} diff --git a/settings/dir_test.go b/settings/dir_test.go new file mode 100644 index 00000000..0be07e9f --- /dev/null +++ b/settings/dir_test.go @@ -0,0 +1,74 @@ +package settings + +import ( + "errors" + "testing" + + fberrors "github.com/filebrowser/filebrowser/v2/errors" + "github.com/filebrowser/filebrowser/v2/users" +) + +// stubStore is a minimal users.Store used to exercise CreateUserHome. +type stubStore struct { + byScope map[string]*users.User +} + +func (s *stubStore) Get(_ string, _ bool, _ interface{}) (*users.User, error) { + return nil, fberrors.ErrNotExist +} + +func (s *stubStore) GetByScope(scope string) (*users.User, error) { + if u, ok := s.byScope[scope]; ok { + return u, nil + } + return nil, fberrors.ErrNotExist +} + +func (s *stubStore) Gets(_ string, _ bool) ([]*users.User, error) { return nil, nil } +func (s *stubStore) Update(_ *users.User, _ ...string) error { return nil } +func (s *stubStore) Save(_ *users.User) error { return nil } +func (s *stubStore) Delete(_ interface{}) error { return nil } +func (s *stubStore) LastUpdate(_ uint) int64 { return 0 } + +// A user provisioned with CreateUserDir must receive a per-user home directory +// derived from its username, not the default scope which normalizes to the +// server root. +func TestCreateUserHomeDerivesPerUserScope(t *testing.T) { + s := &Settings{CreateUserDir: true, UserHomeBasePath: "/users"} + store := &stubStore{byScope: map[string]*users.User{}} + + user := &users.User{Username: "alice", Scope: "."} + if err := s.CreateUserHome(user, store, t.TempDir(), false); err != nil { + t.Fatalf("unexpected error: %v", err) + } + if user.Scope != "/users/alice" { + t.Errorf("expected derived scope /users/alice, got %q", user.Scope) + } +} + +// When the derived scope is already owned by another user, provisioning must be +// rejected so distinct usernames cannot silently share one home directory. +func TestCreateUserHomeRejectsCollision(t *testing.T) { + s := &Settings{CreateUserDir: true, UserHomeBasePath: "/users"} + store := &stubStore{byScope: map[string]*users.User{"/users/alice": {Username: "alice"}}} + + user := &users.User{Username: "alice", Scope: "."} + if err := s.CreateUserHome(user, store, t.TempDir(), false); !errors.Is(err, fberrors.ErrExist) { + t.Fatalf("expected ErrExist on scope collision, got %v", err) + } +} + +// A scope explicitly supplied by the caller (e.g. returned by an auth hook) must +// be preserved instead of being replaced by a derived home directory. +func TestCreateUserHomePreservesExplicitScope(t *testing.T) { + s := &Settings{CreateUserDir: true, UserHomeBasePath: "/users"} + store := &stubStore{byScope: map[string]*users.User{"/users/alice": {Username: "alice"}}} + + user := &users.User{Username: "alice", Scope: "/custom"} + if err := s.CreateUserHome(user, store, t.TempDir(), true); err != nil { + t.Fatalf("unexpected error: %v", err) + } + if user.Scope != "/custom" { + t.Errorf("explicit scope should be preserved, got %q", user.Scope) + } +}