Show config errors on diagnostics page

With the previous commits it is now possible to add a warning if their
rare issues with loading of the config, either environment variables
which we were not able to parse correctly, or if loading the
`config.json` failed instead of just showing it wasn't used, while it
does exists, which might be confusing.

Signed-off-by: BlackDex <black.dex@gmail.com>

src/api/admin.rs

@@ -782,6 +782,8 @@ async fn diagnostics(_token: AdminToken, ip_header: IpHeader, conn: DbConn) -> A
     .into_keys()
     .collect();
 
+    let invalid_env_vars = crate::util::INVALID_ENV_VARS.lock().unwrap().join(", ");
+
     let diagnostics_json = json!({
         "dns_resolved": dns_resolved,
         "current_release": VERSION,
@@ -804,7 +806,9 @@ async fn diagnostics(_token: AdminToken, ip_header: IpHeader, conn: DbConn) -> A
         "db_version": get_sql_server_version(&conn).await,
         "admin_url": format!("{}/diagnostics", admin_url()),
         "overrides": &CONFIG.get_overrides().join(", "),
+        "config_file_status": &CONFIG.get_config_file_status(),
         "template_overrides": check_template_overrides().join(", "),
+        "invalid_env_vars": invalid_env_vars,
         "invalid_feature_flags": invalid_feature_flags,
         "host_arch": env::consts::ARCH,
         "host_os":  env::consts::OS,

src/config.rs

@@ -111,6 +111,7 @@ macro_rules! make_config {
             _usr: ConfigBuilder,
 
             _overrides: Vec<&'static str>,
+            _config_file_status: ConfigFileStatus,
         }
 
         // Custom Deserialize for ConfigBuilder, mainly based upon https://serde.rs/deserialize-struct.html
@@ -259,17 +260,18 @@ macro_rules! make_config {
                 builder
             }
 
-            async fn from_file() -> Result<Self, Error> {
+            async fn from_file() -> Result<Option<Self>, Error> {
                 let operator = storage::operator_for_path(&CONFIG_FILE_PARENT_DIR)?;
                 let config_bytes = match operator.read(&CONFIG_FILENAME).await {
                     Ok(bytes) => bytes,
                     // No config file is fine, should not return any error
-                    Err(e) if e.kind() == opendal::ErrorKind::NotFound => return Ok(Self::default()),
+                    Err(e) if e.kind() == opendal::ErrorKind::NotFound => return Ok(None),
                     // Any other error might be an issue and should be passed back to the caller
                     Err(e) => return Err(e.into()),
                 };
+                let builder = serde_json::from_slice(&config_bytes.to_vec())?;
                 println!("[INFO] Using saved config from `{}` for configuration.\n", *CONFIG_FILE);
-                serde_json::from_slice(&config_bytes.to_vec()).map_err(Into::into)
+                Ok(Some(builder))
             }
 
             fn clear_non_editable(&mut self) {
@@ -446,6 +448,8 @@ macro_rules! make_config {
                     "domain_origin",
                     "domain_path",
                     "domain",
+                    "duo_ikey",
+                    "duo_host",
                     "helo_name",
                     "org_creation_users",
                     "signups_domains_whitelist",
@@ -457,6 +461,8 @@ macro_rules! make_config {
                     "sso_authority",
                     "sso_callback_path",
                     "sso_client_id",
+                    "yubico_client_id",
+                    "yubico_server",
                 ];
 
                 let cfg = {
@@ -489,6 +495,10 @@ macro_rules! make_config {
                 };
                 overrides
             }
+
+            pub fn get_config_file_status(&self) -> ConfigFileStatus {
+                self.inner.read().unwrap()._config_file_status
+            }
         }
     };
 }
@@ -1481,11 +1491,21 @@ pub const SUPPORTED_FEATURE_FLAGS: &[&str] = &[
     "pm-32009-new-item-types",
 ];
 
+#[derive(Clone, Copy, Serialize)]
+pub enum ConfigFileStatus {
+    NotFound,
+    Loaded,
+    Error,
+}
+
 impl Config {
     pub async fn load() -> Result<Self, Error> {
         // Loading from env and file
         let env = ConfigBuilder::from_env();
-        let usr = ConfigBuilder::from_file().await.unwrap_or_else(|e| {
+        let (usr, config_file_status) = match ConfigBuilder::from_file().await {
+            Ok(Some(usr)) => (usr, ConfigFileStatus::Loaded),
+            Ok(None) => (ConfigBuilder::default(), ConfigFileStatus::NotFound),
+            Err(e) => {
                 println!(
                     "[ERROR] Failed to load configuration from `{}`\n\
                     #####################################################################################\n\
@@ -1496,8 +1516,9 @@ impl Config {
                 {e:?}\n",
                     *CONFIG_FILE
                 );
-            ConfigBuilder::default()
-        });
+                (ConfigBuilder::default(), ConfigFileStatus::Error)
+            }
+        };
 
         // Create merged config, config file overwrites env
         let mut overrides = Vec::new();
@@ -1517,6 +1538,7 @@ impl Config {
                 _env: env,
                 _usr: usr,
                 _overrides: overrides,
+                _config_file_status: config_file_status,
             }),
         })
     }
@@ -1550,6 +1572,7 @@ impl Config {
             writer.config = config;
             writer._usr = builder;
             writer._overrides = overrides;
+            writer._config_file_status = ConfigFileStatus::Loaded;
         }
 
         //Save to file
@@ -1641,6 +1664,7 @@ impl Config {
             writer.config = config;
             writer._usr = usr;
             writer._overrides = Vec::new();
+            writer._config_file_status = ConfigFileStatus::NotFound;
         }
 
         // Invalidate CSS Cache because several config items might have impact on the rendered CSS

src/static/scripts/admin_diagnostics.js

@@ -93,7 +93,9 @@ async function generateSupportString(event, dj) {
     supportString += `* Running within a container: ${yn(dj.running_within_container)} (Base: ${dj.container_base_image})\n`;
     supportString += `* Database type: ${dj.db_type}\n`;
     supportString += `* Database version: ${dj.db_version}\n`;
-    supportString += `* Uses config.json: ${yn(dj.overrides !== "")}\n`;
+    supportString += `* Uses config.json: ${yn(dj.config_file_status !== "NotFound")}`;
+    supportString += dj.config_file_status !== "NotFound" ? ` (Valid: ${chk(dj.config_file_status === "Loaded")})\n` : "\n";
+    supportString += `* Environment variables are valid: ${chk(dj.invalid_env_vars == "")}\n`;
     supportString += `* Uses custom templates: ${yn(dj.template_overrides !== "")}${template_overrides}\n`;
     supportString += `* Uses a reverse proxy: ${yn(dj.ip_header_exists)}\n`;
     if (dj.ip_header_exists) {
@@ -116,7 +118,7 @@ async function generateSupportString(event, dj) {
     }
     supportString += `* HTTP Response Checks: ${chk(httpResponseCheck)}\n`;
     if (dj.invalid_feature_flags != "") {
-        supportString += "* Invalid feature flags: true\n";
+        supportString += "* Invalid feature flags: true :x:\n";
     }
 
     const jsonResponse = await fetch(`${BASE_URL}/admin/diagnostics/config`, {
@@ -132,6 +134,11 @@ async function generateSupportString(event, dj) {
     supportString += "\n### Config & Details (Generated via diagnostics page)\n\n";
     supportString += "<details><summary>Show Config & Details</summary>\n";
 
+    // Show invalid environment variables if they exists
+    if (dj.invalid_env_vars != "") {
+        supportString += `\n**Invalid environment variables:** ${dj.invalid_env_vars}\n`;
+    }
+
     // Add overrides if they exists
     if (dj.overrides != "") {
         supportString += `\n**Environment settings which are overridden:** ${dj.overrides}\n`;

src/static/templates/admin/diagnostics.hbs

@@ -69,12 +69,26 @@
                     </dd>
                     <dt class="col-sm-5">Uses config.json</dt>
                     <dd class="col-sm-7">
-                    {{#if page_data.overrides}}
+                    {{#if (eq page_data.config_file_status "Loaded")}}
                         <span class="d-inline"><b>Yes</b></span>
+                        {{#if page_data.overrides}}
                         <span class="badge bg-info text-dark abbr-badge" title="Environment variables are overwritten by a config.json.&#013;&#010;{{page_data.overrides}}">Details</span>
                         {{/if}}
-                    {{#unless page_data.overrides}}
-                        <span class="d-block"><b>No</b></span>
+                    {{~else if (eq page_data.config_file_status "Error")}}
+                        <span class="d-inline"><b>Errors</b></span>
+                        <span class="badge bg-danger abbr-badge" title="Config file could not be loaded and was ignored. Check server logs for details.">Details</span>
+                    {{~else}}
+                        <span class="d-inline"><b>No</b></span>
+                    {{/if}}
+                    </dd>
+                    <dt class="col-sm-5">Environment variables are valid</dt>
+                    <dd class="col-sm-7">
+                    {{#if page_data.invalid_env_vars}}
+                        <span class="d-inline"><b>No</b></span>
+                        <span class="badge bg-danger abbr-badge" title="Invalid environment variables are found and defaults are used instead.&#013;&#010;{{page_data.invalid_env_vars}}">Details</span>
+                    {{/if}}
+                    {{#unless page_data.invalid_env_vars}}
+                        <span class="d-block"><b>Yes</b></span>
                     {{/unless}}
                     </dd>
                     <dt class="col-sm-5">Uses custom templates</dt>

src/util.rs

@@ -447,7 +447,9 @@ pub fn get_env_str_value(key: &str) -> Option<String> {
     }
 }
 
-pub fn get_env<V>(key: &str) -> Option<V>
+pub static INVALID_ENV_VARS: std::sync::Mutex<Vec<&'static str>> = std::sync::Mutex::new(Vec::new());
+
+pub fn get_env<V>(key: &'static str) -> Option<V>
 where
     V: FromStr,
 {
@@ -456,11 +458,12 @@ where
     let parsed: Option<V> = try_parse_string(value);
     if parsed.is_none() && value_is_some {
         println!("[ERROR] `{key}` contains an invalid value for its type and is ignored, the default is used instead.");
+        INVALID_ENV_VARS.lock().unwrap().push(key);
     }
     parsed
 }
 
-pub fn get_env_bool(key: &str) -> Option<bool> {
+pub fn get_env_bool(key: &'static str) -> Option<bool> {
     const TRUE_VALUES: &[&str] = &["true", "t", "yes", "y", "1"];
     const FALSE_VALUES: &[&str] = &["false", "f", "no", "n", "0"];
 
@@ -469,6 +472,7 @@ pub fn get_env_bool(key: &str) -> Option<bool> {
         Some(val) if FALSE_VALUES.contains(&val.to_lowercase().as_ref()) => Some(false),
         Some(_) => {
             println!("[ERROR] `{key}` contains an invalid boolean value and is ignored, the default is used instead.");
+            INVALID_ENV_VARS.lock().unwrap().push(key);
             None
         }
         _ => None,