fix(nut): log in with the verbs upsd 2.8.5 actually answers to
`NutClient.login()` sent `USER`/`PASS`. The deployed upsd 2.8.5 implements neither — its own `HELP` advertises `USERNAME`/`PASSWORD` — so each line answered `ERR UNKNOWN-COMMAND`, `_read_line()` raised, and `POST /api/ups/command` could only ever 502 before `instcmd()`, credentials or not. Values go out unquoted: upsd takes the rest of the line literally, so quoting them would make the quotes part of the credential. `ERROR_HINTS["ACCESS-DENIED"]` blamed the upsd.conf ACL/LISTEN lines, but that code is only reachable on an authenticated session (an unauthenticated client gets `ERR USERNAME-REQUIRED`), so the cause is the account's `instcmds` grant. The hint now names the one thing the operator can change. The fake upsd in tests has no `USER`/`PASS` branch, so a regression answers `ERR UNKNOWN-COMMAND` exactly like the real server, and its `PASSWORD` branch checks the secret so a wrong-credential refusal is expressible. LLM-Generated: true
This commit is contained in:
@@ -54,8 +54,13 @@ class NutError(Exception):
|
||||
|
||||
#: upsd error tokens -> what the operator should actually do about them.
|
||||
ERROR_HINTS = {
|
||||
"ACCESS-DENIED": "upsd refused: this client is not in an ACL that allows it (upsd.conf `ACL`/"
|
||||
"`LISTEN`, or the user lacks the privilege in upsd.users)",
|
||||
# Reached only on an *authenticated* session — an unauthenticated client gets
|
||||
# ERR USERNAME-REQUIRED instead. So the cause is the account's privileges, never the
|
||||
# upsd.conf ACL/LISTEN lines that used to be blamed here, and the sentence has to name
|
||||
# the one thing the operator can actually change.
|
||||
"ACCESS-DENIED": "upsd accepted the login but the account may not send this: an instant "
|
||||
"command needs its own `instcmds` line in upsd.users (or `instcmds = all`), "
|
||||
"and nut-server must restart to pick the change up",
|
||||
"INVALID-PASSWORD": "wrong NUT_PASSWORD for NUT_USER (upsd.users)",
|
||||
"INVALID-USERNAME": "NUT_USER is not defined in upsd.users",
|
||||
"INVALID-ARGUMENT": "malformed command or unknown UPS",
|
||||
@@ -181,12 +186,20 @@ class NutClient:
|
||||
raise NutError(f"{kind} list too long", code="UNEXPECTED")
|
||||
|
||||
async def login(self) -> bool:
|
||||
"""Authenticate for actions. Returns False when no credentials are configured."""
|
||||
"""Authenticate for actions. Returns False when no credentials are configured.
|
||||
|
||||
`USERNAME` + `PASSWORD`, never `USER`/`PASS`. Captured from the deployed upsd 2.8.5,
|
||||
whose own `HELP` answers `Commands: HELP VER PROTVER GET LIST SET INSTCMD LOGIN LOGOUT
|
||||
USERNAME PASSWORD STARTTLS` and replies `ERR UNKNOWN-COMMAND` to both `USER` and
|
||||
`PASS` — with those verbs the login raised here and `POST /api/ups/command` could only
|
||||
ever 502, credentials or not. Values go out **unquoted**: upsd takes the rest of the
|
||||
line literally, so quoting them would make the quotes part of the credential.
|
||||
"""
|
||||
if not self.username or not self.password:
|
||||
return False
|
||||
await self._send(f"USER {self.username}")
|
||||
await self._send(f"USERNAME {self.username}")
|
||||
await self._read_line()
|
||||
await self._send(f"PASS {self.password}")
|
||||
await self._send(f"PASSWORD {self.password}")
|
||||
await self._read_line()
|
||||
return True
|
||||
|
||||
|
||||
+43
-3
@@ -104,12 +104,20 @@ class FakeUpsd:
|
||||
send("END LIST RW")
|
||||
else:
|
||||
send("ERR UNKNOWN-COMMAND")
|
||||
elif verb == "USER":
|
||||
elif verb == "USERNAME":
|
||||
self.user = rest
|
||||
send("OK")
|
||||
elif verb == "PASS":
|
||||
self.authenticated = not self.require_auth or getattr(self, "user", "") == "monitor"
|
||||
elif verb == "PASSWORD":
|
||||
# The password has to matter: a fake that authenticates on the username alone
|
||||
# cannot express a wrong-credential refusal, which is the commonest reason the
|
||||
# command buttons stay dead after someone fills in NUT_PASSWORD.
|
||||
self.authenticated = not self.require_auth or (
|
||||
getattr(self, "user", "") == "monitor" and rest == "pw"
|
||||
)
|
||||
send("OK" if self.authenticated else "ERR ACCESS-DENIED")
|
||||
# No USER/PASS branches on purpose: real upsd 2.8.5 has neither verb, and answering
|
||||
# ERR UNKNOWN-COMMAND here (the fallthrough below) is what catches a client that
|
||||
# starts sending them again.
|
||||
elif verb == "INSTCMD":
|
||||
if self.require_auth and not self.authenticated:
|
||||
send("ERR ACCESS-DENIED")
|
||||
@@ -190,6 +198,38 @@ async def test_instcmd_with_credentials_succeeds() -> None:
|
||||
await server.stop()
|
||||
|
||||
|
||||
async def test_login_uses_the_verbs_upsd_2_8_5_answers_to() -> None:
|
||||
"""The verbs are a protocol contract, not a style choice.
|
||||
|
||||
`USER`/`PASS` are not in the deployed upsd's `HELP` list and it answers them
|
||||
`ERR UNKNOWN-COMMAND`, which made every login fail. This pins the pair that actually
|
||||
authenticates, unquoted, and fails if either legacy verb comes back.
|
||||
"""
|
||||
server = FakeUpsd(require_auth=True)
|
||||
port = await server.start()
|
||||
try:
|
||||
async with NutClient("127.0.0.1", port, username="monitor", password="pw") as client:
|
||||
assert await client.login() is True
|
||||
finally:
|
||||
await server.stop()
|
||||
assert server.received[:2] == ["USERNAME monitor", "PASSWORD pw"]
|
||||
assert not any(line.split(" ")[0] in ("USER", "PASS") for line in server.received)
|
||||
|
||||
|
||||
async def test_login_reports_a_rejected_password_as_an_upsd_error() -> None:
|
||||
"""A bad credential must surface as the server's refusal, not a silent False."""
|
||||
server = FakeUpsd(require_auth=True)
|
||||
port = await server.start()
|
||||
try:
|
||||
async with NutClient("127.0.0.1", port, username="monitor", password="wrong") as client:
|
||||
with pytest.raises(NutError) as excinfo:
|
||||
await client.login()
|
||||
finally:
|
||||
await server.stop()
|
||||
assert excinfo.value.code == "ACCESS-DENIED"
|
||||
assert server.received[:1] == ["USERNAME monitor"]
|
||||
|
||||
|
||||
async def test_upsd_rejection_carries_the_error_code() -> None:
|
||||
server = FakeUpsd(require_auth=True)
|
||||
port = await server.start()
|
||||
|
||||
Reference in New Issue
Block a user