mirror of
https://github.com/zap-proto/zip.git
synced 2026-10-19 15:38:57 +00:00
A rule is handed the operation as served
The authorizer and the result hook were given the op as DECLARED: for anything declared on a group, the bare leaf and an id derived from it. So a rule keyed on op.Path governed an address nobody serves — /probe/:id where the route table, the registry and the document all say /v1/crm/probe/:id — and one definition mounted at two prefixes reported a single name for two operations. They now see what the route matched: the composed pattern, and the composed id by the same rule the registry uses, where a declared id is the operation's own name and composition does not get a vote. The concrete address stays off Op, because path parameters are input and the hook is promised the operation and the outcome and never the input; a handler that needs the resolved address asks AddressOf. Co-authored-by: Hanzo Dev <[email protected]>
This commit is contained in:
+53
-25
@@ -4,6 +4,7 @@ import (
|
||||
"context"
|
||||
"encoding/json"
|
||||
"net/http/httptest"
|
||||
"strings"
|
||||
"testing"
|
||||
|
||||
"github.com/zap-proto/zip"
|
||||
@@ -57,32 +58,31 @@ func TestAddressOf_IsThisCallsConcreteAddress(t *testing.T) {
|
||||
}
|
||||
}
|
||||
|
||||
// THE CONCRETE ADDRESS IS NOT ON Op, deliberately.
|
||||
// THE CONCRETE ADDRESS IS NOT ON Op, deliberately — but the PATTERN on it is
|
||||
// the one the call was served at.
|
||||
//
|
||||
// Op is what the authorizer is handed and what OnResult is told, and both are
|
||||
// narrow on purpose — the result hook is given the operation and the outcome and
|
||||
// never the input. A concrete address carries path parameters, which ARE input,
|
||||
// so putting it on Op would widen two contracts as a side effect of serving a
|
||||
// third. This is the test that says so, because a later reader would otherwise
|
||||
// fix the "inconsistency" by helpfully resolving Op.Path.
|
||||
// Op is what the authorizer is handed and what OnResult is told. It carries the
|
||||
// composed pattern and the composed operation id: the same identity the route
|
||||
// table and the by-name registry publish. It does NOT carry the concrete
|
||||
// address, because path parameters are input and OnResult is promised the
|
||||
// operation and the outcome and never the input.
|
||||
//
|
||||
// It also pins what those two are handed TODAY, which is worth knowing: the
|
||||
// DECLARED leaf, not the address the call is served at. So Op.Path means the
|
||||
// declaration to a rule and the served pattern to [zip.OpOf], and a rule cannot
|
||||
// tell two mountings of one definition apart. That is a real limit and it is
|
||||
// older than this test; changing what authorization sees is its own decision,
|
||||
// not a side effect of giving a handler its address.
|
||||
func TestAddressOf_WhatARuleIsHandedIsUnchanged(t *testing.T) {
|
||||
// Both halves were wrong the other way once. A rule keyed on op.Path saw the
|
||||
// bare leaf for anything declared on a group — "/traces/:id" for an op served
|
||||
// at "/v1/o11y/traces/:id" — so every path-keyed rule in a service that
|
||||
// declares relatively governed an address nobody serves, and one definition
|
||||
// mounted at two prefixes reported one name for two operations.
|
||||
func TestOp_ARuleIsHandedTheOpAsServed(t *testing.T) {
|
||||
app := zip.New(zip.Config{AppName: "traces", DisableStartupMessage: true})
|
||||
app.Group("/v1/o11y").Get("/traces/:id", tracer{}.Spans)
|
||||
|
||||
var asked []string
|
||||
var toldPath string
|
||||
var told string
|
||||
app.Authorize(func(_ context.Context, op zip.Op, _ any) (zip.Decision, error) {
|
||||
asked = append(asked, op.Path)
|
||||
asked = append(asked, op.Method+" "+op.Path+" id="+op.OperationID)
|
||||
return zip.Decision{Effect: zip.Allow}, nil
|
||||
})
|
||||
app.OnResult(func(_ context.Context, op zip.Op, _ error) { toldPath = op.Path })
|
||||
app.OnResult(func(_ context.Context, op zip.Op, _ error) { told = op.Path + " id=" + op.OperationID })
|
||||
if err := app.Build(); err != nil {
|
||||
t.Fatal(err)
|
||||
}
|
||||
@@ -93,20 +93,48 @@ func TestAddressOf_WhatARuleIsHandedIsUnchanged(t *testing.T) {
|
||||
}
|
||||
_ = resp.Body.Close()
|
||||
|
||||
// The declaration, unresolved — and no concrete address anywhere near it.
|
||||
if len(asked) != 1 || asked[0] != "/traces/:id" {
|
||||
t.Errorf("authorizer saw %v, want the declared leaf", asked)
|
||||
// The composed pattern and the id derived from it, exactly as the route
|
||||
// table and the registry carry them.
|
||||
want := "GET /v1/o11y/traces/:id id=" + zip.ID("GET", "/v1/o11y/traces/:id")
|
||||
if len(asked) != 1 || asked[0] != want {
|
||||
t.Errorf("authorizer saw %v, want [%q]", asked, want)
|
||||
}
|
||||
if toldPath != "/traces/:id" {
|
||||
t.Errorf("result hook was told %q, want the declared leaf", toldPath)
|
||||
if told != "/v1/o11y/traces/:id id="+zip.ID("GET", "/v1/o11y/traces/:id") {
|
||||
t.Errorf("result hook was told %q", told)
|
||||
}
|
||||
for _, p := range append(asked, toldPath) {
|
||||
if p == "/v1/o11y/traces/abc123" {
|
||||
t.Errorf("a rule was handed the concrete address %q; path parameters are input", p)
|
||||
|
||||
// And never the resolved address: parameters are input.
|
||||
for _, s := range append(asked, told) {
|
||||
if strings.Contains(s, "abc123") {
|
||||
t.Errorf("a rule was handed the concrete address in %q; path parameters are input", s)
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
// A DECLARED id is the operation's own name, and composition does not get a
|
||||
// vote on it — the same rule occurrenceID applies in the registry.
|
||||
func TestOp_ADeclaredIDSurvivesComposition(t *testing.T) {
|
||||
app := zip.New(zip.Config{AppName: "traces", DisableStartupMessage: true})
|
||||
app.Group("/v1/o11y").Get("/traces/:id", tracer{}.Spans).ID("traces.spans")
|
||||
|
||||
var seen string
|
||||
app.Authorize(func(_ context.Context, op zip.Op, _ any) (zip.Decision, error) {
|
||||
seen = op.OperationID + " at " + op.Path
|
||||
return zip.Decision{Effect: zip.Allow}, nil
|
||||
})
|
||||
if err := app.Build(); err != nil {
|
||||
t.Fatal(err)
|
||||
}
|
||||
resp, err := app.Test(httptest.NewRequest("GET", "/v1/o11y/traces/abc", nil))
|
||||
if err != nil {
|
||||
t.Fatal(err)
|
||||
}
|
||||
_ = resp.Body.Close()
|
||||
if seen != "traces.spans at /v1/o11y/traces/:id" {
|
||||
t.Errorf("rule saw %q, want the declared id at the composed path", seen)
|
||||
}
|
||||
}
|
||||
|
||||
// An op with no parameter has an address equal to its pattern, and pays no
|
||||
// reflection to find that out.
|
||||
func TestAddressOf_NoParameterIsThePatternItself(t *testing.T) {
|
||||
|
||||
@@ -6,7 +6,9 @@ func TestExtract_TwoRawCallsAreBothDocumented(t *testing.T) {
|
||||
p := load(t, "twoaddr")
|
||||
if len(p.Ops) != 2 {
|
||||
var k []string
|
||||
for _, o := range p.Ops { k = append(k, o.Key()) }
|
||||
for _, o := range p.Ops {
|
||||
k = append(k, o.Key())
|
||||
}
|
||||
t.Fatalf("ops = %d, want 2; got %v", len(p.Ops), k)
|
||||
}
|
||||
for _, want := range []string{"POST /v1/billing/invoices/:id/reminders", "POST /v1/billing/send-invoice-reminder"} {
|
||||
|
||||
@@ -655,6 +655,18 @@ func registerTyped[In, Out any](depth int, on *App, method, path string, fn Type
|
||||
// The authorizer below and the result hook are handed [Op] as an argument
|
||||
// and are unaffected: neither is given the address.
|
||||
ctx = withAddress(ctx, meta, in)
|
||||
|
||||
// AS SERVED, not as declared. A rule keyed on the path has to see the
|
||||
// address the caller reached, and a recorder has to name the operation
|
||||
// the way the route table and the registry name it. Both used to be
|
||||
// handed the registration — the bare leaf for anything declared on a
|
||||
// group — so a path-keyed rule silently governed an address nobody
|
||||
// serves, and one definition mounted twice reported one name for two
|
||||
// operations.
|
||||
served := meta
|
||||
if c, ok := ctx.Value(opKey{}).(*invocation); ok {
|
||||
served = c.op
|
||||
}
|
||||
// TOLD ONCE, ON EVERY WAY OUT. This is the one contract every projection
|
||||
// funnels through — REST, an MCP tools/call, the call plane, the graph and
|
||||
// an in-process invoke — so a hook here is told about all of them, and a
|
||||
@@ -672,7 +684,7 @@ func registerTyped[In, Out any](depth int, on *App, method, path string, fn Type
|
||||
// and which of those happened is carried by the [Approval] the caller
|
||||
// receives rather than by this.
|
||||
if told := op.result(); told != nil {
|
||||
defer func() { told(ctx, meta, err) }()
|
||||
defer func() { told(ctx, served, err) }()
|
||||
}
|
||||
if err := validate(in); err != nil {
|
||||
return nil, ErrBadRequest(err.Error())
|
||||
@@ -684,7 +696,7 @@ func registerTyped[In, Out any](depth int, on *App, method, path string, fn Type
|
||||
// projection funnels through, so allow, deny and approve reach REST, MCP,
|
||||
// the call plane, the graph and the CLI without a branch in any of them.
|
||||
if auth := op.rule(); auth != nil {
|
||||
d, err := auth(ctx, meta, in)
|
||||
d, err := auth(ctx, served, in)
|
||||
if err != nil {
|
||||
return nil, err // the check itself failed, which is not a refusal
|
||||
}
|
||||
@@ -785,7 +797,14 @@ func registerTyped[In, Out any](depth int, on *App, method, path string, fn Type
|
||||
// its absolute path, so the pattern fiber matched is where this op is
|
||||
// served — including the prefix of whichever host composed it, which the
|
||||
// registration-time identity cannot name.
|
||||
served := Op{Method: meta.Method, Path: c.Route().Path, OperationID: meta.OperationID}
|
||||
//
|
||||
// The id follows the same rule the registry uses (see occurrenceID): a
|
||||
// DECLARED id is the op's own name and composition does not get a vote;
|
||||
// an underived one is computed from the address it is actually served at.
|
||||
served := Op{Method: meta.Method, Path: c.Route().Path, OperationID: op.OperationID}
|
||||
if served.OperationID == "" {
|
||||
served.OperationID = ID(meta.Method, served.Path)
|
||||
}
|
||||
out, err := op.invoke(withOp(callerContext(c), served), jsonenc.Unmarshal, body, c.Queries(), path, header)
|
||||
if err != nil {
|
||||
return err
|
||||
|
||||
Reference in New Issue
Block a user