From 4344a307e40b3665ca6c0f39aa1f3f27ab828fca Mon Sep 17 00:00:00 2001 From: Drew Newberry Date: Wed, 22 Jul 2026 17:54:48 -0700 Subject: [PATCH 1/2] fix(auth): report gateway authentication status Signed-off-by: Drew Newberry --- .agents/skills/openshell-cli/SKILL.md | 7 +- .agents/skills/openshell-cli/cli-reference.md | 4 +- Cargo.lock | 3 + architecture/gateway.md | 14 + crates/openshell-cli/Cargo.toml | 1 + crates/openshell-cli/src/completers.rs | 8 +- crates/openshell-cli/src/main.rs | 55 +++- crates/openshell-cli/src/oidc_auth.rs | 75 +++-- crates/openshell-cli/src/run.rs | 264 +++++++++++++++--- crates/openshell-cli/src/tls.rs | 17 ++ crates/openshell-sdk/Cargo.toml | 2 + crates/openshell-sdk/README.md | 4 +- crates/openshell-sdk/src/config.rs | 5 +- crates/openshell-sdk/src/oidc.rs | 80 +++++- crates/openshell-server-macros/src/lib.rs | 15 +- crates/openshell-server/src/auth/authz.rs | 38 ++- .../openshell-server/src/auth/method_authz.rs | 19 +- .../src/grpc/authentication.rs | 93 ++++++ crates/openshell-server/src/grpc/mod.rs | 1 + crates/openshell-server/src/multiplex.rs | 35 ++- docs/reference/gateway-auth.mdx | 8 +- docs/sandboxes/manage-gateways.mdx | 10 +- proto/openshell.proto | 30 ++ 23 files changed, 670 insertions(+), 118 deletions(-) create mode 100644 crates/openshell-server/src/grpc/authentication.rs diff --git a/.agents/skills/openshell-cli/SKILL.md b/.agents/skills/openshell-cli/SKILL.md index a97b6a7af2..a36a372df7 100644 --- a/.agents/skills/openshell-cli/SKILL.md +++ b/.agents/skills/openshell-cli/SKILL.md @@ -54,7 +54,10 @@ Use an `http://` endpoint only for trusted local port-forwarding or a protected openshell status ``` -Confirm the gateway is reachable and shows a version. +Confirm the gateway is reachable, authentication is valid or not required, and +the output shows a version. `Status: Connected` only proves the public health +endpoint is reachable; inspect the separate `Authentication` line before +running protected commands. ### Step 3: Create a sandbox @@ -615,7 +618,7 @@ $ openshell sandbox upload --help | Task | Command | |------|---------| | Register local port-forwarded gateway | `openshell gateway add http://127.0.0.1:8080 --local --name local` | -| Check gateway health | `openshell status` | +| Check gateway health and authentication | `openshell status` | | List/switch gateways | `openshell gateway select [name]` | | Connect directly to a gateway | `openshell --gateway-endpoint status` | | Create sandbox (interactive) | `openshell sandbox create` | diff --git a/.agents/skills/openshell-cli/cli-reference.md b/.agents/skills/openshell-cli/cli-reference.md index 1135fe630d..109eda7b54 100644 --- a/.agents/skills/openshell-cli/cli-reference.md +++ b/.agents/skills/openshell-cli/cli-reference.md @@ -181,7 +181,9 @@ package-managed or Helm gateways, use `systemctl`, `journalctl`, `kubectl`, and ### `openshell status` -Show server connectivity and version for the active gateway. +Show server connectivity, authentication status, and version for the active +gateway. Connectivity uses the public health RPC; authentication is checked +separately and can fail while the gateway remains connected. --- diff --git a/Cargo.lock b/Cargo.lock index 268da3982d..21c81cc945 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -3855,6 +3855,7 @@ dependencies = [ "openshell-core", "openshell-policy", "openshell-providers", + "openshell-sdk", "openshell-tui", "owo-colors", "prost-types", @@ -4143,6 +4144,7 @@ dependencies = [ "reqwest 0.12.28", "rustls 0.23.38", "serde", + "serde_json", "thiserror 2.0.18", "tokio", "tokio-rustls 0.26.4", @@ -4151,6 +4153,7 @@ dependencies = [ "tonic", "tower 0.5.3", "tracing", + "wiremock", ] [[package]] diff --git a/architecture/gateway.md b/architecture/gateway.md index cca6599281..35b775230e 100644 --- a/architecture/gateway.md +++ b/architecture/gateway.md @@ -137,6 +137,20 @@ Supported auth modes: | Cloudflare JWT | Edge-authenticated deployments where Cloudflare Access supplies identity. | | OIDC | Bearer-token auth for users, with browser PKCE or client credentials login. | +The CLI persists the scopes requested during OIDC login in gateway metadata and +reuses them when refreshing an access token. This preserves the intended API +resource selection for identity providers that bind access-token audiences to +OAuth scopes. + +Gateway health and user authentication are separate probes. `OpenShell.Health` +remains unauthenticated so deployment and load-balancer health checks do not +depend on user credentials. `Authentication.GetStatus` is side-effect-free and +traverses every configured authentication layer, but deliberately requires no +application role or OAuth scope. The CLI combines both results so a reachable +gateway with an expired or rejected token is reported as connected but +unauthenticated; gateways with no configured user-auth layer report that +authentication is not required. + Sandbox supervisor RPCs authenticate with explicit sandbox credentials; mTLS does not grant sandbox identity. Kubernetes deployments use the gateway-minted JWT bootstrap path: the supervisor starts with a projected diff --git a/crates/openshell-cli/Cargo.toml b/crates/openshell-cli/Cargo.toml index 1cbfd7d474..d7b8fd502c 100644 --- a/crates/openshell-cli/Cargo.toml +++ b/crates/openshell-cli/Cargo.toml @@ -19,6 +19,7 @@ openshell-bootstrap = { path = "../openshell-bootstrap" } openshell-core = { path = "../openshell-core", default-features = false } openshell-policy = { path = "../openshell-policy" } openshell-providers = { path = "../openshell-providers" } +openshell-sdk = { path = "../openshell-sdk" } openshell-tui = { path = "../openshell-tui" } serde = { workspace = true } serde_json = { workspace = true } diff --git a/crates/openshell-cli/src/completers.rs b/crates/openshell-cli/src/completers.rs index 572d601eaa..9d3b88d594 100644 --- a/crates/openshell-cli/src/completers.rs +++ b/crates/openshell-cli/src/completers.rs @@ -140,7 +140,13 @@ async fn completion_grpc_client( Some("oidc") => { if let Some(bundle) = load_oidc_token(gateway_name) { if is_token_expired(&bundle) { - match oidc_refresh_token(&bundle, tls_opts.gateway_insecure).await { + match oidc_refresh_token( + &bundle, + meta.oidc_scopes.as_deref(), + tls_opts.gateway_insecure, + ) + .await + { Ok(refreshed) => { let _ = store_oidc_token(gateway_name, &refreshed); tls_opts.oidc_token = Some(refreshed.access_token); diff --git a/crates/openshell-cli/src/main.rs b/crates/openshell-cli/src/main.rs index df4e983e5b..a44b0eafa8 100644 --- a/crates/openshell-cli/src/main.rs +++ b/crates/openshell-cli/src/main.rs @@ -157,19 +157,30 @@ fn resolve_gateway_name(gateway_flag: &Option) -> Option { /// and setting it on `TlsOptions`. For OIDC, automatically refreshes the token /// if it's near expiry. fn apply_auth(tls: &mut TlsOptions, gateway_name: &str) { - let Some(meta) = get_gateway_metadata(gateway_name) else { - return; - }; + let _ = apply_auth_with_status(tls, gateway_name); +} + +/// Apply stored authentication and return a user-facing preparation failure, +/// if the CLI already knows the credentials cannot be used. +fn apply_auth_with_status(tls: &mut TlsOptions, gateway_name: &str) -> Option { + let meta = get_gateway_metadata(gateway_name)?; match meta.auth_mode.as_deref() { Some("cloudflare_jwt") => { if let Some(token) = load_edge_token(gateway_name) { tls.edge_token = Some(token); + None + } else { + Some(format!( + "edge credentials are missing; run `openshell gateway login {gateway_name}`" + )) } } Some("oidc") => { let Some(bundle) = openshell_bootstrap::oidc_token::load_oidc_token(gateway_name) else { - return; + return Some(format!( + "OIDC credentials are missing; run `openshell gateway login {gateway_name}`" + )); }; if openshell_bootstrap::oidc_token::is_token_expired(&bundle) { let insecure = std::env::var("OPENSHELL_GATEWAY_INSECURE") @@ -178,7 +189,11 @@ fn apply_auth(tls: &mut TlsOptions, gateway_name: &str) { // so the async refresh can run within the sync apply_auth call. match tokio::task::block_in_place(|| { tokio::runtime::Handle::current().block_on( - openshell_cli::oidc_auth::oidc_refresh_token(&bundle, insecure), + openshell_cli::oidc_auth::oidc_refresh_token( + &bundle, + meta.oidc_scopes.as_deref(), + insecure, + ), ) }) { Ok(refreshed) => { @@ -187,19 +202,24 @@ fn apply_auth(tls: &mut TlsOptions, gateway_name: &str) { &refreshed, ); tls.oidc_token = Some(refreshed.access_token); + None } Err(e) => { tracing::warn!("OIDC token refresh failed: {e}"); // Use the expired token anyway — server will reject it // with a clear error prompting re-login. tls.oidc_token = Some(bundle.access_token); + Some(format!( + "OIDC token refresh failed; run `openshell gateway login {gateway_name}`" + )) } } } else { tls.oidc_token = Some(bundle.access_token); + None } } - _ => {} + _ => None, } } @@ -2253,8 +2273,8 @@ async fn main() -> Result<()> { Some(Commands::Status) => { if let Ok(ctx) = resolve_gateway(&cli.gateway, &cli.gateway_endpoint) { let mut tls = tls.with_gateway_name(&ctx.name); - apply_auth(&mut tls, &ctx.name); - run::gateway_status(&ctx.name, &ctx.endpoint, &tls).await?; + let auth_error = apply_auth_with_status(&mut tls, &ctx.name); + run::gateway_status(&ctx.name, &ctx.endpoint, &tls, auth_error.as_deref()).await?; } else { println!("{}", "Gateway Status".cyan().bold()); println!(); @@ -4150,6 +4170,25 @@ mod tests { }); } + #[test] + fn apply_auth_reports_missing_edge_credentials() { + let tmp = tempfile::tempdir().unwrap(); + with_tmp_xdg(tmp.path(), || { + store_gateway_metadata( + "edge-gateway", + &edge_metadata("edge-gateway", "https://gw.example.com"), + ) + .unwrap(); + + let mut tls = TlsOptions::default(); + let error = apply_auth_with_status(&mut tls, "edge-gateway") + .expect("missing credentials should be reported"); + + assert!(error.contains("edge credentials are missing")); + assert!(!tls.is_bearer_auth()); + }); + } + /// Verify the flag names the TUI uses to build its `ProxyCommand` are /// accepted by the `SshProxy` subcommand and land in the right fields. /// This catches drift when CLI flags are renamed or restructured. diff --git a/crates/openshell-cli/src/oidc_auth.rs b/crates/openshell-cli/src/oidc_auth.rs index 379a531127..2aacdb0c9c 100644 --- a/crates/openshell-cli/src/oidc_auth.rs +++ b/crates/openshell-cli/src/oidc_auth.rs @@ -17,9 +17,10 @@ use miette::{IntoDiagnostic, Result}; use oauth2::basic::BasicClient; use oauth2::{ AuthType, AuthUrl, AuthorizationCode, ClientId, ClientSecret, CsrfToken, PkceCodeChallenge, - RedirectUrl, RefreshToken, Scope, TokenResponse, TokenUrl, + RedirectUrl, Scope, TokenResponse, TokenUrl, }; use openshell_bootstrap::oidc_token::OidcTokenBundle; +use openshell_sdk::oidc::RefreshTokenInput; use serde::Deserialize; use std::convert::Infallible; use std::sync::{Arc, Mutex}; @@ -227,10 +228,14 @@ pub async fn oidc_client_credentials_flow( /// Refresh an OIDC token using the `refresh_token` grant. /// +/// Reuses the configured login scopes when supplied so providers can select +/// the same API resource for the refreshed access token. +/// /// Preserves the existing refresh token if the server does not return a new /// one (per OAuth 2.0 spec, the refresh response may omit `refresh_token`). pub async fn oidc_refresh_token( bundle: &OidcTokenBundle, + scopes: Option<&str>, insecure: bool, ) -> Result { let refresh_token = bundle.refresh_token.as_deref().ok_or_else(|| { @@ -239,24 +244,37 @@ pub async fn oidc_refresh_token( ) })?; - let discovery = discover(&bundle.issuer, insecure).await?; - - let client = BasicClient::new(ClientId::new(bundle.client_id.clone())) - .set_token_uri(TokenUrl::new(discovery.token_endpoint).into_diagnostic()?); - - let http = http_client(insecure); - let token_response = client - .exchange_refresh_token(&RefreshToken::new(refresh_token.to_string())) - .request_async(&http) + let scopes = scopes.map_or_else(Vec::new, |scopes| { + scopes.split_whitespace().map(str::to_owned).collect() + }); + let input = RefreshTokenInput::new(refresh_token, &bundle.issuer, &bundle.client_id) + .with_scopes(scopes) + .with_insecure(insecure); + let refreshed = openshell_sdk::oidc::refresh_token(&input) .await - .map_err(|e| miette::miette!("token refresh failed: {e}"))?; + .map_err(miette::Report::new)?; - let mut refreshed = - bundle_from_oauth2_response(&token_response, &bundle.issuer, &bundle.client_id); - if refreshed.refresh_token.is_none() { - refreshed.refresh_token.clone_from(&bundle.refresh_token); + Ok(bundle_from_refresh_output( + bundle, + refreshed.access_token, + refreshed.refresh_token, + refreshed.expires_at, + )) +} + +fn bundle_from_refresh_output( + previous: &OidcTokenBundle, + access_token: String, + refresh_token: Option, + expires_at: Option, +) -> OidcTokenBundle { + OidcTokenBundle { + access_token, + refresh_token: refresh_token.or_else(|| previous.refresh_token.clone()), + expires_at, + issuer: previous.issuer.clone(), + client_id: previous.client_id.clone(), } - Ok(refreshed) } /// Ensure we have a valid OIDC token for the given gateway, refreshing if needed. @@ -279,7 +297,10 @@ pub async fn ensure_valid_oidc_token(gateway_name: &str, insecure: bool) -> Resu gateway = gateway_name, "OIDC token expired, attempting refresh" ); - let refreshed = oidc_refresh_token(&bundle, insecure).await?; + let scopes = openshell_bootstrap::load_gateway_metadata(gateway_name) + .ok() + .and_then(|metadata| metadata.oidc_scopes); + let refreshed = oidc_refresh_token(&bundle, scopes.as_deref(), insecure).await?; openshell_bootstrap::oidc_token::store_oidc_token(gateway_name, &refreshed)?; Ok(refreshed.access_token) } @@ -531,4 +552,24 @@ mod tests { assert_eq!(bundle.client_id, "my-client"); assert!(bundle.expires_at.is_some()); } + + #[test] + fn refresh_output_preserves_omitted_refresh_token() { + let previous = OidcTokenBundle { + access_token: "expired-access".to_string(), + refresh_token: Some("refresh-secret".to_string()), + expires_at: Some(0), + issuer: "https://issuer.example".to_string(), + client_id: "client-id".to_string(), + }; + + let refreshed = + bundle_from_refresh_output(&previous, "refreshed-access".to_string(), None, Some(300)); + + assert_eq!(refreshed.access_token, "refreshed-access"); + assert_eq!(refreshed.refresh_token, previous.refresh_token); + assert_eq!(refreshed.issuer, previous.issuer); + assert_eq!(refreshed.client_id, previous.client_id); + assert_eq!(refreshed.expires_at, Some(300)); + } } diff --git a/crates/openshell-cli/src/run.rs b/crates/openshell-cli/src/run.rs index f9a7035226..a5b0c31755 100644 --- a/crates/openshell-cli/src/run.rs +++ b/crates/openshell-cli/src/run.rs @@ -21,7 +21,7 @@ use crate::commands::common::{ use crate::policy_update::build_policy_update_plan; use crate::tls::{ TlsOptions, build_insecure_rustls_config, build_rustls_config, grpc_client, - grpc_inference_client, require_tls_materials, + grpc_inference_client, grpc_status_clients, require_tls_materials, }; use bytes::Bytes; use dialoguer::{Confirm, Select, theme::ColorfulTheme}; @@ -44,25 +44,26 @@ use openshell_bootstrap::{ use openshell_core::proto::ProviderProfileCategory; use openshell_core::proto::{ ApproveAllDraftChunksRequest, ApproveDraftChunkRequest, AttachSandboxProviderRequest, - ClearDraftChunksRequest, ConfigureProviderRefreshRequest, CreateProviderRequest, - CreateSandboxRequest, CreateSshSessionRequest, DeleteInferenceRouteRequest, - DeleteProviderProfileRequest, DeleteProviderRefreshRequest, DeleteProviderRequest, - DeleteSandboxRequest, DeleteServiceRequest, DetachSandboxProviderRequest, ExecSandboxRequest, - ExposeServiceRequest, GetDraftHistoryRequest, GetDraftPolicyRequest, GetGatewayConfigRequest, - GetGatewayInfoRequest, GetInferenceRouteRequest, GetProviderProfileRequest, - GetProviderRefreshStatusRequest, GetProviderRequest, GetSandboxConfigRequest, - GetSandboxLogsRequest, GetSandboxPolicyStatusRequest, GetSandboxRequest, GetServiceRequest, - GpuResourceRequirements, HealthRequest, ImportProviderProfilesRequest, - LintProviderProfilesRequest, ListProviderProfilesRequest, ListProvidersRequest, - ListSandboxPoliciesRequest, ListSandboxProvidersRequest, ListSandboxesRequest, - ListServicesRequest, PolicySource, PolicyStatus, Provider, ProviderCredentialRefreshStatus, - ProviderCredentialRefreshStrategy, ProviderProfile, ProviderProfileDiagnostic, - ProviderProfileImportItem, RejectDraftChunkRequest, ResourceRequirements, - RevokeSshSessionRequest, RotateProviderCredentialRequest, Sandbox, SandboxPhase, SandboxPolicy, - SandboxSpec, SandboxTemplate, ServiceEndpointResponse, ServiceStatus, SetInferenceRouteRequest, - SettingScope, TcpForwardFrame, TcpForwardInit, TcpRelayTarget, UpdateConfigRequest, - UpdateProviderProfilesRequest, UpdateProviderRequest, WatchSandboxRequest, exec_sandbox_event, - setting_value, tcp_forward_init, + AuthenticationProvider, ClearDraftChunksRequest, ConfigureProviderRefreshRequest, + CreateProviderRequest, CreateSandboxRequest, CreateSshSessionRequest, + DeleteInferenceRouteRequest, DeleteProviderProfileRequest, DeleteProviderRefreshRequest, + DeleteProviderRequest, DeleteSandboxRequest, DeleteServiceRequest, + DetachSandboxProviderRequest, ExecSandboxRequest, ExposeServiceRequest, + GetAuthenticationStatusRequest, GetAuthenticationStatusResponse, GetDraftHistoryRequest, + GetDraftPolicyRequest, GetGatewayConfigRequest, GetGatewayInfoRequest, + GetInferenceRouteRequest, GetProviderProfileRequest, GetProviderRefreshStatusRequest, + GetProviderRequest, GetSandboxConfigRequest, GetSandboxLogsRequest, + GetSandboxPolicyStatusRequest, GetSandboxRequest, GetServiceRequest, GpuResourceRequirements, + HealthRequest, ImportProviderProfilesRequest, LintProviderProfilesRequest, + ListProviderProfilesRequest, ListProvidersRequest, ListSandboxPoliciesRequest, + ListSandboxProvidersRequest, ListSandboxesRequest, ListServicesRequest, PolicySource, + PolicyStatus, Provider, ProviderCredentialRefreshStatus, ProviderCredentialRefreshStrategy, + ProviderProfile, ProviderProfileDiagnostic, ProviderProfileImportItem, RejectDraftChunkRequest, + ResourceRequirements, RevokeSshSessionRequest, RotateProviderCredentialRequest, Sandbox, + SandboxPhase, SandboxPolicy, SandboxSpec, SandboxTemplate, ServiceEndpointResponse, + ServiceStatus, SetInferenceRouteRequest, SettingScope, TcpForwardFrame, TcpForwardInit, + TcpRelayTarget, UpdateConfigRequest, UpdateProviderProfilesRequest, UpdateProviderRequest, + WatchSandboxRequest, exec_sandbox_event, setting_value, tcp_forward_init, }; use openshell_core::settings; use openshell_core::{ObjectId, ObjectName, ObjectWorkspace}; @@ -124,24 +125,46 @@ struct ComputeDriverCapabilitiesView { /// Show gateway status. #[allow(clippy::branches_sharing_code)] -pub async fn gateway_status(gateway_name: &str, server: &str, tls: &TlsOptions) -> Result<()> { +pub async fn gateway_status( + gateway_name: &str, + server: &str, + tls: &TlsOptions, + auth_preparation_error: Option<&str>, +) -> Result<()> { println!("{}", "Server Status".cyan().bold()); println!(); println!(" {} {}", "Gateway:".dimmed(), gateway_name); println!(" {} {}", "Server:".dimmed(), server); - if tls.is_bearer_auth() { - println!(" {} Edge (bearer token)", "Auth:".dimmed()); - } // Try to connect and get health - match grpc_client(server, tls).await { - Ok(mut client) => match client.health(HealthRequest {}).await { + match grpc_status_clients(server, tls).await { + Ok((mut client, mut authentication)) => match client.health(HealthRequest {}).await { Ok(response) => { let health = response.into_inner(); println!(" {} {}", "Status:".dimmed(), "Connected".green()); + let authentication = match auth_preparation_error { + Some(error) => GatewayAuthenticationState::Failed(error.to_string()), + None => gateway_authentication_state( + authentication + .get_status(GetAuthenticationStatusRequest {}) + .await + .map(tonic::Response::into_inner), + tls, + server, + ), + }; + print_gateway_authentication_state(&authentication); println!(" {} {}", "Version:".dimmed(), health.version); } Err(e) => { + let authentication = auth_preparation_error.map_or_else( + || { + GatewayAuthenticationState::Unverified( + "gRPC health check failed".to_string(), + ) + }, + |error| GatewayAuthenticationState::Failed(error.to_string()), + ); if let Some(status) = http_health_check(server, tls).await? { if status.is_success() { println!(" {} {}", "Status:".dimmed(), "Connected (HTTP)".yellow()); @@ -156,9 +179,14 @@ pub async fn gateway_status(gateway_name: &str, server: &str, tls: &TlsOptions) println!(" {} {}", "Status:".dimmed(), "Error".red()); println!(" {} {}", "Error:".dimmed(), e); } + print_gateway_authentication_state(&authentication); } }, Err(e) => { + let authentication = auth_preparation_error.map_or_else( + || GatewayAuthenticationState::Unverified("gateway unreachable".to_string()), + |error| GatewayAuthenticationState::Failed(error.to_string()), + ); if let Some(status) = http_health_check(server, tls).await? { if status.is_success() { println!(" {} {}", "Status:".dimmed(), "Connected (HTTP)".yellow()); @@ -173,12 +201,108 @@ pub async fn gateway_status(gateway_name: &str, server: &str, tls: &TlsOptions) println!(" {} {}", "Status:".dimmed(), "Disconnected".red()); println!(" {} {}", "Error:".dimmed(), e); } + print_gateway_authentication_state(&authentication); } } Ok(()) } +#[derive(Debug, Clone, PartialEq, Eq)] +enum GatewayAuthenticationState { + Authenticated(&'static str), + NotRequired(&'static str), + NotEnforced, + Failed(String), + Unverified(String), +} + +fn gateway_authentication_state( + result: std::result::Result, + tls: &TlsOptions, + server: &str, +) -> GatewayAuthenticationState { + match result { + Ok(response) if response.authenticated => { + let provider = match response.provider() { + AuthenticationProvider::Oidc => "OIDC", + AuthenticationProvider::Mtls => "mTLS", + AuthenticationProvider::CloudflareAccess => "edge", + AuthenticationProvider::LocalDev => "local development", + AuthenticationProvider::Unspecified => "gateway", + }; + GatewayAuthenticationState::Authenticated(provider) + } + Ok(response) if response.provider() == AuthenticationProvider::LocalDev => { + GatewayAuthenticationState::NotRequired("local development") + } + Ok(_) if tls.edge_token.is_some() => GatewayAuthenticationState::Authenticated("edge"), + Ok(_) if tls.oidc_token.is_some() || server.starts_with("https://") => { + GatewayAuthenticationState::NotEnforced + } + Ok(_) => GatewayAuthenticationState::NotRequired("gateway"), + Err(status) if status.code() == Code::Unauthenticated => { + GatewayAuthenticationState::Failed(status.message().to_string()) + } + Err(status) if status.code() == Code::PermissionDenied => { + GatewayAuthenticationState::Authenticated("authorization denied") + } + Err(status) if status.code() == Code::Unimplemented && tls.edge_token.is_some() => { + GatewayAuthenticationState::Authenticated("edge") + } + Err(status) + if status.code() == Code::Unimplemented + && !tls.is_bearer_auth() + && server.starts_with("http://") => + { + GatewayAuthenticationState::NotRequired("gateway") + } + Err(status) + if status.code() == Code::Unimplemented + && !tls.is_bearer_auth() + && server.starts_with("https://") => + { + GatewayAuthenticationState::Authenticated("mTLS transport") + } + Err(status) if status.code() == Code::Unimplemented => { + GatewayAuthenticationState::Unverified( + "gateway does not support authentication checks".to_string(), + ) + } + Err(status) => GatewayAuthenticationState::Unverified(status.message().to_string()), + } +} + +fn print_gateway_authentication_state(state: &GatewayAuthenticationState) { + match state { + GatewayAuthenticationState::Authenticated(provider) => println!( + " {} {} ({provider})", + "Authentication:".dimmed(), + "Authenticated".green() + ), + GatewayAuthenticationState::NotRequired(mode) => println!( + " {} {} ({mode})", + "Authentication:".dimmed(), + "Not required".green() + ), + GatewayAuthenticationState::NotEnforced => println!( + " {} {}", + "Authentication:".dimmed(), + "Not enforced by gateway".yellow() + ), + GatewayAuthenticationState::Failed(error) => println!( + " {} {} ({error})", + "Authentication:".dimmed(), + "Failed".red() + ), + GatewayAuthenticationState::Unverified(reason) => println!( + " {} {} ({reason})", + "Authentication:".dimmed(), + "Unverified".yellow() + ), + } +} + fn gateway_service_status_name(status: i32) -> &'static str { match ServiceStatus::try_from(status) { Ok(ServiceStatus::Healthy) => "healthy", @@ -7993,11 +8117,12 @@ fn format_endpoint(endpoint: &openshell_core::proto::NetworkEndpoint) -> String #[cfg(test)] mod tests { use super::{ - ComputeDriverCapabilitiesView, ComputeDriverInfoView, GatewayInfoView, PolicyGetView, - ProvisioningStep, TlsOptions, build_sandbox_resource_limits, - dockerfile_sources_supported_for_gateway, format_endpoint, format_gateway_select_header, - format_gateway_select_items, format_provider_attachment_table, gateway_add, - gateway_auth_label, gateway_env_override_warning, gateway_info_to_json, + ComputeDriverCapabilitiesView, ComputeDriverInfoView, GatewayAuthenticationState, + GatewayInfoView, PolicyGetView, ProvisioningStep, TlsOptions, + build_sandbox_resource_limits, dockerfile_sources_supported_for_gateway, format_endpoint, + format_gateway_select_header, format_gateway_select_items, + format_provider_attachment_table, gateway_add, gateway_auth_label, + gateway_authentication_state, gateway_env_override_warning, gateway_info_to_json, gateway_remote_label, gateway_select_with, gateway_to_json, gateway_type_label, git_sync_files, http_health_check, import_local_package_mtls_bundle, inferred_provider_type, mtls_certs_exist_for_gateway, package_managed_tls_dirs, @@ -8029,13 +8154,80 @@ mod tests { PROGRESS_STEP_STARTING_SANDBOX, }; use openshell_core::proto::{ - GpuResourceRequirements, PolicyStatus, Provider, ProviderCredentialRefresh, - ProviderCredentialRefreshStatus, ProviderCredentialRefreshStrategy, - ProviderCredentialTokenGrant, ProviderProfile, ProviderProfileCredential, - ResourceRequirements, SandboxCondition, SandboxPolicyRevision, SandboxStatus, - datamodel::v1::ObjectMeta, + AuthenticationProvider, GetAuthenticationStatusResponse, GpuResourceRequirements, + PolicyStatus, Provider, ProviderCredentialRefresh, ProviderCredentialRefreshStatus, + ProviderCredentialRefreshStrategy, ProviderCredentialTokenGrant, ProviderProfile, + ProviderProfileCredential, ResourceRequirements, SandboxCondition, SandboxPolicyRevision, + SandboxStatus, datamodel::v1::ObjectMeta, }; + #[test] + fn gateway_status_reports_authenticated_oidc_principal() { + let mut tls = TlsOptions::default(); + tls.oidc_token = Some("token".to_string()); + + let state = gateway_authentication_state( + Ok(GetAuthenticationStatusResponse { + authenticated: true, + provider: AuthenticationProvider::Oidc.into(), + }), + &tls, + "https://gateway.example.com", + ); + + assert_eq!(state, GatewayAuthenticationState::Authenticated("OIDC")); + } + + #[test] + fn gateway_status_reports_rejected_bearer_token() { + let mut tls = TlsOptions::default(); + tls.oidc_token = Some("expired".to_string()); + + let state = gateway_authentication_state( + Err(Status::unauthenticated("invalid token: ExpiredSignature")), + &tls, + "https://gateway.example.com", + ); + + assert_eq!( + state, + GatewayAuthenticationState::Failed("invalid token: ExpiredSignature".to_string()) + ); + } + + #[test] + fn gateway_status_marks_oidc_unverified_on_older_gateway() { + let mut tls = TlsOptions::default(); + tls.oidc_token = Some("token".to_string()); + + let state = gateway_authentication_state( + Err(Status::unimplemented("unknown service")), + &tls, + "https://gateway.example.com", + ); + + assert_eq!( + state, + GatewayAuthenticationState::Unverified( + "gateway does not support authentication checks".to_string() + ) + ); + } + + #[test] + fn gateway_status_reports_local_plaintext_auth_not_required() { + let state = gateway_authentication_state( + Ok(GetAuthenticationStatusResponse { + authenticated: false, + provider: AuthenticationProvider::Unspecified.into(), + }), + &TlsOptions::default(), + "http://127.0.0.1:8080", + ); + + assert_eq!(state, GatewayAuthenticationState::NotRequired("gateway")); + } + #[test] fn policy_revision_json_includes_revision_provenance() { let revision = SandboxPolicyRevision { diff --git a/crates/openshell-cli/src/tls.rs b/crates/openshell-cli/src/tls.rs index 10df401a5b..7d47f6d442 100644 --- a/crates/openshell-cli/src/tls.rs +++ b/crates/openshell-cli/src/tls.rs @@ -3,6 +3,7 @@ use miette::{IntoDiagnostic, Result, WrapErr}; use openshell_core::auth::EdgeAuthInterceptor; +use openshell_core::proto::authentication_client::AuthenticationClient; use openshell_core::proto::inference_client::InferenceClient; use openshell_core::proto::open_shell_client::OpenShellClient; use rustls::{ @@ -24,6 +25,9 @@ use tracing::debug; /// Concrete gRPC client type used by all commands. pub type GrpcClient = OpenShellClient>; +/// Concrete authentication status client type. +pub type GrpcAuthenticationClient = + AuthenticationClient>; /// Concrete inference client type. pub type GrpcInferenceClient = InferenceClient>; @@ -440,6 +444,19 @@ pub async fn grpc_client(server: &str, tls: &TlsOptions) -> Result { Ok(OpenShellClient::with_interceptor(channel, interceptor)) } +/// Build `OpenShell` and authentication clients over one shared channel. +pub async fn grpc_status_clients( + server: &str, + tls: &TlsOptions, +) -> Result<(GrpcClient, GrpcAuthenticationClient)> { + let channel = build_channel(server, tls).await?; + let interceptor = interceptor_from_tls(tls)?; + Ok(( + OpenShellClient::with_interceptor(channel.clone(), interceptor.clone()), + AuthenticationClient::with_interceptor(channel, interceptor), + )) +} + fn interceptor_from_tls(tls: &TlsOptions) -> Result { EdgeAuthInterceptor::new(tls.oidc_token.as_deref(), tls.edge_token.as_deref()) } diff --git a/crates/openshell-sdk/Cargo.toml b/crates/openshell-sdk/Cargo.toml index 364cfd8d56..a1016baec0 100644 --- a/crates/openshell-sdk/Cargo.toml +++ b/crates/openshell-sdk/Cargo.toml @@ -30,8 +30,10 @@ tower = { workspace = true } tracing = { workspace = true } [dev-dependencies] +serde_json = { workspace = true } tokio = { workspace = true, features = ["macros", "rt-multi-thread"] } tokio-stream = { workspace = true } +wiremock = "0.6" [lints] workspace = true diff --git a/crates/openshell-sdk/README.md b/crates/openshell-sdk/README.md index c7039fe9cb..93afc5359e 100644 --- a/crates/openshell-sdk/README.md +++ b/crates/openshell-sdk/README.md @@ -26,7 +26,9 @@ client bound to the current token. When a refresher is wired, use `force_refresh` to recover after a raw RPC returns `Unauthenticated`. The SDK consumes a `Refresh` trait that the caller implements; it does not run -the OIDC browser flow itself. +the OIDC browser flow itself. Its non-interactive refresh-token exchange also +accepts scopes that identity providers may require to select the API resource +for the refreshed access token. ## Transport modes diff --git a/crates/openshell-sdk/src/config.rs b/crates/openshell-sdk/src/config.rs index 94cfc28c0b..27a7411734 100644 --- a/crates/openshell-sdk/src/config.rs +++ b/crates/openshell-sdk/src/config.rs @@ -4,8 +4,9 @@ //! Public input types for the SDK: how callers describe a gateway and the //! credentials used to talk to it. //! -//! The CLI keeps its own filesystem-aware `TlsOptions` for plumbing; it -//! converts to a `ClientConfig` at the moment of dialing the gateway. +//! The CLI keeps its own filesystem-aware `TlsOptions` for transport plumbing +//! and reuses the SDK's OIDC refresh primitive. Other consumers describe the +//! complete transport with [`ClientConfig`]. use crate::refresh::Refresh; use std::sync::Arc; diff --git a/crates/openshell-sdk/src/oidc.rs b/crates/openshell-sdk/src/oidc.rs index 3f9366ad5e..3f59c25cd0 100644 --- a/crates/openshell-sdk/src/oidc.rs +++ b/crates/openshell-sdk/src/oidc.rs @@ -8,7 +8,7 @@ use crate::error::{Result, SdkError}; use oauth2::basic::BasicClient; -use oauth2::{ClientId, RefreshToken, TokenResponse, TokenUrl}; +use oauth2::{ClientId, RefreshToken, Scope, TokenResponse, TokenUrl}; use serde::Deserialize; /// OIDC discovery document (subset of fields callers consume). @@ -30,6 +30,9 @@ pub struct RefreshTokenInput { pub refresh_token: String, pub issuer: String, pub client_id: String, + /// Scopes to resend with the refresh request. Some identity providers use + /// these to select the API resource for the refreshed access token. + pub scopes: Vec, pub insecure: bool, } @@ -39,6 +42,7 @@ impl std::fmt::Debug for RefreshTokenInput { f.debug_struct("RefreshTokenInput") .field("issuer", &self.issuer) .field("client_id", &self.client_id) + .field("scopes", &self.scopes) .field("insecure", &self.insecure) .finish_non_exhaustive() } @@ -54,10 +58,22 @@ impl RefreshTokenInput { refresh_token: refresh_token.into(), issuer: issuer.into(), client_id: client_id.into(), + scopes: Vec::new(), insecure: false, } } + /// Set the scopes to resend with the refresh-token grant. + #[must_use] + pub fn with_scopes(mut self, scopes: I) -> Self + where + I: IntoIterator, + S: Into, + { + self.scopes = scopes.into_iter().map(Into::into).collect(); + self + } + #[must_use] pub fn with_insecure(mut self, insecure: bool) -> Self { self.insecure = insecure; @@ -132,9 +148,11 @@ pub fn http_client(insecure: bool) -> reqwest::Client { /// Refresh an OIDC access token using the `refresh_token` grant. /// -/// The caller is responsible for preserving the prior refresh token when -/// the output's `refresh_token` is `None` — per OAuth 2.0 the server may -/// omit it from the refresh response. +/// The request resends any configured scopes so providers that use scopes to +/// select an API resource mint the correct access token. The caller is +/// responsible for preserving the prior refresh token when the output's +/// `refresh_token` is `None` — per OAuth 2.0 the server may omit it from the +/// refresh response. pub async fn refresh_token(input: &RefreshTokenInput) -> Result { let discovery = discover(&input.issuer, input.insecure).await?; @@ -143,9 +161,14 @@ pub async fn refresh_token(input: &RefreshTokenInput) -> Result Refr #[cfg(test)] mod tests { use super::*; + use wiremock::matchers::{body_string_contains, method, path}; + use wiremock::{Mock, MockServer, ResponseTemplate}; #[test] fn debug_redacts_tokens() { - let input = RefreshTokenInput::new("refresh-secret", "https://idp", "cli"); + let input = RefreshTokenInput::new("refresh-secret", "https://idp", "cli") + .with_scopes(["openid", "api://cli/access_as_user"]); let rendered = format!("{input:?}"); assert!(!rendered.contains("refresh-secret")); assert!(rendered.contains("cli")); @@ -187,4 +213,44 @@ mod tests { assert!(!rendered.contains("refresh-secret")); assert!(rendered.contains("has_refresh_token")); } + + #[tokio::test] + async fn refresh_sends_configured_scopes() { + let server = MockServer::start().await; + let issuer = server.uri(); + + Mock::given(method("GET")) + .and(path("/.well-known/openid-configuration")) + .respond_with(ResponseTemplate::new(200).set_body_json(serde_json::json!({ + "issuer": issuer, + "authorization_endpoint": format!("{}/authorize", server.uri()), + "token_endpoint": format!("{}/token", server.uri()), + }))) + .expect(1) + .mount(&server) + .await; + + Mock::given(method("POST")) + .and(path("/token")) + .and(body_string_contains("scope=")) + .and(body_string_contains("access_as_user")) + .respond_with(ResponseTemplate::new(200).set_body_json(serde_json::json!({ + "access_token": "refreshed-access", + "token_type": "bearer", + "expires_in": 300, + }))) + .expect(1) + .mount(&server) + .await; + + let input = RefreshTokenInput::new("refresh-secret", &issuer, "client-id") + .with_scopes(["openid", "api://client-id/access_as_user"]); + let refreshed = refresh_token(&input) + .await + .expect("configured scopes should be sent on refresh"); + + assert_eq!(refreshed.access_token, "refreshed-access"); + assert!(refreshed.refresh_token.is_none()); + assert!(refreshed.expires_at.is_some()); + } } diff --git a/crates/openshell-server-macros/src/lib.rs b/crates/openshell-server-macros/src/lib.rs index a698ae6623..fc0632638d 100644 --- a/crates/openshell-server-macros/src/lib.rs +++ b/crates/openshell-server-macros/src/lib.rs @@ -5,8 +5,9 @@ //! //! `#[rpc_authz(service = "...")]` is applied to a tonic service `impl` //! block. Each method inside the impl carries an `#[rpc_auth(...)]` -//! attribute describing its auth mode, optional Bearer scope, and -//! required role. The macro emits a const `&[MethodAuth]` adjacent to +//! attribute describing its auth mode and optional Bearer scope/role pair. +//! Omitting both on a Bearer method makes it authentication-only. The macro +//! emits a const `&[MethodAuth]` adjacent to //! the impl and re-emits the impl block with the per-method //! `#[rpc_auth]` attributes stripped so other macros (notably //! `#[tonic::async_trait]`) see a clean impl. @@ -131,16 +132,10 @@ impl RpcAuth { } } AuthMode::Bearer | AuthMode::Dual => { - if scope.is_none() { + if scope.is_some() != role.is_some() { return Err(Error::new( span, - "`auth = \"bearer\"` and `auth = \"dual\"` require `scope = \"...\"`", - )); - } - if role.is_none() { - return Err(Error::new( - span, - "`auth = \"bearer\"` and `auth = \"dual\"` require `role = \"...\"`", + "`auth = \"bearer\"` and `auth = \"dual\"` require both `scope` and `role`, or neither for authentication-only methods", )); } } diff --git a/crates/openshell-server/src/auth/authz.rs b/crates/openshell-server/src/auth/authz.rs index 1c04b09766..e349e1abeb 100644 --- a/crates/openshell-server/src/auth/authz.rs +++ b/crates/openshell-server/src/auth/authz.rs @@ -64,17 +64,23 @@ impl AuthzPolicy { /// (authentication-only mode for providers like GitHub). #[allow(clippy::result_large_err)] pub fn check(&self, identity: &Identity, method: &str) -> Result<(), Status> { - let required = match method_authz::required_role(method) { - Some(Role::Admin) => &self.admin_role, + let required = method_authz::lookup(method).map_or( // Default to user role for unknown methods, matching the // pre-annotation behavior. The exhaustiveness test ensures // every real RPC has an explicit declaration. - Some(Role::User) | None => &self.user_role, - }; + Some(&self.user_role), + |method| match method.role { + Some(Role::Admin) => Some(&self.admin_role), + Some(Role::User) => Some(&self.user_role), + None => None, + }, + ); // Empty role name = skip role check for this level (auth-only mode). // Scope enforcement still applies if enabled. - if !required.is_empty() { + if let Some(required) = required + && !required.is_empty() + { // Admin role implicitly satisfies user role requirements. let has_role = identity.roles.iter().any(|r| r == required) || (!self.admin_role.is_empty() @@ -108,7 +114,15 @@ impl AuthzPolicy { return Ok(()); } - let required_scope = method_authz::required_scope(method).unwrap_or(SCOPE_ALL); + let required_scope = match method_authz::lookup(method) { + Some(method) => { + let Some(scope) = method.scope else { + return Ok(()); + }; + scope + } + None => SCOPE_ALL, + }; if identity.scopes.iter().any(|s| s == required_scope) { return Ok(()); @@ -312,6 +326,18 @@ mod tests { ); } + #[test] + fn authentication_status_requires_no_role_or_scope() { + let id = identity_with_roles_and_scopes(&[], &[]); + let policy = scoped_policy(); + + assert!( + policy + .check(&id, "/openshell.v1.Authentication/GetStatus") + .is_ok() + ); + } + #[test] fn scoped_access_allowed() { let id = diff --git a/crates/openshell-server/src/auth/method_authz.rs b/crates/openshell-server/src/auth/method_authz.rs index ec8dc5bca3..5937e6f37b 100644 --- a/crates/openshell-server/src/auth/method_authz.rs +++ b/crates/openshell-server/src/auth/method_authz.rs @@ -54,7 +54,11 @@ pub enum Role { /// Add a new service by appending its module's `AUTH_METADATA` const here. /// The constant name is fixed by `#[rpc_authz]`; service disambiguation /// comes from the module path. -const SERVICES: &[&[MethodAuth]] = &[crate::grpc::AUTH_METADATA, crate::inference::AUTH_METADATA]; +const SERVICES: &[&[MethodAuth]] = &[ + crate::grpc::AUTH_METADATA, + crate::grpc::authentication::AUTH_METADATA, + crate::inference::AUTH_METADATA, +]; /// Find the auth metadata for `method`, if any. #[must_use] @@ -73,19 +77,6 @@ pub fn all_paths() -> impl Iterator { SERVICES.iter().flat_map(|s| s.iter()).map(|m| m.path) } -/// Required Bearer scope for the method, or `None` if scopes don't -/// apply (`unauthenticated`, `sandbox`). -#[must_use] -pub fn required_scope(method: &str) -> Option<&'static str> { - lookup(method).and_then(|m| m.scope) -} - -/// Required role for the method on the Bearer path. -#[must_use] -pub fn required_role(method: &str) -> Option { - lookup(method).and_then(|m| m.role) -} - /// `true` if the method bypasses authentication entirely. /// /// Note: this checks the per-method tables only. The prefix-based diff --git a/crates/openshell-server/src/grpc/authentication.rs b/crates/openshell-server/src/grpc/authentication.rs new file mode 100644 index 0000000000..7b8a31ec3d --- /dev/null +++ b/crates/openshell-server/src/grpc/authentication.rs @@ -0,0 +1,93 @@ +// SPDX-FileCopyrightText: Copyright (c) 2025-2026 NVIDIA CORPORATION & AFFILIATES. All rights reserved. +// SPDX-License-Identifier: Apache-2.0 + +//! Side-effect-free gateway authentication status RPC. + +use crate::auth::identity::IdentityProvider; +use crate::auth::principal::Principal; +use openshell_core::proto::{ + AuthenticationProvider, GetAuthenticationStatusRequest, GetAuthenticationStatusResponse, + authentication_server::Authentication, +}; +use openshell_server_macros::rpc_authz; +use tonic::{Request, Response, Status}; + +#[derive(Debug, Clone, Copy, Default)] +pub struct AuthenticationService; + +#[rpc_authz(service = "openshell.v1.Authentication")] +#[tonic::async_trait] +impl Authentication for AuthenticationService { + #[rpc_auth(auth = "bearer")] + async fn get_status( + &self, + request: Request, + ) -> Result, Status> { + let (authenticated, provider) = match request.extensions().get::() { + Some(Principal::User(user)) => { + let provider = match user.identity.provider { + IdentityProvider::Oidc => AuthenticationProvider::Oidc, + IdentityProvider::Mtls => AuthenticationProvider::Mtls, + IdentityProvider::CloudflareAccess => AuthenticationProvider::CloudflareAccess, + IdentityProvider::LocalDev => AuthenticationProvider::LocalDev, + }; + ( + user.identity.provider != IdentityProvider::LocalDev, + provider, + ) + } + Some(Principal::Sandbox(_) | Principal::Anonymous) | None => { + (false, AuthenticationProvider::Unspecified) + } + }; + + Ok(Response::new(GetAuthenticationStatusResponse { + authenticated, + provider: provider.into(), + })) + } +} + +#[cfg(test)] +mod tests { + use super::*; + use crate::auth::identity::Identity; + use crate::auth::principal::UserPrincipal; + + #[tokio::test] + async fn reports_authenticated_oidc_principal() { + let mut request = Request::new(GetAuthenticationStatusRequest {}); + request + .extensions_mut() + .insert(Principal::User(UserPrincipal { + identity: Identity { + subject: "alice".to_string(), + display_name: None, + roles: Vec::new(), + scopes: Vec::new(), + provider: IdentityProvider::Oidc, + }, + })); + + let response = AuthenticationService + .get_status(request) + .await + .expect("authentication status") + .into_inner(); + + assert!(response.authenticated); + assert_eq!(response.provider(), AuthenticationProvider::Oidc); + } + + #[tokio::test] + async fn reports_no_application_principal() { + let response = AuthenticationService + .get_status(Request::new(GetAuthenticationStatusRequest {})) + .await + .expect("authentication status") + .into_inner(); + + assert!(!response.authenticated); + assert_eq!(response.provider(), AuthenticationProvider::Unspecified); + } +} diff --git a/crates/openshell-server/src/grpc/mod.rs b/crates/openshell-server/src/grpc/mod.rs index ecd96ea90d..f8d4beefd1 100644 --- a/crates/openshell-server/src/grpc/mod.rs +++ b/crates/openshell-server/src/grpc/mod.rs @@ -4,6 +4,7 @@ //! gRPC service implementation. mod auth_rpc; +pub mod authentication; pub mod policy; pub mod provider; mod sandbox; diff --git a/crates/openshell-server/src/multiplex.rs b/crates/openshell-server/src/multiplex.rs index 83e27d27cb..414219c742 100644 --- a/crates/openshell-server/src/multiplex.rs +++ b/crates/openshell-server/src/multiplex.rs @@ -19,7 +19,8 @@ use hyper_util::{ use metrics::{counter, histogram}; use openshell_core::Config; use openshell_core::proto::{ - inference_server::InferenceServer, open_shell_server::OpenShellServer, + authentication_server::AuthenticationServer, inference_server::InferenceServer, + open_shell_server::OpenShellServer, }; use openshell_gateway_interceptors::{EvaluationContext, GatewayInterceptorRuntime}; use std::collections::BTreeMap; @@ -41,6 +42,7 @@ use crate::{ auth::identity::Identity, auth::oidc::{self, OidcAuthenticator}, auth::principal::{Principal, UserPrincipal}, + grpc::authentication::AuthenticationService, http_router, inference::InferenceService, service_http_router, @@ -162,6 +164,8 @@ impl MultiplexService { GatewayInterceptorGrpcService::new(openshell, self.state.gateway_interceptors.clone()); let inference = InferenceServer::new(InferenceService::new(self.state.clone())) .max_decoding_message_size(MAX_GRPC_DECODE_SIZE); + let authentication = AuthenticationServer::new(AuthenticationService) + .max_decoding_message_size(MAX_GRPC_DECODE_SIZE); let authz_policy = self.state.config.oidc.as_ref().map(|oidc| AuthzPolicy { admin_role: oidc.admin_role.clone(), user_role: oidc.user_role.clone(), @@ -169,7 +173,7 @@ impl MultiplexService { }); let authenticator_chain = build_authenticator_chain(&self.state); let grpc_service = AuthGrpcRouter::with_peer_identity( - GrpcRouter::new(openshell, inference), + GrpcRouter::new(openshell, authentication, inference), authenticator_chain, authz_policy, self.state @@ -638,31 +642,39 @@ where } } -/// Combined gRPC service that routes between `OpenShell` and Inference services -/// based on the request path prefix. +/// Combined gRPC service that routes between the `OpenShell`, `Authentication`, +/// and `Inference` services based on the request path prefix. #[derive(Clone)] -pub struct GrpcRouter { +pub struct GrpcRouter { openshell: N, + authentication: A, inference: I, } -impl GrpcRouter { - fn new(openshell: N, inference: I) -> Self { +impl GrpcRouter { + fn new(openshell: N, authentication: A, inference: I) -> Self { Self { openshell, + authentication, inference, } } } +const AUTHENTICATION_PATH_PREFIX: &str = "/openshell.v1.Authentication/"; const INFERENCE_PATH_PREFIX: &str = "/openshell.inference.v1.Inference/"; -impl tower::Service> for GrpcRouter +impl tower::Service> for GrpcRouter where N: tower::Service> + Clone + Send + 'static, N::Response: Send, N::Future: Send, N::Error: Send, + A: tower::Service, Response = N::Response, Error = N::Error> + + Clone + + Send + + 'static, + A::Future: Send, I: tower::Service, Response = N::Response, Error = N::Error> + Clone + Send @@ -679,9 +691,12 @@ where } fn call(&mut self, req: Request) -> Self::Future { - let is_inference = req.uri().path().starts_with(INFERENCE_PATH_PREFIX); + let path = req.uri().path(); - if is_inference { + if path.starts_with(AUTHENTICATION_PATH_PREFIX) { + let mut svc = self.authentication.clone(); + Box::pin(async move { svc.ready().await?.call(req).await }) + } else if path.starts_with(INFERENCE_PATH_PREFIX) { let mut svc = self.inference.clone(); Box::pin(async move { svc.ready().await?.call(req).await }) } else { diff --git a/docs/reference/gateway-auth.mdx b/docs/reference/gateway-auth.mdx index 94aca659ee..3e119d3bda 100644 --- a/docs/reference/gateway-auth.mdx +++ b/docs/reference/gateway-auth.mdx @@ -20,6 +20,12 @@ When any CLI command needs to talk to the gateway, it resolves the target throug The CLI loads gateway metadata from `~/.config/openshell/gateways//metadata.json` first and falls back to `/etc/openshell/gateways//metadata.json` when no user entry exists. +## Authentication Status + +`openshell status` reports gateway reachability and authentication separately. The public health RPC determines whether the gateway is connected and supplies its version. A separate side-effect-free authentication RPC verifies configured credentials without requiring a resource-specific role or scope. + +For example, an expired bearer token can produce `Status: Connected` and `Authentication: Failed`. This means the gateway is healthy but protected commands cannot use the current credentials. Run `openshell gateway login ` to re-authenticate. When a gateway predates the authentication status RPC, the CLI reports `Authentication: Unverified` rather than treating a successful public health check as proof of authentication. + ## Authentication Modes The CLI uses one of these authentication modes depending on the gateway's configuration. @@ -119,7 +125,7 @@ When you register or log in to an OIDC gateway, the CLI uses the Authorization C The connection flow: 1. The CLI loads the stored OIDC token bundle. -2. If the access token is expired and a refresh token is available, the CLI refreshes it. +2. If the access token is expired and a refresh token is available, the CLI refreshes it with the OIDC scopes saved in the gateway metadata. 3. The CLI connects to the gateway and attaches `authorization: Bearer ` metadata to each gRPC request. 4. The gateway validates the JWT signature, issuer, audience, expiration, and key ID against the issuer's JWKS. 5. The gateway extracts roles and optional scopes from the configured claim paths. diff --git a/docs/sandboxes/manage-gateways.mdx b/docs/sandboxes/manage-gateways.mdx index 443a65a750..91cfdfad28 100644 --- a/docs/sandboxes/manage-gateways.mdx +++ b/docs/sandboxes/manage-gateways.mdx @@ -113,13 +113,19 @@ openshell status -g staging ## Inspect Gateway Status -Use `openshell status` for a live gateway check. It verifies that the CLI can -reach the running gateway, then reports health and the gateway version: +Use `openshell status` for a live gateway check. It reports gateway +reachability, authentication, and the gateway version independently: ```shell openshell status ``` +`Status: Connected` means the public health endpoint responded. +`Authentication: Authenticated` confirms the configured credentials also +passed the gateway authentication layer. If authentication fails while status +remains connected, run `openshell gateway login ` to refresh the stored +credentials. + Use `openshell gateway info` when you need elevated runtime details such as initialized compute drivers and driver-reported capability versions: diff --git a/proto/openshell.proto b/proto/openshell.proto index 1b447a17e3..4a9f60a4a3 100644 --- a/proto/openshell.proto +++ b/proto/openshell.proto @@ -275,6 +275,14 @@ service OpenShell { rpc ListWorkspaceMembers(ListWorkspaceMembersRequest) returns (ListWorkspaceMembersResponse); } +// Authentication exposes a side-effect-free credential check independently +// from the public health endpoint and resource-specific authorization. +service Authentication { + // Verify the caller's configured gateway credentials. + rpc GetStatus(GetAuthenticationStatusRequest) + returns (GetAuthenticationStatusResponse); +} + // IssueSandboxToken request. Empty body; identity is established by the // authentication credentials carried in the request headers (a projected // Kubernetes ServiceAccount JWT in the K8s driver path). @@ -318,6 +326,28 @@ message HealthResponse { string version = 2; } +// Authentication status request. +message GetAuthenticationStatusRequest {} + +// Authentication status response. Invalid credentials are rejected by the +// gateway authentication middleware before this response is produced. +message GetAuthenticationStatusResponse { + // Whether the gateway established an authenticated user principal. + bool authenticated = 1; + + // Authentication provider that established the principal. + AuthenticationProvider provider = 2; +} + +// Authentication provider used for the current gateway request. +enum AuthenticationProvider { + AUTHENTICATION_PROVIDER_UNSPECIFIED = 0; + AUTHENTICATION_PROVIDER_OIDC = 1; + AUTHENTICATION_PROVIDER_MTLS = 2; + AUTHENTICATION_PROVIDER_CLOUDFLARE_ACCESS = 3; + AUTHENTICATION_PROVIDER_LOCAL_DEV = 4; +} + // Gateway info request. message GetGatewayInfoRequest {} From 7071812381809463b7f9c4759d1b1e042a3b1f28 Mon Sep 17 00:00:00 2001 From: Drew Newberry Date: Thu, 23 Jul 2026 11:32:28 -0700 Subject: [PATCH 2/2] fix(auth): reuse gateway info for status probe Signed-off-by: Drew Newberry --- .agents/skills/openshell-cli/cli-reference.md | 3 +- architecture/gateway.md | 13 +- crates/openshell-cli/src/run.rs | 134 +++++++++--------- crates/openshell-cli/src/tls.rs | 17 --- crates/openshell-server-macros/src/lib.rs | 15 +- crates/openshell-server/src/auth/authz.rs | 38 +---- .../openshell-server/src/auth/method_authz.rs | 19 ++- .../src/grpc/authentication.rs | 93 ------------ crates/openshell-server/src/grpc/mod.rs | 1 - crates/openshell-server/src/multiplex.rs | 35 ++--- docs/reference/gateway-auth.mdx | 4 +- proto/openshell.proto | 30 ---- 12 files changed, 115 insertions(+), 287 deletions(-) delete mode 100644 crates/openshell-server/src/grpc/authentication.rs diff --git a/.agents/skills/openshell-cli/cli-reference.md b/.agents/skills/openshell-cli/cli-reference.md index 109eda7b54..8095660817 100644 --- a/.agents/skills/openshell-cli/cli-reference.md +++ b/.agents/skills/openshell-cli/cli-reference.md @@ -183,7 +183,8 @@ package-managed or Helm gateways, use `systemctl`, `journalctl`, `kubectl`, and Show server connectivity, authentication status, and version for the active gateway. Connectivity uses the public health RPC; authentication is checked -separately and can fail while the gateway remains connected. +with the protected gateway-info capability query and can fail while the gateway +remains connected. --- diff --git a/architecture/gateway.md b/architecture/gateway.md index 35b775230e..c8b323ea13 100644 --- a/architecture/gateway.md +++ b/architecture/gateway.md @@ -144,12 +144,13 @@ OAuth scopes. Gateway health and user authentication are separate probes. `OpenShell.Health` remains unauthenticated so deployment and load-balancer health checks do not -depend on user credentials. `Authentication.GetStatus` is side-effect-free and -traverses every configured authentication layer, but deliberately requires no -application role or OAuth scope. The CLI combines both results so a reachable -gateway with an expired or rejected token is reported as connected but -unauthenticated; gateways with no configured user-auth layer report that -authentication is not required. +depend on user credentials. The CLI uses the existing, side-effect-free +`OpenShell.GetGatewayInfo` capability query as its protected authentication +probe. `Unauthenticated` means the credentials were rejected, while +`PermissionDenied` proves authentication succeeded before the caller failed +the capability query's admin authorization check. The CLI combines the health +and capability results so a reachable gateway with an expired or rejected +token is reported as connected but unauthenticated. Sandbox supervisor RPCs authenticate with explicit sandbox credentials; mTLS does not grant sandbox identity. Kubernetes deployments use the diff --git a/crates/openshell-cli/src/run.rs b/crates/openshell-cli/src/run.rs index a5b0c31755..a2ad725a4c 100644 --- a/crates/openshell-cli/src/run.rs +++ b/crates/openshell-cli/src/run.rs @@ -21,7 +21,7 @@ use crate::commands::common::{ use crate::policy_update::build_policy_update_plan; use crate::tls::{ TlsOptions, build_insecure_rustls_config, build_rustls_config, grpc_client, - grpc_inference_client, grpc_status_clients, require_tls_materials, + grpc_inference_client, require_tls_materials, }; use bytes::Bytes; use dialoguer::{Confirm, Select, theme::ColorfulTheme}; @@ -44,26 +44,25 @@ use openshell_bootstrap::{ use openshell_core::proto::ProviderProfileCategory; use openshell_core::proto::{ ApproveAllDraftChunksRequest, ApproveDraftChunkRequest, AttachSandboxProviderRequest, - AuthenticationProvider, ClearDraftChunksRequest, ConfigureProviderRefreshRequest, - CreateProviderRequest, CreateSandboxRequest, CreateSshSessionRequest, - DeleteInferenceRouteRequest, DeleteProviderProfileRequest, DeleteProviderRefreshRequest, - DeleteProviderRequest, DeleteSandboxRequest, DeleteServiceRequest, - DetachSandboxProviderRequest, ExecSandboxRequest, ExposeServiceRequest, - GetAuthenticationStatusRequest, GetAuthenticationStatusResponse, GetDraftHistoryRequest, - GetDraftPolicyRequest, GetGatewayConfigRequest, GetGatewayInfoRequest, - GetInferenceRouteRequest, GetProviderProfileRequest, GetProviderRefreshStatusRequest, - GetProviderRequest, GetSandboxConfigRequest, GetSandboxLogsRequest, - GetSandboxPolicyStatusRequest, GetSandboxRequest, GetServiceRequest, GpuResourceRequirements, - HealthRequest, ImportProviderProfilesRequest, LintProviderProfilesRequest, - ListProviderProfilesRequest, ListProvidersRequest, ListSandboxPoliciesRequest, - ListSandboxProvidersRequest, ListSandboxesRequest, ListServicesRequest, PolicySource, - PolicyStatus, Provider, ProviderCredentialRefreshStatus, ProviderCredentialRefreshStrategy, - ProviderProfile, ProviderProfileDiagnostic, ProviderProfileImportItem, RejectDraftChunkRequest, - ResourceRequirements, RevokeSshSessionRequest, RotateProviderCredentialRequest, Sandbox, - SandboxPhase, SandboxPolicy, SandboxSpec, SandboxTemplate, ServiceEndpointResponse, - ServiceStatus, SetInferenceRouteRequest, SettingScope, TcpForwardFrame, TcpForwardInit, - TcpRelayTarget, UpdateConfigRequest, UpdateProviderProfilesRequest, UpdateProviderRequest, - WatchSandboxRequest, exec_sandbox_event, setting_value, tcp_forward_init, + ClearDraftChunksRequest, ConfigureProviderRefreshRequest, CreateProviderRequest, + CreateSandboxRequest, CreateSshSessionRequest, DeleteInferenceRouteRequest, + DeleteProviderProfileRequest, DeleteProviderRefreshRequest, DeleteProviderRequest, + DeleteSandboxRequest, DeleteServiceRequest, DetachSandboxProviderRequest, ExecSandboxRequest, + ExposeServiceRequest, GetDraftHistoryRequest, GetDraftPolicyRequest, GetGatewayConfigRequest, + GetGatewayInfoRequest, GetInferenceRouteRequest, GetProviderProfileRequest, + GetProviderRefreshStatusRequest, GetProviderRequest, GetSandboxConfigRequest, + GetSandboxLogsRequest, GetSandboxPolicyStatusRequest, GetSandboxRequest, GetServiceRequest, + GpuResourceRequirements, HealthRequest, ImportProviderProfilesRequest, + LintProviderProfilesRequest, ListProviderProfilesRequest, ListProvidersRequest, + ListSandboxPoliciesRequest, ListSandboxProvidersRequest, ListSandboxesRequest, + ListServicesRequest, PolicySource, PolicyStatus, Provider, ProviderCredentialRefreshStatus, + ProviderCredentialRefreshStrategy, ProviderProfile, ProviderProfileDiagnostic, + ProviderProfileImportItem, RejectDraftChunkRequest, ResourceRequirements, + RevokeSshSessionRequest, RotateProviderCredentialRequest, Sandbox, SandboxPhase, SandboxPolicy, + SandboxSpec, SandboxTemplate, ServiceEndpointResponse, ServiceStatus, SetInferenceRouteRequest, + SettingScope, TcpForwardFrame, TcpForwardInit, TcpRelayTarget, UpdateConfigRequest, + UpdateProviderProfilesRequest, UpdateProviderRequest, WatchSandboxRequest, exec_sandbox_event, + setting_value, tcp_forward_init, }; use openshell_core::settings; use openshell_core::{ObjectId, ObjectName, ObjectWorkspace}; @@ -137,18 +136,18 @@ pub async fn gateway_status( println!(" {} {}", "Server:".dimmed(), server); // Try to connect and get health - match grpc_status_clients(server, tls).await { - Ok((mut client, mut authentication)) => match client.health(HealthRequest {}).await { + match grpc_client(server, tls).await { + Ok(mut client) => match client.health(HealthRequest {}).await { Ok(response) => { let health = response.into_inner(); println!(" {} {}", "Status:".dimmed(), "Connected".green()); let authentication = match auth_preparation_error { Some(error) => GatewayAuthenticationState::Failed(error.to_string()), None => gateway_authentication_state( - authentication - .get_status(GetAuthenticationStatusRequest {}) + client + .get_gateway_info(GetGatewayInfoRequest {}) .await - .map(tonic::Response::into_inner), + .map(|_| ()), tls, server, ), @@ -212,40 +211,36 @@ pub async fn gateway_status( enum GatewayAuthenticationState { Authenticated(&'static str), NotRequired(&'static str), - NotEnforced, Failed(String), Unverified(String), } fn gateway_authentication_state( - result: std::result::Result, + result: std::result::Result<(), Status>, tls: &TlsOptions, server: &str, ) -> GatewayAuthenticationState { match result { - Ok(response) if response.authenticated => { - let provider = match response.provider() { - AuthenticationProvider::Oidc => "OIDC", - AuthenticationProvider::Mtls => "mTLS", - AuthenticationProvider::CloudflareAccess => "edge", - AuthenticationProvider::LocalDev => "local development", - AuthenticationProvider::Unspecified => "gateway", - }; - GatewayAuthenticationState::Authenticated(provider) - } - Ok(response) if response.provider() == AuthenticationProvider::LocalDev => { - GatewayAuthenticationState::NotRequired("local development") - } - Ok(_) if tls.edge_token.is_some() => GatewayAuthenticationState::Authenticated("edge"), - Ok(_) if tls.oidc_token.is_some() || server.starts_with("https://") => { - GatewayAuthenticationState::NotEnforced + Ok(()) if tls.oidc_token.is_some() => GatewayAuthenticationState::Authenticated("OIDC"), + Ok(()) if tls.edge_token.is_some() => GatewayAuthenticationState::Authenticated("edge"), + Ok(()) if server.starts_with("https://") => { + GatewayAuthenticationState::Authenticated("mTLS transport") } - Ok(_) => GatewayAuthenticationState::NotRequired("gateway"), + Ok(()) => GatewayAuthenticationState::NotRequired("gateway"), Err(status) if status.code() == Code::Unauthenticated => { GatewayAuthenticationState::Failed(status.message().to_string()) } Err(status) if status.code() == Code::PermissionDenied => { - GatewayAuthenticationState::Authenticated("authorization denied") + let provider = if tls.oidc_token.is_some() { + "OIDC; authorization denied" + } else if tls.edge_token.is_some() { + "edge; authorization denied" + } else if server.starts_with("https://") { + "mTLS transport; authorization denied" + } else { + "authorization denied" + }; + GatewayAuthenticationState::Authenticated(provider) } Err(status) if status.code() == Code::Unimplemented && tls.edge_token.is_some() => { GatewayAuthenticationState::Authenticated("edge") @@ -285,11 +280,6 @@ fn print_gateway_authentication_state(state: &GatewayAuthenticationState) { "Authentication:".dimmed(), "Not required".green() ), - GatewayAuthenticationState::NotEnforced => println!( - " {} {}", - "Authentication:".dimmed(), - "Not enforced by gateway".yellow() - ), GatewayAuthenticationState::Failed(error) => println!( " {} {} ({error})", "Authentication:".dimmed(), @@ -8154,28 +8144,38 @@ mod tests { PROGRESS_STEP_STARTING_SANDBOX, }; use openshell_core::proto::{ - AuthenticationProvider, GetAuthenticationStatusResponse, GpuResourceRequirements, - PolicyStatus, Provider, ProviderCredentialRefresh, ProviderCredentialRefreshStatus, - ProviderCredentialRefreshStrategy, ProviderCredentialTokenGrant, ProviderProfile, - ProviderProfileCredential, ResourceRequirements, SandboxCondition, SandboxPolicyRevision, - SandboxStatus, datamodel::v1::ObjectMeta, + GpuResourceRequirements, PolicyStatus, Provider, ProviderCredentialRefresh, + ProviderCredentialRefreshStatus, ProviderCredentialRefreshStrategy, + ProviderCredentialTokenGrant, ProviderProfile, ProviderProfileCredential, + ResourceRequirements, SandboxCondition, SandboxPolicyRevision, SandboxStatus, + datamodel::v1::ObjectMeta, }; #[test] - fn gateway_status_reports_authenticated_oidc_principal() { + fn gateway_status_reports_authenticated_oidc_probe() { + let mut tls = TlsOptions::default(); + tls.oidc_token = Some("token".to_string()); + + let state = gateway_authentication_state(Ok(()), &tls, "https://gateway.example.com"); + + assert_eq!(state, GatewayAuthenticationState::Authenticated("OIDC")); + } + + #[test] + fn gateway_status_treats_authorization_denial_as_authenticated() { let mut tls = TlsOptions::default(); tls.oidc_token = Some("token".to_string()); let state = gateway_authentication_state( - Ok(GetAuthenticationStatusResponse { - authenticated: true, - provider: AuthenticationProvider::Oidc.into(), - }), + Err(Status::permission_denied("role 'openshell-admin' required")), &tls, "https://gateway.example.com", ); - assert_eq!(state, GatewayAuthenticationState::Authenticated("OIDC")); + assert_eq!( + state, + GatewayAuthenticationState::Authenticated("OIDC; authorization denied") + ); } #[test] @@ -8216,14 +8216,8 @@ mod tests { #[test] fn gateway_status_reports_local_plaintext_auth_not_required() { - let state = gateway_authentication_state( - Ok(GetAuthenticationStatusResponse { - authenticated: false, - provider: AuthenticationProvider::Unspecified.into(), - }), - &TlsOptions::default(), - "http://127.0.0.1:8080", - ); + let state = + gateway_authentication_state(Ok(()), &TlsOptions::default(), "http://127.0.0.1:8080"); assert_eq!(state, GatewayAuthenticationState::NotRequired("gateway")); } diff --git a/crates/openshell-cli/src/tls.rs b/crates/openshell-cli/src/tls.rs index 7d47f6d442..10df401a5b 100644 --- a/crates/openshell-cli/src/tls.rs +++ b/crates/openshell-cli/src/tls.rs @@ -3,7 +3,6 @@ use miette::{IntoDiagnostic, Result, WrapErr}; use openshell_core::auth::EdgeAuthInterceptor; -use openshell_core::proto::authentication_client::AuthenticationClient; use openshell_core::proto::inference_client::InferenceClient; use openshell_core::proto::open_shell_client::OpenShellClient; use rustls::{ @@ -25,9 +24,6 @@ use tracing::debug; /// Concrete gRPC client type used by all commands. pub type GrpcClient = OpenShellClient>; -/// Concrete authentication status client type. -pub type GrpcAuthenticationClient = - AuthenticationClient>; /// Concrete inference client type. pub type GrpcInferenceClient = InferenceClient>; @@ -444,19 +440,6 @@ pub async fn grpc_client(server: &str, tls: &TlsOptions) -> Result { Ok(OpenShellClient::with_interceptor(channel, interceptor)) } -/// Build `OpenShell` and authentication clients over one shared channel. -pub async fn grpc_status_clients( - server: &str, - tls: &TlsOptions, -) -> Result<(GrpcClient, GrpcAuthenticationClient)> { - let channel = build_channel(server, tls).await?; - let interceptor = interceptor_from_tls(tls)?; - Ok(( - OpenShellClient::with_interceptor(channel.clone(), interceptor.clone()), - AuthenticationClient::with_interceptor(channel, interceptor), - )) -} - fn interceptor_from_tls(tls: &TlsOptions) -> Result { EdgeAuthInterceptor::new(tls.oidc_token.as_deref(), tls.edge_token.as_deref()) } diff --git a/crates/openshell-server-macros/src/lib.rs b/crates/openshell-server-macros/src/lib.rs index fc0632638d..a698ae6623 100644 --- a/crates/openshell-server-macros/src/lib.rs +++ b/crates/openshell-server-macros/src/lib.rs @@ -5,9 +5,8 @@ //! //! `#[rpc_authz(service = "...")]` is applied to a tonic service `impl` //! block. Each method inside the impl carries an `#[rpc_auth(...)]` -//! attribute describing its auth mode and optional Bearer scope/role pair. -//! Omitting both on a Bearer method makes it authentication-only. The macro -//! emits a const `&[MethodAuth]` adjacent to +//! attribute describing its auth mode, optional Bearer scope, and +//! required role. The macro emits a const `&[MethodAuth]` adjacent to //! the impl and re-emits the impl block with the per-method //! `#[rpc_auth]` attributes stripped so other macros (notably //! `#[tonic::async_trait]`) see a clean impl. @@ -132,10 +131,16 @@ impl RpcAuth { } } AuthMode::Bearer | AuthMode::Dual => { - if scope.is_some() != role.is_some() { + if scope.is_none() { return Err(Error::new( span, - "`auth = \"bearer\"` and `auth = \"dual\"` require both `scope` and `role`, or neither for authentication-only methods", + "`auth = \"bearer\"` and `auth = \"dual\"` require `scope = \"...\"`", + )); + } + if role.is_none() { + return Err(Error::new( + span, + "`auth = \"bearer\"` and `auth = \"dual\"` require `role = \"...\"`", )); } } diff --git a/crates/openshell-server/src/auth/authz.rs b/crates/openshell-server/src/auth/authz.rs index e349e1abeb..1c04b09766 100644 --- a/crates/openshell-server/src/auth/authz.rs +++ b/crates/openshell-server/src/auth/authz.rs @@ -64,23 +64,17 @@ impl AuthzPolicy { /// (authentication-only mode for providers like GitHub). #[allow(clippy::result_large_err)] pub fn check(&self, identity: &Identity, method: &str) -> Result<(), Status> { - let required = method_authz::lookup(method).map_or( + let required = match method_authz::required_role(method) { + Some(Role::Admin) => &self.admin_role, // Default to user role for unknown methods, matching the // pre-annotation behavior. The exhaustiveness test ensures // every real RPC has an explicit declaration. - Some(&self.user_role), - |method| match method.role { - Some(Role::Admin) => Some(&self.admin_role), - Some(Role::User) => Some(&self.user_role), - None => None, - }, - ); + Some(Role::User) | None => &self.user_role, + }; // Empty role name = skip role check for this level (auth-only mode). // Scope enforcement still applies if enabled. - if let Some(required) = required - && !required.is_empty() - { + if !required.is_empty() { // Admin role implicitly satisfies user role requirements. let has_role = identity.roles.iter().any(|r| r == required) || (!self.admin_role.is_empty() @@ -114,15 +108,7 @@ impl AuthzPolicy { return Ok(()); } - let required_scope = match method_authz::lookup(method) { - Some(method) => { - let Some(scope) = method.scope else { - return Ok(()); - }; - scope - } - None => SCOPE_ALL, - }; + let required_scope = method_authz::required_scope(method).unwrap_or(SCOPE_ALL); if identity.scopes.iter().any(|s| s == required_scope) { return Ok(()); @@ -326,18 +312,6 @@ mod tests { ); } - #[test] - fn authentication_status_requires_no_role_or_scope() { - let id = identity_with_roles_and_scopes(&[], &[]); - let policy = scoped_policy(); - - assert!( - policy - .check(&id, "/openshell.v1.Authentication/GetStatus") - .is_ok() - ); - } - #[test] fn scoped_access_allowed() { let id = diff --git a/crates/openshell-server/src/auth/method_authz.rs b/crates/openshell-server/src/auth/method_authz.rs index 5937e6f37b..ec8dc5bca3 100644 --- a/crates/openshell-server/src/auth/method_authz.rs +++ b/crates/openshell-server/src/auth/method_authz.rs @@ -54,11 +54,7 @@ pub enum Role { /// Add a new service by appending its module's `AUTH_METADATA` const here. /// The constant name is fixed by `#[rpc_authz]`; service disambiguation /// comes from the module path. -const SERVICES: &[&[MethodAuth]] = &[ - crate::grpc::AUTH_METADATA, - crate::grpc::authentication::AUTH_METADATA, - crate::inference::AUTH_METADATA, -]; +const SERVICES: &[&[MethodAuth]] = &[crate::grpc::AUTH_METADATA, crate::inference::AUTH_METADATA]; /// Find the auth metadata for `method`, if any. #[must_use] @@ -77,6 +73,19 @@ pub fn all_paths() -> impl Iterator { SERVICES.iter().flat_map(|s| s.iter()).map(|m| m.path) } +/// Required Bearer scope for the method, or `None` if scopes don't +/// apply (`unauthenticated`, `sandbox`). +#[must_use] +pub fn required_scope(method: &str) -> Option<&'static str> { + lookup(method).and_then(|m| m.scope) +} + +/// Required role for the method on the Bearer path. +#[must_use] +pub fn required_role(method: &str) -> Option { + lookup(method).and_then(|m| m.role) +} + /// `true` if the method bypasses authentication entirely. /// /// Note: this checks the per-method tables only. The prefix-based diff --git a/crates/openshell-server/src/grpc/authentication.rs b/crates/openshell-server/src/grpc/authentication.rs deleted file mode 100644 index 7b8a31ec3d..0000000000 --- a/crates/openshell-server/src/grpc/authentication.rs +++ /dev/null @@ -1,93 +0,0 @@ -// SPDX-FileCopyrightText: Copyright (c) 2025-2026 NVIDIA CORPORATION & AFFILIATES. All rights reserved. -// SPDX-License-Identifier: Apache-2.0 - -//! Side-effect-free gateway authentication status RPC. - -use crate::auth::identity::IdentityProvider; -use crate::auth::principal::Principal; -use openshell_core::proto::{ - AuthenticationProvider, GetAuthenticationStatusRequest, GetAuthenticationStatusResponse, - authentication_server::Authentication, -}; -use openshell_server_macros::rpc_authz; -use tonic::{Request, Response, Status}; - -#[derive(Debug, Clone, Copy, Default)] -pub struct AuthenticationService; - -#[rpc_authz(service = "openshell.v1.Authentication")] -#[tonic::async_trait] -impl Authentication for AuthenticationService { - #[rpc_auth(auth = "bearer")] - async fn get_status( - &self, - request: Request, - ) -> Result, Status> { - let (authenticated, provider) = match request.extensions().get::() { - Some(Principal::User(user)) => { - let provider = match user.identity.provider { - IdentityProvider::Oidc => AuthenticationProvider::Oidc, - IdentityProvider::Mtls => AuthenticationProvider::Mtls, - IdentityProvider::CloudflareAccess => AuthenticationProvider::CloudflareAccess, - IdentityProvider::LocalDev => AuthenticationProvider::LocalDev, - }; - ( - user.identity.provider != IdentityProvider::LocalDev, - provider, - ) - } - Some(Principal::Sandbox(_) | Principal::Anonymous) | None => { - (false, AuthenticationProvider::Unspecified) - } - }; - - Ok(Response::new(GetAuthenticationStatusResponse { - authenticated, - provider: provider.into(), - })) - } -} - -#[cfg(test)] -mod tests { - use super::*; - use crate::auth::identity::Identity; - use crate::auth::principal::UserPrincipal; - - #[tokio::test] - async fn reports_authenticated_oidc_principal() { - let mut request = Request::new(GetAuthenticationStatusRequest {}); - request - .extensions_mut() - .insert(Principal::User(UserPrincipal { - identity: Identity { - subject: "alice".to_string(), - display_name: None, - roles: Vec::new(), - scopes: Vec::new(), - provider: IdentityProvider::Oidc, - }, - })); - - let response = AuthenticationService - .get_status(request) - .await - .expect("authentication status") - .into_inner(); - - assert!(response.authenticated); - assert_eq!(response.provider(), AuthenticationProvider::Oidc); - } - - #[tokio::test] - async fn reports_no_application_principal() { - let response = AuthenticationService - .get_status(Request::new(GetAuthenticationStatusRequest {})) - .await - .expect("authentication status") - .into_inner(); - - assert!(!response.authenticated); - assert_eq!(response.provider(), AuthenticationProvider::Unspecified); - } -} diff --git a/crates/openshell-server/src/grpc/mod.rs b/crates/openshell-server/src/grpc/mod.rs index f8d4beefd1..ecd96ea90d 100644 --- a/crates/openshell-server/src/grpc/mod.rs +++ b/crates/openshell-server/src/grpc/mod.rs @@ -4,7 +4,6 @@ //! gRPC service implementation. mod auth_rpc; -pub mod authentication; pub mod policy; pub mod provider; mod sandbox; diff --git a/crates/openshell-server/src/multiplex.rs b/crates/openshell-server/src/multiplex.rs index 414219c742..83e27d27cb 100644 --- a/crates/openshell-server/src/multiplex.rs +++ b/crates/openshell-server/src/multiplex.rs @@ -19,8 +19,7 @@ use hyper_util::{ use metrics::{counter, histogram}; use openshell_core::Config; use openshell_core::proto::{ - authentication_server::AuthenticationServer, inference_server::InferenceServer, - open_shell_server::OpenShellServer, + inference_server::InferenceServer, open_shell_server::OpenShellServer, }; use openshell_gateway_interceptors::{EvaluationContext, GatewayInterceptorRuntime}; use std::collections::BTreeMap; @@ -42,7 +41,6 @@ use crate::{ auth::identity::Identity, auth::oidc::{self, OidcAuthenticator}, auth::principal::{Principal, UserPrincipal}, - grpc::authentication::AuthenticationService, http_router, inference::InferenceService, service_http_router, @@ -164,8 +162,6 @@ impl MultiplexService { GatewayInterceptorGrpcService::new(openshell, self.state.gateway_interceptors.clone()); let inference = InferenceServer::new(InferenceService::new(self.state.clone())) .max_decoding_message_size(MAX_GRPC_DECODE_SIZE); - let authentication = AuthenticationServer::new(AuthenticationService) - .max_decoding_message_size(MAX_GRPC_DECODE_SIZE); let authz_policy = self.state.config.oidc.as_ref().map(|oidc| AuthzPolicy { admin_role: oidc.admin_role.clone(), user_role: oidc.user_role.clone(), @@ -173,7 +169,7 @@ impl MultiplexService { }); let authenticator_chain = build_authenticator_chain(&self.state); let grpc_service = AuthGrpcRouter::with_peer_identity( - GrpcRouter::new(openshell, authentication, inference), + GrpcRouter::new(openshell, inference), authenticator_chain, authz_policy, self.state @@ -642,39 +638,31 @@ where } } -/// Combined gRPC service that routes between the `OpenShell`, `Authentication`, -/// and `Inference` services based on the request path prefix. +/// Combined gRPC service that routes between `OpenShell` and Inference services +/// based on the request path prefix. #[derive(Clone)] -pub struct GrpcRouter { +pub struct GrpcRouter { openshell: N, - authentication: A, inference: I, } -impl GrpcRouter { - fn new(openshell: N, authentication: A, inference: I) -> Self { +impl GrpcRouter { + fn new(openshell: N, inference: I) -> Self { Self { openshell, - authentication, inference, } } } -const AUTHENTICATION_PATH_PREFIX: &str = "/openshell.v1.Authentication/"; const INFERENCE_PATH_PREFIX: &str = "/openshell.inference.v1.Inference/"; -impl tower::Service> for GrpcRouter +impl tower::Service> for GrpcRouter where N: tower::Service> + Clone + Send + 'static, N::Response: Send, N::Future: Send, N::Error: Send, - A: tower::Service, Response = N::Response, Error = N::Error> - + Clone - + Send - + 'static, - A::Future: Send, I: tower::Service, Response = N::Response, Error = N::Error> + Clone + Send @@ -691,12 +679,9 @@ where } fn call(&mut self, req: Request) -> Self::Future { - let path = req.uri().path(); + let is_inference = req.uri().path().starts_with(INFERENCE_PATH_PREFIX); - if path.starts_with(AUTHENTICATION_PATH_PREFIX) { - let mut svc = self.authentication.clone(); - Box::pin(async move { svc.ready().await?.call(req).await }) - } else if path.starts_with(INFERENCE_PATH_PREFIX) { + if is_inference { let mut svc = self.inference.clone(); Box::pin(async move { svc.ready().await?.call(req).await }) } else { diff --git a/docs/reference/gateway-auth.mdx b/docs/reference/gateway-auth.mdx index 3e119d3bda..b278f0f403 100644 --- a/docs/reference/gateway-auth.mdx +++ b/docs/reference/gateway-auth.mdx @@ -22,9 +22,9 @@ The CLI loads gateway metadata from `~/.config/openshell/gateways//metadat ## Authentication Status -`openshell status` reports gateway reachability and authentication separately. The public health RPC determines whether the gateway is connected and supplies its version. A separate side-effect-free authentication RPC verifies configured credentials without requiring a resource-specific role or scope. +`openshell status` reports gateway reachability and authentication separately. The public health RPC determines whether the gateway is connected and supplies its version. The CLI then calls the existing protected gateway-info capability query to verify the configured credentials. An authorization denial still confirms that the gateway authenticated the caller. -For example, an expired bearer token can produce `Status: Connected` and `Authentication: Failed`. This means the gateway is healthy but protected commands cannot use the current credentials. Run `openshell gateway login ` to re-authenticate. When a gateway predates the authentication status RPC, the CLI reports `Authentication: Unverified` rather than treating a successful public health check as proof of authentication. +For example, an expired bearer token can produce `Status: Connected` and `Authentication: Failed`. This means the gateway is healthy but protected commands cannot use the current credentials. Run `openshell gateway login ` to re-authenticate. When a gateway predates the gateway-info capability query, the CLI reports `Authentication: Unverified` rather than treating a successful public health check as proof of authentication. ## Authentication Modes diff --git a/proto/openshell.proto b/proto/openshell.proto index 4a9f60a4a3..1b447a17e3 100644 --- a/proto/openshell.proto +++ b/proto/openshell.proto @@ -275,14 +275,6 @@ service OpenShell { rpc ListWorkspaceMembers(ListWorkspaceMembersRequest) returns (ListWorkspaceMembersResponse); } -// Authentication exposes a side-effect-free credential check independently -// from the public health endpoint and resource-specific authorization. -service Authentication { - // Verify the caller's configured gateway credentials. - rpc GetStatus(GetAuthenticationStatusRequest) - returns (GetAuthenticationStatusResponse); -} - // IssueSandboxToken request. Empty body; identity is established by the // authentication credentials carried in the request headers (a projected // Kubernetes ServiceAccount JWT in the K8s driver path). @@ -326,28 +318,6 @@ message HealthResponse { string version = 2; } -// Authentication status request. -message GetAuthenticationStatusRequest {} - -// Authentication status response. Invalid credentials are rejected by the -// gateway authentication middleware before this response is produced. -message GetAuthenticationStatusResponse { - // Whether the gateway established an authenticated user principal. - bool authenticated = 1; - - // Authentication provider that established the principal. - AuthenticationProvider provider = 2; -} - -// Authentication provider used for the current gateway request. -enum AuthenticationProvider { - AUTHENTICATION_PROVIDER_UNSPECIFIED = 0; - AUTHENTICATION_PROVIDER_OIDC = 1; - AUTHENTICATION_PROVIDER_MTLS = 2; - AUTHENTICATION_PROVIDER_CLOUDFLARE_ACCESS = 3; - AUTHENTICATION_PROVIDER_LOCAL_DEV = 4; -} - // Gateway info request. message GetGatewayInfoRequest {}