Fix enterprise settings not taking effect immediately, ehance gateway…

… token error logging (#865)

* fix bugs, including a bug related to enterprise settings

* move delay

* fix tests

* log token errors

src/bin/defguard.rs

@@ -9,7 +9,7 @@ use defguard::{
     db::{init_db, AppEvent, GatewayEvent, Settings, User},
     enterprise::{
         license::{run_periodic_license_check, set_cached_license, License},
-        limits::update_counts,
+        limits::{run_periodic_count_update, update_counts},
     },
     grpc::{run_grpc_bidi_stream, run_grpc_server, GatewayMap, WorkerState},
     init_dev_env, init_vpn_location,
@@ -123,7 +123,9 @@ async fn main() -> Result<(), anyhow::Error> {
         res = run_mail_handler(mail_rx, pool.clone()) => error!("Mail handler returned early: {res:#?}"),
         res = run_periodic_peer_disconnect(pool.clone(), wireguard_tx) => error!("Periodic peer disconnect task returned early: {res:#?}"),
         res = run_periodic_stats_purge(pool.clone(), config.stats_purge_frequency.into(), config.stats_purge_threshold.into()), if !config.disable_stats_purge => error!("Periodic stats purge task returned early: {res:#?}"),
-        res = run_periodic_license_check(pool) => error!("Periodic license check task returned early: {res:#?}"),
+        res = run_periodic_license_check(pool.clone()) => error!("Periodic license check task returned early: {res:#?}"),
+        // Temporary. Change to a database trigger when they are implemented.
+        res = run_periodic_count_update(&pool) => error!("Periodic count update task returned early: {res:#?}"),
     }
     Ok(())
 }

src/enterprise/handlers/mod.rs

@@ -1,5 +1,5 @@
 use crate::{
-    auth::SessionInfo,
+    auth::{SessionInfo, UserAdminRole},
     handlers::{ApiResponse, ApiResult},
 };
 
@@ -45,7 +45,19 @@ where
     }
 }
 
+/// Gets basic enterprise status.
 pub async fn check_enterprise_status() -> ApiResult {
+    let enterprise_enabled = is_enterprise_enabled();
+    Ok(ApiResponse {
+        json: serde_json::json!({
+            "enabled": enterprise_enabled,
+        }),
+        status: StatusCode::OK,
+    })
+}
+
+/// Gets full information about enterprise status.
+pub async fn check_enterprise_info(_admin: UserAdminRole, _session: SessionInfo) -> ApiResult {
     let enterprise_enabled = is_enterprise_enabled();
     let needs_license = needs_enterprise_license();
     let license = get_cached_license();

src/enterprise/handlers/openid_login.rs

@@ -26,7 +26,7 @@ use super::LicenseInfo;
 use crate::{
     appstate::AppState,
     db::{Id, Settings, User},
-    enterprise::db::models::openid_provider::OpenIdProvider,
+    enterprise::{db::models::openid_provider::OpenIdProvider, limits::update_counts},
     error::WebError,
     handlers::{
         auth::create_session,
@@ -270,6 +270,7 @@ pub(crate) async fn user_from_claims(
         }
     };
 
+    update_counts(pool).await?;
     Ok(user)
 }
 

src/enterprise/limits.rs

@@ -1,5 +1,9 @@
 use sqlx::{error::Error as SqlxError, query_as, PgPool};
-use std::sync::{RwLock, RwLockReadGuard};
+use std::{
+    sync::{RwLock, RwLockReadGuard},
+    time::Duration,
+};
+use tokio::time::sleep;
 
 #[derive(Debug)]
 pub(crate) struct Counts {
@@ -50,6 +54,15 @@ pub async fn update_counts(pool: &PgPool) -> Result<(), SqlxError> {
     Ok(())
 }
 
+// Just to make sure we don't miss any user/device/network count changes
+pub async fn run_periodic_count_update(pool: &PgPool) -> Result<(), SqlxError> {
+    let delay = Duration::from_secs(60 * 60);
+    loop {
+        update_counts(pool).await?;
+        sleep(delay).await;
+    }
+}
+
 impl Counts {
     pub(crate) fn is_over_limit(&self) -> bool {
         self.user > 5 || self.device > 10 || self.wireguard_network > 1

src/grpc/enrollment.rs

@@ -20,7 +20,7 @@ use crate::{
         },
         Device, GatewayEvent, Id, Settings, User,
     },
-    enterprise::db::models::enterprise_settings::EnterpriseSettings,
+    enterprise::{db::models::enterprise_settings::EnterpriseSettings, limits::update_counts},
     grpc::utils::build_device_config_response,
     handlers::{mail::send_new_device_added_email, user::check_password_strength},
     headers::get_device_info,
@@ -302,6 +302,7 @@ impl EnrollmentServer {
             Status::internal("unexpected error")
         })?;
         debug!("Updating user details ended with success.");
+        let _ = update_counts(&self.pool).await;
 
         // sync with LDAP
         debug!("Add user to ldap: {}.", self.ldap_feature_active);
@@ -468,6 +469,7 @@ impl EnrollmentServer {
             Status::internal("unexpected error")
         })?;
         info!("New device created: {device:?}.");
+        let _ = update_counts(&self.pool).await;
 
         debug!(
             "Adding device {} to all existing user networks for user {}({:?}).",

src/grpc/interceptor.rs

@@ -18,37 +18,57 @@ impl JwtInterceptor {
 
 impl Interceptor for JwtInterceptor {
     fn call(&mut self, mut req: tonic::Request<()>) -> Result<tonic::Request<()>, Status> {
+        // This is only used for logging purposes, so no proper error handling
+        let hostname = req
+            .metadata()
+            .get("hostname")
+            .map(|h| h.to_str().unwrap_or("UNKNOWN"))
+            .unwrap_or("UNKNOWN");
+
         let token = match req.metadata().get("authorization") {
-            Some(token) => token
-                .to_str()
-                .map_err(|_| Status::unauthenticated("Invalid token"))?,
+            Some(token) => token.to_str().map_err(|err| {
+                warn!("Failed to parse authorization header during handling gRPC request from hostname {}. \
+                If you recognize this hostname, there may be an issue with the token used for authorization. \
+                Cause of the failed parsing: {:?}", hostname, err);
+                Status::unauthenticated("Invalid token")
+            })?,
             None => return Err(Status::unauthenticated("Missing authorization header")),
         };
-        if let Ok(claims) = Claims::from_jwt(self.claims_type, token) {
-            let request_metadata = req.metadata_mut();
 
-            if let ClaimsType::Gateway = self.claims_type {
+        match Claims::from_jwt(self.claims_type, token) {
+            Ok(claims) => {
+                let request_metadata = req.metadata_mut();
+
+                if let ClaimsType::Gateway = self.claims_type {
+                    request_metadata.insert(
+                        "gateway_network_id",
+                        claims
+                            .client_id
+                            .parse()
+                            .map_err(|_| Status::unknown("Network ID parsing error"))?,
+                    );
+                }
+
+                // FIXME: can we push whole Claims object into metadata?
                 request_metadata.insert(
-                    "gateway_network_id",
+                    "username",
                     claims
-                        .client_id
+                        .sub
                         .parse()
-                        .map_err(|_| Status::unknown("Network ID parsing error"))?,
+                        .map_err(|_| Status::unknown("Username parsing error"))?,
                 );
+                debug!("Authorization successful for user {}", claims.sub);
+                Ok(req)
+            }
+            Err(err) => {
+                warn!(
+                    "Failed to authorize a gRPC request from hostname {}. \
+                    If you recognize this hostname, there may be an issue with the token used for authorization. \
+                    Cause of the failed authorization: {:?}",
+                    hostname, err
+                );
+                Err(Status::unauthenticated("Invalid token"))
             }
-
-            // FIXME: can we push whole Claims object into metadata?
-            request_metadata.insert(
-                "username",
-                claims
-                    .sub
-                    .parse()
-                    .map_err(|_| Status::unknown("Username parsing error"))?,
-            );
-            debug!("Authorization successful for user {}", claims.sub);
-            Ok(req)
-        } else {
-            Err(Status::unauthenticated("Invalid token"))
         }
     }
 }

src/handlers/settings.rs

@@ -21,7 +21,7 @@ use crate::{
 static DEFAULT_NAV_LOGO_URL: &str = "/svg/defguard-nav-logo.svg";
 static DEFAULT_MAIN_LOGO_URL: &str = "/svg/logo-defguard-white.svg";
 
-pub async fn get_settings(State(appstate): State<AppState>) -> ApiResult {
+pub async fn get_settings(_admin: AdminRole, State(appstate): State<AppState>) -> ApiResult {
     debug!("Retrieving settings");
     if let Some(mut settings) = Settings::get(&appstate.pool).await? {
         if settings.nav_logo_url.is_empty() {

src/handlers/user.rs

@@ -21,7 +21,7 @@ use crate::{
         AppEvent, GatewayEvent, OAuth2AuthorizedApp, Settings, User, UserDetails, UserInfo, Wallet,
         WebAuthn, WireguardNetwork,
     },
-    enterprise::limits::update_counts,
+    enterprise::{db::models::enterprise_settings::EnterpriseSettings, limits::update_counts},
     error::WebError,
     ldap::utils::{ldap_add_user, ldap_change_password, ldap_delete_user, ldap_modify_user},
     mail::Mail,
@@ -482,6 +482,13 @@ pub async fn start_remote_desktop_configuration(
         session.user.username
     );
 
+    let settings = EnterpriseSettings::get(&appstate.pool).await?;
+    if settings.admin_device_management && !session.is_admin {
+        return Err(WebError::Forbidden(
+            "Only admin users can manage devices".into(),
+        ));
+    }
+
     debug!("Verify that the user from the current session is an admin or only peforms desktop activation for self.");
     let user = user_for_admin_or_self(&appstate.pool, &session, &username).await?;
     debug!("Successfully fetched user data: {user:?}");

src/lib.rs

@@ -12,7 +12,7 @@ use axum::{
     serve, Extension, Json, Router,
 };
 use enterprise::handlers::{
-    check_enterprise_status,
+    check_enterprise_info, check_enterprise_status,
     enterprise_settings::{get_enterprise_settings, patch_enterprise_settings},
     openid_login::{auth_callback, get_auth_info},
     openid_providers::{add_openid_provider, delete_openid_provider, get_current_openid_provider},
@@ -414,7 +414,9 @@ pub fn build_webapp(
             .route("/callback", post(auth_callback))
             .route("/auth_info", get(get_auth_info)),
     );
-    let webapp = webapp.route("/api/v1/enterprise_status", get(check_enterprise_status));
+    let webapp = webapp
+        .route("/api/v1/enterprise_status", get(check_enterprise_status))
+        .route("/api/v1/enterprise_info", get(check_enterprise_info));
 
     #[cfg(feature = "openid")]
     let webapp = webapp

web/src/components/AppLoader.tsx

@@ -28,8 +28,8 @@ export const AppLoader = () => {
   const appSettings = useAppStore((state) => state.settings);
   const {
     getAppInfo,
-    getEnterpriseStatus,
     user: { getMe },
+    getEnterpriseStatus,
     settings: { getEssentialSettings, getEnterpriseSettings },
   } = useApi();
   const [userLoading, setUserLoading] = useState(true);
@@ -68,6 +68,18 @@ export const AppLoader = () => {
     enabled: !isUndefined(currentUser),
   });
 
+  useQuery([QueryKeys.FETCH_ENTERPRISE_SETTINGS], getEnterpriseSettings, {
+    onSuccess: (settings) => {
+      setAppStore({ enterprise_settings: settings });
+    },
+    onError: (err) => {
+      console.error(err);
+    },
+    refetchOnWindowFocus: true,
+    retry: false,
+    enabled: !isUndefined(currentUser),
+  });
+
   useQuery([QueryKeys.FETCH_ENTERPRISE_STATUS], getEnterpriseStatus, {
     onSuccess: (status) => {
       setAppStore({
@@ -83,16 +95,6 @@ export const AppLoader = () => {
     retry: false,
   });
 
-  useQuery([QueryKeys.FETCH_ENTERPRISE_SETTINGS], getEnterpriseSettings, {
-    onSuccess: (settings) => {
-      setAppStore({ enterprise_settings: settings });
-    },
-    onError: (err) => {
-      console.error(err);
-    },
-    refetchOnWindowFocus: true,
-    retry: false,
-  });
   const { isLoading: settingsLoading, data: essentialSettings } = useQuery(
     [QueryKeys.FETCH_ESSENTIAL_SETTINGS],
     getEssentialSettings,

web/src/pages/auth/AuthPage.tsx

@@ -116,7 +116,10 @@ export const AuthPage = () => {
           // check where to navigate administrator
           const appInfo = await getAppInfo();
           const settings = await getSettings();
-          setAppStore({ appInfo, settings });
+          setAppStore({
+            appInfo,
+            settings,
+          });
           if (settings.wireguard_enabled) {
             if (!appInfo?.network_present) {
               navigateURL = '/admin/wizard';

web/src/pages/settings/components/EnterpriseSettings/EnterpriseSettings.tsx

@@ -1,23 +1,40 @@
+import { useQuery } from '@tanstack/react-query';
 import parse from 'html-react-parser';
 
 import { useI18nContext } from '../../../../i18n/i18n-react';
 import { BigInfoBox } from '../../../../shared/defguard-ui/components/Layout/BigInfoBox/BigInfoBox';
-import { useAppStore } from '../../../../shared/hooks/store/useAppStore';
+import { LoaderSpinner } from '../../../../shared/defguard-ui/components/Layout/LoaderSpinner/LoaderSpinner';
+import useApi from '../../../../shared/hooks/useApi';
+import { QueryKeys } from '../../../../shared/queries';
 import { EnterpriseForm } from './components/EnterpriseForm';
 
 export const EnterpriseSettings = () => {
-  const enterpriseStatus = useAppStore((state) => state.enterprise_status);
   const { LL } = useI18nContext();
   const localLL = LL.settingsPage.enterpriseOnly;
+  const { getEnterpriseInfo } = useApi();
+  const { data: enterpriseInfo, isLoading } = useQuery({
+    queryFn: getEnterpriseInfo,
+    queryKey: [QueryKeys.FETCH_ENTERPRISE_INFO],
+    refetchOnMount: true,
+    refetchOnWindowFocus: false,
+  });
+  if (isLoading) {
+    return (
+      <div className="spinner-container">
+        <LoaderSpinner size={100} />
+      </div>
+    );
+  }
+
   return (
     <>
-      {!enterpriseStatus?.enabled && (
+      {!enterpriseInfo?.enabled && (
         <div className="enterprise-info-backdrop">
           <div className="enterprise-info">
             <div>
               <h2>{localLL.title()}</h2>
               {/* If enterprise is disabled but we have some license info, the license probably expired */}
-              {enterpriseStatus?.license_info && <p>{localLL.currentExpired()}</p>}
+              {enterpriseInfo?.license_info && <p>{localLL.currentExpired()}</p>}
               <p>
                 {localLL.subtitle()}{' '}
                 <a href="https://defguard.net/pricing/" target="_blank" rel="noreferrer">
@@ -29,7 +46,7 @@ export const EnterpriseSettings = () => {
           </div>
         </div>
       )}
-      {!enterpriseStatus?.needs_license && !enterpriseStatus?.license_info && (
+      {!enterpriseInfo?.needs_license && !enterpriseInfo?.license_info && (
         <div className="license-not-required-container">
           <BigInfoBox
             message={parse(LL.settingsPage.license.licenseInfo.licenseNotRequired())}