updates on the otp updates on the customer app

This commit is contained in:
2026-09-15 11:50:12 +05:30
parent e8f4c0a593
commit 89321c9e06
17 changed files with 1072 additions and 69 deletions

138
CLAUDE.md
View File

@@ -237,8 +237,11 @@ websocket routes. **[verified this session]**
confirm current values rather than assuming): admin login at
`suriya@doormile.com`; hub staff accounts pattern `hub.[city]@doormile.in`
plus partner variants; miler test phone numbers + PINs.
- Auth: Firebase OTP for customer login — this cannot be scripted/bypassed
from the command line, which is why the last live E2E test stalled (§9).
- Auth: **not Firebase.** Customer login is a 4-digit OTP to phone or email,
issued by `issueCxOtp` and stored in Redis (§8.5). `CX_STAGING_OTP` makes it
a fixed code outside production, which is what lets it be scripted — the
earlier "cannot be bypassed from the command line" note (§9) predates that
and predates the `/customer/*` rebuild.
---
@@ -597,6 +600,137 @@ key, after the legacy rider app's key had to be revoked), `MILER_CALL_PROXY`
---
## 8.6 Customer sign-in, config hardening & the ordering fix (2026-09-11)
**[verified this session]** — six defects were reported against customer
bookings. Five were real; one was not. Everything below was verified against
the code, and the schema question against the **production database**.
### The one that locked every customer out (DM-01)
`CxVerifyOtp` read `json:"code"`. The customer app sent `otp` — because
`docs/customer-app-api-crisp.md` documented `otp`, while
`docs/openapi-customer.yaml` correctly said `code`. **The two docs disagreed
and the app was built from the wrong one.** `req.Code` was therefore always
empty, the empty-code branch always fired, and every sign-in failed with
`400 "Enter the code we sent you"` — a correct code failed exactly like a
wrong one. No amount of SMS-gateway credit would have fixed it.
The handler now accepts `otp` as a **deprecated alias**; `code` wins when both
are present. This deliberately fixes builds already in customers' hands, which
an app-side fix alone cannot. `controllers/cxOtpFieldAlias_test.go` pins both
names, the precedence, and that a blank/missing code is still rejected. Remove
the alias once the install base has moved on.
### A failed OTP send no longer punishes the customer (DM-05)
`issueCxOtp` writes the code, the resend cooldown and the rate-limit slot
*before* attempting delivery — it has to, the code must exist to be sent. But
on failure it kept all three: the customer was told something went wrong, could
not resend until the cooldown expired, had spent one of their five hourly
codes, and a valid code they never received sat live in Redis for its full TTL.
All three are now rolled back on a delivery error (`DEL` the code and cooldown,
`DECR` the rate counter), so a retry is immediate and nothing usable is left.
### Production credentials are no longer defaults (DM-06)
`config.Load()` defaulted `JWT_SECRET_KEY` to a literal, and `NATS_URL` /
`NATS_USER` / `NATS_PASSWORD` / `AI_LAYER_BASE_URL` / `ROUTE_OPTIMIZER_URL` /
`DB_PASSWORD` to the real production values. Two consequences, both live:
a clone of this repository could mint a valid token for any user id and any
role against any deployment that had not overridden the secret; and `go run .`
on a laptop silently joined the production NATS cluster and competed with the
real workers for the same durable consumer. **This was hit accidentally on
2026-09-11** — a local instance pulled `api.v1.bookings.update` for real
booking ids and caused redelivery churn on production for ~40 seconds.
All now default to empty, and the empty case is handled rather than assumed:
`InitNATS` skips connecting, `routing.BaseURL == ""` already disabled
sequencing, and the AI layer returns an error so the caller's existing
`AI_LAYER_FALLBACK` path takes over with legacy scoring. A second hardcoded
production URL in `internal/assignment/ai_layer.go` (not in the original
report) was removed too.
`JWT_SECRET_KEY` is special-cased because an empty signing key is worse than a
shared one: **`cfg.Validate()`, called from `main`, refuses to start** when it
is unset in production. Outside production an ephemeral per-process key is
generated with a warning, so local development needs no configuration while
tokens stop surviving a restart. `GEOCODER_URL` deliberately keeps its default
— Nominatim is a public service, not a Doormile host.
### `GET /admin/bookings` is ordered (DM-04)
Added `Order("bookingid DESC")`. Without it the row order was unspecified —
Postgres heap order, oldest first — which put the newest booking on the LAST
page, outside the console's bounded drain window, and made `OFFSET` paging
unstable enough to duplicate and skip rows. The primary key is unique, so the
sort needs no tiebreaker.
The console half lives in the admin console repo (`krow_talent_app` — the
directory name is stale; it is the Doormile Express Console): it requests
`pagesize=1000`, receives 100, and stops after 12 pages, so it sees 1200 rows
regardless. With the list now newest-first those 1200 are the most recent ones
rather than the oldest, which turns a silent disappearance into a bounded view.
### DM-03 (timestamp drift) is NOT a production bug — do not "fix" it
Reported as `DBNow()` relabelling IST wall-clock as UTC against
`timestamp with time zone` columns, causing a +5:30 drift that hid evening
bookings from the console. **Verified against production and it is false
there:**
```
pickupbookings.createdat timestamp without time zone
pickupbookings.preferredpickupfrom timestamp without time zone
appcustomers.createdat timestamp without time zone
```
Which is exactly what `DBNow()`'s own comment assumes. A round-trip confirmed
it: a customer created at a known `12:10:34 IST` stored as `12:10:34.094323`.
Zero drift. **Changing `DBNow()` would introduce the bug, not fix it.**
The real finding is the reporter's own fallback: GORM's Postgres driver maps
`time.Time` to `timestamptz`, so a schema built fresh from `AutoMigrate` does
NOT match production, and every new dev environment WILL show the +5:30 drift
that production does not. That is why they saw it. Pin the column types
explicitly in the models before this bites someone again.
### Docs corrected
`docs/customer-app-api-crisp.md` had **three** request shapes that did not
match their parsers, all failing silently through `BodyParser` — no error, just
a zero value:
| Endpoint | Documented | Actually parsed |
|---|---|---|
| `auth/otp/verify` | `otp` | `code` (now both) |
| `fare/estimate` | `pickup.latitude`/`longitude`, `packages[].weightKg` | `pickup.lat`/`lng`, `packageCount` |
| `bookings` | flat destination fields, `pickup.latitude` | nested `details{}`, `pickup.lat`/`lng` |
The booking **response** block was wrong the same way (`latitude`/`longitude`
where `renderCxBooking` emits `lat`/`lng`). `docs/express-console-api.md` also
claimed pagination "default 500, cap 1000" when the code is default 20, cap 100
— which is what made the console size its page budget for twelve times the rows
it actually receives.
**When a doc and a parser disagree here, the parser has won every time.** Three
separate client teams have now built against wrong Doormile docs in one week.
### Still open from this report
- **DM-02: email OTP returns 500 on production.** `SMTP_HOST`, `SMTP_USER` and
`SMTP_PASSWORD` all default to `""` and are not set. Either configure SMTP or
hide the app's Email tab — offering a path that always fails is worse than
not offering it.
- The **committed secrets** (`.env` and a live GCP service-account private key)
are still tracked in git and pushed. Removing the defaults above does not
help until those keys are **rotated**.
- `GET /api/v1/ready` returns **503 while its body says `"status":"ready"`**
(`routes/routes.go`) — a monitor reading the body sees the opposite of the
status code.
---
## 9. Current blockers & open work (whole-project level)
**[carried forward]**