From 6343295c9d25375d5b6a4187abc6b9ea40f467cd Mon Sep 17 00:00:00 2001 From: James MacMahon Date: Fri, 25 Feb 2022 16:28:46 -0500 Subject: [PATCH 01/24] Introduce concept of Silos Add silos, which will isolate organizations, and provide a namespace for users and groups. This required adding Silo id to Actor, so users that have authenticated now have an associated Silo id that can be used to restrict organization lookup. Silos can be created, read, and deleted. modification is a TODO. A few tests have been modified to use `authn_as` because an earlier version of this branch added OpContext to every endpoint, but that was reverted because the blast radius of the PR would have been too large. What remains are a few modified tests that make authenticated calls. When all endpoints are protected and each datastore function has an OpContext, Silo can be looked up on Actor. For now, there are places hard coding as the built-in Silo. Still TODO: - authz for silos and silo users - some testing is dependent on ^ - PUT /silos/{name} - building on top of silos --- common/src/api/external/mod.rs | 2 + common/src/sql/dbinit.sql | 53 ++- nexus/src/authn/external/mod.rs | 16 +- nexus/src/authn/external/session_cookie.rs | 6 +- nexus/src/authn/external/spoof.rs | 31 +- nexus/src/authn/mod.rs | 50 +- nexus/src/authz/actor.rs | 2 +- nexus/src/authz/roles.rs | 4 +- nexus/src/context.rs | 19 +- nexus/src/db/datastore.rs | 430 ++++++++++++++++-- nexus/src/db/fixed_data/mod.rs | 3 +- nexus/src/db/fixed_data/silo_builtin.rs | 11 + nexus/src/db/fixed_data/user_builtin.rs | 11 + nexus/src/db/model.rs | 92 +++- nexus/src/db/schema.rs | 29 ++ nexus/src/external_api/console_api.rs | 16 +- nexus/src/external_api/http_entrypoints.rs | 134 +++++- nexus/src/external_api/params.rs | 15 + nexus/src/external_api/views.rs | 24 +- nexus/src/nexus.rs | 80 +++- nexus/src/populate.rs | 4 + nexus/test-utils/src/http_testing.rs | 24 + nexus/test-utils/src/resource_helpers.rs | 39 +- nexus/tests/integration_tests/authn_http.rs | 9 +- nexus/tests/integration_tests/basic.rs | 215 +++++---- nexus/tests/integration_tests/disks.rs | 29 +- nexus/tests/integration_tests/mod.rs | 1 + .../tests/integration_tests/organizations.rs | 17 +- nexus/tests/integration_tests/silos.rs | 149 ++++++ nexus/tests/output/nexus_tags.txt | 7 + openapi/nexus.json | 249 ++++++++++ 31 files changed, 1574 insertions(+), 197 deletions(-) create mode 100644 nexus/src/db/fixed_data/silo_builtin.rs create mode 100644 nexus/tests/integration_tests/silos.rs diff --git a/common/src/api/external/mod.rs b/common/src/api/external/mod.rs index a890106e10b..7837db9fa00 100644 --- a/common/src/api/external/mod.rs +++ b/common/src/api/external/mod.rs @@ -564,6 +564,8 @@ impl TryFrom for Generation { #[display(style = "kebab-case")] pub enum ResourceType { Fleet, + Silo, + SiloUser, Organization, Project, Dataset, diff --git a/common/src/sql/dbinit.sql b/common/src/sql/dbinit.sql index 531f37f26f0..317b0990245 100644 --- a/common/src/sql/dbinit.sql +++ b/common/src/sql/dbinit.sql @@ -172,6 +172,52 @@ CREATE TABLE omicron.public.volume ( data TEXT NOT NULL ); +/* + * Silos + */ + +CREATE TABLE omicron.public.silo ( + /* Identity metadata */ + id UUID PRIMARY KEY, + + name STRING(128) NOT NULL, + description STRING(512) NOT NULL, + + discoverable BOOL NOT NULL, + + time_created TIMESTAMPTZ NOT NULL, + time_modified TIMESTAMPTZ NOT NULL, + time_deleted TIMESTAMPTZ, + + /* child resource generation number, per RFD 192 */ + rcgen INT NOT NULL +); + +CREATE UNIQUE INDEX ON omicron.public.silo ( + name +) WHERE + time_deleted IS NULL; + +/* + * Silo users + */ +CREATE TABLE omicron.public.silo_user ( + /* silo user id */ + id UUID PRIMARY KEY, + + silo_id UUID NOT NULL, + + /* XXX SCIM parameters? */ + name TEXT NOT NULL, + + time_created TIMESTAMPTZ NOT NULL, + time_modified TIMESTAMPTZ NOT NULL, + time_deleted TIMESTAMPTZ, + + /* internal user id used by authz */ + internal_user_id UUID NOT NULL +); + /* * Organizations */ @@ -179,6 +225,10 @@ CREATE TABLE omicron.public.volume ( CREATE TABLE omicron.public.organization ( /* Identity metadata */ id UUID PRIMARY KEY, + + /* FK into Silo table */ + silo_id UUID NOT NULL, + name STRING(63) NOT NULL, description STRING(512) NOT NULL, time_created TIMESTAMPTZ NOT NULL, @@ -707,8 +757,7 @@ CREATE TABLE omicron.public.console_session ( token STRING(40) PRIMARY KEY, time_created TIMESTAMPTZ NOT NULL, time_last_used TIMESTAMPTZ NOT NULL, - -- we're agnostic about what this means until work starts on users, but the - -- naive interpretation is that it points to a row in the User table + -- this maps to internal_user_id in the silo_user table user_id UUID NOT NULL ); diff --git a/nexus/src/authn/external/mod.rs b/nexus/src/authn/external/mod.rs index 2521059166a..e50c15c0c59 100644 --- a/nexus/src/authn/external/mod.rs +++ b/nexus/src/authn/external/mod.rs @@ -22,7 +22,7 @@ impl Authenticator where T: Send + Sync + 'static, { - /// Build a new authentiator that allows only the specified schemes + /// Build a new authenticator that allows only the specified schemes pub fn new( allowed_schemes: Vec>>, ) -> Authenticator { @@ -187,9 +187,10 @@ mod test { let count1 = Arc::new(AtomicU8::new(0)); let mut expected_count1 = 0; let name1 = authn::SchemeName("grunt1"); - let actor1 = authn::Actor( - "1c91bab2-4841-669f-cc32-de80da5bbf39".parse().unwrap(), - ); + let actor1 = authn::Actor { + id: "1c91bab2-4841-669f-cc32-de80da5bbf39".parse().unwrap(), + silo_id: *crate::db::fixed_data::silo_builtin::SILO_ID, + }; let grunt1 = Box::new(GruntScheme { name: name1, next: Arc::clone(&flag1), @@ -201,9 +202,10 @@ mod test { let count2 = Arc::new(AtomicU8::new(0)); let mut expected_count2 = 0; let name2 = authn::SchemeName("grunt2"); - let actor2 = authn::Actor( - "799684af-533a-cb66-b5ac-ab55a791d5ef".parse().unwrap(), - ); + let actor2 = authn::Actor { + id: "799684af-533a-cb66-b5ac-ab55a791d5ef".parse().unwrap(), + silo_id: *crate::db::fixed_data::silo_builtin::SILO_ID, + }; let grunt2 = Box::new(GruntScheme { name: name2, next: Arc::clone(&flag2), diff --git a/nexus/src/authn/external/session_cookie.rs b/nexus/src/authn/external/session_cookie.rs index 863106b80dc..1f15d923885 100644 --- a/nexus/src/authn/external/session_cookie.rs +++ b/nexus/src/authn/external/session_cookie.rs @@ -18,6 +18,7 @@ use uuid::Uuid; pub trait Session { fn user_id(&self) -> Uuid; + fn silo_id(&self) -> Uuid; fn time_last_used(&self) -> DateTime; fn time_created(&self) -> DateTime; } @@ -104,7 +105,7 @@ where } }; - let actor = Actor(session.user_id()); + let actor = Actor { id: session.user_id(), silo_id: session.silo_id() }; // if the session has gone unused for longer than idle_timeout, it is expired let now = Utc::now(); @@ -196,6 +197,9 @@ mod test { fn user_id(&self) -> Uuid { Uuid::new_v4() } + fn silo_id(&self) -> Uuid { + Uuid::new_v4() + } fn time_created(&self) -> DateTime { self.time_created } diff --git a/nexus/src/authn/external/spoof.rs b/nexus/src/authn/external/spoof.rs index 06232bc79f3..da1500ba16a 100644 --- a/nexus/src/authn/external/spoof.rs +++ b/nexus/src/authn/external/spoof.rs @@ -55,8 +55,10 @@ const SPOOF_PREFIX: &str = "oxide-spoof-"; lazy_static! { /// Actor (id) used for the special "bad credentials" error - static ref SPOOF_RESERVED_BAD_CREDS_ACTOR: Actor = - Actor("22222222-2222-2222-2222-222222222222".parse().unwrap()); + static ref SPOOF_RESERVED_BAD_CREDS_ACTOR: Actor = Actor { + id: "22222222-2222-2222-2222-222222222222".parse().unwrap(), + silo_id: *crate::db::fixed_data::silo_builtin::SILO_ID, + }; /// Complete HTTP header value to trigger the "bad actor" error pub static ref SPOOF_HEADER_BAD_ACTOR: Authorization = make_header_value_str(SPOOF_RESERVED_BAD_ACTOR).unwrap(); @@ -119,7 +121,13 @@ fn authn_spoof(raw_value: Option<&Authorization>) -> SchemeResult { } match Uuid::parse_str(str_value).context("parsing header value as UUID") { - Ok(id) => SchemeResult::Authenticated(Details { actor: Actor(id) }), + Ok(id) => { + let actor = Actor { + id, + silo_id: *crate::db::fixed_data::silo_builtin::SILO_ID, + }; + SchemeResult::Authenticated(Details { actor }) + } Err(source) => SchemeResult::Failed(Reason::BadFormat { source }), } } @@ -160,15 +168,12 @@ pub fn make_header_value_raw( #[cfg(test)] mod test { - use super::super::super::Details; use super::super::super::Reason; use super::super::SchemeResult; use super::authn_spoof; use super::make_header_value; use super::make_header_value_raw; use super::make_header_value_str; - use crate::authn; - use authn::Actor; use headers::authorization::Bearer; use headers::authorization::Credentials; use headers::Authorization; @@ -228,12 +233,14 @@ mod test { // Success case: the client provided a valid uuid in the header. let success_case = authn_spoof(Some(&test_header)); - assert!(matches!( - success_case, - SchemeResult::Authenticated( - Details { actor: Actor(i) } - ) if i == test_uuid - )); + match success_case { + SchemeResult::Authenticated(details) => { + assert_eq!(details.actor.id, test_uuid); + } + _ => { + assert!(false); + } + }; } #[test] diff --git a/nexus/src/authn/mod.rs b/nexus/src/authn/mod.rs index c6c9df3661b..d7306a7a2e6 100644 --- a/nexus/src/authn/mod.rs +++ b/nexus/src/authn/mod.rs @@ -63,6 +63,7 @@ impl Context { } /// Returns the authenticated actor if present, Unauthenticated error otherwise + // TODO this should maybe return omicron_common::api::external::Error pub fn actor_required(&self) -> Result<&Actor, dropshot::HttpError> { match &self.kind { Kind::Authenticated(Details { actor }) => Ok(actor), @@ -74,6 +75,19 @@ impl Context { } } + pub fn silo_required( + &self, + ) -> Result { + match self.actor_required() { + Ok(actor) => Ok(actor.silo_id), + Err(_) => { + Err(omicron_common::api::external::Error::Unauthenticated { + internal_message: "Actor required".to_string(), + }) + } + } + } + /// Returns the list of schemes tried, in order /// /// This should generally *not* be exposed to clients. @@ -88,23 +102,31 @@ impl Context { /// Returns an authenticated context for handling internal API contexts pub fn internal_api() -> Context { - Context::context_for_actor(USER_INTERNAL_API.id) + Context::context_for_actor( + USER_INTERNAL_API.id, + USER_INTERNAL_API.silo_id, + ) } /// Returns an authenticated context for saga recovery pub fn internal_saga_recovery() -> Context { - Context::context_for_actor(USER_SAGA_RECOVERY.id) + Context::context_for_actor( + USER_SAGA_RECOVERY.id, + USER_SAGA_RECOVERY.silo_id, + ) } /// Returns an authenticated context for Nexus-startup database /// initialization pub fn internal_db_init() -> Context { - Context::context_for_actor(USER_DB_INIT.id) + Context::context_for_actor(USER_DB_INIT.id, USER_DB_INIT.silo_id) } - fn context_for_actor(actor_id: Uuid) -> Context { + fn context_for_actor(actor_id: Uuid, silo_id: Uuid) -> Context { Context { - kind: Kind::Authenticated(Details { actor: Actor(actor_id) }), + kind: Kind::Authenticated(Details { + actor: Actor { id: actor_id, silo_id: silo_id }, + }), schemes_tried: Vec::new(), } } @@ -118,7 +140,10 @@ impl Context { /// /// This is used for testing. pub fn test_context_for_actor(actor_id: Uuid) -> Context { - Context::context_for_actor(actor_id) + Context::context_for_actor( + actor_id, + *crate::db::fixed_data::silo_builtin::SILO_ID, + ) } } @@ -141,19 +166,19 @@ mod test { // The privileges are (or will be) verified in authz tests. let authn = Context::internal_test_user(); let actor = authn.actor().unwrap(); - assert_eq!(actor.0, USER_TEST_PRIVILEGED.id); + assert_eq!(actor.id, USER_TEST_PRIVILEGED.id); let authn = Context::internal_db_init(); let actor = authn.actor().unwrap(); - assert_eq!(actor.0, USER_DB_INIT.id); + assert_eq!(actor.id, USER_DB_INIT.id); let authn = Context::internal_saga_recovery(); let actor = authn.actor().unwrap(); - assert_eq!(actor.0, USER_SAGA_RECOVERY.id); + assert_eq!(actor.id, USER_SAGA_RECOVERY.id); let authn = Context::internal_api(); let actor = authn.actor().unwrap(); - assert_eq!(actor.0, USER_INTERNAL_API.id); + assert_eq!(actor.id, USER_INTERNAL_API.id); } } @@ -179,7 +204,10 @@ pub struct Details { /// Who is performing an operation #[derive(Clone, Copy, Debug, Deserialize, Eq, PartialEq, Serialize)] -pub struct Actor(pub Uuid); +pub struct Actor { + pub id: Uuid, + pub silo_id: Uuid, +} /// Label for a particular authentication scheme (used in log messages and /// internal error messages) diff --git a/nexus/src/authz/actor.rs b/nexus/src/authz/actor.rs index 6d4c9df0afb..572262c8354 100644 --- a/nexus/src/authz/actor.rs +++ b/nexus/src/authz/actor.rs @@ -23,7 +23,7 @@ impl AnyActor { let actor = authn.actor(); AnyActor { authenticated: actor.is_some(), - actor_id: actor.map(|a| a.0), + actor_id: actor.map(|a| a.id), roles, } } diff --git a/nexus/src/authz/roles.rs b/nexus/src/authz/roles.rs index 8ada1131e34..1dc9f3210a0 100644 --- a/nexus/src/authz/roles.rs +++ b/nexus/src/authz/roles.rs @@ -136,7 +136,7 @@ pub async fn load_roles_for_resource( // ... then fetch all the roles for this user that are associated with // this resource. trace!(opctx.log, "loading roles"; - "actor_id" => actor_id.0.to_string(), + "actor_id" => actor_id.id.to_string(), "resource_type" => ?resource_type, "resource_id" => resource_id.to_string(), ); @@ -144,7 +144,7 @@ pub async fn load_roles_for_resource( let roles = datastore .role_asgn_builtin_list_for( opctx, - actor_id.0, + actor_id.id, resource_type, resource_id, ) diff --git a/nexus/src/context.rs b/nexus/src/context.rs index 25fc6201ec5..92023f0fad8 100644 --- a/nexus/src/context.rs +++ b/nexus/src/context.rs @@ -11,9 +11,8 @@ use super::config; use super::db; use super::Nexus; use crate::authn::external::session_cookie::{Session, SessionStore}; -use crate::authn::Actor; use crate::authz::AuthorizedResource; -use crate::db::model::ConsoleSession; +use crate::db::model::ConsoleSessionWithSiloId; use crate::db::DataStore; use crate::saga_interface::SagaContext; use async_trait::async_trait; @@ -280,7 +279,8 @@ impl OpContext { ) -> (slog::Logger, BTreeMap) { let mut metadata = BTreeMap::new(); - let log = if let Some(Actor(actor_id)) = authn.actor() { + let log = if let Some(actor) = authn.actor() { + let actor_id = actor.id; metadata .insert(String::from("authenticated"), String::from("true")); metadata.insert(String::from("actor"), actor_id.to_string()); @@ -501,7 +501,7 @@ mod test { #[async_trait] impl SessionStore for Arc { - type SessionModel = ConsoleSession; + type SessionModel = ConsoleSessionWithSiloId; async fn session_fetch(&self, token: String) -> Option { self.nexus.session_fetch(token).await.ok() @@ -527,14 +527,17 @@ impl SessionStore for Arc { } } -impl Session for ConsoleSession { +impl Session for ConsoleSessionWithSiloId { fn user_id(&self) -> Uuid { - self.user_id + self.console_session.user_id + } + fn silo_id(&self) -> Uuid { + self.silo_id } fn time_last_used(&self) -> DateTime { - self.time_last_used + self.console_session.time_last_used } fn time_created(&self) -> DateTime { - self.time_created + self.console_session.time_created } } diff --git a/nexus/src/db/datastore.rs b/nexus/src/db/datastore.rs index 39c1f00d0a0..d81d6d14108 100644 --- a/nexus/src/db/datastore.rs +++ b/nexus/src/db/datastore.rs @@ -34,6 +34,7 @@ use crate::authz; use crate::context::OpContext; use crate::db::fixed_data::role_assignment_builtin::BUILTIN_ROLE_ASSIGNMENTS; use crate::db::fixed_data::role_builtin::BUILTIN_ROLES; +use crate::db::fixed_data::silo_builtin::SILO_ID; use crate::external_api::params; use async_bb8_diesel::{ AsyncConnection, AsyncRunQueryDsl, ConnectionError, ConnectionManager, @@ -68,12 +69,12 @@ use crate::db::{ self, error::{public_error_from_diesel_pool, ErrorHandler, TransactionError}, model::{ - ConsoleSession, Dataset, DatasetKind, Disk, DiskRuntimeState, + ConsoleSession, ConsoleSessionWithSiloId, Dataset, DatasetKind, Disk, DiskRuntimeState, Generation, IncompleteNetworkInterface, Instance, InstanceRuntimeState, Name, NetworkInterface, Organization, OrganizationUpdate, OximeterInfo, ProducerEndpoint, Project, ProjectUpdate, Region, RoleAssignmentBuiltin, RoleBuiltin, RouterRoute, RouterRouteUpdate, - Sled, UpdateArtifactKind, UpdateAvailableArtifact, UserBuiltin, Volume, + Silo, SiloUser, Sled, UpdateArtifactKind, UpdateAvailableArtifact, UserBuiltin, Volume, Vpc, VpcFirewallRule, VpcRouter, VpcRouterUpdate, VpcSubnet, VpcSubnetUpdate, VpcUpdate, Zpool, }, @@ -544,20 +545,26 @@ impl DataStore { opctx.authorize(authz::Action::CreateChild, &authz::FLEET).await?; let name = organization.name().as_str().to_string(); - diesel::insert_into(dsl::organization) - .values(organization) - .returning(Organization::as_returning()) - .get_result_async(self.pool_authorized(opctx).await?) - .await - .map_err(|e| { + let silo_id = organization.silo_id(); + + Silo::insert_resource( + silo_id, + diesel::insert_into(dsl::organization).values(organization), + ) + .insert_and_get_result_async(self.pool_authorized(opctx).await?) + .await + .map_err(|e| match e { + AsyncInsertError::CollectionNotFound => Error::ObjectNotFound { + type_name: ResourceType::Silo, + lookup_type: LookupType::ById(silo_id), + }, + AsyncInsertError::DatabaseError(e) => { public_error_from_diesel_pool( e, - ErrorHandler::Conflict( - ResourceType::Organization, - name.as_str(), - ), + ErrorHandler::Conflict(ResourceType::Organization, &name), ) - }) + } + }) } /// Fetches an Organization from the database and returns both the database @@ -591,12 +598,20 @@ impl DataStore { // doing an authz check. async fn organization_lookup_noauthz( &self, + // XXX enable when all endpoints are protected by authn + // opctx: &OpContext, name: &Name, ) -> LookupResult<(authz::Organization, Organization)> { use db::schema::organization::dsl; + + // XXX uncomment when all endpoints are protected by authn + //let silo_id = opctx.authn.silo_required()?; + let silo_id = *SILO_ID; + dsl::organization .filter(dsl::time_deleted.is_null()) .filter(dsl::name.eq(name.clone())) + .filter(dsl::silo_id.eq(silo_id)) .select(Organization::as_select()) .get_result_async::(self.pool()) .await @@ -621,6 +636,7 @@ impl DataStore { /// Fetch an [`authz::Organization`] based on its id pub async fn organization_lookup_by_id( &self, + // TODO: OpContext, to verify actor has permission to lookup organization_id: Uuid, ) -> LookupResult { use db::schema::organization::dsl; @@ -2670,9 +2686,9 @@ impl DataStore { pub async fn session_fetch( &self, token: String, - ) -> LookupResult { + ) -> LookupResult { use db::schema::console_session::dsl; - dsl::console_session + let console_session = dsl::console_session .filter(dsl::token.eq(token.clone())) .select(ConsoleSession::as_select()) .first_async(self.pool()) @@ -2682,7 +2698,19 @@ impl DataStore { "error fetching session: {:?}", e )) - }) + })?; + + let silo_id = self + .get_silo_id_from_internal_user_id(console_session.user_id) + .await + .map_err(|e| { + Error::internal_error(&format!( + "error fetching silo id: {:?}", + e + )) + })?; + + Ok(ConsoleSessionWithSiloId { console_session, silo_id }) } pub async fn session_create( @@ -2707,10 +2735,10 @@ impl DataStore { pub async fn session_update_last_used( &self, token: String, - ) -> UpdateResult { + ) -> UpdateResult { use db::schema::console_session::dsl; - diesel::update(dsl::console_session) + let console_session = diesel::update(dsl::console_session) .filter(dsl::token.eq(token.clone())) .set((dsl::time_last_used.eq(Utc::now()),)) .returning(ConsoleSession::as_returning()) @@ -2721,7 +2749,19 @@ impl DataStore { "error renewing session: {:?}", e )) - }) + })?; + + let silo_id = self + .get_silo_id_from_internal_user_id(console_session.user_id) + .await + .map_err(|e| { + Error::internal_error(&format!( + "error fetching silo id: {:?}", + e + )) + })?; + + Ok(ConsoleSessionWithSiloId { console_session, silo_id }) } // putting "hard" in the name because we don't do this with any other model @@ -2817,6 +2857,20 @@ impl DataStore { }) .collect::>(); + debug!(opctx.log, "creating silo_user entries for built-in users"); + + for builtin_user in &builtin_users { + self.silo_user_create(SiloUser::new( + *SILO_ID, + Uuid::new_v4(), /* silo user id */ + builtin_user.identity.name.to_string(), /* name */ + builtin_user.identity.id, /* internal user id */ + )) + .await?; + } + + info!(opctx.log, "created silo_user entries for built-in users"); + debug!(opctx.log, "attempting to create built-in users"); let count = diesel::insert_into(dsl::user_builtin) .values(builtin_users) @@ -2828,6 +2882,7 @@ impl DataStore { public_error_from_diesel_pool(e, ErrorHandler::Server) })?; info!(opctx.log, "created {} built-in users", count); + Ok(()) } @@ -3040,6 +3095,298 @@ impl DataStore { )) }) } + + pub async fn get_silo_id_from_silo_user_id( + &self, + silo_user_id: Uuid, + ) -> LookupResult { + let silo_user: SiloUser = + self.silo_user_fetch_by_id(silo_user_id).await?; + + Ok(silo_user.silo_id) + } + + pub async fn get_silo_id_from_internal_user_id( + &self, + internal_user_id: Uuid, + ) -> LookupResult { + let silo_user: SiloUser = + self.silo_user_fetch_by_internal_id(internal_user_id).await?; + + Ok(silo_user.silo_id) + } + + pub async fn silo_user_create( + &self, + silo_user: SiloUser, + ) -> CreateResult { + use db::schema::silo_user::dsl; + + diesel::insert_into(dsl::silo_user) + .values(silo_user) + .returning(SiloUser::as_returning()) + .get_result_async(self.pool()) + .await + .map_err(|e| { + Error::internal_error(&format!( + "error creating silo user: {:?}", + e + )) + }) + } + + pub async fn silo_user_fetch_by_id( + &self, + id: Uuid, + ) -> CreateResult { + use db::schema::silo_user::dsl; + + dsl::silo_user + .filter(dsl::id.eq(id)) + .filter(dsl::time_deleted.is_null()) + .select(SiloUser::as_select()) + .first_async(self.pool()) + .await + .map_err(|e| { + Error::internal_error(&format!( + "error fetching silo user: {:?}", + e + )) + }) + } + + pub async fn silo_user_fetch_by_internal_id( + &self, + internal_user_id: Uuid, + ) -> CreateResult { + use db::schema::silo_user::dsl; + + dsl::silo_user + .filter(dsl::internal_user_id.eq(internal_user_id)) + .filter(dsl::time_deleted.is_null()) + .select(SiloUser::as_select()) + .first_async(self.pool()) + .await + .map_err(|e| { + Error::internal_error(&format!( + "error fetching silo user: {:?}", + e + )) + }) + } + + /// Load built-in silos into the database + pub async fn load_builtin_silos( + &self, + opctx: &OpContext, + ) -> Result<(), Error> { + debug!(opctx.log, "attempting to create built-in silo"); + + let builtin_silo = Silo::new_with_id( + *SILO_ID, + params::SiloCreate { + identity: IdentityMetadataCreateParams { + name: "fakesilo".parse().unwrap(), + description: "fake silo".to_string(), + }, + discoverable: false, + }, + ); + + let _create_result = self.silo_create(opctx, builtin_silo).await?; + info!(opctx.log, "created built-in silo"); + + Ok(()) + } + + pub async fn silo_create( + &self, + opctx: &OpContext, + silo: Silo, + ) -> CreateResult { + use db::schema::silo::dsl; + + // TODO opctx.authorize + + let silo_id = silo.id(); + + diesel::insert_into(dsl::silo) + .values(silo) + .returning(Silo::as_returning()) + .get_result_async(self.pool_authorized(opctx).await?) + .await + .map_err(|e| { + public_error_from_diesel_pool( + e, + ErrorHandler::Conflict( + ResourceType::Silo, + silo_id.to_string().as_str(), + ), + ) + }) + } + + pub async fn silos_list_by_id( + &self, + opctx: &OpContext, + pagparams: &DataPageParams<'_, Uuid>, + ) -> ListResultVec { + use db::schema::silo::dsl; + // TODO opctx.authorize + paginated(dsl::silo, dsl::id, pagparams) + .filter(dsl::time_deleted.is_null()) + .filter(dsl::discoverable.eq(true)) + .select(Silo::as_select()) + .load_async::(self.pool_authorized(opctx).await?) + .await + .map_err(|e| public_error_from_diesel_pool(e, ErrorHandler::Server)) + } + + pub async fn silos_list_by_name( + &self, + opctx: &OpContext, + pagparams: &DataPageParams<'_, Name>, + ) -> ListResultVec { + use db::schema::silo::dsl; + // TODO opctx.authorize + paginated(dsl::silo, dsl::name, pagparams) + .filter(dsl::time_deleted.is_null()) + .filter(dsl::discoverable.eq(true)) + .select(Silo::as_select()) + .load_async::(self.pool_authorized(opctx).await?) + .await + .map_err(|e| public_error_from_diesel_pool(e, ErrorHandler::Server)) + } + + pub async fn silo_fetch( + &self, + _opctx: &OpContext, + name: &Name, + ) -> LookupResult { + // TODO opctx.authorize + self.silo_lookup_noauthz(name).await + } + + pub async fn silo_lookup_noauthz( + &self, + // XXX enable when all endpoints are protected by authn + // opctx: &OpContext, + name: &Name, + ) -> LookupResult { + use db::schema::silo::dsl; + + // TODO opctx.authorize + + dsl::silo + .filter(dsl::time_deleted.is_null()) + .filter(dsl::name.eq(name.clone())) + .select(Silo::as_select()) + .get_result_async::(self.pool()) + .await + .map_err(|e| { + public_error_from_diesel_pool( + e, + ErrorHandler::NotFoundByLookup( + ResourceType::Silo, + LookupType::ByName(name.as_str().to_owned()), + ), + ) + }) + } + + pub async fn silo_delete( + &self, + opctx: &OpContext, + name: &Name, + ) -> DeleteResult { + use db::schema::organization; + use db::schema::silo; + use db::schema::silo_user; + + // TODO opctx.authorize + + let (id, rcgen) = silo::dsl::silo + .filter(silo::dsl::time_deleted.is_null()) + .filter(silo::dsl::name.eq(name.clone())) + .select((silo::dsl::id, silo::dsl::rcgen)) + .get_result_async::<(Uuid, Generation)>(self.pool()) + .await + .map_err(|e| { + public_error_from_diesel_pool( + e, + ErrorHandler::NotFoundByLookup( + ResourceType::Silo, + LookupType::ByName(name.to_string()), + ), + ) + })?; + + // Make sure there are no organizations present within this silo. + let org_found = diesel_pool_result_optional( + organization::dsl::organization + .filter(organization::dsl::silo_id.eq(id)) + .filter(organization::dsl::time_deleted.is_null()) + .select(organization::dsl::id) + .limit(1) + .first_async::(self.pool()) + .await, + ) + .map_err(|e| public_error_from_diesel_pool(e, ErrorHandler::Server))?; + + if org_found.is_some() { + return Err(Error::InvalidRequest { + message: "silo to be deleted contains an organization" + .to_string(), + }); + } + + let now = Utc::now(); + let updated_rows = diesel::update(silo::dsl::silo) + .filter(silo::dsl::time_deleted.is_null()) + .filter(silo::dsl::id.eq(id)) + .filter(silo::dsl::rcgen.eq(rcgen)) + .set(silo::dsl::time_deleted.eq(now)) + .execute_async(self.pool()) + .await + .map_err(|e| { + public_error_from_diesel_pool( + e, + ErrorHandler::NotFoundByLookup( + ResourceType::Silo, + LookupType::ById(id), + ), + ) + })?; + + if updated_rows == 0 { + return Err(Error::InvalidRequest { + message: "silo deletion failed due to concurrent modification" + .to_string(), + }); + } + + info!(opctx.log, "deleted silo {}", id); + + // If silo deletion succeeded, delete all silo users + let updated_rows = diesel::update(silo_user::dsl::silo_user) + .filter(silo_user::dsl::silo_id.eq(id)) + .set(silo_user::dsl::time_deleted.eq(now)) + .execute_async(self.pool()) + .await + .map_err(|e| { + public_error_from_diesel_pool( + e, + ErrorHandler::NotFoundByLookup( + ResourceType::Silo, + LookupType::ById(id), + ), + ) + })?; + + info!(opctx.log, "deleted {} silo users for silo {}", updated_rows, id,); + + Ok(()) + } } /// Constructs a DataStore for use in test suites that has preloaded the @@ -3065,6 +3412,7 @@ pub async fn datastore_test( datastore.load_builtin_users(&opctx).await.unwrap(); datastore.load_builtin_roles(&opctx).await.unwrap(); datastore.load_builtin_role_asgns(&opctx).await.unwrap(); + datastore.load_builtin_silos(&opctx).await.unwrap(); // Create an OpContext with the credentials of "test-privileged" for general // testing. @@ -3098,12 +3446,16 @@ mod test { let logctx = dev::test_setup_log("test_project_creation"); let mut db = test_setup_database(&logctx.log).await; let (opctx, datastore) = datastore_test(&logctx, &db).await; - let organization = Organization::new(params::OrganizationCreate { - identity: IdentityMetadataCreateParams { - name: "org".parse().unwrap(), - description: "desc".to_string(), + let organization = Organization::new( + params::OrganizationCreate { + identity: IdentityMetadataCreateParams { + name: "org".parse().unwrap(), + description: "desc".to_string(), + }, }, - }); + *SILO_ID, + ); + let organization = datastore.organization_create(&opctx, organization).await.unwrap(); @@ -3121,6 +3473,7 @@ mod test { LookupType::ById(organization.id()), ); datastore.project_create(&opctx, &org, project).await.unwrap(); + let (_, organization_after_project_create) = datastore .organization_fetch(&opctx, organization.name()) .await @@ -3146,9 +3499,28 @@ mod test { let _ = datastore.session_create(session.clone()).await; + // Associate silo with user + let silo_user = datastore + .silo_user_create(SiloUser::new( + Uuid::new_v4(), /* silo id */ + Uuid::new_v4(), /* silo user id */ + "fakeuser".to_string(), + session.user_id, /* internal user id */ + )) + .await + .unwrap(); + + assert_eq!( + silo_user.silo_id, + datastore + .get_silo_id_from_internal_user_id(session.user_id) + .await + .unwrap(), + ); + // fetch the one we just created let fetched = datastore.session_fetch(token.clone()).await.unwrap(); - assert_eq!(session.user_id, fetched.user_id); + assert_eq!(session.user_id, fetched.console_session.user_id); // trying to insert the same one again fails let duplicate = datastore.session_create(session.clone()).await; @@ -3160,11 +3532,15 @@ mod test { // update last used (i.e., renew token) let renewed = datastore.session_update_last_used(token.clone()).await.unwrap(); - assert!(renewed.time_last_used > session.time_last_used); + assert!( + renewed.console_session.time_last_used > session.time_last_used + ); // time_last_used change persists in DB let fetched = datastore.session_fetch(token.clone()).await.unwrap(); - assert!(fetched.time_last_used > session.time_last_used); + assert!( + fetched.console_session.time_last_used > session.time_last_used + ); // delete it and fetch should come back with nothing let delete = datastore.session_hard_delete(token.clone()).await; diff --git a/nexus/src/db/fixed_data/mod.rs b/nexus/src/db/fixed_data/mod.rs index 0a81e1a8000..afec9c8be84 100644 --- a/nexus/src/db/fixed_data/mod.rs +++ b/nexus/src/db/fixed_data/mod.rs @@ -27,12 +27,13 @@ // UUID PREFIX RESOURCE // 001de000-05e4 built-in users ("05e4" looks a bit like "user") // 001de000-1334 built-in fleet ("1334" looks like the "leet" in "fleet") -// +// 001de000-5110 built-in silo ("5110" looks like "silo") use lazy_static::lazy_static; pub mod role_assignment_builtin; pub mod role_builtin; +pub mod silo_builtin; pub mod user_builtin; lazy_static! { diff --git a/nexus/src/db/fixed_data/silo_builtin.rs b/nexus/src/db/fixed_data/silo_builtin.rs new file mode 100644 index 00000000000..6610f673c4e --- /dev/null +++ b/nexus/src/db/fixed_data/silo_builtin.rs @@ -0,0 +1,11 @@ +// This Source Code Form is subject to the terms of the Mozilla Public +// License, v. 2.0. If a copy of the MPL was not distributed with this +// file, You can obtain one at https://mozilla.org/MPL/2.0/. + +use lazy_static::lazy_static; + +lazy_static! { + pub static ref SILO_ID: uuid::Uuid = "001de000-5110-4000-8000-000000000000" + .parse() + .expect("invalid uuid for builtin silo id"); +} diff --git a/nexus/src/db/fixed_data/user_builtin.rs b/nexus/src/db/fixed_data/user_builtin.rs index ebe65a295ab..d751feb8946 100644 --- a/nexus/src/db/fixed_data/user_builtin.rs +++ b/nexus/src/db/fixed_data/user_builtin.rs @@ -3,12 +3,14 @@ // file, You can obtain one at https://mozilla.org/MPL/2.0/. //! Built-in users +use crate::db::fixed_data::silo_builtin::SILO_ID; use lazy_static::lazy_static; use omicron_common::api; use uuid::Uuid; pub struct UserBuiltinConfig { pub id: Uuid, + pub silo_id: Uuid, pub name: api::external::Name, pub description: &'static str, } @@ -16,11 +18,15 @@ pub struct UserBuiltinConfig { impl UserBuiltinConfig { fn new_static( id: &str, + silo_id: &str, name: &str, description: &'static str, ) -> UserBuiltinConfig { UserBuiltinConfig { id: id.parse().expect("invalid uuid for builtin user id"), + silo_id: silo_id + .parse() + .expect("invalid uuid for builtin user silo id"), name: name.parse().expect("invalid name for builtin user name"), description, } @@ -35,6 +41,7 @@ lazy_static! { // "0001" is the first possible user that wouldn't be confused with // 0, or root. "001de000-05e4-4000-8000-000000000001", + &SILO_ID.to_string().as_str(), "db-init", "used for seeding initial database data", ); @@ -43,6 +50,7 @@ lazy_static! { pub static ref USER_INTERNAL_API: UserBuiltinConfig = UserBuiltinConfig::new_static( "001de000-05e4-4000-8000-000000000002", + &SILO_ID.to_string().as_str(), "internal-api", "used by Nexus when handling internal API requests", ); @@ -52,6 +60,7 @@ lazy_static! { UserBuiltinConfig::new_static( // "3a8a" looks a bit like "saga". "001de000-05e4-4000-8000-000000003a8a", + &SILO_ID.to_string().as_str(), "saga-recovery", "used by Nexus when recovering sagas", ); @@ -64,6 +73,7 @@ lazy_static! { UserBuiltinConfig::new_static( // "4007" looks a bit like "root". "001de000-05e4-4000-8000-000000004007", + &SILO_ID.to_string().as_str(), "test-privileged", "used for testing with all privileges", ); @@ -73,6 +83,7 @@ lazy_static! { UserBuiltinConfig::new_static( // 60001 is the decimal uid for "nobody" on Helios. "001de000-05e4-4000-8000-000000060001", + &SILO_ID.to_string().as_str(), "test-unprivileged", "used for testing with no privileges", ); diff --git a/nexus/src/db/model.rs b/nexus/src/db/model.rs index 8870ff0b6f1..4b589a7a5ae 100644 --- a/nexus/src/db/model.rs +++ b/nexus/src/db/model.rs @@ -9,7 +9,7 @@ use crate::db::identity::{Asset, Resource}; use crate::db::schema::{ console_session, dataset, disk, instance, metric_producer, network_interface, organization, oximeter, project, rack, region, - role_assignment_builtin, role_builtin, router_route, sled, + role_assignment_builtin, role_builtin, router_route, silo, silo_user, sled, update_available_artifact, user_builtin, volume, vpc, vpc_firewall_rule, vpc_router, vpc_subnet, zpool, }; @@ -778,6 +778,80 @@ impl Volume { } } +/// Describes a silo within the database. +#[derive(Queryable, Insertable, Debug, Resource, Selectable)] +#[table_name = "silo"] +pub struct Silo { + #[diesel(embed)] + identity: SiloIdentity, + + pub discoverable: bool, + + /// child resource generation number, per RFD 192 + pub rcgen: Generation, +} + +impl Silo { + /// Creates a new database Silo object. + pub fn new(params: params::SiloCreate) -> Self { + Self::new_with_id(Uuid::new_v4(), params) + } + + pub fn new_with_id(id: Uuid, params: params::SiloCreate) -> Self { + Self { + identity: SiloIdentity::new(id, params.identity), + discoverable: params.discoverable, + rcgen: Generation::new(), + } + } +} + +impl DatastoreCollection for Silo { + type CollectionId = Uuid; + type GenerationNumberColumn = silo::dsl::rcgen; + type CollectionTimeDeletedColumn = silo::dsl::time_deleted; + type CollectionIdColumn = organization::dsl::silo_id; +} + +/// Describes a silo user within the database. +#[derive(Queryable, Insertable, Debug, Selectable)] +#[table_name = "silo_user"] +pub struct SiloUser { + pub id: Uuid, + pub silo_id: Uuid, + + pub name: String, + + pub time_created: DateTime, + pub time_modified: DateTime, + pub time_deleted: Option>, + + pub internal_user_id: Uuid, +} + +impl SiloUser { + pub fn new( + silo_id: Uuid, + user_id: Uuid, + name: String, + internal_user_id: Uuid, + ) -> Self { + let now = Utc::now(); + Self { + id: user_id, + silo_id, + + name, + + time_created: now, + time_modified: now, + time_deleted: None, + + internal_user_id, + } + } +} + /// Describes an organization within the database. #[derive(Queryable, Insertable, Debug, Resource, Selectable)] #[table_name = "organization"] @@ -785,19 +859,26 @@ pub struct Organization { #[diesel(embed)] identity: OrganizationIdentity, + silo_id: Uuid, + /// child resource generation number, per RFD 192 pub rcgen: Generation, } impl Organization { /// Creates a new database Organization object. - pub fn new(params: params::OrganizationCreate) -> Self { + pub fn new(params: params::OrganizationCreate, silo_id: Uuid) -> Self { let id = Uuid::new_v4(); Self { identity: OrganizationIdentity::new(id, params.identity), + silo_id, rcgen: Generation::new(), } } + + pub fn silo_id(&self) -> Uuid { + self.silo_id + } } impl DatastoreCollection for Organization { @@ -2058,6 +2139,13 @@ impl ConsoleSession { } } +// Return this instead, by performing an additional SELECT to get silo id +#[derive(Clone, Debug)] +pub struct ConsoleSessionWithSiloId { + pub console_session: ConsoleSession, + pub silo_id: Uuid, +} + /// Describes a built-in user, as stored in the database #[derive(Queryable, Insertable, Debug, Resource, Selectable)] #[table_name = "user_builtin"] diff --git a/nexus/src/db/schema.rs b/nexus/src/db/schema.rs index ba665e533fb..7dd31008cc9 100644 --- a/nexus/src/db/schema.rs +++ b/nexus/src/db/schema.rs @@ -78,9 +78,38 @@ table! { } } +table! { + silo (id) { + id -> Uuid, + name -> Text, + description -> Text, + discoverable -> Bool, + time_created -> Timestamptz, + time_modified -> Timestamptz, + time_deleted -> Nullable, + rcgen -> Int8, + } +} + +table! { + silo_user (id) { + id -> Uuid, + silo_id -> Uuid, + + name -> Text, + + time_created -> Timestamptz, + time_modified -> Timestamptz, + time_deleted -> Nullable, + + internal_user_id -> Uuid, + } +} + table! { organization (id) { id -> Uuid, + silo_id -> Uuid, name -> Text, description -> Text, time_created -> Timestamptz, diff --git a/nexus/src/external_api/console_api.rs b/nexus/src/external_api/console_api.rs index 2c9b09fd8e5..2eb3e94524f 100644 --- a/nexus/src/external_api/console_api.rs +++ b/nexus/src/external_api/console_api.rs @@ -67,10 +67,18 @@ pub async fn spoof_login( .body("".into())?); // TODO: failed login response body? } - let session = nexus - // TODO: obviously - .session_create(user_id.unwrap()) - .await?; + let user_id = user_id.unwrap(); + + // look up silo id, make sure user is legal + if let Err(e) = nexus.get_silo_id_from_internal_user_id(user_id).await { + return Err(HttpError::for_internal_error(format!( + "no silo id found! {}", + e + ))); + } + + let session = nexus.session_create(user_id).await?; + Ok(Response::builder() .status(StatusCode::OK) .header( diff --git a/nexus/src/external_api/http_entrypoints.rs b/nexus/src/external_api/http_entrypoints.rs index b4b481f8601..99b6456daf7 100644 --- a/nexus/src/external_api/http_entrypoints.rs +++ b/nexus/src/external_api/http_entrypoints.rs @@ -12,7 +12,9 @@ use crate::ServerContext; use super::{ console_api, params, - views::{Organization, Project, Rack, Role, Sled, User, Vpc, VpcSubnet}, + views::{ + Organization, Project, Rack, Role, Silo, Sled, User, Vpc, VpcSubnet, + }, }; use crate::context::OpContext; use dropshot::ApiDescription; @@ -70,6 +72,11 @@ type NexusApiDescription = ApiDescription>; */ pub fn external_api() -> NexusApiDescription { fn register_endpoints(api: &mut NexusApiDescription) -> Result<(), String> { + api.register(silos_get)?; + api.register(silos_post)?; + api.register(silos_get_silo)?; + api.register(silos_delete_silo)?; + api.register(organizations_get)?; api.register(organizations_post)?; api.register(organizations_get_organization)?; @@ -203,6 +210,129 @@ pub fn external_api() -> NexusApiDescription { * a function name is a breaking change from a client perspective. */ +// TODO authz for silo endpoints + +/** + * List all silos (that are discoverable). + */ +#[endpoint { + method = GET, + path = "/silos", + tags = ["silos"], +}] +async fn silos_get( + rqctx: Arc>>, + query_params: Query, +) -> Result>, HttpError> { + let apictx = rqctx.context(); + let nexus = &apictx.nexus; + let handler = async { + let opctx = OpContext::for_external_api(&rqctx).await?; + let query = query_params.into_inner(); + let params = ScanByNameOrId::from_query(&query)?; + let field = pagination_field_for_scan_params(params); + + let silos = match field { + PagField::Id => { + let page_selector = data_page_params_nameid_id(&rqctx, &query)?; + nexus.silos_list_by_id(&opctx, &page_selector).await? + } + + PagField::Name => { + let page_selector = + data_page_params_nameid_name(&rqctx, &query)? + .map_name(|n| Name::ref_cast(n)); + nexus.silos_list_by_name(&opctx, &page_selector).await? + } + } + .into_iter() + .map(|p| p.into()) + .collect(); + Ok(HttpResponseOk(ScanByNameOrId::results_page(&query, silos)?)) + }; + apictx.external_latencies.instrument_dropshot_handler(&rqctx, handler).await +} + +/** + * Create a new silo. + */ +#[endpoint { + method = POST, + path = "/silos", + tags = ["silos"], +}] +async fn silos_post( + rqctx: Arc>>, + new_silo_params: TypedBody, +) -> Result, HttpError> { + let apictx = rqctx.context(); + let nexus = &apictx.nexus; + let handler = async { + let opctx = OpContext::for_external_api(&rqctx).await?; + let silo = + nexus.silo_create(&opctx, new_silo_params.into_inner()).await?; + Ok(HttpResponseCreated(silo.into())) + }; + apictx.external_latencies.instrument_dropshot_handler(&rqctx, handler).await +} + +/** + * Path parameters for Silo requests + */ +#[derive(Deserialize, JsonSchema)] +struct SiloPathParam { + /// The silo's unique name. + silo_name: Name, +} + +/** + * Fetch a specific silo + */ +#[endpoint { + method = GET, + path = "/silos/{silo_name}", + tags = ["silos"], +}] +async fn silos_get_silo( + rqctx: Arc>>, + path_params: Path, +) -> Result, HttpError> { + let apictx = rqctx.context(); + let nexus = &apictx.nexus; + let path = path_params.into_inner(); + let silo_name = &path.silo_name; + let handler = async { + let opctx = OpContext::for_external_api(&rqctx).await?; + let silo = nexus.silo_fetch(&opctx, &silo_name).await?; + Ok(HttpResponseOk(silo.into())) + }; + apictx.external_latencies.instrument_dropshot_handler(&rqctx, handler).await +} + +/** + * Delete a specific silo. + */ +#[endpoint { + method = DELETE, + path = "/silos/{silo_name}", + tags = ["silos"], +}] +async fn silos_delete_silo( + rqctx: Arc>>, + path_params: Path, +) -> Result { + let apictx = rqctx.context(); + let nexus = &apictx.nexus; + let params = path_params.into_inner(); + let silo_name = ¶ms.silo_name; + let handler = async { + let opctx = OpContext::for_external_api(&rqctx).await?; + nexus.silo_delete(&opctx, &silo_name).await?; + Ok(HttpResponseDeleted()) + }; + apictx.external_latencies.instrument_dropshot_handler(&rqctx, handler).await +} + /** * List all organizations. */ @@ -210,7 +340,7 @@ pub fn external_api() -> NexusApiDescription { method = GET, path = "/organizations", tags = ["organizations"], - }] +}] async fn organizations_get( rqctx: Arc>>, query_params: Query, diff --git a/nexus/src/external_api/params.rs b/nexus/src/external_api/params.rs index 40f37ec63e4..15e916c95fe 100644 --- a/nexus/src/external_api/params.rs +++ b/nexus/src/external_api/params.rs @@ -14,6 +14,21 @@ use schemars::JsonSchema; use serde::{Deserialize, Serialize}; use uuid::Uuid; +/* + * Silos + */ + +/** + * Create-time parameters for a [`Silo`](crate::external_api::views::Silo) + */ +#[derive(Clone, Debug, Deserialize, Serialize, JsonSchema)] +pub struct SiloCreate { + #[serde(flatten)] + pub identity: IdentityMetadataCreateParams, + + pub discoverable: bool, +} + /* * ORGANIZATIONS */ diff --git a/nexus/src/external_api/views.rs b/nexus/src/external_api/views.rs index 3958a825083..5deb3e289f6 100644 --- a/nexus/src/external_api/views.rs +++ b/nexus/src/external_api/views.rs @@ -18,6 +18,27 @@ use serde::{Deserialize, Serialize}; use std::net::SocketAddr; use uuid::Uuid; +/* + * SILOS + */ + +/** + * Client view of a ['Silo'] + */ +#[derive(ObjectIdentity, Clone, Debug, Deserialize, Serialize, JsonSchema)] +pub struct Silo { + #[serde(flatten)] + pub identity: IdentityMetadata, + + pub discoverable: bool, +} + +impl Into for model::Silo { + fn into(self) -> Silo { + Silo { identity: self.identity(), discoverable: self.discoverable } + } +} + /* * ORGANIZATIONS */ @@ -29,6 +50,7 @@ use uuid::Uuid; pub struct Organization { #[serde(flatten)] pub identity: IdentityMetadata, + // Important: Silo ID does not get presented to user } impl Into for model::Organization { @@ -205,7 +227,7 @@ pub struct SessionUser { impl Into for authn::Actor { fn into(self) -> SessionUser { - SessionUser { id: self.0 } + SessionUser { id: self.id } } } diff --git a/nexus/src/nexus.rs b/nexus/src/nexus.rs index 4adb092b7d7..c5cb33b1120 100644 --- a/nexus/src/nexus.rs +++ b/nexus/src/nexus.rs @@ -14,6 +14,7 @@ use crate::db; use crate::db::identity::{Asset, Resource}; use crate::db::model::DatasetKind; use crate::db::model::Name; +use crate::db::model::SiloUser; use crate::db::subnet_allocation::SubnetError; use crate::defaults; use crate::external_api::params; @@ -494,6 +495,51 @@ impl Nexus { }) } + /* + * Silos + */ + + pub async fn silo_create( + &self, + opctx: &OpContext, + new_silo_params: params::SiloCreate, + ) -> CreateResult { + let silo = db::model::Silo::new(new_silo_params); + self.db_datastore.silo_create(opctx, silo).await + } + + pub async fn silo_fetch( + &self, + opctx: &OpContext, + name: &Name, + ) -> LookupResult { + self.db_datastore.silo_fetch(opctx, name).await + } + + pub async fn silos_list_by_name( + &self, + opctx: &OpContext, + pagparams: &DataPageParams<'_, Name>, + ) -> ListResultVec { + self.db_datastore.silos_list_by_name(opctx, pagparams).await + } + + pub async fn silos_list_by_id( + &self, + opctx: &OpContext, + pagparams: &DataPageParams<'_, Uuid>, + ) -> ListResultVec { + self.db_datastore.silos_list_by_id(opctx, pagparams).await + } + + pub async fn silo_delete( + &self, + opctx: &OpContext, + name: &Name, + ) -> DeleteResult { + self.db_datastore.silo_delete(opctx, name).await + } + /* * Organizations */ @@ -503,7 +549,9 @@ impl Nexus { opctx: &OpContext, new_organization: ¶ms::OrganizationCreate, ) -> CreateResult { - let db_org = db::model::Organization::new(new_organization.clone()); + let silo_id = opctx.authn.silo_required()?; + let db_org = + db::model::Organization::new(new_organization.clone(), silo_id); self.db_datastore.organization_create(opctx, db_org).await } @@ -2625,7 +2673,7 @@ impl Nexus { pub async fn session_fetch( &self, token: String, - ) -> LookupResult { + ) -> LookupResult { self.db_datastore.session_fetch(token).await } @@ -2642,7 +2690,7 @@ impl Nexus { pub async fn session_update_last_used( &self, token: String, - ) -> UpdateResult { + ) -> UpdateResult { Ok(self.db_datastore.session_update_last_used(token).await?) } @@ -2878,6 +2926,32 @@ impl Nexus { })?; Ok(body) } + + pub async fn get_silo_id_from_silo_user_id( + &self, + silo_user_id: Uuid, + ) -> LookupResult { + self.db_datastore.get_silo_id_from_silo_user_id(silo_user_id).await + } + + pub async fn get_silo_id_from_internal_user_id( + &self, + user_id: Uuid, + ) -> LookupResult { + self.db_datastore.get_silo_id_from_internal_user_id(user_id).await + } + + pub async fn silo_user_create( + &self, + silo_id: Uuid, + silo_user_id: Uuid, + name: String, + internal_user_id: Uuid, + ) -> CreateResult { + let silo_user = + SiloUser::new(silo_id, silo_user_id, name, internal_user_id); + Ok(self.db_datastore.silo_user_create(silo_user).await?) + } } fn generate_session_token() -> String { diff --git a/nexus/src/populate.rs b/nexus/src/populate.rs index 50c923f6adc..70558fe936d 100644 --- a/nexus/src/populate.rs +++ b/nexus/src/populate.rs @@ -53,10 +53,14 @@ async fn populate( async { datastore.load_builtin_role_asgns(opctx).await.map(|_| ()) } .boxed() }; + let populate_silos = || { + async { datastore.load_builtin_silos(opctx).await.map(|_| ()) }.boxed() + }; let populators = [ Populator { name: "users", func: &populate_users }, Populator { name: "roles", func: &populate_roles }, Populator { name: "role assignments", func: &populate_role_asgns }, + Populator { name: "silos", func: &populate_silos }, ]; for p in populators { diff --git a/nexus/test-utils/src/http_testing.rs b/nexus/test-utils/src/http_testing.rs index 91bcb544556..e9abd842b2d 100644 --- a/nexus/test-utils/src/http_testing.rs +++ b/nexus/test-utils/src/http_testing.rs @@ -11,6 +11,7 @@ use dropshot::test_util::ClientTestContext; use dropshot::ResultsPage; use headers::authorization::Credentials; use omicron_nexus::authn::external::spoof; +use omicron_nexus::db::model::ConsoleSession; use std::convert::TryInto; use std::fmt::Debug; @@ -380,6 +381,7 @@ where } /// Represents a response from an HTTP server +#[derive(Debug)] pub struct TestResponse { pub status: http::StatusCode, pub headers: http::HeaderMap, @@ -443,6 +445,14 @@ impl<'a> NexusRequest<'a> { self } + pub fn authn_with_session(mut self, session: &ConsoleSession) -> Self { + self.request_builder = self.request_builder.header( + &http::header::COOKIE, + format!("session={}", session.token), + ); + self + } + /// See [`RequestBuilder::execute()`]. pub async fn execute(self) -> Result { self.request_builder.execute().await @@ -507,6 +517,20 @@ impl<'a> NexusRequest<'a> { ) } + pub fn expect_failure_with_body( + testctx: &'a ClientTestContext, + expected_status: http::StatusCode, + method: http::Method, + uri: &str, + body: &B, + ) -> Self { + NexusRequest::new( + RequestBuilder::new(testctx, method, uri) + .body(Some(body)) + .expect_status(Some(expected_status)), + ) + } + /// Iterates a collection (like `dropshot::test_util::iter_collection`) /// using authenticated requests. pub async fn iter_collection_authn( diff --git a/nexus/test-utils/src/resource_helpers.rs b/nexus/test-utils/src/resource_helpers.rs index ea004cd5a88..f01f2e5c77d 100644 --- a/nexus/test-utils/src/resource_helpers.rs +++ b/nexus/test-utils/src/resource_helpers.rs @@ -18,8 +18,9 @@ use omicron_common::api::external::Instance; use omicron_common::api::external::InstanceCpuCount; use omicron_common::api::external::VpcRouter; use omicron_nexus::crucible_agent_client::types::State as RegionState; +use omicron_nexus::db::model::ConsoleSession; use omicron_nexus::external_api::params; -use omicron_nexus::external_api::views::{Organization, Project, Vpc}; +use omicron_nexus::external_api::views::{Organization, Project, Silo, Vpc}; use omicron_sled_agent::sim::SledAgent; use std::sync::Arc; use uuid::Uuid; @@ -40,6 +41,23 @@ where .unwrap() } +pub async fn objects_list_page_authz_with_session( + client: &ClientTestContext, + path: &str, + session: &ConsoleSession, +) -> dropshot::ResultsPage +where + ItemType: serde::de::DeserializeOwned, +{ + NexusRequest::object_get(client, path) + .authn_with_session(session) + .execute() + .await + .expect("failed to make request") + .parsed_body() + .unwrap() +} + pub async fn object_create( client: &ClientTestContext, path: &str, @@ -58,6 +76,25 @@ where .unwrap() } +pub async fn create_silo( + client: &ClientTestContext, + silo_name: &str, + discoverable: bool, +) -> Silo { + object_create( + client, + "/silos", + ¶ms::SiloCreate { + identity: IdentityMetadataCreateParams { + name: silo_name.parse().unwrap(), + description: "a silo".to_string(), + }, + discoverable, + }, + ) + .await +} + pub async fn create_organization( client: &ClientTestContext, organization_name: &str, diff --git a/nexus/tests/integration_tests/authn_http.rs b/nexus/tests/integration_tests/authn_http.rs index b48847ee237..e021af7503a 100644 --- a/nexus/tests/integration_tests/authn_http.rs +++ b/nexus/tests/integration_tests/authn_http.rs @@ -105,16 +105,19 @@ async fn test_authn_session_cookie() { > = vec![Box::new(session_cookie::HttpAuthnSessionCookie)]; let valid_session = FakeSession { user_id: Uuid::new_v4(), + silo_id: Uuid::new_v4(), time_last_used: Utc::now() - Duration::seconds(5), time_created: Utc::now() - Duration::seconds(5), }; let idle_expired_session = FakeSession { user_id: Uuid::new_v4(), + silo_id: Uuid::new_v4(), time_last_used: Utc::now() - Duration::hours(2), time_created: Utc::now() - Duration::hours(3), }; let abs_expired_session = FakeSession { user_id: Uuid::new_v4(), + silo_id: Uuid::new_v4(), time_last_used: Utc::now(), time_created: Utc::now() - Duration::hours(10), }; @@ -308,6 +311,7 @@ struct WhoamiServerState { #[derive(Clone, Copy)] struct FakeSession { user_id: Uuid, + silo_id: Uuid, time_created: DateTime, time_last_used: DateTime, } @@ -316,6 +320,9 @@ impl session_cookie::Session for FakeSession { fn user_id(&self) -> Uuid { self.user_id } + fn silo_id(&self) -> Uuid { + self.silo_id + } fn time_created(&self) -> DateTime { self.time_created } @@ -380,7 +387,7 @@ async fn whoami_get( ) -> Result, dropshot::HttpError> { let whoami_state = rqctx.context(); let authn = whoami_state.authn.authn_request(&rqctx).await?; - let actor = authn.actor().map(|a| a.0.to_string()); + let actor = authn.actor().map(|a| a.id.to_string()); let authenticated = actor.is_some(); let schemes_tried = authn.schemes_tried().iter().map(|s| s.to_string()).collect(); diff --git a/nexus/tests/integration_tests/basic.rs b/nexus/tests/integration_tests/basic.rs index 6047a667889..91ee6edd9f5 100644 --- a/nexus/tests/integration_tests/basic.rs +++ b/nexus/tests/integration_tests/basic.rs @@ -55,118 +55,137 @@ async fn test_basic_failures(cptestctx: &ControlPlaneTestContext) { /* * Error case: GET /organizations/test-org/projects/nonexistent (a possible - * value that does not exist inside a collection that does exist) - */ + * value that does not exist inside a collection that does exist) from an + * unauthorized user results in a 401. + + // XXX uncomment when this endpoint is protected by authn! + let error = client .make_request( Method::GET, "/organizations/test-org/projects/nonexistent", None as Option<()>, - StatusCode::NOT_FOUND, + StatusCode::UNAUTHORIZED, ) .await .expect_err("expected error"); - assert_eq!("not found: project with name \"nonexistent\"", error.message); - - /* - * Error case: GET /organizations/test-org/projects/-invalid-name - * TODO-correctness is 400 the right error code here or is 404 more - * appropriate? + assert_eq!("credentials missing or invalid", error.message); */ - let error = client - .make_request( - Method::GET, - "/organizations/test-org/projects/-invalid-name", - None as Option<()>, - StatusCode::BAD_REQUEST, - ) - .await - .expect_err("expected error"); - assert_eq!( - "bad parameter in URL path: name must begin with an ASCII lowercase \ - character", - error.message - ); - - /* Error case: PUT /organizations/test-org/projects */ - let error = client - .make_request( - Method::PUT, - "/organizations/test-org/projects", - None as Option<()>, - StatusCode::METHOD_NOT_ALLOWED, - ) - .await - .expect_err("expected error"); - assert_eq!("Method Not Allowed", error.message); - /* Error case: DELETE /organizations/test-org/projects */ - let error = client - .make_request( - Method::DELETE, - "/organizations/test-org/projects", - None as Option<()>, - StatusCode::METHOD_NOT_ALLOWED, - ) - .await - .expect_err("expected error"); - assert_eq!("Method Not Allowed", error.message); - - /* Error case: list instances in a nonexistent project. */ - let error = client - .make_request_with_body( - Method::GET, - "/organizations/test-org/projects/nonexistent/instances", - "".into(), - StatusCode::NOT_FOUND, - ) - .await - .expect_err("expected error"); - assert_eq!("not found: project with name \"nonexistent\"", error.message); + struct TestCase<'a> { + method: http::Method, + uri: &'a str, + expected_code: http::StatusCode, + expected_error: &'a str, + body: Option, + } - /* Error case: fetch an instance in a nonexistent project. */ - let error = client - .make_request_with_body( - Method::GET, - "/organizations/test-org/projects/nonexistent/instances/my-instance", - "".into(), - StatusCode::NOT_FOUND, - ) - .await - .expect_err("expected error"); - assert_eq!("not found: project with name \"nonexistent\"", error.message); + let test_cases = vec![ + /* + * Error case: GET /organizations/test-org/projects/nonexistent (a + * possible value that does not exist inside a collection that does + * exist) from an authorized user results in a 404. + */ + TestCase { + method: Method::GET, + uri: "/organizations/test-org/projects/nonexistent", + expected_code: StatusCode::NOT_FOUND, + expected_error: "not found: project with name \"nonexistent\"", + body: None, + }, + /* + * Error case: GET /organizations/test-org/projects/-invalid-name + * TODO-correctness is 400 the right error code here or is 404 more + * appropriate? + */ + TestCase { + method: Method::GET, + uri: "/organizations/test-org/projects/-invalid-name", + expected_code: StatusCode::BAD_REQUEST, + expected_error: "bad parameter in URL path: name must begin with \ + an ASCII lowercase character", + body: None, + }, + /* Error case: PUT /organizations/test-org/projects */ + TestCase { + method: Method::PUT, + uri: "/organizations/test-org/projects", + expected_code: StatusCode::METHOD_NOT_ALLOWED, + expected_error: "Method Not Allowed", + body: None, + }, + /* Error case: DELETE /organizations/test-org/projects */ + TestCase { + method: Method::DELETE, + uri: "/organizations/test-org/projects", + expected_code: StatusCode::METHOD_NOT_ALLOWED, + expected_error: "Method Not Allowed", + body: None, + }, + /* Error case: list instances in a nonexistent project. */ + TestCase { + method: Method::GET, + uri: "/organizations/test-org/projects/nonexistent/instances", + expected_code: StatusCode::NOT_FOUND, + expected_error: "not found: project with name \"nonexistent\"", + body: Some("".into()), + }, + /* Error case: fetch an instance in a nonexistent project. */ + TestCase { + method: Method::GET, + uri: "/organizations/test-org/projects/nonexistent/instances/my-instance", + expected_code: StatusCode::NOT_FOUND, + expected_error: "not found: project with name \"nonexistent\"", + body: Some("".into()), + }, + /* Error case: fetch an instance with an invalid name. */ + TestCase { + method: Method::GET, + uri: "/organizations/test-org/projects/nonexistent/instances/my_instance", + expected_code: StatusCode::BAD_REQUEST, + expected_error: "bad parameter in URL path: name contains \ + invalid character: \"_\" (allowed characters are lowercase \ + ASCII, digits, and \"-\")", + body: Some("".into()), + }, + /* Error case: delete an instance with an invalid name. */ + TestCase { + method: Method::DELETE, + uri: "/organizations/test-org/projects/nonexistent/instances/my_instance", + expected_code: StatusCode::BAD_REQUEST, + expected_error: "bad parameter in URL path: name contains \ + invalid character: \"_\" (allowed characters are lowercase \ + ASCII, digits, and \"-\")", + body: Some("".into()), + }, + ]; - /* Error case: fetch an instance with an invalid name. */ - let error = client - .make_request_with_body( - Method::GET, - "/organizations/test-org/projects/nonexistent/instances/my_instance", - "".into(), - StatusCode::BAD_REQUEST, - ) + for test_case in test_cases { + let error = if let Some(body) = test_case.body { + NexusRequest::expect_failure_with_body( + client, + test_case.expected_code, + test_case.method, + test_case.uri, + &body, + ) + } else { + NexusRequest::expect_failure( + client, + test_case.expected_code, + test_case.method, + test_case.uri, + ) + } + .authn_as(AuthnMode::PrivilegedUser) + .execute() .await - .expect_err("expected error"); - assert_eq!( - "bad parameter in URL path: name contains invalid character: \"_\" \ - (allowed characters are lowercase ASCII, digits, and \"-\")", - error.message - ); + .unwrap() + .parsed_body::() + .unwrap(); - /* Error case: delete an instance with an invalid name. */ - let error = client - .make_request_with_body( - Method::DELETE, - "/organizations/test-org/projects/nonexistent/instances/my_instance", - "".into(), - StatusCode::BAD_REQUEST, - ) - .await - .expect_err("expected error"); - assert_eq!( - "bad parameter in URL path: name contains invalid character: \"_\" \ - (allowed characters are lowercase ASCII, digits, and \"-\")", - error.message - ); + assert_eq!(test_case.expected_error, error.message); + } } #[nexus_test] diff --git a/nexus/tests/integration_tests/disks.rs b/nexus/tests/integration_tests/disks.rs index 50ef122c5ef..26272e96fbe 100644 --- a/nexus/tests/integration_tests/disks.rs +++ b/nexus/tests/integration_tests/disks.rs @@ -81,9 +81,16 @@ async fn test_disk_not_found_before_creation( // Make sure we get a 404 if we fetch one. let disk_url = format!("{}/{}", disks_url, DISK_NAME); - let error = client - .make_request_error(Method::GET, &disk_url, StatusCode::NOT_FOUND) - .await; + let error = NexusRequest::new( + RequestBuilder::new(client, Method::GET, &disk_url) + .expect_status(Some(StatusCode::NOT_FOUND)), + ) + .authn_as(AuthnMode::PrivilegedUser) + .execute() + .await + .expect("unexpected success") + .parsed_body::() + .unwrap(); assert_eq!( error.message, format!("not found: disk with name \"{}\"", DISK_NAME) @@ -222,9 +229,19 @@ async fn test_disk_create_attach_detach_delete( assert_eq!(disks_list(&client, &disks_url).await.len(), 0); // We shouldn't find it if we request it explicitly. - let error = client - .make_request_error(Method::GET, &disk_url, StatusCode::NOT_FOUND) - .await; + let error = NexusRequest::expect_failure( + client, + StatusCode::NOT_FOUND, + Method::GET, + &disk_url, + ) + .authn_as(AuthnMode::PrivilegedUser) + .execute() + .await + .unwrap() + .parsed_body::() + .unwrap(); + assert_eq!( error.message, format!("not found: disk with name \"{}\"", DISK_NAME) diff --git a/nexus/tests/integration_tests/mod.rs b/nexus/tests/integration_tests/mod.rs index 939ba538de8..1c3ea8f562d 100644 --- a/nexus/tests/integration_tests/mod.rs +++ b/nexus/tests/integration_tests/mod.rs @@ -15,6 +15,7 @@ mod oximeter; mod projects; mod roles_builtin; mod router_routes; +mod silos; mod subnet_allocation; mod timeseries; mod unauthorized; diff --git a/nexus/tests/integration_tests/organizations.rs b/nexus/tests/integration_tests/organizations.rs index a28407e8002..f4af75cce8b 100644 --- a/nexus/tests/integration_tests/organizations.rs +++ b/nexus/tests/integration_tests/organizations.rs @@ -45,13 +45,16 @@ async fn test_organizations(cptestctx: &ControlPlaneTestContext) { assert_eq!(organization.identity.name, o2_name); // Verify requesting a non-existent organization fails - client - .make_request_error( - Method::GET, - "/organizations/fake-org", - StatusCode::NOT_FOUND, - ) - .await; + NexusRequest::expect_failure( + &client, + StatusCode::NOT_FOUND, + Method::GET, + &"/organizations/fake-org", + ) + .authn_as(AuthnMode::PrivilegedUser) + .execute() + .await + .expect("failed to make request"); // Verify GET /organizations works let organizations = diff --git a/nexus/tests/integration_tests/silos.rs b/nexus/tests/integration_tests/silos.rs new file mode 100644 index 00000000000..e7d8f622117 --- /dev/null +++ b/nexus/tests/integration_tests/silos.rs @@ -0,0 +1,149 @@ +// This Source Code Form is subject to the terms of the Mozilla Public +// License, v. 2.0. If a copy of the MPL was not distributed with this +// file, You can obtain one at https://mozilla.org/MPL/2.0/. + +use uuid::Uuid; + +use nexus_test_utils::http_testing::{AuthnMode, NexusRequest}; +use omicron_nexus::external_api::views::{Organization, Silo}; + +use http::method::Method; +use http::StatusCode; +use nexus_test_utils::resource_helpers::{ + create_organization, create_silo, objects_list_page_authz, + objects_list_page_authz_with_session, +}; + +use nexus_test_utils::ControlPlaneTestContext; +use nexus_test_utils_macros::nexus_test; + +#[nexus_test] +async fn test_silos(cptestctx: &ControlPlaneTestContext) { + let client = &cptestctx.external_client; + let nexus = &cptestctx.server.apictx.nexus; + + // Create two silos: one discoverable, one not + create_silo(&client, &"discoverable", true).await; + create_silo(&client, &"hidden", false).await; + + // Verify GET /silos/{silo} works for both discoverable and not + let discoverable_url = "/silos/discoverable"; + let hidden_url = "/silos/hidden"; + + let silo: Silo = NexusRequest::object_get(&client, &discoverable_url) + .authn_as(AuthnMode::PrivilegedUser) + .execute() + .await + .expect("failed to make request") + .parsed_body() + .unwrap(); + assert_eq!(silo.identity.name, "discoverable"); + + let silo: Silo = NexusRequest::object_get(&client, &hidden_url) + .authn_as(AuthnMode::PrivilegedUser) + .execute() + .await + .expect("failed to make request") + .parsed_body() + .unwrap(); + assert_eq!(silo.identity.name, "hidden"); + + // Verify 404 if silo doesn't exist + NexusRequest::expect_failure( + &client, + StatusCode::NOT_FOUND, + Method::GET, + &"/silos/testpost", + ) + .authn_as(AuthnMode::PrivilegedUser) + .execute() + .await + .expect("failed to make request"); + + // Verify GET /silos only returns discoverable silos + let silos = objects_list_page_authz::(client, "/silos").await.items; + assert_eq!(silos.len(), 1); + assert_eq!(silos[0].identity.name, "discoverable"); + + // Create a new user in the discoverable silo, then create a console session + let new_silo_user = nexus + .silo_user_create( + silos[0].identity.id, /* silo id */ + Uuid::new_v4(), /* silo user id */ + "new silo user".to_string(), /* name */ + Uuid::new_v4(), /* internal user id */ + ) + .await + .unwrap(); + + let session = + nexus.session_create(new_silo_user.internal_user_id).await.unwrap(); + + // Create organization with built-in user auth + // Note: this currently goes to the built-in silo! + create_organization(&client, "someorg").await; + + // Verify GET /organizations works with built-in user auth + let organizations = + objects_list_page_authz::(client, "/organizations") + .await + .items; + assert_eq!(organizations.len(), 1); + assert_eq!(organizations[0].identity.name, "someorg"); + + // TODO: uncomment when silo users can have role assignments + /* + // Verify GET /organizations doesn't list anything if authing under + // different silo. + let organizations = + objects_list_page_authz_with_session::( + client, "/organizations", &session, + ) + .await + .items; + assert_eq!(organizations.len(), 0); + */ + + // Verify DELETE doesn't work if organizations exist + // TODO: put someorg in discoverable silo, not built-in + NexusRequest::expect_failure( + &client, + StatusCode::BAD_REQUEST, + Method::DELETE, + &"/silos/fakesilo", + ) + .authn_as(AuthnMode::PrivilegedUser) + .execute() + .await + .expect("failed to make request"); + + // Delete organization + NexusRequest::object_delete(&client, &"/organizations/someorg") + .authn_as(AuthnMode::PrivilegedUser) + .execute() + .await + .expect("failed to make request"); + + // Verify silo DELETE works + NexusRequest::object_delete(&client, &"/silos/discoverable") + .authn_as(AuthnMode::PrivilegedUser) + .execute() + .await + .expect("failed to make request"); + + // Verify silo user was also deleted + nexus + .get_silo_id_from_internal_user_id(new_silo_user.internal_user_id) + .await + .expect_err("unexpected success"); + nexus + .get_silo_id_from_silo_user_id(new_silo_user.id) + .await + .expect_err("unexpected success"); + + // Verify new user's console session isn't valid anymore. + nexus + .session_fetch(session.token.clone()) + .await + .expect_err("unexpected success"); +} diff --git a/nexus/tests/output/nexus_tags.txt b/nexus/tests/output/nexus_tags.txt index e7f4fa0fe11..c009066df9b 100644 --- a/nexus/tests/output/nexus_tags.txt +++ b/nexus/tests/output/nexus_tags.txt @@ -81,6 +81,13 @@ OPERATION ID URL PATH sagas_get /sagas sagas_get_saga /sagas/{saga_id} +API operations found with tag "silos" +OPERATION ID URL PATH +silos_delete_silo /silos/{silo_name} +silos_get /silos +silos_get_silo /silos/{silo_name} +silos_post /silos + API operations found with tag "sleds" OPERATION ID URL PATH hardware_sleds_get /hardware/sleds diff --git a/openapi/nexus.json b/openapi/nexus.json index a88e86dd099..b7513b08863 100644 --- a/openapi/nexus.json +++ b/openapi/nexus.json @@ -3440,6 +3440,170 @@ } } }, + "/silos": { + "get": { + "tags": [ + "silos" + ], + "summary": "List all silos (that are discoverable).", + "operationId": "silos_get", + "parameters": [ + { + "in": "query", + "name": "limit", + "description": "Maximum number of items returned by a single call", + "schema": { + "nullable": true, + "type": "integer", + "format": "uint32", + "minimum": 1 + }, + "style": "form" + }, + { + "in": "query", + "name": "page_token", + "description": "Token returned by previous call to retreive the subsequent page", + "schema": { + "nullable": true, + "type": "string" + }, + "style": "form" + }, + { + "in": "query", + "name": "sort_by", + "schema": { + "$ref": "#/components/schemas/NameOrIdSortMode" + }, + "style": "form" + } + ], + "responses": { + "200": { + "description": "successful operation", + "content": { + "application/json": { + "schema": { + "$ref": "#/components/schemas/SiloResultsPage" + } + } + } + }, + "4XX": { + "$ref": "#/components/responses/Error" + }, + "5XX": { + "$ref": "#/components/responses/Error" + } + }, + "x-dropshot-pagination": true + }, + "post": { + "tags": [ + "silos" + ], + "summary": "Create a new silo.", + "operationId": "silos_post", + "requestBody": { + "content": { + "application/json": { + "schema": { + "$ref": "#/components/schemas/SiloCreate" + } + } + }, + "required": true + }, + "responses": { + "201": { + "description": "successful creation", + "content": { + "application/json": { + "schema": { + "$ref": "#/components/schemas/Silo" + } + } + } + }, + "4XX": { + "$ref": "#/components/responses/Error" + }, + "5XX": { + "$ref": "#/components/responses/Error" + } + } + } + }, + "/silos/{silo_name}": { + "get": { + "tags": [ + "silos" + ], + "summary": "Fetch a specific silo", + "operationId": "silos_get_silo", + "parameters": [ + { + "in": "path", + "name": "silo_name", + "description": "The silo's unique name.", + "required": true, + "schema": { + "$ref": "#/components/schemas/Name" + }, + "style": "simple" + } + ], + "responses": { + "200": { + "description": "successful operation", + "content": { + "application/json": { + "schema": { + "$ref": "#/components/schemas/Silo" + } + } + } + }, + "4XX": { + "$ref": "#/components/responses/Error" + }, + "5XX": { + "$ref": "#/components/responses/Error" + } + } + }, + "delete": { + "tags": [ + "silos" + ], + "summary": "Delete a specific silo.", + "operationId": "silos_delete_silo", + "parameters": [ + { + "in": "path", + "name": "silo_name", + "description": "The silo's unique name.", + "required": true, + "schema": { + "$ref": "#/components/schemas/Name" + }, + "style": "simple" + } + ], + "responses": { + "204": { + "description": "successful deletion" + }, + "4XX": { + "$ref": "#/components/responses/Error" + }, + "5XX": { + "$ref": "#/components/responses/Error" + } + } + } + }, "/timeseries/schema": { "get": { "tags": [ @@ -5054,6 +5218,91 @@ "id" ] }, + "Silo": { + "description": "Client view of a ['Silo']", + "type": "object", + "properties": { + "description": { + "description": "human-readable free-form text about a resource", + "type": "string" + }, + "discoverable": { + "type": "boolean" + }, + "id": { + "description": "unique, immutable, system-controlled identifier for each resource", + "type": "string", + "format": "uuid" + }, + "name": { + "description": "unique, mutable, user-controlled identifier for each resource", + "allOf": [ + { + "$ref": "#/components/schemas/Name" + } + ] + }, + "time_created": { + "description": "timestamp when this resource was created", + "type": "string", + "format": "date-time" + }, + "time_modified": { + "description": "timestamp when this resource was last modified", + "type": "string", + "format": "date-time" + } + }, + "required": [ + "description", + "discoverable", + "id", + "name", + "time_created", + "time_modified" + ] + }, + "SiloCreate": { + "description": "Create-time parameters for a [`Silo`](crate::external_api::views::Silo)", + "type": "object", + "properties": { + "description": { + "type": "string" + }, + "discoverable": { + "type": "boolean" + }, + "name": { + "$ref": "#/components/schemas/Name" + } + }, + "required": [ + "description", + "discoverable", + "name" + ] + }, + "SiloResultsPage": { + "description": "A single page of results", + "type": "object", + "properties": { + "items": { + "description": "list of items on this page of results", + "type": "array", + "items": { + "$ref": "#/components/schemas/Silo" + } + }, + "next_page": { + "nullable": true, + "description": "token used to fetch the next page of results (if any)", + "type": "string" + } + }, + "required": [ + "items" + ] + }, "Sled": { "description": "Client view of an [`Sled`]", "type": "object", From 6f08a14bdde274795edf41ba5bac367a94c5c565 Mon Sep 17 00:00:00 2001 From: James MacMahon Date: Thu, 10 Mar 2022 17:10:31 -0500 Subject: [PATCH 02/24] fmt --- nexus/src/db/datastore.rs | 17 +++++++++-------- 1 file changed, 9 insertions(+), 8 deletions(-) diff --git a/nexus/src/db/datastore.rs b/nexus/src/db/datastore.rs index d81d6d14108..d3a5bafc586 100644 --- a/nexus/src/db/datastore.rs +++ b/nexus/src/db/datastore.rs @@ -69,14 +69,15 @@ use crate::db::{ self, error::{public_error_from_diesel_pool, ErrorHandler, TransactionError}, model::{ - ConsoleSession, ConsoleSessionWithSiloId, Dataset, DatasetKind, Disk, DiskRuntimeState, - Generation, IncompleteNetworkInterface, Instance, InstanceRuntimeState, - Name, NetworkInterface, Organization, OrganizationUpdate, OximeterInfo, - ProducerEndpoint, Project, ProjectUpdate, Region, - RoleAssignmentBuiltin, RoleBuiltin, RouterRoute, RouterRouteUpdate, - Silo, SiloUser, Sled, UpdateArtifactKind, UpdateAvailableArtifact, UserBuiltin, Volume, - Vpc, VpcFirewallRule, VpcRouter, VpcRouterUpdate, VpcSubnet, - VpcSubnetUpdate, VpcUpdate, Zpool, + ConsoleSession, ConsoleSessionWithSiloId, Dataset, DatasetKind, Disk, + DiskRuntimeState, Generation, IncompleteNetworkInterface, Instance, + InstanceRuntimeState, Name, NetworkInterface, Organization, + OrganizationUpdate, OximeterInfo, ProducerEndpoint, Project, + ProjectUpdate, Region, RoleAssignmentBuiltin, RoleBuiltin, RouterRoute, + RouterRouteUpdate, Silo, SiloUser, Sled, UpdateArtifactKind, + UpdateAvailableArtifact, UserBuiltin, Volume, Vpc, VpcFirewallRule, + VpcRouter, VpcRouterUpdate, VpcSubnet, VpcSubnetUpdate, VpcUpdate, + Zpool, }, pagination::paginated, pagination::paginated_multicolumn, From ca3f95243da00939f8211b7a157e83f6221be341 Mon Sep 17 00:00:00 2001 From: James MacMahon Date: Mon, 21 Mar 2022 12:03:35 -0400 Subject: [PATCH 03/24] remove silo user name --- common/src/sql/dbinit.sql | 3 --- nexus/src/db/datastore.rs | 8 +++----- nexus/src/db/model.rs | 11 +---------- nexus/src/nexus.rs | 4 +--- nexus/tests/integration_tests/silos.rs | 7 +++---- 5 files changed, 8 insertions(+), 25 deletions(-) diff --git a/common/src/sql/dbinit.sql b/common/src/sql/dbinit.sql index 317b0990245..4c8d46288d3 100644 --- a/common/src/sql/dbinit.sql +++ b/common/src/sql/dbinit.sql @@ -207,9 +207,6 @@ CREATE TABLE omicron.public.silo_user ( silo_id UUID NOT NULL, - /* XXX SCIM parameters? */ - name TEXT NOT NULL, - time_created TIMESTAMPTZ NOT NULL, time_modified TIMESTAMPTZ NOT NULL, time_deleted TIMESTAMPTZ, diff --git a/nexus/src/db/datastore.rs b/nexus/src/db/datastore.rs index d3a5bafc586..7c46917369f 100644 --- a/nexus/src/db/datastore.rs +++ b/nexus/src/db/datastore.rs @@ -2863,8 +2863,7 @@ impl DataStore { for builtin_user in &builtin_users { self.silo_user_create(SiloUser::new( *SILO_ID, - Uuid::new_v4(), /* silo user id */ - builtin_user.identity.name.to_string(), /* name */ + Uuid::new_v4(), /* silo user id */ builtin_user.identity.id, /* internal user id */ )) .await?; @@ -3503,9 +3502,8 @@ mod test { // Associate silo with user let silo_user = datastore .silo_user_create(SiloUser::new( - Uuid::new_v4(), /* silo id */ - Uuid::new_v4(), /* silo user id */ - "fakeuser".to_string(), + Uuid::new_v4(), /* silo id */ + Uuid::new_v4(), /* silo user id */ session.user_id, /* internal user id */ )) .await diff --git a/nexus/src/db/model.rs b/nexus/src/db/model.rs index 4b589a7a5ae..4b462228518 100644 --- a/nexus/src/db/model.rs +++ b/nexus/src/db/model.rs @@ -820,8 +820,6 @@ pub struct SiloUser { pub id: Uuid, pub silo_id: Uuid, - pub name: String, - pub time_created: DateTime, pub time_modified: DateTime, pub time_deleted: Option>, @@ -830,19 +828,12 @@ pub struct SiloUser { } impl SiloUser { - pub fn new( - silo_id: Uuid, - user_id: Uuid, - name: String, - internal_user_id: Uuid, - ) -> Self { + pub fn new(silo_id: Uuid, user_id: Uuid, internal_user_id: Uuid) -> Self { let now = Utc::now(); Self { id: user_id, silo_id, - name, - time_created: now, time_modified: now, time_deleted: None, diff --git a/nexus/src/nexus.rs b/nexus/src/nexus.rs index c5cb33b1120..66a6422a6fe 100644 --- a/nexus/src/nexus.rs +++ b/nexus/src/nexus.rs @@ -2945,11 +2945,9 @@ impl Nexus { &self, silo_id: Uuid, silo_user_id: Uuid, - name: String, internal_user_id: Uuid, ) -> CreateResult { - let silo_user = - SiloUser::new(silo_id, silo_user_id, name, internal_user_id); + let silo_user = SiloUser::new(silo_id, silo_user_id, internal_user_id); Ok(self.db_datastore.silo_user_create(silo_user).await?) } } diff --git a/nexus/tests/integration_tests/silos.rs b/nexus/tests/integration_tests/silos.rs index e7d8f622117..6b9ba03139a 100644 --- a/nexus/tests/integration_tests/silos.rs +++ b/nexus/tests/integration_tests/silos.rs @@ -68,10 +68,9 @@ async fn test_silos(cptestctx: &ControlPlaneTestContext) { // Create a new user in the discoverable silo, then create a console session let new_silo_user = nexus .silo_user_create( - silos[0].identity.id, /* silo id */ - Uuid::new_v4(), /* silo user id */ - "new silo user".to_string(), /* name */ - Uuid::new_v4(), /* internal user id */ + silos[0].identity.id, /* silo id */ + Uuid::new_v4(), /* silo user id */ + Uuid::new_v4(), /* internal user id */ ) .await .unwrap(); From 3003cafaeafb6c899e0bc5a48ccc5cf022a51652 Mon Sep 17 00:00:00 2001 From: James MacMahon Date: Mon, 21 Mar 2022 13:01:26 -0400 Subject: [PATCH 04/24] remove internal_user_id from silo user --- common/src/sql/dbinit.sql | 9 ++-- nexus/src/authn/external/session_cookie.rs | 7 +-- nexus/src/authn/mod.rs | 2 +- nexus/src/context.rs | 4 +- nexus/src/db/datastore.rs | 51 +++++---------------- nexus/src/db/model.rs | 12 ++--- nexus/src/db/schema.rs | 6 +-- nexus/src/external_api/console_api.rs | 2 +- nexus/src/nexus.rs | 10 +--- nexus/tests/integration_tests/authn_http.rs | 14 +++--- nexus/tests/integration_tests/silos.rs | 9 +--- 11 files changed, 36 insertions(+), 90 deletions(-) diff --git a/common/src/sql/dbinit.sql b/common/src/sql/dbinit.sql index 4c8d46288d3..3d888b65829 100644 --- a/common/src/sql/dbinit.sql +++ b/common/src/sql/dbinit.sql @@ -209,10 +209,7 @@ CREATE TABLE omicron.public.silo_user ( time_created TIMESTAMPTZ NOT NULL, time_modified TIMESTAMPTZ NOT NULL, - time_deleted TIMESTAMPTZ, - - /* internal user id used by authz */ - internal_user_id UUID NOT NULL + time_deleted TIMESTAMPTZ ); /* @@ -754,8 +751,8 @@ CREATE TABLE omicron.public.console_session ( token STRING(40) PRIMARY KEY, time_created TIMESTAMPTZ NOT NULL, time_last_used TIMESTAMPTZ NOT NULL, - -- this maps to internal_user_id in the silo_user table - user_id UUID NOT NULL + -- this maps to id in the silo_user table + silo_user_id UUID NOT NULL ); -- to be used for cleaning up old tokens diff --git a/nexus/src/authn/external/session_cookie.rs b/nexus/src/authn/external/session_cookie.rs index 1f15d923885..4f22fc84c98 100644 --- a/nexus/src/authn/external/session_cookie.rs +++ b/nexus/src/authn/external/session_cookie.rs @@ -17,7 +17,7 @@ use uuid::Uuid; // https://cheatsheetseries.owasp.org/cheatsheets/Session_Management_Cheat_Sheet.html pub trait Session { - fn user_id(&self) -> Uuid; + fn silo_user_id(&self) -> Uuid; fn silo_id(&self) -> Uuid; fn time_last_used(&self) -> DateTime; fn time_created(&self) -> DateTime; @@ -105,7 +105,8 @@ where } }; - let actor = Actor { id: session.user_id(), silo_id: session.silo_id() }; + let actor = + Actor { id: session.silo_user_id(), silo_id: session.silo_id() }; // if the session has gone unused for longer than idle_timeout, it is expired let now = Utc::now(); @@ -194,7 +195,7 @@ mod test { } impl Session for FakeSession { - fn user_id(&self) -> Uuid { + fn silo_user_id(&self) -> Uuid { Uuid::new_v4() } fn silo_id(&self) -> Uuid { diff --git a/nexus/src/authn/mod.rs b/nexus/src/authn/mod.rs index d7306a7a2e6..47edc3176ed 100644 --- a/nexus/src/authn/mod.rs +++ b/nexus/src/authn/mod.rs @@ -205,7 +205,7 @@ pub struct Details { /// Who is performing an operation #[derive(Clone, Copy, Debug, Deserialize, Eq, PartialEq, Serialize)] pub struct Actor { - pub id: Uuid, + pub id: Uuid, // silo user id pub silo_id: Uuid, } diff --git a/nexus/src/context.rs b/nexus/src/context.rs index 92023f0fad8..08cc0d1bda3 100644 --- a/nexus/src/context.rs +++ b/nexus/src/context.rs @@ -528,8 +528,8 @@ impl SessionStore for Arc { } impl Session for ConsoleSessionWithSiloId { - fn user_id(&self) -> Uuid { - self.console_session.user_id + fn silo_user_id(&self) -> Uuid { + self.console_session.silo_user_id } fn silo_id(&self) -> Uuid { self.silo_id diff --git a/nexus/src/db/datastore.rs b/nexus/src/db/datastore.rs index 7c46917369f..c8c1d6779e5 100644 --- a/nexus/src/db/datastore.rs +++ b/nexus/src/db/datastore.rs @@ -2702,7 +2702,7 @@ impl DataStore { })?; let silo_id = self - .get_silo_id_from_internal_user_id(console_session.user_id) + .get_silo_id_from_silo_user_id(console_session.silo_user_id) .await .map_err(|e| { Error::internal_error(&format!( @@ -2753,7 +2753,7 @@ impl DataStore { })?; let silo_id = self - .get_silo_id_from_internal_user_id(console_session.user_id) + .get_silo_id_from_silo_user_id(console_session.silo_user_id) .await .map_err(|e| { Error::internal_error(&format!( @@ -2863,8 +2863,7 @@ impl DataStore { for builtin_user in &builtin_users { self.silo_user_create(SiloUser::new( *SILO_ID, - Uuid::new_v4(), /* silo user id */ - builtin_user.identity.id, /* internal user id */ + builtin_user.identity.id, /* silo user id */ )) .await?; } @@ -3106,16 +3105,6 @@ impl DataStore { Ok(silo_user.silo_id) } - pub async fn get_silo_id_from_internal_user_id( - &self, - internal_user_id: Uuid, - ) -> LookupResult { - let silo_user: SiloUser = - self.silo_user_fetch_by_internal_id(internal_user_id).await?; - - Ok(silo_user.silo_id) - } - pub async fn silo_user_create( &self, silo_user: SiloUser, @@ -3155,26 +3144,6 @@ impl DataStore { }) } - pub async fn silo_user_fetch_by_internal_id( - &self, - internal_user_id: Uuid, - ) -> CreateResult { - use db::schema::silo_user::dsl; - - dsl::silo_user - .filter(dsl::internal_user_id.eq(internal_user_id)) - .filter(dsl::time_deleted.is_null()) - .select(SiloUser::as_select()) - .first_async(self.pool()) - .await - .map_err(|e| { - Error::internal_error(&format!( - "error fetching silo user: {:?}", - e - )) - }) - } - /// Load built-in silos into the database pub async fn load_builtin_silos( &self, @@ -3489,12 +3458,15 @@ mod test { let logctx = dev::test_setup_log("test_session_methods"); let mut db = test_setup_database(&logctx.log).await; let (_, datastore) = datastore_test(&logctx, &db).await; + let token = "a_token".to_string(); + let silo_user_id = Uuid::new_v4(); + let session = ConsoleSession { token: token.clone(), time_created: Utc::now() - Duration::minutes(5), time_last_used: Utc::now() - Duration::minutes(5), - user_id: Uuid::new_v4(), + silo_user_id, }; let _ = datastore.session_create(session.clone()).await; @@ -3502,9 +3474,8 @@ mod test { // Associate silo with user let silo_user = datastore .silo_user_create(SiloUser::new( - Uuid::new_v4(), /* silo id */ - Uuid::new_v4(), /* silo user id */ - session.user_id, /* internal user id */ + Uuid::new_v4(), /* silo id */ + silo_user_id, )) .await .unwrap(); @@ -3512,14 +3483,14 @@ mod test { assert_eq!( silo_user.silo_id, datastore - .get_silo_id_from_internal_user_id(session.user_id) + .get_silo_id_from_silo_user_id(session.silo_user_id) .await .unwrap(), ); // fetch the one we just created let fetched = datastore.session_fetch(token.clone()).await.unwrap(); - assert_eq!(session.user_id, fetched.console_session.user_id); + assert_eq!(session.silo_user_id, fetched.console_session.silo_user_id); // trying to insert the same one again fails let duplicate = datastore.session_create(session.clone()).await; diff --git a/nexus/src/db/model.rs b/nexus/src/db/model.rs index 4b462228518..efd518b5904 100644 --- a/nexus/src/db/model.rs +++ b/nexus/src/db/model.rs @@ -823,12 +823,10 @@ pub struct SiloUser { pub time_created: DateTime, pub time_modified: DateTime, pub time_deleted: Option>, - - pub internal_user_id: Uuid, } impl SiloUser { - pub fn new(silo_id: Uuid, user_id: Uuid, internal_user_id: Uuid) -> Self { + pub fn new(silo_id: Uuid, user_id: Uuid) -> Self { let now = Utc::now(); Self { id: user_id, @@ -837,8 +835,6 @@ impl SiloUser { time_created: now, time_modified: now, time_deleted: None, - - internal_user_id, } } } @@ -2120,13 +2116,13 @@ pub struct ConsoleSession { pub token: String, pub time_created: DateTime, pub time_last_used: DateTime, - pub user_id: Uuid, + pub silo_user_id: Uuid, } impl ConsoleSession { - pub fn new(token: String, user_id: Uuid) -> Self { + pub fn new(token: String, silo_user_id: Uuid) -> Self { let now = Utc::now(); - Self { token, user_id, time_last_used: now, time_created: now } + Self { token, silo_user_id, time_last_used: now, time_created: now } } } diff --git a/nexus/src/db/schema.rs b/nexus/src/db/schema.rs index 7dd31008cc9..e80181fe97c 100644 --- a/nexus/src/db/schema.rs +++ b/nexus/src/db/schema.rs @@ -96,13 +96,9 @@ table! { id -> Uuid, silo_id -> Uuid, - name -> Text, - time_created -> Timestamptz, time_modified -> Timestamptz, time_deleted -> Nullable, - - internal_user_id -> Uuid, } } @@ -180,7 +176,7 @@ table! { token -> Text, time_created -> Timestamptz, time_last_used -> Timestamptz, - user_id -> Uuid, + silo_user_id -> Uuid, } } diff --git a/nexus/src/external_api/console_api.rs b/nexus/src/external_api/console_api.rs index 2eb3e94524f..6e3a4cb73be 100644 --- a/nexus/src/external_api/console_api.rs +++ b/nexus/src/external_api/console_api.rs @@ -70,7 +70,7 @@ pub async fn spoof_login( let user_id = user_id.unwrap(); // look up silo id, make sure user is legal - if let Err(e) = nexus.get_silo_id_from_internal_user_id(user_id).await { + if let Err(e) = nexus.get_silo_id_from_silo_user_id(user_id).await { return Err(HttpError::for_internal_error(format!( "no silo id found! {}", e diff --git a/nexus/src/nexus.rs b/nexus/src/nexus.rs index 66a6422a6fe..451ac0a96b4 100644 --- a/nexus/src/nexus.rs +++ b/nexus/src/nexus.rs @@ -2934,20 +2934,12 @@ impl Nexus { self.db_datastore.get_silo_id_from_silo_user_id(silo_user_id).await } - pub async fn get_silo_id_from_internal_user_id( - &self, - user_id: Uuid, - ) -> LookupResult { - self.db_datastore.get_silo_id_from_internal_user_id(user_id).await - } - pub async fn silo_user_create( &self, silo_id: Uuid, silo_user_id: Uuid, - internal_user_id: Uuid, ) -> CreateResult { - let silo_user = SiloUser::new(silo_id, silo_user_id, internal_user_id); + let silo_user = SiloUser::new(silo_id, silo_user_id); Ok(self.db_datastore.silo_user_create(silo_user).await?) } } diff --git a/nexus/tests/integration_tests/authn_http.rs b/nexus/tests/integration_tests/authn_http.rs index e021af7503a..1455c1b34ea 100644 --- a/nexus/tests/integration_tests/authn_http.rs +++ b/nexus/tests/integration_tests/authn_http.rs @@ -104,19 +104,19 @@ async fn test_authn_session_cookie() { Box + 'static>, > = vec![Box::new(session_cookie::HttpAuthnSessionCookie)]; let valid_session = FakeSession { - user_id: Uuid::new_v4(), + silo_user_id: Uuid::new_v4(), silo_id: Uuid::new_v4(), time_last_used: Utc::now() - Duration::seconds(5), time_created: Utc::now() - Duration::seconds(5), }; let idle_expired_session = FakeSession { - user_id: Uuid::new_v4(), + silo_user_id: Uuid::new_v4(), silo_id: Uuid::new_v4(), time_last_used: Utc::now() - Duration::hours(2), time_created: Utc::now() - Duration::hours(3), }; let abs_expired_session = FakeSession { - user_id: Uuid::new_v4(), + silo_user_id: Uuid::new_v4(), silo_id: Uuid::new_v4(), time_last_used: Utc::now(), time_created: Utc::now() - Duration::hours(10), @@ -141,7 +141,7 @@ async fn test_authn_session_cookie() { whoami_request(None, Some(valid_header), &testctx).await.unwrap(), WhoamiResponse { authenticated: true, - actor: Some(valid_session.user_id.to_string()), + actor: Some(valid_session.silo_user_id.to_string()), schemes_tried: tried_cookie.clone(), } ); @@ -310,15 +310,15 @@ struct WhoamiServerState { #[derive(Clone, Copy)] struct FakeSession { - user_id: Uuid, + silo_user_id: Uuid, silo_id: Uuid, time_created: DateTime, time_last_used: DateTime, } impl session_cookie::Session for FakeSession { - fn user_id(&self) -> Uuid { - self.user_id + fn silo_user_id(&self) -> Uuid { + self.silo_user_id } fn silo_id(&self) -> Uuid { self.silo_id diff --git a/nexus/tests/integration_tests/silos.rs b/nexus/tests/integration_tests/silos.rs index 6b9ba03139a..40743d939a0 100644 --- a/nexus/tests/integration_tests/silos.rs +++ b/nexus/tests/integration_tests/silos.rs @@ -11,7 +11,6 @@ use http::method::Method; use http::StatusCode; use nexus_test_utils::resource_helpers::{ create_organization, create_silo, objects_list_page_authz, - objects_list_page_authz_with_session, }; use nexus_test_utils::ControlPlaneTestContext; @@ -70,13 +69,11 @@ async fn test_silos(cptestctx: &ControlPlaneTestContext) { .silo_user_create( silos[0].identity.id, /* silo id */ Uuid::new_v4(), /* silo user id */ - Uuid::new_v4(), /* internal user id */ ) .await .unwrap(); - let session = - nexus.session_create(new_silo_user.internal_user_id).await.unwrap(); + let session = nexus.session_create(new_silo_user.id).await.unwrap(); // Create organization with built-in user auth // Note: this currently goes to the built-in silo! @@ -131,10 +128,6 @@ async fn test_silos(cptestctx: &ControlPlaneTestContext) { .expect("failed to make request"); // Verify silo user was also deleted - nexus - .get_silo_id_from_internal_user_id(new_silo_user.internal_user_id) - .await - .expect_err("unexpected success"); nexus .get_silo_id_from_silo_user_id(new_silo_user.id) .await From 172ea8285501ca530d88e2634c06ed21091b47a3 Mon Sep 17 00:00:00 2001 From: James MacMahon Date: Mon, 21 Mar 2022 13:09:23 -0400 Subject: [PATCH 05/24] remove laziness --- nexus/src/authn/external/session_cookie.rs | 17 ++++++++++++++--- 1 file changed, 14 insertions(+), 3 deletions(-) diff --git a/nexus/src/authn/external/session_cookie.rs b/nexus/src/authn/external/session_cookie.rs index 4f22fc84c98..6fde989032d 100644 --- a/nexus/src/authn/external/session_cookie.rs +++ b/nexus/src/authn/external/session_cookie.rs @@ -190,16 +190,18 @@ mod test { #[derive(Clone, Copy)] struct FakeSession { + silo_user_id: Uuid, + silo_id: Uuid, time_created: DateTime, time_last_used: DateTime, } impl Session for FakeSession { fn silo_user_id(&self) -> Uuid { - Uuid::new_v4() + self.silo_user_id } fn silo_id(&self) -> Uuid { - Uuid::new_v4() + self.silo_id } fn time_created(&self) -> DateTime { self.time_created @@ -282,6 +284,8 @@ mod test { sessions: Mutex::new(HashMap::from([( "abc".to_string(), FakeSession { + silo_user_id: Uuid::new_v4(), + silo_id: Uuid::new_v4(), time_last_used: Utc::now() - Duration::hours(2), time_created: Utc::now() - Duration::hours(2), }, @@ -306,6 +310,8 @@ mod test { sessions: Mutex::new(HashMap::from([( "abc".to_string(), FakeSession { + silo_user_id: Uuid::new_v4(), + silo_id: Uuid::new_v4(), time_last_used: Utc::now(), time_created: Utc::now() - Duration::hours(20), }, @@ -331,7 +337,12 @@ mod test { let context = TestServerContext { sessions: Mutex::new(HashMap::from([( "abc".to_string(), - FakeSession { time_last_used, time_created: Utc::now() }, + FakeSession { + silo_user_id: Uuid::new_v4(), + silo_id: Uuid::new_v4(), + time_last_used, + time_created: Utc::now(), + }, )])), }; let result = authn_with_cookie(&context, Some("session=abc")).await; From 4e4b616470d150232938c9386ae31cc40f71ed21 Mon Sep 17 00:00:00 2001 From: James MacMahon Date: Mon, 21 Mar 2022 14:41:45 -0400 Subject: [PATCH 06/24] add some silo commands to oxapi_demo --- tools/oxapi_demo | 32 ++++++++++++++++++++++++++++++++ 1 file changed, 32 insertions(+) diff --git a/tools/oxapi_demo b/tools/oxapi_demo index 94abf753fc9..e5800c9876e 100755 --- a/tools/oxapi_demo +++ b/tools/oxapi_demo @@ -19,6 +19,13 @@ GENERAL OPTIONS (default behavior: use "spoof" authentication for endpoints that require it) +SILOS + + silos_list + silo_create_demo SILO NAME + silo_get SILO_NAME + silo_delete SILO_NAME + ORGANIZATIONS organizations_list @@ -195,6 +202,31 @@ function mkjson # API commands # +function cmd_silos_list +{ + [[ $# != 0 ]] && usage "expected no arguments" + do_curl_authn /silos +} + +function cmd_silo_create_demo +{ + [[ $# != 1 ]] && usage "expected SILO_NAME" + mkjson name="${1}" description="a silo called ${1}" discoverable="true" | + do_curl_authn /silos -X POST -T - +} + +function cmd_silo_get +{ + [[ $# != 1 ]] && usage "expected SILO_NAME" + do_curl_authn "/silos/${1}" +} + +function cmd_silo_delete +{ + [[ $# != 1 ]] && usage "expected SILO_NAME" + do_curl_authn "/silos/${1}" -X DELETE +} + function cmd_organizations_list { [[ $# != 0 ]] && usage "expected no arguments" From 507ec3d718ba10224b4fff67248a29a796baa6d0 Mon Sep 17 00:00:00 2001 From: James MacMahon Date: Mon, 21 Mar 2022 14:44:14 -0400 Subject: [PATCH 07/24] small comment about discoverable --- nexus/src/external_api/views.rs | 2 ++ 1 file changed, 2 insertions(+) diff --git a/nexus/src/external_api/views.rs b/nexus/src/external_api/views.rs index 5deb3e289f6..53321840604 100644 --- a/nexus/src/external_api/views.rs +++ b/nexus/src/external_api/views.rs @@ -30,6 +30,8 @@ pub struct Silo { #[serde(flatten)] pub identity: IdentityMetadata, + /// A silo where discoverable is false can be retrieved only by its id - it + /// will not be part of the "list all silos" output. pub discoverable: bool, } From 9cb914b24f50c0504e942dfe92d3241d056acdd9 Mon Sep 17 00:00:00 2001 From: James MacMahon Date: Mon, 21 Mar 2022 14:44:49 -0400 Subject: [PATCH 08/24] TODO-security --- nexus/tests/integration_tests/basic.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/nexus/tests/integration_tests/basic.rs b/nexus/tests/integration_tests/basic.rs index 91ee6edd9f5..bebe8a387cc 100644 --- a/nexus/tests/integration_tests/basic.rs +++ b/nexus/tests/integration_tests/basic.rs @@ -58,7 +58,7 @@ async fn test_basic_failures(cptestctx: &ControlPlaneTestContext) { * value that does not exist inside a collection that does exist) from an * unauthorized user results in a 401. - // XXX uncomment when this endpoint is protected by authn! + // TODO-security uncomment when this endpoint is protected by authn! let error = client .make_request( From ba37d656895ac7c03d627519639f091a2a3f4541 Mon Sep 17 00:00:00 2001 From: James MacMahon Date: Mon, 21 Mar 2022 14:52:05 -0400 Subject: [PATCH 09/24] remove commented TODO-security code --- nexus/tests/integration_tests/basic.rs | 19 ------------------- openapi/nexus.json | 1 + 2 files changed, 1 insertion(+), 19 deletions(-) diff --git a/nexus/tests/integration_tests/basic.rs b/nexus/tests/integration_tests/basic.rs index bebe8a387cc..7c525812758 100644 --- a/nexus/tests/integration_tests/basic.rs +++ b/nexus/tests/integration_tests/basic.rs @@ -53,25 +53,6 @@ async fn test_basic_failures(cptestctx: &ControlPlaneTestContext) { .expect_err("expected error"); assert_eq!("Not Found", error.message); - /* - * Error case: GET /organizations/test-org/projects/nonexistent (a possible - * value that does not exist inside a collection that does exist) from an - * unauthorized user results in a 401. - - // TODO-security uncomment when this endpoint is protected by authn! - - let error = client - .make_request( - Method::GET, - "/organizations/test-org/projects/nonexistent", - None as Option<()>, - StatusCode::UNAUTHORIZED, - ) - .await - .expect_err("expected error"); - assert_eq!("credentials missing or invalid", error.message); - */ - struct TestCase<'a> { method: http::Method, uri: &'a str, diff --git a/openapi/nexus.json b/openapi/nexus.json index b7513b08863..f7b3304c0d5 100644 --- a/openapi/nexus.json +++ b/openapi/nexus.json @@ -5227,6 +5227,7 @@ "type": "string" }, "discoverable": { + "description": "A silo where discoverable is false can be retrieved only by its id - it will not be part of the \"list all silos\" output.", "type": "boolean" }, "id": { From 227fb3c100db5f8061e3316f8302906411a6a935 Mon Sep 17 00:00:00 2001 From: James MacMahon Date: Mon, 21 Mar 2022 15:58:38 -0400 Subject: [PATCH 10/24] use session token string --- nexus/test-utils/src/http_testing.rs | 4 ++-- nexus/test-utils/src/resource_helpers.rs | 4 ++-- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/nexus/test-utils/src/http_testing.rs b/nexus/test-utils/src/http_testing.rs index e9abd842b2d..b1fb1b72b68 100644 --- a/nexus/test-utils/src/http_testing.rs +++ b/nexus/test-utils/src/http_testing.rs @@ -445,10 +445,10 @@ impl<'a> NexusRequest<'a> { self } - pub fn authn_with_session(mut self, session: &ConsoleSession) -> Self { + pub fn authn_with_session(mut self, session_token: &str) -> Self { self.request_builder = self.request_builder.header( &http::header::COOKIE, - format!("session={}", session.token), + format!("session={}", session_token), ); self } diff --git a/nexus/test-utils/src/resource_helpers.rs b/nexus/test-utils/src/resource_helpers.rs index f01f2e5c77d..4ba4af270d8 100644 --- a/nexus/test-utils/src/resource_helpers.rs +++ b/nexus/test-utils/src/resource_helpers.rs @@ -44,13 +44,13 @@ where pub async fn objects_list_page_authz_with_session( client: &ClientTestContext, path: &str, - session: &ConsoleSession, + session_token: &str, ) -> dropshot::ResultsPage where ItemType: serde::de::DeserializeOwned, { NexusRequest::object_get(client, path) - .authn_with_session(session) + .authn_with_session(session_token) .execute() .await .expect("failed to make request") From d8aa2431ab354893d240e3bbd87a1e898a0b4b40 Mon Sep 17 00:00:00 2001 From: James MacMahon Date: Mon, 21 Mar 2022 16:28:37 -0400 Subject: [PATCH 11/24] move ConsoleSessionWithSiloId to authn mod --- nexus/src/authn/mod.rs | 8 ++++++++ nexus/src/context.rs | 2 +- nexus/src/db/datastore.rs | 25 ++++++++++++------------ nexus/src/db/model.rs | 7 ------- nexus/src/nexus.rs | 4 ++-- nexus/test-utils/src/http_testing.rs | 1 - nexus/test-utils/src/resource_helpers.rs | 1 - 7 files changed, 23 insertions(+), 25 deletions(-) diff --git a/nexus/src/authn/mod.rs b/nexus/src/authn/mod.rs index 47edc3176ed..3e5a95d1980 100644 --- a/nexus/src/authn/mod.rs +++ b/nexus/src/authn/mod.rs @@ -32,6 +32,7 @@ pub use crate::db::fixed_data::user_builtin::USER_INTERNAL_API; pub use crate::db::fixed_data::user_builtin::USER_SAGA_RECOVERY; pub use crate::db::fixed_data::user_builtin::USER_TEST_PRIVILEGED; pub use crate::db::fixed_data::user_builtin::USER_TEST_UNPRIVILEGED; +use crate::db::model::ConsoleSession; use serde::Deserialize; use serde::Serialize; @@ -209,6 +210,13 @@ pub struct Actor { pub silo_id: Uuid, } +/// A console session with the silo id of the authenticated user +#[derive(Clone, Debug)] +pub struct ConsoleSessionWithSiloId { + pub console_session: ConsoleSession, + pub silo_id: Uuid, +} + /// Label for a particular authentication scheme (used in log messages and /// internal error messages) #[derive(Clone, Copy, Debug, Eq, PartialEq)] diff --git a/nexus/src/context.rs b/nexus/src/context.rs index 08cc0d1bda3..5042c0b0bd3 100644 --- a/nexus/src/context.rs +++ b/nexus/src/context.rs @@ -11,8 +11,8 @@ use super::config; use super::db; use super::Nexus; use crate::authn::external::session_cookie::{Session, SessionStore}; +use crate::authn::ConsoleSessionWithSiloId; use crate::authz::AuthorizedResource; -use crate::db::model::ConsoleSessionWithSiloId; use crate::db::DataStore; use crate::saga_interface::SagaContext; use async_trait::async_trait; diff --git a/nexus/src/db/datastore.rs b/nexus/src/db/datastore.rs index c8c1d6779e5..869717a6aed 100644 --- a/nexus/src/db/datastore.rs +++ b/nexus/src/db/datastore.rs @@ -69,15 +69,14 @@ use crate::db::{ self, error::{public_error_from_diesel_pool, ErrorHandler, TransactionError}, model::{ - ConsoleSession, ConsoleSessionWithSiloId, Dataset, DatasetKind, Disk, - DiskRuntimeState, Generation, IncompleteNetworkInterface, Instance, - InstanceRuntimeState, Name, NetworkInterface, Organization, - OrganizationUpdate, OximeterInfo, ProducerEndpoint, Project, - ProjectUpdate, Region, RoleAssignmentBuiltin, RoleBuiltin, RouterRoute, - RouterRouteUpdate, Silo, SiloUser, Sled, UpdateArtifactKind, - UpdateAvailableArtifact, UserBuiltin, Volume, Vpc, VpcFirewallRule, - VpcRouter, VpcRouterUpdate, VpcSubnet, VpcSubnetUpdate, VpcUpdate, - Zpool, + ConsoleSession, Dataset, DatasetKind, Disk, DiskRuntimeState, + Generation, IncompleteNetworkInterface, Instance, InstanceRuntimeState, + Name, NetworkInterface, Organization, OrganizationUpdate, OximeterInfo, + ProducerEndpoint, Project, ProjectUpdate, Region, + RoleAssignmentBuiltin, RoleBuiltin, RouterRoute, RouterRouteUpdate, + Silo, SiloUser, Sled, UpdateArtifactKind, UpdateAvailableArtifact, + UserBuiltin, Volume, Vpc, VpcFirewallRule, VpcRouter, VpcRouterUpdate, + VpcSubnet, VpcSubnetUpdate, VpcUpdate, Zpool, }, pagination::paginated, pagination::paginated_multicolumn, @@ -2687,7 +2686,7 @@ impl DataStore { pub async fn session_fetch( &self, token: String, - ) -> LookupResult { + ) -> LookupResult { use db::schema::console_session::dsl; let console_session = dsl::console_session .filter(dsl::token.eq(token.clone())) @@ -2711,7 +2710,7 @@ impl DataStore { )) })?; - Ok(ConsoleSessionWithSiloId { console_session, silo_id }) + Ok(authn::ConsoleSessionWithSiloId { console_session, silo_id }) } pub async fn session_create( @@ -2736,7 +2735,7 @@ impl DataStore { pub async fn session_update_last_used( &self, token: String, - ) -> UpdateResult { + ) -> UpdateResult { use db::schema::console_session::dsl; let console_session = diesel::update(dsl::console_session) @@ -2762,7 +2761,7 @@ impl DataStore { )) })?; - Ok(ConsoleSessionWithSiloId { console_session, silo_id }) + Ok(authn::ConsoleSessionWithSiloId { console_session, silo_id }) } // putting "hard" in the name because we don't do this with any other model diff --git a/nexus/src/db/model.rs b/nexus/src/db/model.rs index efd518b5904..565dd0bf172 100644 --- a/nexus/src/db/model.rs +++ b/nexus/src/db/model.rs @@ -2126,13 +2126,6 @@ impl ConsoleSession { } } -// Return this instead, by performing an additional SELECT to get silo id -#[derive(Clone, Debug)] -pub struct ConsoleSessionWithSiloId { - pub console_session: ConsoleSession, - pub silo_id: Uuid, -} - /// Describes a built-in user, as stored in the database #[derive(Queryable, Insertable, Debug, Resource, Selectable)] #[table_name = "user_builtin"] diff --git a/nexus/src/nexus.rs b/nexus/src/nexus.rs index 451ac0a96b4..504e170b8a2 100644 --- a/nexus/src/nexus.rs +++ b/nexus/src/nexus.rs @@ -2673,7 +2673,7 @@ impl Nexus { pub async fn session_fetch( &self, token: String, - ) -> LookupResult { + ) -> LookupResult { self.db_datastore.session_fetch(token).await } @@ -2690,7 +2690,7 @@ impl Nexus { pub async fn session_update_last_used( &self, token: String, - ) -> UpdateResult { + ) -> UpdateResult { Ok(self.db_datastore.session_update_last_used(token).await?) } diff --git a/nexus/test-utils/src/http_testing.rs b/nexus/test-utils/src/http_testing.rs index b1fb1b72b68..cb54c478887 100644 --- a/nexus/test-utils/src/http_testing.rs +++ b/nexus/test-utils/src/http_testing.rs @@ -11,7 +11,6 @@ use dropshot::test_util::ClientTestContext; use dropshot::ResultsPage; use headers::authorization::Credentials; use omicron_nexus::authn::external::spoof; -use omicron_nexus::db::model::ConsoleSession; use std::convert::TryInto; use std::fmt::Debug; diff --git a/nexus/test-utils/src/resource_helpers.rs b/nexus/test-utils/src/resource_helpers.rs index 4ba4af270d8..9e96e5eff5e 100644 --- a/nexus/test-utils/src/resource_helpers.rs +++ b/nexus/test-utils/src/resource_helpers.rs @@ -18,7 +18,6 @@ use omicron_common::api::external::Instance; use omicron_common::api::external::InstanceCpuCount; use omicron_common::api::external::VpcRouter; use omicron_nexus::crucible_agent_client::types::State as RegionState; -use omicron_nexus::db::model::ConsoleSession; use omicron_nexus::external_api::params; use omicron_nexus::external_api::views::{Organization, Project, Silo, Vpc}; use omicron_sled_agent::sim::SledAgent; From bf8ad67d9b6b46ffdb02f2dee4d5a1080358eb4e Mon Sep 17 00:00:00 2001 From: James MacMahon Date: Mon, 21 Mar 2022 16:40:07 -0400 Subject: [PATCH 12/24] create check_login_allowed function get rid of get_silo_id_from_silo_user_id, use silo_user_fetch instead. --- nexus/src/db/datastore.rs | 64 +++++++++++++------------- nexus/src/external_api/console_api.rs | 11 ++--- nexus/src/nexus.rs | 37 +++++++++++++-- nexus/tests/integration_tests/silos.rs | 2 +- 4 files changed, 71 insertions(+), 43 deletions(-) diff --git a/nexus/src/db/datastore.rs b/nexus/src/db/datastore.rs index 869717a6aed..d73cbffa0e5 100644 --- a/nexus/src/db/datastore.rs +++ b/nexus/src/db/datastore.rs @@ -2688,6 +2688,7 @@ impl DataStore { token: String, ) -> LookupResult { use db::schema::console_session::dsl; + let console_session = dsl::console_session .filter(dsl::token.eq(token.clone())) .select(ConsoleSession::as_select()) @@ -2700,8 +2701,8 @@ impl DataStore { )) })?; - let silo_id = self - .get_silo_id_from_silo_user_id(console_session.silo_user_id) + let silo_user = self + .silo_user_fetch(console_session.silo_user_id) .await .map_err(|e| { Error::internal_error(&format!( @@ -2710,7 +2711,10 @@ impl DataStore { )) })?; - Ok(authn::ConsoleSessionWithSiloId { console_session, silo_id }) + Ok(authn::ConsoleSessionWithSiloId { + console_session, + silo_id: silo_user.silo_id, + }) } pub async fn session_create( @@ -2751,8 +2755,8 @@ impl DataStore { )) })?; - let silo_id = self - .get_silo_id_from_silo_user_id(console_session.silo_user_id) + let silo_user = self + .silo_user_fetch(console_session.silo_user_id) .await .map_err(|e| { Error::internal_error(&format!( @@ -2761,7 +2765,10 @@ impl DataStore { )) })?; - Ok(authn::ConsoleSessionWithSiloId { console_session, silo_id }) + Ok(authn::ConsoleSessionWithSiloId { + console_session, + silo_id: silo_user.silo_id, + }) } // putting "hard" in the name because we don't do this with any other model @@ -3094,50 +3101,40 @@ impl DataStore { }) } - pub async fn get_silo_id_from_silo_user_id( + pub async fn silo_user_fetch( &self, silo_user_id: Uuid, - ) -> LookupResult { - let silo_user: SiloUser = - self.silo_user_fetch_by_id(silo_user_id).await?; - - Ok(silo_user.silo_id) - } - - pub async fn silo_user_create( - &self, - silo_user: SiloUser, - ) -> CreateResult { + ) -> LookupResult { use db::schema::silo_user::dsl; - diesel::insert_into(dsl::silo_user) - .values(silo_user) - .returning(SiloUser::as_returning()) - .get_result_async(self.pool()) + dsl::silo_user + .filter(dsl::id.eq(silo_user_id)) + .filter(dsl::time_deleted.is_null()) + .select(SiloUser::as_select()) + .first_async(self.pool()) .await .map_err(|e| { Error::internal_error(&format!( - "error creating silo user: {:?}", + "error fetching silo user: {:?}", e )) }) } - pub async fn silo_user_fetch_by_id( + pub async fn silo_user_create( &self, - id: Uuid, + silo_user: SiloUser, ) -> CreateResult { use db::schema::silo_user::dsl; - dsl::silo_user - .filter(dsl::id.eq(id)) - .filter(dsl::time_deleted.is_null()) - .select(SiloUser::as_select()) - .first_async(self.pool()) + diesel::insert_into(dsl::silo_user) + .values(silo_user) + .returning(SiloUser::as_returning()) + .get_result_async(self.pool()) .await .map_err(|e| { Error::internal_error(&format!( - "error fetching silo user: {:?}", + "error creating silo user: {:?}", e )) }) @@ -3482,9 +3479,10 @@ mod test { assert_eq!( silo_user.silo_id, datastore - .get_silo_id_from_silo_user_id(session.silo_user_id) + .silo_user_fetch(session.silo_user_id) .await - .unwrap(), + .unwrap() + .silo_id, ); // fetch the one we just created diff --git a/nexus/src/external_api/console_api.rs b/nexus/src/external_api/console_api.rs index 6e3a4cb73be..59395283682 100644 --- a/nexus/src/external_api/console_api.rs +++ b/nexus/src/external_api/console_api.rs @@ -69,12 +69,11 @@ pub async fn spoof_login( let user_id = user_id.unwrap(); - // look up silo id, make sure user is legal - if let Err(e) = nexus.get_silo_id_from_silo_user_id(user_id).await { - return Err(HttpError::for_internal_error(format!( - "no silo id found! {}", - e - ))); + if !nexus.login_allowed(user_id).await? { + return Ok(Response::builder() + .status(StatusCode::UNAUTHORIZED) + .header(header::SET_COOKIE, clear_session_cookie_header_value()) + .body("".into())?); // TODO: failed login response body? } let session = nexus.session_create(user_id).await?; diff --git a/nexus/src/nexus.rs b/nexus/src/nexus.rs index 504e170b8a2..ff030d61d10 100644 --- a/nexus/src/nexus.rs +++ b/nexus/src/nexus.rs @@ -2927,11 +2927,42 @@ impl Nexus { Ok(body) } - pub async fn get_silo_id_from_silo_user_id( + pub async fn login_allowed( &self, silo_user_id: Uuid, - ) -> LookupResult { - self.db_datastore.get_silo_id_from_silo_user_id(silo_user_id).await + ) -> Result { + // Was this silo user deleted? + let fetch_result = + self.db_datastore.silo_user_fetch(silo_user_id).await; + + match fetch_result { + Err(e) => { + match e { + Error::ObjectNotFound { type_name: _, lookup_type: _ } => { + // if the silo user was deleted, they're not allowed to + // log in :) + return Ok(false); + } + + _ => { + return Err(e); + } + } + } + + Ok(_) => { + // they're allowed + } + } + + Ok(true) + } + + pub async fn silo_user_fetch( + &self, + silo_user_id: Uuid, + ) -> LookupResult { + self.db_datastore.silo_user_fetch(silo_user_id).await } pub async fn silo_user_create( diff --git a/nexus/tests/integration_tests/silos.rs b/nexus/tests/integration_tests/silos.rs index 40743d939a0..ef9b2da1b93 100644 --- a/nexus/tests/integration_tests/silos.rs +++ b/nexus/tests/integration_tests/silos.rs @@ -129,7 +129,7 @@ async fn test_silos(cptestctx: &ControlPlaneTestContext) { // Verify silo user was also deleted nexus - .get_silo_id_from_silo_user_id(new_silo_user.id) + .silo_user_fetch(new_silo_user.id) .await .expect_err("unexpected success"); From 886a400b913be7b1a55bcff825c9ae8a65182e76 Mon Sep 17 00:00:00 2001 From: James MacMahon Date: Mon, 21 Mar 2022 16:57:07 -0400 Subject: [PATCH 13/24] private login_allowed, do not create session for now allowed users --- nexus/src/external_api/console_api.rs | 7 ------- nexus/src/nexus.rs | 12 ++++++++---- 2 files changed, 8 insertions(+), 11 deletions(-) diff --git a/nexus/src/external_api/console_api.rs b/nexus/src/external_api/console_api.rs index 59395283682..f3de02d8c67 100644 --- a/nexus/src/external_api/console_api.rs +++ b/nexus/src/external_api/console_api.rs @@ -69,13 +69,6 @@ pub async fn spoof_login( let user_id = user_id.unwrap(); - if !nexus.login_allowed(user_id).await? { - return Ok(Response::builder() - .status(StatusCode::UNAUTHORIZED) - .header(header::SET_COOKIE, clear_session_cookie_header_value()) - .body("".into())?); // TODO: failed login response body? - } - let session = nexus.session_create(user_id).await?; Ok(Response::builder() diff --git a/nexus/src/nexus.rs b/nexus/src/nexus.rs index ff030d61d10..25cd911caab 100644 --- a/nexus/src/nexus.rs +++ b/nexus/src/nexus.rs @@ -2681,8 +2681,15 @@ impl Nexus { &self, user_id: Uuid, ) -> CreateResult { + if !self.login_allowed(user_id).await? { + return Err(Error::Unauthenticated { + internal_message: "User not allowed to login".to_string(), + }); + } + let session = db::model::ConsoleSession::new(generate_session_token(), user_id); + Ok(self.db_datastore.session_create(session).await?) } @@ -2927,10 +2934,7 @@ impl Nexus { Ok(body) } - pub async fn login_allowed( - &self, - silo_user_id: Uuid, - ) -> Result { + async fn login_allowed(&self, silo_user_id: Uuid) -> Result { // Was this silo user deleted? let fetch_result = self.db_datastore.silo_user_fetch(silo_user_id).await; From b31ffa61154707e5651b83b9f5068fd02c4cbbae Mon Sep 17 00:00:00 2001 From: James MacMahon Date: Mon, 21 Mar 2022 17:04:29 -0400 Subject: [PATCH 14/24] TODO-security for silo_id code --- nexus/src/db/datastore.rs | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/nexus/src/db/datastore.rs b/nexus/src/db/datastore.rs index d73cbffa0e5..194349c62d9 100644 --- a/nexus/src/db/datastore.rs +++ b/nexus/src/db/datastore.rs @@ -598,13 +598,11 @@ impl DataStore { // doing an authz check. async fn organization_lookup_noauthz( &self, - // XXX enable when all endpoints are protected by authn - // opctx: &OpContext, name: &Name, ) -> LookupResult<(authz::Organization, Organization)> { use db::schema::organization::dsl; - // XXX uncomment when all endpoints are protected by authn + // TODO-security uncomment when all endpoints are protected by authn //let silo_id = opctx.authn.silo_required()?; let silo_id = *SILO_ID; From cf4c125e50ee7163cdd9389d390a2ae431b9d074 Mon Sep 17 00:00:00 2001 From: James MacMahon Date: Mon, 21 Mar 2022 17:18:37 -0400 Subject: [PATCH 15/24] do not return 500 in session_fetch --- common/src/api/external/error.rs | 3 +++ common/src/api/external/mod.rs | 1 + nexus/src/db/datastore.rs | 24 ++++++++++-------------- 3 files changed, 14 insertions(+), 14 deletions(-) diff --git a/common/src/api/external/error.rs b/common/src/api/external/error.rs index 6e4fcf77431..eaad54eca99 100644 --- a/common/src/api/external/error.rs +++ b/common/src/api/external/error.rs @@ -74,6 +74,8 @@ pub enum LookupType { ByName(String), /** a specific id was requested */ ById(Uuid), + /** a token was requested */ + ByToken(String), } impl LookupType { @@ -180,6 +182,7 @@ impl From for HttpError { let (lookup_field, lookup_value) = match lt { LookupType::ByName(name) => ("name", name), LookupType::ById(id) => ("id", id.to_string()), + LookupType::ByToken(token) => ("token", token), }; let message = format!( "not found: {} with {} \"{}\"", diff --git a/common/src/api/external/mod.rs b/common/src/api/external/mod.rs index 7837db9fa00..f2c241b0f2b 100644 --- a/common/src/api/external/mod.rs +++ b/common/src/api/external/mod.rs @@ -566,6 +566,7 @@ pub enum ResourceType { Fleet, Silo, SiloUser, + ConsoleSession, Organization, Project, Dataset, diff --git a/nexus/src/db/datastore.rs b/nexus/src/db/datastore.rs index 194349c62d9..7b621c3ef26 100644 --- a/nexus/src/db/datastore.rs +++ b/nexus/src/db/datastore.rs @@ -2693,21 +2693,17 @@ impl DataStore { .first_async(self.pool()) .await .map_err(|e| { - Error::internal_error(&format!( - "error fetching session: {:?}", - e - )) + public_error_from_diesel_pool( + e, + ErrorHandler::NotFoundByLookup( + ResourceType::ConsoleSession, + LookupType::ByToken(token), + ), + ) })?; - let silo_user = self - .silo_user_fetch(console_session.silo_user_id) - .await - .map_err(|e| { - Error::internal_error(&format!( - "error fetching silo id: {:?}", - e - )) - })?; + let silo_user = + self.silo_user_fetch(console_session.silo_user_id).await?; Ok(authn::ConsoleSessionWithSiloId { console_session, @@ -3515,7 +3511,7 @@ mod test { let fetched = datastore.session_fetch(token.clone()).await; assert!(matches!( fetched, - Err(Error::InternalError { internal_message: _ }) + Err(Error::ObjectNotFound { type_name: _, lookup_type: _ }) )); // deleting an already nonexistent is considered a success From 8bcaba3f6a8244b3c0fe16a7021be4c01d59704d Mon Sep 17 00:00:00 2001 From: James MacMahon Date: Mon, 21 Mar 2022 17:21:56 -0400 Subject: [PATCH 16/24] super nit :) --- nexus/tests/integration_tests/silos.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/nexus/tests/integration_tests/silos.rs b/nexus/tests/integration_tests/silos.rs index ef9b2da1b93..7734f0efed4 100644 --- a/nexus/tests/integration_tests/silos.rs +++ b/nexus/tests/integration_tests/silos.rs @@ -22,8 +22,8 @@ async fn test_silos(cptestctx: &ControlPlaneTestContext) { let nexus = &cptestctx.server.apictx.nexus; // Create two silos: one discoverable, one not - create_silo(&client, &"discoverable", true).await; - create_silo(&client, &"hidden", false).await; + create_silo(&client, "discoverable", true).await; + create_silo(&client, "hidden", false).await; // Verify GET /silos/{silo} works for both discoverable and not let discoverable_url = "/silos/discoverable"; From 6574e7985e26f148a400690f0f9436e0d710b693 Mon Sep 17 00:00:00 2001 From: James MacMahon Date: Mon, 21 Mar 2022 17:27:41 -0400 Subject: [PATCH 17/24] actor_required returns omicron_common::api::external::Error --- nexus/src/authn/mod.rs | 13 +++++++------ 1 file changed, 7 insertions(+), 6 deletions(-) diff --git a/nexus/src/authn/mod.rs b/nexus/src/authn/mod.rs index 3e5a95d1980..4e322226f4d 100644 --- a/nexus/src/authn/mod.rs +++ b/nexus/src/authn/mod.rs @@ -64,15 +64,16 @@ impl Context { } /// Returns the authenticated actor if present, Unauthenticated error otherwise - // TODO this should maybe return omicron_common::api::external::Error - pub fn actor_required(&self) -> Result<&Actor, dropshot::HttpError> { + pub fn actor_required( + &self, + ) -> Result<&Actor, omicron_common::api::external::Error> { match &self.kind { Kind::Authenticated(Details { actor }) => Ok(actor), - Kind::Unauthenticated => Err(dropshot::HttpError::from( - omicron_common::api::external::Error::Unauthenticated { + Kind::Unauthenticated => { + Err(omicron_common::api::external::Error::Unauthenticated { internal_message: "Actor required".to_string(), - }, - )), + }) + } } } From 4979abae908b742f2f5b34962c70b32702062f7b Mon Sep 17 00:00:00 2001 From: James MacMahon Date: Thu, 24 Mar 2022 13:57:17 -0400 Subject: [PATCH 18/24] drop obvious comment --- common/src/sql/dbinit.sql | 1 - 1 file changed, 1 deletion(-) diff --git a/common/src/sql/dbinit.sql b/common/src/sql/dbinit.sql index 3d888b65829..0a018beb130 100644 --- a/common/src/sql/dbinit.sql +++ b/common/src/sql/dbinit.sql @@ -751,7 +751,6 @@ CREATE TABLE omicron.public.console_session ( token STRING(40) PRIMARY KEY, time_created TIMESTAMPTZ NOT NULL, time_last_used TIMESTAMPTZ NOT NULL, - -- this maps to id in the silo_user table silo_user_id UUID NOT NULL ); From 51b9876a3910529aa14146e427928380200c0eaa Mon Sep 17 00:00:00 2001 From: James MacMahon Date: Thu, 24 Mar 2022 14:00:19 -0400 Subject: [PATCH 19/24] silo_required in terms of actor_required --- nexus/src/authn/mod.rs | 9 +-------- 1 file changed, 1 insertion(+), 8 deletions(-) diff --git a/nexus/src/authn/mod.rs b/nexus/src/authn/mod.rs index 4e322226f4d..9cc867c9839 100644 --- a/nexus/src/authn/mod.rs +++ b/nexus/src/authn/mod.rs @@ -80,14 +80,7 @@ impl Context { pub fn silo_required( &self, ) -> Result { - match self.actor_required() { - Ok(actor) => Ok(actor.silo_id), - Err(_) => { - Err(omicron_common::api::external::Error::Unauthenticated { - internal_message: "Actor required".to_string(), - }) - } - } + self.actor_required().map(|actor| actor.silo_id) } /// Returns the list of schemes tried, in order From 464278335de745dddddf926f7a77ef49117b7c31 Mon Sep 17 00:00:00 2001 From: James MacMahon Date: Thu, 24 Mar 2022 14:06:05 -0400 Subject: [PATCH 20/24] remove objects_list_page_authz_with_session --- nexus/test-utils/src/resource_helpers.rs | 17 ----------------- 1 file changed, 17 deletions(-) diff --git a/nexus/test-utils/src/resource_helpers.rs b/nexus/test-utils/src/resource_helpers.rs index 9e96e5eff5e..0341213705c 100644 --- a/nexus/test-utils/src/resource_helpers.rs +++ b/nexus/test-utils/src/resource_helpers.rs @@ -40,23 +40,6 @@ where .unwrap() } -pub async fn objects_list_page_authz_with_session( - client: &ClientTestContext, - path: &str, - session_token: &str, -) -> dropshot::ResultsPage -where - ItemType: serde::de::DeserializeOwned, -{ - NexusRequest::object_get(client, path) - .authn_with_session(session_token) - .execute() - .await - .expect("failed to make request") - .parsed_body() - .unwrap() -} - pub async fn object_create( client: &ClientTestContext, path: &str, From 0dd7903c037c06b694c80950d0d1ab1657f4ba61 Mon Sep 17 00:00:00 2001 From: James MacMahon Date: Thu, 24 Mar 2022 14:15:20 -0400 Subject: [PATCH 21/24] remove authn_with_session, replace with AuthnMode::Session --- nexus/test-utils/src/http_testing.rs | 40 +++++++++++++++++----------- 1 file changed, 25 insertions(+), 15 deletions(-) diff --git a/nexus/test-utils/src/http_testing.rs b/nexus/test-utils/src/http_testing.rs index cb54c478887..79112e46550 100644 --- a/nexus/test-utils/src/http_testing.rs +++ b/nexus/test-utils/src/http_testing.rs @@ -404,6 +404,7 @@ impl TestResponse { pub enum AuthnMode { UnprivilegedUser, PrivilegedUser, + Session(String), } /// Helper for constructing requests to Nexus's external API @@ -432,23 +433,32 @@ impl<'a> NexusRequest<'a> { /// `mode` pub fn authn_as(mut self, mode: AuthnMode) -> Self { use omicron_nexus::authn; - let header_value = match mode { - AuthnMode::UnprivilegedUser => authn::USER_TEST_UNPRIVILEGED.id, - AuthnMode::PrivilegedUser => authn::USER_TEST_PRIVILEGED.id, - }; - self.request_builder = self.request_builder.header( - &http::header::AUTHORIZATION, - spoof::make_header_value(header_value).0.encode(), - ); - self - } + match mode { + AuthnMode::UnprivilegedUser | AuthnMode::PrivilegedUser => { + let header_value = match mode { + AuthnMode::UnprivilegedUser => { + authn::USER_TEST_UNPRIVILEGED.id + } + AuthnMode::PrivilegedUser => authn::USER_TEST_PRIVILEGED.id, + _ => { + panic!("unreachable!") + } + }; + + self.request_builder = self.request_builder.header( + &http::header::AUTHORIZATION, + spoof::make_header_value(header_value).0.encode(), + ); + } + AuthnMode::Session(session_token) => { + self.request_builder = self.request_builder.header( + &http::header::COOKIE, + format!("session={}", session_token), + ); + } + } - pub fn authn_with_session(mut self, session_token: &str) -> Self { - self.request_builder = self.request_builder.header( - &http::header::COOKIE, - format!("session={}", session_token), - ); self } From 2497a8d7bfde3cb667f886f6ff7ef124b95aef80 Mon Sep 17 00:00:00 2001 From: James MacMahon Date: Thu, 24 Mar 2022 14:25:47 -0400 Subject: [PATCH 22/24] move silo_user_create into TestInterfaces --- nexus/src/nexus.rs | 24 +++++++++++++++--------- nexus/tests/integration_tests/silos.rs | 1 + 2 files changed, 16 insertions(+), 9 deletions(-) diff --git a/nexus/src/nexus.rs b/nexus/src/nexus.rs index 25cd911caab..a6d285ee304 100644 --- a/nexus/src/nexus.rs +++ b/nexus/src/nexus.rs @@ -109,6 +109,12 @@ pub trait TestInterfaces { &self, session: db::model::ConsoleSession, ) -> CreateResult; + + async fn silo_user_create( + &self, + silo_id: Uuid, + silo_user_id: Uuid, + ) -> CreateResult; } pub static BASE_ARTIFACT_DIR: &str = "/var/tmp/oxide_artifacts"; @@ -2968,15 +2974,6 @@ impl Nexus { ) -> LookupResult { self.db_datastore.silo_user_fetch(silo_user_id).await } - - pub async fn silo_user_create( - &self, - silo_id: Uuid, - silo_user_id: Uuid, - ) -> CreateResult { - let silo_user = SiloUser::new(silo_id, silo_user_id); - Ok(self.db_datastore.silo_user_create(silo_user).await?) - } } fn generate_session_token() -> String { @@ -3034,4 +3031,13 @@ impl TestInterfaces for Nexus { ) -> CreateResult { Ok(self.db_datastore.session_create(session).await?) } + + async fn silo_user_create( + &self, + silo_id: Uuid, + silo_user_id: Uuid, + ) -> CreateResult { + let silo_user = SiloUser::new(silo_id, silo_user_id); + Ok(self.db_datastore.silo_user_create(silo_user).await?) + } } diff --git a/nexus/tests/integration_tests/silos.rs b/nexus/tests/integration_tests/silos.rs index 7734f0efed4..9dba23461de 100644 --- a/nexus/tests/integration_tests/silos.rs +++ b/nexus/tests/integration_tests/silos.rs @@ -6,6 +6,7 @@ use uuid::Uuid; use nexus_test_utils::http_testing::{AuthnMode, NexusRequest}; use omicron_nexus::external_api::views::{Organization, Silo}; +use omicron_nexus::TestInterfaces as _; use http::method::Method; use http::StatusCode; From 6cf24305759af119aae868a73d24a9b138cc960f Mon Sep 17 00:00:00 2001 From: James MacMahon Date: Thu, 24 Mar 2022 14:37:31 -0400 Subject: [PATCH 23/24] non-existent silo is an internal error --- nexus/src/db/datastore.rs | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/nexus/src/db/datastore.rs b/nexus/src/db/datastore.rs index 7b621c3ef26..e0ab006a10a 100644 --- a/nexus/src/db/datastore.rs +++ b/nexus/src/db/datastore.rs @@ -554,9 +554,8 @@ impl DataStore { .insert_and_get_result_async(self.pool_authorized(opctx).await?) .await .map_err(|e| match e { - AsyncInsertError::CollectionNotFound => Error::ObjectNotFound { - type_name: ResourceType::Silo, - lookup_type: LookupType::ById(silo_id), + AsyncInsertError::CollectionNotFound => Error::InternalError { + internal_message: format!("attempting to create an organization under non-existent silo {}", silo_id), }, AsyncInsertError::DatabaseError(e) => { public_error_from_diesel_pool( From 2d24c6ebd3364441874944ab3c1eeb98a09a1581 Mon Sep 17 00:00:00 2001 From: James MacMahon Date: Thu, 24 Mar 2022 14:44:22 -0400 Subject: [PATCH 24/24] ByToken -> BySessionToken --- common/src/api/external/error.rs | 8 +++++--- nexus/src/db/datastore.rs | 2 +- 2 files changed, 6 insertions(+), 4 deletions(-) diff --git a/common/src/api/external/error.rs b/common/src/api/external/error.rs index eaad54eca99..2e8343a22c6 100644 --- a/common/src/api/external/error.rs +++ b/common/src/api/external/error.rs @@ -74,8 +74,8 @@ pub enum LookupType { ByName(String), /** a specific id was requested */ ById(Uuid), - /** a token was requested */ - ByToken(String), + /** a session token was requested */ + BySessionToken(String), } impl LookupType { @@ -182,7 +182,9 @@ impl From for HttpError { let (lookup_field, lookup_value) = match lt { LookupType::ByName(name) => ("name", name), LookupType::ById(id) => ("id", id.to_string()), - LookupType::ByToken(token) => ("token", token), + LookupType::BySessionToken(token) => { + ("session token", token) + } }; let message = format!( "not found: {} with {} \"{}\"", diff --git a/nexus/src/db/datastore.rs b/nexus/src/db/datastore.rs index e0ab006a10a..500ce51775a 100644 --- a/nexus/src/db/datastore.rs +++ b/nexus/src/db/datastore.rs @@ -2696,7 +2696,7 @@ impl DataStore { e, ErrorHandler::NotFoundByLookup( ResourceType::ConsoleSession, - LookupType::ByToken(token), + LookupType::BySessionToken(token), ), ) })?;