From a4c27f9ca95bee4ff87b3f4b426bba19f497915f Mon Sep 17 00:00:00 2001 From: krishicks Date: Wed, 21 Oct 2026 15:58:14 +0000 Subject: [PATCH] refactor(auth): share audit helpers for gateway denials and rejections (#4777) Handlers recorded each refusal by emitting an OCSF event and then building the status by hand, so a new refusal could skip the event. These helpers do both in one call: - audit::deny emits an authorization denial for the current request and returns the permission-denied status. Guards, workspace authorization, and the token and current-user handlers use it. - audit::reject_credential emits an Authentication failure for a credential a handler verifies itself and returns the rejection's status. It replaces emit_credential_failure for SSH session tokens and IssueSandboxToken. RefreshSandboxToken now takes its path and origin from the request scope, as every other handler does, instead of rebuilding them from the request. - Failure and logon events share one Authentication builder. - AuthMechanism and AuthFailureReason carry their audit labels. The logon digest uses the label instead of the variant's Debug name. Events and statuses are unchanged. Refs #2912 Signed-off-by: Kris Hicks --- crates/openshell-server/src/auth/audit.rs | 143 ++++++++---------- .../src/auth/authenticator.rs | 32 ++++ crates/openshell-server/src/auth/guard.rs | 37 +++-- .../src/auth/workspace_authz.rs | 54 ++++--- crates/openshell-server/src/grpc/auth_rpc.rs | 109 ++++++------- .../src/grpc/mutation_replay.rs | 4 +- crates/openshell-server/src/grpc/sandbox.rs | 23 +-- crates/openshell-server/src/grpc/workspace.rs | 4 +- 8 files changed, 190 insertions(+), 216 deletions(-) diff --git a/crates/openshell-server/src/auth/audit.rs b/crates/openshell-server/src/auth/audit.rs index 8c4af3b77..2584f4207 100644 --- a/crates/openshell-server/src/auth/audit.rs +++ b/crates/openshell-server/src/auth/audit.rs @@ -9,14 +9,15 @@ use openshell_ocsf::{ ActionId, ActivityId, ApiActivityBuilder, AuthProtocolId, AuthenticationBuilder, ConfidenceId, - DetectionFindingBuilder, DispositionId, Endpoint, FindingInfo, HttpMethod, HttpRequest, - ResourceDetails, Service, SeverityId, StatusId, Url, User, UserTypeId, ocsf_emit, + DetectionFindingBuilder, DispositionId, Endpoint, EventContext, FindingInfo, HttpMethod, + HttpRequest, ResourceDetails, Service, SeverityId, StatusId, Url, User, UserTypeId, ocsf_emit, }; use std::collections::{HashSet, VecDeque}; use std::future::Future; use std::net::SocketAddr; use std::sync::{Arc, Mutex, PoisonError}; use std::time::{Duration, Instant}; +use tonic::Status; use super::identity::IdentityProvider; use super::principal::{Principal, SandboxIdentitySource}; @@ -60,21 +61,7 @@ impl RequestOrigin { } } -use super::authenticator::{AuthFailureReason, AuthMechanism, bearer_credential}; - -fn category(reason: AuthFailureReason) -> &'static str { - match reason { - AuthFailureReason::MissingCredential => "missing_credential", - AuthFailureReason::ConflictingCredentials => "conflicting_credentials", - AuthFailureReason::MalformedCredential => "malformed_credential", - AuthFailureReason::InvalidCredential => "invalid_credential", - AuthFailureReason::ExpiredCredential => "expired_credential", - AuthFailureReason::AudienceOrIssuerMismatch => "audience_or_issuer_mismatch", - AuthFailureReason::UnknownIdentity => "unknown_identity", - AuthFailureReason::StaleIdentity => "stale_identity", - AuthFailureReason::IdentityProviderUnavailable => "identity_provider_unavailable", - } -} +use super::authenticator::{AuthFailureReason, AuthMechanism, AuthRejection, bearer_credential}; fn severity(reason: AuthFailureReason) -> SeverityId { match reason { @@ -91,20 +78,39 @@ fn severity(reason: AuthFailureReason) -> SeverityId { } } -fn with_protocol( - builder: AuthenticationBuilder<'_>, - mechanism: AuthMechanism, -) -> AuthenticationBuilder<'_> { - match mechanism { - AuthMechanism::Oidc => builder.auth_protocol(AuthProtocolId::OpenId), - AuthMechanism::Mtls => builder.auth_protocol_other("mtls"), - AuthMechanism::SandboxJwt => builder.auth_protocol_other("sandbox_jwt"), - AuthMechanism::ComputeDriver => builder.auth_protocol_other("compute_driver"), - AuthMechanism::GatewayPeer => builder.auth_protocol_other("k8s_service_account"), - AuthMechanism::SshSessionToken => builder.auth_protocol_other("ssh_session_token"), +/// The request to `path` as an OCSF HTTP request; gRPC calls are POSTs. +fn post(path: &str) -> HttpRequest { + HttpRequest { + http_method: HttpMethod::Post, + url: Some(Url::from_path(path)), } } +/// An Authentication event for `user`'s request to `path` from `origin`, +/// naming the protocol of `mechanism` when known. +fn authentication<'a>( + ctx: &'a EventContext, + path: &str, + origin: &RequestOrigin, + user: User, + mechanism: Option, +) -> AuthenticationBuilder<'a> { + let mut builder = AuthenticationBuilder::new(ctx, user, Service::new("openshell-gateway")) + .http_request(post(path)); + match mechanism { + Some(AuthMechanism::Oidc) => builder = builder.auth_protocol(AuthProtocolId::OpenId), + Some(mechanism) => builder = builder.auth_protocol_other(mechanism.as_str()), + None => {} + } + if let Some(source) = origin.source { + builder = builder.src_endpoint(source.endpoint()); + } + if let Some(request_id) = &origin.request_id { + builder = builder.correlation_uid(request_id); + } + builder +} + /// Emit an Authentication failure event for a gRPC request to `path`. pub fn emit_authentication_failure( path: &str, @@ -116,20 +122,21 @@ pub fn emit_authentication_failure( } /// Emit an Authentication failure event for a credential that a handler -/// verifies itself, after the router established `principal`. -pub fn emit_credential_failure( - principal: &Principal, - mechanism: AuthMechanism, - reason: AuthFailureReason, -) { +/// verifies itself, returning the status that refuses the current request. +/// +/// The event names `principal`, the caller the router established from a +/// separate credential, or leaves the user unidentified when the rejected +/// credential is the one that named the caller. +pub fn reject_credential(principal: Option<&Principal>, rejection: AuthRejection) -> Status { let (path, origin) = current_request(); authentication_failure( &path, &origin, - principal_user(principal), - Some(mechanism), - reason, + principal.map_or_else(User::unidentified, principal_user), + Some(rejection.mechanism), + rejection.reason, ); + rejection.status } fn authentication_failure( @@ -140,26 +147,13 @@ fn authentication_failure( reason: AuthFailureReason, ) { let ctx = crate::gateway_ocsf::context("", ""); - let mut builder = AuthenticationBuilder::new(&ctx, user, Service::new("openshell-gateway")) + let builder = authentication(&ctx, path, origin, user, mechanism) .action(ActionId::Denied) .disposition(DispositionId::Blocked) .severity(severity(reason)) .status(StatusId::Failure) - .status_detail(category(reason)) - .http_request(HttpRequest { - http_method: HttpMethod::Post, - url: Some(Url::from_path(path)), - }) - .message(format!("authentication failed: {}", category(reason))); - if let Some(mechanism) = mechanism { - builder = with_protocol(builder, mechanism); - } - if let Some(source) = origin.source { - builder = builder.src_endpoint(source.endpoint()); - } - if let Some(request_id) = &origin.request_id { - builder = builder.correlation_uid(request_id); - } + .status_detail(reason.as_str()) + .message(format!("authentication failed: {}", reason.as_str())); ocsf_emit!(builder.build()); } @@ -271,7 +265,7 @@ fn digest( let mut material = principal.audit_actor().into_bytes(); material.push(0); if let Some(mechanism) = mechanism { - material.extend_from_slice(format!("{mechanism:?}").as_bytes()); + material.extend_from_slice(mechanism.as_str().as_bytes()); material.push(0); } material.extend_from_slice(credential.as_bytes()); @@ -355,29 +349,12 @@ fn logon( mechanism: Option, ) { let ctx = crate::gateway_ocsf::context("", ""); - let mut builder = AuthenticationBuilder::new( - &ctx, - principal_user(principal), - Service::new("openshell-gateway"), - ) - .action(ActionId::Allowed) - .disposition(DispositionId::Allowed) - .severity(SeverityId::Informational) - .status(StatusId::Success) - .http_request(HttpRequest { - http_method: HttpMethod::Post, - url: Some(Url::from_path(path)), - }) - .message("authentication succeeded"); - if let Some(mechanism) = mechanism { - builder = with_protocol(builder, mechanism); - } - if let Some(source) = origin.source { - builder = builder.src_endpoint(source.endpoint()); - } - if let Some(request_id) = &origin.request_id { - builder = builder.correlation_uid(request_id); - } + let builder = authentication(&ctx, path, origin, principal_user(principal), mechanism) + .action(ActionId::Allowed) + .disposition(DispositionId::Allowed) + .severity(SeverityId::Informational) + .status(StatusId::Success) + .message("authentication succeeded"); ocsf_emit!(builder.build()); } @@ -536,6 +513,13 @@ pub fn emit_authorization_denial(principal: &Principal, denial: AuthzDenial<'_>) emit_authorization_denial_for(&path, &origin, principal, denial); } +/// Emit an authorization denial raised by a handler for the current request, +/// returning the status that refuses it with `message`. +pub fn deny(principal: &Principal, denial: AuthzDenial<'_>, message: impl Into) -> Status { + emit_authorization_denial(principal, denial); + Status::permission_denied(message) +} + /// Emit an API Activity denial for a gRPC request to `path`, paired with a /// Detection Finding when a sandbox principal crossed its boundary. pub fn emit_authorization_denial_for( @@ -573,10 +557,7 @@ pub fn emit_authorization_denial_for( if !service.is_empty() { builder = builder .api_service(Service::new(service)) - .http_request(HttpRequest { - http_method: HttpMethod::Post, - url: Some(Url::from_path(path)), - }); + .http_request(post(path)); } if let Some(resource) = denial.resource() { builder = builder.resource(resource); diff --git a/crates/openshell-server/src/auth/authenticator.rs b/crates/openshell-server/src/auth/authenticator.rs index b94cdd596..2527b7b43 100644 --- a/crates/openshell-server/src/auth/authenticator.rs +++ b/crates/openshell-server/src/auth/authenticator.rs @@ -91,6 +91,23 @@ pub enum AuthFailureReason { IdentityProviderUnavailable, } +impl AuthFailureReason { + /// The category recorded in audit events. + pub fn as_str(self) -> &'static str { + match self { + Self::MissingCredential => "missing_credential", + Self::ConflictingCredentials => "conflicting_credentials", + Self::MalformedCredential => "malformed_credential", + Self::InvalidCredential => "invalid_credential", + Self::ExpiredCredential => "expired_credential", + Self::AudienceOrIssuerMismatch => "audience_or_issuer_mismatch", + Self::UnknownIdentity => "unknown_identity", + Self::StaleIdentity => "stale_identity", + Self::IdentityProviderUnavailable => "identity_provider_unavailable", + } + } +} + /// The mechanism a caller attempted to authenticate with. #[derive(Debug, Clone, Copy, PartialEq, Eq)] pub enum AuthMechanism { @@ -103,6 +120,21 @@ pub enum AuthMechanism { SshSessionToken, } +impl AuthMechanism { + /// A stable label for the mechanism. Audit events record it as the + /// protocol, except OIDC, which they record as the OCSF `OpenID` protocol. + pub fn as_str(self) -> &'static str { + match self { + Self::Mtls => "mtls", + Self::Oidc => "oidc", + Self::SandboxJwt => "sandbox_jwt", + Self::ComputeDriver => "compute_driver", + Self::GatewayPeer => "k8s_service_account", + Self::SshSessionToken => "ssh_session_token", + } + } +} + /// A rejected authentication attempt: the status returned to the caller and /// the category recorded in the audit event. #[derive(Debug, Clone)] diff --git a/crates/openshell-server/src/auth/guard.rs b/crates/openshell-server/src/auth/guard.rs index 53dd73251..cfea707e9 100644 --- a/crates/openshell-server/src/auth/guard.rs +++ b/crates/openshell-server/src/auth/guard.rs @@ -8,7 +8,7 @@ //! `sandbox_id`. User principals retain the broad scope the RBAC layer //! already evaluated. -use super::audit::{AuthzDenial, emit_authorization_denial}; +use super::audit::{AuthzDenial, deny}; use super::principal::Principal; use super::principal::SandboxPrincipal; use tonic::Status; @@ -30,23 +30,20 @@ use tonic::Status; pub fn ensure_sandbox_scope(principal: &Principal, claimed_sandbox_id: &str) -> Result<(), Status> { match principal { Principal::User(_) | Principal::Operator(_) => Ok(()), - Principal::Peer(_) => { - emit_authorization_denial(principal, AuthzDenial::MethodNotPermitted); - Err(Status::permission_denied( - "gateway peer principals may not call sandbox-scoped methods", - )) - } + Principal::Peer(_) => Err(deny( + principal, + AuthzDenial::MethodNotPermitted, + "gateway peer principals may not call sandbox-scoped methods", + )), Principal::Sandbox(p) => { if p.sandbox_id == claimed_sandbox_id { Ok(()) } else { - emit_authorization_denial( + Err(deny( principal, AuthzDenial::CrossSandbox { sandbox_id: claimed_sandbox_id, }, - ); - Err(Status::permission_denied( "cross-sandbox access denied: principal does not own this sandbox", )) } @@ -91,12 +88,11 @@ pub fn ensure_sandbox_principal_scope( ensure_sandbox_scope(principal, claimed_sandbox_id)?; Ok(p.clone()) } - Principal::User(_) | Principal::Operator(_) | Principal::Peer(_) => { - emit_authorization_denial(principal, AuthzDenial::MethodNotPermitted); - Err(Status::permission_denied( - "supervisor RPCs require a sandbox principal", - )) - } + Principal::User(_) | Principal::Operator(_) | Principal::Peer(_) => Err(deny( + principal, + AuthzDenial::MethodNotPermitted, + "supervisor RPCs require a sandbox principal", + )), Principal::Anonymous => Err(Status::unauthenticated( "supervisor RPCs require an authenticated sandbox principal", )), @@ -123,10 +119,11 @@ pub fn require_operator( None | Some(Principal::Anonymous) => { Err(Status::unauthenticated("operator authentication required")) } - Some(principal) => { - emit_authorization_denial(principal, AuthzDenial::OperatorRequired); - Err(Status::permission_denied("verified mTLS operator required")) - } + Some(principal) => Err(deny( + principal, + AuthzDenial::OperatorRequired, + "verified mTLS operator required", + )), } } diff --git a/crates/openshell-server/src/auth/workspace_authz.rs b/crates/openshell-server/src/auth/workspace_authz.rs index fe1d4c017..2ea3dda3c 100644 --- a/crates/openshell-server/src/auth/workspace_authz.rs +++ b/crates/openshell-server/src/auth/workspace_authz.rs @@ -8,7 +8,7 @@ //! auth mode + scope + global role; this module validates workspace membership //! and workspace-level role. -use super::audit::{AuthzDenial, emit_authorization_denial}; +use super::audit::{AuthzDenial, deny}; use super::principal::Principal; use openshell_core::proto::{ WorkspaceRole as ProtoWorkspaceRole, WorkspaceSelector, @@ -159,39 +159,39 @@ pub async fn authorize_workspace( .map_err(|e| Status::internal(format!("membership lookup failed: {e}")))?; let Some(member) = member else { - emit_authorization_denial( + let workspace_arg = shell_quote_for_hint(&workspace); + let subject_arg = shell_quote_for_hint(&user.identity.subject); + return Err(deny( principal, AuthzDenial::NotWorkspaceMember { workspace: &workspace, }, - ); - let workspace_arg = shell_quote_for_hint(&workspace); - let subject_arg = shell_quote_for_hint(&user.identity.subject); - return Err(Status::permission_denied(format!( - "not a member of workspace '{workspace}'; ask a platform admin to run: \ - openshell workspace member add --workspace {workspace_arg} \ - --subject {subject_arg} --role user" - ))); + format!( + "not a member of workspace '{workspace}'; ask a platform admin to run: \ + openshell workspace member add --workspace {workspace_arg} \ + --subject {subject_arg} --role user" + ), + )); }; let member_role = ProtoWorkspaceRole::try_from(member.role) .unwrap_or(ProtoWorkspaceRole::Unspecified); if !role_satisfies(member_role, min_role) { - emit_authorization_denial( + let workspace_arg = shell_quote_for_hint(&workspace); + let subject_arg = shell_quote_for_hint(&user.identity.subject); + let role = min_role.as_str(); + return Err(deny( principal, AuthzDenial::InsufficientWorkspaceRole { workspace: &workspace, }, - ); - let workspace_arg = shell_quote_for_hint(&workspace); - let subject_arg = shell_quote_for_hint(&user.identity.subject); - let role = min_role.as_str(); - return Err(Status::permission_denied(format!( - "workspace role '{role}' required in workspace '{workspace}'; ask a platform \ - admin to run: openshell workspace member add --workspace {workspace_arg} \ - --subject {subject_arg} --role {role}" - ))); + format!( + "workspace role '{role}' required in workspace '{workspace}'; ask a \ + platform admin to run: openshell workspace member add --workspace \ + {workspace_arg} --subject {subject_arg} --role {role}" + ), + )); } Ok(AuthorizedWorkspace { @@ -207,12 +207,11 @@ pub async fn authorize_workspace( workspace, grant: AuthGrant::Sandbox, }), - Principal::Peer(_) => { - emit_authorization_denial(principal, AuthzDenial::MethodNotPermitted); - Err(Status::permission_denied( - "gateway peer principals cannot perform workspace operations", - )) - } + Principal::Peer(_) => Err(deny( + principal, + AuthzDenial::MethodNotPermitted, + "gateway peer principals cannot perform workspace operations", + )), Principal::Anonymous => Err(Status::unauthenticated("authentication required")), } } @@ -248,8 +247,7 @@ pub fn require_platform_admin(admin_role: &str, principal: &Principal) -> Result Principal::Peer(_) => "gateway peer principals cannot perform cross-workspace operations", Principal::Anonymous => return Err(Status::unauthenticated("authentication required")), }; - emit_authorization_denial(principal, AuthzDenial::PlatformAdminRequired); - Err(Status::permission_denied(message)) + Err(deny(principal, AuthzDenial::PlatformAdminRequired, message)) } /// Check whether the caller's OIDC roles include the platform admin role. diff --git a/crates/openshell-server/src/grpc/auth_rpc.rs b/crates/openshell-server/src/grpc/auth_rpc.rs index b53329a07..b635bf041 100644 --- a/crates/openshell-server/src/grpc/auth_rpc.rs +++ b/crates/openshell-server/src/grpc/auth_rpc.rs @@ -15,11 +15,10 @@ //! ordinary RPCs or select another successor. use crate::ServerState; -use crate::auth::audit::{self, AuthzDenial, ConnectionSource, RequestOrigin}; +use crate::auth::audit::{self, AuthzDenial}; use crate::auth::authenticator::{AuthFailureReason, AuthMechanism, AuthRejection}; use crate::auth::identity::IdentityProvider; use crate::auth::principal::{Principal, SandboxIdentitySource, SandboxPrincipal}; -use crate::auth::sandbox_jwt::REFRESH_SANDBOX_TOKEN_PATH; use openshell_core::proto::{ ExtensionServiceCredential, GetCurrentUserRequest, GetCurrentUserResponse, GetSandboxConfigRequest, IssueSandboxTokenRequest, IssueSandboxTokenResponse, @@ -38,8 +37,9 @@ pub async fn handle_get_current_user( ) -> Result, Status> { let principal = super::extract_principal(&request)?; let Some(identity) = principal.user_identity().cloned() else { - audit::emit_authorization_denial(&principal, AuthzDenial::MethodNotPermitted); - return Err(Status::permission_denied( + return Err(audit::deny( + &principal, + AuthzDenial::MethodNotPermitted, "GetCurrentUser requires a user principal", )); }; @@ -71,8 +71,9 @@ pub async fn handle_issue_sandbox_token( .ok_or_else(|| Status::unauthenticated("missing principal"))?; let Principal::Sandbox(sandbox) = &principal else { - audit::emit_authorization_denial(&principal, AuthzDenial::MethodNotPermitted); - return Err(Status::permission_denied( + return Err(audit::deny( + &principal, + AuthzDenial::MethodNotPermitted, "IssueSandboxToken requires a sandbox principal", )); }; @@ -88,8 +89,9 @@ pub async fn handle_issue_sandbox_token( sandbox_id = %sandbox.sandbox_id, "IssueSandboxToken rejected: non-bootstrap principal source" ); - audit::emit_authorization_denial(&principal, AuthzDenial::MethodNotPermitted); - return Err(Status::permission_denied( + return Err(audit::deny( + &principal, + AuthzDenial::MethodNotPermitted, "this principal cannot mint a sandbox token; use RefreshSandboxToken", )); }; @@ -107,24 +109,26 @@ pub async fn handle_issue_sandbox_token( // The driver vouched for a runtime; it must be the one recorded at // creation, so these refusals are authentication failures. - let reject = |reason, message: &str| { - audit::emit_credential_failure(&principal, AuthMechanism::ComputeDriver, reason); - Status::permission_denied(message) + let reject = |reason, status| { + audit::reject_credential( + Some(&principal), + AuthRejection::new(AuthMechanism::ComputeDriver, reason, status), + ) }; let sandbox_record = ensure_sandbox_exists(state, &sandbox.sandbox_id) .await - .inspect_err(|status| { + .map_err(|status| { let reason = if status.code() == tonic::Code::Internal { AuthFailureReason::IdentityProviderUnavailable } else { AuthFailureReason::UnknownIdentity }; - audit::emit_credential_failure(&principal, AuthMechanism::ComputeDriver, reason); + reject(reason, status) })?; let metadata = sandbox_record.metadata.as_ref().ok_or_else(|| { reject( AuthFailureReason::UnknownIdentity, - "sandbox runtime identity is unavailable", + Status::permission_denied("sandbox runtime identity is unavailable"), ) })?; let expected_driver = metadata @@ -141,7 +145,7 @@ pub async fn handle_issue_sandbox_token( ); return Err(reject( AuthFailureReason::StaleIdentity, - "compute runtime identity does not match the sandbox", + Status::permission_denied("compute runtime identity does not match the sandbox"), )); } @@ -150,7 +154,7 @@ pub async fn handle_issue_sandbox_token( .map_err(|_| { reject( AuthFailureReason::UnknownIdentity, - "sandbox runtime identity is invalid", + Status::permission_denied("sandbox runtime identity is invalid"), ) })?; audit::emit_handler_logon(&state.logons, &principal, request.metadata().as_ref()); @@ -251,8 +255,9 @@ pub async fn handle_refresh_sandbox_token( .ok_or_else(|| Status::unauthenticated("missing principal"))?; let Principal::Sandbox(sandbox) = principal else { - audit::emit_authorization_denial(&principal, AuthzDenial::MethodNotPermitted); - return Err(Status::permission_denied( + return Err(audit::deny( + &principal, + AuthzDenial::MethodNotPermitted, "RefreshSandboxToken requires a sandbox principal", )); }; @@ -264,11 +269,9 @@ pub async fn handle_refresh_sandbox_token( sandbox_id = %sandbox.sandbox_id, "RefreshSandboxToken rejected: non-gateway-JWT principal source" ); - audit::emit_authorization_denial( + return Err(audit::deny( &Principal::Sandbox(sandbox.clone()), AuthzDenial::MethodNotPermitted, - ); - return Err(Status::permission_denied( "this principal cannot refresh; use IssueSandboxToken for bootstrap", )); }; @@ -299,18 +302,9 @@ pub async fn handle_refresh_sandbox_token( issued_at, ) .await - .map_err(|rejection| { - audit::emit_authentication_failure( - REFRESH_SANDBOX_TOKEN_PATH, - &RequestOrigin::new( - request.extensions().get::().copied(), - request.metadata().as_ref(), - ), - Some(rejection.mechanism), - rejection.reason, - ); - rejection.status - })?; + // The rejected bearer is what named the principal, so the event leaves + // the user unidentified. + .map_err(|rejection| audit::reject_credential(None, rejection))?; audit::emit_handler_logon( &state.logons, &Principal::Sandbox(sandbox.clone()), @@ -464,13 +458,11 @@ fn mint_extension_credentials( .iter() .map(|name| { let service = available.get(name.as_str()).ok_or_else(|| { - audit::emit_authorization_denial( + audit::deny( &Principal::Sandbox(sandbox.clone()), AuthzDenial::ExtensionServiceNotSelected { service: name }, - ); - Status::permission_denied(format!( - "extension service '{name}' is not selected by the sandbox policy" - )) + format!("extension service '{name}' is not selected by the sandbox policy"), + ) })?; if service.allow_insecure_transport { return Err(Status::failed_precondition(format!( @@ -534,10 +526,13 @@ async fn ensure_sandbox_exists( mod tests { use super::*; use crate::ServerState; + use crate::auth::audit::{ConnectionSource, RequestOrigin}; use crate::auth::compute_driver::ISSUE_SANDBOX_TOKEN_PATH; use crate::auth::identity::Identity; use crate::auth::principal::{Principal, SandboxPrincipal, UserPrincipal}; - use crate::auth::sandbox_jwt::{ExtensionJwtIssuer, SandboxSessionJwtAuthority}; + use crate::auth::sandbox_jwt::{ + ExtensionJwtIssuer, REFRESH_SANDBOX_TOKEN_PATH, SandboxSessionJwtAuthority, + }; use crate::compute::new_test_runtime; use crate::persistence::Store; use crate::sandbox_index::SandboxIndex; @@ -856,8 +851,6 @@ mod tests { #[tokio::test] async fn refresh_with_a_consumed_bearer_emits_stale_identity() { - use openshell_ocsf::test_support::OcsfCapture; - let state = state_with_issuer().await; let request = || { let mut request = Request::new(RefreshSandboxTokenRequest { @@ -880,33 +873,20 @@ mod tests { .await .expect("successor refresh"); - let capture = OcsfCapture::default(); - let error = { - let _guard = capture.install(); - let mut replay = request(); - set_refresh_authorization(&mut replay, &first); - replay.extensions_mut().insert(ConnectionSource::Peer( - "203.0.113.7:51234".parse().expect("peer address"), - )); - replay.metadata_mut().insert( - "x-request-id", - "6f1c2d3e-4b5a-4c6d-8e7f-9a0b1c2d3e4f".parse().unwrap(), - ); - handle_refresh_sandbox_token(&state, replay) - .await - .expect_err("consumed predecessor must be rejected") - }; + let mut replay = request(); + set_refresh_authorization(&mut replay, &first); + let (result, events) = audited( + REFRESH_SANDBOX_TOKEN_PATH, + Box::pin(handle_refresh_sandbox_token(&state, replay)), + ) + .await; + let error = result.expect_err("consumed predecessor must be rejected"); assert_eq!(error.code(), tonic::Code::Unauthenticated); assert_eq!( error.message(), "gateway refresh retry does not match the active lineage" ); - let events: Vec<_> = capture - .events() - .iter() - .map(|event| event.to_json().unwrap()) - .collect(); assert_eq!(events.len(), 1, "{events:?}"); assert_eq!(events[0]["class_uid"], 3002); assert_eq!(events[0]["status_detail"], "stale_identity"); @@ -916,10 +896,7 @@ mod tests { REFRESH_SANDBOX_TOKEN_PATH ); assert_eq!(events[0]["src_endpoint"]["ip"], "203.0.113.7"); - assert_eq!( - events[0]["metadata"]["correlation_uid"], - "6f1c2d3e-4b5a-4c6d-8e7f-9a0b1c2d3e4f" - ); + assert_eq!(events[0]["metadata"]["correlation_uid"], REQUEST_ID); assert!(!events[0].to_string().contains(&first)); } diff --git a/crates/openshell-server/src/grpc/mutation_replay.rs b/crates/openshell-server/src/grpc/mutation_replay.rs index 9cfb799c4..1a39b3dea 100644 --- a/crates/openshell-server/src/grpc/mutation_replay.rs +++ b/crates/openshell-server/src/grpc/mutation_replay.rs @@ -157,11 +157,9 @@ pub(super) async fn run( let request_id = validate_request_id(request.get_ref().request_id())?; let principal = super::extract_principal(&request)?; let Some(identity) = principal.user_identity() else { - crate::auth::audit::emit_authorization_denial( + return Err(crate::auth::audit::deny( &principal, crate::auth::audit::AuthzDenial::MethodNotPermitted, - ); - return Err(Status::permission_denied( "request admission requires a user principal", )); }; diff --git a/crates/openshell-server/src/grpc/sandbox.rs b/crates/openshell-server/src/grpc/sandbox.rs index 9e3f51281..eae8f3fa2 100644 --- a/crates/openshell-server/src/grpc/sandbox.rs +++ b/crates/openshell-server/src/grpc/sandbox.rs @@ -2747,13 +2747,13 @@ async fn acquire_forward_connection_guard( let token = init.authorization_token.trim(); if token.is_empty() { if !matches!(init.target, Some(tcp_forward_init::Target::Tcp(_))) { - crate::auth::audit::emit_credential_failure( - principal, - AuthMechanism::SshSessionToken, - AuthFailureReason::MissingCredential, - ); - return Err(Status::unauthenticated( - "authorization_token is required for SSH targets", + return Err(crate::auth::audit::reject_credential( + Some(principal), + AuthRejection::new( + AuthMechanism::SshSessionToken, + AuthFailureReason::MissingCredential, + Status::unauthenticated("authorization_token is required for SSH targets"), + ), )); } acquire_sandbox_connection_slot(&state.ssh_connections_by_sandbox, &sandbox_id)?; @@ -2766,14 +2766,7 @@ async fn acquire_forward_connection_guard( validate_ssh_forward_token(state, token, &sandbox_id) .await - .map_err(|rejection| { - crate::auth::audit::emit_credential_failure( - principal, - rejection.mechanism, - rejection.reason, - ); - rejection.status - })?; + .map_err(|rejection| crate::auth::audit::reject_credential(Some(principal), rejection))?; crate::auth::audit::emit_ssh_session_logon(&state.logons, principal, token); acquire_ssh_connection_slots( &state.ssh_connections_by_token, diff --git a/crates/openshell-server/src/grpc/workspace.rs b/crates/openshell-server/src/grpc/workspace.rs index 5d6103611..d2063e988 100644 --- a/crates/openshell-server/src/grpc/workspace.rs +++ b/crates/openshell-server/src/grpc/workspace.rs @@ -469,11 +469,9 @@ pub(super) fn authorize_member_role( )); } if role == WorkspaceRole::Admin && grant != AuthGrant::PlatformAdmin { - crate::auth::audit::emit_authorization_denial( + return Err(crate::auth::audit::deny( principal, crate::auth::audit::AuthzDenial::PlatformAdminRequired, - ); - return Err(Status::permission_denied( "only platform admins can assign the workspace admin role", )); }