diff --git a/API Server/internal/pb/client.go b/API Server/internal/pb/client.go index 3ed0544..fe22fe4 100644 --- a/API Server/internal/pb/client.go +++ b/API Server/internal/pb/client.go @@ -134,9 +134,34 @@ func (c *Client) currentToken() string { return c.token } -// do executes a request, attaching the auth token. On a 401 it re-authenticates -// once and retries. +// ensureToken acquires a superuser token when none is cached yet, so that a +// superuser call never goes out unauthenticated. +// +// It matters because PocketBase answers a record read it will not allow with +// 404, not 401 — it hides the record rather than refusing the credentials. So a +// request sent with no Authorization header comes back "The requested resource +// wasn't found", the 401 retries below never fire, and the caller is told the +// record is missing when the truth is that this client never signed in. The +// cache is empty in exactly the cases that matter: a startup where Authenticate +// failed because PocketBase wasn't up yet (deliberately non-fatal), and a +// Reconfigure, which clears the token so the new credentials are used. +// +// With no service account configured there is nothing to acquire, so the call +// proceeds as before — the endpoints that need superuser access answer 503 on +// their own. +func (c *Client) ensureToken(ctx context.Context) error { + if c.currentToken() != "" || !c.Configured() { + return nil + } + return c.Authenticate(ctx) +} + +// do executes a request, attaching the auth token — signing in first if there +// isn't one yet. On a 401 it re-authenticates once and retries. func (c *Client) do(ctx context.Context, method, path string, payload any) ([]byte, error) { + if err := c.ensureToken(ctx); err != nil { + return nil, err + } raw, status, err := c.attempt(ctx, method, path, payload) if err != nil { return nil, err @@ -191,6 +216,9 @@ func (c *Client) attempt(ctx context.Context, method, path string, payload any) // PocketBase's own validation errors to the client verbatim (user/organization // management) use this; handlers that want Go errors use the typed CRUD helpers. func (c *Client) Raw(ctx context.Context, method, path string, payload any) ([]byte, int, error) { + if err := c.ensureToken(ctx); err != nil { + return nil, 0, err + } raw, status, err := c.attempt(ctx, method, path, payload) if err != nil { return nil, 0, err @@ -396,6 +424,9 @@ func (c *Client) Delete(ctx context.Context, collection, id string) error { // superuser (this project's collections have no public access rules). func (c *Client) GetFile(ctx context.Context, collection, recordID, filename string) ([]byte, string, error) { path := "/api/files/" + collection + "/" + recordID + "/" + filename + if err := c.ensureToken(ctx); err != nil { + return nil, "", err + } raw, ctype, status, err := c.attemptGetFile(ctx, path) if err != nil { return nil, "", err @@ -448,6 +479,9 @@ func (c *Client) RequestVerification(ctx context.Context, collection, email stri // fields (e.g. the users.avatar upload), which PocketBase doesn't accept as // plain JSON. Pass fileField == "" to send only the plain fields. func (c *Client) UpdateMultipart(ctx context.Context, collection, id string, fields map[string]string, fileField, filename string, fileContent []byte) error { + if err := c.ensureToken(ctx); err != nil { + return err + } status, err := c.attemptMultipart(ctx, collection, id, fields, fileField, filename, fileContent) if status == http.StatusUnauthorized { if aerr := c.Authenticate(ctx); aerr != nil { diff --git a/API Server/internal/pb/client_test.go b/API Server/internal/pb/client_test.go new file mode 100644 index 0000000..10ef848 --- /dev/null +++ b/API Server/internal/pb/client_test.go @@ -0,0 +1,192 @@ +package pb + +import ( + "context" + "encoding/json" + "net/http" + "net/http/httptest" + "sync" + "testing" +) + +// PocketBase hides a record it will not let you read behind a 404 rather than a +// 401. A superuser call that goes out with no token therefore comes back +// "not found", and the client's 401 retry never fires — so these tests pin down +// that the client signs in *before* its first request whenever it holds no +// token: after a startup where the up-front Authenticate failed, and after a +// Reconfigure onto new credentials. + +const ( + fakeSvcToken = "svc-token" + fakeRecordID = "rec1" +) + +// fakePB stands in for PocketBase: it serves one record, but only to a caller +// carrying the superuser token, and records how it was signed into. +type fakePB struct { + mu sync.Mutex + + // Credentials that will be accepted. Everything else gets a 400, as + // PocketBase gives for a bad identity/password. + email string + password string + + authCalls int + lastIdentity string + guestAttempts int // record reads that arrived without the superuser token +} + +func (f *fakePB) handler() http.Handler { + mux := http.NewServeMux() + + mux.HandleFunc("POST /api/collections/_superusers/auth-with-password", func(w http.ResponseWriter, r *http.Request) { + var body struct{ Identity, Password string } + _ = json.NewDecoder(r.Body).Decode(&body) + f.mu.Lock() + f.authCalls++ + f.lastIdentity = body.Identity + ok := body.Identity == f.email && body.Password == f.password + f.mu.Unlock() + if !ok { + writeTestJSON(w, 400, map[string]any{"message": "Failed to authenticate."}) + return + } + writeTestJSON(w, 200, map[string]any{"token": fakeSvcToken}) + }) + + mux.HandleFunc("GET /api/collections/users/records/{id}", func(w http.ResponseWriter, r *http.Request) { + if r.Header.Get("Authorization") != fakeSvcToken { + f.mu.Lock() + f.guestAttempts++ + f.mu.Unlock() + writeTestJSON(w, 404, map[string]any{ + "data": map[string]any{}, "message": "The requested resource wasn't found.", "status": 404, + }) + return + } + writeTestJSON(w, 200, map[string]any{"id": r.PathValue("id"), "email": "driver@test.local"}) + }) + + return mux +} + +func writeTestJSON(w http.ResponseWriter, status int, body any) { + w.Header().Set("Content-Type", "application/json") + w.WriteHeader(status) + _ = json.NewEncoder(w).Encode(body) +} + +func newFakePB(t *testing.T, f *fakePB) string { + t.Helper() + srv := httptest.NewServer(f.handler()) + t.Cleanup(srv.Close) + return srv.URL +} + +func (f *fakePB) counts() (auth, guest int) { + f.mu.Lock() + defer f.mu.Unlock() + return f.authCalls, f.guestAttempts +} + +// A client that never managed to authenticate at startup must sign in on its +// first call rather than sending it as a guest and reporting PocketBase's 404. +func TestGetOneAuthenticatesWhenNoTokenCached(t *testing.T) { + f := &fakePB{email: "admin@test.local", password: "pw"} + c := New(newFakePB(t, f), f.email, f.password) + + var rec struct{ ID, Email string } + if err := c.GetOne(context.Background(), "users", fakeRecordID, &rec); err != nil { + t.Fatalf("GetOne: %v", err) + } + if rec.ID != fakeRecordID { + t.Fatalf("record id = %q, want %q", rec.ID, fakeRecordID) + } + + auth, guest := f.counts() + if guest != 0 { + t.Errorf("%d record read(s) went out unauthenticated, want 0", guest) + } + if auth != 1 { + t.Errorf("authenticated %d time(s), want 1", auth) + } +} + +// The token is cached: a second call reuses it instead of signing in again. +func TestGetOneReusesCachedToken(t *testing.T) { + f := &fakePB{email: "admin@test.local", password: "pw"} + c := New(newFakePB(t, f), f.email, f.password) + + for i := 0; i < 3; i++ { + if err := c.GetOne(context.Background(), "users", fakeRecordID, new(struct{})); err != nil { + t.Fatalf("GetOne #%d: %v", i, err) + } + } + if auth, _ := f.counts(); auth != 1 { + t.Errorf("authenticated %d time(s), want 1", auth) + } +} + +// Retargeting from the panel clears the cached token, so the next call has to +// sign in again — under the new credentials. +func TestReconfigureReauthenticates(t *testing.T) { + f := &fakePB{email: "admin@test.local", password: "pw"} + url := newFakePB(t, f) + c := New(url, f.email, f.password) + + if err := c.GetOne(context.Background(), "users", fakeRecordID, new(struct{})); err != nil { + t.Fatalf("GetOne before reconfigure: %v", err) + } + + f.mu.Lock() + f.email, f.password = "new@test.local", "pw2" + f.mu.Unlock() + c.Reconfigure(url, "new@test.local", "pw2") + + if err := c.GetOne(context.Background(), "users", fakeRecordID, new(struct{})); err != nil { + t.Fatalf("GetOne after reconfigure: %v", err) + } + + auth, guest := f.counts() + if guest != 0 { + t.Errorf("%d record read(s) went out unauthenticated, want 0", guest) + } + if auth != 2 { + t.Errorf("authenticated %d time(s), want 2", auth) + } + f.mu.Lock() + defer f.mu.Unlock() + if f.lastIdentity != "new@test.local" { + t.Errorf("signed in as %q, want the reconfigured account", f.lastIdentity) + } +} + +// Bad credentials must surface as the authentication failure they are, not as +// PocketBase's 404 for a record the client was never allowed to see. +func TestGetOneReportsAuthFailureNotNotFound(t *testing.T) { + f := &fakePB{email: "admin@test.local", password: "pw"} + c := New(newFakePB(t, f), "admin@test.local", "wrong") + + err := c.GetOne(context.Background(), "users", fakeRecordID, new(struct{})) + if err == nil { + t.Fatal("GetOne succeeded with bad credentials") + } + if apiErr, ok := err.(*APIError); ok && apiErr.Status == http.StatusNotFound { + t.Fatalf("got PocketBase's 404 instead of an auth failure: %v", err) + } +} + +// With no service account configured there is nothing to sign in with, so the +// call goes out as before — the endpoints that need superuser access answer 503 +// on their own rather than relying on this layer. +func TestNoServiceAccountSkipsAuthentication(t *testing.T) { + f := &fakePB{email: "admin@test.local", password: "pw"} + c := New(newFakePB(t, f), "", "") + + if err := c.GetOne(context.Background(), "users", fakeRecordID, new(struct{})); err == nil { + t.Fatal("GetOne succeeded without a service account") + } + if auth, _ := f.counts(); auth != 0 { + t.Errorf("attempted %d sign-in(s) with no credentials, want 0", auth) + } +}