diff --git a/crates/didbot-scope/src/error.rs b/crates/didbot-scope/src/error.rs index 2d92c9a1..f129a935 100644 --- a/crates/didbot-scope/src/error.rs +++ b/crates/didbot-scope/src/error.rs @@ -79,10 +79,9 @@ pub enum ScopeParseError { /// legacy scopes. #[error("`{0}` is not a recognised transition scope")] UnknownTransition(String), - /// The atom gave no value for the parameter its kind requires: an - /// `identity:` or `account:` attribute, a `blob:` type, or an `include:` - /// set. An `include:` with a value that names no permission set still - /// parses; see [`Include::parse`](crate::Include::parse). + /// The atom gave no value for the parameter its kind requires, such as + /// `repo:`'s collection. An `include:` whose value names no permission + /// set still parses; see [`Include::new`](crate::Include::new). #[error("`{0}` is missing the value its kind requires")] MissingValue(String), /// A `?query` segment had a pair with no `=`. diff --git a/docs/operations.md b/docs/operations.md index af6f6623..38d25513 100644 --- a/docs/operations.md +++ b/docs/operations.md @@ -199,3 +199,30 @@ one would make the log the thing a flood amplifies. `bot.did.stats` and `bot.did.listAccounts` are public by default; under `--close-disclosure` they answer 403 and the figures are in the startup line. + +## Scopes a sign-in is not granted, and forms outside the spec + +A sign-in that asks for a `transition:`, `identity:` or `account:` scope has +that scope dropped and the rest granted. So does one that names a scope this +server cannot read, such as the unknown kind `space:*` or the value +`account:status`. The agent's decision record lists each dropped scope +under `cut`. + +An app may write a scope in a form the permission spec does not define. +The server reads it as well as it can, and once it has checked the client +it writes a `warn` line naming the client, the atom as the app wrote it, +and how it was read. The operator's own `[oauth] scope_ceiling` is read +without one, so its partial wildcards write nothing. + +| Form | Read as | +|---|---| +| A partial wildcard, such as `repo:app.bsky.*` | The NSIDs under that prefix, at any depth | +| `rpc:` with no `aud` | Every audience | +| `rpc:*?aud=*` | Every method at every audience, as written | +| An `aud` that is not `*` or a DID with a service fragment | That audience alone | +| `repo:…?action=manage` | No record operation, so the scope adds no write | +| A comma list of actions, `action=create,update` | Each action in it | +| A parameter its kind does not take | Ignored | +| A list parameter both after the `:` and by name, such as `blob:image/png?accept=text/html` | Every value | +| A parameter that takes one value, given twice | The first | +| A scope this server cannot read | Kept as written, and dropped | diff --git a/docs/testing.md b/docs/testing.md index 9f5eab1e..8affde6c 100644 --- a/docs/testing.md +++ b/docs/testing.md @@ -58,6 +58,7 @@ command column is what runs it. | Bluesky's own `@atproto/oauth-client-node` signs in to a running server as a public native client and writes before and after a refresh; as a confidential web client it pushes with its key, is refused approval because a browser would carry its code, and is declined | `npm ci --prefix tools/oauth-interop`, then `cargo test -p didbot-serve --test oauth_interop` | `crates/didbot-serve/tests/oauth_interop.rs` runs `tools/oauth-interop/sign-in.mjs` against `didbot-pds` and answers each sign-in as `didbot-agentd` does; without `node` or the installed tree it passes having said so | | The loopback client form, and a client whose metadata is served from a hostile origin | `cargo test -p didbot-serve --test oauth_standard_client` and `--test oauth_interop` | jacquard-oauth's loopback client and `@atproto/oauth-client-node`'s sign in; a hostile origin is not driven | | Scope narrowing: a token cannot write outside its grant, a tightened ceiling narrows the next write | `cargo test -p didbot-serve --test token_scope` | `crates/didbot-serve/tests/token_scope.rs` and `crates/didbot-scope/tests/ceiling_boundary.rs` | +| Every scope form the permission spec defines reads without a warning; each other form reads as its warning says, and PAR logs it at `warn` with the client; a never-granted scope is dropped and Leaflet and Tangled sign in | `cargo test -p didbot-scope`, then `cargo test -p didbot-serve --test scope_warnings --test permission_sets --test token_scope` | `crates/didbot-scope/src/tests.rs`, and the three `didbot-serve` suites against the real router | | A scripted agent that approves everything, against the default ceiling | none | none | | A second agent cannot redeem a consent reference it did not receive | `cargo test -p didbot-serve --test oauth_account_flow` | the account check is the credential's, and the flow test drives it from both sides | | Every `bot.did.*` route answers in the shape its document declares | `cargo test -p didbot --test conformance bot_did` | `crates/didbot/tests/conformance/bot_did.rs` | diff --git a/plan/oauth.md b/plan/oauth.md index e686e858..0d48f09d 100644 --- a/plan/oauth.md +++ b/plan/oauth.md @@ -402,11 +402,12 @@ as above, and for that one the page says the sign-in was approved. against a broader one, `rpc:`'s `aud` narrowing, an empty ceiling, an empty request, a scope naming a collection outside the ceiling, and a ceiling whose atoms admit a ceiling-order-independent grant (the one - intersect bug review already found here). An unrecognised `kind:` - fails the whole scope string at parse time — see the module doc's - "Unknown prefixes" section — so a client sending a prefix this server - doesn't implement gets no grant at all, rather than a grant scoped to - whatever this server understood. `didbot_operator::operate::scope` is + intersect bug review already found here). Every form the permission spec + defines reads, the query forms included, and one it does not define + reads as well as it can with a `didbot_scope::ScopeWarning`, which PAR + logs at `warn` with the client once the client is checked. An atom + naming no scope the grammar reads is kept as `Scope::Unknown`, which the + decision drops. `didbot_operator::operate::scope` is the first real consumer, building `didbot operate`'s own request scope out of the grammar instead of a hand-written literal. - [x] **DPoP, verifier side.** `crates/didbot-serve/src/oauth/dpop.rs`: diff --git a/plan/scope-policy.md b/plan/scope-policy.md index c869b7d6..3b26c8cc 100644 --- a/plan/scope-policy.md +++ b/plan/scope-policy.md @@ -155,31 +155,33 @@ an agent approves. rotation and migration out; `transition:*` is on the list for the opposite reason, carrying records, blobs and RPC in one string and making every granular rule beside it decorative. A request naming any - of them is refused whole — `oauth::decision::hard_blocked`, checked - before the ceiling is consulted, giving a `deny` verdict whose rule is - `hard-blocked: ` and no token to approve it with. + of them has that atom dropped whole and the rest granted: + `oauth::decision::hard_blocked`, applied before the ceiling is + consulted. The atom is in the decision's `cut`, under the ceiling's + rule, so the agent sees what was dropped. A request left with nothing + grantable is denied as any narrowing to nothing is. An atom the grammar + cannot read, such as an unknown kind like `space:*` or a value like + `account:status`, is dropped the same way rather than failing the + sign-in. A permission set cannot carry one. `didbot_scope::Include::grants` reads only a set's `repo` and `rpc` permissions, as the permission spec - allows, so an `include:` never yields a hard-blocked atom. A set that - lists an `account` permission still grants the rest of what it lists. - Leaflet's scope is refused all the same, because it names - `transition:email` directly: its push mints a record that denies it, - and the authorize step answers `access_denied`. `account:` and - `identity:` scopes read as the permission spec defines them. - `account:` names `email` or `repo`, `read` unless it says `manage`. - `identity:` names `handle` or `*`, and takes no other parameter. So - `account:email?action=read` and `identity?attr=handle` are refused - here the same way, and `identity:handle?action=manage` does not parse. + allows, so an `include:` never yields a hard-blocked atom. Leaflet signs + in with its four sets and without `transition:email` and + `account:email`, and Tangled without `identity:handle`: see + `leaflets_real_scope_string_signs_in_without_email` in + `crates/didbot-serve/tests/permission_sets.rs` and + `tangleds_real_scope_string_signs_in_without_its_handle` in + `crates/didbot-serve/tests/token_scope.rs`. A narrow ceiling cannot express this and must not be asked to. A blocked atom overlaps the atoms a ceiling *does* admit — `transition:generic` covers `repo:*` and `rpc:*` — so a ceiling check on its own pays it out in those pieces, which is a wider grant than the rest of the request would have got. `ScopeSet::narrow` projects - from the requested atom onto the ceiling and only within one kind, so - that holds even with no list loaded, and the list is what makes the - answer a refusal rather than a silence. + from the requested atom onto the ceiling and only within one kind, and + `ScopeSet::new` merges an atom only into one of its own kind, so the + `repo:` scopes asked for beside `transition:generic` survive its drop. - [x] **An `include:` is judged as what its set grants.** Each `include:` in a pushed request becomes the `repo:` and `rpc:` atoms its permission