diff --git a/app/Actions/Comment.php b/app/Actions/Comment.php index f138ee4a1..655d45221 100644 --- a/app/Actions/Comment.php +++ b/app/Actions/Comment.php @@ -2,9 +2,15 @@ use BookStack\Ownable; +/** + * @property string text + * @property string html + * @property int|null parent_id + * @property int local_id + */ class Comment extends Ownable { - protected $fillable = ['text', 'html', 'parent_id']; + protected $fillable = ['text', 'parent_id']; protected $appends = ['created', 'updated']; /** diff --git a/app/Actions/CommentRepo.php b/app/Actions/CommentRepo.php index 5ff639e2e..0d8a12ce9 100644 --- a/app/Actions/CommentRepo.php +++ b/app/Actions/CommentRepo.php @@ -1,23 +1,20 @@ comment = $comment; @@ -25,65 +22,71 @@ class CommentRepo /** * Get a comment by ID. - * @param $id - * @return \BookStack\Actions\Comment|\Illuminate\Database\Eloquent\Model */ - public function getById($id) + public function getById(int $id): Comment { return $this->comment->newQuery()->findOrFail($id); } /** * Create a new comment on an entity. - * @param \BookStack\Entities\Entity $entity - * @param array $data - * @return \BookStack\Actions\Comment */ - public function create(Entity $entity, $data = []) + public function create(Entity $entity, string $text, ?int $parent_id): Comment { $userId = user()->id; - $comment = $this->comment->newInstance($data); + $comment = $this->comment->newInstance(); + + $comment->text = $text; + $comment->html = $this->commentToHtml($text); $comment->created_by = $userId; $comment->updated_by = $userId; $comment->local_id = $this->getNextLocalId($entity); + $comment->parent_id = $parent_id; + $entity->comments()->save($comment); return $comment; } /** * Update an existing comment. - * @param \BookStack\Actions\Comment $comment - * @param array $input - * @return mixed */ - public function update($comment, $input) + public function update(Comment $comment, string $text): Comment { $comment->updated_by = user()->id; - $comment->update($input); + $comment->text = $text; + $comment->html = $this->commentToHtml($text); + $comment->save(); return $comment; } /** * Delete a comment from the system. - * @param \BookStack\Actions\Comment $comment - * @return mixed */ - public function delete($comment) + public function delete(Comment $comment) { - return $comment->delete(); + $comment->delete(); + } + + /** + * Convert the given comment markdown text to HTML. + */ + protected function commentToHtml(string $commentText): string + { + $converter = new CommonMarkConverter([ + 'html_input' => 'strip', + 'max_nesting_level' => 10, + 'allow_unsafe_links' => false, + ]); + + return $converter->convertToHtml($commentText); } /** * Get the next local ID relative to the linked entity. - * @param \BookStack\Entities\Entity $entity - * @return int */ - protected function getNextLocalId(Entity $entity) + protected function getNextLocalId(Entity $entity): int { $comments = $entity->comments(false)->orderBy('local_id', 'desc')->first(); - if ($comments === null) { - return 1; - } - return $comments->local_id + 1; + return ($comments->local_id ?? 0) + 1; } } diff --git a/app/Http/Controllers/CommentController.php b/app/Http/Controllers/CommentController.php index 068358d72..4eb56a4b0 100644 --- a/app/Http/Controllers/CommentController.php +++ b/app/Http/Controllers/CommentController.php @@ -10,9 +10,6 @@ class CommentController extends Controller { protected $commentRepo; - /** - * CommentController constructor. - */ public function __construct(CommentRepo $commentRepo) { $this->commentRepo = $commentRepo; @@ -23,11 +20,11 @@ class CommentController extends Controller * Save a new comment for a Page * @throws ValidationException */ - public function savePageComment(Request $request, int $pageId, int $commentId = null) + public function savePageComment(Request $request, int $pageId) { $this->validate($request, [ 'text' => 'required|string', - 'html' => 'required|string', + 'parent_id' => 'nullable|integer', ]); $page = Page::visible()->find($pageId); @@ -35,8 +32,6 @@ class CommentController extends Controller return response('Not found', 404); } - $this->checkOwnablePermission('page-view', $page); - // Prevent adding comments to draft pages if ($page->draft) { return $this->jsonError(trans('errors.cannot_add_comment_to_draft'), 400); @@ -44,7 +39,7 @@ class CommentController extends Controller // Create a new comment. $this->checkPermission('comment-create-all'); - $comment = $this->commentRepo->create($page, $request->only(['html', 'text', 'parent_id'])); + $comment = $this->commentRepo->create($page, $request->get('text'), $request->get('parent_id')); Activity::add($page, 'commented_on', $page->book->id); return view('comments.comment', ['comment' => $comment]); } @@ -57,14 +52,13 @@ class CommentController extends Controller { $this->validate($request, [ 'text' => 'required|string', - 'html' => 'required|string', ]); $comment = $this->commentRepo->getById($commentId); $this->checkOwnablePermission('page-view', $comment->entity); $this->checkOwnablePermission('comment-update', $comment); - $comment = $this->commentRepo->update($comment, $request->only(['html', 'text'])); + $comment = $this->commentRepo->update($comment, $request->get('text')); return view('comments.comment', ['comment' => $comment]); } diff --git a/app/Ownable.php b/app/Ownable.php index e660a0500..bf24fad5d 100644 --- a/app/Ownable.php +++ b/app/Ownable.php @@ -2,6 +2,10 @@ use BookStack\Auth\User; +/** + * @property int created_by + * @property int updated_by + */ abstract class Ownable extends Model { /** diff --git a/composer.json b/composer.json index 6da651898..80e8b0a61 100644 --- a/composer.json +++ b/composer.json @@ -16,12 +16,15 @@ "barryvdh/laravel-dompdf": "^0.8.5", "barryvdh/laravel-snappy": "^0.4.5", "doctrine/dbal": "^2.9", + "facade/ignition": "^1.4", "fideloper/proxy": "^4.0", "gathercontent/htmldiff": "^0.2.1", "intervention/image": "^2.5", "laravel/framework": "^6.12", "laravel/socialite": "^4.3.2", + "league/commonmark": "^1.4", "league/flysystem-aws-s3-v3": "^1.0", + "nunomaduro/collision": "^3.0", "onelogin/php-saml": "^3.3", "predis/predis": "^1.1", "socialiteproviders/discord": "^2.0", @@ -29,9 +32,7 @@ "socialiteproviders/microsoft-azure": "^3.0", "socialiteproviders/okta": "^1.0", "socialiteproviders/slack": "^3.0", - "socialiteproviders/twitch": "^5.0", - "facade/ignition": "^1.4", - "nunomaduro/collision": "^3.0" + "socialiteproviders/twitch": "^5.0" }, "require-dev": { "barryvdh/laravel-debugbar": "^3.2.8", diff --git a/composer.lock b/composer.lock index fd444ffa8..3ddd28e5a 100644 --- a/composer.lock +++ b/composer.lock @@ -4,7 +4,7 @@ "Read more about it at https://getcomposer.org/doc/01-basic-usage.md#installing-dependencies", "This file is @generated automatically" ], - "content-hash": "7836017e48e93254d6ff924b07f84597", + "content-hash": "bbe47cff4f167fd6ce7047dff4602a78", "packages": [ { "name": "aws/aws-sdk-php", @@ -1775,16 +1775,16 @@ }, { "name": "league/commonmark", - "version": "1.3.2", + "version": "1.4.2", "source": { "type": "git", "url": "https://github.com/thephpleague/commonmark.git", - "reference": "75542a366ccbe1896ed79fcf3e8e68206d6c4257" + "reference": "9e780d972185e4f737a03bade0fd34a9e67bbf31" }, "dist": { "type": "zip", - "url": "https://api.github.com/repos/thephpleague/commonmark/zipball/75542a366ccbe1896ed79fcf3e8e68206d6c4257", - "reference": "75542a366ccbe1896ed79fcf3e8e68206d6c4257", + "url": "https://api.github.com/repos/thephpleague/commonmark/zipball/9e780d972185e4f737a03bade0fd34a9e67bbf31", + "reference": "9e780d972185e4f737a03bade0fd34a9e67bbf31", "shasum": "" }, "require": { @@ -1802,7 +1802,7 @@ "github/gfm": "0.29.0", "michelf/php-markdown": "~1.4", "mikehaertl/php-shellcommand": "^1.4", - "phpstan/phpstan-shim": "^0.11.5", + "phpstan/phpstan": "^0.12", "phpunit/phpunit": "^7.5", "scrutinizer/ocular": "^1.5", "symfony/finder": "^4.2" @@ -1845,7 +1845,33 @@ "md", "parser" ], - "time": "2020-03-25T19:55:28+00:00" + "funding": [ + { + "url": "https://enjoy.gitstore.app/repositories/thephpleague/commonmark", + "type": "custom" + }, + { + "url": "https://www.colinodell.com/sponsor", + "type": "custom" + }, + { + "url": "https://www.paypal.me/colinpodell/10.00", + "type": "custom" + }, + { + "url": "https://github.com/colinodell", + "type": "github" + }, + { + "url": "https://www.patreon.com/colinodell", + "type": "patreon" + }, + { + "url": "https://tidelift.com/funding/github/packagist/league/commonmark", + "type": "tidelift" + } + ], + "time": "2020-04-24T13:39:56+00:00" }, { "name": "league/flysystem", @@ -7716,5 +7742,6 @@ "platform-dev": [], "platform-overrides": { "php": "7.2.0" - } + }, + "plugin-api-version": "1.1.0" } diff --git a/resources/js/components/page-comments.js b/resources/js/components/page-comments.js index 5d8d16958..5d826cba1 100644 --- a/resources/js/components/page-comments.js +++ b/resources/js/components/page-comments.js @@ -1,8 +1,5 @@ -import MarkdownIt from "markdown-it"; import {scrollAndHighlightElement} from "../services/util"; -const md = new MarkdownIt({ html: false }); - class PageComments { constructor(elem) { @@ -68,12 +65,11 @@ class PageComments { let text = form.querySelector('textarea').value; let reqData = { text: text, - html: md.render(text), parent_id: this.parentId || null, }; this.showLoading(form); let commentId = this.editingComment.getAttribute('comment'); - window.$http.put(window.baseUrl(`/ajax/comment/${commentId}`), reqData).then(resp => { + window.$http.put(`/ajax/comment/${commentId}`, reqData).then(resp => { let newComment = document.createElement('div'); newComment.innerHTML = resp.data; this.editingComment.innerHTML = newComment.children[0].innerHTML; @@ -88,7 +84,7 @@ class PageComments { deleteComment(commentElem) { let id = commentElem.getAttribute('comment'); this.showLoading(commentElem.querySelector('[comment-content]')); - window.$http.delete(window.baseUrl(`/ajax/comment/${id}`)).then(resp => { + window.$http.delete(`/ajax/comment/${id}`).then(resp => { commentElem.parentNode.removeChild(commentElem); window.$events.emit('success', window.trans('entities.comment_deleted_success')); this.updateCount(); @@ -102,11 +98,10 @@ class PageComments { let text = this.formInput.value; let reqData = { text: text, - html: md.render(text), parent_id: this.parentId || null, }; this.showLoading(this.form); - window.$http.post(window.baseUrl(`/ajax/page/${this.pageId}/comment`), reqData).then(resp => { + window.$http.post(`/ajax/page/${this.pageId}/comment`, reqData).then(resp => { let newComment = document.createElement('div'); newComment.innerHTML = resp.data; let newElem = newComment.children[0]; @@ -171,17 +166,17 @@ class PageComments { } showLoading(formElem) { - let groups = formElem.querySelectorAll('.form-group'); - for (let i = 0, len = groups.length; i < len; i++) { - groups[i].style.display = 'none'; + const groups = formElem.querySelectorAll('.form-group'); + for (let group of groups) { + group.style.display = 'none'; } formElem.querySelector('.form-group.loading').style.display = 'block'; } hideLoading(formElem) { - let groups = formElem.querySelectorAll('.form-group'); - for (let i = 0, len = groups.length; i < len; i++) { - groups[i].style.display = 'block'; + const groups = formElem.querySelectorAll('.form-group'); + for (let group of groups) { + group.style.display = 'block'; } formElem.querySelector('.form-group.loading').style.display = 'none'; } diff --git a/tests/Entity/CommentTest.php b/tests/Entity/CommentTest.php index a2126407b..2562f7e7d 100644 --- a/tests/Entity/CommentTest.php +++ b/tests/Entity/CommentTest.php @@ -42,7 +42,6 @@ class CommentTest extends TestCase $newText = 'updated text content'; $resp = $this->putJson("/ajax/comment/$comment->id", [ 'text' => $newText, - 'html' => '

'.$newText.'

', ]); $resp->assertStatus(200); @@ -72,4 +71,46 @@ class CommentTest extends TestCase 'id' => $comment->id ]); } + + public function test_comments_converts_markdown_input_to_html() + { + $page = Page::first(); + $this->asAdmin()->postJson("/ajax/page/$page->id/comment", [ + 'text' => '# My Title', + ]); + + $this->assertDatabaseHas('comments', [ + 'entity_id' => $page->id, + 'entity_type' => $page->getMorphClass(), + 'text' => '# My Title', + 'html' => "

My Title

\n", + ]); + + $pageView = $this->get($page->getUrl()); + $pageView->assertSee('

My Title

'); + } + + public function test_html_cannot_be_injected_via_comment_content() + { + $this->asAdmin(); + $page = Page::first(); + + $script = '\n\n# sometextinthecomment'; + $this->postJson("/ajax/page/$page->id/comment", [ + 'text' => $script, + ]); + + $pageView = $this->get($page->getUrl()); + $pageView->assertDontSee($script); + $pageView->assertSee('sometextinthecomment'); + + $comment = $page->comments()->first(); + $this->putJson("/ajax/comment/$comment->id", [ + 'text' => $script . 'updated', + ]); + + $pageView = $this->get($page->getUrl()); + $pageView->assertDontSee($script); + $pageView->assertSee('sometextinthecommentupdated'); + } }