diff --git a/server/internal/api/api.go b/server/internal/api/api.go index 16c6c77..cc7cffd 100644 --- a/server/internal/api/api.go +++ b/server/internal/api/api.go @@ -300,68 +300,68 @@ func (s *Server) Routes() *http.ServeMux { s.authed(s.handleRevokeOtherSessions)) // --- the people who work here --- - mux.HandleFunc("GET /api/team", s.authed(s.handleTeam)) - mux.HandleFunc("PATCH /api/team/{id}", s.authed(s.handleUpdateTeamMember)) - mux.HandleFunc("POST /api/team/members", s.authed(s.handleCreateMember)) - mux.HandleFunc("POST /api/team/{id}/password", s.authed(s.handleResetPassword)) - mux.HandleFunc("GET /api/team/invitations", s.authed(s.handleInvitations)) - mux.HandleFunc("POST /api/team/invitations", s.authed(s.handleInvite)) + mux.HandleFunc("GET /api/team", s.tenantOnly(s.handleTeam)) + mux.HandleFunc("PATCH /api/team/{id}", s.tenantOnly(s.handleUpdateTeamMember)) + mux.HandleFunc("POST /api/team/members", s.tenantOnly(s.handleCreateMember)) + mux.HandleFunc("POST /api/team/{id}/password", s.tenantOnly(s.handleResetPassword)) + mux.HandleFunc("GET /api/team/invitations", s.tenantOnly(s.handleInvitations)) + mux.HandleFunc("POST /api/team/invitations", s.tenantOnly(s.handleInvite)) mux.HandleFunc("DELETE /api/team/invitations/{id}", - s.authed(s.handleRevokeInvitation)) + s.tenantOnly(s.handleRevokeInvitation)) - mux.HandleFunc("GET /api/reports/footfall", s.authed(s.handleFootfall)) - mux.HandleFunc("GET /api/reports/conversion", s.authed(s.handleConversion)) - mux.HandleFunc("GET /api/sites", s.authed(s.handleSites)) - mux.HandleFunc("POST /api/sites", s.authed(s.handleCreateSite)) - mux.HandleFunc("DELETE /api/sites/{site}", s.authed(s.handleDeleteSite)) - mux.HandleFunc("PATCH /api/sites/{site}", s.authed(s.handleUpdateSite)) + mux.HandleFunc("GET /api/reports/footfall", s.tenantOnly(s.handleFootfall)) + mux.HandleFunc("GET /api/reports/conversion", s.tenantOnly(s.handleConversion)) + mux.HandleFunc("GET /api/sites", s.tenantOnly(s.handleSites)) + mux.HandleFunc("POST /api/sites", s.tenantOnly(s.handleCreateSite)) + mux.HandleFunc("DELETE /api/sites/{site}", s.tenantOnly(s.handleDeleteSite)) + mux.HandleFunc("PATCH /api/sites/{site}", s.tenantOnly(s.handleUpdateSite)) // Cameras, onboarded from head office. The shop PC still does the // connecting - it is the only thing on the camera's network - so these // write desired state that its agent pulls and applies. - mux.HandleFunc("GET /api/cameras", s.authed(s.handleCameras)) - mux.HandleFunc("POST /api/sites/{site}/cameras", s.authed(s.handleCreateCamera)) - mux.HandleFunc("PATCH /api/cameras/{id}", s.authed(s.handleUpdateCamera)) - mux.HandleFunc("DELETE /api/cameras/{id}", s.authed(s.handleDeleteCamera)) - mux.HandleFunc("GET /api/cameras/{id}/snapshot.jpg", s.authed(s.handleGetSnapshot)) - mux.HandleFunc("GET /api/cameras/{id}/live", s.authed(s.handleWatchLive)) + mux.HandleFunc("GET /api/cameras", s.tenantOnly(s.handleCameras)) + mux.HandleFunc("POST /api/sites/{site}/cameras", s.tenantOnly(s.handleCreateCamera)) + mux.HandleFunc("PATCH /api/cameras/{id}", s.tenantOnly(s.handleUpdateCamera)) + mux.HandleFunc("DELETE /api/cameras/{id}", s.tenantOnly(s.handleDeleteCamera)) + mux.HandleFunc("GET /api/cameras/{id}/snapshot.jpg", s.tenantOnly(s.handleGetSnapshot)) + mux.HandleFunc("GET /api/cameras/{id}/live", s.tenantOnly(s.handleWatchLive)) // Prove a camera works: "connection" asks whether the shop PC can open the // stream, "placement" asks whether somebody walking past produces a view // good enough to recognise. Two questions, because a camera passes the // first and fails the second all the time - that is the Office1 case. - mux.HandleFunc("POST /api/cameras/{id}/check", s.authed(s.handleRequestCheck)) + mux.HandleFunc("POST /api/cameras/{id}/check", s.tenantOnly(s.handleRequestCheck)) // The end-to-end answer for one shop, assembled from what head office // already knows - so it works even when the shop PC is off, which is one of // the things it reports. - mux.HandleFunc("GET /api/sites/{site}/check", s.authed(s.handleSiteCheck)) + mux.HandleFunc("GET /api/sites/{site}/check", s.tenantOnly(s.handleSiteCheck)) mux.HandleFunc("POST /api/sites/{site}/enrolment-code", - s.authed(s.handleIssueEnrolmentCode)) + s.tenantOnly(s.handleIssueEnrolmentCode)) // The assistant. Every tool it calls runs as the signed-in user, so it can // only ever see what the person asking could already see. - mux.HandleFunc("POST /api/assistant", s.authed(s.handleAssistant)) + mux.HandleFunc("POST /api/assistant", s.tenantOnly(s.handleAssistant)) // The live feed. `visitors` searches a customer list by name; `visits` // answers the question a shop screen or a mobile app actually asks - who // came through the door just now - and carries each person's photo with // them so rendering four simultaneous arrivals is one request, not nine. - mux.HandleFunc("GET /api/visits", s.authed(s.handleArrivals)) - mux.HandleFunc("GET /api/visits/stream", s.authed(s.handleArrivalStream)) + mux.HandleFunc("GET /api/visits", s.tenantOnly(s.handleArrivals)) + mux.HandleFunc("GET /api/visits/stream", s.tenantOnly(s.handleArrivalStream)) - mux.HandleFunc("GET /api/visitors", s.authed(s.handleVisitors)) - mux.HandleFunc("GET /api/visitors/{id}/history", s.authed(s.handleVisitorHistory)) - mux.HandleFunc("PUT /api/visitors/{id}/profile", s.authed(s.handleSaveProfile)) - mux.HandleFunc("POST /api/purchases", s.authed(s.handlePurchase)) + mux.HandleFunc("GET /api/visitors", s.tenantOnly(s.handleVisitors)) + mux.HandleFunc("GET /api/visitors/{id}/history", s.tenantOnly(s.handleVisitorHistory)) + mux.HandleFunc("PUT /api/visitors/{id}/profile", s.tenantOnly(s.handleSaveProfile)) + mux.HandleFunc("POST /api/purchases", s.tenantOnly(s.handlePurchase)) // Reading sales, not just aggregating them. /api/reports/conversion has // summed this table since it existed; nothing could read a row of it, so // "revenue was 41,000" could not be checked against a till. - mux.HandleFunc("GET /api/sales", s.authed(s.handleSales)) - mux.HandleFunc("GET /api/sales/{id}", s.authed(s.handleSale)) + mux.HandleFunc("GET /api/sales", s.tenantOnly(s.handleSales)) + mux.HandleFunc("GET /api/sales/{id}", s.tenantOnly(s.handleSale)) // The merchant home screen in one call, composed from the functions the // reports already use rather than from new arithmetic. - mux.HandleFunc("GET /api/dashboard/summary", s.authed(s.handleDashboard)) + mux.HandleFunc("GET /api/dashboard/summary", s.tenantOnly(s.handleDashboard)) // Platform administration. Not public registration: an open endpoint that // mints tenants is a far larger thing to secure than one behind an account @@ -402,15 +402,15 @@ func (s *Server) Routes() *http.ServeMux { mux.HandleFunc("GET /api/agent/checks", s.agentAuthed(s.handleAgentChecks)) mux.HandleFunc("POST /api/agent/checks", s.agentAuthed(s.handleAgentCheckResult)) - mux.HandleFunc("GET /api/visitors/{id}/image", s.authed(s.handleVisitorImage)) + mux.HandleFunc("GET /api/visitors/{id}/image", s.tenantOnly(s.handleVisitorImage)) // The bytes of a face this server holds itself. Session-authenticated // rather than a signed link: there is no third party to delegate to, and an // unauthenticated URL would be a way to reach a customer's photograph with // no session at all. - mux.HandleFunc("GET /api/faces/{id}", s.authed(s.handleGetFace)) + mux.HandleFunc("GET /api/faces/{id}", s.tenantOnly(s.handleGetFace)) // The erasure path. Destroys the template and the photo; keeps the // anonymous visit counts, which are legitimate aggregate data. - mux.HandleFunc("DELETE /api/visitors/{id}", s.authed(s.handleForgetVisitor)) + mux.HandleFunc("DELETE /api/visitors/{id}", s.tenantOnly(s.handleForgetVisitor)) return mux } @@ -428,6 +428,36 @@ func PrincipalFrom(ctx context.Context) auth.Principal { return p } +// tenantOnly gates the routes that read or write one company's data. +// +// It exists because a platform admin has NO client - that absence is what +// defines them - and every tenant query scopes on `client_id = $1::uuid`. +// Handing it the empty string makes Postgres cast ” to a uuid, which is an +// ERROR rather than an empty result, so five live endpoints answered 500 to a +// signed-in platform admin: /api/visits, /api/cameras, /api/sites, +// /api/visitors and /api/reports/footfall. Found by calling them. +// +// 403 and not 404, unlike adminOnly. The two hide opposite things: a tenant +// must not learn that a platform surface exists, while a platform admin +// already knows the tenant surface does - they are looking at its data through +// /api/admin. Nothing is concealed by pretending otherwise, and "use the admin +// routes" is the useful answer. +// +// Guarding here rather than in each query is deliberate: a per-query fix is +// one a new query forgets, and the next one would 500 in production exactly +// like these did. +func (s *Server) tenantOnly(next http.HandlerFunc) http.HandlerFunc { + return s.authed(func(w http.ResponseWriter, r *http.Request) { + if PrincipalFrom(r.Context()).ClientID == "" { + writeErr(w, http.StatusForbidden, "not_a_tenant_account", + "This is a company's own data. A platform administrator "+ + "reads it through /api/admin/clients/{id}/...") + return + } + next(w, r) + }) +} + func (s *Server) authed(next http.HandlerFunc) http.HandlerFunc { return func(w http.ResponseWriter, r *http.Request) { tok := auth.BearerToken(r) diff --git a/server/internal/api/tenant_guard_test.go b/server/internal/api/tenant_guard_test.go new file mode 100644 index 0000000..23c4ccb --- /dev/null +++ b/server/internal/api/tenant_guard_test.go @@ -0,0 +1,68 @@ +package api + +import ( + "net/http" + "strings" + "testing" +) + +// A platform admin has no client, and every tenant query scopes on one. Before +// this guard the empty string reached Postgres as `client_id = ”::uuid`, +// which is a cast ERROR and not an empty result - so five live endpoints +// answered 500 to a signed-in platform admin. Found by calling them, not by a +// test: the in-memory fake compares strings and is perfectly happy with "". +func TestATenantRouteRefusesAnAccountWithNoCompany(t *testing.T) { + s, fs := newServer(t) + seedPlatformAdmin(fs) + sess := login(t, s, "root@loyaly.ai", "admin123") + + for _, path := range []string{ + "/api/visits", "/api/cameras", "/api/sites", "/api/visitors", + "/api/reports/footfall", "/api/sales", "/api/dashboard/summary", + } { + rec := do(t, s, "GET", path, sess.Token, nil) + if rec.Code != http.StatusForbidden { + t.Errorf("%s: got %d, want 403 - a platform admin reads a company's "+ + "data through /api/admin, and a 500 here reads as a broken "+ + "server rather than a wrong door", path, rec.Code) + } + // The message has to say where to go instead; "forbidden" alone sends + // somebody hunting a permissions problem that does not exist. + if !strings.Contains(rec.Body.String(), "/api/admin") { + t.Errorf("%s: refusal should point at the admin routes: %s", + path, rec.Body.String()) + } + } +} + +// The guard must not lock a platform admin out of their own session, which is +// not a company's data and is how they sign out or revoke a lost device. +func TestAPlatformAdminKeepsTheirOwnSessionRoutes(t *testing.T) { + s, fs := newServer(t) + seedPlatformAdmin(fs) + sess := login(t, s, "root@loyaly.ai", "admin123") + + for _, path := range []string{"/api/auth/me", "/api/auth/sessions"} { + if rec := do(t, s, "GET", path, sess.Token, nil); rec.Code != http.StatusOK { + t.Errorf("%s: got %d, want 200", path, rec.Code) + } + } +} + +// And the ordinary case must be untouched: a tenant user still reaches +// everything they always did. +func TestATenantUserIsUnaffectedByTheGuard(t *testing.T) { + s, fs := newServer(t) + seedUser(fs) + sess := login(t, s, "manager@acme.com", "correct horse battery") + + for _, path := range []string{ + "/api/visits", "/api/cameras", "/api/sites", "/api/visitors", + "/api/sales", "/api/dashboard/summary", + } { + if rec := do(t, s, "GET", path, sess.Token, nil); rec.Code != http.StatusOK { + t.Errorf("%s: got %d, want 200 for an ordinary tenant account: %s", + path, rec.Code, rec.Body.String()) + } + } +}