Problem/Motivation

Flag permissions for own items or profile have no effect

Steps to reproduce

Install Drupal 10.
Install the Flag module.
Expose additional permissions for Flag Bookmark and Flag Follower.
Do not give permissions to users to flag own items or own profile.
They can still do it.

Issue fork flag-3439272

Command icon Show commands

Start within a Git clone of the project using the version control instructions.

Or, if you do not have SSH keys set up on git.drupalcode.org:

Comments

Stado created an issue. See original summary.

aaron.ferris’s picture

It looks like the access condition in the UserFlagType class needs either the parent access result, OR the own/other profile permission.

In this configuration, unless im missing something, the parent::actionAccess will return Allowed and the 'own' items access result will be neutral, so the user can action this flag even though the user actually doesn't have the permission.

$access = parent::actionAccess($action, $flag, $account, $flaggable);

    if ($flaggable && $this->hasExtraPermission('owner')) {
      // Permit selfies.
      $permission = $action . ' ' . $flag->id() . ' own user account';
      $selfies_permission_access = AccessResult::allowedIfHasPermission($account, $permission)
        ->addCacheContexts(['user']);
      $account_match_access = AccessResult::allowedIf($account->id() == $flaggable->id());
      $own_access = $selfies_permission_access->andIf($account_match_access);
      $access = $access->orIf($own_access);

aaron.ferris’s picture

Not convinced that push is the way to go on this to be honest, but it does resolve this specific issue (unsure of any unintended consequences)

1. With own account flag permission off = no access to own account flag
2. With own account flag permission on = access to own account flag
3. With other accounts flag permission on = access to other account flag
4. With other accounts flag permission off = no access to other account flag

Id need to have a look at the failing tests, but probably better to get some more eyes on this before doing so for thoughts.

alfthecat’s picture

I'm experiencing this issue, and when creating a new (global) flag and trying to set the permissions for it the site throws a WSOD on saving the permissions with error:

RuntimeException: Adding non-existent permissions to a role is not allowed. The incorrect permissions are "flag tec_draft_order_excel_lover", "unflag ***". in Drupal\user\Entity\Role->calculateDependencies() (line 207 of /var/www/***/core/modules/user/src/Entity/Role.php).

I can't make new flags available anymore to any users, only UID1 is able to see and use flags.

deaom’s picture

Version: 8.x-4.0-beta4 » 5.x-dev

I struggled a little bit with this one and first came to the same conclusion as @aaron.ferris did in his comment and merge request and the test kept failing. Then had a look at the test and which permission are actually set and set the same permission to my authenticated user and I do think this works as designed, just going based on the test provided.
Let me explain my understanding, when you select to expose additional permissions it's meant to use only those permissions without the parent/main one. Meaning you then select just or both (other's, own) and that works as expected. So in the case of Following user accounts, if you select Flag Following other's profiles, you can't follow yours, if you select both, you can, if you select just Flag Following own profile, you can't follow others. That works as expected. When you then decide to include the parent permission in this case Flag Following, it overrides the others' and own profile permissions, as it's a parent and therefore main permission. So works as designed. Now if this is how maintianers intended for this functionality to work, I don't know, but going off of the test provided which does not set any main flag permissions (Flag Following), this works as expected and that is why tests are passing. As soon as you add the Flag Following permission to the existing test, the test will fail.
I would mark this works as designed, but that would then close the ticket, so leaving the status as is and maintainers can decide on what to do, but will switch the version to the latest 5.x dev as this is where I did my testings.

ivnish’s picture

Issue summary: View changes
ivnish’s picture

Status: Active » Closed (works as designed)

I think we can close this.

Now that this issue is closed, review the contribution record.

As a contributor, attribute any organization that helped you, or if you volunteered your own time.

Maintainers, credit people who helped resolve this issue.