2 files changed, 132 insertions(+), 14 deletions(-)

M bearer/bearer.go
M bearer/bearer_test.go

@@ 240,6 240,21 @@ type Token struct {
 // waited out.
 func (t *Token) Registered() bool { return t.TokenID != 0 }
 
+// Authorize is step 3 of SPEC ch. 6, split out so that it can be asked where the
+// action is known — in the handler that implements it — rather than in the
+// middleware that resolved the identity. See Inspect.
+//
+// It reports ErrForbidden and not ErrInvalid: the credential is good and the
+// caller is who they say they are, and what is missing is a permission. That
+// distinction is what stops an agent retrying forever with a token that will
+// never grow the grant it needs.
+func (t *Token) Authorize(action string) error {
+	if !t.Grants.Has(action) {
+		return fmt.Errorf("%w: %q is not in %q", ErrForbidden, action, t.Grants.String())
+	}
+	return nil
+}
+
 // New builds a Validator, refusing options that would only fail later, one
 // request at a time, as an error about the network.
 func New(opts Options) (*Validator, error) {


@@ 324,6 339,70 @@ func New(opts Options) (*Validator, error) {
 //
 // and one that does not turns ErrNotOurs into 401 alongside ErrInvalid.
 func (v *Validator) Validate(ctx context.Context, presented, action string) (*Token, error) {
+	tok, err := decodeOurs(presented)
+	if err != nil {
+		return tok, err
+	}
+	// Step 3 before step 4, deliberately: a token that does not carry the grant
+	// is refused without a round trip to the daemon. Inspect cannot keep this
+	// ordering — it has no action to check — which is the one cost of the split
+	// and the reason this method still exists for callers that know both.
+	if err := tok.Authorize(action); err != nil {
+		return nil, err
+	}
+	if err := v.revoked(ctx, tok); err != nil {
+		return nil, err
+	}
+	return tok, nil
+}
+
+// Inspect is Validate without step 3: it answers who a token belongs to, what it
+// may do, and whether it is still live — and leaves the question of whether that
+// covers *this* action to the caller.
+//
+// It exists because of where the two questions get answered in a sourcehut
+// service. Identity is resolved once per request in middleware, upstream of the
+// router: that is where the cookie plane and the bearer plane meet and where a
+// principal is put on the context, and at that point nothing knows yet which
+// route will run, so nothing knows the action. The action is known one layer
+// down, in the handler that implements it. A validator that insisted on both at
+// once would force every service either to invent an action before it has one,
+// or to lift its bearer plane out of the middleware every other plane goes
+// through — and the second is how a surface ends up with two different ideas of
+// who is calling.
+//
+// So: call Inspect in the resolver and carry the Grants on the principal, then
+// call Token.Authorize in the handler. Validate stays for callers that know both
+// at one point, and is exactly those two calls.
+//
+// Everything Validate's doc comment says — about the sentinels, about step 2
+// being per-service policy, and about the returned token being nil for every
+// failure except ErrNotOurs — applies here unchanged.
+func (v *Validator) Inspect(ctx context.Context, presented string) (*Token, error) {
+	tok, err := decodeOurs(presented)
+	if err != nil {
+		return tok, err
+	}
+	if err := v.revoked(ctx, tok); err != nil {
+		return nil, err
+	}
+	return tok, nil
+}
+
+// revoked is step 4: a registered token has a revocation to ask about, a
+// stateless one has no row and so completes without any network at all — which
+// is the common case under the default configuration.
+func (v *Validator) revoked(ctx context.Context, tok *Token) error {
+	if tok.TokenID == 0 {
+		return nil
+	}
+	return v.checkRevocation(ctx, tok.TokenID)
+}
+
+// decodeOurs is steps 1 and 2 plus the grant parse: everything that can be
+// decided from the token itself, with no clock but the real one and no network
+// at all.
+func decodeOurs(presented string) (*Token, error) {
 	// Step 1. Signature, version and expiry, all of it local.
 	//
 	// DecodeBearerToken returns nil for all three and distinguishes none of


@@ 360,20 439,6 @@ func (v *Validator) Validate(ctx context.Context, presented, action string) (*To
 		Expires:  bt.Expires.Time(),
 	}
 
-	// Step 3. Does it cover what is being attempted?
-	if !g.Has(action) {
-		return nil, fmt.Errorf("%w: %q is not in %q", ErrForbidden, action, g.String())
-	}
-
-	// Step 4. Only a registered token has a revocation to check. A stateless one
-	// has no row, so there is nothing to ask about and nothing to reach for:
-	// this is the common case and it completes without any network at all.
-	if tok.TokenID == 0 {
-		return tok, nil
-	}
-	if err := v.checkRevocation(ctx, tok.TokenID); err != nil {
-		return nil, err
-	}
 	return tok, nil
 }
 

@@ 308,6 308,59 @@ func TestMissingGrantIsForbiddenAndStopsBeforeTheNetwork(t *testing.T) {
 		"step 3 refuses locally; a token that cannot do the thing must not cost a round trip")
 }
 
+// Inspect is what a resolver calls in middleware, where the action is not known
+// yet — so it must answer for a token whose grants would not cover whatever runs
+// next, and leave that refusal to the handler one layer down.
+func TestInspectAnswersWithoutAnActionAndLeavesStepThreeToTheCaller(t *testing.T) {
+	d := newDaemon(t, http.StatusNoContent)
+	v := newValidator(t, d.server.URL, nil)
+
+	tok, err := v.Inspect(context.Background(), ourToken("bench:read id:42"))
+	require.NoError(t, err, "a token is inspectable whatever the caller goes on to attempt")
+	assert.Equal(t, 42, tok.TokenID)
+	assert.True(t, tok.Grants.Has("bench:read"))
+
+	// The revocation half is not the caller's to skip, so it was still asked.
+	// This is the one cost of the split: Inspect cannot keep Validate's
+	// refuse-before-the-network ordering, because it has no action to refuse on.
+	assert.Equal(t, 1, d.count())
+
+	// And step 3 is available where the action finally is known.
+	require.NoError(t, tok.Authorize("bench:read"))
+	err = tok.Authorize("bench:upload")
+	require.Error(t, err)
+	assert.True(t, errors.Is(err, ErrForbidden), "want ErrForbidden, got %v", err)
+}
+
+// A stateless token has no row, so Inspect completes without touching the
+// network at all — the common case under the default configuration.
+func TestInspectOfAStatelessTokenTouchesNoNetwork(t *testing.T) {
+	d := newDaemon(t, http.StatusNoContent)
+	v := newValidator(t, d.server.URL, nil)
+
+	tok, err := v.Inspect(context.Background(), ourToken("bench:upload"))
+	require.NoError(t, err)
+	assert.Zero(t, tok.TokenID)
+	assert.False(t, tok.Registered())
+	assert.Zero(t, d.count())
+}
+
+// A foreign token is not ours whether it is asked about with an action or
+// without one, and both ways hand the decoded token back so the service can
+// apply its own meta-PAT policy.
+func TestInspectReportsAForeignTokenTheSameWayValidateDoes(t *testing.T) {
+	v := newValidator(t, "https://tokens.srht.bigb.es", nil)
+	// A meta.sr.ht PAT: same signing key, foreign ClientID, and a grant string in
+	// core-go's OAuth grammar that our parser would reject — which is exactly why
+	// step 2 has to come before the parse.
+	pat := seal("bigbes", "meta.sr.ht", "git.sr.ht/OBJECTS:RW", time.Now().Add(time.Hour))
+
+	tok, err := v.Inspect(context.Background(), pat)
+	require.Error(t, err)
+	assert.True(t, errors.Is(err, ErrNotOurs), "want ErrNotOurs, got %v", err)
+	require.NotNil(t, tok, "step 2 hands the token back; that is the whole point of it")
+}
+
 func TestUniversalGrantAdmitsTheAction(t *testing.T) {
 	v := newValidator(t, "https://tokens.srht.bigb.es", nil)