Add UniFi Access local-API client#5
Merged
Merged
Conversation
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Adds the Wiegand-26 facility code (0-255) as a required integer field in UnifiConfig so the UniFi client can encode card_ids into nfc_id format. Validation enforces type and range; config.example.toml and all tests updated. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
… values Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Introduces UnifiClient and UnifiClientError. The constructor performs a raw SSL handshake before building the httpx.Client, SHA-256-hashes the peer certificate in DER form, and rejects mismatches (including colon- separated fingerprints). Implements context-manager protocol via __enter__ / __exit__ -> close(). 18 tests pass; mypy --strict and ruff clean. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Implements _unwrap, _with_retries, _backoff_seconds, _parse_retry_after, and _request on UnifiClient, mirroring the CiviCRM client's retry semantics. Adds pytest-httpx-based envelope and retry tests (red: fetch_users pending Task 6). Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Adds fetch_users() with paginated GET /api/v1/developer/users, _row_to_unifi_user helper, instance caches (_unifi_user_id_by_contact, _nfc_cards_by_contact, _nfc_token_map, _fetched_users_done), logging with card-ID redaction, and 6 new tests. Also removes now-stale type: ignore[attr-defined] comments from Task 5 tests. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Adds _apply_update_credential to UnifiClient: pre-imports new cards, PUTs name changes, DELETEs old NFC cards, and binds the new card token. Wires it into apply() and covers it with two new tests (card-swap and name-only update paths). ResolvedMember import dropped as unused (mypy infers the type from Diff's annotation; ruff F401 confirmed). Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Adds _preimport_unknown_cards (called from apply() before all buckets), _apply_add, and helpers _reactivate_existing, _create_user, _bind_card_if_set, _assign_policy_if_set. Removes the redundant per-bucket pre-import that was inside _apply_update_credential. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
- apply() in dry-run now calls _populate_token_map_for_dry_run() so the
read-side token-map fetch runs when the diff has card-bearing entries,
satisfying spec §8 ("reads execute normally in dry-run").
- Added test_fetch_users_warning_does_not_leak_card_id to assert the
multi-card warning never contains raw card numbers (architecture §11).
- Added test_apply_dry_run_still_fetches_token_map_when_diff_has_cards
and test_apply_dry_run_no_token_map_when_diff_has_no_cards to cover
the new dry-run token-map behaviour (spec §13 #11).
- Updated test_apply_dry_run_makes_no_writes to register the token-map
response and assert on write-method absence rather than request count.
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
…d would-import dry-run log - Fix 1 (CRITICAL): treat config.host as a full URL (matching CivicrmClient pattern); use urlsplit to extract hostname+port for the TLS socket call so a validated https://host:port value no longer produces https://https://... - Update _unifi_config() in tests to use host="https://192.0.2.1:12445" and add test_unifi_client_constructs_from_loaded_config to catch this regression. - Fix 2 (IMPORTANT): remove nfc_id from the debug log in _ensure_nfc_token_map to prevent card CN leakage (architecture §11). - Fix 3 (MINOR): emit would-import log lines in _log_dry_run_actions for any card_id in to_add/to_update_credential not already in the token map; add test_apply_dry_run_logs_would_import_for_unknown_cards. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
§9 is correct (verified end-to-end against the real controller during design); the §8 step 2 wording was a stale draft. Implementation already follows §9. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
There was a problem hiding this comment.
Pull request overview
Adds a synchronous UniFi Access local-API client to support read/write reconciliation (fetch users and apply diffs), including credential import/binding and policy updates, plus the required config and documentation updates.
Changes:
- Introduces
UnifiClientwith TLS fingerprint pinning, retries/envelope unwrapping,fetch_users(), andapply(diff)write orchestration. - Extends UniFi configuration with
facility_code(validated 0–255) and updates example config + config tests. - Adds a comprehensive UniFi client test suite (TLS pinning, retries, pagination, dry-run, import/bind/update/deactivate flows) and supporting docs/spec/plan.
Reviewed changes
Copilot reviewed 6 out of 8 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
src/door_sync/unifi/client.py |
New UniFi Access local-API client implementation (TLS pinning, retries, fetch/apply, card import/bind). |
src/door_sync/config.py |
Adds facility_code to UnifiConfig and validates it during config load. |
config.example.toml |
Documents and provides an example facility_code value for UniFi. |
tests/test_unifi_client.py |
New end-to-end unit tests for UniFi client behaviors using pytest-httpx. |
tests/test_config.py |
Updates existing config tests and adds facility-code validation tests. |
docs/superpowers/specs/2026-05-22-unifi-client-design.md |
Design spec describing the UniFi client API, flows, and constraints. |
docs/superpowers/plans/2026-05-22-unifi-client.md |
Implementation plan/checklist for the UniFi client slice. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
If config.host omits a port (e.g. https://controller.example.org), _verify_tls_fingerprint defaulted to 12445 while httpx.Client defaulted to 443 — TLS pinned on one endpoint, API calls hit another. Resolve hostname+port once in __init__, store on the instance, and use them for both. Adds two regression tests covering no-port and custom-port hosts. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
CiviCRM contact_ids are positive integers (auto-increment from 1), so a UniFi user with employee_number "0" or negative was not provisioned by this reconciler. Without this guard, such a user would land in to_deactivate on every cycle since the reconciler's diff finds no matching ResolvedMember and treats them as a stale sync-managed user. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
The previous error message included the raw nfc_id from a malformed import response, which encodes both facility code and card number — a violation of architecture §11 "never log full card IDs". The replacement surfaces only the FC byte (operational, not credential) when the value parses as hex, and falls back to a structural "not valid hex" message otherwise. Two regression tests assert neither the raw string nor the card-number portion ever appears. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
CodeQL flagged ssl.SSLContext(PROTOCOL_TLS_CLIENT) for permitting TLS 1.0/1.1. Modern Python+OpenSSL default to TLS 1.2+ already, but setting minimum_version explicitly is defense in depth (and silences the finding) — even with fingerprint pinning, refusing to negotiate older protocol versions removes a downgrade attack vector. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
UnifiClient.fetch_users()andUnifiClient.apply(diff)for read/write reconciliation against the UniFi Access local API on port 12445 (self-signed cert pinned via SHA-256 fingerprint). Synchttpx, mirrors theCivicrmClientshape — same retry helper (3 attempts, exponential backoff with jitter,Retry-Afterhonored), context-manager protocol, single public exception (UnifiClientError).card_id(decimal HID Prox 26-bit) and UniFi NFC tokens via the documented 2-column CSV import endpoint. The encodingnfc_id = (facility_code << 16) | card_id(uppercase hex) was verified end-to-end during design against two real cards, including a door unlock after API-imported binding.facility_codetoUnifiConfig. Honorsdry_run(zero HTTP writes; redacted log lines with last-4 only). 75ms inter-call delay between writes per architecture §5. Apply order: pre-import → deactivate → update_credential → update_policy → add (create + reactivate paths).Test Plan
uv run pytest— 201/201 passing (50 new UniFi-client tests + 2 new config tests, plus 149 pre-existing)uv run mypy --strict src tests— cleanuv run ruff check .— cleanconfig.example.tomlparses cleanly with a minimal env file (covered bytest_unifi_client_constructs_from_loaded_configand the existing example-config drift test)🤖 Generated with Claude Code