Skip to content

Commit 34313bf

Browse files
jpage-godaddyjgowdy-godaddyclaude
authored
feat!: fail-closed authentication via AuthRequirement; populate PKCE identity (#17)
Replace the implicit eager-auth model with an explicit, fail-closed per-command authentication policy, and populate credential identity from PKCE tokens. Authentication policy (`CommandSpec.auth: AuthRequirement`): - Required (default): the engine resolves the credential before the handler runs and renders an auth-error if it cannot — a gated command cannot execute unauthenticated even if its handler never reads the credential, and its audit/activity identity is always populated. - Optional (`auth_optional()`): resolution is deferred; the handler calls `resolve()`/`try_resolve()` only when a decision depends on the credential. - None (`no_auth(true)`): never authenticates; default-env injection is suppressed. Credential resolution stays memoized behind a `CredentialResolver`, and `--schema`/`--dry-run` short-circuit before any Required resolve, so they never trigger an auth flow. A forgotten annotation now over-prompts rather than letting a gated command run unauthenticated. Auth-error outcomes are classified by the error a handler or authorizer actually returns (`CliCoreError::is_auth()`, with provider failures wrapped in `CliCoreError::AuthProvider`) instead of a sticky flag, so a swallowed-then- unrelated error is no longer misclassified. Auth failures attribute the activity backend to the auth provider. `PkceAuthProvider` now fills `Credential.identity`/`sub` by decoding the access-token JWT payload (display-only, no signature verification), with a configurable claim priority via `with_identity_claims`. BREAKING CHANGE: `CommandSpec.no_auth` (bool) is replaced by `CommandSpec.auth` (`AuthRequirement`), and `MiddlewareRequest.no_auth` by `MiddlewareRequest.auth`. `CommandContext.credential` is now a `CredentialResolver` instead of `Option<Credential>`; `RuntimeCommandSpec::new` and `new_typed` handler closures receive a `CredentialResolver`; and `Authorizer::authorize` receives `&CredentialResolver` instead of `Option<&Credential>`. The `no_auth(true)` builder still works and maps to `AuthRequirement::None`; `auth_optional()` and `auth(AuthRequirement)` select the other policies. Co-authored-by: Jay Gowdy <jgowdy@godaddy.com> Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
1 parent c21db13 commit 34313bf

13 files changed

Lines changed: 1121 additions & 136 deletions

File tree

AGENTS.md

Lines changed: 13 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -130,7 +130,7 @@ When a command has many flags, complex validation, or an existing `#[derive(clap
130130
use the typed path instead:
131131

132132
```rust
133-
use cli_engine::{CommandResult, CommandSpec, Credential, RuntimeCommandSpec};
133+
use cli_engine::{CommandResult, CommandSpec, CredentialResolver, RuntimeCommandSpec};
134134
use serde_json::json;
135135

136136
#[derive(Debug, Clone, clap::Args)]
@@ -146,7 +146,7 @@ let command = RuntimeCommandSpec::new_typed::<ListArgs, _, _, _>(
146146
CommandSpec::from_args::<ListArgs>("list", "List projects")
147147
.with_system("projects-api")
148148
.with_default_fields("id,name,status"),
149-
async |_credential: Option<Credential>, args: ListArgs| {
149+
async |_credential: CredentialResolver, args: ListArgs| {
150150
Ok(CommandResult::new(json!([
151151
{"id": "p1", "name": "Portal", "team": args.team, "limit": args.limit}
152152
])))
@@ -219,7 +219,17 @@ Command checklist:
219219
- Set `.with_default_fields(...)` for list-style output.
220220
- Set `.with_json_schema::<T>()` when the response shape is known.
221221
- Add `clap::Arg` values with the exact user-facing flag names the CLI should expose.
222-
- Use `.no_auth(true)` only for commands that genuinely do not need credentials.
222+
- Authentication is fail-closed by default (`AuthRequirement::Required`): the engine resolves the
223+
credential before the handler runs, so a command that should be gated cannot execute
224+
unauthenticated even if its handler never reads the credential. Handlers receive a
225+
`CredentialResolver`; for `Required` commands the credential is already resolved, so
226+
`resolver.resolve().await?` (or `ctx.credential().await?`) is a memoized lookup. `--schema` and
227+
`--dry-run` short-circuit before resolution, so they never trigger an auth flow.
228+
- Use `.auth_optional()` for commands that must run while logged out and only enrich output when a
229+
credential happens to be present; the engine does not resolve on their behalf, so the handler
230+
decides via `resolver.try_resolve().await?`. Use `.no_auth(true)` for commands that never
231+
authenticate (this also suppresses default-env injection). Forgetting these annotations only
232+
over-prompts; it never lets a gated command run unauthenticated.
223233
- Use `.with_tier(...)` or `.mutates(true)` for mutating commands so `--dry-run` can short-circuit them.
224234
- Prefer returning structured JSON values from handlers; let cli-engine render JSON, human, and TOON formats.
225235
- Prefer `CommandSpec::from_args::<T>()` + `RuntimeCommandSpec::new_typed` when the command has many flags, needs clap validation attributes, or when porting existing derive-based commands. Use the builder path for simple commands with one or two flags.

docs/concepts.md

Lines changed: 6 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -172,9 +172,12 @@ the bounded channel can fill and the handler will wait on `send` until the write
172172

173173
```rust
174174
async fn handler(
175-
credential: Option<cli_engine::Credential>,
175+
credential: cli_engine::CredentialResolver,
176176
args: cli_engine::middleware::ValueMap,
177177
) -> cli_engine::Result<cli_engine::CommandResult> {
178+
// Auth is fail-closed by default: the engine resolves the credential before
179+
// this handler runs, so `credential.resolve().await?` here is a memoized
180+
// lookup. Mark the command `.auth_optional()` or `.no_auth(true)` to opt out.
178181
Ok(cli_engine::CommandResult::new(serde_json::json!({ "ok": true })))
179182
}
180183
```
@@ -188,7 +191,7 @@ Commands can also define arguments with `#[derive(clap::Args)]` structs instead
188191
builders. This gives compile-time type safety from argument definition through handler consumption:
189192

190193
```rust
191-
use cli_engine::{CommandResult, CommandSpec, Credential, RuntimeCommandSpec};
194+
use cli_engine::{CommandResult, CommandSpec, CredentialResolver, RuntimeCommandSpec};
192195
use serde_json::json;
193196

194197
#[derive(Debug, Clone, clap::Args)]
@@ -204,7 +207,7 @@ let command = RuntimeCommandSpec::new_typed::<ListArgs, _, _, _>(
204207
CommandSpec::from_args::<ListArgs>("list", "List projects")
205208
.with_system("projects-api")
206209
.with_default_fields("id,name,status"),
207-
async |_credential: Option<Credential>, args: ListArgs| {
210+
async |_credential: CredentialResolver, args: ListArgs| {
208211
Ok(CommandResult::new(json!([
209212
{"id": "p1", "name": "Portal", "team": args.team}
210213
])))

docs/design.md

Lines changed: 8 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -202,7 +202,7 @@ fn list_projects() -> RuntimeCommandSpec {
202202
CommandSpec::from_args::<ListArgs>("list", "List projects")
203203
.with_system("projects-api")
204204
.with_default_fields("id,name,status"),
205-
async |_credential: Option<Credential>, args: ListArgs| {
205+
async |_credential: CredentialResolver, args: ListArgs| {
206206
Ok(CommandResult::new(json!([
207207
{"id": "p1", "name": "Portal", "team": args.team, "limit": args.limit}
208208
])))
@@ -286,7 +286,13 @@ The provider process contract and transport injectors are described in
286286
[Authentication and Transport](auth.md).
287287

288288
Authorization is optional and supplied by an `Authorizer` attached to middleware. The authorizer
289-
receives command path, effective args, optional credential, reason, and tier.
289+
receives command path, effective args, a `CredentialResolver`, reason, and tier. Authentication
290+
policy is declared per command via `AuthRequirement` and defaults to `Required` (fail-closed): the
291+
engine resolves the credential before the handler runs and renders an `auth-error` if it cannot, so
292+
a command that should be gated cannot execute unauthenticated. The `CredentialResolver` memoizes a
293+
single resolution shared by the engine, the authorizer, and the handler. `Optional` commands defer
294+
resolution to the handler, `None` commands never authenticate, and `--schema`/`--dry-run`
295+
short-circuit before resolution so they never trigger an auth flow.
290296

291297
Auditors and activity emitters are also pluggable traits. They receive enough context to record
292298
success, auth failures, authorization denials, dry-runs, command errors, and command duration.

examples/typed.rs

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,7 @@
11
use std::process::ExitCode;
22

33
use cli_engine::{
4-
BuildInfo, Cli, CliConfig, CommandResult, CommandSpec, Credential, GroupSpec, Module,
4+
BuildInfo, Cli, CliConfig, CommandResult, CommandSpec, CredentialResolver, GroupSpec, Module,
55
RuntimeCommandSpec, RuntimeGroupSpec,
66
};
77
use serde_json::json;
@@ -24,7 +24,7 @@ async fn main() -> ExitCode {
2424
.with_system("projects-api")
2525
.with_default_fields("id,name,status")
2626
.no_auth(true),
27-
async |_credential: Option<Credential>, args: ListArgs| {
27+
async |_credential: CredentialResolver, args: ListArgs| {
2828
Ok(CommandResult::new(json!([
2929
{
3030
"id": "project-1",

src/auth/pkce.rs

Lines changed: 163 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -49,6 +49,7 @@ use base64::{Engine as _, engine::general_purpose::URL_SAFE_NO_PAD};
4949
use chrono::{SecondsFormat, Utc};
5050
use rand::Rng;
5151
use serde::{Deserialize, Serialize};
52+
use serde_json::{Map, Value};
5253
use sha2::{Digest, Sha256};
5354
use tokio::sync::RwLock;
5455
use zeroize::{Zeroize, ZeroizeOnDrop};
@@ -108,10 +109,17 @@ pub struct PkceAuthProvider {
108109
app_id: String,
109110
env_prefix: String,
110111
allow_file_fallback: bool,
112+
/// Prioritized JWT claim names used to derive `Credential.identity` from the
113+
/// decoded access-token payload. First non-empty string claim wins.
114+
identity_claims: Vec<String>,
111115
/// In-process token cache keyed by env.
112116
cache: Arc<RwLock<HashMap<String, StoredToken>>>,
113117
}
114118

119+
/// Default prioritized claim names for deriving a human-readable identity.
120+
const DEFAULT_IDENTITY_CLAIMS: &[&str] =
121+
&["email", "preferred_username", "username", "name", "sub"];
122+
115123
impl PkceAuthProvider {
116124
/// Creates a new PKCE provider.
117125
///
@@ -141,6 +149,10 @@ impl PkceAuthProvider {
141149
app_id: String::new(),
142150
env_prefix,
143151
allow_file_fallback: false,
152+
identity_claims: DEFAULT_IDENTITY_CLAIMS
153+
.iter()
154+
.map(|claim| (*claim).to_owned())
155+
.collect(),
144156
cache: Arc::new(RwLock::new(HashMap::new())),
145157
}
146158
}
@@ -191,6 +203,47 @@ impl PkceAuthProvider {
191203
self
192204
}
193205

206+
/// Overrides the prioritized JWT claim names used to derive
207+
/// [`Credential::identity`](crate::Credential) from the decoded access-token
208+
/// payload.
209+
///
210+
/// The first claim whose value is a non-empty string wins. The default order
211+
/// is `email`, `preferred_username`, `username`, `name`, `sub`. Use this when
212+
/// the identity provider exposes the human identity under a non-standard
213+
/// claim name.
214+
#[must_use]
215+
pub fn with_identity_claims(mut self, claims: &[impl AsRef<str>]) -> Self {
216+
self.identity_claims = claims.iter().map(|c| c.as_ref().to_owned()).collect();
217+
self
218+
}
219+
220+
/// Builds a [`Credential`] from a stored token, deriving `identity` and `sub`
221+
/// from the access-token JWT claims when present.
222+
fn build_credential(&self, env: &str, token: &StoredToken) -> Credential {
223+
let claims = decode_jwt_claims(&token.access_token);
224+
let identity = claims
225+
.as_ref()
226+
.map(|claims| extract_identity(claims, &self.identity_claims))
227+
.unwrap_or_default();
228+
let sub = claims
229+
.as_ref()
230+
.and_then(|claims| claims.get("sub"))
231+
.and_then(Value::as_str)
232+
.unwrap_or_default()
233+
.to_owned();
234+
Credential {
235+
token: token.access_token.clone(),
236+
env: env.to_owned(),
237+
provider: self.name.clone(),
238+
expires_at: chrono::DateTime::from_timestamp(token.expires_at, 0)
239+
.map(|dt| dt.to_rfc3339_opts(SecondsFormat::Secs, true))
240+
.unwrap_or_default(),
241+
identity,
242+
sub,
243+
..Credential::default()
244+
}
245+
}
246+
194247
fn effective_client_id(&self) -> String {
195248
let key = format!("{}_OAUTH_CLIENT_ID", self.env_prefix);
196249
std::env::var(&key).unwrap_or_else(|_| self.client_id.clone())
@@ -592,15 +645,7 @@ impl AuthProvider for PkceAuthProvider {
592645

593646
async fn get_credential(&self, env: &str, _command: &str, _tier: &str) -> Result<Credential> {
594647
let token = self.resolve_token(env).await?;
595-
Ok(Credential {
596-
token: token.access_token.clone(),
597-
env: env.to_owned(),
598-
provider: self.name.clone(),
599-
expires_at: chrono::DateTime::from_timestamp(token.expires_at, 0)
600-
.map(|dt| dt.to_rfc3339_opts(SecondsFormat::Secs, true))
601-
.unwrap_or_default(),
602-
..Credential::default()
603-
})
648+
Ok(self.build_credential(env, &token))
604649
}
605650

606651
async fn status(&self, env: &str) -> Result<Credential> {
@@ -609,15 +654,7 @@ impl AuthProvider for PkceAuthProvider {
609654
"not logged in for environment {env:?}"
610655
)));
611656
};
612-
Ok(Credential {
613-
token: token.access_token.clone(),
614-
env: env.to_owned(),
615-
provider: self.name.clone(),
616-
expires_at: chrono::DateTime::from_timestamp(token.expires_at, 0)
617-
.map(|dt| dt.to_rfc3339_opts(SecondsFormat::Secs, true))
618-
.unwrap_or_default(),
619-
..Credential::default()
620-
})
657+
Ok(self.build_credential(env, &token))
621658
}
622659

623660
async fn logout(&self, env: &str) -> Result<()> {
@@ -978,6 +1015,31 @@ struct TokenResponse {
9781015
refresh_token: Option<String>,
9791016
}
9801017

1018+
/// Decodes the claims (payload) segment of a JWT **without verifying the
1019+
/// signature**.
1020+
///
1021+
/// The returned claims are used only to display a human-readable identity in
1022+
/// `auth status` and audit logs — never for trust or authorization decisions, so
1023+
/// signature verification is intentionally skipped. Opaque (non-JWT) tokens and
1024+
/// any decode/parse failure yield `None`, leaving the identity blank.
1025+
fn decode_jwt_claims(token: &str) -> Option<Map<String, Value>> {
1026+
// A JWT is `header.payload.signature`; the payload is the middle segment,
1027+
// base64url-encoded without padding.
1028+
let payload = token.split('.').nth(1)?;
1029+
let bytes = URL_SAFE_NO_PAD.decode(payload).ok()?;
1030+
serde_json::from_slice(&bytes).ok()
1031+
}
1032+
1033+
/// Returns the first claim value that is a non-empty string, in priority order.
1034+
fn extract_identity(claims: &Map<String, Value>, priority: &[String]) -> String {
1035+
priority
1036+
.iter()
1037+
.filter_map(|name| claims.get(name).and_then(Value::as_str))
1038+
.find(|value| !value.is_empty())
1039+
.unwrap_or_default()
1040+
.to_owned()
1041+
}
1042+
9811043
async fn parse_token_response(response: reqwest::Response, _env: &str) -> Result<StoredToken> {
9821044
let body: TokenResponse = response
9831045
.json()
@@ -997,6 +1059,8 @@ async fn parse_token_response(response: reqwest::Response, _env: &str) -> Result
9971059
// module serialises all access so usage here is data-race-free.
9981060
#[allow(unsafe_code)]
9991061
mod tests {
1062+
use serde_json::json;
1063+
10001064
use super::*;
10011065

10021066
/// Serialises access to XDG_CONFIG_HOME (and restores it) so env-var tests
@@ -1336,4 +1400,85 @@ mod tests {
13361400
let envs = provider.list_environments().await.expect("list");
13371401
assert!(envs.is_empty(), "expected empty list for a fresh provider");
13381402
}
1403+
1404+
/// Builds an unsigned-looking JWT (`header.payload.signature`) whose payload
1405+
/// is the given claims object, base64url-encoded without padding.
1406+
fn make_jwt(claims: &Value) -> String {
1407+
let header = URL_SAFE_NO_PAD.encode(br#"{"alg":"none","typ":"JWT"}"#);
1408+
let payload = URL_SAFE_NO_PAD.encode(serde_json::to_vec(claims).expect("serialize claims"));
1409+
format!("{header}.{payload}.signature")
1410+
}
1411+
1412+
#[test]
1413+
fn decode_jwt_claims_extracts_payload() {
1414+
let token = make_jwt(&json!({"email": "user@example.com", "sub": "abc123"}));
1415+
let claims = decode_jwt_claims(&token).expect("claims decode");
1416+
assert_eq!(
1417+
claims.get("email").and_then(Value::as_str),
1418+
Some("user@example.com")
1419+
);
1420+
assert_eq!(claims.get("sub").and_then(Value::as_str), Some("abc123"));
1421+
}
1422+
1423+
#[test]
1424+
fn decode_jwt_claims_returns_none_for_non_jwt() {
1425+
assert!(decode_jwt_claims("opaque-access-token").is_none());
1426+
assert!(decode_jwt_claims("only.two").is_none());
1427+
// Valid structure but the payload is not valid base64/JSON.
1428+
assert!(decode_jwt_claims("aaa.!!!.bbb").is_none());
1429+
}
1430+
1431+
#[test]
1432+
fn extract_identity_honors_priority_and_skips_empty() {
1433+
let priority: Vec<String> = DEFAULT_IDENTITY_CLAIMS
1434+
.iter()
1435+
.map(|c| (*c).to_owned())
1436+
.collect();
1437+
// `email` is empty, so the next non-empty claim (`preferred_username`) wins.
1438+
let claims = serde_json::from_value(json!({
1439+
"email": "",
1440+
"preferred_username": "jdoe",
1441+
"name": "Jane Doe",
1442+
}))
1443+
.expect("claims map");
1444+
assert_eq!(extract_identity(&claims, &priority), "jdoe");
1445+
1446+
// No matching claim yields an empty identity.
1447+
let empty = serde_json::from_value(json!({"unrelated": "x"})).expect("claims map");
1448+
assert_eq!(extract_identity(&empty, &priority), "");
1449+
}
1450+
1451+
#[test]
1452+
fn build_credential_populates_identity_and_sub() {
1453+
let provider = test_provider();
1454+
let token = valid_token(&make_jwt(&json!({
1455+
"email": "user@example.com",
1456+
"sub": "subject-1",
1457+
})));
1458+
let credential = provider.build_credential("prod", &token);
1459+
assert_eq!(credential.identity, "user@example.com");
1460+
assert_eq!(credential.sub, "subject-1");
1461+
assert_eq!(credential.env, "prod");
1462+
assert_eq!(credential.provider, "test");
1463+
}
1464+
1465+
#[test]
1466+
fn build_credential_leaves_identity_blank_for_opaque_token() {
1467+
let provider = test_provider();
1468+
let token = valid_token("opaque-token");
1469+
let credential = provider.build_credential("prod", &token);
1470+
assert_eq!(credential.identity, "");
1471+
assert_eq!(credential.sub, "");
1472+
}
1473+
1474+
#[test]
1475+
fn with_identity_claims_overrides_selection() {
1476+
let provider = test_provider().with_identity_claims(&["custom_user"]);
1477+
let token = valid_token(&make_jwt(&json!({
1478+
"email": "ignored@example.com",
1479+
"custom_user": "picked",
1480+
})));
1481+
let credential = provider.build_credential("prod", &token);
1482+
assert_eq!(credential.identity, "picked");
1483+
}
13391484
}

src/cli.rs

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -987,7 +987,7 @@ impl Cli {
987987
user_args,
988988
args,
989989
default_fields: &default_fields,
990-
no_auth: command.spec.no_auth,
990+
auth: command.spec.auth,
991991
},
992992
Arc::new(leaf.clone()),
993993
streaming_handler,
@@ -1017,7 +1017,7 @@ impl Cli {
10171017
user_args,
10181018
args,
10191019
default_fields: &default_fields,
1020-
no_auth: command.spec.no_auth,
1020+
auth: command.spec.auth,
10211021
},
10221022
async move |credential| {
10231023
handler(CommandContext {

0 commit comments

Comments
 (0)