From deb573e4bc624a0c9aa3644f405610e4797368cc Mon Sep 17 00:00:00 2001 From: Sean Klein Date: Fri, 31 May 2024 17:16:11 -0700 Subject: [PATCH] review feedback --- nexus/auth/src/authz/api_resources.rs | 180 +++++++++----------------- nexus/auth/src/authz/context.rs | 36 ++---- nexus/auth/src/authz/oso_generic.rs | 18 +-- nexus/auth/src/context.rs | 2 +- nexus/db-fixed-data/Cargo.toml | 1 + 5 files changed, 81 insertions(+), 156 deletions(-) diff --git a/nexus/auth/src/authz/api_resources.rs b/nexus/auth/src/authz/api_resources.rs index e2cbddc034..98a24b68b5 100644 --- a/nexus/auth/src/authz/api_resources.rs +++ b/nexus/auth/src/authz/api_resources.rs @@ -109,18 +109,12 @@ impl AuthorizedResource for T where T: ApiResource + oso::PolarClass + Clone, { - fn load_roles<'a, 'b, 'c, 'd, 'e>( - &'a self, - opctx: &'b OpContext, - authn: &'c authn::Context, - roleset: &'d mut RoleSet, - ) -> BoxFuture<'e, Result<(), Error>> - where - 'a: 'e, - 'b: 'e, - 'c: 'e, - 'd: 'e, - { + fn load_roles<'fut>( + &'fut self, + opctx: &'fut OpContext, + authn: &'fut authn::Context, + roleset: &'fut mut RoleSet, + ) -> BoxFuture<'fut, Result<(), Error>> { load_roles_for_resource_tree(self, opctx, authn, roleset).boxed() } @@ -261,18 +255,12 @@ impl oso::PolarClass for BlueprintConfig { } impl AuthorizedResource for BlueprintConfig { - fn load_roles<'a, 'b, 'c, 'd, 'e>( - &'a self, - opctx: &'b OpContext, - authn: &'c authn::Context, - roleset: &'d mut RoleSet, - ) -> futures::future::BoxFuture<'e, Result<(), Error>> - where - 'a: 'e, - 'b: 'e, - 'c: 'e, - 'd: 'e, - { + fn load_roles<'fut>( + &'fut self, + opctx: &'fut OpContext, + authn: &'fut authn::Context, + roleset: &'fut mut RoleSet, + ) -> futures::future::BoxFuture<'fut, Result<(), Error>> { // There are no roles on the BlueprintConfig, only permissions. But we // still need to load the Fleet-related roles to verify that the actor // has the "admin" role on the Fleet (possibly conferred from a Silo @@ -318,18 +306,12 @@ impl oso::PolarClass for ConsoleSessionList { } impl AuthorizedResource for ConsoleSessionList { - fn load_roles<'a, 'b, 'c, 'd, 'e>( - &'a self, - opctx: &'b OpContext, - authn: &'c authn::Context, - roleset: &'d mut RoleSet, - ) -> futures::future::BoxFuture<'e, Result<(), Error>> - where - 'a: 'e, - 'b: 'e, - 'c: 'e, - 'd: 'e, - { + fn load_roles<'fut>( + &'fut self, + opctx: &'fut OpContext, + authn: &'fut authn::Context, + roleset: &'fut mut RoleSet, + ) -> futures::future::BoxFuture<'fut, Result<(), Error>> { load_roles_for_resource_tree(&FLEET, opctx, authn, roleset).boxed() } @@ -371,18 +353,12 @@ impl oso::PolarClass for DnsConfig { } impl AuthorizedResource for DnsConfig { - fn load_roles<'a, 'b, 'c, 'd, 'e>( - &'a self, - opctx: &'b OpContext, - authn: &'c authn::Context, - roleset: &'d mut RoleSet, - ) -> futures::future::BoxFuture<'e, Result<(), Error>> - where - 'a: 'e, - 'b: 'e, - 'c: 'e, - 'd: 'e, - { + fn load_roles<'fut>( + &'fut self, + opctx: &'fut OpContext, + authn: &'fut authn::Context, + roleset: &'fut mut RoleSet, + ) -> futures::future::BoxFuture<'fut, Result<(), Error>> { load_roles_for_resource_tree(&FLEET, opctx, authn, roleset).boxed() } @@ -424,18 +400,12 @@ impl oso::PolarClass for IpPoolList { } impl AuthorizedResource for IpPoolList { - fn load_roles<'a, 'b, 'c, 'd, 'e>( - &'a self, - opctx: &'b OpContext, - authn: &'c authn::Context, - roleset: &'d mut RoleSet, - ) -> futures::future::BoxFuture<'e, Result<(), Error>> - where - 'a: 'e, - 'b: 'e, - 'c: 'e, - 'd: 'e, - { + fn load_roles<'fut>( + &'fut self, + opctx: &'fut OpContext, + authn: &'fut authn::Context, + roleset: &'fut mut RoleSet, + ) -> futures::future::BoxFuture<'fut, Result<(), Error>> { // There are no roles on the IpPoolList, only permissions. But we still // need to load the Fleet-related roles to verify that the actor has the // "admin" role on the Fleet (possibly conferred from a Silo role). @@ -472,18 +442,12 @@ impl oso::PolarClass for DeviceAuthRequestList { } impl AuthorizedResource for DeviceAuthRequestList { - fn load_roles<'a, 'b, 'c, 'd, 'e>( - &'a self, - opctx: &'b OpContext, - authn: &'c authn::Context, - roleset: &'d mut RoleSet, - ) -> futures::future::BoxFuture<'e, Result<(), Error>> - where - 'a: 'e, - 'b: 'e, - 'c: 'e, - 'd: 'e, - { + fn load_roles<'fut>( + &'fut self, + opctx: &'fut OpContext, + authn: &'fut authn::Context, + roleset: &'fut mut RoleSet, + ) -> futures::future::BoxFuture<'fut, Result<(), Error>> { // There are no roles on the DeviceAuthRequestList, only permissions. But we // still need to load the Fleet-related roles to verify that the actor has the // "admin" role on the Fleet. @@ -527,18 +491,12 @@ impl oso::PolarClass for Inventory { } impl AuthorizedResource for Inventory { - fn load_roles<'a, 'b, 'c, 'd, 'e>( - &'a self, - opctx: &'b OpContext, - authn: &'c authn::Context, - roleset: &'d mut RoleSet, - ) -> futures::future::BoxFuture<'e, Result<(), Error>> - where - 'a: 'e, - 'b: 'e, - 'c: 'e, - 'd: 'e, - { + fn load_roles<'fut>( + &'fut self, + opctx: &'fut OpContext, + authn: &'fut authn::Context, + roleset: &'fut mut RoleSet, + ) -> futures::future::BoxFuture<'fut, Result<(), Error>> { load_roles_for_resource_tree(&FLEET, opctx, authn, roleset).boxed() } @@ -583,18 +541,12 @@ impl oso::PolarClass for SiloCertificateList { } impl AuthorizedResource for SiloCertificateList { - fn load_roles<'a, 'b, 'c, 'd, 'e>( - &'a self, - opctx: &'b OpContext, - authn: &'c authn::Context, - roleset: &'d mut RoleSet, - ) -> futures::future::BoxFuture<'e, Result<(), Error>> - where - 'a: 'e, - 'b: 'e, - 'c: 'e, - 'd: 'e, - { + fn load_roles<'fut>( + &'fut self, + opctx: &'fut OpContext, + authn: &'fut authn::Context, + roleset: &'fut mut RoleSet, + ) -> futures::future::BoxFuture<'fut, Result<(), Error>> { // There are no roles on this resource, but we still need to load the // Silo-related roles. self.silo().load_roles(opctx, authn, roleset) @@ -641,18 +593,12 @@ impl oso::PolarClass for SiloIdentityProviderList { } impl AuthorizedResource for SiloIdentityProviderList { - fn load_roles<'a, 'b, 'c, 'd, 'e>( - &'a self, - opctx: &'b OpContext, - authn: &'c authn::Context, - roleset: &'d mut RoleSet, - ) -> futures::future::BoxFuture<'e, Result<(), Error>> - where - 'a: 'e, - 'b: 'e, - 'c: 'e, - 'd: 'e, - { + fn load_roles<'fut>( + &'fut self, + opctx: &'fut OpContext, + authn: &'fut authn::Context, + roleset: &'fut mut RoleSet, + ) -> futures::future::BoxFuture<'fut, Result<(), Error>> { // There are no roles on this resource, but we still need to load the // Silo-related roles. self.silo().load_roles(opctx, authn, roleset) @@ -696,18 +642,12 @@ impl oso::PolarClass for SiloUserList { } impl AuthorizedResource for SiloUserList { - fn load_roles<'a, 'b, 'c, 'd, 'e>( - &'a self, - opctx: &'b OpContext, - authn: &'c authn::Context, - roleset: &'d mut RoleSet, - ) -> futures::future::BoxFuture<'e, Result<(), Error>> - where - 'a: 'e, - 'b: 'e, - 'c: 'e, - 'd: 'e, - { + fn load_roles<'fut>( + &'fut self, + opctx: &'fut OpContext, + authn: &'fut authn::Context, + roleset: &'fut mut RoleSet, + ) -> futures::future::BoxFuture<'fut, Result<(), Error>> { // There are no roles on this resource, but we still need to load the // Silo-related roles. self.silo().load_roles(opctx, authn, roleset) diff --git a/nexus/auth/src/authz/context.rs b/nexus/auth/src/authz/context.rs index 2ca7a23218..bd375321e3 100644 --- a/nexus/auth/src/authz/context.rs +++ b/nexus/auth/src/authz/context.rs @@ -164,17 +164,12 @@ pub trait AuthorizedResource: oso::ToPolar + Send + Sync + 'static { /// That's how this works for most resources. There are other kinds of /// resources (like the Database itself) that aren't stored in the database /// and for which a different mechanism might be used. - fn load_roles<'a, 'b, 'c, 'd, 'e>( - &'a self, - opctx: &'b OpContext, - authn: &'c authn::Context, - roleset: &'d mut RoleSet, - ) -> BoxFuture<'e, Result<(), Error>> - where - 'a: 'e, - 'b: 'e, - 'c: 'e, - 'd: 'e; + fn load_roles<'fut>( + &'fut self, + opctx: &'fut OpContext, + authn: &'fut authn::Context, + roleset: &'fut mut RoleSet, + ) -> BoxFuture<'fut, Result<(), Error>>; /// Invoked on authz failure to determine the final authz result /// @@ -227,7 +222,7 @@ mod test { _resource_type: ResourceType, _resource_id: Uuid, ) -> Result, Error> { - todo!(); + unimplemented!("This test is not expected to access the database"); } } @@ -257,17 +252,12 @@ mod test { #[derive(Clone, PolarClass)] struct UnregisteredResource; impl AuthorizedResource for UnregisteredResource { - fn load_roles<'a, 'b, 'c, 'd, 'e>( - &'a self, - _: &'b OpContext, - _: &'c authn::Context, - _: &'d mut RoleSet, - ) -> futures::future::BoxFuture<'e, Result<(), Error>> - where - 'a: 'e, - 'b: 'e, - 'c: 'e, - 'd: 'e, + fn load_roles<'fut>( + &'fut self, + _: &'fut OpContext, + _: &'fut authn::Context, + _: &'fut mut RoleSet, + ) -> futures::future::BoxFuture<'fut, Result<(), Error>> { // authorize() shouldn't get far enough to call this. unimplemented!(); diff --git a/nexus/auth/src/authz/oso_generic.rs b/nexus/auth/src/authz/oso_generic.rs index 7ad48c1dec..383a06e985 100644 --- a/nexus/auth/src/authz/oso_generic.rs +++ b/nexus/auth/src/authz/oso_generic.rs @@ -266,18 +266,12 @@ impl oso::PolarClass for Database { } impl AuthorizedResource for Database { - fn load_roles<'a, 'b, 'c, 'd, 'e>( - &'a self, - _: &'b OpContext, - _: &'c authn::Context, - _: &'d mut RoleSet, - ) -> BoxFuture<'e, Result<(), Error>> - where - 'a: 'e, - 'b: 'e, - 'c: 'e, - 'd: 'e, - { + fn load_roles<'fut>( + &'fut self, + _: &'fut OpContext, + _: &'fut authn::Context, + _: &'fut mut RoleSet, + ) -> BoxFuture<'fut, Result<(), Error>> { // We don't use (database) roles to grant access to the database. The // role assignment is hardcoded for all authenticated users. See the // "has_role" Polar method above. diff --git a/nexus/auth/src/context.rs b/nexus/auth/src/context.rs index 01516359c3..0aac0900c5 100644 --- a/nexus/auth/src/context.rs +++ b/nexus/auth/src/context.rs @@ -380,7 +380,7 @@ mod test { _resource_type: ResourceType, _resource_id: Uuid, ) -> Result, Error> { - todo!(); + unimplemented!("This test is not expected to access the database"); } } diff --git a/nexus/db-fixed-data/Cargo.toml b/nexus/db-fixed-data/Cargo.toml index d24bf25c7d..486df15686 100644 --- a/nexus/db-fixed-data/Cargo.toml +++ b/nexus/db-fixed-data/Cargo.toml @@ -3,6 +3,7 @@ name = "nexus-db-fixed-data" version = "0.1.0" edition = "2021" license = "MPL-2.0" +description = "Hard-coded database data, including defaults and built-ins" [lints] workspace = true