diff --git a/crates/openshell-cli/src/oidc_auth.rs b/crates/openshell-cli/src/oidc_auth.rs index 379a53112..14b758774 100644 --- a/crates/openshell-cli/src/oidc_auth.rs +++ b/crates/openshell-cli/src/oidc_auth.rs @@ -29,6 +29,8 @@ use tokio::sync::oneshot; use tracing::debug; const AUTH_TIMEOUT: Duration = Duration::from_secs(120); +const DEFAULT_OIDC_CALLBACK_BIND: &str = "127.0.0.1:0"; +const OIDC_CALLBACK_PORT_ENV: &str = "OPENSHELL_OIDC_CALLBACK_PORT"; /// OIDC discovery document (subset of fields we need). #[derive(Debug, Deserialize)] @@ -95,6 +97,25 @@ fn build_ci_scopes(scopes: Option<&str>) -> Vec { .collect() } +fn oidc_callback_bind_address() -> Result { + match std::env::var(OIDC_CALLBACK_PORT_ENV) { + Ok(raw) => { + let port = raw.parse::().map_err(|_| { + miette::miette!( + "{OIDC_CALLBACK_PORT_ENV} must be a valid TCP port number, got '{raw}'" + ) + })?; + if port == 0 { + return Err(miette::miette!( + "{OIDC_CALLBACK_PORT_ENV} must be greater than 0" + )); + } + Ok(format!("127.0.0.1:{port}")) + } + Err(_) => Ok(DEFAULT_OIDC_CALLBACK_BIND.to_string()), + } +} + /// Run the OIDC Authorization Code + PKCE browser flow. /// /// Opens the user's browser to the Keycloak login page and waits for @@ -108,7 +129,9 @@ pub async fn oidc_browser_auth_flow( ) -> Result { let discovery = discover(issuer, insecure).await?; - let listener = TcpListener::bind("127.0.0.1:0").await.into_diagnostic()?; + let listener = TcpListener::bind(oidc_callback_bind_address()?) + .await + .into_diagnostic()?; let port = listener.local_addr().into_diagnostic()?.port(); let redirect_uri = format!("http://127.0.0.1:{port}/callback"); @@ -141,6 +164,7 @@ pub async fn oidc_browser_auth_flow( let server_handle = tokio::spawn(run_oidc_callback_server(listener, tx, expected_state)); eprintln!(" Opening browser for OIDC authentication..."); + if let Err(e) = crate::auth::open_browser_url(auth_url.as_str()) { debug!(error = %e, "failed to open browser"); eprintln!("Could not open browser automatically."); @@ -449,6 +473,39 @@ fn html_response(status: StatusCode, message: &str) -> Response> { #[cfg(test)] mod tests { use super::*; + use crate::TEST_ENV_LOCK as ENV_LOCK; + + struct EnvVarGuard { + key: &'static str, + original: Option, + } + + impl EnvVarGuard { + #[allow(unsafe_code)] + fn set(key: &'static str, value: &str) -> Self { + let original = std::env::var(key).ok(); + unsafe { std::env::set_var(key, value) }; + Self { key, original } + } + + #[allow(unsafe_code)] + fn remove(key: &'static str) -> Self { + let original = std::env::var(key).ok(); + unsafe { std::env::remove_var(key) }; + Self { key, original } + } + } + + impl Drop for EnvVarGuard { + #[allow(unsafe_code)] + fn drop(&mut self) { + if let Some(value) = &self.original { + unsafe { std::env::set_var(self.key, value) }; + } else { + unsafe { std::env::remove_var(self.key) }; + } + } + } #[test] fn http_client_secure_rejects_self_signed() { @@ -516,6 +573,37 @@ mod tests { assert!(scopes.is_empty()); } + #[test] + fn callback_bind_address_defaults_to_ephemeral_loopback() { + let _lock = ENV_LOCK + .lock() + .unwrap_or_else(std::sync::PoisonError::into_inner); + let _guard = EnvVarGuard::remove(OIDC_CALLBACK_PORT_ENV); + assert_eq!( + oidc_callback_bind_address().unwrap(), + DEFAULT_OIDC_CALLBACK_BIND + ); + } + + #[test] + fn callback_bind_address_uses_fixed_port_env() { + let _lock = ENV_LOCK + .lock() + .unwrap_or_else(std::sync::PoisonError::into_inner); + let _guard = EnvVarGuard::set(OIDC_CALLBACK_PORT_ENV, "8765"); + assert_eq!(oidc_callback_bind_address().unwrap(), "127.0.0.1:8765"); + } + + #[test] + fn callback_bind_address_rejects_invalid_port_env() { + let _lock = ENV_LOCK + .lock() + .unwrap_or_else(std::sync::PoisonError::into_inner); + let _guard = EnvVarGuard::set(OIDC_CALLBACK_PORT_ENV, "not-a-port"); + let err = oidc_callback_bind_address().unwrap_err(); + assert!(err.to_string().contains("valid TCP port")); + } + #[test] fn bundle_from_response_sets_fields() { use oauth2::basic::BasicTokenResponse; diff --git a/crates/openshell-core/src/config.rs b/crates/openshell-core/src/config.rs index 98562c8a6..1d562ae7f 100644 --- a/crates/openshell-core/src/config.rs +++ b/crates/openshell-core/src/config.rs @@ -83,6 +83,47 @@ impl FromStr for ComputeDriverKind { } } +/// OIDC provider profile presets for common issuer-specific claim layouts. +#[derive(Debug, Clone, Copy, Default, Serialize, Deserialize, PartialEq, Eq)] +#[serde(rename_all = "kebab-case")] +pub enum OidcProviderProfile { + /// Generic OIDC provider. Use explicit claim paths when defaults don't fit. + #[default] + Generic, + /// Okta Workforce identity using access tokens from a custom authorization server. + OktaWorkforce, +} + +impl OidcProviderProfile { + #[must_use] + pub const fn as_str(self) -> &'static str { + match self { + Self::Generic => "generic", + Self::OktaWorkforce => "okta-workforce", + } + } +} + +impl fmt::Display for OidcProviderProfile { + fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { + f.write_str(self.as_str()) + } +} + +impl FromStr for OidcProviderProfile { + type Err = String; + + fn from_str(value: &str) -> Result { + match value.trim().to_ascii_lowercase().as_str() { + "generic" => Ok(Self::Generic), + "okta-workforce" => Ok(Self::OktaWorkforce), + other => Err(format!( + "unsupported OIDC provider profile '{other}'. expected one of: generic, okta-workforce" + )), + } + } +} + /// Auto-detect the appropriate compute driver based on the runtime environment. /// /// Priority order: Kubernetes → Podman → Docker. @@ -306,6 +347,10 @@ pub struct OidcConfig { /// Expected audience (`aud`) claim. Typically the OIDC client ID. pub audience: String, + /// Optional provider-specific claim preset. + #[serde(default)] + pub provider_profile: OidcProviderProfile, + /// JWKS cache TTL in seconds. Defaults to 3600 (1 hour). #[serde(default = "default_jwks_ttl_secs")] pub jwks_ttl_secs: u64, @@ -331,6 +376,104 @@ pub struct OidcConfig { pub scopes_claim: String, } +impl OidcConfig { + /// Effective roles claim after applying provider profile defaults. + #[must_use] + pub fn effective_roles_claim(&self) -> &str { + match self.provider_profile { + OidcProviderProfile::OktaWorkforce if self.roles_claim == default_roles_claim() => { + "groups" + } + _ => self.roles_claim.as_str(), + } + } + + /// Effective scopes claim after applying provider profile defaults. + #[must_use] + pub fn effective_scopes_claim(&self) -> &str { + match self.provider_profile { + OidcProviderProfile::OktaWorkforce if self.scopes_claim.is_empty() => "scp", + _ => self.scopes_claim.as_str(), + } + } + + /// Return startup validation errors for OIDC configurations that are + /// incompatible with the selected provider profile. + #[must_use] + pub fn startup_errors(&self) -> Vec { + let mut errors = Vec::new(); + + if self.provider_profile == OidcProviderProfile::OktaWorkforce + && self.is_okta_org_authorization_server() + { + errors.push( + "OIDC provider_profile='okta-workforce' requires an Okta custom authorization server issuer (for example `https://example.okta.com/oauth2/default`). Okta org authorization server access tokens are not suitable for OpenShell gateway RBAC." + .to_string(), + ); + } + + errors + } + + /// Return advisory startup warnings for OIDC configurations that are valid + /// but likely surprising in practice. + #[must_use] + pub fn advisory_warnings(&self) -> Vec { + let mut warnings = Vec::new(); + let effective_roles_claim = self.effective_roles_claim(); + let effective_scopes_claim = self.effective_scopes_claim(); + + if self.is_okta_issuer() { + if self.provider_profile == OidcProviderProfile::Generic + && self.roles_claim == default_roles_claim() + && self.scopes_claim.is_empty() + { + warnings.push( + "Okta issuer detected with the generic OIDC provider profile and default claim settings. Consider provider_profile='okta-workforce' so OpenShell defaults to Okta-friendly access-token claims (`groups` and `scp`)." + .to_string(), + ); + } + + if self.is_okta_org_authorization_server() { + warnings.push( + "Okta issuer appears to be the org authorization server. OpenShell validates access tokens as a resource server; Okta org authorization server access tokens are intended for Okta APIs and aren't recommended for third-party resource-server validation. Prefer an Okta custom authorization server." + .to_string(), + ); + } + + if effective_roles_claim == "groups" && self.is_okta_org_authorization_server() { + warnings.push( + "Okta issuer appears to be the org authorization server while roles_claim=groups. OpenShell authorizes from access-token claims; Okta org authorization servers do not emit groups in access tokens. Use an Okta custom authorization server for group-based RBAC." + .to_string(), + ); + } + + if effective_roles_claim == "realm_access.roles" { + warnings.push( + "Okta issuer detected but roles_claim is still set to 'realm_access.roles', which is the OpenShell default for Keycloak. Okta gateway RBAC usually uses 'groups' or another custom access-token claim from an Okta custom authorization server." + .to_string(), + ); + } + + if !effective_scopes_claim.is_empty() && effective_scopes_claim != "scp" { + warnings.push(format!( + "Okta issuer detected but scopes_claim is set to '{effective_scopes_claim}'. Okta access tokens typically expose OAuth scopes via the 'scp' claim." + )); + } + } + + warnings + } + + fn is_okta_issuer(&self) -> bool { + self.issuer.contains(".okta.com") || self.issuer.contains(".oktapreview.com") + } + + fn is_okta_org_authorization_server(&self) -> bool { + self.is_okta_issuer() && !self.issuer.contains("/oauth2/") + } +} + /// mTLS user authentication for local, single-user gateways. #[derive(Debug, Clone, Default, Serialize, Deserialize)] pub struct MtlsAuthConfig { @@ -594,8 +737,8 @@ const fn default_ssh_session_ttl_secs() -> u64 { #[cfg(test)] mod tests { use super::{ - ComputeDriverKind, Config, DEFAULT_SERVICE_ROUTING_DOMAIN, detect_driver, - docker_host_unix_socket_path, is_unix_socket, + ComputeDriverKind, Config, DEFAULT_SERVICE_ROUTING_DOMAIN, OidcConfig, OidcProviderProfile, + default_roles_claim, detect_driver, docker_host_unix_socket_path, is_unix_socket, }; use std::net::SocketAddr; #[cfg(unix)] @@ -699,6 +842,137 @@ mod tests { assert_eq!(cfg.health_bind_address, Some(addr)); } + #[test] + fn oidc_advisory_warnings_flag_okta_org_server_groups_claim() { + let cfg = OidcConfig { + issuer: "https://example.okta.com".to_string(), + audience: "openshell-cli".to_string(), + provider_profile: OidcProviderProfile::Generic, + jwks_ttl_secs: 3600, + roles_claim: "groups".to_string(), + admin_role: "openshell-admin".to_string(), + user_role: "openshell-user".to_string(), + scopes_claim: String::new(), + }; + let warnings = cfg.advisory_warnings(); + assert_eq!(warnings.len(), 2); + assert!(warnings.iter().any(|w| w.contains("resource server"))); + assert!( + warnings + .iter() + .any(|w| w.contains("groups in access tokens")) + ); + } + + #[test] + fn oidc_advisory_warnings_accept_okta_custom_server_groups_claim() { + let cfg = OidcConfig { + issuer: "https://example.okta.com/oauth2/default".to_string(), + audience: "openshell-cli".to_string(), + provider_profile: OidcProviderProfile::Generic, + jwks_ttl_secs: 3600, + roles_claim: "groups".to_string(), + admin_role: "openshell-admin".to_string(), + user_role: "openshell-user".to_string(), + scopes_claim: "scp".to_string(), + }; + assert!(cfg.advisory_warnings().is_empty()); + } + + #[test] + fn oidc_advisory_warnings_flag_okta_default_keycloak_roles_claim() { + let cfg = OidcConfig { + issuer: "https://example.okta.com/oauth2/default".to_string(), + audience: "openshell-cli".to_string(), + provider_profile: OidcProviderProfile::Generic, + jwks_ttl_secs: 3600, + roles_claim: "realm_access.roles".to_string(), + admin_role: "openshell-admin".to_string(), + user_role: "openshell-user".to_string(), + scopes_claim: String::new(), + }; + let warnings = cfg.advisory_warnings(); + assert_eq!(warnings.len(), 2); + assert!(warnings.iter().any(|w| w.contains("realm_access.roles"))); + assert!( + warnings + .iter() + .any(|w| w.contains("provider_profile='okta-workforce'")) + ); + } + + #[test] + fn oidc_advisory_warnings_flag_okta_nonstandard_scopes_claim() { + let cfg = OidcConfig { + issuer: "https://example.okta.com/oauth2/default".to_string(), + audience: "openshell-cli".to_string(), + provider_profile: OidcProviderProfile::Generic, + jwks_ttl_secs: 3600, + roles_claim: "groups".to_string(), + admin_role: "openshell-admin".to_string(), + user_role: "openshell-user".to_string(), + scopes_claim: "scope".to_string(), + }; + let warnings = cfg.advisory_warnings(); + assert_eq!(warnings.len(), 1); + assert!(warnings[0].contains("'scp'")); + } + + #[test] + fn okta_workforce_profile_supplies_effective_claim_defaults() { + let cfg = OidcConfig { + issuer: "https://example.okta.com/oauth2/default".to_string(), + audience: "openshell-cli".to_string(), + provider_profile: OidcProviderProfile::OktaWorkforce, + jwks_ttl_secs: 3600, + roles_claim: default_roles_claim(), + admin_role: "openshell-admin".to_string(), + user_role: "openshell-user".to_string(), + scopes_claim: String::new(), + }; + assert_eq!(cfg.effective_roles_claim(), "groups"); + assert_eq!(cfg.effective_scopes_claim(), "scp"); + assert!(cfg.startup_errors().is_empty()); + assert!(cfg.advisory_warnings().is_empty()); + } + + #[test] + fn generic_okta_profile_with_default_claims_suggests_okta_workforce_profile() { + let cfg = OidcConfig { + issuer: "https://example.okta.com/oauth2/default".to_string(), + audience: "openshell-cli".to_string(), + provider_profile: OidcProviderProfile::Generic, + jwks_ttl_secs: 3600, + roles_claim: default_roles_claim(), + admin_role: "openshell-admin".to_string(), + user_role: "openshell-user".to_string(), + scopes_claim: String::new(), + }; + let warnings = cfg.advisory_warnings(); + assert!( + warnings + .iter() + .any(|w| w.contains("provider_profile='okta-workforce'")) + ); + } + + #[test] + fn okta_workforce_profile_rejects_org_authorization_server() { + let cfg = OidcConfig { + issuer: "https://example.okta.com".to_string(), + audience: "openshell-cli".to_string(), + provider_profile: OidcProviderProfile::OktaWorkforce, + jwks_ttl_secs: 3600, + roles_claim: default_roles_claim(), + admin_role: "openshell-admin".to_string(), + user_role: "openshell-user".to_string(), + scopes_claim: String::new(), + }; + let errors = cfg.startup_errors(); + assert_eq!(errors.len(), 1); + assert!(errors[0].contains("custom authorization server issuer")); + } + #[test] fn detect_driver_returns_none_without_k8s_env_or_binaries() { // When KUBERNETES_SERVICE_HOST is not set and no docker/podman binaries diff --git a/crates/openshell-core/src/lib.rs b/crates/openshell-core/src/lib.rs index 2c003f38c..2698fabf4 100644 --- a/crates/openshell-core/src/lib.rs +++ b/crates/openshell-core/src/lib.rs @@ -28,7 +28,7 @@ pub mod time; pub use config::{ ComputeDriverKind, Config, GatewayAuthConfig, GatewayJwtConfig, MtlsAuthConfig, OidcConfig, - TlsConfig, + OidcProviderProfile, TlsConfig, }; pub use error::{ComputeDriverError, Error, Result}; pub use metadata::{GetResourceVersion, ObjectId, ObjectLabels, ObjectName, SetResourceVersion}; diff --git a/crates/openshell-server/src/auth/authz.rs b/crates/openshell-server/src/auth/authz.rs index b4aa072d6..c9a239570 100644 --- a/crates/openshell-server/src/auth/authz.rs +++ b/crates/openshell-server/src/auth/authz.rs @@ -16,120 +16,212 @@ use super::identity::Identity; use tonic::Status; use tracing::debug; -/// gRPC methods that require the admin role. -/// All other authenticated methods require the user role. -const ADMIN_METHODS: &[&str] = &[ - // Provider management - "/openshell.v1.OpenShell/CreateProvider", - "/openshell.v1.OpenShell/UpdateProvider", - "/openshell.v1.OpenShell/DeleteProvider", - "/openshell.v1.OpenShell/ConfigureProviderRefresh", - "/openshell.v1.OpenShell/RotateProviderCredential", - "/openshell.v1.OpenShell/DeleteProviderRefresh", - // Global config and policy - "/openshell.v1.OpenShell/UpdateConfig", - // Draft policy approvals - "/openshell.v1.OpenShell/ApproveDraftChunk", - "/openshell.v1.OpenShell/ApproveAllDraftChunks", - "/openshell.v1.OpenShell/RejectDraftChunk", - "/openshell.v1.OpenShell/EditDraftChunk", - "/openshell.v1.OpenShell/UndoDraftChunk", - "/openshell.v1.OpenShell/ClearDraftChunks", - // Cluster inference write - "/openshell.inference.v1.Inference/SetClusterInference", -]; +const SCOPE_ALL: &str = "openshell:all"; + +#[derive(Debug, Clone, Copy, PartialEq, Eq)] +pub struct MethodPermission { + pub permission: &'static str, + pub scope: &'static str, + pub requires_admin: bool, +} -/// Exhaustive mapping of Bearer-authenticated gRPC methods to required scopes. -/// Methods not listed here require `openshell:all` when scope enforcement is enabled. -const SCOPED_METHODS: &[(&str, &str)] = &[ - // sandbox:read - ("/openshell.v1.OpenShell/GetSandbox", "sandbox:read"), - ("/openshell.v1.OpenShell/ListSandboxes", "sandbox:read"), +impl MethodPermission { + const fn new(permission: &'static str, scope: &'static str, requires_admin: bool) -> Self { + Self { + permission, + scope, + requires_admin, + } + } +} + +/// Exhaustive mapping of Bearer-authenticated gRPC methods to `OpenShell` +/// permissions and scopes. Methods not listed here fall back to +/// `openshell:all` when scope enforcement is enabled. +const METHOD_PERMISSIONS: &[(&str, MethodPermission)] = &[ + // sandbox.read + ( + "/openshell.v1.OpenShell/GetSandbox", + MethodPermission::new("sandbox.read", "sandbox:read", false), + ), + ( + "/openshell.v1.OpenShell/ListSandboxes", + MethodPermission::new("sandbox.read", "sandbox:read", false), + ), ( "/openshell.v1.OpenShell/ListSandboxProviders", - "sandbox:read", + MethodPermission::new("sandbox.read", "sandbox:read", false), + ), + ( + "/openshell.v1.OpenShell/WatchSandbox", + MethodPermission::new("sandbox.read", "sandbox:read", false), + ), + ( + "/openshell.v1.OpenShell/GetSandboxLogs", + MethodPermission::new("sandbox.read", "sandbox:read", false), + ), + ( + "/openshell.v1.OpenShell/GetService", + MethodPermission::new("sandbox.read", "sandbox:read", false), + ), + ( + "/openshell.v1.OpenShell/ListServices", + MethodPermission::new("sandbox.read", "sandbox:read", false), ), - ("/openshell.v1.OpenShell/WatchSandbox", "sandbox:read"), - ("/openshell.v1.OpenShell/GetSandboxLogs", "sandbox:read"), - ("/openshell.v1.OpenShell/GetService", "sandbox:read"), - ("/openshell.v1.OpenShell/ListServices", "sandbox:read"), ( "/openshell.v1.OpenShell/GetSandboxPolicyStatus", - "sandbox:read", + MethodPermission::new("sandbox.read", "sandbox:read", false), ), ( "/openshell.v1.OpenShell/ListSandboxPolicies", - "sandbox:read", - ), - // sandbox:write - ("/openshell.v1.OpenShell/CreateSandbox", "sandbox:write"), - ("/openshell.v1.OpenShell/DeleteSandbox", "sandbox:write"), - ("/openshell.v1.OpenShell/ExecSandbox", "sandbox:write"), - ("/openshell.v1.OpenShell/ForwardTcp", "sandbox:write"), - ("/openshell.v1.OpenShell/CreateSshSession", "sandbox:write"), - ("/openshell.v1.OpenShell/RevokeSshSession", "sandbox:write"), - ("/openshell.v1.OpenShell/ExposeService", "sandbox:write"), - ("/openshell.v1.OpenShell/DeleteService", "sandbox:write"), + MethodPermission::new("sandbox.read", "sandbox:read", false), + ), + // sandbox.write + ( + "/openshell.v1.OpenShell/CreateSandbox", + MethodPermission::new("sandbox.write", "sandbox:write", false), + ), + ( + "/openshell.v1.OpenShell/DeleteSandbox", + MethodPermission::new("sandbox.write", "sandbox:write", false), + ), + ( + "/openshell.v1.OpenShell/ExecSandbox", + MethodPermission::new("sandbox.write", "sandbox:write", false), + ), + ( + "/openshell.v1.OpenShell/ForwardTcp", + MethodPermission::new("sandbox.write", "sandbox:write", false), + ), + ( + "/openshell.v1.OpenShell/CreateSshSession", + MethodPermission::new("sandbox.write", "sandbox:write", false), + ), + ( + "/openshell.v1.OpenShell/RevokeSshSession", + MethodPermission::new("sandbox.write", "sandbox:write", false), + ), + ( + "/openshell.v1.OpenShell/ExposeService", + MethodPermission::new("sandbox.write", "sandbox:write", false), + ), + ( + "/openshell.v1.OpenShell/DeleteService", + MethodPermission::new("sandbox.write", "sandbox:write", false), + ), ( "/openshell.v1.OpenShell/AttachSandboxProvider", - "sandbox:write", + MethodPermission::new("sandbox.write", "sandbox:write", false), ), ( "/openshell.v1.OpenShell/DetachSandboxProvider", - "sandbox:write", + MethodPermission::new("sandbox.write", "sandbox:write", false), + ), + // provider.read + ( + "/openshell.v1.OpenShell/GetProvider", + MethodPermission::new("provider.read", "provider:read", false), + ), + ( + "/openshell.v1.OpenShell/ListProviders", + MethodPermission::new("provider.read", "provider:read", false), ), - // provider:read - ("/openshell.v1.OpenShell/GetProvider", "provider:read"), - ("/openshell.v1.OpenShell/ListProviders", "provider:read"), ( "/openshell.v1.OpenShell/GetProviderRefreshStatus", - "provider:read", + MethodPermission::new("provider.read", "provider:read", false), + ), + // provider.write + ( + "/openshell.v1.OpenShell/CreateProvider", + MethodPermission::new("provider.write", "provider:write", true), + ), + ( + "/openshell.v1.OpenShell/UpdateProvider", + MethodPermission::new("provider.write", "provider:write", true), + ), + ( + "/openshell.v1.OpenShell/DeleteProvider", + MethodPermission::new("provider.write", "provider:write", true), ), - // provider:write - ("/openshell.v1.OpenShell/CreateProvider", "provider:write"), - ("/openshell.v1.OpenShell/UpdateProvider", "provider:write"), - ("/openshell.v1.OpenShell/DeleteProvider", "provider:write"), ( "/openshell.v1.OpenShell/ConfigureProviderRefresh", - "provider:write", + MethodPermission::new("provider.write", "provider:write", true), ), ( "/openshell.v1.OpenShell/RotateProviderCredential", - "provider:write", + MethodPermission::new("provider.write", "provider:write", true), ), ( "/openshell.v1.OpenShell/DeleteProviderRefresh", - "provider:write", + MethodPermission::new("provider.write", "provider:write", true), + ), + // config.read + ( + "/openshell.v1.OpenShell/GetGatewayConfig", + MethodPermission::new("config.read", "config:read", false), + ), + ( + "/openshell.v1.OpenShell/GetSandboxConfig", + MethodPermission::new("config.read", "config:read", false), + ), + ( + "/openshell.v1.OpenShell/GetDraftPolicy", + MethodPermission::new("config.read", "config:read", false), + ), + ( + "/openshell.v1.OpenShell/GetDraftHistory", + MethodPermission::new("config.read", "config:read", false), + ), + // config.write + ( + "/openshell.v1.OpenShell/UpdateConfig", + MethodPermission::new("config.write", "config:write", true), + ), + ( + "/openshell.v1.OpenShell/ApproveDraftChunk", + MethodPermission::new("config.write", "config:write", true), ), - // config:read - ("/openshell.v1.OpenShell/GetGatewayConfig", "config:read"), - ("/openshell.v1.OpenShell/GetSandboxConfig", "config:read"), - ("/openshell.v1.OpenShell/GetDraftPolicy", "config:read"), - ("/openshell.v1.OpenShell/GetDraftHistory", "config:read"), - // config:write - ("/openshell.v1.OpenShell/UpdateConfig", "config:write"), - ("/openshell.v1.OpenShell/ApproveDraftChunk", "config:write"), ( "/openshell.v1.OpenShell/ApproveAllDraftChunks", - "config:write", + MethodPermission::new("config.write", "config:write", true), + ), + ( + "/openshell.v1.OpenShell/RejectDraftChunk", + MethodPermission::new("config.write", "config:write", true), + ), + ( + "/openshell.v1.OpenShell/EditDraftChunk", + MethodPermission::new("config.write", "config:write", true), ), - ("/openshell.v1.OpenShell/RejectDraftChunk", "config:write"), - ("/openshell.v1.OpenShell/EditDraftChunk", "config:write"), - ("/openshell.v1.OpenShell/UndoDraftChunk", "config:write"), - ("/openshell.v1.OpenShell/ClearDraftChunks", "config:write"), - // inference:read + ( + "/openshell.v1.OpenShell/UndoDraftChunk", + MethodPermission::new("config.write", "config:write", true), + ), + ( + "/openshell.v1.OpenShell/ClearDraftChunks", + MethodPermission::new("config.write", "config:write", true), + ), + // inference.read ( "/openshell.inference.v1.Inference/GetClusterInference", - "inference:read", + MethodPermission::new("inference.read", "inference:read", false), ), - // inference:write + // inference.write ( "/openshell.inference.v1.Inference/SetClusterInference", - "inference:write", + MethodPermission::new("inference.write", "inference:write", true), ), ]; -const SCOPE_ALL: &str = "openshell:all"; +const UNKNOWN_METHOD_PERMISSION: MethodPermission = + MethodPermission::new("gateway.unknown", SCOPE_ALL, false); + +fn method_permission(method: &str) -> MethodPermission { + METHOD_PERMISSIONS + .iter() + .find(|(candidate, _)| *candidate == method) + .map_or(UNKNOWN_METHOD_PERMISSION, |(_, permission)| *permission) +} /// Authorization policy configuration. /// @@ -169,6 +261,11 @@ impl AuthzPolicy { } impl AuthzPolicy { + #[must_use] + pub(crate) fn requirement_for(method: &str) -> MethodPermission { + method_permission(method) + } + /// Check whether the identity is authorized to call the given method. /// /// Returns `Ok(())` if authorized, `Err(PERMISSION_DENIED)` if not. @@ -176,7 +273,8 @@ 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 = if ADMIN_METHODS.contains(&method) { + let permission = method_permission(method); + let required = if permission.requires_admin { &self.admin_role } else { &self.user_role @@ -194,34 +292,36 @@ impl AuthzPolicy { if !has_role { debug!( sub = %identity.subject, + required_permission = permission.permission, required_role = required, user_roles = ?identity.roles, method = method, "authorization denied: missing role" ); return Err(Status::permission_denied(format!( - "role '{required}' required" + "permission '{}' requires role '{required}'", + permission.permission, ))); } } if self.scopes_enabled { - self.check_scope(identity, method)?; + Self::check_scope(identity, method, permission)?; } Ok(()) } - #[allow(clippy::result_large_err, clippy::unused_self)] - fn check_scope(&self, identity: &Identity, method: &str) -> Result<(), Status> { + #[allow(clippy::result_large_err)] + fn check_scope( + identity: &Identity, + method: &str, + permission: MethodPermission, + ) -> Result<(), Status> { if identity.scopes.iter().any(|s| s == SCOPE_ALL) { return Ok(()); } - - let required_scope = SCOPED_METHODS - .iter() - .find(|(m, _)| *m == method) - .map_or(SCOPE_ALL, |(_, s)| *s); + let required_scope = permission.scope; if identity.scopes.iter().any(|s| s == required_scope) { return Ok(()); @@ -229,13 +329,15 @@ impl AuthzPolicy { debug!( sub = %identity.subject, + required_permission = permission.permission, required_scope = required_scope, user_scopes = ?identity.scopes, method = method, "authorization denied: missing scope" ); Err(Status::permission_denied(format!( - "scope '{required_scope}' required" + "permission '{}' requires scope '{required_scope}'", + permission.permission, ))) } } @@ -660,4 +762,48 @@ mod tests { .is_err() ); } + + #[test] + fn method_permission_maps_sandbox_write_methods() { + let permission = method_permission("/openshell.v1.OpenShell/CreateSandbox"); + assert_eq!(permission.permission, "sandbox.write"); + assert_eq!(permission.scope, "sandbox:write"); + assert!(!permission.requires_admin); + } + + #[test] + fn method_permission_maps_admin_provider_writes() { + let permission = method_permission("/openshell.v1.OpenShell/CreateProvider"); + assert_eq!(permission.permission, "provider.write"); + assert_eq!(permission.scope, "provider:write"); + assert!(permission.requires_admin); + } + + #[test] + fn method_permission_falls_back_for_unknown_methods() { + let permission = method_permission("/openshell.v1.OpenShell/SomeFutureMethod"); + assert_eq!(permission, UNKNOWN_METHOD_PERMISSION); + } + + #[test] + fn denied_role_message_includes_permission_name() { + let id = identity_with_roles(&["openshell-user"]); + let policy = default_policy(); + let err = policy + .check(&id, "/openshell.v1.OpenShell/CreateProvider") + .unwrap_err(); + assert!(err.message().contains("provider.write")); + assert!(err.message().contains("openshell-admin")); + } + + #[test] + fn denied_scope_message_includes_permission_name() { + let id = identity_with_roles_and_scopes(&["openshell-user"], &["sandbox:read"]); + let policy = scoped_policy(); + let err = policy + .check(&id, "/openshell.v1.OpenShell/CreateSandbox") + .unwrap_err(); + assert!(err.message().contains("sandbox.write")); + assert!(err.message().contains("sandbox:write")); + } } diff --git a/crates/openshell-server/src/auth/oidc.rs b/crates/openshell-server/src/auth/oidc.rs index 5e5a23500..cd9fadca6 100644 --- a/crates/openshell-server/src/auth/oidc.rs +++ b/crates/openshell-server/src/auth/oidc.rs @@ -333,12 +333,13 @@ impl JwksCache { })?; let mut claims = token_data.claims; - claims.extract_roles(&self.config.roles_claim); + claims.extract_roles(self.config.effective_roles_claim()); - let scopes = if self.config.scopes_claim.is_empty() { + let scopes_claim = self.config.effective_scopes_claim(); + let scopes = if scopes_claim.is_empty() { vec![] } else { - claims.extract_scopes(&self.config.scopes_claim) + claims.extract_scopes(scopes_claim) }; Ok(Identity { @@ -390,6 +391,7 @@ impl Authenticator for OidcAuthenticator { #[cfg(test)] mod tests { use super::*; + use openshell_core::OidcProviderProfile; #[test] fn health_is_unauthenticated() { @@ -512,4 +514,52 @@ mod tests { let scopes = claims.extract_scopes("scope"); assert!(scopes.is_empty()); } + + #[test] + fn okta_workforce_profile_prefers_groups_and_scp_defaults() { + let config = OidcConfig { + issuer: "https://example.okta.com/oauth2/default".to_string(), + audience: "openshell-cli".to_string(), + provider_profile: OidcProviderProfile::OktaWorkforce, + jwks_ttl_secs: 3600, + roles_claim: "realm_access.roles".to_string(), + admin_role: "openshell-admin".to_string(), + user_role: "openshell-user".to_string(), + scopes_claim: String::new(), + }; + let json = serde_json::json!({ + "sub": "user1", + "groups": ["openshell-admin"], + "scp": ["openid", "sandbox:write", "provider:read"], + "realm_access": { "roles": ["ignored-role"] } + }); + let mut claims: OidcClaims = serde_json::from_value(json).unwrap(); + claims.extract_roles(config.effective_roles_claim()); + let scopes = claims.extract_scopes(config.effective_scopes_claim()); + assert_eq!(claims.roles, vec!["openshell-admin"]); + assert_eq!(scopes, vec!["sandbox:write", "provider:read"]); + } + + #[test] + fn generic_profile_keeps_explicit_claim_settings() { + let config = OidcConfig { + issuer: "https://example.okta.com/oauth2/default".to_string(), + audience: "openshell-cli".to_string(), + provider_profile: OidcProviderProfile::Generic, + jwks_ttl_secs: 3600, + roles_claim: "realm_access.roles".to_string(), + admin_role: "openshell-admin".to_string(), + user_role: "openshell-user".to_string(), + scopes_claim: String::new(), + }; + let json = serde_json::json!({ + "sub": "user1", + "groups": ["openshell-admin"], + "realm_access": { "roles": ["kept-role"] } + }); + let mut claims: OidcClaims = serde_json::from_value(json).unwrap(); + claims.extract_roles(config.effective_roles_claim()); + assert_eq!(claims.roles, vec!["kept-role"]); + assert!(config.effective_scopes_claim().is_empty()); + } } diff --git a/crates/openshell-server/src/auth/principal.rs b/crates/openshell-server/src/auth/principal.rs index a95eb831b..f3c35cf9b 100644 --- a/crates/openshell-server/src/auth/principal.rs +++ b/crates/openshell-server/src/auth/principal.rs @@ -76,3 +76,70 @@ pub enum SandboxIdentitySource { /// via `IssueSandboxToken`. Populated only on that one RPC path. K8sServiceAccount { pod_name: String, pod_uid: String }, } + +impl Principal { + /// Stable actor kind for gateway audit logs. + #[must_use] + pub const fn audit_actor_kind(&self) -> &'static str { + match self { + Self::User(_) => "user", + Self::Sandbox(_) => "sandbox", + Self::Anonymous => "anonymous", + } + } + + /// Stable actor subject for gateway audit logs. + #[must_use] + pub fn audit_actor_subject(&self) -> &str { + match self { + Self::User(user) => &user.identity.subject, + Self::Sandbox(sandbox) => &sandbox.sandbox_id, + Self::Anonymous => "anonymous", + } + } + + /// Optional human-readable actor display for gateway audit logs. + #[must_use] + pub fn audit_actor_display(&self) -> Option<&str> { + match self { + Self::User(user) => user.identity.display_name.as_deref(), + Self::Sandbox(_) | Self::Anonymous => None, + } + } +} + +#[cfg(test)] +mod tests { + use super::*; + use crate::auth::identity::{Identity, IdentityProvider}; + + #[test] + fn user_principal_formats_audit_actor_fields() { + let principal = Principal::User(UserPrincipal { + identity: Identity { + subject: "user-123".to_string(), + display_name: Some("alice".to_string()), + roles: vec!["openshell-user".to_string()], + scopes: vec![], + provider: IdentityProvider::Oidc, + }, + }); + assert_eq!(principal.audit_actor_kind(), "user"); + assert_eq!(principal.audit_actor_subject(), "user-123"); + assert_eq!(principal.audit_actor_display(), Some("alice")); + } + + #[test] + fn sandbox_principal_formats_audit_actor_fields() { + let principal = Principal::Sandbox(SandboxPrincipal { + sandbox_id: "sandbox-123".to_string(), + source: SandboxIdentitySource::BootstrapJwt { + issuer: "openshell-gateway:test".to_string(), + }, + trust_domain: Some("openshell".to_string()), + }); + assert_eq!(principal.audit_actor_kind(), "sandbox"); + assert_eq!(principal.audit_actor_subject(), "sandbox-123"); + assert_eq!(principal.audit_actor_display(), None); + } +} diff --git a/crates/openshell-server/src/cli.rs b/crates/openshell-server/src/cli.rs index b8d345f9e..24061e015 100644 --- a/crates/openshell-server/src/cli.rs +++ b/crates/openshell-server/src/cli.rs @@ -6,8 +6,8 @@ use clap::parser::ValueSource; use clap::{ArgAction, ArgMatches, Command, CommandFactory, FromArgMatches, Parser}; use miette::{IntoDiagnostic, Result}; -use openshell_core::ComputeDriverKind; use openshell_core::config::DEFAULT_SERVER_PORT; +use openshell_core::{ComputeDriverKind, OidcProviderProfile}; use std::net::{IpAddr, SocketAddr}; use std::path::PathBuf; use tracing::{info, warn}; @@ -140,12 +140,23 @@ struct RunArgs { #[arg(long, env = "OPENSHELL_OIDC_AUDIENCE", default_value = "openshell-cli")] oidc_audience: String, + /// OIDC provider profile preset for provider-specific claim layouts. + #[arg( + long, + env = "OPENSHELL_OIDC_PROVIDER_PROFILE", + default_value = "generic", + value_parser = parse_oidc_provider_profile + )] + oidc_provider_profile: OidcProviderProfile, + /// JWKS key cache TTL in seconds. #[arg(long, env = "OPENSHELL_OIDC_JWKS_TTL", default_value_t = 3600)] oidc_jwks_ttl: u64, /// Dot-separated path to the roles array in the JWT claims. /// Keycloak: `realm_access.roles` (default). Entra ID: "roles". Okta: "groups". + /// For Okta group-based RBAC, prefer a custom authorization server so + /// the groups claim is available in access tokens. #[arg( long, env = "OPENSHELL_OIDC_ROLES_CLAIM", @@ -171,7 +182,7 @@ struct RunArgs { /// Dot-separated path to the scopes value in the JWT claims. /// When set, the server enforces scope-based permissions on top of roles. - /// Keycloak: "scope". Okta: "scp". Leave empty to disable scope enforcement. + /// Keycloak: "scope". Entra ID / Okta: "scp". Leave empty to disable scope enforcement. #[arg(long, env = "OPENSHELL_OIDC_SCOPES_CLAIM", default_value = "")] oidc_scopes_claim: String, @@ -367,6 +378,7 @@ async fn run_from_args(mut args: RunArgs, matches: ArgMatches) -> Result<()> { config = config.with_oidc(openshell_core::OidcConfig { issuer, audience: args.oidc_audience.clone(), + provider_profile: args.oidc_provider_profile, jwks_ttl_secs: args.oidc_jwks_ttl, roles_claim: args.oidc_roles_claim.clone(), admin_role: args.oidc_admin_role.clone(), @@ -445,6 +457,10 @@ fn parse_compute_driver(value: &str) -> std::result::Result std::result::Result { + value.parse() +} + fn resolve_config_path(args: &RunArgs) -> Result> { if let Some(path) = args.config.clone() { return Ok(Some(path)); @@ -592,6 +608,9 @@ fn merge_file_into_args(args: &mut RunArgs, file: &GatewayFileSection, matches: if arg_defaulted(matches, "oidc_audience") { args.oidc_audience.clone_from(&oidc.audience); } + if arg_defaulted(matches, "oidc_provider_profile") { + args.oidc_provider_profile = oidc.provider_profile; + } if arg_defaulted(matches, "oidc_jwks_ttl") { args.oidc_jwks_ttl = oidc.jwks_ttl_secs; } @@ -726,6 +745,7 @@ mod tests { use super::{Cli, command}; use crate::TEST_ENV_LOCK as ENV_LOCK; use clap::Parser; + use openshell_core::OidcProviderProfile; use std::net::{IpAddr, Ipv4Addr}; struct EnvVarGuard { @@ -1271,6 +1291,7 @@ log_level = "debug" .unwrap_or_else(std::sync::PoisonError::into_inner); let _g1 = EnvVarGuard::remove("OPENSHELL_OIDC_ISSUER"); let _g2 = EnvVarGuard::remove("OPENSHELL_OIDC_AUDIENCE"); + let _g3 = EnvVarGuard::remove("OPENSHELL_OIDC_PROVIDER_PROFILE"); let (mut args, matches) = parse_with_args(&["openshell-gateway", "--db-url", "sqlite::memory:"]); @@ -1279,12 +1300,17 @@ log_level = "debug" [openshell.gateway.oidc] issuer = "https://idp.example.com" audience = "openshell-cli" +provider_profile = "okta-workforce" "#, ); merge_file_into_args(&mut args, &file.openshell.gateway, &matches); assert_eq!(args.oidc_issuer.as_deref(), Some("https://idp.example.com")); assert_eq!(args.oidc_audience, "openshell-cli"); + assert_eq!( + args.oidc_provider_profile, + OidcProviderProfile::OktaWorkforce + ); } #[test] diff --git a/crates/openshell-server/src/grpc/policy.rs b/crates/openshell-server/src/grpc/policy.rs index bdc96d862..264c95e33 100644 --- a/crates/openshell-server/src/grpc/policy.rs +++ b/crates/openshell-server/src/grpc/policy.rs @@ -1028,6 +1028,11 @@ pub(super) async fn handle_update_config( ); } info!( + actor_kind = principal.as_ref().map_or("unknown", Principal::audit_actor_kind), + actor_subject = principal + .as_ref() + .map_or("unknown", Principal::audit_actor_subject), + actor_display = principal.as_ref().and_then(Principal::audit_actor_display), sandbox_id = %sandbox_id, version, policy_hash = %hash, @@ -1086,6 +1091,11 @@ pub(super) async fn handle_update_config( .await .map_err(|e| super::persistence_error_to_status(e, "backfill spec.policy"))?; info!( + actor_kind = principal.as_ref().map_or("unknown", Principal::audit_actor_kind), + actor_subject = principal + .as_ref() + .map_or("unknown", Principal::audit_actor_subject), + actor_display = principal.as_ref().and_then(Principal::audit_actor_display), sandbox_id = %sandbox_id, "UpdateConfig: backfilled spec.policy from sandbox-discovered policy" ); @@ -1128,6 +1138,11 @@ pub(super) async fn handle_update_config( state.sandbox_watch_bus.notify(&sandbox_id); info!( + actor_kind = principal.as_ref().map_or("unknown", Principal::audit_actor_kind), + actor_subject = principal + .as_ref() + .map_or("unknown", Principal::audit_actor_subject), + actor_display = principal.as_ref().and_then(Principal::audit_actor_display), sandbox_id = %sandbox_id, version = next_version, policy_hash = %hash, diff --git a/crates/openshell-server/src/grpc/provider.rs b/crates/openshell-server/src/grpc/provider.rs index 3ddaae037..d12be6980 100644 --- a/crates/openshell-server/src/grpc/provider.rs +++ b/crates/openshell-server/src/grpc/provider.rs @@ -11,7 +11,7 @@ use crate::persistence::{ use openshell_core::proto::{Provider, Sandbox}; use prost::Message; use tonic::Status; -use tracing::warn; +use tracing::{info, warn}; use super::validation::validate_provider_fields; use super::{ @@ -624,6 +624,7 @@ impl ObjectType for Provider { // --------------------------------------------------------------------------- use crate::ServerState; +use crate::auth::principal::Principal; use openshell_core::proto::{ ConfigureProviderRefreshRequest, ConfigureProviderRefreshResponse, CreateProviderRequest, DeleteProviderProfileRequest, DeleteProviderProfileResponse, DeleteProviderRefreshRequest, @@ -648,12 +649,24 @@ pub(super) async fn handle_create_provider( state: &Arc, request: Request, ) -> Result, Status> { + let principal = request.extensions().get::().cloned(); let req = request.into_inner(); let provider = req .provider .ok_or_else(|| Status::invalid_argument("provider is required"))?; let provider = create_provider_record(state.store.as_ref(), provider).await?; + info!( + actor_kind = principal.as_ref().map_or("unknown", Principal::audit_actor_kind), + actor_subject = principal + .as_ref() + .map_or("unknown", Principal::audit_actor_subject), + actor_display = principal.as_ref().and_then(Principal::audit_actor_display), + provider_name = provider.object_name(), + provider_type = %provider.r#type, + "CreateProvider request completed successfully" + ); + Ok(Response::new(ProviderResponse { provider: Some(provider), })) @@ -1081,6 +1094,7 @@ pub(super) async fn handle_update_provider( state: &Arc, request: Request, ) -> Result, Status> { + let principal = request.extensions().get::().cloned(); let req = request.into_inner(); let mut provider = req .provider @@ -1090,6 +1104,17 @@ pub(super) async fn handle_update_provider( .extend(req.credential_expires_at_ms); let provider = update_provider_record(state.store.as_ref(), provider).await?; + info!( + actor_kind = principal.as_ref().map_or("unknown", Principal::audit_actor_kind), + actor_subject = principal + .as_ref() + .map_or("unknown", Principal::audit_actor_subject), + actor_display = principal.as_ref().and_then(Principal::audit_actor_display), + provider_name = provider.object_name(), + provider_type = %provider.r#type, + "UpdateProvider request completed successfully" + ); + Ok(Response::new(ProviderResponse { provider: Some(provider), })) @@ -1139,6 +1164,7 @@ pub(super) async fn handle_configure_provider_refresh( state: &Arc, request: Request, ) -> Result, Status> { + let principal = request.extensions().get::().cloned(); let request = request.into_inner(); let provider_name = request.provider.trim(); let credential_key = request.credential_key.trim(); @@ -1326,6 +1352,20 @@ pub(super) async fn handle_configure_provider_refresh( update_provider_record(state.store.as_ref(), updated).await?; } + info!( + actor_kind = principal + .as_ref() + .map_or("unknown", Principal::audit_actor_kind), + actor_subject = principal + .as_ref() + .map_or("unknown", Principal::audit_actor_subject), + actor_display = principal.as_ref().and_then(Principal::audit_actor_display), + provider_name, + credential_key, + strategy = crate::provider_refresh::refresh_strategy_name(strategy as i32), + "ConfigureProviderRefresh request completed successfully" + ); + Ok(Response::new(ConfigureProviderRefreshResponse { status: Some(crate::provider_refresh::refresh_status_from_state( &state_record, @@ -1337,6 +1377,7 @@ pub(super) async fn handle_rotate_provider_credential( state: &Arc, request: Request, ) -> Result, Status> { + let principal = request.extensions().get::().cloned(); let request = request.into_inner(); let provider_name = request.provider.trim(); let credential_key = request.credential_key.trim(); @@ -1353,6 +1394,19 @@ pub(super) async fn handle_rotate_provider_credential( ) .await?; + info!( + actor_kind = principal + .as_ref() + .map_or("unknown", Principal::audit_actor_kind), + actor_subject = principal + .as_ref() + .map_or("unknown", Principal::audit_actor_subject), + actor_display = principal.as_ref().and_then(Principal::audit_actor_display), + provider_name, + credential_key, + "RotateProviderCredential request completed successfully" + ); + Ok(Response::new(RotateProviderCredentialResponse { status: Some(crate::provider_refresh::refresh_status_from_state( &refresh_state, @@ -1364,6 +1418,7 @@ pub(super) async fn handle_delete_provider_refresh( state: &Arc, request: Request, ) -> Result, Status> { + let principal = request.extensions().get::().cloned(); let request = request.into_inner(); let provider_name = request.provider.trim(); let credential_key = request.credential_key.trim(); @@ -1421,6 +1476,20 @@ pub(super) async fn handle_delete_provider_refresh( update_provider_record(state.store.as_ref(), updated).await?; } + info!( + actor_kind = principal + .as_ref() + .map_or("unknown", Principal::audit_actor_kind), + actor_subject = principal + .as_ref() + .map_or("unknown", Principal::audit_actor_subject), + actor_display = principal.as_ref().and_then(Principal::audit_actor_display), + provider_name, + credential_key, + deleted = deleted_refresh_state, + "DeleteProviderRefresh request completed successfully" + ); + Ok(Response::new(DeleteProviderRefreshResponse { deleted: deleted_refresh_state, })) @@ -1430,9 +1499,21 @@ pub(super) async fn handle_delete_provider( state: &Arc, request: Request, ) -> Result, Status> { + let principal = request.extensions().get::().cloned(); let name = request.into_inner().name; let deleted = delete_provider_record(state.store.as_ref(), &name).await?; + info!( + actor_kind = principal.as_ref().map_or("unknown", Principal::audit_actor_kind), + actor_subject = principal + .as_ref() + .map_or("unknown", Principal::audit_actor_subject), + actor_display = principal.as_ref().and_then(Principal::audit_actor_display), + provider_name = %name, + deleted, + "DeleteProvider request completed successfully" + ); + Ok(Response::new(DeleteProviderResponse { deleted })) } diff --git a/crates/openshell-server/src/grpc/sandbox.rs b/crates/openshell-server/src/grpc/sandbox.rs index 1855972d7..bce4faf5c 100644 --- a/crates/openshell-server/src/grpc/sandbox.rs +++ b/crates/openshell-server/src/grpc/sandbox.rs @@ -10,6 +10,7 @@ #![allow(clippy::cast_possible_wrap)] // Intentional u32->i32 conversions for proto compat use crate::ServerState; +use crate::auth::principal::Principal; use crate::persistence::{ObjectType, WriteCondition, generate_name}; use futures::future; use openshell_core::proto::{ @@ -59,6 +60,7 @@ pub(super) async fn handle_create_sandbox( ) -> Result, Status> { use crate::persistence::current_time_ms; + let principal = request.extensions().get::().cloned(); let request = request.into_inner(); let spec = request .spec @@ -156,6 +158,11 @@ pub(super) async fn handle_create_sandbox( let sandbox = state.compute.create_sandbox(sandbox, sandbox_token).await?; info!( + actor_kind = principal.as_ref().map_or("unknown", Principal::audit_actor_kind), + actor_subject = principal + .as_ref() + .map_or("unknown", Principal::audit_actor_subject), + actor_display = principal.as_ref().and_then(Principal::audit_actor_display), sandbox_id = %id, sandbox_name = %name, "CreateSandbox request completed successfully" @@ -224,6 +231,7 @@ pub(super) async fn handle_attach_sandbox_provider( state: &Arc, request: Request, ) -> Result, Status> { + let principal = request.extensions().get::().cloned(); let request = request.into_inner(); if request.provider_name.is_empty() { return Err(Status::invalid_argument("provider_name is required")); @@ -322,6 +330,11 @@ pub(super) async fn handle_attach_sandbox_provider( let attached = attached.load(Ordering::Relaxed); info!( + actor_kind = principal.as_ref().map_or("unknown", Principal::audit_actor_kind), + actor_subject = principal + .as_ref() + .map_or("unknown", Principal::audit_actor_subject), + actor_display = principal.as_ref().and_then(Principal::audit_actor_display), sandbox_name = %request.sandbox_name, provider_name = %request.provider_name, attached, @@ -338,6 +351,7 @@ pub(super) async fn handle_detach_sandbox_provider( state: &Arc, request: Request, ) -> Result, Status> { + let principal = request.extensions().get::().cloned(); let request = request.into_inner(); if request.provider_name.is_empty() { return Err(Status::invalid_argument("provider_name is required")); @@ -397,6 +411,11 @@ pub(super) async fn handle_detach_sandbox_provider( let detached = detached.load(Ordering::Relaxed); info!( + actor_kind = principal.as_ref().map_or("unknown", Principal::audit_actor_kind), + actor_subject = principal + .as_ref() + .map_or("unknown", Principal::audit_actor_subject), + actor_display = principal.as_ref().and_then(Principal::audit_actor_display), sandbox_name = %request.sandbox_name, provider_name = %request.provider_name, detached, @@ -413,13 +432,23 @@ pub(super) async fn handle_delete_sandbox( state: &Arc, request: Request, ) -> Result, Status> { + let principal = request.extensions().get::().cloned(); let name = request.into_inner().name; if name.is_empty() { return Err(Status::invalid_argument("name is required")); } let deleted = state.compute.delete_sandbox(&name).await?; - info!(sandbox_name = %name, "DeleteSandbox request completed successfully"); + info!( + actor_kind = principal.as_ref().map_or("unknown", Principal::audit_actor_kind), + actor_subject = principal + .as_ref() + .map_or("unknown", Principal::audit_actor_subject), + actor_display = principal.as_ref().and_then(Principal::audit_actor_display), + sandbox_name = %name, + deleted, + "DeleteSandbox request completed successfully" + ); Ok(Response::new(DeleteSandboxResponse { deleted })) } diff --git a/crates/openshell-server/src/lib.rs b/crates/openshell-server/src/lib.rs index b7e145bde..ec84b95d2 100644 --- a/crates/openshell-server/src/lib.rs +++ b/crates/openshell-server/src/lib.rs @@ -199,11 +199,14 @@ pub async fn run_server( let store = Arc::new(Store::connect(database_url).await?); let oidc_cache = if let Some(ref oidc) = config.oidc { + if let Some(error) = oidc.startup_errors().into_iter().next() { + return Err(Error::config(error)); + } // Validate RBAC configuration before starting. let policy = auth::authz::AuthzPolicy { admin_role: oidc.admin_role.clone(), user_role: oidc.user_role.clone(), - scopes_enabled: !oidc.scopes_claim.is_empty(), + scopes_enabled: !oidc.effective_scopes_claim().is_empty(), }; policy.validate().map_err(Error::config)?; @@ -211,6 +214,9 @@ pub async fn run_server( .await .map_err(|e| Error::config(format!("OIDC initialization failed: {e}")))?; info!("OIDC JWT validation enabled (issuer: {})", oidc.issuer); + for warning in oidc.advisory_warnings() { + warn!(issuer = %oidc.issuer, "{warning}"); + } Some(Arc::new(cache)) } else { None diff --git a/crates/openshell-server/src/multiplex.rs b/crates/openshell-server/src/multiplex.rs index 4fcb3993a..caf8dbdfa 100644 --- a/crates/openshell-server/src/multiplex.rs +++ b/crates/openshell-server/src/multiplex.rs @@ -28,7 +28,7 @@ use std::time::{Duration, Instant}; use tokio::io::{AsyncRead, AsyncWrite}; use tower::ServiceExt; use tower_http::request_id::{MakeRequestId, RequestId}; -use tracing::Span; +use tracing::{Span, warn}; use crate::{ OpenShellService, ServerState, @@ -158,7 +158,7 @@ impl MultiplexService { let authz_policy = self.state.config.oidc.as_ref().map(|oidc| AuthzPolicy { admin_role: oidc.admin_role.clone(), user_role: oidc.user_role.clone(), - scopes_enabled: !oidc.scopes_claim.is_empty(), + scopes_enabled: !oidc.effective_scopes_claim().is_empty(), }); let authenticator_chain = build_authenticator_chain(&self.state); let grpc_service = AuthGrpcRouter::with_peer_identity( @@ -445,11 +445,35 @@ where if let Some(ref policy) = authz_policy && let Err(status) = policy.check(&user.identity, &path) { + let requirement = AuthzPolicy::requirement_for(&path); + warn!( + actor_kind = %principal.audit_actor_kind(), + actor_subject = %principal.audit_actor_subject(), + actor_display = principal.audit_actor_display(), + permission = requirement.permission, + required_scope = requirement.scope, + required_role = if requirement.requires_admin { + policy.admin_role.as_str() + } else { + policy.user_role.as_str() + }, + method = %path, + grpc_code = ?status.code(), + grpc_message = status.message(), + "authorization denied for authenticated gateway caller" + ); return Ok(status_response(status)); } } Principal::Sandbox(_) => { if !crate::auth::sandbox_methods::is_sandbox_callable(&path) { + warn!( + actor_kind = %principal.audit_actor_kind(), + actor_subject = %principal.audit_actor_subject(), + actor_display = principal.audit_actor_display(), + method = %path, + "authorization denied for sandbox principal on non-sandbox method" + ); return Ok(status_response(tonic::Status::permission_denied( "sandbox principals may not call this method", ))); diff --git a/docs/get-started/tutorials/index.mdx b/docs/get-started/tutorials/index.mdx index 0d82509ad..3d4294702 100644 --- a/docs/get-started/tutorials/index.mdx +++ b/docs/get-started/tutorials/index.mdx @@ -22,6 +22,11 @@ Create a sandbox, observe default-deny networking, apply a read-only L7 policy, Launch Claude Code in a sandbox, diagnose a policy denial, and iterate on a custom GitHub policy from outside the sandbox. + + +Configure an Okta-protected gateway and sign in from the CLI with a native app and PKCE. + + Configure a Providers v2 Microsoft Graph provider with gateway-managed OAuth2 refresh-token rotation. diff --git a/docs/get-started/tutorials/okta-gateway-login.mdx b/docs/get-started/tutorials/okta-gateway-login.mdx new file mode 100644 index 000000000..a18a13b85 --- /dev/null +++ b/docs/get-started/tutorials/okta-gateway-login.mdx @@ -0,0 +1,162 @@ +--- +# SPDX-FileCopyrightText: Copyright (c) 2025-2026 NVIDIA CORPORATION & AFFILIATES. All rights reserved. +# SPDX-License-Identifier: Apache-2.0 +title: "Sign In to an Okta-Protected Gateway" +sidebar-title: "Okta Gateway Login" +slug: "get-started/tutorials/okta-gateway-login" +description: "Configure Okta OIDC for an OpenShell gateway and sign in from the CLI with a native app and PKCE." +keywords: "Generative AI, Cybersecurity, Tutorial, Okta, OIDC, Gateway, PKCE, Access Token" +--- + +Use Okta to sign in to an OpenShell gateway from the CLI. Okta authenticates the user in the browser, the CLI stores the resulting access token, and the gateway uses that token to decide which actions the user can perform. + +After completing this tutorial, you have: + +- An OpenShell gateway configured to validate Okta-issued access tokens. +- An Okta native app configured for CLI browser login with PKCE. +- A CLI gateway registration that can log in and call the gateway as an Okta-backed user. + + +This tutorial covers gateway sign-in only. It does not configure sandbox runtime credentials or delegated token exchange. + + +## Prerequisites + +- A working OpenShell installation and a gateway you can restart with new auth settings. +- An Okta org and admin access. +- A custom Okta authorization server if you want group-based gateway RBAC from access-token claims. + + +OpenShell reads roles and scopes from the access token, not the ID token. If you want to use Okta groups for gateway roles, use a custom authorization server that places groups in access tokens. + + + + +## Create the Okta Native App + +In Okta, create an OIDC native application for the OpenShell CLI browser callback. + +Use these settings: + +- Application type: `Native` +- Sign-in redirect URI: `http://127.0.0.1:8765/callback` +- Grant type: Authorization Code with PKCE + +Make a note of: + +- Okta issuer URL +- CLI client ID + +If the gateway will enforce scopes, decide which scopes the CLI should request during login. + +## Configure the Gateway + +Start or restart the gateway with Okta OIDC enabled: + +```shell +openshell-gateway \ + --oidc-issuer https://your-okta-domain/oauth2/default \ + --oidc-audience openshell-cli \ + --oidc-roles-claim groups \ + --oidc-admin-role openshell-admin \ + --oidc-user-role openshell-user \ + --oidc-scopes-claim scp +``` + +For Okta, use: + +- `groups` for role mapping +- `scp` for scope checks + +If you start the gateway from environment variables instead, set: + +- `OPENSHELL_OIDC_ISSUER` +- `OPENSHELL_OIDC_AUDIENCE` +- `OPENSHELL_OIDC_ROLES_CLAIM` +- `OPENSHELL_OIDC_ADMIN_ROLE` +- `OPENSHELL_OIDC_USER_ROLE` +- `OPENSHELL_OIDC_SCOPES_CLAIM` + +## Register the Gateway in the CLI + +Register the gateway and tell the CLI to use the Okta native app: + +```shell +openshell gateway add https://gateway.example.com \ + --name okta-gateway \ + --oidc-issuer https://your-okta-domain/oauth2/default \ + --oidc-client-id YOUR_OKTA_NATIVE_APP_CLIENT_ID \ + --oidc-audience openshell-cli +``` + +If the gateway enforces scopes, request them at registration time: + +```shell +openshell gateway add https://gateway.example.com \ + --name okta-gateway \ + --oidc-issuer https://your-okta-domain/oauth2/default \ + --oidc-client-id YOUR_OKTA_NATIVE_APP_CLIENT_ID \ + --oidc-audience openshell-cli \ + --oidc-scopes "config:read provider:read sandbox:read" +``` + +## Sign In with the Browser Flow + +Force the browser callback port to match the redirect URI you configured in Okta: + +```shell +OPENSHELL_OIDC_CALLBACK_PORT=8765 \ +openshell gateway login okta-gateway +``` + +The CLI opens your browser, completes the login, and stores the token bundle under: + +```text +~/.config/openshell/gateways/okta-gateway/oidc_token.json +``` + +## Verify Gateway Access + +Once the login finishes, verify that the CLI can call the gateway: + +```shell +openshell gateway ls +openshell sandbox ls -g okta-gateway +``` + +If your Okta user has the expected role and any required scopes, the gateway accepts the request. + +## Troubleshooting + +### `redirect_uri` must be a Login redirect URI + +The callback URI in Okta and the CLI callback port do not match. + +If Okta is configured for: + +```text +http://127.0.0.1:8765/callback +``` + +then login with: + +```shell +OPENSHELL_OIDC_CALLBACK_PORT=8765 openshell gateway login okta-gateway +``` + +### Login succeeds but gateway methods are denied + +This usually means one of these common setup issues: + +- the access token does not contain the expected `groups` +- the gateway is looking at the wrong claim +- the CLI did not request the scopes required by the gateway + +Check: + +- the Okta authorization server issuing the access token +- `rolesClaim=groups` +- `scopesClaim=scp` +- `--oidc-scopes` on the CLI registration + + diff --git a/docs/kubernetes/access-control.mdx b/docs/kubernetes/access-control.mdx index 5c333bb53..a22bc761e 100644 --- a/docs/kubernetes/access-control.mdx +++ b/docs/kubernetes/access-control.mdx @@ -23,6 +23,8 @@ For how the CLI resolves gateways and stores credentials, refer to [Gateway Auth Set `server.oidc.issuer` to enable OIDC. The gateway validates the `Authorization: Bearer ` header on every request against the issuer's JWKS endpoint. +OpenShell authorizes gateway requests from access-token claims. For Okta, that means the choice of authorization server matters. If you want to map Okta groups to OpenShell gateway roles, use an Okta custom authorization server that emits groups in access tokens. Okta's org authorization server can emit groups in ID tokens, but not in access tokens. + ```shell helm upgrade openshell \ oci://ghcr.io/nvidia/openshell/helm-chart \ @@ -74,6 +76,10 @@ Both `adminRole` and `userRole` must be set, or both must be empty. Setting only | Microsoft Entra ID | `roles` | | Okta | `groups` | +For Okta, pair `rolesClaim=groups` with a custom authorization server when you need gateway RBAC from group membership. If you also enable scope-based authorization, use `server.oidc.scopesClaim=scp`. + +For interactive CLI login against an Okta-backed gateway, register the CLI as an Okta native app and use Authorization Code with PKCE. When the gateway enforces scopes, the CLI must also request those scopes during login with `openshell gateway add --oidc-scopes "..."` or the resulting access token will authenticate successfully but still be denied for scoped methods. + ## Reverse-Proxy Auth Termination When an access proxy, such as Cloudflare Access, ngrok, or a corporate SSO gateway, handles authentication in front of the OpenShell gateway, you can explicitly allow unauthenticated user calls at the gateway: diff --git a/docs/reference/gateway-auth.mdx b/docs/reference/gateway-auth.mdx index 90f1dd668..8fbdf148d 100644 --- a/docs/reference/gateway-auth.mdx +++ b/docs/reference/gateway-auth.mdx @@ -68,6 +68,12 @@ Gateways can validate OpenID Connect access tokens on gRPC requests. Configure O OIDC is application-layer authentication. TLS still controls the transport. If TLS client certificates remain required, the CLI must also have an mTLS bundle for the gateway. +For Okta, pay attention to which authorization server issues the access token that the CLI sends to the gateway. OpenShell authorizes requests from access-token claims, not from the ID token. If you want to use Okta groups for gateway RBAC, use an Okta custom authorization server that includes the groups claim in access tokens. Okta's org authorization server can include groups in ID tokens, but not in access tokens. + +For interactive CLI login, register the OpenShell CLI as an Okta native app and use Authorization Code with PKCE. Web-app client settings and confidential-client flows are a different shape and are not the right fit for the local browser callback used by `openshell gateway login`. + +For a complete Okta setup walkthrough, refer to [Okta Gateway Login](/get-started/tutorials/okta-gateway-login). + Configure the gateway with an issuer and audience: ```shell @@ -91,6 +97,15 @@ The same settings are available through environment variables: | `OPENSHELL_OIDC_USER_ROLE` | Role required for standard user operations. | `openshell-user` | | `OPENSHELL_OIDC_SCOPES_CLAIM` | Dot-separated claim path containing scopes. Empty disables scope enforcement. | Empty | +Provider-specific claim guidance: + +| Provider | `rolesClaim` | `scopesClaim` | Notes | +|---|---|---|---| +| Keycloak | `realm_access.roles` | `scope` | Default OpenShell examples use this shape. | +| Microsoft Entra ID | `roles` | `scp` | `scp` is an array in access tokens. | +| Okta custom authorization server | `groups` | `scp` | Recommended for OpenShell gateway RBAC. | +| Okta org authorization server | `groups` | `scp` | Use only if you do not depend on groups in the access token. Okta doesn't include groups in org-server access tokens. | + For Helm deployments, set the same values under `server.oidc`: ```yaml @@ -114,6 +129,17 @@ openshell gateway add https://gateway.example.com \ --oidc-audience openshell-cli ``` +When the gateway enforces scope-based authorization, also tell the CLI which scopes to request during login: + +```shell +openshell gateway add https://gateway.example.com \ + --name production \ + --oidc-issuer https://idp.example.com/realms/openshell \ + --oidc-client-id openshell-cli \ + --oidc-audience openshell-cli \ + --oidc-scopes "config:write provider:write sandbox:write" +``` + When you register or log in to an OIDC gateway, the CLI uses the Authorization Code flow with PKCE. It opens a browser, receives the authorization code on a localhost callback, exchanges the code for tokens, and stores the token bundle under the gateway credential directory. If `OPENSHELL_OIDC_CLIENT_SECRET` is set, the CLI uses the client credentials flow instead. Use that mode for CI and other non-interactive automation. The connection flow: @@ -127,6 +153,8 @@ The connection flow: If `OPENSHELL_OIDC_SCOPES_CLAIM` is set, the gateway also enforces scopes. It accepts space-delimited scope strings such as `scope: "openid sandbox:read"` and JSON arrays such as `scp: ["sandbox:read"]`. Standard OIDC scopes such as `openid`, `profile`, `email`, and `offline_access` are ignored for authorization. `openshell:all` grants access to all scoped methods. +OpenShell treats roles and scopes as separate checks. For example, an admin-level config update can require both the `openshell-admin` role and the `config:write` scope in the same access token. If the token only carries the role, or only carries the scope, the gateway denies the request. + Supervisor-to-gateway RPCs do not use user OIDC tokens or mTLS user identity. Each sandbox supervisor presents a gateway-minted `Authorization: Bearer` token scoped to its sandbox ID. On Kubernetes, the gateway mints that token only after TokenReview validates the projected ServiceAccount token, the pod UID matches the live pod, and the pod's controlling `Sandbox` ownerReference matches the live Sandbox CR. Log upload, policy status, credential environment lookup, inference bundle lookup, and sandbox config sync run with sandbox-restricted scope, while CLI users authenticate with OIDC, edge auth, local mTLS user authentication, or an explicitly enabled unauthenticated local developer mode. `GetInferenceBundle` returns route material that includes provider credentials, so it requires a sandbox principal; user callers manage inference configuration through the user-facing inference APIs instead. Re-authenticate an OIDC gateway with: