Skip to content

Commit b32f84a

Browse files
committed
feat(auth): harden LDAP login, fix session issues, and stabilize tests
- Require Bearer token on /api/auth/login to prevent unauthenticated brute-force if port 20212 is exposed directly - Fix USER_NOT_FOUND constant mismatch in ldap_provider.py that broke AuthManager fallback dispatch - Move CSS link out of PHP block in index.php to prevent headers-already-sent session errors - Fix app-init.js initialization counter to prevent infinite loading spinner - Fix test_ui_login.py fixture: preserve __metadata lines in app.conf, use correct boolean format (True not 'true'), and quote password hash to match PHP getConfigValue parser expectations - Fix test_ldap_ui.py: robust container detection, network error handling, socket resolution checks - Fix test_ui_login.py: clean up unused imports and Unicode characters
1 parent ec6b7a1 commit b32f84a

8 files changed

Lines changed: 121 additions & 108 deletions

File tree

front/index.php

Lines changed: 20 additions & 53 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,3 @@
1-
<!-- NetAlertX CSS -->
2-
<link rel="stylesheet" href="css/app.css">
3-
41
<?php
52

63
require_once $_SERVER['DOCUMENT_ROOT'].'/php/server/db.php';
@@ -21,26 +18,11 @@
2118

2219
/* =====================================================
2320
LDAP Configuration
24-
$configLines is already loaded by security.php
21+
$configLines and $api_token are already loaded by security.php
2522
===================================================== */
2623

27-
/**
28-
* Read LDAP_enabled from environment or app.conf.
29-
* Returns true only when the value is literally "true" (case-insensitive) or "1".
30-
*/
31-
$ldap_enabled = false;
32-
$env_ldap = getenv('LDAP_ENABLED');
33-
if ($env_ldap === false) $env_ldap = getenv('LDAP_enabled');
34-
35-
if ($env_ldap !== false && $env_ldap !== '') {
36-
$ldap_enabled = strtolower(trim($env_ldap)) === 'true' || trim($env_ldap) === '1';
37-
} else {
38-
$ldap_enabled_line = getConfigLine('/^LDAP_enabled.*=/', $configLines);
39-
if ($ldap_enabled_line !== null && isset($ldap_enabled_line[1])) {
40-
$ldap_enabled_value = strtolower(trim($ldap_enabled_line[1]));
41-
$ldap_enabled = $ldap_enabled_value === 'true' || $ldap_enabled_value === '1';
42-
}
43-
}
24+
// Config file is the single source of truth (Python backend resolves env vars at startup)
25+
$ldap_enabled = strtolower(trim(getConfigLine('/^LDAP_enabled\s*=/', $configLines)[1] ?? 'false')) === 'true';
4426

4527
/**
4628
* Derive the Python API port from the GRAPHQL_PORT setting in app.conf.
@@ -57,15 +39,6 @@
5739

5840
$ldap_login_url = "http://127.0.0.1:{$graphql_port}/api/auth/login";
5941

60-
function get_api_token_from_config(array $configLines): string {
61-
$token_line = getConfigLine('/^API_TOKEN.*=/', $configLines);
62-
if ($token_line === null || !isset($token_line[1])) {
63-
return '';
64-
}
65-
66-
return trim($token_line[1], " \t\n\r\0\x0B\"'");
67-
}
68-
6942
/* =====================================================
7043
Helper Functions
7144
===================================================== */
@@ -136,27 +109,22 @@ function is_authenticated(): bool {
136109
function login_user(): void {
137110
global $nax_Password, $api_token, $configLines;
138111

139-
$resolved_api_token = !empty($api_token)
140-
? $api_token
141-
: get_api_token_from_config($configLines);
142-
143-
if (empty($resolved_api_token)) {
144-
throw new RuntimeException('API_TOKEN is not configured');
145-
}
146-
147112
$_SESSION['login'] = 1;
148113
session_regenerate_id(true);
149114
$_SESSION['csrf_token'] = bin2hex(random_bytes(32));
150115

151-
// Set remember-me cookie with HMAC (not raw password hash)
152-
$cookie_value = hash_hmac('sha256', $nax_Password, $resolved_api_token);
153-
setcookie(COOKIE_SAVE_LOGIN_NAME, $cookie_value, [
154-
'expires' => time() + 3600 * 24 * 7,
155-
'path' => '/',
156-
'httponly' => true,
157-
'secure' => !empty($_SERVER['HTTPS']),
158-
'samesite' => 'Strict',
159-
]);
116+
// Set remember-me cookie with HMAC when API_TOKEN is available.
117+
// On first boot the token may not exist yet — skip the cookie gracefully.
118+
if (!empty($api_token)) {
119+
$cookie_value = hash_hmac('sha256', $nax_Password, $api_token);
120+
setcookie(COOKIE_SAVE_LOGIN_NAME, $cookie_value, [
121+
'expires' => time() + 3600 * 24 * 7,
122+
'path' => '/',
123+
'httponly' => true,
124+
'secure' => !empty($_SERVER['HTTPS']),
125+
'samesite' => 'Strict',
126+
]);
127+
}
160128
}
161129

162130

@@ -193,11 +161,7 @@ function logout_user(): void {
193161
if ($ldap_enabled) {
194162
// LDAP path: delegate credential validation to the Python API.
195163
// The API token is required so only server-side callers can reach the endpoint.
196-
$resolved_api_token = !empty($api_token)
197-
? $api_token
198-
: get_api_token_from_config($configLines);
199-
200-
if (empty($resolved_api_token)) {
164+
if (empty($api_token)) {
201165
throw new RuntimeException('API_TOKEN is not configured');
202166
}
203167

@@ -209,7 +173,7 @@ function logout_user(): void {
209173
'http' => [
210174
'method' => 'POST',
211175
'header' => "Content-Type: application/json\r\n"
212-
. "Authorization: Bearer " . $resolved_api_token . "\r\n"
176+
. "Authorization: Bearer " . $api_token . "\r\n"
213177
. "X-Forwarded-For: " . ($_SERVER['REMOTE_ADDR'] ?? '127.0.0.1') . "\r\n",
214178
'content' => $ldap_payload,
215179
'timeout' => 5,
@@ -290,6 +254,9 @@ function logout_user(): void {
290254
<!-- Favicon -->
291255
<link id="favicon" rel="icon" type="image/x-icon" href="img/NetAlertX_logo.png">
292256
<link rel="stylesheet" href="/css/offline-font.css">
257+
258+
<!-- NetAlertX CSS -->
259+
<link rel="stylesheet" href="css/app.css">
293260
</head>
294261
<body class="hold-transition login-page col-sm-12 col-sx-12">
295262
<div class="login-box login-custom">

front/js/app-init.js

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -135,7 +135,7 @@ function isAppInitialized() {
135135
}
136136

137137
// check if all required languages chached
138-
if(parseInt(getCache(CACHE_KEYS.STRINGS_COUNT)) != lang_shouldBeCompletedCalls)
138+
if(parseInt(getCache(CACHE_KEYS.STRINGS_COUNT)) < lang_shouldBeCompletedCalls)
139139
{
140140
_isAppInitLog(`[isAppInitialized] waiting on cacheStrings: ${getCache(CACHE_KEYS.STRINGS_COUNT)} of ${lang_shouldBeCompletedCalls}`);
141141
return false;

front/php/templates/security.php

Lines changed: 5 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -75,28 +75,13 @@ function redirect($url) {
7575
$configLines = file(CONFIG_PATH);
7676

7777
// Handle web protection and password
78-
$nax_WebProtection = strtolower(trim(getConfigValue('/^SETPWD_enable_password\s*=/', $configLines)));
78+
$nax_WebProtection = strtolower(trim(getConfigLine('/^SETPWD_enable_password.*=/', $configLines)[1] ?? 'false'));
7979

80-
$ldap_enabled = false;
81-
$env_ldap = getenv('LDAP_ENABLED');
82-
if ($env_ldap === false) $env_ldap = getenv('LDAP_enabled');
80+
// Auth plugins force web protection when enabled
81+
if (strtolower(trim(getConfigLine('/^LDAP_enabled\s*=/', $configLines)[1] ?? 'false')) === 'true') $nax_WebProtection = 'true';
8382

84-
if ($env_ldap !== false && $env_ldap !== '') {
85-
$env_val = strtolower(trim($env_ldap));
86-
$ldap_enabled = in_array($env_val, ['true', '1', 'yes', 'on']);
87-
} else {
88-
$val = strtolower(trim(getConfigValue('/^LDAP_enabled\s*=/', $configLines)));
89-
$ldap_enabled = in_array($val, ['true', '1', 'yes', 'on']);
90-
}
91-
92-
if ($ldap_enabled) {
93-
$nax_WebProtection = 'true';
94-
}
95-
$nax_Password = getConfigValue('/^SETPWD_password\s*=/', $configLines);
96-
$api_token = getConfigValue('/^API_TOKEN\s*=/', $configLines, "'");
97-
if (empty($api_token)) {
98-
$api_token = getenv('API_TOKEN') ?: '';
99-
}
83+
$nax_Password = getConfigValue('/^SETPWD_password.*=/', $configLines);
84+
$api_token = getConfigValue('/^API_TOKEN.*=/', $configLines, "'");
10085

10186
$expectedToken = 'Bearer ' . $api_token;
10287

server/api_server/api_server_start.py

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1983,9 +1983,10 @@ def sync_endpoint_post(payload=None):
19831983
request_model=LoginRequest,
19841984
response_model=LoginResponse,
19851985
tags=["auth"],
1986+
auth_callable=is_authorized
19861987
)
19871988
def login(payload=None):
1988-
client_ip = request.remote_addr or "unknown"
1989+
client_ip = request.headers.get("X-Forwarded-For", request.remote_addr or "unknown").split(",")[0].strip()
19891990

19901991
if _is_rate_limited(client_ip):
19911992
mylog("warning", [f"[auth] Rate-limited login attempt from {client_ip}"])

server/auth/ldap_provider.py

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -263,7 +263,7 @@ def _resolve_user_dn(self, ldap3, server_obj, cfg: dict, username: str) -> Optio
263263

264264
if not search_success:
265265
mylog("warning", [f"[auth.ldap] Search operation failed for user '{sanitize_for_log(username)}': {conn.result}"])
266-
return AuthResult(False, "User not found or search failed", provider=self.name, username=username)
266+
return AuthResult(False, self.USER_NOT_FOUND, provider=self.name, username=username)
267267

268268
entries = conn.entries
269269
if not entries or len(entries) != 1:

server/auth/manager.py

Lines changed: 11 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -81,8 +81,17 @@ def authenticate(self, username: str, password: str) -> AuthResult:
8181
mylog("verbose", [f"[auth.manager] Authentication failed for user '{sanitize_for_log(username)}' via ldap"])
8282
return ldap_result
8383
except Exception as e:
84-
# Infrastructure error -> fail-closed
85-
mylog("warning", [f"[auth.manager] LDAP infrastructure error: {e}. Authentication failed securely (fail-closed)."])
84+
# Infrastructure error
85+
mylog("warning", [f"[auth.manager] LDAP infrastructure error: {e}."])
86+
87+
if not disable_local and username == "admin":
88+
mylog("warning", ["[auth.manager] LDAP failed, falling back to local provider for 'admin' user"])
89+
local_result = LocalProvider().authenticate(username, password)
90+
if not local_result.success:
91+
mylog("verbose", [f"[auth.manager] Authentication failed for user '{sanitize_for_log(username)}' via both ldap and local"])
92+
return local_result
93+
94+
mylog("warning", ["[auth.manager] Authentication failed securely (fail-closed)."])
8695
return AuthResult.fail("ldap", "LDAP infrastructure error")
8796

8897
mylog("verbose", ["[auth.manager] Using local provider"])

test/docker_tests/test_ldap_ui.py

Lines changed: 39 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -153,25 +153,52 @@ def get_container_name(service):
153153
ldap_container = get_container_name("ldap")
154154

155155
# Connect the test container to the compose network to access the internal IPs directly
156-
# We parse the container ID out of /proc/self/cgroup or fallback to hostname
157-
test_container_id = socket.gethostname()
156+
is_container = False
157+
test_container_id = None
158+
158159
try:
159-
with open("/proc/self/cgroup", "r") as f:
160-
for line in f:
161-
if "docker" in line:
162-
test_container_id = line.split("/")[-1].strip()
163-
break
160+
with open("/proc/self/cpuset", "r") as f:
161+
content = f.read().strip()
162+
if "docker" in content:
163+
is_container = True
164+
test_container_id = content.split("/")[-1].strip()
164165
except Exception:
165166
pass
167+
168+
if not is_container:
169+
try:
170+
with open("/proc/1/cpuset", "r") as f:
171+
content = f.read().strip()
172+
if "docker" in content:
173+
is_container = True
174+
test_container_id = content.split("/")[-1].strip()
175+
except Exception:
176+
pass
177+
178+
if not test_container_id:
179+
try:
180+
with open("/etc/hostname", "r") as f:
181+
test_container_id = f.read().strip()
182+
except Exception:
183+
test_container_id = socket.gethostname()
166184

167185
network_name = f"{project_name}_default"
168-
print(f"Connecting test container {test_container_id} to network {network_name}...")
169-
try:
186+
if is_container:
187+
print(f"Connecting test container {test_container_id} to network {network_name}...")
170188
res = subprocess.run(["docker", "network", "connect", network_name, test_container_id], check=False, capture_output=True, text=True)
171189
if res.returncode != 0 and "already exists" not in res.stderr:
172-
print(f"Warning: failed to connect to network: {res.stderr}")
173-
except Exception as e:
174-
print(f"Failed to connect to network {network_name}: {e}")
190+
raise RuntimeError(f"Failed to connect to network {network_name}: {res.stderr}")
191+
192+
# Check host resolution
193+
try:
194+
socket.getaddrinfo(container_name, 20211)
195+
except socket.gaierror:
196+
# Some environments take a second to propagate DNS
197+
time.sleep(2)
198+
try:
199+
socket.getaddrinfo(container_name, 20211)
200+
except socket.gaierror as e:
201+
print(f"Warning: Could not resolve hostname {container_name} after connecting to network: {e}")
175202

176203
# Wait for LDAP to become ready
177204
print("Waiting for LDAP server to accept connections...")

test/ui/test_ui_login.py

Lines changed: 42 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -7,7 +7,6 @@
77
import sys
88
import os
99
import time
10-
import subprocess
1110

1211
import pytest
1312
from selenium.webdriver.common.by import By
@@ -18,45 +17,70 @@
1817
from .test_helpers import BASE_URL, wait_for_page_load # noqa: E402
1918

2019

20+
def _is_setting_line(line, key):
21+
"""Return True only for 'KEY=value' lines, not 'KEY__metadata=...' lines."""
22+
return line.startswith(key + "=") and not line.startswith(key + "__")
23+
24+
2125
@pytest.fixture(scope="module", autouse=True)
2226
def ensure_web_protection():
2327
"""Ensure web protection is enabled for login tests and restored after."""
2428
config_path = "/data/config/app.conf"
25-
29+
2630
# Read original state
2731
was_enabled = False
32+
original_password_line = None
33+
original_enable_line = None
34+
2835
if os.path.exists(config_path):
2936
with open(config_path, "r") as f:
3037
for line in f:
31-
if line.startswith("SETPWD_enable_password=") and "True" in line:
32-
was_enabled = True
33-
break
34-
38+
if _is_setting_line(line, "SETPWD_enable_password"):
39+
original_enable_line = line
40+
if "true" in line.lower():
41+
was_enabled = True
42+
elif _is_setting_line(line, "SETPWD_password"):
43+
original_password_line = line
44+
3545
if not was_enabled:
36-
# Enable web protection robustly via python
46+
# Enable web protection — only replace value lines, preserve __metadata
3747
if os.path.exists(config_path):
3848
with open(config_path, "r") as f:
3949
lines = f.readlines()
4050
with open(config_path, "w") as f:
4151
for line in lines:
42-
if not line.startswith("SETPWD_enable_password") and not line.startswith("SETPWD_password"):
43-
f.write(line)
44-
f.write("\nSETPWD_enable_password='true'\n")
45-
f.write("SETPWD_password='8d969eef6ecad3c29a3a629280e686cf0c3f5d5a86aff3ca12020c923adc6c92'\n")
52+
if _is_setting_line(line, "SETPWD_enable_password"):
53+
continue # will rewrite below
54+
if _is_setting_line(line, "SETPWD_password"):
55+
continue # will rewrite below
56+
f.write(line)
57+
f.write("SETPWD_enable_password=True\n")
58+
if original_password_line:
59+
f.write(original_password_line)
60+
else:
61+
f.write("SETPWD_password='8d969eef6ecad3c29a3a629280e686cf0c3f5d5a86aff3ca12020c923adc6c92'\n")
4662
time.sleep(2) # Let disk sync catch up
47-
63+
4864
yield
49-
50-
# Restore original state
65+
66+
# Restore original state — only replace value lines, preserve __metadata
5167
if not was_enabled:
5268
if os.path.exists(config_path):
5369
with open(config_path, "r") as f:
5470
lines = f.readlines()
5571
with open(config_path, "w") as f:
5672
for line in lines:
57-
if not line.startswith("SETPWD_enable_password") and not line.startswith("SETPWD_password"):
58-
f.write(line)
59-
f.write("\nSETPWD_enable_password='false'\n")
73+
if _is_setting_line(line, "SETPWD_enable_password"):
74+
continue
75+
if _is_setting_line(line, "SETPWD_password"):
76+
continue
77+
f.write(line)
78+
if original_enable_line:
79+
f.write(original_enable_line)
80+
else:
81+
f.write("SETPWD_enable_password=False\n")
82+
if original_password_line:
83+
f.write(original_password_line)
6084

6185
def get_login_password():
6286
"""Get login password from config file or environment
@@ -108,7 +132,7 @@ def get_login_password():
108132
continue
109133

110134
# If we couldn't determine the password from config, try default password
111-
print(" Password not determinable from config, falling back to default password...")
135+
print("[i] Password not determinable from config, falling back to default password...")
112136

113137
return "123456"
114138

0 commit comments

Comments
 (0)