From 7da88fae2d57f20d73cb4aa5d5ed2d45e904437a Mon Sep 17 00:00:00 2001 From: yashau Date: Fri, 21 Aug 2026 04:17:48 +0500 Subject: [PATCH] feat(cli): revoke the session on logout instead of only deleting the file `prk logout` deleted the credential file and stopped there. A refresh token stays valid at the authorization server until it expires or someone revokes it, so that left a working credential behind while reporting success -- on exactly the machine someone had decided to stop trusting. It now hands the token back first (RFC 7009), then deletes the file. The refresh token is the one revoked when there is one: section 2.1 has the server invalidate the access tokens issued from it, and it is the half worth ending, because an access token expires on its own within minutes while a refresh token is what keeps a stolen credential alive. Revocation is advisory and deletion is not. Revocation needs the network; deletion does not. If a failed request could fail the command, a machine with no connectivity could not be signed out at all -- a worse outcome than signing out locally and telling the server later, because the local credential is the part this machine controls. So the file goes whatever the server said, and a revocation that did not happen is a warning naming what is still live rather than a silent omission. `--no-revoke` skips the request outright, for a host where it would only stall. The endpoint is recorded at login so logout does not repeat discovery on the command most likely to run somewhere with a worse network. A credential written before that field existed -- which is every credential in existence as this lands -- falls back to discovering it from the stored issuer, rather than declining to revoke the sessions most likely to be signed out first. Under `--json`, `revoked` separates the three outcomes: true revoked, false attempted and not done, null not attempted. The warnings print even under `--json`, which otherwise leaves stderr byte-empty on success, for the same reason the unprotected-server warning does. Co-Authored-By: Claude Opus 5 --- crates/prick-auth/src/discovery.rs | 8 ++ crates/prick-auth/src/oauth.rs | 71 +++++++++++++ crates/prick-auth/src/session.rs | 1 + crates/prick-auth/src/store.rs | 64 +++++++++++ crates/prick-auth/tests/login.rs | 105 ++++++++++++++++++ crates/prk/src/cli.rs | 6 +- crates/prk/src/commands/auth.rs | 164 +++++++++++++++++++++++++++-- crates/prk/src/commands/mod.rs | 3 +- docs/guides/authentication.md | 10 ++ docs/reference/cli/sign-in.md | 59 ++++++++++- 10 files changed, 478 insertions(+), 13 deletions(-) diff --git a/crates/prick-auth/src/discovery.rs b/crates/prick-auth/src/discovery.rs index b52580e..efef007 100644 --- a/crates/prick-auth/src/discovery.rs +++ b/crates/prick-auth/src/discovery.rs @@ -235,6 +235,13 @@ pub struct AuthorizationServer { /// Where a client registers itself, when the server supports RFC 7591. #[serde(default)] pub registration_endpoint: Option, + /// Where a token is revoked, when the server supports RFC 7009. + /// + /// Optional in the metadata and optional here. A server that advertises + /// none cannot be asked to forget a token, and `prk logout` says so rather + /// than implying it revoked something. + #[serde(default)] + pub revocation_endpoint: Option, /// The PKCE methods on offer. #[serde(default)] pub code_challenge_methods_supported: Option>, @@ -695,6 +702,7 @@ mod tests { authorization_endpoint: "https://x/authorize".to_owned(), token_endpoint: "https://x/token".to_owned(), registration_endpoint: Some("https://x/register".to_owned()), + revocation_endpoint: Some("https://x/revoke".to_owned()), code_challenge_methods_supported: Some(vec!["S256".to_owned()]), scopes_supported: None, }; diff --git a/crates/prick-auth/src/oauth.rs b/crates/prick-auth/src/oauth.rs index 1907dc2..99df887 100644 --- a/crates/prick-auth/src/oauth.rs +++ b/crates/prick-auth/src/oauth.rs @@ -224,6 +224,75 @@ fn now() -> u64 { SystemTime::now().duration_since(UNIX_EPOCH).map_or(0, |elapsed| elapsed.as_secs()) } +/// The RFC 7009 hint for a refresh token. +pub const HINT_REFRESH_TOKEN: &str = "refresh_token"; + +/// The RFC 7009 hint for an access token. +pub const HINT_ACCESS_TOKEN: &str = "access_token"; + +/// Asks the authorization server to forget a token (RFC 7009). +/// +/// # Which token to hand over +/// +/// The refresh token, when there is one. RFC 7009 section 2.1 says a server +/// SHOULD invalidate the access tokens issued from a refresh token it revokes, +/// so revoking the renewable half is what actually ends the session -- and it +/// is the half worth ending, because an access token expires on its own within +/// minutes while a refresh token is what keeps a stolen credential alive. +/// +/// # Why an unknown token is success +/// +/// Section 2.2 requires a `200` both when the token was revoked and when the +/// client submitted an invalid one, because the two are indistinguishable to a +/// client and telling them apart would turn this endpoint into an oracle for +/// whether a token is live. So a token the server has already forgotten is not +/// an error here either: the desired state is "this token does not work", and +/// it already holds. +/// +/// # Errors +/// +/// [`AuthError::Denied`] for an RFC 6749 error body, or a transport failure. +/// Callers are expected to treat any of them as advisory: see +/// `prk logout`, which discards the local credential either way. +pub async fn revoke( + client: &Client, + revocation_endpoint: &str, + client_id: &str, + token: &SecretString, + token_type_hint: &str, +) -> Result<(), AuthError> { + let form = [ + ("token", token.expose_secret()), + ("token_type_hint", token_type_hint), + // A public client authenticates with nothing but its identity: the + // registration used `token_endpoint_auth_method: none`, and there is no + // secret to present here. + ("client_id", client_id), + ]; + + let received = + client.fetch(reqwest::Method::POST, revocation_endpoint, Body::Form(&form)).await?; + let facts = &received.facts; + + if facts.status < 400 { + return Ok(()); + } + + if let Ok(body) = serde_json::from_slice::(received.body()) { + return Err(AuthError::Denied { error: body.error }); + } + + Err(match prick_api::response::classify(facts) { + Some(classified) => { + AuthError::Api(prick_api::ApiError::from_response(facts.clone(), classified)) + } + None => AuthError::Api(prick_api::ApiError::from_server( + facts.clone(), + format!("the revocation endpoint returned HTTP {}", facts.status), + )), + }) +} + /// Posts to the token endpoint and interprets the result. /// /// The one place `invalid_grant` is turned into [`AuthError::AuthExpired`], so @@ -521,6 +590,7 @@ where client_id: registration.client_id, token_endpoint: server.token_endpoint, resource, + revocation_endpoint: server.revocation_endpoint, tokens, }, probe, @@ -538,6 +608,7 @@ mod tests { authorization_endpoint: "https://example.cloudflareaccess.com/authorize".to_owned(), token_endpoint: "https://example.cloudflareaccess.com/token".to_owned(), registration_endpoint: Some("https://example.cloudflareaccess.com/register".to_owned()), + revocation_endpoint: Some("https://example.cloudflareaccess.com/revoke".to_owned()), code_challenge_methods_supported: Some(vec!["S256".to_owned()]), scopes_supported: None, } diff --git a/crates/prick-auth/src/session.rs b/crates/prick-auth/src/session.rs index d880e25..6f5b0a6 100644 --- a/crates/prick-auth/src/session.rs +++ b/crates/prick-auth/src/session.rs @@ -157,6 +157,7 @@ mod tests { client_id: "client-1".to_owned(), token_endpoint: "https://example.cloudflareaccess.com/token".to_owned(), resource: Some("https://prick.example.com".to_owned()), + revocation_endpoint: Some("https://example.cloudflareaccess.com/revoke".to_owned()), tokens: Tokens { access_token: SecretString::from("access-abc"), refresh_token: refreshable.then(|| SecretString::from("refresh-xyz")), diff --git a/crates/prick-auth/src/store.rs b/crates/prick-auth/src/store.rs index acc0850..3bd558b 100644 --- a/crates/prick-auth/src/store.rs +++ b/crates/prick-auth/src/store.rs @@ -125,6 +125,18 @@ pub struct StoredSession { /// every login did until Access started refusing it, so an old session /// refreshes as well as it ever did rather than failing to load. pub resource: Option, + /// The RFC 7009 revocation endpoint, so `prk logout` can hand the token + /// back without repeating discovery. + /// + /// Stored rather than rediscovered because logout is the one command that + /// must work when the network is worse than usual -- a laptop being handed + /// on, a machine being decommissioned -- and a discovery round trip is one + /// more thing between the operator and a revoked token. + /// + /// `None` for a server that advertises no revocation endpoint, and for a + /// session written before this field existed. Both mean the same thing at + /// logout: fall back to discovery, and say so if that finds nothing. + pub revocation_endpoint: Option, /// The tokens themselves. pub tokens: Tokens, } @@ -161,6 +173,8 @@ struct Wire { token_endpoint: String, #[serde(default, skip_serializing_if = "Option::is_none")] resource: Option, + #[serde(default, skip_serializing_if = "Option::is_none")] + revocation_endpoint: Option, access_token: String, #[serde(default, skip_serializing_if = "Option::is_none")] refresh_token: Option, @@ -183,6 +197,7 @@ impl Wire { client_id: session.client_id.clone(), token_endpoint: session.token_endpoint.clone(), resource: session.resource.clone(), + revocation_endpoint: session.revocation_endpoint.clone(), access_token: session.tokens.access_token.expose_secret().to_owned(), refresh_token: session .tokens @@ -200,6 +215,7 @@ impl Wire { client_id: std::mem::take(&mut self.client_id), token_endpoint: std::mem::take(&mut self.token_endpoint), resource: self.resource.take(), + revocation_endpoint: self.revocation_endpoint.take(), tokens: Tokens { access_token: SecretString::from(std::mem::take(&mut self.access_token)), refresh_token: self.refresh_token.take().map(SecretString::from), @@ -621,6 +637,7 @@ mod tests { client_id: "client-123".to_owned(), token_endpoint: "https://example.cloudflareaccess.com/token".to_owned(), resource: Some("https://prick.example.com".to_owned()), + revocation_endpoint: Some("https://example.cloudflareaccess.com/revoke".to_owned()), tokens: Tokens { access_token: SecretString::from("access-abc"), refresh_token: Some(SecretString::from("refresh-xyz")), @@ -782,6 +799,53 @@ mod tests { assert!(!loaded.is_refreshable()); } + #[test] + fn the_revocation_endpoint_survives_a_round_trip() { + let (_dir, store) = store(); + store.save(&session()).expect("save"); + + let loaded = store.load().expect("load").expect("a session"); + assert_eq!( + loaded.revocation_endpoint.as_deref(), + Some("https://example.cloudflareaccess.com/revoke") + ); + } + + #[test] + fn a_credential_written_before_revocation_existed_still_loads() { + // The compatibility case that matters: everyone signed in today has a + // file with no `revocation_endpoint` in it, and a logout that refused to + // read it would leave them unable to sign out at all. + let (_dir, store) = store(); + store.save(&session()).expect("save"); + + let raw = std::fs::read_to_string(store.path()).expect("read"); + let older: serde_json::Value = serde_json::from_str(&raw).expect("JSON"); + let mut older = older.as_object().expect("an object").clone(); + older.remove("revocation_endpoint"); + std::fs::write(store.path(), serde_json::to_string(&older).expect("JSON")) + .expect("write the older shape back"); + + let loaded = store.load().expect("an older file still loads").expect("a session"); + assert!(loaded.revocation_endpoint.is_none()); + // And the rest of it is intact, so the fallback is the only difference. + assert_eq!(loaded.client_id, session().client_id); + assert!(loaded.is_refreshable()); + } + + #[test] + fn a_session_with_nowhere_to_revoke_writes_no_such_field() { + // `skip_serializing_if`, so a server that advertises no revocation + // endpoint does not get a null recorded for one. + let (_dir, store) = store(); + let mut without = session(); + without.revocation_endpoint = None; + store.save(&without).expect("save"); + + let raw = std::fs::read_to_string(store.path()).expect("read"); + assert!(!raw.contains("revocation_endpoint"), "{raw}"); + } + #[test] fn the_keyring_backend_says_so_rather_than_writing_a_file() { let dir = tempfile::tempdir().expect("a temporary directory"); diff --git a/crates/prick-auth/tests/login.rs b/crates/prick-auth/tests/login.rs index 5fd16af..3b650f4 100644 --- a/crates/prick-auth/tests/login.rs +++ b/crates/prick-auth/tests/login.rs @@ -79,6 +79,7 @@ async fn mount_discovery(server: &MockServer) { "authorization_endpoint": format!("{}/authorize", server.uri()), "token_endpoint": format!("{}/token", server.uri()), "registration_endpoint": format!("{}/register", server.uri()), + "revocation_endpoint": format!("{}/revoke", server.uri()), "code_challenge_methods_supported": ["S256"], "scopes_supported": ["openid", "email", "profile", "offline_access"], }))) @@ -177,6 +178,13 @@ async fn a_full_login_produces_a_storable_session() { outcome.session.tokens.refresh_token.as_ref().map(SecretString::expose_secret), Some("refresh-1") ); + // Recorded at login so that logout can revoke without repeating discovery, + // on the one command most likely to run somewhere with a worse network than + // the login had. + assert_eq!( + outcome.session.revocation_endpoint.as_deref(), + Some(format!("{}/revoke", server.uri()).as_str()) + ); assert!( outcome.session.tokens.expires_at.is_some(), "expires_in was not turned into a deadline" @@ -779,6 +787,7 @@ async fn a_stale_session_is_renewed_before_the_request_that_needs_it() { client_id: "client-1".to_owned(), token_endpoint: format!("{}/token", server.uri()), resource: Some(server.uri()), + revocation_endpoint: Some(format!("{}/revoke", server.uri())), tokens: Tokens { access_token: SecretString::from("access-stale"), refresh_token: Some(SecretString::from("refresh-1")), @@ -832,6 +841,7 @@ async fn a_revoked_session_surfaces_as_expired_rather_than_as_a_server_error() { client_id: "client-1".to_owned(), token_endpoint: format!("{}/token", server.uri()), resource: None, + revocation_endpoint: None, tokens: Tokens { access_token: SecretString::from("stale"), refresh_token: Some(SecretString::from("revoked")), @@ -848,6 +858,101 @@ async fn a_revoked_session_surfaces_as_expired_rather_than_as_a_server_error() { assert_eq!(err.exit_code(), 3); } +#[tokio::test] +async fn a_revocation_hands_back_the_refresh_token_as_a_form_post() { + let server = MockServer::start().await; + Mock::given(method("POST")) + .and(path("/revoke")) + .respond_with(ResponseTemplate::new(200)) + .mount(&server) + .await; + + prick_auth::oauth::revoke( + &client_for(&server), + &format!("{}/revoke", server.uri()), + "client-1", + &SecretString::from("refresh-xyz"), + prick_auth::oauth::HINT_REFRESH_TOKEN, + ) + .await + .expect("a 200 means the server has forgotten it"); + + let requests = server.received_requests().await.expect("the server recorded requests"); + let revocation = requests + .iter() + .find(|request| request.url.path() == "/revoke") + .expect("the revocation was sent"); + + let body = String::from_utf8_lossy(&revocation.body); + let sent: std::collections::HashMap = + url::form_urlencoded::parse(body.as_bytes()) + .map(|(key, value)| (key.into_owned(), value.into_owned())) + .collect(); + + assert_eq!(sent.get("token").map(String::as_str), Some("refresh-xyz")); + assert_eq!(sent.get("token_type_hint").map(String::as_str), Some("refresh_token")); + // A public client authenticates with its identity alone, so the id has to be + // in the body -- without it the server cannot tell whose token this is. + assert_eq!(sent.get("client_id").map(String::as_str), Some("client-1")); + + let content_type = revocation + .headers + .get("content-type") + .and_then(|value| value.to_str().ok()) + .unwrap_or_default(); + assert!(content_type.starts_with("application/x-www-form-urlencoded"), "{content_type}"); +} + +#[tokio::test] +async fn a_token_the_server_never_knew_is_not_a_failure() { + // RFC 7009 section 2.2: a `200` covers both "revoked" and "that was not a + // token I recognise", because distinguishing them would make this endpoint + // an oracle for whether a token is live. The desired state already holds. + let server = MockServer::start().await; + Mock::given(method("POST")) + .and(path("/revoke")) + .respond_with(ResponseTemplate::new(200)) + .mount(&server) + .await; + + prick_auth::oauth::revoke( + &client_for(&server), + &format!("{}/revoke", server.uri()), + "client-1", + &SecretString::from("never-existed"), + prick_auth::oauth::HINT_ACCESS_TOKEN, + ) + .await + .expect("an unknown token leaves nothing to do"); +} + +#[tokio::test] +async fn a_refused_revocation_reports_the_servers_own_reason() { + let server = MockServer::start().await; + Mock::given(method("POST")) + .and(path("/revoke")) + .respond_with(ResponseTemplate::new(400).set_body_json(serde_json::json!({ + "error": "unsupported_token_type" + }))) + .mount(&server) + .await; + + let err = prick_auth::oauth::revoke( + &client_for(&server), + &format!("{}/revoke", server.uri()), + "client-1", + &SecretString::from("refresh-xyz"), + prick_auth::oauth::HINT_REFRESH_TOKEN, + ) + .await + .expect_err("a 400 is a refusal"); + + match err { + AuthError::Denied { error } => assert_eq!(error, "unsupported_token_type"), + other => panic!("expected the server's own error code, got {other:?}"), + } +} + fn now() -> u64 { std::time::SystemTime::now() .duration_since(std::time::UNIX_EPOCH) diff --git a/crates/prk/src/cli.rs b/crates/prk/src/cli.rs index 1d59408..44c0c63 100644 --- a/crates/prk/src/cli.rs +++ b/crates/prk/src/cli.rs @@ -174,8 +174,8 @@ pub enum Command { /// Authenticate against a prick server. Login(commands::auth::LoginArgs), - /// Discard stored credentials. - Logout, + /// Revoke the session and discard stored credentials. + Logout(commands::auth::LogoutArgs), /// Show the identity the server sees. Whoami, @@ -218,7 +218,7 @@ impl Command { pub fn path(&self) -> &'static str { match self { Self::Login(_) => "login", - Self::Logout => "logout", + Self::Logout(_) => "logout", Self::Whoami => "whoami", Self::Doctor => "doctor", Self::Projects(sub) => sub.path(), diff --git a/crates/prk/src/commands/auth.rs b/crates/prk/src/commands/auth.rs index 99974a4..6e0b12c 100644 --- a/crates/prk/src/commands/auth.rs +++ b/crates/prk/src/commands/auth.rs @@ -4,7 +4,9 @@ use std::io::IsTerminal as _; use clap::Args; -use prick_auth::{AuthError, Probe, RedirectSource, StorageBackend, TokenStore, discovery}; +use prick_auth::{ + AuthError, Probe, RedirectSource, StorageBackend, StoredSession, TokenStore, discovery, +}; use crate::cli::GlobalArgs; use crate::commands::Context; @@ -40,6 +42,18 @@ pub struct LoginArgs { pub no_browser: bool, } +/// Arguments to `prk logout`. +#[derive(Debug, Clone, Args)] +pub struct LogoutArgs { + /// Discard the local credential without asking the server to forget it. + /// + /// For a machine with no route to the authorization server, where the + /// request would only stall. The token keeps working until it expires, so + /// this trades a live credential for not waiting. + #[arg(long)] + pub no_revoke: bool, +} + /// Command-line spelling of [`StorageBackend`]. /// /// A separate type so the `clap` derive does not have to reach into @@ -171,28 +185,164 @@ fn warn_unprotected(out: Output) { out.warn("Put the application behind Cloudflare Access before storing anything in it."); } -/// Discards stored credentials. +/// Discards stored credentials, and asks the server to forget them. +/// +/// # Deleting the file is not signing out +/// +/// A refresh token stays valid at the authorization server until it expires on +/// its own or someone revokes it. Removing the local copy makes it unreachable +/// from this machine and does nothing about the copy the server will still +/// honour, so a logout that only deleted the file would leave a live credential +/// behind while reporting success. +/// +/// So the token is handed back first, then the file goes. +/// +/// # Revocation is advisory, deletion is not +/// +/// Revocation needs the network; deletion does not. If asking the server were +/// allowed to fail the command, a laptop with no connectivity could not be +/// signed out at all -- and "could not sign out" is a worse outcome than "signed +/// out here, tell the server later", because the operator wanted the local +/// credential gone and it is the one thing this machine controls. +/// +/// So the credential is discarded whatever the server said, and a revocation +/// that did not happen is a warning naming what is still live rather than a +/// silent omission. /// /// Idempotent: the state it establishes is "no credentials", and running it /// twice does not make that less true. /// /// # Errors /// -/// [`CliError::Auth`] if the token file exists and cannot be removed. -pub fn logout(global: &GlobalArgs, out: Output) -> Result<(), CliError> { +/// [`CliError::Auth`] if the token file exists and cannot be removed. A failed +/// revocation is reported, not returned. +pub fn logout(args: &LogoutArgs, global: &GlobalArgs, out: Output) -> Result<(), CliError> { let store = TokenStore::new(StorageBackend::File)?; - let had_session = store.load().unwrap_or(None).is_some(); + let session = store.load().unwrap_or(None); + + let revocation = match &session { + Some(session) if !args.no_revoke => Some(revoke_session(session, global, out)), + // Nothing to revoke, or the operator asked for the local half only. + _ => None, + }; + + // Unconditional, and after the attempt: the token is needed to revoke it, + // and the file must go even when the attempt failed. store.clear()?; + report_logout(session.is_some(), revocation, global, out); + Ok(()) +} + +/// What became of the attempt to have the server forget the token. +#[derive(Debug, Clone, Copy, PartialEq, Eq)] +enum Revocation { + /// The server accepted it. + Revoked, + /// The server advertises no revocation endpoint, so there is nothing to ask. + Unsupported, + /// The attempt failed. The token is still live until it expires. + Failed, +} + +/// Hands the session's token back to the authorization server. +/// +/// The refresh token when there is one, because revoking it is what ends the +/// session; otherwise the access token, which is all there is to give back. +fn revoke_session(session: &StoredSession, global: &GlobalArgs, out: Output) -> Revocation { + let (token, hint) = match session.tokens.refresh_token.clone() { + Some(refresh) => (refresh, prick_auth::oauth::HINT_REFRESH_TOKEN), + None => (session.tokens.access_token.clone(), prick_auth::oauth::HINT_ACCESS_TOKEN), + }; + + let mut with_url = global.clone(); + with_url.api_url = Some(session.api_url.clone()); + let context = match Context::new(&with_url) { + Ok(context) => context, + Err(err) => { + out.warn(&format!("could not prepare the revocation request: {err}")); + return Revocation::Failed; + } + }; + + context.block_on(async { + let endpoint = match session.revocation_endpoint.clone() { + Some(endpoint) => endpoint, + // A credential stored before the endpoint was recorded -- which is + // every credential that exists at the moment this ships. The issuer + // is in the file, so ask it once rather than declining to revoke the + // very sessions most likely to be signed out first. + None => { + match discovery::fetch_authorization_server(context.client(), &session.issuer).await + { + Ok(server) => match server.revocation_endpoint { + Some(endpoint) => endpoint, + None => return Revocation::Unsupported, + }, + Err(err) => { + out.warn(&format!("could not find where to revoke the token: {err}")); + return Revocation::Failed; + } + } + } + }; + + match prick_auth::oauth::revoke( + context.client(), + &endpoint, + &session.client_id, + &token, + hint, + ) + .await + { + Ok(()) => Revocation::Revoked, + Err(err) => { + out.warn(&format!("the token could not be revoked: {err}")); + Revocation::Failed + } + } + }) +} + +/// Reports what happened, and what is still live if anything. +/// +/// The warning for a token left behind goes through [`Output::warn`], which +/// `--json` does not suppress, for the same reason the unprotected-server +/// warning does: a credential that still works somewhere is worth breaking the +/// byte-empty-stderr rule for. +fn report_logout( + had_session: bool, + revocation: Option, + global: &GlobalArgs, + out: Output, +) { if global.json { - out.json(&serde_json::json!({ "logged_out": had_session })); + out.json(&serde_json::json!({ + "logged_out": had_session, + "revoked": match revocation { + Some(Revocation::Revoked) => Some(true), + Some(Revocation::Unsupported | Revocation::Failed) => Some(false), + None => None, + }, + })); } else if had_session { out.data("Signed out."); } else { out.data("No stored credentials."); } - Ok(()) + match revocation { + Some(Revocation::Failed) => out.warn( + "The credential is gone from this machine, but the server was not told. It keeps \ + working until it expires -- revoke the session in Zero Trust > Access if that matters.", + ), + Some(Revocation::Unsupported) => out.warn( + "This authorization server advertises no revocation endpoint, so the token could only \ + be discarded locally. It keeps working until it expires.", + ), + Some(Revocation::Revoked) | None => (), + } } /// Shows the identity the server resolved for this caller. diff --git a/crates/prk/src/commands/mod.rs b/crates/prk/src/commands/mod.rs index 37b26a1..1cedc87 100644 --- a/crates/prk/src/commands/mod.rs +++ b/crates/prk/src/commands/mod.rs @@ -277,7 +277,7 @@ pub fn dispatch(cli: &Cli, out: Output) -> Result<(), CliError> { Command::Version => version::run(out), Command::Login(args) => auth::login(args, &cli.global, out), - Command::Logout => auth::logout(&cli.global, out), + Command::Logout(args) => auth::logout(args, &cli.global, out), Command::Whoami => auth::whoami(&cli.global, out), Command::Doctor => doctor::run(&cli.global, out), @@ -421,6 +421,7 @@ mod tests { client_id: "client-123".to_owned(), token_endpoint: "https://example.cloudflareaccess.com/token".to_owned(), resource: None, + revocation_endpoint: None, tokens: prick_auth::Tokens { access_token: SecretString::from("access-abc"), refresh_token: None, diff --git a/docs/guides/authentication.md b/docs/guides/authentication.md index defb322..f53cdeb 100644 --- a/docs/guides/authentication.md +++ b/docs/guides/authentication.md @@ -122,6 +122,16 @@ prk logout Signed out. ``` +This revokes the session at the authorization server and then deletes the local +credential. Both halves matter: a refresh token stays valid until it expires or +is revoked, so deleting the file alone would leave a working credential behind +on a machine you thought you had signed out of. + +The file is deleted whether or not the revocation succeeded, and a revocation +that did not happen is warned about rather than passed over silently. See +[`prk logout`](/reference/cli/sign-in#prk-logout) for the failure cases and for +`--no-revoke`. + ## Authenticate a machine CI uses an Access **service token**, not `prk login`. diff --git a/docs/reference/cli/sign-in.md b/docs/reference/cli/sign-in.md index 8d369c0..4c97d1f 100644 --- a/docs/reference/cli/sign-in.md +++ b/docs/reference/cli/sign-in.md @@ -163,10 +163,14 @@ store anything. See [Quickstart step 9](/getting-started/quickstart). ## `prk logout` ``` -prk logout +prk logout [--no-revoke] ``` -Discard stored credentials. +Revoke the session and discard stored credentials. + +| Flag | Default | Meaning | +| ------------- | ------- | ------------------------------------------------------- | +| `--no-revoke` | off | Discard the local credential without telling the server | ```bash prk logout @@ -185,6 +189,57 @@ No stored credentials. Running it twice is harmless — it establishes the state "no credentials", and that is idempotent. +### Deleting the file is not signing out + +A refresh token stays valid at the authorization server until it expires on its +own or someone revokes it. Removing the local copy makes it unreachable from +_this machine_ and does nothing about the copy the server will still honour — so +`prk logout` hands the token back first, then deletes the file. + +The refresh token is the one revoked, when there is one. RFC 7009 says a server +should invalidate the access tokens issued from a refresh token it revokes, and +the refresh token is the half worth ending: an access token expires on its own +within minutes, while a refresh token is what keeps a stolen credential alive. + +### The local credential goes either way + +Revocation needs the network. Deleting the file does not. If a failed request +could fail the command, a laptop with no connectivity could not be signed out at +all — and that is a worse outcome than signing out locally and telling the server +later, because the local credential is the part this machine controls. + +So the credential is discarded whatever the server said, and a revocation that +did not happen is a warning naming what is still live rather than a silent +omission: + +``` +Signed out. +warning: The credential is gone from this machine, but the server was not told. It keeps working until it expires -- revoke the session in Zero Trust > Access if that matters. +``` + +The same warning, with its own wording, covers an authorization server that +advertises no revocation endpoint at all — there is nothing to ask, and the token +lives out its lifetime. + +`--no-revoke` skips the request deliberately, for a machine with no route to the +authorization server where it would only stall. It trades a live credential for +not waiting. + +Under `--json`, `revoked` distinguishes the three outcomes — `true` revoked, +`false` attempted and not done, `null` not attempted: + +```bash +prk logout --json +``` + +```json +{ "logged_out": true, "revoked": true } +``` + +The warnings are printed even under `--json`, which otherwise leaves stderr +byte-empty on success. A credential that still works somewhere is worth breaking +that rule for, the same way an unprotected server is. + ## `prk whoami` ```