Skip to content

Add effective fleet and silo role to /v1/me response #8515

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
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
14 changes: 14 additions & 0 deletions nexus/auth/src/authz/context.rs
Original file line number Diff line number Diff line change
Expand Up @@ -149,6 +149,20 @@ impl Context {
}
}
}

/// Get the opctx actor's roles on a given resource
pub async fn get_roles<Resource>(
&self,
opctx: &OpContext,
resource: Resource,
) -> Result<RoleSet, Error>
where
Resource: AuthorizedResource + Clone,
{
let mut roles = RoleSet::new();
resource.load_roles(opctx, &self.authn, &mut roles).await?;
Ok(roles)
}
}

pub trait AuthorizedResource: oso::ToPolar + Send + Sync + 'static {
Expand Down
24 changes: 23 additions & 1 deletion nexus/auth/src/authz/roles.rs
Original file line number Diff line number Diff line change
Expand Up @@ -49,7 +49,7 @@ use uuid::Uuid;
/// For more on roles, see dbinit.rs.
#[derive(Clone, Debug)]
pub struct RoleSet {
roles: BTreeSet<(ResourceType, Uuid, String)>,
pub roles: BTreeSet<(ResourceType, Uuid, String)>,
}

impl RoleSet {
Expand Down Expand Up @@ -82,6 +82,28 @@ impl RoleSet {
String::from(role_name),
));
}

/// Effective, i.e., strongest, role in set for specified resource
pub fn effective_role(
&self,
resource_type: &ResourceType,
resource_id: &Uuid,
) -> Option<String> {
self.roles
.iter()
.filter(|(typ, id, _)| typ == resource_type && id == resource_id)
.max_by_key(|(_, _, role)| role_num(role))
.map(|(_, _, role)| role.clone())
}
}

fn role_num(role: &str) -> u8 {
match role {
"viewer" => 1,
"collaborator" => 2,
"admin" => 3,
_ => 0,
}
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this is the worst part

Copy link
Contributor Author

@david-crespo david-crespo Jul 3, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

My other random PR #7477 is relevant here, especially the suggestion of representing roles as an enum rather than a string (now that we know roles don't need to be free-form), which would make this a lot less hacky.

}

pub async fn load_roles_for_resource_tree<R>(
Expand Down
11 changes: 11 additions & 0 deletions nexus/auth/src/context.rs
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ use super::authz;
use crate::authn::ConsoleSessionWithSiloId;
use crate::authn::external::session_cookie::Session;
use crate::authz::AuthorizedResource;
use crate::authz::RoleSet;
use crate::storage::Storage;
use chrono::{DateTime, Utc};
use omicron_common::api::external::Error;
Expand Down Expand Up @@ -285,6 +286,16 @@ impl OpContext {
result
}

pub async fn get_roles<Resource>(
&self,
resource: &Resource,
) -> Result<RoleSet, Error>
where
Resource: AuthorizedResource + Debug + Clone,
{
self.authz.get_roles(self, resource.clone()).await
}

/// Returns an error if we're currently in a context where expensive or
/// complex operations should not be allowed
///
Expand Down
25 changes: 23 additions & 2 deletions nexus/src/external_api/http_entrypoints.rs
Original file line number Diff line number Diff line change
Expand Up @@ -39,6 +39,7 @@ use dropshot::{WebsocketChannelResult, WebsocketConnection};
use dropshot::{http_response_found, http_response_see_other};
use http::{Response, StatusCode, header};
use ipnetwork::IpNetwork;
use nexus_auth::authz::{ApiResource, ApiResourceWithRoles};
use nexus_db_lookup::lookup::ImageLookup;
use nexus_db_lookup::lookup::ImageParentLookup;
use nexus_db_queries::authn::external::session_cookie::{self, SessionStore};
Expand Down Expand Up @@ -6942,11 +6943,31 @@ impl NexusExternalApi for NexusExternalApiImpl {
let handler = async {
let opctx =
crate::context::op_context_for_external_api(&rqctx).await?;

let user = nexus.silo_user_fetch_self(&opctx).await?;
let (_, silo) = nexus.current_silo_lookup(&opctx)?.fetch().await?;
let (authz_silo, db_silo) =
nexus.current_silo_lookup(&opctx)?.fetch().await?;

let silo_roles = opctx.get_roles(&authz_silo).await;
Ok(HttpResponseOk(views::CurrentUser {
user: user.into(),
silo_name: silo.name().clone(),
silo_name: db_silo.name().clone(),
fleet_role: silo_roles.clone().map_or(None, |roleset| {
roleset.effective_role(
&authz::FLEET.resource_type(),
&authz::FLEET.resource_id(),
)
}),
// TODO: one weird wrinkle here is that the polar policy gives
// every user read perms on their own silo, but that is not
// equivalent to viewer because it's not inherited, so we are
// probably fine leaving the no-role case as None
silo_role: silo_roles.map_or(None, |roleset| {
roleset.effective_role(
&authz_silo.resource_type(),
&authz_silo.resource_id(),
)
}),
}))
};
apictx
Expand Down
44 changes: 39 additions & 5 deletions nexus/test-utils/src/resource_helpers.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1106,11 +1106,8 @@ pub async fn grant_iam<T>(
let existing_policy: shared::Policy<T> =
NexusRequest::object_get(client, &policy_url)
.authn_as(run_as.clone())
.execute()
.await
.expect("failed to fetch policy")
.parsed_body()
.expect("failed to parse policy");
.execute_and_parse_unwrap()
.await;
let new_role_assignment = shared::RoleAssignment {
identity_type: IdentityType::SiloUser,
identity_id: grant_user,
Expand All @@ -1132,6 +1129,43 @@ pub async fn grant_iam<T>(
.expect("failed to update policy");
}

/// Same as `grant_iam`, except that instead of adding a role, we are
/// filtering out any matching roles for the user on that resource
pub async fn revoke_iam<T>(
client: &ClientTestContext,
grant_resource_url: &str,
grant_role: T,
grant_user: Uuid,
run_as: AuthnMode,
) where
T: serde::Serialize + serde::de::DeserializeOwned + PartialEq,
{
let policy_url = format!("{}/policy", grant_resource_url);
let existing_policy: shared::Policy<T> =
NexusRequest::object_get(client, &policy_url)
.authn_as(run_as.clone())
.execute_and_parse_unwrap()
.await;

let filtered_role_assignments = existing_policy
.role_assignments
.into_iter()
.filter(|ra| {
!(ra.identity_id == grant_user && ra.role_name == grant_role)
})
.collect();

let new_policy =
shared::Policy { role_assignments: filtered_role_assignments };

// TODO-correctness use etag when we have it
NexusRequest::object_put(client, &policy_url, Some(&new_policy))
.authn_as(run_as)
.execute()
.await
.expect("failed to update policy");
}

pub async fn project_get(
client: &ClientTestContext,
project_url: &str,
Expand Down
Loading
Loading