diff --git a/README.md b/README.md index a973f69..ca4fc5e 100644 --- a/README.md +++ b/README.md @@ -432,10 +432,34 @@ binding (`Transcoded`). The page names a procedure and a request message, never a path, a prefix, an audience or an authority, and authorization stays at the module. +Authority is minted per method. A method may declare its own `Scopes`, which +replace the module's for a call to that method — never add to them — so one +module's methods that need different authority are declared under one `as`: + +```go +solution.ConsumedModule{ + As: "tasks", + Methods: []solution.ConsumedMethod{ + {Name: ownListMethod, Scopes: []solution.Scope{{ResourceKind: "tasks", Actions: []string{"list"}}}, WholeResponse: true}, + {Name: orgListMethod, Scopes: []solution.Scope{{ResourceKind: "tasks", Actions: []string{"inspect"}}}, WholeResponse: true}, + }, +} +``` + +Each call mints only its own method's authority. A viewer who lacks one +method's authority is refused that method as `permission_denied`, with the +issuer's reason (which names the missing permission), and is still served +every other method; the mint is never widened or retried with less. Every +method of a module that is not `ViewerBearer` needs scopes, its own or the +module's, and `ViewerBearer` takes none at either level. + Least privilege by default. A method that is not declared is `not_found`. At boot, `Serve` refuses a module whose `as` is not in the solution's api.consumes, a method missing from the registry or with no binding under `/v1/`, a -streaming method, and a method with no declared response fields. `Response` +streaming method, a method with no declared response fields, a method with no +scopes of its own or its module's (unless the module is `ViewerBearer`), scopes +on a `ViewerBearer` module or method, and a scope with no resource kind, no +action, or a blank or repeated action. `Response` names the fields returned to the page; `WholeResponse` must be said explicitly. `Pin` fixes part of every request server-side (merged with `proto.Merge`): a pinned scalar replaces the page's value and a pinned repeated value is added to diff --git a/passthrough.go b/passthrough.go index b60f27a..0bb2889 100644 --- a/passthrough.go +++ b/passthrough.go @@ -58,9 +58,10 @@ type ConsumedModule struct { // for it is minted for. A module the solution does not consume under this // name is refused at boot. As string - // Scopes is the authority minted for the viewer on every call (ForModule), - // no more than the declared operations need. Exactly one of Scopes and - // ViewerBearer is set. + // Scopes is the authority minted for the viewer on every call (ForModule) + // of a method that declares none of its own, no more than those methods + // need. A module that is not ViewerBearer sets Scopes, or sets Scopes on + // every one of its methods. Scopes []Scope // ViewerBearer forwards only the viewer's bearer: for a module that // authenticates the viewer and mints its own authority from that bearer, @@ -76,6 +77,14 @@ type ConsumedMethod struct { // "/package.Service/Method" constant a gRPC or Connect stub exports. The // module's generated package must be linked into the solution. Name string + // Scopes, when set, is the authority minted for the viewer on a call to + // this method, instead of the module's Scopes — never in addition to them. + // It is how one module's methods that need different authority are + // declared under one `as`: each call mints only what its own method needs, + // so a viewer who lacks one method's authority is refused that method + // (permission_denied) and is still served every other. Not allowed on a + // ViewerBearer module, which mints nothing. + Scopes []Scope // Response is the allowlist of response fields returned to the page. A // field the module adds later is dropped until someone names it here. Response FieldMask @@ -145,11 +154,14 @@ func resolvePassthrough(modules []ConsumedModule) (map[string]passthroughRoute, return nil, fmt.Errorf("consumed module %q: as must be one path segment, the `as` of its api.consumes entry", module.As) case seen[module.As]: return nil, fmt.Errorf("consumed module %q is declared twice", module.As) - case module.ViewerBearer == (len(module.Scopes) > 0): + case module.ViewerBearer && len(module.Scopes) > 0: return nil, fmt.Errorf("consumed module %q: declare exactly one of Scopes (the authority minted for the viewer) and ViewerBearer (the module authenticates the viewer itself)", module.As) case len(module.Methods) == 0: return nil, fmt.Errorf("consumed module %q declares no methods", module.As) } + if err := checkScopes(module.Scopes); err != nil { + return nil, fmt.Errorf("consumed module %q: %w", module.As, err) + } seen[module.As] = true for _, method := range module.Methods { md, err := methodDescriptor(method.Name) @@ -171,6 +183,15 @@ func resolvePassthrough(modules []ConsumedModule) (map[string]passthroughRoute, case method.Response.desc != nil && method.Response.desc.FullName() != md.Output().FullName(): return nil, fmt.Errorf("consumed module %q: %s answers %s, but its response mask is over %s", module.As, md.FullName(), md.Output().FullName(), method.Response.desc.FullName()) } + switch { + case module.ViewerBearer && len(method.Scopes) > 0: + return nil, fmt.Errorf("consumed module %q: %s declares Scopes, but the module is ViewerBearer: it mints nothing, so the scopes would never be asked for", module.As, md.FullName()) + case !module.ViewerBearer && len(module.Scopes) == 0 && len(method.Scopes) == 0: + return nil, fmt.Errorf("consumed module %q: %s declares no Scopes and the module declares none either; name the authority minted for the viewer, or declare the module ViewerBearer", module.As, md.FullName()) + } + if err := checkScopes(method.Scopes); err != nil { + return nil, fmt.Errorf("consumed module %q: %s: %w", module.As, md.FullName(), err) + } if method.Pin != nil && method.Pin.ProtoReflect().Descriptor().FullName() != md.Input().FullName() { return nil, fmt.Errorf("consumed module %q: %s reads %s, but its pin is a %s", module.As, md.FullName(), md.Input().FullName(), method.Pin.ProtoReflect().Descriptor().FullName()) } @@ -184,6 +205,39 @@ func resolvePassthrough(modules []ConsumedModule) (map[string]passthroughRoute, return routes, nil } +// checkScopes refuses a scope that could not name an authority: one with no +// resource kind, no action, or a blank or duplicated action. +func checkScopes(scopes []Scope) error { + for _, scope := range scopes { + if strings.TrimSpace(scope.ResourceKind) == "" || scope.ResourceKind != strings.TrimSpace(scope.ResourceKind) { + return fmt.Errorf("a scope names no resource kind (or pads it with spaces)") + } + if len(scope.Actions) == 0 { + return fmt.Errorf("scope %q names no action", scope.ResourceKind) + } + seen := map[string]bool{} + for _, action := range scope.Actions { + if strings.TrimSpace(action) == "" || action != strings.TrimSpace(action) { + return fmt.Errorf("scope %q names a blank action (or pads one with spaces)", scope.ResourceKind) + } + if seen[action] { + return fmt.Errorf("scope %q names the action %q twice", scope.ResourceKind, action) + } + seen[action] = true + } + } + return nil +} + +// scopes is the authority a call to this route mints for the viewer: the +// method's own, else the module's. Never the union. +func (r passthroughRoute) scopes() []Scope { + if len(r.method.Scopes) > 0 { + return r.method.Scopes + } + return r.module.Scopes +} + // checkConsumed refuses a declaration for a module the solution does not // consume under that name: the gateway would federate no /v1/ prefix for // it, so every call would fail at the gateway instead of here. @@ -286,7 +340,7 @@ func (s *Server) forward(ctx context.Context, route passthroughRoute, req *conne defer cancel() gw := newGateway(s.cfg.gatewayURL, bearer, req.Header().Get(orgHeader), req.Header().Get(sessionHeader)) if !route.module.ViewerBearer { - acting, err := gw.ForModule(ctx, route.module.As, route.module.Scopes...) + acting, err := gw.ForModule(ctx, route.module.As, route.scopes()...) if err != nil { return nil, relayedError(err) } @@ -324,6 +378,23 @@ func (s *Server) forward(ctx context.Context, route passthroughRoute, req *conne // answer to the page's own request. Anything else (a transport failure, an // undecodable answer) is reported by kind only, never by its internal detail. func relayedError(err error) error { + var refusal *WorkContextRefusal + if errors.As(err, &refusal) && refusal != nil && refusal.StatusCode >= 400 && refusal.StatusCode < 500 { + // The issuer refused the viewer's authority for this call: say so, in + // its own code and words (they name the missing permission), rather + // than as a failed module call. It is the viewer's own authorization, + // so nothing of another principal is disclosed. + code := codeForStatus(refusal.StatusCode) + var parsed connect.Code + if refusal.Code != "" && parsed.UnmarshalText([]byte(refusal.Code)) == nil && parsed != connect.CodeUnknown { + code = parsed + } + message := "the viewer's authority for the " + refusal.Audience + " module was refused" + if refusal.Message != "" { + message += ": " + truncated(refusal.Message) + } + return connect.NewError(code, errors.New(message)) + } var clientErr *ClientError if errors.As(err, &clientErr) && clientErr != nil { return connect.NewError(codeForStatus(clientErr.StatusCode), errors.New(clientErr.Message)) @@ -410,7 +481,7 @@ func PassthroughOperations(modules ...ConsumedModule) ([]Operation, error) { for _, path := range paths { route := routes[path] callers := fmt.Sprintf("Signed-in viewers. The %s module authorizes every call itself, under a Work Context minted for the viewer (audience %q, %s).", - route.module.As, route.module.As, scopeText(route.module.Scopes)) + route.module.As, route.module.As, scopeText(route.scopes())) if route.module.ViewerBearer { callers = fmt.Sprintf("Signed-in viewers. The %s module authenticates the viewer's bearer and authorizes every call itself.", route.module.As) } diff --git a/passthrough_scopes_test.go b/passthrough_scopes_test.go new file mode 100644 index 0000000..092d88b --- /dev/null +++ b/passthrough_scopes_test.go @@ -0,0 +1,174 @@ +package solution + +import ( + "net/http" + "strings" + "testing" +) + +// perMethodModule declares two methods of one module that need different +// authority: Search a member's read, Get an administrator's inspect. +func perMethodModule() ConsumedModule { + return ConsumedModule{ + As: "things", + Methods: []ConsumedMethod{ + { + Name: "/things.v1.Things/Search", + Scopes: []Scope{{ResourceKind: "things", Actions: []string{"list"}}}, + WholeResponse: true, + }, + { + Name: "/things.v1.Things/Get", + Scopes: []Scope{{ResourceKind: "things", Actions: []string{"inspect"}}}, + WholeResponse: true, + }, + }, + } +} + +const getPath = "/modules/things/things.v1.Things/Get" + +func mintedActions(m mintRequest) string { + var parts []string + for _, scope := range m.AuthorityScopes { + parts = append(parts, scope.ResourceKind+":"+strings.Join(scope.Actions, ",")) + } + return strings.Join(parts, " ") +} + +func TestPassthroughMintsOnlyTheCalledMethodsScopes(t *testing.T) { + gw := newModuleGateway(t, http.StatusOK, `{"entryId":"e1"}`) + s := passthroughServer(gw.URL, perMethodModule()) + + if status, body := call(t, s, searchPath, "Bearer viewer", `{"entryId":"e1"}`); status != http.StatusOK { + t.Fatalf("search: status %d: %v", status, body) + } + if status, body := call(t, s, getPath, "Bearer viewer", `{"entryId":"e1"}`); status != http.StatusOK { + t.Fatalf("get: status %d: %v", status, body) + } + if len(gw.mints) != 2 { + t.Fatalf("mints = %+v, want one per call", gw.mints) + } + // Least privilege: each mint carries its own method's scope and nothing + // of the other's — never the union. + if got := mintedActions(gw.mints[0]); got != "things:list" { + t.Fatalf("search minted %q, want things:list only", got) + } + if got := mintedActions(gw.mints[1]); got != "things:inspect" { + t.Fatalf("get minted %q, want things:inspect only", got) + } + if gw.mints[0].Audience != "things" || gw.mints[1].Audience != "things" { + t.Fatalf("both mints are for the module's one audience: %+v", gw.mints) + } +} + +func TestPassthroughMethodScopesReplaceTheModules(t *testing.T) { + gw := newModuleGateway(t, http.StatusOK, `{"entryId":"e1"}`) + module := perMethodModule() + module.Scopes = []Scope{{ResourceKind: "things", Actions: []string{"read"}}} + module.Methods[0].Scopes = nil // Search falls back to the module's. + s := passthroughServer(gw.URL, module) + + call(t, s, searchPath, "Bearer viewer", `{}`) + call(t, s, getPath, "Bearer viewer", `{"entryId":"e1"}`) + if len(gw.mints) != 2 || mintedActions(gw.mints[0]) != "things:read" || mintedActions(gw.mints[1]) != "things:inspect" { + t.Fatalf("mints = %+v, want module scope for search and method scope (alone) for get", gw.mints) + } +} + +func TestPassthroughRefusesOnlyTheMethodWhoseAuthorityTheViewerLacks(t *testing.T) { + gw := newModuleGateway(t, http.StatusOK, `{"entryId":"e1"}`) + gw.deny = "inspect" + s := passthroughServer(gw.URL, perMethodModule()) + + status, body := call(t, s, getPath, "Bearer member", `{"entryId":"e1"}`) + if status != http.StatusForbidden || body["code"] != "permission_denied" { + t.Fatalf("get without inspect: status %d, body %v; want 403 permission_denied", status, body) + } + if msg, _ := body["message"].(string); !strings.Contains(msg, "things:inspect") { + t.Fatalf("message %q does not name the missing permission", msg) + } + if len(gw.calls) != 0 { + t.Fatalf("the module was called without a capability: %v", gw.calls) + } + // The member's own method is unaffected, and its mint was not widened. + status, body = call(t, s, searchPath, "Bearer member", `{"entryId":"e1"}`) + if status != http.StatusOK || body["entryId"] != "e1" { + t.Fatalf("search after a refused get: status %d, body %v", status, body) + } + if last := gw.mints[len(gw.mints)-1]; mintedActions(last) != "things:list" { + t.Fatalf("search minted %q, want things:list only", mintedActions(last)) + } + if len(gw.calls) != 1 { + t.Fatalf("module calls = %d, want only the search", len(gw.calls)) + } +} + +func TestPassthroughRefusesAnUnservableScopeDeclarationAtBoot(t *testing.T) { + search := func(scopes ...Scope) ConsumedMethod { + return ConsumedMethod{Name: "/things.v1.Things/Search", Scopes: scopes, WholeResponse: true} + } + read := Scope{ResourceKind: "things", Actions: []string{"read"}} + cases := map[string]struct { + module ConsumedModule + want string + }{ + "method scopes on a viewer-bearer module": { + ConsumedModule{As: "things", ViewerBearer: true, Methods: []ConsumedMethod{search(read)}}, + "ViewerBearer", + }, + "no scopes anywhere": { + ConsumedModule{As: "things", Methods: []ConsumedMethod{search()}}, + "declares no Scopes", + }, + "one method without scopes and none on the module": { + ConsumedModule{As: "things", Methods: []ConsumedMethod{search(read), {Name: "/things.v1.Things/Get", WholeResponse: true}}}, + "Get declares no Scopes", + }, + "empty action list on a method": { + ConsumedModule{As: "things", Methods: []ConsumedMethod{search(Scope{ResourceKind: "things"})}}, + "names no action", + }, + "empty action list on the module": { + ConsumedModule{As: "things", Scopes: []Scope{{ResourceKind: "things", Actions: []string{}}}, Methods: []ConsumedMethod{search()}}, + "names no action", + }, + "blank action": { + ConsumedModule{As: "things", Methods: []ConsumedMethod{search(Scope{ResourceKind: "things", Actions: []string{" "}})}}, + "blank action", + }, + "no resource kind": { + ConsumedModule{As: "things", Methods: []ConsumedMethod{search(Scope{Actions: []string{"read"}})}}, + "no resource kind", + }, + "duplicated action": { + ConsumedModule{As: "things", Methods: []ConsumedMethod{search(Scope{ResourceKind: "things", Actions: []string{"read", "read"}})}}, + "twice", + }, + } + for name, tc := range cases { + t.Run(name, func(t *testing.T) { + _, err := resolvePassthrough([]ConsumedModule{tc.module}) + if err == nil || !strings.Contains(err.Error(), tc.want) { + t.Fatalf("err = %v, want one mentioning %q", err, tc.want) + } + }) + } +} + +func TestPassthroughOperationsDocumentEachMethodsOwnScopes(t *testing.T) { + ops, err := PassthroughOperations(perMethodModule()) + if err != nil { + t.Fatal(err) + } + found := map[string]string{} + for _, op := range ops { + found[op.Path] = op.Callers + } + if !strings.Contains(found[searchPath], "things:[list]") || strings.Contains(found[searchPath], "inspect") { + t.Fatalf("search callers = %q", found[searchPath]) + } + if !strings.Contains(found[getPath], "things:[inspect]") || strings.Contains(found[getPath], "list") { + t.Fatalf("get callers = %q", found[getPath]) + } +} diff --git a/passthrough_test.go b/passthrough_test.go index f23abc6..169c553 100644 --- a/passthrough_test.go +++ b/passthrough_test.go @@ -37,6 +37,9 @@ type moduleGateway struct { *httptest.Server status int reply string + // deny, when set, is an action the viewer does not hold: a mint asking for + // it is refused the way accounts refuses one. + deny string mu sync.Mutex mints []mintRequest @@ -56,6 +59,17 @@ func newModuleGateway(t *testing.T, status int, reply string) *moduleGateway { _ = json.Unmarshal(body, &mint) mint.Bearer = r.Header.Get("authorization") g.mints = append(g.mints, mint) + for _, scope := range mint.AuthorityScopes { + for _, action := range scope.Actions { + if g.deny != "" && action == g.deny { + writeJSON(w, http.StatusForbidden, map[string]any{ + "code": "permission_denied", + "message": "owner is not allowed " + scope.ResourceKind + ":" + action + " at requested scope", + }) + return + } + } + } writeJSON(w, http.StatusOK, map[string]any{ "token": "context-" + mint.Audience + ".1", "orgId": mint.OrgID, "ownerPrincipalId": "viewer", "currentActorPrincipalId": "viewer", diff --git a/solution.go b/solution.go index fa1f1fd..8f532ce 100644 --- a/solution.go +++ b/solution.go @@ -2174,6 +2174,25 @@ func (g *Gateway) workContext(ctx context.Context) (codefly.WorkContextToken, er }) } +// WorkContextRefusal is the issuer's refusal to mint a Work Context: most +// often an authority the viewer does not hold ("owner is not allowed +// kind:action at requested scope"). It keeps the issuer's Connect code and +// message, so a caller can tell "you lack this permission" from "the gateway +// never routed the mint", and unwraps to the GatewayError it has always been. +type WorkContextRefusal struct { + Audience string + StatusCode int + // Code is the issuer's Connect code ("permission_denied"), when it sent one. + Code string + Message string +} + +func (e *WorkContextRefusal) Error() string { + return fmt.Sprintf("work context mint for %q rejected (status %d, %q): %s", e.Audience, e.StatusCode, e.Code, e.Message) +} + +func (e *WorkContextRefusal) Unwrap() error { return &GatewayError{StatusCode: e.StatusCode} } + // startTaskRequest is saas.accounts.v1.StartTaskWorkContextRequest on the wire. // The runtime speaks it as Connect JSON rather than linking the accounts client: // a solution's host is not this package's dependency. Leaving actorPrincipalId @@ -2213,8 +2232,9 @@ func (g *Gateway) mint(ctx context.Context, ask startTaskRequest) (codefly.WorkC Message string `json:"message"` } _ = json.NewDecoder(io.LimitReader(resp.Body, 4<<10)).Decode(&refusal) - return codefly.WorkContextToken{}, time.Time{}, fmt.Errorf("work context mint for %q rejected (status %d, %q): %s: %w", - ask.Audience, resp.StatusCode, refusal.Code, refusal.Message, &GatewayError{StatusCode: resp.StatusCode}) + return codefly.WorkContextToken{}, time.Time{}, &WorkContextRefusal{ + Audience: ask.Audience, StatusCode: resp.StatusCode, Code: refusal.Code, Message: refusal.Message, + } } var issued struct { Token string `json:"token"`