diff --git a/hermes_cli/auth.py b/hermes_cli/auth.py index 165410bcd..021905c3e 100644 --- a/hermes_cli/auth.py +++ b/hermes_cli/auth.py @@ -1322,38 +1322,10 @@ def get_provider_auth_state(provider_id: str) -> Optional[Dict[str, Any]]: return _load_provider_state(auth_store, provider_id) -def _active_provider_from_store(auth_store: Dict[str, Any]) -> Optional[str]: - """Return the active provider for a loaded auth store. - - In profile mode, falls back to the global-root ``auth.json`` when the - profile store has no ``active_provider`` set. This mirrors the per-provider - shadowing already used by ``_load_provider_state`` and - ``read_credential_pool``: a named profile that never selected its own - provider still resolves the provider the user authenticated at the global - root (e.g. a Nous OAuth login), so ``model.provider: auto`` works under a - profile. A profile that has its own ``active_provider`` always wins; the - fallback only fires when the profile has none. Returns ``None`` when - neither scope has one. In classic mode ``_load_global_auth_store`` returns - an empty dict, so this is a no-op. See issue #18594 follow-up. - """ - active = auth_store.get("active_provider") - if active: - return active - global_store = _load_global_auth_store() - if global_store: - return global_store.get("active_provider") - return None - - def get_active_provider() -> Optional[str]: - """Return the currently active provider ID from auth store. - - In profile mode this falls back to the global-root ``active_provider`` - when the profile has not selected one of its own — see - ``_active_provider_from_store``. - """ + """Return the currently active provider ID from auth store.""" auth_store = _load_auth_store() - return _active_provider_from_store(auth_store) + return auth_store.get("active_provider") def is_provider_explicitly_configured(provider_id: str) -> bool: @@ -1575,14 +1547,10 @@ def resolve_provider( if explicit_api_key or explicit_base_url: return "openrouter" - # Check auth store for an active OAuth provider. In profile mode this - # honors the global-root active_provider when the profile has none of its - # own, mirroring the credential-pool / provider-state fallbacks so a - # named profile running model.provider: auto can use a globally - # authenticated provider. See issue #18594 follow-up. + # Check auth store for an active OAuth provider try: auth_store = _load_auth_store() - active = _active_provider_from_store(auth_store) + active = auth_store.get("active_provider") if active and active in PROVIDER_REGISTRY: status = get_auth_status(active) if status.get("logged_in"): diff --git a/plugins/platforms/google_chat/adapter.py b/plugins/platforms/google_chat/adapter.py index 0fdf1ea9d..f91a54417 100644 --- a/plugins/platforms/google_chat/adapter.py +++ b/plugins/platforms/google_chat/adapter.py @@ -1390,7 +1390,7 @@ class GoogleChatAdapter(BasePlatformAdapter): if arg == "start": if not oauth_helper._client_secret_path().exists(): await _reply( - "⚠️ No client credentials stored on the host. Send " + "⚠️ No client credentials stored for this profile. Send " "`/setup-files` (no args) for setup instructions." ) return True diff --git a/plugins/platforms/google_chat/oauth.py b/plugins/platforms/google_chat/oauth.py index 3b1101106..3d481b3ea 100644 --- a/plugins/platforms/google_chat/oauth.py +++ b/plugins/platforms/google_chat/oauth.py @@ -50,10 +50,8 @@ Token storage layout ``${HERMES_HOME}/google_chat_user_oauth_pending/.json`` - Legacy pending state: ``${HERMES_HOME}/google_chat_user_oauth_pending.json`` -- Shared OAuth client (one per host, anchored at the default Hermes root so - every profile sees it; a profile-local copy under ``${HERMES_HOME}`` wins - when present): - ``/google_chat_user_client_secret.json`` (default ``~/.hermes``) +- OAuth client secret (profile-scoped — each profile registers its own): + ``${HERMES_HOME}/google_chat_user_client_secret.json`` """ from __future__ import annotations @@ -77,11 +75,7 @@ logger = logging.getLogger("gateway.platforms.google_chat_user_oauth") # Use the project's HERMES_HOME helper so the token follows the user's # profile (e.g. tests can override via HERMES_HOME=/tmp/...). try: - from hermes_constants import ( - display_hermes_home, - get_default_hermes_root, - get_hermes_home, - ) + from hermes_constants import display_hermes_home, get_hermes_home except (ModuleNotFoundError, ImportError): # Fallback for environments where hermes_constants isn't importable # (mirrors the same fallback used by the google-workspace skill's @@ -90,24 +84,6 @@ except (ModuleNotFoundError, ImportError): val = os.environ.get("HERMES_HOME", "").strip() return Path(val) if val else Path.home() / ".hermes" - def get_default_hermes_root() -> Path: - # Mirror hermes_constants.get_default_hermes_root(): resolve the - # profile root so host-wide files (the shared client secret) are - # found regardless of which profile is active. - native_home = Path.home() / ".hermes" - env_home = os.environ.get("HERMES_HOME", "").strip() - if not env_home: - return native_home - env_path = Path(env_home) - try: - env_path.resolve().relative_to(native_home.resolve()) - return native_home - except ValueError: - pass - if env_path.parent.name == "profiles": - return env_path.parent.parent - return env_path - def display_hermes_home() -> str: home = get_hermes_home() try: @@ -164,24 +140,7 @@ def _token_path(email: Optional[str] = None) -> Path: def _client_secret_path() -> Path: - """Path to the shared OAuth client secret (one per host). - - The client secret identifies the OAuth *app*, not a user or a profile, - so it is anchored at the default Hermes root (``~/.hermes`` — or the - Docker root) rather than the active profile's ``HERMES_HOME``. That way - the one-time ``--client-secret`` host setup is visible to gateways - running under any named profile, exactly as the docs describe ("one - file per host is enough no matter how many users authorize later"). - - A profile-local secret (``$HERMES_HOME/google_chat_user_client_secret.json``) - still takes precedence when present, for installs that seeded one under - the previous profile-scoped behavior or that deliberately run a separate - OAuth app per profile. - """ - profile_local = _hermes_home() / "google_chat_user_client_secret.json" - if profile_local.exists(): - return profile_local - return get_default_hermes_root() / "google_chat_user_client_secret.json" + return _hermes_home() / "google_chat_user_client_secret.json" def _pending_auth_path(email: Optional[str] = None) -> Path: diff --git a/tests/gateway/test_google_chat.py b/tests/gateway/test_google_chat.py index 03ab232eb..b75902785 100644 --- a/tests/gateway/test_google_chat.py +++ b/tests/gateway/test_google_chat.py @@ -16,7 +16,6 @@ import json import os import sys import types -from pathlib import Path from unittest.mock import AsyncMock, MagicMock, patch import pytest @@ -1651,48 +1650,6 @@ class TestUserOAuthHelper: }, ) - def test_client_secret_is_shared_across_profiles(self, tmp_path, monkeypatch): - """The OAuth client secret is host-wide infra: a secret seeded at the - default root by the documented one-time `--client-secret` host step - must be visible to a gateway running under a named profile. - - Regression: `_client_secret_path()` used to scope to the active - HERMES_HOME, so a profile gateway reported 'No client credentials - stored on the host' even after the host setup had been run. - """ - root = tmp_path / ".hermes" - profile_home = root / "profiles" / "bot1" - profile_home.mkdir(parents=True) - monkeypatch.setattr(Path, "home", lambda: tmp_path) - - # Seed the secret at the default root, as the host setup does. - secret = root / "google_chat_user_client_secret.json" - secret.write_text("{}", encoding="utf-8") - - # Resolve from inside a named profile. - monkeypatch.setenv("HERMES_HOME", str(profile_home)) - from plugins.platforms.google_chat.oauth import _client_secret_path - assert _client_secret_path() == secret - assert _client_secret_path().exists() - - def test_profile_local_client_secret_takes_precedence(self, tmp_path, monkeypatch): - """A profile-local secret (separate OAuth app per bot, or a legacy - profile-scoped seed) overrides the host-wide default when present.""" - root = tmp_path / ".hermes" - profile_home = root / "profiles" / "bot1" - profile_home.mkdir(parents=True) - monkeypatch.setattr(Path, "home", lambda: tmp_path) - monkeypatch.setenv("HERMES_HOME", str(profile_home)) - - (root / "google_chat_user_client_secret.json").write_text( - "{}", encoding="utf-8" - ) - profile_secret = profile_home / "google_chat_user_client_secret.json" - profile_secret.write_text("{}", encoding="utf-8") - - from plugins.platforms.google_chat.oauth import _client_secret_path - assert _client_secret_path() == profile_secret - def test_store_client_secret_writes_private_json(self, tmp_path, monkeypatch): monkeypatch.setenv("HERMES_HOME", str(tmp_path)) src = tmp_path / "client_secret.json" diff --git a/tests/hermes_cli/test_auth_profile_fallback.py b/tests/hermes_cli/test_auth_profile_fallback.py index d041b4efa..5210404c4 100644 --- a/tests/hermes_cli/test_auth_profile_fallback.py +++ b/tests/hermes_cli/test_auth_profile_fallback.py @@ -17,18 +17,12 @@ from pathlib import Path import pytest -def _make_auth_store( - pool: dict | None = None, - providers: dict | None = None, - active_provider: str | None = None, -) -> dict: +def _make_auth_store(pool: dict | None = None, providers: dict | None = None) -> dict: store: dict = {"version": 1} if pool is not None: store["credential_pool"] = pool if providers is not None: store["providers"] = providers - if active_provider is not None: - store["active_provider"] = active_provider return store @@ -456,101 +450,3 @@ def test_write_credential_pool_targets_profile_not_global(profile_env): # Subsequent read returns profile (shadows global). assert [e["id"] for e in read_credential_pool("openrouter")] == ["prof-new"] - - -# --------------------------------------------------------------------------- -# get_active_provider — global active_provider fallback (issue #18594 follow-up) -# -# The per-provider state/pool fallbacks let a profile *read* a provider that -# was only authenticated at the global root, but ``resolve_provider()`` picks -# the ``auto`` provider from ``active_provider`` — which only ever read the -# profile store. A named profile running ``model.provider: auto`` could see -# the global Nous login (``get_provider_auth_state('nous')`` succeeds) yet -# still fail to select it. These pin the active_provider shadowing so the -# selection mirrors the state/pool fallbacks: profile wins when present, fall -# back to global when the profile never chose its own provider. -# --------------------------------------------------------------------------- - - -def test_active_provider_falls_back_to_global(profile_env): - """An empty profile inherits the global-root active_provider selection.""" - from hermes_cli.auth import get_active_provider - - _write(profile_env["global"] / "auth.json", _make_auth_store( - providers={"nous": {"access_token": "nous-global"}}, - active_provider="nous", - )) - _write(profile_env["profile"] / "auth.json", _make_auth_store(providers={})) - - assert get_active_provider() == "nous" - - -def test_active_provider_profile_wins_over_global(profile_env): - """A profile that selected its own provider shadows the global selection.""" - from hermes_cli.auth import get_active_provider - - _write(profile_env["global"] / "auth.json", _make_auth_store( - providers={"nous": {"access_token": "nous-global"}}, - active_provider="nous", - )) - _write(profile_env["profile"] / "auth.json", _make_auth_store( - providers={"anthropic": {"access_token": "ant-profile"}}, - active_provider="anthropic", - )) - - assert get_active_provider() == "anthropic" - - -def test_active_provider_none_when_neither_has_it(profile_env): - """No selection anywhere stays None — the fallback must not invent one.""" - from hermes_cli.auth import get_active_provider - - _write(profile_env["global"] / "auth.json", _make_auth_store(providers={})) - _write(profile_env["profile"] / "auth.json", _make_auth_store(providers={})) - - assert get_active_provider() is None - - -def test_active_provider_classic_mode_reads_profile(tmp_path, monkeypatch): - """In classic mode there is no global to fall back to; behavior is unchanged.""" - fake_home = tmp_path / "home" - fake_home.mkdir() - monkeypatch.setattr(Path, "home", lambda: fake_home) - hermes_home = tmp_path / "classic" - hermes_home.mkdir() - monkeypatch.setenv("HERMES_HOME", str(hermes_home)) - - _write(hermes_home / "auth.json", _make_auth_store( - providers={"nous": {"access_token": "classic-token"}}, - active_provider="nous", - )) - - from hermes_cli.auth import get_active_provider - - assert get_active_provider() == "nous" - - -def test_resolve_provider_uses_global_active_provider(profile_env, monkeypatch): - """resolve_provider('auto') honors the global-root active_provider. - - This is the user-visible contract: a named profile with no provider entry - of its own, started with ``model.provider: auto`` while a valid login - exists at the global root, resolves that provider instead of raising - ``No inference provider configured``. ``get_auth_status`` is stubbed so the - login check stays offline (no Nous token refresh / network). - """ - import hermes_cli.auth as auth - - _write(profile_env["global"] / "auth.json", _make_auth_store( - providers={"nous": {"access_token": "nous-global"}}, - active_provider="nous", - )) - _write(profile_env["profile"] / "auth.json", _make_auth_store(providers={})) - - monkeypatch.setattr( - auth, - "get_auth_status", - lambda provider=None: {"logged_in": True, "provider": provider}, - ) - - assert auth.resolve_provider("auto") == "nous" diff --git a/website/docs/user-guide/messaging/google_chat.md b/website/docs/user-guide/messaging/google_chat.md index d34ebbd2e..eeeb69c6c 100644 --- a/website/docs/user-guide/messaging/google_chat.md +++ b/website/docs/user-guide/messaging/google_chat.md @@ -231,28 +231,29 @@ There's no IAM role or scope that fixes this. The endpoint only accepts user credentials. So the bot has to act *as a user* whenever it uploads a file — specifically, as the user who asked for the file. -### One-time host setup +### One-time setup (per profile) 1. Go to **APIs & Services → Credentials** in the same GCP project. 2. **Create credentials → OAuth client ID → Desktop app**. 3. Download the JSON. Move it onto the host that runs Hermes. -4. On the host, register the client with Hermes: +4. Register the client with Hermes (run under the profile you want it scoped to): ```bash +# Default profile: python -m plugins.platforms.google_chat.oauth \ --client-secret /path/to/client_secret.json + +# A named profile gets its own separate registration: +hermes -p python -m plugins.platforms.google_chat.oauth \ + --client-secret /path/to/client_secret.json ``` -That writes `~/.hermes/google_chat_user_client_secret.json`. This is shared -infrastructure — it identifies the OAuth *app*, not any individual user. One -file per host is enough no matter how many users authorize later. - -This file lives at the default Hermes root, so a gateway running under a named -profile (`hermes -p gateway …`) finds the same host-wide secret — you do -**not** re-run this step per profile. To deliberately use a separate OAuth app -for one profile, drop a `google_chat_user_client_secret.json` inside that -profile's `HERMES_HOME` and it takes precedence. Per-user tokens always stay -scoped to the active profile. +That writes the client secret into the active profile's Hermes home (e.g. +`~/.hermes/google_chat_user_client_secret.json` for the default profile). The +client secret is **profile-scoped, not shared across profiles** — each profile +registers its own. This is deliberate: profiles are isolated auth boundaries, so +two profiles can point at different Google OAuth apps / accounts. Register it +once per profile that needs Google Chat attachment delivery. ### Per-user authorization (in chat) @@ -333,14 +334,20 @@ The asker has no per-user OAuth token and there's no legacy fallback. Run `/setup-files` in their DM and follow Step 10. After the exchange completes the next file request uploads natively without a gateway restart. -**`/setup-files start` says "No client credentials stored on the host."** +**`/setup-files start` says "No client credentials stored."** -The one-time host setup wasn't done. From a terminal on the host that runs -Hermes: +The one-time setup wasn't done *for this profile* (the client secret is +profile-scoped, so a registration under one profile won't be seen by another). +From a terminal, run it under the profile the gateway uses: ```bash +# Default profile: python -m plugins.platforms.google_chat.oauth \ --client-secret /path/to/client_secret.json + +# Named profile: +hermes -p python -m plugins.platforms.google_chat.oauth \ + --client-secret /path/to/client_secret.json ``` Then send `/setup-files start` again.