Cookiebot should be showing for Administrators too

Created on 26 October 2023, about 1 year ago
Updated 20 November 2023, about 1 year ago

Problem/Motivation

In a project, we have some users who unfortunately needs to use the Administrator role. Also, we have some functionalities which are not loaded at all without the given cookie consent.

Cookiebot checks if a user has the 'skip cookiebot consent' permission, and if they have, bails out without loading the functionality. The problem is that the Administrator role has implicitly all the permissions, so it's not currently possible for them to give the consent, to make some functionalities work.

Steps to reproduce

Have an Administrator user, and try to use the Cookiebot functionality.

Proposed resolution

Make it configurable if Cookiebot should be visible to all users alike. This would be a backwards compatible change so that no functionality would change unless you actually configure it.

User interface changes

Adds a checkbox option to the configuration form; makes Cookiebot optionally work for all users, even Administrators.

Feature request
Status

RTBC

Version

1.0

Component

Code

Created by

🇫🇮Finland jhuhta

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

Comments & Activities

  • Issue created by @jhuhta
  • Issue was unassigned.
  • Status changed to Needs review about 1 year ago
  • Open on Drupal.org →
    Core: 10.1.4 + Environment: PHP 8.1 & MySQL 5.7
    last update about 1 year ago
    Waiting for branch to pass
  • 🇫🇮Finland jhuhta

    This should provide a solution. Sorry for the mostly bogus diff for the schema.yml, the original had dos newlines.

  • 🇧🇪Belgium BramDriesen Belgium 🇧🇪

    Wouldn't it be better to do the opposite of what you're trying to do with the patch?

    - Create a checkbox "Don't display for users with the admin role"
    - Remove the is User 1 check
    - Wrap the code similar to like what you're doing with the current patch

    Open to discussion of course 😇. I'm not a fan of having user 1 checks.

  • 🇫🇮Finland jhuhta

    Me neither. I just wanted to create as little intrusive change as possible, to spare the need for warning users or creating update hooks. I don't really get the point of hardcoding uid 1 there, so if you think, as a maintainer, that refactoring that would be a better approach, I can probably take another look at it later.

  • 🇧🇪Belgium BramDriesen Belgium 🇧🇪

    I think it makes much more sense to replace the exclude_uid_1 with exclude_administrator_role or even more dynamic that you could select multiple roles. It will indeed take a bit more effort (updating the schema and an update hook to process it) but I think that's the more future proof way to go 😇.

  • Open on Drupal.org →
    Core: 10.1.4 + Environment: PHP 8.1 & MySQL 5.7
    last update about 1 year ago
    Waiting for branch to pass
  • 🇫🇮Finland jhuhta

    How about this then? I decided to abandon the permission based approach completely and use role based one. This is maybe not an optimal way to do Drupal features, but I do think this is way better than before anyway (thanks for the suggestion!) and the permission based one just can't work for admins. This is clean and straightforward, and the admin can decide the roles which are affected, if any.

    Also, the update path should work. Please review.

  • 🇫🇮Finland pontus.talvikarhu

    Fix has been tested and works as intended.

  • Status changed to RTBC about 1 year ago
  • Open on Drupal.org →
    Core: 10.1.4 + Environment: PHP 8.1 & MySQL 5.7
    last update about 1 year ago
    Waiting for branch to pass
  • 🇫🇮Finland jhuhta

    Fixing a missing import. Works even better now.

  • The last submitted patch, 9: 3396895-cookiebot-for-admins-9.patch, failed testing. View results
    - codesniffer_fixes.patch Interdiff of automated coding standards fixes only.

  • Status changed to Needs work about 1 year ago
  • 🇫🇮Finland pontus.talvikarhu

    Patch addressing the very small coding standards errors the tests brought up.

  • Status changed to Needs review about 1 year ago
  • Open on Drupal.org →
    Core: 10.1.4 + Environment: PHP 8.1 & MySQL 5.7
    last update about 1 year ago
    Waiting for branch to pass
  • Status changed to RTBC about 1 year ago
  • 🇫🇮Finland jhuhta

    I'm RTBC'ing the last change by @pontus.talvikarhu - maintainer consideration is ofc needed too as I wrote the original patch.

  • 🇫🇮Finland sokru

    Apparently this is not so rare usecase. +1 for RTBC. Included a related issue that should be closed if this gets fixed.

  • 🇧🇪Belgium BramDriesen Belgium 🇧🇪

    Looks good to me. Will leave it a RTBC for a bit in case another maintainer wants to chime in. Closed the other issue as well as this one has the fix I suggested there as well.

Production build 0.71.5 2024