Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
373 changes: 213 additions & 160 deletions nexus/auth/src/authz/api_resources.rs

Large diffs are not rendered by default.

27 changes: 13 additions & 14 deletions nexus/auth/src/authz/context.rs
Original file line number Diff line number Diff line change
Expand Up @@ -11,13 +11,13 @@ use crate::authz::Action;
use crate::authz::oso_generic;
use crate::context::OpContext;
use crate::storage::Storage;
use futures::future::BoxFuture;
use omicron_common::api::external::Error;
use omicron_common::bail_unless;
use oso::Oso;
use oso::OsoError;
use slog::debug;
use std::collections::BTreeSet;
use std::future::Future;
use std::sync::Arc;

/// Server-wide authorization context
Expand Down Expand Up @@ -164,12 +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<'fut>(
&'fut self,
opctx: &'fut OpContext,
authn: &'fut authn::Context,
roleset: &'fut mut RoleSet,
) -> BoxFuture<'fut, Result<(), Error>>;
fn load_roles(
&self,
opctx: &OpContext,
authn: &authn::Context,
roleset: &mut RoleSet,
) -> impl Future<Output = Result<(), Error>>;

/// Invoked on authz failure to determine the final authz result
///
Expand Down Expand Up @@ -252,13 +252,12 @@ mod test {
#[derive(Clone, PolarClass)]
struct UnregisteredResource;
impl AuthorizedResource for UnregisteredResource {
fn load_roles<'fut>(
&'fut self,
_: &'fut OpContext,
_: &'fut authn::Context,
_: &'fut mut RoleSet,
) -> futures::future::BoxFuture<'fut, Result<(), Error>>
{
async fn load_roles(
&self,
_: &OpContext,
_: &authn::Context,
_: &mut RoleSet,
) -> Result<(), Error> {
// authorize() shouldn't get far enough to call this.
unimplemented!();
}
Expand Down
16 changes: 7 additions & 9 deletions nexus/auth/src/authz/oso_generic.rs
Original file line number Diff line number Diff line change
Expand Up @@ -14,8 +14,6 @@ use crate::authn;
use crate::context::OpContext;
use anyhow::Context;
use anyhow::ensure;
use futures::FutureExt;
use futures::future::BoxFuture;
use omicron_common::api::external::Error;
use oso::Oso;
use oso::PolarClass;
Expand Down Expand Up @@ -288,12 +286,12 @@ impl oso::PolarClass for Database {
}

impl AuthorizedResource for Database {
fn load_roles<'fut>(
&'fut self,
_: &'fut OpContext,
_: &'fut authn::Context,
_: &'fut mut RoleSet,
) -> BoxFuture<'fut, Result<(), Error>> {
async fn load_roles(
&self,
_opctx: &OpContext,
_authn: &authn::Context,
_roleset: &mut RoleSet,
) -> 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 All @@ -303,7 +301,7 @@ impl AuthorizedResource for Database {
// the type signature of roles supported by RoleSet. RoleSet is really
// for roles on database objects -- it assumes they have a ResourceType
// and id, neither of which is true for `Database`.
futures::future::ready(Ok(())).boxed()
Ok(())
}

fn on_unauthorized(
Expand Down
5 changes: 3 additions & 2 deletions nexus/auth/src/authz/roles.rs
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,8 @@
//! request, and we don't want that thread to block while we hit the database.
//! Both of these issues could be addressed with considerably more work.

use super::api_resources::ApiResource;
use super::api_resources::ApiResourceWithParent;
use super::context::AuthorizedResource;
use crate::authn;
use crate::context::OpContext;
use omicron_common::api::external::Error;
Expand Down Expand Up @@ -91,7 +92,7 @@ pub async fn load_roles_for_resource_tree<R>(
roleset: &mut RoleSet,
) -> Result<(), Error>
where
R: ApiResource,
R: ApiResourceWithParent,
{
// If roles can be assigned directly on this resource, load them.
if let Some(with_roles) = resource.as_resource_with_roles() {
Expand Down
9 changes: 6 additions & 3 deletions nexus/authz-macros/outputs/instance.txt
Original file line number Diff line number Diff line change
Expand Up @@ -64,9 +64,6 @@ impl oso::PolarClass for Instance {
}
}
impl ApiResource for Instance {
fn parent(&self) -> Option<&dyn AuthorizedResource> {
Some(&self.parent)
}
fn resource_type(&self) -> ResourceType {
ResourceType::Instance
}
Expand All @@ -77,3 +74,9 @@ impl ApiResource for Instance {
None
}
}
impl ApiResourceWithParent for Instance {
type Parent = Project;
fn parent(&self) -> Option<&Project> {
Some(&self.parent)
}
}
9 changes: 6 additions & 3 deletions nexus/authz-macros/outputs/organization.txt
Original file line number Diff line number Diff line change
Expand Up @@ -60,9 +60,6 @@ impl oso::PolarClass for Organization {
}
}
impl ApiResource for Organization {
fn parent(&self) -> Option<&dyn AuthorizedResource> {
Some(&self.parent)
}
fn resource_type(&self) -> ResourceType {
ResourceType::Organization
}
Expand All @@ -73,3 +70,9 @@ impl ApiResource for Organization {
None
}
}
impl ApiResourceWithParent for Organization {
type Parent = Fleet;
fn parent(&self) -> Option<&Fleet> {
Some(&self.parent)
}
}
9 changes: 6 additions & 3 deletions nexus/authz-macros/outputs/rack.txt
Original file line number Diff line number Diff line change
Expand Up @@ -60,9 +60,6 @@ impl oso::PolarClass for Rack {
}
}
impl ApiResource for Rack {
fn parent(&self) -> Option<&dyn AuthorizedResource> {
Some(&self.parent)
}
fn resource_type(&self) -> ResourceType {
ResourceType::Rack
}
Expand All @@ -73,3 +70,9 @@ impl ApiResource for Rack {
None
}
}
impl ApiResourceWithParent for Rack {
type Parent = Fleet;
fn parent(&self) -> Option<&Fleet> {
Some(&self.parent)
}
}
12 changes: 8 additions & 4 deletions nexus/authz-macros/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -606,10 +606,6 @@ fn do_authz_resource(
}

impl ApiResource for #resource_name {
fn parent(&self) -> Option<&dyn AuthorizedResource> {
Some(&self.parent)
}

fn resource_type(&self) -> ResourceType {
ResourceType::#resource_name
}
Expand All @@ -625,6 +621,14 @@ fn do_authz_resource(
}
}

impl ApiResourceWithParent for #resource_name {
type Parent = #parent_resource_name;

fn parent(&self) -> Option<&#parent_resource_name> {
Some(&self.parent)
}
}

#api_resource_roles_trait
})
}
Expand Down
2 changes: 1 addition & 1 deletion nexus/db-queries/src/policy_test/coverage.rs
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,7 @@ impl Coverage {

/// Record that the Polar class associated with `covered` is covered by the
/// test
pub fn covered(&mut self, covered: &dyn AuthorizedResource) {
pub fn covered<R: AuthorizedResource>(&mut self, covered: &R) {
self.covered_class(covered.polar_class())
}

Expand Down
11 changes: 7 additions & 4 deletions nexus/db-queries/src/policy_test/resource_builder.rs
Original file line number Diff line number Diff line change
Expand Up @@ -8,10 +8,10 @@
use super::coverage::Coverage;
use crate::db;
use crate::db::datastore::SiloUserApiOnly;
use authz::ApiResource;
use futures::FutureExt;
use futures::future::BoxFuture;
use nexus_auth::authz;
use nexus_auth::authz::ApiResource;
use nexus_auth::authz::ApiResourceWithRolesType;
use nexus_auth::authz::AuthorizedResource;
use nexus_auth::context::OpContext;
Expand Down Expand Up @@ -69,7 +69,10 @@ impl<'a> ResourceBuilder<'a> {

/// Register a new resource for later testing, with no associated users or
/// role assignments
pub fn new_resource<T: DynAuthorizedResource>(&mut self, resource: T) {
pub fn new_resource<T>(&mut self, resource: T)
where
T: DynAuthorizedResource + AuthorizedResource,
{
self.coverage.covered(&resource);
self.resources.push(Arc::new(resource));
}
Expand All @@ -79,8 +82,8 @@ impl<'a> ResourceBuilder<'a> {
pub async fn new_resource_with_users<T>(&mut self, resource: T)
where
T: DynAuthorizedResource
+ ApiResourceWithRolesType
+ AuthorizedResource
+ ApiResourceWithRolesType
+ Clone,
T::AllowedRoles: IntoEnumIterator,
{
Expand Down Expand Up @@ -179,7 +182,7 @@ impl ResourceSet {
/// all of them. (We could also change `authorize()` to be dynamically-
/// dispatched. This would be a much more sprawling change. And it's not clear
/// that our use case has much application outside of a test like this.)
pub trait DynAuthorizedResource: AuthorizedResource + std::fmt::Debug {
pub trait DynAuthorizedResource: std::fmt::Debug + Send + Sync {
fn do_authorize<'a, 'b>(
&'a self,
opctx: &'b OpContext,
Expand Down
Loading