Problem/Motivation

Unassigned not manager users must not be able to view or update task Items.

Right now an unassigned regular translator user is able to translate a task item not assigned to him just typing the url.

Proposed resolution

Remaining tasks

User interface changes

API changes

Data model changes

CommentFileSizeAuthor
#7 interdiff-unassigned_not_manager-2688939-5-7.txt9.46 KBedurenye
#7 unassigned_not_manager-2688939-7.patch4.7 KBedurenye
#5 unassigned_not_manager-2688939-5.patch3.99 KBedurenye
#2 unassigned_not_manager-2688939-2.patch2.12 KBedurenye

Comments

edurenye created an issue. See original summary.

edurenye’s picture

edurenye commented
Status: Active » Needs review
StatusFileSize
new unassigned_not_manager-2688939-2.patch2.12 KB

Test only, to show how bad this is.

Status: Needs review » Needs work

The last submitted patch, 2: unassigned_not_manager-2688939-2.patch, failed testing.

edurenye’s picture

edurenye commented
Title: Unassigned not manager users must not be able to view or update tasks or task Items » Unassigned not manager users must not be able to view or update task Items
Issue summary: View changes
Related issues: +#2656810: [meta] Add Assign/unassign permissions

Just with task items, otherwise we cannot assign it to ourselves.
Also maybe for a followup, inside a task a user should be able to assign an unassigned task to himself.

edurenye’s picture

edurenye commented
Status: Needs work » Needs review
StatusFileSize
new unassigned_not_manager-2688939-5.patch3.99 KB

Fixed the problem, changed the test according to the new issue details.
Also now I just let the admin or the manager to change the title, as if a translator changes the title could give wrong information to the manager.

berdir’s picture

berdir
Primary language German
Location Switzerland
commented
Status: Needs review » Needs work

Nice catch.

+++ b/translators/tmgmt_local/src/Form/LocalTaskForm.php
@@ -68,6 +68,7 @@ class LocalTaskForm extends ContentEntityForm {
 
+    $form['title']['widget']['#access'] = \Drupal::currentUser()->hasPermission('administer tmgmt') || \Drupal::currentUser()->hasPermission('administer translation tasks');

This looks like a widget, then you can do this in the access control handler, see \Drupal\node\NodeAccessControlHandler::checkFieldAccess as an example

edurenye’s picture

edurenye commented
Status: Needs work » Needs review
StatusFileSize
new unassigned_not_manager-2688939-7.patch4.7 KB
new interdiff-unassigned_not_manager-2688939-5-7.txt9.46 KB

Done.

berdir’s picture

berdir
Primary language German
Location Switzerland
commented 
+++ b/translators/tmgmt_local/src/Entity/Controller/LocalTaskAccessControlHandler.php
@@ -46,6 +48,16 @@ class LocalTaskAccessControlHandler extends EntityAccessControlHandler {
+    if ($operation == 'edit') {
+      return AccessResult::allowedIfHasPermissions($account, ['administer tmgmt', 'administer translation tasks']);
+    }
+    return parent::checkFieldAccess($operation, $field_definition, $account, $items);
+  }

Do we really limit edit to *all* fields for edit?

  • Berdir committed 28570f0 on 8.x-1.x authored by edurenye 
    Issue #2688939 by edurenye: Unassigned not manager users must not be...
berdir’s picture

berdir
Primary language German
Location Switzerland
commented
Status: Needs review » Fixed

Discusssed, OK for now.

Status: Fixed » Closed (fixed)

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