Skip to content

Commit 6c742ea

Browse files
authored
authz: protect GET for Projects (and prepare for more) (#618)
1 parent 2a3c355 commit 6c742ea

File tree

8 files changed

+178
-149
lines changed

8 files changed

+178
-149
lines changed

nexus/src/authz/api_resources.rs

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -331,6 +331,11 @@ pub struct Project {
331331
}
332332

333333
impl Project {
334+
// TODO-cleanup see note on Organization::id above.
335+
pub fn id(&self) -> Uuid {
336+
self.project_id
337+
}
338+
334339
/// Returns an authz resource representing any child of this Project (e.g.,
335340
/// an Instance or Disk)
336341
///

nexus/src/db/datastore.rs

Lines changed: 53 additions & 35 deletions
Original file line numberDiff line numberDiff line change
@@ -325,7 +325,7 @@ impl DataStore {
325325
///
326326
/// This function does no authz checks because it is not possible to know
327327
/// just by looking up an Organization's id what privileges are required.
328-
pub async fn organization_lookup_id(
328+
pub async fn organization_lookup_path(
329329
&self,
330330
name: &Name,
331331
) -> LookupResult<authz::Organization> {
@@ -340,8 +340,6 @@ impl DataStore {
340340
) -> LookupResult<(authz::Organization, Organization)> {
341341
let (authz_org, db_org) =
342342
self.organization_lookup_noauthz(name).await?;
343-
// TODO-security See the note in authz::authorize(). This needs to
344-
// return a 404, not a 403.
345343
opctx.authorize(authz::Action::Read, &authz_org).await?;
346344
Ok((authz_org, db_org))
347345
}
@@ -475,7 +473,7 @@ impl DataStore {
475473
) -> UpdateResult<Organization> {
476474
use db::schema::organization::dsl;
477475

478-
let authz_org = self.organization_lookup_id(name).await?;
476+
let authz_org = self.organization_lookup_path(name).await?;
479477
opctx.authorize(authz::Action::Modify, &authz_org).await?;
480478

481479
diesel::update(dsl::organization)
@@ -528,29 +526,72 @@ impl DataStore {
528526
})
529527
}
530528

531-
/// Lookup a project by name.
532-
pub async fn project_fetch(
529+
/// Fetches a Project from the database and returns both the database row
530+
/// and an authz::Project for doing authz checks
531+
///
532+
/// See [`DataStore::organization_lookup_noauthz()`] for intended use cases
533+
/// and caveats.
534+
// TODO-security See the note on organization_lookup_noauthz().
535+
async fn project_lookup_noauthz(
533536
&self,
534-
organization_id: &Uuid,
535-
name: &Name,
536-
) -> LookupResult<Project> {
537+
authz_org: &authz::Organization,
538+
project_name: &Name,
539+
) -> LookupResult<(authz::Project, Project)> {
537540
use db::schema::project::dsl;
538541
dsl::project
539542
.filter(dsl::time_deleted.is_null())
540-
.filter(dsl::organization_id.eq(*organization_id))
541-
.filter(dsl::name.eq(name.clone()))
543+
.filter(dsl::organization_id.eq(authz_org.id()))
544+
.filter(dsl::name.eq(project_name.clone()))
542545
.select(Project::as_select())
543546
.first_async(self.pool())
544547
.await
545548
.map_err(|e| {
546549
public_error_from_diesel_pool(
547550
e,
548551
ResourceType::Project,
549-
LookupType::ByName(name.as_str().to_owned()),
552+
LookupType::ByName(project_name.as_str().to_owned()),
553+
)
554+
})
555+
.map(|p| {
556+
(
557+
authz_org
558+
.project(p.id(), LookupType::from(&project_name.0)),
559+
p,
550560
)
551561
})
552562
}
553563

564+
/// Look up the id for a Project based on its name
565+
///
566+
/// Returns an [`authz::Project`] (which makes the id available).
567+
///
568+
/// This function does no authz checks because it is not possible to know
569+
/// just by looking up an Project's id what privileges are required.
570+
pub async fn project_lookup_path(
571+
&self,
572+
organization_name: &Name,
573+
project_name: &Name,
574+
) -> LookupResult<authz::Project> {
575+
let authz_org =
576+
self.organization_lookup_path(organization_name).await?;
577+
self.project_lookup_noauthz(&authz_org, project_name)
578+
.await
579+
.map(|(p, _)| p)
580+
}
581+
582+
/// Lookup a project by name.
583+
pub async fn project_fetch(
584+
&self,
585+
opctx: &OpContext,
586+
authz_org: &authz::Organization,
587+
name: &Name,
588+
) -> LookupResult<(authz::Project, Project)> {
589+
let (authz_org, db_org) =
590+
self.project_lookup_noauthz(authz_org, name).await?;
591+
opctx.authorize(authz::Action::Read, &authz_org).await?;
592+
Ok((authz_org, db_org))
593+
}
594+
554595
/// Delete a project
555596
/*
556597
* TODO-correctness This needs to check whether there are any resources that
@@ -582,29 +623,6 @@ impl DataStore {
582623
Ok(())
583624
}
584625

585-
/// Look up the id for a project based on its name
586-
pub async fn project_lookup_id_by_name(
587-
&self,
588-
organization_id: &Uuid,
589-
name: &Name,
590-
) -> Result<Uuid, Error> {
591-
use db::schema::project::dsl;
592-
dsl::project
593-
.filter(dsl::time_deleted.is_null())
594-
.filter(dsl::organization_id.eq(*organization_id))
595-
.filter(dsl::name.eq(name.clone()))
596-
.select(dsl::id)
597-
.get_result_async::<Uuid>(self.pool())
598-
.await
599-
.map_err(|e| {
600-
public_error_from_diesel_pool(
601-
e,
602-
ResourceType::Project,
603-
LookupType::ByName(name.as_str().to_owned()),
604-
)
605-
})
606-
}
607-
608626
pub async fn projects_list_by_id(
609627
&self,
610628
opctx: &OpContext,

nexus/src/external_api/http_entrypoints.rs

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -477,8 +477,10 @@ async fn organization_projects_get_project(
477477
let organization_name = &path.organization_name;
478478
let project_name = &path.project_name;
479479
let handler = async {
480-
let project =
481-
nexus.project_fetch(&organization_name, &project_name).await?;
480+
let opctx = OpContext::for_external_api(&rqctx).await?;
481+
let project = nexus
482+
.project_fetch(&opctx, &organization_name, &project_name)
483+
.await?;
482484
Ok(HttpResponseOk(project.into()))
483485
};
484486
apictx.external_latencies.instrument_dropshot_handler(&rqctx, handler).await

0 commit comments

Comments
 (0)