Merge pull request #12 from devinslick/ThoreFeedback

Strict typing, more coverage, updated fmd public server

Author: devinslick

.coverage

@@ -44,5 +44,5 @@ repos:
           - aiohttp
           - argon2-cffi
           - cryptography
-        args: ['--install-types', '--non-interactive']
+        args: ['--install-types', '--non-interactive', '--strict', '--show-error-codes']
         exclude: '^tests/'

.pre-commit-config.yaml

@@ -208,7 +208,7 @@ This client targets the FMD ecosystem:
 
 - https://fmd-foss.org/
 - https://gitlab.com/fmd-foss
-- Public community instance: https://fmd.nulide.de/
+- Public community instance: https://server.fmd-foss.org/
  - Listed on the official FMD community page: https://fmd-foss.org/docs/fmd-server/community
 
 MIT © 2025 Devin Slick

README.md

@@ -0,0 +1,41 @@
+# MyPy Strict Typing Errors - Baseline Assessment
+
+This document captures the initial mypy errors found during Phase 1 of the strict typing enforcement plan.
+
+Run command: `mypy fmd_api/ --strict --show-error-codes`
+
+Date: November 18, 2025
+
+## Summary
+- Total errors: 12
+- Files affected: client.py (9 errors), device.py (3 errors)
+- Checked files: 7 source files
+
+## Errors by File
+
+### fmd_api/client.py
+1. Line 99: Function is missing a type annotation for one or more arguments [no-untyped-def]
+2. Line 103: Function is missing a return type annotation [no-untyped-def]
+3. Line 202: Returning Any from function declared to return "str" [no-any-return]
+4. Line 206: Returning Any from function declared to return "str" [no-any-return]
+5. Line 209: Returning Any from function declared to return "str" [no-any-return]
+6. Line 372: Function is missing a return type annotation [no-untyped-def]
+7. Line 821: Returning Any from function declared to return "float" [no-any-return]
+
+### fmd_api/device.py
+1. Line 36: Function is missing a return type annotation [no-untyped-def]
+2. Line 55: Function is missing a type annotation for one or more arguments [no-untyped-def]
+3. Line 94: Missing type parameters for generic type "dict" [type-arg]
+4. Line 124: Missing type parameters for generic type "dict" [type-arg]
+5. Line 142: Missing type parameters for generic type "dict" [type-arg]
+
+## Next Steps
+These errors will be addressed in Phase 2 (Core Module Typing). Priority order:
+1. Add missing type annotations to functions
+2. Replace Any returns with proper types
+3. Add type parameters to generic types
+
+## Configuration
+- Python version: 3.9
+- Strict mode: enabled
+- Tests excluded: yes (ignore_errors = true)

docs/mypy_baseline_errors.md

@@ -0,0 +1,120 @@
+# Release v2.0.4: Password-Free Auth Artifacts, Picture API Cleanup, Stronger Wipe Validation, Higher Coverage
+
+## Summary
+v2.0.4 is a hardening + ergonomics release focused on secure password-free resume flows, clarified and modernized picture APIs, safer destructive actions, and significantly expanded test coverage (now ~98%). It remains fully backward compatible for consumers relying on deprecated method names; all new functionality is additive.
+
+## Highlights
+
+### 1. Authentication Artifacts (Password-Free Resume)
+- New methods:
+  - `FmdClient.from_auth_artifacts(artifacts: dict)` – Restore a client without the raw password.
+  - `FmdClient.resume(...)` – Lower-level variant accepting explicit fields.
+  - `await client.export_auth_artifacts()` – Export `base_url`, `fmd_id`, `access_token`, `private_key` (PEM), optional `password_hash`, `session_duration`, `token_issued_at`.
+  - `await client.drop_password()` / `drop_password=True` in `create()` – Immediately discard raw password after onboarding.
+- 401 handling logic hierarchy:
+  1. If raw password present → reauthenticate (existing flow)
+  2. Else if `password_hash` present → hash-based token refresh (`_reauth_with_hash()`)
+  3. Else → raise `FmdApiException` (caller must re-onboard)
+- Private key load now supports both PEM and DER (fallback path tested).
+
+### 2. Picture API Renaming & Deprecations
+- New canonical methods on `Device`:
+  - `get_picture_blobs()` – fetch encrypted Base64 blobs.
+  - `decode_picture()` – decrypt + decode into `PhotoResult`.
+- Deprecated (still functional, emit `DeprecationWarning`):
+  - `take_front_photo()`, `take_rear_photo()` → use `take_front_picture()`, `take_rear_picture()`
+  - `fetch_pictures()`, `get_pictures()` (Device) → use `get_picture_blobs()`
+  - `download_photo()`, `get_picture()` (Device) → use `decode_picture()`
+- `download_photo()` now points directly to `decode_picture()` (avoids chained deprecated call).
+
+### 3. Wipe (Factory Reset) Hardening
+- `Device.wipe()` now strictly requires:
+  - `confirm=True`
+  - `pin` argument present
+  - PIN must be alphanumeric ASCII (no spaces). Future enforcement of 16+ length is noted (fmd-android MR 379).
+- Removed redundant space validation branch.
+
+### 4. Lock Message Support
+- `Device.lock(message=...)` allows an optional user message; sanitized (quotes / backticks / semicolons removed, whitespace collapsed, 120 char cap). Falls back cleanly if server ignores payload.
+
+### 5. Expanded Export Functionality & Robustness
+- `export_data_zip()` improvements:
+  - Picture file extension detection: `.png` via magic bytes, default `.jpg` else.
+  - Resilient manifest entries capturing per-item decryption errors (non-fatal).
+  - Additional validation of location/picture list types with graceful fallbacks.
+
+### 6. Coverage & Testing Improvements
+- Overall coverage ~98% (client.py ~98%, device.py mid/high 90s, models 100%).
+- New targeted tests exercise:
+  - DER private key resume path
+  - Missing artifact field errors
+  - 401 reauth when neither password nor hash is available (expected exception)
+  - Hash-based 401 reauth success path
+  - PNG export branch + unknown image default `.jpg`
+  - Deprecated wrapper warning emission
+  - Non-dict JSON fallback handling & non-list picture responses
+  - Error recording during export (location & picture failures)
+  - Retry logic: 429 (Retry-After numeric/date/negative), 500/502 sequences, connection errors, backoff jitter/no-jitter paths
+  - Masking helpers and retry-after parsing edge cases
+
+### 7. Documentation Updates
+- README: Added password-free artifact usage, wipe PIN notes, lock message mention, community listing.
+- `MIGRATE_FROM_V1.md`: Corrected camera method naming, clarified wipe requirements, updated picture usage.
+- `AUTH_ARTIFACTS_DESIGN.md`: Formal specification of artifact-based resume workflow.
+
+### 8. Internal / Quality Enhancements
+- Removed chained deprecation in `Device.download_photo()`.
+- Simplified `wipe()` validation logic (single branch covers spaces & non-alphanumeric).
+- Eliminated redundant PIN space check.
+- Minor consistency and defensive branches now covered or documented.
+
+## Deprecations (No Immediate Removal)
+| Deprecated | Replacement |
+|------------|-------------|
+| `Device.take_front_photo()` | `Device.take_front_picture()` |
+| `Device.take_rear_photo()` | `Device.take_rear_picture()` |
+| `Device.fetch_pictures()` | `Device.get_picture_blobs()` |
+| `Device.get_pictures()` (Device wrapper) | `Device.get_picture_blobs()` |
+| `Device.download_photo()` | `Device.decode_picture()` |
+| `Device.get_picture()` | `Device.decode_picture()` |
+
+Plan: Monitor usage; consider removal or formal EOL notice in a future minor/major once ecosystem migrates.
+
+## Security Considerations
+- Encourages immediate password discarding (`drop_password=True`), reducing exposure of raw credentials.
+- `password_hash` still sensitive—store using platform secret storage if possible.
+- Wipe PIN validation prevents accidental destructive actions with weak/empty inputs.
+- Lock message sanitization avoids command injection edge cases in future server parsing contexts.
+
+## Migration Notes (2.0.3 → 2.0.4)
+- Existing code continues to function; deprecation warnings guide picture API migration.
+- To adopt password-free resume:
+  1. Onboard normally with `create(..., drop_password=True)`.
+  2. Persist `await client.export_auth_artifacts()` securely.
+  3. Resume with `await FmdClient.from_auth_artifacts(artifacts)`.
+
+## Example: Password-Free Cycle
+```python
+client = await FmdClient.create(url, fmd_id, password, drop_password=True)
+artifacts = await client.export_auth_artifacts()
+# Persist artifacts securely...
+await client.close()
+
+client2 = await FmdClient.from_auth_artifacts(artifacts)
+locations = await client2.get_locations(1)
+```
+
+## Potential Follow-Ups (Not Included)
+- Enforce future 16+ PIN length once upstream server mandates it.
+- Add a CHANGELOG.md consolidating releases (this file can seed that entry).
+- Provide optional encrypted artifact export (password-protected ZIP or keyring integration).
+- Add coverage for final remaining defensive lines (currently low-risk).
+
+## Version
+`fmd_api.__version__ == "2.0.4"`
+
+## Acknowledgements
+Thanks to contributors and early testers providing feedback on artifact-based auth and API naming clarity.
+
+---
+Released: 2025-11-09

docs/release/v2.0.4.md

@@ -0,0 +1,70 @@
+# Strict Typing Enforcement Plan for fmd_api
+
+## Introduction
+The fmd_api repository currently has warnings related to type checking, likely from mypy or similar tools. This plan outlines a structured approach to enforce strict typing across the codebase, improving code quality, reducing bugs, and enhancing developer experience.
+
+## Current State Assessment
+- Run mypy on the codebase to identify current warnings.
+- Categorize issues: missing type annotations, Any types, untyped functions, etc.
+- Baseline coverage: Measure current mypy strictness level.
+
+## Goals
+- Achieve 100% mypy strict mode compliance.
+- Ensure all public APIs are fully typed.
+- Integrate type checking into CI pipeline.
+- Provide clear migration path for contributors.
+
+## Plan Steps
+
+### Phase 1: Assessment and Setup (Week 1)
+- Install and configure mypy with strict settings.
+- Run full mypy check and document all errors.
+- Set up pre-commit hooks for mypy.
+- Update pyproject.toml with mypy configuration.
+
+### Phase 2: Core Module Typing (Weeks 2-4)
+- Start with fmd_api/client.py: Add type annotations to all functions, classes, and variables.
+- Move to fmd_api/device.py, models.py, exceptions.py, etc.
+- Replace Any with specific types where possible.
+- Handle complex types like async iterators, optional fields.
+
+### Phase 3: Test Suite Typing (Weeks 5-6)
+- Type all test files in tests/unit/ and tests/functional/.
+- Ensure fixtures and mocks are properly typed.
+- Update conftest.py with types.
+
+### Phase 4: Utilities and Helpers (Week 7)
+- Type helpers.py, _version.py, and any utility modules.
+- Ensure all imports are typed.
+
+### Phase 5: CI Integration and Validation (Week 8)
+- Add mypy to GitHub Actions workflow.
+- Fail CI on mypy errors.
+- Update README with typing requirements.
+- Add typing badges if applicable.
+
+### Phase 6: Maintenance and Monitoring (Ongoing)
+- Monitor for new typing issues in PRs.
+- Update types as dependencies change.
+- Consider adding pyright or other type checkers for redundancy.
+
+## Tools and Dependencies
+- mypy: Primary type checker.
+- typing_extensions: For backporting newer typing features if needed.
+- Pre-commit: For local checks.
+
+## Challenges and Mitigations
+- Complex async code: Use proper typing for coroutines and iterators.
+- Third-party libraries: Ensure stubs are available or add type: ignore comments.
+- Backward compatibility: Maintain Python 3.8+ support.
+
+## Timeline
+- Total duration: 8 weeks.
+- Weekly milestones with PRs for each phase.
+
+## Resources
+- MyPy documentation: https://mypy.readthedocs.io/
+- Typing best practices: PEP 484, PEP 526.
+
+## Conclusion
+Enforcing strict typing will make the codebase more robust and maintainable. This plan provides a clear path to achieve that goal.

docs/strict_typing_enforcement_plan.md

@@ -1 +1 @@
-__version__ = "2.0.4"
+__version__ = "2.0.5"

fmd_api/_version.py

@@ -22,6 +22,7 @@
 import time
 import random
 from typing import Optional, List, Any, Dict, cast
+from types import TracebackType
 
 import aiohttp
 from argon2.low_level import hash_secret_raw, Type
@@ -96,7 +97,12 @@ def __init__(
     async def __aenter__(self) -> "FmdClient":
         return self
 
-    async def __aexit__(self, exc_type, exc, tb) -> None:
+    async def __aexit__(
+        self,
+        exc_type: Optional[type[BaseException]],
+        exc: Optional[BaseException],
+        tb: Optional[TracebackType],
+    ) -> None:
         await self.close()
 
     @classmethod
@@ -114,7 +120,7 @@ async def create(
         conn_limit_per_host: Optional[int] = None,
         keepalive_timeout: Optional[float] = None,
         drop_password: bool = False,
-    ):
+    ) -> "FmdClient":
         inst = cls(
             base_url,
             session_duration,
@@ -140,7 +146,7 @@ async def create(
 
     async def _ensure_session(self) -> None:
         if self._session is None or self._session.closed:
-            connector_kwargs = {}
+            connector_kwargs: Dict[str, Any] = {}
             if self._ssl is not None:
                 connector_kwargs["ssl"] = self._ssl
             if self._conn_limit is not None:
@@ -186,7 +192,7 @@ async def authenticate(self, fmd_id: str, password: str, session_duration: int)
     def _hash_password(self, password: str, salt: str) -> str:
         salt_bytes = base64.b64decode(_pad_base64(salt))
         password_bytes = (CONTEXT_STRING_LOGIN + password).encode("utf-8")
-        hash_bytes = hash_secret_raw(
+        hash_bytes: bytes = hash_secret_raw(
             secret=password_bytes,
             salt=salt_bytes,
             time_cost=1,
@@ -199,14 +205,16 @@ def _hash_password(self, password: str, salt: str) -> str:
         return f"$argon2id$v=19$m=131072,t=1,p=4${salt}${hash_b64}"
 
     async def _get_salt(self, fmd_id: str) -> str:
-        return await self._make_api_request("PUT", "/api/v1/salt", {"IDT": fmd_id, "Data": ""})
+        return cast(str, await self._make_api_request("PUT", "/api/v1/salt", {"IDT": fmd_id, "Data": ""}))
 
     async def _get_access_token(self, fmd_id: str, password_hash: str, session_duration: int) -> str:
         payload = {"IDT": fmd_id, "Data": password_hash, "SessionDurationSeconds": session_duration}
-        return await self._make_api_request("PUT", "/api/v1/requestAccess", payload)
+        return cast(str, await self._make_api_request("PUT", "/api/v1/requestAccess", payload))
 
     async def _get_private_key_blob(self) -> str:
-        return await self._make_api_request("PUT", "/api/v1/key", {"IDT": self.access_token, "Data": "unused"})
+        return cast(
+            str, await self._make_api_request("PUT", "/api/v1/key", {"IDT": self.access_token, "Data": "unused"})
+        )
 
     def _decrypt_private_key_blob(self, key_b64: str, password: str) -> bytes:
         key_bytes = base64.b64decode(_pad_base64(key_b64))
@@ -379,7 +387,7 @@ async def _make_api_request(
         retry_auth: bool = True,
         timeout: Optional[float] = None,
         max_retries: Optional[int] = None,
-    ):
+    ) -> Any:
         """
         Makes an API request and returns Data or text depending on expect_json/stream.
         Mirrors get_all_locations/_make_api_request logic from original file (including 401 re-auth).
@@ -817,8 +825,8 @@ def _compute_backoff(base: float, attempt: int, max_delay: float, jitter: bool)
     delay = min(max_delay, base * (2**attempt))
     if jitter:
         # Full jitter: random between 0 and delay
-        return random.uniform(0, delay)
-    return delay
+        return float(random.uniform(0, delay))
+    return float(delay)
 
 
 def _parse_retry_after(retry_after_header: Optional[str]) -> Optional[float]:

fmd_api/client.py

@@ -33,7 +33,7 @@ def __init__(self, client: FmdClient, fmd_id: str, raw: Optional[Dict[str, Any]]
         self.cached_location: Optional[Location] = None
         self._last_refresh = None
 
-    async def refresh(self, *, force: bool = False):
+    async def refresh(self, *, force: bool = False) -> None:
         """Refresh the device's most recent location (uses client.get_locations(1))."""
         if not force and self.cached_location is not None:
             return
@@ -52,7 +52,9 @@ async def get_location(self, *, force: bool = False) -> Optional[Location]:
             await self.refresh(force=force)
         return self.cached_location
 
-    async def get_history(self, start=None, end=None, limit: int = -1) -> AsyncIterator[Location]:
+    async def get_history(
+        self, start: Optional[Any] = None, end: Optional[Any] = None, limit: int = -1
+    ) -> AsyncIterator[Location]:
         """
         Iterate historical locations. Uses client.get_locations() under the hood.
         Yields decrypted Location objects newest-first (matches get_all_locations when requesting N recent).
@@ -91,7 +93,7 @@ async def take_rear_photo(self) -> bool:
         )
         return await self.take_rear_picture()
 
-    async def fetch_pictures(self, num_to_get: int = -1) -> List[dict]:
+    async def fetch_pictures(self, num_to_get: int = -1) -> List[Dict[str, Any]]:
         warnings.warn(
             "Device.fetch_pictures() is deprecated; use get_picture_blobs()",
             DeprecationWarning,
@@ -121,7 +123,7 @@ async def take_rear_picture(self) -> bool:
         """Request a picture from the rear camera."""
         return await self.client.take_picture("back")
 
-    async def get_pictures(self, num_to_get: int = -1) -> List[dict]:
+    async def get_pictures(self, num_to_get: int = -1) -> List[Dict[str, Any]]:
         """Deprecated: use get_picture_blobs()."""
         warnings.warn(
             "Device.get_pictures() is deprecated; use get_picture_blobs()",
@@ -139,7 +141,7 @@ async def get_picture(self, picture_blob_b64: str) -> PhotoResult:
         )
         return await self.decode_picture(picture_blob_b64)
 
-    async def get_picture_blobs(self, num_to_get: int = -1) -> List[dict]:
+    async def get_picture_blobs(self, num_to_get: int = -1) -> List[Dict[str, Any]]:
         """Get raw picture blobs (base64-encoded encrypted strings) from the server."""
         return await self.client.get_pictures(num_to_get=num_to_get)