From 154b3accf99a378d85b96b13c9651e3c8bc49ff3 Mon Sep 17 00:00:00 2001 From: Mantas Masalskis Date: Thu, 24 Feb 2022 12:15:19 +0100 Subject: [PATCH] Fixes #2603 - Limit access to KB Categories based on roles. --- .../knowledge_base/agent_controller.coffee | 75 ++++++-- .../content_can_be_published_form.coffee | 2 +- .../knowledge_base/editor_coordinator.coffee | 6 + .../knowledge_base/navigation.coffee | 6 +- .../knowledge_base/permissions_dialog.coffee | 92 +++++++++ .../knowledge_base/reader_controller.coffee | 14 +- .../reader_list_container.coffee | 8 +- .../reader_list_controller.coffee | 17 +- .../knowledge_base/reader_list_item.coffee | 5 +- .../controllers/knowledge_base/sidebar.coffee | 23 ++- .../sidebar/_generic_list.coffee | 6 + .../knowledge_base/sidebar/attachments.coffee | 4 +- .../sidebar/linked_tickets.coffee | 4 +- .../knowledge_base/sidebar/tags.coffee | 3 + .../lib/mixins/knowledge_base_access.coffee | 33 ++++ .../lib/mixins/knowledge_base_actions.coffee | 9 + .../mixins/knowledge_base_translatable.coffee | 18 +- .../app/models/knowledge_base.coffee | 38 +++- .../app/models/knowledge_base_answer.coffee | 3 +- .../app/models/knowledge_base_category.coffee | 3 + .../knowledge_base_category_permission.coffee | 2 + .../app/views/knowledge_base/base_form.coffee | 1 - .../knowledge_base/permissions_dialog.jst.eco | 33 ++++ app/assets/stylesheets/zammad.scss | 7 +- .../knowledge_base/permissions_controller.rb | 45 +++++ .../public/answers_controller.rb | 4 +- .../knowledge_base/public/base_controller.rb | 46 +++-- .../public/categories_controller.rb | 7 +- .../knowledge_base/public/tags_controller.rb | 9 +- app/controllers/knowledge_bases_controller.rb | 30 ++- app/helpers/knowledge_base_top_bar_helper.rb | 2 +- .../knowledge_base_visibility_class_helper.rb | 2 +- .../knowledge_base_visibility_note_helper.rb | 2 +- app/jobs/checks_kb_client_notification_job.rb | 14 +- app/jobs/checks_kb_client_visibility_job.rb | 14 ++ .../concerns/checks_kb_client_notification.rb | 25 +-- .../concerns/checks_kb_client_visibility.rb | 17 ++ app/models/knowledge_base.rb | 23 +++ app/models/knowledge_base/answer.rb | 9 +- app/models/knowledge_base/category.rb | 42 ++-- app/models/knowledge_base/permission.rb | 16 ++ app/models/role.rb | 32 +++- .../answers_controller_policy.rb | 31 ++- .../categories_controller_policy.rb | 34 +++- .../knowledge_bases_controller_policy.rb | 14 ++ app/policies/knowledge_base/answer_policy.rb | 32 +++- .../knowledge_base/category_policy.rb | 42 +++- app/policies/knowledge_base_policy.rb | 22 ++- config/routes/knowledge_base.rb | 7 + ...0190531180304_initialize_knowledge_base.rb | 21 +- ...63819_create_knowledge_base_permissions.rb | 22 +++ db/seeds/permissions.rb | 9 +- i18n/zammad.pot | 17 +- .../calculations/pluck_as_hash.rb | 21 ++ lib/knowledge_base/category/permission.rb | 33 ++++ lib/knowledge_base/effective_permission.rb | 58 ++++++ lib/knowledge_base/internal_assets.rb | 120 ++++++++++++ lib/knowledge_base/permissions_update.rb | 97 ++++++++++ spec/factories/knowledge_base_permissions.rb | 9 + spec/factories/role.rb | 6 + .../category/permission_spec.rb | 73 +++++++ .../effective_permission_spec.rb | 91 +++++++++ .../knowledge_base/internal_assets_spec.rb | 87 +++++++++ .../knowledge_base/permissions_update_spec.rb | 180 ++++++++++++++++++ spec/models/knowledge_base/category_spec.rb | 1 + spec/models/knowledge_base/permission_spec.rb | 50 +++++ .../knowledge_base/answer_policy_spec.rb | 64 +++++++ .../knowledge_base/category_policy_spec.rb | 45 +++++ .../knowledge_base_policy_examples.rb | 29 +++ spec/policies/knowledge_base_policy_spec.rb | 19 ++ spec/support/assets_matchers.rb | 4 +- .../knowledge_base/locale/answer/edit_spec.rb | 25 +-- .../locale/category/permissions_spec.rb | 177 +++++++++++++++++ .../locale/knowledge_base/permissions_spec.rb | 118 ++++++++++++ spec/system/ticket/zoom_spec.rb | 1 - 75 files changed, 2123 insertions(+), 187 deletions(-) create mode 100644 app/assets/javascripts/app/controllers/knowledge_base/permissions_dialog.coffee create mode 100644 app/assets/javascripts/app/lib/mixins/knowledge_base_access.coffee create mode 100644 app/assets/javascripts/app/models/knowledge_base_category_permission.coffee create mode 100644 app/assets/javascripts/app/views/knowledge_base/permissions_dialog.jst.eco create mode 100644 app/controllers/knowledge_base/permissions_controller.rb create mode 100644 app/jobs/checks_kb_client_visibility_job.rb create mode 100644 app/models/concerns/checks_kb_client_visibility.rb create mode 100644 app/models/knowledge_base/permission.rb create mode 100644 db/migrate/20210918163819_create_knowledge_base_permissions.rb create mode 100644 lib/knowledge_base/category/permission.rb create mode 100644 lib/knowledge_base/effective_permission.rb create mode 100644 lib/knowledge_base/internal_assets.rb create mode 100644 lib/knowledge_base/permissions_update.rb create mode 100644 spec/factories/knowledge_base_permissions.rb create mode 100644 spec/lib/knowledge_base/category/permission_spec.rb create mode 100644 spec/lib/knowledge_base/effective_permission_spec.rb create mode 100644 spec/lib/knowledge_base/internal_assets_spec.rb create mode 100644 spec/lib/knowledge_base/permissions_update_spec.rb create mode 100644 spec/models/knowledge_base/permission_spec.rb create mode 100644 spec/policies/knowledge_base/answer_policy_spec.rb create mode 100644 spec/policies/knowledge_base/category_policy_spec.rb create mode 100644 spec/policies/knowledge_base_policy_examples.rb create mode 100644 spec/policies/knowledge_base_policy_spec.rb create mode 100644 spec/system/knowledge_base/locale/category/permissions_spec.rb create mode 100644 spec/system/knowledge_base/locale/knowledge_base/permissions_spec.rb diff --git a/app/assets/javascripts/app/controllers/knowledge_base/agent_controller.coffee b/app/assets/javascripts/app/controllers/knowledge_base/agent_controller.coffee index 5440b8d6a..ef73b0d3a 100644 --- a/app/assets/javascripts/app/controllers/knowledge_base/agent_controller.coffee +++ b/app/assets/javascripts/app/controllers/knowledge_base/agent_controller.coffee @@ -11,6 +11,10 @@ class App.KnowledgeBaseAgentController extends App.Controller super @controllerBind('config_update_local', (data) => @configUpdated(data)) + @listenTo App.User.current(), 'refresh', @visibilityMayHaveChanged + + App.Event.bind 'kb_visibility_may_have_changed', @visibilityMayHaveChanged + if @permissionCheck('knowledge_base.*') and App.Config.get('kb_active') @updateNavMenu() else if App.Config.get('kb_active_publicly') @@ -51,15 +55,8 @@ class App.KnowledgeBaseAgentController extends App.Controller @loadChange(pushed_data) , 1000, key, 'kb_data_changed_loading') ) - @listenTo App.KnowledgeBase, 'kb_data_change_loaded', => - return if !@displayingError - - object = @constructor.pickObjectUsing(@lastParams, @) - - if !@objectVisibleInternally(object) - return - - @renderControllers(@lastParams) + @listenTo App.KnowledgeBase, 'kb_data_change_loaded', @renderAfterChangeLoaded + @listenTo App.KnowledgeBase, 'kb_visibility_change_loaded', @renderAfterChangeLoaded @checkForUpdates() @@ -112,6 +109,19 @@ class App.KnowledgeBaseAgentController extends App.Controller notifyChangeLoaded: -> App.KnowledgeBase.trigger('kb_data_change_loaded') + notifyVisibilityChangeLoaded: -> + App.KnowledgeBase.trigger('kb_visibility_change_loaded') + + renderAfterChangeLoaded: => + return if !@displayingError + + object = @constructor.pickObjectUsing(@lastParams, @) + + if !@objectVisibleInternally(object) + return + + @renderControllers(@lastParams) + active: (state) -> return @shown if state is undefined @shown = state @@ -138,6 +148,10 @@ class App.KnowledgeBaseAgentController extends App.Controller if !@permissionCheckRedirect("knowledge_base.#{@requiredPermissionSuffix(params)}") return + if @loaded && @requiredPermissionSuffix(params) == 'editor' && @access(params) != 'editor' + @navigate params.match[0].replace(/\/edit$/, ''), { hideCurrentLocationFromHistory: true } + return + if @loaded && @rendered && @lastParams && !params.knowledge_base_id && @contentController && @kb_locale()? @navigate @lastParams.match[0] , { hideCurrentLocationFromHistory: true } return @@ -169,6 +183,7 @@ class App.KnowledgeBaseAgentController extends App.Controller @pendingParams = params renderScreenErrorInContent: (text) -> + @contentController?.releaseController() @contentController = undefined @renderScreenError(detail: text, el: @$('.page-content')) @displayingError = true @@ -255,12 +270,12 @@ class App.KnowledgeBaseAgentController extends App.Controller kb.kb_locales().filter((elem) => elem.system_locale_id == @lastParams.selectedSystemLocale.id)[0] getKnowledgeBase: -> - App.KnowledgeBase.find(@lastParams.knowledge_base_id) + App.KnowledgeBase.find(@lastParams?.knowledge_base_id) fetchAndRender: => @fetch(true, true) - fetch: (showLoader, processLoaded) -> + fetch: (showLoader, processLoaded, notifyVisibilityChangeLoaded) -> if showLoader @startLoading() @@ -278,6 +293,9 @@ class App.KnowledgeBaseAgentController extends App.Controller if processLoaded @processLoaded() + + if notifyVisibilityChangeLoaded + @notifyVisibilityChangeLoaded() , error: (xhr) => if showLoader @@ -308,10 +326,11 @@ class App.KnowledgeBaseAgentController extends App.Controller App[elem.modelName].find(id)?.remove(clear: true) calculateIdsToDelete: (data) -> - Object - .keys(data) - .filter (elem) -> elem.match(/^KnowledgeBase/) + App.KnowledgeBase + .allKbModelNames() .map (model) -> + return {ids : []} if !data[model] + newIds = Object.keys data[model] oldIds = App[model].all().map (elem) -> elem.id diff = oldIds.filter (elem) -> !_.includes(newIds, String(elem)) @@ -339,8 +358,13 @@ class App.KnowledgeBaseAgentController extends App.Controller parentController: @ ) + access: (params) -> + @constructor + .pickObjectUsing(params, @) + ?.access() + isEditor: -> - App.User.current().permission('knowledge_base.editor') + @access(@lastParams) == 'editor' checkForUpdates: -> @interval(@checkUpdatesAction, 10 * 60 * 1000, 'kb_interval_check') @@ -371,6 +395,27 @@ class App.KnowledgeBaseAgentController extends App.Controller clicked: -> window.open(App.KnowledgeBase.first().publicBaseUrl(), '_blank') + visibilityMayHaveChanged: => + kb = @getKnowledgeBase() + + return if !kb + + @ajax + id: 'kb_pull_visibility' + type: 'GET' + url: @apiPath + '/knowledge_bases/visible_ids' + processData: true + success: (data, status, xhr) => + didRemove = kb.removeAssetsIfNeeded(data) + hasAssetsToLoad = kb.hasAssetsToLoad(data) + + if hasAssetsToLoad + @fetch(false, false, true) + return + + if didRemove + @notifyVisibilityChangeLoaded() + @pickObjectUsing: (params, parentController) -> kb = parentController.getKnowledgeBase() return if !kb diff --git a/app/assets/javascripts/app/controllers/knowledge_base/content_can_be_published_form.coffee b/app/assets/javascripts/app/controllers/knowledge_base/content_can_be_published_form.coffee index 553f221be..653e684c9 100644 --- a/app/assets/javascripts/app/controllers/knowledge_base/content_can_be_published_form.coffee +++ b/app/assets/javascripts/app/controllers/knowledge_base/content_can_be_published_form.coffee @@ -103,7 +103,7 @@ class App.KnowledgeBaseContentCanBePublishedForm extends App.ControllerForm , value: 'internal' name: __('Internal') - note: __('Visible to agents & editors') + note: __('Visible to readers & editors') , value: 'published' name: __('Public') diff --git a/app/assets/javascripts/app/controllers/knowledge_base/editor_coordinator.coffee b/app/assets/javascripts/app/controllers/knowledge_base/editor_coordinator.coffee index 4e8f6309e..ff8cfedbe 100644 --- a/app/assets/javascripts/app/controllers/knowledge_base/editor_coordinator.coffee +++ b/app/assets/javascripts/app/controllers/knowledge_base/editor_coordinator.coffee @@ -15,6 +15,12 @@ class App.KnowledgeBaseEditorCoordinator parentController: @parentController ) + clickedPermissions: (object) -> + new App.KnowledgeBasePermissionsDialog( + object: object + container: @parentController.el + ) + # built-in Spine's function doesn't work when object has no ID set and includes "undefined" in URL urlFor: (object) -> if object.id diff --git a/app/assets/javascripts/app/controllers/knowledge_base/navigation.coffee b/app/assets/javascripts/app/controllers/knowledge_base/navigation.coffee index 2825db9e9..9a1da8b3c 100644 --- a/app/assets/javascripts/app/controllers/knowledge_base/navigation.coffee +++ b/app/assets/javascripts/app/controllers/knowledge_base/navigation.coffee @@ -13,8 +13,8 @@ class App.KnowledgeBaseNavigation extends App.Controller @controllerBind('knowledge_base::navigation::rerender', => @needsUpdate()) - @listenTo App.KnowledgeBase, 'kb_data_change_loaded', => - @needsUpdate() + @listenTo App.KnowledgeBase, 'kb_data_change_loaded', @needsUpdate + @listenTo App.KnowledgeBase, 'kb_visibility_change_loaded', @needsUpdate buildCrumbsForRendering: (array, kb_locale, action) -> if action is 'search' @@ -73,7 +73,7 @@ class App.KnowledgeBaseNavigation extends App.Controller else false - needsUpdate: -> + needsUpdate: => @show(@savedParams, @savedAction) selectedLocaleDisplay: -> diff --git a/app/assets/javascripts/app/controllers/knowledge_base/permissions_dialog.coffee b/app/assets/javascripts/app/controllers/knowledge_base/permissions_dialog.coffee new file mode 100644 index 000000000..f17466340 --- /dev/null +++ b/app/assets/javascripts/app/controllers/knowledge_base/permissions_dialog.coffee @@ -0,0 +1,92 @@ +class App.KnowledgeBasePermissionsDialog extends App.ControllerModal + events: + #'submit form': 'submitPermissions' + 'click td.u-clickable': 'cellBackgroundClicked' + + head: 'Permissions' + includeForm: true + buttonSubmit: true + accessLevels: { editor: 'Editor', reader: 'Reader', none: 'None' } + + cellBackgroundClicked: (e) -> + return if e.target != e.currentTarget + + e.preventDefault() + e.currentTarget.querySelector('input')?.click() + + data: null + + constructor: (params) -> + super + + @load() + + content: => + return if !@data + + App.view('knowledge_base/permissions_dialog')( + accessLevels: @accessLevels + params: @loadedParams(@data) + roles: @formRoles(@data) + ) + + loadedParams: (data) -> + params = [] + + for permission in data.permissions + params[permission.role_id] = permission.access + + for role in data.roles_editor + params[role.id] ||= 'editor' + + for role in data.roles_reader + params[role.id] ||= 'reader' + + params + + formRoles: (data) -> + data.roles_editor.forEach (elem) => @formRolesItem(elem, 'editor', data) + data.roles_reader.forEach (elem) => @formRolesItem(elem, 'reader', data) + + _.sortBy data.roles_editor.concat(data.roles_reader), (elem) -> elem.name + + formRolesItem: (elem, role_name, data) -> + elem.accessLevel = role_name + elem.limit = _.findWhere(data.inherited, { role_id: elem.id })?.access + + load: => + @ajax( + id: 'knowledge_base_permissions_get' + type: 'get' + url: @object.generateURL('permissions') + processData: true + success: (data, status, xhr) => + @data = data + @update() + error: (xhr) => + @showAlert(xhr.responseJSON?.error || __('Unable to load changes')) + ) + + toggleDisabled: (state) => + @el.find('input, button').attr('disabled', state) + + onSubmit: (e) => + @clearAlerts() + @toggleDisabled(true) + + data = @formParams() + + params = { permissions_dialog: { permissions: data } } + + @ajax( + id: 'knowledge_base_permissions_patch' + type: 'PATCH' + data: JSON.stringify(params) + url: @object.generateURL('permissions') + processData: true + success: (data, status, xhr) => + @close() + error: (xhr) => + @toggleDisabled(false) + @showAlert(xhr.responseJSON?.error || __('Unable to save changes')) + ) diff --git a/app/assets/javascripts/app/controllers/knowledge_base/reader_controller.coffee b/app/assets/javascripts/app/controllers/knowledge_base/reader_controller.coffee index 29134002f..298334455 100644 --- a/app/assets/javascripts/app/controllers/knowledge_base/reader_controller.coffee +++ b/app/assets/javascripts/app/controllers/knowledge_base/reader_controller.coffee @@ -54,7 +54,10 @@ class App.KnowledgeBaseReaderController extends App.Controller @listenTo App.KnowledgeBase, 'kb_data_change_loaded', => @renderAnswer(@object, kb_locale) - renderAnswer: (answer, kb_locale) -> + @listenTo App.KnowledgeBase, 'kb_visibility_change_loaded', => + @renderAnswer(@object, kb_locale, true) + + renderAnswer: (answer, kb_locale, onlyVisibility) -> if !answer @parentController.renderNotFound() return @@ -63,13 +66,16 @@ class App.KnowledgeBaseReaderController extends App.Controller @parentController.renderNotAvailableAnymore() return + paginator = new App.KnowledgeBaseReaderPagination(object: @object, kb_locale: kb_locale) + @answerPagination.html paginator.el + + if onlyVisibility + return + @renderAttachments(answer.attachments) @renderTags(answer.tags) @renderLinkedTickets(answer.translation(kb_locale.id)?.linked_tickets()) - paginator = new App.KnowledgeBaseReaderPagination(object: @object, kb_locale: kb_locale) - @answerPagination.html paginator.el - answer_translation = answer.translation(kb_locale.id) if !answer_translation diff --git a/app/assets/javascripts/app/controllers/knowledge_base/reader_list_container.coffee b/app/assets/javascripts/app/controllers/knowledge_base/reader_list_container.coffee index a3eb47263..7ad2c842f 100644 --- a/app/assets/javascripts/app/controllers/knowledge_base/reader_list_container.coffee +++ b/app/assets/javascripts/app/controllers/knowledge_base/reader_list_container.coffee @@ -3,13 +3,13 @@ class App.KnowledgeBaseReaderListContainer extends App.Controller super @render() - @listenTo App.KnowledgeBase, 'kb_data_change_loaded', => - @parentRefreshed() + @listenTo App.KnowledgeBase, 'kb_data_change_loaded', @parentRefreshed + @listenTo App.KnowledgeBase, 'kb_visibility_change_loaded', @parentRefreshed tag: 'ul' className: 'sections' - parentRefreshed: -> + parentRefreshed: => newIds = @children().map (elem) -> elem.id oldIds = @el.children().toArray().map (elem) -> parseInt(elem.dataset.id) @@ -59,7 +59,9 @@ class App.KnowledgeBaseReaderListContainer.Categories extends App.KnowledgeBaseR @parent.children() else [] + # coffeelint: enable=indentation + # if !@isEditor items = items.filter (elem) => elem.visibleInternally(@kb_locale) diff --git a/app/assets/javascripts/app/controllers/knowledge_base/reader_list_controller.coffee b/app/assets/javascripts/app/controllers/knowledge_base/reader_list_controller.coffee index be3c4171b..e674c7ffc 100644 --- a/app/assets/javascripts/app/controllers/knowledge_base/reader_list_controller.coffee +++ b/app/assets/javascripts/app/controllers/knowledge_base/reader_list_controller.coffee @@ -3,9 +3,16 @@ class App.KnowledgeBaseReaderListController extends App.Controller super @render() - @listenTo App.KnowledgeBase, 'kb_data_change_loaded', => - if !@objectVisibleInternally() - @parentController.renderNotAvailableAnymore() + @listenTo App.KnowledgeBase, 'kb_data_change_loaded', @changeLoaded + @listenTo App.KnowledgeBase, 'kb_visibility_may_have_changed', => @changeLoaded(true) + + changeLoaded: => + if !@objectVisibleInternally() + @parentController.renderNotAvailableAnymore() + return + + if @renderEmptinessState != @object.isEmpty() + @render() elements: '.js-readerListContainer': 'container' @@ -18,11 +25,13 @@ class App.KnowledgeBaseReaderListController extends App.Controller @parentController.renderNotFound() return + @renderEmptinessState = @object.isEmpty() + if @object.isEmpty() @renderScreenPlaceholder( icon: App.Utils.icon('mood-ok') detail: __('This category is empty') - action: __('Start Editing') + action: if @parentController.isEditor() then __('Start Editing') actionCallback: => url = @object.uiUrl(@parentController.kb_locale(), 'edit') @navigate url diff --git a/app/assets/javascripts/app/controllers/knowledge_base/reader_list_item.coffee b/app/assets/javascripts/app/controllers/knowledge_base/reader_list_item.coffee index 595ccd1a8..cb1db0dc6 100644 --- a/app/assets/javascripts/app/controllers/knowledge_base/reader_list_item.coffee +++ b/app/assets/javascripts/app/controllers/knowledge_base/reader_list_item.coffee @@ -18,7 +18,10 @@ class App.KnowledgeBaseReaderListItem extends App.Controller @sort_order = @item.position - attrs = @item.attributesForRendering(@kb_locale, isEditor: @isEditor) + try + attrs = @item.attributesForRendering(@kb_locale, isEditor: @isEditor) + catch e + attrs = {} @el .prop('className') diff --git a/app/assets/javascripts/app/controllers/knowledge_base/sidebar.coffee b/app/assets/javascripts/app/controllers/knowledge_base/sidebar.coffee index dc7782a50..2766e1809 100644 --- a/app/assets/javascripts/app/controllers/knowledge_base/sidebar.coffee +++ b/app/assets/javascripts/app/controllers/knowledge_base/sidebar.coffee @@ -6,6 +6,7 @@ class App.KnowledgeBaseSidebar extends App.Controller constructor: -> super + @renderedWidgets = [] @show() @controllerBind 'knowledge_base::sidebar::rerender', => @rerender() @@ -14,9 +15,9 @@ class App.KnowledgeBaseSidebar extends App.Controller @rerender() true - rerender: -> + rerender: => @delay( => - @show(@savedParams, @savedAction) + @update() , 300, 'rerender') contentActionClicked: (e) -> @@ -24,10 +25,15 @@ class App.KnowledgeBaseSidebar extends App.Controller actionName = switch e.target.dataset.action when 'delete' then 'clickedDelete' when 'visibility' then 'clickedCanBePublished' + when 'permissions' then 'clickedPermissions' # coffeelint: enable=indentation @parentController.bodyModal = @parentController.coordinator[actionName]?(@savedParams) + update: => + for elem in @renderedWidgets + elem.updateIfNeeded?() + show: (object, action) -> isEdit = action is 'edit' @@ -39,17 +45,22 @@ class App.KnowledgeBaseSidebar extends App.Controller if !isEdit return - for widget in @widgets(object) - @el.append new widget( + @renderedWidgets = [] + + for elem in @getWidgets(object) + widget = new elem( object: object kb_locale: @parentController.kb_locale() parentController: @parentController - ).el + ) + + @renderedWidgets.push widget + @el.append widget.el hide: -> @el.addClass('hidden') - widgets: (object) -> + getWidgets: (object) -> output = [App.KnowledgeBaseSidebarActions] if object instanceof App.KnowledgeBase || object instanceof App.KnowledgeBaseCategory diff --git a/app/assets/javascripts/app/controllers/knowledge_base/sidebar/_generic_list.coffee b/app/assets/javascripts/app/controllers/knowledge_base/sidebar/_generic_list.coffee index 06271fd7e..e44fb5ad8 100644 --- a/app/assets/javascripts/app/controllers/knowledge_base/sidebar/_generic_list.coffee +++ b/app/assets/javascripts/app/controllers/knowledge_base/sidebar/_generic_list.coffee @@ -8,6 +8,9 @@ class App.KnowledgeBaseSidebarGenericList extends App.Controller constructor: -> super + @render() + + render: -> @html App.view('knowledge_base/sidebar/generic_list')(@templateOptions()) templateOptions: -> @@ -52,3 +55,6 @@ class App.KnowledgeBaseSidebarGenericList extends App.Controller urlNew: -> #has to be overridden + + updateIfNeeded: -> + @render() diff --git a/app/assets/javascripts/app/controllers/knowledge_base/sidebar/attachments.coffee b/app/assets/javascripts/app/controllers/knowledge_base/sidebar/attachments.coffee index 93b9f3936..cce218fca 100644 --- a/app/assets/javascripts/app/controllers/knowledge_base/sidebar/attachments.coffee +++ b/app/assets/javascripts/app/controllers/knowledge_base/sidebar/attachments.coffee @@ -19,9 +19,9 @@ class App.KnowledgeBaseSidebarAttachments extends App.Controller super @render() - @listenTo @object, 'refresh', @needsUpdate + @listenTo @object, 'refresh', @updateIfNeeded - needsUpdate: => + updateIfNeeded: => @render() render: -> diff --git a/app/assets/javascripts/app/controllers/knowledge_base/sidebar/linked_tickets.coffee b/app/assets/javascripts/app/controllers/knowledge_base/sidebar/linked_tickets.coffee index aff24ca8d..0274ae30f 100644 --- a/app/assets/javascripts/app/controllers/knowledge_base/sidebar/linked_tickets.coffee +++ b/app/assets/javascripts/app/controllers/knowledge_base/sidebar/linked_tickets.coffee @@ -12,9 +12,9 @@ class App.KnowledgeBaseSidebarLinkedTickets extends App.Controller super @render() - @listenTo @object, 'refresh', @needsUpdate + @listenTo @object, 'refresh', @updateIfNeeded - needsUpdate: => + updateIfNeeded: => @render() render: -> diff --git a/app/assets/javascripts/app/controllers/knowledge_base/sidebar/tags.coffee b/app/assets/javascripts/app/controllers/knowledge_base/sidebar/tags.coffee index 14f8ea9b8..b5ac9a6ce 100644 --- a/app/assets/javascripts/app/controllers/knowledge_base/sidebar/tags.coffee +++ b/app/assets/javascripts/app/controllers/knowledge_base/sidebar/tags.coffee @@ -11,3 +11,6 @@ class App.KnowledgeBaseSidebarTags extends App.Controller object: @object tags: @object.tags ) + + updateIfNeeded: -> + @widget.reload(@object.tags) diff --git a/app/assets/javascripts/app/lib/mixins/knowledge_base_access.coffee b/app/assets/javascripts/app/lib/mixins/knowledge_base_access.coffee new file mode 100644 index 000000000..8e5df11cb --- /dev/null +++ b/app/assets/javascripts/app/lib/mixins/knowledge_base_access.coffee @@ -0,0 +1,33 @@ +InstanceMethods = + access: -> + permission_reader = App.Permission.findByAttribute('name', 'knowledge_base.reader') + permission_editor = App.Permission.findByAttribute('name', 'knowledge_base.editor') + + permissions_effective = switch @constructor + when App.KnowledgeBaseAnswer + @category().permissions_effective + when App.KnowledgeBaseCategory, App.KnowledgeBase + @permissions_effective + + access = 'none' + + for role_id in App.User.current().role_ids + kb_permission = _.findWhere(permissions_effective, { role_id: role_id }) + + if kb_permission + switch kb_permission.access + when 'editor' + return 'editor' + when 'reader' + access = 'reader' + else if role = App.Role.find(role_id) + if role.permission_ids.indexOf(permission_editor.id) > -1 + return 'editor' + if role.permission_ids.indexOf(permission_reader.id) > -1 + access = 'reader' + + return access + +App.KnowledgeBaseAccess = + extended: -> + @include InstanceMethods diff --git a/app/assets/javascripts/app/lib/mixins/knowledge_base_actions.coffee b/app/assets/javascripts/app/lib/mixins/knowledge_base_actions.coffee index dc3a12837..11009b90b 100644 --- a/app/assets/javascripts/app/lib/mixins/knowledge_base_actions.coffee +++ b/app/assets/javascripts/app/lib/mixins/knowledge_base_actions.coffee @@ -11,6 +11,15 @@ InstanceMethods = disabled: @isNew() } + if (@ instanceof App.KnowledgeBaseCategory) || (@ instanceof App.KnowledgeBase) + buttons.push { + iconName: 'lock' + name: 'Permissions' + action: 'permissions' + cssClass: 'btn--success' + disabled: @isNew() + } + if !(@ instanceof App.KnowledgeBase) buttons.push { iconName: 'trash' diff --git a/app/assets/javascripts/app/lib/mixins/knowledge_base_translatable.coffee b/app/assets/javascripts/app/lib/mixins/knowledge_base_translatable.coffee index bb1aff51f..3e75a0255 100644 --- a/app/assets/javascripts/app/lib/mixins/knowledge_base_translatable.coffee +++ b/app/assets/javascripts/app/lib/mixins/knowledge_base_translatable.coffee @@ -34,29 +34,13 @@ InstanceMethods = attrs.iconFont = true attrs.icon = @category_icon attrs.count = @countDeepAnswers() - - if options.isEditor - attrs.editorOnly = !@visibleInternally(kb_locale) - else - attrs.editorOnly = false - - attrs.state = @visibilityState(kb_locale) + attrs.state = @visibilityState(kb_locale) if @ instanceof App.KnowledgeBaseAnswer attrs.icon = 'knowledge-base-answer' attrs.state = @can_be_published_state() attrs.tags = @tags - if options.isEditor - attrs.editorOnly = !@is_internally_published(kb_locale) - else - attrs.editorOnly = false - - # attrs.className = if attrs.missingTranslation - # 'kb-item--missing-translation' - # else if attrs.editorOnly - # 'kb-item--invisible' - attrs.icons = {} if attrs.missingTranslation diff --git a/app/assets/javascripts/app/models/knowledge_base.coffee b/app/assets/javascripts/app/models/knowledge_base.coffee index 6a03ee5e6..cc4b12a0f 100644 --- a/app/assets/javascripts/app/models/knowledge_base.coffee +++ b/app/assets/javascripts/app/models/knowledge_base.coffee @@ -2,6 +2,7 @@ class App.KnowledgeBase extends App.Model @configure 'KnowledgeBase', 'iconset', 'color_highlight', 'color_header', 'color_header_link', 'translation_ids', 'locale_ids', 'homepage_layout', 'category_layout', 'custom_address' @extend Spine.Model.Ajax @extend App.KnowledgeBaseActions + @extend App.KnowledgeBaseAccess @url: @apiPath + '/knowledge_bases' @manageUrl: @apiPath + '/knowledge_bases/manage' @@ -76,7 +77,7 @@ class App.KnowledgeBase extends App.Model , initial visibleInternally: (kb_locale) -> - @active + @active && @access() != 'none' visiblePublicly: (kb_locale) -> @active @@ -88,6 +89,41 @@ class App.KnowledgeBase extends App.Model attrs + loadedAnswerIds: -> + App.KnowledgeBaseAnswer + .all() + .filter (elem) => elem.knowledge_base().id == @id + .map (elem) -> elem.id + + loadedCategoryIds: -> + App.KnowledgeBaseCategory + .all() + .map (elem) -> elem.id + + removeAssetsIfNeeded: (data) => + removeAnswers = _.difference @loadedAnswerIds(), data.answer_ids + removeCategories = _.difference @loadedAnswerIds(), data.category_ids + + for answer_id in removeAnswers + App.KnowledgeBaseAnswer.find(answer_id)?.remove(clear: true) + + for category_id in removeCategories + App.KnowledgeBaseCategory.find(category_id)?.remove(clear: true) + + !_.isEmpty(removeAnswers) || !_.isEmpty(removeCategories) + + hasAssetsToLoad: (data) => + needsLoadingAnswers = _.difference data.answer_ids, @loadedAnswerIds() + needsLoadingCategories = _.difference data.category_ids, @loadedCategoryIds() + + !_.isEmpty(needsLoadingAnswers) || !_.isEmpty(needsLoadingCategories) + + @allKbModelNames: -> + Object + .keys(App) + .filter (elem) -> + elem.match(/^KnowledgeBase/) && App[elem]?.prototype instanceof App.Model + @configure_attributes: [ { name: 'translation::title' diff --git a/app/assets/javascripts/app/models/knowledge_base_answer.coffee b/app/assets/javascripts/app/models/knowledge_base_answer.coffee index 221a22f66..3c7c12729 100644 --- a/app/assets/javascripts/app/models/knowledge_base_answer.coffee +++ b/app/assets/javascripts/app/models/knowledge_base_answer.coffee @@ -3,6 +3,7 @@ class App.KnowledgeBaseAnswer extends App.Model @extend Spine.Model.Ajax @extend App.KnowledgeBaseActions @extend App.KnowledgeBaseCanBePublished + @extend App.KnowledgeBaseAccess @serverClassName: 'KnowledgeBase::Answer' @@ -95,4 +96,4 @@ class App.KnowledgeBaseAnswer extends App.Model 'Answer' visibleInternally: (kb_locale) => - @is_internally_published(kb_locale) + (@is_internally_published(kb_locale) && @access() != 'none') || @is_published(kb_locale) diff --git a/app/assets/javascripts/app/models/knowledge_base_category.coffee b/app/assets/javascripts/app/models/knowledge_base_category.coffee index c26100388..bcdda5256 100644 --- a/app/assets/javascripts/app/models/knowledge_base_category.coffee +++ b/app/assets/javascripts/app/models/knowledge_base_category.coffee @@ -2,6 +2,7 @@ class App.KnowledgeBaseCategory extends App.Model @configure 'KnowledgeBaseCategory', 'category_icon', 'parent_id', 'child_ids', 'translation_ids' @extend Spine.Model.Ajax @extend App.KnowledgeBaseActions + @extend App.KnowledgeBaseAccess url: -> @knowledge_base().generateURL('categories') @@ -166,6 +167,8 @@ class App.KnowledgeBaseCategory extends App.Model 'draft' visibleInternally: (kb_locale) => + #return false if @access() == 'none' + @findDeepAnswer( (record) -> record.is_internally_published(kb_locale) )? diff --git a/app/assets/javascripts/app/models/knowledge_base_category_permission.coffee b/app/assets/javascripts/app/models/knowledge_base_category_permission.coffee new file mode 100644 index 000000000..df05ad9b8 --- /dev/null +++ b/app/assets/javascripts/app/models/knowledge_base_category_permission.coffee @@ -0,0 +1,2 @@ +class App.KnowledgeBaseCategoryPermission extends App.Model + @configure 'KnowledgeBaseCategoryPermission', 'access' diff --git a/app/assets/javascripts/app/views/knowledge_base/base_form.coffee b/app/assets/javascripts/app/views/knowledge_base/base_form.coffee index c925e38d5..0a2b284b0 100644 --- a/app/assets/javascripts/app/views/knowledge_base/base_form.coffee +++ b/app/assets/javascripts/app/views/knowledge_base/base_form.coffee @@ -65,7 +65,6 @@ class App.KnowledgeBaseForm extends App.Controller submit: (e) -> @preventDefaultAndStopPropagation(e) - #debuggerj formController = @formControllers.filter((elem) -> (elem.form[0] is e.currentTarget) or (e.currentTarget.contains(elem.form[0])))[0] params = @formParam(formController.form) diff --git a/app/assets/javascripts/app/views/knowledge_base/permissions_dialog.jst.eco b/app/assets/javascripts/app/views/knowledge_base/permissions_dialog.jst.eco new file mode 100644 index 000000000..7dd6ecf0e --- /dev/null +++ b/app/assets/javascripts/app/views/knowledge_base/permissions_dialog.jst.eco @@ -0,0 +1,33 @@ +
+ + + + <% for role in @roles: %> + + + <% end %> +
<%- @T('Role') %> + <% for key, text of @accessLevels: %> + <%- @T(text) %> + <% end %> +
+ <%= role.name %> + <% for key, text of @accessLevels: %> + + + <% end %> +
+
diff --git a/app/assets/stylesheets/zammad.scss b/app/assets/stylesheets/zammad.scss index 7e33f4eb3..56dd898c3 100644 --- a/app/assets/stylesheets/zammad.scss +++ b/app/assets/stylesheets/zammad.scss @@ -10921,10 +10921,15 @@ output { } } - .settings-list-checkbox-cell { + .settings-list-checkbox-cell, + .settings-list-radio-cell { vertical-align: middle; padding-left: 8px; } + + .settings-list-radio-cell { + text-align: center; + } } .select-boxes { diff --git a/app/controllers/knowledge_base/permissions_controller.rb b/app/controllers/knowledge_base/permissions_controller.rb new file mode 100644 index 000000000..791804ccc --- /dev/null +++ b/app/controllers/knowledge_base/permissions_controller.rb @@ -0,0 +1,45 @@ +# Copyright (C) 2012-2022 Zammad Foundation, https://zammad-foundation.org/ + +class KnowledgeBase::PermissionsController < ApplicationController + prepend_before_action :authentication_check + before_action :fetch_object + + def show + render json: response_hash + end + + def update + permissions_params = params.require(:permissions_dialog).permit(permissions: {}) + + KnowledgeBase::PermissionsUpdate.new(@object, current_user).update_using_params!(permissions_params) + + render json: response_hash + end + + private + + def fetch_object + if params[:knowledge_base_id] + @object = KnowledgeBase::Category.includes(:permissions).find params[:id] + authorize @object, :permissions? + else + @object = KnowledgeBase.includes(:permissions).find params[:id] + authorize @object, :update? + end + end + + def parent_object + return if !@object.is_a? KnowledgeBase::Category + + @object.parent || @object.knowledge_base + end + + def response_hash + { + roles_reader: Role.with_permissions('knowledge_base.reader').pluck_as_hash(:id, :name), + roles_editor: Role.with_permissions('knowledge_base.editor').pluck_as_hash(:id, :name), + permissions: @object.permissions_effective.pluck_as_hash(:id, :access, :role_id), + inherited: parent_object&.permissions_effective&.pluck_as_hash(:id, :access, :role_id) + } + end +end diff --git a/app/controllers/knowledge_base/public/answers_controller.rb b/app/controllers/knowledge_base/public/answers_controller.rb index 612e6f192..aeb970291 100644 --- a/app/controllers/knowledge_base/public/answers_controller.rb +++ b/app/controllers/knowledge_base/public/answers_controller.rb @@ -13,9 +13,7 @@ class KnowledgeBase::Public::AnswersController < KnowledgeBase::Public::BaseCont private def render_alternative - @alternative = policy_scope(@knowledge_base.answers) - .eager_load(translations: :kb_locale) - .find_by(id: params[:answer]) + @alternative = find_answer @knowledge_base.answers.eager_load(translations: :kb_locale), params[:answer], locale: false raise ActiveRecord::RecordNotFound if !@alternative&.translations&.any? diff --git a/app/controllers/knowledge_base/public/base_controller.rb b/app/controllers/knowledge_base/public/base_controller.rb index eb0b61d48..7cb9ab15c 100644 --- a/app/controllers/knowledge_base/public/base_controller.rb +++ b/app/controllers/knowledge_base/public/base_controller.rb @@ -2,7 +2,8 @@ class KnowledgeBase::Public::BaseController < ApplicationController before_action :load_kb - helper_method :system_locale_via_uri, :fallback_locale, :current_user, :find_category, :filter_primary_kb_locale, :menu_items, :all_locales + helper_method :system_locale_via_uri, :fallback_locale, :current_user, :find_category, + :filter_primary_kb_locale, :menu_items, :all_locales, :can_preview? layout 'knowledge_base' @@ -31,12 +32,9 @@ class KnowledgeBase::Public::BaseController < ApplicationController end def fallback_locale - if all_locales.find { |locale| locale.id == system_locale_via_uri&.id } - system_locale_via_uri - else - filter_primary_kb_locale || all_locales.first - end + return system_locale_via_uri if all_locales.find { |locale| locale.id == system_locale_via_uri&.id } + filter_primary_kb_locale || all_locales.first end def filter_primary_kb_locale @@ -62,16 +60,40 @@ class KnowledgeBase::Public::BaseController < ApplicationController list .localed(system_locale_via_uri) .sorted - .select { |category| policy(category).show? } + .select { |category| policy(category).show_public? } end - def find_answer(scope, id) + def answers_filter(list) + answers = list + .localed(system_locale_via_uri) + .sorted + + if current_user&.permissions?('knowledge_base.editor') + answers.filter { |answer| policy(answer).show_public? } + else + answers.published + end + end + + def find_answer(scope, id, locale: system_locale_via_uri) return if scope.nil? - policy_scope(scope) - .localed(system_locale_via_uri) - .include_contents - .find_by(id: id) + scope = scope.include_contents + scope = scope.localed(locale) if locale + + if !current_user&.permissions?('knowledge_base.editor') + return scope.published.find_by(id: id) + end + + if (item = scope.find_by(id: id)) && policy(item).show_public? + return item + end + + nil + end + + def can_preview? + @can_preview ||= policy(@knowledge_base).update? end def not_found(e) diff --git a/app/controllers/knowledge_base/public/categories_controller.rb b/app/controllers/knowledge_base/public/categories_controller.rb index f86314cc0..17c3148c1 100644 --- a/app/controllers/knowledge_base/public/categories_controller.rb +++ b/app/controllers/knowledge_base/public/categories_controller.rb @@ -15,14 +15,11 @@ class KnowledgeBase::Public::CategoriesController < KnowledgeBase::Public::BaseC def show @object = find_category(params[:category]) - render_alternatives && return if @object.nil? || !policy(@object).show? + render_alternatives && return if @object.nil? || !policy(@object).show_public? @categories = categories_filter(@object.children) @object_locales = find_locales(@object) - - @answers = policy_scope(@object.answers) - .localed(system_locale_via_uri) - .sorted + @answers = answers_filter(@object.answers) render :index end diff --git a/app/controllers/knowledge_base/public/tags_controller.rb b/app/controllers/knowledge_base/public/tags_controller.rb index 04bf01f59..5e22382f4 100644 --- a/app/controllers/knowledge_base/public/tags_controller.rb +++ b/app/controllers/knowledge_base/public/tags_controller.rb @@ -2,12 +2,7 @@ class KnowledgeBase::Public::TagsController < KnowledgeBase::Public::BaseController def show - @object = [:tag, params[:tag]] - - all_tagged = KnowledgeBase::Answer.tag_objects(params[:tag]) - - @answers = policy_scope(all_tagged) - .localed(system_locale_via_uri) - .sorted + @object = [:tag, params[:tag]] + @answers = answers_filter KnowledgeBase::Answer.tag_objects(params[:tag]) end end diff --git a/app/controllers/knowledge_bases_controller.rb b/app/controllers/knowledge_bases_controller.rb index 51ccf8352..4667c42c9 100644 --- a/app/controllers/knowledge_bases_controller.rb +++ b/app/controllers/knowledge_bases_controller.rb @@ -5,15 +5,41 @@ class KnowledgeBasesController < KnowledgeBase::BaseController render json: assets(params[:answer_translation_content_ids]) end + def visible_ids + render json: KnowledgeBase::InternalAssets.new(current_user).visible_ids + end + private def assets(answer_translation_content_ids = nil) - return editor_assets(answer_translation_content_ids) if current_user&.permissions?('knowledge_base.editor') - return reader_assets(answer_translation_content_ids) if current_user&.permissions?('knowledge_base.reader') + if KnowledgeBase.granular_permissions? + return granular_assets(answer_translation_content_ids) if kb_permissions? + else + return editor_assets(answer_translation_content_ids) if kb_permission_editor? + return reader_assets(answer_translation_content_ids) if kb_permission_reader? + end public_assets end + def kb_permissions? + current_user&.permissions?(%w[knowledge_base.editor knowledge_base.reader]) + end + + def kb_permission_editor? + current_user&.permissions?('knowledge_base.editor') + end + + def kb_permission_reader? + current_user&.permissions?('knowledge_base.reader') + end + + def granular_assets(answer_translation_content_ids) + KnowledgeBase::InternalAssets + .new(current_user, answer_translation_content_ids: answer_translation_content_ids) + .collect_assets + end + def editor_assets(answer_translation_content_ids) assets = [ KnowledgeBase, diff --git a/app/helpers/knowledge_base_top_bar_helper.rb b/app/helpers/knowledge_base_top_bar_helper.rb index 9a4092adc..d002e3c18 100644 --- a/app/helpers/knowledge_base_top_bar_helper.rb +++ b/app/helpers/knowledge_base_top_bar_helper.rb @@ -36,7 +36,7 @@ module KnowledgeBaseTopBarHelper end def render_top_bar_if_needed(object, knowledge_base) - return if !policy(:knowledge_base).edit? + return if !can_preview? editable = object || knowledge_base diff --git a/app/helpers/knowledge_base_visibility_class_helper.rb b/app/helpers/knowledge_base_visibility_class_helper.rb index 0666bf13d..bd5cd39e6 100644 --- a/app/helpers/knowledge_base_visibility_class_helper.rb +++ b/app/helpers/knowledge_base_visibility_class_helper.rb @@ -2,7 +2,7 @@ module KnowledgeBaseVisibilityClassHelper def visibility_class_name(object) - return if !policy(:knowledge_base).edit? + return if !can_preview? suffix = case object when CanBePublished diff --git a/app/helpers/knowledge_base_visibility_note_helper.rb b/app/helpers/knowledge_base_visibility_note_helper.rb index 861766559..a068eae0f 100644 --- a/app/helpers/knowledge_base_visibility_note_helper.rb +++ b/app/helpers/knowledge_base_visibility_note_helper.rb @@ -2,7 +2,7 @@ module KnowledgeBaseVisibilityNoteHelper def visibility_note(object) - return if !policy(:knowledge_base).edit? + return if !can_preview? text = visibility_text(object) diff --git a/app/jobs/checks_kb_client_notification_job.rb b/app/jobs/checks_kb_client_notification_job.rb index b2403e143..5402c9641 100644 --- a/app/jobs/checks_kb_client_notification_job.rb +++ b/app/jobs/checks_kb_client_notification_job.rb @@ -4,12 +4,12 @@ class ChecksKbClientNotificationJob < ApplicationJob include HasActiveJobLock def lock_key - # "ChecksKbClientNotificationJob/KnowledgeBase::Answer/42/destroy" - "#{self.class.name}/#{arguments[0]}/#{arguments[1]}/#{arguments[2]}" + # "ChecksKbClientNotificationJob/KnowledgeBase::Answer/42" + "#{self.class.name}/#{arguments[0]}/#{arguments[1]}" end - def perform(klass_name, id, event) - object = klass_name.constantize.find_by(id: id) + def perform(klass_name, object_id) + object = klass_name.constantize.find_by(id: object_id) return if object.blank? level = needs_editor?(object) ? 'editor' : '*' @@ -24,7 +24,7 @@ class ChecksKbClientNotificationJob < ApplicationJob def build_data(object, event) timestamp = event == :destroy ? Time.zone.now : object.updated_at - url = event == :destroy ? nil : object.api_url + url = event == :destroy ? nil : object.try(:api_url) { class: object.class.to_s, @@ -57,8 +57,4 @@ class ChecksKbClientNotificationJob < ApplicationJob .filter_map { |user_id| User.find_by(id: user_id) } .select { |user| user.permissions? "knowledge_base.#{permission_suffix}" } end - - def self.notify_later(object, event) - perform_later(object.class.to_s, object.id, event.to_s) - end end diff --git a/app/jobs/checks_kb_client_visibility_job.rb b/app/jobs/checks_kb_client_visibility_job.rb new file mode 100644 index 000000000..3b41e3ec9 --- /dev/null +++ b/app/jobs/checks_kb_client_visibility_job.rb @@ -0,0 +1,14 @@ +# Copyright (C) 2012-2022 Zammad Foundation, https://zammad-foundation.org/ + +class ChecksKbClientVisibilityJob < ApplicationJob + include HasActiveJobLock + + def lock_key + # "ChecksKbClientVisibilityJob" + self.class.name + end + + def perform + Sessions.broadcast({ event: 'kb_visibility_may_have_changed' }) + end +end diff --git a/app/models/concerns/checks_kb_client_notification.rb b/app/models/concerns/checks_kb_client_notification.rb index 40bef7d55..f98a57a92 100644 --- a/app/models/concerns/checks_kb_client_notification.rb +++ b/app/models/concerns/checks_kb_client_notification.rb @@ -4,10 +4,7 @@ module ChecksKbClientNotification extend ActiveSupport::Concern included do - after_create :notify_kb_clients_after_create - after_update :notify_kb_clients_after_update - after_touch :notify_kb_clients_after_touch - after_destroy :notify_kb_clients_after_destroy + after_commit :notify_kb_clients_after class_attribute :notify_kb_clients_suspend, default: false end @@ -30,25 +27,9 @@ module ChecksKbClientNotification # generic call - def notify_kb_clients(event) + def notify_kb_clients_after return if self.class.notify_kb_clients_suspend? - ChecksKbClientNotificationJob.notify_later(self, event) - end - - def notify_kb_clients_after_create - notify_kb_clients(:create) - end - - def notify_kb_clients_after_update - notify_kb_clients(:update) - end - - def notify_kb_clients_after_touch - notify_kb_clients(:touch) - end - - def notify_kb_clients_after_destroy - notify_kb_clients(:destroy) + ChecksKbClientNotificationJob.perform_later(self.class.name, id) end end diff --git a/app/models/concerns/checks_kb_client_visibility.rb b/app/models/concerns/checks_kb_client_visibility.rb new file mode 100644 index 000000000..d441a4b74 --- /dev/null +++ b/app/models/concerns/checks_kb_client_visibility.rb @@ -0,0 +1,17 @@ +# Copyright (C) 2012-2022 Zammad Foundation, https://zammad-foundation.org/ + +module ChecksKbClientVisibility + extend ActiveSupport::Concern + + included do + after_commit :notify_kb_client_visibility + end + + private + + def notify_kb_client_visibility + return if self.class.notify_kb_clients_suspend? + + ChecksKbClientVisibilityJob.perform_later + end +end diff --git a/app/models/knowledge_base.rb b/app/models/knowledge_base.rb index c5a8e9d20..b28815b4b 100644 --- a/app/models/knowledge_base.rb +++ b/app/models/knowledge_base.rb @@ -24,6 +24,11 @@ class KnowledgeBase < ApplicationModel has_many :answers, through: :categories + has_many :permissions, class_name: 'KnowledgeBase::Permission', + as: :permissionable, + autosave: true, + dependent: :destroy + validates :category_layout, inclusion: { in: KnowledgeBase::LAYOUTS } validates :homepage_layout, inclusion: { in: KnowledgeBase::LAYOUTS } @@ -156,6 +161,24 @@ class KnowledgeBase < ApplicationModel .any? { |e| e > 1 } end + def permissions_effective + cache_key = KnowledgeBase::Permission.cache_key self + + Rails.cache.fetch cache_key do + permissions + end + end + + def attributes_with_association_ids + attrs = super + attrs[:permissions_effective] = permissions_effective + attrs + end + + def self.granular_permissions? + KnowledgeBase::Permission.any? + end + private def set_defaults diff --git a/app/models/knowledge_base/answer.rb b/app/models/knowledge_base/answer.rb index 2fd0a28f8..4f97139ca 100644 --- a/app/models/knowledge_base/answer.rb +++ b/app/models/knowledge_base/answer.rb @@ -6,6 +6,7 @@ class KnowledgeBase::Answer < ApplicationModel include HasTags include CanBePublished include ChecksKbClientNotification + include ChecksKbClientVisibility include CanCloneAttachments AGENT_ALLOWED_ATTRIBUTES = %i[category_id promoted internal_note].freeze @@ -49,7 +50,13 @@ class KnowledgeBase::Answer < ApplicationModel siblings = category.answers if !User.lookup(id: UserInfo.current_user_id)&.permissions?('knowledge_base.editor') - siblings = siblings.internal + ep = KnowledgeBase::EffectivePermission.new User.find(UserInfo.current_user_id), category + + siblings = if ep.access_effective == 'public_reader' + siblings.published + else + siblings.internal + end end data = ApplicationModel::CanAssets.reduce(siblings, data) diff --git a/app/models/knowledge_base/category.rb b/app/models/knowledge_base/category.rb index 6edd93e63..6f351277b 100644 --- a/app/models/knowledge_base/category.rb +++ b/app/models/knowledge_base/category.rb @@ -4,25 +4,31 @@ class KnowledgeBase::Category < ApplicationModel include HasTranslations include HasAgentAllowedParams include ChecksKbClientNotification + include ChecksKbClientVisibility AGENT_ALLOWED_ATTRIBUTES = %i[knowledge_base_id parent_id category_icon].freeze AGENT_ALLOWED_NESTED_RELATIONS = %i[translations].freeze belongs_to :knowledge_base, inverse_of: :categories - has_many :answers, class_name: 'KnowledgeBase::Answer', - inverse_of: :category, - dependent: :restrict_with_exception + has_many :answers, class_name: 'KnowledgeBase::Answer', + inverse_of: :category, + dependent: :restrict_with_exception - has_many :children, class_name: 'KnowledgeBase::Category', - foreign_key: :parent_id, - inverse_of: :parent, - dependent: :restrict_with_exception + has_many :children, class_name: 'KnowledgeBase::Category', + foreign_key: :parent_id, + inverse_of: :parent, + dependent: :restrict_with_exception - belongs_to :parent, class_name: 'KnowledgeBase::Category', - inverse_of: :children, - touch: true, - optional: true + belongs_to :parent, class_name: 'KnowledgeBase::Category', + inverse_of: :children, + touch: true, + optional: true + + has_many :permissions, class_name: 'KnowledgeBase::Permission', + as: :permissionable, + autosave: true, + dependent: :destroy validates :category_icon, presence: true @@ -119,6 +125,20 @@ class KnowledgeBase::Category < ApplicationModel Rails.application.routes.url_helpers.knowledge_base_category_path(knowledge_base, self) end + def permissions_effective + cache_key = KnowledgeBase::Permission.cache_key self + + Rails.cache.fetch cache_key do + KnowledgeBase::Category::Permission.new(self).permissions_effective + end + end + + def attributes_with_association_ids + attrs = super + attrs[:permissions_effective] = permissions_effective + attrs + end + private def cannot_be_child_of_parent diff --git a/app/models/knowledge_base/permission.rb b/app/models/knowledge_base/permission.rb new file mode 100644 index 000000000..4f5b4dd33 --- /dev/null +++ b/app/models/knowledge_base/permission.rb @@ -0,0 +1,16 @@ +# Copyright (C) 2012-2022 Zammad Foundation, https://zammad-foundation.org/ + +class KnowledgeBase::Permission < ApplicationModel + belongs_to :permissionable, polymorphic: true, touch: true + belongs_to :role + + validates :access, inclusion: { in: %w[editor reader none] } + validates :role, uniqueness: { scope: %i[permissionable_id permissionable_type] } + + # cache key for calculated permissions + # @param permissionable [KnowledgeBase::Category, KnowledgeBase] + # @return [String] + def self.cache_key(permissionable) + "#{permissionable.class}::aws::#{permissionable.id}::permission::#{permissionable.updated_at}" + end +end diff --git a/app/models/role.rb b/app/models/role.rb index 9d8e4d30e..daa7afbbb 100644 --- a/app/models/role.rb +++ b/app/models/role.rb @@ -14,11 +14,12 @@ class Role < ApplicationModel has_and_belongs_to_many :users, after_add: :cache_update, after_remove: :cache_update has_and_belongs_to_many :permissions, before_add: %i[validate_agent_limit_by_permission validate_permissions], - after_add: :cache_update, + after_add: %i[cache_update cache_add_kb_permission], before_remove: :last_admin_check_by_permission, - after_remove: :cache_update + after_remove: %i[cache_update cache_remove_kb_permission] validates :name, presence: true store :preferences + has_many :knowledge_base_permissions, class_name: 'KnowledgeBase::Permission', dependent: :destroy before_create :check_default_at_signup_permissions before_update :last_admin_check_by_attribute, :validate_agent_limit_by_attributes, :check_default_at_signup_permissions @@ -232,4 +233,31 @@ returns raise Exceptions::UnprocessableEntity, "Cannot set default at signup when role has #{forbidden_permissions.join(', ')} permissions." end + def cache_add_kb_permission(permission) + return if !permission.name.starts_with? 'knowledge_base.' + return if !KnowledgeBase.granular_permissions? + + KnowledgeBase::Category.all.each(&:touch) + end + + def cache_remove_kb_permission(permission) + return if !permission.name.starts_with? 'knowledge_base.' + return if !KnowledgeBase.granular_permissions? + + has_editor = permissions.where(name: 'knowledge_base.editor').any? + has_reader = permissions.where(name: 'knowledge_base.reader').any? + + KnowledgeBase::Permission + .where(role: self) + .each do |elem| + if !has_editor && !has_reader + elem.destroy! + elsif !has_editor && has_reader + elem.update!(access: 'reader') if permission.access == 'editor' + end + + end + + KnowledgeBase::Category.all.each(&:touch) + end end diff --git a/app/policies/controllers/knowledge_base/answers_controller_policy.rb b/app/policies/controllers/knowledge_base/answers_controller_policy.rb index b530b5cee..a01d7db4a 100644 --- a/app/policies/controllers/knowledge_base/answers_controller_policy.rb +++ b/app/policies/controllers/knowledge_base/answers_controller_policy.rb @@ -2,9 +2,34 @@ class Controllers::KnowledgeBase::AnswersControllerPolicy < Controllers::KnowledgeBase::BaseControllerPolicy def show? - return true if user.permissions?('knowledge_base.editor') + access(__method__) + end - object = record.klass.find(record.params[:id]) - object.can_be_published_aasm.internal? || object.can_be_published_aasm.published? + def create? + verify_category(__method__) + end + + def update? + access(__method__) && verify_category(__method__) + end + + def destroy? + access(__method__) + end + + private + + def object + @object ||= record.klass.find(record.params[:id]) + end + + def access(method) + KnowledgeBase::AnswerPolicy.new(user, object).send(method) + end + + def verify_category(method) + new_category = KnowledgeBase::Category.find(record.params[:category_id]) + + KnowledgeBase::CategoryPolicy.new(user, new_category).send(method) end end diff --git a/app/policies/controllers/knowledge_base/categories_controller_policy.rb b/app/policies/controllers/knowledge_base/categories_controller_policy.rb index 932d2bb5a..3bf82a255 100644 --- a/app/policies/controllers/knowledge_base/categories_controller_policy.rb +++ b/app/policies/controllers/knowledge_base/categories_controller_policy.rb @@ -2,8 +2,38 @@ class Controllers::KnowledgeBase::CategoriesControllerPolicy < Controllers::KnowledgeBase::BaseControllerPolicy def show? - return if user.permissions?('knowledge_base.editor') + access(__method__) + end - record.klass.find(record.params[:id]).internal_content? + def create? + verify_parent(__method__) + end + + def update? + access(__method__) && verify_parent(__method__) + end + + def destroy? + access(__method__) + end + + private + + def object + @object ||= record.klass.find(record.params[:id]) + end + + def access(method) + KnowledgeBase::CategoryPolicy.new(user, object).send(method) + end + + def verify_parent(method) + if record.params[:parent_id].blank? + return user.permissions?('knowledge_base.editor') + end + + parent = KnowledgeBase::Category.find(record.params[:parent_id]) + + KnowledgeBase::CategoryPolicy.new(user, parent).send(method) end end diff --git a/app/policies/controllers/knowledge_bases_controller_policy.rb b/app/policies/controllers/knowledge_bases_controller_policy.rb index 9b7baa589..8a90a5621 100644 --- a/app/policies/controllers/knowledge_bases_controller_policy.rb +++ b/app/policies/controllers/knowledge_bases_controller_policy.rb @@ -12,4 +12,18 @@ class Controllers::KnowledgeBasesControllerPolicy < Controllers::KnowledgeBase:: def destroy? false end + + def update? + access(__method__) + end + + private + + def object + @object ||= record.klass.find(record.params[:id]) + end + + def access(method) + KnowledgeBase::CategoryPolicy.new(user, object).send(method) + end end diff --git a/app/policies/knowledge_base/answer_policy.rb b/app/policies/knowledge_base/answer_policy.rb index 99f84d41b..bd030b05a 100644 --- a/app/policies/knowledge_base/answer_policy.rb +++ b/app/policies/knowledge_base/answer_policy.rb @@ -2,13 +2,39 @@ class KnowledgeBase::AnswerPolicy < ApplicationPolicy def show? - return true if user&.permissions?(%w[knowledge_base.editor]) + return true if access_editor? record.visible? || - (user&.permissions?(%w[knowledge_base.reader]) && record.visible_internally?) + (access_reader? && record.visible_internally?) + end + + def show_public? + access_editor? || record.visible? + end + + def create? + access_editor? + end + + def update? + access_editor? end def destroy? - user&.permissions?(%w[knowledge_base.editor]) + access_editor? + end + + private + + def access + @access ||= KnowledgeBase::EffectivePermission.new(user, record.category).access_effective + end + + def access_editor? + access == 'editor' + end + + def access_reader? + access == 'reader' end end diff --git a/app/policies/knowledge_base/category_policy.rb b/app/policies/knowledge_base/category_policy.rb index 18576c765..30699b351 100644 --- a/app/policies/knowledge_base/category_policy.rb +++ b/app/policies/knowledge_base/category_policy.rb @@ -2,13 +2,51 @@ class KnowledgeBase::CategoryPolicy < ApplicationPolicy def show? - return true if user&.permissions?('knowledge_base.editor') + access_editor? || access_reader? + end - record.public_content? + def show_public? + access_editor? || record.public_content? + end + + def permissions? + access_editor? + end + + def create? + parent_editor? + end + + def update? + access_editor? + end + + def destroy? + parent_editor? end private + def access + @access ||= KnowledgeBase::EffectivePermission.new(user, record).access_effective + end + + def access_editor? + access == 'editor' + end + + def access_reader? + access == 'reader' + end + + def parent_access + @parent_access ||= KnowledgeBase::EffectivePermission.new(user, (record.parent || record.knowledge_base)).access_effective + end + + def parent_editor? + parent_access == 'editor' + end + def user_required? false end diff --git a/app/policies/knowledge_base_policy.rb b/app/policies/knowledge_base_policy.rb index 376742a66..9c5c6ef77 100644 --- a/app/policies/knowledge_base_policy.rb +++ b/app/policies/knowledge_base_policy.rb @@ -1,7 +1,25 @@ # Copyright (C) 2012-2022 Zammad Foundation, https://zammad-foundation.org/ class KnowledgeBasePolicy < ApplicationPolicy - def edit? - user&.permissions?('knowledge_base.editor') + def show? + access_editor? || access_reader? + end + + def update? + access_editor? + end + + private + + def access + @access ||= KnowledgeBase::EffectivePermission.new(user, record).access_effective + end + + def access_editor? + access == 'editor' + end + + def access_reader? + access == 'reader' end end diff --git a/config/routes/knowledge_base.rb b/config/routes/knowledge_base.rb index 374287496..ce7ad52d0 100644 --- a/config/routes/knowledge_base.rb +++ b/config/routes/knowledge_base.rb @@ -20,6 +20,7 @@ Zammad::Application.routes.draw do resources :knowledge_bases, only: %i[show update] do collection do post :init + get :visible_ids post :search, controller: 'knowledge_base/search' get :recent_answers, controller: 'knowledge_base/answers' @@ -35,11 +36,17 @@ Zammad::Application.routes.draw do end end + member do + resource :permissions, controller: 'knowledge_base/permissions', only: %i[update show] + end + resources :categories, controller: 'knowledge_base/categories', except: %i[new edit] do member do patch :reorder_categories, :reorder_answers + + resource :permissions, controller: 'knowledge_base/permissions', only: %i[update show] end collection do diff --git a/db/migrate/20190531180304_initialize_knowledge_base.rb b/db/migrate/20190531180304_initialize_knowledge_base.rb index 84d9fe336..9df3ff832 100644 --- a/db/migrate/20190531180304_initialize_knowledge_base.rb +++ b/db/migrate/20190531180304_initialize_knowledge_base.rb @@ -108,6 +108,18 @@ class InitializeKnowledgeBase < ActiveRecord::Migration[5.0] t.timestamps # rubocop:disable Zammad/ExistsDateTimePrecision end + create_table :knowledge_base_permissions do |t| + t.references :permissionable, polymorphic: true, null: false, index: { name: 'index_knowledge_base_permissions_on_permissionable' } + t.references :role, null: false, foreign_key: { to_table: :roles } + + t.string 'access', limit: 50, default: 'full', null: false + t.index 'access' + + t.index %i[role_id permissionable_id permissionable_type], unique: true, name: 'knowledge_base_permissions_uniqueness' + + t.timestamps limit: 3 + end + Setting.create_if_not_exists( title: 'Kb multi-lingual support', name: 'kb_multi_lingual_support', @@ -169,11 +181,12 @@ class InitializeKnowledgeBase < ActiveRecord::Migration[5.0] ) Permission.create_if_not_exists( - name: 'knowledge_base.reader', - note: 'Access %s', - preferences: { + name: 'knowledge_base.reader', + note: 'Access %s', + preferences: { translations: ['Knowledge Base'] - } + }, + allow_signup: true, ) Permission.create_if_not_exists( diff --git a/db/migrate/20210918163819_create_knowledge_base_permissions.rb b/db/migrate/20210918163819_create_knowledge_base_permissions.rb new file mode 100644 index 000000000..182eb6df7 --- /dev/null +++ b/db/migrate/20210918163819_create_knowledge_base_permissions.rb @@ -0,0 +1,22 @@ +# Copyright (C) 2012-2022 Zammad Foundation, https://zammad-foundation.org/ + +# Using older 5.0 migration to stick to Integer primary keys. Otherwise migration fails in MySQL. +class CreateKnowledgeBasePermissions < ActiveRecord::Migration[5.0] + def change + return if !Setting.exists?(name: 'system_init_done') + + create_table :knowledge_base_permissions do |t| + t.references :permissionable, polymorphic: true, null: false, index: { name: 'index_knowledge_base_permissions_on_permissionable' } + t.references :role, null: false, foreign_key: { to_table: :roles } + + t.string 'access', limit: 50, default: 'full', null: false + t.index 'access' + + t.index %i[role_id permissionable_id permissionable_type], unique: true, name: 'knowledge_base_permissions_uniqueness' + + t.timestamps limit: 3 + end + + Permission.where(name: 'knowledge_base.reader').update_all(allow_signup: true) # rubocop:disable Rails/SkipsModelValidations + end +end diff --git a/db/seeds/permissions.rb b/db/seeds/permissions.rb index e16ccf5df..4ecd2540f 100644 --- a/db/seeds/permissions.rb +++ b/db/seeds/permissions.rb @@ -441,11 +441,12 @@ Permission.create_if_not_exists( ) Permission.create_if_not_exists( - name: 'knowledge_base.reader', - note: __('Manage %s'), - preferences: { + name: 'knowledge_base.reader', + note: __('Manage %s'), + preferences: { translations: [__('Knowledge Base Reader')] - } + }, + allow_signup: true, ) admin = Role.find_by(name: 'Admin') diff --git a/i18n/zammad.pot b/i18n/zammad.pot index 7623502f9..7a3e5e981 100644 --- a/i18n/zammad.pot +++ b/i18n/zammad.pot @@ -5163,6 +5163,10 @@ msgstr "" msgid "Invalid payload, need data:image in logo param" msgstr "" +#: lib/knowledge_base/permissions_update.rb +msgid "Invalid permissions, do not lock out yourself" +msgstr "" + #: app/models/concerns/checks_condition_validation.rb msgid "Invalid ticket selector conditions" msgstr "" @@ -7613,6 +7617,7 @@ msgstr "" #: app/assets/javascripts/app/controllers/_ui_element/core_workflow_condition.coffee #: app/assets/javascripts/app/controllers/role.coffee #: app/assets/javascripts/app/views/integration/ldap.jst.eco +#: app/assets/javascripts/app/views/knowledge_base/permissions_dialog.jst.eco msgid "Role" msgstr "" @@ -9570,6 +9575,14 @@ msgstr "" msgid "URL (AJAX endpoint)" msgstr "" +#: app/assets/javascripts/app/controllers/knowledge_base/permissions_dialog.coffee +msgid "Unable to load changes" +msgstr "" + +#: app/assets/javascripts/app/controllers/knowledge_base/permissions_dialog.coffee +msgid "Unable to save changes" +msgstr "" + #: app/controllers/first_steps_controller.rb #: db/seeds/overviews.rb msgid "Unassigned & Open Tickets" @@ -10024,11 +10037,11 @@ msgid "Visibility" msgstr "" #: app/assets/javascripts/app/controllers/knowledge_base/content_can_be_published_form.coffee -msgid "Visible to agents & editors" +msgid "Visible to everyone" msgstr "" #: app/assets/javascripts/app/controllers/knowledge_base/content_can_be_published_form.coffee -msgid "Visible to everyone" +msgid "Visible to readers & editors" msgstr "" #: app/assets/javascripts/app/controllers/_integration/placetel.coffee diff --git a/lib/core_ext/active_record/calculations/pluck_as_hash.rb b/lib/core_ext/active_record/calculations/pluck_as_hash.rb index a0a31eaf5..371121cdd 100644 --- a/lib/core_ext/active_record/calculations/pluck_as_hash.rb +++ b/lib/core_ext/active_record/calculations/pluck_as_hash.rb @@ -37,3 +37,24 @@ module ActiveRecord end end end + +module Enumerable + def pluck_as_hash(*column_names) + column_names.flatten! # flatten args in case array was given + + pluck(*column_names) + .map { |elem| pluck_as_hash_map(column_names, elem) } + end + + private + + def pluck_as_hash_map(keys, values) + if keys.one? + { + keys.first => values + } + else + keys.zip(values).to_h + end + end +end diff --git a/lib/knowledge_base/category/permission.rb b/lib/knowledge_base/category/permission.rb new file mode 100644 index 000000000..c86387bf2 --- /dev/null +++ b/lib/knowledge_base/category/permission.rb @@ -0,0 +1,33 @@ +# Copyright (C) 2012-2022 Zammad Foundation, https://zammad-foundation.org/ + +class KnowledgeBase + class Category + class Permission + def initialize(category) + @category = category + end + + def permissions_effective + parents_for_category + .map(&:permissions) + .flatten + .each_with_object([]) do |elem, memo| + memo << elem if !memo.find { |added| added.role == elem.role } + end + end + + private + + def parents_for_category + categories_tree = @category.self_with_parents + + categories_with_permissions = KnowledgeBase::Category.where(id: categories_tree).includes(:permissions).to_a + + sorted_with_permissions = categories_tree + .map { |elem| categories_with_permissions.find { |elem_with_permissions| elem_with_permissions == elem } } + + sorted_with_permissions + [@category.knowledge_base] + end + end + end +end diff --git a/lib/knowledge_base/effective_permission.rb b/lib/knowledge_base/effective_permission.rb new file mode 100644 index 000000000..901e3e3d0 --- /dev/null +++ b/lib/knowledge_base/effective_permission.rb @@ -0,0 +1,58 @@ +# Copyright (C) 2012-2022 Zammad Foundation, https://zammad-foundation.org/ + +class KnowledgeBase + class EffectivePermission + def initialize(user, object) + @user = user + @object = object + end + + def access_effective + return 'none' if !@user + + @user.roles.reduce('none') do |memo, role| + access = access_role_effective(role) + + return access if access == 'editor' + + memo == 'reader' ? memo : access + end + end + + private + + def permissions + @permissions ||= @object.permissions_effective + end + + def access_role_effective(role) + permission = permissions.find { |elem| elem.role == role } + + return default_role_access(role) if !permission + + calculate_role(role, permission) + end + + def calculate_role(role, permission) + if permission.access == 'editor' && role.with_permission?('knowledge_base.editor') + 'editor' + elsif %w[editor reader].include?(permission.access) && role.with_permission?(%w[knowledge_base.editor knowledge_base.reader]) + 'reader' + elsif @object.public_content? + 'public_reader' + else + 'none' + end + end + + def default_role_access(role) + if role.with_permission?('knowledge_base.editor') + 'editor' + elsif role.with_permission?('knowledge_base.reader') + 'reader' + else + 'none' + end + end + end +end diff --git a/lib/knowledge_base/internal_assets.rb b/lib/knowledge_base/internal_assets.rb new file mode 100644 index 000000000..714c8f531 --- /dev/null +++ b/lib/knowledge_base/internal_assets.rb @@ -0,0 +1,120 @@ +# Copyright (C) 2012-2022 Zammad Foundation, https://zammad-foundation.org/ + +class KnowledgeBase + class InternalAssets + CategoriesCache = Struct.new(:editor, :reader, :public_reader, keyword_init: true) do + def all + editor + reader + public_reader + end + end + + attr_reader :assets + + def initialize(user, answer_translation_content_ids: []) + @user = user + @assets = {} + @answer_translation_content_ids = answer_translation_content_ids + end + + def collect_assets + collect_base_assets + + add_to_assets accessible_categories.all, type: :essential + add_to_assets KnowledgeBase::Category::Translation.where(category: accessible_categories.all) + + collect_all_answer_assets + + @assets + end + + def accessible_categories + @accessible_categories ||= accessible_categories_calculate + end + + def all_answer_ids + all_answer_batches.each_with_object([]) do |elem, sum| + sum.concat elem.pluck(:id) + end + end + + def all_category_ids + accessible_categories.all.pluck(:id) + end + + def visible_ids + { + answer_ids: all_answer_ids, + category_ids: all_category_ids + } + end + + private + + def accessible_categories_calculate + struct = CategoriesCache.new editor: [], reader: [], public_reader: [] + + KnowledgeBase::Category.all.find_in_batches do |group| + group.each do |cat| + case KnowledgeBase::EffectivePermission.new(@user, cat).access_effective + when 'editor' + struct.editor << cat + when 'reader' + struct.reader << cat if cat.internal_content? + when 'public_reader' + struct.public_reader << cat if cat.public_content? + end + end + end + + struct + end + + def add_to_assets(objects, type: nil) + @assets = ApplicationModel::CanAssets.reduce(objects, @assets, type) + end + + def collect_base_assets + [KnowledgeBase, KnowledgeBase::Translation, KnowledgeBase::Locale] + .each do |klass| + klass.find_in_batches do |group| + add_to_assets group, type: :essential + end + end + end + + def all_answer_batches + [ + KnowledgeBase::Answer.where(category: accessible_categories.editor), + KnowledgeBase::Answer.internal.where(category: accessible_categories.reader), + KnowledgeBase::Answer.published.where(category: accessible_categories.public_reader) + ] + end + + def collect_all_answer_assets + all_answer_batches.each do |batch| + collect_answers_assets batch + end + end + + def collect_answers_assets(scope) + scope.find_in_batches do |group| + add_to_assets group, type: :essential + + translations = KnowledgeBase::Answer::Translation.where(answer: group) + + add_to_assets translations, type: :essential + + if @answer_translation_content_ids.present? + contents = KnowledgeBase::Answer::Translation::Content + .joins(:translation) + .where( + id: @answer_translation_content_ids, + knowledge_base_answer_translations: { answer_id: group } + ) + + add_to_assets contents + end + end + end + end +end diff --git a/lib/knowledge_base/permissions_update.rb b/lib/knowledge_base/permissions_update.rb new file mode 100644 index 000000000..fd3488599 --- /dev/null +++ b/lib/knowledge_base/permissions_update.rb @@ -0,0 +1,97 @@ +# Copyright (C) 2012-2022 Zammad Foundation, https://zammad-foundation.org/ + +class KnowledgeBase + class PermissionsUpdate + def initialize(object, user = nil) + @object = object + @user = user + end + + def update!(**roles_to_permissions) + ActiveRecord::Base.transaction do + update_object(roles_to_permissions) + + next if !@object.changed_for_autosave? + + @object.save! + + update_all_children + ensure_editable! + end + end + + def update_using_params!(params) + roles_to_permissions = params[:permissions].transform_keys { |key| Role.find key } + + update!(**roles_to_permissions) + end + + private + + def update_object(roles_to_permissions) + @object.permissions.reject { |elem| roles_to_permissions.key? elem.role }.each(&:mark_for_destruction) + + roles_to_permissions.each do |role, access| + update_object_permission(role, access) + end + end + + def update_object_permission(role, access) + permission = @object.permissions.detect { |elem| elem.role == role } || @object.permissions.build(role: role) + permission.access = access + + mark_permission_for_cleanup_if_needed(permission, parent_object_permissions) + end + + def parent_object_permissions + @parent_object_permissions ||= begin + if @object.is_a? KnowledgeBase::Category + (@object.parent || @object.knowledge_base).permissions_effective || [] + else + [] + end + end + end + + def all_children + case @object + when KnowledgeBase::Category + @object.self_with_children - [@object] + when KnowledgeBase + @object.categories.root.map(&:self_with_children).flatten + end + end + + def update_single_child(child) + inherited_permissions = child.permissions_effective + + child.permissions.each do |child_permission| + mark_permission_for_cleanup_if_needed(child_permission, inherited_permissions) + end + + child.changed_for_autosave? ? child.save! : child.touch # rubocop:disable Rails/SkipsModelValidations + end + + def update_all_children + all_children.each do |child| + update_single_child(child) + end + end + + def ensure_editable! + return if !@user + return if KnowledgeBase::EffectivePermission.new(@user, @object).access_effective == 'editor' + + raise Exceptions::UnprocessableEntity, __('Invalid permissions, do not lock out yourself') + end + + def mark_permission_for_cleanup_if_needed(permission, parents) + matching = parents.find { |elem| elem.role == permission.role } + + return if !matching + return if matching.access != permission.access && matching.access != 'none' + + permission.mark_for_destruction + end + end +end diff --git a/spec/factories/knowledge_base_permissions.rb b/spec/factories/knowledge_base_permissions.rb new file mode 100644 index 000000000..620118071 --- /dev/null +++ b/spec/factories/knowledge_base_permissions.rb @@ -0,0 +1,9 @@ +# Copyright (C) 2012-2022 Zammad Foundation, https://zammad-foundation.org/ + +FactoryBot.define do + factory 'knowledge_base/permission', aliases: %i[knowledge_base_permission] do + permissionable { create(:knowledge_base_category) } + role { create(:role) } + access { 'editor' } + end +end diff --git a/spec/factories/role.rb b/spec/factories/role.rb index d6e379a09..fce02c702 100644 --- a/spec/factories/role.rb +++ b/spec/factories/role.rb @@ -6,6 +6,12 @@ FactoryBot.define do created_by_id { 1 } updated_by_id { 1 } + transient do + permission_names { nil } + end + + permissions { Permission.where(name: permission_names) } + factory :agent_role do permissions { Permission.where(name: 'ticket.agent') } end diff --git a/spec/lib/knowledge_base/category/permission_spec.rb b/spec/lib/knowledge_base/category/permission_spec.rb new file mode 100644 index 000000000..7b549c722 --- /dev/null +++ b/spec/lib/knowledge_base/category/permission_spec.rb @@ -0,0 +1,73 @@ +# Copyright (C) 2012-2022 Zammad Foundation, https://zammad-foundation.org/ + +require 'rails_helper' + +RSpec.describe KnowledgeBase::Category::Permission do + include_context 'basic Knowledge Base' + + describe '#permissions_effective' do + let(:child_category) { create(:knowledge_base_category, knowledge_base: knowledge_base, parent: category) } + let(:child_permission) { create(:knowledge_base_permission, permissionable: child_category, access: 'reader') } + let(:parent_permission) { create(:knowledge_base_permission, permissionable: category) } + + context 'when no permissions exist' do + it 'parent category returns nil' do + expect(described_class.new(category).permissions_effective).to be_blank + end + + it 'child category returns nil' do + expect(described_class.new(child_category).permissions_effective).to be_blank + end + end + + context 'when parent category has permissions' do + before { parent_permission && child_category } + + it 'parent category returns parent permission' do + expect(described_class.new(category).permissions_effective).to eq [parent_permission] + end + + it 'child category returns parent permission' do + expect(described_class.new(child_category).permissions_effective).to eq [parent_permission] + end + end + + context 'when child category has permissions' do + before { child_permission } + + it 'parent category returns parent permission' do + expect(described_class.new(category).permissions_effective).to be_blank + end + + it 'child category returns parent permission' do + expect(described_class.new(child_category).permissions_effective).to eq [child_permission] + end + end + + context 'when both parent and child categories have permissions' do + before { child_permission && parent_permission } + + it 'parent category returns parent permission' do + expect(described_class.new(category).permissions_effective).to eq [parent_permission] + end + + it 'child category returns parent permission' do + expect(described_class.new(child_category).permissions_effective).to eq [child_permission, parent_permission] + end + end + + context 'when both parent child categories have colliding permissions for the same role' do + let(:child_permission_on_same_role) { create(:knowledge_base_permission, permissionable: child_category, access: 'reader', role: parent_permission.role) } + + before { parent_permission && child_permission && child_permission_on_same_role } + + it 'parent category returns parent permission' do + expect(described_class.new(category).permissions_effective).to eq [parent_permission] + end + + it 'child category returns child permission override' do + expect(described_class.new(child_category).permissions_effective).to eq [child_permission, child_permission_on_same_role] + end + end + end +end diff --git a/spec/lib/knowledge_base/effective_permission_spec.rb b/spec/lib/knowledge_base/effective_permission_spec.rb new file mode 100644 index 000000000..990c98179 --- /dev/null +++ b/spec/lib/knowledge_base/effective_permission_spec.rb @@ -0,0 +1,91 @@ +# Copyright (C) 2012-2022 Zammad Foundation, https://zammad-foundation.org/ + +require 'rails_helper' + +RSpec.describe KnowledgeBase::EffectivePermission do + include_context 'basic Knowledge Base' + + describe '#access_effective' do + let(:role_editor) { create(:role, permission_names: 'knowledge_base.editor') } + let(:role_reader) { create(:role, permission_names: 'knowledge_base.reader') } + let(:role_non_kb) { create(:role, :admin) } + + let(:user) { create(:user, roles: [role_editor, role_reader, role_non_kb]) } + let(:user_editor) { create(:user, roles: [role_editor]) } + let(:user_admin) { create(:admin) } + let(:user_reader) { create(:user, roles: [role_reader]) } + let(:user_nonkb) { create(:user, roles: [role_non_kb]) } + + let(:child_category) { create(:knowledge_base_category, parent: category) } + + it 'editor with no permissions defined returns editor' do + expect(described_class.new(user_editor, category).access_effective).to eq 'editor' + end + + it 'user with multiple permissions defined returns editor' do + expect(described_class.new(user, category).access_effective).to eq 'editor' + end + + it 'reader with no permissions defined returns reader' do + expect(described_class.new(user_reader, category).access_effective).to eq 'reader' + end + + it 'non-kb with no permissions defined returns none' do + expect(described_class.new(user_nonkb, category).access_effective).to eq 'none' + end + + it 'editor with both reader and editor permissions returns editor' do + create_permission(role_reader, 'reader') + create_permission(role_editor, 'editor') + + expect(described_class.new(user_admin, category).access_effective).to eq 'editor' + end + + it 'editor with reader permission on parent category returns reader' do + create_permission(role_editor, 'reader') + + expect(described_class.new(user_editor, child_category).access_effective).to eq 'reader' + end + + it 'editor with reader permission on KB returns reader' do + create_permission(role_editor, 'reader', permissionable: knowledge_base) + + expect(described_class.new(user_editor, category).access_effective).to eq 'reader' + end + + it 'editor with reader permission on parent category but editor permission on category returns editor' do + create_permission(role_editor, 'reader', permissionable: category) + create_permission(role_editor, 'editor', permissionable: child_category) + + expect(described_class.new(user_editor, child_category).access_effective).to eq 'editor' + end + + it 'editor with editor permission on parent category but reader permission on category returns reader' do + create_permission(role_editor, 'editor', permissionable: category) + create_permission(role_editor, 'reader', permissionable: child_category) + + expect(described_class.new(user_editor, child_category).access_effective).to eq 'reader' + end + + it 'reader with reader and non-effective permissions returns reader' do + create_permission(role_reader, 'reader') + create_permission(role_editor, 'editor') + + expect(described_class.new(user_reader, category).access_effective).to eq 'reader' + end + + it 'reader with no matching permissions returns reader' do + create_permission(role_editor, 'editor') + + expect(described_class.new(user_reader, category).access_effective).to eq 'reader' + end + + it 'retuns none when user not given' do + expect(described_class.new(nil, category).access_effective).to eq 'none' + end + end + + def create_permission(role, access, permissionable: category) + create(:knowledge_base_permission, role: role, permissionable: permissionable, access: access) + end +end diff --git a/spec/lib/knowledge_base/internal_assets_spec.rb b/spec/lib/knowledge_base/internal_assets_spec.rb new file mode 100644 index 000000000..d9599f8fd --- /dev/null +++ b/spec/lib/knowledge_base/internal_assets_spec.rb @@ -0,0 +1,87 @@ +# Copyright (C) 2012-2022 Zammad Foundation, https://zammad-foundation.org/ + +require 'rails_helper' + +RSpec.describe KnowledgeBase::InternalAssets do + include_context 'basic Knowledge Base' do + before do + draft_answer + internal_answer + published_answer + end + end + + describe '#collect_assets' do + subject(:assets) { described_class.new(user).collect_assets } + + context 'when for KB editor' do + let(:user) { create(:user, roles: Role.where(name: 'Admin')) } + + it 'returns assets for all KB objects' do + expect(assets).to include_assets_of(knowledge_base, category, draft_answer, internal_answer, published_answer) + end + + context 'when has editor permission' do + before do + KnowledgeBase::PermissionsUpdate.new(category).update! user.roles.first => 'editor' + end + + it 'returns assets for all KB objects' do + expect(assets).to include_assets_of(knowledge_base, category, draft_answer, internal_answer, published_answer) + end + end + + context 'when has reader permission' do + before do + KnowledgeBase::PermissionsUpdate.new(category).update! user.roles.first => 'reader' + end + + it 'returns assets for internally visible KB objects' do + expect(assets) + .to include_assets_of(knowledge_base, category, internal_answer, published_answer) + .and not_include_assets_of(draft_answer) + end + end + + context 'when has none permission' do + before do + KnowledgeBase::PermissionsUpdate.new(category).update! user.roles.first => 'none' + end + + it 'does not return assets for internally visible KB objects' do + published_answer.destroy # make sure public item does not make category visible + + expect(assets) + .to include_assets_of(knowledge_base) + .and not_include_assets_of(category, draft_answer, internal_answer, published_answer) + end + + it 'returns assets for published answer and it\'s category' do + expect(assets) + .to include_assets_of(knowledge_base, category, published_answer) + .and not_include_assets_of(draft_answer, internal_answer) + end + end + end + + context 'when for agent' do + let(:user) { create(:agent) } + + it 'returns assets for all KB objects' do + expect(assets) + .to include_assets_of(knowledge_base, category, internal_answer, published_answer) + .and not_include_assets_of(draft_answer) + end + end + + context 'when for customer' do + let(:user) { create(:customer) } + + it 'returns assets for all KB objects' do + expect(assets) + .to include_assets_of(knowledge_base) + .and not_include_assets_of(category, draft_answer, internal_answer, published_answer) + end + end + end +end diff --git a/spec/lib/knowledge_base/permissions_update_spec.rb b/spec/lib/knowledge_base/permissions_update_spec.rb new file mode 100644 index 000000000..7af4ae3a0 --- /dev/null +++ b/spec/lib/knowledge_base/permissions_update_spec.rb @@ -0,0 +1,180 @@ +# Copyright (C) 2012-2022 Zammad Foundation, https://zammad-foundation.org/ + +require 'rails_helper' + +RSpec.describe KnowledgeBase::PermissionsUpdate do + describe '#update!' do + include_context 'basic Knowledge Base' + let(:role_editor) { create(:role, permission_names: %w[knowledge_base.editor]) } + let(:role_another) { create(:role, permission_names: %w[knowledge_base.editor]) } + let(:child_category) { create(:knowledge_base_category, parent: category) } + + describe 'updating itself' do + shared_examples 'updating itself' do |object_name:| + let(:object) { send(object_name) } + it 'adds role permission for self' do + described_class.new(object).update! role_editor => 'editor' + + expect(object.permissions) + .to contain_exactly have_attributes(permissionable: object, role: role_editor, access: 'editor') + end + + it 'adds additional role permission for self' do + described_class.new(object).update! role_editor => 'reader' + described_class.new(object).update! role_editor => 'reader', role_another => 'reader' + + expect(object.permissions) + .to contain_exactly(have_attributes(role: role_editor), have_attributes(role: role_another)) + end + + it 'does not update when re-adding an existing permission' do + described_class.new(object).update! role_editor => 'reader' + + travel 1.minute + + expect { described_class.new(object).update! role_editor => 'reader' } + .not_to change(object, :updated_at) + end + end + + context 'when saving role on KB itself' do + include_context 'updating itself', object_name: :knowledge_base + end + + context 'when saving role on KB category' do + include_context 'updating itself', object_name: :category + end + end + + describe 'updating descendants' do + context 'when saving role on KB itself' do + it 'adds effective permissions to descendant categories' do + described_class.new(knowledge_base).update! role_editor => 'reader' + + expect(category.permissions_effective) + .to contain_exactly have_attributes(role: role_editor, access: 'reader', permissionable: knowledge_base) + end + + it 'removing permission opens up access to descendants' do + described_class.new(knowledge_base).update! role_editor => 'editor' + described_class.new(knowledge_base).update!(**{}) + + expect(category.permissions_effective).to be_blank + end + + context 'when category has editor role has editor role with editor permission' do + before do + described_class.new(category).update! role_editor => 'editor' + category.reload + + travel 1.minute + end + + it 'removes identical permissions on descendant roles' do + described_class.new(knowledge_base).update! role_editor => 'editor' + category.reload + + expect(category.permissions_effective) + .to contain_exactly have_attributes(role: role_editor, access: 'editor', permissionable: knowledge_base) + end + end + end + + context 'when saving role on KB category' do + it 'adds effective permissions to descendant roles' do + described_class.new(category).update! role_editor => 'reader' + + expect(child_category.permissions_effective) + .to contain_exactly have_attributes(role: role_editor, access: 'reader', permissionable: category) + end + + context 'when child category has editor role with editor permission' do + before do + described_class.new(child_category).update! role_editor => 'editor' + category.reload + child_category.reload + + travel 1.minute + end + + it 'removes conflicting permissions on descendant roles' do + described_class.new(category).update! role_editor => 'none' + category.reload + child_category.reload + + expect(child_category.permissions_effective) + .to contain_exactly have_attributes(role: role_editor, access: 'none', permissionable: category) + end + + it 'removes identical permissions on descendant roles' do + described_class.new(category).update! role_editor => 'editor' + category.reload + child_category.reload + + expect(child_category.permissions_effective) + .to contain_exactly have_attributes(role: role_editor, access: 'editor', permissionable: category) + end + end + + context 'when category has role editor with none permission' do + before do + described_class.new(category).update! role_editor => 'none' + category.reload + + travel 1.minute + end + + it 'removing permission opens up access to descendants' do + described_class.new(category).update!(**{}) + category.reload + + expect(child_category.permissions_effective).to be_blank + end + end + end + end + + describe 'preventing user lockout' do + let(:user) { create(:admin) } + let(:role) { user.roles.first } + + shared_examples 'preventing user lockout' do |object_name:| + let(:object) { send(object_name) } + + it 'raises an error when saving a lockout change for a given user' do + expect { described_class.new(object, user).update! role => 'reader' } + .to raise_error(Exceptions::UnprocessableEntity) + end + + it 'allows to save same change without a user' do + expect { described_class.new(object).update! role => 'reader' }.not_to raise_error + end + end + + context 'when saving role on KB itself' do + include_context 'preventing user lockout', object_name: 'knowledge_base' + end + + context 'when saving role on KB category' do + include_context 'preventing user lockout', object_name: 'category' + end + end + end + + describe '#update_using_params!' do + subject(:updater) { described_class.new(category) } + + let(:role) { create(:role, permission_names: %w[knowledge_base.editor]) } + let(:category) { create(:knowledge_base_category) } + + it 'calls update! with given roles' do + updater.update_using_params!({ permissions: { role.id => 'editor' } }) + expect(category.permissions.first).to have_attributes(role: role, access: 'editor', permissionable: category) + end + + it 'raises an error when given a non existant role' do + expect { updater.update_using_params!({ permissions: { (role.id + 1) => 'editor' } }) } + .to raise_error(ActiveRecord::RecordNotFound) + end + end +end diff --git a/spec/models/knowledge_base/category_spec.rb b/spec/models/knowledge_base/category_spec.rb index 66c237067..22aebaa0d 100644 --- a/spec/models/knowledge_base/category_spec.rb +++ b/spec/models/knowledge_base/category_spec.rb @@ -16,6 +16,7 @@ RSpec.describe KnowledgeBase::Category, type: :model, current_user_id: 1 do it { is_expected.to have_many(:answers) } it { is_expected.to have_many(:children) } + it { is_expected.to have_many(:permissions) } it { is_expected.to belong_to(:parent).optional } it { is_expected.to belong_to(:knowledge_base) } diff --git a/spec/models/knowledge_base/permission_spec.rb b/spec/models/knowledge_base/permission_spec.rb new file mode 100644 index 000000000..099fca702 --- /dev/null +++ b/spec/models/knowledge_base/permission_spec.rb @@ -0,0 +1,50 @@ +# Copyright (C) 2012-2022 Zammad Foundation, https://zammad-foundation.org/ + +require 'rails_helper' +require 'models/contexts/factory_context' + +RSpec.describe KnowledgeBase::Permission, type: :model do + subject(:kb_category_permission) { create(:knowledge_base_permission) } + + include_context 'basic Knowledge Base' + include_context 'factory' + + describe '#permissionable' do + it { is_expected.to belong_to(:permissionable).touch(true) } + + it 'allows multiple permissions for the same category' do + permission = build(:knowledge_base_permission, permissionable: kb_category_permission.permissionable) + permission.save + + expect(permission).to be_persisted + end + + it 'does not allow same role/permission conbination' do + permission = build(:knowledge_base_permission, + permissionable: kb_category_permission.permissionable, + role: kb_category_permission.role) + permission.save + + expect(permission).not_to be_persisted + end + end + + describe '#role' do + it { is_expected.to belong_to(:role) } + + it 'allows multiple permissions for the same category' do + permission = build(:knowledge_base_permission, role: kb_category_permission.role) + permission.save + + expect(permission).to be_persisted + end + end + + describe '#access' do + it { is_expected.to validate_presence_of(:access).with_message(%r{}) } + it { is_expected.to allow_value('editor').for(:access) } + it { is_expected.to allow_value('reader').for(:access) } + it { is_expected.to allow_value('none').for(:access) } + it { is_expected.not_to allow_value('foobar').for(:access) } + end +end diff --git a/spec/policies/knowledge_base/answer_policy_spec.rb b/spec/policies/knowledge_base/answer_policy_spec.rb new file mode 100644 index 000000000..9224b274a --- /dev/null +++ b/spec/policies/knowledge_base/answer_policy_spec.rb @@ -0,0 +1,64 @@ +# Copyright (C) 2012-2022 Zammad Foundation, https://zammad-foundation.org/ + +require 'rails_helper' +require 'policies/knowledge_base_policy_examples' + +describe KnowledgeBase::AnswerPolicy do + subject(:policy) { described_class.new(user, record) } + + let(:record) { create(:knowledge_base_answer) } + let(:user) { create(:user) } + + shared_context 'with answer visibility' do |visible:, visible_internally:| + before do + allow(record).to receive(:visible?).and_return(visible) + allow(record).to receive(:visible_internally?).and_return(visible_internally) + end + end + + describe '#show?' do + context 'when visible and visible internally' do + include_examples 'with answer visibility', visible: true, visible_internally: true + include_examples 'with KB policy check', editor: true, reader: true, none: true, method: :show? + end + + context 'when visible internally only' do + include_examples 'with answer visibility', visible: false, visible_internally: true + include_examples 'with KB policy check', editor: true, reader: true, none: false, method: :show? + end + + context 'when not visible' do + include_examples 'with answer visibility', visible: false, visible_internally: false + include_examples 'with KB policy check', editor: true, reader: false, none: false, method: :show? + end + end + + describe '#show_public?' do + context 'when visible and visible internally' do + include_examples 'with answer visibility', visible: true, visible_internally: true + include_examples 'with KB policy check', editor: true, reader: true, none: true, method: :show_public? + end + + context 'when visible internally only' do + include_examples 'with answer visibility', visible: false, visible_internally: true + include_examples 'with KB policy check', editor: true, reader: false, none: false, method: :show_public? + end + + context 'when not visible' do + include_examples 'with answer visibility', visible: false, visible_internally: false + include_examples 'with KB policy check', editor: true, reader: false, none: false, method: :show_public? + end + end + + describe '#update?' do + include_examples 'with KB policy check', editor: true, reader: false, none: false, method: :update? + end + + describe '#create?' do + include_examples 'with KB policy check', editor: true, reader: false, none: false, method: :create? + end + + describe '#destroy?' do + include_examples 'with KB policy check', editor: true, reader: false, none: false, method: :destroy? + end +end diff --git a/spec/policies/knowledge_base/category_policy_spec.rb b/spec/policies/knowledge_base/category_policy_spec.rb new file mode 100644 index 000000000..2a483022e --- /dev/null +++ b/spec/policies/knowledge_base/category_policy_spec.rb @@ -0,0 +1,45 @@ +# Copyright (C) 2012-2022 Zammad Foundation, https://zammad-foundation.org/ + +require 'rails_helper' +require 'policies/knowledge_base_policy_examples' + +describe KnowledgeBase::CategoryPolicy do + subject(:policy) { described_class.new(user, record) } + + let(:record) { create(:knowledge_base_category) } + let(:user) { create(:user) } + + describe '#show?' do + include_examples 'with KB policy check', editor: true, reader: true, none: false, method: :show? + end + + describe '#show_public?' do + context 'when category has public content' do + before { allow(record).to receive(:public_content?).and_return(true) } + + include_examples 'with KB policy check', editor: true, reader: true, none: true, method: :show_public? + end + + context 'when category has no public content' do + before { allow(record).to receive(:public_content?).and_return(false) } + + include_examples 'with KB policy check', editor: true, reader: false, none: false, method: :show_public? + end + end + + describe '#permissions?' do + include_examples 'with KB policy check', editor: true, reader: false, none: false, method: :permissions? + end + + describe '#update?' do + include_examples 'with KB policy check', editor: true, reader: false, none: false, method: :update? + end + + describe '#create?' do + include_examples 'with KB policy check', editor: true, reader: false, none: false, method: :create?, access_method: :parent_access + end + + describe '#destroy?' do + include_examples 'with KB policy check', editor: true, reader: false, none: false, method: :destroy?, access_method: :parent_access + end +end diff --git a/spec/policies/knowledge_base_policy_examples.rb b/spec/policies/knowledge_base_policy_examples.rb new file mode 100644 index 000000000..db5229980 --- /dev/null +++ b/spec/policies/knowledge_base_policy_examples.rb @@ -0,0 +1,29 @@ +# Copyright (C) 2012-2022 Zammad Foundation, https://zammad-foundation.org/ + +RSpec.shared_context 'with KB policy check' do |editor:, reader:, none:, method:, access_method: :access| + let(:access_method) { access_method } + + it 'returns true if editor' do + mock_permission 'editor' + + expect(policy.send(method)).to be editor + end + + it 'returns true if reader' do + mock_permission 'reader' + + expect(policy.send(method)).to be reader + end + + it 'returns false if none' do + mock_permission 'none' + + expect(policy.send(method)).to be none + end + + def mock_permission(access) + allow(policy) + .to receive(access_method) + .and_return(access) + end +end diff --git a/spec/policies/knowledge_base_policy_spec.rb b/spec/policies/knowledge_base_policy_spec.rb new file mode 100644 index 000000000..995c033c7 --- /dev/null +++ b/spec/policies/knowledge_base_policy_spec.rb @@ -0,0 +1,19 @@ +# Copyright (C) 2012-2022 Zammad Foundation, https://zammad-foundation.org/ + +require 'rails_helper' +require 'policies/knowledge_base_policy_examples' + +describe KnowledgeBasePolicy do + subject(:policy) { described_class.new(user, record) } + + let(:record) { create(:knowledge_base) } + let(:user) { create(:user) } + + describe '#show?' do + include_examples 'with KB policy check', editor: true, reader: true, none: false, method: :show? + end + + describe 'update?' do + include_examples 'with KB policy check', editor: true, reader: false, none: false, method: :update? + end +end diff --git a/spec/support/assets_matchers.rb b/spec/support/assets_matchers.rb index a89401c4d..950397c40 100644 --- a/spec/support/assets_matchers.rb +++ b/spec/support/assets_matchers.rb @@ -54,7 +54,9 @@ RSpec::Matchers.define :include_assets_of do # # @return [Hash, nil] def find_assets_of(object, actual) - actual.dig(object.class.name.gsub(%r{::}, ''), object.id.to_s) + actual + .deep_stringify_keys + .dig(object.class.name.gsub(%r{::}, ''), object.id.to_s) end end diff --git a/spec/system/knowledge_base/locale/answer/edit_spec.rb b/spec/system/knowledge_base/locale/answer/edit_spec.rb index 5542f1b40..d4d3b70e3 100644 --- a/spec/system/knowledge_base/locale/answer/edit_spec.rb +++ b/spec/system/knowledge_base/locale/answer/edit_spec.rb @@ -96,18 +96,6 @@ RSpec.describe 'Knowledge Base Locale Answer Edit', type: :system do let(:new_tag_name) { 'capybara_kb_tag' } it 'adds a new tag' do - within :active_content do - click '.js-newTagLabel' - - elem = find('.js-newTagInput') - elem.fill_in with: new_tag_name - elem.send_keys :return - - expect(page).to have_css('a.js-tag', text: new_tag_name) - end - end - - it 'saves new tag to the database' do within :active_content do click '.js-newTagLabel' @@ -116,6 +104,7 @@ RSpec.describe 'Knowledge Base Locale Answer Edit', type: :system do elem.send_keys :return wait.until_exists { published_answer_with_tag.reload.tag_list.include? new_tag_name } + expect(page).to have_css('a.js-tag', text: new_tag_name) end end @@ -127,22 +116,10 @@ RSpec.describe 'Knowledge Base Locale Answer Edit', type: :system do it 'deletes a tag' do within :active_content do - click '.js-newTagLabel' - find('.list-item', text: published_answer_tag_name) .find('.js-delete').click expect(page).to have_no_css('a.js-tag', text: published_answer_tag_name) - end - end - - it 'deletes the tag from the database' do - within :active_content do - click '.js-newTagLabel' - - find('.list-item', text: published_answer_tag_name) - .find('.js-delete').click - wait.until_exists { published_answer_with_tag.reload.tag_list.exclude? published_answer_tag_name } end end diff --git a/spec/system/knowledge_base/locale/category/permissions_spec.rb b/spec/system/knowledge_base/locale/category/permissions_spec.rb new file mode 100644 index 000000000..5e1fa81db --- /dev/null +++ b/spec/system/knowledge_base/locale/category/permissions_spec.rb @@ -0,0 +1,177 @@ +# Copyright (C) 2012-2022 Zammad Foundation, https://zammad-foundation.org/ + +require 'rails_helper' + +RSpec.describe 'Knowledge Base Locale Category Permissions', type: :system do + include_context 'basic Knowledge Base' + + let(:role_editor) { Role.find_by name: 'Admin' } + let(:role_another_editor) { create(:role, permission_names: %w[knowledge_base.editor]) } + let(:role_reader) { Role.find_by name: 'Agent' } + + let(:child_category) { create(:knowledge_base_category, parent: category) } + + it 'shows roles with has KB permissions only' do + open_page category + + find('[data-action=permissions]').click + + in_modal disappears: false do + expect(page) + .to have_text(%r{Admin}i) + .and(have_text(%r{Agent}i)) + .and(have_no_text(%r{Customer}i)) + end + end + + describe 'permissions shown' do + it 'shows existing permissions when category has no permissions' do + open_page category + + find('[data-action=permissions]').click + + in_modal disappears: false do + expect(page) + .to have_css("input[name='#{role_editor.id}'][value='editor'][checked]:not([disabled])", visible: :all) + .and(have_css("input[name='#{role_editor.id}'][value='reader']:not([disabled])", visible: :all)) + .and(have_css("input[name='#{role_editor.id}'][value='none']:not([disabled])", visible: :all)) + end + end + + it 'shows existing permissions when category has inerited permissions only' do + KnowledgeBase::PermissionsUpdate.new(category).update! role_reader => 'none' + + open_page category + + find('[data-action=permissions]').click + + in_modal disappears: false do + expect(page) + .to have_css("input[name='#{role_reader.id}'][value='reader']:not([disabled])", visible: :all) + .and(have_css("input[name='#{role_reader.id}'][value='none'][checked]:not([disabled])", visible: :all)) + end + end + + it 'shows existing permissions' do + KnowledgeBase::PermissionsUpdate.new(child_category).update! role_reader => 'none' + + open_page child_category + + find('[data-action=permissions]').click + + in_modal disappears: false do + expect(page) + .to have_css("input[name='#{role_reader.id}'][value='reader']:not([disabled])", visible: :all) + .and(have_css("input[name='#{role_reader.id}'][value='none'][checked]:not([disabled])", visible: :all)) + end + end + + it 'shows editor permission not limited by parent category being read only' do + KnowledgeBase::PermissionsUpdate.new(category).update! role_another_editor => 'reader' + + open_page child_category + + find('[data-action=permissions]').click + + in_modal disappears: false do + expect(page) + .to have_css("input[name='#{role_another_editor.id}'][value='none']:not([disabled])", visible: :all) + .and(have_css("input[name='#{role_another_editor.id}'][value='reader'][checked]:not([disabled])", visible: :all)) + .and(have_css("input[name='#{role_another_editor.id}'][value='editor'][disabled]", visible: :all)) + end + end + + it 'shows editor permissions limited by parent category' do + KnowledgeBase::PermissionsUpdate.new(category).update! role_another_editor => 'none' + + open_page child_category + + find('[data-action=permissions]').click + + in_modal disappears: false do + expect(page) + .to have_css("input[name='#{role_another_editor.id}'][value='none'][checked]:not([disabled])", visible: :all) + .and(have_css("input[name='#{role_another_editor.id}'][value='reader'][disabled]", visible: :all)) + .and(have_css("input[name='#{role_another_editor.id}'][value='editor'][disabled]", visible: :all)) + end + end + + it 'shows reader permissions limited by parent category' do + KnowledgeBase::PermissionsUpdate.new(category).update! role_reader => 'none' + + open_page child_category + + find('[data-action=permissions]').click + + in_modal disappears: false do + expect(page) + .to have_css("input[name='#{role_reader.id}'][value='none'][checked]:not([disabled])", visible: :all) + .and(have_css("input[name='#{role_reader.id}'][value='reader'][disabled]", visible: :all)) + end + end + end + + describe 'saving changes' do + it 'saves permissions' do + open_page category + + find('[data-action=permissions]').click + + in_modal do + find("input[name='#{role_reader.id}'][value='none']", visible: :all) + .ancestor('label') + .click + + click_on 'Submit' + end + + expect(category.reload.permissions) + .to contain_exactly( + have_attributes(role: role_reader, access: 'none', permissionable: category), + have_attributes(role: role_editor, access: 'editor', permissionable: category) + ) + end + + it 'allows to modify existing permissions' do + KnowledgeBase::PermissionsUpdate.new(category).update! role_reader => 'none' + + open_page category + + find('[data-action=permissions]').click + + in_modal do + find("input[name='#{role_reader.id}'][value='reader']", visible: :all) + .ancestor('label') + .click + + click_on 'Submit' + end + + expect(category.reload.permissions) + .to contain_exactly( + have_attributes(role: role_reader, access: 'reader', permissionable: category), + have_attributes(role: role_editor, access: 'editor', permissionable: category) + ) + end + + it 'does not allow to lock user himself' do + open_page category + + find('[data-action=permissions]').click + + in_modal disappears: false do + find("input[name='#{role_editor.id}'][value='reader']", visible: :all) + .ancestor('label') + .click + + click_on 'Submit' + + expect(page).to have_css('.alert') + end + end + end + + def open_page(category) + visit "knowledge_base/#{knowledge_base.id}/locale/#{Locale.first.locale}/category/#{category.id}/edit" + end +end diff --git a/spec/system/knowledge_base/locale/knowledge_base/permissions_spec.rb b/spec/system/knowledge_base/locale/knowledge_base/permissions_spec.rb new file mode 100644 index 000000000..afe9ba07d --- /dev/null +++ b/spec/system/knowledge_base/locale/knowledge_base/permissions_spec.rb @@ -0,0 +1,118 @@ +# Copyright (C) 2012-2022 Zammad Foundation, https://zammad-foundation.org/ + +require 'rails_helper' + +RSpec.describe 'Knowledge Base Locale Knowledge Base Permissions', type: :system do + include_context 'basic Knowledge Base' + + let(:role_editor) { Role.find_by name: 'Admin' } + let(:role_another_editor) { create(:role, permission_names: %w[knowledge_base.editor]) } + let(:role_reader) { Role.find_by name: 'Agent' } + + it 'shows roles with has KB permissions only' do + open_page + + find('[data-action=permissions]').click + + in_modal disappears: false do + expect(page) + .to have_text(%r{Admin}i) + .and(have_text(%r{Agent}i)) + .and(have_no_text(%r{Customer}i)) + end + end + + describe 'permissions shown' do + it 'shows existing permissions when KB has no permissions' do + open_page + + find('[data-action=permissions]').click + + in_modal disappears: false do + expect(page) + .to have_css("input[name='#{role_editor.id}'][value='editor'][checked]", visible: :all) + .and(have_css("input[name='#{role_editor.id}'][value='reader']", visible: :all)) + end + end + + it 'shows existing permissions' do + KnowledgeBase::PermissionsUpdate.new(knowledge_base).update! role_another_editor => 'reader' + + open_page + + find('[data-action=permissions]').click + + in_modal disappears: false do + expect(page) + .to have_css("input[name='#{role_another_editor.id}'][value='reader'][checked]", visible: :all) + .and(have_css("input[name='#{role_another_editor.id}'][value='editor']", visible: :all)) + end + end + end + + describe 'saving changes' do + it 'saves permissions' do + role_another_editor + open_page + + find('[data-action=permissions]').click + + in_modal do + find("input[name='#{role_another_editor.id}'][value='reader']", visible: :all) + .ancestor('label') + .click + + click_on 'Submit' + end + + expect(knowledge_base.reload.permissions) + .to contain_exactly( + have_attributes(role: role_reader, access: 'reader', permissionable: knowledge_base), + have_attributes(role: role_another_editor, access: 'reader', permissionable: knowledge_base), + have_attributes(role: role_editor, access: 'editor', permissionable: knowledge_base) + ) + end + + it 'allows to modify existing permissions' do + KnowledgeBase::PermissionsUpdate.new(knowledge_base).update! role_another_editor => 'reader' + open_page + + find('[data-action=permissions]').click + + in_modal do + find("input[name='#{role_another_editor.id}'][value='editor']", visible: :all) + .ancestor('label') + .click + + click_on 'Submit' + end + + expect(knowledge_base.reload.permissions) + .to contain_exactly( + have_attributes(role: role_reader, access: 'reader', permissionable: knowledge_base), + have_attributes(role: role_another_editor, access: 'editor', permissionable: knowledge_base), + have_attributes(role: role_editor, access: 'editor', permissionable: knowledge_base) + ) + end + + it 'does not allow to lock user himself' do + open_page + + find('[data-action=permissions]').click + + in_modal disappears: false do + find("input[name='#{role_editor.id}'][value='reader']", visible: :all) + .ancestor('label') + .click + + click_on 'Submit' + + expect(page).to have_css('.alert') + end + end + end + + def open_page + visit "knowledge_base/#{knowledge_base.id}/locale/#{Locale.first.locale}/edit" + end +end diff --git a/spec/system/ticket/zoom_spec.rb b/spec/system/ticket/zoom_spec.rb index f6f3a4ba9..bd5bb44bb 100644 --- a/spec/system/ticket/zoom_spec.rb +++ b/spec/system/ticket/zoom_spec.rb @@ -939,7 +939,6 @@ RSpec.describe 'Ticket zoom', type: :system do def forward within :active_content do - # binding.pry wait.until_exists { find('.textBubble-content .richtext-content') } click '.js-ArticleAction[data-type=emailForward]' fill_in 'To', with: 'customer@example.com'