diff --git a/.sqlx/query-973c64873ac510cc407d43708efaaa1f93553237be1c6767564c2499b9d8f67d.json b/.sqlx/query-008165e46a6cdb8ce9512ec87c79f095627621f148d092f5bf596e637eb7cfca.json similarity index 57% rename from .sqlx/query-973c64873ac510cc407d43708efaaa1f93553237be1c6767564c2499b9d8f67d.json rename to .sqlx/query-008165e46a6cdb8ce9512ec87c79f095627621f148d092f5bf596e637eb7cfca.json index 304137add8..9f531abb81 100644 --- a/.sqlx/query-973c64873ac510cc407d43708efaaa1f93553237be1c6767564c2499b9d8f67d.json +++ b/.sqlx/query-008165e46a6cdb8ce9512ec87c79f095627621f148d092f5bf596e637eb7cfca.json @@ -1,6 +1,6 @@ { "db_name": "PostgreSQL", - "query": "UPDATE \"vpn_client_session\" SET \"location_id\" = $2,\"user_id\" = $3,\"device_id\" = $4,\"created_at\" = $5,\"connected_at\" = $6,\"disconnected_at\" = $7,\"mfa_method\" = $8,\"state\" = $9,\"preshared_key\" = $10 WHERE id = $1", + "query": "UPDATE \"vpn_client_session\" SET \"location_id\" = $2,\"user_id\" = $3,\"device_id\" = $4,\"created_at\" = $5,\"connected_at\" = $6,\"disconnected_at\" = $7,\"is_mfa_session\" = $8,\"state\" = $9,\"preshared_key\" = $10 WHERE id = $1", "describe": { "columns": [], "parameters": { @@ -12,20 +12,7 @@ "Timestamp", "Timestamp", "Timestamp", - { - "Custom": { - "name": "vpn_client_mfa_method", - "kind": { - "Enum": [ - "totp", - "email", - "oidc", - "biometric", - "mobileapprove" - ] - } - } - }, + "Bool", { "Custom": { "name": "vpn_client_session_state", @@ -43,5 +30,5 @@ }, "nullable": [] }, - "hash": "973c64873ac510cc407d43708efaaa1f93553237be1c6767564c2499b9d8f67d" + "hash": "008165e46a6cdb8ce9512ec87c79f095627621f148d092f5bf596e637eb7cfca" } diff --git a/.sqlx/query-04f3b04ee13d7bde1ab05f673ba7c07bca2ba859485cfcf0100863d4c5c58d72.json b/.sqlx/query-04f3b04ee13d7bde1ab05f673ba7c07bca2ba859485cfcf0100863d4c5c58d72.json new file mode 100644 index 0000000000..2511eaa73f --- /dev/null +++ b/.sqlx/query-04f3b04ee13d7bde1ab05f673ba7c07bca2ba859485cfcf0100863d4c5c58d72.json @@ -0,0 +1,88 @@ +{ + "db_name": "PostgreSQL", + "query": "SELECT s.id, location_id, user_id, device_id, created_at, s.connected_at, disconnected_at, is_mfa_session, state \"state: VpnClientSessionState\", preshared_key FROM vpn_client_session s LEFT JOIN LATERAL ( SELECT latest_handshake FROM vpn_session_stats WHERE session_id = s.id ORDER BY latest_handshake DESC LIMIT 1 ) ss ON true WHERE location_id = $1 AND state = 'connected' AND (NOW() - ss.latest_handshake) > $2 * interval '1 second'", + "describe": { + "columns": [ + { + "ordinal": 0, + "name": "id", + "type_info": "Int8" + }, + { + "ordinal": 1, + "name": "location_id", + "type_info": "Int8" + }, + { + "ordinal": 2, + "name": "user_id", + "type_info": "Int8" + }, + { + "ordinal": 3, + "name": "device_id", + "type_info": "Int8" + }, + { + "ordinal": 4, + "name": "created_at", + "type_info": "Timestamp" + }, + { + "ordinal": 5, + "name": "connected_at", + "type_info": "Timestamp" + }, + { + "ordinal": 6, + "name": "disconnected_at", + "type_info": "Timestamp" + }, + { + "ordinal": 7, + "name": "is_mfa_session", + "type_info": "Bool" + }, + { + "ordinal": 8, + "name": "state: VpnClientSessionState", + "type_info": { + "Custom": { + "name": "vpn_client_session_state", + "kind": { + "Enum": [ + "new", + "connected", + "disconnected" + ] + } + } + } + }, + { + "ordinal": 9, + "name": "preshared_key", + "type_info": "Text" + } + ], + "parameters": { + "Left": [ + "Int8", + "Float8" + ] + }, + "nullable": [ + false, + false, + false, + false, + false, + true, + true, + false, + false, + true + ] + }, + "hash": "04f3b04ee13d7bde1ab05f673ba7c07bca2ba859485cfcf0100863d4c5c58d72" +} diff --git a/.sqlx/query-723c4cadc9212c4c64171642a6c16bb0e2a54479b73b3e543483407ca70be765.json b/.sqlx/query-0ba0e5b745e3b583d16f62e01504a3ced793ec0f35cfc69917ff170e2ce7c970.json similarity index 71% rename from .sqlx/query-723c4cadc9212c4c64171642a6c16bb0e2a54479b73b3e543483407ca70be765.json rename to .sqlx/query-0ba0e5b745e3b583d16f62e01504a3ced793ec0f35cfc69917ff170e2ce7c970.json index ee3fa934e6..0de9af7b47 100644 --- a/.sqlx/query-723c4cadc9212c4c64171642a6c16bb0e2a54479b73b3e543483407ca70be765.json +++ b/.sqlx/query-0ba0e5b745e3b583d16f62e01504a3ced793ec0f35cfc69917ff170e2ce7c970.json @@ -1,6 +1,6 @@ { "db_name": "PostgreSQL", - "query": "SELECT id, \"location_id\",\"user_id\",\"device_id\",\"created_at\",\"connected_at\",\"disconnected_at\",\"mfa_method\" \"mfa_method?: _\",\"state\" \"state: _\",\"preshared_key\" FROM \"vpn_client_session\" LIMIT $1 OFFSET $2", + "query": "SELECT id, \"location_id\",\"user_id\",\"device_id\",\"created_at\",\"connected_at\",\"disconnected_at\",\"is_mfa_session\",\"state\" \"state: _\",\"preshared_key\" FROM \"vpn_client_session\" LIMIT $1 OFFSET $2", "describe": { "columns": [ { @@ -40,21 +40,8 @@ }, { "ordinal": 7, - "name": "mfa_method?: _", - "type_info": { - "Custom": { - "name": "vpn_client_mfa_method", - "kind": { - "Enum": [ - "totp", - "email", - "oidc", - "biometric", - "mobileapprove" - ] - } - } - } + "name": "is_mfa_session", + "type_info": "Bool" }, { "ordinal": 8, @@ -92,10 +79,10 @@ false, true, true, - true, + false, false, true ] }, - "hash": "723c4cadc9212c4c64171642a6c16bb0e2a54479b73b3e543483407ca70be765" + "hash": "0ba0e5b745e3b583d16f62e01504a3ced793ec0f35cfc69917ff170e2ce7c970" } diff --git a/.sqlx/query-16829102fa71c7e4d458c2111aef2a3b7b6c151b5c3c2de57abf5299d53d8477.json b/.sqlx/query-16829102fa71c7e4d458c2111aef2a3b7b6c151b5c3c2de57abf5299d53d8477.json new file mode 100644 index 0000000000..4225813af9 --- /dev/null +++ b/.sqlx/query-16829102fa71c7e4d458c2111aef2a3b7b6c151b5c3c2de57abf5299d53d8477.json @@ -0,0 +1,23 @@ +{ + "db_name": "PostgreSQL", + "query": "DELETE FROM vpn_client_mfa_session WHERE location_id = $1 AND device_id = $2 RETURNING token_hash", + "describe": { + "columns": [ + { + "ordinal": 0, + "name": "token_hash", + "type_info": "Text" + } + ], + "parameters": { + "Left": [ + "Int8", + "Int8" + ] + }, + "nullable": [ + false + ] + }, + "hash": "16829102fa71c7e4d458c2111aef2a3b7b6c151b5c3c2de57abf5299d53d8477" +} diff --git a/.sqlx/query-22cb3d2e4f8f3e8bf6e62bcaa5f0ce3219b991dc82c085d8bf334b13fec6107b.json b/.sqlx/query-22cb3d2e4f8f3e8bf6e62bcaa5f0ce3219b991dc82c085d8bf334b13fec6107b.json new file mode 100644 index 0000000000..3b18f9e8c6 --- /dev/null +++ b/.sqlx/query-22cb3d2e4f8f3e8bf6e62bcaa5f0ce3219b991dc82c085d8bf334b13fec6107b.json @@ -0,0 +1,12 @@ +{ + "db_name": "PostgreSQL", + "query": "DELETE FROM vpn_client_mfa_session WHERE expires_at < (now() AT TIME ZONE 'UTC')", + "describe": { + "columns": [], + "parameters": { + "Left": [] + }, + "nullable": [] + }, + "hash": "22cb3d2e4f8f3e8bf6e62bcaa5f0ce3219b991dc82c085d8bf334b13fec6107b" +} diff --git a/.sqlx/query-9cd4fb6b8bb2f231d136f7e02bb9e0f094c419e60e3f8d8bba86f9df5b7d4c9f.json b/.sqlx/query-2e8eaeb9529ef248b1e1ef6a2f8d047f2a634bdda2727ffb7e2967e19eda4d33.json similarity index 68% rename from .sqlx/query-9cd4fb6b8bb2f231d136f7e02bb9e0f094c419e60e3f8d8bba86f9df5b7d4c9f.json rename to .sqlx/query-2e8eaeb9529ef248b1e1ef6a2f8d047f2a634bdda2727ffb7e2967e19eda4d33.json index d5be1a7441..5b7c3e265f 100644 --- a/.sqlx/query-9cd4fb6b8bb2f231d136f7e02bb9e0f094c419e60e3f8d8bba86f9df5b7d4c9f.json +++ b/.sqlx/query-2e8eaeb9529ef248b1e1ef6a2f8d047f2a634bdda2727ffb7e2967e19eda4d33.json @@ -1,6 +1,6 @@ { "db_name": "PostgreSQL", - "query": "SELECT id, location_id, user_id, device_id, created_at, connected_at, disconnected_at, mfa_method \"mfa_method: VpnClientMfaMethod\", state \"state: VpnClientSessionState\", preshared_key FROM vpn_client_session WHERE location_id = $1 AND state = 'connected'::vpn_client_session_state", + "query": "SELECT id, location_id, user_id, device_id, created_at, connected_at, disconnected_at, is_mfa_session, state \"state: VpnClientSessionState\", preshared_key FROM vpn_client_session WHERE location_id = $1 AND state = 'connected'::vpn_client_session_state", "describe": { "columns": [ { @@ -40,21 +40,8 @@ }, { "ordinal": 7, - "name": "mfa_method: VpnClientMfaMethod", - "type_info": { - "Custom": { - "name": "vpn_client_mfa_method", - "kind": { - "Enum": [ - "totp", - "email", - "oidc", - "biometric", - "mobileapprove" - ] - } - } - } + "name": "is_mfa_session", + "type_info": "Bool" }, { "ordinal": 8, @@ -91,10 +78,10 @@ false, true, true, - true, + false, false, true ] }, - "hash": "9cd4fb6b8bb2f231d136f7e02bb9e0f094c419e60e3f8d8bba86f9df5b7d4c9f" + "hash": "2e8eaeb9529ef248b1e1ef6a2f8d047f2a634bdda2727ffb7e2967e19eda4d33" } diff --git a/.sqlx/query-675fe2562e81e9886d488ce639a8c6a9b3fa7d140b30bfee7657e3eacf6de6aa.json b/.sqlx/query-34c62f2a550fe73b47be4fea1758f6b172e693dcc4ceb0d7cf995ad438aa3cac.json similarity index 71% rename from .sqlx/query-675fe2562e81e9886d488ce639a8c6a9b3fa7d140b30bfee7657e3eacf6de6aa.json rename to .sqlx/query-34c62f2a550fe73b47be4fea1758f6b172e693dcc4ceb0d7cf995ad438aa3cac.json index 5583f37d4e..1c58a2eeb5 100644 --- a/.sqlx/query-675fe2562e81e9886d488ce639a8c6a9b3fa7d140b30bfee7657e3eacf6de6aa.json +++ b/.sqlx/query-34c62f2a550fe73b47be4fea1758f6b172e693dcc4ceb0d7cf995ad438aa3cac.json @@ -1,6 +1,6 @@ { "db_name": "PostgreSQL", - "query": "SELECT id, \"location_id\",\"user_id\",\"device_id\",\"created_at\",\"connected_at\",\"disconnected_at\",\"mfa_method\" \"mfa_method?: _\",\"state\" \"state: _\",\"preshared_key\" FROM \"vpn_client_session\"", + "query": "SELECT id, \"location_id\",\"user_id\",\"device_id\",\"created_at\",\"connected_at\",\"disconnected_at\",\"is_mfa_session\",\"state\" \"state: _\",\"preshared_key\" FROM \"vpn_client_session\"", "describe": { "columns": [ { @@ -40,21 +40,8 @@ }, { "ordinal": 7, - "name": "mfa_method?: _", - "type_info": { - "Custom": { - "name": "vpn_client_mfa_method", - "kind": { - "Enum": [ - "totp", - "email", - "oidc", - "biometric", - "mobileapprove" - ] - } - } - } + "name": "is_mfa_session", + "type_info": "Bool" }, { "ordinal": 8, @@ -89,10 +76,10 @@ false, true, true, - true, + false, false, true ] }, - "hash": "675fe2562e81e9886d488ce639a8c6a9b3fa7d140b30bfee7657e3eacf6de6aa" + "hash": "34c62f2a550fe73b47be4fea1758f6b172e693dcc4ceb0d7cf995ad438aa3cac" } diff --git a/.sqlx/query-364ce80371bf5213dc01aa9361ae65e142c80ef0bac668106f23f18baa82e4b7.json b/.sqlx/query-364ce80371bf5213dc01aa9361ae65e142c80ef0bac668106f23f18baa82e4b7.json new file mode 100644 index 0000000000..cbadeb5347 --- /dev/null +++ b/.sqlx/query-364ce80371bf5213dc01aa9361ae65e142c80ef0bac668106f23f18baa82e4b7.json @@ -0,0 +1,80 @@ +{ + "db_name": "PostgreSQL", + "query": "SELECT id, \"token_hash\",\"location_id\",\"device_id\",\"user_id\",\"steps_snapshot\" \"steps_snapshot: _\",\"current_step\",\"ephemeral_state\" \"ephemeral_state: _\",\"failed_attempts\",\"created_at\",\"expires_at\" FROM \"vpn_client_mfa_session\"", + "describe": { + "columns": [ + { + "ordinal": 0, + "name": "id", + "type_info": "Int8" + }, + { + "ordinal": 1, + "name": "token_hash", + "type_info": "Text" + }, + { + "ordinal": 2, + "name": "location_id", + "type_info": "Int8" + }, + { + "ordinal": 3, + "name": "device_id", + "type_info": "Int8" + }, + { + "ordinal": 4, + "name": "user_id", + "type_info": "Int8" + }, + { + "ordinal": 5, + "name": "steps_snapshot: _", + "type_info": "Jsonb" + }, + { + "ordinal": 6, + "name": "current_step", + "type_info": "Int4" + }, + { + "ordinal": 7, + "name": "ephemeral_state: _", + "type_info": "Jsonb" + }, + { + "ordinal": 8, + "name": "failed_attempts", + "type_info": "Int4" + }, + { + "ordinal": 9, + "name": "created_at", + "type_info": "Timestamp" + }, + { + "ordinal": 10, + "name": "expires_at", + "type_info": "Timestamp" + } + ], + "parameters": { + "Left": [] + }, + "nullable": [ + false, + false, + false, + false, + false, + false, + false, + true, + false, + false, + false + ] + }, + "hash": "364ce80371bf5213dc01aa9361ae65e142c80ef0bac668106f23f18baa82e4b7" +} diff --git a/.sqlx/query-3f9af4ef27a9865648c24b1fb4f34670ce43deb3af46fbabebf23c8be9d54357.json b/.sqlx/query-3f9af4ef27a9865648c24b1fb4f34670ce43deb3af46fbabebf23c8be9d54357.json new file mode 100644 index 0000000000..990cc869f8 --- /dev/null +++ b/.sqlx/query-3f9af4ef27a9865648c24b1fb4f34670ce43deb3af46fbabebf23c8be9d54357.json @@ -0,0 +1,82 @@ +{ + "db_name": "PostgreSQL", + "query": "SELECT id, \"token_hash\",\"location_id\",\"device_id\",\"user_id\",\"steps_snapshot\" \"steps_snapshot: _\",\"current_step\",\"ephemeral_state\" \"ephemeral_state: _\",\"failed_attempts\",\"created_at\",\"expires_at\" FROM \"vpn_client_mfa_session\" WHERE id = $1", + "describe": { + "columns": [ + { + "ordinal": 0, + "name": "id", + "type_info": "Int8" + }, + { + "ordinal": 1, + "name": "token_hash", + "type_info": "Text" + }, + { + "ordinal": 2, + "name": "location_id", + "type_info": "Int8" + }, + { + "ordinal": 3, + "name": "device_id", + "type_info": "Int8" + }, + { + "ordinal": 4, + "name": "user_id", + "type_info": "Int8" + }, + { + "ordinal": 5, + "name": "steps_snapshot: _", + "type_info": "Jsonb" + }, + { + "ordinal": 6, + "name": "current_step", + "type_info": "Int4" + }, + { + "ordinal": 7, + "name": "ephemeral_state: _", + "type_info": "Jsonb" + }, + { + "ordinal": 8, + "name": "failed_attempts", + "type_info": "Int4" + }, + { + "ordinal": 9, + "name": "created_at", + "type_info": "Timestamp" + }, + { + "ordinal": 10, + "name": "expires_at", + "type_info": "Timestamp" + } + ], + "parameters": { + "Left": [ + "Int8" + ] + }, + "nullable": [ + false, + false, + false, + false, + false, + false, + false, + true, + false, + false, + false + ] + }, + "hash": "3f9af4ef27a9865648c24b1fb4f34670ce43deb3af46fbabebf23c8be9d54357" +} diff --git a/.sqlx/query-43e4cdfce44015ad629db8d8d344957448bb8a09735aedfe13e5837c721ea0db.json b/.sqlx/query-43e4cdfce44015ad629db8d8d344957448bb8a09735aedfe13e5837c721ea0db.json new file mode 100644 index 0000000000..ce1928b5c9 --- /dev/null +++ b/.sqlx/query-43e4cdfce44015ad629db8d8d344957448bb8a09735aedfe13e5837c721ea0db.json @@ -0,0 +1,22 @@ +{ + "db_name": "PostgreSQL", + "query": "UPDATE vpn_client_mfa_session SET failed_attempts = failed_attempts + 1 WHERE id = $1 RETURNING failed_attempts", + "describe": { + "columns": [ + { + "ordinal": 0, + "name": "failed_attempts", + "type_info": "Int4" + } + ], + "parameters": { + "Left": [ + "Int8" + ] + }, + "nullable": [ + false + ] + }, + "hash": "43e4cdfce44015ad629db8d8d344957448bb8a09735aedfe13e5837c721ea0db" +} diff --git a/.sqlx/query-4b05abebeafeda2f88fff48f6d9d45938371b3f39822c4ff68a1a9515767e0ad.json b/.sqlx/query-4d4d06efa0450ebebc951926843b0f17a712079bb4c7e7e5c5a0dab016f935be.json similarity index 71% rename from .sqlx/query-4b05abebeafeda2f88fff48f6d9d45938371b3f39822c4ff68a1a9515767e0ad.json rename to .sqlx/query-4d4d06efa0450ebebc951926843b0f17a712079bb4c7e7e5c5a0dab016f935be.json index cdbe9ae673..0f857cf9f0 100644 --- a/.sqlx/query-4b05abebeafeda2f88fff48f6d9d45938371b3f39822c4ff68a1a9515767e0ad.json +++ b/.sqlx/query-4d4d06efa0450ebebc951926843b0f17a712079bb4c7e7e5c5a0dab016f935be.json @@ -1,6 +1,6 @@ { "db_name": "PostgreSQL", - "query": "SELECT id, \"location_id\",\"user_id\",\"device_id\",\"created_at\",\"connected_at\",\"disconnected_at\",\"mfa_method\" \"mfa_method?: _\",\"state\" \"state: _\",\"preshared_key\" FROM \"vpn_client_session\" WHERE id = $1", + "query": "SELECT id, \"location_id\",\"user_id\",\"device_id\",\"created_at\",\"connected_at\",\"disconnected_at\",\"is_mfa_session\",\"state\" \"state: _\",\"preshared_key\" FROM \"vpn_client_session\" WHERE id = $1", "describe": { "columns": [ { @@ -40,21 +40,8 @@ }, { "ordinal": 7, - "name": "mfa_method?: _", - "type_info": { - "Custom": { - "name": "vpn_client_mfa_method", - "kind": { - "Enum": [ - "totp", - "email", - "oidc", - "biometric", - "mobileapprove" - ] - } - } - } + "name": "is_mfa_session", + "type_info": "Bool" }, { "ordinal": 8, @@ -91,10 +78,10 @@ false, true, true, - true, + false, false, true ] }, - "hash": "4b05abebeafeda2f88fff48f6d9d45938371b3f39822c4ff68a1a9515767e0ad" + "hash": "4d4d06efa0450ebebc951926843b0f17a712079bb4c7e7e5c5a0dab016f935be" } diff --git a/.sqlx/query-5ad0e808f8631fd87e6a00db909f85f70901908e8219ce331e6fdfa95b9d8885.json b/.sqlx/query-5ad0e808f8631fd87e6a00db909f85f70901908e8219ce331e6fdfa95b9d8885.json new file mode 100644 index 0000000000..0b7dc9a235 --- /dev/null +++ b/.sqlx/query-5ad0e808f8631fd87e6a00db909f85f70901908e8219ce331e6fdfa95b9d8885.json @@ -0,0 +1,83 @@ +{ + "db_name": "PostgreSQL", + "query": "SELECT id, \"token_hash\",\"location_id\",\"device_id\",\"user_id\",\"steps_snapshot\" \"steps_snapshot: _\",\"current_step\",\"ephemeral_state\" \"ephemeral_state: _\",\"failed_attempts\",\"created_at\",\"expires_at\" FROM \"vpn_client_mfa_session\" LIMIT $1 OFFSET $2", + "describe": { + "columns": [ + { + "ordinal": 0, + "name": "id", + "type_info": "Int8" + }, + { + "ordinal": 1, + "name": "token_hash", + "type_info": "Text" + }, + { + "ordinal": 2, + "name": "location_id", + "type_info": "Int8" + }, + { + "ordinal": 3, + "name": "device_id", + "type_info": "Int8" + }, + { + "ordinal": 4, + "name": "user_id", + "type_info": "Int8" + }, + { + "ordinal": 5, + "name": "steps_snapshot: _", + "type_info": "Jsonb" + }, + { + "ordinal": 6, + "name": "current_step", + "type_info": "Int4" + }, + { + "ordinal": 7, + "name": "ephemeral_state: _", + "type_info": "Jsonb" + }, + { + "ordinal": 8, + "name": "failed_attempts", + "type_info": "Int4" + }, + { + "ordinal": 9, + "name": "created_at", + "type_info": "Timestamp" + }, + { + "ordinal": 10, + "name": "expires_at", + "type_info": "Timestamp" + } + ], + "parameters": { + "Left": [ + "Int8", + "Int8" + ] + }, + "nullable": [ + false, + false, + false, + false, + false, + false, + false, + true, + false, + false, + false + ] + }, + "hash": "5ad0e808f8631fd87e6a00db909f85f70901908e8219ce331e6fdfa95b9d8885" +} diff --git a/.sqlx/query-673e54170128d24f80b817373e98a11e27d1df5f7aa62c7d955e3c97f8d659b6.json b/.sqlx/query-673e54170128d24f80b817373e98a11e27d1df5f7aa62c7d955e3c97f8d659b6.json new file mode 100644 index 0000000000..9dcce62fd1 --- /dev/null +++ b/.sqlx/query-673e54170128d24f80b817373e98a11e27d1df5f7aa62c7d955e3c97f8d659b6.json @@ -0,0 +1,15 @@ +{ + "db_name": "PostgreSQL", + "query": "UPDATE vpn_client_mfa_session SET ephemeral_state = $2 WHERE id = $1", + "describe": { + "columns": [], + "parameters": { + "Left": [ + "Int8", + "Jsonb" + ] + }, + "nullable": [] + }, + "hash": "673e54170128d24f80b817373e98a11e27d1df5f7aa62c7d955e3c97f8d659b6" +} diff --git a/.sqlx/query-676f3035dfe8107668174297a0d4ae437265a53d32de83ffef4d1d46637db7b4.json b/.sqlx/query-676f3035dfe8107668174297a0d4ae437265a53d32de83ffef4d1d46637db7b4.json new file mode 100644 index 0000000000..f1bdbb5d9c --- /dev/null +++ b/.sqlx/query-676f3035dfe8107668174297a0d4ae437265a53d32de83ffef4d1d46637db7b4.json @@ -0,0 +1,82 @@ +{ + "db_name": "PostgreSQL", + "query": "SELECT id, token_hash, location_id, device_id, user_id, steps_snapshot \"steps_snapshot: Json\", current_step, ephemeral_state \"ephemeral_state: Json\", failed_attempts, created_at, expires_at FROM vpn_client_mfa_session WHERE token_hash = $1 AND expires_at > (now() AT TIME ZONE 'UTC')", + "describe": { + "columns": [ + { + "ordinal": 0, + "name": "id", + "type_info": "Int8" + }, + { + "ordinal": 1, + "name": "token_hash", + "type_info": "Text" + }, + { + "ordinal": 2, + "name": "location_id", + "type_info": "Int8" + }, + { + "ordinal": 3, + "name": "device_id", + "type_info": "Int8" + }, + { + "ordinal": 4, + "name": "user_id", + "type_info": "Int8" + }, + { + "ordinal": 5, + "name": "steps_snapshot: Json", + "type_info": "Jsonb" + }, + { + "ordinal": 6, + "name": "current_step", + "type_info": "Int4" + }, + { + "ordinal": 7, + "name": "ephemeral_state: Json", + "type_info": "Jsonb" + }, + { + "ordinal": 8, + "name": "failed_attempts", + "type_info": "Int4" + }, + { + "ordinal": 9, + "name": "created_at", + "type_info": "Timestamp" + }, + { + "ordinal": 10, + "name": "expires_at", + "type_info": "Timestamp" + } + ], + "parameters": { + "Left": [ + "Text" + ] + }, + "nullable": [ + false, + false, + false, + false, + false, + false, + false, + true, + false, + false, + false + ] + }, + "hash": "676f3035dfe8107668174297a0d4ae437265a53d32de83ffef4d1d46637db7b4" +} diff --git a/.sqlx/query-694e5861890efb0bbf4f48f3f7df91def7a522a0b7ca8b418788c8ccae33f1bd.json b/.sqlx/query-694e5861890efb0bbf4f48f3f7df91def7a522a0b7ca8b418788c8ccae33f1bd.json new file mode 100644 index 0000000000..be3f12771d --- /dev/null +++ b/.sqlx/query-694e5861890efb0bbf4f48f3f7df91def7a522a0b7ca8b418788c8ccae33f1bd.json @@ -0,0 +1,24 @@ +{ + "db_name": "PostgreSQL", + "query": "UPDATE \"vpn_client_mfa_session\" SET \"token_hash\" = $2,\"location_id\" = $3,\"device_id\" = $4,\"user_id\" = $5,\"steps_snapshot\" = $6,\"current_step\" = $7,\"ephemeral_state\" = $8,\"failed_attempts\" = $9,\"created_at\" = $10,\"expires_at\" = $11 WHERE id = $1", + "describe": { + "columns": [], + "parameters": { + "Left": [ + "Int8", + "Text", + "Int8", + "Int8", + "Int8", + "Jsonb", + "Int4", + "Jsonb", + "Int4", + "Timestamp", + "Timestamp" + ] + }, + "nullable": [] + }, + "hash": "694e5861890efb0bbf4f48f3f7df91def7a522a0b7ca8b418788c8ccae33f1bd" +} diff --git a/.sqlx/query-75a46ab478156cbde2e8667fdf8bdbbc14436b8ed6ad5914a90fe36851f50e93.json b/.sqlx/query-75a46ab478156cbde2e8667fdf8bdbbc14436b8ed6ad5914a90fe36851f50e93.json new file mode 100644 index 0000000000..e328ee279f --- /dev/null +++ b/.sqlx/query-75a46ab478156cbde2e8667fdf8bdbbc14436b8ed6ad5914a90fe36851f50e93.json @@ -0,0 +1,28 @@ +{ + "db_name": "PostgreSQL", + "query": "UPDATE vpn_client_mfa_session SET steps_snapshot = CASE WHEN ephemeral_state IS NOT NULL THEN jsonb_set( steps_snapshot, ARRAY['steps', current_step::text, 'satisfied'], ephemeral_state->'selected_method' ) ELSE steps_snapshot END, ephemeral_state = NULL, current_step = current_step + 1, failed_attempts = 0 WHERE id = $1 RETURNING current_step, steps_snapshot \"steps_snapshot: Json\"", + "describe": { + "columns": [ + { + "ordinal": 0, + "name": "current_step", + "type_info": "Int4" + }, + { + "ordinal": 1, + "name": "steps_snapshot: Json", + "type_info": "Jsonb" + } + ], + "parameters": { + "Left": [ + "Int8" + ] + }, + "nullable": [ + false, + false + ] + }, + "hash": "75a46ab478156cbde2e8667fdf8bdbbc14436b8ed6ad5914a90fe36851f50e93" +} diff --git a/.sqlx/query-76aa712da8fdec30ac4316d8d7c085667c53bb675a3326ddc37c0dea9aebc9fe.json b/.sqlx/query-76aa712da8fdec30ac4316d8d7c085667c53bb675a3326ddc37c0dea9aebc9fe.json new file mode 100644 index 0000000000..2b0352ced2 --- /dev/null +++ b/.sqlx/query-76aa712da8fdec30ac4316d8d7c085667c53bb675a3326ddc37c0dea9aebc9fe.json @@ -0,0 +1,89 @@ +{ + "db_name": "PostgreSQL", + "query": "INSERT INTO vpn_client_mfa_session (token_hash, location_id, device_id, user_id, steps_snapshot, current_step, ephemeral_state, failed_attempts, created_at, expires_at) VALUES ($1, $2, $3, $4, $5, 0, $6, 0, $7, $8) ON CONFLICT (location_id, device_id) DO UPDATE SET token_hash = EXCLUDED.token_hash, user_id = EXCLUDED.user_id, steps_snapshot = EXCLUDED.steps_snapshot, current_step = EXCLUDED.current_step, ephemeral_state = EXCLUDED.ephemeral_state, failed_attempts = EXCLUDED.failed_attempts, created_at = EXCLUDED.created_at, expires_at = EXCLUDED.expires_at RETURNING id, token_hash, location_id, device_id, user_id, steps_snapshot \"steps_snapshot: Json\", current_step, ephemeral_state \"ephemeral_state: Json\", failed_attempts, created_at, expires_at", + "describe": { + "columns": [ + { + "ordinal": 0, + "name": "id", + "type_info": "Int8" + }, + { + "ordinal": 1, + "name": "token_hash", + "type_info": "Text" + }, + { + "ordinal": 2, + "name": "location_id", + "type_info": "Int8" + }, + { + "ordinal": 3, + "name": "device_id", + "type_info": "Int8" + }, + { + "ordinal": 4, + "name": "user_id", + "type_info": "Int8" + }, + { + "ordinal": 5, + "name": "steps_snapshot: Json", + "type_info": "Jsonb" + }, + { + "ordinal": 6, + "name": "current_step", + "type_info": "Int4" + }, + { + "ordinal": 7, + "name": "ephemeral_state: Json", + "type_info": "Jsonb" + }, + { + "ordinal": 8, + "name": "failed_attempts", + "type_info": "Int4" + }, + { + "ordinal": 9, + "name": "created_at", + "type_info": "Timestamp" + }, + { + "ordinal": 10, + "name": "expires_at", + "type_info": "Timestamp" + } + ], + "parameters": { + "Left": [ + "Text", + "Int8", + "Int8", + "Int8", + "Jsonb", + "Jsonb", + "Timestamp", + "Timestamp" + ] + }, + "nullable": [ + false, + false, + false, + false, + false, + false, + false, + true, + false, + false, + false + ] + }, + "hash": "76aa712da8fdec30ac4316d8d7c085667c53bb675a3326ddc37c0dea9aebc9fe" +} diff --git a/.sqlx/query-d86d5f9cb508b1840f0de3c40a993ee77d9cb3c80d8028d99bcc08ccd4c78dd0.json b/.sqlx/query-855ba62730e251f1a15d7aeeaeafaff30514df6360bfbeaac276d55f3abd1b9c.json similarity index 68% rename from .sqlx/query-d86d5f9cb508b1840f0de3c40a993ee77d9cb3c80d8028d99bcc08ccd4c78dd0.json rename to .sqlx/query-855ba62730e251f1a15d7aeeaeafaff30514df6360bfbeaac276d55f3abd1b9c.json index f18173e22b..da769b8309 100644 --- a/.sqlx/query-d86d5f9cb508b1840f0de3c40a993ee77d9cb3c80d8028d99bcc08ccd4c78dd0.json +++ b/.sqlx/query-855ba62730e251f1a15d7aeeaeafaff30514df6360bfbeaac276d55f3abd1b9c.json @@ -1,6 +1,6 @@ { "db_name": "PostgreSQL", - "query": "SELECT id, location_id, user_id, device_id, created_at, connected_at, disconnected_at, mfa_method \"mfa_method: VpnClientMfaMethod\", state \"state: VpnClientSessionState\", preshared_key FROM vpn_client_session WHERE location_id = $1 AND state = 'new' AND (NOW() - created_at) > $2 * interval '1 second'", + "query": "SELECT id, location_id, user_id, device_id, created_at, connected_at, disconnected_at, is_mfa_session, state \"state: VpnClientSessionState\", preshared_key FROM vpn_client_session WHERE location_id = $1 AND state = 'new' AND (NOW() - created_at) > $2 * interval '1 second'", "describe": { "columns": [ { @@ -40,21 +40,8 @@ }, { "ordinal": 7, - "name": "mfa_method: VpnClientMfaMethod", - "type_info": { - "Custom": { - "name": "vpn_client_mfa_method", - "kind": { - "Enum": [ - "totp", - "email", - "oidc", - "biometric", - "mobileapprove" - ] - } - } - } + "name": "is_mfa_session", + "type_info": "Bool" }, { "ordinal": 8, @@ -92,10 +79,10 @@ false, true, true, - true, + false, false, true ] }, - "hash": "d86d5f9cb508b1840f0de3c40a993ee77d9cb3c80d8028d99bcc08ccd4c78dd0" + "hash": "855ba62730e251f1a15d7aeeaeafaff30514df6360bfbeaac276d55f3abd1b9c" } diff --git a/.sqlx/query-8832443485dfb2303acb1a652648750fdbc73841db0133b9bbea918fa6e15d33.json b/.sqlx/query-8832443485dfb2303acb1a652648750fdbc73841db0133b9bbea918fa6e15d33.json new file mode 100644 index 0000000000..d7738bd922 --- /dev/null +++ b/.sqlx/query-8832443485dfb2303acb1a652648750fdbc73841db0133b9bbea918fa6e15d33.json @@ -0,0 +1,31 @@ +{ + "db_name": "PostgreSQL", + "query": "INSERT INTO \"vpn_client_mfa_session\" (\"token_hash\",\"location_id\",\"device_id\",\"user_id\",\"steps_snapshot\",\"current_step\",\"ephemeral_state\",\"failed_attempts\",\"created_at\",\"expires_at\") VALUES ($1,$2,$3,$4,$5,$6,$7,$8,$9,$10) RETURNING id", + "describe": { + "columns": [ + { + "ordinal": 0, + "name": "id", + "type_info": "Int8" + } + ], + "parameters": { + "Left": [ + "Text", + "Int8", + "Int8", + "Int8", + "Jsonb", + "Int4", + "Jsonb", + "Int4", + "Timestamp", + "Timestamp" + ] + }, + "nullable": [ + false + ] + }, + "hash": "8832443485dfb2303acb1a652648750fdbc73841db0133b9bbea918fa6e15d33" +} diff --git a/.sqlx/query-5a856149fa68d294e5a15ceacdfc5da77a32a11145804dff5692df7c3d74ce7b.json b/.sqlx/query-8e0fc1e62f766a720e54c817646b49904e7e4f1c6c5a7f1d0e4126243e89017f.json similarity index 67% rename from .sqlx/query-5a856149fa68d294e5a15ceacdfc5da77a32a11145804dff5692df7c3d74ce7b.json rename to .sqlx/query-8e0fc1e62f766a720e54c817646b49904e7e4f1c6c5a7f1d0e4126243e89017f.json index 9698c37940..d586a867c1 100644 --- a/.sqlx/query-5a856149fa68d294e5a15ceacdfc5da77a32a11145804dff5692df7c3d74ce7b.json +++ b/.sqlx/query-8e0fc1e62f766a720e54c817646b49904e7e4f1c6c5a7f1d0e4126243e89017f.json @@ -1,6 +1,6 @@ { "db_name": "PostgreSQL", - "query": "SELECT id, location_id, user_id, device_id, created_at, connected_at, disconnected_at, mfa_method \"mfa_method: VpnClientMfaMethod\", state \"state: VpnClientSessionState\", preshared_key FROM vpn_client_session WHERE location_id = $1 AND device_id = $2 AND state IN ('new', 'connected') ORDER BY created_at DESC, id DESC", + "query": "SELECT id, location_id, user_id, device_id, created_at, connected_at, disconnected_at, is_mfa_session, state \"state: VpnClientSessionState\", preshared_key FROM vpn_client_session WHERE location_id = $1 AND device_id = $2 AND state IN ('new', 'connected') ORDER BY created_at DESC, id DESC LIMIT 1", "describe": { "columns": [ { @@ -40,21 +40,8 @@ }, { "ordinal": 7, - "name": "mfa_method: VpnClientMfaMethod", - "type_info": { - "Custom": { - "name": "vpn_client_mfa_method", - "kind": { - "Enum": [ - "totp", - "email", - "oidc", - "biometric", - "mobileapprove" - ] - } - } - } + "name": "is_mfa_session", + "type_info": "Bool" }, { "ordinal": 8, @@ -92,10 +79,10 @@ false, true, true, - true, + false, false, true ] }, - "hash": "5a856149fa68d294e5a15ceacdfc5da77a32a11145804dff5692df7c3d74ce7b" + "hash": "8e0fc1e62f766a720e54c817646b49904e7e4f1c6c5a7f1d0e4126243e89017f" } diff --git a/.sqlx/query-32b5d4d820a72da19cbd3bb1a33e17c9555d0350d03679d9cf3b7ccc6451c7ae.json b/.sqlx/query-92ea1292afc227fe09d6abdba065b04c200ad454c99e2a719891097b260da7be.json similarity index 60% rename from .sqlx/query-32b5d4d820a72da19cbd3bb1a33e17c9555d0350d03679d9cf3b7ccc6451c7ae.json rename to .sqlx/query-92ea1292afc227fe09d6abdba065b04c200ad454c99e2a719891097b260da7be.json index 35150a6a35..bbcad34641 100644 --- a/.sqlx/query-32b5d4d820a72da19cbd3bb1a33e17c9555d0350d03679d9cf3b7ccc6451c7ae.json +++ b/.sqlx/query-92ea1292afc227fe09d6abdba065b04c200ad454c99e2a719891097b260da7be.json @@ -1,6 +1,6 @@ { "db_name": "PostgreSQL", - "query": "INSERT INTO \"vpn_client_session\" (\"location_id\",\"user_id\",\"device_id\",\"created_at\",\"connected_at\",\"disconnected_at\",\"mfa_method\",\"state\",\"preshared_key\") VALUES ($1,$2,$3,$4,$5,$6,$7,$8,$9) RETURNING id", + "query": "INSERT INTO \"vpn_client_session\" (\"location_id\",\"user_id\",\"device_id\",\"created_at\",\"connected_at\",\"disconnected_at\",\"is_mfa_session\",\"state\",\"preshared_key\") VALUES ($1,$2,$3,$4,$5,$6,$7,$8,$9) RETURNING id", "describe": { "columns": [ { @@ -17,20 +17,7 @@ "Timestamp", "Timestamp", "Timestamp", - { - "Custom": { - "name": "vpn_client_mfa_method", - "kind": { - "Enum": [ - "totp", - "email", - "oidc", - "biometric", - "mobileapprove" - ] - } - } - }, + "Bool", { "Custom": { "name": "vpn_client_session_state", @@ -50,5 +37,5 @@ false ] }, - "hash": "32b5d4d820a72da19cbd3bb1a33e17c9555d0350d03679d9cf3b7ccc6451c7ae" + "hash": "92ea1292afc227fe09d6abdba065b04c200ad454c99e2a719891097b260da7be" } diff --git a/.sqlx/query-9a56d54d860eba45d275137dfedef46eaff54d8b262cd311dc384ffa83f811e6.json b/.sqlx/query-9a56d54d860eba45d275137dfedef46eaff54d8b262cd311dc384ffa83f811e6.json new file mode 100644 index 0000000000..a1cfb43796 --- /dev/null +++ b/.sqlx/query-9a56d54d860eba45d275137dfedef46eaff54d8b262cd311dc384ffa83f811e6.json @@ -0,0 +1,20 @@ +{ + "db_name": "PostgreSQL", + "query": "SELECT count(*) FROM \"vpn_client_mfa_session\"", + "describe": { + "columns": [ + { + "ordinal": 0, + "name": "count", + "type_info": "Int8" + } + ], + "parameters": { + "Left": [] + }, + "nullable": [ + null + ] + }, + "hash": "9a56d54d860eba45d275137dfedef46eaff54d8b262cd311dc384ffa83f811e6" +} diff --git a/.sqlx/query-a2a31a9e9d53d830f658131eab155413d1dd6ce5a24b87b9fc4060dd1ae704be.json b/.sqlx/query-a2a31a9e9d53d830f658131eab155413d1dd6ce5a24b87b9fc4060dd1ae704be.json deleted file mode 100644 index f5b56480af..0000000000 --- a/.sqlx/query-a2a31a9e9d53d830f658131eab155413d1dd6ce5a24b87b9fc4060dd1ae704be.json +++ /dev/null @@ -1,101 +0,0 @@ -{ - "db_name": "PostgreSQL", - "query": "SELECT s.id, location_id, user_id, device_id, created_at, s.connected_at, disconnected_at, mfa_method \"mfa_method: VpnClientMfaMethod\", state \"state: VpnClientSessionState\", preshared_key FROM vpn_client_session s LEFT JOIN LATERAL ( SELECT latest_handshake FROM vpn_session_stats WHERE session_id = s.id ORDER BY latest_handshake DESC LIMIT 1 ) ss ON true WHERE location_id = $1 AND state = 'connected' AND (NOW() - ss.latest_handshake) > $2 * interval '1 second'", - "describe": { - "columns": [ - { - "ordinal": 0, - "name": "id", - "type_info": "Int8" - }, - { - "ordinal": 1, - "name": "location_id", - "type_info": "Int8" - }, - { - "ordinal": 2, - "name": "user_id", - "type_info": "Int8" - }, - { - "ordinal": 3, - "name": "device_id", - "type_info": "Int8" - }, - { - "ordinal": 4, - "name": "created_at", - "type_info": "Timestamp" - }, - { - "ordinal": 5, - "name": "connected_at", - "type_info": "Timestamp" - }, - { - "ordinal": 6, - "name": "disconnected_at", - "type_info": "Timestamp" - }, - { - "ordinal": 7, - "name": "mfa_method: VpnClientMfaMethod", - "type_info": { - "Custom": { - "name": "vpn_client_mfa_method", - "kind": { - "Enum": [ - "totp", - "email", - "oidc", - "biometric", - "mobileapprove" - ] - } - } - } - }, - { - "ordinal": 8, - "name": "state: VpnClientSessionState", - "type_info": { - "Custom": { - "name": "vpn_client_session_state", - "kind": { - "Enum": [ - "new", - "connected", - "disconnected" - ] - } - } - } - }, - { - "ordinal": 9, - "name": "preshared_key", - "type_info": "Text" - } - ], - "parameters": { - "Left": [ - "Int8", - "Float8" - ] - }, - "nullable": [ - false, - false, - false, - false, - false, - true, - true, - true, - false, - true - ] - }, - "hash": "a2a31a9e9d53d830f658131eab155413d1dd6ce5a24b87b9fc4060dd1ae704be" -} diff --git a/.sqlx/query-a8b40bf22ad9072430ebe70980ef6bc93bbb7202935192e5eb4307eb6801e1f0.json b/.sqlx/query-a8b40bf22ad9072430ebe70980ef6bc93bbb7202935192e5eb4307eb6801e1f0.json new file mode 100644 index 0000000000..8d571aa9a5 --- /dev/null +++ b/.sqlx/query-a8b40bf22ad9072430ebe70980ef6bc93bbb7202935192e5eb4307eb6801e1f0.json @@ -0,0 +1,23 @@ +{ + "db_name": "PostgreSQL", + "query": "SELECT count(*) FROM vpn_client_mfa_session WHERE location_id = $1 AND device_id = $2", + "describe": { + "columns": [ + { + "ordinal": 0, + "name": "count", + "type_info": "Int8" + } + ], + "parameters": { + "Left": [ + "Int8", + "Int8" + ] + }, + "nullable": [ + null + ] + }, + "hash": "a8b40bf22ad9072430ebe70980ef6bc93bbb7202935192e5eb4307eb6801e1f0" +} diff --git a/.sqlx/query-e87af2b4e3fd79709a28381e04690aee96054585a87e6673a5efa275795dc060.json b/.sqlx/query-bc6e7c27ca6cd7eb829081482a491cbc7299d3dabd2323964657a54b35d2ec6c.json similarity index 67% rename from .sqlx/query-e87af2b4e3fd79709a28381e04690aee96054585a87e6673a5efa275795dc060.json rename to .sqlx/query-bc6e7c27ca6cd7eb829081482a491cbc7299d3dabd2323964657a54b35d2ec6c.json index 2eebd02c59..aecaece746 100644 --- a/.sqlx/query-e87af2b4e3fd79709a28381e04690aee96054585a87e6673a5efa275795dc060.json +++ b/.sqlx/query-bc6e7c27ca6cd7eb829081482a491cbc7299d3dabd2323964657a54b35d2ec6c.json @@ -1,6 +1,6 @@ { "db_name": "PostgreSQL", - "query": "SELECT id, location_id, user_id, device_id, created_at, connected_at, disconnected_at, mfa_method \"mfa_method: VpnClientMfaMethod\", state \"state: VpnClientSessionState\", preshared_key FROM vpn_client_session WHERE location_id = $1 AND device_id = $2 AND state IN ('new', 'connected') ORDER BY created_at DESC, id DESC LIMIT 1", + "query": "SELECT id, location_id, user_id, device_id, created_at, connected_at, disconnected_at, is_mfa_session, state \"state: VpnClientSessionState\", preshared_key FROM vpn_client_session WHERE location_id = $1 AND device_id = $2 AND state IN ('new', 'connected') ORDER BY created_at DESC, id DESC", "describe": { "columns": [ { @@ -40,21 +40,8 @@ }, { "ordinal": 7, - "name": "mfa_method: VpnClientMfaMethod", - "type_info": { - "Custom": { - "name": "vpn_client_mfa_method", - "kind": { - "Enum": [ - "totp", - "email", - "oidc", - "biometric", - "mobileapprove" - ] - } - } - } + "name": "is_mfa_session", + "type_info": "Bool" }, { "ordinal": 8, @@ -92,10 +79,10 @@ false, true, true, - true, + false, false, true ] }, - "hash": "e87af2b4e3fd79709a28381e04690aee96054585a87e6673a5efa275795dc060" + "hash": "bc6e7c27ca6cd7eb829081482a491cbc7299d3dabd2323964657a54b35d2ec6c" } diff --git a/.sqlx/query-d2d2f819b6e1528018ad1187fff46fc06c4a122145d236070ba16ca9b4f0df23.json b/.sqlx/query-d2d2f819b6e1528018ad1187fff46fc06c4a122145d236070ba16ca9b4f0df23.json new file mode 100644 index 0000000000..4d00e80b2c --- /dev/null +++ b/.sqlx/query-d2d2f819b6e1528018ad1187fff46fc06c4a122145d236070ba16ca9b4f0df23.json @@ -0,0 +1,14 @@ +{ + "db_name": "PostgreSQL", + "query": "DELETE FROM \"vpn_client_mfa_session\" WHERE id = $1", + "describe": { + "columns": [], + "parameters": { + "Left": [ + "Int8" + ] + }, + "nullable": [] + }, + "hash": "d2d2f819b6e1528018ad1187fff46fc06c4a122145d236070ba16ca9b4f0df23" +} diff --git a/.sqlx/query-e75cc56595cfc83444054f79e0f7f56425277c742f159df73f88ce5b90fd31bc.json b/.sqlx/query-e75cc56595cfc83444054f79e0f7f56425277c742f159df73f88ce5b90fd31bc.json new file mode 100644 index 0000000000..c31c565ec0 --- /dev/null +++ b/.sqlx/query-e75cc56595cfc83444054f79e0f7f56425277c742f159df73f88ce5b90fd31bc.json @@ -0,0 +1,16 @@ +{ + "db_name": "PostgreSQL", + "query": "UPDATE vpn_client_mfa_session SET ephemeral_state = jsonb_set(ephemeral_state, ARRAY[$2]::text[], 'true'::jsonb) WHERE id = $1 AND ephemeral_state IS NOT NULL AND ephemeral_state->>'step_attempt_id' = $3", + "describe": { + "columns": [], + "parameters": { + "Left": [ + "Int8", + "Text", + "Text" + ] + }, + "nullable": [] + }, + "hash": "e75cc56595cfc83444054f79e0f7f56425277c742f159df73f88ce5b90fd31bc" +} diff --git a/Cargo.lock b/Cargo.lock index 0f971fef40..f5b0e546e5 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -815,9 +815,9 @@ dependencies = [ [[package]] name = "cc" -version = "1.4.2" +version = "1.4.3" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "5d262e149917187838d5b42777c8253bcb64500067342904e7d429499a6f277e" +checksum = "509591b7bcd67f4ef775afad7662703b4935daaa6ec0e5605cfb1090b32a2b6d" dependencies = [ "find-msvc-tools", "jobserver", @@ -1706,6 +1706,7 @@ dependencies = [ "defguard_grpc_tls", "defguard_proto", "defguard_version", + "ed25519-dalek", "hyper-rustls", "hyper-util", "ipnetwork", @@ -2358,9 +2359,9 @@ checksum = "28dea519a9695b9977216879a3ebfddf92f1c08c05d984f8996aecd6ecdc811d" [[package]] name = "find-msvc-tools" -version = "0.1.10" +version = "0.1.11" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "26b73573e6edcd2af0cdf47bd6cb58f0b3839491263c314eaad1ccf24430e1de" +checksum = "d45db016d36b838f563236e9193d0ee6ce38f3f68b6c94e914b4929c96bbb890" [[package]] name = "fixedbitset" @@ -2722,9 +2723,9 @@ dependencies = [ [[package]] name = "h2" -version = "0.4.15" +version = "0.4.16" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "6cb093c84e8bd9b188d4c4a8cb6579fc016968d14c99882163cd3ff402a4f155" +checksum = "a9f37a958b41b3b19ee2707c06439c0e9e547e847223eb791ecb0cb821c65e27" dependencies = [ "atomic-waker", "bytes", @@ -3591,9 +3592,9 @@ checksum = "b6d2cec3eae94f9f509c767b45932f1ada8350c4bdb85af2fcab4a3c14807981" [[package]] name = "libredox" -version = "0.1.19" +version = "0.1.20" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "2026a5056764a10b2bf5d56488cba40da507f5493a6a429340e2004d9ed085fa" +checksum = "28d0a00925a9f930d679b6789b721e3a7f9ed110f41b86d2497caa780c3a070a" dependencies = [ "bitflags 2.13.1", "libc", @@ -4795,9 +4796,9 @@ dependencies = [ [[package]] name = "pkg-config" -version = "0.3.33" +version = "0.3.34" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "19f132c84eca552bf34cab8ec81f1c1dcc229b811638f9d283dceabe58c5569e" +checksum = "f6b464fbc74e149a392436b17d523f769e057cb6877f6a5c4618bc6f11800548" [[package]] name = "plain" @@ -5118,9 +5119,9 @@ dependencies = [ [[package]] name = "quinn-proto" -version = "0.11.16" +version = "0.11.17" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "2f4bfc015262b9df63c8845072ce59068853ff5872180c2ce2f13038b970e560" +checksum = "04759210543be93709136e28212294a659ef5001836ff4eab4d663e4529bba83" dependencies = [ "bytes", "getrandom 0.4.3", @@ -7390,9 +7391,9 @@ checksum = "e2eebbbfe4093922c2b6734d7c679ebfebd704a0d7e56dfcb0d05818ce28977d" [[package]] name = "uuid" -version = "1.24.0" +version = "1.24.1" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "bf3923a6f5c4c6382e0b653c4117f48d631ea17f38ed86e2a828e6f7412f5239" +checksum = "2cefc03fd367c0c6d4305de1b312cf00248c4114f4a0418ce6a6af769e3b0bd9" dependencies = [ "getrandom 0.4.3", "js-sys", @@ -8215,9 +8216,9 @@ dependencies = [ [[package]] name = "zerovec-derive" -version = "0.11.4" +version = "0.11.5" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "47402523226a02bfe5230160dc3ccc089aa6f6f19e7fcbb4e6f824bbb1b4aa62" +checksum = "9f212a141d820099d57ffafb9569be9617a6f27d3dc881fbee8fb56642f917a9" dependencies = [ "proc-macro2", "quote", diff --git a/Cargo.toml b/Cargo.toml index c3d392630a..bb291aa42a 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -17,7 +17,6 @@ redundant_closure = "warn" [workspace.dependencies] # internal crates - defguard_setup = { path = "./crates/defguard_setup", version = "0.0.0" } defguard_common = { path = "./crates/defguard_common", version = "2.2.0" } defguard_static_ip = { path = "./crates/defguard_static_ip", version = "0.0.0" } @@ -57,9 +56,11 @@ chrono = { version = "0.4", default-features = false, features = [ ] } claims = "0.8" clap = { version = "4.5", features = ["derive", "env"] } +ed25519-dalek = { version = "2.2", features = ["rand_core"] } futures = "0.3" http = "1.5" hyper-rustls = { version = "0.27", features = ["http2"] } +hyper-util = "0.1" humantime = "2.1" # match version used by sqlx ipnetwork = "0.20" diff --git a/crates/defguard_common/src/db/models/biometric_auth.rs b/crates/defguard_common/src/db/models/biometric_auth.rs index 7450e9cbf3..3fb8642a4c 100644 --- a/crates/defguard_common/src/db/models/biometric_auth.rs +++ b/crates/defguard_common/src/db/models/biometric_auth.rs @@ -1,6 +1,7 @@ use base64::{Engine, engine::general_purpose, prelude::BASE64_STANDARD}; use ed25519_dalek::{Signature, Verifier, VerifyingKey}; use model_derive::Model; +use serde::{Deserialize, Serialize}; use sqlx::{PgExecutor, query, query_as}; use thiserror::Error; @@ -123,7 +124,7 @@ impl BiometricAuth { } } -#[derive(Clone, Debug)] +#[derive(Clone, Debug, Deserialize, PartialEq, Serialize)] pub struct BiometricChallenge { pub auth_pub_key: Option, pub challenge: String, diff --git a/crates/defguard_common/src/db/models/device.rs b/crates/defguard_common/src/db/models/device.rs index eb2872bb62..f889570b23 100644 --- a/crates/defguard_common/src/db/models/device.rs +++ b/crates/defguard_common/src/db/models/device.rs @@ -993,10 +993,7 @@ mod test { use crate::{ csv::AsCsv, db::{ - models::{ - gateway::Gateway, vpn_client_session::VpnClientMfaMethod, - vpn_session_stats::VpnSessionStats, - }, + models::{gateway::Gateway, vpn_session_stats::VpnSessionStats}, setup_pool, }, }; @@ -1400,7 +1397,7 @@ mod test { created_at: Utc::now().naive_utc(), connected_at: None, disconnected_at: None, - mfa_method: Some(VpnClientMfaMethod::Totp), + is_mfa_session: true, state: VpnClientSessionState::New, preshared_key: None, }; @@ -1451,7 +1448,7 @@ mod test { created_at: Utc::now().naive_utc(), connected_at: Some(Utc::now().naive_utc()), disconnected_at: None, - mfa_method: Some(VpnClientMfaMethod::Totp), + is_mfa_session: true, state: VpnClientSessionState::Connected, preshared_key: Some("runtime-session-psk".into()), }; @@ -1521,13 +1518,7 @@ mod test { ); wireguard_network_device.insert(&pool).await.unwrap(); - let session = VpnClientSession::new( - network.id, - user.id, - device.id, - None, - Some(VpnClientMfaMethod::Totp), - ); + let session = VpnClientSession::new(network.id, user.id, device.id, None, true); session.save(&pool).await.unwrap(); let device_info = DeviceInfo::from_device(&pool, device).await.unwrap(); @@ -1603,7 +1594,7 @@ mod test { user.id, device.id, Some(Utc::now().naive_utc()), - Some(VpnClientMfaMethod::Totp), + true, ); session.preshared_key = Some("device-info-session-psk".into()); session.save(&pool).await.unwrap(); @@ -1679,7 +1670,7 @@ mod test { ); wireguard_network_device.insert(&pool).await.unwrap(); - let mut session = VpnClientSession::new(network.id, user.id, device.id, None, None); + let mut session = VpnClientSession::new(network.id, user.id, device.id, None, false); session.preshared_key = Some("legacy-session-psk".into()); session.save(&pool).await.unwrap(); @@ -1768,7 +1759,7 @@ mod test { user.id, device.id, Some(last_successful_connection), - None, + false, ); connected_session.created_at = last_successful_connection; let connected_session = connected_session.save(&pool).await.unwrap(); @@ -1789,7 +1780,7 @@ mod test { .unwrap(); let mut disconnected_session = - VpnClientSession::new(network.id, user.id, device.id, None, None); + VpnClientSession::new(network.id, user.id, device.id, None, false); disconnected_session.created_at = newer_session_created_at; disconnected_session.disconnected_at = Some(newer_session_created_at); disconnected_session.state = VpnClientSessionState::Disconnected; @@ -1894,14 +1885,14 @@ mod test { user.id, device.id, Some(last_successful_connection), - None, + false, ); connected_session.created_at = last_successful_connection; connected_session.disconnected_at = Some(disconnected_at); connected_session.state = VpnClientSessionState::Disconnected; connected_session.save(&pool).await.unwrap(); - let mut new_session = VpnClientSession::new(network.id, user.id, device.id, None, None); + let mut new_session = VpnClientSession::new(network.id, user.id, device.id, None, false); new_session.created_at = newer_session_created_at; new_session.save(&pool).await.unwrap(); @@ -1984,7 +1975,7 @@ mod test { .expect("expected valid time"); let session = - VpnClientSession::new(network.id, user.id, device.id, Some(connected_at), None) + VpnClientSession::new(network.id, user.id, device.id, Some(connected_at), false) .save(&pool) .await .unwrap(); @@ -2081,7 +2072,7 @@ mod test { .expect("expected valid time"); let mut attempted_session = - VpnClientSession::new(network.id, user.id, device.id, None, None); + VpnClientSession::new(network.id, user.id, device.id, None, false); attempted_session.created_at = attempted_at; let attempted_session = attempted_session.save(&pool).await.unwrap(); @@ -2167,7 +2158,7 @@ mod test { .and_hms_opt(3, 4, 5) .expect("expected valid time"); - VpnClientSession::new(network.id, user.id, device.id, Some(connected_at), None) + VpnClientSession::new(network.id, user.id, device.id, Some(connected_at), false) .save(&pool) .await .unwrap(); @@ -2249,7 +2240,7 @@ mod test { .expect("expected valid time"); let session = - VpnClientSession::new(network.id, user.id, device.id, Some(connected_at), None) + VpnClientSession::new(network.id, user.id, device.id, Some(connected_at), false) .save(&pool) .await .unwrap(); diff --git a/crates/defguard_common/src/db/models/mod.rs b/crates/defguard_common/src/db/models/mod.rs index 2fb6388152..bcbc1be54a 100644 --- a/crates/defguard_common/src/db/models/mod.rs +++ b/crates/defguard_common/src/db/models/mod.rs @@ -20,6 +20,7 @@ pub mod session; pub mod settings; pub mod setup_auto_adoption; pub mod user; +pub mod vpn_client_mfa_session; pub mod vpn_client_session; pub mod vpn_session_stats; pub mod webauthn; diff --git a/crates/defguard_common/src/db/models/vpn_client_mfa_session.rs b/crates/defguard_common/src/db/models/vpn_client_mfa_session.rs new file mode 100644 index 0000000000..f367b8fac4 --- /dev/null +++ b/crates/defguard_common/src/db/models/vpn_client_mfa_session.rs @@ -0,0 +1,471 @@ +use std::time::Duration; + +use base64::{Engine, engine::general_purpose::URL_SAFE_NO_PAD}; +use chrono::{NaiveDateTime, TimeDelta, Utc}; +use model_derive::Model; +use serde::{Deserialize, Serialize}; +use sha2::{Digest, Sha256}; +use sqlx::{ + Connection, PgConnection, PgExecutor, PgPool, query, query_as, query_scalar, types::Json, +}; +use tracing::debug; + +use crate::{ + db::{ + Id, NoId, + models::{ + biometric_auth::BiometricChallenge, device::Device, user::User, + vpn_client_session::VpnClientMfaMethod, wireguard::WireguardNetwork, + }, + }, + random::gen_alphanumeric, +}; + +/// Fixed wall-clock window for the whole in-progress MFA flow, including collection. +/// +/// Supersedes the in-memory `ClientLoginSession`, whose map entries lived for +/// `CLIENT_SESSION_TIMEOUT` (5 minutes). The window is deliberately doubled: a VPN MFA login +/// may require a remote mobile approval or an OIDC redirect that outlives the old in-memory +/// entry, and the durable row is reaped by a background job instead of a per-entry expiry. +pub const VPN_MFA_SESSION_TIMEOUT: Duration = Duration::from_mins(10); + +/// Per-step cap on proof-verification failures. A sanity/abuse limit, not a lockout. +pub const MFA_FAILED_ATTEMPT_CAP: i32 = 5; + +/// Point-in-time snapshot of the resolved MFA flow, frozen at `start`. +/// +/// `flow_id` is attribution-only: written once and recorded in the immutable +/// authorization activity-log entry at delivery, never re-read to drive the flow. +#[derive(Clone, Debug, Deserialize, PartialEq, Serialize)] +pub struct StepsSnapshot { + pub flow_id: Id, + pub steps: Vec, +} + +/// Attribution for a completed MFA session: the frozen step snapshot plus the governing flow's +/// title. `flow_name` is resolved at collection and is `None` when the flow was deleted +/// mid-session, which is a display concern rather than an error. +#[derive(Clone, Debug, Deserialize, PartialEq, Serialize)] +pub struct MfaAttribution { + pub snapshot: StepsSnapshot, + pub flow_name: Option, +} + +/// A single step within a frozen flow snapshot. +#[derive(Clone, Debug, Deserialize, PartialEq, Serialize)] +pub struct Step { + pub methods: Vec, + /// The method that satisfied this step, written by `advance` from the step's + /// `ephemeral_state.selected_method` before that state is cleared. + #[serde(default)] + pub satisfied: Option, +} + +/// Per-step ephemeral attempt state, cleared to NULL on `advance`. +#[derive(Clone, Debug, Deserialize, PartialEq, Serialize)] +pub struct EphemeralState { + pub step_attempt_id: String, + pub selected_method: VpnClientMfaMethod, + #[serde(default)] + pub openid_auth_completed: bool, + #[serde(default)] + pub mobile_approved: bool, + #[serde(default)] + pub biometric_challenge: Option, +} + +/// Result of `start`, carrying the raw token (returned exactly once), the id of the first +/// attempt minted alongside the row, and the hash of any session that was superseded. +pub struct StartOutcome { + pub token: String, + pub step_attempt_id: String, + pub superseded_token_hash: Option, +} + +/// The location, device, and user a VPN MFA session references, loaded together for event +/// context and authorization. +pub struct MfaSessionContext { + pub location: WireguardNetwork, + pub device: Device, + pub user: User, +} + +/// Outcome of advancing to the next step. +#[derive(Clone, Debug, PartialEq)] +pub enum StepOutcome { + /// The session advanced to `next_step` (0-indexed). + Advanced { next_step: usize }, + /// The final step completed; collection is the next action. + Complete, +} + +/// Row shape returned by `advance`: the new step index and the updated snapshot. +struct AdvanceRow { + current_step: i32, + steps_snapshot: Json, +} + +/// A durable in-progress VPN MFA session. +#[derive(Clone, Debug, Model)] +#[table(vpn_client_mfa_session)] +pub struct VpnClientMfaSession { + pub id: I, + pub token_hash: String, + pub location_id: Id, + pub device_id: Id, + pub user_id: Id, + /// `#[model(json)]` makes the derive emit a `"steps_snapshot: _"` type override so the + /// generated queries decode the `jsonb` column into `Json`, and binds it by + /// reference with a no-op cast so sqlx's compile-time type check (which maps `jsonb` to + /// `serde_json::Value`) is skipped. + #[model(json)] + pub steps_snapshot: Json, + pub current_step: i32, + /// Same `#[model(json)]` treatment as `steps_snapshot`, for a nullable `jsonb` column. + #[model(json)] + pub ephemeral_state: Option>, + pub failed_attempts: i32, + pub created_at: NaiveDateTime, + pub expires_at: NaiveDateTime, +} + +/// Hash an opaque token for storage and lookup: base64url-nopad SHA-256. +#[must_use] +pub fn hash_token(token: &str) -> String { + URL_SAFE_NO_PAD.encode(Sha256::digest(token.as_bytes())) +} + +/// Mint a fresh attempt on the current step, returning its `step_attempt_id` and the JSON the +/// `ephemeral_state` column stores. Shared by `start` (which writes it in the same statement +/// that mints the row) and `begin_attempt` (which overwrites a prior attempt in place). +fn new_attempt_state( + method: VpnClientMfaMethod, + challenge: Option, +) -> sqlx::Result<(String, serde_json::Value)> { + let step_attempt_id = gen_alphanumeric(32); + let state = EphemeralState { + step_attempt_id: step_attempt_id.clone(), + selected_method: method, + openid_auth_completed: false, + mobile_approved: false, + biometric_challenge: challenge, + }; + let state_json = + serde_json::to_value(&state).map_err(|err| sqlx::Error::Decode(Box::new(err)))?; + + Ok((step_attempt_id, state_json)) +} + +impl VpnClientMfaSession { + /// Begin a new in-progress MFA session, superseding any existing session for the same + /// `(location_id, device_id)`. + /// + /// The first attempt is minted here rather than by a follow-up `begin_attempt`: `method` + /// and `challenge` are written by the same statement that mints the row, so the session is + /// born with its attempt. Splitting the two writes left a window in which a concurrent + /// `start` could take the `ON CONFLICT DO UPDATE` branch (which preserves the row id) and + /// have the losing caller's `begin_attempt` land on the winner's row, handing that client a + /// token whose attempt carries a method it never selected. + /// + /// `ttl` is a parameter (rather than a read of `VPN_MFA_SESSION_TIMEOUT`) so expiry can be + /// exercised in tests without a 10-minute wait. + #[allow(clippy::too_many_arguments)] + pub async fn start( + conn: &mut PgConnection, + location_id: Id, + device_id: Id, + user_id: Id, + flow_id: Id, + steps: Vec>, + method: VpnClientMfaMethod, + challenge: Option, + ttl: Duration, + ) -> sqlx::Result<(Self, StartOutcome)> { + let token = gen_alphanumeric(32); + let hash = hash_token(&token); + let snapshot = StepsSnapshot { + flow_id, + steps: steps + .into_iter() + .map(|methods| Step { + methods, + satisfied: None, + }) + .collect(), + }; + let snapshot_json = + serde_json::to_value(&snapshot).map_err(|err| sqlx::Error::Decode(Box::new(err)))?; + let (step_attempt_id, state_json) = new_attempt_state(method, challenge)?; + let created_at = Utc::now().naive_utc(); + let expires_at = created_at + TimeDelta::seconds(ttl.as_secs() as i64); + + // Supersede any existing session for this (location, device), capturing its token hash so + // the caller can cancel its waiter. The DELETE and the upsert run in one transaction so a + // concurrent reader never observes the gap between them. The unique index plus the + // `ON CONFLICT` upsert below closes the concurrent double-`Start` race (last-writer-wins), + // and because the upsert also writes `ephemeral_state`, the winner's attempt is committed + // with its row rather than by a second write a loser could interleave with. + // + // This is deliberately a transaction rather than one data-modifying CTE: in + // `WITH d AS (DELETE ...) INSERT ...` the index insert is checked before the delete + // becomes visible, so the upsert can still raise a unique violation. + let mut tx = conn.begin().await?; + + let superseded_token_hash = query_scalar!( + "DELETE FROM vpn_client_mfa_session \ + WHERE location_id = $1 AND device_id = $2 \ + RETURNING token_hash", + location_id, + device_id, + ) + .fetch_optional(&mut *tx) + .await?; + + let session = query_as!( + Self, + "INSERT INTO vpn_client_mfa_session \ + (token_hash, location_id, device_id, user_id, steps_snapshot, current_step, ephemeral_state, failed_attempts, created_at, expires_at) \ + VALUES ($1, $2, $3, $4, $5, 0, $6, 0, $7, $8) \ + ON CONFLICT (location_id, device_id) DO UPDATE SET \ + token_hash = EXCLUDED.token_hash, \ + user_id = EXCLUDED.user_id, \ + steps_snapshot = EXCLUDED.steps_snapshot, \ + current_step = EXCLUDED.current_step, \ + ephemeral_state = EXCLUDED.ephemeral_state, \ + failed_attempts = EXCLUDED.failed_attempts, \ + created_at = EXCLUDED.created_at, \ + expires_at = EXCLUDED.expires_at \ + RETURNING \ + id, token_hash, location_id, device_id, user_id, \ + steps_snapshot \"steps_snapshot: Json\", current_step, \ + ephemeral_state \"ephemeral_state: Json\", failed_attempts, \ + created_at, expires_at", + hash, + location_id, + device_id, + user_id, + snapshot_json, + state_json, + created_at, + expires_at, + ) + .fetch_one(&mut *tx) + .await?; + + tx.commit().await?; + + Ok(( + session, + StartOutcome { + token, + step_attempt_id, + superseded_token_hash, + }, + )) + } + + /// Look up an active session by raw token, hashing internally. + /// + /// Returns `Ok(None)` for an unknown token and an expired session. A row whose snapshot + /// fails to deserialize surfaces as an error, as do other database errors; the caller owns + /// the decision of how to surface them. + pub async fn find_active_by_token<'e, E: PgExecutor<'e>>( + executor: E, + token: &str, + ) -> sqlx::Result> { + let hash = hash_token(token); + query_as!( + Self, + "SELECT id, token_hash, location_id, device_id, user_id, \ + steps_snapshot \"steps_snapshot: Json\", current_step, \ + ephemeral_state \"ephemeral_state: Json\", failed_attempts, \ + created_at, expires_at \ + FROM vpn_client_mfa_session \ + WHERE token_hash = $1 AND expires_at > (now() AT TIME ZONE 'UTC')", + hash, + ) + .fetch_optional(executor) + .await + } + + /// Load the location, device, and user this session references. + /// + /// Returns `Ok(None)` if any referenced entity no longer exists (deleted after the session + /// was started); callers map the `None` to their own status. + pub async fn load_context(&self, pool: &PgPool) -> sqlx::Result> { + let Some(location) = WireguardNetwork::find_by_id(pool, self.location_id).await? else { + return Ok(None); + }; + let Some(device) = Device::find_by_id(pool, self.device_id).await? else { + return Ok(None); + }; + let Some(user) = User::find_by_id(pool, self.user_id).await? else { + return Ok(None); + }; + Ok(Some(MfaSessionContext { + location, + device, + user, + })) + } + + /// The methods available on the current step. + #[must_use] + pub fn current_step_methods(&self) -> &[VpnClientMfaMethod] { + self.steps_snapshot + .0 + .steps + .get(self.current_step as usize) + .map_or(&[], |step| step.methods.as_slice()) + } + + /// Begin (or re-issue) an attempt on the current step, overwriting any prior attempt. + /// + /// Returns the fresh `step_attempt_id`, which every async completion (OIDC callback, + /// mobile approve) must carry and match. + /// + /// The *first* attempt of a session is not minted here: `start` writes it inline, so this is + /// for re-issuing an attempt on the current step and for arming each subsequent step. + pub async fn begin_attempt( + &self, + conn: &mut PgConnection, + method: VpnClientMfaMethod, + challenge: Option, + ) -> sqlx::Result { + let (step_attempt_id, state_json) = new_attempt_state(method, challenge)?; + + query!( + "UPDATE vpn_client_mfa_session SET ephemeral_state = $2 WHERE id = $1", + self.id, + state_json, + ) + .execute(&mut *conn) + .await?; + + Ok(step_attempt_id) + } + + /// Mark the current attempt's OIDC verification complete. + /// + /// Returns `true` if the mark applied; a stale `step_attempt_id` is a no-op. + pub async fn mark_oidc_completed( + &self, + conn: &mut PgConnection, + step_attempt_id: &str, + ) -> sqlx::Result { + self.mark_flag(conn, step_attempt_id, "openid_auth_completed") + .await + } + + /// Mark the current attempt's mobile approval complete. + /// + /// Symmetric to [`Self::mark_oidc_completed`]: returns `true` if the mark applied, and a + /// stale `step_attempt_id` is a no-op. The caller verifies the approval signature first. + pub async fn mark_mobile_approved( + &self, + conn: &mut PgConnection, + step_attempt_id: &str, + ) -> sqlx::Result { + self.mark_flag(conn, step_attempt_id, "mobile_approved") + .await + } + + /// Set a named completion flag on the current attempt, gated on a matching + /// `step_attempt_id`. Returns `true` if the flag was set (0 rows otherwise). + async fn mark_flag( + &self, + conn: &mut PgConnection, + step_attempt_id: &str, + flag: &str, + ) -> sqlx::Result { + let result = query!( + "UPDATE vpn_client_mfa_session \ + SET ephemeral_state = jsonb_set(ephemeral_state, ARRAY[$2]::text[], 'true'::jsonb) \ + WHERE id = $1 AND ephemeral_state IS NOT NULL AND ephemeral_state->>'step_attempt_id' = $3", + self.id, + flag, + step_attempt_id, + ) + .execute(&mut *conn) + .await?; + Ok(result.rows_affected() > 0) + } + + /// Advance to the next step, clearing `ephemeral_state` and resetting `failed_attempts`. + /// + /// Records the closing step's proof into the snapshot first: + /// `steps[current_step].satisfied = ephemeral_state.selected_method`. The write and the + /// clear land in one statement so the proof cannot be lost between them. A NULL + /// `ephemeral_state` leaves `satisfied` unset rather than erroring. + /// + /// Returns the new step outcome and the updated snapshot, so the caller can read the + /// just-recorded `satisfied` method without a second query. Does not extend `expires_at` + /// (fixed window). + pub async fn advance( + &self, + conn: &mut PgConnection, + ) -> sqlx::Result<(StepOutcome, StepsSnapshot)> { + let row = query_as!( + AdvanceRow, + "UPDATE vpn_client_mfa_session \ + SET steps_snapshot = CASE \ + WHEN ephemeral_state IS NOT NULL THEN jsonb_set( \ + steps_snapshot, \ + ARRAY['steps', current_step::text, 'satisfied'], \ + ephemeral_state->'selected_method' \ + ) \ + ELSE steps_snapshot \ + END, \ + ephemeral_state = NULL, \ + current_step = current_step + 1, \ + failed_attempts = 0 \ + WHERE id = $1 \ + RETURNING current_step, steps_snapshot \"steps_snapshot: Json\"", + self.id, + ) + .fetch_one(&mut *conn) + .await?; + + let total_steps = self.steps_snapshot.0.steps.len() as i32; + let outcome = if row.current_step >= total_steps { + StepOutcome::Complete + } else { + StepOutcome::Advanced { + next_step: row.current_step as usize, + } + }; + + Ok((outcome, row.steps_snapshot.0)) + } + + /// Increment the per-step proof-failure counter. + /// + /// Returns `true` once [`MFA_FAILED_ATTEMPT_CAP`] is reached. Does not delete the session; + /// the orchestrator owns deletion and the terminal event. + pub async fn increment_failed_attempts(&self, conn: &mut PgConnection) -> sqlx::Result { + let failed_attempts = query_scalar!( + "UPDATE vpn_client_mfa_session \ + SET failed_attempts = failed_attempts + 1 \ + WHERE id = $1 \ + RETURNING failed_attempts", + self.id, + ) + .fetch_one(&mut *conn) + .await?; + Ok(failed_attempts >= MFA_FAILED_ATTEMPT_CAP) + } +} + +/// Delete every session whose fixed window has elapsed. Silent hygiene, not correctness. +pub async fn reap_expired(pool: &PgPool) -> sqlx::Result { + let result = + query!("DELETE FROM vpn_client_mfa_session WHERE expires_at < (now() AT TIME ZONE 'UTC')") + .execute(pool) + .await?; + let count = result.rows_affected(); + debug!("Reaped {count} expired MFA session(s)"); + Ok(count) +} + +#[cfg(test)] +mod tests; diff --git a/crates/defguard_common/src/db/models/vpn_client_mfa_session/tests.rs b/crates/defguard_common/src/db/models/vpn_client_mfa_session/tests.rs new file mode 100644 index 0000000000..be6fdf7458 --- /dev/null +++ b/crates/defguard_common/src/db/models/vpn_client_mfa_session/tests.rs @@ -0,0 +1,699 @@ +use std::{ + sync::atomic::{AtomicUsize, Ordering}, + time::Duration, +}; + +use sqlx::postgres::{PgConnectOptions, PgPoolOptions}; + +use super::*; +use crate::db::{ + Id, + models::{ + biometric_auth::BiometricChallenge, + device::{Device, DeviceType}, + user::User, + vpn_client_session::VpnClientMfaMethod, + wireguard::WireguardNetwork, + }, + setup_pool, +}; + +static COUNTER: AtomicUsize = AtomicUsize::new(0); + +fn next_suffix() -> String { + COUNTER.fetch_add(1, Ordering::Relaxed).to_string() +} + +async fn create_location(pool: &sqlx::PgPool) -> WireguardNetwork { + create_location_with_address(pool, "10.0.6.1/24").await +} + +async fn create_location_with_address(pool: &sqlx::PgPool, address: &str) -> WireguardNetwork { + WireguardNetwork::default() + .try_set_address(address) + .unwrap() + .save(pool) + .await + .unwrap() +} + +async fn create_user(pool: &sqlx::PgPool) -> User { + let suffix = next_suffix(); + User::new( + format!("mfa-session-user-{suffix}"), + None, + "Ln".to_string(), + "Fn".to_string(), + format!("mfa-{suffix}@t.com"), + None, + ) + .save(pool) + .await + .unwrap() +} + +async fn create_device(pool: &sqlx::PgPool, user_id: Id) -> Device { + let suffix = next_suffix(); + Device::new( + format!("mfa-session-device-{suffix}"), + format!("device-pubkey-{suffix}"), + user_id, + DeviceType::User, + None, + true, + ) + .save(pool) + .await + .unwrap() +} + +async fn start_session(pool: &sqlx::PgPool) -> (VpnClientMfaSession, StartOutcome) { + start_session_with_ttl(pool, Duration::from_mins(10)).await +} + +async fn start_session_with_ttl( + pool: &sqlx::PgPool, + ttl: Duration, +) -> (VpnClientMfaSession, StartOutcome) { + let location = create_location(pool).await; + let user = create_user(pool).await; + let device = create_device(pool, user.id).await; + let mut tx = pool.begin().await.unwrap(); + let result = VpnClientMfaSession::::start( + &mut tx, + location.id, + device.id, + user.id, + 1, + vec![ + vec![VpnClientMfaMethod::Totp], + vec![VpnClientMfaMethod::Email], + ], + VpnClientMfaMethod::Totp, + None, + ttl, + ) + .await + .unwrap(); + tx.commit().await.unwrap(); + result +} + +async fn refetch(pool: &sqlx::PgPool, token: &str) -> VpnClientMfaSession { + VpnClientMfaSession::::find_active_by_token(pool, token) + .await + .unwrap() + .expect("expected active session") +} + +#[sqlx::test] +async fn test_start_supersedes_existing_session(_: PgPoolOptions, options: PgConnectOptions) { + let pool = setup_pool(options).await; + let location = create_location(&pool).await; + let user = create_user(&pool).await; + let device = create_device(&pool, user.id).await; + let steps = vec![vec![VpnClientMfaMethod::Totp]]; + + let mut tx = pool.begin().await.unwrap(); + let (first, first_outcome) = VpnClientMfaSession::::start( + &mut tx, + location.id, + device.id, + user.id, + 1, + steps.clone(), + VpnClientMfaMethod::Totp, + None, + Duration::from_mins(10), + ) + .await + .unwrap(); + tx.commit().await.unwrap(); + + assert_eq!(first.current_step_methods(), [VpnClientMfaMethod::Totp]); + // The raw token is never stored; only its hash is. + assert_eq!(first.token_hash, hash_token(&first_outcome.token)); + assert_ne!(first.token_hash, first_outcome.token); + assert!( + VpnClientMfaSession::::find_active_by_token(&pool, &first_outcome.token) + .await + .unwrap() + .is_some() + ); + + let mut tx = pool.begin().await.unwrap(); + let (_second, second_outcome) = VpnClientMfaSession::::start( + &mut tx, + location.id, + device.id, + user.id, + 1, + steps, + VpnClientMfaMethod::Totp, + None, + Duration::from_mins(10), + ) + .await + .unwrap(); + tx.commit().await.unwrap(); + + assert_eq!( + second_outcome.superseded_token_hash.as_deref(), + Some(first.token_hash.as_str()) + ); + // The superseded token no longer validates; the new one does. + assert!( + VpnClientMfaSession::::find_active_by_token(&pool, &first_outcome.token) + .await + .unwrap() + .is_none() + ); + assert!( + VpnClientMfaSession::::find_active_by_token(&pool, &second_outcome.token) + .await + .unwrap() + .is_some() + ); +} + +#[sqlx::test] +async fn test_start_returns_superseded_token_hash(_: PgPoolOptions, options: PgConnectOptions) { + let pool = setup_pool(options).await; + let location = create_location(&pool).await; + let user = create_user(&pool).await; + let device = create_device(&pool, user.id).await; + let steps = vec![vec![VpnClientMfaMethod::Totp]]; + + let mut tx = pool.begin().await.unwrap(); + let (first, _) = VpnClientMfaSession::::start( + &mut tx, + location.id, + device.id, + user.id, + 1, + steps.clone(), + VpnClientMfaMethod::Totp, + None, + Duration::from_mins(10), + ) + .await + .unwrap(); + tx.commit().await.unwrap(); + + let mut tx = pool.begin().await.unwrap(); + let (_, outcome) = VpnClientMfaSession::::start( + &mut tx, + location.id, + device.id, + user.id, + 1, + steps, + VpnClientMfaMethod::Totp, + None, + Duration::from_mins(10), + ) + .await + .unwrap(); + tx.commit().await.unwrap(); + + assert_eq!( + outcome.superseded_token_hash.as_deref(), + Some(first.token_hash.as_str()) + ); +} + +/// `start` must commit the first attempt with the row it mints. If the attempt were a second +/// write, a concurrent `start` taking the `ON CONFLICT DO UPDATE` branch (which preserves the +/// row id) could have the losing caller's attempt land on the winner's row. +#[sqlx::test] +async fn test_start_mints_first_attempt_with_row(_: PgPoolOptions, options: PgConnectOptions) { + let pool = setup_pool(options).await; + let location = create_location(&pool).await; + let user = create_user(&pool).await; + let device = create_device(&pool, user.id).await; + let steps = vec![vec![VpnClientMfaMethod::MobileApprove]]; + let challenge = BiometricChallenge::new(); + + let mut tx = pool.begin().await.unwrap(); + let (session, outcome) = VpnClientMfaSession::::start( + &mut tx, + location.id, + device.id, + user.id, + 1, + steps.clone(), + VpnClientMfaMethod::MobileApprove, + Some(challenge.clone()), + Duration::from_mins(10), + ) + .await + .unwrap(); + tx.commit().await.unwrap(); + + // The row comes back already armed: no follow-up write is needed to begin the attempt. + let state = session + .ephemeral_state + .clone() + .expect("start must return a session carrying its first attempt") + .0; + assert_eq!(state.step_attempt_id, outcome.step_attempt_id); + assert_eq!(state.selected_method, VpnClientMfaMethod::MobileApprove); + assert_eq!( + state.biometric_challenge.as_ref().map(|c| &c.challenge), + Some(&challenge.challenge) + ); + assert!(!state.openid_auth_completed); + assert!(!state.mobile_approved); + + // What was returned is what was persisted. + let persisted = refetch(&pool, &outcome.token) + .await + .ephemeral_state + .unwrap() + .0; + assert_eq!(persisted, state); + + // Superseding rewrites the attempt in that same write, so the surviving token is always + // paired with the attempt its own `start` minted. + let mut tx = pool.begin().await.unwrap(); + let (_second, second_outcome) = VpnClientMfaSession::::start( + &mut tx, + location.id, + device.id, + user.id, + 1, + steps, + VpnClientMfaMethod::Totp, + None, + Duration::from_mins(10), + ) + .await + .unwrap(); + tx.commit().await.unwrap(); + + let persisted = refetch(&pool, &second_outcome.token) + .await + .ephemeral_state + .unwrap() + .0; + assert_ne!(second_outcome.step_attempt_id, outcome.step_attempt_id); + assert_eq!(persisted.step_attempt_id, second_outcome.step_attempt_id); + assert_eq!(persisted.selected_method, VpnClientMfaMethod::Totp); + assert!(persisted.biometric_challenge.is_none()); +} + +#[sqlx::test] +async fn test_find_active_by_token_rejects_expired(_: PgPoolOptions, options: PgConnectOptions) { + let pool = setup_pool(options).await; + let location = create_location(&pool).await; + let user = create_user(&pool).await; + let device = create_device(&pool, user.id).await; + + let mut tx = pool.begin().await.unwrap(); + let (_session, outcome) = VpnClientMfaSession::::start( + &mut tx, + location.id, + device.id, + user.id, + 1, + vec![vec![VpnClientMfaMethod::Totp]], + VpnClientMfaMethod::Totp, + None, + Duration::ZERO, + ) + .await + .unwrap(); + tx.commit().await.unwrap(); + + assert!( + VpnClientMfaSession::::find_active_by_token(&pool, &outcome.token) + .await + .unwrap() + .is_none() + ); +} + +#[sqlx::test] +async fn test_find_active_by_token_rejects_unknown(_: PgPoolOptions, options: PgConnectOptions) { + let pool = setup_pool(options).await; + assert!( + VpnClientMfaSession::::find_active_by_token(&pool, "nonexistent-token") + .await + .unwrap() + .is_none() + ); +} + +#[sqlx::test] +async fn test_advance_clears_ephemeral_state(_: PgPoolOptions, options: PgConnectOptions) { + let pool = setup_pool(options).await; + let (session, outcome) = start_session(&pool).await; + + let mut tx = pool.begin().await.unwrap(); + session + .begin_attempt(&mut tx, VpnClientMfaMethod::Totp, None) + .await + .unwrap(); + tx.commit().await.unwrap(); + assert!( + refetch(&pool, &outcome.token) + .await + .ephemeral_state + .is_some() + ); + + let session = refetch(&pool, &outcome.token).await; + let mut tx = pool.begin().await.unwrap(); + let (result, _) = session.advance(&mut tx).await.unwrap(); + tx.commit().await.unwrap(); + assert_eq!(result, StepOutcome::Advanced { next_step: 1 }); + + let session = refetch(&pool, &outcome.token).await; + assert!(session.ephemeral_state.is_none()); + assert_eq!(session.current_step, 1); + assert_eq!(session.failed_attempts, 0); +} + +#[sqlx::test] +async fn test_advance_records_satisfied_method(_: PgPoolOptions, options: PgConnectOptions) { + let pool = setup_pool(options).await; + let (session, outcome) = start_session(&pool).await; + + let mut tx = pool.begin().await.unwrap(); + session + .begin_attempt(&mut tx, VpnClientMfaMethod::Totp, None) + .await + .unwrap(); + tx.commit().await.unwrap(); + + let session = refetch(&pool, &outcome.token).await; + let mut tx = pool.begin().await.unwrap(); + let (result, _) = session.advance(&mut tx).await.unwrap(); + tx.commit().await.unwrap(); + assert_eq!(result, StepOutcome::Advanced { next_step: 1 }); + + let snapshot = &refetch(&pool, &outcome.token).await.steps_snapshot.0; + assert_eq!(snapshot.steps[0].satisfied, Some(VpnClientMfaMethod::Totp)); + assert_eq!(snapshot.steps[1].satisfied, None); +} + +#[sqlx::test] +async fn test_advance_does_not_extend_expiry(_: PgPoolOptions, options: PgConnectOptions) { + let pool = setup_pool(options).await; + let (session, outcome) = start_session(&pool).await; + let original_expiry = session.expires_at; + + let mut tx = pool.begin().await.unwrap(); + session.advance(&mut tx).await.unwrap(); + tx.commit().await.unwrap(); + + assert_eq!( + refetch(&pool, &outcome.token).await.expires_at, + original_expiry + ); +} + +#[sqlx::test] +async fn test_increment_failed_attempts_caps_at_five(_: PgPoolOptions, options: PgConnectOptions) { + let pool = setup_pool(options).await; + let (session, outcome) = start_session(&pool).await; + + let mut tx = pool.begin().await.unwrap(); + let mut at_cap = false; + for i in 0..MFA_FAILED_ATTEMPT_CAP { + at_cap = session.increment_failed_attempts(&mut tx).await.unwrap(); + if i + 1 < MFA_FAILED_ATTEMPT_CAP { + assert!(!at_cap); + } + } + tx.commit().await.unwrap(); + assert!(at_cap); + + assert_eq!( + refetch(&pool, &outcome.token).await.failed_attempts, + MFA_FAILED_ATTEMPT_CAP + ); +} + +#[sqlx::test] +async fn test_mark_oidc_completed_ignores_stale_attempt( + _: PgPoolOptions, + options: PgConnectOptions, +) { + let pool = setup_pool(options).await; + let (session, outcome) = start_session(&pool).await; + + let mut tx = pool.begin().await.unwrap(); + let attempt_id = session + .begin_attempt(&mut tx, VpnClientMfaMethod::Oidc, None) + .await + .unwrap(); + tx.commit().await.unwrap(); + + let session = refetch(&pool, &outcome.token).await; + let mut tx = pool.begin().await.unwrap(); + assert!( + !session + .mark_oidc_completed(&mut tx, "stale-id") + .await + .unwrap() + ); + tx.commit().await.unwrap(); + assert!( + !refetch(&pool, &outcome.token) + .await + .ephemeral_state + .unwrap() + .openid_auth_completed + ); + + let session = refetch(&pool, &outcome.token).await; + let mut tx = pool.begin().await.unwrap(); + assert!( + session + .mark_oidc_completed(&mut tx, &attempt_id) + .await + .unwrap() + ); + tx.commit().await.unwrap(); + assert!( + refetch(&pool, &outcome.token) + .await + .ephemeral_state + .unwrap() + .openid_auth_completed + ); +} + +#[sqlx::test] +async fn test_mark_mobile_approved_ignores_stale_attempt( + _: PgPoolOptions, + options: PgConnectOptions, +) { + let pool = setup_pool(options).await; + let (session, outcome) = start_session(&pool).await; + + let mut tx = pool.begin().await.unwrap(); + let attempt_id = session + .begin_attempt(&mut tx, VpnClientMfaMethod::MobileApprove, None) + .await + .unwrap(); + tx.commit().await.unwrap(); + + let session = refetch(&pool, &outcome.token).await; + let mut tx = pool.begin().await.unwrap(); + assert!( + !session + .mark_mobile_approved(&mut tx, "stale-id") + .await + .unwrap() + ); + tx.commit().await.unwrap(); + assert!( + !refetch(&pool, &outcome.token) + .await + .ephemeral_state + .unwrap() + .mobile_approved + ); + + let session = refetch(&pool, &outcome.token).await; + let mut tx = pool.begin().await.unwrap(); + assert!( + session + .mark_mobile_approved(&mut tx, &attempt_id) + .await + .unwrap() + ); + tx.commit().await.unwrap(); + assert!( + refetch(&pool, &outcome.token) + .await + .ephemeral_state + .unwrap() + .mobile_approved + ); +} + +#[sqlx::test] +async fn test_begin_attempt_replaces_prior_attempt(_: PgPoolOptions, options: PgConnectOptions) { + let pool = setup_pool(options).await; + let (session, outcome) = start_session(&pool).await; + + let mut tx = pool.begin().await.unwrap(); + let first = session + .begin_attempt(&mut tx, VpnClientMfaMethod::Totp, None) + .await + .unwrap(); + let second = session + .begin_attempt(&mut tx, VpnClientMfaMethod::Email, None) + .await + .unwrap(); + tx.commit().await.unwrap(); + assert_ne!(first, second); + + let state = refetch(&pool, &outcome.token) + .await + .ephemeral_state + .unwrap(); + assert_eq!(state.step_attempt_id, second); + assert_eq!(state.selected_method, VpnClientMfaMethod::Email); +} + +#[sqlx::test] +async fn test_reap_expired_deletes_only_expired(_: PgPoolOptions, options: PgConnectOptions) { + let pool = setup_pool(options).await; + let (_expired, expired_outcome) = start_session_with_ttl(&pool, Duration::ZERO).await; + let (_active, active_outcome) = start_session_with_ttl(&pool, Duration::from_mins(10)).await; + + let reaped = reap_expired(&pool).await.unwrap(); + assert_eq!(reaped, 1); + assert!( + VpnClientMfaSession::::find_active_by_token(&pool, &active_outcome.token) + .await + .unwrap() + .is_some() + ); + assert!( + VpnClientMfaSession::::find_active_by_token(&pool, &expired_outcome.token) + .await + .unwrap() + .is_none() + ); +} + +#[sqlx::test] +async fn test_concurrent_starts_leave_single_row(_: PgPoolOptions, options: PgConnectOptions) { + let pool = setup_pool(options).await; + let location = create_location(&pool).await; + let user = create_user(&pool).await; + let device = create_device(&pool, user.id).await; + let steps = vec![vec![VpnClientMfaMethod::Totp]]; + + let mut conn_a = pool.acquire().await.unwrap(); + let mut conn_b = pool.acquire().await.unwrap(); + + let (a, b) = tokio::join!( + VpnClientMfaSession::::start( + &mut conn_a, + location.id, + device.id, + user.id, + 1, + steps.clone(), + VpnClientMfaMethod::Totp, + None, + Duration::from_mins(10), + ), + VpnClientMfaSession::::start( + &mut conn_b, + location.id, + device.id, + user.id, + 1, + steps.clone(), + VpnClientMfaMethod::Totp, + None, + Duration::from_mins(10), + ), + ); + let ((_, a_outcome), (_, b_outcome)) = (a.unwrap(), b.unwrap()); + + // Exactly one live row for this (location, device), regardless of interleaving. + let count = sqlx::query_scalar!( + "SELECT count(*) FROM vpn_client_mfa_session WHERE location_id = $1 AND device_id = $2", + location.id, + device.id, + ) + .fetch_one(&pool) + .await + .unwrap(); + assert_eq!(count, Some(1)); + + // Exactly one of the two minted tokens survives; the other was superseded. + let a_live = VpnClientMfaSession::::find_active_by_token(&pool, &a_outcome.token) + .await + .unwrap() + .is_some(); + let b_live = VpnClientMfaSession::::find_active_by_token(&pool, &b_outcome.token) + .await + .unwrap() + .is_some(); + assert_ne!( + a_live, b_live, + "exactly one token must survive concurrent start" + ); +} + +#[sqlx::test] +async fn test_same_device_two_locations_both_live(_: PgPoolOptions, options: PgConnectOptions) { + let pool = setup_pool(options).await; + let user = create_user(&pool).await; + let device = create_device(&pool, user.id).await; + let location_a = create_location_with_address(&pool, "10.0.6.1/24").await; + let location_b = create_location_with_address(&pool, "10.0.7.1/24").await; + let steps = vec![vec![VpnClientMfaMethod::Totp]]; + + let mut conn_a = pool.acquire().await.unwrap(); + let mut conn_b = pool.acquire().await.unwrap(); + + let (a, b) = tokio::join!( + VpnClientMfaSession::::start( + &mut conn_a, + location_a.id, + device.id, + user.id, + 1, + steps.clone(), + VpnClientMfaMethod::Totp, + None, + Duration::from_mins(10), + ), + VpnClientMfaSession::::start( + &mut conn_b, + location_b.id, + device.id, + user.id, + 1, + steps.clone(), + VpnClientMfaMethod::Totp, + None, + Duration::from_mins(10), + ), + ); + let ((_, a_outcome), (_, b_outcome)) = (a.unwrap(), b.unwrap()); + + // Uniqueness is per (location, device), so both rows stay live. + assert!( + VpnClientMfaSession::::find_active_by_token(&pool, &a_outcome.token) + .await + .unwrap() + .is_some() + ); + assert!( + VpnClientMfaSession::::find_active_by_token(&pool, &b_outcome.token) + .await + .unwrap() + .is_some() + ); +} diff --git a/crates/defguard_common/src/db/models/vpn_client_session.rs b/crates/defguard_common/src/db/models/vpn_client_session.rs index 347ae2fb25..de210b1a1f 100644 --- a/crates/defguard_common/src/db/models/vpn_client_session.rs +++ b/crates/defguard_common/src/db/models/vpn_client_session.rs @@ -77,8 +77,7 @@ pub struct VpnClientSession { pub created_at: NaiveDateTime, pub connected_at: Option, pub disconnected_at: Option, - #[model(option)] - pub mfa_method: Option, + pub is_mfa_session: bool, #[model(enum)] pub state: VpnClientSessionState, pub preshared_key: Option, @@ -91,7 +90,7 @@ impl VpnClientSession { user_id: Id, device_id: Id, connected_at: Option, - mfa_method: Option, + is_mfa_session: bool, ) -> Self { // determine session state let state = if connected_at.is_some() { @@ -108,7 +107,7 @@ impl VpnClientSession { created_at: Utc::now().naive_utc(), connected_at, disconnected_at: None, - mfa_method, + is_mfa_session, state, preshared_key: None, } @@ -127,7 +126,7 @@ impl VpnClientSession { query_as!( Self, "SELECT id, location_id, user_id, device_id, created_at, connected_at, disconnected_at, \ - mfa_method \"mfa_method: VpnClientMfaMethod\", state \"state: VpnClientSessionState\", preshared_key \ + is_mfa_session, state \"state: VpnClientSessionState\", preshared_key \ FROM vpn_client_session \ WHERE location_id = $1 AND device_id = $2 AND state IN ('new', 'connected') \ ORDER BY created_at DESC, id DESC \ @@ -165,7 +164,7 @@ impl VpnClientSession { query_as!( Self, "SELECT s.id, location_id, user_id, device_id, created_at, s.connected_at, disconnected_at, \ - mfa_method \"mfa_method: VpnClientMfaMethod\", state \"state: VpnClientSessionState\", preshared_key \ + is_mfa_session, state \"state: VpnClientSessionState\", preshared_key \ FROM vpn_client_session s \ LEFT JOIN LATERAL ( \ SELECT latest_handshake \ @@ -189,7 +188,7 @@ impl VpnClientSession { query_as!( Self, "SELECT id, location_id, user_id, device_id, created_at, connected_at, disconnected_at, \ - mfa_method \"mfa_method: VpnClientMfaMethod\", state \"state: VpnClientSessionState\", preshared_key \ + is_mfa_session, state \"state: VpnClientSessionState\", preshared_key \ FROM vpn_client_session \ WHERE location_id = $1 AND state = 'new' \ AND (NOW() - created_at) > $2 * interval '1 second'", @@ -207,7 +206,7 @@ impl VpnClientSession { query_as!( Self, "SELECT id, location_id, user_id, device_id, created_at, connected_at, disconnected_at, \ - mfa_method \"mfa_method: VpnClientMfaMethod\", state \"state: VpnClientSessionState\", preshared_key \ + is_mfa_session, state \"state: VpnClientSessionState\", preshared_key \ FROM vpn_client_session \ WHERE location_id = $1 AND device_id = $2 AND state IN ('new', 'connected') \ ORDER BY created_at DESC, id DESC", diff --git a/crates/defguard_common/src/db/models/wireguard.rs b/crates/defguard_common/src/db/models/wireguard.rs index e80575a989..904cba1d37 100644 --- a/crates/defguard_common/src/db/models/wireguard.rs +++ b/crates/defguard_common/src/db/models/wireguard.rs @@ -27,7 +27,7 @@ use crate::{ db::{ Id, NoId, models::{ - vpn_client_session::{VpnClientMfaMethod, VpnClientSession, VpnClientSessionState}, + vpn_client_session::{VpnClientSession, VpnClientSessionState}, vpn_session_stats::{VpnSessionStats, endpoint_without_port}, }, }, @@ -1495,7 +1495,7 @@ impl WireguardNetwork { query_as!( VpnClientSession, "SELECT id, location_id, user_id, device_id, created_at, connected_at, \ - disconnected_at, mfa_method \"mfa_method: VpnClientMfaMethod\", \ + disconnected_at, is_mfa_session, \ state \"state: VpnClientSessionState\", preshared_key \ FROM vpn_client_session \ WHERE location_id = $1 AND state = 'connected'::vpn_client_session_state", diff --git a/crates/defguard_core/src/db/models/activity_log/metadata.rs b/crates/defguard_core/src/db/models/activity_log/metadata.rs index c74fe07cb6..239c3b5c2b 100644 --- a/crates/defguard_core/src/db/models/activity_log/metadata.rs +++ b/crates/defguard_core/src/db/models/activity_log/metadata.rs @@ -10,6 +10,7 @@ use defguard_common::db::{ proxy::Proxy, settings::{LdapSyncStatus, OpenIdUsernameHandling, smtp::SmtpEncryption}, user::User, + vpn_client_mfa_session::MfaAttribution, }, }; @@ -194,7 +195,10 @@ pub struct VpnClientMetadata { pub struct VpnClientMfaMetadata { pub location: WireguardNetwork, pub device: Device, - pub method: ClientMFAMethod, + /// The complete challenge-and-response record plus the governing flow title. Flattened so + /// the serialized activity-log shape stays `{ snapshot, flow_name }`. + #[serde(flatten)] + pub attribution: MfaAttribution, /// Name of the device used to approve the login when the mobile approve MFA /// method is used. Omitted for all other methods. #[serde(skip_serializing_if = "Option::is_none")] diff --git a/crates/defguard_core/src/db/models/activity_log/mod.rs b/crates/defguard_core/src/db/models/activity_log/mod.rs index 5ac452fd02..742aba07fc 100644 --- a/crates/defguard_core/src/db/models/activity_log/mod.rs +++ b/crates/defguard_core/src/db/models/activity_log/mod.rs @@ -89,6 +89,7 @@ pub enum EventType { VpnClientMfaFailed, VpnClientSessionSuperseded, VpnClientMfaSessionSuperseded, + VpnClientMfaLoginSuperseded, // Enrollment events EnrollmentTokenAdded, EnrollmentStarted, diff --git a/crates/defguard_core/src/enterprise/directory_sync/mod.rs b/crates/defguard_core/src/enterprise/directory_sync/mod.rs index b4ddd2accc..f5b6a52b83 100644 --- a/crates/defguard_core/src/enterprise/directory_sync/mod.rs +++ b/crates/defguard_core/src/enterprise/directory_sync/mod.rs @@ -21,12 +21,11 @@ use super::{ }, ldap::utils::ldap_update_users_state, }; -#[cfg(not(test))] -use crate::enterprise::is_business_license_active; use crate::{ enterprise::{ db::models::openid_provider::DirectorySyncUserBehavior, handlers::openid_login::prune_username, + is_business_license_active, ldap::{ model::ldap_sync_allowed_for_user, utils::{ldap_add_users_to_groups, ldap_delete_users, ldap_remove_users_from_groups}, @@ -438,7 +437,6 @@ async fn sync_user_groups( pub(crate) async fn test_directory_sync_connection( pool: &PgPool, ) -> Result<(), DirectorySyncError> { - #[cfg(not(test))] if !is_business_license_active() { debug!("Enterprise is not enabled, skipping testing directory sync connection"); return Ok(()); @@ -465,7 +463,6 @@ pub async fn sync_user_groups_if_configured( ldap_tx: &UnboundedSender, dirsync_tx: &UnboundedSender, ) -> Result<(), DirectorySyncError> { - #[cfg(not(test))] if !is_business_license_active() { debug!("Enterprise is not enabled, skipping syncing user groups"); return Ok(()); @@ -1258,7 +1255,6 @@ pub(crate) async fn do_directory_sync( ldap_tx: &UnboundedSender, dirsync_tx: &UnboundedSender, ) -> Result<(), DirectorySyncError> { - #[cfg(not(test))] if !is_business_license_active() { debug!("Enterprise is not enabled, skipping performing directory sync"); return Ok(()); diff --git a/crates/defguard_core/src/enterprise/directory_sync/tests.rs b/crates/defguard_core/src/enterprise/directory_sync/tests.rs index ea865ed8aa..14596a0236 100644 --- a/crates/defguard_core/src/enterprise/directory_sync/tests.rs +++ b/crates/defguard_core/src/enterprise/directory_sync/tests.rs @@ -31,6 +31,22 @@ mod test { grpc::proto::enterprise::license::LicenseLimits, }; + /// Install a Business-tier licence with no limits. + /// + /// Tests needing specific limits build their own licence instead. + fn set_business_license() { + set_cached_license(Some(License::new( + "test".to_owned(), + false, + None, + None, + None, + LicenseTier::Business, + SupportType::Basic, + vec![], + ))); + } + async fn do_test_directory_sync(pool: &PgPool, gateway_tx: &broadcast::Sender) { let (ldap_tx, _ldap_rx) = mpsc::unbounded_channel::(); let (dirsync_tx, _dirsync_rx) = dirsync_test_channel(); @@ -69,6 +85,12 @@ mod test { target: DirectorySyncTarget, prefetch_users: bool, ) -> OpenIdProvider { + // Directory sync is a business feature and its licence gate is compiled into test + // builds, so without a licence every entry point below returns `Ok(())` without doing + // any work. Seed one here; a test wanting the unlicensed path calls + // `set_cached_license(None)` afterwards. + set_business_license(); + Settings::initialize_runtime_defaults(pool).await.unwrap(); initialize_current_settings(pool).await.unwrap(); diff --git a/crates/defguard_core/src/enterprise/grpc/desktop_client_mfa.rs b/crates/defguard_core/src/enterprise/grpc/desktop_client_mfa.rs index a3c046d4cd..51962e10ce 100644 --- a/crates/defguard_core/src/enterprise/grpc/desktop_client_mfa.rs +++ b/crates/defguard_core/src/enterprise/grpc/desktop_client_mfa.rs @@ -1,4 +1,13 @@ -use defguard_common::{db::models::Settings, types::AuthFlowType}; +use defguard_common::{ + db::{ + Id, + models::{ + Settings, + vpn_client_mfa_session::{MfaSessionContext, VpnClientMfaSession}, + }, + }, + types::AuthFlowType, +}; use defguard_proto::{ client_types::MfaMethod, proxy::{ClientMfaOidcAuthenticateRequest, DeviceInfo}, @@ -8,14 +17,11 @@ use tonic::Status; use crate::{ enterprise::{ - handlers::openid_login::{extract_state_data, user_from_claims}, + handlers::openid_login::{MfaOidcState, extract_state_data, user_from_claims}, is_business_license_active, }, events::{BidiRequestContext, BidiStreamEvent, BidiStreamEventType, DesktopClientMfaEvent}, - grpc::{ - proxy::client_mfa::{ClientLoginSession, ClientMfaServer}, - utils::parse_client_ip_agent, - }, + grpc::{proxy::client_mfa::ClientMfaServer, utils::parse_client_ip_agent}, }; impl ClientMfaServer { @@ -25,44 +31,69 @@ impl ClientMfaServer { request: ClientMfaOidcAuthenticateRequest, info: Option, ) -> Result<(), Status> { - debug!("Received OIDC MFA authentication request: {request:?}"); + debug!("Received OIDC MFA authentication request"); if !is_business_license_active() { error!("OIDC MFA method requires enterprise feature to be enabled"); return Err(Status::invalid_argument("OIDC MFA method is not supported")); } - let token = extract_state_data(&request.state).ok_or_else(|| { - error!( - "Failed to extract state data from state: {:?}", - request.state - ); + let state_data = extract_state_data(&request.state).ok_or_else(|| { + error!("Failed to extract state data from state"); Status::invalid_argument("invalid state data") })?; - if token.is_empty() { - debug!("Empty token provided in request"); - return Err(Status::invalid_argument("empty token provided")); - } - let pubkey = Self::parse_token(&token)?; - - // fetch login session - let Some(session) = self - .sessions - .read() - .expect("Failed to read-lock ClientMfaServer::sessions") - .get(&pubkey) - .cloned() + + // The MFA flow's state carries ".". A state with no valid + // attempt id (for example, an OIDC login in flight across the upgrade) is rejected rather + // than silently accepted. + let MfaOidcState { token, attempt_id } = + MfaOidcState::parse(&state_data).ok_or_else(|| { + debug!("OIDC MFA state carries no valid . payload"); + Status::invalid_argument("invalid state data") + })?; + + // Fetch the durable in-progress session by the opaque token. + let Some(session) = VpnClientMfaSession::::find_active_by_token(&self.pool, &token) + .await + .map_err(|err| { + error!("Failed to find MFA session: {err}"); + Status::internal("unexpected error") + })? else { debug!("Client login session not found"); return Err(Status::invalid_argument("login session not found")); }; - let ClientLoginSession { - method, - device, + + // Fetch the related objects for event context. + let Some(MfaSessionContext { location, + device, user, - openid_auth_completed, - biometric_challenge: _, - } = session; + }) = session.load_context(&self.pool).await.map_err(|err| { + error!("Failed to load MFA session context: {err}"); + Status::internal("unexpected error") + })? + else { + error!("MFA session references a missing location, device, or user"); + return Err(Status::internal("unexpected error")); + }; + + // The attempt recorded at start holds the selected method and step attempt id. + let Some(ephemeral) = session.ephemeral_state.as_ref() else { + debug!("No MFA attempt in progress"); + return Err(Status::invalid_argument("no MFA attempt in progress")); + }; + + // Bind the callback to the attempt it was issued for before any branch below can mutate + // or delete the session. Every abort path from here on deletes the row, so a callback + // belonging to a superseded attempt must be turned away first: otherwise a late callback + // carrying a stale id could tear down the attempt that replaced it. + if ephemeral.step_attempt_id != attempt_id { + debug!("OIDC MFA callback arrived for a superseded attempt"); + return Err(Status::invalid_argument("stale OIDC MFA attempt")); + } + + let method: MfaMethod = ephemeral.selected_method.into(); + let openid_auth_completed = ephemeral.openid_auth_completed; if openid_auth_completed { debug!("Client login session already completed"); @@ -71,14 +102,14 @@ impl ClientMfaServer { if method != MfaMethod::Oidc { debug!("Invalid MFA method for OIDC authentication: {method:?}"); - self.sessions - .write() - .expect("Failed to write-lock ClientMfaServer::sessions") - .remove(&pubkey); + self.delete_mfa_session(session).await?; 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(|err| { + error!("Failed to parse client IP and agent during OIDC MFA: {err}"); + Status::internal("unexpected error") + })?; let context = BidiRequestContext::new( user.id, user.username.clone(), @@ -95,10 +126,7 @@ impl ClientMfaServer { }) { Ok(url) => url, Err(status) => { - self.sessions - .write() - .expect("Failed to write-lock ClientMfaServer::sessions") - .remove(&pubkey); + self.delete_mfa_session(session).await?; self.emit_event(BidiStreamEvent { context, event: BidiStreamEventType::DesktopClientMfa(Box::new( @@ -131,10 +159,7 @@ impl ClientMfaServer { // if thats not our user, prevent login if claims_user.id != user.id { info!("User {claims_user} tried to use OIDC MFA for another user: {user}"); - self.sessions - .write() - .expect("Failed to write-lock ClientMfaServer::sessions") - .remove(&pubkey); + self.delete_mfa_session(session).await?; self.emit_event(BidiStreamEvent { context, event: BidiStreamEventType::DesktopClientMfa(Box::new( @@ -155,10 +180,7 @@ impl ClientMfaServer { } Err(err) => { info!("Failed to verify OIDC code: {err}"); - self.sessions - .write() - .expect("Failed to write-lock ClientMfaServer::sessions") - .remove(&pubkey); + self.delete_mfa_session(session).await?; self.emit_event(BidiStreamEvent { context, event: BidiStreamEventType::DesktopClientMfa(Box::new( @@ -174,21 +196,33 @@ impl ClientMfaServer { } } - self.sessions - .write() - .expect("Failed to write-lock ClientMfaServer::sessions") - .insert( - pubkey.clone(), - ClientLoginSession { - method, - device: device.clone(), - location: location.clone(), - user: user.clone(), - openid_auth_completed: true, - biometric_challenge: None, - }, - ); + // Mark the OIDC attempt complete, gated on the attempt id the state carried. A stale or + // absent attempt is a no-op (returns false) and must be rejected, not silently accepted. + // This repeats the check made above against the row read at the start of the request, and + // deliberately so: the SQL predicate re-evaluates the id at write time, so an attempt + // re-issued while the OIDC round trip was in flight is caught here rather than marked. + let mut conn = self.acquire_conn().await?; + let marked = session + .mark_oidc_completed(&mut conn, &attempt_id) + .await + .map_err(|err| { + error!("Failed to mark OIDC attempt complete: {err}"); + Status::internal("unexpected error") + })?; + if !marked { + debug!("OIDC MFA callback arrived for a superseded or absent attempt"); + return Err(Status::invalid_argument("stale OIDC MFA attempt")); + } Ok(()) } + + /// Delete a durable MFA session, mapping database errors to a gRPC status. + async fn delete_mfa_session(&self, session: VpnClientMfaSession) -> Result<(), Status> { + let mut conn = self.acquire_conn().await?; + session.delete(&mut *conn).await.map_err(|err| { + error!("Failed to delete MFA session: {err}"); + Status::internal("unexpected error") + }) + } } diff --git a/crates/defguard_core/src/enterprise/handlers/openid_login.rs b/crates/defguard_core/src/enterprise/handlers/openid_login.rs index 4eadbc6326..f3715fe701 100644 --- a/crates/defguard_core/src/enterprise/handlers/openid_login.rs +++ b/crates/defguard_core/src/enterprise/handlers/openid_login.rs @@ -172,6 +172,39 @@ pub(crate) fn extract_state_data(state: &str) -> Option { } } +/// The OIDC MFA `state` payload: the opaque in-progress session token plus the `step_attempt_id` +/// the authorize URL was issued for. Serialized as `.`. +/// +/// The enrollment flow shares `extract_state_data` but carries no nonce and no dot in its +/// payload, so this type is specific to the MFA flow. +#[derive(Clone, Debug, PartialEq)] +pub struct MfaOidcState { + pub token: String, + pub attempt_id: String, +} + +impl MfaOidcState { + /// Build the dotted `.` payload. + #[must_use] + pub fn build(token: &str, attempt_id: &str) -> String { + format!("{token}.{attempt_id}") + } + + /// Parse the dotted payload back into its parts. `None` when there is no separator or when + /// either part is empty. + #[must_use] + pub fn parse(payload: &str) -> Option { + let (token, attempt_id) = payload.split_once('.')?; + if token.is_empty() || attempt_id.is_empty() { + return None; + } + Some(Self { + token: token.to_owned(), + attempt_id: attempt_id.to_owned(), + }) + } +} + /// Build OpenID Connect client. /// `url`: redirect/callback URL pub async fn make_oidc_client( @@ -969,6 +1002,27 @@ mod test { assert_eq!(extracted, Some("data.with.dots".to_owned())); } + #[test] + fn test_state_round_trips_mfa_attempt_id() { + // The MFA flow's state data is ".". The dotted payload must + // survive build_state -> extract_state_data and split back into its two fields. + let data = MfaOidcState::build("opaque-token", "attempt-id-123"); + let state = build_state(Some(data.clone())); + let extracted = extract_state_data(state.secret()); + assert_eq!(extracted.as_deref(), Some(data.as_str())); + let parsed = MfaOidcState::parse(&extracted.unwrap()).unwrap(); + assert_eq!(parsed.token, "opaque-token"); + assert_eq!(parsed.attempt_id, "attempt-id-123"); + + // An enrollment-shaped state carries no attempt id and must round-trip unchanged. + let enrollment = "enrollment-token"; + let state = build_state(Some(enrollment.to_owned())); + assert_eq!( + extract_state_data(state.secret()), + Some(enrollment.to_owned()) + ); + } + #[test] fn test_reached_user_license_limit_reached() { set_counts(Counts::new(2, 0, 0, 0)); @@ -1138,6 +1192,19 @@ mod test { pool: &PgPool, target: DirectorySyncTarget, ) -> (User, Group) { + // Group sync is a business feature and its licence gate is compiled into test builds, + // so without a licence `sync_user_groups_if_configured` returns without syncing. + set_cached_license(Some(License::new( + "test".to_owned(), + false, + None, + None, + None, + LicenseTier::Business, + SupportType::Basic, + vec![], + ))); + let _ = SERVER_CONFIG.set(DefGuardConfig::new_test_config()); Settings::initialize_runtime_defaults(pool).await.unwrap(); initialize_current_settings(pool).await.unwrap(); diff --git a/crates/defguard_core/src/events.rs b/crates/defguard_core/src/events.rs index 625a3f00fd..7697b9febd 100644 --- a/crates/defguard_core/src/events.rs +++ b/crates/defguard_core/src/events.rs @@ -10,6 +10,7 @@ use defguard_common::db::{ mfa_flow::{LocationMfaFlowAssignmentSnapshot, MfaFlowSnapshot}, oauth2client::OAuth2Client, proxy::Proxy, + vpn_client_mfa_session::MfaAttribution, }, }; use defguard_proto::{client_types::MfaMethod, enterprise::posture::DevicePostureData}; @@ -461,7 +462,8 @@ pub enum DesktopClientMfaEvent { Success { device: Device, location: WireguardNetwork, - method: ClientMFAMethod, + /// The complete challenge-and-response record and the governing flow's title. + attribution: MfaAttribution, /// Name of the device used to approve the login when the mobile approve /// MFA method is used. `None` for all other methods. mobile_auth_device_name: Option, @@ -488,11 +490,19 @@ pub enum DesktopClientMfaEvent { device_posture_data: Option, failed_checks: Vec, }, + /// An authorized VPN session was replaced by a new authorization. SessionSuperseded { device: Device, location: WireguardNetwork, is_mfa_session: bool, }, + /// An in-progress MFA login was replaced by a new login attempt for the same device and + /// location. Distinct from [`Self::SessionSuperseded`]: nothing was authorized yet, so no + /// VPN session existed to supersede. + MfaLoginSuperseded { + device: Device, + location: WireguardNetwork, + }, } #[derive(Clone, Debug, PartialEq)] diff --git a/crates/defguard_core/src/grpc/proxy/client_mfa.rs b/crates/defguard_core/src/grpc/proxy/client_mfa.rs index ec66d54cf7..d5251d0722 100644 --- a/crates/defguard_core/src/grpc/proxy/client_mfa.rs +++ b/crates/defguard_core/src/grpc/proxy/client_mfa.rs @@ -7,7 +7,6 @@ use std::{ use chrono::Utc; use defguard_common::{ - auth::claims::{Claims, ClaimsType}, db::{ Id, models::{ @@ -15,8 +14,11 @@ use defguard_common::{ device::{DeviceNetworkInfo, WireguardNetworkDevice}, mfa_flow::MfaFlow, polling_token::PollingToken, + vpn_client_mfa_session::{ + MfaAttribution, MfaSessionContext, StepOutcome, VPN_MFA_SESSION_TIMEOUT, + VpnClientMfaSession, hash_token, + }, vpn_client_session::{VpnClientMfaMethod, VpnClientSession, VpnClientSessionState}, - wireguard::LocationMfaMode, }, }, types::user_info::UserInfo, @@ -33,7 +35,7 @@ use defguard_proto::{ core_response::Payload, }, }; -use sqlx::{PgConnection, PgPool}; +use sqlx::{PgConnection, PgPool, Postgres, pool::PoolConnection}; use thiserror::Error; use tokio::{ sync::{ @@ -56,8 +58,6 @@ use crate::{ mail::templates::mfa_code_mail, }; -const CLIENT_SESSION_TIMEOUT: u64 = 60 * 5; // 5 minutes - // How much time the user has to approve remote MFA with mobile device const REMOTE_AUTH_TIMEOUT: Duration = Duration::from_mins(1); @@ -73,16 +73,6 @@ impl From for Status { } } -#[derive(Clone)] -pub struct ClientLoginSession { - pub(crate) method: MfaMethod, - pub(crate) location: WireguardNetwork, - pub(crate) device: Device, - pub(crate) user: User, - pub(crate) openid_auth_completed: bool, - pub(crate) biometric_challenge: Option, -} - pub enum SessionDisconnectReason { /// Closed because a new authorization is creating a replacement session. Superseded, @@ -93,11 +83,30 @@ pub enum SessionDisconnectReason { pub struct ClientMfaServer { pub(crate) pool: PgPool, gateway_tx: Sender, - pub(crate) sessions: Arc>>, remote_mfa_responses: Arc>>>, bidi_event_tx: UnboundedSender, } +/// Acquire a pooled connection, mapping a pool error to an internal status. +async fn acquire_connection(pool: &PgPool) -> Result, Status> { + pool.acquire().await.map_err(|_| { + error!("Failed to acquire DB connection"); + Status::internal("unexpected error") + }) +} + +/// Remove a remote-MFA waiter from the map, dropping the entry so a never-finishing client or a +/// dropped sender cannot leak a map entry. +fn remove_remote_mfa_waiter( + waiters: &Arc>>>, + hash: &str, +) { + waiters + .write() + .expect("Failed to write-lock ClientMfaServer::remote_mfa_responses") + .remove(hash); +} + impl ClientMfaServer { fn build_authorized_gateway_network_info( network_device: WireguardNetworkDevice, @@ -116,60 +125,40 @@ impl ClientMfaServer { gateway_tx: Sender, bidi_event_tx: UnboundedSender, remote_mfa_responses: Arc>>>, - sessions: Arc>>, ) -> Self { Self { pool, gateway_tx, - sessions, remote_mfa_responses, bidi_event_tx, } } - fn generate_token(pubkey: &str) -> Result { - Claims::new( - ClaimsType::DesktopClient, - String::new(), - pubkey.into(), - CLIENT_SESSION_TIMEOUT, - ) - .to_jwt() - .map_err(|err| { - error!("Failed to generate JWT token: {err}"); - Status::internal("unexpected error") - }) - } - - /// Validate JWT and extract client pubkey - pub(crate) fn parse_token(token: &str) -> Result { - let claims = Claims::from_jwt(ClaimsType::DesktopClient, token).map_err(|err| { - error!("Failed to parse JWT token: {err}"); - Status::invalid_argument("invalid token") - })?; - Ok(claims.client_id) - } - /// Emit given event to the channel. pub(crate) fn emit_event(&self, event: BidiStreamEvent) -> Result<(), ClientMfaServerError> { Ok(self.bidi_event_tx.send(event)?) } + /// Acquire a pooled connection, mapping a pool error to an internal status. + pub(crate) async fn acquire_conn(&self) -> Result, Status> { + acquire_connection(&self.pool).await + } + /// Allows Edge to verify if token is valid and active. #[instrument(skip_all)] pub async fn validate_mfa_token( &mut self, request: ClientMfaTokenValidationRequest, ) -> Result { - let pubkey = Self::parse_token(&request.token)?; - let session_active = self - .sessions - .read() - .expect("Failed to read-lock ClientMfaServer::sessions") - .contains_key(&pubkey); - Ok(ClientMfaTokenValidationResponse { - token_valid: session_active, - }) + let token_valid = + VpnClientMfaSession::::find_active_by_token(&self.pool, &request.token) + .await + .map_err(|err| { + error!("Failed to validate MFA token: {err}"); + Status::internal("unexpected error") + })? + .is_some(); + Ok(ClientMfaTokenValidationResponse { token_valid }) } #[instrument(skip_all)] @@ -216,6 +205,14 @@ impl ClientMfaServer { // validate user is allowed to connect to a given location Self::validate_location_access(&self.pool, &location, &device, &user_info).await?; + // Parse the caller's device info before anything is written. This is pure request + // validation with no database dependency, and rejecting it later would return an error + // to the client while leaving a live session row behind (and, on the supersede path, + // having already torn down the caller's previous session). It sits after the entity + // lookups so their more specific `not found` errors keep precedence, and before the + // posture block, which is the first thing here that can write. + let (ip, _user_agent) = parse_client_ip_agent(&info).map_err(Status::internal)?; + // Evaluate postures if necessary. let has_postures = location.has_postures(&self.pool).await.map_err(|err| { error!( @@ -244,7 +241,6 @@ impl ClientMfaServer { } }; - let (ip, _user_agent) = parse_client_ip_agent(&info).map_err(Status::internal)?; let context = BidiRequestContext::new(user.id, user.username.clone(), ip, device.name.clone()); @@ -300,18 +296,17 @@ impl ClientMfaServer { Status::invalid_argument("invalid MFA method selected") })?; - // Derive the legacy single-factor mode for this location. `None` means the location's - // flow configuration cannot be expressed as a legacy mode (multi-flow, multi-step, or a - // subset of the internal method set), so no current client can enforce it. Fail closed - // rather than fall back to a mode: `mfa_enabled` is a stored column now, so it no longer - // implies that a legacy mode is derivable. - let Some(location_mfa_mode) = MfaFlow::derive_legacy_mode(&self.pool, request.location_id) + // Reject locations whose flow configuration cannot be expressed as a legacy + // single-factor mode (multi-flow, multi-step, or a subset of the internal method set). + // Fail closed rather than silently driving only the first step. + if MfaFlow::derive_legacy_mode(&self.pool, location.id) .await .map_err(|err| { error!("Failed to derive legacy MFA mode: {err}"); Status::internal("unexpected error") })? - else { + .is_none() + { error!( "Location {location} has an MFA flow configuration that cannot be enforced by \ this client" @@ -319,32 +314,49 @@ impl ClientMfaServer { return Err(Status::failed_precondition( "Defguard client version is too old to connect to this location. Please update your client.", )); - }; + } - // check if selected MFA method matches location settings - match (&location_mfa_mode, selected_method) { - ( - LocationMfaMode::Internal, - MfaMethod::Totp - | MfaMethod::Email - | MfaMethod::Biometric - | MfaMethod::MobileApprove, - ) => { - debug!("Location uses internal MFA. Selected method: {selected_method}"); - } - (LocationMfaMode::External, MfaMethod::Oidc) => { - debug!("Location uses external MFA. Selected method: {selected_method}"); - } - _ => { - error!( - "Selected MFA method ({selected_method}) is not supported by location \ - {location}" - ); + // Resolve the MFA flow that applies to this user at this location. The legacy adapter + // drives only the first step, so license-filter its methods and validate the client's + // selected method against them. + let mut conn = self.acquire_conn().await?; + let Some((flow, steps)) = MfaFlow::resolve_for_user(&mut conn, location.id, user.id) + .await + .map_err(|err| { + error!("Failed to resolve MFA flow: {err}"); + Status::internal("unexpected error") + })? + else { + error!( + "Location {location} has no MFA flow that applies to user {}", + user.username + ); + return Err(Status::failed_precondition( + "location MFA configuration is not supported by this client", + )); + }; - return Err(Status::invalid_argument( - "selected MFA method is not supported by location", - )); - } + let Some(first_step) = steps.first() else { + error!("Resolved MFA flow has no steps"); + return Err(Status::internal("unexpected error")); + }; + let first_step_methods: Vec = first_step + .methods + .iter() + .copied() + // OIDC MFA is a business feature, so an unlicensed deployment must not offer it. + .filter(|method| *method != VpnClientMfaMethod::Oidc || is_business_license_active()) + .collect(); + + let selected_client_method: VpnClientMfaMethod = selected_method.into(); + if !first_step_methods.contains(&selected_client_method) { + error!( + "Selected MFA method ({selected_method}) is not supported by location \ + {location}" + ); + return Err(Status::invalid_argument( + "selected MFA method is not supported by location", + )); } let mut selected_mobile_auth: Option> = None; @@ -416,13 +428,9 @@ impl ClientMfaServer { })?; } MfaMethod::Oidc => { - if !is_business_license_active() { - error!("OIDC MFA method requires enterprise feature to be enabled"); - return Err(Status::invalid_argument( - "selected MFA method is not available", - )); - } - + // No license check here: `first_step_methods` above drops OIDC unless + // `oidc_mfa_enabled()`, and a method absent from it is already rejected, so + // reaching this arm means the gate passed. if OpenIdProvider::get_current(&self.pool) .await .map_err(|err| { @@ -439,14 +447,6 @@ impl ClientMfaServer { } } - // generate auth token - let token = Self::generate_token(&request.pubkey)?; - - info!( - "Desktop client MFA login started for {} at location {}", - user.username, location.name - ); - let biometric_challenge: Option = match selected_method { MfaMethod::Biometric => match selected_mobile_auth { Some(mobile_auth) => { @@ -471,24 +471,55 @@ impl ClientMfaServer { .as_ref() .map(|challenge| challenge.challenge.clone()); - // store login session - self.sessions - .write() - .expect("Failed to write-lock ClientMfaServer::sessions") - .insert( - request.pubkey, - ClientLoginSession { - method: selected_method, - location, - device, - user, - openid_auth_completed: false, - biometric_challenge, - }, - ); + // Start the durable in-progress session, freezing the license-filtered first step. The + // first attempt (selected method and challenge) is written by the same statement that + // mints the row, so a concurrent start cannot leave this client's token pointing at an + // attempt another caller selected. + let (_session, outcome) = VpnClientMfaSession::::start( + &mut conn, + location.id, + device.id, + user.id, + flow.id, + vec![first_step_methods], + selected_method.into(), + biometric_challenge, + VPN_MFA_SESSION_TIMEOUT, + ) + .await + .map_err(|err| { + error!("Failed to start MFA session: {err}"); + Status::internal("unexpected error") + })?; + + // Cancel the superseded session's waiter (best-effort hygiene) and emit the supersede + // event. + if let Some(superseded_token_hash) = outcome.superseded_token_hash { + self.remote_mfa_responses + .write() + .expect("Failed to write-lock ClientMfaServer::remote_mfa_responses") + .remove(&superseded_token_hash); + + let context = + BidiRequestContext::new(user.id, user.username.clone(), ip, device.name.clone()); + self.emit_event(BidiStreamEvent { + context, + event: BidiStreamEventType::DesktopClientMfa(Box::new( + DesktopClientMfaEvent::MfaLoginSuperseded { + location: location.clone(), + device: device.clone(), + }, + )), + })?; + } + + info!( + "Desktop client MFA login started for {} at location {}", + user.username, location.name + ); Ok(ClientMfaStartOutcome::Approved(ClientMfaStartResponse { - token, + token: outcome.token, challenge: response_challenge, rejections: Vec::new(), })) @@ -502,10 +533,7 @@ impl ClientMfaServer { user_info: &UserInfo, ) -> Result<(), Status> { // acquire connection - let mut conn = pool.acquire().await.map_err(|_| { - error!("Failed to acquire DB connection"); - Status::internal("unexpected error") - })?; + let mut conn = acquire_connection(pool).await?; // fetch allowed group names for a given location let allowed_groups = location @@ -556,13 +584,30 @@ impl ClientMfaServer { response_tx: UnboundedSender, request_id: u64, ) -> Result<(), Status> { - debug!("Finishing desktop client login: {request:?}"); + debug!("Awaiting remote MFA finish for request_id {request_id}"); + + // Register a waiter only for a token that maps to a live in-progress session, so an + // unauthenticated caller cannot grow the waiter map without bound. + if VpnClientMfaSession::::find_active_by_token(&self.pool, &request.token) + .await + .map_err(|err| { + error!("Failed to find MFA session: {err}"); + Status::internal("unexpected error") + })? + .is_none() + { + error!("Client login session not found"); + return Err(Status::invalid_argument("login session not found")); + } + + let hash = hash_token(&request.token); let (tx, rx) = oneshot::channel(); self.remote_mfa_responses .write() .expect("Failed to write-lock ClientMfaServer::remote_mfa_responses") - .insert(request.token.clone(), tx); + .insert(hash.clone(), tx); + let waiters = self.remote_mfa_responses.clone(); // Spawn a task that waits for remote MFA process to conclude to get the preshared key. tokio::spawn(async move { match time::timeout(REMOTE_AUTH_TIMEOUT, rx).await { @@ -581,9 +626,13 @@ impl ClientMfaServer { let _ = response_tx.send(req); } Ok(Err(err)) => { + // Drop the waiter so a dropped sender cannot leak a map entry. + remove_remote_mfa_waiter(&waiters, &hash); error!("Remote MFA response channel failed: {err:?}"); } Err(_) => { + // Drop the waiter so a client that never finishes cannot leak map entries. + remove_remote_mfa_waiter(&waiters, &hash); warn!("Remote MFA process with request_id {request_id} timed out"); } } @@ -592,37 +641,72 @@ impl ClientMfaServer { Ok(()) } + /// Record a proof-verification failure, deleting the session once the per-step cap is + /// reached so a subsequent finish fails closed. + async fn record_mfa_failure(&self, session: VpnClientMfaSession) -> Result<(), Status> { + let mut conn = self.acquire_conn().await?; + let at_cap = session + .increment_failed_attempts(&mut conn) + .await + .map_err(|err| { + error!("Failed to record MFA failure: {err}"); + Status::internal("unexpected error") + })?; + if at_cap { + session.delete(&mut *conn).await.map_err(|err| { + error!("Failed to delete MFA session: {err}"); + Status::internal("unexpected error") + })?; + } + Ok(()) + } + #[instrument(skip_all)] pub async fn finish_client_mfa_login( &mut self, request: ClientMfaFinishRequest, info: Option, ) -> Result { - debug!("Finishing desktop client login: {request:?}"); - // get pubkey from token - let pubkey = Self::parse_token(&request.token)?; - - // fetch login session - let Some(session) = self - .sessions - .read() - .expect("Failed to read-lock ClientMfaServer::sessions") - .get(&pubkey) - .cloned() + debug!("Finishing desktop client login"); + + // Fetch the durable in-progress session by the opaque token. + let Some(session) = + VpnClientMfaSession::::find_active_by_token(&self.pool, &request.token) + .await + .map_err(|err| { + error!("Failed to find MFA session: {err}"); + Status::internal("unexpected error") + })? else { error!("Client login session not found"); return Err(Status::invalid_argument("login session not found")); }; - let ClientLoginSession { - method, - device, + + // Fetch the related objects for event context and authorization. + let Some(MfaSessionContext { location, + device, user, - openid_auth_completed, - biometric_challenge, - } = session; + }) = session.load_context(&self.pool).await.map_err(|err| { + error!("Failed to load MFA session context: {err}"); + Status::internal("unexpected error") + })? + else { + error!("MFA session references a missing location, device, or user"); + return Err(Status::internal("unexpected error")); + }; + + // The legacy adapter drives a single step, so the attempt recorded at start holds the + // selected method and challenge. + let Some(ephemeral) = session.ephemeral_state.as_ref() else { + error!("No MFA attempt in progress"); + return Err(Status::invalid_argument("no MFA attempt in progress")); + }; + let method: MfaMethod = ephemeral.selected_method.into(); + let openid_auth_completed = ephemeral.openid_auth_completed; + let biometric_challenge = ephemeral.biometric_challenge.clone(); - // Prepare event context + // Prepare event context. let (ip, _user_agent) = parse_client_ip_agent(&info).map_err(Status::internal)?; let context = BidiRequestContext::new(user.id, user.username.clone(), ip, format!("{device}")); @@ -642,11 +726,18 @@ impl ClientMfaServer { Status::invalid_argument("Signature not found in request") })?; let auth_device_pub_key = request.auth_pub_key.ok_or_else(|| { + error!("Authorization device key missing in request"); Status::invalid_argument("Authorization device key missing in request") })?; if !BiometricAuth::verify_owner(&self.pool, user.id, &auth_device_pub_key) .await - .map_err(|_| Status::internal("unexpected error"))? + .map_err(|err| { + error!( + "Failed to verify mobile approve owner for user {}: {err}", + user.id + ); + Status::internal("unexpected error") + })? { return Err(Status::invalid_argument("Arguments invalid")); } @@ -654,7 +745,13 @@ impl ClientMfaServer { mobile_auth_device_name = BiometricAuth::find_device(&self.pool, user.id, &auth_device_pub_key) .await - .map_err(|_| Status::internal("unexpected error"))? + .map_err(|err| { + error!( + "Failed to find mobile approve device for user {}: {err}", + user.id + ); + Status::internal("unexpected error") + })? .map(|auth_device| auth_device.name); match challenge.verify(signature.as_str(), Some(auth_device_pub_key)) { Ok(()) => { @@ -676,6 +773,7 @@ impl ClientMfaServer { }, )), })?; + self.record_mfa_failure(session).await?; return Err(Status::unauthenticated("unauthorized")); } } @@ -711,6 +809,7 @@ impl ClientMfaServer { }, )), })?; + self.record_mfa_failure(session).await?; return Err(Status::unauthenticated("unauthorized")); } } @@ -746,6 +845,7 @@ impl ClientMfaServer { }, )), })?; + self.record_mfa_failure(session).await?; return Err(Status::unauthenticated("unauthorized")); } } @@ -780,6 +880,7 @@ impl ClientMfaServer { }, )), })?; + self.record_mfa_failure(session).await?; return Err(Status::unauthenticated("unauthorized")); } } @@ -827,6 +928,28 @@ impl ClientMfaServer { return Err(Status::internal("unexpected error")); }; + // Advance the single step BEFORE authorizing, so the satisfied method is recorded into + // the snapshot and the step outcome is verified (code-review finding: `Complete` must be + // confirmed before minting a session). + let (advance, snapshot) = session.advance(&mut transaction).await.map_err(|err| { + error!("Failed to advance MFA session: {err}"); + Status::internal("unexpected error") + })?; + if advance != StepOutcome::Complete { + error!("MFA session did not complete after its single step: {advance:?}"); + return Err(Status::internal("unexpected error")); + } + + // Resolve the flow name for attribution. A flow deleted mid-session simply leaves the + // name unresolved; that is a display concern, not an error. + let flow_name = MfaFlow::find_by_id(&mut *transaction, snapshot.flow_id) + .await + .map_err(|err| { + error!("Failed to resolve MFA flow for attribution: {err}"); + Status::internal("unexpected error") + })? + .map(|flow| flow.title); + // generate PSK let key = WireguardNetwork::genkey(); @@ -837,7 +960,7 @@ impl ClientMfaServer { &location, &user, &device, - Some(method.into()), + true, key.public.clone(), ) .await @@ -871,7 +994,10 @@ impl ClientMfaServer { DesktopClientMfaEvent::Success { location, device, - method, + attribution: MfaAttribution { + snapshot, + flow_name, + }, mobile_auth_device_name, }, )), @@ -887,11 +1013,11 @@ impl ClientMfaServer { result: None, }; - // remove login session from map - self.sessions - .write() - .expect("Failed to write-lock ClientMfaServer::sessions") - .remove(&pubkey); + // Delete the in-progress session atomically with the authorization. + session.delete(&mut *transaction).await.map_err(|err| { + error!("Failed to delete MFA session: {err}"); + Status::internal("unexpected error") + })?; // commit transaction transaction.commit().await.map_err(|_| { @@ -900,11 +1026,12 @@ impl ClientMfaServer { })?; // If there is a desktop client websocket waiting for the preshared key, send it. + // The waiter is keyed by the token hash, matching the durable session's lookup key. if let Some(tx) = self .remote_mfa_responses .write() .expect("Failed to write-lock ClientMfaServer::remote_mfa_responses") - .remove(&request.token) + .remove(&hash_token(&request.token)) { let _ = tx.send(key.public.clone()); } @@ -1115,7 +1242,7 @@ impl ClientMfaServer { &location, &user, &device, - None, // posture-only session has no MFA method + false, key.public.clone(), ) .await?; @@ -1206,7 +1333,7 @@ impl ClientMfaServer { let mut events = Vec::new(); for mut session in active_sessions { let is_connected = session.state == VpnClientSessionState::Connected; - let is_mfa_session = session.mfa_method.is_some(); + let is_mfa_session = session.is_mfa_session; let disconnect_timestamp = Utc::now().naive_utc(); session.disconnected_at = Some(disconnect_timestamp); session.state = VpnClientSessionState::Disconnected; @@ -1246,7 +1373,7 @@ impl ClientMfaServer { location: &WireguardNetwork, user: &User, device: &Device, - mfa_method: Option, + is_mfa_session: bool, preshared_key: String, ) -> Result, Status> { debug!( @@ -1288,7 +1415,8 @@ impl ClientMfaServer { } // create new MFA session - let mut session = VpnClientSession::new(location.id, user.id, device.id, None, mfa_method); + let mut session = + VpnClientSession::new(location.id, user.id, device.id, None, is_mfa_session); session.preshared_key = Some(preshared_key); session.save(conn).await.map_err(|err| { error!("Failed to create new VPN client session for device {device} in location {location}: {err}"); @@ -1307,7 +1435,7 @@ impl ClientMfaServer { reason: SessionDisconnectReason, ) -> Result<(), Status> { let is_connected = session.state == VpnClientSessionState::Connected; - let is_mfa_session = session.mfa_method.is_some(); + let is_mfa_session = session.is_mfa_session; let requires_gateway_update = is_mfa_session || location.has_postures(&mut *conn).await.map_err(|err| { error!("Failed to fetch postures for location {location}: {err}"); @@ -1387,7 +1515,11 @@ mod tests { use std::{ collections::HashMap, net::{IpAddr, Ipv4Addr}, - sync::{Arc, RwLock}, + sync::{ + Arc, RwLock, + atomic::{AtomicUsize, Ordering}, + }, + time::{Duration, SystemTime}, }; use chrono::Utc; @@ -1399,17 +1531,22 @@ mod tests { mfa_flow::{LocationMfaFlowAssignment, MfaFlow}, polling_token::PollingToken, settings::initialize_current_settings, + user::{TOTP_CODE_DIGITS, TOTP_CODE_VALIDITY_PERIOD}, + vpn_client_mfa_session::{ + MFA_FAILED_ATTEMPT_CAP, MfaAttribution, VPN_MFA_SESSION_TIMEOUT, + VpnClientMfaSession, + }, vpn_client_session::{VpnClientMfaMethod, VpnClientSession, VpnClientSessionState}, wireguard::ServiceLocationMode, }, setup_pool, }; use defguard_proto::{ - client_types::{ClientMfaStartRequest, MfaMethod}, + client_types::{ClientMfaFinishRequest, ClientMfaStartRequest, MfaMethod}, enterprise::posture::{ BoolCheck, DevicePostureCheckRequest, DevicePostureData, bool_check, }, - proxy::DeviceInfo, + proxy::{ClientMfaOidcAuthenticateRequest, ClientMfaTokenValidationRequest, DeviceInfo}, }; use ipnetwork::IpNetwork; use sqlx::{ @@ -1418,13 +1555,15 @@ mod tests { }; use tokio::sync::{broadcast, mpsc, oneshot}; use tonic::Code; + use totp_lite::{Sha1, totp_custom}; - use super::{ClientLoginSession, ClientMfaServer}; + use super::{ClientMfaServer, ClientMfaStartOutcome}; use crate::{ enterprise::{ db::models::device_posture::{ DevicePosture, DevicePostureLocation, DevicePostureOsRule, OsType, }, + handlers::openid_login::build_state, license::{License, LicenseTier, SupportType, set_cached_license}, limits::{Counts, set_counts}, }, @@ -1533,7 +1672,7 @@ mod tests { user.id, device.id, Some(Utc::now().naive_utc()), - None, + false, ); old_session.preshared_key = Some("old-posture-psk".to_owned()); old_session.state = VpnClientSessionState::Connected; @@ -1741,7 +1880,7 @@ mod tests { user.id, victim.id, Some(Utc::now().naive_utc()), - None, + false, ); victim_session.preshared_key = Some("victim-psk".to_owned()); victim_session.state = VpnClientSessionState::Connected; @@ -2051,7 +2190,7 @@ mod tests { user.id, device.id, Some(Utc::now().naive_utc()), - None, + false, ); active_session.preshared_key = Some("active-posture-psk".to_owned()); active_session.state = VpnClientSessionState::Connected; @@ -2182,7 +2321,7 @@ mod tests { user.id, device.id, Some(Utc::now().naive_utc()), - Some(VpnClientMfaMethod::Totp), + true, ); active_session.preshared_key = Some("active-mfa-psk".to_owned()); let active_session = active_session @@ -2332,7 +2471,7 @@ mod tests { user.id, device.id, Some(Utc::now().naive_utc()), - None, + false, ) .save(&pool) .await @@ -2387,7 +2526,7 @@ mod tests { user.id, device.id, Some(Utc::now().naive_utc()), - Some(VpnClientMfaMethod::Totp), + true, ) .save(&pool) .await @@ -2402,7 +2541,7 @@ mod tests { &location, &user, &device, - Some(VpnClientMfaMethod::Totp), + true, REPLACEMENT_MFA_PRESHARED_KEY.to_owned(), ) .await @@ -2457,16 +2596,10 @@ mod tests { let user = create_user(&pool).await; let device = create_device(&pool, user.id).await; attach_device_to_location(&pool, location.id, device.id).await; - let old_session = VpnClientSession::new( - location.id, - user.id, - device.id, - None, - Some(VpnClientMfaMethod::Totp), - ) - .save(&pool) - .await - .expect("failed to create existing new MFA session"); + let old_session = VpnClientSession::new(location.id, user.id, device.id, None, true) + .save(&pool) + .await + .expect("failed to create existing new MFA session"); let (server, mut event_rx, mut gateway_rx) = make_server(pool.clone()); let mut conn = pool.acquire().await.expect("failed to acquire connection"); @@ -2477,7 +2610,7 @@ mod tests { &location, &user, &device, - Some(VpnClientMfaMethod::Totp), + true, REPLACEMENT_MFA_PRESHARED_KEY.to_owned(), ) .await @@ -2517,28 +2650,28 @@ mod tests { let (bidi_event_tx, bidi_event_rx) = mpsc::unbounded_channel(); let remote_mfa_responses: Arc>>> = Arc::default(); - let sessions: Arc>> = Arc::default(); ( - ClientMfaServer::new( - pool, - gateway_tx, - bidi_event_tx, - remote_mfa_responses, - sessions, - ), + ClientMfaServer::new(pool, gateway_tx, bidi_event_tx, remote_mfa_responses), bidi_event_rx, gateway_rx, ) } + static COUNTER: AtomicUsize = AtomicUsize::new(0); + + fn next_suffix() -> usize { + COUNTER.fetch_add(1, Ordering::Relaxed) + } + async fn create_user(pool: &PgPool) -> User { + let suffix = next_suffix(); User::new( - "client-mfa-test", + format!("client-mfa-test-{suffix}"), Some("pass123"), - "Tester", - "ClientMfa", - "client-mfa@example.com", + "Tester".to_owned(), + "ClientMfa".to_owned(), + format!("client-mfa-{suffix}@example.com"), None, ) .save(pool) @@ -2547,9 +2680,10 @@ mod tests { } async fn create_device(pool: &PgPool, user_id: Id) -> Device { + let suffix = next_suffix(); Device::new( - "client-mfa-device".to_owned(), - "client-mfa-pubkey".to_owned(), + format!("client-mfa-device-{suffix}"), + format!("client-mfa-pubkey-{suffix}"), user_id, DeviceType::User, None, @@ -2586,7 +2720,7 @@ mod tests { user.id, device.id, Some(Utc::now().naive_utc()), - Some(VpnClientMfaMethod::Totp), + true, ); previous_session.preshared_key = Some("old-psk".to_owned()); previous_session.state = VpnClientSessionState::Connected; @@ -2604,7 +2738,6 @@ mod tests { Arc::new(RwLock::new( HashMap::>::new(), )), - Arc::new(RwLock::new(HashMap::::new())), ); let mut conn = pool .acquire() @@ -2617,7 +2750,7 @@ mod tests { &location, &user, &device, - Some(VpnClientMfaMethod::Totp), + true, NEW_MFA_PRESHARED_KEY.to_owned(), ) .await @@ -2707,6 +2840,578 @@ mod tests { .expect("failed to attach device to location"); } + async fn create_and_assign_mfa_flow(pool: &PgPool, location_id: Id) { + let mut tx = pool.begin().await.expect("failed to begin transaction"); + let (flow, _steps) = MfaFlow::create( + &mut tx, + "Default Internal MFA".into(), + vec![vec![ + VpnClientMfaMethod::Totp, + VpnClientMfaMethod::Email, + VpnClientMfaMethod::Biometric, + VpnClientMfaMethod::MobileApprove, + ]], + ) + .await + .expect("failed to create MFA flow"); + MfaFlow::assign_to_location( + &mut tx, + location_id, + &[LocationMfaFlowAssignment { + flow_id: flow.id, + is_default: true, + group_ids: Vec::new(), + }], + ) + .await + .expect("failed to assign MFA flow to location"); + tx.commit().await.expect("failed to commit transaction"); + } + + #[sqlx::test] + #[allow(deprecated)] + async fn test_finish_client_mfa_login_totp_authorizes_session( + _: PgPoolOptions, + options: PgConnectOptions, + ) { + set_enterprise_license(); + let pool = setup_pool(options).await; + initialize_current_settings(&pool) + .await + .expect("failed to init settings"); + let location = create_mfa_location(&pool).await; + create_and_assign_mfa_flow(&pool, location.id).await; + let mut user = create_user(&pool).await; + let secret = vec![1u8, 2, 3, 4, 5, 6, 7, 8, 9, 10]; + user.totp_secret = Some(secret.clone()); + user.totp_enabled = true; + user.save(&pool).await.expect("failed to configure TOTP"); + let device = create_device(&pool, user.id).await; + attach_device_to_location(&pool, location.id, device.id).await; + + let (mut server, mut event_rx, _gateway_rx) = make_server(pool.clone()); + + let start = server + .start_client_mfa_login( + ClientMfaStartRequest { + location_id: location.id, + pubkey: device.wireguard_pubkey.clone(), + #[allow(deprecated)] + method: MfaMethod::Totp as i32, + posture_data: None, + selected_methods: Vec::new(), + }, + device_info(), + ) + .await + .expect("start should succeed"); + let token = match start { + ClientMfaStartOutcome::Approved(response) => response.token, + ClientMfaStartOutcome::Rejected { .. } => panic!("unexpected rejection"), + }; + + let timestamp = SystemTime::now() + .duration_since(SystemTime::UNIX_EPOCH) + .unwrap() + .as_secs(); + let code = totp_custom::( + TOTP_CODE_VALIDITY_PERIOD, + TOTP_CODE_DIGITS, + &secret, + timestamp, + ); + + let response = server + .finish_client_mfa_login( + ClientMfaFinishRequest { + token: token.clone(), + code: Some(code), + auth_pub_key: None, + }, + device_info(), + ) + .await + .expect("finish should succeed"); + assert!(!response.preshared_key.is_empty()); + + // The successful finish is audited. + let event = event_rx + .try_recv() + .expect("expected desktop client MFA success event"); + match event.event { + BidiStreamEventType::DesktopClientMfa(event) => match *event { + DesktopClientMfaEvent::Success { + attribution: + MfaAttribution { + snapshot, + flow_name, + }, + .. + } => { + assert_eq!(flow_name.as_deref(), Some("Default Internal MFA")); + assert_eq!(snapshot.steps.len(), 1); + assert_eq!(snapshot.steps[0].satisfied, Some(VpnClientMfaMethod::Totp)); + assert!( + snapshot.steps[0] + .methods + .contains(&VpnClientMfaMethod::Totp) + ); + } + other => panic!("unexpected bidi event: {other:?}"), + }, + other => panic!("unexpected bidi stream event type: {other:?}"), + } + + // The authorized session records only that MFA was used. + let sessions = VpnClientSession::get_all_active_device_sessions_in_location( + &pool, + location.id, + device.id, + ) + .await + .expect("failed to fetch active sessions"); + assert_eq!(sessions.len(), 1); + assert!(sessions[0].is_mfa_session); + + // The in-progress session is gone. + assert!( + VpnClientMfaSession::::find_active_by_token(&pool, &token) + .await + .unwrap() + .is_none() + ); + } + + #[sqlx::test] + async fn test_finish_client_mfa_login_failure_cap_deletes_session( + _: PgPoolOptions, + options: PgConnectOptions, + ) { + set_enterprise_license(); + let pool = setup_pool(options).await; + initialize_current_settings(&pool) + .await + .expect("failed to init settings"); + let location = create_mfa_location(&pool).await; + create_and_assign_mfa_flow(&pool, location.id).await; + let mut user = create_user(&pool).await; + let secret = vec![1u8, 2, 3, 4, 5, 6, 7, 8, 9, 10]; + user.totp_secret = Some(secret); + user.totp_enabled = true; + user.save(&pool).await.expect("failed to configure TOTP"); + let device = create_device(&pool, user.id).await; + attach_device_to_location(&pool, location.id, device.id).await; + + let (mut server, mut event_rx, _gateway_rx) = make_server(pool.clone()); + + let start = server + .start_client_mfa_login( + ClientMfaStartRequest { + location_id: location.id, + pubkey: device.wireguard_pubkey.clone(), + #[allow(deprecated)] + method: MfaMethod::Totp as i32, + posture_data: None, + selected_methods: Vec::new(), + }, + device_info(), + ) + .await + .expect("start should succeed"); + let token = match start { + ClientMfaStartOutcome::Approved(response) => response.token, + ClientMfaStartOutcome::Rejected { .. } => panic!("unexpected rejection"), + }; + + // Repeating a wrong code trips the per-step cap and deletes the session. + for _ in 0..MFA_FAILED_ATTEMPT_CAP { + let result = server + .finish_client_mfa_login( + ClientMfaFinishRequest { + token: token.clone(), + code: Some("000000".to_owned()), + auth_pub_key: None, + }, + device_info(), + ) + .await; + assert!(result.is_err()); + } + + // The session is deleted once the cap is reached. + assert!( + VpnClientMfaSession::::find_active_by_token(&pool, &token) + .await + .unwrap() + .is_none() + ); + + // Each rejected proof is audited. + for _ in 0..MFA_FAILED_ATTEMPT_CAP { + let event = event_rx + .try_recv() + .expect("expected desktop client MFA failed event"); + match event.event { + BidiStreamEventType::DesktopClientMfa(event) => match *event { + DesktopClientMfaEvent::Failed { .. } => {} + other => panic!("unexpected bidi event: {other:?}"), + }, + other => panic!("unexpected bidi stream event type: {other:?}"), + } + } + } + + async fn start_mfa_session_direct(pool: &PgPool, ttl: Duration) -> String { + let location = create_mfa_location(pool).await; + let user = create_user(pool).await; + let device = create_device(pool, user.id).await; + let mut tx = pool.begin().await.unwrap(); + let (_, outcome) = VpnClientMfaSession::::start( + &mut tx, + location.id, + device.id, + user.id, + 1, + vec![vec![VpnClientMfaMethod::Totp]], + VpnClientMfaMethod::Totp, + None, + ttl, + ) + .await + .unwrap(); + tx.commit().await.unwrap(); + outcome.token + } + + #[sqlx::test] + async fn test_validate_mfa_token(_: PgPoolOptions, options: PgConnectOptions) { + let pool = setup_pool(options).await; + let (mut server, _event_rx, _gateway_rx) = make_server(pool.clone()); + + // Unknown token. + let resp = server + .validate_mfa_token(ClientMfaTokenValidationRequest { + token: "nonexistent".to_owned(), + }) + .await + .unwrap(); + assert!(!resp.token_valid); + + // Expired token. + let expired = start_mfa_session_direct(&pool, Duration::ZERO).await; + let resp = server + .validate_mfa_token(ClientMfaTokenValidationRequest { token: expired }) + .await + .unwrap(); + assert!(!resp.token_valid); + + // Active token. + let active = start_mfa_session_direct(&pool, VPN_MFA_SESSION_TIMEOUT).await; + let resp = server + .validate_mfa_token(ClientMfaTokenValidationRequest { token: active }) + .await + .unwrap(); + assert!(resp.token_valid); + } + + #[sqlx::test] + async fn test_start_client_mfa_login_supersedes_existing_session( + _: PgPoolOptions, + options: PgConnectOptions, + ) { + set_enterprise_license(); + let pool = setup_pool(options).await; + initialize_current_settings(&pool) + .await + .expect("failed to init settings"); + let location = create_mfa_location(&pool).await; + create_and_assign_mfa_flow(&pool, location.id).await; + let mut user = create_user(&pool).await; + user.enable_totp(&pool) + .await + .expect("failed to enable TOTP"); + let device = create_device(&pool, user.id).await; + attach_device_to_location(&pool, location.id, device.id).await; + + let (mut server, mut event_rx, _gateway_rx) = make_server(pool.clone()); + + let request = || ClientMfaStartRequest { + location_id: location.id, + pubkey: device.wireguard_pubkey.clone(), + #[allow(deprecated)] + method: MfaMethod::Totp as i32, + posture_data: None, + selected_methods: Vec::new(), + }; + + let first = server + .start_client_mfa_login(request(), device_info()) + .await + .expect("first start should succeed"); + let first_token = match first { + ClientMfaStartOutcome::Approved(response) => response.token, + ClientMfaStartOutcome::Rejected { .. } => panic!("unexpected rejection"), + }; + assert!( + VpnClientMfaSession::::find_active_by_token(&pool, &first_token) + .await + .unwrap() + .is_some() + ); + + let second = server + .start_client_mfa_login(request(), device_info()) + .await + .expect("second start should succeed"); + let second_token = match second { + ClientMfaStartOutcome::Approved(response) => response.token, + ClientMfaStartOutcome::Rejected { .. } => panic!("unexpected rejection"), + }; + + // The first token no longer validates; the second one does. + assert!( + VpnClientMfaSession::::find_active_by_token(&pool, &first_token) + .await + .unwrap() + .is_none() + ); + assert!( + VpnClientMfaSession::::find_active_by_token(&pool, &second_token) + .await + .unwrap() + .is_some() + ); + + let event = event_rx + .try_recv() + .expect("expected an audit event for the superseded login"); + match event.event { + BidiStreamEventType::DesktopClientMfa(event) => match *event { + DesktopClientMfaEvent::MfaLoginSuperseded { + location: event_location, + device: event_device, + } => { + assert_eq!(event_location.id, location.id); + assert_eq!(event_device.id, device.id); + } + other => panic!("unexpected bidi event: {other:?}"), + }, + other => panic!("unexpected bidi stream event type: {other:?}"), + } + } + + /// Malformed device info is rejected before anything is written. Were it parsed after the + /// session was persisted, the failing request would leave a live orphan row behind and, + /// worse, would already have superseded the caller's previous session. + #[sqlx::test] + async fn test_start_client_mfa_login_rejects_bad_device_info_without_persisting( + _: PgPoolOptions, + options: PgConnectOptions, + ) { + set_enterprise_license(); + let pool = setup_pool(options).await; + initialize_current_settings(&pool) + .await + .expect("failed to init settings"); + let location = create_mfa_location(&pool).await; + create_and_assign_mfa_flow(&pool, location.id).await; + let mut user = create_user(&pool).await; + user.enable_totp(&pool) + .await + .expect("failed to enable TOTP"); + let device = create_device(&pool, user.id).await; + attach_device_to_location(&pool, location.id, device.id).await; + + let (mut server, _event_rx, _gateway_rx) = make_server(pool.clone()); + + let request = || ClientMfaStartRequest { + location_id: location.id, + pubkey: device.wireguard_pubkey.clone(), + #[allow(deprecated)] + method: MfaMethod::Totp as i32, + posture_data: None, + selected_methods: Vec::new(), + }; + + let established = server + .start_client_mfa_login(request(), device_info()) + .await + .expect("first start should succeed"); + let established_token = match established { + ClientMfaStartOutcome::Approved(response) => response.token, + ClientMfaStartOutcome::Rejected { .. } => panic!("unexpected rejection"), + }; + + // A start carrying no device info must fail. + assert!( + server + .start_client_mfa_login(request(), None) + .await + .is_err(), + "start without device info should be rejected" + ); + + // The established session is untouched, and no orphan row was left behind. + assert!( + VpnClientMfaSession::::find_active_by_token(&pool, &established_token) + .await + .unwrap() + .is_some() + ); + let rows = sqlx::query_scalar!( + "SELECT count(*) FROM vpn_client_mfa_session WHERE location_id = $1 AND device_id = $2", + location.id, + device.id, + ) + .fetch_one(&pool) + .await + .unwrap(); + assert_eq!(rows, Some(1)); + } + + #[sqlx::test] + #[allow(deprecated)] + async fn test_finish_survives_server_restart(_: PgPoolOptions, options: PgConnectOptions) { + set_enterprise_license(); + let pool = setup_pool(options).await; + initialize_current_settings(&pool) + .await + .expect("failed to init settings"); + let location = create_mfa_location(&pool).await; + create_and_assign_mfa_flow(&pool, location.id).await; + let mut user = create_user(&pool).await; + let secret = vec![1u8, 2, 3, 4, 5, 6, 7, 8, 9, 10]; + user.totp_secret = Some(secret.clone()); + user.totp_enabled = true; + user.save(&pool).await.expect("failed to configure TOTP"); + let device = create_device(&pool, user.id).await; + attach_device_to_location(&pool, location.id, device.id).await; + + // Start the login on one server instance. + let (mut server_a, _event_rx, _gateway_rx) = make_server(pool.clone()); + let start = server_a + .start_client_mfa_login( + ClientMfaStartRequest { + location_id: location.id, + pubkey: device.wireguard_pubkey.clone(), + #[allow(deprecated)] + method: MfaMethod::Totp as i32, + posture_data: None, + selected_methods: Vec::new(), + }, + device_info(), + ) + .await + .expect("start should succeed"); + let token = match start { + ClientMfaStartOutcome::Approved(response) => response.token, + ClientMfaStartOutcome::Rejected { .. } => panic!("unexpected rejection"), + }; + + // A "restart" is a fresh server instance with a fresh in-memory waiter map over the + // same database. The durable session must survive it. + let (mut server_b, _event_rx, _gateway_rx) = make_server(pool.clone()); + + let timestamp = SystemTime::now() + .duration_since(SystemTime::UNIX_EPOCH) + .unwrap() + .as_secs(); + let code = totp_custom::( + TOTP_CODE_VALIDITY_PERIOD, + TOTP_CODE_DIGITS, + &secret, + timestamp, + ); + + let response = server_b + .finish_client_mfa_login( + ClientMfaFinishRequest { + token: token.clone(), + code: Some(code), + auth_pub_key: None, + }, + device_info(), + ) + .await + .expect("finish should succeed after restart"); + assert!(!response.preshared_key.is_empty()); + } + + #[sqlx::test] + async fn test_auth_mfa_session_with_oidc_rejects_non_oidc_method( + _: PgPoolOptions, + options: PgConnectOptions, + ) { + set_enterprise_license(); + let pool = setup_pool(options).await; + initialize_current_settings(&pool) + .await + .expect("failed to init settings"); + let location = create_mfa_location(&pool).await; + create_and_assign_mfa_flow(&pool, location.id).await; + let mut user = create_user(&pool).await; + user.enable_totp(&pool) + .await + .expect("failed to enable TOTP"); + let device = create_device(&pool, user.id).await; + attach_device_to_location(&pool, location.id, device.id).await; + + let (mut server, _event_rx, _gateway_rx) = make_server(pool.clone()); + let start = server + .start_client_mfa_login( + ClientMfaStartRequest { + location_id: location.id, + pubkey: device.wireguard_pubkey.clone(), + #[allow(deprecated)] + method: MfaMethod::Totp as i32, + posture_data: None, + selected_methods: Vec::new(), + }, + device_info(), + ) + .await + .expect("start should succeed"); + let token = match start { + ClientMfaStartOutcome::Approved(response) => response.token, + ClientMfaStartOutcome::Rejected { .. } => panic!("unexpected rejection"), + }; + + // Build a state that encodes the token and the session's step_attempt_id, as the + // OIDC redirect does for the MFA flow. + let session = VpnClientMfaSession::::find_active_by_token(&pool, &token) + .await + .unwrap() + .expect("expected an active session"); + let attempt_id = session + .ephemeral_state + .as_ref() + .expect("expected an attempt in progress") + .step_attempt_id + .clone(); + let state = build_state(Some(format!("{token}.{attempt_id}"))); + let status = server + .auth_mfa_session_with_oidc( + ClientMfaOidcAuthenticateRequest { + code: "dummy".to_owned(), + state: state.secret().to_owned(), + nonce: "dummy".to_owned(), + }, + device_info(), + ) + .await + .expect_err("a non-OIDC session must be rejected"); + assert_eq!(status.code(), Code::InvalidArgument); + assert_eq!(status.message(), "invalid MFA method"); + + // The mismatched session is deleted. + assert!( + VpnClientMfaSession::::find_active_by_token(&pool, &token) + .await + .unwrap() + .is_none() + ); + } + fn set_enterprise_license() { let license = License::new( "test".to_owned(), @@ -2721,7 +3426,7 @@ mod tests { None, LicenseTier::Enterprise, SupportType::Basic, - vec![], + Vec::new(), ); set_cached_license(Some(license)); set_counts(Counts::new(1, 1, 1, 1)); diff --git a/crates/defguard_core/src/location_management/allowed_peers.rs b/crates/defguard_core/src/location_management/allowed_peers.rs index 1887bc414b..f649d1b5a6 100644 --- a/crates/defguard_core/src/location_management/allowed_peers.rs +++ b/crates/defguard_core/src/location_management/allowed_peers.rs @@ -296,7 +296,7 @@ mod test { ); network_device.insert(&mut *conn).await.unwrap(); - VpnClientSession::new(network.id, user.id, device.id, None, None) + VpnClientSession::new(network.id, user.id, device.id, None, false) .save(&mut *conn) .await .unwrap(); @@ -432,7 +432,8 @@ mod test { .await .unwrap(); - let mut new_session = VpnClientSession::new(network.id, user.id, new_device.id, None, None); + let mut new_session = + VpnClientSession::new(network.id, user.id, new_device.id, None, false); new_session.preshared_key = Some("new-session-psk".into()); new_session.save(&mut *conn).await.unwrap(); @@ -441,7 +442,7 @@ mod test { user.id, connected_device.id, Some(Utc::now().naive_utc()), - None, + false, ); connected_session.preshared_key = Some("connected-session-psk".into()); connected_session.save(&mut *conn).await.unwrap(); diff --git a/crates/defguard_core/src/utility_thread.rs b/crates/defguard_core/src/utility_thread.rs index 22322adf02..46daeb5a3a 100644 --- a/crates/defguard_core/src/utility_thread.rs +++ b/crates/defguard_core/src/utility_thread.rs @@ -4,7 +4,7 @@ use chrono::{NaiveDateTime, TimeDelta, Utc}; use defguard_common::{ db::models::{ Certificates, CoreCertSource, ProxyCertSource, User, WireguardNetwork, - wireguard::ServiceLocationMode, + vpn_client_mfa_session::reap_expired, wireguard::ServiceLocationMode, }, types::proxy::ProxyControlMessage, }; @@ -38,6 +38,7 @@ const UTILITY_THREAD_MAIN_SLEEP_TIME: Duration = Duration::from_secs(5); const COUNT_UPDATE_INTERVAL: u64 = 60 * 60; const UPDATES_CHECK_INTERVAL: u64 = 60 * 60 * 6; const EXPIRED_ACL_RULES_CHECK_INTERVAL: u64 = 60 * 5; +const MFA_SESSION_REAP_INTERVAL: u64 = 60 * 5; const ENTERPRISE_STATUS_CHECK_INTERVAL: u64 = 60 * 5; const LETSENCRYPT_EXPIRY_CHECK_INTERVAL: u64 = 60 * 60 * 24; const CERTIFICATE_EXPIRY_CHECK_INTERVAL: u64 = 60 * 60 * 24; // 1 day @@ -61,6 +62,7 @@ pub async fn run_utility_thread( let mut last_enterprise_status_check = Instant::now(); let mut last_letsencrypt_expiry_check = Instant::now(); let mut last_certificate_check = Instant::now(); + let mut last_mfa_session_reap = Instant::now(); // helper variable which stores previous enterprise features status let mut enterprise_enabled = is_business_license_active(); @@ -112,6 +114,15 @@ pub async fn run_utility_thread( } }; + let mfa_session_reap_task = || async { + if let Err(err) = reap_expired(pool) + .instrument(info_span!("mfa_session_reap_task")) + .await + { + error!("Failed to reap expired MFA sessions: {err}"); + } + }; + let letsencrypt_refresh_task = || async { if let Err(e) = do_letsencrypt_refresh(pool, proxy_control_tx.clone()) .instrument(info_span!("letsencrypt_refresh_task")) @@ -126,6 +137,7 @@ pub async fn run_utility_thread( updates_check_task().await; ldap_sync_task().await; expired_acl_rules_task().await; + mfa_session_reap_task().await; letsencrypt_refresh_task().await; check_certificates(pool, &proxy_control_tx, &web_reload_tx).await; @@ -162,6 +174,12 @@ pub async fn run_utility_thread( last_expired_acl_rules_check = Instant::now(); } + // Reap expired in-progress MFA sessions + if last_mfa_session_reap.elapsed().as_secs() >= MFA_SESSION_REAP_INTERVAL { + mfa_session_reap_task().await; + last_mfa_session_reap = Instant::now(); + } + // Check LE cert expiry dates and refresh if necessary if last_letsencrypt_expiry_check.elapsed().as_secs() >= LETSENCRYPT_EXPIRY_CHECK_INTERVAL { letsencrypt_refresh_task().await; diff --git a/crates/defguard_core/tests/integration/api/location_stats.rs b/crates/defguard_core/tests/integration/api/location_stats.rs index 11745b0c4a..fbbf3e3d4d 100644 --- a/crates/defguard_core/tests/integration/api/location_stats.rs +++ b/crates/defguard_core/tests/integration/api/location_stats.rs @@ -111,7 +111,7 @@ async fn test_location_connected_devices_stats(_: PgPoolOptions, options: PgConn client_state.test_user.id, user_device.id, Some(now), - None, + false, ) .save(&client_state.pool) .await @@ -121,7 +121,7 @@ async fn test_location_connected_devices_stats(_: PgPoolOptions, options: PgConn client_state.test_user.id, network_device.id, Some(now), - None, + false, ) .save(&client_state.pool) .await diff --git a/crates/defguard_core/tests/integration/api/user.rs b/crates/defguard_core/tests/integration/api/user.rs index 7d0f942e1a..8b74bdab2a 100644 --- a/crates/defguard_core/tests/integration/api/user.rs +++ b/crates/defguard_core/tests/integration/api/user.rs @@ -721,7 +721,7 @@ async fn test_get_user_exposes_active_network_state(_: PgPoolOptions, options: P user.id, device.id, Some(session_connected_at), - None, + false, ) .save(&pool) .await @@ -829,7 +829,7 @@ async fn test_get_user_keeps_last_successful_connection_for_newer_disconnected_s user.id, device.id, Some(last_successful_connection), - None, + false, ); connected_session.created_at = last_successful_connection; let connected_session = connected_session.save(&pool).await.unwrap(); @@ -850,7 +850,7 @@ async fn test_get_user_keeps_last_successful_connection_for_newer_disconnected_s .unwrap(); let mut disconnected_session = - VpnClientSession::new(network.id, user.id, device.id, None, None); + VpnClientSession::new(network.id, user.id, device.id, None, false); disconnected_session.created_at = disconnected_at; disconnected_session.disconnected_at = Some(disconnected_at); disconnected_session.state = VpnClientSessionState::Disconnected; @@ -1029,7 +1029,7 @@ async fn test_add_user_blocked_when_user_count_exceeds_license_limit( None, LicenseTier::Business, SupportType::Basic, - vec![], + Vec::new(), ))); let new_user = AddUserData { @@ -1081,7 +1081,7 @@ async fn test_disabled_users_not_counted_towards_license_limit( None, LicenseTier::Business, SupportType::Basic, - vec![], + Vec::new(), ))); // only admin is active, so there is still room under the limit of 2 @@ -1146,7 +1146,7 @@ async fn test_modify_user_enable_blocked_when_it_would_exceed_license_limit( None, LicenseTier::Business, SupportType::Basic, - vec![], + Vec::new(), ))); // active count is already at the limit of 2 (admin, hpotter), so re-enabling must be blocked @@ -1228,7 +1228,7 @@ async fn test_bulk_enable_users_blocked_when_it_would_exceed_license_limit( None, LicenseTier::Business, SupportType::Basic, - vec![], + Vec::new(), ))); // active count is 2 (admin, hpotter); re-enabling both would bring it to 4, over the limit of 3 diff --git a/crates/defguard_event_logger/src/lib.rs b/crates/defguard_event_logger/src/lib.rs index 52f408fec5..123f46eb64 100644 --- a/crates/defguard_event_logger/src/lib.rs +++ b/crates/defguard_event_logger/src/lib.rs @@ -705,14 +705,14 @@ fn map_to_activity_log_event(message: EventLoggerMessage) -> ActivityLogEvent Some(match mobile_auth_device_name { Some(auth_device) => format!( - "Device {device} completed MFA authorization for location {location} using {method} (approved on {auth_device})" + "Device {device} completed MFA authorization for location {location} (approved on {auth_device})" ), None => format!( - "Device {device} completed MFA authorization for location {location} using {method}" + "Device {device} completed MFA authorization for location {location}" ), }), DesktopClientMfaEvent::Failed { @@ -754,19 +754,22 @@ fn map_to_activity_log_event(message: EventLoggerMessage) -> ActivityLogEvent Some(format!( "VPN session for {device} in location {location} superseded by new authorization" )), + DesktopClientMfaEvent::MfaLoginSuperseded { device, location } => Some(format!( + "MFA login for {device} in location {location} superseded by a new login attempt" + )), }; let (event_type, metadata) = match *event { DesktopClientMfaEvent::Success { location, device, - method, + attribution, mobile_auth_device_name, } => ( EventType::VpnClientMfaSuccess, serde_json::to_value(VpnClientMfaMetadata { location, device, - method, + attribution, mobile_auth_device_name, }) .ok(), @@ -836,6 +839,10 @@ fn map_to_activity_log_event(message: EventLoggerMessage) -> ActivityLogEvent ( + EventType::VpnClientMfaLoginSuperseded, + serde_json::to_value(VpnClientMetadata { location, device }).ok(), + ), }; let module = bidi_event_module(&event_type); (module, event_type, description, metadata) diff --git a/crates/defguard_event_logger/src/message.rs b/crates/defguard_event_logger/src/message.rs index 5a338fdade..74349ce1e3 100644 --- a/crates/defguard_event_logger/src/message.rs +++ b/crates/defguard_event_logger/src/message.rs @@ -87,7 +87,8 @@ impl EventLoggerMessage { | DesktopClientMfaEvent::Disconnected { location, .. } | DesktopClientMfaEvent::PostureCheckPassed { location, .. } | DesktopClientMfaEvent::PostureCheckFailed { location, .. } - | DesktopClientMfaEvent::SessionSuperseded { location, .. } => { + | DesktopClientMfaEvent::SessionSuperseded { location, .. } + | DesktopClientMfaEvent::MfaLoginSuperseded { location, .. } => { Some(location.clone()) } }, diff --git a/crates/defguard_event_logger/src/tests/mod.rs b/crates/defguard_event_logger/src/tests/mod.rs index 981c353a6b..01d33895e6 100644 --- a/crates/defguard_event_logger/src/tests/mod.rs +++ b/crates/defguard_event_logger/src/tests/mod.rs @@ -12,6 +12,7 @@ use defguard_common::db::{ oauth2client::OAuth2Client, proxy::Proxy, settings::set_settings, + vpn_client_mfa_session::{MfaAttribution, Step, StepsSnapshot}, vpn_client_session::VpnClientMfaMethod, wireguard::ServiceLocationMode, }, @@ -204,6 +205,40 @@ fn test_maps_replaced_bidi_events_from_non_mfa_sessions_to_standard_superseded_l assert_eq!(result.module, ActivityLogModule::Vpn); } +/// A superseded in-progress MFA login is not a superseded VPN session: nothing was authorized, +/// so the entry must not claim a session existed or that an authorization replaced it. +#[test] +fn test_maps_superseded_mfa_login_to_its_own_event_and_description() { + let event = BidiStreamEvent { + context: sample_bidi_context(), + event: BidiStreamEventType::DesktopClientMfa(Box::new( + DesktopClientMfaEvent::MfaLoginSuperseded { + location: sample_location(), + device: sample_device(), + }, + )), + }; + + let result = map_to_activity_log_event(EventLoggerMessage::from_bidi_event(event)); + + assert_eq!(result.event, EventType::VpnClientMfaLoginSuperseded); + assert_eq!(result.module, ActivityLogModule::Vpn); + + let description = result.description.expect("expected a description"); + assert!( + description.contains("MFA login"), + "description should name the login, got: {description}" + ); + assert!( + !description.contains("VPN session"), + "description must not claim a VPN session was superseded, got: {description}" + ); + assert!( + !description.contains("authorization"), + "description must not claim an authorization occurred, got: {description}" + ); +} + // Helper struct for testing mapping of all existing events // to activity log entries struct EventTestCase { @@ -1336,7 +1371,16 @@ fn bidi_event_cases() -> Vec { BidiStreamEventType::DesktopClientMfa(Box::new(DesktopClientMfaEvent::Success { location: location.clone(), device: device.clone(), - method: defguard_core::events::ClientMFAMethod::MobileApprove, + attribution: MfaAttribution { + snapshot: StepsSnapshot { + flow_id: 1, + steps: vec![Step { + methods: vec![VpnClientMfaMethod::MobileApprove], + satisfied: Some(VpnClientMfaMethod::MobileApprove), + }], + }, + flow_name: Some("flow".to_owned()), + }, mobile_auth_device_name: Some("pixel-7".to_owned()), })), Some(location.clone()), @@ -1393,6 +1437,21 @@ fn bidi_event_cases() -> Vec { module: ActivityLogModule::Vpn, description_contains: Some("superseded"), }, + EventTestCase { + name: "MfaLoginSuperseded", + message: bidi_msg( + BidiStreamEventType::DesktopClientMfa(Box::new( + DesktopClientMfaEvent::MfaLoginSuperseded { + location: location.clone(), + device: device.clone(), + }, + )), + Some(location.clone()), + ), + event_type: EventType::VpnClientMfaLoginSuperseded, + module: ActivityLogModule::Vpn, + description_contains: Some("superseded"), + }, EventTestCase { name: "DevicePostureCheckPassed", message: bidi_msg( diff --git a/crates/defguard_gateway_manager/src/handler.rs b/crates/defguard_gateway_manager/src/handler.rs index 14baa8de7c..7567a48f38 100644 --- a/crates/defguard_gateway_manager/src/handler.rs +++ b/crates/defguard_gateway_manager/src/handler.rs @@ -1487,7 +1487,8 @@ mod tests { .await .unwrap(); - let mut new_session = VpnClientSession::new(network.id, user.id, new_device.id, None, None); + let mut new_session = + VpnClientSession::new(network.id, user.id, new_device.id, None, false); new_session.preshared_key = Some("new-session-psk".into()); new_session.save(&pool).await.unwrap(); @@ -1496,7 +1497,7 @@ mod tests { user.id, connected_device.id, Some(Utc::now().naive_utc()), - None, + false, ); connected_session.preshared_key = Some("connected-session-psk".into()); connected_session.save(&pool).await.unwrap(); diff --git a/crates/defguard_gateway_manager/src/tests/gateway_manager/handler/support.rs b/crates/defguard_gateway_manager/src/tests/gateway_manager/handler/support.rs index 91033b338f..e1358d184f 100644 --- a/crates/defguard_gateway_manager/src/tests/gateway_manager/handler/support.rs +++ b/crates/defguard_gateway_manager/src/tests/gateway_manager/handler/support.rs @@ -103,7 +103,7 @@ pub(crate) async fn create_authorized_mfa_device_for_network( .expect("failed to load MFA test network") .expect("expected MFA test network"); - let mut session = VpnClientSession::new(network_id, device.user_id, device.id, None, None); + let mut session = VpnClientSession::new(network_id, device.user_id, device.id, None, false); session.preshared_key = Some(preshared_key.to_owned()); session .save(&context.pool) diff --git a/crates/defguard_proto/src/lib.rs b/crates/defguard_proto/src/lib.rs index 8ebd79f678..a3410a125a 100644 --- a/crates/defguard_proto/src/lib.rs +++ b/crates/defguard_proto/src/lib.rs @@ -157,6 +157,40 @@ impl From for CoreError { } } +impl CoreError { + /// An `invalid_argument` error carrying a caller-facing message. + pub fn invalid_argument(message: impl Into) -> Self { + Self { + status_code: tonic::Code::InvalidArgument.into(), + message: message.into(), + } + } + + /// An `internal` error carrying a caller-facing message. + pub fn internal(message: impl Into) -> Self { + Self { + status_code: tonic::Code::Internal.into(), + message: message.into(), + } + } + + /// A `not_found` error carrying a caller-facing message. + pub fn not_found(message: impl Into) -> Self { + Self { + status_code: tonic::Code::NotFound.into(), + message: message.into(), + } + } + + /// A `failed_precondition` error carrying a caller-facing message. + pub fn failed_precondition(message: impl Into) -> Self { + Self { + status_code: tonic::Code::FailedPrecondition.into(), + message: message.into(), + } + } +} + impl From> for client_types::Device { fn from(device: Device) -> Self { Self { diff --git a/crates/defguard_proxy_manager/Cargo.toml b/crates/defguard_proxy_manager/Cargo.toml index 347806031d..48014275f7 100644 --- a/crates/defguard_proxy_manager/Cargo.toml +++ b/crates/defguard_proxy_manager/Cargo.toml @@ -35,7 +35,8 @@ tracing.workspace = true defguard_common = { workspace = true, features = ["test-support"] } base32.workspace = true base64.workspace = true -hyper-util = "0.1" +ed25519-dalek.workspace = true +hyper-util.workspace = true ipnetwork.workspace = true jsonwebkey = { workspace = true } jsonwebtoken.workspace = true diff --git a/crates/defguard_proxy_manager/src/handler.rs b/crates/defguard_proxy_manager/src/handler.rs index 9eda4bdd8c..2cb2f47b78 100644 --- a/crates/defguard_proxy_manager/src/handler.rs +++ b/crates/defguard_proxy_manager/src/handler.rs @@ -13,7 +13,9 @@ use defguard_common::{ VERSION, db::{ Id, - models::{Certificates, Settings, proxy::Proxy}, + models::{ + Certificates, Settings, proxy::Proxy, vpn_client_mfa_session::VpnClientMfaSession, + }, }, types::AuthFlowType, }; @@ -25,7 +27,8 @@ use defguard_core::{ directory_sync::sync_user_groups_if_configured, grpc::polling::PollingServer, handlers::openid_login::{ - SELECT_ACCOUNT_SUPPORTED_PROVIDERS, build_state, make_oidc_client, user_from_claims, + MfaOidcState, SELECT_ACCOUNT_SUPPORTED_PROVIDERS, build_state, make_oidc_client, + user_from_claims, }, is_business_license_active, ldap::utils::ldap_update_user_state, @@ -34,9 +37,7 @@ use defguard_core::{ events::{ApiEvent, DirectorySyncEvent, LdapSyncEventType, ProxyConnectionEvent}, grpc::{ GatewayCommand, - proxy::client_mfa::{ - ClientLoginSession, ClientMfaServer, ClientMfaStartOutcome, PostureCheckOutcome, - }, + proxy::client_mfa::{ClientMfaServer, ClientMfaStartOutcome, PostureCheckOutcome}, }, version::{IncompatibleComponents, IncompatibleProxyData, is_proxy_version_supported}, }; @@ -52,7 +53,10 @@ use defguard_proto::{ use defguard_version::{ ComponentInfo, DefguardComponent, client::ClientVersionInterceptor, get_tracing_variables, }; -use openidconnect::{AuthorizationCode, Nonce, Scope, core::CoreAuthenticationFlow}; +use openidconnect::{ + AuthorizationCode, EndpointMaybeSet, EndpointNotSet, EndpointSet, Nonce, Scope, + core::{CoreAuthenticationFlow, CoreClient}, +}; use reqwest::Url; use semver::Version; use sqlx::PgPool; @@ -82,6 +86,89 @@ use crate::{ const VERSION_ZERO: Version = Version::new(0, 0, 0); +/// Compute the OIDC `state` payload for an `AuthInfo` request. +/// +/// The MFA flow's payload is the opaque session token plus the session's current +/// `step_attempt_id` (`.`), so the OIDC callback can bind to the attempt +/// it was issued for rather than a superseded one. The enrollment and legacy flows have no such +/// nonce and their payload is returned unchanged. +async fn build_auth_info_state( + pool: &PgPool, + auth_flow_type: ProtoAuthFlowType, + state: Option, +) -> Result, CoreError> { + if auth_flow_type != ProtoAuthFlowType::Mfa { + return Ok(state); + } + + let Some(token) = state.as_deref() else { + debug!("OIDC MFA AuthInfo request is missing the session token"); + return Err(CoreError::invalid_argument("missing MFA session token")); + }; + + let Some(session) = VpnClientMfaSession::::find_active_by_token(pool, token) + .await + .map_err(|err| { + error!("Failed to find MFA session: {err}"); + CoreError::internal("failed to find MFA session") + })? + else { + debug!("OIDC MFA AuthInfo request references an unknown or expired session"); + return Err(CoreError::invalid_argument("MFA session not found")); + }; + + let Some(ephemeral) = session.ephemeral_state.as_ref() else { + debug!("OIDC MFA AuthInfo request references a session with no attempt in progress"); + return Err(CoreError::invalid_argument("no MFA attempt in progress")); + }; + + Ok(Some(MfaOidcState::build(token, &ephemeral.step_attempt_id))) +} + +/// The concrete OpenID Connect client `make_oidc_client` builds for the Core auth flow. +type CoreOidcClient = CoreClient< + EndpointSet, + EndpointNotSet, + EndpointNotSet, + EndpointNotSet, + EndpointMaybeSet, + EndpointMaybeSet, +>; + +/// Build the `AuthInfo` payload for a successfully built state: construct the authorize URL from +/// the client, the provider, and the state data, and wrap the resulting CSRF token, nonce, and +/// provider display name. +fn build_auth_info_payload( + client: &CoreOidcClient, + provider: &OpenIdProvider, + state_data: Option, +) -> core_response::Payload { + let mut authorize_url_builder = client + .authorize_url( + CoreAuthenticationFlow::AuthorizationCode, + || build_state(state_data), + Nonce::new_random, + ) + .add_scope(Scope::new("email".to_owned())) + .add_scope(Scope::new("profile".to_owned())); + + if SELECT_ACCOUNT_SUPPORTED_PROVIDERS + .iter() + .all(|p| p.eq_ignore_ascii_case(&provider.name)) + { + authorize_url_builder = + authorize_url_builder.add_prompt(openidconnect::core::CoreAuthPrompt::SelectAccount); + } + let (url, csrf_token, nonce) = authorize_url_builder.url(); + + core_response::Payload::AuthInfo(AuthInfoResponse { + url: url.into(), + csrf_token: csrf_token.secret().to_owned(), + nonce: nonce.secret().to_owned(), + button_display_name: provider.display_name.clone(), + }) +} + type ShutdownReceiver = tokio::sync::oneshot::Receiver; #[cfg(test)] @@ -135,14 +222,13 @@ impl ProxyHandler { url: Url, tx: &ProxyTxSet, remote_mfa_responses: Arc>>>, - sessions: Arc>>, shutdown_signal: Arc>, proxy_id: Id, proxy_cookie_key: Key, handler_tx_map: HandlerTxMap, ) -> Self { // Instantiate gRPC servers. - let services = ProxyServices::new(&pool, tx, remote_mfa_responses, sessions); + let services = ProxyServices::new(&pool, tx, remote_mfa_responses); Self { pool, @@ -167,7 +253,6 @@ impl ProxyHandler { pool: PgPool, tx: &ProxyTxSet, remote_mfa_responses: Arc>>>, - sessions: Arc>>, shutdown_signal: Arc>, proxy_cookie_key: Key, handler_tx_map: HandlerTxMap, @@ -179,7 +264,6 @@ impl ProxyHandler { url, tx, remote_mfa_responses, - sessions, shutdown_signal, proxy_id, proxy_cookie_key, @@ -800,7 +884,8 @@ impl ProxyHandler { } Some(core_request::Payload::AuthInfo(request)) => { if is_business_license_active() { - let redirect_url = match request.auth_flow_type() { + let auth_flow_type = request.auth_flow_type(); + let redirect_url = match auth_flow_type { ProtoAuthFlowType::Enrollment => { let settings = Settings::get_current_settings(); settings.edge_callback_url(AuthFlowType::Enrollment) @@ -823,66 +908,57 @@ impl ProxyHandler { { match make_oidc_client(redirect_url, &provider).await { Ok((_client_id, client)) => { - let mut authorize_url_builder = client - .authorize_url( - CoreAuthenticationFlow::AuthorizationCode, - || build_state(request.state), - Nonce::new_random, - ) - .add_scope(Scope::new("email".to_owned())) - .add_scope(Scope::new("profile".to_owned())); - - if SELECT_ACCOUNT_SUPPORTED_PROVIDERS - .iter() - .all(|p| p.eq_ignore_ascii_case(&provider.name)) + match build_auth_info_state( + &pool, + auth_flow_type, + request.state, + ) + .await { - authorize_url_builder = authorize_url_builder - .add_prompt( - openidconnect::core::CoreAuthPrompt::SelectAccount, + Ok(state_data) => { + Some(build_auth_info_payload( + &client, &provider, state_data, + )) + } + Err(err) => { + error!( + "Failed to build OIDC state: {}", + err.message ); + Some(core_response::Payload::CoreError(err)) + } } - let (url, csrf_token, nonce) = - authorize_url_builder.url(); - - Some(core_response::Payload::AuthInfo( - AuthInfoResponse { - url: url.into(), - csrf_token: csrf_token.secret().to_owned(), - nonce: nonce.secret().to_owned(), - button_display_name: provider.display_name, - }, - )) } Err(err) => { error!( "Failed to setup external OIDC provider client: {err}" ); - Some(core_response::Payload::CoreError(CoreError { - status_code: Code::Internal as i32, - message: "failed to build OIDC client".into(), - })) + Some(core_response::Payload::CoreError( + CoreError::internal( + "failed to build OIDC client", + ), + )) } } } else { error!("Failed to get current OpenID provider"); - Some(core_response::Payload::CoreError(CoreError { - status_code: Code::NotFound as i32, - message: "failed to get current OpenID provider".into(), - })) + Some(core_response::Payload::CoreError( + CoreError::not_found( + "failed to get current OpenID provider", + ), + )) } } else { error!("Invalid redirect URL in authentication info request"); - Some(core_response::Payload::CoreError(CoreError { - status_code: Code::Internal as i32, - message: "invalid redirect URL".into(), - })) + Some(core_response::Payload::CoreError(CoreError::internal( + "invalid redirect URL", + ))) } } else { warn!("Enterprise license required"); - Some(core_response::Payload::CoreError(CoreError { - status_code: Code::FailedPrecondition as i32, - message: "no valid license".into(), - })) + Some(core_response::Payload::CoreError( + CoreError::failed_precondition("no valid license"), + )) } } Some(core_request::Payload::AuthCallback(request)) => { @@ -993,10 +1069,9 @@ impl ProxyHandler { "Proxy requested an OpenID authentication info for a \ callback URL that couldn't be built. Details: {err}" ); - Some(core_response::Payload::CoreError(CoreError { - status_code: Code::Internal as i32, - message: "invalid callback URL".into(), - })) + Some(core_response::Payload::CoreError(CoreError::internal( + "invalid callback URL", + ))) } } } @@ -1103,7 +1178,6 @@ impl ProxyHandler { url: Url, tx: &ProxyTxSet, remote_mfa_responses: Arc>>>, - sessions: Arc>>, shutdown_signal: Arc>, proxy_id: Id, proxy_cookie_key: Key, @@ -1115,7 +1189,6 @@ impl ProxyHandler { url, tx, remote_mfa_responses, - sessions, shutdown_signal, proxy_id, proxy_cookie_key, @@ -1270,7 +1343,6 @@ impl ProxyServices { pool: &PgPool, tx: &ProxyTxSet, remote_mfa_responses: Arc>>>, - sessions: Arc>>, ) -> Self { let enrollment = EnrollmentServer::new( pool.clone(), @@ -1285,7 +1357,6 @@ impl ProxyServices { tx.wireguard.clone(), tx.bidi_events.clone(), remote_mfa_responses, - sessions, ); let polling = PollingServer::new(pool.clone()); diff --git a/crates/defguard_proxy_manager/src/lib.rs b/crates/defguard_proxy_manager/src/lib.rs index dd013627db..601e13faac 100644 --- a/crates/defguard_proxy_manager/src/lib.rs +++ b/crates/defguard_proxy_manager/src/lib.rs @@ -16,7 +16,6 @@ use defguard_core::{ events::{ ApiEvent, BidiStreamEvent, DirectorySyncEvent, LdapSyncEventType, ProxyConnectionEvent, }, - grpc::proxy::client_mfa::ClientLoginSession, version::IncompatibleComponents, }; use defguard_proto::proxy::{CoreResponse, HttpsCerts, PublicSettings, core_response}; @@ -200,7 +199,6 @@ impl ProxyManager { &self, proxy: &Proxy, remote_mfa_responses: Arc>>>, - sessions: Arc>>, handler_tx_map: HandlerTxMap, shutdown_rx: Arc>>, proxy_cookie_key: Key, @@ -224,7 +222,6 @@ impl ProxyManager { self.pool.clone(), &self.tx, remote_mfa_responses, - sessions, shutdown_rx, proxy_cookie_key, handler_tx_map, @@ -242,7 +239,6 @@ impl ProxyManager { self.pool.clone(), &self.tx, remote_mfa_responses, - sessions, shutdown_rx, proxy_cookie_key, handler_tx_map, @@ -255,7 +251,6 @@ impl ProxyManager { pub async fn run(mut self) -> Result<(), ProxyError> { debug!("ProxyManager starting"); let remote_mfa_responses = Arc::default(); - let sessions = Arc::default(); let (certs_tx, certs_rx) = watch::channel(Arc::new(HashMap::new())); // Prime the cache to avoid race with connection loop. refresh_certs(&self.pool, &certs_tx).await; @@ -281,7 +276,6 @@ impl ProxyManager { self.build_handler( proxy, Arc::clone(&remote_mfa_responses), - Arc::clone(&sessions), Arc::clone(&handler_tx_map), Arc::new(Mutex::new(shutdown_rx)), self.proxy_cookie_key.clone(), @@ -328,7 +322,6 @@ impl ProxyManager { match self.build_handler( &proxy_model, Arc::clone(&remote_mfa_responses), - Arc::clone(&sessions), Arc::clone(&handler_tx_map), Arc::new(Mutex::new(shutdown_rx)), self.proxy_cookie_key.clone(), diff --git a/crates/defguard_proxy_manager/src/tests/common/mod.rs b/crates/defguard_proxy_manager/src/tests/common/mod.rs index 476b405eed..d28bb8b1aa 100644 --- a/crates/defguard_proxy_manager/src/tests/common/mod.rs +++ b/crates/defguard_proxy_manager/src/tests/common/mod.rs @@ -451,7 +451,6 @@ impl HandlerTestContext { .expect("failed to build proxy url"); let remote_mfa_responses = Arc::default(); - let sessions = Arc::default(); let (shutdown_tx, shutdown_rx) = oneshot::channel::(); let handler = ProxyHandler::new_with_test_socket( @@ -459,7 +458,6 @@ impl HandlerTestContext { url, &tx_set, remote_mfa_responses, - sessions, Arc::new(tokio::sync::Mutex::new(shutdown_rx)), proxy.id, axum_extra::extract::cookie::Key::derive_from( diff --git a/crates/defguard_proxy_manager/src/tests/proxy_manager/handler/mfa.rs b/crates/defguard_proxy_manager/src/tests/proxy_manager/handler/mfa.rs index 71360d37e1..3691253f99 100644 --- a/crates/defguard_proxy_manager/src/tests/proxy_manager/handler/mfa.rs +++ b/crates/defguard_proxy_manager/src/tests/proxy_manager/handler/mfa.rs @@ -8,11 +8,13 @@ use tokio::{task, time::timeout}; use tonic::Code; use super::support::{ - assert_error_response, assert_vpn_session_exists, clear_test_license, complete_proxy_handshake, - create_external_mfa_network, create_mfa_network, create_network, create_user_with_device, - expect_bidi_mfa_success, generate_totp_code, make_device_info, send_mfa_finish, - send_mfa_finish_no_recv, send_mfa_finish_raw, send_mfa_start, send_token_validation, - setup_user_email_mfa, setup_user_totp_mfa, + assert_error_response, assert_error_response_with_message, assert_vpn_session_exists, + biometric_pub_key, clear_test_license, complete_proxy_handshake, create_external_mfa_network, + create_mfa_network, create_network, create_user_with_device, expect_bidi_mfa_success, + generate_totp_code, make_device_info, register_biometric_key, send_mfa_finish, + send_mfa_finish_no_recv, send_mfa_finish_raw, send_mfa_finish_signed, send_mfa_start, + send_mfa_start_with_challenge, send_token_validation, set_test_license_business, + setup_user_email_mfa, setup_user_totp_mfa, sign_challenge, }; use crate::tests::common::{HandlerTestContext, RECEIVE_TIMEOUT}; @@ -30,7 +32,7 @@ async fn test_mfa_start_fails_for_disabled_location(_: PgPoolOptions, options: P context.mock_proxy().send_request(CoreRequest { id: 1, - device_info: None, + device_info: Some(make_device_info()), payload: Some(core_request::Payload::ClientMfaStart( ClientMfaStartRequest { location_id: network.id, @@ -64,7 +66,7 @@ async fn test_mfa_start_fails_for_unknown_location(_: PgPoolOptions, options: Pg context.mock_proxy().send_request(CoreRequest { id: 2, - device_info: None, + device_info: Some(make_device_info()), payload: Some(core_request::Payload::ClientMfaStart( ClientMfaStartRequest { location_id: nonexistent_location_id, @@ -160,6 +162,116 @@ async fn test_mfa_finish_succeeds_with_totp_code(_: PgPoolOptions, options: PgCo context.finish().await.expect_server_finished().await; } +/// The legacy single-step biometric flow completes end-to-end against the DB-backed session. +/// +/// `start` issues a challenge bound to the device's enrolled key; `finish` returns the signature +/// as `code` and the handler verifies it against that key. +#[sqlx::test] +async fn test_mfa_finish_succeeds_with_biometric_signature( + _: PgPoolOptions, + options: PgConnectOptions, +) { + let mut context = HandlerTestContext::new(options).await; + complete_proxy_handshake(&mut context).await; + + let network = create_mfa_network(&context.pool).await; + let (_user, device) = create_user_with_device(&context.pool).await; + let signing_key = register_biometric_key(&context.pool, device.id).await; + + let (_, token, challenge) = send_mfa_start_with_challenge( + &mut context, + network.id, + &device.wireguard_pubkey, + MfaMethod::Biometric, + ) + .await; + let challenge = challenge.expect("biometric start must return a challenge to sign"); + + // Subscribe before finish so the handler's gateway_tx.send() has a receiver. + let mut gateway_rx = context.gateway_tx.subscribe(); + + let signature = sign_challenge(&signing_key, &challenge); + let (_, psk) = send_mfa_finish(&mut context, &token, Some(&signature)).await; + assert!( + !psk.is_empty(), + "PSK must not be empty after successful biometric MFA" + ); + + let session = assert_vpn_session_exists(&context.pool, network.id, device.id).await; + assert!(session.preshared_key.is_some()); + + let event = timeout(RECEIVE_TIMEOUT, gateway_rx.recv()) + .await + .expect("timed out waiting for GatewayCommand::VpnSessionAuthorized") + .expect("gateway command channel closed"); + let gateway_loc_id = match event { + GatewayCommand::VpnSessionAuthorized(loc_id, _, _) => loc_id, + other => panic!("expected VpnSessionAuthorized, got: {other:?}"), + }; + assert_eq!(gateway_loc_id, network.id); + + let event_loc_id = expect_bidi_mfa_success(&mut context.bidi_events_rx).await; + assert_eq!(event_loc_id, network.id); + + context.finish().await.expect_server_finished().await; +} + +/// The legacy single-step mobile-approve flow completes end-to-end against the DB-backed session. +/// +/// This is the fused path: the approving device's key rides in `auth_pub_key` on `finish` and the +/// handler verifies the signature and authorizes in one call. The durable-mark route, where an +/// out-of-band approval is collected by a later `finish` poll, arrives with #3046. +#[sqlx::test] +async fn test_mfa_finish_succeeds_with_mobile_approve_signature( + _: PgPoolOptions, + options: PgConnectOptions, +) { + let mut context = HandlerTestContext::new(options).await; + complete_proxy_handshake(&mut context).await; + + let network = create_mfa_network(&context.pool).await; + let (_user, device) = create_user_with_device(&context.pool).await; + let signing_key = register_biometric_key(&context.pool, device.id).await; + let auth_pub_key = biometric_pub_key(&signing_key); + + let (_, token, challenge) = send_mfa_start_with_challenge( + &mut context, + network.id, + &device.wireguard_pubkey, + MfaMethod::MobileApprove, + ) + .await; + let challenge = challenge.expect("mobile approve start must return a challenge to sign"); + + let mut gateway_rx = context.gateway_tx.subscribe(); + + let signature = sign_challenge(&signing_key, &challenge); + let (_, psk) = + send_mfa_finish_signed(&mut context, &token, Some(&signature), Some(&auth_pub_key)).await; + assert!( + !psk.is_empty(), + "PSK must not be empty after successful mobile-approve MFA" + ); + + let session = assert_vpn_session_exists(&context.pool, network.id, device.id).await; + assert!(session.preshared_key.is_some()); + + let event = timeout(RECEIVE_TIMEOUT, gateway_rx.recv()) + .await + .expect("timed out waiting for GatewayCommand::VpnSessionAuthorized") + .expect("gateway command channel closed"); + let gateway_loc_id = match event { + GatewayCommand::VpnSessionAuthorized(loc_id, _, _) => loc_id, + other => panic!("expected VpnSessionAuthorized, got: {other:?}"), + }; + assert_eq!(gateway_loc_id, network.id); + + let event_loc_id = expect_bidi_mfa_success(&mut context.bidi_events_rx).await; + assert_eq!(event_loc_id, network.id); + + context.finish().await.expect_server_finished().await; +} + #[sqlx::test] async fn test_mfa_finish_fails_with_wrong_totp_code(_: PgPoolOptions, options: PgConnectOptions) { let mut context = HandlerTestContext::new(options).await; @@ -209,7 +321,7 @@ async fn test_mfa_start_fails_for_unknown_device(_: PgPoolOptions, options: PgCo context.mock_proxy().send_request(CoreRequest { id: 1, - device_info: None, + device_info: Some(make_device_info()), payload: Some(core_request::Payload::ClientMfaStart( ClientMfaStartRequest { location_id: network.id, @@ -244,7 +356,7 @@ async fn test_mfa_start_fails_when_email_mfa_not_enabled( context.mock_proxy().send_request(CoreRequest { id: 1, - device_info: None, + device_info: Some(make_device_info()), payload: Some(core_request::Payload::ClientMfaStart( ClientMfaStartRequest { location_id: network.id, @@ -407,22 +519,28 @@ async fn test_mfa_finish_fails_with_wrong_code(_: PgPoolOptions, options: PgConn context.finish().await.expect_server_finished().await; } +/// Without a business license, OIDC is removed from the flow's available methods, so selecting +/// it is rejected as unsupported by the location. +/// +/// This is written as a differential test on purpose. Every rejection on this path returns +/// `InvalidArgument`, so asserting the code alone proves nothing: it passes just as well when +/// the license gate is not enforced at all. The licensed run pins that down - it must fail for +/// a *different* reason (the unconfigured OIDC provider), which it can only do if the gate +/// changed the outcome. #[sqlx::test] async fn test_mfa_oidc_start_requires_license(_: PgPoolOptions, options: PgConnectOptions) { let mut context = HandlerTestContext::new(options).await; complete_proxy_handshake(&mut context).await; - clear_test_license(); - - // External MFA location + OIDC method but no business license + // External MFA location + OIDC method, no OIDC provider configured let network = create_external_mfa_network(&context.pool).await; let (mut user, device) = create_user_with_device(&context.pool).await; // email MFA is irrelevant for OIDC path but user still needs to exist setup_user_email_mfa(&context.pool, &mut user).await; - context.mock_proxy().send_request(CoreRequest { - id: 1, - device_info: None, + let request = |id: u64| CoreRequest { + id, + device_info: Some(make_device_info()), payload: Some(core_request::Payload::ClientMfaStart( ClientMfaStartRequest { location_id: network.id, @@ -433,11 +551,25 @@ async fn test_mfa_oidc_start_requires_license(_: PgPoolOptions, options: PgConne selected_methods: Vec::new(), }, )), - }); + }; + // Unlicensed: the license gate filters OIDC out of the first step, so the method is not + // among those the location offers. + clear_test_license(); + context.mock_proxy().send_request(request(1)); let response = context.mock_proxy_mut().recv_outbound().await; - let code = assert_error_response(&response); + let (code, message) = assert_error_response_with_message(&response); + assert_eq!(code, Code::InvalidArgument); + assert_eq!(message, "selected MFA method is not supported by location"); + + // Licensed: OIDC survives the filter, so the request gets past the gate and fails further + // in, on the provider that was never configured. + set_test_license_business(); + context.mock_proxy().send_request(request(2)); + let response = context.mock_proxy_mut().recv_outbound().await; + let (code, message) = assert_error_response_with_message(&response); assert_eq!(code, Code::InvalidArgument); + assert_eq!(message, "selected MFA method is not available"); context.finish().await.expect_server_finished().await; } diff --git a/crates/defguard_proxy_manager/src/tests/proxy_manager/handler/oidc.rs b/crates/defguard_proxy_manager/src/tests/proxy_manager/handler/oidc.rs index 536732d9dc..e8da9f4f77 100644 --- a/crates/defguard_proxy_manager/src/tests/proxy_manager/handler/oidc.rs +++ b/crates/defguard_proxy_manager/src/tests/proxy_manager/handler/oidc.rs @@ -1,9 +1,16 @@ #![allow(deprecated)] -use defguard_common::db::models::settings::{Settings, update_current_settings}; +use base64::{Engine, prelude::BASE64_STANDARD}; +use defguard_common::db::{ + Id, + models::{ + settings::{Settings, update_current_settings}, + vpn_client_mfa_session::VpnClientMfaSession, + }, +}; use defguard_core::{ db::models::enrollment::Token, enterprise::{ - handlers::openid_login::build_state, + handlers::openid_login::{MfaOidcState, build_state}, license::{License, LicenseTier, SupportType, set_cached_license}, limits::update_counts, }, @@ -17,6 +24,7 @@ use defguard_proto::{ core_response, }, }; +use reqwest::Url; use sqlx::postgres::{PgConnectOptions, PgPoolOptions}; use tokio::time::timeout; @@ -175,11 +183,33 @@ async fn test_auth_info_mfa_returns_authorize_url(_: PgPoolOptions, options: PgC let _provider = create_oidc_provider(&context.pool, &mock).await; set_public_proxy_url(&context.pool, &mock.base_url).await; + // The MFA flow requires an active session whose token rides in `state`. Start one for the + // external (OIDC) network so `start_client_mfa_login` accepts the Oidc method. + let network = create_external_mfa_network(&context.pool).await; + let (_user, device) = create_user_with_device(&context.pool).await; + let (_id, mfa_token) = send_mfa_start( + &mut context, + network.id, + &device.wireguard_pubkey, + MfaMethod::Oidc, + ) + .await; + let session = VpnClientMfaSession::::find_active_by_token(&context.pool, &mfa_token) + .await + .expect("failed to find active MFA session") + .expect("expected an active MFA session"); + let attempt_id = session + .ephemeral_state + .as_ref() + .expect("expected an attempt in progress") + .step_attempt_id + .clone(); + context.mock_proxy().send_request(CoreRequest { id: 50, device_info: None, payload: Some(core_request::Payload::AuthInfo(AuthInfoRequest { - state: None, + state: Some(mfa_token.clone()), auth_flow_type: AuthFlowType::Mfa as i32, ..Default::default() })), @@ -213,6 +243,24 @@ async fn test_auth_info_mfa_returns_authorize_url(_: PgPoolOptions, options: PgC ); assert!(!auth_info.nonce.is_empty(), "expected non-empty nonce"); + // The authorize URL's `state` must carry "..". The csrf + // prefix is the browser's CSRF nonce; the tail is what the callback parses back out. + let url = Url::parse(&auth_info.url).expect("failed to parse authorize URL"); + let state_param = url + .query_pairs() + .find(|(key, _)| key == "state") + .map(|(_, value)| value.into_owned()) + .expect("authorize URL must carry a state parameter"); + let decoded = BASE64_STANDARD + .decode(state_param.as_bytes()) + .expect("state must be base64"); + let decoded = String::from_utf8(decoded).expect("state must be UTF-8"); + let (csrf, tail) = decoded + .split_once('.') + .expect("state must be ."); + assert!(!csrf.is_empty(), "state must carry a csrf prefix"); + assert_eq!(tail, MfaOidcState::build(&mfa_token, &attempt_id)); + clear_test_license(); context.finish().await.expect_server_finished().await; } @@ -416,8 +464,21 @@ async fn test_mfa_oidc_full_flow(_: PgPoolOptions, options: PgConnectOptions) { .await; // ---- Step 2: ClientMfaOidcAuthenticate ---- - // Build the `state` field by encoding the mfa_token inside it. - let state = build_state(Some(mfa_token.clone())).secret().clone(); + // Build the `state` field the way the authorize-URL builder does for the MFA flow: + // encode ".". + let session = VpnClientMfaSession::::find_active_by_token(&context.pool, &mfa_token) + .await + .expect("failed to find active MFA session") + .expect("expected an active MFA session"); + let attempt_id = session + .ephemeral_state + .as_ref() + .expect("expected an attempt in progress") + .step_attempt_id + .clone(); + let state = build_state(Some(MfaOidcState::build(&mfa_token, &attempt_id))) + .secret() + .clone(); let raw_nonce = "mfa-oidc-nonce"; let code = make_oidc_code(&user.email, &user.email, raw_nonce); @@ -460,6 +521,160 @@ async fn test_mfa_oidc_full_flow(_: PgPoolOptions, options: PgConnectOptions) { context.finish().await.expect_server_finished().await; } +/// A callback whose state-carried `step_attempt_id` does not match the live row is rejected. +/// +/// The genuine end-to-end case - a callback from an attempt superseded by a re-issue on the SAME +/// token - is not reachable until #3045 adds re-attempts; no production path re-issues an attempt +/// on a live token yet. This test constructs a stale id against a live row instead, so the gap is +/// covered explicitly rather than left looking tested. +#[sqlx::test] +async fn test_mfa_oidc_rejects_stale_attempt_id(_: PgPoolOptions, options: PgConnectOptions) { + let mut context = HandlerTestContext::new(options).await; + complete_proxy_handshake(&mut context).await; + set_test_license_business(); + + let network = create_external_mfa_network(&context.pool).await; + let (user, device) = create_user_with_device(&context.pool).await; + + let mock = MockOidcProvider::start().await; + let _provider = create_oidc_provider(&context.pool, &mock).await; + set_public_proxy_url(&context.pool, &mock.base_url).await; + + let (_id, mfa_token) = send_mfa_start( + &mut context, + network.id, + &device.wireguard_pubkey, + MfaMethod::Oidc, + ) + .await; + + // Build a state carrying a stale attempt id that does not match the live row. + let state = build_state(Some(MfaOidcState::build(&mfa_token, "stale-attempt-id"))) + .secret() + .clone(); + + let raw_nonce = "mfa-oidc-stale-nonce"; + let oidc_code = make_oidc_code(&user.email, &user.email, raw_nonce); + + context.mock_proxy().send_request(CoreRequest { + id: 31, + device_info: Some(make_device_info()), + payload: Some(core_request::Payload::ClientMfaOidcAuthenticate( + ClientMfaOidcAuthenticateRequest { + code: oidc_code, + state, + nonce: raw_nonce.to_owned(), + }, + )), + }); + + // The handler must reject the stale attempt rather than silently ignore it. + let response = context.mock_proxy_mut().recv_outbound().await; + let error_code = assert_error_response(&response); + assert_eq!( + error_code, + tonic::Code::InvalidArgument, + "expected InvalidArgument for a stale attempt id" + ); + + // The live attempt is untouched: the mark was a no-op, so the session is still pending OIDC. + let session = VpnClientMfaSession::::find_active_by_token(&context.pool, &mfa_token) + .await + .expect("failed to find active MFA session") + .expect("expected the session to remain live"); + assert!( + !session + .ephemeral_state + .as_ref() + .expect("expected an attempt in progress") + .openid_auth_completed, + "stale callback must not mark the attempt complete" + ); + + clear_test_license(); + context.finish().await.expect_server_finished().await; +} + +/// A callback carrying a stale `step_attempt_id` must not destroy the live attempt, even when the +/// callback would otherwise fail on a path that deletes the session. +/// +/// Every abort path in the handler (wrong method, bad callback URL, wrong account, bad code) +/// deletes the row. The attempt-id check therefore has to run before all of them: otherwise a late +/// callback from a superseded attempt tears down the attempt that replaced it. This drives an +/// unverifiable OIDC code so the request would reach the delete-on-failure branch, and asserts the +/// session is still live afterwards. +#[sqlx::test] +async fn test_mfa_oidc_stale_attempt_id_does_not_delete_session( + _: PgPoolOptions, + options: PgConnectOptions, +) { + let mut context = HandlerTestContext::new(options).await; + complete_proxy_handshake(&mut context).await; + set_test_license_business(); + + let network = create_external_mfa_network(&context.pool).await; + let (_user, device) = create_user_with_device(&context.pool).await; + + let mock = MockOidcProvider::start().await; + let _provider = create_oidc_provider(&context.pool, &mock).await; + set_public_proxy_url(&context.pool, &mock.base_url).await; + + let (_id, mfa_token) = send_mfa_start( + &mut context, + network.id, + &device.wireguard_pubkey, + MfaMethod::Oidc, + ) + .await; + + let state = build_state(Some(MfaOidcState::build(&mfa_token, "stale-attempt-id"))) + .secret() + .clone(); + + // A code for an account that does not exist: verification fails, and that failure path is one + // of the branches that deletes the session. + let raw_nonce = "mfa-oidc-stale-delete-nonce"; + let oidc_code = make_oidc_code("no-such-sub", "no-such-user@example.com", raw_nonce); + + context.mock_proxy().send_request(CoreRequest { + id: 32, + device_info: Some(make_device_info()), + payload: Some(core_request::Payload::ClientMfaOidcAuthenticate( + ClientMfaOidcAuthenticateRequest { + code: oidc_code, + state, + nonce: raw_nonce.to_owned(), + }, + )), + }); + + // Rejected for the stale attempt id, not for the bad code: the binding is checked first. + let response = context.mock_proxy_mut().recv_outbound().await; + let error_code = assert_error_response(&response); + assert_eq!( + error_code, + tonic::Code::InvalidArgument, + "expected InvalidArgument for a stale attempt id" + ); + + // The decisive assertion: the live session survived a failing callback bound to a dead attempt. + let session = VpnClientMfaSession::::find_active_by_token(&context.pool, &mfa_token) + .await + .expect("failed to find active MFA session") + .expect("a stale callback must not delete the live session"); + assert!( + !session + .ephemeral_state + .as_ref() + .expect("expected an attempt in progress") + .openid_auth_completed, + "stale callback must not mark the attempt complete" + ); + + clear_test_license(); + context.finish().await.expect_server_finished().await; +} + /// When the OIDC code's email matches a pre-existing user the handler must /// return a valid enrollment token bound to that user (not create a new one). #[sqlx::test] diff --git a/crates/defguard_proxy_manager/src/tests/proxy_manager/handler/support.rs b/crates/defguard_proxy_manager/src/tests/proxy_manager/handler/support.rs index 241006c19d..0877c3a32b 100644 --- a/crates/defguard_proxy_manager/src/tests/proxy_manager/handler/support.rs +++ b/crates/defguard_proxy_manager/src/tests/proxy_manager/handler/support.rs @@ -5,11 +5,13 @@ use std::{ time::SystemTime, }; +use base64::{Engine, prelude::BASE64_STANDARD}; use defguard_common::{ db::{ Id, NoId, models::{ Device, DeviceType, User, WireguardNetwork, + biometric_auth::BiometricAuth, mfa_flow::{LocationMfaFlowAssignment, MfaFlow}, polling_token::PollingToken, settings::{Settings, update_current_settings}, @@ -47,6 +49,7 @@ use defguard_proto::{ core_request, core_response, }, }; +use ed25519_dalek::{Signer, SigningKey}; use ipnetwork::IpNetwork; use sqlx::PgPool; use tokio::{sync::mpsc::UnboundedReceiver, time::timeout}; @@ -95,8 +98,18 @@ pub(crate) fn assert_device_config_response(response: &CoreResponse) -> &DeviceC /// Assert that a `CoreResponse` carries a `CoreError` payload and return the /// tonic status code. pub(crate) fn assert_error_response(response: &CoreResponse) -> Code { + assert_error_response_with_message(response).0 +} + +/// Like [`assert_error_response`], but also returns the error message. +/// +/// Needed wherever several distinct rejection reasons share a status code: asserting the code +/// alone would pass no matter which of them fired. +pub(crate) fn assert_error_response_with_message(response: &CoreResponse) -> (Code, String) { match &response.payload { - Some(core_response::Payload::CoreError(err)) => Code::from_i32(err.status_code), + Some(core_response::Payload::CoreError(err)) => { + (Code::from_i32(err.status_code), err.message.clone()) + } other => panic!( "expected CoreError response, got: {:?}", other.as_ref().map(discriminant) @@ -650,11 +663,29 @@ pub(crate) async fn send_mfa_start( pubkey: &str, method: MfaMethod, ) -> (u64, String) { + let (id, token, _challenge) = + send_mfa_start_with_challenge(context, location_id, pubkey, method).await; + (id, token) +} + +/// Send `ClientMfaStart` and return `(request id, token, challenge)`. +/// +/// The challenge is `None` for methods that do not issue one (TOTP, email, OIDC); the biometric +/// and mobile-approve flows return the string the client must sign. +/// +/// Requires `device_info` because the handler calls `parse_client_ip_agent`, same as +/// [`send_mfa_finish`]. +pub(crate) async fn send_mfa_start_with_challenge( + context: &mut HandlerTestContext, + location_id: Id, + pubkey: &str, + method: MfaMethod, +) -> (u64, String, Option) { static MFA_CTR: AtomicU64 = AtomicU64::new(2000); let id = MFA_CTR.fetch_add(1, Ordering::Relaxed); context.mock_proxy().send_request(CoreRequest { id, - device_info: None, + device_info: Some(make_device_info()), payload: Some(core_request::Payload::ClientMfaStart( ClientMfaStartRequest { location_id, @@ -667,8 +698,8 @@ pub(crate) async fn send_mfa_start( )), }); let response = context.mock_proxy_mut().recv_outbound().await; - let token = match &response.payload { - Some(core_response::Payload::ClientMfaStart(r)) => r.token.clone(), + let (token, challenge) = match &response.payload { + Some(core_response::Payload::ClientMfaStart(r)) => (r.token.clone(), r.challenge.clone()), Some(core_response::Payload::CoreError(e)) => panic!( "send_mfa_start: got CoreError status={} msg={}", e.status_code, e.message @@ -678,7 +709,31 @@ pub(crate) async fn send_mfa_start( other.as_ref().map(discriminant) ), }; - (id, token) + (id, token, challenge) +} + +/// Register an ed25519 biometric-auth key for `device_id` and return the signing key. +/// +/// Both legacy signature flows verify a challenge against a key the device enrolled up front, so +/// a test has to plant one before it can produce a signature the handler will accept. +pub(crate) async fn register_biometric_key(pool: &PgPool, device_id: Id) -> SigningKey { + let signing_key = SigningKey::generate(&mut rand::rngs::OsRng); + let pub_key = BASE64_STANDARD.encode(signing_key.verifying_key().as_bytes()); + BiometricAuth::new(device_id, pub_key) + .save(pool) + .await + .expect("failed to save biometric auth key"); + signing_key +} + +/// Base64 public key matching [`register_biometric_key`]'s signing key. +pub(crate) fn biometric_pub_key(signing_key: &SigningKey) -> String { + BASE64_STANDARD.encode(signing_key.verifying_key().as_bytes()) +} + +/// Sign a challenge the way the client does: ed25519 over the raw challenge bytes, base64-encoded. +pub(crate) fn sign_challenge(signing_key: &SigningKey, challenge: &str) -> String { + BASE64_STANDARD.encode(signing_key.sign(challenge.as_bytes()).to_bytes()) } /// Send `ClientMfaFinish` and return `(response, preshared_key)`. @@ -689,6 +744,19 @@ pub(crate) async fn send_mfa_finish( context: &mut HandlerTestContext, token: &str, code: Option<&str>, +) -> (CoreResponse, String) { + send_mfa_finish_signed(context, token, code, None).await +} + +/// Send `ClientMfaFinish` carrying an `auth_pub_key` and return `(response, preshared_key)`. +/// +/// Mobile approve needs the approving device's key alongside the signature; biometric passes +/// `None` because the challenge already remembers its owner. Panics if the handler errors. +pub(crate) async fn send_mfa_finish_signed( + context: &mut HandlerTestContext, + token: &str, + code: Option<&str>, + auth_pub_key: Option<&str>, ) -> (CoreResponse, String) { static MFA_CTR: AtomicU64 = AtomicU64::new(2000); let id = MFA_CTR.fetch_add(1, Ordering::Relaxed); @@ -699,7 +767,7 @@ pub(crate) async fn send_mfa_finish( ClientMfaFinishRequest { token: token.to_owned(), code: code.map(str::to_owned), - auth_pub_key: None, + auth_pub_key: auth_pub_key.map(str::to_owned), }, )), }); diff --git a/crates/defguard_session_manager/src/lib.rs b/crates/defguard_session_manager/src/lib.rs index a689d07c2f..d3f0e74fa3 100644 --- a/crates/defguard_session_manager/src/lib.rs +++ b/crates/defguard_session_manager/src/lib.rs @@ -291,7 +291,7 @@ impl SessionManager { ) -> Result<(), SessionManagerError> { let disconnect_timestamp = Utc::now().naive_utc(); let is_connected = session.connected_at.is_some(); - let is_mfa_session = session.mfa_method.is_some(); + let is_mfa_session = session.is_mfa_session; // update session record in DB session.disconnected_at = Some(disconnect_timestamp); diff --git a/crates/defguard_session_manager/src/session_state.rs b/crates/defguard_session_manager/src/session_state.rs index a7a821d479..2621a4c684 100644 --- a/crates/defguard_session_manager/src/session_state.rs +++ b/crates/defguard_session_manager/src/session_state.rs @@ -307,7 +307,7 @@ impl ActiveSessionsMap { location, user, device, - is_mfa_session: db_session.mfa_method.is_some(), + is_mfa_session: db_session.is_mfa_session, }) } else { None @@ -395,7 +395,7 @@ impl ActiveSessionsMap { user.id, device_id, Some(stats_update.latest_handshake), - None, + false, ) .save(transaction) .await?; diff --git a/crates/defguard_session_manager/tests/common/mod.rs b/crates/defguard_session_manager/tests/common/mod.rs index f0a5b991c9..7ffa56f198 100644 --- a/crates/defguard_session_manager/tests/common/mod.rs +++ b/crates/defguard_session_manager/tests/common/mod.rs @@ -11,7 +11,7 @@ use defguard_common::{ Device, DeviceType, User, WireguardNetwork, device::WireguardNetworkDevice, gateway::Gateway, - vpn_client_session::{VpnClientMfaMethod, VpnClientSession, VpnClientSessionState}, + vpn_client_session::{VpnClientSession, VpnClientSessionState}, vpn_session_stats::VpnSessionStats, wireguard::ServiceLocationMode, }, @@ -269,7 +269,7 @@ pub(crate) async fn authorize_device_in_location( user_id, device_id, Some(truncate_timestamp(chrono::Utc::now().naive_utc())), - Some(VpnClientMfaMethod::Totp), + true, ); session.preshared_key = Some(preshared_key.to_owned()); session.state = VpnClientSessionState::Connected; @@ -323,11 +323,16 @@ pub(crate) async fn create_session( user_id: Id, device_id: Id, connected_at: Option, - mfa_method: Option, + is_mfa_session: bool, preshared_key: Option<&str>, ) -> VpnClientSession { - let mut session = - VpnClientSession::new(location_id, user_id, device_id, connected_at, mfa_method); + let mut session = VpnClientSession::new( + location_id, + user_id, + device_id, + connected_at, + is_mfa_session, + ); session.preshared_key = preshared_key.map(str::to_owned); session .save(pool) diff --git a/crates/defguard_session_manager/tests/session_manager/db_invariants.rs b/crates/defguard_session_manager/tests/session_manager/db_invariants.rs index 3b62ccf2fb..b98932b8e0 100644 --- a/crates/defguard_session_manager/tests/session_manager/db_invariants.rs +++ b/crates/defguard_session_manager/tests/session_manager/db_invariants.rs @@ -19,8 +19,8 @@ async fn insert_session( let connected_at = (state == "connected").then(|| Utc::now().naive_utc()); query_scalar( - "INSERT INTO vpn_client_session (location_id, user_id, device_id, connected_at, mfa_method, state, preshared_key) \ - VALUES ($1, $2, $3, $4, NULL, $5::vpn_client_session_state, NULL) \ + "INSERT INTO vpn_client_session (location_id, user_id, device_id, connected_at, is_mfa_session, state, preshared_key) \ + VALUES ($1, $2, $3, $4, false, $5::vpn_client_session_state, NULL) \ RETURNING id", ) .bind(location_id) diff --git a/crates/defguard_session_manager/tests/session_manager/disconnects.rs b/crates/defguard_session_manager/tests/session_manager/disconnects.rs index d288bedeb4..6ec755d1c8 100644 --- a/crates/defguard_session_manager/tests/session_manager/disconnects.rs +++ b/crates/defguard_session_manager/tests/session_manager/disconnects.rs @@ -32,7 +32,7 @@ async fn test_inactive_connected_sessions_are_disconnected_after_threshold( user.id, device.id, Some(stale_handshake), - None, + false, None, ) .await; @@ -80,7 +80,7 @@ async fn test_recent_connected_sessions_remain_active(_: PgPoolOptions, options: user.id, device.id, Some(recent_handshake), - None, + false, None, ) .await; diff --git a/crates/defguard_session_manager/tests/session_manager/event_flow.rs b/crates/defguard_session_manager/tests/session_manager/event_flow.rs index 68ddbf9fe5..d183ceef41 100644 --- a/crates/defguard_session_manager/tests/session_manager/event_flow.rs +++ b/crates/defguard_session_manager/tests/session_manager/event_flow.rs @@ -78,7 +78,7 @@ async fn test_reusing_existing_connected_session_does_not_emit_duplicate_connect user.id, device.id, Some(connected_at), - None, + false, None, ) .await; @@ -120,7 +120,7 @@ async fn test_session_manager_emits_disconnect_event_for_inactive_standard_sessi user.id, device.id, Some(stale_handshake), - None, + false, None, ) .await; diff --git a/crates/defguard_session_manager/tests/session_manager/mfa.rs b/crates/defguard_session_manager/tests/session_manager/mfa.rs index 2d898075e9..8bae940339 100644 --- a/crates/defguard_session_manager/tests/session_manager/mfa.rs +++ b/crates/defguard_session_manager/tests/session_manager/mfa.rs @@ -4,7 +4,7 @@ use chrono::{TimeDelta, Utc}; use defguard_common::{ db::{ models::{ - vpn_client_session::{VpnClientMfaMethod, VpnClientSession, VpnClientSessionState}, + vpn_client_session::{VpnClientSession, VpnClientSessionState}, vpn_session_stats::VpnSessionStats, }, setup_pool, @@ -78,16 +78,7 @@ async fn test_mfa_new_session_upgrades_to_connected_on_stats( let gateway = create_gateway(&pool, location.id, user.fullname()).await; let mut harness = SessionManagerHarness::new(pool.clone()); - let session = create_session( - &pool, - location.id, - user.id, - device.id, - None, - Some(VpnClientMfaMethod::Totp), - None, - ) - .await; + let session = create_session(&pool, location.id, user.id, device.id, None, true, None).await; let endpoint: SocketAddr = "203.0.113.10:51820".parse().unwrap(); let handshake = truncate_timestamp(Utc::now().naive_utc()); @@ -190,16 +181,7 @@ async fn test_duplicate_first_stats_on_mfa_new_session_are_idempotent( let gateway = create_gateway(&pool, location.id, user.fullname()).await; let mut harness = SessionManagerHarness::new(pool.clone()); - let session = create_session( - &pool, - location.id, - user.id, - device.id, - None, - Some(VpnClientMfaMethod::Totp), - None, - ) - .await; + let session = create_session(&pool, location.id, user.id, device.id, None, true, None).await; let endpoint: SocketAddr = "203.0.113.10:51820".parse().unwrap(); let handshake = truncate_timestamp(Utc::now().naive_utc()); @@ -275,16 +257,7 @@ async fn test_repeated_later_stats_on_mfa_session_remain_idempotent( let gateway = create_gateway(&pool, location.id, user.fullname()).await; let mut harness = SessionManagerHarness::new(pool.clone()); - let session = create_session( - &pool, - location.id, - user.id, - device.id, - None, - Some(VpnClientMfaMethod::Totp), - None, - ) - .await; + let session = create_session(&pool, location.id, user.id, device.id, None, true, None).await; let endpoint: SocketAddr = "203.0.113.10:51820".parse().unwrap(); let first_handshake = truncate_timestamp(Utc::now().naive_utc() - TimeDelta::seconds(30)); @@ -380,16 +353,7 @@ async fn test_closed_event_channel_keeps_mfa_first_stats_upgrade_idempotent( let gateway = create_gateway(&pool, location.id, user.fullname()).await; let mut harness = SessionManagerHarness::new(pool.clone()); - let session = create_session( - &pool, - location.id, - user.id, - device.id, - None, - Some(VpnClientMfaMethod::Totp), - None, - ) - .await; + let session = create_session(&pool, location.id, user.id, device.id, None, true, None).await; let endpoint: SocketAddr = "203.0.113.10:51820".parse().unwrap(); let first_handshake = truncate_timestamp(Utc::now().naive_utc() - TimeDelta::seconds(30)); @@ -542,7 +506,7 @@ async fn test_never_connected_mfa_new_sessions_disconnect_after_threshold( user.id, device.id, None, - Some(VpnClientMfaMethod::Totp), + true, Some("psk-before-timeout"), ) .await; diff --git a/crates/defguard_session_manager/tests/session_manager/sessions.rs b/crates/defguard_session_manager/tests/session_manager/sessions.rs index b18145bb76..5472de9809 100644 --- a/crates/defguard_session_manager/tests/session_manager/sessions.rs +++ b/crates/defguard_session_manager/tests/session_manager/sessions.rs @@ -275,7 +275,7 @@ async fn test_existing_new_session_becomes_connected_on_stats( let mut harness = SessionManagerHarness::new(pool.clone()); let existing_session = - create_session(&pool, location.id, user.id, device.id, None, None, None).await; + create_session(&pool, location.id, user.id, device.id, None, false, None).await; assert_eq!(existing_session.state, VpnClientSessionState::New); let endpoint: SocketAddr = "203.0.113.10:51820".parse().unwrap(); @@ -321,7 +321,7 @@ async fn test_never_connected_posture_new_session_disconnects_after_threshold( user.id, device.id, None, - None, + false, Some("posture-psk-before-timeout"), ) .await; @@ -381,7 +381,7 @@ async fn test_inactive_posture_connected_session_disconnects_and_clears_authoriz user.id, device.id, Some(stale_handshake), - None, + false, Some("posture-psk-before-disconnect"), ) .await; @@ -636,7 +636,7 @@ async fn test_existing_session_in_db_is_reused_instead_of_creating_duplicate( user.id, device.id, Some(base_time - TimeDelta::seconds(5)), - None, + false, None, ) .await; diff --git a/crates/defguard_session_manager/tests/session_manager/stats.rs b/crates/defguard_session_manager/tests/session_manager/stats.rs index f74bf82120..278ebab524 100644 --- a/crates/defguard_session_manager/tests/session_manager/stats.rs +++ b/crates/defguard_session_manager/tests/session_manager/stats.rs @@ -202,7 +202,7 @@ async fn test_out_of_order_updates_for_existing_db_session_are_discarded( user.id, device.id, Some(first_handshake), - None, + false, None, ) .await; diff --git a/crates/model_derive/src/lib.rs b/crates/model_derive/src/lib.rs index 2b2137081c..3d73afa6ee 100644 --- a/crates/model_derive/src/lib.rs +++ b/crates/model_derive/src/lib.rs @@ -14,6 +14,8 @@ enum ModelType { Any, Enum, Ip, + Json, + List, Option, OptionRef, Ref, @@ -32,6 +34,10 @@ impl From<&Ident> for ModelType { Self::Enum } else if value == "ip" { Self::Ip + } else if value == "json" { + Self::Json + } else if value == "list" { + Self::List } else if value == "option" { Self::Option } else if value == "option_ref" { @@ -178,7 +184,9 @@ fn expand(ast: &DeriveInput) -> syn::Result { ModelType::Secret => format!("\"{name}\" \"{name}?: SecretString\""), ModelType::Ip => format!("\"{name}\" \"{name}: IpAddr\""), ModelType::Option | ModelType::OptionRef => format!("\"{name}\" \"{name}?: _\""), - ModelType::Enum | ModelType::Ref => format!("\"{name}\" \"{name}: _\""), + ModelType::Enum | ModelType::Ref | ModelType::List | ModelType::Json => { + format!("\"{name}\" \"{name}: _\"") + } }); query_args.push(match model_type { @@ -202,6 +210,10 @@ fn expand(ast: &DeriveInput) -> syn::Result { ModelType::Secret => quote! { &self.#name as &Option }, // FIXME: hard-coded struct name ModelType::Ip => quote! { &self.#name as &IpAddr }, + ModelType::List | ModelType::Json => { + let ty = &field.ty; + quote! { &self.#name as &#ty } + } ModelType::Ref => quote! { &self.#name }, }); struct_fields.push(quote! { #name: self.#name }); diff --git a/crates/model_derive/src/tests.rs b/crates/model_derive/src/tests.rs index e9fef691ae..5bc48e85a8 100644 --- a/crates/model_derive/src/tests.rs +++ b/crates/model_derive/src/tests.rs @@ -120,6 +120,7 @@ fn model_attr_parses_every_supported_property() { let cases = [ ("enum", ModelType::Enum), ("ip", ModelType::Ip), + ("json", ModelType::Json), ("option", ModelType::Option), ("option_ref", ModelType::OptionRef), ("ref", ModelType::Ref), @@ -307,6 +308,16 @@ fn each_model_type_produces_its_own_column_alias() { ); } +#[test] +fn json_type_emits_wildcard_select_alias() { + let queries = queries("struct T { id: Id, #[model(json)] data: Json }"); + assert_eq!(queries[ALL], "SELECT id, \"data\" \"data: _\" FROM \"t\""); + assert_eq!( + queries[INSERT], + "INSERT INTO \"t\" (\"data\") VALUES ($1) RETURNING id" + ); +} + #[test] fn derived_queries_share_the_select_prefix() { let queries = queries("struct T { id: Id, a: A }"); @@ -394,6 +405,14 @@ fn bind_args_cast_according_to_model_type() { bind_arg("#[model(ip)] value: IpAddr"), quote!(&self.value as &IpAddr).to_string() ); + assert_eq!( + bind_arg("#[model(list)] value: Vec"), + quote!(&self.value as &Vec).to_string() + ); + assert_eq!( + bind_arg("#[model(json)] value: Json"), + quote!(&self.value as &Json).to_string() + ); assert_eq!( bind_arg("#[model(secret)] value: Option"), quote!(&self.value as &Option).to_string() diff --git a/flake.lock b/flake.lock index 164ceb9b56..9e4a0f79b5 100644 --- a/flake.lock +++ b/flake.lock @@ -32,11 +32,11 @@ }, "nixpkgs": { "locked": { - "lastModified": 1786514691, - "narHash": "sha256-9dkt1i5JNbEfPb+EjLcSDNs8zqEHQ/1JlSaKgj0SBAg=", + "lastModified": 1787001381, + "narHash": "sha256-Ue1Yo8gfHdD4TMtNewhA4tkSYeFqXThju0nCyJc3ALo=", "owner": "NixOS", "repo": "nixpkgs", - "rev": "867dcbc30bafe3c862ef88620f2e7a109d7d3be5", + "rev": "ec2d622de0773551768cf98f3fc50cbcc003b9c5", "type": "github" }, "original": { @@ -74,11 +74,11 @@ ] }, "locked": { - "lastModified": 1786594607, - "narHash": "sha256-5f9qi0HFGX07xuLG4X7MVsXb6imb1pEhEhlYB+ts4Xk=", + "lastModified": 1787022047, + "narHash": "sha256-wNlYM/obKOkkRELrOfto1JT5CRANR6hB/RjOELp223U=", "owner": "oxalica", "repo": "rust-overlay", - "rev": "cb8f92ed710cb8402805a509d4083418b5435edb", + "rev": "b32685dd7c5a965aa8273adb7ddaf7f5b40d0faa", "type": "github" }, "original": { diff --git a/migrations/20260814093434_[2.2.0]_mfa_session_store.down.sql b/migrations/20260814093434_[2.2.0]_mfa_session_store.down.sql new file mode 100644 index 0000000000..3ed5e447ef --- /dev/null +++ b/migrations/20260814093434_[2.2.0]_mfa_session_store.down.sql @@ -0,0 +1,8 @@ +-- Drop the durable in-progress MFA session table. +DROP TABLE IF EXISTS vpn_client_mfa_session; + +-- Recreate the legacy mfa_method column, left NULL for every row (lossy by construction: +-- which method was used is recorded in the activity log, not recoverable from a boolean). +ALTER TABLE vpn_client_session + ADD COLUMN mfa_method vpn_client_mfa_method NULL, + DROP COLUMN is_mfa_session; diff --git a/migrations/20260814093434_[2.2.0]_mfa_session_store.up.sql b/migrations/20260814093434_[2.2.0]_mfa_session_store.up.sql new file mode 100644 index 0000000000..0eb3bc374d --- /dev/null +++ b/migrations/20260814093434_[2.2.0]_mfa_session_store.up.sql @@ -0,0 +1,27 @@ +-- Authorized session records ONLY whether it was MFA-gated. The methods used and the +-- governing flow live in the immutable authorization activity-log entry, not on this row. +ALTER TABLE vpn_client_session ADD COLUMN is_mfa_session boolean NOT NULL DEFAULT false; +UPDATE vpn_client_session SET is_mfa_session = true WHERE mfa_method IS NOT NULL; +ALTER TABLE vpn_client_session DROP COLUMN mfa_method; + +-- Durable in-progress MFA session. Token is OPAQUE (random); only its hash is stored. +-- All per-step ephemeral state lives in `ephemeral_state` (JSONB), cleared to NULL on advance. +CREATE TABLE vpn_client_mfa_session ( + id bigserial PRIMARY KEY, + token_hash text NOT NULL UNIQUE, -- base64url-nopad SHA-256 of the opaque token; raw token never stored + location_id bigint NOT NULL REFERENCES wireguard_network(id) ON DELETE CASCADE, + device_id bigint NOT NULL REFERENCES device(id) ON DELETE CASCADE, + user_id bigint NOT NULL REFERENCES "user"(id) ON DELETE CASCADE, -- denormalized; NOT part of the key + steps_snapshot jsonb NOT NULL, -- {"flow_id": , "steps": [{"methods": [...], "satisfied": |null}, ...]} + current_step integer NOT NULL DEFAULT 0, + ephemeral_state jsonb NULL, -- per-step attempt state; cleared on advance + failed_attempts integer NOT NULL DEFAULT 0, + created_at timestamp without time zone NOT NULL DEFAULT (now() AT TIME ZONE 'UTC'), + expires_at timestamp without time zone NOT NULL +); + +-- The (location_id, device_id) identity is enforced by construction: a concurrent double-Start +-- cannot leave two live rows, because `start` supersedes via a DELETE plus an ON CONFLICT upsert +-- in a single transaction, which also writes the first attempt's ephemeral_state. +CREATE UNIQUE INDEX vpn_client_mfa_session_location_device_unique + ON vpn_client_mfa_session (location_id, device_id); diff --git a/tools/defguard_generator/src/activity_log.rs b/tools/defguard_generator/src/activity_log.rs index 2b653f9322..09ee395961 100644 --- a/tools/defguard_generator/src/activity_log.rs +++ b/tools/defguard_generator/src/activity_log.rs @@ -3,7 +3,10 @@ use chrono::{Duration, Utc}; use defguard_common::db::{ Id, NoId, models::{ - Device, DeviceType, MFAMethod, Settings, User, WebAuthn, WireguardNetwork, group::Group, + Device, DeviceType, MFAMethod, Settings, User, WebAuthn, WireguardNetwork, + group::Group, + vpn_client_mfa_session::{MfaAttribution, Step, StepsSnapshot}, + vpn_client_session::VpnClientMfaMethod, }, }; use defguard_core::{ @@ -1049,6 +1052,14 @@ fn build_vpn_event( ), EventType::VpnClientMfaSuccess => { let method = random_client_mfa_method(rng); + let satisfied: VpnClientMfaMethod = method.into(); + let snapshot = StepsSnapshot { + flow_id: 1, + steps: vec![Step { + methods: vec![satisfied], + satisfied: Some(satisfied), + }], + }; ( get_vpn_event_description(&VpnEvent::ClientMfaSuccess { location: location.clone(), @@ -1058,7 +1069,10 @@ fn build_vpn_event( serde_json::to_value(VpnClientMfaMetadata { location, device, - method, + attribution: MfaAttribution { + snapshot, + flow_name: Some("Default Internal MFA".to_owned()), + }, mobile_auth_device_name: None, }) .ok(), diff --git a/tools/defguard_generator/src/vpn_session_stats.rs b/tools/defguard_generator/src/vpn_session_stats.rs index ba5bdaae73..557206ae54 100644 --- a/tools/defguard_generator/src/vpn_session_stats.rs +++ b/tools/defguard_generator/src/vpn_session_stats.rs @@ -159,7 +159,7 @@ async fn generate_stats_for_location( device.user_id, device.id, Some(session_start), - None, + false, ); // mark all but the first session as disconnected diff --git a/web/messages/en/activity.json b/web/messages/en/activity.json index b4c87c55dd..68aa35d471 100644 --- a/web/messages/en/activity.json +++ b/web/messages/en/activity.json @@ -36,6 +36,7 @@ "activity_event_vpn_client_mfa_failed": "VPN client MFA failed", "activity_event_vpn_client_session_superseded": "VPN client session superseded", "activity_event_vpn_client_mfa_session_superseded": "VPN client MFA session superseded", + "activity_event_vpn_client_mfa_login_superseded": "VPN client MFA login superseded", "activity_event_enrollment_token_added": "Enrollment token added", "activity_event_enrollment_started": "Enrollment started", "activity_event_enrollment_device_added": "Enrollment device added", diff --git a/web/src/shared/api/activity-log-types.ts b/web/src/shared/api/activity-log-types.ts index e1ed9f5835..7da43259c7 100644 --- a/web/src/shared/api/activity-log-types.ts +++ b/web/src/shared/api/activity-log-types.ts @@ -57,6 +57,7 @@ export const ActivityLogEventType = { VpnClientMfaFailed: 'vpn_client_mfa_failed', VpnClientSessionSuperseded: 'vpn_client_session_superseded', VpnClientMfaSessionSuperseded: 'vpn_client_mfa_session_superseded', + VpnClientMfaLoginSuperseded: 'vpn_client_mfa_login_superseded', EnrollmentTokenAdded: 'enrollment_token_added', EnrollmentStarted: 'enrollment_started',