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