From 24d423610964d067a3301738db9ecf2f664def27 Mon Sep 17 00:00:00 2001 From: Michael McQuade Date: Thu, 20 Aug 2026 22:07:12 -0500 Subject: [PATCH 1/2] fix(generator): an operation declaring security: [] still got the credential Auth was applied in do for every request the client made, and no per-operation security reached the IR, so an endpoint the spec documents as needing no credential received one anyway: sent to that host, written to its logs, and against what the spec plainly said. An empty security requirement now reaches the operation and turns the credential off for it. An absent one is a different declaration and still inherits the document's, so only the explicit opt-out changes. Choosing between several schemes per operation is left alone: the client holds one AuthProvider, and honoring per-operation selection means either several providers or one that knows which scheme it satisfies. The opt-out does not depend on that being settled. --- README.md | 2 +- internal/analyzer/operations.go | 3 + internal/analyzer/security_test.go | 47 +++++++ .../generator/e2e_operation_security_test.go | 116 ++++++++++++++++++ internal/ir/operations.go | 7 +- internal/templates/client.go.tmpl | 8 +- internal/templates/operations.go.tmpl | 4 +- 7 files changed, 179 insertions(+), 8 deletions(-) create mode 100644 internal/generator/e2e_operation_security_test.go diff --git a/README.md b/README.md index c73fdf1..0a9378d 100644 --- a/README.md +++ b/README.md @@ -11,7 +11,7 @@ Given any OpenAPI 3.1 (or 3.0) spec, it outputs a complete, idiomatic Go client - **Webhooks and callbacks**: typed payloads and a dispatcher for the requests the API sends you - **Server URLs**: `DefaultBaseURL` from the spec, with a builder for templated servers - **Response headers**: status and headers captured through the context, with declared headers parsed per operation -- **Authentication** — `AuthProvider` interface with built-in Bearer, API key, and Basic auth +- **Authentication**: `AuthProvider` interface with built-in Bearer, API key, and Basic auth, skipped for operations the spec marks as needing none - **Error handling** — `APIError` with sentinel errors (`errors.Is`), typed error wrappers with parsed response bodies (`errors.As`), readable messages via `x-ms-primary-error-message` - **Pagination**: auto-detected cursor, offset, and page pagination with a generic `PageIterator[T]` - **Retries**: configurable exponential backoff with jitter, honoring `Retry-After` in both forms and declining a wait past `MaxDelay` diff --git a/internal/analyzer/operations.go b/internal/analyzer/operations.go index f5d3f1d..d8b382b 100644 --- a/internal/analyzer/operations.go +++ b/internal/analyzer/operations.go @@ -235,6 +235,9 @@ func (a *Analyzer) convertOperation(httpMethod, path string, pathItem *v3high.Pa Path: path, Tags: op.Tags, Deprecated: op.Deprecated != nil && *op.Deprecated, + // An absent security field inherits the document's; an empty one + // overrides it to say this operation needs no credential. + NoAuth: op.Security != nil && len(op.Security) == 0, } // Merge path-level and operation-level parameters. diff --git a/internal/analyzer/security_test.go b/internal/analyzer/security_test.go index a0f11a4..d1676df 100644 --- a/internal/analyzer/security_test.go +++ b/internal/analyzer/security_test.go @@ -149,3 +149,50 @@ func TestSecuritySchemes_UnsupportedDegradeToAWarning(t *testing.T) { t.Errorf("warnings = %v, want one each for legacy, mtls, and weird", pkg.Warnings) } } + +const operationSecuritySpec = `openapi: 3.1.0 +info: { title: t, version: "1" } +security: + - bearer: [] +paths: + /inherits: + get: + operationId: inherits + responses: { "204": { description: ok } } + /public: + get: + operationId: optsOut + security: [] + responses: { "204": { description: ok } } + /picks: + get: + operationId: picks + security: + - bearer: [] + responses: { "204": { description: ok } } +components: + securitySchemes: + bearer: { type: http, scheme: bearer } +` + +// An absent security field inherits the document's; an empty one overrides it to +// say the operation takes no credential. The two are different declarations and +// only the second one opts out. +func TestOperationSecurity_EmptyRequirementOptsOut(t *testing.T) { + pkg, _ := analyzeSpec(t, operationSecuritySpec) + + byName := make(map[string]*ir.OperationDef, len(pkg.Operations)) + for _, op := range pkg.Operations { + byName[op.Name] = op + } + + if op := byName["Inherits"]; op == nil || op.NoAuth { + t.Errorf("Inherits NoAuth = %v, want false: it declares nothing and inherits", op) + } + if op := byName["OptsOut"]; op == nil || !op.NoAuth { + t.Errorf("OptsOut NoAuth = %v, want true", op) + } + if op := byName["Picks"]; op == nil || op.NoAuth { + t.Errorf("Picks NoAuth = %v, want false: it names a scheme", op) + } +} diff --git a/internal/generator/e2e_operation_security_test.go b/internal/generator/e2e_operation_security_test.go new file mode 100644 index 0000000..84e1bbe --- /dev/null +++ b/internal/generator/e2e_operation_security_test.go @@ -0,0 +1,116 @@ +package generator + +import "testing" + +const operationSecurityAPISpec = `openapi: 3.1.0 +info: { title: sec, version: "1" } +security: + - bearer: [] +paths: + /private: + get: + operationId: getPrivate + responses: { "204": { description: ok } } + /public: + get: + operationId: getPublic + security: [] + responses: { "204": { description: ok } } + /public-body: + post: + operationId: postPublic + security: [] + requestBody: + required: true + content: + application/json: { schema: { type: string } } + responses: { "204": { description: ok } } +components: + securitySchemes: + bearer: { type: http, scheme: bearer } +` + +// TestE2E_OperationSecurityOptOut covers an operation declaring security: []. +// Auth was applied for every request the client made, so an endpoint the spec +// documents as needing no credential received one anyway. +func TestE2E_OperationSecurityOptOut(t *testing.T) { + files, _ := generateFromSpec(t, operationSecurityAPISpec, "secapi") + + runGeneratedWireTest(t, files, "operationsecurity", `package secapi + +import ( + "net/http" + "net/http/httptest" + "testing" +) + +func authSeen(t *testing.T) (*Client, *[]string) { + t.Helper() + var seen []string + srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + seen = append(seen, r.Header.Get("Authorization")) + w.WriteHeader(http.StatusNoContent) + })) + t.Cleanup(srv.Close) + return NewClient(srv.URL, WithAuth(&BearerAuth{Token: "s3cr3t"})), &seen +} + +func TestPublicOperationSendsNoCredential(t *testing.T) { + client, seen := authSeen(t) + + if err := client.GetPrivate(t.Context()); err != nil { + t.Fatalf("GetPrivate: %v", err) + } + if err := client.GetPublic(t.Context()); err != nil { + t.Fatalf("GetPublic: %v", err) + } + + if (*seen)[0] != "Bearer s3cr3t" { + t.Errorf("the operation inheriting the document's security got %q", (*seen)[0]) + } + if (*seen)[1] != "" { + t.Errorf("the operation declaring security: [] got %q", (*seen)[1]) + } +} + +// Opting out of auth changes nothing else about the request. +func TestPublicOperationStillSendsItsBody(t *testing.T) { + var payload, auth string + srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + buf := make([]byte, r.ContentLength) + r.Body.Read(buf) + payload, auth = string(buf), r.Header.Get("Authorization") + w.WriteHeader(http.StatusNoContent) + })) + defer srv.Close() + + client := NewClient(srv.URL, WithAuth(&BearerAuth{Token: "s3cr3t"})) + if err := client.PostPublic(t.Context(), "hello"); err != nil { + t.Fatalf("PostPublic: %v", err) + } + if payload != `+"`"+`"hello"`+"`"+` { + t.Errorf("payload = %q", payload) + } + if auth != "" { + t.Errorf("Authorization = %q, want none", auth) + } +} + +// A client with no provider is unaffected either way. +func TestNoProviderIsUnaffected(t *testing.T) { + var seen []string + srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + seen = append(seen, r.Header.Get("Authorization")) + w.WriteHeader(http.StatusNoContent) + })) + defer srv.Close() + + if err := NewClient(srv.URL).GetPrivate(t.Context()); err != nil { + t.Fatalf("GetPrivate: %v", err) + } + if seen[0] != "" { + t.Errorf("Authorization = %q, want none", seen[0]) + } +} +`) +} diff --git a/internal/ir/operations.go b/internal/ir/operations.go index f81b35d..3d12b0c 100644 --- a/internal/ir/operations.go +++ b/internal/ir/operations.go @@ -17,8 +17,11 @@ type OperationDef struct { SuccessResponse *ResponseDef // The primary 2xx response ErrorResponses []*ResponseDef // 4xx/5xx responses SecurityReqs [][]SecurityReq // OR of (AND of scheme refs) - Deprecated bool - Pagination *PaginationDef // nil if not paginated + // NoAuth records that the operation declares an empty security requirement, + // which overrides the document's to say it takes no credential. + NoAuth bool + Deprecated bool + Pagination *PaginationDef // nil if not paginated } // ParamDef represents an operation parameter. diff --git a/internal/templates/client.go.tmpl b/internal/templates/client.go.tmpl index fb8afd8..af6b01d 100644 --- a/internal/templates/client.go.tmpl +++ b/internal/templates/client.go.tmpl @@ -62,8 +62,10 @@ func NewClient(baseURL string, opts ...ClientOption) *Client { } // do executes an HTTP request and decodes the response. contentType selects the -// request body encoding and is sent as the Content-Type header. -func (c *Client) do(ctx context.Context, method string, path string, body any, contentType string, result any, accept string, headers ...http.Header) error { +// request body encoding and is sent as the Content-Type header. An operation +// whose spec declares an empty security requirement passes authenticated false, +// since it says it takes no credential. +func (c *Client) do(ctx context.Context, method string, path string, body any, contentType string, result any, accept string, authenticated bool, headers ...http.Header) error { fullURL := c.baseURL + path var payload []byte @@ -130,7 +132,7 @@ func (c *Client) do(ctx context.Context, method string, path string, body any, c } } -{{ if .AuthSchemes }} if c.auth != nil { +{{ if .AuthSchemes }} if c.auth != nil && authenticated { if err := c.auth.Apply(req); err != nil { return fmt.Errorf("applying auth: %w", err) } diff --git a/internal/templates/operations.go.tmpl b/internal/templates/operations.go.tmpl index 65bf802..ad28840 100644 --- a/internal/templates/operations.go.tmpl +++ b/internal/templates/operations.go.tmpl @@ -50,11 +50,11 @@ type {{ .Name }}Params struct { {{ end }}{{ end }} {{- $errType := errorType . -}} {{ if successType . }} var result {{ successType . }} - if err := c.do(ctx, "{{ .HTTPMethod }}", path, {{ if hasBody . }}body{{ else }}nil{{ end }}, {{ printf "%q" (requestContentType .) }}, &result, {{ printf "%q" (successContentType .) }}{{ if $needHeaders }}, headers{{ end }}); err != nil { + if err := c.do(ctx, "{{ .HTTPMethod }}", path, {{ if hasBody . }}body{{ else }}nil{{ end }}, {{ printf "%q" (requestContentType .) }}, &result, {{ printf "%q" (successContentType .) }}, {{ not .NoAuth }}{{ if $needHeaders }}, headers{{ end }}); err != nil { return nil, {{ if $errType }}parse{{ $errType }}(err){{ else }}err{{ end }} } return &result, nil -{{ else }} if err := c.do(ctx, "{{ .HTTPMethod }}", path, {{ if hasBody . }}body{{ else }}nil{{ end }}, {{ printf "%q" (requestContentType .) }}, nil, {{ printf "%q" (successContentType .) }}{{ if $needHeaders }}, headers{{ end }}); err != nil { +{{ else }} if err := c.do(ctx, "{{ .HTTPMethod }}", path, {{ if hasBody . }}body{{ else }}nil{{ end }}, {{ printf "%q" (requestContentType .) }}, nil, {{ printf "%q" (successContentType .) }}, {{ not .NoAuth }}{{ if $needHeaders }}, headers{{ end }}); err != nil { return {{ if $errType }}parse{{ $errType }}(err){{ else }}err{{ end }} } return nil From 7a171df9b2d54a207ac40b6cb175377d5a50709f Mon Sep 17 00:00:00 2001 From: Michael McQuade Date: Fri, 21 Aug 2026 10:07:32 -0500 Subject: [PATCH 2/2] chore(parser): use t.Chdir in the multi-file reference tests The helper added with those tests reimplemented what t.Chdir does, which usetesting flags, so lint has been failing on canary since it merged. --- internal/parser/multifile_test.go | 16 ++-------------- 1 file changed, 2 insertions(+), 14 deletions(-) diff --git a/internal/parser/multifile_test.go b/internal/parser/multifile_test.go index af41e11..f94d336 100644 --- a/internal/parser/multifile_test.go +++ b/internal/parser/multifile_test.go @@ -31,7 +31,7 @@ components: `) // Deliberately not the spec's directory: this is the CI case. - chdir(t, t.TempDir()) + t.Chdir(t.TempDir()) result, err := Parse(filepath.Join(dir, "api.yaml"), Config{}) if err != nil { @@ -81,7 +81,7 @@ components: shared: { $ref: "../common.yaml#/components/schemas/Shared" } `) - chdir(t, t.TempDir()) + t.Chdir(t.TempDir()) if _, err := Parse(filepath.Join(specDir, "api.yaml"), Config{}); err != nil { t.Fatalf("Parse: %v", err) @@ -94,15 +94,3 @@ func writeSpecFile(t *testing.T, dir, name, content string) { t.Fatalf("writing %s: %v", name, err) } } - -func chdir(t *testing.T, dir string) { - t.Helper() - previous, err := os.Getwd() - if err != nil { - t.Fatalf("getwd: %v", err) - } - if err := os.Chdir(dir); err != nil { - t.Fatalf("chdir: %v", err) - } - t.Cleanup(func() { os.Chdir(previous) }) -}