Repository navigation
Conversation
…cation APIs `POST /api/applications` and `PATCH /api/applications/:id` accepted any key under `customClientMetadata` and silently dropped the unknown ones, so a typo such as `refreshTokenTtlInDay` was answered 200 while the setting was never applied. The API input guard is now `.strict()` on that object, which yields a 400 `guard.invalid_input` naming the key. The row guard in `Applications` stays lenient so legacy rows that still carry removed keys keep parsing (and koaGuard already strips them from responses, so Console never echoes such a key back). Closes logto-io#9885 Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
| .extend({ | ||
| /** | ||
| * Strict on the API input only: an unknown key (e.g. a typo such as `refreshTokenTtlInDay`) | ||
| * is rejected with a 400 instead of being silently stripped and answered 200. The row guard | ||
| * in `Applications` stays lenient so that legacy rows carrying removed keys still parse. | ||
| */ | ||
| customClientMetadata: customClientMetadataGuard.strict().optional(), |
There was a problem hiding this comment.
Since this input is now .strict(), the set of keys in customClientMetadataGuard becomes part of the API contract. Could you add a note for future maintainers, ideally right on customClientMetadataGuard in packages/schemas/src/foundations/jsonb-types/oidc-module.ts, since that's where keys get added and removed:
- Never remove or rename an existing key. A client that still sends it would go from 200 to 400. Keep it and mark it
@deprecatedinstead, the wayrefreshTokenTtlis handled. - New keys must be
.optional(). A required key would reject every existing create/update payload that sendscustomClientMetadatawithout it, and PATCH replaces the whole object.
Also, the last sentence here ("legacy rows carrying removed keys still parse") reads as though removing keys is a supported path, which is the opposite of what we want now. Could you reword it to point at the rule above?
There was a problem hiding this comment.
Done in 9e729a1. The two rules (never remove or rename a key, mark it @deprecated like refreshTokenTtl; new keys must be .optional() since PATCH replaces the whole object) now sit in a doc comment right on customClientMetadataGuard, and the comment on applicationCreateGuard no longer mentions legacy rows: it says the keys are part of the API contract and points to those rules.
|
|
||
| reject unknown `customClientMetadata` keys on the application APIs | ||
|
|
||
| `POST /api/applications` and `PATCH /api/applications/:id` used to accept any key under `customClientMetadata` and silently drop the ones they did not know, so a typo such as `refreshTokenTtlInDay` was answered 200 while the setting was never applied. An unknown key is now rejected with a 400 `guard.invalid_input` that names it. Stored applications are not affected: the row guard still strips removed legacy keys on read. |
There was a problem hiding this comment.
This goes into the user-facing changelog, so I'd drop the row-guard wording (no key has ever been removed from this guard anyway). Also, the guard.invalid_input message itself is generic; the key name is in the error data. Suggest:
An unknown key is now rejected with a 400
guard.invalid_inputerror whosedatalists the key. Existing applications are not affected.
|
The generated OpenAPI spec won't show that |
|
Thanks for the review. OpenAPI: added a |
Summary
Closes #9885 (labelled
contribution-welcome).POST /api/applicationsandPATCH /api/applications/:idacceptcustomClientMetadatathrough a plainz.object(...), which strips unknown keys. A misspelled or unsupported key was therefore answered 200 and silently dropped: a typo inrefreshTokenTtlInDaysfrom a script looked applied while the sessions kept the default TTL.This PR makes the API input guard strict on that object. An unknown key is now rejected with a 400
guard.invalid_inputwhose message names the key:{ "code": "guard.invalid_input", "message": "...", "data": [{ "code": "unrecognized_keys", "keys": ["refreshTokenTtlInDay"], "path": ["customClientMetadata"] }] }Why only the API guard
customClientMetadataGuardis also the row guard (Applications.guard/createGuard) and is used byoidc/utils.tsthrough.shape. Making it strict there would make a stored application with a removed legacy key fail to parse, so the override lives inapplicationCreateGuard(packages/schemas/src/types/application.ts), whichapplicationPatchGuardderives from. Same reasoning as the comment onpasswordExpirationPolicyGuard.koaGuardoverwrites responses with the parsed value, so a legacy key stored in a row is never echoed back to a client that would then re-send it onPATCH.pickonlyname,descriptionandcustomData, so they are untouched.Tests
application.test.ts: an unknown key is rejected on create and on update, and the update leaves the stored value unchanged.@logto/schemas: create/patch withrefreshTokenTtlInDay⇒unrecognized_keys, patch with a valid key ⇒ ok, row guard with an extra legacy key ⇒ still parses.tscforschemas,coreandintegration-tests,eslintandprettierare clean on the changed files.Changeset
@logto/schemas+@logto/corepatch: callers that relied on the lenient behaviour now get a 400 instead of a silent drop, so this is called out as a behaviour change.🤖 Generated with Claude Code