Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

f[WIP] eat(sdf): Protect Admin Routes behind a new Administer role #5007

Draft
wants to merge 1 commit into
base: main
Choose a base branch
from
Draft
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 5 additions & 0 deletions component/spicedb/schema.zed
Original file line number Diff line number Diff line change
Expand Up @@ -6,3 +6,8 @@
permission approve = approver+owner
permission manage = owner
}

definition system {
relation admin: user
permission administer = admin
}
7 changes: 7 additions & 0 deletions lib/permissions/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -26,20 +26,23 @@ type Result<T> = result::Result<T, Error>;
pub enum ObjectType {
User,
Workspace,
System,
}

#[derive(Clone, Copy, strum::Display)]
#[strum(serialize_all = "snake_case")]
pub enum Permission {
Approve,
Manage,
Administer,
}

#[derive(Clone, Copy, strum::Display, Debug)]
#[strum(serialize_all = "snake_case")]
pub enum Relation {
Approver,
Owner,
Admin,
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You know - I actually wonder if owner can also just work here. Only it'd be owner of the system vs the workspace

}

/// RelationBuilder allows defining a relationship in SpiceDb.
Expand Down Expand Up @@ -206,6 +209,10 @@ impl PermissionBuilder {
self.object(ObjectType::Workspace, id)
}

pub fn system_object(self) -> Self {
self.object(ObjectType::System, "system")
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do we ever want different permission sets for different systems? Maybe this should be "sdf"

}

pub fn permission(mut self, permission: Permission) -> Self {
self.permission = Some(permission);
self
Expand Down
2 changes: 2 additions & 0 deletions lib/sdf-server/src/middleware.rs
Original file line number Diff line number Diff line change
@@ -1,3 +1,5 @@
mod system_permission;
mod workspace_permission;

pub use self::system_permission::{SystemPermission, SystemPermissionLayer};
pub use self::workspace_permission::{WorkspacePermission, WorkspacePermissionLayer};
96 changes: 96 additions & 0 deletions lib/sdf-server/src/middleware/system_permission.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,96 @@
use std::task::{Context, Poll};

use axum::{
body::Body,
extract::FromRequestParts,
http::Request,
response::{IntoResponse, Response},
};
use futures::future::BoxFuture;
use permissions::{Permission, PermissionBuilder};
use tower::{Layer, Service};

use crate::{
extract::{self, Authorization},
AppState,
};

#[derive(Clone)]
pub struct SystemPermissionLayer {
state: AppState,
permission: Permission,
}

impl SystemPermissionLayer {
pub fn new(state: AppState, permission: Permission) -> Self {
Self { state, permission }
}
}

impl<S> Layer<S> for SystemPermissionLayer {
type Service = SystemPermission<S>;

fn layer(&self, inner: S) -> Self::Service {
SystemPermission {
inner,
state: self.state.clone(),
permission: self.permission,
}
}
}

#[derive(Clone)]
pub struct SystemPermission<S> {
inner: S,
state: AppState,
permission: Permission,
}

impl<S> Service<Request<Body>> for SystemPermission<S>
where
S: Service<Request<Body>, Response = Response> + Clone + Send + 'static,
S::Future: Send + 'static,
{
type Response = S::Response;
type Error = S::Error;
type Future = BoxFuture<'static, Result<Self::Response, Self::Error>>;

fn poll_ready(&mut self, cx: &mut Context<'_>) -> Poll<Result<(), Self::Error>> {
self.inner.poll_ready(cx)
}

fn call(&mut self, req: Request<Body>) -> Self::Future {
let mut me = self.clone();

Box::pin(async move {
let (mut parts, body) = req.into_parts();

let Authorization(claim) =
match Authorization::from_request_parts(&mut parts, &me.state).await {
Ok(claim) => claim,
Err(err) => return Ok(err.into_response()),
};

if let Some(client) = me.state.spicedb_client() {
let is_allowed = match PermissionBuilder::new()
.system_object()
.permission(me.permission)
.user_subject(claim.user_pk.into())
.has_permission(client)
.await
{
Ok(is_allowed) => is_allowed,
Err(_) => return Ok(extract::unauthorized_error().into_response()),
};
if !is_allowed {
return Ok(extract::unauthorized_error().into_response());
}
}

let req = Request::from_parts(parts, body);

let response = me.inner.call(req).await?;
Ok(response)
})
}
}
10 changes: 5 additions & 5 deletions lib/sdf-server/src/service/v2/admin.rs
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@ use serde::{Deserialize, Serialize};
use telemetry::prelude::*;
use thiserror::Error;

use crate::{extract::AdminAccessBuilder, service::ApiError, AppState};
use crate::{middleware::SystemPermissionLayer, service::ApiError, AppState};

mod get_snapshot;
mod kill_execution;
Expand Down Expand Up @@ -188,8 +188,8 @@ pub fn v2_routes(state: AppState) -> Router<AppState> {
post(set_snapshot::set_snapshot),
)
.layer(DefaultBodyLimit::max(MAX_UPLOAD_BYTES))
.route_layer(axum::middleware::from_extractor_with_state::<
AdminAccessBuilder,
AppState,
>(state))
.layer(SystemPermissionLayer::new(
state.clone(),
permissions::Permission::Administer,
))
}