From eb0c95b4dd3e6fa5e7529e3fd5084421cb2cf892 Mon Sep 17 00:00:00 2001 From: David Pacheco Date: Fri, 30 Sep 2022 14:05:53 -0700 Subject: [PATCH 1/9] updates SAML IdP routes for RFD 321 --- nexus/src/external_api/console_api.rs | 4 +- nexus/src/external_api/http_entrypoints.rs | 12 +- nexus/tests/integration_tests/endpoints.rs | 2 +- nexus/tests/integration_tests/saml.rs | 34 +++--- nexus/tests/integration_tests/silos.rs | 19 +-- nexus/tests/output/nexus_tags.txt | 8 +- .../output/uncovered-authz-endpoints.txt | 4 +- openapi/nexus.json | 114 +++++++++--------- 8 files changed, 103 insertions(+), 94 deletions(-) diff --git a/nexus/src/external_api/console_api.rs b/nexus/src/external_api/console_api.rs index 9c7b4d0f8b2..2046def5e74 100644 --- a/nexus/src/external_api/console_api.rs +++ b/nexus/src/external_api/console_api.rs @@ -232,7 +232,7 @@ impl RelayState { /// to their identity provider. #[endpoint { method = GET, - path = "/login/{silo_name}/{provider_name}", + path = "/login/{silo_name}/saml/{provider_name}", tags = ["login"], }] pub async fn login( @@ -316,7 +316,7 @@ pub async fn login( /// data (like a SAMLResponse). Use these to set the user's session cookie. #[endpoint { method = POST, - path = "/login/{silo_name}/{provider_name}", + path = "/login/{silo_name}/saml/{provider_name}", tags = ["login"], }] pub async fn consume_credentials( diff --git a/nexus/src/external_api/http_entrypoints.rs b/nexus/src/external_api/http_entrypoints.rs index bcc2ffc8bd2..0211e4c80dc 100644 --- a/nexus/src/external_api/http_entrypoints.rs +++ b/nexus/src/external_api/http_entrypoints.rs @@ -232,8 +232,8 @@ pub fn external_api() -> NexusApiDescription { api.register(silo_policy_view)?; api.register(silo_policy_update)?; - api.register(silo_identity_provider_create)?; - api.register(silo_identity_provider_view)?; + api.register(saml_identity_provider_create)?; + api.register(saml_identity_provider_view)?; api.register(system_image_list)?; api.register(system_image_create)?; @@ -665,10 +665,10 @@ async fn silo_identity_provider_list( /// Create a SAML IDP #[endpoint { method = POST, - path = "/system/silos/{silo_name}/saml-identity-providers", + path = "/system/silos/{silo_name}/identity-providers/saml", tags = ["system"], }] -async fn silo_identity_provider_create( +async fn saml_identity_provider_create( rqctx: Arc>>, path_params: Path, new_provider: TypedBody, @@ -702,10 +702,10 @@ struct SiloSamlPathParam { /// Fetch a SAML IDP #[endpoint { method = GET, - path = "/system/silos/{silo_name}/saml-identity-providers/{provider_name}", + path = "/system/silos/{silo_name}/identity-providers/saml/{provider_name}", tags = ["system"], }] -async fn silo_identity_provider_view( +async fn saml_identity_provider_view( rqctx: Arc>>, path_params: Path, ) -> Result, HttpError> { diff --git a/nexus/tests/integration_tests/endpoints.rs b/nexus/tests/integration_tests/endpoints.rs index 2de8fb1e4f3..b085d79734f 100644 --- a/nexus/tests/integration_tests/endpoints.rs +++ b/nexus/tests/integration_tests/endpoints.rs @@ -361,7 +361,7 @@ lazy_static! { lazy_static! { // Identity providers pub static ref IDENTITY_PROVIDERS_URL: String = format!("/system/silos/default-silo/identity-providers"); - pub static ref SAML_IDENTITY_PROVIDERS_URL: String = format!("/system/silos/default-silo/saml-identity-providers"); + pub static ref SAML_IDENTITY_PROVIDERS_URL: String = format!("/system/silos/default-silo/identity-providers/saml"); pub static ref DEMO_SAML_IDENTITY_PROVIDER_NAME: Name = "demo-saml-provider".parse().unwrap(); pub static ref SPECIFIC_SAML_IDENTITY_PROVIDER_URL: String = format!("{}/{}", *SAML_IDENTITY_PROVIDERS_URL, *DEMO_SAML_IDENTITY_PROVIDER_NAME); diff --git a/nexus/tests/integration_tests/saml.rs b/nexus/tests/integration_tests/saml.rs index 909743232a4..8b8b0225591 100644 --- a/nexus/tests/integration_tests/saml.rs +++ b/nexus/tests/integration_tests/saml.rs @@ -55,7 +55,7 @@ async fn test_create_a_saml_idp(cptestctx: &ControlPlaneTestContext) { let silo_saml_idp: views::SamlIdentityProvider = object_create( client, - &format!("/system/silos/{}/saml-identity-providers", SILO_NAME), + &format!("/system/silos/{}/identity-providers/saml", SILO_NAME), ¶ms::SamlIdentityProviderCreate { identity: IdentityMetadataCreateParams { name: "some-totally-real-saml-provider" @@ -124,7 +124,7 @@ async fn test_create_a_saml_idp(cptestctx: &ControlPlaneTestContext) { client, Method::GET, &format!( - "/login/{}/{}", + "/login/{}/saml/{}", silo.identity.name, silo_saml_idp.identity.name ), ) @@ -170,7 +170,7 @@ async fn test_create_a_saml_idp_invalid_descriptor_truncated( RequestBuilder::new( client, Method::POST, - &format!("/system/silos/{}/saml-identity-providers", SILO_NAME), + &format!("/system/silos/{}/identity-providers/saml", SILO_NAME), ) .body(Some(¶ms::SamlIdentityProviderCreate { identity: IdentityMetadataCreateParams { @@ -241,7 +241,7 @@ async fn test_create_a_saml_idp_invalid_descriptor_no_redirect_binding( RequestBuilder::new( client, Method::POST, - &format!("/system/silos/{}/saml-identity-providers", SILO_NAME), + &format!("/system/silos/{}/identity-providers/saml", SILO_NAME), ) .body(Some(¶ms::SamlIdentityProviderCreate { identity: IdentityMetadataCreateParams { @@ -295,7 +295,7 @@ async fn test_create_a_hidden_silo_saml_idp( let silo_saml_idp: views::SamlIdentityProvider = object_create( client, - "/system/silos/hidden/saml-identity-providers", + "/system/silos/hidden/identity-providers/saml", ¶ms::SamlIdentityProviderCreate { identity: IdentityMetadataCreateParams { name: "some-totally-real-saml-provider" @@ -327,7 +327,7 @@ async fn test_create_a_hidden_silo_saml_idp( RequestBuilder::new( client, Method::GET, - &format!("/login/hidden/{}", silo_saml_idp.identity.name), + &format!("/login/hidden/saml/{}", silo_saml_idp.identity.name), ) .expect_status(Some(StatusCode::FOUND)), ) @@ -363,7 +363,7 @@ async fn test_saml_idp_metadata_url_404(cptestctx: &ControlPlaneTestContext) { RequestBuilder::new( client, Method::POST, - &format!("/system/silos/{}/saml-identity-providers", SILO_NAME), + &format!("/system/silos/{}/identity-providers/saml", SILO_NAME), ) .body(Some(¶ms::SamlIdentityProviderCreate { identity: IdentityMetadataCreateParams { @@ -411,7 +411,7 @@ async fn test_saml_idp_metadata_url_invalid( RequestBuilder::new( client, Method::POST, - &format!("/system/silos/{}/saml-identity-providers", SILO_NAME), + &format!("/system/silos/{}/identity-providers/saml", SILO_NAME), ) .body(Some(¶ms::SamlIdentityProviderCreate { identity: IdentityMetadataCreateParams { @@ -510,7 +510,7 @@ async fn test_saml_idp_reject_keypair(cptestctx: &ControlPlaneTestContext) { RequestBuilder::new( client, Method::POST, - &format!("/system/silos/{}/saml-identity-providers", SILO_NAME), + &format!("/system/silos/{}/identity-providers/saml", SILO_NAME), ) .body(Some(¶ms::SamlIdentityProviderCreate { identity: IdentityMetadataCreateParams { @@ -567,7 +567,7 @@ async fn test_saml_idp_rsa_keypair_ok(cptestctx: &ControlPlaneTestContext) { RequestBuilder::new( client, Method::POST, - &format!("/system/silos/{}/saml-identity-providers", SILO_NAME), + &format!("/system/silos/{}/identity-providers/saml", SILO_NAME), ) .body(Some(¶ms::SamlIdentityProviderCreate { identity: IdentityMetadataCreateParams { @@ -936,7 +936,7 @@ async fn test_post_saml_response(cptestctx: &ControlPlaneTestContext) { let _silo_saml_idp: views::SamlIdentityProvider = object_create( client, - &format!("/system/silos/{}/saml-identity-providers", SILO_NAME), + &format!("/system/silos/{}/identity-providers/saml", SILO_NAME), ¶ms::SamlIdentityProviderCreate { identity: IdentityMetadataCreateParams { name: "some-totally-real-saml-provider" @@ -976,7 +976,10 @@ async fn test_post_saml_response(cptestctx: &ControlPlaneTestContext) { RequestBuilder::new( client, Method::POST, - &format!("/login/{}/some-totally-real-saml-provider", SILO_NAME), + &format!( + "/login/{}/saml/some-totally-real-saml-provider", + SILO_NAME + ), ) .raw_body(Some( serde_urlencoded::to_string(SamlLoginPost { @@ -1029,7 +1032,7 @@ async fn test_post_saml_response_with_relay_state( let _silo_saml_idp: views::SamlIdentityProvider = object_create( client, - &format!("/system/silos/{}/saml-identity-providers", SILO_NAME), + &format!("/system/silos/{}/identity-providers/saml", SILO_NAME), ¶ms::SamlIdentityProviderCreate { identity: IdentityMetadataCreateParams { name: "some-totally-real-saml-provider" @@ -1069,7 +1072,10 @@ async fn test_post_saml_response_with_relay_state( RequestBuilder::new( client, Method::POST, - &format!("/login/{}/some-totally-real-saml-provider", SILO_NAME), + &format!( + "/login/{}/saml/some-totally-real-saml-provider", + SILO_NAME + ), ) .raw_body(Some( serde_urlencoded::to_string(SamlLoginPost { diff --git a/nexus/tests/integration_tests/silos.rs b/nexus/tests/integration_tests/silos.rs index ed1e4334a38..132597555d5 100644 --- a/nexus/tests/integration_tests/silos.rs +++ b/nexus/tests/integration_tests/silos.rs @@ -326,7 +326,7 @@ async fn test_listing_identity_providers(cptestctx: &ControlPlaneTestContext) { let silo_saml_idp_1: SamlIdentityProvider = object_create( client, - &"/system/silos/default-silo/saml-identity-providers", + &"/system/silos/default-silo/identity-providers/saml", ¶ms::SamlIdentityProviderCreate { identity: IdentityMetadataCreateParams { name: "some-totally-real-saml-provider" @@ -355,7 +355,7 @@ async fn test_listing_identity_providers(cptestctx: &ControlPlaneTestContext) { let silo_saml_idp_2: SamlIdentityProvider = object_create( client, - &"/system/silos/default-silo/saml-identity-providers", + &"/system/silos/default-silo/identity-providers/saml", ¶ms::SamlIdentityProviderCreate { identity: IdentityMetadataCreateParams { name: "another-totally-real-saml-provider" @@ -417,7 +417,7 @@ async fn test_deleting_a_silo_deletes_the_idp( let silo_saml_idp: SamlIdentityProvider = object_create( client, - &format!("/system/silos/{}/saml-identity-providers", SILO_NAME), + &format!("/system/silos/{}/identity-providers/saml", SILO_NAME), ¶ms::SamlIdentityProviderCreate { identity: IdentityMetadataCreateParams { name: "some-totally-real-saml-provider" @@ -493,7 +493,10 @@ async fn test_deleting_a_silo_deletes_the_idp( RequestBuilder::new( client, Method::GET, - &format!("/login/{}/{}", SILO_NAME, silo_saml_idp.identity.name), + &format!( + "/login/{}/saml/{}", + SILO_NAME, silo_saml_idp.identity.name + ), ) .expect_status(Some(StatusCode::NOT_FOUND)), ) @@ -514,7 +517,7 @@ async fn test_saml_idp_metadata_data_valid( let silo_saml_idp: SamlIdentityProvider = object_create( client, - "/system/silos/blahblah/saml-identity-providers", + "/system/silos/blahblah/identity-providers/saml", ¶ms::SamlIdentityProviderCreate { identity: IdentityMetadataCreateParams { name: "some-totally-real-saml-provider" @@ -546,7 +549,7 @@ async fn test_saml_idp_metadata_data_valid( RequestBuilder::new( client, Method::GET, - &format!("/login/blahblah/{}", silo_saml_idp.identity.name), + &format!("/login/blahblah/saml/{}", silo_saml_idp.identity.name), ) .expect_status(Some(StatusCode::FOUND)), ) @@ -577,7 +580,7 @@ async fn test_saml_idp_metadata_data_truncated( RequestBuilder::new( client, Method::POST, - "/system/silos/blahblah/saml-identity-providers", + "/system/silos/blahblah/identity-providers/saml", ) .body(Some(¶ms::SamlIdentityProviderCreate { identity: IdentityMetadataCreateParams { @@ -630,7 +633,7 @@ async fn test_saml_idp_metadata_data_invalid( RequestBuilder::new( client, Method::POST, - &format!("/system/silos/{}/saml-identity-providers", SILO_NAME), + &format!("/system/silos/{}/identity-providers/saml", SILO_NAME), ) .body(Some(¶ms::SamlIdentityProviderCreate { identity: IdentityMetadataCreateParams { diff --git a/nexus/tests/output/nexus_tags.txt b/nexus/tests/output/nexus_tags.txt index a46f39818e3..6051edcf211 100644 --- a/nexus/tests/output/nexus_tags.txt +++ b/nexus/tests/output/nexus_tags.txt @@ -49,8 +49,8 @@ instance_view_by_id /by-id/instances/{id} API operations found with tag "login" OPERATION ID URL PATH -consume_credentials /login/{silo_name}/{provider_name} -login /login/{silo_name}/{provider_name} +consume_credentials /login/{silo_name}/saml/{provider_name} +login /login/{silo_name}/saml/{provider_name} API operations found with tag "metrics" OPERATION ID URL PATH @@ -128,11 +128,11 @@ rack_list /system/hardware/racks rack_view /system/hardware/racks/{rack_id} saga_list /system/sagas saga_view /system/sagas/{saga_id} +saml_identity_provider_create /system/silos/{silo_name}/identity-providers/saml +saml_identity_provider_view /system/silos/{silo_name}/identity-providers/saml/{provider_name} silo_create /system/silos silo_delete /system/silos/{silo_name} -silo_identity_provider_create /system/silos/{silo_name}/saml-identity-providers silo_identity_provider_list /system/silos/{silo_name}/identity-providers -silo_identity_provider_view /system/silos/{silo_name}/saml-identity-providers/{provider_name} silo_list /system/silos silo_policy_update /system/silos/{silo_name}/policy silo_policy_view /system/silos/{silo_name}/policy diff --git a/nexus/tests/output/uncovered-authz-endpoints.txt b/nexus/tests/output/uncovered-authz-endpoints.txt index 6107c0f27fa..c0adaeb957b 100644 --- a/nexus/tests/output/uncovered-authz-endpoints.txt +++ b/nexus/tests/output/uncovered-authz-endpoints.txt @@ -1,8 +1,8 @@ API endpoints with no coverage in authz tests: -login (get "/login/{silo_name}/{provider_name}") +login (get "/login/{silo_name}/saml/{provider_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}") +consume_credentials (post "/login/{silo_name}/saml/{provider_name}") logout (post "/logout") diff --git a/openapi/nexus.json b/openapi/nexus.json index a0c6da2df53..01121801819 100644 --- a/openapi/nexus.json +++ b/openapi/nexus.json @@ -558,7 +558,7 @@ } } }, - "/login/{silo_name}/{provider_name}": { + "/login/{silo_name}/saml/{provider_name}": { "get": { "tags": [ "login" @@ -6731,13 +6731,13 @@ "x-dropshot-pagination": true } }, - "/system/silos/{silo_name}/policy": { - "get": { + "/system/silos/{silo_name}/identity-providers/saml": { + "post": { "tags": [ "system" ], - "summary": "Fetch a silo's IAM policy", - "operationId": "silo_policy_view", + "summary": "Create a SAML IDP", + "operationId": "saml_identity_provider_create", "parameters": [ { "in": "path", @@ -6750,13 +6750,23 @@ "style": "simple" } ], + "requestBody": { + "content": { + "application/json": { + "schema": { + "$ref": "#/components/schemas/SamlIdentityProviderCreate" + } + } + }, + "required": true + }, "responses": { - "200": { - "description": "successful operation", + "201": { + "description": "successful creation", "content": { "application/json": { "schema": { - "$ref": "#/components/schemas/SiloRolePolicy" + "$ref": "#/components/schemas/SamlIdentityProvider" } } } @@ -6768,14 +6778,26 @@ "$ref": "#/components/responses/Error" } } - }, - "put": { + } + }, + "/system/silos/{silo_name}/identity-providers/saml/{provider_name}": { + "get": { "tags": [ "system" ], - "summary": "Update a silo's IAM policy", - "operationId": "silo_policy_update", + "summary": "Fetch a SAML IDP", + "operationId": "saml_identity_provider_view", "parameters": [ + { + "in": "path", + "name": "provider_name", + "description": "The SAML identity provider's name", + "required": true, + "schema": { + "$ref": "#/components/schemas/Name" + }, + "style": "simple" + }, { "in": "path", "name": "silo_name", @@ -6787,23 +6809,13 @@ "style": "simple" } ], - "requestBody": { - "content": { - "application/json": { - "schema": { - "$ref": "#/components/schemas/SiloRolePolicy" - } - } - }, - "required": true - }, "responses": { "200": { "description": "successful operation", "content": { "application/json": { "schema": { - "$ref": "#/components/schemas/SiloRolePolicy" + "$ref": "#/components/schemas/SamlIdentityProvider" } } } @@ -6817,13 +6829,13 @@ } } }, - "/system/silos/{silo_name}/saml-identity-providers": { - "post": { + "/system/silos/{silo_name}/policy": { + "get": { "tags": [ "system" ], - "summary": "Create a SAML IDP", - "operationId": "silo_identity_provider_create", + "summary": "Fetch a silo's IAM policy", + "operationId": "silo_policy_view", "parameters": [ { "in": "path", @@ -6836,23 +6848,13 @@ "style": "simple" } ], - "requestBody": { - "content": { - "application/json": { - "schema": { - "$ref": "#/components/schemas/SamlIdentityProviderCreate" - } - } - }, - "required": true - }, "responses": { - "201": { - "description": "successful creation", + "200": { + "description": "successful operation", "content": { "application/json": { "schema": { - "$ref": "#/components/schemas/SamlIdentityProvider" + "$ref": "#/components/schemas/SiloRolePolicy" } } } @@ -6864,26 +6866,14 @@ "$ref": "#/components/responses/Error" } } - } - }, - "/system/silos/{silo_name}/saml-identity-providers/{provider_name}": { - "get": { + }, + "put": { "tags": [ "system" ], - "summary": "Fetch a SAML IDP", - "operationId": "silo_identity_provider_view", + "summary": "Update a silo's IAM policy", + "operationId": "silo_policy_update", "parameters": [ - { - "in": "path", - "name": "provider_name", - "description": "The SAML identity provider's name", - "required": true, - "schema": { - "$ref": "#/components/schemas/Name" - }, - "style": "simple" - }, { "in": "path", "name": "silo_name", @@ -6895,13 +6885,23 @@ "style": "simple" } ], + "requestBody": { + "content": { + "application/json": { + "schema": { + "$ref": "#/components/schemas/SiloRolePolicy" + } + } + }, + "required": true + }, "responses": { "200": { "description": "successful operation", "content": { "application/json": { "schema": { - "$ref": "#/components/schemas/SamlIdentityProvider" + "$ref": "#/components/schemas/SiloRolePolicy" } } } From b575ec28eceaeaa41ff3a5b47ea81a255faa5c0d Mon Sep 17 00:00:00 2001 From: David Pacheco Date: Fri, 30 Sep 2022 15:23:29 -0700 Subject: [PATCH 2/9] UserProvisionType, AuthenticationMode, SiloIdentityMode --- common/src/sql/dbinit.sql | 2 +- nexus/db-model/src/silo.rs | 20 +++-- nexus/src/app/silo.rs | 8 +- nexus/src/db/fixed_data/silo.rs | 2 +- nexus/test-utils/src/resource_helpers.rs | 4 +- nexus/tests/integration_tests/authz.rs | 80 ++++++++++++++------ nexus/tests/integration_tests/endpoints.rs | 2 +- nexus/tests/integration_tests/saml.rs | 22 +++--- nexus/tests/integration_tests/silos.rs | 88 +++++++++++++--------- nexus/types/src/external_api/params.rs | 2 +- nexus/types/src/external_api/shared.rs | 54 +++++++++++-- nexus/types/src/external_api/views.rs | 4 +- openapi/nexus.json | 46 +++++------ 13 files changed, 221 insertions(+), 113 deletions(-) diff --git a/common/src/sql/dbinit.sql b/common/src/sql/dbinit.sql index 198bb09a6f4..6e0cb7caf0c 100644 --- a/common/src/sql/dbinit.sql +++ b/common/src/sql/dbinit.sql @@ -306,7 +306,7 @@ CREATE INDEX on omicron.public.volume ( */ CREATE TYPE omicron.public.user_provision_type AS ENUM ( - 'fixed', + 'api_only', 'jit' ); diff --git a/nexus/db-model/src/silo.rs b/nexus/db-model/src/silo.rs index 4ed28dcb635..de0ea0f06cb 100644 --- a/nexus/db-model/src/silo.rs +++ b/nexus/db-model/src/silo.rs @@ -7,6 +7,7 @@ use crate::collection::DatastoreCollectionConfig; use crate::impl_enum_type; use crate::schema::{organization, silo}; use db_macros::Resource; +use nexus_types::external_api::shared::SiloIdentityMode; use nexus_types::external_api::views; use nexus_types::external_api::{params, shared}; use nexus_types::identity::Resource; @@ -22,14 +23,14 @@ impl_enum_type!( pub enum UserProvisionType; // Enum values - Fixed => b"fixed" + ApiOnly => b"api_only" Jit => b"jit" ); impl From for UserProvisionType { fn from(params: shared::UserProvisionType) -> Self { match params { - shared::UserProvisionType::Fixed => UserProvisionType::Fixed, + shared::UserProvisionType::ApiOnly => UserProvisionType::ApiOnly, shared::UserProvisionType::Jit => UserProvisionType::Jit, } } @@ -38,7 +39,7 @@ impl From for UserProvisionType { impl From for shared::UserProvisionType { fn from(model: UserProvisionType) -> Self { match model { - UserProvisionType::Fixed => Self::Fixed, + UserProvisionType::ApiOnly => Self::ApiOnly, UserProvisionType::Jit => Self::Jit, } } @@ -69,7 +70,10 @@ impl Silo { Self { identity: SiloIdentity::new(id, params.identity), discoverable: params.discoverable, - user_provision_type: params.user_provision_type.into(), + user_provision_type: params + .identity_mode + .user_provision_type() + .into(), rcgen: Generation::new(), } } @@ -77,10 +81,16 @@ impl Silo { impl From for views::Silo { fn from(silo: Silo) -> Self { + // In the future, we'll want to store the authentication mode and look + // at that here, too. + let identity_mode = match silo.user_provision_type { + UserProvisionType::Jit => SiloIdentityMode::SamlJit, + UserProvisionType::ApiOnly => SiloIdentityMode::LocalOnly, + }; Self { identity: silo.identity(), discoverable: silo.discoverable, - user_provision_type: silo.user_provision_type.into(), + identity_mode, } } } diff --git a/nexus/src/app/silo.rs b/nexus/src/app/silo.rs index d6e498fee2a..a4490a714b4 100644 --- a/nexus/src/app/silo.rs +++ b/nexus/src/app/silo.rs @@ -181,9 +181,9 @@ impl super::Nexus { // external id. The next action depends on the silo's user // provision type. match db_silo.user_provision_type { - // If the user provision type is fixed, do not a new user if - // one does not exist. - db::model::UserProvisionType::Fixed => { + // If the user provision type is ApiOnly, do not a new user + // if one does not exist. + db::model::UserProvisionType::ApiOnly => { return Ok(None); } @@ -212,7 +212,7 @@ impl super::Nexus { for group in &authenticated_subject.groups { let silo_group = match db_silo.user_provision_type { - db::model::UserProvisionType::Fixed => { + db::model::UserProvisionType::ApiOnly => { self.db_datastore .silo_group_optional_lookup( opctx, diff --git a/nexus/src/db/fixed_data/silo.rs b/nexus/src/db/fixed_data/silo.rs index 1419d39d139..3c910177291 100644 --- a/nexus/src/db/fixed_data/silo.rs +++ b/nexus/src/db/fixed_data/silo.rs @@ -19,7 +19,7 @@ lazy_static! { description: "default silo".to_string(), }, discoverable: false, - user_provision_type: shared::UserProvisionType::Fixed, + identity_mode: shared::SiloIdentityMode::LocalOnly, admin_group_name: None, } ); diff --git a/nexus/test-utils/src/resource_helpers.rs b/nexus/test-utils/src/resource_helpers.rs index f1790878644..3ead59bff43 100644 --- a/nexus/test-utils/src/resource_helpers.rs +++ b/nexus/test-utils/src/resource_helpers.rs @@ -108,7 +108,7 @@ pub async fn create_silo( client: &ClientTestContext, silo_name: &str, discoverable: bool, - user_provision_type: shared::UserProvisionType, + identity_mode: shared::SiloIdentityMode, ) -> Silo { object_create( client, @@ -119,7 +119,7 @@ pub async fn create_silo( description: "a silo".to_string(), }, discoverable, - user_provision_type, + identity_mode, admin_group_name: None, }, ) diff --git a/nexus/tests/integration_tests/authz.rs b/nexus/tests/integration_tests/authz.rs index 07ef212fe26..8654a6bae91 100644 --- a/nexus/tests/integration_tests/authz.rs +++ b/nexus/tests/integration_tests/authz.rs @@ -28,9 +28,13 @@ 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::SiloIdentityMode::LocalOnly, + ) + .await; let user1 = Uuid::new_v4(); nexus @@ -130,9 +134,13 @@ async fn test_global_image_read_for_unpriv( 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::SiloIdentityMode::LocalOnly, + ) + .await; let new_silo_user_id = Uuid::new_v4(); nexus @@ -206,9 +214,13 @@ 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::SiloIdentityMode::LocalOnly, + ) + .await; let new_silo_user_id = Uuid::new_v4(); nexus @@ -217,9 +229,13 @@ async fn test_list_silo_users_for_unpriv(cptestctx: &ControlPlaneTestContext) { .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::SiloIdentityMode::LocalOnly, + ) + .await; nexus .silo_user_create(silo.identity.id, Uuid::new_v4(), "otheruser".into()) @@ -249,9 +265,13 @@ 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::SiloIdentityMode::LocalOnly, + ) + .await; let new_silo_user_id = Uuid::new_v4(); nexus @@ -279,9 +299,13 @@ 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::SiloIdentityMode::LocalOnly, + ) + .await; let new_silo_user_id = Uuid::new_v4(); nexus @@ -306,9 +330,13 @@ 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::SiloIdentityMode::LocalOnly, + ) + .await; let new_silo_user_id = Uuid::new_v4(); nexus @@ -317,9 +345,13 @@ async fn test_silo_read_for_unpriv(cptestctx: &ControlPlaneTestContext) { .unwrap(); // Create another silo - let _silo = - create_silo(&client, "other", true, shared::UserProvisionType::Fixed) - .await; + let _silo = create_silo( + &client, + "other", + true, + shared::SiloIdentityMode::LocalOnly, + ) + .await; // That user can access their own silo let _silo: views::Silo = diff --git a/nexus/tests/integration_tests/endpoints.rs b/nexus/tests/integration_tests/endpoints.rs index b085d79734f..42f8d58122c 100644 --- a/nexus/tests/integration_tests/endpoints.rs +++ b/nexus/tests/integration_tests/endpoints.rs @@ -56,7 +56,7 @@ lazy_static! { description: String::from(""), }, discoverable: true, - user_provision_type: shared::UserProvisionType::Fixed, + identity_mode: shared::SiloIdentityMode::LocalOnly, admin_group_name: None, }; diff --git a/nexus/tests/integration_tests/saml.rs b/nexus/tests/integration_tests/saml.rs index 8b8b0225591..be8b34e6e37 100644 --- a/nexus/tests/integration_tests/saml.rs +++ b/nexus/tests/integration_tests/saml.rs @@ -32,7 +32,7 @@ async fn test_create_a_saml_idp(cptestctx: &ControlPlaneTestContext) { let client = &cptestctx.external_client; const SILO_NAME: &str = "saml-silo"; - create_silo(&client, SILO_NAME, true, shared::UserProvisionType::Fixed) + create_silo(&client, SILO_NAME, true, shared::SiloIdentityMode::LocalOnly) .await; let silo: Silo = NexusRequest::object_get( &client, @@ -151,7 +151,7 @@ async fn test_create_a_saml_idp_invalid_descriptor_truncated( let client = &cptestctx.external_client; const SILO_NAME: &str = "saml-silo"; - create_silo(&client, SILO_NAME, true, shared::UserProvisionType::Fixed) + create_silo(&client, SILO_NAME, true, shared::SiloIdentityMode::LocalOnly) .await; let saml_idp_descriptor = { @@ -211,7 +211,7 @@ async fn test_create_a_saml_idp_invalid_descriptor_no_redirect_binding( let client = &cptestctx.external_client; const SILO_NAME: &str = "saml-silo"; - create_silo(&client, SILO_NAME, true, shared::UserProvisionType::Fixed) + create_silo(&client, SILO_NAME, true, shared::SiloIdentityMode::LocalOnly) .await; let saml_idp_descriptor = { @@ -281,7 +281,7 @@ async fn test_create_a_hidden_silo_saml_idp( ) { let client = &cptestctx.external_client; - create_silo(&client, "hidden", false, shared::UserProvisionType::Fixed) + create_silo(&client, "hidden", false, shared::SiloIdentityMode::LocalOnly) .await; // Valid IdP descriptor @@ -350,7 +350,7 @@ async fn test_saml_idp_metadata_url_404(cptestctx: &ControlPlaneTestContext) { let client = &cptestctx.external_client; const SILO_NAME: &str = "saml-silo"; - create_silo(&client, SILO_NAME, true, shared::UserProvisionType::Fixed) + create_silo(&client, SILO_NAME, true, shared::SiloIdentityMode::LocalOnly) .await; let server = Server::run(); @@ -404,7 +404,7 @@ async fn test_saml_idp_metadata_url_invalid( let client = &cptestctx.external_client; const SILO_NAME: &str = "saml-silo"; - create_silo(&client, SILO_NAME, true, shared::UserProvisionType::Fixed) + create_silo(&client, SILO_NAME, true, shared::SiloIdentityMode::LocalOnly) .await; NexusRequest::new( @@ -465,7 +465,7 @@ async fn test_saml_idp_reject_keypair(cptestctx: &ControlPlaneTestContext) { ); const SILO_NAME: &str = "saml-silo"; - create_silo(&client, SILO_NAME, true, shared::UserProvisionType::Fixed) + create_silo(&client, SILO_NAME, true, shared::SiloIdentityMode::LocalOnly) .await; let test_cases = vec![ @@ -560,7 +560,7 @@ async fn test_saml_idp_rsa_keypair_ok(cptestctx: &ControlPlaneTestContext) { ); const SILO_NAME: &str = "saml-silo"; - create_silo(&client, SILO_NAME, true, shared::UserProvisionType::Fixed) + create_silo(&client, SILO_NAME, true, shared::SiloIdentityMode::LocalOnly) .await; NexusRequest::new( @@ -932,7 +932,8 @@ async fn test_post_saml_response(cptestctx: &ControlPlaneTestContext) { let client = &cptestctx.external_client; const SILO_NAME: &str = "saml-silo"; - create_silo(&client, SILO_NAME, true, shared::UserProvisionType::Jit).await; + create_silo(&client, SILO_NAME, true, shared::SiloIdentityMode::SamlJit) + .await; let _silo_saml_idp: views::SamlIdentityProvider = object_create( client, @@ -1028,7 +1029,8 @@ async fn test_post_saml_response_with_relay_state( let client = &cptestctx.external_client; const SILO_NAME: &str = "saml-silo"; - create_silo(&client, SILO_NAME, true, shared::UserProvisionType::Jit).await; + create_silo(&client, SILO_NAME, true, shared::SiloIdentityMode::SamlJit) + .await; let _silo_saml_idp: views::SamlIdentityProvider = object_create( client, diff --git a/nexus/tests/integration_tests/silos.rs b/nexus/tests/integration_tests/silos.rs index 132597555d5..f270f2685f4 100644 --- a/nexus/tests/integration_tests/silos.rs +++ b/nexus/tests/integration_tests/silos.rs @@ -42,10 +42,10 @@ async fn test_silos(cptestctx: &ControlPlaneTestContext) { &client, "discoverable", true, - shared::UserProvisionType::Fixed, + shared::SiloIdentityMode::LocalOnly, ) .await; - create_silo(&client, "hidden", false, shared::UserProvisionType::Fixed) + create_silo(&client, "hidden", false, shared::SiloIdentityMode::LocalOnly) .await; // Verify GET /system/silos/{silo} works for both discoverable and not @@ -227,7 +227,7 @@ async fn test_silo_admin_group(cptestctx: &ControlPlaneTestContext) { description: "a silo".to_string(), }, discoverable: false, - user_provision_type: shared::UserProvisionType::Jit, + identity_mode: shared::SiloIdentityMode::SamlJit, admin_group_name: Some("administrator".into()), }, ) @@ -512,7 +512,7 @@ async fn test_saml_idp_metadata_data_valid( ) { let client = &cptestctx.external_client; - create_silo(&client, "blahblah", true, shared::UserProvisionType::Fixed) + create_silo(&client, "blahblah", true, shared::SiloIdentityMode::LocalOnly) .await; let silo_saml_idp: SamlIdentityProvider = object_create( @@ -573,7 +573,7 @@ async fn test_saml_idp_metadata_data_truncated( ) { let client = &cptestctx.external_client; - create_silo(&client, "blahblah", true, shared::UserProvisionType::Fixed) + create_silo(&client, "blahblah", true, shared::SiloIdentityMode::LocalOnly) .await; NexusRequest::new( @@ -626,7 +626,7 @@ async fn test_saml_idp_metadata_data_invalid( let client = &cptestctx.external_client; const SILO_NAME: &str = "saml-silo"; - create_silo(&client, SILO_NAME, true, shared::UserProvisionType::Fixed) + create_silo(&client, SILO_NAME, true, shared::SiloIdentityMode::LocalOnly) .await; NexusRequest::new( @@ -667,7 +667,7 @@ async fn test_saml_idp_metadata_data_invalid( } struct TestSiloUserProvisionTypes { - provision_type: shared::UserProvisionType, + identity_mode: shared::SiloIdentityMode, existing_silo_user: bool, expect_user: bool, } @@ -681,28 +681,28 @@ async fn test_silo_user_provision_types(cptestctx: &ControlPlaneTestContext) { // A silo configured with a "fixed" user provision type should fetch a // user if it exists already. TestSiloUserProvisionTypes { - provision_type: shared::UserProvisionType::Fixed, + identity_mode: shared::SiloIdentityMode::LocalOnly, existing_silo_user: true, expect_user: true, }, - // A silo configured with a "fixed" user provision type should not create a - // user if one does not exist already. + // A silo configured with a "fixed" user provision type should not + // create a user if one does not exist already. TestSiloUserProvisionTypes { - provision_type: shared::UserProvisionType::Fixed, + identity_mode: shared::SiloIdentityMode::LocalOnly, existing_silo_user: false, expect_user: false, }, // A silo configured with a "JIT" user provision type should fetch a // user if it exists already. TestSiloUserProvisionTypes { - provision_type: shared::UserProvisionType::Jit, + identity_mode: shared::SiloIdentityMode::SamlJit, existing_silo_user: true, expect_user: true, }, - // A silo configured with a "JIT" user provision type should create a user - // if one does not exist already. + // A silo configured with a "JIT" user provision type should create a + // user if one does not exist already. TestSiloUserProvisionTypes { - provision_type: shared::UserProvisionType::Jit, + identity_mode: shared::SiloIdentityMode::SamlJit, existing_silo_user: false, expect_user: true, }, @@ -710,7 +710,7 @@ async fn test_silo_user_provision_types(cptestctx: &ControlPlaneTestContext) { for test_case in test_cases { let silo = - create_silo(&client, "test-silo", true, test_case.provision_type) + create_silo(&client, "test-silo", true, test_case.identity_mode) .await; if test_case.existing_silo_user { @@ -771,7 +771,7 @@ async fn test_silo_user_fetch_by_external_id( &client, "test-silo", true, - shared::UserProvisionType::Fixed, + shared::SiloIdentityMode::LocalOnly, ) .await; @@ -890,7 +890,7 @@ async fn test_silo_users_list(cptestctx: &ControlPlaneTestContext) { // able to see the users in the first Silo. let silo = - create_silo(client, "silo2", true, shared::UserProvisionType::Fixed) + create_silo(client, "silo2", true, shared::SiloIdentityMode::LocalOnly) .await; let new_silo_user_id = "6922f0b2-9a92-659b-da6b-93ad4955a3a3".parse().unwrap(); @@ -946,9 +946,13 @@ async fn test_silo_groups_jit(cptestctx: &ControlPlaneTestContext) { let client = &cptestctx.external_client; let nexus = &cptestctx.server.apictx.nexus; - let silo = - create_silo(&client, "test-silo", true, shared::UserProvisionType::Jit) - .await; + let silo = create_silo( + &client, + "test-silo", + true, + shared::SiloIdentityMode::SamlJit, + ) + .await; // Create a user in advance let silo_user_id = Uuid::new_v4(); @@ -1022,7 +1026,7 @@ async fn test_silo_groups_fixed(cptestctx: &ControlPlaneTestContext) { &client, "test-silo", true, - shared::UserProvisionType::Fixed, + shared::SiloIdentityMode::LocalOnly, ) .await; @@ -1081,9 +1085,13 @@ async fn test_silo_groups_remove_from_one_group( let client = &cptestctx.external_client; let nexus = &cptestctx.server.apictx.nexus; - let silo = - create_silo(&client, "test-silo", true, shared::UserProvisionType::Jit) - .await; + let silo = create_silo( + &client, + "test-silo", + true, + shared::SiloIdentityMode::SamlJit, + ) + .await; // Create a user in advance let silo_user_id = Uuid::new_v4(); @@ -1197,9 +1205,13 @@ async fn test_silo_groups_remove_from_both_groups( let client = &cptestctx.external_client; let nexus = &cptestctx.server.apictx.nexus; - let silo = - create_silo(&client, "test-silo", true, shared::UserProvisionType::Jit) - .await; + let silo = create_silo( + &client, + "test-silo", + true, + shared::SiloIdentityMode::SamlJit, + ) + .await; // Create a user in advance let silo_user_id = Uuid::new_v4(); @@ -1313,9 +1325,13 @@ async fn test_silo_delete_clean_up_groups(cptestctx: &ControlPlaneTestContext) { let nexus = &cptestctx.server.apictx.nexus; // Create a silo - let silo = - create_silo(&client, "test-silo", true, shared::UserProvisionType::Jit) - .await; + let silo = create_silo( + &client, + "test-silo", + true, + shared::SiloIdentityMode::SamlJit, + ) + .await; let opctx_external_authn = nexus.opctx_external_authn(); let opctx = OpContext::for_tests( @@ -1391,9 +1407,13 @@ async fn test_ensure_same_silo_group(cptestctx: &ControlPlaneTestContext) { let nexus = &cptestctx.server.apictx.nexus; // Create a silo - let silo = - create_silo(&client, "test-silo", true, shared::UserProvisionType::Jit) - .await; + let silo = create_silo( + &client, + "test-silo", + true, + shared::SiloIdentityMode::SamlJit, + ) + .await; let opctx = OpContext::for_tests( cptestctx.logctx.log.new(o!()), diff --git a/nexus/types/src/external_api/params.rs b/nexus/types/src/external_api/params.rs index 7ff37c83ba0..a2d897e119e 100644 --- a/nexus/types/src/external_api/params.rs +++ b/nexus/types/src/external_api/params.rs @@ -28,7 +28,7 @@ pub struct SiloCreate { pub discoverable: bool, - pub user_provision_type: shared::UserProvisionType, + pub identity_mode: shared::SiloIdentityMode, /// If set, this group will be created during Silo creation and granted the /// "Silo Admin" role. Identity providers can assert that users belong to diff --git a/nexus/types/src/external_api/shared.rs b/nexus/types/src/external_api/shared.rs index 359c307fe0a..93be7a29ac8 100644 --- a/nexus/types/src/external_api/shared.rs +++ b/nexus/types/src/external_api/shared.rs @@ -90,16 +90,60 @@ pub enum IdentityType { SiloGroup, } +/// Describes how identities are managed and users are authenticated in this +/// Silo +#[derive(Clone, Debug, Deserialize, Serialize, JsonSchema)] +#[serde(rename_all = "snake_case")] +pub enum SiloIdentityMode { + /// Users are authenticated with SAML using an external authentication + /// provider. The system updates information about users and groups only + /// during authentication (i.e,. "JIT provisioning" of users and groups). + SamlJit, + + /// The system is the source of truth about users. There is no linkage to + /// an external authentication provider or identity provider. + // NOTE: authentication for these users is not supported yet at all. It + // will eventually be password-based. + LocalOnly, +} + +impl SiloIdentityMode { + pub fn authn_type(&self) -> Option { + match self { + SiloIdentityMode::LocalOnly => None, + SiloIdentityMode::SamlJit => Some(AuthenticationMode::Saml), + } + } + + pub fn user_provision_type(&self) -> UserProvisionType { + match self { + SiloIdentityMode::LocalOnly => UserProvisionType::ApiOnly, + SiloIdentityMode::SamlJit => UserProvisionType::Jit, + } + } +} + +// XXX-dap remove from "shared"? +/// How users are authenticated in this Silo +#[derive(Clone, Debug, Deserialize, Serialize, JsonSchema)] +#[serde(rename_all = "snake_case")] +pub enum AuthenticationMode { + /// Authentication is via SAML using an external authentication provider + Saml, +} + +// XXX-dap remove from "shared"? /// How users will be provisioned in a silo during authentication. #[derive(Clone, Debug, Deserialize, Serialize, JsonSchema)] #[serde(rename_all = "snake_case")] pub enum UserProvisionType { - /// Do not automatically create users during authentication if they do not - /// exist in the database already. - Fixed, + /// Identities are managed directly by explicit calls to the external API. + /// They are not synchronized from any external identity provider nor + /// automatically created or updated when a user logs in. + ApiOnly, - /// Create users during authentication if they do not exist in the database - /// already, using information provided by the identity provider. + /// Users and groups are created or updated during authentication using + /// information provided by the authentication provider Jit, } diff --git a/nexus/types/src/external_api/views.rs b/nexus/types/src/external_api/views.rs index c035060f80a..aa68b82ecc5 100644 --- a/nexus/types/src/external_api/views.rs +++ b/nexus/types/src/external_api/views.rs @@ -58,8 +58,8 @@ pub struct Silo { /// will not be part of the "list all silos" output. pub discoverable: bool, - /// User provision type - pub user_provision_type: shared::UserProvisionType, + /// How users and groups are managed in this Silo + pub identity_mode: shared::SiloIdentityMode, } // IDENTITY PROVIDER diff --git a/openapi/nexus.json b/openapi/nexus.json index 01121801819..d906606778d 100644 --- a/openapi/nexus.json +++ b/openapi/nexus.json @@ -10592,6 +10592,14 @@ "type": "string", "format": "uuid" }, + "identity_mode": { + "description": "How users and groups are managed in this Silo", + "allOf": [ + { + "$ref": "#/components/schemas/SiloIdentityMode" + } + ] + }, "name": { "description": "unique, mutable, user-controlled identifier for each resource", "allOf": [ @@ -10609,24 +10617,16 @@ "description": "timestamp when this resource was last modified", "type": "string", "format": "date-time" - }, - "user_provision_type": { - "description": "User provision type", - "allOf": [ - { - "$ref": "#/components/schemas/UserProvisionType" - } - ] } }, "required": [ "description", "discoverable", "id", + "identity_mode", "name", "time_created", - "time_modified", - "user_provision_type" + "time_modified" ] }, "SiloCreate": { @@ -10644,18 +10644,26 @@ "discoverable": { "type": "boolean" }, + "identity_mode": { + "$ref": "#/components/schemas/SiloIdentityMode" + }, "name": { "$ref": "#/components/schemas/Name" - }, - "user_provision_type": { - "$ref": "#/components/schemas/UserProvisionType" } }, "required": [ "description", "discoverable", - "name", - "user_provision_type" + "identity_mode", + "name" + ] + }, + "SiloIdentityMode": { + "description": "Describes how identities are managed and users are authenticated in this Silo", + "type": "string", + "enum": [ + "saml_jit", + "local_only" ] }, "SiloResultsPage": { @@ -11126,14 +11134,6 @@ "items" ] }, - "UserProvisionType": { - "description": "How users will be provisioned in a silo during authentication.", - "type": "string", - "enum": [ - "fixed", - "jit" - ] - }, "UserResultsPage": { "description": "A single page of results", "type": "object", From 79326e242fa35c410746530bc83c04293b52e8d2 Mon Sep 17 00:00:00 2001 From: David Pacheco Date: Fri, 30 Sep 2022 15:42:52 -0700 Subject: [PATCH 3/9] RFD 321: add authn mode enum, unified enum --- common/src/sql/dbinit.sql | 6 ++ nexus/db-model/src/schema.rs | 1 + nexus/db-model/src/silo.rs | 73 +++++++++++++++++++--- nexus/src/external_api/http_entrypoints.rs | 10 +-- nexus/types/src/external_api/shared.rs | 9 ++- 5 files changed, 81 insertions(+), 18 deletions(-) diff --git a/common/src/sql/dbinit.sql b/common/src/sql/dbinit.sql index 6e0cb7caf0c..0a417405b65 100644 --- a/common/src/sql/dbinit.sql +++ b/common/src/sql/dbinit.sql @@ -305,6 +305,11 @@ CREATE INDEX on omicron.public.volume ( * Silos */ +CREATE TYPE omicron.public.authentication_mode AS ENUM ( + 'local', + 'saml' +); + CREATE TYPE omicron.public.user_provision_type AS ENUM ( 'api_only', 'jit' @@ -320,6 +325,7 @@ CREATE TABLE omicron.public.silo ( time_deleted TIMESTAMPTZ, discoverable BOOL NOT NULL, + authentication_mode omicron.public.authentication_mode NOT NULL, user_provision_type omicron.public.user_provision_type NOT NULL, /* child resource generation number, per RFD 192 */ diff --git a/nexus/db-model/src/schema.rs b/nexus/db-model/src/schema.rs index b2e30402e96..b2f8eb0711e 100644 --- a/nexus/db-model/src/schema.rs +++ b/nexus/db-model/src/schema.rs @@ -196,6 +196,7 @@ table! { time_deleted -> Nullable, discoverable -> Bool, + authentication_mode -> crate::AuthenticationModeEnum, user_provision_type -> crate::UserProvisionTypeEnum, rcgen -> Int8, diff --git a/nexus/db-model/src/silo.rs b/nexus/db-model/src/silo.rs index de0ea0f06cb..4e43c44dd41 100644 --- a/nexus/db-model/src/silo.rs +++ b/nexus/db-model/src/silo.rs @@ -11,8 +11,41 @@ use nexus_types::external_api::shared::SiloIdentityMode; use nexus_types::external_api::views; use nexus_types::external_api::{params, shared}; use nexus_types::identity::Resource; +use omicron_common::api::external::Error; use uuid::Uuid; +impl_enum_type!( + #[derive(SqlType, Debug, QueryId)] + #[diesel(postgres_type(name = "authentication_mode"))] + pub struct AuthenticationModeEnum; + + #[derive(Copy, Clone, Debug, AsExpression, FromSqlRow, PartialEq)] + #[diesel(sql_type = AuthenticationModeEnum)] + pub enum AuthenticationMode; + + // Enum values + Local => b"local" + Saml => b"saml" +); + +impl From for AuthenticationMode { + fn from(params: shared::AuthenticationMode) -> Self { + match params { + shared::AuthenticationMode::Local => AuthenticationMode::Local, + shared::AuthenticationMode::Saml => AuthenticationMode::Saml, + } + } +} + +impl From for shared::AuthenticationMode { + fn from(model: AuthenticationMode) -> Self { + match model { + AuthenticationMode::Local => Self::Local, + AuthenticationMode::Saml => Self::Saml, + } + } +} + impl_enum_type!( #[derive(SqlType, Debug, QueryId)] #[diesel(postgres_type(name = "user_provision_type"))] @@ -54,6 +87,7 @@ pub struct Silo { pub discoverable: bool, + pub authentication_mode: AuthenticationMode, pub user_provision_type: UserProvisionType, /// child resource generation number, per RFD 192 @@ -70,6 +104,10 @@ impl Silo { Self { identity: SiloIdentity::new(id, params.identity), discoverable: params.discoverable, + authentication_mode: params + .identity_mode + .authentication_mode() + .into(), user_provision_type: params .identity_mode .user_provision_type() @@ -79,19 +117,34 @@ impl Silo { } } -impl From for views::Silo { - fn from(silo: Silo) -> Self { - // In the future, we'll want to store the authentication mode and look - // at that here, too. - let identity_mode = match silo.user_provision_type { - UserProvisionType::Jit => SiloIdentityMode::SamlJit, - UserProvisionType::ApiOnly => SiloIdentityMode::LocalOnly, - }; - Self { +impl TryFrom for views::Silo { + type Error = Error; + fn try_from(silo: Silo) -> Result { + let authn_mode = &silo.authentication_mode; + let user_type = &silo.user_provision_type; + let identity_mode = match (authn_mode, user_type) { + (AuthenticationMode::Saml, UserProvisionType::Jit) => { + Some(SiloIdentityMode::SamlJit) + } + (AuthenticationMode::Saml, UserProvisionType::ApiOnly) => None, + (AuthenticationMode::Local, UserProvisionType::ApiOnly) => { + Some(SiloIdentityMode::LocalOnly) + } + (AuthenticationMode::Local, UserProvisionType::Jit) => None, + } + .ok_or_else(|| { + Error::internal_error(&format!( + "unsupported combination of authentication mode ({:?}) and \ + user provision type ({:?})", + authn_mode, user_type + )) + })?; + + Ok(Self { identity: silo.identity(), discoverable: silo.discoverable, identity_mode, - } + }) } } diff --git a/nexus/src/external_api/http_entrypoints.rs b/nexus/src/external_api/http_entrypoints.rs index 0211e4c80dc..26b6c234552 100644 --- a/nexus/src/external_api/http_entrypoints.rs +++ b/nexus/src/external_api/http_entrypoints.rs @@ -460,8 +460,8 @@ async fn silo_list( } } .into_iter() - .map(|p| p.into()) - .collect(); + .map(|p| p.try_into()) + .collect::, Error>>()?; Ok(HttpResponseOk(ScanByNameOrId::results_page( &query, silos, @@ -487,7 +487,7 @@ async fn silo_create( let opctx = OpContext::for_external_api(&rqctx).await?; let silo = nexus.silo_create(&opctx, new_silo_params.into_inner()).await?; - Ok(HttpResponseCreated(silo.into())) + Ok(HttpResponseCreated(silo.try_into()?)) }; apictx.external_latencies.instrument_dropshot_handler(&rqctx, handler).await } @@ -518,7 +518,7 @@ async fn silo_view( let handler = async { let opctx = OpContext::for_external_api(&rqctx).await?; let silo = nexus.silo_fetch(&opctx, &silo_name).await?; - Ok(HttpResponseOk(silo.into())) + Ok(HttpResponseOk(silo.try_into()?)) }; apictx.external_latencies.instrument_dropshot_handler(&rqctx, handler).await } @@ -540,7 +540,7 @@ async fn silo_view_by_id( let handler = async { let opctx = OpContext::for_external_api(&rqctx).await?; let silo = nexus.silo_fetch_by_id(&opctx, id).await?; - Ok(HttpResponseOk(silo.into())) + Ok(HttpResponseOk(silo.try_into()?)) }; apictx.external_latencies.instrument_dropshot_handler(&rqctx, handler).await } diff --git a/nexus/types/src/external_api/shared.rs b/nexus/types/src/external_api/shared.rs index 93be7a29ac8..58e98bde90c 100644 --- a/nexus/types/src/external_api/shared.rs +++ b/nexus/types/src/external_api/shared.rs @@ -108,10 +108,10 @@ pub enum SiloIdentityMode { } impl SiloIdentityMode { - pub fn authn_type(&self) -> Option { + pub fn authentication_mode(&self) -> AuthenticationMode { match self { - SiloIdentityMode::LocalOnly => None, - SiloIdentityMode::SamlJit => Some(AuthenticationMode::Saml), + SiloIdentityMode::LocalOnly => AuthenticationMode::Local, + SiloIdentityMode::SamlJit => AuthenticationMode::Saml, } } @@ -130,6 +130,9 @@ impl SiloIdentityMode { pub enum AuthenticationMode { /// Authentication is via SAML using an external authentication provider Saml, + + /// Authentication is local to the Oxide system + Local, } // XXX-dap remove from "shared"? From a98dced6aed5aead6ce536cc1b38c9f5039739eb Mon Sep 17 00:00:00 2001 From: David Pacheco Date: Fri, 30 Sep 2022 16:19:34 -0700 Subject: [PATCH 4/9] remove XXX --- nexus/types/src/external_api/shared.rs | 2 -- 1 file changed, 2 deletions(-) diff --git a/nexus/types/src/external_api/shared.rs b/nexus/types/src/external_api/shared.rs index 58e98bde90c..b028fd7c3d3 100644 --- a/nexus/types/src/external_api/shared.rs +++ b/nexus/types/src/external_api/shared.rs @@ -123,7 +123,6 @@ impl SiloIdentityMode { } } -// XXX-dap remove from "shared"? /// How users are authenticated in this Silo #[derive(Clone, Debug, Deserialize, Serialize, JsonSchema)] #[serde(rename_all = "snake_case")] @@ -135,7 +134,6 @@ pub enum AuthenticationMode { Local, } -// XXX-dap remove from "shared"? /// How users will be provisioned in a silo during authentication. #[derive(Clone, Debug, Deserialize, Serialize, JsonSchema)] #[serde(rename_all = "snake_case")] From 394c3be9836510b0147a24722654c991fa060418 Mon Sep 17 00:00:00 2001 From: David Pacheco Date: Fri, 30 Sep 2022 16:40:14 -0700 Subject: [PATCH 5/9] prevent creating SAML IdP in LocalOnly Silos --- nexus/src/app/silo.rs | 17 ++++++++++++-- nexus/tests/integration_tests/endpoints.rs | 2 +- nexus/tests/integration_tests/saml.rs | 16 ++++++------- nexus/tests/integration_tests/silos.rs | 26 +++++++++++++--------- nexus/types/src/external_api/shared.rs | 3 ++- 5 files changed, 42 insertions(+), 22 deletions(-) diff --git a/nexus/src/app/silo.rs b/nexus/src/app/silo.rs index a4490a714b4..4d288bacbc0 100644 --- a/nexus/src/app/silo.rs +++ b/nexus/src/app/silo.rs @@ -181,8 +181,8 @@ impl super::Nexus { // external id. The next action depends on the silo's user // provision type. match db_silo.user_provision_type { - // If the user provision type is ApiOnly, do not a new user - // if one does not exist. + // If the user provision type is ApiOnly, do not create a + // new user if one does not exist. db::model::UserProvisionType::ApiOnly => { return Ok(None); } @@ -384,6 +384,19 @@ impl super::Nexus { // an external source. opctx.authorize(authz::Action::CreateChild, &authz_idp_list).await?; + // The authentication mode is immutable so it's safe to check this here + // and bail out. + if db_silo.authentication_mode + != nexus_db_model::AuthenticationMode::Saml + { + return Err(Error::invalid_request(&format!( + "cannot create SAML identity provider for this Silo type \ + (expected authentication mode {:?}, found {:?})", + nexus_db_model::AuthenticationMode::Saml, + &db_silo.authentication_mode, + ))); + } + let idp_metadata_document_string = match ¶ms.idp_metadata_source { params::IdpMetadataSource::Url { url } => { // Download the SAML IdP descriptor, and write it into the DB. diff --git a/nexus/tests/integration_tests/endpoints.rs b/nexus/tests/integration_tests/endpoints.rs index 42f8d58122c..e2f48aa1b84 100644 --- a/nexus/tests/integration_tests/endpoints.rs +++ b/nexus/tests/integration_tests/endpoints.rs @@ -56,7 +56,7 @@ lazy_static! { description: String::from(""), }, discoverable: true, - identity_mode: shared::SiloIdentityMode::LocalOnly, + identity_mode: shared::SiloIdentityMode::SamlJit, admin_group_name: None, }; diff --git a/nexus/tests/integration_tests/saml.rs b/nexus/tests/integration_tests/saml.rs index be8b34e6e37..8398fef4136 100644 --- a/nexus/tests/integration_tests/saml.rs +++ b/nexus/tests/integration_tests/saml.rs @@ -32,7 +32,7 @@ async fn test_create_a_saml_idp(cptestctx: &ControlPlaneTestContext) { let client = &cptestctx.external_client; const SILO_NAME: &str = "saml-silo"; - create_silo(&client, SILO_NAME, true, shared::SiloIdentityMode::LocalOnly) + create_silo(&client, SILO_NAME, true, shared::SiloIdentityMode::SamlJit) .await; let silo: Silo = NexusRequest::object_get( &client, @@ -151,7 +151,7 @@ async fn test_create_a_saml_idp_invalid_descriptor_truncated( let client = &cptestctx.external_client; const SILO_NAME: &str = "saml-silo"; - create_silo(&client, SILO_NAME, true, shared::SiloIdentityMode::LocalOnly) + create_silo(&client, SILO_NAME, true, shared::SiloIdentityMode::SamlJit) .await; let saml_idp_descriptor = { @@ -211,7 +211,7 @@ async fn test_create_a_saml_idp_invalid_descriptor_no_redirect_binding( let client = &cptestctx.external_client; const SILO_NAME: &str = "saml-silo"; - create_silo(&client, SILO_NAME, true, shared::SiloIdentityMode::LocalOnly) + create_silo(&client, SILO_NAME, true, shared::SiloIdentityMode::SamlJit) .await; let saml_idp_descriptor = { @@ -281,7 +281,7 @@ async fn test_create_a_hidden_silo_saml_idp( ) { let client = &cptestctx.external_client; - create_silo(&client, "hidden", false, shared::SiloIdentityMode::LocalOnly) + create_silo(&client, "hidden", false, shared::SiloIdentityMode::SamlJit) .await; // Valid IdP descriptor @@ -350,7 +350,7 @@ async fn test_saml_idp_metadata_url_404(cptestctx: &ControlPlaneTestContext) { let client = &cptestctx.external_client; const SILO_NAME: &str = "saml-silo"; - create_silo(&client, SILO_NAME, true, shared::SiloIdentityMode::LocalOnly) + create_silo(&client, SILO_NAME, true, shared::SiloIdentityMode::SamlJit) .await; let server = Server::run(); @@ -404,7 +404,7 @@ async fn test_saml_idp_metadata_url_invalid( let client = &cptestctx.external_client; const SILO_NAME: &str = "saml-silo"; - create_silo(&client, SILO_NAME, true, shared::SiloIdentityMode::LocalOnly) + create_silo(&client, SILO_NAME, true, shared::SiloIdentityMode::SamlJit) .await; NexusRequest::new( @@ -465,7 +465,7 @@ async fn test_saml_idp_reject_keypair(cptestctx: &ControlPlaneTestContext) { ); const SILO_NAME: &str = "saml-silo"; - create_silo(&client, SILO_NAME, true, shared::SiloIdentityMode::LocalOnly) + create_silo(&client, SILO_NAME, true, shared::SiloIdentityMode::SamlJit) .await; let test_cases = vec![ @@ -560,7 +560,7 @@ async fn test_saml_idp_rsa_keypair_ok(cptestctx: &ControlPlaneTestContext) { ); const SILO_NAME: &str = "saml-silo"; - create_silo(&client, SILO_NAME, true, shared::SiloIdentityMode::LocalOnly) + create_silo(&client, SILO_NAME, true, shared::SiloIdentityMode::SamlJit) .await; NexusRequest::new( diff --git a/nexus/tests/integration_tests/silos.rs b/nexus/tests/integration_tests/silos.rs index f270f2685f4..b264d263a66 100644 --- a/nexus/tests/integration_tests/silos.rs +++ b/nexus/tests/integration_tests/silos.rs @@ -303,11 +303,13 @@ async fn test_silo_admin_group(cptestctx: &ControlPlaneTestContext) { #[nexus_test] async fn test_listing_identity_providers(cptestctx: &ControlPlaneTestContext) { let client = &cptestctx.external_client; + create_silo(&client, "test-silo", true, shared::SiloIdentityMode::SamlJit) + .await; // List providers - should be none let providers = objects_list_page_authz::( client, - "/system/silos/default-silo/identity-providers", + "/system/silos/test-silo/identity-providers", ) .await .items; @@ -326,7 +328,7 @@ async fn test_listing_identity_providers(cptestctx: &ControlPlaneTestContext) { let silo_saml_idp_1: SamlIdentityProvider = object_create( client, - &"/system/silos/default-silo/identity-providers/saml", + &"/system/silos/test-silo/identity-providers/saml", ¶ms::SamlIdentityProviderCreate { identity: IdentityMetadataCreateParams { name: "some-totally-real-saml-provider" @@ -355,7 +357,7 @@ async fn test_listing_identity_providers(cptestctx: &ControlPlaneTestContext) { let silo_saml_idp_2: SamlIdentityProvider = object_create( client, - &"/system/silos/default-silo/identity-providers/saml", + &"/system/silos/test-silo/identity-providers/saml", ¶ms::SamlIdentityProviderCreate { identity: IdentityMetadataCreateParams { name: "another-totally-real-saml-provider" @@ -385,7 +387,7 @@ async fn test_listing_identity_providers(cptestctx: &ControlPlaneTestContext) { // List providers again - expect 2 let providers = objects_list_page_authz::( client, - "/system/silos/default-silo/identity-providers", + "/system/silos/test-silo/identity-providers", ) .await .items; @@ -405,7 +407,9 @@ async fn test_deleting_a_silo_deletes_the_idp( ) { let client = &cptestctx.external_client; - const SILO_NAME: &str = "default-silo"; + const SILO_NAME: &str = "test-silo"; + create_silo(&client, SILO_NAME, true, shared::SiloIdentityMode::SamlJit) + .await; let saml_idp_descriptor = SAML_IDP_DESCRIPTOR; @@ -512,7 +516,7 @@ async fn test_saml_idp_metadata_data_valid( ) { let client = &cptestctx.external_client; - create_silo(&client, "blahblah", true, shared::SiloIdentityMode::LocalOnly) + create_silo(&client, "blahblah", true, shared::SiloIdentityMode::SamlJit) .await; let silo_saml_idp: SamlIdentityProvider = object_create( @@ -573,7 +577,7 @@ async fn test_saml_idp_metadata_data_truncated( ) { let client = &cptestctx.external_client; - create_silo(&client, "blahblah", true, shared::SiloIdentityMode::LocalOnly) + create_silo(&client, "blahblah", true, shared::SiloIdentityMode::SamlJit) .await; NexusRequest::new( @@ -626,7 +630,7 @@ async fn test_saml_idp_metadata_data_invalid( let client = &cptestctx.external_client; const SILO_NAME: &str = "saml-silo"; - create_silo(&client, SILO_NAME, true, shared::SiloIdentityMode::LocalOnly) + create_silo(&client, SILO_NAME, true, shared::SiloIdentityMode::SamlJit) .await; NexusRequest::new( @@ -678,14 +682,14 @@ async fn test_silo_user_provision_types(cptestctx: &ControlPlaneTestContext) { let nexus = &cptestctx.server.apictx.nexus; let test_cases: Vec = vec![ - // A silo configured with a "fixed" user provision type should fetch a + // A silo configured with a "ApiOnly" user provision type should fetch a // user if it exists already. TestSiloUserProvisionTypes { identity_mode: shared::SiloIdentityMode::LocalOnly, existing_silo_user: true, expect_user: true, }, - // A silo configured with a "fixed" user provision type should not + // A silo configured with a "ApiOnly" user provision type should not // create a user if one does not exist already. TestSiloUserProvisionTypes { identity_mode: shared::SiloIdentityMode::LocalOnly, @@ -1455,4 +1459,6 @@ async fn test_ensure_same_silo_group(cptestctx: &ControlPlaneTestContext) { .await .expect("silo_user_from_authenticated_subject 2") .unwrap(); + + // TODO-coverage were we intending to verify something here? } diff --git a/nexus/types/src/external_api/shared.rs b/nexus/types/src/external_api/shared.rs index b028fd7c3d3..3a6a9dbd352 100644 --- a/nexus/types/src/external_api/shared.rs +++ b/nexus/types/src/external_api/shared.rs @@ -97,7 +97,8 @@ pub enum IdentityType { pub enum SiloIdentityMode { /// Users are authenticated with SAML using an external authentication /// provider. The system updates information about users and groups only - /// during authentication (i.e,. "JIT provisioning" of users and groups). + /// during successful authentication (i.e,. "JIT provisioning" of users and + /// groups). SamlJit, /// The system is the source of truth about users. There is no linkage to From d321b79b0a614b9069f4b8adade7455b1e43d7a1 Mon Sep 17 00:00:00 2001 From: David Pacheco Date: Fri, 30 Sep 2022 17:05:24 -0700 Subject: [PATCH 6/9] fix test --- nexus/db-macros/src/lookup.rs | 4 ++- nexus/tests/integration_tests/endpoints.rs | 24 ++++++++++++-- nexus/tests/integration_tests/unauthorized.rs | 33 +++++++++++++++---- 3 files changed, 51 insertions(+), 10 deletions(-) diff --git a/nexus/db-macros/src/lookup.rs b/nexus/db-macros/src/lookup.rs index 562bc9d2527..cce460e96b1 100644 --- a/nexus/db-macros/src/lookup.rs +++ b/nexus/db-macros/src/lookup.rs @@ -115,7 +115,9 @@ impl Config { let child_resources = input.children; let parent = input.ancestors.last().map(|s| Resource::for_name(&s)); - let siloed = input.ancestors.iter().any(|s| s == "Silo"); + // XXX-dap + let siloed = resource.name != "SamlIdentityProvider" && + input.ancestors.iter().any(|s| s == "Silo"); Config { resource, diff --git a/nexus/tests/integration_tests/endpoints.rs b/nexus/tests/integration_tests/endpoints.rs index e2f48aa1b84..7ac08a56e94 100644 --- a/nexus/tests/integration_tests/endpoints.rs +++ b/nexus/tests/integration_tests/endpoints.rs @@ -27,6 +27,7 @@ use omicron_common::api::external::RouterRouteUpdateParams; use omicron_common::api::external::VpcFirewallRuleUpdateParams; use omicron_nexus::authn; use omicron_nexus::authz; +use omicron_nexus::db::identity::Asset; use omicron_nexus::external_api::params; use omicron_nexus::external_api::shared; use omicron_nexus::external_api::shared::IpRange; @@ -59,6 +60,16 @@ lazy_static! { identity_mode: shared::SiloIdentityMode::SamlJit, admin_group_name: None, }; + pub static ref DEMO_SILO_POLICY: shared::Policy = + shared::Policy { + role_assignments: vec![ + shared::RoleAssignment { + identity_type: shared::IdentityType::SiloUser, + identity_id: authn::USER_TEST_PRIVILEGED.id(), + role_name: authz::SiloRole::Admin, + }, + ] + }; // Organization used for testing pub static ref DEMO_ORG_NAME: Name = "demo-org".parse().unwrap(); @@ -360,8 +371,8 @@ lazy_static! { lazy_static! { // Identity providers - pub static ref IDENTITY_PROVIDERS_URL: String = format!("/system/silos/default-silo/identity-providers"); - pub static ref SAML_IDENTITY_PROVIDERS_URL: String = format!("/system/silos/default-silo/identity-providers/saml"); + pub static ref IDENTITY_PROVIDERS_URL: String = format!("/system/silos/demo-silo/identity-providers"); + pub static ref SAML_IDENTITY_PROVIDERS_URL: String = format!("/system/silos/demo-silo/identity-providers/saml"); pub static ref DEMO_SAML_IDENTITY_PROVIDER_NAME: Name = "demo-saml-provider".parse().unwrap(); pub static ref SPECIFIC_SAML_IDENTITY_PROVIDER_URL: String = format!("{}/{}", *SAML_IDENTITY_PROVIDERS_URL, *DEMO_SAML_IDENTITY_PROVIDER_NAME); @@ -1438,7 +1449,14 @@ lazy_static! { VerifyEndpoint { url: &*SAML_IDENTITY_PROVIDERS_URL, - visibility: Visibility::Public, + // The visibility here deserves some explanation. In order to + // create a real SAML identity provider for doing tests, we have to + // do it in a non-default Silo (because the default one does not + // support creating a SAML identity provider). But unprivileged + // users won't be able to see that Silo. So from their perspective, + // it's like an object in a container they can't see (which is what + // Visibility::Protected means). + visibility: Visibility::Protected, unprivileged_access: UnprivilegedAccess::None, allowed_methods: vec![AllowedMethod::Post( serde_json::to_value(&*SAML_IDENTITY_PROVIDER).unwrap(), diff --git a/nexus/tests/integration_tests/unauthorized.rs b/nexus/tests/integration_tests/unauthorized.rs index cc77cc1e76a..c7329a914d8 100644 --- a/nexus/tests/integration_tests/unauthorized.rs +++ b/nexus/tests/integration_tests/unauthorized.rs @@ -80,6 +80,15 @@ async fn test_unauthorized(cptestctx: &ControlPlaneTestContext) { .unwrap(), id_routes, ), + SetupReq::Put { url, body, id_routes } => ( + url, + NexusRequest::object_put(client, url, Some(body)) + .authn_as(AuthnMode::PrivilegedUser) + .execute() + .await + .unwrap(), + id_routes, + ), }; setup_results.insert(url, result.clone()); @@ -137,11 +146,11 @@ G GET PUT POST DEL TRCE G URL /// Describes a request made during the setup phase to create a resource that /// we'll use later in the verification phase /// -/// The setup phase takes a list of `SetupReq` enums and issues a `GET` or `POST` -/// request to each one's `url`. `id_results` is a list of URLs that are associated -/// to the results of the setup request with any `{id}` params in the URL replaced with -/// the result's URL. This is used to later verify ID endpoints without first having to -/// know the ID. +/// The setup phase takes a list of `SetupReq` enums and issues a `GET` or +/// `POST` request to each one's `url`. `id_results` is a list of URLs that are +/// associated to the results of the setup request with any `{id}` params in the +/// URL replaced with the result's URL. This is used to later verify ID +/// endpoints without first having to know the ID. enum SetupReq { Get { @@ -153,6 +162,11 @@ enum SetupReq { body: serde_json::Value, id_routes: Vec<&'static str>, }, + Put { + url: &'static str, + body: serde_json::Value, + id_routes: Vec<&'static str>, + }, } lazy_static! { @@ -183,12 +197,19 @@ lazy_static! { /// List of requests to execute at setup time static ref SETUP_REQUESTS: Vec = vec![ - // Create a separate Silo (not used for anything else) + // Create a separate Silo SetupReq::Post { url: "/system/silos", body: serde_json::to_value(&*DEMO_SILO_CREATE).unwrap(), id_routes: vec!["/system/by-id/silos/{id}"], }, + // Grant the privileged test user admin privileges on the demo Silo so + // that it can see the identity providers. + SetupReq::Put { + url: &*DEMO_SILO_POLICY_URL, + body: serde_json::to_value(&*DEMO_SILO_POLICY).unwrap(), + id_routes: vec![], + }, // Create an IP pool SetupReq::Post { url: &*DEMO_IP_POOLS_URL, From dcdd5d5e834c8bd49e8c0d0c676d3a9ccc973948 Mon Sep 17 00:00:00 2001 From: David Pacheco Date: Fri, 30 Sep 2022 18:38:29 -0700 Subject: [PATCH 7/9] rustfmt --- nexus/db-macros/src/lookup.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/nexus/db-macros/src/lookup.rs b/nexus/db-macros/src/lookup.rs index cce460e96b1..cf66c76df51 100644 --- a/nexus/db-macros/src/lookup.rs +++ b/nexus/db-macros/src/lookup.rs @@ -116,8 +116,8 @@ impl Config { let child_resources = input.children; let parent = input.ancestors.last().map(|s| Resource::for_name(&s)); // XXX-dap - let siloed = resource.name != "SamlIdentityProvider" && - input.ancestors.iter().any(|s| s == "Silo"); + let siloed = resource.name != "SamlIdentityProvider" + && input.ancestors.iter().any(|s| s == "Silo"); Config { resource, From cf2ece100ff757409c4521c89d757f3316023f08 Mon Sep 17 00:00:00 2001 From: David Pacheco Date: Mon, 3 Oct 2022 13:06:13 -0700 Subject: [PATCH 8/9] replace hardcoded behavior with configurable --- nexus/db-macros/src/lookup.rs | 22 +++++++++++++++------- nexus/src/db/lookup.rs | 3 ++- 2 files changed, 17 insertions(+), 8 deletions(-) diff --git a/nexus/db-macros/src/lookup.rs b/nexus/db-macros/src/lookup.rs index cf66c76df51..1c58d2153b0 100644 --- a/nexus/db-macros/src/lookup.rs +++ b/nexus/db-macros/src/lookup.rs @@ -42,6 +42,14 @@ pub struct Input { primary_key_columns: Vec, /// This resources supports soft-deletes soft_deletes: bool, + /// This resource appears under the `Silo` hierarchy, but nevertheless + /// should be visible to users in other Silos + /// + /// This is "false" by default. If you don't specify this, + /// `lookup_resource!` determines whether something should be visible based + /// on whether it's nested under a `Silo`. + #[serde(default)] + visible_outside_silo: bool, } #[derive(serde::Deserialize)] @@ -67,8 +75,9 @@ pub struct Config { /// This resources supports soft-deletes soft_deletes: bool, - /// This resource is nested under the Silo hierarchy - siloed: bool, + /// This resource is inside a Silo and only visible to users in the same + /// Silo + silo_restricted: bool, // The path to the resource /// list of type names for this resource and its parents @@ -115,13 +124,12 @@ impl Config { let child_resources = input.children; let parent = input.ancestors.last().map(|s| Resource::for_name(&s)); - // XXX-dap - let siloed = resource.name != "SamlIdentityProvider" + let silo_restricted = !input.visible_outside_silo && input.ancestors.iter().any(|s| s == "Silo"); Config { resource, - siloed, + silo_restricted, path_types, path_authz_names, parent, @@ -291,7 +299,7 @@ fn generate_misc_helpers(config: &Config) -> TokenStream { }; let resource_authz_name = &config.resource.authz_name; - let silo_check_fn = if config.siloed { + let silo_check_fn = if config.silo_restricted { quote! { /// For a "siloed" resource (i.e., one that's nested under "Silo" in /// the resource hierarchy), check whether a given resource's Silo @@ -479,7 +487,7 @@ fn generate_lookup_methods(config: &Config) -> TokenStream { // If this resource is "Siloed", then tack on an extra check that the // resource's Silo matches the Silo of the actor doing the fetch/lookup. // See the generation of `silo_check()` for details. - let (silo_check_lookup, silo_check_fetch) = if config.siloed { + let (silo_check_lookup, silo_check_fetch) = if config.silo_restricted { ( quote! { .and_then(|input| { diff --git a/nexus/src/db/lookup.rs b/nexus/src/db/lookup.rs index b7349148aeb..957600384d0 100644 --- a/nexus/src/db/lookup.rs +++ b/nexus/src/db/lookup.rs @@ -495,7 +495,8 @@ lookup_resource! { soft_deletes = true, primary_key_columns = [ { column_name = "id", rust_type = Uuid }, - ] + ], + visible_outside_silo = true } lookup_resource! { From 84ad026e0a3c8f17a42c63b043b0efb65ad7f361 Mon Sep 17 00:00:00 2001 From: David Pacheco Date: Mon, 3 Oct 2022 13:27:08 -0700 Subject: [PATCH 9/9] Fleet privileges should be enough to see IdPs --- nexus/src/authz/omicron.polar | 30 ++++++++++++++----- nexus/tests/integration_tests/endpoints.rs | 11 ------- nexus/tests/integration_tests/unauthorized.rs | 21 ------------- 3 files changed, 22 insertions(+), 40 deletions(-) diff --git a/nexus/src/authz/omicron.polar b/nexus/src/authz/omicron.polar index 721d00af654..d01387e9cbd 100644 --- a/nexus/src/authz/omicron.polar +++ b/nexus/src/authz/omicron.polar @@ -286,17 +286,24 @@ resource IdentityProvider { "create_child", "list_children", ]; - relations = { parent_silo: Silo }; + relations = { parent_silo: Silo, parent_fleet: Fleet }; + # Silo-level roles grant privileges on identity providers. "read" if "viewer" on "parent_silo"; "list_children" if "viewer" on "parent_silo"; - - # Only silo admins can create silo identity providers "modify" if "admin" on "parent_silo"; "create_child" if "admin" on "parent_silo"; + + # Fleet-level roles also grant privileges on identity providers. + "read" if "viewer" on "parent_fleet"; + "list_children" if "viewer" on "parent_fleet"; + "modify" if "admin" on "parent_fleet"; + "create_child" if "admin" on "parent_fleet"; } has_relation(silo: Silo, "parent_silo", identity_provider: IdentityProvider) if identity_provider.silo = silo; +has_relation(fleet: Fleet, "parent_fleet", collection: IdentityProvider) + if collection.silo.fleet = fleet; resource SamlIdentityProvider { permissions = [ @@ -305,17 +312,24 @@ resource SamlIdentityProvider { "create_child", "list_children", ]; - relations = { parent_silo: Silo }; - - # Only silo admins have permissions for specific identity provider details - "read" if "admin" on "parent_silo"; - "list_children" if "admin" on "parent_silo"; + relations = { parent_silo: Silo, parent_fleet: Fleet }; + # Silo-level roles grant privileges on identity providers. + "read" if "viewer" on "parent_silo"; + "list_children" if "viewer" on "parent_silo"; "modify" if "admin" on "parent_silo"; "create_child" if "admin" on "parent_silo"; + + # Fleet-level roles also grant privileges on identity providers. + "read" if "viewer" on "parent_fleet"; + "list_children" if "viewer" on "parent_fleet"; + "modify" if "admin" on "parent_fleet"; + "create_child" if "admin" on "parent_fleet"; } has_relation(silo: Silo, "parent_silo", saml_identity_provider: SamlIdentityProvider) if saml_identity_provider.silo = silo; +has_relation(fleet: Fleet, "parent_fleet", collection: SamlIdentityProvider) + if collection.silo.fleet = fleet; # # SYNTHETIC RESOURCES OUTSIDE THE SILO HIERARCHY diff --git a/nexus/tests/integration_tests/endpoints.rs b/nexus/tests/integration_tests/endpoints.rs index 7ac08a56e94..02f2ecedf71 100644 --- a/nexus/tests/integration_tests/endpoints.rs +++ b/nexus/tests/integration_tests/endpoints.rs @@ -27,7 +27,6 @@ use omicron_common::api::external::RouterRouteUpdateParams; use omicron_common::api::external::VpcFirewallRuleUpdateParams; use omicron_nexus::authn; use omicron_nexus::authz; -use omicron_nexus::db::identity::Asset; use omicron_nexus::external_api::params; use omicron_nexus::external_api::shared; use omicron_nexus::external_api::shared::IpRange; @@ -60,16 +59,6 @@ lazy_static! { identity_mode: shared::SiloIdentityMode::SamlJit, admin_group_name: None, }; - pub static ref DEMO_SILO_POLICY: shared::Policy = - shared::Policy { - role_assignments: vec![ - shared::RoleAssignment { - identity_type: shared::IdentityType::SiloUser, - identity_id: authn::USER_TEST_PRIVILEGED.id(), - role_name: authz::SiloRole::Admin, - }, - ] - }; // Organization used for testing pub static ref DEMO_ORG_NAME: Name = "demo-org".parse().unwrap(); diff --git a/nexus/tests/integration_tests/unauthorized.rs b/nexus/tests/integration_tests/unauthorized.rs index c7329a914d8..c807efad2e9 100644 --- a/nexus/tests/integration_tests/unauthorized.rs +++ b/nexus/tests/integration_tests/unauthorized.rs @@ -80,15 +80,6 @@ async fn test_unauthorized(cptestctx: &ControlPlaneTestContext) { .unwrap(), id_routes, ), - SetupReq::Put { url, body, id_routes } => ( - url, - NexusRequest::object_put(client, url, Some(body)) - .authn_as(AuthnMode::PrivilegedUser) - .execute() - .await - .unwrap(), - id_routes, - ), }; setup_results.insert(url, result.clone()); @@ -162,11 +153,6 @@ enum SetupReq { body: serde_json::Value, id_routes: Vec<&'static str>, }, - Put { - url: &'static str, - body: serde_json::Value, - id_routes: Vec<&'static str>, - }, } lazy_static! { @@ -203,13 +189,6 @@ lazy_static! { body: serde_json::to_value(&*DEMO_SILO_CREATE).unwrap(), id_routes: vec!["/system/by-id/silos/{id}"], }, - // Grant the privileged test user admin privileges on the demo Silo so - // that it can see the identity providers. - SetupReq::Put { - url: &*DEMO_SILO_POLICY_URL, - body: serde_json::to_value(&*DEMO_SILO_POLICY).unwrap(), - id_routes: vec![], - }, // Create an IP pool SetupReq::Post { url: &*DEMO_IP_POOLS_URL,