Merge pull request #19 from kagenti/feat/per-user-provider-ownership-1995

feat(auth): per-user provider ownership enforcement

Author: pdettori

crates/openshell-server/src/auth/authz.rs

@@ -185,7 +185,7 @@ mod tests {
         let policy = default_policy();
         assert!(
             policy
-                .check(&id, "/openshell.v1.OpenShell/CreateProvider")
+                .check(&id, "/openshell.v1.OpenShell/ImportProviderProfiles")
                 .is_err()
         );
     }
@@ -408,7 +408,7 @@ mod tests {
     }
 
     #[test]
-    fn provider_refresh_methods_require_provider_scopes_and_admin_for_writes() {
+    fn provider_refresh_methods_require_provider_scopes_and_user_role() {
         let policy = scoped_policy();
         let reader = identity_with_roles_and_scopes(&["openshell-user"], &["provider:read"]);
         assert!(
@@ -417,37 +417,28 @@ mod tests {
                 .is_ok()
         );
 
-        let writer_without_admin =
-            identity_with_roles_and_scopes(&["openshell-user"], &["provider:write"]);
-        let err = policy
-            .check(
-                &writer_without_admin,
-                "/openshell.v1.OpenShell/ConfigureProviderRefresh",
-            )
-            .unwrap_err();
-        assert_eq!(err.code(), tonic::Code::PermissionDenied);
-        assert!(err.message().contains("openshell-admin"));
+        // User with provider:write scope and user role can call write methods
+        // (ownership enforcement happens in the handler, not RBAC).
+        let user_writer = identity_with_roles_and_scopes(&["openshell-user"], &["provider:write"]);
+        for method in [
+            "/openshell.v1.OpenShell/ConfigureProviderRefresh",
+            "/openshell.v1.OpenShell/RotateProviderCredential",
+            "/openshell.v1.OpenShell/DeleteProviderRefresh",
+        ] {
+            assert!(policy.check(&user_writer, method).is_ok(), "{method}");
+        }
 
-        let admin_without_scope =
-            identity_with_roles_and_scopes(&["openshell-admin"], &["provider:read"]);
+        // Without the write scope, user is still denied.
+        let user_without_scope =
+            identity_with_roles_and_scopes(&["openshell-user"], &["provider:read"]);
         let err = policy
             .check(
-                &admin_without_scope,
-                "/openshell.v1.OpenShell/RotateProviderCredential",
+                &user_without_scope,
+                "/openshell.v1.OpenShell/ConfigureProviderRefresh",
             )
             .unwrap_err();
         assert_eq!(err.code(), tonic::Code::PermissionDenied);
         assert!(err.message().contains("provider:write"));
-
-        let admin_writer =
-            identity_with_roles_and_scopes(&["openshell-admin"], &["provider:write"]);
-        for method in [
-            "/openshell.v1.OpenShell/ConfigureProviderRefresh",
-            "/openshell.v1.OpenShell/RotateProviderCredential",
-            "/openshell.v1.OpenShell/DeleteProviderRefresh",
-        ] {
-            assert!(policy.check(&admin_writer, method).is_ok(), "{method}");
-        }
     }
 
     #[test]
@@ -493,10 +484,16 @@ mod tests {
                 .check(&id, "/openshell.v1.OpenShell/GetProvider")
                 .is_ok()
         );
-        // admin methods still denied by role check
+        // User can now create providers (ownership enforced in handler).
         assert!(
             policy
                 .check(&id, "/openshell.v1.OpenShell/CreateProvider")
+                .is_ok()
+        );
+        // Admin methods still denied by role check.
+        assert!(
+            policy
+                .check(&id, "/openshell.v1.OpenShell/ImportProviderProfiles")
                 .is_err()
         );
     }

crates/openshell-server/src/auth/ownership.rs

@@ -1,11 +1,11 @@
 // SPDX-FileCopyrightText: Copyright (c) 2025-2026 NVIDIA CORPORATION & AFFILIATES. All rights reserved.
 // SPDX-License-Identifier: Apache-2.0
 
-//! Per-user sandbox ownership enforcement.
+//! Per-user object ownership enforcement.
 //!
-//! Stamps an `openshell.ai/owner` label on sandboxes at creation time (from the
-//! verified caller identity) and gates access on subsequent operations. Admin
-//! callers bypass the ownership check.
+//! Stamps an `openshell.ai/owner` label on objects (sandboxes, providers) at
+//! creation time (from the verified caller identity) and gates access on
+//! subsequent operations. Admin callers bypass the ownership check.
 
 #![allow(clippy::result_large_err)]
 
@@ -18,7 +18,7 @@ use tonic::Status;
 use super::identity::Identity;
 use super::principal::Principal;
 
-/// Reserved label key for sandbox ownership. Server-set, never client-controlled.
+/// Reserved label key for object ownership. Server-set, never client-controlled.
 pub const OWNER_LABEL: &str = "openshell.ai/owner";
 
 /// Reserved label key prefix. All keys starting with this are stripped from
@@ -84,13 +84,13 @@ pub fn check_owner(
     admin_role: &str,
 ) -> Result<(), Status> {
     let Some(owner_value) = sandbox_labels.get(OWNER_LABEL) else {
-        // No owner label → legacy sandbox, allow access.
+        // No owner label → legacy/shared object, allow access.
         return Ok(());
     };
 
     let Some(identity) = principal_identity(principal) else {
         return Err(Status::permission_denied(
-            "sandbox is owned; authenticated identity required",
+            "object is owned; authenticated identity required",
         ));
     };
 
@@ -101,7 +101,7 @@ pub fn check_owner(
 
     let caller_value = sanitize_subject(&identity.subject)?;
     if caller_value != *owner_value {
-        return Err(Status::permission_denied("you do not own this sandbox"));
+        return Err(Status::permission_denied("you do not own this resource"));
     }
 
     Ok(())
@@ -140,7 +140,7 @@ fn require_identity(principal: Option<&Principal>) -> Result<&Identity, Status>
     match principal {
         Some(Principal::User(user)) => Ok(&user.identity),
         Some(Principal::Sandbox(_) | Principal::Anonymous) | None => Err(Status::unauthenticated(
-            "authenticated user identity required for sandbox operations",
+            "authenticated user identity required for ownership operations",
         )),
     }
 }

crates/openshell-server/src/grpc/mod.rs

@@ -356,7 +356,7 @@ impl OpenShell for OpenShellService {
 
     // --- Providers ---
 
-    #[rpc_auth(auth = "bearer", scope = "provider:write", role = "admin")]
+    #[rpc_auth(auth = "bearer", scope = "provider:write", role = "user")]
     async fn create_provider(
         &self,
         request: Request<CreateProviderRequest>,
@@ -412,7 +412,7 @@ impl OpenShell for OpenShellService {
         provider::handle_lint_provider_profiles(&self.state, request).await
     }
 
-    #[rpc_auth(auth = "bearer", scope = "provider:write", role = "admin")]
+    #[rpc_auth(auth = "bearer", scope = "provider:write", role = "user")]
     async fn update_provider(
         &self,
         request: Request<UpdateProviderRequest>,
@@ -428,31 +428,31 @@ impl OpenShell for OpenShellService {
         provider::handle_get_provider_refresh_status(&self.state, request).await
     }
 
-    #[rpc_auth(auth = "bearer", scope = "provider:write", role = "admin")]
+    #[rpc_auth(auth = "bearer", scope = "provider:write", role = "user")]
     async fn configure_provider_refresh(
         &self,
         request: Request<ConfigureProviderRefreshRequest>,
     ) -> Result<Response<ConfigureProviderRefreshResponse>, Status> {
         provider::handle_configure_provider_refresh(&self.state, request).await
     }
 
-    #[rpc_auth(auth = "bearer", scope = "provider:write", role = "admin")]
+    #[rpc_auth(auth = "bearer", scope = "provider:write", role = "user")]
     async fn rotate_provider_credential(
         &self,
         request: Request<RotateProviderCredentialRequest>,
     ) -> Result<Response<RotateProviderCredentialResponse>, Status> {
         provider::handle_rotate_provider_credential(&self.state, request).await
     }
 
-    #[rpc_auth(auth = "bearer", scope = "provider:write", role = "admin")]
+    #[rpc_auth(auth = "bearer", scope = "provider:write", role = "user")]
     async fn delete_provider_refresh(
         &self,
         request: Request<DeleteProviderRefreshRequest>,
     ) -> Result<Response<DeleteProviderRefreshResponse>, Status> {
         provider::handle_delete_provider_refresh(&self.state, request).await
     }
 
-    #[rpc_auth(auth = "bearer", scope = "provider:write", role = "admin")]
+    #[rpc_auth(auth = "bearer", scope = "provider:write", role = "user")]
     async fn delete_provider(
         &self,
         request: Request<DeleteProviderRequest>,