From ed2db3ff7ceff04cd4a3792d2272e835789cc466 Mon Sep 17 00:00:00 2001 From: Jim Crossley Date: Tue, 7 Jan 2025 14:19:46 -0500 Subject: [PATCH] feat: remove the --devmode runtime option From what I can gather from the code, --devmode essentially did 2 things: 1) ran the db migrations, if any, and b) fired up an embedded OIDC server. The concept of --devmode is a holdover from version 1, but since version 2 introduced the concept of "PM mode". Having both modes is a source of confusion, and we have too many runtime parameters, anyway. I also removed some public fn's that didn't seem to be used. It's difficult enough to understand our OIDC config code without it being conflated with a "dev mode". I assumed from the code that our dev mode defaults were reasonable defaults whether dev mode was a thing or not. Signed-off-by: Jim Crossley --- common/auth/src/auth.rs | 15 +++---- common/auth/src/authenticator/config.rs | 11 +++--- common/auth/src/client/provider/openid.rs | 46 ---------------------- common/auth/src/{devmode.rs => default.rs} | 12 +++--- common/auth/src/lib.rs | 2 +- common/auth/src/swagger_ui.rs | 22 +++-------- server/src/embedded_oidc.rs | 4 +- server/src/profile/api.rs | 42 ++++++++------------ server/src/profile/importer.rs | 2 +- 9 files changed, 41 insertions(+), 115 deletions(-) rename common/auth/src/{devmode.rs => default.rs} (85%) diff --git a/common/auth/src/auth.rs b/common/auth/src/auth.rs index b9e55c57a..0d5900f07 100644 --- a/common/auth/src/auth.rs +++ b/common/auth/src/auth.rs @@ -53,17 +53,14 @@ pub fn is_default(d: &D) -> bool { impl AuthConfigArguments { pub fn split( self, - devmode: bool, + defaults: bool, ) -> Result, anyhow::Error> { - // disabled overrides devmode if self.disabled { return Ok(None); } - - // check for devmode - if devmode { - log::warn!("Running in developer mode"); - return Ok(Some((AuthenticatorConfig::devmode(), Default::default()))); + if defaults { + log::warn!("Running with default auth config"); + return Ok(Some(Default::default())); } Ok(Some(match self.config { @@ -96,7 +93,7 @@ mod tests { use super::*; #[test] - fn auth_disabled_devmode_false() { + fn auth_disabled_no_defaults() { let args = AuthConfigArguments { disabled: true, config: None, @@ -108,7 +105,7 @@ mod tests { } #[test] - fn auth_enabled_devmode_true() { + fn auth_enabled_with_defaults() { let args = AuthConfigArguments { disabled: false, config: None, diff --git a/common/auth/src/authenticator/config.rs b/common/auth/src/authenticator/config.rs index 8eb5a5ad3..f23da079e 100644 --- a/common/auth/src/authenticator/config.rs +++ b/common/auth/src/authenticator/config.rs @@ -1,4 +1,4 @@ -use crate::{authenticator::default_scope_mappings, devmode}; +use crate::{authenticator::default_scope_mappings, default}; use clap::ArgAction; use serde::{Deserialize, Serialize}; use std::collections::HashMap; @@ -31,15 +31,14 @@ pub struct AuthenticatorConfig { pub clients: Vec, } -impl AuthenticatorConfig { - /// Create settings when using `--devmode`. - pub fn devmode() -> Self { +impl Default for AuthenticatorConfig { + fn default() -> Self { Self { - clients: devmode::CLIENT_IDS + clients: default::CLIENT_IDS .iter() .map(|client_id| AuthenticatorClientConfig { client_id: client_id.to_string(), - issuer_url: devmode::issuer_url(), + issuer_url: default::issuer_url(), scope_mappings: default_scope_mappings(), additional_permissions: Default::default(), required_audience: None, diff --git a/common/auth/src/client/provider/openid.rs b/common/auth/src/client/provider/openid.rs index 516dcbf4d..863ca54be 100644 --- a/common/auth/src/client/provider/openid.rs +++ b/common/auth/src/client/provider/openid.rs @@ -3,10 +3,8 @@ use super::{ {Credentials, TokenProvider}, }; use crate::client::Expires; -use crate::devmode; use anyhow::Context; use core::fmt::{self, Debug, Formatter}; -use std::time::Duration; use std::{ops::Deref, sync::Arc}; use tokio::sync::RwLock; use url::Url; @@ -52,34 +50,10 @@ pub struct OpenIdTokenProviderConfigArguments { pub tls_insecure: bool, } -impl OpenIdTokenProviderConfigArguments { - pub fn devmode() -> OpenIdTokenProviderConfigArguments { - Self { - issuer_url: Some(devmode::issuer_url()), - client_id: Some(devmode::SERVICE_CLIENT_ID.to_string()), - client_secret: Some(devmode::SSO_CLIENT_SECRET.to_string()), - refresh_before: Duration::from_secs(30).into(), - tls_insecure: false, - } - } -} - impl OpenIdTokenProviderConfigArguments { pub async fn into_provider(self) -> anyhow::Result> { OpenIdTokenProviderConfig::new_provider(OpenIdTokenProviderConfig::from_args(self)).await } - - pub async fn into_provider_or_devmode( - self, - devmode: bool, - ) -> anyhow::Result> { - let config = match devmode { - true => Some(OpenIdTokenProviderConfig::devmode()), - false => OpenIdTokenProviderConfig::from_args(self), - }; - - OpenIdTokenProviderConfig::new_provider(config).await - } } #[derive(Clone, Debug, PartialEq, Eq, clap::Args)] @@ -92,16 +66,6 @@ pub struct OpenIdTokenProviderConfig { } impl OpenIdTokenProviderConfig { - pub fn devmode() -> Self { - Self { - issuer_url: devmode::issuer_url(), - client_id: devmode::SERVICE_CLIENT_ID.to_string(), - client_secret: devmode::SSO_CLIENT_SECRET.to_string(), - refresh_before: Duration::from_secs(30).into(), - tls_insecure: false, - } - } - pub async fn new_provider(config: Option) -> anyhow::Result> { Ok(match config { Some(config) => Arc::new(OpenIdTokenProvider::with_config(config).await?), @@ -109,16 +73,6 @@ impl OpenIdTokenProviderConfig { }) } - pub fn from_args_or_devmode( - arguments: OpenIdTokenProviderConfigArguments, - devmode: bool, - ) -> Option { - match devmode { - true => Some(Self::devmode()), - false => Self::from_args(arguments), - } - } - pub fn from_args(arguments: OpenIdTokenProviderConfigArguments) -> Option { match ( arguments.client_id, diff --git a/common/auth/src/devmode.rs b/common/auth/src/default.rs similarity index 85% rename from common/auth/src/devmode.rs rename to common/auth/src/default.rs index e3316b9e7..f9b26615e 100644 --- a/common/auth/src/devmode.rs +++ b/common/auth/src/default.rs @@ -1,16 +1,16 @@ -/// The default issuer when using `--devmode`. +/// The default issuer url pub const ISSUER_URL: &str = "http://localhost:8090/realms/trustify"; /// The default client id for the frontend pub const FRONTEND_CLIENT_ID: &str = "frontend"; -/// The default "service" client ID for devmode +/// The default "service" client ID pub const SERVICE_CLIENT_ID: &str = "testing-manager"; pub const PUBLIC_CLIENT_IDS: &[&str] = &[FRONTEND_CLIENT_ID]; pub const CONFIDENTIAL_CLIENT_IDS: &[&str] = &["walker", "testing-user", SERVICE_CLIENT_ID]; -/// The clients which will be accepted by services when running with `--devmode`. +/// The clients which will be accepted by services /// /// This also includes the "testing" clients, as this allows running the testsuite against an /// already spun-up set of services. @@ -28,10 +28,10 @@ pub const SWAGGER_UI_CLIENT_ID: &str = FRONTEND_CLIENT_ID; /// This is not a secret. Don't use this in production. pub const SSO_CLIENT_SECRET: &str = "R8A6KFeyxJsMDBhjfHbpZTIF0GWt43HP"; -/// Get the issuer URL for `--devmode`. +/// Get the issuer URL /// -/// This can be either the value of [`ISSUER_URL`], or it can be overridden using the environment -/// variable `ISSUER_URL`. +/// This can be either the value of [`ISSUER_URL`], or it can be +/// overridden using the environment variable `TRUSTD_ISSUER_URL`. pub fn issuer_url() -> String { std::env::var("TRUSTD_ISSUER_URL").unwrap_or_else(|_| ISSUER_URL.to_string()) } diff --git a/common/auth/src/lib.rs b/common/auth/src/lib.rs index 79b3924ad..15f420350 100644 --- a/common/auth/src/lib.rs +++ b/common/auth/src/lib.rs @@ -5,7 +5,7 @@ pub mod auth; pub mod authenticator; pub mod authorizer; pub mod client; -pub mod devmode; +pub mod default; #[cfg(feature = "swagger")] pub mod swagger_ui; diff --git a/common/auth/src/swagger_ui.rs b/common/auth/src/swagger_ui.rs index daf32807b..7e0ecf5ca 100644 --- a/common/auth/src/swagger_ui.rs +++ b/common/auth/src/swagger_ui.rs @@ -1,4 +1,4 @@ -use crate::devmode::{self, SWAGGER_UI_CLIENT_ID}; +use crate::default::{self, SWAGGER_UI_CLIENT_ID}; use actix_web::dev::HttpServiceFactory; use openid::{Client, Discovered, Provider, StandardClaims}; use std::sync::Arc; @@ -11,7 +11,7 @@ use utoipa::openapi::{ }; use utoipa_swagger_ui::{oauth, Config, SwaggerUi}; -#[derive(Clone, Debug, Default, clap::Args)] +#[derive(Clone, Debug, clap::Args)] #[command( rename_all_env = "SCREAMING_SNAKE_CASE", next_help_heading = "Swagger UI OIDC" @@ -44,12 +44,12 @@ pub struct SwaggerUiOidcConfig { pub swagger_ui_oidc_client_id: String, } -impl SwaggerUiOidcConfig { - pub fn devmode() -> Self { +impl Default for SwaggerUiOidcConfig { + fn default() -> Self { Self { tls_insecure: false, ca_certificates: vec![], - swagger_ui_oidc_issuer_url: Some(devmode::issuer_url()), + swagger_ui_oidc_issuer_url: Some(default::issuer_url()), swagger_ui_oidc_client_id: SWAGGER_UI_CLIENT_ID.to_string(), } } @@ -98,18 +98,6 @@ impl SwaggerUiOidc { })) } - pub async fn from_devmode_or_config( - devmode: bool, - config: SwaggerUiOidcConfig, - ) -> anyhow::Result> { - let config = match devmode { - true => SwaggerUiOidcConfig::devmode(), - false => config, - }; - - Self::new(config).await - } - pub fn apply_to_schema(&self, openapi: &mut OpenApi) { if let Some(components) = &mut openapi.components { // The swagger UI OIDC client still is weird, let's use OAuth2 diff --git a/server/src/embedded_oidc.rs b/server/src/embedded_oidc.rs index 9145cd724..6dc0e2416 100644 --- a/server/src/embedded_oidc.rs +++ b/server/src/embedded_oidc.rs @@ -9,7 +9,7 @@ use rand::Rng; use std::time::Duration; use tokio::sync::oneshot; use tokio::task::JoinHandle; -use trustify_auth::devmode::{ +use trustify_auth::default::{ CONFIDENTIAL_CLIENT_IDS, ISSUER_URL, PUBLIC_CLIENT_IDS, SSO_CLIENT_SECRET, }; use trustify_infrastructure::health::Check; @@ -62,8 +62,6 @@ fn create(enabled: bool) -> anyhow::Result> { }); } - // take the devmode url and extract all information for building the server - let url = Url::parse(ISSUER_URL)?; let port = url.port().unwrap_or(8090); let mut path = url diff --git a/server/src/profile/api.rs b/server/src/profile/api.rs index fd4fbc769..d77d274a0 100644 --- a/server/src/profile/api.rs +++ b/server/src/profile/api.rs @@ -18,7 +18,7 @@ use trustify_auth::{ auth::AuthConfigArguments, authenticator::Authenticator, authorizer::Authorizer, - devmode::{FRONTEND_CLIENT_ID, ISSUER_URL, PUBLIC_CLIENT_IDS}, + default::{FRONTEND_CLIENT_ID, ISSUER_URL, PUBLIC_CLIENT_IDS}, swagger_ui::{swagger_ui_with_auth, SwaggerUiOidc, SwaggerUiOidcConfig}, }; use trustify_common::{config::Database, db, model::BinaryByteSize}; @@ -53,9 +53,6 @@ use crate::embedded_oidc; /// Run the API server #[derive(clap::Args, Debug)] pub struct Run { - #[arg(long, env)] - pub devmode: bool, - #[arg(long, env)] pub sample_data: bool, @@ -194,21 +191,16 @@ impl Run { impl InitData { async fn new(context: InitContext, run: Run) -> anyhow::Result { - // The devmode for the auth parts. This allows us to enable devmode for auth, but not - // for other parts. #[allow(unused_mut)] - let mut auth_devmode = run.devmode; + let mut auth_embedded = false; #[cfg(feature = "garage-door")] let embedded_oidc = { - // When running with the embedded OIDC server, re-use devmode. Running the embedded OIDC - // without devmode doesn't make any sense. However, the pm-mode doesn't know about - // devmode. Also, enabling devmode might trigger other logic. - auth_devmode = run.embedded_oidc; + auth_embedded = run.embedded_oidc; embedded_oidc::spawn(run.embedded_oidc).await? }; - let (authn, authz) = run.auth.split(auth_devmode)?.unzip(); + let (authn, authz) = run.auth.split(auth_embedded)?.unzip(); let authenticator: Option> = Authenticator::from_config(authn).await?.map(Arc::new); let authorizer = Authorizer::new(authz); @@ -218,19 +210,19 @@ impl InitData { } let swagger_oidc = match authenticator.is_some() { - true => SwaggerUiOidc::from_devmode_or_config(auth_devmode, run.swagger_ui_oidc) - .await? - .map(Arc::new), + true => SwaggerUiOidc::new(if auth_embedded { + Default::default() + } else { + run.swagger_ui_oidc + }) + .await? + .map(Arc::new), false => None, }; let db = db::Database::new(&run.database).await?; - if run.devmode { - db.migrate().await?; - } - - if run.devmode || run.sample_data { + if run.sample_data { sample_data(db.clone()).await?; } @@ -252,12 +244,10 @@ impl InitData { .as_ref() .cloned() .unwrap_or_else(|| PathBuf::from("./.trustify/storage")); - if run.devmode { - create_dir_all(&storage).context(format!( - "Failed to create filesystem storage directory: {:?}", - run.storage.fs_path - ))?; - } + create_dir_all(&storage).context(format!( + "Failed to create filesystem storage directory: {:?}", + run.storage.fs_path + ))?; DispatchBackend::Filesystem( FileSystemBackend::new(storage, run.storage.compression).await?, ) diff --git a/server/src/profile/importer.rs b/server/src/profile/importer.rs index 322c314d0..555240ec1 100644 --- a/server/src/profile/importer.rs +++ b/server/src/profile/importer.rs @@ -18,7 +18,7 @@ use trustify_auth::{ auth::AuthConfigArguments, authenticator::Authenticator, authorizer::Authorizer, - devmode::{FRONTEND_CLIENT_ID, ISSUER_URL, PUBLIC_CLIENT_IDS}, + default::{FRONTEND_CLIENT_ID, ISSUER_URL, PUBLIC_CLIENT_IDS}, swagger_ui::{swagger_ui_with_auth, SwaggerUiOidc, SwaggerUiOidcConfig}, }; use trustify_common::{config::Database, db, model::BinaryByteSize};