Skip to content

Commit 956cfe9

Browse files
feat(auth): add workspace-scoped authorization (Phase 2)
Implement RFC 0011 Phase 2 authorization enforcement: - Add proto-level AuthorizationRule annotations on every RPC method - Replace #[rpc_authz] proc macro with DescriptorPool-based runtime resolution of per-method auth mode, workspace role, and global role - Add authorize_workspace() as the single authorization entry point for all workspace-scoped handlers, enforcing membership and role - Wire workspace authorization checks into all handler call sites (sandbox, provider, policy, inference, workspace management) - Add require_platform_admin() for cross-workspace operations - Add authorize_sandbox_workspace() for data-plane operations - Add privilege escalation guard on workspace member role assignment - Add Keycloak OIDC test infrastructure with admin/user/user-b identities and audience mappers - Add comprehensive e2e coverage: Python workspace_authz_test with ~40 parametrized non-member rejection cases, Rust PKCE cross-workspace isolation tests, and workspace admin scenario tests Signed-off-by: Derek Carr <decarr@redhat.com>
1 parent e9ac0ee commit 956cfe9

36 files changed

Lines changed: 5172 additions & 1097 deletions

‎Cargo.lock‎

Lines changed: 1 addition & 1 deletion
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

‎crates/openshell-cli/src/run.rs‎

Lines changed: 8 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -2026,11 +2026,17 @@ pub async fn sandbox_create(
20262026
}
20272027
None => None,
20282028
};
2029-
let providers_v2_enabled = gateway_providers_v2_enabled(&mut client).await?;
2029+
let inferred_provider = inferred_provider_type(command);
2030+
let providers_v2_enabled =
2031+
if inferred_provider.is_some() && auto_providers_override != Some(false) {
2032+
gateway_providers_v2_enabled(&mut client).await?
2033+
} else {
2034+
false
2035+
};
20302036
let inferred_types: Vec<String> = if providers_v2_enabled {
20312037
Vec::new()
20322038
} else {
2033-
inferred_provider_type(command).into_iter().collect()
2039+
inferred_provider.into_iter().collect()
20342040
};
20352041
let configured_providers = ensure_required_providers(
20362042
&mut client,

‎crates/openshell-cli/tests/sandbox_create_lifecycle_integration.rs‎

Lines changed: 39 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -31,7 +31,7 @@ use std::collections::HashMap;
3131
use std::fs;
3232
use std::os::unix::fs::PermissionsExt;
3333
use std::sync::Arc;
34-
use std::sync::atomic::{AtomicBool, Ordering};
34+
use std::sync::atomic::{AtomicBool, AtomicUsize, Ordering};
3535
use std::time::{Duration, Instant};
3636
use tempfile::TempDir;
3737
use tokio::net::TcpListener;
@@ -48,6 +48,7 @@ struct SandboxState {
4848
vm_slow_progress_before_ready: Arc<AtomicBool>,
4949
vm_log_churn_before_ready: Arc<AtomicBool>,
5050
global_settings: Arc<Mutex<HashMap<String, SettingValue>>>,
51+
gateway_config_requests: Arc<AtomicUsize>,
5152
}
5253

5354
#[derive(Clone, Default)]
@@ -182,6 +183,9 @@ impl OpenShell for TestOpenShell {
182183
&self,
183184
_request: tonic::Request<GetGatewayConfigRequest>,
184185
) -> Result<Response<GetGatewayConfigResponse>, Status> {
186+
self.state
187+
.gateway_config_requests
188+
.fetch_add(1, Ordering::SeqCst);
185189
Ok(Response::new(GetGatewayConfigResponse {
186190
settings: self.state.global_settings.lock().await.clone(),
187191
settings_revision: 1,
@@ -1208,6 +1212,40 @@ async fn sandbox_create_keeps_command_sessions_by_default() {
12081212
);
12091213
}
12101214

1215+
#[tokio::test]
1216+
async fn sandbox_create_without_inferred_provider_skips_gateway_config() {
1217+
let server = run_server().await;
1218+
let fake_ssh_dir = tempfile::tempdir().unwrap();
1219+
let xdg_dir = tempfile::tempdir().unwrap();
1220+
let _env = test_env(&fake_ssh_dir, &xdg_dir);
1221+
let tls = test_tls(&server);
1222+
install_fake_ssh(&fake_ssh_dir);
1223+
1224+
run::sandbox_create(
1225+
&server.endpoint,
1226+
"openshell",
1227+
run::SandboxCreateConfig {
1228+
name: Some("no-provider-config"),
1229+
command: &["echo".into(), "OK".into()],
1230+
..test_config()
1231+
},
1232+
"default",
1233+
&tls,
1234+
)
1235+
.await
1236+
.expect("sandbox create should succeed without reading gateway config");
1237+
1238+
assert_eq!(
1239+
server
1240+
.openshell
1241+
.state
1242+
.gateway_config_requests
1243+
.load(Ordering::SeqCst),
1244+
0,
1245+
"commands without an inferred provider must not require global gateway settings"
1246+
);
1247+
}
1248+
12111249
#[tokio::test]
12121250
async fn sandbox_create_sends_cpu_and_memory_limits_only() {
12131251
let server = run_server().await;

‎crates/openshell-server/Cargo.toml‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -26,7 +26,6 @@ openshell-policy = { path = "../openshell-policy" }
2626
openshell-prover = { path = "../openshell-prover" }
2727
openshell-providers = { path = "../openshell-providers" }
2828
openshell-router = { path = "../openshell-router" }
29-
openshell-server-macros = { path = "../openshell-server-macros" }
3029
openshell-supervisor-middleware = { path = "../openshell-supervisor-middleware" }
3130
openshell-supervisor-middleware-builtins = { path = "../openshell-supervisor-middleware-builtins" }
3231

@@ -40,6 +39,7 @@ tokio = { workspace = true }
4039
# gRPC
4140
tonic = { workspace = true, features = ["channel", "tls-native-roots"] }
4241
prost = { workspace = true }
42+
prost-reflect = { workspace = true }
4343
prost-types = { workspace = true }
4444

4545
# HTTP server

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

Lines changed: 68 additions & 35 deletions
Original file line numberDiff line numberDiff line change
@@ -13,7 +13,7 @@
1313
//! authorization is a gateway concern.
1414
1515
use super::identity::Identity;
16-
use super::method_authz::{self, Role};
16+
use super::{descriptor_authz, method_authz};
1717
use tonic::Status;
1818
use tracing::debug;
1919

@@ -62,14 +62,16 @@ impl AuthzPolicy {
6262
/// Returns `Ok(())` if authorized, `Err(PERMISSION_DENIED)` if not.
6363
/// When both role names are empty, all authenticated callers are authorized
6464
/// (authentication-only mode for providers like GitHub).
65+
///
66+
/// Methods annotated with `global_role` (e.g. `"platform_admin"`) require
67+
/// the `admin_role` OIDC claim. Methods annotated with only
68+
/// `workspace_role` require the `user_role` OIDC claim — the handler
69+
/// enforces workspace-level role via `authorize_workspace()`.
6570
#[allow(clippy::result_large_err)]
6671
pub fn check(&self, identity: &Identity, method: &str) -> Result<(), Status> {
67-
let required = match method_authz::required_role(method) {
68-
Some(Role::Admin) => &self.admin_role,
69-
// Default to user role for unknown methods, matching the
70-
// pre-annotation behavior. The exhaustiveness test ensures
71-
// every real RPC has an explicit declaration.
72-
Some(Role::User) | None => &self.user_role,
72+
let required = match descriptor_authz::lookup(method) {
73+
Some(entry) if entry.global_role.is_some() => &self.admin_role,
74+
_ => &self.user_role,
7375
};
7476

7577
// Empty role name = skip role check for this level (auth-only mode).
@@ -180,25 +182,51 @@ mod tests {
180182
}
181183

182184
#[test]
183-
fn user_cannot_access_admin_methods() {
185+
fn user_blocked_for_platform_admin_methods() {
184186
let id = identity_with_roles(&["openshell-user"]);
185187
let policy = default_policy();
186188
assert!(
187189
policy
188-
.check(&id, "/openshell.v1.OpenShell/CreateProvider")
190+
.check(&id, "/openshell.v1.OpenShell/CreateWorkspace")
191+
.is_err()
192+
);
193+
assert!(
194+
policy
195+
.check(&id, "/openshell.v1.OpenShell/GetGatewayInfo")
189196
.is_err()
190197
);
191198
}
192199

193200
#[test]
194-
fn admin_can_access_admin_methods() {
195-
let id = identity_with_roles(&["openshell-admin", "openshell-user"]);
201+
fn user_passes_middleware_for_workspace_admin_methods() {
202+
let id = identity_with_roles(&["openshell-user"]);
196203
let policy = default_policy();
197204
assert!(
198205
policy
199206
.check(&id, "/openshell.v1.OpenShell/CreateProvider")
200207
.is_ok()
201208
);
209+
assert!(
210+
policy
211+
.check(&id, "/openshell.v1.OpenShell/DeleteProvider")
212+
.is_ok()
213+
);
214+
assert!(
215+
policy
216+
.check(&id, "/openshell.v1.OpenShell/AddWorkspaceMember")
217+
.is_ok()
218+
);
219+
}
220+
221+
#[test]
222+
fn admin_can_access_platform_admin_methods() {
223+
let id = identity_with_roles(&["openshell-admin", "openshell-user"]);
224+
let policy = default_policy();
225+
assert!(
226+
policy
227+
.check(&id, "/openshell.v1.OpenShell/CreateWorkspace")
228+
.is_ok()
229+
);
202230
}
203231

204232
#[test]
@@ -253,7 +281,7 @@ mod tests {
253281
};
254282
assert!(
255283
policy
256-
.check(&id, "/openshell.v1.OpenShell/CreateProvider")
284+
.check(&id, "/openshell.v1.OpenShell/CreateWorkspace")
257285
.is_ok()
258286
);
259287
assert!(
@@ -408,7 +436,7 @@ mod tests {
408436
}
409437

410438
#[test]
411-
fn provider_refresh_methods_require_provider_scopes_and_admin_for_writes() {
439+
fn provider_refresh_methods_require_provider_scopes() {
412440
let policy = scoped_policy();
413441
let reader = identity_with_roles_and_scopes(&["openshell-user"], &["provider:read"]);
414442
assert!(
@@ -417,17 +445,26 @@ mod tests {
417445
.is_ok()
418446
);
419447

420-
let writer_without_admin =
421-
identity_with_roles_and_scopes(&["openshell-user"], &["provider:write"]);
422-
let err = policy
423-
.check(
424-
&writer_without_admin,
425-
"/openshell.v1.OpenShell/ConfigureProviderRefresh",
426-
)
427-
.unwrap_err();
428-
assert_eq!(err.code(), tonic::Code::PermissionDenied);
429-
assert!(err.message().contains("openshell-admin"));
448+
// Workspace-admin methods now pass middleware with user role + correct scope.
449+
// Handler enforces workspace membership.
450+
let writer = identity_with_roles_and_scopes(&["openshell-user"], &["provider:write"]);
451+
assert!(
452+
policy
453+
.check(&writer, "/openshell.v1.OpenShell/ConfigureProviderRefresh")
454+
.is_ok()
455+
);
456+
assert!(
457+
policy
458+
.check(&writer, "/openshell.v1.OpenShell/RotateProviderCredential")
459+
.is_ok()
460+
);
461+
assert!(
462+
policy
463+
.check(&writer, "/openshell.v1.OpenShell/DeleteProviderRefresh")
464+
.is_ok()
465+
);
430466

467+
// Wrong scope still rejected.
431468
let admin_without_scope =
432469
identity_with_roles_and_scopes(&["openshell-admin"], &["provider:read"]);
433470
let err = policy
@@ -438,16 +475,6 @@ mod tests {
438475
.unwrap_err();
439476
assert_eq!(err.code(), tonic::Code::PermissionDenied);
440477
assert!(err.message().contains("provider:write"));
441-
442-
let admin_writer =
443-
identity_with_roles_and_scopes(&["openshell-admin"], &["provider:write"]);
444-
for method in [
445-
"/openshell.v1.OpenShell/ConfigureProviderRefresh",
446-
"/openshell.v1.OpenShell/RotateProviderCredential",
447-
"/openshell.v1.OpenShell/DeleteProviderRefresh",
448-
] {
449-
assert!(policy.check(&admin_writer, method).is_ok(), "{method}");
450-
}
451478
}
452479

453480
#[test]
@@ -493,10 +520,16 @@ mod tests {
493520
.check(&id, "/openshell.v1.OpenShell/GetProvider")
494521
.is_ok()
495522
);
496-
// admin methods still denied by role check
523+
// Workspace-admin methods pass middleware with user role.
497524
assert!(
498525
policy
499526
.check(&id, "/openshell.v1.OpenShell/CreateProvider")
527+
.is_ok()
528+
);
529+
// Platform-admin methods still denied by role check.
530+
assert!(
531+
policy
532+
.check(&id, "/openshell.v1.OpenShell/CreateWorkspace")
500533
.is_err()
501534
);
502535
}
@@ -507,7 +540,7 @@ mod tests {
507540
let policy = scoped_policy();
508541
assert!(
509542
policy
510-
.check(&id, "/openshell.v1.OpenShell/CreateProvider")
543+
.check(&id, "/openshell.v1.OpenShell/CreateWorkspace")
511544
.is_ok()
512545
);
513546
assert!(

0 commit comments

Comments
 (0)