Skip to content

Commit 33ad127

Browse files
authored
Use typed UUIDs for silo user and group (#8803)
This commit changes SiloUser and SiloGroup to use typed UUIDs. The biggest reason that this couldn't happen without a bunch of work was the `lookup_resource` macro: for a resource like SshKey, it looked like ``` lookup_resource! { name = "SshKey", ancestors = [ "Silo", "SiloUser" ], lookup_by_name = true, soft_deletes = true, primary_key_columns = [ { column_name = "id", rust_type = Uuid } ] } ``` One of the methods that the `lookup_resource` macro generates will create a lookup for ancestors of the resource, and this was one using the type returned from the corresponding authz resource's `id` method: ``` quote! { .filter(dsl::#parent_id.eq(#parent_authz_name.id())) }, ``` Changing SiloUser to use a typed UUID in the authz resource: ``` authz_resource! { name = "SiloUser", parent = "Silo", primary_key = { uuid_kind = SiloUserKind }, <-- here roles_allowed = false, polar_snippet = Custom, } ``` and changing the `SiloUser` db model to use `DbTypedUuid` meant that a call to `to_db_typed_uuid` was required. The lookup_resource macro has no type information from the string "SiloUser", so this PR adds a check: if the ancestor string is suffixed with a '*', then the lookup_resource macro should assume that the `parent_id` is a typed UUID, and generate the call to `to_db_typed_uuid`. Most of the work after that was mechanical, changing Uuid to their typed equivalent, changing method argument types, etc etc. Some other related things made it into this PR: - UserBuiltIn now also uses a typed UUID as well, distinguishing them from silo users - Actor no longer has the `actor_id` method, instead requiring call sites to check which variant of Actor is being used - AuthenticatedActor stores the full Actor instead of only the actor id, leading to typed comparisons in its oso::PolarClass impl - User and Group path params are now typed
1 parent c2862a0 commit 33ad127

Some content is hidden

Large Commits have some content hidden by default. Use the searchbox below for content that may be hidden.

62 files changed

+839
-419
lines changed

Cargo.lock

Lines changed: 2 additions & 0 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

clients/oxide-client/Cargo.toml

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -26,3 +26,4 @@ thiserror.workspace = true
2626
tokio = { workspace = true, features = [ "net" ] }
2727
uuid.workspace = true
2828
omicron-workspace-hack.workspace = true
29+
omicron-uuid-kinds.workspace = true

nexus/auth/src/authn/external/mod.rs

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,7 @@ use crate::authn;
1010
use crate::probes;
1111
use async_trait::async_trait;
1212
use authn::Reason;
13+
use omicron_uuid_kinds::SiloUserUuid;
1314
use slog::trace;
1415
use std::borrow::Borrow;
1516
use uuid::Uuid;
@@ -153,7 +154,10 @@ pub enum SchemeResult {
153154
/// A context that can look up a Silo user's Silo.
154155
#[async_trait]
155156
pub trait SiloUserSilo {
156-
async fn silo_user_silo(&self, silo_user_id: Uuid) -> Result<Uuid, Reason>;
157+
async fn silo_user_silo(
158+
&self,
159+
silo_user_id: SiloUserUuid,
160+
) -> Result<Uuid, Reason>;
157161
}
158162

159163
#[cfg(test)]

nexus/auth/src/authn/external/session_cookie.rs

Lines changed: 8 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -14,6 +14,7 @@ use dropshot::HttpError;
1414
use http::HeaderValue;
1515
use nexus_types::authn::cookies::parse_cookies;
1616
use omicron_uuid_kinds::ConsoleSessionUuid;
17+
use omicron_uuid_kinds::SiloUserUuid;
1718
use slog::debug;
1819
use uuid::Uuid;
1920

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

2324
pub trait Session {
2425
fn id(&self) -> ConsoleSessionUuid;
25-
fn silo_user_id(&self) -> Uuid;
26+
fn silo_user_id(&self) -> SiloUserUuid;
2627
fn silo_id(&self) -> Uuid;
2728
fn time_last_used(&self) -> DateTime<Utc>;
2829
fn time_created(&self) -> DateTime<Utc>;
@@ -202,6 +203,7 @@ mod test {
202203
use chrono::{DateTime, Duration, Utc};
203204
use http;
204205
use omicron_uuid_kinds::ConsoleSessionUuid;
206+
use omicron_uuid_kinds::SiloUserUuid;
205207
use slog;
206208
use std::sync::Mutex;
207209
use uuid::Uuid;
@@ -216,7 +218,7 @@ mod test {
216218
struct FakeSession {
217219
id: ConsoleSessionUuid,
218220
token: String,
219-
silo_user_id: Uuid,
221+
silo_user_id: SiloUserUuid,
220222
silo_id: Uuid,
221223
time_created: DateTime<Utc>,
222224
time_last_used: DateTime<Utc>,
@@ -226,7 +228,7 @@ mod test {
226228
fn id(&self) -> ConsoleSessionUuid {
227229
self.id
228230
}
229-
fn silo_user_id(&self) -> Uuid {
231+
fn silo_user_id(&self) -> SiloUserUuid {
230232
self.silo_user_id
231233
}
232234
fn silo_id(&self) -> Uuid {
@@ -331,7 +333,7 @@ mod test {
331333
sessions: Mutex::new(vec![FakeSession {
332334
id: ConsoleSessionUuid::new_v4(),
333335
token: "abc".to_string(),
334-
silo_user_id: Uuid::new_v4(),
336+
silo_user_id: SiloUserUuid::new_v4(),
335337
silo_id: Uuid::new_v4(),
336338
time_last_used: Utc::now() - Duration::hours(2),
337339
time_created: Utc::now() - Duration::hours(2),
@@ -357,7 +359,7 @@ mod test {
357359
sessions: Mutex::new(vec![FakeSession {
358360
id: ConsoleSessionUuid::new_v4(),
359361
token: "abc".to_string(),
360-
silo_user_id: Uuid::new_v4(),
362+
silo_user_id: SiloUserUuid::new_v4(),
361363
silo_id: Uuid::new_v4(),
362364
time_last_used: Utc::now(),
363365
time_created: Utc::now() - Duration::hours(20),
@@ -384,7 +386,7 @@ mod test {
384386
sessions: Mutex::new(vec![FakeSession {
385387
id: ConsoleSessionUuid::new_v4(),
386388
token: "abc".to_string(),
387-
silo_user_id: Uuid::new_v4(),
389+
silo_user_id: SiloUserUuid::new_v4(),
388390
silo_id: Uuid::new_v4(),
389391
time_last_used,
390392
time_created: Utc::now(),

nexus/auth/src/authn/external/spoof.rs

Lines changed: 9 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -18,8 +18,9 @@ use anyhow::anyhow;
1818
use async_trait::async_trait;
1919
use headers::HeaderMapExt;
2020
use headers::authorization::{Authorization, Bearer};
21+
use omicron_uuid_kinds::SiloUserUuid;
2122
use slog::debug;
22-
use uuid::Uuid;
23+
use std::str::FromStr;
2324

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

119120
fn authn_spoof_parse_id(
120121
raw_value: Option<&Authorization<Bearer>>,
121-
) -> Result<Option<Uuid>, Reason> {
122+
) -> Result<Option<SiloUserUuid>, Reason> {
122123
let token = match raw_value {
123124
None => return Ok(None),
124125
Some(bearer) => bearer.token(),
@@ -142,15 +143,15 @@ fn authn_spoof_parse_id(
142143
});
143144
}
144145

145-
Uuid::parse_str(str_value)
146+
SiloUserUuid::from_str(str_value)
146147
.context("parsing header value as UUID")
147148
.map(|silo_user_id| Some(silo_user_id))
148149
.map_err(|source| Reason::BadFormat { source })
149150
}
150151

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

@@ -193,6 +194,7 @@ mod test {
193194
use headers::HeaderMapExt;
194195
use headers::authorization::Bearer;
195196
use headers::authorization::Credentials;
197+
use omicron_uuid_kinds::SiloUserUuid;
196198
use uuid::Uuid;
197199

198200
#[test]
@@ -243,14 +245,14 @@ mod test {
243245
#[test]
244246
fn test_spoof_header_valid() {
245247
let test_uuid_str = "37b56e4f-8c60-453b-a37e-99be6efe8a89";
246-
let test_uuid = test_uuid_str.parse::<Uuid>().unwrap();
248+
let test_uuid = test_uuid_str.parse::<SiloUserUuid>().unwrap();
247249
let test_header = make_header_value(test_uuid);
248250

249251
// Success case: the client provided a valid uuid in the header.
250252
let success_case = authn_spoof_parse_id(Some(&test_header));
251253
match success_case {
252-
Ok(Some(actor_id)) => {
253-
assert_eq!(actor_id, test_uuid);
254+
Ok(Some(silo_user_id)) => {
255+
assert_eq!(silo_user_id, test_uuid);
254256
}
255257
_ => {
256258
assert!(false);

nexus/auth/src/authn/mod.rs

Lines changed: 22 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -44,6 +44,8 @@ use nexus_types::external_api::shared::FleetRole;
4444
use nexus_types::external_api::shared::SiloRole;
4545
use nexus_types::identity::Asset;
4646
use omicron_common::api::external::LookupType;
47+
use omicron_uuid_kinds::BuiltInUserUuid;
48+
use omicron_uuid_kinds::SiloUserUuid;
4749
use serde::Deserialize;
4850
use serde::Serialize;
4951
use std::collections::BTreeMap;
@@ -197,7 +199,7 @@ impl Context {
197199
Context::context_for_builtin_user(USER_SERVICE_BALANCER.id)
198200
}
199201

200-
fn context_for_builtin_user(user_builtin_id: Uuid) -> Context {
202+
fn context_for_builtin_user(user_builtin_id: BuiltInUserUuid) -> Context {
201203
Context {
202204
kind: Kind::Authenticated(
203205
Details { actor: Actor::UserBuiltin { user_builtin_id } },
@@ -239,7 +241,7 @@ impl Context {
239241
/// Returns an authenticated context for the specific Silo user. Not marked
240242
/// as #[cfg(test)] so that this is available in integration tests.
241243
pub fn for_test_user(
242-
silo_user_id: Uuid,
244+
silo_user_id: SiloUserUuid,
243245
silo_id: Uuid,
244246
silo_authn_policy: SiloAuthnPolicy,
245247
) -> Context {
@@ -311,35 +313,35 @@ mod test {
311313
// The privileges are (or will be) verified in authz tests.
312314
let authn = Context::privileged_test_user();
313315
let actor = authn.actor().unwrap();
314-
assert_eq!(actor.actor_id(), USER_TEST_PRIVILEGED.id());
316+
assert_eq!(actor.silo_user_id(), Some(USER_TEST_PRIVILEGED.id()));
315317

316318
let authn = Context::unprivileged_test_user();
317319
let actor = authn.actor().unwrap();
318-
assert_eq!(actor.actor_id(), USER_TEST_UNPRIVILEGED.id());
320+
assert_eq!(actor.silo_user_id(), Some(USER_TEST_UNPRIVILEGED.id()));
319321

320322
let authn = Context::internal_read();
321323
let actor = authn.actor().unwrap();
322-
assert_eq!(actor.actor_id(), USER_INTERNAL_READ.id);
324+
assert_eq!(actor.built_in_user_id(), Some(USER_INTERNAL_READ.id));
323325

324326
let authn = Context::external_authn();
325327
let actor = authn.actor().unwrap();
326-
assert_eq!(actor.actor_id(), USER_EXTERNAL_AUTHN.id);
328+
assert_eq!(actor.built_in_user_id(), Some(USER_EXTERNAL_AUTHN.id));
327329

328330
let authn = Context::internal_db_init();
329331
let actor = authn.actor().unwrap();
330-
assert_eq!(actor.actor_id(), USER_DB_INIT.id);
332+
assert_eq!(actor.built_in_user_id(), Some(USER_DB_INIT.id));
331333

332334
let authn = Context::internal_service_balancer();
333335
let actor = authn.actor().unwrap();
334-
assert_eq!(actor.actor_id(), USER_SERVICE_BALANCER.id);
336+
assert_eq!(actor.built_in_user_id(), Some(USER_SERVICE_BALANCER.id));
335337

336338
let authn = Context::internal_saga_recovery();
337339
let actor = authn.actor().unwrap();
338-
assert_eq!(actor.actor_id(), USER_SAGA_RECOVERY.id);
340+
assert_eq!(actor.built_in_user_id(), Some(USER_SAGA_RECOVERY.id));
339341

340342
let authn = Context::internal_api();
341343
let actor = authn.actor().unwrap();
342-
assert_eq!(actor.actor_id(), USER_INTERNAL_API.id);
344+
assert_eq!(actor.built_in_user_id(), Some(USER_INTERNAL_API.id));
343345
}
344346
}
345347

@@ -366,31 +368,31 @@ pub struct Details {
366368
/// Who is performing an operation
367369
#[derive(Clone, Copy, Deserialize, Eq, PartialEq, Serialize)]
368370
pub enum Actor {
369-
UserBuiltin { user_builtin_id: Uuid },
370-
SiloUser { silo_user_id: Uuid, silo_id: Uuid },
371+
UserBuiltin { user_builtin_id: BuiltInUserUuid },
372+
SiloUser { silo_user_id: SiloUserUuid, silo_id: Uuid },
371373
}
372374

373375
impl Actor {
374-
pub fn actor_id(&self) -> Uuid {
375-
match self {
376-
Actor::UserBuiltin { user_builtin_id, .. } => *user_builtin_id,
377-
Actor::SiloUser { silo_user_id, .. } => *silo_user_id,
378-
}
379-
}
380-
381376
pub fn silo_id(&self) -> Option<Uuid> {
382377
match self {
383378
Actor::UserBuiltin { .. } => None,
384379
Actor::SiloUser { silo_id, .. } => Some(*silo_id),
385380
}
386381
}
387382

388-
pub fn silo_user_id(&self) -> Option<Uuid> {
383+
pub fn silo_user_id(&self) -> Option<SiloUserUuid> {
389384
match self {
390385
Actor::UserBuiltin { .. } => None,
391386
Actor::SiloUser { silo_user_id, .. } => Some(*silo_user_id),
392387
}
393388
}
389+
390+
pub fn built_in_user_id(&self) -> Option<BuiltInUserUuid> {
391+
match self {
392+
Actor::UserBuiltin { user_builtin_id } => Some(*user_builtin_id),
393+
Actor::SiloUser { .. } => None,
394+
}
395+
}
394396
}
395397

396398
impl From<&Actor> for nexus_db_model::IdentityType {

nexus/auth/src/authz/actor.rs

Lines changed: 27 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -38,8 +38,7 @@ impl oso::PolarClass for AnyActor {
3838
})
3939
.add_attribute_getter("authn_actor", |a: &AnyActor| {
4040
a.actor.map(|actor| AuthenticatedActor {
41-
actor_id: actor.actor_id(),
42-
silo_id: actor.silo_id(),
41+
actor,
4342
roles: a.roles.clone(),
4443
silo_policy: a.silo_policy.clone(),
4544
})
@@ -50,8 +49,7 @@ impl oso::PolarClass for AnyActor {
5049
/// Represents an authenticated [`authn::Context`] for Polar
5150
#[derive(Clone, Debug)]
5251
pub struct AuthenticatedActor {
53-
actor_id: Uuid,
54-
silo_id: Option<Uuid>,
52+
actor: authn::Actor,
5553
roles: RoleSet,
5654
silo_policy: Option<authn::SiloAuthnPolicy>,
5755
}
@@ -94,7 +92,7 @@ impl AuthenticatedActor {
9492

9593
impl PartialEq for AuthenticatedActor {
9694
fn eq(&self, other: &Self) -> bool {
97-
self.actor_id == other.actor_id
95+
self.actor == other.actor
9896
}
9997
}
10098

@@ -106,30 +104,36 @@ impl oso::PolarClass for AuthenticatedActor {
106104
.with_equality_check()
107105
.add_constant(
108106
AuthenticatedActor {
109-
actor_id: authn::USER_DB_INIT.id,
110-
silo_id: None,
107+
actor: authn::Actor::UserBuiltin {
108+
user_builtin_id: authn::USER_DB_INIT.id,
109+
},
111110
roles: RoleSet::new(),
112111
silo_policy: None,
113112
},
114113
"USER_DB_INIT",
115114
)
116115
.add_constant(
117116
AuthenticatedActor {
118-
actor_id: authn::USER_INTERNAL_API.id,
119-
silo_id: None,
117+
actor: authn::Actor::UserBuiltin {
118+
user_builtin_id: authn::USER_INTERNAL_API.id,
119+
},
120120
roles: RoleSet::new(),
121121
silo_policy: None,
122122
},
123123
"USER_INTERNAL_API",
124124
)
125125
.add_attribute_getter("silo", |a: &AuthenticatedActor| {
126-
a.silo_id.map(|silo_id| {
127-
super::Silo::new(
128-
super::FLEET,
129-
silo_id,
130-
LookupType::ById(silo_id),
131-
)
132-
})
126+
match a.actor {
127+
authn::Actor::SiloUser { silo_id, .. } => {
128+
Some(super::Silo::new(
129+
super::FLEET,
130+
silo_id,
131+
LookupType::ById(silo_id),
132+
))
133+
}
134+
135+
authn::Actor::UserBuiltin { .. } => None,
136+
}
133137
})
134138
.add_method(
135139
"confers_fleet_role",
@@ -139,7 +143,13 @@ impl oso::PolarClass for AuthenticatedActor {
139143
)
140144
.add_method(
141145
"equals_silo_user",
142-
|a: &AuthenticatedActor, u: SiloUser| a.actor_id == u.id(),
146+
|a: &AuthenticatedActor, u: SiloUser| match a.actor {
147+
authn::Actor::SiloUser { silo_user_id, .. } => {
148+
silo_user_id == u.id()
149+
}
150+
151+
authn::Actor::UserBuiltin { .. } => false,
152+
},
143153
)
144154
}
145155
}

0 commit comments

Comments
 (0)