diff --git a/AGENTS.md b/AGENTS.md index 8fc7d20b..bd922de6 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -87,6 +87,11 @@ machines whose "disk" is 20 GiB of tmpfs, next to a long tail of small repositor - Open at the application (no credential): `/healthz`, `/readyz`, `/repos.js`, `/repos.mjs`, `/_auth/*` (the sign-in flow itself) and **`/services/public/*`** (data-free; today `install.sh` + `ca.pem`; everything else under it 404; never reads repo data or takes a bearer — test `public_lane_serves_only_the_installer_without_auth`). +- Git authentication help identifies an OIDC provider by `server.auth.provider_name` when set, otherwise by + the configured issuer URL. The name is display-only: no vendor guessing, discovery request or change to + credential validation is part of rendering an error. Static-token mode names no external provider. + Permission denials direct users to the administrator; verifier outages ask them to retry shortly. + Neither includes sign-in or client setup instructions. - **The server answers an invalid/expired credential with a real 401** — that is what makes git `erase` it from its helpers and ask again; the friendly 200 + in-band ERR is reserved for failures a retry cannot fix (account not allowed, verifier down). A 200 leaves git re-storing a dead token for its cache's lifetime. diff --git a/crates/walgit-config/src/lib.rs b/crates/walgit-config/src/lib.rs index e3cdaf15..04e62f98 100644 --- a/crates/walgit-config/src/lib.rs +++ b/crates/walgit-config/src/lib.rs @@ -153,6 +153,9 @@ pub struct AuthConfig { /// (Google, Microsoft Entra, Okta, Auth0, Keycloak, Dex, GitLab, ...). No default: /// `Config::validate` refuses `oidc` mode until it is set. pub issuer: String, + /// Friendly OIDC provider name for client-facing authentication help. Unset: show `issuer`. + /// Display only; does not affect discovery or token validation. + pub provider_name: Option, /// Email domains accepted by `oidc` (the `email` claim, `email_verified` required). pub allowed_domains: Vec, /// Individual identities accepted by `oidc` (exact `email` match). @@ -819,6 +822,7 @@ impl Default for AuthConfig { anonymous_read: true, tokens: vec![], issuer: String::new(), + provider_name: None, allowed_domains: vec![], allowed_emails: vec![], audiences: vec![], @@ -1204,6 +1208,12 @@ impl Config { ); } if a.mode == AuthMode::Oidc { + if let Some(name) = &a.provider_name { + anyhow::ensure!( + !name.trim().is_empty() && !name.chars().any(char::is_control), + "server.auth.provider_name must be nonempty and contain no control characters" + ); + } anyhow::ensure!( !a.anonymous_read, "server.auth.anonymous_read must be false in oidc mode" @@ -1665,6 +1675,42 @@ audiences = ["walgit-cli", "https://git.example.com"] assert_eq!(tok.server.auth.issuer, ""); } + #[test] + fn oidc_provider_name_is_display_only_and_safe_for_terminal_output() { + let mut cfg = Config::parse( + r#" +[server.auth] +mode = "oidc" +issuer = "https://login.example.com" +provider_name = "Acme SSO" +anonymous_read = false +allowed_domains = ["example.com"] +audiences = ["git"] +"#, + ) + .unwrap(); + assert_eq!(cfg.server.auth.provider_name.as_deref(), Some("Acme SSO")); + cfg.apply_env( + [( + "WALGIT__SERVER__AUTH__PROVIDER_NAME".into(), + "Équipe SSO".into(), + )] + .into_iter(), + ) + .unwrap(); + cfg.validate().unwrap(); + assert_eq!(cfg.server.auth.provider_name.as_deref(), Some("Équipe SSO")); + assert_eq!(cfg.server.auth.issuer, "https://login.example.com"); + + for invalid in ["", " ", "Acme\nSSO", "Acme\x1b[31mSSO"] { + cfg.server.auth.provider_name = Some(invalid.into()); + let err = cfg.validate().unwrap_err(); + assert!(err.to_string().contains("provider_name"), "{err}"); + } + cfg.server.auth.provider_name = None; + cfg.validate().unwrap(); + } + #[test] fn events_section_parses_and_validates() { let c = Config::parse( diff --git a/crates/walgit-server/src/smart.rs b/crates/walgit-server/src/smart.rs index ca89d49c..0fbd8198 100644 --- a/crates/walgit-server/src/smart.rs +++ b/crates/walgit-server/src/smart.rs @@ -54,15 +54,12 @@ pub async fn info_refs( // 200 + ERR answer made git keep — and re-`store` — a dead cached token for the cache's // whole lifetime (rig, 2026-08-22: every clone failed for 50 minutes). let has_creds = headers.contains_key(axum::http::header::AUTHORIZATION); - let retry_cannot_help = matches!( - e, - crate::auth::AuthError::Forbidden | crate::auth::AuthError::Unavailable - ); - if is_git_client(headers) && !service_param.is_empty() && has_creds && retry_cannot_help { - return Ok(git_err_response( - &service_param, - &auth_help_message(st, headers, &e), - )); + if is_git_client(headers) + && !service_param.is_empty() + && has_creds + && let Some(message) = auth_help_message(st, headers, &e) + { + return Ok(git_err_response(&service_param, &message)); } return Err(auth_err(e)); } @@ -1443,35 +1440,38 @@ pub(crate) fn request_base_url(st: &AppState, headers: &HeaderMap) -> String { } } -/// One-time client setup text for `base_url` (web UI overview + auth errors). -pub(crate) fn client_setup(st: &AppState, base_url: &str) -> String { - crate::setup::recipes(&st.cfg, base_url, None).setup_text -} - -/// Human-readable instructions for authenticating a git client against this host. +/// Guidance for failures that refreshing credentials cannot fix. Invalid or missing +/// credentials have no in-band message so the caller preserves the HTTP 401 response. pub(crate) fn auth_help_message( st: &AppState, headers: &HeaderMap, e: &crate::auth::AuthError, -) -> String { +) -> Option { let base = request_base_url(st, headers); let host = base .trim_start_matches("https://") .trim_start_matches("http://") .to_string(); - let why = match e { - crate::auth::AuthError::Forbidden => "your identity is not allowed to access this host", - crate::auth::AuthError::Unavailable => { - "the token verifier is temporarily unavailable; retry" - } - _ => "a valid bearer token is required; refresh or replace an expired token", + let auth = &st.cfg.server.auth; + let provider = if auth.mode == walgit_config::AuthMode::Oidc { + let name = auth + .provider_name + .as_deref() + .map_or(auth.issuer.as_str(), str::trim); + format!("Identity provider: {name}.\n") + } else { + String::new() }; - format!( - "walgit: authentication failed: {why}.\n\ - To authenticate git for {host} (Google Identity-Aware Proxy):\n\ - \n{}", - client_setup(st, &base) - ) + match e { + crate::auth::AuthError::Forbidden => Some(format!( + "walgit: permission denied for {host}. Your identity is not allowed to perform this operation.\n\ + Contact the server administrator.\n{provider}" + )), + crate::auth::AuthError::Unavailable => Some(format!( + "walgit: authentication is temporarily unavailable for {host}. Try again shortly.\n{provider}" + )), + crate::auth::AuthError::Invalid | crate::auth::AuthError::Unauthorized => None, + } } /// Message for a repository whose pack set cannot live on this instance. diff --git a/crates/walgit-server/tests/e2e.rs b/crates/walgit-server/tests/e2e.rs index e37eea27..bb1a5342 100644 --- a/crates/walgit-server/tests/e2e.rs +++ b/crates/walgit-server/tests/e2e.rs @@ -2170,6 +2170,203 @@ async fn public_lane_serves_only_the_installer_without_auth() -> TestResult { Ok(()) } +async fn auth_help_server( + mode: walgit_config::AuthMode, + browser_tokens: bool, + provider_name: Option<&str>, +) -> anyhow::Result { + use walgit_config::{AuthMode, StaticToken}; + + Server::start_with_tweak(|c| { + c.server.auth.mode = mode; + c.server.auth.provider_name = provider_name.map(str::to_string); + c.server.auth.anonymous_read = false; + c.server.auth.tokens = vec![StaticToken { + token: "read-only".into(), + principal: "reader".into(), + write: false, + token_env: None, + admin: false, + }]; + if mode == AuthMode::Oidc { + c.server.auth.issuer = "https://login.example.com".into(); + c.server.auth.allowed_domains = vec!["example.com".into()]; + c.server.auth.audiences = vec!["git".into()]; + if browser_tokens { + c.server.auth.oauth_client_id = Some("git".into()); + c.server.auth.oauth_client_secret = Some("client-secret".into()); + c.server.auth.session_secret = Some("0123456789abcdef0123456789abcdef".into()); + } + } + }) + .await +} + +/// Access-denied help names the provider without suggesting that setup fixes permissions. +#[tokio::test(flavor = "multi_thread", worker_threads = 4)] +async fn auth_help_gives_error_specific_guidance() -> TestResult { + use walgit_config::AuthMode; + + for (mode, browser_tokens, provider_name) in [ + (AuthMode::Token, false, Some("Unused provider")), + (AuthMode::Oidc, false, None), + (AuthMode::Oidc, true, None), + (AuthMode::Oidc, true, Some(" Acme SSO ")), + ] { + let server = auth_help_server(mode, browser_tokens, provider_name).await?; + + let url = format!( + "{}/info/refs?service=git-receive-pack", + server.repo_url("t", "r") + ); + let response = reqwest::Client::new() + .get(&url) + .header("User-Agent", "git/2.54.0") + .bearer_auth("read-only") + .send() + .await?; + assert_eq!(response.status(), 200); + assert_eq!( + response.headers()["content-type"], + "application/x-git-receive-pack-advertisement" + ); + let body = response.text().await?; + assert!(body.contains("ERR walgit: permission denied for"), "{body}"); + assert!( + body.contains("not allowed to perform this operation"), + "{body}" + ); + assert!(body.contains("Contact the server administrator"), "{body}"); + assert!(!body.contains("Google Identity-Aware Proxy"), "{body}"); + if mode == AuthMode::Oidc { + let name = provider_name.map_or("https://login.example.com", str::trim); + assert!( + body.contains(&format!("Identity provider: {name}.")), + "{body}" + ); + } else { + assert!(!body.contains("OpenID Connect"), "{body}"); + assert!(!body.contains("Unused provider"), "{body}"); + } + assert!(!body.contains("Sign in"), "{body}"); + assert!(!body.contains("Tokens:"), "{body}"); + assert!(!body.contains("/_auth/tokens"), "{body}"); + assert!(!body.contains("install.sh"), "{body}"); + + // Missing/invalid credentials still get a real 401 so Git can ask its helpers again. + let read_url = format!( + "{}/info/refs?service=git-upload-pack", + server.repo_url("t", "r") + ); + for password in [None, Some("invalid")] { + let mut request = reqwest::Client::new() + .get(&read_url) + .header("User-Agent", "git/2.54.0"); + if let Some(password) = password { + request = request.basic_auth("git", Some(password)); + } + assert_eq!(request.send().await?.status(), 401); + } + } + Ok(()) +} + +/// A correctly signed access token still obeys the OIDC email allowlist. +#[tokio::test(flavor = "multi_thread", worker_threads = 4)] +async fn auth_help_identifies_provider_for_disallowed_oidc_email() -> TestResult { + for provider_name in [None, Some(" Acme SSO ")] { + let server = auth_help_server(walgit_config::AuthMode::Oidc, true, provider_name).await?; + let token = server + .state + .auth + .access_token("reader@other.example") + .unwrap(); + assert!(token.starts_with("wgt_")); + assert!(server.state.auth.access_token_claims(&token).is_some()); + let url = format!( + "{}/info/refs?service=git-upload-pack", + server.repo_url("t", "r") + ); + let client = reqwest::Client::new(); + // A non-Git client confirms this is authorization failure, not an invalid token. + assert_eq!( + client.get(&url).bearer_auth(&token).send().await?.status(), + 403 + ); + let response = client + .get(&url) + .header("User-Agent", "git/2.54.0") + .bearer_auth(&token) + .send() + .await?; + assert_eq!(response.status(), 200); + let body = response.text().await?; + assert!(body.contains("ERR walgit: permission denied for"), "{body}"); + assert!(body.contains("Contact the server administrator."), "{body}"); + let provider = provider_name.map_or("https://login.example.com", str::trim); + assert!( + body.contains(&format!("Identity provider: {provider}.")), + "{body}" + ); + assert!(!body.contains("Sign in"), "{body}"); + assert!(!body.contains("install.sh"), "{body}"); + } + Ok(()) +} + +/// A verifier outage calls for retrying, not replacing credentials or repeating setup. +#[tokio::test(flavor = "multi_thread", worker_threads = 4)] +async fn auth_help_verifier_outage_asks_for_retry() -> TestResult { + let listener = tokio::net::TcpListener::bind("127.0.0.1:0").await?; + let issuer = format!("https://{}", listener.local_addr()?); + let task = tokio::spawn(async move { + // Simulate an unavailable HTTPS issuer by closing each connection before TLS. + while let Ok((connection, _)) = listener.accept().await { + drop(connection); + } + }); + let server = Server::start_with_tweak(|c| { + c.server.auth.mode = walgit_config::AuthMode::Oidc; + c.server.auth.anonymous_read = false; + c.server.auth.issuer = issuer.clone(); + c.server.auth.allowed_domains = vec!["example.com".into()]; + c.server.auth.audiences = vec!["git".into()]; + }) + .await?; + // Syntactically valid RS256 header; discovery fails before signature verification. + let response = reqwest::Client::new() + .get(format!( + "{}/info/refs?service=git-upload-pack", + server.repo_url("t", "r") + )) + .header("User-Agent", "git/2.54.0") + .bearer_auth("eyJhbGciOiJSUzI1NiJ9.e30.AA") + .send() + .await?; + task.abort(); + assert_eq!(response.status(), 200); + let body = response.text().await?; + assert!( + body.contains("ERR walgit: authentication is temporarily unavailable"), + "{body}" + ); + assert!(body.contains("Try again shortly."), "{body}"); + assert!( + body.contains(&format!("Identity provider: {issuer}.")), + "{body}" + ); + for misleading in [ + "Sign in", + "Tokens:", + "install.sh", + "replace", + "permission denied", + ] { + assert!(!body.contains(misleading), "{body}"); + } + Ok(()) +} + /// A stale credential in git's cache (an expired or rotated token) must cost exactly one failed /// command: the server answers the dead token with a real 401, git `erase`s it from its helpers, /// and the next command asks the helpers again (a fresh token) and succeeds. A 200 + in-band ERR diff --git a/walgit.example.toml b/walgit.example.toml index 79584aa9..61f67ae5 100644 --- a/walgit.example.toml +++ b/walgit.example.toml @@ -50,6 +50,8 @@ anonymous_read = true # must be false in oidc mode # admin_emails = [] # oidc: emails that may delete repos or PUT/DELETE settings and policy.json # admin_domains = [] # oidc: email domains that may delete repos or PUT/DELETE settings and policy.json # issuer = "https://id.example.com" # oidc: required, no default. Discovery at /.well-known/openid-configuration +# provider_name = "Acme SSO" # oidc: optional display name in Git auth help; defaults to the issuer URL. +# # Display only (no provider detection or changes to token validation). # allowed_domains = ["example.com"] # oidc: email domains admitted (email_verified required) # allowed_emails = [] # oidc: individual identities admitted # write_domains = ["example.com"] # oidc: omit to let every admitted identity write