From 2e3612503effa7134882ac4b19281f5be97f2931 Mon Sep 17 00:00:00 2001 From: James MacMahon Date: Fri, 1 Jul 2022 09:21:41 -0400 Subject: [PATCH 01/13] Authenticated user can list and read global images Add polar statements so that any authenticated user can list and read global images. In order to test this, the set of integration tests that check endpoints for their authz visibility had to change: `GET /images/` was now accessible by any authenticated user, even if they had no permissions at all, meaning new type of visibility was required. But `POST /images` was protected by authz, meaning separate visibility settings were required for different HTTP methods. This commit also adds per-method authz visibility and updates each VerifyEndpoint struct accordingly. A few of the endpoints in uncovered-authz-endpoints.txt were able to be added in because of the new visibility variant, but not all. In order to test those for authz visibility, more work is required to support endpoints that redirect (for example) or endpoints with query parameters. --- nexus/src/authz/omicron.polar | 4 + nexus/tests/integration_tests/endpoints.rs | 1005 +++++++++++------ nexus/tests/integration_tests/unauthorized.rs | 85 +- .../unauthorized_coverage.rs | 3 +- .../output/uncovered-authz-endpoints.txt | 7 - 5 files changed, 746 insertions(+), 358 deletions(-) diff --git a/nexus/src/authz/omicron.polar b/nexus/src/authz/omicron.polar index 2e39545a5ca..c3d067409c8 100644 --- a/nexus/src/authz/omicron.polar +++ b/nexus/src/authz/omicron.polar @@ -346,6 +346,10 @@ resource GlobalImageList { has_relation(fleet: Fleet, "parent_fleet", global_image_list: GlobalImageList) if global_image_list.fleet = fleet; +# Any authenticated user can list and read global images +has_permission(_actor: AuthenticatedActor, "list_children", _global_image_list: GlobalImageList); +has_permission(_actor: AuthenticatedActor, "read", _global_image: GlobalImage); + # Describes the policy for creating and managing web console sessions. resource ConsoleSessionList { permissions = [ "create_child" ]; diff --git a/nexus/tests/integration_tests/endpoints.rs b/nexus/tests/integration_tests/endpoints.rs index e928c365c9a..700b763ddde 100644 --- a/nexus/tests/integration_tests/endpoints.rs +++ b/nexus/tests/integration_tests/endpoints.rs @@ -313,6 +313,21 @@ lazy_static! { }, disk: DEMO_DISK_NAME.clone(), }; + + // SSH keys + pub static ref DEMO_SSHKEYS_URL: &'static str = "/session/me/sshkeys"; + pub static ref DEMO_SSHKEY_NAME: Name = "aaaaa-ssh-key".parse().unwrap(); + pub static ref DEMO_SSHKEY_CREATE: params::SshKeyCreate = params::SshKeyCreate { + identity: IdentityMetadataCreateParams { + name: DEMO_SSHKEY_NAME.clone(), + description: "a demo key".to_string(), + }, + + public_key: "AAAAAAAAAAAAAAA".to_string(), + }; + + pub static ref DEMO_SPECIFIC_SSHKEY_URL: String = + format!("{}/{}", *DEMO_SSHKEYS_URL, *DEMO_SSHKEY_NAME); } lazy_static! { @@ -346,6 +361,7 @@ lazy_static! { /// /// These structs are also used to check whether we're covering all endpoints in /// the public OpenAPI spec. +#[derive(Debug)] pub struct VerifyEndpoint { /// URL path for the HTTP resource to test /// @@ -356,16 +372,8 @@ pub struct VerifyEndpoint { /// "/organizations/{organization_name}". pub url: &'static str, - /// Specifies whether an HTTP resource handled by this endpoint is visible - /// to unauthenticated or unauthorized users - /// - /// If it's [`Visibility::Public`] (like "/organizations"), unauthorized - /// users can expect to get back a 401 or 403 when they attempt to access - /// it. If it's [`Visibility::Protected`] (like a specific Organization), - /// unauthorized users will get a 404. - pub visibility: Visibility, - - /// Specifies what HTTP methods are supported for this HTTP resource + /// Specifies what HTTP methods are supported for this HTTP resource, along + /// with its authz visibility. /// /// The test runner tests a variety of HTTP methods. For each method, if /// it's not in this list, we expect a 405 "Method Not Allowed" response. @@ -378,21 +386,39 @@ pub struct VerifyEndpoint { } /// Describes the visibility of an HTTP resource +#[derive(Debug)] pub enum Visibility { - /// All users can see the resource (including unauthenticated or - /// unauthorized users) + /// Any authenticated user can see the resource, it is not protected by + /// authz policy. + /// + /// "/images" doesn't require a permission grant, for example. + PublicNoPermissionRequired, + + /// All authenticated users can see the resource (including unauthorized + /// users) but permissions are required to access. /// - /// "/organizations" is Public, for example. - Public, + /// "/organizations", for example. + PublicPermissionRequired, - /// Only users with certain privileges can see this endpoint + /// Only authenticated users with certain privileges can see this endpoint /// /// "/organizations/demo-org" is not public, for example. Protected, } -/// Describes an HTTP method supported by a particular API endpoint -pub enum AllowedMethod { +/// Describes an HTTP method supported by a particular API endpoint, and its +/// authz visibility +#[derive(Debug)] +pub struct AllowedMethod { + pub method: MethodAndBody, + + /// Specifies whether an HTTP resource handled by this endpoint is visible + /// to unauthorized users. See [`Visibility`] for more information. + pub visibility: Visibility, +} + +#[derive(Debug, Clone)] +pub enum MethodAndBody { /// HTTP "DELETE" method Delete, /// HTTP "GET" method @@ -425,16 +451,16 @@ pub enum AllowedMethod { Put(serde_json::Value), } -impl AllowedMethod { +impl MethodAndBody { /// Returns the [`http::Method`] used to make a request for this HTTP method pub fn http_method(&self) -> &'static http::Method { match self { - AllowedMethod::Delete => &Method::DELETE, - AllowedMethod::Get => &Method::GET, - AllowedMethod::GetNonexistent => &Method::GET, - AllowedMethod::GetUnimplemented => &Method::GET, - AllowedMethod::Post(_) => &Method::POST, - AllowedMethod::Put(_) => &Method::PUT, + MethodAndBody::Delete => &Method::DELETE, + MethodAndBody::Get => &Method::GET, + MethodAndBody::GetNonexistent => &Method::GET, + MethodAndBody::GetUnimplemented => &Method::GET, + MethodAndBody::Post(_) => &Method::POST, + MethodAndBody::Put(_) => &Method::PUT, } } @@ -444,12 +470,12 @@ impl AllowedMethod { /// If this returns `None`, the request body should be empty. pub fn body(&self) -> Option<&serde_json::Value> { match self { - AllowedMethod::Delete - | AllowedMethod::Get - | AllowedMethod::GetNonexistent - | AllowedMethod::GetUnimplemented => None, - AllowedMethod::Post(body) => Some(&body), - AllowedMethod::Put(body) => Some(&body), + MethodAndBody::Delete + | MethodAndBody::Get + | MethodAndBody::GetNonexistent + | MethodAndBody::GetUnimplemented => None, + MethodAndBody::Post(body) => Some(&body), + MethodAndBody::Put(body) => Some(&body), } } } @@ -463,150 +489,216 @@ lazy_static! { // Global IAM policy VerifyEndpoint { url: *POLICY_URL, - visibility: Visibility::Public, allowed_methods: vec![ - AllowedMethod::Get, - AllowedMethod::Put( - serde_json::to_value( - &shared::Policy:: { - role_assignments: vec![] - } - ).unwrap() - ), + AllowedMethod { + method: MethodAndBody::Get, + visibility: Visibility::PublicPermissionRequired, + }, + AllowedMethod { + method: MethodAndBody::Put( + serde_json::to_value( + &shared::Policy:: { + role_assignments: vec![] + } + ).unwrap() + ), + visibility: Visibility::PublicPermissionRequired, + }, ], }, // IP Pools top-level endpoint VerifyEndpoint { url: *DEMO_IP_POOLS_URL, - visibility: Visibility::Public, allowed_methods: vec![ - AllowedMethod::Get, - AllowedMethod::Post( - serde_json::to_value(&*DEMO_IP_POOL_CREATE).unwrap() - ), + AllowedMethod { + method: MethodAndBody::Get, + visibility: Visibility::PublicPermissionRequired, + }, + AllowedMethod { + method: MethodAndBody::Post( + serde_json::to_value(&*DEMO_IP_POOL_CREATE).unwrap() + ), + visibility: Visibility::PublicPermissionRequired, + }, ], }, // Single IP Pool endpoint VerifyEndpoint { url: &*DEMO_IP_POOL_URL, - visibility: Visibility::Protected, allowed_methods: vec![ - AllowedMethod::Get, - AllowedMethod::Put( - serde_json::to_value(&*DEMO_IP_POOL_UPDATE).unwrap() - ), - AllowedMethod::Delete, + AllowedMethod { + method: MethodAndBody::Get, + visibility: Visibility::Protected, + }, + AllowedMethod { + method: MethodAndBody::Put( + serde_json::to_value(&*DEMO_IP_POOL_UPDATE).unwrap() + ), + visibility: Visibility::Protected, + }, + AllowedMethod { + method: MethodAndBody::Delete, + visibility: Visibility::Protected, + }, ], }, // IP Pool ranges endpoint VerifyEndpoint { url: &*DEMO_IP_POOL_RANGES_URL, - visibility: Visibility::Protected, allowed_methods: vec![ - AllowedMethod::Get + AllowedMethod { + method: MethodAndBody::Get, + visibility: Visibility::Protected, + }, ], }, // IP Pool ranges/add endpoint VerifyEndpoint { url: &*DEMO_IP_POOL_RANGES_ADD_URL, - visibility: Visibility::Protected, allowed_methods: vec![ - AllowedMethod::Post( - serde_json::to_value(&*DEMO_IP_POOL_RANGE).unwrap() - ), + AllowedMethod { + method: MethodAndBody::Post( + serde_json::to_value(&*DEMO_IP_POOL_RANGE).unwrap() + ), + visibility: Visibility::Protected, + }, ], }, // IP Pool ranges/delete endpoint VerifyEndpoint { url: &*DEMO_IP_POOL_RANGES_DEL_URL, - visibility: Visibility::Protected, allowed_methods: vec![ - AllowedMethod::Post( - serde_json::to_value(&*DEMO_IP_POOL_RANGE).unwrap() - ), + AllowedMethod { + method: MethodAndBody::Post( + serde_json::to_value(&*DEMO_IP_POOL_RANGE).unwrap() + ), + visibility: Visibility::Protected, + }, ], }, /* Silos */ VerifyEndpoint { url: "/silos", - visibility: Visibility::Public, allowed_methods: vec![ - AllowedMethod::Get, - AllowedMethod::Post( - serde_json::to_value(&*DEMO_SILO_CREATE).unwrap() - ) + AllowedMethod { + method: MethodAndBody::Get, + visibility: Visibility::PublicPermissionRequired, + }, + AllowedMethod { + method: MethodAndBody::Post( + serde_json::to_value(&*DEMO_SILO_CREATE).unwrap() + ), + visibility: Visibility::PublicPermissionRequired, + }, ], }, VerifyEndpoint { url: &*DEMO_SILO_URL, - visibility: Visibility::Protected, allowed_methods: vec![ - AllowedMethod::Get, - AllowedMethod::Delete, + AllowedMethod { + method: MethodAndBody::Get, + visibility: Visibility::Protected, + }, + AllowedMethod { + method: MethodAndBody::Delete, + visibility: Visibility::Protected, + }, ], }, VerifyEndpoint { url: &*DEMO_SILO_POLICY_URL, - visibility: Visibility::Protected, allowed_methods: vec![ - AllowedMethod::Get, - AllowedMethod::Put( - serde_json::to_value( - &shared::Policy:: { - role_assignments: vec![] - } - ).unwrap() - ), + AllowedMethod { + method: MethodAndBody::Get, + visibility: Visibility::Protected, + }, + AllowedMethod { + method: MethodAndBody::Put( + serde_json::to_value( + &shared::Policy:: { + role_assignments: vec![] + } + ).unwrap() + ), + visibility: Visibility::Protected, + }, ], }, + VerifyEndpoint { + url: "/users", + allowed_methods: vec![ + AllowedMethod { + method: MethodAndBody::Get, + visibility: Visibility::PublicNoPermissionRequired, + }, + ], + }, /* Organizations */ VerifyEndpoint { url: "/organizations", - visibility: Visibility::Public, allowed_methods: vec![ - AllowedMethod::Get, - AllowedMethod::Post( - serde_json::to_value(&*DEMO_ORG_CREATE).unwrap() - ) + AllowedMethod { + method: MethodAndBody::Get, + visibility: Visibility::PublicPermissionRequired, + }, + AllowedMethod { + method: MethodAndBody::Post( + serde_json::to_value(&*DEMO_ORG_CREATE).unwrap() + ), + visibility: Visibility::PublicPermissionRequired, + }, ], }, VerifyEndpoint { url: &*DEMO_ORG_URL, - visibility: Visibility::Protected, - allowed_methods: vec![ - AllowedMethod::Get, - AllowedMethod::Delete, - AllowedMethod::Put( - serde_json::to_value(¶ms::OrganizationUpdate { - identity: IdentityMetadataUpdateParams { - name: None, - description: Some("different".to_string()) - } - }).unwrap() - ), + allowed_methods: vec![ + AllowedMethod { + method: MethodAndBody::Get, + visibility: Visibility::Protected, + }, + AllowedMethod { + method: MethodAndBody::Delete, + visibility: Visibility::Protected, + }, + AllowedMethod { + method: MethodAndBody::Put( + serde_json::to_value(¶ms::OrganizationUpdate { + identity: IdentityMetadataUpdateParams { + name: None, + description: Some("different".to_string()) + } + }).unwrap() + ), + visibility: Visibility::Protected, + }, ], }, VerifyEndpoint { url: &*DEMO_ORG_POLICY_URL, - visibility: Visibility::Protected, allowed_methods: vec![ - AllowedMethod::Get, - AllowedMethod::Put( - serde_json::to_value( - &shared::Policy:: { - role_assignments: vec![] - } - ).unwrap() - ), + AllowedMethod { + method: MethodAndBody::Get, + visibility: Visibility::Protected, + }, + AllowedMethod { + method: MethodAndBody::Put( + serde_json::to_value( + &shared::Policy:: { + role_assignments: vec![] + } + ).unwrap() + ), + visibility: Visibility::Protected, + } ], }, @@ -622,123 +714,174 @@ lazy_static! { // incredibly expensive to determine in general. VerifyEndpoint { url: &*DEMO_ORG_PROJECTS_URL, - visibility: Visibility::Protected, allowed_methods: vec![ - AllowedMethod::Get, - AllowedMethod::Post( - serde_json::to_value(&*DEMO_PROJECT_CREATE).unwrap() - ), + AllowedMethod { + method: MethodAndBody::Get, + visibility: Visibility::Protected, + }, + AllowedMethod { + method: MethodAndBody::Post( + serde_json::to_value(&*DEMO_PROJECT_CREATE).unwrap() + ), + visibility: Visibility::Protected, + }, ], }, VerifyEndpoint { url: &*DEMO_PROJECT_URL, - visibility: Visibility::Protected, - allowed_methods: vec![ - AllowedMethod::Get, - AllowedMethod::Delete, - AllowedMethod::Put( - serde_json::to_value(params::ProjectUpdate{ - identity: IdentityMetadataUpdateParams { - name: None, - description: Some("different".to_string()) - }, - }).unwrap() - ), + allowed_methods: vec![ + AllowedMethod { + method: MethodAndBody::Get, + visibility: Visibility::Protected, + }, + AllowedMethod { + method: MethodAndBody::Delete, + visibility: Visibility::Protected, + }, + AllowedMethod { + method: MethodAndBody::Put( + serde_json::to_value(params::ProjectUpdate{ + identity: IdentityMetadataUpdateParams { + name: None, + description: Some("different".to_string()) + }, + }).unwrap() + ), + visibility: Visibility::Protected, + }, ], }, VerifyEndpoint { url: &*DEMO_PROJECT_POLICY_URL, - visibility: Visibility::Protected, allowed_methods: vec![ - AllowedMethod::Get, - AllowedMethod::Put( - serde_json::to_value( - &shared::Policy:: { - role_assignments: vec![] - } - ).unwrap() - ), + AllowedMethod { + method: MethodAndBody::Get, + visibility: Visibility::Protected, + }, + AllowedMethod { + method: MethodAndBody::Put( + serde_json::to_value( + &shared::Policy:: { + role_assignments: vec![] + } + ).unwrap() + ), + visibility: Visibility::Protected, + }, ], }, /* VPCs */ VerifyEndpoint { url: &*DEMO_PROJECT_URL_VPCS, - visibility: Visibility::Protected, allowed_methods: vec![ - AllowedMethod::Get, - AllowedMethod::Post( - serde_json::to_value(&*DEMO_VPC_CREATE).unwrap() - ), + AllowedMethod { + method: MethodAndBody::Get, + visibility: Visibility::Protected, + }, + AllowedMethod { + method: MethodAndBody::Post( + serde_json::to_value(&*DEMO_VPC_CREATE).unwrap() + ), + visibility: Visibility::Protected, + }, ], }, VerifyEndpoint { url: &*DEMO_VPC_URL, - visibility: Visibility::Protected, - allowed_methods: vec![ - AllowedMethod::Get, - AllowedMethod::Put( - serde_json::to_value(¶ms::VpcUpdate { - identity: IdentityMetadataUpdateParams { - name: None, - description: Some("different".to_string()) - }, - dns_name: None, - }).unwrap() - ), - AllowedMethod::Delete, + allowed_methods: vec![ + AllowedMethod { + method: MethodAndBody::Get, + visibility: Visibility::Protected, + }, + AllowedMethod { + method: MethodAndBody::Put( + serde_json::to_value(¶ms::VpcUpdate { + identity: IdentityMetadataUpdateParams { + name: None, + description: Some("different".to_string()) + }, + dns_name: None, + }).unwrap() + ), + visibility: Visibility::Protected, + }, + AllowedMethod { + method: MethodAndBody::Delete, + visibility: Visibility::Protected, + }, ], }, /* Firewall rules */ VerifyEndpoint { url: &*DEMO_VPC_URL_FIREWALL_RULES, - visibility: Visibility::Protected, - allowed_methods: vec![ - AllowedMethod::Get, - AllowedMethod::Put( - serde_json::to_value(VpcFirewallRuleUpdateParams { - rules: vec![], - }).unwrap() - ), + allowed_methods: vec![ + AllowedMethod { + method: MethodAndBody::Get, + visibility: Visibility::Protected, + }, + AllowedMethod { + method: MethodAndBody::Put( + serde_json::to_value(VpcFirewallRuleUpdateParams { + rules: vec![], + }).unwrap() + ), + visibility: Visibility::Protected, + }, ], }, /* VPC Subnets */ VerifyEndpoint { url: &*DEMO_VPC_URL_SUBNETS, - visibility: Visibility::Protected, allowed_methods: vec![ - AllowedMethod::Get, - AllowedMethod::Post( - serde_json::to_value(&*DEMO_VPC_SUBNET_CREATE).unwrap() - ), + AllowedMethod { + method: MethodAndBody::Get, + visibility: Visibility::Protected, + }, + AllowedMethod { + method: MethodAndBody::Post( + serde_json::to_value(&*DEMO_VPC_SUBNET_CREATE).unwrap() + ), + visibility: Visibility::Protected, + }, ], }, VerifyEndpoint { url: &*DEMO_VPC_SUBNET_URL, - visibility: Visibility::Protected, - allowed_methods: vec![ - AllowedMethod::Get, - AllowedMethod::Put( - serde_json::to_value(¶ms::VpcSubnetUpdate { - identity: IdentityMetadataUpdateParams { - name: None, - description: Some("different".to_string()) - }, - }).unwrap() - ), - AllowedMethod::Delete, + allowed_methods: vec![ + AllowedMethod { + method: MethodAndBody::Get, + visibility: Visibility::Protected, + }, + AllowedMethod { + method: MethodAndBody::Put( + serde_json::to_value(¶ms::VpcSubnetUpdate { + identity: IdentityMetadataUpdateParams { + name: None, + description: Some("different".to_string()) + }, + }).unwrap() + ), + visibility: Visibility::Protected, + }, + AllowedMethod { + method: MethodAndBody::Delete, + visibility: Visibility::Protected, + }, ], }, VerifyEndpoint { url: &*DEMO_VPC_SUBNET_INTERFACES_URL, - visibility: Visibility::Protected, allowed_methods: vec![ - AllowedMethod::Get, + AllowedMethod { + method: MethodAndBody::Get, + visibility: Visibility::Protected, + }, ], }, @@ -746,29 +889,42 @@ lazy_static! { VerifyEndpoint { url: &*DEMO_VPC_URL_ROUTERS, - visibility: Visibility::Protected, allowed_methods: vec![ - AllowedMethod::Get, - AllowedMethod::Post( - serde_json::to_value(&*DEMO_VPC_ROUTER_CREATE).unwrap() - ), + AllowedMethod { + method: MethodAndBody::Get, + visibility: Visibility::Protected, + }, + AllowedMethod { + method: MethodAndBody::Post( + serde_json::to_value(&*DEMO_VPC_ROUTER_CREATE).unwrap() + ), + visibility: Visibility::Protected, + }, ], }, VerifyEndpoint { url: &*DEMO_VPC_ROUTER_URL, - visibility: Visibility::Protected, - allowed_methods: vec![ - AllowedMethod::Get, - AllowedMethod::Put( - serde_json::to_value(¶ms::VpcRouterUpdate { - identity: IdentityMetadataUpdateParams { - name: None, - description: Some("different".to_string()) - }, - }).unwrap() - ), - AllowedMethod::Delete, + allowed_methods: vec![ + AllowedMethod { + method: MethodAndBody::Get, + visibility: Visibility::Protected, + }, + AllowedMethod { + method: MethodAndBody::Put( + serde_json::to_value(¶ms::VpcRouterUpdate { + identity: IdentityMetadataUpdateParams { + name: None, + description: Some("different".to_string()) + }, + }).unwrap() + ), + visibility: Visibility::Protected, + }, + AllowedMethod { + method: MethodAndBody::Delete, + visibility: Visibility::Protected, + }, ], }, @@ -776,86 +932,114 @@ lazy_static! { VerifyEndpoint { url: &*DEMO_VPC_ROUTER_URL_ROUTES, - visibility: Visibility::Protected, allowed_methods: vec![ - AllowedMethod::Get, - AllowedMethod::Post( - serde_json::to_value(&*DEMO_ROUTER_ROUTE_CREATE).unwrap() - ), + AllowedMethod { + method: MethodAndBody::Get, + visibility: Visibility::Protected, + }, + AllowedMethod { + method: MethodAndBody::Post( + serde_json::to_value(&*DEMO_ROUTER_ROUTE_CREATE).unwrap() + ), + visibility: Visibility::Protected, + }, ], }, VerifyEndpoint { url: &*DEMO_ROUTER_ROUTE_URL, - visibility: Visibility::Protected, - allowed_methods: vec![ - AllowedMethod::Get, - AllowedMethod::Put( - serde_json::to_value(&RouterRouteUpdateParams { - identity: IdentityMetadataUpdateParams { - name: None, - description: Some("different".to_string()) - }, - target: RouteTarget::Ip( - IpAddr::from(Ipv4Addr::new(127, 0, 0, 1))), - destination: RouteDestination::Subnet( - "loopback".parse().unwrap()), - }).unwrap() - ), - AllowedMethod::Delete, + allowed_methods: vec![ + AllowedMethod { + method: MethodAndBody::Get, + visibility: Visibility::Protected, + }, + AllowedMethod { + method: MethodAndBody::Put( + serde_json::to_value(&RouterRouteUpdateParams { + identity: IdentityMetadataUpdateParams { + name: None, + description: Some("different".to_string()) + }, + target: RouteTarget::Ip( + IpAddr::from(Ipv4Addr::new(127, 0, 0, 1))), + destination: RouteDestination::Subnet( + "loopback".parse().unwrap()), + }).unwrap() + ), + visibility: Visibility::Protected, + }, + AllowedMethod { + method: MethodAndBody::Delete, + visibility: Visibility::Protected, + }, ], }, - /* Disks */ VerifyEndpoint { url: &*DEMO_PROJECT_URL_DISKS, - visibility: Visibility::Protected, allowed_methods: vec![ - AllowedMethod::Get, - AllowedMethod::Post( - serde_json::to_value(&*DEMO_DISK_CREATE).unwrap() - ), + AllowedMethod { + method: MethodAndBody::Get, + visibility: Visibility::Protected, + }, + AllowedMethod { + method: MethodAndBody::Post( + serde_json::to_value(&*DEMO_DISK_CREATE).unwrap() + ), + visibility: Visibility::Protected, + }, ], }, VerifyEndpoint { url: &*DEMO_DISK_URL, - visibility: Visibility::Protected, allowed_methods: vec![ - AllowedMethod::Get, - AllowedMethod::Delete, + AllowedMethod { + method: MethodAndBody::Get, + visibility: Visibility::Protected, + }, + AllowedMethod { + method: MethodAndBody::Delete, + visibility: Visibility::Protected, + }, ], }, VerifyEndpoint { url: &*DEMO_INSTANCE_DISKS_URL, - visibility: Visibility::Protected, allowed_methods: vec![ - AllowedMethod::Get, + AllowedMethod { + method: MethodAndBody::Get, + visibility: Visibility::Protected, + }, ], }, VerifyEndpoint { url: &*DEMO_INSTANCE_DISKS_ATTACH_URL, - visibility: Visibility::Protected, allowed_methods: vec![ - AllowedMethod::Post( - serde_json::to_value(params::DiskIdentifier { - name: DEMO_DISK_NAME.clone() - }).unwrap() - ) + AllowedMethod { + method: MethodAndBody::Post( + serde_json::to_value(params::DiskIdentifier { + name: DEMO_DISK_NAME.clone() + }).unwrap() + ), + visibility: Visibility::Protected, + }, ], }, VerifyEndpoint { url: &*DEMO_INSTANCE_DISKS_DETACH_URL, - visibility: Visibility::Protected, allowed_methods: vec![ - AllowedMethod::Post( - serde_json::to_value(params::DiskIdentifier { - name: DEMO_DISK_NAME.clone() - }).unwrap() - ) + AllowedMethod { + method: MethodAndBody::Post( + serde_json::to_value(params::DiskIdentifier { + name: DEMO_DISK_NAME.clone() + }).unwrap() + ), + visibility: Visibility::Protected, + }, ], }, @@ -863,21 +1047,31 @@ lazy_static! { VerifyEndpoint { url: &*DEMO_PROJECT_URL_IMAGES, - visibility: Visibility::Protected, allowed_methods: vec![ - AllowedMethod::GetUnimplemented, - AllowedMethod::Post( - serde_json::to_value(&*DEMO_IMAGE_CREATE).unwrap() - ), + AllowedMethod { + method: MethodAndBody::GetUnimplemented, + visibility: Visibility::Protected, + }, + AllowedMethod { + method: MethodAndBody::Post( + serde_json::to_value(&*DEMO_IMAGE_CREATE).unwrap() + ), + visibility: Visibility::Protected, + }, ], }, VerifyEndpoint { url: &*DEMO_PROJECT_IMAGE_URL, - visibility: Visibility::Protected, allowed_methods: vec![ - AllowedMethod::GetUnimplemented, - AllowedMethod::Delete, + AllowedMethod { + method: MethodAndBody::GetUnimplemented, + visibility: Visibility::Protected, + }, + AllowedMethod { + method: MethodAndBody::Delete, + visibility: Visibility::Protected, + }, ], }, @@ -885,104 +1079,147 @@ lazy_static! { VerifyEndpoint { url: &*DEMO_PROJECT_URL_SNAPSHOTS, - visibility: Visibility::Protected, allowed_methods: vec![ - AllowedMethod::GetUnimplemented, - AllowedMethod::Post( - serde_json::to_value(DEMO_SNAPSHOT_CREATE.clone()).unwrap(), - ) + AllowedMethod { + method: MethodAndBody::GetUnimplemented, + visibility: Visibility::Protected, + }, + AllowedMethod { + method: MethodAndBody::Post( + serde_json::to_value(DEMO_SNAPSHOT_CREATE.clone()).unwrap(), + ), + visibility: Visibility::Protected, + }, ] }, VerifyEndpoint { url: &*DEMO_SNAPSHOT_URL, - visibility: Visibility::Protected, allowed_methods: vec![ - AllowedMethod::GetUnimplemented, - AllowedMethod::Delete, + AllowedMethod { + method: MethodAndBody::GetUnimplemented, + visibility: Visibility::Protected, + }, + AllowedMethod { + method: MethodAndBody::Delete, + visibility: Visibility::Protected, + }, ] }, /* Instances */ VerifyEndpoint { url: &*DEMO_PROJECT_URL_INSTANCES, - visibility: Visibility::Protected, allowed_methods: vec![ - AllowedMethod::Get, - AllowedMethod::Post( - serde_json::to_value(&*DEMO_INSTANCE_CREATE).unwrap() - ), + AllowedMethod { + method: MethodAndBody::Get, + visibility: Visibility::Protected, + }, + AllowedMethod { + method: MethodAndBody::Post( + serde_json::to_value(&*DEMO_INSTANCE_CREATE).unwrap() + ), + visibility: Visibility::Protected, + }, ], }, VerifyEndpoint { url: &*DEMO_INSTANCE_URL, - visibility: Visibility::Protected, allowed_methods: vec![ - AllowedMethod::Get, - AllowedMethod::Delete, + AllowedMethod { + method: MethodAndBody::Get, + visibility: Visibility::Protected, + }, + AllowedMethod { + method: MethodAndBody::Delete, + visibility: Visibility::Protected, + }, ], }, VerifyEndpoint { url: &*DEMO_INSTANCE_START_URL, - visibility: Visibility::Protected, allowed_methods: vec![ - AllowedMethod::Post(serde_json::Value::Null) + AllowedMethod { + method: MethodAndBody::Post(serde_json::Value::Null), + visibility: Visibility::Protected, + }, ], }, VerifyEndpoint { url: &*DEMO_INSTANCE_STOP_URL, - visibility: Visibility::Protected, allowed_methods: vec![ - AllowedMethod::Post(serde_json::Value::Null) + AllowedMethod { + method: MethodAndBody::Post(serde_json::Value::Null), + visibility: Visibility::Protected, + }, ], }, VerifyEndpoint { url: &*DEMO_INSTANCE_REBOOT_URL, - visibility: Visibility::Protected, allowed_methods: vec![ - AllowedMethod::Post(serde_json::Value::Null) + AllowedMethod { + method: MethodAndBody::Post(serde_json::Value::Null), + visibility: Visibility::Protected, + }, ], }, VerifyEndpoint { url: &*DEMO_INSTANCE_MIGRATE_URL, - visibility: Visibility::Protected, allowed_methods: vec![ - AllowedMethod::Post(serde_json::to_value( - params::InstanceMigrate { - dst_sled_id: uuid::Uuid::new_v4(), - } - ).unwrap()), + AllowedMethod { + method: MethodAndBody::Post(serde_json::to_value( + params::InstanceMigrate { + dst_sled_id: uuid::Uuid::new_v4(), + } + ).unwrap()), + visibility: Visibility::Protected, + }, ], }, VerifyEndpoint { url: &*DEMO_INSTANCE_SERIAL_URL, - visibility: Visibility::Protected, allowed_methods: vec![ - AllowedMethod::GetNonexistent // has required query parameters + AllowedMethod { + method: MethodAndBody::GetNonexistent, // has required query parameters + visibility: Visibility::Protected, + }, ], }, /* Instance NICs */ VerifyEndpoint { url: &*DEMO_INSTANCE_NICS_URL, - visibility: Visibility::Protected, allowed_methods: vec![ - AllowedMethod::Get, - AllowedMethod::Post( - serde_json::to_value(&*DEMO_INSTANCE_NIC_CREATE).unwrap() - ), + AllowedMethod { + method: MethodAndBody::Get, + visibility: Visibility::Protected, + }, + AllowedMethod { + method: MethodAndBody::Post( + serde_json::to_value(&*DEMO_INSTANCE_NIC_CREATE).unwrap() + ), + visibility: Visibility::Protected, + }, ], }, VerifyEndpoint { url: &*DEMO_INSTANCE_NIC_URL, - visibility: Visibility::Protected, allowed_methods: vec![ - AllowedMethod::Get, - AllowedMethod::Delete, - AllowedMethod::Put( - serde_json::to_value(&*DEMO_INSTANCE_NIC_PUT).unwrap() - ), + AllowedMethod { + method: MethodAndBody::Get, + visibility: Visibility::Protected, + }, + AllowedMethod { + method: MethodAndBody::Delete, + visibility: Visibility::Protected, + }, + AllowedMethod { + method: MethodAndBody::Put( + serde_json::to_value(&*DEMO_INSTANCE_NIC_PUT).unwrap() + ), + visibility: Visibility::Protected, + }, ], }, @@ -990,128 +1227,238 @@ lazy_static! { VerifyEndpoint { url: "/roles", - visibility: Visibility::Public, - allowed_methods: vec![AllowedMethod::Get], + allowed_methods: vec![ + AllowedMethod { + method: MethodAndBody::Get, + visibility: Visibility::PublicPermissionRequired, + }, + ], }, VerifyEndpoint { url: "/roles/fleet.admin", - visibility: Visibility::Protected, - allowed_methods: vec![AllowedMethod::Get], + allowed_methods: vec![ + AllowedMethod { + method: MethodAndBody::Get, + visibility: Visibility::Protected, + }, + ], }, VerifyEndpoint { url: "/users_builtin", - visibility: Visibility::Public, - allowed_methods: vec![AllowedMethod::Get], + allowed_methods: vec![ + AllowedMethod { + method: MethodAndBody::Get, + visibility: Visibility::PublicPermissionRequired, + }, + ], }, VerifyEndpoint { url: &*URL_USERS_DB_INIT, - visibility: Visibility::Protected, - allowed_methods: vec![AllowedMethod::Get], + allowed_methods: vec![ + AllowedMethod { + method: MethodAndBody::Get, + visibility: Visibility::Protected, + }, + ], }, /* Hardware */ VerifyEndpoint { url: "/hardware/racks", - visibility: Visibility::Public, - allowed_methods: vec![AllowedMethod::Get], + allowed_methods: vec![ + AllowedMethod { + method: MethodAndBody::Get, + visibility: Visibility::PublicPermissionRequired, + }, + ], }, VerifyEndpoint { url: &*HARDWARE_RACK_URL, - visibility: Visibility::Protected, - allowed_methods: vec![AllowedMethod::Get], + allowed_methods: vec![ + AllowedMethod { + method: MethodAndBody::Get, + visibility: Visibility::Protected, + }, + ], }, VerifyEndpoint { url: "/hardware/sleds", - visibility: Visibility::Public, - allowed_methods: vec![AllowedMethod::Get], + allowed_methods: vec![ + AllowedMethod { + method: MethodAndBody::Get, + visibility: Visibility::PublicPermissionRequired, + }, + ], }, VerifyEndpoint { url: &*HARDWARE_SLED_URL, - visibility: Visibility::Protected, - allowed_methods: vec![AllowedMethod::Get], + allowed_methods: vec![ + AllowedMethod { + method: MethodAndBody::Get, + visibility: Visibility::Protected, + }, + ], }, /* Sagas */ VerifyEndpoint { url: "/sagas", - visibility: Visibility::Public, - allowed_methods: vec![AllowedMethod::Get], + allowed_methods: vec![ + AllowedMethod { + method: MethodAndBody::Get, + visibility: Visibility::PublicPermissionRequired, + }, + ], }, VerifyEndpoint { url: "/sagas/48a1b8c8-fc1c-6fea-9de9-fdeb8dda7823", - visibility: Visibility::Public, - allowed_methods: vec![AllowedMethod::GetNonexistent], + allowed_methods: vec![ + AllowedMethod { + method: MethodAndBody::GetNonexistent, + visibility: Visibility::PublicPermissionRequired, + }, + ], }, /* Timeseries schema */ VerifyEndpoint { url: "/timeseries/schema", - visibility: Visibility::Public, - allowed_methods: vec![AllowedMethod::Get], + allowed_methods: vec![ + AllowedMethod { + method: MethodAndBody::Get, + visibility: Visibility::PublicPermissionRequired, + }, + ], }, /* Updates */ VerifyEndpoint { url: "/updates/refresh", - visibility: Visibility::Public, - allowed_methods: vec![AllowedMethod::Post( - serde_json::Value::Null - )], + allowed_methods: vec![ + AllowedMethod { + method: MethodAndBody::Post( + serde_json::Value::Null + ), + visibility: Visibility::PublicPermissionRequired, + }, + ], }, /* Global Images */ VerifyEndpoint { url: "/images", - visibility: Visibility::Public, allowed_methods: vec![ - AllowedMethod::Get, - AllowedMethod::Post( - serde_json::to_value(&*DEMO_GLOBAL_IMAGE_CREATE).unwrap() - ), + AllowedMethod { + method: MethodAndBody::Get, + visibility: Visibility::PublicNoPermissionRequired, + }, + AllowedMethod { + method: MethodAndBody::Post( + serde_json::to_value(&*DEMO_GLOBAL_IMAGE_CREATE).unwrap() + ), + visibility: Visibility::PublicPermissionRequired, + }, ], }, VerifyEndpoint { url: &*DEMO_GLOBAL_IMAGE_URL, - visibility: Visibility::Protected, allowed_methods: vec![ - AllowedMethod::Get, - AllowedMethod::Delete, + AllowedMethod { + method: MethodAndBody::Get, + visibility: Visibility::PublicNoPermissionRequired, + }, + AllowedMethod { + method: MethodAndBody::Delete, + visibility: Visibility::Protected, // XXX is this right? + }, ], }, /* Silo identity providers */ - /* VerifyEndpoint { - url: &*IDENTITY_PROVIDERS_URL, // in ignore list - visibility: Visibility::Public, + url: &*IDENTITY_PROVIDERS_URL, allowed_methods: vec![ - AllowedMethod::Get, + AllowedMethod { + method: MethodAndBody::Get, + visibility: Visibility::PublicNoPermissionRequired, + }, ], }, - */ + VerifyEndpoint { url: &*SAML_IDENTITY_PROVIDERS_URL, - visibility: Visibility::Public, - allowed_methods: vec![AllowedMethod::Post( - serde_json::to_value(&*SAML_IDENTITY_PROVIDER).unwrap(), - )], + allowed_methods: vec![ + AllowedMethod { + method: MethodAndBody::Post( + serde_json::to_value(&*SAML_IDENTITY_PROVIDER).unwrap(), + ), + visibility: Visibility::PublicPermissionRequired, + }, + ], }, VerifyEndpoint { url: &*SPECIFIC_SAML_IDENTITY_PROVIDER_URL, - visibility: Visibility::Protected, - allowed_methods: vec![AllowedMethod::Get], + allowed_methods: vec![ + AllowedMethod { + method: MethodAndBody::Get, + visibility: Visibility::Protected, + }, + ], + }, + + /* Misc */ + + VerifyEndpoint { + url: "/session/me", + allowed_methods: vec![ + AllowedMethod { + method: MethodAndBody::Get, + visibility: Visibility::PublicNoPermissionRequired, + }, + ], + }, + + /* SSH keys */ + + VerifyEndpoint { + url: &*DEMO_SSHKEYS_URL, + allowed_methods: vec![ + AllowedMethod { + method: MethodAndBody::Get, + visibility: Visibility::Protected, // XXX is this right? + }, + AllowedMethod { + method: MethodAndBody::Post( + serde_json::to_value(&*DEMO_SSHKEY_CREATE).unwrap(), + ), + visibility: Visibility::Protected, // XXX is this right? + }, + ], + }, + VerifyEndpoint { + url: &*DEMO_SPECIFIC_SSHKEY_URL, + allowed_methods: vec![ + AllowedMethod { + method: MethodAndBody::Get, + visibility: Visibility::Protected, // XXX is this right? + }, + AllowedMethod { + method: MethodAndBody::Delete, + visibility: Visibility::Protected, // XXX is this right? + }, + ], }, ]; } diff --git a/nexus/tests/integration_tests/unauthorized.rs b/nexus/tests/integration_tests/unauthorized.rs index d83f63c64ce..5b127153ef0 100644 --- a/nexus/tests/integration_tests/unauthorized.rs +++ b/nexus/tests/integration_tests/unauthorized.rs @@ -216,6 +216,11 @@ lazy_static! { url: &*SAML_IDENTITY_PROVIDERS_URL, body: serde_json::to_value(&*SAML_IDENTITY_PROVIDER).unwrap(), }, + // Create a SSH key + SetupReq { + url: &*DEMO_SSHKEYS_URL, + body: serde_json::to_value(&*DEMO_SSHKEY_CREATE).unwrap(), + }, ]; } @@ -281,22 +286,22 @@ async fn verify_endpoint( // "401 Unauthorized" status code. let unauthn_status = StatusCode::UNAUTHORIZED; - // Determine the expected status code for authenticated, unauthorized - // requests, based on the endpoint's visibility. - let unauthz_status = match endpoint.visibility { - Visibility::Public => StatusCode::FORBIDDEN, - Visibility::Protected => StatusCode::NOT_FOUND, - }; - // Make one GET request as an authorized user to make sure we get a "200 OK" // response. Otherwise, the test might later succeed by coincidence. We // might find a 404 because of something that actually doesn't exist rather // than something that's just hidden from unauthorized users. - let get_allowed = endpoint.allowed_methods.iter().find(|allowed| { - matches!(allowed, AllowedMethod::Get | AllowedMethod::GetUnimplemented) - }); + let get_allowed = endpoint + .allowed_methods + .iter() + .find(|allowed| { + matches!( + allowed.method, + MethodAndBody::Get | MethodAndBody::GetUnimplemented + ) + }) + .map(|x| x.method.clone()); let resource_before = match get_allowed { - Some(AllowedMethod::Get) => { + Some(MethodAndBody::Get) => { info!(log, "test: privileged GET"); record_operation(WhichTest::PrivilegedGet(Some( &http::StatusCode::OK, @@ -311,7 +316,7 @@ async fn verify_endpoint( .unwrap(), ) } - Some(AllowedMethod::GetUnimplemented) => { + Some(MethodAndBody::GetUnimplemented) => { info!(log, "test: privileged GET (unimplemented)"); let expected_status = http::StatusCode::INTERNAL_SERVER_ERROR; record_operation(WhichTest::PrivilegedGet(Some(&expected_status))); @@ -345,16 +350,49 @@ async fn verify_endpoint( let allowed = endpoint .allowed_methods .iter() - .find(|allowed| method == *allowed.http_method()); + .find(|allowed| method == *allowed.method.http_method()); + + let body = allowed.and_then(|a| a.method.body()).cloned(); - let body = allowed.and_then(|a| a.body()).cloned(); + info!( + log, + "processing endpoint {:?} allowed {:?} method {:?}", + endpoint, + allowed, + method + ); // First, make an authenticated, unauthorized request. - info!(log, "test: authenticated, unauthorized"; "method" => ?method); + info!(log, "test: authenticated, unauthorized"; "method" => ?method, "body" => ?body); + let expected_status = match allowed { - Some(_) => unauthz_status, + Some(allowed) => { + // Determine the expected status code for authenticated, unauthorized + // requests, based on the endpoint's visibility. If set to + // "IfAuthenticated", the endpoint is visible to any authenticated user. + match allowed.visibility { + // can see and access, but only for GET + Visibility::PublicNoPermissionRequired => match method { + Method::GET => StatusCode::OK, + _ => panic!( + "Visibility::PublicNoPermissionRequired for {}", + method + ), + }, + + // can see, but not access + Visibility::PublicPermissionRequired => { + StatusCode::FORBIDDEN + } + + // cannot see + Visibility::Protected => StatusCode::NOT_FOUND, + } + } + None => StatusCode::METHOD_NOT_ALLOWED, }; + let response = NexusRequest::new( RequestBuilder::new(client, method.clone(), endpoint.url) .body(body.as_ref()) @@ -364,7 +402,7 @@ async fn verify_endpoint( .execute() .await .unwrap(); - verify_response(&response); + verify_response(&response, &expected_status); record_operation(WhichTest::Unprivileged(&expected_status)); // Next, make an unauthenticated request. @@ -380,7 +418,7 @@ async fn verify_endpoint( .execute() .await .unwrap(); - verify_response(&response); + verify_response(&response, &expected_status); record_operation(WhichTest::Unauthenticated(&expected_status)); // Now try a few requests with bogus credentials. We should get the @@ -412,7 +450,7 @@ async fn verify_endpoint( .execute() .await .unwrap(); - verify_response(&response); + verify_response(&response, &expected_status); record_operation(WhichTest::UnknownUser(&expected_status)); // Now try a syntactically invalid authn header. @@ -429,7 +467,7 @@ async fn verify_endpoint( .execute() .await .unwrap(); - verify_response(&response); + verify_response(&response, &expected_status); record_operation(WhichTest::InvalidHeader(&expected_status)); print!(" "); @@ -472,7 +510,12 @@ async fn verify_endpoint( } /// Verifies the body of an HTTP response for status codes 401, 403, 404, or 405 -fn verify_response(response: &TestResponse) { +fn verify_response(response: &TestResponse, expected_status: &StatusCode) { + // If processing a visible endpoint + if *expected_status == StatusCode::OK { + return; + } + let error: HttpErrorResponseBody = response.parsed_body().unwrap(); match response.status { StatusCode::UNAUTHORIZED => { diff --git a/nexus/tests/integration_tests/unauthorized_coverage.rs b/nexus/tests/integration_tests/unauthorized_coverage.rs index e92a5af98be..a89b354007a 100644 --- a/nexus/tests/integration_tests/unauthorized_coverage.rs +++ b/nexus/tests/integration_tests/unauthorized_coverage.rs @@ -81,7 +81,8 @@ fn test_unauthorized_coverage() { ); for v in &*VERIFY_ENDPOINTS { for m in &v.allowed_methods { - let method_string = m.http_method().to_string().to_uppercase(); + let method_string = + m.method.http_method().to_string().to_uppercase(); let found = spec_operations.iter().find(|(op, regex)| { op.method.to_uppercase() == method_string && regex.is_match(v.url) diff --git a/nexus/tests/output/uncovered-authz-endpoints.txt b/nexus/tests/output/uncovered-authz-endpoints.txt index 04fd2cc3a8e..0088161352d 100644 --- a/nexus/tests/output/uncovered-authz-endpoints.txt +++ b/nexus/tests/output/uncovered-authz-endpoints.txt @@ -1,16 +1,9 @@ API endpoints with no coverage in authz tests: -sshkeys_delete_key (delete "/session/me/sshkeys/{ssh_key_name}") device_auth_verify (get "/device/verify") login (get "/login/{silo_name}/{provider_name}") -session_me (get "/session/me") -sshkeys_get (get "/session/me/sshkeys") -sshkeys_get_key (get "/session/me/sshkeys/{ssh_key_name}") -silos_get_identity_providers (get "/silos/{silo_name}/identity_providers") -silo_users_get (get "/users") device_auth_request (post "/device/auth") device_auth_confirm (post "/device/confirm") device_access_token (post "/device/token") spoof_login (post "/login") consume_credentials (post "/login/{silo_name}/{provider_name}") logout (post "/logout") -sshkeys_post (post "/session/me/sshkeys") From bf8a5d3993df87461da21ce9b842c05fc294e55e Mon Sep 17 00:00:00 2001 From: James MacMahon Date: Thu, 7 Jul 2022 10:12:16 -0400 Subject: [PATCH 02/13] Undo changes, add unprivileged_access instead Can't get SSH keys tests to work, deferring for another commit. --- nexus/tests/integration_tests/endpoints.rs | 1067 +++++++---------- nexus/tests/integration_tests/unauthorized.rs | 78 +- .../unauthorized_coverage.rs | 3 +- .../output/uncovered-authz-endpoints.txt | 4 + 4 files changed, 473 insertions(+), 679 deletions(-) diff --git a/nexus/tests/integration_tests/endpoints.rs b/nexus/tests/integration_tests/endpoints.rs index 700b763ddde..c81b700f64c 100644 --- a/nexus/tests/integration_tests/endpoints.rs +++ b/nexus/tests/integration_tests/endpoints.rs @@ -372,8 +372,19 @@ pub struct VerifyEndpoint { /// "/organizations/{organization_name}". pub url: &'static str, - /// Specifies what HTTP methods are supported for this HTTP resource, along - /// with its authz visibility. + /// Specifies whether an HTTP resource handled by this endpoint is visible + /// to unauthenticated or unauthorized users + /// + /// If it's [`Visibility::Public`] (like "/organizations"), unauthorized + /// users can expect to get back a 401 or 403 when they attempt to access + /// it. If it's [`Visibility::Protected`] (like a specific Organization), + /// unauthorized users will get a 404. + pub visibility: Visibility, + + /// Specify level of unprivileged access + pub unprivileged_access: UnprivilegedAccess, + + /// Specifies what HTTP methods are supported for this HTTP resource /// /// The test runner tests a variety of HTTP methods. For each method, if /// it's not in this list, we expect a 405 "Method Not Allowed" response. @@ -385,40 +396,37 @@ pub struct VerifyEndpoint { pub allowed_methods: Vec, } +/// Describe what access authenticated unprivileged users have. +#[derive(Debug)] +pub enum UnprivilegedAccess { + /// Users have full CRUD access to the endpoint + Full, + + /// Users only have read access to the endpoint + ReadOnly, + + /// Users have no access at all to the endpoint + None, +} + /// Describes the visibility of an HTTP resource #[derive(Debug)] pub enum Visibility { - /// Any authenticated user can see the resource, it is not protected by - /// authz policy. - /// - /// "/images" doesn't require a permission grant, for example. - PublicNoPermissionRequired, - - /// All authenticated users can see the resource (including unauthorized - /// users) but permissions are required to access. + /// All users can see the resource (including unauthenticated or + /// unauthorized users) /// - /// "/organizations", for example. - PublicPermissionRequired, + /// "/organizations" is Public, for example. + Public, - /// Only authenticated users with certain privileges can see this endpoint + /// Only users with certain privileges can see this endpoint /// /// "/organizations/demo-org" is not public, for example. Protected, } -/// Describes an HTTP method supported by a particular API endpoint, and its -/// authz visibility +/// Describes an HTTP method supported by a particular API endpoint #[derive(Debug)] -pub struct AllowedMethod { - pub method: MethodAndBody, - - /// Specifies whether an HTTP resource handled by this endpoint is visible - /// to unauthorized users. See [`Visibility`] for more information. - pub visibility: Visibility, -} - -#[derive(Debug, Clone)] -pub enum MethodAndBody { +pub enum AllowedMethod { /// HTTP "DELETE" method Delete, /// HTTP "GET" method @@ -451,16 +459,16 @@ pub enum MethodAndBody { Put(serde_json::Value), } -impl MethodAndBody { +impl AllowedMethod { /// Returns the [`http::Method`] used to make a request for this HTTP method pub fn http_method(&self) -> &'static http::Method { match self { - MethodAndBody::Delete => &Method::DELETE, - MethodAndBody::Get => &Method::GET, - MethodAndBody::GetNonexistent => &Method::GET, - MethodAndBody::GetUnimplemented => &Method::GET, - MethodAndBody::Post(_) => &Method::POST, - MethodAndBody::Put(_) => &Method::PUT, + AllowedMethod::Delete => &Method::DELETE, + AllowedMethod::Get => &Method::GET, + AllowedMethod::GetNonexistent => &Method::GET, + AllowedMethod::GetUnimplemented => &Method::GET, + AllowedMethod::Post(_) => &Method::POST, + AllowedMethod::Put(_) => &Method::PUT, } } @@ -470,12 +478,12 @@ impl MethodAndBody { /// If this returns `None`, the request body should be empty. pub fn body(&self) -> Option<&serde_json::Value> { match self { - MethodAndBody::Delete - | MethodAndBody::Get - | MethodAndBody::GetNonexistent - | MethodAndBody::GetUnimplemented => None, - MethodAndBody::Post(body) => Some(&body), - MethodAndBody::Put(body) => Some(&body), + AllowedMethod::Delete + | AllowedMethod::Get + | AllowedMethod::GetNonexistent + | AllowedMethod::GetUnimplemented => None, + AllowedMethod::Post(body) => Some(&body), + AllowedMethod::Put(body) => Some(&body), } } } @@ -489,155 +497,124 @@ lazy_static! { // Global IAM policy VerifyEndpoint { url: *POLICY_URL, - allowed_methods: vec![ - AllowedMethod { - method: MethodAndBody::Get, - visibility: Visibility::PublicPermissionRequired, - }, - AllowedMethod { - method: MethodAndBody::Put( - serde_json::to_value( - &shared::Policy:: { - role_assignments: vec![] - } - ).unwrap() - ), - visibility: Visibility::PublicPermissionRequired, - }, + visibility: Visibility::Public, + unprivileged_access: UnprivilegedAccess::None, + allowed_methods: vec![ + AllowedMethod::Get, + AllowedMethod::Put( + serde_json::to_value( + &shared::Policy:: { + role_assignments: vec![] + } + ).unwrap() + ), ], }, // IP Pools top-level endpoint VerifyEndpoint { url: *DEMO_IP_POOLS_URL, + visibility: Visibility::Public, + unprivileged_access: UnprivilegedAccess::None, allowed_methods: vec![ - AllowedMethod { - method: MethodAndBody::Get, - visibility: Visibility::PublicPermissionRequired, - }, - AllowedMethod { - method: MethodAndBody::Post( - serde_json::to_value(&*DEMO_IP_POOL_CREATE).unwrap() - ), - visibility: Visibility::PublicPermissionRequired, - }, + AllowedMethod::Get, + AllowedMethod::Post( + serde_json::to_value(&*DEMO_IP_POOL_CREATE).unwrap() + ), ], }, // Single IP Pool endpoint VerifyEndpoint { url: &*DEMO_IP_POOL_URL, - allowed_methods: vec![ - AllowedMethod { - method: MethodAndBody::Get, - visibility: Visibility::Protected, - }, - AllowedMethod { - method: MethodAndBody::Put( - serde_json::to_value(&*DEMO_IP_POOL_UPDATE).unwrap() - ), - visibility: Visibility::Protected, - }, - AllowedMethod { - method: MethodAndBody::Delete, - visibility: Visibility::Protected, - }, + visibility: Visibility::Protected, + unprivileged_access: UnprivilegedAccess::None, + allowed_methods: vec![ + AllowedMethod::Get, + AllowedMethod::Put( + serde_json::to_value(&*DEMO_IP_POOL_UPDATE).unwrap() + ), + AllowedMethod::Delete, ], }, // IP Pool ranges endpoint VerifyEndpoint { url: &*DEMO_IP_POOL_RANGES_URL, + visibility: Visibility::Protected, + unprivileged_access: UnprivilegedAccess::None, allowed_methods: vec![ - AllowedMethod { - method: MethodAndBody::Get, - visibility: Visibility::Protected, - }, + AllowedMethod::Get ], }, // IP Pool ranges/add endpoint VerifyEndpoint { url: &*DEMO_IP_POOL_RANGES_ADD_URL, + visibility: Visibility::Protected, + unprivileged_access: UnprivilegedAccess::None, allowed_methods: vec![ - AllowedMethod { - method: MethodAndBody::Post( - serde_json::to_value(&*DEMO_IP_POOL_RANGE).unwrap() - ), - visibility: Visibility::Protected, - }, + AllowedMethod::Post( + serde_json::to_value(&*DEMO_IP_POOL_RANGE).unwrap() + ), ], }, // IP Pool ranges/delete endpoint VerifyEndpoint { url: &*DEMO_IP_POOL_RANGES_DEL_URL, + visibility: Visibility::Protected, + unprivileged_access: UnprivilegedAccess::None, allowed_methods: vec![ - AllowedMethod { - method: MethodAndBody::Post( - serde_json::to_value(&*DEMO_IP_POOL_RANGE).unwrap() - ), - visibility: Visibility::Protected, - }, + AllowedMethod::Post( + serde_json::to_value(&*DEMO_IP_POOL_RANGE).unwrap() + ), ], }, /* Silos */ VerifyEndpoint { url: "/silos", + visibility: Visibility::Public, + unprivileged_access: UnprivilegedAccess::None, allowed_methods: vec![ - AllowedMethod { - method: MethodAndBody::Get, - visibility: Visibility::PublicPermissionRequired, - }, - AllowedMethod { - method: MethodAndBody::Post( - serde_json::to_value(&*DEMO_SILO_CREATE).unwrap() - ), - visibility: Visibility::PublicPermissionRequired, - }, + AllowedMethod::Get, + AllowedMethod::Post( + serde_json::to_value(&*DEMO_SILO_CREATE).unwrap() + ) ], }, VerifyEndpoint { url: &*DEMO_SILO_URL, + visibility: Visibility::Protected, + unprivileged_access: UnprivilegedAccess::None, allowed_methods: vec![ - AllowedMethod { - method: MethodAndBody::Get, - visibility: Visibility::Protected, - }, - AllowedMethod { - method: MethodAndBody::Delete, - visibility: Visibility::Protected, - }, + AllowedMethod::Get, + AllowedMethod::Delete, ], }, VerifyEndpoint { url: &*DEMO_SILO_POLICY_URL, - allowed_methods: vec![ - AllowedMethod { - method: MethodAndBody::Get, - visibility: Visibility::Protected, - }, - AllowedMethod { - method: MethodAndBody::Put( - serde_json::to_value( - &shared::Policy:: { - role_assignments: vec![] - } - ).unwrap() - ), - visibility: Visibility::Protected, - }, + visibility: Visibility::Protected, + unprivileged_access: UnprivilegedAccess::None, + allowed_methods: vec![ + AllowedMethod::Get, + AllowedMethod::Put( + serde_json::to_value( + &shared::Policy:: { + role_assignments: vec![] + } + ).unwrap() + ), ], }, VerifyEndpoint { url: "/users", + visibility: Visibility::Public, + unprivileged_access: UnprivilegedAccess::ReadOnly, allowed_methods: vec![ - AllowedMethod { - method: MethodAndBody::Get, - visibility: Visibility::PublicNoPermissionRequired, - }, + AllowedMethod::Get, ], }, @@ -645,60 +622,45 @@ lazy_static! { VerifyEndpoint { url: "/organizations", + visibility: Visibility::Public, + unprivileged_access: UnprivilegedAccess::None, allowed_methods: vec![ - AllowedMethod { - method: MethodAndBody::Get, - visibility: Visibility::PublicPermissionRequired, - }, - AllowedMethod { - method: MethodAndBody::Post( - serde_json::to_value(&*DEMO_ORG_CREATE).unwrap() - ), - visibility: Visibility::PublicPermissionRequired, - }, + AllowedMethod::Get, + AllowedMethod::Post( + serde_json::to_value(&*DEMO_ORG_CREATE).unwrap() + ) ], }, VerifyEndpoint { url: &*DEMO_ORG_URL, - allowed_methods: vec![ - AllowedMethod { - method: MethodAndBody::Get, - visibility: Visibility::Protected, - }, - AllowedMethod { - method: MethodAndBody::Delete, - visibility: Visibility::Protected, - }, - AllowedMethod { - method: MethodAndBody::Put( - serde_json::to_value(¶ms::OrganizationUpdate { - identity: IdentityMetadataUpdateParams { - name: None, - description: Some("different".to_string()) - } - }).unwrap() - ), - visibility: Visibility::Protected, - }, + visibility: Visibility::Protected, + unprivileged_access: UnprivilegedAccess::None, + allowed_methods: vec![ + AllowedMethod::Get, + AllowedMethod::Delete, + AllowedMethod::Put( + serde_json::to_value(¶ms::OrganizationUpdate { + identity: IdentityMetadataUpdateParams { + name: None, + description: Some("different".to_string()) + } + }).unwrap() + ), ], }, VerifyEndpoint { url: &*DEMO_ORG_POLICY_URL, - allowed_methods: vec![ - AllowedMethod { - method: MethodAndBody::Get, - visibility: Visibility::Protected, - }, - AllowedMethod { - method: MethodAndBody::Put( - serde_json::to_value( - &shared::Policy:: { - role_assignments: vec![] - } - ).unwrap() - ), - visibility: Visibility::Protected, - } + visibility: Visibility::Protected, + unprivileged_access: UnprivilegedAccess::None, + allowed_methods: vec![ + AllowedMethod::Get, + AllowedMethod::Put( + serde_json::to_value( + &shared::Policy:: { + role_assignments: vec![] + } + ).unwrap() + ), ], }, @@ -714,174 +676,132 @@ lazy_static! { // incredibly expensive to determine in general. VerifyEndpoint { url: &*DEMO_ORG_PROJECTS_URL, + visibility: Visibility::Protected, + unprivileged_access: UnprivilegedAccess::None, allowed_methods: vec![ - AllowedMethod { - method: MethodAndBody::Get, - visibility: Visibility::Protected, - }, - AllowedMethod { - method: MethodAndBody::Post( - serde_json::to_value(&*DEMO_PROJECT_CREATE).unwrap() - ), - visibility: Visibility::Protected, - }, + AllowedMethod::Get, + AllowedMethod::Post( + serde_json::to_value(&*DEMO_PROJECT_CREATE).unwrap() + ), ], }, VerifyEndpoint { url: &*DEMO_PROJECT_URL, - allowed_methods: vec![ - AllowedMethod { - method: MethodAndBody::Get, - visibility: Visibility::Protected, - }, - AllowedMethod { - method: MethodAndBody::Delete, - visibility: Visibility::Protected, - }, - AllowedMethod { - method: MethodAndBody::Put( - serde_json::to_value(params::ProjectUpdate{ - identity: IdentityMetadataUpdateParams { - name: None, - description: Some("different".to_string()) - }, - }).unwrap() - ), - visibility: Visibility::Protected, - }, + visibility: Visibility::Protected, + unprivileged_access: UnprivilegedAccess::None, + allowed_methods: vec![ + AllowedMethod::Get, + AllowedMethod::Delete, + AllowedMethod::Put( + serde_json::to_value(params::ProjectUpdate{ + identity: IdentityMetadataUpdateParams { + name: None, + description: Some("different".to_string()) + }, + }).unwrap() + ), ], }, VerifyEndpoint { url: &*DEMO_PROJECT_POLICY_URL, - allowed_methods: vec![ - AllowedMethod { - method: MethodAndBody::Get, - visibility: Visibility::Protected, - }, - AllowedMethod { - method: MethodAndBody::Put( - serde_json::to_value( - &shared::Policy:: { - role_assignments: vec![] - } - ).unwrap() - ), - visibility: Visibility::Protected, - }, + visibility: Visibility::Protected, + unprivileged_access: UnprivilegedAccess::None, + allowed_methods: vec![ + AllowedMethod::Get, + AllowedMethod::Put( + serde_json::to_value( + &shared::Policy:: { + role_assignments: vec![] + } + ).unwrap() + ), ], }, /* VPCs */ VerifyEndpoint { url: &*DEMO_PROJECT_URL_VPCS, + visibility: Visibility::Protected, + unprivileged_access: UnprivilegedAccess::None, allowed_methods: vec![ - AllowedMethod { - method: MethodAndBody::Get, - visibility: Visibility::Protected, - }, - AllowedMethod { - method: MethodAndBody::Post( - serde_json::to_value(&*DEMO_VPC_CREATE).unwrap() - ), - visibility: Visibility::Protected, - }, + AllowedMethod::Get, + AllowedMethod::Post( + serde_json::to_value(&*DEMO_VPC_CREATE).unwrap() + ), ], }, VerifyEndpoint { url: &*DEMO_VPC_URL, - allowed_methods: vec![ - AllowedMethod { - method: MethodAndBody::Get, - visibility: Visibility::Protected, - }, - AllowedMethod { - method: MethodAndBody::Put( - serde_json::to_value(¶ms::VpcUpdate { - identity: IdentityMetadataUpdateParams { - name: None, - description: Some("different".to_string()) - }, - dns_name: None, - }).unwrap() - ), - visibility: Visibility::Protected, - }, - AllowedMethod { - method: MethodAndBody::Delete, - visibility: Visibility::Protected, - }, + visibility: Visibility::Protected, + unprivileged_access: UnprivilegedAccess::None, + allowed_methods: vec![ + AllowedMethod::Get, + AllowedMethod::Put( + serde_json::to_value(¶ms::VpcUpdate { + identity: IdentityMetadataUpdateParams { + name: None, + description: Some("different".to_string()) + }, + dns_name: None, + }).unwrap() + ), + AllowedMethod::Delete, ], }, /* Firewall rules */ VerifyEndpoint { url: &*DEMO_VPC_URL_FIREWALL_RULES, - allowed_methods: vec![ - AllowedMethod { - method: MethodAndBody::Get, - visibility: Visibility::Protected, - }, - AllowedMethod { - method: MethodAndBody::Put( - serde_json::to_value(VpcFirewallRuleUpdateParams { - rules: vec![], - }).unwrap() - ), - visibility: Visibility::Protected, - }, + visibility: Visibility::Protected, + unprivileged_access: UnprivilegedAccess::None, + allowed_methods: vec![ + AllowedMethod::Get, + AllowedMethod::Put( + serde_json::to_value(VpcFirewallRuleUpdateParams { + rules: vec![], + }).unwrap() + ), ], }, /* VPC Subnets */ VerifyEndpoint { url: &*DEMO_VPC_URL_SUBNETS, + visibility: Visibility::Protected, + unprivileged_access: UnprivilegedAccess::None, allowed_methods: vec![ - AllowedMethod { - method: MethodAndBody::Get, - visibility: Visibility::Protected, - }, - AllowedMethod { - method: MethodAndBody::Post( - serde_json::to_value(&*DEMO_VPC_SUBNET_CREATE).unwrap() - ), - visibility: Visibility::Protected, - }, + AllowedMethod::Get, + AllowedMethod::Post( + serde_json::to_value(&*DEMO_VPC_SUBNET_CREATE).unwrap() + ), ], }, VerifyEndpoint { url: &*DEMO_VPC_SUBNET_URL, - allowed_methods: vec![ - AllowedMethod { - method: MethodAndBody::Get, - visibility: Visibility::Protected, - }, - AllowedMethod { - method: MethodAndBody::Put( - serde_json::to_value(¶ms::VpcSubnetUpdate { - identity: IdentityMetadataUpdateParams { - name: None, - description: Some("different".to_string()) - }, - }).unwrap() - ), - visibility: Visibility::Protected, - }, - AllowedMethod { - method: MethodAndBody::Delete, - visibility: Visibility::Protected, - }, + visibility: Visibility::Protected, + unprivileged_access: UnprivilegedAccess::None, + allowed_methods: vec![ + AllowedMethod::Get, + AllowedMethod::Put( + serde_json::to_value(¶ms::VpcSubnetUpdate { + identity: IdentityMetadataUpdateParams { + name: None, + description: Some("different".to_string()) + }, + }).unwrap() + ), + AllowedMethod::Delete, ], }, VerifyEndpoint { url: &*DEMO_VPC_SUBNET_INTERFACES_URL, + visibility: Visibility::Protected, + unprivileged_access: UnprivilegedAccess::None, allowed_methods: vec![ - AllowedMethod { - method: MethodAndBody::Get, - visibility: Visibility::Protected, - }, + AllowedMethod::Get, ], }, @@ -889,42 +809,31 @@ lazy_static! { VerifyEndpoint { url: &*DEMO_VPC_URL_ROUTERS, + visibility: Visibility::Protected, + unprivileged_access: UnprivilegedAccess::None, allowed_methods: vec![ - AllowedMethod { - method: MethodAndBody::Get, - visibility: Visibility::Protected, - }, - AllowedMethod { - method: MethodAndBody::Post( - serde_json::to_value(&*DEMO_VPC_ROUTER_CREATE).unwrap() - ), - visibility: Visibility::Protected, - }, + AllowedMethod::Get, + AllowedMethod::Post( + serde_json::to_value(&*DEMO_VPC_ROUTER_CREATE).unwrap() + ), ], }, VerifyEndpoint { url: &*DEMO_VPC_ROUTER_URL, - allowed_methods: vec![ - AllowedMethod { - method: MethodAndBody::Get, - visibility: Visibility::Protected, - }, - AllowedMethod { - method: MethodAndBody::Put( - serde_json::to_value(¶ms::VpcRouterUpdate { - identity: IdentityMetadataUpdateParams { - name: None, - description: Some("different".to_string()) - }, - }).unwrap() - ), - visibility: Visibility::Protected, - }, - AllowedMethod { - method: MethodAndBody::Delete, - visibility: Visibility::Protected, - }, + visibility: Visibility::Protected, + unprivileged_access: UnprivilegedAccess::None, + allowed_methods: vec![ + AllowedMethod::Get, + AllowedMethod::Put( + serde_json::to_value(¶ms::VpcRouterUpdate { + identity: IdentityMetadataUpdateParams { + name: None, + description: Some("different".to_string()) + }, + }).unwrap() + ), + AllowedMethod::Delete, ], }, @@ -932,46 +841,35 @@ lazy_static! { VerifyEndpoint { url: &*DEMO_VPC_ROUTER_URL_ROUTES, + visibility: Visibility::Protected, + unprivileged_access: UnprivilegedAccess::None, allowed_methods: vec![ - AllowedMethod { - method: MethodAndBody::Get, - visibility: Visibility::Protected, - }, - AllowedMethod { - method: MethodAndBody::Post( - serde_json::to_value(&*DEMO_ROUTER_ROUTE_CREATE).unwrap() - ), - visibility: Visibility::Protected, - }, + AllowedMethod::Get, + AllowedMethod::Post( + serde_json::to_value(&*DEMO_ROUTER_ROUTE_CREATE).unwrap() + ), ], }, VerifyEndpoint { url: &*DEMO_ROUTER_ROUTE_URL, - allowed_methods: vec![ - AllowedMethod { - method: MethodAndBody::Get, - visibility: Visibility::Protected, - }, - AllowedMethod { - method: MethodAndBody::Put( - serde_json::to_value(&RouterRouteUpdateParams { - identity: IdentityMetadataUpdateParams { - name: None, - description: Some("different".to_string()) - }, - target: RouteTarget::Ip( - IpAddr::from(Ipv4Addr::new(127, 0, 0, 1))), - destination: RouteDestination::Subnet( - "loopback".parse().unwrap()), - }).unwrap() - ), - visibility: Visibility::Protected, - }, - AllowedMethod { - method: MethodAndBody::Delete, - visibility: Visibility::Protected, - }, + visibility: Visibility::Protected, + unprivileged_access: UnprivilegedAccess::None, + allowed_methods: vec![ + AllowedMethod::Get, + AllowedMethod::Put( + serde_json::to_value(&RouterRouteUpdateParams { + identity: IdentityMetadataUpdateParams { + name: None, + description: Some("different".to_string()) + }, + target: RouteTarget::Ip( + IpAddr::from(Ipv4Addr::new(127, 0, 0, 1))), + destination: RouteDestination::Subnet( + "loopback".parse().unwrap()), + }).unwrap() + ), + AllowedMethod::Delete, ], }, @@ -979,67 +877,56 @@ lazy_static! { VerifyEndpoint { url: &*DEMO_PROJECT_URL_DISKS, + visibility: Visibility::Protected, + unprivileged_access: UnprivilegedAccess::None, allowed_methods: vec![ - AllowedMethod { - method: MethodAndBody::Get, - visibility: Visibility::Protected, - }, - AllowedMethod { - method: MethodAndBody::Post( - serde_json::to_value(&*DEMO_DISK_CREATE).unwrap() - ), - visibility: Visibility::Protected, - }, + AllowedMethod::Get, + AllowedMethod::Post( + serde_json::to_value(&*DEMO_DISK_CREATE).unwrap() + ), ], }, VerifyEndpoint { url: &*DEMO_DISK_URL, + visibility: Visibility::Protected, + unprivileged_access: UnprivilegedAccess::None, allowed_methods: vec![ - AllowedMethod { - method: MethodAndBody::Get, - visibility: Visibility::Protected, - }, - AllowedMethod { - method: MethodAndBody::Delete, - visibility: Visibility::Protected, - }, + AllowedMethod::Get, + AllowedMethod::Delete, ], }, VerifyEndpoint { url: &*DEMO_INSTANCE_DISKS_URL, + visibility: Visibility::Protected, + unprivileged_access: UnprivilegedAccess::None, allowed_methods: vec![ - AllowedMethod { - method: MethodAndBody::Get, - visibility: Visibility::Protected, - }, + AllowedMethod::Get, ], }, VerifyEndpoint { url: &*DEMO_INSTANCE_DISKS_ATTACH_URL, - allowed_methods: vec![ - AllowedMethod { - method: MethodAndBody::Post( - serde_json::to_value(params::DiskIdentifier { - name: DEMO_DISK_NAME.clone() - }).unwrap() - ), - visibility: Visibility::Protected, - }, + visibility: Visibility::Protected, + unprivileged_access: UnprivilegedAccess::None, + allowed_methods: vec![ + AllowedMethod::Post( + serde_json::to_value(params::DiskIdentifier { + name: DEMO_DISK_NAME.clone() + }).unwrap() + ) ], }, VerifyEndpoint { url: &*DEMO_INSTANCE_DISKS_DETACH_URL, - allowed_methods: vec![ - AllowedMethod { - method: MethodAndBody::Post( - serde_json::to_value(params::DiskIdentifier { - name: DEMO_DISK_NAME.clone() - }).unwrap() - ), - visibility: Visibility::Protected, - }, + visibility: Visibility::Protected, + unprivileged_access: UnprivilegedAccess::None, + allowed_methods: vec![ + AllowedMethod::Post( + serde_json::to_value(params::DiskIdentifier { + name: DEMO_DISK_NAME.clone() + }).unwrap() + ) ], }, @@ -1047,31 +934,23 @@ lazy_static! { VerifyEndpoint { url: &*DEMO_PROJECT_URL_IMAGES, + visibility: Visibility::Protected, + unprivileged_access: UnprivilegedAccess::None, allowed_methods: vec![ - AllowedMethod { - method: MethodAndBody::GetUnimplemented, - visibility: Visibility::Protected, - }, - AllowedMethod { - method: MethodAndBody::Post( - serde_json::to_value(&*DEMO_IMAGE_CREATE).unwrap() - ), - visibility: Visibility::Protected, - }, + AllowedMethod::GetUnimplemented, + AllowedMethod::Post( + serde_json::to_value(&*DEMO_IMAGE_CREATE).unwrap() + ), ], }, VerifyEndpoint { url: &*DEMO_PROJECT_IMAGE_URL, + visibility: Visibility::Protected, + unprivileged_access: UnprivilegedAccess::None, allowed_methods: vec![ - AllowedMethod { - method: MethodAndBody::GetUnimplemented, - visibility: Visibility::Protected, - }, - AllowedMethod { - method: MethodAndBody::Delete, - visibility: Visibility::Protected, - }, + AllowedMethod::GetUnimplemented, + AllowedMethod::Delete, ], }, @@ -1079,147 +958,115 @@ lazy_static! { VerifyEndpoint { url: &*DEMO_PROJECT_URL_SNAPSHOTS, + visibility: Visibility::Protected, + unprivileged_access: UnprivilegedAccess::None, allowed_methods: vec![ - AllowedMethod { - method: MethodAndBody::GetUnimplemented, - visibility: Visibility::Protected, - }, - AllowedMethod { - method: MethodAndBody::Post( - serde_json::to_value(DEMO_SNAPSHOT_CREATE.clone()).unwrap(), - ), - visibility: Visibility::Protected, - }, + AllowedMethod::GetUnimplemented, + AllowedMethod::Post( + serde_json::to_value(DEMO_SNAPSHOT_CREATE.clone()).unwrap(), + ) ] }, VerifyEndpoint { url: &*DEMO_SNAPSHOT_URL, + visibility: Visibility::Protected, + unprivileged_access: UnprivilegedAccess::None, allowed_methods: vec![ - AllowedMethod { - method: MethodAndBody::GetUnimplemented, - visibility: Visibility::Protected, - }, - AllowedMethod { - method: MethodAndBody::Delete, - visibility: Visibility::Protected, - }, + AllowedMethod::GetUnimplemented, + AllowedMethod::Delete, ] }, /* Instances */ VerifyEndpoint { url: &*DEMO_PROJECT_URL_INSTANCES, + visibility: Visibility::Protected, + unprivileged_access: UnprivilegedAccess::None, allowed_methods: vec![ - AllowedMethod { - method: MethodAndBody::Get, - visibility: Visibility::Protected, - }, - AllowedMethod { - method: MethodAndBody::Post( - serde_json::to_value(&*DEMO_INSTANCE_CREATE).unwrap() - ), - visibility: Visibility::Protected, - }, + AllowedMethod::Get, + AllowedMethod::Post( + serde_json::to_value(&*DEMO_INSTANCE_CREATE).unwrap() + ), ], }, VerifyEndpoint { url: &*DEMO_INSTANCE_URL, + visibility: Visibility::Protected, + unprivileged_access: UnprivilegedAccess::None, allowed_methods: vec![ - AllowedMethod { - method: MethodAndBody::Get, - visibility: Visibility::Protected, - }, - AllowedMethod { - method: MethodAndBody::Delete, - visibility: Visibility::Protected, - }, + AllowedMethod::Get, + AllowedMethod::Delete, ], }, VerifyEndpoint { url: &*DEMO_INSTANCE_START_URL, + visibility: Visibility::Protected, + unprivileged_access: UnprivilegedAccess::None, allowed_methods: vec![ - AllowedMethod { - method: MethodAndBody::Post(serde_json::Value::Null), - visibility: Visibility::Protected, - }, + AllowedMethod::Post(serde_json::Value::Null) ], }, VerifyEndpoint { url: &*DEMO_INSTANCE_STOP_URL, + visibility: Visibility::Protected, + unprivileged_access: UnprivilegedAccess::None, allowed_methods: vec![ - AllowedMethod { - method: MethodAndBody::Post(serde_json::Value::Null), - visibility: Visibility::Protected, - }, + AllowedMethod::Post(serde_json::Value::Null) ], }, VerifyEndpoint { url: &*DEMO_INSTANCE_REBOOT_URL, + visibility: Visibility::Protected, + unprivileged_access: UnprivilegedAccess::None, allowed_methods: vec![ - AllowedMethod { - method: MethodAndBody::Post(serde_json::Value::Null), - visibility: Visibility::Protected, - }, + AllowedMethod::Post(serde_json::Value::Null) ], }, VerifyEndpoint { url: &*DEMO_INSTANCE_MIGRATE_URL, - allowed_methods: vec![ - AllowedMethod { - method: MethodAndBody::Post(serde_json::to_value( - params::InstanceMigrate { - dst_sled_id: uuid::Uuid::new_v4(), - } - ).unwrap()), - visibility: Visibility::Protected, - }, + visibility: Visibility::Protected, + unprivileged_access: UnprivilegedAccess::None, + allowed_methods: vec![ + AllowedMethod::Post(serde_json::to_value( + params::InstanceMigrate { + dst_sled_id: uuid::Uuid::new_v4(), + } + ).unwrap()), ], }, VerifyEndpoint { url: &*DEMO_INSTANCE_SERIAL_URL, + visibility: Visibility::Protected, + unprivileged_access: UnprivilegedAccess::None, allowed_methods: vec![ - AllowedMethod { - method: MethodAndBody::GetNonexistent, // has required query parameters - visibility: Visibility::Protected, - }, + AllowedMethod::GetNonexistent // has required query parameters ], }, /* Instance NICs */ VerifyEndpoint { url: &*DEMO_INSTANCE_NICS_URL, + visibility: Visibility::Protected, + unprivileged_access: UnprivilegedAccess::None, allowed_methods: vec![ - AllowedMethod { - method: MethodAndBody::Get, - visibility: Visibility::Protected, - }, - AllowedMethod { - method: MethodAndBody::Post( - serde_json::to_value(&*DEMO_INSTANCE_NIC_CREATE).unwrap() - ), - visibility: Visibility::Protected, - }, + AllowedMethod::Get, + AllowedMethod::Post( + serde_json::to_value(&*DEMO_INSTANCE_NIC_CREATE).unwrap() + ), ], }, VerifyEndpoint { url: &*DEMO_INSTANCE_NIC_URL, - allowed_methods: vec![ - AllowedMethod { - method: MethodAndBody::Get, - visibility: Visibility::Protected, - }, - AllowedMethod { - method: MethodAndBody::Delete, - visibility: Visibility::Protected, - }, - AllowedMethod { - method: MethodAndBody::Put( - serde_json::to_value(&*DEMO_INSTANCE_NIC_PUT).unwrap() - ), - visibility: Visibility::Protected, - }, + visibility: Visibility::Protected, + unprivileged_access: UnprivilegedAccess::None, + allowed_methods: vec![ + AllowedMethod::Get, + AllowedMethod::Delete, + AllowedMethod::Put( + serde_json::to_value(&*DEMO_INSTANCE_NIC_PUT).unwrap() + ), ], }, @@ -1227,161 +1074,117 @@ lazy_static! { VerifyEndpoint { url: "/roles", - allowed_methods: vec![ - AllowedMethod { - method: MethodAndBody::Get, - visibility: Visibility::PublicPermissionRequired, - }, - ], + visibility: Visibility::Public, + unprivileged_access: UnprivilegedAccess::None, + allowed_methods: vec![AllowedMethod::Get], }, VerifyEndpoint { url: "/roles/fleet.admin", - allowed_methods: vec![ - AllowedMethod { - method: MethodAndBody::Get, - visibility: Visibility::Protected, - }, - ], + visibility: Visibility::Protected, + unprivileged_access: UnprivilegedAccess::None, + allowed_methods: vec![AllowedMethod::Get], }, VerifyEndpoint { url: "/users_builtin", - allowed_methods: vec![ - AllowedMethod { - method: MethodAndBody::Get, - visibility: Visibility::PublicPermissionRequired, - }, - ], + visibility: Visibility::Public, + unprivileged_access: UnprivilegedAccess::None, + allowed_methods: vec![AllowedMethod::Get], }, VerifyEndpoint { url: &*URL_USERS_DB_INIT, - allowed_methods: vec![ - AllowedMethod { - method: MethodAndBody::Get, - visibility: Visibility::Protected, - }, - ], + visibility: Visibility::Protected, + unprivileged_access: UnprivilegedAccess::None, + allowed_methods: vec![AllowedMethod::Get], }, /* Hardware */ VerifyEndpoint { url: "/hardware/racks", - allowed_methods: vec![ - AllowedMethod { - method: MethodAndBody::Get, - visibility: Visibility::PublicPermissionRequired, - }, - ], + visibility: Visibility::Public, + unprivileged_access: UnprivilegedAccess::None, + allowed_methods: vec![AllowedMethod::Get], }, VerifyEndpoint { url: &*HARDWARE_RACK_URL, - allowed_methods: vec![ - AllowedMethod { - method: MethodAndBody::Get, - visibility: Visibility::Protected, - }, - ], + visibility: Visibility::Protected, + unprivileged_access: UnprivilegedAccess::None, + allowed_methods: vec![AllowedMethod::Get], }, VerifyEndpoint { url: "/hardware/sleds", - allowed_methods: vec![ - AllowedMethod { - method: MethodAndBody::Get, - visibility: Visibility::PublicPermissionRequired, - }, - ], + visibility: Visibility::Public, + unprivileged_access: UnprivilegedAccess::None, + allowed_methods: vec![AllowedMethod::Get], }, VerifyEndpoint { url: &*HARDWARE_SLED_URL, - allowed_methods: vec![ - AllowedMethod { - method: MethodAndBody::Get, - visibility: Visibility::Protected, - }, - ], + visibility: Visibility::Protected, + unprivileged_access: UnprivilegedAccess::None, + allowed_methods: vec![AllowedMethod::Get], }, /* Sagas */ VerifyEndpoint { url: "/sagas", - allowed_methods: vec![ - AllowedMethod { - method: MethodAndBody::Get, - visibility: Visibility::PublicPermissionRequired, - }, - ], + visibility: Visibility::Public, + unprivileged_access: UnprivilegedAccess::None, + allowed_methods: vec![AllowedMethod::Get], }, VerifyEndpoint { url: "/sagas/48a1b8c8-fc1c-6fea-9de9-fdeb8dda7823", - allowed_methods: vec![ - AllowedMethod { - method: MethodAndBody::GetNonexistent, - visibility: Visibility::PublicPermissionRequired, - }, - ], + visibility: Visibility::Public, + unprivileged_access: UnprivilegedAccess::None, + allowed_methods: vec![AllowedMethod::GetNonexistent], }, /* Timeseries schema */ VerifyEndpoint { url: "/timeseries/schema", - allowed_methods: vec![ - AllowedMethod { - method: MethodAndBody::Get, - visibility: Visibility::PublicPermissionRequired, - }, - ], + visibility: Visibility::Public, + unprivileged_access: UnprivilegedAccess::None, + allowed_methods: vec![AllowedMethod::Get], }, /* Updates */ VerifyEndpoint { url: "/updates/refresh", - allowed_methods: vec![ - AllowedMethod { - method: MethodAndBody::Post( - serde_json::Value::Null - ), - visibility: Visibility::PublicPermissionRequired, - }, - ], + visibility: Visibility::Public, + unprivileged_access: UnprivilegedAccess::None, + allowed_methods: vec![AllowedMethod::Post( + serde_json::Value::Null + )], }, /* Global Images */ VerifyEndpoint { url: "/images", + visibility: Visibility::Public, + unprivileged_access: UnprivilegedAccess::ReadOnly, allowed_methods: vec![ - AllowedMethod { - method: MethodAndBody::Get, - visibility: Visibility::PublicNoPermissionRequired, - }, - AllowedMethod { - method: MethodAndBody::Post( - serde_json::to_value(&*DEMO_GLOBAL_IMAGE_CREATE).unwrap() - ), - visibility: Visibility::PublicPermissionRequired, - }, + AllowedMethod::Get, + AllowedMethod::Post( + serde_json::to_value(&*DEMO_GLOBAL_IMAGE_CREATE).unwrap() + ), ], }, VerifyEndpoint { url: &*DEMO_GLOBAL_IMAGE_URL, + visibility: Visibility::Protected, + unprivileged_access: UnprivilegedAccess::ReadOnly, allowed_methods: vec![ - AllowedMethod { - method: MethodAndBody::Get, - visibility: Visibility::PublicNoPermissionRequired, - }, - AllowedMethod { - method: MethodAndBody::Delete, - visibility: Visibility::Protected, // XXX is this right? - }, + AllowedMethod::Get, + AllowedMethod::Delete, ], }, @@ -1389,76 +1192,62 @@ lazy_static! { VerifyEndpoint { url: &*IDENTITY_PROVIDERS_URL, + visibility: Visibility::Public, + unprivileged_access: UnprivilegedAccess::ReadOnly, allowed_methods: vec![ - AllowedMethod { - method: MethodAndBody::Get, - visibility: Visibility::PublicNoPermissionRequired, - }, + AllowedMethod::Get, ], }, VerifyEndpoint { url: &*SAML_IDENTITY_PROVIDERS_URL, - allowed_methods: vec![ - AllowedMethod { - method: MethodAndBody::Post( - serde_json::to_value(&*SAML_IDENTITY_PROVIDER).unwrap(), - ), - visibility: Visibility::PublicPermissionRequired, - }, - ], + visibility: Visibility::Public, + unprivileged_access: UnprivilegedAccess::None, + allowed_methods: vec![AllowedMethod::Post( + serde_json::to_value(&*SAML_IDENTITY_PROVIDER).unwrap(), + )], }, VerifyEndpoint { url: &*SPECIFIC_SAML_IDENTITY_PROVIDER_URL, - allowed_methods: vec![ - AllowedMethod { - method: MethodAndBody::Get, - visibility: Visibility::Protected, - }, - ], + visibility: Visibility::Protected, + unprivileged_access: UnprivilegedAccess::None, + allowed_methods: vec![AllowedMethod::Get], }, /* Misc */ VerifyEndpoint { url: "/session/me", + visibility: Visibility::Public, + unprivileged_access: UnprivilegedAccess::ReadOnly, allowed_methods: vec![ - AllowedMethod { - method: MethodAndBody::Get, - visibility: Visibility::PublicNoPermissionRequired, - }, + AllowedMethod::Get, ], }, /* SSH keys */ + /* TODO fix authz policy VerifyEndpoint { url: &*DEMO_SSHKEYS_URL, + visibility: Visibility::Protected, + unprivileged_access: UnprivilegedAccess::Full, allowed_methods: vec![ - AllowedMethod { - method: MethodAndBody::Get, - visibility: Visibility::Protected, // XXX is this right? - }, - AllowedMethod { - method: MethodAndBody::Post( - serde_json::to_value(&*DEMO_SSHKEY_CREATE).unwrap(), - ), - visibility: Visibility::Protected, // XXX is this right? - }, + AllowedMethod::Get, + AllowedMethod::Post( + serde_json::to_value(&*DEMO_SSHKEY_CREATE).unwrap(), + ), ], }, VerifyEndpoint { url: &*DEMO_SPECIFIC_SSHKEY_URL, + visibility: Visibility::Protected, + unprivileged_access: UnprivilegedAccess::Full, allowed_methods: vec![ - AllowedMethod { - method: MethodAndBody::Get, - visibility: Visibility::Protected, // XXX is this right? - }, - AllowedMethod { - method: MethodAndBody::Delete, - visibility: Visibility::Protected, // XXX is this right? - }, + AllowedMethod::Get, + AllowedMethod::Delete, ], }, + */ ]; } diff --git a/nexus/tests/integration_tests/unauthorized.rs b/nexus/tests/integration_tests/unauthorized.rs index 5b127153ef0..7f0c5ba6b03 100644 --- a/nexus/tests/integration_tests/unauthorized.rs +++ b/nexus/tests/integration_tests/unauthorized.rs @@ -290,18 +290,11 @@ async fn verify_endpoint( // response. Otherwise, the test might later succeed by coincidence. We // might find a 404 because of something that actually doesn't exist rather // than something that's just hidden from unauthorized users. - let get_allowed = endpoint - .allowed_methods - .iter() - .find(|allowed| { - matches!( - allowed.method, - MethodAndBody::Get | MethodAndBody::GetUnimplemented - ) - }) - .map(|x| x.method.clone()); + let get_allowed = endpoint.allowed_methods.iter().find(|allowed| { + matches!(allowed, AllowedMethod::Get | AllowedMethod::GetUnimplemented) + }); let resource_before = match get_allowed { - Some(MethodAndBody::Get) => { + Some(AllowedMethod::Get) => { info!(log, "test: privileged GET"); record_operation(WhichTest::PrivilegedGet(Some( &http::StatusCode::OK, @@ -316,7 +309,7 @@ async fn verify_endpoint( .unwrap(), ) } - Some(MethodAndBody::GetUnimplemented) => { + Some(AllowedMethod::GetUnimplemented) => { info!(log, "test: privileged GET (unimplemented)"); let expected_status = http::StatusCode::INTERNAL_SERVER_ERROR; record_operation(WhichTest::PrivilegedGet(Some(&expected_status))); @@ -350,43 +343,46 @@ async fn verify_endpoint( let allowed = endpoint .allowed_methods .iter() - .find(|allowed| method == *allowed.method.http_method()); - - let body = allowed.and_then(|a| a.method.body()).cloned(); + .find(|allowed| method == *allowed.http_method()); - info!( - log, - "processing endpoint {:?} allowed {:?} method {:?}", - endpoint, - allowed, - method - ); + let body = allowed.and_then(|a| a.body()).cloned(); // First, make an authenticated, unauthorized request. info!(log, "test: authenticated, unauthorized"; "method" => ?method, "body" => ?body); let expected_status = match allowed { - Some(allowed) => { + Some(_) => { // Determine the expected status code for authenticated, unauthorized // requests, based on the endpoint's visibility. If set to // "IfAuthenticated", the endpoint is visible to any authenticated user. - match allowed.visibility { + match endpoint.unprivileged_access { // can see and access, but only for GET - Visibility::PublicNoPermissionRequired => match method { + UnprivilegedAccess::ReadOnly => match method { + Method::GET => StatusCode::OK, + + _ => match endpoint.visibility { + Visibility::Public => StatusCode::FORBIDDEN, + Visibility::Protected => StatusCode::NOT_FOUND, + }, + }, + + // authenticated users always have full CRUD permissions + UnprivilegedAccess::Full => match method { Method::GET => StatusCode::OK, + Method::POST => StatusCode::CREATED, + Method::PUT => StatusCode::OK, + Method::DELETE => StatusCode::NO_CONTENT, + _ => panic!( - "Visibility::PublicNoPermissionRequired for {}", - method + "UnprivilegedAccess::Full with method {}", + method, ), }, - // can see, but not access - Visibility::PublicPermissionRequired => { - StatusCode::FORBIDDEN - } - - // cannot see - Visibility::Protected => StatusCode::NOT_FOUND, + UnprivilegedAccess::None => match endpoint.visibility { + Visibility::Public => StatusCode::FORBIDDEN, + Visibility::Protected => StatusCode::NOT_FOUND, + }, } } @@ -509,11 +505,17 @@ async fn verify_endpoint( println!(" {}", endpoint.url); } -/// Verifies the body of an HTTP response for status codes 401, 403, 404, or 405 +/// Verifies the body of an HTTP response for status codes 401, 403, 404, or +/// 405. If the response is expected to be a 200 level, match and return early - +/// do not attempt to parse the response body. fn verify_response(response: &TestResponse, expected_status: &StatusCode) { - // If processing a visible endpoint - if *expected_status == StatusCode::OK { - return; + match *expected_status { + StatusCode::OK | StatusCode::CREATED | StatusCode::NO_CONTENT => { + assert_eq!(response.status, *expected_status); + return; + } + + _ => {} } let error: HttpErrorResponseBody = response.parsed_body().unwrap(); diff --git a/nexus/tests/integration_tests/unauthorized_coverage.rs b/nexus/tests/integration_tests/unauthorized_coverage.rs index a89b354007a..e92a5af98be 100644 --- a/nexus/tests/integration_tests/unauthorized_coverage.rs +++ b/nexus/tests/integration_tests/unauthorized_coverage.rs @@ -81,8 +81,7 @@ fn test_unauthorized_coverage() { ); for v in &*VERIFY_ENDPOINTS { for m in &v.allowed_methods { - let method_string = - m.method.http_method().to_string().to_uppercase(); + let method_string = m.http_method().to_string().to_uppercase(); let found = spec_operations.iter().find(|(op, regex)| { op.method.to_uppercase() == method_string && regex.is_match(v.url) diff --git a/nexus/tests/output/uncovered-authz-endpoints.txt b/nexus/tests/output/uncovered-authz-endpoints.txt index 0088161352d..1519be59bbb 100644 --- a/nexus/tests/output/uncovered-authz-endpoints.txt +++ b/nexus/tests/output/uncovered-authz-endpoints.txt @@ -1,9 +1,13 @@ API endpoints with no coverage in authz tests: +sshkeys_delete_key (delete "/session/me/sshkeys/{ssh_key_name}") device_auth_verify (get "/device/verify") login (get "/login/{silo_name}/{provider_name}") +sshkeys_get (get "/session/me/sshkeys") +sshkeys_get_key (get "/session/me/sshkeys/{ssh_key_name}") device_auth_request (post "/device/auth") device_auth_confirm (post "/device/confirm") device_access_token (post "/device/token") spoof_login (post "/login") consume_credentials (post "/login/{silo_name}/{provider_name}") logout (post "/logout") +sshkeys_post (post "/session/me/sshkeys") From 79de404e89dffe61c686a2b4d907dd6a45ffbcd8 Mon Sep 17 00:00:00 2001 From: James MacMahon Date: Thu, 7 Jul 2022 10:39:36 -0400 Subject: [PATCH 03/13] correct comments --- nexus/tests/integration_tests/unauthorized.rs | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/nexus/tests/integration_tests/unauthorized.rs b/nexus/tests/integration_tests/unauthorized.rs index 7f0c5ba6b03..36fbea36e20 100644 --- a/nexus/tests/integration_tests/unauthorized.rs +++ b/nexus/tests/integration_tests/unauthorized.rs @@ -353,10 +353,10 @@ async fn verify_endpoint( let expected_status = match allowed { Some(_) => { // Determine the expected status code for authenticated, unauthorized - // requests, based on the endpoint's visibility. If set to - // "IfAuthenticated", the endpoint is visible to any authenticated user. + // requests, based on the endpoint's visibility and unprivileged + // access setting. match endpoint.unprivileged_access { - // can see and access, but only for GET + // authenticated users can only GET UnprivilegedAccess::ReadOnly => match method { Method::GET => StatusCode::OK, @@ -379,6 +379,7 @@ async fn verify_endpoint( ), }, + // authenticated users have no permissions UnprivilegedAccess::None => match endpoint.visibility { Visibility::Public => StatusCode::FORBIDDEN, Visibility::Protected => StatusCode::NOT_FOUND, From d525f41a1c2302cbbb1caed623ae33f2d4acf4e9 Mon Sep 17 00:00:00 2001 From: James MacMahon Date: Thu, 7 Jul 2022 11:53:14 -0400 Subject: [PATCH 04/13] actor.silo is not a list, use = --- nexus/src/authz/omicron.polar | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/nexus/src/authz/omicron.polar b/nexus/src/authz/omicron.polar index c3d067409c8..e3f4d02074a 100644 --- a/nexus/src/authz/omicron.polar +++ b/nexus/src/authz/omicron.polar @@ -162,14 +162,14 @@ has_relation(fleet: Fleet, "parent_fleet", silo: Silo) has_permission(actor: AuthenticatedActor, "read", silo: Silo) # TODO-security TODO-coverage We should have a test that exercises this # syntax. - if silo in actor.silo; + if silo = actor.silo; # Any authenticated user should be allowed to list the identity providers of # their silo. has_permission(actor: AuthenticatedActor, "list_identity_providers", silo: Silo) # TODO-security TODO-coverage We should have a test that exercises this # syntax. - if silo in actor.silo; + if silo = actor.silo; resource Organization { permissions = [ From cc0b58d1f1bd2c7dbd23ca327fed32d15624aade Mon Sep 17 00:00:00 2001 From: James MacMahon Date: Thu, 7 Jul 2022 12:38:51 -0400 Subject: [PATCH 05/13] make ssh keys work --- nexus/src/authz/actor.rs | 4 ++++ nexus/src/authz/omicron.polar | 21 +++++++++++++++++++-- nexus/tests/integration_tests/endpoints.rs | 2 -- 3 files changed, 23 insertions(+), 4 deletions(-) diff --git a/nexus/src/authz/actor.rs b/nexus/src/authz/actor.rs index 1547dca7c71..21651b47a5b 100644 --- a/nexus/src/authz/actor.rs +++ b/nexus/src/authz/actor.rs @@ -6,6 +6,7 @@ use super::roles::RoleSet; use crate::authn; +use crate::authz::SiloUser; use omicron_common::api::external::LookupType; use omicron_common::api::external::ResourceType; use uuid::Uuid; @@ -90,5 +91,8 @@ impl oso::PolarClass for AuthenticatedActor { ) }) }) + .add_method("equals_silo_user", |a: &AuthenticatedActor, u: SiloUser| { + a.actor_id == u.id() + }) } } diff --git a/nexus/src/authz/omicron.polar b/nexus/src/authz/omicron.polar index e3f4d02074a..750b743efe4 100644 --- a/nexus/src/authz/omicron.polar +++ b/nexus/src/authz/omicron.polar @@ -162,14 +162,18 @@ has_relation(fleet: Fleet, "parent_fleet", silo: Silo) has_permission(actor: AuthenticatedActor, "read", silo: Silo) # TODO-security TODO-coverage We should have a test that exercises this # syntax. - if silo = actor.silo; + # TODO actor.silo is *not* a list, so `in` is incorrect here, but if you + # replace that with `=` it fails! + if silo in actor.silo; # Any authenticated user should be allowed to list the identity providers of # their silo. has_permission(actor: AuthenticatedActor, "list_identity_providers", silo: Silo) # TODO-security TODO-coverage We should have a test that exercises this # syntax. - if silo = actor.silo; + # TODO actor.silo is *not* a list, so `in` is incorrect here, but if you + # replace that with `=` it fails! + if silo in actor.silo; resource Organization { permissions = [ @@ -240,15 +244,28 @@ resource SiloUser { "create_child", ]; + roles = ["admin", "viewer"]; + relations = { parent_silo: Silo }; "list_children" if "viewer" on "parent_silo"; "read" if "viewer" on "parent_silo"; "modify" if "admin" on "parent_silo"; "create_child" if "admin" on "parent_silo"; + + "list_children" if "viewer"; + "read" if "viewer"; + "modify" if "admin"; + "create_child" if "admin"; + + "viewer" if "admin"; } has_relation(silo: Silo, "parent_silo", user: SiloUser) if user.silo = silo; +# authenticated actors can administrate themselves +has_role(actor: AuthenticatedActor, "admin", silo_user: SiloUser) + if actor.equals_silo_user(silo_user); + resource SshKey { permissions = [ "read", "modify" ]; relations = { silo_user: SiloUser }; diff --git a/nexus/tests/integration_tests/endpoints.rs b/nexus/tests/integration_tests/endpoints.rs index 4205defeea9..d9059c0e044 100644 --- a/nexus/tests/integration_tests/endpoints.rs +++ b/nexus/tests/integration_tests/endpoints.rs @@ -1227,7 +1227,6 @@ lazy_static! { /* SSH keys */ - /* TODO fix authz policy VerifyEndpoint { url: &*DEMO_SSHKEYS_URL, visibility: Visibility::Protected, @@ -1248,6 +1247,5 @@ lazy_static! { AllowedMethod::Delete, ], }, - */ ]; } From d0ced98bbfba341e61323d5e8b66d8c10add8d21 Mon Sep 17 00:00:00 2001 From: James MacMahon Date: Thu, 7 Jul 2022 12:59:53 -0400 Subject: [PATCH 06/13] remove roles that do not exist in the db fmt, clippy, and uncovered-authz-endpoints.txt update --- nexus/src/authz/actor.rs | 7 ++++--- nexus/src/authz/omicron.polar | 19 ++++++++----------- .../output/uncovered-authz-endpoints.txt | 4 ---- 3 files changed, 12 insertions(+), 18 deletions(-) diff --git a/nexus/src/authz/actor.rs b/nexus/src/authz/actor.rs index 21651b47a5b..55812ddb6fc 100644 --- a/nexus/src/authz/actor.rs +++ b/nexus/src/authz/actor.rs @@ -91,8 +91,9 @@ impl oso::PolarClass for AuthenticatedActor { ) }) }) - .add_method("equals_silo_user", |a: &AuthenticatedActor, u: SiloUser| { - a.actor_id == u.id() - }) + .add_method( + "equals_silo_user", + |a: &AuthenticatedActor, u: SiloUser| a.actor_id == u.id(), + ) } } diff --git a/nexus/src/authz/omicron.polar b/nexus/src/authz/omicron.polar index 750b743efe4..0ccbe56a7dc 100644 --- a/nexus/src/authz/omicron.polar +++ b/nexus/src/authz/omicron.polar @@ -244,26 +244,23 @@ resource SiloUser { "create_child", ]; - roles = ["admin", "viewer"]; - relations = { parent_silo: Silo }; "list_children" if "viewer" on "parent_silo"; "read" if "viewer" on "parent_silo"; "modify" if "admin" on "parent_silo"; "create_child" if "admin" on "parent_silo"; - - "list_children" if "viewer"; - "read" if "viewer"; - "modify" if "admin"; - "create_child" if "admin"; - - "viewer" if "admin"; } has_relation(silo: Silo, "parent_silo", user: SiloUser) if user.silo = silo; -# authenticated actors can administrate themselves -has_role(actor: AuthenticatedActor, "admin", silo_user: SiloUser) +# authenticated actors have all permissions on themselves +has_permission(actor: AuthenticatedActor, "list_children", silo_user: SiloUser) + if actor.equals_silo_user(silo_user); +has_permission(actor: AuthenticatedActor, "modify", silo_user: SiloUser) + if actor.equals_silo_user(silo_user); +has_permission(actor: AuthenticatedActor, "read", silo_user: SiloUser) + if actor.equals_silo_user(silo_user); +has_permission(actor: AuthenticatedActor, "create_child", silo_user: SiloUser) if actor.equals_silo_user(silo_user); resource SshKey { diff --git a/nexus/tests/output/uncovered-authz-endpoints.txt b/nexus/tests/output/uncovered-authz-endpoints.txt index 80827bbc6f4..0088161352d 100644 --- a/nexus/tests/output/uncovered-authz-endpoints.txt +++ b/nexus/tests/output/uncovered-authz-endpoints.txt @@ -1,13 +1,9 @@ API endpoints with no coverage in authz tests: -session_sshkey_delete (delete "/session/me/sshkeys/{ssh_key_name}") device_auth_verify (get "/device/verify") login (get "/login/{silo_name}/{provider_name}") -session_sshkey_list (get "/session/me/sshkeys") -session_sshkey_view (get "/session/me/sshkeys/{ssh_key_name}") device_auth_request (post "/device/auth") device_auth_confirm (post "/device/confirm") device_access_token (post "/device/token") spoof_login (post "/login") consume_credentials (post "/login/{silo_name}/{provider_name}") logout (post "/logout") -session_sshkey_create (post "/session/me/sshkeys") From 88d8c73e2bd5b99f9ccbef81a2937ffe701e899a Mon Sep 17 00:00:00 2001 From: James MacMahon Date: Thu, 7 Jul 2022 15:41:45 -0400 Subject: [PATCH 07/13] blanket permission statement --- nexus/src/authz/omicron.polar | 8 +------- 1 file changed, 1 insertion(+), 7 deletions(-) diff --git a/nexus/src/authz/omicron.polar b/nexus/src/authz/omicron.polar index 0ccbe56a7dc..045a1abb54f 100644 --- a/nexus/src/authz/omicron.polar +++ b/nexus/src/authz/omicron.polar @@ -254,13 +254,7 @@ has_relation(silo: Silo, "parent_silo", user: SiloUser) if user.silo = silo; # authenticated actors have all permissions on themselves -has_permission(actor: AuthenticatedActor, "list_children", silo_user: SiloUser) - if actor.equals_silo_user(silo_user); -has_permission(actor: AuthenticatedActor, "modify", silo_user: SiloUser) - if actor.equals_silo_user(silo_user); -has_permission(actor: AuthenticatedActor, "read", silo_user: SiloUser) - if actor.equals_silo_user(silo_user); -has_permission(actor: AuthenticatedActor, "create_child", silo_user: SiloUser) +has_permission(actor: AuthenticatedActor, _perm: String, silo_user: SiloUser) if actor.equals_silo_user(silo_user); resource SshKey { From 10c7e99e135767a0ef482ddaac8d97f2e72f7e31 Mon Sep 17 00:00:00 2001 From: James MacMahon Date: Thu, 7 Jul 2022 16:23:21 -0400 Subject: [PATCH 08/13] Roll back changes to unauthorized.rs Roll back changes to unauthorized.rs (mostly) and add specific authz tests for policy that allows unprivileged access. --- nexus/tests/integration_tests/authz.rs | 305 ++++++++++++++++++ nexus/tests/integration_tests/endpoints.rs | 2 +- nexus/tests/integration_tests/mod.rs | 1 + nexus/tests/integration_tests/unauthorized.rs | 106 +++--- .../unauthorized_coverage.rs | 2 + 5 files changed, 346 insertions(+), 70 deletions(-) create mode 100644 nexus/tests/integration_tests/authz.rs diff --git a/nexus/tests/integration_tests/authz.rs b/nexus/tests/integration_tests/authz.rs new file mode 100644 index 00000000000..85a8f053eb4 --- /dev/null +++ b/nexus/tests/integration_tests/authz.rs @@ -0,0 +1,305 @@ +// This Source Code Form is subject to the terms of the Mozilla Public +// License, v. 2.0. If a copy of the MPL was not distributed with this +// file, You can obtain one at https://mozilla.org/MPL/2.0/. + +//! Tests for authz policy not covered in the set of unauthorized tests + +use nexus_test_utils::http_testing::{AuthnMode, NexusRequest}; +use nexus_test_utils::ControlPlaneTestContext; +use nexus_test_utils_macros::nexus_test; + +use omicron_common::api::external::IdentityMetadataCreateParams; +use omicron_nexus::external_api::params; +use omicron_nexus::external_api::shared; +use omicron_nexus::external_api::views; +use omicron_nexus::TestInterfaces; + +use dropshot::ResultsPage; +use nexus_test_utils::resource_helpers::create_silo; + +use httptest::{matchers::*, responders::*, Expectation, ServerBuilder}; + +use uuid::Uuid; + + +// Test that an authenticated, unprivileged user has full CRUD access to their SSH keys +#[nexus_test] +async fn test_ssh_key_crud_for_unpriv(cptestctx: &ControlPlaneTestContext) { + let client = &cptestctx.external_client; + let nexus = &cptestctx.server.apictx.nexus; + + // Create a silo with an unprivileged user + let silo = create_silo( + &client, + "authz", + true, + shared::UserProvisionType::Fixed, + ) + .await; + + let new_silo_user_id = Uuid::new_v4(); + nexus + .silo_user_create( + silo.identity.id, + new_silo_user_id, + "unpriv".into(), + ) + .await + .unwrap(); + + let name = "akey"; + let description = "authz test"; + let public_key = "AAAAAAAAAAAAAAA"; + + // Create a key + let _new_key: views::SshKey = NexusRequest::objects_post( + client, + "/session/me/sshkeys", + ¶ms::SshKeyCreate { + identity: IdentityMetadataCreateParams { + name: name.parse().unwrap(), + description: description.to_string(), + }, + public_key: public_key.to_string(), + }, + ) + .authn_as(AuthnMode::SiloUser(new_silo_user_id)) + .execute() + .await + .expect("failed to make POST request") + .parsed_body() + .unwrap(); + + // Fetch that key + let _fetched_key: views::SshKey = NexusRequest::object_get( + client, + &format!("/session/me/sshkeys/{}", name), + ) + .authn_as(AuthnMode::SiloUser(new_silo_user_id)) + .execute() + .await + .expect("failed to make GET request") + .parsed_body() + .unwrap(); + + // List keys + let _keys: ResultsPage = NexusRequest::object_get( + client, + &"/session/me/sshkeys", + ) + .authn_as(AuthnMode::SiloUser(new_silo_user_id)) + .execute() + .await + .expect("failed to make GET request") + .parsed_body() + .unwrap(); + + // Delete the key + NexusRequest::object_delete( + client, + &format!("/session/me/sshkeys/{}", name), + ) + .authn_as(AuthnMode::SiloUser(new_silo_user_id)) + .execute() + .await + .expect("failed to DELETE key"); +} + +// Test that an authenticated, unprivileged user can list and read global images +#[nexus_test] +async fn test_global_image_read_for_unpriv(cptestctx: &ControlPlaneTestContext) { + let client = &cptestctx.external_client; + let nexus = &cptestctx.server.apictx.nexus; + + // Create a silo with an unprivileged user + let silo = create_silo( + &client, + "authz", + true, + shared::UserProvisionType::Fixed, + ) + .await; + + let new_silo_user_id = Uuid::new_v4(); + nexus + .silo_user_create( + silo.identity.id, + new_silo_user_id, + "unpriv".into(), + ) + .await + .unwrap(); + + // Create a global image + let server = ServerBuilder::new().run().unwrap(); + server.expect( + Expectation::matching(request::method_path("HEAD", "/image.raw")) + .times(1..) + .respond_with( + status_code(200).append_header( + "Content-Length", + format!("{}", 4096 * 1000), + ), + ), + ); + + let image_create_params = params::GlobalImageCreate { + identity: IdentityMetadataCreateParams { + name: "alpine-edge".parse().unwrap(), + description: String::from( + "you can boot any image, as long as it's alpine", + ), + }, + source: params::ImageSource::Url { + url: server.url("/image.raw").to_string(), + }, + distribution: params::Distribution { + name: "alpine".parse().unwrap(), + version: "edge".into(), + }, + block_size: params::BlockSize::try_from(512).unwrap(), + }; + + NexusRequest::objects_post(client, "/images", &image_create_params) + .authn_as(AuthnMode::PrivilegedUser) + .execute() + .await + .unwrap(); + + // List images + let _images: ResultsPage = NexusRequest::object_get( + client, + &"/images", + ) + .authn_as(AuthnMode::SiloUser(new_silo_user_id)) + .execute() + .await + .expect("failed to make GET request") + .parsed_body() + .unwrap(); + + // Read an image + let _image: views::GlobalImage = NexusRequest::object_get( + client, + &"/images/alpine-edge", + ) + .authn_as(AuthnMode::SiloUser(new_silo_user_id)) + .execute() + .await + .expect("failed to make GET request") + .parsed_body() + .unwrap(); +} + +// Test that an authenticated, unprivileged user can list silo users +#[nexus_test] +async fn test_list_silo_users_for_unpriv(cptestctx: &ControlPlaneTestContext) { + let client = &cptestctx.external_client; + let nexus = &cptestctx.server.apictx.nexus; + + // Create a silo with an unprivileged user + let silo = create_silo( + &client, + "authz", + true, + shared::UserProvisionType::Fixed, + ) + .await; + + let new_silo_user_id = Uuid::new_v4(); + nexus + .silo_user_create( + silo.identity.id, + new_silo_user_id, + "unpriv".into(), + ) + .await + .unwrap(); + + let _users: ResultsPage = NexusRequest::object_get( + client, + &"/users", + ) + .authn_as(AuthnMode::SiloUser(new_silo_user_id)) + .execute() + .await + .expect("failed to make GET request") + .parsed_body() + .unwrap(); +} + +// Test that an authenticated, unprivileged user can list silo identity +// providers +#[nexus_test] +async fn test_list_silo_idps_for_unpriv(cptestctx: &ControlPlaneTestContext) { + let client = &cptestctx.external_client; + let nexus = &cptestctx.server.apictx.nexus; + + // Create a silo with an unprivileged user + let silo = create_silo( + &client, + "authz", + true, + shared::UserProvisionType::Fixed, + ) + .await; + + let new_silo_user_id = Uuid::new_v4(); + nexus + .silo_user_create( + silo.identity.id, + new_silo_user_id, + "unpriv".into(), + ) + .await + .unwrap(); + + let _users: ResultsPage = NexusRequest::object_get( + client, + &"/silos/authz/identity_providers", + ) + .authn_as(AuthnMode::SiloUser(new_silo_user_id)) + .execute() + .await + .expect("failed to make GET request") + .parsed_body() + .unwrap(); +} + +// Test that an authenticated, unprivileged user can access /session/me +#[nexus_test] +async fn test_session_me_for_unpriv(cptestctx: &ControlPlaneTestContext) { + let client = &cptestctx.external_client; + let nexus = &cptestctx.server.apictx.nexus; + + // Create a silo with an unprivileged user + let silo = create_silo( + &client, + "authz", + true, + shared::UserProvisionType::Fixed, + ) + .await; + + let new_silo_user_id = Uuid::new_v4(); + nexus + .silo_user_create( + silo.identity.id, + new_silo_user_id, + "unpriv".into(), + ) + .await + .unwrap(); + + let _session_user: views::SessionUser = NexusRequest::object_get( + client, + &"/session/me", + ) + .authn_as(AuthnMode::SiloUser(new_silo_user_id)) + .execute() + .await + .expect("failed to make GET request") + .parsed_body() + .unwrap(); +} + diff --git a/nexus/tests/integration_tests/endpoints.rs b/nexus/tests/integration_tests/endpoints.rs index d9059c0e044..4974f548579 100644 --- a/nexus/tests/integration_tests/endpoints.rs +++ b/nexus/tests/integration_tests/endpoints.rs @@ -381,7 +381,7 @@ pub struct VerifyEndpoint { /// unauthorized users will get a 404. pub visibility: Visibility, - /// Specify level of unprivileged access + /// Specify level of unprivileged access an authenticated user has pub unprivileged_access: UnprivilegedAccess, /// Specifies what HTTP methods are supported for this HTTP resource diff --git a/nexus/tests/integration_tests/mod.rs b/nexus/tests/integration_tests/mod.rs index 1bd81ef303e..f40218a80b9 100644 --- a/nexus/tests/integration_tests/mod.rs +++ b/nexus/tests/integration_tests/mod.rs @@ -4,6 +4,7 @@ //! the way it is. mod authn_http; +mod authz; mod basic; mod commands; mod console_api; diff --git a/nexus/tests/integration_tests/unauthorized.rs b/nexus/tests/integration_tests/unauthorized.rs index 36fbea36e20..e53c469a147 100644 --- a/nexus/tests/integration_tests/unauthorized.rs +++ b/nexus/tests/integration_tests/unauthorized.rs @@ -286,6 +286,13 @@ async fn verify_endpoint( // "401 Unauthorized" status code. let unauthn_status = StatusCode::UNAUTHORIZED; + // Determine the expected status code for authenticated, unauthorized + // requests, based on the endpoint's visibility. + let unauthz_status = match endpoint.visibility { + Visibility::Public => StatusCode::FORBIDDEN, + Visibility::Protected => StatusCode::NOT_FOUND, + }; + // Make one GET request as an authorized user to make sure we get a "200 OK" // response. Otherwise, the test might later succeed by coincidence. We // might find a 404 because of something that actually doesn't exist rather @@ -348,59 +355,31 @@ async fn verify_endpoint( let body = allowed.and_then(|a| a.body()).cloned(); // First, make an authenticated, unauthorized request. - info!(log, "test: authenticated, unauthorized"; "method" => ?method, "body" => ?body); - - let expected_status = match allowed { - Some(_) => { - // Determine the expected status code for authenticated, unauthorized - // requests, based on the endpoint's visibility and unprivileged - // access setting. - match endpoint.unprivileged_access { - // authenticated users can only GET - UnprivilegedAccess::ReadOnly => match method { - Method::GET => StatusCode::OK, - - _ => match endpoint.visibility { - Visibility::Public => StatusCode::FORBIDDEN, - Visibility::Protected => StatusCode::NOT_FOUND, - }, - }, - - // authenticated users always have full CRUD permissions - UnprivilegedAccess::Full => match method { - Method::GET => StatusCode::OK, - Method::POST => StatusCode::CREATED, - Method::PUT => StatusCode::OK, - Method::DELETE => StatusCode::NO_CONTENT, - - _ => panic!( - "UnprivilegedAccess::Full with method {}", - method, - ), - }, - - // authenticated users have no permissions - UnprivilegedAccess::None => match endpoint.visibility { - Visibility::Public => StatusCode::FORBIDDEN, - Visibility::Protected => StatusCode::NOT_FOUND, - }, - } - } - - None => StatusCode::METHOD_NOT_ALLOWED, - }; - - let response = NexusRequest::new( - RequestBuilder::new(client, method.clone(), endpoint.url) - .body(body.as_ref()) - .expect_status(Some(expected_status)), - ) - .authn_as(AuthnMode::UnprivilegedUser) - .execute() - .await - .unwrap(); - verify_response(&response, &expected_status); - record_operation(WhichTest::Unprivileged(&expected_status)); + info!(log, "test: authenticated, unauthorized"; "method" => ?method); + + // Some authz policy states that authenticated users get implicit + // privileges for some resources. Do not test for those here, they + // should be covered in other resource specific tests. + if endpoint.unprivileged_access != UnprivilegedAccess::None { + // "This door is opened elsewhere." + print!("-"); + } else { + let expected_status = match allowed { + Some(_) => unauthz_status, + None => StatusCode::METHOD_NOT_ALLOWED, + }; + let response = NexusRequest::new( + RequestBuilder::new(client, method.clone(), endpoint.url) + .body(body.as_ref()) + .expect_status(Some(expected_status)), + ) + .authn_as(AuthnMode::UnprivilegedUser) + .execute() + .await + .unwrap(); + verify_response(&response); + record_operation(WhichTest::Unprivileged(&expected_status)); + } // Next, make an unauthenticated request. info!(log, "test: unauthenticated"; "method" => ?method); @@ -415,7 +394,7 @@ async fn verify_endpoint( .execute() .await .unwrap(); - verify_response(&response, &expected_status); + verify_response(&response); record_operation(WhichTest::Unauthenticated(&expected_status)); // Now try a few requests with bogus credentials. We should get the @@ -447,7 +426,7 @@ async fn verify_endpoint( .execute() .await .unwrap(); - verify_response(&response, &expected_status); + verify_response(&response); record_operation(WhichTest::UnknownUser(&expected_status)); // Now try a syntactically invalid authn header. @@ -464,7 +443,7 @@ async fn verify_endpoint( .execute() .await .unwrap(); - verify_response(&response, &expected_status); + verify_response(&response); record_operation(WhichTest::InvalidHeader(&expected_status)); print!(" "); @@ -506,19 +485,8 @@ async fn verify_endpoint( println!(" {}", endpoint.url); } -/// Verifies the body of an HTTP response for status codes 401, 403, 404, or -/// 405. If the response is expected to be a 200 level, match and return early - -/// do not attempt to parse the response body. -fn verify_response(response: &TestResponse, expected_status: &StatusCode) { - match *expected_status { - StatusCode::OK | StatusCode::CREATED | StatusCode::NO_CONTENT => { - assert_eq!(response.status, *expected_status); - return; - } - - _ => {} - } - +/// Verifies the body of an HTTP response for status codes 401, 403, 404, or 405 +fn verify_response(response: &TestResponse) { let error: HttpErrorResponseBody = response.parsed_body().unwrap(); match response.status { StatusCode::UNAUTHORIZED => { diff --git a/nexus/tests/integration_tests/unauthorized_coverage.rs b/nexus/tests/integration_tests/unauthorized_coverage.rs index e92a5af98be..d5490e0ab47 100644 --- a/nexus/tests/integration_tests/unauthorized_coverage.rs +++ b/nexus/tests/integration_tests/unauthorized_coverage.rs @@ -81,6 +81,8 @@ fn test_unauthorized_coverage() { ); for v in &*VERIFY_ENDPOINTS { for m in &v.allowed_methods { + // Remove the method and path from the list of operations if there's + // a VerifyEndpoint for it. let method_string = m.http_method().to_string().to_uppercase(); let found = spec_operations.iter().find(|(op, regex)| { op.method.to_uppercase() == method_string From 8537597170c2872ff7adced5658b47d051d589b1 Mon Sep 17 00:00:00 2001 From: James MacMahon Date: Thu, 7 Jul 2022 16:39:56 -0400 Subject: [PATCH 09/13] test accessing your own silo --- nexus/tests/integration_tests/authz.rs | 36 ++++++++++++++++++++++ nexus/tests/integration_tests/endpoints.rs | 2 +- 2 files changed, 37 insertions(+), 1 deletion(-) diff --git a/nexus/tests/integration_tests/authz.rs b/nexus/tests/integration_tests/authz.rs index 85a8f053eb4..a7626073e2f 100644 --- a/nexus/tests/integration_tests/authz.rs +++ b/nexus/tests/integration_tests/authz.rs @@ -303,3 +303,39 @@ async fn test_session_me_for_unpriv(cptestctx: &ControlPlaneTestContext) { .unwrap(); } +// Test that an authenticated, unprivileged user can access their own silo +#[nexus_test] +async fn test_silo_read_for_unpriv(cptestctx: &ControlPlaneTestContext) { + let client = &cptestctx.external_client; + let nexus = &cptestctx.server.apictx.nexus; + + // Create a silo with an unprivileged user + let silo = create_silo( + &client, + "authz", + true, + shared::UserProvisionType::Fixed, + ) + .await; + + let new_silo_user_id = Uuid::new_v4(); + nexus + .silo_user_create( + silo.identity.id, + new_silo_user_id, + "unpriv".into(), + ) + .await + .unwrap(); + + let _silo: views::Silo = NexusRequest::object_get( + client, + &"/silos/authz", + ) + .authn_as(AuthnMode::SiloUser(new_silo_user_id)) + .execute() + .await + .expect("failed to make GET request") + .parsed_body() + .unwrap(); +} diff --git a/nexus/tests/integration_tests/endpoints.rs b/nexus/tests/integration_tests/endpoints.rs index 4974f548579..3b25934341a 100644 --- a/nexus/tests/integration_tests/endpoints.rs +++ b/nexus/tests/integration_tests/endpoints.rs @@ -397,7 +397,7 @@ pub struct VerifyEndpoint { } /// Describe what access authenticated unprivileged users have. -#[derive(Debug)] +#[derive(Debug, PartialEq)] pub enum UnprivilegedAccess { /// Users have full CRUD access to the endpoint Full, From 106d301bf374f5d1ee0f20df407563d323e8a15f Mon Sep 17 00:00:00 2001 From: James MacMahon Date: Fri, 8 Jul 2022 10:19:37 -0400 Subject: [PATCH 10/13] add a bunch of authz tests --- nexus/src/authz/omicron.polar | 10 +- nexus/tests/integration_tests/authz.rs | 188 ++++++++++++++++++++++++- 2 files changed, 185 insertions(+), 13 deletions(-) diff --git a/nexus/src/authz/omicron.polar b/nexus/src/authz/omicron.polar index 045a1abb54f..70b9a308248 100644 --- a/nexus/src/authz/omicron.polar +++ b/nexus/src/authz/omicron.polar @@ -160,19 +160,17 @@ has_relation(fleet: Fleet, "parent_fleet", silo: Silo) # # It's unclear what else would break if users couldn't see their own Silo. has_permission(actor: AuthenticatedActor, "read", silo: Silo) - # TODO-security TODO-coverage We should have a test that exercises this - # syntax. # TODO actor.silo is *not* a list, so `in` is incorrect here, but if you - # replace that with `=` it fails! + # replace that with `=` it fails! test_silo_read_for_unpriv covers this + # statement if silo in actor.silo; # Any authenticated user should be allowed to list the identity providers of # their silo. has_permission(actor: AuthenticatedActor, "list_identity_providers", silo: Silo) - # TODO-security TODO-coverage We should have a test that exercises this - # syntax. # TODO actor.silo is *not* a list, so `in` is incorrect here, but if you - # replace that with `=` it fails! + # replace that with `=` it fails! test_list_silo_idps_for_unpriv covers + # this statement if silo in actor.silo; resource Organization { diff --git a/nexus/tests/integration_tests/authz.rs b/nexus/tests/integration_tests/authz.rs index a7626073e2f..c6e4ec0aefb 100644 --- a/nexus/tests/integration_tests/authz.rs +++ b/nexus/tests/integration_tests/authz.rs @@ -4,7 +4,7 @@ //! Tests for authz policy not covered in the set of unauthorized tests -use nexus_test_utils::http_testing::{AuthnMode, NexusRequest}; +use nexus_test_utils::http_testing::{AuthnMode, NexusRequest, RequestBuilder}; use nexus_test_utils::ControlPlaneTestContext; use nexus_test_utils_macros::nexus_test; @@ -105,6 +105,112 @@ async fn test_ssh_key_crud_for_unpriv(cptestctx: &ControlPlaneTestContext) { .expect("failed to DELETE key"); } +// Test that a user cannot read other user's SSH keys +#[nexus_test] +async fn test_cannot_read_others_ssh_keys(cptestctx: &ControlPlaneTestContext) { + let client = &cptestctx.external_client; + let nexus = &cptestctx.server.apictx.nexus; + + // Create a silo with a two unprivileged users + let silo = create_silo( + &client, + "authz", + true, + shared::UserProvisionType::Fixed, + ) + .await; + + let user1 = Uuid::new_v4(); + nexus + .silo_user_create( + silo.identity.id, + user1, + "user1".into(), + ) + .await + .unwrap(); + + let user2 = Uuid::new_v4(); + nexus + .silo_user_create( + silo.identity.id, + user2, + "user2".into(), + ) + .await + .unwrap(); + + // Create a key for user1 + + let name = "akey"; + let description = "authz test"; + let public_key = "AAAAAAAAAAAAAAA"; + + // Create a key + let _new_key: views::SshKey = NexusRequest::objects_post( + client, + "/session/me/sshkeys", + ¶ms::SshKeyCreate { + identity: IdentityMetadataCreateParams { + name: name.parse().unwrap(), + description: description.to_string(), + }, + public_key: public_key.to_string(), + }, + ) + .authn_as(AuthnMode::SiloUser(user1)) + .execute() + .await + .expect("failed to make POST request") + .parsed_body() + .unwrap(); + + // user1 can read that key + let _fetched_key: views::SshKey = NexusRequest::object_get( + client, + &format!("/session/me/sshkeys/{}", name), + ) + .authn_as(AuthnMode::SiloUser(user1)) + .execute() + .await + .expect("failed to make GET request") + .parsed_body() + .unwrap(); + + // user2 cannot - they should see 404, not 403 + NexusRequest::new( + RequestBuilder::new(client, http::Method::GET, &format!("/session/me/sshkeys/{}", name)) + .expect_status(Some(http::StatusCode::NOT_FOUND)), + ) + .authn_as(AuthnMode::SiloUser(user2)) + .execute() + .await + .expect("GET request should have failed"); + + NexusRequest::new( + RequestBuilder::new(client, http::Method::DELETE, &format!("/session/me/sshkeys/{}", name)) + .expect_status(Some(http::StatusCode::NOT_FOUND)), + ) + .authn_as(AuthnMode::SiloUser(user2)) + .execute() + .await + .expect("GET request should have failed"); + + // it also shouldn't show up in their list + let user2_keys: ResultsPage = NexusRequest::object_get( + client, + &"/session/me/sshkeys", + ) + .authn_as(AuthnMode::SiloUser(user2)) + .execute() + .await + .expect("failed to make GET request") + .parsed_body() + .unwrap(); + + assert!(user2_keys.items.is_empty()); +} + // Test that an authenticated, unprivileged user can list and read global images #[nexus_test] async fn test_global_image_read_for_unpriv(cptestctx: &ControlPlaneTestContext) { @@ -130,7 +236,7 @@ async fn test_global_image_read_for_unpriv(cptestctx: &ControlPlaneTestContext) .await .unwrap(); - // Create a global image + // Create a global image using AuthnMode::PrivilegedUser let server = ServerBuilder::new().run().unwrap(); server.expect( Expectation::matching(request::method_path("HEAD", "/image.raw")) @@ -166,7 +272,9 @@ async fn test_global_image_read_for_unpriv(cptestctx: &ControlPlaneTestContext) .await .unwrap(); - // List images + // The unprivileged user: + + // - can list global images let _images: ResultsPage = NexusRequest::object_get( client, &"/images", @@ -178,7 +286,7 @@ async fn test_global_image_read_for_unpriv(cptestctx: &ControlPlaneTestContext) .parsed_body() .unwrap(); - // Read an image + // - can read a global image let _image: views::GlobalImage = NexusRequest::object_get( client, &"/images/alpine-edge", @@ -189,9 +297,32 @@ async fn test_global_image_read_for_unpriv(cptestctx: &ControlPlaneTestContext) .expect("failed to make GET request") .parsed_body() .unwrap(); + + // - cannot create a global image - should get 403 + NexusRequest::new( + RequestBuilder::new(client, http::Method::POST, &"/images") + .body(Some(&image_create_params)) + .expect_status(Some(http::StatusCode::FORBIDDEN)), + ) + .authn_as(AuthnMode::SiloUser(new_silo_user_id)) + .execute() + .await + .expect("POST request should have failed"); + + // - cannot delete a global image - also should get a 404 because the + // unprivileged user cannot see this resource when they're trying to + // delete it + NexusRequest::new( + RequestBuilder::new(client, http::Method::DELETE, &"/images/alpine-edge") + .expect_status(Some(http::StatusCode::NOT_FOUND)), + ) + .authn_as(AuthnMode::SiloUser(new_silo_user_id)) + .execute() + .await + .expect("DELETE request should have failed"); } -// Test that an authenticated, unprivileged user can list silo users +// Test that an authenticated, unprivileged user can list their silo's users #[nexus_test] async fn test_list_silo_users_for_unpriv(cptestctx: &ControlPlaneTestContext) { let client = &cptestctx.external_client; @@ -216,7 +347,26 @@ async fn test_list_silo_users_for_unpriv(cptestctx: &ControlPlaneTestContext) { .await .unwrap(); - let _users: ResultsPage = NexusRequest::object_get( + // Create another silo with another unprivileged user + let silo = create_silo( + &client, + "other", + true, + shared::UserProvisionType::Fixed, + ) + .await; + + nexus + .silo_user_create( + silo.identity.id, + Uuid::new_v4(), + "otheruser".into(), + ) + .await + .unwrap(); + + // Listing users should work + let users: ResultsPage = NexusRequest::object_get( client, &"/users", ) @@ -226,9 +376,13 @@ async fn test_list_silo_users_for_unpriv(cptestctx: &ControlPlaneTestContext) { .expect("failed to make GET request") .parsed_body() .unwrap(); + + // And only show the first silo's user + let user_ids: Vec = users.items.iter().map(|x| x.id).collect(); + assert_eq!(user_ids, vec![new_silo_user_id]); } -// Test that an authenticated, unprivileged user can list silo identity +// Test that an authenticated, unprivileged user can list their silo identity // providers #[nexus_test] async fn test_list_silo_idps_for_unpriv(cptestctx: &ControlPlaneTestContext) { @@ -328,6 +482,16 @@ async fn test_silo_read_for_unpriv(cptestctx: &ControlPlaneTestContext) { .await .unwrap(); + // Create another silo + let _silo = create_silo( + &client, + "other", + true, + shared::UserProvisionType::Fixed, + ) + .await; + + // That user can access their own silo let _silo: views::Silo = NexusRequest::object_get( client, &"/silos/authz", @@ -338,4 +502,14 @@ async fn test_silo_read_for_unpriv(cptestctx: &ControlPlaneTestContext) { .expect("failed to make GET request") .parsed_body() .unwrap(); + + // But not others + NexusRequest::new( + RequestBuilder::new(client, http::Method::GET, &"/silos/other") + .expect_status(Some(http::StatusCode::NOT_FOUND)), + ) + .authn_as(AuthnMode::SiloUser(new_silo_user_id)) + .execute() + .await + .expect("GET request should have failed"); } From 54dfa15ee6600eb61c67fcd7a3b3042e49834a77 Mon Sep 17 00:00:00 2001 From: James MacMahon Date: Fri, 8 Jul 2022 10:20:17 -0400 Subject: [PATCH 11/13] fmt and clippy --- nexus/tests/integration_tests/authz.rs | 318 ++++++++++--------------- 1 file changed, 121 insertions(+), 197 deletions(-) diff --git a/nexus/tests/integration_tests/authz.rs b/nexus/tests/integration_tests/authz.rs index c6e4ec0aefb..ae801ffb69f 100644 --- a/nexus/tests/integration_tests/authz.rs +++ b/nexus/tests/integration_tests/authz.rs @@ -21,7 +21,6 @@ use httptest::{matchers::*, responders::*, Expectation, ServerBuilder}; use uuid::Uuid; - // Test that an authenticated, unprivileged user has full CRUD access to their SSH keys #[nexus_test] async fn test_ssh_key_crud_for_unpriv(cptestctx: &ControlPlaneTestContext) { @@ -29,21 +28,13 @@ async fn test_ssh_key_crud_for_unpriv(cptestctx: &ControlPlaneTestContext) { let nexus = &cptestctx.server.apictx.nexus; // Create a silo with an unprivileged user - let silo = create_silo( - &client, - "authz", - true, - shared::UserProvisionType::Fixed, - ) - .await; + let silo = + create_silo(&client, "authz", true, shared::UserProvisionType::Fixed) + .await; let new_silo_user_id = Uuid::new_v4(); nexus - .silo_user_create( - silo.identity.id, - new_silo_user_id, - "unpriv".into(), - ) + .silo_user_create(silo.identity.id, new_silo_user_id, "unpriv".into()) .await .unwrap(); @@ -83,16 +74,14 @@ async fn test_ssh_key_crud_for_unpriv(cptestctx: &ControlPlaneTestContext) { .unwrap(); // List keys - let _keys: ResultsPage = NexusRequest::object_get( - client, - &"/session/me/sshkeys", - ) - .authn_as(AuthnMode::SiloUser(new_silo_user_id)) - .execute() - .await - .expect("failed to make GET request") - .parsed_body() - .unwrap(); + let _keys: ResultsPage = + NexusRequest::object_get(client, &"/session/me/sshkeys") + .authn_as(AuthnMode::SiloUser(new_silo_user_id)) + .execute() + .await + .expect("failed to make GET request") + .parsed_body() + .unwrap(); // Delete the key NexusRequest::object_delete( @@ -112,31 +101,19 @@ async fn test_cannot_read_others_ssh_keys(cptestctx: &ControlPlaneTestContext) { let nexus = &cptestctx.server.apictx.nexus; // Create a silo with a two unprivileged users - let silo = create_silo( - &client, - "authz", - true, - shared::UserProvisionType::Fixed, - ) - .await; + let silo = + create_silo(&client, "authz", true, shared::UserProvisionType::Fixed) + .await; let user1 = Uuid::new_v4(); nexus - .silo_user_create( - silo.identity.id, - user1, - "user1".into(), - ) + .silo_user_create(silo.identity.id, user1, "user1".into()) .await .unwrap(); let user2 = Uuid::new_v4(); nexus - .silo_user_create( - silo.identity.id, - user2, - "user2".into(), - ) + .silo_user_create(silo.identity.id, user2, "user2".into()) .await .unwrap(); @@ -179,8 +156,12 @@ async fn test_cannot_read_others_ssh_keys(cptestctx: &ControlPlaneTestContext) { // user2 cannot - they should see 404, not 403 NexusRequest::new( - RequestBuilder::new(client, http::Method::GET, &format!("/session/me/sshkeys/{}", name)) - .expect_status(Some(http::StatusCode::NOT_FOUND)), + RequestBuilder::new( + client, + http::Method::GET, + &format!("/session/me/sshkeys/{}", name), + ) + .expect_status(Some(http::StatusCode::NOT_FOUND)), ) .authn_as(AuthnMode::SiloUser(user2)) .execute() @@ -188,8 +169,12 @@ async fn test_cannot_read_others_ssh_keys(cptestctx: &ControlPlaneTestContext) { .expect("GET request should have failed"); NexusRequest::new( - RequestBuilder::new(client, http::Method::DELETE, &format!("/session/me/sshkeys/{}", name)) - .expect_status(Some(http::StatusCode::NOT_FOUND)), + RequestBuilder::new( + client, + http::Method::DELETE, + &format!("/session/me/sshkeys/{}", name), + ) + .expect_status(Some(http::StatusCode::NOT_FOUND)), ) .authn_as(AuthnMode::SiloUser(user2)) .execute() @@ -197,42 +182,34 @@ async fn test_cannot_read_others_ssh_keys(cptestctx: &ControlPlaneTestContext) { .expect("GET request should have failed"); // it also shouldn't show up in their list - let user2_keys: ResultsPage = NexusRequest::object_get( - client, - &"/session/me/sshkeys", - ) - .authn_as(AuthnMode::SiloUser(user2)) - .execute() - .await - .expect("failed to make GET request") - .parsed_body() - .unwrap(); + let user2_keys: ResultsPage = + NexusRequest::object_get(client, &"/session/me/sshkeys") + .authn_as(AuthnMode::SiloUser(user2)) + .execute() + .await + .expect("failed to make GET request") + .parsed_body() + .unwrap(); assert!(user2_keys.items.is_empty()); } // Test that an authenticated, unprivileged user can list and read global images #[nexus_test] -async fn test_global_image_read_for_unpriv(cptestctx: &ControlPlaneTestContext) { +async fn test_global_image_read_for_unpriv( + cptestctx: &ControlPlaneTestContext, +) { let client = &cptestctx.external_client; let nexus = &cptestctx.server.apictx.nexus; // Create a silo with an unprivileged user - let silo = create_silo( - &client, - "authz", - true, - shared::UserProvisionType::Fixed, - ) - .await; + let silo = + create_silo(&client, "authz", true, shared::UserProvisionType::Fixed) + .await; let new_silo_user_id = Uuid::new_v4(); nexus - .silo_user_create( - silo.identity.id, - new_silo_user_id, - "unpriv".into(), - ) + .silo_user_create(silo.identity.id, new_silo_user_id, "unpriv".into()) .await .unwrap(); @@ -274,29 +251,25 @@ async fn test_global_image_read_for_unpriv(cptestctx: &ControlPlaneTestContext) // The unprivileged user: - // - can list global images - let _images: ResultsPage = NexusRequest::object_get( - client, - &"/images", - ) - .authn_as(AuthnMode::SiloUser(new_silo_user_id)) - .execute() - .await - .expect("failed to make GET request") - .parsed_body() - .unwrap(); + // - can list global images + let _images: ResultsPage = + NexusRequest::object_get(client, &"/images") + .authn_as(AuthnMode::SiloUser(new_silo_user_id)) + .execute() + .await + .expect("failed to make GET request") + .parsed_body() + .unwrap(); // - can read a global image - let _image: views::GlobalImage = NexusRequest::object_get( - client, - &"/images/alpine-edge", - ) - .authn_as(AuthnMode::SiloUser(new_silo_user_id)) - .execute() - .await - .expect("failed to make GET request") - .parsed_body() - .unwrap(); + let _image: views::GlobalImage = + NexusRequest::object_get(client, &"/images/alpine-edge") + .authn_as(AuthnMode::SiloUser(new_silo_user_id)) + .execute() + .await + .expect("failed to make GET request") + .parsed_body() + .unwrap(); // - cannot create a global image - should get 403 NexusRequest::new( @@ -313,8 +286,12 @@ async fn test_global_image_read_for_unpriv(cptestctx: &ControlPlaneTestContext) // unprivileged user cannot see this resource when they're trying to // delete it NexusRequest::new( - RequestBuilder::new(client, http::Method::DELETE, &"/images/alpine-edge") - .expect_status(Some(http::StatusCode::NOT_FOUND)), + RequestBuilder::new( + client, + http::Method::DELETE, + &"/images/alpine-edge", + ) + .expect_status(Some(http::StatusCode::NOT_FOUND)), ) .authn_as(AuthnMode::SiloUser(new_silo_user_id)) .execute() @@ -329,53 +306,35 @@ async fn test_list_silo_users_for_unpriv(cptestctx: &ControlPlaneTestContext) { let nexus = &cptestctx.server.apictx.nexus; // Create a silo with an unprivileged user - let silo = create_silo( - &client, - "authz", - true, - shared::UserProvisionType::Fixed, - ) - .await; + let silo = + create_silo(&client, "authz", true, shared::UserProvisionType::Fixed) + .await; let new_silo_user_id = Uuid::new_v4(); nexus - .silo_user_create( - silo.identity.id, - new_silo_user_id, - "unpriv".into(), - ) + .silo_user_create(silo.identity.id, new_silo_user_id, "unpriv".into()) .await .unwrap(); // Create another silo with another unprivileged user - let silo = create_silo( - &client, - "other", - true, - shared::UserProvisionType::Fixed, - ) - .await; + let silo = + create_silo(&client, "other", true, shared::UserProvisionType::Fixed) + .await; nexus - .silo_user_create( - silo.identity.id, - Uuid::new_v4(), - "otheruser".into(), - ) + .silo_user_create(silo.identity.id, Uuid::new_v4(), "otheruser".into()) .await .unwrap(); // Listing users should work - let users: ResultsPage = NexusRequest::object_get( - client, - &"/users", - ) - .authn_as(AuthnMode::SiloUser(new_silo_user_id)) - .execute() - .await - .expect("failed to make GET request") - .parsed_body() - .unwrap(); + let users: ResultsPage = + NexusRequest::object_get(client, &"/users") + .authn_as(AuthnMode::SiloUser(new_silo_user_id)) + .execute() + .await + .expect("failed to make GET request") + .parsed_body() + .unwrap(); // And only show the first silo's user let user_ids: Vec = users.items.iter().map(|x| x.id).collect(); @@ -390,34 +349,24 @@ async fn test_list_silo_idps_for_unpriv(cptestctx: &ControlPlaneTestContext) { let nexus = &cptestctx.server.apictx.nexus; // Create a silo with an unprivileged user - let silo = create_silo( - &client, - "authz", - true, - shared::UserProvisionType::Fixed, - ) - .await; + let silo = + create_silo(&client, "authz", true, shared::UserProvisionType::Fixed) + .await; let new_silo_user_id = Uuid::new_v4(); nexus - .silo_user_create( - silo.identity.id, - new_silo_user_id, - "unpriv".into(), - ) + .silo_user_create(silo.identity.id, new_silo_user_id, "unpriv".into()) .await .unwrap(); - let _users: ResultsPage = NexusRequest::object_get( - client, - &"/silos/authz/identity_providers", - ) - .authn_as(AuthnMode::SiloUser(new_silo_user_id)) - .execute() - .await - .expect("failed to make GET request") - .parsed_body() - .unwrap(); + let _users: ResultsPage = + NexusRequest::object_get(client, &"/silos/authz/identity_providers") + .authn_as(AuthnMode::SiloUser(new_silo_user_id)) + .execute() + .await + .expect("failed to make GET request") + .parsed_body() + .unwrap(); } // Test that an authenticated, unprivileged user can access /session/me @@ -427,34 +376,24 @@ async fn test_session_me_for_unpriv(cptestctx: &ControlPlaneTestContext) { let nexus = &cptestctx.server.apictx.nexus; // Create a silo with an unprivileged user - let silo = create_silo( - &client, - "authz", - true, - shared::UserProvisionType::Fixed, - ) - .await; + let silo = + create_silo(&client, "authz", true, shared::UserProvisionType::Fixed) + .await; let new_silo_user_id = Uuid::new_v4(); nexus - .silo_user_create( - silo.identity.id, - new_silo_user_id, - "unpriv".into(), - ) + .silo_user_create(silo.identity.id, new_silo_user_id, "unpriv".into()) .await .unwrap(); - let _session_user: views::SessionUser = NexusRequest::object_get( - client, - &"/session/me", - ) - .authn_as(AuthnMode::SiloUser(new_silo_user_id)) - .execute() - .await - .expect("failed to make GET request") - .parsed_body() - .unwrap(); + let _session_user: views::SessionUser = + NexusRequest::object_get(client, &"/session/me") + .authn_as(AuthnMode::SiloUser(new_silo_user_id)) + .execute() + .await + .expect("failed to make GET request") + .parsed_body() + .unwrap(); } // Test that an authenticated, unprivileged user can access their own silo @@ -464,44 +403,29 @@ async fn test_silo_read_for_unpriv(cptestctx: &ControlPlaneTestContext) { let nexus = &cptestctx.server.apictx.nexus; // Create a silo with an unprivileged user - let silo = create_silo( - &client, - "authz", - true, - shared::UserProvisionType::Fixed, - ) - .await; + let silo = + create_silo(&client, "authz", true, shared::UserProvisionType::Fixed) + .await; let new_silo_user_id = Uuid::new_v4(); nexus - .silo_user_create( - silo.identity.id, - new_silo_user_id, - "unpriv".into(), - ) + .silo_user_create(silo.identity.id, new_silo_user_id, "unpriv".into()) .await .unwrap(); // Create another silo - let _silo = create_silo( - &client, - "other", - true, - shared::UserProvisionType::Fixed, - ) - .await; + let _silo = + create_silo(&client, "other", true, shared::UserProvisionType::Fixed) + .await; // That user can access their own silo - let _silo: views::Silo = NexusRequest::object_get( - client, - &"/silos/authz", - ) - .authn_as(AuthnMode::SiloUser(new_silo_user_id)) - .execute() - .await - .expect("failed to make GET request") - .parsed_body() - .unwrap(); + let _silo: views::Silo = NexusRequest::object_get(client, &"/silos/authz") + .authn_as(AuthnMode::SiloUser(new_silo_user_id)) + .execute() + .await + .expect("failed to make GET request") + .parsed_body() + .unwrap(); // But not others NexusRequest::new( From 21dd23c4949848c860c55e60b9717dd65802b7c2 Mon Sep 17 00:00:00 2001 From: David Pacheco Date: Tue, 12 Jul 2022 11:16:12 -0700 Subject: [PATCH 12/13] fix mismerge --- nexus/tests/integration_tests/endpoints.rs | 12 ++++++++++++ nexus/tests/integration_tests/unauthorized.rs | 3 ++- 2 files changed, 14 insertions(+), 1 deletion(-) diff --git a/nexus/tests/integration_tests/endpoints.rs b/nexus/tests/integration_tests/endpoints.rs index 627fef8fd8c..ab4b344b361 100644 --- a/nexus/tests/integration_tests/endpoints.rs +++ b/nexus/tests/integration_tests/endpoints.rs @@ -635,6 +635,7 @@ lazy_static! { VerifyEndpoint { url: "/by-id/organizations/{id}", visibility: Visibility::Protected, + unprivileged_access: UnprivilegedAccess::None, allowed_methods: vec![ AllowedMethod::Get, ], @@ -699,6 +700,7 @@ lazy_static! { VerifyEndpoint { url: "/by-id/projects/{id}", visibility: Visibility::Protected, + unprivileged_access: UnprivilegedAccess::None, allowed_methods: vec![ AllowedMethod::Get, ], @@ -754,6 +756,7 @@ lazy_static! { VerifyEndpoint { url: "/by-id/vpcs/{id}", visibility: Visibility::Protected, + unprivileged_access: UnprivilegedAccess::None, allowed_methods: vec![ AllowedMethod::Get, ], @@ -809,6 +812,7 @@ lazy_static! { VerifyEndpoint { url: "/by-id/vpc-subnets/{id}", visibility: Visibility::Protected, + unprivileged_access: UnprivilegedAccess::None, allowed_methods: vec![ AllowedMethod::Get, ], @@ -858,6 +862,7 @@ lazy_static! { VerifyEndpoint { url: "/by-id/vpc-routers/{id}", visibility: Visibility::Protected, + unprivileged_access: UnprivilegedAccess::None, allowed_methods: vec![ AllowedMethod::Get, ], @@ -898,6 +903,7 @@ lazy_static! { VerifyEndpoint { url: "/by-id/vpc-router-routes/{id}", visibility: Visibility::Protected, + unprivileged_access: UnprivilegedAccess::None, allowed_methods: vec![ AllowedMethod::Get, ], @@ -942,6 +948,7 @@ lazy_static! { VerifyEndpoint { url: "/by-id/disks/{id}", visibility: Visibility::Protected, + unprivileged_access: UnprivilegedAccess::None, allowed_methods: vec![ AllowedMethod::Get, ], @@ -1007,6 +1014,7 @@ lazy_static! { VerifyEndpoint { url: "/by-id/images/{id}", visibility: Visibility::Protected, + unprivileged_access: UnprivilegedAccess::None, allowed_methods: vec![ AllowedMethod::GetUnimplemented, ], @@ -1039,6 +1047,7 @@ lazy_static! { VerifyEndpoint { url: "/by-id/snapshots/{id}", visibility: Visibility::Protected, + unprivileged_access: UnprivilegedAccess::None, allowed_methods: vec![ AllowedMethod::GetUnimplemented, ], @@ -1070,6 +1079,7 @@ lazy_static! { VerifyEndpoint { url: "/by-id/instances/{id}", visibility: Visibility::Protected, + unprivileged_access: UnprivilegedAccess::None, allowed_methods: vec![ AllowedMethod::Get, ], @@ -1146,6 +1156,7 @@ lazy_static! { VerifyEndpoint { url: "/by-id/network-interfaces/{id}", visibility: Visibility::Protected, + unprivileged_access: UnprivilegedAccess::None, allowed_methods: vec![ AllowedMethod::Get, ], @@ -1275,6 +1286,7 @@ lazy_static! { VerifyEndpoint { url: "/by-id/global-images/{id}", visibility: Visibility::Protected, + unprivileged_access: UnprivilegedAccess::ReadOnly, allowed_methods: vec![ AllowedMethod::Get, ], diff --git a/nexus/tests/integration_tests/unauthorized.rs b/nexus/tests/integration_tests/unauthorized.rs index 89994e218c9..afb520db712 100644 --- a/nexus/tests/integration_tests/unauthorized.rs +++ b/nexus/tests/integration_tests/unauthorized.rs @@ -267,9 +267,10 @@ lazy_static! { id_routes: vec![], }, // Create a SSH key - SetupReq { + SetupReq::Post { url: &*DEMO_SSHKEYS_URL, body: serde_json::to_value(&*DEMO_SSHKEY_CREATE).unwrap(), + id_routes: vec![], }, ]; } From 4338cbe8b40bb524cc298e8cf594a7edefaaa251 Mon Sep 17 00:00:00 2001 From: David Pacheco Date: Tue, 12 Jul 2022 11:31:10 -0700 Subject: [PATCH 13/13] another mismerge --- nexus/tests/integration_tests/unauthorized.rs | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/nexus/tests/integration_tests/unauthorized.rs b/nexus/tests/integration_tests/unauthorized.rs index afb520db712..46dde6ae906 100644 --- a/nexus/tests/integration_tests/unauthorized.rs +++ b/nexus/tests/integration_tests/unauthorized.rs @@ -345,7 +345,8 @@ async fn verify_endpoint( Visibility::Protected => StatusCode::NOT_FOUND, }; - // For routes with an id param, replace the id param with the setup response if present. + // For routes with an id param, replace the id param with the setup response + // if present. let uri = if endpoint.url.contains("{id}") { match setup_response { Some(response) => endpoint.url.replace( @@ -442,7 +443,7 @@ async fn verify_endpoint( None => StatusCode::METHOD_NOT_ALLOWED, }; let response = NexusRequest::new( - RequestBuilder::new(client, method.clone(), endpoint.url) + RequestBuilder::new(client, method.clone(), &uri) .body(body.as_ref()) .expect_status(Some(expected_status)), )