Skip to content

Use typed UUIDs for silo user and group #8803

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 3 commits 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
2 changes: 2 additions & 0 deletions Cargo.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

1 change: 1 addition & 0 deletions clients/oxide-client/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -26,3 +26,4 @@ thiserror.workspace = true
tokio = { workspace = true, features = [ "net" ] }
uuid.workspace = true
omicron-workspace-hack.workspace = true
omicron-uuid-kinds.workspace = true
6 changes: 6 additions & 0 deletions clients/oxide-client/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,12 @@ progenitor::generate_api!(
spec = "../../openapi/nexus.json",
interface = Builder,
tags = Separate,
replace = {
TypedUuidForAlertReceiverKind = omicron_uuid_kinds::AlertReceiverUuid,
TypedUuidForAlertKind = omicron_uuid_kinds::AlertUuid,
TypedUuidForSiloUserKind = omicron_uuid_kinds::SiloUserUuid,
TypedUuidForSiloGroupKind = omicron_uuid_kinds::SiloGroupUuid,
},
);

/// Custom reqwest DNS resolver intended for use with the Oxide client
Expand Down
6 changes: 5 additions & 1 deletion nexus/auth/src/authn/external/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@ use crate::authn;
use crate::probes;
use async_trait::async_trait;
use authn::Reason;
use omicron_uuid_kinds::SiloUserUuid;
use slog::trace;
use std::borrow::Borrow;
use uuid::Uuid;
Expand Down Expand Up @@ -153,7 +154,10 @@ pub enum SchemeResult {
/// A context that can look up a Silo user's Silo.
#[async_trait]
pub trait SiloUserSilo {
async fn silo_user_silo(&self, silo_user_id: Uuid) -> Result<Uuid, Reason>;
async fn silo_user_silo(
&self,
silo_user_id: SiloUserUuid,
) -> Result<Uuid, Reason>;
}

#[cfg(test)]
Expand Down
14 changes: 8 additions & 6 deletions nexus/auth/src/authn/external/session_cookie.rs
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@ use dropshot::HttpError;
use http::HeaderValue;
use nexus_types::authn::cookies::parse_cookies;
use omicron_uuid_kinds::ConsoleSessionUuid;
use omicron_uuid_kinds::SiloUserUuid;
use slog::debug;
use uuid::Uuid;

Expand All @@ -22,7 +23,7 @@ use uuid::Uuid;

pub trait Session {
fn id(&self) -> ConsoleSessionUuid;
fn silo_user_id(&self) -> Uuid;
fn silo_user_id(&self) -> SiloUserUuid;
fn silo_id(&self) -> Uuid;
fn time_last_used(&self) -> DateTime<Utc>;
fn time_created(&self) -> DateTime<Utc>;
Expand Down Expand Up @@ -202,6 +203,7 @@ mod test {
use chrono::{DateTime, Duration, Utc};
use http;
use omicron_uuid_kinds::ConsoleSessionUuid;
use omicron_uuid_kinds::SiloUserUuid;
use slog;
use std::sync::Mutex;
use uuid::Uuid;
Expand All @@ -216,7 +218,7 @@ mod test {
struct FakeSession {
id: ConsoleSessionUuid,
token: String,
silo_user_id: Uuid,
silo_user_id: SiloUserUuid,
silo_id: Uuid,
time_created: DateTime<Utc>,
time_last_used: DateTime<Utc>,
Expand All @@ -226,7 +228,7 @@ mod test {
fn id(&self) -> ConsoleSessionUuid {
self.id
}
fn silo_user_id(&self) -> Uuid {
fn silo_user_id(&self) -> SiloUserUuid {
self.silo_user_id
}
fn silo_id(&self) -> Uuid {
Expand Down Expand Up @@ -331,7 +333,7 @@ mod test {
sessions: Mutex::new(vec![FakeSession {
id: ConsoleSessionUuid::new_v4(),
token: "abc".to_string(),
silo_user_id: Uuid::new_v4(),
silo_user_id: SiloUserUuid::new_v4(),
silo_id: Uuid::new_v4(),
time_last_used: Utc::now() - Duration::hours(2),
time_created: Utc::now() - Duration::hours(2),
Expand All @@ -357,7 +359,7 @@ mod test {
sessions: Mutex::new(vec![FakeSession {
id: ConsoleSessionUuid::new_v4(),
token: "abc".to_string(),
silo_user_id: Uuid::new_v4(),
silo_user_id: SiloUserUuid::new_v4(),
silo_id: Uuid::new_v4(),
time_last_used: Utc::now(),
time_created: Utc::now() - Duration::hours(20),
Expand All @@ -384,7 +386,7 @@ mod test {
sessions: Mutex::new(vec![FakeSession {
id: ConsoleSessionUuid::new_v4(),
token: "abc".to_string(),
silo_user_id: Uuid::new_v4(),
silo_user_id: SiloUserUuid::new_v4(),
silo_id: Uuid::new_v4(),
time_last_used,
time_created: Utc::now(),
Expand Down
16 changes: 9 additions & 7 deletions nexus/auth/src/authn/external/spoof.rs
Original file line number Diff line number Diff line change
Expand Up @@ -18,8 +18,9 @@ use anyhow::anyhow;
use async_trait::async_trait;
use headers::HeaderMapExt;
use headers::authorization::{Authorization, Bearer};
use omicron_uuid_kinds::SiloUserUuid;
use slog::debug;
use uuid::Uuid;
use std::str::FromStr;

// This scheme is intended for demos, development, and testing until we have a
// more automatable identity provider that can be used for those purposes.
Expand Down Expand Up @@ -118,7 +119,7 @@ where

fn authn_spoof_parse_id(
raw_value: Option<&Authorization<Bearer>>,
) -> Result<Option<Uuid>, Reason> {
) -> Result<Option<SiloUserUuid>, Reason> {
let token = match raw_value {
None => return Ok(None),
Some(bearer) => bearer.token(),
Expand All @@ -142,15 +143,15 @@ fn authn_spoof_parse_id(
});
}

Uuid::parse_str(str_value)
SiloUserUuid::from_str(str_value)
.context("parsing header value as UUID")
.map(|silo_user_id| Some(silo_user_id))
.map_err(|source| Reason::BadFormat { source })
}

/// Returns a value of the `Authorization` header for this actor that will be
/// accepted using this scheme
pub fn make_header_value(id: Uuid) -> Authorization<Bearer> {
pub fn make_header_value<T: std::fmt::Display>(id: T) -> Authorization<Bearer> {
make_header_value_str(&id.to_string()).unwrap()
}

Expand Down Expand Up @@ -193,6 +194,7 @@ mod test {
use headers::HeaderMapExt;
use headers::authorization::Bearer;
use headers::authorization::Credentials;
use omicron_uuid_kinds::SiloUserUuid;
use uuid::Uuid;

#[test]
Expand Down Expand Up @@ -243,14 +245,14 @@ mod test {
#[test]
fn test_spoof_header_valid() {
let test_uuid_str = "37b56e4f-8c60-453b-a37e-99be6efe8a89";
let test_uuid = test_uuid_str.parse::<Uuid>().unwrap();
let test_uuid = test_uuid_str.parse::<SiloUserUuid>().unwrap();
let test_header = make_header_value(test_uuid);

// Success case: the client provided a valid uuid in the header.
let success_case = authn_spoof_parse_id(Some(&test_header));
match success_case {
Ok(Some(actor_id)) => {
assert_eq!(actor_id, test_uuid);
Ok(Some(silo_user_id)) => {
assert_eq!(silo_user_id, test_uuid);
}
_ => {
assert!(false);
Expand Down
42 changes: 22 additions & 20 deletions nexus/auth/src/authn/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -44,6 +44,8 @@ use nexus_types::external_api::shared::FleetRole;
use nexus_types::external_api::shared::SiloRole;
use nexus_types::identity::Asset;
use omicron_common::api::external::LookupType;
use omicron_uuid_kinds::BuiltInUserUuid;
use omicron_uuid_kinds::SiloUserUuid;
use serde::Deserialize;
use serde::Serialize;
use std::collections::BTreeMap;
Expand Down Expand Up @@ -197,7 +199,7 @@ impl Context {
Context::context_for_builtin_user(USER_SERVICE_BALANCER.id)
}

fn context_for_builtin_user(user_builtin_id: Uuid) -> Context {
fn context_for_builtin_user(user_builtin_id: BuiltInUserUuid) -> Context {
Context {
kind: Kind::Authenticated(
Details { actor: Actor::UserBuiltin { user_builtin_id } },
Expand Down Expand Up @@ -239,7 +241,7 @@ impl Context {
/// Returns an authenticated context for the specific Silo user. Not marked
/// as #[cfg(test)] so that this is available in integration tests.
pub fn for_test_user(
silo_user_id: Uuid,
silo_user_id: SiloUserUuid,
silo_id: Uuid,
silo_authn_policy: SiloAuthnPolicy,
) -> Context {
Expand Down Expand Up @@ -311,35 +313,35 @@ mod test {
// The privileges are (or will be) verified in authz tests.
let authn = Context::privileged_test_user();
let actor = authn.actor().unwrap();
assert_eq!(actor.actor_id(), USER_TEST_PRIVILEGED.id());
assert_eq!(actor.silo_user_id(), Some(USER_TEST_PRIVILEGED.id()));

let authn = Context::unprivileged_test_user();
let actor = authn.actor().unwrap();
assert_eq!(actor.actor_id(), USER_TEST_UNPRIVILEGED.id());
assert_eq!(actor.silo_user_id(), Some(USER_TEST_UNPRIVILEGED.id()));

let authn = Context::internal_read();
let actor = authn.actor().unwrap();
assert_eq!(actor.actor_id(), USER_INTERNAL_READ.id);
assert_eq!(actor.built_in_user_id(), Some(USER_INTERNAL_READ.id));

let authn = Context::external_authn();
let actor = authn.actor().unwrap();
assert_eq!(actor.actor_id(), USER_EXTERNAL_AUTHN.id);
assert_eq!(actor.built_in_user_id(), Some(USER_EXTERNAL_AUTHN.id));

let authn = Context::internal_db_init();
let actor = authn.actor().unwrap();
assert_eq!(actor.actor_id(), USER_DB_INIT.id);
assert_eq!(actor.built_in_user_id(), Some(USER_DB_INIT.id));

let authn = Context::internal_service_balancer();
let actor = authn.actor().unwrap();
assert_eq!(actor.actor_id(), USER_SERVICE_BALANCER.id);
assert_eq!(actor.built_in_user_id(), Some(USER_SERVICE_BALANCER.id));

let authn = Context::internal_saga_recovery();
let actor = authn.actor().unwrap();
assert_eq!(actor.actor_id(), USER_SAGA_RECOVERY.id);
assert_eq!(actor.built_in_user_id(), Some(USER_SAGA_RECOVERY.id));

let authn = Context::internal_api();
let actor = authn.actor().unwrap();
assert_eq!(actor.actor_id(), USER_INTERNAL_API.id);
assert_eq!(actor.built_in_user_id(), Some(USER_INTERNAL_API.id));
}
}

Expand All @@ -366,31 +368,31 @@ pub struct Details {
/// Who is performing an operation
#[derive(Clone, Copy, Deserialize, Eq, PartialEq, Serialize)]
pub enum Actor {
UserBuiltin { user_builtin_id: Uuid },
SiloUser { silo_user_id: Uuid, silo_id: Uuid },
UserBuiltin { user_builtin_id: BuiltInUserUuid },
SiloUser { silo_user_id: SiloUserUuid, silo_id: Uuid },
}

impl Actor {
pub fn actor_id(&self) -> Uuid {
match self {
Actor::UserBuiltin { user_builtin_id, .. } => *user_builtin_id,
Actor::SiloUser { silo_user_id, .. } => *silo_user_id,
}
}

pub fn silo_id(&self) -> Option<Uuid> {
match self {
Actor::UserBuiltin { .. } => None,
Actor::SiloUser { silo_id, .. } => Some(*silo_id),
}
}

pub fn silo_user_id(&self) -> Option<Uuid> {
pub fn silo_user_id(&self) -> Option<SiloUserUuid> {
match self {
Actor::UserBuiltin { .. } => None,
Actor::SiloUser { silo_user_id, .. } => Some(*silo_user_id),
}
}

pub fn built_in_user_id(&self) -> Option<BuiltInUserUuid> {
match self {
Actor::UserBuiltin { user_builtin_id } => Some(*user_builtin_id),
Actor::SiloUser { .. } => None,
}
}
}

impl From<&Actor> for nexus_db_model::IdentityType {
Expand Down
44 changes: 27 additions & 17 deletions nexus/auth/src/authz/actor.rs
Original file line number Diff line number Diff line change
Expand Up @@ -38,8 +38,7 @@ impl oso::PolarClass for AnyActor {
})
.add_attribute_getter("authn_actor", |a: &AnyActor| {
a.actor.map(|actor| AuthenticatedActor {
actor_id: actor.actor_id(),
silo_id: actor.silo_id(),
actor,
roles: a.roles.clone(),
silo_policy: a.silo_policy.clone(),
})
Expand All @@ -50,8 +49,7 @@ impl oso::PolarClass for AnyActor {
/// Represents an authenticated [`authn::Context`] for Polar
#[derive(Clone, Debug)]
pub struct AuthenticatedActor {
actor_id: Uuid,
silo_id: Option<Uuid>,
actor: authn::Actor,
roles: RoleSet,
silo_policy: Option<authn::SiloAuthnPolicy>,
}
Expand Down Expand Up @@ -94,7 +92,7 @@ impl AuthenticatedActor {

impl PartialEq for AuthenticatedActor {
fn eq(&self, other: &Self) -> bool {
self.actor_id == other.actor_id
self.actor == other.actor
}
}

Expand All @@ -106,30 +104,36 @@ impl oso::PolarClass for AuthenticatedActor {
.with_equality_check()
.add_constant(
AuthenticatedActor {
actor_id: authn::USER_DB_INIT.id,
silo_id: None,
actor: authn::Actor::UserBuiltin {
user_builtin_id: authn::USER_DB_INIT.id,
},
roles: RoleSet::new(),
silo_policy: None,
},
"USER_DB_INIT",
)
.add_constant(
AuthenticatedActor {
actor_id: authn::USER_INTERNAL_API.id,
silo_id: None,
actor: authn::Actor::UserBuiltin {
user_builtin_id: authn::USER_INTERNAL_API.id,
},
roles: RoleSet::new(),
silo_policy: None,
},
"USER_INTERNAL_API",
)
.add_attribute_getter("silo", |a: &AuthenticatedActor| {
a.silo_id.map(|silo_id| {
super::Silo::new(
super::FLEET,
silo_id,
LookupType::ById(silo_id),
)
})
match a.actor {
authn::Actor::SiloUser { silo_id, .. } => {
Some(super::Silo::new(
super::FLEET,
silo_id,
LookupType::ById(silo_id),
))
}

authn::Actor::UserBuiltin { .. } => None,
}
})
.add_method(
"confers_fleet_role",
Expand All @@ -139,7 +143,13 @@ impl oso::PolarClass for AuthenticatedActor {
)
.add_method(
"equals_silo_user",
|a: &AuthenticatedActor, u: SiloUser| a.actor_id == u.id(),
|a: &AuthenticatedActor, u: SiloUser| match a.actor {
authn::Actor::SiloUser { silo_user_id, .. } => {
silo_user_id == u.id()
}

authn::Actor::UserBuiltin { .. } => false,
},
)
}
}
Loading
Loading