refactor: drop the unreachable XOAUTH2 .authenticate fallback

T3-21: aioimaplib's IMAP4 always has .xoauth2 and never .authenticate, so the
fallback branch and its _sasl_xoauth2 helper could not run. call
mail.xoauth2 directly.

Signed-off-by: disqualifier <dev@disqualifier.me>

README.md

@@ -11,22 +11,22 @@ This reads codes from email; it does not generate them (that is `pyotp`'s job).
 `requirements.txt`:
 
 ```
-aiomail @ git+ssh://git@git.rethinkstudios.io/rethink-public/aiomail.git@v0.1.8
+aiomail @ git+ssh://git@git.rethinkstudios.io/rethink-public/aiomail.git@v1.2.0
 # OAuth token providers (Microsoft / Google) need the extra:
-aiomail[oauth] @ git+ssh://git@git.rethinkstudios.io/rethink-public/aiomail.git@v0.1.8
+aiomail[oauth] @ git+ssh://git@git.rethinkstudios.io/rethink-public/aiomail.git@v1.2.0
 ```
 
 Direct:
 
 ```bash
-pip install "aiomail @ git+ssh://git@git.rethinkstudios.io/rethink-public/aiomail.git@v0.1.8"
+pip install "aiomail @ git+ssh://git@git.rethinkstudios.io/rethink-public/aiomail.git@v1.2.0"
-pip install "aiomail[oauth] @ git+ssh://git@git.rethinkstudios.io/rethink-public/aiomail.git@v0.1.8"
+pip install "aiomail[oauth] @ git+ssh://git@git.rethinkstudios.io/rethink-public/aiomail.git@v1.2.0"
 ```
 
 Requires `aioimaplib` and `beautifulsoup4` (pulled transitively). The `oauth`
 extra adds `aiohttp` for the refresh-token providers.
 
-Drop the `@v0.1.8` suffix from the line above to install the latest unpinned.
+Drop the `@v1.2.0` suffix from the line above to install the latest unpinned.
 
 ## Password auth
 

pyproject.toml

@@ -4,7 +4,7 @@ build-backend = "hatchling.build"
 
 [project]
 name = "aiomail"
-version = "0.1.8"
+version = "1.2.0"
 description = "async IMAP one-time-code retrieval with password/OAuth2 auth and dynamic matching"
 requires-python = ">=3.10"
 dependencies = [

src/aiomail/__init__.py

@@ -1,9 +1,15 @@
-"""aiomail — async IMAP one-time-code retrieval.
+"""aiomail - async IMAP one-time-code retrieval, password or OAuth2 auth, dynamic matching.
 
-reads OTP / login codes out of IMAP mailboxes (accounts you own). supports plain
+facade over auth (PasswordAuth/OAuth2Auth), IMAPClient (connection lifecycle), and
-password and OAuth2 (XOAUTH2) auth, and dynamic sender/subject/code matching via
+retrieve_otp (folder-scan orchestration); see each module's docstring for detail.
-substrings, regexes, or callables.
+
+footguns: an IMAPClient instance is not safe for concurrent callers beyond its internal
+connect/reconnect lock; sequence-number ids from before a reconnect are invalid after
+(pass use_uid=True if ids must survive one); credentials are always caller-supplied via
+an injected Auth, never read from config.
 """
+from importlib.metadata import version, PackageNotFoundError
+
 from .auth import Auth, OAuth2Auth, PasswordAuth
 from .client import IMAPClient
 from .extract import (
@@ -31,4 +37,7 @@ __all__ = [
     "DEFAULT_FOLDERS",
 ]
 
-__version__ = "0.1.8"
+try:
+    __version__ = version("aiomail")
+except PackageNotFoundError:
+    __version__ = "0.0.0+unknown"

src/aiomail/auth.py

@@ -1,15 +1,10 @@
-"""authentication mechanisms for the IMAP client.
+"""authentication mechanisms for the IMAP client: `PasswordAuth` (LOGIN), `OAuth2Auth` (XOAUTH2)."""
-
-`PasswordAuth` (LOGIN) and `OAuth2Auth` (XOAUTH2); credentials always injected.
-"""
-import base64
 import logging
 from typing import Awaitable, Callable, Optional, Protocol, Union, runtime_checkable
 
 log = logging.getLogger(__name__)
 
-# a token provider is any (optionally async) callable returning a fresh access
+# see aiomail.oauth for ready-made Microsoft / Google providers
-# token string; see aiomail.oauth for ready-made Microsoft / Google providers
 TokenProvider = Callable[[], Union[str, Awaitable[str]]]
 
 
@@ -36,16 +31,10 @@ class PasswordAuth:
 
 
 def _as_str(token) -> str:
-    """coerce a token to str (a provider may hand back bytes); both XOAUTH2 entrypoints downstream need str"""
+    """coerce a token to str (a provider may hand back bytes)"""
     return token.decode() if isinstance(token, bytes) else token
 
 
-def _sasl_xoauth2(user: str, token: str) -> str:
-    """build the base64 XOAUTH2 SASL initial-response string"""
-    raw = f"user={user}\x01auth=Bearer {token}\x01\x01".encode()
-    return base64.b64encode(raw).decode()
-
-
 class OAuth2Auth:
     """XOAUTH2 auth: a static `token`, or a `token_provider` (sync/async callable) fetched fresh at connect time"""
 
@@ -73,21 +62,8 @@ class OAuth2Auth:
 
     async def authenticate(self, mail) -> None:
         token = await self._resolve_token()
-        # mail.xoauth2(user, token) f-string-interpolates token, so it MUST be str
+        # mail.xoauth2(user, token) f-string-interpolates token, so it MUST be str -
-        # — bytes would interpolate the b'...' repr and corrupt the Bearer value.
+        # bytes would interpolate the b'...' repr and corrupt the Bearer value.
-        # _resolve_token already guarantees str via _as_str.
+        result, data = await mail.xoauth2(self.user, token)
-        xoauth2 = getattr(mail, "xoauth2", None)
-        if xoauth2 is not None:
-            result, data = await xoauth2(self.user, token)
-        elif hasattr(mail, "authenticate"):
-            # escape hatch for a non-aioimaplib client (aioimaplib's IMAP4 always
-            # has .xoauth2, never .authenticate); untested against any real driver
-            result, data = await mail.authenticate(
-                "XOAUTH2", lambda _: _sasl_xoauth2(self.user, token)
-            )
-        else:
-            raise RuntimeError(
-                "IMAP client exposes no XOAUTH2 entrypoint (no .xoauth2 or .authenticate)"
-            )
         if result != "OK":
             raise RuntimeError(f"xoauth2 auth failed: {result} {data}")

src/aiomail/client.py

@@ -1,9 +1,9 @@
 """async IMAP client wrapping aioimaplib: connect/retry/reconnect/close plus folders/search/fetch/mark-seen.
 
 auth is injected. reconnect-on-stale re-selects the prior folder, but sequence-number ids from
-before a reconnect are not valid after (a fresh SELECT can renumber the mailbox)
+before a reconnect are not valid after (a fresh SELECT can renumber the mailbox) - pass
-— pass `use_uid=True` if ids need to survive a reconnect. one instance is not
+`use_uid=True` if ids need to survive a reconnect. one instance is not safe for concurrent
-safe for concurrent callers beyond the internal connect/reconnect lock.
+callers beyond the internal connect/reconnect lock.
 """
 import asyncio
 import email
@@ -18,18 +18,13 @@ from .auth import Auth
 
 log = logging.getLogger(__name__)
 
-# IMAP LIST reply: (flags) "<delim>" <name>  — delim is server-defined (often "/" or
+# IMAP LIST reply: (flags) "<delim>" <name> - delim is server-defined (often "/" or
 # "." or NIL); capture the trailing name regardless, quoted or bare
 _LIST_RE = re.compile(rb'^\([^)]*\)\s+(?:"[^"]*"|NIL)\s+(.+)$')
 
 
 def _folder_name(raw: bytes) -> Optional[str]:
-    """extract the folder name from a LIST reply line, delimiter-agnostic
+    """extract the folder name from a LIST reply line, or None on no match"""
-
-    returns None on a non-matching line instead of a last-token rsplit fallback,
-    so the tagged completion line aioimaplib appends to the same response list
-    (e.g. `b"LIST completed."`) gets dropped instead of read as a phantom folder.
-    """
     match = _LIST_RE.match(raw.strip())
     if not match:
         return None
@@ -39,8 +34,7 @@ def _folder_name(raw: bytes) -> Optional[str]:
 class IMAPClient:
     """connection-managing IMAP client driven by an injected auth mechanism
 
-    `use_uid` (UID vs sequence-number addressing) is independent of `use_ssl` —
+    `use_uid` (UID vs sequence-number addressing) is independent of `use_ssl` - unrelated concerns.
-    an earlier draft conflated them (`use_uid = use_ssl`); unrelated concerns.
     """
 
     def __init__(
@@ -73,11 +67,7 @@ class IMAPClient:
         await self.close()
 
     async def connect(self) -> bool:
-        """open a connection and authenticate, retrying with linear backoff
+        """open a connection and authenticate, retrying with linear backoff; serialized by an internal lock"""
-
-        serialized by an internal lock: queued callers never tear down each
-        other's in-progress handshake, and a superseded connection is logged out.
-        """
         async with self._lock:
             return await self._connect_locked()
 
@@ -103,23 +93,22 @@ class IMAPClient:
                     self._mail = None
                 if attempt < self.max_retries - 1:
                     await asyncio.sleep(2 * (attempt + 1))
+        # terminal signal: all attempts are exhausted and this SWALLOWS the failure into a
+        # False return the caller branches on. without this line the per-attempt WARNINGs are
+        # the only trace, so a genuinely dead connection could look like routine noise - log
+        # the terminal exhaustion loudly so real degradation is visible, then return False.
+        log.warning("connect to %s failed after %d attempts", self.host, self.max_retries)
         return False
 
     @staticmethod
     async def _discard_mail(mail) -> None:
-        """tear down a half-built IMAP4 without leaking its connect task
+        """tear down a half-built IMAP4 without leaking its fire-and-forget connect task"""
-
-        aioimaplib schedules `create_connection` as a fire-and-forget task; on a
-        refused connection it raises and asyncio logs a noisy "Task exception was
-        never retrieved" traceback unless retrieved here first.
-        """
         task = getattr(mail, "_client_task", None)
         if task is not None and not task.done():
             task.cancel()
         if task is not None:
-            # shield distinguishes our own task.cancel() from an external cancel of
+            # shield: a bare `await task` would also swallow an external cancel; under
-            # this coroutine: a bare `await task` swallowed both, resisting
+            # shield, CancelledError here means external only.
-            # cancellation. under shield, CancelledError here means external only.
             try:
                 await asyncio.shield(task)
             except asyncio.CancelledError:
@@ -148,13 +137,8 @@ class IMAPClient:
         self._selected_folder = None
 
     async def ensure_connection(self) -> bool:
-        """return a live, SELECTED-if-applicable connection, reconnecting if the link is stale
+        """return a live, SELECTED-if-applicable connection, reconnecting (and re-selecting the prior
-
+        folder) if the link is stale; see module docstring for the sequence-number-vs-use_uid caveat"""
-        re-selects the previously-selected folder after a reconnect; sequence-number
-        ids from before the reconnect are NOT valid against the new session (a fresh
-        SELECT can renumber the mailbox) unless use_uid=True. serialized by the
-        internal lock so a queued caller rechecks liveness before reconnecting.
-        """
         async with self._lock:
             if self._mail is not None:
                 try:
@@ -197,15 +181,21 @@ class IMAPClient:
         for folder in folder_list or []:
             try:
                 name = _folder_name(folder)
-            except Exception:
+            except Exception as exc:
+                log.debug("skipping unparseable folder entry %r: %s", folder, exc)
                 continue
             if name is not None:
                 folders.append(name)
         return folders
 
     async def select(self, folder: str) -> bool:
-        """select a folder, returning whether it succeeded"""
+        """select a folder, returning whether it succeeded
-        if not await self.ensure_connection():
+
+        connects/reconnects first if needed; a failed re-select of the *previously*
+        selected folder during reconnect does not block attempting this call's own
+        target folder, since a live connection can still select it.
+        """
+        if not await self.ensure_connection() and self._mail is None:
             return False
         try:
             result, _ = await self._mail.select(f'"{folder}"')
@@ -239,10 +229,16 @@ class IMAPClient:
         return sorted(set(ids), reverse=True)
 
     async def fetch(self, email_id: int, *, icloud: bool = False) -> Optional[email.message.Message]:
-        """fetch and parse a full message by id"""
+        """fetch and parse a full message by id without setting \\Seen
+
+        uses BODY.PEEK[]: RFC822 / BODY[] on a SELECTed mailbox set \\Seen as a side effect
+        (RFC 3501), so a fetched-then-rejected message would vanish from every later UNSEEN
+        search. only mark_seen() flags a message. `icloud` is kept for compatibility and is
+        now a no-op (BODY.PEEK[] already works there)
+        """
         if not await self.ensure_connection():
             return None
-        part = "(BODY[])" if icloud else "(RFC822)"
+        part = "(BODY.PEEK[])"
         try:
             if self.use_uid:
                 result, data = await self._mail.uid("fetch", str(email_id), part)
@@ -266,12 +262,18 @@ class IMAPClient:
         return None
 
     async def mark_seen(self, email_id: int) -> bool:
-        """flag a message as read without deleting it"""
+        """flag a message as read without deleting it
+
+        aioimaplib does not apply its own timeout to the UID STORE command path, so the
+        use_uid=True call is wrapped here to bound it the same as the non-uid path.
+        """
         if not await self.ensure_connection():
             return False
         try:
             if self.use_uid:
-                result, _ = await self._mail.uid("store", str(email_id), "+FLAGS", "(\\Seen)")
+                result, _ = await asyncio.wait_for(
+                    self._mail.uid("store", str(email_id), "+FLAGS", "(\\Seen)"), self.timeout
+                )
             else:
                 result, _ = await self._mail.store(str(email_id), "+FLAGS", "(\\Seen)")
             return result == "OK"

src/aiomail/extract.py

@@ -1,12 +1,8 @@
-"""code extraction and dynamic matching for email messages, pure logic with no network IO.
+"""code extraction and dynamic matching for email messages, pure logic with no network IO."""
-
-`extract_code` pulls a one-time code out of a message; `as_predicate` turns a
-string / compiled regex / callable into a uniform match function for filtering
-senders and subjects.
-"""
 import email.message
 import logging
 import re
+from email.errors import HeaderParseError
 from email.header import decode_header, make_header
 from typing import Callable, Iterable, Iterator, Optional, Pattern, Sequence, Union
 
@@ -19,12 +15,13 @@ log = logging.getLogger(__name__)
 MatchSpec = Union[str, Pattern, Callable[[Optional[str]], bool], None]
 
 # patterns tried (in order) against the subject then each body part; the first
-# capturing group is the code, or the whole match if there are no groups
+# capturing group is the code, or the whole match if there are no groups. no bare
+# digit catch-all here - that is the `lengths` fallback, which runs only after every
+# text missed every pattern
 DEFAULT_PATTERNS: Sequence[Union[str, Pattern]] = (
     r"(\d{4,8})\s+is\s+your",
     r"your.{0,20}?code(?:\s+is)?[:\s]+(\d{4,8})",
     r"code(?:\s+is)?[:\s]+(\d{4,8})",
-    r"\b(\d{4,8})\b",
 )
 
 # default digit-run lengths to treat as a standalone code when no pattern hits
@@ -49,7 +46,7 @@ def decode_header_value(raw: str) -> str:
         return raw
     try:
         return str(make_header(decode_header(raw)))
-    except (UnicodeDecodeError, LookupError, ValueError) as exc:
+    except (UnicodeDecodeError, LookupError, ValueError, HeaderParseError) as exc:
         log.debug("header decode failed (%s): %s", raw, exc)
         return raw
 
@@ -62,7 +59,7 @@ def _decode_part(part: email.message.Message) -> Optional[str]:
     charset = part.get_content_charset() or "utf-8"
     try:
         return payload.decode(charset, errors="replace")
-    except (LookupError, TypeError):
+    except (LookupError, TypeError, ValueError):
         return payload.decode("utf-8", errors="replace")
 
 
@@ -83,15 +80,12 @@ def _iter_text(message: email.message.Message) -> Iterator[str]:
         yield text
 
 
-def _scan(text: str, patterns: list[Pattern], lengths: set[int]) -> Optional[str]:
+def _match(text: str, patterns: list[Pattern]) -> Optional[str]:
-    """run patterns then a standalone digit-run fallback over one block of text"""
+    """first pattern hit in one block of text"""
     for pat in patterns:
         m = pat.search(text)
         if m:
             return m.group(1) if m.groups() else m.group(0)
-    for run in re.findall(r"\d+", text):
-        if len(run) in lengths:
-            return run
     return None
 
 
@@ -103,22 +97,22 @@ def extract_code(
 ) -> Optional[str]:
     """extract a one-time code from a message, subject first then body parts
 
-    `patterns` are regexes tried in order (first capturing group wins, else the
+    `patterns` are regexes tried in order (first capturing group wins, else the whole
-    whole match); if none hit, a standalone digit run whose length is in
+    match) over the subject and every body part; only if none hit anywhere is a
-    `lengths` is returned. both are parameters so callers tune per provider.
+    standalone digit run whose length is in `lengths` returned, from the same texts.
     """
     compiled = _compile(patterns)
     length_set = set(lengths)
+    texts = [decode_header_value(message.get("Subject", "") or ""), *_iter_text(message)]
 
-    subject = decode_header_value(message.get("Subject", "") or "")
+    for text in texts:
-    hit = _scan(subject, compiled, length_set)
+        hit = _match(text, compiled)
-    if hit:
-        return hit
-
-    for text in _iter_text(message):
-        hit = _scan(text, compiled, length_set)
         if hit:
             return hit
+    for text in texts:
+        for run in re.findall(r"\d+", text):
+            if len(run) in length_set:
+                return run
     return None
 
 

src/aiomail/oauth.py

@@ -1,8 +1,6 @@
-"""optional OAuth2 token providers (refresh-token -> access-token) for `OAuth2Auth`, credentials always caller-supplied.
+"""optional OAuth2 token providers (refresh-token -> access-token) for `OAuth2Auth`, credentials always
-
+caller-supplied. aiohttp is an optional extra; missing it raises a clear error only when a provider is
-aiohttp is an optional extra so the core stays light; missing it raises a clear
+instantiated, not on import."""
-error only when a provider is instantiated, not on import.
-"""
 import asyncio
 import logging
 import time
@@ -80,14 +78,13 @@ class _RefreshTokenProvider:
                                 if token:
                                     self._failures = 0
                                     return token
-                                # log a truncated body (never whole, may carry sensitive
+                                # truncated, never whole: the body may carry sensitive material
-                                # material) so a 200-with-no-token isn't a silent drop
                                 log.warning(
                                     "token endpoint %s -> 200 with no access_token: %s",
                                     endpoint, str(body_json)[:200],
                                 )
                             else:
-                                # truncated only — the body may carry sensitive material
+                                # truncated only - the body may carry sensitive material
                                 body = (await resp.text())[:200]
                                 log.warning("token endpoint %s -> %s: %s", endpoint, resp.status, body)
                 except Exception as exc:

src/aiomail/retrieve.py

@@ -1,8 +1,4 @@
-"""orchestration: `retrieve_otp` ties the client and extractor together to find the most recent valid OTP.
+"""orchestration: `retrieve_otp` ties the client and extractor together to find the most recent valid OTP."""
-
-sender/subject accept the flexible match specs from `extract`; provider quirks
-(folders, age, patterns) live in the arguments, not hardcoded branches.
-"""
 import asyncio
 import logging
 import time
@@ -26,14 +22,12 @@ log = logging.getLogger(__name__)
 DEFAULT_FOLDERS: Sequence[str] = ("INBOX", "Junk", "Spam", "Archive", "All Mail")
 
 
-def _server_query(sender: MatchSpec, subject: MatchSpec, match_field: str = "from") -> str:
+def _server_query(sender: MatchSpec, subject: MatchSpec, match_field: str = "from", unseen: bool = False) -> str:
-    """build a narrowing IMAP query from plain-string specs only, falling back to ALL for regex/callable specs
+    """build a narrowing IMAP query from plain-string specs (ALL when nothing narrows)
 
-    `match_field="to"` searches TO OR FROM (a forwarded code may keep the
+    `unseen` adds UNSEEN so a code already consumed (marked seen) is never served again
-    original From), matching the client-side forwarded-From fallback so the
-    server query never narrows out a result the client would have accepted.
     """
-    parts: List[str] = []
+    parts: List[str] = ["UNSEEN"] if unseen else []
     if isinstance(sender, str):
         if match_field == "to":
             parts.append(f'OR TO "{sender}" FROM "{sender}"')
@@ -82,19 +76,20 @@ async def retrieve_otp(
     sender/subject accept a substring, a compiled regex, or a callable.
     `match_field="to"` matches the recipient address and additionally accepts a
     forwarded match on From. set `max_age=None` to disable the freshness check.
+    with `mark_seen=True` (default) only UNSEEN messages are searched, so a code is
+    returned once; `mark_seen=False` searches seen messages too.
     """
     folders = list(folders) if folders is not None else list(DEFAULT_FOLDERS)
+    lengths = list(lengths)
     sender_ok = as_predicate(sender)
     subject_ok = as_predicate(subject)
-    query = _server_query(sender, subject, match_field)
+    query = _server_query(sender, subject, match_field, unseen=mark_seen)
 
     for attempt in range(retries + 1):
         for folder in folders:
             if not await client.select(folder):
                 continue
             ids = await client.search(query)
-            if not ids and query != "ALL":
-                ids = await client.search("ALL")
             if not ids:
                 continue