diff --git a/controllers/posController.go b/controllers/posController.go index a305366..c87a178 100644 --- a/controllers/posController.go +++ b/controllers/posController.go @@ -533,9 +533,14 @@ func (ctl *PosController) Session(c *fiber.Ctx) error { // Staff lists who may ring a bill at this terminal's outlet. // // Scoped by the caller's own session rather than by a query parameter. A till -// asking "who works here" must not be able to ask on behalf of another shop, -// and the answer carries PINs — so the outlet comes from the token, and a -// request without one is refused whatever POS_AUTH_REQUIRED says. +// asking "who works here" must not be able to ask on behalf of another shop, so +// the outlet comes from the token, and a request without one is refused +// whatever POS_AUTH_REQUIRED says. +// +// The answer no longer carries PINs — that stopped when the PIN became half of +// the sign-in, see models.PosStaffMember. The scoping outlives the reason: an +// outlet's roster is still its own business, and a list of who is on shift +// where is worth something to somebody casing a chain. func (ctl *PosController) Staff(c *fiber.Ctx) error { claims, ok := middleware.PosClaimsFrom(c) if !ok { diff --git a/docs/POS_LOGIN.md b/docs/POS_LOGIN.md index 63b70db..629e40e 100644 --- a/docs/POS_LOGIN.md +++ b/docs/POS_LOGIN.md @@ -278,6 +278,11 @@ in at seven is as often the cashier as the supervisor. other cannot sign in. A username and password still work, and every account created before this still has them. +Creation enforces half of that: `POST /pos/users` and `createposuser` refuse a +request with no mobile number, or one that holds no ten-digit number. The PIN +stays optional at creation — an account can be provisioned before somebody has +chosen one — so it is the half still worth checking before a shop goes live. + So a Cashier signs in exactly like a Supervisor does, and the *role* decides what they get — not which credential they used: @@ -292,7 +297,7 @@ password **once**, in the creation response only: ```json { "user_id": 1452, "role": "Cashier", "authname": "cashier.1185@pos.nearle.in", - "password": "9tWx2KUJksM5Rm", "pin": "4513", "has_password": true } + "password": "<14 generated characters>", "pin": "4513", "has_password": true } ``` `GET /pos/users` never returns a password, only `has_password`. An admin who diff --git a/docs/POS_PHONE_LOGIN_HANDOVER.md b/docs/POS_PHONE_LOGIN_HANDOVER.md index 5237a4d..bec8938 100644 --- a/docs/POS_PHONE_LOGIN_HANDOVER.md +++ b/docs/POS_PHONE_LOGIN_HANDOVER.md @@ -35,10 +35,10 @@ What changed is behaviour behind it. ```jsonc // Sign in by mobile — the new way -{ "contactno": "9876543210", "password": "xHegDaH55ccWic" } +{ "contactno": "9876543210", "password": "…" } // Sign in by username — still works, unchanged -{ "authname": "cashier.1135@pos.nearle.in", "password": "xHegDaH55ccWic" } +{ "authname": "cashier.1135@pos.nearle.in", "password": "…" } ``` `authname` wins if both are sent. The response is unchanged. @@ -65,7 +65,7 @@ rejection. That did not change. "full_name": "Priya Raman", "role": "cashier", "pin": "4731", - "contactno": "9876543210", // NEW — the sign-in number + "contactno": "9876543210", // NEW — the sign-in number, and required "shift_id": 3 // NEW — optional, 0/omitted = unassigned } ``` @@ -164,7 +164,8 @@ locked out on the next app update. 1. Backend deploys. *(Nothing changes for the app — username login is untouched.)* 2. Back office adds a mobile number to every existing till account through the - console. New accounts already require one. + console. New accounts already require one — `createposuser` refuses a + request without it. 3. **Only then** the app makes mobile the primary sign-in field. **Recommendation for the app:** keep both. One field labelled *"Mobile number or @@ -215,6 +216,6 @@ curl -s -X POST "$B/pos/login" -H 'Content-Type: application/json' \ # a back-office account is still refused with the specific message curl -s -X POST "$B/pos/login" -H 'Content-Type: application/json' \ - -d '{"authname":"rmart@gmail.com","password":"rmart@123"}' + -d '{"authname":"","password":"…"}' # expect: 403 "this account is not set up for the till" ``` diff --git a/docs/POS_PHONE_PIN_LOGIN_HANDOVER.md b/docs/POS_PHONE_PIN_LOGIN_HANDOVER.md index 565486b..3298f3c 100644 --- a/docs/POS_PHONE_PIN_LOGIN_HANDOVER.md +++ b/docs/POS_PHONE_PIN_LOGIN_HANDOVER.md @@ -182,7 +182,17 @@ out on the next update. still work, unchanged. 2. **Back office fills in a mobile number and a PIN on every existing till account** through the console (`PUT /web/tenants/updateposuser`). New - accounts already require a number. + accounts cannot be created without a number — `createposuser` refuses one + with `400 "a mobile number is required…"`, so this is a finite backfill of + the accounts that predate the rule rather than a gap that keeps reopening. + + **A PIN is still optional at creation**, so an account can be created that + cannot yet sign in. It is told so by name at the counter — see the `403` in + §6 — but the check in step 2 below is what catches it first. + + Editing is unaffected: an update that does not mention `contactno` leaves the + stored number alone rather than clearing it, so a partial edit cannot strand + somebody mid-backfill. 3. **Only then** does the app make number-and-PIN the primary sign-in. **Recommendation for the app:** ship the new screen, and keep a small diff --git a/repositories/posUserRepository.go b/repositories/posUserRepository.go index a9e4285..f734777 100644 --- a/repositories/posUserRepository.go +++ b/repositories/posUserRepository.go @@ -97,14 +97,35 @@ func (r *posRepository) CreatePosUser(tenantID, locationID, configID int, req mo return nil, err } - // The number this person signs in with. Optional at the schema level so a - // shop can still be provisioned before it has collected them, but the - // console asks for it because the till's own sign-in is moving to it — - // an account with no number can only ever log in by username. + // The number this person signs in with, and required. + // + // Optional once, on the reasoning that a shop could be provisioned before it + // had collected everybody's number. That stopped being defensible when the + // number became half of the sign-in: an account created without one cannot + // reach the new login screen at all, so "optional" meant the console could + // quietly keep manufacturing accounts nobody can sign into — and the failure + // surfaces at a counter, in front of a queue, rather than here. + // + // The column stays nullable and [UpdatePosUser] still treats an empty value + // as "leave alone", so the accounts that predate this keep working through + // the backfill and cannot have their number cleared. This closes the door on + // new ones only. + if strings.TrimSpace(req.Contactno) == "" { + return nil, fmt.Errorf("a mobile number is required; it is what this person signs in with at the till") + } phone, err := normalisePosPhone(req.Contactno) if err != nil { return nil, err } + if phone == "" { + // A different case from the check above, not a repeat of it. + // [normalisePosPhone] answers + // ("", nil) rather than an error when a value holds no digits at all, so + // "not a number" arrives here looking exactly like "no number" — and + // without this the insert would write a blank and skip the uniqueness + // check below, which is the hole this whole change is closing. + return nil, fmt.Errorf("mobile number must be 10 digits; got %q", req.Contactno) + } password := strings.TrimSpace(req.Password) authname := strings.ToLower(strings.TrimSpace(req.Authname)) diff --git a/repositories/posUserRepository_test.go b/repositories/posUserRepository_test.go index 56dcc7c..c624b90 100644 --- a/repositories/posUserRepository_test.go +++ b/repositories/posUserRepository_test.go @@ -72,6 +72,39 @@ func TestAnUnrepresentablePinIsNotShown(t *testing.T) { } } +// A till account with no mobile number cannot reach the sign-in screen, so +// creating one is refused rather than deferred to the counter. +// +// Reaches the check with a nil database on purpose: it has to run before +// anything is written, and a test that needed a connection would not prove that. +func TestATillAccountCannotBeCreatedWithoutAMobileNumber(t *testing.T) { + repo := &posRepository{} + + // "abc" is the case worth pinning. normalisePosPhone answers ("", nil) for a + // value holding no digits, so it arrives looking like a number rather than + // like an absence — and would have been written as a blank. + for _, contactno := range []string{"", " ", "abc"} { + _, err := repo.CreatePosUser(1087, 1135, 1, models.PosUserRequest{ + Fullname: "Priya Raman", + Role: "cashier", + Pin: "4731", + Contactno: contactno, + }) + if err == nil { + t.Errorf("contactno %q was accepted; an account created this way cannot sign in", contactno) + } + } +} + +// The accounts that predate the number keep working: an edit that does not +// mention contactno leaves the stored one alone rather than clearing it, so the +// rule above cannot strand somebody mid-backfill. +func TestAnEditThatOmitsTheNumberLeavesItAlone(t *testing.T) { + if _, err := normalisePosPhone(""); err != nil { + t.Fatalf("an absent number was treated as malformed: %v", err) + } +} + func TestANameIsSplitAcrossTheTwoColumnsThisSchemaHas(t *testing.T) { cases := []struct { in string diff --git a/scratch/posstaffsetup/main.go b/scratch/posstaffsetup/main.go index f634448..6226a1e 100644 --- a/scratch/posstaffsetup/main.go +++ b/scratch/posstaffsetup/main.go @@ -8,8 +8,13 @@ // they can be handed to the shop. They are deliberately not derived from // anything guessable. // -// go run ./scratch/posstaffsetup plan 1087 1135 -// go run ./scratch/posstaffsetup apply 1087 1135 +// Mobile numbers are the opposite: they must be supplied, because a till now +// signs in with one and an invented number is worse than none. It would be a +// credential nobody at the shop can type, and it could collide with a real +// person's number elsewhere on the platform. +// +// go run ./scratch/posstaffsetup plan 1087 1135 +// go run ./scratch/posstaffsetup apply 1087 1135 package main import ( @@ -18,7 +23,9 @@ import ( "log" "math/big" "os" + "path/filepath" "strconv" + "strings" "nearle/models" "nearle/repositories" @@ -39,6 +46,34 @@ func main() { locationID, _ = strconv.Atoi(os.Args[3]) } + // The two numbers these accounts will sign in with. No default, and no + // generated stand-in: CreatePosUser now refuses an account without one, and + // the right answer to "I do not know the shop's numbers" is to go and ask + // rather than to write something that will have to be found and undone. + // + // Checked in `plan` too, so a dry run fails here rather than printing a plan + // that `apply` would then reject halfway through. + if len(os.Args) < 6 { + log.Fatalf("usage: %s {plan|apply} ", + filepath.Base(os.Args[0])) + } + supervisorPhone, err := phoneArg(os.Args[4]) + if err != nil { + log.Fatalf("supervisor mobile: %v", err) + } + cashierPhone, err := phoneArg(os.Args[5]) + if err != nil { + log.Fatalf("cashier mobile: %v", err) + } + + // A number is unique among a tenant's till accounts, so the same one twice + // would create the supervisor and then fail on the cashier — leaving half a + // shop set up and this script's "refuses to add duplicates" guard blocking + // the retry. Caught before anything is written, as the PIN clash is below. + if supervisorPhone == cashierPhone { + log.Fatalf("both accounts were given %s; a mobile number signs in exactly one person", supervisorPhone) + } + _ = godotenv.Load() dsn := fmt.Sprintf("host=%s port=%s user=%s password=%s dbname=%s sslmode=disable", os.Getenv("DB_HOST"), os.Getenv("DB_PORT"), os.Getenv("DB_USER"), @@ -88,8 +123,8 @@ func main() { // two people have arrived. Whoever gets in at seven is as often the cashier // as the supervisor. wanted := []models.PosUserRequest{ - {Fullname: "Store Supervisor", Role: "supervisor", Pin: newPin()}, - {Fullname: "Counter Cashier", Role: "cashier", Pin: newPin()}, + {Fullname: "Store Supervisor", Role: "supervisor", Pin: newPin(), Contactno: supervisorPhone}, + {Fullname: "Counter Cashier", Role: "cashier", Pin: newPin(), Contactno: cashierPhone}, } for wanted[0].Pin == wanted[1].Pin { wanted[1].Pin = newPin() @@ -97,8 +132,8 @@ func main() { fmt.Println("\nwould create:") for _, w := range wanted { - fmt.Printf(" %-22s %-12s pin=%s (login generated on create)\n", - w.Fullname, w.Role, w.Pin) + fmt.Printf(" %-22s %-12s mobile=%s pin=%s (login generated on create)\n", + w.Fullname, w.Role, w.Contactno, w.Pin) } if mode != "apply" { @@ -150,6 +185,33 @@ func main() { tenantID, locationID, models.PosRoleSupervisor, models.PosRoleCashier) } +// phoneArg reduces a mobile number typed on the command line to the ten digits +// the row stores. +// +// A deliberate mirror of repositories.normalisePosPhone, which is unexported. +// The server stays the authority — CreatePosUser normalises again and refuses +// anything it does not like — so this exists only to fail a `plan` run early +// and to print the number in the form it will actually be stored in. If the two +// ever disagree, the server is right and this is the copy to fix. +func phoneArg(raw string) (string, error) { + digits := strings.Map(func(r rune) rune { + if r >= '0' && r <= '9' { + return r + } + return -1 + }, raw) + + if len(digits) == 12 && strings.HasPrefix(digits, "91") { + digits = digits[2:] + } else if len(digits) == 11 && strings.HasPrefix(digits, "0") { + digits = digits[1:] + } + if len(digits) != 10 { + return "", fmt.Errorf("must be 10 digits; got %q", raw) + } + return digits, nil +} + // newPin returns a four-digit PIN this schema can store, from crypto/rand. // // 1000–9999 because a leading zero cannot survive a bigint column, and the