WProofreader does not respect its permission to administer its config

Created on 5 May 2024, about 2 months ago
Updated 15 June 2024, 8 days ago

Problem/Motivation

In the permissions for CKEditor 5 Premium Features WProofreader, I have de-selected the permission: "Administer the CKEditor 5 WProofreader configuration", however any user with any role that can use the ck5 editor is still able to adjust the wproofreader settings by clicking on the Wproofreader text checker button.

As it is right now, one logged in user can turn off spell checking for anywhere the wproofreader is used (across the entire website).

Hence marking this issue as "Major".

Steps to reproduce

  1. Set up ck5 to use wproofreader. (You'll need a wproofreader service ID).
  2. Enable the wproofreader text checker button.
  3. De-select all roles from having the permission "Administer the CKEditor 5 WProofreader configuration".
  4. Log in with an authenticated role and click on the wproofreader text checker button when editing a content. Notice you can still adjust the settings even though you do not have the permissions to do so.

Please note - I am testing this on version 1.2.6, but I expect this bug to still exist in 1.2.8 as I have not noticed any fixes going in 1.2.8 around this.

✨ Feature request
Status

RTBC

Version

1.2

Component

Plugin: WProofreader

Created by

πŸ‡ΉπŸ‡ΉTrinidad and Tobago xamount

Live updates comments and jobs are added and updated live.
Sign in to follow issues

Merge Requests

Comments & Activities

  • Issue created by @xamount
  • πŸ‡΅πŸ‡±Poland salmonek

    Hi @xamount

    The "Administer the CKEditor 5 WProofreader configuration" applies to the main config form - /admin/config/ckeditor5-premium-features/wproofreader

    The settings form available directly in the editor is not affecting whole site. Those settings are stored in the browser's local storage, so if you log in to another user's account on the same browser then indeed it will retain the changes made on first user's account.
    If you log in on another browser or icognito mode then the settings will be again defaults.
    We can do an additional permission to disallow personal configuration (I'm not 100% sure if we can do it for whole form), however for us it doesn't look like a major bug.

  • πŸ‡ΉπŸ‡ΉTrinidad and Tobago xamount

    Ah I see thanks for the explanation. Yes you might be right about the local browser cache. Lowering the priority to Normal then.

    I guess what's needed here is a drupal permission to disallow users from being able to access the settings area when they click on the wproofreader text checker button. Is this possible?

    In my case, spell checking is mandatory and I do not want to allow site editors to be able to enable/disable it. I wrote a hook_form_alter to block the node edit form from saving if there are wproofreader detected errors. If site editors can then disable the wproofreader, then it defeats the purpose of the hook_form_alter validation check.

  • πŸ‡΅πŸ‡±Poland salmonek

    Here is a patch that adds 4 new permissions for each section in the plugin settings menu (dictionary, language, general and ignore).
    When user does not have access to any of sections, the settings menu won't be available at all.
    Additionally the local settings storage will be disabled for sections that are not available for user and default settings will apply. The default settings can be adjusted at /admin/config/ckeditor5-premium-features/wproofreader

    When this update will be released, we'll most probably set the hook_update to enable permissions by default for sites that already have module installed (so there won't be unexpected loss of functionality). In such case we'll provide info about that here before release of the feature.

  • Status changed to Needs work about 1 month ago
  • πŸ‡ΉπŸ‡ΉTrinidad and Tobago xamount

    Thanks @salmonek

    I create an issue fork that includes your patch and also:

    - fixed a few typos
    - fixed a php deprecation notice.

    I tested and everything works for far, but the main request of this ticket is still missing which is to disable to "toggle" functionality.

  • Status changed to Needs review about 1 month ago
  • πŸ‡΅πŸ‡±Poland salmonek

    Ok, pushed an update with additional permission.
    Thank you for the fixes.

  • Status changed to Needs work about 1 month ago
  • πŸ‡ΉπŸ‡ΉTrinidad and Tobago xamount

    I tested the patch and I can see the new config and it's working.

    I realised that with this patch, 2 specific functionalities have gone missing. I have highlighted them in the screenshot below.

    The one on the left "Ignore all" is most important. Without this, a user cannot manually ignore a word.

  • πŸ‡ΉπŸ‡ΉTrinidad and Tobago xamount

    Additionally, the "add to dictionary" button is missing.

  • Status changed to Needs review 10 days ago
  • πŸ‡΅πŸ‡±Poland salmonek

    @xamount

    I've restored those actions and pushed to the working branch.
    Ignore all and add to dictionary options are dependent on ignore/dictionary permissions.
    Today we've released a 1.2.9 verion, but it doesn't yet contain changes for this ticket, so patch still needed for now.

  • Status changed to RTBC 8 days ago
  • πŸ‡ΉπŸ‡ΉTrinidad and Tobago xamount

    Thank you so much! @salmonek

    I tested the latest MR in 1.2.8 and 1.2.9 and all works as expected now.

    I'll use the patch until this is merged. Do you know the ETA on 1.3.0?

Production build 0.69.0 2024