Skip to content

Include authentication in generated OpenAPI specs. Closes #1147 - #2028

Open
waldekmastykarz wants to merge 6 commits into
dotnet:mainfrom
waldekmastykarz:openapi-auth-generation
Open

waldekmastykarz wants to merge 6 commits into
dotnet:mainfrom
waldekmastykarz:openapi-auth-generation

Conversation

@waldekmastykarz

Copy link
Copy Markdown
Collaborator

Closes #1147

Summary

  • Moves auth detection out of TypeSpecGeneratorPlugin into a shared Generation/AuthDetector.cs. TypeSpec output is unchanged; the new TypeSpec auth tests pass on both the old and the new code.
  • OpenApiSpecGeneratorPlugin now writes components.securitySchemes and per-operation security:
    • API key in a header or the query string → apiKey. Authorization: Basic … is also treated as an API key header, the same way TypeSpec handles it.
    • Opaque Bearer token → http / bearer
    • Entra ID JWT → oauth2 (EntraOAuth2Auth). It uses an authorizationCode flow when the token has scp and clientCredentials when it has roles. Token, authorize and refresh URLs follow the token version (v1 or v2).
    • Other JWT issuers → oauth2 (APIOAuth2Auth)
  • Scopes are combined across recorded requests. If one operation was called with different kinds of auth, each kind is listed as an alternative requirement.
  • A detected API key query parameter is no longer emitted as a regular parameter, so its value can't end up in the spec.

Tests

GenerationAuthIntegrationTests covers each auth type, scope merging, alternative requirements, no auth, v2.0 output (securityDefinitions) and TypeSpec parity. The full test suite passes.

Note (not addressed here)

Query, header and path parameters and response headers are silently dropped from generated OpenAPI specs. The plugin adds them with operation.Parameters?.Add(...) and similar calls, and those collections are null by default in Microsoft.OpenApi 3.x, so nothing gets added.

Extract auth detection from TypeSpecGeneratorPlugin into a shared
AuthDetector and use it in OpenApiSpecGeneratorPlugin to emit
components.securitySchemes and operation security requirements for
API keys (header/query), bearer tokens, and OAuth2 (Entra ID and
generic JWT issuers). Scopes are merged across recorded requests and
detected API key query parameters are no longer emitted as regular
parameters. TypeSpec output is unchanged.

Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Copilot AI balanced review requested due to automatic review settings October 31, 2026 09:03
@waldekmastykarz
waldekmastykarz requested a review from a team as a code owner October 31, 2026 09:03

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟡 Changes recommended

Unresolved defects in OAuth endpoints, token-flow detection, and security merging can produce unusable authentication metadata and incorrectly require authentication for operations that allow anonymous access.

6 open issues
What changed in this PR

Adds authentication metadata to generated OpenAPI specifications for #1147, reusing the authentication detection previously embedded in the TypeSpec generator.

Changes:

  • Extracts shared detection for API keys, bearer tokens, and JWT-based authentication.
  • Adds OpenAPI security schemes and operation requirements, merges scopes, and excludes detected query API keys from regular parameters.
  • Adds integration coverage for authentication output, OpenAPI 2.0 serialization, and TypeSpec parity.
File Description
DevProxy.Plugins/​Generation/​TypeSpecGeneratorPlugin.cs Uses the shared detector while preserving existing authentication mappings.
DevProxy.Plugins/​Generation/​OpenApiSpecGeneratorPlugin.cs Generates and merges authentication schemes and operation requirements.
DevProxy.Plugins/​Generation/​AuthDetector.cs Centralizes authentication detection for both generators.
DevProxy.Integration.Tests/​GenerationAuthIntegrationTests.cs Tests authentication generation and TypeSpec parity.

Review effort: Balanced

- Normalize Entra v1 issuer to login.microsoftonline.com endpoints
- Prefer authorizationCode flow when token has scp claim
- Emit separate requirements per OAuth2 flow
- Mark auth optional when successful anonymous calls were recorded
- Emit http bearer (bearerFormat JWT) for non-Entra JWTs

Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟡 Changes recommended

Authentication detection records only one mechanism when a request carries both bearer and API-key credentials, causing generated clients to omit a required credential and fail to authenticate.

2 open issues
2 previously missed issues

In code that hasn't changed since last review

Medium severity Recognize api_key as an API-key query parameter

DevProxy.Plugins/​Generation/​AuthDetector.cs:96

The header-name whitelist does not contain api_key, so a request such as /users?api_key=secret-value is classified as anonymous and receives no API-key security scheme. This conventional query spelling is already recognized in Models.Http.SensitiveQueryParameters, but only api-key and apikey are in AuthHeaders. Recognize the underscore spelling here as well so API-key query authentication is not omitted.

Medium severity Keep OAuth roles separate from scopes and use /.default for client credentials

DevProxy.Plugins/​Generation/​AuthDetector.cs:142

Appending roles makes application roles appear as requestable OAuth scopes: the mixed-claims test emits Admin alongside User.Read, which can make authorization fail with invalid_scope. Entra v2 client-credentials requests likewise need the resource's /.default scope rather than individual role names. Keep roles separate from delegated scp values, resolve the appropriate resource scope for client credentials, and update the affected test expectations.

6 resolved since last review

Review effort: Balanced

- Don't treat cookie-carrying requests as anonymous
- Recognize api_key as an API key name

Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
@waldekmastykarz

Copy link
Copy Markdown
Collaborator Author

Re: the two "previously missed" issues from the latest review:

  • Recognize api_key as an API-key query parameter: fixed in eb6b0c4. api_key is now a known auth name, used by both generators. Test added.
  • Keep OAuth roles separate from scopes / use /.default for client credentials: keeping as-is. The generated spec documents the permissions observed on recorded calls, not the exact parameters for a token request. This also matches how the TypeSpec generator has always reported scp and roles.

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🔵 Needs a closer look

Issuer substring matching can advertise an attacker-controlled OAuth token endpoint, potentially exposing consumers’ client secrets.

No open issues 🎉

3 previously missed issues

In code that hasn't changed since last review

High severity Validate Entra issuer host before generating token endpoints

DevProxy.Plugins/​Generation/​AuthDetector.cs:169

Substring matching accepts an issuer such as https://attacker.example/https://login.microsoftonline.com/tenant/v2.0 as Entra, causing the generated OpenAPI flow to advertise a token endpoint on attacker.example. A consumer using that client-credentials flow could send its client secret to that host. Parse the issuer and require HTTPS with an exact supported hostname; otherwise use the existing bearer fallback.

Medium severity Keep application roles out of OAuth scope definitions

DevProxy.Plugins/​Generation/​AuthDetector.cs:142

Appending roles to Scopes makes the generated OpenAPI flow advertise application roles as requestable OAuth scopes. For example, the delegated-token test advertises Admin alongside User.Read, which can cause clients requesting those scopes to receive invalid_scope. Keep roles separate from OAuth scope dictionaries and requirements; use scp for delegated flows and the resource's /.default scope for v2 client credentials, falling back to bearer auth when the resource cannot be determined.

Medium severity Use separate Swagger 2.0 schemes for delegated and client-credentials flows

DevProxy.Plugins/​Generation/​OpenApiSpecGeneratorPlugin.cs:646

When targeting v2_0, recording both delegated and application tokens puts both OAuth flows under one security-scheme name, but Swagger 2.0 permits only one flow per security definition. At least one recorded alternative therefore loses its matching flow/scopes while both requirements still reference EntraOAuth2Auth. Give client credentials a separate scheme name for v2 output and add a v2 integration test covering both flows.

2 resolved since last review

Review effort: Balanced

- Require HTTPS and an exact Entra host when detecting Entra issuers
- Use a separate client credentials scheme for Swagger 2.0 output

Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
@waldekmastykarz

Copy link
Copy Markdown
Collaborator Author

Re: the "previously missed" issues from the latest review:

  • Validate Entra issuer host: fixed in a660e96. The issuer must now be HTTPS with an exact login.microsoftonline.com or sts.windows.net host, otherwise it falls back to bearer auth (both generators). Tests added.
  • Separate Swagger 2.0 schemes for delegated and client-credentials flows: fixed in a660e96. For v2_0 output, client credentials use a separate EntraOAuth2Auth_clientCredentials scheme. v2 test added.
  • Keep application roles out of OAuth scopes: same as the earlier comment. Keeping as-is, because the spec documents the permissions observed on recorded calls, consistent with the TypeSpec generator.

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🔵 Needs a closer look

Human assessment is needed because role-to-scope mapping and missed query access tokens can produce invalid OAuth scopes and incorrectly mark authentication as optional.

No open issues 🎉

Review effort: Balanced

Use a dedicated list of auth query parameters (access_token, api_key,
apikey, code, key, sig, subscription-key) so that query-authenticated
calls aren't treated as anonymous.

Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
@waldekmastykarz

Copy link
Copy Markdown
Collaborator Author

Re: the latest review summary (no inline threads):

  • Missed query access tokens: fixed in 169e59a. Query auth detection now uses a dedicated list (access_token, api_key, apikey, code, key, sig, subscription-key) instead of the header list. A call such as ?access_token=... is no longer treated as anonymous, so it no longer makes auth optional for an operation that is also called with a bearer token. Tests added, including TypeSpec.
  • Role-to-scope mapping: keeping as-is, as discussed earlier. The spec documents the permissions observed on recorded calls, consistent with the TypeSpec generator.

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟡 Changes recommended

The query credential detection drops previously supported names such as api-key, causing TypeSpec to omit authentication and OpenAPI to potentially mark it optional.

1 open issue

Review effort: Balanced

Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🔵 Needs a closer look

Human review is needed to assess the authentication-flow implications of treating application roles as requestable OAuth scopes, which can cause invalid_scope failures.

No open issues 🎉

1 resolved since last review

Review effort: Balanced

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Feature Request - Open API Spec generation - include Authentication

2 participants