Latest patches are for the 7.x-3.x branch

Not sure if this could find it's way into the 6.x-2.x branch or will have to wait for D7 but here goes:

The assign domain editors permission allows users to assign domain editors to any domain, even if they are not a member of that domain.

I think it would be useful to add a permission that allows users to assign domain editors only to domains they are a member of.

I have a patch that adds a new permission for this case. It honors user default domains even if the assigning user doesn't have access to those domains. Seems to me to work.

It is against 6.x-2.1 due to there not being a 6.x-2.x-dev branch at the moment.

CommentFileSizeAuthor
#21 domain-add_assign_own_domain_editors_perm-655922-21.patch5.03 KBmvwensen
#15 domain-add_assign_own_domain_editors_perm-655922-15.patch4.84 KBmvwensen
#13 domain-add_assign_own_domain_editors_perm-655922-13.patch3.24 KBduaelfr
#2 domain-assign_own_domain_editors_3.patch2.84 KBagileware
#1 domain-assign_own_domain_editors_2.patch2.83 KBagileware
domain-assign_own_domain_editors.patch2.83 KBagileware

Comments

agileware’s picture

agileware commented
Status: Active » Needs review
StatusFileSize
new domain-assign_own_domain_editors_2.patch2.83 KB

Sorry, I found a little bug in that patch. Here is a new one.

agileware’s picture

agileware commented
StatusFileSize
new domain-assign_own_domain_editors_3.patch2.84 KB

Sorry, I think my brain is having a meltdown today.

Here it is again with another bug fix.

agentrickard’s picture

agentrickard
he/him
Primary language English
Location Georgia (US)
commented

This could go in D7 as a patch against HEAD. For D6, do you think it could work as a stand-alone module?

agileware’s picture

agileware commented

If I make the stand alone modules weight so it runs after domain it probably could be done like that.

I can have a look into it shortly when I get a free moment.
It would be a very tiny module though.

agentrickard’s picture

agentrickard
he/him
Primary language English
Location Georgia (US)
commented

You can hook_form_alter() the user form, which is generated after hook-user() runs.

Tiny is fine. If we don't lock the 6.x features, I can never start on the D7 port.

agileware’s picture

agileware commented
Status: Needs review » Needs work

Yeah no worries.

When I get a bit of free time I'll re-roll this patch for HEAD and turn this one into an add on module for D6.

agentrickard’s picture

agentrickard
he/him
Primary language English
Location Georgia (US)
commented

You might be able to add it to the Domain Bonus Pack module set.

agentrickard’s picture

agentrickard
he/him
Primary language English
Location Georgia (US)
commented

Hm. You can't really write it against D7 yet, because HEAD has not been ported. So a patch against DRUPAL-6--2 branch might be more appropriate. Otherwise, this would have to wait.

agileware’s picture

agileware commented

OK, I'll submit it to the domain bonus issue queue and reroll this for DRUPAL-6--2

agentrickard’s picture

agentrickard
he/him
Primary language English
Location Georgia (US)
commented
Version: 6.x-2.1 » 7.x-2.x-dev
Status: Needs work » Postponed

Revisit later.

agentrickard’s picture

agentrickard
he/him
Primary language English
Location Georgia (US)
commented
Version: 7.x-2.x-dev » 7.x-3.x-dev
Status: Postponed » Active
duaelfr’s picture

duaelfr
he/him
Primary language French
Location Montpellier, France
commented
Status: Active » Needs review
StatusFileSize
new domain-add_assign_own_domain_editors_perm-655922-13.patch3.24 KB

I needed it so I did it :)

This patch is part of the #1day1patch initiative.

agentrickard’s picture

agentrickard
he/him
Primary language English
Location Georgia (US)
commented

Nice. Could use a test, of course.

mvwensen’s picture

mvwensen commented
Issue summary: View changes
StatusFileSize
new domain-add_assign_own_domain_editors_perm-655922-15.patch4.84 KB

The #13 was not secure. An editor removes domains from a user he is not assigned to + by editting the post data the editor can assign domains to people he has not been assigned to (Security thread).

Modified the patch:
- For the global user, get user domains instead of assuming they are on the user object (in some cases they are not set).
- Added validation checking if an editor submitted domain ids that are matching the one's he is assigned to (prevents fraudulent data)
- Adding the domain ids for the domains that are not assigned to the editor but the user was already assigned to.

Tested on todays dev-branch and the latest stable.

We will add the patch to drupal.org/project/dvg, so it will be great if somebody could review it.

mvwensen’s picture

mvwensen commented
Issue summary: View changes
ruudvanoijen’s picture

ruudvanoijen commented
Assigned: Unassigned » ruudvanoijen
Status: Needs review » Reviewed & tested by the community

Tested and reviewed this patch. Works as described.

mvwensen’s picture

mvwensen commented
Assigned: ruudvanoijen » Unassigned

Would be great if someone else reviews it too and it would be better if it ends up in the module ;)

askibinski’s picture

askibinski commented

Also reviewed this patch. Looks good and works great.

askibinski’s picture

askibinski commented
Status: Reviewed & tested by the community » Needs work

Found an edge case which needs work:
users like root or adminsitrators who have the "Administer domain records and settings" permission should be able to set domain access for their own account or other account.

mvwensen’s picture

mvwensen commented
Status: Needs work » Needs review
StatusFileSize
new domain-add_assign_own_domain_editors_perm-655922-21.patch5.03 KB

Nice catch!
Added a check on the validation for uid 1 and the "assign domain editors" (all domains) is on. "Administer domain records and settings" is not really a permission to give to supereditors etc.

Patch attached.

mvwensen’s picture

mvwensen commented
Issue summary: View changes
askibinski’s picture

askibinski commented
Status: Needs review » Reviewed & tested by the community

Better, looks good to me, tested this with various user, roles and permissions and works as expected.

agentrickard’s picture

agentrickard
he/him
Primary language English
Location Georgia (US)
commented
Status: Reviewed & tested by the community » Needs work

Apologies for the late review.

I think there is a case that this doesn't account for, in which an editor of domain B edits a user assigned to domains A, B, and C.

In that case, the editor should be able to remove the access to B, but not to A and C.

See the very ugly $form['domains_raw'] handling code in domain_form_alter(), which is how we account for that case with nodes.

Note that I just added this feature to the Drupal 8 version.

bluegeek9’s picture

bluegeek9 commented
Status: Needs work » Closed (outdated)

Drupal 7 in End of Life and no longer supported. We encourage you to upgrade to a supported version of Drupal. For more information, see https://www.drupal.org/upgrade.

//www.flaticon.com/free-icons/thank-you Thank you for your contribution! Your continued support of this project makes other volunteer contributions more sustainable.
There are multiple ways to show appreciation for the work contributed to this project, including:
  • Triaging issues and adding more context to existing issues.
  • Writing documentation or patches for this project.