diff --git a/app/api/isolarcloud.py b/app/api/isolarcloud.py index e8e0a0f..aeae547 100644 --- a/app/api/isolarcloud.py +++ b/app/api/isolarcloud.py @@ -7,15 +7,21 @@ from pydantic import BaseModel from app.config import settings from app.db.repositories import get_sync_state from app.db.session import get_session +from app.isolarcloud import shared_token_store router = APIRouter(prefix="/api/isolarcloud", tags=["isolarcloud"]) @router.get("/status") def status() -> dict: - token_key = "isolarcloud_oauth_access_token" if settings.isolarcloud_auth_mode == "oauth2" else "isolarcloud_token" - with get_session() as session: - has_token = bool(get_sync_state(session, token_key)) + if settings.isolarcloud_auth_mode == "oauth2" and shared_token_store.enabled(): + with shared_token_store.locked() as fd: + state = shared_token_store.read(fd) + has_token = bool(state and state.get("access_token")) + else: + token_key = "isolarcloud_oauth_access_token" if settings.isolarcloud_auth_mode == "oauth2" else "isolarcloud_token" + with get_session() as session: + has_token = bool(get_sync_state(session, token_key)) return {"use_mock": settings.use_mock_isolarcloud, "auth_mode": settings.isolarcloud_auth_mode, "connected": has_token} diff --git a/app/config.py b/app/config.py index 6dc7114..0f7aff9 100644 --- a/app/config.py +++ b/app/config.py @@ -32,6 +32,18 @@ class Settings(BaseSettings): isolarcloud_oauth_app_id: str = "" # "applicationId" from the developer portal isolarcloud_oauth_redirect_uri: str = "https://localhost:8000/isolarcloud/callback" + # If this app is deployed as more than one independent instance against + # the SAME iSolarCloud account (e.g. two households' plants under one + # family member's login), Sungrow's OAuth server only keeps one active + # token session per (app, account) alive -- each instance refreshing its + # own token independently silently invalidates every other instance's + # refresh token. Pointing every instance at the same file here (a path + # under a volume shared/bind-mounted across all of them) makes them + # share one token session instead of fighting over it. Leave blank for a + # single-instance deployment; tokens then stay in this instance's own + # DB as before. + isolarcloud_shared_token_path: str = "" + # Set to true until real credentials are configured; the scheduler then # polls app.isolarcloud.mock_client instead of the real API. use_mock_isolarcloud: bool = True diff --git a/app/isolarcloud/oauth_client.py b/app/isolarcloud/oauth_client.py index 52d57b0..9930283 100644 --- a/app/isolarcloud/oauth_client.py +++ b/app/isolarcloud/oauth_client.py @@ -54,6 +54,7 @@ import httpx from app.config import settings from app.db.repositories import get_sync_state, set_sync_state from app.db.session import get_session +from app.isolarcloud import shared_token_store from app.isolarcloud.parsing import POINT_EXPORT, POINT_GENERATION, POINT_IMPORT, POINT_LOAD, TS_FORMAT, merge_frames from app.isolarcloud.schemas import IntervalDataPoint @@ -147,13 +148,50 @@ class ISolarCloudOAuthClient: def _store_tokens(self, result_data: dict) -> None: expires_at = int(time.time()) + int(result_data["expires_in"]) - 60 - with get_session() as session: - set_sync_state(session, "isolarcloud_oauth_access_token", result_data["access_token"]) - set_sync_state(session, "isolarcloud_oauth_refresh_token", result_data["refresh_token"]) - set_sync_state(session, "isolarcloud_oauth_expires_at", str(expires_at)) - self._access_token = result_data["access_token"] + token_state = { + "access_token": result_data["access_token"], + "refresh_token": result_data["refresh_token"], + "expires_at": expires_at, + } + if shared_token_store.enabled(): + with shared_token_store.locked() as fd: + shared_token_store.write(fd, token_state) + else: + with get_session() as session: + set_sync_state(session, "isolarcloud_oauth_access_token", token_state["access_token"]) + set_sync_state(session, "isolarcloud_oauth_refresh_token", token_state["refresh_token"]) + set_sync_state(session, "isolarcloud_oauth_expires_at", str(expires_at)) + self._access_token = token_state["access_token"] def _get_access_token(self) -> str: + if shared_token_store.enabled(): + # Holds the lock across the whole check-and-maybe-refresh + # sequence (including the network round-trip), not just the + # file I/O either side of it -- otherwise two instances waking + # up around the same moment could both read the same + # about-to-expire refresh_token and race to redeem it, and + # only one of those calls will succeed. + with shared_token_store.locked() as fd: + state = shared_token_store.read(fd) + if not state or not state.get("access_token") or not state.get("refresh_token"): + raise ISolarCloudAuthError( + "No iSolarCloud OAuth tokens stored yet -- connect your account from the Settings page first." + ) + if int(state.get("expires_at") or 0) < int(time.time()): + data = self._post(REFRESH_PATH, {"refresh_token": state["refresh_token"]}) + result_data = data.get("result_data") or {} + if not isinstance(result_data, dict) or not result_data.get("access_token"): + raise ISolarCloudAuthError(f"Token refresh failed: {data} -- reconnect from Settings.") + expires_at = int(time.time()) + int(result_data["expires_in"]) - 60 + state = { + "access_token": result_data["access_token"], + "refresh_token": result_data["refresh_token"], + "expires_at": expires_at, + } + shared_token_store.write(fd, state) + self._access_token = state["access_token"] + return state["access_token"] + with get_session() as session: access_token = get_sync_state(session, "isolarcloud_oauth_access_token") refresh_token = get_sync_state(session, "isolarcloud_oauth_refresh_token") diff --git a/app/isolarcloud/shared_token_store.py b/app/isolarcloud/shared_token_store.py new file mode 100644 index 0000000..ba208b3 --- /dev/null +++ b/app/isolarcloud/shared_token_store.py @@ -0,0 +1,58 @@ +"""File-based OAuth token store shared across multiple instances of this +app that authenticate against the same iSolarCloud account (see the +comment on `Settings.isolarcloud_shared_token_path`). + +Uses `flock` on the shared file itself as the lock, not a separate lock +file -- `flock` is a host-kernel-level advisory lock keyed on the +underlying inode, so it works across container boundaries as long as +every instance bind-mounts the *same* host file (not independent copies). +The lock is held for the whole read-check-refresh-write sequence in +`oauth_client._get_access_token`, not just the file I/O, so two instances +waking up at the same moment can't both refresh against the +soon-to-be-superseded refresh token. +""" + +from __future__ import annotations + +import contextlib +import fcntl +import json +import os +from collections.abc import Iterator +from pathlib import Path + +from app.config import settings + + +def enabled() -> bool: + return bool(settings.isolarcloud_shared_token_path) + + +@contextlib.contextmanager +def locked() -> Iterator[int]: + path = Path(settings.isolarcloud_shared_token_path) + path.parent.mkdir(parents=True, exist_ok=True) + fd = os.open(path, os.O_RDWR | os.O_CREAT, 0o600) + try: + fcntl.flock(fd, fcntl.LOCK_EX) + yield fd + finally: + fcntl.flock(fd, fcntl.LOCK_UN) + os.close(fd) + + +def read(fd: int) -> dict | None: + os.lseek(fd, 0, os.SEEK_SET) + raw = os.read(fd, 1_000_000) + if not raw: + return None + try: + return json.loads(raw) + except json.JSONDecodeError: + return None + + +def write(fd: int, data: dict) -> None: + os.lseek(fd, 0, os.SEEK_SET) + os.ftruncate(fd, 0) + os.write(fd, json.dumps(data).encode()) diff --git a/app/isolarcloud/tests/__init__.py b/app/isolarcloud/tests/__init__.py new file mode 100644 index 0000000..e69de29 diff --git a/app/isolarcloud/tests/test_shared_token_store.py b/app/isolarcloud/tests/test_shared_token_store.py new file mode 100644 index 0000000..31700f5 --- /dev/null +++ b/app/isolarcloud/tests/test_shared_token_store.py @@ -0,0 +1,67 @@ +import time + +import pytest + +from app.config import settings +from app.isolarcloud import shared_token_store +from app.isolarcloud.oauth_client import ISolarCloudOAuthClient + + +@pytest.fixture +def shared_path(tmp_path, monkeypatch): + path = tmp_path / "isolarcloud_oauth.json" + monkeypatch.setattr(settings, "isolarcloud_shared_token_path", str(path)) + return path + + +def test_disabled_by_default(monkeypatch): + monkeypatch.setattr(settings, "isolarcloud_shared_token_path", "") + assert shared_token_store.enabled() is False + + +def test_read_write_roundtrip(shared_path): + assert shared_token_store.enabled() is True + with shared_token_store.locked() as fd: + assert shared_token_store.read(fd) is None # file just created, empty + shared_token_store.write(fd, {"access_token": "a", "refresh_token": "r", "expires_at": 123}) + + with shared_token_store.locked() as fd: + assert shared_token_store.read(fd) == {"access_token": "a", "refresh_token": "r", "expires_at": 123} + + +def test_second_instance_reuses_token_written_by_first(shared_path): + """The whole point: two ISolarCloudOAuthClient instances (standing in + for two separate Docker containers) pointed at the same shared path + see each other's tokens rather than needing their own independent + authorization.""" + client_a = ISolarCloudOAuthClient() + client_a._store_tokens({"access_token": "tok-a", "refresh_token": "ref-a", "expires_in": 3600}) + + client_b = ISolarCloudOAuthClient() + # Not expired yet, so this must return client_a's token without + # attempting any HTTP refresh call. + client_b._post = lambda *a, **k: pytest.fail("should not need to refresh a still-valid token") + assert client_b._get_access_token() == "tok-a" + + +def test_expired_token_refreshes_once_and_shares_result(shared_path): + client_a = ISolarCloudOAuthClient() + with shared_token_store.locked() as fd: + shared_token_store.write(fd, {"access_token": "stale", "refresh_token": "ref-old", "expires_at": int(time.time()) - 10}) + + refresh_calls = [] + + def fake_post(path, payload, access_token=None): + refresh_calls.append(payload) + return {"result_data": {"access_token": "fresh", "refresh_token": "ref-new", "expires_in": 3600}} + + client_a._post = fake_post + assert client_a._get_access_token() == "fresh" + assert len(refresh_calls) == 1 + assert refresh_calls[0] == {"refresh_token": "ref-old"} + + # A second instance reads the already-refreshed token and doesn't + # trigger a second refresh call of its own. + client_b = ISolarCloudOAuthClient() + client_b._post = lambda *a, **k: pytest.fail("token was already refreshed by client_a") + assert client_b._get_access_token() == "fresh" diff --git a/docker-compose.yml b/docker-compose.yml index 9202b13..54921f3 100644 --- a/docker-compose.yml +++ b/docker-compose.yml @@ -14,10 +14,21 @@ services: - /app/data/power-compare-persistent/.env environment: - DB_PATH=/data/power_compare.db + # Shared with the McDowall stack's identical setting via the volume + # below -- both instances authenticate against the same iSolarCloud + # account, and Sungrow's OAuth server only keeps one active token + # session per (app, account), so each instance refreshing its own + # copy independently was silently invalidating the other's. Pointing + # both at the same bind-mounted file makes them share one session. + - ISOLARCLOUD_SHARED_TOKEN_PATH=/shared-oauth/isolarcloud_oauth.json volumes: # Bind-mount SOURCE paths, by contrast, are resolved by the real # Docker daemon against the actual host filesystem (the daemon isn't # inside the stack manager's container) -- so this one stays a real # host path, not an /app/data/... one. - /Users/davidlawler/docker/power-compare-data/data:/data + # Same host path as the McDowall stack -- this MUST stay identical + # across both compose files, since the point is for both containers + # to bind-mount the literal same file. + - /Users/davidlawler/docker/power-compare-shared-oauth:/shared-oauth restart: unless-stopped