diff --git a/Cargo.lock b/Cargo.lock index d76b85326df..e5694a83e53 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -802,8 +802,11 @@ checksum = "3ee2393c4a91429dffb4bedf19f4d6abf27d8a732c8ce4980305d782e5426d57" name = "db-macros" version = "0.1.0" dependencies = [ + "heck 0.4.0", "proc-macro2", "quote", + "serde", + "serde_tokenstream", "syn", ] diff --git a/nexus/src/authz/api_resources.rs b/nexus/src/authz/api_resources.rs index 64d12ea2186..6e731825635 100644 --- a/nexus/src/authz/api_resources.rs +++ b/nexus/src/authz/api_resources.rs @@ -421,6 +421,10 @@ impl Project { lookup_type, } } + + pub fn organization(&self) -> &Organization { + &self.parent + } } impl Eq for Project {} diff --git a/nexus/src/db/datastore.rs b/nexus/src/db/datastore.rs index e26d30efcca..641c2f181ee 100644 --- a/nexus/src/db/datastore.rs +++ b/nexus/src/db/datastore.rs @@ -119,11 +119,11 @@ impl DataStore { // the database. Eventually, this function should only be used for doing // authentication in the first place (since we can't do an authz check in // that case). - fn pool(&self) -> &bb8::Pool> { + pub(super) fn pool(&self) -> &bb8::Pool> { self.pool.pool() } - async fn pool_authorized( + pub(super) async fn pool_authorized( &self, opctx: &OpContext, ) -> Result<&bb8::Pool>, Error> { diff --git a/nexus/src/db/db-macros/Cargo.toml b/nexus/src/db/db-macros/Cargo.toml index 2d0cce7eaf5..0031ff2f465 100644 --- a/nexus/src/db/db-macros/Cargo.toml +++ b/nexus/src/db/db-macros/Cargo.toml @@ -9,6 +9,9 @@ license = "MPL-2.0" proc-macro = true [dependencies] +heck = "0.4" proc-macro2 = { version = "1.0" } quote = { version = "1.0" } +serde = { version = "1.0", features = [ "derive" ] } +serde_tokenstream = "0.1" syn = { version = "1.0", features = [ "full", "derive", "extra-traits" ] } diff --git a/nexus/src/db/db-macros/src/lib.rs b/nexus/src/db/db-macros/src/lib.rs index 3b29099c5dc..534aa0ddbb6 100644 --- a/nexus/src/db/db-macros/src/lib.rs +++ b/nexus/src/db/db-macros/src/lib.rs @@ -18,6 +18,62 @@ use quote::{format_ident, quote}; use syn::spanned::Spanned; use syn::{Data, DataStruct, DeriveInput, Error, Fields, Ident, Lit, Meta}; +mod lookup; + +/// Defines a structure and helper functions for looking up resources +/// +/// # Examples +/// +/// ```ignore +/// lookup_resource! { +/// name = "Organization", +/// ancestors = [], +/// children = [ "Project" ], +/// authz_kind = Typed +/// } +/// ``` +/// +/// See [`lookup::Input`] for documentation on the named arguments. +/// +/// This defines a struct `Organization<'a>` with functions `fetch()`, +/// `fetch_for(authz::Action)`, and `lookup_for(authz::Action)` for looking up +/// an Organization in the database. These functions are all protected by +/// access controls. +/// +/// Building on that, we have: +/// +/// ```ignore +/// lookup_resource! { +/// name = "Organization", +/// ancestors = [], +/// children = [ "Project" ], +/// authz_kind = Typed +/// } +/// +/// lookup_resource! { +/// name = "Instance", +/// ancestors = [ "Organization", "Project" ], +/// children = [], +/// authz_kind = Generic +/// } +/// ``` +/// +/// These define `Project<'a>` and `Instance<'a>`. For more on these structs +/// and how they're used, see nexus/src/db/lookup.rs. +// Allow private intra-doc links. This is useful because the `Input` struct +// cannot be exported (since we're a proc macro crate, and we can't expose +// a struct), but its documentation is very useful. +#[allow(rustdoc::private_intra_doc_links)] +#[proc_macro] +pub fn lookup_resource( + input: proc_macro::TokenStream, +) -> proc_macro::TokenStream { + match lookup::lookup_resource(input.into()) { + Ok(output) => output.into(), + Err(error) => error.to_compile_error().into(), + } +} + /// Looks for a Meta-style attribute with a particular identifier. /// /// As an example, for an attribute like `#[foo = "bar"]`, we can find this diff --git a/nexus/src/db/db-macros/src/lookup.rs b/nexus/src/db/db-macros/src/lookup.rs new file mode 100644 index 00000000000..0acceef5031 --- /dev/null +++ b/nexus/src/db/db-macros/src/lookup.rs @@ -0,0 +1,659 @@ +// 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/. + +//! Procedure macro for generating lookup structures and related functions +//! +//! See nexus/src/db/lookup.rs. + +use proc_macro2::TokenStream; +use quote::{format_ident, quote}; + +// +// INPUT (arguments to the macro) +// + +/// Arguments for [`lookup_resource!`] +// NOTE: this is only "pub" for the `cargo doc` link on [`lookup_resource!`]. +#[derive(serde::Deserialize)] +pub struct Input { + /// Name of the resource + /// + /// This is taken as the name of the database model type in + /// `omicron_nexus::db::model`, the name of the authz type in + /// `omicron_nexus::authz`, and will be the name of the new type created by + /// this macro. The snake case version of the name is taken as the name of + /// the Diesel table interface in `db::schema`. + /// + /// This value is typically PascalCase (e.g., "Organization"). + name: String, + /// ordered list of resources that are ancestors of this resource, starting + /// with the top of the hierarchy + /// (e.g., for an Instance, this would be `[ "Organization", "Project" ]` + ancestors: Vec, + /// unordered list of resources that are direct children of this resource + /// (e.g., for a Project, these would include "Instance" and "Disk") + children: Vec, + /// describes how the authz object for a resource is constructed from its + /// parent's authz object + authz_kind: AuthzKind, +} + +/// Describes how the authz object for a resource is constructed from its +/// parent's authz object +/// +/// By "authz object", we mean the objects in nexus/src/authz/api_resources.rs. +/// +/// This ought to be made more uniform with more typed authz objects, but that's +/// not the way they work today. +#[derive(serde::Deserialize)] +enum AuthzKind { + /// The authz object is constructed using + /// `authz_parent.child_generic(ResourceType, Uuid, LookupType)` + Generic, + + /// The authz object is constructed using + /// `authz_parent.$resource_type(Uuid, LookupType)`. + Typed, +} + +// +// MACRO STATE +// + +/// Configuration for [`lookup_resource()`] and its helper functions +/// +/// This is all computable from [`Input`]. This precomputes a bunch of useful +/// identifiers and token streams, which makes the generator functions a lot +/// easier to grok. +pub struct Config { + // The resource itself that we're generating + /// Basic information about the resource we're generating + resource: Resource, + + /// See [`AuthzKind`] + authz_kind: AuthzKind, + + // The path to the resource + /// list of type names for this resource and its parents + /// (e.g., [`Organization`, `Project`]) + path_types: Vec, + + /// list of identifiers used for the authz objects for this resource and its + /// parents, in the same order as `authz_path_types` + /// (e.g., [`authz_organization`, `authz_project`]) + path_authz_names: Vec, + + // Child resources + /// list of names of child resources, in the same form and with the same + /// assumptions as [`Input::name`] (i.e., typically PascalCase) + child_resources: Vec, + + // Parent resource, if any + /// Information about the parent resource, if any + parent: Option, +} + +impl Config { + fn for_input(input: Input) -> Config { + let resource = Resource::for_name(&input.name); + + let mut path_types: Vec<_> = + input.ancestors.iter().map(|a| format_ident!("{}", a)).collect(); + path_types.push(resource.name.clone()); + + let mut path_authz_names: Vec<_> = input + .ancestors + .iter() + .map(|a| { + format_ident!("authz_{}", heck::AsSnakeCase(&a).to_string()) + }) + .collect(); + path_authz_names.push(resource.authz_name.clone()); + + Config { + resource, + authz_kind: input.authz_kind, + path_types, + path_authz_names, + parent: input.ancestors.last().map(|s| Resource::for_name(&s)), + child_resources: input.children, + } + } +} + +/// Information about a resource (either the one we're generating or an +/// ancestor in its path) +struct Resource { + /// PascalCase resource name itself (e.g., `Project`) + /// + /// See [`Input::name`] for more on the assumptions and how it's used. + name: syn::Ident, + /// snake_case resource name (e.g., `project`) + name_as_snake: String, + /// identifier for an authz object for this resource (e.g., `authz_project`) + authz_name: syn::Ident, +} + +impl Resource { + fn for_name(name: &str) -> Resource { + let name_as_snake = heck::AsSnakeCase(&name).to_string(); + let name = format_ident!("{}", name); + let authz_name = format_ident!("authz_{}", name_as_snake); + Resource { name, authz_name, name_as_snake } + } +} + +// +// MACRO IMPLEMENTATION +// + +/// Implementation of [`lookup_resource!]'. +pub fn lookup_resource( + raw_input: TokenStream, +) -> Result { + let input = serde_tokenstream::from_tokenstream::(&raw_input)?; + let config = Config::for_input(input); + + let resource_name = &config.resource.name; + let the_struct = generate_struct(&config); + let misc_helpers = generate_misc_helpers(&config); + let child_selectors = generate_child_selectors(&config); + let lookup_methods = generate_lookup_methods(&config); + let database_functions = generate_database_functions(&config); + + Ok(quote! { + #the_struct + + impl<'a> #resource_name<'a> { + #child_selectors + + #lookup_methods + + #misc_helpers + + #database_functions + } + }) +} + +/// Generates the struct definition for this resource +fn generate_struct(config: &Config) -> TokenStream { + let root_sym = format_ident!("Root"); + let resource_name = &config.resource.name; + let parent_resource_name = + config.parent.as_ref().map(|p| &p.name).unwrap_or_else(|| &root_sym); + let doc_struct = format!( + "Selects a resource of type {} (or any of its children, using the \ + functions on this struct) for lookup or fetch", + resource_name + ); + + quote! { + #[doc = #doc_struct] + pub struct #resource_name<'a> { + key: Key<'a, #parent_resource_name<'a>> + } + } +} + +/// Generates the child selectors for this resource +/// +/// For example, for the "Project" resource with child resources "Instance" and +/// "Disk", this will generate the `Project::instance_name()` and +/// `Project::disk_name()` functions. +fn generate_child_selectors(config: &Config) -> TokenStream { + let child_resource_types: Vec<_> = + config.child_resources.iter().map(|c| format_ident!("{}", c)).collect(); + let child_selector_fn_names: Vec<_> = config + .child_resources + .iter() + .map(|c| format_ident!("{}_name", heck::AsSnakeCase(c).to_string())) + .collect(); + let child_selector_fn_docs: Vec<_> = config + .child_resources + .iter() + .map(|child| { + format!( + "Select a resource of type {} within this {}, \ + identified by its name", + child, config.resource.name, + ) + }) + .collect(); + + quote! { + #( + #[doc = #child_selector_fn_docs] + pub fn #child_selector_fn_names<'b, 'c>( + self, + name: &'b Name + ) -> #child_resource_types<'c> + where + 'a: 'c, + 'b: 'c, + { + #child_resource_types { + key: Key::Name(self, name), + } + } + )* + } +} + +/// Generates the simple helper functions for this resource +fn generate_misc_helpers(config: &Config) -> TokenStream { + let fleet_name = format_ident!("Fleet"); + let resource_name = &config.resource.name; + let parent_resource_name = + config.parent.as_ref().map(|p| &p.name).unwrap_or(&fleet_name); + + // Given a parent authz type, when we want to construct an authz object for + // a child resource, there are two different patterns. We need to pick the + // right one. For "typed" resources (`AuthzKind::Typed`), the parent + // resource has a method with the snake case name of the child resource. + // For example: `authz_organization.project()`. For "generic" resources + // (`AuthzKind::Generic`), the parent has a function called `child_generic` + // that's used to construct all child resources, and there's an extra + // `ResourceType` argument to say what resource it is. + let (mkauthz_func, mkauthz_arg) = match &config.authz_kind { + AuthzKind::Generic => ( + format_ident!("child_generic"), + quote! { ResourceType::#resource_name, }, + ), + AuthzKind::Typed => { + (format_ident!("{}", config.resource.name_as_snake), quote! {}) + } + }; + + quote! { + /// Build the `authz` object for this resource + fn make_authz( + authz_parent: &authz::#parent_resource_name, + db_row: &model::#resource_name, + lookup_type: LookupType, + ) -> authz::#resource_name { + authz_parent.#mkauthz_func( + #mkauthz_arg + db_row.id(), + lookup_type + ) + } + + /// Getting the [`LookupPath`] for this lookup + /// + /// This is used when we actually query the database. At that + /// point, we need the `OpContext` and `DataStore` that are being + /// used for this lookup. + fn lookup_root(&self) -> &LookupPath<'a> { + match &self.key { + Key::Name(parent, _) => parent.lookup_root(), + Key::Id(root, _) => root.lookup_root(), + } + } + } +} + +/// Generates the lookup-related methods, including the public ones (`fetch()`, +/// `fetch_for()`, and `lookup_for()`) and the private helper (`lookup()`). +fn generate_lookup_methods(config: &Config) -> TokenStream { + let path_types = &config.path_types; + let path_authz_names = &config.path_authz_names; + let resource_name = &config.resource.name; + let resource_authz_name = &config.resource.authz_name; + let (ancestors_authz_names_assign, parent_lookup_arg_actual) = + if let Some(p) = &config.parent { + let nancestors = config.path_authz_names.len() - 1; + let ancestors_authz_names = &config.path_authz_names[0..nancestors]; + let parent_authz_name = &p.authz_name; + ( + quote! { + let (#(#ancestors_authz_names,)*) = parent.lookup().await?; + }, + quote! { &#parent_authz_name, }, + ) + } else { + (quote! {}, quote! {}) + }; + + quote! { + /// Fetch the record corresponding to the selected resource + /// + /// This is equivalent to `fetch_for(authz::Action::Read)`. + pub async fn fetch( + &self, + ) -> LookupResult<(#(authz::#path_types,)* model::#resource_name)> { + self.fetch_for(authz::Action::Read).await + } + + /// Fetch the record corresponding to the selected resource and + /// check whether the caller is allowed to do the specified `action` + /// + /// The return value is a tuple that also includes the `authz` + /// objects for all resources along the path to this one (i.e., all + /// parent resources) and the authz object for this resource itself. + /// These objects are useful for identifying those resources by + /// id, for doing other authz checks, or for looking up related + /// objects. + pub async fn fetch_for( + &self, + action: authz::Action, + ) -> LookupResult<(#(authz::#path_types,)* model::#resource_name)> { + let lookup = self.lookup_root(); + let opctx = &lookup.opctx; + let datastore = &lookup.datastore; + + match &self.key { + Key::Name(parent, name) => { + #ancestors_authz_names_assign + let (#resource_authz_name, db_row) = Self::fetch_by_name_for( + opctx, + datastore, + #parent_lookup_arg_actual + *name, + action, + ).await?; + Ok((#(#path_authz_names,)* db_row)) + } + Key::Id(_, id) => { + Self::fetch_by_id_for( + opctx, + datastore, + *id, + action, + ).await + } + } + } + + /// Fetch an `authz` object for the selected resource and check + /// whether the caller is allowed to do the specified `action` + /// + /// The return value is a tuple that also includes the `authz` + /// objects for all resources along the path to this one (i.e., all + /// parent resources) and the authz object for this resource itself. + /// These objects are useful for identifying those resources by + /// id, for doing other authz checks, or for looking up related + /// objects. + pub async fn lookup_for( + &self, + action: authz::Action, + ) -> LookupResult<(#(authz::#path_types,)*)> { + let lookup = self.lookup_root(); + let opctx = &lookup.opctx; + let (#(#path_authz_names,)*) = self.lookup().await?; + opctx.authorize(action, &#resource_authz_name).await?; + Ok((#(#path_authz_names,)*)) + } + + /// Fetch the "authz" objects for the selected resource and all its + /// parents + /// + /// This function does not check whether the caller has permission + /// to read this information. That's why it's not `pub`. Outside + /// this module, you want `lookup_for(authz::Action)`. + // Do NOT make this function public. It's a helper for fetch() and + // lookup_for(). It's exposed in a safer way via lookup_for(). + async fn lookup( + &self, + ) -> LookupResult<(#(authz::#path_types,)*)> { + let lookup = self.lookup_root(); + let opctx = &lookup.opctx; + let datastore = &lookup.datastore; + + match &self.key { + Key::Name(parent, name) => { + // When doing a by-name lookup, we have to look up the + // parent first. Since this is recursive, we wind up + // hitting the database once for each item in the path, + // in order descending from the root of the tree. (So + // we'll look up Organization, then Project, then + // Instance, etc.) + // TODO-performance Instead of doing database queries at + // each level of recursion, we could be building up one + // big "join" query and hit the database just once. + #ancestors_authz_names_assign + let (#resource_authz_name, _) = + Self::lookup_by_name_no_authz( + opctx, + datastore, + #parent_lookup_arg_actual + *name + ).await?; + Ok((#(#path_authz_names,)*)) + } + Key::Id(_, id) => { + // When doing a by-id lookup, we start directly with the + // resource we're looking up. But we still want to + // return a full path of authz objects. So we look up + // the parent by id, then its parent, etc. Like the + // by-name case, we wind up hitting the database once + // for each item in the path, but in the reverse order. + // So we'll look up the Instance, then the Project, then + // the Organization. + // TODO-performance Instead of doing database queries at + // each level of recursion, we could be building up one + // big "join" query and hit the database just once. + let (#(#path_authz_names,)* _) = + Self::lookup_by_id_no_authz( + opctx, + datastore, + *id + ).await?; + Ok((#(#path_authz_names,)*)) + } + } + } + } +} + +/// Generates low-level functions to fetch database records for this resource +/// +/// These are standalone functions, not methods. They operate on more primitive +/// objects than do the methods: authz objects (representing parent ids), names, +/// and ids. They also take the `opctx` and `datastore` directly as arguments. +fn generate_database_functions(config: &Config) -> TokenStream { + let resource_name = &config.resource.name; + let resource_authz_name = &config.resource.authz_name; + let resource_as_snake = format_ident!("{}", &config.resource.name_as_snake); + let path_types = &config.path_types; + let path_authz_names = &config.path_authz_names; + let ( + parent_lookup_arg_formal, + parent_lookup_arg_actual, + ancestors_authz_names_assign, + lookup_filter, + parent_authz_value, + ) = if let Some(p) = &config.parent { + let nancestors = config.path_authz_names.len() - 1; + let ancestors_authz_names = &config.path_authz_names[0..nancestors]; + let parent_resource_name = &p.name; + let parent_authz_name = &p.authz_name; + let parent_id = format_ident!("{}_id", &p.name_as_snake); + ( + quote! { #parent_authz_name: &authz::#parent_resource_name, }, + quote! { #parent_authz_name, }, + quote! { + let (#(#ancestors_authz_names,)* _) = + #parent_resource_name::lookup_by_id_no_authz( + _opctx, datastore, db_row.#parent_id + ).await?; + }, + quote! { .filter(dsl::#parent_id.eq(#parent_authz_name.id())) }, + quote! { #parent_authz_name }, + ) + } else { + (quote! {}, quote! {}, quote! {}, quote! {}, quote! { &authz::FLEET }) + }; + + quote! { + /// Fetch the database row for a resource by doing a lookup by name, + /// possibly within a collection + /// + /// This function checks whether the caller has permissions to read + /// the requested data. However, it's not intended to be used + /// outside this module. See `fetch_for(authz::Action)`. + // Do NOT make this function public. + async fn fetch_by_name_for( + opctx: &OpContext, + datastore: &DataStore, + #parent_lookup_arg_formal + name: &Name, + action: authz::Action, + ) -> LookupResult<(authz::#resource_name, model::#resource_name)> { + let (#resource_authz_name, db_row) = Self::lookup_by_name_no_authz( + opctx, + datastore, + #parent_lookup_arg_actual + name + ).await?; + opctx.authorize(action, &#resource_authz_name).await?; + Ok((#resource_authz_name, db_row)) + } + + /// Lowest-level function for looking up a resource in the database + /// by name, possibly within a collection + /// + /// This function does not check whether the caller has permission + /// to read this information. That's why it's not `pub`. Outside + /// this module, you want `fetch()` or `lookup_for(authz::Action)`. + // Do NOT make this function public. + async fn lookup_by_name_no_authz( + _opctx: &OpContext, + datastore: &DataStore, + #parent_lookup_arg_formal + name: &Name, + ) -> LookupResult<(authz::#resource_name, model::#resource_name)> { + use db::schema::#resource_as_snake::dsl; + + // TODO-security See the note about pool_authorized() below. + let conn = datastore.pool(); + dsl::#resource_as_snake + .filter(dsl::time_deleted.is_null()) + .filter(dsl::name.eq(name.clone())) + #lookup_filter + .select(model::#resource_name::as_select()) + .get_result_async(conn) + .await + .map_err(|e| { + public_error_from_diesel_pool( + e, + ErrorHandler::NotFoundByLookup( + ResourceType::#resource_name, + LookupType::ByName(name.as_str().to_string()) + ) + ) + }) + .map(|db_row| {( + Self::make_authz( + #parent_authz_value, + &db_row, + LookupType::ByName(name.as_str().to_string()) + ), + db_row + )}) + } + + /// Fetch the database row for a resource by doing a lookup by id + /// + /// This function checks whether the caller has permissions to read + /// the requested data. However, it's not intended to be used + /// outside this module. See `fetch_for(authz::Action)`. + // Do NOT make this function public. + async fn fetch_by_id_for( + opctx: &OpContext, + datastore: &DataStore, + id: Uuid, + action: authz::Action, + ) -> LookupResult<(#(authz::#path_types,)* model::#resource_name)> { + let (#(#path_authz_names,)* db_row) = + Self::lookup_by_id_no_authz( + opctx, + datastore, + id + ).await?; + opctx.authorize(action, &#resource_authz_name).await?; + Ok((#(#path_authz_names,)* db_row)) + } + + /// Lowest-level function for looking up a resource in the database + /// by id + /// + /// This function does not check whether the caller has permission + /// to read this information. That's why it's not `pub`. Outside + /// this module, you want `fetch()` or `lookup_for(authz::Action)`. + // Do NOT make this function public. + async fn lookup_by_id_no_authz( + _opctx: &OpContext, + datastore: &DataStore, + id: Uuid, + ) -> LookupResult<(#(authz::#path_types,)* model::#resource_name)> { + use db::schema::#resource_as_snake::dsl; + + // TODO-security This could use pool_authorized() instead. + // However, it will change the response code for this case: + // unauthenticated users will get a 401 rather than a 404 + // because we'll kick them out sooner than we used to -- they + // won't even be able to make this database query. That's a + // good thing but this change can be deferred to a follow-up PR. + let conn = datastore.pool(); + let db_row = dsl::#resource_as_snake + .filter(dsl::time_deleted.is_null()) + .filter(dsl::id.eq(id)) + .select(model::#resource_name::as_select()) + .get_result_async(conn) + .await + .map_err(|e| { + public_error_from_diesel_pool( + e, + ErrorHandler::NotFoundByLookup( + ResourceType::#resource_name, + LookupType::ById(id) + ) + ) + })?; + #ancestors_authz_names_assign + let #resource_authz_name = Self::make_authz( + &#parent_authz_value, + &db_row, + LookupType::ById(id) + ); + Ok((#(#path_authz_names,)* db_row)) + } + } +} + +// This isn't so much a test (although it does make sure we don't panic on some +// basic cases). This is a way to dump the output of the macro for some common +// inputs. This is invaluable for debugging. If there's a bug where the macro +// generates syntactically invalid Rust, `cargo expand` will often not print the +// macro's output. Instead, you can paste the output of this test into +// lookup.rs, replacing the call to the macro, then reformat the file, and then +// build it in order to see the compiler error in context. +#[cfg(test)] +#[test] +fn test_lookup_dump() { + let output = lookup_resource( + quote! { + name = "Organization", + ancestors = [], + children = [ "Project" ], + authz_kind = Typed + } + .into(), + ) + .unwrap(); + println!("{}", output); + + let output = lookup_resource( + quote! { + name = "Project", + ancestors = ["Organization"], + children = [ "Disk", "Instance" ], + authz_kind = Typed + } + .into(), + ) + .unwrap(); + println!("{}", output); +} diff --git a/nexus/src/db/lookup.rs b/nexus/src/db/lookup.rs new file mode 100644 index 00000000000..379401d0166 --- /dev/null +++ b/nexus/src/db/lookup.rs @@ -0,0 +1,317 @@ +// 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/. + +//! Look up API resources from the database + +use super::datastore::DataStore; +use super::identity::Resource; +use super::model; +use crate::{ + authz::{self}, + context::OpContext, + db, + db::error::{public_error_from_diesel_pool, ErrorHandler}, + db::model::Name, +}; +use async_bb8_diesel::AsyncRunQueryDsl; +use db_macros::lookup_resource; +use diesel::{ExpressionMethods, QueryDsl, SelectableHelper}; +use omicron_common::api::external::{LookupResult, LookupType, ResourceType}; +use uuid::Uuid; + +/// Look up an API resource in the database +/// +/// `LookupPath` provides a builder-like interface for identifying a resource by +/// id or a path of names. Once you've selected a resource, you can use one of +/// a few different functions to get information about it from the database: +/// +/// * `fetch()`: fetches the database record and `authz` objects for all parents +/// in the path to this object. This function checks that the caller has +/// permission to `authz::Action::Read` the resoure. +/// * `fetch_for(authz::Action)`: like `fetch()`, but allows you to specify some +/// other action that will be checked rather than `authz::Action::Read`. +/// * `lookup_for(authz::Action)`: fetch just the `authz` objects for a resource +/// and its parents. This function checks that the caller has permissions to +/// perform the specified action. +/// +/// # Examples +/// +/// ``` +/// # use omicron_nexus::authz; +/// # use omicron_nexus::context::OpContext; +/// # use omicron_nexus::db; +/// # use omicron_nexus::db::DataStore; +/// # use omicron_nexus::db::lookup::LookupPath; +/// # use uuid::Uuid; +/// # async fn foo(opctx: &OpContext, datastore: &DataStore) +/// # -> Result<(), omicron_common::api::external::Error> { +/// +/// // Fetch an organization by name +/// let organization_name = db::model::Name("engineering".parse().unwrap()); +/// let (authz_org, db_org): (authz::Organization, db::model::Organization) = +/// LookupPath::new(opctx, datastore) +/// .organization_name(&organization_name) +/// .fetch() +/// .await?; +/// +/// // Fetch an organization by id +/// let id: Uuid = todo!(); +/// let (authz_org, db_org): (authz::Organization, db::model::Organization) = +/// LookupPath::new(opctx, datastore) +/// .organization_id(id) +/// .fetch() +/// .await?; +/// +/// // Lookup a Project with the intent of creating an Instance inside it. For +/// // this purpose, we don't need the database row for the Project, so we use +/// // `lookup_for()`. +/// let project_name = db::model::Name("omicron".parse().unwrap()); +/// let (authz_org, authz_project) = +/// LookupPath::new(opctx, datastore) +/// .organization_name(&organization_name) +/// .project_name(&project_name) +/// .lookup_for(authz::Action::CreateChild) +/// .await?; +/// +/// // Fetch an Instance by a path of names (Organization name, Project name, +/// // Instance name) +/// let instance_name = db::model::Name("test-server".parse().unwrap()); +/// let (authz_org, authz_project, authz_instance, db_instance) = +/// LookupPath::new(opctx, datastore) +/// .organization_name(&organization_name) +/// .project_name(&project_name) +/// .instance_name(&instance_name) +/// .fetch() +/// .await?; +/// +/// // Having looked up the Instance, you have the `authz::Project`. Use this +/// // to look up a Disk that you expect is in the same Project. +/// let disk_name = db::model::Name("my-disk".parse().unwrap()); +/// let (_, _, authz_disk, db_disk) = +/// LookupPath::new(opctx, datastore) +/// .project_id(authz_project.id()) +/// .disk_name(&disk_name) +/// .fetch() +/// .await?; +/// # } +/// ``` +// Implementation notes +// +// We say that a caller using `LookupPath` is building a _selection path_ for a +// resource. They use this builder interface to _select_ a specific resource. +// Example selection paths: +// +// - From the root, select Organization with name "org1", then Project with name +// "proj1", then Instance with name "instance1". +// +// - From the root, select Project with id 123, then Instance "instance1". +// +// A selection path always starts at the root, then _may_ contain a lookup-by-id +// node, and then _may_ contain any number of lookup-by-name nodes. It must +// include at least one lookup-by-id or lookup-by-name node. +// +// Once constructed, it looks like this: +// +// Instance +// key: Key::Name(p, "instance1") +// | +// +----------+ +// | +// v +// Project +// key: Key::Name(o, "proj") +// | +// +----------+ +// | +// v +// Organization +// key: Key::Name(r, "org1") +// | +// +----------+ +// | +// v +// Root +// lookup_root: LookupPath (references OpContext and +// DataStore) +// +// This is essentially a singly-linked list, except that each node _owns_ +// (rather than references) the previous node. This is important: the caller's +// going to do something like this: +// +// let (authz_org, authz_project, authz_instance, db_instance) = +// LookupPath::new(opctx, datastore) // returns LookupPath +// .organization_name("org1") // consumes LookupPath, +// // returns Organization +// .project_name("proj1") // consumes Organization, +// returns Project +// .instance_name("instance1") // consumes Project, +// returns Instance +// .fetch().await?; +// +// As you can see, at each step, a selection function (like "organization_name") +// consumes the current tail of the list and returns a new tail. We don't want +// the caller to have to keep track of multiple objects, so that implies that +// the tail must own all the state that we're building up as we go. +pub struct LookupPath<'a> { + opctx: &'a OpContext, + datastore: &'a DataStore, +} + +impl<'a> LookupPath<'a> { + /// Begin selecting a resource for lookup + /// + /// Authorization checks will be applied to the caller in `opctx`. + pub fn new<'b, 'c>( + opctx: &'b OpContext, + datastore: &'c DataStore, + ) -> LookupPath<'a> + where + 'b: 'a, + 'c: 'a, + { + LookupPath { opctx, datastore } + } + + // The top-level selection functions are implemented by hand because the + // macro is not in a great position to do this. + + /// Select a resource of type Organization, identified by its name + pub fn organization_name<'b, 'c>(self, name: &'b Name) -> Organization<'c> + where + 'a: 'c, + 'b: 'c, + { + Organization { key: Key::Name(Root { lookup_root: self }, name) } + } + + /// Select a resource of type Organization, identified by its id + pub fn organization_id(self, id: Uuid) -> Organization<'a> { + Organization { key: Key::Id(Root { lookup_root: self }, id) } + } + + /// Select a resource of type Project, identified by its id + pub fn project_id(self, id: Uuid) -> Project<'a> { + Project { key: Key::Id(Root { lookup_root: self }, id) } + } + + /// Select a resource of type Instance, identified by its id + pub fn instance_id(self, id: Uuid) -> Instance<'a> { + Instance { key: Key::Id(Root { lookup_root: self }, id) } + } + + /// Select a resource of type Disk, identified by its id + pub fn disk_id(self, id: Uuid) -> Disk<'a> { + Disk { key: Key::Id(Root { lookup_root: self }, id) } + } +} + +/// Describes a node along the selection path of a resource +enum Key<'a, P> { + /// We're looking for a resource with the given name within the given parent + /// collection + Name(P, &'a Name), + + /// We're looking for a resource with the given id + /// + /// This has no parent container -- a by-id lookup is always global. + Id(Root<'a>, Uuid), +} + +/// Represents the head of the selection path for a resource +struct Root<'a> { + lookup_root: LookupPath<'a>, +} + +impl<'a> Root<'a> { + fn lookup_root(&self) -> &LookupPath<'a> { + &self.lookup_root + } +} + +// Define the specific builder types for each resource. The `lookup_resource` +// macro defines a struct for the resource, helper functions for selecting child +// resources, and the publicly-exposed fetch functions (fetch(), fetch_for(), +// and lookup_for()). + +lookup_resource! { + name = "Organization", + ancestors = [], + children = [ "Project" ], + authz_kind = Typed +} + +lookup_resource! { + name = "Project", + ancestors = [ "Organization" ], + children = [ "Disk", "Instance" ], + authz_kind = Typed +} + +lookup_resource! { + name = "Instance", + ancestors = [ "Organization", "Project" ], + children = [], + authz_kind = Generic +} + +lookup_resource! { + name = "Disk", + ancestors = [ "Organization", "Project" ], + children = [], + authz_kind = Generic +} + +#[cfg(test)] +mod test { + use super::Instance; + use super::Key; + use super::LookupPath; + use super::Organization; + use super::Project; + use crate::context::OpContext; + use crate::db::model::Name; + use nexus_test_utils::db::test_setup_database; + use omicron_test_utils::dev; + use std::sync::Arc; + + /* This is a smoke test that things basically appear to work. */ + #[tokio::test] + async fn test_lookup() { + let logctx = dev::test_setup_log("test_lookup"); + let mut db = test_setup_database(&logctx.log).await; + let (_, datastore) = + crate::db::datastore::datastore_test(&logctx, &db).await; + let opctx = + OpContext::for_tests(logctx.log.new(o!()), Arc::clone(&datastore)); + let org_name: Name = Name("my-org".parse().unwrap()); + let project_name: Name = Name("my-project".parse().unwrap()); + let instance_name: Name = Name("my-instance".parse().unwrap()); + + let leaf = LookupPath::new(&opctx, &datastore) + .organization_name(&org_name) + .project_name(&project_name) + .instance_name(&instance_name); + assert!(matches!(&leaf, + Instance { + key: Key::Name(Project { + key: Key::Name(Organization { + key: Key::Name(_, o) + }, p) + }, i) + } + if **o == org_name && **p == project_name && **i == instance_name)); + + let org_id = "006f29d9-0ff0-e2d2-a022-87e152440122".parse().unwrap(); + let leaf = LookupPath::new(&opctx, &datastore) + .organization_id(org_id) + .project_name(&project_name); + assert!(matches!(&leaf, Project { + key: Key::Name(Organization { + key: Key::Id(_, o) + }, p) + } if *o == org_id && **p == project_name)); + + db.cleanup().await.unwrap(); + } +} diff --git a/nexus/src/db/mod.rs b/nexus/src/db/mod.rs index 341c023c9cb..46d15f4ee2d 100644 --- a/nexus/src/db/mod.rs +++ b/nexus/src/db/mod.rs @@ -14,6 +14,7 @@ pub mod datastore; mod error; mod explain; pub mod fixed_data; +pub mod lookup; mod pagination; mod pool; mod saga_recovery; diff --git a/nexus/src/lib.rs b/nexus/src/lib.rs index bf7d2183c23..07f3ed06693 100644 --- a/nexus/src/lib.rs +++ b/nexus/src/lib.rs @@ -14,13 +14,13 @@ #![allow(clippy::style)] pub mod authn; // Public only for testing -pub mod authz; -pub mod config; // public for testing -mod context; -pub mod db; // Public only for some documentation examples +pub mod authz; // Public for documentation examples +pub mod config; // Public for testing +pub mod context; // Public for documentation examples +pub mod db; // Public for documentation examples mod defaults; -pub mod external_api; // public for testing -pub mod internal_api; // public for testing +pub mod external_api; // Public for testing +pub mod internal_api; // Public for testing mod nexus; mod populate; mod saga_interface; diff --git a/nexus/src/nexus.rs b/nexus/src/nexus.rs index c5b27852646..3ad7de6efe7 100644 --- a/nexus/src/nexus.rs +++ b/nexus/src/nexus.rs @@ -10,6 +10,7 @@ use crate::config; use crate::context::OpContext; use crate::db; use crate::db::identity::{Asset, Resource}; +use crate::db::lookup::LookupPath; use crate::db::model::DatasetKind; use crate::db::model::Name; use crate::db::model::RouterRoute; @@ -602,15 +603,18 @@ impl Nexus { organization_name: &Name, new_project: ¶ms::ProjectCreate, ) -> CreateResult { - let org = self - .db_datastore - .organization_lookup_by_path(organization_name) + let (authz_org,) = LookupPath::new(opctx, &self.db_datastore) + .organization_name(organization_name) + .lookup_for(authz::Action::CreateChild) .await?; // Create a project. - let db_project = db::model::Project::new(org.id(), new_project.clone()); let db_project = - self.db_datastore.project_create(opctx, &org, db_project).await?; + db::model::Project::new(authz_org.id(), new_project.clone()); + let db_project = self + .db_datastore + .project_create(opctx, &authz_org, db_project) + .await?; // TODO: We probably want to have "project creation" and "default VPC // creation" co-located within a saga for atomicity. @@ -618,6 +622,9 @@ impl Nexus { // Until then, we just perform the operations sequentially. // Create a default VPC associated with the project. + // TODO-correctness We need to be using the project_id we just created. + // project_create() should return authz::Project and we should use that + // here. let _ = self .project_create_vpc( opctx, @@ -646,15 +653,12 @@ impl Nexus { organization_name: &Name, project_name: &Name, ) -> LookupResult { - let authz_org = self - .db_datastore - .organization_lookup_by_path(organization_name) - .await?; - Ok(self - .db_datastore - .project_fetch(opctx, &authz_org, project_name) + Ok(LookupPath::new(opctx, &self.db_datastore) + .organization_name(organization_name) + .project_name(project_name) + .fetch() .await? - .1) + .2) } pub async fn projects_list_by_name( @@ -663,9 +667,9 @@ impl Nexus { organization_name: &Name, pagparams: &DataPageParams<'_, Name>, ) -> ListResultVec { - let authz_org = self - .db_datastore - .organization_lookup_by_path(organization_name) + let (authz_org,) = LookupPath::new(opctx, &self.db_datastore) + .organization_name(organization_name) + .lookup_for(authz::Action::CreateChild) .await?; self.db_datastore .projects_list_by_name(opctx, &authz_org, pagparams) @@ -678,9 +682,9 @@ impl Nexus { organization_name: &Name, pagparams: &DataPageParams<'_, Uuid>, ) -> ListResultVec { - let authz_org = self - .db_datastore - .organization_lookup_by_path(organization_name) + let (authz_org,) = LookupPath::new(opctx, &self.db_datastore) + .organization_name(organization_name) + .lookup_for(authz::Action::CreateChild) .await?; self.db_datastore .projects_list_by_id(opctx, &authz_org, pagparams) @@ -1424,20 +1428,19 @@ impl Nexus { instance_name: &Name, disk_name: &Name, ) -> UpdateResult { - // TODO: This shouldn't be looking up multiple database entries by name, - // it should resolve names to IDs first. - let authz_project = self - .db_datastore - .project_lookup_by_path(organization_name, project_name) - .await?; - let (authz_disk, db_disk) = self - .db_datastore - .disk_fetch(opctx, &authz_project, disk_name) - .await?; - let (authz_instance, db_instance) = self - .db_datastore - .instance_fetch(opctx, &authz_project, instance_name) - .await?; + let (_, authz_project, authz_disk, db_disk) = + LookupPath::new(opctx, &self.db_datastore) + .organization_name(organization_name) + .project_name(project_name) + .disk_name(disk_name) + .fetch() + .await?; + let (_, _, authz_instance, db_instance) = + LookupPath::new(opctx, &self.db_datastore) + .project_id(authz_project.id()) + .instance_name(instance_name) + .fetch() + .await?; let instance_id = &authz_instance.id(); // Enforce attached disks limit @@ -1574,20 +1577,19 @@ impl Nexus { instance_name: &Name, disk_name: &Name, ) -> UpdateResult { - // TODO: This shouldn't be looking up multiple database entries by name, - // it should resolve names to IDs first. - let authz_project = self - .db_datastore - .project_lookup_by_path(organization_name, project_name) - .await?; - let (authz_disk, db_disk) = self - .db_datastore - .disk_fetch(opctx, &authz_project, disk_name) - .await?; - let (authz_instance, db_instance) = self - .db_datastore - .instance_fetch(opctx, &authz_project, instance_name) - .await?; + let (_, authz_project, authz_disk, db_disk) = + LookupPath::new(opctx, &self.db_datastore) + .organization_name(organization_name) + .project_name(project_name) + .disk_name(disk_name) + .fetch() + .await?; + let (_, _, authz_instance, db_instance) = + LookupPath::new(opctx, &self.db_datastore) + .project_id(authz_project.id()) + .instance_name(instance_name) + .fetch() + .await?; let instance_id = &authz_instance.id(); match &db_disk.state().into() { diff --git a/nexus/test-utils/src/http_testing.rs b/nexus/test-utils/src/http_testing.rs index f3e16f0e0bd..4d3066dde20 100644 --- a/nexus/test-utils/src/http_testing.rs +++ b/nexus/test-utils/src/http_testing.rs @@ -466,6 +466,14 @@ impl<'a> NexusRequest<'a> { self.request_builder.execute().await } + /// Convenience function that executes the request, parses the body, and + /// unwraps any `Result`s along the way. + pub async fn execute_and_parse_unwrap( + self, + ) -> T { + self.execute().await.unwrap().parsed_body().unwrap() + } + /// Returns a new `NexusRequest` suitable for `POST $uri` with the given /// `body` pub fn objects_post( diff --git a/nexus/tests/integration_tests/basic.rs b/nexus/tests/integration_tests/basic.rs index 9005559bbe2..633f4d1a9a2 100644 --- a/nexus/tests/integration_tests/basic.rs +++ b/nexus/tests/integration_tests/basic.rs @@ -39,18 +39,6 @@ async fn test_basic_failures(cptestctx: &ControlPlaneTestContext) { let org_name = "test-org"; create_organization(&client, &org_name).await; - // Error case: GET /nonexistent (a path with no route at all) - let error = client - .make_request( - Method::GET, - "/nonexistent", - None as Option<()>, - StatusCode::NOT_FOUND, - ) - .await - .expect_err("expected error"); - assert_eq!("Not Found", error.message); - struct TestCase<'a> { method: http::Method, uri: &'a str, @@ -60,6 +48,15 @@ async fn test_basic_failures(cptestctx: &ControlPlaneTestContext) { } let test_cases = vec![ + // Error case: GET /nonexistent (a path with no route at all)( + TestCase { + method: Method::GET, + uri: "/nonexistent", + expected_code: StatusCode::NOT_FOUND, + expected_error: "Not Found", + body: None, + }, + // 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. @@ -136,7 +133,7 @@ async fn test_basic_failures(cptestctx: &ControlPlaneTestContext) { ]; for test_case in test_cases { - let error = if let Some(body) = test_case.body { + let error: HttpErrorResponseBody = if let Some(body) = test_case.body { NexusRequest::expect_failure_with_body( client, test_case.expected_code, @@ -153,11 +150,8 @@ async fn test_basic_failures(cptestctx: &ControlPlaneTestContext) { ) } .authn_as(AuthnMode::PrivilegedUser) - .execute() - .await - .unwrap() - .parsed_body::() - .unwrap(); + .execute_and_parse_unwrap() + .await; assert_eq!(test_case.expected_error, error.message); } diff --git a/nexus/tests/integration_tests/unauthorized.rs b/nexus/tests/integration_tests/unauthorized.rs index c879569455c..8f2d8ac1367 100644 --- a/nexus/tests/integration_tests/unauthorized.rs +++ b/nexus/tests/integration_tests/unauthorized.rs @@ -42,13 +42,12 @@ use omicron_nexus::authn::external::spoof; // TODO-coverage: // * It would be good to add a built-in test user that can read everything in // the world and use that to exercise 404 vs. 401/403 behavior. -// * It'd be nice to verify that all the endpoints listed here are within the -// OpenAPI spec. -// * It'd be nice to produce a list of endpoints from the OpenAPI spec that are -// not checked here. We could put this into an expectorate file and make sure -// that we don't add new unchecked endpoints. // * When we finish authz, maybe the hardcoded information here can come instead // from the OpenAPI spec? +// * For each endpoint that hits a real resource, we should hit the same +// endpoint with a non-existent resource to ensure that we get the same result +// (so that we don't leak information about existence based on, say, 401 vs. +// 403). #[nexus_test] async fn test_unauthorized(cptestctx: &ControlPlaneTestContext) { DiskTest::new(cptestctx).await;