Fix logging in via oidc for the first time when exceeding limits

crates/defguard/src/main.rs

@@ -244,7 +244,12 @@ async fn main() -> Result<(), anyhow::Error> {
     let proxy_secret_key = settings.secret_key_required()?;
     let proxy_manager = ProxyManager::new(
         pool.clone(),
-        ProxyTxSet::new(gateway_tx.clone(), bidi_event_tx.clone(), ldap_tx.clone()),
+        ProxyTxSet::new(
+            gateway_tx.clone(),
+            bidi_event_tx.clone(),
+            ldap_tx.clone(),
+            api_event_tx.clone(),
+        ),
         Arc::clone(&incompatible_components),
         proxy_control_rx,
         proxy_secret_key,

crates/defguard_core/src/db/models/activity_log/metadata.rs

@@ -113,6 +113,14 @@ pub struct UserMetadata {
     pub user: UserNoSecrets,
 }
 
+#[derive(Serialize)]
+pub struct UserImportBlockedMetadata {
+    pub username: String,
+    pub email: String,
+    pub user_count: u32,
+    pub limit: u32,
+}
+
 #[derive(Serialize)]
 pub struct UserModifiedMetadata {
     pub before: UserNoSecrets,

crates/defguard_core/src/db/models/activity_log/mod.rs

@@ -45,6 +45,7 @@ pub enum EventType {
     MfaSecurityKeyRemoved,
     // User management
     UserAdded,
+    UserImportBlocked,
     UserRemoved,
     UserModified,
     UserGroupsModified,

crates/defguard_core/src/enterprise/grpc/desktop_client_mfa.rs

@@ -78,7 +78,7 @@ impl ClientMfaServer {
             return Err(Status::invalid_argument("invalid MFA method"));
         }
 
-        let (ip, _user_agent) = parse_client_ip_agent(&info).map_err(Status::internal)?;
+        let (ip, user_agent) = parse_client_ip_agent(&info).map_err(Status::internal)?;
         let context = BidiRequestContext::new(
             user.id,
             user.username.clone(),
@@ -114,7 +114,19 @@ impl ClientMfaServer {
             }
         };
 
-        match user_from_claims(&self.pool, Nonce::new(request.nonce.clone()), code, url).await {
+        // This path only re-verifies an already-existing user's identity via OpenID
+        // for MFA, so it never creates a new account, hence no `ApiEvent` channel.
+        match user_from_claims(
+            &self.pool,
+            Nonce::new(request.nonce.clone()),
+            code,
+            url,
+            Some(ip),
+            Some(&user_agent),
+            None,
+        )
+        .await
+        {
             Ok(claims_user) => {
                 // if thats not our user, prevent login
                 if claims_user.id != user.id {

crates/defguard_core/src/enterprise/handlers/openid_login.rs

@@ -1,3 +1,5 @@
+use std::net::IpAddr;
+
 use axum::{Json, extract::State, http::StatusCode};
 use axum_extra::{
     TypedHeader,
@@ -24,6 +26,7 @@ use reqwest::Url;
 use serde_json::json;
 use sqlx::PgPool;
 use time::Duration;
+use tokio::sync::mpsc::UnboundedSender;
 
 const COOKIE_MAX_AGE: Duration = Duration::days(1);
 static CSRF_COOKIE_NAME: &str = "csrf";
@@ -43,6 +46,7 @@ use crate::{
         limits::{get_counts, update_counts},
     },
     error::WebError,
+    events::{ApiEvent, ApiEventType, ApiRequestContext},
     handlers::{
         ApiResponse, AuthResponse, ClientIpAddr, SESSION_COOKIE_NAME, SIGN_IN_COOKIE_NAME,
         auth::create_session,
@@ -200,11 +204,20 @@ pub async fn make_oidc_client(
 }
 
 /// Get or create `User` from OpenID claims.
+///
+/// `ip_addr`/`user_agent`/`event_tx` are only used to emit an activity log event
+/// when account creation is blocked by the license user limit. Pass `None` for
+/// `event_tx` only if this call can never create a new account (e.g. the desktop
+/// client MFA flow, which merely re-verifies an existing user's identity);
+/// any caller that can create accounts should supply a real `ApiEvent` sender.
 pub async fn user_from_claims(
     pool: &PgPool,
     nonce: Nonce,
     code: AuthorizationCode,
     callback_url: Url,
+    ip_addr: Option<IpAddr>,
+    user_agent: Option<&str>,
+    event_tx: Option<&UnboundedSender<ApiEvent>>,
 ) -> Result<User<Id>, WebError> {
     let Some(provider) = OpenIdProvider::get_current(pool).await? else {
         return Err(WebError::ObjectNotFound("OpenID provider not set".into()));
@@ -410,6 +423,10 @@ pub async fn user_from_claims(
                 }
 
                 if let Some((user_count, limit)) = reached_user_license_limit() {
+                    // Details (username/email/counts) are recorded in the activity
+                    // log and admin notification email, but deliberately not
+                    // returned to the client, which only learns that it should
+                    // contact an administrator.
                     error!(
                         "Skipping OpenID account creation for user {username} (email: {}) because \
                         license user limit has been reached ({user_count}/{limit})",
@@ -421,7 +438,30 @@ pub async fn user_from_claims(
                             {err}"
                         );
                     }
-                    return Err(WebError::Forbidden("License limit reached."));
+                    if let Some(event_tx) = event_tx
+                        && let Err(err) = event_tx.send(ApiEvent {
+                            context: ApiRequestContext::new(
+                                None::<Id>,
+                                username.clone(),
+                                ip_addr,
+                                user_agent.unwrap_or_default().to_string(),
+                            ),
+                            event: Box::new(ApiEventType::UserImportBlocked {
+                                username: username.clone(),
+                                email: email.as_str().to_string(),
+                                user_count,
+                                limit,
+                            }),
+                        })
+                    {
+                        error!(
+                            "Failed to emit activity log event for blocked OpenID account \
+                            creation: {err}"
+                        );
+                    }
+                    return Err(WebError::LicenseLimitReached(
+                        "Could not log in. Please contact your administrator.".to_string(),
+                    ));
                 }
 
                 // Extract all necessary information from the token or call the userinfo endpoint.
@@ -634,6 +674,9 @@ pub async fn auth_callback(
         Nonce::new(cookie_nonce),
         payload.code,
         settings.callback_url()?,
+        Some(ip_addr),
+        Some(user_agent.as_str()),
+        Some(&appstate.event_tx),
     )
     .await?;
 

crates/defguard_core/src/error.rs

@@ -46,6 +46,8 @@ pub enum WebError {
     Authorization(String),
     #[error("User groups not synced: {0}")]
     UserGroupsNotSynced(String),
+    #[error("License limit reached: {0}")]
+    LicenseLimitReached(String),
     #[error("Authentication error")]
     Authentication,
     #[error("Forbidden error: {0}")]

crates/defguard_core/src/events.rs

@@ -31,7 +31,9 @@ use crate::{
 #[derive(Debug, Clone, PartialEq)]
 pub struct ApiRequestContext {
     pub timestamp: NaiveDateTime,
-    pub user_id: Id,
+    /// `None` for events about a user that doesn't have an account yet, e.g. an
+    /// OpenID login blocked before the corresponding account could be created.
+    pub user_id: Option<Id>,
     pub username: String,
     pub ip: Option<IpAddr>,
     pub device: String,
@@ -40,15 +42,15 @@ pub struct ApiRequestContext {
 impl ApiRequestContext {
     #[must_use]
     pub fn new(
-        user_id: Id,
+        user_id: impl Into<Option<Id>>,
         username: String,
         ip: impl Into<Option<IpAddr>>,
         device: String,
     ) -> Self {
         let timestamp = Utc::now().naive_utc();
         Self {
             timestamp,
-            user_id,
+            user_id: user_id.into(),
             username,
             ip: ip.into(),
             device,
@@ -133,6 +135,14 @@ pub enum ApiEventType {
     UserAdded {
         user: User<Id>,
     },
+    /// Account auto-provisioning (e.g. via OpenID or LDAP) was blocked because it
+    /// would have exceeded the license user limit.
+    UserImportBlocked {
+        username: String,
+        email: String,
+        user_count: u32,
+        limit: u32,
+    },
     UserRemoved {
         user: User<Id>,
     },

crates/defguard_core/src/handlers/mod.rs

@@ -67,6 +67,7 @@ pub(crate) mod yubikey;
 pub enum WebErrorCode {
     NetworkFull,
     UserGroupsNotSynced,
+    LicenseLimitReached,
     CertMissingCertPem,
     CertMissingKeyPem,
     CertInvalidCertOrKey,
@@ -303,6 +304,13 @@ impl From<WebError> for ApiResponse {
                     StatusCode::UNAUTHORIZED,
                 )
             }
+            WebError::LicenseLimitReached(msg) => {
+                warn!(msg);
+                Self::new(
+                    json!({"msg": msg, "code": WebErrorCode::LicenseLimitReached}),
+                    StatusCode::FORBIDDEN,
+                )
+            }
             WebError::CertMissingCertPem => Self::new(
                 json!({"msg": web_error.to_string(), "code": WebErrorCode::CertMissingCertPem}),
                 StatusCode::BAD_REQUEST,

crates/defguard_core/tests/integration/api/common/client.rs

@@ -193,7 +193,8 @@ impl TestClient {
                 "Event type mismatch at index {index}: expected {expected_event:?}, got {event:?}",
             );
             assert_eq!(
-                expected_user_id, user_id,
+                Some(*expected_user_id),
+                *user_id,
                 "User ID mismatch at index {index}: expected {expected_user_id:?}, got {user_id:?}",
             );
             assert_eq!(
@@ -206,7 +207,7 @@ impl TestClient {
     /// Receive all messages currently present in API event queue
     ///
     /// Can also be used to clear the queue.
-    pub fn drain_all_events(&mut self) -> Vec<(ApiEventType, Id, String)> {
+    pub fn drain_all_events(&mut self) -> Vec<(ApiEventType, Option<Id>, String)> {
         let mut all_events = Vec::new();
 
         loop {

crates/defguard_event_logger/src/description.rs

@@ -56,6 +56,15 @@ pub fn get_api_event_description(event: &ApiEventType) -> Option<String> {
                 user.email
             ))
         }
+        ApiEventType::UserImportBlocked {
+            username,
+            email,
+            user_count,
+            limit,
+        } => Some(format!(
+            "Blocked automatic creation of account for user {username} (email: {email}) \
+            because the license user limit has been reached ({user_count}/{limit})"
+        )),
         ApiEventType::UserRemoved { user } => Some(format!("Removed user {user}")),
         ApiEventType::UserModified { before, after } => {
             let mut description = format!("Modified user {after}");

crates/defguard_event_logger/src/lib.rs

@@ -18,10 +18,11 @@ use defguard_core::{
             OpenIdAppModifiedMetadata, OpenIdAppStateChangedMetadata, OpenIdProviderMetadata,
             PasswordChangedByAdminMetadata, PasswordResetMetadata, ProxyDeletedMetadata,
             ProxyModifiedMetadata, SettingsUpdateMetadata, UserGroupsModifiedMetadata,
-            UserMetadata, UserMfaDisabledMetadata, UserModifiedMetadata, UserSnatBindingMetadata,
-            UserSnatBindingModifiedMetadata, VpnClientMetadata, VpnClientMfaFailedMetadata,
-            VpnClientMfaMetadata, VpnLocationMetadata, VpnLocationModifiedMetadata,
-            WebHookMetadata, WebHookModifiedMetadata, WebHookStateChangedMetadata,
+            UserImportBlockedMetadata, UserMetadata, UserMfaDisabledMetadata, UserModifiedMetadata,
+            UserSnatBindingMetadata, UserSnatBindingModifiedMetadata, VpnClientMetadata,
+            VpnClientMfaFailedMetadata, VpnClientMfaMetadata, VpnLocationMetadata,
+            VpnLocationModifiedMetadata, WebHookMetadata, WebHookModifiedMetadata,
+            WebHookStateChangedMetadata,
         },
     },
     events::{
@@ -332,6 +333,21 @@ fn map_to_activity_log_event(message: EventLoggerMessage) -> ActivityLogEvent<No
                     EventType::UserAdded,
                     serde_json::to_value(UserMetadata { user: user.into() }).ok(),
                 ),
+                ApiEventType::UserImportBlocked {
+                    username,
+                    email,
+                    user_count,
+                    limit,
+                } => (
+                    EventType::UserImportBlocked,
+                    serde_json::to_value(UserImportBlockedMetadata {
+                        username,
+                        email,
+                        user_count,
+                        limit,
+                    })
+                    .ok(),
+                ),
                 ApiEventType::UserRemoved { user } => (
                     EventType::UserRemoved,
                     serde_json::to_value(UserMetadata { user: user.into() }).ok(),

crates/defguard_event_logger/src/message.rs

@@ -155,7 +155,7 @@ impl EventContext {
 
         Self {
             timestamp: val.timestamp,
-            user_id: Some(val.user_id),
+            user_id: val.user_id,
             username: val.username,
             location,
             ip: val.ip,

crates/defguard_event_logger/src/tests/mod.rs

@@ -562,6 +562,18 @@ fn api_event_cases() -> Vec<EventTestCase> {
             module: ActivityLogModule::Defguard,
             description_contains: Some("Added"),
         },
+        EventTestCase {
+            name: "UserImportBlocked",
+            message: api_message(ApiEventType::UserImportBlocked {
+                username: "testuser".to_string(),
+                email: "testuser@example.com".to_string(),
+                user_count: 10,
+                limit: 10,
+            }),
+            event_type: EventType::UserImportBlocked,
+            module: ActivityLogModule::Defguard,
+            description_contains: Some("Blocked"),
+        },
         EventTestCase {
             name: "UserRemoved",
             message: api_message(ApiEventType::UserRemoved { user: user.clone() }),

crates/defguard_proxy_manager/src/handler.rs

@@ -31,7 +31,7 @@ use defguard_core::{
         ldap::utils::ldap_update_user_state,
     },
     error::WebError,
-    events::LdapSyncEventType,
+    events::{ApiEvent, LdapSyncEventType},
     grpc::{
         GatewayCommand,
         proxy::client_mfa::{
@@ -879,6 +879,9 @@ impl ProxyHandler {
                                         Nonce::new(request.nonce),
                                         code,
                                         callback_url,
+                                        None,
+                                        None,
+                                        Some(&self.services.event_tx),
                                     )
                                     .await
                                     {
@@ -952,6 +955,9 @@ impl ProxyHandler {
                                                 WebError::BadRequest(message) => {
                                                     (Code::InvalidArgument as i32, message)
                                                 }
+                                                WebError::LicenseLimitReached(message) => {
+                                                    (Code::ResourceExhausted as i32, message)
+                                                }
                                                 _ => (
                                                     Code::Internal as i32,
                                                     "OpenID authentication failed".to_owned(),
@@ -1231,6 +1237,7 @@ struct ProxyServices {
     client_mfa: ClientMfaServer,
     polling: PollingServer,
     ldap: UnboundedSender<LdapSyncEventType>,
+    event_tx: UnboundedSender<ApiEvent>,
 }
 
 impl ProxyServices {
@@ -1263,6 +1270,7 @@ impl ProxyServices {
             client_mfa,
             polling,
             ldap: tx.ldap.clone(),
+            event_tx: tx.event_tx.clone(),
         }
     }
 }