Skip to content

Commit b7b24d5

Browse files
authored
Delete builtin_roles (#7477)
This came up in a meeting with @davepacheco — storing the roles in the DB and letting them be listed through the API is a legacy idea with no practical application that we know of. When I heard that I was curious how much could be deleted. Fundamentally, all this machinery I've deleted was just in service of the `role_view` and `role_list` endpoints, which let you get something like this list: ``` fleet admin fleet collaborator fleet viewer silo admin silo collaborator silo viewer project admin project collaborator project viewer ``` I think we thought this would be a dynamic list of roles, but all these things are hard-coded. As far as I can tell there is no relationship to the actual authorization system — this is basically just an unrelated copy of that stuff. This claim borne out by the fact that deleting all this stuff doesn't affect anything else, i.e., doesn't cause any tests to fail.
1 parent c8d6c04 commit b7b24d5

File tree

29 files changed

+38
-934
lines changed

29 files changed

+38
-934
lines changed

common/src/api/external/mod.rs

Lines changed: 2 additions & 116 deletions
Original file line numberDiff line numberDiff line change
@@ -497,72 +497,6 @@ fn name_schema(
497497
.into()
498498
}
499499

500-
/// Name for a built-in role
501-
#[derive(
502-
Clone,
503-
Debug,
504-
DeserializeFromStr,
505-
Display,
506-
Eq,
507-
FromStr,
508-
Ord,
509-
PartialEq,
510-
PartialOrd,
511-
SerializeDisplay,
512-
)]
513-
#[display("{resource_type}.{role_name}")]
514-
pub struct RoleName {
515-
// "resource_type" is generally the String value of one of the
516-
// `ResourceType` variants. We could store the parsed `ResourceType`
517-
// instead, but it's useful to be able to represent RoleNames for resource
518-
// types that we don't know about. That could happen if we happen to find
519-
// them in the database, for example.
520-
#[from_str(regex = "[a-z-]+")]
521-
resource_type: String,
522-
#[from_str(regex = "[a-z-]+")]
523-
role_name: String,
524-
}
525-
526-
impl RoleName {
527-
pub fn new(resource_type: &str, role_name: &str) -> RoleName {
528-
RoleName {
529-
resource_type: String::from(resource_type),
530-
role_name: String::from(role_name),
531-
}
532-
}
533-
}
534-
535-
/// Custom JsonSchema implementation to encode the constraints on RoleName
536-
impl JsonSchema for RoleName {
537-
fn schema_name() -> String {
538-
"RoleName".to_string()
539-
}
540-
fn json_schema(
541-
_: &mut schemars::gen::SchemaGenerator,
542-
) -> schemars::schema::Schema {
543-
schemars::schema::Schema::Object(schemars::schema::SchemaObject {
544-
metadata: Some(Box::new(schemars::schema::Metadata {
545-
title: Some("A name for a built-in role".to_string()),
546-
description: Some(
547-
"Role names consist of two string components \
548-
separated by dot (\".\")."
549-
.to_string(),
550-
),
551-
..Default::default()
552-
})),
553-
instance_type: Some(schemars::schema::SingleOrVec::Single(
554-
Box::new(schemars::schema::InstanceType::String),
555-
)),
556-
string: Some(Box::new(schemars::schema::StringValidation {
557-
max_length: Some(63),
558-
min_length: None,
559-
pattern: Some("[a-z-]+\\.[a-z-]+".to_string()),
560-
})),
561-
..Default::default()
562-
})
563-
}
564-
}
565-
566500
/// Byte count to express memory or storage capacity.
567501
//
568502
// The maximum supported byte count is [`i64::MAX`]. This makes it somewhat
@@ -3595,8 +3529,8 @@ mod test {
35953529
use super::VpcFirewallRuleHostFilter;
35963530
use super::VpcFirewallRuleTarget;
35973531
use super::{
3598-
ByteCount, Digest, L4Port, L4PortRange, Name, RoleName,
3599-
VpcFirewallRuleAction, VpcFirewallRuleDirection, VpcFirewallRuleFilter,
3532+
ByteCount, Digest, L4Port, L4PortRange, Name, VpcFirewallRuleAction,
3533+
VpcFirewallRuleDirection, VpcFirewallRuleFilter,
36003534
VpcFirewallRulePriority, VpcFirewallRuleProtocol,
36013535
VpcFirewallRuleStatus, VpcFirewallRuleUpdate,
36023536
VpcFirewallRuleUpdateParams,
@@ -3679,54 +3613,6 @@ mod test {
36793613
}
36803614
}
36813615

3682-
#[test]
3683-
fn test_role_name_parse() {
3684-
// Error cases
3685-
let bad_inputs = vec![
3686-
// empty string is always worth testing
3687-
"",
3688-
// missing dot
3689-
"project",
3690-
// extra dot (or, illegal character in the second component)
3691-
"project.admin.super",
3692-
// missing resource type (or, another bogus resource type)
3693-
".admin",
3694-
// missing role name
3695-
"project.",
3696-
// illegal characters in role name
3697-
"project.not_good",
3698-
];
3699-
3700-
for input in bad_inputs {
3701-
eprintln!("check name {:?} (expecting error)", input);
3702-
let result =
3703-
input.parse::<RoleName>().expect_err("unexpectedly succeeded");
3704-
eprintln!("(expected) error: {:?}", result);
3705-
}
3706-
3707-
eprintln!("check name \"project.admin\" (expecting success)");
3708-
let role_name =
3709-
"project.admin".parse::<RoleName>().expect("failed to parse");
3710-
assert_eq!(role_name.to_string(), "project.admin");
3711-
assert_eq!(role_name.resource_type, "project");
3712-
assert_eq!(role_name.role_name, "admin");
3713-
3714-
eprintln!("check name \"barf.admin\" (expecting success)");
3715-
let role_name =
3716-
"barf.admin".parse::<RoleName>().expect("failed to parse");
3717-
assert_eq!(role_name.to_string(), "barf.admin");
3718-
assert_eq!(role_name.resource_type, "barf");
3719-
assert_eq!(role_name.role_name, "admin");
3720-
3721-
eprintln!("check name \"organization.super-user\" (expecting success)");
3722-
let role_name = "organization.super-user"
3723-
.parse::<RoleName>()
3724-
.expect("failed to parse");
3725-
assert_eq!(role_name.to_string(), "organization.super-user");
3726-
assert_eq!(role_name.resource_type, "organization");
3727-
assert_eq!(role_name.role_name, "super-user");
3728-
}
3729-
37303616
#[test]
37313617
fn test_resource_name_parse() {
37323618
let bad_inputs = vec![

nexus/auth/src/authz/api_resources.rs

Lines changed: 0 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -960,14 +960,6 @@ authz_resource! {
960960
polar_snippet = FleetChild,
961961
}
962962

963-
authz_resource! {
964-
name = "RoleBuiltin",
965-
parent = "Fleet",
966-
primary_key = (String, String),
967-
roles_allowed = false,
968-
polar_snippet = FleetChild,
969-
}
970-
971963
authz_resource! {
972964
name = "UserBuiltin",
973965
parent = "Fleet",

nexus/auth/src/authz/mod.rs

Lines changed: 12 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -67,9 +67,8 @@
6767
//!
6868
//! ## Role lookup
6969
//!
70-
//! Users, roles, and API resources are all stored in the database. Naturally,
71-
//! so is the relationship that says a particular user has a particular role for
72-
//! a particular resource.
70+
//! Users and API resources are stored in the database, as is the relationship
71+
//! that says a particular user has a particular role for a particular resource.
7372
//!
7473
//! Suppose a built-in user "cookie-monster" has the "viewer" role for a Project
7574
//! "monster-foodies". It looks like this:
@@ -91,26 +90,16 @@
9190
//! | | |
9291
//! | | +---------------------------------------------------------------+
9392
//! | | |
94-
//! | | table: "role_builtin" |
95-
//! | | primary key: (resource_type, role_name) |
96-
//! | | +---------------+-----------+-----+ |
97-
//! | | | resource_type | role_name | ... | |
98-
//! | | +---------------+-----------+-----+ |
99-
//! +---|-> "project " | "viewer" | ... | |
100-
//! | | +---------------+--^--------+-----+ |
101-
//! | | | |
102-
//! | +-|--------------------+ |
103-
//! | | | |
104-
//! | | | table: "role_assignment" |
105-
//! | | | (assigns built-in roles to users on arbitrary resources) |
106-
//! | | | +---------------+-----------+-------------+-------------+---+ |
107-
//! | | | | resource_type | role_name | resource_id | identity_id |...| |
108-
//! | | | +---------------+-----------+-------------+-------------+---+ |
109-
//! | | | | "project " | "viewer" | 123 | 234 |...| |
110-
//! | | | +--^------------+--^--------+----------^--+-----------^-+---+ |
111-
//! | | | | | | | |
112-
//! +-|-|----+ | | +------------+
113-
//! +-|--------------------+ |
93+
//! | | table: "role_assignment" |
94+
//! | | (assigns roles to users on arbitrary resources) |
95+
//! | | +---------------+-----------+-------------+-------------+---+ |
96+
//! | | | resource_type | role_name | resource_id | identity_id |...| |
97+
//! | | +---------------+-----------+-------------+-------------+---+ |
98+
//! | | | "project " | "viewer" | 123 | 234 |...| |
99+
//! | | +--^------------+--^--------+----------^--+-----------^-+---+ |
100+
//! | | | | | |
101+
//! +---|----+ | +------------+
102+
//! | |
114103
//! +----------------------------------------+
115104
//! ```
116105
//!

nexus/auth/src/authz/oso_generic.rs

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -153,7 +153,6 @@ pub fn make_omicron_oso(log: &slog::Logger) -> Result<OsoInit, anyhow::Error> {
153153
DeviceAccessToken::init(),
154154
PhysicalDisk::init(),
155155
Rack::init(),
156-
RoleBuiltin::init(),
157156
SshKey::init(),
158157
Silo::init(),
159158
SiloUser::init(),

nexus/db-fixed-data/src/lib.rs

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -39,7 +39,6 @@ use std::sync::LazyLock;
3939
pub mod allow_list;
4040
pub mod project;
4141
pub mod role_assignment;
42-
pub mod role_builtin;
4342
pub mod silo;
4443
pub mod silo_user;
4544
pub mod user_builtin;

nexus/db-fixed-data/src/role_assignment.rs

Lines changed: 12 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -4,10 +4,10 @@
44
//! Built-in assignments for built-in users and built-in roles
55
66
use super::FLEET_ID;
7-
use super::role_builtin;
87
use super::user_builtin;
98
use nexus_db_model::IdentityType;
109
use nexus_db_model::RoleAssignment;
10+
use omicron_common::api::external::ResourceType;
1111
use std::sync::LazyLock;
1212

1313
pub static BUILTIN_ROLE_ASSIGNMENTS: LazyLock<Vec<RoleAssignment>> =
@@ -20,9 +20,9 @@ pub static BUILTIN_ROLE_ASSIGNMENTS: LazyLock<Vec<RoleAssignment>> =
2020
RoleAssignment::new(
2121
IdentityType::UserBuiltin,
2222
user_builtin::USER_INTERNAL_API.id,
23-
role_builtin::FLEET_ADMIN.resource_type,
23+
ResourceType::Fleet,
2424
*FLEET_ID,
25-
role_builtin::FLEET_ADMIN.role_name,
25+
"admin",
2626
),
2727
// The "USER_SERVICE_BALANCER" user gets the "admin" role on the
2828
// Fleet.
@@ -33,9 +33,9 @@ pub static BUILTIN_ROLE_ASSIGNMENTS: LazyLock<Vec<RoleAssignment>> =
3333
RoleAssignment::new(
3434
IdentityType::UserBuiltin,
3535
user_builtin::USER_SERVICE_BALANCER.id,
36-
role_builtin::FLEET_ADMIN.resource_type,
36+
ResourceType::Fleet,
3737
*FLEET_ID,
38-
role_builtin::FLEET_ADMIN.role_name,
38+
"admin",
3939
),
4040
// The "internal-read" user gets the "viewer" role on the sole
4141
// Fleet. This will grant them the ability to read various control
@@ -44,19 +44,19 @@ pub static BUILTIN_ROLE_ASSIGNMENTS: LazyLock<Vec<RoleAssignment>> =
4444
RoleAssignment::new(
4545
IdentityType::UserBuiltin,
4646
user_builtin::USER_INTERNAL_READ.id,
47-
role_builtin::FLEET_VIEWER.resource_type,
47+
ResourceType::Fleet,
4848
*FLEET_ID,
49-
role_builtin::FLEET_VIEWER.role_name,
49+
"viewer",
5050
),
51-
// The "external-authenticator" user gets the "authenticator" role
52-
// on the sole fleet. This grants them the ability to create
53-
// sessions.
51+
// The "external-authenticator" user gets the
52+
// "external-authenticator" role on the sole fleet. This grants
53+
// them the ability to create sessions.
5454
RoleAssignment::new(
5555
IdentityType::UserBuiltin,
5656
user_builtin::USER_EXTERNAL_AUTHN.id,
57-
role_builtin::FLEET_AUTHENTICATOR.resource_type,
57+
ResourceType::Fleet,
5858
*FLEET_ID,
59-
role_builtin::FLEET_AUTHENTICATOR.role_name,
59+
"external-authenticator",
6060
),
6161
]
6262
});

0 commit comments

Comments
 (0)