From d660eb9a1ba1b3d75aff6e6df59bd36f0e6ba785 Mon Sep 17 00:00:00 2001 From: Allon Moritz Date: Wed, 21 Sep 2022 11:51:33 +0200 Subject: [PATCH 1/4] Do not checkout a record when the user is not logged in --- libraries/src/MVC/Model/FormModel.php | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/libraries/src/MVC/Model/FormModel.php b/libraries/src/MVC/Model/FormModel.php index e9a14c3ce47ee..736876888ffd1 100644 --- a/libraries/src/MVC/Model/FormModel.php +++ b/libraries/src/MVC/Model/FormModel.php @@ -129,6 +129,13 @@ public function checkin($pk = null) */ public function checkout($pk = null) { + $user = $this->getCurrentUser(); + + // When the user is a guest, don't do a checkout + if (!$user->id) { + return true; + } + // Only attempt to check the row in if it exists. if ($pk) { // Get an instance of the row to checkout. @@ -150,7 +157,6 @@ public function checkout($pk = null) return true; } - $user = $this->getCurrentUser(); $checkedOutField = $table->getColumnAlias('checked_out'); // Check if this is the user having previously checked out the row. From 00bd7e50f9c8e14ecb685a6d1a162a1801153ae0 Mon Sep 17 00:00:00 2001 From: Allon Moritz Date: Wed, 21 Sep 2022 13:00:32 +0200 Subject: [PATCH 2/4] do not load early --- libraries/src/MVC/Model/FormModel.php | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-) diff --git a/libraries/src/MVC/Model/FormModel.php b/libraries/src/MVC/Model/FormModel.php index 736876888ffd1..56ea1cdbcf9b1 100644 --- a/libraries/src/MVC/Model/FormModel.php +++ b/libraries/src/MVC/Model/FormModel.php @@ -129,13 +129,6 @@ public function checkin($pk = null) */ public function checkout($pk = null) { - $user = $this->getCurrentUser(); - - // When the user is a guest, don't do a checkout - if (!$user->id) { - return true; - } - // Only attempt to check the row in if it exists. if ($pk) { // Get an instance of the row to checkout. @@ -157,6 +150,13 @@ public function checkout($pk = null) return true; } + $user = $this->getCurrentUser(); + + // When the user is a guest, don't do a checkout + if (!$user->id) { + return true; + } + $checkedOutField = $table->getColumnAlias('checked_out'); // Check if this is the user having previously checked out the row. From edf5592bd78d02c386beb6e3b9aefec87f106151 Mon Sep 17 00:00:00 2001 From: Allon Moritz Date: Wed, 21 Sep 2022 13:21:56 +0200 Subject: [PATCH 3/4] correct return --- libraries/src/MVC/Model/FormModel.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/libraries/src/MVC/Model/FormModel.php b/libraries/src/MVC/Model/FormModel.php index 56ea1cdbcf9b1..ffb80a6c3e13a 100644 --- a/libraries/src/MVC/Model/FormModel.php +++ b/libraries/src/MVC/Model/FormModel.php @@ -154,7 +154,7 @@ public function checkout($pk = null) // When the user is a guest, don't do a checkout if (!$user->id) { - return true; + return false; } $checkedOutField = $table->getColumnAlias('checked_out'); From 7c4fe6b3c52c2423583fa8ca96b5e19ce881649b Mon Sep 17 00:00:00 2001 From: Allon Moritz Date: Wed, 21 Sep 2022 13:41:11 +0200 Subject: [PATCH 4/4] add test --- .../Libraries/Cms/MVC/Model/FormModelTest.php | 49 +++++++++++++++++-- 1 file changed, 44 insertions(+), 5 deletions(-) diff --git a/tests/Unit/Libraries/Cms/MVC/Model/FormModelTest.php b/tests/Unit/Libraries/Cms/MVC/Model/FormModelTest.php index 6874c3ad4636d..ccc582c4a26b4 100644 --- a/tests/Unit/Libraries/Cms/MVC/Model/FormModelTest.php +++ b/tests/Unit/Libraries/Cms/MVC/Model/FormModelTest.php @@ -244,7 +244,7 @@ public function getForm($data = array(), $loadData = true) * * @since 4.2.0 */ - public function testSucessfullCheckout() + public function testSuccessfulCheckout() { $table = $this->createStub(Table::class); $table->checked_out = 0; @@ -263,7 +263,11 @@ public function getForm($data = array(), $loadData = true) return null; } }; - $model->setCurrentUser(new User()); + + // Must be a valid user + $user = new User(); + $user->id = 1; + $model->setCurrentUser($user); $this->assertTrue($model->checkout(1)); } @@ -275,7 +279,7 @@ public function getForm($data = array(), $loadData = true) * * @since 4.2.0 */ - public function testSucessfullCheckoutWithEmptyRecord() + public function testSuccessfulCheckoutWithEmptyRecord() { $model = new class (['dbo' => $this->createStub(DatabaseInterface::class)], $this->createStub(MVCFactoryInterface::class)) extends FormModel { @@ -307,6 +311,41 @@ public function testFailedCheckout() $mvcFactory = $this->createStub(MVCFactoryInterface::class); $mvcFactory->method('createTable')->willReturn($table); + $model = new class (['dbo' => $this->createStub(DatabaseInterface::class)], $mvcFactory) extends FormModel + { + public function getForm($data = array(), $loadData = true) + { + return null; + } + }; + + // Must be a valid user + $user = new User(); + $user->id = 1; + $model->setCurrentUser($user); + + $this->assertFalse($model->checkout(1)); + } + + /** + * @testdox can't checkout a record when the current user is a guest + * + * @return void + * + * @since 4.2.0 + */ + public function testFailedCheckoutAsGuest() + { + $table = $this->createStub(Table::class); + $table->checked_out = 0; + $table->method('load')->willReturn(true); + $table->method('hasField')->willReturn(true); + $table->method('checkIn')->willReturn(false); + $table->method('getColumnAlias')->willReturn('checked_out'); + + $mvcFactory = $this->createStub(MVCFactoryInterface::class); + $mvcFactory->method('createTable')->willReturn($table); + $model = new class (['dbo' => $this->createStub(DatabaseInterface::class)], $mvcFactory) extends FormModel { public function getForm($data = array(), $loadData = true) @@ -353,7 +392,7 @@ public function getForm($data = array(), $loadData = true) * * @since 4.2.0 */ - public function testSuccessfullCheckoutFieldNotAvailableCheck() + public function testSuccessfulCheckoutFieldNotAvailableCheck() { $table = $this->createStub(Table::class); $table->checked_out = 0; @@ -381,7 +420,7 @@ public function getForm($data = array(), $loadData = true) * * @since 4.2.0 */ - public function testSuccessfullCheckoutWhenCurrentUserIsDifferent() + public function testSuccessfulCheckoutWhenCurrentUserIsDifferent() { $table = $this->createStub(Table::class); $table->checked_out = 1;