diff --git a/app/Http/Requests/Api/Client/Servers/Subusers/StoreSubuserRequest.php b/app/Http/Requests/Api/Client/Servers/Subusers/StoreSubuserRequest.php index 6e87225164..28accf20df 100644 --- a/app/Http/Requests/Api/Client/Servers/Subusers/StoreSubuserRequest.php +++ b/app/Http/Requests/Api/Client/Servers/Subusers/StoreSubuserRequest.php @@ -14,7 +14,7 @@ public function permission(): string public function rules(): array { return [ - 'email' => 'required|email|between:1,191', + 'email' => 'required|email:strict|between:1,191', 'permissions' => 'required|array', 'permissions.*' => 'string', ]; diff --git a/app/Models/User.php b/app/Models/User.php index 02df0ca093..e0de1d4251 100644 --- a/app/Models/User.php +++ b/app/Models/User.php @@ -167,7 +167,7 @@ class User extends Model implements */ public static array $validationRules = [ 'uuid' => 'required|string|size:36|unique:users,uuid', - 'email' => 'required|email|between:1,191|unique:users,email', + 'email' => 'required|email:strict|between:1,191|unique:users,email', 'external_id' => 'sometimes|nullable|string|max:191|unique:users,external_id', 'username' => 'required|between:1,191|unique:users,username', 'name_first' => 'required|string|between:1,191', diff --git a/tests/Integration/Api/Client/AccountControllerTest.php b/tests/Integration/Api/Client/AccountControllerTest.php index 27d50f28c9..0e3ce8422f 100644 --- a/tests/Integration/Api/Client/AccountControllerTest.php +++ b/tests/Integration/Api/Client/AccountControllerTest.php @@ -99,6 +99,35 @@ public function testEmailIsNotUpdatedWhenNotValid() $response->assertStatus(Response::HTTP_UNPROCESSABLE_ENTITY); $response->assertJsonPath('errors.0.meta.rule', 'email'); $response->assertJsonPath('errors.0.detail', 'The email must be a valid email address.'); + + + /* + * RFCs limit certain parts of an email to certain character limits. + * A limit of <= 64 for the local, then <= 63 for each domain label. + */ + $local = str_repeat(Str::random(10), 6) . '1234'; + $label = str_repeat(Str::random(10), 6) . '1'; + + + $response = $this->actingAs($user)->putJson('/api/client/account/email', [ + 'email' => "1$local@$label.$label", // exceed RFC limit for local part + 'password' => 'password', + ]); + + $response->assertStatus(Response::HTTP_UNPROCESSABLE_ENTITY); + $response->assertJsonPath('errors.0.detail', 'The email must be a valid email address.'); + $response->assertJsonPath('errors.0.meta.source_field', 'email'); + + + $response = $this->actingAs($user)->putJson('/api/client/account/email', [ + 'email' => "$local@1234$label.$label", // exceed RFC limit for label part + 'password' => 'password', + ]); + + $response->assertStatus(Response::HTTP_UNPROCESSABLE_ENTITY); + $response->assertJsonPath('errors.0.detail', 'The email must be a valid email address.'); + $response->assertJsonPath('errors.0.meta.source_field', 'email'); + } /** @@ -137,8 +166,8 @@ public function testPasswordIsUpdated() $this->assertNotEquals($server->node_id, $server2->node_id); Bus::assertDispatchedTimes(RevokeSftpAccessJob::class, 2); - Bus::assertDispatched(fn (RevokeSftpAccessJob $job) => $job->user === $user->uuid && $job->target->is($server->node)); - Bus::assertDispatched(fn (RevokeSftpAccessJob $job) => $job->user === $user->uuid && $job->target->is($server2->node)); + Bus::assertDispatched(fn(RevokeSftpAccessJob $job) => $job->user === $user->uuid && $job->target->is($server->node)); + Bus::assertDispatched(fn(RevokeSftpAccessJob $job) => $job->user === $user->uuid && $job->target->is($server2->node)); } /** diff --git a/tests/Integration/Api/Client/Server/Subuser/CreateServerSubuserTest.php b/tests/Integration/Api/Client/Server/Subuser/CreateServerSubuserTest.php index 12aef45e88..86725c76f5 100644 --- a/tests/Integration/Api/Client/Server/Subuser/CreateServerSubuserTest.php +++ b/tests/Integration/Api/Client/Server/Subuser/CreateServerSubuserTest.php @@ -79,7 +79,21 @@ public function testSubuserWithExcessivelyLongEmailCannotBeCreated() { [$user, $server] = $this->generateTestAccount(); - $email = str_repeat(Str::random(20), 9) . '1@gmail.com'; // 191 is the hard limit for the column in MySQL. + /* + * RFCs limit certain parts of an email to certain character limits. + * + * A limit of <= 64 for the local, then <= 63 for each domain label. + * We will stay below the limit to make sure we're within the 191 column limit for emails. + */ + $local = str_repeat(Str::random(10), 6) . '1234'; + $label = str_repeat(Str::random(10), 6) . '1'; + + // Make sure we're within the column limit + $email = "$local@$label.$label.au"; + + $this->assertSame(64, strlen($local)); + $this->assertSame(61, strlen($label)); + $this->assertSame(191, strlen($email)); $response = $this->actingAs($user)->postJson($this->link($server) . '/users', [ 'email' => $email, @@ -90,8 +104,13 @@ public function testSubuserWithExcessivelyLongEmailCannotBeCreated() $response->assertOk(); + // Exceed column limit of 1 >= and <= 191 + $email = "$local@$label.$label.com"; + + $this->assertSame(192, strlen($email)); + $response = $this->actingAs($user)->postJson($this->link($server) . '/users', [ - 'email' => $email . '.au', + 'email' => $email, 'permissions' => [ Permission::ACTION_USER_CREATE, ],