From 1f18ce4dbbdbec83a8fa8ef70655595ae4571f66 Mon Sep 17 00:00:00 2001 From: botandbang <337765928+botandbang@users.noreply.github.com> Date: Fri, 30 Oct 2026 18:00:08 -0400 Subject: [PATCH 1/6] feat(loclib-plugin): accounts/ensure, for a plugin's optional scopes --- apps/locallibrary/CHANGELOG.md | 4 ++- apps/locallibrary/tests/it/plugins.rs | 3 +- crates/loclib-plugin/README.md | 11 +++++- .../examples/echo/locallibrary-plugin.toml | 6 ++++ crates/loclib-plugin/examples/echo/plugin.rs | 2 ++ crates/loclib-plugin/src/hooks/accounts.rs | 35 +++++++++++++++++-- crates/loclib-plugin/src/lib.rs | 4 ++- crates/loclib-plugin/src/protocol.rs | 1 + crates/loclib-plugin/src/sdk.rs | 11 ++++-- crates/loclib-plugin/src/sdk/tests.rs | 28 +++++++++++++++ 10 files changed, 96 insertions(+), 9 deletions(-) diff --git a/apps/locallibrary/CHANGELOG.md b/apps/locallibrary/CHANGELOG.md index 6edcfaab0..b654cdcce 100644 --- a/apps/locallibrary/CHANGELOG.md +++ b/apps/locallibrary/CHANGELOG.md @@ -438,7 +438,9 @@ Windows builds are experimental and untested on hardware (see Platforms in the R one. The player's palette signs in and out, and has a form for a plugin's access. The sidecar has them as `accounts.*`. Plugins no longer have accounts of their own: `plugin account` and the sidecar's `plugins.account*` are gone, and `plugin info` and `plugins.describe` show the - access a plugin has instead. + access a plugin has instead. A plugin asks for the scopes it can do without as it needs them + (`accounts/ensure`, `Host::ensure_account` in the SDK), and learns which it may use and which + you declined, never a token. - **The library is the unit.** Each drive holding a library is a clone of it, and its name doesn't matter much: a library's name works where a drive does (but in commands about one particular drive) and means its lead clone, the newest of its drives plugged in (the primary when they're diff --git a/apps/locallibrary/tests/it/plugins.rs b/apps/locallibrary/tests/it/plugins.rs index 7175fe074..cab09f3ce 100644 --- a/apps/locallibrary/tests/it/plugins.rs +++ b/apps/locallibrary/tests/it/plugins.rs @@ -222,8 +222,7 @@ fn plugins_are_listed_and_their_settings_kept() { let list = env.ok(&["plugin"]); assert!(list.contains("echo Echo 0.1.0"), "{list}"); assert!(list.contains("may read your libraries' tracks and playlists"), "{list}"); - let adds = - "adds commands echo, titles, steps, wait; destinations folder, nowhere; operation playlist; analyzer size"; + let adds = "adds commands echo, titles, steps, ensure, wait; destinations folder, nowhere; operation playlist; analyzer size"; assert!(list.contains(adds), "{list}"); let listed = env.json(&["plugin", "list"]); assert_eq!(ids(&listed), [&SHIPPED[..], &["echo"]].concat()); diff --git a/crates/loclib-plugin/README.md b/crates/loclib-plugin/README.md index d68c49426..091fa5fc6 100644 --- a/crates/loclib-plugin/README.md +++ b/crates/loclib-plugin/README.md @@ -262,7 +262,7 @@ The host signs the user in through an account plugin that provides the type (OAu authorization code flow with PKCE, the browser coming back to `127.0.0.1`), keeps the tokens in the system's keychain, refreshes them a minute before they expire, and gives the plugin access to the account (the scopes it needs, and the optional ones the user allows). The feature has no -methods: it's the SDK's own. +methods of the host's: it's the SDK's own. The plugin never sees a token: a `net/fetch` request names the account as `{account:}` (the plugin's default access for the type) or `{account:/}` (the access's 8 hex @@ -273,6 +273,15 @@ fails before it's sent, as `access` (no such access, or `reason` `signed_out`) o `fields` naming the `type`, the `scopes` and the `account_plugins`. Passed on, either is an `account` error saying how to sign in. +Before it needs one of its optional scopes, a hook can ask for it: `accounts/ensure` (`{type, +scopes, access?}`, `Host::ensure_account`) answers which of `scopes` it may use (`granted`) and +which the user declined (`declined`), never a token. Where the access hasn't a scope, or the +service didn't grant what it asks, the host asks the user, signing in again when it has to, and +waits for them; a scope they decline isn't asked about again until they give it. Where it can't +ask (`--json`, the sidecar, `watch`), it fails as a request would (`access` or `scope`, `needs` +naming the scopes), which passed on says how to give them. A scope the plugin's `[[accounts]]` +doesn't ask for is refused. + The type's `token` says where the token goes: `Authorization: Bearer ` unless it says otherwise. `token = { scheme = "OAuth" }` sends `Authorization: OAuth ` (SoundCloud), and `token = { query = "access_token" }` sends it as that query parameter instead (Mixcloud). The diff --git a/crates/loclib-plugin/examples/echo/locallibrary-plugin.toml b/crates/loclib-plugin/examples/echo/locallibrary-plugin.toml index 0f6488583..4b4c90a4a 100644 --- a/crates/loclib-plugin/examples/echo/locallibrary-plugin.toml +++ b/crates/loclib-plugin/examples/echo/locallibrary-plugin.toml @@ -20,6 +20,12 @@ name = "steps" title = "Report progress in steps" mode = "headless" +[[contributes.commands]] +name = "ensure" +title = "Ask for more of an account" +mode = "headless" +params = { type = "object", properties = { type = { type = "string" }, scopes = { type = "array", items = { type = "string" } }, access = { type = "string" } }, required = ["type", "scopes"] } + [[contributes.commands]] name = "wait" title = "Wait until cancelled" diff --git a/crates/loclib-plugin/examples/echo/plugin.rs b/crates/loclib-plugin/examples/echo/plugin.rs index 488337bdd..73fd33f9b 100644 --- a/crates/loclib-plugin/examples/echo/plugin.rs +++ b/crates/loclib-plugin/examples/echo/plugin.rs @@ -62,6 +62,8 @@ pub fn run_command(req: RunCommand, host: &Host) -> Result { } Ok(Value::Null) } + // Its params are an `accounts/ensure` request, its answer the host's. + "ensure" => Ok(serde_json::to_value(host.ensure_account(¶ms(req.params)?)?).expect("an answer is JSON")), _ => { let ctx = host.ctx(); ctx.phase_start(Phase::Scan, 0, 0); diff --git a/crates/loclib-plugin/src/hooks/accounts.rs b/crates/loclib-plugin/src/hooks/accounts.rs index 6c576e177..9ae25b4c7 100644 --- a/crates/loclib-plugin/src/hooks/accounts.rs +++ b/crates/loclib-plugin/src/hooks/accounts.rs @@ -42,9 +42,40 @@ pub fn account_placeholder(value: &str) -> Option<(&str, Option<&str>)> { const EXCHANGE_ORIGIN: &str = "https://localnetwork.computer"; const EXCHANGE_PATH: &str = "/api/token-exchange/"; -// Every plugin written with the SDK has it: what's an account in a request is the host's to fill. +/// Plugin to host (feature `accounts`): [`EnsureAccount`] -> [`Ensured`]. +pub const ACCOUNTS_ENSURE: &str = "accounts/ensure"; + +// Every plugin written with the SDK has it: what's an account in a request is the host's to fill, +// and what more of one it may use the host's to ask. pub(crate) const FEATURE: Feature = - Feature { name: "accounts", methods: &[], sends: &[], contributed: |m| !m.accounts.is_empty() }; + Feature { name: "accounts", methods: &[], sends: &[ACCOUNTS_ENSURE], contributed: |m| !m.accounts.is_empty() }; + +/// `accounts/ensure`: that the plugin may use `scopes` (the type's, by name, of those its +/// `[[accounts]]` asks for) of the account its access of the type names, its default for the type +/// unless it names another. The host answers which it may use and which the user declined, asking +/// the user about the rest first where it can (which takes as long as they do), and otherwise +/// fails as a `net/fetch` request naming the account would (`access` or `scope`). +#[derive(Debug, Clone, PartialEq, Serialize, Deserialize, JsonSchema)] +pub struct EnsureAccount { + /// The account type (`google`). + #[serde(rename = "type")] + pub kind: String, + pub scopes: Vec, + /// The access, by its id (8 lowercase hex digits), when it isn't the default. + #[serde(default, skip_serializing_if = "Option::is_none")] + pub access: Option, +} + +/// What `accounts/ensure` answers: each scope it asked about, by name, in one list or the other. +/// Never a token. +#[derive(Debug, Clone, Default, PartialEq, Serialize, Deserialize, JsonSchema)] +pub struct Ensured { + /// It may use these. + pub granted: Vec, + /// The user (or the service) declined these: they're not asked about again until the user + /// gives them. + pub declined: Vec, +} /// `[[accounts]]`: an account type a plugin asks for, the scopes of it it needs and why, and those /// it can do without. diff --git a/crates/loclib-plugin/src/lib.rs b/crates/loclib-plugin/src/lib.rs index b417e8e4c..e6fbaeeac 100644 --- a/crates/loclib-plugin/src/lib.rs +++ b/crates/loclib-plugin/src/lib.rs @@ -20,7 +20,9 @@ pub use choice::{Choice, ChoiceItem}; pub use error::{ErrorData, RpcError, params}; pub use features::{FEATURES, Feature}; pub use hooks::account_types::{AccountType, SUBJECT_PLACEHOLDER, Scope, ScopeRequest, Whoami}; -pub use hooks::accounts::{ACCOUNT_PLACEHOLDER, AccountNeed, OptionalScopes, TokenPlace, account_placeholder}; +pub use hooks::accounts::{ + ACCOUNT_PLACEHOLDER, AccountNeed, EnsureAccount, Ensured, OptionalScopes, TokenPlace, account_placeholder, +}; pub use hooks::analysis::{Output, WaveformBlob}; pub use hooks::analyzers::{ Analysis, AnalyzeTrack, Analyzer, FieldDisplay, FieldFormat, FieldType, FieldValue, Role, Track, diff --git a/crates/loclib-plugin/src/protocol.rs b/crates/loclib-plugin/src/protocol.rs index 3377fe997..283604035 100644 --- a/crates/loclib-plugin/src/protocol.rs +++ b/crates/loclib-plugin/src/protocol.rs @@ -18,6 +18,7 @@ pub const PROTOCOL_VERSION: &str = "0.2.0"; /// The methods' names. pub mod method { + pub use crate::hooks::accounts::ACCOUNTS_ENSURE; pub use crate::hooks::analyzers::ANALYZE_TRACK; pub use crate::hooks::checks::{PLAN_CHECK, RUN_CHECK}; pub use crate::hooks::commands::RUN_COMMAND; diff --git a/crates/loclib-plugin/src/sdk.rs b/crates/loclib-plugin/src/sdk.rs index 9c6e33ca0..d62fa36cd 100644 --- a/crates/loclib-plugin/src/sdk.rs +++ b/crates/loclib-plugin/src/sdk.rs @@ -17,8 +17,8 @@ use crate::protocol::{ StepInfo, StepStarted, check_protocol, method, }; use crate::{ - Contributions, EventNotify, FEATURES, HostNotify, Incoming, ListItem, Manifest, PROTOCOL_VERSION, Peer, Request, - RpcError, + Contributions, EnsureAccount, Ensured, EventNotify, FEATURES, HostNotify, Incoming, ListItem, Manifest, + PROTOCOL_VERSION, Peer, Request, RpcError, }; /// A request the host sends a hook: its method, what it answers, and the contribution it's for, @@ -163,6 +163,13 @@ impl Host { self.call(method::NET_FETCH, req) } + /// `accounts/ensure`: which of the scopes `req` names of an account the hook may use, and which + /// the user declined (see [`EnsureAccount`]). The host may ask the user first: this waits for + /// them, until the host cancels the hook. + pub fn ensure_account(&self, req: &EnsureAccount) -> Result { + self.call(method::ACCOUNTS_ENSURE, req) + } + /// Sends the host the next rows of the table a view command will answer (`partial`), to /// show while it works out the rest. The view it answers still has them, first. pub fn partial_rows(&self, rows: Vec>) { diff --git a/crates/loclib-plugin/src/sdk/tests.rs b/crates/loclib-plugin/src/sdk/tests.rs index bd00889ff..9bf3fb8bf 100644 --- a/crates/loclib-plugin/src/sdk/tests.rs +++ b/crates/loclib-plugin/src/sdk/tests.rs @@ -1100,6 +1100,34 @@ fn cancelling_a_hook_reaches_it_and_its_callback(tc: TestCase) { s.served.join().unwrap(); } +/// For any of an account type's scopes, naming an access or not: echo's `ensure` asks the host +/// which it may use (`accounts/ensure`, its request as the host gets it), and answers what the host +/// does, which scopes are granted and which declined, or its error, as it is. +#[hegel::test(test_cases = 25)] +fn a_hook_asks_the_host_which_scopes_of_an_account_it_may_use(tc: TestCase) { + let scopes: Vec = + ["read", "write", "upload"].into_iter().filter(|_| tc.draw(gs::booleans())).map(String::from).collect(); + let access = tc.draw(gs::optional(gs::sampled_from(vec!["0a1b2c3d".to_string()]))); + let ensure = EnsureAccount { kind: "mock".into(), scopes: scopes.clone(), access: access.clone() }; + let s = greeted(echo::hooks()); + let asked = json!({ "type": "mock", "scopes": scopes, "access": access }); + let pending = s.host.send_request(method::RUN_COMMAND, &run("ensure", asked, None)).unwrap(); + let Incoming::Request(req) = s.next() else { panic!("not a callback") }; + assert_eq!(req.method, method::ACCOUNTS_ENSURE); + assert_eq!(req.params, serde_json::to_value(&ensure).unwrap()); + assert_eq!((&req.params["type"], req.params.get("access").is_some()), (&json!("mock"), access.is_some())); + let answer = match tc.draw(gs::booleans()) { + true => Err(RpcError::failed("scope", "it may not")), + false => { + let (granted, declined) = scopes.into_iter().partition(|_| tc.draw(gs::booleans())); + Ok(Ensured { granted, declined }) + } + }; + s.host.respond(&req.id, answer.clone().map(|a| serde_json::to_value(a).unwrap())); + assert_eq!(pending.wait_timeout::(Duration::from_secs(10)).unwrap(), answer); + s.stop(); +} + /// A hook fetches through the host (`net/fetch`, its request as the host gets it), and sends a /// view's rows and items in parts, which reach the host's request before its answer, in order. #[test] From 85cf02a94495b5e275bf1f8ddb29c1a74e140af4 Mon Sep 17 00:00:00 2001 From: botandbang <337765928+botandbang@users.noreply.github.com> Date: Fri, 30 Oct 2026 18:00:12 -0400 Subject: [PATCH 2/6] feat(locallibrary): answer accounts/ensure --- apps/locallibrary/docs/rpc/locallibrary.d.ts | 7 +- apps/locallibrary/docs/rpc/schema.json | 2 +- apps/locallibrary/src/hints.rs | 6 + apps/locallibrary/src/hints/tests.rs | 7 +- crates/locallibrary/src/plugin.rs | 14 +- crates/locallibrary/src/plugin/accounts.rs | 29 ++- .../src/plugin/accounts/access.rs | 100 ++++---- .../src/plugin/accounts/ensure.rs | 151 ++++++++++++ .../src/plugin/accounts/shared.rs | 102 ++++---- .../locallibrary/src/plugin/accounts/token.rs | 80 ++++--- crates/locallibrary/src/plugin/callbacks.rs | 6 +- .../src/plugin/callbacks/tests.rs | 5 +- crates/locallibrary/src/plugin/process.rs | 12 +- .../locallibrary/src/plugin/process/tests.rs | 7 +- crates/locallibrary/tests/it/plugins.rs | 1 + .../locallibrary/tests/it/plugins/accesses.rs | 38 +-- .../tests/it/plugins/destinations.rs | 11 +- .../locallibrary/tests/it/plugins/ensure.rs | 220 ++++++++++++++++++ 18 files changed, 634 insertions(+), 164 deletions(-) create mode 100644 crates/locallibrary/src/plugin/accounts/ensure.rs create mode 100644 crates/locallibrary/tests/it/plugins/ensure.rs diff --git a/apps/locallibrary/docs/rpc/locallibrary.d.ts b/apps/locallibrary/docs/rpc/locallibrary.d.ts index 716337e53..e3833e3b3 100644 --- a/apps/locallibrary/docs/rpc/locallibrary.d.ts +++ b/apps/locallibrary/docs/rpc/locallibrary.d.ts @@ -7880,9 +7880,10 @@ export type Access = { /** 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. + * The scopes (by name) the user declined when the plugin asked for them, or a sign-in for it + * asked the service for and the service didn't grant all they ask, until it's given them + * again: the plugin isn't asked about them till then. Those it needs are in `scopes` too, and + * still can't be used. */ declined?: string[]; }; diff --git a/apps/locallibrary/docs/rpc/schema.json b/apps/locallibrary/docs/rpc/schema.json index d0526ae8d..9349a2089 100644 --- a/apps/locallibrary/docs/rpc/schema.json +++ b/apps/locallibrary/docs/rpc/schema.json @@ -30453,7 +30453,7 @@ "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." + "description": "The scopes (by name) the user declined when the plugin asked for them, or a sign-in for it\nasked the service for and the service didn't grant all they ask, until it's given them\nagain: the plugin isn't asked about them till then. Those it needs are in `scopes` too, and\nstill can't be used." } }, "required": [ diff --git a/apps/locallibrary/src/hints.rs b/apps/locallibrary/src/hints.rs index 6b493ab2f..7ae990daa 100644 --- a/apps/locallibrary/src/hints.rs +++ b/apps/locallibrary/src/hints.rs @@ -113,6 +113,12 @@ fn command(a: &Action) -> Option { let subject = a.params.get("account").and_then(|x| x.get("subject")).and_then(Value::as_str).map(quote); format!("locallibrary plugin access {} --default {}", p("plugin")?, subject?) } + "accounts.access.give" => { + let subject = a.params.get("account").and_then(|x| x.get("subject")).and_then(Value::as_str).map(quote); + let scopes = a.params.get("scopes").and_then(Value::as_array).into_iter().flatten(); + let scopes: String = scopes.filter_map(Value::as_str).map(|s| format!(" --scope {}", quote(s))).collect(); + format!("locallibrary plugin access {} --give {}{scopes}", p("plugin")?, subject?) + } "settings.schema" => "locallibrary settings schema".into(), "settings.import.plan" => "locallibrary settings import ".into(), "library.list" => "locallibrary library ls".into(), diff --git a/apps/locallibrary/src/hints/tests.rs b/apps/locallibrary/src/hints/tests.rs index ff3a0ae68..5ec093760 100644 --- a/apps/locallibrary/src/hints/tests.rs +++ b/apps/locallibrary/src/hints/tests.rs @@ -114,6 +114,7 @@ fn hints_are_command_lines_that_parse(tc: hegel::TestCase) { "account.login.begin", "accounts.sign_in.begin", "accounts.access.set_default", + "accounts.access.give", "settings.schema", "settings.import.plan", "library.list", @@ -163,7 +164,7 @@ fn hints_are_command_lines_that_parse(tc: hegel::TestCase) { "snapshot.label" => &["label"], "accounts.sign_in.begin" if again => &["with", "for"], "accounts.sign_in.begin" => &["type", "with", "for"], - "accounts.access.set_default" => &["plugin"], + "accounts.access.set_default" | "accounts.access.give" => &["plugin"], "undo.list" | "account.login.begin" | "settings.schema" | "settings.import.plan" | "library.list" => &[], _ => &["drive"], }; @@ -175,6 +176,10 @@ fn hints_are_command_lines_that_parse(tc: hegel::TestCase) { 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()), + "accounts.access.give" => { + comes_back(params["account"]["subject"].as_str().unwrap()); + scopes.iter().for_each(|s| comes_back(s)); + } _ => {} } diff --git a/crates/locallibrary/src/plugin.rs b/crates/locallibrary/src/plugin.rs index b3a4122c9..7bc8bbe20 100644 --- a/crates/locallibrary/src/plugin.rs +++ b/crates/locallibrary/src/plugin.rs @@ -59,7 +59,7 @@ mod sources; use std::ffi::OsString; use std::path::PathBuf; -use std::sync::{Arc, Mutex, OnceLock}; +use std::sync::Arc; use std::time::Duration; use indexmap::IndexMap; @@ -74,6 +74,7 @@ use crate::{Error, Result}; pub use accounts::REFRESH_BEFORE; pub use accounts::access::{Access, AccountId}; +pub use accounts::ensure::{AccessQuestion, AskAccess}; pub use accounts::ready::Unready; pub use accounts::shared::{Account, AccountSignIn, AccountSignedIn, AccountSignedOut, Accounts}; pub use analysis::{AnalysisPart, ApplyAnalysis, same_grid}; @@ -192,6 +193,9 @@ pub struct Options { /// What plugins tell the user (`host/notify`), and that a service was left off: each front /// end shows them its own way. Called on the thread reading the plugin, so it mustn't wait. pub notices: Notices, + /// How the front end asks the user whether a plugin may use more of an account it has access + /// to (`accounts/ensure`, see [`AskAccess`]): with none, the plugin is told how to give it. + pub ask: Option>, /// How many times in a row a service may stop or fail before it's left off. pub restarts: u32, /// How long a service that stopped waits to be started again, the first time: twice that the @@ -227,6 +231,7 @@ impl Default for Options { log: Arc::new(|id, line| eprintln!("{id}: {line}")), events: 64, notices: Arc::new(|n| eprintln!("{}: {}", n.plugin, n.message)), + ask: None, restarts: 5, backoff: Duration::from_secs(1), } @@ -238,8 +243,6 @@ pub struct Plugins { plugins: IndexMap>, /// What every plugin is hosted with. host: Arc, - /// Sign-ins to accounts of a type waiting for the browser. - signing_in: accounts::SigningIn, } impl Plugins { @@ -248,8 +251,7 @@ impl Plugins { /// [`PluginSpec::built_in`]), two plugins with one id, and two commands (or operations) with /// one `cli` place (the app checks them against its own commands). pub fn new(specs: Vec, reads: Arc, options: Options) -> Result { - let host = - Arc::new(process::Host { reads, options, turns: Mutex::default(), account_plugins: OnceLock::new() }); + let host = Arc::new(process::Host::new(reads, options)); let mut plugins = IndexMap::new(); let mut cli: FxHashSet = FxHashSet::default(); for spec in specs { @@ -273,7 +275,7 @@ impl Plugins { let account_plugins = account_plugins.map(|(id, s)| (id.clone(), Arc::downgrade(&s.plugin))).collect(); // Set here only, once every plugin is. let _ = host.account_plugins.set(account_plugins); - Ok(Plugins { plugins, host, signing_in: Default::default() }) + Ok(Plugins { plugins, host }) } /// Each plugin's manifest, in the order the app gave them. diff --git a/crates/locallibrary/src/plugin/accounts.rs b/crates/locallibrary/src/plugin/accounts.rs index a0bb47b9c..170a0694d 100644 --- a/crates/locallibrary/src/plugin/accounts.rs +++ b/crates/locallibrary/src/plugin/accounts.rs @@ -31,7 +31,7 @@ use serde::{Deserialize, Serialize}; use super::Plugins; use super::net::unreachable; -use super::process::Plugin; +use super::process::{Host, Plugin}; use crate::callback::{self, CallbackQuery}; use crate::export::one_line; use crate::net::with_causes; @@ -40,6 +40,7 @@ use crate::store::Store; use crate::{Error, Result, lock}; pub(super) mod access; +pub(super) mod ensure; mod kept; pub(super) mod ready; pub(super) mod shared; @@ -255,26 +256,26 @@ impl Plugin { } } -impl Plugins { - /// Starts a sign-in to what `asked` asks for in `store` at `t`'s service (of `slot`), asking - /// for `scopes` and sending the authorize `params` besides: its id, the address to open in the - /// browser, and where the browser comes back to. +impl Plugin { + /// Starts a sign-in to what `asked` asks for in `store` at its account type `t`'s service, + /// asking for `scopes` and sending the authorize `params` besides: its id, the address to open + /// in the browser, and where the browser comes back to. fn begin( &self, - slot: &Plugin, t: &AccountType, scopes: &[String], params: &IndexMap, asked: shared::Asked, store: &Store, ) -> Result<(String, String, String)> { + let signing_in = &self.host.signing_in; if let Some(port) = t.redirect_port { // A sign-in still waiting on its port gives way: the browser comes back to the newest. - lock(&self.signing_in).retain(|_, p| p.listener.local_addr().map_or(true, |a| a.port() != port)); + lock(signing_in).retain(|_, p| p.listener.local_addr().map_or(true, |a| a.port() != port)); } let (listener, redirect_uri) = callback::bind(t.redirect_port.unwrap_or(0))?; let redirect = RedirectUrl::new(redirect_uri.clone()).map_err(|e| Error::Account(e.to_string()))?; - let client = slot.sign_in_client(t, Some(redirect))?; + let client = self.sign_in_client(t, Some(redirect))?; let (challenge, verifier) = PkceCodeChallenge::new_random_sha256(); let scopes = scopes.iter().cloned().map(Scope::new); let mut request = client.authorize_url(CsrfToken::new_random).add_scopes(scopes).set_pkce_challenge(challenge); @@ -282,18 +283,20 @@ impl Plugins { request = request.add_extra_param(name, value); } let (url, state) = request.url(); - if let Some(why) = unreachable(&url, slot.loopback()) { - return Err(slot.misbehaved(format!("its {} account's sign-in page can't be opened: {why}", t.title))); + if let Some(why) = unreachable(&url, self.loopback()) { + return Err(self.misbehaved(format!("its {} account's sign-in page can't be opened: {why}", t.title))); } let id = uuid::Uuid::new_v4().simple().to_string(); - let mut pending = lock(&self.signing_in); + let mut pending = lock(signing_in); pending.retain(|_, p| p.started.elapsed() < PENDING_LIMIT); let waiting = Pending { asked, store: store.clone(), listener, client, state, verifier, started: Instant::now() }; pending.insert(id.clone(), waiting); Ok((id, url.to_string(), redirect_uri)) } +} +impl Host { /// Takes sign-in `id` from those waiting for the browser. fn pending(&self, id: &str) -> Result { lock(&self.signing_in).remove(id).ok_or_else(|| { @@ -303,10 +306,12 @@ impl Plugins { )) }) } +} +impl Plugins { /// Forgets sign-in `id`, if it's waiting. pub fn cancel_sign_in(&self, id: &str) -> bool { - lock(&self.signing_in).remove(id).is_some() + lock(&self.host.signing_in).remove(id).is_some() } } diff --git a/crates/locallibrary/src/plugin/accounts/access.rs b/crates/locallibrary/src/plugin/accounts/access.rs index 17600d837..c58beaa6c 100644 --- a/crates/locallibrary/src/plugin/accounts/access.rs +++ b/crates/locallibrary/src/plugin/accounts/access.rs @@ -35,9 +35,10 @@ 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. + /// The scopes (by name) the user declined when the plugin asked for them, or a sign-in for it + /// asked the service for and the service didn't grant all they ask, until it's given them + /// again: the plugin isn't asked about them till then. Those it needs are in `scopes` too, and + /// still can't be used. #[serde(default, skip_serializing_if = "Vec::is_empty")] pub declined: Vec, } @@ -49,6 +50,19 @@ pub struct AccountId { pub subject: String, } +impl Access { + /// Gives it `scopes` besides those it has, remembering that `declined` were declined (and no + /// longer any it's given). + pub(super) fn more(&mut self, scopes: &[String], declined: &[String]) { + let mut all: IndexSet = self.scopes.drain(..).collect(); + all.extend(scopes.iter().cloned()); + self.scopes = all.into_iter().collect(); + let mut still: IndexSet = self.declined.drain(..).filter(|s| !scopes.contains(s)).collect(); + still.extend(declined.iter().cloned()); + self.declined = still.into_iter().collect(); + } +} + impl AccountId { fn of(k: &Kept) -> AccountId { AccountId { plugin: k.plugin.clone(), subject: k.subject.clone() } @@ -82,14 +96,8 @@ impl Book { self.accesses.len() - 1 } }; - let a = &mut self.accesses[i]; - 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() + self.accesses[i].more(scopes, declined); + self.accesses[i].clone() } /// The accesses to `k` say what it's shown as now. @@ -189,9 +197,9 @@ impl Plugins { } /// 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 -/// making each `store` keeps of the type its default. +/// type `kind` (one it asks for; any it asks for when it doesn't), as its error `said`: giving its +/// access more, or signing in again as its account (see [`more_of`]), signing in to one for it, +/// and making each `store` keeps of the type its default. pub(crate) fn actions( host: &Host, manifest: &Manifest, @@ -208,7 +216,7 @@ pub(crate) fn actions( let plugin = &manifest.id; let mut actions = Vec::new(); for kind in kinds { - actions.extend(sign_in_again(host, manifest, kind, &book, said)); + actions.extend(more_of(host, manifest, kind, &book, said).into_iter().flatten()); 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) { @@ -219,19 +227,20 @@ pub(crate) fn actions( 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 (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( +/// Giving the plugin (its `manifest`) more of the account of its access of type `kind` that its +/// error `said` it used, else its default: giving the access the scopes the error says it `needs` +/// that it hasn't and the account was granted what they ask, and signing in again as the account +/// when that 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 another (a fresh +/// token for it, or one the account wasn't granted). Only scopes the manifest asks for are given +/// or allowed, whatever the error says, and the access and account are the store's. +fn more_of( host: &Host, manifest: &Manifest, kind: &str, book: &Book, said: &Map, -) -> Option { +) -> 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); @@ -242,25 +251,32 @@ fn sign_in_again( let needs: Vec<&str> = said.get("needs").and_then(Value::as_array).into_iter().flatten().filter_map(Value::as_str).collect(); 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)) + let needed = |name: &String| needs.contains(&name.as_str()); + let givable = |name: &String, scope: &Scope| needed(name) && !a.scopes.contains(name) && granted(scope); + let asked = need.scopes.iter().chain(need.optional.iter().flat_map(|o| &o.scopes)); + type Named<'a> = Vec<(&'a String, &'a Scope)>; + let (giving, allowing): (Named, Named) = asked .filter_map(|name| Some((name, t.scopes.get(name)?))) - .filter(|(name, scope)| needs.contains(&name.as_str()) || a.scopes.contains(name) && !granted(scope)) - .collect(); - if allowing.is_empty() { - return None; + .filter(|(name, scope)| needed(name) || a.scopes.contains(name) && !granted(scope)) + .partition(|(name, scope)| givable(name, scope)); + let titles = |of: &Named| of.iter().map(|(_, s)| s.title.as_str()).collect::>().join(", "); + let names = |of: &Named| of.iter().map(|(name, _)| name.to_string()).collect::>(); + let mut actions = Vec::new(); + if !giving.is_empty() { + let params = json!({ "plugin": manifest.id, "account": a.account, "scopes": names(&giving) }); + let label = format!("Allow {} with {}", titles(&giving), a.name); + actions.push(Action::call("accounts.access.give", params, label)); + } + if !allowing.is_empty() { + let label = format!("Sign in again as {}, allowing {}", a.name, titles(&allowing)); + let params = json!({ + "type": kind, + "with": a.account.plugin, + "account": a.account.subject, + "for": manifest.id, + "scopes": names(&allowing), + }); + actions.push(Action::call("accounts.sign_in.begin", params, label)); } - 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)) + Some(actions) } diff --git a/crates/locallibrary/src/plugin/accounts/ensure.rs b/crates/locallibrary/src/plugin/accounts/ensure.rs new file mode 100644 index 000000000..19dc555df --- /dev/null +++ b/crates/locallibrary/src/plugin/accounts/ensure.rs @@ -0,0 +1,151 @@ +//! Which of an account's scopes a plugin may use (`accounts/ensure`), each of them one its manifest +//! asks for: those its access has and the account was granted what they ask; not those the user +//! (or the service) declined for the access before; and the rest as the user says, asked by the +//! front end (see [`AskAccess`]). Allowed, the access is given them, signing in again first when +//! the account is signed out or wasn't granted what they ask; declined, the access remembers it, +//! and the plugin isn't asked about them again until the user gives them. Where nobody can be +//! asked, it fails as `scope`, `needs` naming them, which passed on says how to give them. Without +//! an access of the type it fails as a request would (`access`). Never a token. + +use std::sync::Arc; + +use loclib_plugin::{AccountNeed, EnsureAccount, Ensured, RpcError, params}; +use serde_json::Value; + +use super::PENDING_LIMIT; +use super::access::Access; +use super::kept; +use super::token::{about, failed}; +use crate::Error; +use crate::api::RpcFailure; +use crate::plugin::callbacks::{for_plugin, known_roots}; +use crate::plugin::process::{Live, Plugin}; +use crate::progress::Ctx; +use crate::store::Store; + +/// What a plugin asks to use of an account it has access to, for the user to allow. +#[derive(Debug, Clone, PartialEq)] +pub struct AccessQuestion { + /// The plugin, as it's shown. + pub plugin: String, + /// Its access to the account. + pub access: Access, + /// The account type's title. + pub title: String, + /// The scopes it asks for, by name, and their titles. + pub scopes: Vec, + pub titles: Vec, + /// Why it asks for them, as its manifest says. + pub why: Vec, + /// Allowing them signs in again first: the account is signed out, or wasn't granted what they + /// ask. + pub sign_in: bool, +} + +/// How a front end asks the user whether a plugin may use more of an account it has access to +/// (see [`crate::plugin::Options::ask`]). +pub trait AskAccess: Send + Sync { + /// Whether the user allows what `question` asks: `None` when they can't be asked here. + fn allow(&self, question: &AccessQuestion) -> Option; + + /// Opens `url` in the browser to sign in again (or says to): the host waits for the browser + /// to come back. + fn sign_in(&self, url: &str); +} + +/// Where the scopes a plugin asks about stand, as the store has them now. +struct Standing { + /// The account plugin that signed in the access's account. + with: Arc, + granted: Vec, + declined: Vec, + /// The others, for the user to allow. + question: AccessQuestion, +} + +impl Plugin { + /// Answers `accounts/ensure` (`asked`, an [`EnsureAccount`]) with [`Ensured`], asking the user + /// as the module's docs say; cancelled through `ctx`. A scope it doesn't ask for is refused. + pub(crate) fn ensure(&self, live: &Live, asked: Value, ctx: &Ctx<'_>) -> Result { + let req: EnsureAccount = params(asked)?; + let refused = |e: Error| RpcError::invalid_params(e.to_string()); + self.given(&req.kind, &req.scopes).map_err(refused)?; + let need = self.needs(&req.kind).map_err(refused)?; + let mut now = self.standing(live, need, &req)?; + if !now.question.scopes.is_empty() { + self.ask(live, need, now, ctx)?; + now = self.standing(live, need, &req)?; + } + let declined = now.declined.into_iter().chain(now.question.scopes).collect(); + Ok(serde_json::to_value(Ensured { granted: now.granted, declined }).expect("an answer is JSON")) + } + + /// Where the scopes `req` asks about stand (see [`Standing`]). + fn standing(&self, live: &Live, need: &AccountNeed, req: &EnsureAccount) -> Result { + self.with_access(live, need, req.access.as_deref(), |a, k, with, t| { + let serviced = |name: &String| { + let asks = t.scopes.get(name).map(|s| &s.asks); + k.is_some_and(|k| asks.is_some_and(|asks| asks.iter().all(|x| k.scopes.contains(x)))) + }; + let (granted, rest): (Vec, Vec) = + req.scopes.iter().cloned().partition(|s| a.scopes.contains(s) && serviced(s)); + let (declined, open): (Vec, Vec) = rest.into_iter().partition(|s| a.declined.contains(s)); + let titles = open.iter().map(|s| t.scopes.get(s).map_or(s, |s| &s.title).clone()).collect(); + let groups = + std::iter::once((&need.scopes, &need.why)).chain(need.optional.iter().map(|o| (&o.scopes, &o.why))); + let why: Vec = groups + .filter(|(scopes, _)| scopes.iter().any(|s| open.contains(s))) + .map(|(_, why)| why.clone()) + .collect(); + let question = AccessQuestion { + plugin: self.manifest.name.clone(), + access: a.clone(), + title: t.title.clone(), + sign_in: !open.iter().all(serviced), + scopes: open, + titles, + why, + }; + Ok(Standing { with: with.clone(), granted, declined, question }) + }) + } + + /// Asks the user about `s`'s question, and does as they say: gives the access its scopes + /// (signing in again first when it has to) or remembers they declined them. Fails as the + /// module's docs say when nobody can be asked. + fn ask(&self, live: &Live, need: &AccountNeed, s: Standing, ctx: &Ctx<'_>) -> Result<(), RpcError> { + let Standing { with, question: q, .. } = s; + let store = live.store.as_ref().expect("an access is to an account a store keeps"); + let asked = self.host.options.ask.as_ref().and_then(|ask| Some((ask, ask.allow(&q)?))); + let Some((ask, allowed)) = asked else { return Err(unasked(need, &q)) }; + let (id, scopes) = (&q.access.id, &q.scopes); + let external = |e: Error| for_plugin(RpcFailure::from(e), &known_roots(Some(store))); + if !allowed { + return remember(store, id, |a| a.more(&[], scopes)).map_err(external); + } + if !q.sign_in { + return remember(store, id, |a| a.more(scopes, &[])).map_err(external); + } + let account = &q.access.account.subject; + let start = with.begin_sign_in(&q.access.kind, Some(self), Some(account), scopes, store).map_err(external)?; + ask.sign_in(&start.url); + self.host.finish_sign_in(&start.id, PENDING_LIMIT, ctx).map(drop).map_err(external) + } +} + +/// Changes the access `id` in `store` with `f`. +fn remember(store: &Store, id: &str, f: impl FnMut(&mut Access)) -> crate::Result<()> { + kept::update(store, |book| { + book.accesses.iter_mut().filter(|a| a.id == id).for_each(f); + Ok(()) + }) +} + +/// What a plugin is answered when nobody can be asked `q`: `scope`, naming `q`'s scopes, which +/// giving the access or signing in again `needs` to allow. +fn unasked(need: &AccountNeed, q: &AccessQuestion) -> RpcError { + let a = &q.access; + let why = + format!("it may not {} with the {} account {} until it's given them", q.titles.join(", "), need.name, a.name); + about(failed("scope", why, need, &q.scopes, vec![a.account.plugin.clone()]), a, &q.scopes) +} diff --git a/crates/locallibrary/src/plugin/accounts/shared.rs b/crates/locallibrary/src/plugin/accounts/shared.rs index 3173fca7f..8ef986539 100644 --- a/crates/locallibrary/src/plugin/accounts/shared.rs +++ b/crates/locallibrary/src/plugin/accounts/shared.rs @@ -21,7 +21,7 @@ use crate::callback::CallbackQuery; use crate::error::Reason; use crate::keychain::Storage; use crate::plugin::Plugins; -use crate::plugin::process::Plugin; +use crate::plugin::process::{Host, Plugin}; use crate::plugin::secrets::secret; use crate::progress::Ctx; use crate::store::Store; @@ -109,9 +109,9 @@ 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 as far as - /// the service grants them. - for_plugin: Option<(String, Vec)>, + /// The plugin it's for, the type's scopes (by name) it's given of the account as far as the + /// service grants them, and those it needs, which it's given whatever the service grants. + for_plugin: Option<(String, Vec, Vec)>, /// The account signed in again, by subject. account: Option, /// The service's scopes. @@ -316,8 +316,48 @@ impl Plugins { scopes: &[String], store: &Store, ) -> Result { - let slot = self.slot(with)?; - let t = slot.account_type(kind)?; + let for_plugin = for_plugin.map(|p| self.slot(p)).transpose()?; + self.slot(with)?.begin_sign_in(kind, for_plugin.map(|s| &*s.plugin), account, scopes, store) + } + + /// Waits for the browser to come back to sign-in `id` (at most `timeout`, an hour at most), + /// trades its code for the account's tokens, asks the service whose they are, and keeps the + /// account. The browser is told how it went. + pub fn finish_account_sign_in(&self, id: &str, timeout: Duration, ctx: &Ctx<'_>) -> Result { + self.host.finish_sign_in(id, timeout, ctx) + } + + /// Finishes sign-in `id` with the address the browser ended up on, pasted from another + /// computer. + pub fn finish_account_sign_in_with_url(&self, id: &str, url: &str) -> Result { + let p = self.host.pending(id)?; + let query = pasted(&p, url)?; + self.slot(&p.asked.with)?.signed_in(p, &query) + } +} + +impl Host { + /// [`Plugins::finish_account_sign_in`]. + pub(super) fn finish_sign_in(&self, id: &str, timeout: Duration, ctx: &Ctx<'_>) -> Result { + let p = self.pending(id)?; + let with = + self.account_plugin(&p.asked.with).ok_or_else(|| Error::NotFound(format!("plugin {:?}", p.asked.with)))?; + come_back(p, timeout, ctx, |p, query| with.signed_in(p, query), |s| s.account.name.clone()) + } +} + +impl Plugin { + /// [`Plugins::begin_account_sign_in`] through it, for `for_plugin` if it's for one. + pub(super) fn begin_sign_in( + &self, + kind: &str, + for_plugin: Option<&Plugin>, + account: Option<&str>, + scopes: &[String], + store: &Store, + ) -> Result { + let with = self.manifest.id.as_str(); + let t = self.account_type(kind)?; let book = kept::read(store)?; let mut asking: IndexSet = t.always.iter().cloned().collect(); let mut params = t.params.clone(); @@ -341,10 +381,10 @@ impl Plugins { None if book.accounts.iter().any(|k| k.plugin == with) => params.extend(t.another_account.clone()), None => {} } - let given = - for_plugin.map(|plugin| Ok::<_, Error>((plugin.to_string(), self.slot(plugin)?.given(kind, scopes)?))); + let given = for_plugin + .map(|p| Ok::<_, Error>((p.manifest.id.clone(), p.given(kind, scopes)?, p.needs(kind)?.scopes.clone()))); let given = given.transpose()?; - for name in scopes.iter().chain(given.iter().flat_map(|(_, given)| given)) { + 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()); @@ -357,56 +397,36 @@ impl Plugins { account: account.map(String::from), scopes: scopes.clone(), }; - let (id, url, redirect_uri) = self.begin(slot, t, &scopes, ¶ms, asked, store)?; + let (id, url, redirect_uri) = self.begin(t, &scopes, ¶ms, asked, store)?; Ok(AccountSignIn { id, kind: kind.into(), plugin: with.into(), url, redirect_uri }) } - /// Waits for the browser to come back to sign-in `id` (at most `timeout`, an hour at most), - /// trades its code for the account's tokens, asks the service whose they are, and keeps the - /// account. The browser is told how it went. - pub fn finish_account_sign_in(&self, id: &str, timeout: Duration, ctx: &Ctx<'_>) -> Result { - let p = self.pending(id)?; - come_back(p, timeout, ctx, |p, query| self.signed_in_account(p, query), |s| s.account.name.clone()) - } - - /// Finishes sign-in `id` with the address the browser ended up on, pasted from another - /// computer. - pub fn finish_account_sign_in_with_url(&self, id: &str, url: &str) -> Result { - let p = self.pending(id)?; - let query = pasted(&p, url)?; - self.signed_in_account(p, &query) - } - - /// The account `p` asked for, signed in from what the browser came back to it with. - fn signed_in_account(&self, p: Pending, query: &CallbackQuery) -> Result { + /// The account `p` asked for, signed in through it from what the browser came back to `p` + /// with. + fn signed_in(&self, p: Pending, query: &CallbackQuery) -> Result { let asked = p.asked; - let slot = self.slot(&asked.with)?; - let t = slot.account_type(&asked.kind)?; - let answer = slot.traded(t, &p.client, p.verifier, query)?; + let t = self.account_type(&asked.kind)?; + let answer = self.traded(t, &p.client, p.verifier, query)?; let access_token = answer.access_token().secret(); - slot.net.saw(access_token); - let (subject, name) = slot.whoami(t, access_token)?; + self.net.saw(access_token); + let (subject, name) = self.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 // 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 had = had.and_then(|k| self.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); + self.seen(&tokens); 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)) { @@ -422,7 +442,7 @@ impl Plugins { kept::keep(storage, store, k, Some(&tokens))?; let k = k.clone(); book.renamed(&k); - let Some((plugin, scopes)) = &for_plugin else { return Ok((listed(k, true), None, Vec::new())) }; + let Some((plugin, scopes, needs)) = &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(); @@ -433,7 +453,9 @@ impl Plugins { })?; Ok(AccountSignedIn { account, instead_of, access, declined }) } +} +impl Plugins { /// The access token of the account `subject` that `with` signed in to in `store`, as a /// plugin's request gets it (see [`Plugin::account_token`]). pub(crate) fn account_token(&self, store: &Store, with: &str, subject: &str) -> Result { diff --git a/crates/locallibrary/src/plugin/accounts/token.rs b/crates/locallibrary/src/plugin/accounts/token.rs index 642abf41e..0ab1e87cd 100644 --- a/crates/locallibrary/src/plugin/accounts/token.rs +++ b/crates/locallibrary/src/plugin/accounts/token.rs @@ -12,12 +12,12 @@ use std::sync::Arc; -use loclib_plugin::{AccountNeed, ErrorData, RpcError, Scope, TokenPlace}; +use loclib_plugin::{AccountNeed, AccountType, ErrorData, RpcError, Scope, TokenPlace}; use reqwest::Url; use serde_json::{Map, json}; use super::access::Access; -use super::kept; +use super::kept::{self, Kept}; use crate::Error; use crate::error::Reason; use crate::plugin::process::{Live, Plugin}; @@ -36,7 +36,7 @@ pub(crate) struct Usable { } /// A failure of kind `kind` naming the type `need` asks for, `scopes` and the account `plugins`. -fn failed(kind: &str, why: String, need: &AccountNeed, scopes: &[String], plugins: Vec) -> RpcError { +pub(super) fn failed(kind: &str, why: String, need: &AccountNeed, scopes: &[String], plugins: Vec) -> RpcError { let fields = Map::from_iter([ ("type".to_string(), json!(need.name)), ("scopes".to_string(), json!(scopes)), @@ -55,7 +55,7 @@ fn signed_out(why: String, need: &AccountNeed, a: &Access) -> RpcError { /// `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 { +pub(super) 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)); @@ -75,6 +75,50 @@ impl Plugin { method: &str, url: &Url, ) -> Result { + let kind = &need.name; + self.with_access(live, need, access, |a, k, with, t| { + 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); + let k = k.ok_or_else(gone)?; + // 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 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, &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, &[])); + } + 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, &names)); + } + let scopes = scopes().filter(|(_, s)| is_granted(s)).map(|(n, s)| (n.clone(), s.clone())).collect(); + Ok(Usable { with: with.clone(), access: a.clone(), need: need.clone(), place: t.token.clone(), scopes }) + }) + } + + /// What `f` makes of its access of the type `need` asks for that `access` names, else its + /// default for the type: the access, its account unless it's signed out, and the account plugin + /// that signed it in with the type as it provides it. `access` when it has no such access, or + /// that account plugin isn't installed. + pub(super) fn with_access( + &self, + live: &Live, + need: &AccountNeed, + access: Option<&str>, + f: impl FnOnce(&Access, Option<&Kept>, &Arc, &AccountType) -> Result, + ) -> Result { let kind = &need.name; let no_access = |why: String| failed("access", why, need, &need.scopes, self.host.providing(kind)); let Some(store) = live.store.as_ref() else { @@ -89,37 +133,13 @@ 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); - 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 { + let (Some(with), Some(t)) = (&with, t) else { 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 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, &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, &[])); - } - 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, &names)); - } - 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 }) + f(a, book.account(on, &a.account.subject), with, t) } /// The token of `usable`'s account, told to [`crate::plugin::net::State::scrub`]: `access` diff --git a/crates/locallibrary/src/plugin/callbacks.rs b/crates/locallibrary/src/plugin/callbacks.rs index 784a9913a..27e52ecd0 100644 --- a/crates/locallibrary/src/plugin/callbacks.rs +++ b/crates/locallibrary/src/plugin/callbacks.rs @@ -3,7 +3,8 @@ //! capabilities grant, with only the params listed for each, naming drives the host finds itself //! (never by path), in pages of a bounded size, a few calls at a time; what it gets back, errors //! too, says nothing of where things are on this computer (see [`answer_for_plugin`]). A plugin -//! with the `net` feature calls `net/fetch` here too (see [`super::net`]). +//! with the `net` feature calls `net/fetch` here too (see [`super::net`]), and one with the +//! `accounts` feature `accounts/ensure` (see [`super::accounts::ensure`]). use std::cmp::Reverse; use std::path::PathBuf; @@ -186,6 +187,9 @@ pub(crate) fn answer( let reads = &*plugin.host.reads; let run = || match req.method.as_str() { method::NET_FETCH if live.sends(method::NET_FETCH) => plugin.fetch(&live, req.params, &req.cancel), + method::ACCOUNTS_ENSURE if live.sends(method::ACCOUNTS_ENSURE) => { + plugin.ensure(&live, req.params, &Ctx::new(&NoProgress, req.cancel.clone())) + } _ => { let params = check(&plugin.manifest, reads, &req.method, req.params); let read = diff --git a/crates/locallibrary/src/plugin/callbacks/tests.rs b/crates/locallibrary/src/plugin/callbacks/tests.rs index 0716e7dda..84ba03f7c 100644 --- a/crates/locallibrary/src/plugin/callbacks/tests.rs +++ b/crates/locallibrary/src/plugin/callbacks/tests.rs @@ -1,7 +1,7 @@ use std::io::pipe; use std::path::{Path, PathBuf}; +use std::sync::Mutex; use std::sync::mpsc::{self, Sender}; -use std::sync::{Mutex, OnceLock}; use std::time::Duration; use hegel::TestCase; @@ -622,8 +622,7 @@ fn a_plugin_has_at_most_its_quota_of_calls_under_way(tc: TestCase) { let ((entered_tx, entered), (release, release_rx)) = (mpsc::channel(), mpsc::channel()); let reads = Held { entered: Mutex::new(entered_tx), release: Mutex::new(release_rx), file }; let options = Options { callbacks: quota, ..Options::default() }; - let host = - Arc::new(Host { reads: Arc::new(reads), options, turns: Mutex::default(), account_plugins: OnceLock::new() }); + let host = Arc::new(Host::new(Arc::new(reads), options)); let launch = Launch { program: "nothing".into(), args: Vec::new(), env_remove: Vec::new() }; let (host_in, plugin_out) = pipe().unwrap(); let (plugin_in, host_out) = pipe().unwrap(); diff --git a/crates/locallibrary/src/plugin/process.rs b/crates/locallibrary/src/plugin/process.rs index b20dbdc84..11352405f 100644 --- a/crates/locallibrary/src/plugin/process.rs +++ b/crates/locallibrary/src/plugin/process.rs @@ -20,7 +20,7 @@ use serde::Serialize; use serde::de::DeserializeOwned; use serde_json::{Value, json}; -use super::accounts::access; +use super::accounts::{self, access}; use super::blobs::{Scratch, Scratches}; use super::events::Queue; use super::services::Running; @@ -51,9 +51,19 @@ pub(crate) struct Host { /// The account plugins, by id, once every plugin is: those whose accounts plugins' requests /// name. Weak, since each holds the host. pub(crate) account_plugins: OnceLock>>, + /// Sign-ins to accounts of a type waiting for the browser, whichever account plugin began + /// them. + pub(crate) signing_in: accounts::SigningIn, } impl Host { + /// Hosting plugins as `options` say, reading for them through `reads`: with no account plugins + /// until they're set. + pub(crate) fn new(reads: Arc, options: Options) -> Host { + let (turns, account_plugins, signing_in) = Default::default(); + Host { reads, options, turns, account_plugins, signing_in } + } + /// Logs `line` as the plugin `id`'s. pub(crate) fn log(&self, id: &str, line: &str) { (self.options.log)(id, line); diff --git a/crates/locallibrary/src/plugin/process/tests.rs b/crates/locallibrary/src/plugin/process/tests.rs index 94d30f8f0..b2feceea3 100644 --- a/crates/locallibrary/src/plugin/process/tests.rs +++ b/crates/locallibrary/src/plugin/process/tests.rs @@ -156,12 +156,7 @@ fn an_account_plugin_is_never_started() { ) .unwrap(); manifest.validate(false).unwrap(); - let host = Arc::new(Host { - reads: Arc::new(NoReads), - options: Options::default(), - turns: Mutex::default(), - account_plugins: OnceLock::new(), - }); + let host = Arc::new(Host::new(Arc::new(NoReads), Options::default())); // A program that isn't there: starting it would fail otherwise. let launch = Launch { program: "no-such-program".into(), args: Vec::new(), env_remove: Vec::new() }; let slot = Arc::new(Slot::new(manifest, launch, host)); diff --git a/crates/locallibrary/tests/it/plugins.rs b/crates/locallibrary/tests/it/plugins.rs index 0eea33363..fdc722744 100644 --- a/crates/locallibrary/tests/it/plugins.rs +++ b/crates/locallibrary/tests/it/plugins.rs @@ -9,6 +9,7 @@ mod callbacks; mod checks; mod choose; mod destinations; +mod ensure; mod events; mod exports; mod hooks; diff --git a/crates/locallibrary/tests/it/plugins/accesses.rs b/crates/locallibrary/tests/it/plugins/accesses.rs index 07eb1f3c2..46e5b7b34 100644 --- a/crates/locallibrary/tests/it/plugins/accesses.rs +++ b/crates/locallibrary/tests/it/plugins/accesses.rs @@ -16,10 +16,10 @@ 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"]; +pub(super) const SCOPES: [&str; 3] = ["read", "write", "peek"]; /// What the mock type's scope `scope` asks the service for. -fn asks(scope: &str) -> &'static [&'static str] { +pub(super) fn asks(scope: &str) -> &'static [&'static str] { match scope { "read" => &["read"], "write" => &["write", "read"], @@ -36,12 +36,12 @@ fn covers(scope: &str, method: &str, path: &str) -> bool { } } -fn account(subject: &str) -> AccountId { +pub(super) fn account(subject: &str) -> AccountId { AccountId { plugin: WITH.into(), subject: subject.into() } } /// Some of [`SCOPES`], as the test draws them. -fn some(tc: &TestCase) -> Vec<&'static str> { +pub(super) fn some(tc: &TestCase) -> Vec<&'static str> { SCOPES.into_iter().filter(|_| tc.draw(gs::booleans())).collect() } @@ -340,6 +340,13 @@ fn signing_in_again(subject: &str, name: &str, scopes: &[&str]) -> Action { Action::call("accounts.sign_in.begin", params, format!("Sign in again as {name}, allowing {}", scopes.join(", "))) } +/// The action an `account` error a plugin passes on offers to give echo's access to the account +/// `subject`, shown as `name`, `scopes` it was granted what they ask. +fn giving(subject: &str, name: &str, scopes: &[&str]) -> Action { + let params = json!({ "plugin": "echo", "account": account(subject), "scopes": scopes }); + Action::call("accounts.access.give", params, format!("Allow {} with {name}", scopes.join(", "))) +} + /// An account signed out keeps its accesses, which still list, and a request through one fails as /// `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. @@ -452,15 +459,15 @@ 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`. + // Given `read` of a, which was granted `read write`: giving it `write` too helps, and only that. 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"))] { + for (needs, more) 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}"); + let more = more.map(|scope| giving("a", "a", &[scope])); + assert_eq!(e.detail().actions, [more.into_iter().collect(), resolving(&["a", "b"])].concat(), "{needs}"); } } @@ -576,15 +583,14 @@ 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}"); - // 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"], - _ => &[], + // Giving echo's access what its request was refused, when the account was granted it, else + // signing in again as the account, when that gives it. + let mut actions: Vec = match failing { + SignedOut | Refused => vec![signing_in_again("a", "a", &["read"])], + Ungranted => vec![signing_in_again("a", "a", &["write"])], + Missing if !lying => vec![giving("a", "a", &["read"])], + _ => Vec::new(), }; - 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 } }); diff --git a/crates/locallibrary/tests/it/plugins/destinations.rs b/crates/locallibrary/tests/it/plugins/destinations.rs index cc268978f..edc0b9f6a 100644 --- a/crates/locallibrary/tests/it/plugins/destinations.rs +++ b/crates/locallibrary/tests/it/plugins/destinations.rs @@ -751,9 +751,16 @@ fn a_destination_starts_only_when_its_accounts_will_last(tc: TestCase) { let e = started.err().unwrap(); assert_eq!((e.code(), e.reason()), ("account", Some(reason)), "{said}: {e}"); assert!(asked().is_empty(), "{said}: echo was asked"); + // What resolves it for the account echo has access to comes first: giving its access what it + // lacks when the account was granted that, else signing in again as the account. 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:?}"); + let first = actions.first().and_then(|a| Some((a.method.clone()?, a.params.get("account")?.clone()))); + let givable = matches!(mistake, Some(Mistake::Ungiven)) && asks(needs[0]).iter().all(|a| grants.contains(a)); + let resolving = given.then(|| match givable { + true => ("accounts.access.give".to_string(), json!({ "plugin": WITH, "subject": "a" })), + false => ("accounts.sign_in.begin".to_string(), json!("a")), + }); + assert_eq!(first, resolving, "{said}: {actions:?}"); } /// A step of [`a_store_copied_through_a_plugin_comes_back_as_it_was_last_copied`]. diff --git a/crates/locallibrary/tests/it/plugins/ensure.rs b/crates/locallibrary/tests/it/plugins/ensure.rs new file mode 100644 index 000000000..e4c3b6094 --- /dev/null +++ b/crates/locallibrary/tests/it/plugins/ensure.rs @@ -0,0 +1,220 @@ +//! What more of an account a plugin may use (`accounts/ensure`): what its access has and the +//! service granted, not what the user declined before, and otherwise what the user says when the +//! front end can ask them, signing in again when the service hasn't granted it; with nobody to +//! ask, an error saying how to give it. Never a token. + +use std::sync::OnceLock; +use std::thread::JoinHandle; + +use hegel::TestCase; +use hegel::generators as gs; +use locallibrary::error::Reason; +use locallibrary::plugin::{AccessQuestion, AskAccess}; + +use super::accesses::{SCOPES, account, asks, some}; +use super::accounts::{come_back_to, opened_at}; +use super::shared_accounts::{Mock, WITH, asking, mock_with, options, who}; +use super::*; +use crate::mock_oauth::{Service, account_plugin}; + +/// A front end asking as the test says: answering `answer` to every question (`None`: it can't +/// ask), and signing in again in the browser with the mock service granting `grants`. +#[derive(Default)] +struct Script { + answer: Mutex>, + grants: Mutex>, + asked: Mutex>, + service: OnceLock>>, + browser: Mutex>>, +} + +impl AskAccess for Script { + fn allow(&self, question: &AccessQuestion) -> Option { + self.asked.lock().unwrap().push(question.clone()); + *self.answer.lock().unwrap() + } + + fn sign_in(&self, url: &str) { + let query = opened_at(url); + { + let mut s = self.service.get().unwrap().lock().unwrap(); + (s.challenge, s.signing_in_as) = (query["code_challenge"].clone(), who(json!("a"), None)); + s.grants = Some(self.grants.lock().unwrap().clone()); + } + let back = come_back_to(&query["redirect_uri"], format!("code=c0de&state={}", query["state"])); + self.browser.lock().unwrap().push(back); + } +} + +/// Echo's access to the account `a`, as the test models it: its scopes, those declined, and what +/// the service granted the account (`None` while it's signed out). +#[derive(Debug, Default)] +struct Model { + scopes: Vec<&'static str>, + declined: Vec<&'static str>, + granted: Option>, +} + +impl Model { + /// Whether the account is signed in, and the service granted what `scope` asks. + fn serviced(&self, scope: &str) -> bool { + self.granted.as_ref().is_some_and(|granted| asks(scope).iter().all(|a| granted.contains(a))) + } + + /// Whether echo may use `scope`: its access has it, and the service granted what it asks. + fn usable(&self, scope: &str) -> bool { + self.scopes.contains(&scope) && self.serviced(scope) + } + + /// A sign-in for echo, giving it `given` (the scopes it needs and those asked for), its + /// service granting `grants`: it has those it needs, and the others it was granted what they + /// ask, and remembers those it wasn't. + fn signed_in(&mut self, needs: &[&'static str], given: &[&'static str], grants: Vec<&'static str>) { + self.granted = Some(grants); + let mut given = given.to_vec(); + given.sort_unstable(); + given.dedup(); + let declined: Vec<&str> = given.iter().copied().filter(|s| !self.serviced(s)).collect(); + let giving = given.iter().copied().filter(|s| needs.contains(s) || !declined.contains(s)); + self.declined.retain(|s| !given.contains(s)); + for s in giving { + if !self.scopes.contains(&s) { + self.scopes.push(s); + } + } + self.declined.extend(declined.into_iter().filter(|s| !self.declined.contains(s)).collect::>()); + } +} + +/// How the front end answers in [`ensure_answers_granted_or_declined_never_a_token`]. +#[derive(Debug, Clone, Copy, PartialEq)] +enum Answer { + /// There's no front end to ask. + Nobody, + /// The front end can't ask here. + Cannot, + Allow, + Decline, +} + +/// `scopes`, sorted, to compare as a set. +fn sorted(scopes: &[&str]) -> Vec { + let mut all: Vec = scopes.iter().map(|s| s.to_string()).collect(); + all.sort_unstable(); + all +} + +/// For an echo plugin needing any of the mock type's scopes (the rest optional), given an account +/// signed in for it (the service granting any of what they ask), maybe signed out since, and +/// asking about any scopes in turn, the user answering each time as drawn and the service granting +/// any of what they ask when they sign in again: a scope its access has and the account was +/// granted what it asks is granted; one declined before is declined unasked; any other is asked +/// about, and allowed it's given (signing in again first unless the account was granted what it +/// asks), declined it's remembered, and with nobody to ask the plugin fails as `scope` saying how +/// to give them. One its manifest doesn't ask for is refused. No token gets out. +#[hegel::test(test_cases = 16, suppress_health_check = [hegel::HealthCheck::TooSlow])] +fn ensure_answers_granted_or_declined_never_a_token(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 script = Arc::new(Script::default()); + // Mostly a front end that asks. + let nobody = tc.draw(gs::sampled_from(vec![false, false, true])); + let ask = (!nobody).then(|| script.clone() as Arc); + let manifest = |server: &str| account_plugin(server, &format!("{server}/userinfo"), ""); + let m: Mock = mock_with(3600, manifest, &asking(&needs, &optional), Options { ask, ..options() }); + assert!(script.service.set(m.state.clone()).is_ok()); + let grants = || ["read", "write"].into_iter().filter(|_| tc.draw(gs::booleans())).collect::>(); + + let mut model = Model::default(); + let first = grants(); + m.state.lock().unwrap().grants = Some(first.iter().map(|s| s.to_string()).collect()); + let asked: Vec<&str> = optional.iter().copied().filter(|_| tc.draw(gs::booleans())).collect(); + m.sign_in(who(json!("a"), None), None, &asked).0.unwrap(); + model.signed_in(&needs, &[needs.clone(), asked].concat(), first); + if tc.draw(gs::booleans()) { + m.f.plugins.sign_out_account(WITH, "a", &m.f.store).unwrap(); + model.granted = None; + } + + for step in 0..tc.draw(gs::integers::().min_value(1).max_value(4)) { + let scopes = some(&tc); + let unasked = tc.draw(gs::booleans()) && step == 0; + let answer = match nobody { + true => Answer::Nobody, + false => tc.draw(gs::sampled_from(vec![Answer::Cannot, Answer::Allow, Answer::Decline])), + }; + *script.answer.lock().unwrap() = match answer { + Answer::Allow => Some(true), + Answer::Decline => Some(false), + _ => None, + }; + let again = grants(); + *script.grants.lock().unwrap() = again.iter().map(|s| s.to_string()).collect(); + let named: Vec<&str> = scopes.iter().copied().chain(unasked.then_some("nope")).collect(); + let before = script.asked.lock().unwrap().len(); + let done = m.f.command("ensure", json!({ "type": "mock", "scopes": named }), &Ctx::silent()); + script.browser.lock().unwrap().drain(..).for_each(|b| drop(b.join().unwrap())); + let said = format!("step {step}, {answer:?} {scopes:?} with {model:?}"); + if unasked { + assert_eq!(done.unwrap_err().code(), "invalid", "{said}"); + continue; + } + let open: Vec<&str> = + scopes.iter().copied().filter(|s| !model.usable(s) && !model.declined.contains(s)).collect(); + let sign_in = !open.iter().all(|s| model.serviced(s)); + let questions = script.asked.lock().unwrap()[before..].to_vec(); + match (open.is_empty(), answer) { + (true, _) | (_, Answer::Nobody) => assert!(questions.is_empty(), "{said}: asked {questions:?}"), + _ => { + let [q] = questions.as_slice() else { panic!("{said}: asked {questions:?}") }; + assert_eq!((&q.plugin, &q.access.name, q.sign_in), (&"Echo".to_string(), &"a".to_string(), sign_in)); + let open: Vec = open.iter().map(|s| s.to_string()).collect(); + assert_eq!((&q.scopes, &q.titles), (&open, &open), "{said}"); + } + } + match answer { + _ if open.is_empty() => {} + Answer::Nobody | Answer::Cannot => { + let e = done.as_ref().unwrap_err(); + assert_eq!((e.code(), e.reason()), ("account", Some(Reason::MissingScope)), "{said}: {e}"); + let actions = e.detail().actions; + let offered: Vec<(&str, Vec)> = actions + .iter() + .filter(|a| a.params.get("account").is_some_and(|x| x == "a" || x == &json!(account("a")))) + .map(|a| (a.method.as_deref().unwrap_or_default(), scopes_of(&a.params))) + .collect(); + assert!(!offered.is_empty(), "{said}: nothing offered for a: {e:?}"); + let covered: Vec = offered.iter().flat_map(|(_, s)| s.clone()).collect(); + assert!(open.iter().all(|s| covered.contains(&s.to_string())), "{said}: {offered:?}"); + assert!(!m.leaked(&[json!(e.to_string())]), "a token is out: {said}"); + continue; + } + Answer::Decline => model.declined.extend(open.iter().copied()), + Answer::Allow if sign_in => model.signed_in(&needs, &[needs.clone(), open.clone()].concat(), again), + Answer::Allow => { + model.scopes.extend(open.iter().copied()); + model.declined.retain(|s| !open.contains(s)); + } + } + let answered = done.unwrap(); + let CommandOutput::Headless(answered) = answered else { panic!("{said}: {answered:?}") }; + let (granted, declined): (Vec<&str>, Vec<&str>) = scopes.iter().copied().partition(|s| model.usable(s)); + let listed = |key: &str| answered[key].as_array().unwrap().iter().map(|s| s.as_str().unwrap()).collect(); + let (said_granted, said_declined): (Vec<&str>, Vec<&str>) = (listed("granted"), listed("declined")); + assert_eq!((sorted(&said_granted), sorted(&said_declined)), (sorted(&granted), sorted(&declined)), "{said}"); + assert!(!m.leaked(&[answered]), "a token is out: {said}"); + let access = &m.f.plugins.accesses(&m.f.store, Some("echo"), None).unwrap()[0]; + let have: Vec<&str> = access.scopes.iter().map(String::as_str).collect(); + let left_out: Vec<&str> = access.declined.iter().map(String::as_str).collect(); + assert_eq!((sorted(&have), sorted(&left_out)), (sorted(&model.scopes), sorted(&model.declined)), "{said}"); + } +} + +/// The scopes an action's `params` name. +fn scopes_of(params: &serde_json::Map) -> Vec { + let scopes = params.get("scopes").and_then(Value::as_array).into_iter().flatten(); + scopes.filter_map(Value::as_str).map(String::from).collect() +} From 11c958180d968d4309e4bc3311b21674d105bb4e Mon Sep 17 00:00:00 2001 From: botandbang <337765928+botandbang@users.noreply.github.com> Date: Fri, 30 Oct 2026 18:00:13 -0400 Subject: [PATCH 3/6] feat(locallibrary): ask in a terminal and the player when a plugin wants more of an account --- apps/locallibrary/CHANGELOG.md | 4 +- apps/locallibrary/docs/plugins.md | 7 ++ apps/locallibrary/src/account_cmd.rs | 4 +- apps/locallibrary/src/account_cmd/asking.rs | 35 +++++++++ apps/locallibrary/src/browser.rs | 21 ++++-- apps/locallibrary/src/main.rs | 8 ++ apps/locallibrary/src/output.rs | 28 ++++++- apps/locallibrary/src/output/tests.rs | 10 ++- apps/locallibrary/src/plugins.rs | 35 ++++++++- apps/locallibrary/src/tui/app.rs | 3 + apps/locallibrary/src/tui/form.rs | 4 + apps/locallibrary/src/tui/mod.rs | 2 + apps/locallibrary/src/tui/plugins.rs | 5 +- apps/locallibrary/src/tui/plugins/access.rs | 61 ++++++++++++++- apps/locallibrary/src/tui/plugins/accounts.rs | 8 +- .../src/tui/plugins/tests/accounts.rs | 52 +++++++++++++ apps/locallibrary/src/tui/ui.rs | 1 + .../tests/it/plugins/accounts/cli.rs | 75 +++++++++++++++++++ 18 files changed, 337 insertions(+), 26 deletions(-) create mode 100644 apps/locallibrary/src/account_cmd/asking.rs diff --git a/apps/locallibrary/CHANGELOG.md b/apps/locallibrary/CHANGELOG.md index b654cdcce..3e99bc797 100644 --- a/apps/locallibrary/CHANGELOG.md +++ b/apps/locallibrary/CHANGELOG.md @@ -440,7 +440,9 @@ Windows builds are experimental and untested on hardware (see Platforms in the R the sidecar's `plugins.account*` are gone, and `plugin info` and `plugins.describe` show the access a plugin has instead. A plugin asks for the scopes it can do without as it needs them (`accounts/ensure`, `Host::ensure_account` in the SDK), and learns which it may use and which - you declined, never a token. + you declined, never a token. The terminal and the player ask you first (signing in again when + the account wasn't granted them), and a no is remembered until `plugin access --give` gives the + scope; with `--json`, in `watch` and in the sidecar it fails saying how to give it. - **The library is the unit.** Each drive holding a library is a clone of it, and its name doesn't matter much: a library's name works where a drive does (but in commands about one particular drive) and means its lead clone, the newest of its drives plugged in (the primary when they're diff --git a/apps/locallibrary/docs/plugins.md b/apps/locallibrary/docs/plugins.md index f6f7f7657..4c11677a2 100644 --- a/apps/locallibrary/docs/plugins.md +++ b/apps/locallibrary/docs/plugins.md @@ -239,6 +239,13 @@ In the player's palette, "sign in to a TYPE account…" signs in to one, "sign o 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. +A plugin asks for the scopes it can do without when it needs them. In a terminal, and in the +player, you're asked whether it may, and why it wants them; when the account wasn't granted them, +allowing it signs in again in the browser. A no is remembered: it isn't asked again until +`locallibrary plugin access PLUGIN --give ACCOUNT --scope SCOPE` gives the scope. With `--json`, +`--yes` or no terminal, in `watch` and in the sidecar, nothing's asked: the plugin fails, saying +how to give it the scope. + The tokens are kept in the system's keychain, as secret settings are (see [Settings](#settings)), and refreshed a minute before they expire. A plugin never sees them: a request it makes names the account by its type (`{account:TYPE}`) where the type says its token goes, the `Authorization` diff --git a/apps/locallibrary/src/account_cmd.rs b/apps/locallibrary/src/account_cmd.rs index a836db0a3..168522c9f 100644 --- a/apps/locallibrary/src/account_cmd.rs +++ b/apps/locallibrary/src/account_cmd.rs @@ -1,10 +1,12 @@ //! `locallibrary account`: the accounts the store keeps. With no arguments, the DJ account (an //! Atmosphere account, see [`atmosphere`]) then every other; `account TYPE sign-in` signs in to //! one through an account plugin, and `account TYPE:NAME status|sign-out` shows one or signs out -//! of it (see [`accounts`]). [`access`] gives plugins access to them. +//! of it (see [`accounts`]). [`access`] gives plugins access to them, and [`asking`] asks in the +//! terminal when a plugin wants more of one. pub mod access; mod accounts; +pub mod asking; #[cfg(feature = "account")] mod atmosphere; diff --git a/apps/locallibrary/src/account_cmd/asking.rs b/apps/locallibrary/src/account_cmd/asking.rs new file mode 100644 index 000000000..3b6539816 --- /dev/null +++ b/apps/locallibrary/src/account_cmd/asking.rs @@ -0,0 +1,35 @@ +//! A plugin wanting more of an account it has access to (`accounts/ensure`), asked about in the +//! terminal a command runs in, with its progress bar out of the way: allowing it, or signing in +//! again in the browser to, when the account wasn't granted what it asks. + +use locallibrary::plugin::{AccessQuestion, AskAccess}; + +use crate::browser; +use crate::output::{Drawing, Output, dim}; + +/// Asks on stderr, reading the answer from the terminal. +pub struct Terminal { + pub out: Output, + pub drawing: Drawing, +} + +/// What `q`'s plugin wants, and why. +pub fn wants(q: &AccessQuestion) -> String { + let a = &q.access; + let (titles, why) = (q.titles.join(", "), q.why.join("; ")); + format!("{} wants to {titles} with the {} account {} ({why})", q.plugin, q.title, a.name) +} + +impl AskAccess for Terminal { + fn allow(&self, q: &AccessQuestion) -> Option { + let allow = if q.sign_in { "Sign in again to allow it?" } else { "Allow it?" }; + Some(self.drawing.suspend(|| self.out.confirm(&format!("{}. {allow}", wants(q))))) + } + + fn sign_in(&self, url: &str) { + self.drawing.suspend(|| { + browser::open(url, true); + eprintln!("{}", dim("Waiting for you to approve (Ctrl-C stops)…")); + }); + } +} diff --git a/apps/locallibrary/src/browser.rs b/apps/locallibrary/src/browser.rs index dde194e18..fef37e96a 100644 --- a/apps/locallibrary/src/browser.rs +++ b/apps/locallibrary/src/browser.rs @@ -44,6 +44,18 @@ impl Default for BrowserArgs { } } +/// Opens `url` in the browser to sign in when it `opens` and can, saying so on stderr, or says to +/// open it. +pub fn open(url: &str, opens: bool) { + match opens && locallibrary::opener::open_url(url).is_ok() { + true => { + eprintln!("Opening your browser to sign in…"); + eprintln!("{}", dim(format!("If it didn't open: {url}"))); + } + false => eprintln!("Sign in: open this link\n {url}"), + } +} + /// The browser round trip of a sign-in begun at `url`: opened (or printed), then `finish`ed /// once the browser comes back (waiting as long as `browser` says), or `paste`d the address it /// ended up on, read from the terminal; `cancel`led when there's no terminal to paste into. The @@ -56,14 +68,7 @@ pub fn round_trip( paste: impl FnOnce(&str) -> locallibrary::Result, cancel: impl FnOnce() -> bool, ) -> Result { - let opened = browser.opens() && locallibrary::opener::open_url(url).is_ok(); - match opened { - true => { - eprintln!("Opening your browser to sign in…"); - eprintln!("{}", dim(format!("If it didn't open: {url}"))); - } - false => eprintln!("Sign in: open this link\n {url}"), - } + open(url, browser.opens()); if !browser.paste { eprintln!("{}", dim(format!("Waiting for you to approve (up to {}; Ctrl-C stops)…", span(browser.wait())))); return Ok(finish(browser.wait())?); diff --git a/apps/locallibrary/src/main.rs b/apps/locallibrary/src/main.rs index 7f7e18669..ba402097b 100644 --- a/apps/locallibrary/src/main.rs +++ b/apps/locallibrary/src/main.rs @@ -49,6 +49,7 @@ use indexmap::IndexMap; use std::io::IsTerminal; use std::path::PathBuf; use std::process::ExitCode; +use std::sync::Arc; use anyhow::{Context, Result, bail}; use clap::{Args, FromArgMatches, Parser, Subcommand, ValueEnum}; @@ -1162,6 +1163,13 @@ fn run(cli: Cli, out: Output, contributed: Option) -> R // operations. locallibrary::plugin::set_hosted(|| plugins::host().ok()); let progress = out.progress(); + // A plugin wanting more of an account asks in the terminal: not with `--json` or `--yes`, and + // never in `watch`, the sidecar or a plugin serving, which run on their own. + let alone = cli.command.as_ref().is_some_and(|c| c.handles_signals() || matches!(c, Command::Watch(_))); + if plugin_cmd::choose::interactive(out) && !out.yes && !alone { + let drawing = progress.drawing(); + plugins::ask_to(Some(Arc::new(account_cmd::asking::Terminal { out, drawing }))); + } let ctx = Ctx::new(&progress, cancel); let store = || open_store(&cli.store); diff --git a/apps/locallibrary/src/output.rs b/apps/locallibrary/src/output.rs index 859e63b7c..fd7cbaee1 100644 --- a/apps/locallibrary/src/output.rs +++ b/apps/locallibrary/src/output.rs @@ -3,8 +3,8 @@ use std::borrow::Cow; use std::io::{IsTerminal, Write}; use std::process::ExitCode; -use std::sync::Mutex; use std::sync::atomic::{AtomicBool, Ordering}; +use std::sync::{Arc, Mutex, PoisonError}; use std::time::Duration; use console::style; @@ -304,7 +304,7 @@ impl Output { None } pub fn progress(&self) -> CliProgress { - CliProgress { bar: Mutex::new(None), enabled: !self.json && !self.quiet && std::io::stderr().is_terminal() } + CliProgress { bar: Arc::default(), enabled: !self.json && !self.quiet && std::io::stderr().is_terminal() } } } @@ -605,10 +605,32 @@ pub fn duration(secs: u64) -> String { /// Progress bars on stderr, driven by library events. pub struct CliProgress { /// The current bar, and whether it counts items (true) or bytes (false). - bar: Mutex>, + bar: Arc>>, enabled: bool, } +impl CliProgress { + /// Where it draws, for something else to say there (a question) with the bar out of the way. + pub fn drawing(&self) -> Drawing { + Drawing(self.bar.clone()) + } +} + +/// Where a [`CliProgress`] draws its bars. +#[derive(Clone)] +pub struct Drawing(Arc>>); + +impl Drawing { + /// Runs `f` with the bar hidden, drawing it again after. + pub fn suspend(&self, f: impl FnOnce() -> T) -> T { + let bar = self.0.lock().unwrap_or_else(PoisonError::into_inner).as_ref().map(|(bar, _)| bar.clone()); + match bar { + Some(bar) => bar.suspend(f), + None => f(), + } + } +} + fn phase_label(p: Phase) -> &'static str { match p { Phase::Scan => "Scanning", diff --git a/apps/locallibrary/src/output/tests.rs b/apps/locallibrary/src/output/tests.rs index 916cf7568..302acfb7b 100644 --- a/apps/locallibrary/src/output/tests.rs +++ b/apps/locallibrary/src/output/tests.rs @@ -49,7 +49,7 @@ fn a_wrong_name_declines_an_erase() { /// new phase replaces the bar and its end clears it. #[test] fn progress_bars_count_what_their_phase_counts() { - let p = CliProgress { bar: Mutex::new(None), enabled: true }; + let p = CliProgress { bar: Arc::default(), enabled: true }; let bar = || p.bar.lock().unwrap().as_ref().map(|(b, _)| (b.prefix(), b.position(), b.message())); let item = |path: &str| Event::Item { phase: Phase::Scan, path: path.into(), bytes: 10 }; p.on_event(&Event::PhaseStart { phase: Phase::Scan, items: 3, bytes: 0 }); @@ -69,10 +69,14 @@ fn progress_bars_count_what_their_phase_counts() { // Nothing to count: a spinner. p.on_event(&Event::PhaseStart { phase: Phase::Verify, items: 0, bytes: 0 }); assert_eq!(bar().map(|(prefix, position, _)| (prefix, position)), Some(("Verifying".into(), 0))); + // A question is asked with the bar out of the way, which is drawn again after. + assert_eq!(p.drawing().suspend(|| "asked"), "asked"); + assert!(bar().is_some()); p.on_event(&Event::PhaseEnd { phase: Phase::Verify }); + assert_eq!(p.drawing().suspend(|| "asked"), "asked"); // `--json`, `--quiet` or no terminal: no bars. - let off = CliProgress { bar: Mutex::new(None), enabled: false }; + let off = CliProgress { bar: Arc::default(), enabled: false }; off.on_event(&Event::PhaseStart { phase: Phase::Scan, items: 3, bytes: 0 }); off.on_event(&Event::Warning { message: "said anyway".into() }); assert!(off.bar.lock().unwrap().is_none()); @@ -242,7 +246,7 @@ fn progress_bars_follow_their_events(tc: hegel::TestCase) { Phase::Analyze, Phase::Upload, ]; - let p = CliProgress { bar: Mutex::new(None), enabled: true }; + let p = CliProgress { bar: Arc::default(), enabled: true }; // (label, counts items, position, message), as the bar should have them. let mut model: Option<(&str, bool, u64, String)> = None; for _ in 0..tc.draw(gs::integers::().max_value(20)) { diff --git a/apps/locallibrary/src/plugins.rs b/apps/locallibrary/src/plugins.rs index 9ace6fa5b..c1ccf22ce 100644 --- a/apps/locallibrary/src/plugins.rs +++ b/apps/locallibrary/src/plugins.rs @@ -20,7 +20,7 @@ use clap::{CommandFactory, ValueEnum}; use indexmap::IndexMap; use locallibrary::api::RpcFailure; use locallibrary::mix::report::ShownField; -use locallibrary::plugin::{Access, Launch, Log, Options, PluginSpec, Plugins, Reads}; +use locallibrary::plugin::{Access, AccessQuestion, AskAccess, Launch, Log, Options, PluginSpec, Plugins, Reads}; use locallibrary::progress::Ctx; use locallibrary::{Error, Result}; use loclib_plugin::{AccountType, Capability, ExportFrom, Manifest, SettingType}; @@ -147,7 +147,8 @@ fn load() -> Result { Some(log) => log(id, line), None => stderr(id, line), }); - let mut options = Options { notices: events::notices(log.clone()), log, ..Options::default() }; + let ask: Arc = Arc::new(Asking); + let mut options = Options { notices: events::notices(log.clone()), log, ask: Some(ask), ..Options::default() }; // The CLI's tests: no plugin they run goes online (to MusicBrainz, say). #[cfg(debug_assertions)] { @@ -170,6 +171,36 @@ pub fn log_to(log: Option) { *LOG.lock().unwrap_or_else(PoisonError::into_inner) = log; } +/// Who asks the user whether a plugin may use more of an account it has access to: nobody until +/// a front end that can ask says so (see [`ask_to`]). +static ASK: Mutex>> = Mutex::new(None); + +/// Has `ask` ask the user whether a plugin may use more of an account it has access to (the +/// terminal a command runs in, the player), or with `None` nobody: the plugin is told how to give +/// it instead. +pub fn ask_to(ask: Option>) { + *ASK.lock().unwrap_or_else(PoisonError::into_inner) = ask; +} + +/// Asks through whoever [`ask_to`] said, when anyone. +struct Asking; + +impl Asking { + fn asker() -> Option> { + ASK.lock().unwrap_or_else(PoisonError::into_inner).clone() + } +} + +impl AskAccess for Asking { + fn allow(&self, question: &AccessQuestion) -> Option { + Asking::asker()?.allow(question) + } + + fn sign_in(&self, url: &str) { + Asking::asker().into_iter().for_each(|ask| ask.sign_in(url)); + } +} + /// The locallibrary binary, which serves the plugins. #[cfg(not(test))] fn exe() -> std::io::Result { diff --git a/apps/locallibrary/src/tui/app.rs b/apps/locallibrary/src/tui/app.rs index 2fa0d7426..0e9624249 100644 --- a/apps/locallibrary/src/tui/app.rs +++ b/apps/locallibrary/src/tui/app.rs @@ -759,6 +759,8 @@ pub enum Msg { Notice(locallibrary::plugin::PluginNotice), /// What plugins suggested, as it's kept now: to review from the palette. Suggested(Vec), + /// A plugin asks whether it may use more of an account, waiting for the answer here. + AskAccess(Box, Sender), /// The playing track's analysis, artwork and matches, for the play with this id. TrackInfo(u64, Box), Details(Box
), @@ -1868,6 +1870,7 @@ impl App { Msg::Log(line) => self.remember(line, Level::Info), Msg::Notice(notice) => self.told(notice), Msg::Suggested(kept) => self.suggested = kept, + Msg::AskAccess(question, reply) => self.ask_access(&question, reply), Msg::TrackInfo(id, info) => { let upcoming = self.upcoming.as_mut().map(|u| &mut u.now); if let Some(now) = self.now.as_mut().into_iter().chain(upcoming).find(|n| n.id == id) { diff --git a/apps/locallibrary/src/tui/form.rs b/apps/locallibrary/src/tui/form.rs index 166387f98..0c546a456 100644 --- a/apps/locallibrary/src/tui/form.rs +++ b/apps/locallibrary/src/tui/form.rs @@ -59,6 +59,9 @@ pub enum For { Fetch { kind: loclib_plugin::ProviderKind, root: std::path::PathBuf, tracks: Vec }, /// Which account of each type the plugin with this id asks for it uses, of those offered. Access(&'static str, Vec), + /// Whether a plugin may use more of an account, the answer going back on this (see + /// [`super::plugins::access`]): closed, it's never answered. + AskAccess(std::sync::mpsc::Sender), } pub struct Field { @@ -215,6 +218,7 @@ impl App { } For::Fetch { kind, root, tracks } => self.fetch(kind, root, tracks, params), For::Access(plugin, choices) => self.save_access(plugin, choices, params), + For::AskAccess(reply) => super::plugins::access::answer(&reply, ¶ms), }; } Err((i, why)) => (f.selected, f.error) = (i, Some(why)), diff --git a/apps/locallibrary/src/tui/mod.rs b/apps/locallibrary/src/tui/mod.rs index 4fc0d3ca9..212a13d98 100644 --- a/apps/locallibrary/src/tui/mod.rs +++ b/apps/locallibrary/src/tui/mod.rs @@ -51,6 +51,7 @@ pub fn run(store_root: PathBuf, drive: Option) -> Result<()> { // Plugins' logs would draw over the screen: they go to the messages (`!`) while it runs. crate::plugins::log_to(Some(app.plugin_log())); crate::plugins::events::notices_to(Some(app.plugin_notices())); + crate::plugins::ask_to(Some(app.plugin_asker())); app.start_plugins(); // (Not `ratatui::init`, whose panic hook would put the screen back for a worker's panic.) enable_raw_mode()?; @@ -67,6 +68,7 @@ pub fn run(store_root: PathBuf, drive: Option) -> Result<()> { stderr::restore(); crate::plugins::log_to(None); crate::plugins::events::notices_to(None); + crate::plugins::ask_to(None); SIGNAL.store(IDLE, Ordering::Relaxed); // The loop also ends when the terminal goes away mid-write: the write still finishes. app.finish_write(); diff --git a/apps/locallibrary/src/tui/plugins.rs b/apps/locallibrary/src/tui/plugins.rs index 8d2c34235..6b403f802 100644 --- a/apps/locallibrary/src/tui/plugins.rs +++ b/apps/locallibrary/src/tui/plugins.rs @@ -5,13 +5,14 @@ //! confirmed and applied as any edit is, and undone with `u`. A question either asks first is a //! list to choose from (several, one after another), after which it runs again with the answers. //! Each plugin's settings are a form, kept in the store, with a switch for each of its services, -//! and its access to the store's accounts is beside them, as is signing in to and out of those. +//! and its access to the store's accounts is beside them, as is signing in to and out of those; a +//! plugin wanting more of an account it has access to asks in a form of its own. //! What the other families of hooks add (cover art and tags from providers, analyzers, checks, //! exports, imports, what plugins tell the user and suggest) is in a module each. Nothing here tells one plugin from another: it's //! all drawn from their manifests, through the host the command line uses (see //! [`crate::plugins`]). -mod access; +pub(super) mod access; mod accounts; mod analyzers; pub(super) mod checks; diff --git a/apps/locallibrary/src/tui/plugins/access.rs b/apps/locallibrary/src/tui/plugins/access.rs index 0668114f7..0706da879 100644 --- a/apps/locallibrary/src/tui/plugins/access.rs +++ b/apps/locallibrary/src/tui/plugins/access.rs @@ -2,15 +2,20 @@ //! palette opens a form with a choice for each account type the plugin asks for, saying what it //! needs it for: the accounts of that type signed in, and signing in to another (through each //! account plugin providing the type, when several do). What's chosen becomes its default, in -//! the background. +//! the background. A plugin wanting more of an account it has access to asks in a form too +//! (see [`App::ask_access`]), its hook waiting for the answer. + +use std::sync::Arc; +use std::sync::mpsc::{Sender, channel}; use locallibrary::Store; -use locallibrary::plugin::{AccountId, Plugins}; +use locallibrary::plugin::{AccessQuestion, AccountId, AskAccess, Plugins}; use loclib_plugin::Manifest; use serde_json::{Map, Value, json}; -use super::accounts::{BROWSER_WAIT, open}; -use crate::tui::app::{App, Level}; +use super::accounts::{BROWSER_WAIT, open, unopened}; +use crate::account_cmd::asking::wants; +use crate::tui::app::{App, Level, Msg}; use crate::tui::edits::Modal; use crate::tui::form::{For, Form}; use crate::tui::keymap::Act; @@ -139,4 +144,52 @@ impl App { Ok(Box::new(move |app: &mut App| app.toast(said, Level::Info))) }); } + + /// The form asking whether a plugin may use more of an account it has access to (`q`), its + /// answer going back on `reply`. Closed, it's never answered: the plugin is told how to give + /// it instead. + pub(in crate::tui) fn ask_access(&mut self, q: &AccessQuestion, reply: Sender) { + let allow = if q.sign_in { SIGN_IN } else { ALLOW }; + let answer = json!({ "type": "string", "enum": [allow, DECLINE], "description": wants(q) }); + let schema = json!({ "type": "object", "properties": { ANSWER: answer }, "required": [ANSWER] }); + let form = Form::with(format!("Access for {}", q.plugin), Some(schema), &Map::new(), For::AskAccess(reply)); + self.modal = Some(Modal::Form(form)); + } + + /// Where plugins ask whether they may use more of an account while the player runs: the + /// form ([`App::ask_access`]), each waiting for its answer; a sign-in it takes opens the + /// browser, saying where to go when it can't. + pub(in crate::tui) fn plugin_asker(&self) -> Arc { + Arc::new(Asking(self.sender())) + } +} + +/// The form's field, and its choices: allowing what a plugin asks (signing in again to, when the +/// account wasn't granted it), or not. +const ANSWER: &str = "answer"; +const ALLOW: &str = "allow"; +const SIGN_IN: &str = "sign in again to allow"; +const DECLINE: &str = "don't allow"; + +/// The answer `given` in the form asking whether a plugin may use more of an account, sent back +/// on `reply`. +pub(in crate::tui) fn answer(reply: &Sender, given: &Value) { + let _ = reply.send(given[ANSWER] != DECLINE); +} + +/// Asks in the player (see [`App::plugin_asker`]). +struct Asking(Sender); + +impl AskAccess for Asking { + fn allow(&self, q: &AccessQuestion) -> Option { + let (reply, answered) = channel(); + self.0.send(Msg::AskAccess(Box::new(q.clone()), reply)).ok()?; + answered.recv().ok() + } + + fn sign_in(&self, url: &str) { + if let Some(link) = unopened(url) { + let _ = self.0.send(Msg::Note(link, Level::Info)); + } + } } diff --git a/apps/locallibrary/src/tui/plugins/accounts.rs b/apps/locallibrary/src/tui/plugins/accounts.rs index 4ac65533b..a4ce80ecb 100644 --- a/apps/locallibrary/src/tui/plugins/accounts.rs +++ b/apps/locallibrary/src/tui/plugins/accounts.rs @@ -50,12 +50,16 @@ fn mark(a: &Account) -> u64 { /// Opens `url` in the browser to sign in, or shows it when it can't be. pub(super) fn open(url: &str, show: &dyn Fn(crate::tui::edits::Done)) { - if locallibrary::opener::open_url(url).is_err() { - let link = format!("open this in your browser to sign in: {url}"); + if let Some(link) = unopened(url) { show(Box::new(move |app: &mut App| app.toast(link, Level::Info))); } } +/// Opens `url` in the browser to sign in: what to show instead, when it can't be. +pub(super) fn unopened(url: &str) -> Option { + locallibrary::opener::open_url(url).is_err().then(|| format!("open this in your browser to sign in: {url}")) +} + impl App { /// Sign in to an account of type `kind` through the account plugin `with` in the background: /// the browser opens on the service's sign-in page (the link is shown when it can't), and diff --git a/apps/locallibrary/src/tui/plugins/tests/accounts.rs b/apps/locallibrary/src/tui/plugins/tests/accounts.rs index 340c12112..98d1e3f90 100644 --- a/apps/locallibrary/src/tui/plugins/tests/accounts.rs +++ b/apps/locallibrary/src/tui/plugins/tests/accounts.rs @@ -3,6 +3,7 @@ use hegel::TestCase; use hegel::generators as gs; +use locallibrary::plugin::{Access, AccessQuestion, AccountId}; use ratatui::crossterm::event::KeyCode; use super::super::super::app::testing::{blank, key, last_message, plugins_once, press, wait}; @@ -164,3 +165,54 @@ fn signing_in_and_out_from_the_palette_leaves_the_accounts_as_the_steps_say(tc: assert_eq!(subjects(&app), if signed_in { vec!["a".to_string()] } else { Vec::new() }); } } + +/// What a plugin asks of the mock account `a`: `write`, signing in again to it or not. +fn wanting(sign_in: bool) -> AccessQuestion { + let account = AccountId { plugin: "mock-accounts".into(), subject: "a".into() }; + let (kind, name, scopes) = ("mock".to_string(), "a@mock.example".to_string(), vec!["read".to_string()]); + let access = Access { + id: "0a1b2c3d".into(), + plugin: "echo".into(), + kind, + account, + name, + scopes, + default: true, + declined: vec![], + }; + let write = vec!["write".to_string()]; + let why = vec!["More".to_string()]; + AccessQuestion { + plugin: "Echo".into(), + access, + title: "Mock".into(), + scopes: write.clone(), + titles: write, + why, + sign_in, + } +} + +/// For a plugin wanting more of an account, signing in again to or not, the player's asker shows +/// a form saying what it wants and why, and waits for its answer: enter allows it (or signs in +/// again to), "don't allow" doesn't, and esc leaves it unanswered; a sign-in it takes opens the +/// browser, or says where to go when it can't. +#[hegel::test(test_cases = 8)] +fn a_plugin_wanting_more_of_an_account_asks_in_a_form(tc: TestCase) { + let (mut app, _store) = blank(Screen::Library); + let ask = app.plugin_asker(); + let sign_in = tc.draw(gs::booleans()); + let (keys, answered) = tc.draw(gs::sampled_from(vec![("\n", Some(true)), (" \n", Some(false)), ("\u{1b}", None)])); + let asking = std::thread::spawn(move || ask.allow(&wanting(sign_in))); + wait(&mut app, |app| app.modal.is_some()); + let s = screen(&mut app, 120, 30); + assert!(s.contains("Access for Echo"), "{s}"); + assert!(s.contains("Echo wants to write with the Mock account a@mock.example (More)"), "{s}"); + assert!(s.contains(if sign_in { "sign in again to allow" } else { "allow" }), "{s}"); + press(&mut app, keys); + assert!(app.modal.is_none(), "{}", last_message(&app)); + assert_eq!(asking.join().unwrap(), answered); + + app.plugin_asker().sign_in("http://127.0.0.1:9/authorize?state=x"); + wait(&mut app, |app| last_message(app).contains("open this in your browser to sign in")); +} diff --git a/apps/locallibrary/src/tui/ui.rs b/apps/locallibrary/src/tui/ui.rs index a25a77a73..8ca774ccf 100644 --- a/apps/locallibrary/src/tui/ui.rs +++ b/apps/locallibrary/src/tui/ui.rs @@ -2652,6 +2652,7 @@ fn draw_form(f: &mut Frame, app: &mut App, area: Rect) { | super::form::For::ProviderSettings | super::form::For::CheckSettings | super::form::For::Access(..) => "save", + super::form::For::AskAccess(_) => "answer", }; let b = block(title(&form.title, true), true) .title_bottom(Line::styled(format!(" ↑↓ field · {keys}enter {then} · esc cancel "), faint()).right_aligned()) diff --git a/apps/locallibrary/tests/it/plugins/accounts/cli.rs b/apps/locallibrary/tests/it/plugins/accounts/cli.rs index 987a60ff8..12452b11a 100644 --- a/apps/locallibrary/tests/it/plugins/accounts/cli.rs +++ b/apps/locallibrary/tests/it/plugins/accounts/cli.rs @@ -381,6 +381,81 @@ fn a_destination_without_access_asks_which_account_in_a_terminal() { assert!(out.starts_with("✓ Echo has access to the Mock account b@mock.example\n"), "{out}"); } +/// Where [`a_plugin_wanting_more_of_an_account_asks_only_in_a_terminal`] runs echo's `ensure`. +#[derive(Debug, Clone, Copy)] +enum Asked { + /// In a terminal, answering yes or no. + Terminal(bool), + Json, + Yes, + /// Without a terminal. + Piped, +} + +/// For echo given access to a mock account for `read` alone, the service having granted `write` +/// too or not: its `ensure` asking for `write` asks in a terminal whether echo may (to sign in +/// again first when the account wasn't granted it), and answers as the user says, a no remembered +/// so it isn't asked again; with `--json`, `--yes` or no terminal nothing's asked, and it fails +/// offering to give echo `write` or sign in again allowing it. +#[cfg(unix)] +#[hegel::test(test_cases = 6, suppress_health_check = [hegel::HealthCheck::TooSlow])] +fn a_plugin_wanting_more_of_an_account_asks_only_in_a_terminal(tc: TestCase) { + let m = Mock::new(); + let granted = tc.draw(gs::booleans()); + let grants = + |scopes: &[&str]| m.state.lock().unwrap().grants = Some(scopes.iter().map(|s| s.to_string()).collect()); + grants(if granted { &["read", "write"] } else { &["read"] }); + assert_eq!(m.sign_in("a", &["--for", "echo"]).0, 0); + grants(&["read", "write"]); + let asked = tc.draw(gs::sampled_from(vec![ + Asked::Terminal(true), + Asked::Terminal(false), + Asked::Json, + Asked::Yes, + Asked::Piped, + ])); + let ensure = ["echo", "ensure", "--params", r#"{"type": "mock", "scopes": ["write"]}"#]; + let (code, out, err) = match asked { + Asked::Terminal(yes) => m.typed(&ensure, if yes { "y\n" } else { "n\n" }, "a"), + Asked::Json => m.run(&[&["--json"][..], &ensure].concat()), + Asked::Yes => m.run(&[&["--yes"][..], &ensure].concat()), + Asked::Piped => m.run(&ensure), + }; + let said = format!("{asked:?}, granted {granted}: {out}{err}"); + let Asked::Terminal(yes) = asked else { + assert!(!err.contains("? Echo wants"), "asked: {said}"); + assert_eq!(code, 1, "{said}"); + let more = match granted { + true => "Allow write with a@mock.example: locallibrary plugin access echo --give a --scope write", + false => "Sign in again as a@mock.example, allowing write: locallibrary account mock:a sign-in --with", + }; + match asked { + Asked::Json => { + let actions = &serde_json::from_str::(&out).unwrap()["error"]["actions"]; + let label = more.split(':').next().unwrap(); + assert_eq!(actions[0]["label"], label, "{said}"); + } + _ => assert!(err.contains(&format!("hint: {more}")), "{said}"), + } + return; + }; + assert_eq!(code, 0, "{said}"); + let allow = if granted { "Allow it?" } else { "Sign in again to allow it?" }; + let question = format!("? Echo wants to write with the Mock account a@mock.example (More). {allow} [y/N]"); + assert!(err.contains(&question), "{said}"); + let answer: Value = serde_json::from_str(&out).unwrap(); + let (granting, declining) = if yes { (json!(["write"]), json!([])) } else { (json!([]), json!(["write"])) }; + assert_eq!((&answer["granted"], &answer["declined"]), (&granting, &declining), "{said}"); + let (_, listed, _) = m.run(&["plugin", "access", "echo"]); + let has = if yes { "read, write" } else { "read" }; + assert!(listed.contains(&format!("a@mock.example (default) · {has}")), "{listed}"); + // Asked again, a no stands, unasked; a yes is answered as it is. + let (code, out, err) = m.run(&ensure); + assert_eq!(code, 0, "{err}"); + assert_eq!(serde_json::from_str::(&out).unwrap(), answer, "{err}"); + assert!(!err.contains("? Echo wants"), "{err}"); +} + /// A second account plugin's id, providing the `mock` type too. const TOO: &str = "mock-accounts-too"; From af4b3d921879441f8c7176c22372cc3d7624966f Mon Sep 17 00:00:00 2001 From: botandbang <337765928+botandbang@users.noreply.github.com> Date: Fri, 30 Oct 2026 18:00:14 -0400 Subject: [PATCH 4/6] feat(locallibrary): say when each account signs itself out, and warn in its last 2 days --- apps/locallibrary/CHANGELOG.md | 5 ++ apps/locallibrary/docs/plugins.md | 5 +- apps/locallibrary/docs/rpc/locallibrary.d.ts | 18 +++++ apps/locallibrary/docs/rpc/schema.json | 24 +++++++ apps/locallibrary/docs/sidecar.md | 10 +-- apps/locallibrary/src/account_cmd.rs | 4 +- apps/locallibrary/src/account_cmd/access.rs | 47 ++++++++++--- apps/locallibrary/src/account_cmd/accounts.rs | 32 +++++++-- .../src/account_cmd/atmosphere.rs | 21 ++++-- apps/locallibrary/src/publish_cmd.rs | 8 +++ apps/locallibrary/src/tui/account.rs | 12 ++-- apps/locallibrary/src/tui/account/tests.rs | 6 +- apps/locallibrary/src/tui/mod.rs | 1 + apps/locallibrary/src/tui/plugins/accounts.rs | 20 +++++- .../src/tui/plugins/tests/accounts.rs | 24 +++++++ apps/locallibrary/tests/it/account.rs | 8 ++- .../tests/it/plugins/accounts/cli.rs | 69 ++++++++++++++++-- apps/locallibrary/tests/it/publish.rs | 11 +++ crates/locallibrary/src/atmosphere/mod.rs | 6 ++ crates/locallibrary/src/plugin.rs | 2 +- crates/locallibrary/src/plugin/accounts.rs | 70 +++++++++++++++---- .../locallibrary/src/plugin/accounts/kept.rs | 18 ++++- .../src/plugin/accounts/shared.rs | 17 ++++- .../locallibrary/src/plugin/accounts/tests.rs | 54 ++++++++++++++ .../locallibrary/tests/it/props_atmosphere.rs | 6 +- .../locallibrary/tests/support/mock_oauth.rs | 5 ++ 26 files changed, 438 insertions(+), 65 deletions(-) diff --git a/apps/locallibrary/CHANGELOG.md b/apps/locallibrary/CHANGELOG.md index 3e99bc797..750a0ce50 100644 --- a/apps/locallibrary/CHANGELOG.md +++ b/apps/locallibrary/CHANGELOG.md @@ -443,6 +443,11 @@ Windows builds are experimental and untested on hardware (see Platforms in the R you declined, never a token. The terminal and the player ask you first (signing in again when the account wasn't granted them), and a no is remembered until `plugin access --give` gives the scope; with `--json`, in `watch` and in the sidecar it fails saying how to give it. + `account` says when each account signs itself out (its token expiring unrefreshed, or its + refresh token running out when the service says), and in its last two days it's a warning there, + before a destination uses it (offering to sign in again) and as the player starts; the + Atmosphere account's "sign in again by" turns into a warning then too, and publishing offers to + sign in again first. - **The library is the unit.** Each drive holding a library is a clone of it, and its name doesn't matter much: a library's name works where a drive does (but in commands about one particular drive) and means its lead clone, the newest of its drives plugged in (the primary when they're diff --git a/apps/locallibrary/docs/plugins.md b/apps/locallibrary/docs/plugins.md index 4c11677a2..c10816f97 100644 --- a/apps/locallibrary/docs/plugins.md +++ b/apps/locallibrary/docs/plugins.md @@ -247,7 +247,10 @@ allowing it signs in again in the browser. A no is remembered: it isn't asked ag how to give it the scope. The tokens are kept in the system's keychain, as secret settings are (see [Settings](#settings)), -and refreshed a minute before they expire. A plugin never sees them: a request it makes names the +and refreshed a minute before they expire. An account that isn't refreshed signs itself out as its +token expires, and one whose service says how long its refresh token lasts signs itself out then: +`locallibrary account` says when (`signs_out` with `--json`), and in its last two days warns, as a +destination using it does (offering to sign in again in a terminal) and the player as it starts. A plugin never sees them: a request it makes names the 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 diff --git a/apps/locallibrary/docs/rpc/locallibrary.d.ts b/apps/locallibrary/docs/rpc/locallibrary.d.ts index e3833e3b3..0eb5b627c 100644 --- a/apps/locallibrary/docs/rpc/locallibrary.d.ts +++ b/apps/locallibrary/docs/rpc/locallibrary.d.ts @@ -7861,6 +7861,12 @@ export type StoreAccount = { expires?: string | null; /** It's refreshed without signing in again. */ refreshes: boolean; + /** + * When it signs itself out, when it does: as its access token expires when it isn't + * refreshed, else as its refresh token runs out, when the service said. Front ends warn + * within [`crate::plugin::WARN_DAYS`] of it. + */ + signs_out?: string | null; /** Its tokens are kept (the keychain may have lost them). */ signed_in: boolean; }; @@ -7922,6 +7928,12 @@ export type AccountSignedIn = { expires?: string | null; /** It's refreshed without signing in again. */ refreshes: boolean; + /** + * When it signs itself out, when it does: as its access token expires when it isn't + * refreshed, else as its refresh token runs out, when the service said. Front ends warn + * within [`crate::plugin::WARN_DAYS`] of it. + */ + signs_out?: string | null; /** Its tokens are kept (the keychain may have lost them). */ signed_in: boolean; /** @@ -7956,6 +7968,12 @@ export type AccountSignedOut = { expires?: string | null; /** It's refreshed without signing in again. */ refreshes: boolean; + /** + * When it signs itself out, when it does: as its access token expires when it isn't + * refreshed, else as its refresh token runs out, when the service said. Front ends warn + * within [`crate::plugin::WARN_DAYS`] of it. + */ + signs_out?: string | null; /** Its tokens are kept (the keychain may have lost them). */ signed_in: boolean; /** The service was told to forget its tokens (it has a revocation endpoint, which answered). */ diff --git a/apps/locallibrary/docs/rpc/schema.json b/apps/locallibrary/docs/rpc/schema.json index 9349a2089..990ee184f 100644 --- a/apps/locallibrary/docs/rpc/schema.json +++ b/apps/locallibrary/docs/rpc/schema.json @@ -30399,6 +30399,14 @@ "type": "boolean", "description": "It's refreshed without signing in again." }, + "signs_out": { + "type": [ + "string", + "null" + ], + "format": "date-time", + "description": "When it signs itself out, when it does: as its access token expires when it isn't\nrefreshed, else as its refresh token runs out, when the service said. Front ends warn\nwithin [`crate::plugin::WARN_DAYS`] of it." + }, "signed_in": { "type": "boolean", "description": "Its tokens are kept (the keychain may have lost them)." @@ -30536,6 +30544,14 @@ "type": "boolean", "description": "It's refreshed without signing in again." }, + "signs_out": { + "type": [ + "string", + "null" + ], + "format": "date-time", + "description": "When it signs itself out, when it does: as its access token expires when it isn't\nrefreshed, else as its refresh token runs out, when the service said. Front ends warn\nwithin [`crate::plugin::WARN_DAYS`] of it." + }, "signed_in": { "type": "boolean", "description": "Its tokens are kept (the keychain may have lost them)." @@ -30614,6 +30630,14 @@ "type": "boolean", "description": "It's refreshed without signing in again." }, + "signs_out": { + "type": [ + "string", + "null" + ], + "format": "date-time", + "description": "When it signs itself out, when it does: as its access token expires when it isn't\nrefreshed, else as its refresh token runs out, when the service said. Front ends warn\nwithin [`crate::plugin::WARN_DAYS`] of it." + }, "signed_in": { "type": "boolean", "description": "Its tokens are kept (the keychain may have lost them)." diff --git a/apps/locallibrary/docs/sidecar.md b/apps/locallibrary/docs/sidecar.md index 229ee7f90..6f9f1b32a 100644 --- a/apps/locallibrary/docs/sidecar.md +++ b/apps/locallibrary/docs/sidecar.md @@ -359,10 +359,12 @@ Native bindings (napi-rs) can wrap the same `api` functions later without changi The accounts a store keeps are signed in to through account plugins, and plugins use them through the access they're given (see [plugins.md](plugins.md#accounts)). No answer holds a token. `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, declined?}`, `declined` -the scopes the service didn't grant at the last sign-in for that plugin). +scopes, expires?, refreshes, signs_out?, signed_in}`, `plugin` the account plugin it was signed in +through, `signs_out` when it signs itself out: as `expires` when it isn't refreshed, else as its +refresh token runs out when the service said), 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, declined?}`, `declined` the scopes the user declined when the plugin asked +for them, or the service didn't grant at a sign-in for that plugin, until they're given again). `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` diff --git a/apps/locallibrary/src/account_cmd.rs b/apps/locallibrary/src/account_cmd.rs index 168522c9f..ad7cbb41f 100644 --- a/apps/locallibrary/src/account_cmd.rs +++ b/apps/locallibrary/src/account_cmd.rs @@ -20,8 +20,10 @@ use locallibrary::plugin::Accounts; use locallibrary::progress::Ctx; use serde::Serialize; +#[cfg(feature = "tui")] +pub use accounts::signing_out; #[cfg(feature = "account")] -pub use atmosphere::sign_in_again; +pub use atmosphere::{again, sign_in_again}; use crate::output::Output; use crate::plugins; diff --git a/apps/locallibrary/src/account_cmd/access.rs b/apps/locallibrary/src/account_cmd/access.rs index 9506ea255..31b16a48e 100644 --- a/apps/locallibrary/src/account_cmd/access.rs +++ b/apps/locallibrary/src/account_cmd/access.rs @@ -15,10 +15,10 @@ use locallibrary::progress::Ctx; use loclib_plugin::{AccountNeed, Choice, ChoiceItem}; use serde_json::json; -use super::accounts::{account_type, called, come_back, declined, find, named, titles}; +use super::accounts::{account_type, called, come_back, declined, find, named, signing_out, titles}; use crate::browser::BrowserArgs; use crate::jobs_cmd::usage; -use crate::output::{Output, dim, heading, ok}; +use crate::output::{Output, dim, heading, ok, warn}; use crate::plugin_cmd::choose::{answer, interactive}; use crate::plugins; @@ -111,21 +111,50 @@ fn list(plugins: &Plugins, plugin: &str, needs: &[AccountNeed], accesses: &[Acce /// 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. +/// asked elsewhere: the destination is refused, saying how to resolve it. Then each account it +/// uses that signs itself out within `WARN_DAYS` is warned about, and signing in again offered +/// in a terminal (see [`signing_out_soon`]). pub fn ask(spec: &str, store: &Store, out: Output, ctx: &Ctx<'_>) -> Result<()> { - if !interactive(out) || out.yes { - return Ok(()); - } - let (plugin, _) = destination_spec(spec)?; + // A spec that names no destination is refused as it's used. + let Ok((plugin, _)) = destination_spec(spec) else { return Ok(()) }; let plugins = plugins::host()?; + let asking = interactive(out) && !out.yes; // 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)))) + while let Some(u) = plugins + .unready(plugin, store)? + .into_iter() + .find(|u| asking && !asked.contains(&(u.kind().to_string(), discriminant(u)))) { asked.push((u.kind().to_string(), discriminant(&u))); resolve(plugins, plugin, u, store, out, ctx)?; } + signing_out_soon(plugins, plugin, store, asking, out, ctx) +} + +/// Warns of each account `plugin`'s default accesses use that signs itself out within `WARN_DAYS`, +/// and when `asking`, offers to sign in again as it. +fn signing_out_soon( + plugins: &Plugins, + plugin: &str, + store: &Store, + asking: bool, + out: Output, + ctx: &Ctx<'_>, +) -> Result<()> { + // Without asking the keychain: one that lost its tokens is signed out, which `ask` resolves. + let accounts = plugins.kept_accounts(store)?; + for x in plugins.accesses(store, Some(plugin), None)?.into_iter().filter(|x| x.default) { + let used = accounts.iter().find(|a| a.plugin == x.account.plugin && a.subject == x.account.subject); + let Some((a, at)) = used.filter(|a| a.due_soon()).and_then(|a| Some((a, a.signs_out?))) else { continue }; + warn(format!("the {} {}", called(plugins, a), signing_out(at))); + if !asking || !out.confirm("Sign in again now?") { + continue; + } + let start = plugins.begin_account_sign_in(&a.kind, &a.plugin, Some(plugin), Some(&a.subject), &[], store)?; + let done = come_back(plugins, &start, BrowserArgs::default(), out, ctx)?; + ok(format!("Signed in to the {}", called(plugins, &done.account))); + } Ok(()) } diff --git a/apps/locallibrary/src/account_cmd/accounts.rs b/apps/locallibrary/src/account_cmd/accounts.rs index bb25b18d2..82e0fae19 100644 --- a/apps/locallibrary/src/account_cmd/accounts.rs +++ b/apps/locallibrary/src/account_cmd/accounts.rs @@ -181,8 +181,20 @@ pub(super) fn declined(plugins: &Plugins, d: &AccountSignedIn) { 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 -/// when, what it was granted, and which plugins have access to it. +/// When an account that signs itself out at `at` does: on its day and in how many days, at its +/// time when that's today, or that it has. +pub fn signing_out(at: chrono::DateTime) -> String { + let left = at - chrono::Utc::now(); + match left.num_days() { + _ if left <= chrono::Duration::zero() => format!("signed itself out at {}", local_time(at, "%Y-%m-%d %H:%M")), + 0 => format!("signs itself out at {} (today)", local_time(at, "%Y-%m-%d %H:%M")), + days => format!("signs itself out on {} ({days} days)", local_time(at, "%Y-%m-%d")), + } +} + +/// Each of `all`'s accounts: its type, name and account plugin, whether it's signed in and when +/// it signs itself out (warning in its last days), what it was granted, and which plugins have +/// access to it. pub(super) fn list(plugins: &Plugins, all: &Accounts) { for a in &all.accounts { let accesses: Vec<&Access> = @@ -193,16 +205,22 @@ 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, 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(), + let again = format!("`locallibrary account {}:{} sign-in` signs in again", a.kind, a.subject); + let mut state = match (a.signed_in, a.refreshes) { + (false, _) => format!("signed out: {again}"), + (true, true) => "signed in, refreshed on its own".into(), + (true, false) => "signed in".into(), }; + if let Some(at) = a.signs_out.filter(|_| a.signed_in) { + state = format!("{state} · {}", signing_out(at)); + } match a.scopes.is_empty() { true => say!(" {}", dim(state)), false => say!(" {}", dim(format!("{state} · allowed: {}", a.scopes.join(" ")))), } + if let Some(at) = a.signs_out.filter(|_| a.due_soon()) { + warn(format!("it {}: {again}", signing_out(at))); + } if !accesses.is_empty() { let who: Vec = accesses .iter() diff --git a/apps/locallibrary/src/account_cmd/atmosphere.rs b/apps/locallibrary/src/account_cmd/atmosphere.rs index 135185092..f27466a23 100644 --- a/apps/locallibrary/src/account_cmd/atmosphere.rs +++ b/apps/locallibrary/src/account_cmd/atmosphere.rs @@ -277,17 +277,26 @@ fn render_account(a: &AccountInfo) { render_details(a); } -fn render_details(a: &AccountInfo) { - let host = |url: &str| url.split("://").nth(1).unwrap_or(url).trim_end_matches('/').to_string(); - say!(" {}", dim(format!("{} · data on {} · signed in at {}", a.did, host(&a.pds), host(&a.auth_server)))); - let again = if a.days_left() >= 1 { +/// When `a` is to sign in again: by its day and in how many days, by its time when that's today, +/// or that its session has likely ended. +pub fn again(a: &AccountInfo) -> String { + if a.days_left() >= 1 { format!("sign in again by {} ({} days)", local_time(a.sign_in_again_by, "%Y-%m-%d"), a.days_left()) } else if !a.expired() { format!("sign in again by {} (today)", local_time(a.sign_in_again_by, "%Y-%m-%d %H:%M")) } else { "the session has likely ended; sign in again".to_string() - }; - say!(" signed in {} · {again}", local_time(a.signed_in_at, "%Y-%m-%d")); + } +} + +fn render_details(a: &AccountInfo) { + let host = |url: &str| url.split("://").nth(1).unwrap_or(url).trim_end_matches('/').to_string(); + say!(" {}", dim(format!("{} · data on {} · signed in at {}", a.did, host(&a.pds), host(&a.auth_server)))); + let line = format!("signed in {} · {}", local_time(a.signed_in_at, "%Y-%m-%d"), again(a)); + match a.due_soon() { + true => warn(line), + false => say!(" {line}"), + } let publishes = a.scopes.iter().any(|s| atmosphere::PUBLISH_SCOPES.contains(&s.as_str())); let reports = a.scopes.iter().any(|s| s == atmosphere::REPORT_SCOPE); let known = |s: &String| { diff --git a/apps/locallibrary/src/publish_cmd.rs b/apps/locallibrary/src/publish_cmd.rs index f94633891..0b60ef0f7 100644 --- a/apps/locallibrary/src/publish_cmd.rs +++ b/apps/locallibrary/src/publish_cmd.rs @@ -95,6 +95,14 @@ fn publish_flow( if p.noop { return Ok(unchanged(out, &p, what)); } + // In its last days, signing in again is offered first. + if account.due_soon() { + warn(format!("{}: {}", account.name(), crate::account_cmd::again(&account))); + if !out.yes && std::io::stdin().is_terminal() && out.confirm("Sign in again now?") { + crate::account_cmd::sign_in_again(root, &account, &[], "publishing", browser, out, ctx)?; + p = plan()?; + } + } if p.missing_scope.is_some() { let signed_in = match sign_in_to_publish(Run { root, dry_run, browser, out, ctx }, &account)? { Ok(signed_in) => signed_in, diff --git a/apps/locallibrary/src/tui/account.rs b/apps/locallibrary/src/tui/account.rs index b206c4493..e4ea7aae1 100644 --- a/apps/locallibrary/src/tui/account.rs +++ b/apps/locallibrary/src/tui/account.rs @@ -117,14 +117,10 @@ impl Account { let again = if a.expired() { Span::styled(" · the session has likely ended; L signs in again", fg(theme::current().warning)) } else { - Span::styled( - format!( - " · sign in again by {} ({} days)", - local_time(a.sign_in_again_by, "%Y-%m-%d"), - a.days_left() - ), - dim(), - ) + let by = local_time(a.sign_in_again_by, "%Y-%m-%d"); + let said = format!(" · sign in again by {by} ({} days)", a.days_left()); + // A warning in its last days. + Span::styled(said, if a.due_soon() { fg(theme::current().warning) } else { dim() }) }; let mut spans = vec![ Span::styled(" ● ", fg(theme::current().good)), diff --git a/apps/locallibrary/src/tui/account/tests.rs b/apps/locallibrary/src/tui/account/tests.rs index 6a9cd4106..f8dea8a11 100644 --- a/apps/locallibrary/src/tui/account/tests.rs +++ b/apps/locallibrary/src/tui/account/tests.rs @@ -279,7 +279,7 @@ fn the_bar_follows_typing_and_signing_in(tc: hegel::TestCase) { fn the_bar_says_whos_signed_in_and_l_twice_signs_out(tc: hegel::TestCase) { use hegel::generators as gs; let (mut app, _dir) = blank(View::Profile); - let days = tc.draw(gs::sampled_from(&[-400i64, -1, 3, 200][..])); + let days = tc.draw(gs::sampled_from(&[-400i64, -1, 1, 3, 200][..])); let until = (chrono::Utc::now() + chrono::Duration::days(days) + chrono::Duration::hours(12)).to_rfc3339(); let mut info = signed_in(&until); let loopback = tc.draw(gs::booleans()); @@ -296,6 +296,10 @@ fn the_bar_says_whos_signed_in_and_l_twice_signs_out(tc: hegel::TestCase) { } else { let by = local_time(info.sign_in_again_by, "%Y-%m-%d"); assert!(bar.contains(&format!("sign in again by {by} ({} days)", info.days_left())), "{bar}"); + // A warning in its last two days. + let line = app.profile.account.line(width); + let again = line.spans.iter().find(|s| s.content.contains("sign in again by")).unwrap(); + assert_eq!(again.style.fg == Some(crate::tui::theme::current().warning.into()), days == 1, "{bar}"); } // A handle being typed is put away when another view is shown. if tc.draw(gs::booleans()) { diff --git a/apps/locallibrary/src/tui/mod.rs b/apps/locallibrary/src/tui/mod.rs index 212a13d98..6ed7e1dad 100644 --- a/apps/locallibrary/src/tui/mod.rs +++ b/apps/locallibrary/src/tui/mod.rs @@ -53,6 +53,7 @@ pub fn run(store_root: PathBuf, drive: Option) -> Result<()> { crate::plugins::events::notices_to(Some(app.plugin_notices())); crate::plugins::ask_to(Some(app.plugin_asker())); app.start_plugins(); + app.warn_signing_out(); // (Not `ratatui::init`, whose panic hook would put the screen back for a worker's panic.) enable_raw_mode()?; execute!(stdout(), EnterAlternateScreen)?; diff --git a/apps/locallibrary/src/tui/plugins/accounts.rs b/apps/locallibrary/src/tui/plugins/accounts.rs index a4ce80ecb..0e2c8ce15 100644 --- a/apps/locallibrary/src/tui/plugins/accounts.rs +++ b/apps/locallibrary/src/tui/plugins/accounts.rs @@ -1,6 +1,7 @@ //! Accounts in the player: signing in to an account of each type the account plugins provide, -//! and out of each the store keeps, from the palette beside the plugins' settings. Each runs in the background: nothing here reads -//! the keychain or reaches a service while the player draws. +//! and out of each the store keeps, from the palette beside the plugins' settings, and a warning as +//! it starts of each that signs itself out within `WARN_DAYS`. Each runs in the background: nothing +//! here reads the keychain or reaches a service while the player draws. use std::hash::{DefaultHasher, Hash, Hasher}; use std::path::Path; @@ -77,6 +78,21 @@ impl App { }); } + /// As the player starts, a warning of each account the store keeps that signs itself out + /// within `WARN_DAYS` (read without the keychain), saying where to sign in again. + pub(in crate::tui) fn warn_signing_out(&mut self) { + let Ok(plugins) = crate::plugins::host() else { return }; + let Ok(Some(store)) = Store::open_existing(&self.store_root) else { return }; + let kept = plugins.kept_accounts(&store).unwrap_or_default(); + let due = kept.iter().filter(|a| a.due_soon()).filter_map(|a| { + Some(format!("the {} {} {}", title(plugins, a), a.name, crate::account_cmd::signing_out(a.signs_out?))) + }); + let due: Vec = due.collect(); + if !due.is_empty() { + self.toast(format!("{}: sign in to it again from the palette (:)", due.join("; ")), Level::Info); + } + } + /// Sign out of the account the palette marked `mark` in the background: it's forgotten here, /// and its service is told to forget its tokens too when it has a way to be. Plugins keep /// their access to it. diff --git a/apps/locallibrary/src/tui/plugins/tests/accounts.rs b/apps/locallibrary/src/tui/plugins/tests/accounts.rs index 98d1e3f90..248637b9b 100644 --- a/apps/locallibrary/src/tui/plugins/tests/accounts.rs +++ b/apps/locallibrary/src/tui/plugins/tests/accounts.rs @@ -166,6 +166,30 @@ fn signing_in_and_out_from_the_palette_leaves_the_accounts_as_the_steps_say(tc: } } +/// For a mock account the store keeps, refreshed or not, signing itself out in any number of +/// hours (as its access token expires, or its refresh token runs out) or never: the player warns +/// of it as it starts exactly when that's within `WARN_DAYS`, saying where to sign in again. +#[hegel::test(test_cases = 20)] +fn the_player_warns_of_an_account_signing_itself_out_soon(tc: TestCase) { + let (mut app, _store) = blank(Screen::Library); + plugins_once(); + let refreshes = tc.draw(gs::booleans()); + let hours = tc.draw(gs::optional(gs::integers::().min_value(1).max_value(24 * 30))); + let at = hours.map(|h| chrono::Utc::now() + chrono::Duration::hours(h)); + let lasting = if refreshes { "refresh_expires_at" } else { "expires_at" }; + kept(&app, &["a"]); + let file = app.store_root.join("accounts.json"); + let mut book: serde_json::Value = serde_json::from_str(&std::fs::read_to_string(&file).unwrap()).unwrap(); + book["accounts"][0]["refreshes"] = refreshes.into(); + book["accounts"][0][lasting] = serde_json::json!(at); + std::fs::write(&file, book.to_string()).unwrap(); + app.warn_signing_out(); + let warned = hours.is_some_and(|h| h <= 24 * locallibrary::plugin::WARN_DAYS); + let said = last_message(&app); + assert_eq!(said.contains("the Mock account a@mock.example signs itself out "), warned, "{hours:?}: {said}"); + assert_eq!(said.ends_with(": sign in to it again from the palette (:)"), warned, "{said}"); +} + /// What a plugin asks of the mock account `a`: `write`, signing in again to it or not. fn wanting(sign_in: bool) -> AccessQuestion { let account = AccountId { plugin: "mock-accounts".into(), subject: "a".into() }; diff --git a/apps/locallibrary/tests/it/account.rs b/apps/locallibrary/tests/it/account.rs index 08be80217..5d67d3f98 100644 --- a/apps/locallibrary/tests/it/account.rs +++ b/apps/locallibrary/tests/it/account.rs @@ -106,7 +106,7 @@ fn the_status_says_what_the_session_allows_and_until_when() { let day = |t: &str| locallibrary::local_time(chrono::DateTime::parse_from_rfc3339(t).unwrap(), "%Y-%m-%d").to_string(); let (signed_in, again) = (day("2026-10-05T00:00:00Z"), day("2099-01-01T00:00:00Z")); - assert!(out.contains(&format!(" signed in {signed_in} - sign in again by {again} (")), "{out}"); + assert!(out.contains(&format!("\n signed in {signed_in} - sign in again by {again} (")), "{out}"); assert!(out.contains("allowed: atproto transition:generic - session kept "), "{out}"); if cfg!(target_os = "macos") { assert!(out.contains("session kept in the macOS Keychain"), "{out}"); @@ -114,9 +114,13 @@ fn the_status_says_what_the_session_allows_and_until_when() { assert!(out.contains(" client https://dj.example.com/c.json\n"), "{out}"); // Signed in to publish: the server granted the permission set's scopes in its place. + // Within its last two days, it's a warning. sign_in(&env.store, pds, &PUBLISHING, json!({ "sign_in_again_by": from_now(2) })); let out = env.ok(&["account", "status"]); - assert!(out.contains(" (today)\n"), "{out}"); + assert!( + out.contains(&format!("! signed in {signed_in} - sign in again by ")) && out.contains(" (today)\n"), + "{out}" + ); assert!( out.contains( "can publish your My Settings, profile, sets and their recordings; can't post or change anything else" diff --git a/apps/locallibrary/tests/it/plugins/accounts/cli.rs b/apps/locallibrary/tests/it/plugins/accounts/cli.rs index 12452b11a..a3f3de165 100644 --- a/apps/locallibrary/tests/it/plugins/accounts/cli.rs +++ b/apps/locallibrary/tests/it/plugins/accounts/cli.rs @@ -381,6 +381,25 @@ fn a_destination_without_access_asks_which_account_in_a_terminal() { assert!(out.starts_with("✓ Echo has access to the Mock account b@mock.example\n"), "{out}"); } +/// A destination whose account signs itself out within two days (its refresh token running out) +/// warns of it, and in a terminal offers to sign in again first. +#[cfg(unix)] +#[test] +fn a_destination_warns_of_an_account_signing_itself_out_soon() { + let m = Mock::new(); + m.state.lock().unwrap().refresh_expires_in = Some(129_600); + assert_eq!(m.sign_in("a", &["--for", "echo"]).0, 0); + let warned = "! the Mock account a@mock.example signs itself out on "; + let (code, out, err) = m.run(©); + assert_eq!(code, 0, "{err}"); + assert!(out.contains(warned) && out.contains(" (1 days)\n"), "{out}"); + assert!(!err.contains("Sign in again now?"), "asked without a terminal: {err}"); + let (code, out, err) = m.typed(©, "y\n", "a"); + assert_eq!(code, 0, "{err}"); + assert!(err.contains("? Sign in again now? [y/N]"), "{err}"); + assert!(out.contains(warned) && out.contains("✓ Signed in to the Mock account a@mock.example\n"), "{out}"); +} + /// Where [`a_plugin_wanting_more_of_an_account_asks_only_in_a_terminal`] runs echo's `ensure`. #[derive(Debug, Clone, Copy)] enum Asked { @@ -492,6 +511,8 @@ enum Naming { enum Lasting { /// An hour, then they're refreshed. Refreshed, + /// An hour, then they're refreshed for a day and a half, as the service says. + RunsOut, /// An hour, with nothing to refresh them with. Hour, /// Until they're revoked, with nothing to refresh them with. @@ -583,6 +604,19 @@ fn echo_accesses(m: &Mock, two: bool) -> Value { instead = true, grants_write = false )] +#[hegel::explicit_test_case( + two = false, + revokes = false, + unscoped = false, + lasting = Lasting::RunsOut, + naming = Naming::Subject, + json = false, + a_with = WITH, + b_with = WITH, + same_name = false, + instead = false, + grants_write = true +)] #[hegel::explicit_test_case( two = true, revokes = true, @@ -600,7 +634,8 @@ fn every_account_command_says_in_words_what_it_answers_as_json(tc: TestCase) { let two = tc.draw(gs::booleans()); let revokes = tc.draw(gs::booleans()); let unscoped = tc.draw(gs::booleans()); - let lasting = tc.draw(gs::sampled_from(vec![Lasting::Refreshed, Lasting::Hour, Lasting::Ever])); + let lasting = tc.draw(gs::sampled_from(vec![Lasting::Refreshed, Lasting::RunsOut, Lasting::Hour, Lasting::Ever])); + let refreshed = matches!(lasting, Lasting::Refreshed | Lasting::RunsOut); let naming = tc.draw(gs::sampled_from(vec![Naming::Subject, Naming::Name, Naming::Typed])); let json = tc.draw(gs::booleans()); let (m, providers) = providing(two, revokes); @@ -617,8 +652,9 @@ fn every_account_command_says_in_words_what_it_answers_as_json(tc: TestCase) { 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.refreshless = !refreshed; s.expires_in = (!matches!(lasting, Lasting::Ever)).then_some(3600); + s.refresh_expires_in = matches!(lasting, Lasting::RunsOut).then_some(129_600); } let called = |k: &Kept| match two { true => format!("Mock account {} (through {})", k.name, k.with), @@ -723,16 +759,37 @@ fn every_account_command_says_in_words_what_it_answers_as_json(tc: TestCase) { let account = &one["accounts"][0]; assert_eq!((&account["subject"], &account["plugin"]), (&json!(k.subject), &json!(k.with)), "{one}"); assert!(listed["accounts"].as_array().unwrap().contains(account), "{listed}"); - assert_eq!(account["refreshes"], json!(matches!(lasting, Lasting::Refreshed)), "{one}"); + assert_eq!(account["refreshes"], json!(refreshed), "{one}"); assert_eq!(account["expires"].is_null(), matches!(lasting, Lasting::Ever), "{one}"); + // It signs itself out as its access token expires, unless it's refreshed: then as its + // refresh token runs out, when the service said. + let time = |key: &str| serde_json::from_value::>(account[key].clone()).ok(); + let signs_out = time("signs_out"); + let (at, then) = (signs_out.unwrap_or_default(), chrono::Utc::now()); + let (day, minute) = + (|t| locallibrary::local_time(t, "%Y-%m-%d"), |t| locallibrary::local_time(t, "%Y-%m-%d %H:%M")); let state = match lasting { Lasting::Refreshed => "signed in, refreshed on its own".to_string(), + Lasting::RunsOut => { + assert!((at - then - chrono::Duration::seconds(129_600)).num_seconds().abs() < 60, "{one}"); + format!("signed in, refreshed on its own · signs itself out on {} (1 days)", day(at)) + } Lasting::Hour => { - let at: chrono::DateTime = serde_json::from_value(account["expires"].clone()).unwrap(); - format!("signed in until {}", locallibrary::local_time(at, "%Y-%m-%d %H:%M")) + assert_eq!(signs_out, time("expires"), "{one}"); + format!("signed in · signs itself out at {} (today)", minute(at)) } Lasting::Ever => "signed in".to_string(), }; + assert_eq!(signs_out.is_some(), matches!(lasting, Lasting::RunsOut | Lasting::Hour), "{one}"); + // Both within two days, it's a warning too. + let warning = match signs_out.is_some() { + true => format!( + "! it {}: `locallibrary account mock:{} sign-in` signs in again\n", + state.rsplit(" · ").next().unwrap(), + k.subject + ), + false => String::new(), + }; // What the service granted, or when it doesn't say, what was asked. let allowed = joined(&account["scopes"], " "); assert_eq!(allowed == granted, !unscoped, "{allowed}"); @@ -740,7 +797,7 @@ fn every_account_command_says_in_words_what_it_answers_as_json(tc: TestCase) { true => "", false => " access: echo (default)\n", }; - let shown = format!("{}\n {state} · allowed: {allowed}\n{access}", called(k)); + let shown = format!("{}\n {state} · allowed: {allowed}\n{warning}{access}", called(k)); assert_eq!(m.run(&["account", &target, "status"]).1, shown); } diff --git a/apps/locallibrary/tests/it/publish.rs b/apps/locallibrary/tests/it/publish.rs index 7ddb1425c..59ffec18d 100644 --- a/apps/locallibrary/tests/it/publish.rs +++ b/apps/locallibrary/tests/it/publish.rs @@ -51,6 +51,17 @@ fn settings_publish_and_import_from_the_command_line() { let out = env.ok(&["settings", "publish", "--yes"]); assert!(out.contains("already published, unchanged"), "{out}"); + // In the session's last two days, publishing warns that it's to sign in again (offering to + // only in a terminal, without --yes). + env.ok(&["--yes", "settings", "set", "quantize=on"]); + let soon = (chrono::Utc::now() + chrono::Duration::hours(30)).to_rfc3339(); + sign_in(&env.store, &pds.url, &PUBLISHING, json!({ "sign_in_again_by": soon })); + let out = env.ok(&["settings", "publish", "--yes"]); + assert!(out.contains("! @dj.example.com: sign in again by ") && out.contains("published"), "{out}"); + assert!(!out.contains("Sign in again now?"), "{out}"); + env.ok(&["--yes", "settings", "set", "quantize=off"]); + env.ok(&["--yes", "settings", "publish"]); + // Another computer imports them, signed out, naming the account. let other = Env::new().with_directory(&pds.url); let from = ["settings", "import", "--from-handle", DID]; diff --git a/crates/locallibrary/src/atmosphere/mod.rs b/crates/locallibrary/src/atmosphere/mod.rs index 3ed2bc821..85b517ef9 100644 --- a/crates/locallibrary/src/atmosphere/mod.rs +++ b/crates/locallibrary/src/atmosphere/mod.rs @@ -232,6 +232,12 @@ impl AccountInfo { pub fn expired(&self) -> bool { Utc::now() >= self.sign_in_again_by } + + /// Whether signing in again is due within [`crate::plugin::WARN_DAYS`], or was: front ends + /// warn, offering to. + pub fn due_soon(&self) -> bool { + crate::plugin::due_soon(self.sign_in_again_by) + } } // --------------------------------------------------------------------------------------------- diff --git a/crates/locallibrary/src/plugin.rs b/crates/locallibrary/src/plugin.rs index 7bc8bbe20..d5c830323 100644 --- a/crates/locallibrary/src/plugin.rs +++ b/crates/locallibrary/src/plugin.rs @@ -72,11 +72,11 @@ use crate::fields::roles::{Offered, RoleField}; use crate::keychain::Storage; use crate::{Error, Result}; -pub use accounts::REFRESH_BEFORE; pub use accounts::access::{Access, AccountId}; pub use accounts::ensure::{AccessQuestion, AskAccess}; pub use accounts::ready::Unready; pub use accounts::shared::{Account, AccountSignIn, AccountSignedIn, AccountSignedOut, Accounts}; +pub use accounts::{REFRESH_BEFORE, WARN_DAYS, due_soon}; pub use analysis::{AnalysisPart, ApplyAnalysis, same_grid}; pub use analyzers::Analyzed; pub use apply::ApplyFields; diff --git a/crates/locallibrary/src/plugin/accounts.rs b/crates/locallibrary/src/plugin/accounts.rs index 170a0694d..6265c121b 100644 --- a/crates/locallibrary/src/plugin/accounts.rs +++ b/crates/locallibrary/src/plugin/accounts.rs @@ -20,10 +20,13 @@ use std::time::{Duration, Instant}; use chrono::{DateTime, Utc}; use indexmap::IndexMap; use loclib_plugin::AccountType; -use oauth2::basic::{BasicClient, BasicErrorResponse, BasicTokenResponse}; +use oauth2::basic::{ + BasicErrorResponse, BasicRevocationErrorResponse, BasicTokenIntrospectionResponse, BasicTokenType, +}; use oauth2::{ - AuthUrl, AuthorizationCode, ClientId, CsrfToken, EndpointNotSet, EndpointSet, HttpRequest, HttpResponse, - PkceCodeChallenge, PkceCodeVerifier, RedirectUrl, RefreshToken, RequestTokenError, Scope, TokenResponse, TokenUrl, + AuthUrl, AuthorizationCode, ClientId, CsrfToken, EndpointNotSet, EndpointSet, ExtraTokenFields, HttpRequest, + HttpResponse, PkceCodeChallenge, PkceCodeVerifier, RedirectUrl, RefreshToken, RequestTokenError, Scope, + StandardRevocableToken, StandardTokenResponse, TokenResponse, TokenUrl, }; use reqwest::Url; use rustc_hash::FxHashMap; @@ -56,10 +59,46 @@ const PENDING_LIMIT: Duration = Duration::from_secs(15 * 60); /// The most of a token endpoint's answer read. const MAX_ANSWER: u64 = 1 << 20; +/// How many days before an account signs itself out the front ends warn, offering to sign in +/// again: an Atmosphere account's session, and an account through an account plugin that isn't +/// refreshed, or whose refresh token runs out. +pub const WARN_DAYS: i64 = 2; + +/// Whether an account that signs itself out at `at` is to be warned about now: it does within +/// [`WARN_DAYS`], or did. +pub fn due_soon(at: DateTime) -> bool { + at - Utc::now() <= chrono::Duration::days(WARN_DAYS) +} + +/// What a token endpoint answers besides OAuth 2's own fields: how many seconds the refresh token +/// lasts, when the service says (as GitHub and Microsoft do). +#[derive(Debug, Clone, Serialize, Deserialize)] +struct Lasting { + #[serde(default, skip_serializing_if = "Option::is_none")] + refresh_token_expires_in: Option, +} + +impl ExtraTokenFields for Lasting {} + +/// What a token endpoint answers. +type Answer = StandardTokenResponse; + /// An account type's client at its service, for signing in (with the browser's way back) and -/// refreshing. Revoking is the host's own request (see [`Plugin::revoke`]): the crate's refuses an -/// endpoint on this computer, which tests have. -type Client = BasicClient; +/// refreshing, as `oauth2` builds it (with its authorization and token endpoints set). Revoking is +/// the host's own request (see [`Plugin::revoke`]): the crate's refuses an endpoint on this +/// computer, which tests have. +type Client = oauth2::Client< + BasicErrorResponse, + Answer, + BasicTokenIntrospectionResponse, + StandardRevocableToken, + BasicRevocationErrorResponse, + HasAuthUrl, + EndpointNotSet, + EndpointNotSet, + EndpointNotSet, + HasTokenUrl, +>; /// What's kept of an account's sign-in, as JSON. Never printed: it has no `Debug`. #[derive(Serialize, Deserialize)] @@ -69,20 +108,27 @@ struct Tokens { refresh_token: Option, #[serde(default, skip_serializing_if = "Option::is_none")] expires_at: Option>, + /// When the refresh token runs out, when the service said. + #[serde(default, skip_serializing_if = "Option::is_none")] + refresh_expires_at: Option>, #[serde(default, skip_serializing_if = "Vec::is_empty")] scopes: Vec, } impl Tokens { - /// What a token endpoint answered, keeping the refresh token (and scopes) it had when a - /// refresh's answer leaves them out. - fn from(answer: &BasicTokenResponse, had: Option) -> Tokens { + /// What a token endpoint answered, keeping the refresh token it had (and when that runs out) + /// and the scopes when a refresh's answer leaves them out. + fn from(answer: &Answer, had: Option) -> Tokens { + let at = |d: Duration| chrono::Duration::from_std(d).ok().map(|d| Utc::now() + d); let scopes = answer.scopes().map(|s| s.iter().map(|s| s.to_string()).collect()); let refresh_token = answer.refresh_token().map(|t| t.secret().clone()); + let lasts = answer.extra_fields().refresh_token_expires_in.map(Duration::from_secs).and_then(at); + let kept = refresh_token.is_none().then(|| had.as_ref().and_then(|h| h.refresh_expires_at)).flatten(); Tokens { access_token: answer.access_token().secret().clone(), - expires_at: answer.expires_in().and_then(|d| chrono::Duration::from_std(d).ok()).map(|d| Utc::now() + d), + expires_at: answer.expires_in().and_then(at), refresh_token: refresh_token.or_else(|| had.as_ref().and_then(|h| h.refresh_token.clone())), + refresh_expires_at: lasts.or(kept), scopes: scopes.or_else(|| had.map(|h| h.scopes)).unwrap_or_default(), } } @@ -123,7 +169,7 @@ impl Plugin { self.misbehaved(format!("its {} account's {what} can't be used: {e}", t.title)) }; let token_url = t.token_url.as_ref().or(t.token_exchange.as_ref()).cloned().unwrap_or_default(); - let client = BasicClient::new(ClientId::new(t.client_id.clone())) + let client = Client::::new(ClientId::new(t.client_id.clone())) .set_auth_uri(AuthUrl::new(t.authorize_url.clone()).map_err(|e| bad("authorize_url", e))?) .set_token_uri(TokenUrl::new(token_url).map_err(|e| bad("token_url", e))?); Ok(match redirect { @@ -228,7 +274,7 @@ impl Plugin { client: &Client, verifier: PkceCodeVerifier, query: &CallbackQuery, - ) -> Result { + ) -> Result { if let Some(why) = query.refusal() { return Err(Error::Account(why)); } diff --git a/crates/locallibrary/src/plugin/accounts/kept.rs b/crates/locallibrary/src/plugin/accounts/kept.rs index eb440bc18..07b37a8e4 100644 --- a/crates/locallibrary/src/plugin/accounts/kept.rs +++ b/crates/locallibrary/src/plugin/accounts/kept.rs @@ -54,6 +54,9 @@ pub(super) struct Kept { pub expires_at: Option>, /// It's refreshed without signing in again. pub refreshes: bool, + /// When its refresh token runs out, when the service said. + #[serde(default, skip_serializing_if = "Option::is_none")] + pub refresh_expires_at: Option>, } impl Kept { @@ -62,12 +65,22 @@ impl Kept { self.plugin == plugin && self.subject == subject } - /// What `tokens` say of it: what the service granted, when they expire and whether they - /// refresh. + /// What `tokens` say of it: what the service granted, when they expire and whether (and till + /// when) they refresh. pub fn granted(&mut self, tokens: &Tokens) { self.scopes = tokens.scopes.clone(); self.expires_at = tokens.expires_at; self.refreshes = tokens.refresh_token.is_some(); + self.refresh_expires_at = tokens.refresh_expires_at; + } + + /// When it signs itself out, when it does: as its access token expires when it isn't + /// refreshed, else as its refresh token runs out, when the service said. + pub fn signs_out(&self) -> Option> { + match self.refreshes { + true => self.refresh_expires_at, + false => self.expires_at, + } } } @@ -112,6 +125,7 @@ pub(super) fn keep(storage: Storage, store: &Store, kept: &Kept, tokens: Option< access_token: t.access_token.clone(), refresh_token: t.refresh_token.clone(), expires_at: None, + refresh_expires_at: None, scopes: Vec::new(), }; serde_json::to_string(&only).map_err(|e| Error::invalid(e.to_string())) diff --git a/crates/locallibrary/src/plugin/accounts/shared.rs b/crates/locallibrary/src/plugin/accounts/shared.rs index 8ef986539..9d0152b76 100644 --- a/crates/locallibrary/src/plugin/accounts/shared.rs +++ b/crates/locallibrary/src/plugin/accounts/shared.rs @@ -47,10 +47,22 @@ pub struct Account { pub expires: Option>, /// It's refreshed without signing in again. pub refreshes: bool, + /// When it signs itself out, when it does: as its access token expires when it isn't + /// refreshed, else as its refresh token runs out, when the service said. Front ends warn + /// within [`crate::plugin::WARN_DAYS`] of it. + #[serde(skip_serializing_if = "Option::is_none")] + pub signs_out: Option>, /// Its tokens are kept (the keychain may have lost them). pub signed_in: bool, } +impl Account { + /// Whether it's signed in and signs itself out within [`crate::plugin::WARN_DAYS`]. + pub fn due_soon(&self) -> bool { + self.signed_in && self.signs_out.is_some_and(super::due_soon) + } +} + /// The accounts a store keeps and the accesses plugins were given to them, as hosts show them. #[derive(Debug, Clone, Default, PartialEq, Serialize, JsonSchema)] pub struct Accounts { @@ -121,6 +133,7 @@ pub(crate) struct Asked { /// `kept` as hosts show it. fn listed(kept: Kept, signed_in: bool) -> Account { Account { + signs_out: kept.signs_out(), kind: kept.kind, plugin: kept.plugin, subject: kept.subject, @@ -154,7 +167,8 @@ impl Plugin { let secret = secret(self.host.options.secrets, &kept::place(store, &kept.plugin, &kept.subject))?; let read = |s: String| { let tokens = self.read_tokens(&kept.name, &s)?; - Ok(Tokens { expires_at: kept.expires_at, scopes: kept.scopes.clone(), ..tokens }) + let (expires_at, refresh_expires_at) = (kept.expires_at, kept.refresh_expires_at); + Ok(Tokens { expires_at, refresh_expires_at, scopes: kept.scopes.clone(), ..tokens }) }; secret.map(read).transpose() } @@ -264,6 +278,7 @@ impl Plugins { scopes: Vec::new(), expires: None, refreshes: false, + signs_out: None, signed_in: false, }); } diff --git a/crates/locallibrary/src/plugin/accounts/tests.rs b/crates/locallibrary/src/plugin/accounts/tests.rs index 37407ed54..e00d4d6ea 100644 --- a/crates/locallibrary/src/plugin/accounts/tests.rs +++ b/crates/locallibrary/src/plugin/accounts/tests.rs @@ -1,5 +1,6 @@ use hegel::TestCase; use hegel::generators as gs; +use serde_json::json; use super::*; @@ -13,8 +14,61 @@ fn a_token_is_refreshed_once_it_expires_within_a_minute(tc: TestCase) { access_token: "a".into(), refresh_token: None, expires_at: left.map(|s| now + chrono::Duration::seconds(s)), + refresh_expires_at: None, scopes: Vec::new(), }; let refreshed = kept.expiring(now); assert_eq!(refreshed, left.is_some_and(|s| s <= REFRESH_BEFORE.as_secs() as i64), "{left:?}"); } + +/// An account signing itself out is warned about within `WARN_DAYS` of it, or once it has. +#[hegel::test(test_cases = 300)] +fn an_account_is_due_soon_within_warn_days(tc: TestCase) { + let day = 86_400; + let offset = tc.draw(gs::integers::().min_value(-40 * day).max_value(40 * day)); + // Checked a moment after: far enough from the boundary not to cross it. + tc.assume((offset - WARN_DAYS * day).abs() > 5); + assert_eq!(due_soon(Utc::now() + chrono::Duration::seconds(offset)), offset <= WARN_DAYS * day, "{offset}"); +} + +/// For any token endpoint answer, saying how long its tokens last or not, a refresh token or not, +/// and the tokens it had before or not: the access token expires and the refresh token runs out +/// when the answer says; a refresh token it leaves out is the one it had, running out when that +/// did, and a new one runs out only when the answer says. +#[hegel::test(test_cases = 200)] +fn tokens_last_as_the_service_says(tc: TestCase) { + let lasting = || tc.draw(gs::optional(gs::integers::().min_value(60).max_value(400 * 86_400))); + let (expires_in, refresh_lasts) = (lasting(), lasting()); + let refresh = tc.draw(gs::booleans()); + let answer = json!({ + "access_token": "at-2", + "token_type": "bearer", + "expires_in": expires_in, + "refresh_token": refresh.then_some("rt-2"), + "refresh_token_expires_in": refresh_lasts, + }); + let answer: Answer = serde_json::from_value(answer).unwrap(); + let before = Utc::now() + chrono::Duration::days(3); + let had = tc.draw(gs::booleans()).then(|| Tokens { + access_token: "at-1".into(), + refresh_token: Some("rt-1".into()), + expires_at: None, + refresh_expires_at: Some(before), + scopes: Vec::new(), + }); + let kept_refresh = had.is_some(); + let now = Utc::now(); + let t = Tokens::from(&answer, had); + // When `secs` from about now is. + let about = |at: Option>, secs: Option| match (at, secs) { + (Some(at), Some(s)) => (at - now - chrono::Duration::seconds(s as i64)).num_seconds().abs() < 5, + (at, s) => at.is_none() && s.is_none(), + }; + assert!(about(t.expires_at, expires_in), "{:?} {expires_in:?}", t.expires_at); + match (refresh, refresh_lasts) { + (false, None) if kept_refresh => { + assert_eq!((t.refresh_token.as_deref(), t.refresh_expires_at), (Some("rt-1"), Some(before))); + } + _ => assert!(about(t.refresh_expires_at, refresh_lasts), "{:?} {refresh_lasts:?}", t.refresh_expires_at), + } +} diff --git a/crates/locallibrary/tests/it/props_atmosphere.rs b/crates/locallibrary/tests/it/props_atmosphere.rs index e154c34c4..aa64d8406 100644 --- a/crates/locallibrary/tests/it/props_atmosphere.rs +++ b/crates/locallibrary/tests/it/props_atmosphere.rs @@ -129,8 +129,9 @@ fn the_publish_scopes_cover_exactly_locallibrarys_records(tc: TestCase) { assert!(atmosphere::missing_scope(&account, "app.example.thing").is_some()); } -/// Whole days left until signing in again, and whether that's passed, agree with the time it's -/// due; an account is named by its handle when it has one, its DID otherwise. +/// Whole days left until signing in again, whether that's passed, and whether it's due within +/// `WARN_DAYS` (warned about), agree with the time it's due; an account is named by its handle +/// when it has one, its DID otherwise. #[hegel::test(test_cases = 300)] fn days_left_and_expiry_agree_with_the_due_date(tc: TestCase) { let day = 86_400; @@ -141,6 +142,7 @@ fn days_left_and_expiry_agree_with_the_due_date(tc: TestCase) { let account = info("did:plc:abc", handle.clone(), vec![], Utc::now() + chrono::Duration::seconds(offset)); assert_eq!(account.days_left(), offset / day, "{offset}"); assert_eq!(account.expired(), offset < 0); + assert_eq!(account.due_soon(), offset <= locallibrary::plugin::WARN_DAYS * day, "{offset}"); assert_eq!(account.name(), handle.map_or("did:plc:abc".to_string(), |h| format!("@{h}"))); } diff --git a/crates/locallibrary/tests/support/mock_oauth.rs b/crates/locallibrary/tests/support/mock_oauth.rs index be49b6526..4f842e7ee 100644 --- a/crates/locallibrary/tests/support/mock_oauth.rs +++ b/crates/locallibrary/tests/support/mock_oauth.rs @@ -91,6 +91,8 @@ impl Server { #[derive(Default)] pub struct Service { pub expires_in: Option, + /// How many seconds a refresh token it answers lasts, which it says when it's given. + pub refresh_expires_in: Option, /// Whether a refresh answers a new refresh token. pub rotates: bool, /// It never answers a refresh token, so what's signed in to there isn't refreshed. @@ -206,6 +208,9 @@ pub fn service(state: Arc>) -> Server { } if !s.refreshless && (f["grant_type"] == "authorization_code" || s.rotates) { answer["refresh_token"] = format!("rt-{n}").into(); + if let Some(secs) = s.refresh_expires_in { + answer["refresh_token_expires_in"] = secs.into(); + } } if let Some(secs) = s.expires_in { answer["expires_in"] = secs.into(); From 7381deacd0f8b519d6fe428fcbdb103baba5af09 Mon Sep 17 00:00:00 2001 From: botandbang <337765928+botandbang@users.noreply.github.com> Date: Sun, 1 Nov 2026 23:35:33 -0500 Subject: [PATCH 5/6] test(locallibrary): check the bar's warning in the colour the terminal draws The profile bar test compared the "sign in again by" span's colour with the theme's warning as plain RGB. The bar draws it through the theme, which gives RGB only on a terminal that says it takes 24-bit colour (COLORTERM, or a known TERM_PROGRAM) and an xterm-256 index otherwise, so the test passed in a truecolor terminal and failed on CI's runners. It now compares with what theme::fg gives for the warning colour, and passes with and without COLORTERM and with NO_COLOR. --- apps/locallibrary/src/tui/account/tests.rs | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/apps/locallibrary/src/tui/account/tests.rs b/apps/locallibrary/src/tui/account/tests.rs index f8dea8a11..e362bab8a 100644 --- a/apps/locallibrary/src/tui/account/tests.rs +++ b/apps/locallibrary/src/tui/account/tests.rs @@ -296,10 +296,11 @@ fn the_bar_says_whos_signed_in_and_l_twice_signs_out(tc: hegel::TestCase) { } else { let by = local_time(info.sign_in_again_by, "%Y-%m-%d"); assert!(bar.contains(&format!("sign in again by {by} ({} days)", info.days_left())), "{bar}"); - // A warning in its last two days. + // A warning in its last two days, in the colour the terminal draws it with. let line = app.profile.account.line(width); let again = line.spans.iter().find(|s| s.content.contains("sign in again by")).unwrap(); - assert_eq!(again.style.fg == Some(crate::tui::theme::current().warning.into()), days == 1, "{bar}"); + let warning = crate::tui::theme::fg(crate::tui::theme::current().warning); + assert_eq!(again.style.fg == warning.fg, days == 1, "{bar}"); } // A handle being typed is put away when another view is shown. if tc.draw(gs::booleans()) { From db735b8ebe51335fb87d2776ec50dc5fab1f8131 Mon Sep 17 00:00:00 2001 From: botandbang <337765928+botandbang@users.noreply.github.com> Date: Mon, 2 Nov 2026 08:38:16 -0500 Subject: [PATCH 6/6] test(locallibrary): build the ensure test's Asked only where its test runs `Asked`, the ways a_plugin_wanting_more_of_an_account_asks_only_in_a_terminal runs echo's `ensure`, was compiled on every platform, but its one test is `#[cfg(unix)]`, so on Windows it was never used and clippy's `-D warnings` stopped the merge queue's other-os job. It now has the same `#[cfg(unix)]` as its test. --- apps/locallibrary/tests/it/plugins/accounts/cli.rs | 1 + 1 file changed, 1 insertion(+) diff --git a/apps/locallibrary/tests/it/plugins/accounts/cli.rs b/apps/locallibrary/tests/it/plugins/accounts/cli.rs index a3f3de165..e40cab988 100644 --- a/apps/locallibrary/tests/it/plugins/accounts/cli.rs +++ b/apps/locallibrary/tests/it/plugins/accounts/cli.rs @@ -401,6 +401,7 @@ fn a_destination_warns_of_an_account_signing_itself_out_soon() { } /// Where [`a_plugin_wanting_more_of_an_account_asks_only_in_a_terminal`] runs echo's `ensure`. +#[cfg(unix)] #[derive(Debug, Clone, Copy)] enum Asked { /// In a terminal, answering yes or no.