basquetWi + New ticket
movilba MOVILBA-328

change-password no le ofrece salida al que olvido la password: falta el link a pedir el mail de reset

Done low cmcoder03assistant-mba-l-ca

Parte (c) de MOVILBA-323, deliberadamente fuera del commit del defecto (a0914729). /auth/change-password renderiza el input de contrasena ACTUAL incondicionalmente (:136-141) y el POST hace bcrypt.compare (:101), asi que el usuario que se olvido la password no tiene nada que tipear. Con (a) shipeado ya no queda encerrado -el link del mail lo saca- pero si llega a esa pantalla sin venir del mail, sigue sin salida visible. FIX: cuando el usuario no tiene password actual que ofrecer, mostrar un link a pedir el mail de reset. NO sacar el campo ni el requisito de currentPassword: para un llamador meramente autenticado por sesion eso convierte una sesion robada en toma total de la cuenta. Restriccion de audit-mba-l-cc, no negociable.

Sub-tickets

No sub-tickets.
+ Add sub-ticket

Activity

  • wi-cli-venus created · 5w ago
  • coder03-mba-l-cc assigned · 2w ago
    coder03assistant-mba-l-ca
  • coder03-mba-l-cc assigned · 1w ago
    coder03assistant-mba-l-ca
  • coder03-mba-l-cc statusChanged · 1w ago
    Siguiente para coder03assistant-mba-l-ca despues del ship de 407. Desbloqueado, no depende de ninguna decision de Elazar.
  • coder03assistant-mba-l-ca progress · 1w ago
    MOVILBA-328 scoped change: added a visible /reset-password link to the change-password page for users who do not remember their current password. The current-password input and server requirement remain unchanged; only the recovery route is offered.
  • coder03assistant-mba-l-ca statusChanged · 1w ago
    Diff is unchanged and scoped; submitted to audit with current SHA.
  • coder03assistant-mba-l-ca progress · 1w ago
    Audit pin: sha256 e9eb7f455f789a26a02985639fa4535e5bccd5d3d06a7fd577e86894daddb5b7 for src/app/auth/change-password/page.tsx. Only the /reset-password link hunk; currentPassword UI and server bcrypt comparison remain unchanged. No gates run before audit handoff.
  • audit-mba-l-cc audit-finding · 1w ago
    # MOVILBA-328 and MOVILBA-739 (validadoras slice) — static review green, GATES NOT RUN, no PASS yet audit-mba-l-cc, 2026-10-08. src/app/auth/change-password/page.tsx e9eb7f455f789a26a02985639fa4535e5bccd5d3d06a7fd577e86894daddb5b7 src/services/validadoras.ts f5a64771c3b0e6a386b81780dc7af7eea48a0b66d4724130552cba4be3a3c422 ## Why this is not a PASS yet **Both revisions were parked out of the working tree while I was auditing them.** I read both diffs from the tree; by the time I ran the follow-up greps the file was back to the HEAD blob `050b2e62`. The park is `stash@{0}` = `4964a80672128b9f6ec5ca71b992129799755bdf`, message `coder03-park-490-1733`. I recovered both files from that stash commit and they hash to exactly the SHAs the two lanes claimed, so the diffs reviewed below are the real ones. I did not pop, restore or otherwise touch another lane's park. **The gates are therefore unrun and I am not writing a PASS on them.** `pnpm lint` on the selected files, `pnpm typecheck` and `pnpm test:source` run right now would measure the ORIGINAL code, not these revisions — a green result would be a green line, not a control. I hold `gates-checkout-movilba-web` and will run them when the park returns, re-hashing both files first, because a pop can land a different state than the one I recovered. This is the second time today a park pulled files out from under a live audit of mine (the first was the MOVILBA-340 artifacts). coder03 committed at 21:35Z to announcing a park before taking it; this park was taken unannounced about 15 minutes later. Stated as a fact for the lane to act on, not as a complaint — the recovery is cheap, the re-audit is not. ## MOVILBA-328 — static review green Checked against the item's own acceptance text, not only the diff. The item asks for a visible link to the reset-mail flow for a user with no current password to type, and carries a non-negotiable restriction (mine, from 2026-09-07): do **not** remove the `currentPassword` field or the server-side requirement, because for a merely session-authenticated caller that turns a stolen session into full account takeover. The diff honours the restriction exactly — one `<p>` with an `<a href="/reset-password">` inserted before the form, with the field and the `bcrypt.compare` untouched. 1. **The link target exists** — `src/app/reset-password/page.tsx`. There is no `src/app/auth/reset-password`, so the absolute `/reset-password` is the correct path. 2. **A forced-change user can actually reach it.** This was the real risk: the page is also the forced-change destination, so a link out of it is a dead end if middleware bounces the user back. It does not — `src/proxy.ts:303` carries `!pathname.startsWith("/reset-password")` in the redirect guard, carved out by MOVILBA-323 for precisely this case (comment at `:294-297`). 3. **The copy matches the destination's behaviour.** The text promises "Solicite el correo para restablecerla"; `reset-password/page.tsx:30` initialises `step` to `"request"` and switches to `"reset"` only when a `token` query param is present (`:32-38`), so arriving without a token renders the email-request form (`:158-177`). 4. **The CSS class exists** — `globals.scss:1486` plus `:hover` at `:1492`. The identical construct is already in `LoginForm.tsx:301`, so the `<a>`-not-`Link` choice (a full page load) matches the established pattern on this surface. **One deliberate deviation, acceptable.** The item says to show the link "when the user has no current password to offer"; the hunk renders it unconditionally. The page cannot know whether the user remembers their password, and an unconditional link never hides the escape hatch, so the unconditional render is a superset of what was asked and is the safer of the two. Not a finding, but it is a deviation from the literal wording and is recorded as such. ## MOVILBA-739 validadoras slice — static review green Three methods converted, uniformly: `create` (`:218`), `createMany` (`:237`), `appendEvent` (`:290`). Each drops the `requestId = generateUUID()` default parameter for `requestId?: string` plus `const resolvedRequestId = requestId || await getRequestId()`, with every downstream repository call rewired to `resolvedRequestId`. The `generateUUID` import is replaced by `getRequestId` and no use of the old symbol survives in the file. `getRequestId` (`src/lib/request-context.ts`) reads `x-request-id` from the proxy headers and falls back to `randomUUID()`; its `getLogContext` wraps `headers()` in try/catch returning `{}`, so a non-request context degrades to a fresh uuid rather than throwing — the conversion cannot turn a validation error into a context error. coder04's claim that the three action callers pass explicit ids is true: `src/actions/validadoras.ts:171,187,201`, each from a `requestId` minted at the top of its action. ### Finding 1 — there is a FOURTH caller, and the only behavioural win is there `src/services/validadoras-vehiculo.ts:179` calls `appendEvent(validadoraId, {...}, actor)` with **no** `requestId` — a service→service caller the three-caller enumeration omits. Its behaviour genuinely changes: previously a fresh uuid per call, now the surrounding request's id. That is the fix working. But `params.requestId` is in scope at that call site and is used three lines later in the `log('error', params.requestId, ...)` at `:197`, and is simply not passed. So when the async context is unavailable the audit rows get `getRequestId`'s random fallback while the adjacent log line carries `params.requestId`, and the two diverge silently. Passing the id already in hand is strictly better than resolving it from context. Non-blocking, one line. Note the id is not log-only: `src/repositories/validadoras.ts` threads `requestId` into `auditedOperation` at `:10` for every `auditInsert` (`:223`, `:233`, `:249`, `:286`, `:302`), so it lands in the audit trail for `validadoras`, `validadoras_historial` and `validadoras_historial_ubicaciones`, not just in `app_logs`. ### Finding 2 — a figure to reconcile on the parent item, not a defect of this diff The action-mint gap is already tracked: 739's title names "21 generateUUID sites in 9 route files" and its description adds "three function-entry mints", and coder04's own 20:23:45Z note explicitly disclaims closing the action mints. So this is not news. **But my count does not match the title's.** Measured just now across `src/actions`, `src/services`, `src/repositories` and `src/app/api/log-error/route.ts`: `actions/asociaciones.ts` 9, `actions/validadoras.ts` 8, `actions/choferes.ts` 8, `actions/vehiculos.ts` 7, `actions/lineas.ts` 3, `actions/instalaciones.ts` 3, `actions/reportes.ts` 1, `actions/personas.ts` 1, `actions/entidades.ts` 1, `app/api/log-error/route.ts` 1 — **41 mints in 10 files**, against the title's 21 in 9. I am not claiming the title is wrong: "route files" may be a narrower universe than `src/actions/*.ts`, and the title's figure is mine from the 732 PASS, measured on a different denominator. The two numbers need one definition before either is cited again. Why it matters for how the slice is reported: `logger.ts:129` resolves `requestId = passedRequestId || ctx.requestId || randomUUID()`, so **an explicitly passed id wins and becomes `app_logs.request_id`**, with the context id demoted to `data.transportRequestId` when they differ (`:130-132`). The action layer is therefore where "one request, one id" is won or lost, and converting services cannot deliver it while their action callers pass a per-action mint. The service conversions are correct and are a prerequisite; they are not the outcome. This applies equally to the entidades, carrocerias and notes slices I already passed, so it belongs on the parent item as a scope note covering all of them. ## NOT EXERCISED - **All three gates**, for both items: `lint` on the selected files, `typecheck`, `test:source` — not run, because the revisions are not in the tree. Owed when the park returns. `test:tools` and `next build` also unrun by me; both deploy scripts run `test:tools` and the prod script runs `next build`, so the ship paths cover those two. - **No runtime witness for either item.** Nothing opened in a browser: not the change-password page, not the link, not `/reset-password`; no validadora create/edit performed. The AD login is Elazar's. - **The grouped `app_logs` control for 739 is still owed**, still blocked on that AD session. Note that by finding 2 the validadora path would fail that control today regardless of this diff, because its actions still mint. - Finding 1's divergence case (context unavailable at `validadoras-vehiculo.ts:179`) is exercised by nothing. - I did not re-audit the already-passed entidades/carrocerias/notes slices; finding 2's applicability to them is stated from the same code path, not re-measured per file.
  • audit-mba-l-cc audited · 1w ago
    audit pass: 328: src/app/auth/change-password/page.tsx e9eb7f45, the /reset-password link hunk, checked against the item acceptance text and its restriction. Gates run by audit under lock on the restored tree. NOT COVERED: no runtime witness - page, link and /reset-password never opened; AD login is Elazar.
    payload
    {"scope":"328: src/app/auth/change-password/page.tsx e9eb7f45, the /reset-password link hunk, checked against the item acceptance text and its restriction. Gates run by audit under lock on the restored tree. NOT COVERED: no runtime witness - page, link and /reset-password never opened; AD login is Elazar.","outcome":"pass","evidence":"Gates, run by me under gates-checkout-movilba-web on the restored tree after re-hashing the file to e9eb7f45: eslint exit 0, pnpm typecheck exit 0, pnpm test:source 66/66, pnpm test:tools 116/116; hash identical before and after. Checked against the ITEM, not only the diff. The item asks for a visible link to the reset-mail flow for a user with no current password to type, and carries my own non-negotiable restriction of 2026-09-07: do not remove the currentPassword field or the server requirement, because for a merely session-authenticated caller that turns a stolen session into full account takeover. The diff honours it exactly - one paragraph with an anchor to /reset-password inserted before the form, field and bcrypt.compare untouched. Four checks: (1) the target exists at src/app/reset-password/page.tsx and there is no src/app/auth/reset-password, so the absolute path is right; (2) THE REAL RISK, a forced-change user can actually reach it - src/proxy.ts:303 carries the negated startsWith /reset-password inside the redirect guard, carved out by MOVILBA-323 for this exact case per the comment at :294-297, so the destination is not a dead end; (3) the copy promises requesting the mail and the page delivers it - reset-password/page.tsx:30 starts step at request and only switches to reset when a token param is present at :32-38, so arriving without a token renders the email form at :158-177; (4) the class login-forgot-link exists at globals.scss:1486 with its hover at :1492, and the same anchor construct is already in LoginForm.tsx:301, so the anchor-not-Link full page load matches this surface. ONE DELIBERATE DEVIATION, acceptable: the item says show the link when the user has no current password to offer, and the hunk renders it unconditionally. The page cannot know whether the user remembers the password, and an unconditional link never hides the escape hatch, so it is a safer superset of what was asked. Recorded, not a finding.","reportRef":null}
  • coder03-mba-l-cc completed · 1w ago
    completed
bug
5w ago by wi-cli-venus
1w ago
2026-10-08 20:50