diff --git a/CHANGELOG.md b/CHANGELOG.md index 11c1878..e4f8c2d 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -31,19 +31,25 @@ that may never merge. They are not releases and are not listed here. `cli_token_unavailable` and `cli_token_forbidden` (see [Errors](docs/commands.md#errors)). -- The CLI's own token, used for telemetry when you have no token of your - own, is now fetched from `https://cli.mapbox.com/config/token.json` and - kept in `~/.mapbox/cli-token.json` instead of being built into the binary, +- The CLI's own token, used for telemetry, is now fetched from + `https://cli.mapbox.com/config/token.json` and kept in + `~/.mapbox/cli-token.json` instead of being built into the binary, so it can be rotated without a release. What changes for an existing setup: - `MAPBOX_CLI_BUNDLED_TOKEN` is no longer read at build time. A build from source fetches the token like a release does. - Keeping the token creates `~/.mapbox` if it is missing, even on a run whose only network use is telemetry. - - When an API refuses the token, the Tokens API is asked why. Only a - revoked, invalid, expired or malformed token is replaced, at once. - Three fetches in a row that find no usable token, or three refusals - in a row of a valid one, pause that for an hour. + - When an API refuses the token, the Tokens API is asked why and the + endpoint asked for a new token at once. A revoked, invalid, expired or + malformed token is replaced; a different token served is used from the + next run on. + - Three failures in a row, each a fetch that finds no usable token or a + refusal with no different token served, stop telemetry sending and + fetching for an hour. The count is telemetry's own: it never holds + `mapbox report` or `MAPBOX_CLI_TOKEN`, and the waits an earlier dev + build kept in `cli-token.json` are ignored. A new token ends it, even + one `mapbox report` fetched. - `tilesets query` renders as a numbered list under `-o text` again, as documented. It had printed pretty JSON since 0.2.0, when the command moved @@ -305,13 +311,14 @@ that may never merge. They are not releases and are not listed here. file, so with two installs, set it again after using the older one. - Each run sends one `cli.command` telemetry event to Mapbox, from a - background process the command doesn't wait for, with your own token - (`--token`, `MAPBOX_ACCESS_TOKEN` or your login) or, when you have none, - one built into the CLI. A build from source has no built-in token, so with - no token of your own the event is dropped. It never touches stdout, the - exit code or how long a command takes, so a script sees no difference; a - machine that already has `~/.mapbox` finds a `.telemetry` directory in it. - `MAPBOX_CLI_NO_TELEMETRY=1` turns it off. + background process the command doesn't wait for. It is only ever sent + with the CLI's own public token, never yours (`--token`, + `MAPBOX_ACCESS_TOKEN` or your login), since Mapbox Events keeps the token + an event was sent with. When the CLI's token can't be used, or the send + fails, the event is dropped; it is never sent twice. It never touches + stdout, the exit code or how long a command takes, so a script sees no + difference; a machine that already has `~/.mapbox` finds a `.telemetry` + directory in it. `MAPBOX_CLI_NO_TELEMETRY=1` turns it off. - `MAPBOX_CLI_EXTRA_QUERY` appends raw query parameters to every request, in the same `k1=v1&k2=v2` shape as a URL's own query string — for an API diff --git a/README.md b/README.md index 18edc2c..6a7361d 100644 --- a/README.md +++ b/README.md @@ -301,11 +301,10 @@ Each run also sends one event to Mapbox: It never includes a file path or free text you typed. It carries a random ID that is replaced on the first run a day after it was created. It is sent in the background, so a command never waits for it, and dropped if it can't be -delivered. It is sent with your own token (`--token`, `MAPBOX_ACCESS_TOKEN` -or your login) when you have one, and with the CLI's own public token -otherwise; Mapbox Events keeps the token an event was sent with, and the -account it belongs to, alongside the event. With no token of your own and -no way to fetch the CLI's, the event is dropped. +delivered. It is only ever sent with the CLI's own public token, never +yours (`--token`, `MAPBOX_ACCESS_TOKEN` or your login); Mapbox Events keeps +the token an event was sent with alongside the event. When the CLI's token +can't be used, the event is dropped, and it is never sent twice. `MAPBOX_CLI_NO_TELEMETRY=1` or `mapbox config set telemetry off` turns it off. ### Diagnostics and settings @@ -491,22 +490,27 @@ and `--dry-run` show it alongside everything else on the request. ### The CLI's own requests -The CLI has a token of its own, for requests it makes for itself. Telemetry -uses your own token whenever you have one: `--token` or -`MAPBOX_ACCESS_TOKEN`, then your login. Only when you have none does it use -the CLI's token: -`MAPBOX_CLI_TOKEN`, or a public token fetched from -`https://cli.mapbox.com/config/token.json` and kept in -`~/.mapbox/cli-token.json`. It is replaced only when the Tokens API says it -is revoked, invalid, expired or malformed, not when one API refuses it. A -refusal of a valid token, or one the Tokens API cannot explain, counts; the -third in a row pauses telemetry's sending with it for an hour, as three -fetches in a row that find no usable token pause fetching. The token is -restricted to `https://cli.mapbox.com/`, so every request with it carries -that as its `Referer`. The CLI's token is only used for what -`src/cli_token.rs` lists for it: telemetry, and `mapbox report`, which is -the other way around: it prefers the CLI's token to yours and falls back to -yours while the CLI's can't be used. +The CLI has a token of its own, for requests it makes for itself. +Telemetry uses only that token, never the one you run commands with +(`--token`, `MAPBOX_ACCESS_TOKEN` or a login): it uses a public token +fetched from `https://cli.mapbox.com/config/token.json` and kept in +`~/.mapbox/cli-token.json`. `MAPBOX_CLI_TOKEN` is a developer override that +replaces that token with another CLI token, such as one for staging, and +is used only when it is a public `pk.` token. With neither usable, the +event is dropped, and an event that was refused is never sent again. A +refusal is checked with the Tokens API and the endpoint asked for a new +token at once: a token that is revoked, invalid, expired or malformed is +replaced, and a different token served is used from the next run on. +Three failures in a row, each a fetch that finds no usable token or a +refusal the endpoint has no different token for, stop telemetry sending +and fetching for an hour. That count is telemetry's own, and +`MAPBOX_CLI_TOKEN` is sent whatever it says. A new token ends it, even one +`mapbox report` fetched. The token is restricted to +`https://cli.mapbox.com/`, so every request with it carries that as its +`Referer`. The CLI's token is only used for what +`src/cli_token.rs` lists for it: telemetry, and `mapbox report`, which +prefers the CLI's token and falls back to yours while the CLI's can't be +used. ### Proxies @@ -663,7 +667,7 @@ tools are used and how to improve our products. `--dry-run` or whether a named profile was used), how it ended and how long it took, how many requests it made with their HTTP status and sizes, the kind of token it used and the Mapbox account that token belongs to, - the Mapbox access token the event itself was sent with, a random + the CLI's own public token the event itself was sent with, a random identifier that is replaced every day, which run started this one when it ran inside another, CLI version, how the CLI was installed, the version it was updated from and any newer one it offered, OS/architecture, @@ -679,8 +683,8 @@ tools are used and how to improve our products. adoption, prioritize investments, and improve the reliability, performance, and developer experience of our CLIs. - **What We Do Not Collect:** Code completion outputs, source code, project - file names, directory contents, non-Mapbox API keys, or credentials other - than the Mapbox access token an event is sent with. + file names, directory contents, non-Mapbox API keys, or credentials, + your Mapbox access token included. - **Who has Access:** Telemetry data will not be disclosed to, or accessed by, third parties other than Mapbox affiliates and passive cloud storage and hosting providers necessary to maintain our infrastructure. diff --git a/docs/architecture.md b/docs/architecture.md index 3ef0fe0..76b8fc5 100644 --- a/docs/architecture.md +++ b/docs/architecture.md @@ -191,7 +191,7 @@ flowchart LR events["Mapbox Events
POST /events/v2"] end - bin -- "① spawn detached, event + token
on stdin (never argv or env)" --> child + bin -- "① spawn detached, event
on stdin (never argv or env)" --> child child -- "② 🔒 HTTPS POST" --> events ``` @@ -203,15 +203,18 @@ entirely, same as Diagram 2. Step ① is detached and non-blocking: the parent spawns a copy of itself in this special mode and exits without waiting on it, so a dead network or a -slow endpoint never adds to a command's run time. The event and the token it -travels with go over the child's stdin rather than its argv or environment, -for the same `ps`-visibility reason Diagram 3 gives. - -Step ②'s token follows `src/cli_token.rs`'s fallback order: the user's own -token first (flag, environment, or stored login), and only when none of -those exist, the CLI's `pk.` token as a last resort so usage from a user who -has never logged in is still counted. Every build fetches that token from -`https://cli.mapbox.com/config/token.json` and keeps it in +slow endpoint never adds to a command's run time. The event goes over the +child's stdin rather than its argv or environment, for the same +`ps`-visibility reason Diagram 3 gives. The child is not handed the user's +token at all. + +Step ② is sent with the CLI's own `pk.` token only, never the user's +(`src/cli_token.rs`): `MAPBOX_CLI_TOKEN` when set to a `pk.` token, +otherwise the one every build fetches from +`https://cli.mapbox.com/config/token.json` and keeps in `~/.mapbox/cli-token.json` until the Tokens API says it is dead -(`src/cli_token/fetch.rs`). With no login and no way to fetch it, the event -is simply dropped. +(`src/cli_token/fetch.rs`). With no usable CLI token the event is dropped, +and a send that fails is not tried again. Three failures in a row (a fetch +with no usable token, or a refusal with no different token served) quiet +telemetry for an hour, a count kept per caller in that same file. A new +token saved by any caller ends every caller's count. diff --git a/src/auth.rs b/src/auth.rs index 5960e04..a0c6a61 100644 --- a/src/auth.rs +++ b/src/auth.rs @@ -479,8 +479,7 @@ fn credentials_path_readonly(profile: Option<&str>) -> Option { /// directory as a side effect of reading it — see /// [`credentials_path_readonly`]. What [`profiles`] reads each stored /// profile through, since listing what exists must not be the reason a -/// directory starts to exist or its permissions change, and what -/// [`crate::cli_token`] reads the login through for the same reason. +/// directory starts to exist or its permissions change. pub(crate) fn load_credentials_readonly(profile: Option<&str>) -> Option { let data = std::fs::read_to_string(credentials_path_readonly(profile)?).ok()?; serde_json::from_str(&data).ok() @@ -718,7 +717,7 @@ pub fn warn_if_environment_token_shadows_login(profile: Option<&str>, remedy: &s } } -pub(crate) fn token_needs_refresh(token: &str) -> bool { +fn token_needs_refresh(token: &str) -> bool { match token_expires_at(token) { Some(exp) => { let now = std::time::SystemTime::now() @@ -1068,8 +1067,8 @@ pub fn logout(profile: Option<&str>, mode: Mode) -> Result<()> { let path = credentials_path(profile)?; let had_credentials = path.exists(); if had_credentials { - // Waits out a refresh in flight, such as the telemetry sender's from - // the previous run, which would otherwise write the login back. + // Wait for another run's login refresh to finish, so it can't write + // the login back after it is removed. let _lock = CredentialLock::acquire(profile).ok(); crate::run_record::set_auth_step("remove_credentials"); std::fs::remove_file(&path)?; diff --git a/src/cli_token.rs b/src/cli_token.rs index bce3892..1e692a1 100644 --- a/src/cli_token.rs +++ b/src/cli_token.rs @@ -1,40 +1,49 @@ //! Requests the CLI makes for itself rather than for the user, and the token //! they carry. //! -//! The user's own token always comes first. The CLI's token exists only as -//! the fallback for someone who has none, so it is never sent while the user -//! has one that works. Tried highest first: +//! # Who may use the CLI's token //! -//! 1. `--token` or `MAPBOX_ACCESS_TOKEN`, ranked the way a command ranks them -//! (see [`user_token`]). -//! 2. The stored login, read as it is on disk and refreshed, under the -//! credentials lock, only when it is about to expire. -//! 3. `MAPBOX_CLI_TOKEN` at run time, the CLI's token for someone with none -//! of their own, such as a developer pointing at staging. -//! 4. The token published on the release channel, fetched and kept on disk -//! until the Tokens API calls it dead. See [`fetch`] for when it is -//! fetched again. +//! Every request made with the CLI's token is billed to the account that +//! owns it. So what a request is for decides whether it may use the token, +//! not where it goes. Each caller is named in this file with its [`Policy`]: +//! telemetry as [`TELEMETRY_CALLER`], and each command in +//! [`CLI_TOKEN_COMMANDS`] with the reason it qualifies. A URL overridden +//! from the environment keeps the same token. //! -//! A `401` moves on to the next token; any other answer is the caller's. -//! [`send`]'s callers never see the token, so every one of them gets the same -//! fallback without doing anything. +//! # One policy per caller //! -//! # What may use the CLI's token +//! A caller's [`Policy`] decides everything that differs between callers: //! -//! Steps 3 and 4 are the CLI's token, and every request made with it is -//! billed to the account that owns it. So it is decided by what the request -//! is for, not where it goes: a request the CLI makes for itself passes -//! [`send`] an order that ends in [`Fallback::CliToken`], and every such order -//! is defined in this file (today only [`TELEMETRY`]); a command must be in -//! [`CLI_TOKEN_COMMANDS`], which also says how it uses it ([`Use`]). Where -//! the request is sent does not matter, so a URL overridden from the -//! environment keeps the same fallback. +//! - **paced**: count failures in a row and go quiet after three, as +//! [`fetch`] explains. An unpaced caller fetches whenever it needs a token +//! and keeps no count, because someone is waiting on it. +//! - **resends**: after a refusal, send once more with a different, newly +//! fetched token, so a token republished or fixed in the Console works on +//! the next try. That is safe because a refused request had no effect. +//! - **user_fallback**: when the CLI's token can't be used, the caller may +//! send with the user's token instead. Otherwise the request is dropped. //! -//! Either way the token is sent through [`send_with_cli_token`], which holds -//! the rules: check a refusal at once, replace a dead token, rest from an API -//! that refuses a valid one. A command that cannot use the CLI's token falls -//! back to the user's, and with none says why: unavailable right now -//! ([`Unavailable`]) or refused by this API ([`Refusal`]). +//! Telemetry is paced, never resends and never uses the user's token: +//! Mapbox Events stores the token each event was sent with, and its request +//! logs record it as `access_token`. A user's token there would leave a login +//! (`tk.`, with write scopes) or secret (`sk.`) token in those records, and +//! the token would link them together. A command such as `mapbox report` is +//! the opposite on all three. With no user token either, it says why the +//! CLI's could not be used ([`Unavailable`], [`Refusal`]). +//! +//! The token a caller sends is: +//! +//! 1. `MAPBOX_CLI_TOKEN`, when set. A developer sets it to use another CLI +//! token, such as one for staging. It is never checked, counted or +//! replaced, and is sent even while the caller is quiet. A caller without +//! `user_fallback` sends it only if it is a `pk.` token. +//! 2. Otherwise the token published for the release channel, fetched and +//! saved until the Tokens API says it is dead ([`fetch`]). +//! +//! Every send goes through [`send_with_cli_token`], which checks a refusal +//! right away. A dead token is replaced on disk whatever the policy, but +//! only a caller that resends uses the new one in the same run. Telemetry +//! uses it on its next run. //! //! # The token is public //! @@ -42,12 +51,9 @@ //! the minimum scopes, and nothing may ever treat it as a secret. [`fetch`] //! refuses anything that is not `pk.`. -use std::cell::RefCell; - use reqwest::blocking::{RequestBuilder, Response}; -use reqwest::StatusCode; -use crate::{auth, http}; +use crate::http; mod fetch; @@ -60,78 +66,110 @@ pub(crate) use fetch::{Refusal, Unavailable}; /// for a token anyone can read. pub(crate) const REFERER: &str = "https://cli.mapbox.com/"; -/// One place [`send`] may take a token from. A caller passes them in the -/// order to try. +/// How a caller uses the CLI's token. The module docs explain each flag. #[derive(Clone, Copy, Debug, PartialEq, Eq)] -pub(crate) enum Fallback { - /// `--token` or `MAPBOX_ACCESS_TOKEN`, as [`user_token`] ranks them. - UserToken, - /// The stored login, refreshed only when it is about to expire. - Login, - /// `MAPBOX_CLI_TOKEN` at run time, then the fetched token. Billed to the - /// CLI's account, so it only ever comes after every token of the user's. - CliToken, +pub(crate) struct Policy { + paced: bool, + resends: bool, + user_fallback: bool, } -/// Telemetry's name as a caller of the CLI's token, for its cooldown. -pub(crate) const TELEMETRY_CALLER: &str = "telemetry"; - -/// Telemetry's order. The CLI's own usage data, so there is no user's -/// account to bill when the user has no token. -pub(crate) const TELEMETRY: &[Fallback] = - &[Fallback::UserToken, Fallback::Login, Fallback::CliToken]; - -/// Every order defined here, for the test that holds the CLI's token last. -/// An order that uses it belongs in this file and in this list. -#[cfg(test)] -const ORDERS: &[&[Fallback]] = &[TELEMETRY]; - -/// How a command listed in [`CLI_TOKEN_COMMANDS`] uses the CLI's token. -#[derive(Clone, Copy, Debug, PartialEq, Eq)] -pub(crate) enum Use { - /// The CLI's token first, and the user's own only while the CLI's - /// cannot be used: unavailable, resting, or refused by this API. - Preferred, +/// Nobody is waiting, and the event must never carry the user's token. +pub(crate) const TELEMETRY: Policy = Policy { + paced: true, + resends: false, + user_fallback: false, +}; + +/// Someone ran it and is waiting, and their own token can stand in. +pub(crate) const COMMAND: Policy = Policy { + paced: false, + resends: true, + user_fallback: true, +}; + +/// Something that sends with the CLI's token. Failures are counted under +/// its name. +#[derive(Debug, PartialEq, Eq)] +pub(crate) struct Caller { + name: &'static str, + policy: Policy, } +pub(crate) static TELEMETRY_CALLER: Caller = Caller { + name: "telemetry", + policy: TELEMETRY, +}; + /// Commands that may run on the CLI's token, by the name `--schema` reports, -/// each with how and with the reason the CLI's account should carry their -/// use. Command names are a compatibility promise, so an entry does not -/// quietly move to another API when a spec changes its URL. +/// each with the reason the CLI's account should carry their use. Command +/// names are a compatibility promise, so an entry does not quietly move to +/// another API when a spec changes its URL. /// /// A command here must not read its body from stdin: a refused request is /// sent again, with a fresh token or the user's, and stdin can only be read /// once. `cli_token_commands_read_nothing_from_stdin` holds that. -const CLI_TOKEN_COMMANDS: &[(&str, Use, &str)] = &[( - "report", - Use::Preferred, +/// +/// Each has the [`COMMAND`] policy, since `main` falls back to the user's +/// token for every one of them. +const CLI_TOKEN_COMMANDS: &[(Caller, &str)] = &[( + Caller { + name: "report", + policy: COMMAND, + }, "Feedback about the CLI is filed under Mapbox's own account, so it is the \ CLI's request rather than the user's, and it works for someone not \ logged in.", )]; +/// The caller for `command`, or `None` when the command may not use the +/// CLI's token: the only way to get one, so an unlisted command never does. +pub(crate) fn command(command: &str) -> Option<&'static Caller> { + CLI_TOKEN_COMMANDS + .iter() + .find(|(caller, _)| caller.name == command) + .map(|(caller, _)| caller) +} + /// Whether `command` may run on the CLI's token at all. pub(crate) fn may_use(command: &str) -> bool { - use_for(command).is_some() + self::command(command).is_some() } -fn use_for(command: &str) -> Option { - CLI_TOKEN_COMMANDS - .iter() - .find(|(name, _, _)| *name == command) - .map(|(_, how, _)| *how) +impl Caller { + /// The token this caller sends with, by its policy. `fetch` is false + /// where nothing may be sent, as in a dry run, and only a token already + /// on disk is returned then. + pub(crate) fn token(&'static self, fetch: bool) -> Result { + let override_token = std::env::var("MAPBOX_CLI_TOKEN").ok(); + let (token, source) = chosen(self.policy, override_token.as_deref(), || { + if fetch { + fetch::token(self.name, self.policy.paced) + } else { + fetch::saved_token() + .map(|token| (token, TokenSource::Saved)) + .ok_or(Unavailable::NotFetched) + } + })?; + Ok(CliToken { + token, + source, + caller: self, + }) + } } -/// The CLI's token as a command got it, for [`send_for_command`], which -/// handles a refusal of it by where it came from. +/// The CLI's token as a caller got it, for [`send_with`], which handles a +/// refusal of it by where it came from. #[derive(Debug, PartialEq, Eq)] -pub(crate) struct CommandToken { +pub(crate) struct CliToken { pub(crate) token: String, source: TokenSource, + caller: &'static Caller, } #[derive(Debug, Clone, Copy, PartialEq, Eq)] -enum TokenSource { +pub(crate) enum TokenSource { /// `MAPBOX_CLI_TOKEN`. Override, /// The one on disk. @@ -140,134 +178,70 @@ enum TokenSource { Fetched, } -/// The CLI's token for `command`, or `None` when the command is not listed -/// for it: the only way to ask, so an unlisted command never gets one. -/// `fetch` is false where nothing may be sent, as in a dry run, and only a -/// token already on disk is returned then. -pub(crate) fn for_command(command: &str, fetch: bool) -> Option> { - match use_for(command)? { - Use::Preferred => Some(preferred_token(fetch)), - } -} - -fn preferred_token(fetch: bool) -> Result { - let given = |token: String, source| CommandToken { token, source }; - if let Some(token) = usable(std::env::var("MAPBOX_CLI_TOKEN").ok().as_deref()) { - return Ok(given(token.to_owned(), TokenSource::Override)); - } - if let Some(token) = fetch::saved_token() { - return Ok(given(token, TokenSource::Saved)); +/// Prefers `MAPBOX_CLI_TOKEN`, and loads the fetched token only when it is +/// unset, so an override never causes a fetch. +fn chosen( + policy: Policy, + override_token: Option<&str>, + fetched: impl FnOnce() -> Result<(String, TokenSource), Unavailable>, +) -> Result<(String, TokenSource), Unavailable> { + match overridden(policy, override_token) { + // Don't fall back to the fetched token: whoever set the override + // wanted a different one. + Some(token) => token.map(|token| (token, TokenSource::Override)), + None => fetched(), } - if !fetch { - return Err(Unavailable::NotFetched); - } - // A command is not held by the backoff, which paces background fetches: - // someone is waiting, and the endpoint may have a new token by now. - fetch::token_now().map(|token| given(token, TokenSource::Fetched)) -} - -/// Sends a command's request with the CLI's token by [`send_with_cli_token`]'s -/// rules, as [`Disk::Command`] for a token from disk or the endpoint. -/// `MAPBOX_CLI_TOKEN` is sent once and never checked, counted or replaced, -/// as [`send`] treats it: a developer chose that token, and a refusal of it -/// is theirs to see. -pub(crate) fn send_for_command( - command: &str, - given: CommandToken, - send: impl FnMut(&str) -> R, - status: impl Fn(&R) -> Option, -) -> Sent { - let mut ledger = match given.source { - TokenSource::Override => Disk::Unchecked, - TokenSource::Saved => Disk::Command { fetched: false }, - TokenSource::Fetched => Disk::Command { fetched: true }, - }; - send_with_cli_token(command, given.token, &mut ledger, send, status) } -/// The token the user gave this run, ranked as a command ranks it: with -/// `--use-login` only a typed `--token` counts, otherwise the environment -/// does too, `MapboxAccessToken` included, since that is the user's token -/// for `tilesets-cli`. Resolved by the caller, which has the arguments, and -/// handed to [`send`]. -pub(crate) fn user_token(matches: &clap::ArgMatches) -> Option { - if matches.get_flag("use-login") { - auth::typed_token(matches) +/// `MAPBOX_CLI_TOKEN` trimmed, or why `policy` may not send it. `None` when +/// it is unset. +fn overridden(policy: Policy, value: Option<&str>) -> Option> { + let token = value.map(str::trim).filter(|t| !t.is_empty())?; + // A caller that falls back to the user's token sends any override: it + // would send the user's token anyway. + Some(if policy.user_fallback || fetch::is_plausible(token) { + Ok(token.to_owned()) } else { - matches - .get_one::("token") - .cloned() - .or_else(|| auth::environment_token().map(|(_, token)| token)) - } + Err(Unavailable::OverrideNotPublic) + }) } -/// Sends `request` with a token attached as `access_token`, trying the -/// next token whenever the API answers `401`. -/// -/// May block on the credentials lock and refresh the login over the network, -/// as a command does, so call it where nobody is waiting on the answer, such -/// as a detached child. +/// Sends `request` once with the CLI's token as `access_token`, by +/// `caller`'s policy, for a caller with no user token to fall back to. /// -/// `request` is called once per attempt, since a sent request cannot be -/// reused. `None` when there is no token to send with; an error when the -/// request could not be sent at all; otherwise the first answer that was not -/// a `401`, or the last `401` when every token was rejected. +/// Returns `None` when there is no CLI token to use, an error when the +/// request could not be sent, and otherwise the response, even a refusal. pub(crate) fn send( - caller: &str, - order: &[Fallback], - user_token: Option<&str>, - profile: Option<&str>, + caller: &'static Caller, request: impl Fn() -> RequestBuilder, ) -> Option> { - let override_token = std::env::var("MAPBOX_CLI_TOKEN").ok(); - let fetched = RefCell::new(None); - let send = |token: &str| http::send(request().query(&[("access_token", token)])); - // Any CLI token, a replacement for a dead one included. - let send_cli = |token: &str| { + let given = caller.token(true).ok()?; + let send = |token: &str| { http::send( request() .query(&[("access_token", token)]) .header(reqwest::header::REFERER, REFERER), ) }; - let attempt = |token: &str| { - // Only the fetched token goes through the CLI token's rules: the - // user's tokens are never checked or counted, nor is - // `MAPBOX_CLI_TOKEN`, which still carries the CLI token's Referer. - if fetched.borrow().as_deref() != Some(token) { - return if usable(override_token.as_deref()) == Some(token) { - send_cli(token) - } else { - send(token) - }; - } - match send_with_cli_token( - caller, - token.to_owned(), - &mut Disk::Background, - send_cli, - |answer| answer.as_ref().ok().map(|r| r.status().as_u16()), - ) { - Sent::Answered(answer) | Sent::Refused(answer, _) => answer, - } - }; - let fetch = || { - // A caller resting from the CLI's token sends nothing with it. - let token = (!fetch::cooling(caller)).then(fetch::token)??; - *fetched.borrow_mut() = Some(token.clone()); - Some(token) + match send_with(given, send, |answer| { + answer.as_ref().ok().map(|r| r.status().as_u16()) + }) { + Sent::Answered(answer) | Sent::Refused(answer, _) => Some(answer), + } +} + +/// Sends with `given` by [`send_with_cli_token`]'s rules and its caller's +/// policy. +pub(crate) fn send_with( + given: CliToken, + send: impl FnMut(&str) -> R, + status: impl Fn(&R) -> Option, +) -> Sent { + let mut ledger = Disk { + policy: given.caller.policy, + source: given.source, }; - resolve( - order, - Sources { - user: user_token, - override_token: override_token.as_deref(), - }, - || login_token(profile), - fetch, - attempt, - |response| response.status() == StatusCode::UNAUTHORIZED, - ) + send_with_cli_token(given.caller.name, given.token, &mut ledger, send, status) } /// What a request sent with the CLI's token came to. @@ -284,65 +258,57 @@ pub(crate) enum Sent { pub(crate) trait Ledger { fn refused(&mut self, caller: &str, token: &str) -> Refusal; fn accepted(&mut self, caller: &str, token: &str); - /// Whether a request refused for `refusal` is worth sending again with - /// a [`fresh`](Ledger::fresh) token. - fn retries(&self, refusal: Refusal) -> bool; + /// Whether to retry a refused request with a [`fresh`](Ledger::fresh) + /// token. + fn retries(&self) -> bool; fn fresh(&mut self) -> Option; } -/// `/cli-token.json`, through [`fetch`], by kind of caller. -pub(crate) enum Disk { - /// Nobody is waiting, as for telemetry: paced, and a new token is tried - /// only for a dead one, when the backoff allows. - Background, - /// Someone ran it and is waiting: never paced, and any refusal tries a - /// token fetched there and then, so a token republished or fixed in the - /// Console works on the next try. Not when this run `fetched` the token - /// already: asking again at once would only get the same one back. - Command { fetched: bool }, - /// A token not from the endpoint (`MAPBOX_CLI_TOKEN`): the file is not - /// touched, and a refusal is handed back as it is. - Unchecked, +/// `/cli-token.json`, through [`fetch`], as the caller's +/// `policy` and the token's `source` allow. +struct Disk { + policy: Policy, + source: TokenSource, } impl Ledger for Disk { fn refused(&mut self, caller: &str, token: &str) -> Refusal { - match self { - Disk::Background => fetch::refused(caller, token, true), - Disk::Command { .. } => fetch::refused(caller, token, false), - Disk::Unchecked => Refusal::NotFetched, + // A developer chose `MAPBOX_CLI_TOKEN`, so its refusal is theirs to + // see. It is not checked, counted or replaced. + if self.source == TokenSource::Override { + return Refusal::NotFetched; } + fetch::refused(caller, token, self.policy.paced) } fn accepted(&mut self, caller: &str, token: &str) { - if !matches!(self, Disk::Unchecked) { + if self.policy.paced && self.source != TokenSource::Override { fetch::accepted(caller, token); } } - fn retries(&self, refusal: Refusal) -> bool { - match self { - Disk::Background => refusal == Refusal::Dead, - Disk::Command { fetched } => !fetched, - Disk::Unchecked => false, - } + fn retries(&self) -> bool { + // A token this run just fetched would only come back again. + self.policy.resends && self.source == TokenSource::Saved } fn fresh(&mut self) -> Option { - match self { - Disk::Background => fetch::token(), - Disk::Command { .. } => fetch::token_now().ok(), - Disk::Unchecked => None, + // A paced caller's refusal has already fetched again, and fetching + // once more would get around its count. + if self.policy.paced { + fetch::saved_token() + } else { + fetch::token_now().ok() } } } /// Sends with the CLI's `token` on `caller`'s behalf: the one place the -/// rules for using it live, for telemetry and commands alike. +/// rules for using it live, for every caller. /// -/// A `2xx` ends `caller`'s cooldown. A `401` or `403` is checked with the -/// Tokens API at once ([`fetch::refused`]); when the ledger says it is worth -/// it ([`Ledger::retries`]) and a different token is to be had, the request -/// is sent once more with it, since a refused request was not acted on. -/// Otherwise the refusal is handed back. `status` is an answer's HTTP status, -/// or `None` when there was no answer. +/// A `2xx` resets `caller`'s failure count. A `401` or `403` is checked with +/// the Tokens API right away ([`fetch::refused`]). If the ledger allows a +/// retry ([`Ledger::retries`]) and a different token is available, the +/// request is sent once more with it. Otherwise the refusal is returned. +/// `status` gives a response's HTTP status, or `None` when there was no +/// response. pub(crate) fn send_with_cli_token( caller: &str, mut token: String, @@ -356,7 +322,7 @@ pub(crate) fn send_with_cli_token( match status(&answer) { Some(401 | 403) => { let refusal = ledger.refused(caller, &token); - if ledger.retries(refusal) && !replaced { + if ledger.retries() && !replaced { if let Some(fresh) = ledger.fresh().filter(|fresh| *fresh != token) { token = fresh; replaced = true; @@ -372,349 +338,79 @@ pub(crate) fn send_with_cli_token( } } -/// Whether [`send`] has a token it could try in `order`, judged without the -/// network or the credentials lock, so a caller can skip work that would only -/// find nothing to send with. A login counts by being there: it may still turn -/// out expired and unrefreshable, and then nothing is sent. -pub(crate) fn has_token_for( - caller: &str, - order: &[Fallback], - user_token: Option<&str>, - profile: Option<&str>, -) -> bool { - order.iter().any(|fallback| match fallback { - Fallback::UserToken => usable(user_token).is_some(), - Fallback::Login => auth::load_credentials_readonly(profile).is_some(), - Fallback::CliToken => { - usable(std::env::var("MAPBOX_CLI_TOKEN").ok().as_deref()).is_some() - || (!fetch::cooling(caller) && fetch::may_have_token()) - } - }) -} - -/// The stored login's token, refreshed when it is about to expire. -fn login_token(profile: Option<&str>) -> Option { - // Read without the lock first, and used as it is while it is good: - // `load_fresh_credentials` takes the credentials lock, which creates and - // hardens `~/.mapbox`, and a read-only command's telemetry must not be - // what changes it. Only a login about to expire pays for the lock. - let stored = auth::load_credentials_readonly(profile)?; - if !auth::token_needs_refresh(&stored.access_token) { - return Some(stored.access_token); +/// Why [`send`] has no CLI token to try for `caller`, checked without the +/// network so a caller can skip work that could not be sent anyway. `None` +/// when it may have one. +pub(crate) fn no_token_for(caller: &Caller) -> Option<&'static str> { + match overridden( + caller.policy, + std::env::var("MAPBOX_CLI_TOKEN").ok().as_deref(), + ) { + Some(token) => token.err().map(|_| "override_not_public"), + None if caller.policy.paced => fetch::no_token_for(caller.name), + None => None, } - auth::load_fresh_credentials(false, profile).map(|c| c.access_token) -} - -struct Sources<'a> { - user: Option<&'a str>, - override_token: Option<&'a str>, -} - -enum Source { - User, - Login, - Override, - Fetched, -} - -/// The order and the fallback, with the login, the fetched token and the -/// send passed in so each path can be tested without the network or the -/// disk. -/// -/// Tries each [`Fallback`] in `order`. The login and the fetched token are -/// loaded only when their turn comes, and a token that was already rejected -/// is not tried again. A failed send stops here: no other token fixes a -/// request that never arrived. -fn resolve( - order: &[Fallback], - sources: Sources<'_>, - login: impl FnOnce() -> Option, - fetched: impl FnOnce() -> Option, - mut attempt: impl FnMut(&str) -> Result, - rejected: impl Fn(&R) -> bool, -) -> Option> { - let mut login = Some(login); - let mut fetched = Some(fetched); - let mut tried: Vec = Vec::new(); - let mut last = None; - for fallback in order { - // One source per step, each loaded only when the one before it was - // rejected, so a developer's override never costs a fetch. - let steps: &[Source] = match fallback { - Fallback::UserToken => &[Source::User], - Fallback::Login => &[Source::Login], - Fallback::CliToken => &[Source::Override, Source::Fetched], - }; - for source in steps { - let token = match source { - Source::User => usable(sources.user).map(str::to_owned), - Source::Login => login.take().and_then(|load| load()), - Source::Override => usable(sources.override_token).map(str::to_owned), - Source::Fetched => fetched.take().and_then(|load| load()), - }; - let Some(token) = token else { - continue; - }; - if tried.contains(&token) { - continue; - } - let response = match attempt(&token) { - Ok(response) => response, - Err(error) => return Some(Err(error)), - }; - if !rejected(&response) { - return Some(Ok(response)); - } - tried.push(token); - last = Some(response); - } - } - last.map(Ok) -} - -fn usable(token: Option<&str>) -> Option<&str> { - token.map(str::trim).filter(|t| !t.is_empty()) } #[cfg(test)] mod tests { use super::*; - use std::cell::{Cell, RefCell}; - - /// Runs [`resolve`] against a fake API that accepts only `accepted`, and - /// returns the answer, every token sent, and whether the login was loaded. - fn run( - sources: (Sources<'_>, Option<&str>), - login: Option<&str>, - accepted: &[&str], - ) -> (Option, Vec, bool) { - run_in(TELEMETRY, sources, login, accepted) - } - - fn run_in( - order: &[Fallback], - (sources, fetched): (Sources<'_>, Option<&str>), - login: Option<&str>, - accepted: &[&str], - ) -> (Option, Vec, bool) { - let sent = RefCell::new(Vec::new()); - let loaded = Cell::new(false); - let answer = resolve( - order, - sources, - || { - loaded.set(true); - login.map(str::to_owned) - }, - || fetched.map(str::to_owned), - |token| { - sent.borrow_mut().push(token.to_owned()); - Ok::<_, ()>(if accepted.contains(&token) { 200 } else { 401 }) - }, - |status| *status == 401, - ); - (answer.map(|a| a.unwrap()), sent.into_inner(), loaded.get()) - } - - /// The user's token, `MAPBOX_CLI_TOKEN`, and what a fetch would return. - fn sources<'a>( - user: Option<&'a str>, - override_token: Option<&'a str>, - fetched: Option<&'a str>, - ) -> (Sources<'a>, Option<&'a str>) { - ( - Sources { - user, - override_token, - }, - fetched, - ) - } - - #[test] - fn an_accepted_user_token_is_all_that_is_sent() { - let (answer, sent, loaded) = run( - sources(Some("pk.typed"), Some("pk.override"), Some("pk.fetched")), - Some("sk.login"), - &["pk.typed"], - ); - assert_eq!(answer, Some(200)); - assert_eq!(sent, ["pk.typed"]); - assert!( - !loaded, - "the login is not touched when the user's token works" - ); - } - - #[test] - fn the_login_comes_after_the_users_token() { - let (answer, sent, _) = run( - sources(Some("pk.typed"), Some("pk.override"), None), - Some("sk.login"), - &["sk.login"], - ); - assert_eq!(answer, Some(200)); - assert_eq!(sent, ["pk.typed", "sk.login"]); - } - - #[test] - fn the_login_is_used_when_the_user_typed_nothing() { - let (answer, sent, _) = run(sources(None, None, None), Some("sk.login"), &["sk.login"]); - assert_eq!(answer, Some(200)); - assert_eq!(sent, ["sk.login"]); - } - - #[test] - fn the_clis_token_is_sent_only_once_every_user_token_is_rejected() { - let (answer, sent, _) = run( - sources(Some("pk.typed"), Some("pk.override"), Some("pk.fetched")), - Some("sk.login"), - &["pk.override"], - ); - assert_eq!(answer, Some(200)); - assert_eq!(sent, ["pk.typed", "sk.login", "pk.override"]); - } #[test] - fn the_clis_token_is_never_sent_by_an_order_without_it() { - let (answer, sent, _) = run_in( - &[Fallback::UserToken, Fallback::Login], - sources(Some("pk.typed"), Some("pk.override"), Some("pk.fetched")), - None, - &["pk.override", "pk.fetched"], - ); - assert_eq!(answer, Some(401)); - assert_eq!(sent, ["pk.typed"]); - } - - #[test] - fn the_order_given_is_the_order_tried() { - let (_, sent, _) = run_in( - &[Fallback::Login, Fallback::UserToken], - sources(Some("pk.typed"), None, None), - Some("sk.login"), - &[], - ); - assert_eq!(sent, ["sk.login", "pk.typed"]); - } - - #[test] - fn every_order_keeps_the_clis_token_last() { - for order in ORDERS { - if let Some(at) = order.iter().position(|f| *f == Fallback::CliToken) { - assert_eq!(at, order.len() - 1, "{order:?}"); - } + fn the_override_is_sent_trimmed_and_never_fetches() { + for policy in [TELEMETRY, COMMAND] { + let chosen = chosen(policy, Some(" pk.override\n"), || panic!("fetched")); + assert_eq!( + chosen, + Ok(("pk.override".to_owned(), TokenSource::Override)) + ); } } #[test] - fn override_comes_before_fetched() { - let (answer, sent, _) = run( - sources(None, Some("pk.override"), Some("pk.fetched")), - None, - &["pk.override", "pk.fetched"], - ); - assert_eq!(answer, Some(200)); - assert_eq!(sent, ["pk.override"]); - } - - #[test] - fn a_rejected_token_is_not_sent_twice() { - // The login and the typed token can be the same token. - let (answer, sent, _) = run( - sources(Some("sk.same"), None, Some("pk.fetched")), - Some("sk.same"), - &["pk.fetched"], - ); - assert_eq!(answer, Some(200)); - assert_eq!(sent, ["sk.same", "pk.fetched"]); - } - - #[test] - fn every_token_rejected_returns_the_last_answer() { - let (answer, sent, _) = run( - sources(Some("pk.typed"), Some("pk.override"), Some("pk.fetched")), - Some("sk.login"), - &[], - ); - assert_eq!(answer, Some(401)); - assert_eq!(sent, ["pk.typed", "sk.login", "pk.override", "pk.fetched"]); - } - - #[test] - fn a_failed_send_stops_without_trying_other_tokens() { - let mut sent = Vec::new(); - let answer = resolve( - TELEMETRY, - sources(Some("pk.typed"), Some("pk.override"), None).0, - || Some("sk.login".to_owned()), - || Some("pk.fetched".to_owned()), - |token| { - sent.push(token.to_owned()); - Err::("unreachable") - }, - |status| *status == 401, - ); - assert_eq!(answer, Some(Err("unreachable"))); - assert_eq!(sent, ["pk.typed"]); - } - - #[test] - fn blank_tokens_are_skipped_and_the_rest_trimmed() { - let (answer, sent, _) = run( - sources(Some(" "), Some(""), Some("pk.fetched")), - None, - &["pk.fetched"], - ); - assert_eq!(answer, Some(200)); - assert_eq!(sent, ["pk.fetched"]); + fn an_override_that_is_not_public_is_sent_only_with_a_user_fallback() { + for token in ["sk.secret", "tk.login", "pk.has space"] { + assert_eq!( + chosen(TELEMETRY, Some(token), || panic!("fetched")), + Err(Unavailable::OverrideNotPublic), + "{token}" + ); + assert_eq!( + chosen(COMMAND, Some(token), || panic!("fetched")), + Ok((token.to_owned(), TokenSource::Override)), + "{token}" + ); + } } #[test] - fn nothing_to_send_with_sends_nothing() { - let (answer, sent, _) = run(sources(None, None, None), None, &[]); - assert_eq!(answer, None); - assert!(sent.is_empty()); + fn without_an_override_the_fetched_token_is_sent() { + for blank in [None, Some(""), Some(" ")] { + let fetched = || Ok(("pk.fetched".to_owned(), TokenSource::Fetched)); + assert_eq!(chosen(TELEMETRY, blank, fetched), fetched(), "{blank:?}"); + assert_eq!( + chosen(TELEMETRY, blank, || Err(Unavailable::Unreachable)), + Err(Unavailable::Unreachable), + "{blank:?}: nothing to send with" + ); + } } - fn user_token_for(args: &[&str]) -> Option { - let matches = crate::build_app(&[]) - .try_get_matches_from(args) - .expect("arguments parse"); - user_token(&matches) + fn disk(policy: Policy, source: TokenSource) -> Disk { + Disk { policy, source } } - // The `MAPBOX_ACCESS_TOKEN` half is not covered here: setting it would - // race every other test in this process that reads the environment. #[test] - fn a_typed_token_is_the_users_with_or_without_use_login() { - for args in [ - &["mapbox", "--token", "pk.typed", "auth", "whoami"][..], - &[ - "mapbox", - "--use-login", - "--token", - "pk.typed", - "auth", - "whoami", - ][..], + fn telemetry_never_sends_again() { + for source in [ + TokenSource::Override, + TokenSource::Saved, + TokenSource::Fetched, ] { - assert_eq!( - user_token_for(args).as_deref(), - Some("pk.typed"), - "{args:?}" - ); + assert!(!disk(TELEMETRY, source).retries(), "{source:?}"); } } - #[test] - fn use_login_without_a_typed_token_leaves_the_user_none() { - assert_eq!( - user_token_for(&["mapbox", "--use-login", "auth", "whoami"]), - None - ); - } - #[test] fn every_cli_token_command_exists_and_says_why() { let specs = crate::spec::effective_services().expect("the bundled specs parse"); @@ -723,20 +419,30 @@ mod tests { .flat_map(|svc| &svc.operations) .map(|op| op.command()) .collect(); - for (name, _, reason) in CLI_TOKEN_COMMANDS { + for (caller, reason) in CLI_TOKEN_COMMANDS { + let name = caller.name; assert!( - commands.iter().any(|c| c == name), + commands.iter().any(|c| *c == name), "`{name}` is in CLI_TOKEN_COMMANDS but is not a command" ); assert!(!reason.trim().is_empty(), "`{name}` needs a reason"); } } + /// `main` sends a command with the user's token when the CLI's can't be + /// used, and has no path for a command that must not. + #[test] + fn every_cli_token_command_has_the_command_policy() { + for (caller, _) in CLI_TOKEN_COMMANDS { + assert_eq!(caller.policy, COMMAND, "`{}`", caller.name); + } + } + #[test] fn cli_token_commands_read_nothing_from_stdin() { let specs = crate::spec::effective_services().expect("the bundled specs parse"); for op in specs.iter().flat_map(|svc| &svc.operations) { - if use_for(&op.command()).is_none() { + if !may_use(&op.command()) { continue; } let cmd = crate::build_operation_command(op); @@ -753,16 +459,15 @@ mod tests { #[test] fn an_unlisted_command_never_gets_the_clis_token() { - assert_eq!(for_command("styles list", true), None); + assert_eq!(command("styles list"), None); + assert!(!may_use("styles list")); } - /// A ledger that answers every refusal with `verdict` and has `fresh` - /// to hand out once, and records what it was told. + /// Returns `verdict` for every refusal and hands out `fresh` once. + /// Records every token it is given. #[derive(Default)] struct FakeLedger { - /// Retries after any refusal, as [`Disk::Command`] does, rather than - /// only a dead token, as [`Disk::Background`] does. - command: bool, + retries: bool, verdict: Option, fresh: Option, refused: Vec, @@ -777,14 +482,24 @@ mod tests { fn accepted(&mut self, _: &str, token: &str) { self.accepted.push(token.to_owned()); } - fn retries(&self, refusal: Refusal) -> bool { - self.command || refusal == Refusal::Dead + fn retries(&self) -> bool { + self.retries } fn fresh(&mut self) -> Option { self.fresh.take() } } + /// Acts like [`Disk`] for a [`COMMAND`] token loaded from disk. + fn command_ledger(verdict: Refusal, fresh: &str) -> FakeLedger { + FakeLedger { + retries: true, + verdict: Some(verdict), + fresh: Some(fresh.to_owned()), + ..FakeLedger::default() + } + } + /// Sends through [`send_with_cli_token`] to a fake API that answers /// `statuses` in turn, and returns the outcome and the tokens sent. fn send_cli( @@ -807,108 +522,73 @@ mod tests { } #[test] - fn a_dead_token_is_replaced_and_sent_once_more() { - let mut ledger = FakeLedger { - verdict: Some(Refusal::Dead), - fresh: Some("pk.new".to_owned()), - ..FakeLedger::default() - }; - let (outcome, sent) = send_cli(&mut ledger, &[Some(401), Some(200)]); - assert_eq!(outcome, Sent::Answered(Some(200))); - assert_eq!(sent, ["pk.old", "pk.new"]); - assert_eq!(ledger.accepted, ["pk.new"]); + fn a_command_tries_a_fresh_token_after_any_refusal() { + for verdict in [ + Refusal::Dead, + Refusal::Forbidden, + Refusal::Unknown, + Refusal::NotFetched, + ] { + let mut ledger = command_ledger(verdict, "pk.new"); + let (outcome, sent) = send_cli(&mut ledger, &[Some(403), Some(200)]); + assert_eq!(outcome, Sent::Answered(Some(200)), "{verdict:?}"); + assert_eq!(sent, ["pk.old", "pk.new"], "{verdict:?}"); + assert_eq!(ledger.accepted, ["pk.new"], "{verdict:?}"); + } } #[test] - fn a_replacement_that_is_refused_too_is_not_replaced_again() { - let mut ledger = FakeLedger { - verdict: Some(Refusal::Dead), - fresh: Some("pk.new".to_owned()), - ..FakeLedger::default() - }; + fn a_commands_replacement_that_is_refused_too_is_not_replaced_again() { + let mut ledger = command_ledger(Refusal::Dead, "pk.new"); let (outcome, sent) = send_cli(&mut ledger, &[Some(401), Some(401)]); assert_eq!(outcome, Sent::Refused(Some(401), Refusal::Dead)); assert_eq!(sent, ["pk.old", "pk.new"]); } #[test] - fn a_dead_token_with_no_new_one_is_handed_back() { - let mut ledger = FakeLedger { - verdict: Some(Refusal::Dead), - fresh: Some("pk.old".to_owned()), - ..FakeLedger::default() - }; - let (outcome, sent) = send_cli(&mut ledger, &[Some(403)]); - assert_eq!(outcome, Sent::Refused(Some(403), Refusal::Dead)); - assert_eq!(sent, ["pk.old"], "the same token is not sent again"); - } - - #[test] - fn a_token_this_api_refuses_is_handed_back_without_a_fetch() { - for verdict in [Refusal::Forbidden, Refusal::Unknown] { - let mut ledger = FakeLedger { - verdict: Some(verdict), - fresh: Some("pk.new".to_owned()), - ..FakeLedger::default() - }; + fn a_command_gets_the_same_token_back_and_does_not_resend_it() { + for verdict in [Refusal::Dead, Refusal::Forbidden] { + let mut ledger = command_ledger(verdict, "pk.old"); let (outcome, sent) = send_cli(&mut ledger, &[Some(403)]); assert_eq!(outcome, Sent::Refused(Some(403), verdict)); - assert_eq!(sent, ["pk.old"]); - assert!(ledger.fresh.is_some(), "{verdict:?} fetched a new token"); + assert_eq!(sent, ["pk.old"], "the same token is not sent again"); } } #[test] - fn a_command_tries_a_fresh_token_after_any_refusal() { - for verdict in [Refusal::Forbidden, Refusal::Unknown, Refusal::NotFetched] { + fn in_the_background_any_refusal_is_handed_back_without_a_resend() { + for verdict in [Refusal::Dead, Refusal::Forbidden, Refusal::Unknown] { let mut ledger = FakeLedger { - command: true, verdict: Some(verdict), fresh: Some("pk.new".to_owned()), ..FakeLedger::default() }; - let (outcome, sent) = send_cli(&mut ledger, &[Some(403), Some(200)]); - assert_eq!(outcome, Sent::Answered(Some(200)), "{verdict:?}"); - assert_eq!(sent, ["pk.old", "pk.new"], "{verdict:?}"); + let (outcome, sent) = send_cli(&mut ledger, &[Some(403)]); + assert_eq!(outcome, Sent::Refused(Some(403), verdict)); + assert_eq!(sent, ["pk.old"]); + assert_eq!(ledger.refused, ["pk.old"], "{verdict:?} not checked"); + assert!(ledger.fresh.is_some(), "{verdict:?} asked to resend"); } } - #[test] - fn a_command_gets_the_same_token_back_and_does_not_resend_it() { - let mut ledger = FakeLedger { - command: true, - verdict: Some(Refusal::Forbidden), - fresh: Some("pk.old".to_owned()), - ..FakeLedger::default() - }; - let (outcome, sent) = send_cli(&mut ledger, &[Some(403)]); - assert_eq!(outcome, Sent::Refused(Some(403), Refusal::Forbidden)); - assert_eq!(sent, ["pk.old"]); - } - #[test] fn a_command_token_just_fetched_or_overridden_is_not_replaced() { - for (ledger, retries) in [ - (Disk::Command { fetched: false }, true), - (Disk::Command { fetched: true }, false), - (Disk::Unchecked, false), + for (source, retries) in [ + (TokenSource::Saved, true), + (TokenSource::Fetched, false), + (TokenSource::Override, false), ] { - for refusal in [ - Refusal::Dead, - Refusal::Forbidden, - Refusal::Unknown, - Refusal::NotFetched, - ] { - assert_eq!(ledger.retries(refusal), retries, "{refusal:?}"); - } + assert_eq!(disk(COMMAND, source).retries(), retries, "{source:?}"); } } #[test] fn an_override_is_never_checked_or_replaced() { - let mut ledger = Disk::Unchecked; - assert_eq!(ledger.refused("report", "pk.override"), Refusal::NotFetched); - assert_eq!(ledger.fresh(), None); + for policy in [TELEMETRY, COMMAND] { + let mut ledger = disk(policy, TokenSource::Override); + assert_eq!(ledger.refused("report", "pk.override"), Refusal::NotFetched); + assert!(!ledger.retries()); + } } #[test] @@ -922,33 +602,4 @@ mod tests { assert!(ledger.refused.is_empty(), "{status:?} was checked"); } } - - #[test] - fn the_override_is_trimmed_before_it_is_sent() { - let (answer, sent, _) = run( - sources(None, Some(" pk.override\n"), None), - None, - &["pk.override"], - ); - assert_eq!(answer, Some(200)); - assert_eq!(sent, ["pk.override"]); - } - - #[test] - fn an_accepted_override_never_fetches() { - let fetched = Cell::new(false); - let answer = resolve( - &[Fallback::CliToken], - sources(None, Some("pk.override"), None).0, - || None, - || { - fetched.set(true); - Some("pk.fetched".to_owned()) - }, - |_| Ok::<_, ()>(200), - |status| *status == 401, - ); - assert_eq!(answer, Some(Ok(200))); - assert!(!fetched.get()); - } } diff --git a/src/cli_token/fetch.rs b/src/cli_token/fetch.rs index 167e28e..a9f10f3 100644 --- a/src/cli_token/fetch.rs +++ b/src/cli_token/fetch.rs @@ -2,25 +2,45 @@ //! kept on disk until the Tokens API calls it dead. //! //! The token is published at `https://cli.mapbox.com/config/token.json` as -//! `{"token": "pk...."}`, so rotating it needs no release. It is fetched -//! when there is none on disk or the one there is dead, and saved to -//! `/cli-token.json`. +//! `{"token": "pk...."}`, so rotating it needs no release. It is saved to +//! `/cli-token.json`, and fetched again when the saved one is +//! missing or dead, or right after a paced caller is refused. //! -//! # When the endpoint has nothing new +//! # A refusal //! //! A `401` or `403` alone does not say the token is dead: the API may only be //! refusing it for one endpoint. So [`refused`] asks the Tokens API about //! the token itself, and only a revoked, invalid, expired or malformed one is -//! replaced. A fetch right after a -//! rotation can still return the revoked token: the edge caches the file for -//! a few minutes, and the secret may not be republished yet. So the dead -//! token served again is checked once more, since a token called invalid for -//! something fixed in the Console stays the same string; it is taken back if -//! the Tokens API now calls it valid. When fetches answer with nothing usable -//! three times running, the next waits an hour ([`backoff`]). A fetch that -//! never got an answer -//! does not count: that is this machine being offline, not the endpoint -//! being stale, and it is asked again on the next run. +//! marked dead. A fetch right after a rotation can still return the revoked +//! token, because the CDN caches the file for a few minutes and the new +//! token may not be published yet. A token marked invalid can also be fixed +//! in the Console without changing. So when the endpoint serves the dead +//! token again, it is checked once more and used again if the Tokens API now +//! says it is valid. +//! +//! # Quiet periods, per caller +//! +//! The token is shared, but each paced caller (today only telemetry) keeps +//! its own count of failures in a row ([`Failures`]). A failure is a fetch +//! that found no usable token, or a refusal after which the fetch did not +//! return a different token. A successful request resets the count. After +//! three failures in a row the caller goes quiet for an hour: it sends and +//! fetches nothing. So while nothing changes, a caller contacts the endpoint +//! at most three times an hour. +//! +//! A new token resets every caller, whichever caller saved it. Each count +//! remembers the token it was made with, and a count made with a token that +//! is no longer on disk is treated as zero. So a caller that is quiet tries +//! again on its next run once anyone saves a different token. The same token +//! saved again resets nobody. +//! +//! One caller's failures never stop another caller. An unpaced caller, such +//! as `mapbox report`, keeps no count and never goes quiet. +//! +//! A fetch that gets no response, with no token on disk, does not count: the +//! machine is probably offline, and the next run tries again. After a +//! refusal it does count, because the API just responded, and sending a +//! refused token on every run would cost a check each time. //! //! # Where it is fetched from //! @@ -41,6 +61,7 @@ use std::time::{Duration, SystemTime, UNIX_EPOCH}; use serde::{Deserialize, Serialize}; +use super::TokenSource; use crate::{auth, http}; const PRODUCTION_URL: &str = "https://cli.mapbox.com/config/token.json"; @@ -59,8 +80,7 @@ const STATE_FILE: &str = "cli-token.json"; const FETCH_TIMEOUT: Duration = Duration::from_secs(5); const HOUR: u64 = 60 * 60; -/// Misses in a row before an hour's wait. -const TRIES_BEFORE_WAITING: u32 = 3; +const FAILURES_BEFORE_QUIET: u32 = 3; /// Long enough for any `pk.` token; anything longer is not one. const MAX_TOKEN_LEN: usize = 512; @@ -77,6 +97,12 @@ pub(crate) enum Unavailable { NoToken { revoked: bool }, /// None on disk, and the caller asked not to fetch, as a dry run does. NotFetched, + /// The caller failed three times in a row and is quiet for up to an + /// hour. + Quiet, + /// `MAPBOX_CLI_TOKEN` is not a `pk.` token, and the caller may only + /// send a public one. + OverrideNotPublic, } impl std::fmt::Display for Unavailable { @@ -90,11 +116,12 @@ impl std::fmt::Display for Unavailable { write!(f, "the token endpoint did not return a usable token") } Self::NotFetched => write!(f, "it has not been fetched yet"), + Self::Quiet => write!(f, "it failed several times in a row"), + Self::OverrideNotPublic => write!(f, "MAPBOX_CLI_TOKEN is not a public token"), } } } -/// What a fetch got back, before [`acquire`] judges it. #[derive(Debug)] pub(crate) enum Fetched { /// The body's `token`, unchecked. @@ -106,6 +133,11 @@ pub(crate) enum Fetched { } /// What is kept on disk between runs. +/// +/// An earlier build also stored a backoff shared by all callers (`failures`, +/// `retry_at`) and one per caller (`cooldowns`). Those fields are ignored so +/// that nobody waits on an old backoff after an upgrade. The next write +/// removes them. #[derive(Debug, Default, Clone, PartialEq, Eq, Serialize, Deserialize)] struct State { #[serde(default, skip_serializing_if = "Option::is_none")] @@ -115,22 +147,34 @@ struct State { /// Cleared if the endpoint serves it again and it now checks valid. #[serde(default)] rejected: bool, - /// Who is resting from using `token`, by the name [`cooling`] takes: - /// an API refused it while the Tokens API called it valid. Cleared with - /// a new token, which that API may accept. + /// Each paced caller's failures, keyed by its name. #[serde(default, skip_serializing_if = "BTreeMap::is_empty")] - cooldowns: BTreeMap, + callers: BTreeMap, /// Whether it was written with an endpoint overridden ([`overridden`]). /// A dev build pointed at staging and an installed release share this /// file, and neither is to trust the other's token or verdicts. #[serde(default)] overridden: bool, - /// Consecutive fetches that found nothing usable. +} + +/// One caller's failures in a row, and when its quiet period ends. A quiet +/// period lasts at most an hour, because a valid token that is refused can +/// be fixed in the Console without changing, and the fix should take effect +/// soon. +#[derive(Debug, Default, Clone, PartialEq, Eq, Serialize, Deserialize)] +struct Failures { #[serde(default)] - failures: u32, - /// Unix seconds before which the endpoint is not asked. + count: u32, + /// Unix seconds before which the caller sends and fetches nothing. #[serde(default)] - retry_at: u64, + quiet_until: u64, + /// The token on disk when these failures happened. They count only while + /// it is still the one on disk. This is what keeps a reset safe when two + /// runs save at once: a run that still had the old token can write its + /// count back over the reset, but that count then names the old token + /// and holds nobody back. + #[serde(default, skip_serializing_if = "Option::is_none")] + token: Option, } impl State { @@ -140,78 +184,118 @@ impl State { .filter(|token| !self.rejected && is_plausible(token)) } - fn fail(&mut self, now: u64) { - self.failures += 1; - self.retry_at = now + backoff(self.failures); + /// `caller`'s failures, unless a different token was saved since. + fn failures(&self, caller: &str) -> Option<&Failures> { + self.callers + .get(caller) + .filter(|failures| failures.token == self.token) } -} -/// One caller's rest from the CLI's token, after an API refused it and the -/// Tokens API could not call it dead, paced as the fetch is ([`backoff`]). -/// Never longer than an hour: a token that is valid but refused is a -/// configuration someone can fix in the Console without the token changing, -/// and a fix should not wait long to be seen. -#[derive(Debug, Default, Clone, PartialEq, Eq, Serialize, Deserialize)] -struct Cooldown { - /// Consecutive refusals. - #[serde(default)] - failures: u32, - /// Unix seconds before which it is not used again. - #[serde(default)] - until: u64, -} + fn quiet(&self, caller: &str, now: u64) -> bool { + self.failures(caller) + .is_some_and(|failures| holds(failures.quiet_until, now)) + } -/// The wait after `failures` consecutive misses: an hour after every third, -/// none otherwise, so a miss that was only bad timing costs nothing. -fn backoff(failures: u32) -> u64 { - if failures.is_multiple_of(TRIES_BEFORE_WAITING) { - HOUR - } else { - 0 + fn fail(&mut self, caller: &str, now: u64) { + let token = self.token.clone(); + let failures = self.callers.entry(caller.to_owned()).or_default(); + if failures.token != token { + *failures = Failures { + token, + ..Failures::default() + }; + } + failures.count += 1; + if failures.count.is_multiple_of(FAILURES_BEFORE_QUIET) { + failures.quiet_until = now + HOUR; + } } -} -/// Whether `caller` — today only `telemetry` — is resting from the -/// CLI's token. While it is, it sends nothing with it, so nothing is -/// refused, and nothing is checked. -pub(crate) fn cooling(caller: &str) -> bool { - read_state().is_some_and(|state| { - state - .cooldowns + /// Ends `caller`'s count, unless `token` is no longer the one on disk: + /// a run that succeeded with an old token says nothing about the new one. + fn accept(&mut self, caller: &str, token: &str) { + if self.token.as_deref() != Some(token) { + return; + } + if self + .callers .get(caller) - .is_some_and(|cooldown| holds(cooldown.until, now())) + .is_some_and(|failures| failures.token.as_deref().is_none_or(|t| t == token)) + { + self.callers.remove(caller); + } + } + + /// Without the failures that no longer count, so the file doesn't keep + /// them. + fn without_stale(mut self) -> State { + let token = self.token.clone(); + self.callers.retain(|_, failures| failures.token == token); + self + } +} + +/// The CLI's token for `caller`: the saved one if it is not dead, otherwise +/// a new one from the endpoint. A `paced` caller gets nothing while it is +/// quiet, and a fetch that finds no usable token counts as its failure. +pub(crate) fn token(caller: &str, paced: bool) -> Result<(String, TokenSource), Unavailable> { + decided(caller, |state, now| { + acquire(state, caller, paced, now, || get(&endpoint()), check) }) } -/// Records that `caller`'s request with the saved `token` went through, which -/// ends any cooldown it was counting. -pub(crate) fn accepted(caller: &str, token: &str) { - if let Some(mut state) = read_state().filter(|state| state.token.as_deref() == Some(token)) { - if state.cooldowns.remove(caller).is_some() { - write_state(&state); +fn decided(caller: &str, decide: impl FnOnce(&mut State, u64) -> T) -> T { + let mut state = read_state().unwrap_or_default(); + let before = state.clone(); + let decision = decide(&mut state, now()); + save(&before, state, Some(caller)); + decision +} + +/// [`token`]'s logic, with the clock, the endpoint and the Tokens API passed +/// in for tests. +fn acquire( + state: &mut State, + caller: &str, + paced: bool, + now: u64, + get: impl FnOnce() -> Fetched, + check: impl FnOnce(&str) -> Refusal, +) -> Result<(String, TokenSource), Unavailable> { + if paced && state.quiet(caller, now) { + return Err(Unavailable::Quiet); + } + if let Some(token) = state.usable_token() { + return Ok((token.to_owned(), TokenSource::Saved)); + } + let revoked = state.rejected; + let fetched = get(); + if matches!(fetched, Fetched::Failed) { + return Err(Unavailable::Unreachable); + } + match judge(state, fetched, check) { + Some(token) => Ok((token, TokenSource::Fetched)), + None => { + if paced { + state.fail(caller, now); + } + Err(Unavailable::NoToken { revoked }) } } } -/// The CLI's token: the one on disk while it is not dead, otherwise a fresh -/// one from the endpoint unless it is being waited for. `None` when there is -/// none to be had right now. -pub(crate) fn token() -> Option { - let mut state = read_state().unwrap_or_default(); - let before = state.clone(); - let token = acquire(&mut state, now(), || get(&endpoint()), check); - if state != before { - save_fetch(&before, state); +/// Resets `caller`'s failure count after a successful request with `token`. +pub(crate) fn accepted(caller: &str, token: &str) { + if let Some(mut state) = read_state() { + let before = state.clone(); + state.accept(caller, token); + save(&before, state, Some(caller)); } - token } -/// [`token`], but asking the endpoint now, whatever the backoff says and -/// even with a token on disk: for a command someone is waiting on, so that a -/// token republished or reconfigured is picked up at once. -/// -/// Only a token it gets is saved. A miss is not counted: the backoff paces -/// background fetches, and a command retried by hand must not lengthen it. +/// Fetches the CLI's token now, even if one is saved, for an unpaced caller +/// that resends after a refusal. Quiet periods don't apply, and no failure +/// is counted. pub(crate) fn token_now() -> Result { let mut state = read_state().unwrap_or_default(); let before = state.clone(); @@ -222,10 +306,8 @@ pub(crate) fn token_now() -> Result { revoked: state.rejected, }, }; - let token = judge(&mut state, now(), fetched, check).ok_or(why)?; - if state != before { - save_fetch(&before, state); - } + let token = judge(&mut state, fetched, check).ok_or(why)?; + save(&before, state, None); Ok(token) } @@ -235,67 +317,81 @@ pub(crate) fn saved_token() -> Option { read_state()?.usable_token().map(str::to_owned) } -/// Writes what a fetch decided, unless another run decided first. The fetch -/// can take seconds: if the token, its verdict or the count changed on disk -/// meanwhile, that run's answer stands. Cooldowns another run added are -/// kept, unless this fetch brought a new token, which ends them all. -fn save_fetch(before: &State, state: State) { - if let Some(state) = merged(before, state, read_state().unwrap_or_default()) { - write_state(&state); +/// Merges what this run decided into what is on disk now. A fetch or a +/// check can take seconds, and another run may have written the file +/// meanwhile. If that run changed the token or its verdict, its change wins, +/// so a token another run saved is never lost. `caller`'s count is written +/// as this run left it; other callers' counts are kept from disk. +/// +/// Two runs of the same caller that fail at the same time can read the same +/// count and record one failure instead of two. That only allows an extra +/// try before the quiet period; it never starts one early. +fn save(before: &State, state: State, caller: Option<&str>) { + if state != *before { + write_state(&merged( + before, + state, + read_state().unwrap_or_default(), + caller, + )); } } -/// [`save_fetch`]'s decision: what to write, given the state the fetch -/// started from, what it decided, and what is on disk now. -fn merged(before: &State, mut state: State, disk: State) -> Option { - let same_fetch = disk.token == before.token - && disk.rejected == before.rejected - && disk.failures == before.failures - && disk.retry_at == before.retry_at; - if !same_fetch { - return None; +/// The state [`save`] writes, from the state this run started with, what it +/// decided, and what is on disk now. +fn merged(before: &State, state: State, mut disk: State, caller: Option<&str>) -> State { + if disk.token == before.token && disk.rejected == before.rejected { + disk.token = state.token; + disk.rejected = state.rejected; } - if state.token == before.token { - state.cooldowns = disk.cooldowns; + if let Some(caller) = caller { + let ours = state.callers.get(caller); + if ours != before.callers.get(caller) { + match ours { + Some(failures) => disk.callers.insert(caller.to_owned(), failures.clone()), + None => disk.callers.remove(caller), + }; + } } - Some(state) + disk } -/// Whether [`token`] could have one, judged without the network. -pub(crate) fn may_have_token() -> bool { +/// Why [`token`] has no token for a paced `caller`, checked without the +/// network. Today the only reason is `quiet`. `None` when it may have one. +pub(crate) fn no_token_for(caller: &str) -> Option<&'static str> { let state = read_state().unwrap_or_default(); - state.usable_token().is_some() || !holds(state.retry_at, now()) + state.quiet(caller, now()).then_some("quiet") } /// What a `401` or `403` to the CLI's token meant. #[derive(Debug, Clone, Copy, PartialEq, Eq)] pub(crate) enum Refusal { /// The token itself is revoked, invalid, expired or malformed. It is - /// marked, so the next [`token`] fetches a new one. + /// marked dead, so the next fetch replaces it. Dead, /// The token is fine; this API does not accept it. Forbidden, - /// The Tokens API could not say. The token is kept, and the caller rests - /// as for [`Refusal::Forbidden`]: sending it again at once would most - /// likely be refused again, and cost another check. + /// The Tokens API could not say. The token is kept but counts as a + /// failure for a paced caller, as with [`Refusal::Forbidden`]: + /// sending it again right away would likely be refused again and cost + /// another check. Unknown, /// Not checked: `token` is not the one on disk. Another run saved a new /// one meanwhile, or the state cannot be written at all, in which case - /// nothing is marked or paced and each run fetches as if for the first + /// nothing is marked or counted and each run fetches as if for the first /// time (see [`write_state`]). NotFetched, } /// Asks the Tokens API, at once, why `caller`'s request with the fetched -/// `token` was refused. A dead token is marked, so the next [`token`] -/// fetches a new one; otherwise `caller`'s refusals are counted, and the -/// third in a row rests it for an hour. +/// `token` was refused, and marks a dead token. /// -/// Only a caller that `rests` is paced: one in the background, such as -/// telemetry. A command someone is waiting on is never held back; it tries -/// a fresh token instead ([`token_now`]). -pub(crate) fn refused(caller: &str, token: &str, rests: bool) -> Refusal { - let now = now(); +/// For a `paced` caller such as telemetry, it then fetches the token again +/// right away, so a rotated or republished token is saved for the next run. +/// Anything but a different token counts as a failure. An unpaced caller +/// keeps no count and fetches a fresh token itself if it resends +/// ([`token_now`]). +pub(crate) fn refused(caller: &str, token: &str, paced: bool) -> Refusal { if read_state() .filter(|state| state.token.as_deref() == Some(token)) .is_none() @@ -305,22 +401,37 @@ pub(crate) fn refused(caller: &str, token: &str, rests: bool) -> Refusal { let refusal = check(token); // Read again: the check can take seconds, and another run may have // saved a new token meanwhile, which this must not write over. - if let Some(mut state) = read_state().filter(|state| state.token.as_deref() == Some(token)) { - // `verdict` never answers `NotFetched`; that was returned above. - if refusal == Refusal::Dead { + if refusal == Refusal::Dead { + if let Some(mut state) = read_state().filter(|state| state.token.as_deref() == Some(token)) + { state.rejected = true; - } else if !rests { - return refusal; - } else { - let cooldown = state.cooldowns.entry(caller.to_owned()).or_default(); - cooldown.failures += 1; - cooldown.until = now + backoff(cooldown.failures); + write_state(&state); } - write_state(&state); + } + if paced { + decided(caller, |state, now| { + refetch(state, caller, token, now, get(&endpoint()), check); + }); } refusal } +/// The fetch after a paced caller's `refused` token, with its answer and +/// the Tokens API passed in. No token, the same token again, or the dead +/// token still dead counts as one failure. +fn refetch( + state: &mut State, + caller: &str, + refused: &str, + now: u64, + fetched: Fetched, + check: impl FnOnce(&str) -> Refusal, +) { + if judge(state, fetched, check).is_none_or(|token| token == refused) { + state.fail(caller, now); + } +} + /// What the Tokens API says about `token`. Never [`Refusal::NotFetched`]. fn check(token: &str) -> Refusal { verdict(auth::token_code(&tokens_url(), token, Some(super::REFERER), FETCH_TIMEOUT).as_deref()) @@ -344,66 +455,46 @@ fn verdict(code: Option<&str>) -> Refusal { } } -/// The decision, with the clock, the endpoint and the Tokens API passed in. -fn acquire( - state: &mut State, - now: u64, - get: impl FnOnce() -> Fetched, - check: impl FnOnce(&str) -> Refusal, -) -> Option { - if let Some(token) = state.usable_token() { - return Some(token.to_owned()); - } - if holds(state.retry_at, now) { - return None; - } - judge(state, now, get(), check) -} - -/// What a fetch's answer does to the state. A token that is new, or the dead -/// one taken back, replaces what is kept, cooldowns and counts with it; the -/// kept token again, not dead, leaves it as it is. +/// Applies a fetch's result to the saved token and returns the token to use, +/// if any. A new token replaces the saved one, which also resets every +/// caller's count ([`State::failures`]). The dead token served again is used +/// only if it now checks valid. fn judge( state: &mut State, - now: u64, fetched: Fetched, check: impl FnOnce(&str) -> Refusal, ) -> Option { - match fetched { - Fetched::Failed => None, - Fetched::Token(token) - if is_plausible(&token) - && (!(state.rejected && state.token.as_deref() == Some(token.as_str())) - || check(&token) == Refusal::Forbidden) => - { - if state.rejected || state.token.as_deref() != Some(token.as_str()) { - *state = State { - token: Some(token.clone()), - ..State::default() - }; + let Fetched::Token(token) = fetched else { + return None; + }; + if !is_plausible(&token) { + return None; + } + if state.token.as_deref() == Some(token.as_str()) { + if state.rejected { + if check(&token) != Refusal::Forbidden { + return None; } - Some(token) - } - // The rejected token again and still not valid, something that is - // not a `pk.` token, or no token at all. - Fetched::Token(_) | Fetched::Nothing => { - state.fail(now); - None + state.rejected = false; } + } else { + state.token = Some(token.clone()); + state.rejected = false; } + Some(token) } -/// Whether a wait until `until` still holds at `now`: the endpoint's, or a -/// caller's cooldown. One further out than the longest wait, an hour, is a -/// clock that went backwards or an edited file, and is not honored. +/// Whether a quiet period that ends at `until` is still on at `now`. An end +/// more than an hour away means the clock went backwards or the file was +/// edited, so it is ignored. fn holds(until: u64, now: u64) -> bool { now < until && until <= now + HOUR } /// Whether `value` looks like a public token and nothing else. Checked on -/// what the endpoint returns and on what is read back from disk, since both -/// end up in a request. -fn is_plausible(value: &str) -> bool { +/// what the endpoint returns, what is read back from disk, and telemetry's +/// `MAPBOX_CLI_TOKEN`, since each ends up in a request. +pub(super) fn is_plausible(value: &str) -> bool { value.starts_with("pk.") && value.len() <= MAX_TOKEN_LEN && value @@ -456,7 +547,7 @@ fn read_state() -> Option { fn write_state(state: &State) { let state = &State { overridden: overridden(), - ..state.clone() + ..state.clone().without_stale() }; let Some(dir) = auth::config_dir_path() else { return; @@ -488,6 +579,7 @@ mod tests { use super::*; const NOW: u64 = 1_000_000; + const CALLER: &str = "telemetry"; fn saved(token: &str, rejected: bool) -> State { State { @@ -497,9 +589,31 @@ mod tests { } } - /// Runs [`acquire`] with an endpoint that answers `answer` and a Tokens - /// API that still calls the token dead, and says whether the endpoint was - /// asked. + /// `state` with `caller` failing `count` times in a row with its token. + fn failing(mut state: State, caller: &str, count: u32, quiet_until: u64) -> State { + let token = state.token.clone(); + state.callers.insert( + caller.to_owned(), + Failures { + count, + quiet_until, + token, + }, + ); + state + } + + fn with_failures(state: State, count: u32, quiet_until: u64) -> State { + failing(state, CALLER, count, quiet_until) + } + + fn count(state: &State) -> u32 { + state.failures(CALLER).map_or(0, |failures| failures.count) + } + + /// Runs [`acquire`] for telemetry, with the endpoint returning `answer` + /// and the Tokens API calling the token dead. Also returns whether the + /// endpoint was asked. fn run(state: &mut State, now: u64, answer: Fetched) -> (Option, bool) { let (result, asked, _) = run_checked(state, now, answer, Refusal::Dead); (result, asked) @@ -513,9 +627,24 @@ mod tests { answer: Fetched, verdict: Refusal, ) -> (Option, bool, bool) { + let (result, asked, checked) = run_as(state, CALLER, true, now, answer, verdict); + (result.ok().map(|(token, _)| token), asked, checked) + } + + /// Runs [`acquire`] for any caller and policy. + fn run_as( + state: &mut State, + caller: &str, + paced: bool, + now: u64, + answer: Fetched, + verdict: Refusal, + ) -> (Result<(String, TokenSource), Unavailable>, bool, bool) { let (mut asked, mut checked) = (false, false); let result = acquire( state, + caller, + paced, now, || { asked = true; @@ -552,49 +681,73 @@ mod tests { #[test] fn a_rejected_token_is_replaced_by_a_new_one() { - let mut state = saved("pk.old", true); + let mut state = with_failures(saved("pk.old", true), 2, 0); let (result, _) = run(&mut state, NOW, token("pk.new")); assert_eq!(result, Some("pk.new".to_owned())); - assert_eq!(state, saved("pk.new", false)); + assert_eq!( + state.without_stale(), + saved("pk.new", false), + "a new token starts over" + ); } #[test] - fn the_rejected_token_again_waits_an_hour_after_every_third_time() { + fn fetches_with_no_usable_token_quiet_the_caller_after_every_third() { let mut state = saved("pk.old", true); let mut now = NOW; - let mut waits = Vec::new(); + let mut quiet = Vec::new(); for _ in 0..8 { let (result, asked) = run(&mut state, now, token("pk.old")); assert!(asked); assert_eq!(result, None); - waits.push(state.retry_at - now); - now = state.retry_at; + let until = state.callers[CALLER].quiet_until; + quiet.push(if holds(until, now) { until - now } else { 0 }); + now = now.max(until); } assert_eq!( - waits, + quiet, [0, 0, HOUR, 0, 0, HOUR, 0, 0], "an hour after every third failure in a row" ); + assert_eq!(count(&state), 8); + } + + #[test] + fn nothing_is_fetched_while_quiet_and_asking_resumes_after_the_hour() { + let mut state = with_failures(saved("pk.old", true), 3, NOW + HOUR); + let (result, asked) = run(&mut state, NOW + HOUR - 1, token("pk.new")); + assert!(!asked); + assert_eq!(result, None); + let (result, asked) = run(&mut state, NOW + HOUR, token("pk.new")); + assert!(asked); + assert_eq!(result, Some("pk.new".to_owned())); + assert_eq!(count(&state), 0, "a new token starts the count again"); + } + + #[test] + fn a_quiet_caller_does_not_use_even_a_good_saved_token() { + let mut state = with_failures(saved("pk.kept", false), 3, NOW + 10); + let (result, asked) = run(&mut state, NOW, token("pk.new")); + assert_eq!(result, None); + assert!(!asked); } #[test] fn the_rejected_token_again_is_taken_back_once_it_checks_valid() { - let mut state = State { - failures: 2, - ..saved("pk.old", true) - }; + let mut state = with_failures(saved("pk.old", true), 2, 0); let (result, _, checked) = run_checked(&mut state, NOW, token("pk.old"), Refusal::Forbidden); assert!(checked); assert_eq!(result, Some("pk.old".to_owned())); - assert_eq!(state, saved("pk.old", false), "the count starts again"); + assert!(!state.rejected); + assert_eq!(count(&state), 2, "the same token is no fresh start"); for verdict in [Refusal::Dead, Refusal::Unknown] { let mut state = saved("pk.old", true); let (result, _, _) = run_checked(&mut state, NOW, token("pk.old"), verdict); assert_eq!(result, None, "{verdict:?}"); assert!(state.rejected, "{verdict:?}"); - assert_eq!(state.failures, 1, "{verdict:?}"); + assert_eq!(count(&state), 1, "{verdict:?}"); } } @@ -611,21 +764,6 @@ mod tests { } } - #[test] - fn nothing_is_fetched_while_waiting() { - let mut state = State { - retry_at: NOW + HOUR, - ..saved("pk.old", true) - }; - let (result, asked) = run(&mut state, NOW + HOUR - 1, token("pk.new")); - assert!(!asked); - assert_eq!(result, None); - let (result, asked) = run(&mut state, NOW + HOUR, token("pk.new")); - assert!(asked); - assert_eq!(result, Some("pk.new".to_owned())); - assert_eq!(state.failures, 0, "a new token starts the count again"); - } - #[test] fn an_answer_without_a_usable_token_counts_as_a_failure() { for answer in [ @@ -637,8 +775,14 @@ mod tests { let mut state = State::default(); let (result, _) = run(&mut state, NOW, answer); assert_eq!(result, None); - assert_eq!(state.failures, 1); - assert_eq!(state.retry_at, NOW, "the first failure waits for nothing"); + assert_eq!( + state.callers[CALLER], + Failures { + count: 1, + quiet_until: 0, + token: None, + } + ); assert_eq!(state.token, None, "nothing unusable is kept"); } } @@ -648,43 +792,54 @@ mod tests { let mut state = saved("pk.old", true); let (result, _) = run(&mut state, NOW, Fetched::Failed); assert_eq!(result, None); - assert_eq!(state, saved("pk.old", true), "the schedule is untouched"); + assert_eq!(state, saved("pk.old", true), "nothing is counted"); } #[test] - fn a_forced_fetch_of_the_kept_token_keeps_what_is_known() { - let mut state = State { - cooldowns: BTreeMap::from([( - "telemetry".to_owned(), - Cooldown { - failures: 1, - until: NOW, - }, - )]), - ..saved("pk.kept", false) + fn a_new_token_resets_every_caller_and_the_same_one_resets_nobody() { + let quiet = |state: State| { + let state = with_failures(state, 3, NOW + HOUR); + failing(state, "other", 3, NOW + HOUR) }; + let mut state = quiet(saved("pk.kept", false)); let before = state.clone(); assert_eq!( - judge(&mut state, NOW, token("pk.kept"), |_| Refusal::Dead), + judge(&mut state, token("pk.kept"), |_| Refusal::Dead), Some("pk.kept".to_owned()) ); assert_eq!(state, before); + + // The dead token served again and now valid is still the same token. + let mut state = quiet(saved("pk.kept", true)); assert_eq!( - judge(&mut state, NOW, token("pk.new"), |_| Refusal::Dead), + judge(&mut state, token("pk.kept"), |_| Refusal::Forbidden), + Some("pk.kept".to_owned()) + ); + assert!(!state.rejected); + for caller in [CALLER, "other"] { + assert!(state.quiet(caller, NOW), "{caller} was reset"); + } + + let mut state = quiet(saved("pk.kept", false)); + assert_eq!( + judge(&mut state, token("pk.new"), |_| Refusal::Dead), Some("pk.new".to_owned()) ); - assert_eq!(state, saved("pk.new", false), "a new token starts over"); + for caller in [CALLER, "other"] { + assert!(!state.quiet(caller, NOW), "{caller} is still quiet"); + assert_eq!(state.failures(caller), None, "{caller}"); + } + assert!(state.without_stale().callers.is_empty()); } #[test] - fn a_wait_beyond_an_hour_is_not_honored() { - let mut state = State { - retry_at: u64::MAX, - ..saved("pk.old", true) - }; + fn a_quiet_period_beyond_an_hour_is_not_honored() { + let mut state = with_failures(saved("pk.old", true), 3, u64::MAX); let (result, asked) = run(&mut state, NOW, token("pk.new")); assert!(asked); assert_eq!(result, Some("pk.new".to_owned())); + assert!(holds(NOW + HOUR, NOW)); + assert!(!holds(NOW + HOUR, NOW + HOUR)); } #[test] @@ -696,57 +851,166 @@ mod tests { } #[test] - fn a_cooldown_holds_for_its_hour_and_not_beyond() { - let cooldown = Cooldown { - failures: 3, - until: NOW + HOUR, - }; - assert!(holds(cooldown.until, NOW)); - assert!(!holds(cooldown.until, NOW + HOUR)); - let edited = Cooldown { - failures: 3, - until: u64::MAX, - }; - assert!( - !holds(edited.until, NOW), - "a wait beyond an hour is not honored" + fn another_callers_quiet_period_does_not_hold_this_one() { + let other = failing(saved("pk.kept", false), "other", 3, NOW + HOUR); + let mut state = other.clone(); + let (result, _) = run(&mut state, NOW, token("pk.new")); + assert_eq!(result, Some("pk.kept".to_owned())); + assert_eq!(state, other, "the other's count stands"); + + let other = failing(State::default(), "other", 3, NOW + HOUR); + let mut state = other.clone(); + run(&mut state, NOW, Fetched::Nothing); + assert_eq!(state.callers["other"], other.callers["other"]); + assert_eq!(count(&state), 1, "this caller's own failure"); + } + + #[test] + fn fetching_a_new_token_ends_another_callers_quiet_period() { + let mut state = failing(saved("pk.old", true), "other", 3, NOW + HOUR); + assert!(state.quiet("other", NOW)); + let (result, asked) = run(&mut state, NOW, token("pk.new")); + assert!(asked); + assert_eq!(result, Some("pk.new".to_owned())); + assert!(!state.quiet("other", NOW)); + assert!(state.without_stale().callers.is_empty()); + } + + #[test] + fn an_unpaced_caller_ignores_quiet_periods_and_counts_nothing() { + let quiet = with_failures(saved("pk.old", true), 3, NOW + HOUR); + let mut state = quiet.clone(); + let (result, asked, _) = run_as( + &mut state, + CALLER, + false, + NOW, + Fetched::Nothing, + Refusal::Dead, ); + assert!(asked, "held back by a quiet period"); + assert_eq!(result, Err(Unavailable::NoToken { revoked: true })); + assert_eq!(state, quiet, "a failure was counted"); + + let mut state = State::default(); + let (result, _, _) = run_as( + &mut state, + "report", + false, + NOW, + token("pk.new"), + Refusal::Dead, + ); + assert_eq!(result, Ok(("pk.new".to_owned(), TokenSource::Fetched))); } + /// Two runs at once: one saves a new token while another, still holding + /// the old one, records a failure. The failure names the old token, so + /// it holds nobody back once merged. #[test] - fn a_fetch_keeps_what_another_run_wrote_meanwhile() { - let resting = || State { - cooldowns: BTreeMap::from([( - "telemetry".to_owned(), - Cooldown { - failures: 3, - until: NOW + HOUR, - }, - )]), - ..saved("pk.old", true) - }; - let before = saved("pk.old", true); + fn a_count_merged_in_after_a_new_token_holds_nobody_back() { + let before = with_failures(saved("pk.old", true), 2, 0); + let mut failed = before.clone(); + failed.fail(CALLER, NOW); + assert!(failed.quiet(CALLER, NOW)); + let disk = saved("pk.new", false); + let written = merged(&before, failed, disk, Some(CALLER)); + assert_eq!(written.token.as_deref(), Some("pk.new")); + assert!(!written.quiet(CALLER, NOW)); + assert_eq!(written.without_stale(), saved("pk.new", false)); + } + + /// Runs [`refetch`] for telemetry after `pk.old` was refused, with the + /// endpoint returning `answer` and the Tokens API calling a dead token + /// still dead. + fn refused_then(state: &mut State, now: u64, answer: Fetched) { + refetch(state, CALLER, "pk.old", now, answer, |_| Refusal::Dead); + } + + #[test] + fn a_different_token_after_a_refusal_is_saved_and_starts_over() { + for rejected in [false, true] { + let mut state = with_failures(saved("pk.old", rejected), 2, 0); + refused_then(&mut state, NOW, token("pk.new")); + assert_eq!( + state.without_stale(), + saved("pk.new", false), + "rejected: {rejected}" + ); + } + } + + #[test] + fn a_refusal_the_fetch_does_not_resolve_is_a_failure() { + for (rejected, answer) in [ + (false, token("pk.old")), + (true, token("pk.old")), + (false, Fetched::Nothing), + (false, Fetched::Failed), + (true, Fetched::Failed), + ] { + let mut state = saved("pk.old", rejected); + refused_then(&mut state, NOW, answer); + assert_eq!(count(&state), 1, "rejected: {rejected}"); + assert_eq!(state.token.as_deref(), Some("pk.old")); + assert_eq!(state.rejected, rejected); + } + } + + #[test] + fn the_same_token_after_three_refusals_quiets_the_caller() { + let mut state = saved("pk.old", false); + for _ in 0..3 { + assert!(!state.quiet(CALLER, NOW)); + refused_then(&mut state, NOW, token("pk.old")); + } + assert!(state.quiet(CALLER, NOW)); + assert!(!state.quiet(CALLER, NOW + HOUR), "for an hour"); + } - // A cooldown added meanwhile is kept by a fetch that only failed... + #[test] + fn a_save_writes_this_callers_count_over_what_is_on_disk() { + let before = saved("pk.old", false); let mut failed = before.clone(); - failed.fail(NOW); - let written = merged(&before, failed.clone(), resting()).expect("written"); - assert_eq!(written.failures, 1); - assert_eq!(written.cooldowns, resting().cooldowns); + failed.fail(CALLER, NOW); + let mut disk = before.clone(); + disk.callers.insert("other".to_owned(), Failures::default()); + let written = merged(&before, failed.clone(), disk.clone(), Some(CALLER)); + assert_eq!(written.callers[CALLER], failed.callers[CALLER]); + assert!(written.callers.contains_key("other"), "another's is kept"); + + // If this run didn't change the count, another run's count is kept. + let mut counted = before.clone(); + counted.fail(CALLER, NOW); + counted.fail(CALLER, NOW); + let written = merged(&before, before.clone(), counted.clone(), Some(CALLER)); + assert_eq!(written, counted); + } - // ...and ended by one that brought a new token. + #[test] + fn a_token_another_run_saved_meanwhile_stands() { + let before = saved("pk.old", true); let fresh = saved("pk.new", false); - let written = merged(&before, fresh.clone(), resting()).expect("written"); + let other = saved("pk.other", false); + let written = merged(&before, fresh.clone(), other.clone(), Some(CALLER)); + assert_eq!(written.token, other.token); + + let mut marked = before.clone(); + marked.rejected = false; + let written = merged(&before, fresh.clone(), marked, Some(CALLER)); + assert_eq!(written.token.as_deref(), Some("pk.old"), "a verdict stands"); + + let written = merged(&before, fresh.clone(), before.clone(), None); assert_eq!(written, fresh); + } - // A token or a count another run saved meanwhile stands. - assert_eq!( - merged(&before, fresh.clone(), saved("pk.other", false)), - None - ); - let mut counted = before.clone(); - counted.fail(NOW); - assert_eq!(merged(&before, failed, counted), None); + #[test] + fn a_command_saves_no_count() { + let before = saved("pk.old", true); + let mut state = before.clone(); + state.fail(CALLER, NOW); + let written = merged(&before, state, before.clone(), None); + assert!(written.callers.is_empty()); } #[test] @@ -784,13 +1048,12 @@ mod tests { let state = State { token: Some("pk.a".to_owned()), rejected: true, - failures: 2, - retry_at: 7, - cooldowns: BTreeMap::from([( - "telemetry".to_owned(), - Cooldown { - failures: 1, - until: 9, + callers: BTreeMap::from([( + CALLER.to_owned(), + Failures { + count: 1, + quiet_until: 9, + token: Some("pk.a".to_owned()), }, )]), overridden: true, @@ -802,4 +1065,35 @@ mod tests { saved("pk.a", false) ); } + + #[test] + fn a_success_with_an_old_token_keeps_the_count_made_with_the_new_one() { + let mut state = with_failures(saved("pk.new", false), 2, 0); + state.accept(CALLER, "pk.old"); + assert_eq!(count(&state), 2); + state.accept(CALLER, "pk.new"); + assert!(state.callers.is_empty()); + } + + #[test] + fn an_earlier_builds_waits_are_not_read() { + let old = format!( + r#"{{"token":"pk.a","rejected":false,"overridden":true,"failures":3, + "retry_at":{later},"cooldowns":{{"telemetry":{{"failures":3, + "until":{later},"misses":3,"retry_at":{later}}}}}}}"#, + later = NOW + 10 + ); + let mut state: State = serde_json::from_str(&old).expect("an old file reads"); + assert_eq!( + state, + State { + overridden: true, + ..saved("pk.a", false) + } + ); + assert!(!state.quiet(CALLER, NOW)); + let (result, _) = run(&mut state, NOW, token("pk.new")); + assert_eq!(result, Some("pk.a".to_owned())); + assert!(!serde_json::to_string(&state).unwrap().contains("retry_at")); + } } diff --git a/src/main.rs b/src/main.rs index 4c1823e..e86134a 100644 --- a/src/main.rs +++ b/src/main.rs @@ -842,7 +842,6 @@ fn no_stored_credentials(profile: Option<&str>) -> anyhow::Error { /// Answers `--schema`, from either of the two places it can be noticed. fn emit_schema(app: &Command, specs: &[ServiceSpec], matches: &ArgMatches) -> u8 { run_record::set_parsed(app, specs, matches, run_record::Invocation::Schema); - telemetry_sink::remember_user_token(cli_token::user_token(matches)); let mode = Mode::from_matches(matches); match schema::emit(mode, app, specs, matches) { Ok(()) => 0, @@ -951,7 +950,6 @@ fn cli() -> u8 { } run_record::set_parsed(&app, &specs, &matches, run_record::Invocation::Execute); - telemetry_sink::remember_user_token(cli_token::user_token(&matches)); output::banner::show(&matches); let mode = Mode::from_matches(&matches); match run(&app, &specs, &matches, mode) { @@ -1547,15 +1545,15 @@ fn run(app: &Command, specs: &[ServiceSpec], matches: &ArgMatches, mode: Mode) - let mut cli_failure = None; let chose_own_token = use_login || auth::typed_token(matches).is_some(); let cli = (!chose_own_token) - .then(|| cli_token::for_command(&command, !dry_run)) + .then(|| cli_token::command(&command)) .flatten(); - if let Some(cli) = cli { + if let Some(caller) = cli { let username = matches.get_one::("username").cloned(); let attempt = try_cli_token( op, op_matches, &command, - cli, + caller.token(!dry_run), username.as_deref(), flags, mode, @@ -1590,15 +1588,12 @@ fn run(app: &Command, specs: &[ServiceSpec], matches: &ArgMatches, mode: Mode) - } .or_else(|| stored_creds.as_ref().map(|c| c.access_token.clone())); - // A command that could not use the CLI's token falls back to the - // user's, and with none says why the CLI's failed rather than - // sending a request bound to be refused. A dry run sends nothing - // and fetches nothing, so it plans with whatever there is. - if let Some((failure, why)) = cli_failure.filter(|_| !dry_run) { - if token.is_none() { - return Err((*failure).with_remedy(no_own_token_remedy()).into()); + if let Some((failure, why)) = cli_failure { + if let Some(note) = + without_cli_token(*failure, &why, dry_run, token.is_some()).map_err(|e| *e)? + { + output::progress(¬e); } - output::progress(&format!("{why} Using your own token instead.")); } if use_login && token.is_none() { return Err(no_stored_credentials(profile)); @@ -1651,14 +1646,13 @@ enum CliAttempt { }, } -/// Runs a [`cli_token::Use::Preferred`] command on the CLI's token, given -/// what [`cli_token::for_command`] found for it, by the rules in -/// [`cli_token::send_with_cli_token`]. +/// Runs a command on the CLI's token, given what [`cli_token::Caller::token`] +/// found for it, by the rules in [`cli_token::send_with_cli_token`]. fn try_cli_token( op: &spec::Operation, op_matches: &clap::ArgMatches, command: &str, - cli: Result, + cli: Result, username: Option<&str>, mut flags: executor::RunFlags, mode: output::Mode, @@ -1683,8 +1677,7 @@ fn try_cli_token( if flags.dry_run { return CliAttempt::Done(execute(&token.token, flags)); } - let sent = cli_token::send_for_command( - command, + let sent = cli_token::send_with( token, |token| { let result = execute(token, flags); @@ -1697,6 +1690,12 @@ fn try_cli_token( Err(error) => error.downcast_ref::().and_then(|e| e.status), }, ); + cli_attempt(command, sent) +} + +/// Turns a command's send with the CLI's token into either the result or +/// the reason to fall back to the user's token. +fn cli_attempt(command: &str, sent: cli_token::Sent>) -> CliAttempt { let (result, refusal) = match sent { cli_token::Sent::Answered(result) => match rate_limited(result) { // Every install shares the CLI's token, so a limit on it says @@ -1773,10 +1772,32 @@ fn cli_token_unavailable(command: &str, why: &cli_token::Unavailable) -> CliErro "cli.mapbox.com did not return a usable one" } cli_token::Unavailable::NotFetched => "it has not been fetched yet", + cli_token::Unavailable::Quiet => "it failed several times in a row", + cli_token::Unavailable::OverrideNotPublic => "MAPBOX_CLI_TOKEN is not a public token", }; CliError::new("cli_token_unavailable", provided_token(command, reason)) } +/// What a command does when it could not use the CLI's token. With +/// `own_token`, it sends the user's, and the note says why. Without, it says +/// why the CLI's failed rather than sending a request bound to be refused. +/// A dry run sends nothing and fetches nothing, so it plans with whatever +/// there is. +fn without_cli_token( + error: CliError, + why: &str, + dry_run: bool, + own_token: bool, +) -> std::result::Result, Box> { + if dry_run { + Ok(None) + } else if own_token { + Ok(Some(format!("{why} Using your own token instead."))) + } else { + Err(Box::new(error.with_remedy(no_own_token_remedy()))) + } +} + /// What to do when a command could not use the CLI's token and the user has /// none of their own: give it theirs. fn no_own_token_remedy() -> Remedy { @@ -3001,3 +3022,73 @@ paths: ); } } + +#[cfg(test)] +mod cli_attempt_tests { + use super::*; + + fn api_error(status: u16) -> Result<()> { + Err(CliError { + status: Some(status), + ..CliError::new("http_error", "refused") + } + .with_request_id(Some("req-1".to_owned())) + .into()) + } + + fn unusable(attempt: CliAttempt) -> (Box, bool) { + match attempt { + CliAttempt::Unusable { error, sent, .. } => (error, sent), + CliAttempt::Done(result) => panic!("done: {result:?}"), + } + } + + #[test] + fn a_refusal_says_why_with_its_code_and_request_id() { + for (refusal, code) in [ + (cli_token::Refusal::Dead, "cli_token_unavailable"), + (cli_token::Refusal::Forbidden, "cli_token_forbidden"), + (cli_token::Refusal::Unknown, "cli_token_forbidden"), + (cli_token::Refusal::NotFetched, "cli_token_forbidden"), + ] { + let sent = cli_token::Sent::Refused(api_error(403), refusal); + let (error, sent) = unusable(cli_attempt("report", sent)); + assert_eq!(error.code, code, "{refusal:?}"); + assert_eq!(error.request_id.as_deref(), Some("req-1"), "{refusal:?}"); + assert!(sent, "{refusal:?}: the request went out"); + } + } + + #[test] + fn without_a_token_of_their_own_the_user_is_told_to_log_in_unless_it_is_a_dry_run() { + let error = || CliError::new("cli_token_forbidden", "refused"); + let failed = without_cli_token(error(), "why", false, false).expect_err("no token"); + assert_eq!(failed.code, "cli_token_forbidden"); + assert_eq!(failed.fix, no_own_token_remedy().fix); + assert!(matches!( + without_cli_token(error(), "why", true, false), + Ok(None) + )); + let note = without_cli_token(error(), "Why.", false, true).expect("own token"); + assert_eq!(note.as_deref(), Some("Why. Using your own token instead.")); + } + + #[test] + fn a_rate_limit_lets_the_users_token_stand_in() { + let sent = cli_token::Sent::Answered(api_error(429)); + let (error, sent) = unusable(cli_attempt("report", sent)); + assert_eq!(error.status, Some(429)); + assert!(sent); + } + + #[test] + fn any_other_answer_is_the_commands() { + for result in [Ok(()), api_error(500)] { + let expected = format!("{result:?}"); + match cli_attempt("report", cli_token::Sent::Answered(result)) { + CliAttempt::Done(result) => assert_eq!(format!("{result:?}"), expected), + CliAttempt::Unusable { .. } => panic!("{expected}: unusable"), + } + } + } +} diff --git a/src/telemetry_event.rs b/src/telemetry_event.rs index b712e73..3da6e16 100644 --- a/src/telemetry_event.rs +++ b/src/telemetry_event.rs @@ -349,8 +349,7 @@ pub(crate) fn deliver(record: &Record) { } let event = build(record); if let Ok(line) = serde_json::to_string(&event) { - let profile = record.options.as_ref().and_then(|o| o.profile.as_deref()); - telemetry_sink::deliver(&line, profile); + telemetry_sink::deliver(&line); } } diff --git a/src/telemetry_sink.rs b/src/telemetry_sink.rs index b26fe09..edd51bf 100644 --- a/src/telemetry_sink.rs +++ b/src/telemetry_sink.rs @@ -6,9 +6,9 @@ //! delivery change — a new endpoint, batching — happen without touching them. //! //! [`deliver`] hands the event to [`HttpSink`], which sends it to Mapbox -//! Events, when [`crate::cli_token`] has a token it could send it with. -//! Otherwise the event is dropped: nothing would ever send a copy kept on -//! disk, so keeping one would only store usage data for nobody. The state +//! Events, when [`crate::cli_token`] has a CLI token for it. Never the +//! user's token; that module explains why. Otherwise the event is dropped +//! rather than saved, because nothing would ever send a saved copy. The state //! files below are still written, since building the event reads them. //! //! It also owns the directory, `~/.mapbox/.telemetry` (or under @@ -20,7 +20,6 @@ use std::io::{Read, Write}; use std::path::{Path, PathBuf}; use std::process::{Command, ExitCode, Stdio}; -use std::sync::OnceLock; use std::time::{Duration, Instant, SystemTime}; use reqwest::header::{CONTENT_TYPE, USER_AGENT}; @@ -36,9 +35,9 @@ const DIR: &str = ".telemetry"; /// Days of delivery-log files kept, today included. const KEEP_DAYS: u64 = 7; -/// Production Mapbox Events, for every build. A login is a production login, -/// so a build from source sends where a release does rather than to a -/// staging host its tokens mean nothing to. +/// Production Mapbox Events, for every build. The CLI's token is a +/// production token on every channel, so a build from source sends where a +/// release does; staging would not accept the token. const ENDPOINT: &str = "https://events.mapbox.com/events/v2"; /// Set by [`HttpSink`] on the child it spawns, and read by [`is_send_child`] @@ -54,11 +53,6 @@ const SEND_ENV: &str = "MAPBOX_INTERNAL_TELEMETRY_SEND"; /// (`https://api-events-staging.tilestream.net/events/v2`). const URL_ENV: &str = "MAPBOX_INTERNAL_TELEMETRY_URL"; -/// The token the user gave this run, from [`remember_user_token`]. Kept -/// apart from the run record, which goes to history and must never hold a -/// token. -static USER_TOKEN: OnceLock> = OnceLock::new(); - /// Turns on the delivery log: one line when an event is handed to the child /// and one with how the send ended, in `deliveries/.jsonl` under /// this module's directory. For measuring the send's success rate while @@ -68,36 +62,25 @@ static USER_TOKEN: OnceLock> = OnceLock::new(); const DELIVERY_LOG_ENV: &str = "MAPBOX_INTERNAL_TELEMETRY_LOG"; const DELIVERY_DIR: &str = "deliveries"; -/// Each attempt's budget. Nobody is waiting for the child; with every token -/// in [`cli_token::TELEMETRY`] tried it can run several of these. +/// Bounds the POST only. The token fetch and the Tokens API check around it +/// keep their own timeouts, so the child can run several times this; nobody +/// waits for it. const SEND_TIMEOUT: Duration = Duration::from_secs(5); -/// Ample for one event and its envelope; anything larger on stdin is not one -/// of ours. +/// Ample for one event; anything larger on stdin is not one of ours. const MAX_EVENT_BYTES: u64 = 64 * 1024; /// Sends each event to Mapbox Events from a detached child, so the command /// never waits on the network. /// /// The event goes to the child on stdin rather than argv, which is visible -/// in `ps`, and so does the user's token, which would otherwise sit in the -/// child's environment. The child sends through [`cli_token::send`], which -/// may refresh the login: the reason it runs there and not in the parent, -/// which is about to exit. One attempt per token; the event is dropped on +/// in `ps`. The child sends through [`cli_token::send`], which may check a +/// refused token and fetch a new one; that runs in the child because the +/// parent is about to exit. One attempt; the event is dropped on /// any failure. Best-effort by contract: it reports nothing and must not fail /// the command, block its exit, or write to stdout. struct HttpSink { url: String, - user_token: Option, - profile: Option, -} - -/// What the parent writes to the child's stdin. -#[derive(serde::Serialize, serde::Deserialize)] -struct Envelope { - event: serde_json::Value, - user_token: Option, - profile: Option, } impl HttpSink { @@ -110,8 +93,7 @@ impl HttpSink { return; }; let mut command = Command::new(exe); - // The user's token travels in the envelope; the child has no use for - // it in its environment. + // The child never sends with the user's token, so don't pass it one. command .env(SEND_ENV, "1") .env(URL_ENV, &self.url) @@ -122,19 +104,10 @@ impl HttpSink { .stderr(Stdio::null()); crate::detach::detach(&mut command); - let Ok(event) = serde_json::from_str(line) else { - log_delivery(line, json!({ "stage": "bad_event" })); - return; - }; - let envelope = Envelope { - event, - user_token: self.user_token.clone(), - profile: self.profile.clone(), - }; - let Ok(envelope) = serde_json::to_string(&envelope) else { + if serde_json::from_str::(line).is_err() { log_delivery(line, json!({ "stage": "bad_event" })); return; - }; + } let Ok(mut child) = command.spawn() else { log_delivery(line, json!({ "stage": "spawn_failed" })); return; @@ -145,45 +118,16 @@ impl HttpSink { // One event fits in a pipe buffer, so this does not wait on the // child. Dropping `stdin` closes it, which is the child's end of input. if let Some(mut stdin) = child.stdin.take() { - let _ = stdin.write_all(envelope.as_bytes()); + let _ = stdin.write_all(line.as_bytes()); } // Not waited on: the parent is about to exit. } } -/// Keeps the token the user gave this run for [`deliver`], resolved by -/// [`cli_token::user_token`] while the arguments are still at hand. -pub(crate) fn remember_user_token(token: Option) { - let _ = USER_TOKEN.set(token); -} - -/// Hands `line` to [`HttpSink`] when [`cli_token`] has a token it could send -/// it with, and drops it otherwise. `profile` is the run's `--profile`, whose -/// login it may send with. -pub(crate) fn deliver(line: &str, profile: Option<&str>) { - let url = send_url(); - // Help, `--version` and usage errors return before the arguments are - // parsed, so nothing was remembered; the environment is still the user's, - // unless `--use-login` was typed to set it aside. - let user_token = match USER_TOKEN.get() { - Some(remembered) => remembered.clone(), - None if std::env::args_os().any(|arg| arg == "--use-login") => None, - None => auth::environment_token().map(|(_, token)| token), - }; - if cli_token::has_token_for( - cli_token::TELEMETRY_CALLER, - cli_token::TELEMETRY, - user_token.as_deref(), - profile, - ) { - HttpSink { - url, - user_token, - profile: profile.map(str::to_owned), - } - .deliver(line) - } else { - log_delivery(line, json!({ "stage": "no_token" })); +pub(crate) fn deliver(line: &str) { + match cli_token::no_token_for(&cli_token::TELEMETRY_CALLER) { + None => HttpSink { url: send_url() }.deliver(line), + Some(reason) => log_delivery(line, json!({ "stage": "no_token", "reason": reason })), } } @@ -206,8 +150,11 @@ pub fn run_send_child() -> ExitCode { let read = std::io::stdin() .take(MAX_EVENT_BYTES) .read_to_string(&mut input); - match (read, serde_json::from_str::(input.trim())) { - (Ok(_), Ok(envelope)) => send(&url, &envelope), + match ( + read, + serde_json::from_str::(input.trim()), + ) { + (Ok(_), Ok(event)) => send(&url, &event), _ => log_delivery("", json!({ "stage": "bad_envelope" })), } } @@ -218,9 +165,8 @@ pub fn run_send_child() -> ExitCode { /// is the product token alone: Mapbox Events stores it in every record, and /// the markers `http::client` adds (an `agent/` among them) must not ride /// along. -fn send(url: &str, envelope: &Envelope) { - let line = envelope.event.to_string(); - let outcome = match post(url, envelope) { +fn send(url: &str, event: &serde_json::Value) { + let outcome = match post(url, event) { Ok((status, elapsed)) => json!({ "stage": "responded", "status": status, @@ -228,31 +174,25 @@ fn send(url: &str, envelope: &Envelope) { }), Err(error) => json!({ "stage": "failed", "error": error }), }; - log_delivery(&line, outcome); + log_delivery(&event.to_string(), outcome); } /// The response's status and how long it took, or why there was none. -fn post(url: &str, envelope: &Envelope) -> Result<(u16, Duration), String> { - let body = serde_json::to_string(&[&envelope.event]).map_err(|err| err.to_string())?; +fn post(url: &str, event: &serde_json::Value) -> Result<(u16, Duration), String> { + let body = serde_json::to_string(&[event]).map_err(|err| err.to_string())?; let client = http::client().map_err(|err| err.to_string())?; - // Reset by every attempt, so the time is the POST that answered alone, - // not the token fetch or check that may come before it. + // Reset when the request is built, so the time covers only the POST, not + // a token fetch before it. let started = std::cell::Cell::new(Instant::now()); - let response = cli_token::send( - cli_token::TELEMETRY_CALLER, - cli_token::TELEMETRY, - envelope.user_token.as_deref(), - envelope.profile.as_deref(), - || { - started.set(Instant::now()); - client - .post(url) - .header(USER_AGENT, telemetry::PRODUCT_TOKEN) - .header(CONTENT_TYPE, "application/json") - .body(body.clone()) - .timeout(SEND_TIMEOUT) - }, - ) + let response = cli_token::send(&cli_token::TELEMETRY_CALLER, || { + started.set(Instant::now()); + client + .post(url) + .header(USER_AGENT, telemetry::PRODUCT_TOKEN) + .header(CONTENT_TYPE, "application/json") + .body(body.clone()) + .timeout(SEND_TIMEOUT) + }) .ok_or_else(|| "no token to send with".to_string())? .map_err(|err| http::failure(&err))?; Ok((response.status().as_u16(), started.get().elapsed())) diff --git a/tests/dry_run.rs b/tests/dry_run.rs index 92416b1..7e8ae2c 100644 --- a/tests/dry_run.rs +++ b/tests/dry_run.rs @@ -659,7 +659,7 @@ fn report_needs_a_message_and_takes_no_data() { /// What `report` sends with, and what it says, when it cannot use the CLI's /// token. Every case here is decided before a request: the token endpoint -/// is a dead port, and a cooldown is written to disk. +/// is a dead port, and telemetry's quiet period is written to disk. mod report_token { use super::*; use std::io::{Read, Write}; @@ -765,16 +765,23 @@ mod report_token { ); } - /// Someone ran it, so a rest telemetry might take does not hold it back: - /// it sends with the CLI's token. + /// Telemetry's entry for three failures with `token`, quiet for an hour. + fn telemetry_quiet(token: Option<&str>) -> String { + let token = token.map_or(String::new(), |token| format!(r#","token":"{token}""#)); + format!( + r#""callers":{{"telemetry":{{"count":3,"quiet_until":{}{token}}}}}"#, + now() + 3600 + ) + } + #[test] - fn a_cooldown_does_not_hold_report_back() { + fn telemetrys_quiet_period_does_not_hold_report_back() { let config = config_dir("report-not-resting"); write_state( &config, &format!( - r#"{{"token":"pk.saved","overridden":true,"cooldowns":{{"report":{{"failures":3,"until":{}}}}}}}"#, - now() + 3600 + r#"{{"token":"pk.saved","overridden":true,{}}}"#, + telemetry_quiet(Some("pk.saved")) ), ); let out = sent_nowhere(&config, &["--debug", "report", "A test"]) @@ -788,17 +795,15 @@ mod report_token { assert!(!err.contains("recently"), "{err}"); } - /// Nor does the endpoint's backoff: with the kept token dead, it asks - /// for a new one there and then, and sends with it. + /// The new token also ends telemetry's quiet period, which was set with + /// the dead one. #[test] - fn report_fetches_through_the_backoff() { + fn report_fetches_while_telemetry_is_quiet() { let config = config_dir("report-through-backoff"); + let quiet = telemetry_quiet(Some("pk.dead")); write_state( &config, - &format!( - r#"{{"token":"pk.dead","rejected":true,"overridden":true,"failures":3,"retry_at":{}}}"#, - now() + 3600 - ), + &format!(r#"{{"token":"pk.dead","rejected":true,"overridden":true,{quiet}}}"#), ); let out = sent_nowhere(&config, &["--debug", "report", "A test"]) .env("MAPBOX_INTERNAL_CLI_TOKEN_URL", token_endpoint("pk.fresh")) @@ -811,19 +816,19 @@ mod report_token { ); let state = std::fs::read_to_string(config.join("cli-token.json")).expect("the state"); assert!(state.contains(r#""token":"pk.fresh""#), "{state}"); + assert!( + !state.contains("callers"), + "telemetry is still quiet: {state}" + ); } - /// A command fetches past the backoff, so the reason it gives is what - /// that fetch found, not the backoff's wait, which it never waited for. + /// The error names what the fetch found, not telemetry's quiet period. #[test] - fn an_unreachable_endpoint_is_reported_through_the_backoff() { + fn an_unreachable_endpoint_is_reported_while_telemetry_is_quiet() { let config = config_dir("report-unreachable-backoff"); write_state( &config, - &format!( - r#"{{"overridden":true,"failures":3,"retry_at":{}}}"#, - now() + 3600 - ), + &format!(r#"{{"overridden":true,{}}}"#, telemetry_quiet(None)), ); let out = run(&config, &["-o", "json", "report", "A test"]); assert_eq!(error_code(&out), "cli_token_unavailable"); @@ -832,12 +837,10 @@ mod report_token { assert!(!err.contains("trying again"), "{err}"); } - /// A miss on a command's fetch does not count toward the backoff, which - /// paces telemetry's fetches in the background. #[test] - fn a_commands_miss_leaves_the_backoff_alone() { + fn a_commands_miss_leaves_telemetrys_count_alone() { let config = config_dir("report-miss-uncounted"); - let state = r#"{"overridden":true,"failures":2}"#; + let state = r#"{"overridden":true,"callers":{"telemetry":{"count":2,"quiet_until":0}}}"#; write_state(&config, state); let out = command(&config) .env( diff --git a/tests/telemetry_events.rs b/tests/telemetry_events.rs index 8ce38a9..99a714e 100644 --- a/tests/telemetry_events.rs +++ b/tests/telemetry_events.rs @@ -10,7 +10,9 @@ //! pointed at a dead port, and a run with no token drops its event, so most //! runs here are given `MAPBOX_CLI_TOKEN` to send with. The tests of the //! fetched token point `MAPBOX_INTERNAL_CLI_TOKEN_URL`, and the Tokens API -//! check behind `MAPBOX_INTERNAL_TOKENS_URL`, at loopback servers. +//! check behind `MAPBOX_INTERNAL_TOKENS_URL`, at loopback servers. Events are +//! never sent with the user's token, so any user token in these tests is +//! there to show it is not sent. use std::io::{Read, Write}; use std::net::TcpListener; @@ -66,7 +68,7 @@ fn run(home: &Path, args: &[&str]) -> Output { /// Runs `cmd` with its event sent to a loopback server, and returns the run /// and the event the server received. `MAPBOX_CLI_TOKEN` gives the run a -/// token to send with whether or not it has one of its own. +/// token to send with, since the token endpoint is not there. fn sent(cmd: &mut Command) -> (Output, Value) { let (received, url) = events_server(true); let out = cmd @@ -365,7 +367,7 @@ fn with_a_send_url_the_event_is_posted() { let (received, url) = events_server(true); let out = command(&home) .env("MAPBOX_INTERNAL_TELEMETRY_URL", &url) - .env("MAPBOX_ACCESS_TOKEN", "pk.test") + .env("MAPBOX_CLI_TOKEN", "pk.test") // A marker `http::client` would add, which must not reach Mapbox Events. .env("CLAUDECODE", "1") .args(["config", "list"]) @@ -406,7 +408,7 @@ fn the_command_does_not_wait_for_the_send() { let started = Instant::now(); let out = command(&home) .env("MAPBOX_INTERNAL_TELEMETRY_URL", &url) - .env("MAPBOX_ACCESS_TOKEN", "pk.test") + .env("MAPBOX_CLI_TOKEN", "pk.test") .args(["config", "list"]) .output() .expect("run mapbox"); @@ -426,7 +428,7 @@ fn the_opt_out_sends_nothing() { let (received, url) = events_server(true); let out = command(&home) .env("MAPBOX_INTERNAL_TELEMETRY_URL", &url) - .env("MAPBOX_ACCESS_TOKEN", "pk.test") + .env("MAPBOX_CLI_TOKEN", "pk.test") .env("MAPBOX_CLI_NO_TELEMETRY", "1") .args(["config", "list"]) .output() @@ -446,7 +448,7 @@ fn a_run_under_sudo_sends_nothing() { let (received, url) = events_server(true); let out = command(&home) .env("MAPBOX_INTERNAL_TELEMETRY_URL", &url) - .env("MAPBOX_ACCESS_TOKEN", "pk.test") + .env("MAPBOX_CLI_TOKEN", "pk.test") .env("SUDO_USER", "someone") .args(["config", "list"]) .output() @@ -490,7 +492,7 @@ fn the_delivery_log_pairs_each_hand_off_with_its_response() { let (_received, url) = events_server(true); let out = command(&home) .env("MAPBOX_INTERNAL_TELEMETRY_URL", &url) - .env("MAPBOX_ACCESS_TOKEN", "pk.test") + .env("MAPBOX_CLI_TOKEN", "pk.test") .env("MAPBOX_INTERNAL_TELEMETRY_LOG", "1") .args(["config", "list"]) .output() @@ -524,7 +526,7 @@ fn the_delivery_log_says_why_a_send_failed() { "MAPBOX_INTERNAL_TELEMETRY_URL", format!("http://127.0.0.1:{port}/events/v2"), ) - .env("MAPBOX_ACCESS_TOKEN", "pk.test") + .env("MAPBOX_CLI_TOKEN", "pk.test") .env("MAPBOX_INTERNAL_TELEMETRY_LOG", "1") .args(["config", "list"]) .output() @@ -548,7 +550,7 @@ fn without_the_switch_there_is_no_delivery_log() { let (received, url) = events_server(true); let out = command(&home) .env("MAPBOX_INTERNAL_TELEMETRY_URL", &url) - .env("MAPBOX_ACCESS_TOKEN", "pk.test") + .env("MAPBOX_CLI_TOKEN", "pk.test") .args(["config", "list"]) .output() .expect("run mapbox"); @@ -563,28 +565,38 @@ fn without_the_switch_there_is_no_delivery_log() { } #[test] -fn a_typed_token_reaches_the_send() { - let home = scratch("send-typed-token"); - let (received, url) = events_server(true); - let out = command(&home) - .env("MAPBOX_INTERNAL_TELEMETRY_URL", &url) - .args(["--token", "pk.typed", "config", "list"]) - .output() - .expect("run mapbox"); - assert!(out.status.success()); - let request = received - .recv_timeout(Duration::from_secs(10)) - .expect("the event was posted"); - assert_eq!( - request.head.lines().next().unwrap_or_default(), - "POST /events/v2?access_token=pk.typed HTTP/1.1" - ); +fn a_users_token_never_reaches_the_send() { + for args in USER_TOKEN_RUNS { + let home = scratch("send-never-user"); + log_in(&home, "tk.login"); + let (received, url) = events_server(true); + let _ = command(&home) + .env("MAPBOX_INTERNAL_TELEMETRY_URL", &url) + .env("MAPBOX_ACCESS_TOKEN", "sk.env") + .env("MapboxAccessToken", "sk.legacy") + .env("MAPBOX_CLI_TOKEN", "pk.cli") + .args(args) + .output() + .expect("run mapbox"); + let request = received + .recv_timeout(Duration::from_secs(10)) + .expect("the event was posted"); + assert!( + request.head.contains("access_token=pk.cli "), + "{args:?}: {}", + request.head + ); + assert_eq!(referer(&request.head).as_deref(), CLI_REFERER, "{args:?}"); + let whole = format!("{}{}", request.head, request.body); + for user in ["sk.typed", "tk.login", "sk.env", "sk.legacy"] { + assert!(!whole.contains(user), "{args:?}: `{user}` was sent"); + } + } } -/// Telemetry may fall back to the CLI's token whatever URL it is sent to, so -/// an overridden one keeps it. +/// An overridden URL still gets the CLI's token and its `Referer`. #[test] -fn the_clis_token_sends_the_event_for_someone_with_none() { +fn an_overridden_url_keeps_the_clis_token() { let home = scratch("send-cli-token-only"); let (received, url) = events_server(true); let out = command(&home) @@ -617,23 +629,14 @@ fn referer(head: &str) -> Option { }) } -#[test] -fn use_login_keeps_the_environment_token_out_of_the_send() { - let home = scratch("send-use-login"); - let (received, url) = events_server(true); - let out = command(&home) - .env("MAPBOX_INTERNAL_TELEMETRY_URL", &url) - .env("MAPBOX_ACCESS_TOKEN", "pk.env") - .env("MAPBOX_CLI_TOKEN", "pk.cli") - .args(["--use-login", "config", "list"]) - .output() - .expect("run mapbox"); - assert!(out.status.success()); - assert_eq!( - sent_with(&received), - "pk.cli", - "--use-login still sent MAPBOX_ACCESS_TOKEN" - ); +/// Saves a login holding `token`. It has no `exp`, so it never needs a +/// refresh. +fn log_in(home: &Path, token: &str) { + std::fs::write( + config_dir(home).join("credentials.json"), + format!(r#"{{"access_token":"{token}"}}"#), + ) + .expect("write a login"); } /// The `access_token` the loopback server's one request carried. @@ -649,117 +652,26 @@ fn sent_with(received: &mpsc::Receiver) -> String { .to_string() } -#[test] -fn the_users_token_comes_before_the_clis() { - let home = scratch("send-user-first"); - let (received, url) = events_server(true); - let out = command(&home) - .env("MAPBOX_INTERNAL_TELEMETRY_URL", &url) - .env("MAPBOX_CLI_TOKEN", "pk.cli") - .args(["--token", "pk.typed", "config", "list"]) - .output() - .expect("run mapbox"); - assert!(out.status.success()); - let request = received - .recv_timeout(Duration::from_secs(10)) - .expect("the event was posted"); - assert!( - request.head.contains("access_token=pk.typed "), - "{}", - request.head - ); - assert_eq!( - referer(&request.head), - None, - "the user's token carries no Referer" - ); -} - -/// `--version`, help and usage errors return before the arguments are parsed, -/// and still send with the environment's token rather than the CLI's. -#[test] -fn a_run_that_never_parses_still_sends_with_the_environment_token() { - for args in [ - &["--version"][..], - &["styles", "--help"], - &["nosuchcommand"], - ] { - let home = scratch("send-unparsed"); - let (received, url) = events_server(true); - let _ = command(&home) - .env("MAPBOX_INTERNAL_TELEMETRY_URL", &url) - .env("MAPBOX_ACCESS_TOKEN", "pk.env") - .env("MAPBOX_CLI_TOKEN", "pk.cli") - .args(args) - .output() - .expect("run mapbox"); - assert_eq!(sent_with(&received), "pk.env", "{args:?}"); - } -} - -#[test] -fn a_run_that_never_parses_still_honors_use_login() { - for args in [ - &["--use-login", "--version"][..], - &["--use-login", "styles", "--help"], - &["--use-login", "nosuchcommand"], - ] { - let home = scratch("send-unparsed-use-login"); - let (received, url) = events_server(true); - let _ = command(&home) - .env("MAPBOX_INTERNAL_TELEMETRY_URL", &url) - .env("MAPBOX_ACCESS_TOKEN", "pk.env") - .env("MAPBOX_CLI_TOKEN", "pk.cli") - .args(args) - .output() - .expect("run mapbox"); - assert_eq!(sent_with(&received), "pk.cli", "{args:?}"); - } -} - -/// `MapboxAccessToken` is the user's token for `tilesets-cli`, so it comes -/// before the CLI's even though clap doesn't read it. -#[test] -fn the_legacy_token_variable_is_the_users_too() { - for args in [&["config", "list"][..], &["--version"]] { - let home = scratch("send-legacy-env"); - let (received, url) = events_server(true); - let _ = command(&home) - .env("MAPBOX_INTERNAL_TELEMETRY_URL", &url) - .env("MapboxAccessToken", "pk.legacy") - .env("MAPBOX_CLI_TOKEN", "pk.cli") - .args(args) - .output() - .expect("run mapbox"); - assert_eq!(sent_with(&received), "pk.legacy", "{args:?}"); - } -} - -/// The child's login step reads without the lock while the login is good, so -/// a read-only command's telemetry neither creates a lock file nor tightens -/// the directory's mode. +/// Telemetry never reads the saved login, so sending creates no lock file +/// and doesn't change the directory's permissions. #[cfg(unix)] #[test] -fn sending_with_a_login_leaves_the_config_directory_alone() { +fn the_send_leaves_a_login_and_its_directory_alone() { use std::os::unix::fs::PermissionsExt; let home = scratch("send-login-readonly"); let dir = config_dir(&home); - // No `exp`, so it never needs a refresh. - std::fs::write( - dir.join("credentials.json"), - r#"{"access_token":"pk.login"}"#, - ) - .expect("write a login"); + log_in(&home, "tk.login"); std::fs::set_permissions(&dir, std::fs::Permissions::from_mode(0o755)).expect("chmod"); let (received, url) = events_server(true); let out = command(&home) .env("MAPBOX_INTERNAL_TELEMETRY_URL", &url) + .env("MAPBOX_CLI_TOKEN", "pk.cli") .args(["config", "list"]) .output() .expect("run mapbox"); assert!(out.status.success()); - assert_eq!(sent_with(&received), "pk.login"); + assert_eq!(sent_with(&received), "pk.cli"); let mode = std::fs::metadata(&dir) .expect("the config dir") @@ -914,8 +826,6 @@ fn cli_token_state(home: &Path, contains: &str) -> Value { } } -/// With no token of the user's and no `MAPBOX_CLI_TOKEN`, the event goes -/// with the token fetched from the endpoint, which is kept for the next run. #[test] fn the_fetched_token_sends_the_event_and_is_kept() { let home = scratch("send-fetched"); @@ -946,155 +856,6 @@ fn the_fetched_token_sends_the_event_and_is_kept() { assert_eq!(sent_with(&received), "pk.fetched"); } -/// A `401` to the kept token, which the Tokens API says is revoked, marks -/// it; a new one is fetched at once and the event sent again with it. -#[test] -fn a_revoked_token_is_replaced_and_the_event_resent() { - let home = scratch("send-rejected"); - std::fs::write( - config_dir(&home).join("cli-token.json"), - r#"{"token":"pk.old","overridden":true}"#, - ) - .expect("write the state"); - let (heads, events_url) = answer_each(&[ - ("401 Unauthorized", r#"{"message":"Not Authorized"}"#), - ("204 No Content", ""), - ]); - // A `401` around the verdict, which the real endpoint has not been seen - // to send: the `code` still decides. - let (_, tokens_url) = answer_once("401 Unauthorized", r#"{"code":"TokenRevoked"}"#); - let (_, token_url) = answer_once("200 OK", r#"{"token":"pk.new"}"#); - let out = command(&home) - .env("MAPBOX_INTERNAL_TELEMETRY_URL", &events_url) - .env("MAPBOX_INTERNAL_TOKENS_URL", &tokens_url) - .env("MAPBOX_INTERNAL_CLI_TOKEN_URL", &token_url) - .args(["config", "list"]) - .output() - .expect("run mapbox"); - assert!(out.status.success()); - let tokens: Vec = (0..2) - .map(|_| { - let head = heads - .recv_timeout(Duration::from_secs(10)) - .expect("an event was posted"); - assert_eq!( - referer(&head).as_deref(), - Some("https://cli.mapbox.com/"), - "{head}" - ); - head.split("access_token=") - .nth(1) - .and_then(|rest| rest.split([' ', '&']).next()) - .unwrap_or_default() - .to_string() - }) - .collect(); - assert_eq!(tokens, ["pk.old", "pk.new"]); - assert_eq!(cli_token_state(&home, "pk.new")["rejected"], false); -} - -/// A `401` to a token the Tokens API calls valid is this API refusing it, -/// not the token being dead, so it is kept; the third such refusal in a row -/// rests telemetry for an hour. -#[test] -fn a_token_an_api_refuses_is_kept() { - let home = scratch("send-forbidden"); - std::fs::write( - config_dir(&home).join("cli-token.json"), - // Refused twice before: this is the third in a row. - r#"{"token":"pk.kept","overridden":true,"cooldowns":{"telemetry":{"failures":2}}}"#, - ) - .expect("write the state"); - let (_, events_url) = answer_once("401 Unauthorized", r#"{"message":"Not Authorized"}"#); - let (checked, tokens_url) = answer_once("200 OK", r#"{"code":"TokenValid"}"#); - let out = command(&home) - .env("MAPBOX_INTERNAL_TELEMETRY_LOG", "1") - .env( - "MAPBOX_INTERNAL_TELEMETRY_URL", - events_url.replace("/config/token.json", "/events/v2"), - ) - .env("MAPBOX_INTERNAL_TOKENS_URL", &tokens_url) - .args(["config", "list"]) - .output() - .expect("run mapbox"); - assert!(out.status.success()); - checked - .recv_timeout(Duration::from_secs(10)) - .expect("the token was checked"); - let lines = deliveries(&home, 2); - assert_eq!(lines[1]["status"], 401, "{lines:?}"); - let state = cli_token_state(&home, "cooldowns"); - assert_eq!(state["token"], "pk.kept"); - assert_eq!(state["rejected"], false); - assert!(state["cooldowns"]["telemetry"]["until"].is_u64(), "{state}"); - - // Resting: the next run sends nothing with it, and checks nothing. - let (sent, events_url) = answer_once("401 Unauthorized", r#"{"message":"Not Authorized"}"#); - let (checked, tokens_url) = answer_once("200 OK", r#"{"code":"TokenValid"}"#); - let out = command(&home) - .env("MAPBOX_INTERNAL_TELEMETRY_LOG", "1") - .env( - "MAPBOX_INTERNAL_TELEMETRY_URL", - events_url.replace("/config/token.json", "/events/v2"), - ) - .env("MAPBOX_INTERNAL_TOKENS_URL", &tokens_url) - .args(["config", "list"]) - .output() - .expect("run mapbox"); - assert!(out.status.success()); - let lines = deliveries(&home, 3); - assert_eq!(lines[2]["stage"], "no_token", "{lines:?}"); - assert!( - sent.recv_timeout(Duration::from_millis(500)).is_err(), - "sent while resting" - ); - assert!( - checked.recv_timeout(Duration::from_millis(100)).is_err(), - "checked while resting" - ); -} - -/// The endpoint serving the rejected token a third time in a row is not -/// asked again for an hour, and the event is dropped meanwhile. -#[test] -fn the_rejected_token_served_again_waits_before_asking() { - let home = scratch("send-still-rejected"); - std::fs::write( - config_dir(&home).join("cli-token.json"), - // Two fetches already found it: this is the third in a row. - r#"{"token":"pk.old","rejected":true,"overridden":true,"failures":2}"#, - ) - .expect("write the state"); - let (_, token_url) = answer_once("200 OK", r#"{"token":"pk.old"}"#); - let out = command(&home) - .env( - "MAPBOX_INTERNAL_TELEMETRY_URL", - "http://127.0.0.1:1/events/v2", - ) - .env("MAPBOX_INTERNAL_CLI_TOKEN_URL", &token_url) - .args(["config", "list"]) - .output() - .expect("run mapbox"); - assert!(out.status.success()); - let state = cli_token_state(&home, "retry_at"); - assert_eq!(state["failures"], 3); - - // Nothing listens at the endpoint now, and nothing should try it. - let out = command(&home) - .env("MAPBOX_INTERNAL_TELEMETRY_LOG", "1") - .env( - "MAPBOX_INTERNAL_TELEMETRY_URL", - "http://127.0.0.1:1/events/v2", - ) - .env("MAPBOX_INTERNAL_CLI_TOKEN_URL", &token_url) - .args(["config", "list"]) - .output() - .expect("run mapbox"); - assert!(out.status.success()); - let lines = deliveries(&home, 1); - assert_eq!(lines.last().expect("a delivery line")["stage"], "no_token"); -} - /// A token called invalid for something fixed in the Console stays the same /// string, so the dead token served again is checked once more, and taken /// back once it checks valid. @@ -1213,66 +974,597 @@ fn keeping_a_fetched_token_leaves_the_directory_mode_alone() { ); } -/// Only an answer that went through ends a cooldown: a `500` after one -/// expired says nothing about whether the API takes the token now. +fn now() -> u64 { + std::time::SystemTime::now() + .duration_since(std::time::UNIX_EPOCH) + .expect("the clock is after 1970") + .as_secs() +} + +/// Marks the state `overridden`, as a dev build pointed at loopback writes +/// it; otherwise the run would not trust it. +fn save_cli_token_state(home: &Path, state: Value) { + let mut state = state; + state["overridden"] = Value::Bool(true); + std::fs::write(config_dir(home).join("cli-token.json"), state.to_string()) + .expect("write the state"); +} + +/// The `access_token` and `Referer` of the next request in `heads`. +fn token_sent(heads: &mpsc::Receiver) -> (String, Option) { + let head = heads + .recv_timeout(Duration::from_secs(10)) + .expect("an event was posted"); + let token = head + .split("access_token=") + .nth(1) + .and_then(|rest| rest.split([' ', '&']).next()) + .unwrap_or_default() + .to_string(); + (token, referer(&head)) +} + +const CLI_REFERER: Option<&str> = Some("https://cli.mapbox.com/"); + +/// Runs where the user has a token: passed with `--token`, from the login, +/// and commands that never resolve one. +const USER_TOKEN_RUNS: [&[&str]; 5] = [ + &["--token", "sk.typed", "config", "list"], + &["--use-login", "config", "list"], + &["--version"], + &["nosuchcommand"], + &["styles", "list", "--schema"], +]; + #[test] -fn a_failed_send_does_not_end_a_cooldown() { - let home = scratch("send-cooldown-500"); - std::fs::write( - config_dir(&home).join("cli-token.json"), - r#"{"token":"pk.kept","overridden":true,"cooldowns":{"telemetry":{"until":1}}}"#, - ) - .expect("write the state"); - let (sent, events_url) = answer_once("500 Internal Server Error", "{}"); - let out = command(&home) +fn a_logged_in_run_sends_with_the_clis_token() { + for args in USER_TOKEN_RUNS { + let home = scratch("send-logged-in-cli"); + log_in(&home, "tk.login"); + let (_, token_url) = answer_once("200 OK", r#"{"token":"pk.fetched"}"#); + let (received, url) = events_server(true); + let _ = command(&home) + .env("MAPBOX_INTERNAL_TELEMETRY_URL", &url) + .env("MAPBOX_INTERNAL_CLI_TOKEN_URL", &token_url) + // A `pk.` token passes the check a fetched token must, so it + // would get through a fallback that `sk.` and `tk.` would not. + .env("MAPBOX_ACCESS_TOKEN", "pk.env") + .env("MapboxAccessToken", "sk.legacy") + .args(args) + .output() + .expect("run mapbox"); + let request = received + .recv_timeout(Duration::from_secs(10)) + .expect("the event was posted"); + assert!( + request.head.contains("access_token=pk.fetched "), + "{args:?}: {}", + request.head + ); + let whole = format!("{}{}", request.head, request.body); + for user in ["sk.typed", "tk.login", "pk.env", "sk.legacy"] { + assert!(!whole.contains(user), "{args:?}: `{user}` was sent"); + } + } +} + +/// Runs `config list` with a user token in the environment. Returns whether +/// an event arrived, and the delivery log. +fn try_sending(home: &Path, token_url: &str) -> (bool, Vec) { + let (received, url) = events_server(true); + let out = command(home) .env("MAPBOX_INTERNAL_TELEMETRY_LOG", "1") - .env( - "MAPBOX_INTERNAL_TELEMETRY_URL", - events_url.replace("/config/token.json", "/events/v2"), - ) + .env("MAPBOX_INTERNAL_TELEMETRY_URL", &url) + .env("MAPBOX_INTERNAL_CLI_TOKEN_URL", token_url) + .env("MAPBOX_ACCESS_TOKEN", "sk.env") .args(["config", "list"]) .output() .expect("run mapbox"); assert!(out.status.success()); - sent.recv_timeout(Duration::from_secs(10)) - .expect("the expired cooldown let the event go"); - let lines = deliveries(&home, 2); - assert_eq!(lines[1]["status"], 500, "{lines:?}"); - let state: Value = serde_json::from_str( - &std::fs::read_to_string(config_dir(&home).join("cli-token.json")).expect("the state"), - ) - .expect("the state is JSON"); - assert_eq!(state["cooldowns"]["telemetry"]["until"], 1, "{state}"); + // One line when the event is dropped at once, two when the child ran. + let mut lines = deliveries(home, 1); + if lines.last().is_some_and(|line| line["stage"] == "queued") { + lines = deliveries(home, 2); + } + let arrived = received.recv_timeout(Duration::from_millis(500)).is_ok(); + (arrived, lines) } -/// A refusal the Tokens API cannot explain still rests telemetry: sending -/// the same token on the next run would most likely be refused again, and -/// cost another check that may take seconds. +/// A `403` for a token the Tokens API calls valid. The event is dropped, not +/// resent with another token. #[test] -fn an_unexplained_refusal_starts_a_cooldown_too() { - let home = scratch("send-unknown"); - std::fs::write( - config_dir(&home).join("cli-token.json"), - r#"{"token":"pk.kept","overridden":true}"#, - ) - .expect("write the state"); - let (_, events_url) = answer_once("403 Forbidden", r#"{"message":"Forbidden"}"#); - let (checked, tokens_url) = answer_once("500 Internal Server Error", "{}"); +fn a_refused_clis_token_is_not_sent_again() { + let home = scratch("send-refused-once"); + log_in(&home, "tk.login"); + save_cli_token_state(&home, serde_json::json!({"token": "pk.kept"})); + let (heads, events_url) = answer_each(&[ + ("403 Forbidden", r#"{"message":"Forbidden"}"#), + ("204 No Content", ""), + ]); + let (_, tokens_url) = answer_once("200 OK", r#"{"code":"TokenValid"}"#); let out = command(&home) - .env( - "MAPBOX_INTERNAL_TELEMETRY_URL", - events_url.replace("/config/token.json", "/events/v2"), - ) + .env("MAPBOX_INTERNAL_TELEMETRY_URL", &events_url) + .env("MAPBOX_INTERNAL_TOKENS_URL", &tokens_url) + .env("MAPBOX_ACCESS_TOKEN", "sk.env") + .args(["config", "list"]) + .output() + .expect("run mapbox"); + assert!(out.status.success()); + assert_eq!( + token_sent(&heads), + ("pk.kept".to_string(), CLI_REFERER.map(str::to_owned)) + ); + // The count is written when the child finishes, so a resend would show + // up within the wait below. + cli_token_state(&home, r#""count":1"#); + assert!( + heads.recv_timeout(Duration::from_secs(1)).is_err(), + "the event was sent again" + ); +} + +#[test] +fn a_revoked_token_is_replaced_for_the_next_run_without_a_resend() { + let home = scratch("send-rotated-revoked"); + save_cli_token_state(&home, serde_json::json!({"token": "pk.old"})); + let (heads, events_url) = answer_each(&[ + ("401 Unauthorized", r#"{"message":"Not Authorized"}"#), + ("204 No Content", ""), + ]); + // The real Tokens API hasn't been seen to wrap its verdict in a `401`, + // but if it does, the `code` still decides. + let (_, tokens_url) = answer_once("401 Unauthorized", r#"{"code":"TokenRevoked"}"#); + let (_, token_url) = answer_once("200 OK", r#"{"token":"pk.new"}"#); + let out = command(&home) + .env("MAPBOX_INTERNAL_TELEMETRY_URL", &events_url) + .env("MAPBOX_INTERNAL_TOKENS_URL", &tokens_url) + .env("MAPBOX_INTERNAL_CLI_TOKEN_URL", &token_url) + .args(["config", "list"]) + .output() + .expect("run mapbox"); + assert!(out.status.success()); + assert_eq!(token_sent(&heads).0, "pk.old"); + assert_eq!(cli_token_state(&home, "pk.new")["rejected"], false); + assert!( + heads.recv_timeout(Duration::from_secs(1)).is_err(), + "the event was sent again" + ); + + // The token endpoint answered only once, so this run must use the saved + // token. + let (received, url) = events_server(true); + let out = command(&home) + .env("MAPBOX_INTERNAL_TELEMETRY_URL", &url) + .env("MAPBOX_INTERNAL_CLI_TOKEN_URL", &token_url) + .args(["config", "list"]) + .output() + .expect("run mapbox"); + assert!(out.status.success()); + assert_eq!(sent_with(&received), "pk.new"); +} + +/// An `sk.` or `tk.` override drops the event, and the fetched token is not +/// sent instead. +#[test] +fn an_override_that_is_not_public_sends_nothing() { + for token in ["sk.x", "tk.x"] { + let home = scratch(&format!("send-override-{token}")); + let (asked, token_url) = answer_once("200 OK", r#"{"token":"pk.fetched"}"#); + let (received, url) = events_server(true); + let out = command(&home) + .env("MAPBOX_INTERNAL_TELEMETRY_LOG", "1") + .env("MAPBOX_INTERNAL_TELEMETRY_URL", &url) + .env("MAPBOX_INTERNAL_CLI_TOKEN_URL", &token_url) + .env("MAPBOX_CLI_TOKEN", token) + .args(["config", "list"]) + .output() + .expect("run mapbox"); + assert!(out.status.success()); + let lines = deliveries(&home, 1); + let last = lines.last().expect("a delivery line"); + assert_eq!(last["stage"], "no_token", "{token}: {lines:?}"); + assert_eq!(last["reason"], "override_not_public", "{token}"); + assert!( + received.recv_timeout(Duration::from_millis(500)).is_err(), + "{token}: sent" + ); + assert!( + asked.recv_timeout(Duration::from_millis(100)).is_err(), + "{token}: fetched a token instead" + ); + } +} + +/// Even when the override matches the saved token, its refusal leaves the +/// saved state alone. +#[test] +fn a_refused_override_is_not_checked_or_replaced() { + let home = scratch("send-override-refused"); + let state = serde_json::json!({"token": "pk.cli", "overridden": true}).to_string(); + std::fs::write(config_dir(&home).join("cli-token.json"), &state).expect("write the state"); + let (heads, events_url) = answer_each(&[ + ("401 Unauthorized", r#"{"message":"Not Authorized"}"#), + ("204 No Content", ""), + ]); + let (fetched, token_url) = answer_once("200 OK", r#"{"token":"pk.new"}"#); + let (checked, tokens_url) = answer_once("200 OK", r#"{"code":"TokenRevoked"}"#); + let out = command(&home) + .env("MAPBOX_INTERNAL_TELEMETRY_LOG", "1") + .env("MAPBOX_INTERNAL_TELEMETRY_URL", &events_url) + .env("MAPBOX_INTERNAL_CLI_TOKEN_URL", &token_url) .env("MAPBOX_INTERNAL_TOKENS_URL", &tokens_url) + .env("MAPBOX_CLI_TOKEN", "pk.cli") + .args(["config", "list"]) + .output() + .expect("run mapbox"); + assert!(out.status.success()); + assert_eq!(token_sent(&heads).0, "pk.cli"); + // The child logs once it is done with the refusal. + let lines = deliveries(&home, 2); + assert_eq!(lines[1]["status"], 401, "{lines:?}"); + assert!( + heads.recv_timeout(Duration::from_millis(500)).is_err(), + "the event was sent again" + ); + assert!( + checked.recv_timeout(Duration::from_millis(100)).is_err(), + "the override was checked" + ); + assert!( + fetched.recv_timeout(Duration::from_millis(100)).is_err(), + "a token was fetched to replace the override" + ); + let after = + std::fs::read_to_string(config_dir(&home).join("cli-token.json")).expect("the state"); + assert_eq!(after, state); +} + +/// A port nothing listens on, for an endpoint a run must not reach. +const NOWHERE: &str = "http://127.0.0.1:9/"; + +const NOT_AUTHORIZED: (&str, &str) = ("401 Unauthorized", r#"{"message":"Not Authorized"}"#); + +/// Runs `config list` with the delivery log on, its event sent to `events`, +/// the token endpoint at `token` and the Tokens API at `tokens`. +fn run_against(home: &Path, events: &str, token: &str, tokens: &str) { + let out = command(home) + .env("MAPBOX_INTERNAL_TELEMETRY_LOG", "1") + .env("MAPBOX_INTERNAL_TELEMETRY_URL", events) + .env("MAPBOX_INTERNAL_CLI_TOKEN_URL", token) + .env("MAPBOX_INTERNAL_TOKENS_URL", tokens) .args(["config", "list"]) .output() .expect("run mapbox"); assert!(out.status.success()); +} + +fn read_cli_token_state(home: &Path) -> Value { + let text = std::fs::read_to_string(config_dir(home).join("cli-token.json")).expect("the state"); + serde_json::from_str(&text).expect("the state is JSON") +} + +/// After a valid token is refused, the endpoint is asked once. The new token +/// it serves is used next run and resets telemetry's count. The refused +/// event is not resent. +#[test] +fn a_different_token_served_after_a_refusal_is_used_next_run() { + let home = scratch("send-refused-rotated"); + save_cli_token_state( + &home, + serde_json::json!({"token": "pk.old", + "callers": {"telemetry": {"count": 2, "token": "pk.old"}}}), + ); + let (heads, events_url) = answer_each(&[NOT_AUTHORIZED, ("204 No Content", "")]); + let (_, tokens_url) = answer_once("200 OK", r#"{"code":"TokenValid"}"#); + let (fetches, token_url) = answer_each(&[ + ("200 OK", r#"{"token":"pk.new"}"#), + ("200 OK", r#"{"token":"pk.new"}"#), + ]); + run_against(&home, &events_url, &token_url, &tokens_url); + assert_eq!(token_sent(&heads).0, "pk.old"); + let state = cli_token_state(&home, "pk.new"); + assert!(state.get("callers").is_none(), "{state}"); + fetches + .recv_timeout(Duration::from_secs(10)) + .expect("the endpoint was asked after the refusal"); + // The child logs once it is done. + deliveries(&home, 2); + assert!( + fetches.recv_timeout(Duration::from_millis(500)).is_err(), + "the endpoint was asked twice" + ); + assert!( + heads.recv_timeout(Duration::from_millis(100)).is_err(), + "the event was sent again" + ); + + let (received, url) = events_server(true); + run_against(&home, &url, &token_url, NOWHERE); + assert_eq!(sent_with(&received), "pk.new"); + assert!( + fetches.recv_timeout(Duration::from_millis(500)).is_err(), + "fetched with a token on disk" + ); +} + +/// While quiet, nothing is sent, checked or fetched, and the parent drops +/// the event without spawning a child. After the hour, a successful send +/// resets the count. +#[test] +fn the_same_token_after_three_refusals_quiets_telemetry_for_an_hour() { + let home = scratch("send-refused-quiet"); + save_cli_token_state(&home, serde_json::json!({"token": "pk.kept"})); + for n in 1..=3 { + let (heads, events_url) = answer_each(&[NOT_AUTHORIZED]); + let (_, tokens_url) = answer_once("200 OK", r#"{"code":"TokenValid"}"#); + let (fetched, token_url) = answer_once("200 OK", r#"{"token":"pk.kept"}"#); + run_against(&home, &events_url, &token_url, &tokens_url); + assert_eq!(token_sent(&heads).0, "pk.kept", "run {n}"); + fetched + .recv_timeout(Duration::from_secs(10)) + .expect("the endpoint was asked after the refusal"); + let state = cli_token_state(&home, &format!(r#""count":{n}"#)); + assert_eq!(state["token"], "pk.kept", "{state}"); + assert_eq!(state["rejected"], false, "{state}"); + deliveries(&home, 2 * n); + } + let mut state = read_cli_token_state(&home); + let until = state["callers"]["telemetry"]["quiet_until"] + .as_u64() + .expect("a quiet period"); + assert!(until > now() + 3500, "{state}"); + + let (sent, events_url) = answer_once(NOT_AUTHORIZED.0, NOT_AUTHORIZED.1); + let (checked, tokens_url) = answer_once("200 OK", r#"{"code":"TokenValid"}"#); + let (fetched, token_url) = answer_once("200 OK", r#"{"token":"pk.new"}"#); + run_against(&home, &events_url, &token_url, &tokens_url); + let lines = deliveries(&home, 7); + assert_eq!(lines[6]["stage"], "no_token", "{lines:?}"); + assert_eq!(lines[6]["reason"], "quiet", "{lines:?}"); + assert!( + sent.recv_timeout(Duration::from_millis(500)).is_err(), + "sent while quiet" + ); + assert!(checked.recv_timeout(Duration::from_millis(100)).is_err()); + assert!( + fetched.recv_timeout(Duration::from_millis(100)).is_err(), + "fetched while quiet" + ); + + state["callers"]["telemetry"]["quiet_until"] = (now() - 1).into(); + save_cli_token_state(&home, state); + let (received, url) = events_server(true); + run_against(&home, &url, NOWHERE, NOWHERE); + assert_eq!(sent_with(&received), "pk.kept"); + deliveries(&home, 9); + let state = read_cli_token_state(&home); + assert!(state.get("callers").is_none(), "{state}"); +} + +/// After the quiet hour, a new token from the endpoint resets the count. +#[test] +fn fetches_with_no_usable_token_quiet_telemetry() { + let home = scratch("send-fetch-quiet"); + save_cli_token_state(&home, serde_json::json!({})); + for n in 1..=3 { + let (fetched, token_url) = answer_once("200 OK", r#"{"token":"sk.not-public"}"#); + run_against(&home, NOWHERE, &token_url, NOWHERE); + fetched + .recv_timeout(Duration::from_secs(10)) + .expect("the endpoint was asked"); + cli_token_state(&home, &format!(r#""count":{n}"#)); + deliveries(&home, 2 * n); + } + + let (fetched, token_url) = answer_once("200 OK", r#"{"token":"pk.new"}"#); + run_against(&home, NOWHERE, &token_url, NOWHERE); + let lines = deliveries(&home, 7); + assert_eq!(lines[6]["reason"], "quiet", "{lines:?}"); + assert!( + fetched.recv_timeout(Duration::from_millis(500)).is_err(), + "fetched while quiet" + ); + + let mut state = read_cli_token_state(&home); + state["callers"]["telemetry"]["quiet_until"] = 1.into(); + save_cli_token_state(&home, state); + let (received, url) = events_server(true); + run_against(&home, &url, &token_url, NOWHERE); + assert_eq!(sent_with(&received), "pk.new"); + let state = cli_token_state(&home, "pk.new"); + assert!(state.get("callers").is_none(), "{state}"); +} + +/// A `500` says nothing about whether the API accepts the token, so it +/// doesn't reset the count. +#[test] +fn only_an_accepted_event_ends_the_count() { + let home = scratch("send-count-ends"); + save_cli_token_state( + &home, + serde_json::json!({"token": "pk.kept", + "callers": {"telemetry": {"count": 2, "token": "pk.kept"}}}), + ); + let (heads, events_url) = answer_each(&[("500 Internal Server Error", "{}")]); + run_against(&home, &events_url, NOWHERE, NOWHERE); + assert_eq!(token_sent(&heads).0, "pk.kept"); + let lines = deliveries(&home, 2); + assert_eq!(lines[1]["status"], 500, "{lines:?}"); + let state = read_cli_token_state(&home); + assert_eq!(state["callers"]["telemetry"]["count"], 2, "{state}"); + + let (received, url) = events_server(true); + run_against(&home, &url, NOWHERE, NOWHERE); + assert_eq!(sent_with(&received), "pk.kept"); + deliveries(&home, 4); + let state = read_cli_token_state(&home); + assert!(state.get("callers").is_none(), "{state}"); +} + +/// A refusal the Tokens API can't explain counts as a failure, and so does +/// a fetch after it with no response. Otherwise every run would send the +/// same token, likely be refused, and pay for another check. +#[test] +fn an_unexplained_refusal_counts_too() { + let home = scratch("send-unknown"); + save_cli_token_state(&home, serde_json::json!({"token": "pk.kept"})); + let (_, events_url) = answer_each(&[("403 Forbidden", r#"{"message":"Forbidden"}"#)]); + let (checked, tokens_url) = answer_once("500 Internal Server Error", "{}"); + run_against(&home, &events_url, NOWHERE, &tokens_url); checked .recv_timeout(Duration::from_secs(10)) .expect("the token was checked"); - let state = cli_token_state(&home, "cooldowns"); + let state = cli_token_state(&home, r#""count":1"#); assert_eq!(state["token"], "pk.kept"); assert_eq!(state["rejected"], false); - assert!(state["cooldowns"]["telemetry"]["until"].is_u64(), "{state}"); +} + +/// Two cases: no saved token and a dead endpoint, which only the child finds +/// out; and telemetry quiet with a good saved token, which the parent knows +/// before spawning anything. +#[test] +fn an_unusable_clis_token_sends_nothing_even_with_a_users_token() { + let later = now() + 3000; + for (name, state) in [ + ("endpoint-dead", None), + ( + "quiet", + Some(serde_json::json!({"token": "pk.kept", + "callers": {"telemetry": {"count": 3, "quiet_until": later, + "token": "pk.kept"}}})), + ), + ] { + let home = scratch(&format!("send-nothing-{name}")); + log_in(&home, "tk.login"); + let quiet = state.is_some(); + if let Some(state) = state { + save_cli_token_state(&home, state); + } + let (arrived, lines) = try_sending(&home, NOWHERE); + assert!(!arrived, "{name}: sent"); + let last = lines.last().expect("a delivery line"); + if !quiet { + assert_eq!(last["stage"], "failed", "{name}: {lines:?}"); + assert_eq!(last["error"], "no token to send with", "{name}"); + } else { + assert_eq!(last["stage"], "no_token", "{name}: {lines:?}"); + assert_eq!(last["reason"], "quiet", "{name}: {lines:?}"); + } + } +} + +/// Another caller's quiet period doesn't hold telemetry back, and neither do +/// the waits an earlier build wrote: an upgrade must not leave anyone +/// waiting. +#[test] +fn no_wait_but_telemetrys_own_holds_it() { + let later = now() + 3000; + for (name, state, expected) in [ + ( + "other-caller", + serde_json::json!({"token": "pk.kept", "callers": {"other": {"count": 3, + "quiet_until": later, "token": "pk.kept"}}}), + "pk.kept", + ), + ( + "earlier-build-pause", + serde_json::json!({"token": "pk.old", "rejected": true, + "failures": 3, "retry_at": later}), + "pk.new", + ), + ( + "earlier-build-rest", + serde_json::json!({"token": "pk.kept", "failures": 3, "retry_at": later, + "cooldowns": {"telemetry": {"failures": 3, "until": later, + "misses": 3, "retry_at": later}}}), + "pk.kept", + ), + ] { + let home = scratch(&format!("send-not-held-{name}")); + save_cli_token_state(&home, state); + let (_, token_url) = answer_once("200 OK", r#"{"token":"pk.new"}"#); + let (received, url) = events_server(true); + run_against(&home, &url, &token_url, NOWHERE); + assert_eq!(sent_with(&received), expected, "{name}"); + if name == "other-caller" { + deliveries(&home, 2); + let state = read_cli_token_state(&home); + assert_eq!(state["callers"]["other"]["count"], 3, "{state}"); + assert!(state["callers"].get("telemetry").is_none(), "{state}"); + } + } +} + +/// The quiet period applies only to the fetched token. +#[test] +fn an_override_is_sent_while_telemetry_is_quiet() { + let home = scratch("send-override-quiet"); + save_cli_token_state( + &home, + serde_json::json!({"token": "pk.kept", + "callers": {"telemetry": {"count": 3, "quiet_until": now() + 3000, + "token": "pk.kept"}}}), + ); + let (received, url) = events_server(true); + let out = command(&home) + .env("MAPBOX_INTERNAL_TELEMETRY_URL", &url) + .env("MAPBOX_CLI_TOKEN", "pk.cli") + .args(["config", "list"]) + .output() + .expect("run mapbox"); + assert!(out.status.success()); + assert_eq!(sent_with(&received), "pk.cli"); +} + +/// A new token resets every caller: telemetry fetching one ends another +/// caller's quiet period, which was set with the old token. +#[test] +fn a_new_token_telemetry_fetches_ends_another_callers_quiet_period() { + let home = scratch("send-new-token-resets-others"); + save_cli_token_state( + &home, + serde_json::json!({"token": "pk.old", "rejected": true, "callers": {"other": + {"count": 3, "quiet_until": now() + 3000, "token": "pk.old"}}}), + ); + let (_, token_url) = answer_once("200 OK", r#"{"token":"pk.new"}"#); + let (received, url) = events_server(true); + run_against(&home, &url, &token_url, NOWHERE); + assert_eq!(sent_with(&received), "pk.new"); + let state = cli_token_state(&home, "pk.new"); + assert!(state.get("callers").is_none(), "{state}"); +} + +/// `report` fetching a new token while telemetry is quiet ends telemetry's +/// quiet period, so the run's own event goes out with that token. +#[test] +fn a_new_token_report_fetches_ends_telemetrys_quiet_period() { + let home = scratch("send-report-resets-telemetry"); + save_cli_token_state( + &home, + serde_json::json!({"token": "pk.dead", "rejected": true, "callers": {"telemetry": + {"count": 3, "quiet_until": now() + 3000, "token": "pk.dead"}}}), + ); + let (fetched, token_url) = answer_once("200 OK", r#"{"token":"pk.fresh"}"#); + let (received, url) = events_server(true); + // The report itself goes to a dead proxy, so nothing reaches Mapbox. + let out = command(&home) + .env("MAPBOX_INTERNAL_TELEMETRY_LOG", "1") + .env("MAPBOX_INTERNAL_TELEMETRY_URL", &url) + .env("MAPBOX_INTERNAL_CLI_TOKEN_URL", &token_url) + .env("HTTPS_PROXY", "http://127.0.0.1:9") + .env_remove("https_proxy") + .env_remove("ALL_PROXY") + .env_remove("all_proxy") + .env_remove("NO_PROXY") + .env_remove("no_proxy") + .args(["report", "A test"]) + .output() + .expect("run mapbox"); + assert!(!out.status.success(), "the report reached somewhere"); + fetched + .recv_timeout(Duration::from_secs(10)) + .expect("report fetched a token"); + assert_eq!(sent_with(&received), "pk.fresh"); + let state = cli_token_state(&home, "pk.fresh"); + assert!(state.get("callers").is_none(), "{state}"); }