diff --git a/meshterm/core/room_store.py b/meshterm/core/room_store.py index 6672dfd..c2c8d60 100644 --- a/meshterm/core/room_store.py +++ b/meshterm/core/room_store.py @@ -40,8 +40,9 @@ class RoomMembership: key: The :func:`~meshterm.core.admin_store.admin_key` the room is stored under — the same key its admin password has, so the two stores agree on which room is which (the record's key in the file, not one of its fields). - password: The room password that got us in, ``""`` for a room open to anyone, or - ``None`` when we got in as its admin (that password lives in the admin store). + password: The room password that got us in, ``""`` for a blank login the room + accepted (an open room, or one that knew us), or ``None`` when no room password + is known — we got in as its admin, whose password lives in the admin store. access: The access the room granted at our last login: a :class:`~meshterm.core.models.RoomAccess` value. label: The room's name, for display. @@ -136,27 +137,50 @@ class RoomStore: record`), for the room password: kept when the room accepts it, forgotten when the node *refuses* it, and left as it is when nothing came back — silence is not a verdict, and from a room it is also how a wrong password sounds, so a password that - never once worked is simply never written. An admin login is remembered as the - access alone; its password is the admin store's to keep. + never once worked is simply never written. + + "Accepted" is not one proof, though, and a password is only kept for what it proved: + + * **Admin** with a password typed in: the password is the admin store's to keep + (it is the room's admin password); here, only the access, beside whatever room + password was already remembered. + * **Member** with a password typed in: that password is the room password. + * **Read-only** proves nothing about the password at all — a room that lets + readers in lets *any* password in — so it never replaces one already + remembered, which may be a member's; it is kept only where nothing was. + * **Blank** asks "do you know me?" and proves only that the room does. It never + replaces a remembered password either: a member the room forgets on its next + restart would otherwise have swapped a password that works for one that can't. Args: room: The room the login addressed. password: The password that was tried. login: How the login ended. """ + records = self._load() + key = admin_key(room) + known = records.get(key) if login.result is LoginResult.ACCEPTED and login.access is not None: - records = self._load() - key = admin_key(room) + kept = known.password if known is not None else None + if login.access is RoomAccess.ADMIN and password: + remembered = kept + elif login.access is RoomAccess.MEMBER and password: + remembered = password + else: + remembered = kept if kept is not None else password records[key] = RoomMembership( key=key, - password=None if login.access is RoomAccess.ADMIN else password, + password=remembered, access=login.access.value, label=room.name, last_login=utcnow().isoformat(), ) self._write(records) - elif login.result is LoginResult.REFUSED: - self.forget(room) + elif login.result is LoginResult.REFUSED and known is not None: + # Only the password the node turned down is forgotten. + if known.password == password: + records.pop(key) + self._write(records) def forget(self, room: Contact) -> None: """Remove everything remembered about ``room``. diff --git a/meshterm/services/rooms.py b/meshterm/services/rooms.py index f4c8a3a..ff4ef23 100644 --- a/meshterm/services/rooms.py +++ b/meshterm/services/rooms.py @@ -21,6 +21,7 @@ Session-scoped state on the :class:`~meshterm.context.AppContext` (``ctx.rooms`` from __future__ import annotations +import asyncio import time from typing import TYPE_CHECKING @@ -45,6 +46,8 @@ class RoomService: #: When each room was last heard from this session (a login or a post), on the #: monotonic clock, keyed by room peer. self._heard: dict[str, float] = {} + #: The login on its way to each room, and the password it carries, keyed by peer. + self._inflight: dict[str, tuple[asyncio.Future, str]] = {} @staticmethod def _peer(room: Contact) -> str: @@ -54,14 +57,17 @@ class RoomService: def password(self, room: Contact) -> str | None: """The password to log in to ``room`` with, or ``None`` when we hold none. - The room password first — it is what a member was given, and what got us in — - then the room's admin password, which Repeater admin may already remember and - which gets us in as its admin. ``""`` is a password (an open room). + The room's admin password first, when Repeater admin (or a join) remembers one: it + gets us everything the room password does and more. The order is not taste — a + room's login *replaces* a known member's role with the one the password earns, so + an owner who logged in with the room password would be demoted to member until + their next admin login. Then the room password. ``""`` is a password (a blank + login the room accepted). """ - stored = self._ctx.room_store.password(room) if self._ctx.room_store else None - if stored is not None: - return stored - return self._ctx.admin_store.get(room) + admin = self._ctx.admin_store.get(room) + if admin: + return admin + return self._ctx.room_store.password(room) if self._ctx.room_store else None def joined(self, room: Contact) -> bool: """Whether MeshTerm can log in to ``room`` without asking for a password.""" @@ -100,18 +106,32 @@ class RoomService: ) def login_due(self, room: Contact) -> bool: - """Whether opening ``room`` should log in to it: a password at hand and a quiet room.""" - return self.joined(room) and not self.heard_recently(room) + """Whether opening ``room`` should log in: a password at hand, and a quiet room. + + Never while a login to it is already on its way — that one's answer is this one's. + """ + return ( + self.joined(room) + and not self.heard_recently(room) + and self._peer(room) not in self._inflight + ) async def login(self, room: Contact, password: str) -> RoomLogin: """Log in to ``room`` with ``password`` — one exchange — and remember the outcome. An accepted login is remembered the way each password is kept: the access and the - room password in the room store, an admin password in the admin store (shared with - Repeater admin, which will log in with it silently from now on). A login the node - *refused* forgets the password that was tried; one that met silence keeps - everything, since a room also stays silent for a wrong password and only a - password that once worked is ever stored. + room password in the room store (:meth:`~meshterm.core.room_store.RoomStore. + record` says what each kind of acceptance proves), and an admin password typed in + in the admin store, shared with Repeater admin, which logs in with it silently from + then on. A login the node *refused* forgets the password that was tried; one that + met silence keeps everything, since a room also stays silent for a wrong password + and only a password that once worked is ever stored. + + One login to a room at a time, whoever asks. A login can take most of its reply + budget, and a room view closed and opened again inside it would otherwise send a + second one — both of which the room answers by restarting its catch-up. A caller + asking with the password already on its way shares that login's answer; one with + a different password waits for it to finish, then sends its own. Args: room: The room server to log in to. @@ -124,11 +144,38 @@ class RoomService: Exception: Propagates a device error (no connection, a contact the companion cannot address); nothing is remembered then. """ + peer = self._peer(room) + running = self._inflight.get(peer) + if running is not None: + task, sent = running + if sent == password: + return await asyncio.shield(task) + try: + await asyncio.shield(task) + except Exception: # noqa: BLE001 - its own caller hears about its failure + pass + task = asyncio.ensure_future(self._login_once(room, password)) + self._inflight[peer] = (task, password) + # Cleared when the exchange ends, not when this caller stops waiting: a room view + # closed mid-login leaves the login running, and it is still the one in flight. + task.add_done_callback(lambda done: self._settle(peer, done)) + return await asyncio.shield(task) + + def _settle(self, peer: str, task: asyncio.Future) -> None: + """Forget a finished login as the one in flight, retrieving a failure no one awaits.""" + if self._inflight.get(peer, (None, None))[0] is task: + del self._inflight[peer] + if not task.cancelled(): + task.exception() + + async def _login_once(self, room: Contact, password: str) -> RoomLogin: + """One login exchange, and what its outcome means for the stores and the session.""" device = await self._ctx.device() login = await device.room_login(room, password) if self._ctx.room_store is not None: self._ctx.room_store.record(room, password, login) - if login.access is RoomAccess.ADMIN: + if login.access is RoomAccess.ADMIN and password: + # A blank login proves the room knows us, not what the admin password is. self._ctx.admin_store.record(room, password, LoginResult.ACCEPTED) elif login.result is LoginResult.REFUSED and password == self._ctx.admin_store.get(room): self._ctx.admin_store.record(room, password, login.result) diff --git a/meshterm/ui/chat.py b/meshterm/ui/chat.py index 83e03ab..60a7042 100644 --- a/meshterm/ui/chat.py +++ b/meshterm/ui/chat.py @@ -363,6 +363,15 @@ class ChatScreen(Screen): #: What an empty transcript says. _empty_text = "No messages yet — say hello!" + @property + def _replies(self) -> bool: + """Whether Enter on a picked message replies to it (else it opens its paths). + + Where there are many voices and an @mention to say which one is answered — and + only while there is a compose line to put the reply on. + """ + return self._multiparty and self._composing + def _compose_line(self, width: int) -> RenderableType: """The compose line under the transcript: the input, with its byte budget. @@ -690,7 +699,7 @@ class ChatScreen(Screen): message = self._messages[self._selected] sender, _ = self._sender_and_body(message) who = "this message" if message.outbound or sender == "·" else sender - if self._multiparty: + if self._replies: return Text( f"↩ Enter to reply to {who} with an @mention · End to cancel", style="accent", @@ -758,7 +767,7 @@ class ChatScreen(Screen): """ if action == "enter": if self._selected is not None: - if self._multiparty: + if self._replies: self._begin_reply() else: self._open_paths(self._selected) @@ -868,7 +877,7 @@ class ChatScreen(Screen): is one line); an empty result never opens the dialog. One paste dialog at a time, scheduled off the key handler like the paths view. """ - if self._paste_open: + if self._paste_open or not self._composing: return clean = "".join(ch if ch.isprintable() else " " for ch in data).strip() if not clean: diff --git a/meshterm/ui/room.py b/meshterm/ui/room.py index 8b5291c..b7dacaf 100644 --- a/meshterm/ui/room.py +++ b/meshterm/ui/room.py @@ -281,16 +281,29 @@ class RoomScreen(ChatScreen): def handle(self, action: str, data: str = "") -> None: """^L logs in; everything else is the chat's.""" if action == "login": - self.begin_login(ask=self._login_state in (NO_REPLY, REFUSED, NOT_JOINED)) + # Asked for, rather than sent with what is remembered, wherever that would only + # repeat the outcome on screen: after silence (a room's answer to a wrong + # password too), and when all we are is read-only — the room password is what + # lets a reader post. + ask = self._login_state in (NO_REPLY, REFUSED, NOT_JOINED) or ( + self._access is RoomAccess.READ_ONLY + ) + self.begin_login(ask=ask) self._session.invalidate() return super().handle(action, data) + def _retry_target(self) -> ChatMessage | None: + """An unacknowledged post to send again — never for a reader the room won't hear.""" + return super()._retry_target() if self._composing else None + @property def footer_hint(self) -> str: """Key hint: the chat's, in a board's words, with ^L while a login could start.""" if self._selected is not None: code = " · ^U QR" if self._picked_urls() else "" + if not self._composing: # nowhere to put a reply: Enter shows the post's paths + return f"Enter paths{code} · ↑↓ pick · ^End/Esc cancel" return f"Enter reply (@mention) · ^P paths{code} · ↑↓ pick · ^End/Esc cancel" atoms = ["Enter send"] if self._composing else [] atoms.append("↑ pick a message") diff --git a/tests/test_rooms.py b/tests/test_rooms.py index ea2e5c9..2abdc3f 100644 --- a/tests/test_rooms.py +++ b/tests/test_rooms.py @@ -316,14 +316,81 @@ class _Ctx: return self._device -def test_the_room_password_is_tried_before_the_admin_password(tmp_path: Path) -> None: - """A member's password first; the admin password, which also gets us in, after.""" +def test_the_admin_password_is_tried_before_the_room_password(tmp_path: Path) -> None: + """A login replaces a known member's role, so the room password would demote an owner.""" ctx = _Ctx(tmp_path, MockDevice()) assert ctx.rooms.password(ROOM) is None and not ctx.rooms.joined(ROOM) - ctx.admin_store.remember(ROOM, "admin") - assert ctx.rooms.password(ROOM) == "admin" ctx.room_store.record(ROOM, "hello", RoomLogin(LoginResult.ACCEPTED, RoomAccess.MEMBER)) assert ctx.rooms.password(ROOM) == "hello" + ctx.admin_store.remember(ROOM, "admin") + assert ctx.rooms.password(ROOM) == "admin" + + +def test_read_only_never_replaces_the_room_password(rooms: RoomStore) -> None: + """A room that lets readers in lets any password in: that proves nothing about one.""" + rooms.record(ROOM, "hello", RoomLogin(LoginResult.ACCEPTED, RoomAccess.MEMBER)) + rooms.record(ROOM, "helo", RoomLogin(LoginResult.ACCEPTED, RoomAccess.READ_ONLY)) + assert rooms.password(ROOM) == "hello" + assert rooms.access(ROOM) is RoomAccess.READ_ONLY + first = RoomStore(rooms._path.with_name("other.json")) + first.record(ROOM, "guess", RoomLogin(LoginResult.ACCEPTED, RoomAccess.READ_ONLY)) + assert first.password(ROOM) == "guess", "with nothing better, it is what gets back in" + + +def test_a_blank_login_never_replaces_a_password(rooms: RoomStore) -> None: + """Blank proves the room knows us — until it restarts and forgets a member.""" + rooms.record(ROOM, "hello", RoomLogin(LoginResult.ACCEPTED, RoomAccess.MEMBER)) + rooms.record(ROOM, "", RoomLogin(LoginResult.ACCEPTED, RoomAccess.MEMBER)) + assert rooms.password(ROOM) == "hello" + rooms.record(ROOM, "", RoomLogin(LoginResult.ACCEPTED, RoomAccess.ADMIN)) + assert rooms.password(ROOM) == "hello" + + +def test_an_admin_login_keeps_the_room_password_beside_it(rooms: RoomStore) -> None: + """The admin password lives in the admin store; the member's stays here as the fallback.""" + rooms.record(ROOM, "hello", RoomLogin(LoginResult.ACCEPTED, RoomAccess.MEMBER)) + rooms.record(ROOM, "admin", RoomLogin(LoginResult.ACCEPTED, RoomAccess.ADMIN)) + assert rooms.password(ROOM) == "hello" and rooms.access(ROOM) is RoomAccess.ADMIN + + +def test_a_refusal_forgets_only_the_password_it_refused(rooms: RoomStore) -> None: + """A node turning one password down says nothing about another.""" + rooms.record(ROOM, "hello", RoomLogin(LoginResult.ACCEPTED, RoomAccess.MEMBER)) + rooms.record(ROOM, "other", RoomLogin(LoginResult.REFUSED)) + assert rooms.password(ROOM) == "hello" + + +async def test_a_blank_admin_login_leaves_the_admin_password_alone(tmp_path: Path) -> None: + """Repeater admin shares that password; a blank probe must not erase it.""" + ctx = _Ctx(tmp_path, MockDevice()) + await ctx.rooms.login(ROOM, "admin") + login = await ctx.rooms.login(ROOM, "") + assert login.access is RoomAccess.ADMIN + assert ctx.admin_store.get(ROOM) == "admin" + + +async def test_one_login_to_a_room_at_a_time(tmp_path: Path) -> None: + """A view closed and reopened mid-login shares the login rather than sending another.""" + device = MockDevice() + ctx = _Ctx(tmp_path, device) + ctx.room_store.record(ROOM, "hello", RoomLogin(LoginResult.ACCEPTED, RoomAccess.MEMBER)) + sent: list[str] = [] + original = device.room_login + + async def slow(room, password): # noqa: ANN001, ANN202 + sent.append(password) + await asyncio.sleep(0.05) + return await original(room, password) + + device.room_login = slow + first = asyncio.ensure_future(ctx.rooms.login(ROOM, "hello")) + await asyncio.sleep(0) + assert not ctx.rooms.login_due(ROOM), "a login on its way is not due again" + second = await ctx.rooms.login(ROOM, "hello") + assert (await first) == second + assert sent == ["hello"] + third = await ctx.rooms.login(ROOM, "hello") # once it is answered, a new one may go + assert third and sent == ["hello", "hello"] async def test_joining_with_the_room_password_makes_us_a_member(tmp_path: Path) -> None: @@ -890,3 +957,34 @@ def test_chat_send_to_a_room_is_a_post(cli) -> None: # noqa: ANN001 receipt = json.loads(cli("--json", "chat", "send", "--to", "Lakeside BBS", "hi").stdout) assert receipt["kind"] == "room" assert receipt["acked"] is False, "this simulator never joined, so the room drops it" + + +async def test_a_read_only_member_is_asked_for_a_password_and_offered_no_retry() -> None: + """^L asks (the room password is what lets a reader post); ^R would only be dropped.""" + unacked = ChatMessage(text="lost", outbound=True, peer=ROOM.key_prefix, acked=False) + screen, calls = _view([unacked], access=RoomAccess.READ_ONLY) + assert screen._retry_target() is None + assert "^R" not in screen.footer_hint + screen.handle("login") + while screen._login_open: + await asyncio.sleep(0) + assert calls["asked"] == 1 + + +async def test_a_read_only_member_picks_a_post_to_see_its_paths_not_to_reply() -> None: + """With no compose line, Enter on a post has nothing to prime, and a paste nowhere to go.""" + screen, _ = _view(_board(), access=RoomAccess.READ_ONLY) + opened: list[ChatMessage] = [] + + async def paths(message: ChatMessage) -> None: + opened.append(message) + + screen._paths = paths + screen.handle("paste", "pasted text") + assert not screen._paste_open and screen._editor.text == "" + screen.handle("up") + assert screen.footer_hint.startswith("Enter paths") + screen.handle("enter") + await asyncio.sleep(0) + assert screen._editor.text == "" + assert [m.text for m in opened] == ["I'll check it"]