Test and sync: a refused password is reported as “the server names no principal”, in English, and a test asserts it #28

New issue

Closed

opened 2026-10-01 10:47:55 +00:00 by tiagoagueda · 1 comment

tiagoagueda commented

2026-10-01 10:47:55 +00:00

Owner

Found on 2026-09-29 in a read-only review of postulo-dav at 97abc9779, following a 401 from the server through principal() to what the person is shown.

Related to #7.

A wrong or revoked password is the most likely thing to go wrong with this connection, and the plugin describes it as a discovery problem, in an untranslated sentence with a class name in front. DavError is documented as "The message is for the person" (src/postulo_dav/dav.py:69-70), and a plugin translates its own strings, but none of its messages go through gettext.

Where: src/postulo_dav/dav.py:126-137 (Server.principal), src/postulo_dav/dav.py:90-91 (_explain), :122 and :150 (the other DavError sentences), src/postulo_dav/sync.py:354-355 (DavSync.test), tests/test_dav.py:376-378.
What happens: principal() tries /.well-known/<kind> and /, and except DavError: continue discards every failure, a 401 or 403 included. With nothing found it raises DavError(f"the server names no principal for {well_known}"). Run with a wrong password against the suite's server: both requests answered 401, Test showed DavError: the server names no principal for carddav, and DavSync().sync raised the same error, which the core stores on the connection as DavError: the server names no principal for carddav (plugins/syncing.py:85 prefixes the class name). test_a_wrong_password_is_a_sentence asserts "principal" in result.message, so the misleading wording is what the suite holds in place.
Why it matters: the person is sent looking at the server address when the password is the problem, and reads it in English whatever their language.
Fix:
- In principal(), treat 401 and 403 as an answer, not as "try the next place": raise at once a DavError subclass whose message is a translated sentence saying the server refused the user name or password. Keep the status code on the error so the caller need not parse text.
- Pass the other DavError sentences through gettext (_explain, "unreadable answer", "names no principal", "names no … home"), and add them to the catalogues.
- In DavSync.test, show str(error) for a DavError and keep the class-name prefix only for an exception nobody expected. In DavSync.sync, catch DavError and return SyncReport(error=str(error)), so the core does not add DavError: in front.
- Do not raise the core's ConnectionUnusable from sync: sync_connection does not handle it (only documents/archiving.py:140 and notifications/service.py:73 do), so it would be shown as one more class name.
- Change the test to assert the authentication sentence, and that the word "principal" is not in it.

Done when

tests/test_dav.py::test_a_wrong_password_is_a_sentence asserts a message about the user name or password, with no class name, for both test() and sync(), and a second assertion under translation.override("fr-fr") sees the French sentence.
uv run postulo-messages extract lists the new strings, with fr-fr and pt-pt filled.

Found on 2026-09-29 in a read-only review of postulo-dav at 97abc9779, following a 401 from the server through `principal()` to what the person is shown. Related to #7. A wrong or revoked password is the most likely thing to go wrong with this connection, and the plugin describes it as a discovery problem, in an untranslated sentence with a class name in front. `DavError` is documented as "The message is for the person" (`src/postulo_dav/dav.py:69-70`), and a plugin translates its own strings, but none of its messages go through gettext. - **Where:** `src/postulo_dav/dav.py:126-137` (`Server.principal`), `src/postulo_dav/dav.py:90-91` (`_explain`), `:122` and `:150` (the other `DavError` sentences), `src/postulo_dav/sync.py:354-355` (`DavSync.test`), `tests/test_dav.py:376-378`. - **What happens:** `principal()` tries `/.well-known/<kind>` and `/`, and `except DavError: continue` discards every failure, a 401 or 403 included. With nothing found it raises `DavError(f"the server names no principal for {well_known}")`. Run with a wrong password against the suite's server: both requests answered 401, *Test* showed `DavError: the server names no principal for carddav`, and `DavSync().sync` raised the same error, which the core stores on the connection as `DavError: the server names no principal for carddav` (`plugins/syncing.py:85` prefixes the class name). `test_a_wrong_password_is_a_sentence` asserts `"principal" in result.message`, so the misleading wording is what the suite holds in place. - **Why it matters:** the person is sent looking at the server address when the password is the problem, and reads it in English whatever their language. - **Fix:** - In `principal()`, treat 401 and 403 as an answer, not as "try the next place": raise at once a `DavError` subclass whose message is a translated sentence saying the server refused the user name or password. Keep the status code on the error so the caller need not parse text. - Pass the other `DavError` sentences through gettext (`_explain`, "unreadable answer", "names no principal", "names no … home"), and add them to the catalogues. - In `DavSync.test`, show `str(error)` for a `DavError` and keep the class-name prefix only for an exception nobody expected. In `DavSync.sync`, catch `DavError` and return `SyncReport(error=str(error))`, so the core does not add `DavError:` in front. - Do not raise the core's `ConnectionUnusable` from `sync`: `sync_connection` does not handle it (only `documents/archiving.py:140` and `notifications/service.py:73` do), so it would be shown as one more class name. - Change the test to assert the authentication sentence, and that the word "principal" is not in it. ## Done when - `tests/test_dav.py::test_a_wrong_password_is_a_sentence` asserts a message about the user name or password, with no class name, for both `test()` and `sync()`, and a second assertion under `translation.override("fr-fr")` sees the French sentence. - `uv run postulo-messages extract` lists the new strings, with fr-fr and pt-pt filled.

tiagoagueda added this to the 0.6.0 milestone

2026-10-01 10:47:55 +00:00

tiagoagueda added the

bug

label

2026-10-01 10:47:55 +00:00

tiagoagueda referenced this issue from a commit

2026-10-19 07:13:10 +00:00

Review fixes: pairing by UID, bounded reads, per-resource tolerance, notes per reader, and a calendar that carries no attendee

tiagoagueda referenced this issue

2026-10-19 07:13:15 +00:00

Review fixes: pairing by UID, bounded reads, notes per reader, per-resource tolerance #35