fix(openapi): improve spec quality for SDK generation#2620
Draft
grdsdev wants to merge 6 commits into
Draft
Conversation
- Add operationId to every operation (59 total) so SDK generators
produce readable method names instead of path-derived ones.
- Fix /user/identities/{identityId} 401/403 responses that
incorrectly $ref'd a Response object from inside `schema:` instead
of ErrorSchema.
- Add discriminator to the /factors/{factorId}/challenge response
oneOf so typed SDKs can pick the right variant.
Legacy endpoints documented lowercase "s256" while /oauth/authorize documented uppercase "S256" for the same parameter. Server parses this case-insensitively (ParseCodeChallengeMethod, oauthserver authorize.go) and the discovery document already advertises "S256", so standardize all docs on that casing.
POST /admin/oauth/clients and POST /oauth/clients/register carried a byte-identical inline request schema. Extract it into OAuthClientCreateRequest and $ref it from both. The PUT update body stays inline since its shape genuinely differs (no client_type, nothing required).
- Replace invalid patternProperties (SAMLAttributeMappingSchema.keys, /settings external) with additionalProperties. - Drop invalid `optional: true` keyword on /settings sms_provider (properties are optional by default unless in `required`). - Add missing `security: [APIKeyAuth]` to POST /callback, POST /oauth/clients/register, POST /oauth/token, GET /oauth/authorize — all were oversights vs. sibling operations that already declare it; none of these bypass the gateway apikey requirement.
Verified each flagged operation against its Go handler before touching
anything -- only added responses that are real, and left redirect-only
flows alone rather than fabricating a fake 2xx.
Genuine gaps fixed:
- GET /verify, GET /authorize: add 400 (GetExternalProviderRedirectURL/
verifyTokenHash return real validation errors); /authorize also gets
404 for a missing invite_token.
- GET /user: add 401 (UserAuth-guarded, was undocumented).
- GET /user/identities/authorize: LinkIdentity actually returns a 200
JSON {url} when skip_http_redirect=true (mirrors the existing /sso
pattern) but that query param and response were never documented.
Added both, plus 400/401.
- GET /admin/sso/providers: add 401/403 to match every sibling admin
list endpoint.
For the remaining 11 warnings the operations genuinely have no 2xx or
no 4xx path (redirect-only success, or no rejectable input) -- added
.redocly.lint-ignore.yaml with comments explaining why each is
intentional instead of inventing fake responses.
…est schemas Each of these endpoints crammed every grant-type/verification-type/ factor-type's fields into one flat, all-optional object, so generated SDKs got a single loosely-typed request struct instead of a real choice of shapes. Split each into named request schemas per real Go handler struct/validation (PasswordGrantParams, RefreshTokenGrantParams, IdTokenGrantParams, PKCEGrantParams, Web3GrantParams, VerifyParams, EnrollFactorParams), wired as oneOf. /token and /verify dispatch on grant_type/token_hash-vs-token which live outside (query param) or across (token vs token_hash) the request body, so no discriminator applies there. /factors dispatches on factor_type inside the body, so that one gets a real discriminator. Also: - fixed /token's 401/403 responses, which pointed at the wrong response ref (401 was ForbiddenResponse, 403 was UnauthorizedResponse). - documented link_identity on the id_token grant and issuer's real default on TOTP enrollment -- both real, previously undocumented fields found while reading the handlers.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Series of fixes to
openapi.yamlaimed at improving SDK-generation quality. Verified every change againstredocly lintand, where behavior was in question, against the actual Go handlers ininternal/api.$ref:/user/identities/{identityId}401/403 responses pointedschema:at a Response object instead ofErrorSchema— invalid OpenAPI, now fixed./factors/{factorId}/challengeresponseoneOfso typed SDKs can pick the right variant.code_challenge_methodcasing: unified onS256everywhere (server parses case-insensitively; discovery doc already advertisedS256).POST /admin/oauth/clientsandPOST /oauth/clients/registerhad a byte-identical inline request body, copy-pasted; extracted intoOAuthClientCreateRequest.patternProperties/optionalkeywords (not valid in OpenAPI 3.0) and added missingsecurityblocks to 4 operations that were inconsistent with their siblings.GET /userwas missing 401;GET /user/identities/authorizewas missing an entirely undocumented200response — turns outskip_http_redirect=truereturns real JSON, mirroring/sso, but neither the param nor the response were documented). For operations that generally have no 2xx or 4xx (redirect-only success flows, or endpoints with no rejectable input), added a.redocly.lint-ignore.yamlwith comments instead of fabricating fake responses.oneOfs:/token,/verify, and/factorseach crammed every variant's fields into one all-optional blob. Split into named schemas matching the real Go param structs (PasswordGrantParams,RefreshTokenGrantParams, etc.)./factorsalso gets a realdiscriminatoronfactor_typesince that field lives in the body;/token//verifydon't, since their dispatch key is either a query param (grant_type) or a choice between two body fields (tokenvstoken_hash) — neither fits OpenAPI's discriminator model./token's 401/403 responses which were swapped (401 pointed atForbiddenResponse, 403 atUnauthorizedResponse), and documentedlink_identityon the id_token grant, both real but previously undocumented.redocly lint openapi.yamlpasses clean (0 errors, 0 warnings, 11 explicitly-documented ignores).Not included
CustomOAuthProviderSchema's create/update request bodies look similar but were deliberately left alone — create requires several fields and has real defaults, update is PATCH-style (all optional, no defaults since "absent" means "don't touch"). Merging them would misrepresent one of the two endpoints.Test plan
redocly lint openapi.yaml— 0 errors, 0 warningsredocly bundle— all newoneOf/discriminator schemas resolve correctly