From 2f849f3bf406cdf412a35c734f2aef362ef067bb Mon Sep 17 00:00:00 2001 From: s-zaizen Date: Sun, 21 Jun 2026 21:35:12 +0900 Subject: [PATCH] Enforce repository write permissions Require explicit write, admin, or owner repository permissions before mutation paths across gRPC, HTTP, protocol storage, and QUIC. Keep read authorization requiring read-like permissions so empty resource claims cannot grant access. Signed-off-by: s-zaizen --- lore-server/src/auth/jwt.rs | 100 +++++++++++++++++- .../src/grpc/handlers/branch_create.rs | 2 + .../src/grpc/handlers/branch_delete.rs | 2 + .../src/grpc/handlers/branch_metadata_set.rs | 2 + .../src/grpc/handlers/branch_protect.rs | 2 + lore-server/src/grpc/handlers/branch_push.rs | 2 + .../src/grpc/handlers/branch_unprotect.rs | 2 + .../grpc/handlers/repository_metadata_set.rs | 7 +- lore-server/src/grpc/mod.rs | 20 ++++ .../repository/v1/repository_metadata_set.rs | 7 +- .../src/grpc/revision/v1/branch_create.rs | 57 ++++++++++ .../src/grpc/revision/v1/branch_delete.rs | 2 + .../grpc/revision/v1/branch_metadata_set.rs | 2 + .../src/grpc/revision/v1/branch_push.rs | 6 ++ lore-server/src/grpc/storage/v1/copy.rs | 2 + .../storage/v1/mutable_compare_and_swap.rs | 2 + .../src/grpc/storage/v1/mutable_store.rs | 2 + lore-server/src/grpc/storage/v1/put.rs | 2 + .../src/grpc/thinclient/v1/revision_tree.rs | 2 +- .../content/presign_repository_content.rs | 9 ++ .../contents/put_repository_content.rs | 28 +++-- lore-server/src/protocol/storage/copy.rs | 3 + .../src/protocol/storage/mutable_cas.rs | 6 ++ .../protocol/storage/mutable_store_handler.rs | 6 ++ lore-server/src/protocol/storage/put.rs | 6 ++ lore-server/src/protocol/storage/session.rs | 24 +++++ lore-server/src/quic/storage_service_v4.rs | 31 +++++- lore-server/src/quic/stream_handler.rs | 8 +- 28 files changed, 320 insertions(+), 24 deletions(-) diff --git a/lore-server/src/auth/jwt.rs b/lore-server/src/auth/jwt.rs index d2f1eb2..f7a3f9a 100644 --- a/lore-server/src/auth/jwt.rs +++ b/lore-server/src/auth/jwt.rs @@ -18,6 +18,11 @@ use tracing::warn; use super::jwk::JWKServiceError; use crate::auth::jwk::JWKService; +pub const PERMISSION_ADMIN: &str = "admin"; +pub const PERMISSION_OWNER: &str = "owner"; +pub const PERMISSION_READ: &str = "read"; +pub const PERMISSION_WRITE: &str = "write"; + #[serde_as] #[derive(Debug, Deserialize, Clone, Serialize, PartialEq)] pub struct JWTUserInfo { @@ -53,6 +58,12 @@ impl ResourcePermission { pub fn matches_repository(&self, repository_id: &String) -> bool { self.resource_id == *repository_id || self.is_wildcard_resource() } + + pub fn has_any_permission(&self, permissions: &[&str]) -> bool { + permissions + .iter() + .any(|permission| self.permission.iter().any(|item| item == permission)) + } } #[serde_as] @@ -168,15 +179,67 @@ pub fn verify_authorization( authorization: &AuthorizationToken, repository: lore_revision::lore::RepositoryId, ) -> Result<(), JwtVerifierError> { + if has_repository_read_permission(authorization, repository) { + return Ok(()); + } + + Err(JwtVerifierError::NotAuthorized) +} + +pub fn has_repository_read_permission( + authorization: &AuthorizationToken, + repository: lore_revision::lore::RepositoryId, +) -> bool { + has_any_repository_permission( + authorization, + repository, + &[ + PERMISSION_READ, + PERMISSION_WRITE, + PERMISSION_ADMIN, + PERMISSION_OWNER, + ], + ) +} + +pub fn has_repository_write_permission( + authorization: &AuthorizationToken, + repository: lore_revision::lore::RepositoryId, +) -> bool { + has_any_repository_permission( + authorization, + repository, + &[PERMISSION_WRITE, PERMISSION_ADMIN, PERMISSION_OWNER], + ) +} + +pub fn has_any_repository_permission( + authorization: &AuthorizationToken, + repository: lore_revision::lore::RepositoryId, + permissions: &[&str], +) -> bool { if let Some(resources) = authorization.resources.as_ref() { let checked_repository = format!("urc-{repository}"); for authorized_resource in resources.iter() { - if authorized_resource.matches_repository(&checked_repository) { - return Ok(()); + if authorized_resource.matches_repository(&checked_repository) + && authorized_resource.has_any_permission(permissions) + { + return true; } } } + false +} + +pub fn verify_repository_write_authorization( + authorization: &AuthorizationToken, + repository: lore_revision::lore::RepositoryId, +) -> Result<(), JwtVerifierError> { + if has_repository_write_permission(authorization, repository) { + return Ok(()); + } + Err(JwtVerifierError::NotAuthorized) } @@ -227,7 +290,7 @@ mod tests { fn verify_authorization_allows_repo_from_token() { let allowed_repository_id = "urc-0194b726b34e72b0b45550b88a967076".to_string(); let resource_permission = ResourcePermission { - permission: vec![], + permission: vec![PERMISSION_READ.to_string()], resource_id: allowed_repository_id.clone(), }; let authorization_token = AuthorizationToken { @@ -259,7 +322,7 @@ mod tests { #[test] fn verify_authorization_allows_all_repos_for_wildcard_token() { let resource_permission = ResourcePermission { - permission: vec![], + permission: vec![PERMISSION_READ.to_string()], resource_id: "urc-*".to_string(), }; let wildcard_authorization_token = AuthorizationToken { @@ -294,6 +357,35 @@ mod tests { } } + #[test] + fn verify_authorization_rejects_repo_without_read_permission() { + let repository_id = "urc-0194b726b34e72b0b45550b88a967076".to_string(); + let resource_permission = ResourcePermission { + permission: vec![], + resource_id: repository_id.clone(), + }; + let authorization_token = AuthorizationToken { + audience: vec!["test".to_string()], + env: "test".to_string(), + expires: 1234, + user_id: "test".to_string(), + idp: "test".to_string(), + issuer: "test".to_string(), + name: "test".to_string(), + preferred_username: "test".to_string(), + groups: None, + is_service_account: Some(false), + issued_at: 123, + resources: Some(vec![resource_permission]), + }; + let repository: RepositoryId = Context::from_str("0194b726b34e72b0b45550b88a967076") + .unwrap() + .into(); + + verify_authorization(&authorization_token, repository) + .expect_err("resource without read-like permission must be rejected"); + } + mod jwt_verifier { use std::error::Error; diff --git a/lore-server/src/grpc/handlers/branch_create.rs b/lore-server/src/grpc/handlers/branch_create.rs index eee6758..5f43308 100644 --- a/lore-server/src/grpc/handlers/branch_create.rs +++ b/lore-server/src/grpc/handlers/branch_create.rs @@ -23,6 +23,7 @@ use crate::grpc::get_repository; use crate::grpc::get_user_id; use crate::grpc::get_write_token; use crate::grpc::hook_error_to_status; +use crate::grpc::require_write_permission; use crate::hooks::HookContext; use crate::hooks::HookDispatcher; use crate::hooks::HookPoint; @@ -66,6 +67,7 @@ pub async fn handler( instrument_provider: &impl InstrumentProvider, ) -> Result, Status> { let repository_id = get_repository(request.metadata())?; + require_write_permission(request.extensions(), repository_id)?; let user_id = get_user_id(request.extensions()); let correlation_id = extract_correlation_id(&request).unwrap_or_default(); let req = request.into_inner(); diff --git a/lore-server/src/grpc/handlers/branch_delete.rs b/lore-server/src/grpc/handlers/branch_delete.rs index 8e2a782..71cc0ec 100644 --- a/lore-server/src/grpc/handlers/branch_delete.rs +++ b/lore-server/src/grpc/handlers/branch_delete.rs @@ -22,6 +22,7 @@ use crate::grpc::extract_correlation_id; use crate::grpc::get_repository; use crate::grpc::get_user_id; use crate::grpc::hook_error_to_status; +use crate::grpc::require_write_permission; use crate::hooks::HookContext; use crate::hooks::HookDispatcher; use crate::hooks::HookPoint; @@ -37,6 +38,7 @@ pub async fn handler( instrument_provider: &impl InstrumentProvider, ) -> Result, Status> { let repository_id = get_repository(request.metadata())?; + require_write_permission(request.extensions(), repository_id)?; let user_id = get_user_id(request.extensions()); let correlation_id = extract_correlation_id(&request).unwrap_or_default(); let req = request.into_inner(); diff --git a/lore-server/src/grpc/handlers/branch_metadata_set.rs b/lore-server/src/grpc/handlers/branch_metadata_set.rs index 08d5baf..67d98e2 100644 --- a/lore-server/src/grpc/handlers/branch_metadata_set.rs +++ b/lore-server/src/grpc/handlers/branch_metadata_set.rs @@ -23,6 +23,7 @@ use crate::grpc::extract_correlation_id; use crate::grpc::get_repository; use crate::grpc::get_user_id; use crate::grpc::get_write_token; +use crate::grpc::require_write_permission; use crate::grpc::warn_error_to_status; use crate::util::setup_execution; @@ -100,6 +101,7 @@ pub async fn handler( mutable_store: Arc, ) -> Result, Status> { let repository_id = get_repository(request.metadata())?; + require_write_permission(request.extensions(), repository_id)?; let user_id = get_user_id(request.extensions()); let correlation_id = extract_correlation_id(&request).unwrap_or_default(); let req = request.into_inner(); diff --git a/lore-server/src/grpc/handlers/branch_protect.rs b/lore-server/src/grpc/handlers/branch_protect.rs index 695e1a3..aeb1d15 100644 --- a/lore-server/src/grpc/handlers/branch_protect.rs +++ b/lore-server/src/grpc/handlers/branch_protect.rs @@ -17,6 +17,7 @@ use tracing::warn; use crate::grpc::extract_correlation_id; use crate::grpc::get_repository; use crate::grpc::get_user_id; +use crate::grpc::require_write_permission; use crate::util::setup_execution; #[tracing::instrument(name = "BranchProtect::handle", skip_all)] @@ -26,6 +27,7 @@ pub async fn handler( mutable_store: Arc, ) -> Result, Status> { let repository_id = get_repository(request.metadata())?; + require_write_permission(request.extensions(), repository_id)?; let user_id = get_user_id(request.extensions()); let correlation_id = extract_correlation_id(&request).unwrap_or_default(); let req = request.into_inner(); diff --git a/lore-server/src/grpc/handlers/branch_push.rs b/lore-server/src/grpc/handlers/branch_push.rs index 0cc52d0..6e5952e 100644 --- a/lore-server/src/grpc/handlers/branch_push.rs +++ b/lore-server/src/grpc/handlers/branch_push.rs @@ -48,6 +48,7 @@ use crate::grpc::get_repository; use crate::grpc::get_user_id; use crate::grpc::get_write_token; use crate::grpc::hook_error_to_status; +use crate::grpc::require_write_permission; use crate::grpc::warn_error_to_status; use crate::hooks::HookContext; use crate::hooks::HookDispatcher; @@ -87,6 +88,7 @@ pub async fn handler( let user_id = get_user_id(request.extensions()); let correlation_id = extract_correlation_id(&request).unwrap_or_default(); let repository = get_repository(request.metadata())?; + require_write_permission(request.extensions(), repository)?; // TODO(mjansson): Once we have authz permission model with read/write/admin // this should be upgraded to check for the correct permission rather than diff --git a/lore-server/src/grpc/handlers/branch_unprotect.rs b/lore-server/src/grpc/handlers/branch_unprotect.rs index a281c7a..83da32a 100644 --- a/lore-server/src/grpc/handlers/branch_unprotect.rs +++ b/lore-server/src/grpc/handlers/branch_unprotect.rs @@ -17,6 +17,7 @@ use tracing::warn; use crate::grpc::extract_correlation_id; use crate::grpc::get_repository; use crate::grpc::get_user_id; +use crate::grpc::require_write_permission; use crate::util::setup_execution; #[tracing::instrument(name = "BranchUnprotect::handle", skip_all)] @@ -26,6 +27,7 @@ pub async fn handler( mutable_store: Arc, ) -> Result, Status> { let repository_id = get_repository(request.metadata())?; + require_write_permission(request.extensions(), repository_id)?; let user_id = get_user_id(request.extensions()); let correlation_id = extract_correlation_id(&request).unwrap_or_default(); let req = request.into_inner(); diff --git a/lore-server/src/grpc/handlers/repository_metadata_set.rs b/lore-server/src/grpc/handlers/repository_metadata_set.rs index a4d475c..09d24fe 100644 --- a/lore-server/src/grpc/handlers/repository_metadata_set.rs +++ b/lore-server/src/grpc/handlers/repository_metadata_set.rs @@ -22,6 +22,7 @@ use tonic::Status; use crate::grpc::extract_correlation_id; use crate::grpc::get_user_id; use crate::grpc::get_write_token; +use crate::grpc::require_write_permission; use crate::grpc::warn_error_to_status; use crate::util::setup_execution; @@ -97,12 +98,12 @@ pub async fn handler( ) -> Result, Status> { let user_id = get_user_id(request.extensions()); let correlation_id = extract_correlation_id(&request).unwrap_or_default(); - let req = request.into_inner(); - - let repository_id: Context = req.repository_id.into(); + let repository_id: Context = request.get_ref().repository_id.clone().into(); if repository_id == Context::default() { return Err(Status::invalid_argument("Missing repository ID")); } + require_write_permission(request.extensions(), repository_id.into())?; + let req = request.into_inner(); let expected_hash: Hash = req.expected_hash.into(); let new_hash: Hash = req.new_hash.into(); diff --git a/lore-server/src/grpc/mod.rs b/lore-server/src/grpc/mod.rs index d40395b..dff4233 100644 --- a/lore-server/src/grpc/mod.rs +++ b/lore-server/src/grpc/mod.rs @@ -55,6 +55,7 @@ use tracing::warn; use crate::auth::jwt::AuthorizationToken; use crate::auth::jwt::ResourcePermission; +use crate::auth::jwt::has_any_repository_permission; use crate::auth::jwt::verify_authorization; use crate::hooks::traits::HookError; use crate::hooks::traits::StatusCode; @@ -293,6 +294,25 @@ pub fn can_admin_lock(extensions: &Extensions, repository: RepositoryId) -> bool has_required_permission(extensions, repository, "migrate") } +pub fn can_write(extensions: &Extensions, repository: RepositoryId) -> bool { + get_authorization(extensions) + .ok() + .is_some_and(|authorization| { + has_any_repository_permission(&authorization, repository, &["write", "admin", "owner"]) + }) +} + +pub fn require_write_permission( + extensions: &Extensions, + repository: RepositoryId, +) -> Result<(), Status> { + if get_authorization(extensions).is_err() || can_write(extensions, repository) { + Ok(()) + } else { + Err(Status::permission_denied("Write permission required")) + } +} + pub fn get_matching_permissions( extensions: &Extensions, repository: RepositoryId, diff --git a/lore-server/src/grpc/repository/v1/repository_metadata_set.rs b/lore-server/src/grpc/repository/v1/repository_metadata_set.rs index d9de10d..f1b8129 100644 --- a/lore-server/src/grpc/repository/v1/repository_metadata_set.rs +++ b/lore-server/src/grpc/repository/v1/repository_metadata_set.rs @@ -22,6 +22,7 @@ use tonic::Status; use crate::grpc::extract_correlation_id; use crate::grpc::get_user_id; use crate::grpc::get_write_token; +use crate::grpc::require_write_permission; use crate::grpc::warn_error_to_status; use crate::util::setup_execution; @@ -44,12 +45,12 @@ pub async fn handler( ) -> Result, Status> { let user_id = get_user_id(request.extensions()); let correlation_id = extract_correlation_id(&request).unwrap_or_default(); - let req = request.into_inner(); - - let repository_id: Context = req.id.into(); + let repository_id: Context = request.get_ref().id.clone().into(); if repository_id == Context::default() { return Err(Status::invalid_argument("Missing repository id")); } + require_write_permission(request.extensions(), repository_id.into())?; + let req = request.into_inner(); let expected: Hash = req.expected.into(); let updated: Hash = req.updated.into(); diff --git a/lore-server/src/grpc/revision/v1/branch_create.rs b/lore-server/src/grpc/revision/v1/branch_create.rs index 163d750..51be2a9 100644 --- a/lore-server/src/grpc/revision/v1/branch_create.rs +++ b/lore-server/src/grpc/revision/v1/branch_create.rs @@ -27,6 +27,7 @@ use crate::grpc::get_repository; use crate::grpc::get_user_id; use crate::grpc::get_write_token; use crate::grpc::hook_error_to_status; +use crate::grpc::require_write_permission; use crate::hooks::HookContext; use crate::hooks::HookDispatcher; use crate::hooks::HookPoint; @@ -71,6 +72,7 @@ pub async fn handler( instrument_provider: &impl InstrumentProvider, ) -> Result, Status> { let repository_id = get_repository(request.metadata())?; + require_write_permission(request.extensions(), repository_id)?; let user_id = get_user_id(request.extensions()); let correlation_id = extract_correlation_id(&request).unwrap_or_default(); let req = request.into_inner(); @@ -226,6 +228,9 @@ mod test { use super::*; use crate::auth::jwt::AuthorizationToken; + use crate::auth::jwt::PERMISSION_READ; + use crate::auth::jwt::PERMISSION_WRITE; + use crate::auth::jwt::ResourcePermission; use crate::hooks::HookDispatcher; use crate::notification::testing::MockNotificationSender; use crate::store::test_store_create; @@ -362,6 +367,10 @@ mod test { ); request.extensions_mut().insert(AuthorizationToken { user_id: "jwt-user".into(), + resources: Some(vec![ResourcePermission { + permission: vec![PERMISSION_WRITE.to_string()], + resource_id: format!("urc-{repository}"), + }]), ..AuthorizationToken::default() }); @@ -386,6 +395,54 @@ mod test { .await; } + #[tokio::test] + async fn read_only_jwt_cannot_create_branch() { + let repository = random::(); + let (immutable_store, mutable_store, execution) = + test_store_create().await.expect("Failed to create stores"); + + let notification_sender = Arc::new(MockNotificationSender::new()); + let instrument_provider = TestInstrumentProvider {}; + + Box::pin(LORE_CONTEXT.scope(execution.clone(), async move { + let branch_id = BranchId::from(uuid::Uuid::now_v7()); + let mut request = Request::new(BranchCreateRequest { + id: branch_id.into(), + name: "main".into(), + creator: Some("alice".into()), + category: "default".into(), + stack: vec![], + }); + request.metadata_mut().insert_bin( + REPOSITORY_ID_KEY, + tonic::metadata::BinaryMetadataValue::from_bytes(repository.data()), + ); + request.extensions_mut().insert(AuthorizationToken { + user_id: "jwt-user".into(), + resources: Some(vec![ResourcePermission { + permission: vec![PERMISSION_READ.to_string()], + resource_id: format!("urc-{repository}"), + }]), + ..AuthorizationToken::default() + }); + + let hook_dispatcher = HookDispatcher::empty(); + let err = handler( + request, + immutable_store.clone(), + mutable_store.clone(), + notification_sender.clone(), + &hook_dispatcher, + &instrument_provider, + ) + .await + .expect_err("read-only token should not create branches"); + assert_eq!(err.code(), tonic::Code::PermissionDenied); + assert_eq!(err.message(), "Write permission required"); + })) + .await; + } + #[tokio::test] async fn duplicate_id_returns_already_exists() { let repository = random::(); diff --git a/lore-server/src/grpc/revision/v1/branch_delete.rs b/lore-server/src/grpc/revision/v1/branch_delete.rs index 976c3aa..caacd14 100644 --- a/lore-server/src/grpc/revision/v1/branch_delete.rs +++ b/lore-server/src/grpc/revision/v1/branch_delete.rs @@ -24,6 +24,7 @@ use crate::grpc::extract_correlation_id; use crate::grpc::get_repository; use crate::grpc::get_user_id; use crate::grpc::hook_error_to_status; +use crate::grpc::require_write_permission; use crate::hooks::HookContext; use crate::hooks::HookDispatcher; use crate::hooks::HookPoint; @@ -44,6 +45,7 @@ pub async fn handler( instrument_provider: &impl InstrumentProvider, ) -> Result, Status> { let repository_id = get_repository(request.metadata())?; + require_write_permission(request.extensions(), repository_id)?; let user_id = get_user_id(request.extensions()); let correlation_id = extract_correlation_id(&request).unwrap_or_default(); let req = request.into_inner(); diff --git a/lore-server/src/grpc/revision/v1/branch_metadata_set.rs b/lore-server/src/grpc/revision/v1/branch_metadata_set.rs index a0f7e69..9a18835 100644 --- a/lore-server/src/grpc/revision/v1/branch_metadata_set.rs +++ b/lore-server/src/grpc/revision/v1/branch_metadata_set.rs @@ -26,6 +26,7 @@ use crate::grpc::get_user_id; use crate::grpc::get_write_token; use crate::grpc::handlers::branch_metadata_set::validate_binary_blobs; use crate::grpc::handlers::branch_metadata_set::validate_read_only_fields; +use crate::grpc::require_write_permission; use crate::grpc::warn_error_to_status; use crate::util::setup_execution; @@ -47,6 +48,7 @@ pub async fn handler( mutable_store: Arc, ) -> Result, Status> { let repository_id = get_repository(request.metadata())?; + require_write_permission(request.extensions(), repository_id)?; let user_id = get_user_id(request.extensions()); let correlation_id = extract_correlation_id(&request).unwrap_or_default(); let req = request.into_inner(); diff --git a/lore-server/src/grpc/revision/v1/branch_push.rs b/lore-server/src/grpc/revision/v1/branch_push.rs index db11de7..51065ac 100644 --- a/lore-server/src/grpc/revision/v1/branch_push.rs +++ b/lore-server/src/grpc/revision/v1/branch_push.rs @@ -34,6 +34,7 @@ use crate::grpc::handlers::branch_push::dispatch_response_message; use crate::grpc::handlers::branch_push::extract_client_ip; use crate::grpc::handlers::branch_push::push; use crate::grpc::hook_error_to_status; +use crate::grpc::require_write_permission; use crate::hooks::HookContext; use crate::hooks::HookDispatcher; use crate::hooks::HookPoint; @@ -65,6 +66,7 @@ pub async fn handler( let user_id = get_user_id(request.extensions()); let correlation_id = extract_correlation_id(&request).unwrap_or_default(); let repository_id = get_repository(request.metadata())?; + require_write_permission(request.extensions(), repository_id)?; // Service accounts bypass branch-protection (mirroring path). let mut bypass_protection = false; @@ -419,6 +421,10 @@ mod test { .insert(crate::auth::jwt::AuthorizationToken { user_id: "service-bot".into(), is_service_account: Some(true), + resources: Some(vec![crate::auth::jwt::ResourcePermission { + permission: vec![crate::auth::jwt::PERMISSION_WRITE.to_string()], + resource_id: format!("urc-{repository}"), + }]), ..crate::auth::jwt::AuthorizationToken::default() }); request diff --git a/lore-server/src/grpc/storage/v1/copy.rs b/lore-server/src/grpc/storage/v1/copy.rs index 8776ad1..d61307a 100644 --- a/lore-server/src/grpc/storage/v1/copy.rs +++ b/lore-server/src/grpc/storage/v1/copy.rs @@ -39,6 +39,7 @@ use crate::grpc::get_user_id; use crate::grpc::interpret_streaming_error; use crate::grpc::log_server_error; use crate::grpc::map_message_handle_error_to_status; +use crate::grpc::require_write_permission; use crate::grpc::rpc_code_to_str; use crate::protocol::storage::copy::handle_copy; use crate::protocol::storage::messages::LoreResponse; @@ -59,6 +60,7 @@ pub async fn handler( instrument_provider: &impl InstrumentProvider, ) -> Result, Status> { let destination_repository = get_repository(request.metadata())?; + require_write_permission(request.extensions(), destination_repository)?; let auth_token = get_authorization(request.extensions()).ok(); let user_id = get_user_id(request.extensions()); let correlation_id = extract_correlation_id(&request).unwrap_or_default(); diff --git a/lore-server/src/grpc/storage/v1/mutable_compare_and_swap.rs b/lore-server/src/grpc/storage/v1/mutable_compare_and_swap.rs index 9e1e23e..87db569 100644 --- a/lore-server/src/grpc/storage/v1/mutable_compare_and_swap.rs +++ b/lore-server/src/grpc/storage/v1/mutable_compare_and_swap.rs @@ -14,6 +14,7 @@ use crate::grpc::extract_correlation_id; use crate::grpc::get_repository; use crate::grpc::get_user_id; use crate::grpc::log_server_error; +use crate::grpc::require_write_permission; use crate::grpc::simple_map_message_handle_error; use crate::protocol::storage::messages::LoreResponse; use crate::protocol::storage::mutable_cas::handle_mutable_cas; @@ -25,6 +26,7 @@ pub async fn handler( mutable_store: Arc, ) -> Result, Status> { let repository = get_repository(request.metadata())?; + require_write_permission(request.extensions(), repository)?; let user_id = get_user_id(request.extensions()); let correlation_id = extract_correlation_id(&request).unwrap_or_default(); let execution = setup_execution(module_path!(), correlation_id.clone(), user_id.clone()); diff --git a/lore-server/src/grpc/storage/v1/mutable_store.rs b/lore-server/src/grpc/storage/v1/mutable_store.rs index 273b875..2bec62d 100644 --- a/lore-server/src/grpc/storage/v1/mutable_store.rs +++ b/lore-server/src/grpc/storage/v1/mutable_store.rs @@ -14,6 +14,7 @@ use crate::grpc::extract_correlation_id; use crate::grpc::get_repository; use crate::grpc::get_user_id; use crate::grpc::log_server_error; +use crate::grpc::require_write_permission; use crate::grpc::simple_map_message_handle_error; use crate::protocol::storage::mutable_store_handler::handle_mutable_store; use crate::util::setup_execution; @@ -24,6 +25,7 @@ pub async fn handler( mutable_store: Arc, ) -> Result, Status> { let repository = get_repository(request.metadata())?; + require_write_permission(request.extensions(), repository)?; let user_id = get_user_id(request.extensions()); let correlation_id = extract_correlation_id(&request).unwrap_or_default(); let execution = setup_execution(module_path!(), correlation_id.clone(), user_id.clone()); diff --git a/lore-server/src/grpc/storage/v1/put.rs b/lore-server/src/grpc/storage/v1/put.rs index 39daa67..b6c8b0f 100644 --- a/lore-server/src/grpc/storage/v1/put.rs +++ b/lore-server/src/grpc/storage/v1/put.rs @@ -37,6 +37,7 @@ use crate::grpc::get_user_id; use crate::grpc::interpret_streaming_error; use crate::grpc::log_server_error; use crate::grpc::map_message_handle_error_to_status; +use crate::grpc::require_write_permission; use crate::grpc::rpc_code_to_str; use crate::protocol::storage::messages::LoreResponse; use crate::protocol::storage::put::UnvalidatedPut; @@ -57,6 +58,7 @@ pub async fn handler( instrument_provider: &impl InstrumentProvider, ) -> Result, Status> { let repository = get_repository(request.metadata())?; + require_write_permission(request.extensions(), repository)?; let user_id = get_user_id(request.extensions()); let correlation_id = extract_correlation_id(&request).unwrap_or_default(); diff --git a/lore-server/src/grpc/thinclient/v1/revision_tree.rs b/lore-server/src/grpc/thinclient/v1/revision_tree.rs index fb6aea1..02fbf2f 100644 --- a/lore-server/src/grpc/thinclient/v1/revision_tree.rs +++ b/lore-server/src/grpc/thinclient/v1/revision_tree.rs @@ -1079,7 +1079,7 @@ mod test { repos .iter() .map(|id| ResourcePermission { - permission: vec![], + permission: vec!["read".to_string()], resource_id: format!("urc-{id}"), }) .collect(), diff --git a/lore-server/src/http/repositories/repository/contents/content/presign_repository_content.rs b/lore-server/src/http/repositories/repository/contents/content/presign_repository_content.rs index eaa0c3d..6198eb9 100644 --- a/lore-server/src/http/repositories/repository/contents/content/presign_repository_content.rs +++ b/lore-server/src/http/repositories/repository/contents/content/presign_repository_content.rs @@ -24,6 +24,7 @@ use thiserror::Error; use tracing::warn; use crate::auth::jwt::AuthorizationToken; +use crate::auth::jwt::verify_authorization; use crate::http::presign_token::CURRENT_TOKEN_VERSION; use crate::http::presign_token::PresignTokenPayload; use crate::http::presign_token::sign; @@ -39,6 +40,8 @@ pub enum PresignError { ParseAddress(FromHexError), #[error("Presign feature is not configured")] NotConfigured, + #[error("Read permission required")] + Forbidden, #[error("Content not found")] NotFound, #[error("Store error checking content existence")] @@ -59,6 +62,7 @@ impl IntoResponse for PresignError { StatusCode::NOT_FOUND, "presigned URL feature is not enabled".to_string(), ), + PresignError::Forbidden => (StatusCode::FORBIDDEN, "Read permission required".into()), PresignError::StoreError | PresignError::SystemTime(_) => ( StatusCode::INTERNAL_SERVER_ERROR, "Something went wrong. See server log for more info.".to_string(), @@ -102,6 +106,11 @@ pub async fn handler( let repository = repository_id .parse::() .map_err(PresignError::ParseRepository)?; + if let Some(token) = user_info.as_ref() + && verify_authorization(token, repository).is_err() + { + return Err(PresignError::Forbidden); + } let parsed_address = address .parse::
() .map_err(PresignError::ParseAddress)?; diff --git a/lore-server/src/http/repositories/repository/contents/put_repository_content.rs b/lore-server/src/http/repositories/repository/contents/put_repository_content.rs index 149efc8..c09b197 100644 --- a/lore-server/src/http/repositories/repository/contents/put_repository_content.rs +++ b/lore-server/src/http/repositories/repository/contents/put_repository_content.rs @@ -24,6 +24,7 @@ use tracing::debug; use tracing::info; use crate::auth::jwt::AuthorizationToken; +use crate::auth::jwt::verify_repository_write_authorization; use crate::http::server::ServerState; use crate::util::get_user_id_from_token; use crate::util::setup_execution; @@ -56,16 +57,12 @@ pub async fn handler( .and_then(|header_value| header_value.to_str().map(str::to_string).ok()) .unwrap_or_default(); - let user_id = get_user_id_from_token(user_info); + let user_id = get_user_id_from_token(user_info.clone()); let execution = setup_execution(module_path!(), correlation_id, user_id); LORE_CONTEXT .scope(execution, async move { - let repository = match repository_id.parse::() { - Ok(repository_id) => Arc::new(RepositoryContext::new_server_context( - state.immutable_store.clone(), - state.mutable_store.clone(), - repository_id.into(), - )), + let repository_id = match repository_id.parse::() { + Ok(repository_id) => repository_id, Err(error) => { debug!("Error parsing the repository ID {}", error); return ( @@ -77,6 +74,23 @@ pub async fn handler( } }; + if let Some(token) = user_info.as_ref() + && verify_repository_write_authorization(token, repository_id.into()).is_err() + { + return ( + StatusCode::FORBIDDEN, + header_error, + Body::from("Write permission required"), + ) + .into_response(); + } + + let repository = Arc::new(RepositoryContext::new_server_context( + state.immutable_store.clone(), + state.mutable_store.clone(), + repository_id.into(), + )); + let context = uuid::Uuid::now_v7().into(); let (address, _fragment) = match immutable::write( repository.clone(), diff --git a/lore-server/src/protocol/storage/copy.rs b/lore-server/src/protocol/storage/copy.rs index 8c157ea..d65262f 100644 --- a/lore-server/src/protocol/storage/copy.rs +++ b/lore-server/src/protocol/storage/copy.rs @@ -14,6 +14,7 @@ use tracing::warn; use crate::auth::jwt::AuthorizationToken; use crate::auth::jwt::verify_authorization; +use crate::auth::jwt::verify_repository_write_authorization; use crate::correlation::CorrelationId; use crate::protocol::attribute_map::AttributeMap; use crate::protocol::attribute_map::get_user_id_from_context; @@ -140,6 +141,8 @@ impl Message for Copy { .get_or::(MessageHandleError::NotConnected)?; if let Some(token) = context.get::() { + verify_repository_write_authorization(&token, destination_repository) + .map_err(|err| MessageHandleError::AuthorizationFailure(err.to_string()))?; verify_authorization(&token, self.source_repository) .map_err(|err| MessageHandleError::AuthorizationFailure(err.to_string()))?; } diff --git a/lore-server/src/protocol/storage/mutable_cas.rs b/lore-server/src/protocol/storage/mutable_cas.rs index 6c6ce83..5738792 100644 --- a/lore-server/src/protocol/storage/mutable_cas.rs +++ b/lore-server/src/protocol/storage/mutable_cas.rs @@ -14,6 +14,8 @@ use tracing::debug; use tracing::warn; use zerocopy::IntoBytes; +use crate::auth::jwt::AuthorizationToken; +use crate::auth::jwt::verify_repository_write_authorization; use crate::correlation::CorrelationId; use crate::protocol::attribute_map::AttributeMap; use crate::protocol::attribute_map::get_user_id_from_context; @@ -102,6 +104,10 @@ impl Message for MutableCas { ) -> Result { let repository = *context .get_or::(MessageHandleError::NotConnected)?; + if let Some(token) = context.get::() { + verify_repository_write_authorization(&token, repository) + .map_err(|err| MessageHandleError::AuthorizationFailure(err.to_string()))?; + } let user_id = get_user_id_from_context(&context); let correlation_id = context.get::().unwrap_or_default(); handle_mutable_cas( diff --git a/lore-server/src/protocol/storage/mutable_store_handler.rs b/lore-server/src/protocol/storage/mutable_store_handler.rs index 821212c..70ae537 100644 --- a/lore-server/src/protocol/storage/mutable_store_handler.rs +++ b/lore-server/src/protocol/storage/mutable_store_handler.rs @@ -13,6 +13,8 @@ use lore_storage::StoreError; use tracing::debug; use tracing::warn; +use crate::auth::jwt::AuthorizationToken; +use crate::auth::jwt::verify_repository_write_authorization; use crate::correlation::CorrelationId; use crate::protocol::attribute_map::AttributeMap; use crate::protocol::attribute_map::get_user_id_from_context; @@ -91,6 +93,10 @@ impl Message for MutableStoreOp { ) -> Result { let repository = *context .get_or::(MessageHandleError::NotConnected)?; + if let Some(token) = context.get::() { + verify_repository_write_authorization(&token, repository) + .map_err(|err| MessageHandleError::AuthorizationFailure(err.to_string()))?; + } let user_id = get_user_id_from_context(&context); let correlation_id = context.get::().unwrap_or_default(); handle_mutable_store( diff --git a/lore-server/src/protocol/storage/put.rs b/lore-server/src/protocol/storage/put.rs index 02ff00e..5caed78 100644 --- a/lore-server/src/protocol/storage/put.rs +++ b/lore-server/src/protocol/storage/put.rs @@ -22,6 +22,8 @@ use opentelemetry::metrics::Histogram; use tracing::debug; use tracing::warn; +use crate::auth::jwt::AuthorizationToken; +use crate::auth::jwt::verify_repository_write_authorization; use crate::correlation::CorrelationId; use crate::protocol::attribute_map::AttributeMap; use crate::protocol::attribute_map::get_user_id_from_context; @@ -282,6 +284,10 @@ impl Message for Put { ) -> Result { let repository = *context .get_or::(MessageHandleError::NotConnected)?; + if let Some(token) = context.get::() { + verify_repository_write_authorization(&token, repository) + .map_err(|err| MessageHandleError::AuthorizationFailure(err.to_string()))?; + } let user_id = get_user_id_from_context(&context); let correlation_id = context.get::().unwrap_or_default(); handle_put( diff --git a/lore-server/src/protocol/storage/session.rs b/lore-server/src/protocol/storage/session.rs index 1004e13..b56e3ca 100644 --- a/lore-server/src/protocol/storage/session.rs +++ b/lore-server/src/protocol/storage/session.rs @@ -13,6 +13,7 @@ pub struct SessionEntry { pub repository: RepositoryId, pub correlation_id: String, pub user_id: String, + pub can_write: bool, } /// Per-connection session state for the `lore-storage/0.4` protocol. @@ -51,6 +52,16 @@ impl SessionMap { repository: RepositoryId, correlation_id: String, user_id: String, + ) -> Result<(u32, String), SessionError> { + self.start_with_permissions(repository, correlation_id, user_id, true) + } + + pub fn start_with_permissions( + &self, + repository: RepositoryId, + correlation_id: String, + user_id: String, + can_write: bool, ) -> Result<(u32, String), SessionError> { if self.entries.len() >= MAX_CONCURRENT_SESSIONS as usize { return Err(SessionError::LimitReached); @@ -75,6 +86,7 @@ impl SessionMap { repository, correlation_id: correlation_id.clone(), user_id, + can_write, }, ); @@ -188,6 +200,18 @@ mod tests { assert_eq!(entry.repository, repo); assert_eq!(entry.correlation_id, "corr-1"); assert_eq!(entry.user_id, "user-42"); + assert!(entry.can_write); + } + + #[test] + fn start_with_permissions_records_write_access() { + let map = SessionMap::default(); + let repo = random::(); + let (id, _) = map + .start_with_permissions(repo, "corr-1".into(), "user-42".into(), false) + .unwrap(); + let entry = map.get(id).unwrap(); + assert!(!entry.can_write); } #[test] diff --git a/lore-server/src/quic/storage_service_v4.rs b/lore-server/src/quic/storage_service_v4.rs index fcaea98..9ab17c1 100644 --- a/lore-server/src/quic/storage_service_v4.rs +++ b/lore-server/src/quic/storage_service_v4.rs @@ -18,6 +18,7 @@ use tracing::Span; use tracing::debug; use crate::auth::jwt::JwtVerifier; +use crate::auth::jwt::has_repository_write_permission; use crate::protocol::attribute_map::AttributeMap; use crate::protocol::attribute_map::ConnectionId; use crate::protocol::storage::authorize::AuthorizeAction; @@ -169,6 +170,7 @@ impl QuicService for StorageServiceV4 { auth_token, } => { let mut user_id = String::new(); + let mut can_write = true; if let Some(jwt_verifier) = self.jwt_verifier.as_ref() { let token_str = String::from_utf8(auth_token).map_err(|err| { @@ -189,11 +191,17 @@ impl QuicService for StorageServiceV4 { crate::auth::jwt::verify_authorization(&authorization, repository) .map_err(|err| MessageHandleError::AuthorizationFailure(err.to_string()))?; + can_write = has_repository_write_permission(&authorization, repository); user_id = crate::util::get_user_id_from_token(Some(authorization)); } let session_map = self.session_map.clone(); - match session_map.start(repository, correlation_id, user_id) { + match session_map.start_with_permissions( + repository, + correlation_id, + user_id, + can_write, + ) { Ok((session_id, correlation_id)) => { debug!( session_id, @@ -234,6 +242,7 @@ impl QuicService for StorageServiceV4 { let repository = session.repository; let correlation_id = session.correlation_id.clone(); let user_id = session.user_id.clone(); + let can_write = session.can_write; drop(session); // Parse the storage command payload using v4-aware parsers — Copy carries an @@ -266,6 +275,11 @@ impl QuicService for StorageServiceV4 { .await } crate::quic::storage_service::ParsedStorageRequest::Put(put) => { + if !can_write { + return Err(MessageHandleError::AuthorizationFailure( + "write permission required".to_string(), + )); + } handle_put( &put, repository, @@ -294,6 +308,11 @@ impl QuicService for StorageServiceV4 { .await } crate::quic::storage_service::ParsedStorageRequest::Copy(copy) => { + if !can_write { + return Err(MessageHandleError::AuthorizationFailure( + "write permission required".to_string(), + )); + } handle_copy( copy.source_repository, copy.source_address, @@ -318,6 +337,11 @@ impl QuicService for StorageServiceV4 { .await } crate::quic::storage_service::ParsedStorageRequest::MutableStoreOp(store) => { + if !can_write { + return Err(MessageHandleError::AuthorizationFailure( + "write permission required".to_string(), + )); + } handle_mutable_store( store.key, store.value, @@ -330,6 +354,11 @@ impl QuicService for StorageServiceV4 { .await } crate::quic::storage_service::ParsedStorageRequest::MutableCas(cas) => { + if !can_write { + return Err(MessageHandleError::AuthorizationFailure( + "write permission required".to_string(), + )); + } handle_mutable_cas( cas.key, cas.expected, diff --git a/lore-server/src/quic/stream_handler.rs b/lore-server/src/quic/stream_handler.rs index de8df9e..171d53e 100644 --- a/lore-server/src/quic/stream_handler.rs +++ b/lore-server/src/quic/stream_handler.rs @@ -409,12 +409,10 @@ where } else { info!(command_header = ?header, handler_error_label = error.message_handle_label, response_error_code = error.response_error_code, "non-internal error handling message"); } + } else if error.is_internal_error { + debug!(command_header = ?header, handler_error_label = error.message_handle_label, response_error_code = error.response_error_code, "internal error handling message"); } else { - if error.is_internal_error { - debug!(command_header = ?header, handler_error_label = error.message_handle_label, response_error_code = error.response_error_code, "internal error handling message"); - } else { - debug!(command_header = ?header, handler_error_label = error.message_handle_label, response_error_code = error.response_error_code, "non-internal error handling message"); - } + debug!(command_header = ?header, handler_error_label = error.message_handle_label, response_error_code = error.response_error_code, "non-internal error handling message"); } let response_header = header.response_error(error.response_error_code); let (header_buf, header_len) = response_header.response_bytes();