diff --git a/.agents/skills/loom-framework/SKILL.md b/.agents/skills/loom-framework/SKILL.md index e95df8d71..905c79fa0 100644 --- a/.agents/skills/loom-framework/SKILL.md +++ b/.agents/skills/loom-framework/SKILL.md @@ -340,7 +340,21 @@ consumer validation. IR consume this projection; keep required scopes on each requirement's fresh scheme data and preserve them through Dup. Globally deduplicated transport schemes do not own OR-branch authentication decisions. Keep union-based - unknown-scope admission and OpenAPI's protocol-specific scope-array rules. + unknown-scope admission. SchemeRequirements binds explicit names to scheme + identity: OAuth scopes must be declared; non-OAuth roles need no inventory. + ScopeFor and RoleFor add to shared Scope requirements. Deep-copy these arrays + on inheritance and include them in requirement identity. Import planning owns + normalized requirement validation; OpenAPI exports both OAuth scope arrays and + non-OAuth role arrays. Keep expr/tla/security_requirements and the generated + credential regressions aligned; role assignment remains application-owned. + +- `expr.SchemeExpr.FlowScopes` owns effective OAuth inventories: nil inherits + scheme defaults, nonnil empty replaces them. ScopeNames projects available + names only for requirements and callback discovery. Import, OpenAPI export + and generated OAuthFlow maps retain each flow's names and descriptions. When + any flow overrides defaults, resolve every runtime flow explicitly so the + available-name union cannot leak into inherited inventories. Keep the bounded + model in expr/tla/oauth_flow_scopes aligned; token grants remain app-owned. - `service.assignClientMembers` owns the service Client receiver: reserve all authored methods, record synthetic mixed-stream method names, then allocate diff --git a/.agents/skills/loom-framework/references/formal-models.md b/.agents/skills/loom-framework/references/formal-models.md index 3891dc263..7af2ba1ae 100644 --- a/.agents/skills/loom-framework/references/formal-models.md +++ b/.agents/skills/loom-framework/references/formal-models.md @@ -261,6 +261,7 @@ Rendered schema validation and actual decoder behavior are separate obligations. | Public message ownership, history masking, diagnostic exclusion, transparent wrappers and independent joins | [Public disclosure](../../../../pkg/tla/error_disclosure/README.md); finite owner/message model, with HTTP/gRPC/JSON-RPC unary/SSE/WebSocket wire tests | | gRPC aggregate error status and detail ownership | [Error contracts](../../../../grpc/tla/error_contract/README.md); bounded branch-order and complete-failure checks, with wrappers and generated unary/streaming mappings tested in Go | | Nearest raw forwarding boundary and prefix independence | [Forwarded client trust](../../../../http/tla/forwarded_client/README.md); bounded chains, with IP parsing and repeated-header grouping checked in Go | +| Per-scheme security scopes and roles with AND/OR/anonymous alternatives | [Security requirements](../../../../expr/tla/security_requirements/README.md); flattened-name counterexample and generated credential checks | | Authentication alternatives, optional credential extraction and failed-context isolation | [Credential alternatives](../../../../internal/securitygen/tla/credential_alternatives/README.md); finite extraction/control model, with actual framing and HTTP/JSON-RPC/gRPC wire regressions | | gRPC pre-frame admission, complete application input, original cancellation and middleware rechecks; ordered AND/OR contexts and failed-branch reset | [Stream admission](../../../../security/tla/stream_admission/README.md); positive and deliberate negative models paired with runtime and generated transport regressions | | Scalar query/header/cookie absence, defaults and conversion admission | [Scalar binding presence](../../../../http/codegen/tla/scalar_binding_presence/README.md); supplied-empty counterexample, shared decoder and mounted HTTP/JSON-RPC controls | @@ -343,3 +344,5 @@ The [discriminator admission model](../../../../http/codegen/openapi/internal/ir records optional/fallback admission and 3.1 projection. Go tests cover reference resolution and graph retention outside its Boolean assumptions. | JSON-RPC envelope ID source, presence and validation ordering | [Envelope ID admission](../../../../http/codegen/tla/jsonrpc_id_admission/README.md); params-first and late-handler counterexamples, shared carrier/CLI validation and mounted unary/SSE/WebSocket controls | + +OAuth flow inventories: [independent defaults and overrides](../../../../expr/tla/oauth_flow_scopes/README.md), with a flattening counterexample and generated callback checks. diff --git a/.agents/skills/loom/SKILL.md b/.agents/skills/loom/SKILL.md index 6fa16065f..7fdb77c68 100644 --- a/.agents/skills/loom/SKILL.md +++ b/.agents/skills/loom/SKILL.md @@ -587,11 +587,12 @@ operations. It reports skipped operations and omitted root members separately. These root members include servers and unsupported info metadata. Tag metadata, response summaries, supported serialization, structured examples, and reusable examples import losslessly. API-key, HTTP basic, HTTP bearer, and supported -OAuth 2.0 schemes also import losslessly. OAuth 2.0 flows must use the same -scope map. Root and operation security requirements retain OAuth scopes and +OAuth 2.0 schemes also import losslessly. OAuth 2.0 flows retain independent scope maps, including empty inventories. +Author flow-local scopes in an optional final block on each flow constructor; +no block inherits scheme-level scopes, while an empty block replaces them. Root and operation security requirements retain OAuth scopes and AND or alternative semantics. Schemes using the same wire credential share one -payload field while retaining each authentication check. A conjunction whose -schemes require different scope sets is rejected; equal scope sets and separate +payload field while retaining each authentication check. Each scheme retains its own required names: OAuth scopes render with +`ScopeFor`, and non-OAuth roles render with `RoleFor`. Empty lists and separate alternatives remain supported. Partial import omits unused root security when every retained operation explicitly overrides it. An anonymous `{}` alternative renders as `Security()` and an explicit operation `security: []` @@ -1212,8 +1213,13 @@ projected onto each scheme's declared scopes, including Basic and API keys. Callbacks can use `scheme.Validate(grantedScopes)` directly: an unscoped key alongside a scoped JWT receives no JWT scopes. Shared scopes apply to each scheme that declares them; undeclared scopes fail design validation. Separate `Security` -alternatives retain their own scope sets. Regenerate endpoints and OpenAPI to -adopt this correction. +alternatives retain their own scope sets. Inside Security, use +`ScopeFor(oauth, "read")` to bind OAuth scopes to one scheme and +`RoleFor(key, "admin")` for non-OAuth application roles. Both are additive to +shared Scope declarations. Role names need no scheme inventory. Non-OAuth +callbacks receive them in RequiredScopes and must validate application-owned +grants; they are not extra wire credentials. Regenerate endpoints and OpenAPI; +non-OAuth required names now export as role arrays. `CookieTransport(scheme, "", fn...)` is the transport-owned browser-cookie mode. It emits the security contract without synthesizing a payload field or diff --git a/codegen/generator/testdata/security_required_scopes.go.txt b/codegen/generator/testdata/security_required_scopes.go.txt index 66987675e..61a0eed64 100644 --- a/codegen/generator/testdata/security_required_scopes.go.txt +++ b/codegen/generator/testdata/security_required_scopes.go.txt @@ -23,6 +23,11 @@ func TestScopeCallbacks(t *testing.T) { wantCalls []string wantError bool }{ + {"machine scope", "flow", map[string][]string{"oauth": {"machine"}}, []string{"oauth"}, false}, + {"machine scope missing", "flow", map[string][]string{"oauth": {"oauth"}}, []string{"oauth"}, true}, + {"isolated valid", "isolated", map[string][]string{"oauth": {"shared"}, "second": {"oauth"}, "key": {"admin"}}, []string{"oauth", "second", "key"}, false}, + {"isolated missing role", "isolated", map[string][]string{"oauth": {"shared"}, "second": {"oauth"}}, []string{"oauth", "second", "key"}, true}, + {"isolated wrong scheme grant", "isolated", map[string][]string{"oauth": {"oauth"}, "second": {"shared"}, "key": {"admin"}}, []string{"oauth"}, true}, {"compound valid", "compound", map[string][]string{"basic": {"shared", "basic"}, "key": {"shared", "key"}, "jwt": {"shared", "read"}, "oauth": {"shared", "oauth"}}, []string{"basic", "key", "jwt", "oauth"}, false}, {"compound missing", "compound", map[string][]string{"basic": {"basic"}}, []string{"basic"}, true}, {"first alternative", "alternative", map[string][]string{"jwt": {"read"}}, []string{"jwt", "unscoped"}, false}, @@ -35,6 +40,9 @@ func TestScopeCallbacks(t *testing.T) { check := func(name string, declared, required []string) { authCalls = append(authCalls, strings.TrimPrefix(name, "scope_")) for _, scope := range required { + if tc.method == "isolated" && name == "key" { + continue + } require.True(t, slices.Contains(declared, scope), "%s requires undeclared %s", name, scope) } } @@ -52,10 +60,29 @@ func TestScopeCallbacks(t *testing.T) { } oauth := func(ctx context.Context, _ string, s *security.OAuth2Scheme) (context.Context, error) { check(s.Name, s.Scopes, s.RequiredScopes) + if s.Name == "oauth" { + require.Len(t, s.Flows, 3) + require.Equal(t, map[string]string{"oauth": "User consent", "shared": "User grant"}, s.Flows[0].Scopes) + require.Equal(t, map[string]string{"machine": "Machine grant", "shared": "Machine permission"}, s.Flows[1].Scopes) + require.NotNil(t, s.Flows[2].Scopes) + require.Empty(t, s.Flows[2].Scopes) + } + if s.Name == "second" { + require.Len(t, s.Flows, 2) + require.Equal(t, map[string]string{"oauth": "no description", "shared": "no description"}, s.Flows[0].Scopes) + require.NotNil(t, s.Flows[1].Scopes) + require.Empty(t, s.Flows[1].Scopes) + } return ctx, s.Validate(tc.grants[strings.TrimPrefix(s.Name, "scope_")]) } var err error switch tc.method { + case "flow": + endpoint := scopes.NewFlowEndpoint(svc, oauth) + _, err = endpoint(context.Background(), &scopes.FlowPayload{Token: "token"}) + case "isolated": + endpoint := scopes.NewIsolatedEndpoint(svc, oauth, key) + _, err = endpoint(context.Background(), &scopes.IsolatedPayload{Token: "token", Key: "key"}) case "compound": endpoint := scopes.NewCompoundEndpoint(svc, basic, key, jwt, oauth) _, err = endpoint(context.Background(), &scopes.CompoundPayload{User: "user", Password: "pass", Key: "key", Token: "jwt", Access: "oauth"}) @@ -87,3 +114,13 @@ func (s *scopeService) Alternative(context.Context, *scopes.AlternativePayload) s.calls = append(s.calls, "alternative") return nil } + +func (s *scopeService) Isolated(context.Context, *scopes.IsolatedPayload) error { + s.calls = append(s.calls, "isolated") + return nil +} + +func (s *scopeService) Flow(context.Context, *scopes.FlowPayload) error { + s.calls = append(s.calls, "flow") + return nil +} diff --git a/codegen/service/endpoint_method_section.go b/codegen/service/endpoint_method_section.go index a8d2a63ea..f03f4d7a9 100644 --- a/codegen/service/endpoint_method_section.go +++ b/codegen/service/endpoint_method_section.go @@ -170,6 +170,13 @@ func buildOAuth2SchemeAuth(group *jen.Group, scheme *SchemeData, payload, contex for _, flow := range scheme.Flows { flows.Op("&").Add(codegen.Expr("security.OAuthFlow")).CustomFunc(multilineValues, func(fields *jen.Group) { fields.Id("Type").Op(":").Lit(flow.Type()) + if flow.Scopes != nil { + scopes := make(jen.Dict, len(flow.Scopes)) + for _, scope := range flow.Scopes { + scopes[jen.Lit(scope.Name)] = jen.Lit(scope.Description) + } + fields.Id("Scopes").Op(":").Map(jen.String()).String().Values(scopes) + } if flow.AuthorizationURL != "" { fields.Id("AuthorizationURL").Op(":").Lit(flow.AuthorizationURL) } diff --git a/codegen/service/security_required_scopes_test.go b/codegen/service/security_required_scopes_test.go index 3ca97ae7f..de1c50f76 100644 --- a/codegen/service/security_required_scopes_test.go +++ b/codegen/service/security_required_scopes_test.go @@ -20,6 +20,8 @@ func TestEndpointRequiredScopeOwnership(t *testing.T) { method string want map[string][][]string }{ + {"flow", map[string][][]string{"oauth": {{"machine"}}}}, + {"isolated", map[string][][]string{"oauth": {{"shared"}}, "second": {{"oauth"}}, "key": {{"admin"}}}}, {"compound", map[string][][]string{"scope_basic": {{"shared", "basic"}}, "key": {{"shared", "key"}}, "scope_jwt": {{"shared", "read"}}, "oauth": {{"shared", "oauth"}}}}, {"alternative", map[string][][]string{"scope_jwt": {{"read"}, {"write"}}, "unscoped": {{}, {}}}}, } { diff --git a/codegen/service/service_data_methods.go b/codegen/service/service_data_methods.go index 9907629bd..5aaa67923 100644 --- a/codegen/service/service_data_methods.go +++ b/codegen/service/service_data_methods.go @@ -2,6 +2,7 @@ package service import ( "fmt" + "slices" "github.com/CaliLuke/loom/codegen" "github.com/CaliLuke/loom/expr" @@ -413,7 +414,7 @@ func BuildSchemeData(s *expr.SchemeExpr, m *expr.MethodExpr) *SchemeData { if !expr.IsObject(m.Payload.Type) { return nil } - scopes := schemeScopeNames(s) + scopes := s.ScopeNames() switch s.Kind { case expr.BasicAuthKind: userAtt := expr.TaggedAttribute(m.Payload, "security:username") @@ -445,23 +446,25 @@ func BuildSchemeData(s *expr.SchemeExpr, m *expr.MethodExpr) *SchemeData { data := buildCredentialSchemeData(s, m, "security:accesstoken", scopes) if data != nil { data.Flows = s.Flows + if slices.ContainsFunc(s.Flows, func(flow *expr.FlowExpr) bool { + return flow.Scopes != nil + }) { + data.Flows = make([]*expr.FlowExpr, len(s.Flows)) + for i, flow := range s.Flows { + copy := *flow + copy.Scopes = s.FlowScopes(flow) + if copy.Scopes == nil { + copy.Scopes = []*expr.ScopeExpr{} + } + data.Flows[i] = © + } + } } return data } return nil } -func schemeScopeNames(s *expr.SchemeExpr) []string { - if len(s.Scopes) == 0 { - return nil - } - scopes := make([]string, len(s.Scopes)) - for i, scope := range s.Scopes { - scopes[i] = scope.Name - } - return scopes -} - func buildCredentialSchemeData(s *expr.SchemeExpr, m *expr.MethodExpr, tag string, scopes []string) *SchemeData { keyAtt := expr.TaggedAttribute(m.Payload, tag) if keyAtt == "" { diff --git a/codegen/service/service_data_security.go b/codegen/service/service_data_security.go index e3ece6b09..5eaf08728 100644 --- a/codegen/service/service_data_security.go +++ b/codegen/service/service_data_security.go @@ -74,7 +74,8 @@ type ( KeyAttr string // Scopes lists the scopes that apply to the scheme. Scopes []string - // RequiredScopes lists this requirement's scopes declared by the scheme. + // RequiredScopes holds this requirement's names for the scheme: declared + // OAuth scopes or application permission/role names for non-OAuth schemes. RequiredScopes []string // Flows describes the OAuth2 flows. Flows []*expr.FlowExpr diff --git a/codegen/service/testdata/security_required_scopes.go b/codegen/service/testdata/security_required_scopes.go index 748316bb5..0c23e94dc 100644 --- a/codegen/service/testdata/security_required_scopes.go +++ b/codegen/service/testdata/security_required_scopes.go @@ -2,6 +2,7 @@ package testdata import ( . "github.com/CaliLuke/loom/dsl" + "github.com/CaliLuke/loom/expr" ) // SecurityRequiredScopesDSL exercises compound and alternative scope ownership. @@ -20,12 +21,9 @@ func SecurityRequiredScopesDSL() { Scope("write") Scope("shared") }) - oauth := OAuth2Security("oauth", func() { - AuthorizationCodeFlow("/authorize", "/token", "/refresh") - Scope("oauth") - Scope("shared") - }) + oauth, second := securityFlowScopeSchemes() Service("scopes", func() { + securityFlowScopeMethods(oauth, second, key) Method("compound", func() { Security(basic, key, jwt, oauth, func() { Scope("shared") @@ -100,3 +98,57 @@ func SecurityOAuthScopesDSL() { }) }) } + +func securityFlowScopeSchemes() (*expr.SchemeExpr, *expr.SchemeExpr) { + oauth := OAuth2Security("oauth", func() { + AuthorizationCodeFlow("/authorize", "/token", "/refresh", func() { + Scope("oauth", "Superseded description") + Scope("oauth", "User consent") + Scope("shared", "User grant") + }) + ClientCredentialsFlow("https://example.com/token", "", func() { + Scope("machine", "Machine grant") + Scope("shared", "Machine permission") + }) + ImplicitFlow("https://example.com/authorize", "", func() {}) + }) + second := OAuth2Security("second", func() { + Scope("oauth", "Superseded default") + ClientCredentialsFlow("https://example.com/token", "") + ImplicitFlow("https://example.com/authorize", "", func() {}) + Scope("oauth") + Scope("shared") + }) + return oauth, second +} + +func securityFlowScopeMethods(oauth, second, key *expr.SchemeExpr) { + Method("flow", func() { + Security(oauth, func() { + ScopeFor(oauth, "machine") + }) + Payload(func() { + AccessToken("token") + Required("token") + }) + HTTP(func() { + GET("/flow") + }) + }) + Method("isolated", func() { + Security(oauth, second, key, func() { + ScopeFor(oauth, "shared") + ScopeFor(second, "oauth") + RoleFor(key, "admin") + }) + Payload(func() { + AccessToken("token") + APIKey("key", "key") + Required("token", "key") + }) + HTTP(func() { + GET("/isolated") + Header("key:X-Key") + }) + }) +} diff --git a/docs/authorization.md b/docs/authorization.md index 8e2bd585e..76e31afa6 100644 --- a/docs/authorization.md +++ b/docs/authorization.md @@ -100,8 +100,35 @@ that scheme, in requirement order. An unscoped API key combined with a JWT that declares `read` receives no required scopes; the JWT receives `read`. A scope declared by both schemes is required by both. Scopes declared by none are rejected in the design. Separate `Security` calls remain independent alternatives. -Regenerate endpoints and OpenAPI after upgrading; callbacks can continue using -`scheme.Validate(grantedScopes)` without filtering another scheme's scopes. +Use `ScopeFor(oauth, "read")` inside `Security` when OAuth schemes have +independent requirements, including overlapping scope inventories. Use +`RoleFor(key, "admin")` for non-OAuth role names. Both calls add requirements only +to their selected scheme; they add to any shared `Scope` declarations. Empty +per-scheme lists require authentication without extra grants. A name required +by one scheme never becomes required by another merely because both declare it. + +Role names have no protocol-defined inventory and are not exchanged on the +wire. Basic, API-key and JWT callbacks receive these application permission +names in `RequiredScopes`; `scheme.Validate(grantedNames)` requires all of them. +The application obtains the granted names from its authenticated principal and +must call `Validate` or enforce equivalent checks before returning success. +OAuth callbacks continue receiving declared OAuth scopes in the same field. +ScopeFor on non-OAuth schemes, RoleFor on OAuth schemes, unknown selected +schemes and unknown OAuth scopes are rejected. + +OAuth flow constructors accept an optional final block of `Scope(name, description)` +declarations. A flow without a block inherits the scheme-level scopes. A supplied +block replaces those defaults; `func() {}` declares an empty inventory. Different +flows may use disjoint names or different descriptions for the same name. +Requirements may reference names available in any effective flow. The generated +OAuth callback receives each flow's inventory in `Flows`; `Scopes` lists the +available names across flows. Neither inventory grants access: the application +validates the token, its issuer and flow, and its actual granted scopes. + +Regenerate endpoints and OpenAPI after upgrading. OpenAPI now exports required +non-OAuth names as role arrays, including requirements authored with shared +`Scope`. Existing credential wire formats and the callbacks' validation rule +are unchanged. `StrictAuthorization()` at API or service scope requires every method to declare `Authorize`, `AuthorizeBy`, or `NoAccessCheck`. Existing designs remain unchanged diff --git a/docs/dsl-reference.md b/docs/dsl-reference.md index 53680361f..8589a881f 100644 --- a/docs/dsl-reference.md +++ b/docs/dsl-reference.md @@ -1765,6 +1765,26 @@ semantics and chain the context returned by each successful authorizer. Only the context from the requirement that ultimately succeeds reaches the service method. +Within a requirement, `Scope("read")` applies to each scheme that declares that +name. Use `ScopeFor(oauth, "read")` to require it from only one OAuth scheme, +and `RoleFor(key, "admin")` to require an application role from one non-OAuth +scheme. These calls add to shared Scope declarations and accept a scheme name +or reference already listed in Security. Unknown OAuth scopes and references to +schemes outside that requirement are rejected; role names need no inventory. +Callbacks receive their own required names in `RequiredScopes` and validate +application-owned grants with `scheme.Validate(grantedNames)`. Roles are not +extra credentials exchanged on the wire. Regenerate transports and OpenAPI; +required non-OAuth names are now emitted as OpenAPI role arrays. + +OAuth flow constructors accept an optional final block of `Scope(name, description)` +declarations. A flow without a block inherits the scheme-level scopes. A supplied +block replaces those defaults; `func() {}` declares an empty inventory. Different +flows may use disjoint names or different descriptions for the same name. +Requirements may reference names available in any effective flow. The generated +OAuth callback receives each flow's inventory in `Flows`; `Scopes` lists the +available names across flows. Neither inventory grants access: the application +validates the token, its issuer and flow, and its actual granted scopes. + Each separate `Security(...)` call adds an OR alternative. `Security()` with no scheme adds an anonymous alternative, so callers may satisfy the endpoint without credentials while the other alternatives remain available. This maps diff --git a/docs/openapi-import-coverage.md b/docs/openapi-import-coverage.md index 24998fb3b..8245b5d6d 100644 --- a/docs/openapi-import-coverage.md +++ b/docs/openapi-import-coverage.md @@ -91,7 +91,7 @@ reads. Specification extensions are patterns, not fixed fields. | Responses | Concrete status-code entries | None | `default` and `x-*` | | Response | `summary`, `description`, supported headers and content, and supported `x-*` | None | Direct `$ref` plans and links | | Header | `description`, `required`, schema, `style: simple`, and `allowReserved` | `deprecated`, unsupported examples | Other serialization, content, direct `$ref`, and `x-*` | -| Security Scheme | API keys, HTTP basic, HTTP bearer, and supported OAuth 2.0 flows and scopes | None | OpenID Connect, bearer formats, device authorization, and incompatible OAuth scope maps | +| Security Scheme | API keys, HTTP basic, HTTP bearer, and supported OAuth 2.0 flows and scopes | None | OpenID Connect, bearer formats, device authorization | | Example | JSON-compatible inline, structured, and reusable component examples | Unrenderable examples | External values and incompatible value forms | @@ -106,16 +106,24 @@ in application code. Duplicate credentials no longer become unrelated request body fields. Each security requirement object is a conjunction; separate objects remain -alternatives. Loom applies one scope list to every scheme in a conjunction, so -import rejects different per-scheme lists rather than changing authentication. -For example, `[{Bearer: []}, {OAuth: [read]}]` is supported, while -`[{Bearer: [], OAuth: [read]}]` is not. Two OAuth schemes with the same required -scopes and conjunctions with empty lists remain supported. This restriction also -applies with `--allow-lossy`. Partial import skips operations that inherit an -unsupported root requirement while retaining public or secured operations that -explicitly override it. The retained design includes root security only when a -retained operation inherits it; strict whole-document import still rejects the -unsupported root declaration. +alternatives. Import preserves each scheme's independent array, including empty +arrays and overlapping OAuth scope inventories. For example, +`[{Bearer: [admin], OAuth: [read]}]` requires both the application's `admin` +role and the OAuth `read` scope. Import renders `ScopeFor` for OAuth scopes and +`RoleFor` for non-OAuth role names. Roles have no protocol-defined inventory; +unknown OAuth scopes still fail closed. Non-OAuth role arrays require OpenAPI +3.1 or 3.2; OpenAPI 3.0 requires those arrays to be empty. Anonymous alternatives and explicit +operation overrides remain distinct. Partial import skips operations that +inherit an invalid root requirement while retaining public or secured overrides. +The retained design includes root security only when a retained operation +inherits it. `security_per_scheme_test.go` and the generated credential fixture +cover per-scheme fidelity and runtime enforcement. + +OAuth flows retain their own scope names and descriptions, including disjoint, +overlapping and empty inventories. Import emits a flow-local Scope block for +each flow. Regeneration preserves the exact flow maps; requirement validation +checks names across the effective inventories without assigning token grants. +`security_flow_scopes_test.go` covers the imported and regenerated contract. Path and operation extensions use a documented allowlist. Unknown extensions at those supported scopes produce diagnostics. Document, schema, parameter, diff --git a/dsl/security.go b/dsl/security.go index f9b16169b..9313001cd 100644 --- a/dsl/security.go +++ b/dsl/security.go @@ -236,7 +236,9 @@ func CookieName(name string) { // The requirement refers to one or more OAuth2Security, BasicAuthSecurity, // APIKeySecurity or JWTSecurity security scheme. Required scopes may be listed by // name in the Security DSL. Each scheme receives the required scopes that it -// declares; scopes declared by no scheme are rejected. All the listed schemes +// declares; scopes declared by no scheme are rejected. ScopeFor binds OAuth2 +// scopes to one scheme; RoleFor binds application roles to one non-OAuth scheme. +// These explicit requirements add to the shared Scope requirements. All listed schemes // must be validated for the request to be authorized. Security may appear multiple times // in the same scope in which case the client may validate any one of the // requirements for the request to be authorized. diff --git a/dsl/security_flow_scopes_test.go b/dsl/security_flow_scopes_test.go new file mode 100644 index 000000000..2425c8bfa --- /dev/null +++ b/dsl/security_flow_scopes_test.go @@ -0,0 +1,104 @@ +package dsl_test + +import ( + "testing" + + "github.com/stretchr/testify/require" + + . "github.com/CaliLuke/loom/dsl" + "github.com/CaliLuke/loom/expr" +) + +func TestOAuthFlowScopeDSL(t *testing.T) { + root := expr.RunDSL(t, func() { + auth := OAuth2Security("auth", func() { + AuthorizationCodeFlow("https://example.com/auth", "https://example.com/token", "", func() { + Scope("read", "User consent") + }) + ImplicitFlow("https://example.com/auth", "", func() {}) + PasswordFlow("https://example.com/token", "") + ClientCredentialsFlow("https://example.com/token", "", func() { + Scope("write", "Machine grant") + }) + DeviceAuthorizationFlow("https://example.com/device", "https://example.com/token", "", func() { + Scope("device", "Device grant") + }) + Scope("default", "Shared default declared after flows") + }) + Service("flows", func() { + Method("read", func() { + Security(auth, func() { + ScopeFor(auth, "read", "write", "device", "default") + }) + Payload(func() { + AccessToken("token") + }) + }) + }) + }) + scheme := root.Schemes[0] + require.Len(t, scheme.Flows, 5) + for i, want := range [][]string{{"read"}, {}, {"default"}, {"write"}, {"device"}} { + scopes := scheme.FlowScopes(scheme.Flows[i]) + names := make([]string, 0, len(scopes)) + for _, scope := range scopes { + names = append(names, scope.Name) + } + require.ElementsMatch(t, want, names) + } +} + +func TestOAuthFlowScopeBlockErrors(t *testing.T) { + for _, tc := range []struct { + name string + body func() + want string + }{ + {"multiple blocks", func() { + ClientCredentialsFlow("https://example.com/token", "", func() {}, func() {}) + }, "too many"}, + {"nil block", func() { + ClientCredentialsFlow("https://example.com/token", "", nil) + }, "must not be nil"}, + {"invalid nested DSL", func() { + ClientCredentialsFlow("https://example.com/token", "", func() { + RoleFor("auth", "admin") + }) + }, "invalid use"}, + } { + t.Run(tc.name, func(t *testing.T) { + err := expr.RunInvalidDSL(t, func() { + OAuth2Security("auth", tc.body) + }) + require.ErrorContains(t, err, tc.want) + }) + } +} + +func TestOAuthFlowScopeDefaultsCannotGrantUnavailableNames(t *testing.T) { + for _, explicit := range []bool{false, true} { + t.Run(map[bool]string{false: "shared requirement", true: "scheme requirement"}[explicit], func(t *testing.T) { + err := expr.RunInvalidDSL(t, func() { + auth := OAuth2Security("auth", func() { + Scope("default") + ClientCredentialsFlow("https://example.com/token", "", func() {}) + }) + Service("flows", func() { + Method("read", func() { + Security(auth, func() { + if explicit { + ScopeFor(auth, "default") + } else { + Scope("default") + } + }) + Payload(func() { + AccessToken("token") + }) + }) + }) + }) + require.ErrorContains(t, err, "default") + }) + } +} diff --git a/dsl/security_oauth_flows.go b/dsl/security_oauth_flows.go index fda532e85..dc232b33b 100644 --- a/dsl/security_oauth_flows.go +++ b/dsl/security_oauth_flows.go @@ -5,13 +5,14 @@ import ( "github.com/CaliLuke/loom/expr" ) -// Scope has two uses: in a security scheme it defines a scope supported by -// that scheme. In Security it lists required scopes. +// Scope defines a supported scope in a security scheme, or a default for +// OAuth flows. In an OAuth flow block it defines +// that flow's complete inventory. In Security it lists required scopes. // -// Scope must appear in Security, BasicAuthSecurity, APIKeySecurity, JWTSecurity or OAuth2Security. +// Scope must appear in Security, a security scheme, or an OAuth flow block. // // Scope accepts one or two arguments: the first argument is the scope name and -// when used in a scheme the optional second argument is a description. +// when used in a scheme or flow the optional second argument is a description. // // Example: // @@ -26,24 +27,25 @@ import ( // }) // }) func Scope(name string, desc ...string) { + if len(desc) > 1 { + eval.TooManyArgError() + return + } + description := "no description" + if len(desc) == 1 { + description = desc[0] + } switch current := eval.Current().(type) { case *expr.SecurityExpr: - if len(desc) >= 1 { + if len(desc) > 0 { eval.TooManyArgError() return } current.Scopes = append(current.Scopes, name) case *expr.SchemeExpr: - if len(desc) > 1 { - eval.TooManyArgError() - return - } - d := "no description" - if len(desc) == 1 { - d = desc[0] - } - current.Scopes = append(current.Scopes, - &expr.ScopeExpr{Name: name, Description: d}) + current.Scopes = append(current.Scopes, &expr.ScopeExpr{Name: name, Description: description}) + case *expr.FlowExpr: + current.Scopes = append(current.Scopes, &expr.ScopeExpr{Name: name, Description: description}) default: eval.IncompatibleDSL() } @@ -55,23 +57,15 @@ func Scope(name string, desc ...string) { // AuthorizationCodeFlow must be used in OAuth2Security. // // AuthorizationCodeFlow accepts three arguments: the authorization, token and -// refresh URLs. -func AuthorizationCodeFlow(authorizationURL, tokenURL, refreshURL string) { - current, ok := eval.Current().(*expr.SchemeExpr) - if !ok { - eval.IncompatibleDSL() - return - } - if current.Kind != expr.OAuth2Kind { - eval.ReportError("cannot specify flow for non-oauth2 security scheme.") - return - } - current.Flows = append(current.Flows, &expr.FlowExpr{ +// refresh URLs, followed by an optional flow-local Scope block. A supplied +// block replaces scheme-level scope defaults, including when empty. +func AuthorizationCodeFlow(authorizationURL, tokenURL, refreshURL string, fn ...func()) { + oauthFlow(&expr.FlowExpr{ Kind: expr.AuthorizationCodeFlowKind, AuthorizationURL: authorizationURL, TokenURL: tokenURL, RefreshURL: refreshURL, - }) + }, fn) } // ImplicitFlow defines an implicit OAuth2 flow as described in section 1.3.2 @@ -79,22 +73,14 @@ func AuthorizationCodeFlow(authorizationURL, tokenURL, refreshURL string) { // // ImplicitFlow must be used in OAuth2Security. // -// ImplicitFlow accepts two arguments: the authorization and refresh URLs. -func ImplicitFlow(authorizationURL, refreshURL string) { - current, ok := eval.Current().(*expr.SchemeExpr) - if !ok { - eval.IncompatibleDSL() - return - } - if current.Kind != expr.OAuth2Kind { - eval.ReportError("cannot specify flow for non-oauth2 security scheme.") - return - } - current.Flows = append(current.Flows, &expr.FlowExpr{ +// ImplicitFlow accepts the authorization and refresh URLs and an optional +// flow-local Scope block. A supplied block replaces scheme-level defaults. +func ImplicitFlow(authorizationURL, refreshURL string, fn ...func()) { + oauthFlow(&expr.FlowExpr{ Kind: expr.ImplicitFlowKind, AuthorizationURL: authorizationURL, RefreshURL: refreshURL, - }) + }, fn) } // PasswordFlow defines an Resource Owner Password Credentials OAuth2 flow as @@ -102,22 +88,14 @@ func ImplicitFlow(authorizationURL, refreshURL string) { // // PasswordFlow must be used in OAuth2Security. // -// PasswordFlow accepts two arguments: the token and refresh URLs. -func PasswordFlow(tokenURL, refreshURL string) { - current, ok := eval.Current().(*expr.SchemeExpr) - if !ok { - eval.IncompatibleDSL() - return - } - if current.Kind != expr.OAuth2Kind { - eval.ReportError("cannot specify flow for non-oauth2 security scheme.") - return - } - current.Flows = append(current.Flows, &expr.FlowExpr{ +// PasswordFlow accepts the token and refresh URLs and an optional flow-local +// Scope block. A supplied block replaces scheme-level defaults. +func PasswordFlow(tokenURL, refreshURL string, fn ...func()) { + oauthFlow(&expr.FlowExpr{ Kind: expr.PasswordFlowKind, TokenURL: tokenURL, RefreshURL: refreshURL, - }) + }, fn) } // ClientCredentialsFlow defines an clientCredentials OAuth2 flow as described @@ -125,27 +103,29 @@ func PasswordFlow(tokenURL, refreshURL string) { // // ClientCredentialsFlow must be used in OAuth2Security. // -// ClientCredentialsFlow accepts two arguments: the token and refresh URLs. -func ClientCredentialsFlow(tokenURL, refreshURL string) { - current, ok := eval.Current().(*expr.SchemeExpr) - if !ok { - eval.IncompatibleDSL() - return - } - if current.Kind != expr.OAuth2Kind { - eval.ReportError("cannot specify flow for non-oauth2 security scheme.") - return - } - current.Flows = append(current.Flows, &expr.FlowExpr{ +// ClientCredentialsFlow accepts the token and refresh URLs and an optional +// flow-local Scope block. A supplied block replaces scheme-level defaults. +func ClientCredentialsFlow(tokenURL, refreshURL string, fn ...func()) { + oauthFlow(&expr.FlowExpr{ Kind: expr.ClientCredentialsFlowKind, TokenURL: tokenURL, RefreshURL: refreshURL, - }) + }, fn) } // DeviceAuthorizationFlow defines an OAuth 2 device authorization flow as -// described by RFC 8628. It must be used in OAuth2Security. -func DeviceAuthorizationFlow(deviceAuthorizationURL, tokenURL, refreshURL string) { +// described by RFC 8628. It must be used in OAuth2Security. An optional Scope +// block supplies the complete flow inventory instead of scheme-level defaults. +func DeviceAuthorizationFlow(deviceAuthorizationURL, tokenURL, refreshURL string, fn ...func()) { + oauthFlow(&expr.FlowExpr{ + Kind: expr.DeviceAuthorizationFlowKind, + DeviceAuthorizationURL: deviceAuthorizationURL, + TokenURL: tokenURL, + RefreshURL: refreshURL, + }, fn) +} + +func oauthFlow(flow *expr.FlowExpr, fn []func()) { current, ok := eval.Current().(*expr.SchemeExpr) if !ok { eval.IncompatibleDSL() @@ -155,10 +135,18 @@ func DeviceAuthorizationFlow(deviceAuthorizationURL, tokenURL, refreshURL string eval.ReportError("cannot specify flow for non-oauth2 security scheme.") return } - current.Flows = append(current.Flows, &expr.FlowExpr{ - Kind: expr.DeviceAuthorizationFlowKind, - DeviceAuthorizationURL: deviceAuthorizationURL, - TokenURL: tokenURL, - RefreshURL: refreshURL, - }) + if len(fn) > 1 { + eval.TooManyArgError() + return + } + if len(fn) == 1 { + flow.Scopes = []*expr.ScopeExpr{} + if fn[0] == nil || !eval.Execute(fn[0], flow) { + if fn[0] == nil { + eval.ReportError("OAuth flow scope block must not be nil") + } + return + } + } + current.Flows = append(current.Flows, flow) } diff --git a/dsl/security_requirements.go b/dsl/security_requirements.go new file mode 100644 index 000000000..bc6c763f9 --- /dev/null +++ b/dsl/security_requirements.go @@ -0,0 +1,58 @@ +package dsl + +import ( + "github.com/CaliLuke/loom/eval" + "github.com/CaliLuke/loom/expr" +) + +// ScopeFor requires OAuth2 scopes from one scheme in the enclosing Security +// requirement. The scheme is a name or a scheme reference already listed in +// Security. Each scope must be declared by that scheme. Repeated calls add +// requirements; they do not replace Scope or another scheme's requirements. +func ScopeFor(scheme any, scopes ...string) { + requireSchemeNames(scheme, scopes, true) +} + +// RoleFor requires application role names from one non-OAuth scheme in the +// enclosing Security requirement. The scheme is a name or a scheme reference +// already listed in Security. Roles need no scheme-level inventory and are not +// exchanged on the wire. The application's authentication callback receives +// them in RequiredScopes and validates them against its own role assignments. +func RoleFor(scheme any, roles ...string) { + requireSchemeNames(scheme, roles, false) +} + +func requireSchemeNames(scheme any, names []string, oauth bool) { + current, ok := eval.Current().(*expr.SecurityExpr) + if !ok { + eval.IncompatibleDSL() + return + } + var name string + switch value := scheme.(type) { + case string: + name = value + case *expr.SchemeExpr: + if value != nil { + name = value.SchemeName + } + default: + eval.InvalidArgError("security scheme or security scheme name", scheme) + return + } + for _, selected := range current.Schemes { + if selected.SchemeName != name { + continue + } + if (selected.Kind == expr.OAuth2Kind) != oauth || selected.Kind == expr.NoKind { + eval.ReportError("ScopeFor requires an OAuth2 scheme; RoleFor requires a non-OAuth scheme") + return + } + if current.SchemeRequirements == nil { + current.SchemeRequirements = make(map[string][]string) + } + current.SchemeRequirements[name] = append(current.SchemeRequirements[name], names...) + return + } + eval.ReportError("security scheme %q is not in this requirement", name) +} diff --git a/dsl/security_requirements_test.go b/dsl/security_requirements_test.go new file mode 100644 index 000000000..74e6fe488 --- /dev/null +++ b/dsl/security_requirements_test.go @@ -0,0 +1,63 @@ +package dsl_test + +import ( + "testing" + + "github.com/stretchr/testify/require" + + . "github.com/CaliLuke/loom/dsl" + "github.com/CaliLuke/loom/expr" +) + +func TestPerSchemeRequirementsDSL(t *testing.T) { + design := func(requirements func()) func() { + return func() { + OAuth2Security("oauth", func() { + ClientCredentialsFlow("https://example.com/token", "") + Scope("read") + }) + APIKeySecurity("key") + Service("secure", func() { + Security("oauth", "key", requirements) + Method("read", func() { + Payload(func() { + AccessToken("token") + APIKey("key", "key") + }) + }) + }) + } + } + root := expr.RunDSL(t, design(func() { + ScopeFor("oauth", "read") + RoleFor("key", "admin") + })) + req := root.Services[0].Methods[0].EffectiveRequirements()[0] + require.Equal(t, map[string][]string{"oauth": {"read"}, "key": {"admin"}}, req.SchemeRequirements) + for _, tc := range []struct { + name string + dsl func() + want string + }{ + {"unknown scope", func() { + ScopeFor("oauth", "unknown") + }, "not found in scheme"}, + {"wrong scheme", func() { + ScopeFor("missing", "read") + }, "not in this requirement"}, + {"roles on oauth", func() { + RoleFor("oauth", "admin") + }, "non-OAuth scheme"}, + {"scopes on key", func() { + ScopeFor("key", "read") + }, "OAuth2 scheme"}, + {"nil scheme", func() { + RoleFor((*expr.SchemeExpr)(nil), "admin") + }, "not in this requirement"}, + } { + t.Run(tc.name, func(t *testing.T) { + err := expr.RunInvalidDSL(t, design(tc.dsl)) + require.ErrorContains(t, err, tc.want) + }) + } +} diff --git a/expr/method.go b/expr/method.go index 6541bcb94..d3cf7e1b6 100644 --- a/expr/method.go +++ b/expr/method.go @@ -3,6 +3,7 @@ package expr import ( "errors" "fmt" + "slices" "github.com/CaliLuke/loom/eval" ) @@ -419,22 +420,7 @@ func copyReqs(reqs []*SecurityExpr) []*SecurityExpr { } reqs2 := make([]*SecurityExpr, len(reqs)) for i, req := range reqs { - req2 := &SecurityExpr{Scopes: req.Scopes} - schs := make([]*SchemeExpr, len(req.Schemes)) - for j, sch := range req.Schemes { - schs[j] = &SchemeExpr{ - Kind: sch.Kind, - SchemeName: sch.SchemeName, - Description: sch.Description, - In: sch.In, - Name: sch.Name, - Scopes: sch.Scopes, - Flows: sch.Flows, - Meta: sch.Meta, - } - } - req2.Schemes = schs - reqs2[i] = req2 + reqs2[i] = DupRequirement(req) } return reqs2 } @@ -532,7 +518,7 @@ func containsRequirement(requirements []*SecurityExpr, candidate *SecurityExpr) } func requirementEqual(left *SecurityExpr, right *SecurityExpr) bool { - if len(left.Scopes) != len(right.Scopes) || len(left.Schemes) != len(right.Schemes) { + if len(left.Scopes) != len(right.Scopes) || len(left.Schemes) != len(right.Schemes) || len(left.SchemeRequirements) != len(right.SchemeRequirements) { return false } for i, scope := range left.Scopes { @@ -540,6 +526,12 @@ func requirementEqual(left *SecurityExpr, right *SecurityExpr) bool { return false } } + for name, names := range left.SchemeRequirements { + other, ok := right.SchemeRequirements[name] + if !ok || !slices.Equal(names, other) { + return false + } + } for i, scheme := range left.Schemes { other := right.Schemes[i] if scheme.Kind != other.Kind || scheme.SchemeName != other.SchemeName || scheme.In != other.In || scheme.Name != other.Name { diff --git a/expr/method_validate_security.go b/expr/method_validate_security.go index 505526350..bebbf2592 100644 --- a/expr/method_validate_security.go +++ b/expr/method_validate_security.go @@ -2,6 +2,8 @@ package expr import ( "fmt" + "maps" + "slices" "strings" "github.com/CaliLuke/loom/eval" @@ -162,6 +164,28 @@ func (m *MethodExpr) validateRequirementScopes(r *SecurityExpr) *eval.Validation } verr.Add(m, "security scope %q not found in any of the security schemes.", scope) } + for _, name := range slices.Sorted(maps.Keys(r.SchemeRequirements)) { + names := r.SchemeRequirements[name] + var selected *SchemeExpr + for _, scheme := range r.Schemes { + if scheme.SchemeName == name { + selected = scheme + break + } + } + if selected == nil || selected.Kind == NoKind { + verr.Add(m, "security scheme %q is not in this requirement", name) + continue + } + if selected.Kind != OAuth2Kind { + continue + } + for _, scope := range names { + if !requirementHasScope(&SecurityExpr{Schemes: []*SchemeExpr{selected}}, scope) { + verr.Add(m, "security scope %q not found in scheme %q", scope, name) + } + } + } return verr } @@ -170,10 +194,8 @@ func requirementHasScope(r *SecurityExpr, scope string) bool { if !schemeSupportsScopes(s) { continue } - for _, se := range s.Scopes { - if se.Name == scope { - return true - } + if slices.Contains(s.ScopeNames(), scope) { + return true } } return false diff --git a/expr/security.go b/expr/security.go index c5874ef81..b3c2a5031 100644 --- a/expr/security.go +++ b/expr/security.go @@ -66,6 +66,10 @@ type ( Schemes []*SchemeExpr // Scopes list the required scopes if any. Scopes []string + // SchemeRequirements holds names required only by the named scheme. + // OAuth2 values are declared scopes; other values are application roles. + // These requirements are additive to Scopes, never shared across schemes. + SchemeRequirements map[string][]string } // SessionAuthExpr defines a logical auth contract backed by one or more @@ -110,7 +114,7 @@ type ( // Name refers to a header or parameter name, based on In's // value. Name string - // Scopes lists the Basic, APIKey, JWT or OAuth2 scopes. + // Scopes lists Basic, APIKey or JWT scopes, or default OAuth2 flow scopes. Scopes []*ScopeExpr // Flows determine the oauth2 flows supported by this scheme. Flows []*FlowExpr @@ -132,6 +136,9 @@ type ( RefreshURL string // DeviceAuthorizationURL starts a device authorization flow. DeviceAuthorizationURL string + // Scopes is this flow's complete scope inventory. Nil inherits the + // scheme defaults; a nonnil empty slice declares no scopes. + Scopes []*ScopeExpr } // ScopeExpr defines a security scope. @@ -263,8 +270,9 @@ func (s *SecurityExpr) EvalName() string { // DupRequirement creates a copy of the given security requirement. func DupRequirement(req *SecurityExpr) *SecurityExpr { dup := &SecurityExpr{ - Scopes: req.Scopes, - Schemes: make([]*SchemeExpr, 0, len(req.Schemes)), + Scopes: append([]string(nil), req.Scopes...), + Schemes: make([]*SchemeExpr, 0, len(req.Schemes)), + SchemeRequirements: dupSchemeRequirements(req.SchemeRequirements), } for _, s := range req.Schemes { dup.Schemes = append(dup.Schemes, DupScheme(s)) @@ -279,10 +287,16 @@ func DupScheme(sch *SchemeExpr) *SchemeExpr { SchemeName: sch.SchemeName, Description: sch.Description, In: sch.In, - Scopes: sch.Scopes, - Flows: sch.Flows, + Name: sch.Name, + Scopes: dupScopeDeclarations(sch.Scopes), + Flows: append([]*FlowExpr(nil), sch.Flows...), Meta: sch.Meta, } + for i, flow := range sch.Flows { + copy := *flow + copy.Scopes = dupScopeDeclarations(flow.Scopes) + dup.Flows[i] = © + } return &dup } diff --git a/expr/security_flow_scopes_test.go b/expr/security_flow_scopes_test.go new file mode 100644 index 000000000..6b4e1c380 --- /dev/null +++ b/expr/security_flow_scopes_test.go @@ -0,0 +1,48 @@ +package expr + +import ( + "testing" + + "github.com/stretchr/testify/require" +) + +func TestOAuthFlowScopeOwnership(t *testing.T) { + scheme := &SchemeExpr{Kind: OAuth2Kind, Scopes: []*ScopeExpr{{Name: "shared", Description: "default"}}, Flows: []*FlowExpr{ + {Kind: PasswordFlowKind}, + {Kind: ImplicitFlowKind, Scopes: []*ScopeExpr{}}, + {Kind: ClientCredentialsFlowKind, Scopes: []*ScopeExpr{{Name: "write", Description: "machine"}, {Name: "shared", Description: "client"}}}, + }} + require.Equal(t, scheme.Scopes, scheme.FlowScopes(scheme.Flows[0])) + require.Empty(t, scheme.FlowScopes(scheme.Flows[1])) + require.NotNil(t, scheme.FlowScopes(scheme.Flows[1])) + require.Equal(t, "client", scheme.FlowScopes(scheme.Flows[2])[1].Description) + require.Equal(t, []string{"shared", "write"}, scheme.ScopeNames()) + require.Equal(t, []string{"write"}, (&SecurityExpr{Scopes: []string{"write"}}).RequiredScopes(scheme)) + copy := DupScheme(scheme) + copy.Scopes[0].Description = "changed" + copy.Flows[2].Scopes[0].Name = "changed" + require.Equal(t, "default", scheme.Scopes[0].Description) + require.Equal(t, "write", scheme.Flows[2].Scopes[0].Name) + require.NotNil(t, copy.Flows[1].Scopes) + projected := scheme.FlowScopes(scheme.Flows[0]) + projected[0].Name = "changed" + require.Equal(t, "shared", scheme.Scopes[0].Name) +} + +func TestOAuthFlowScopeDuplicateDescriptions(t *testing.T) { + for _, local := range []bool{false, true} { + t.Run(map[bool]string{false: "defaults", true: "local"}[local], func(t *testing.T) { + inventory := []*ScopeExpr{{Name: "read", Description: "first"}, {Name: "write", Description: "other"}, {Name: "read", Description: "last"}} + scheme := &SchemeExpr{Scopes: inventory} + flow := &FlowExpr{} + if local { + flow.Scopes = inventory + } + got := scheme.FlowScopes(flow) + require.Equal(t, []*ScopeExpr{{Name: "read", Description: "last"}, {Name: "write", Description: "other"}}, got) + require.Equal(t, "first", inventory[0].Description) + got[0].Description = "changed" + require.Equal(t, "last", inventory[2].Description) + }) + } +} diff --git a/expr/security_scopes.go b/expr/security_scopes.go index 75c65641f..51a11adcd 100644 --- a/expr/security_scopes.go +++ b/expr/security_scopes.go @@ -1,17 +1,95 @@ package expr +import "slices" + // RequiredScopes returns the requirement scopes declared by scheme, in authored -// requirement order. The result is independently owned. Requirement validation -// separately rejects scopes absent from every scheme in the requirement. +// requirement order, followed by its explicit per-scheme requirements. For +// non-OAuth schemes the explicit names are application roles. The result is +// independently owned. Validation separately rejects unknown OAuth scopes. func (s *SecurityExpr) RequiredScopes(scheme *SchemeExpr) []string { var scopes []string + inventory := scheme.ScopeNames() for _, required := range s.Scopes { - for _, declared := range scheme.Scopes { - if required == declared.Name { - scopes = append(scopes, required) - break - } + if slices.Contains(inventory, required) { + scopes = append(scopes, required) + } + } + for _, name := range s.SchemeRequirements[scheme.SchemeName] { + if !slices.Contains(scopes, name) { + scopes = append(scopes, name) } } return scopes } + +func dupSchemeRequirements(source map[string][]string) map[string][]string { + if source == nil { + return nil + } + result := make(map[string][]string, len(source)) + for scheme, names := range source { + result[scheme] = slices.Clone(names) + } + return result +} + +// FlowScopes returns a flow's effective scope inventory. A nil flow inventory +// inherits the scheme defaults; a nonnil empty inventory remains empty. The +// returned scope declarations are independently owned. Repeated names retain +// their first position and last description, matching OpenAPI map semantics. +func (s *SchemeExpr) FlowScopes(flow *FlowExpr) []*ScopeExpr { + inventory := flow.Scopes + if inventory == nil { + inventory = s.Scopes + } + if inventory == nil { + return nil + } + result := make([]*ScopeExpr, 0, len(inventory)) + positions := make(map[string]int, len(inventory)) + for _, scope := range inventory { + if position, exists := positions[scope.Name]; exists { + result[position].Description = scope.Description + continue + } + positions[scope.Name] = len(result) + copy := *scope + result = append(result, ©) + } + return result +} + +// ScopeNames returns the names available to requirement validation and auth +// callbacks, in first-declaration order. For OAuth2 it collects names from the +// effective flow inventories without combining their descriptions or changing +// any flow. Flow selection and granted permissions remain application-owned. +func (s *SchemeExpr) ScopeNames() []string { + inventories := [][]*ScopeExpr{s.Scopes} + if s.Kind == OAuth2Kind && len(s.Flows) > 0 { + inventories = make([][]*ScopeExpr, len(s.Flows)) + for i, flow := range s.Flows { + inventories[i] = s.FlowScopes(flow) + } + } + var names []string + for _, inventory := range inventories { + for _, scope := range inventory { + if !slices.Contains(names, scope.Name) { + names = append(names, scope.Name) + } + } + } + return names +} + +func dupScopeDeclarations(source []*ScopeExpr) []*ScopeExpr { + if source == nil { + return nil + } + result := make([]*ScopeExpr, len(source)) + for i, scope := range source { + value := *scope + result[i] = &value + } + return result +} diff --git a/expr/security_scopes_test.go b/expr/security_scopes_test.go index 81a6ce77d..767941455 100644 --- a/expr/security_scopes_test.go +++ b/expr/security_scopes_test.go @@ -24,3 +24,22 @@ func TestRequiredScopeProjection(t *testing.T) { }) } } + +func TestPerSchemeRequirementOwnership(t *testing.T) { + left := &SchemeExpr{SchemeName: "left", Kind: OAuth2Kind, Name: "Authorization", Scopes: []*ScopeExpr{{Name: "read"}}} + right := &SchemeExpr{SchemeName: "right", Kind: OAuth2Kind, Scopes: left.Scopes} + key := &SchemeExpr{SchemeName: "key", Kind: APIKeyKind} + req := &SecurityExpr{Schemes: []*SchemeExpr{left, right, key}, SchemeRequirements: map[string][]string{"left": {"read"}, "right": {}, "key": {"admin"}}} + require.Equal(t, []string{"read"}, req.RequiredScopes(left)) + require.Empty(t, req.RequiredScopes(right), "a shared inventory must not share requirements") + require.Equal(t, []string{"admin"}, req.RequiredScopes(key), "roles need no inventory") + copy := DupRequirement(req) + require.True(t, requirementEqual(req, copy)) + require.Equal(t, "Authorization", copy.Schemes[0].Name) + copy.SchemeRequirements["left"][0] = "write" + require.False(t, requirementEqual(req, copy)) + require.Equal(t, []string{"read"}, req.RequiredScopes(left)) + req.Scopes = []string{"read"} + require.Equal(t, []string{"read"}, req.RequiredScopes(left), "shared and explicit requirements are additive without duplicates") + require.Equal(t, []string{"read"}, req.RequiredScopes(right)) +} diff --git a/expr/testdata/security_scope.go b/expr/testdata/security_scope.go index bc2b58d6e..d95df4456 100644 --- a/expr/testdata/security_scope.go +++ b/expr/testdata/security_scope.go @@ -157,3 +157,56 @@ func matrixMethod(transport, path string, security func(), credentials ...string JSONRPC(func() {}) } } + +// SecurityPerSchemeDSL checks separate requirements on schemes with overlapping +// scope inventories and separate alternatives on one physical credential. +func SecurityPerSchemeDSL(transport string) func() { + return func() { + left := OAuth2Security("left", func() { + ClientCredentialsFlow("https://example.com/token", "", func() { + Scope("read") + Scope("write") + }) + }) + right := OAuth2Security("right", func() { + ClientCredentialsFlow("https://example.com/token", "", func() { + Scope("read") + Scope("write") + }) + }) + API("per_scheme", func() { + if transport == "jsonrpc" { + JSONRPC(func() {}) + } + }) + Service("per_scheme", func() { + matrixTransportService(transport, "per_scheme") + Method("read", func() { + Security(left, right, func() { + ScopeFor(left, "read") + ScopeFor(right, "write") + }) + Security(right, func() { + ScopeFor(right, "read") + }) + Security() + Payload(func() { + AccessToken("token") + if transport == "jsonrpc" { + ID("id", String) + } + }) + switch transport { + case "http": + HTTP(func() { + GET("/per_scheme") + }) + case "grpc": + GRPC(func() {}) + case "jsonrpc": + JSONRPC(func() {}) + } + }) + }) + } +} diff --git a/expr/tla/oauth_flow_scopes/Inventories.tla b/expr/tla/oauth_flow_scopes/Inventories.tla new file mode 100644 index 000000000..942f7137f --- /dev/null +++ b/expr/tla/oauth_flow_scopes/Inventories.tla @@ -0,0 +1,15 @@ +------------------------- MODULE Inventories ------------------------- +EXTENDS TLC +CONSTANT Flatten +VARIABLES shared, local, explicit +Flows == {"code", "client"} +Names == {"read", "write"} +Init == /\ shared \in SUBSET Names + /\ local \in [Flows -> SUBSET Names] + /\ explicit \in [Flows -> BOOLEAN] +Expected(f) == IF explicit[f] THEN local[f] ELSE shared +Inventory(f) == IF Flatten THEN UNION {Expected(g): g \in Flows} ELSE Expected(f) +Isolation == \A f \in Flows: Inventory(f) = Expected(f) +Next == UNCHANGED <> +Spec == Init /\ [][Next]_<> +======================================================================= diff --git a/expr/tla/oauth_flow_scopes/README.md b/expr/tla/oauth_flow_scopes/README.md new file mode 100644 index 000000000..02eb0db8d --- /dev/null +++ b/expr/tla/oauth_flow_scopes/README.md @@ -0,0 +1,26 @@ +# OAuth flow inventories + +`Inventories.tla` checks two flows and two scope names with shared defaults, +flow-local inventories, and an explicit-block flag. `FlowScopes` in +`expr/security_scopes.go` implements the selected inventory. A supplied empty +block overrides defaults; a missing block inherits them. `ScopeNames` computes +availability for requirement admission, never a replacement flow inventory. + +Run explicitly with a locally installed TLA+ 1.7.4 jar: + +```sh +java -cp "$TLA_JAR" tlc2.TLC -config checked.cfg Inventories.tla +java -cp "$TLA_JAR" tlc2.TLC -config flatten.cfg Inventories.tla +``` + +The checked model explores 256 distinct states (512 generated) and preserves +Isolation. The flattening candidate fails initially: an empty inherited flow +receives a name declared only by another flow. These are opt-in design checks. +No proof tools are required by ordinary builds, hooks or CI. + +The model abstracts descriptions, URLs and authentication policy. Expression +and DSL tests cover inheritance, explicit empty inventories and deep copies; +import round trips and rendered OpenAPI tests cover exact names/descriptions; +the generated security fixture checks callback inventories, required grants +and denial. Authentication, issuer validation and token grants remain owned +by the application. The finite model does not prove the Go implementation. diff --git a/expr/tla/oauth_flow_scopes/checked.cfg b/expr/tla/oauth_flow_scopes/checked.cfg new file mode 100644 index 000000000..409dabf2a --- /dev/null +++ b/expr/tla/oauth_flow_scopes/checked.cfg @@ -0,0 +1,3 @@ +CONSTANT Flatten = FALSE +SPECIFICATION Spec +INVARIANT Isolation diff --git a/expr/tla/oauth_flow_scopes/flatten.cfg b/expr/tla/oauth_flow_scopes/flatten.cfg new file mode 100644 index 000000000..4b00ed0c9 --- /dev/null +++ b/expr/tla/oauth_flow_scopes/flatten.cfg @@ -0,0 +1,3 @@ +CONSTANT Flatten = TRUE +SPECIFICATION Spec +INVARIANT Isolation diff --git a/expr/tla/security_requirements/README.md b/expr/tla/security_requirements/README.md new file mode 100644 index 000000000..793ef2059 --- /dev/null +++ b/expr/tla/security_requirements/README.md @@ -0,0 +1,35 @@ +# Per-scheme security requirements + +`Requirements.tla` enumerates two schemes, two permission names, every list of +AND branches (including empty/anonymous branches), and every per-scheme grant +set. `checked.cfg` preserves each scheme's required names and passed 4,096 +states (8,192 generated) with TLC 1.7.4. `flatten.cfg` deliberately combines +requirements across a conjunction; its initial-state counterexample requires +no grants from scheme a and read from b, but incorrectly requires read from a. +This compares design candidates; the prior importer explicitly rejected unequal +lists rather than admitting that faulty projection. + +Run explicitly from this directory with Java and the installed TLA+ 1.7.4 jar: + +```sh +java -XX:+UseParallelGC -cp "$TLA_JAR" tlc2.TLC -workers 1 -config checked.cfg -metadir /tmp/loom-security-requirements Requirements.tla +``` + +Substitute `flatten.cfg` and a separate temporary metadir for the negative case. +This is an opt-in design tool, never a build, hook, CI or release gate. + +`expr.SecurityExpr.SchemeRequirements` owns explicit names by scheme identity. +`RequiredScopes` adds shared declared requirements without crossing scheme +boundaries. `ScopeFor` admits OAuth scopes; `RoleFor` admits non-OAuth roles. +Shared Scope remains a convenience for names required by every declaring scheme; +explicit requirements add to it rather than silently replacing authored intent. +Import uses explicit per-scheme requirements. The application determines grants; +callbacks receive them through the existing RequiredScopes/Validate contract. + +Direct DSL/expression tests cover admission, inheritance copies and alternative +identity. The generated scope harness checks exact callback arrays, rejected +roles and grants attached to the wrong scheme. The imported credential fixture +checks wire execution and export. HTTP/gRPC/JSON-RPC projection tests cover shared +service ownership. Wire credential parsing, callbacks, OAuth inventory validity, +list ordering and copying are outside this finite set model and remain Go test +obligations. Keep raw checker output and temporary state out of the repository. diff --git a/expr/tla/security_requirements/Requirements.tla b/expr/tla/security_requirements/Requirements.tla new file mode 100644 index 000000000..fa2bc2e37 --- /dev/null +++ b/expr/tla/security_requirements/Requirements.tla @@ -0,0 +1,20 @@ +--------------------------- MODULE Requirements --------------------------- +EXTENDS TLC +CONSTANT Flatten +VARIABLES requirements, required, granted +Schemes == {"a", "b"} +Names == {"read", "admin"} +Init == /\ requirements \in SUBSET (SUBSET Schemes) + /\ required \in [Schemes -> SUBSET Names] + /\ granted \in [Schemes -> SUBSET Names] +Expected == \E branch \in requirements: \A scheme \in branch: + required[scheme] \subseteq granted[scheme] +Projected(branch, scheme) == IF Flatten + THEN UNION {required[s]: s \in branch} + ELSE required[scheme] +Actual == \E branch \in requirements: \A scheme \in branch: + Projected(branch, scheme) \subseteq granted[scheme] +Agreement == Actual = Expected +Next == UNCHANGED <> +Spec == Init /\ [][Next]_<> +============================================================================= diff --git a/expr/tla/security_requirements/checked.cfg b/expr/tla/security_requirements/checked.cfg new file mode 100644 index 000000000..449cf511e --- /dev/null +++ b/expr/tla/security_requirements/checked.cfg @@ -0,0 +1,3 @@ +CONSTANT Flatten = FALSE +SPECIFICATION Spec +INVARIANT Agreement diff --git a/expr/tla/security_requirements/flatten.cfg b/expr/tla/security_requirements/flatten.cfg new file mode 100644 index 000000000..4838614b2 --- /dev/null +++ b/expr/tla/security_requirements/flatten.cfg @@ -0,0 +1,3 @@ +CONSTANT Flatten = TRUE +SPECIFICATION Spec +INVARIANT Agreement diff --git a/grpc/codegen/security_scope_test.go b/grpc/codegen/security_scope_test.go index cc10f32bc..a75222114 100644 --- a/grpc/codegen/security_scope_test.go +++ b/grpc/codegen/security_scope_test.go @@ -86,3 +86,14 @@ func grpcRequirementSchemeNames(requirements service.RequirementsData) []string } return names } + +func TestGRPCPerSchemeSecurityProjection(t *testing.T) { + root := RunGRPCDSL(t, testdata.SecurityPerSchemeDSL("grpc")) + endpoint := CreateGRPCServices(root).Get("per_scheme").Endpoint("read") + requirements := endpoint.Method.Requirements + require.Len(t, requirements, 3) + require.Equal(t, []string{"read"}, requirements[0].Schemes[0].RequiredScopes) + require.Equal(t, []string{"write"}, requirements[0].Schemes[1].RequiredScopes) + require.Equal(t, []string{"read"}, requirements[1].Schemes[0].RequiredScopes) + require.Empty(t, requirements[2].Schemes) +} diff --git a/http/codegen/openapi/internal/ir/security_bindings.go b/http/codegen/openapi/internal/ir/security_bindings.go index 9a756f9cd..02bb1c1dd 100644 --- a/http/codegen/openapi/internal/ir/security_bindings.go +++ b/http/codegen/openapi/internal/ir/security_bindings.go @@ -147,10 +147,7 @@ func (bindings *securityBindings) requirements(requirements []*expr.SecurityExpr if scheme.Kind == expr.NoKind { continue } - scopes := []string{} - if scheme.Kind == expr.OAuth2Kind { - scopes = append(scopes, requirement.RequiredScopes(scheme)...) - } + scopes := append([]string{}, requirement.RequiredScopes(scheme)...) result[i][bindings.names[securityBindingFor(scheme)]] = scopes } } diff --git a/http/codegen/openapi/v3/builder.go b/http/codegen/openapi/v3/builder.go index cbbe4b72c..9c3960e20 100644 --- a/http/codegen/openapi/v3/builder.go +++ b/http/codegen/openapi/v3/builder.go @@ -538,12 +538,13 @@ func buildSecurityScheme(se *expr.SchemeExpr) *SecurityScheme { } func buildOAuthFlows(se *expr.SchemeExpr) *OAuthFlows { - scopes := make(map[string]string, len(se.Scopes)) - for _, scope := range se.Scopes { - scopes[scope.Name] = scope.Description - } flows := new(OAuthFlows) for _, flow := range se.Flows { + inventory := se.FlowScopes(flow) + scopes := make(map[string]string, len(inventory)) + for _, scope := range inventory { + scopes[scope.Name] = scope.Description + } assignOAuthFlow(flows, flow, scopes) } return flows diff --git a/http/codegen/openapi/v3/security_required_scopes_test.go b/http/codegen/openapi/v3/security_required_scopes_test.go index 644531c3e..6da350532 100644 --- a/http/codegen/openapi/v3/security_required_scopes_test.go +++ b/http/codegen/openapi/v3/security_required_scopes_test.go @@ -1,6 +1,8 @@ package openapiv3_test import ( + "os" + "path/filepath" "testing" "github.com/stretchr/testify/require" @@ -31,3 +33,29 @@ func TestRenderedOAuthRequiredScopeOwnership(t *testing.T) { }) } } + +func TestRenderedPerSchemeRequirements(t *testing.T) { + var vacuum contractVacuum + if os.Getenv("LOOM_OPENAPI_CONTRACT") != "" { + vacuum = installContractVacuum(t) + } + for _, version := range []struct{ target, want string }{ + {"3.1", openapiv3.OpenAPICompatibilityVersion}, {"3.2", openapiv3.OpenAPIVersion}, + } { + t.Run(version.target, func(t *testing.T) { + artifacts := renderOpenAPIArtifactsForVersion(t, servicetestdata.SecurityRequiredScopesDSL, version.target, version.want) + spec := decodeOpenAPIJSON(t, artifacts.JSON) + operation := spec["paths"].(map[string]any)["/isolated"].(map[string]any)["get"].(map[string]any) + require.Equal(t, []any{map[string]any{"oauth": []any{"shared"}, "second": []any{"oauth"}, "key": []any{"admin"}}}, operation["security"]) + flows := spec["components"].(map[string]any)["securitySchemes"].(map[string]any)["oauth"].(map[string]any)["flows"].(map[string]any) + require.Equal(t, map[string]any{"oauth": "User consent", "shared": "User grant"}, flows["authorizationCode"].(map[string]any)["scopes"]) + require.Equal(t, map[string]any{"machine": "Machine grant", "shared": "Machine permission"}, flows["clientCredentials"].(map[string]any)["scopes"]) + require.Empty(t, flows["implicit"].(map[string]any)["scopes"]) + if os.Getenv("LOOM_OPENAPI_CONTRACT") != "" { + path := filepath.Join(t.TempDir(), "openapi.yaml") + require.NoError(t, os.WriteFile(path, artifacts.YAML, 0o600)) + require.NoError(t, vacuum.lint(path)) + } + }) + } +} diff --git a/http/codegen/openapi/v3/testdata/golden/security_file0.golden b/http/codegen/openapi/v3/testdata/golden/security_file0.golden index edd7df5ed..6b03028ec 100644 --- a/http/codegen/openapi/v3/testdata/golden/security_file0.golden +++ b/http/codegen/openapi/v3/testdata/golden/security_file0.golden @@ -71,7 +71,9 @@ { "api_key_query_k": [], "basic": [], - "jwt_header_X-Authorization": [] + "jwt_header_X-Authorization": [ + "api:read" + ] } ], "summary": "testEndpointA testService", diff --git a/http/codegen/openapi/v3/testdata/golden/security_file1.golden b/http/codegen/openapi/v3/testdata/golden/security_file1.golden index e1a38bc8e..b137f7fc5 100644 --- a/http/codegen/openapi/v3/testdata/golden/security_file1.golden +++ b/http/codegen/openapi/v3/testdata/golden/security_file1.golden @@ -20,7 +20,8 @@ paths: security: - api_key_query_k: [] basic: [] - jwt_header_X-Authorization: [] + jwt_header_X-Authorization: + - api:read post: tags: - testService diff --git a/http/codegen/openapi_operation_security_test.go b/http/codegen/openapi_operation_security_test.go index c296802e0..8d8dc235a 100644 --- a/http/codegen/openapi_operation_security_test.go +++ b/http/codegen/openapi_operation_security_test.go @@ -48,7 +48,7 @@ func TestOpenAPIOperationSecurityRequirements(t *testing.T) { require.Empty(t, security.([]any)) }) - t.Run("HTTP bearer requirements do not publish scopes", func(t *testing.T) { + t.Run("HTTP bearer requirements publish application roles", func(t *testing.T) { root := RunHTTPDSL(t, scopedBearerOperationDSL) spec := renderOpenAPIJSON(t, openapiv3.Files, root) @@ -58,7 +58,7 @@ func TestOpenAPIOperationSecurityRequirements(t *testing.T) { security, ok := op["security"].([]any) require.True(t, ok) require.Len(t, security, 1) - require.Equal(t, map[string]any{"jwt": []any{}}, security[0]) + require.Equal(t, map[string]any{"jwt": []any{"api:read"}}, security[0]) }) t.Run("service session security applies to secured meal planner operations only", func(t *testing.T) { diff --git a/http/codegen/security_scope_test.go b/http/codegen/security_scope_test.go index 86d0908f9..b4758ca05 100644 --- a/http/codegen/security_scope_test.go +++ b/http/codegen/security_scope_test.go @@ -456,3 +456,14 @@ func assertUnauthorizedProblem(t *testing.T, rec *httptest.ResponseRecorder) { } } ` + +func TestHTTPPerSchemeSecurityProjection(t *testing.T) { + root := RunHTTPDSL(t, testdata.SecurityPerSchemeDSL("http")) + endpoint := CreateHTTPServices(root).Get("per_scheme").Endpoint("read") + requirements := endpoint.Method.Requirements + require.Len(t, requirements, 3) + require.Equal(t, []string{"read"}, requirements[0].Schemes[0].RequiredScopes) + require.Equal(t, []string{"write"}, requirements[0].Schemes[1].RequiredScopes) + require.Equal(t, []string{"read"}, requirements[1].Schemes[0].RequiredScopes) + require.Empty(t, requirements[2].Schemes) +} diff --git a/internal/openapiimport/model.go b/internal/openapiimport/model.go index eca65434f..c727c589e 100644 --- a/internal/openapiimport/model.go +++ b/internal/openapiimport/model.go @@ -92,8 +92,6 @@ type SecurityScheme struct { OAuth2MetadataURL string // OAuthFlows contains supported OAuth2 flows in specification order. OAuthFlows []OAuthFlow - // Scopes contains the shared OAuth2 scope definitions. - Scopes []SecurityScope // Extensions contains scheme-level vendor extensions. Extensions map[string]any } @@ -108,6 +106,8 @@ type OAuthFlow struct { TokenURL string // RefreshURL is the optional flow refresh endpoint. RefreshURL string + // Scopes contains this flow's exact scope names and descriptions. + Scopes []SecurityScope } // SecurityScope is one OAuth2 scope definition. diff --git a/internal/openapiimport/security.go b/internal/openapiimport/security.go index 450aeb2a1..6ace17173 100644 --- a/internal/openapiimport/security.go +++ b/internal/openapiimport/security.go @@ -98,13 +98,12 @@ func (a *analyzer) securityScheme( return SecurityScheme{}, false } case "oauth2": - flows, scopes, ok := a.oauthFlows(source.Flows, path+"/flows") + flows, ok := a.oauthFlows(source.Flows, path+"/flows") if !ok || source.Scheme != "" || source.BearerFormat != "" || source.In != "" || source.Name != "" || source.OpenIdConnectUrl != "" { a.unsupported("security-scheme", path, "OAuth2 scheme contains fields outside the strict import subset") return SecurityScheme{}, false } scheme.OAuthFlows = flows - scheme.Scopes = scopes default: a.unsupported("security-scheme", path, fmt.Sprintf("security scheme type %q is not in the strict import subset", source.Type)) return SecurityScheme{}, false @@ -149,18 +148,18 @@ func (a *analyzer) supportedHTTPScheme(source *v3.SecurityScheme, path string) b return true } -func (a *analyzer) oauthFlows(source *v3.OAuthFlows, path string) ([]OAuthFlow, []SecurityScope, bool) { +func (a *analyzer) oauthFlows(source *v3.OAuthFlows, path string) ([]OAuthFlow, bool) { if source == nil { a.unsupported("security-scheme", path, "OAuth2 security scheme must define flows") - return nil, nil, false + return nil, false } if source.Device != nil || oauthDeviceAuthorizationPresent(source) { a.unsupported("security-scheme", path+"/deviceAuthorization", "OAuth device authorization import is blocked until the parser exposes the official field") - return nil, nil, false + return nil, false } if orderedmap.Len(source.Extensions) > 0 { a.unsupported("security-scheme", path, "OAuth flow-container extensions are not in the strict import subset") - return nil, nil, false + return nil, false } types := []struct { kind string @@ -172,7 +171,6 @@ func (a *analyzer) oauthFlows(source *v3.OAuthFlows, path string) ([]OAuthFlow, {kind: "clientCredentials", flow: source.ClientCredentials}, } var result []OAuthFlow - var sharedScopes []SecurityScope for _, candidate := range types { if candidate.flow == nil { continue @@ -180,28 +178,23 @@ func (a *analyzer) oauthFlows(source *v3.OAuthFlows, path string) ([]OAuthFlow, flowPath := path + "/" + candidate.kind if orderedmap.Len(candidate.flow.Extensions) > 0 { a.unsupported("security-scheme", flowPath, "OAuth flow extensions are not in the strict import subset") - return nil, nil, false + return nil, false } if !a.validOAuthFlow(candidate.kind, candidate.flow, flowPath) { - return nil, nil, false - } - scopes := oauthScopes(candidate.flow.Scopes) - if sharedScopes == nil { - sharedScopes = scopes - } else if !equalSecurityScopes(sharedScopes, scopes) { - a.unsupported("security-scheme", flowPath+"/scopes", "all OAuth flows must define the same scopes for Loom to preserve them") - return nil, nil, false + return nil, false } + result = append(result, OAuthFlow{ Kind: candidate.kind, AuthorizationURL: candidate.flow.AuthorizationUrl, TokenURL: candidate.flow.TokenUrl, RefreshURL: candidate.flow.RefreshUrl, + Scopes: oauthScopes(candidate.flow.Scopes), }) } if len(result) == 0 { a.unsupported("security-scheme", path, "OAuth2 security scheme must define at least one supported flow") - return nil, nil, false + return nil, false } - return result, sharedScopes, true + return result, true } func oauthDeviceAuthorizationPresent(flows *v3.OAuthFlows) bool { @@ -249,18 +242,6 @@ func (a *analyzer) validOAuthFlow(kind string, flow *v3.OAuthFlow, path string) return true } -func equalSecurityScopes(left, right []SecurityScope) bool { - if len(left) != len(right) { - return false - } - for index := range left { - if left[index] != right[index] { - return false - } - } - return true -} - func (a *analyzer) securityRequirements(source []*base.SecurityRequirement, path string, components Components) SecurityRequirements { requirements := make(SecurityRequirements, 0, len(source)) for index, sourceRequirement := range source { @@ -277,16 +258,7 @@ func (a *analyzer) securityRequirements(source []*base.SecurityRequirement, path a.unsupported("security-requirement", schemePath, fmt.Sprintf("security scheme %q is not a supported component", name)) continue } - if scheme.Type != "oauth2" && len(scopes) > 0 { - a.unsupported("security-requirement", schemePath, "non-OAuth security requirement scopes must be empty") - } - if scheme.Type == "oauth2" { - for _, scope := range scopes { - if !securityScopeDefined(scheme.Scopes, scope) { - a.unsupported("security-requirement", schemePath, fmt.Sprintf("OAuth scope %q is not defined by scheme %q", scope, name)) - } - } - } + normalizedScopes := append([]string(nil), scopes...) sort.Strings(normalizedScopes) requirement.Schemes = append(requirement.Schemes, SecurityRequirementScheme{ @@ -299,26 +271,12 @@ func (a *analyzer) securityRequirements(source []*base.SecurityRequirement, path return requirements } -func equalStrings(left, right []string) bool { - if len(left) != len(right) { - return false - } - want := append([]string(nil), left...) - got := append([]string(nil), right...) - sort.Strings(want) - sort.Strings(got) - for index := range want { - if want[index] != got[index] { - return false - } - } - return true -} - -func securityScopeDefined(scopes []SecurityScope, name string) bool { - for _, scope := range scopes { - if scope.Name == name { - return true +func securityScopeDefined(scheme SecurityScheme, name string) bool { + for _, flow := range scheme.OAuthFlows { + for _, scope := range flow.Scopes { + if scope.Name == name { + return true + } } } return false @@ -363,19 +321,20 @@ func (r *renderer) securityScheme(scheme SecurityScheme) error { for _, flow := range scheme.OAuthFlows { switch flow.Kind { case "authorizationCode": - r.line("AuthorizationCodeFlow(%q, %q, %q)", flow.AuthorizationURL, flow.TokenURL, flow.RefreshURL) + r.open("AuthorizationCodeFlow(%q, %q, %q, func()", flow.AuthorizationURL, flow.TokenURL, flow.RefreshURL) case "implicit": - r.line("ImplicitFlow(%q, %q)", flow.AuthorizationURL, flow.RefreshURL) + r.open("ImplicitFlow(%q, %q, func()", flow.AuthorizationURL, flow.RefreshURL) case "password": - r.line("PasswordFlow(%q, %q)", flow.TokenURL, flow.RefreshURL) + r.open("PasswordFlow(%q, %q, func()", flow.TokenURL, flow.RefreshURL) case "clientCredentials": - r.line("ClientCredentialsFlow(%q, %q)", flow.TokenURL, flow.RefreshURL) + r.open("ClientCredentialsFlow(%q, %q, func()", flow.TokenURL, flow.RefreshURL) default: return fmt.Errorf("render OpenAPI design: security scheme %q has unsupported OAuth flow %q", scheme.Name, flow.Kind) } - } - for _, scope := range scheme.Scopes { - r.line("Scope(%q, %q)", scope.Name, scope.Description) + for _, scope := range flow.Scopes { + r.line("Scope(%q, %q)", scope.Name, scope.Description) + } + r.close() } if err := r.emitExtensions("", scheme.Extensions); err != nil { return err @@ -392,8 +351,7 @@ func (r *renderer) securityRequirements(requirements SecurityRequirements) error continue } arguments := make([]string, 0, len(requirement.Schemes)) - scopes, err := securityRequirementScopes(requirement) - if err != nil { + if err := validateSecurityRequirement(requirement, r.document.Components, r.document.OpenAPIVersion); err != nil { return fmt.Errorf("render OpenAPI design: %w", err) } for _, reference := range requirement.Schemes { @@ -403,37 +361,61 @@ func (r *renderer) securityRequirements(requirements SecurityRequirements) error } arguments = append(arguments, scheme.GoName) } - if len(scopes) == 0 { + hasNames := false + for _, reference := range requirement.Schemes { + hasNames = hasNames || len(reference.Scopes) > 0 + } + if !hasNames { r.line("Security(%s)", strings.Join(arguments, ", ")) continue } r.open("Security(%s, func()", strings.Join(arguments, ", ")) - for _, scope := range scopes { - r.line("Scope(%q)", scope) + for _, reference := range requirement.Schemes { + scheme := securitySchemeByName(r.document.Components.SecuritySchemes, reference.Name) + call := "RoleFor" + if scheme.Type == "oauth2" { + call = "ScopeFor" + } + for _, name := range reference.Scopes { + r.line("%s(%s, %q)", call, scheme.GoName, name) + } } r.close() } return nil } -// securityRequirementScopes admits only a source conjunction that one Loom -// requirement scope list can express without changing any scheme's checks. -func securityRequirementScopes(requirement SecurityRequirement) ([]string, error) { - if len(requirement.Schemes) == 0 { - return nil, nil - } - first := requirement.Schemes[0] - for _, reference := range requirement.Schemes[1:] { - if !equalStrings(first.Scopes, reference.Scopes) { - return nil, fmt.Errorf("security schemes %q and %q in one requirement use different scopes; Loom requires the same scopes for every scheme in a conjunction", first.Name, reference.Name) +// validateSecurityRequirement also checks normalized documents supplied directly +// to Render. Role names have no protocol-defined inventory. +func validateSecurityRequirement(requirement SecurityRequirement, components Components, version string) error { + seen := make(map[string]bool) + for _, reference := range requirement.Schemes { + scheme := securitySchemeByName(components.SecuritySchemes, reference.Name) + if scheme == nil { + return fmt.Errorf("security scheme %q is not defined", reference.Name) + } + if seen[reference.Name] { + return fmt.Errorf("security scheme %q occurs twice in one requirement", reference.Name) + } + seen[reference.Name] = true + if scheme.Type != "oauth2" { + if strings.HasPrefix(version, "3.0.") && len(reference.Scopes) > 0 { + return fmt.Errorf("non-OAuth security requirement arrays must be empty in OpenAPI 3.0") + } + continue + } + for _, scope := range reference.Scopes { + if !securityScopeDefined(*scheme, scope) { + return fmt.Errorf("OAuth scope %q is not defined by scheme %q", scope, reference.Name) + } } } - return first.Scopes, nil + return nil } func (p *documentPlanner) securityRequirements(requirements SecurityRequirements, path string) { for index, requirement := range requirements { - if _, err := securityRequirementScopes(requirement); err != nil { + if err := validateSecurityRequirement(requirement, p.document.Components, p.document.OpenAPIVersion); err != nil { p.unsupported("security-requirement", fmt.Sprintf("%s/%d", path, index), err.Error()) } } diff --git a/internal/openapiimport/security_credentials.go b/internal/openapiimport/security_credentials.go index b6fea7441..828cd2bb4 100644 --- a/internal/openapiimport/security_credentials.go +++ b/internal/openapiimport/security_credentials.go @@ -46,7 +46,7 @@ func (r *renderer) securityParameters(operation *Operation, parameters []rendere seen := make(map[string]bool) slots := make(map[credentialSlot]int) for requirementIndex, requirement := range requirements { - if _, err := securityRequirementScopes(requirement); err != nil { + if err := validateSecurityRequirement(requirement, r.document.Components, r.document.OpenAPIVersion); err != nil { return nil, fmt.Errorf("render OpenAPI design: %s/%d: %w", path, requirementIndex, err) } for schemeIndex, reference := range requirement.Schemes { diff --git a/internal/openapiimport/security_credentials_import_test.go b/internal/openapiimport/security_credentials_import_test.go index 6afb3d1c7..716469726 100644 --- a/internal/openapiimport/security_credentials_import_test.go +++ b/internal/openapiimport/security_credentials_import_test.go @@ -69,7 +69,7 @@ func TestImportedSecurityCredentialsGenerated(t *testing.T) { } } -func TestSecurityCredentialScopePartialImport(t *testing.T) { +func TestSecurityCredentialUnknownScopePartialImport(t *testing.T) { source, err := os.ReadFile("testdata/security_credentials/imported.json") require.NoError(t, err) document, diagnostics, err := Analyze(source) @@ -78,16 +78,16 @@ func TestSecurityCredentialScopePartialImport(t *testing.T) { // Render also rejects a normalized source rather than relying on the parser. document.Operations[0].SecurityDefined = true document.Operations[0].Security = SecurityRequirements{{Schemes: []SecurityRequirementScheme{ - {Name: "Bearer"}, {Name: "OAuth", Scopes: []string{"read"}}, + {Name: "Bearer"}, {Name: "OAuth", Scopes: []string{"unknown"}}, }}} _, err = Render(document, Options{}) - require.ErrorContains(t, err, "use different scopes") + require.ErrorContains(t, err, "not defined by scheme") var input map[string]any require.NoError(t, json.Unmarshal(source, &input)) input["paths"] = map[string]any{ "/bad": map[string]any{"get": map[string]any{ - "security": []any{map[string]any{"Bearer": []any{}, "OAuth": []any{"read"}}}, + "security": []any{map[string]any{"Bearer": []any{}, "OAuth": []any{"unknown"}}}, "responses": map[string]any{"204": map[string]any{"description": "Done"}}, }}, "/good": map[string]any{"get": map[string]any{ @@ -101,7 +101,7 @@ func TestSecurityCredentialScopePartialImport(t *testing.T) { require.NoError(t, err) require.Len(t, partial.Skipped, 1) require.Equal(t, "/bad", partial.Skipped[0].Path) - require.Equal(t, "#/paths/~1bad/get/security/0", partial.Skipped[0].Diagnostics[0].Path) + require.Contains(t, partial.Skipped[0].Diagnostics[0].Path, "#/paths/~1bad/get/security/0") require.Equal(t, "security-requirement", partial.Skipped[0].Diagnostics[0].Code) require.Len(t, partial.Document.Operations, 1) require.Equal(t, "/good", partial.Document.Operations[0].Path) diff --git a/internal/openapiimport/security_credentials_partial_test.go b/internal/openapiimport/security_credentials_partial_test.go index a0bfa29e7..4d512b352 100644 --- a/internal/openapiimport/security_credentials_partial_test.go +++ b/internal/openapiimport/security_credentials_partial_test.go @@ -22,7 +22,7 @@ func TestSecurityCredentialRootScopePartialImport(t *testing.T) { fatal, _ := diagnostics.Classify(lossy) require.NotEmpty(t, fatal, "strict import must reject unsupported root requirements") _, err = Render(document, Options{}) - require.ErrorContains(t, err, "use different scopes") + require.ErrorContains(t, err, "not defined by scheme") partial, _, err := AnalyzePartial(source, Selection{}, lossy) if err != nil { @@ -31,8 +31,8 @@ func TestSecurityCredentialRootScopePartialImport(t *testing.T) { } require.Len(t, partial.Skipped, 1) require.Equal(t, "/inherit", partial.Skipped[0].Path) - require.Len(t, partial.Skipped[0].Diagnostics, 1) - require.Equal(t, "#/security/0", partial.Skipped[0].Diagnostics[0].Path) + require.NotEmpty(t, partial.Skipped[0].Diagnostics) + require.Contains(t, partial.Skipped[0].Diagnostics[0].Path, "#/security/0") require.Equal(t, "security-requirement", partial.Skipped[0].Diagnostics[0].Code) require.Len(t, partial.Document.Operations, len(paths)) require.False(t, partial.Document.SecurityDefined) @@ -44,9 +44,8 @@ func TestSecurityCredentialRootScopePartialImport(t *testing.T) { } else { require.Len(t, operation.Security, 1) require.Len(t, operation.Security[0].Schemes, 2) - for _, scheme := range operation.Security[0].Schemes { - require.Equal(t, []string{"read"}, scheme.Scopes) - } + require.Equal(t, []string{"read"}, operation.Security[0].Schemes[0].Scopes) + require.Empty(t, operation.Security[0].Schemes[1].Scopes) } } _, err = Render(partial.Document, Options{PackageName: "design"}) @@ -96,7 +95,7 @@ func partialCredentialSource(t *testing.T, retained []string) []byte { require.NoError(t, err) var input map[string]any require.NoError(t, json.Unmarshal(source, &input)) - input["security"] = []any{map[string]any{"Bearer": []any{}, "OAuth": []any{"read"}}} + input["security"] = []any{map[string]any{"Bearer": []any{}, "OAuth": []any{"unknown"}}} paths := input["paths"].(map[string]any) selected := map[string]any{"/inherit": paths["/inherit"]} for _, path := range retained { diff --git a/internal/openapiimport/security_credentials_readiness_test.go b/internal/openapiimport/security_credentials_readiness_test.go index 5dc38bea4..f76651e13 100644 --- a/internal/openapiimport/security_credentials_readiness_test.go +++ b/internal/openapiimport/security_credentials_readiness_test.go @@ -127,13 +127,14 @@ func TestSecurityCredentialReadinessScopeAdmission(t *testing.T) { requirement string unsupported bool }{ - {"{Bearer: [], OAuth: [read]}", true}, - {"{OAuth: [read], Bearer: []}", true}, - {"{OAuth: [], Other: [read]}", true}, - {"{OAuth: [read], Other: []}", true}, + {"{Bearer: [], OAuth: [read]}", false}, + {"{OAuth: [read], Bearer: []}", false}, + {"{OAuth: [], Other: [read]}", false}, + {"{OAuth: [read], Other: []}", false}, {"{OAuth: [read], Other: [read]}", false}, {"{Bearer: [], OAuth: []}", false}, {"{Bearer: []}, {OAuth: [read]}", false}, + {"{Bearer: [admin], OAuth: [read]}", false}, } { t.Run(tc.requirement, func(t *testing.T) { source := []byte(`openapi: 3.2.0 @@ -183,7 +184,7 @@ func TestSecurityCredentialRoles(t *testing.T) { }, Components: Components{SecuritySchemes: []SecurityScheme{ {Name: "JWT", GoName: "JWT", Type: "http", Scheme: "bearer"}, - {Name: "OAuth", GoName: "OAuth", Type: "oauth2"}, + {Name: "OAuth", GoName: "OAuth", Type: "oauth2", OAuthFlows: []OAuthFlow{{Kind: "clientCredentials", TokenURL: "https://example.com/token", Scopes: []SecurityScope{{Name: "read"}}}}}, }}, } before, err := json.Marshal(document, json.Deterministic(true)) diff --git a/internal/openapiimport/security_flow_scopes_test.go b/internal/openapiimport/security_flow_scopes_test.go new file mode 100644 index 000000000..07d1ecad1 --- /dev/null +++ b/internal/openapiimport/security_flow_scopes_test.go @@ -0,0 +1,48 @@ +package openapiimport + +import ( + "testing" + + "github.com/stretchr/testify/require" +) + +func TestIndependentOAuthFlowScopes(t *testing.T) { + source := []byte(`openapi: 3.2.0 +info: {title: Flow inventories, version: "1"} +security: [{Auth: [read, write]}] +paths: + /value: + get: + responses: {"204": {description: done}} +components: + securitySchemes: + Auth: + type: oauth2 + flows: + authorizationCode: + authorizationUrl: https://example.com/authorize + tokenUrl: https://example.com/token + scopes: {read: Read with consent, shared: User grant} + clientCredentials: + tokenUrl: https://example.com/token + scopes: {write: Machine write, shared: Machine grant} + implicit: + authorizationUrl: https://example.com/authorize + scopes: {} +`) + document, diagnostics, err := Analyze(source) + require.NoError(t, err) + require.Empty(t, diagnostics) + rendered, err := Render(document, Options{PackageName: "design"}) + require.NoError(t, err) + require.Contains(t, string(rendered), `Scope("shared", "User grant")`) + require.Contains(t, string(rendered), `Scope("shared", "Machine grant")`) + contract := readGeneratedOpenAPIContract(t, requireRenderedDesignGenerates(t, rendered)) + want := map[string]any{ + "authorizationCode": map[string]any{"authorizationUrl": "https://example.com/authorize", "tokenUrl": "https://example.com/token", "scopes": map[string]any{"read": "Read with consent", "shared": "User grant"}}, + "clientCredentials": map[string]any{"tokenUrl": "https://example.com/token", "scopes": map[string]any{"write": "Machine write", "shared": "Machine grant"}}, + "implicit": map[string]any{"authorizationUrl": "https://example.com/authorize", "scopes": map[string]any{}}, + } + got := contract["components"].(map[string]any)["securitySchemes"].(map[string]any)["Auth"].(map[string]any)["flows"] + require.Equal(t, want, got) +} diff --git a/internal/openapiimport/security_per_scheme_test.go b/internal/openapiimport/security_per_scheme_test.go new file mode 100644 index 000000000..5e2c78068 --- /dev/null +++ b/internal/openapiimport/security_per_scheme_test.go @@ -0,0 +1,97 @@ +package openapiimport + +import ( + "fmt" + "strings" + "testing" + + "github.com/stretchr/testify/require" +) + +func TestSecurityPerSchemeRequirements(t *testing.T) { + source := []byte(`openapi: 3.2.0 +info: {title: Per-scheme requirements, version: "1"} +security: [{Read: [read], Write: [], Key: [admin]}, {}] +paths: + /secure: + get: + responses: {"204": {description: done}} +components: + securitySchemes: + Read: + type: oauth2 + flows: + clientCredentials: {tokenUrl: https://example.com/token, scopes: {read: Read, write: Write}} + Write: + type: oauth2 + flows: + clientCredentials: {tokenUrl: https://example.com/token, scopes: {read: Read, write: Write}} + Key: {type: apiKey, in: header, name: X-Key} +`) + document, diagnostics, err := Analyze(source) + require.NoError(t, err) + require.Empty(t, diagnostics) + rendered, err := Render(document, Options{PackageName: "design"}) + require.NoError(t, err) + require.Contains(t, string(rendered), `ScopeFor(ImportedReadSecurity, "read")`) + require.Contains(t, string(rendered), `RoleFor(ImportedKeySecurity, "admin")`) + require.NotContains(t, string(rendered), `ScopeFor(ImportedWriteSecurity, "read")`) + for _, tc := range []struct { + name string + refs []SecurityRequirementScheme + want string + }{ + {"unknown scheme", []SecurityRequirementScheme{{Name: "missing"}}, "not defined"}, + {"unknown scope", []SecurityRequirementScheme{{Name: "Read", Scopes: []string{"missing"}}}, "not defined by scheme"}, + {"duplicate scheme", []SecurityRequirementScheme{{Name: "Read"}, {Name: "Read"}}, "occurs twice"}, + } { + t.Run(tc.name, func(t *testing.T) { + copy := *document + copy.Security = SecurityRequirements{{Schemes: tc.refs}} + _, err := Render(©, Options{}) + require.ErrorContains(t, err, tc.want) + }) + } +} + +func TestSecurityRoleVersionBoundary(t *testing.T) { + for _, version := range []string{"3.0.4", "3.1.1", "3.2.0"} { + for _, roles := range []bool{false, true} { + t.Run(fmt.Sprintf("%s/roles=%t", version, roles), func(t *testing.T) { + keyNames := "[]" + if roles { + keyNames = "[admin]" + } + source := []byte(fmt.Sprintf(`openapi: %s +info: {title: Versioned requirements, version: "1"} +security: [{Read: [read], Write: [], Key: %s}] +paths: + /secure: + get: + responses: {"204": {description: done}} +components: + securitySchemes: + Read: + type: oauth2 + flows: + clientCredentials: {tokenUrl: https://example.com/token, scopes: {read: Read}} + Write: + type: oauth2 + flows: + clientCredentials: {tokenUrl: https://example.com/token, scopes: {read: Read}} + Key: {type: apiKey, in: header, name: X-Key} +`, version, keyNames)) + document, diagnostics, err := Analyze(source) + require.NoError(t, err) + _, renderErr := Render(document, Options{}) + if roles && strings.HasPrefix(version, "3.0.") { + require.Contains(t, diagnostics.Error(), "must be empty in OpenAPI 3.0") + require.ErrorContains(t, renderErr, "must be empty in OpenAPI 3.0") + } else { + require.Empty(t, diagnostics) + require.NoError(t, renderErr) + } + }) + } + } +} diff --git a/internal/openapiimport/symmetry_test.go b/internal/openapiimport/symmetry_test.go index b5bbafad9..72e7bcce0 100644 --- a/internal/openapiimport/symmetry_test.go +++ b/internal/openapiimport/symmetry_test.go @@ -167,11 +167,11 @@ components: `Meta("openapi:deprecated", "true")`, `var ImportedOAuthSecurity = OAuth2Security("OAuth", func() {`, `Meta("openapi:oauth2MetadataUrl", "https://auth.example.com/.well-known/oauth-authorization-server")`, - `AuthorizationCodeFlow("https://auth.example.com/authorize", "https://auth.example.com/token", "https://auth.example.com/refresh")`, - `ClientCredentialsFlow("https://auth.example.com/token", "")`, + `AuthorizationCodeFlow("https://auth.example.com/authorize", "https://auth.example.com/token", "https://auth.example.com/refresh", func() {`, + `ClientCredentialsFlow("https://auth.example.com/token", "", func() {`, `Scope("read", "Read access.")`, `Security(ImportedOAuthSecurity, func() {`, - `Scope("read")`, + `ScopeFor(ImportedOAuthSecurity, "read")`, `Username("basicUsername", String)`, `Password("basicPassword", String)`, `Token("bearerToken", String, func() {`, diff --git a/internal/openapiimport/testdata/security_credentials/imported.json b/internal/openapiimport/testdata/security_credentials/imported.json index 94fef064a..3cb2c01ba 100644 --- a/internal/openapiimport/testdata/security_credentials/imported.json +++ b/internal/openapiimport/testdata/security_credentials/imported.json @@ -43,8 +43,12 @@ }, "security": [ { - "Bearer": [], - "OAuth": [] + "Bearer": [ + "admin" + ], + "OAuth": [ + "read" + ] } ] } @@ -110,9 +114,7 @@ "OAuth": [ "read" ], - "OAuthSecond": [ - "read" - ] + "OAuthSecond": [] } ] } diff --git a/internal/openapiimport/testdata/security_credentials/imported_wire.go.txt b/internal/openapiimport/testdata/security_credentials/imported_wire.go.txt index 361eeb2c7..c45e19a6a 100644 --- a/internal/openapiimport/testdata/security_credentials/imported_wire.go.txt +++ b/internal/openapiimport/testdata/security_credentials/imported_wire.go.txt @@ -21,6 +21,7 @@ type implementation struct { auth []string failJWT bool denyOAuth bool + denyRole bool } func (s *implementation) Either(context.Context, *svc.EitherPayload) error { @@ -74,14 +75,17 @@ func (s *implementation) JWTAuth(ctx context.Context, token string, scheme *secu if s.failJWT || token != "token" { return ctx, fmt.Errorf("JWT denied") } - return ctx, nil + if s.denyRole { + return ctx, scheme.Validate(nil) + } + return ctx, scheme.Validate([]string{"admin"}) } func (s *implementation) OAuth2Auth(ctx context.Context, token string, scheme *security.OAuth2Scheme) (context.Context, error) { s.auth = append(s.auth, fmt.Sprintf("%s:%s:%v", scheme.Name, token, scheme.RequiredScopes)) if s.denyOAuth || token != "token" { return ctx, fmt.Errorf("OAuth denied") } - return ctx, nil + return ctx, scheme.Validate([]string{"read"}) } func (s *implementation) BasicAuth(ctx context.Context, user, pass string, scheme *security.BasicScheme) (context.Context, error) { s.auth = append(s.auth, fmt.Sprintf("%s:%s:%s", scheme.Name, user, pass)) @@ -104,11 +108,11 @@ func TestImportedCredentialAuthWire(t *testing.T) { {"basic", "either", "Basic dXNlcjpwYXNz", "", false, false, 1, []string{"Basic:user:pass"}}, {"JWT", "either", "Bearer token", "", false, false, 1, []string{"Basic::", "Bearer:token:[]"}}, {"OAuth fallback scopes", "either", "Bearer token", "", true, false, 1, []string{"Basic::", "Bearer:token:[]", "OAuth:token:[read]"}}, - {"AND both", "both", "Bearer token", "", false, false, 1, []string{"Bearer:token:[]", "OAuth:token:[]"}}, - {"AND denied", "both", "Bearer token", "", false, true, 0, []string{"Bearer:token:[]", "OAuth:token:[]"}}, + {"AND both", "both", "Bearer token", "", false, false, 1, []string{"Bearer:token:[admin]", "OAuth:token:[read]"}}, + {"AND denied", "both", "Bearer token", "", false, true, 0, []string{"Bearer:token:[admin]", "OAuth:token:[read]"}}, {"shared Basic roles", "basic_roles", "Basic dXNlcjpwYXNz", "", false, false, 1, []string{"Basic:user:pass", "BasicSecond:user:pass"}}, {"shared bearer roles", "bearer_roles", "Bearer token", "", false, false, 1, []string{"Bearer:token:[]", "BearerSecond:token:[]"}}, - {"OAuth scopes", "oauth_roles", "Bearer token", "", false, false, 1, []string{"OAuth:token:[read]", "OAuthSecond:token:[read]"}}, + {"OAuth scopes", "oauth_roles", "Bearer token", "", false, false, 1, []string{"OAuth:token:[read]", "OAuthSecond:token:[]"}}, {"inherited", "inherit", "Bearer token", "", false, false, 1, []string{"Bearer:token:[]"}}, {"public override", "public", "", "", false, false, 1, nil}, {"anonymous alternative", "optional", "", "", true, false, 1, []string{"Bearer::[]"}}, @@ -182,7 +186,7 @@ func TestImportedCredentialClientWire(t *testing.T) { }{ {"either", &svc.EitherPayload{BasicUsername: &user, BasicPassword: &pass}, client.EncodeEitherRequest(nil), []string{"Basic:user:pass"}}, {"either", &svc.EitherPayload{BearerToken: &token}, client.EncodeEitherRequest(nil), []string{"Basic::", "Bearer:token:[]"}}, - {"both", &svc.BothPayload{BearerToken: &token}, client.EncodeBothRequest(nil), []string{"Bearer:token:[]", "OAuth:token:[]"}}, + {"both", &svc.BothPayload{BearerToken: &token}, client.EncodeBothRequest(nil), []string{"Bearer:token:[admin]", "OAuth:token:[read]"}}, {"keys", &svc.KeysPayload{OneCredential: &key}, client.EncodeKeysRequest(nil), []string{"One:key", "Two:key"}}, } { t.Run(tc.path+tc.auth[0], func(t *testing.T) { @@ -204,3 +208,19 @@ func TestImportedCredentialClientWire(t *testing.T) { }) } } + +func TestImportedMissingRole(t *testing.T) { + s := &implementation{denyRole: true} + mux := loomhttp.NewMuxer() + server.Mount(mux, server.New(svc.NewEndpoints(s), mux, loomhttp.RequestDecoder, loomhttp.ResponseEncoder, nil, nil)) + req := httptest.NewRequest(http.MethodGet, "/both", nil) + req.Header.Set("Authorization", "Bearer token") + rec := httptest.NewRecorder() + mux.ServeHTTP(rec, req) + if s.calls != 0 || rec.Code < 400 { + t.Errorf("missing role allowed invocation: calls=%d status=%d", s.calls, rec.Code) + } + if !reflect.DeepEqual(s.auth, []string{"Bearer:token:[admin]"}) { + t.Errorf("unexpected callbacks after role denial: %v", s.auth) + } +} diff --git a/internal/openapiimport/testdata/security_credentials/partial_wire.go.txt b/internal/openapiimport/testdata/security_credentials/partial_wire.go.txt index b97b0abba..a05ab2ccc 100644 --- a/internal/openapiimport/testdata/security_credentials/partial_wire.go.txt +++ b/internal/openapiimport/testdata/security_credentials/partial_wire.go.txt @@ -48,9 +48,9 @@ func TestPartialCredentialAuthWire(t *testing.T) { }{ {name: "public", path: "/public", calls: 1, status: 204}, {name: "scoped conjunction", path: "/oauth_roles", token: "token", calls: 1, status: 204, - auth: []string{"OAuth:token:[read]", "OAuthSecond:token:[read]"}}, + auth: []string{"OAuth:token:[read]", "OAuthSecond:token:[]"}}, {name: "second role denies", path: "/oauth_roles", token: "token", denySecond: true, - auth: []string{"OAuth:token:[read]", "OAuthSecond:token:[read]"}}, + auth: []string{"OAuth:token:[read]", "OAuthSecond:token:[]"}}, {name: "missing credential", path: "/oauth_roles", auth: []string{"OAuth::[read]"}}, {name: "inherited unsupported omitted", path: "/inherit", status: 404}, } { diff --git a/internal/openapiimport/tla/security_credentials/README.md b/internal/openapiimport/tla/security_credentials/README.md index 186d6b82c..f1fa7b58c 100644 --- a/internal/openapiimport/tla/security_credentials/README.md +++ b/internal/openapiimport/tla/security_credentials/README.md @@ -62,12 +62,12 @@ scoped secured overrides, blocked inheritance, strict rejection, the empty subset and actual generated authentication. Valid inherited root requirements remain covered by the existing partial-import tests. -Scopes are intentionally outside this physical grouping model. The existing -Loom requirement has one scope list applied to every scheme; OpenAPI carries one -list per scheme. A generated counterexample records that disagreement and direct -import tests demand explicit unsupported diagnostics for differing per-scheme -lists. Do not infer scope unions, scope intersections or a public policy from -this model. Header parsing, presence/default semantics, generated identifiers, +Per-scheme names are outside this physical grouping model. The shared model in +`expr/tla/security_requirements` now checks that independent scope/role lists +preserve AND, OR and anonymous alternatives. Import retains those arrays; +`security_credentials_import_test.go` checks generated role denial and unequal +OAuth requirements. Invalid OAuth scopes still reject, including during partial +import. Header parsing, presence/default semantics, generated identifiers, custom mappings, arbitrary graph size, repeated query values and malformed credentials remain concrete-test obligations. Raw Authorization conflicts and Basic/Bearer conjunctions remain under the existing expr admission model. diff --git a/jsonrpc/codegen/security_scope_test.go b/jsonrpc/codegen/security_scope_test.go index 4e8c19257..91b733e4a 100644 --- a/jsonrpc/codegen/security_scope_test.go +++ b/jsonrpc/codegen/security_scope_test.go @@ -32,3 +32,14 @@ func TestJSONRPCSecurityScope(t *testing.T) { }) } } + +func TestJSONRPCPerSchemeSecurityProjection(t *testing.T) { + root := RunJSONRPCDSL(t, testdata.SecurityPerSchemeDSL("jsonrpc")) + endpoint := CreateJSONRPCServices(root).Get("per_scheme").Endpoint("read") + requirements := endpoint.Method.Requirements + require.Len(t, requirements, 3) + require.Equal(t, []string{"read"}, requirements[0].Schemes[0].RequiredScopes) + require.Equal(t, []string{"write"}, requirements[0].Schemes[1].RequiredScopes) + require.Equal(t, []string{"read"}, requirements[1].Schemes[0].RequiredScopes) + require.Empty(t, requirements[2].Schemes) +} diff --git a/security/scheme.go b/security/scheme.go index 39faad214..2c3823bf1 100644 --- a/security/scheme.go +++ b/security/scheme.go @@ -24,8 +24,9 @@ type ( Name string // Scopes holds a list of scopes for the scheme. Scopes []string - // RequiredScopes holds a list of scopes which are required - // by the scheme. It is a subset of Scopes field. + // RequiredScopes holds required application permission or role names. + // RoleFor names need not occur in Scopes. Validate checks all these + // names against the authenticated principal's application-owned grants. RequiredScopes []string } @@ -36,8 +37,9 @@ type ( Name string // Scopes holds a list of scopes for the scheme. Scopes []string - // RequiredScopes holds a list of scopes which are required - // by the scheme. It is a subset of Scopes field. + // RequiredScopes holds required application permission or role names. + // RoleFor names need not occur in Scopes. Validate checks all these + // names against the authenticated principal's application-owned grants. RequiredScopes []string } @@ -48,8 +50,9 @@ type ( Name string // Scopes holds a list of scopes for the scheme. Scopes []string - // RequiredScopes holds a list of scopes which are required - // by the scheme. It is a subset of Scopes field. + // RequiredScopes holds required application permission or role names. + // RoleFor names need not occur in Scopes. Validate checks all these + // names against the authenticated principal's application-owned grants. RequiredScopes []string } @@ -57,7 +60,8 @@ type ( OAuth2Scheme struct { // Name is the scheme name defined in the design. Name string - // Scopes holds a list of scopes for the scheme. + // Scopes lists distinct available names across the effective flow inventories. + // It describes availability, not grants held by an authenticated token. Scopes []string // RequiredScopes holds a list of scopes which are required // by the scheme. It is a subset of Scopes field. @@ -76,6 +80,10 @@ type ( TokenURL string // RefreshURL to be used for obtaining refresh token. RefreshURL string + // Scopes retains this flow's scope names and descriptions. Nil uses + // the enclosing scheme's shared Scopes names; a nonnil empty map + // declares an empty inventory. Inventories do not grant permissions. + Scopes map[string]string } // AuthBasicFunc is the function type that implements the basic auth