Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

"unit.check" permission is tested inconsistently #4590

Closed
rageandengage opened this issue Sep 25, 2020 · 2 comments · Fixed by #4597
Closed

"unit.check" permission is tested inconsistently #4590

rageandengage opened this issue Sep 25, 2020 · 2 comments · Fixed by #4597
Assignees
Labels
bug Something is broken.
Milestone

Comments

@rageandengage
Copy link
Contributor

rageandengage commented Sep 25, 2020

Describe the bug

The "unit.check" permission ("Ignore failing check") is tested incorrectly in several areas;

In weblate/trans/views.py, the following tests occur:

def ignore_check(request, check_id):
    (...)
    if not request.user.has_perm("unit.check", project) or obj.is_enforced():
        raise PermissionDenied()
    (...)

def ignore_check_source(request, check_id):
    (...)
    if (
        not request.user.has_perm("unit.check", project)
        or obj.is_enforced()
        or not request.user.has_perm("source.edit", unit.translation.component)
    ):
        raise PermissionDenied()
    (...)

But, in templates/translate.html, the offering is based on:

{% perm 'unit.check' unit.translation as user_can_ignore_check %}

The "unit.check" permission doesn't appear to be granted at a Project-level at all. This leads to a scenario where, during Custom permissions, if a role is granted this perm, the translation UI offers the ability to ignore but always throws a PermissionDenied exception.

Expected behavior

Either test for permissions based on translation or component, like so:

    if not request.user.has_perm("unit.check", obj.unit.translation.component) or obj.is_enforced():
        raise PermissionDenied()

This has the correct behavior.

@nijel nijel added the bug Something is broken. label Sep 25, 2020
@nijel nijel added this to the 4.3 milestone Sep 25, 2020
@nijel nijel self-assigned this Sep 25, 2020
@nijel
Copy link
Member

nijel commented Sep 25, 2020

This is relict from past when checks were bound to a project and checksum of the unit, not to actual unit. I guess that similar issue will exist for comments or suggestions where same model has been used.

@nijel nijel linked a pull request Sep 25, 2020 that will close this issue
4 tasks
@kodiakhq kodiakhq bot closed this as completed in #4597 Sep 25, 2020
kodiakhq bot pushed a commit that referenced this issue Sep 25, 2020
This way it properly reflects component or translation level permission.

Issue #4590
kodiakhq bot pushed a commit that referenced this issue Sep 25, 2020
This allows to integrate the enforced check to the permission avoiding
need to duplicate this condition in other locations. Also remove not
needed check_access invocation as that is already covered by the
permission check.

Fixes #4590
@github-actions
Copy link

Thank you for your report, the issue you have reported has just been fixed.

  • In case you see a problem with the fix, please comment on this issue.
  • In case you see a similar problem, please open a separate issue.
  • If you are happy with the outcome, consider supporting Weblate by donating.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something is broken.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants