From bc3d75a94a4b921fb24e2ebc231fb35e4e14064f Mon Sep 17 00:00:00 2001
From: Henrique Dias <mail@hacdias.com>
Date: Sat, 25 Jul 2026 22:15:35 +0200
Subject: [PATCH] test(users): cover concurrent provisioning into one scope

Two usernames that normalize to the same home directory must not both be
provisioned when their requests are handled concurrently. Also cover that
an explicit, non-derived scope may still be legitimately shared.

Refs GHSA-j7jh-37pf-mf8h
---
 users/storage_test.go | 113 ++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 113 insertions(+)

diff --git a/users/storage_test.go b/users/storage_test.go
index 65d13de8..21a9e337 100644
--- a/users/storage_test.go
+++ b/users/storage_test.go
@@ -1,4 +1,117 @@
 package users
 
+import (
+	"errors"
+	"strings"
+	"sync"
+	"testing"
+	"time"
+
+	fberrors "github.com/filebrowser/filebrowser/v2/errors"
+)
+
 // Interface is implemented by storage
 var _ Store = &Storage{}
+
+// slowBackend is a minimal StorageBackend whose scope lookup is deliberately
+// slow, so that an unsynchronized check-then-save reliably interleaves.
+type slowBackend struct {
+	mu    sync.Mutex
+	users []*User
+	delay time.Duration
+}
+
+func (b *slowBackend) GetByScope(scope string) (*User, error) {
+	time.Sleep(b.delay)
+
+	b.mu.Lock()
+	defer b.mu.Unlock()
+	for _, u := range b.users {
+		if strings.EqualFold(u.Scope, scope) {
+			return u, nil
+		}
+	}
+	return nil, fberrors.ErrNotExist
+}
+
+func (b *slowBackend) Save(u *User) error {
+	b.mu.Lock()
+	defer b.mu.Unlock()
+	b.users = append(b.users, u)
+	return nil
+}
+
+func (b *slowBackend) GetBy(interface{}) (*User, error) { return nil, fberrors.ErrNotExist }
+func (b *slowBackend) Gets() ([]*User, error)           { return b.users, nil }
+func (b *slowBackend) Update(*User, ...string) error    { return nil }
+func (b *slowBackend) DeleteByID(uint) error            { return nil }
+func (b *slowBackend) DeleteByUsername(string) error    { return nil }
+func (b *slowBackend) CountAdmins() (int, error)        { return 0, nil }
+
+// Two usernames that normalize to the same home directory must not both be
+// provisioned, even when their requests are handled concurrently: checking the
+// scope and saving the user has to be a single atomic step.
+func TestSaveProvisionedRejectsConcurrentScopeCollision(t *testing.T) {
+	back := &slowBackend{delay: 50 * time.Millisecond}
+	store := NewStorage(back)
+
+	errs := make([]error, 2)
+	var wg, ready sync.WaitGroup
+	ready.Add(2)
+	for i, username := range []string{"teamone-x", "teamone/x"} {
+		wg.Add(1)
+		go func(i int, username string) {
+			defer wg.Done()
+			// Only start once both goroutines are actually running, so that an
+			// unsynchronized implementation is guaranteed to interleave rather
+			// than to pass by scheduling luck.
+			ready.Done()
+			ready.Wait()
+			errs[i] = store.SaveProvisioned(&User{
+				Username: username,
+				Password: "pw",
+				Scope:    "/users/teamone-x",
+			}, true)
+		}(i, username)
+	}
+	wg.Wait()
+
+	saved := 0
+	for _, err := range errs {
+		switch {
+		case err == nil:
+			saved++
+		case !errors.Is(err, fberrors.ErrExist):
+			t.Fatalf("unexpected error: %v", err)
+		}
+	}
+
+	if saved != 1 {
+		t.Fatalf("VULNERABLE: %d users provisioned into /users/teamone-x, want 1", saved)
+	}
+	if len(back.users) != 1 {
+		t.Fatalf("%d users stored, want 1", len(back.users))
+	}
+}
+
+// A scope that was not derived from the username (explicitly configured by an
+// administrator or returned by an auth hook) may legitimately be shared, so it
+// must not be subject to the collision check.
+func TestSaveProvisionedAllowsSharedExplicitScope(t *testing.T) {
+	back := &slowBackend{}
+	store := NewStorage(back)
+
+	for _, username := range []string{"alice", "bob"} {
+		if err := store.SaveProvisioned(&User{
+			Username: username,
+			Password: "pw",
+			Scope:    "/shared/team",
+		}, false); err != nil {
+			t.Fatalf("saving %q with an explicit shared scope: %v", username, err)
+		}
+	}
+
+	if len(back.users) != 2 {
+		t.Fatalf("%d users stored, want 2", len(back.users))
+	}
+}