Revert "updates on the otp updates on the customer app"
This reverts commit 89321c9e06.
This commit is contained in:
138
CLAUDE.md
138
CLAUDE.md
@@ -237,11 +237,8 @@ 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: **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.
|
||||
- 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).
|
||||
|
||||
---
|
||||
|
||||
@@ -600,137 +597,6 @@ 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]**
|
||||
|
||||
Reference in New Issue
Block a user