diff --git a/src/dashboards/sources/nut.py b/src/dashboards/sources/nut.py index 57376b8..6c4ec4a 100644 --- a/src/dashboards/sources/nut.py +++ b/src/dashboards/sources/nut.py @@ -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 diff --git a/tests/test_nut.py b/tests/test_nut.py index 7ba0039..3a473b1 100644 --- a/tests/test_nut.py +++ b/tests/test_nut.py @@ -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()