diff --git a/authn/bearer.go b/authn/bearer.go index cd61aabe6cef116ac440fe4f37b9a526dc81d26f..df0f920106220ab53babe1c6180830607bc7f768 100644 --- a/authn/bearer.go +++ b/authn/bearer.go @@ -67,8 +67,11 @@ LookupUser(ctx context.Context, username string) (InstanceUser, error) } // resolveInstanceToken runs the tokens.sr.ht plane against a presented bearer -// credential. Its answer is final: there is one agent credential plane, so a -// refusal here is the service's refusal. +// credential. Its answer is final: this plane has nothing behind it, so a refusal +// here is the service's refusal on every surface that reaches it. /query is the +// one that may not — it routes a PAT to MetaAuth before asking this — and that +// choice is made on the credential, upstream, rather than by this function +// handing a refusal on to somebody else. // // It used to report a third thing — whether the caller should fall back to // spec's own agent_token store — and exactly two refusals said yes: @@ -81,8 +84,9 @@ // business rather than this one's, and falling through cost one hash lookup // that would miss. // // With that store gone both are plain refusals. The one consequence worth -// naming is ErrNotOurs: IsAuthFailure now counts it permanent, so a meta PAT -// presented here earns a 401 rather than the 503 an unclassified error would. +// naming is ErrNotOurs: IsAuthFailure counts it permanent, so a meta PAT that +// does reach this plane earns a 401 rather than the 503 an unclassified error +// would. // // The rest of the mapping is unchanged and lives in StatusFor: ErrInvalid and // ErrRevoked are 401, ErrForbidden and a foreign owner are 403, and @@ -148,16 +152,20 @@ } return "tokens.sr.ht (stateless)" } -// StatusFor maps an error out of Resolve — or out of a later Authorize — onto -// the status the surface must answer with. It is one function so that the three -// surfaces cannot each invent their own table. +// StatusFor maps an error out of Resolve — or out of a later Authorize, or out +// of MetaAuth.VerifyToken — onto the status the surface must answer with. It is +// one function so that the surfaces cannot each invent their own table, and one +// function across both credential planes so that the two cannot answer a client +// differently for the same kind of failure. That is the whole of what makes them +// consistent: the prose of a refusal is per-plane and its status is not. // // Everything the credential itself can be wrong about is bearer.StatusFor's // answer, not a second copy of it: ErrForbidden is 403, ErrUnavailable is 503 // and never 401, and ErrInvalid, ErrRevoked and ErrNotOurs are 401. That last // arm is only reached because ErrNotOurs is decided before we ask — a meta.sr.ht // PAT used to fall through to spec's own token store, and with that store gone -// it is a refusal at the door. +// it is a refusal at the door of every surface but /query, which routes one to +// MetaAuth instead. // // The ErrUnavailable line is the one worth restating even though it is no longer // spelled here. Reading "I could not reach tokens.sr.ht" as "your token is @@ -167,31 +175,37 @@ // clients their credentials are bad when the truth is that one service is down. // // What this function adds is what bearer cannot know: // -// - ErrMissingGrant and ErrNotInstanceOwner are 403. The credential verifies -// and the holder is who they say they are, so retrying is pointless and what -// they need is a wider grant, not another login. Both are asked before the -// bearer table, because ErrMissingGrant is raised beside a token that -// verified and must not be read as one that did not. -// - Whatever else IsAuthFailure calls permanent is 401 — today that is -// ErrNoToken, nothing having been presented on a surface that requires a -// credential. The predicate is asked rather than the sentinel listed a second -// time, so that a sentinel added to one of them cannot be missing from the -// other: this package's two answers to "is the credential the problem?" have -// to agree, and the cheapest way to guarantee that is for one to be built -// from the other. -// - ErrNoAgentPlane, and anything else at all, is 503. An instance with no -// [tokens.sr.ht] origin cannot check any credential, and telling the holder -// of a good token that it is bad would send them to re-provision it; an -// unclassified error is a backend that could not answer. This is where the -// two tables' defaults deliberately differ — bearer's unrecognised failure -// is the caller's credential, because everything reaching it is about a -// credential, while an unrecognised failure here can be the database this -// resolver had to consult, which must never read as a bad token. +// - ErrMissingGrant, ErrMissingScope and ErrNotInstanceOwner are 403. The +// credential verifies and the holder is who they say they are, so retrying is +// pointless and what they need is a wider permission, not another login. All +// three are asked before the bearer table, because each is raised beside a +// token that verified and must not be read as one that did not. +// - Whatever else IsAuthFailure calls permanent is 401 — ErrNoToken, nothing +// having been presented on a surface that requires a credential, and +// ErrInvalidPersonalToken, the meta plane's counterpart of the bearer +// sentinels above. The predicate is asked rather than its members listed a +// second time, so that a sentinel added to one of them cannot be missing from +// the other: this package's two answers to "is the credential the problem?" +// have to agree, and the cheapest way to guarantee that is for one to be +// built from the other. +// - ErrNoAgentPlane, ErrMetaUnavailable, and anything else at all, is 503. An +// instance with no [tokens.sr.ht] origin cannot check any credential, a +// meta.sr.ht that will not answer means this one could not be checked, and +// telling the holder of a good token that it is bad would send them to +// re-provision it; an unclassified error is a backend that could not answer. +// This is where the two tables' defaults deliberately differ — bearer's +// unrecognised failure is the caller's credential, because everything reaching +// it is about a credential, while an unrecognised failure here can be the +// database this resolver had to consult, which must never read as a bad token. +// The meta plane leans on that default rather than being listed: its +// classification is written to land on this arm for a sentinel nobody has seen +// before, which is the fail-closed direction. func StatusFor(err error) int { switch { case err == nil: return http.StatusOK - case errors.Is(err, ErrMissingGrant), errors.Is(err, ErrNotInstanceOwner): + case errors.Is(err, ErrMissingGrant), errors.Is(err, ErrMissingScope), + errors.Is(err, ErrNotInstanceOwner): return http.StatusForbidden case isBearerRefusal(err): return bearer.StatusFor(err) diff --git a/authn/doc.go b/authn/doc.go index 3670a64a76f5b9036e686fdb06060e214829778a..455bc48f0de8df903b6801ecdb3085d856936242 100644 --- a/authn/doc.go +++ b/authn/doc.go @@ -14,29 +14,52 @@ // and nothing finer. The boundary that bounds an agent's damage is the refs rule // (agents may only write proposals/*), and that lives in gitx; nothing here // replaces it. // -// # One agent credential plane +// # Two agent credential planes, and one surface that takes the second // // An agent is recognised by a tokens.sr.ht working token — PlaneInstance: // signed by the instance, expiring, owned by a meta.sr.ht account, and carrying // a grant set (ActionPropose, ActionRead). It is validated by sr-ht-ecore's -// bearer package, and it is the only credential this service authenticates an -// agent with. +// bearer package, and it is the credential of every surface this service has: +// the REST write plane, /mcp, the machine formats of the read plane, and the +// `git push` hook path. +// +// /query takes a second one — PlaneMeta, an ordinary meta.sr.ht personal access +// token scoped by ScopeRead — and it takes it because of api.sr.ht rather than +// because a second credential is desirable. The gateway forwards ONE client +// Authorization header to every service a federated query touches: its +// AuthMiddleware copies the client's header verbatim into the request context, +// and the internal credential it can mint is used only to fetch schemas at +// startup. So a federated caller arrives here holding whatever credential the +// client had, and the only credential a client can hold that works across the +// whole instance is a meta PAT. An endpoint that refuses one can never be +// federated: adding it to the gateway would be one `api-origin=` line that +// produces 401s. +// +// The scope of that exception is narrow and is kept narrow structurally rather +// than by discipline. MetaAuth is not part of Resolver, so nothing that resolves +// identity through Resolver can produce a PlaneMeta principal — graph builds the +// plane itself, wired from cmd/specsrht at the single call site that may use it, +// and no other surface holds one. A personal access token is therefore not a way +// around the tokens.sr.ht grant an upload, an MCP tool or a push requires. // -// spec used to mint its own as well — the agent_token row: one instance-wide -// shared secret, hashed at rest, with no owner, no expiry and no grants. That -// plane is gone. Issuance is centralised in tokens.sr.ht, so there is one door -// and nothing behind it: a credential this plane refuses is refused, rather than -// being offered to a second store that might say yes. A well-formed token from -// another issuer (a meta.sr.ht PAT — bearer.ErrNotOurs) used to fall through to -// that store and now fails at the door, which is the same answer one hash lookup -// later, said honestly. +// spec used to mint its own credential as well — the agent_token row: one +// instance-wide shared secret, hashed at rest, with no owner, no expiry and no +// grants. That plane is gone. Issuance is centralised — tokens.sr.ht mints a +// working token, meta.sr.ht mints a PAT — so each plane has one door and nothing +// behind it: a credential a plane refuses is refused, rather than being offered +// to a second store that might say yes. A meta PAT presented to any surface but +// /query is bearer.ErrNotOurs and fails at that door, which is the same answer +// one hash lookup later, said honestly. // -// The instance plane names an owner where the local secret had none. +// Both credential planes name an owner where the local secret had none. // Principal.Owner means "the human this agent acts for", which on this // single-owner instance is always [sr.ht] owner-name — a token belonging to // anybody else is refused rather than admitted as a second identity, because // every consumer of that field (the provenance committer, the refs rule's -// principal kind, the coreauth AuthContext) is written for one human. +// principal kind, the coreauth AuthContext) is written for one human. That rule +// is why MetaAuth is told the owner too: a PAT is the credential every account on +// the instance can mint, so without it the widest possible credential would be +// the one that skipped the narrowest check. // // Grants are orthogonal to the refs rule and to provenance, and replace neither. // A grant says what an instance token was minted for; the refs rule still says @@ -86,10 +109,13 @@ // Sentinel errors. Callers compare with errors.Is. The split that matters is // permanent (the credential is bad — 401/403) versus transient (the backend // could not answer — 503); IsAuthFailure draws it. // -// Everything the credential itself can be wrong about is now spelled by -// sr-ht-ecore's bearer package — ErrInvalid, ErrNotOurs, ErrRevoked — because -// there is one issuer and one validator. The sentinels below are what this -// service adds on top of that answer. +// Everything a working token can be wrong about is spelled by sr-ht-ecore's +// bearer package — ErrInvalid, ErrNotOurs, ErrRevoked — because that plane has +// one issuer and one validator. The sentinels below are what this service adds +// on top of that answer, and they now include the meta plane's three: its +// validator's refusals are metapat's, and its prose has to name meta.sr.ht where +// bearer's names tokens.sr.ht, while the status each one maps to is deliberately +// identical. See ScopeRead and MetaAuth. var ( // ErrNoToken is returned when a bearer credential was expected but the // request carried no Authorization header, or one in another scheme. @@ -138,6 +164,50 @@ // // It is raised by Principal.Authorize, at the layer that knows the action — // never by the resolver, which runs before the router and so knows none. ErrMissingGrant = errors.New("token does not grant this action") + + // ErrMissingScope marks a meta.sr.ht personal access token that authenticated + // fine and was not minted to read through this service: it does not carry + // ScopeRead. 403, exactly as ErrMissingGrant is and for the same reason. + // + // It is deliberately not ErrMissingGrant. The two name permissions in + // vocabularies that do not overlap — no PAT can carry "spec:read", because + // meta's personal-token page cannot spell it, and no working token can carry + // "spec.sr.ht/SPECS", because ecore's grants grammar does not read that shape + // — so a refusal has to name the one the caller can actually go and obtain. + // A PAT holder sent looking for "spec:read" would be hunting a checkbox that + // does not exist. + // + // Unlike ErrMissingGrant it is raised at resolution rather than at the router, + // which is not an inconsistency: the plane that raises it guards /query alone, + // every field of which is a read, so the action is known before the router. + ErrMissingScope = errors.New("token does not carry the required OAuth scope") + + // ErrInvalidPersonalToken marks a personal access token this service will not + // accept: the signature or the expiry did not hold, meta.sr.ht reports it + // revoked, or it names an account meta will not resolve. 401 through + // IsAuthFailure, beside the bearer sentinels that say the same thing about the + // other plane. + // + // It is spec's own rather than a reuse of bearer.ErrInvalid because bearer's + // sentinels are worded about tokens.sr.ht ("bearer: token was not issued by + // tokens.sr.ht"), and for this plane every one of those words is wrong. The + // status is what has to agree between the two planes, not the prose. + ErrInvalidPersonalToken = errors.New("personal access token was refused") + + // ErrMetaUnavailable marks a personal access token that could not be + // *checked*: the profile mirror or the revocation lookup at meta.sr.ht did not + // answer. + // + // It is pointedly absent from IsAuthFailure, so StatusFor's fail-closed + // default answers 503. "I could not decide" is not "your credential is bad", + // and answering 401 to a meta.sr.ht restart would tell every federated client + // on the instance to go and re-mint credentials that were never broken. + // + // It is not bearer.ErrUnavailable for the same reason as above, sharpened: that + // sentinel says tokens.sr.ht could not be reached, and tokens.sr.ht can be + // perfectly healthy while this is raised. An operator reading the wrong daemon + // out of a log line goes and looks in the wrong place. + ErrMetaUnavailable = errors.New("meta.sr.ht could not be reached") ) // IsAuthFailure reports whether err is a permanent credential failure — the @@ -148,13 +218,21 @@ // never reads as a valid credential. // // bearer.ErrNotOurs joined the set when the local plane left it. A meta.sr.ht // PAT used to fall through to spec's own store, where it missed; with one door -// there is nothing to fall through to, and "that credential was issued by -// somebody whose tokens this service does not take" is as permanent a refusal as -// a signature that does not verify. bearer.ErrUnavailable is pointedly absent — -// see StatusFor, which is what surfaces should map with. +// per plane there is nothing to fall through to, and "that credential was issued +// by somebody whose tokens this surface does not take" is as permanent a refusal +// as a signature that does not verify. It is still the answer everywhere but +// /query, which routes a PAT to MetaAuth before this plane is asked and so never +// reaches it — see graph's resolveCaller. +// +// ErrInvalidPersonalToken is that plane's counterpart and sits here for the same +// reason its siblings do: what makes the two planes consistent is that a client +// gets the same status for the same kind of failure, whichever credential it +// presented. bearer.ErrUnavailable and ErrMetaUnavailable are both pointedly +// absent — see StatusFor, which is what surfaces should map with. func IsAuthFailure(err error) bool { return errors.Is(err, ErrNoToken) || errors.Is(err, bearer.ErrInvalid) || errors.Is(err, bearer.ErrNotOurs) || - errors.Is(err, bearer.ErrRevoked) + errors.Is(err, bearer.ErrRevoked) || + errors.Is(err, ErrInvalidPersonalToken) } diff --git a/authn/meta.go b/authn/meta.go new file mode 100644 index 0000000000000000000000000000000000000000..02d751a80e1560be2157eb09d766dc8ddf1a53f5 --- /dev/null +++ b/authn/meta.go @@ -0,0 +1,263 @@ +package authn + +import ( + "context" + "errors" + "fmt" + "strings" + + "sourcecraft.dev/bigbes/sr-ht-core/auth" + + "sourcecraft.dev/bigbes/sr-ht-ecore/metapat" + + "sourcecraft.dev/bigbes/sr-ht-spec/core" +) + +// ScopeRead is the OAuth grant a meta.sr.ht personal access token must carry to +// read through this service: "spec.sr.ht/SPECS", at :RO or better. +// +// It is meta.sr.ht's vocabulary and it is not ActionRead spelled differently. +// The two grammars share a token format and nothing else: ActionRead +// ("spec:read") is what tokens.sr.ht seals into a working token, and no PAT can +// carry it because meta's personal-token page cannot spell it; ScopeRead is a +// checkbox on that page, and no working token can carry it because ecore's +// grants parser does not read that shape at all. A credential has one or the +// other and never both, which is why the two planes are asked different +// questions about the same request (Principal.Authorize, metapat.Allows). +// +// The service half is ConfigSection, so a rename breaks the build here rather +// than leaving behind a scope nobody can be granted. The bare half — "SPECS" — +// is what api-meta.json publishes, because meta prefixes the service name +// itself; cmd/specsrht derives that list from this constant rather than spelling +// it a second time, and a test on each side asserts the two agree. A scope +// published and not checked admits what should have been refused; one checked +// and not published cannot be minted at all; neither failure is visible from +// inside a single file. +const ScopeRead = ConfigSection + "/SPECS" + +// MetaValidator is the sliver of sr-ht-ecore's metapat.Validator this plane +// needs. +// +// The interface is declared in the consumer, as BearerValidator and UserLookup +// are and for the same reasons: it states exactly how much of the shared +// validator this package depends on — one method — and it is what keeps authn +// testable with no meta.sr.ht, no network and no Postgres. The concrete +// validator reaches core-go's auth.LookupUser, which reads a database handle out +// of the context and panics without one; holding it behind an interface is what +// keeps that dependency in cmd/ where the wiring lives, exactly as this package's +// doc comment promises. +type MetaValidator interface { + // Resolve verifies the signature and expiry locally, refuses a token sealed + // by tokens.sr.ht, mirrors the owner's profile and asks meta.sr.ht whether + // the token has been revoked. It answers with core-go's own OAuth2 caller, or + // with one of the metapat package's sentinels. + Resolve(ctx context.Context, presented string) (*auth.AuthContext, error) +} + +// Compile-time proof that the shared validator satisfies the port. It is what +// lets this package depend on the interface rather than on *metapat.Validator, +// and it fails the build the moment either side drifts. +var _ MetaValidator = (*metapat.Validator)(nil) + +// MetaAuth is the meta.sr.ht plane: an ordinary personal access token, the +// credential every upstream service on this instance already accepts. +// +// # Why spec.sr.ht accepts one at all +// +// It did not, and the reason it does now is the gateway. api.sr.ht forwards ONE +// client "Authorization" header to every service a federated query touches — its +// AuthMiddleware copies the client's header verbatim into the request context, +// and the internal credential it can mint is used only to fetch schemas at +// startup — so a federated caller arrives holding whatever credential the client +// had. The only credential that works across the whole instance is a meta PAT, +// so an endpoint that refuses them answers 401 to the first authenticated +// federated query and can never be part of the gateway's schema, however correct +// each individual refusal looks. +// +// # Where it is and is not accepted +// +// This plane is deliberately NOT part of Resolver. It is reachable from /query +// alone, which is the only surface with the federation problem; the REST write +// plane, /mcp and the push hook keep asking for a tokens.sr.ht working token, +// where a narrow, short-lived, revocable grant is worth what it costs an agent to +// obtain. Nothing on those paths holds a MetaAuth, so nothing on them can produce +// a PlaneMeta principal, and a PAT is not a way around a grant. +// +// # What it is left deciding +// +// It owns no crypto and no lookups: verification is ecore's metapat, one copy of +// that check for every service on the instance. Three questions are this +// service's own and are why the type exists at all — whether the token's owner is +// the one human this instance answers to, whether it carries the scope, and which +// failures of the shared validator are a bad credential, which are a missing +// permission, and which are an outage. +type MetaAuth struct { + validator MetaValidator + owner string + scope string +} + +// NewMetaAuth builds the plane over a validator, the instance owner username +// from [sr.ht] owner-name, and the OAuth scope a token must carry to read +// through this service. +// +// Each argument is refused rather than tolerated when it is empty, and the three +// reasons are different: +// +// - A nil validator is not the "this instance has no such plane" configuration. +// That one is a nil *MetaAuth, which graph.New refuses outright because the +// plane needs no per-instance origin and so can never be legitimately absent. +// A validator-less plane would fail every PAT while looking configured. +// - An empty owner would compare every token's account against "", so either no +// PAT would ever be admitted, or — worse, if the comparison were ever loosened +// — the check that makes this a single-owner instance would be the one that +// silently did nothing. NewResolver validates its owner for the same reason +// and with the same rule, so the two planes cannot disagree about who bigbes +// is. +// - An empty scope would have metapat.Allows asked about the grant name "", +// which no token carries and no meta checkbox can mint, so every PAT on the +// instance would be refused with a message naming a permission that does not +// exist. +func NewMetaAuth(validator MetaValidator, owner, scope string) (*MetaAuth, error) { + if validator == nil { + return nil, fmt.Errorf("authn: nil MetaValidator") + } + owner = strings.TrimPrefix(owner, "~") + if err := core.ValidateOwner(owner); err != nil { + return nil, fmt.Errorf("authn: meta plane instance owner: %w", err) + } + if scope == "" { + return nil, fmt.Errorf("authn: empty OAuth scope, e.g. %s", ScopeRead) + } + return &MetaAuth{validator: validator, owner: owner, scope: scope}, nil +} + +// Scope is the OAuth grant this plane requires, for a caller that wants to name +// it in a refusal without spelling it a second time. +func (a *MetaAuth) Scope() string { return a.scope } + +// Owner is the instance owner username this plane admits tokens for. +func (a *MetaAuth) Owner() string { return a.owner } + +// VerifyToken validates a presented personal access token and returns the +// principal of its owner. +// +// presented is the bare credential, with the "Bearer " scheme already stripped. +// +// The principal is KindAgent, never KindOwner, even though the token belongs to +// the instance owner's own meta.sr.ht account. That is the security decision of +// this whole change: KindOwner may approve proposals and manage webhooks, and it +// is reached by a unified-login cookie — a browser session a human is sitting in +// front of — while a PAT is a bearer string any process holding it can present, +// forwarded through a gateway by whatever client asked. Reading one as the owner +// would hand the approved branch to the widest credential on the instance. +// +// It carries no Grants either: those are tokens.sr.ht's vocabulary and a PAT is +// in meta's, which is checked here instead — once, against the scope this plane +// was built with, because the whole surface it guards is a read. That is also why +// Principal.Authorize passes a PlaneMeta caller through: asking it for a grant a +// PAT can never carry would refuse every one of them. +// +// Agent identity and session are left empty, and this plane is given the +// credential rather than the request precisely so that they cannot be otherwise. +// Provenance is demanded at a write (AgentWrite.Validate) and /query performs +// none — its only mutations are the webhook ones, which webhookAuthorized +// restricts to KindOwner — so there is nothing here to attribute. A plane handed +// the request could also read a cookie off it, which would quietly reintroduce +// the ambient authority this endpoint exists without. +// +// The failures are this package's sentinels, so that graph turns them into +// statuses through the same StatusFor table it already uses for the other plane. +func (a *MetaAuth) VerifyToken(ctx context.Context, presented string) (Principal, error) { + if presented == "" { + return Anonymous(), ErrNoToken + } + + ac, err := a.validator.Resolve(ctx, presented) + if err != nil { + return Anonymous(), classifyResolve(err) + } + if ac.UserID == 0 { + // metapat refuses this itself; the check is kept because the seam is an + // interface, and a validator that answered with an empty context must not + // produce a principal that owns whichever row has an unset owner id. + return Anonymous(), fmt.Errorf( + "%w: it resolved to no meta.sr.ht user id", ErrInvalidPersonalToken) + } + + // The token names a meta.sr.ht account, and spec.sr.ht has exactly one that + // means anything. This is the rule resolveInstanceToken applies to a working + // token and the cookie plane applies to a session, and it matters most here: + // a PAT is the credential every account on the instance can mint for itself, + // so without this check the widest credential in existence would be the one + // that skipped the narrowest identity rule, and any user of the instance could + // read the whole corpus through /query. + // + // It is a refusal rather than a downgrade to anonymous because a presented + // credential that fails must fail at the door: the asymmetry this package's + // doc comment draws between cookies and bearer tokens. + username := strings.TrimPrefix(ac.Username, "~") + if username != a.owner { + return Anonymous(), fmt.Errorf( + "%w: the personal access token belongs to ~%s, and this instance answers only to ~%s", + ErrNotInstanceOwner, username, a.owner) + } + + if !metapat.Allows(ac, a.scope, auth.RO) { + return Anonymous(), fmt.Errorf( + "%w: this personal access token does not carry %s", ErrMissingScope, a.scope) + } + + return Principal{ + Kind: KindAgent, + Owner: a.owner, + // Diagnostics only, as on the other plane. There is no row id to print: + // meta.sr.ht does not number a PAT the way tokens.sr.ht numbers a working + // token, and naming the plane is the useful half anyway — it says which of + // two credentials a log line is about. + TokenName: "meta.sr.ht personal access token", + Plane: PlaneMeta, + UserID: ac.UserID, + }, nil +} + +// classifyResolve maps metapat's sentinels onto this package's, and is the one +// place spec.sr.ht decides what each refusal of the shared PAT validator means +// here. +// +// It is deliberately the same shape as the tokens.sr.ht plane's mapping, arm for +// arm — resolveInstanceToken wraps bearer's sentinels and StatusFor reads them — +// because the two planes owe the surfaces above the same three answers, 401, 403 +// and 503. A difference between the tables would be a difference in what a client +// is told about the same kind of failure, decided by which credential it happened +// to be holding. +// +// metapat.ErrNotOurs is classified for totality and is not reachable in +// production: graph routes on metapat.PlaneOf before this plane is asked, so a +// working token has already gone to the other one. Were it ever to arrive here it +// is a bad credential *for this plane*, and 401 is the honest answer. +// +// An unrecognised error is ErrMetaUnavailable, and that is the fail-closed +// direction rather than a shrug: a sentinel this table has never seen must read +// as "I could not decide" — a 503 the caller retries — and never as a verdict +// about the credential. Answering 401 to something this function does not +// understand would tell a client to re-mint a token that may be perfectly good. +func classifyResolve(err error) error { + switch { + case errors.Is(err, metapat.ErrInvalid), + errors.Is(err, metapat.ErrNotOurs), + errors.Is(err, metapat.ErrRevoked): + return fmt.Errorf("%w: %w", ErrInvalidPersonalToken, err) + case errors.Is(err, metapat.ErrForbidden): + // Not reachable through Resolve, which is never told a scope; classified + // so that the table is total. + return fmt.Errorf("%w: %w", ErrMissingScope, err) + case errors.Is(err, metapat.ErrUnavailable): + // meta.sr.ht and not tokens.sr.ht, which is the entire reason this + // sentinel exists beside bearer.ErrUnavailable rather than reusing it. + return fmt.Errorf("%w: %w", ErrMetaUnavailable, err) + default: + return fmt.Errorf("%w: validating a meta.sr.ht personal access token: %w", + ErrMetaUnavailable, err) + } +} diff --git a/authn/meta_test.go b/authn/meta_test.go new file mode 100644 index 0000000000000000000000000000000000000000..ed40830a4fbf6fa93306f8411fb11cbcc1aa71fc --- /dev/null +++ b/authn/meta_test.go @@ -0,0 +1,331 @@ +package authn + +import ( + "context" + "errors" + "net/http" + "testing" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" + "github.com/vaughan0/go-ini" + + "sourcecraft.dev/bigbes/sr-ht-core/auth" + "sourcecraft.dev/bigbes/sr-ht-core/config" + + "sourcecraft.dev/bigbes/sr-ht-ecore/metapat" +) + +// The meta.sr.ht plane as a unit: what it makes of an answer from the shared +// validator, and what each of that validator's refusals means here. +// +// graph/credential_test.go drives the same plane through a real handler with a +// real metapat.Validator behind it. These go in at the seam instead, because the +// arms that matter most — a meta.sr.ht that will not answer, a sentinel nobody +// has written yet — cannot be provoked from the outside without breaking +// something on purpose. + +// --------------------------------------------------------------------------- +// Fixtures +// --------------------------------------------------------------------------- + +// stubPATs is an in-memory MetaValidator: it answers with whatever the test put +// in it, which is how every arm of the classification below is exercised with no +// meta.sr.ht, no network and no database. +type stubPATs struct { + ac *auth.AuthContext + err error + calls int +} + +func (s *stubPATs) Resolve(context.Context, string) (*auth.AuthContext, error) { + s.calls++ + if s.err != nil { + return nil, s.err + } + return s.ac, nil +} + +// resolvedPAT is what metapat.Resolve leaves behind for a live personal access +// token of username carrying grantString: core-go's own OAuth2 caller, with the +// grants decoded in meta's vocabulary. +// +// They are decoded through a context naming this service because +// auth.DecodeGrants reads that name to expand a grant written without one — and +// panics when nothing put it there. That is the trap metapat.Options.Service +// exists to close, met here from the other side. +func resolvedPAT(t *testing.T, username, grantString string) *auth.AuthContext { + t.Helper() + g, err := auth.DecodeGrants( + config.Context(context.Background(), ini.File{}, ConfigSection), grantString) + require.NoError(t, err) + + return &auth.AuthContext{ + AuthMethod: auth.AUTH_OAUTH2, + UserID: 1, + Username: username, + Email: username + "@example.org", + BearerToken: &auth.BearerToken{Username: username, Grants: grantString}, + Grants: g, + } +} + +// newMetaAuth builds the plane over a validator that answers every credential +// the same way, for the instance owner these tests use throughout. +func newMetaAuth(t *testing.T, ac *auth.AuthContext, resolveErr error) (*MetaAuth, *stubPATs) { + t.Helper() + stub := &stubPATs{ac: ac, err: resolveErr} + plane, err := NewMetaAuth(stub, "bigbes", ScopeRead) + require.NoError(t, err) + return plane, stub +} + +// errUnknownToTheTable is a refusal this package's classification has never seen. +// It stands for the sentinel a future version of metapat adds without telling +// anybody, which is the case the fail-closed default exists for. +var errUnknownToTheTable = errors.New("metapat: something nobody has classified") + +// --------------------------------------------------------------------------- +// Construction +// --------------------------------------------------------------------------- + +func TestNewMetaAuthRefusesAPlaneThatCannotWork(t *testing.T) { + t.Run("a nil validator", func(t *testing.T) { + // Not the "this instance has no such plane" configuration — that one is a + // nil *MetaAuth, and graph.New refuses even that — but a plane that would + // fail every PAT while looking configured. + _, err := NewMetaAuth(nil, "bigbes", ScopeRead) + require.Error(t, err) + assert.Contains(t, err.Error(), "MetaValidator") + }) + + t.Run("an empty owner", func(t *testing.T) { + // Every token's account would be compared against "", so the check that + // makes this a single-owner instance would be the one silently doing + // nothing. + _, err := NewMetaAuth(&stubPATs{}, "", ScopeRead) + require.Error(t, err) + assert.Contains(t, err.Error(), "owner") + }) + + t.Run("an owner that is not a usable name", func(t *testing.T) { + // The same rule NewResolver applies, so the two planes cannot disagree + // about who the instance owner is. + _, err := NewMetaAuth(&stubPATs{}, "not a username", ScopeRead) + require.Error(t, err) + }) + + t.Run("an empty scope", func(t *testing.T) { + // metapat.Allows would be asked about the grant name "", which no token + // carries and no meta checkbox can mint: every PAT on the instance + // refused, with a message naming a permission that does not exist. + _, err := NewMetaAuth(&stubPATs{}, "bigbes", "") + require.Error(t, err) + assert.Contains(t, err.Error(), "scope") + }) +} + +func TestTheOwnerAndScopeAreSpelledOnceAndReadBack(t *testing.T) { + plane, _ := newMetaAuth(t, nil, nil) + assert.Equal(t, ScopeRead, plane.Scope()) + assert.Equal(t, "bigbes", plane.Owner()) + + // The leading '~' is a display convention, not part of the name, and both + // planes strip it — a resolver built with "~bigbes" and one built with + // "bigbes" must admit the same tokens. + tilde, err := NewMetaAuth(&stubPATs{}, "~bigbes", ScopeRead) + require.NoError(t, err) + assert.Equal(t, "bigbes", tilde.Owner()) + + // The two halves the instance has to agree on: what meta prefixes into a + // checkbox, and what this service checks. + assert.Equal(t, ScopeRead, metapat.Scope(ConfigSection, metapat.ScopeName(ScopeRead))) +} + +// --------------------------------------------------------------------------- +// VerifyToken +// --------------------------------------------------------------------------- + +func TestAPersonalAccessTokenCarryingTheScopeIsAnAgent(t *testing.T) { + plane, stub := newMetaAuth(t, resolvedPAT(t, "bigbes", ScopeRead+":RO"), nil) + + p, err := plane.VerifyToken(t.Context(), "presented") + require.NoError(t, err) + + assert.Equal(t, 1, stub.calls) + assert.Equal(t, KindAgent, p.Kind) + assert.Equal(t, PlaneMeta, p.Plane) + assert.Equal(t, "bigbes", p.Owner) + assert.Equal(t, 1, p.UserID, "the row id every user-scoped lookup keys off") + assert.True(t, p.CanRead()) + + // It may read and it may not approve: KindOwner is the human at a browser, + // and a PAT is a string any process holding it can present. + assert.False(t, p.IsOwner()) + + // It carries no tokens.sr.ht grants, and must not be asked for any — no PAT + // can ever be minted with spec:read, so a grant check here would refuse every + // one of them rather than scope it. + assert.NoError(t, p.Authorize(ActionRead)) + assert.NoError(t, p.Authorize(ActionPropose)) +} + +func TestAnUngrantedPersonalAccessTokenIsUniversal(t *testing.T) { + // meta.sr.ht mints a token with no grants selected and core-go reads that as + // every permission (auth.Grants.HasAll). Refusing it here would refuse the + // commonest credential on the instance — and the one a federated query is + // most likely to arrive holding. + plane, _ := newMetaAuth(t, resolvedPAT(t, "bigbes", ""), nil) + + p, err := plane.VerifyToken(t.Context(), "presented") + require.NoError(t, err) + assert.Equal(t, PlaneMeta, p.Plane) +} + +func TestAPersonalAccessTokenWithoutTheScopeIsForbidden(t *testing.T) { + plane, _ := newMetaAuth(t, resolvedPAT(t, "bigbes", "meta.sr.ht/PROFILE:RO"), nil) + + p, err := plane.VerifyToken(t.Context(), "presented") + require.ErrorIs(t, err, ErrMissingScope) + assert.Equal(t, http.StatusForbidden, StatusFor(err), + "403: the credential is fine, the permission is not") + assert.False(t, IsAuthFailure(err), "401 would send the holder re-minting a good token") + assert.True(t, p.IsAnonymous()) + assert.Contains(t, err.Error(), ScopeRead, "the refusal must name what to go and obtain") +} + +func TestAPersonalAccessTokenOfAnotherOwnerIsForbidden(t *testing.T) { + // The single-owner rule, applied to the credential every account on the + // instance can mint for itself. Without it the widest credential in existence + // would be the one that skipped the narrowest identity check. + // + // 403 and not 401, and the same sentinel a foreign working token gets: the + // token verifies and the holder is who they say they are, there is simply + // nothing on this instance to grant them, so retrying will not help. + plane, _ := newMetaAuth(t, resolvedPAT(t, "somebody-else", ScopeRead), nil) + + p, err := plane.VerifyToken(t.Context(), "presented") + require.ErrorIs(t, err, ErrNotInstanceOwner) + assert.Equal(t, http.StatusForbidden, StatusFor(err)) + assert.True(t, p.IsAnonymous()) + + // The refusal is about whose token it is and not about what it carries: this + // one holds the scope and is refused anyway, because a scope says what a + // token may do and never whose it is. + assert.NotErrorIs(t, err, ErrMissingScope) +} + +func TestTheOwnerIsRecognisedThroughATildeInTheToken(t *testing.T) { + // meta.sr.ht writes usernames both ways depending on the call, and the other + // plane already strips one leading '~' before comparing. A plane that did not + // would refuse the instance owner's own token half the time. + plane, _ := newMetaAuth(t, resolvedPAT(t, "~bigbes", ScopeRead), nil) + + p, err := plane.VerifyToken(t.Context(), "presented") + require.NoError(t, err) + assert.Equal(t, "bigbes", p.Owner) +} + +func TestAnAbsentCredentialIsNotARefusal(t *testing.T) { + // ErrNoToken is what the caller reads as "nothing was presented" rather than + // as a rejection, and nothing is asked of meta.sr.ht for it. + plane, stub := newMetaAuth(t, nil, nil) + + _, err := plane.VerifyToken(t.Context(), "") + require.ErrorIs(t, err, ErrNoToken) + assert.Zero(t, stub.calls, "nothing to validate, so nothing is asked") +} + +func TestAResolvedCallerWithNoIDIsRefused(t *testing.T) { + // metapat refuses this itself; the check is kept because the seam is an + // interface, and a zero id downstream would own whichever row has an unset + // owner. + ac := resolvedPAT(t, "bigbes", ScopeRead) + ac.UserID = 0 + plane, _ := newMetaAuth(t, ac, nil) + + _, err := plane.VerifyToken(t.Context(), "presented") + require.ErrorIs(t, err, ErrInvalidPersonalToken) + assert.True(t, IsAuthFailure(err)) + assert.Equal(t, http.StatusUnauthorized, StatusFor(err)) +} + +// --------------------------------------------------------------------------- +// classifyResolve — the table the surfaces turn into statuses +// --------------------------------------------------------------------------- + +func TestEveryRefusalOfTheSharedValidatorIsClassified(t *testing.T) { + cases := []struct { + name string + from error + want error + status int + }{ + { + name: "a forged or expired token", from: metapat.ErrInvalid, + want: ErrInvalidPersonalToken, status: http.StatusUnauthorized, + }, + { + name: "a revoked token", from: metapat.ErrRevoked, + want: ErrInvalidPersonalToken, status: http.StatusUnauthorized, + }, + { + // Unreachable in production — graph routes on metapat.PlaneOf before + // this plane is asked — and classified so the table is total. Were it + // ever to arrive it is a bad credential *for this plane*. + name: "a working token that reached the wrong plane", from: metapat.ErrNotOurs, + want: ErrInvalidPersonalToken, status: http.StatusUnauthorized, + }, + { + // Resolve is never told a scope, so it cannot raise this; classified + // so the table is total. + name: "a scope refusal from the validator itself", from: metapat.ErrForbidden, + want: ErrMissingScope, status: http.StatusForbidden, + }, + { + // The arm that has to be defended: "I could not check" is not "your + // credential is bad", and 401 here would tell every federated client + // on the instance to re-mint over a meta.sr.ht restart. + name: "meta.sr.ht unreachable", from: metapat.ErrUnavailable, + want: ErrMetaUnavailable, status: http.StatusServiceUnavailable, + }, + { + // Fail-closed: a sentinel this table has never seen must read as "I + // could not decide", never as a verdict about the credential. + name: "a sentinel nobody has classified yet", from: errUnknownToTheTable, + want: ErrMetaUnavailable, status: http.StatusServiceUnavailable, + }, + } + + for _, tc := range cases { + t.Run(tc.name, func(t *testing.T) { + plane, _ := newMetaAuth(t, nil, tc.from) + + p, err := plane.VerifyToken(t.Context(), "presented") + require.ErrorIs(t, err, tc.want) + assert.ErrorIs(t, err, tc.from, "the cause must survive for the log") + assert.Equal(t, tc.status, StatusFor(err)) + assert.True(t, p.IsAnonymous(), "a refused caller carries no authority") + }) + } +} + +// The two planes owe the surfaces above the same three answers, so their tables +// must not drift apart. This asserts the property rather than the code: every +// refusal this plane can produce lands on exactly one of 401, 403 and 503, and +// never on 200. +func TestTheClassificationIsTotalAndUnambiguous(t *testing.T) { + for _, from := range []error{ + metapat.ErrInvalid, metapat.ErrNotOurs, metapat.ErrRevoked, + metapat.ErrForbidden, metapat.ErrUnavailable, errUnknownToTheTable, + } { + plane, _ := newMetaAuth(t, nil, from) + _, err := plane.VerifyToken(t.Context(), "presented") + require.Error(t, err) + + status := StatusFor(err) + assert.Contains(t, + []int{http.StatusUnauthorized, http.StatusForbidden, http.StatusServiceUnavailable}, + status, "%v mapped to %d", from, status) + } +} diff --git a/authn/principal.go b/authn/principal.go index a813f7eadf6836531527d32f0f94525c5df17df9..2bdf5091e10846de0e5b3ba11c306268bc7cd9c1 100644 --- a/authn/principal.go +++ b/authn/principal.go @@ -33,10 +33,12 @@ ) // Plane names the credential plane an agent authenticated on. // -// One plane is left, and the field outlives its sibling because the distinction -// it draws is no longer "which of two stores said yes" but "was there a -// credential at all". Only a credential carries a grant set, and a check that -// reads Grants has to know whether there were any to read. +// What the field draws is not "which of two stores said yes" but "in which +// vocabulary, if any, was this credential scoped". Only PlaneInstance carries a +// tokens.sr.ht grant set, so a check that reads Grants has to know whether there +// were any to read — and reading the zero set of a plane that was never scoped +// in that grammar would refuse a caller for lacking a permission its credential +// could not have been minted with. // // Empty for every principal that is not an agent, and for the one agent that is // not credential-backed: `specsrht doc propose`, which runs as the operator on @@ -49,6 +51,26 @@ const ( // PlaneInstance is a tokens.sr.ht working token: signed, expiring, owned by // a meta.sr.ht account, and carrying the grant set Authorize checks. PlaneInstance Plane = "instance" + + // PlaneMeta is a meta.sr.ht personal access token, scoped in meta's own OAuth + // vocabulary (ScopeRead) rather than in tokens.sr.ht's, and reachable on + // /query alone. + // + // It exists because api.sr.ht forwards one client credential to every service + // a federated query touches, so a /query that refused the credential the rest + // of the instance uses could never be federated. MetaAuth's own comment + // carries the argument; what matters here is the scope of the exception — + // nothing on the REST, MCP or push paths can produce this plane, because none + // of them holds a MetaAuth, so a PAT is not a way around the grants those + // surfaces require. + // + // A principal on this plane is KindAgent and never KindOwner, even though the + // token belongs to the instance owner's own meta account. KindOwner is the + // human at a browser, and it is the only principal that may approve a proposal + // or manage a webhook; promoting a credential that any process holding a + // string can present into that role would hand the approved branch to whatever + // is holding it. + PlaneMeta Plane = "meta" ) // Principal is the resolved identity of a request. It is a value type with no @@ -145,12 +167,23 @@ // instance token and so has no grant to be missing. The two questions are // separate on purpose: the resolver answers identity in middleware, upstream of // the router, and only the layer that knows the action can ask this one. // -// A principal off the instance plane passes. That is not a hole left over from -// the agent_token days: grants describe machine credentials, and the principals -// with no plane are the owner's cookie — a person, whose authority is their -// identity — and the CLI's locally asserted agent, which runs as the operator on -// the daemon's host and presented nothing to have a grant clipped out of. Every -// agent the resolver produces is on the instance plane and is checked here. +// A principal off the instance plane passes, and each of the three ways that +// happens is deliberate rather than a hole left over from the agent_token days: +// +// - The owner's cookie is a person, whose authority is their identity. There is +// no grant to read, and checking a zero set would refuse every logged-in +// human on the site. +// - The CLI's locally asserted agent runs as the operator on the daemon's own +// host and presented nothing to have a grant clipped out of. +// - A meta.sr.ht personal access token (PlaneMeta) is scoped in a vocabulary +// this method does not speak. No PAT can ever carry "spec:read" — meta's +// personal-token page cannot spell it — so checking one here would refuse +// every PAT on the instance rather than scope it, which is the opposite of +// what a grant check is for. A PAT is scoped once, in meta's own grammar, at +// the point it is resolved (MetaAuth.VerifyToken), and /query's read gate is +// the only surface it can reach at all. +// +// Every agent the resolver produces is on the instance plane and is checked here. func (p Principal) Authorize(action string) error { if p.Plane != PlaneInstance { return nil @@ -179,11 +212,19 @@ if session == "" { session = "(no session)" } line := fmt.Sprintf("agent %s session %s for ~%s", agent, session, p.Owner) - // Only a credential-backed agent is annotated: the grant set is what the - // annotation says, and an agent a local process asserted has none to - // print. - if p.Plane == PlaneInstance { + // Each credential-backed agent is annotated with the plane that admitted + // it, and only the instance plane's annotation carries a grant set: that + // set is what its annotation says, and neither a PAT nor an agent a local + // process asserted has one to print. Naming the plane is what lets a log + // line distinguish the two credentials afterwards, which is the whole + // reason a reader would look — a PAT reaches /query and nothing else, so + // "which plane" is also "which surface" when one turns up somewhere + // surprising. + switch p.Plane { + case PlaneInstance: line += " (tokens.sr.ht: " + p.Grants.String() + ")" + case PlaneMeta: + line += " (meta.sr.ht personal access token)" } return line default: diff --git a/cmd/specsrht/graphql.go b/cmd/specsrht/graphql.go index 548dd97ca40ff60a19dfae37b7b67b3c89999d47..8e6135c866030d8d387c313e88138f57116a0443 100644 --- a/cmd/specsrht/graphql.go +++ b/cmd/specsrht/graphql.go @@ -1,9 +1,15 @@ package main import ( + "fmt" "strconv" "github.com/vaughan0/go-ini" + + "sourcecraft.dev/bigbes/sr-ht-ecore/metapat" + + "sourcecraft.dev/bigbes/sr-ht-spec/authn" + "sourcecraft.dev/bigbes/sr-ht-spec/graph" ) // queryRoute is where the GraphQL schema answers. It is core-go's own path, @@ -13,18 +19,64 @@ // sr-ht-ecore's apimeta, at apimeta.Path, because a service that mounts its own // /query is the one thing core-go does not serve that file for. const queryRoute = "/query" -// apiScopes is what this service publishes at apimeta.Path: nothing. +// apiScopes is what this service publishes at apimeta.Path: the scopes a +// meta.sr.ht personal access token can be minted for here. // // A scope is the part after the service name in a meta.sr.ht personal-token -// grant, and spec.sr.ht defines none — no AccessScope enum, no @access directive -// on any field, and no code path that reads one. Its grant vocabulary is -// tokens.sr.ht's (authn.ActionRead, authn.ActionPropose), which meta neither -// mints nor advertises, so the honest list is empty and not a placeholder. +// grant, and meta builds the checkboxes of /oauth2/personal-token by fetching +// this file from every service it discovers and prefixing each entry with that +// service's own name. So "SPECS" here is what makes "spec.sr.ht/SPECS:RO" a token +// a human can actually obtain. +// +// It published nothing until /query started accepting a PAT, and that empty list +// was not merely an omission. It meant no PAT could be scoped for this service +// even in principle, so the credential api.sr.ht forwards to every service a +// federated query touches could never have been presented here — whatever the +// endpoint's own code said about it. // -// It is a variable so that what the daemon serves and what its test asserts are -// one value rather than two spellings of an intention. apimeta marshals it as [] -// and never as null; see the test for why that distinction is instance-wide. -var apiScopes []string +// It is graph.GrantScopes and not a literal, so that what the daemon serves is +// derived from the same authn.ScopeRead the endpoint checks rather than being a +// second spelling of it. apimeta marshals it as [] and never as null; see the +// test for why that distinction is instance-wide. +var apiScopes = graph.GrantScopes + +// newMetaPlane builds the meta.sr.ht personal access token plane /query accepts +// beside the tokens.sr.ht working token every other surface takes. +// +// It is built here, at the single call site that may use it, and deliberately not +// on the *authn.Resolver the daemon hands to every surface. Putting it there +// would give it to the REST write plane, /mcp and the push hook as well, which is +// precisely what must not happen: those require a tokens.sr.ht grant, and a +// personal access token — the credential every account on the instance can mint +// for itself, in a vocabulary that cannot spell spec:propose — must not be a way +// around one. Building it at one call site makes the scope of the exception +// something a reader can see rather than something they have to trust. +// +// owner is the same [sr.ht] owner-name the resolver was built with, because the +// single-owner rule belongs to the service and not to one plane: a PAT belonging +// to anybody else is refused exactly as a working token of theirs is. +// +// The validator is metapat's default backend, which is core-go — it mirrors the +// owner's profile and asks meta.sr.ht about revocation through the database and +// config in the request context. /query has both: mountGraphQL puts it in a Group +// carrying core-go's config and database middleware, which the webhook resolvers +// already needed. +// +// Unlike the working-token plane this one needs no configuration and can never be +// legitimately absent: it depends on no per-instance origin. So a failure here is +// fatal to startup rather than a degraded mode — an instance that came up without +// it would answer 401 to every federated query while looking configured. +func newMetaPlane(owner string) (*authn.MetaAuth, error) { + pats, err := metapat.New(metapat.Options{Service: authn.ConfigSection}) + if err != nil { + return nil, fmt.Errorf("assemble the meta.sr.ht token plane: %w", err) + } + plane, err := authn.NewMetaAuth(pats, owner, authn.ScopeRead) + if err != nil { + return nil, fmt.Errorf("assemble the meta.sr.ht token plane: %w", err) + } + return plane, nil +} // defaultMaxComplexity is the bound core-go's server.WithSchema would have // applied. It is repeated here because this daemon does not call WithSchema — diff --git a/cmd/specsrht/graphql_test.go b/cmd/specsrht/graphql_test.go index be5678c9638a57acabed6b75df6684e96ff281cd..a85601ab5194a9274c0bbe2b264052b64e92bd24 100644 --- a/cmd/specsrht/graphql_test.go +++ b/cmd/specsrht/graphql_test.go @@ -18,6 +18,9 @@ "sourcecraft.dev/bigbes/sr-ht-core/config" "sourcecraft.dev/bigbes/sr-ht-core/crypto" "sourcecraft.dev/bigbes/sr-ht-core/database" "sourcecraft.dev/bigbes/sr-ht-ecore/apimeta" + + "sourcecraft.dev/bigbes/sr-ht-spec/authn" + "sourcecraft.dev/bigbes/sr-ht-spec/graph" ) // TestMain initialises the crypto globals apimeta.Handler reads the webhook @@ -38,33 +41,45 @@ assert.Equal(t, "/query", queryRoute) assert.Equal(t, queryRoute+"/api-meta.json", apimeta.Path) } -// api-meta.json must be served, and its scope list must be an empty array and -// never a JSON null. +// api-meta.json must be served, it must publish exactly the scope the endpoint +// checks, and that list must be a JSON array and never a null. // // meta.sr.ht fetches this file from every service it discovers when it renders // /oauth2/personal-token, and iterates the "scopes" field to build the grant -// checkboxes. A null there is a nil iteration in meta — a 500 on that page for -// the WHOLE instance, every service's grants and not just this one's. It is a -// failure nobody would find by testing the service that caused it, which is why -// the assertion lives here even though the marshalling is sr-ht-ecore's. +// checkboxes. Two different failures live in that one field: // -// Empty is also the honest answer for spec.sr.ht rather than a placeholder: this -// service defines no meta OAuth scope and no @access directive to check one -// against. Its grant vocabulary is tokens.sr.ht's — authn.ActionRead and -// authn.ActionPropose — which meta neither mints nor advertises. -func TestAPIMetaAdvertisesNoScopeAndNeverNull(t *testing.T) { +// - A null is a nil iteration in meta — a 500 on that page for the WHOLE +// instance, every service's grants and not just this one's. It is a failure +// nobody would find by testing the service that caused it, which is why the +// assertion lives here even though the marshalling is sr-ht-ecore's. +// - An empty list means no personal access token can be scoped for this service +// at all, so /query could never be federated whatever its own code said: the +// credential api.sr.ht forwards to every service a query touches would be one +// no checkbox could mint. That was spec.sr.ht's state until the endpoint grew +// the meta plane, and it is the half of the refusal no amount of code in +// graph/ could have worked around. +// +// The published scope is compared against graph.GrantScopes rather than a +// literal, because the two spellings that must agree are what meta turns into a +// checkbox and what the endpoint checks. A scope published and not checked admits +// what should have been refused; one checked and not published cannot be minted +// at all; neither is visible from inside a single file. The wire spelling is +// asserted as well, since meta reads the bytes and not the Go value. +func TestAPIMetaPublishesTheScopeTheEndpointChecks(t *testing.T) { rec := httptest.NewRecorder() // apiScopes and not a literal: this asserts what mountWeb actually serves. apimeta.Handler(apiScopes...).ServeHTTP(rec, httptest.NewRequest(http.MethodGet, apimeta.Path, nil)) require.Equal(t, http.StatusOK, rec.Code) - assert.Contains(t, rec.Body.String(), `"scopes":[]`, - "a JSON null here is a 500 on meta's personal-token page for the whole instance") + assert.Contains(t, rec.Body.String(), `"scopes":["SPECS"]`, + "the wire spelling meta.sr.ht reads, and prefixes into spec.sr.ht/SPECS") var got apimeta.Meta require.NoError(t, json.Unmarshal(rec.Body.Bytes(), &got)) - assert.NotNil(t, got.Scopes) - assert.Empty(t, got.Scopes) + require.NotNil(t, got.Scopes, + "a JSON null here is a 500 on meta's personal-token page for the whole instance") + assert.Equal(t, graph.GrantScopes, got.Scopes, + "the file must publish exactly what the endpoint checks") assert.NotEmpty(t, got.WebhookPubkey, "a webhook consumer verifies payloads with this") } @@ -128,9 +143,35 @@ require.Equal(t, http.StatusOK, resp.StatusCode) var got apimeta.Meta require.NoError(t, json.NewDecoder(resp.Body).Decode(&got)) - assert.NotNil(t, got.Scopes) - assert.Empty(t, got.Scopes) + require.NotNil(t, got.Scopes) + assert.Equal(t, graph.GrantScopes, got.Scopes, + "what meta.sr.ht mints a token for is what /query checks") }) +} + +// The meta.sr.ht plane the daemon hands to /query and to nothing else. +// +// What is worth asserting here is not that it builds — graph.New already refuses +// a nil one — but what it is built *with*: the instance owner, so a personal +// access token belonging to anybody else is refused exactly as a foreign working +// token is, and the scope api-meta.json publishes, so the checkbox meta renders +// is the permission this endpoint checks. +func TestNewMetaPlane(t *testing.T) { + plane, err := newMetaPlane("bigbes") + require.NoError(t, err) + require.NotNil(t, plane) + + assert.Equal(t, "bigbes", plane.Owner(), + "a PAT of any other account must be refused, and a PAT is what every "+ + "account on the instance can mint for itself") + assert.Equal(t, authn.ScopeRead, plane.Scope()) + assert.Equal(t, authn.ScopeRead, + authn.ConfigSection+"/"+apiScopes[0], "the published half and the checked half") + + // A bad owner is a startup failure and not a plane that admits nobody: the + // daemon must say so while somebody is watching it start. + _, err = newMetaPlane("") + require.Error(t, err) } // The complexity bound core-go's WithSchema would have set. The daemon does not diff --git a/cmd/specsrht/main.go b/cmd/specsrht/main.go index 9b3f2b5eb1265a8eeb23b4e75856cd37f9457f66..60587417aed90d94a7f7c62d0c17d3f0c60daf51 100644 --- a/cmd/specsrht/main.go +++ b/cmd/specsrht/main.go @@ -292,11 +292,17 @@ return err } defer pool.Close() - // WithInstanceTokens builds the daemon's one agent credential plane. It is - // required, not offered: spec.sr.ht mints no credential of its own any more, - // so an instance with no [tokens.sr.ht] section could authenticate no agent - // at all, over HTTP or over `git push`. service.New fails here rather than - // letting the daemon come up and refuse every agent one request at a time. + // WithInstanceTokens builds the agent credential plane every surface shares. + // It is required, not offered: spec.sr.ht mints no credential of its own any + // more, so an instance with no [tokens.sr.ht] section could authenticate no + // agent at all, over HTTP or over `git push`. service.New fails here rather + // than letting the daemon come up and refuse every agent one request at a + // time. + // + // /query holds a second plane the resolver knows nothing about — the + // meta.sr.ht personal access token newMetaPlane builds — and that one is + // deliberately not here, because a plane on the resolver is a plane on every + // surface. It is built beside the graph.New call in newSurfaces instead. svc, err := service.New(cfg, pool, service.WithInstanceTokens(conf)) if err != nil { return err @@ -356,11 +362,13 @@ // the email queue that WithQueues hands the webhook delivery worker — not // for the authenticated router it decorates, which now carries no routes at // all. There is deliberately no WithSchema: it would mount /query on that // authenticated router, behind core-go's auth.Middleware, which speaks - // meta.sr.ht's OAuth vocabulary and not the tokens.sr.ht one every other - // surface of this service accepts. /query is mounted on the anonymous - // router by mountRoutes instead, with graph's own credential middleware in - // front of it, and api-meta.json is served there too because core-go serves - // that file only for the schemas it hosts itself. + // meta.sr.ht's OAuth vocabulary and 401s the tokens.sr.ht one every other + // surface of this service accepts. /query takes both — a working token and a + // meta PAT — and serving two planes from one endpoint is exactly what that + // middleware cannot do. It is mounted on the anonymous router by mountRoutes + // instead, with graph's own credential middleware in front of it, and + // api-meta.json is served there too because core-go serves that file only for + // the schemas it hosts itself. // // MaxComplexity is the one thing WithSchema set that still has to be set, // and its reader has nothing to do with serving /query: the webhook delivery @@ -600,15 +608,30 @@ // /query and the web UI apply. spec_propose stays fail-closed in service/ too; // the gate just makes the read tools match. mcp = svc.Resolver().Middleware()(mcpsrv.Gate(mcp)) - // /query, with the same credential plane /mcp and /api use — the resolver is - // the one svc holds, so a token that reads through one surface reads through - // all three. graph.Server installs that middleware itself, which is why it is - // mounted on the anonymous router below and not through core-go's WithSchema. + // The one credential plane that is not shared. /query is federated into + // api.sr.ht, which forwards a single client Authorization header to every + // service a query touches, so it has to take the meta.sr.ht personal access + // token that header carries; the three surfaces above must not, because a PAT + // would then be a way around the tokens.sr.ht grant they require. Building it + // beside the one endpoint that gets it is what keeps that true — see + // newMetaPlane. + metaPlane, err := newMetaPlane(svc.Resolver().Owner()) + if err != nil { + index.Close() + return nil, err + } + + // /query, with the credential plane /mcp and /api use — the resolver is the + // one svc holds, so a token that reads through one surface reads through all + // three — plus the meta plane above, which this surface alone holds. + // graph.Server installs that middleware itself, which is why it is mounted on + // the anonymous router below and not through core-go's WithSchema. gql, err := graph.New(graph.Options{ Reader: svc, Searcher: index, Proposals: graph.NewProposals(svc), Resolver: svc.Resolver(), + Meta: metaPlane, }) if err != nil { index.Close() diff --git a/go.mod b/go.mod index e01e6256cac4615f6e0308c75d1dee9885e65695..8061cfa5d877d2df44e331c39260706de61a1897 100644 --- a/go.mod +++ b/go.mod @@ -22,7 +22,7 @@ github.com/yuin/goldmark v1.8.2 go.bigb.es/auxilia v0.7.0 gopkg.in/yaml.v3 v3.0.1 sourcecraft.dev/bigbes/sr-ht-core v0.0.0-20260816094344-effb7ced05b7 - sourcecraft.dev/bigbes/sr-ht-ecore v0.0.0-20260816081411-3bd158fbb232 + sourcecraft.dev/bigbes/sr-ht-ecore v0.0.0-20260816184219-89fa694cbf54 ) require ( diff --git a/go.sum b/go.sum index 558af70aa9dbe7b638058a4a434cbc3a8dbdd92f..99a0c87eeccfa666fb16e041762805043a167ca8 100644 --- a/go.sum +++ b/go.sum @@ -423,3 +423,5 @@ sourcecraft.dev/bigbes/sr-ht-core v0.0.0-20260816094344-effb7ced05b7 h1:YpwRaM3MSFqaujYVY3JVQWCTTU96qwqexo/rGYGHu5Q= sourcecraft.dev/bigbes/sr-ht-core v0.0.0-20260816094344-effb7ced05b7/go.mod h1:Mu1Vx39ws/OTKWGoVERXvkdRSPLBdhuFTYv0ftVV31c= sourcecraft.dev/bigbes/sr-ht-ecore v0.0.0-20260816081411-3bd158fbb232 h1:vfuiF+4BwTFyLOaxuuIGdyL22is6RN/7yF/CV35frTQ= sourcecraft.dev/bigbes/sr-ht-ecore v0.0.0-20260816081411-3bd158fbb232/go.mod h1:Hu5gSbJ9ZQup0KGfupXRxeako5ajV86zB6fC2DbleWY= +sourcecraft.dev/bigbes/sr-ht-ecore v0.0.0-20260816184219-89fa694cbf54 h1:odN8rYDbV7ieJmYbULr1+9v6yvAmwpXY54Sk2ydEhkM= +sourcecraft.dev/bigbes/sr-ht-ecore v0.0.0-20260816184219-89fa694cbf54/go.mod h1:Hu5gSbJ9ZQup0KGfupXRxeako5ajV86zB6fC2DbleWY= diff --git a/graph/credential_test.go b/graph/credential_test.go index 5cc0698441768dd3bd3431ea3fec9bc28d2f6c7a..c1532efa67016fe003dc2c01a9e90316a927c4ef 100644 --- a/graph/credential_test.go +++ b/graph/credential_test.go @@ -2,6 +2,7 @@ package graph import ( "bytes" + "context" "encoding/json" "net/http" "net/http/httptest" @@ -13,6 +14,7 @@ "github.com/stretchr/testify/require" "sourcecraft.dev/bigbes/sr-ht-core/auth" "sourcecraft.dev/bigbes/sr-ht-ecore/bearer" + "sourcecraft.dev/bigbes/sr-ht-ecore/metapat" "sourcecraft.dev/bigbes/sr-ht-spec/authn" ) @@ -43,22 +45,76 @@ } return req, httptest.NewRecorder() } -// metaPAT mints a bearer token sealed with the instance's key but stamped with -// an OAuth client id rather than tokens.sr.ht's. That is exactly what a -// meta.sr.ht personal access token is on this instance — same format, same key, -// different issuer — and the client id is the only thing that tells the two -// apart (see sr-ht-ecore/bearer, step 2). -func metaPAT() string { +// personalToken mints a bearer token sealed with the instance's key but stamped +// with an OAuth client id rather than tokens.sr.ht's, carrying grantString in +// meta.sr.ht's own vocabulary — "spec.sr.ht/SPECS:RO", or "" for the ungranted +// token meta treats as universal. +// +// That is exactly what a meta.sr.ht personal access token is on this instance: +// same format, same key, different issuer. The client id is the ONLY thing that +// tells the two planes apart (see sr-ht-ecore/bearer, step 2), which is why +// routing can be one local decode and why this fixture needs nothing else to be +// a convincing PAT. +func personalToken(username, grantString string) string { bt := &auth.BearerToken{ Version: auth.TokenVersion, Expires: auth.ToTimestamp(time.Now().Add(time.Hour)), - Grants: "", + Grants: grantString, + ClientID: "00000000-0000-0000-0000-00000000beef", + Username: username, + } + return bt.Encode() +} + +// expiredPersonalToken is a PAT whose expiry is already in the past. It is worth +// a fixture of its own because of where it lands: auth.DecodeBearerToken checks +// expiry before it reports an issuer, so metapat.PlaneOf answers PlaneUnknown and +// this never reaches the meta plane at all — see resolveCaller's routing table. +func expiredPersonalToken(username string) string { + bt := &auth.BearerToken{ + Version: auth.TokenVersion, + Expires: auth.ToTimestamp(time.Now().Add(-time.Hour)), ClientID: "00000000-0000-0000-0000-00000000beef", - Username: "bigbes", + Username: username, } return bt.Encode() } +// patBackend is metapat's port over the same fixed user stubUsers answers the +// working-token plane with, so both planes resolve one owner to one row and a +// test that compares them is comparing planes rather than two fixtures. +// +// It answers "not revoked" always. Revocation is metapat's own contract and is +// tested there against a backend that says otherwise; what this suite is about is +// which plane a credential reaches and what it may then read. +type patBackend struct{} + +func (patBackend) LookupUser(_ context.Context, username string, out *auth.AuthContext) error { + out.UserID = 1 + out.Username = username + return nil +} + +func (patBackend) IsRevoked(context.Context, string, [64]byte, string) (bool, error) { + return false, nil +} + +// testMetaAuth is the production meta.sr.ht plane over ecore's real +// metapat.Validator. Nothing in the signature check, the grant decoding or the +// scope comparison is faked — only the profile mirror behind them, which is the +// one step that would need a meta.sr.ht. +func testMetaAuth(t *testing.T) *authn.MetaAuth { + t.Helper() + pats, err := metapat.New(metapat.Options{ + Service: authn.ConfigSection, + Backend: patBackend{}, + }) + require.NoError(t, err) + plane, err := authn.NewMetaAuth(pats, "bigbes", authn.ScopeRead) + require.NoError(t, err) + return plane +} + // foreignToken mints a working token belonging to somebody who is not the // instance owner. func foreignToken(grantString string) string { @@ -137,7 +193,10 @@ }) t.Run("under a router that does resolve the cookie", func(t *testing.T) { resolver := testResolver(t) - srv, err := New(Options{Reader: newFakeReader(), Searcher: &fakeSearcher{}, Resolver: resolver}) + srv, err := New(Options{ + Reader: newFakeReader(), Searcher: &fakeSearcher{}, + Resolver: resolver, Meta: testMetaAuth(t), + }) require.NoError(t, err) // The cookie plane, installed above the endpoint. It resolves the @@ -150,30 +209,131 @@ assert.NotContains(t, r.body, "~bigbes/rfcs", "the refusal leaked content") }) } -// A meta.sr.ht personal access token is refused, and this is a deliberate -// difference from dolt.sr.ht rather than an oversight. -// -// spec.sr.ht authenticates through one issuer (authn's package comment) and -// publishes no OAuth scope for meta to grant against — cmd/specsrht serves -// api-meta.json with an empty scope list — so there is nothing a PAT could be -// scoped *for* here. bearer classifies it ErrNotOurs and authn calls that -// permanent, hence 401 with the challenge rather than a 503. +// The other plane, and the reason this endpoint has two. A federated query +// arrives from api.sr.ht carrying the client's own meta.sr.ht token, because the +// gateway forwards one Authorization header to every service it touches — so a +// personal access token has to work here, scoped in meta's vocabulary rather than +// in tokens.sr.ht's. +func TestTheReadScopeIsRequiredOfAPersonalAccessToken(t *testing.T) { + cases := []struct { + name string + grants string + status int + }{ + {"the scope itself", authn.ScopeRead, http.StatusOK}, + {"an explicit :RO", authn.ScopeRead + ":RO", http.StatusOK}, + {"a write scope covers a read", authn.ScopeRead + ":RW", http.StatusOK}, + { + // meta mints a token with no grants selected and core-go reads that + // as every permission (auth.Grants.HasAll). Refusing it here would + // refuse the commonest credential on the instance. + "no grants at all is universal, as meta defines it", "", http.StatusOK, + }, + {"another service's scope", "bench.sr.ht/RESULTS:RO", http.StatusForbidden}, + {"meta's own profile scope is not this one", "meta.sr.ht/PROFILE:RO", http.StatusForbidden}, + } + + for _, tc := range cases { + t.Run(tc.name, func(t *testing.T) { + h := newHarness(t, false) + + r := post(t, h, probeQuery, func(req *http.Request) { + req.Header.Set("Authorization", "Bearer "+personalToken("bigbes", tc.grants)) + }) + + require.Equal(t, tc.status, r.status, "body %s", r.body) + if tc.status != http.StatusForbidden { + return + } + assert.Contains(t, r.body, authn.ScopeRead, + "a 403 must name meta's scope, not tokens.sr.ht's grant: a PAT holder "+ + "sent looking for spec:read will not find a checkbox for it") + assert.NotContains(t, r.body, authn.ActionRead) + assert.Empty(t, r.header.Get("WWW-Authenticate"), + "a challenge would send the holder re-minting a token that is fine") + assert.NotContains(t, r.body, "~bigbes/rfcs", "the refusal leaked content") + }) + } +} + +// A personal access token reads as the owner's agent, and the emphasis is on +// agent: it is a bearer string any process holding it can present, forwarded +// through a gateway by whatever client asked, so reading it as authn.KindOwner +// would hand the approved branch and the webhook subscriptions to the widest +// credential on the instance. The scope says the token may read through spec at +// all; who it is stays the ACL's input. +func TestAPersonalAccessTokenReadsAsAnAgentAndNotTheOwner(t *testing.T) { + h := newHarness(t, false) + + r := post(t, h, probeQuery, func(req *http.Request) { + req.Header.Set("Authorization", "Bearer "+personalToken("bigbes", authn.ScopeRead)) + }) + require.Equal(t, http.StatusOK, r.status, "body %s", r.body) + assert.Contains(t, string(r.Data), "~bigbes/rfcs") + + // The principal itself, since a 200 on a read cannot tell an agent from the + // owner: both may read, and only one may approve. + plane := testMetaAuth(t) + p, err := plane.VerifyToken(t.Context(), personalToken("bigbes", authn.ScopeRead)) + require.NoError(t, err) + assert.True(t, p.IsAgent()) + assert.False(t, p.IsOwner(), "a PAT must not reach the webhook mutations") + assert.Equal(t, authn.PlaneMeta, p.Plane) + assert.NoError(t, p.Authorize(authn.ActionRead), + "a PAT can never carry spec:read; asking for it would refuse every one of them") +} + +// A personal access token belonging to another meta.sr.ht account is 403 and is +// not admitted as a second identity — the same rule, and the same status, a +// foreign working token gets. // -// It is the credential this endpoint used to take, when core-go's -// auth.Middleware stood in front of it, and the test exists to pin the change -// rather than to celebrate it: reopening that plane means giving spec.sr.ht a -// meta scope first. -func TestMetaPersonalAccessTokenIsRefused(t *testing.T) { +// This is the check that matters most on this plane. A PAT is the credential +// every account on the instance can mint for itself, so without it the widest +// credential in existence would be the one that skipped the narrowest identity +// rule, and any user of this instance could read the whole corpus. +func TestPersonalAccessTokenOfAnotherOwnerIsRefused(t *testing.T) { h := newHarness(t, false) r := post(t, h, probeQuery, func(req *http.Request) { - req.Header.Set("Authorization", "Bearer "+metaPAT()) + req.Header.Set("Authorization", "Bearer "+personalToken("somebody-else", authn.ScopeRead)) }) - assert.Equal(t, http.StatusUnauthorized, r.status, "body %s", r.body) + assert.Equal(t, http.StatusForbidden, r.status, "body %s", r.body) assert.NotContains(t, r.body, "~bigbes/rfcs", "the refusal leaked content") } +// The two vocabularies do not overlap, and this is the test that says so. Each +// plane is reached by the client id its token was sealed with, and neither can be +// talked into honouring the other's permission — but they refuse it differently, +// and the difference is worth pinning because it is not arbitrary. +func TestOnePlanesPermissionIsNotTheOthers(t *testing.T) { + t.Run("a PAT spelling tokens.sr.ht's grant", func(t *testing.T) { + // Well-formed in meta's grammar — a scope this service does not publish — + // so it authenticates and is refused on permission: 403. + h := newHarness(t, false) + r := post(t, h, probeQuery, func(req *http.Request) { + req.Header.Set("Authorization", + "Bearer "+personalToken("bigbes", authn.ConfigSection+"/"+authn.ActionRead)) + }) + + assert.Equal(t, http.StatusForbidden, r.status, "body %s", r.body) + }) + + t.Run("a working token spelling meta's scope", func(t *testing.T) { + // Not even parseable: ecore's grants grammar is ":", and + // "spec.sr.ht/SPECS" is not a grant string in it. The credential is + // malformed rather than insufficient, so this is 401 with the challenge — + // a stricter answer than the one above, arrived at one step earlier. + h := newHarness(t, false) + r := post(t, h, probeQuery, func(req *http.Request) { + req.Header.Set("Authorization", "Bearer "+agentToken(authn.ScopeRead)) + }) + + assert.Equal(t, http.StatusUnauthorized, r.status, "body %s", r.body) + assert.NotEmpty(t, r.header.Get("WWW-Authenticate")) + }) +} + // A working token belonging to another meta.sr.ht account is 403, not 401 and // not admitted as a second identity: the token verifies and the holder is who // they say they are, there is simply nothing on this single-owner instance to @@ -203,8 +363,13 @@ "owner's cookie": func(req *http.Request) { login(req, "bigbes") }, "a forged token": func(req *http.Request) { req.Header.Set("Authorization", "Bearer not-a-real-token") }, - "a meta PAT": func(req *http.Request) { - req.Header.Set("Authorization", "Bearer "+metaPAT()) + // Expired, and therefore unreadable rather than refused: DecodeBearerToken + // checks expiry before it reports an issuer, so metapat.PlaneOf cannot say + // which plane sealed this and resolveCaller sends it to the working one. + // Both planes owe it the same 401, which is exactly why that routing + // choice is safe — and this asserts the challenge survives it. + "an expired personal access token": func(req *http.Request) { + req.Header.Set("Authorization", "Bearer "+expiredPersonalToken("bigbes")) }, } for name, credential := range cases { diff --git a/graph/graph_test.go b/graph/graph_test.go index d6733c3154268d9ed1c81c96531638437ab09cd2..3c3aabe7cf5a117eb26171054f2c8dfec0725ff8 100644 --- a/graph/graph_test.go +++ b/graph/graph_test.go @@ -15,6 +15,9 @@ "testing" "time" "github.com/go-chi/chi/v5" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" + "sourcecraft.dev/bigbes/sr-ht-core/auth" "sourcecraft.dev/bigbes/sr-ht-core/crypto" "sourcecraft.dev/bigbes/sr-ht-ecore/bearer" @@ -330,7 +333,12 @@ func newHarness(t *testing.T, withProposals bool) harness { t.Helper() resolver := testResolver(t) searcher := &fakeSearcher{} - opts := Options{Reader: newFakeReader(), Searcher: searcher, Resolver: resolver} + opts := Options{ + Reader: newFakeReader(), + Searcher: searcher, + Resolver: resolver, + Meta: testMetaAuth(t), + } var proposals *fakeProposals if withProposals { proposals = &fakeProposals{} @@ -344,9 +352,14 @@ return harness{handler: srv, searcher: searcher, proposals: proposals} } // response is one GraphQL response, decoded far enough to assert on. +// +// The headers are kept because a refusal's status is only half of it: a 401 owes +// the caller the bearer challenge, and a 403 must not carry one — a challenge +// tells a client to go and re-mint a credential that is in fact perfectly good. type response struct { status int body string + header http.Header Data json.RawMessage `json:"data"` Errors []graphqlError `json:"errors"` } @@ -392,7 +405,7 @@ } rec := httptest.NewRecorder() h.handler.ServeHTTP(rec, req) - out := response{status: rec.Code, body: rec.Body.String()} + out := response{status: rec.Code, body: rec.Body.String(), header: rec.Header()} if strings.HasPrefix(rec.Header().Get("Content-Type"), "application/json") { if err := json.Unmarshal(rec.Body.Bytes(), &out); err != nil { t.Fatalf("decode response %q: %v", rec.Body.String(), err) @@ -1074,7 +1087,10 @@ // middleware of its own, the endpoint at /query, and a query that goes through. // The daemon's one line is the line under test here. func TestMountedOnAChiRouter(t *testing.T) { resolver := testResolver(t) - srv, err := New(Options{Reader: newFakeReader(), Searcher: &fakeSearcher{}, Resolver: resolver}) + srv, err := New(Options{ + Reader: newFakeReader(), Searcher: &fakeSearcher{}, + Resolver: resolver, Meta: testMetaAuth(t), + }) if err != nil { t.Fatalf("New: %v", err) } @@ -1095,17 +1111,45 @@ } } // New refuses a half-wired server at startup rather than failing inside the -// first query. +// first query, and says which seam is missing. +// +// The meta plane is the case that would not announce itself, and it is why the +// error text is asserted rather than only the failure. An endpoint built without +// it starts, serves every working token exactly as before and passes every other +// test in this file — while answering 401 to every federated query, a failure +// visible only from api.sr.ht and only once somebody has added the `api-origin=` +// line. "New succeeded" would not have been enough to tell that seam from any +// other. func TestNewRequiresItsSeams(t *testing.T) { - resolver := testResolver(t) - cases := map[string]Options{ - "no reader": {Searcher: &fakeSearcher{}, Resolver: resolver}, - "no searcher": {Reader: newFakeReader(), Resolver: resolver}, - "no resolver": {Reader: newFakeReader(), Searcher: &fakeSearcher{}}, + full := func() Options { + return Options{ + Reader: newFakeReader(), + Searcher: &fakeSearcher{}, + Resolver: testResolver(t), + Meta: testMetaAuth(t), + } + } + cases := map[string]func(*Options){ + "Reader": func(o *Options) { o.Reader = nil }, + "Searcher": func(o *Options) { o.Searcher = nil }, + "Resolver": func(o *Options) { o.Resolver = nil }, + "MetaAuthenticator": func(o *Options) { o.Meta = nil }, } - for name, opts := range cases { - if _, err := New(opts); err == nil { - t.Errorf("%s: New succeeded", name) - } + for name, drop := range cases { + t.Run("without a "+name, func(t *testing.T) { + opts := full() + drop(&opts) + + _, err := New(opts) + require.Error(t, err) + assert.Contains(t, err.Error(), name) + }) } + + // The control: everything above fails for the seam it dropped and not because + // the fixture was never buildable. Proposals stays nil throughout, being the + // one genuinely optional seam — while it is, the `proposals` field fails + // loudly rather than answering "none". + _, err := New(full()) + require.NoError(t, err) } diff --git a/graph/resolver.go b/graph/resolver.go index 08d0a752f1c1d79ea77e50777496229eccb31bdb..8cb1afe4f495d282ef708c388ebdf541834dfad1 100644 --- a/graph/resolver.go +++ b/graph/resolver.go @@ -6,6 +6,7 @@ "time" "github.com/99designs/gqlgen/graphql" + "sourcecraft.dev/bigbes/sr-ht-spec/authn" "sourcecraft.dev/bigbes/sr-ht-spec/core" "sourcecraft.dev/bigbes/sr-ht-spec/doc" "sourcecraft.dev/bigbes/sr-ht-spec/search" @@ -90,13 +91,28 @@ type Proposals interface { ListProposals(ctx context.Context, space core.SpaceRef, state core.ProposalState) ([]Proposal, error) } +// MetaAuthenticator turns a presented meta.sr.ht personal access token into a +// principal. The production implementation is *authn.MetaAuth. +// +// It takes the credential and not the request, which is the whole difference +// from the *authn.Resolver this endpoint holds beside it, and it is deliberate +// twice over. The credential has already been read by the time this is asked — +// resolveCaller routed on it to get here — so handing the request back would +// invite a second parse that could disagree with the first. And a plane that +// cannot see the request cannot read a cookie off it, which makes this +// endpoint's bearer-only rule structural rather than a thing to remember. +type MetaAuthenticator interface { + VerifyToken(ctx context.Context, presented string) (authn.Principal, error) +} + // Compile-time assertions that the production types satisfy the interfaces this // package is written against. They are here rather than in a test so that a -// signature change in service/ or search/ breaks the build of the package that -// depends on them, not of a test somebody may not run. +// signature change in service/, search/ or authn/ breaks the build of the +// package that depends on them, not of a test somebody may not run. var ( - _ Reader = (*service.Service)(nil) - _ Searcher = (*search.Index)(nil) + _ Reader = (*service.Service)(nil) + _ Searcher = (*search.Index)(nil) + _ MetaAuthenticator = (*authn.MetaAuth)(nil) ) // Resolver is the root resolver. It holds the seams above — every read field of diff --git a/graph/server.go b/graph/server.go index ed6f989d88e743915241c63e6c06dfc3eb5e6b12..f4c059b97dfa9b62abf9de5c0127f1084c33b3d8 100644 --- a/graph/server.go +++ b/graph/server.go @@ -28,22 +28,46 @@ // viewer with no read authority gets 401 before the query is parsed — including // for introspection, which is why a gateway federating this schema has to // present a token like any other client. // -// The credential is the bearer plane /mcp and the REST write plane already -// define, and nothing else: +// The credential is a bearer token, and this is the one surface of this service +// that takes either of the instance's two: // // - A tokens.sr.ht working token, verified through sr-ht-ecore's bearer // package by authn.Resolver, owned by [sr.ht] owner-name, and carrying // authn.ActionRead. A token that verifies but does not carry that grant is // 403; one that does not verify is 401 with the bearer challenge. +// - A meta.sr.ht personal access token, verified through sr-ht-ecore's metapat +// package by authn.MetaAuth, owned by that same [sr.ht] owner-name, and +// carrying authn.ScopeRead — the same permission in meta's OAuth vocabulary +// rather than in tokens.sr.ht's. The statuses are the other plane's arm for +// arm; what differs is the permission a 403 names, because the two +// vocabularies do not overlap. // - No cookie. An API client is not a browser. The unified-login cookie is // web/'s plane, and this endpoint is deliberately outside it — so the // principal is overwritten with the anonymous one when no bearer credential // is presented, rather than inherited from whatever middleware happens to // sit above the mount point. -// - No meta.sr.ht personal access token. spec.sr.ht authenticates through one -// issuer (see authn's package comment) and publishes no OAuth scope for meta -// to grant against, so there is nothing a PAT could be scoped for here. A -// PAT is bearer.ErrNotOurs and is refused at the door with 401. +// +// # Why this surface takes a meta PAT when no other one does +// +// Because of the gateway, and for no other reason. api.sr.ht forwards ONE client +// "Authorization" header to every service a federated query touches — its +// AuthMiddleware copies the client's header verbatim into the request context, +// and the internal credential it can mint is used only to fetch schemas at +// startup — so a federated caller arrives here holding whatever credential the +// client had. The only credential that works across the whole instance is a meta +// PAT, so an endpoint that refuses them can never be part of the gateway's +// schema: federating it would be one `api-origin=` line that produced 401s. +// +// The REST write plane, /mcp and the push hook keep exactly one plane, +// tokens.sr.ht's, because they have no such problem and a narrow, revocable, +// short-lived grant is worth its cost there. They resolve identity through +// authn.Resolver, which holds no MetaAuth and therefore cannot produce an +// authn.PlaneMeta principal at all — the scope of the exception is structural +// rather than a convention to remember. +// +// A PAT admitted here is an agent and never the owner, so webhook management +// stays out of its reach: those mutations ask authn.Principal.IsOwner, which only +// the unified-login cookie satisfies and which this endpoint reads none of. // // # What the cmd layer wires // @@ -51,6 +75,7 @@ // gql, err := graph.New(graph.Options{ // Reader: svc, // *service.Service // Searcher: index, // *search.Index // Resolver: svc.Resolver(), +// Meta: metaAuth, // *authn.MetaAuth, this surface alone // }) // if err != nil { // return err @@ -59,12 +84,14 @@ // router.Handle("/query", gql) // // Server installs its own credential middleware, so it goes on the *anonymous* // router: core-go's server.WithSchema would mount it on the authenticated one, -// whose auth.Middleware answers meta's OAuth vocabulary and 401s anything else -// — the vocabulary this service deliberately does not speak. A service that -// mounts its own /query owes the instance api-meta.json as well, because -// core-go serves that file only for the schemas it hosts itself; sr-ht-ecore's -// apimeta package is what serves it, and cmd/specsrht wires it beside the -// route. +// whose auth.Middleware speaks meta's OAuth vocabulary and 401s anything else — +// including the working token every other surface of this service takes. Serving +// both planes from one endpoint is exactly what that middleware cannot do, which +// is why this package resolves the credential itself. A service that mounts its +// own /query owes the instance api-meta.json as well, because core-go serves that +// file only for the schemas it hosts itself; sr-ht-ecore's apimeta package is +// what serves it, and cmd/specsrht wires it beside the route with the scope list +// GrantScopes derives from authn.ScopeRead. // // The router it is mounted on must carry core-go's config and database // middleware. The read path does not need either, but the webhook management @@ -73,6 +100,7 @@ // WithDefaultMiddleware installs that on the authenticated router only. package graph import ( + "errors" "fmt" "log/slog" "net/http" @@ -83,11 +111,37 @@ "github.com/99designs/gqlgen/graphql/handler/extension" "github.com/99designs/gqlgen/graphql/handler/transport" "go.bigb.es/auxilia/scribe" + "sourcecraft.dev/bigbes/sr-ht-ecore/metapat" + "sourcecraft.dev/bigbes/sr-ht-spec/authn" "sourcecraft.dev/bigbes/sr-ht-spec/coreauth" "sourcecraft.dev/bigbes/sr-ht-spec/graph/api" ) +// GrantScopes is what this service publishes in api-meta.json: the scopes a +// meta.sr.ht personal access token can be minted for here. +// +// meta.sr.ht reads that file to build the checkboxes of its personal-token page, +// prefixing each entry with the service's own name — so "SPECS" here is what +// makes "spec.sr.ht/SPECS:RO" a token a human can actually obtain. An empty list, +// which this service published until /query started accepting a PAT, does not +// mean "a service with no scopes": it means no PAT can be scoped for this service +// at all, so the credential the rest of the instance uses could never be +// presented here even in principle. That was the stronger half of the refusal, +// and the half no amount of code in this package could have worked around. +// +// It is derived from authn.ScopeRead rather than spelled a second time, because +// the two spellings must agree: what meta's checkbox mints, and what +// MetaAuth.VerifyToken checks. A scope published and not checked admits what +// should have been refused, one checked and not published cannot be minted at +// all, and neither failure is visible from inside a single file — so a test on +// each side asserts the pair. +// +// This is meta.sr.ht's vocabulary. The other plane's permission is +// authn.ActionRead ("spec:read"), in tokens.sr.ht's; the two grammars share a +// token format and nothing else (sr-ht-ecore's grants and metapat packages). +var GrantScopes = []string{metapat.ScopeName(authn.ScopeRead)} + // Options is everything a Server needs. New says which one is missing rather // than failing later inside a resolver. type Options struct { @@ -103,9 +157,20 @@ // and while it is nil the `proposals` field of the schema fails with an // error saying so rather than answering "none". Proposals Proposals - // Resolver verifies the bearer credential a caller presents. Its cookie - // plane is not used here: see the package comment. + // Resolver verifies a presented tokens.sr.ht working token. Its cookie plane + // is not used here: see the package comment. Resolver *authn.Resolver + + // Meta verifies a presented meta.sr.ht personal access token — the plane that + // exists so this endpoint can be federated into api.sr.ht at all (see + // authn.MetaAuth). + // + // Required, and unlike the working-token plane it needs no configuration to be + // available: it depends on no per-instance origin, only on the meta.sr.ht + // every SourceHut service already talks to. So there is no legitimate "this + // instance has no such plane" case to tolerate a nil for, and an endpoint + // built without it would answer 401 to the gateway while looking configured. + Meta MetaAuthenticator } // Server is the /query endpoint: the executable schema behind the credential @@ -150,6 +215,12 @@ func New(opts Options) (*Server, error) { if opts.Resolver == nil { return nil, fmt.Errorf("graph: authn Resolver is required") } + // The seam whose absence would not announce itself: an endpoint with no PAT + // plane starts, serves every working token exactly as before, and answers 401 + // to every federated query — a failure visible only from the gateway. + if opts.Meta == nil { + return nil, fmt.Errorf("graph: MetaAuthenticator is required") + } schema, err := newSchema(opts) if err != nil { return nil, err @@ -168,7 +239,7 @@ exec.Use(extension.Introspection{}) return &Server{ schema: schema, - http: resolveCaller(opts.Resolver, gate(coreContext(exec))), + http: resolveCaller(opts.Resolver, opts.Meta, gate(coreContext(exec))), }, nil } @@ -192,17 +263,54 @@ // "no cookie" rule made structural: mounted under a router that already // resolved a cookie identity, this endpoint still sees anonymous and still // answers 401. // -// The statuses are authn.StatusFor's, which is the one table this service maps -// a credential failure with: 401 for a credential that does not verify — forged, -// expired, revoked, or issued by somebody else — 403 for one that verifies and -// belongs to a human this single-owner instance has nothing to grant, and 503 -// for a credential that could not be *checked*. The last is not "your token is -// bad": answering 401 to a restart of tokens.sr.ht tells every agent to re-mint -// credentials that were never broken. +// # Two planes, and which one gets the request +// +// This is where spec.sr.ht's /query differs from every other surface it has, and +// the difference is api.sr.ht's. The gateway forwards ONE client "Authorization" +// header to every service a federated query touches, so a federated caller +// arrives holding whatever credential the client had — in practice a meta.sr.ht +// personal access token, the only credential that works instance-wide. An +// endpoint that refused those could not be federated at all, however correct its +// refusal looked. +// +// So the plane is chosen by what was presented, with metapat.PlaneOf, which +// decodes locally and asks no daemon anything: +// +// PlaneMeta the meta.sr.ht plane, scoped by authn.ScopeRead in meta's own +// OAuth vocabulary +// PlaneWorking the tokens.sr.ht plane, scoped by authn.ActionRead +// PlaneUnknown the tokens.sr.ht plane as well — the credential is unreadable +// (forged, corrupt or expired: DecodeBearerToken checks expiry +// before it reports an issuer), both planes owe it the same 401, +// and routing it to the one that already words that refusal +// keeps one message rather than two +// +// Routing on the credential rather than on the failure of one plane is what lets +// the meta plane work on an instance whose config.ini has no [tokens.sr.ht] +// section at all: there is no working-token validator there to fail first, only +// an authn.ErrNoAgentPlane that would have to be told apart from a real refusal. +// +// # What each plane is then asked +// +// Both answer the same three questions and only the vocabulary differs. Is the +// credential live; does it belong to the one human this instance answers to; was +// it minted for reading. The last is asked at different moments — the working +// plane's grant in gate below, where the action is known, the PAT's scope inside +// VerifyToken, because that plane guards this one read surface and nothing else — +// and that is the only asymmetry between them. +// +// The statuses are authn.StatusFor's, which is the one table this service maps a +// credential failure with, and it is one table across both planes on purpose: +// 401 for a credential that does not verify — forged, expired, revoked, or issued +// by somebody else — 403 for one that verifies and belongs to a human this +// single-owner instance has nothing to grant, and 503 for a credential that could +// not be *checked*. The last is not "your token is bad": answering 401 to a +// restart of tokens.sr.ht, or to a meta.sr.ht that will not answer, tells every +// agent to re-mint credentials that were never broken. // // The messages are written here from what the caller already knows, never from // the error's own text: authn's errors name usernames and token ids. -func resolveCaller(rs *authn.Resolver, next http.Handler) http.Handler { +func resolveCaller(rs *authn.Resolver, meta MetaAuthenticator, next http.Handler) http.Handler { return http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { presented := authn.BearerFromRequest(r) if presented == "" { @@ -211,8 +319,20 @@ authn.WithPrincipal(r.Context(), authn.Anonymous()))) return } - p, err := rs.ResolveAgent(r.Context(), presented, - r.Header.Get(authn.HeaderAgent), r.Header.Get(authn.HeaderAgentSession)) + var ( + p authn.Principal + err error + ) + if metapat.PlaneOf(presented) == metapat.PlaneMeta { + // The credential and not the request: a plane that cannot see the + // request cannot read a cookie off it, and the provenance headers are + // a write's business, which this surface has none of. See + // MetaAuthenticator. + p, err = meta.VerifyToken(r.Context(), presented) + } else { + p, err = rs.ResolveAgent(r.Context(), presented, + r.Header.Get(authn.HeaderAgent), r.Header.Get(authn.HeaderAgentSession)) + } if err != nil { status := authn.StatusFor(err) if status >= http.StatusInternalServerError { @@ -228,21 +348,35 @@ // here is a machine holding a bearer token: naming the scheme // and the realm is what tells it which credential was refused. w.Header().Set("WWW-Authenticate", authn.Challenge()) } - http.Error(w, refusalMessage(status), status) + http.Error(w, refusalMessage(status, err), status) return } next.ServeHTTP(w, r.WithContext(authn.WithPrincipal(r.Context(), p))) }) } -// refusalMessage is what a refused caller is told. It is keyed on the status and -// not on the error, so that nothing about whose token it was, or whether a row -// exists, leaks to a caller holding a credential this service did not accept. -func refusalMessage(status int) string { +// refusalMessage is what a refused caller is told. +// +// It is keyed on the status, so that nothing about whose token it was, or whether +// a row exists, leaks to a caller holding a credential this service did not +// accept. The one thing it reads off the error is authn.ErrMissingScope, and that +// is not a hole in the rule: what it adds is a compile-time constant naming a +// permission the caller can go and tick a box for, which is precisely what gate +// below already does for the other plane's grant. It says nothing about the +// credential, the account or the database. +// +// Without that arm a PAT missing the scope would be told "this token does not +// authorize requests to spec.sr.ht" and have nowhere to go: meta's personal-token +// page is a list of checkboxes, and a client that is not told which one it lacks +// cannot ask for it. +func refusalMessage(status int, err error) string { switch status { case http.StatusUnauthorized: return "the bearer token presented was refused" case http.StatusForbidden: + if errors.Is(err, authn.ErrMissingScope) { + return "this personal access token does not carry " + authn.ScopeRead + } return "this token does not authorize requests to " + authn.ConfigSection default: return "the credential could not be verified, try again" @@ -261,6 +395,13 @@ // // A caller that clears it and authenticated with a tokens.sr.ht working token // must also hold spec:read — the grant half of the same question, asked here // because here is where the action ("read") is known. +// +// A caller on the meta.sr.ht plane passes that half untouched, and has already +// answered it. Principal.Authorize is a no-op for authn.PlaneMeta because no PAT +// can carry "spec:read" at all — asking would refuse every one of them — so the +// equivalent question was put to it in meta's own vocabulary at resolution, where +// authn.ScopeRead is what it had to carry. The permission is checked exactly once +// on either plane; only the moment and the grammar differ. // // It is one check at the boundary rather than one per field, because every read // field of this schema is a read and the surface has one action. The webhook