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.

Sign up for GitHub

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

Jump to bottom

Context checkboxes ignored for traverse permissions #3579

Open
Johni0702 opened this issue Jan 16, 2019 · 1 comment · May be fixed by #6512
Open

Context checkboxes ignored for traverse permissions #3579

Johni0702 opened this issue Jan 16, 2019 · 1 comment · May be fixed by #6512
Labels
acl Everything related to access-control-lists (permission management) bug A bug (error) in the software

Comments

@Johni0702
Copy link
Contributor

Steps to reproduce:

  1. Set @all -traverse on some channel (also add -write if not already set by default)
  2. Using a different account, try to join the channel or any of its children -> access denied (as expected)
  3. Untick Applies to this channel (keep Applies to sub-channels ticked)
  4. Using a different account, try to join the channel or any of its children -> access denied (expected for children but unexpected for the channel itself)
  5. Untick Applies to sub-channels as well
  6. Using a different account, try to join the channel or any of its children -> access denied (unexpected)

The checkboxes should either be disabled when traverse is denied or be properly handled.
Looking at the code responsible for permission checking, I do not see any particular reason why this special case exists (i.e. why not just use granted & Traverse/granted & Write ?), so I'd strongly prefer the latter option.

mumble/src/ACL.cpp

Lines 87 to 94 in 17cdab7

if (acl->pAllow & Traverse)
traverse = true;
if (acl->pDeny & Traverse)
traverse = false;
if (acl->pAllow & Write)
write = true;
if (acl->pDeny & Write)
write = false;

@Johni0702 Johni0702 mentioned this issue Aug 23, 2019
Closed
@Hartmnt Hartmnt added acl Everything related to access-control-lists (permission management) bug A bug (error) in the software labels Jun 2, 2024
Hartmnt added a commit to Hartmnt/mumble that referenced this issue Jul 17, 2024
@Hartmnt
FIX(server): Make sure context is applied to traverse and write ACL
4101c4a 
Previously, both the traverse and write ACL would be evaluated
without taking the "in this channel" and "in sub-channels" context
options into account.
This would lead to denying traverse for channels that were
actually supposed to be traversable.

This commit refactors the ACL calculation for readability
and makes sure the write and traverse checks are actually taking the
context into account.

Fixes mumble-voip#3579
@Hartmnt Hartmnt linked a pull request Jul 17, 2024 that will close this issue
Open
@Hartmnt
Copy link
Member

Hartmnt commented Jul 17, 2024

Note: Only 3) and 4) are unexpected. Revoking traverse on any parent channel is supposed to make all child channels inaccessible even if the denied traverse ACL does not explicitly "propagate" down.

Hartmnt added a commit to Hartmnt/mumble that referenced this issue Sep 22, 2024
@Hartmnt
FIX(server): Make sure context is applied to traverse and write ACL
3d5dcd9 
Previously, both the traverse and write ACL would be evaluated
without taking the "in this channel" and "in sub-channels" context
options into account.
This would lead to denying traverse for channels that were
actually supposed to be traversable.

This commit refactors the ACL calculation for readability
and makes sure the write and traverse checks are actually taking the
context into account.

Fixes mumble-voip#3579
Hartmnt added a commit to Hartmnt/mumble that referenced this issue Oct 1, 2024
@Hartmnt
FIX(server): Make sure context is applied to traverse and write ACL
306de66 
Previously, both the traverse and write ACL would be evaluated
without taking the "in this channel" and "in sub-channels" context
options into account.
This would lead to denying traverse for channels that were
actually supposed to be traversable.

This commit refactors the ACL calculation for readability
and makes sure the write and traverse checks are actually taking the
context into account.

Fixes mumble-voip#3579
Hartmnt added a commit to Hartmnt/mumble that referenced this issue Oct 14, 2024
@Hartmnt
FIX(server): Make sure context is applied to traverse and write ACL
38080a5 
Previously, both the traverse and write ACL would be evaluated
without taking the "in this channel" and "in sub-channels" context
options into account.
This would lead to denying traverse for channels that were
actually supposed to be traversable.

This commit refactors the ACL calculation for readability
and makes sure the write and traverse checks are actually taking the
context into account.

Fixes mumble-voip#3579
Hartmnt added a commit to Hartmnt/mumble that referenced this issue Oct 14, 2024
@Hartmnt
FIX(server): Make sure context is applied to traverse and write ACL
1da8b6f 
Previously, both the traverse and write ACL would be evaluated
without taking the "in this channel" and "in sub-channels" context
options into account.
This would lead to denying traverse for channels that were
actually supposed to be traversable.

This commit refactors the ACL calculation for readability
and makes sure the write and traverse checks are actually taking the
context into account.

Fixes mumble-voip#3579
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
acl Everything related to access-control-lists (permission management) bug A bug (error) in the software
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

FIX(server, client): Fix ACL write and traverse permissions Hartmnt/mumble
2 participants
@Johni0702 @Hartmnt