Problem/Motivation

Currently
Make suggestions
Moderate suggestions
Self-moderate suggestions
do not provide a permission to manage their use from group role config.

And the current permission are not accurate as stated in task https://www.drupal.org/node/3395640

Update: 02/05/2024
Note that permitions for that exist in core's people permissions from Localization Community UI, but those permissions need to be applied at Group permission level instead to allow applying expected rights.

Steps to reproduce

Logged as a non member of the group
You should not be allowed to submit a suggestion
see https://www.drupal.org/node/3395640 for other cases

Proposed resolution

Either fix permissions defined in code or create an access controller for those

CommentFileSizeAuthor
#13 group_permissions.png246.63 KBfmb
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

ericdsd created an issue. See original summary.

ericdsd’s picture

Issue summary: View changes
shmy’s picture

Assigned: Unassigned » shmy
shmy’s picture

I'm not sure how to solve this.

The issue occurs because the access handler grants access when either group or account permission have been granted and authenticated users have the required core permission as mentioned above.

The most straightforward solution would (probably) be to just re-use the (existing) core permissions and change (the above mentioned) access handler resolve the group permission itself. I.e. by checking if the user is a member of the group and has the required core permission. But the group ownership of a translation string is not resolvable from a translation entity as they only have a (two-letter) abbreviation of the language and group entities indicate their language only by their label. Therefore we would need to resolve that mapping issue beforehand.

The alternative path would be native group permission. They are bound to group relationships. For our stories and wikis these are provided by group's node relationship plugin. A custom one for our l10n_server_translation entity type would be needed. That's part one. The second part are the relationships. There are usually created during entity creation. That would potentially require changes to migration. To prevent those additional relationships we could override the storage handler of the group_content entity type. In that case there is the need for a custom translation to group entity mapping too.

Maybe that same principle applies to the permissions and actions of Localization Server + Localization Packager.

Maybe there is another (more straightforward) solution, but until now i haven't find one.

shmy’s picture

A correction on the first case, the custom access handler. It wouldn't be enough to check if the user is just a member of the group, because then they would have the same permission on every group. Instead they would need a specific role within that group. The permission itself could be defined / resolved in code.

fmb’s picture

Priority: Normal » Major
Related issues: +#3395508: New endpoints for l10n_client_contributor

Setting priority as major as this is both a regression and a security issue.

Please note that we should also check that access is correctly dealt with in the REST endpoint (provided by the l10n_remote module). Do we need the L10nServerContributorResource::access() method?

shmy’s picture

I don't even consider the proposed non-group solution as an actual one. I'm just imaging a point where someone from the DA / infra team (or any other consumer of the module) needs to temporarily revoke a permission for some reason. That would require a patch!

Are there any issues with my proposed group solution? I'm willing to address these.
What is the state of Update groups to 3.x? I did not check if its still applies there. If not i guess its a blocker for this issue.

In order to perform the access check on the group entity on the REST endpoint the L10nServerContributorResource::access() is probably not needed. But query access check has to be turned on.
The recently added submit suggestions remotely permission could be checked on the route.

fmb credited harlor.

fmb’s picture

I talked to the maintainer of the Group module today. As it turns out, we can define a group permission (in a .group.permission.yml file), and call $group->hasPermission() wherever we need to check access (the group can be retrieved via the language code, maybe not by using the route, see L10nAccess::check()).

We do no actually need relationship entities, as we do not want to restrict view access to translation suggestions.

harlor’s picture

Assigned: shmy » Unassigned
Status: Active » Needs review

@fmb, I tried to write a draft for the suggestions in #9

Is this going into the right direction?

fmb’s picture

I think we are good regarding suggestions, can somebody review this?

Now, It looks like at least some global permissions defined in l10n_community.permissions.yml should be turned into group permissions. Then, we may get rid of global permission checks in L10nAccess, but this could be done in a follow-up issue.

fmb’s picture

StatusFileSize
new246.63 KB

submit suggestions. moderate suggestions from others, moderate own suggestions are now group permissions instead of global permissions.

In a follow-up issue, we should also review other permissions (see attached capture of Drupal 7 group permissions configuration). As some of them are currently used in routes, I am leaving as they are for the time being.

fmb’s picture

Status: Needs review » Fixed

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

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

Maintainers, please credit people who helped resolve this issue.

Status: Fixed » Closed (fixed)

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