Skip to content

Conversation

@kevalyq
Copy link
Contributor

@kevalyq kevalyq commented Nov 1, 2025

Summary

Implements Sanctum-based API token authentication as part of Issue #50 (Multi-tenant KEK management).

This PR adds token-based authentication for API endpoints, enabling secure API access with token generation, validation, and revocation.

Changes

Core Implementation

  • User Model: Added HasApiTokens trait for Sanctum integration
  • AuthController (77 LOC):
    • POST /v1/auth/token: Generate API tokens with email/password credentials
    • POST /v1/auth/logout: Revoke current token
    • POST /v1/auth/logout-all: Revoke all user tokens
  • Migration: personal_access_tokens table for Sanctum token storage
  • Routes: Public auth endpoint + protected routes with auth:sanctum middleware
  • Example: /v1/me endpoint demonstrating authenticated access

Testing

  • AuthTest (264 LOC, 18 test cases):
    • ✅ Token generation with valid/invalid credentials
    • ✅ Validation (email, password required)
    • ✅ Multi-device token support
    • ✅ Protected endpoint authentication
    • ✅ Token revocation (single/all)
    • ✅ Security: No sensitive data exposure, tokens stored hashed
  • All tests passing: 57 tests (140 assertions)

Quality Gates

  • Pint: PSR-12 compliant (0 errors)
  • PHPStan: Level max (0 errors)
    • Extended ignores for PEST framework patterns
  • PEST: 57 tests passing (140 assertions)
  • Preflight: All checks passed (408 changed lines)

Code Metrics

  • Total: ~450 LOC
  • Controller: 77 LOC
  • Migration: 32 LOC
  • Tests: 264 LOC
  • Routes: 17 LOC
  • Config: 5 LOC (PHPStan ignores)

Review Checklist Compliance

Pre-push review performed per DEVELOPMENT.md:

  • git diff reviewed: No duplication, clear intent, proper structure
  • ddev exec ./vendor/bin/pest: 57 tests passing
  • ddev exec ./vendor/bin/pint: PSR-12 compliant
  • ddev exec ./vendor/bin/phpstan analyse: 0 errors
  • ✅ Test isolation: RefreshDatabase trait used in AuthTest
  • ✅ Single-iteration PR: All quality gates passed before push

Related

Notes

  • Sanctum config already existed in Laravel installation
  • No baseline needed (PHPStan already at level max)
  • Token expiry handled by Sanctum config (sanctum.expiration)
  • Future PRs will integrate with SetTenant middleware for tenant-scoped auth

- Add HasApiTokens trait to User model
- Create personal_access_tokens migration (Sanctum requirement)
- Implement AuthController with token generation, logout, logout-all
- Add public /auth/token endpoint for token generation
- Add protected /auth/logout and /auth/logout-all endpoints
- Add example protected /me endpoint demonstrating auth:sanctum middleware
- Add comprehensive AuthTest with 18 test cases covering:
  * Token generation (valid/invalid credentials, validation, multi-device)
  * Protected endpoint access (authentication required, token validation)
  * Token revocation (single/all tokens, database verification)
  * Security (no sensitive data exposure, token hashing)
- Extend PHPStan ignores for PEST framework test patterns
- All 57 tests passing (140 assertions)
- PHPStan level max: 0 errors
- Pint: PSR-12 compliant

Scope: ~450 LOC (Controller: 77, Migration: 32, Tests: 264, Routes: 17)
Addresses: #50
Copilot AI review requested due to automatic review settings November 1, 2025 20:04
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull Request Overview

This PR implements token-based API authentication using Laravel Sanctum. The implementation includes token generation, protected endpoints, token revocation, and comprehensive security tests.

  • Adds AuthController with methods for token generation, single logout, and logout-all functionality
  • Integrates Laravel Sanctum into the User model and creates the required personal access tokens migration
  • Defines public and protected API routes with Sanctum authentication middleware

Reviewed Changes

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

Show a summary per file
File Description
app/Http/Controllers/AuthController.php New controller implementing token generation and revocation endpoints
app/Models/User.php Adds HasApiTokens trait to enable Sanctum token functionality
database/migrations/2025_11_01_195727_create_personal_access_tokens_table.php Creates database table for storing Sanctum personal access tokens
routes/api.php Defines public token generation route and protected authenticated routes
tests/Feature/AuthTest.php Comprehensive test suite covering authentication flows, token management, and security
phpstan.neon Updates PHPStan ignore patterns to accommodate test method calls

- Extract validation to TokenRequest FormRequest (Laravel best practice)
- Refine PHPStan ignores: specific methods instead of wildcards
- Fix /me endpoint: explicit field selection to prevent sensitive data exposure
- Maintain type safety while supporting PEST framework patterns

Resolves all 4 Copilot review comments from PR #60.
@kevalyq kevalyq requested a review from Copilot November 1, 2025 20:12
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull Request Overview

Copilot reviewed 7 out of 7 changed files in this pull request and generated 20 comments.

- TokenRequest: Add 'nullable' to device_name validation (Comment #6)
- AuthController: Extract /me closure to me() method (Comment #5)
- routes/api.php: Replace inline closure with controller route
- AuthTest.php: Replace ALL assertStatus() with specific methods:
  - assertCreated() for 201 responses (7 occurrences)
  - assertUnprocessable() for 422 responses (4 occurrences)
  - assertUnauthorized() for 401 responses (4 occurrences)
  - assertOk() for 200 responses (5 occurrences)
- phpstan.neon: Add new assertion methods to ignore patterns

All 57 tests passing, Pint compliant, PHPStan 0 errors.
@kevalyq kevalyq requested a review from Copilot November 1, 2025 20:27
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull Request Overview

Copilot reviewed 7 out of 7 changed files in this pull request and generated 2 comments.

- Add messages() method to TokenRequest for custom validation messages
  (Laravel best practice: FormRequests should provide user-friendly errors)

- Fix null-safety in AuthTest line 89: first()->name to first()?->name
  (Prevents potential null pointer exception in test assertions)

These issues should have been caught in pre-commit self-review.
Root cause: Mechanical comment fixes without thorough code quality check.

Quality lesson learned: ALWAYS do comprehensive self-review BEFORE push,
not just fix automated comments. Check Laravel conventions, null-safety,
and best practices proactively.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants