-
Notifications
You must be signed in to change notification settings - Fork 318
fix(cli): preserve per-endpoint security when auth-schemes defined without auth key #16037
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Changes from all commits
5c5f13e
fcb720a
1de933f
c47a2b7
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,6 @@ | ||
| - summary: | | ||
| Fix auth-schemes without explicit auth key to preserve per-endpoint security | ||
| from OpenAPI specs instead of merging all schemes into a single global group. | ||
| This ensures OAuth client-credentials and bearer auth schemes render as | ||
| distinct options in the API playground for dual-auth endpoints. | ||
| type: fix |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -317,12 +317,16 @@ export class OSSWorkspace extends BaseOpenAPIWorkspace { | |
| const documents = await this.loader.loadDocuments({ context, specs }); | ||
|
|
||
| let authOverrides: RawSchemas.WithAuthSchema | undefined = | ||
| this.generatorsConfiguration?.api?.auth != null ? { ...this.generatorsConfiguration?.api } : undefined; | ||
| this.generatorsConfiguration?.api?.auth != null || | ||
| this.generatorsConfiguration?.api?.["auth-schemes"] != null | ||
| ? { ...this.generatorsConfiguration?.api } | ||
| : undefined; | ||
|
|
||
| // Fallback: read auth/auth-schemes from the spec's overrides file if not in generators.yml | ||
| if (authOverrides == null) { | ||
| authOverrides = await getAuthFromOverrideFiles(specs); | ||
| } | ||
|
|
||
| const environmentOverrides = | ||
| this.generatorsConfiguration?.api?.environments != null | ||
| ? { ...this.generatorsConfiguration?.api } | ||
|
|
@@ -567,19 +571,21 @@ export class OSSWorkspace extends BaseOpenAPIWorkspace { | |
| return this.createWorkspaceWithSpecsOverride({ context }, specsOverride, settings); | ||
| } | ||
|
|
||
| // If auth is not in generators.yml and not in settings, try to read it from the spec's overrides files | ||
| // If auth is not in generators.yml and not in settings, try to read it from the spec's overrides files. | ||
| // When only auth-schemes is in generators.yml (no auth key), still check override files for auth. | ||
| let effectiveSettings = settings; | ||
| if (this.generatorsConfiguration?.api?.auth == null && settings?.auth == null) { | ||
| const specs = await this.getOpenAPISpecsCached({ context }); | ||
| const authFromOverrides = await getAuthFromOverrideFiles(specs); | ||
| if (authFromOverrides != null) { | ||
|
Comment on lines
576
to
580
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🔴 When Extended reasoning...The bug: The new condition in Code path that triggers it:
Why other code doesn't save us: The companion change in Impact: Silent data loss for a legitimate split configuration. Generated SDKs will be missing the auth specified in the override file. The previous behavior (before this PR) correctly picked up Step-by-step proof: api:
auth-schemes:
myBearer:
scheme: bearer
auth: myBearer
Suggested fix: Don't conflate
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Fixed in fcb720a. The guard now only requires |
||
| const hasAuthSchemesInGenerators = this.generatorsConfiguration?.api?.["auth-schemes"] != null; | ||
| effectiveSettings = { | ||
| ...settings, | ||
| auth: authFromOverrides.auth as RawSchemas.ApiAuthSchema, | ||
| authSchemes: authFromOverrides["auth-schemes"] as Record< | ||
| string, | ||
| RawSchemas.AuthSchemeDeclarationSchema | ||
| > | ||
| // Only use override file's auth-schemes if generators.yml doesn't already define them | ||
| authSchemes: hasAuthSchemesInGenerators | ||
| ? undefined | ||
| : (authFromOverrides["auth-schemes"] as Record<string, RawSchemas.AuthSchemeDeclarationSchema>) | ||
| }; | ||
| } | ||
| } | ||
|
|
@@ -847,7 +853,7 @@ async function getAuthFromOverrideFiles(specs: Spec[]): Promise<RawSchemas.WithA | |
| try { | ||
| const contents = (await readFile(overridePath)).toString(); | ||
| const parsed = yaml.load(contents) as Record<string, unknown> | null | undefined; | ||
| if (parsed != null && parsed["auth"] != null) { | ||
| if (parsed != null && (parsed["auth"] != null || parsed["auth-schemes"] != null)) { | ||
| return { | ||
| auth: parsed["auth"] as RawSchemas.WithAuthSchema["auth"], | ||
| ...(parsed["auth-schemes"] != null | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🔴 AsyncAPIConverter has no parallel handling for the new
auth-schemes-without-authcase enabled by this PR's OSSWorkspace change, so any workspace combining AsyncAPI specs withauth-schemes-only config in generators.yml (or override files viagetAuthFromOverrideFiles) will have its AsyncAPI security schemes silently wiped from the IR. Mirror the synthesis logic added toOpenAPIConverter.convertSecuritySchemes()inAsyncAPIConverter.convertSecuritySchemes()(synthesizeauth: { any: schemeNames }when onlyauth-schemesis present), or do the synthesis once inOSSWorkspacebefore handing off to converters.Extended reasoning...
What the bug is
The PR's change at
OSSWorkspace.ts:317-323now populatesauthOverrideswhenevergenerators.ymlhasauth-schemes, even with noauthkey. The sameauthOverridesvalue is forwarded to both converters:OpenAPIConverterContext3_1andAsyncAPIConverterContext(OSSWorkspace.ts:373andOSSWorkspace.ts:400respectively). The PR also updatesOpenAPIConverter.convertSecuritySchemes()to handle this new shape by synthesizingauth: { any: schemeNames }, butAsyncAPIConverter.convertSecuritySchemes()(packages/cli/api-importers/asyncapi-to-ir/src/AsyncAPIConverter.ts:166-189) was not given the analogous treatment.The code path that triggers it
convertApiAuth(packages/commons/ir-utils/src/auth/convertApiAuth.ts:19-25) short-circuits whenrawApiFileSchema.auth == null, returning{ requirement: All, schemes: [] }. SoaddAuthToIRwrites empty schemes and the function returns beforeconvertAsyncApiSecuritySchemes()can parse the AsyncAPI spec's owncomponents.securitySchemes.Why existing code does not prevent it
Pre-PR, the guard in
OSSWorkspacewasthis.generatorsConfiguration?.api?.auth != null, so this exact scenario (auth-schemesonly) leftauthOverridesundefined andAsyncAPIConverterfell through toconvertAsyncApiSecuritySchemes(). The new guardauth != null || "auth-schemes" != nullflips that, but onlyOpenAPIConverterwas updated to compensate.Impact
Any workspace combining at least one AsyncAPI spec with a
generators.ymlthat definesauth-schemes(and no top-levelauth) will have all of its AsyncAPI security wiped from the IR with no error. The same regression applies to the override-file path:getAuthFromOverrideFiles(OSSWorkspace.ts:854) was extended in this PR to return anauthOverridesobject containing onlyauth-schemes, so the same wiping triggers for AsyncAPI specs whose overrides file defines onlyauth-schemes. In mixed OpenAPI+AsyncAPI projectsmergeIntermediateRepresentation(mergeIntermediateRepresentation.ts:23-30) takes the longer schemes array, so OpenAPI auth can mask the regression — but AsyncAPI-only projects (or AsyncAPI-namespaced sub-APIs) will ship SDKs with no auth.Step-by-step proof
Given:
and an
events.ymlAsyncAPI document with:OSSWorkspace.getIntermediateRepresentationenters the new branch at line 319-322 becauseapi["auth-schemes"] != null.authOverrides = { "auth-schemes": { apiKey: {...} } }(noauthkey).case "asyncapi":branch at line 391-411;authOverridesis passed intoAsyncAPIConverterContextatOSSWorkspace.ts:400.AsyncAPIConverter.convert()callsconvertSecuritySchemes(). Line 167if (this.context.authOverrides)is truthy.convertApiAuth({ rawApiFileSchema: { "auth-schemes": {...} } })hits the guard atconvertApiAuth.ts:19-25becauserawApiFileSchema.auth == nulland returns{ docs: undefined, requirement: All, schemes: [] }.addAuthToIR({ requirement: All, schemes: [] })is called, thenreturnat line 178 —convertAsyncApiSecuritySchemes()(which would have produced anAuthSchemeforapiKeyfrom the spec'scomponents.securitySchemes) is never invoked.auth.schemesand the generated SDK has no auth wiring for the AsyncAPI events.Pre-PR, step 1 would not have populated
authOverrides(guard requiredauth != null), so step 3'sifwould be falsy and step 5 would correctly read the scheme from the AsyncAPI spec.Fix
Mirror the
OpenAPIConvertersynthesis inAsyncAPIConverter.convertSecuritySchemes()so that whenauthOverrides.auth == nullbutauthOverrides["auth-schemes"]is non-empty, it synthesizesauth: { any: Object.keys(authOverrides["auth-schemes"]) }and passes that intoconvertApiAuth. Alternatively, do the synthesis once inOSSWorkspacebefore handingauthOverridesto either converter so the two stay in sync without duplication.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Fixed in fcb720a. Added the same synthesis logic (
auth: { any: schemeNames }when onlyauth-schemesis present) toAsyncAPIConverter.convertSecuritySchemes(), mirroring whatOpenAPIConverteralready does.