From d71e2cdd4ae1eb932f5112768b62e551fdc2c3cb Mon Sep 17 00:00:00 2001 From: wendybujalski Date: Wed, 24 Jul 2024 15:19:16 -0400 Subject: [PATCH 1/3] feat(sdf) - create workspace permissions system --- .../src/services/workspaces.service.ts | 1 - bin/sdf/src/args.rs | 29 ++++++- lib/dal/src/user.rs | 17 ++++ lib/sdf-server/src/server.rs | 3 +- lib/sdf-server/src/server/config.rs | 54 ++++++++++++ lib/sdf-server/src/server/server.rs | 21 ++++- lib/sdf-server/src/server/service/session.rs | 5 ++ .../server/service/session/auth_connect.rs | 82 ++++++++++++++++--- lib/sdf-server/src/server/state.rs | 15 ++++ lib/si-test-macros/src/sdf_test.rs | 2 + 10 files changed, 214 insertions(+), 15 deletions(-) diff --git a/bin/auth-api/src/services/workspaces.service.ts b/bin/auth-api/src/services/workspaces.service.ts index 0153f1730c..f07530b110 100644 --- a/bin/auth-api/src/services/workspaces.service.ts +++ b/bin/auth-api/src/services/workspaces.service.ts @@ -50,7 +50,6 @@ export async function createWorkspace( instanceUrl, instanceEnvType: newWorkspace.instanceEnvType, isDefaultWorkspace: newWorkspace.isDefault, - // TODO: track env type and other data when it becomes useful }); await prisma.workspaceMembers.create({ diff --git a/bin/sdf/src/args.rs b/bin/sdf/src/args.rs index e84466a6f0..a5224ecb1a 100644 --- a/bin/sdf/src/args.rs +++ b/bin/sdf/src/args.rs @@ -2,7 +2,10 @@ use std::path::PathBuf; use clap::{builder::EnumValueParser, builder::PossibleValuesParser, ArgAction, Parser}; -use sdf_server::{Config, ConfigError, ConfigFile, FeatureFlag, MigrationMode, StandardConfigFile}; +use sdf_server::{ + server::{WorkspacePermissions, WorkspacePermissionsMode}, + Config, ConfigError, ConfigFile, FeatureFlag, MigrationMode, StandardConfigFile, +}; use si_std::SensitiveString; const NAME: &str = "sdf"; @@ -172,7 +175,7 @@ pub(crate) struct Args { #[arg(long, env = "SI_MODULE_INDEX_URL")] pub(crate) module_index_url: Option, - /// The base URL for the module-index API server + /// Allow for Posthog feature flags in SDF #[arg( long, env = "SI_FEATURES", @@ -181,6 +184,19 @@ pub(crate) struct Args { rename_all = "snake_case", )] pub(crate) features: Vec, + + /// Create Workspace Permissions Mode [default: closed] + #[arg(long, env = "SI_CREATE_WORKSPACE_PERMISSIONS", value_parser = PossibleValuesParser::new(WorkspacePermissionsMode::variants()))] + pub(crate) create_workspace_permissions: Option, + + /// Create Workspace Permissions Allowlist + #[arg( + long, + env = "SI_CREATE_WORKSPACE_ALLOWLIST", + value_delimiter = ',', + rename_all = "snake_case" + )] + pub(crate) create_workspace_allowlist: Vec, } impl TryFrom for Config { @@ -282,6 +298,15 @@ impl TryFrom for Config { config_map.set("boot_feature_flags", args.features); + if let Some(create_workspace_permissions) = args.create_workspace_permissions { + config_map.set("create_workspace_permissions", create_workspace_permissions); + } + + config_map.set( + "create_workspace_allowlist", + args.create_workspace_allowlist, + ); + config_map.set("nats.connection_name", NAME); config_map.set("pg.application_name", NAME); config_map.set("layer_db_config.pg_pool_config.application_name", NAME); diff --git a/lib/dal/src/user.rs b/lib/dal/src/user.rs index 73bc563c26..8649f01ea5 100644 --- a/lib/dal/src/user.rs +++ b/lib/dal/src/user.rs @@ -157,6 +157,23 @@ impl User { Ok(()) } + pub async fn is_first_user(&self, ctx: &DalContext) -> UserResult { + let row = ctx + .txns() + .await? + .pg() + .query_opt("SELECT pk FROM users ORDER BY created_at ASC LIMIT 1", &[]) + .await?; + + match row { + Some(row) => { + let oldest_user_pk: UserPk = row.get("pk"); + Ok(oldest_user_pk == self.pk) + } + None => Ok(false), + } + } + pub async fn delete_user_from_workspace( ctx: &DalContext, user_pk: UserPk, diff --git a/lib/sdf-server/src/server.rs b/lib/sdf-server/src/server.rs index 0e4e98a456..1525c67a00 100644 --- a/lib/sdf-server/src/server.rs +++ b/lib/sdf-server/src/server.rs @@ -1,6 +1,7 @@ pub use config::{ detect_and_configure_development, Config, ConfigBuilder, ConfigError, ConfigFile, - IncomingStream, StandardConfig, StandardConfigFile, + IncomingStream, StandardConfig, StandardConfigFile, WorkspacePermissions, + WorkspacePermissionsMode, }; pub use dal::{JobQueueProcessor, MigrationMode, NatsProcessor, ServicesContext}; pub use nats_multiplexer::CRDT_MULTIPLEXER_SUBJECT; diff --git a/lib/sdf-server/src/server/config.rs b/lib/sdf-server/src/server/config.rs index b91da72909..42c2d8a22d 100644 --- a/lib/sdf-server/src/server/config.rs +++ b/lib/sdf-server/src/server/config.rs @@ -1,4 +1,5 @@ use dal::jwt_key::JwtConfig; +use serde_with::{DeserializeFromStr, SerializeDisplay}; use si_crypto::VeritechCryptoConfig; use si_layer_cache::{db::LayerDbConfig, error::LayerDbError}; use std::collections::HashSet; @@ -7,6 +8,7 @@ use std::{ net::{SocketAddr, ToSocketAddrs}, path::{Path, PathBuf}, }; +use strum::{Display, EnumString, VariantNames}; use buck2_resources::Buck2Resources; use dal::feature_flags::FeatureFlag; @@ -26,6 +28,36 @@ pub use si_settings::{StandardConfig, StandardConfigFile}; const DEFAULT_MODULE_INDEX_URL: &str = "https://module-index.systeminit.com"; +#[derive( + Debug, + Default, + Clone, + Copy, + SerializeDisplay, + Display, + DeserializeFromStr, + EnumString, + VariantNames, + PartialEq, + Eq, +)] +#[strum(serialize_all = "camelCase")] +pub enum WorkspacePermissionsMode { + #[default] + Closed, + Allowlist, + Open, +} + +impl WorkspacePermissionsMode { + #[must_use] + pub const fn variants() -> &'static [&'static str] { + ::VARIANTS + } +} + +pub type WorkspacePermissions = String; + #[remain::sorted] #[derive(Debug, Error)] pub enum ConfigError { @@ -88,6 +120,10 @@ pub struct Config { pkgs_path: CanonicalFile, boot_feature_flags: HashSet, + + create_workspace_permissions: WorkspacePermissionsMode, + + create_workspace_allowlist: Vec, } impl StandardConfig for Config { @@ -163,6 +199,16 @@ impl Config { pub fn layer_db_config(&self) -> &LayerDbConfig { &self.layer_db_config } + + // The Create Workspace Permissions Mode should be set via an env variable or it will default to Closed + pub fn create_workspace_permissions(&self) -> &WorkspacePermissionsMode { + &self.create_workspace_permissions + } + + // This Allowlist is a list of email addresses only used in WorkspacePermissionsMode::Allowlist + pub fn create_workspace_allowlist(&self) -> &Vec { + &self.create_workspace_allowlist + } } impl ConfigBuilder { @@ -199,6 +245,10 @@ pub struct ConfigFile { symmetric_crypto_service: SymmetricCryptoServiceConfigFile, #[serde(default)] boot_feature_flags: Vec, + #[serde(default)] + create_workspace_permissions: WorkspacePermissionsMode, + #[serde(default)] + create_workspace_allowlist: Vec, } impl Default for ConfigFile { @@ -215,6 +265,8 @@ impl Default for ConfigFile { module_index_url: default_module_index_url(), symmetric_crypto_service: default_symmetric_crypto_config(), boot_feature_flags: Default::default(), + create_workspace_permissions: Default::default(), + create_workspace_allowlist: Default::default(), } } } @@ -241,6 +293,8 @@ impl TryFrom for Config { config.symmetric_crypto_service(value.symmetric_crypto_service.try_into()?); config.layer_db_config(value.layer_db_config); config.boot_feature_flags(value.boot_feature_flags.into_iter().collect::>()); + config.create_workspace_permissions(value.create_workspace_permissions); + config.create_workspace_allowlist(value.create_workspace_allowlist); config.build().map_err(Into::into) } } diff --git a/lib/sdf-server/src/server/server.rs b/lib/sdf-server/src/server/server.rs index fa4a85055f..73b0d01bfe 100644 --- a/lib/sdf-server/src/server/server.rs +++ b/lib/sdf-server/src/server/server.rs @@ -39,7 +39,10 @@ use ulid::Ulid; use veritech_client::Client as VeritechClient; use super::state::AppState; -use super::{routes, Config, IncomingStream, UdsIncomingStream, UdsIncomingStreamError}; +use super::{ + routes, Config, IncomingStream, UdsIncomingStream, UdsIncomingStreamError, + WorkspacePermissions, WorkspacePermissionsMode, +}; use crate::server::config::VeritechKeyPair; #[remain::sorted] @@ -136,6 +139,8 @@ impl Server<(), ()> { posthog_client, ws_multiplexer_client, crdt_multiplexer_client, + *config.create_workspace_permissions(), + config.create_workspace_allowlist().to_vec(), )?; tokio::spawn(ws_multiplexer.run(shutdown_broadcast_rx.resubscribe())); @@ -180,6 +185,8 @@ impl Server<(), ()> { posthog_client, ws_multiplexer_client, crdt_multiplexer_client, + *config.create_workspace_permissions(), + config.create_workspace_allowlist().to_vec(), )?; tokio::spawn(ws_multiplexer.run(shutdown_broadcast_rx.resubscribe())); @@ -473,6 +480,8 @@ pub fn build_service_for_tests( posthog_client: PosthogClient, ws_multiplexer_client: MultiplexerClient, crdt_multiplexer_client: MultiplexerClient, + create_workspace_permissions: WorkspacePermissionsMode, + create_workspace_allowlist: Vec, ) -> Result<(Router, oneshot::Receiver<()>, broadcast::Receiver<()>)> { build_service_inner( services_context, @@ -481,6 +490,8 @@ pub fn build_service_for_tests( true, ws_multiplexer_client, crdt_multiplexer_client, + create_workspace_permissions, + create_workspace_allowlist, ) } @@ -490,6 +501,8 @@ pub fn build_service( posthog_client: PosthogClient, ws_multiplexer_client: MultiplexerClient, crdt_multiplexer_client: MultiplexerClient, + create_workspace_permissions: WorkspacePermissionsMode, + create_workspace_allowlist: Vec, ) -> Result<(Router, oneshot::Receiver<()>, broadcast::Receiver<()>)> { build_service_inner( services_context, @@ -498,6 +511,8 @@ pub fn build_service( false, ws_multiplexer_client, crdt_multiplexer_client, + create_workspace_permissions, + create_workspace_allowlist, ) } @@ -508,6 +523,8 @@ fn build_service_inner( for_tests: bool, ws_multiplexer_client: MultiplexerClient, crdt_multiplexer_client: MultiplexerClient, + create_workspace_permissions: WorkspacePermissionsMode, + create_workspace_allowlist: Vec, ) -> Result<(Router, oneshot::Receiver<()>, broadcast::Receiver<()>)> { let (shutdown_tx, shutdown_rx) = mpsc::channel(1); let (shutdown_broadcast_tx, shutdown_broadcast_rx) = broadcast::channel(1); @@ -521,6 +538,8 @@ fn build_service_inner( for_tests, ws_multiplexer_client, crdt_multiplexer_client, + create_workspace_permissions, + create_workspace_allowlist, ); let path_filter = Box::new(|path: &str| match path { diff --git a/lib/sdf-server/src/server/service/session.rs b/lib/sdf-server/src/server/service/session.rs index 3e26abdfcb..94a1cdc128 100644 --- a/lib/sdf-server/src/server/service/session.rs +++ b/lib/sdf-server/src/server/service/session.rs @@ -48,6 +48,8 @@ pub enum SessionError { Workspace(#[from] WorkspaceError), #[error("workspace {0} not yet migrated to new snapshot graph version. Migration required")] WorkspaceNotYetMigrated(WorkspacePk), + #[error("you do not have permission to create a workspace on this instance")] + WorkspacePermissions, } #[derive(Debug, Serialize, Deserialize)] @@ -67,6 +69,9 @@ impl IntoResponse for SessionError { Some("WORKSPACE_NOT_INITIALIZED"), None, ), + SessionError::WorkspacePermissions => { + (StatusCode::UNAUTHORIZED, None, Some(self.to_string())) + } _ => (StatusCode::INTERNAL_SERVER_ERROR, None, None), }; diff --git a/lib/sdf-server/src/server/service/session/auth_connect.rs b/lib/sdf-server/src/server/service/session/auth_connect.rs index 7e507361ca..9ea80a5c71 100644 --- a/lib/sdf-server/src/server/service/session/auth_connect.rs +++ b/lib/sdf-server/src/server/service/session/auth_connect.rs @@ -1,6 +1,9 @@ use super::{SessionError, SessionResult}; use crate::server::extract::{HandlerContext, RawAccessToken}; +use crate::server::state::AppState; +use crate::server::{WorkspacePermissions, WorkspacePermissionsMode}; use crate::service::session::AuthApiErrBody; +use axum::extract::State; use axum::Json; use dal::workspace_snapshot::graph::WorkspaceSnapshotGraphDiscriminants; use dal::{DalContext, HistoryActor, KeyPair, Tenancy, User, UserPk, Workspace, WorkspacePk}; @@ -74,6 +77,8 @@ async fn find_or_create_user_and_workspace( mut ctx: DalContext, auth_api_user: AuthApiUser, auth_api_workspace: AuthApiWorkspace, + create_workspace_permissions: WorkspacePermissionsMode, + create_workspace_allowlist: &[String], ) -> SessionResult<(User, Workspace)> { // lookup user or create if we've never seen it before let maybe_user = User::get_by_pk(&ctx, auth_api_user.id).await?; @@ -109,14 +114,26 @@ async fn find_or_create_user_and_workspace( workspace } None => { - let workspace = Workspace::new( - &mut ctx, - auth_api_workspace.id, - auth_api_workspace.display_name, + let create_permission = user_has_permission_to_create_workspace( + &ctx, + &user, + create_workspace_permissions, + create_workspace_allowlist, ) .await?; - let _key_pair = KeyPair::new(&ctx, "default").await?; - workspace + + if create_permission { + let workspace = Workspace::new( + &mut ctx, + auth_api_workspace.id, + auth_api_workspace.display_name, + ) + .await?; + let _key_pair = KeyPair::new(&ctx, "default").await?; + workspace + } else { + return Err(SessionError::WorkspacePermissions); + } } }; @@ -130,6 +147,7 @@ async fn find_or_create_user_and_workspace( pub async fn auth_connect( HandlerContext(builder): HandlerContext, + State(state): State, Json(request): Json, ) -> SessionResult> { let client = reqwest::Client::new(); @@ -157,8 +175,14 @@ pub async fn auth_connect( let ctx = builder.build_default().await?; - let (user, workspace) = - find_or_create_user_and_workspace(ctx, res_body.user, res_body.workspace).await?; + let (user, workspace) = find_or_create_user_and_workspace( + ctx, + res_body.user, + res_body.workspace, + state.create_workspace_permissions(), + state.create_workspace_allowlist(), + ) + .await?; Ok(Json(AuthConnectResponse { user, @@ -170,6 +194,7 @@ pub async fn auth_connect( pub async fn auth_reconnect( HandlerContext(builder): HandlerContext, RawAccessToken(raw_access_token): RawAccessToken, + State(state): State, ) -> SessionResult> { let auth_api_url = match option_env!("LOCAL_AUTH_STACK") { Some(_) => "http://localhost:9001", @@ -196,8 +221,45 @@ pub async fn auth_reconnect( let ctx = builder.build_default().await?; - let (user, workspace) = - find_or_create_user_and_workspace(ctx, res_body.user, res_body.workspace).await?; + let (user, workspace) = find_or_create_user_and_workspace( + ctx, + res_body.user, + res_body.workspace, + state.create_workspace_permissions(), + state.create_workspace_allowlist(), + ) + .await?; Ok(Json(AuthReconnectResponse { user, workspace })) } + +pub async fn user_has_permission_to_create_workspace( + ctx: &DalContext, + user: &User, + mode: WorkspacePermissionsMode, + allowlist: &[WorkspacePermissions], +) -> SessionResult { + match mode { + WorkspacePermissionsMode::Open => Ok(true), + WorkspacePermissionsMode::Closed => Ok(user.is_first_user(ctx).await?), + WorkspacePermissionsMode::Allowlist => { + if user.is_first_user(ctx).await? { + Ok(true) + } else { + let allowed = allowlist.iter().any(|entry| { + if entry.starts_with("*@") { + let mut chars = entry.chars(); + chars.next(); + user.email().ends_with(chars.as_str()) + } else if entry.starts_with('@') { + user.email().ends_with(entry) + } else { + user.email() == entry + } + }); + + Ok(allowed) + } + } + } +} diff --git a/lib/sdf-server/src/server/state.rs b/lib/sdf-server/src/server/state.rs index 20b346f252..c55507a219 100644 --- a/lib/sdf-server/src/server/state.rs +++ b/lib/sdf-server/src/server/state.rs @@ -6,6 +6,7 @@ use std::{ops::Deref, sync::Arc}; use tokio::sync::{broadcast, mpsc, Mutex}; use super::server::ShutdownSource; +use super::{WorkspacePermissions, WorkspacePermissionsMode}; use crate::server::nats_multiplexer::NatsMultiplexerClients; use crate::server::service::ws::crdt::BroadcastGroups; @@ -18,6 +19,8 @@ pub struct AppState { shutdown_broadcast: ShutdownBroadcast, for_tests: bool, nats_multiplexer_clients: NatsMultiplexerClients, + create_workspace_permissions: WorkspacePermissionsMode, + create_workspace_allowlist: Vec, // TODO(fnichol): we're likely going to use this, but we can't allow it to be dropped because // that will trigger the read side and... shutdown. Cool, no? @@ -36,6 +39,8 @@ impl AppState { for_tests: bool, ws_multiplexer_client: MultiplexerClient, crdt_multiplexer_client: MultiplexerClient, + create_workspace_permissions: WorkspacePermissionsMode, + create_workspace_allowlist: Vec, ) -> Self { let nats_multiplexer_clients = NatsMultiplexerClients { ws: Arc::new(Mutex::new(ws_multiplexer_client)), @@ -50,6 +55,8 @@ impl AppState { for_tests, nats_multiplexer_clients, _tmp_shutdown_tx: Arc::new(tmp_shutdown_tx), + create_workspace_permissions, + create_workspace_allowlist, } } @@ -68,6 +75,14 @@ impl AppState { pub fn for_tests(&self) -> bool { self.for_tests } + + pub fn create_workspace_permissions(&self) -> WorkspacePermissionsMode { + self.create_workspace_permissions + } + + pub fn create_workspace_allowlist(&self) -> &[String] { + &self.create_workspace_allowlist + } } #[derive(Clone, Debug, FromRef)] diff --git a/lib/si-test-macros/src/sdf_test.rs b/lib/si-test-macros/src/sdf_test.rs index 37879ff94b..d43e737cdd 100644 --- a/lib/si-test-macros/src/sdf_test.rs +++ b/lib/si-test-macros/src/sdf_test.rs @@ -385,6 +385,8 @@ impl SdfTestFnSetupExpander { #posthog_client, #ws_multiplexer_client, #crdt_multiplexer_client, + ::sdf_server::server::WorkspacePermissionsMode::Open, + vec![], ).wrap_err("failed to build sdf router")?; service }; From 53d3377cebee3f25e981e3ed05d1ceca7d8b7729 Mon Sep 17 00:00:00 2001 From: wendybujalski Date: Wed, 24 Jul 2024 15:42:46 -0400 Subject: [PATCH 2/3] fix lint --- lib/sdf-server/src/server/server.rs | 1 + 1 file changed, 1 insertion(+) diff --git a/lib/sdf-server/src/server/server.rs b/lib/sdf-server/src/server/server.rs index 73b0d01bfe..5ac88a3913 100644 --- a/lib/sdf-server/src/server/server.rs +++ b/lib/sdf-server/src/server/server.rs @@ -516,6 +516,7 @@ pub fn build_service( ) } +#[allow(clippy::too_many_arguments)] fn build_service_inner( services_context: ServicesContext, jwt_public_signing_key: JwtPublicSigningKey, From 33b7c709270e82433c4f3b491f90692278645371 Mon Sep 17 00:00:00 2001 From: Scott Prutton Date: Wed, 24 Jul 2024 17:12:12 -0400 Subject: [PATCH 3/3] feat: adjust configuration for init container --- component/init/configs/service.toml | 2 ++ 1 file changed, 2 insertions(+) diff --git a/component/init/configs/service.toml b/component/init/configs/service.toml index e46184211a..9555634492 100644 --- a/component/init/configs/service.toml +++ b/component/init/configs/service.toml @@ -1,4 +1,6 @@ pkgs_path = "/tmp" +create_workspace_permissions = "$SI_WORKSPACE_PERMISSIONS" +create_workspace_allowlist = [ "$SI_WORKSPACE_ALLOW_LIST" ] [crypto] encryption_key_base64 = "$SI_ENCRYPTION_KEY_BASE64"