Fix cookie-jar pollution; remove dead QR login code; add tests
The reauth flow ran validate_jsessionid against HA's shared aiohttp
session, then update_cookies({"JSESSIONID": ...}) added the new value
with no domain. aiohttp's cookie jar still held the previous
JSESSIONID pinned to smartthingsfind.samsung.com (set via Set-Cookie
during the previous load), and aiohttp prefers the more-specific
domain match — so the *stale* cookie went out, Samsung returned no
_csrf header, and the user saw "Cookie was rejected by SmartThings
Find" even though their cookie was fine.
Two fixes:
* validate_jsessionid now runs in an isolated aiohttp.ClientSession
with its own jar, so the shared HA jar can't shadow the cookie
under test.
* async_setup_entry clear_domain()s the smartthingsfind.samsung.com
cookies before reseating JSESSIONID with response_url, otherwise
the same shadowing breaks the entry reload that follows a
successful UI reauth.
Also remove the QR-code login code (do_login_stage_one / _two,
gen_qr_code_base64, the legacy URL constants and qrcode/base64/
random/string/re/asyncio/io/timedelta imports) — Samsung migrated
account.samsung.com to a SPA-driven IAM/OAuth2 flow months ago, so
the QR scrape no longer works and nothing in the integration
references those helpers anymore. Drops the qrcode/pillow/requests
manifest requirements.
Tests: a minimal conftest stubs the homeassistant.* imports the
integration uses, and four async tests cover validate_jsessionid
including the regression case where a domain-bound stale cookie
sits in the shared jar.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
This commit is contained in:
parent
7457a8284b
commit
4c26792c37
6 changed files with 287 additions and 227 deletions
158
tests/conftest.py
Normal file
158
tests/conftest.py
Normal file
|
|
@ -0,0 +1,158 @@
|
|||
"""Stub the HA imports the integration touches so utils/config_flow can be
|
||||
imported and exercised without a full Home Assistant install.
|
||||
|
||||
Only the bits used by the code under test are faked — anything new the
|
||||
integration starts touching will need to be added here."""
|
||||
from __future__ import annotations
|
||||
|
||||
import sys
|
||||
import types
|
||||
from pathlib import Path
|
||||
from typing import Any
|
||||
|
||||
REPO_ROOT = Path(__file__).resolve().parent.parent
|
||||
sys.path.insert(0, str(REPO_ROOT))
|
||||
|
||||
|
||||
def _module(name: str) -> types.ModuleType:
|
||||
if name not in sys.modules:
|
||||
sys.modules[name] = types.ModuleType(name)
|
||||
return sys.modules[name]
|
||||
|
||||
|
||||
# homeassistant.core
|
||||
ha = _module("homeassistant")
|
||||
core = _module("homeassistant.core")
|
||||
|
||||
|
||||
class HomeAssistant:
|
||||
def __init__(self) -> None:
|
||||
self.data: dict[str, Any] = {}
|
||||
# Tests assign a real aiohttp.ClientSession here.
|
||||
self.clientsession = None
|
||||
|
||||
|
||||
def callback(func):
|
||||
return func
|
||||
|
||||
|
||||
core.HomeAssistant = HomeAssistant
|
||||
core.callback = callback
|
||||
ha.core = core
|
||||
|
||||
# homeassistant.exceptions
|
||||
exc = _module("homeassistant.exceptions")
|
||||
|
||||
|
||||
class ConfigEntryAuthFailed(Exception):
|
||||
pass
|
||||
|
||||
|
||||
class ConfigEntryNotReady(Exception):
|
||||
pass
|
||||
|
||||
|
||||
exc.ConfigEntryAuthFailed = ConfigEntryAuthFailed
|
||||
exc.ConfigEntryNotReady = ConfigEntryNotReady
|
||||
|
||||
# homeassistant.helpers + submodules
|
||||
helpers = _module("homeassistant.helpers")
|
||||
aiohttp_client = _module("homeassistant.helpers.aiohttp_client")
|
||||
|
||||
|
||||
def async_get_clientsession(hass: HomeAssistant):
|
||||
return hass.clientsession
|
||||
|
||||
|
||||
aiohttp_client.async_get_clientsession = async_get_clientsession
|
||||
helpers.aiohttp_client = aiohttp_client
|
||||
|
||||
entity = _module("homeassistant.helpers.entity")
|
||||
|
||||
|
||||
class DeviceInfo(dict):
|
||||
pass
|
||||
|
||||
|
||||
entity.DeviceInfo = DeviceInfo
|
||||
helpers.entity = entity
|
||||
|
||||
device_registry = _module("homeassistant.helpers.device_registry")
|
||||
|
||||
|
||||
class _FakeDeviceRegistry:
|
||||
def async_get_device(self, identifiers):
|
||||
return None
|
||||
|
||||
|
||||
def _async_get(_hass):
|
||||
return _FakeDeviceRegistry()
|
||||
|
||||
|
||||
device_registry.async_get = _async_get
|
||||
helpers.device_registry = device_registry
|
||||
|
||||
typing_module = _module("homeassistant.helpers.typing")
|
||||
typing_module.ConfigType = dict
|
||||
|
||||
# config_entries
|
||||
config_entries = _module("homeassistant.config_entries")
|
||||
|
||||
|
||||
class ConfigEntry:
|
||||
pass
|
||||
|
||||
|
||||
class ConfigFlow:
|
||||
def __init_subclass__(cls, *, domain=None, **kwargs):
|
||||
super().__init_subclass__(**kwargs)
|
||||
|
||||
|
||||
class OptionsFlow:
|
||||
pass
|
||||
|
||||
|
||||
class OptionsFlowWithConfigEntry(OptionsFlow):
|
||||
def __init__(self, config_entry):
|
||||
self.config_entry = config_entry
|
||||
self.options = dict(getattr(config_entry, "options", {}) or {})
|
||||
|
||||
|
||||
CONN_CLASS_CLOUD_POLL = "cloud_poll"
|
||||
ConfigFlowResult = dict
|
||||
|
||||
config_entries.ConfigEntry = ConfigEntry
|
||||
config_entries.ConfigFlow = ConfigFlow
|
||||
config_entries.OptionsFlow = OptionsFlow
|
||||
config_entries.OptionsFlowWithConfigEntry = OptionsFlowWithConfigEntry
|
||||
config_entries.CONN_CLASS_CLOUD_POLL = CONN_CLASS_CLOUD_POLL
|
||||
config_entries.ConfigFlowResult = ConfigFlowResult
|
||||
|
||||
# const
|
||||
const = _module("homeassistant.const")
|
||||
|
||||
|
||||
class Platform:
|
||||
DEVICE_TRACKER = "device_tracker"
|
||||
BUTTON = "button"
|
||||
SENSOR = "sensor"
|
||||
|
||||
|
||||
const.Platform = Platform
|
||||
|
||||
# update_coordinator
|
||||
update_coord = _module("homeassistant.helpers.update_coordinator")
|
||||
|
||||
|
||||
class DataUpdateCoordinator:
|
||||
def __init__(self, *args, **kwargs):
|
||||
pass
|
||||
|
||||
|
||||
class UpdateFailed(Exception):
|
||||
pass
|
||||
|
||||
|
||||
update_coord.DataUpdateCoordinator = DataUpdateCoordinator
|
||||
update_coord.UpdateFailed = UpdateFailed
|
||||
helpers.update_coordinator = update_coord
|
||||
94
tests/test_validate_jsessionid.py
Normal file
94
tests/test_validate_jsessionid.py
Normal file
|
|
@ -0,0 +1,94 @@
|
|||
"""Tests for validate_jsessionid.
|
||||
|
||||
The interesting case is cookie-jar pollution: HA's shared aiohttp session can
|
||||
already hold a JSESSIONID cookie that was stored from a previous Set-Cookie
|
||||
response (so it carries a real domain attribute). When we then pre-load a new
|
||||
JSESSIONID via update_cookies({"JSESSIONID": ...}) without specifying a
|
||||
response_url, the new cookie gets stored with no domain and aiohttp ships the
|
||||
older, more-specific one — making the new (valid) cookie look invalid to
|
||||
Samsung."""
|
||||
from __future__ import annotations
|
||||
|
||||
import aiohttp
|
||||
import pytest
|
||||
from aioresponses import aioresponses
|
||||
from yarl import URL
|
||||
|
||||
from custom_components.smartthings_find.utils import (
|
||||
URL_GET_CSRF,
|
||||
validate_jsessionid,
|
||||
)
|
||||
|
||||
|
||||
@pytest.fixture
|
||||
async def hass(monkeypatch):
|
||||
from homeassistant.core import HomeAssistant
|
||||
|
||||
h = HomeAssistant()
|
||||
async with aiohttp.ClientSession() as session:
|
||||
h.clientsession = session
|
||||
yield h
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_valid_cookie_returns_true(hass):
|
||||
with aioresponses() as m:
|
||||
m.get(URL_GET_CSRF, status=200, headers={"_csrf": "tok123"})
|
||||
assert await validate_jsessionid(hass, "fresh-cookie") is True
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_no_csrf_header_returns_false(hass):
|
||||
with aioresponses() as m:
|
||||
m.get(URL_GET_CSRF, status=200, body="fail")
|
||||
assert await validate_jsessionid(hass, "expired-cookie") is False
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_non_200_returns_false(hass):
|
||||
with aioresponses() as m:
|
||||
m.get(URL_GET_CSRF, status=401, body="Logout")
|
||||
assert await validate_jsessionid(hass, "anything") is False
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_validation_does_not_touch_shared_session(hass):
|
||||
"""The shared HA session may already hold a JSESSIONID cookie with a real
|
||||
domain (set by a previous Set-Cookie). Validation must run in an isolated
|
||||
aiohttp session so:
|
||||
|
||||
* the shared jar is not mutated as a side-effect of validation, and
|
||||
* a stale domain-bound JSESSIONID can't shadow the cookie we're validating.
|
||||
|
||||
Without this, aiohttp's cookie jar prefers the domain-matched (stale)
|
||||
cookie over the bare cookie we just added, so Samsung sees the expired
|
||||
session, returns no `_csrf`, and we tell the user their cookie was
|
||||
rejected even though it was perfectly fine."""
|
||||
hass.clientsession.cookie_jar.update_cookies(
|
||||
{"JSESSIONID": "STALE_VALUE"},
|
||||
response_url=URL("https://smartthingsfind.samsung.com/"),
|
||||
)
|
||||
before = sorted(
|
||||
(c.key, c["domain"], c.value)
|
||||
for c in hass.clientsession.cookie_jar
|
||||
)
|
||||
|
||||
with aioresponses() as m:
|
||||
m.get(URL_GET_CSRF, status=200, headers={"_csrf": "tok"})
|
||||
result = await validate_jsessionid(hass, "FRESH_VALUE")
|
||||
|
||||
after = sorted(
|
||||
(c.key, c["domain"], c.value)
|
||||
for c in hass.clientsession.cookie_jar
|
||||
)
|
||||
|
||||
assert before == after, (
|
||||
"validate_jsessionid mutated the shared HA session's cookie jar; "
|
||||
"it must use an isolated session.\n"
|
||||
f" before: {before}\n after: {after}"
|
||||
)
|
||||
assert result is True, (
|
||||
"validate_jsessionid returned False even though the mocked server "
|
||||
"would have replied with _csrf — the stale shared-jar cookie likely "
|
||||
"shadowed the fresh one"
|
||||
)
|
||||
Loading…
Add table
Add a link
Reference in a new issue