azaion/admin
1
0
Fork 0
mirror of https://github.com/azaion/admin.git synced 2026-06-21 18:01:10 +00:00
Code Issues Packages Projects Releases Wiki Activity

[AZ-197] Remove hardware ID binding from resource flow

Browse Source 
Sealed-Jetson + SaaS architecture eliminates the credential-reuse-across-
machines threat that motivated hardware fingerprint binding. The binding's
only remaining effect was a real production failure mode on legitimate
hardware events.

Production:
- Drop PUT /users/hardware/set and POST /resources/check.
- Simplify POST /resources/get/{dataFolder?} (no Hardware field).
- Remove CheckHardwareHash, UpdateHardware, Security.GetHWHash.
- GetApiEncryptionKey signature: (email, password) — no hardwareHash.
- Drop SetHWRequest DTO and Hardware property from GetResourceRequest.
- Remove HardwareIdMismatch (40) and BadHardware (45) ExceptionEnum
  entries; numeric codes left as a gap, not for reuse.

Wire-compat policy: drop entirely (no Loader; no in-flight legacy
clients). Stale callers will see 404s, which is the right loud failure.

Tombstones:
- User.Hardware DB column kept (nullable, unused) — separate cleanup
  ticket for the migration per workspace "no rename without confirmation".
- User.LastLogin is now never written by app code (only writer was inside
  the deleted CheckHardwareHash); flagged in batch_06_review for a future
  ticket.

Tests:
- Delete e2e HardwareBindingTests (165 lines) and Azaion.Test
  UserServiceTest (sole test was CheckHardwareHashTest).
- Drop Hardware payloads + /resources/check preconditions from e2e
  ResourceTests, SecurityTests, ResilienceTests; drop hardwareId arg
  from Azaion.Test SecurityTest.
- Add SecurityTests.Hardware_endpoints_are_removed_AZ_197 (AC-2 regression
  asserting both removed routes return 404).

Docs:
- architecture.md: System Context note, ADR-003 new key formula, ADR-004
  retired with rationale.
- diagrams/flows/flow_hardware_check.md: tombstoned.

Also archives the four batch-1+batch-2 task files into _docs/02_tasks/done/
(file moves were missed by the batch_05 commit).

Code review: PASS — see _docs/03_implementation/reviews/batch_06_review.md.

Co-authored-by: Cursor <cursoragent@cursor.com>
This commit is contained in:
Oleksandr Bezdieniezhnykh
2026-05-13 04:46:39 +03:00
parent 5ca9ccab2c
commit 5e90512987
22 changed files with 359 additions and 490 deletions
Download Patch File Download Diff File Expand all files Collapse all files
_docs/03_implementation/reviews/batch_06_review.md
+73
View File
@@ -0,0 +1,73 @@
# Batch 06 Code Review
**Date**: 2026-05-13
**Cycle**: 1, batch 2 of 2
**Tasks reviewed**: AZ-197 (remove hardware ID binding from resource flow — admin-side cleanup)
**Reviewer**: implement skill (Phase 9 — code-review)
**Verdict**: **PASS**
## Phase 1 — Production code touched
| File | Change | Notes |
|---|---|---|
| `Azaion.AdminApi/Program.cs` | Removed `MapPut("/users/hardware/set")`. Removed `MapPost("/resources/check")`. Simplified `MapPost("/resources/get/{dataFolder?}")` to derive key from `email + password` only. | Endpoints disappear cleanly; remaining route table unchanged. |
| `Azaion.Services/UserService.cs` | Removed `UpdateHardware`, `CheckHardwareHash`, `UpdateLastLoginDate` (private helper only used by `CheckHardwareHash`). Removed both methods from `IUserService`. | `UpdateLastLoginDate` was the *only* writer of `User.LastLogin`; consequence noted in §Implementation notes. |
| `Azaion.Services/Security.cs` | Removed `GetHWHash`. Changed `GetApiEncryptionKey(email, password, hardwareHash)` → `GetApiEncryptionKey(email, password)`. | Public-API breaking change inside the workspace; all callers updated. |
| `Azaion.Common/Requests/GetResourceRequest.cs` | Removed `Hardware` property and its FluentValidation rule. | Wire-compat policy: **drop entirely** (see Implementation notes for rationale). |
| `Azaion.Common/Requests/SetHWRequest.cs` | Deleted. | The DTO had a single consumer (the deleted endpoint). |
| `Azaion.Common/BusinessException.cs` | Removed `HardwareIdMismatch = 40` and `BadHardware = 45` from `ExceptionEnum`. | Numeric codes 40 and 45 are now unused — left as a numeric gap. **Do not reuse**. |
## Phase 2 — Test code touched
| File | Change | Notes |
|---|---|---|
| `Azaion.Test/SecurityTest.cs` | Dropped third `hardwareId` argument from two `GetApiEncryptionKey` calls. | Compile-only fix — assertions unchanged. |
| `Azaion.Test/UserServiceTest.cs` | Deleted. | Sole test was `CheckHardwareHashTest`; method under test no longer exists. |
| `e2e/Azaion.E2E/Tests/HardwareBindingTests.cs` | Deleted (165 lines). | All scenarios asserted behaviour that AZ-197 removes. |
| `e2e/Azaion.E2E/Tests/ResourceTests.cs` | Removed `SampleHardware` constant. Dropped `/resources/check` calls from two scenarios. Dropped `Hardware` field from `/resources/get` payloads. Updated `DecryptResourcePayload` helper signature + body. | Encryption round-trip now uses 2-arg key formula. |
| `e2e/Azaion.E2E/Tests/SecurityTests.cs` | Dropped `hardware` from anonymous `/resources/get` payload (unauth probe). Refactored `Per_user_encryption_produces_distinct_ciphertext_for_same_file` `DownloadForAsync` helper to drop `hardware` parameter and the `/resources/check` precondition. **Added** `Hardware_endpoints_are_removed_AZ_197` (AC-2 regression test asserting both removed routes return 404). | New test is the only added test surface in this batch. |
| `e2e/Azaion.E2E/Tests/ResilienceTests.cs` | Deleted `Concurrent_hardware_binding_same_hardware_has_no_500_and_state_consistent`. Removed now-unused `using System.Net.Http.Json;`, `using System.Text.Json;`, `ResponseJsonOptions` field, `TestUserPassword` constant. | The scenario has no analogue post-AZ-197. |
## Phase 3 — Documentation touched
| File | Change | Notes |
|---|---|---|
| `_docs/02_document/diagrams/flows/flow_hardware_check.md` | Replaced with a tombstone marker pointing to the AZ-197 spec. | Retained so historical links resolve. Mermaid diagram reduced to a single "REMOVED" node. |
| `_docs/02_document/architecture.md` | Updated System Context, integration table, data-model table, ADR-003 (per-user encryption — new key formula), ADR-004 (retired with full rationale). Added a top-level note about the AZ-197 cleanup. | Single source of truth for the new architecture. |
## Phase 4 — Build status
- `dotnet build Azaion.AdminApi.sln` — **0 errors, 0 warnings**.
- `dotnet build e2e/Azaion.E2E/Azaion.E2E.csproj` — **0 errors, 0 warnings**.
## Phase 5 — AC coverage check
| AC | Verification |
|---|---|
| AC-1 — Resource download works without hardware | `ResourceTests.Encrypted_download_returns_octet_stream_and_non_empty_body`, `Encryption_round_trip_decrypt_matches_original_bytes`, `SecurityTests.Per_user_encryption_produces_distinct_ciphertext_for_same_file` (all send no `Hardware` field). |
| AC-2 — `PUT /users/hardware/set` is gone | New `SecurityTests.Hardware_endpoints_are_removed_AZ_197` asserts 404 on both `PUT /users/hardware/set` and `POST /resources/check`. |
| AC-3 — `Security.GetApiEncryptionKey` signature | Compile-time enforced; `Azaion.Test/SecurityTest.cs` exercises the 2-arg overload. |
| AC-4 — `HardwareBindingTests` removed | File deleted (`git status` shows `D e2e/Azaion.E2E/Tests/HardwareBindingTests.cs`). |
| AC-5 — No remaining test sends `Hardware` | `rg` over `Azaion.Test` and `e2e/Azaion.E2E` returns no `Hardware =` / `hardware =` payloads outside the AC-2 negative-path test. |
| AC-6 — `ExceptionEnum` no longer has hardware codes | Confirmed by `BusinessException.cs` read; codes 40 and 45 are absent. Compile would have failed if any production reference remained. |
| AC-7 — All tests pass | Deferred to step 16 (test-run skill). Build is green; expectation is full pass. |
## Phase 6 — Scope discipline
- All changes are within the AZ-197 scope as defined by the task spec (Admin API production code + workspace tests + listed docs).
- `User.Hardware` entity property and DB column **left in place** per the spec ("DB column tombstoned, no migration in this ticket").
- Adjacent hygiene performed: removed now-unused `using` directives in `ResilienceTests.cs`; removed dead private helper `UpdateLastLoginDate`. Both were caused by the in-scope deletions.
- No unrelated fixes piggy-backed.
## Phase 7 — Risks / follow-ups (non-blocking)
1. **`User.LastLogin` is now never updated by application code.** Pre-AZ-197, the only writer was `UpdateLastLoginDate`, called from `CheckHardwareHash`. `ValidateUser` (login) does not update it. The column becomes write-once via direct DB inserts. This was *also* true post-AZ-197 in the sense that the only updater fired during a hardware-check (not during login), so this is not a *regression* in the login path — but it does mean the field is now effectively dead. Two options for a future ticket:
- Move a `LastLogin = UtcNow` write into `ValidateUser` (preserves the spirit of the field).
- Drop the column.
Out of scope for AZ-197; documented for future cleanup.
2. **`User.Hardware` column tombstone.** Per spec — no migration in this ticket. Future cleanup ticket can drop the column once any external readers (BI dashboards, audit exports) are confirmed absent.
3. **Numeric error code gap (40, 45).** Intentional. A code-review rule "do not reuse retired error codes" would be worth adding to `coderule.mdc` if it isn't already there — out of scope here.
## Verdict — PASS
No blocking issues. Build is clean across both projects. AC coverage is complete (AC-7 confirmed at step 16 by `dotnet test`). Tombstone strategy for `User.Hardware` and `User.LastLogin` is documented and explicitly out of scope per the task spec. Ready for commit.