mirror of
https://github.com/jkingsman/Remote-Terminal-for-MeshCore.git
synced 2026-03-28 17:43:05 +01:00
Overhaul repeater interaction to better deal with login failure clearly
This commit is contained in:
@@ -410,6 +410,11 @@ class RepeaterLoginResponse(BaseModel):
|
||||
"""Response from repeater login."""
|
||||
|
||||
status: str = Field(description="Login result status")
|
||||
authenticated: bool = Field(description="Whether repeater authentication was confirmed")
|
||||
message: str | None = Field(
|
||||
default=None,
|
||||
description="Optional warning or error message when authentication was not confirmed",
|
||||
)
|
||||
|
||||
|
||||
class RepeaterStatusResponse(BaseModel):
|
||||
|
||||
@@ -44,8 +44,21 @@ ACL_PERMISSION_NAMES = {
|
||||
}
|
||||
router = APIRouter(prefix="/contacts", tags=["repeaters"])
|
||||
|
||||
# Delay between repeater radio operations to allow key exchange and path establishment
|
||||
REPEATER_OP_DELAY_SECONDS = 2.0
|
||||
REPEATER_LOGIN_RESPONSE_TIMEOUT_SECONDS = 5.0
|
||||
REPEATER_LOGIN_REJECTED_MESSAGE = (
|
||||
"The repeater replied but did not confirm this login. "
|
||||
"Existing access may still allow some repeater operations, but admin actions may fail."
|
||||
)
|
||||
REPEATER_LOGIN_SEND_FAILED_MESSAGE = (
|
||||
"The login request could not be sent to the repeater. "
|
||||
"The dashboard is still available, but repeater operations may fail until a login succeeds."
|
||||
)
|
||||
REPEATER_LOGIN_TIMEOUT_MESSAGE = (
|
||||
"No login confirmation was heard from the repeater. "
|
||||
"On current repeater firmware, that can mean the password was wrong, "
|
||||
"blank-password login was not allowed by the ACL, or the reply was missed in transit. "
|
||||
"The dashboard is still available; try logging in again if admin actions fail."
|
||||
)
|
||||
|
||||
|
||||
def _monotonic() -> float:
|
||||
@@ -136,31 +149,88 @@ async def _fetch_repeater_response(
|
||||
return None
|
||||
|
||||
|
||||
async def prepare_repeater_connection(mc, contact: Contact, password: str) -> None:
|
||||
"""Prepare connection to a repeater by adding to radio and logging in.
|
||||
async def prepare_repeater_connection(mc, contact: Contact, password: str) -> RepeaterLoginResponse:
|
||||
"""Prepare connection to a repeater by adding to radio and attempting login.
|
||||
|
||||
Args:
|
||||
mc: MeshCore instance
|
||||
contact: The repeater contact
|
||||
password: Password for login (empty string for no password)
|
||||
|
||||
Raises:
|
||||
HTTPException: If login fails
|
||||
"""
|
||||
pubkey_prefix = contact.public_key[:12].lower()
|
||||
loop = asyncio.get_running_loop()
|
||||
login_future = loop.create_future()
|
||||
|
||||
def _resolve_login(event_type: EventType, message: str | None = None) -> None:
|
||||
if login_future.done():
|
||||
return
|
||||
login_future.set_result(
|
||||
RepeaterLoginResponse(
|
||||
status="ok" if event_type == EventType.LOGIN_SUCCESS else "error",
|
||||
authenticated=event_type == EventType.LOGIN_SUCCESS,
|
||||
message=message,
|
||||
)
|
||||
)
|
||||
|
||||
success_subscription = mc.subscribe(
|
||||
EventType.LOGIN_SUCCESS,
|
||||
lambda _event: _resolve_login(EventType.LOGIN_SUCCESS),
|
||||
attribute_filters={"pubkey_prefix": pubkey_prefix},
|
||||
)
|
||||
failed_subscription = mc.subscribe(
|
||||
EventType.LOGIN_FAILED,
|
||||
lambda _event: _resolve_login(
|
||||
EventType.LOGIN_FAILED,
|
||||
REPEATER_LOGIN_REJECTED_MESSAGE,
|
||||
),
|
||||
attribute_filters={"pubkey_prefix": pubkey_prefix},
|
||||
)
|
||||
|
||||
# Add contact to radio with path from DB (non-fatal — contact may already be loaded)
|
||||
logger.info("Adding repeater %s to radio", contact.public_key[:12])
|
||||
await _ensure_on_radio(mc, contact)
|
||||
try:
|
||||
logger.info("Adding repeater %s to radio", contact.public_key[:12])
|
||||
await _ensure_on_radio(mc, contact)
|
||||
|
||||
# Send login with password
|
||||
logger.info("Sending login to repeater %s", contact.public_key[:12])
|
||||
login_result = await mc.commands.send_login(contact.public_key, password)
|
||||
logger.info("Sending login to repeater %s", contact.public_key[:12])
|
||||
login_result = await mc.commands.send_login(contact.public_key, password)
|
||||
|
||||
if login_result.type == EventType.ERROR:
|
||||
raise HTTPException(status_code=401, detail=f"Login failed: {login_result.payload}")
|
||||
if login_result.type == EventType.ERROR:
|
||||
return RepeaterLoginResponse(
|
||||
status="error",
|
||||
authenticated=False,
|
||||
message=f"{REPEATER_LOGIN_SEND_FAILED_MESSAGE} ({login_result.payload})",
|
||||
)
|
||||
|
||||
# Wait for key exchange to complete before sending requests
|
||||
logger.debug("Waiting %.1fs for key exchange to complete", REPEATER_OP_DELAY_SECONDS)
|
||||
await asyncio.sleep(REPEATER_OP_DELAY_SECONDS)
|
||||
try:
|
||||
return await asyncio.wait_for(
|
||||
login_future,
|
||||
timeout=REPEATER_LOGIN_RESPONSE_TIMEOUT_SECONDS,
|
||||
)
|
||||
except asyncio.TimeoutError:
|
||||
logger.warning(
|
||||
"No login response from repeater %s within %.1fs",
|
||||
contact.public_key[:12],
|
||||
REPEATER_LOGIN_RESPONSE_TIMEOUT_SECONDS,
|
||||
)
|
||||
return RepeaterLoginResponse(
|
||||
status="timeout",
|
||||
authenticated=False,
|
||||
message=REPEATER_LOGIN_TIMEOUT_MESSAGE,
|
||||
)
|
||||
except HTTPException as exc:
|
||||
logger.warning(
|
||||
"Repeater login setup failed for %s: %s",
|
||||
contact.public_key[:12],
|
||||
exc.detail,
|
||||
)
|
||||
return RepeaterLoginResponse(
|
||||
status="error",
|
||||
authenticated=False,
|
||||
message=f"{REPEATER_LOGIN_SEND_FAILED_MESSAGE} ({exc.detail})",
|
||||
)
|
||||
finally:
|
||||
success_subscription.unsubscribe()
|
||||
failed_subscription.unsubscribe()
|
||||
|
||||
|
||||
def _require_repeater(contact: Contact) -> None:
|
||||
@@ -180,7 +250,7 @@ def _require_repeater(contact: Contact) -> None:
|
||||
|
||||
@router.post("/{public_key}/repeater/login", response_model=RepeaterLoginResponse)
|
||||
async def repeater_login(public_key: str, request: RepeaterLoginRequest) -> RepeaterLoginResponse:
|
||||
"""Log in to a repeater. Adds contact to radio, sends login, waits for key exchange."""
|
||||
"""Attempt repeater login and report whether auth was confirmed."""
|
||||
require_connected()
|
||||
contact = await _resolve_contact_or_404(public_key)
|
||||
_require_repeater(contact)
|
||||
@@ -190,9 +260,7 @@ async def repeater_login(public_key: str, request: RepeaterLoginRequest) -> Repe
|
||||
pause_polling=True,
|
||||
suspend_auto_fetch=True,
|
||||
) as mc:
|
||||
await prepare_repeater_connection(mc, contact, request.password)
|
||||
|
||||
return RepeaterLoginResponse(status="ok")
|
||||
return await prepare_repeater_connection(mc, contact, request.password)
|
||||
|
||||
|
||||
@router.post("/{public_key}/repeater/status", response_model=RepeaterStatusResponse)
|
||||
|
||||
@@ -76,6 +76,17 @@ function createInitialPaneData(): PaneData {
|
||||
|
||||
const repeaterDashboardCache = new Map<string, RepeaterDashboardCacheEntry>();
|
||||
|
||||
function getLoginToastTitle(status: string): string {
|
||||
switch (status) {
|
||||
case 'timeout':
|
||||
return 'Login confirmation not heard';
|
||||
case 'error':
|
||||
return 'Login not confirmed';
|
||||
default:
|
||||
return 'Repeater login not confirmed';
|
||||
}
|
||||
}
|
||||
|
||||
function clonePaneData(data: PaneData): PaneData {
|
||||
return { ...data };
|
||||
}
|
||||
@@ -255,13 +266,22 @@ export function useRepeaterDashboard(
|
||||
setLoginLoading(true);
|
||||
setLoginError(null);
|
||||
try {
|
||||
await api.repeaterLogin(publicKey, password);
|
||||
const result = await api.repeaterLogin(publicKey, password);
|
||||
if (activeIdRef.current !== conversationId) return;
|
||||
setLoggedIn(true);
|
||||
if (!result.authenticated) {
|
||||
const msg = result.message ?? 'Repeater login was not confirmed';
|
||||
setLoginError(msg);
|
||||
toast.error(getLoginToastTitle(result.status), { description: msg });
|
||||
}
|
||||
} catch (err) {
|
||||
if (activeIdRef.current !== conversationId) return;
|
||||
const msg = err instanceof Error ? err.message : 'Login failed';
|
||||
setLoggedIn(true);
|
||||
setLoginError(msg);
|
||||
toast.error('Login request failed', {
|
||||
description: `${msg}. The dashboard is still available, but repeater operations may fail until a login succeeds.`,
|
||||
});
|
||||
} finally {
|
||||
if (activeIdRef.current === conversationId) {
|
||||
setLoginLoading(false);
|
||||
|
||||
@@ -35,6 +35,8 @@ vi.mock('../components/ui/sonner', () => ({
|
||||
// Get mock reference — cast to Record<string, Mock> for type-safe mock method access
|
||||
const { api: _rawApi } = await import('../api');
|
||||
const mockApi = _rawApi as unknown as Record<string, Mock>;
|
||||
const { toast } = await import('../components/ui/sonner');
|
||||
const mockToast = toast as unknown as Record<string, Mock>;
|
||||
|
||||
const REPEATER_KEY = 'aa'.repeat(32);
|
||||
|
||||
@@ -58,7 +60,11 @@ describe('useRepeaterDashboard', () => {
|
||||
});
|
||||
|
||||
it('login sets loggedIn on success', async () => {
|
||||
mockApi.repeaterLogin.mockResolvedValueOnce({ status: 'ok' });
|
||||
mockApi.repeaterLogin.mockResolvedValueOnce({
|
||||
status: 'ok',
|
||||
authenticated: true,
|
||||
message: null,
|
||||
});
|
||||
|
||||
const { result } = renderHook(() => useRepeaterDashboard(repeaterConversation));
|
||||
|
||||
@@ -72,7 +78,11 @@ describe('useRepeaterDashboard', () => {
|
||||
});
|
||||
|
||||
it('login sets error on failure', async () => {
|
||||
mockApi.repeaterLogin.mockRejectedValueOnce(new Error('Auth failed'));
|
||||
mockApi.repeaterLogin.mockResolvedValueOnce({
|
||||
status: 'error',
|
||||
authenticated: false,
|
||||
message: 'Auth failed',
|
||||
});
|
||||
|
||||
const { result } = renderHook(() => useRepeaterDashboard(repeaterConversation));
|
||||
|
||||
@@ -80,12 +90,19 @@ describe('useRepeaterDashboard', () => {
|
||||
await result.current.login('bad');
|
||||
});
|
||||
|
||||
expect(result.current.loggedIn).toBe(false);
|
||||
expect(result.current.loggedIn).toBe(true);
|
||||
expect(result.current.loginError).toBe('Auth failed');
|
||||
expect(mockToast.error).toHaveBeenCalledWith('Login not confirmed', {
|
||||
description: 'Auth failed',
|
||||
});
|
||||
});
|
||||
|
||||
it('loginAsGuest calls login with empty password', async () => {
|
||||
mockApi.repeaterLogin.mockResolvedValueOnce({ status: 'ok' });
|
||||
mockApi.repeaterLogin.mockResolvedValueOnce({
|
||||
status: 'ok',
|
||||
authenticated: true,
|
||||
message: null,
|
||||
});
|
||||
|
||||
const { result } = renderHook(() => useRepeaterDashboard(repeaterConversation));
|
||||
|
||||
@@ -97,6 +114,23 @@ describe('useRepeaterDashboard', () => {
|
||||
expect(result.current.loggedIn).toBe(true);
|
||||
});
|
||||
|
||||
it('login still opens dashboard when request rejects', async () => {
|
||||
mockApi.repeaterLogin.mockRejectedValueOnce(new Error('Network error'));
|
||||
|
||||
const { result } = renderHook(() => useRepeaterDashboard(repeaterConversation));
|
||||
|
||||
await act(async () => {
|
||||
await result.current.login('secret');
|
||||
});
|
||||
|
||||
expect(result.current.loggedIn).toBe(true);
|
||||
expect(result.current.loginError).toBe('Network error');
|
||||
expect(mockToast.error).toHaveBeenCalledWith('Login request failed', {
|
||||
description:
|
||||
'Network error. The dashboard is still available, but repeater operations may fail until a login succeeds.',
|
||||
});
|
||||
});
|
||||
|
||||
it('refreshPane stores data on success', async () => {
|
||||
const statusData = {
|
||||
battery_volts: 4.2,
|
||||
@@ -376,7 +410,11 @@ describe('useRepeaterDashboard', () => {
|
||||
|
||||
it('restores dashboard state when navigating away and back to the same repeater', async () => {
|
||||
const statusData = { battery_volts: 4.2 };
|
||||
mockApi.repeaterLogin.mockResolvedValueOnce({ status: 'ok' });
|
||||
mockApi.repeaterLogin.mockResolvedValueOnce({
|
||||
status: 'ok',
|
||||
authenticated: true,
|
||||
message: null,
|
||||
});
|
||||
mockApi.repeaterStatus.mockResolvedValueOnce(statusData);
|
||||
mockApi.sendRepeaterCommand.mockResolvedValueOnce({
|
||||
command: 'ver',
|
||||
|
||||
@@ -349,6 +349,8 @@ export interface CommandResponse {
|
||||
|
||||
export interface RepeaterLoginResponse {
|
||||
status: string;
|
||||
authenticated: boolean;
|
||||
message: string | null;
|
||||
}
|
||||
|
||||
export interface RepeaterStatusResponse {
|
||||
|
||||
@@ -6,13 +6,14 @@ import pytest
|
||||
from fastapi import HTTPException
|
||||
from meshcore import EventType
|
||||
|
||||
from app.models import CommandRequest, Contact, RepeaterLoginRequest
|
||||
from app.models import CommandRequest, Contact, RepeaterLoginRequest, RepeaterLoginResponse
|
||||
from app.radio import radio_manager
|
||||
from app.repository import ContactRepository
|
||||
from app.routers.contacts import request_trace
|
||||
from app.routers.repeaters import (
|
||||
_batch_cli_fetch,
|
||||
_fetch_repeater_response,
|
||||
prepare_repeater_connection,
|
||||
repeater_acl,
|
||||
repeater_advert_intervals,
|
||||
repeater_login,
|
||||
@@ -73,6 +74,7 @@ async def _insert_contact(public_key: str, name: str = "Node", contact_type: int
|
||||
def _mock_mc():
|
||||
mc = MagicMock()
|
||||
mc.commands = MagicMock()
|
||||
mc.commands.send_login = AsyncMock(return_value=_radio_result(EventType.MSG_SENT))
|
||||
mc.commands.req_status_sync = AsyncMock()
|
||||
mc.commands.fetch_all_neighbours = AsyncMock()
|
||||
mc.commands.req_acl_sync = AsyncMock()
|
||||
@@ -82,6 +84,7 @@ def _mock_mc():
|
||||
mc.commands.add_contact = AsyncMock(return_value=_radio_result(EventType.OK))
|
||||
mc.commands.send_trace = AsyncMock(return_value=_radio_result(EventType.OK))
|
||||
mc.wait_for_event = AsyncMock()
|
||||
mc.subscribe = MagicMock(return_value=MagicMock(unsubscribe=MagicMock()))
|
||||
mc.stop_auto_message_fetching = AsyncMock()
|
||||
mc.start_auto_message_fetching = AsyncMock()
|
||||
return mc
|
||||
@@ -537,9 +540,16 @@ class TestRepeaterLogin:
|
||||
new_callable=AsyncMock,
|
||||
) as mock_prepare,
|
||||
):
|
||||
mock_prepare.return_value = RepeaterLoginResponse(
|
||||
status="ok",
|
||||
authenticated=True,
|
||||
message=None,
|
||||
)
|
||||
response = await repeater_login(KEY_A, RepeaterLoginRequest(password="secret"))
|
||||
|
||||
assert response.status == "ok"
|
||||
assert response.authenticated is True
|
||||
assert response.message is None
|
||||
mock_prepare.assert_awaited_once()
|
||||
|
||||
@pytest.mark.asyncio
|
||||
@@ -567,21 +577,89 @@ class TestRepeaterLogin:
|
||||
assert "not a repeater" in exc.value.detail.lower()
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_login_error_raises(self, test_db):
|
||||
async def test_login_error_returns_warning_response(self, test_db):
|
||||
mc = _mock_mc()
|
||||
await _insert_contact(KEY_A, name="Repeater", contact_type=2)
|
||||
|
||||
async def _prepare_fail(*args, **kwargs):
|
||||
raise HTTPException(status_code=401, detail="Login failed")
|
||||
return RepeaterLoginResponse(
|
||||
status="error",
|
||||
authenticated=False,
|
||||
message="Login failed",
|
||||
)
|
||||
|
||||
with (
|
||||
patch("app.routers.repeaters.require_connected", return_value=mc),
|
||||
patch.object(radio_manager, "_meshcore", mc),
|
||||
patch("app.routers.repeaters.prepare_repeater_connection", side_effect=_prepare_fail),
|
||||
):
|
||||
with pytest.raises(HTTPException) as exc:
|
||||
await repeater_login(KEY_A, RepeaterLoginRequest(password="bad"))
|
||||
assert exc.value.status_code == 401
|
||||
response = await repeater_login(KEY_A, RepeaterLoginRequest(password="bad"))
|
||||
assert response.status == "error"
|
||||
assert response.authenticated is False
|
||||
assert response.message == "Login failed"
|
||||
|
||||
|
||||
class TestPrepareRepeaterConnection:
|
||||
@pytest.mark.asyncio
|
||||
async def test_returns_success_when_login_confirmed(self):
|
||||
mc = _mock_mc()
|
||||
contact = _make_contact()
|
||||
subscriptions: dict[EventType, tuple[object, object]] = {}
|
||||
|
||||
def _subscribe(event_type, callback, attribute_filters=None):
|
||||
subscriptions[event_type] = (callback, attribute_filters)
|
||||
return MagicMock(unsubscribe=MagicMock())
|
||||
|
||||
async def _send_login(*args, **kwargs):
|
||||
callback, filters = subscriptions[EventType.LOGIN_SUCCESS]
|
||||
assert filters == {"pubkey_prefix": KEY_A[:12]}
|
||||
callback(_radio_result(EventType.LOGIN_SUCCESS, {"pubkey_prefix": KEY_A[:12]}))
|
||||
return _radio_result(EventType.MSG_SENT)
|
||||
|
||||
mc.subscribe = MagicMock(side_effect=_subscribe)
|
||||
mc.commands.send_login = AsyncMock(side_effect=_send_login)
|
||||
|
||||
response = await prepare_repeater_connection(mc, contact, "secret")
|
||||
|
||||
assert response.status == "ok"
|
||||
assert response.authenticated is True
|
||||
assert response.message is None
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_returns_error_when_login_rejected(self):
|
||||
mc = _mock_mc()
|
||||
contact = _make_contact()
|
||||
subscriptions: dict[EventType, tuple[object, object]] = {}
|
||||
|
||||
def _subscribe(event_type, callback, attribute_filters=None):
|
||||
subscriptions[event_type] = (callback, attribute_filters)
|
||||
return MagicMock(unsubscribe=MagicMock())
|
||||
|
||||
async def _send_login(*args, **kwargs):
|
||||
callback, _filters = subscriptions[EventType.LOGIN_FAILED]
|
||||
callback(_radio_result(EventType.LOGIN_FAILED, {"pubkey_prefix": KEY_A[:12]}))
|
||||
return _radio_result(EventType.MSG_SENT)
|
||||
|
||||
mc.subscribe = MagicMock(side_effect=_subscribe)
|
||||
mc.commands.send_login = AsyncMock(side_effect=_send_login)
|
||||
|
||||
response = await prepare_repeater_connection(mc, contact, "bad")
|
||||
|
||||
assert response.status == "error"
|
||||
assert response.authenticated is False
|
||||
assert "did not confirm this login" in (response.message or "")
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_returns_timeout_when_no_login_response(self):
|
||||
mc = _mock_mc()
|
||||
contact = _make_contact()
|
||||
|
||||
with patch("app.routers.repeaters.REPEATER_LOGIN_RESPONSE_TIMEOUT_SECONDS", 0):
|
||||
response = await prepare_repeater_connection(mc, contact, "secret")
|
||||
|
||||
assert response.status == "timeout"
|
||||
assert response.authenticated is False
|
||||
assert "No login confirmation was heard from the repeater" in (response.message or "")
|
||||
|
||||
|
||||
class TestRepeaterStatus:
|
||||
|
||||
Reference in New Issue
Block a user