From 8ddd3d1db9b9f5727d0bf96ea0e7d9a25a8692b4 Mon Sep 17 00:00:00 2001 From: Henrique Dias Date: Sat, 25 Jul 2026 07:55:12 +0200 Subject: [PATCH] fix(auth): isolate auto-provisioned proxy and hook users to their own home Proxy- and hook-authenticated users were auto-provisioned by applying the default scope (".") and passing it straight to MakeUserDir, which normalizes "." to "/". With CreateUserDir enabled, every provisioned user therefore received the server root as its scope instead of a per-user home directory, letting one user read, overwrite and delete another user's files. The signup handler already cleared the scope before deriving the home directory. Centralize that logic into Settings.CreateUserHome (clear the scope when CreateUserDir is on and no explicit scope was supplied, derive the home dir, then reject a scope already owned by another user) and use it from signup, proxy and hook auth so the three provisioning paths cannot diverge. Refs GHSA-j7jh-37pf-mf8h, GHSA-j2fc-28fx-hc8q --- auth/hook.go | 14 +++++++------- auth/proxy.go | 8 ++------ http/auth.go | 28 +++++----------------------- settings/dir.go | 36 ++++++++++++++++++++++++++++++++++++ 4 files changed, 50 insertions(+), 36 deletions(-) diff --git a/auth/hook.go b/auth/hook.go index a6dc25b8..e53768cb 100644 --- a/auth/hook.go +++ b/auth/hook.go @@ -157,15 +157,15 @@ func (a *HookAuth) SaveUser() (*users.User, error) { } u = a.GetUser(d) - userHome, err := a.Settings.MakeUserDir(u.Username, u.Scope, a.Server.Root) - if err != nil { - return nil, fmt.Errorf("user: failed to mkdir user home dir: [%s]", userHome) + // A scope explicitly returned by the hook takes precedence over the + // automatic per-user home directory derivation. + _, explicitScope := a.Fields.Values["user.scope"] + if err := a.Settings.CreateUserHome(u, a.Users, a.Server.Root, explicitScope); err != nil { + return nil, err } - u.Scope = userHome - log.Printf("user: %s, home dir: [%s].", u.Username, userHome) + log.Printf("user: %s, home dir: [%s].", u.Username, u.Scope) - err = a.Users.Save(u) - if err != nil { + if err := a.Users.Save(u); err != nil { return nil, err } } else if p := !users.CheckPwd(a.Cred.Password, u.Password); len(a.Fields.Values) > 1 || p { diff --git a/auth/proxy.go b/auth/proxy.go index ab6227d4..c655f8b6 100644 --- a/auth/proxy.go +++ b/auth/proxy.go @@ -50,15 +50,11 @@ func (a ProxyAuth) createUser(usr users.Store, setting *settings.Settings, srv * user.Perm.Execute = false user.Commands = []string{} - var userHome string - userHome, err = setting.MakeUserDir(user.Username, user.Scope, srv.Root) - if err != nil { + if err = setting.CreateUserHome(user, usr, srv.Root, false); err != nil { return nil, err } - user.Scope = userHome - err = usr.Save(user) - if err != nil { + if err = usr.Save(user); err != nil { return nil, err } diff --git a/http/auth.go b/http/auth.go index b96819b8..d4f31916 100644 --- a/http/auth.go +++ b/http/auth.go @@ -191,33 +191,15 @@ var signupHandler = func(w http.ResponseWriter, r *http.Request, d *data) (int, } user.Password = pwd - if d.settings.CreateUserDir { - user.Scope = "" - } - userHome, err := d.settings.MakeUserDir(user.Username, user.Scope, d.server.Root) - if err != nil { - log.Printf("create user: failed to mkdir user home dir: [%s]", userHome) + switch err := d.settings.CreateUserHome(user, d.store.Users, d.server.Root, false); { + case errors.Is(err, fberrors.ErrExist): + return http.StatusConflict, fberrors.ErrExist + case err != nil: return http.StatusInternalServerError, err } - user.Scope = userHome - // When home directories are created from the username, distinct usernames - // can normalize to the same scope (cleanUsername is many-to-one), which would - // silently hand the new user another user's home directory. Reject the signup - // if the derived scope is already taken. When CreateUserDir is off, all - // signups intentionally share the configured default scope, so this check - // does not apply. - if d.settings.CreateUserDir { - switch _, err := d.store.Users.GetByScope(user.Scope); { - case err == nil: - return http.StatusConflict, fberrors.ErrExist - case !errors.Is(err, fberrors.ErrNotExist): - return http.StatusInternalServerError, err - } - } - - log.Printf("new user: %s, home dir: [%s].", user.Username, userHome) + log.Printf("new user: %s, home dir: [%s].", user.Username, user.Scope) err = d.store.Users.Save(user) if errors.Is(err, fberrors.ErrExist) { diff --git a/settings/dir.go b/settings/dir.go index 0e857950..1259d24a 100644 --- a/settings/dir.go +++ b/settings/dir.go @@ -10,6 +10,9 @@ import ( "strings" "github.com/spf13/afero" + + fberrors "github.com/filebrowser/filebrowser/v2/errors" + "github.com/filebrowser/filebrowser/v2/users" ) var ( @@ -39,6 +42,39 @@ func (s *Settings) MakeUserDir(username, userScope, serverRoot string) (string, return userScope, nil } +// CreateUserHome derives and creates the home directory for a user that is +// being provisioned (via signup, proxy auth or hook auth) and sets user.Scope +// to the resulting path. When CreateUserDir is enabled and the caller did not +// supply an explicit scope, the scope is cleared so that MakeUserDir derives a +// per-user home from the username instead of falling back to the default scope +// (which normalizes to the server root, leaving every provisioned user sharing +// it). When a home directory is derived, it also rejects a scope already owned +// by another user, so that distinct usernames cannot silently share one home +// directory. +func (s *Settings) CreateUserHome(user *users.User, store users.Store, serverRoot string, explicitScope bool) error { + derived := s.CreateUserDir && !explicitScope + if derived { + user.Scope = "" + } + + userHome, err := s.MakeUserDir(user.Username, user.Scope, serverRoot) + if err != nil { + return err + } + user.Scope = userHome + + if derived { + switch _, err := store.GetByScope(user.Scope); { + case err == nil: + return fberrors.ErrExist + case !errors.Is(err, fberrors.ErrNotExist): + return err + } + } + + return nil +} + func cleanUsername(s string) string { // Remove any trailing space to avoid ending on - s = strings.Trim(s, " ")