From 99c6d70c5160e3c050218eec44180176718a36d8 Mon Sep 17 00:00:00 2001 From: Mark James Date: Mon, 31 Dec 2018 17:01:49 +1100 Subject: [PATCH 01/23] Initial updates to allow for page copy when the user can read the page but can't update it. --- app/Http/Controllers/PageController.php | 4 ++-- resources/views/pages/show.blade.php | 6 ++++-- 2 files changed, 6 insertions(+), 4 deletions(-) diff --git a/app/Http/Controllers/PageController.php b/app/Http/Controllers/PageController.php index 74595443b..de3720f97 100644 --- a/app/Http/Controllers/PageController.php +++ b/app/Http/Controllers/PageController.php @@ -641,7 +641,7 @@ class PageController extends Controller public function showCopy($bookSlug, $pageSlug) { $page = $this->pageRepo->getPageBySlug($pageSlug, $bookSlug); - $this->checkOwnablePermission('page-update', $page); + $this->checkOwnablePermission('page-view', $page); session()->flashInput(['name' => $page->name]); return view('pages/copy', [ 'book' => $page->book, @@ -660,7 +660,7 @@ class PageController extends Controller public function copy($bookSlug, $pageSlug, Request $request) { $page = $this->pageRepo->getPageBySlug($pageSlug, $bookSlug); - $this->checkOwnablePermission('page-update', $page); + $this->checkOwnablePermission('page-view', $page); $entitySelection = $request->get('entity_selection', null); if ($entitySelection === null || $entitySelection === '') { diff --git a/resources/views/pages/show.blade.php b/resources/views/pages/show.blade.php index 0b6aa7d14..19df337fc 100644 --- a/resources/views/pages/show.blade.php +++ b/resources/views/pages/show.blade.php @@ -17,12 +17,14 @@ @if(userCan('page-update', $page)) @icon('edit'){{ trans('common.edit') }} @endif - @if(userCan('page-update', $page) || userCan('restrictions-manage', $page) || userCan('page-delete', $page)) + @if(userCan('page-create-own') || userCan('page-create-all') || userCan('page-update', $page) || userCan('restrictions-manage', $page) || userCan('page-delete', $page))
@icon('more') {{ trans('common.more') }}
- \ No newline at end of file + From 9511d10ec852bab3b9f7aefaa8848c5d97d28918 Mon Sep 17 00:00:00 2001 From: Xiphoseer Date: Tue, 12 Feb 2019 12:24:01 +0100 Subject: [PATCH 06/23] Update entities.php Add german shelve localizations --- resources/lang/de/entities.php | 33 +++++++++++++++++++++++++++++++++ 1 file changed, 33 insertions(+) diff --git a/resources/lang/de/entities.php b/resources/lang/de/entities.php index 7c27be17b..9b24bd72c 100644 --- a/resources/lang/de/entities.php +++ b/resources/lang/de/entities.php @@ -60,6 +60,39 @@ return [ 'search_created_after' => 'Erstellt nach', 'search_set_date' => 'Datum auswählen', 'search_update' => 'Suche aktualisieren', + + /* + * Shelves + */ + 'shelf' => 'Regal', + 'shelves' => 'Regale', + 'shelves_long' => 'Bücherregal', + 'shelves_empty' => 'Es wurden noch keine Regale angelegt', + 'shelves_create' => 'Erzeuge ein Regal', + 'shelves_popular' => 'Beliebte Regale', + 'shelves_new' => 'Kürzlich erstellte Regale', + 'shelves_popular_empty' => 'Die beliebtesten Regale werden hier angezeigt.', + 'shelves_new_empty' => 'Die neusten Regale werden hier angezeigt.', + 'shelves_save' => 'Regal speichern', + 'shelves_books' => 'Bücher in diesem Regal', + 'shelves_add_books' => 'Buch zu diesem Regal hinzufügen', + 'shelves_drag_books' => 'Bücher hier hin ziehen um sie dem Regal hinzuzufügen', + 'shelves_empty_contents' => 'Diesem Regal sind keine Bücher zugewiesen', + 'shelves_edit_and_assign' => 'Regal bearbeiten um Bücher hinzuzufügen', + 'shelves_edit_named' => 'Bücherregal :name bearbeiten', + 'shelves_edit' => 'Bücherregal bearbeiten', + 'shelves_delete' => 'Bücherregal löschen', + 'shelves_delete_named' => 'Bücherregal :name löschen', + 'shelves_delete_explain' => "Sie sind im Begriff das Bücherregal mit dem Namen ':name' zu löschen. Enthaltene Bücher werden nicht gelöscht.", + 'shelves_delete_confirmation' => 'Sind Sie sicher, dass Sie dieses Bücherregal löschen wollen?', + 'shelves_permissions' => 'Regal-Berechtigungen', + 'shelves_permissions_updated' => 'Regal-Berechtigungen aktualisiert', + 'shelves_permissions_active' => 'Regal-Berechtigungen aktiv', + 'shelves_copy_permissions_to_books' => 'Kopiere die Berechtigungen zum Buch', + 'shelves_copy_permissions' => 'Berechtigungen kopieren', + 'shelves_copy_permissions_explain' => 'Hiermit werden die Berechtigungen des aktuellen Regals auf alle enthaltenen Bücher übertragen. Überprüfe vor der Aktivierung ob alle Berechtigungsänderungen am aktuellen Buch gespeichert wurden.', + 'shelves_copy_permission_success' => 'Regal-Berechtigungen wurden zu :count Büchern kopiert', + /** * Books */ From 19bb11a1c90643fb8814289b61120bd3276075ed Mon Sep 17 00:00:00 2001 From: Xiphoseer Date: Tue, 12 Feb 2019 12:28:48 +0100 Subject: [PATCH 07/23] Update entities.php Add informal german shelve localisations --- resources/lang/de_informal/entities.php | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/resources/lang/de_informal/entities.php b/resources/lang/de_informal/entities.php index 21fdbb13d..8d7abc4fa 100644 --- a/resources/lang/de_informal/entities.php +++ b/resources/lang/de_informal/entities.php @@ -9,6 +9,13 @@ return [ 'no_pages_recently_created' => 'Du hast bisher keine Seiten angelegt.', 'no_pages_recently_updated' => 'Du hast bisher keine Seiten aktualisiert.', + /** + * Shelves + */ + 'shelves_delete_explain' => "Du bist im Begriff das Bücherregal mit dem Namen ':name' zu löschen. Enthaltene Bücher werden nicht gelöscht.", + 'shelves_delete_confirmation' => 'Bist du sicher, dass du dieses Bücherregal löschen willst?', + 'shelves_copy_permissions_explain' => 'Hiermit werden die Berechtigungen des aktuellen Regals auf alle enthaltenen Bücher übertragen. Überprüfe vor der Aktivierung, ob alle Berechtigungsänderungen am aktuellen Buch gespeichert wurden.', + /** * Books */ From edd98c00e565cd421cd6290dc01a4235162ba519 Mon Sep 17 00:00:00 2001 From: Xiphoseer Date: Tue, 12 Feb 2019 12:30:12 +0100 Subject: [PATCH 08/23] Update entities.php --- resources/lang/de/entities.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/resources/lang/de/entities.php b/resources/lang/de/entities.php index 9b24bd72c..07a92e2c7 100644 --- a/resources/lang/de/entities.php +++ b/resources/lang/de/entities.php @@ -90,7 +90,7 @@ return [ 'shelves_permissions_active' => 'Regal-Berechtigungen aktiv', 'shelves_copy_permissions_to_books' => 'Kopiere die Berechtigungen zum Buch', 'shelves_copy_permissions' => 'Berechtigungen kopieren', - 'shelves_copy_permissions_explain' => 'Hiermit werden die Berechtigungen des aktuellen Regals auf alle enthaltenen Bücher übertragen. Überprüfe vor der Aktivierung ob alle Berechtigungsänderungen am aktuellen Buch gespeichert wurden.', + 'shelves_copy_permissions_explain' => 'Hiermit werden die Berechtigungen des aktuellen Regals auf alle enthaltenen Bücher übertragen. Überprüfen Sie vor der Aktivierung, ob alle Berechtigungsänderungen am aktuellen Regal gespeichert wurden.', 'shelves_copy_permission_success' => 'Regal-Berechtigungen wurden zu :count Büchern kopiert', /** From 058cc2cbd65ce7d963e05228d751f34104b8c69b Mon Sep 17 00:00:00 2001 From: Xiphoseer Date: Tue, 12 Feb 2019 12:30:43 +0100 Subject: [PATCH 09/23] Update entities.php --- resources/lang/de_informal/entities.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/resources/lang/de_informal/entities.php b/resources/lang/de_informal/entities.php index 8d7abc4fa..1decdd7b7 100644 --- a/resources/lang/de_informal/entities.php +++ b/resources/lang/de_informal/entities.php @@ -14,7 +14,7 @@ return [ */ 'shelves_delete_explain' => "Du bist im Begriff das Bücherregal mit dem Namen ':name' zu löschen. Enthaltene Bücher werden nicht gelöscht.", 'shelves_delete_confirmation' => 'Bist du sicher, dass du dieses Bücherregal löschen willst?', - 'shelves_copy_permissions_explain' => 'Hiermit werden die Berechtigungen des aktuellen Regals auf alle enthaltenen Bücher übertragen. Überprüfe vor der Aktivierung, ob alle Berechtigungsänderungen am aktuellen Buch gespeichert wurden.', + 'shelves_copy_permissions_explain' => 'Hiermit werden die Berechtigungen des aktuellen Regals auf alle enthaltenen Bücher übertragen. Überprüfe vor der Aktivierung, ob alle Berechtigungsänderungen am aktuellen Regal gespeichert wurden.', /** * Books From e471d0c52a59752c609a19f29af0d4ccebf0048b Mon Sep 17 00:00:00 2001 From: Dan Brown Date: Sat, 2 Mar 2019 08:52:14 +0000 Subject: [PATCH 10/23] Added lua to code languages Closes #1223 --- resources/assets/js/services/code.js | 4 +++- resources/views/components/code-editor.blade.php | 1 + 2 files changed, 4 insertions(+), 1 deletion(-) diff --git a/resources/assets/js/services/code.js b/resources/assets/js/services/code.js index cfeabd3be..bd749033d 100644 --- a/resources/assets/js/services/code.js +++ b/resources/assets/js/services/code.js @@ -8,6 +8,7 @@ import 'codemirror/mode/diff/diff'; import 'codemirror/mode/go/go'; import 'codemirror/mode/htmlmixed/htmlmixed'; import 'codemirror/mode/javascript/javascript'; +import 'codemirror/mode/lua/lua'; import 'codemirror/mode/markdown/markdown'; import 'codemirror/mode/nginx/nginx'; import 'codemirror/mode/php/php'; @@ -38,12 +39,13 @@ const modeMap = { javascript: 'javascript', json: {name: 'javascript', json: true}, js: 'javascript', - php: 'php', + lua: 'lua', md: 'markdown', mdown: 'markdown', markdown: 'markdown', nginx: 'nginx', powershell: 'powershell', + php: 'php', py: 'python', python: 'python', ruby: 'ruby', diff --git a/resources/views/components/code-editor.blade.php b/resources/views/components/code-editor.blade.php index cc6c10949..67e7b7773 100644 --- a/resources/views/components/code-editor.blade.php +++ b/resources/views/components/code-editor.blade.php @@ -21,6 +21,7 @@ Java JavaScript JSON + Lua PHP Powershell MarkDown From b273b9d6d04e3c0de999cda0ce52560137dc47f1 Mon Sep 17 00:00:00 2001 From: Dan Brown Date: Sat, 2 Mar 2019 09:08:01 +0000 Subject: [PATCH 11/23] Improved alignment classes used by WYSIWYG editor - Fixed table cells being floated, Fixes #1284. - Made it possible to easily center linked images. --- resources/assets/sass/_pages.scss | 11 +++++++++-- 1 file changed, 9 insertions(+), 2 deletions(-) diff --git a/resources/assets/sass/_pages.scss b/resources/assets/sass/_pages.scss index 21fdf90de..84280319d 100755 --- a/resources/assets/sass/_pages.scss +++ b/resources/assets/sass/_pages.scss @@ -51,15 +51,22 @@ margin: $-xs $-s $-xs 0; } .align-right { - float: right !important; + text-align: right !important; } img.align-right, table.align-right { - text-align: right; + float: right !important; margin: $-xs 0 $-xs $-s; } .align-center { text-align: center; } + img.align-center { + display: block; + } + img.align-center, table.align-center { + margin-left: auto; + margin-right: auto; + } img { max-width: 100%; height:auto; From 6f710225b51f10ea474aa2b2f62573c5d9251209 Mon Sep 17 00:00:00 2001 From: Jamie Schouten Date: Wed, 6 Mar 2019 17:10:15 +0100 Subject: [PATCH 12/23] Update Dutch password_hint translation to correspond with validation rule At the moment the translation says ```Minimaal 5 tekens``` which means your password should be at least 5 characters long. But a 5 character long password is not allowed by the validator. I think this was a translation error from the English one where it says ```Must be over 5 characters```. To make the Dutch translation correct the Dutch translation should be changed to ```Minimaal 6 tekens```. ``` /** * Get a validator for an incoming registration request. * * @param array $data * @return \Illuminate\Contracts\Validation\Validator */ protected function validator(array $data) { return Validator::make($data, [ 'name' => 'required|max:255', 'email' => 'required|email|max:255|unique:users', 'password' => 'required|min:6', ]); } ``` --- resources/lang/nl/auth.php | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/resources/lang/nl/auth.php b/resources/lang/nl/auth.php index d8813f07b..31bd330cc 100644 --- a/resources/lang/nl/auth.php +++ b/resources/lang/nl/auth.php @@ -27,7 +27,7 @@ return [ 'email' => 'Email', 'password' => 'Wachtwoord', 'password_confirm' => 'Wachtwoord Bevestigen', - 'password_hint' => 'Minimaal 5 tekens', + 'password_hint' => 'Minimaal 6 tekens', 'forgot_password' => 'Wachtwoord vergeten?', 'remember_me' => 'Mij onthouden', 'ldap_email_hint' => 'Geef een email op waarmee je dit account wilt gebruiken.', @@ -73,4 +73,4 @@ return [ 'email_not_confirmed_click_link' => 'Klik op de link in de e-mail die vlak na je registratie is verstuurd.', 'email_not_confirmed_resend' => 'Als je deze e-mail niet kunt vinden kun je deze met onderstaande formulier opnieuw verzenden.', 'email_not_confirmed_resend_button' => 'Bevestigingsmail Opnieuw Verzenden', -]; \ No newline at end of file +]; From 98a4359198d84fe3519361d5ccc3d0a6ff26fd64 Mon Sep 17 00:00:00 2001 From: Dan Brown Date: Thu, 7 Mar 2019 21:09:23 +0000 Subject: [PATCH 13/23] Updated user language select to use correct default - Updated localisation system to take note of system defaul locale before replacing the current locale Fixes #1316 --- app/Http/Middleware/Localization.php | 3 +-- app/Settings/SettingService.php | 1 + resources/views/users/edit.blade.php | 3 ++- 3 files changed, 4 insertions(+), 3 deletions(-) diff --git a/app/Http/Middleware/Localization.php b/app/Http/Middleware/Localization.php index e65b417d5..ff5526cc7 100644 --- a/app/Http/Middleware/Localization.php +++ b/app/Http/Middleware/Localization.php @@ -51,6 +51,7 @@ class Localization public function handle($request, Closure $next) { $defaultLang = config('app.locale'); + config()->set('app.default_locale', $defaultLang); if (user()->isDefault() && config('app.auto_detect_locale')) { $locale = $this->autoDetectLocale($request, $defaultLang); @@ -63,8 +64,6 @@ class Localization config()->set('app.rtl', true); } - - app()->setLocale($locale); Carbon::setLocale($locale); $this->setSystemDateLocale($locale); diff --git a/app/Settings/SettingService.php b/app/Settings/SettingService.php index c903bd60a..c9491e3ee 100644 --- a/app/Settings/SettingService.php +++ b/app/Settings/SettingService.php @@ -41,6 +41,7 @@ class SettingService if ($default === false) { $default = config('setting-defaults.' . $key, false); } + if (isset($this->localCache[$key])) { return $this->localCache[$key]; } diff --git a/resources/views/users/edit.blade.php b/resources/views/users/edit.blade.php index c4b02f9b9..1b0514f9c 100644 --- a/resources/views/users/edit.blade.php +++ b/resources/views/users/edit.blade.php @@ -38,8 +38,9 @@
From 1c312906bc8eec26d75934e74cab01284dee2bb3 Mon Sep 17 00:00:00 2001 From: Dan Brown Date: Fri, 8 Mar 2019 21:06:37 +0000 Subject: [PATCH 14/23] Added a configurable upload size limit Closes #1293 --- resources/assets/js/vues/components/dropzone.js | 1 + 1 file changed, 1 insertion(+) diff --git a/resources/assets/js/vues/components/dropzone.js b/resources/assets/js/vues/components/dropzone.js index 31a84a267..9d3d22b4d 100644 --- a/resources/assets/js/vues/components/dropzone.js +++ b/resources/assets/js/vues/components/dropzone.js @@ -16,6 +16,7 @@ function mounted() { addRemoveLinks: true, dictRemoveFile: trans('components.image_upload_remove'), timeout: Number(window.uploadTimeout) || 60000, + maxFilesize: Number(window.uploadLimit) || 256, url: function() { return _this.uploadUrl; }, From d96baf2d4ac79909959361165887dbb963851ce0 Mon Sep 17 00:00:00 2001 From: Dan Brown Date: Fri, 8 Mar 2019 21:32:31 +0000 Subject: [PATCH 15/23] Set 'uploaded_to' parameters for editor-pasted/dragged images Allows image-listing permission system to work as intended. Fixes #1287 --- .../assets/js/components/markdown-editor.js | 43 ++++++++++++------- .../assets/js/components/wysiwyg-editor.js | 43 ++++++++++++------- 2 files changed, 55 insertions(+), 31 deletions(-) diff --git a/resources/assets/js/components/markdown-editor.js b/resources/assets/js/components/markdown-editor.js index 9228cfe2c..b8e2bc040 100644 --- a/resources/assets/js/components/markdown-editor.js +++ b/resources/assets/js/components/markdown-editor.js @@ -8,7 +8,11 @@ class MarkdownEditor { constructor(elem) { this.elem = elem; - this.textDirection = document.getElementById('page-editor').getAttribute('text-direction'); + + const pageEditor = document.getElementById('page-editor'); + this.pageId = pageEditor.getAttribute('page-id'); + this.textDirection = pageEditor.getAttribute('text-direction'); + this.markdown = new MarkdownIt({html: true}); this.markdown.use(mdTasksLists, {label: true}); @@ -98,7 +102,9 @@ class MarkdownEditor { } codeMirrorSetup() { - let cm = this.cm; + const cm = this.cm; + const context = this; + // Text direction // cm.setOption('direction', this.textDirection); cm.setOption('direction', 'ltr'); // Will force to remain as ltr for now due to issues when HTML is in editor. @@ -266,17 +272,18 @@ class MarkdownEditor { } // Insert image into markdown - let id = "image-" + Math.random().toString(16).slice(2); - let placeholderImage = window.baseUrl(`/loading.gif#upload${id}`); - let selectedText = cm.getSelection(); - let placeHolderText = `![${selectedText}](${placeholderImage})`; - let cursor = cm.getCursor(); + const id = "image-" + Math.random().toString(16).slice(2); + const placeholderImage = window.baseUrl(`/loading.gif#upload${id}`); + const selectedText = cm.getSelection(); + const placeHolderText = `![${selectedText}](${placeholderImage})`; + const cursor = cm.getCursor(); cm.replaceSelection(placeHolderText); cm.setCursor({line: cursor.line, ch: cursor.ch + selectedText.length + 3}); - let remoteFilename = "image-" + Date.now() + "." + ext; - let formData = new FormData(); + const remoteFilename = "image-" + Date.now() + "." + ext; + const formData = new FormData(); formData.append('file', file, remoteFilename); + formData.append('uploaded_to', context.pageId); window.$http.post('/images/gallery/upload', formData).then(resp => { const newContent = `[![${selectedText}](${resp.data.thumbs.display})](${resp.data.url})`; @@ -302,7 +309,7 @@ class MarkdownEditor { } actionInsertImage() { - let cursorPos = this.cm.getCursor('from'); + const cursorPos = this.cm.getCursor('from'); window.ImageManager.show(image => { let selectedText = this.cm.getSelection(); let newText = "[![" + (selectedText || image.name) + "](" + image.thumbs.display + ")](" + image.url + ")"; @@ -313,7 +320,7 @@ class MarkdownEditor { } actionShowImageManager() { - let cursorPos = this.cm.getCursor('from'); + const cursorPos = this.cm.getCursor('from'); window.ImageManager.show(image => { this.insertDrawing(image, cursorPos); }, 'drawio'); @@ -321,7 +328,7 @@ class MarkdownEditor { // Show the popup link selector and insert a link when finished actionShowLinkSelector() { - let cursorPos = this.cm.getCursor('from'); + const cursorPos = this.cm.getCursor('from'); window.EntitySelectorPopup.show(entity => { let selectedText = this.cm.getSelection() || entity.name; let newText = `[${selectedText}](${entity.link})`; @@ -357,7 +364,7 @@ class MarkdownEditor { } insertDrawing(image, originalCursor) { - let newText = `
`; + const newText = `
`; this.cm.focus(); this.cm.replaceSelection(newText); this.cm.setCursor(originalCursor.line, originalCursor.ch + newText.length); @@ -365,9 +372,13 @@ class MarkdownEditor { // Show draw.io if enabled and handle save. actionEditDrawing(imgContainer) { - if (document.querySelector('[drawio-enabled]').getAttribute('drawio-enabled') !== 'true') return; - let cursorPos = this.cm.getCursor('from'); - let drawingId = imgContainer.getAttribute('drawio-diagram'); + const drawingDisabled = document.querySelector('[drawio-enabled]').getAttribute('drawio-enabled') !== 'true'; + if (drawingDisabled) { + return; + } + + const cursorPos = this.cm.getCursor('from'); + const drawingId = imgContainer.getAttribute('drawio-diagram'); DrawIO.show(() => { return window.$http.get(window.baseUrl(`/images/base64/${drawingId}`)).then(resp => { diff --git a/resources/assets/js/components/wysiwyg-editor.js b/resources/assets/js/components/wysiwyg-editor.js index 39bce4959..9deb1d0a7 100644 --- a/resources/assets/js/components/wysiwyg-editor.js +++ b/resources/assets/js/components/wysiwyg-editor.js @@ -4,22 +4,24 @@ import DrawIO from "../services/drawio"; /** * Handle pasting images from clipboard. * @param {ClipboardEvent} event + * @param {WysiwygEditor} wysiwygComponent * @param editor */ -function editorPaste(event, editor) { +function editorPaste(event, editor, wysiwygComponent) { if (!event.clipboardData || !event.clipboardData.items) return; - let items = event.clipboardData.items; - for (let i = 0; i < items.length; i++) { - if (items[i].type.indexOf("image") === -1) continue; + for (let clipboardItem of event.clipboardData.items) { + if (clipboardItem.type.indexOf("image") === -1) continue; event.preventDefault(); - let id = "image-" + Math.random().toString(16).slice(2); - let loadingImage = window.baseUrl('/loading.gif'); - let file = items[i].getAsFile(); + const id = "image-" + Math.random().toString(16).slice(2); + const loadingImage = window.baseUrl('/loading.gif'); + const file = clipboardItem.getAsFile(); + setTimeout(() => { editor.insertContent(`

`); - uploadImageFile(file).then(resp => { + + uploadImageFile(file, wysiwygComponent).then(resp => { editor.dom.setAttrib(id, 'src', resp.thumbs.display); }).catch(err => { editor.dom.remove(id); @@ -33,9 +35,12 @@ function editorPaste(event, editor) { /** * Upload an image file to the server * @param {File} file + * @param {WysiwygEditor} wysiwygComponent */ -function uploadImageFile(file) { - if (file === null || file.type.indexOf('image') !== 0) return Promise.reject(`Not an image file`); +async function uploadImageFile(file, wysiwygComponent) { + if (file === null || file.type.indexOf('image') !== 0) { + throw new Error(`Not an image file`); + } let ext = 'png'; if (file.name) { @@ -43,11 +48,13 @@ function uploadImageFile(file) { if (fileNameMatches.length > 1) ext = fileNameMatches[1]; } - let remoteFilename = "image-" + Date.now() + "." + ext; - let formData = new FormData(); + const remoteFilename = "image-" + Date.now() + "." + ext; + const formData = new FormData(); formData.append('file', file, remoteFilename); + formData.append('uploaded_to', wysiwygComponent.pageId); - return window.$http.post(window.baseUrl('/images/gallery/upload'), formData).then(resp => (resp.data)); + const resp = await window.$http.post(window.baseUrl('/images/gallery/upload'), formData); + return resp.data; } function registerEditorShortcuts(editor) { @@ -370,7 +377,10 @@ class WysiwygEditor { constructor(elem) { this.elem = elem; - this.textDirection = document.getElementById('page-editor').getAttribute('text-direction'); + + const pageEditor = document.getElementById('page-editor'); + this.pageId = pageEditor.getAttribute('page-id'); + this.textDirection = pageEditor.getAttribute('text-direction'); this.plugins = "image table textcolor paste link autolink fullscreen imagetools code customhr autosave lists codeeditor media"; this.loadPlugins(); @@ -397,6 +407,9 @@ class WysiwygEditor { } getTinyMceConfig() { + + const context = this; + return { selector: '#html-editor', content_css: [ @@ -586,7 +599,7 @@ class WysiwygEditor { }); // Paste image-uploads - editor.on('paste', event => editorPaste(event, editor)); + editor.on('paste', event => editorPaste(event, editor, context)); } }; } From 646fd822c5e35485af23aa5319e3f34559a5a799 Mon Sep 17 00:00:00 2001 From: Dan Brown Date: Fri, 8 Mar 2019 22:42:48 +0000 Subject: [PATCH 16/23] Updated redis config logic, Now takes a password - Previous config did not use multiple servers in any way. - Cluster will now be created automatically if multiple servers given. - Removed REDIS_CLUSTER option. Closes #1283 --- .env.example.complete | 6 ++++++ config/database.php | 36 ++++++++++++++++++++++++++---------- 2 files changed, 32 insertions(+), 10 deletions(-) diff --git a/.env.example.complete b/.env.example.complete index 8851bd268..77a0508dc 100644 --- a/.env.example.complete +++ b/.env.example.complete @@ -75,6 +75,12 @@ CACHE_PREFIX=bookstack # For multiple servers separate with a comma MEMCACHED_SERVERS=127.0.0.1:11211:100 +# Redis server configuration +# This follows the following format: HOST:PORT:DATABASE +# or, if using a password: HOST:PORT:DATABASE:PASSWORD +# For multiple servers separate with a comma. These will be clustered. +REDIS_SERVERS=127.0.0.1:6379:0 + # Queue driver to use # Queue not really currently used but may be configurable in the future. # Would advise not to change this for now. diff --git a/config/database.php b/config/database.php index 6ca902944..93a44854f 100644 --- a/config/database.php +++ b/config/database.php @@ -8,23 +8,39 @@ * Do not edit this file unless you're happy to maintain any changes yourself. */ -// REDIS - Split out configuration into an array +// REDIS +// Split out configuration into an array if (env('REDIS_SERVERS', false)) { - $redisServerKeys = ['host', 'port', 'database']; + + $redisDefaults = ['host' => '127.0.0.1', 'port' => '6379', 'database' => '0', 'password' => null]; $redisServers = explode(',', trim(env('REDIS_SERVERS', '127.0.0.1:6379:0'), ',')); - $redisConfig = [ - 'cluster' => env('REDIS_CLUSTER', false) - ]; + $redisConfig = []; + $cluster = count($redisServers) > 1; + + if ($cluster) { + $redisConfig['clusters'] = ['default' => []]; + } + foreach ($redisServers as $index => $redisServer) { - $redisServerName = ($index === 0) ? 'default' : 'redis-server-' . $index; $redisServerDetails = explode(':', $redisServer); - if (count($redisServerDetails) < 2) $redisServerDetails[] = '6379'; - if (count($redisServerDetails) < 3) $redisServerDetails[] = '0'; - $redisConfig[$redisServerName] = array_combine($redisServerKeys, $redisServerDetails); + + $serverConfig = []; + $configIndex = 0; + foreach ($redisDefaults as $configKey => $configDefault) { + $serverConfig[$configKey] = ($redisServerDetails[$configIndex] ?? $configDefault); + $configIndex++; + } + + if ($cluster) { + $redisConfig['clusters']['default'][] = $serverConfig; + } else { + $redisConfig['default'] = $serverConfig; + } } } -// MYSQL - Split out port from host if set +// MYSQL +// Split out port from host if set $mysql_host = env('DB_HOST', 'localhost'); $mysql_host_exploded = explode(':', $mysql_host); $mysql_port = env('DB_PORT', 3306); From 6be95cd2ac0232c8dd290bbcd5d268eb1f72154d Mon Sep 17 00:00:00 2001 From: Dan Brown Date: Fri, 8 Mar 2019 22:57:24 +0000 Subject: [PATCH 17/23] Re-centered dropzone error arrow --- resources/assets/sass/_components.scss | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/resources/assets/sass/_components.scss b/resources/assets/sass/_components.scss index 7604705f7..1f34166c6 100644 --- a/resources/assets/sass/_components.scss +++ b/resources/assets/sass/_components.scss @@ -534,7 +534,7 @@ body.flexbox-support #entity-selector-wrap .popup-body .form-group { content: ''; position: absolute; top: -6px; - left: 52px; + left: 44px; width: 0; height: 0; border-left: 6px solid transparent; From 55b07c7076d3296116714470ea53d75f69cb4387 Mon Sep 17 00:00:00 2001 From: Daniel Fanara Date: Fri, 8 Mar 2019 23:55:11 -0500 Subject: [PATCH 18/23] Issue #1306 - Specify display name attribute from LDAP --- app/Auth/Access/LdapService.php | 6 ++++-- config/services.php | 1 + 2 files changed, 5 insertions(+), 2 deletions(-) diff --git a/app/Auth/Access/LdapService.php b/app/Auth/Access/LdapService.php index 654ea2f99..c8548b98a 100644 --- a/app/Auth/Access/LdapService.php +++ b/app/Auth/Access/LdapService.php @@ -80,7 +80,9 @@ class LdapService public function getUserDetails($userName) { $emailAttr = $this->config['email_attribute']; - $user = $this->getUserWithAttributes($userName, ['cn', 'uid', 'dn', $emailAttr]); + $displayNameAttr = $this->config['display_name_attribute']; + + $user = $this->getUserWithAttributes($userName, ['cn', 'uid', 'dn', $emailAttr, $displayNameAttr]); if ($user === null) { return null; @@ -88,7 +90,7 @@ class LdapService return [ 'uid' => (isset($user['uid'])) ? $user['uid'][0] : $user['dn'], - 'name' => $user['cn'][0], + 'name' => (isset($uset[$displayNameAttr])) ? (is_array($user[$displayNameAttr]) ? $user[$displayNameAttr][0] : $user[$displayNameAttr]) : $user['cn'][0], 'dn' => $user['dn'], 'email' => (isset($user[$emailAttr])) ? (is_array($user[$emailAttr]) ? $user[$emailAttr][0] : $user[$emailAttr]) : null ]; diff --git a/config/services.php b/config/services.php index f713f9d38..97cb71ddc 100644 --- a/config/services.php +++ b/config/services.php @@ -141,6 +141,7 @@ return [ 'user_filter' => env('LDAP_USER_FILTER', '(&(uid=${user}))'), 'version' => env('LDAP_VERSION', false), 'email_attribute' => env('LDAP_EMAIL_ATTRIBUTE', 'mail'), + 'display_name_attribute' => env('LDAP_DISPLAY_NAME_ATTRIBUTE', 'cn'), 'follow_referrals' => env('LDAP_FOLLOW_REFERRALS', false), 'user_to_groups' => env('LDAP_USER_TO_GROUPS',false), 'group_attribute' => env('LDAP_GROUP_ATTRIBUTE', 'memberOf'), From 502ea608bf2387a0bc928eb1cb521809920309c2 Mon Sep 17 00:00:00 2001 From: Daniel Fanara Date: Sat, 9 Mar 2019 01:08:49 -0500 Subject: [PATCH 19/23] Issue #1306 - Unit Tests for LdapService Changes --- .env.example.complete | 1 + app/Auth/Access/LdapService.php | 2 +- tests/Auth/LdapTest.php | 76 +++++++++++++++++++++++++++++++++ 3 files changed, 78 insertions(+), 1 deletion(-) diff --git a/.env.example.complete b/.env.example.complete index 77a0508dc..911d924df 100644 --- a/.env.example.complete +++ b/.env.example.complete @@ -177,6 +177,7 @@ LDAP_USER_FILTER=false LDAP_VERSION=false LDAP_TLS_INSECURE=false LDAP_EMAIL_ATTRIBUTE=mail +LDAP_DISPLAY_NAME_ATTRIBUTE=cn LDAP_FOLLOW_REFERRALS=true # LDAP group sync configuration diff --git a/app/Auth/Access/LdapService.php b/app/Auth/Access/LdapService.php index c8548b98a..c48a72f98 100644 --- a/app/Auth/Access/LdapService.php +++ b/app/Auth/Access/LdapService.php @@ -90,7 +90,7 @@ class LdapService return [ 'uid' => (isset($user['uid'])) ? $user['uid'][0] : $user['dn'], - 'name' => (isset($uset[$displayNameAttr])) ? (is_array($user[$displayNameAttr]) ? $user[$displayNameAttr][0] : $user[$displayNameAttr]) : $user['cn'][0], + 'name' => (isset($user[$displayNameAttr])) ? (is_array($user[$displayNameAttr]) ? $user[$displayNameAttr][0] : $user[$displayNameAttr]) : $user['cn'][0], 'dn' => $user['dn'], 'email' => (isset($user[$emailAttr])) ? (is_array($user[$emailAttr]) ? $user[$emailAttr][0] : $user[$emailAttr]) : null ]; diff --git a/tests/Auth/LdapTest.php b/tests/Auth/LdapTest.php index 16ba11358..d23c9ec84 100644 --- a/tests/Auth/LdapTest.php +++ b/tests/Auth/LdapTest.php @@ -23,6 +23,7 @@ class LdapTest extends BrowserKitTest 'auth.method' => 'ldap', 'services.ldap.base_dn' => 'dc=ldap,dc=local', 'services.ldap.email_attribute' => 'mail', + 'services.ldap.display_name_attribute' => 'cn', 'services.ldap.user_to_groups' => false, 'auth.providers.users.driver' => 'ldap', ]); @@ -372,4 +373,79 @@ class LdapTest extends BrowserKitTest ]); } + public function test_login_uses_specified_display_name_attribute() + { + app('config')->set([ + 'services.ldap.display_name_attribute' => 'displayName' + ]); + + $this->mockLdap->shouldReceive('connect')->once()->andReturn($this->resourceId); + $this->mockLdap->shouldReceive('setVersion')->once(); + $this->mockLdap->shouldReceive('setOption')->times(4); + $this->mockLdap->shouldReceive('searchAndGetEntries')->times(4) + ->with($this->resourceId, config('services.ldap.base_dn'), \Mockery::type('string'), \Mockery::type('array')) + ->andReturn(['count' => 1, 0 => [ + 'uid' => [$this->mockUser->name], + 'cn' => [$this->mockUser->name], + 'dn' => ['dc=test' . config('services.ldap.base_dn')], + 'displayName' => 'displayNameAttribute' + ]]); + $this->mockLdap->shouldReceive('bind')->times(6)->andReturn(true); + $this->mockEscapes(4); + + $this->visit('/login') + ->see('Username') + ->type($this->mockUser->name, '#username') + ->type($this->mockUser->password, '#password') + ->press('Log In') + ->seePageIs('/login')->see('Please enter an email to use for this account.'); + + $this->type($this->mockUser->email, '#email') + ->press('Log In') + ->seePageIs('/') + ->see('displayNameAttribute') + ->seeInDatabase('users', ['email' => $this->mockUser->email, 'email_confirmed' => false, 'external_auth_id' => $this->mockUser->name, 'name' => 'displayNameAttribute']); + + app('config')->set([ + 'services.ldap.display_name_attribute' => 'cn' + ]); + } + + public function test_login_uses_default_display_name_attribute_if_specified_not_present() + { + app('config')->set([ + 'services.ldap.display_name_attribute' => 'displayName' + ]); + + $this->mockLdap->shouldReceive('connect')->once()->andReturn($this->resourceId); + $this->mockLdap->shouldReceive('setVersion')->once(); + $this->mockLdap->shouldReceive('setOption')->times(4); + $this->mockLdap->shouldReceive('searchAndGetEntries')->times(4) + ->with($this->resourceId, config('services.ldap.base_dn'), \Mockery::type('string'), \Mockery::type('array')) + ->andReturn(['count' => 1, 0 => [ + 'uid' => [$this->mockUser->name], + 'cn' => [$this->mockUser->name], + 'dn' => ['dc=test' . config('services.ldap.base_dn')] + ]]); + $this->mockLdap->shouldReceive('bind')->times(6)->andReturn(true); + $this->mockEscapes(4); + + $this->visit('/login') + ->see('Username') + ->type($this->mockUser->name, '#username') + ->type($this->mockUser->password, '#password') + ->press('Log In') + ->seePageIs('/login')->see('Please enter an email to use for this account.'); + + $this->type($this->mockUser->email, '#email') + ->press('Log In') + ->seePageIs('/') + ->see($this->mockUser->name) + ->seeInDatabase('users', ['email' => $this->mockUser->email, 'email_confirmed' => false, 'external_auth_id' => $this->mockUser->name, 'name' => $this->mockUser->name]); + + app('config')->set([ + 'services.ldap.display_name_attribute' => 'cn' + ]); + } + } From 6d20bdc1fbb4da20e7f5c40c0beafcafc296a18d Mon Sep 17 00:00:00 2001 From: Daniel Fanara Date: Sat, 9 Mar 2019 01:13:30 -0500 Subject: [PATCH 20/23] Preserve original display_name_attribute configuration values. --- tests/Auth/LdapTest.php | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/tests/Auth/LdapTest.php b/tests/Auth/LdapTest.php index d23c9ec84..54cf84266 100644 --- a/tests/Auth/LdapTest.php +++ b/tests/Auth/LdapTest.php @@ -375,6 +375,7 @@ class LdapTest extends BrowserKitTest public function test_login_uses_specified_display_name_attribute() { + $originalAttribute = config('services.ldap.display_name_attribute'); app('config')->set([ 'services.ldap.display_name_attribute' => 'displayName' ]); @@ -407,12 +408,13 @@ class LdapTest extends BrowserKitTest ->seeInDatabase('users', ['email' => $this->mockUser->email, 'email_confirmed' => false, 'external_auth_id' => $this->mockUser->name, 'name' => 'displayNameAttribute']); app('config')->set([ - 'services.ldap.display_name_attribute' => 'cn' + 'services.ldap.display_name_attribute' => $originalAttribute ]); } public function test_login_uses_default_display_name_attribute_if_specified_not_present() { + $originalAttribute = config('services.ldap.display_name_attribute'); app('config')->set([ 'services.ldap.display_name_attribute' => 'displayName' ]); @@ -444,7 +446,7 @@ class LdapTest extends BrowserKitTest ->seeInDatabase('users', ['email' => $this->mockUser->email, 'email_confirmed' => false, 'external_auth_id' => $this->mockUser->name, 'name' => $this->mockUser->name]); app('config')->set([ - 'services.ldap.display_name_attribute' => 'cn' + 'services.ldap.display_name_attribute' => $originalAttribute ]); } From 5c9b528517f1cecd1f9277e87a6036c330e86bfe Mon Sep 17 00:00:00 2001 From: Dan Brown Date: Sat, 9 Mar 2019 16:50:22 +0000 Subject: [PATCH 21/23] Abstracted userCanCreatePage helper to work for any permisison - Added test to cover scenario where someone with create-own permission would want to copy a viewable item into a container entity that they own. --- app/Auth/Permissions/PermissionService.php | 13 ++++----- app/helpers.php | 18 ++++++------ resources/views/pages/show.blade.php | 6 ++-- tests/Entity/SortTest.php | 32 ++++++++++++++++++++++ 4 files changed, 51 insertions(+), 18 deletions(-) diff --git a/app/Auth/Permissions/PermissionService.php b/app/Auth/Permissions/PermissionService.php index b28f59cc5..33d214963 100644 --- a/app/Auth/Permissions/PermissionService.php +++ b/app/Auth/Permissions/PermissionService.php @@ -557,19 +557,17 @@ class PermissionService } /** - * Checks if a user has a book or chapter available to create a page - * @param Ownable $ownable - * @param $permission + * Checks if a user has the given permission for any items in the system. + * @param string $permission * @return bool */ - public function checkAvailableCreatePageAccess() + public function checkUserHasPermissionOnAnything(string $permission) { - $userRoleIds = $this->currentUser()->roles()->pluck('id')->toArray(); + $userRoleIds = $this->currentUser()->roles()->select('id')->pluck('id')->toArray(); $userId = $this->currentUser()->id; - $canCreatePage = $this->db->table('joint_permissions') - ->where('action', '=', 'page-create') + ->where('action', '=', $permission) ->whereIn('role_id', $userRoleIds) ->where(function ($query) use ($userId) { $query->where('has_permission', '=', 1) @@ -580,6 +578,7 @@ class PermissionService }) ->get()->count() > 0; + $this->clean(); return $canCreatePage; } diff --git a/app/helpers.php b/app/helpers.php index 383e25c15..0825a2e4a 100644 --- a/app/helpers.php +++ b/app/helpers.php @@ -1,5 +1,6 @@ can($permission); } // Check permission on ownable item - $permissionService = app(\BookStack\Auth\Permissions\PermissionService::class); + $permissionService = app(PermissionService::class); return $permissionService->checkOwnableUserAccess($ownable, $permission); } /** - * Check if the current user has the ability to create a page for an existing object + * Check if the current user has the given permission + * on any item in the system. + * @param string $permission * @return bool */ -function userCanCreatePage() +function userCanOnAny(string $permission) { - // Check for create page permissions - $permissionService = app(\BookStack\Auth\Permissions\PermissionService::class); - return $permissionService->checkAvailableCreatePageAccess(); + $permissionService = app(PermissionService::class); + return $permissionService->checkUserHasPermissionOnAnything($permission); } /** diff --git a/resources/views/pages/show.blade.php b/resources/views/pages/show.blade.php index 6f221edee..db2f1462e 100644 --- a/resources/views/pages/show.blade.php +++ b/resources/views/pages/show.blade.php @@ -17,14 +17,14 @@ @if(userCan('page-update', $page)) @icon('edit'){{ trans('common.edit') }} @endif - @if((userCan('page-view', $page) && userCanCreatePage()) || userCan('page-update', $page) || userCan('restrictions-manage', $page) || userCan('page-delete', $page)) + @if((userCan('page-view', $page) && userCanOnAny('page-create')) || userCan('page-update', $page) || userCan('restrictions-manage', $page) || userCan('page-delete', $page))
@icon('more') {{ trans('common.more') }}
    - @if(userCanCreatePage()) + @if(userCanOnAny('page-create'))
  • @icon('copy'){{ trans('common.copy') }}
    • @endif - @if(userCan('page-delete', $page) && userCan('page-update, $page)) + @if(userCan('page-delete', $page) && userCan('page-update', $page))
  • @icon('folder'){{ trans('common.move') }}
    • @endif @if(userCan('page-update', $page)) diff --git a/tests/Entity/SortTest.php b/tests/Entity/SortTest.php index 11294f7df..a3c20e84c 100644 --- a/tests/Entity/SortTest.php +++ b/tests/Entity/SortTest.php @@ -1,5 +1,6 @@ assertTrue($pageCopy->id !== $page->id, 'Page copy is not the same instance'); } + public function test_page_can_be_copied_without_edit_permission() + { + $page = Page::first(); + $currentBook = $page->book; + $newBook = Book::where('id', '!=', $currentBook->id)->first(); + $viewer = $this->getViewer(); + + $resp = $this->actingAs($viewer)->get($page->getUrl()); + $resp->assertDontSee($page->getUrl('/copy')); + + $newBook->created_by = $viewer->id; + $newBook->save(); + $this->giveUserPermissions($viewer, ['page-create-own']); + $this->regenEntityPermissions($newBook); + + $resp = $this->actingAs($viewer)->get($page->getUrl()); + $resp->assertSee($page->getUrl('/copy')); + + $movePageResp = $this->post($page->getUrl('/copy'), [ + 'entity_selection' => 'book:' . $newBook->id, + 'name' => 'My copied test page' + ]); + $movePageResp->assertRedirect(); + + $this->assertDatabaseHas('pages', [ + 'name' => 'My copied test page', + 'created_by' => $viewer->id, + 'book_id' => $newBook->id, + ]); + } + } \ No newline at end of file From 042a6f9760337d28138d4c1e4c7ab570bce8804c Mon Sep 17 00:00:00 2001 From: Dan Brown Date: Sat, 9 Mar 2019 21:15:45 +0000 Subject: [PATCH 22/23] Updated shelf menu item to show on custom permission - Extended new 'userCanOnAny' helper to take a entity class for filtering. Closes #1201 --- app/Auth/Permissions/PermissionService.php | 25 ++++++++++++++-------- app/helpers.php | 6 ++++-- resources/views/base.blade.php | 2 +- tests/Entity/BookShelfTest.php | 18 ++++++++++++++++ 4 files changed, 39 insertions(+), 12 deletions(-) diff --git a/app/Auth/Permissions/PermissionService.php b/app/Auth/Permissions/PermissionService.php index 33d214963..8fc70e916 100644 --- a/app/Auth/Permissions/PermissionService.php +++ b/app/Auth/Permissions/PermissionService.php @@ -558,28 +558,35 @@ class PermissionService /** * Checks if a user has the given permission for any items in the system. + * Can be passed an entity instance to filter on a specific type. * @param string $permission + * @param string $entityClass * @return bool */ - public function checkUserHasPermissionOnAnything(string $permission) + public function checkUserHasPermissionOnAnything(string $permission, string $entityClass = null) { $userRoleIds = $this->currentUser()->roles()->select('id')->pluck('id')->toArray(); $userId = $this->currentUser()->id; - $canCreatePage = $this->db->table('joint_permissions') + $permissionQuery = $this->db->table('joint_permissions') ->where('action', '=', $permission) ->whereIn('role_id', $userRoleIds) ->where(function ($query) use ($userId) { $query->where('has_permission', '=', 1) - ->orWhere(function ($query2) use ($userId) { - $query2->where('has_permission_own', '=', 1) - ->where('created_by', '=', $userId); - }); - }) - ->get()->count() > 0; + ->orWhere(function ($query2) use ($userId) { + $query2->where('has_permission_own', '=', 1) + ->where('created_by', '=', $userId); + }); + }) ; + if (!is_null($entityClass)) { + $entityInstance = app()->make($entityClass); + $permissionQuery = $permissionQuery->where('entity_type', '=', $entityInstance->getMorphClass()); + } + + $hasPermission = $permissionQuery->count() > 0; $this->clean(); - return $canCreatePage; + return $hasPermission; } /** diff --git a/app/helpers.php b/app/helpers.php index 0825a2e4a..d9533645d 100644 --- a/app/helpers.php +++ b/app/helpers.php @@ -1,6 +1,7 @@ checkUserHasPermissionOnAnything($permission); + return $permissionService->checkUserHasPermissionOnAnything($permission, $entityClass); } /** diff --git a/resources/views/base.blade.php b/resources/views/base.blade.php index c7a5acca8..fdd248091 100644 --- a/resources/views/base.blade.php +++ b/resources/views/base.blade.php @@ -48,7 +48,7 @@
- @if(userCan('bookshelf-view-all') || userCan('bookshelf-view-own')) + @if(userCanOnAny('view', \BookStack\Entities\Bookshelf::class) || userCan('bookshelf-view-own')) @icon('bookshelf'){{ trans('entities.shelves') }} @endif @icon('book'){{ trans('entities.books') }} diff --git a/tests/Entity/BookShelfTest.php b/tests/Entity/BookShelfTest.php index 5d71ec6f6..bdba812d5 100644 --- a/tests/Entity/BookShelfTest.php +++ b/tests/Entity/BookShelfTest.php @@ -1,5 +1,7 @@ assertElementContains('header', 'Shelves'); } + public function test_shelves_shows_in_header_if_have_any_shelve_view_permission() + { + $user = factory(User::class)->create(); + $this->giveUserPermissions($user, ['image-create-all']); + $shelf = Bookshelf::first(); + $userRole = $user->roles()->first(); + + $resp = $this->actingAs($user)->get('/'); + $resp->assertElementNotContains('header', 'Shelves'); + + $this->setEntityRestrictions($shelf, ['view'], [$userRole]); + + $resp = $this->get('/'); + $resp->assertElementContains('header', 'Shelves'); + } + public function test_shelves_page_contains_create_link() { $resp = $this->asEditor()->get('/shelves'); From 44c537de1a37aeef8e55ac3931875dee48bc512e Mon Sep 17 00:00:00 2001 From: Dan Brown Date: Sun, 10 Mar 2019 10:54:19 +0000 Subject: [PATCH 23/23] Performed some LDAP service/test cleanup --- app/Auth/Access/LdapService.php | 24 ++++++++-- tests/Auth/LdapTest.php | 77 ++++++++------------------------- 2 files changed, 39 insertions(+), 62 deletions(-) diff --git a/app/Auth/Access/LdapService.php b/app/Auth/Access/LdapService.php index c48a72f98..9ffbbfbb7 100644 --- a/app/Auth/Access/LdapService.php +++ b/app/Auth/Access/LdapService.php @@ -88,14 +88,32 @@ class LdapService return null; } + $userCn = $this->getUserResponseProperty($user, 'cn', null); return [ - 'uid' => (isset($user['uid'])) ? $user['uid'][0] : $user['dn'], - 'name' => (isset($user[$displayNameAttr])) ? (is_array($user[$displayNameAttr]) ? $user[$displayNameAttr][0] : $user[$displayNameAttr]) : $user['cn'][0], + 'uid' => $this->getUserResponseProperty($user, 'uid', $user['dn']), + 'name' => $this->getUserResponseProperty($user, $displayNameAttr, $userCn), 'dn' => $user['dn'], - 'email' => (isset($user[$emailAttr])) ? (is_array($user[$emailAttr]) ? $user[$emailAttr][0] : $user[$emailAttr]) : null + 'email' => $this->getUserResponseProperty($user, $emailAttr, null), ]; } + /** + * Get a property from an LDAP user response fetch. + * Handles properties potentially being part of an array. + * @param array $userDetails + * @param string $propertyKey + * @param $defaultValue + * @return mixed + */ + protected function getUserResponseProperty(array $userDetails, string $propertyKey, $defaultValue) + { + if (isset($userDetails[$propertyKey])) { + return (is_array($userDetails[$propertyKey]) ? $userDetails[$propertyKey][0] : $userDetails[$propertyKey]); + } + + return $defaultValue; + } + /** * @param Authenticatable $user * @param string $username diff --git a/tests/Auth/LdapTest.php b/tests/Auth/LdapTest.php index 54cf84266..5ccb1415e 100644 --- a/tests/Auth/LdapTest.php +++ b/tests/Auth/LdapTest.php @@ -46,6 +46,15 @@ class LdapTest extends BrowserKitTest }); } + protected function mockUserLogin() + { + return $this->visit('/login') + ->see('Username') + ->type($this->mockUser->name, '#username') + ->type($this->mockUser->password, '#password') + ->press('Log In'); + } + public function test_login() { $this->mockLdap->shouldReceive('connect')->once()->andReturn($this->resourceId); @@ -61,11 +70,7 @@ class LdapTest extends BrowserKitTest $this->mockLdap->shouldReceive('bind')->times(6)->andReturn(true); $this->mockEscapes(4); - $this->visit('/login') - ->see('Username') - ->type($this->mockUser->name, '#username') - ->type($this->mockUser->password, '#password') - ->press('Log In') + $this->mockUserLogin() ->seePageIs('/login')->see('Please enter an email to use for this account.'); $this->type($this->mockUser->email, '#email') @@ -91,11 +96,7 @@ class LdapTest extends BrowserKitTest $this->mockLdap->shouldReceive('bind')->times(3)->andReturn(true); $this->mockEscapes(2); - $this->visit('/login') - ->see('Username') - ->type($this->mockUser->name, '#username') - ->type($this->mockUser->password, '#password') - ->press('Log In') + $this->mockUserLogin() ->seePageIs('/') ->see($this->mockUser->name) ->seeInDatabase('users', ['email' => $this->mockUser->email, 'email_confirmed' => false, 'external_auth_id' => $ldapDn]); @@ -116,11 +117,7 @@ class LdapTest extends BrowserKitTest $this->mockLdap->shouldReceive('bind')->times(3)->andReturn(true, true, false); $this->mockEscapes(2); - $this->visit('/login') - ->see('Username') - ->type($this->mockUser->name, '#username') - ->type($this->mockUser->password, '#password') - ->press('Log In') + $this->mockUserLogin() ->seePageIs('/login')->see('These credentials do not match our records.') ->dontSeeInDatabase('users', ['external_auth_id' => $this->mockUser->name]); } @@ -197,12 +194,7 @@ class LdapTest extends BrowserKitTest $this->mockEscapes(5); $this->mockExplodes(6); - $this->visit('/login') - ->see('Username') - ->type($this->mockUser->name, '#username') - ->type($this->mockUser->password, '#password') - ->press('Log In') - ->seePageIs('/'); + $this->mockUserLogin()->seePageIs('/'); $user = User::where('email', $this->mockUser->email)->first(); $this->seeInDatabase('role_user', [ @@ -250,12 +242,7 @@ class LdapTest extends BrowserKitTest $this->mockEscapes(4); $this->mockExplodes(2); - $this->visit('/login') - ->see('Username') - ->type($this->mockUser->name, '#username') - ->type($this->mockUser->password, '#password') - ->press('Log In') - ->seePageIs('/'); + $this->mockUserLogin()->seePageIs('/'); $user = User::where('email', $this->mockUser->email)->first(); $this->seeInDatabase('role_user', [ @@ -304,12 +291,7 @@ class LdapTest extends BrowserKitTest $this->mockEscapes(4); $this->mockExplodes(2); - $this->visit('/login') - ->see('Username') - ->type($this->mockUser->name, '#username') - ->type($this->mockUser->password, '#password') - ->press('Log In') - ->seePageIs('/'); + $this->mockUserLogin()->seePageIs('/'); $user = User::where('email', $this->mockUser->email)->first(); $this->seeInDatabase('role_user', [ @@ -355,12 +337,7 @@ class LdapTest extends BrowserKitTest $this->mockEscapes(5); $this->mockExplodes(6); - $this->visit('/login') - ->see('Username') - ->type($this->mockUser->name, '#username') - ->type($this->mockUser->password, '#password') - ->press('Log In') - ->seePageIs('/'); + $this->mockUserLogin()->seePageIs('/'); $user = User::where('email', $this->mockUser->email)->first(); $this->seeInDatabase('role_user', [ @@ -375,7 +352,6 @@ class LdapTest extends BrowserKitTest public function test_login_uses_specified_display_name_attribute() { - $originalAttribute = config('services.ldap.display_name_attribute'); app('config')->set([ 'services.ldap.display_name_attribute' => 'displayName' ]); @@ -394,11 +370,7 @@ class LdapTest extends BrowserKitTest $this->mockLdap->shouldReceive('bind')->times(6)->andReturn(true); $this->mockEscapes(4); - $this->visit('/login') - ->see('Username') - ->type($this->mockUser->name, '#username') - ->type($this->mockUser->password, '#password') - ->press('Log In') + $this->mockUserLogin() ->seePageIs('/login')->see('Please enter an email to use for this account.'); $this->type($this->mockUser->email, '#email') @@ -406,15 +378,10 @@ class LdapTest extends BrowserKitTest ->seePageIs('/') ->see('displayNameAttribute') ->seeInDatabase('users', ['email' => $this->mockUser->email, 'email_confirmed' => false, 'external_auth_id' => $this->mockUser->name, 'name' => 'displayNameAttribute']); - - app('config')->set([ - 'services.ldap.display_name_attribute' => $originalAttribute - ]); } public function test_login_uses_default_display_name_attribute_if_specified_not_present() { - $originalAttribute = config('services.ldap.display_name_attribute'); app('config')->set([ 'services.ldap.display_name_attribute' => 'displayName' ]); @@ -432,11 +399,7 @@ class LdapTest extends BrowserKitTest $this->mockLdap->shouldReceive('bind')->times(6)->andReturn(true); $this->mockEscapes(4); - $this->visit('/login') - ->see('Username') - ->type($this->mockUser->name, '#username') - ->type($this->mockUser->password, '#password') - ->press('Log In') + $this->mockUserLogin() ->seePageIs('/login')->see('Please enter an email to use for this account.'); $this->type($this->mockUser->email, '#email') @@ -444,10 +407,6 @@ class LdapTest extends BrowserKitTest ->seePageIs('/') ->see($this->mockUser->name) ->seeInDatabase('users', ['email' => $this->mockUser->email, 'email_confirmed' => false, 'external_auth_id' => $this->mockUser->name, 'name' => $this->mockUser->name]); - - app('config')->set([ - 'services.ldap.display_name_attribute' => $originalAttribute - ]); } }