The POS surface was open. A till named its own outlet — `store_id` in a query or in an ingest batch — and was believed, so one number changed in Settings read another tenant's catalogue or posted bills into their books. There was no middleware in the codebase at all, and the `JWT_SECRET_KEY` in the config was read and never used. Products were never mis-scoped: `resolvePosStore` already derived the tenant from the location and the catalogue query already filtered on both. The tenant was never taken from the wire. What was missing was any check that the caller was entitled to the location they named. So the outlet now comes *out* of a sign-in rather than going *in* from the till. `POST /pos/login` authenticates against the same `app_users` rows the web console uses — one account store, so deactivating a leaver closes both doors — and answers with the outlets that account may reach, sealed in an HMAC-SHA256 token the terminal cannot edit. Two checks then guard everything else, in order: the token verifies, and the outlet named in the request belongs to the token's tenant. The second is the one that matters — a valid token is a licence to name *your* outlets, not any. Notes on the awkward parts: - The guard reads the outlet from the body as well as the query. The two routes that write carry `store_id` in a JSON batch and never in the URL, so a query-only check would have left exactly the dangerous call unguarded. - Three spellings of one thing survive — `store_id`, `locationid`, `location_id`. All three are read rather than normalised, because renaming them breaks terminals already in the field. - `POS_AUTH_REQUIRED` defaults to false. Tills are billing real customers against the open endpoints right now and enforcing at deploy would stop every one mid-trade. A token is still verified when sent, and a wrong-tenant token still refused; the flag only governs requests carrying none. - `POS_TOKEN_SECRET` has no baked-in fallback and fails loudly. A development secret in source is the same as no signature at all. - `configid` is inferred when the till does not send it, because a person at a counter has no way to know theirs. `authname` is not unique in this schema — live data has one address twice under one configid — so an ambiguous match is refused rather than resolved by LIMIT 1, which could bill into the wrong tenant's books. Verified against live data: 58 accounts across 34 tenants can open a till, an account pinned to a location resolves to it alone, a tenant-level account gets all six of its outlets, and a cross-tenant outlet request is refused. Passwords are still plaintext platform-wide. Flagged at the comparison site; fixing it is a migration touching every login path, not this endpoint. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
152 lines
4.5 KiB
Go
152 lines
4.5 KiB
Go
package middleware
|
|
|
|
import (
|
|
"net/http/httptest"
|
|
"strings"
|
|
"testing"
|
|
|
|
"github.com/gofiber/fiber/v2"
|
|
)
|
|
|
|
// The outlet a request names has to be found wherever the route happens to put
|
|
// it. These cover the extraction alone — it is the part that decides whether
|
|
// the authorisation check runs at all, and a miss here reads exactly like a
|
|
// pass.
|
|
|
|
func locationFor(t *testing.T, method, target, body string) int {
|
|
t.Helper()
|
|
|
|
app := fiber.New()
|
|
found := -1
|
|
app.All("/probe", func(c *fiber.Ctx) error {
|
|
found = requestedLocation(c)
|
|
return c.SendStatus(fiber.StatusOK)
|
|
})
|
|
|
|
var reader *strings.Reader
|
|
if body == "" {
|
|
reader = strings.NewReader("")
|
|
} else {
|
|
reader = strings.NewReader(body)
|
|
}
|
|
|
|
req := httptest.NewRequest(method, target, reader)
|
|
if body != "" {
|
|
req.Header.Set("Content-Type", "application/json")
|
|
}
|
|
|
|
if _, err := app.Test(req); err != nil {
|
|
t.Fatalf("probing: %v", err)
|
|
}
|
|
return found
|
|
}
|
|
|
|
func TestTheOutletIsFoundUnderEveryNameTheRoutesUse(t *testing.T) {
|
|
// Three spellings for one thing across the POS routes. Missing any of them
|
|
// leaves that route unguarded.
|
|
cases := []struct {
|
|
name string
|
|
target string
|
|
}{
|
|
{"catalogue says store_id", "/probe?store_id=1135"},
|
|
{"sales say locationid", "/probe?locationid=1135"},
|
|
{"health says location_id", "/probe?location_id=1135"},
|
|
}
|
|
|
|
for _, tc := range cases {
|
|
t.Run(tc.name, func(t *testing.T) {
|
|
if got := locationFor(t, "GET", tc.target, ""); got != 1135 {
|
|
t.Fatalf("wanted outlet 1135, got %d", got)
|
|
}
|
|
})
|
|
}
|
|
}
|
|
|
|
// The two routes that *write* carry the outlet in a JSON batch and never in the
|
|
// URL. Checking only the query string would leave the exact call that posts
|
|
// bills into another tenant's books unguarded.
|
|
func TestTheOutletIsFoundInAnIngestBody(t *testing.T) {
|
|
body := `{"batch_id":"b1","terminal_id":"T5EDD","store_id":"1135","orders":[]}`
|
|
|
|
if got := locationFor(t, "POST", "/probe", body); got != 1135 {
|
|
t.Fatalf("wanted outlet 1135 from the batch body, got %d", got)
|
|
}
|
|
}
|
|
|
|
// The till quotes its ids; other callers send them bare. Accepting only one
|
|
// shape silently skips the check for the other.
|
|
func TestAnOutletIsReadWhetherQuotedOrNot(t *testing.T) {
|
|
quoted := `{"store_id":"1135"}`
|
|
bare := `{"store_id":1135}`
|
|
|
|
if got := locationFor(t, "POST", "/probe", quoted); got != 1135 {
|
|
t.Fatalf("quoted store_id: wanted 1135, got %d", got)
|
|
}
|
|
if got := locationFor(t, "POST", "/probe", bare); got != 1135 {
|
|
t.Fatalf("bare store_id: wanted 1135, got %d", got)
|
|
}
|
|
}
|
|
|
|
func TestAHealthBodyNamesItsOutlet(t *testing.T) {
|
|
body := `{"terminal_id":"T5EDD","location_id":"1135","status":"online"}`
|
|
|
|
if got := locationFor(t, "POST", "/probe", body); got != 1135 {
|
|
t.Fatalf("wanted outlet 1135 from the health body, got %d", got)
|
|
}
|
|
}
|
|
|
|
// A request naming no outlet is not an error — /session names none — so it must
|
|
// come back as "nothing to check" rather than as outlet zero.
|
|
func TestARequestNamingNoOutletReportsNone(t *testing.T) {
|
|
if got := locationFor(t, "GET", "/probe", ""); got != 0 {
|
|
t.Fatalf("wanted 0 for a request naming no outlet, got %d", got)
|
|
}
|
|
if got := locationFor(t, "POST", "/probe", `{"batch_id":"b1"}`); got != 0 {
|
|
t.Fatalf("wanted 0 for a body naming no outlet, got %d", got)
|
|
}
|
|
}
|
|
|
|
// A body this middleware cannot parse must not be treated as naming an outlet.
|
|
// The handler will refuse it on its own terms; guessing here would either
|
|
// reject a good request or wave a bad one through.
|
|
func TestAnUnparseableBodyNamesNoOutlet(t *testing.T) {
|
|
if got := locationFor(t, "POST", "/probe", `{not json at all`); got != 0 {
|
|
t.Fatalf("wanted 0 for an unparseable body, got %d", got)
|
|
}
|
|
}
|
|
|
|
func TestABearerTokenIsReadInEveryFormTheFieldSends(t *testing.T) {
|
|
app := fiber.New()
|
|
var got string
|
|
app.Get("/probe", func(c *fiber.Ctx) error {
|
|
got = bearerToken(c)
|
|
return c.SendStatus(fiber.StatusOK)
|
|
})
|
|
|
|
cases := []struct {
|
|
name string
|
|
header string
|
|
value string
|
|
want string
|
|
}{
|
|
{"the standard form", "Authorization", "Bearer abc.def", "abc.def"},
|
|
{"a bare token, which terminals send", "Authorization", "abc.def", "abc.def"},
|
|
{"the fallback header", "X-Pos-Token", "abc.def", "abc.def"},
|
|
{"a scheme we do not issue", "Authorization", "Basic dXNlcjpwdw==", ""},
|
|
}
|
|
|
|
for _, tc := range cases {
|
|
t.Run(tc.name, func(t *testing.T) {
|
|
got = ""
|
|
req := httptest.NewRequest("GET", "/probe", nil)
|
|
req.Header.Set(tc.header, tc.value)
|
|
if _, err := app.Test(req); err != nil {
|
|
t.Fatalf("probing: %v", err)
|
|
}
|
|
if got != tc.want {
|
|
t.Fatalf("wanted %q, got %q", tc.want, got)
|
|
}
|
|
})
|
|
}
|
|
}
|