From f713b8a49ed497d1363d7d6a3374e160a7c98684 Mon Sep 17 00:00:00 2001 From: botandbang <337765928+botandbang@users.noreply.github.com> Date: Fri, 30 Oct 2026 13:28:20 -0400 Subject: [PATCH 01/13] feat(loclib-plugin): send an account type's params with every sign-in --- apps/locallibrary/docs/rpc/locallibrary.d.ts | 5 ++ apps/locallibrary/docs/rpc/schema.json | 7 +++ .../src/plugin/accounts/shared.rs | 21 +++---- .../tests/it/plugins/shared_accounts.rs | 6 +- crates/loclib-plugin/README.md | 8 ++- .../loclib-plugin/src/hooks/account_types.rs | 16 +++++- crates/loclib-plugin/src/manifest/tests.rs | 56 ++++++++++++++----- .../google-account/locallibrary-plugin.toml | 3 + 8 files changed, 93 insertions(+), 29 deletions(-) diff --git a/apps/locallibrary/docs/rpc/locallibrary.d.ts b/apps/locallibrary/docs/rpc/locallibrary.d.ts index 6263ae699..34e1e9dae 100644 --- a/apps/locallibrary/docs/rpc/locallibrary.d.ts +++ b/apps/locallibrary/docs/rpc/locallibrary.d.ts @@ -8117,6 +8117,11 @@ export type AccountType = { * (`prompt = "select_account"`). */ another_account?: { [key: string]: string }; + /** + * The authorize params every sign-in sends besides (`access_type = "offline"`), none of + * `one_account`'s or `another_account`'s. + */ + params?: { [key: string]: string }; /** What a plugin may be given access to, by name (`youtube-upload`). */ scopes?: { [key: string]: Scope }; }; diff --git a/apps/locallibrary/docs/rpc/schema.json b/apps/locallibrary/docs/rpc/schema.json index 12f9a89f7..bacae393d 100644 --- a/apps/locallibrary/docs/rpc/schema.json +++ b/apps/locallibrary/docs/rpc/schema.json @@ -31163,6 +31163,13 @@ }, "description": "The authorize params that sign in to another account than those the browser has\n(`prompt = \"select_account\"`)." }, + "params": { + "type": "object", + "additionalProperties": { + "type": "string" + }, + "description": "The authorize params every sign-in sends besides (`access_type = \"offline\"`), none of\n`one_account`'s or `another_account`'s." + }, "scopes": { "type": "object", "additionalProperties": { diff --git a/crates/locallibrary/src/plugin/accounts/shared.rs b/crates/locallibrary/src/plugin/accounts/shared.rs index 4b2b02090..29242c0b8 100644 --- a/crates/locallibrary/src/plugin/accounts/shared.rs +++ b/crates/locallibrary/src/plugin/accounts/shared.rs @@ -7,7 +7,7 @@ use std::time::Duration; use chrono::{DateTime, Utc}; -use indexmap::{IndexMap, IndexSet}; +use indexmap::IndexSet; use loclib_plugin::{AccountType, SUBJECT_PLACEHOLDER}; use oauth2::TokenResponse; use schemars::JsonSchema; @@ -250,10 +250,10 @@ impl Plugins { /// `with`, for the plugin `for_plugin` if it's for one: the address to open in the browser, /// which comes back to this computer. With `account`, it signs in again as that account (the /// subject of one `with` signed in); without, the service is asked to offer another than those - /// it signed in, when there are any. It asks the service for what the type always asks for, - /// what `account` was granted, and what `scopes` (the type's, by name) ask for, and for a - /// plugin, what the scopes it needs ask for: it's given access to those, and to those of - /// `scopes` it can do without (see [`Plugins::give_access`]). Finished with + /// it signed in, when there are any; the type's own params go with either. It asks the service + /// for what the type always asks for, what `account` was granted, and what `scopes` (the + /// type's, by name) ask for, and for a plugin, what the scopes it needs ask for: it's given + /// access to those, and to those of `scopes` it can do without (see [`Plugins::give_access`]). Finished with /// [`Plugins::finish_account_sign_in`] (or [`Plugins::finish_account_sign_in_with_url`], when /// the browser runs on another computer), within a quarter of an hour. pub fn begin_account_sign_in( @@ -281,13 +281,14 @@ impl Plugins { let scope = scope.ok_or_else(|| Error::NotFound(format!("the {kind} account type's scope {name:?}")))?; asking.extend(scope.asks.iter().cloned()); } - let params = match account { + let mut params = t.params.clone(); + match account { Some(subject) => { - t.one_account.iter().map(|(k, v)| (k.clone(), v.replace(SUBJECT_PLACEHOLDER, subject))).collect() + params.extend(t.one_account.iter().map(|(k, v)| (k.clone(), v.replace(SUBJECT_PLACEHOLDER, subject)))); } - None if all.iter().any(|k| k.plugin == with) => t.another_account.clone(), - None => IndexMap::new(), - }; + None if all.iter().any(|k| k.plugin == with) => params.extend(t.another_account.clone()), + None => {} + } if let Some(k) = again { // The keychain can be used, before the browser is sent anywhere. Only whether it can, // what it keeps dropped first: CodeQL follows a `?` on it into the sign-in link. diff --git a/crates/locallibrary/tests/it/plugins/shared_accounts.rs b/crates/locallibrary/tests/it/plugins/shared_accounts.rs index 6b19148cf..9f9821b0c 100644 --- a/crates/locallibrary/tests/it/plugins/shared_accounts.rs +++ b/crates/locallibrary/tests/it/plugins/shared_accounts.rs @@ -353,16 +353,18 @@ fn a_service_that_doesnt_say_what_it_granted_granted_what_was_asked() { } /// The browser is asked to sign in as the account signed in again, or to another than those the -/// account plugin has when there are any, and to nothing in particular before. +/// account plugin has when there are any, and to nothing in particular before; the type's own +/// params go with every sign-in. #[test] fn the_browser_is_asked_for_one_account_or_another() { - let m = mock(3600); + let m = mock_at(3600, "/userinfo", "params = { access_type = \"offline\" }"); let (_, first) = m.sign_in(who(json!("a"), None), None, &[]); assert!(!first.contains_key("prompt") && !first.contains_key("login_hint")); let (_, another) = m.sign_in(who(json!("b"), None), None, &[]); assert_eq!((another.get("prompt").map(String::as_str), another.get("login_hint")), (Some("select_account"), None)); let (done, again) = m.sign_in(who(json!("b"), None), Some("a"), &[]); assert_eq!((again.get("prompt"), again["login_hint"].as_str()), (None, "a")); + assert!([first, another, again].iter().all(|q| has(q, "access_type", "offline")), "the type's params"); assert_eq!(done.unwrap().instead_of.as_deref(), Some("a"), "signed in as another than asked"); let subjects: Vec = m.listed().into_iter().map(|a| a.subject).collect(); assert_eq!(subjects, ["a", "b"]); diff --git a/crates/loclib-plugin/README.md b/crates/loclib-plugin/README.md index 301de7334..d68c49426 100644 --- a/crates/loclib-plugin/README.md +++ b/crates/loclib-plugin/README.md @@ -205,6 +205,7 @@ let manifest = Manifest::parse(r#" always = ["openid", "email"] whoami = { url = "https://openidconnect.googleapis.com/v1/userinfo", subject = "/sub", name = "/email" } one_account = { login_hint = "{subject}" } + params = { access_type = "offline" } [contributes.account_types.scopes.youtube-upload] title = "upload videos to YouTube" @@ -242,8 +243,11 @@ assert!(!upload.covers("POST", &Url::parse("https://www.googleapis.com.evil.exam one it doesn't know, or isn't set up for yet, fails the sign-in with what the website said. - `one_account` and `another_account` are authorize params for signing in again as one account the host knows (`login_hint = "{subject}"`, the only placeholder) and for adding another - (`prompt = "select_account"`). They may be `login_hint`, `prompt`, `hd` and `access_type` only: - the host sets the rest itself, and others (`request_uri`) could replace what it sets. + (`prompt = "select_account"`), and `params` are those every sign-in sends + (`access_type = "offline"`, which Google wants before it gives a web client a refresh token). + They may be `login_hint`, `prompt`, `hd` and `access_type` only: the host sets the rest itself, + and others (`request_uri`) could replace what it sets. A param set in `params` isn't set in the + other two. - `[contributes.account_types.scopes.]`: `title`, `asks` (the service's scopes it needs, none for one only locallibrary knows) and `requests`, each a `method` (`GET`, `HEAD`, `POST`, `PUT`, `PATCH` or `DELETE`) and a `url` written as it's read (`https://a.example/`, not diff --git a/crates/loclib-plugin/src/hooks/account_types.rs b/crates/loclib-plugin/src/hooks/account_types.rs index 48fe90df8..88fa064e3 100644 --- a/crates/loclib-plugin/src/hooks/account_types.rs +++ b/crates/loclib-plugin/src/hooks/account_types.rs @@ -68,6 +68,10 @@ pub struct AccountType { /// (`prompt = "select_account"`). #[serde(default, skip_serializing_if = "IndexMap::is_empty")] pub another_account: IndexMap, + /// The authorize params every sign-in sends besides (`access_type = "offline"`), none of + /// `one_account`'s or `another_account`'s. + #[serde(default, skip_serializing_if = "IndexMap::is_empty")] + pub params: IndexMap, /// What a plugin may be given access to, by name (`youtube-upload`). #[serde(default, skip_serializing_if = "IndexMap::is_empty")] pub scopes: IndexMap, @@ -132,7 +136,8 @@ impl Scope { /// `[[contributes.account_types]]`: one, in a manifest with nothing else; its type and scopes' /// names, titles, endpoints (a token endpoint, or Local Network's token exchange in a built-in /// plugin), where its token goes and the port the browser comes back to, the service's scopes, -/// pointers into who signed in, authorize params, and the requests each scope covers. +/// pointers into who signed in, authorize params (each set in one place), and the requests each +/// scope covers. pub(crate) fn validate(m: &Manifest, built_in: bool) -> Result<(), ManifestError> { let (at, c) = ("contributes.account_types", &m.contributes); let Some(x) = c.account_types.first() else { return Ok(()) }; @@ -174,6 +179,13 @@ pub(crate) fn validate(m: &Manifest, built_in: bool) -> Result<(), ManifestError } params(&format!("{at}.one_account"), &x.one_account, true)?; params(&format!("{at}.another_account"), &x.another_account, false)?; + params(&format!("{at}.params"), &x.params, false)?; + let elsewhere = [("one_account", &x.one_account), ("another_account", &x.another_account)]; + for key in x.params.keys() { + if let Some((which, _)) = elsewhere.iter().find(|(_, set)| set.contains_key(key)) { + return Err(invalid(format!("{at}.params.{key}"), format!("is in {which} too: set it in one place"))); + } + } for (name, scope) in &x.scopes { let at = format!("{at}.scopes.{name}"); if !is_name(name) { @@ -235,7 +247,7 @@ fn params(at: &str, params: &IndexMap, subject: bool) -> Result< if unnamed.contains(['{', '}']) { let why = match subject { true => format!("{value:?} has a {{ or }} besides {SUBJECT_PLACEHOLDER}"), - false => format!("{value:?} has a {{ or }}: there's no account yet to name"), + false => format!("{value:?} has a {{ or }}: only one_account names an account"), }; return Err(invalid(at, why)); } diff --git a/crates/loclib-plugin/src/manifest/tests.rs b/crates/loclib-plugin/src/manifest/tests.rs index 16dc841ca..2513b922c 100644 --- a/crates/loclib-plugin/src/manifest/tests.rs +++ b/crates/loclib-plugin/src/manifest/tests.rs @@ -383,6 +383,7 @@ always = ["openid", "email"] whoami = { url = "https://openidconnect.googleapis.com/v1/userinfo", subject = "/sub", name = "/email" } one_account = { login_hint = "{subject}" } another_account = { prompt = "select_account" } +params = { access_type = "offline" } [contributes.account_types.scopes.youtube-upload] title = "upload videos to YouTube" @@ -407,7 +408,10 @@ fn an_account_plugin_provides_one_account_type_and_nothing_else() { assert!(!Manifest::parse(ECHO).unwrap().is_account_plugin()); let google = &m.contributes.account_types[0]; assert_eq!((google.name.as_str(), &google.token, google.redirect_port), ("google", &TokenPlace::default(), None)); - assert_eq!(google.one_account["login_hint"], "{subject}"); + assert_eq!( + (google.one_account["login_hint"].as_str(), google.params["access_type"].as_str()), + ("{subject}", "offline") + ); assert_eq!(google.scopes["youtube-upload"].asks, ["https://www.googleapis.com/auth/youtube.upload"]); let at = "contributes.account_types[0]"; assert_eq!( @@ -564,7 +568,7 @@ fn an_account_type_says_where_its_token_goes_and_where_the_browser_comes_back() } /// An account type sets only the authorize params that can't change where a code goes or what it's -/// for, and names the account only as `{subject}`, only to sign in to it again. +/// for, each in one place, and names the account only as `{subject}`, only to sign in to it again. #[test] fn an_account_types_params_name_the_account_only_as_its_subject() { let at = "contributes.account_types[0]"; @@ -574,8 +578,15 @@ fn an_account_types_params_name_the_account_only_as_its_subject() { let another = |key: &str, value: &str| { google_with(|m| _ = m.contributes.account_types[0].another_account.insert(key.into(), value.into())) }; + let every = |key: &str, value: &str| { + google_with(|m| _ = m.contributes.account_types[0].params.insert(key.into(), value.into())) + }; assert_eq!(one("login_hint", "id:{subject}{subject}"), ""); assert_eq!(another("hd", "example.com"), ""); + assert_eq!(every("hd", "example.com"), ""); + for (key, which) in [("login_hint", "one_account"), ("prompt", "another_account")] { + assert_eq!(every(key, "x"), format!("{at}.params.{key}: is in {which} too: set it in one place")); + } let said = "isn't one of the params an account type may set: login_hint, prompt, hd, access_type"; for key in [ "client_id", @@ -594,6 +605,7 @@ fn an_account_types_params_name_the_account_only_as_its_subject() { ] { assert_eq!(one(key, "x"), format!("{at}.one_account.{key}: {said}")); assert_eq!(another(key, "x"), format!("{at}.another_account.{key}: {said}")); + assert_eq!(every(key, "x"), format!("{at}.params.{key}: {said}")); } for value in ["{email}", "{subject", "{sub{subject}ject}", "}"] { let said = format!("{at}.one_account.login_hint: {value:?} has a {{ or }} besides {{subject}}"); @@ -601,7 +613,7 @@ fn an_account_types_params_name_the_account_only_as_its_subject() { } assert_eq!( another("login_hint", "{subject}"), - format!("{at}.another_account.login_hint: \"{{subject}}\" has a {{ or }}: there's no account yet to name") + format!("{at}.another_account.login_hint: \"{{subject}}\" has a {{ or }}: only one_account names an account") ); } @@ -2887,13 +2899,14 @@ fn pointer(tc: &TestCase) -> String { tc.draw(gs::from_regex("(/([a-z0-9]|~[01]){0,6}){0,3}")) } -/// Authorize params an account type may set, their values naming the account (only as -/// `{subject}`) where `subject`. +/// Authorize params an account type may set, but those `taken` sets, their values naming the +/// account (only as `{subject}`) where `subject`. #[hegel::test_helper] -fn authorize_params(tc: &TestCase, subject: bool) -> IndexMap { +fn authorize_params(tc: &TestCase, subject: bool, taken: &[&IndexMap]) -> IndexMap { let parts = if subject { 3 } else { 1 }; let keys = tc.draw(gs::vecs(gs::sampled_from(AUTHORIZE_PARAMS.to_vec())).max_size(3).unique(true)); keys.into_iter() + .filter(|key| !taken.iter().any(|set| set.contains_key(*key))) .map(|key| { let parts = tc.draw(gs::vecs(gs::text().max_size(6)).min_size(1).max_size(parts)); let parts: Vec = parts.iter().map(|p| p.replace(['{', '}'], "")).collect(); @@ -2925,6 +2938,8 @@ fn scope(tc: &TestCase) -> Scope { #[hegel::test_helper] fn account_type(tc: &TestCase, built_in: bool) -> AccountType { let exchanges = built_in && tc.draw(gs::booleans()); + let (one_account, another_account) = (authorize_params(tc, true, &[]), authorize_params(tc, false, &[])); + let params = authorize_params(tc, false, &[&one_account, &another_account]); AccountType { name: tc.draw(gs::from_regex(DRAWN_NAME)), title: title(tc), @@ -2937,8 +2952,9 @@ fn account_type(tc: &TestCase, built_in: bool) -> AccountType { redirect_port: tc.draw(gs::optional(gs::integers::().min_value(1024))), always: service_scopes(tc), whoami: Whoami { url: endpoint(tc), subject: pointer(tc), name: pointer(tc) }, - one_account: authorize_params(tc, true), - another_account: authorize_params(tc, false), + one_account, + another_account, + params, scopes: drawn_names(tc, 3).into_iter().map(|name| (name, scope(tc))).collect(), } } @@ -3003,6 +3019,7 @@ fn each_kind_of_account_type_mistake_is_refused_saying_where(tc: TestCase) { "pointer", "param", "placeholder", + "clash", "scope name", "scope title", "no requests", @@ -3155,10 +3172,12 @@ fn each_kind_of_account_type_mistake_is_refused_saying_where(tc: TestCase) { format!("{at}.whoami.{key}: {wrong:?} isn't a JSON pointer, like /sub") } "param" | "placeholder" => { - let (params, which) = match tc.draw(gs::booleans()) { - true => (&mut x.one_account, "one_account"), - false => (&mut x.another_account, "another_account"), - }; + let (params, which) = + match tc.draw(gs::sampled_from(vec!["one_account", "another_account", "params"])) { + "one_account" => (&mut x.one_account, "one_account"), + "another_account" => (&mut x.another_account, "another_account"), + _ => (&mut x.params, "params"), + }; let (key, value, said) = match mistake { "param" => { let key = tc.draw( @@ -3180,7 +3199,7 @@ fn each_kind_of_account_type_mistake_is_refused_saying_where(tc: TestCase) { let value = format!("{}{named}{bad}{}", word(&tc), word(&tc)); let said = match which { "one_account" => format!("{value:?} has a {{ or }} besides {{subject}}"), - _ => format!("{value:?} has a {{ or }}: there's no account yet to name"), + _ => format!("{value:?} has a {{ or }}: only one_account names an account"), }; ("login_hint".to_string(), value, said) } @@ -3188,6 +3207,17 @@ fn each_kind_of_account_type_mistake_is_refused_saying_where(tc: TestCase) { params.insert(key.clone(), value); format!("{at}.{which}.{key}: {said}") } + "clash" => { + let key = tc.draw(gs::sampled_from(AUTHORIZE_PARAMS.to_vec())); + let set = match tc.draw(gs::booleans()) { + true => &mut x.one_account, + false => &mut x.another_account, + }; + set.entry(key.into()).or_insert_with(|| "x".into()); + x.params.insert(key.into(), "x".into()); + let which = if x.one_account.contains_key(key) { "one_account" } else { "another_account" }; + format!("{at}.params.{key}: is in {which} too: set it in one place") + } "scope name" => { let name = bad_name(&tc); x.scopes.insert(name.clone(), scope(&tc)); diff --git a/plugins/accounts/google-account/locallibrary-plugin.toml b/plugins/accounts/google-account/locallibrary-plugin.toml index f12b9d11e..dcf4ce6be 100644 --- a/plugins/accounts/google-account/locallibrary-plugin.toml +++ b/plugins/accounts/google-account/locallibrary-plugin.toml @@ -19,6 +19,9 @@ always = ["openid", "email"] whoami = { url = "https://openidconnect.googleapis.com/v1/userinfo", subject = "/sub", name = "/email" } one_account = { login_hint = "{subject}" } another_account = { prompt = "select_account" } +# Google gives a web client (Local Network's, behind the token exchange) a refresh token only when +# the sign-in asks for offline access. +params = { access_type = "offline" } # YouTube's resumable upload: the POST that starts a session, and the PUTs that send the video to # the address it answers with, the same one with the session's `upload_id` in its query. From a33847c6475ee3749c434841bf276e848db5c554 Mon Sep 17 00:00:00 2001 From: botandbang <337765928+botandbang@users.noreply.github.com> Date: Fri, 30 Oct 2026 13:28:21 -0400 Subject: [PATCH 02/13] fix(locallibrary): sign in again for the scopes of every access, keeping the refresh token --- apps/locallibrary/docs/rpc/locallibrary.d.ts | 5 +- apps/locallibrary/docs/rpc/schema.json | 2 +- apps/locallibrary/src/account_cmd/accounts.rs | 6 +- apps/locallibrary/src/rpc/methods/accounts.rs | 3 +- .../tests/it/plugins/accounts/cli.rs | 6 ++ .../src/plugin/accounts/shared.rs | 77 +++++++++++-------- .../locallibrary/tests/it/plugins/accesses.rs | 72 +++++++++++++++++ .../tests/it/plugins/shared_accounts.rs | 20 +++-- 8 files changed, 145 insertions(+), 46 deletions(-) diff --git a/apps/locallibrary/docs/rpc/locallibrary.d.ts b/apps/locallibrary/docs/rpc/locallibrary.d.ts index 34e1e9dae..3445b1fda 100644 --- a/apps/locallibrary/docs/rpc/locallibrary.d.ts +++ b/apps/locallibrary/docs/rpc/locallibrary.d.ts @@ -2260,7 +2260,10 @@ export type BeginSignIn = { * can do without. */ for?: string | null; - /** An account the store keeps, by subject, to sign in to again (asking for more scopes, say). */ + /** + * An account the store keeps, or one signed out that plugins still have access to, by + * subject, to sign in to again (asking for more scopes, say). + */ account?: string | null; /** The type's scopes to ask for besides, by name. */ scopes?: string[]; diff --git a/apps/locallibrary/docs/rpc/schema.json b/apps/locallibrary/docs/rpc/schema.json index bacae393d..e22acda87 100644 --- a/apps/locallibrary/docs/rpc/schema.json +++ b/apps/locallibrary/docs/rpc/schema.json @@ -11865,7 +11865,7 @@ "string", "null" ], - "description": "An account the store keeps, by subject, to sign in to again (asking for more scopes, say).", + "description": "An account the store keeps, or one signed out that plugins still have access to, by\nsubject, to sign in to again (asking for more scopes, say).", "default": null }, "scopes": { diff --git a/apps/locallibrary/src/account_cmd/accounts.rs b/apps/locallibrary/src/account_cmd/accounts.rs index f4acd4c6d..d992c3ffc 100644 --- a/apps/locallibrary/src/account_cmd/accounts.rs +++ b/apps/locallibrary/src/account_cmd/accounts.rs @@ -64,9 +64,9 @@ pub(super) fn run( } if action == Action::SignIn { let store = Store::open_or_init(root)?; - // Signed in again as an account kept; one signed out is signed in to anew. - let kept = plugins.store_accounts(&store)?; - let again = named.map(|named| find(&kept, kind, named)).transpose()?; + // Signed in again as an account kept, or one signed out that plugins still have access to. + let known = plugins.accounts_and_accesses(&store)?.accounts; + let again = named.map(|named| find(&known, kind, named)).transpose()?; let with = match (args.with, again) { (Some(with), _) => with, (None, Some(a)) => a.plugin.clone(), diff --git a/apps/locallibrary/src/rpc/methods/accounts.rs b/apps/locallibrary/src/rpc/methods/accounts.rs index 6150d8d44..ab19a69c8 100644 --- a/apps/locallibrary/src/rpc/methods/accounts.rs +++ b/apps/locallibrary/src/rpc/methods/accounts.rs @@ -24,7 +24,8 @@ struct BeginSignIn { /// can do without. #[serde(default, rename = "for")] for_plugin: Option, - /// An account the store keeps, by subject, to sign in to again (asking for more scopes, say). + /// An account the store keeps, or one signed out that plugins still have access to, by + /// subject, to sign in to again (asking for more scopes, say). #[serde(default)] account: Option, /// The type's scopes to ask for besides, by name. diff --git a/apps/locallibrary/tests/it/plugins/accounts/cli.rs b/apps/locallibrary/tests/it/plugins/accounts/cli.rs index 81cba6edd..4ad46ff07 100644 --- a/apps/locallibrary/tests/it/plugins/accounts/cli.rs +++ b/apps/locallibrary/tests/it/plugins/accounts/cli.rs @@ -106,6 +106,12 @@ fn an_account_is_signed_in_listed_and_signed_out_on_the_command_line() { let (_, out, _) = m.run(&["account"]); let signed_out = " signed out: `locallibrary account mock sign-in` signs in again\n access: echo (default)\n"; assert!(out.contains(signed_out), "{out}"); + // Signed out, it's still named for signing in to again: echo has access to it. + let mut c = m.command(); + c.args(["account", "mock:a@mock.example", "sign-in", "--no-browser", "--timeout", "10"]); + let (code, out, err) = m.approving(c, &who("a")); + assert_eq!((code, out.as_str()), (0, "✓ Signed in to the Mock account a@mock.example\n"), "{err}"); + assert_eq!(m.run(&["account", "mock:a", "sign-out"]).0, 0); let (code, out, _) = m.run(&["plugin", "access", "echo", "--take-away", "a"]); assert_eq!((code, out.as_str()), (0, "✓ Took echo's access to the Mock account a@mock.example away\n")); let (_, out, _) = m.run(&["plugin", "access", "echo"]); diff --git a/crates/locallibrary/src/plugin/accounts/shared.rs b/crates/locallibrary/src/plugin/accounts/shared.rs index 29242c0b8..102baec75 100644 --- a/crates/locallibrary/src/plugin/accounts/shared.rs +++ b/crates/locallibrary/src/plugin/accounts/shared.rs @@ -1,8 +1,9 @@ //! Accounts of a type, signed in to through the account plugins that provide it and kept once per //! store (see [`kept`]), for plugins to be given access to. Signing in asks the service for what -//! the type always asks for, what the account was granted before, and what the scopes asked for -//! ask for; whoever signed in is then asked of the service (see [`Plugin::whoami`]), and the same -//! account signed in again through the same account plugin is changed in place. +//! the type always asks for, what the account was granted before and what every access to it +//! needs, and what the scopes asked for ask for; whoever signed in is then asked of the service +//! (see [`Plugin::whoami`]), and the same account signed in again through the same account plugin +//! is changed in place, keeping its refresh token when the service answers none. use std::time::Duration; @@ -249,13 +250,15 @@ impl Plugins { /// Starts signing in to an account of type `kind` in `store` through the account plugin /// `with`, for the plugin `for_plugin` if it's for one: the address to open in the browser, /// which comes back to this computer. With `account`, it signs in again as that account (the - /// subject of one `with` signed in); without, the service is asked to offer another than those - /// it signed in, when there are any; the type's own params go with either. It asks the service - /// for what the type always asks for, what `account` was granted, and what `scopes` (the - /// type's, by name) ask for, and for a plugin, what the scopes it needs ask for: it's given - /// access to those, and to those of `scopes` it can do without (see [`Plugins::give_access`]). Finished with - /// [`Plugins::finish_account_sign_in`] (or [`Plugins::finish_account_sign_in_with_url`], when - /// the browser runs on another computer), within a quarter of an hour. + /// subject of one `with` signed in, or of one signed out that plugins still have access to); + /// without, the service is asked to offer another than those it signed in, when there are + /// any; the type's own params go with either. It asks the service for what the type always + /// asks for, what `account` was granted and what every access to it needs, and what `scopes` + /// (the type's, by name) ask for, and for a plugin, what the scopes it needs ask for: it's + /// given access to those, and to those of `scopes` it can do without (see + /// [`Plugins::give_access`]). Finished with [`Plugins::finish_account_sign_in`] (or + /// [`Plugins::finish_account_sign_in_with_url`], when the browser runs on another computer), + /// within a quarter of an hour. pub fn begin_account_sign_in( &self, kind: &str, @@ -267,32 +270,36 @@ impl Plugins { ) -> Result { let slot = self.slot(with)?; let t = slot.account_type(kind)?; - let all = kept::read(store)?.accounts; - let again = - account.map(|subject| all.iter().find(|k| k.is(with, subject)).ok_or_else(|| not_kept(with, subject))); - let again = again.transpose()?; + let book = kept::read(store)?; let mut asking: IndexSet = t.always.iter().cloned().collect(); - asking.extend(again.iter().flat_map(|k| k.scopes.iter().cloned())); - let given = - for_plugin.map(|plugin| Ok::<_, Error>((plugin.to_string(), self.slot(plugin)?.given(kind, scopes)?))); - let given = given.transpose()?; - for name in scopes.iter().chain(given.iter().flat_map(|(_, given)| given)) { - let scope = t.scopes.get(name); - let scope = scope.ok_or_else(|| Error::NotFound(format!("the {kind} account type's scope {name:?}")))?; - asking.extend(scope.asks.iter().cloned()); - } let mut params = t.params.clone(); match account { Some(subject) => { + let accesses: Vec<&Access> = + book.accesses.iter().filter(|a| a.account.plugin == with && a.account.subject == subject).collect(); + let k = book.account(with, subject); + if k.is_none() && accesses.is_empty() { + return Err(not_kept(with, subject)); + } + asking.extend(k.into_iter().flat_map(|k| k.scopes.iter().cloned())); + let needed = accesses.iter().flat_map(|a| &a.scopes).filter_map(|name| t.scopes.get(name)); + asking.extend(needed.flat_map(|scope| scope.asks.iter().cloned())); params.extend(t.one_account.iter().map(|(k, v)| (k.clone(), v.replace(SUBJECT_PLACEHOLDER, subject)))); + // The keychain can be used, before the browser is sent anywhere. Only whether it + // can, what it keeps dropped first: CodeQL follows a `?` on it into the sign-in + // link. + secret(self.host.options.secrets, &kept::place(store, with, subject)).map(drop)?; } - None if all.iter().any(|k| k.plugin == with) => params.extend(t.another_account.clone()), + None if book.accounts.iter().any(|k| k.plugin == with) => params.extend(t.another_account.clone()), None => {} } - if let Some(k) = again { - // The keychain can be used, before the browser is sent anywhere. Only whether it can, - // what it keeps dropped first: CodeQL follows a `?` on it into the sign-in link. - secret(self.host.options.secrets, &kept::place(store, with, &k.subject)).map(drop)?; + let given = + for_plugin.map(|plugin| Ok::<_, Error>((plugin.to_string(), self.slot(plugin)?.given(kind, scopes)?))); + let given = given.transpose()?; + for name in scopes.iter().chain(given.iter().flat_map(|(_, given)| given)) { + let scope = t.scopes.get(name); + let scope = scope.ok_or_else(|| Error::NotFound(format!("the {kind} account type's scope {name:?}")))?; + asking.extend(scope.asks.iter().cloned()); } let scopes: Vec = asking.into_iter().collect(); let asked = Asked { @@ -328,15 +335,21 @@ impl Plugins { let slot = self.slot(&asked.with)?; let t = slot.account_type(&asked.kind)?; let answer = slot.traded(t, &p.client, p.verifier, query)?; - let mut tokens = Tokens::from(&answer, None); + let access_token = answer.access_token().secret(); + slot.net.saw(access_token); + let (subject, name) = slot.whoami(t, access_token)?; + let (store, storage) = (&p.store, self.host.options.secrets); + let _one = kept::hold(store, &asked.with, &subject)?; + // Signed in again, it keeps the refresh token it had when the service answers none. One + // that can't be read is signed in to anew. + let had = kept::read(store)?.account(&asked.with, &subject).cloned(); + let had = had.and_then(|k| slot.kept_tokens(store, &k).ok().flatten()); + let mut tokens = Tokens::from(&answer, had); // A service that doesn't say what it granted granted what it was asked (RFC 6749 5.1). if answer.scopes().is_none() { tokens.scopes = asked.scopes; } slot.seen(&tokens); - let (subject, name) = slot.whoami(t, &tokens.access_token)?; - let (store, storage) = (&p.store, self.host.options.secrets); - let _one = kept::hold(store, &asked.with, &subject)?; let (account, access) = kept::update(store, |book| { let all = &mut book.accounts; let i = match all.iter().position(|k| k.is(&asked.with, &subject)) { diff --git a/crates/locallibrary/tests/it/plugins/accesses.rs b/crates/locallibrary/tests/it/plugins/accesses.rs index 6319919cb..b4000d417 100644 --- a/crates/locallibrary/tests/it/plugins/accesses.rs +++ b/crates/locallibrary/tests/it/plugins/accesses.rs @@ -9,6 +9,8 @@ use hegel::generators as gs; use locallibrary::error::{Action, Reason}; use locallibrary::plugin::AccountId; +use super::accounts::{form, has}; +use super::net::Got; use super::shared_accounts::{Mock, WITH, asking, mock_for, who}; use super::*; @@ -278,6 +280,76 @@ fn a_signed_out_accounts_access_lists_and_says_to_sign_in() { assert_eq!(m.f.plugins.accesses(&m.f.store, Some("echo"), None).unwrap(), listed, "one access, as it was"); } +/// For an account granted what any of the mock type's scopes ask, echo's access to it and another +/// plugin's (each with any scopes, or none), signed out or not, and signed in to again (for echo or +/// not) asking for any scopes besides, the service answering a new refresh token or none: the +/// browser is asked to sign in as the account, for what the type always asks, what the account +/// was granted, what every access's scopes ask and what's asked for besides; the refresh token it +/// had is kept when the service answers none; and one signed out that no plugin has access to +/// can't be signed in to again. +#[hegel::test(test_cases = 12, suppress_health_check = [hegel::HealthCheck::TooSlow])] +fn signing_in_again_asks_for_what_every_access_needs(tc: TestCase) { + let m = mock_for(30, "/userinfo", "", &asking(&["read"], &["write", "peek"])); + m.state.lock().unwrap().unscoped = true; + let first = some(&tc); + m.sign_in_for(None, who(json!("a"), None), None, &first).0.unwrap(); + // The scopes whose asks the sign-in again asks for. + let mut named: Vec = Vec::new(); + let echo = tc.draw(gs::booleans()).then(|| some(&tc)); + if let Some(extra) = &echo { + let extra: Vec = extra.iter().map(|s| s.to_string()).collect(); + named.extend(m.f.plugins.give_access("echo", &account("a"), &extra, &m.f.store).unwrap().scopes); + } + let other = tc.draw(gs::booleans()).then(|| some(&tc)); + if let Some(scopes) = &other { + let file = m.f.store.root().join("accounts.json"); + let mut kept: Value = serde_json::from_str(&std::fs::read_to_string(&file).unwrap()).unwrap(); + let access = json!({ "id": "0a1b2c3d", "plugin": "other", "type": "mock", "account": account("a"), + "name": "a", "scopes": scopes, "default": true }); + kept["accesses"].as_array_mut().unwrap().push(access); + std::fs::write(&file, kept.to_string()).unwrap(); + named.extend(scopes.iter().map(|s| s.to_string())); + } + let signed_out = tc.draw(gs::booleans()); + match signed_out { + true => drop(m.f.plugins.sign_out_account(WITH, "a", &m.f.store).unwrap()), + false => named.extend(first.iter().map(|s| s.to_string())), + } + let refreshless = tc.draw(gs::booleans()); + m.state.lock().unwrap().refreshless = refreshless; + let for_echo = tc.draw(gs::booleans()); + let added = some(&tc); + let (done, opened) = m.sign_in_for(for_echo.then_some("echo"), who(json!("a"), None), Some("a"), &added); + if signed_out && echo.is_none() && other.is_none() { + assert_eq!(done.unwrap_err().code(), "not_found"); + return; + } + let done = done.unwrap(); + named.extend(added.iter().map(|s| s.to_string())); + if for_echo { + named.push("read".into()); + } + let mut expected: Vec<&str> = named.iter().flat_map(|s| asks(s)).copied().chain(["openid"]).collect(); + expected.sort_unstable(); + expected.dedup(); + let mut asked: Vec<&str> = opened["scope"].split(' ').collect(); + asked.sort_unstable(); + assert_eq!((asked, opened["login_hint"].as_str()), (expected, "a")); + // Refreshed with the refresh token it had, when the service answered none. + let refreshes = !(refreshless && signed_out); + assert_eq!(done.account.refreshes, refreshes); + let token = m.token("a"); + let tokens: Vec = m.server.got().into_iter().filter(|g| g.target == "/token").collect(); + match refreshes { + true => { + assert!(token.is_ok_and(|t| t == "at-3"), "not refreshed"); + let spent = if refreshless { "rt-1" } else { "rt-2" }; + assert!(has(&form(tokens.last().unwrap()), "refresh_token", spent), "not the refresh token it had"); + } + false => assert_eq!(token.unwrap_err().code(), "account"), + } +} + /// What resolves an `access` or `scope` error a plugin passes on comes from its manifest and the /// accounts the store keeps, whatever type, plugin or account plugins its fields name. #[test] diff --git a/crates/locallibrary/tests/it/plugins/shared_accounts.rs b/crates/locallibrary/tests/it/plugins/shared_accounts.rs index 9f9821b0c..090cfa381 100644 --- a/crates/locallibrary/tests/it/plugins/shared_accounts.rs +++ b/crates/locallibrary/tests/it/plugins/shared_accounts.rs @@ -168,12 +168,12 @@ enum Step { } /// For any order of signing in as one person or another (by a string or an integer subject), in -/// again as an account kept (the browser maybe signing in as another), out, and refreshing: the -/// store keeps exactly the accounts signed in since they were last signed out, once each, in the -/// order they were first signed in then, each changed in place; the browser is asked to sign in -/// as the account signed in again, or to another when the account plugin has signed in any; -/// signing in again as another says so; and no token gets into `accounts.json`, an error, a -/// listing or a log. +/// again as an account kept or one signed out that the plugin it's for still has access to (the +/// browser maybe signing in as another), out, and refreshing: the store keeps exactly the +/// accounts signed in since they were last signed out, once each, in the order they were first +/// signed in then, each changed in place; the browser is asked to sign in as the account signed in +/// again, or to another when the account plugin has signed in any; signing in again as another +/// says so; and no token gets into `accounts.json`, an error, a listing or a log. #[hegel::test(test_cases = 8, suppress_health_check = [hegel::HealthCheck::TooSlow])] fn an_account_is_kept_once_whoever_signs_in(tc: TestCase) { let m = mock(30); @@ -181,8 +181,9 @@ fn an_account_is_kept_once_whoever_signs_in(tc: TestCase) { let each = people.clone().flat_map(|p| [Step::SignIn(p), Step::SignOut(p), Step::Refresh(p)]); let again = people.clone().flat_map(|of| people.clone().map(move |p| Step::Again(of, p))); let steps = tc.draw(gs::vecs(gs::sampled_from(each.chain(again).collect::>())).max_size(6)); - // The accounts kept: subject to name. + // The accounts kept: subject to name; and those echo has access to, which outlast signing out. let mut kept: IndexMap = IndexMap::new(); + let mut accessed: Vec = Vec::new(); let mut answers = Vec::new(); for (i, step) in steps.into_iter().enumerate() { match step { @@ -193,7 +194,7 @@ fn an_account_is_kept_once_whoever_signs_in(tc: TestCase) { }; let (subject, said, name) = person(p); let (done, opened) = m.sign_in(said, again.as_deref(), &["read"]); - match again.as_ref().filter(|a| !kept.contains_key(*a)) { + match again.as_ref().filter(|a| !kept.contains_key(*a) && !accessed.contains(a)) { Some(_) => { let e = done.unwrap_err(); assert_eq!(e.code(), "not_found", "step {i}: {e}"); @@ -209,6 +210,9 @@ fn an_account_is_kept_once_whoever_signs_in(tc: TestCase) { assert_eq!((done.instead_of.clone(), given), (instead, Some("echo"))); assert_eq!((&done.account.subject, &done.account.name), (&subject, &name), "step {i}"); answers.push(serde_json::to_value(&done).unwrap()); + if !accessed.contains(&subject) { + accessed.push(subject.clone()); + } kept.insert(subject, name); } } From 4176d0820518a202f1844b39b674a3913b4f2728 Mon Sep 17 00:00:00 2001 From: botandbang <337765928+botandbang@users.noreply.github.com> Date: Fri, 30 Oct 2026 13:28:22 -0400 Subject: [PATCH 03/13] feat(locallibrary): give a plugin only the scopes the service granted, saying which it declined --- apps/locallibrary/docs/plugins.md | 7 +- apps/locallibrary/docs/rpc/locallibrary.d.ts | 13 +++- apps/locallibrary/docs/rpc/schema.json | 16 +++- apps/locallibrary/docs/sidecar.md | 17 +++-- apps/locallibrary/src/account_cmd/access.rs | 9 ++- apps/locallibrary/src/account_cmd/accounts.rs | 22 +++++- apps/locallibrary/src/tui/plugins/access.rs | 35 +++++---- .../tests/it/plugins/accounts/cli.rs | 39 +++++++--- .../src/plugin/accounts/access.rs | 26 +++++-- .../src/plugin/accounts/shared.rs | 37 ++++++--- .../locallibrary/tests/it/plugins/accesses.rs | 75 +++++++++++++++++++ .../tests/it/plugins/shared_accounts.rs | 16 ++-- .../locallibrary/tests/support/mock_oauth.rs | 6 +- 13 files changed, 253 insertions(+), 65 deletions(-) diff --git a/apps/locallibrary/docs/plugins.md b/apps/locallibrary/docs/plugins.md index 249d4e74c..ec2674963 100644 --- a/apps/locallibrary/docs/plugins.md +++ b/apps/locallibrary/docs/plugins.md @@ -213,7 +213,12 @@ plugins that have access to it. `locallibrary account TYPE sign-in` opens the se page in the browser and waits for it to come back to this computer (`--no-browser` prints the link, `--paste` takes the address the browser ended up on when it runs on another computer). `--for PLUGIN` gives a plugin access to it, `--with PLUGIN` picks the account plugin when several -provide the type, and `--scope NAME` asks for one of the type's scopes besides. +provide the type, and `--scope NAME` asks for one of the type's scopes besides. A service may let +you leave some of that out on its page: a plugin then goes without the scopes it can do without +that you left out, and the sign-in says which. `locallibrary account TYPE:NAME sign-in` signs in +again as that account, also once it's signed out, asking for what every plugin's access to it +needs; when the browser signs in to someone else, that account is kept too, but the plugin it was +for gets no access to it. `locallibrary account TYPE:NAME sign-out` forgets its tokens and tells the service when it can; plugins keep their access, unused until it's signed in again. diff --git a/apps/locallibrary/docs/rpc/locallibrary.d.ts b/apps/locallibrary/docs/rpc/locallibrary.d.ts index 3445b1fda..8c2107a76 100644 --- a/apps/locallibrary/docs/rpc/locallibrary.d.ts +++ b/apps/locallibrary/docs/rpc/locallibrary.d.ts @@ -7871,6 +7871,12 @@ export type Access = { scopes: string[]; /** It's the plugin's default for the type: `{account:}` names it. */ default: boolean; + /** + * The scopes (by name) a sign-in for it asked the service for, which didn't grant all they + * ask, until it's given them again. Those it needs are in `scopes` too, and still can't be + * used. + */ + declined?: string[]; }; /** A sign-in to an account of a type, waiting for the browser. */ @@ -7911,11 +7917,16 @@ export type AccountSignedIn = { signed_in: boolean; /** * The account it was to sign in again, when the browser signed in to another: that one is - * kept too, and this one as it was. + * kept too, and this one as it was. The plugin it was for isn't given access to it. */ instead_of?: string | null; /** The access it gave the plugin it was signed in for, if it was. */ access?: Access | null; + /** + * The scopes (by name) asked for that plugin that the service didn't grant all they ask: + * those it can do without aren't in its access. + */ + declined?: string[]; }; /** What signing out of an account of a type did. */ diff --git a/apps/locallibrary/docs/rpc/schema.json b/apps/locallibrary/docs/rpc/schema.json index e22acda87..9350fa2c0 100644 --- a/apps/locallibrary/docs/rpc/schema.json +++ b/apps/locallibrary/docs/rpc/schema.json @@ -30403,6 +30403,13 @@ "type": "boolean", "description": "It's the plugin's default for the type: `{account:}` names it.", "default": false + }, + "declined": { + "type": "array", + "items": { + "type": "string" + }, + "description": "The scopes (by name) a sign-in for it asked the service for, which didn't grant all they\nask, until it's given them again. Those it needs are in `scopes` too, and still can't be\nused." } }, "required": [ @@ -30494,7 +30501,7 @@ "string", "null" ], - "description": "The account it was to sign in again, when the browser signed in to another: that one is\nkept too, and this one as it was." + "description": "The account it was to sign in again, when the browser signed in to another: that one is\nkept too, and this one as it was. The plugin it was for isn't given access to it." }, "access": { "anyOf": [ @@ -30506,6 +30513,13 @@ } ], "description": "The access it gave the plugin it was signed in for, if it was." + }, + "declined": { + "type": "array", + "items": { + "type": "string" + }, + "description": "The scopes (by name) asked for that plugin that the service didn't grant all they ask:\nthose it can do without aren't in its access." } }, "required": [ diff --git a/apps/locallibrary/docs/sidecar.md b/apps/locallibrary/docs/sidecar.md index 5eb866c5a..c0d4bcfe9 100644 --- a/apps/locallibrary/docs/sidecar.md +++ b/apps/locallibrary/docs/sidecar.md @@ -361,15 +361,20 @@ the access they're given (see [plugins.md](plugins.md#accounts)). No answer hold `accounts.list` answers `{accounts, accesses}`: each account kept (`{type, plugin, subject, name, scopes, expires?, refreshes, signed_in}`, `plugin` the account plugin it was signed in through), then each one signed out that a plugin still has access to, `signed_in: false`; and each access -(`{id, plugin, type, account: {plugin, subject}, name, scopes, default}`). +(`{id, plugin, type, account: {plugin, subject}, name, scopes, default, declined?}`, `declined` +the scopes the service didn't grant at the last sign-in for that plugin). `accounts.sign_in.begin {type, with?, for?, account?, scopes?}` starts a sign-in through the account plugin `with` (the only one providing the type when it's left out), for the plugin `for` -when it's given, again as the account `account` (a subject) when it's given, asking for the type's -`scopes` besides; it answers `{id, type, plugin, url, redirect_uri}`. Open `url` in the browser, -then `accounts.sign_in.finish {id, timeout_secs?}` waits for it to come back (300 seconds unless it -says, an hour at most) and answers the account, with `instead_of` when the browser signed in to -another than `account`, and `access` when it was for a plugin: the access that plugin was given. +when it's given, again as the account `account` (a subject, of one kept or one signed out that a +plugin still has access to) when it's given, asking for the type's `scopes` besides; signing in +again asks for what every access to the account needs too. It answers `{id, type, plugin, url, +redirect_uri}`. Open `url` in the browser, then `accounts.sign_in.finish {id, timeout_secs?}` +waits for it to come back (300 seconds unless it says, an hour at most) and answers the account, +with `instead_of` when the browser signed in to another than `account` (the plugin it was for +isn't given access then), `access` when it was for a plugin: the access that plugin was given, and +`declined`: the scopes asked for that plugin that the service didn't grant. The plugin gets those +it needs all the same, and goes without the others. `accounts.sign_in.paste {id, url}` finishes with the address the browser ended up on, and `accounts.sign_in.cancel {id}` answers `{cancelled}`. `accounts.sign_out {account: {plugin, subject}}` answers the account as it was, with `revoked`; plugins keep their access to it. diff --git a/apps/locallibrary/src/account_cmd/access.rs b/apps/locallibrary/src/account_cmd/access.rs index 2c628b2ad..f31263576 100644 --- a/apps/locallibrary/src/account_cmd/access.rs +++ b/apps/locallibrary/src/account_cmd/access.rs @@ -14,7 +14,7 @@ use locallibrary::progress::Ctx; use loclib_plugin::{AccountNeed, Choice, ChoiceItem}; use serde_json::json; -use super::accounts::{account_type, called, come_back, find}; +use super::accounts::{account_type, called, come_back, declined, find, titles}; use crate::browser::BrowserArgs; use crate::jobs_cmd::usage; use crate::output::{Output, dim, heading, ok}; @@ -117,8 +117,7 @@ fn give(plugins: &Plugins, plugin: &str, need: &AccountNeed, store: &Store, out: let signed_in: Vec<&Account> = kept.iter().filter(|a| a.kind == *kind && a.signed_in).collect(); let t = providing.iter().find_map(|with| account_type(plugins, with, kind)); let title = t.map_or(kind.as_str(), |t| &t.title); - let scopes: Vec<&str> = - need.scopes.iter().map(|s| t.and_then(|t| t.scopes.get(s)).map_or(s.as_str(), |s| &s.title)).collect(); + let scopes = titles(t, &need.scopes); let several = providing.len() > 1; let mut items: Vec = signed_in .iter() @@ -142,7 +141,9 @@ fn give(plugins: &Plugins, plugin: &str, need: &AccountNeed, store: &Store, out: None => { let with = &providing[i - signed_in.len()]; let start = plugins.begin_account_sign_in(kind, with, Some(plugin), None, &[], store)?; - come_back(plugins, &start, BrowserArgs::default(), out, ctx)?.account + let done = come_back(plugins, &start, BrowserArgs::default(), out, ctx)?; + declined(plugins, &done); + done.account } }; plugins.set_default_access(plugin, &AccountId { plugin: a.plugin.clone(), subject: a.subject.clone() }, store)?; diff --git a/apps/locallibrary/src/account_cmd/accounts.rs b/apps/locallibrary/src/account_cmd/accounts.rs index d992c3ffc..8e2677d73 100644 --- a/apps/locallibrary/src/account_cmd/accounts.rs +++ b/apps/locallibrary/src/account_cmd/accounts.rs @@ -76,7 +76,7 @@ pub(super) fn run( let start = plugins.begin_account_sign_in(kind, &with, args.for_plugin.as_deref(), subject, &args.scopes, &store)?; let done = come_back(plugins, &start, args.browser, out, ctx)?; - return Ok(out.done(&done, |d| signed_in(plugins, d))); + return Ok(out.done(&done, |d| signed_in(plugins, d, args.for_plugin.as_deref()))); } let named = named.ok_or_else(|| usage(format!("name an account: {kind}:NAME (`locallibrary account` lists them)")))?; @@ -150,16 +150,30 @@ pub(super) fn called(plugins: &Plugins, a: &Account) -> String { } } -/// What signing in did. -fn signed_in(plugins: &Plugins, d: &AccountSignedIn) { +/// The titles of `scopes` (by name) as `t` has them, their names where it doesn't. +pub(super) fn titles<'a>(t: Option<&'a AccountType>, scopes: &'a [String]) -> Vec<&'a str> { + scopes.iter().map(|s| t.and_then(|t| t.scopes.get(s)).map_or(s.as_str(), |s| &s.title)).collect() +} + +/// What signing in did, for `for_plugin` when it was for one. +fn signed_in(plugins: &Plugins, d: &AccountSignedIn, for_plugin: Option<&str>) { ok(format!("Signed in to the {}", called(plugins, &d.account))); if let Some(other) = &d.instead_of { - warn(format!("the browser signed in to another account than {other}, which is still kept")); + let unused = for_plugin.map(|p| format!(", so {p} wasn't given access")).unwrap_or_default(); + warn(format!("the browser signed in to another account than {other}, which is still kept{unused}")); } if let Some(access) = &d.access { let default = if access.default { ", its default" } else { "" }; say!(" {}", dim(format!("{} has access to it{default}", access.plugin))); } + declined(plugins, d); +} + +/// What the service declined of what signing in `d` was to give a plugin, if it declined any. +pub(super) fn declined(plugins: &Plugins, d: &AccountSignedIn) { + let Some(access) = d.access.as_ref().filter(|_| !d.declined.is_empty()) else { return }; + let t = account_type(plugins, &d.account.plugin, &d.account.kind); + warn(format!("the service didn't allow {} to {}", access.plugin, titles(t, &d.declined).join(", "))); } /// Each of `all`'s accounts: its type, name and account plugin, whether it's signed in and until diff --git a/apps/locallibrary/src/tui/plugins/access.rs b/apps/locallibrary/src/tui/plugins/access.rs index 6f9269395..0668114f7 100644 --- a/apps/locallibrary/src/tui/plugins/access.rs +++ b/apps/locallibrary/src/tui/plugins/access.rs @@ -69,16 +69,20 @@ fn choices(plugins: &Plugins, m: &Manifest, store: Option<&Store>) -> (Vec String { - let t = plugins.providing(&need.name).iter().find_map(|with| { +/// The titles of the account type `kind`'s `scopes` (by name), as an account plugin providing the +/// type has them. +fn titles(plugins: &Plugins, kind: &str, scopes: &[String]) -> Vec { + let t = plugins.providing(kind).iter().find_map(|with| { let types = &plugins.manifest(with).ok()?.contributes.account_types; - types.iter().find(|t| t.name == need.name).cloned() + types.iter().find(|t| t.name == kind).cloned() }); - let scope = |s: &String| t.as_ref().and_then(|t| t.scopes.get(s)).map_or(s.clone(), |s| s.title.clone()); - let scopes: Vec = need.scopes.iter().map(scope).collect(); - format!("{} needs it to {}: {}", m.name, scopes.join(", "), need.why) + scopes.iter().map(|s| t.as_ref().and_then(|t| t.scopes.get(s)).map_or(s.clone(), |s| s.title.clone())).collect() +} + +/// What the form asks of the account type `need` asks for: what `m` needs it for (its scopes' +/// titles) and why. +fn about(plugins: &Plugins, m: &Manifest, need: &loclib_plugin::AccountNeed) -> String { + format!("{} needs it to {}: {}", m.name, titles(plugins, &need.name, &need.scopes).join(", "), need.why) } impl App { @@ -103,14 +107,15 @@ impl App { } /// Give `plugin` the accounts the form has chosen (`given`) as its defaults, in the - /// background, signing in to those chosen to sign in to first. + /// background, signing in to those chosen to sign in to first, and say what the service + /// declined of it. pub(in crate::tui) fn save_access(&mut self, plugin: &'static str, all: Vec, given: Value) { let Ok(plugins) = crate::plugins::host() else { return }; let Ok(name) = plugins.manifest(plugin).map(|m| m.name.clone()) else { return }; let store_root = self.store_root.clone(); self.task_showing("Giving access".into(), move |ctx, show| { let store = Store::open_or_init(&store_root)?; - let mut used = Vec::new(); + let (mut used, mut declined) = (Vec::new(), Vec::new()); for c in &all { let chosen = given.get(&c.kind).and_then(Value::as_str); let Some((_, pick)) = c.picks.iter().find(|(l, _)| Some(l.as_str()) == chosen) else { continue }; @@ -119,14 +124,18 @@ impl App { Pick::SignIn(with) => { let start = plugins.begin_account_sign_in(&c.kind, with, Some(plugin), None, &[], &store)?; open(&start.url, show); - let a = plugins.finish_account_sign_in(&start.id, BROWSER_WAIT, ctx)?.account; - AccountId { plugin: a.plugin, subject: a.subject } + let done = plugins.finish_account_sign_in(&start.id, BROWSER_WAIT, ctx)?; + declined.extend(titles(plugins, &c.kind, &done.declined)); + AccountId { plugin: done.account.plugin, subject: done.account.subject } } }; let access = plugins.set_default_access(plugin, &account, &store)?; used.push(access.name); } - let said = format!("{name} uses {}", used.join(", ")); + let mut said = format!("{name} uses {}", used.join(", ")); + if !declined.is_empty() { + said.push_str(&format!("; the service didn't allow it to {}", declined.join(", "))); + } Ok(Box::new(move |app: &mut App| app.toast(said, Level::Info))) }); } diff --git a/apps/locallibrary/tests/it/plugins/accounts/cli.rs b/apps/locallibrary/tests/it/plugins/accounts/cli.rs index 4ad46ff07..27afc1f4b 100644 --- a/apps/locallibrary/tests/it/plugins/accounts/cli.rs +++ b/apps/locallibrary/tests/it/plugins/accounts/cli.rs @@ -436,10 +436,11 @@ fn echo_accesses(m: &Mock, two: bool) -> Value { answered } -/// For one account plugin providing the type or two, a service that says what it granted or not, -/// revokes or not and whose tokens are refreshed, last an hour or last, accounts named by subject, -/// name or TYPE:SUBJECT, and `--json` or not: signing in for echo and not, again as a kept account -/// (the browser maybe approving another), each account's status, echo's accesses, making one its +/// For one account plugin providing the type or two, a service that says what it granted (maybe +/// not what echo can do without) or not, revokes or not and whose tokens are refreshed, last an +/// hour or last, accounts named by subject, name or TYPE:SUBJECT, and `--json` or not: signing in +/// for echo (saying what the service declined) and not, again as a kept account (the browser +/// maybe approving another), each account's status, echo's accesses, making one its /// default, signing out and taking the access away say in words what they answer as JSON; a name /// two accounts share is refused, naming their subjects; with two account plugins, signing in /// without `--with` is refused, naming them, and so is signing in for echo with a scope it doesn't @@ -456,7 +457,8 @@ fn echo_accesses(m: &Mock, two: bool) -> Value { a_with = TOO, b_with = WITH, same_name = true, - instead = true + instead = true, + grants_write = true )] #[hegel::explicit_test_case( two = false, @@ -468,7 +470,8 @@ fn echo_accesses(m: &Mock, two: bool) -> Value { a_with = WITH, b_with = WITH, same_name = false, - instead = true + instead = true, + grants_write = false )] #[hegel::explicit_test_case( two = true, @@ -480,7 +483,8 @@ fn echo_accesses(m: &Mock, two: bool) -> Value { a_with = WITH, b_with = TOO, same_name = true, - instead = false + instead = false, + grants_write = false )] fn every_account_command_says_in_words_what_it_answers_as_json(tc: TestCase) { let two = tc.draw(gs::booleans()); @@ -496,9 +500,13 @@ fn every_account_command_says_in_words_what_it_answers_as_json(tc: TestCase) { let b_with = tc.draw(gs::sampled_from(providers)); let same_name = tc.draw(gs::booleans()); let instead = tc.draw(gs::booleans()); + // Whether the service grants what echo's optional `write` asks, which a is signed in for. + let grants_write = tc.draw(gs::booleans()); + let granted = if grants_write || unscoped { "read write" } else { "read" }; { let mut s = m.state.lock().unwrap(); s.unscoped = unscoped; + s.grants = Some(granted.split(' ').map(String::from).collect()); s.refreshless = !matches!(lasting, Lasting::Refreshed); s.expires_in = (!matches!(lasting, Lasting::Ever)).then_some(3600); } @@ -533,19 +541,28 @@ fn every_account_command_says_in_words_what_it_answers_as_json(tc: TestCase) { let a = Kept { subject: "a", name: "a@mock.example".into(), with: a_with }; let b_name = if same_name { a.name.clone() } else { "b@mock.example".into() }; let b = Kept { subject: "b", name: b_name, with: b_with }; - for (k, more) in [(&a, &["--for", "echo"][..]), (&b, &[])] { + let declined = !grants_write && !unscoped; + for (k, more) in [(&a, &["--for", "echo", "--scope", "write"][..]), (&b, &[])] { let mut c = m.command(); c.args(maybe_json(json, &["account", "mock", "sign-in", "--no-browser", "--timeout", "10", "--with", k.with])); c.args(more); let (code, out, err) = m.approving(c, &json!({ "sub": k.subject, "email": k.name })); assert_eq!(code, 0, "{err}"); + let for_echo = !more.is_empty(); match json { true => { let done = report(&out); assert_eq!((&done["subject"], &done["plugin"]), (&json!(k.subject), &json!(k.with)), "{done}"); - assert_eq!(done["access"]["default"], if more.is_empty() { Value::Null } else { json!(true) }); + assert_eq!(done["access"]["default"], if for_echo { json!(true) } else { Value::Null }); + let given = if declined { json!(["read"]) } else { json!(["read", "write"]) }; + assert_eq!(done["access"]["scopes"], if for_echo { given } else { Value::Null }, "{done}"); + let left_out = if declined && for_echo { json!(["write"]) } else { Value::Null }; + assert_eq!((&done["declined"], &done["access"]["declined"]), (&left_out, &left_out), "{done}"); + } + false => { + assert!(out.starts_with(&format!("✓ Signed in to the {}\n", called(k))), "{out}"); + assert_eq!(out.contains("! the service didn't allow echo to write\n"), declined && for_echo, "{out}"); } - false => assert!(out.starts_with(&format!("✓ Signed in to the {}\n", called(k))), "{out}"), } } let mut kept = vec![a.clone(), b.clone()]; @@ -608,7 +625,7 @@ fn every_account_command_says_in_words_what_it_answers_as_json(tc: TestCase) { }; // What the service granted, or when it doesn't say, what was asked. let allowed = joined(&account["scopes"], " "); - assert_eq!(allowed == "read write", !unscoped, "{allowed}"); + assert_eq!(allowed == granted, !unscoped, "{allowed}"); let access = match one["accesses"].as_array().unwrap().is_empty() { true => "", false => " access: echo (default)\n", diff --git a/crates/locallibrary/src/plugin/accounts/access.rs b/crates/locallibrary/src/plugin/accounts/access.rs index b5701f083..c891a1a37 100644 --- a/crates/locallibrary/src/plugin/accounts/access.rs +++ b/crates/locallibrary/src/plugin/accounts/access.rs @@ -35,6 +35,11 @@ pub struct Access { /// It's the plugin's default for the type: `{account:}` names it. #[serde(default)] pub default: bool, + /// The scopes (by name) a sign-in for it asked the service for, which didn't grant all they + /// ask, until it's given them again. Those it needs are in `scopes` too, and still can't be + /// used. + #[serde(default, skip_serializing_if = "Vec::is_empty")] + pub declined: Vec, } /// An account kept in a store: the account plugin that signed it in, and its subject there. @@ -61,9 +66,10 @@ fn new_id(taken: &[Access]) -> String { } impl Book { - /// Gives `plugin` access to `k`'s `scopes` besides those it has: the access it has, or a new - /// one, its default for the type when it has none of the type. - pub(super) fn give(&mut self, plugin: &str, k: &Kept, scopes: &[String]) -> Access { + /// Gives `plugin` access to `k`'s `scopes` besides those it has, remembering that the service + /// `declined` those (and no longer any it's given): the access it has, or a new one, its + /// default for the type when it has none of the type. + pub(super) fn give(&mut self, plugin: &str, k: &Kept, scopes: &[String], declined: &[String]) -> Access { let account = AccountId::of(k); let i = match self.accesses.iter().position(|a| a.plugin == plugin && a.account == account) { Some(i) => i, @@ -71,7 +77,8 @@ impl Book { let first = !self.accesses.iter().any(|a| a.plugin == plugin && a.kind == k.kind); let id = new_id(&self.accesses); let (plugin, kind, name) = (plugin.into(), k.kind.clone(), k.name.clone()); - self.accesses.push(Access { id, plugin, kind, account, name, scopes: Vec::new(), default: first }); + let (scopes, declined) = (Vec::new(), Vec::new()); + self.accesses.push(Access { id, plugin, kind, account, name, scopes, default: first, declined }); self.accesses.len() - 1 } }; @@ -79,6 +86,9 @@ impl Book { let mut all: IndexSet = a.scopes.drain(..).collect(); all.extend(scopes.iter().cloned()); a.scopes = all.into_iter().collect(); + let mut still: IndexSet = a.declined.drain(..).filter(|s| !scopes.contains(s)).collect(); + still.extend(declined.iter().cloned()); + a.declined = still.into_iter().collect(); a.clone() } @@ -125,15 +135,15 @@ impl Plugins { } /// Gives `plugin` access to `account` (one `store` keeps): to the scopes it needs of the - /// account's type, and those of `scopes` it can do without, besides what it has. Its first - /// access of the type is its default. + /// account's type, and those of `scopes` it can do without, besides what it has, none of them + /// declined any more. Its first access of the type is its default. pub fn give_access(&self, plugin: &str, account: &AccountId, scopes: &[String], store: &Store) -> Result { let slot = self.slot(plugin)?; kept::update(store, |book| { let k = book.account(&account.plugin, &account.subject).cloned(); let k = k.ok_or_else(|| not_kept(&account.plugin, &account.subject))?; let given = slot.given(&k.kind, scopes)?; - Ok(book.give(plugin, &k, &given)) + Ok(book.give(plugin, &k, &given, &[])) }) } @@ -148,7 +158,7 @@ impl Plugins { let k = book.account(&account.plugin, &account.subject).cloned(); let k = k.ok_or_else(|| not_kept(&account.plugin, &account.subject))?; let given = slot.given(&k.kind, &[])?; - book.give(plugin, &k, &given); + book.give(plugin, &k, &given, &[]); book.accesses.len() - 1 } }; diff --git a/crates/locallibrary/src/plugin/accounts/shared.rs b/crates/locallibrary/src/plugin/accounts/shared.rs index 102baec75..e854cffd9 100644 --- a/crates/locallibrary/src/plugin/accounts/shared.rs +++ b/crates/locallibrary/src/plugin/accounts/shared.rs @@ -82,12 +82,16 @@ pub struct AccountSignedIn { #[serde(flatten)] pub account: Account, /// The account it was to sign in again, when the browser signed in to another: that one is - /// kept too, and this one as it was. + /// kept too, and this one as it was. The plugin it was for isn't given access to it. #[serde(skip_serializing_if = "Option::is_none")] pub instead_of: Option, /// The access it gave the plugin it was signed in for, if it was. #[serde(skip_serializing_if = "Option::is_none")] pub access: Option, + /// The scopes (by name) asked for that plugin that the service didn't grant all they ask: + /// those it can do without aren't in its access. + #[serde(skip_serializing_if = "Vec::is_empty")] + pub declined: Vec, } /// What signing out of an account of a type did. @@ -105,7 +109,8 @@ pub(crate) struct Asked { kind: String, /// The account plugin signing in. with: String, - /// The plugin it's for, and the type's scopes (by name) it's given of the account. + /// The plugin it's for, and the type's scopes (by name) it's given of the account as far as + /// the service grants them. for_plugin: Option<(String, Vec)>, /// The account signed in again, by subject. account: Option, @@ -255,8 +260,8 @@ impl Plugins { /// any; the type's own params go with either. It asks the service for what the type always /// asks for, what `account` was granted and what every access to it needs, and what `scopes` /// (the type's, by name) ask for, and for a plugin, what the scopes it needs ask for: it's - /// given access to those, and to those of `scopes` it can do without (see - /// [`Plugins::give_access`]). Finished with [`Plugins::finish_account_sign_in`] (or + /// given access to those, and to those of `scopes` it can do without that the service grants + /// (see [`Plugins::give_access`]). Finished with [`Plugins::finish_account_sign_in`] (or /// [`Plugins::finish_account_sign_in_with_url`], when the browser runs on another computer), /// within a quarter of an hour. pub fn begin_account_sign_in( @@ -338,6 +343,15 @@ impl Plugins { let access_token = answer.access_token().secret(); slot.net.saw(access_token); let (subject, name) = slot.whoami(t, access_token)?; + let instead_of = asked.account.filter(|a| *a != subject); + // The plugin it's for is given the scopes it needs whatever the service grants (a request + // through one not granted says to sign in again), those it can do without only when + // granted, and nothing when the browser signed in to another account than asked. + let for_plugin = asked.for_plugin.filter(|_| instead_of.is_none()); + let needs = match &for_plugin { + Some((plugin, _)) => self.slot(plugin)?.needs(&asked.kind)?.scopes.clone(), + None => Vec::new(), + }; let (store, storage) = (&p.store, self.host.options.secrets); let _one = kept::hold(store, &asked.with, &subject)?; // Signed in again, it keeps the refresh token it had when the service answers none. One @@ -350,7 +364,7 @@ impl Plugins { tokens.scopes = asked.scopes; } slot.seen(&tokens); - let (account, access) = kept::update(store, |book| { + let (account, access, declined) = kept::update(store, |book| { let all = &mut book.accounts; let i = match all.iter().position(|k| k.is(&asked.with, &subject)) { Some(i) => i, @@ -365,11 +379,16 @@ impl Plugins { kept::keep(storage, store, k, Some(&tokens))?; let k = k.clone(); book.renamed(&k); - let access = asked.for_plugin.as_ref().map(|(plugin, scopes)| book.give(plugin, &k, scopes)); - Ok((listed(k, true), access)) + let Some((plugin, scopes)) = &for_plugin else { return Ok((listed(k, true), None, Vec::new())) }; + let is_granted = + |name: &String| t.scopes.get(name).is_some_and(|s| s.asks.iter().all(|a| k.scopes.contains(a))); + let declined: Vec = scopes.iter().filter(|s| !is_granted(s)).cloned().collect(); + let given: Vec = + scopes.iter().filter(|s| needs.contains(s) || !declined.contains(s)).cloned().collect(); + let access = book.give(plugin, &k, &given, &declined); + Ok((listed(k, true), Some(access), declined)) })?; - let instead_of = asked.account.filter(|a| *a != subject); - Ok(AccountSignedIn { account, instead_of, access }) + Ok(AccountSignedIn { account, instead_of, access, declined }) } /// The access token of the account `subject` that `with` signed in to in `store`, as a diff --git a/crates/locallibrary/tests/it/plugins/accesses.rs b/crates/locallibrary/tests/it/plugins/accesses.rs index b4000d417..9cf1fd0fa 100644 --- a/crates/locallibrary/tests/it/plugins/accesses.rs +++ b/crates/locallibrary/tests/it/plugins/accesses.rs @@ -164,6 +164,81 @@ fn a_token_goes_only_where_an_access_scope_covers(tc: TestCase) { assert!(!m.leaked(&[answer["error"].clone()]), "a token is out where it may not be"); } +/// `scopes`, sorted, to compare as a set. +fn sorted>(scopes: &[S]) -> Vec<&str> { + let mut all: Vec<&str> = scopes.iter().map(AsRef::as_ref).collect(); + all.sort_unstable(); + all.dedup(); + all +} + +/// For an echo plugin needing any of the mock type's scopes (the rest optional) and any sign-ins +/// for it, as either of two people, maybe again as an account kept, asking for any of the scopes +/// it can do without, the service granting any of what they ask (or not saying, so granting what +/// it was asked): its access gets the scopes it needs, whatever was granted, and those it can do +/// without whose asks were all granted; the sign-in says which scopes the service declined, and +/// the access remembers them until a sign-in for it grants them; and a sign-in again that comes +/// back as another person keeps that account but gives echo no access to it. +#[hegel::test(test_cases = 12, suppress_health_check = [hegel::HealthCheck::TooSlow])] +fn a_plugin_is_given_only_what_the_service_granted(tc: TestCase) { + let mut needs = some(&tc); + if needs.is_empty() { + needs.push(tc.draw(gs::sampled_from(SCOPES.to_vec()))); + } + let optional: Vec<&str> = SCOPES.into_iter().filter(|s| !needs.contains(s)).collect(); + let m = mock_for(3600, "/userinfo", "", &asking(&needs, &optional)); + // Echo's access to each of `SUBJECTS[..2]`, if it has one: its scopes, and those declined. + let mut model: [Option<(Vec<&str>, Vec<&str>)>; 2] = [None, None]; + let mut known = Vec::new(); + for step in 0..tc.draw(gs::integers::().min_value(1).max_value(3)) { + let p = tc.draw(gs::integers::().max_value(1)); + let again = match known.is_empty() { + true => None, + false => tc.draw(gs::optional(gs::sampled_from(known.clone()))), + }; + let asked: Vec<&str> = optional.iter().copied().filter(|_| tc.draw(gs::booleans())).collect(); + let grants: Option> = tc + .draw(gs::booleans()) + .then(|| ["read", "write"].into_iter().filter(|_| tc.draw(gs::booleans())).collect()); + { + let mut s = m.state.lock().unwrap(); + s.unscoped = grants.is_none(); + s.grants = grants.as_ref().map(|g| g.iter().map(|s| s.to_string()).collect()); + } + let again_as = again.map(|j: usize| SUBJECTS[j]); + let done = m.sign_in(who(json!(SUBJECTS[p]), None), again_as, &asked).0.unwrap(); + if !known.contains(&p) { + known.push(p); + } + if again.is_some_and(|j| j != p) { + assert_eq!((done.instead_of.as_deref(), &done.access, done.declined.len()), (again_as, &None, 0)); + assert!(m.listed().iter().any(|a| a.subject == SUBJECTS[p]), "step {step}: not kept"); + } else { + let given: Vec<&str> = needs.iter().chain(&asked).copied().collect(); + let granted = |s: &str| grants.as_ref().is_none_or(|g| asks(s).iter().all(|a| g.contains(a))); + let declined: Vec<&str> = given.iter().copied().filter(|s| !granted(s)).collect(); + let giving: Vec<&str> = given.iter().copied().filter(|s| needs.contains(s) || granted(s)).collect(); + let (scopes, was) = model[p].take().unwrap_or_default(); + let still: Vec<&str> = was.into_iter().filter(|s| !giving.contains(s)).chain(declined.clone()).collect(); + let now = ([scopes, giving].concat(), still); + assert_eq!(sorted(&done.declined), sorted(&declined), "step {step}"); + let access = done.access.unwrap(); + assert_eq!( + (sorted(&access.scopes), sorted(&access.declined)), + (sorted(&now.0), sorted(&now.1)), + "step {step}" + ); + model[p] = Some(now); + } + for (i, expected) in model.iter().enumerate() { + let listed = m.f.plugins.accesses(&m.f.store, Some("echo"), Some(&account(SUBJECTS[i]))).unwrap(); + let said = listed.first().map(|a| (sorted(&a.scopes), sorted(&a.declined))); + let expected = expected.as_ref().map(|(scopes, declined)| (sorted(scopes), sorted(declined))); + assert_eq!(said, expected, "step {step}"); + } + } +} + /// A step of [`the_default_access_answers_the_type_alone`], naming accounts by their place in /// [`SUBJECTS`]. #[derive(Debug, Clone, Copy)] diff --git a/crates/locallibrary/tests/it/plugins/shared_accounts.rs b/crates/locallibrary/tests/it/plugins/shared_accounts.rs index 090cfa381..bc5502174 100644 --- a/crates/locallibrary/tests/it/plugins/shared_accounts.rs +++ b/crates/locallibrary/tests/it/plugins/shared_accounts.rs @@ -173,7 +173,8 @@ enum Step { /// accounts signed in since they were last signed out, once each, in the order they were first /// signed in then, each changed in place; the browser is asked to sign in as the account signed in /// again, or to another when the account plugin has signed in any; signing in again as another -/// says so; and no token gets into `accounts.json`, an error, a listing or a log. +/// says so and gives the plugin it was for no access to it; and no token gets into +/// `accounts.json`, an error, a listing or a log. #[hegel::test(test_cases = 8, suppress_health_check = [hegel::HealthCheck::TooSlow])] fn an_account_is_kept_once_whoever_signs_in(tc: TestCase) { let m = mock(30); @@ -206,11 +207,13 @@ fn an_account_is_kept_once_whoever_signs_in(tc: TestCase) { let select = (again.is_none() && !kept.is_empty()).then_some("select_account"); assert_eq!(opened.get("prompt").map(String::as_str), select, "step {i}"); let instead = again.clone().filter(|a| *a != subject); + // The plugin it was for is given access only to the account it was for. let given = done.access.as_ref().map(|a| a.plugin.as_str()); - assert_eq!((done.instead_of.clone(), given), (instead, Some("echo"))); + let expected = instead.is_none().then_some("echo"); + assert_eq!((done.instead_of.clone(), given), (instead, expected), "step {i}"); assert_eq!((&done.account.subject, &done.account.name), (&subject, &name), "step {i}"); answers.push(serde_json::to_value(&done).unwrap()); - if !accessed.contains(&subject) { + if given.is_some() && !accessed.contains(&subject) { accessed.push(subject.clone()); } kept.insert(subject, name); @@ -358,7 +361,8 @@ fn a_service_that_doesnt_say_what_it_granted_granted_what_was_asked() { /// The browser is asked to sign in as the account signed in again, or to another than those the /// account plugin has when there are any, and to nothing in particular before; the type's own -/// params go with every sign-in. +/// params go with every sign-in; and the plugin a sign-in again is for isn't given access to +/// another account the browser signs in to. #[test] fn the_browser_is_asked_for_one_account_or_another() { let m = mock_at(3600, "/userinfo", "params = { access_type = \"offline\" }"); @@ -369,7 +373,9 @@ fn the_browser_is_asked_for_one_account_or_another() { let (done, again) = m.sign_in(who(json!("b"), None), Some("a"), &[]); assert_eq!((again.get("prompt"), again["login_hint"].as_str()), (None, "a")); assert!([first, another, again].iter().all(|q| has(q, "access_type", "offline")), "the type's params"); - assert_eq!(done.unwrap().instead_of.as_deref(), Some("a"), "signed in as another than asked"); + let done = done.unwrap(); + assert_eq!(done.instead_of.as_deref(), Some("a"), "signed in as another than asked"); + assert_eq!(done.access, None, "echo was given access to another account than it was for"); let subjects: Vec = m.listed().into_iter().map(|a| a.subject).collect(); assert_eq!(subjects, ["a", "b"]); } diff --git a/crates/locallibrary/tests/support/mock_oauth.rs b/crates/locallibrary/tests/support/mock_oauth.rs index 05aad6727..c4f933809 100644 --- a/crates/locallibrary/tests/support/mock_oauth.rs +++ b/crates/locallibrary/tests/support/mock_oauth.rs @@ -111,6 +111,8 @@ pub struct Service { pub spent: Vec, /// The token endpoint doesn't say what it granted. pub unscoped: bool, + /// What the token endpoint says it granted, when it isn't `read write`. + pub grants: Option>, } /// The form a request sent, by name. @@ -193,8 +195,8 @@ pub fn service(state: Arc>) -> Server { None => s.signing_in_as.clone(), }; s.who.insert(n, who); - let mut answer = - json!({ "access_token": format!("at-{n}"), "token_type": "bearer", "scope": "read write" }); + let granted = s.grants.as_ref().map_or("read write".into(), |g| g.join(" ")); + let mut answer = json!({ "access_token": format!("at-{n}"), "token_type": "bearer", "scope": granted }); if s.unscoped { answer.as_object_mut().unwrap().remove("scope"); } From c843be06e3b1e779d706ccf5c743d40ac5847724 Mon Sep 17 00:00:00 2001 From: botandbang <337765928+botandbang@users.noreply.github.com> Date: Fri, 30 Oct 2026 13:28:23 -0400 Subject: [PATCH 04/13] feat(locallibrary): offer signing in again as the account, and giving a plugin more scopes --- apps/locallibrary/docs/commands.md | 2 +- apps/locallibrary/docs/plugins.md | 7 +- apps/locallibrary/docs/rpc/locallibrary.d.ts | 10 +++ apps/locallibrary/docs/rpc/schema.json | 44 ++++++++++++ apps/locallibrary/docs/sidecar.md | 13 ++-- apps/locallibrary/src/account_cmd/access.rs | 30 +++++--- apps/locallibrary/src/hints.rs | 7 +- apps/locallibrary/src/hints/tests.rs | 9 +++ apps/locallibrary/src/plugin_cmd.rs | 16 +++-- apps/locallibrary/src/rpc/methods/accounts.rs | 14 ++++ .../tests/it/plugins/accounts/cli.rs | 13 +++- .../tests/it/plugins/shared_accounts.rs | 21 +++++- .../src/plugin/accounts/access.rs | 69 +++++++++++++++++-- .../locallibrary/src/plugin/accounts/token.rs | 47 ++++++++----- crates/locallibrary/src/plugin/process.rs | 7 +- .../locallibrary/tests/it/plugins/accesses.rs | 44 ++++++++++-- 16 files changed, 297 insertions(+), 56 deletions(-) diff --git a/apps/locallibrary/docs/commands.md b/apps/locallibrary/docs/commands.md index da8dddf9d..625c4051e 100644 --- a/apps/locallibrary/docs/commands.md +++ b/apps/locallibrary/docs/commands.md @@ -193,7 +193,7 @@ its drives. | `locallibrary account [login\|logout\|status]` | Sign in with your Atmosphere account as the DJ account (the one you use for Bluesky and other AT Protocol apps; alpha), see [Atmosphere account](account.md) | | `locallibrary account sign-in [--with PLUGIN] [--for PLUGIN] [--scope NAME]...` | Sign in to an account of a type (`google`) in the browser, through an account plugin, giving the plugin `--for` names access to it, see [Accounts](plugins.md#accounts) | | `locallibrary account : [status\|sign-out]` | One of those accounts, or sign out of it (plugins keep their access, unused until it's signed in again) | -| `locallibrary plugin access [--default ACCOUNT \| --take-away ACCOUNT]` | A plugin's access to your accounts: make one its default for its type, or take its access away, see [Accounts](plugins.md#accounts) | +| `locallibrary plugin access [--give ACCOUNT [--scope S] \| --default ACCOUNT \| --take-away ACCOUNT]` | A plugin's access to your accounts: give it access to one, make one its default for its type, or take its access away, see [Accounts](plugins.md#accounts) | | `locallibrary plugin [list\|info ID\|settings ID [KEY [VALUE \| --reset \| --secret]]\|roles [ROLE [PLUGIN.ANALYZER \| --reset]]\|checks [PLUGIN.CHECK [--on\|--off] [--blocking\|--no-blocking] [--reset]] [--timeout SECS]\|providers [--auto-pick SCORE \| --reset]]` | The plugins locallibrary has, what each may do and adds, their settings (`--secret` reads a secret one from stdin, kept in the keychain; `services.NAME on` turns a service on), which analyzer each role (a track's key, tempo, level) is read from, which of their checks run with `check` and `doctor` and which guards stop changes they find something about, and the providers `art fetch` and `track fetch` ask, with the score one's candidate is chosen for you at; the commands and operations they add are `locallibrary ID NAME` or where the plugin puts them (`--choose PARAM=VALUE` answers what one asks), see [Plugins](plugins.md#roles) | | `locallibrary bug [--title T] [-m TEXT] [--no-browser]` | Report a bug, with this version and system filled in: posted from your Atmosphere account to userinput.app, or on GitHub's bug report form, see [Reporting a bug](account.md#reporting-a-bug) | | `locallibrary completions SHELL [--static]` | Shell completion for bash, zsh, fish, elvish or PowerShell, see [Shell completion and man pages](install.md#shell-completion-and-man-pages) | diff --git a/apps/locallibrary/docs/plugins.md b/apps/locallibrary/docs/plugins.md index ec2674963..bb60a51e4 100644 --- a/apps/locallibrary/docs/plugins.md +++ b/apps/locallibrary/docs/plugins.md @@ -222,8 +222,11 @@ for gets no access to it. `locallibrary account TYPE:NAME sign-out` forgets its tokens and tells the service when it can; plugins keep their access, unused until it's signed in again. -`locallibrary plugin access PLUGIN` lists a plugin's access, `--default ACCOUNT` makes an account -the one it uses for that type, and `--take-away ACCOUNT` takes its access away. Before a plugin's +`locallibrary plugin access PLUGIN` lists a plugin's access, `--give ACCOUNT` gives it access to +an account (`--scope NAME` adds a scope it can do without), `--default ACCOUNT` makes an account +the one it uses for that type, and `--take-away ACCOUNT` takes its access away. When a plugin's +request fails because its account is signed out or wasn't granted a scope, the error offers to +sign in again as that account, allowing what's missing. Before a plugin's destination is used, a plugin with no access to a type it needs asks in the terminal which of your accounts it may use, or to sign in to another. With `--json`, `--yes` or no terminal, and in the sidecar, nothing is asked: it fails and says how to give it access. diff --git a/apps/locallibrary/docs/rpc/locallibrary.d.ts b/apps/locallibrary/docs/rpc/locallibrary.d.ts index 8c2107a76..70babe1e3 100644 --- a/apps/locallibrary/docs/rpc/locallibrary.d.ts +++ b/apps/locallibrary/docs/rpc/locallibrary.d.ts @@ -2301,6 +2301,14 @@ export type ListAccesses = { plugin?: string | null; }; +export type GiveAccess = { + /** The plugin's id. */ + plugin: string; + account: AccountId; + /** Scopes of the account's type it can do without to give it besides those it needs, by name. */ + scopes?: string[]; +}; + export type AccessRef = { /** The plugin's id. */ plugin: string; @@ -9974,6 +9982,8 @@ export interface Methods { /** read method */ "accounts.access.list": { params: ListAccesses; result: Access[] }; /** immediate method */ + "accounts.access.give": { params: GiveAccess & WriteOptions; result: Access }; + /** immediate method */ "accounts.access.set_default": { params: AccessRef & WriteOptions; result: Access }; /** immediate method */ "accounts.access.take_away": { params: AccessRef & WriteOptions; result: Access }; diff --git a/apps/locallibrary/docs/rpc/schema.json b/apps/locallibrary/docs/rpc/schema.json index 9350fa2c0..02c5a5b58 100644 --- a/apps/locallibrary/docs/rpc/schema.json +++ b/apps/locallibrary/docs/rpc/schema.json @@ -4962,6 +4962,26 @@ } } }, + { + "name": "accounts.access.give", + "aliases": [], + "kind": "immediate", + "long_running": false, + "destructive": false, + "params": { + "allOf": [ + { + "$ref": "#/$defs/GiveAccess" + }, + { + "$ref": "#/$defs/WriteOptions" + } + ] + }, + "result": { + "$ref": "#/$defs/Access" + } + }, { "name": "accounts.access.set_default", "aliases": [], @@ -11970,6 +11990,30 @@ } } }, + "GiveAccess": { + "type": "object", + "properties": { + "plugin": { + "type": "string", + "description": "The plugin's id." + }, + "account": { + "$ref": "#/$defs/AccountId" + }, + "scopes": { + "type": "array", + "items": { + "type": "string" + }, + "description": "Scopes of the account's type it can do without to give it besides those it needs, by name.", + "default": [] + } + }, + "required": [ + "plugin", + "account" + ] + }, "AccessRef": { "type": "object", "properties": { diff --git a/apps/locallibrary/docs/sidecar.md b/apps/locallibrary/docs/sidecar.md index c0d4bcfe9..229ee7f90 100644 --- a/apps/locallibrary/docs/sidecar.md +++ b/apps/locallibrary/docs/sidecar.md @@ -379,11 +379,14 @@ it needs all the same, and goes without the others. `accounts.sign_in.cancel {id}` answers `{cancelled}`. `accounts.sign_out {account: {plugin, subject}}` answers the account as it was, with `revoked`; plugins keep their access to it. -`accounts.access.list {plugin?}` answers the accesses, `accounts.access.set_default {plugin, -account}` makes one the plugin's default for its type (giving the access first when it has none), -and `accounts.access.take_away {plugin, account}` answers the access taken away. A plugin's -`account` error with the reason `no_access`, `not_signed_in` or `missing_scope` carries actions -calling `accounts.sign_in.begin` and `accounts.access.set_default`. +`accounts.access.list {plugin?}` answers the accesses, `accounts.access.give {plugin, account, +scopes?}` gives a plugin access to an account (the scopes it needs, and those of `scopes` it can do +without, which aren't declined any more), `accounts.access.set_default {plugin, account}` makes +one the plugin's default for its type (giving the access first when it has none), and +`accounts.access.take_away {plugin, account}` answers the access taken away. A plugin's `account` +error with the reason `no_access`, `not_signed_in` or `missing_scope` carries actions calling +`accounts.sign_in.begin` (again as the account of the plugin's access, for the scopes it's missing, +when that can help) and `accounts.access.set_default`. ## Plugins diff --git a/apps/locallibrary/src/account_cmd/access.rs b/apps/locallibrary/src/account_cmd/access.rs index f31263576..77204b3bc 100644 --- a/apps/locallibrary/src/account_cmd/access.rs +++ b/apps/locallibrary/src/account_cmd/access.rs @@ -21,19 +21,25 @@ use crate::output::{Output, dim, heading, ok}; use crate::plugin_cmd::choose::{answer, interactive}; use crate::plugins; +/// What `plugin access PLUGIN` changes, at most one of: the account made its default, the one its +/// access is taken away from, or the one it's given access to, with the scopes it can do without +/// that it's given besides. +pub struct Change { + pub default: Option, + pub take_away: Option, + pub give: Option, + pub scopes: Vec, +} + /// `plugin access PLUGIN`: its accesses; with `--default ACCOUNT`, that account made its default -/// for its type; with `--take-away ACCOUNT`, its access to it taken away. -pub fn access( - plugin: &str, - default: Option, - take_away: Option, - store_path: &Option, - out: Output, -) -> Result { +/// for its type; with `--take-away ACCOUNT`, its access to it taken away; with `--give ACCOUNT`, +/// access to it given (no longer declined). +pub fn access(plugin: &str, change: Change, store_path: &Option, out: Output) -> Result { let plugins = plugins::host()?; let manifest = plugins.manifest(plugin)?; let root = store_path.clone().unwrap_or_else(Store::default_root); - let Some(named) = default.as_deref().or(take_away.as_deref()) else { + let Change { default, take_away, give, scopes } = change; + let Some(named) = default.as_deref().or(take_away.as_deref()).or(give.as_deref()) else { let store = Store::open_existing(&root)?; let accesses = store.as_ref().map(|s| plugins.accesses(s, Some(plugin), None)).transpose()?; out.emit(&accesses.unwrap_or_default(), |a| list(plugins, plugin, &manifest.accounts, a)); @@ -49,6 +55,12 @@ pub fn access( let access = plugins.set_default_access(plugin, &account, &store)?; return Ok(out.done(&access, |_| ok(format!("{plugin} uses the {}", called(plugins, a))))); } + if give.is_some() { + let access = plugins.give_access(plugin, &account, &scopes, &store)?; + let gave = + |x: &Access| ok(format!("Gave {plugin} access to the {}: {}", called(plugins, a), x.scopes.join(", "))); + return Ok(out.done(&access, gave)); + } let access = plugins.take_away_access(plugin, &account, &store)?; Ok(out.done(&access, |_| ok(format!("Took {plugin}'s access to the {} away", called(plugins, a))))) } diff --git a/apps/locallibrary/src/hints.rs b/apps/locallibrary/src/hints.rs index c702ccfd3..6b493ab2f 100644 --- a/apps/locallibrary/src/hints.rs +++ b/apps/locallibrary/src/hints.rs @@ -97,12 +97,17 @@ fn command(a: &Action) -> Option { "account.login.begin" if yes("publish") => "locallibrary account login --publish".into(), "account.login.begin" => "locallibrary account login".into(), "accounts.sign_in.begin" => { + // Again as an account, named TYPE:SUBJECT. + let target = match a.params.get("account").and_then(Value::as_str) { + Some(subject) => quote(&format!("{}:{subject}", a.params.get("type")?.as_str()?)), + None => p("type")?, + }; let flags = [("with", p("with")), ("for", p("for"))] .into_iter() .filter_map(|(f, v)| Some(format!(" --{f} {}", v?))); let scopes = a.params.get("scopes").and_then(Value::as_array).into_iter().flatten(); let scopes = scopes.filter_map(Value::as_str).map(|s| format!(" --scope {}", quote(s))); - format!("locallibrary account {} sign-in{}", p("type")?, flags.chain(scopes).collect::()) + format!("locallibrary account {target} sign-in{}", flags.chain(scopes).collect::()) } "accounts.access.set_default" => { let subject = a.params.get("account").and_then(|x| x.get("subject")).and_then(Value::as_str).map(quote); diff --git a/apps/locallibrary/src/hints/tests.rs b/apps/locallibrary/src/hints/tests.rs index 63d8bfb25..ff3a0ae68 100644 --- a/apps/locallibrary/src/hints/tests.rs +++ b/apps/locallibrary/src/hints/tests.rs @@ -136,6 +136,11 @@ fn hints_are_command_lines_that_parse(tc: hegel::TestCase) { params.insert(key.into(), tc.draw(gs::booleans()).into()); } let method = tc.draw(gs::sampled_from(&methods[..])); + // Signing in again names the account by its subject. + let again = method == "accounts.sign_in.begin" && tc.draw(gs::booleans()); + if again { + params.insert("account".into(), value(&tc).into()); + } let e = locallibrary::Error::Conflict("x".into()).action(Action::call(method, Value::Object(params.clone()), "Do it")); let hint = &hints(&e)[0]; @@ -156,6 +161,7 @@ fn hints_are_command_lines_that_parse(tc: hegel::TestCase) { "sync.plan" => &["left", "right"], "merge.plan" => &["source", "target"], "snapshot.label" => &["label"], + "accounts.sign_in.begin" if again => &["with", "for"], "accounts.sign_in.begin" => &["type", "with", "for"], "accounts.access.set_default" => &["plugin"], "undo.list" | "account.login.begin" | "settings.schema" | "settings.import.plan" | "library.list" => &[], @@ -163,6 +169,9 @@ fn hints_are_command_lines_that_parse(tc: hegel::TestCase) { }; let comes_back = |v: &str| assert!(words.iter().any(|w| w == v), "{v:?} doesn't come back from {cmd}: {words:?}"); used.iter().filter_map(|key| params.get(*key).and_then(Value::as_str)).for_each(comes_back); + if again { + comes_back(&format!("{}:{}", params["type"].as_str().unwrap(), params["account"].as_str().unwrap())); + } match method { "accounts.sign_in.begin" => scopes.iter().for_each(|s| comes_back(s)), "accounts.access.set_default" => comes_back(params["account"]["subject"].as_str().unwrap()), diff --git a/apps/locallibrary/src/plugin_cmd.rs b/apps/locallibrary/src/plugin_cmd.rs index d7d2fc98c..c826148f4 100644 --- a/apps/locallibrary/src/plugin_cmd.rs +++ b/apps/locallibrary/src/plugin_cmd.rs @@ -68,8 +68,9 @@ enum PluginCommand { #[arg(long, requires = "key", conflicts_with_all = ["value", "reset"])] secret: bool, }, - /// A plugin's access to your accounts: show it, make an account its default for the account's - /// type (--default ACCOUNT), or take its access to one away (--take-away ACCOUNT) + /// A plugin's access to your accounts: show it, give it access to one (--give ACCOUNT), make + /// an account its default for the account's type (--default ACCOUNT), or take its access to + /// one away (--take-away ACCOUNT) Access { /// The plugin's id plugin: String, @@ -80,6 +81,12 @@ enum PluginCommand { /// The account to take its access to away #[arg(long, value_name = "ACCOUNT", conflicts_with = "default")] take_away: Option, + /// The account to give it access to: the scopes it needs, and those of --scope + #[arg(long, value_name = "ACCOUNT", conflicts_with_all = ["default", "take_away"])] + give: Option, + /// With --give: a scope it can do without to give it besides (repeatable) + #[arg(long = "scope", value_name = "SCOPE", requires = "give")] + scopes: Vec, }, /// Which analyzer each role is read from (a track's key, tempo and level, for doctor, the /// check before a set, ordering a set and `analyze --apply`; a published set's MusicBrainz @@ -166,8 +173,9 @@ pub fn run(args: PluginArgs, store_path: &Option, out: Output) -> Resul let change = listing::Change { value, reset, secret }; return listing::settings(plugins, plugin, key, change, store_path, out); } - PluginCommand::Access { plugin, default, take_away } => { - return crate::account_cmd::access::access(&plugin, default, take_away, store_path, out); + PluginCommand::Access { plugin, default, take_away, give, scopes } => { + let change = crate::account_cmd::access::Change { default, take_away, give, scopes }; + return crate::account_cmd::access::access(&plugin, change, store_path, out); } PluginCommand::Roles { role, analyzer, reset, none } => { return listing::roles(role, analyzer, reset, none, store_path, out); diff --git a/apps/locallibrary/src/rpc/methods/accounts.rs b/apps/locallibrary/src/rpc/methods/accounts.rs index ab19a69c8..15f816fbd 100644 --- a/apps/locallibrary/src/rpc/methods/accounts.rs +++ b/apps/locallibrary/src/rpc/methods/accounts.rs @@ -79,6 +79,16 @@ struct AccessRef { account: AccountId, } +#[derive(Deserialize, JsonSchema)] +struct GiveAccess { + /// The plugin's id. + plugin: String, + account: AccountId, + /// Scopes of the account's type it can do without to give it besides those it needs, by name. + #[serde(default)] + scopes: Vec, +} + pub(super) fn register(r: &mut Registry) { r.read("accounts.list", |c, _: NoParams| c.or_empty(|store| host()?.accounts_and_accesses(store))).quick(); r.immediate("accounts.sign_in.begin", |c, req: BeginSignIn| { @@ -110,6 +120,10 @@ pub(super) fn register(r: &mut Registry) { c.or_empty(|store| host()?.accesses(store, req.plugin.as_deref(), None)) }) .quick(); + r.immediate("accounts.access.give", |c, req: GiveAccess| { + Ok(host()?.give_access(&req.plugin, &req.account, &req.scopes, &c.store()?)?) + }) + .keeps_cache(); r.immediate("accounts.access.set_default", |c, req: AccessRef| { Ok(host()?.set_default_access(&req.plugin, &req.account, &c.store()?)?) }) diff --git a/apps/locallibrary/tests/it/plugins/accounts/cli.rs b/apps/locallibrary/tests/it/plugins/accounts/cli.rs index 27afc1f4b..841db77d3 100644 --- a/apps/locallibrary/tests/it/plugins/accounts/cli.rs +++ b/apps/locallibrary/tests/it/plugins/accounts/cli.rs @@ -76,8 +76,9 @@ impl Mock { } /// An account is signed in to for a plugin, listed with the access it gave (by `account`, `account -/// TYPE:NAME status` and `plugin access`), signed out of, listed for that access, and the access -/// taken away; and a command that can't name an account says why. +/// TYPE:NAME status` and `plugin access`), given to it with more scopes, signed out of, listed for +/// that access, signed in to again by name, and the access taken away; and a command that can't +/// name an account says why. #[test] fn an_account_is_signed_in_listed_and_signed_out_on_the_command_line() { let m = Mock::new(); @@ -100,6 +101,14 @@ fn an_account_is_signed_in_listed_and_signed_out_on_the_command_line() { } let (_, out, _) = m.run(&["plugin", "access", "echo"]); assert_eq!(out, "mock: Tests\n a@mock.example (default) · read\n"); + let (code, _, err) = m.run(&["plugin", "access", "echo", "--scope", "write"]); + assert!(code == 2 && err.contains("--give"), "{err}"); + let (code, out, err) = m.run(&["plugin", "access", "echo", "--give", "a", "--scope", "write"]); + assert_eq!( + (code, out.as_str()), + (0, "✓ Gave echo access to the Mock account a@mock.example: read, write\n"), + "{err}" + ); let (code, out, _) = m.run(&["account", "mock:a", "sign-out"]); assert_eq!((code, out.as_str()), (0, "✓ Signed out of a@mock.example\n")); diff --git a/apps/locallibrary/tests/it/plugins/shared_accounts.rs b/apps/locallibrary/tests/it/plugins/shared_accounts.rs index cba9da498..da4012474 100644 --- a/apps/locallibrary/tests/it/plugins/shared_accounts.rs +++ b/apps/locallibrary/tests/it/plugins/shared_accounts.rs @@ -193,7 +193,8 @@ fn a_sign_in_the_sidecar_cant_begin_says_why() { } /// An access error the echo plugin passes on carries actions: each names a method the sidecar -/// has, with parameters its contract takes, and calling it does what its label says. +/// has, with parameters its contract takes, and calling it does what its label says. Given more +/// scopes, then signed out, echo's access offers first to sign in again as its account. #[test] fn the_actions_of_an_access_error_are_methods_the_sidecar_has() { let m = Mock::new(); @@ -216,6 +217,24 @@ fn the_actions_of_an_access_error_are_methods_the_sidecar_has() { } let accesses = s.call(20, "accounts.access.list", json!({ "plugin": "echo" }))["result"].clone(); assert_eq!((&accesses[0]["account"], &accesses[0]["default"]), (&account("a"), &json!(true)), "{accesses}"); + let more = json!({ "plugin": "echo", "account": account("a"), "scopes": ["write"] }); + let given = s.call(21, "accounts.access.give", more); + assert_eq!(given["result"]["scopes"], json!(["read", "write"]), "{given}"); + + s.call(22, "accounts.sign_out", json!({ "account": account("a") })); + let failing = + json!({ "fail": "access", "message": "it can't", "reason": "signed_out", "fields": { "type": "mock" } }); + let e = s.call(23, "plugins.run", json!({ "plugin": "echo", "command": "echo", "params": failing })); + let again = &e["error"]["data"]["actions"][0]; + let label = "Sign in again as a@mock.example, allowing read, write"; + assert_eq!((&again["method"], &again["label"]), (&json!("accounts.sign_in.begin"), &json!(label)), "{e}"); + let described = contract()["methods"].as_array().unwrap().iter().find(|m| m["name"] == again["method"]).unwrap(); + assert_eq!(violation(&again["params"], &format!("{}/params", at(described))), None, "{again}"); + let begun = s.call(24, "accounts.sign_in.begin", again["params"].clone()); + let (_, query) = begun["result"]["url"].as_str().unwrap().split_once('?').unwrap(); + let query: Vec<(String, String)> = form_urlencoded::parse(query.as_bytes()).into_owned().collect(); + let at = |key: &str| query.iter().find(|(k, _)| k == key).map(|(_, v)| v.as_str()); + assert_eq!((at("login_hint"), at("scope")), (Some("a"), Some("openid read write")), "{begun}"); } /// Who the steps of the property tests here are about, by subject. diff --git a/crates/locallibrary/src/plugin/accounts/access.rs b/crates/locallibrary/src/plugin/accounts/access.rs index c891a1a37..186760e01 100644 --- a/crates/locallibrary/src/plugin/accounts/access.rs +++ b/crates/locallibrary/src/plugin/accounts/access.rs @@ -5,16 +5,16 @@ //! or the one left when the default is taken away. use indexmap::IndexSet; -use loclib_plugin::{AccountNeed, Manifest}; +use loclib_plugin::{AccountNeed, Manifest, Scope}; use schemars::JsonSchema; use serde::{Deserialize, Serialize}; -use serde_json::json; +use serde_json::{Map, Value, json}; use super::kept::{self, Book, Kept}; use super::shared::not_kept; use crate::error::{Action, Reason}; use crate::plugin::Plugins; -use crate::plugin::process::Plugin; +use crate::plugin::process::{Host, Plugin}; use crate::store::Store; use crate::{Error, Result}; @@ -205,14 +205,22 @@ impl Plugins { let kind = &need.name; let e = Error::Account(format!("the {} plugin has no access to a {kind} account", manifest.name)); let e = e.because(Reason::NoAccess).with("type", kind); - Err(actions(manifest, Some(kind), Some(store)).into_iter().fold(e, Error::action)) + let actions = actions(&self.host, manifest, Some(kind), Some(store), &Map::new()); + Err(actions.into_iter().fold(e, Error::action)) } } /// What resolves a plugin (its `manifest`) having no access, or not enough, to an account of -/// type `kind` (one it asks for; any it asks for when it doesn't): signing in to one for it, and +/// type `kind` (one it asks for; any it asks for when it doesn't), as its error `said`: signing in +/// again as the account of its access (see [`sign_in_again`]), signing in to one for it, and /// making each `store` keeps of the type its default. -pub(crate) fn actions(manifest: &Manifest, kind: Option<&str>, store: Option<&Store>) -> Vec { +pub(crate) fn actions( + host: &Host, + manifest: &Manifest, + kind: Option<&str>, + store: Option<&Store>, + said: &Map, +) -> Vec { let asked = manifest.accounts.iter().map(|n| n.name.as_str()); let kinds: Vec<&str> = match kind.filter(|k| manifest.accounts.iter().any(|n| n.name == *k)) { Some(kind) => vec![kind], @@ -222,6 +230,7 @@ pub(crate) fn actions(manifest: &Manifest, kind: Option<&str>, store: Option<&St let plugin = &manifest.id; let mut actions = Vec::new(); for kind in kinds { + actions.extend(sign_in_again(host, manifest, kind, &book, said)); let begin = json!({ "type": kind, "for": plugin }); actions.push(Action::call("accounts.sign_in.begin", begin, format!("Sign in to a {kind} account"))); for k in book.accounts.iter().filter(|k| k.kind == kind) { @@ -231,3 +240,51 @@ pub(crate) fn actions(manifest: &Manifest, kind: Option<&str>, store: Option<&St } actions } + +/// Signing in again as the account of the plugin's (its `manifest`'s) access of type `kind` that +/// its error `said` it used, else its default, when signing in again can give it more: the +/// account is signed out (and so has none of the access's scopes), or wasn't granted what one of +/// them asks, or the error says it needs one the access hasn't. Only scopes the manifest asks for +/// are allowed, whatever the error says, and the access and account are the store's. +fn sign_in_again( + host: &Host, + manifest: &Manifest, + kind: &str, + book: &Book, + said: &Map, +) -> Option { + let need = manifest.accounts.iter().find(|n| n.name == kind)?; + let its = || book.accesses.iter().filter(|a| a.plugin == manifest.id && a.kind == kind); + let named = said.get("access").and_then(Value::as_str); + let a = its().find(|a| Some(a.id.as_str()) == named).or_else(|| its().find(|a| a.default))?; + let with = host.account_plugin(&a.account.plugin)?; + let t = with.account_type(kind).ok()?; + let k = book.account(&a.account.plugin, &a.account.subject); + let needs: Vec<&str> = + said.get("needs").and_then(Value::as_array).into_iter().flatten().filter_map(Value::as_str).collect(); + let usable = |name: &String, scope: &Scope| { + a.scopes.contains(name) && k.is_some_and(|k| scope.asks.iter().all(|asks| k.scopes.contains(asks))) + }; + let allowing: Vec<(&String, &Scope)> = need + .scopes + .iter() + .chain(need.optional.iter().flat_map(|o| &o.scopes)) + .filter(|name| a.scopes.contains(name) || needs.contains(&name.as_str())) + .filter_map(|name| Some((name, t.scopes.get(name)?))) + .filter(|(name, scope)| !usable(name, scope)) + .collect(); + if allowing.is_empty() { + return None; + } + let titles: Vec<&str> = allowing.iter().map(|(_, scope)| scope.title.as_str()).collect(); + let label = format!("Sign in again as {}, allowing {}", a.name, titles.join(", ")); + let scopes: Vec<&String> = allowing.iter().map(|(name, _)| *name).collect(); + let params = json!({ + "type": kind, + "with": a.account.plugin, + "account": a.account.subject, + "for": manifest.id, + "scopes": scopes, + }); + Some(Action::call("accounts.sign_in.begin", params, label)) +} diff --git a/crates/locallibrary/src/plugin/accounts/token.rs b/crates/locallibrary/src/plugin/accounts/token.rs index b94748321..7e42d4c2a 100644 --- a/crates/locallibrary/src/plugin/accounts/token.rs +++ b/crates/locallibrary/src/plugin/accounts/token.rs @@ -4,7 +4,8 @@ //! the account was granted what that scope asks of the service, and the access has every scope the //! plugin needs. Anything else fails before the request is sent: `access` when the plugin has no //! such access or the account is signed out (`reason` `signed_out`), `scope` when its scopes don't -//! reach, with the type, the scopes and the account plugins in its fields. +//! reach, with the type, the scopes and the account plugins in its fields, and once there's an +//! access, its id, its account and the scopes signing in again `needs` to allow. use std::sync::Arc; @@ -12,6 +13,7 @@ use loclib_plugin::{AccountNeed, ErrorData, RpcError, Scope, TokenPlace}; use reqwest::Url; use serde_json::{Map, json}; +use super::access::Access; use super::kept; use crate::error::Reason; use crate::plugin::process::{Live, Plugin}; @@ -22,7 +24,7 @@ use crate::store::Store; pub(crate) struct Usable { /// The account plugin, which gets the token. with: Arc, - subject: String, + access: Access, pub(crate) place: TokenPlace, pub(crate) scopes: Vec, } @@ -38,13 +40,24 @@ fn failed(kind: &str, why: String, need: &AccountNeed, scopes: &[String], plugin RpcError { data: Some(Box::new(data)), ..RpcError::failed(kind, why) } } -/// The account its access is to is signed out, through the account plugin `with`. -fn signed_out(why: String, need: &AccountNeed, scopes: &[String], with: &str) -> RpcError { - let mut e = failed("access", why, need, scopes, vec![with.into()]); +/// The account of the access `a` is signed out, which signing in again gives back every scope of. +fn signed_out(why: String, need: &AccountNeed, a: &Access) -> RpcError { + let mut e = about(failed("access", why, need, &a.scopes, vec![a.account.plugin.clone()]), a, &a.scopes); e.data.iter_mut().for_each(|d| d.reason = Some("signed_out".into())); e } +/// `e`, naming the access `a` it's about, its account, and the scopes signing in again `needs` to +/// allow. +fn about(mut e: RpcError, a: &Access, needs: &[String]) -> RpcError { + for d in e.data.iter_mut() { + d.fields.insert("access".into(), json!(a.id)); + d.fields.insert("account".into(), json!(a.account)); + d.fields.insert("needs".into(), json!(needs)); + } + e +} + impl Plugin { /// The access a `method` request to `url` (as the plugin wrote it) may use of the type `need` /// asks for: the one named `access`, or its default for the type (see the module's docs). @@ -70,44 +83,46 @@ impl Plugin { } }; let (on, at) = (&a.account.plugin, &a.name); - let gone = || signed_out(format!("the {kind} account {at} is signed out: sign in again"), need, &a.scopes, on); + let gone = || signed_out(format!("the {kind} account {at} is signed out: sign in again"), need, a); let k = book.account(on, &a.account.subject).ok_or_else(gone)?; let with = self.host.account_plugin(on); let t = with.as_ref().and_then(|w| w.account_type(kind).ok()); let (Some(with), Some(t)) = (with.clone(), t) else { - return Err(no_access(format!( - "the {kind} account {at} was signed in through {on}, which isn't installed" - ))); + let why = format!("the {kind} account {at} was signed in through {on}, which isn't installed"); + return Err(about(no_access(why), a, &[])); + }; + // Each naming the scopes signing in again needs to allow: none, when it can't help. + let scope_failed = |why: String, scopes: &[String], needs: &[String]| { + about(failed("scope", why, need, scopes, vec![on.clone()]), a, needs) }; - let scope_failed = |why: String, scopes: &[String]| failed("scope", why, need, scopes, vec![on.clone()]); let missing: Vec = need.scopes.iter().filter(|s| !a.scopes.contains(s)).cloned().collect(); if !missing.is_empty() { let why = format!("its access to the {kind} account {at} doesn't have every scope it needs: give it again"); - return Err(scope_failed(why, &missing)); + return Err(scope_failed(why, &missing, &missing)); } let scopes = || a.scopes.iter().filter_map(|n| Some((n, t.scopes.get(n)?))); let covering: Vec<(&String, &Scope)> = scopes().filter(|(_, s)| s.covers(method, url)).collect(); if covering.is_empty() { let why = format!("none of its scopes on the {kind} account {at} covers {method} {url}"); - return Err(scope_failed(why, &a.scopes)); + return Err(scope_failed(why, &a.scopes, &[])); } let is_granted = |s: &Scope| s.asks.iter().all(|asks| k.scopes.contains(asks)); if !covering.iter().any(|(_, s)| is_granted(s)) { let names: Vec = covering.iter().map(|(n, _)| n.to_string()).collect(); let why = format!("the {kind} account {at} wasn't granted what {} asks: sign in again", names.join(", ")); - return Err(scope_failed(why, &names)); + return Err(scope_failed(why, &names, &names)); } let scopes = scopes().filter(|(_, s)| is_granted(s)).map(|(_, s)| s.clone()).collect(); - Ok(Usable { with, subject: k.subject.clone(), place: t.token.clone(), scopes }) + Ok(Usable { with, access: a.clone(), place: t.token.clone(), scopes }) } /// The token of `usable`'s account, told to [`crate::plugin::net::State::scrub`]: `access` /// (`signed_out`) when it's signed out, or the service signs it out. pub(crate) fn usable_token(&self, usable: &Usable, need: &AccountNeed, store: &Store) -> Result { - let token = usable.with.account_token(store, &usable.subject).map_err(|e| { + let token = usable.with.account_token(store, &usable.access.account.subject).map_err(|e| { let why = self.net.scrub(&e.to_string()); match e.reason() { - Some(Reason::SessionEnded) => signed_out(why, need, &need.scopes, &usable.with.manifest.id), + Some(Reason::SessionEnded) => signed_out(why, need, &usable.access), _ => RpcError::failed("external", why), } })?; diff --git a/crates/locallibrary/src/plugin/process.rs b/crates/locallibrary/src/plugin/process.rs index de93436e3..b20dbdc84 100644 --- a/crates/locallibrary/src/plugin/process.rs +++ b/crates/locallibrary/src/plugin/process.rs @@ -309,8 +309,9 @@ impl Slot { /// plugin never runs elevated, so running locallibrary so can't help) and `stale_plan` (only /// the host's plans go stale) too. `access` and `scope` (its request couldn't use an account, /// see [`super::accounts::token`]) are `account` errors, whose actions come from its - /// manifest, never from what it says. Its connection closing means it stopped. The secrets - /// the host filled in for it are left out. + /// manifest and the store: of what its fields say, they take only the type, which of its own + /// accesses it used and the scopes it needs that the manifest asks for. Its connection + /// closing means it stopped. The secrets the host filled in for it are left out. fn error(&self, p: &Process, mut e: RpcError) -> Error { e.message = self.net.scrub(&e.message); let said = format!("{} ({})", e.message, self.label()); @@ -320,7 +321,7 @@ impl Slot { let signed_out = e.data.as_ref().is_some_and(|d| d.reason.as_deref() == Some("signed_out")); let account = |reason: Reason| { let kind = fields.get("type").and_then(Value::as_str); - let actions = access::actions(&self.manifest, kind, p.live.store.as_ref()); + let actions = access::actions(&self.host, &self.manifest, kind, p.live.store.as_ref(), &fields); actions.into_iter().fold(Error::Account(said.clone()).because(reason), Error::action) }; let error = match (e.kind(), file) { diff --git a/crates/locallibrary/tests/it/plugins/accesses.rs b/crates/locallibrary/tests/it/plugins/accesses.rs index 9cf1fd0fa..8a8a17e7c 100644 --- a/crates/locallibrary/tests/it/plugins/accesses.rs +++ b/crates/locallibrary/tests/it/plugins/accesses.rs @@ -332,8 +332,16 @@ fn resolving(accounts: &[&str]) -> Vec { actions } +/// The action an `account` error a plugin passes on offers to sign in again as the account +/// `subject`, shown as `name`, for echo, allowing `scopes`. +fn signing_in_again(subject: &str, name: &str, scopes: &[&str]) -> Action { + let params = json!({ "type": "mock", "with": WITH, "account": subject, "for": "echo", "scopes": scopes }); + Action::call("accounts.sign_in.begin", params, format!("Sign in again as {name}, allowing {}", scopes.join(", "))) +} + /// An account signed out keeps its accesses, which still list, and a request through one fails as -/// `access` (`signed_out`), which passed on says to sign in; signed in again, it has its token. +/// `access` (`signed_out`) naming the access, its account and what signing in again allows, which +/// passed on says to sign in again as it; signed in again, it has its token. #[test] fn a_signed_out_accounts_access_lists_and_says_to_sign_in() { let m = mock_for(3600, "/userinfo", "", &asking(&["read"], &[])); @@ -346,11 +354,14 @@ fn a_signed_out_accounts_access_lists_and_says_to_sign_in() { let answer = m.fetch_as("GET", "/r/x", "{account:mock}", None); let failed = &answer["error"]["data"]; assert_eq!((&failed["kind"], &failed["reason"]), (&json!("access"), &json!("signed_out")), "{answer}"); + let named = (&failed["fields"]["access"], &failed["fields"]["account"], &failed["fields"]["needs"]); + assert_eq!(named, (&json!(listed[0].id), &json!(account("a")), &json!(["read"])), "{answer}"); let e = m.passed_on("access", Some("signed_out"), &failed["fields"]); assert_eq!((e.code(), e.reason()), ("account", Some(Reason::NotSignedIn)), "{e}"); - assert_eq!(e.detail().actions, resolving(&[])); + let again = signing_in_again("a", "a@mock.example", &["read"]); + assert_eq!(e.detail().actions, [vec![again], resolving(&[])].concat()); - m.sign_in(who(json!("a"), Some("a@mock.example")), None, &[]).0.unwrap(); + m.sign_in(who(json!("a"), Some("a@mock.example")), Some("a"), &["read"]).0.unwrap(); assert_eq!(m.fetch_as("GET", "/r/x", "{account:mock}", None)["result"]["body"], "/r/x Bearer at-2"); assert_eq!(m.f.plugins.accesses(&m.f.store, Some("echo"), None).unwrap(), listed, "one access, as it was"); } @@ -426,10 +437,12 @@ fn signing_in_again_asks_for_what_every_access_needs(tc: TestCase) { } /// What resolves an `access` or `scope` error a plugin passes on comes from its manifest and the -/// accounts the store keeps, whatever type, plugin or account plugins its fields name. +/// accounts and accesses the store keeps, whatever type, plugin, account plugins or account its +/// fields name: of the scopes they say it needs, only those its manifest asks for are signed in +/// again for, as the account of its own access. #[test] fn actions_come_from_the_manifest_whatever_the_plugin_says() { - let m = mock_for(3600, "/userinfo", "", &asking(&["read"], &[])); + let m = mock_for(3600, "/userinfo", "", &asking(&["read"], &["write"])); m.sign_in_for(None, who(json!("a"), None), None, &[]).0.unwrap(); let lying = json!({ "type": "evil", "for": "other", "account_plugins": ["evil-accounts"] }); for (kind, reason) in [("access", Reason::NoAccess), ("scope", Reason::MissingScope)] { @@ -438,6 +451,16 @@ fn actions_come_from_the_manifest_whatever_the_plugin_says() { assert_eq!(e.detail().actions, resolving(&["a"]), "{kind}"); assert_eq!(e.to_string(), "it can't (the Echo plugin)"); } + // Given `read` of a, which was granted it: signing in again helps only for `write`. + m.sign_in_for(None, who(json!("b"), None), None, &[]).0.unwrap(); + m.f.plugins.give_access("echo", &account("a"), &[], &m.f.store).unwrap(); + let forged = json!({ "plugin": WITH, "subject": "b" }); + for (needs, again) in [(json!(["peek", "nope"]), None), (json!(["write", "peek"]), Some("write"))] { + let fields = json!({ "type": "mock", "account": forged, "access": "0a1b2c3d", "needs": needs }); + let e = m.passed_on("scope", None, &fields); + let again = again.map(|scope| signing_in_again("a", "a", &[scope])); + assert_eq!(e.detail().actions, [again.into_iter().collect(), resolving(&["a", "b"])].concat(), "{needs}"); + } } /// How a request echo makes through its access to the account `a` fails, in @@ -552,7 +575,16 @@ fn a_request_an_access_cant_serve_fails_as_the_host_passes_it_on(tc: TestCase) { None => Reason::MissingScope, }; assert_eq!((passed.code(), passed.reason()), ("account", Some(resolves)), "{said}"); - let mut actions = resolving(&[]); + // Signing in again as the account, when that gives echo what its request was refused. + let again: &[&str] = match failing { + SignedOut | Refused => &["read"], + Ungranted => &["write"], + Missing if !lying => &["read"], + _ => &[], + }; + let mut actions: Vec = + (!again.is_empty()).then(|| signing_in_again("a", "a", again)).into_iter().collect(); + actions.extend(resolving(&[])); for k in &kept { let params = json!({ "plugin": "echo", "account": { "plugin": k.plugin, "subject": k.subject } }); actions.push(Action::call("accounts.access.set_default", params, format!("Use {}", k.name))); From 371dd8f08ae1d5f33b5f6a65b4c278d8f10d6679 Mon Sep 17 00:00:00 2001 From: botandbang <337765928+botandbang@users.noreply.github.com> Date: Fri, 30 Oct 2026 13:28:23 -0400 Subject: [PATCH 05/13] feat(locallibrary): check a plugin's accounts before its destination starts --- apps/locallibrary/docs/plugins.md | 13 +- apps/locallibrary/docs/rpc/locallibrary.d.ts | 2 +- apps/locallibrary/docs/rpc/schema.json | 2 +- apps/locallibrary/src/account_cmd/access.rs | 76 +++++++-- apps/locallibrary/src/account_cmd/accounts.rs | 19 ++- .../tests/it/plugins/accounts/cli.rs | 31 +++- crates/locallibrary/src/error.rs | 2 +- crates/locallibrary/src/plugin.rs | 1 + crates/locallibrary/src/plugin/accounts.rs | 1 + .../src/plugin/accounts/access.rs | 36 +---- .../locallibrary/src/plugin/accounts/ready.rs | 146 ++++++++++++++++++ .../locallibrary/src/plugin/destinations.rs | 17 +- .../src/plugin/destinations/publish.rs | 14 +- crates/locallibrary/src/plugin/net.rs | 7 +- .../tests/it/plugins/destinations.rs | 145 +++++++++++++++++ 15 files changed, 444 insertions(+), 68 deletions(-) create mode 100644 crates/locallibrary/src/plugin/accounts/ready.rs diff --git a/apps/locallibrary/docs/plugins.md b/apps/locallibrary/docs/plugins.md index bb60a51e4..86a3bdecb 100644 --- a/apps/locallibrary/docs/plugins.md +++ b/apps/locallibrary/docs/plugins.md @@ -226,10 +226,15 @@ plugins keep their access, unused until it's signed in again. an account (`--scope NAME` adds a scope it can do without), `--default ACCOUNT` makes an account the one it uses for that type, and `--take-away ACCOUNT` takes its access away. When a plugin's request fails because its account is signed out or wasn't granted a scope, the error offers to -sign in again as that account, allowing what's missing. Before a plugin's -destination is used, a plugin with no access to a type it needs asks in the terminal which of your -accounts it may use, or to sign in to another. With `--json`, `--yes` or no terminal, and in the -sidecar, nothing is asked: it fails and says how to give it access. +sign in again as that account, allowing what's missing. A plugin's destination (and publishing +a set there) starts only once each type the plugin needs has an account it may use: signed in, +with every scope it needs, granted by the service, and, when the service doesn't refresh it, +signed in for an hour yet besides how long sending the recording may take. In the terminal, a +plugin with no access asks which of your accounts it may use, or to sign in to another; one whose +account is signed out, won't stay signed in or wasn't granted what it needs asks to sign in again; +one whose access lacks a scope it needs asks to give it. With `--json`, `--yes` or no terminal, +and in the sidecar, nothing is asked: it fails and says how to resolve it. + In the player's palette, "sign in to a TYPE account…" signs in to one, "sign out of NAME" signs out of one, and "access for PLUGIN…" opens a form choosing which account of each type the plugin diff --git a/apps/locallibrary/docs/rpc/locallibrary.d.ts b/apps/locallibrary/docs/rpc/locallibrary.d.ts index 70babe1e3..716337e53 100644 --- a/apps/locallibrary/docs/rpc/locallibrary.d.ts +++ b/apps/locallibrary/docs/rpc/locallibrary.d.ts @@ -9435,7 +9435,7 @@ export type Reason = | "needs_admin" /** * Not signed in to an Atmosphere account, or the account a plugin's access is to is signed - * out: sign in. + * out or won't stay signed in long enough: sign in. */ | "not_signed_in" /** The session ended or was revoked: sign in again. */ diff --git a/apps/locallibrary/docs/rpc/schema.json b/apps/locallibrary/docs/rpc/schema.json index 02c5a5b58..d0526ae8d 100644 --- a/apps/locallibrary/docs/rpc/schema.json +++ b/apps/locallibrary/docs/rpc/schema.json @@ -34546,7 +34546,7 @@ { "type": "string", "const": "not_signed_in", - "description": "Not signed in to an Atmosphere account, or the account a plugin's access is to is signed\nout: sign in." + "description": "Not signed in to an Atmosphere account, or the account a plugin's access is to is signed\nout or won't stay signed in long enough: sign in." }, { "type": "string", diff --git a/apps/locallibrary/src/account_cmd/access.rs b/apps/locallibrary/src/account_cmd/access.rs index 77204b3bc..9506ea255 100644 --- a/apps/locallibrary/src/account_cmd/access.rs +++ b/apps/locallibrary/src/account_cmd/access.rs @@ -1,20 +1,21 @@ //! Plugins' access to the accounts the store keeps: `plugin access PLUGIN` lists a plugin's, -//! makes an account its default for the account's type or takes its access away; and before a -//! plugin's destination is used, a terminal is asked which account it may use of each type it -//! needs and has no access to. +//! gives it one, makes an account its default for the account's type or takes its access away; +//! and before a plugin's destination is used, a terminal is asked about what keeps it from using +//! an account of each type it needs. +use std::mem::discriminant; use std::path::PathBuf; use std::process::ExitCode; use anyhow::Result; use indexmap::IndexSet; use locallibrary::Store; -use locallibrary::plugin::{Access, Account, AccountId, Plugins, destination_spec}; +use locallibrary::plugin::{Access, Account, AccountId, Plugins, Unready, destination_spec}; use locallibrary::progress::Ctx; use loclib_plugin::{AccountNeed, Choice, ChoiceItem}; use serde_json::json; -use super::accounts::{account_type, called, come_back, declined, find, titles}; +use super::accounts::{account_type, called, come_back, declined, find, named, titles}; use crate::browser::BrowserArgs; use crate::jobs_cmd::usage; use crate::output::{Output, dim, heading, ok}; @@ -106,21 +107,76 @@ fn list(plugins: &Plugins, plugin: &str, needs: &[AccountNeed], accesses: &[Acce } /// Before the plugin a destination `spec` names is used: in a terminal (not with `--json` or -/// `--yes`), each account type the plugin needs and has no access to is asked about, and the -/// account chosen made its default, signing in to another when that's chosen. Nothing's asked -/// elsewhere: the destination is refused, saying how to give it access. +/// `--yes`), whatever keeps it from using an account of a type it asks for (see [`Unready`]) is +/// asked about once: which account it may use when it has no access (made its default, signing in +/// to another when that's chosen), signing in again when the account is signed out, won't stay +/// signed in or wasn't granted what it needs, and giving it the scopes its access lacks. Nothing's +/// asked elsewhere: the destination is refused, saying how to resolve it. pub fn ask(spec: &str, store: &Store, out: Output, ctx: &Ctx<'_>) -> Result<()> { if !interactive(out) || out.yes { return Ok(()); } let (plugin, _) = destination_spec(spec)?; let plugins = plugins::host()?; - for need in plugins.without_access(plugin, store)? { - give(plugins, plugin, need, store, out, ctx)?; + // One turned down isn't asked again: the destination refuses it. + let mut asked = Vec::new(); + while let Some(u) = + plugins.unready(plugin, store)?.into_iter().find(|u| !asked.contains(&(u.kind().to_string(), discriminant(u)))) + { + asked.push((u.kind().to_string(), discriminant(&u))); + resolve(plugins, plugin, u, store, out, ctx)?; } Ok(()) } +/// Asks about what keeps `plugin` from using an account of a type (see [`ask`]), and resolves it +/// as the user says. +fn resolve(plugins: &Plugins, plugin: &str, u: Unready, store: &Store, out: Output, ctx: &Ctx<'_>) -> Result<()> { + let manifest = plugins.manifest(plugin)?; + let name = &manifest.name; + let a = match &u { + Unready::NoAccess(kind) => { + let need = manifest.accounts.iter().find(|n| n.name == *kind); + return need.map_or(Ok(()), |need| give(plugins, plugin, need, store, out, ctx)); + } + Unready::SignedOut(a) | Unready::Expiring(a) | Unready::Ungiven(a, _) | Unready::Ungranted(a, _) => a, + }; + let t = account_type(plugins, &a.account.plugin, &a.kind); + let account = named(plugins, &a.kind, &a.account.plugin, &a.name); + let question = match &u { + Unready::Ungiven(_, scopes) => { + if out.confirm(&format!( + "{name} needs to {} with the {account}. Give it access?", + titles(t, scopes).join(", ") + )) { + plugins.give_access(plugin, &a.account, &[], store)?; + ok(format!("{name} has access to the {account}")); + } + return Ok(()); + } + Unready::Ungranted(_, scopes) => { + format!("The {account} doesn't allow {name} to {}. Sign in again?", titles(t, scopes).join(", ")) + } + Unready::Expiring(_) => format!("The {account} won't stay signed in long enough for {name}. Sign in again?"), + _ => format!("The {account}, which {name} uses, is signed out. Sign in again?"), + }; + if !out.confirm(&question) { + return Ok(()); + } + let start = plugins.begin_account_sign_in( + &a.kind, + &a.account.plugin, + Some(plugin), + Some(&a.account.subject), + &[], + store, + )?; + let done = come_back(plugins, &start, BrowserArgs::default(), out, ctx)?; + ok(format!("Signed in to the {}", called(plugins, &done.account))); + declined(plugins, &done); + Ok(()) +} + /// Asks which account of the type `need` asks for `plugin` may use, and makes it its default. fn give(plugins: &Plugins, plugin: &str, need: &AccountNeed, store: &Store, out: Output, ctx: &Ctx<'_>) -> Result<()> { let (kind, name) = (&need.name, &plugins.manifest(plugin)?.name); diff --git a/apps/locallibrary/src/account_cmd/accounts.rs b/apps/locallibrary/src/account_cmd/accounts.rs index 8e2677d73..bb25b18d2 100644 --- a/apps/locallibrary/src/account_cmd/accounts.rs +++ b/apps/locallibrary/src/account_cmd/accounts.rs @@ -140,13 +140,18 @@ pub(super) fn account_type<'a>(plugins: &'a Plugins, with: &str, kind: &str) -> plugins.manifest(with).ok()?.contributes.account_types.iter().find(|t| t.name == kind) } -/// What `a` is called: its type's title, `account` and its name, then the account plugin it was -/// signed in through when several provide the type. +/// What `a` is called: see [`named`]. pub(super) fn called(plugins: &Plugins, a: &Account) -> String { - let title = account_type(plugins, &a.plugin, &a.kind).map_or(a.kind.as_str(), |t| &t.title); - match plugins.providing(&a.kind).len() > 1 { - true => format!("{title} account {} (through {})", a.name, a.plugin), - false => format!("{title} account {}", a.name), + named(plugins, &a.kind, &a.plugin, &a.name) +} + +/// What the account of type `kind` shown as `name` that `with` signed in is called: its type's +/// title, `account` and its name, then the account plugin when several provide the type. +pub(super) fn named(plugins: &Plugins, kind: &str, with: &str, name: &str) -> String { + let title = account_type(plugins, with, kind).map_or(kind, |t| &t.title); + match plugins.providing(kind).len() > 1 { + true => format!("{title} account {name} (through {with})"), + false => format!("{title} account {name}"), } } @@ -189,7 +194,7 @@ pub(super) fn list(plugins: &Plugins, all: &Accounts) { fn show(plugins: &Plugins, a: &Account, accesses: &[&Access]) { heading(&called(plugins, a)); let state = match (a.signed_in, a.refreshes, a.expires) { - (false, ..) => format!("signed out: `locallibrary account {} sign-in` signs in again", a.kind), + (false, ..) => format!("signed out: `locallibrary account {}:{} sign-in` signs in again", a.kind, a.subject), (true, true, _) => "signed in, refreshed on its own".into(), (true, false, Some(at)) => format!("signed in until {}", local_time(at, "%Y-%m-%d %H:%M")), (true, false, None) => "signed in".into(), diff --git a/apps/locallibrary/tests/it/plugins/accounts/cli.rs b/apps/locallibrary/tests/it/plugins/accounts/cli.rs index 841db77d3..987a60ff8 100644 --- a/apps/locallibrary/tests/it/plugins/accounts/cli.rs +++ b/apps/locallibrary/tests/it/plugins/accounts/cli.rs @@ -113,7 +113,7 @@ fn an_account_is_signed_in_listed_and_signed_out_on_the_command_line() { let (code, out, _) = m.run(&["account", "mock:a", "sign-out"]); assert_eq!((code, out.as_str()), (0, "✓ Signed out of a@mock.example\n")); let (_, out, _) = m.run(&["account"]); - let signed_out = " signed out: `locallibrary account mock sign-in` signs in again\n access: echo (default)\n"; + let signed_out = " signed out: `locallibrary account mock:a sign-in` signs in again\n access: echo (default)\n"; assert!(out.contains(signed_out), "{out}"); // Signed out, it's still named for signing in to again: echo has access to it. let mut c = m.command(); @@ -353,6 +353,32 @@ fn a_destination_without_access_asks_which_account_in_a_terminal() { assert!(out.starts_with("✓ Echo uses the Mock account b@mock.example\n"), "{out}"); let (_, out, _) = m.run(&["plugin", "access", "echo"]); assert_eq!(out, "mock: Tests\n b@mock.example (default) · read\n"); + + // Signed out, it's offered signing in again as b; turned down, the destination is refused, + // saying so. + assert_eq!(m.run(&["account", "mock:b", "sign-out"]).0, 0); + let (code, _, err) = m.typed(©, "n\n", "b"); + assert_eq!(code, 1, "{err}"); + assert!( + err.contains("? The Mock account b@mock.example, which Echo uses, is signed out. Sign in again? [y/N]"), + "{err}" + ); + assert!( + err.contains("hint: Sign in again as b@mock.example, allowing read: locallibrary account mock:b sign-in"), + "{err}" + ); + let (code, out, err) = m.typed(©, "y\n", "b"); + assert_eq!(code, 0, "{err}"); + assert!(out.starts_with("✓ Signed in to the Mock account b@mock.example\n"), "{out}"); + // Its access lacking what echo needs, it's offered giving it. + let file = m.env.store.join("accounts.json"); + let mut kept: Value = serde_json::from_str(&std::fs::read_to_string(&file).unwrap()).unwrap(); + kept["accesses"][0]["scopes"] = json!([]); + std::fs::write(&file, kept.to_string()).unwrap(); + let (code, out, err) = m.typed(©, "y\n", "b"); + assert_eq!(code, 0, "{err}"); + assert!(err.contains("? Echo needs to read with the Mock account b@mock.example. Give it access? [y/N]"), "{err}"); + assert!(out.starts_with("✓ Echo has access to the Mock account b@mock.example\n"), "{out}"); } /// A second account plugin's id, providing the `mock` type too. @@ -673,7 +699,8 @@ fn every_account_command_says_in_words_what_it_answers_as_json(tc: TestCase) { (false, false) => assert_eq!(out, format!("✓ Signed out of {} on this computer\n", a.name)), } let (_, shown, _) = m.run(&["account"]); - let signed_out = "signed out: `locallibrary account mock sign-in` signs in again\n access: echo\n"; + let signed_out = + format!("signed out: `locallibrary account mock:{} sign-in` signs in again\n access: echo\n", a.subject); assert!(shown.contains(&format!("{}\n {signed_out}", called(&a))), "{shown}"); let (code, out, err) = m.run(&maybe_json(json, &["plugin", "access", "echo", "--take-away", &target])); assert_eq!(code, 0, "{err}"); diff --git a/crates/locallibrary/src/error.rs b/crates/locallibrary/src/error.rs index b3db04eb2..bf1605158 100644 --- a/crates/locallibrary/src/error.rs +++ b/crates/locallibrary/src/error.rs @@ -200,7 +200,7 @@ pub enum Reason { // `account`. /// Not signed in to an Atmosphere account, or the account a plugin's access is to is signed - /// out: sign in. + /// out or won't stay signed in long enough: sign in. NotSignedIn, /// The session ended or was revoked: sign in again. SessionEnded, diff --git a/crates/locallibrary/src/plugin.rs b/crates/locallibrary/src/plugin.rs index 450d4ba98..b3a4122c9 100644 --- a/crates/locallibrary/src/plugin.rs +++ b/crates/locallibrary/src/plugin.rs @@ -74,6 +74,7 @@ use crate::{Error, Result}; pub use accounts::REFRESH_BEFORE; pub use accounts::access::{Access, AccountId}; +pub use accounts::ready::Unready; pub use accounts::shared::{Account, AccountSignIn, AccountSignedIn, AccountSignedOut, Accounts}; pub use analysis::{AnalysisPart, ApplyAnalysis, same_grid}; pub use analyzers::Analyzed; diff --git a/crates/locallibrary/src/plugin/accounts.rs b/crates/locallibrary/src/plugin/accounts.rs index 80be42c40..8964157b7 100644 --- a/crates/locallibrary/src/plugin/accounts.rs +++ b/crates/locallibrary/src/plugin/accounts.rs @@ -41,6 +41,7 @@ use crate::{Error, Result, lock}; pub(super) mod access; mod kept; +pub(super) mod ready; pub(super) mod shared; mod token; mod whoami; diff --git a/crates/locallibrary/src/plugin/accounts/access.rs b/crates/locallibrary/src/plugin/accounts/access.rs index 186760e01..17600d837 100644 --- a/crates/locallibrary/src/plugin/accounts/access.rs +++ b/crates/locallibrary/src/plugin/accounts/access.rs @@ -12,7 +12,7 @@ use serde_json::{Map, Value, json}; use super::kept::{self, Book, Kept}; use super::shared::not_kept; -use crate::error::{Action, Reason}; +use crate::error::Action; use crate::plugin::Plugins; use crate::plugin::process::{Host, Plugin}; use crate::store::Store; @@ -188,28 +188,6 @@ impl Plugins { } } -impl Plugins { - /// What `plugin` asks for of the account types it has no default access to in `store`: those - /// a request naming the type (`{account:}`) can't use. - pub fn without_access(&self, plugin: &str, store: &Store) -> Result> { - let accesses = kept::read(store)?.accesses; - let has = |n: &AccountNeed| accesses.iter().any(|a| a.plugin == plugin && a.kind == n.name && a.default); - Ok(self.manifest(plugin)?.accounts.iter().filter(|n| !has(n)).collect()) - } - - /// Fails `account` (`no_access`, with what resolves it) when `plugin` has no default access in - /// `store` to an account of a type it asks for. - pub fn check_access(&self, plugin: &str, store: &Store) -> Result<()> { - let Some(need) = self.without_access(plugin, store)?.first().copied() else { return Ok(()) }; - let manifest = self.manifest(plugin)?; - let kind = &need.name; - let e = Error::Account(format!("the {} plugin has no access to a {kind} account", manifest.name)); - let e = e.because(Reason::NoAccess).with("type", kind); - let actions = actions(&self.host, manifest, Some(kind), Some(store), &Map::new()); - Err(actions.into_iter().fold(e, Error::action)) - } -} - /// What resolves a plugin (its `manifest`) having no access, or not enough, to an account of /// type `kind` (one it asks for; any it asks for when it doesn't), as its error `said`: signing in /// again as the account of its access (see [`sign_in_again`]), signing in to one for it, and @@ -244,8 +222,9 @@ pub(crate) fn actions( /// Signing in again as the account of the plugin's (its `manifest`'s) access of type `kind` that /// its error `said` it used, else its default, when signing in again can give it more: the /// account is signed out (and so has none of the access's scopes), or wasn't granted what one of -/// them asks, or the error says it needs one the access hasn't. Only scopes the manifest asks for -/// are allowed, whatever the error says, and the access and account are the store's. +/// them asks, or the error says it `needs` one (a fresh token for it, or one the access hasn't). +/// Only scopes the manifest asks for are allowed, whatever the error says, and the access and +/// account are the store's. fn sign_in_again( host: &Host, manifest: &Manifest, @@ -262,16 +241,13 @@ fn sign_in_again( let k = book.account(&a.account.plugin, &a.account.subject); let needs: Vec<&str> = said.get("needs").and_then(Value::as_array).into_iter().flatten().filter_map(Value::as_str).collect(); - let usable = |name: &String, scope: &Scope| { - a.scopes.contains(name) && k.is_some_and(|k| scope.asks.iter().all(|asks| k.scopes.contains(asks))) - }; + let granted = |scope: &Scope| k.is_some_and(|k| scope.asks.iter().all(|asks| k.scopes.contains(asks))); let allowing: Vec<(&String, &Scope)> = need .scopes .iter() .chain(need.optional.iter().flat_map(|o| &o.scopes)) - .filter(|name| a.scopes.contains(name) || needs.contains(&name.as_str())) .filter_map(|name| Some((name, t.scopes.get(name)?))) - .filter(|(name, scope)| !usable(name, scope)) + .filter(|(name, scope)| needs.contains(&name.as_str()) || a.scopes.contains(name) && !granted(scope)) .collect(); if allowing.is_empty() { return None; diff --git a/crates/locallibrary/src/plugin/accounts/ready.rs b/crates/locallibrary/src/plugin/accounts/ready.rs new file mode 100644 index 000000000..f63af705c --- /dev/null +++ b/crates/locallibrary/src/plugin/accounts/ready.rs @@ -0,0 +1,146 @@ +//! Whether a plugin's accounts will do for what it's about to start (a destination, publishing a +//! set), checked before it starts, so nothing begins (a render, an upload) that a request of its +//! would fail half way through: of each type it asks for, it has a default access, to an account +//! signed in, with every scope the plugin needs, each granted what it asks, and, when the account +//! isn't refreshed, signed in for an hour yet besides how long sending its files may take. + +use std::time::Duration; + +use chrono::Utc; +use serde_json::{Map, json}; + +use super::access::{Access, actions}; +use super::kept; +use crate::error::Reason; +use crate::plugin::Plugins; +use crate::plugin::net::sending; +use crate::plugin::process::Plugin; +use crate::plugin::secrets::secret; +use crate::store::Store; +use crate::{Error, Result}; + +/// How long a plugin's accounts have to stay signed in once it starts, besides sending its files. +const LASTING: Duration = Duration::from_secs(60 * 60); + +/// What keeps a plugin from using its default access of an account type it asks for. +#[derive(Debug, Clone, PartialEq)] +pub enum Unready { + /// It has none of the type (by name), or none whose account plugin is installed. + NoAccess(String), + /// The access's account is signed out, or its tokens are lost. + SignedOut(Access), + /// The access's account isn't refreshed, and won't stay signed in long enough. + Expiring(Access), + /// The access hasn't these of the scopes (by name) the plugin needs. + Ungiven(Access, Vec), + /// The account wasn't granted what these of the scopes (by name) the plugin needs ask. + Ungranted(Access, Vec), +} + +impl Unready { + /// The account type it's about. + pub fn kind(&self) -> &str { + match self { + Unready::NoAccess(kind) => kind, + Unready::SignedOut(a) | Unready::Expiring(a) | Unready::Ungiven(a, _) | Unready::Ungranted(a, _) => &a.kind, + } + } +} + +impl Plugin { + /// What keeps it from using its default access of each account type it asks for in `store` + /// for as long as sending `bytes` of files may take, besides [`LASTING`]: the first of + /// [`Unready`]'s, in its order, for each type. + fn unready(&self, store: &Store, bytes: u64) -> Result> { + let book = kept::read(store)?; + // None: later than anything expires. + let until = + chrono::Duration::from_std(LASTING + sending(bytes)).ok().and_then(|d| Utc::now().checked_add_signed(d)); + let mut unready = Vec::new(); + for need in &self.manifest.accounts { + let kind = &need.name; + let default = book.accesses.iter().find(|a| a.plugin == self.manifest.id && a.kind == *kind && a.default); + let Some(a) = default else { + unready.push(Unready::NoAccess(kind.clone())); + continue; + }; + let k = book.account(&a.account.plugin, &a.account.subject); + let tokens = match k { + Some(k) => secret(self.host.options.secrets, &kept::place(store, &k.plugin, &k.subject))?, + None => None, + }; + let (Some(k), Some(_)) = (k, tokens) else { + unready.push(Unready::SignedOut(a.clone())); + continue; + }; + let with = self.host.account_plugin(&a.account.plugin); + let Some(t) = with.as_ref().and_then(|w| w.account_type(kind).ok()) else { + unready.push(Unready::NoAccess(kind.clone())); + continue; + }; + let ungiven: Vec = need.scopes.iter().filter(|s| !a.scopes.contains(s)).cloned().collect(); + let is_granted = + |name: &&String| t.scopes.get(*name).is_some_and(|s| s.asks.iter().all(|asks| k.scopes.contains(asks))); + let ungranted: Vec = need.scopes.iter().filter(|s| !is_granted(s)).cloned().collect(); + let expiring = !k.refreshes && k.expires_at.is_some_and(|at| until.is_none_or(|until| at <= until)); + unready.extend(match () { + _ if !ungiven.is_empty() => Some(Unready::Ungiven(a.clone(), ungiven)), + _ if !ungranted.is_empty() => Some(Unready::Ungranted(a.clone(), ungranted)), + _ if expiring => Some(Unready::Expiring(a.clone())), + _ => None, + }); + } + Ok(unready) + } + + /// Fails `account` when its accounts won't do for sending `bytes` of files in `store` (see + /// [`Plugin::unready`]): of the first that won't, with the reason (`no_access`, + /// `not_signed_in`, `missing_scope`), its type, and what resolves it (see [`actions`]). + pub(crate) fn check_accounts(&self, store: &Store, bytes: u64) -> Result<()> { + let Some(u) = self.unready(store, bytes)?.into_iter().next() else { return Ok(()) }; + let name = &self.manifest.name; + let (reason, why, about) = match &u { + Unready::NoAccess(kind) => { + (Reason::NoAccess, format!("the {name} plugin has no access to a {kind} account"), None) + } + Unready::SignedOut(a) => { + let why = format!("the {} account {} is signed out: sign in again", a.kind, a.name); + (Reason::NotSignedIn, why, Some((a, &a.scopes))) + } + Unready::Expiring(a) => { + let why = format!("the {} account {} won't stay signed in long enough: sign in again", a.kind, a.name); + (Reason::NotSignedIn, why, Some((a, &a.scopes))) + } + Unready::Ungiven(a, scopes) => { + let why = format!( + "the {name} plugin's access to the {} account {} doesn't have every scope it needs: give it again", + a.kind, a.name + ); + (Reason::MissingScope, why, Some((a, scopes))) + } + Unready::Ungranted(a, scopes) => { + let why = format!( + "the {} account {} wasn't granted what {} asks: sign in again", + a.kind, + a.name, + scopes.join(", ") + ); + (Reason::MissingScope, why, Some((a, scopes))) + } + }; + let said = + about.map(|(a, needs)| Map::from_iter([("access".into(), json!(a.id)), ("needs".into(), json!(needs))])); + let kind = u.kind(); + let e = Error::Account(why).because(reason).with("type", kind); + let actions = actions(&self.host, &self.manifest, Some(kind), Some(store), &said.unwrap_or_default()); + Err(actions.into_iter().fold(e, Error::action)) + } +} + +impl Plugins { + /// What keeps `plugin` from using its default access of each account type it asks for in + /// `store`, for an hour (see [`Unready`]). + pub fn unready(&self, plugin: &str, store: &Store) -> Result> { + self.slot(plugin)?.unready(store, 0) + } +} diff --git a/crates/locallibrary/src/plugin/destinations.rs b/crates/locallibrary/src/plugin/destinations.rs index 619478537..1a9a3ec31 100644 --- a/crates/locallibrary/src/plugin/destinations.rs +++ b/crates/locallibrary/src/plugin/destinations.rs @@ -47,15 +47,10 @@ pub fn destination_spec(spec: &str) -> Result<(&str, &str)> { } impl Plugins { - /// The destination `spec` names (`:`), which must be of `kind`, its - /// plugin with access in `store` to an account of each type it asks for (see - /// [`Plugins::check_access`]). - fn destination(&self, spec: &str, kind: DestinationKind, store: Option<&Store>) -> Result<(&Arc, String)> { + /// The destination `spec` names (`:`), which must be of `kind`. + fn destination(&self, spec: &str, kind: DestinationKind) -> Result<(&Arc, String)> { let (plugin, name) = destination_spec(spec)?; let slot = self.slot(plugin)?; - if let Some(store) = store { - self.check_access(plugin, store)?; - } let declared = slot.declared("destination", name, &slot.manifest.contributes.destinations, |d| &d.name)?; if declared.kind != kind { let (title, kind) = (&declared.title, kind.name()); @@ -65,9 +60,13 @@ impl Plugins { } /// The store destination `spec` names (`:`), for copying the store to, - /// and restoring and repairing from. + /// and restoring and repairing from: in `store`, once its plugin's accounts will do (see + /// [`super::accounts::ready`]). pub fn store_destination(&self, spec: &str, store: Option<&Store>) -> Result { - let (slot, name) = self.destination(spec, DestinationKind::Store, store)?; + let (slot, name) = self.destination(spec, DestinationKind::Store)?; + if let Some(store) = store { + slot.check_accounts(store, 0)?; + } Ok(StoreDestination { slot: slot.clone(), name, store: store.cloned() }) } } diff --git a/crates/locallibrary/src/plugin/destinations/publish.rs b/crates/locallibrary/src/plugin/destinations/publish.rs index 0eae0b98f..2648e9079 100644 --- a/crates/locallibrary/src/plugin/destinations/publish.rs +++ b/crates/locallibrary/src/plugin/destinations/publish.rs @@ -4,7 +4,7 @@ //! heard, and the host keeps that as the set's link, with the link's name and the Atmosphere //! record holding the recording when the plugin gives them. -use std::path::PathBuf; +use std::path::{Path, PathBuf}; use std::sync::{Arc, Weak}; use chrono::{DateTime, Utc}; @@ -17,6 +17,7 @@ use serde::Serialize; use super::super::Plugins; use super::super::process::Slot; +use crate::error::IoContext; use crate::op::{Operation, Planned, Probe, Touched}; use crate::profile::{PREFS_FILE, ProfilePrefs}; use crate::progress::Ctx; @@ -100,6 +101,12 @@ fn published(set: &Set, unnamed: &Unnamed) -> (PublishedSet, Vec (details, tracklist) } +/// How big the recording at `path` is, which the plugin's accounts have to stay signed in for +/// sending. +fn size(path: &Path) -> Result { + Ok(std::fs::metadata(path).at(path)?.len()) +} + impl Plugins { /// Plans publishing the set `set` names (see [`SetBook::find`]) with its recording at the /// set destination `spec` names (`:`): the plugin's dry run says what @@ -116,7 +123,7 @@ impl Plugins { store: &Store, ctx: &Ctx<'_>, ) -> Result> { - let (slot, destination) = self.destination(spec, DestinationKind::Set, Some(store))?; + let (slot, destination) = self.destination(spec, DestinationKind::Set)?; let book = SetBook::load(store)?; let set = book.find(set)?; let recording = set.recording.as_ref().map(|r| r.path.clone()).ok_or_else(|| { @@ -125,6 +132,7 @@ impl Plugins { if !recording.is_file() { return Err(Error::NotFound(format!("{}, the set's recording", recording.display()))); } + slot.check_accounts(store, size(&recording)?)?; let unnamed = set.unnamed(hide, &ProfilePrefs::load(store)?)?; let (details, tracklist) = published(set, &unnamed); let (context, _) = slot.context(loclib_plugin::Context::default(), Some(store))?; @@ -180,6 +188,8 @@ impl Operation for SetPublish { return Err(Error::stale(format!("set {}", p.set), "changed")); } let slot = p.slot.upgrade().ok_or_else(|| Error::stale(format!("the {} plugin", p.plugin), "stopped"))?; + // Again: they may not do any more, and nothing's sent yet. + slot.check_accounts(store, size(&p.publish.recording)?)?; // Lent its recording: what it sends of it is the publishing's progress. let lent = [p.publish.recording.as_path()]; let Published { url, name, record, warnings } = diff --git a/crates/locallibrary/src/plugin/net.rs b/crates/locallibrary/src/plugin/net.rs index 9dbd1d810..033c2e004 100644 --- a/crates/locallibrary/src/plugin/net.rs +++ b/crates/locallibrary/src/plugin/net.rs @@ -55,6 +55,11 @@ const CHECK_EVERY: Duration = Duration::from_millis(50); /// What a secret's placeholder starts with: `{secret:}`. const SECRET: &str = "{secret:"; +/// How long sending `bytes` of files may take, at [`SLOWEST`]. +pub(super) fn sending(bytes: u64) -> Duration { + Duration::from_secs(bytes / SLOWEST) +} + /// A plugin's `net/fetch` state, whichever of its processes asks. #[derive(Default)] pub(crate) struct State { @@ -427,7 +432,7 @@ impl Plugin { let to = url.host_str().unwrap_or_default(); let said = format!("net/fetch: sending {} of files to {to} ({})", op::bytes(files), sent.method); self.host.log(&self.manifest.id, &said); - request = request.timeout(TIMEOUT + Duration::from_secs(files / SLOWEST)); + request = request.timeout(TIMEOUT + sending(files)); } request = body.send(request, live, cancel)?; } diff --git a/crates/locallibrary/tests/it/plugins/destinations.rs b/crates/locallibrary/tests/it/plugins/destinations.rs index a344f8a78..cc268978f 100644 --- a/crates/locallibrary/tests/it/plugins/destinations.rs +++ b/crates/locallibrary/tests/it/plugins/destinations.rs @@ -10,6 +10,7 @@ use hegel::generators as gs; use locallibrary::api::{self, RepairRequest, VerifyRequest}; use locallibrary::catalog::LibraryOutcome; use locallibrary::device::Device; +use locallibrary::error::Reason; use locallibrary::hash::Hash; use locallibrary::offsite::{self, CopyOptions, RestoreOptions}; use locallibrary::op::AnyPlanned; @@ -20,6 +21,7 @@ use locallibrary::sets::{Recording, Set, SetBook, SetEntry}; use locallibrary::sync::{self, PullOptions}; use loclib_plugin::StrongRef; +use super::shared_accounts::{WITH, asking, mock_for, who}; use super::*; impl Fixture { @@ -611,6 +613,149 @@ fn a_set_is_published_as_its_dry_run_said(tc: TestCase) { assert_eq!(now(), (kept.0, kept.1, record), "the host keeps where it's published, and the record"); } +/// How long the mock service's tokens last in +/// [`a_destination_starts_only_when_its_accounts_will_last`]. +#[derive(Debug, Clone, Copy)] +enum Lasting { + /// An hour, then they're refreshed. + Refreshed, + /// Two hours, with nothing to refresh them with. + TwoHours, + /// Half an hour, with nothing to refresh them with. + HalfAnHour, + /// Until they're revoked. + Ever, +} + +/// What's wrong with echo's access to the account in +/// [`a_destination_starts_only_when_its_accounts_will_last`]. +#[derive(Debug, Clone, Copy, PartialEq)] +enum Mistake { + /// The account is signed out. + SignedOut, + /// The account's tokens are lost. + Lost, + /// The access lacks a scope echo needs now. + Ungiven, +} + +/// For echo needing any of the mock type's `read` and `write`, an account signed in for it or not, +/// the service granting any of what they ask, its tokens refreshed or lasting two hours, half an +/// hour or until they're revoked, and the account then signed out, its tokens lost or echo's access +/// lacking a scope it needs, or none of these: echo's store destination, and publishing a set with +/// a recording small or a gigabyte big (which takes over two hours to send at the slowest), start +/// only when echo has a default access to an account signed in, with every scope it needs, each +/// granted what it asks, and staying signed in for an hour besides sending the recording when it +/// isn't refreshed; otherwise each fails as `account` (`no_access`, `not_signed_in`, +/// `missing_scope`), offering to sign in again as the account once echo has access, and echo is +/// never asked. Publishing checks again before it's applied: a plan whose account was signed out +/// since is refused, and echo isn't asked to publish. +#[hegel::test(test_cases = 16, suppress_health_check = [hegel::HealthCheck::TooSlow])] +#[hegel::explicit_test_case( + needs = vec!["read"], + lasting = Lasting::TwoHours, + grants = vec!["read"], + given = true, + mistake = Option::::None, + publishing = true, + big = false, + later = true +)] +fn a_destination_starts_only_when_its_accounts_will_last(tc: TestCase) { + let some = vec![vec![], vec!["read"], vec!["write"], vec!["read", "write"]]; + let needs: Vec<&str> = tc.draw(gs::sampled_from(some[1..].to_vec())); + let m = mock_for(3600, "/userinfo", "", &asking(&needs, &[])); + let lasting = + tc.draw(gs::sampled_from(vec![Lasting::Refreshed, Lasting::TwoHours, Lasting::HalfAnHour, Lasting::Ever])); + let grants: Vec<&str> = tc.draw(gs::sampled_from(some)); + { + let mut s = m.state.lock().unwrap(); + s.refreshless = !matches!(lasting, Lasting::Refreshed); + s.expires_in = match lasting { + Lasting::Refreshed => Some(3600), + Lasting::TwoHours => Some(7200), + Lasting::HalfAnHour => Some(1800), + Lasting::Ever => None, + }; + s.grants = Some(grants.iter().map(|g| g.to_string()).collect()); + } + let given = tc.draw(gs::booleans()); + m.sign_in_for(given.then_some("echo"), who(json!("a"), None), None, &[]).0.unwrap(); + let mistake = + tc.draw(gs::sampled_from(vec![None, Some(Mistake::SignedOut), Some(Mistake::Lost), Some(Mistake::Ungiven)])); + let store = &m.f.store; + match mistake { + Some(Mistake::SignedOut) => drop(m.f.plugins.sign_out_account(WITH, "a", store).unwrap()), + Some(Mistake::Lost) => std::fs::remove_file(store.root().join("secrets/accounts.json")).unwrap(), + Some(Mistake::Ungiven) if given => { + let file = store.root().join("accounts.json"); + let mut kept: Value = serde_json::from_str(&std::fs::read_to_string(&file).unwrap()).unwrap(); + kept["accesses"][0]["scopes"] = json!(needs[1..]); + std::fs::write(&file, kept.to_string()).unwrap(); + } + _ => {} + } + let publishing = tc.draw(gs::booleans()); + let big = tc.draw(gs::booleans()); + let big = big && publishing; + + let recording = store.root().join("friday.wav"); + std::fs::File::create(&recording).unwrap().set_len(if big { 1 << 30 } else { 4 }).unwrap(); + let s = Set { + id: "3m4nset0000000".into(), + title: Some("Friday".into()), + date: NaiveDate::from_ymd_opt(2026, 10, 2).unwrap(), + recording: Some(Recording { path: recording.clone(), report: None }), + created_at: Utc::now(), + updated_at: Utc::now(), + ..Set::default() + }; + SetBook { sets: vec![s], ..SetBook::default() }.save(store).unwrap(); + + let asks = |scope: &str| if scope == "write" { vec!["write", "read"] } else { vec!["read"] }; + let granted = needs.iter().all(|n| asks(n).iter().all(|a| grants.contains(a))); + let lasts = match lasting { + Lasting::Refreshed | Lasting::Ever => true, + Lasting::TwoHours => !big, + Lasting::HalfAnHour => false, + }; + let expected = match mistake { + _ if !given => Some(Reason::NoAccess), + Some(Mistake::SignedOut | Mistake::Lost) => Some(Reason::NotSignedIn), + Some(Mistake::Ungiven) => Some(Reason::MissingScope), + None if !granted => Some(Reason::MissingScope), + None if !lasts => Some(Reason::NotSignedIn), + None => None, + }; + let asked = || std::fs::read_to_string(Fixture::kept(store).with_file_name("asked")).unwrap_or_default(); + let started = match publishing { + true => m.f.plugins.publish_plan("echo:nowhere", "last", &[], store, &Ctx::silent()).map(Some), + false => m.f.plugins.store_destination("echo:folder", Some(store)).map(|_| None), + }; + let said = format!("{lasting:?} {mistake:?} given {given} needs {needs:?} grants {grants:?} big {big}"); + let Some(reason) = expected else { + let Some(plan) = started.unwrap() else { return }; + assert_eq!(asked().lines().count(), 1, "{said}: the dry run"); + let later = tc.draw(gs::booleans()); + if later { + m.f.plugins.sign_out_account(WITH, "a", store).unwrap(); + } + let done = plan.apply(Some(store), &Ctx::silent()); + assert_eq!(done.is_ok(), !later, "{said}"); + if later { + assert_eq!(done.unwrap_err().reason(), Some(Reason::NotSignedIn), "{said}"); + assert_eq!(asked().lines().count(), 1, "{said}: asked to publish"); + } + return; + }; + let e = started.err().unwrap(); + assert_eq!((e.code(), e.reason()), ("account", Some(reason)), "{said}: {e}"); + assert!(asked().is_empty(), "{said}: echo was asked"); + let actions = e.detail().actions; + let again = actions.first().is_some_and(|a| a.params.get("account") == Some(&json!("a"))); + assert_eq!(again, given, "{said}: {actions:?}"); +} + /// A step of [`a_store_copied_through_a_plugin_comes_back_as_it_was_last_copied`]. #[derive(Debug, Clone, Copy)] enum Step { From b8e254577e318e3e768155d7f1150d5dcb5edea2 Mon Sep 17 00:00:00 2001 From: botandbang <337765928+botandbang@users.noreply.github.com> Date: Fri, 30 Oct 2026 13:28:24 -0400 Subject: [PATCH 06/13] feat(locallibrary): refresh once when a service refuses a token, and read insufficient_scope --- apps/locallibrary/docs/plugins.md | 15 +- crates/locallibrary/src/plugin/accounts.rs | 2 +- .../src/plugin/accounts/shared.rs | 47 +++- .../locallibrary/src/plugin/accounts/token.rs | 88 ++++++-- crates/locallibrary/src/plugin/net.rs | 200 +++++++++++++----- .../locallibrary/src/plugin/net/challenge.rs | 96 +++++++++ .../src/plugin/net/challenge/tests.rs | 68 ++++++ .../locallibrary/tests/it/plugins/accesses.rs | 142 +++++++++++++ .../tests/it/plugins/shared_accounts.rs | 2 +- .../locallibrary/tests/support/mock_oauth.rs | 10 +- 10 files changed, 591 insertions(+), 79 deletions(-) create mode 100644 crates/locallibrary/src/plugin/net/challenge.rs create mode 100644 crates/locallibrary/src/plugin/net/challenge/tests.rs diff --git a/apps/locallibrary/docs/plugins.md b/apps/locallibrary/docs/plugins.md index 86a3bdecb..f6f7f7657 100644 --- a/apps/locallibrary/docs/plugins.md +++ b/apps/locallibrary/docs/plugins.md @@ -235,7 +235,6 @@ account is signed out, won't stay signed in or wasn't granted what it needs asks one whose access lacks a scope it needs asks to give it. With `--json`, `--yes` or no terminal, and in the sidecar, nothing is asked: it fails and says how to resolve it. - In the player's palette, "sign in to a TYPE account…" signs in to one, "sign out of NAME" signs out of one, and "access for PLUGIN…" opens a form choosing which account of each type the plugin uses, saying what for, or signing in to another. @@ -245,7 +244,10 @@ and refreshed a minute before they expire. A plugin never sees them: a request i account by its type (`{account:TYPE}`) where the type says its token goes, the `Authorization` header unless the service only takes it in the address (Mixcloud), and locallibrary fills in the token once it has checked the request against the scopes of the plugin's access, never sending it -on to another server a redirect leads to. +on to another server a redirect leads to. When the service refuses the token (a 401), locallibrary +refreshes it and sends the request once more; one it can't refresh is signed out, and a second +refusal saying the token is no good signs it out too. When the service says the token lacks a +scope, the request fails saying which, and signing in again is offered. Some services (SoundCloud) only take the browser back at the exact address their app was registered with, port included. An account type on one of those names its port, and its @@ -571,10 +573,11 @@ such a request names it (an account's token, only where its account type says it of the plugin's access covers), filled in by locallibrary, never on to another server a redirect leads to, and is left out of the plugin's log and of the errors it's told. What a server answers is passed on as it is, but for an account's -token sent in the address, which comes back as the placeholder that named it. The files a plugin -sends are those in its request's folder (see below) and a set's recording while it publishes the -set, nothing else. A destination is given stored files to read, never the store's folder to -write: what it gives back is checked before locallibrary keeps it. +token sent in the address, which comes back as the placeholder that named it, and a server +refusing an account's token, which locallibrary answers itself (see [Accounts](#accounts)). The +files a plugin sends are those in its request's folder (see below) and a set's recording while it +publishes the set, nothing else. A destination is given stored files to read, never the store's +folder to write: what it gives back is checked before locallibrary keeps it. Images and other large answers come back as files in a folder locallibrary makes for each request (`plugins/ID/scratch/`), which it reads itself, never past the folder (nor a link, or a file with another name) or more than 512 MiB of them, and removes; `store gc` removes those a locallibrary diff --git a/crates/locallibrary/src/plugin/accounts.rs b/crates/locallibrary/src/plugin/accounts.rs index 8964157b7..a0bb47b9c 100644 --- a/crates/locallibrary/src/plugin/accounts.rs +++ b/crates/locallibrary/src/plugin/accounts.rs @@ -43,7 +43,7 @@ pub(super) mod access; mod kept; pub(super) mod ready; pub(super) mod shared; -mod token; +pub(super) mod token; mod whoami; /// How long before its access token expires an account's is refreshed. diff --git a/crates/locallibrary/src/plugin/accounts/shared.rs b/crates/locallibrary/src/plugin/accounts/shared.rs index e854cffd9..3173fca7f 100644 --- a/crates/locallibrary/src/plugin/accounts/shared.rs +++ b/crates/locallibrary/src/plugin/accounts/shared.rs @@ -163,6 +163,19 @@ impl Plugin { /// it's about to expire: one refresh at a time, whichever host on the store asks, each reading /// the tokens again once it's its turn. One the service won't refresh any more is signed out. pub(crate) fn account_token(&self, store: &Store, subject: &str) -> Result { + self.token(store, subject, None) + } + + /// The access token of the account `subject` it signed in to in `store` to use instead of + /// `refused`, which its service refused: the one another host refreshed it to since, else + /// `refused` refreshed (as [`Plugin::account_token`] refreshes). Without a refresh token, or + /// one the service won't refresh any more, it's signed out. + pub(crate) fn account_token_refused(&self, store: &Store, subject: &str, refused: &str) -> Result { + self.token(store, subject, Some(refused)) + } + + /// [`Plugin::account_token`], or with `refused`, [`Plugin::account_token_refused`]. + fn token(&self, store: &Store, subject: &str, refused: Option<&str>) -> Result { let with = &self.manifest.id; let _one = kept::hold(store, with, subject)?; let k = find(store, with, subject)?; @@ -172,11 +185,20 @@ impl Plugin { Error::Account(why).because(Reason::SessionEnded) }; let tokens = self.kept_tokens(store, &k)?.ok_or_else(signed_out)?; - if !tokens.expiring(Utc::now()) { + let stale = match refused { + Some(refused) => tokens.access_token == refused, + None => tokens.expiring(Utc::now()), + }; + if !stale { return Ok(tokens.access_token); } - let Some(refresh) = tokens.refresh_token.clone() else { return Err(signed_out()) }; let storage = self.host.options.secrets; + let Some(refresh) = tokens.refresh_token.clone() else { + if refused.is_some() { + forget(store, storage, &k)?; + } + return Err(signed_out()); + }; let Some(fresh) = self.refreshed(t, refresh, tokens)? else { forget(store, storage, &k)?; return Err(signed_out()); @@ -187,6 +209,27 @@ impl Plugin { })?; Ok(fresh.access_token) } + + /// Signs out of the account `subject` it signed in to in `store` when its access token is + /// still `refused`, which its service said is no good. + pub(crate) fn forget_refused(&self, store: &Store, subject: &str, refused: &str) -> Result<()> { + let _one = kept::hold(store, &self.manifest.id, subject)?; + let Some(k) = kept::read(store)?.account(&self.manifest.id, subject).cloned() else { return Ok(()) }; + match self.kept_tokens(store, &k)? { + Some(tokens) if tokens.access_token == refused => forget(store, self.host.options.secrets, &k), + _ => Ok(()), + } + } + + /// The account `subject` it signed in to in `store` wasn't granted `asks` after all, as its + /// service said. + pub(crate) fn not_granted(&self, store: &Store, subject: &str, asks: &[String]) -> Result<()> { + kept::update(store, |book| { + let k = book.accounts.iter_mut().filter(|k| k.is(&self.manifest.id, subject)); + k.for_each(|k| k.scopes.retain(|s| !asks.contains(s))); + Ok(()) + }) + } } impl Plugins { diff --git a/crates/locallibrary/src/plugin/accounts/token.rs b/crates/locallibrary/src/plugin/accounts/token.rs index 7e42d4c2a..642abf41e 100644 --- a/crates/locallibrary/src/plugin/accounts/token.rs +++ b/crates/locallibrary/src/plugin/accounts/token.rs @@ -5,7 +5,10 @@ //! plugin needs. Anything else fails before the request is sent: `access` when the plugin has no //! such access or the account is signed out (`reason` `signed_out`), `scope` when its scopes don't //! reach, with the type, the scopes and the account plugins in its fields, and once there's an -//! access, its id, its account and the scopes signing in again `needs` to allow. +//! access, its id, its account and the scopes signing in again `needs` to allow. A service +//! refusing the token is answered here too: it's refreshed for the request to be sent again (see +//! [`Plugin::refused_token`]), signed out (see [`Plugin::invalid_token`]), or no longer granted +//! the scopes the service says it lacks (see [`Plugin::scope_refused`]). use std::sync::Arc; @@ -15,18 +18,21 @@ use serde_json::{Map, json}; use super::access::Access; use super::kept; +use crate::Error; use crate::error::Reason; use crate::plugin::process::{Live, Plugin}; use crate::store::Store; -/// An access a request may use: where its account's token goes, and the scopes it may use it -/// for (those the account was granted what they ask), which a redirect has to stay within. +/// An access a request may use, of the type `need` asks for: where its account's token goes, and +/// the scopes (by name) it may use it for (those the account was granted what they ask), which a +/// redirect has to stay within. pub(crate) struct Usable { /// The account plugin, which gets the token. with: Arc, access: Access, + need: AccountNeed, pub(crate) place: TokenPlace, - pub(crate) scopes: Vec, + pub(crate) scopes: Vec<(String, Scope)>, } /// A failure of kind `kind` naming the type `need` asks for, `scopes` and the account `plugins`. @@ -112,21 +118,75 @@ impl Plugin { let why = format!("the {kind} account {at} wasn't granted what {} asks: sign in again", names.join(", ")); return Err(scope_failed(why, &names, &names)); } - let scopes = scopes().filter(|(_, s)| is_granted(s)).map(|(_, s)| s.clone()).collect(); - Ok(Usable { with, access: a.clone(), place: t.token.clone(), scopes }) + let scopes = scopes().filter(|(_, s)| is_granted(s)).map(|(n, s)| (n.clone(), s.clone())).collect(); + Ok(Usable { with, access: a.clone(), need: need.clone(), place: t.token.clone(), scopes }) } /// The token of `usable`'s account, told to [`crate::plugin::net::State::scrub`]: `access` /// (`signed_out`) when it's signed out, or the service signs it out. - pub(crate) fn usable_token(&self, usable: &Usable, need: &AccountNeed, store: &Store) -> Result { - let token = usable.with.account_token(store, &usable.access.account.subject).map_err(|e| { - let why = self.net.scrub(&e.to_string()); - match e.reason() { - Some(Reason::SessionEnded) => signed_out(why, need, &usable.access), - _ => RpcError::failed("external", why), - } - })?; + pub(crate) fn usable_token(&self, usable: &Usable, store: &Store) -> Result { + let token = usable.with.account_token(store, &usable.access.account.subject); + let token = token.map_err(|e| self.token_failed(usable, e))?; self.net.saw(&token); Ok(token) } + + /// `e`, which getting `usable`'s token failed with, as the plugin is told it. + fn token_failed(&self, usable: &Usable, e: Error) -> RpcError { + let why = self.net.scrub(&e.to_string()); + match e.reason() { + Some(Reason::SessionEnded) => signed_out(why, &usable.need, &usable.access), + _ => RpcError::failed("external", why), + } + } + + /// Its service refused `token`, `usable`'s account's: the account's token is refreshed, unless + /// another host already has (see [`Plugin::account_token_refused`]), for the request to be + /// sent again with it. `access` (`signed_out`) when the service won't refresh it. + pub(crate) fn refused_token(&self, usable: &Usable, token: &str, store: &Store) -> Result<(), RpcError> { + let subject = &usable.access.account.subject; + usable.with.account_token_refused(store, subject, token).map(drop).map_err(|e| self.token_failed(usable, e)) + } + + /// Its service refused `token` again, saying it's no good: `usable`'s account is signed out, + /// unless another host signed it in again since. + pub(crate) fn invalid_token(&self, usable: &Usable, token: &str, store: &Store) -> Result<(), RpcError> { + let subject = &usable.access.account.subject; + usable.with.forget_refused(store, subject, token).map_err(|e| RpcError::failed("external", e.to_string())) + } + + /// Its service said `usable`'s token hasn't the scopes a `method` request to `url` (as the + /// plugin wrote it) needs, which it calls `asked` when it says: `scope`, naming the type's + /// scopes whose asks are among them (else those of `usable`'s covering the request), which + /// signing in again `needs` to allow. What the account was granted no longer has `asked` (else + /// what those ask), so nothing through them is sent again until it's signed in again. + pub(crate) fn scope_refused( + &self, + usable: &Usable, + asked: &[String], + method: &str, + url: &Url, + store: &Store, + ) -> RpcError { + let (kind, at, on) = (&usable.need.name, &usable.access.name, &usable.access.account.plugin); + let t = usable.with.account_type(kind).ok(); + let meeting = t.iter().flat_map(|t| &t.scopes).filter(|(_, s)| s.asks.iter().any(|a| asked.contains(a))); + let mut needs: Vec<(&String, &Scope)> = meeting.collect(); + if needs.is_empty() { + needs = usable.scopes.iter().filter(|(_, s)| s.covers(method, url)).map(|(n, s)| (n, s)).collect(); + } + let ungranted: Vec = match asked { + [] => needs.iter().flat_map(|(_, s)| s.asks.iter().cloned()).collect(), + asked => asked.to_vec(), + }; + let needs: Vec = needs.into_iter().map(|(n, _)| n.clone()).collect(); + if let Err(e) = usable.with.not_granted(store, &usable.access.account.subject, &ungranted) { + return RpcError::failed("external", e.to_string()); + } + let why = format!( + "the service says the {kind} account {at} wasn't granted what {} asks: sign in again", + needs.join(", ") + ); + about(failed("scope", why, &usable.need, &needs, vec![on.clone()]), &usable.access, &needs) + } } diff --git a/crates/locallibrary/src/plugin/net.rs b/crates/locallibrary/src/plugin/net.rs index 033c2e004..46d303b75 100644 --- a/crates/locallibrary/src/plugin/net.rs +++ b/crates/locallibrary/src/plugin/net.rs @@ -10,11 +10,13 @@ //! checked, never on to another server a redirect leads to, nor to an address no scope of the //! access covers, and left out of its log and the errors it's told (see [`State::scrub`]); what a //! server answers is passed on as it is, but for a token it was sent in the query, given back as -//! the placeholder it was named with. +//! the placeholder it was named with, and a server refusing a token it was sent, which the host +//! answers (see [`Plugin::answered`]). //! Answers are given up to [`super::Options::max_body`], or written to a file in the request's //! scratch folder; nothing is kept. What it sends is text, or files it was given (see [`body`]). mod body; +mod challenge; use std::fs::File; use std::io::{Read, Write}; @@ -24,7 +26,7 @@ use std::time::{Duration, Instant}; use indexmap::IndexMap; use loclib_plugin::{ - ACCOUNT_PLACEHOLDER, AccountNeed, Capability, Fetch, Fetched, FormPart, RpcError, Scope, SettingType, TokenPlace, + ACCOUNT_PLACEHOLDER, AccountNeed, Capability, Fetch, Fetched, FormPart, RpcError, SettingType, TokenPlace, account_placeholder, }; use reqwest::blocking::{Client, Response}; @@ -36,6 +38,8 @@ use reqwest::{Method, Url}; use serde_json::Value; use self::body::{Body, Field, FilePart}; +use self::challenge::{Challenge, challenges, insufficient_scope}; +use super::accounts::token::Usable; use super::blobs; use super::process::{Live, Plugin}; use crate::export::one_line; @@ -110,8 +114,24 @@ struct Sent { body: Option, /// Whether the body has a secret in it. secret: bool, - /// For each account of a type whose token is filled in, the scopes it may go to. - scopes: Vec>, + /// The accounts whose tokens are filled in, which go only where their scopes cover. + accounts: Vec, +} + +/// An account's token filled into a request, and the access it's through. +struct Filled { + usable: Usable, + token: String, +} + +/// The accounts' tokens filled into a request's query, and the placeholders that named them. +type InQuery = Vec<(String, String)>; + +/// What a request was answered: its status and headers, and its body, to be read. +struct Answer { + status: u16, + headers: HeaderMap, + body: Box, } fn denied(why: String) -> RpcError { @@ -244,9 +264,94 @@ impl Plugin { } } - /// Answers the plugin's `net/fetch` (see the module's docs). + /// Answers the plugin's `net/fetch` (see the module's docs). A server refusing an account's + /// token, on the last hop it went to, is answered for the plugin (see [`Plugin::answered`]). pub(crate) fn fetch(&self, live: &Live, params: Value, cancel: &CancelToken) -> Result { let req: Fetch = loclib_plugin::params(params)?; + let (url, sent, in_query) = self.prepare(live, &req)?; + // Before it's sent: an answer with nowhere to go is never asked for. + let into = req.into_blob.as_deref().map(|blob| self.new_blob(live, blob)).transpose()?; + let (response, attached) = self.follow(live, url, sent, cancel)?; + let (answer, in_query) = self.answered(live, &req, response, attached, in_query, cancel)?; + // A server says the address it was asked at back (a next page's link, a `Location`): a token + // sent in the query is given back as the placeholder that named it, which asks for it again. + let named = |text: String| { + in_query.iter().fold(text, |text, (token, placeholder)| { + let encoded: String = form_urlencoded::byte_serialize(token.as_bytes()).collect(); + text.replace(token, placeholder).replace(&encoded, placeholder) + }) + }; + let mut answered: IndexMap = IndexMap::new(); + for (name, value) in &answer.headers { + let value = named(String::from_utf8_lossy(value.as_bytes()).into_owned()); + answered.entry(name.as_str().into()).and_modify(|v| *v = format!("{v}, {value}")).or_insert(value); + } + let fetched = Fetched { status: answer.status, headers: answered, body: None, blob: None }; + let fetched = match into { + Some(into) => Fetched { blob: Some(self.write_blob(into, answer.body)?), ..fetched }, + None => Fetched { body: Some(named(self.text(answer.body)?)), ..fetched }, + }; + serde_json::to_value(fetched).map_err(|e| RpcError::internal(e.to_string())) + } + + /// `response`, which `req` was answered on a hop it went to with the tokens `attached`, or + /// what's answered instead when the server refused one (a 401 or 403; not after a hop that + /// dropped them, which isn't about them): saying the token lacks scopes (see + /// [`insufficient_scope`]), `scope`, its body not passed on (see [`Plugin::scope_refused`]); + /// a 401 otherwise, the request sent again once with the account's token refreshed (see + /// [`Plugin::refused_token`]), and its answer passed on, the account signed out when it's a + /// 401 again saying `error="invalid_token"`. With the placeholders of the tokens the request + /// that was answered had in its query. + fn answered( + &self, + live: &Live, + req: &Fetch, + mut response: Response, + mut attached: Vec, + mut in_query: InQuery, + cancel: &CancelToken, + ) -> Result<(Answer, InQuery), RpcError> { + let mut again = false; + loop { + let (status, headers) = (response.status().as_u16(), response.headers().clone()); + let Some(store) = live.store.as_ref().filter(|_| !attached.is_empty() && matches!(status, 401 | 403)) + else { + return Ok((Answer { status, headers, body: Box::new(response) }, in_query)); + }; + let said = headers.get_all(WWW_AUTHENTICATE).iter().filter_map(|v| v.to_str().ok()); + let said: Vec = said.flat_map(challenges).collect(); + let mut body = Vec::new(); + (&mut response) + .take(self.host.options.max_body) + .read_to_end(&mut body) + .map_err(|e| RpcError::failed("external", self.net.scrub(&with_causes(&e))))?; + if let Some(scopes) = insufficient_scope(status, &said, &body) { + let written = Url::parse(&req.url).map_err(|_| invalid("it isn't a web address"))?; + return Err(self.scope_refused(&attached[0].usable, &scopes, &req.method, &written, store)); + } + let body: Box = Box::new(std::io::Cursor::new(body).chain(response)); + if status == 403 || again { + let invalid = again && status == 401 && said.iter().any(|c| c.param("error") == Some("invalid_token")); + if invalid { + for f in &attached { + self.invalid_token(&f.usable, &f.token, store)?; + } + } + return Ok((Answer { status, headers, body }, in_query)); + } + for f in &attached { + self.refused_token(&f.usable, &f.token, store)?; + } + again = true; + let (url, sent, query) = self.prepare(live, req)?; + in_query = query; + (response, attached) = self.follow(live, url, sent, cancel)?; + } + } + + /// The request `req` asks for, as it's sent to its URL, its secrets and accounts' tokens + /// filled in: with the tokens filled into its query, and the placeholders that named them. + fn prepare(&self, live: &Live, req: &Fetch) -> Result<(Url, Sent, InQuery), RpcError> { let mut url = Url::parse(&req.url).map_err(|_| invalid(format!("{} isn't a web address", one_line(&req.url))))?; if let Some(why) = refused(&self.manifest.capabilities, &url, self.loopback()).or_else(|| self.offline(&url)) { @@ -270,7 +375,7 @@ impl Plugin { // What a scope covers is the request as the plugin wrote it. let asked = (method.clone(), url.clone()); let (plain, filled) = (HeaderMap::new(), HeaderMap::new()); - let mut sent = Sent { method, plain, filled, body: None, secret: false, scopes: Vec::new() }; + let mut sent = Sent { method, plain, filled, body: None, secret: false, accounts: Vec::new() }; // The accounts' tokens filled into the query, and the placeholders that named them. let mut in_query = Vec::new(); if url.query_pairs().any(|(_, v)| v.contains(SECRET) || v.contains(ACCOUNT_PLACEHOLDER)) { @@ -282,7 +387,7 @@ impl Plugin { filled.push((k, self.fill(live, &v)?)); continue; } - let token = self.account(live, &v, Some(&k), &asked, &mut sent.scopes)?; + let token = self.account(live, &v, Some(&k), &asked, &mut sent.accounts)?; filled.push((k, token.clone())); in_query.push((token, v)); } @@ -299,8 +404,9 @@ impl Plugin { if name != AUTHORIZATION { return Err(self.misplaced(value)); } - let mut filled = HeaderValue::from_str(&self.account(live, value, None, &asked, &mut sent.scopes)?) - .map_err(|_| RpcError::failed("external", "the account's token can't be sent in a header"))?; + let mut filled = + HeaderValue::from_str(&self.account(live, value, None, &asked, &mut sent.accounts)?) + .map_err(|_| RpcError::failed("external", "the account's token can't be sent in a header"))?; filled.set_sensitive(true); sent.filled.append(AUTHORIZATION, filled); continue; @@ -325,43 +431,21 @@ impl Plugin { sent.plain.remove(CONTENT_TYPE); sent.body = Some(Body::Form(self.form(live, &req.multipart)?)); } - // Before it's sent: an answer with nowhere to go is never asked for. - let into = req.into_blob.as_deref().map(|blob| self.new_blob(live, blob)).transpose()?; - let response = self.follow(live, url, sent, cancel)?; - let status = response.status().as_u16(); - // A server says the address it was asked at back (a next page's link, a `Location`): a token - // sent in the query is given back as the placeholder that named it, which asks for it again. - let named = |text: String| { - in_query.iter().fold(text, |text, (token, placeholder)| { - let encoded: String = form_urlencoded::byte_serialize(token.as_bytes()).collect(); - text.replace(token, placeholder).replace(&encoded, placeholder) - }) - }; - let mut answered: IndexMap = IndexMap::new(); - for (name, value) in response.headers() { - let value = named(String::from_utf8_lossy(value.as_bytes()).into_owned()); - answered.entry(name.as_str().into()).and_modify(|v| *v = format!("{v}, {value}")).or_insert(value); - } - let fetched = Fetched { status, headers: answered, body: None, blob: None }; - let fetched = match into { - Some(into) => Fetched { blob: Some(self.write_blob(into, response)?), ..fetched }, - None => Fetched { body: Some(named(self.text(response)?)), ..fetched }, - }; - serde_json::to_value(fetched).map_err(|e| RpcError::internal(e.to_string())) + Ok((url, sent, in_query)) } /// What goes where a request (`asked`, its method and URL as the plugin wrote it) names an /// account by type as the whole of `value`, `{account:}` of a type the plugin asks for: /// in the `Authorization` header (`query` is `None`), its scheme and access token; as the query /// parameter `query`, the token. Only where the account type says its token goes, and where the - /// plugin's access may go, which `scopes` gets (see [`Plugin::usable`]). + /// plugin's access may go (see [`Plugin::usable`]), which `accounts` gets. fn account( &self, live: &Live, value: &str, query: Option<&str>, asked: &(Method, Url), - scopes: &mut Vec>, + accounts: &mut Vec, ) -> Result { let Some((need, access)) = self.by_type(value) else { return Err(match account_placeholder(value) { @@ -372,13 +456,17 @@ impl Plugin { let (method, url) = asked; let usable = self.usable(live, need, access, method.as_str(), url)?; let store = live.store.as_ref().ok_or_else(|| invalid("there's no store"))?; - let token = || self.usable_token(&usable, need, store); - let filled = match (&usable.place, query) { - (TokenPlace::Scheme(scheme), None) => format!("{scheme} {}", token()?), - (TokenPlace::Query(name), Some(query)) if name == query => token()?, - (place, _) => return Err(misplaced(Some(place))), + let filled = |token: &str| match (&usable.place, query) { + (TokenPlace::Scheme(scheme), None) => Some(format!("{scheme} {token}")), + (TokenPlace::Query(name), Some(query)) if name == query => Some(token.to_string()), + _ => None, }; - scopes.push(usable.scopes); + if filled("").is_none() { + return Err(misplaced(Some(&usable.place))); + } + let token = self.usable_token(&usable, store)?; + let filled = filled(&token).unwrap_or_default(); + accounts.push(Filled { usable, token }); Ok(filled) } @@ -414,12 +502,18 @@ impl Plugin { } /// Sends `sent` to `url`, and follows the redirects it's answered with as browsers do, each - /// to where the plugin may go ([`refused`]) and in its turn: its last answer. Once one leads - /// to another server, or where no scope of an access whose token it carries covers, the - /// headers its secrets were filled into are left out from then on; once it's another server, - /// those reqwest never sends on to another (its credentials) too, and a body with a secret in - /// it is never sent there. - fn follow(&self, live: &Live, mut url: Url, mut sent: Sent, cancel: &CancelToken) -> Result { + /// to where the plugin may go ([`refused`]) and in its turn: its last answer, and the accounts + /// whose tokens went with it. Once one leads to another server, or where no scope of an + /// access whose token it carries covers, the headers its secrets were filled into are left + /// out from then on; once it's another server, those reqwest never sends on to another (its + /// credentials) too, and a body with a secret in it is never sent there. + fn follow( + &self, + live: &Live, + mut url: Url, + mut sent: Sent, + cancel: &CancelToken, + ) -> Result<(Response, Vec), RpcError> { let external = |why: String| RpcError::failed("external", self.net.scrub(&why)); let mut hops = 0; loop { @@ -443,7 +537,7 @@ impl Plugin { let response = sending.map_err(|e| external(with_causes(&e)))?; let location = response.headers().get(LOCATION).and_then(|l| l.to_str().ok()); let (Some(location), 301 | 302 | 303 | 307 | 308) = (location, response.status().as_u16()) else { - return Ok(response); + return Ok((response, sent.accounts)); }; if hops == MAX_REDIRECTS { return Err(external(format!("too many redirects (it follows {MAX_REDIRECTS})"))); @@ -470,14 +564,14 @@ impl Plugin { } let leaves = next.origin() != url.origin(); let method = sent.method.as_str(); - let uncovered = sent.scopes.iter().any(|scopes| !scopes.iter().any(|s| s.covers(method, &next))); + let uncovered = sent.accounts.iter().any(|f| !f.usable.scopes.iter().any(|(_, s)| s.covers(method, &next))); if leaves || uncovered { // The server put a secret in the address it sent the request on to: it stays here. if self.net.scrub(next.as_str()) != next.as_str() { return Err(external(format!("redirected to {next}, which a secret in its address isn't for"))); } sent.filled.clear(); - sent.scopes.clear(); + sent.accounts.clear(); } if leaves { if sent.secret { @@ -492,10 +586,10 @@ impl Plugin { } /// An answer's body as text, up to [`super::Options::max_body`]. - fn text(&self, response: Response) -> Result { + fn text(&self, answer: impl Read) -> Result { let most = self.host.options.max_body; let mut body = Vec::new(); - response + answer .take(most + 1) .read_to_end(&mut body) .map_err(|e| RpcError::failed("external", self.net.scrub(&with_causes(&e))))?; @@ -517,10 +611,10 @@ impl Plugin { /// Writes an answer's body to `into`, as far as its folder has room (see /// [`super::Options::max_blobs`]). - fn write_blob(&self, mut into: NewBlob, response: Response) -> Result { + fn write_blob(&self, mut into: NewBlob, answer: impl Read) -> Result { let room = into.room; let copied = - std::io::copy(&mut response.take(room + 1), &mut into.out).and_then(|n| into.out.flush().map(|()| n)); + std::io::copy(&mut answer.take(room + 1), &mut into.out).and_then(|n| into.out.flush().map(|()| n)); match copied { Ok(n) if n > room => Err(RpcError::failed( "too_big", diff --git a/crates/locallibrary/src/plugin/net/challenge.rs b/crates/locallibrary/src/plugin/net/challenge.rs new file mode 100644 index 000000000..f8fa8877e --- /dev/null +++ b/crates/locallibrary/src/plugin/net/challenge.rs @@ -0,0 +1,96 @@ +//! What a server refusing a request's token says why: the auth-params of its `WWW-Authenticate` +//! challenges (RFC 7235 2.1), such as RFC 6750's `error="insufficient_scope"` and the `scope` it +//! needs, and Google's `ACCESS_TOKEN_SCOPE_INSUFFICIENT` in a JSON body. + +use serde_json::Value; + +/// A challenge: its scheme, and its auth-params (names in lowercase), in order. +#[derive(Debug, Clone, PartialEq)] +pub(super) struct Challenge { + pub scheme: String, + pub params: Vec<(String, String)>, +} + +impl Challenge { + /// The value of its auth-param `name`. + pub fn param(&self, name: &str) -> Option<&str> { + self.params.iter().find(|(n, _)| n == name).map(|(_, v)| v.as_str()) + } +} + +/// Whether `c` is a character of a token (RFC 7230 3.2.6). +fn is_tchar(c: char) -> bool { + c.is_ascii_alphanumeric() || "!#$%&'*+-.^_`|~".contains(c) +} + +/// The challenges a `WWW-Authenticate` header's `value` makes: each a scheme, then auth-params +/// (`name=token` or `name="quoted string"`) separated by commas. A token68 (`Basic dGVzdA==`) +/// isn't understood, and what can't be read is skipped. +pub(super) fn challenges(value: &str) -> Vec { + let mut all: Vec = Vec::new(); + let mut rest = value; + loop { + rest = rest.trim_start_matches([' ', '\t', ',']); + let end = rest.find(|c| !is_tchar(c)).unwrap_or(rest.len()); + let (token, after) = rest.split_at(end); + if token.is_empty() { + match after.chars().next() { + Some(c) => rest = &after[c.len_utf8()..], + None => return all, + } + continue; + } + let after = after.trim_start_matches([' ', '\t']); + let Some(value) = after.strip_prefix('=') else { + all.push(Challenge { scheme: token.into(), params: Vec::new() }); + rest = after; + continue; + }; + let value = value.trim_start_matches([' ', '\t']); + let (said, left) = match value.strip_prefix('"') { + Some(quoted) => unquoted(quoted), + None => { + let end = value.find(|c| !is_tchar(c)).unwrap_or(value.len()); + (value[..end].to_string(), &value[end..]) + } + }; + if let Some(c) = all.last_mut() { + c.params.push((token.to_ascii_lowercase(), said)); + } + rest = left; + } +} + +/// A quoted string's text, `quoted` being what follows its opening quote, and what follows its +/// closing one (none when it isn't closed). +fn unquoted(quoted: &str) -> (String, &str) { + let mut text = String::new(); + let mut chars = quoted.char_indices(); + while let Some((i, c)) = chars.next() { + match c { + '"' => return (text, "ed[i + 1..]), + '\\' => text.extend(chars.next().map(|(_, c)| c)), + c => text.push(c), + } + } + (text, "") +} + +/// The scopes (as the service names them, maybe none) a server refusing a request with `status` +/// says its token lacks, when that's why: a 401 or 403 whose challenge says +/// `error="insufficient_scope"` (with them in `scope`), or a 403 whose JSON `body` says +/// `ACCESS_TOKEN_SCOPE_INSUFFICIENT` (Google's), not just `insufficientPermissions`. +pub(super) fn insufficient_scope(status: u16, challenges: &[Challenge], body: &[u8]) -> Option> { + let said = challenges.iter().find(|c| c.param("error") == Some("insufficient_scope")); + if let Some(c) = said.filter(|_| matches!(status, 401 | 403)) { + return Some(c.param("scope").unwrap_or_default().split_whitespace().map(String::from).collect()); + } + let json: Value = serde_json::from_slice(body).ok().filter(|_| status == 403)?; + let details = json.pointer("/error/details").and_then(Value::as_array)?; + let google = + details.iter().any(|d| d.get("reason").and_then(Value::as_str) == Some("ACCESS_TOKEN_SCOPE_INSUFFICIENT")); + google.then(Vec::new) +} + +#[cfg(test)] +mod tests; diff --git a/crates/locallibrary/src/plugin/net/challenge/tests.rs b/crates/locallibrary/src/plugin/net/challenge/tests.rs new file mode 100644 index 000000000..f381c6e8a --- /dev/null +++ b/crates/locallibrary/src/plugin/net/challenge/tests.rs @@ -0,0 +1,68 @@ +use hegel::TestCase; +use hegel::generators as gs; + +use super::*; + +/// A token: a scheme's or an auth-param's name, or a value that needs no quotes. +fn token() -> impl hegel::PrintableGenerator { + gs::from_regex("[A-Za-z0-9!#$%&'*+.^_`|~-]{1,8}").fullmatch(true) +} + +/// For any challenges (each a scheme and auth-params, any value written as a token when it can be +/// or quoted, its quotes and backslashes escaped, with any spaces around `=` and between them), +/// what a `WWW-Authenticate` header says is read back as they were, names in lowercase. +#[hegel::test(test_cases = 200)] +fn a_challenge_reads_back_as_it_was_written(tc: TestCase) { + let made: Vec<(String, Vec<(String, String)>)> = tc.draw( + gs::vecs(gs::tuples!( + token(), + gs::vecs(gs::tuples!(token(), hegel::one_of!(token(), gs::text().max_size(12)))).max_size(3) + )) + .min_size(1) + .max_size(3), + ); + let space = || tc.draw(gs::sampled_from(vec!["", " ", " ", "\t"])); + let mut header = Vec::new(); + for (scheme, params) in &made { + let params: Vec = params + .iter() + .map(|(name, value)| { + let quoted = value.is_empty() || !value.chars().all(is_tchar) || tc.draw(gs::booleans()); + let value = match quoted { + true => format!("\"{}\"", value.replace('\\', "\\\\").replace('"', "\\\"")), + false => value.clone(), + }; + format!("{name}{}={}{value}", space(), space()) + }) + .collect(); + header.push(format!("{scheme} {}", params.join(&format!(",{}", space())))); + } + let header = header.join(", "); + let read = challenges(&header); + let expected: Vec = made + .into_iter() + .map(|(scheme, params)| Challenge { + scheme, + params: params.into_iter().map(|(n, v)| (n.to_ascii_lowercase(), v)).collect(), + }) + .collect(); + assert_eq!(read, expected, "{header:?}"); +} + +/// A token's scopes are insufficient when a 401 or 403's challenge says so (naming them or not), +/// or a 403's JSON body says Google's `ACCESS_TOKEN_SCOPE_INSUFFICIENT`; not for another error, a +/// body saying only `insufficientPermissions`, or another status. +#[test] +fn insufficient_scope_is_read_from_the_challenge_or_googles_body() { + let said = |status, header: &str, body: &str| insufficient_scope(status, &challenges(header), body.as_bytes()); + let scoped = r#"Bearer realm="x", error="insufficient_scope", scope="a b""#; + assert_eq!(said(403, scoped, ""), Some(vec!["a".to_string(), "b".to_string()])); + assert_eq!(said(401, r#"DPoP error="insufficient_scope""#, ""), Some(Vec::new())); + assert_eq!(said(400, scoped, ""), None); + assert_eq!(said(401, r#"Bearer error="invalid_token""#, ""), None); + let google = r#"{"error":{"errors":[{"reason":"insufficientPermissions"}],"details":[{"reason":"ACCESS_TOKEN_SCOPE_INSUFFICIENT"}]}}"#; + assert_eq!(said(403, "", google), Some(Vec::new())); + assert_eq!(said(401, "", google), None); + assert_eq!(said(403, "", r#"{"error":{"errors":[{"reason":"insufficientPermissions"}]}}"#), None); + assert_eq!(said(403, "", "not json"), None); +} diff --git a/crates/locallibrary/tests/it/plugins/accesses.rs b/crates/locallibrary/tests/it/plugins/accesses.rs index 8a8a17e7c..07eb1f3c2 100644 --- a/crates/locallibrary/tests/it/plugins/accesses.rs +++ b/crates/locallibrary/tests/it/plugins/accesses.rs @@ -13,6 +13,7 @@ use super::accounts::{form, has}; use super::net::Got; use super::shared_accounts::{Mock, WITH, asking, mock_for, who}; use super::*; +use crate::mock_oauth::sent_at; /// The mock type's scopes (see `shared_accounts::account_plugin`). const SCOPES: [&str; 3] = ["read", "write", "peek"]; @@ -592,3 +593,144 @@ fn a_request_an_access_cant_serve_fails_as_the_host_passes_it_on(tc: TestCase) { assert_eq!(passed.detail().actions, actions, "{said}"); } } + +/// What the mock service answers a request echo makes through its access, in +/// [`a_refused_token_is_refreshed_once_then_given_up`]. +#[derive(Debug, Clone, Copy, PartialEq)] +enum Reply { + /// 401, saying nothing more. + Unauthorized, + /// 401, its challenge saying `error="invalid_token"`. + InvalidToken, + /// This status, its challenge saying `error="insufficient_scope"` for the service's `write`. + Insufficient(u16), + /// 403, Google's body saying `ACCESS_TOKEN_SCOPE_INSUFFICIENT`. + Google, + /// 403, its body saying only `insufficientPermissions`. + Forbidden, + /// 200. + Fine, +} + +impl Reply { + fn status(self) -> u16 { + match self { + Reply::Unauthorized | Reply::InvalidToken => 401, + Reply::Insufficient(status) => status, + Reply::Google | Reply::Forbidden => 403, + Reply::Fine => 200, + } + } + + fn reply(self) -> super::net::Reply { + let challenge = |said: &str| vec![("WWW-Authenticate", format!("Bearer realm=\"mock\", {said}"))]; + let headers = match self { + Reply::InvalidToken => challenge(r#"error="invalid_token", error_description="revoked""#), + Reply::Insufficient(_) => challenge(r#"error="insufficient_scope", scope="write""#), + _ => Vec::new(), + }; + let body = match self { + Reply::Google => json!({ "error": { "code": 403, "errors": [{ "reason": "insufficientPermissions" }], + "details": [{ "reason": "ACCESS_TOKEN_SCOPE_INSUFFICIENT" }] } }), + Reply::Forbidden => { + json!({ "error": { "code": 403, "errors": [{ "reason": "insufficientPermissions" }] } }) + } + _ => json!({ "said": self.status() }), + }; + super::net::Reply { status: self.status(), headers, body: body.to_string().into_bytes() } + } + + /// The scopes the service says the token lacks, by echo's names, when it says that's why. + fn needs(self) -> Option<&'static [&'static str]> { + match self { + Reply::Insufficient(_) => Some(&["write"]), + Reply::Google => Some(&["read"]), + _ => None, + } + } +} + +/// How the account's token refreshes in [`a_refused_token_is_refreshed_once_then_given_up`]. +#[derive(Debug, Clone, Copy, PartialEq)] +enum Refreshing { + Refreshes, + /// It has no refresh token. + Never, + /// The service refuses its refresh token (`invalid_grant`). + Refused, +} + +/// For a request echo makes through its access to an account (its token in the header or the +/// query, and redirected to where no scope covers or not), and anything the service answers it +/// with twice (refusing the token, saying it lacks a scope in a challenge or Google's body, +/// forbidding it otherwise, or answering): an answer to a request that still had the token +/// refusing it (a 401) gets the token refreshed once and the request sent again, its answer +/// passed on, the account signed out when it's refused again as `invalid_token`; an account +/// whose token doesn't refresh is signed out, failing `access` (`signed_out`); one the service +/// says lacks a scope fails `scope`, naming what signing in again needs to allow, and is no +/// longer granted what the service named (else what those ask); anything else, and any answer +/// once the token was dropped, is passed on. No request is sent more than twice, and no token +/// gets out. +#[hegel::test(test_cases = 24, suppress_health_check = [hegel::HealthCheck::TooSlow])] +fn a_refused_token_is_refreshed_once_then_given_up(tc: TestCase) { + let query = tc.draw(gs::sampled_from(vec![None, Some("access_token")])); + let place = query.map_or(String::new(), |name| format!("token = {{ query = \"{name}\" }}")); + let m = mock_for(3600, "/userinfo", &place, &asking(&["read"], &["write"])); + let refreshing = tc.draw(gs::sampled_from(vec![Refreshing::Refreshes, Refreshing::Never, Refreshing::Refused])); + m.state.lock().unwrap().refreshless = refreshing == Refreshing::Never; + m.sign_in(who(json!("a"), None), None, &[]).0.unwrap(); + let all = vec![ + Reply::Unauthorized, + Reply::InvalidToken, + Reply::Insufficient(401), + Reply::Insufficient(403), + Reply::Google, + Reply::Forbidden, + Reply::Fine, + ]; + let replies = tc.draw(gs::vecs(gs::sampled_from(all)).min_size(1).max_size(2)); + let redirected = tc.draw(gs::booleans()); + { + let mut s = m.state.lock().unwrap(); + s.script = replies.iter().map(|r| r.reply()).collect(); + if refreshing == Refreshing::Refused { + s.refuses = Some(("invalid_grant".into(), "spent".into())); + } + } + let path = if redirected { "/r/go/o/x" } else { "/r/x" }; + let answer = m.fetch_as("GET", path, "{account:mock}", query); + let said = format!("{replies:?} {refreshing:?} redirected {redirected} {query:?}: {answer}"); + + let at = if redirected { "/o/x" } else { "/r/x" }; + let sent: Vec = m.server.got().into_iter().filter(|g| g.target.starts_with(at)).collect(); + let refreshed = m.refreshes(); + let refused = !redirected && matches!(replies[0], Reply::Unauthorized | Reply::InvalidToken); + let again = refused && refreshing == Refreshing::Refreshes; + let last = if again { replies.get(1).copied().unwrap_or(Reply::Fine) } else { replies[0] }; + assert_eq!(sent.len(), if again { 2 } else { 1 }, "{said}"); + assert_eq!(refreshed, usize::from(refused && refreshing != Refreshing::Never), "{said}"); + if again { + assert_eq!(sent_at(&sent[1]), Some(2), "{said}: not sent again with the token refreshed"); + } + let signed_out = match () { + _ if refused && !again => true, + _ => again && last == Reply::InvalidToken, + }; + let kept = m.listed(); + assert_eq!(kept.is_empty(), signed_out, "{said}"); + let e = &answer["error"]["data"]; + match last.needs().filter(|_| !redirected) { + _ if refused && !again => { + assert_eq!((e["kind"].as_str(), e["reason"].as_str()), (Some("access"), Some("signed_out")), "{said}") + } + Some(needs) => { + assert_eq!((e["kind"].as_str(), &e["fields"]["needs"]), (Some("scope"), &json!(needs)), "{said}"); + assert!(answer["result"].is_null(), "{said}: its body passed on"); + let lost = if last == Reply::Google { "read" } else { "write" }; + assert!(!kept[0].scopes.iter().any(|s| s == lost), "{said}: still granted {lost}"); + } + None => assert_eq!(answer["result"]["status"], last.status(), "{said}"), + } + // What the service answered is the service's: it echoes the token it got. + assert!(!m.leaked(&[answer["error"].clone()]), "a token is out: {said}"); +} diff --git a/crates/locallibrary/tests/it/plugins/shared_accounts.rs b/crates/locallibrary/tests/it/plugins/shared_accounts.rs index bc5502174..f737da298 100644 --- a/crates/locallibrary/tests/it/plugins/shared_accounts.rs +++ b/crates/locallibrary/tests/it/plugins/shared_accounts.rs @@ -131,7 +131,7 @@ impl Mock { } /// How many times the service was asked to refresh a token. - fn refreshes(&self) -> usize { + pub(super) fn refreshes(&self) -> usize { let refreshing = |g: &Got| g.target == "/token" && has(&form(g), "grant_type", "refresh_token"); self.server.got().iter().filter(|g| refreshing(g)).count() } diff --git a/crates/locallibrary/tests/support/mock_oauth.rs b/crates/locallibrary/tests/support/mock_oauth.rs index c4f933809..be49b6526 100644 --- a/crates/locallibrary/tests/support/mock_oauth.rs +++ b/crates/locallibrary/tests/support/mock_oauth.rs @@ -5,6 +5,7 @@ //! browser isn't played: a test comes back to the sign-in itself. #![allow(dead_code)] +use std::collections::VecDeque; use std::io::{BufRead, BufReader, Read, Write}; use std::net::TcpListener; use std::sync::{Arc, Mutex}; @@ -113,6 +114,9 @@ pub struct Service { pub unscoped: bool, /// What the token endpoint says it granted, when it isn't `read write`. pub grants: Option>, + /// What it answers the requests it doesn't serve itself, in turn, before answering what + /// they were sent. + pub script: VecDeque, } /// The form a request sent, by name. @@ -150,7 +154,7 @@ pub fn sent_at(got: &Got) -> Option { /// token for tokens, and so does Local Network's token exchange for it, taking the same as JSON; /// `/revoke` takes one back; `/userinfo` says who an access token is for, and /// `/userinfo/moved` sends there; `/home` sends to `/here`, and `/r/go/` to `/`; and -/// anything else answers what it was sent. +/// anything else answers its [`Service::script`]'s next reply, else what it was sent. pub fn service(state: Arc>) -> Server { Server::new(move |base, got| { let mut s = state.lock().unwrap(); @@ -221,7 +225,9 @@ pub fn service(state: Arc>) -> Server { _ if path.starts_with("/r/go/") => { Reply { status: 307, headers: vec![("Location", format!("{base}/{}", &path[6..]))], body: Vec::new() } } - _ => ok(format!("{} {}", got.target, got.headers.get("authorization").map_or("-", String::as_str))), + _ => s.script.pop_front().unwrap_or_else(|| { + ok(format!("{} {}", got.target, got.headers.get("authorization").map_or("-", String::as_str))) + }), } }) } From 5df503a46df0a9749c0e8d274e9569e1901a39e4 Mon Sep 17 00:00:00 2001 From: botandbang <337765928+botandbang@users.noreply.github.com> Date: Fri, 30 Oct 2026 13:28:25 -0400 Subject: [PATCH 07/13] refactor(youtube): leave a refused token to locallibrary --- plugins/youtube/src/tests/publishing.rs | 12 +++++------- plugins/youtube/src/upload.rs | 3 +-- 2 files changed, 6 insertions(+), 9 deletions(-) diff --git a/plugins/youtube/src/tests/publishing.rs b/plugins/youtube/src/tests/publishing.rs index 1a6e12757..afbba304e 100644 --- a/plugins/youtube/src/tests/publishing.rs +++ b/plugins/youtube/src/tests/publishing.rs @@ -388,9 +388,10 @@ fn an_upload_failing_again_and_again_stops() { assert!(!mp4.exists()); } -/// YouTube refusing says why in its words, and what to do where there's something to do: a -/// sign-in it doesn't take, an account without a channel, a quota or limit used up, details it -/// won't take, an upload it dropped. None is asked again; a server error is, and then fails. +/// YouTube refusing says why in its words, and what to do where there's something to do: an +/// account without a channel, a quota or limit used up, details it won't take, an upload it +/// dropped. A token it doesn't take (which locallibrary refreshes or signs out of before it gets +/// here) is refused as it says. None is asked again; a server error is, and then fails. #[test] fn youtube_refusing_says_why() { use youtube::{Reply, refusal}; @@ -402,10 +403,7 @@ fn youtube_refusing_says_why() { "start", refusal(401, "authError", "Invalid Credentials"), "permission", - format!( - "{details} (401 authError: Invalid Credentials): sign in again: locallibrary account google sign-in \ - --for youtube" - ), + format!("{details} (401 authError: Invalid Credentials)"), ), ( "start", diff --git a/plugins/youtube/src/upload.rs b/plugins/youtube/src/upload.rs index 7a452d316..313e33542 100644 --- a/plugins/youtube/src/upload.rs +++ b/plugins/youtube/src/upload.rs @@ -226,7 +226,6 @@ fn refused(answer: &Fetched, what: &str) -> RpcError { (_, "youtubeSignupRequired") => { ("permission", "the Google account has no YouTube channel: make one and publish again") } - (401, _) => ("permission", "sign in again: locallibrary account google sign-in --for youtube"), (_, "uploadLimitExceeded") => ("external", "the channel has uploaded as many videos as it may today"), (_, "quotaExceeded" | "dailyLimitExceeded") => { ("external", "Local Network's share of the YouTube API is used up for today: publish again tomorrow") @@ -234,7 +233,7 @@ fn refused(answer: &Fetched, what: &str) -> RpcError { (429, _) | (_, "rateLimitExceeded" | "userRateLimitExceeded") => { ("external", "YouTube is taking too many requests: publish again in a while") } - (403, _) => ("permission", ""), + (401 | 403, _) => ("permission", ""), (404, _) => ("external", "the upload's session expired: publish again"), (400..=499, _) => ("invalid", ""), _ => ("external", ""), From 9870b89ef521a07d318411adc499762745ae7e1e Mon Sep 17 00:00:00 2001 From: botandbang <337765928+botandbang@users.noreply.github.com> Date: Fri, 30 Oct 2026 13:28:25 -0400 Subject: [PATCH 08/13] refactor(mixcloud): leave a refused token to locallibrary --- plugins/mixcloud/src/tests/properties.rs | 2 +- plugins/mixcloud/src/tests/publishing.rs | 6 +++--- plugins/mixcloud/src/upload.rs | 4 +--- 3 files changed, 5 insertions(+), 7 deletions(-) diff --git a/plugins/mixcloud/src/tests/properties.rs b/plugins/mixcloud/src/tests/properties.rs index 739955f80..77ae43735 100644 --- a/plugins/mixcloud/src/tests/properties.rs +++ b/plugins/mixcloud/src/tests/properties.rs @@ -186,7 +186,7 @@ fn a_refusal_is_said_in_mixclouds_words(tc: TestCase) { let limit = format!("{kind} {message}").to_lowercase().contains("limit"); let (expected, then) = match status { _ if rate_limited => ("external", "publish again in 16 minutes"), - 401 => ("permission", "sign in again: locallibrary account mixcloud sign-in --for mixcloud"), + 401 => ("permission", ")"), _ if limit => ("invalid", "move one to your drafts, or go Pro"), 403 => ("permission", ")"), _ => ("invalid", ")"), diff --git a/plugins/mixcloud/src/tests/publishing.rs b/plugins/mixcloud/src/tests/publishing.rs index 3126a7ca3..2804f89b9 100644 --- a/plugins/mixcloud/src/tests/publishing.rs +++ b/plugins/mixcloud/src/tests/publishing.rs @@ -310,9 +310,9 @@ fn refusals_say_why_and_go_once() { let said = |kind: &str, message: &str| (kind.to_string(), message.to_string()); let token = refused(error(401, "OAuthException", "An invalid access token was provided.")); - let sign_in = "Mixcloud refused the show (401 OAuthException: An invalid access token was provided.): sign in \ - again: locallibrary account mixcloud sign-in --for mixcloud"; - assert_eq!(token, said("permission", sign_in)); + // One locallibrary didn't refresh, or sign out of, before it got here. + let why = "Mixcloud refused the show (401 OAuthException: An invalid access token was provided.)"; + assert_eq!(token, said("permission", why)); let details = json!({ "mp3": ["Unsupported audio.", "Try another file."], "name": "Too long." }); let body = json!({ "error": { "type": "PostValidationError", "message": "Invalid fields." }, "details": details }); diff --git a/plugins/mixcloud/src/upload.rs b/plugins/mixcloud/src/upload.rs index 4f59af7ad..d8088b7cf 100644 --- a/plugins/mixcloud/src/upload.rs +++ b/plugins/mixcloud/src/upload.rs @@ -151,9 +151,7 @@ fn refused(answer: &Fetched) -> (Next, RpcError) { let limit = format!("{} {}", said.kind, said.message).to_lowercase().contains("limit"); match status { 408 | 500.. => (Next::Again(None), failed("external", "")), - 401 => { - (Next::Stop, failed("permission", "sign in again: locallibrary account mixcloud sign-in --for mixcloud")) - } + 401 => (Next::Stop, failed("permission", "")), 400..=499 if limit => { let why = "a free Mixcloud account keeps 10 shows published: move one to your drafts, or go Pro"; (Next::Stop, failed("invalid", why)) From 5f582b282c878f0c6b311555b1a10e8be461db19 Mon Sep 17 00:00:00 2001 From: botandbang <337765928+botandbang@users.noreply.github.com> Date: Fri, 30 Oct 2026 13:28:26 -0400 Subject: [PATCH 09/13] refactor(soundcloud): leave a refused token to locallibrary --- plugins/soundcloud/src/tests.rs | 12 ++++-------- plugins/soundcloud/src/upload.rs | 3 +-- 2 files changed, 5 insertions(+), 10 deletions(-) diff --git a/plugins/soundcloud/src/tests.rs b/plugins/soundcloud/src/tests.rs index 6b8785ae4..185bf2b36 100644 --- a/plugins/soundcloud/src/tests.rs +++ b/plugins/soundcloud/src/tests.rs @@ -370,7 +370,7 @@ fn a_track_made_anyway_is_the_answer() { (Reply::Unreachable, "connection reset"), ( Reply::Status(401, Vec::new(), refusal(401, "Unauthorized")), - "SoundCloud refused the account's tracks (401: Unauthorized): sign in again: locallibrary account soundcloud sign-in --for soundcloud", + "SoundCloud refused the account's tracks (401: Unauthorized)", ), ( Reply::Status(200, Vec::new(), "[]".into()), @@ -402,18 +402,14 @@ fn a_track_made_anyway_is_the_answer() { } /// SoundCloud refusing says why, in its words, and what to do where there's something to do: a -/// sign-in it doesn't take, a track it won't take. None is sent again. +/// track it won't take. A token it doesn't take (which locallibrary refreshes or signs out of +/// before it gets here) is refused as it says. None is sent again. #[test] fn soundcloud_refusing_says_why() { let dir = tempfile::tempdir().unwrap(); let path = file(dir.path(), "set.ogg", 10); let cases = [ - ( - 401, - refusal(401, "invalid_token"), - "permission", - "SoundCloud refused the set (401: invalid_token): sign in again: locallibrary account soundcloud sign-in --for soundcloud", - ), + (401, refusal(401, "invalid_token"), "permission", "SoundCloud refused the set (401: invalid_token)"), (403, refusal(403, "Forbidden"), "permission", "SoundCloud refused the set (403: Forbidden)"), ( 422, diff --git a/plugins/soundcloud/src/upload.rs b/plugins/soundcloud/src/upload.rs index 7c805ca79..28d73ee9b 100644 --- a/plugins/soundcloud/src/upload.rs +++ b/plugins/soundcloud/src/upload.rs @@ -197,8 +197,7 @@ fn refused(answer: &Fetched, what: &str) -> RpcError { let words = words.map(|w| w.trim().to_string()).find(|w| !w.is_empty()); let words = words.map_or(status.to_string(), |w| format!("{status}: {w}")); let (kind, why) = match status { - 401 => ("permission", "sign in again: locallibrary account soundcloud sign-in --for soundcloud"), - 403 => ("permission", ""), + 401 | 403 => ("permission", ""), 413 => ("invalid", "the recording is bigger than SoundCloud takes"), 429 => ("external", "SoundCloud is taking too many requests"), 400..=499 => ("invalid", ""), From 7b868285f286da6ae3a36fbed3cf05b5597d1897 Mon Sep 17 00:00:00 2001 From: botandbang <337765928+botandbang@users.noreply.github.com> Date: Sat, 31 Oct 2026 09:13:41 -0400 Subject: [PATCH 10/13] fix(locallibrary): link only public items from store_destination's docs rustdoc denies the private accounts::ready link under -D warnings; link Plugins::unready instead. --- crates/locallibrary/src/plugin/destinations.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/crates/locallibrary/src/plugin/destinations.rs b/crates/locallibrary/src/plugin/destinations.rs index 1a9a3ec31..0d20c5ad1 100644 --- a/crates/locallibrary/src/plugin/destinations.rs +++ b/crates/locallibrary/src/plugin/destinations.rs @@ -61,7 +61,7 @@ impl Plugins { /// The store destination `spec` names (`:`), for copying the store to, /// and restoring and repairing from: in `store`, once its plugin's accounts will do (see - /// [`super::accounts::ready`]). + /// [`Plugins::unready`]). pub fn store_destination(&self, spec: &str, store: Option<&Store>) -> Result { let (slot, name) = self.destination(spec, DestinationKind::Store)?; if let Some(store) = store { From 806715f90a19c1dcacb419b8e94858c61bbfd29d Mon Sep 17 00:00:00 2001 From: botandbang <337765928+botandbang@users.noreply.github.com> Date: Sat, 31 Oct 2026 12:56:27 -0400 Subject: [PATCH 11/13] docs(locallibrary): say what account escalation changes, in changes/account-escalation.md --- apps/locallibrary/changes/account-escalation.md | 12 ++++++++++++ 1 file changed, 12 insertions(+) create mode 100644 apps/locallibrary/changes/account-escalation.md diff --git a/apps/locallibrary/changes/account-escalation.md b/apps/locallibrary/changes/account-escalation.md new file mode 100644 index 000000000..2c135525d --- /dev/null +++ b/apps/locallibrary/changes/account-escalation.md @@ -0,0 +1,12 @@ +### Added + +- **More of an account when a plugin needs it.** Signing in again asks the service for every + scope the account's plugins use plus what's added, as the same account (`account TYPE:NAME + sign-in` works for a signed-out account plugins still have access to), and keeps the refresh + token when the service sends no new one. A plugin gets only the scopes the service granted, and + is told which it declined; `plugin access PLUGIN --give ACCOUNT --scope S` gives one later. + Before a plugin destination starts, and again before a publish goes out, its accounts are + checked: signed in, with the scopes it needs, and a token that lasts the upload. A token a + service refuses is refreshed once and the request tried again, and a service saying the token + lacks a scope fails asking you to sign in again for it. google-account asks Google for offline + access, so its accounts stay signed in. From 188245e90d974e7c017ac4b94f00412dfc6916aa Mon Sep 17 00:00:00 2001 From: botandbang <337765928+botandbang@users.noreply.github.com> Date: Sun, 1 Nov 2026 15:41:21 -0500 Subject: [PATCH 12/13] fix(locallibrary): sign an account out on a 401 only when the service says its token is invalid --- .../changes/account-escalation.md | 4 +-- apps/locallibrary/docs/plugins.md | 6 ++-- .../src/plugin/accounts/shared.rs | 11 +++---- .../locallibrary/src/plugin/accounts/token.rs | 15 ++++++--- crates/locallibrary/src/plugin/net.rs | 18 ++++++----- .../locallibrary/tests/it/plugins/accesses.rs | 32 ++++++++++++------- 6 files changed, 51 insertions(+), 35 deletions(-) diff --git a/apps/locallibrary/changes/account-escalation.md b/apps/locallibrary/changes/account-escalation.md index 2c135525d..a5fbe47c0 100644 --- a/apps/locallibrary/changes/account-escalation.md +++ b/apps/locallibrary/changes/account-escalation.md @@ -7,6 +7,6 @@ is told which it declined; `plugin access PLUGIN --give ACCOUNT --scope S` gives one later. Before a plugin destination starts, and again before a publish goes out, its accounts are checked: signed in, with the scopes it needs, and a token that lasts the upload. A token a - service refuses is refreshed once and the request tried again, and a service saying the token - lacks a scope fails asking you to sign in again for it. google-account asks Google for offline + service refuses is refreshed once and the request tried again, the account signed out only + when the service says the token is no good, and a service saying the token lacks a scope fails asking you to sign in again for it. google-account asks Google for offline access, so its accounts stay signed in. diff --git a/apps/locallibrary/docs/plugins.md b/apps/locallibrary/docs/plugins.md index d33aed8d2..cdc12beeb 100644 --- a/apps/locallibrary/docs/plugins.md +++ b/apps/locallibrary/docs/plugins.md @@ -245,8 +245,10 @@ account by its type (`{account:TYPE}`) where the type says its token goes, the ` header unless the service only takes it in the address (Mixcloud), and locallibrary fills in the token once it has checked the request against the scopes of the plugin's access, never sending it on to another server a redirect leads to. When the service refuses the token (a 401), locallibrary -refreshes it and sends the request once more; one it can't refresh is signed out, and a second -refusal saying the token is no good signs it out too. When the service says the token lacks a +refreshes it and sends the request once more. A refusal that isn't sent again (the second, or +one for an account with no refresh token) is passed on to the plugin, and signs the account out +only when the service says the token is no good (`error="invalid_token"`); one the service won't +refresh signs it out too. When the service says the token lacks a scope, the request fails saying which, and signing in again is offered. Some services (SoundCloud) only take the browser back at the exact address their app was diff --git a/crates/locallibrary/src/plugin/accounts/shared.rs b/crates/locallibrary/src/plugin/accounts/shared.rs index 3173fca7f..a63166111 100644 --- a/crates/locallibrary/src/plugin/accounts/shared.rs +++ b/crates/locallibrary/src/plugin/accounts/shared.rs @@ -168,8 +168,8 @@ impl Plugin { /// The access token of the account `subject` it signed in to in `store` to use instead of /// `refused`, which its service refused: the one another host refreshed it to since, else - /// `refused` refreshed (as [`Plugin::account_token`] refreshes). Without a refresh token, or - /// one the service won't refresh any more, it's signed out. + /// `refused` refreshed (as [`Plugin::account_token`] refreshes), else, without a refresh + /// token, `refused` itself. One the service won't refresh any more is signed out. pub(crate) fn account_token_refused(&self, store: &Store, subject: &str, refused: &str) -> Result { self.token(store, subject, Some(refused)) } @@ -194,10 +194,9 @@ impl Plugin { } let storage = self.host.options.secrets; let Some(refresh) = tokens.refresh_token.clone() else { - if refused.is_some() { - forget(store, storage, &k)?; - } - return Err(signed_out()); + // Nothing to refresh it with: the refusal is passed on, signing it out only when the + // service says the token is no good (see `Plugin::invalid_token`). + return refused.map(|_| tokens.access_token).ok_or_else(signed_out); }; let Some(fresh) = self.refreshed(t, refresh, tokens)? else { forget(store, storage, &k)?; diff --git a/crates/locallibrary/src/plugin/accounts/token.rs b/crates/locallibrary/src/plugin/accounts/token.rs index 642abf41e..476cc0d8d 100644 --- a/crates/locallibrary/src/plugin/accounts/token.rs +++ b/crates/locallibrary/src/plugin/accounts/token.rs @@ -142,14 +142,19 @@ impl Plugin { /// Its service refused `token`, `usable`'s account's: the account's token is refreshed, unless /// another host already has (see [`Plugin::account_token_refused`]), for the request to be - /// sent again with it. `access` (`signed_out`) when the service won't refresh it. - pub(crate) fn refused_token(&self, usable: &Usable, token: &str, store: &Store) -> Result<(), RpcError> { + /// sent again with it. Whether there's another token to send, which there isn't without a + /// refresh token; `access` (`signed_out`) when the service won't refresh it. + pub(crate) fn refused_token(&self, usable: &Usable, token: &str, store: &Store) -> Result { let subject = &usable.access.account.subject; - usable.with.account_token_refused(store, subject, token).map(drop).map_err(|e| self.token_failed(usable, e)) + let instead = + usable.with.account_token_refused(store, subject, token).map_err(|e| self.token_failed(usable, e))?; + Ok(instead != token) } - /// Its service refused `token` again, saying it's no good: `usable`'s account is signed out, - /// unless another host signed it in again since. + /// Its service refused `token` with a 401 it won't be sent again after, saying + /// `error="invalid_token"`: `usable`'s account is signed out, unless another host signed it in + /// again since. Only the service saying so signs it out: a plugin can draw a 401 from any URL + /// its scope covers, for reasons of the service's own. pub(crate) fn invalid_token(&self, usable: &Usable, token: &str, store: &Store) -> Result<(), RpcError> { let subject = &usable.access.account.subject; usable.with.forget_refused(store, subject, token).map_err(|e| RpcError::failed("external", e.to_string())) diff --git a/crates/locallibrary/src/plugin/net.rs b/crates/locallibrary/src/plugin/net.rs index 5556ba728..161a876d9 100644 --- a/crates/locallibrary/src/plugin/net.rs +++ b/crates/locallibrary/src/plugin/net.rs @@ -307,9 +307,10 @@ impl Plugin { /// one (a 401 or 403; not after a hop that dropped them, which isn't about them): saying the /// token lacks scopes (see [`insufficient_scope`]), `scope`, its body not passed on (see /// [`Plugin::scope_refused`]); a 401 otherwise, the request sent again once with the account's - /// token refreshed (see [`Plugin::refused_token`]), and its answer passed on, the account - /// signed out when it's a 401 again saying `error="invalid_token"`. With the placeholders of - /// the tokens the request that was answered had in its query. + /// token refreshed (see [`Plugin::refused_token`]), and its answer passed on. A 401 that isn't + /// sent again, the second or one with no refresh token to refresh, signs the account out only + /// when it says `error="invalid_token"` (see [`Plugin::invalid_token`]). With the placeholders + /// of the tokens the request that was answered had in its query. fn answered( &self, live: &Live, @@ -344,8 +345,12 @@ impl Plugin { let written = Url::parse(&req.url).map_err(|_| invalid("it isn't a web address"))?; return Err(self.scope_refused(&attached[0].usable, &scopes, &req.method, &written, store)); } - if status == 403 || again { - let invalid = again && status == 401 && said.iter().any(|c| c.param("error") == Some("invalid_token")); + let resend = status == 401 && !again && { + let refreshed = attached.iter().map(|f| self.refused_token(&f.usable, &f.token, store)); + refreshed.collect::, _>>()?.contains(&true) + }; + if !resend { + let invalid = status == 401 && said.iter().any(|c| c.param("error") == Some("invalid_token")); if invalid { for f in &attached { self.invalid_token(&f.usable, &f.token, store)?; @@ -353,9 +358,6 @@ impl Plugin { } return Ok((Answer { status, headers, read: body, response }, in_query)); } - for f in &attached { - self.refused_token(&f.usable, &f.token, store)?; - } again = true; let (url, sent, query) = self.prepare(live, req)?; in_query = query; diff --git a/crates/locallibrary/tests/it/plugins/accesses.rs b/crates/locallibrary/tests/it/plugins/accesses.rs index 07eb1f3c2..32d76e81b 100644 --- a/crates/locallibrary/tests/it/plugins/accesses.rs +++ b/crates/locallibrary/tests/it/plugins/accesses.rs @@ -665,12 +665,13 @@ enum Refreshing { /// with twice (refusing the token, saying it lacks a scope in a challenge or Google's body, /// forbidding it otherwise, or answering): an answer to a request that still had the token /// refusing it (a 401) gets the token refreshed once and the request sent again, its answer -/// passed on, the account signed out when it's refused again as `invalid_token`; an account -/// whose token doesn't refresh is signed out, failing `access` (`signed_out`); one the service -/// says lacks a scope fails `scope`, naming what signing in again needs to allow, and is no -/// longer granted what the service named (else what those ask); anything else, and any answer -/// once the token was dropped, is passed on. No request is sent more than twice, and no token -/// gets out. +/// passed on; a 401 that isn't sent again (the second, or the first without a refresh token) is +/// passed on too, signing the account out only when it says `invalid_token`, and otherwise the +/// next request still gets the token. An account whose refresh token the service refuses is +/// signed out, failing `access` (`signed_out`); one the service says lacks a scope fails +/// `scope`, naming what signing in again needs to allow, and is no longer granted what the +/// service named (else what those ask); anything else, and any answer once the token was +/// dropped, is passed on. No request is sent more than twice, and no token gets out. #[hegel::test(test_cases = 24, suppress_health_check = [hegel::HealthCheck::TooSlow])] fn a_refused_token_is_refreshed_once_then_given_up(tc: TestCase) { let query = tc.draw(gs::sampled_from(vec![None, Some("access_token")])); @@ -712,15 +713,13 @@ fn a_refused_token_is_refreshed_once_then_given_up(tc: TestCase) { if again { assert_eq!(sent_at(&sent[1]), Some(2), "{said}: not sent again with the token refreshed"); } - let signed_out = match () { - _ if refused && !again => true, - _ => again && last == Reply::InvalidToken, - }; + let refresh_refused = refused && refreshing == Refreshing::Refused; + let signed_out = refresh_refused || (refused && last == Reply::InvalidToken); let kept = m.listed(); assert_eq!(kept.is_empty(), signed_out, "{said}"); let e = &answer["error"]["data"]; match last.needs().filter(|_| !redirected) { - _ if refused && !again => { + _ if refresh_refused => { assert_eq!((e["kind"].as_str(), e["reason"].as_str()), (Some("access"), Some("signed_out")), "{said}") } Some(needs) => { @@ -731,6 +730,15 @@ fn a_refused_token_is_refreshed_once_then_given_up(tc: TestCase) { } None => assert_eq!(answer["result"]["status"], last.status(), "{said}"), } + let mut answers = vec![answer["error"].clone()]; + if refused && !signed_out && last.needs().is_none() { + m.state.lock().unwrap().script.clear(); + let next = m.fetch_as("GET", "/r/x", "{account:mock}", query); + assert_eq!(next["result"]["status"], 200, "{said}: then {next}"); + let got = m.server.got().into_iter().rfind(|g| g.target.starts_with("/r/x")); + assert_eq!(got.as_ref().and_then(sent_at), Some(1 + usize::from(again)), "{said}: then not sent its token"); + answers.push(next["error"].clone()); + } // What the service answered is the service's: it echoes the token it got. - assert!(!m.leaked(&[answer["error"].clone()]), "a token is out: {said}"); + assert!(!m.leaked(&answers), "a token is out: {said}"); } From 9699d4ec22444fa9936a2719f7af9d347eb0e16a Mon Sep 17 00:00:00 2001 From: botandbang <337765928+botandbang@users.noreply.github.com> Date: Sun, 1 Nov 2026 20:20:47 -0500 Subject: [PATCH 13/13] refactor(locallibrary): keep a plugin request's address free of secrets prepare used to check where a plugin's request goes, fill in its secrets and accounts' tokens, and return the address with the query's filled in. Now address() parses the address and checks the plugin may go there, with no secret in it, and prepare returns what's sent: its headers, body and accounts, and its query with everything filled in, kept apart. follow sends the first hop to the address without its query and adds the filled pairs with RequestBuilder::query, which encodes them the same way, so the first request on the wire doesn't change. The address follow keeps and resolves each redirect against never holds a token, so a redirect back to the same address (an empty or fragment-only Location) no longer sends one again. Mixcloud still takes its token in the query, since its API reads it nowhere else (https://www.mixcloud.com/developers/). --- crates/locallibrary/src/plugin/net.rs | 39 +++++++++++++++++++-------- 1 file changed, 28 insertions(+), 11 deletions(-) diff --git a/crates/locallibrary/src/plugin/net.rs b/crates/locallibrary/src/plugin/net.rs index 161a876d9..50b38a760 100644 --- a/crates/locallibrary/src/plugin/net.rs +++ b/crates/locallibrary/src/plugin/net.rs @@ -115,6 +115,9 @@ struct Sent { secret: bool, /// The accounts whose tokens are filled in, which go only where their scopes cover. accounts: Vec, + /// Its query, with its secrets and tokens filled in, when it has one to fill: the first hop + /// is sent with it in place of the address's own, which a secret is never put in. + query: Option>, } /// An account's token filled into a request, and the access it's through. @@ -275,7 +278,8 @@ impl Plugin { /// token, on the last hop it went to, is answered for the plugin (see [`Plugin::answered`]). pub(crate) fn fetch(&self, live: &Live, params: Value, cancel: &CancelToken) -> Result { let req: Fetch = loclib_plugin::params(params)?; - let (url, sent, in_query) = self.prepare(live, &req)?; + let url = self.address(&req)?; + let (sent, in_query) = self.prepare(live, &req, &url)?; // Before it's sent: an answer with nowhere to go is never asked for. let into = req.into_blob.as_deref().map(|blob| self.new_blob(live, blob)).transpose()?; let requests = Requests::new().map_err(|e| RpcError::internal(format!("the network: {e}")))?; @@ -359,20 +363,26 @@ impl Plugin { return Ok((Answer { status, headers, read: body, response }, in_query)); } again = true; - let (url, sent, query) = self.prepare(live, req)?; + let url = self.address(req)?; + let (sent, query) = self.prepare(live, req, &url)?; in_query = query; (response, attached) = self.follow(live, url, sent, requests, cancel)?; } } - /// The request `req` asks for, as it's sent to its URL, its secrets and accounts' tokens - /// filled in: with the tokens filled into its query, and the placeholders that named them. - fn prepare(&self, live: &Live, req: &Fetch) -> Result<(Url, Sent, InQuery), RpcError> { - let mut url = - Url::parse(&req.url).map_err(|_| invalid(format!("{} isn't a web address", one_line(&req.url))))?; + /// The address `req` asks for, as the plugin wrote it, when the plugin may go there. + fn address(&self, req: &Fetch) -> Result { + let url = Url::parse(&req.url).map_err(|_| invalid(format!("{} isn't a web address", one_line(&req.url))))?; if let Some(why) = refused(&self.manifest.capabilities, &url, self.loopback()).or_else(|| self.offline(&url)) { return Err(denied(why)); } + Ok(url) + } + + /// The request `req` asks for at `url` (its [`Plugin::address`]), as it's sent, its secrets + /// and accounts' tokens filled in, the query's kept apart from the address: with the tokens + /// filled into its query, and the placeholders that named them. + fn prepare(&self, live: &Live, req: &Fetch, url: &Url) -> Result<(Sent, InQuery), RpcError> { let method = Method::from_bytes(req.method.as_bytes()) .map_err(|_| invalid(format!("{} isn't an HTTP method", one_line(&req.method))))?; if req.body.is_some() && req.body_blob.is_some() { @@ -391,7 +401,7 @@ impl Plugin { // What a scope covers is the request as the plugin wrote it. let asked = (method.clone(), url.clone()); let (plain, filled) = (HeaderMap::new(), HeaderMap::new()); - let mut sent = Sent { method, plain, filled, body: None, secret: false, accounts: Vec::new() }; + let mut sent = Sent { method, plain, filled, body: None, secret: false, accounts: Vec::new(), query: None }; // The accounts' tokens filled into the query, and the placeholders that named them. let mut in_query = Vec::new(); if url.query_pairs().any(|(_, v)| v.contains(SECRET) || v.contains(ACCOUNT_PLACEHOLDER)) { @@ -407,7 +417,7 @@ impl Plugin { filled.push((k, token.clone())); in_query.push((token, v)); } - url.query_pairs_mut().clear().extend_pairs(filled); + sent.query = Some(filled); } for (name, value) in &req.headers { let name = HeaderName::from_bytes(name.as_bytes()) @@ -447,7 +457,7 @@ impl Plugin { sent.plain.remove(CONTENT_TYPE); sent.body = Some(Body::Form(self.form(live, &req.multipart)?)); } - Ok((url, sent, in_query)) + Ok((sent, in_query)) } /// What goes where a request (`asked`, its method and URL as the plugin wrote it) names an @@ -535,7 +545,14 @@ impl Plugin { let mut hops = 0; loop { self.wait_turn(&url, cancel)?; - let mut request = self.client()?.request(sent.method.clone(), url.clone()); + let mut request = match sent.query.take() { + Some(query) => { + let mut bare = url.clone(); + bare.set_query(None); + self.client()?.request(sent.method.clone(), bare).query(&query) + } + None => self.client()?.request(sent.method.clone(), url.clone()), + }; request = request.headers(sent.plain.clone()).headers(sent.filled.clone()); if let Some(body) = &sent.body { let files = body.file_bytes();