diff --git a/.travis.yml b/.travis.yml index 637fe2650..1c8beab66 100644 --- a/.travis.yml +++ b/.travis.yml @@ -33,7 +33,7 @@ before_install: before_script: # install deps and UF - - composer install + - COMPOSER_MEMORY_LIMIT=-1 travis_retry composer install --no-interaction - php bakery debug - php bakery build-assets - php bakery migrate diff --git a/CHANGELOG.md b/CHANGELOG.md index 4c6854d9d..ab4774da2 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -5,6 +5,25 @@ All notable changes to this project will be documented in this file. The format is based on [Keep a Changelog](http://keepachangelog.com/en/1.0.0/) and this project adheres to [Semantic Versioning](http://semver.org/spec/v2.0.0.html). +## [v4.2.1] + +### Added +- `UserFrosting\Sprinkle\Core\Database\Models\Session` model for the `sessions` db table. +- `TEST_SESSION_HANDLER` environment variable to set the session save handler to use for Testing. +- `withDatabaseSessionHandler` Trait for testing. Use `$this->useDatabaseSessionHandler()` to use database session handler in tests. + +### Fixed +- Italian translation ([#950]) +- User Registration failing when trying to register two accounts with the same email address ([#953]) +- Bad test case for `CoreController::getAsset`. +- User Model `forceDelete` doesn't remove the record from the DB ([#951]) +- Fix PHP Fatal error that can be thrown when registering a new User +- Session not working with database handler ([#952]) +- Remove any persistences when forceDeleting user to prevent Foreign Key Constraints issue ([#963]) +- More helpful error message in checkEnvironment.php (Thanks @amosfolz; [#958]) +- Hide locale select from UI if only one locale is available (Thanks @avsdev-cw; [#968]) +- Download CSV filename error ([#893]) + ## [v4.2.0] ### Changed Requirements - Changed minimum Node.js version to **v10.12.0** @@ -702,7 +721,16 @@ See [http://learn.userfrosting.com/upgrading/40-to-41](Upgrading 4.0.x to 4.1.x [#869]: https://github.com/userfrosting/UserFrosting/issues/869 [#872]: https://github.com/userfrosting/UserFrosting/issues/872 [#888]: https://github.com/userfrosting/UserFrosting/issues/888 +[#893]: https://github.com/userfrosting/UserFrosting/issues/893 [#919]: https://github.com/userfrosting/UserFrosting/issues/919 [#940]: https://github.com/userfrosting/UserFrosting/issues/940 +[#950]: https://github.com/userfrosting/UserFrosting/issues/950 +[#951]: https://github.com/userfrosting/UserFrosting/issues/951 +[#952]: https://github.com/userfrosting/UserFrosting/issues/952 +[#953]: https://github.com/userfrosting/UserFrosting/issues/953 +[#958]: https://github.com/userfrosting/UserFrosting/issues/958 +[#963]: https://github.com/userfrosting/UserFrosting/issues/963 +[#968]: https://github.com/userfrosting/UserFrosting/issues/968 [v4.2.0]: https://github.com/userfrosting/UserFrosting/compare/v4.1.22...v4.2.0 +[v4.2.1]: https://github.com/userfrosting/UserFrosting/compare/v4.2.0...v4.2.1 diff --git a/app/defines.php b/app/defines.php index 28fb5ba03..522296035 100755 --- a/app/defines.php +++ b/app/defines.php @@ -10,7 +10,7 @@ namespace UserFrosting; // Some standard defines -define('UserFrosting\VERSION', '4.2.0'); +define('UserFrosting\VERSION', '4.2.1'); define('UserFrosting\DS', '/'); define('UserFrosting\PHP_MIN_VERSION', '5.6'); define('UserFrosting\PHP_RECOMMENDED_VERSION', '7.1'); diff --git a/app/sprinkles/account/locale/it_IT/messages.php b/app/sprinkles/account/locale/it_IT/messages.php index 780e8ad82..55c7b8c6c 100644 --- a/app/sprinkles/account/locale/it_IT/messages.php +++ b/app/sprinkles/account/locale/it_IT/messages.php @@ -182,5 +182,5 @@ 'USER_OR_EMAIL_INVALID' => "L'indirizzo mail o il nome utente non sono validi", 'USER_OR_PASS_INVALID' => 'Il nome utente o la password non sono validi', - 'WELCOME' => 'Bentornato, {{display_name}}' + 'WELCOME' => 'Bentornato, {{first_name}}' ]; diff --git a/app/sprinkles/account/src/Account/Registration.php b/app/sprinkles/account/src/Account/Registration.php index 10cf01102..e49af5ac1 100644 --- a/app/sprinkles/account/src/Account/Registration.php +++ b/app/sprinkles/account/src/Account/Registration.php @@ -161,15 +161,15 @@ public function validate() // Check if username is unique if (!$this->usernameIsUnique($this->userdata['user_name'])) { - $e = new HttpException(); - $e->addUserMessage('USERNAME.IN_USE'); + $e = new HttpException('Username is already in use.'); + $e->addUserMessage('USERNAME.IN_USE', ['user_name' => $this->userdata['user_name']]); throw $e; } // Check if email is unique if (!$this->emailIsUnique($this->userdata['email'])) { - $e = new HttpException(); - $e->addUserMessage('EMAIL.IN_USE'); + $e = new HttpException('Email is already in use.'); + $e->addUserMessage('EMAIL.IN_USE', ['email' => $this->userdata['email']]); throw $e; } diff --git a/app/sprinkles/account/src/Controller/AccountController.php b/app/sprinkles/account/src/Controller/AccountController.php index 46d890896..e38b383cd 100644 --- a/app/sprinkles/account/src/Controller/AccountController.php +++ b/app/sprinkles/account/src/Controller/AccountController.php @@ -509,12 +509,22 @@ public function pageRegister(Request $request, Response $response, $args) // Get locale information $currentLocales = $localePathBuilder->getLocales(); + // Hide the locale field if there is only 1 locale available + $fields = [ + 'hidden' => [], + 'disabled' => [] + ]; + if (count($config->getDefined('site.locales.available')) <= 1) { + $fields['hidden'][] = 'locale'; + } + return $this->ci->view->render($response, 'pages/register.html.twig', [ 'page' => [ 'validators' => [ 'register' => $validatorRegister->rules('json', false) ] ], + 'fields' => $fields, 'locales' => [ 'available' => $config['site.locales.available'], 'current' => end($currentLocales) @@ -658,8 +668,18 @@ public function pageSettings(Request $request, Response $response, $args) // Get a list of all locales $locales = $config->getDefined('site.locales.available'); + // Hide the locale field if there is only 1 locale available + $fields = [ + 'hidden' => [], + 'disabled' => [] + ]; + if (count($config->getDefined('site.locales.available')) <= 1) { + $fields['hidden'][] = 'locale'; + } + return $this->ci->view->render($response, 'pages/account-settings.html.twig', [ 'locales' => $locales, + 'fields' => $fields, 'page' => [ 'validators' => [ 'account_settings' => $validatorAccountSettings->rules('json', false), @@ -766,6 +786,11 @@ public function profile(Request $request, Response $response, $args) $error = false; + // Ensure that in the case of using a single locale, that the locale is set + if (count($config->getDefined('site.locales.available')) <= 1) { + $data['locale'] = $currentUser->locale; + } + // Validate, and halt on validation errors. $validator = new ServerSideValidator($schema, $this->ci->translator); if (!$validator->validate($data)) { @@ -877,6 +902,11 @@ public function register(Request $request, Response $response, $args) $error = false; + // Ensure that in the case of using a single locale, that the locale is set + if (count($config->getDefined('site.locales.available')) <= 1) { + $data['locale'] = $config['site.registration.user_defaults.locale']; + } + // Validate request data $validator = new ServerSideValidator($schema, $this->ci->translator); if (!$validator->validate($data)) { @@ -913,12 +943,9 @@ public function register(Request $request, Response $response, $args) // Now that we check the form, we can register the actual user $registration = new Registration($this->ci, $data); - try { - $user = $registration->register(); - } catch (\Exception $e) { - $ms->addMessageTranslated('danger', $e->getMessage(), $data); - $error = true; - } + // Try registration. An HttpException will be thrown if it fails + // No need to catch, as this kind of exception will automatically returns the addMessageTranslated + $user = $registration->register(); // Success message if ($config['site.registration.require_email_verification']) { @@ -1163,6 +1190,11 @@ public function settings(Request $request, Response $response, $args) $error = false; + // Ensure that in the case of using a single locale, that the locale is set + if (count($config->getDefined('site.locales.available')) <= 1) { + $data['locale'] = $currentUser->locale; + } + // Validate, and halt on validation errors. $validator = new ServerSideValidator($schema, $this->ci->translator); if (!$validator->validate($data)) { diff --git a/app/sprinkles/account/src/Database/Models/User.php b/app/sprinkles/account/src/Database/Models/User.php index 3f3981eda..30d98c212 100644 --- a/app/sprinkles/account/src/Database/Models/User.php +++ b/app/sprinkles/account/src/Database/Models/User.php @@ -188,17 +188,18 @@ public function delete($hardDelete = false) // Remove all role associations $this->roles()->detach(); - // Remove all user activities - $classMapper->staticMethod('activity', 'where', 'user_id', $this->id)->delete(); + // Remove last activity association + $this->lastActivity()->dissociate(); + $this->save(); // Remove all user tokens - $classMapper->staticMethod('password_reset', 'where', 'user_id', $this->id)->delete(); + $this->activities()->delete(); + $this->passwordResets()->delete(); $classMapper->staticMethod('verification', 'where', 'user_id', $this->id)->delete(); - - // TODO: remove any persistences + $classMapper->staticMethod('persistence', 'where', 'user_id', $this->id)->delete(); // Delete the user - $result = parent::forceDelete(); + $result = $this->forceDelete(); } else { // Soft delete the user, leaving all associated records alone $result = parent::delete(); diff --git a/app/sprinkles/account/src/Log/UserActivityDatabaseHandler.php b/app/sprinkles/account/src/Log/UserActivityDatabaseHandler.php index 3833094b8..a8972c04a 100644 --- a/app/sprinkles/account/src/Log/UserActivityDatabaseHandler.php +++ b/app/sprinkles/account/src/Log/UserActivityDatabaseHandler.php @@ -28,7 +28,7 @@ protected function write(array $record) if (isset($record['extra']['user_id'])) { $user = $this->classMapper->staticMethod('user', 'find', $record['extra']['user_id']); - $user->last_activity_id = $log->id; + $user->lastActivity()->associate($log); $user->save(); } } diff --git a/app/sprinkles/account/src/Repository/PasswordResetRepository.php b/app/sprinkles/account/src/Repository/PasswordResetRepository.php index 389f8bb6c..275d540db 100644 --- a/app/sprinkles/account/src/Repository/PasswordResetRepository.php +++ b/app/sprinkles/account/src/Repository/PasswordResetRepository.php @@ -9,6 +9,7 @@ namespace UserFrosting\Sprinkle\Account\Repository; +use UserFrosting\Sprinkle\Account\Database\Models\Interfaces\UserInterface; use UserFrosting\Sprinkle\Account\Facades\Password; /** @@ -27,7 +28,7 @@ class PasswordResetRepository extends TokenRepository /** * {@inheritdoc} */ - protected function updateUser($user, $args) + protected function updateUser(UserInterface $user, $args) { $user->password = Password::hash($args['password']); // TODO: generate user activity? or do this in controller? diff --git a/app/sprinkles/account/src/Repository/VerificationRepository.php b/app/sprinkles/account/src/Repository/VerificationRepository.php index d421185f2..e6c73df35 100644 --- a/app/sprinkles/account/src/Repository/VerificationRepository.php +++ b/app/sprinkles/account/src/Repository/VerificationRepository.php @@ -9,6 +9,8 @@ namespace UserFrosting\Sprinkle\Account\Repository; +use UserFrosting\Sprinkle\Account\Database\Models\Interfaces\UserInterface; + /** * Token repository class for new account verifications. * @@ -25,7 +27,7 @@ class VerificationRepository extends TokenRepository /** * {@inheritdoc} */ - protected function updateUser($user, $args) + protected function updateUser(UserInterface $user, $args) { $user->flag_verified = 1; // TODO: generate user activity? or do this in controller? diff --git a/app/sprinkles/account/src/ServicesProvider/ServicesProvider.php b/app/sprinkles/account/src/ServicesProvider/ServicesProvider.php index 2f1692229..c1917bfb8 100644 --- a/app/sprinkles/account/src/ServicesProvider/ServicesProvider.php +++ b/app/sprinkles/account/src/ServicesProvider/ServicesProvider.php @@ -69,6 +69,7 @@ public function register(ContainerInterface $container) $classMapper->setClassMapping('activity', 'UserFrosting\Sprinkle\Account\Database\Models\Activity'); $classMapper->setClassMapping('password_reset', 'UserFrosting\Sprinkle\Account\Database\Models\PasswordReset'); $classMapper->setClassMapping('verification', 'UserFrosting\Sprinkle\Account\Database\Models\Verification'); + $classMapper->setClassMapping('persistence', 'UserFrosting\Sprinkle\Account\Database\Models\Persistence'); return $classMapper; }); diff --git a/app/sprinkles/account/templates/forms/settings-profile.html.twig b/app/sprinkles/account/templates/forms/settings-profile.html.twig index 0b0a788ab..109bc5576 100644 --- a/app/sprinkles/account/templates/forms/settings-profile.html.twig +++ b/app/sprinkles/account/templates/forms/settings-profile.html.twig @@ -20,6 +20,7 @@ + {% if 'locale' not in fields.hidden %}

{{translate("LOCALE.ACCOUNT")}}.

+ {% endif %} {% endblock %} + {% if 'locale' not in fields.hidden %}

{{translate("LOCALE.ACCOUNT")}}.

+ {% endif %} {% if site.registration.captcha %}
diff --git a/app/sprinkles/account/tests/Integration/AuthenticatorTest.php b/app/sprinkles/account/tests/Integration/AuthenticatorTest.php index 38ddde710..75bc42563 100644 --- a/app/sprinkles/account/tests/Integration/AuthenticatorTest.php +++ b/app/sprinkles/account/tests/Integration/AuthenticatorTest.php @@ -12,8 +12,10 @@ use UserFrosting\Sprinkle\Account\Authenticate\Authenticator; use UserFrosting\Sprinkle\Account\Facades\Password; use UserFrosting\Sprinkle\Account\Tests\withTestUser; +use UserFrosting\Sprinkle\Core\Database\Models\Session as SessionTable; use UserFrosting\Sprinkle\Core\Tests\TestDatabase; use UserFrosting\Sprinkle\Core\Tests\RefreshDatabase; +use UserFrosting\Sprinkle\Core\Tests\withDatabaseSessionHandler; use UserFrosting\Tests\TestCase; /** @@ -26,6 +28,7 @@ class AuthenticatorTest extends TestCase use TestDatabase; use RefreshDatabase; use withTestUser; + use withDatabaseSessionHandler; /** * Setup the test database. @@ -80,6 +83,54 @@ public function testLogin(Authenticator $authenticator) $this->assertNotSame($testUser->id, $this->ci->session[$key]); } + /** + * @depends testConstructor + * @param Authenticator $authenticator + */ + public function testLoginWithSessionDatabase(Authenticator $authenticator) + { + // Reset CI Session + $this->useDatabaseSessionHandler(); + + // Create a test user + $testUser = $this->createTestUser(); + + // Check the table + $this->assertSame(0, SessionTable::count()); + + // Test session to avoid false positive + $key = $this->ci->config['session.keys.current_user_id']; + $this->assertNull($this->ci->session[$key]); + $this->assertNotSame($testUser->id, $this->ci->session[$key]); + + // Login the test user + $authenticator->login($testUser, false); + + // Test session to see if user was logged in + $this->assertNotNull($this->ci->session[$key]); + $this->assertSame($testUser->id, $this->ci->session[$key]); + + // Close session to initiate write + session_write_close(); + + // Check the table again + $this->assertSame(1, SessionTable::count()); + + // Reopen session + $this->ci->session->start(); + + // Must logout to avoid test issue + $authenticator->logout(true); + + // We'll test the logout system works too while we're at it (and depend on it) + $key = $this->ci->config['session.keys.current_user_id']; + $this->assertNull($this->ci->session[$key]); + $this->assertNotSame($testUser->id, $this->ci->session[$key]); + + // Make sure table entry has been removed + $this->assertSame(0, SessionTable::count()); + } + /** * @depends testConstructor * @expectedException \UserFrosting\Sprinkle\Account\Authenticate\Exception\AccountInvalidException diff --git a/app/sprinkles/account/tests/Integration/Database/Models/UserModelTest.php b/app/sprinkles/account/tests/Integration/Database/Models/UserModelTest.php new file mode 100644 index 000000000..447be7c21 --- /dev/null +++ b/app/sprinkles/account/tests/Integration/Database/Models/UserModelTest.php @@ -0,0 +1,97 @@ +setupTestDatabase(); + $this->refreshDatabase(); + } + + /** + * Test user hard deletion with user relations. + * This is not a totaly acurate test, as each relations are added manually + * and new relations might not be added automatically to accuratly test + */ + public function testUserHardDeleteWithUserRelations() + { + $fm = $this->ci->factory; + + // Create a user & make sure it exist + $user = $this->createTestUser(); + $this->assertInstanceOf(User::class, User::withTrashed()->find($user->id)); + + //$user->activities - activities + $this->ci->userActivityLogger->info("test", [ + 'type' => 'group_create', + 'user_id' => $user->id + ]); + $this->assertSame(1, $user->activities()->count()); + + //$user->passwordResets - password_resets + $this->ci->repoPasswordReset->create($user, $this->ci->config['password_reset.timeouts.reset']); + $this->assertSame(1, $user->passwordResets()->count()); + + //{no relations} - persistences + $persistence = new Persistence([ + 'user_id' => $user->id, + 'token' => '', + 'persistent_token' => '', + 'expires_at' => null + ]); + $persistence->save(); + $this->assertSame(1, Persistence::where('user_id', $user->id)->count()); + + //$user->roles - role_users + $role = $fm->create('UserFrosting\Sprinkle\Account\Database\Models\Role'); + $user->roles()->attach($role->id); + $this->assertSame(1, $user->roles()->count()); + + //{no relations} - verification + $this->ci->repoVerification->create($user, $this->ci->config['verification.timeout']); + $this->assertSame(1, $this->ci->classMapper->staticMethod('verification', 'where', 'user_id', $user->id)->count()); + + // Force delete. Now user can't be found at all + $this->assertTrue($user->delete(true)); + $this->assertNull(User::withTrashed()->find($user->id)); + + // Assert deletions worked + $this->assertSame(0, $user->activities()->count()); + $this->assertSame(0, $user->passwordResets()->count()); + $this->assertSame(0, $user->roles()->count()); + $this->assertSame(0, Persistence::where('user_id', $user->id)->count()); + $this->assertSame(0, $this->ci->classMapper->staticMethod('verification', 'where', 'user_id', $user->id)->count()); + } +} diff --git a/app/sprinkles/account/tests/Unit/RegistrationTest.php b/app/sprinkles/account/tests/Unit/RegistrationTest.php index dead5af3f..05ad7d1cd 100644 --- a/app/sprinkles/account/tests/Unit/RegistrationTest.php +++ b/app/sprinkles/account/tests/Unit/RegistrationTest.php @@ -15,7 +15,7 @@ use UserFrosting\Sprinkle\Core\Tests\RefreshDatabase; use UserFrosting\Sprinkle\Account\Account\Registration; use UserFrosting\Sprinkle\Account\Database\Models\Interfaces\UserInterface; -use UserFrosting\Support\Exception\HttpException; +use UserFrosting\Sprinkle\Account\Database\Models\User; /** * RegistrationTest Class @@ -26,6 +26,17 @@ class RegistrationTest extends TestCase use TestDatabase; use RefreshDatabase; + /** + * @var array Test user data + */ + protected $fakeUserData = [ + 'user_name' => 'FooBar', + 'first_name' => 'Foo', + 'last_name' => 'Bar', + 'email' => 'Foo@Bar.com', + 'password' => 'FooBarFooBar123' + ]; + public function tearDown() { parent::tearDown(); @@ -45,7 +56,43 @@ public function setUp() } /** - * testNormalRegistration + * Test validation works + */ + public function testValidation() + { + $registration = new Registration($this->ci, [ + 'user_name' => 'OwlFancy', + 'first_name' => 'Owl', + 'last_name' => 'Fancy', + 'email' => 'owl@fancy.com', + 'password' => 'owlFancy1234' + ]); + + $validation = $registration->validate(); + $this->assertTrue($validation); + } + + /** + * Test the $requiredProperties property + * @depends testValidation + * @expectedException UserFrosting\Support\Exception\HttpException + * @expectedExceptionMessage Account can't be registrated as 'first_name' is required to create a new user. + */ + public function testMissingFields() + { + $registration = new Registration($this->ci, [ + 'user_name' => 'OwlFancy', + //'first_name' => 'Owl', + 'last_name' => 'Fancy', + 'email' => 'owl@fancy.com', + 'password' => 'owlFancy1234' + ]); + + $validation = $registration->validate(); + } + + /** + * @depends testValidation */ public function testNormalRegistration() { @@ -56,77 +103,53 @@ public function testNormalRegistration() // Tests can't mail properly $this->ci->config['site.registration.require_email_verification'] = false; - // Genereate user data - $fakeUserData = [ - 'user_name' => 'FooBar', - 'first_name' => 'Foo', - 'last_name' => 'Bar', - 'email' => 'Foo@Bar.com', - 'password' => 'FooBarFooBar123' - ]; - // Get class - $registration = new Registration($this->ci, $fakeUserData); + $registration = new Registration($this->ci, $this->fakeUserData); $this->assertInstanceOf(Registration::class, $registration); // Register user $user = $registration->register(); - // Registration should return a valid user + // Registration should return a valid user, with a new ID $this->assertInstanceOf(UserInterface::class, $user); $this->assertEquals('FooBar', $user->user_name); + $this->assertInternalType('int', $user->id); - // We try to register the same user again. Should throw an error - $registration = new Registration($this->ci, $fakeUserData); - $this->expectException(HttpException::class); - $validation = $registration->validate(); - - // Should throw email error if we change the username - $fakeUserData['user_name'] = 'BarFoo'; - $registration = new Registration($this->ci, $fakeUserData); - $this->expectException(HttpException::class); - $validation = $registration->validate(); + // Make sure the user is added to the db by querying it + $users = User::where('email', 'Foo@Bar.com')->get(); + $this->assertCount(1, $users); + $this->assertSame('FooBar', $users->first()['user_name']); } /** - * Test validation works + * @depends testNormalRegistration + * @expectedException UserFrosting\Support\Exception\HttpException + * @expectedExceptionMessage Username is already in use. */ - public function testValidation() + public function testValidationWithDuplicateUsername() { - // Reset database - $this->refreshDatabase(); - - $registration = new Registration($this->ci, [ - 'user_name' => 'FooBar', - 'first_name' => 'Foo', - 'last_name' => 'Bar', - 'email' => 'Foo@Bar.com', - 'password' => 'FooBarFooBar123' - ]); + // Create the first user to test against + $this->testNormalRegistration(); - // Validate user. Shouldn't tell us the username is already in use since we reset the database + // We try to register the same user again. Should throw an error + $registration = new Registration($this->ci, $this->fakeUserData); $validation = $registration->validate(); - $this->assertTrue($validation); } /** - * Test the $requiredProperties property + * @depends testNormalRegistration + * @expectedException UserFrosting\Support\Exception\HttpException + * @expectedExceptionMessage Email is already in use. */ - public function testMissingFields() + public function testValidationWithDuplicateEmail() { - // Reset database - $this->refreshDatabase(); + // Create the first user to test against + $this->testNormalRegistration(); - $registration = new Registration($this->ci, [ - 'user_name' => 'FooBar', - //'first_name' => 'Foo', - 'last_name' => 'Bar', - 'email' => 'Foo@Bar.com', - 'password' => 'FooBarFooBar123' - ]); - - // Validate user. Shouldn't tell us the username is already in use since we reset the database - $this->expectException(HttpException::class); + // Should throw email error if we change the username + $fakeUserData = $this->fakeUserData; + $fakeUserData['user_name'] = 'BarFoo'; + $registration = new Registration($this->ci, $fakeUserData); $validation = $registration->validate(); } } diff --git a/app/sprinkles/account/tests/Unit/UserModelTest.php b/app/sprinkles/account/tests/Unit/UserModelTest.php new file mode 100644 index 000000000..8f243ed13 --- /dev/null +++ b/app/sprinkles/account/tests/Unit/UserModelTest.php @@ -0,0 +1,68 @@ +setupTestDatabase(); + $this->refreshDatabase(); + } + + /** + * Test user soft deletion. + */ + public function testUserSoftDelete() + { + // Create a user & make sure it exist + $user = $this->createTestUser(); + $this->assertInstanceOf(User::class, User::withTrashed()->find($user->id)); + + // Soft Delete. User won't be found using normal query, but will withTrash + $this->assertTrue($user->delete()); + $this->assertNull(User::find($user->id)); + $this->assertInstanceOf(User::class, User::withTrashed()->find($user->id)); + } + + /** + * Test user hard deletion. + */ + public function testUserHardDelete() + { + // Create a user & make sure it exist + $user = $this->createTestUser(); + $this->assertInstanceOf(User::class, User::withTrashed()->find($user->id)); + + // Force delete. Now user can't be found at all + $this->assertTrue($user->delete(true)); + $this->assertNull(User::withTrashed()->find($user->id)); + } +} diff --git a/app/sprinkles/admin/src/Controller/UserController.php b/app/sprinkles/admin/src/Controller/UserController.php index d6293ec49..0ea0e2c1d 100644 --- a/app/sprinkles/admin/src/Controller/UserController.php +++ b/app/sprinkles/admin/src/Controller/UserController.php @@ -60,6 +60,9 @@ public function create(Request $request, Response $response, $args) /** @var \UserFrosting\Sprinkle\Account\Database\Models\Interfaces\UserInterface $currentUser */ $currentUser = $this->ci->currentUser; + /** @var \UserFrosting\Support\Repository\Repository $config */ + $config = $this->ci->config; + // Access-controlled page if (!$authorizer->checkAccess($currentUser, 'create_user')) { throw new ForbiddenException(); @@ -77,6 +80,11 @@ public function create(Request $request, Response $response, $args) $error = false; + // Ensure that in the case of using a single locale, that the locale is set bu inheriting from current user + if (count($config->getDefined('site.locales.available')) <= 1) { + $data['locale'] = $currentUser->locale; + } + // Validate request data $validator = new ServerSideValidator($schema, $this->ci->translator); if (!$validator->validate($data)) { @@ -573,6 +581,11 @@ public function getModalCreate(Request $request, Response $response, $args) $fields['disabled'][] = 'group'; } + // Hide the locale field if there is only 1 locale available + if (count($config->getDefined('site.locales.available')) <= 1) { + $fields['hidden'][] = 'locale'; + } + // Create a dummy user to prepopulate fields $data = [ 'group_id' => $currentUser->group_id, @@ -673,6 +686,11 @@ public function getModalEdit(Request $request, Response $response, $args) $fields['disabled'][] = 'group'; } + // Hide the locale field if there is only 1 locale available + if (count($config->getDefined('site.locales.available')) <= 1) { + $fields['hidden'][] = 'locale'; + } + // Load validation rules $schema = new RequestSchema('schema://requests/user/edit-info.yaml'); $validator = new JqueryValidationAdapter($schema, $this->ci->translator); @@ -950,6 +968,11 @@ public function pageInfo(Request $request, Response $response, $args) } } + // Hide the locale field if there is only 1 locale available + if (count($config->getDefined('site.locales.available')) <= 1) { + $fields['hidden'][] = 'locale'; + } + // Determine buttons to display $editButtons = [ 'hidden' => [] diff --git a/app/sprinkles/core/composer.json b/app/sprinkles/core/composer.json index 6fc01c79f..7d5e82a18 100644 --- a/app/sprinkles/core/composer.json +++ b/app/sprinkles/core/composer.json @@ -15,7 +15,8 @@ }, { "name": "Jordan Mele", - "email": "SiliconSoldier@outlook.com.au" + "email": "SiliconSoldier@outlook.com.au", + "homepage": "https://blog.djmm.me" }, { "name": "Mike Jacobs" @@ -45,12 +46,12 @@ "slim/twig-view": "^1.2", "symfony/http-foundation": "*", "twig/twig": "^1.18", - "userfrosting/assets": "^5.0.0", + "userfrosting/assets": "^5.0.4", "userfrosting/config": "~4.2.0", "userfrosting/cache": "~4.2.0", "userfrosting/fortress": "~4.2.0", "userfrosting/i18n": "~4.2.0", - "userfrosting/session": "~4.2.0", + "userfrosting/session": "~4.2.2", "userfrosting/support": "~4.2.0", "vlucas/phpdotenv": "^2" }, diff --git a/app/sprinkles/core/config/testing.php b/app/sprinkles/core/config/testing.php index 36e1e24aa..980219548 100755 --- a/app/sprinkles/core/config/testing.php +++ b/app/sprinkles/core/config/testing.php @@ -60,7 +60,7 @@ * Disable native sessions in tests */ 'session' => [ - 'handler' => 'array' + 'handler' => getenv('TEST_SESSION_HANDLER') ?: 'array' ], /* * Database to use when using the TestDatabase Trait diff --git a/app/sprinkles/core/src/Database/Models/Session.php b/app/sprinkles/core/src/Database/Models/Session.php new file mode 100644 index 000000000..cf04d56ad --- /dev/null +++ b/app/sprinkles/core/src/Database/Models/Session.php @@ -0,0 +1,23 @@ +getQuery()->find($sessionId); + + if ($this->expired($session)) { + $this->exists = true; + + return ''; + } + + if (isset($session->payload)) { + $this->exists = true; + + return base64_decode($session->payload); + } + + return ''; + } +} diff --git a/app/sprinkles/core/src/Sprunje/Sprunje.php b/app/sprinkles/core/src/Sprunje/Sprunje.php index b09dc51b4..c41f70190 100644 --- a/app/sprinkles/core/src/Sprunje/Sprunje.php +++ b/app/sprinkles/core/src/Sprunje/Sprunje.php @@ -188,9 +188,7 @@ public function toResponse(Response $response) $result = $this->getCsv(); // Prepare response - $settings = http_build_query($this->options); - $date = Carbon::now()->format('Ymd'); - $response = $response->withAddedHeader('Content-Disposition', "attachment;filename=$date-{$this->name}-$settings.csv"); + $response = $response->withAddedHeader('Content-Disposition', "attachment;filename={$this->name}.csv"); $response = $response->withAddedHeader('Content-Type', 'text/csv; charset=utf-8'); return $response->write($result); diff --git a/app/sprinkles/core/src/Util/CheckEnvironment.php b/app/sprinkles/core/src/Util/CheckEnvironment.php index fce19bfc7..486977c66 100644 --- a/app/sprinkles/core/src/Util/CheckEnvironment.php +++ b/app/sprinkles/core/src/Util/CheckEnvironment.php @@ -125,6 +125,12 @@ public function checkAll() $problemsFound = true; } + if ($this->checkDirectories()) { + $problemsFound = true; + // Skip checkPermissions() if the required directories do not exist. + return $problemsFound; + } + if ($this->checkPermissions()) { $problemsFound = true; } @@ -257,6 +263,39 @@ public function checkPdo() return $problemsFound; } + /** + * Check that log, cache, and session directories exist. + */ + public function checkDirectories() + { + $problemsFound = false; + + $directoryPaths = [ + 'logs' => $this->locator->findResource('log://'), + 'cache' => $this->locator->findResource('cache://'), + 'sessions' => $this->locator->findResource('session://') + ]; + + foreach ($directoryPaths as $directory => $path) { + if ($path == null) { + $problemsFound = true; + $this->resultsFailed['directory-' . $directory] = [ + 'title' => " A required directory was not found.", + 'message' => "Please check that userfrosting/app/$directory exists.", + 'success' => false + ]; + } else { + $this->resultsSuccess['directory-' . $directory] = [ + 'title' => " File/directory check passed!", + 'message' => "userfrosting/app/$directory exists.", + 'success' => true + ]; + } + } + + return $problemsFound; + } + /** * Check that log, cache, and session directories are writable, and that other directories are set appropriately for the environment. */ @@ -280,19 +319,10 @@ public function checkPermissions() // Check for essential files & perms foreach ($shouldBeWriteable as $file => $assertWriteable) { - $is_dir = false; - if (!file_exists($file)) { + $writeable = is_writable($file); + if ($assertWriteable !== $writeable) { $problemsFound = true; $this->resultsFailed['file-' . $file] = [ - 'title' => " File or directory does not exist.", - 'message' => "We could not find the file or directory $file.", - 'success' => false - ]; - } else { - $writeable = is_writable($file); - if ($assertWriteable !== $writeable) { - $problemsFound = true; - $this->resultsFailed['file-' . $file] = [ 'title' => " Incorrect permissions for file or directory.", 'message' => "$file is " . ($writeable ? 'writeable' : 'not writeable') @@ -303,15 +333,14 @@ public function checkPermissions() . ($assertWriteable ? 'has' : 'does not have') . ' write permissions for this directory.', 'success' => false ]; - } else { - $this->resultsSuccess['file-' . $file] = [ + } else { + $this->resultsSuccess['file-' . $file] = [ 'title' => " File/directory check passed!", 'message' => "$file exists and is correctly set as " . ($writeable ? 'writeable' : 'not writeable') . '.', 'success' => true ]; - } } } diff --git a/app/sprinkles/core/tests/Integration/Controllers/CoreControllerTest.php b/app/sprinkles/core/tests/Integration/Controllers/CoreControllerTest.php index f8dc0415c..4b5aa5947 100644 --- a/app/sprinkles/core/tests/Integration/Controllers/CoreControllerTest.php +++ b/app/sprinkles/core/tests/Integration/Controllers/CoreControllerTest.php @@ -10,6 +10,7 @@ namespace UserFrosting\Sprinkle\Core\Tests; use UserFrosting\Sprinkle\Core\Controller\CoreController; +use UserFrosting\Support\Exception\NotFoundException; /** * Tests CoreController @@ -51,25 +52,35 @@ public function testJsonAlerts(CoreController $controller) /** * @depends testControllerConstructor - * @expectedException \UserFrosting\Support\Exception\NotFoundException * @param CoreController $controller */ public function testGetAsset_ExceptionNoUrl(CoreController $controller) { + $this->expectException(NotFoundException::class); $controller->getAsset($this->getRequest(), $this->getResponse(), []); } /** * @depends testControllerConstructor - * @expectedException \UserFrosting\Support\Exception\NotFoundException * @param CoreController $controller */ public function testGetAsset_ExceptionBadUrl(CoreController $controller) { + $this->expectException(NotFoundException::class); $url = '/' . rand(0, 99999); $controller->getAsset($this->getRequest(), $this->getResponse(), ['url' => $url]); } + /** + * @depends testControllerConstructor + * @param CoreController $controller + */ + public function testGetAsset_ExceptionEmptyUrl(CoreController $controller) + { + $this->expectException(NotFoundException::class); + $controller->getAsset($this->getRequest(), $this->getResponse(), ['url' => '']); + } + /** * @depends testControllerConstructor * @depends testGetAsset_ExceptionNoUrl @@ -78,10 +89,6 @@ public function testGetAsset_ExceptionBadUrl(CoreController $controller) */ public function testGetAsset(CoreController $controller) { - $result = $controller->getAsset($this->getRequest(), $this->getResponse(), ['url' => '']); - $this->assertSame($result->getStatusCode(), 200); - $this->assertSame('', (string) $result->getBody()); - $result = $controller->getAsset($this->getRequest(), $this->getResponse(), ['url' => 'userfrosting/js/uf-alerts.js']); $this->assertSame($result->getStatusCode(), 200); $this->assertNotEmpty((string) $result->getBody()); diff --git a/app/sprinkles/core/tests/Integration/DatabaseMigratorServiceTest.php b/app/sprinkles/core/tests/Integration/DatabaseMigratorServiceTest.php index 494d12d90..e00b69d54 100644 --- a/app/sprinkles/core/tests/Integration/DatabaseMigratorServiceTest.php +++ b/app/sprinkles/core/tests/Integration/DatabaseMigratorServiceTest.php @@ -9,31 +9,18 @@ namespace UserFrosting\Sprinkle\Core\Tests\Integration; -use UserFrosting\Sprinkle\Core\Tests\TestDatabase; +use UserFrosting\Sprinkle\Core\Database\Migrator\Migrator; use UserFrosting\Tests\TestCase; /** - * Tests for the Migrator Service + * Tests for the Migrator Service * - * @author Louis Charette + * @author Louis Charette */ class DatabaseMigratorServiceTest extends TestCase { - use TestDatabase; - - /** - * {@inheritdoc} - */ - public function setUp() - { - parent::setUp(); - - // Setup test database - $this->setupTestDatabase(); - } - public function testMigratorService() { - $this->assertInstanceOf('UserFrosting\Sprinkle\Core\Database\Migrator\Migrator', $this->ci->migrator); + $this->assertInstanceOf(Migrator::class, $this->ci->migrator); } } diff --git a/app/sprinkles/core/tests/Integration/Session/SessionDatabaseHandlerTest.php b/app/sprinkles/core/tests/Integration/Session/SessionDatabaseHandlerTest.php new file mode 100644 index 000000000..885b34443 --- /dev/null +++ b/app/sprinkles/core/tests/Integration/Session/SessionDatabaseHandlerTest.php @@ -0,0 +1,164 @@ +setupTestDatabase(); + $this->refreshDatabase(); + } + + /** + * Test session table connection & existance + */ + public function testSessionTable() + { + $connection = $this->ci->db->connection(); + $config = $this->ci->config; + $table = $config['session.database.table']; + + // Check connexion is ok and returns what's expected from DatabaseSessionHandler + $this->assertInstanceOf(\Illuminate\Database\ConnectionInterface::class, $connection); + $this->assertInstanceOf(\Illuminate\Database\Query\Builder::class, $connection->table($table)); + + // Check table exist + $this->assertTrue($connection->getSchemaBuilder()->hasTable($table)); + } + + /** + * @depends testSessionTable + */ + public function testSessionWrite() + { + $config = $this->ci->config; + $connection = $this->ci->db->connection(); + + // Define random session ID + $session_id = 'test'.rand(1, 100000); + + // Make sure db is empty at first + $this->assertEquals(0, SessionTable::count()); + $this->assertNull(SessionTable::find($session_id)); + + // Get handler + $handler = new DatabaseSessionHandler($connection, $config['session.database.table'], $config['session.minutes']); + + // Write session + // https://github.com/laravel/framework/blob/5.4/src/Illuminate/Session/DatabaseSessionHandler.php#L132 + $this->assertTrue($handler->write($session_id, 'foo')); + + // Closing the handler does nothing anyway + // https://github.com/laravel/framework/blob/5.4/src/Illuminate/Session/DatabaseSessionHandler.php#L78 + $this->assertTrue($handler->close()); + + // Read session + // https://github.com/laravel/framework/blob/5.4/src/Illuminate/Session/DatabaseSessionHandler.php#L86-L101 + $this->assertSame('foo', $handler->read($session_id)); + + // Check manually that the file has been written + $this->assertNotEquals(0, SessionTable::count()); + $this->assertNotNull(SessionTable::find($session_id)); + $this->assertSame(base64_encode('foo'), SessionTable::find($session_id)->payload); + + // Destroy session + // https://github.com/laravel/framework/blob/5.4/src/Illuminate/Session/DatabaseSessionHandler.php#L256 + $this->assertTrue($handler->destroy($session_id)); + + // Check db to make sure it's gone + $this->assertEquals(0, SessionTable::count()); + $this->assertNull(SessionTable::find($session_id)); + } + + /** + * Simulate session service with database handler. + * We can't use the real service as it is created before we can even setup + * the in-memory database with the basic table we need + * + * @depends testSessionWrite + */ + public function testUsingSessionDouble() + { + $this->ci->session->destroy(); + + $config = $this->ci->config; + $connection = $this->ci->db->connection(); + $handler = new DatabaseSessionHandler($connection, $config['session.database.table'], $config['session.minutes']); + $session = new Session($handler, $config['session']); + + $this->assertInstanceOf(Session::class, $session); + $this->assertInstanceOf(DatabaseSessionHandler::class, $session->getHandler()); + $this->assertSame($handler, $session->getHandler()); + + $this->sessionTests($session); + } + + /** + * @depends testUsingSessionDouble + */ + public function testUsingSessionService() + { + // Reset CI Session + $this->useDatabaseSessionHandler(); + + // Make sure config is set + $this->sessionTests($this->ci->session); + } + + /** + * @param Session $session + */ + protected function sessionTests(Session $session) + { + // Make sure session service have correct instance + $this->assertInstanceOf(Session::class, $session); + $this->assertInstanceOf(DatabaseSessionHandler::class, $session->getHandler()); + + // Destroy previously defined session + $session->destroy(); + + // Start new one and validate status + $this->assertSame(PHP_SESSION_NONE, $session->status()); + $session->start(); + $this->assertSame(PHP_SESSION_ACTIVE, $session->status()); + + // Get id + $session_id = $session->getId(); + + // Set something to the session + $session->set('foo', 'bar'); + $this->assertEquals('bar', $session->get('foo')); + + // Close session to initiate write + session_write_close(); + + // Make sure db was filled with something + $this->assertNotEquals(0, SessionTable::count()); + $this->assertNotNull(SessionTable::find($session_id)); + } +} diff --git a/app/sprinkles/core/tests/Integration/Session/SessionFileHandlerTest.php b/app/sprinkles/core/tests/Integration/Session/SessionFileHandlerTest.php new file mode 100644 index 000000000..86a060a1c --- /dev/null +++ b/app/sprinkles/core/tests/Integration/Session/SessionFileHandlerTest.php @@ -0,0 +1,160 @@ +ci->locator->findResource('session://'); + + // Define session filename + $session_file = "$session_dir/$session_id"; + + // Delete existing file to prevent false positive + $fs->delete($session_file); + $this->assertFalse($fs->exists($session_file)); + + // Get handler + $handler = new FileSessionHandler($fs, $session_dir, 120); + + // Write session + // https://github.com/laravel/framework/blob/5.4/src/Illuminate/Session/FileSessionHandler.php#L83 + // write() ==> $this->files->put($this->path.'/'.$sessionId, $data, true); + $this->assertTrue($handler->write($session_id, 'foo')); + + // Closing the handler does nothing anyway + // https://github.com/laravel/framework/blob/5.4/src/Illuminate/Session/FileSessionHandler.php#L61 + // close() ==> return true; + $this->assertTrue($handler->close()); + + // Read session + // https://github.com/laravel/framework/blob/5.4/src/Illuminate/Session/FileSessionHandler.php#L71 + // read() ==> return $this->files->get($path, true); + $this->assertSame('foo', $handler->read($session_id)); + + // Check manually that the file has been written + $this->assertTrue($fs->exists($session_file)); + $this->assertSame('foo', $fs->get($session_file)); + + // Destroy session + // https://github.com/laravel/framework/blob/5.4/src/Illuminate/Session/FileSessionHandler.php#L93 + // destroy() ==> $this->files->delete($this->path.'/'.$sessionId); + $this->assertTrue($handler->destroy($session_id)); + + // Check filesystem to make sure it's gone + $this->assertFalse($fs->exists($session_file)); + } + + /** + * @depends testSessionWrite + */ + public function testUsingSessionDouble() + { + $this->ci->session->destroy(); + $this->sessionTests($this->getSession()); + } + + /** + * @depends testUsingSessionDouble + */ + public function testUsingSessionService() + { + // Force test to use database session handler + putenv('TEST_SESSION_HANDLER=file'); + + // Refresh app to use new setup + $this->ci->session->destroy(); + $this->refreshApplication(); + + // Check setting is ok + $this->assertSame('file', $this->ci->config['session.handler']); + + // Make sure config is set + $this->sessionTests($this->ci->session); + + // Unset the env when test is done to avoid conflict + putenv('TEST_SESSION_HANDLER'); + } + + /** + * Simulate session service with database handler. + * We can't use the real service as it is created before we can even setup + * the in-memory database with the basic table we need + * + * @return Session + */ + protected function getSession() + { + $config = $this->ci->config; + $fs = new Filesystem(); + $handler = new FileSessionHandler($fs, $this->ci->locator->findResource('session://'), 120); + $session = new Session($handler, $config['session']); + + $this->assertInstanceOf(Session::class, $session); + $this->assertInstanceOf(FileSessionHandler::class, $session->getHandler()); + $this->assertSame($handler, $session->getHandler()); + + return $session; + } + + /** + * @param Session $session + */ + protected function sessionTests(Session $session) + { + // Make sure session service have correct instance + $this->assertInstanceOf(Session::class, $session); + $this->assertInstanceOf(FileSessionHandler::class, $session->getHandler()); + + // Destroy previously defined session + $session->destroy(); + + // Start new one and validate status + $this->assertSame(PHP_SESSION_NONE, $session->status()); + $session->start(); + $this->assertSame(PHP_SESSION_ACTIVE, $session->status()); + + // Get id + $session_id = $session->getId(); + + // Set something to the session + $session->set('foo', 'bar'); + $this->assertEquals('bar', $session->get('foo')); + + // Close session to initiate write + session_write_close(); + + // Make sure file was filled with something + $session_dir = $this->ci->locator->findResource('session://'); + $session_file = "$session_dir/$session_id"; + + $fs = new Filesystem(); + $this->assertTrue($fs->exists($session_file)); + $this->assertSame('foo|s:3:"bar";', $fs->get($session_file)); + } +} diff --git a/app/sprinkles/core/tests/RefreshDatabase.php b/app/sprinkles/core/tests/RefreshDatabase.php index 6504e9cd9..450bf3ecf 100644 --- a/app/sprinkles/core/tests/RefreshDatabase.php +++ b/app/sprinkles/core/tests/RefreshDatabase.php @@ -37,7 +37,7 @@ public function refreshDatabase() * * @return bool */ - protected function usingInMemoryDatabase() + public function usingInMemoryDatabase() { $connection = $this->ci->db->getConnection(); diff --git a/app/sprinkles/core/tests/withDatabaseSessionHandler.php b/app/sprinkles/core/tests/withDatabaseSessionHandler.php new file mode 100644 index 000000000..f4cf04880 --- /dev/null +++ b/app/sprinkles/core/tests/withDatabaseSessionHandler.php @@ -0,0 +1,54 @@ +usingInMemoryDatabase()) { + $this->markTestSkipped("Can't run this test on memory database"); + } + + // Force test to use database session handler + putenv('TEST_SESSION_HANDLER=database'); + + // Unset the env when test is done to avoid conflict + $this->beforeApplicationDestroyedCallbacks[] = function() + { + putenv('TEST_SESSION_HANDLER'); + }; + + // Refresh app to use new setup + $this->ci->session->destroy(); + $this->refreshApplication(); + $this->setupTestDatabase(); //<-- N.B.: This is executed after the session is created on the default db... + $this->refreshDatabase(); + + // Make sure it worked + if (!($this->ci->session->getHandler() instanceof DatabaseSessionHandler)) { + $this->markTestSkipped('Session handler not an instance of DatabaseSessionHandler'); + } + } +} diff --git a/app/tests/TestCase.php b/app/tests/TestCase.php index 2ff5ca74b..d9d6c570b 100644 --- a/app/tests/TestCase.php +++ b/app/tests/TestCase.php @@ -109,6 +109,9 @@ protected function tearDown() call_user_func($callback); } + // Force destroy test sessions + $this->ci->session->destroy(); + $this->ci = null; }