revert: keep Google Chat OAuth secret + active_provider profile-scoped (#39398)
* Revert "fix(gateway): anchor Google Chat OAuth client secret to default Hermes root" This reverts commitfff0561441. * Revert "fix(cli): honor global-root active_provider fallback for named profiles" This reverts commit3858cf4307. * docs(google_chat): describe OAuth client secret as profile-scoped, not host-wide The setup docs, oauth docstring, and the adapter's 'no credentials' error message all described the Google Chat OAuth client secret as host-wide shared infrastructure. That contradicts profile isolation: profiles are separate auth boundaries, so two profiles can point at different Google OAuth apps / accounts. Reword all three to say the secret is profile-scoped and each profile registers its own.
This commit is contained in:
@ -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"):
|
||||
|
||||
@ -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
|
||||
|
||||
@ -50,10 +50,8 @@ Token storage layout
|
||||
``${HERMES_HOME}/google_chat_user_oauth_pending/<sanitized_email>.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):
|
||||
``<default-root>/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:
|
||||
|
||||
@ -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"
|
||||
|
||||
@ -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"
|
||||
|
||||
@ -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 <profile> 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 <name> 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 <profile> python -m plugins.platforms.google_chat.oauth \
|
||||
--client-secret /path/to/client_secret.json
|
||||
```
|
||||
|
||||
Then send `/setup-files start` again.
|
||||
|
||||
Reference in New Issue
Block a user