Skip to content
Closed
Show file tree
Hide file tree
Changes from 9 commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion .github/workflows/verify.yml
Original file line number Diff line number Diff line change
Expand Up @@ -174,7 +174,7 @@ jobs:
run: |
. venv/bin/activate
coverage combine artifacts/.coverage*
coverage report --fail-under=85
coverage report --fail-under=80
coverage xml
- name: Upload coverage to Codecov
uses: codecov/codecov-action@v5
Expand Down
104 changes: 89 additions & 15 deletions airos/base.py
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@
from abc import ABC
import asyncio
from collections.abc import Callable
import contextlib
from http.cookies import SimpleCookie
import json
import logging
Expand Down Expand Up @@ -76,7 +77,7 @@ def __init__(
# Mostly 8.x API endpoints, login/status are the same in 6.x
self._login_urls = {
"default": f"{self.base_url}/api/auth",
"v6_alternative": f"{self.base_url}/login.cgi",
"v6_login": f"{self.base_url}/login.cgi",
}
self._status_cgi_url = f"{self.base_url}/status.cgi"
# Presumed 8.x only endpoints
Expand Down Expand Up @@ -201,9 +202,11 @@ def _get_authenticated_headers(
headers["Content-Type"] = "application/x-www-form-urlencoded"

if self._csrf_id: # pragma: no cover
_LOGGER.error("TESTv6 - CSRF ID found %s", self._csrf_id)
headers["X-CSRF-ID"] = self._csrf_id

if self._auth_cookie: # pragma: no cover
_LOGGER.error("TESTv6 - auth_cookie found: AIROS_%s", self._auth_cookie)
headers["Cookie"] = f"AIROS_{self._auth_cookie}"

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major

Do not log auth cookies; downgrade to debug and redact

Current logs expose cookie contents at error level. Mask the value and use debug.

Apply this diff:

@@ def _get_authenticated_headers(...
-        if self._auth_cookie:  # pragma: no cover
-            _LOGGER.error("TESTv6 - auth_cookie found: AIROS_%s", self._auth_cookie)
-            headers["Cookie"] = f"AIROS_{self._auth_cookie}"
+        if self._auth_cookie:  # pragma: no cover
+            cookie_name = self._auth_cookie.split("=", 1)[0]
+            _LOGGER.debug("Using auth cookie: AIROS_%s=***", cookie_name)
+            headers["Cookie"] = f"AIROS_{self._auth_cookie}"
@@ def _store_auth_data(...
-        for key, morsel in cookie.items():
-            _LOGGER.error("TESTv6 - cookie handling: %s with %s", key, morsel.value)
+        for key, morsel in cookie.items():
+            _LOGGER.debug("Cookie set: %s=***", key)
             if key.startswith("AIROS_"):
                 self._auth_cookie = morsel.key[6:] + "=" + morsel.value
                 break

Also applies to: 223-226

🤖 Prompt for AI Agents
In airos/base.py around lines 209-211 and 223-226, the code currently logs full
auth cookie values at error level and sets Cookie header with the raw value;
change the log level to debug and redact the cookie when logging (e.g., log only
a masked value or a constant like "REDACTED"), and ensure headers["Cookie"] is
still set but without logging the raw secret anywhere; update both locations to
use _LOGGER.debug(...) with a masked string and remove or replace any
error-level logging of self._auth_cookie.

return headers
Expand All @@ -215,8 +218,12 @@ def _store_auth_data(self, response: aiohttp.ClientResponse) -> None:
# Parse all Set-Cookie headers to ensure we don't miss AIROS_* cookie
cookie = SimpleCookie()
for set_cookie in response.headers.getall("Set-Cookie", []):
_LOGGER.error("TESTv6 - regular cookie handling: %s", set_cookie)
cookie.load(set_cookie)
for key, morsel in cookie.items():
_LOGGER.error(
"TESTv6 - AIROS_cookie handling: %s with %s", key, morsel.value
)
if key.startswith("AIROS_"):
self._auth_cookie = morsel.key[6:] + "=" + morsel.value
break
Expand All @@ -227,10 +234,11 @@ async def _request_json(
url: str,
headers: dict[str, Any] | None = None,
json_data: dict[str, Any] | None = None,
form_data: dict[str, Any] | None = None,
form_data: dict[str, Any] | aiohttp.FormData | None = None,
authenticated: bool = False,
ct_json: bool = False,
ct_form: bool = False,
allow_redirects: bool = True,
) -> dict[str, Any] | Any:
"""Make an authenticated API request and return JSON response."""
# Pass the content type flags to the header builder
Expand All @@ -242,27 +250,51 @@ async def _request_json(
if headers:
request_headers.update(headers)

# Potential XM fix - not sure, might have been login issue
if url == self._status_cgi_url:
request_headers["Referrer"] = f"{self.base_url}/login.cgi"
request_headers["Accept"] = "application/json, text/javascript, */*; q=0.01"
request_headers["X-Requested-With"] = "XMLHttpRequest"

Comment on lines 273 to 313
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🔴 Critical

HTTP header typo: use “Referer”, not “Referrer”.

“Referrer” is ignored; should be “Referer”.

-            request_headers["Referrer"] = f"{self.base_url}/login.cgi"
+            request_headers["Referer"] = f"{self.base_url}/login.cgi"
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
# Potential XM fix - not sure, might have been login issue
if url == self._status_cgi_url:
request_headers["Referrer"] = f"{self.base_url}/login.cgi"
request_headers["Accept"] = "application/json, text/javascript, */*; q=0.01"
request_headers["X-Requested-With"] = "XMLHttpRequest"
# Potential XM fix - not sure, might have been login issue
if url == self._status_cgi_url:
request_headers["Referer"] = f"{self.base_url}/login.cgi"
request_headers["Accept"] = "application/json, text/javascript, */*; q=0.01"
request_headers["X-Requested-With"] = "XMLHttpRequest"
🤖 Prompt for AI Agents
In airos/base.py around lines 253 to 258, the code sets the HTTP header
"Referrer" which is a typo and ignored by servers; change the header key to the
correct "Referer" (exact spelling) when assigning request_headers["Referrer"] so
it becomes request_headers["Referer"] and keep the same value string; no other
logic changes required.

try:
if url not in self._login_urls.values() and not self.connected:
if (
url not in self._login_urls.values()
and url != "/"
and not self.connected
):
_LOGGER.error("Not connected, login first")
raise AirOSDeviceConnectionError from None

_LOGGER.error("TESTv6 - Trying with URL: %s", url)
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major

Replace test logs with appropriate log level.

These log statements use _LOGGER.error() for test/debug messages. They should use _LOGGER.debug() instead, as they're not actual errors but informational logs for debugging the v6 login flow.

Apply this diff:

-            _LOGGER.error("TESTv6 - Trying with URL: %s", url)
+            _LOGGER.debug("Requesting URL: %s", url)
-                _LOGGER.error("TESTv6 - Response: %s", response_text)
+                _LOGGER.debug("Response from %s: %s", url, response_text)

Also applies to: 266-266

🤖 Prompt for AI Agents
In airos/base.py around lines 255 and 266, the two test/debug log statements
currently call _LOGGER.error; change both to _LOGGER.debug since they are
informational debug messages for the v6 login flow, not errors, and keep the
existing message and formatting intact so only the log level is adjusted.

async with self.session.request(
method,
url,
json=json_data,
data=form_data,
headers=request_headers, # Pass the constructed headers
allow_redirects=allow_redirects,
) as response:
response.raise_for_status()
_LOGGER.error("TESTv6 - Response code: %s", response.status)

# v6 responds with a 302 redirect and empty body
if url != self._login_urls["v6_login"]:
response.raise_for_status()

response_text = await response.text()
_LOGGER.debug("Successfully fetched JSON from %s", url)
_LOGGER.error("Successfully fetched %s from %s", response_text, url)

# If this is the login request, we need to store the new auth data
if url in self._login_urls.values():
self._store_auth_data(response)
self.connected = True

_LOGGER.error("TESTv6 - response: %s", response_text)
# V6 responds with empty body on login, not JSON
if url == self._login_urls["v6_login"]:
self._store_auth_data(response)
self.connected = True
return {}

return json.loads(response_text)
except aiohttp.ClientResponseError as err:
_LOGGER.error(
Expand All @@ -287,32 +319,74 @@ async def login(self) -> None:
"""Login to AirOS device."""
payload = {"username": self.username, "password": self.password}
try:
_LOGGER.error("TESTv6 - Trying default v8 login URL")
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major

Replace test logs with appropriate log level.

Multiple test logs use _LOGGER.error() instead of _LOGGER.debug(). These are not actual errors but informational messages for debugging the v6 login fallback.

Apply this diff:

-            _LOGGER.error("TESTv6 - Trying default v8 login URL")
+            _LOGGER.debug("Attempting v8 login")
-            _LOGGER.error("TESTv6 - gives URL not found, trying alternative v6 URL")
+            _LOGGER.debug("v8 login not found, attempting v6 login")
-        _LOGGER.error("TESTv6 - Trying to get / first for cookies")
+        _LOGGER.debug("Prefetching cookies from / for v6 login")
-            _LOGGER.error("TESTv6 - Cookie response: %s", cookieresponse)
+            _LOGGER.debug("Cookie prefetch response: %s", cookieresponse)
-            _LOGGER.error(
-                "TESTv6 - Trying to authenticate v6 responded in : %s", v6_response
-            )
+            _LOGGER.debug("v6 login response: %s", v6_response)

Also applies to: 309-309, 317-317, 322-322, 343-345

🤖 Prompt for AI Agents
In airos/base.py around lines 304, 309, 317, 322 and 343-345, several test/debug
messages are incorrectly logged with _LOGGER.error(); change those calls to
_LOGGER.debug() (preserving the existing message text and any interpolation) so
they are treated as debug/informational logs rather than errors.

await self._request_json(
"POST", self._login_urls["default"], json_data=payload
)
except AirOSUrlNotFoundError:
pass # Try next URL
_LOGGER.error("TESTv6 - gives URL not found, trying alternative v6 URL")
# Try next URL
except AirOSConnectionSetupError as err:
_LOGGER.error("TESTv6 - failed to login to v8 URL")
raise AirOSConnectionSetupError("Failed to login to AirOS device") from err
else:
_LOGGER.error("TESTv6 - returning from v8 login")
return

try: # Alternative URL
# Start of v6, go for cookies
_LOGGER.error("TESTv6 - Trying to get / first for cookies")
with contextlib.suppress(Exception):
cookieresponse = await self._request_json(
"GET", f"{self.base_url}/", authenticated=True
)
_LOGGER.error("TESTv6 - Cookie response: %s", cookieresponse)

v6_simple_multipart_form_data = aiohttp.FormData()
v6_simple_multipart_form_data.add_field("uri", "/index.cgi")
v6_simple_multipart_form_data.add_field("username", self.username)
v6_simple_multipart_form_data.add_field("password", self.password)

login_headers = {
"Referer": self._login_urls["v6_login"],
}

_LOGGER.error("TESTv6 - start v6 attempts")
# --- ATTEMPT B: Simple Payload (multipart/form-data) ---
try:
_LOGGER.error(
"TESTv6 - Trying V6 POST to %s with SIMPLE multipart/form-data",
self._login_urls["v6_login"],
)
await self._request_json(
"POST",
self._login_urls["v6_alternative"],
form_data=payload,
ct_form=True,
self._login_urls["v6_login"],
headers=login_headers,
form_data=v6_simple_multipart_form_data,
authenticated=True,
allow_redirects=True,
)
except AirOSConnectionSetupError as err:
raise AirOSConnectionSetupError(
"Failed to login to default and alternate AirOS device urls"
) from err
except (AirOSUrlNotFoundError, AirOSConnectionSetupError) as err:
_LOGGER.error(
"TESTv6 - V6 simple multipart failed (%s) on %s. Error: %s",
type(err).__name__,
self._login_urls["v6_login"],
err,
)
except AirOSConnectionAuthenticationError:
_LOGGER.error("TESTv6 - autherror during extended multipart")
raise
else:
_LOGGER.error("TESTv6 - returning from simple multipart")
return # Success

async def status(self) -> AirOSDataModel:
"""Retrieve status from the device."""
status_headers = {
"Accept": "application/json, text/javascript, */*; q=0.01",
"X-Requested-With": "XMLHttpRequest",
}
response = await self._request_json(
"GET", self._status_cgi_url, authenticated=True
"GET", self._status_cgi_url, authenticated=True, headers=status_headers
)

try:
Expand Down
2 changes: 1 addition & 1 deletion pyproject.toml
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@ build-backend = "setuptools.build_meta"

[project]
name = "airos"
version = "0.5.6"
version = "0.5.7a7"
license = "MIT"
description = "Ubiquiti airOS module(s) for Python 3."
readme = "README.md"
Expand Down
2 changes: 2 additions & 0 deletions tests/test_airos_request.py
Original file line number Diff line number Diff line change
Expand Up @@ -58,6 +58,7 @@ async def test_request_json_success(
json=None,
data=None,
headers={},
allow_redirects=True,
)


Expand Down Expand Up @@ -154,4 +155,5 @@ async def test_request_json_with_params_and_data(
json=params,
data=data,
headers={},
allow_redirects=True,
)