diff --git a/common/src/api/external/mod.rs b/common/src/api/external/mod.rs index f4be124e4db..592e5946b42 100644 --- a/common/src/api/external/mod.rs +++ b/common/src/api/external/mod.rs @@ -43,6 +43,7 @@ use std::fmt::Result as FormatResult; use std::net::IpAddr; use std::net::Ipv4Addr; use std::num::{NonZeroU16, NonZeroU32}; +use std::ops::Deref; use std::str::FromStr; use tufaceous_artifact::ArtifactHash; use uuid::Uuid; @@ -1008,6 +1009,7 @@ pub enum ResourceType { ProjectImage, Instance, LoopbackAddress, + SiloAuthSettings, SwitchPortSettings, SupportBundle, IpPool, @@ -3277,6 +3279,68 @@ pub enum ImportExportPolicy { Allow(Vec), } +/// Use instead of Option in API request body structs to get a field that can +/// be null (parsed as `None`) but is not optional. Unlike Option, Nullable +/// will fail to parse if the key is not present. The JSON Schema in the +/// OpenAPI definition will also reflect that the field is required. See +/// . +#[derive(Clone, Debug, Serialize)] +pub struct Nullable(pub Option); + +impl From> for Nullable { + fn from(option: Option) -> Self { + Nullable(option) + } +} + +impl Deref for Nullable { + type Target = Option; + + fn deref(&self) -> &Self::Target { + &self.0 + } +} + +// it looks like we're just using Option's impl here, so why not derive instead? +// For some reason, deriving JsonSchema + #[serde(transparent)] doesn't work -- +// it almost does, but the field does not end up marked required in the schema. +// There must be some special handling of Option somewhere causing it to be +// marked optional rather than nullable + required. + +impl JsonSchema for Nullable { + fn schema_name() -> String { + T::schema_name() + } + + fn json_schema( + generator: &mut schemars::r#gen::SchemaGenerator, + ) -> schemars::schema::Schema { + Option::::json_schema(generator) + } + + fn is_referenceable() -> bool { + Option::::is_referenceable() + } +} + +impl<'de, T: serde::Deserialize<'de>> serde::Deserialize<'de> for Nullable { + fn deserialize>( + deserializer: D, + ) -> Result { + // This line is required to get a parse error on missing fields. + // It seems that when the field is missing in the JSON, struct + // deserialization produces an error before this function is even hit, + // and that error is passed in here inside `deserializer`. If we don't + // do this Value::deserialize to cause that error to be returned as a + // missing field error, Option's deserialize will eat it by turning it + // into a successful parse as None. + let value = serde_json::Value::deserialize(deserializer)?; + + use serde::de::Error; + Option::::deserialize(value).map_err(D::Error::custom).map(Nullable) + } +} + #[cfg(test)] mod test { use serde::Deserialize; diff --git a/nexus/db-model/src/device_auth.rs b/nexus/db-model/src/device_auth.rs index 2cca57cda49..6fc3264f590 100644 --- a/nexus/db-model/src/device_auth.rs +++ b/nexus/db-model/src/device_auth.rs @@ -136,9 +136,12 @@ impl DeviceAccessToken { device_code: String, time_requested: DateTime, silo_user_id: Uuid, + time_expires: Option>, ) -> Self { let now = Utc::now(); assert!(time_requested <= now); + assert!(time_expires.map_or(true, |t| t > now)); + Self { id: TypedUuid::new_v4().into(), token: generate_token(), @@ -147,7 +150,7 @@ impl DeviceAccessToken { silo_user_id, time_requested, time_created: now, - time_expires: None, + time_expires, } } diff --git a/nexus/db-model/src/lib.rs b/nexus/db-model/src/lib.rs index e58783512ec..e0a0910f2aa 100644 --- a/nexus/db-model/src/lib.rs +++ b/nexus/db-model/src/lib.rs @@ -69,6 +69,7 @@ mod project; mod rendezvous_debug_dataset; mod semver_version; mod serde_time_delta; +mod silo_auth_settings; mod switch_interface; mod switch_port; mod target_release; @@ -214,6 +215,7 @@ pub use schema_versions::*; pub use semver_version::*; pub use service_kind::*; pub use silo::*; +pub use silo_auth_settings::*; pub use silo_group::*; pub use silo_user::*; pub use silo_user_password_hash::*; diff --git a/nexus/db-model/src/schema_versions.rs b/nexus/db-model/src/schema_versions.rs index 6f85bc0b6d7..73d946bfeb1 100644 --- a/nexus/db-model/src/schema_versions.rs +++ b/nexus/db-model/src/schema_versions.rs @@ -16,7 +16,7 @@ use std::{collections::BTreeMap, sync::LazyLock}; /// /// This must be updated when you change the database schema. Refer to /// schema/crdb/README.adoc in the root of this repository for details. -pub const SCHEMA_VERSION: Version = Version::new(145, 0, 0); +pub const SCHEMA_VERSION: Version = Version::new(146, 0, 0); /// List of all past database schema versions, in *reverse* order /// @@ -28,6 +28,7 @@ static KNOWN_VERSIONS: LazyLock> = LazyLock::new(|| { // | leaving the first copy as an example for the next person. // v // KnownVersion::new(next_int, "unique-dirname-with-the-sql-files"), + KnownVersion::new(146, "silo-settings-token-expiration"), KnownVersion::new(145, "token-and-session-ids"), KnownVersion::new(144, "inventory-omicron-sled-config"), KnownVersion::new(143, "alerts-renamening"), diff --git a/nexus/db-model/src/silo_auth_settings.rs b/nexus/db-model/src/silo_auth_settings.rs new file mode 100644 index 00000000000..7fce1be0646 --- /dev/null +++ b/nexus/db-model/src/silo_auth_settings.rs @@ -0,0 +1,70 @@ +use crate::SqlU32; +use chrono::{DateTime, Utc}; +use nexus_db_schema::schema::silo_auth_settings; +use nexus_types::external_api::{params, views}; +use serde::{Deserialize, Serialize}; +use uuid::Uuid; + +#[derive( + Queryable, + Insertable, + Debug, + Clone, + Selectable, + Serialize, + Deserialize, + AsChangeset, +)] +#[diesel(table_name = silo_auth_settings)] +pub struct SiloAuthSettings { + pub silo_id: Uuid, + pub time_created: DateTime, + pub time_modified: DateTime, + + /// Max token lifetime in seconds. Null means no max: users can create + /// tokens that never expire. + pub device_token_max_ttl_seconds: Option, +} + +impl SiloAuthSettings { + pub fn new(silo_id: Uuid) -> Self { + Self { + silo_id, + time_created: Utc::now(), + time_modified: Utc::now(), + device_token_max_ttl_seconds: None, + } + } +} + +impl From for views::SiloAuthSettings { + fn from(silo_auth_settings: SiloAuthSettings) -> Self { + Self { + silo_id: silo_auth_settings.silo_id, + device_token_max_ttl_seconds: silo_auth_settings + .device_token_max_ttl_seconds + .map(|ttl| ttl.0), + } + } +} + +// Describes a set of updates for the [`SiloAuthSettings`] model. +#[derive(AsChangeset)] +#[diesel(table_name = silo_auth_settings)] +pub struct SiloAuthSettingsUpdate { + // Needs to be double Option so we can set a value of null in the DB by + // passing Some(None). None by itself is ignored by Diesel. + pub device_token_max_ttl_seconds: Option>, + pub time_modified: DateTime, +} + +impl From for SiloAuthSettingsUpdate { + fn from(params: params::SiloAuthSettingsUpdate) -> Self { + Self { + device_token_max_ttl_seconds: Some( + params.device_token_max_ttl_seconds.map(|ttl| ttl.get().into()), + ), + time_modified: Utc::now(), + } + } +} diff --git a/nexus/db-queries/src/db/datastore/mod.rs b/nexus/db-queries/src/db/datastore/mod.rs index 7ae76115e84..87efcc3f6fc 100644 --- a/nexus/db-queries/src/db/datastore/mod.rs +++ b/nexus/db-queries/src/db/datastore/mod.rs @@ -90,6 +90,7 @@ mod rendezvous_debug_dataset; mod role; mod saga; mod silo; +mod silo_auth_settings; mod silo_group; mod silo_user; mod sled; diff --git a/nexus/db-queries/src/db/datastore/silo.rs b/nexus/db-queries/src/db/datastore/silo.rs index 514b91db85d..f5c10930af8 100644 --- a/nexus/db-queries/src/db/datastore/silo.rs +++ b/nexus/db-queries/src/db/datastore/silo.rs @@ -30,6 +30,7 @@ use nexus_db_fixed_data::silo::{DEFAULT_SILO, INTERNAL_SILO}; use nexus_db_lookup::DbConnection; use nexus_db_model::Certificate; use nexus_db_model::ServiceKind; +use nexus_db_model::SiloAuthSettings; use nexus_db_model::SiloQuotas; use nexus_types::external_api::params; use nexus_types::external_api::shared; @@ -64,24 +65,31 @@ impl DataStore { debug!(opctx.log, "attempting to create built-in silos"); - use nexus_db_schema::schema::silo::dsl; - use nexus_db_schema::schema::silo_quotas::dsl as quotas_dsl; + use nexus_db_schema::schema::silo; + use nexus_db_schema::schema::silo_auth_settings; + use nexus_db_schema::schema::silo_quotas; let conn = self.pool_connection_authorized(opctx).await?; let count = self .transaction_retry_wrapper("load_builtin_silos") .transaction(&conn, |conn| async move { - diesel::insert_into(quotas_dsl::silo_quotas) + diesel::insert_into(silo_quotas::table) .values(SiloQuotas::arbitrarily_high_default( DEFAULT_SILO.id(), )) - .on_conflict(quotas_dsl::silo_id) + .on_conflict(silo_quotas::silo_id) + .do_nothing() + .execute_async(&conn) + .await?; + diesel::insert_into(silo_auth_settings::table) + .values(SiloAuthSettings::new(DEFAULT_SILO.id())) + .on_conflict(silo_auth_settings::silo_id) .do_nothing() .execute_async(&conn) .await?; - let count = diesel::insert_into(dsl::silo) + let count = diesel::insert_into(silo::table) .values([&*DEFAULT_SILO, &*INTERNAL_SILO]) - .on_conflict(dsl::id) + .on_conflict(silo::id) .do_nothing() .execute_async(&conn) .await?; @@ -300,6 +308,12 @@ impl DataStore { ), ) .await?; + self.silo_auth_settings_create( + &conn, + &authz_silo, + SiloAuthSettings::new(authz_silo.id()), + ) + .await?; Ok::>(silo) }) @@ -451,6 +465,8 @@ impl DataStore { } self.silo_quotas_delete(opctx, &conn, &authz_silo).await?; + self.silo_auth_settings_delete(opctx, &conn, &authz_silo) + .await?; self.virtual_provisioning_collection_delete_on_connection( &opctx.log, &conn, id, diff --git a/nexus/db-queries/src/db/datastore/silo_auth_settings.rs b/nexus/db-queries/src/db/datastore/silo_auth_settings.rs new file mode 100644 index 00000000000..d4b3f431d04 --- /dev/null +++ b/nexus/db-queries/src/db/datastore/silo_auth_settings.rs @@ -0,0 +1,114 @@ +use super::DataStore; +use crate::authz; +use crate::context::OpContext; +use async_bb8_diesel::AsyncRunQueryDsl; +use diesel::prelude::*; +use nexus_db_errors::ErrorHandler; +use nexus_db_errors::public_error_from_diesel; +use nexus_db_lookup::DbConnection; +use nexus_db_model::SiloAuthSettings; +use nexus_db_model::SiloAuthSettingsUpdate; +use omicron_common::api::external::DeleteResult; +use omicron_common::api::external::Error; +use omicron_common::api::external::ResourceType; +use omicron_common::api::external::UpdateResult; + +// Directly modeled on settings query functions. + +impl DataStore { + /// Creates new settings for a silo. This is grouped with silo creation + /// and shouldn't be called outside of that flow. + /// + /// An authz check _cannot_ be performed here because the authz initialization + /// isn't complete and will lead to a db deadlock. + /// + /// See + pub async fn silo_auth_settings_create( + &self, + conn: &async_bb8_diesel::Connection, + authz_silo: &authz::Silo, + settings: SiloAuthSettings, + ) -> Result<(), Error> { + let silo_id = authz_silo.id(); + use nexus_db_schema::schema::silo_auth_settings; + + diesel::insert_into(silo_auth_settings::table) + .values(settings) + .execute_async(conn) + .await + .map_err(|e| { + public_error_from_diesel( + e, + ErrorHandler::Conflict( + ResourceType::SiloAuthSettings, + &silo_id.to_string(), + ), + ) + }) + .map(|_| ()) + } + + pub(crate) async fn silo_auth_settings_delete( + &self, + opctx: &OpContext, + conn: &async_bb8_diesel::Connection, + authz_silo: &authz::Silo, + ) -> DeleteResult { + // Given that the settings right now are somewhat of an extension of the + // Silo we just check for delete permission on the silo itself. + opctx.authorize(authz::Action::Delete, authz_silo).await?; + + use nexus_db_schema::schema::silo_auth_settings; + diesel::delete(silo_auth_settings::table) + .filter(silo_auth_settings::silo_id.eq(authz_silo.id())) + .execute_async(conn) + .await + .map_err(|e| public_error_from_diesel(e, ErrorHandler::Server))?; + + Ok(()) + } + + pub async fn silo_auth_settings_update( + &self, + opctx: &OpContext, + authz_silo: &authz::Silo, + updates: SiloAuthSettingsUpdate, + ) -> UpdateResult { + opctx.authorize(authz::Action::Modify, authz_silo).await?; + use nexus_db_schema::schema::silo_auth_settings::dsl; + let silo_id = authz_silo.id(); + diesel::update(dsl::silo_auth_settings) + .filter(dsl::silo_id.eq(silo_id)) + .set(updates) + .returning(SiloAuthSettings::as_returning()) + .get_result_async(&*self.pool_connection_authorized(opctx).await?) + .await + .map_err(|e| { + public_error_from_diesel( + e, + ErrorHandler::Conflict( + ResourceType::SiloAuthSettings, + &silo_id.to_string(), + ), + ) + }) + } + + pub async fn silo_auth_settings_view( + &self, + opctx: &OpContext, + authz_silo: &authz::Silo, + ) -> Result { + // Works for everyone when making a token because everyone can read + // their own silo. Operators looking at silo settings will have silo + // read on all silos. + opctx.authorize(authz::Action::Read, authz_silo).await?; + + use nexus_db_schema::schema::silo_auth_settings::dsl; + dsl::silo_auth_settings + .filter(dsl::silo_id.eq(authz_silo.id())) + .first_async(&*self.pool_connection_authorized(opctx).await?) + .await + .map_err(|e| public_error_from_diesel(e, ErrorHandler::Server)) + } +} diff --git a/nexus/db-schema/src/schema.rs b/nexus/db-schema/src/schema.rs index 7a59ba5b9b8..8312f085850 100644 --- a/nexus/db-schema/src/schema.rs +++ b/nexus/db-schema/src/schema.rs @@ -551,6 +551,15 @@ table! { } } +table! { + silo_auth_settings(silo_id) { + silo_id -> Uuid, + time_created -> Timestamptz, + time_modified -> Timestamptz, + device_token_max_ttl_seconds -> Nullable, + } +} + table! { network_interface (id) { id -> Uuid, diff --git a/nexus/external-api/output/nexus_tags.txt b/nexus/external-api/output/nexus_tags.txt index 60545e1d308..4f381cd47a0 100644 --- a/nexus/external-api/output/nexus_tags.txt +++ b/nexus/external-api/output/nexus_tags.txt @@ -142,6 +142,8 @@ current_user_view GET /v1/me API operations found with tag "silos" OPERATION ID METHOD URL PATH +auth_settings_update PUT /v1/auth-settings +auth_settings_view GET /v1/auth-settings certificate_create POST /v1/certificates certificate_delete DELETE /v1/certificates/{certificate} certificate_list GET /v1/certificates diff --git a/nexus/external-api/src/lib.rs b/nexus/external-api/src/lib.rs index 710ecc25a40..ebfdaeaef35 100644 --- a/nexus/external-api/src/lib.rs +++ b/nexus/external-api/src/lib.rs @@ -278,6 +278,27 @@ pub trait NexusExternalApi { new_policy: TypedBody>, ) -> Result>, HttpError>; + /// Fetch current silo's auth settings + #[endpoint { + method = GET, + path = "/v1/auth-settings", + tags = ["silos"], + }] + async fn auth_settings_view( + rqctx: RequestContext, + ) -> Result, HttpError>; + + /// Update current silo's auth settings + #[endpoint { + method = PUT, + path = "/v1/auth-settings", + tags = ["silos"], + }] + async fn auth_settings_update( + rqctx: RequestContext, + new_settings: TypedBody, + ) -> Result, HttpError>; + /// Fetch resource utilization for user's current silo #[endpoint { method = GET, diff --git a/nexus/src/app/device_auth.rs b/nexus/src/app/device_auth.rs index e4a964de491..e796333cfd3 100644 --- a/nexus/src/app/device_auth.rs +++ b/nexus/src/app/device_auth.rs @@ -52,11 +52,12 @@ use nexus_db_queries::authz; use nexus_db_queries::context::OpContext; use nexus_db_queries::db::model::{DeviceAccessToken, DeviceAuthRequest}; +use anyhow::anyhow; use nexus_types::external_api::params::DeviceAccessTokenRequest; use nexus_types::external_api::views; use omicron_common::api::external::{CreateResult, Error}; -use chrono::Utc; +use chrono::{Duration, Utc}; use serde::Serialize; use uuid::Uuid; @@ -100,18 +101,29 @@ impl super::Nexus { .fetch() .await?; - let (.., authz_user) = LookupPath::new(opctx, &self.db_datastore) - .silo_user_id(silo_user_id) - .lookup_for(authz::Action::CreateChild) - .await?; + let (authz_silo, authz_user) = + LookupPath::new(opctx, &self.db_datastore) + .silo_user_id(silo_user_id) + .lookup_for(authz::Action::CreateChild) + .await?; assert_eq!(authz_user.id(), silo_user_id); + let silo_auth_settings = self + .db_datastore + .silo_auth_settings_view(opctx, &authz_silo) + .await?; + // Create an access token record. let token = DeviceAccessToken::new( db_request.client_id, db_request.device_code, db_request.time_created, silo_user_id, + // Token gets the max TTL for the silo (if there is one) until we + // build a way for the user to ask for a different TTL + silo_auth_settings + .device_token_max_ttl_seconds + .map(|ttl| Utc::now() + Duration::seconds(ttl.0.into())), ); if db_request.time_expires < Utc::now() { @@ -190,6 +202,20 @@ impl super::Nexus { })?; let silo_id = db_silo_user.silo_id; + if let Some(time_expires) = db_access_token.time_expires { + let now = Utc::now(); + if time_expires < now { + return Err(Reason::BadCredentials { + actor: Actor::SiloUser { silo_user_id, silo_id }, + source: anyhow!( + "token expired at {} (current time: {})", + time_expires, + now + ), + }); + } + } + Ok(Actor::SiloUser { silo_user_id, silo_id }) } diff --git a/nexus/src/app/silo.rs b/nexus/src/app/silo.rs index 09995c73da4..e41afde1ba7 100644 --- a/nexus/src/app/silo.rs +++ b/nexus/src/app/silo.rs @@ -9,6 +9,7 @@ use crate::external_api::shared; use anyhow::Context; use nexus_db_lookup::LookupPath; use nexus_db_lookup::lookup; +use nexus_db_model::SiloAuthSettings; use nexus_db_model::{DnsGroup, UserProvisionType}; use nexus_db_queries::authz::ApiResource; use nexus_db_queries::context::OpContext; @@ -225,6 +226,35 @@ impl super::Nexus { Ok(shared::Policy { role_assignments }) } + pub(crate) async fn silo_fetch_auth_settings( + &self, + opctx: &OpContext, + silo_lookup: &lookup::Silo<'_>, + ) -> LookupResult { + let (.., authz_silo) = + silo_lookup.lookup_for(authz::Action::Read).await?; + self.db_datastore.silo_auth_settings_view(opctx, &authz_silo).await + } + + pub(crate) async fn silo_update_auth_settings( + &self, + opctx: &OpContext, + silo_lookup: &lookup::Silo<'_>, + settings: ¶ms::SiloAuthSettingsUpdate, + ) -> UpdateResult { + // TODO: modify seems fine, but look into why policy has its own + // separate permission + let (.., authz_silo) = + silo_lookup.lookup_for(authz::Action::Modify).await?; + self.db_datastore + .silo_auth_settings_update( + opctx, + &authz_silo, + settings.clone().into(), + ) + .await + } + // Users /// Helper function for looking up a user in a Silo diff --git a/nexus/src/external_api/http_entrypoints.rs b/nexus/src/external_api/http_entrypoints.rs index 86b11a251b1..5fb6f0ec805 100644 --- a/nexus/src/external_api/http_entrypoints.rs +++ b/nexus/src/external_api/http_entrypoints.rs @@ -229,6 +229,62 @@ impl NexusExternalApi for NexusExternalApiImpl { .await } + async fn auth_settings_view( + rqctx: RequestContext, + ) -> Result, HttpError> { + let apictx = rqctx.context(); + let handler = async { + let nexus = &apictx.context.nexus; + let opctx = + crate::context::op_context_for_external_api(&rqctx).await?; + let silo: NameOrId = opctx + .authn + .silo_required() + .internal_context("loading current silo")? + .id() + .into(); + + let silo_lookup = nexus.silo_lookup(&opctx, silo)?; + let settings = + nexus.silo_fetch_auth_settings(&opctx, &silo_lookup).await?; + Ok(HttpResponseOk(settings.into())) + }; + apictx + .context + .external_latencies + .instrument_dropshot_handler(&rqctx, handler) + .await + } + + async fn auth_settings_update( + rqctx: RequestContext, + new_settings: TypedBody, + ) -> Result, HttpError> { + let apictx = rqctx.context(); + let handler = async { + let nexus = &apictx.context.nexus; + let new_settings = new_settings.into_inner(); + let opctx = + crate::context::op_context_for_external_api(&rqctx).await?; + let silo: NameOrId = opctx + .authn + .silo_required() + .internal_context("loading current silo")? + .id() + .into(); + let silo_lookup = nexus.silo_lookup(&opctx, silo)?; + let settings = nexus + .silo_update_auth_settings(&opctx, &silo_lookup, &new_settings) + .await?; + Ok(HttpResponseOk(settings.into())) + }; + apictx + .context + .external_latencies + .instrument_dropshot_handler(&rqctx, handler) + .await + } + async fn utilization_view( rqctx: RequestContext, ) -> Result, HttpError> { diff --git a/nexus/tests/config.test.toml b/nexus/tests/config.test.toml index 4cf96ba177a..929ec3e57cc 100644 --- a/nexus/tests/config.test.toml +++ b/nexus/tests/config.test.toml @@ -10,7 +10,7 @@ session_absolute_timeout_minutes = 1440 # 24 hours # List of authentication schemes to support. [authn] -schemes_external = ["spoof", "session_cookie"] +schemes_external = ["spoof", "session_cookie", "access_token"] # # NOTE: for the test suite, if mode = "file", the file path MUST be the sentinel diff --git a/nexus/tests/integration_tests/device_auth.rs b/nexus/tests/integration_tests/device_auth.rs index 40ee0d40ba9..62c1fe281f8 100644 --- a/nexus/tests/integration_tests/device_auth.rs +++ b/nexus/tests/integration_tests/device_auth.rs @@ -2,8 +2,22 @@ // 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/. -use nexus_test_utils::http_testing::{AuthnMode, NexusRequest, RequestBuilder}; +use std::num::NonZeroU32; + +use dropshot::test_util::ClientTestContext; +use nexus_auth::authn::USER_TEST_UNPRIVILEGED; +use nexus_db_queries::db::fixed_data::silo::DEFAULT_SILO; +use nexus_db_queries::db::identity::{Asset, Resource}; +use nexus_test_utils::http_testing::TestResponse; +use nexus_test_utils::resource_helpers::{ + object_get, object_put, object_put_error, +}; +use nexus_test_utils::{ + http_testing::{AuthnMode, NexusRequest, RequestBuilder}, + resource_helpers::grant_iam, +}; use nexus_test_utils_macros::nexus_test; +use nexus_types::external_api::{params, views}; use nexus_types::external_api::{ params::{DeviceAccessTokenRequest, DeviceAuthRequest, DeviceAuthVerify}, views::{ @@ -12,7 +26,9 @@ use nexus_types::external_api::{ }; use http::{StatusCode, header, method::Method}; +use oxide_client::types::SiloRole; use serde::Deserialize; +use tokio::time::{Duration, sleep}; use uuid::Uuid; type ControlPlaneTestContext = @@ -149,4 +165,208 @@ async fn test_device_auth_flow(cptestctx: &ControlPlaneTestContext) { assert_eq!(token.token_type, DeviceAccessTokenType::Bearer); assert_eq!(token.access_token.len(), 52); assert!(token.access_token.starts_with("oxide-token-")); + + // now make a request with the token. it 403s because unpriv user has no + // roles + project_list(&testctx, &token.access_token, StatusCode::FORBIDDEN) + .await + .expect("projects list should 403 with no roles"); + + // make sure it also fails with a nonsense token + project_list(&testctx, "oxide-token-xyz", StatusCode::UNAUTHORIZED) + .await + .expect("projects list should 403 with nonsense token"); + + // grant unprivileged user silo viewer so they can fetch the projects + grant_iam( + testctx, + &format!("/v1/system/silos/{}", DEFAULT_SILO.identity().name), + SiloRole::Viewer, + USER_TEST_UNPRIVILEGED.id(), + AuthnMode::PrivilegedUser, + ) + .await; + + // now make the request again and it should work + project_list(&testctx, &token.access_token, StatusCode::OK) + .await + .expect("failed to get projects with token"); +} + +/// Helper to make the test cute. Goes through the whole flow, returns the token +/// as a string +async fn get_device_token(testctx: &ClientTestContext) -> String { + let client_id = Uuid::new_v4(); + let authn_params = DeviceAuthRequest { client_id }; + + // Start a device authentication flow + let auth_response: DeviceAuthResponse = + RequestBuilder::new(testctx, Method::POST, "/device/auth") + .allow_non_dropshot_errors() + .body_urlencoded(Some(&authn_params)) + .expect_status(Some(StatusCode::OK)) + .execute() + .await + .expect("failed to start client authentication flow") + .parsed_body() + .expect("client authentication response"); + + let device_code = auth_response.device_code; + let user_code = auth_response.user_code; + + let confirm_params = DeviceAuthVerify { user_code }; + + // Confirm the device authentication + NexusRequest::new( + RequestBuilder::new(testctx, Method::POST, "/device/confirm") + .body(Some(&confirm_params)) + .expect_status(Some(StatusCode::NO_CONTENT)), + ) + .authn_as(AuthnMode::PrivilegedUser) + .execute() + .await + .expect("failed to confirm"); + + let token_params = DeviceAccessTokenRequest { + grant_type: "urn:ietf:params:oauth:grant-type:device_code".to_string(), + device_code, + client_id, + }; + + // Get the token + let token: DeviceAccessTokenGrant = NexusRequest::new( + RequestBuilder::new(testctx, Method::POST, "/device/token") + .allow_non_dropshot_errors() + .body_urlencoded(Some(&token_params)) + .expect_status(Some(StatusCode::OK)), + ) + .authn_as(AuthnMode::PrivilegedUser) + .execute() + .await + .expect("failed to get token") + .parsed_body() + .expect("failed to deserialize token response"); + + token.access_token +} + +#[nexus_test] +async fn test_device_token_expiration(cptestctx: &ControlPlaneTestContext) { + let testctx = &cptestctx.external_client; + + let settings: views::SiloAuthSettings = + object_get(testctx, "/v1/auth-settings").await; + assert_eq!(settings.device_token_max_ttl_seconds, None); + + // get a token for the privileged user. default silo max token expiration + // is null, so tokens don't expire + let initial_token = get_device_token(testctx).await; + + // test token works on project list + project_list(&testctx, &initial_token, StatusCode::OK) + .await + .expect("initial token should work"); + + // passing negative or zero gives a 400 + for value in [-3, 0] { + let error = object_put_error( + testctx, + "/v1/auth-settings", + &serde_json::json!({ "device_token_max_ttl_seconds": value }), + StatusCode::BAD_REQUEST, + ) + .await; + let msg = "unable to parse JSON body: \ + device_token_max_ttl_seconds: invalid value"; + assert!(error.message.starts_with(&msg)); + } + for value in [-3, 0] { + let error = object_put_error( + testctx, + "/v1/auth-settings", + &serde_json::json!({ "device_token_max_ttl_seconds": value }), + StatusCode::BAD_REQUEST, + ) + .await; + let msg = "unable to parse JSON body: \ + device_token_max_ttl_seconds: invalid value"; + assert!(error.message.starts_with(&msg)); + } + + // omitting the key is also a 400 + let error = object_put_error( + testctx, + "/v1/auth-settings", + &serde_json::json!({}), + StatusCode::BAD_REQUEST, + ) + .await; + let msg = "unable to parse JSON body: \ + missing field `device_token_max_ttl_seconds`"; + assert!(error.message.starts_with(&msg)); + + // set token expiration on silo to 3 seconds + let settings: views::SiloAuthSettings = object_put( + testctx, + "/v1/auth-settings", + ¶ms::SiloAuthSettingsUpdate { + device_token_max_ttl_seconds: NonZeroU32::new(3).into(), + }, + ) + .await; + + assert_eq!(settings.device_token_max_ttl_seconds, Some(3)); + + // might as well test the get endpoint as well + let settings: views::SiloAuthSettings = + object_get(testctx, "/v1/auth-settings").await; + assert_eq!(settings.device_token_max_ttl_seconds, Some(3)); + + // create token again (this one will have the 3-second expiration) + let expiring_token = get_device_token(testctx).await; + + // immediately use token, it should work + project_list(&testctx, &expiring_token, StatusCode::OK) + .await + .expect("expiring token should work immediately"); + + // wait 4 seconds to ensure token has expired + sleep(Duration::from_secs(4)).await; + + // confirm token has expired + project_list(&testctx, &expiring_token, StatusCode::UNAUTHORIZED) + .await + .expect("expiring token should fail after expiration"); + + // original token should still work (created before the expiration setting) + project_list(&testctx, &initial_token, StatusCode::OK) + .await + .expect("initial token should still work"); + + // now test setting the silo max TTL back to null + let settings: views::SiloAuthSettings = object_put( + testctx, + "/v1/auth-settings", + ¶ms::SiloAuthSettingsUpdate { + device_token_max_ttl_seconds: None.into(), + }, + ) + .await; + assert_eq!(settings.device_token_max_ttl_seconds, None); + + let settings: views::SiloAuthSettings = + object_get(testctx, "/v1/auth-settings").await; + assert_eq!(settings.device_token_max_ttl_seconds, None); +} + +async fn project_list( + testctx: &ClientTestContext, + token: &str, + status: StatusCode, +) -> Result { + RequestBuilder::new(testctx, Method::GET, "/v1/projects") + .header(header::AUTHORIZATION, format!("Bearer {}", token)) + .expect_status(Some(status)) + .execute() + .await } diff --git a/nexus/tests/integration_tests/endpoints.rs b/nexus/tests/integration_tests/endpoints.rs index 0a97ba12d83..5dfcbf02d28 100644 --- a/nexus/tests/integration_tests/endpoints.rs +++ b/nexus/tests/integration_tests/endpoints.rs @@ -34,6 +34,7 @@ use omicron_common::api::external::IdentityMetadataUpdateParams; use omicron_common::api::external::InstanceCpuCount; use omicron_common::api::external::Name; use omicron_common::api::external::NameOrId; +use omicron_common::api::external::Nullable; use omicron_common::api::external::RouteDestination; use omicron_common::api::external::RouteTarget; use omicron_common::api::external::UserId; @@ -42,6 +43,7 @@ use omicron_test_utils::certificates::CertificateChain; use semver::Version; use std::net::IpAddr; use std::net::Ipv4Addr; +use std::num::NonZeroU32; use std::str::FromStr; use std::sync::LazyLock; @@ -1632,6 +1634,22 @@ pub static VERIFY_ENDPOINTS: LazyLock> = ), ], }, + VerifyEndpoint { + url: "/v1/auth-settings", + visibility: Visibility::Public, + unprivileged_access: UnprivilegedAccess::ReadOnly, + allowed_methods: vec![ + AllowedMethod::Get, + AllowedMethod::Put( + serde_json::to_value(¶ms::SiloAuthSettingsUpdate { + device_token_max_ttl_seconds: Nullable( + NonZeroU32::new(3), + ), + }) + .unwrap(), + ), + ], + }, VerifyEndpoint { url: "/v1/users", visibility: Visibility::Public, diff --git a/nexus/types/src/external_api/params.rs b/nexus/types/src/external_api/params.rs index 518efe864b0..90b220d1f9a 100644 --- a/nexus/types/src/external_api/params.rs +++ b/nexus/types/src/external_api/params.rs @@ -13,8 +13,8 @@ use omicron_common::api::external::{ AddressLotKind, AffinityPolicy, AllowedSourceIps, BfdMode, BgpPeer, ByteCount, FailureDomain, Hostname, IdentityMetadataCreateParams, IdentityMetadataUpdateParams, InstanceAutoRestartPolicy, InstanceCpuCount, - LinkFec, LinkSpeed, Name, NameOrId, PaginationOrder, RouteDestination, - RouteTarget, UserId, + LinkFec, LinkSpeed, Name, NameOrId, Nullable, PaginationOrder, + RouteDestination, RouteTarget, UserId, }; use omicron_common::disk::DiskVariant; use oxnet::{IpNet, Ipv4Net, Ipv6Net}; @@ -27,6 +27,7 @@ use serde::{ }; use std::collections::BTreeMap; use std::collections::BTreeSet; +use std::num::NonZeroU32; use std::{net::IpAddr, str::FromStr}; use url::Url; use uuid::Uuid; @@ -481,6 +482,21 @@ pub struct SiloQuotasUpdate { pub storage: Option, } +// TODO: Unlike quota values, silo settings are nullable, so we need passing +// null to be meaningful here. But it's confusing for it to work that way here +// and differently for quotas. Maybe the best thing would be to make them all +// non-nullable on SiloQuotasUpdate. I vaguely remember the latter being the +// direction we wanted to go in general anyway. Can't find the issue where it +// was discussed. + +/// Updateable properties of a silo's settings. +#[derive(Clone, Debug, Serialize, Deserialize, JsonSchema)] +pub struct SiloAuthSettingsUpdate { + /// Maximum lifetime of a device token in seconds. If set to null, users + /// will be able to create tokens that do not expire. + pub device_token_max_ttl_seconds: Nullable, +} + /// Create-time parameters for a `User` #[derive(Clone, Deserialize, JsonSchema)] pub struct UserCreate { diff --git a/nexus/types/src/external_api/views.rs b/nexus/types/src/external_api/views.rs index 61b2e1f578f..302700e6a15 100644 --- a/nexus/types/src/external_api/views.rs +++ b/nexus/types/src/external_api/views.rs @@ -116,6 +116,15 @@ impl SimpleIdentityOrName for SiloUtilization { } } +/// View of silo authentication settings +#[derive(Clone, Debug, Deserialize, Serialize, JsonSchema)] +pub struct SiloAuthSettings { + pub silo_id: Uuid, + /// Maximum lifetime of a device token in seconds. If set to null, users + /// will be able to create tokens that do not expire. + pub device_token_max_ttl_seconds: Option, +} + // AFFINITY GROUPS /// View of an Affinity Group diff --git a/openapi/nexus.json b/openapi/nexus.json index 6bfc55e608e..fa20508660b 100644 --- a/openapi/nexus.json +++ b/openapi/nexus.json @@ -2140,6 +2140,68 @@ } } }, + "/v1/auth-settings": { + "get": { + "tags": [ + "silos" + ], + "summary": "Fetch current silo's auth settings", + "operationId": "auth_settings_view", + "responses": { + "200": { + "description": "successful operation", + "content": { + "application/json": { + "schema": { + "$ref": "#/components/schemas/SiloAuthSettings" + } + } + } + }, + "4XX": { + "$ref": "#/components/responses/Error" + }, + "5XX": { + "$ref": "#/components/responses/Error" + } + } + }, + "put": { + "tags": [ + "silos" + ], + "summary": "Update current silo's auth settings", + "operationId": "auth_settings_update", + "requestBody": { + "content": { + "application/json": { + "schema": { + "$ref": "#/components/schemas/SiloAuthSettingsUpdate" + } + } + }, + "required": true + }, + "responses": { + "200": { + "description": "successful operation", + "content": { + "application/json": { + "schema": { + "$ref": "#/components/schemas/SiloAuthSettings" + } + } + } + }, + "4XX": { + "$ref": "#/components/responses/Error" + }, + "5XX": { + "$ref": "#/components/responses/Error" + } + } + } + }, "/v1/certificates": { "get": { "tags": [ @@ -22544,6 +22606,42 @@ "time_modified" ] }, + "SiloAuthSettings": { + "description": "View of silo authentication settings", + "type": "object", + "properties": { + "device_token_max_ttl_seconds": { + "nullable": true, + "description": "Maximum lifetime of a device token in seconds. If set to null, users will be able to create tokens that do not expire.", + "type": "integer", + "format": "uint32", + "minimum": 0 + }, + "silo_id": { + "type": "string", + "format": "uuid" + } + }, + "required": [ + "silo_id" + ] + }, + "SiloAuthSettingsUpdate": { + "description": "Updateable properties of a silo's settings.", + "type": "object", + "properties": { + "device_token_max_ttl_seconds": { + "nullable": true, + "description": "Maximum lifetime of a device token in seconds. If set to null, users will be able to create tokens that do not expire.", + "type": "integer", + "format": "uint32", + "minimum": 1 + } + }, + "required": [ + "device_token_max_ttl_seconds" + ] + }, "SiloCreate": { "description": "Create-time parameters for a `Silo`", "type": "object", diff --git a/schema/crdb/dbinit.sql b/schema/crdb/dbinit.sql index 55a005cba9e..c26eeb026e9 100644 --- a/schema/crdb/dbinit.sql +++ b/schema/crdb/dbinit.sql @@ -1065,6 +1065,14 @@ WHERE AND s.time_deleted IS NULL; +CREATE TABLE IF NOT EXISTS omicron.public.silo_auth_settings ( + silo_id UUID PRIMARY KEY, + time_created TIMESTAMPTZ NOT NULL, + time_modified TIMESTAMPTZ NOT NULL, + + -- null means no max: users can tokens that never expire + device_token_max_ttl_seconds INT8 CHECK (device_token_max_ttl_seconds > 0) +); /* * Projects */ @@ -5682,7 +5690,7 @@ INSERT INTO omicron.public.db_metadata ( version, target_version ) VALUES - (TRUE, NOW(), NOW(), '145.0.0', NULL) + (TRUE, NOW(), NOW(), '146.0.0', NULL) ON CONFLICT DO NOTHING; COMMIT; diff --git a/schema/crdb/silo-settings-token-expiration/up01.sql b/schema/crdb/silo-settings-token-expiration/up01.sql new file mode 100644 index 00000000000..56f402c1b72 --- /dev/null +++ b/schema/crdb/silo-settings-token-expiration/up01.sql @@ -0,0 +1,8 @@ +CREATE TABLE IF NOT EXISTS omicron.public.silo_auth_settings ( + silo_id UUID PRIMARY KEY, + time_created TIMESTAMPTZ NOT NULL, + time_modified TIMESTAMPTZ NOT NULL, + + -- null means no max: users can tokens that never expire + device_token_max_ttl_seconds INT8 CHECK (device_token_max_ttl_seconds > 0) +); diff --git a/schema/crdb/silo-settings-token-expiration/up02.sql b/schema/crdb/silo-settings-token-expiration/up02.sql new file mode 100644 index 00000000000..fdb5589a5b4 --- /dev/null +++ b/schema/crdb/silo-settings-token-expiration/up02.sql @@ -0,0 +1,9 @@ +INSERT INTO omicron.public.silo_auth_settings ( + silo_id, + time_created, + time_modified, + device_token_max_ttl_seconds +) +SELECT id, NOW(), NOW(), NULL +FROM omicron.public.silo +WHERE time_deleted IS NULL;