Skip to content

Commit

Permalink
pybridge: Improve beiboot error reporting for fatal login failures
Browse files Browse the repository at this point in the history
After all SSH authentication attemps fail, e.g. entering the wrong
password three times, ferny throws an InteractionError. Merely exiting
cockpit-beiboot gets interpreted as "Internal error" which is unfriendly.

Translate the most common errors (DNS resolution and and authentication
failure) to proper cockpit protocol error codes, so that they get
presented properly and translated. As this involves parsing SSH output,
keep the `internal-error` fallback for everything else.
  • Loading branch information
martinpitt committed Sep 28, 2023
1 parent 5280ff8 commit f21866b
Show file tree
Hide file tree
Showing 3 changed files with 19 additions and 12 deletions.
13 changes: 11 additions & 2 deletions src/cockpit/beiboot.py
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,6 @@
import logging
import os
import shlex
import sys
from pathlib import Path
from typing import Dict, Iterable, Optional, Sequence

Expand Down Expand Up @@ -316,8 +315,18 @@ async def run(args) -> None:
bridge.write_control(**message)
bridge.ssh_peer.thaw_endpoint()
except ferny.InteractionError as exc:
sys.exit(str(exc))
logger.debug("ferny.InteractionError: %s", exc)
msg = str(exc)
if 'Permission denied' in msg:
problem = 'authentication-failed'
elif 'resolve host' in msg:
problem = 'unknown-host'
else:
problem = 'internal-error'
bridge.write_control(command='init', problem=problem, message=msg)
return
except CockpitProblem as exc:
logger.debug("CockpitProblem: %s", exc)
bridge.write_control(command='init', problem=exc.problem, **exc.kwargs)
return

Expand Down
13 changes: 5 additions & 8 deletions test/verify/check-client
Original file line number Diff line number Diff line change
Expand Up @@ -147,13 +147,11 @@ Command = /usr/bin/env python3 -m cockpit.beiboot
b.wait_text("#conversation-prompt", "admin@10.111.113.2's password: ")
b.set_val("#conversation-input", "wrong")
b.click("#login-button")
b.wait_in_text("#login-fatal-message", "admin@10.111.113.2: Permission denied")
b.click("#login-again")
b.wait_text("#brand", "Connect to:")
# resets the host field
b.wait_val("#server-field", "")
b.wait_in_text("#login-error-message", "Authentication failed")
b.wait_val("#server-field", "10.111.113.2")

# connect to most recent host
b.open("/") # reset URL from /metrics and last remote =host
b.click("#recent-hosts-list .host-line button.host-name")
b.wait_text("#conversation-prompt", "admin@10.111.113.2's password: ")
b.set_val("#conversation-input", "foobar")
Expand Down Expand Up @@ -208,9 +206,8 @@ Command = /usr/bin/env python3 -m cockpit.beiboot
# unreachable host
b.set_val("#server-field", "unknownhost")
b.click("#login-button")
b.wait_in_text("#login-fatal-message", "Could not resolve hostname unknownhost")
b.click("#login-again")
b.wait_text("#brand", "Connect to:")
b.wait_in_text("#login-error-message", "Host is unknown")
b.wait_val("#server-field", "unknownhost")
# does not appear in recent hosts
b.wait_in_text("#recent-hosts-list", "10.111.113.2")
self.assertNotIn("unknownhost", b.text("#recent-hosts-list"))
Expand Down
5 changes: 3 additions & 2 deletions test/verify/check-static-login
Original file line number Diff line number Diff line change
Expand Up @@ -1017,9 +1017,10 @@ Command = /usr/bin/env python3 -m cockpit.beiboot
b.wait_text("#conversation-prompt", f"admin@{my_ip}'s password: ")
b.set_val("#conversation-input", "wrong")
b.click("#login-button")
# FIXME: better error message
b.wait_in_text("#login-fatal-message", "Internal error in login process")
b.wait_in_text("#login-error-message", "Authentication failed")
check_no_processes()
# goes back to normal login form
b.wait_visible('#login-user-input')

# colliding usernames; user names in "Connect to:" are *not* supported,
# but pin down the behaviour
Expand Down

0 comments on commit f21866b

Please sign in to comment.