From 8b81ee564aa820be1d16804b7f6bcdb8e483b81f Mon Sep 17 00:00:00 2001 From: Olivier 'reivilibre Date: Sun, 21 Apr 2024 16:42:13 +0100 Subject: [PATCH] Allow specifying OIDC Client Secrets in external config file Signed-off-by: Olivier 'reivilibre --- config.toml | 1 - secrets.toml | 3 ++ src/bin/idcoop.rs | 56 +++++++++++++++++++++++++---------- src/config.rs | 14 +++++++-- src/web/oauth_openid/token.rs | 11 ++++--- 5 files changed, 63 insertions(+), 22 deletions(-) create mode 100644 secrets.toml diff --git a/config.toml b/config.toml index 9ca6423..72a7067 100644 --- a/config.toml +++ b/config.toml @@ -11,7 +11,6 @@ rsa_keypair = "keypair.pem" name = "some service" redirect_uris = ["http://localhost:9876/callback"] allow_user_classes = ["user"] -secret = "lol" [postgres] connect = "postgres:" diff --git a/secrets.toml b/secrets.toml new file mode 100644 index 0000000..7582290 --- /dev/null +++ b/secrets.toml @@ -0,0 +1,3 @@ +[oidc_secrets] +x = "abc" + diff --git a/src/bin/idcoop.rs b/src/bin/idcoop.rs index c6f03ac..6e8f8f1 100644 --- a/src/bin/idcoop.rs +++ b/src/bin/idcoop.rs @@ -7,7 +7,7 @@ use comfy_table::presets::UTF8_FULL; use comfy_table::{Attribute, Cell, Color, ContentArrangement, Row, Table}; use confique::{Config, Partial}; use eyre::{bail, Context, ContextCompat}; -use idcoop::config::SecretConfig; +use idcoop::config::{SecretConfig, SeparateSecretConfiguration}; use idcoop::passwords::create_password_hash; use idcoop::store::{CreateUser, IdCoopStore}; use idcoop::{config::Configuration, web}; @@ -22,6 +22,9 @@ struct Options { #[clap(short = 'c', long = "config")] config_files: Vec, + #[clap(short = 'S', long = "secrets")] + secret_files: Vec, + #[clap(subcommand)] subcommand: Subcommand, } @@ -42,6 +45,23 @@ enum Subcommand { }, } +fn load_config_files(files: &[PathBuf]) -> eyre::Result { + let mut partial: C::Partial = Partial::from_env().context("failed to load env vars")?; + + for file in files { + let cfg_file = confique::File::new(file) + .with_context(|| format!("bad file path: {file:?}"))? + .required() + .load() + .with_context(|| format!("failed to load file: {file:?}"))?; + partial = partial.with_fallback(cfg_file); + } + + partial = partial.with_fallback(Partial::default_values()); + + C::from_partial(partial).context("failed to load config") +} + #[tokio::main(flavor = "current_thread")] async fn main() -> eyre::Result<()> { tracing_subscriber::registry() @@ -54,23 +74,29 @@ async fn main() -> eyre::Result<()> { let options = Options::parse(); - let mut partial: ::Partial = - Partial::from_env().context("failed to load env vars")?; + let mut config = load_config_files::(&options.config_files) + .context("failed to load main config file")?; + let mut sep_secrets = load_config_files::(&options.secret_files) + .context("failed to load separated secrets file")?; - for file in &options.config_files { - let cfg_file = confique::File::new(file) - .with_context(|| format!("bad file path: {file:?}"))? - .required() - .load() - .with_context(|| format!("failed to load file: {file:?}"))?; - partial = partial.with_fallback(cfg_file); + // Load secrets into the OIDC Client Configurations + for (oidc_client_id, oidc_client) in &mut config.oidc.clients { + let specified_in_main = oidc_client.secret.is_some(); + match sep_secrets.oidc_secrets.remove(oidc_client_id) { + Some(secret) => { + if specified_in_main { + bail!("Client secret for OIDC Client {oidc_client_id:?} is specified both in main config file and in separated secret file!"); + } + oidc_client.secret = Some(secret); + } + None => { + if !specified_in_main { + bail!("Client secret for OIDC Client {oidc_client_id:?} is not specified!"); + } + } + } } - partial = partial.with_fallback(Partial::default_values()); - - let config: Configuration = - Configuration::from_partial(partial).context("failed to load config")?; - match options.subcommand { Subcommand::Serve { bind } => { let Some(bind) = bind.or(config.listen.bind) else { diff --git a/src/config.rs b/src/config.rs index 5e9eae4..c6c6a0c 100644 --- a/src/config.rs +++ b/src/config.rs @@ -81,13 +81,22 @@ pub struct OidcClientConfiguration { /// Must be explicit because it is security sensitive and we don't want a typo to fail open. /// User classes are defined by the admin but at the very least includes 'active' and 'not active' (implied if no 'active' class set). /// (Design subject to change) + /// TODO not sure this design is current pub allow_user_classes: Vec, /// The shared secret for the client. + /// Must be populated (this is checked on startup). + /// Can be loaded from a separated secret configuration file. /// TODO /// - We should consider supporting other auth methods in the future. - /// - We should allow specifying this out of the main configuration somehow...? - pub secret: String, + pub secret: Option, +} + +#[derive(Config)] +pub struct SeparateSecretConfiguration { + /// Client secrets for the OIDC clients. + #[config(default = {})] + pub oidc_secrets: BTreeMap, } pub struct SecretConfig { @@ -101,6 +110,7 @@ impl SecretConfig { .context("failed to load RSA private key")?; let rsa_key_pair = RsaKeyPair::from_pem(&rsa_keypair_bytes).context("Failed to decode RSA key pair")?; + Ok(Self { rsa_key_pair }) } } diff --git a/src/web/oauth_openid/token.rs b/src/web/oauth_openid/token.rs index b6af3a2..f8dddaf 100644 --- a/src/web/oauth_openid/token.rs +++ b/src/web/oauth_openid/token.rs @@ -108,10 +108,13 @@ pub async fn oidc_token( }; if !bool::from( - basic_auth - .password() - .as_bytes() - .ct_eq(unverified_client_config.secret.as_bytes()), + basic_auth.password().as_bytes().ct_eq( + unverified_client_config + .secret + .as_ref() + .expect("client secret missing; should have been checked at startup") + .as_bytes(), + ), ) { return ( StatusCode::BAD_REQUEST,