mirror of
https://github.com/ruvnet/RuView
synced 2026-07-26 18:01:48 +00:00
c72bbc15dd
Findings from a qe-court adversarial round (4 prosecutors across 2 vendors).
Each was verified against the code before being accepted; the ones below
reproduced, the rest are reported in the PR thread rather than acted on.
FATAL — `/api/field` was reachable with no credential, on both listeners.
The gate protected `/api/v1/*` by prefix. `/api/field` is the REST sibling of
`/ws/field` and serves the same signed FieldEvent stream — live presence, pose,
vitals. `/ws/field` was gated in this PR; its twin one path segment over was
not. Measured with RUVIEW_API_TOKEN set and no credential supplied:
/api/v1/models 401 (control)
/ws/field 401 (gated by this PR)
/api/field 200 on :8080 AND :8765
Fixed by inverting the gate to deny-by-default with an explicit anonymous
allowlist (`/`, `/ui`, `/health`, `/oauth/`). A route added at a new path is
now gated because nobody exposed it, rather than exposed because nobody
protected it — the same inversion already applied to the scope gate.
FATAL — the wiring test disarmed itself exactly when it mattered.
`Server::start` returned an Option that all five tests turned into `return`,
so a server that failed to boot produced "5 passed" with zero assertions run,
and cargo swallows the skip line without --nocapture. The one test that
observes real wiring — the guard against both shipped bypasses — was silent
for any change that breaks startup, including a boot panic in the auth path.
It now panics with the child's stderr.
MAJOR — a malformed client-id list silently disabled the audience check.
An empty allowlist is the opt-out sentinel in verify.rs. `RUVIEW_OAUTH_CLIENT_IDS=","`
is non-empty, passes the guard, then filters to an empty Vec — turning the
audience boundary off with no log and admitting a token minted for any other
Cognitum product. Only a literal `*` may opt out now; anything else that parses
to nothing warns and falls back to the default. Same fail-open shape as the
scope denylist this PR already had to invert.
MAJOR — credentials were world-readable for a window on every refresh.
`fs::write` creates at 0666 & !umask (0644 by default), and both writers
chmodded afterwards. The existing permissions test asserted on the FINAL file
and passed throughout. Affected the CLI refresh token (rotated with reuse
detection — a thief who presents it first takes the session family) and the
browser session secret (the HMAC key for every session; stealing it forges any
account at any scope). Both now create with mode 0600 via OpenOptions.
MAJOR — ui/sw.js cached authenticated API responses.
Closing the /oauth/ leg left the /api/ leg open. `networkFirst` cached every
successful response, keyed by URL alone, purged by nothing at sign-out: sign in
as A, load sensing data, sign out, sign in as B, lose the network, and B is
served A's data with no authorization check. API responses are now network-only
— which is also the correct behaviour for a live sensing dashboard, where
replaying a stale reading can show a room occupied after the person left — plus
a cache purge on sign-out.
CI — 40 of ruview-auth's 87 tests never ran.
The workspace runs --no-default-features, which switches off the `login` and
`pkce` features. Measured: 47 tests vs 87. The whole interactive sign-in path —
credential storage, single-flight refresh, the file lock, the loopback callback
— was green locally and never executed in CI. Added an --all-features step.
(Checked the sensing-server for the same problem and did NOT find it:
bearer_auth's 49 and browser_session's 15 do run under CI flags.)
Tests: +2 wiring tests (one fails against the old gate with
"http port served /api/field to an anonymous caller", passes after), +3 UI
service-worker tests, +3 CLI scope tests, +1 temp-file permission test, +1
client-id parsing test. wifi-densepose-cli/src/auth.rs had zero tests and
builds its own scope string, so the library's least-privilege test said nothing
about what the CLI requests.
Verified: ruview-auth 61+25+2 pass (--all-features), sensing-server bearer_auth
50, browser_session 15, auth_wiring 7, workspace 25 suites clean, UI 22.
Co-Authored-By: Ruflo & AQE
185 lines
7.1 KiB
Rust
185 lines
7.1 KiB
Rust
//! `wifi-densepose login` / `logout` / `whoami` — Cognitum sign-in (ADR-271).
|
|
//!
|
|
//! Signing in yields a Cognitum access token that a RuView sensing server
|
|
//! verifies offline against `auth.cognitum.one`'s published JWKS. It replaces
|
|
//! sharing one `RUVIEW_API_TOKEN` string between everyone who needs access:
|
|
//! requests become attributable to a person, and destructive routes can be
|
|
//! separated from read-only ones by scope.
|
|
|
|
use std::path::PathBuf;
|
|
|
|
use clap::Args;
|
|
use ruview_auth::login::{self, LoginOptions};
|
|
use ruview_auth::scope;
|
|
|
|
#[derive(Debug, Args)]
|
|
pub struct LoginArgs {
|
|
/// Also request `sensing:admin` — the capability to train models and delete
|
|
/// models and recordings.
|
|
///
|
|
/// Off by default on purpose. A session that only streams poses has no
|
|
/// business holding delete capability, and a token that carries it is a
|
|
/// bigger loss if it leaks. Ask for it when you are about to do
|
|
/// administrative work, not as a matter of habit.
|
|
#[arg(long)]
|
|
pub admin: bool,
|
|
|
|
/// Skip the browser and use the paste-a-code flow.
|
|
///
|
|
/// Detected automatically over SSH and inside containers; this forces it.
|
|
#[arg(long)]
|
|
pub no_browser: bool,
|
|
|
|
/// Where to store credentials. Defaults to `~/.ruview/credentials.json`.
|
|
#[arg(long, env = ruview_auth::login::CREDENTIALS_PATH_ENV)]
|
|
pub credentials_path: Option<PathBuf>,
|
|
}
|
|
|
|
#[derive(Debug, Args)]
|
|
pub struct LogoutArgs {
|
|
#[arg(long, env = ruview_auth::login::CREDENTIALS_PATH_ENV)]
|
|
pub credentials_path: Option<PathBuf>,
|
|
}
|
|
|
|
#[derive(Debug, Args)]
|
|
pub struct WhoamiArgs {
|
|
#[arg(long, env = ruview_auth::login::CREDENTIALS_PATH_ENV)]
|
|
pub credentials_path: Option<PathBuf>,
|
|
|
|
/// Refresh the access token now if it has expired, instead of only
|
|
/// reporting that it will be refreshed on next use.
|
|
///
|
|
/// Refreshing rotates the stored refresh token — identity spends the old
|
|
/// one — so this is a real state change, not a read. That is why it is a
|
|
/// flag rather than something `whoami` does silently.
|
|
#[arg(long)]
|
|
pub refresh: bool,
|
|
}
|
|
|
|
fn path_or_default(p: Option<PathBuf>) -> PathBuf {
|
|
p.unwrap_or_else(login::default_credentials_path)
|
|
}
|
|
|
|
/// What `login` asks the authorization server for.
|
|
///
|
|
/// Extracted so it is testable on its own. `LoginOptions::default()` has its own
|
|
/// least-privilege test in the library, but this command does NOT go through
|
|
/// that default — it builds the scope string itself, so the library test says
|
|
/// nothing about what the CLI actually requests.
|
|
fn requested_scope(admin: bool) -> String {
|
|
if admin {
|
|
// Admin implies read: there is no scope hierarchy server-side, so a
|
|
// session that needs both must consent to both explicitly.
|
|
format!("{} {}", scope::SENSING_READ, scope::SENSING_ADMIN)
|
|
} else {
|
|
scope::SENSING_READ.to_string()
|
|
}
|
|
}
|
|
|
|
pub async fn login_cmd(args: LoginArgs) -> anyhow::Result<()> {
|
|
let scope = requested_scope(args.admin);
|
|
|
|
let opts = LoginOptions {
|
|
credentials_path: path_or_default(args.credentials_path),
|
|
scope,
|
|
no_browser: args.no_browser,
|
|
};
|
|
|
|
let mut out = std::io::stdout();
|
|
let stdin = std::io::stdin();
|
|
let mut input = stdin.lock();
|
|
|
|
login::login(&opts, &mut out, &mut input).await?;
|
|
Ok(())
|
|
}
|
|
|
|
pub async fn logout_cmd(args: LogoutArgs) -> anyhow::Result<()> {
|
|
let path = path_or_default(args.credentials_path);
|
|
if login::logout(&path)? {
|
|
println!("Signed out — {} removed.", path.display());
|
|
} else {
|
|
println!("Not signed in; nothing to remove.");
|
|
}
|
|
// Deliberately local-only. This makes the machine unable to act as you;
|
|
// revoking the session for every device is an account-level action.
|
|
println!("Note: this forgets the local credential only. It does not revoke the session server-side.");
|
|
Ok(())
|
|
}
|
|
|
|
pub async fn whoami_cmd(args: WhoamiArgs) -> anyhow::Result<()> {
|
|
let path = path_or_default(args.credentials_path);
|
|
let mut creds = ruview_auth::login::store::load(&path)?;
|
|
|
|
if args.refresh && creds.needs_refresh() {
|
|
println!("Access token expired — refreshing…");
|
|
let session = ruview_auth::login::Session::load_from(path.clone(), reqwest::Client::new())?;
|
|
// Goes through ensure_fresh, so it inherits the single-flight guarantee
|
|
// and the persist-before-return ordering rather than reimplementing a
|
|
// second, subtly different refresh path.
|
|
session.ensure_fresh().await?;
|
|
creds = session.snapshot().await;
|
|
println!("Refreshed.\n");
|
|
}
|
|
|
|
println!("Credentials: {}", path.display());
|
|
println!("Issuer: {}", creds.issuer);
|
|
match &creds.account_email {
|
|
Some(e) => println!("Account: {e}"),
|
|
None => println!("Account: (not reported)"),
|
|
}
|
|
// Falls back to the token's own claim, so a file written before that
|
|
// fallback existed still reports its real scope.
|
|
match creds.effective_scope() {
|
|
Some(s) => println!("Scope: {s}"),
|
|
None => println!("Scope: (not reported)"),
|
|
}
|
|
// State, not just contents: an expired-looking session is the single most
|
|
// common reason a command starts 401ing, so say it plainly here rather than
|
|
// letting the user infer it from a failure elsewhere.
|
|
if creds.needs_refresh() {
|
|
println!("Status: access token expired or expiring — pass --refresh to renew it now");
|
|
} else {
|
|
println!("Status: access token valid");
|
|
}
|
|
if creds.refresh_token.is_none() {
|
|
println!("Warning: no refresh token stored; you will need to sign in again when this expires");
|
|
}
|
|
Ok(())
|
|
}
|
|
|
|
#[cfg(test)]
|
|
mod tests {
|
|
use super::*;
|
|
|
|
#[test]
|
|
fn a_plain_login_asks_for_read_only() {
|
|
// The whole point of splitting the scopes (ADR-060) is that streaming
|
|
// poses must not carry the capability to delete recordings. If this
|
|
// ever returns admin by default, every session silently becomes
|
|
// destructive-capable and nothing else in the suite would notice.
|
|
let s = requested_scope(false);
|
|
assert_eq!(s, scope::SENSING_READ);
|
|
assert!(!s.contains(scope::SENSING_ADMIN), "read-only login leaked admin: {s}");
|
|
}
|
|
|
|
#[test]
|
|
fn admin_login_asks_for_both_because_there_is_no_hierarchy() {
|
|
// The authorization server grants exactly what is requested; admin does
|
|
// not imply read. Asking for admin alone would produce a session that
|
|
// cannot stream.
|
|
let s = requested_scope(true);
|
|
assert!(s.split_whitespace().any(|x| x == scope::SENSING_READ), "{s}");
|
|
assert!(s.split_whitespace().any(|x| x == scope::SENSING_ADMIN), "{s}");
|
|
}
|
|
|
|
#[test]
|
|
fn an_explicit_credentials_path_is_honoured_over_the_default() {
|
|
// `--credentials-path` also carries the RUVIEW_CREDENTIALS_PATH env
|
|
// binding; silently ignoring it would write credentials somewhere the
|
|
// operator did not choose.
|
|
let p = PathBuf::from("/tmp/ruview-cli-explicit-credentials.json");
|
|
assert_eq!(path_or_default(Some(p.clone())), p);
|
|
assert_eq!(path_or_default(None), login::default_credentials_path());
|
|
}
|
|
}
|