Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

3 changes: 2 additions & 1 deletion crates/defguard_core/src/db/models/activity_log/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -130,7 +130,8 @@ pub struct ActivityLogEvent<I = NoId> {
pub user_id: Id,
pub username: String,
pub location: Option<String>,
pub ip: IpNetwork,
#[model(option)]
pub ip: Option<IpNetwork>,
#[model(enum)]
pub event: EventType,
#[model(enum)]
Expand Down
28 changes: 19 additions & 9 deletions crates/defguard_core/src/events.rs
Original file line number Diff line number Diff line change
Expand Up @@ -28,19 +28,24 @@ pub struct ApiRequestContext {
pub timestamp: NaiveDateTime,
pub user_id: Id,
pub username: String,
pub ip: IpAddr,
pub ip: Option<IpAddr>,
pub device: String,
}

impl ApiRequestContext {
#[must_use]
pub fn new(user_id: Id, username: String, ip: IpAddr, device: String) -> Self {
pub fn new(
user_id: Id,
username: String,
ip: impl Into<Option<IpAddr>>,
device: String,
) -> Self {
let timestamp = Utc::now().naive_utc();
Self {
timestamp,
user_id,
username,
ip,
ip: ip.into(),
device,
}
}
Expand All @@ -54,7 +59,7 @@ pub struct GrpcRequestContext {
pub timestamp: NaiveDateTime,
pub user_id: Id,
pub username: String,
pub ip: IpAddr,
pub ip: Option<IpAddr>,
pub device_id: Id,
pub device_name: String,
pub location: WireguardNetwork<Id>,
Expand All @@ -65,7 +70,7 @@ impl GrpcRequestContext {
pub fn new(
user_id: Id,
username: String,
ip: IpAddr,
ip: impl Into<Option<IpAddr>>,
device_id: Id,
device_name: String,
location: WireguardNetwork<Id>,
Expand All @@ -75,7 +80,7 @@ impl GrpcRequestContext {
timestamp,
user_id,
username,
ip,
ip: ip.into(),
device_id,
device_name,
location,
Expand Down Expand Up @@ -328,19 +333,24 @@ pub struct BidiRequestContext {
pub timestamp: NaiveDateTime,
pub user_id: Id,
pub username: String,
pub ip: IpAddr,
pub ip: Option<IpAddr>,
pub device_name: String,
}

impl BidiRequestContext {
#[must_use]
pub fn new(user_id: Id, username: String, ip: IpAddr, device_name: String) -> Self {
pub fn new(
user_id: Id,
username: String,
ip: impl Into<Option<IpAddr>>,
device_name: String,
) -> Self {
let timestamp = Utc::now().naive_utc();
Self {
timestamp,
user_id,
username,
ip,
ip: ip.into(),
device_name,
}
}
Expand Down
2 changes: 1 addition & 1 deletion crates/defguard_core/src/handlers/activity_log.rs
Original file line number Diff line number Diff line change
Expand Up @@ -105,7 +105,7 @@ pub struct ApiActivityLogEvent {
pub user_id: Id,
pub username: String,
pub location: Option<String>,
pub ip: IpNetwork,
pub ip: Option<IpNetwork>,
pub event: String,
pub module: ActivityLogModule,
pub device: String,
Expand Down
72 changes: 72 additions & 0 deletions crates/defguard_core/tests/integration/api/activity_log.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,72 @@
use chrono::{TimeDelta, Utc};
use defguard_common::db::{NoId, setup_pool};
use defguard_core::db::models::activity_log::{ActivityLogEvent, ActivityLogModule, EventType};
use reqwest::StatusCode;
use serde::Deserialize;
use sqlx::postgres::{PgConnectOptions, PgPoolOptions};

use super::common::{get_db_user, make_client_with_db};

#[derive(Deserialize)]
struct PaginatedResponse<T> {
data: Vec<T>,
}

#[derive(Deserialize)]
struct ApiActivityLogEvent {
id: i64,
ip: Option<String>,
description: Option<String>,
}

#[sqlx::test]
async fn test_activity_log_persists_and_returns_null_ip(
_: PgPoolOptions,
options: PgConnectOptions,
) {
let pool = setup_pool(options).await;
let (mut client, db) = make_client_with_db(pool.clone()).await;
let admin = get_db_user(&db, "admin").await;

client.login_user("admin", "pass123").await;

let marker = format!(
"nullable-ip-{}",
Utc::now().timestamp_nanos_opt().unwrap_or_default()
);
let saved_event = ActivityLogEvent {
id: NoId,
timestamp: Utc::now().naive_utc() + TimeDelta::seconds(5),
user_id: admin.id,
username: admin.username,
location: None,
ip: None,
event: EventType::UserLogout,
module: ActivityLogModule::Defguard,
device: "integration-test".to_string(),
description: Some(marker.clone()),
metadata: None,
}
.save(&db)
.await
.expect("activity log event with null ip should persist");

let fetched_event = ActivityLogEvent::find_by_id(&db, saved_event.id)
.await
.expect("activity log event query should succeed")
.expect("saved activity log event should exist");
assert_eq!(fetched_event.ip, None);

let response = client.get("/api/v1/activity_log").send().await;
assert_eq!(response.status(), StatusCode::OK);

let payload: PaginatedResponse<ApiActivityLogEvent> = response.json().await;
let api_event = payload
.data
.into_iter()
.find(|event| event.id == saved_event.id)
.expect("activity log endpoint should return saved event");

assert_eq!(api_event.description.as_deref(), Some(marker.as_str()));
assert_eq!(api_event.ip, None);
}
1 change: 1 addition & 0 deletions crates/defguard_core/tests/integration/api/mod.rs
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
mod acl;
mod activity_log;
mod api_tokens;
mod auth;
mod common;
Expand Down
32 changes: 31 additions & 1 deletion crates/defguard_event_logger/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -573,7 +573,7 @@ pub async fn run_event_logger(
user_id,
username,
location,
ip: ip.into(),
ip: ip.map(Into::into),
event,
module,
device,
Expand Down Expand Up @@ -610,3 +610,33 @@ pub async fn run_event_logger(
transaction.commit().await?;
}
}

#[cfg(test)]
mod tests {
use chrono::Utc;
use defguard_common::db::NoId;
use serde_json::Value;

use super::*;

#[test]
fn activity_log_event_serialization_supports_null_ip() {
let event = ActivityLogEvent {
id: NoId,
timestamp: Utc::now().naive_utc(),
user_id: 1,
username: "admin".to_string(),
location: None,
ip: None,
event: EventType::UserLogin,
module: ActivityLogModule::Defguard,
device: "test-device".to_string(),
description: None,
metadata: None,
};

let serialized = serde_json::to_value(event).expect("activity log event should serialize");

assert_eq!(serialized.get("ip"), Some(&Value::Null));
}
}
2 changes: 1 addition & 1 deletion crates/defguard_event_logger/src/message.rs
Original file line number Diff line number Diff line change
Expand Up @@ -44,7 +44,7 @@ pub struct EventContext {
pub user_id: Id,
pub username: String,
pub location: Option<String>,
pub ip: IpAddr,
pub ip: Option<IpAddr>,
pub device: String,
}

Expand Down
2 changes: 1 addition & 1 deletion crates/defguard_session_manager/src/events.rs
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@ pub struct SessionManagerEventContext {
pub location: WireguardNetwork<Id>,
pub user: User<Id>,
pub device: Device<Id>,
pub public_ip: IpAddr,
pub public_ip: Option<IpAddr>,
}

#[derive(Debug)]
Expand Down
5 changes: 1 addition & 4 deletions crates/defguard_session_manager/src/lib.rs
Original file line number Diff line number Diff line change
@@ -1,5 +1,3 @@
use std::net::{IpAddr, Ipv4Addr};

use chrono::Utc;
use defguard_common::{
db::{
Expand Down Expand Up @@ -312,8 +310,7 @@ impl SessionManager {
location: location.clone(),
user,
device,
// FIXME: this is a workaround since we require an IP for each audit log event
public_ip: IpAddr::V4(Ipv4Addr::UNSPECIFIED),
public_ip: None,
};
let event = SessionManagerEvent {
context,
Expand Down
2 changes: 1 addition & 1 deletion crates/defguard_session_manager/src/session_state.rs
Original file line number Diff line number Diff line change
Expand Up @@ -337,7 +337,7 @@ impl ActiveSessionsMap {
location,
user,
device,
public_ip,
public_ip: Some(public_ip),
};
let event = SessionManagerEvent {
context,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -55,7 +55,7 @@ async fn test_session_manager_emits_connected_event_for_first_stats(
assert_eq!(event.context.location.id, location.id);
assert_eq!(event.context.user.id, user.id);
assert_eq!(event.context.device.id, device.id);
assert_eq!(event.context.public_ip, endpoint.ip());
assert_eq!(event.context.public_ip, Some(endpoint.ip()));
}

#[sqlx::test]
Expand Down Expand Up @@ -149,6 +149,7 @@ async fn test_session_manager_emits_disconnect_event_for_inactive_standard_sessi
assert_eq!(event.context.location.id, location.id);
assert_eq!(event.context.user.id, user.id);
assert_eq!(event.context.device.id, device.id);
assert_eq!(event.context.public_ip, None);

let disconnected_session = VpnClientSession::find_by_id(&pool, session.id)
.await
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -144,7 +144,7 @@ async fn test_duplicate_stats_in_same_batch_reuse_existing_session(
assert_eq!(connected_event.context.location.id, location.id);
assert_eq!(connected_event.context.user.id, user.id);
assert_eq!(connected_event.context.device.id, device.id);
assert_eq!(connected_event.context.public_ip, endpoint.ip());
assert_eq!(connected_event.context.public_ip, Some(endpoint.ip()));
assert_no_session_manager_events(&mut harness);
assert_no_gateway_events(&mut harness);

Expand Down Expand Up @@ -221,7 +221,7 @@ async fn test_duplicate_stats_across_iterations_reuse_existing_session(
assert_eq!(connected_event.context.location.id, location.id);
assert_eq!(connected_event.context.user.id, user.id);
assert_eq!(connected_event.context.device.id, device.id);
assert_eq!(connected_event.context.public_ip, endpoint.ip());
assert_eq!(connected_event.context.public_ip, Some(endpoint.ip()));
assert_no_session_manager_events(&mut harness);
assert_no_gateway_events(&mut harness);

Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
UPDATE activity_log_event
SET ip = '0.0.0.0'::inet
WHERE ip IS NULL;

ALTER TABLE activity_log_event ALTER COLUMN ip SET NOT NULL;
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
ALTER TABLE activity_log_event ALTER COLUMN ip DROP NOT NULL;
Loading
Loading