Skip to content

fix(dashboard-auth): exclude non-interactive providers from interactive login surfaces - #53239

Merged
benbarclay merged 5 commits into
NousResearch:mainfrom
IAvecilla:fix-drain-login
Jun 27, 2026
Merged

fix(dashboard-auth): exclude non-interactive providers from interactive login surfaces#53239
benbarclay merged 5 commits into
NousResearch:mainfrom
IAvecilla:fix-drain-login

Conversation

@IAvecilla

Copy link
Copy Markdown
Contributor

What does this PR do?

  • The login page rendered two buttons: "Sign in with Nous Research" and "Sign in with Drain Control (service credential)." Clicking the Drain one returned Internal Server Error.
  • Separately, once the dashboard session's access-token cookie expired (~15 min), the whole dashboard would 500 and the instance would flip to "reconnecting."

The safe-shutdown work (#52937) added DrainSecretProvider, a machine-to-machine service credential that NAS uses (bearer token) to trigger drains into the shared dashboard auth provider registry. But the interactive login surfaces (the login page, the /auth/login dispatch, and the gate's cookie verify_session / refresh_session loops) iterated every registered provider with no notion of "interactive human login vs. non-interactive service credential." So the drain credential got mis-treated as a sign-in option:

  • rendered as a login button, and dispatched to start_login on click → NotImplementedError → 500;
  • iterated in the cookie verify/refresh loops, where refresh_session raised → 500 on every refresh once the access token expired (wedging the dashboard).

It was never meant to be an interactive login method.

Fix: Add a capability flag supports_session to DashboardAuthProvider (mirroring the existing supports_token) plus a list_session_providers() registry helper. The interactive surfaces now consult only providers that actually do interactive login. DrainSecretProvider sets supports_session = False, so it's excluded from the login page, from /auth/login (which now returns 404 for it instead of 500), and from the verify/refresh loops.

Type of Change

  • 🐛 Bug fix (non-breaking change that fixes an issue)
  • ✨ New feature (non-breaking change that adds functionality)
  • 🔒 Security fix
  • 📝 Documentation update
  • ✅ Tests (adding or improving test coverage)
  • ♻️ Refactor (no behavior change)
  • 🎯 New skill (bundled or hub)

Changes Made

  • hermes_cli/dashboard_auth/base.py: add supports_session: bool = True capability flag to DashboardAuthProvider.
  • hermes_cli/dashboard_auth/registry.py: add list_session_providers() (subset with supports_session=True); mirror of list_token_providers().
  • hermes_cli/dashboard_auth/__init__.py: export list_session_providers.
  • hermes_cli/dashboard_auth/middleware.py: the gate's verify_session and refresh_session loops iterate list_session_providers() instead of list_providers().
  • hermes_cli/dashboard_auth/login_page.py: render login buttons only for list_session_providers().
  • hermes_cli/dashboard_auth/routes.py: /api/auth/providers bootstrap lists only list_session_providers(); /auth/login returns 404 for a provider that isn't interactive (supports_session false) instead of 500-ing on start_login.
  • plugins/dashboard_auth/drain/__init__.py: DrainSecretProvider sets supports_session = False.
  • Tests: test_dashboard_token_auth.py (list_session_providers filtering), test_dashboard_auth_middleware.py (/auth/login?provider=drain-secret → 404 + excluded from bootstrap), test_drain_provider.py (drain declares supports_session=False).

How to Test

  1. Run a hosted agent on a build that includes this branch and open its dashboard login page.
  2. Confirm the login screen shows only "Sign in with Nous Research" — no "Sign in with Drain Control (service credential)" button.
  3. Hit …/auth/login?provider=drain-secret directly → returns 404 (not Internal Server Error).
  4. Sign in, then let the session sit past the access-token expiry (~15 min) or clear the hermes_session_at cookie → the dashboard refreshes transparently and stays up (before: 500 / "reconnecting").
  5. Confirm drain still works: NAS can authenticate to /api/gateway/drain with the per-agent bearer secret (verify_token path unchanged).

Checklist

Code

  • I've read the Contributing Guide
  • My commit messages follow Conventional Commits
  • I searched for existing PRs to make sure this isn't a duplicate
  • My PR contains only changes related to this fix.
  • I've run the dashboard-auth test suites and they pass.
  • I've added tests for my changes.
  • I've tested on my platform: macOS 15 (build/local), Linux container (runtime, hosted agent).

Documentation & Housekeeping

  • I've updated relevant documentation (README, docs/, docstrings) — N/A (no public surface change; inline comments added)
  • I've updated cli-config.yaml.example if I added/changed config keys — N/A
  • I've updated CONTRIBUTING.md or AGENTS.md if I changed architecture or workflows — N/A
  • I've considered cross-platform impact (Windows, macOS) — pure Python/registry logic; no platform-specific behavior
  • I've updated tool descriptions/schemas if I changed tool behavior — N/A

@alt-glitch alt-glitch added type/bug Something isn't working comp/dashboard Web dashboard / control panel UI (dashboard/, landing) comp/cli CLI entry point, hermes_cli/, setup wizard area/auth Authentication, OAuth, credential pools comp/plugins Plugin system and bundled plugins P2 Medium — degraded but workaround exists sweeper:risk-security-boundary Sweeper risk: may affect sandboxing, auth, credentials, or sensitive data labels Jun 26, 2026
@alt-glitch

Copy link
Copy Markdown
Collaborator

This was generated by AI during triage.

@rob-maron Tagging you on this credits-notice item as the soft maintainer (follow-up to #43669).

@IAvecilla
IAvecilla marked this pull request as ready for review June 26, 2026 20:34
@benbarclay
benbarclay merged commit dbe734b into NousResearch:main Jun 27, 2026
27 checks passed
pai-scaffolde pushed a commit to pai-scaffolde/hermes-agent that referenced this pull request Jun 28, 2026
…ve login surfaces (NousResearch#53239)

* Return None instead of erroring on drain login failure

* Fix login on drain

* Remove login for drained endpoints flow and clean the code

* chore: drop unrelated credits changes from this PR

* Remove extra comments that were not really necessary
waefrebeorn pushed a commit to waefrebeorn/slermes that referenced this pull request Jul 2, 2026
…ve login surfaces (NousResearch#53239)

* Return None instead of erroring on drain login failure

* Fix login on drain

* Remove login for drained endpoints flow and clean the code

* chore: drop unrelated credits changes from this PR

* Remove extra comments that were not really necessary
habarmc1223-sudo pushed a commit to habarmc1223-sudo/hermes-agent-fluxmem that referenced this pull request Jul 8, 2026
…ve login surfaces (NousResearch#53239)

* Return None instead of erroring on drain login failure

* Fix login on drain

* Remove login for drained endpoints flow and clean the code

* chore: drop unrelated credits changes from this PR

* Remove extra comments that were not really necessary
santhreal pushed a commit to santhreal/hermes-agent that referenced this pull request Jul 13, 2026
…ve login surfaces (NousResearch#53239)

* Return None instead of erroring on drain login failure

* Fix login on drain

* Remove login for drained endpoints flow and clean the code

* chore: drop unrelated credits changes from this PR

* Remove extra comments that were not really necessary
Gravezzz pushed a commit to Gravezzz/hermes-agent that referenced this pull request Jul 21, 2026
…ve login surfaces (NousResearch#53239)

* Return None instead of erroring on drain login failure

* Fix login on drain

* Remove login for drained endpoints flow and clean the code

* chore: drop unrelated credits changes from this PR

* Remove extra comments that were not really necessary
leewenjie pushed a commit to leewenjie/hermes-agent that referenced this pull request Aug 7, 2026
…ve login surfaces (NousResearch#53239)

* Return None instead of erroring on drain login failure

* Fix login on drain

* Remove login for drained endpoints flow and clean the code

* chore: drop unrelated credits changes from this PR

* Remove extra comments that were not really necessary
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area/auth Authentication, OAuth, credential pools comp/cli CLI entry point, hermes_cli/, setup wizard comp/dashboard Web dashboard / control panel UI (dashboard/, landing) comp/plugins Plugin system and bundled plugins P2 Medium — degraded but workaround exists sweeper:risk-security-boundary Sweeper risk: may affect sandboxing, auth, credentials, or sensitive data type/bug Something isn't working

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants