Skip to content

Commit

Permalink
review feedback
Browse files Browse the repository at this point in the history
  • Loading branch information
smklein committed Jun 1, 2024
1 parent 9967156 commit deb573e
Show file tree
Hide file tree
Showing 5 changed files with 81 additions and 156 deletions.
180 changes: 60 additions & 120 deletions nexus/auth/src/authz/api_resources.rs
Original file line number Diff line number Diff line change
Expand Up @@ -109,18 +109,12 @@ impl<T> 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()
}

Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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()
}

Expand Down Expand Up @@ -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()
}

Expand Down Expand Up @@ -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).
Expand Down Expand Up @@ -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.
Expand Down Expand Up @@ -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()
}

Expand Down Expand Up @@ -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)
Expand Down Expand Up @@ -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)
Expand Down Expand Up @@ -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)
Expand Down
36 changes: 13 additions & 23 deletions nexus/auth/src/authz/context.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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
///
Expand Down Expand Up @@ -227,7 +222,7 @@ mod test {
_resource_type: ResourceType,
_resource_id: Uuid,
) -> Result<Vec<RoleAssignment>, Error> {
todo!();
unimplemented!("This test is not expected to access the database");
}
}

Expand Down Expand Up @@ -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!();
Expand Down
18 changes: 6 additions & 12 deletions nexus/auth/src/authz/oso_generic.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down
2 changes: 1 addition & 1 deletion nexus/auth/src/context.rs
Original file line number Diff line number Diff line change
Expand Up @@ -380,7 +380,7 @@ mod test {
_resource_type: ResourceType,
_resource_id: Uuid,
) -> Result<Vec<RoleAssignment>, Error> {
todo!();
unimplemented!("This test is not expected to access the database");
}
}

Expand Down
1 change: 1 addition & 0 deletions nexus/db-fixed-data/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down

0 comments on commit deb573e

Please sign in to comment.