Skip to content

Commit

Permalink
convert all VPC lookups to new API, plus various by-id lookups (#842)
Browse files Browse the repository at this point in the history
  • Loading branch information
@davepacheco
davepacheco authored Mar 31, 2022
1 parent d0094ca commit b409424
Show file tree
Hide file tree
Showing 4 changed files with 176 additions and 459 deletions.
288 changes: 1 addition & 287 deletions nexus/src/db/datastore.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -626,34 +626,6 @@ 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<authz::Organization> {
use db::schema::organization::dsl;
// We only do this database lookup to verify that the Organization with
// this id exists and hasn't been deleted.
let _: Uuid = dsl::organization
.filter(dsl::time_deleted.is_null())
.filter(dsl::id.eq(organization_id))
.select(dsl::id)
.first_async(self.pool())
.await
.map_err(|e| {
public_error_from_diesel_pool(
e,
ErrorHandler::NotFoundByLookup(
ResourceType::Organization,
LookupType::ById(organization_id),
),
)
})?;
Ok(authz::FLEET
.organization(organization_id, LookupType::ById(organization_id)))
}

/// Look up the id for an organization based on its name
///
/// Returns an [`authz::Organization`] (which makes the id available).
Expand Down Expand Up @@ -882,32 +854,6 @@ impl DataStore {
})
}

/// Fetch an [`authz::Project`] based on its id
pub async fn project_lookup_by_id(
&self,
project_id: Uuid,
) -> LookupResult<authz::Project> {
use db::schema::project::dsl;
let organization_id = dsl::project
.filter(dsl::time_deleted.is_null())
.filter(dsl::id.eq(project_id))
.select(dsl::organization_id)
.first_async(self.pool())
.await
.map_err(|e| {
public_error_from_diesel_pool(
e,
ErrorHandler::NotFoundByLookup(
ResourceType::Project,
LookupType::ById(project_id),
),
)
})?;
let authz_organization =
self.organization_lookup_by_id(organization_id).await?;
Ok(authz_organization.project(project_id, LookupType::ById(project_id)))
}

/// Look up the id for a Project based on its name
///
/// Returns an [`authz::Project`] (which makes the id available).
Expand Down Expand Up @@ -1074,35 +1020,6 @@ impl DataStore {
})
}

/// Fetch an [`authz::Instance`] based on its id
pub async fn instance_lookup_by_id(
&self,
instance_id: Uuid,
) -> LookupResult<authz::Instance> {
use db::schema::instance::dsl;
let project_id = dsl::instance
.filter(dsl::time_deleted.is_null())
.filter(dsl::id.eq(instance_id))
.select(dsl::project_id)
.first_async(self.pool())
.await
.map_err(|e| {
public_error_from_diesel_pool(
e,
ErrorHandler::NotFoundByLookup(
ResourceType::Instance,
LookupType::ById(instance_id),
),
)
})?;
let authz_project = self.project_lookup_by_id(project_id).await?;
Ok(authz_project.child_generic(
ResourceType::Instance,
instance_id,
LookupType::ById(instance_id),
))
}

/// Look up the id for an Instance based on its name
///
/// Returns an [`authz::Instance`] (which makes the id available).
Expand Down Expand Up @@ -1381,35 +1298,6 @@ impl DataStore {
})
}

/// Fetch an [`authz::Disk`] based on its id
pub async fn disk_lookup_by_id(
&self,
disk_id: Uuid,
) -> LookupResult<authz::Disk> {
use db::schema::disk::dsl;
let project_id = dsl::disk
.filter(dsl::time_deleted.is_null())
.filter(dsl::id.eq(disk_id))
.select(dsl::project_id)
.first_async(self.pool())
.await
.map_err(|e| {
public_error_from_diesel_pool(
e,
ErrorHandler::NotFoundByLookup(
ResourceType::Disk,
LookupType::ById(disk_id),
),
)
})?;
let authz_project = self.project_lookup_by_id(project_id).await?;
Ok(authz_project.child_generic(
ResourceType::Disk,
disk_id,
LookupType::ById(disk_id),
))
}

/// Look up the id for a Disk based on its name
///
/// Returns an [`authz::Disk`] (which makes the id available).
Expand All @@ -1420,7 +1308,7 @@ impl DataStore {
// Projects), we don't do an authz check in the "lookup_by_path" functions
// because we don't know if the caller has access to do the lookup. For
// leaf resources (like Instances and Disks), though, we do. We could do
// the authz check here, and in disk_lookup_by_id() too. Should we?
// the authz check here. Should we?
pub async fn disk_lookup_by_path(
&self,
organization_name: &Name,
Expand Down Expand Up @@ -2144,78 +2032,6 @@ impl DataStore {

// VPCs

/// Fetches a Vpc from the database and returns both the database row
/// and an [`authz::Vpc`] for doing authz checks
///
/// See [`DataStore::organization_lookup_noauthz()`] for intended use cases
/// and caveats.
// TODO-security See the note on organization_lookup_noauthz().
async fn vpc_lookup_noauthz(
&self,
authz_project: &authz::Project,
vpc_name: &Name,
) -> LookupResult<(authz::Vpc, Vpc)> {
use db::schema::vpc::dsl;
dsl::vpc
.filter(dsl::time_deleted.is_null())
.filter(dsl::project_id.eq(authz_project.id()))
.filter(dsl::name.eq(vpc_name.clone()))
.select(Vpc::as_select())
.first_async(self.pool())
.await
.map_err(|e| {
public_error_from_diesel_pool(
e,
ErrorHandler::NotFoundByLookup(
ResourceType::Vpc,
LookupType::ByName(vpc_name.as_str().to_owned()),
),
)
})
.map(|d| {
(
authz_project.child_generic(
ResourceType::Vpc,
d.id(),
LookupType::from(&vpc_name.0),
),
d,
)
})
}

/// Look up the id for a Vpc based on its name
///
/// Returns an [`authz::Vpc`] (which makes the id available).
///
/// Like the other "lookup_by_path()" functions, this function does no authz
/// checks.
pub async fn vpc_lookup_by_path(
&self,
organization_name: &Name,
project_name: &Name,
vpc_name: &Name,
) -> LookupResult<authz::Vpc> {
let authz_project = self
.project_lookup_by_path(organization_name, project_name)
.await?;
self.vpc_lookup_noauthz(&authz_project, vpc_name).await.map(|(v, _)| v)
}

/// Lookup a Vpc by name and return the full database record, along
/// with an [`authz::Vpc`] for subsequent authorization checks
pub async fn vpc_fetch(
&self,
opctx: &OpContext,
authz_project: &authz::Project,
name: &Name,
) -> LookupResult<(authz::Vpc, Vpc)> {
let (authz_vpc, db_vpc) =
self.vpc_lookup_noauthz(authz_project, name).await?;
opctx.authorize(authz::Action::Read, &authz_vpc).await?;
Ok((authz_vpc, db_vpc))
}

pub async fn project_list_vpcs(
&self,
opctx: &OpContext,
Expand Down Expand Up @@ -2292,33 +2108,6 @@ impl DataStore {
})
}

// TODO-security TODO-cleanup Remove this function. Update callers to use
// vpc_lookup_by_path() or vpc_fetch() instead.
pub async fn vpc_fetch_by_name(
&self,
project_id: &Uuid,
vpc_name: &Name,
) -> LookupResult<Vpc> {
use db::schema::vpc::dsl;

dsl::vpc
.filter(dsl::time_deleted.is_null())
.filter(dsl::project_id.eq(*project_id))
.filter(dsl::name.eq(vpc_name.clone()))
.select(Vpc::as_select())
.get_result_async(self.pool())
.await
.map_err(|e| {
public_error_from_diesel_pool(
e,
ErrorHandler::NotFoundByLookup(
ResourceType::Vpc,
LookupType::ByName(vpc_name.as_str().to_owned()),
),
)
})
}

pub async fn project_delete_vpc(
&self,
opctx: &OpContext,
Expand Down Expand Up @@ -2489,81 +2278,6 @@ impl DataStore {
.map_err(|e| public_error_from_diesel_pool(e, ErrorHandler::Server))
}

/// Fetches a VpcSubnet from the database and returns both the database row
/// and an [`authz::VpcSubnet`] for doing authz checks
///
/// See [`DataStore::organization_lookup_noauthz()`] for intended use cases
/// and caveats.
// TODO-security See the note on organization_lookup_noauthz().
async fn vpc_subnet_lookup_noauthz(
&self,
authz_vpc: &authz::Vpc,
subnet_name: &Name,
) -> LookupResult<(authz::VpcSubnet, VpcSubnet)> {
use db::schema::vpc_subnet::dsl;
dsl::vpc_subnet
.filter(dsl::time_deleted.is_null())
.filter(dsl::vpc_id.eq(authz_vpc.id()))
.filter(dsl::name.eq(subnet_name.clone()))
.select(VpcSubnet::as_select())
.first_async(self.pool())
.await
.map_err(|e| {
public_error_from_diesel_pool(
e,
ErrorHandler::NotFoundByLookup(
ResourceType::VpcSubnet,
LookupType::ByName(subnet_name.as_str().to_owned()),
),
)
})
.map(|d| {
(
authz_vpc.child_generic(
ResourceType::VpcSubnet,
d.id(),
LookupType::from(&subnet_name.0),
),
d,
)
})
}

/// Look up the id for a VpcSubnet based on its name
///
/// Returns an [`authz::VpcSubnet`] (which makes the id available).
///
/// Like the other "lookup_by_path()" functions, this function does no authz
/// checks.
pub async fn vpc_subnet_lookup_by_path(
&self,
organization_name: &Name,
project_name: &Name,
vpc_name: &Name,
subnet_name: &Name,
) -> LookupResult<authz::Vpc> {
let authz_vpc = self
.vpc_lookup_by_path(organization_name, project_name, vpc_name)
.await?;
self.vpc_subnet_lookup_noauthz(&authz_vpc, subnet_name)
.await
.map(|(v, _)| v)
}

/// Lookup a VpcSubnet by name and return the full database record, along
/// with an [`authz::VpcSubnet`] for subsequent authorization checks
pub async fn vpc_subnet_fetch(
&self,
opctx: &OpContext,
authz_vpc: &authz::Vpc,
name: &Name,
) -> LookupResult<(authz::VpcSubnet, VpcSubnet)> {
let (authz_vpc_subnet, db_vpc_subnet) =
self.vpc_subnet_lookup_noauthz(authz_vpc, name).await?;
opctx.authorize(authz::Action::Read, &authz_vpc_subnet).await?;
Ok((authz_vpc_subnet, db_vpc_subnet))
}

/// Insert a VPC Subnet, checking for unique IP address ranges.
pub async fn vpc_create_subnet(
&self,
Expand Down
14 changes: 13 additions & 1 deletion nexus/src/db/lookup.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -210,6 +210,11 @@ impl<'a> LookupPath<'a> {
Vpc { key: Key::Id(Root { lookup_root: self }, id) }
}

/// Select a resource of type VpcSubnet, identified by its id
pub fn vpc_subnet_id(self, id: Uuid) -> VpcSubnet<'a> {
VpcSubnet { key: Key::Id(Root { lookup_root: self }, id) }
}

/// Select a resource of type VpcRouter, identified by its id
pub fn vpc_router_id(self, id: Uuid) -> VpcRouter<'a> {
VpcRouter { key: Key::Id(Root { lookup_root: self }, id) }
Expand Down Expand Up @@ -280,7 +285,14 @@ lookup_resource! {
lookup_resource! {
name = "Vpc",
ancestors = [ "Organization", "Project" ],
children = [ "VpcRouter" ],
children = [ "VpcRouter", "VpcSubnet" ],
authz_kind = Generic
}

lookup_resource! {
name = "VpcSubnet",
ancestors = [ "Organization", "Project", "Vpc" ],
children = [ ],
authz_kind = Generic
}

Expand Down
Loading

0 comments on commit b409424

Please sign in to comment.