Deprecate ctx based security scopes#2440
Conversation
Generated server code no longer embeds security scheme scopes into the
request context. The per-scheme context key types (e.g.
`bearerAuthContextKey`), scope constants (e.g. `BearerAuthScopes`), and
per-operation context stores flattened the spec's `security`
requirements and could not represent alternative (OR), combined (AND),
or anonymous (`{}`) schemes. Authentication should instead use the
request validation middleware, which evaluates the spec's security
requirements directly. See oapi-codegen#1524.
- Add `compatibility.enable-auth-scopes-on-context` (default off) to
restore the legacy emission, and document it in the README and
configuration schema
- Gate all emission sites on the new option: security scheme context
key types, scope constants, and the context stores in the chi,
gorilla, stdhttp, echo, echo/v5, iris, fiber, and gin templates
- Opt the fiber middleware fixture into the option so the issue-518
anonymous-alternative regression stays meaningful; its generated
output is unchanged
- Add TestEnableAuthScopesOnContext covering both option states
- Regenerate; affected fixtures and examples lose only the now-dead
scope artifacts. The authenticated-api examples already authenticate
via the validation middleware and are otherwise unaffected
Greptile SummaryThis PR deprecates the legacy security-scope-on-context emission pattern — where generated server code embedded per-operation scopes into the request context via typed context keys — and gates it behind a new
Confidence Score: 4/5Safe to merge; the change removes generated boilerplate that was genuinely incorrect and provides a clean opt-in path for users who depend on it. The implementation is thorough — all seven backend templates, the type-generation path in codegen.go, and the scope-constant template are all consistently gated. Configuration struct and JSON schema are kept in sync. The only gap is that the new test only exercises the stdhttp backend; the structurally different guard pattern used in echo/gin/iris/fiber templates is not independently verified. pkg/codegen/codegen_test.go — TestEnableAuthScopesOnContext covers only StdHTTPServer; echo, chi, gin, gorilla, iris, and fiber template guards go untested.
|
| Filename | Overview |
|---|---|
| pkg/codegen/codegen.go | Gates GenerateTypesForSecuritySchemes on EnableAuthScopesOnContext; err variable reuse is valid; logic is clean. |
| pkg/codegen/configuration.go | Adds EnableAuthScopesOnContext bool with yaml tag and full doc comment; consistent with existing CompatibilityOptions fields. |
| configuration-schema.json | JSON schema updated to include enable-auth-scopes-on-context alongside configuration.go changes; satisfies rule 2. |
| pkg/codegen/codegen_test.go | Adds TestEnableAuthScopesOnContext covering both option states for StdHTTPServer only; other backends' template changes are untested. |
| pkg/codegen/templates/constants.tmpl | Correctly gates the scope-constant block on both SecuritySchemeProviderNames presence and EnableAuthScopesOnContext flag. |
| pkg/codegen/templates/chi/chi-middleware.tmpl | Guards context.WithValue emission with and .SecurityDefinitions opts.Compatibility.EnableAuthScopesOnContext; consistent with gorilla/stdhttp pattern. |
| pkg/codegen/templates/echo/echo-wrappers.tmpl | Wraps range over SecurityDefinitions in an outer if on EnableAuthScopesOnContext; functionally equivalent to chi/stdhttp pattern. |
| internal/test/servers/middleware/fiber/config.yaml | Opts fiber middleware fixture into legacy scope emission so the issue-518 anonymous-alternative regression stays meaningful. |
| README.md | Documents the deprecation, explains the limitation of the old mechanism, and shows the opt-in YAML snippet. |
Prompt To Fix All With AI
Fix the following 1 code review issue. Work through them one at a time, proposing concise fixes.
---
### Issue 1 of 1
pkg/codegen/codegen_test.go:493-514
**Test coverage limited to a single backend**
`TestEnableAuthScopesOnContext` only exercises `StdHTTPServer`. All seven backend templates (chi, echo, echo/v5, fiber, gin, gorilla, iris) had guard changes in this PR, and any future template-level regression (e.g., the `if`/`range` ordering in `echo-wrappers.tmpl`) would go undetected. At minimum, adding `EchoServer: true` as a second sub-test case (reusing the same spec) would catch divergence between the chi/gorilla/stdhttp pattern (`if and … opts.Compatibility`) and the echo/gin/iris/fiber pattern (outer `if opts.Compatibility`, inner `range`) that are structurally different in the templates.
Reviews (1): Last reviewed commit: "Deprecate ctx based security scopes" | Re-trigger Greptile
| opts := Configuration{ | ||
| PackageName: "api", | ||
| Generate: GenerateOptions{ | ||
| StdHTTPServer: true, | ||
| Models: true, | ||
| }, | ||
| } | ||
|
|
||
| // By default, no context key types, scope constants, or context values | ||
| // are generated. | ||
| code, err := Generate(swagger, opts) | ||
| require.NoError(t, err) | ||
| assert.NotContains(t, code, "BearerAuthScopes") | ||
| assert.NotContains(t, code, "bearerAuthContextKey") | ||
| assert.NotContains(t, code, "context.WithValue") | ||
|
|
||
| // With the compatibility option set, the legacy scope emission returns. | ||
| opts.Compatibility.EnableAuthScopesOnContext = true | ||
| code, err = Generate(swagger, opts) | ||
| require.NoError(t, err) | ||
| assert.Contains(t, code, `BearerAuthScopes bearerAuthContextKey = "bearerAuth.Scopes"`) | ||
| assert.Contains(t, code, `ctx = context.WithValue(ctx, BearerAuthScopes, []string{"read"})`) |
There was a problem hiding this comment.
TestEnableAuthScopesOnContext only exercises StdHTTPServer. All seven backend templates (chi, echo, echo/v5, fiber, gin, gorilla, iris) had guard changes in this PR, and any future template-level regression (e.g., the if/range ordering in echo-wrappers.tmpl) would go undetected. At minimum, adding EchoServer: true as a second sub-test case (reusing the same spec) would catch divergence between the chi/gorilla/stdhttp pattern (if and … opts.Compatibility) and the echo/gin/iris/fiber pattern (outer if opts.Compatibility, inner range) that are structurally different in the templates.
Prompt To Fix With AI
This is a comment left during a code review.
Path: pkg/codegen/codegen_test.go
Line: 493-514
Comment:
**Test coverage limited to a single backend**
`TestEnableAuthScopesOnContext` only exercises `StdHTTPServer`. All seven backend templates (chi, echo, echo/v5, fiber, gin, gorilla, iris) had guard changes in this PR, and any future template-level regression (e.g., the `if`/`range` ordering in `echo-wrappers.tmpl`) would go undetected. At minimum, adding `EchoServer: true` as a second sub-test case (reusing the same spec) would catch divergence between the chi/gorilla/stdhttp pattern (`if and … opts.Compatibility`) and the echo/gin/iris/fiber pattern (outer `if opts.Compatibility`, inner `range`) that are structurally different in the templates.
How can I resolve this? If you propose a fix, please make it concise.Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!
Generated server code no longer embeds security scheme scopes into the request context. The per-scheme context key types (e.g.
bearerAuthContextKey), scope constants (e.g.BearerAuthScopes), and per-operation context stores flattened the spec'ssecurityrequirements and could not represent alternative (OR), combined (AND), or anonymous ({}) schemes. Authentication should instead use the request validation middleware, which evaluates the spec's security requirements directly. See #1524.compatibility.enable-auth-scopes-on-context(default off) to restore the legacy emission, and document it in the README and configuration schema