From 12a9a45747f3ce3ff58464cd7ccb88f2c42438e8 Mon Sep 17 00:00:00 2001 From: benrubson <6764151+benrubson@users.noreply.github.com> Date: Sun, 9 Feb 2020 10:01:33 +0100 Subject: [PATCH 01/17] Log failed accesses --- app/Http/Controllers/Auth/LoginController.php | 18 ++++++++++++++++++ 1 file changed, 18 insertions(+) diff --git a/app/Http/Controllers/Auth/LoginController.php b/app/Http/Controllers/Auth/LoginController.php index ea584a3b6..75ade74e7 100644 --- a/app/Http/Controllers/Auth/LoginController.php +++ b/app/Http/Controllers/Auth/LoginController.php @@ -101,6 +101,9 @@ class LoginController extends Controller $this->hasTooManyLoginAttempts($request)) { $this->fireLockoutEvent($request); + // Also log some error message + $this->logFailedAccess($request); + return $this->sendLockoutResponse($request); } @@ -117,6 +120,9 @@ class LoginController extends Controller // user surpasses their maximum number of attempts they will get locked out. $this->incrementLoginAttempts($request); + // Also log some error message + $this->logFailedAccess($request); + return $this->sendFailedLoginResponse($request); } @@ -162,4 +168,16 @@ class LoginController extends Controller return redirect('/login'); } + /** + * Log failed accesses, matching the default fail2ban nginx/apache auth rules. + */ + protected function logFailedAccess(Request $request) + { + if (isset($_SERVER['SERVER_SOFTWARE']) && preg_match('/nginx/i', $_SERVER['SERVER_SOFTWARE'])) { + error_log('user "' . $request->get($this->username()) . '" was not found in "BookStack"', 4); + } else { + error_log('user "' . $request->get($this->username()) . '" authentication failure for "BookStack"', 4); + } + } + } From 58df3ad9566061186e62110e7c1e4a4140ed02c2 Mon Sep 17 00:00:00 2001 From: benrubson <6764151+benrubson@users.noreply.github.com> Date: Sun, 3 May 2020 16:20:02 +0200 Subject: [PATCH 02/17] Log failed accesses option --- .env.example.complete | 8 ++++++- app/Http/Controllers/Auth/LoginController.php | 21 ++++++++++++------- 2 files changed, 20 insertions(+), 9 deletions(-) diff --git a/.env.example.complete b/.env.example.complete index 04cd73b90..5b62b1a2a 100644 --- a/.env.example.complete +++ b/.env.example.complete @@ -266,4 +266,10 @@ API_DEFAULT_ITEM_COUNT=100 API_MAX_ITEM_COUNT=500 # The number of API requests that can be made per minute by a single user. -API_REQUESTS_PER_MIN=180 \ No newline at end of file +API_REQUESTS_PER_MIN=180 + +# Failed access +# message to log into webserver logs in case of failed access, for further processing by tools like Fail2Ban +# Apache users should use : user "%u" authentication failure for "BookStack" +# Nginx users should use : user "%u" was not found in "BookStack" +FAILED_ACCESS_MESSAGE='' diff --git a/app/Http/Controllers/Auth/LoginController.php b/app/Http/Controllers/Auth/LoginController.php index 75ade74e7..c000af49e 100644 --- a/app/Http/Controllers/Auth/LoginController.php +++ b/app/Http/Controllers/Auth/LoginController.php @@ -169,15 +169,20 @@ class LoginController extends Controller } /** - * Log failed accesses, matching the default fail2ban nginx/apache auth rules. - */ - protected function logFailedAccess(Request $request) + * Log failed accesses, for further processing by tools like Fail2Ban + * + * @param \Illuminate\Http\Request $request + * @return void + */ + protected function logFailedAccess($request) { - if (isset($_SERVER['SERVER_SOFTWARE']) && preg_match('/nginx/i', $_SERVER['SERVER_SOFTWARE'])) { - error_log('user "' . $request->get($this->username()) . '" was not found in "BookStack"', 4); - } else { - error_log('user "' . $request->get($this->username()) . '" authentication failure for "BookStack"', 4); - } + $log_msg = env('FAILED_ACCESS_MESSAGE', ''); + + if (!is_string($request->get($this->username())) || !is_string($log_msg) || strlen($log_msg)<1) + return; + + $log_msg = str_replace("%u", $request->get($this->username()), $log_msg); + error_log($log_msg, 4); } } From 8f1f73defa321f026d487b3e9344055746bc6f58 Mon Sep 17 00:00:00 2001 From: benrubson <6764151+benrubson@users.noreply.github.com> Date: Sat, 23 May 2020 12:06:37 +0200 Subject: [PATCH 03/17] Properly use env/config functions --- app/Config/logging.php | 5 +++++ app/Http/Controllers/Auth/LoginController.php | 2 +- 2 files changed, 6 insertions(+), 1 deletion(-) diff --git a/app/Config/logging.php b/app/Config/logging.php index 0b55dc24d..406b9f2f9 100644 --- a/app/Config/logging.php +++ b/app/Config/logging.php @@ -79,4 +79,9 @@ return [ ], ], + // Failed Access Message + // Defines the message to log into webserver logs in case of failed access, + // for further processing by tools like Fail2Ban. + 'failed_access_message' => env('FAILED_ACCESS_MESSAGE', ''), + ]; diff --git a/app/Http/Controllers/Auth/LoginController.php b/app/Http/Controllers/Auth/LoginController.php index c000af49e..cf9e44e43 100644 --- a/app/Http/Controllers/Auth/LoginController.php +++ b/app/Http/Controllers/Auth/LoginController.php @@ -176,7 +176,7 @@ class LoginController extends Controller */ protected function logFailedAccess($request) { - $log_msg = env('FAILED_ACCESS_MESSAGE', ''); + $log_msg = config('logging.failed_access_message'); if (!is_string($request->get($this->username())) || !is_string($log_msg) || strlen($log_msg)<1) return; From 9d7ce59b18c5cbeef017349f38f063a63b762188 Mon Sep 17 00:00:00 2001 From: benrubson <6764151+benrubson@users.noreply.github.com> Date: Sat, 23 May 2020 15:37:38 +0200 Subject: [PATCH 04/17] Move logFailedAccess into Activity --- app/Actions/ActivityService.php | 17 ++++++++++++++ app/Http/Controllers/Auth/LoginController.php | 22 +++---------------- 2 files changed, 20 insertions(+), 19 deletions(-) diff --git a/app/Actions/ActivityService.php b/app/Actions/ActivityService.php index f56f1ca57..ca09aaef1 100644 --- a/app/Actions/ActivityService.php +++ b/app/Actions/ActivityService.php @@ -183,4 +183,21 @@ class ActivityService session()->flash('success', $message); } } + + /** + * Log failed accesses, for further processing by tools like Fail2Ban + * + * @param username + * @return void + */ + public function logFailedAccess($username) + { + $log_msg = config('logging.failed_access_message'); + + if (!is_string($username) || !is_string($log_msg) || strlen($log_msg)<1) + return; + + $log_msg = str_replace("%u", $username, $log_msg); + error_log($log_msg, 4); + } } diff --git a/app/Http/Controllers/Auth/LoginController.php b/app/Http/Controllers/Auth/LoginController.php index cf9e44e43..f5479814a 100644 --- a/app/Http/Controllers/Auth/LoginController.php +++ b/app/Http/Controllers/Auth/LoginController.php @@ -2,6 +2,7 @@ namespace BookStack\Http\Controllers\Auth; +use Activity; use BookStack\Auth\Access\SocialAuthService; use BookStack\Exceptions\LoginAttemptEmailNeededException; use BookStack\Exceptions\LoginAttemptException; @@ -102,7 +103,7 @@ class LoginController extends Controller $this->fireLockoutEvent($request); // Also log some error message - $this->logFailedAccess($request); + Activity::logFailedAccess($request->get($this->username())); return $this->sendLockoutResponse($request); } @@ -121,7 +122,7 @@ class LoginController extends Controller $this->incrementLoginAttempts($request); // Also log some error message - $this->logFailedAccess($request); + Activity::logFailedAccess($request->get($this->username())); return $this->sendFailedLoginResponse($request); } @@ -168,21 +169,4 @@ class LoginController extends Controller return redirect('/login'); } - /** - * Log failed accesses, for further processing by tools like Fail2Ban - * - * @param \Illuminate\Http\Request $request - * @return void - */ - protected function logFailedAccess($request) - { - $log_msg = config('logging.failed_access_message'); - - if (!is_string($request->get($this->username())) || !is_string($log_msg) || strlen($log_msg)<1) - return; - - $log_msg = str_replace("%u", $request->get($this->username()), $log_msg); - error_log($log_msg, 4); - } - } From 2ed031712918313b50483d22cf6735aed227dc06 Mon Sep 17 00:00:00 2001 From: Dan Brown Date: Tue, 28 Jul 2020 12:59:43 +0100 Subject: [PATCH 05/17] Updated functionality for logging failed access - Added testing to cover. - Linked logging into Laravel's monolog logging system and made log channel configurable. - Updated env var names to be specific to login access. - Added extra locations as to where failed logins would be captured. Related to #1881 and #728 --- .env.example.complete | 12 ++++---- app/Actions/ActivityService.php | 30 +++++++++---------- app/Config/logging.php | 26 +++++++++++++--- app/Http/Controllers/Auth/LoginController.php | 10 +++---- phpunit.xml | 2 ++ tests/Auth/AuthTest.php | 12 ++++++++ tests/Auth/LdapTest.php | 13 ++++++++ tests/Unit/ConfigTest.php | 23 ++++++++++++++ 8 files changed, 98 insertions(+), 30 deletions(-) diff --git a/.env.example.complete b/.env.example.complete index 26dfdc54b..d7dd02b9a 100644 --- a/.env.example.complete +++ b/.env.example.complete @@ -272,8 +272,10 @@ API_MAX_ITEM_COUNT=500 # The number of API requests that can be made per minute by a single user. API_REQUESTS_PER_MIN=180 -# Failed access -# message to log into webserver logs in case of failed access, for further processing by tools like Fail2Ban -# Apache users should use : user "%u" authentication failure for "BookStack" -# Nginx users should use : user "%u" was not found in "BookStack" -FAILED_ACCESS_MESSAGE='' +# Enable the logging of failed email+password logins with the given message +# The defaul log channel below uses the php 'error_log' function which commonly +# results in messages being output to the webserver error logs. +# The message can contain a %u parameter which will be replaced with the login +# user identifier (Username or email). +LOG_FAILED_LOGIN_MESSAGE=false +LOG_FAILED_LOGIN_CHANNEL=errorlog_plain_webserver diff --git a/app/Actions/ActivityService.php b/app/Actions/ActivityService.php index 0e3ac7861..e6b004f01 100644 --- a/app/Actions/ActivityService.php +++ b/app/Actions/ActivityService.php @@ -4,6 +4,7 @@ use BookStack\Auth\Permissions\PermissionService; use BookStack\Auth\User; use BookStack\Entities\Entity; use Illuminate\Support\Collection; +use Illuminate\Support\Facades\Log; class ActivityService { @@ -49,7 +50,7 @@ class ActivityService protected function newActivityForUser(string $key, ?int $bookId = null): Activity { return $this->activity->newInstance()->forceFill([ - 'key' => strtolower($key), + 'key' => strtolower($key), 'user_id' => $this->user->id, 'book_id' => $bookId ?? 0, ]); @@ -64,8 +65,8 @@ class ActivityService { $activities = $entity->activity()->get(); $entity->activity()->update([ - 'extra' => $entity->name, - 'entity_id' => 0, + 'extra' => $entity->name, + 'entity_id' => 0, 'entity_type' => '', ]); return $activities; @@ -99,7 +100,7 @@ class ActivityService $query = $this->activity->newQuery()->where('entity_type', '=', $entity->getMorphClass()) ->where('entity_id', '=', $entity->id); } - + $activity = $this->permissionService ->filterRestrictedEntityRelations($query, 'activities', 'entity_id', 'entity_type') ->orderBy('created_at', 'desc') @@ -161,19 +162,18 @@ class ActivityService } /** - * Log failed accesses, for further processing by tools like Fail2Ban - * - * @param username - * @return void - */ - public function logFailedAccess($username) + * Log out a failed login attempt, Providing the given username + * as part of the message if the '%u' string is used. + */ + public function logFailedLogin(string $username) { - $log_msg = config('logging.failed_access_message'); - - if (!is_string($username) || !is_string($log_msg) || strlen($log_msg)<1) + $message = config('logging.failed_login.message'); + if (!$message) { return; + } - $log_msg = str_replace("%u", $username, $log_msg); - error_log($log_msg, 4); + $message = str_replace("%u", $username, $message); + $channel = config('logging.failed_login.channel'); + Log::channel($channel)->warning($message); } } diff --git a/app/Config/logging.php b/app/Config/logging.php index ba77ba81e..afd56e482 100644 --- a/app/Config/logging.php +++ b/app/Config/logging.php @@ -1,5 +1,7 @@ 'debug', ], + // Custom errorlog implementation that logs out a plain, + // non-formatted message intended for the webserver log. + 'errorlog_plain_webserver' => [ + 'driver' => 'monolog', + 'level' => 'debug', + 'handler' => ErrorLogHandler::class, + 'handler_with' => [4], + 'formatter' => LineFormatter::class, + 'formatter_with' => [ + 'format' => "%message%", + ], + ], + 'null' => [ 'driver' => 'monolog', 'handler' => NullHandler::class, @@ -86,9 +101,12 @@ return [ ], ], - // Failed Access Message - // Defines the message to log into webserver logs in case of failed access, - // for further processing by tools like Fail2Ban. - 'failed_access_message' => env('FAILED_ACCESS_MESSAGE', ''), + + // Failed Login Message + // Allows a configurable message to be logged when a login request fails. + 'failed_login' => [ + 'message' => env('LOG_FAILED_LOGIN_MESSAGE', null), + 'channel' => env('LOG_FAILED_LOGIN_CHANNEL', 'errorlog_plain_webserver'), + ], ]; diff --git a/app/Http/Controllers/Auth/LoginController.php b/app/Http/Controllers/Auth/LoginController.php index f031c12cf..cd7a4db32 100644 --- a/app/Http/Controllers/Auth/LoginController.php +++ b/app/Http/Controllers/Auth/LoginController.php @@ -99,6 +99,7 @@ class LoginController extends Controller public function login(Request $request) { $this->validateLogin($request); + $username = $request->get($this->username()); // If the class is using the ThrottlesLogins trait, we can automatically throttle // the login attempts for this application. We'll key this by the username and @@ -107,9 +108,7 @@ class LoginController extends Controller $this->hasTooManyLoginAttempts($request)) { $this->fireLockoutEvent($request); - // Also log some error message - Activity::logFailedAccess($request->get($this->username())); - + Activity::logFailedLogin($username); return $this->sendLockoutResponse($request); } @@ -118,6 +117,7 @@ class LoginController extends Controller return $this->sendLoginResponse($request); } } catch (LoginAttemptException $exception) { + Activity::logFailedLogin($username); return $this->sendLoginAttemptExceptionResponse($exception, $request); } @@ -126,9 +126,7 @@ class LoginController extends Controller // user surpasses their maximum number of attempts they will get locked out. $this->incrementLoginAttempts($request); - // Also log some error message - Activity::logFailedAccess($request->get($this->username())); - + Activity::logFailedLogin($username); return $this->sendFailedLoginResponse($request); } diff --git a/phpunit.xml b/phpunit.xml index 85538c446..70f1c1f9c 100644 --- a/phpunit.xml +++ b/phpunit.xml @@ -51,5 +51,7 @@ + + diff --git a/tests/Auth/AuthTest.php b/tests/Auth/AuthTest.php index f1f476966..8900eeeba 100644 --- a/tests/Auth/AuthTest.php +++ b/tests/Auth/AuthTest.php @@ -401,6 +401,18 @@ class AuthTest extends BrowserKitTest $this->assertFalse(auth('saml2')->check()); } + public function test_failed_logins_are_logged_when_message_configured() + { + $log = $this->withTestLogger(); + config()->set(['logging.failed_login.message' => 'Failed login for %u']); + + $this->post('/login', ['email' => 'admin@example.com', 'password' => 'cattreedog']); + $this->assertTrue($log->hasWarningThatContains('Failed login for admin@example.com')); + + $this->post('/login', ['email' => 'admin@admin.com', 'password' => 'password']); + $this->assertFalse($log->hasWarningThatContains('Failed login for admin@admin.com')); + } + /** * Perform a login */ diff --git a/tests/Auth/LdapTest.php b/tests/Auth/LdapTest.php index ed8748f08..0d92241bb 100644 --- a/tests/Auth/LdapTest.php +++ b/tests/Auth/LdapTest.php @@ -593,4 +593,17 @@ class LdapTest extends BrowserKitTest $this->see('A user with the email tester@example.com already exists but with different credentials'); } + + public function test_failed_logins_are_logged_when_message_configured() + { + $log = $this->withTestLogger(); + config()->set(['logging.failed_login.message' => 'Failed login for %u']); + + $this->commonLdapMocks(1, 1, 1, 1, 1); + $this->mockLdap->shouldReceive('searchAndGetEntries')->times(1) + ->andReturn(['count' => 0]); + + $this->post('/login', ['username' => 'timmyjenkins', 'password' => 'cattreedog']); + $this->assertTrue($log->hasWarningThatContains('Failed login for timmyjenkins')); + } } diff --git a/tests/Unit/ConfigTest.php b/tests/Unit/ConfigTest.php index 69b737d7d..1374b3aa9 100644 --- a/tests/Unit/ConfigTest.php +++ b/tests/Unit/ConfigTest.php @@ -1,5 +1,6 @@ checkEnvConfigResult('APP_URL', $oldDefault, 'app.url', ''); } + public function test_errorlog_plain_webserver_channel() + { + // We can't full test this due to it being targeted for the SAPI logging handler + // so we just overwrite that component so we can capture the error log output. + config()->set([ + 'logging.channels.errorlog_plain_webserver.handler_with' => [0], + ]); + + $temp = tempnam(sys_get_temp_dir(), 'bs-test'); + $original = ini_set( 'error_log', $temp); + + Log::channel('errorlog_plain_webserver')->info('Aww, look, a cute puppy'); + + ini_set( 'error_log', $original); + + $output = file_get_contents($temp); + $this->assertStringContainsString('Aww, look, a cute puppy', $output); + $this->assertStringNotContainsString('INFO', $output); + $this->assertStringNotContainsString('info', $output); + $this->assertStringNotContainsString('testing', $output); + } + /** * Set an environment variable of the given name and value * then check the given config key to see if it matches the given result. From 2c0fdf83c129f3a89fb3d1d8720b6af547188af4 Mon Sep 17 00:00:00 2001 From: Dan Brown Date: Tue, 28 Jul 2020 16:27:16 +0100 Subject: [PATCH 06/17] Updated public-login redirect to check url Direct links to the login pages for public instances could lead to a redirect back to an external page upon login. This adds a check to ensure the URL is a URL expected from the current bookstack instance, or at least under the same domain. Fixes #2073 --- app/Http/Controllers/Auth/LoginController.php | 8 ++++++-- tests/Auth/AuthTest.php | 11 +++++++++++ 2 files changed, 17 insertions(+), 2 deletions(-) diff --git a/app/Http/Controllers/Auth/LoginController.php b/app/Http/Controllers/Auth/LoginController.php index cd7a4db32..8084ce1a5 100644 --- a/app/Http/Controllers/Auth/LoginController.php +++ b/app/Http/Controllers/Auth/LoginController.php @@ -77,9 +77,13 @@ class LoginController extends Controller ]); } + // Store the previous location for redirect after login $previous = url()->previous(''); - if (setting('app-public') && $previous && $previous !== url('/login')) { - redirect()->setIntendedUrl($previous); + if ($previous && $previous !== url('/login') && setting('app-public')) { + $isPreviousFromInstance = (strpos($previous, url('/')) === 0); + if ($isPreviousFromInstance) { + redirect()->setIntendedUrl($previous); + } } return view('auth.login', [ diff --git a/tests/Auth/AuthTest.php b/tests/Auth/AuthTest.php index 8900eeeba..6257f841f 100644 --- a/tests/Auth/AuthTest.php +++ b/tests/Auth/AuthTest.php @@ -381,6 +381,17 @@ class AuthTest extends BrowserKitTest ->seePageUrlIs($page->getUrl()); } + public function test_login_intended_redirect_does_not_redirect_to_external_pages() + { + config()->set('app.url', 'http://localhost'); + $this->setSettings(['app-public' => true]); + + $this->get('/login', ['referer' => 'https://example.com']); + $login = $this->post('/login', ['email' => 'admin@admin.com', 'password' => 'password']); + + $login->assertRedirectedTo('http://localhost'); + } + public function test_login_authenticates_admins_on_all_guards() { $this->post('/login', ['email' => 'admin@admin.com', 'password' => 'password']); From 7590ecd37c49b12fb6cf2ad251b1e65c1ea7d1ee Mon Sep 17 00:00:00 2001 From: Dan Brown Date: Tue, 28 Jul 2020 18:19:18 +0100 Subject: [PATCH 07/17] Updated some comment elements and standardised more JS - Updated comment routes to be simpler. - Updated comments JS to align better with updated component system. - Documented available global JS functions/services. - Removed redundant controller method. - Added window.$events helpers for validation messages and success/error. - Updated JS events system to not be class based for simplicity. - Added window.trans_plural method to handle pluralisation/replacements where you already have the translation string itself. Fixes #1836 --- app/Http/Controllers/Controller.php | 18 +--- dev/docs/components.md | 37 +++++++ resources/js/components/page-comments.js | 70 +++++++----- resources/js/index.js | 6 +- resources/js/services/events.js | 111 +++++++++++--------- resources/js/services/http.js | 5 +- resources/js/services/translations.js | 14 ++- resources/views/comments/comments.blade.php | 24 ++--- resources/views/comments/create.blade.php | 9 +- routes/web.php | 6 +- tests/Entity/CommentTest.php | 16 +-- 11 files changed, 192 insertions(+), 124 deletions(-) diff --git a/app/Http/Controllers/Controller.php b/app/Http/Controllers/Controller.php index 2e8e8ed2e..6a1dfcb01 100644 --- a/app/Http/Controllers/Controller.php +++ b/app/Http/Controllers/Controller.php @@ -8,6 +8,7 @@ use Illuminate\Foundation\Validation\ValidatesRequests; use Illuminate\Http\Exceptions\HttpResponseException; use Illuminate\Http\Request; use Illuminate\Routing\Controller as BaseController; +use Illuminate\Validation\ValidationException; abstract class Controller extends BaseController { @@ -132,23 +133,6 @@ abstract class Controller extends BaseController return response()->json(['message' => $messageText, 'status' => 'error'], $statusCode); } - /** - * Create the response for when a request fails validation. - * @param \Illuminate\Http\Request $request - * @param array $errors - * @return \Symfony\Component\HttpFoundation\Response - */ - protected function buildFailedValidationResponse(Request $request, array $errors) - { - if ($request->expectsJson()) { - return response()->json(['validation' => $errors], 422); - } - - return redirect()->to($this->getRedirectUrl()) - ->withInput($request->input()) - ->withErrors($errors, $this->errorBag()); - } - /** * Create a response that forces a download in the browser. * @param string $content diff --git a/dev/docs/components.md b/dev/docs/components.md index ac0e929cd..832765dd6 100644 --- a/dev/docs/components.md +++ b/dev/docs/components.md @@ -59,4 +59,41 @@ Will result with `this.$opts` being: "delay": "500", "show": "" } +``` + +#### Global Helpers + +There are various global helper libraries which can be used in components: + +```js +// HTTP service +window.$http.get(url, params); +window.$http.post(url, data); +window.$http.put(url, data); +window.$http.delete(url, data); +window.$http.patch(url, data); + +// Global event system +// Emit a global event +window.$events.emit(eventName, eventData); +// Listen to a global event +window.$events.listen(eventName, callback); +// Show a success message +window.$events.success(message); +// Show an error message +window.$events.error(message); +// Show validation errors, if existing, as an error notification +window.$events.showValidationErrors(error); + +// Translator +// Take the given plural text and count to decide on what plural option +// to use, Similar to laravel's trans_choice function but instead +// takes the direction directly instead of a translation key. +window.trans_plural(translationString, count, replacements); + +// Component System +// Parse and initialise any components from the given root el down. +window.components.init(rootEl); +// Get the first active component of the given name +window.components.first(name); ``` \ No newline at end of file diff --git a/resources/js/components/page-comments.js b/resources/js/components/page-comments.js index 5d826cba1..c86eead1b 100644 --- a/resources/js/components/page-comments.js +++ b/resources/js/components/page-comments.js @@ -1,16 +1,31 @@ import {scrollAndHighlightElement} from "../services/util"; +/** + * @extends {Component} + */ class PageComments { - constructor(elem) { - this.elem = elem; - this.pageId = Number(elem.getAttribute('page-id')); + setup() { + this.elem = this.$el; + this.pageId = Number(this.$opts.pageId); + + // Element references + this.container = this.$refs.commentContainer; + this.formContainer = this.$refs.formContainer; + this.commentCountBar = this.$refs.commentCountBar; + this.addButtonContainer = this.$refs.addButtonContainer; + this.replyToRow = this.$refs.replyToRow; + + // Translations + this.updatedText = this.$opts.updatedText; + this.deletedText = this.$opts.deletedText; + this.createdText = this.$opts.createdText; + this.countText = this.$opts.countText; + + // Internal State this.editingComment = null; this.parentId = null; - this.container = elem.querySelector('[comment-container]'); - this.formContainer = elem.querySelector('[comment-form-container]'); - if (this.formContainer) { this.form = this.formContainer.querySelector('form'); this.formInput = this.form.querySelector('textarea'); @@ -32,13 +47,14 @@ class PageComments { if (actionElem === null) return; event.preventDefault(); - let action = actionElem.getAttribute('action'); - if (action === 'edit') this.editComment(actionElem.closest('[comment]')); + const action = actionElem.getAttribute('action'); + const comment = actionElem.closest('[comment]'); + if (action === 'edit') this.editComment(comment); if (action === 'closeUpdateForm') this.closeUpdateForm(); - if (action === 'delete') this.deleteComment(actionElem.closest('[comment]')); + if (action === 'delete') this.deleteComment(comment); if (action === 'addComment') this.showForm(); if (action === 'hideForm') this.hideForm(); - if (action === 'reply') this.setReply(actionElem.closest('[comment]')); + if (action === 'reply') this.setReply(comment); if (action === 'remove-reply-to') this.removeReplyTo(); } @@ -69,14 +85,15 @@ class PageComments { }; this.showLoading(form); let commentId = this.editingComment.getAttribute('comment'); - window.$http.put(`/ajax/comment/${commentId}`, reqData).then(resp => { + window.$http.put(`/comment/${commentId}`, reqData).then(resp => { let newComment = document.createElement('div'); newComment.innerHTML = resp.data; this.editingComment.innerHTML = newComment.children[0].innerHTML; - window.$events.emit('success', window.trans('entities.comment_updated_success')); + window.$events.success(this.updatedText); window.components.init(this.editingComment); this.closeUpdateForm(); this.editingComment = null; + }).catch(window.$events.showValidationErrors).then(() => { this.hideLoading(form); }); } @@ -84,9 +101,9 @@ class PageComments { deleteComment(commentElem) { let id = commentElem.getAttribute('comment'); this.showLoading(commentElem.querySelector('[comment-content]')); - window.$http.delete(`/ajax/comment/${id}`).then(resp => { + window.$http.delete(`/comment/${id}`).then(resp => { commentElem.parentNode.removeChild(commentElem); - window.$events.emit('success', window.trans('entities.comment_deleted_success')); + window.$events.success(this.deletedText); this.updateCount(); this.hideForm(); }); @@ -101,21 +118,24 @@ class PageComments { parent_id: this.parentId || null, }; this.showLoading(this.form); - window.$http.post(`/ajax/page/${this.pageId}/comment`, reqData).then(resp => { + window.$http.post(`/comment/${this.pageId}`, reqData).then(resp => { let newComment = document.createElement('div'); newComment.innerHTML = resp.data; let newElem = newComment.children[0]; this.container.appendChild(newElem); window.components.init(newElem); - window.$events.emit('success', window.trans('entities.comment_created_success')); + window.$events.success(this.createdText); this.resetForm(); this.updateCount(); + }).catch(err => { + window.$events.showValidationErrors(err); + this.hideLoading(this.form); }); } updateCount() { let count = this.container.children.length; - this.elem.querySelector('[comments-title]').textContent = window.trans_choice('entities.comment_count', count, {count}); + this.elem.querySelector('[comments-title]').textContent = window.trans_plural(this.countText, count, {count}); } resetForm() { @@ -129,7 +149,7 @@ class PageComments { showForm() { this.formContainer.style.display = 'block'; this.formContainer.parentNode.style.display = 'block'; - this.elem.querySelector('[comment-add-button-container]').style.display = 'none'; + this.addButtonContainer.style.display = 'none'; this.formInput.focus(); this.formInput.scrollIntoView({behavior: "smooth"}); } @@ -137,14 +157,12 @@ class PageComments { hideForm() { this.formContainer.style.display = 'none'; this.formContainer.parentNode.style.display = 'none'; - const addButtonContainer = this.elem.querySelector('[comment-add-button-container]'); if (this.getCommentCount() > 0) { - this.elem.appendChild(addButtonContainer) + this.elem.appendChild(this.addButtonContainer) } else { - const countBar = this.elem.querySelector('[comment-count-bar]'); - countBar.appendChild(addButtonContainer); + this.commentCountBar.appendChild(this.addButtonContainer); } - addButtonContainer.style.display = 'block'; + this.addButtonContainer.style.display = 'block'; } getCommentCount() { @@ -154,15 +172,15 @@ class PageComments { setReply(commentElem) { this.showForm(); this.parentId = Number(commentElem.getAttribute('local-id')); - this.elem.querySelector('[comment-form-reply-to]').style.display = 'block'; - let replyLink = this.elem.querySelector('[comment-form-reply-to] a'); + this.replyToRow.style.display = 'block'; + const replyLink = this.replyToRow.querySelector('a'); replyLink.textContent = `#${this.parentId}`; replyLink.href = `#comment${this.parentId}`; } removeReplyTo() { this.parentId = null; - this.elem.querySelector('[comment-form-reply-to]').style.display = 'none'; + this.replyToRow.style.display = 'none'; } showLoading(formElem) { diff --git a/resources/js/index.js b/resources/js/index.js index 913313603..ffdb54e19 100644 --- a/resources/js/index.js +++ b/resources/js/index.js @@ -7,11 +7,10 @@ window.baseUrl = function(path) { }; // Set events and http services on window -import Events from "./services/events" +import events from "./services/events" import httpInstance from "./services/http" -const eventManager = new Events(); window.$http = httpInstance; -window.$events = eventManager; +window.$events = events; // Translation setup // Creates a global function with name 'trans' to be used in the same way as Laravel's translation system @@ -19,6 +18,7 @@ import Translations from "./services/translations" const translator = new Translations(); window.trans = translator.get.bind(translator); window.trans_choice = translator.getPlural.bind(translator); +window.trans_plural = translator.parsePlural.bind(translator); // Load Components import components from "./components" diff --git a/resources/js/services/events.js b/resources/js/services/events.js index fa3ed7fdf..6668014e7 100644 --- a/resources/js/services/events.js +++ b/resources/js/services/events.js @@ -1,55 +1,66 @@ +const listeners = {}; +const stack = []; + /** - * Simple global events manager + * Emit a custom event for any handlers to pick-up. + * @param {String} eventName + * @param {*} eventData */ -class Events { - constructor() { - this.listeners = {}; - this.stack = []; - } - - /** - * Emit a custom event for any handlers to pick-up. - * @param {String} eventName - * @param {*} eventData - * @returns {Events} - */ - emit(eventName, eventData) { - this.stack.push({name: eventName, data: eventData}); - if (typeof this.listeners[eventName] === 'undefined') return this; - let eventsToStart = this.listeners[eventName]; - for (let i = 0; i < eventsToStart.length; i++) { - let event = eventsToStart[i]; - event(eventData); - } - return this; - } - - /** - * Listen to a custom event and run the given callback when that event occurs. - * @param {String} eventName - * @param {Function} callback - * @returns {Events} - */ - listen(eventName, callback) { - if (typeof this.listeners[eventName] === 'undefined') this.listeners[eventName] = []; - this.listeners[eventName].push(callback); - return this; - } - - /** - * Emit an event for public use. - * Sends the event via the native DOM event handling system. - * @param {Element} targetElement - * @param {String} eventName - * @param {Object} eventData - */ - emitPublic(targetElement, eventName, eventData) { - const event = new CustomEvent(eventName, { - detail: eventData, - bubbles: true - }); - targetElement.dispatchEvent(event); +function emit(eventName, eventData) { + stack.push({name: eventName, data: eventData}); + if (typeof listeners[eventName] === 'undefined') return this; + let eventsToStart = listeners[eventName]; + for (let i = 0; i < eventsToStart.length; i++) { + let event = eventsToStart[i]; + event(eventData); } } -export default Events; \ No newline at end of file +/** + * Listen to a custom event and run the given callback when that event occurs. + * @param {String} eventName + * @param {Function} callback + * @returns {Events} + */ +function listen(eventName, callback) { + if (typeof listeners[eventName] === 'undefined') listeners[eventName] = []; + listeners[eventName].push(callback); +} + +/** + * Emit an event for public use. + * Sends the event via the native DOM event handling system. + * @param {Element} targetElement + * @param {String} eventName + * @param {Object} eventData + */ +function emitPublic(targetElement, eventName, eventData) { + const event = new CustomEvent(eventName, { + detail: eventData, + bubbles: true + }); + targetElement.dispatchEvent(event); +} + +/** + * Notify of a http error. + * Check for standard scenarios such as validation errors and + * formats an error notification accordingly. + * @param {Error} error + */ +function showValidationErrors(error) { + if (!error.status) return; + if (error.status === 422 && error.data) { + const message = Object.values(error.data).flat().join('\n'); + emit('error', message); + } +} + +export default { + emit, + emitPublic, + listen, + success: (msg) => emit('success', msg), + error: (msg) => emit('error', msg), + showValidationErrors, +} \ No newline at end of file diff --git a/resources/js/services/http.js b/resources/js/services/http.js index 5b5e1c496..8ecd6c109 100644 --- a/resources/js/services/http.js +++ b/resources/js/services/http.js @@ -69,7 +69,10 @@ async function dataRequest(method, url, data = null) { // Send data as JSON if a plain object if (typeof data === 'object' && !(data instanceof FormData)) { - options.headers = {'Content-Type': 'application/json'}; + options.headers = { + 'Content-Type': 'application/json', + 'X-Requested-With': 'XMLHttpRequest', + }; options.body = JSON.stringify(data); } diff --git a/resources/js/services/translations.js b/resources/js/services/translations.js index b595a05e6..62bb51f56 100644 --- a/resources/js/services/translations.js +++ b/resources/js/services/translations.js @@ -47,7 +47,19 @@ class Translator { */ getPlural(key, count, replacements) { const text = this.getTransText(key); - const splitText = text.split('|'); + return this.parsePlural(text, count, replacements); + } + + /** + * Parse the given translation and find the correct plural option + * to use. Similar format at laravel's 'trans_choice' helper. + * @param {String} translation + * @param {Number} count + * @param {Object} replacements + * @returns {String} + */ + parsePlural(translation, count, replacements) { + const splitText = translation.split('|'); const exactCountRegex = /^{([0-9]+)}/; const rangeRegex = /^\[([0-9]+),([0-9*]+)]/; let result = null; diff --git a/resources/views/comments/comments.blade.php b/resources/views/comments/comments.blade.php index fc81f13ee..140d0d027 100644 --- a/resources/views/comments/comments.blade.php +++ b/resources/views/comments/comments.blade.php @@ -1,23 +1,23 @@ -
+
- @exposeTranslations([ - 'entities.comment_updated_success', - 'entities.comment_deleted_success', - 'entities.comment_created_success', - 'entities.comment_count', - ]) - -
+
{{ trans_choice('entities.comment_count', count($page->comments), ['count' => count($page->comments)]) }}
@if (count($page->comments) === 0 && userCan('comment-create-all')) -
+
@endif
-
+
@foreach($page->comments as $comment) @include('comments.comment', ['comment' => $comment]) @endforeach @@ -27,7 +27,7 @@ @include('comments.create') @if (count($page->comments) > 0) -
+
diff --git a/resources/views/comments/create.blade.php b/resources/views/comments/create.blade.php index 61e41a354..12216b95b 100644 --- a/resources/views/comments/create.blade.php +++ b/resources/views/comments/create.blade.php @@ -1,6 +1,7 @@ -