Skip to content
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions .mailmap
Original file line number Diff line number Diff line change
Expand Up @@ -106,3 +106,4 @@ xinbenlv <zzn+pa@zzn.im> <zzn+pa@zzn.im>
SaulJWu <saul.jj.wu@gmail.com> <saul.jj.wu@gmail.com>
angelos <angelos@oikos.lan.home.malaiwah.com> <angelos@oikos.lan.home.malaiwah.com>
MestreY0d4-Uninter <241404605+MestreY0d4-Uninter@users.noreply.github.com> <MestreY0d4-Uninter@users.noreply.github.com>
zxcasongs <35259607+zxcasongs@users.noreply.github.com> <zxcasongs@gmail.com>
5 changes: 3 additions & 2 deletions hermes_cli/dashboard_auth/middleware.py
Original file line number Diff line number Diff line change
Expand Up @@ -184,11 +184,12 @@ def _auto_sso_response(request: Request) -> Response | None:
# Zero → nothing to redirect to. Two+ → user must choose at /login.
return None

from hermes_cli.dashboard_auth.prefix import prefix_from_request

provider = providers[0]
if getattr(provider, "supports_password", False):
# Password-only providers have no OAuth redirect flow — they
# need the login page's credential form. Fall through to /login.
return None
from hermes_cli.dashboard_auth.prefix import prefix_from_request

prefix = prefix_from_request(request)
next_param = _safe_next_target(request)
Expand Down
13 changes: 13 additions & 0 deletions hermes_cli/dashboard_auth/routes.py
Original file line number Diff line number Diff line change
Expand Up @@ -201,6 +201,19 @@ async def auth_login(request: Request, provider: str, next: str = ""):
login_url = f"{login_url}?next={quote(safe_next, safe='')}"
return RedirectResponse(url=login_url, status_code=302)

if getattr(p, "supports_password", False):
# Password-only providers have no OAuth redirect flow.
# Redirect to the login page which renders the credential form.
# Use the same safe-next validation + prefix-aware URL building as
# the rest of the auth flow so reverse-proxy and open-redirect
# protections are preserved.
safe_next = _validate_post_login_target(next)
target = f"{_prefix(request)}/login"
if safe_next:
from urllib.parse import quote
target = f"{target}?next={quote(safe_next, safe='')}"
return RedirectResponse(url=target, status_code=303)

try:
ls = p.start_login(redirect_uri=_redirect_uri(request))
except ProviderError as e:
Expand Down
1 change: 1 addition & 0 deletions scripts/release.py
Original file line number Diff line number Diff line change
Expand Up @@ -384,6 +384,7 @@
"github.com@wolfram.ravenwolf.de": "WolframRavenwolf",
"895252509@qq.com": "895252509",
"35259607+zxcasongs@users.noreply.github.com": "zxcasongs",
"zxcasongs@gmail.com": "zxcasongs",
"alfred@my-cloud.me": "alfred-smith-0",
"tangtaizhong792@gmail.com": "tangtaizong666",
"github@aldo.pw": "aldoeliacim",
Expand Down
140 changes: 140 additions & 0 deletions tests/hermes_cli/test_dashboard_auth_password_only_regression.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,140 @@
"""Regression tests for the password-only-provider NotImplementedError crash.

https://github.com/NousResearch/hermes-agent/pull/56886

Covers the two broken entry points:
1. First unauthenticated dashboard hit with a single password-only provider
should land on /login (credential form), not auto-redirect into
/auth/login which calls start_login() and raises NotImplementedError.
2. Direct /auth/login?provider=<password-provider> (bookmark, manual URL,
logout path) should redirect to /login, not call start_login() and 500.
"""

from __future__ import annotations

import pytest
from fastapi.testclient import TestClient

from hermes_cli import web_server
from hermes_cli.dashboard_auth import (
clear_providers,
register_provider,
)
from tests.hermes_cli.conftest_dashboard_auth import StubAuthProvider
from tests.hermes_cli.test_dashboard_auth_password_login import PasswordProvider


@pytest.fixture
def pw_only_app():
clear_providers()
register_provider(PasswordProvider())
prev_required = getattr(web_server.app.state, "auth_required", None)
web_server.app.state.auth_required = True
client = TestClient(web_server.app, base_url="https://fly-app.fly.dev")
yield client
clear_providers()
web_server.app.state.auth_required = prev_required


class TestAutoSSOBypassForPasswordProvider:
"""First unauthenticated dashboard hit with a single password-only
provider should land on /login (credential form), not auto-redirect
into /auth/login which calls start_login() and raises NotImplementedError.
"""

def test_first_hit_lands_on_login_not_500(self, pw_only_app):
resp = pw_only_app.get("/", follow_redirects=False)
assert resp.status_code in (302, 303)
assert "/login" in resp.headers["location"]
assert "/auth/login" not in resp.headers["location"]

def test_first_hit_preserves_next_param(self, pw_only_app):
resp = pw_only_app.get("/sessions", follow_redirects=False)
assert resp.status_code in (302, 303)
location = resp.headers["location"]
assert "/login" in location
assert "next=" in location

def test_first_hit_followed_redirect_renders_login_page(self, pw_only_app):
resp = pw_only_app.get("/", follow_redirects=True)
assert resp.status_code == 200
assert "provider-form" in resp.text or "password" in resp.text.lower()


class TestAuthLoginRecoveryForPasswordProvider:
"""Direct /auth/login?provider=<password-provider> (bookmark, manual URL,
logout path) should redirect to /login, not call start_login() and 500.
"""

def test_direct_auth_login_redirects_to_login_not_500(self, pw_only_app):
resp = pw_only_app.get(
"/auth/login?provider=testpw", follow_redirects=False
)
assert resp.status_code in (302, 303)
assert "/login" in resp.headers["location"]

def test_direct_auth_login_with_safe_next(self, pw_only_app):
resp = pw_only_app.get(
"/auth/login?provider=testpw&next=/sessions",
follow_redirects=False,
)
assert resp.status_code in (302, 303)
location = resp.headers["location"]
assert "/login" in location
assert "next=%2Fsessions" in location or "next=/sessions" in location

def test_direct_auth_login_with_open_redirect_next_drops_it(
self, pw_only_app
):
resp = pw_only_app.get(
"/auth/login?provider=testpw&next=https://evil.example/phish",
follow_redirects=False,
)
assert resp.status_code in (302, 303)
location = resp.headers["location"]
assert "/login" in location
assert "evil.example" not in location

def test_direct_auth_login_with_protocol_relative_next_drops_it(
self, pw_only_app
):
resp = pw_only_app.get(
"/auth/login?provider=testpw&next=//evil.example",
follow_redirects=False,
)
assert resp.status_code in (302, 303)
location = resp.headers["location"]
assert "/login" in location
assert "evil.example" not in location

def test_direct_auth_login_no_next_no_query(self, pw_only_app):
resp = pw_only_app.get(
"/auth/login?provider=testpw",
follow_redirects=False,
)
assert resp.status_code in (302, 303)
location = resp.headers["location"]
assert "/login" in location
assert "next=" not in location

def test_oauth_provider_still_works_via_auth_login(self):
"""Ensure the OAuth path is unaffected -- StubAuthProvider
(supports_password=False) should still start_login normally.
"""
clear_providers()
register_provider(StubAuthProvider())
prev = getattr(web_server.app.state, "auth_required", None)
web_server.app.state.auth_required = True
try:
client = TestClient(
web_server.app, base_url="https://fly-app.fly.dev"
)
resp = client.get(
"/auth/login?provider=stub", follow_redirects=False
)
assert resp.status_code == 302
assert "/login" not in resp.headers["location"]
assert "code=stub_code" in resp.headers["location"]
finally:
clear_providers()
web_server.app.state.auth_required = prev
Loading