admin/_docs/03_implementation/reviews/batch_06_review.md

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.