From d3fd8ebf9af1b73aa1c6fe93d75b6d747ed039d1 Mon Sep 17 00:00:00 2001 From: GhostFrame Date: Fri, 31 Jul 2026 20:44:04 -0400 Subject: [PATCH 1/2] fix(auth): support first-party native sessions Add invite registration and password login to the client and CLI. Persist provider-tagged native sessions while migrating existing OIDC metadata. Verify first-party and OIDC lifecycle behavior with focused tests. --- README.md | 29 +- crates/frameshift-cli/src/cmd/account.rs | 434 ++++++++++++++---- crates/frameshift-client/Cargo.toml | 2 +- crates/frameshift-client/src/account.rs | 375 ++++++++++++++- crates/frameshift-client/src/session.rs | 96 +++- crates/frameshift-client/src/session_store.rs | 401 ++++++++++++---- docs/wiki/Accounts-and-Publisher-Identity.md | 24 +- docs/wiki/CLI-Reference.md | 7 +- docs/wiki/Local-Data-and-Privacy.md | 19 +- 9 files changed, 1178 insertions(+), 209 deletions(-) diff --git a/README.md b/README.md index b8e7f31..0411e8a 100644 --- a/README.md +++ b/README.md @@ -102,21 +102,31 @@ Registry commands use `https://frameshift-api.syntheos.dev` by default. Set `FRA ## Account sessions -`frameshift account login` opens the configured OIDC provider in the system -browser and receives the Authorization Code response on an exact IP-loopback -callback. The flow uses S256 PKCE, a random state value, and a random nonce. -Access and refresh tokens are stored only in the operating system's native -credential store; the owner-only JSON file under the FrameShift data directory -contains non-secret issuer, client, registry, scope, and expiry metadata. +FrameShift supports registry-owned first-party accounts and OIDC providers. +First-party registration and login collect credentials only through hidden +interactive terminal prompts. OIDC login opens the configured provider in the +system browser and receives the Authorization Code response on an exact +IP-loopback callback using S256 PKCE, random state, and a random nonce. + +All native bearer credentials are stored only in the operating system's +credential store. The owner-only JSON file under the FrameShift data directory +contains provider-tagged public metadata and never contains a token, password, +or invitation. ```bash -# Use the production registry's advertised issuer and the frameshift-cli client ID. +# Redeem a single-use invitation and create a first-party account. +frameshift account register + +# Use the registry's advertised provider. OIDC remains preferred when both exist. frameshift account login +# Explicitly use first-party password login when OIDC is also advertised. +frameshift account login --first-party + # Confirm the server-validated account and publisher memberships. frameshift account status -# Attempt provider revocation, then erase the exact local credential and metadata. +# Revoke the provider session, then erase the exact local credential and metadata. frameshift account logout ``` @@ -124,7 +134,8 @@ Deployments that register a different public OAuth client set `FRAMESHIFT_OIDC_CLIENT_ID`. `FRAMESHIFT_OIDC_ISSUER` overrides registry issuer discovery, and `--redirect-uri` selects another pre-registered IP-loopback callback. Login never accepts a bearer token through arguments, environment -variables, stdin, or copy/paste. +variables, or command-line values. First-party credentials require an +interactive terminal and are read through hidden prompts. ## Automate mode diff --git a/crates/frameshift-cli/src/cmd/account.rs b/crates/frameshift-cli/src/cmd/account.rs index 1aa229d..315db77 100644 --- a/crates/frameshift-cli/src/cmd/account.rs +++ b/crates/frameshift-cli/src/cmd/account.rs @@ -1,20 +1,24 @@ -//! Interactive account login, status, and logout commands. +//! Interactive account registration, login, status, and logout commands. //! -//! Login uses the system browser and a loopback Authorization Code callback -//! with S256 PKCE. Tokens never enter command-line arguments or terminal input. +//! OIDC login uses the system browser and a loopback Authorization Code callback +//! with S256 PKCE. First-party credentials use hidden terminal prompts. No +//! password, invitation, or token is accepted through arguments or environment. -use std::io::{Read as _, Write as _}; +use std::io::{IsTerminal as _, Read as _, Write as _}; use std::net::{IpAddr, SocketAddr, TcpListener, TcpStream}; use std::time::{Duration, Instant, SystemTime, UNIX_EPOCH}; use clap::{Args, Subcommand}; -use frameshift_client::account::{get_account, get_auth_config, AccountView}; -use frameshift_client::session::{OidcSession, SessionClient, SessionClientConfig}; +use frameshift_client::account::{ + get_account, get_auth_config, login_local_account, logout_local_account, + register_local_account, AccountView, LocalAccountSession, NativeAuthClient, +}; +use frameshift_client::session::{AuthenticatedSession, SessionClient, SessionClientConfig}; use frameshift_client::session_store::{ - SessionStore, SessionStoreError, SessionStoreMetadata, StoredSession, + SessionAuthentication, SessionStore, SessionStoreError, SessionStoreMetadata, StoredSession, }; use frameshift_client::{registry_base_url, Client, ClientError}; -use secrecy::SecretString; +use secrecy::{ExposeSecret as _, SecretString}; use url::{Position, Url}; use crate::util::{validate_server_url, CliError}; @@ -41,20 +45,25 @@ pub struct AccountArgs { /// Account session operations. #[derive(Debug, Subcommand)] pub enum AccountCommand { - /// Authenticate in the system browser and save the session securely. + /// Authenticate through an advertised provider and save the session securely. Login(AccountLoginArgs), + /// Redeem a first-party invitation and save the new session securely. + Register(AccountRegisterArgs), /// Fetch and print the current authenticated account. Status, /// Revoke the provider session when supported and erase local credentials. Logout, } -/// Browser login options. +/// Account login options. #[derive(Debug, Args)] pub struct AccountLoginArgs { /// Registry API base URL; defaults to `FRAMESHIFT_REGISTRY_URL` or production. #[arg(long)] pub server: Option, + /// Use first-party password login when the registry also advertises OIDC. + #[arg(long)] + pub first_party: bool, /// OIDC issuer override; defaults to `FRAMESHIFT_OIDC_ISSUER` or registry discovery. #[arg(long)] pub issuer: Option, @@ -76,6 +85,22 @@ pub struct AccountLoginArgs { pub timeout_secs: u64, } +/// First-party invitation registration options. +#[derive(Debug, Args)] +pub struct AccountRegisterArgs { + /// Registry API base URL; defaults to `FRAMESHIFT_REGISTRY_URL` or production. + #[arg(long)] + pub server: Option, +} + +/// Authentication mode selected from explicit intent and registry capabilities. +enum AccountLoginMode { + /// OIDC system-browser flow through the exact issuer. + Oidc(Url), + /// First-party password flow through hidden terminal prompts. + FirstParty, +} + /// Accepted callback URL paired with its response stream. struct PendingCallback { /// Exact callback URL reconstructed from the registered redirect. @@ -88,17 +113,30 @@ struct PendingCallback { pub fn run_account(args: AccountArgs) -> Result<(), CliError> { match args.command { AccountCommand::Login(args) => run_login(args), + AccountCommand::Register(args) => run_register(args), AccountCommand::Status => run_status(), AccountCommand::Logout => run_logout(), } } -/// Authenticate through the browser and persist the resulting session. +/// Authenticate through the selected provider and persist the resulting session. fn run_login(args: AccountLoginArgs) -> Result<(), CliError> { - let server = args.server.unwrap_or_else(registry_base_url); + let server = args.server.clone().unwrap_or_else(registry_base_url); validate_server_url(&server)?; let registry_url = Url::parse(&server).map_err(|error| CliError::Account(error.to_string()))?; - let issuer = resolve_issuer(&server, args.issuer)?; + match resolve_login_mode(&server, args.first_party, args.issuer.clone())? { + AccountLoginMode::Oidc(issuer) => run_oidc_login(args, server, registry_url, issuer), + AccountLoginMode::FirstParty => run_first_party_login(&server, registry_url), + } +} + +/// Complete OIDC system-browser login and persist its refreshable session. +fn run_oidc_login( + args: AccountLoginArgs, + server: String, + registry_url: Url, + issuer: Url, +) -> Result<(), CliError> { let client_id = resolve_client_id(args.client_id)?; let redirect_uri = Url::parse(&args.redirect_uri) .map_err(|error| CliError::Account(format!("invalid redirect URI: {error}")))?; @@ -128,10 +166,12 @@ fn run_login(args: AccountLoginArgs) -> Result<(), CliError> { let store = SessionStore::new(client.data_root()); if let Err(error) = store.save( SessionStoreMetadata { - issuer, - client_id, - redirect_uri, - scopes: args.scopes, + authentication: SessionAuthentication::Oidc { + issuer, + client_id, + redirect_uri, + scopes: args.scopes, + }, registry_url, }, &session, @@ -144,26 +184,95 @@ fn run_login(args: AccountLoginArgs) -> Result<(), CliError> { Ok(()) } +/// Prompt for first-party credentials and persist one opaque bearer session. +fn run_first_party_login(server: &str, registry_url: Url) -> Result<(), CliError> { + require_interactive_terminal()?; + let email = prompt_line("Email: ")?; + let password = prompt_secret("Password: ", "password")?; + let authenticated = login_local_account(server, &email, &password, NativeAuthClient::Cli) + .map_err(|error| CliError::Account(error.to_string()))?; + persist_first_party_session(registry_url, &authenticated)?; + let view = get_account(server, authenticated.session.access_token()) + .map_err(|error| CliError::Account(error.to_string()))?; + println!("logged in to {server}"); + print_account(&view); + Ok(()) +} + +/// Redeem one invitation through hidden prompts and persist the initial session. +fn run_register(args: AccountRegisterArgs) -> Result<(), CliError> { + let server = args.server.unwrap_or_else(registry_base_url); + validate_server_url(&server)?; + let config = get_auth_config(&server).map_err(|error| CliError::Account(error.to_string()))?; + if !config.first_party_enabled { + return Err(CliError::Account( + "registry does not advertise first-party account registration".to_string(), + )); + } + if config.registration.as_deref() != Some("invite_only") { + return Err(CliError::Account( + "registry does not advertise invite-only account registration".to_string(), + )); + } + require_interactive_terminal()?; + let invite = prompt_secret("Invitation token: ", "invitation token")?; + let email = prompt_line("Email: ")?; + let display_name = prompt_line_allow_empty("Display name (optional): ")?; + let password = prompt_confirmed_password()?; + let authenticated = register_local_account( + &server, + &invite, + &email, + (!display_name.is_empty()).then_some(display_name.as_str()), + &password, + NativeAuthClient::Cli, + ) + .map_err(|error| CliError::Account(error.to_string()))?; + let registry_url = Url::parse(&server).map_err(|error| CliError::Account(error.to_string()))?; + persist_first_party_session(registry_url, &authenticated)?; + println!("registered and logged in to {server}"); + Ok(()) +} + +/// Persist one first-party session and revoke it if local storage fails. +fn persist_first_party_session( + registry_url: Url, + authenticated: &LocalAccountSession, +) -> Result<(), CliError> { + let client = Client::with_default_data_root()?; + let store = SessionStore::new(client.data_root()); + if let Err(error) = store.save( + SessionStoreMetadata { + authentication: SessionAuthentication::FirstParty, + registry_url: registry_url.clone(), + }, + &authenticated.session, + ) { + let _ = logout_local_account(registry_url.as_str(), authenticated.session.access_token()); + return Err(account_store_error(error)); + } + Ok(()) +} + /// Load, refresh when needed, and print the current registry account. fn run_status() -> Result<(), CliError> { let client = Client::with_default_data_root()?; let store = SessionStore::new(client.data_root()); let mut stored = store.load().map_err(account_store_error)?; - let session_client = session_client_for(&stored)?; - if session_expires_soon(&stored.session) && stored.session.refresh_token().is_some() { - stored.session = session_client - .refresh(&stored.session) - .map_err(|error| CliError::Account(error.to_string()))?; - persist_loaded_session(&store, &stored)?; - } + refresh_loaded_session_if_needed(&store, &mut stored)?; let view = match get_account( stored.metadata.registry_url.as_str(), stored.session.access_token(), ) { Ok(view) => view, Err(ClientError::RegistryRejected { status: 401, .. }) - if stored.session.refresh_token().is_some() => + if stored.session.refresh_token().is_some() + && matches!( + &stored.metadata.authentication, + SessionAuthentication::Oidc { .. } + ) => { + let session_client = session_client_for(&stored)?; stored.session = session_client .refresh(&stored.session) .map_err(|error| CliError::Account(error.to_string()))?; @@ -191,13 +300,7 @@ pub(crate) fn access_token_for_registry(server: &str) -> Result Result<(), CliError> { Err(error) => return Err(account_store_error(error)), }; if let Some(stored) = &stored { - match session_client_for(stored).and_then(|client| { - client - .revoke(&stored.session) - .map_err(|error| CliError::Account(error.to_string())) - }) { + let revocation = match &stored.metadata.authentication { + SessionAuthentication::Oidc { .. } => session_client_for(stored).and_then(|client| { + client + .revoke(&stored.session) + .map_err(|error| CliError::Account(error.to_string())) + }), + SessionAuthentication::FirstParty => logout_local_account( + stored.metadata.registry_url.as_str(), + stored.session.access_token(), + ) + .map_err(|error| CliError::Account(error.to_string())), + }; + match revocation { Ok(()) => {} Err(CliError::Account(message)) if message.contains("does not advertise a revocation endpoint") => {} @@ -249,39 +360,49 @@ fn run_logout() -> Result<(), CliError> { Ok(()) } -/// Resolve the issuer from an override, environment, or registry bootstrap. -fn resolve_issuer(server: &str, override_value: Option) -> Result { - let value = override_value - .or_else(|| nonempty_env("FRAMESHIFT_OIDC_ISSUER")) - .map(Ok) - .unwrap_or_else(|| { - let config = - get_auth_config(server).map_err(|error| CliError::Account(error.to_string()))?; - if !config.enabled { - return Err(CliError::Account( - "registry account authentication is disabled".to_string(), - )); - } - config.issuer.ok_or_else(|| { - // A registry running in first-party-only mode reports - // enabled=true with no OIDC issuer. `frameshift account login` - // only speaks the OIDC browser flow today, so surface an - // accurate, actionable message instead of implying the registry - // misreported an issuer it never advertised. - if config.first_party_enabled { - CliError::Account( - "this registry uses invite-only first-party accounts; \ - `frameshift account login` currently supports only OIDC sign-in. \ - Sign in through the FrameShift web portal, or set FRAMESHIFT_ACCESS_TOKEN \ - to an issued session token for CLI publisher-key commands" - .to_string(), - ) - } else { - CliError::Account("registry omitted its enabled OIDC issuer".to_string()) - } - }) - })?; - Url::parse(&value).map_err(|error| CliError::Account(format!("invalid OIDC issuer: {error}"))) +/// Resolve first-party or OIDC login from explicit intent and registry capability. +fn resolve_login_mode( + server: &str, + first_party: bool, + issuer_override: Option, +) -> Result { + if first_party && issuer_override.is_some() { + return Err(CliError::Account( + "--first-party cannot be combined with --issuer".to_string(), + )); + } + if !first_party { + if let Some(value) = issuer_override.or_else(|| nonempty_env("FRAMESHIFT_OIDC_ISSUER")) { + let issuer = Url::parse(&value) + .map_err(|error| CliError::Account(format!("invalid OIDC issuer: {error}")))?; + return Ok(AccountLoginMode::Oidc(issuer)); + } + } + let config = get_auth_config(server).map_err(|error| CliError::Account(error.to_string()))?; + if !config.enabled { + return Err(CliError::Account( + "registry account authentication is disabled".to_string(), + )); + } + if first_party { + return config + .first_party_enabled + .then_some(AccountLoginMode::FirstParty) + .ok_or_else(|| { + CliError::Account("registry does not advertise first-party login".to_string()) + }); + } + if let Some(value) = config.issuer { + let issuer = Url::parse(&value) + .map_err(|error| CliError::Account(format!("invalid OIDC issuer: {error}")))?; + return Ok(AccountLoginMode::Oidc(issuer)); + } + if config.first_party_enabled { + return Ok(AccountLoginMode::FirstParty); + } + Err(CliError::Account( + "registry omitted every enabled authentication provider".to_string(), + )) } /// Resolve and validate the public OAuth client identifier. @@ -304,6 +425,60 @@ fn nonempty_env(name: &str) -> Option { .filter(|value| !value.trim().is_empty()) } +/// Require an interactive terminal before reading first-party credentials. +fn require_interactive_terminal() -> Result<(), CliError> { + if std::io::stdin().is_terminal() && std::io::stderr().is_terminal() { + Ok(()) + } else { + Err(CliError::Account( + "first-party credentials require an interactive terminal".to_string(), + )) + } +} + +/// Read one required visible line without accepting control characters. +fn prompt_line(prompt: &str) -> Result { + let value = prompt_line_allow_empty(prompt)?; + if value.is_empty() { + return Err(CliError::Account("a required value was empty".to_string())); + } + Ok(value) +} + +/// Read one optional visible line from the interactive terminal. +fn prompt_line_allow_empty(prompt: &str) -> Result { + eprint!("{prompt}"); + std::io::stderr().flush()?; + let mut value = String::new(); + std::io::stdin().read_line(&mut value)?; + let value = value.trim_end_matches(['\r', '\n']).to_string(); + if value.chars().any(char::is_control) { + return Err(CliError::Account( + "interactive account values must not contain control characters".to_string(), + )); + } + Ok(value) +} + +/// Read one non-empty secret through a hidden terminal prompt. +fn prompt_secret(prompt: &str, label: &str) -> Result { + let value = rpassword::prompt_password(prompt)?; + if value.is_empty() { + return Err(CliError::Account(format!("{label} cannot be empty"))); + } + Ok(SecretString::new(value)) +} + +/// Read and exactly confirm one new password through hidden prompts. +fn prompt_confirmed_password() -> Result { + let password = prompt_secret("Password: ", "password")?; + let confirmation = prompt_secret("Confirm password: ", "password confirmation")?; + if password.expose_secret() != confirmation.expose_secret() { + return Err(CliError::Account("passwords did not match".to_string())); + } + Ok(password) +} + /// Bind only the exact IP loopback callback address and explicit port. fn bind_callback_listener(redirect_uri: &Url) -> Result { if redirect_uri.scheme() != "http" @@ -444,32 +619,60 @@ fn respond_to_browser(stream: &mut TcpStream, success: bool) { /// Recreate the provider client for persisted metadata. fn session_client_for(stored: &StoredSession) -> Result { + let SessionAuthentication::Oidc { + issuer, + client_id, + redirect_uri, + scopes, + } = &stored.metadata.authentication + else { + return Err(CliError::Account( + "first-party sessions do not use OIDC discovery".to_string(), + )); + }; SessionClient::discover(SessionClientConfig { - issuer: stored.metadata.issuer.clone(), - client_id: stored.metadata.client_id.clone(), - redirect_uri: stored.metadata.redirect_uri.clone(), - scopes: stored.metadata.scopes.clone(), + issuer: issuer.clone(), + client_id: client_id.clone(), + redirect_uri: redirect_uri.clone(), + scopes: scopes.clone(), }) .map_err(|error| CliError::Account(error.to_string())) } /// Return whether an access token is expired or within the refresh margin. -fn session_expires_soon(session: &OidcSession) -> bool { +fn session_expires_soon(session: &AuthenticatedSession) -> bool { session .summary() .expires_at .is_some_and(|expiry| expiry <= unix_now().saturating_add(REFRESH_MARGIN_SECS)) } +/// Refresh one expiring OIDC session while leaving first-party sessions untouched. +fn refresh_loaded_session_if_needed( + store: &SessionStore, + stored: &mut StoredSession, +) -> Result<(), CliError> { + if matches!( + &stored.metadata.authentication, + SessionAuthentication::Oidc { .. } + ) && session_expires_soon(&stored.session) + && stored.session.refresh_token().is_some() + { + let session_client = session_client_for(stored)?; + stored.session = session_client + .refresh(&stored.session) + .map_err(|error| CliError::Account(error.to_string()))?; + persist_loaded_session(store, stored)?; + } + Ok(()) +} + /// Rewrite one refreshed session using its existing public metadata. fn persist_loaded_session(store: &SessionStore, stored: &StoredSession) -> Result<(), CliError> { store .save( SessionStoreMetadata { - issuer: stored.metadata.issuer.clone(), - client_id: stored.metadata.client_id.clone(), - redirect_uri: stored.metadata.redirect_uri.clone(), - scopes: stored.metadata.scopes.clone(), + authentication: stored.metadata.authentication.clone(), registry_url: stored.metadata.registry_url.clone(), }, &stored.session, @@ -514,6 +717,79 @@ fn unix_now() -> u64 { /// Deterministic account command tests. mod tests { use super::*; + use clap::Parser as _; + + /// Minimal parser that exercises the public account command hierarchy. + #[derive(Debug, clap::Parser)] + #[command(name = "frameshift")] + struct TestCli { + /// Parsed top-level command. + #[command(subcommand)] + command: TestCommand, + } + + /// Top-level command subset needed for account parser tests. + #[derive(Debug, clap::Subcommand)] + enum TestCommand { + /// Account-session command group. + Account(AccountArgs), + } + + /// Registration accepts only public connection options, never credential material. + #[test] + fn parses_registration_without_secret_arguments() { + let parsed = TestCli::try_parse_from([ + "frameshift", + "account", + "register", + "--server", + "https://registry.example", + ]) + .expect("registration arguments"); + let TestCommand::Account(AccountArgs { + command: AccountCommand::Register(arguments), + }) = parsed.command + else { + panic!("expected account registration"); + }; + assert_eq!( + arguments.server.as_deref(), + Some("https://registry.example") + ); + + for secret_flag in ["--password", "--invite-token"] { + assert!(TestCli::try_parse_from([ + "frameshift", + "account", + "register", + secret_flag, + "secret", + ]) + .is_err()); + } + } + + /// Login exposes an explicit first-party selector without accepting a password argument. + #[test] + fn parses_first_party_login_without_password_argument() { + let parsed = TestCli::try_parse_from(["frameshift", "account", "login", "--first-party"]) + .expect("first-party login arguments"); + let TestCommand::Account(AccountArgs { + command: AccountCommand::Login(arguments), + }) = parsed.command + else { + panic!("expected account login"); + }; + assert!(arguments.first_party); + assert!(TestCli::try_parse_from([ + "frameshift", + "account", + "login", + "--password", + "secret", + ]) + .is_err()); + } /// Registry matching ignores a cosmetic trailing slash only. #[test] diff --git a/crates/frameshift-client/Cargo.toml b/crates/frameshift-client/Cargo.toml index 29a05b4..c44d84b 100644 --- a/crates/frameshift-client/Cargo.toml +++ b/crates/frameshift-client/Cargo.toml @@ -34,6 +34,7 @@ frameshift-vault = { path = "../frameshift-vault" } frameshift-vault-local = { path = "../frameshift-vault-local" } age.workspace = true base64.workspace = true +chrono.workspace = true ed25519-dalek.workspace = true # Serializes cross-process publisher-key inventory mutations. fs2.workspace = true @@ -68,4 +69,3 @@ libc = "0.2" tempfile.workspace = true [dev-dependencies] -chrono.workspace = true diff --git a/crates/frameshift-client/src/account.rs b/crates/frameshift-client/src/account.rs index b9d87af..3e3d7e9 100644 --- a/crates/frameshift-client/src/account.rs +++ b/crates/frameshift-client/src/account.rs @@ -3,11 +3,14 @@ //! Access tokens enter only through [`SecretString`] and are attached solely //! to the registry request's `Authorization` header. +use chrono::{DateTime, Utc}; use frameshift_catalog::{AccountRecord, PublisherMembershipRecord, PublisherProfileRecord}; -use secrecy::SecretString; -use serde::Deserialize; +use secrecy::{ExposeSecret as _, SecretString}; +use serde::{Deserialize, Serialize}; +use zeroize::Zeroizing; use crate::error::ClientError; +use crate::session::AuthenticatedSession; /// Public account-authentication bootstrap configuration. #[derive(Debug, Clone, PartialEq, Eq, Deserialize)] @@ -28,6 +31,96 @@ pub struct AccountAuthConfig { /// advertise the field deserialize unchanged. #[serde(default)] pub first_party_enabled: bool, + /// First-party registration policy advertised by the registry. + #[serde(default)] + pub registration: Option, +} + +/// Native first-party client presentation requested from the registry. +#[derive(Debug, Clone, Copy, PartialEq, Eq, Serialize)] +#[serde(rename_all = "snake_case")] +pub enum NativeAuthClient { + /// Desktop application session. + Desktop, + /// Command-line session. + Cli, +} + +/// Successful first-party authentication with its secret-bearing session. +pub struct LocalAccountSession { + /// Durable authenticated account returned by the registry. + pub account: AccountRecord, + /// Opaque native bearer session. + pub session: AuthenticatedSession, +} + +/// Redacted diagnostics for one local authentication result. +impl std::fmt::Debug for LocalAccountSession { + /// Render public account metadata and the session's redacted representation. + fn fmt(&self, formatter: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { + formatter + .debug_struct("LocalAccountSession") + .field("account", &self.account) + .field("session", &self.session) + .finish() + } +} + +/// Password login request serialized only at the HTTP boundary. +#[derive(Serialize)] +struct LoginLocalAccountRequest<'a> { + /// Normalized sign-in email supplied by the caller. + email: &'a str, + /// Secret account password exposed only to the serializer. + password: &'a str, + /// Native session presentation. + client_kind: NativeAuthClient, +} + +/// Invitation registration request serialized only at the HTTP boundary. +#[derive(Serialize)] +struct RegisterLocalAccountRequest<'a> { + /// Secret one-time invitation token exposed only to the serializer. + invite_token: &'a str, + /// Invitation-bound email address. + email: &'a str, + /// Optional account display name. + display_name: Option<&'a str>, + /// Secret account password exposed only to the serializer. + password: &'a str, + /// Native session presentation. + client_kind: NativeAuthClient, +} + +/// Wire response containing a bearer token that is wiped after conversion. +#[derive(Deserialize)] +#[serde(deny_unknown_fields)] +struct LocalAuthResponse { + /// Durable authenticated account. + account: AccountRecord, + /// Opaque token required for native clients. + token: Option, + /// Non-extendable session expiry. + expires_at: DateTime, +} + +/// Wipe the raw response token when its temporary wire value leaves scope. +impl Drop for LocalAuthResponse { + /// Zero the optional raw token string. + fn drop(&mut self) { + use zeroize::Zeroize as _; + if let Some(token) = &mut self.token { + token.zeroize(); + } + } +} + +/// Stable local logout acknowledgement. +#[derive(Deserialize)] +#[serde(deny_unknown_fields)] +struct LocalLogoutResponse { + /// Whether the registry accepted the revocation. + logged_out: bool, } /// Authenticated account profile and its publisher memberships. @@ -68,6 +161,154 @@ pub fn get_auth_config(server_url: &str) -> Result, + password: &SecretString, + client_kind: NativeAuthClient, +) -> Result { + let request = RegisterLocalAccountRequest { + invite_token: invite_token.expose_secret(), + email, + display_name, + password: password.expose_secret(), + client_kind, + }; + post_local_auth(server_url, "register", request) +} + +/// Verify first-party credentials and create a native bearer session. +/// +/// # Errors +/// +/// Returns a registry URL, transport, status, size, JSON, token-validation, or +/// expiry error without including the password or bearer token. +pub fn login_local_account( + server_url: &str, + email: &str, + password: &SecretString, + client_kind: NativeAuthClient, +) -> Result { + let request = LoginLocalAccountRequest { + email, + password: password.expose_secret(), + client_kind, + }; + post_local_auth(server_url, "login", request) +} + +/// Revoke one first-party bearer session at the registry. +/// +/// # Errors +/// +/// Returns a registry URL, transport, status, size, or JSON error without ever +/// including the bearer token in the diagnostic. +pub fn logout_local_account( + server_url: &str, + access_token: &SecretString, +) -> Result<(), ClientError> { + let url = crate::publisher::registry_endpoint_url(server_url, &["v1", "auth", "logout"])?; + let request = ureq::AgentBuilder::new() + .redirects(0) + .timeout(std::time::Duration::from_secs(15)) + .build() + .post(url.as_str()); + match crate::publisher::with_bearer(request, access_token).call() { + Ok(response) => { + let acknowledgement: LocalLogoutResponse = + crate::registry::response_json_bounded(response, url.as_str())?; + if acknowledgement.logged_out { + Ok(()) + } else { + Err(ClientError::RegistryRejected { + url: url.to_string(), + status: 502, + message: "registry did not acknowledge local logout".to_string(), + }) + } + } + Err(ureq::Error::Status(status, response)) => Err(ClientError::RegistryRejected { + url: url.to_string(), + status, + message: crate::registry::response_text_bounded(response, url.as_str()), + }), + Err(error) => Err(ClientError::RegistryHttp { + url: url.to_string(), + detail: error.to_string(), + }), + } +} + +/// Submit one first-party credential request and validate its native session. +fn post_local_auth( + server_url: &str, + operation: &str, + request: impl Serialize, +) -> Result { + let url = crate::publisher::registry_endpoint_url(server_url, &["v1", "auth", operation])?; + let body = Zeroizing::new( + serde_json::to_vec(&request) + .map_err(|error| ClientError::JsonSerialize(error.to_string()))?, + ); + let response = ureq::AgentBuilder::new() + .redirects(0) + .timeout(std::time::Duration::from_secs(15)) + .build() + .post(url.as_str()) + .set("Content-Type", "application/json") + .send_bytes(body.as_slice()); + let mut response: LocalAuthResponse = match response { + Ok(response) => crate::registry::response_json_bounded(response, url.as_str())?, + Err(ureq::Error::Status(status, response)) => { + return Err(ClientError::RegistryRejected { + url: url.to_string(), + status, + message: crate::registry::response_text_bounded(response, url.as_str()), + }); + } + Err(error) => { + return Err(ClientError::RegistryHttp { + url: url.to_string(), + detail: error.to_string(), + }); + } + }; + let token = response + .token + .take() + .ok_or_else(|| ClientError::RegistryRejected { + url: url.to_string(), + status: 502, + message: "registry omitted the native bearer token".to_string(), + })?; + let expires_at = u64::try_from(response.expires_at.timestamp()).map_err(|_| { + ClientError::RegistryRejected { + url: url.to_string(), + status: 502, + message: "registry returned an invalid native session expiry".to_string(), + } + })?; + let session = + AuthenticatedSession::from_first_party_bearer(SecretString::new(token), expires_at) + .map_err(|_| ClientError::RegistryRejected { + url: url.to_string(), + status: 502, + message: "registry returned an invalid native bearer session".to_string(), + })?; + Ok(LocalAccountSession { + account: response.account.clone(), + session, + }) +} + /// Fetch the current account view from one FrameShift registry. /// /// # Errors @@ -104,18 +345,54 @@ mod tests { use std::io::{Read as _, Write as _}; use std::net::TcpListener; use std::thread; + use std::time::Duration; use super::*; - /// Serve one fixed account response and return the captured request. - fn serve_account_response() -> (String, thread::JoinHandle) { + /// Read one complete bounded HTTP request including its declared body. + fn read_request(stream: &mut std::net::TcpStream) -> String { + stream + .set_read_timeout(Some(Duration::from_secs(2))) + .expect("set read timeout"); + let mut request = Vec::new(); + let mut chunk = [0_u8; 1024]; + loop { + let count = stream.read(&mut chunk).expect("read request"); + if count == 0 { + break; + } + request.extend_from_slice(&chunk[..count]); + let Some(headers_end) = request.windows(4).position(|bytes| bytes == b"\r\n\r\n") + else { + continue; + }; + let headers_end = headers_end + 4; + let headers = String::from_utf8_lossy(&request[..headers_end]); + let content_length = headers + .lines() + .find_map(|line| { + line.split_once(':').and_then(|(name, value)| { + name.eq_ignore_ascii_case("content-length") + .then(|| value.trim().parse::().ok()) + .flatten() + }) + }) + .unwrap_or(0); + if request.len() >= headers_end + content_length { + break; + } + } + String::from_utf8(request).expect("request UTF-8") + } + + /// Serve one fixed JSON response and return the captured request. + fn serve_json_response(body: impl Into) -> (String, thread::JoinHandle) { let listener = TcpListener::bind("127.0.0.1:0").expect("bind test server"); let address = listener.local_addr().expect("test server address"); + let body = body.into(); let handle = thread::spawn(move || { let (mut stream, _) = listener.accept().expect("accept request"); - let mut request = vec![0_u8; 4096]; - let count = stream.read(&mut request).expect("read request"); - let body = r#"{"account":{"id":"00000000-0000-0000-0000-000000000001","issuer":"https://issuer.example","subject":"subject-1","email":null,"display_name":"Alice","status":"active","created_at":"2026-01-01T00:00:00Z","updated_at":"2026-01-01T00:00:00Z"},"memberships":[]}"#; + let request = read_request(&mut stream); let response = format!( "HTTP/1.1 200 OK\r\nContent-Type: application/json\r\nContent-Length: {}\r\nConnection: close\r\n\r\n{body}", body.len() @@ -123,15 +400,21 @@ mod tests { stream .write_all(response.as_bytes()) .expect("write response"); - String::from_utf8_lossy(&request[..count]).into_owned() + request }); (format!("http://{address}"), handle) } + /// Return one stable account JSON object for native-auth responses. + fn account_json() -> &'static str { + r#"{"id":"00000000-0000-0000-0000-000000000001","issuer":"https://issuer.example","subject":"subject-1","email":"alice@example.com","display_name":"Alice","status":"active","created_at":"2026-01-01T00:00:00Z","updated_at":"2026-01-01T00:00:00Z"}"# + } + /// Account lookup sends the bearer only in the authorization header. #[test] fn fetches_account_with_bearer_header() { - let (server, handle) = serve_account_response(); + let body = format!(r#"{{"account":{},"memberships":[]}}"#, account_json()); + let (server, handle) = serve_json_response(body); let token = SecretString::new("test-access-token".to_string()); let view = get_account(&server, &token).expect("account response"); assert_eq!(view.account.display_name.as_deref(), Some("Alice")); @@ -141,4 +424,78 @@ mod tests { assert!(request.contains("\r\nAuthorization: Bearer test-access-token\r\n")); assert_eq!(request.matches("test-access-token").count(), 1); } + + /// First-party login sends credentials only in JSON and redacts its bearer result. + #[test] + fn logs_in_with_native_bearer_without_debug_disclosure() { + let body = format!( + r#"{{"account":{},"token":"native-session-token","expires_at":"2099-01-01T00:00:00Z"}}"#, + account_json() + ); + let (server, handle) = serve_json_response(body); + let password = SecretString::new("correct horse battery staple".to_string()); + let authenticated = login_local_account( + &server, + "alice@example.com", + &password, + NativeAuthClient::Cli, + ) + .expect("native login"); + + assert_eq!( + authenticated.account.email.as_deref(), + Some("alice@example.com") + ); + assert_eq!( + authenticated.session.access_token().expose_secret(), + "native-session-token" + ); + let debug = format!("{authenticated:?}"); + assert!(!debug.contains("native-session-token")); + assert!(!debug.contains("correct horse battery staple")); + let request = handle.join().expect("test server thread"); + assert!(request.starts_with("POST /v1/auth/login HTTP/1.1\r\n")); + assert!(request.contains("\"client_kind\":\"cli\"")); + assert!(request.contains("\"password\":\"correct horse battery staple\"")); + assert!(!request.contains("Authorization:")); + } + + /// Invitation registration preserves native client kind and optional profile data. + #[test] + fn registers_invited_native_account() { + let body = format!( + r#"{{"account":{},"token":"registered-session-token","expires_at":"2099-01-01T00:00:00Z"}}"#, + account_json() + ); + let (server, handle) = serve_json_response(body); + let invite = SecretString::new("one-time-invite".to_string()); + let password = SecretString::new("a sufficiently long password".to_string()); + let authenticated = register_local_account( + &server, + &invite, + "alice@example.com", + Some("Alice"), + &password, + NativeAuthClient::Desktop, + ) + .expect("native registration"); + + assert_eq!(authenticated.account.display_name.as_deref(), Some("Alice")); + let request = handle.join().expect("test server thread"); + assert!(request.starts_with("POST /v1/auth/register HTTP/1.1\r\n")); + assert!(request.contains("\"invite_token\":\"one-time-invite\"")); + assert!(request.contains("\"client_kind\":\"desktop\"")); + } + + /// Local logout uses the bearer header and requires an affirmative acknowledgement. + #[test] + fn logs_out_local_session_with_bearer_header() { + let (server, handle) = serve_json_response(r#"{"logged_out":true}"#); + let token = SecretString::new("native-session-token".to_string()); + logout_local_account(&server, &token).expect("local logout"); + let request = handle.join().expect("test server thread"); + assert!(request.starts_with("POST /v1/auth/logout HTTP/1.1\r\n")); + assert!(request.contains("\r\nAuthorization: Bearer native-session-token\r\n")); + assert_eq!(request.matches("native-session-token").count(), 1); + } } diff --git a/crates/frameshift-client/src/session.rs b/crates/frameshift-client/src/session.rs index 16607ba..4e299d7 100644 --- a/crates/frameshift-client/src/session.rs +++ b/crates/frameshift-client/src/session.rs @@ -1,4 +1,4 @@ -//! Provider-neutral OIDC Authorization Code session client. +//! OIDC Authorization Code client and provider-neutral bearer session. //! //! The module owns discovery validation, S256 PKCE construction, exact callback //! validation, bounded token exchange, refresh, and revocation. Callers remain @@ -96,8 +96,12 @@ impl fmt::Debug for AuthorizationFlow { } } -/// Authenticated OAuth session returned by the configured OIDC issuer. -pub struct OidcSession { +/// Secret-bearing authenticated session used for registry API calls. +/// +/// OIDC and first-party authentication both produce the same bearer-session +/// shape. Provider-specific refresh behavior remains owned by [`SessionClient`] +/// and the persisted authentication metadata. +pub struct AuthenticatedSession { /// Bearer access token used only through explicit secret accessors. pub(crate) access_token: SecretString, /// Optional refresh token. @@ -111,11 +115,11 @@ pub struct OidcSession { } /// Redacted session diagnostics. -impl fmt::Debug for OidcSession { +impl fmt::Debug for AuthenticatedSession { /// Render metadata while hiding all token values. fn fmt(&self, formatter: &mut fmt::Formatter<'_>) -> fmt::Result { formatter - .debug_struct("OidcSession") + .debug_struct("AuthenticatedSession") .field("access_token", &"[REDACTED]") .field( "refresh_token", @@ -128,6 +132,9 @@ impl fmt::Debug for OidcSession { } } +/// Backward-compatible name for sessions returned by [`SessionClient`]. +pub type OidcSession = AuthenticatedSession; + /// Public session metadata safe to print in account-status output. #[derive(Debug, Clone, PartialEq, Eq)] pub struct SessionSummary { @@ -504,7 +511,7 @@ impl SessionClient { } /// Secret and metadata accessors for an authenticated session. -impl OidcSession { +impl AuthenticatedSession { /// Borrow the bearer access token for an authenticated API call. pub fn access_token(&self) -> &SecretString { &self.access_token @@ -528,6 +535,45 @@ impl OidcSession { } } + /// Build a non-refreshable first-party bearer session with an exact expiry. + /// + /// # Errors + /// + /// Returns an invalid-configuration error when the token is empty, contains + /// HTTP-header whitespace or controls, or the supplied expiry is not later + /// than the current Unix timestamp. + pub fn from_first_party_bearer( + access_token: SecretString, + expires_at: u64, + ) -> Result { + let token = access_token.expose_secret(); + if token.is_empty() + || token + .chars() + .any(|character| character.is_whitespace() || character.is_control()) + { + return Err(SessionError::InvalidConfiguration( + "first-party bearer token failed validation".to_string(), + )); + } + let acquired_at = unix_now(); + let expires_in = expires_at + .checked_sub(acquired_at) + .filter(|value| *value > 0); + let expires_in = expires_in.ok_or_else(|| { + SessionError::InvalidConfiguration( + "first-party bearer session is already expired".to_string(), + ) + })?; + Ok(Self { + access_token, + refresh_token: None, + expires_in: Some(expires_in), + scope: None, + acquired_at, + }) + } + /// Reconstruct a session from a validated native credential-store payload. pub(crate) fn from_stored_parts( access_token: SecretString, @@ -862,6 +908,44 @@ mod tests { SessionClient::discover_with_http(config(), fake) } + /// First-party sessions validate the bearer and expose only non-secret lifetime metadata. + #[test] + fn builds_non_refreshable_first_party_session() { + let expires_at = unix_now().saturating_add(120); + let session = AuthenticatedSession::from_first_party_bearer( + SecretString::new("native-session-token".to_string()), + expires_at, + ) + .expect("first-party session"); + + assert_eq!( + session.access_token().expose_secret(), + "native-session-token" + ); + assert!(session.refresh_token().is_none()); + assert!(!session.summary().refreshable); + assert_eq!(session.summary().expires_at, Some(expires_at)); + assert!(!format!("{session:?}").contains("native-session-token")); + } + + /// Empty, header-unsafe, and expired first-party bearer values fail closed. + #[test] + fn rejects_invalid_first_party_session_material() { + let future = unix_now().saturating_add(120); + for token in ["", "contains space", "contains\0control"] { + assert!(AuthenticatedSession::from_first_party_bearer( + SecretString::new(token.to_string()), + future, + ) + .is_err()); + } + assert!(AuthenticatedSession::from_first_party_bearer( + SecretString::new("native-session-token".to_string()), + unix_now(), + ) + .is_err()); + } + /// Discovery and authorization construction preserve required protocol fields. #[test] fn discovers_provider_and_builds_s256_authorization() { diff --git a/crates/frameshift-client/src/session_store.rs b/crates/frameshift-client/src/session_store.rs index 1ab459c..2f5c7a1 100644 --- a/crates/frameshift-client/src/session_store.rs +++ b/crates/frameshift-client/src/session_store.rs @@ -14,10 +14,12 @@ use sha2::{Digest as _, Sha256}; use url::Url; use zeroize::{Zeroize as _, Zeroizing}; -use crate::session::OidcSession; +use crate::session::AuthenticatedSession; /// Current non-secret session metadata schema version. -const SESSION_METADATA_SCHEMA_VERSION: u32 = 1; +const SESSION_METADATA_SCHEMA_VERSION: u32 = 2; +/// Legacy OIDC-only metadata schema version accepted during migration. +const LEGACY_SESSION_METADATA_SCHEMA_VERSION: u32 = 1; /// Current secret payload schema version. const SESSION_SECRET_SCHEMA_VERSION: u32 = 1; /// Native credential-store service namespace. @@ -35,14 +37,8 @@ const MAX_SESSION_STORAGE_BYTES: u64 = 1024 * 1024; pub struct StoredSessionMetadata { /// On-disk schema version. pub schema_version: u32, - /// Exact OIDC issuer. - pub issuer: Url, - /// Public OAuth client identifier. - pub client_id: String, - /// Registered callback URI. - pub redirect_uri: Url, - /// Requested OIDC scopes. - pub scopes: Vec, + /// Provider-specific public authentication metadata. + pub authentication: SessionAuthentication, /// FrameShift registry API base URL. pub registry_url: Url, /// Derived native credential-store account. @@ -56,7 +52,7 @@ pub struct StoredSession { /// Non-secret session metadata. pub metadata: StoredSessionMetadata, /// Secret-bearing authenticated session. - pub session: OidcSession, + pub session: AuthenticatedSession, } /// Redacted stored-session diagnostics. @@ -74,16 +70,61 @@ impl std::fmt::Debug for StoredSession { /// Inputs used to persist one newly authenticated session. #[derive(Debug, Clone, PartialEq, Eq)] pub struct SessionStoreMetadata { + /// Provider-specific public authentication metadata. + pub authentication: SessionAuthentication, + /// FrameShift registry API base URL. + pub registry_url: Url, +} + +/// Public metadata needed to manage one authenticated session. +#[derive(Debug, Clone, PartialEq, Eq, Serialize, Deserialize)] +#[serde(tag = "kind", rename_all = "snake_case", deny_unknown_fields)] +pub enum SessionAuthentication { + /// OIDC discovery, refresh, and revocation configuration. + Oidc { + /// Exact OIDC issuer. + issuer: Url, + /// Public OAuth client identifier. + client_id: String, + /// Registered callback URI. + redirect_uri: Url, + /// Requested OIDC scopes. + scopes: Vec, + }, + /// First-party opaque bearer session managed by the registry. + FirstParty, +} + +/// Schema-v1 OIDC metadata accepted without changing its keyring binding. +#[derive(Deserialize)] +#[serde(deny_unknown_fields)] +struct LegacyStoredSessionMetadata { + /// Legacy on-disk schema version. + schema_version: u32, /// Exact OIDC issuer. - pub issuer: Url, + issuer: Url, /// Public OAuth client identifier. - pub client_id: String, + client_id: String, /// Registered callback URI. - pub redirect_uri: Url, + redirect_uri: Url, /// Requested OIDC scopes. - pub scopes: Vec, + scopes: Vec, /// FrameShift registry API base URL. - pub registry_url: Url, + registry_url: Url, + /// Derived native credential-store account. + credential_id: String, + /// Unix timestamp when this session was saved. + saved_at: u64, +} + +/// Current or legacy metadata wire representation. +#[derive(Deserialize)] +#[serde(untagged)] +enum StoredSessionMetadataWire { + /// Current provider-tagged metadata. + Current(StoredSessionMetadata), + /// Legacy OIDC-only metadata. + Legacy(LegacyStoredSessionMetadata), } /// Local session persistence manager. @@ -218,7 +259,7 @@ impl SessionStore { pub fn save( &self, metadata: SessionStoreMetadata, - session: &OidcSession, + session: &AuthenticatedSession, ) -> Result { self.save_with_store(metadata, session, &SystemSessionCredentialStore) } @@ -237,10 +278,11 @@ impl SessionStore { fn save_with_store( &self, metadata: SessionStoreMetadata, - session: &OidcSession, + session: &AuthenticatedSession, credentials: &dyn SessionCredentialStore, ) -> Result { validate_store_metadata(&metadata)?; + validate_session_authentication(&metadata.authentication, session)?; let _lock = self.acquire_lock()?; let prior_metadata = self.load_metadata_optional()?; let credential_id = credential_id(&metadata); @@ -254,10 +296,7 @@ impl SessionStore { let stored = StoredSessionMetadata { schema_version: SESSION_METADATA_SCHEMA_VERSION, - issuer: metadata.issuer, - client_id: metadata.client_id, - redirect_uri: metadata.redirect_uri, - scopes: metadata.scopes, + authentication: metadata.authentication, registry_url: metadata.registry_url, credential_id: credential_id.clone(), saved_at: unix_now(), @@ -297,6 +336,7 @@ impl SessionStore { ) })?; let session = deserialize_session(&payload)?; + validate_session_authentication(&metadata.authentication, &session)?; Ok(StoredSession { metadata, session }) } @@ -355,8 +395,12 @@ impl SessionStore { "metadata exceeded the size limit".to_string(), )); } - let metadata: StoredSessionMetadata = serde_json::from_slice(&bytes) + let metadata: StoredSessionMetadataWire = serde_json::from_slice(&bytes) .map_err(|_| SessionStoreError::Invalid("metadata JSON is malformed".to_string()))?; + let metadata = match metadata { + StoredSessionMetadataWire::Current(metadata) => metadata, + StoredSessionMetadataWire::Legacy(metadata) => migrate_legacy_metadata(metadata)?, + }; validate_stored_metadata(&metadata)?; Ok(Some(metadata)) } @@ -408,45 +452,66 @@ impl SessionStore { /// Validate caller-supplied non-secret metadata. fn validate_store_metadata(metadata: &SessionStoreMetadata) -> Result<(), SessionStoreError> { - if metadata.client_id.trim().is_empty() - || metadata.scopes.is_empty() - || !metadata.scopes.iter().any(|scope| scope == "openid") - || metadata - .scopes - .iter() - .any(|scope| scope.trim().is_empty() || scope.chars().any(char::is_whitespace)) + validate_https_url(&metadata.registry_url, "registry URL")?; + if let SessionAuthentication::Oidc { + issuer, + client_id, + redirect_uri, + scopes, + } = &metadata.authentication { - return Err(SessionStoreError::Invalid( - "client ID and valid openid scope are required".to_string(), - )); + if client_id.trim().is_empty() + || scopes.is_empty() + || !scopes.iter().any(|scope| scope == "openid") + || scopes + .iter() + .any(|scope| scope.trim().is_empty() || scope.chars().any(char::is_whitespace)) + { + return Err(SessionStoreError::Invalid( + "client ID and valid openid scope are required".to_string(), + )); + } + let mut unique_scopes = std::collections::BTreeSet::new(); + if !scopes.iter().all(|scope| unique_scopes.insert(scope)) { + return Err(SessionStoreError::Invalid( + "session scopes must not contain duplicates".to_string(), + )); + } + validate_https_url(issuer, "issuer")?; + validate_redirect_uri(redirect_uri)?; } - let mut unique_scopes = std::collections::BTreeSet::new(); - if !metadata - .scopes - .iter() - .all(|scope| unique_scopes.insert(scope)) + Ok(()) +} + +/// Require secret-session fields that match the persisted provider kind. +fn validate_session_authentication( + authentication: &SessionAuthentication, + session: &AuthenticatedSession, +) -> Result<(), SessionStoreError> { + let summary = session.summary(); + if matches!(authentication, SessionAuthentication::FirstParty) + && (summary.refreshable || summary.scope.is_some() || summary.expires_in.is_none()) { return Err(SessionStoreError::Invalid( - "session scopes must not contain duplicates".to_string(), + "first-party sessions must have an expiry and no OIDC refresh metadata".to_string(), )); } - for (url, label) in [ - (&metadata.issuer, "issuer"), - (&metadata.registry_url, "registry URL"), - ] { - if url.scheme() != "https" - || url.host_str().is_none() - || !url.username().is_empty() - || url.password().is_some() - || url.query().is_some() - || url.fragment().is_some() - { - return Err(SessionStoreError::Invalid(format!( - "{label} must be a credential-free HTTPS URL" - ))); - } + Ok(()) +} + +/// Require one credential-free HTTPS URL without query or fragment state. +fn validate_https_url(url: &Url, label: &str) -> Result<(), SessionStoreError> { + if url.scheme() != "https" + || url.host_str().is_none() + || !url.username().is_empty() + || url.password().is_some() + || url.query().is_some() + || url.fragment().is_some() + { + return Err(SessionStoreError::Invalid(format!( + "{label} must be a credential-free HTTPS URL" + ))); } - validate_redirect_uri(&metadata.redirect_uri)?; Ok(()) } @@ -480,17 +545,11 @@ fn validate_stored_metadata(metadata: &StoredSessionMetadata) -> Result<(), Sess ))); } validate_store_metadata(&SessionStoreMetadata { - issuer: metadata.issuer.clone(), - client_id: metadata.client_id.clone(), - redirect_uri: metadata.redirect_uri.clone(), - scopes: metadata.scopes.clone(), + authentication: metadata.authentication.clone(), registry_url: metadata.registry_url.clone(), })?; let expected = credential_id(&SessionStoreMetadata { - issuer: metadata.issuer.clone(), - client_id: metadata.client_id.clone(), - redirect_uri: metadata.redirect_uri.clone(), - scopes: metadata.scopes.clone(), + authentication: metadata.authentication.clone(), registry_url: metadata.registry_url.clone(), }); if metadata.credential_id != expected { @@ -501,15 +560,50 @@ fn validate_stored_metadata(metadata: &StoredSessionMetadata) -> Result<(), Sess Ok(()) } +/// Convert schema-v1 OIDC metadata into the provider-tagged in-memory shape. +fn migrate_legacy_metadata( + metadata: LegacyStoredSessionMetadata, +) -> Result { + if metadata.schema_version != LEGACY_SESSION_METADATA_SCHEMA_VERSION { + return Err(SessionStoreError::Invalid(format!( + "unsupported legacy metadata schema version {}", + metadata.schema_version + ))); + } + Ok(StoredSessionMetadata { + schema_version: SESSION_METADATA_SCHEMA_VERSION, + authentication: SessionAuthentication::Oidc { + issuer: metadata.issuer, + client_id: metadata.client_id, + redirect_uri: metadata.redirect_uri, + scopes: metadata.scopes, + }, + registry_url: metadata.registry_url, + credential_id: metadata.credential_id, + saved_at: metadata.saved_at, + }) +} + /// Derive a stable credential account without embedding user identifiers. fn credential_id(metadata: &SessionStoreMetadata) -> String { let mut hasher = Sha256::new(); - for value in [ - metadata.issuer.as_str(), - &metadata.client_id, - metadata.redirect_uri.as_str(), - metadata.registry_url.as_str(), - ] { + let values = match &metadata.authentication { + SessionAuthentication::Oidc { + issuer, + client_id, + redirect_uri, + .. + } => vec![ + issuer.as_str(), + client_id.as_str(), + redirect_uri.as_str(), + metadata.registry_url.as_str(), + ], + SessionAuthentication::FirstParty => { + vec!["first_party", metadata.registry_url.as_str()] + } + }; + for value in values { hasher.update((value.len() as u64).to_be_bytes()); hasher.update(value.as_bytes()); } @@ -517,7 +611,9 @@ fn credential_id(metadata: &SessionStoreMetadata) -> String { } /// Serialize only secret-bearing session fields for native storage. -fn serialize_session(session: &OidcSession) -> Result>, SessionStoreError> { +fn serialize_session( + session: &AuthenticatedSession, +) -> Result>, SessionStoreError> { let payload = SessionSecretPayload { schema_version: SESSION_SECRET_SCHEMA_VERSION, access_token: session.access_token.expose_secret().to_owned(), @@ -535,7 +631,7 @@ fn serialize_session(session: &OidcSession) -> Result>, Sessio } /// Decode and validate a native credential payload. -fn deserialize_session(payload: &[u8]) -> Result { +fn deserialize_session(payload: &[u8]) -> Result { if payload.len() as u64 > MAX_SESSION_STORAGE_BYTES { return Err(SessionStoreError::Invalid( "credential payload exceeded the size limit".to_string(), @@ -543,14 +639,20 @@ fn deserialize_session(payload: &[u8]) -> Result } let payload: SessionSecretPayload = serde_json::from_slice(payload) .map_err(|_| SessionStoreError::Invalid("credential payload is malformed".to_string()))?; + let invalid_access_token = invalid_token_value(&payload.access_token); + let invalid_refresh_token = payload + .refresh_token + .as_deref() + .is_some_and(invalid_token_value); if payload.schema_version != SESSION_SECRET_SCHEMA_VERSION - || payload.access_token.trim().is_empty() + || invalid_access_token + || invalid_refresh_token { return Err(SessionStoreError::Invalid( "credential payload failed validation".to_string(), )); } - Ok(OidcSession::from_stored_parts( + Ok(AuthenticatedSession::from_stored_parts( SecretString::new(payload.access_token.clone()), payload.refresh_token.clone().map(SecretString::new), payload.expires_in, @@ -559,6 +661,14 @@ fn deserialize_session(payload: &[u8]) -> Result )) } +/// Reject token values that cannot be placed safely in an authorization header. +fn invalid_token_value(token: &str) -> bool { + token.is_empty() + || token + .chars() + .any(|character| character.is_whitespace() || character.is_control()) +} + /// Restore or delete a credential after metadata persistence fails. fn rollback_credential( credentials: &dyn SessionCredentialStore, @@ -714,17 +824,19 @@ mod tests { /// Build non-secret metadata. fn metadata() -> SessionStoreMetadata { SessionStoreMetadata { - issuer: Url::parse("https://issuer.example").expect("issuer URL"), - client_id: "frameshift-cli".to_string(), - redirect_uri: Url::parse("http://127.0.0.1:8765/callback").expect("redirect URL"), - scopes: vec!["openid".to_string(), "profile".to_string()], + authentication: SessionAuthentication::Oidc { + issuer: Url::parse("https://issuer.example").expect("issuer URL"), + client_id: "frameshift-cli".to_string(), + redirect_uri: Url::parse("http://127.0.0.1:8765/callback").expect("redirect URL"), + scopes: vec!["openid".to_string(), "profile".to_string()], + }, registry_url: Url::parse("https://registry.example").expect("registry URL"), } } /// Build one secret-bearing session. - fn session() -> OidcSession { - OidcSession::from_stored_parts( + fn session() -> AuthenticatedSession { + AuthenticatedSession::from_stored_parts( SecretString::new("secret-access".to_string()), Some(SecretString::new("secret-refresh".to_string())), Some(300), @@ -797,21 +909,30 @@ mod tests { let mut cases = Vec::new(); let mut issuer_query = metadata(); - issuer_query.issuer = - Url::parse("https://issuer.example?tenant=other").expect("issuer URL"); + if let SessionAuthentication::Oidc { issuer, .. } = &mut issuer_query.authentication { + *issuer = Url::parse("https://issuer.example?tenant=other").expect("issuer URL"); + } cases.push(issuer_query); let mut remote_http_redirect = metadata(); - remote_http_redirect.redirect_uri = - Url::parse("http://192.0.2.1:8765/callback").expect("redirect URL"); + if let SessionAuthentication::Oidc { redirect_uri, .. } = + &mut remote_http_redirect.authentication + { + *redirect_uri = Url::parse("http://192.0.2.1:8765/callback").expect("redirect URL"); + } cases.push(remote_http_redirect); let mut duplicate_scopes = metadata(); - duplicate_scopes.scopes = vec!["openid".to_string(), "openid".to_string()]; + if let SessionAuthentication::Oidc { scopes, .. } = &mut duplicate_scopes.authentication { + *scopes = vec!["openid".to_string(), "openid".to_string()]; + } cases.push(duplicate_scopes); let mut embedded_whitespace = metadata(); - embedded_whitespace.scopes = vec!["openid".to_string(), "bad scope".to_string()]; + if let SessionAuthentication::Oidc { scopes, .. } = &mut embedded_whitespace.authentication + { + *scopes = vec!["openid".to_string(), "bad scope".to_string()]; + } cases.push(embedded_whitespace); for invalid in cases { @@ -821,4 +942,114 @@ mod tests { } assert!(!store.metadata_path().exists()); } + + /// First-party sessions use explicit metadata and remain non-refreshable. + #[test] + fn saves_and_loads_first_party_session() { + let temp = tempfile::tempdir().expect("tempdir"); + let store = SessionStore::new(temp.path()); + let credentials = FakeCredentials::default(); + let metadata = SessionStoreMetadata { + authentication: SessionAuthentication::FirstParty, + registry_url: Url::parse("https://registry.example").expect("registry URL"), + }; + let session = AuthenticatedSession::from_stored_parts( + SecretString::new("local-access".to_string()), + None, + Some(600), + None, + 42, + ); + + store + .save_with_store(metadata, &session, &credentials) + .expect("save first-party session"); + let metadata_text = + fs::read_to_string(store.metadata_path()).expect("read first-party metadata"); + assert!(metadata_text.contains("\"kind\": \"first_party\"")); + assert!(!metadata_text.contains("local-access")); + let loaded = store.load_with_store(&credentials).expect("load session"); + assert_eq!( + loaded.metadata.authentication, + SessionAuthentication::FirstParty + ); + assert!(loaded.session.refresh_token().is_none()); + assert_eq!( + loaded.session.access_token().expose_secret(), + "local-access" + ); + } + + /// Provider metadata cannot relabel an OIDC refreshable session as first-party. + #[test] + fn rejects_first_party_metadata_for_oidc_session_shape() { + let temp = tempfile::tempdir().expect("tempdir"); + let store = SessionStore::new(temp.path()); + let credentials = FakeCredentials::default(); + let metadata = SessionStoreMetadata { + authentication: SessionAuthentication::FirstParty, + registry_url: Url::parse("https://registry.example").expect("registry URL"), + }; + + assert!(store + .save_with_store(metadata, &session(), &credentials) + .is_err()); + assert!(!store.metadata_path().exists()); + } + + /// Schema-v1 OIDC metadata keeps its exact existing keyring credential binding. + #[test] + fn migrates_legacy_oidc_metadata_without_changing_credential_id() { + let temp = tempfile::tempdir().expect("tempdir"); + let store = SessionStore::new(temp.path()); + let credentials = FakeCredentials::default(); + let current = metadata(); + let expected_credential_id = credential_id(¤t); + let SessionAuthentication::Oidc { + issuer, + client_id, + redirect_uri, + scopes, + } = ¤t.authentication + else { + panic!("test metadata must be OIDC"); + }; + let legacy = serde_json::json!({ + "schema_version": LEGACY_SESSION_METADATA_SCHEMA_VERSION, + "issuer": issuer, + "client_id": client_id, + "redirect_uri": redirect_uri, + "scopes": scopes, + "registry_url": current.registry_url, + "credential_id": expected_credential_id.clone(), + "saved_at": 41 + }); + let path = store.metadata_path(); + ensure_private_directory(path.parent().expect("metadata parent")) + .expect("create metadata parent"); + write_private_file( + &path, + &serde_json::to_vec(&legacy).expect("serialize legacy metadata"), + ) + .expect("write legacy metadata"); + credentials + .put( + &expected_credential_id, + &serialize_session(&session()).expect("secret payload"), + ) + .expect("write legacy credential"); + + let loaded = store + .load_with_store(&credentials) + .expect("load legacy session"); + assert_eq!( + loaded.metadata.schema_version, + SESSION_METADATA_SCHEMA_VERSION + ); + assert_eq!(loaded.metadata.credential_id, expected_credential_id); + assert!(matches!( + loaded.metadata.authentication, + SessionAuthentication::Oidc { .. } + )); + } } diff --git a/docs/wiki/Accounts-and-Publisher-Identity.md b/docs/wiki/Accounts-and-Publisher-Identity.md index 1ccd927..fd64e91 100644 --- a/docs/wiki/Accounts-and-Publisher-Identity.md +++ b/docs/wiki/Accounts-and-Publisher-Identity.md @@ -38,23 +38,27 @@ invite or session token from the database. ## Sign in securely ```bash +frameshift account register frameshift account login +frameshift account login --first-party frameshift account status ``` -The current `frameshift account login` command uses the configured OIDC -provider. It opens the provider in your browser and uses an exact IP-loopback -callback with S256 PKCE. The desktop first-party login API exists for clients -that implement the explicit bearer-session flow; the CLI command has not -switched to that flow. +`frameshift account register` redeems a single-use invitation through hidden +terminal prompts. `frameshift account login` uses the registry's advertised +provider, preferring OIDC when both OIDC and first-party login are available. +Use `--first-party` to select password login explicitly. OIDC opens the provider +in your system browser and uses an exact IP-loopback callback with S256 PKCE. -The CLI stores OIDC access and refresh tokens in the operating system's native -credential store. The JSON metadata in the managed FrameShift data directory -contains no token. +The CLI accepts no password or invitation through arguments or environment. +First-party credentials require an interactive terminal, and secret values use +hidden prompts. OIDC and first-party bearer credentials remain in the operating +system's native credential store. Provider-tagged JSON metadata in the managed +FrameShift data directory contains no token. `account status` asks the registry for the authenticated account and its -publisher memberships. `account logout` attempts provider revocation and then -removes the exact local credential and metadata. +publisher memberships. `account logout` requests the matching provider's +revocation endpoint and then removes the exact local credential and metadata. A saved session is bound to the registry used during login. Remote key commands reuse and refresh that session only when their `--server` value names the same diff --git a/docs/wiki/CLI-Reference.md b/docs/wiki/CLI-Reference.md index edf71e3..46c953f 100644 --- a/docs/wiki/CLI-Reference.md +++ b/docs/wiki/CLI-Reference.md @@ -19,7 +19,7 @@ The binary is named `frameshift`. | Command | Purpose | |---|---| -| `account` | Log in, inspect, or end an account session | +| `account` | Register, log in, inspect, or end an account session | | `install` | Install a persona pack into the central store | | `activate` | Activate an installed persona for the current project | | `uninstall` | Remove an installed persona from the current project | @@ -50,7 +50,10 @@ The binary is named `frameshift`. ### `frameshift account ` -Use `login`, `status`, or `logout` to manage the current account session. +Use `register`, `login`, `status`, or `logout` to manage the current account +session. Registration and first-party login collect secrets only through hidden +interactive prompts. `login --first-party` selects password login when the +registry also advertises OIDC. ### `frameshift install [OPTIONS] ` diff --git a/docs/wiki/Local-Data-and-Privacy.md b/docs/wiki/Local-Data-and-Privacy.md index b173479..9ee2e75 100644 --- a/docs/wiki/Local-Data-and-Privacy.md +++ b/docs/wiki/Local-Data-and-Privacy.md @@ -58,14 +58,17 @@ already received by a configured endpoint. ## Account sessions -Account login uses an OIDC authorization-code flow with S256 PKCE, state, and -nonce validation. Access and refresh tokens are stored only through the -operating system's native credential store. The adjacent JSON metadata contains -non-secret issuer, client, registry, scope, and expiry information. - -`frameshift account logout` attempts provider revocation, then removes the -exact local credential and its metadata. A provider being unreachable does not -turn the token into a local plaintext file. +Account login supports first-party credentials and OIDC. First-party passwords +and invitation tokens are accepted only through hidden interactive prompts. +OIDC uses an authorization-code flow with S256 PKCE, state, and nonce +validation. All native bearer credentials are stored only through the operating +system's credential store. The adjacent JSON contains provider-tagged public +metadata and no token, password, or invitation. + +`frameshift account logout` requests the matching OIDC or first-party +revocation operation, then removes the exact local credential and its metadata. +A provider being unreachable does not turn the token into a local plaintext +file. ## Vault privacy From db8cc0215fcf56608cdfa3b79620fe713f2b0d61 Mon Sep 17 00:00:00 2001 From: GhostFrame Date: Fri, 31 Jul 2026 20:54:33 -0400 Subject: [PATCH 2/2] fix(auth): compact stored provider metadata Store OIDC callback metadata behind stable indirection while preserving the serialized session schema and credential bindings. --- crates/frameshift-cli/src/cmd/account.rs | 4 ++-- crates/frameshift-client/src/session_store.rs | 12 +++++++----- 2 files changed, 9 insertions(+), 7 deletions(-) diff --git a/crates/frameshift-cli/src/cmd/account.rs b/crates/frameshift-cli/src/cmd/account.rs index 315db77..df3e002 100644 --- a/crates/frameshift-cli/src/cmd/account.rs +++ b/crates/frameshift-cli/src/cmd/account.rs @@ -169,7 +169,7 @@ fn run_oidc_login( authentication: SessionAuthentication::Oidc { issuer, client_id, - redirect_uri, + redirect_uri: Box::new(redirect_uri), scopes: args.scopes, }, registry_url, @@ -633,7 +633,7 @@ fn session_client_for(stored: &StoredSession) -> Result SessionClient::discover(SessionClientConfig { issuer: issuer.clone(), client_id: client_id.clone(), - redirect_uri: redirect_uri.clone(), + redirect_uri: redirect_uri.as_ref().clone(), scopes: scopes.clone(), }) .map_err(|error| CliError::Account(error.to_string())) diff --git a/crates/frameshift-client/src/session_store.rs b/crates/frameshift-client/src/session_store.rs index 2f5c7a1..237fe79 100644 --- a/crates/frameshift-client/src/session_store.rs +++ b/crates/frameshift-client/src/session_store.rs @@ -86,8 +86,8 @@ pub enum SessionAuthentication { issuer: Url, /// Public OAuth client identifier. client_id: String, - /// Registered callback URI. - redirect_uri: Url, + /// Boxed registered callback URI that keeps the provider enum compact. + redirect_uri: Box, /// Requested OIDC scopes. scopes: Vec, }, @@ -575,7 +575,7 @@ fn migrate_legacy_metadata( authentication: SessionAuthentication::Oidc { issuer: metadata.issuer, client_id: metadata.client_id, - redirect_uri: metadata.redirect_uri, + redirect_uri: Box::new(metadata.redirect_uri), scopes: metadata.scopes, }, registry_url: metadata.registry_url, @@ -827,7 +827,9 @@ mod tests { authentication: SessionAuthentication::Oidc { issuer: Url::parse("https://issuer.example").expect("issuer URL"), client_id: "frameshift-cli".to_string(), - redirect_uri: Url::parse("http://127.0.0.1:8765/callback").expect("redirect URL"), + redirect_uri: Box::new( + Url::parse("http://127.0.0.1:8765/callback").expect("redirect URL"), + ), scopes: vec!["openid".to_string(), "profile".to_string()], }, registry_url: Url::parse("https://registry.example").expect("registry URL"), @@ -918,7 +920,7 @@ mod tests { if let SessionAuthentication::Oidc { redirect_uri, .. } = &mut remote_http_redirect.authentication { - *redirect_uri = Url::parse("http://192.0.2.1:8765/callback").expect("redirect URL"); + **redirect_uri = Url::parse("http://192.0.2.1:8765/callback").expect("redirect URL"); } cases.push(remote_http_redirect);