feat: PR-5 - Person API endpoints with auth and permissions (Issue #50) #64

kevalyq · 2025-11-01T23:29:04Z

🎯 Overview

Implements RESTful Person API endpoints following Issue #50 PR-5 acceptance criteria. Full TDD approach with comprehensive test coverage, authentication, authorization, and tenant isolation.

Fixes #50 (PR-5)

📋 Endpoints

POST `/api/v1/tenants/{tenant}/persons`

Permission: person.write
Purpose: Create or update Person

Request:

{
  "email_plain": "user@example.com",
  "phone_plain": "+49 123 456789",  // optional
  "note_enc": "Optional note"       // optional
}

Response: 201 Created

{
  "id": 1,
  "tenant_id": 1,
  "created_at": "2025-11-02T...",
  "updated_at": "2025-11-02T..."
}

GET `/api/v1/tenants/{tenant}/persons/by-email?email=...`

Permission: person.read
Purpose: Search Person by email (case-insensitive)
Response: 200 OK or 404 Not Found

🔐 Security & Architecture

Middleware Stack:

auth:sanctum → tenant → permission:{person.write|person.read}

Features:

✅ Sanctum PAT authentication required
✅ Tenant-scoped access via SetTenant middleware
✅ Spatie Permission RBAC with team context
✅ Request validation via StorePersonRequest
✅ No encrypted fields or blind indexes exposed in responses
✅ Case-insensitive email search (normalized at blind index level)
✅ Phone is optional (nullable phone_enc, phone_idx)

🧪 Testing

PersonApiTest - 16 comprehensive tests:

✅ Authentication: 401 when not authenticated
✅ Authorization: 403 without permissions
✅ Validation: 422 for invalid data, 400 for missing query params
✅ Success: 201 create, 200 search
✅ Tenant isolation: 404 for cross-tenant access
✅ Security: Verifies no _enc/_idx exposure
✅ Case-insensitive search

Test Helper:

givePermissionWithTenant(User $user, int $tenantId, string $permission)

Sets Spatie Permission team context for proper tenant-scoped permissions.

📊 Quality Metrics

Tests: 95 passed (237 assertions) ✅
PHPStan: Level 9, 0 errors ✅
Pint: PSR-12 compliant (53 files) ✅
LOC: ~439 new lines (under 600 limit) ✅
Parallel Tests: Stable with 2 processes ✅

📁 Changes

New Files

app/Http/Controllers/PersonController.php (93 LOC)
app/Http/Requests/StorePersonRequest.php (58 LOC)
tests/Feature/PersonApiTest.php (288 LOC)

Modified Files

routes/api.php - Added Person routes with middleware
bootstrap/app.php - Registered Spatie Permission middleware aliases
app/Models/User.php - Added HasRoles trait
database/migrations/2025_11_01_210213_change_bytea_columns_to_varchar.php - Made phone_enc/phone_idx nullable
phpstan.neon - Added ignores for PEST test patterns

🔗 Dependencies

Requires:

✅ PR-4: Sanctum authentication (feat: Add Sanctum API token authentication (Issue #50 PR-4) #60, fix: Resolve PostgreSQL BYTEA binary encoding issue (Issue #50 PR-4) #61)
✅ PR-3: SetTenant middleware (feat: PR-3 - Tenant Middleware & RBAC Wiring #55)
✅ PR-2: PersonRepository with blind index search
✅ PR-1: Person model with PersonObserver

Prepares for:

⏳ PR-6: Key rotation & maintenance commands

🎓 Design Decisions

Permission Helper Function: Created givePermissionWithTenant() to properly set Spatie Permission team context, avoiding tenant_id NOT NULL constraint violations.
PHPDoc Type Hints: Added @var int $tenantId annotations in controller to satisfy PHPStan Level 9 with mixed route parameters.
PHPStan Ignores: Added patterns for PEST dynamic properties ($this->tenant, $this->user) to avoid false positives.
Nullable Phone: Made phone_enc and phone_idx nullable since phone is an optional field.
Sanitized Responses: PersonController returns only safe fields (id, tenant_id, timestamps) - encrypted fields and blind indexes are hidden via model $hidden property.

✅ Issue #50 PR-5 Acceptance Criteria

Routes under [auth:api, SetTenant]
Spatie middleware permission:person.write/read
Feature tests: 201/200/403
No _enc/_idx exposure
All PRs green in CI (local checks passed)
Small PR (~400-600 LOC)
Conventional Commits format

🧑‍💻 Review Checklist

Test coverage is comprehensive
No sensitive data exposed in responses
Middleware stack is correct
Permissions are properly scoped to tenants
Code follows PSR-12 and PHPStan Level 9
Commit message follows Conventional Commits

Ready for review! 🚀

Implemented process-specific KEK files to eliminate race conditions in parallel test execution. Root cause: - All tests shared the same KEK file (storage/app/keys/kek.key) - Parallel processes overwrote each other's KEK files - Led to 'Failed to unwrap idx_key' errors (~1-10% failure rate) Solution: - Added TenantKey::setKekPath() to support custom KEK paths - Modified getKekPath() to use static $kekPath property when set - Updated TenantKeyTest and PersonTest to use process-specific paths: storage/app/keys/kek-test-{pid}.key - Added cleanup in afterEach hooks Testing: - Verified 10 consecutive successful parallel test runs (79/79 pass) - Tests run in 7-8s with 2 parallel processes - PHPStan Level 9: Clean - Pint PSR-12: Compliant Fixes #62

TenantKey::generateKek() already handles directory creation, so the manual mkdir in PersonTest.php beforeEach is unnecessary. This simplifies the code and reduces duplication. Addresses Copilot review comment on PR #63.

Implements JSON API endpoints for Person resource with full auth/authz. ## Core Implementation **PersonController** (93 LOC): - POST /api/v1/tenants/{tenant}/persons - Create/update Person - GET /api/v1/tenants/{tenant}/persons/by-email - Search by email - Uses PersonRepository for business logic - Returns sanitized JSON (no _enc/_idx exposure) **StorePersonRequest** (58 LOC): - Validates email_plain (required, email format) - phone_plain and note_enc are optional - Custom error messages **Routes** (routes/api.php): - Nested under auth:sanctum middleware - SetTenant middleware for tenant context - Spatie Permission middleware (person.write, person.read) ## Middleware Setup **bootstrap/app.php**: - Registered 'permission' and 'role' middleware aliases - Required for Spatie Permission integration **User Model**: - Added HasRoles trait from Spatie Permission - Enables permission assignment in tests ## Testing **PersonApiTest** (288 LOC, 16 tests): - ✅ Authentication (401 when not authenticated) - ✅ Authorization (403 without permissions) - ✅ Validation (422 for invalid data) - ✅ Success cases (201 create, 200 search) - ✅ Tenant isolation (404 for wrong tenant) - ✅ Security (no _enc/_idx in responses) - ✅ Case-insensitive search - All 95 tests passing (237 assertions) ## Database Changes **Migration update**: - Made phone_enc and phone_idx nullable - Allows creating Person with only email ## Quality Gates - ✅ **Tests**: 95 passed, 237 assertions - ✅ **PHPStan**: Level 9, 0 errors - ✅ **Pint**: PSR-12 compliant - ✅ **LOC**: ~439 new lines (under 600 limit) - ✅ **Parallel tests**: Working perfectly ## API Examples **Create Person:** ```bash POST /api/v1/tenants/1/persons Authorization: Bearer {token} { "email_plain": "user@example.com", "phone_plain": "+49 123 456789", "note_enc": "Optional note" } # Returns: {"id": 1, "tenant_id": 1, "created_at": "...", "updated_at": "..."} ``` **Search by Email:** ```bash GET /api/v1/tenants/1/persons/by-email?email=user@example.com Authorization: Bearer {token} # Returns: {"id": 1, "tenant_id": 1, "created_at": "...", "updated_at": "..."} ``` ## Related - Issue #50 (PR-5: API Endpoints) - Follows: PR-4 (Sanctum Auth), PR-3 (SetTenant Middleware) - Prepares for: PR-6 (Rotation & Maintenance) ## Notes - Permissions are team-scoped (tenant_id) via Spatie configuration - Encrypted fields and blind indexes never exposed in responses - Search is case-insensitive (normalized at blind index level) - Phone is optional (can create Person with email only)

Copilot

Pull Request Overview

This PR implements Person API endpoints with parallel test execution support. The main focus is adding two new REST API endpoints for managing Person resources (create and search by email) and ensuring test isolation when running tests in parallel.

Key changes:

Added two Person API endpoints: POST /v1/tenants/{tenant}/persons and GET /v1/tenants/{tenant}/persons/by-email
Implemented process-specific KEK file paths in tests to enable parallel test execution without file conflicts
Made phone_enc and phone_idx columns nullable in the database migration

Reviewed Changes

Copilot reviewed 11 out of 11 changed files in this pull request and generated 4 comments.

Show a summary per file

File	Description
app/Http/Controllers/PersonController.php	New controller with `store()` and `byEmail()` endpoints for Person resource management
app/Http/Requests/StorePersonRequest.php	Form request class with validation rules for Person creation
routes/api.php	Registered Person endpoints under tenant-scoped routes with permission middleware
app/Models/User.php	Added `HasRoles` trait to enable Spatie permission functionality
app/Models/TenantKey.php	Added `setKekPath()` method to allow test-specific KEK file paths
bootstrap/app.php	Registered `permission` and `role` middleware aliases
tests/Feature/PersonApiTest.php	Comprehensive API tests covering authentication, authorization, validation, and tenant isolation
tests/Feature/TenantKeyTest.php	Updated to use process-specific KEK paths for parallel execution
tests/Feature/PersonTest.php	Updated to use process-specific KEK paths for parallel execution
database/migrations/2025_11_01_210213_change_bytea_columns_to_varchar.php	Made phone fields nullable
phpstan.neon	Added PHPStan ignores for test dynamic properties and API test patterns

tests/Feature/PersonApiTest.php

app/Http/Controllers/PersonController.php

…rces Addresses Copilot PR review comments for Issue #50 PR-5. ## DRY Violations Fixed **1. KEK Path Construction (3x duplication):** - Centralized getTestKekPath() in tests/Pest.php - Removed duplicate functions from PersonTest, TenantKeyTest, PersonApiTest - Added cleanupTestKekFile() helper for consistent cleanup **2. Permission Assignment (middleware duplication):** - Moved givePermissionWithTenant() to tests/Pest.php - Used across PersonApiTest for proper Spatie Permission team context - Single source of truth for tenant-scoped permission logic **3. API Response Structure (2x duplication in Controller):** - Created PersonResource following Laravel best practices - Replaced manual JSON assembly in store() and byEmail() - Disabled resource wrapping with $wrap = null for clean responses - Consistent response format across all Person endpoints ## Changes **New:** - app/Http/Resources/PersonResource.php (45 LOC) **Modified:** - tests/Pest.php (+33 LOC) - Added 3 global test helpers - app/Http/Controllers/PersonController.php (-6 LOC) - Uses PersonResource - tests/Feature/PersonApiTest.php (-13 LOC) - Uses central helpers - tests/Feature/PersonTest.php (-5 LOC) - Uses getTestKekPath() - tests/Feature/TenantKeyTest.php (-10 LOC) - Uses cleanupTestKekFile() ## Benefits - ✅ Single source of truth for test KEK paths - ✅ Consistent permission assignment across tests - ✅ Laravel Resource pattern for API responses - ✅ ~44 LOC reduction through refactoring - ✅ Easier maintenance and updates ## Quality - ✅ Tests: 95 passed (237 assertions) - ✅ PHPStan: Level 9, 0 errors - ✅ Pint: PSR-12 compliant

Copilot

Pull Request Overview

Copilot reviewed 13 out of 13 changed files in this pull request and generated 1 comment.

tests/Feature/PersonApiTest.php

Resolved conflicts in PersonTest.php and TenantKeyTest.php. Kept our branch version (HEAD) which uses centralized test helpers from tests/Pest.php instead of local duplicate functions. This maintains DRY principles established in the refactoring commit.

Addresses Copilot review comment on PR #64. The manual setPermissionsTeamId() call on line 60 was redundant since givePermissionWithTenant() helper already handles setting and resetting the team ID context. This eliminates duplication and makes the code cleaner.

kevalyq added 3 commits November 1, 2025 23:56

refactor: remove redundant directory creation in PersonTest

f550264

TenantKey::generateKek() already handles directory creation, so the manual mkdir in PersonTest.php beforeEach is unnecessary. This simplifies the code and reduces duplication. Addresses Copilot review comment on PR #63.

Copilot AI review requested due to automatic review settings November 1, 2025 23:29

Copilot AI reviewed Nov 1, 2025

View reviewed changes

tests/Feature/PersonApiTest.php Outdated Show resolved Hide resolved

tests/Feature/PersonApiTest.php Outdated Show resolved Hide resolved

tests/Feature/PersonApiTest.php Outdated Show resolved Hide resolved

app/Http/Controllers/PersonController.php Outdated Show resolved Hide resolved

kevalyq requested a review from Copilot November 1, 2025 23:40

Copilot AI reviewed Nov 1, 2025

View reviewed changes

tests/Feature/PersonApiTest.php Outdated Show resolved Hide resolved

kevalyq added 2 commits November 2, 2025 00:43

kevalyq merged commit 2f5bbdd into main Nov 1, 2025
12 checks passed

kevalyq deleted the feat/pr-5-person-api-endpoints branch November 1, 2025 23:47

This was referenced Nov 2, 2025

feat: PR-7 - Hardening & cleanups (Issue #50) #67

Merged

SecPal API: Multi-tenant security, field encryption & blind indexes, Sanctum & Spatie Teams — TDD/PEST, DRY, best practices #50

Closed

kevalyq mentioned this pull request Nov 16, 2025

[EPIC] Secret Management System (Password Vault) #173

Open

4 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

feat: PR-5 - Person API endpoints with auth and permissions (Issue #50) #64

feat: PR-5 - Person API endpoints with auth and permissions (Issue #50) #64

Uh oh!

kevalyq commented Nov 1, 2025

Uh oh!

Copilot AI left a comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Copilot AI left a comment

Uh oh!

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

feat: PR-5 - Person API endpoints with auth and permissions (Issue #50) #64

feat: PR-5 - Person API endpoints with auth and permissions (Issue #50) #64

Uh oh!

Conversation

kevalyq commented Nov 1, 2025

🎯 Overview

📋 Endpoints

POST /api/v1/tenants/{tenant}/persons

GET /api/v1/tenants/{tenant}/persons/by-email?email=...

🔐 Security & Architecture

🧪 Testing

📊 Quality Metrics

📁 Changes

New Files

Modified Files

🔗 Dependencies

🎓 Design Decisions

✅ Issue #50 PR-5 Acceptance Criteria

🧑‍💻 Review Checklist

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull Request Overview

Reviewed Changes

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull Request Overview

Uh oh!

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

POST `/api/v1/tenants/{tenant}/persons`

GET `/api/v1/tenants/{tenant}/persons/by-email?email=...`