Contrib.social

Permissions field on user edit form leads to unexpected results

Open on Drupal.org →
Created on 31 July 2020, almost 4 years ago
Updated 15 March 2024, 3 months ago

Using ddev, I installed a fresh site with Drupal 9 and Permissions by Term (pbt). I then created some test users, content, and a vocabulary for use with pbt. Everything worked as expected until I tried removing a role from a user and realized that the user still had permission to view content as before.

The reason is that the user edit form has both the role field and the new permissions fieldset for pbt. If you setup pbt to allow view access to a term for a user role, add that role to a user, and then subsequently edit the user to remove the role, the user edit form will add that user back to the term as a user because the permissions fieldset pre-populated the permissions field based on their role. Submitting the form then assigns that user to the terms.

I think the solution is to add a new tab to the user entity for Permissions by Term instead of having the form in the edit tab where it's currently located. The new tab would show an edit form for adding the user to the available terms, but it could also note the terms the user has access to already by way of their assigned roles. This report of the current permissions for the user could look similar to the allowed users and roles fieldset shown on node edit forms. It could also look something like what Workbench Access does.

Workbench Access manages role and user-based assignment of editors to terms like pbt. The difference is that workbench access adds a "Workbench Access" tab to the user entity where you can only control user-level assignment to terms and there's a note about role-based assignments below the form:

Sections assigned by role are emphasized and marked with an * but not selected unless they are also assigned directly to the user. They need not be selected. Access granted by role cannot be revoked from this form.

I hope this helps. I would be happy to test patches and give feedback.

Thank you for this module.

🐛 Bug report
Status

Fixed

Version

3.1

Component

User interface

Created by

🇺🇸United States thoogend

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

Merge Requests

Comments & Activities

Not all content is available!

It's likely this issue predates Contrib.social: some issue and comment data are missing.

  • Comment 8 months ago →
    🇺🇸United States chucksimply

    Here's a new patch for 3.1.29. Does the same thing, removes permissions_by_term_form_user_form_alter() and permissions_by_term_user_form_submit().

    Would be great to get disabling this as an option.

  • Status changed to RTBC 7 months ago
  • Comment 7 months ago →
    🇺🇸United States erutan

    Tested the patch in 14 and it cleanly removes the permissions by taxo area from the user edit page.

    I'll set this to RTBC just so people know there's a working patch here that solves the problem, even if building out a UI in the app isn't there. I can't see why we'd really want to tie terms to a specific user vs roles given the scope and functionality of the app.

    If someone disagrees with that feel free to set it back to active!

  • First commit to issue fork.
  • Merge request !39Resolve #3162679 "Terms in user form" → (Merged) created by marcoliver
  • Status changed to Needs review 6 months ago
  • Comment 6 months ago →
    🇩🇪Germany marcoliver Neuss, NRW, Germany

    Hey everybody! Thanks for the input on this issue! I went ahead and created an issue fork that does a two things:

    • In the settings there now is a new option to toggle the display of the terms on the user page on and off. (Defaults to TRUE)
    • If the terms are displayed, they are split up by vocabulary to reduce the potential insanity of having tens of thousands of terms in the same select field

    Could someone with a bit more mileage on this issue take a look at that fork and let me know if these changes make sense?

  • Comment 5 months ago →
    System Message
    • marcoliver → committed 29881be7 on 3.1.x-dev 
      Issue #3162679 by chucksimply, marcoliver, thoogend, Anybody,...
  • Status changed to Fixed 5 months ago
  • Comment 5 months ago →
    🇩🇪Germany marcoliver Neuss, NRW, Germany

    Hi everyone, I went ahead and merged the fork with the new setting. It will be released in 3.1.31.

    Marking this as fixed. Is this issue persists for anyone on 3.1.31, feel free to reopen it or open a new one.

  • Comment 5 months ago →
    🇺🇸United States chucksimply

    Looks good. Thanks!

  • Comment 5 months ago →
    System Message

    Automatically closed - issue fixed for 2 weeks with no activity.

  • Comment 3 months ago →
    🇨🇦Canada dtarc

    Hi, we were setting defaults in the permissions by term field when creating new users and this broke our code.

    In the user form, splitting up `$form['access']['terms']` by vocabulary was probably an unnecessary change that didn't need really need to happen.

    Just making a comment here so that others might find something when looking. It's a hard problem to find if you don't know what's going on.

    Anyway this was a breaking change.

contrib.social Blog FAQ Discussions
Production build 0.69.0 2024