From cd40483cd43c04242816ed7121b283caa17ce724 Mon Sep 17 00:00:00 2001 From: Yisroel Baum Date: Sun, 26 Apr 2026 10:46:07 +0300 Subject: [PATCH] remove default values from user constructors Forcing every call site to be explicit about admin status and password eliminates a class of bugs where an unintended isAdmin=false or empty passwordHash could silently slip through. The CreateUserTest case that asserted the isAdmin default is dropped since the default no longer exists. --- app/User/UseCases/CreateUserRequest.php | 4 ++-- app/User/User.php | 4 ++-- tests/Unit/Auth/UseCases/CreateSessionTest.php | 2 ++ .../UseCases/CreateScheduledNodeTest.php | 7 ++++++- .../Unit/User/UseCases/AuthenticateUserTest.php | 1 + tests/Unit/User/UseCases/CreateUserTest.php | 16 +++++++--------- tests/e2e/Controllers/AuthControllerTest.php | 1 + 7 files changed, 21 insertions(+), 14 deletions(-) diff --git a/app/User/UseCases/CreateUserRequest.php b/app/User/UseCases/CreateUserRequest.php index 52ead72..c70ef72 100644 --- a/app/User/UseCases/CreateUserRequest.php +++ b/app/User/UseCases/CreateUserRequest.php @@ -6,7 +6,7 @@ class CreateUserRequest { public function __construct( public ?string $email, - public ?string $password = null, - public bool $isAdmin = false, + public ?string $password, + public bool $isAdmin, ) {} } diff --git a/app/User/User.php b/app/User/User.php index 9cdc469..9e8ae97 100644 --- a/app/User/User.php +++ b/app/User/User.php @@ -9,8 +9,8 @@ class User public function __construct( private int $id, private EmailAddress $email, - private string $passwordHash = '', - private bool $isAdmin = false, + private string $passwordHash, + private bool $isAdmin, ) {} public function getId(): int diff --git a/tests/Unit/Auth/UseCases/CreateSessionTest.php b/tests/Unit/Auth/UseCases/CreateSessionTest.php index f58f66f..a79eb9c 100644 --- a/tests/Unit/Auth/UseCases/CreateSessionTest.php +++ b/tests/Unit/Auth/UseCases/CreateSessionTest.php @@ -36,6 +36,8 @@ class CreateSessionTest extends TestCase $this->user = new User( id: 7, email: new EmailAddress('test@test.com'), + passwordHash: 'hashed:password1', + isAdmin: false, ); } diff --git a/tests/Unit/ScheduledNode/UseCases/CreateScheduledNodeTest.php b/tests/Unit/ScheduledNode/UseCases/CreateScheduledNodeTest.php index d2bfc2b..477a07a 100644 --- a/tests/Unit/ScheduledNode/UseCases/CreateScheduledNodeTest.php +++ b/tests/Unit/ScheduledNode/UseCases/CreateScheduledNodeTest.php @@ -30,7 +30,12 @@ class CreateScheduledNodeTest extends TestCase $this->planRepo = new FakePlanRepository(); $this->planRepo->create(new CreatePlanDto( name: 'testplan', - user: new User(0, new EmailAddress('test@test.com')), + user: new User( + id: 0, + email: new EmailAddress('test@test.com'), + passwordHash: 'hashed:password1', + isAdmin: false, + ), )); $this->useCase = new CreateScheduledNode( $this->scheduledNodeRepo, diff --git a/tests/Unit/User/UseCases/AuthenticateUserTest.php b/tests/Unit/User/UseCases/AuthenticateUserTest.php index 822eda3..88bb1a9 100644 --- a/tests/Unit/User/UseCases/AuthenticateUserTest.php +++ b/tests/Unit/User/UseCases/AuthenticateUserTest.php @@ -30,6 +30,7 @@ class AuthenticateUserTest extends TestCase $createUser->execute(new CreateUserRequest( email: 'test@test.com', password: 'password1', + isAdmin: false, )); $this->useCase = new AuthenticateUser( $this->userRepo, diff --git a/tests/Unit/User/UseCases/CreateUserTest.php b/tests/Unit/User/UseCases/CreateUserTest.php index 941c98d..0333a52 100644 --- a/tests/Unit/User/UseCases/CreateUserTest.php +++ b/tests/Unit/User/UseCases/CreateUserTest.php @@ -31,6 +31,7 @@ class CreateUserTest extends TestCase $this->useCase->execute(new CreateUserRequest( email: 'test@test.com', password: 'password1', + isAdmin: false, )); $user = $this->userRepo->find(0); $this->assertInstanceOf(User::class, $user); @@ -44,17 +45,9 @@ class CreateUserTest extends TestCase $this->useCase->execute(new CreateUserRequest( email: null, - )); - } - - public function test_is_admin_defaults_to_false(): void - { - $this->useCase->execute(new CreateUserRequest( - email: 'test@test.com', password: 'password1', + isAdmin: false, )); - $user = $this->userRepo->find(0); - $this->assertFalse($user->isAdmin()); } public function test_is_admin_can_be_set_true(): void @@ -73,6 +66,7 @@ class CreateUserTest extends TestCase $this->useCase->execute(new CreateUserRequest( email: 'test@test.com', password: 'password1', + isAdmin: false, )); $this->expectException(BadRequestException::class); @@ -81,6 +75,7 @@ class CreateUserTest extends TestCase $this->useCase->execute(new CreateUserRequest( email: 'test@test.com', password: 'password1', + isAdmin: false )); } @@ -92,6 +87,7 @@ class CreateUserTest extends TestCase $this->useCase->execute(new CreateUserRequest( email: 'test@test.com', password: null, + isAdmin: false, )); } @@ -105,6 +101,7 @@ class CreateUserTest extends TestCase $this->useCase->execute(new CreateUserRequest( email: 'test@test.com', password: 'short', + isAdmin: false, )); } @@ -113,6 +110,7 @@ class CreateUserTest extends TestCase $this->useCase->execute(new CreateUserRequest( email: 'test@test.com', password: 'password1', + isAdmin: false, )); $user = $this->userRepo->find(0); $this->assertNotEquals('password1', $user->getPasswordHash()); diff --git a/tests/e2e/Controllers/AuthControllerTest.php b/tests/e2e/Controllers/AuthControllerTest.php index 19c5770..50e39a5 100644 --- a/tests/e2e/Controllers/AuthControllerTest.php +++ b/tests/e2e/Controllers/AuthControllerTest.php @@ -63,6 +63,7 @@ class AuthControllerTest extends TestCase $this->createUser->execute(new CreateUserRequest( email: 'existing@test.com', password: 'password1', + isAdmin: false, )); $this->controller = new AuthController();