fix(compilers/openapi): report an oauth2 scheme with no flow

compilers/openapi/conformance_test.go

@@ -227,6 +227,7 @@ func conformanceCases() []conformanceCase {
 		{"inline-residue", assertInlineResidue, []string{"inline-anonymous"}},
 		{"servers-variables", assertServersVariables, []string{"servers"}},
 		{"security-schemes", assertSecuritySchemes, []string{"auth-schemes"}},
+		{"oauth2-no-flow", assertOAuth2NoFlow, nil},
 		{"security-or-and", assertSecurityOrAnd, []string{"per-op-auth"}},
 	}
 }
@@ -3221,6 +3222,42 @@ func assertSchemeDetail(t *testing.T, doc *ir.Document) {
 	assert.Equal(t, "https://example.com/refresh", oauth.Flows[0].RefreshURL)
 }
 
+// assertOAuth2NoFlow pins where a flowless oauth2 declaration is reported: once,
+// at the declaration, however many aliases reach it, and never for a scheme
+// whose metadata URL states its endpoints or that declares a flow. Every entry
+// interns either way (GitHub #646).
+func assertOAuth2NoFlow(t *testing.T, doc *ir.Document, diags []ir.Diagnostic) {
+	t.Helper()
+	const code = "openapi/oauth2-no-flow"
+	reported := map[string]bool{"emptyFlows": true, "unknownFlowsOnly": true}
+	byPointer := map[jsontext.Pointer]ir.AuthScheme{}
+	for _, s := range doc.Auth {
+		byPointer[s.Provenance.Pointer] = s
+	}
+	for _, name := range []string{
+		"aliasFirst", "aliasSecond", "emptyFlows", "unknownFlowsOnly", "metadataOnly", "declaredFlow",
+	} {
+		s, ok := byPointer[jsontext.Pointer("/components/securitySchemes/"+name)]
+		require.True(t, ok, "%s interns, flowless or not", name)
+		assert.Equal(t, ir.AuthKindOAuth2, s.Kind, "%s", name)
+		var want []ir.Severity
+		if reported[name] {
+			want = []ir.Severity{ir.SeverityWarning}
+		}
+		assert.Equal(t, want, diagsAt(diags, code, "/components/securitySchemes/"+name),
+			"%s: reported once at a flowless declaration, never at an alias or a control", name)
+	}
+	n := 0
+	for _, d := range diags {
+		if d.Code == code {
+			n++
+		}
+	}
+	assert.Equal(t, 2, n, "two flowless declarations, two reports: %+v", diags)
+	assert.Equal(t, "https://example.com/.well-known/oauth-authorization-server",
+		byPointer["/components/securitySchemes/metadataOnly"].OAuth2MetadataURL, "the control's metadata URL is kept")
+}
+
 func assertSecurityOrAnd(t *testing.T, doc *ir.Document, _ []ir.Diagnostic) {
 	require.Len(t, doc.Services, 1)
 	auth := doc.Services[0].Auth

compilers/openapi/internal/auth/auth.go

@@ -35,7 +35,7 @@ import (
 //
 // An entry that resolves to an object but names no mechanism is refused and
 // reported the same way; see mechanismRefusalDiag. Every other diagnostic from
-// here concerns a scheme that did intern.
+// here concerns a scheme that did intern (see oauthNoFlowDiag).
 func LowerSecuritySchemes(c lowering.Ctx) (map[ir.AuthID]ir.AuthScheme, []ir.Diagnostic) {
 	comps := c.Doc.Components
 	if comps == nil {
@@ -116,7 +116,11 @@ func lowerSecurityScheme(c lowering.Ctx, name string, ss *soa.SecurityScheme,
 	if !named {
 		return ir.AuthScheme{}, false, []ir.Diagnostic{mechanismRefusalDiag(c, name, missing, entry)}
 	}
-	diags = preserveUnreadFields(c, &scheme, ss, decl)
+	if declaresNoFlow(scheme, ss) {
+		// Reported rather than refused; see oauthNoFlowDiag.
+		diags = append(diags, oauthNoFlowDiag(c, decl))
+	}
+	diags = append(diags, preserveUnreadFields(c, &scheme, ss, decl)...)
 	diags = append(diags, applySchemeAnnotations(c, &scheme, ss, decl)...)
 	// Distinct from preserveUnreadFields above it: that keeps the fields OpenAPI
 	// defines for a securityScheme which this entry's own mechanism gives no
@@ -189,6 +193,31 @@ func mechanismRefusalDiag(c lowering.Ctx, name, missing string, entry jsontext.P
 			"no scheme is interned for it, and every requirement naming it is dropped", name, missing)
 }
 
+// declaresNoFlow reports whether an interned oauth2 scheme writes a flows
+// object that declares no flow, with no oauth2MetadataUrl to discover its
+// endpoints from (RFC 8414). An absent flows object is the loader's finding
+// (a REQUIRED field), so it is not this one.
+func declaresNoFlow(scheme ir.AuthScheme, ss *soa.SecurityScheme) bool {
+	if scheme.Kind != ir.AuthKindOAuth2 || ss.GetFlows() == nil {
+		return false
+	}
+	if scheme.OAuth2MetadataURL != "" {
+		return false
+	}
+	return len(scheme.Flows) == 0
+}
+
+// oauthNoFlowDiag reports a flows object declaring no flow; the design record
+// is on diag.OAuth2NoFlow.
+//
+// decl places it, not entry: a flowless declaration is a fact about the text at
+// decl, and a $ref entry holds no flows either way. The message names no entry,
+// so the copies every alias of one declaration draws dedup to one report.
+func oauthNoFlowDiag(c lowering.Ctx, decl jsontext.Pointer) ir.Diagnostic {
+	return c.DiagAt(ir.SeverityWarning, diag.OAuth2NoFlow, decl,
+		"oauth2 security scheme declares no flow: no token endpoint is stated for it")
+}
+
 // fillSchemeKind sets the mechanism kind and its per-kind fields (ir-design §9).
 // An unrecognized type degrades to a custom scheme carrying the raw type, which
 // a later OpenAPI version's own type reaches as readily as a typo does.
@@ -209,6 +238,8 @@ func fillSchemeKind(scheme *ir.AuthScheme, ss *soa.SecurityScheme) (missing stri
 		return fillHTTPScheme(scheme, ss)
 	case soa.SecuritySchemeTypeOAuth2:
 		scheme.Kind = ir.AuthKindOAuth2
+		// An absent or empty flows object lowers to nil flows; the caller
+		// reports the empty one (declaresNoFlow).
 		scheme.Flows = oauthFlows(ss.GetFlows())
 		scheme.OAuth2MetadataURL = ss.GetOAuth2MetadataUrl()
 	case soa.SecuritySchemeTypeOpenIDConnect:

compilers/openapi/internal/auth/auth_internal_test.go

@@ -68,3 +68,32 @@ func TestMechanismFieldNames_AccountForEverySourceField(t *testing.T) {
 	assert.Equal(t, want, mechanismFieldNames(),
 		"mechanismFieldNames must list every per-type field, sorted")
 }
+
+// TestPresentFlows_ReadsEverySourceFlowField holds presentFlows to the flow
+// fields soa.OAuthFlows declares. The no-flow report reads an empty lowered list
+// as "the document declares no flow", which is true only while every flow field
+// is read: a field the upstream model gains would otherwise lower to nothing and
+// draw a false report. Each flow field is set alone through reflection, so the
+// check reads the struct rather than a second hand-written list.
+func TestPresentFlows_ReadsEverySourceFlowField(t *testing.T) {
+	t.Parallel()
+	flowType := reflect.TypeFor[*soa.OAuthFlow]()
+	notFlows := map[string]bool{"Extensions": true}
+	st := reflect.TypeFor[soa.OAuthFlows]()
+	var seen int
+	var all soa.OAuthFlows
+	for f := range st.Fields() {
+		if f.Anonymous || !f.IsExported() || notFlows[f.Name] {
+			continue // the embedded marshaller model is not a document field
+		}
+		require.Equal(t, flowType, f.Type,
+			"OAuthFlows field %q is neither a flow nor listed as one that is not", f.Name)
+		var flows soa.OAuthFlows
+		reflect.ValueOf(&flows).Elem().FieldByIndex(f.Index).Set(reflect.ValueOf(&soa.OAuthFlow{}))
+		assert.Len(t, presentFlows(&flows), 1, "presentFlows does not read the %s flow", f.Name)
+		reflect.ValueOf(&all).Elem().FieldByIndex(f.Index).Set(reflect.ValueOf(&soa.OAuthFlow{}))
+		seen++
+	}
+	require.NotZero(t, seen, "the walk found no flow field at all")
+	assert.Len(t, presentFlows(&all), seen, "each flow field is read exactly once")
+}

compilers/openapi/internal/auth/auth_test.go

@@ -1,6 +1,7 @@
 package auth_test
 
 import (
+	"cmp"
 	"encoding/json/jsontext"
 	"slices"
 	"strings"
@@ -245,6 +246,12 @@ func TestAuth_AllSchemeKinds(t *testing.T) {
 	assert.True(t, sawCustomHTTP, "unknown http scheme is custom")
 }
 
+// TestAuth_OAuthNoFlowsUnknownTypeAndGhostRef pins three entries a document can
+// carry: an oauth2 scheme with no flows object, interned with nil flows and
+// reported only by the loader, whose REQUIRED-field error this lowering does
+// not repeat; an unrecognized type, which degrades to a custom scheme carrying
+// the token; and a $ref naming no target, which the load phase reports
+// elsewhere.
 func TestAuth_OAuthNoFlowsUnknownTypeAndGhostRef(t *testing.T) {
 	t.Parallel()
 	spec := openapitest.PathsSpec(`  /x:
@@ -255,7 +262,7 @@ components:
     weird: {type: bananas}
     ghost: {$ref: '#/components/securitySchemes/Missing'}
 `)
-	doc, _, _ := serviceSpec(t, spec)
+	doc, _, diags := serviceSpec(t, spec)
 	var oauth, custom ir.AuthScheme
 	for _, s := range doc.Auth {
 		if s.Kind == ir.AuthKindOAuth2 {
@@ -268,6 +275,10 @@ components:
 	assert.Equal(t, ir.AuthKindOAuth2, oauth.Kind)
 	assert.Nil(t, oauth.Flows, "oauth2 with no flows lowers to nil flows")
 	assert.Equal(t, "bananas", custom.Scheme, "unknown scheme type degrades to custom")
+
+	assert.Empty(t, messagesAt(diags, diag.OAuth2NoFlow),
+		"an absent flows object is the loader's finding alone: %+v", diags)
+	assert.NotEmpty(t, messagesAtPointer(diags, ""), "and the loader does report it: %+v", diags)
 }
 
 // TestLowerSecuritySchemes_NothingLoweredIsNilNotEmpty pins the guard that
@@ -436,6 +447,177 @@ components:
 	}
 }
 
+// TestLowerSecuritySchemes_AFlowlessDeclarationIsReportedOnceWhereItIsWritten
+// pins the placement lowerSecurityScheme's doc states: what is said of the
+// fields is placed at decl. Two aliases are declared before their target, so
+// the declaration is reached through an alias first; every entry interns, and
+// the one flowless declaration is reported once, at the target.
+func TestLowerSecuritySchemes_AFlowlessDeclarationIsReportedOnceWhereItIsWritten(t *testing.T) {
+	t.Parallel()
+	doc, svc, diags := serviceSpec(t, `openapi: 3.1.0
+info: {title: T, version: "1"}
+security:
+  - alias: []
+paths: {}
+components:
+  securitySchemes:
+    alias: {$ref: '#/components/securitySchemes/target'}
+    other: {$ref: '#/components/securitySchemes/target'}
+    target: {type: oauth2, flows: {}}
+`)
+	for _, name := range []string{"alias", "other", "target"} {
+		require.Contains(t, doc.Auth, ids.Auth(name), "%s interns a named scheme of its own (issue #107)", name)
+	}
+	require.Len(t, svc.Auth, 1)
+	require.Len(t, svc.Auth[0].Schemes, 1)
+	assert.Equal(t, ids.Auth("alias"), svc.Auth[0].Schemes[0].Scheme)
+
+	assert.Equal(t, []string{"/components/securitySchemes/target"},
+		sortedPointersAt(diags, diag.OAuth2NoFlow),
+		"one report, at the declaration the aliases share: %+v", diags)
+	assert.Empty(t, messagesAtPointer(diags, "/components/securitySchemes/alias"),
+		"nothing is reported against an alias, which holds no flows either way")
+	assert.Empty(t, messagesAtPointer(diags, "/components/securitySchemes/other"))
+}
+
+// TestLowerSecuritySchemes_AnOAuth2SchemeWithNoFlowIsReported pins which
+// entries draw the report (GitHub #646): an oauth2 scheme whose flows object is
+// present but declares no flow, with no metadata URL to discover endpoints
+// from. An absent flows object is left to the loader's REQUIRED-field error.
+// Every interned entry keeps the requirement naming it, so its AuthID stays
+// live.
+func TestLowerSecuritySchemes_AnOAuth2SchemeWithNoFlowIsReported(t *testing.T) {
+	t.Parallel()
+	cases := []struct {
+		name string
+		// version is the document's openapi field; "" means 3.1.0.
+		version string
+		// entry is the body written for the securitySchemes entry `s`.
+		entry string
+		// refused reports an entry naming no mechanism, which is refused and
+		// interned nowhere.
+		refused bool
+		// wantReported is whether the code must fire for this entry.
+		wantReported bool
+		// wantOther is the pointer of a pre-existing diagnostic the row expects
+		// beside the code, or "" for none.
+		wantOther string
+		check     func(t *testing.T, s ir.AuthScheme)
+	}{
+		{
+			name: "flows written empty (the issue repro)", entry: `{type: oauth2, flows: {}}`,
+			wantReported: true,
+			check: func(t *testing.T, s ir.AuthScheme) {
+				assert.Equal(t, ir.AuthKindOAuth2, s.Kind)
+				assert.Nil(t, s.Flows, "an empty flows object lowers to nil flows")
+			},
+		},
+		{
+			name: "flows absent, the loader's own error", entry: `{type: oauth2}`,
+			check: func(t *testing.T, s ir.AuthScheme) {
+				assert.Equal(t, ir.AuthKindOAuth2, s.Kind)
+				assert.Nil(t, s.Flows)
+			},
+		},
+		{
+			name:         "flows naming only a key this model does not have",
+			entry:        `{type: oauth2, flows: {application: {tokenUrl: 'https://t', scopes: {}}}}`,
+			wantReported: true, wantOther: "/components/securitySchemes/s/flows/application",
+			check: func(t *testing.T, s ir.AuthScheme) {
+				assert.Equal(t, ir.AuthKindOAuth2, s.Kind)
+				assert.Nil(t, s.Flows, "a flows object this model names no key of declares no flow")
+			},
+		},
+		{
+			name:    "flows empty with a 3.2 metadata url",
+			version: "3.2.0",
+			entry:   `{type: oauth2, flows: {}, oauth2MetadataUrl: 'https://meta'}`,
+			check: func(t *testing.T, s ir.AuthScheme) {
+				assert.Nil(t, s.Flows)
+				assert.Equal(t, "https://meta", s.OAuth2MetadataURL,
+					"RFC 8414 metadata states the endpoints, so the url exempts the entry")
+			},
+		},
+		{
+			name:  "one declared flow",
+			entry: `{type: oauth2, flows: {implicit: {authorizationUrl: 'https://a', scopes: {}}}}`,
+			check: func(t *testing.T, s ir.AuthScheme) {
+				require.Len(t, s.Flows, 1)
+				assert.Equal(t, "implicit", s.Flows[0].Kind)
+			},
+		},
+		{
+			name:  "one declared device flow",
+			entry: `{type: oauth2, flows: {deviceAuthorization: {deviceAuthorizationUrl: 'https://d', tokenUrl: 'https://t', scopes: {}}}}`,
+			check: func(t *testing.T, s ir.AuthScheme) {
+				require.Len(t, s.Flows, 1)
+				assert.Equal(t, "https://d", s.Flows[0].AuthorizationURL)
+			},
+		},
+		{
+			// Pins the Kind guard where the rule lives: a flows object on an
+			// apiKey scheme is a stray field, kept beside it, not a flowless
+			// oauth2 scheme.
+			name:  "an apiKey scheme carrying an empty flows object",
+			entry: `{type: apiKey, name: k, in: header, flows: {}}`,
+			check: func(t *testing.T, s ir.AuthScheme) {
+				assert.Equal(t, ir.AuthKindAPIKey, s.Kind)
+				assert.Contains(t, s.Unmodeled, "openapi:flows", "the stray field is kept")
+			},
+		},
+		{
+			name: "no type at all", entry: `{flows: {}}`, refused: true,
+		},
+	}
+	for _, tc := range cases {
+		t.Run(tc.name, func(t *testing.T) {
+			t.Parallel()
+			version := cmp.Or(tc.version, "3.1.0")
+			doc, svc, diags := serviceSpec(t, `openapi: `+version+`
+info: {title: T, version: "1"}
+security:
+  - s: []
+paths: {}
+components:
+  securitySchemes:
+    s: `+tc.entry+`
+`)
+			s, interned := doc.Auth[ids.Auth("s")]
+			if tc.refused {
+				assert.False(t, interned, "an entry naming no mechanism is refused, not interned")
+				assert.Empty(t, messagesAt(diags, diag.OAuth2NoFlow),
+					"the refusal is its only report: %+v", diags)
+				_, found := firstDiagAt(diags, diag.IncompleteSecurityScheme)
+				assert.True(t, found, "and that refusal is still reported: %+v", diags)
+				return
+			}
+			require.True(t, interned, "the entry is interned, not refused: %+v", diags)
+			require.Len(t, svc.Auth, 1, "the requirement naming it survives")
+			require.Len(t, svc.Auth[0].Schemes, 1)
+			assert.Equal(t, ids.Auth("s"), svc.Auth[0].Schemes[0].Scheme,
+				"an interned scheme keeps a live AuthID, unlike a refused one")
+			tc.check(t, s)
+			if tc.wantOther != "" {
+				assert.NotEmpty(t, messagesAtPointer(diags, tc.wantOther),
+					"the finding that was already there is still reported beside it")
+			}
+
+			if !tc.wantReported {
+				assert.Empty(t, messagesAt(diags, diag.OAuth2NoFlow), "no report: %+v", diags)
+				return
+			}
+			d, found := firstDiagAt(diags, diag.OAuth2NoFlow)
+			require.True(t, found, "the entry is reported: %+v", diags)
+			assert.Equal(t, ir.SeverityWarning, d.Severity)
+			assert.Equal(t, jsontext.Pointer("/components/securitySchemes/s"), d.Provenance.Pointer,
+				"reported at the declaration, not at the flows object inside it")
+			assert.Contains(t, d.Message, "declares no flow")
+			assert.Equal(t, 1, openapitest.CountDiagsAt(diags, diag.OAuth2NoFlow, ir.SeverityWarning),
+				"one report per declaration")
+		})
+	}
+}
+
 // TestLowerSecuritySchemes_ARefusedEntryDropsTheRequirementNamingIt pins the
 // downstream half of the refusal. Nothing is interned, so a requirement naming
 // the entry resolves to no scheme and drops whole under the rule #41

compilers/openapi/internal/diag/diag.go

@@ -248,6 +248,18 @@ const (
 	// there. The document is invalid either way, since OpenAPI requires both
 	// fields.
 	IncompleteSecurityScheme = "openapi/incomplete-security-scheme"
+	// OAuth2NoFlow reports an oauth2 securitySchemes entry whose flows object
+	// is present but declares no flow — `flows: {}`, or only keys the model does
+	// not name — and which has no oauth2MetadataUrl to discover its endpoints
+	// from. The loader accepts that shape, so only the compiler sees it; an
+	// absent `flows` is the loader's own error and is not reported again
+	// (GitHub #646). Placed at the declaration, once however many aliases
+	// reach it.
+	//
+	// Reported, not refused as IncompleteSecurityScheme is: the IR states exactly
+	// what the document said, and refusing would drop the entry's text. A warning,
+	// like ReservedHeaderName, since the document lowers whole.
+	OAuth2NoFlow = "openapi/oauth2-no-flow"
 	// ReservedHeaderName reports a header declaration OpenAPI says SHALL be
 	// ignored, because the protocol layer already owns the name. Three positions:
 	//

compilers/openapi/internal/diag/diag_test.go

@@ -144,6 +144,7 @@ func codes() []string {
 		diag.AliasAmplification, diag.BudgetExceeded,
 		diag.UnattachableRequired, diag.InternalInvariant,
 		diag.DuplicateOperationID, diag.ConflictingOperationID, diag.IncompleteSecurityScheme,
+		diag.OAuth2NoFlow,
 		diag.ReservedHeaderName, diag.UnpreservableConstruct,
 		diag.UnknownSchemaKeyword, diag.UnknownObjectKey, diag.UnknownKeyBudget,
 		diag.UnknownKeyUnreachable, diag.UnknownKeyEntryTaken,