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

[web] Deny access #7180

Closed
4 tasks
tbsbdr opened this issue Jun 24, 2022 · 7 comments · Fixed by #8983
Closed
4 tasks

[web] Deny access #7180

tbsbdr opened this issue Jun 24, 2022 · 7 comments · Fixed by #8983
Labels

Comments

@tbsbdr
Copy link
Contributor

tbsbdr commented Jun 24, 2022

Description

User Stories

  • As a user I want to exclude other users or groups from specific folders within a Space or folder, so that we can prepare a surprise party for the excluded user / group.

Value

  • Meet expectations users have through Sharepoint / Windows Network Drives
  • Increase sharing flexibility

Acceptance Criteria

(see scribble)

  • FYI: Only Space Managers / people with sharing permission can toggle "Deny Access"
  • If the share is inherited: Add new option "Deny access" in share-context menu
  • Option "Deny access" can be toggeled on/off (menu stays open upon click - c.f. view settings)
    • On:
      • replace current sharing role with "Access denied"
      • Hover on "Access denied" show tooltip: "[User/Group] can not access [resourcename]."
      • Stop the inheritance of sharing permissions
      • remove the "via"-link
      • replace Avatar Image with icon stop-circle-line
    • off: (as currently is)
      • show inherited sharing role
      • Inherit given sharing permission
      • show the via link
  • backend side Deny access is still a share (with permission 0), so those need to be filtered from the list of share recipients
  • Icon: stop-circle-line

Definition of ready

[ ] everybody needs to understand the value written in the user story
[ ] acceptance criteria has to be defined
[ ] all dependencies of the user story need to be identified
[ ] feature should be seen from an end user perspective
[ ] user story has to be estimated
[ ] story points need to be less then 20

Definition of done

  • Functional requirements
    [ ] functionality described in the user story works
    [ ] acceptance criteria are fulfilled
  • Quality
    [ ] codre review happened
    [ ] CI is green
    [ ] critical code received unit tests by the developer
    [ ] automated tests passed (if automated tests are not available, this test needs to be created and passed
    [ ] e2e test: verify that the subfolder (on which the Deny access was done) is not visible to the denied person. Revert the Deny access afterwards and verify that the subfolder is visible again.
  • Non-functional requirements
    [ ] no sonar cloud issues
@tbsbdr tbsbdr added the Type:Story User Story label Jun 24, 2022
@tbsbdr
Copy link
Contributor Author

tbsbdr commented Jun 27, 2022

updated screenshot with "inactive" annotation.

@micbar
Copy link
Contributor

micbar commented Jun 27, 2022

ok, Visuals look fine for me.

"Stop Sharing" is IMO misleading from a wording perspective.

We need to transport the information that we break the inheritance. The share on the parent is not removed, which could be the expectation here ( "stop sharing" aka stop doing something or do not share anymore)

IMO it would be better to "start denying" something from this resource downwards.

@micbar
Copy link
Contributor

micbar commented Jun 27, 2022

@phil-davis how could we be more precise from your perspective?

@phil-davis
Copy link
Contributor

@phil-davis how could we be more precise from your perspective?

Maybe "No access".

This would be used in a few typical situations:

  1. The resource itself (folder or a specific file) is already specifically shared with some group(s). Those will appear in the list above "Jane Doe", so the list might show "Finance (group)" => "Editor", then "Jane Doe" => "No access"
  2. A parent folder is already specifically shared with some group(s). We are managing a sub-folder or file inside the folder tree. Maybe the UI should somewhere always show the aggregate-inherited-permissions at the resource we are at. (And those inherited accesses cannot be edited at this point in the tree). That way, "Finance (group)" => "Editor" will be shown. Then "Jane Doe" => "No access" can be added.
  3. A parent folder is already specifically shared with "Jane Doe". (Maybe the "owner" has shared a folder-tree with 3 or 4 users individually). Now the "owner" wants to add "Jane Doe" => "No access" at some lower point in the tree. The back-end should handle this case. The front-end UI will need to specify what-to-show after "Jane Doe" => "No access" has been input - should it show "Jane Doe" => "Editor" in the inherited permissions "from above", and then show "Jane Doe" => "No access", or should it not mention "Jane Doe" at this point in the tree?
  4. The space is available to Jane Doe, or a group. Show that in the aggregate-inherited-permissions. Then "Jane Doe" => "No access" can be added. (A lot like (2))

Also need to specify what is possible when:

  • "Finance" group (and other group(s)) exists and has access to the resource tree (space, folder...)
  • "Jane Doe" is not currently a member of any of those groups, so currently has no access

Can the "owner" add "Jane Doe" => "No access"?

Use case: I am about to add Jane Doe to the Finance group. But I want to block her access to some sub-folder in the "Finance" folder tree. So I need to add the "No access" entry first, then add her to the Finance group. If I do it in the opposite order, then she gets access for a minute, and her desktop client might auto-sync the unintended files...

@tbsbdr
Copy link
Contributor Author

tbsbdr commented Jun 28, 2022

Thanks for your input! "No access" is definitely better than "Stop sharing".
Updated scribbles:

  • Wording "No access"
  • Added surrounding ownCloud Web UI to have better context

@labkode
Copy link

labkode commented Jul 8, 2022

@diocas @elizavetaRa your feelings on this?

@diocas
Copy link
Contributor

diocas commented Jul 8, 2022

Looks pretty much the same as the (quick) implementation we've done in our fork, so I would give it a thumbs up.

For @phil-davis' point 3), I guess we should only show one of them, otherwise users will be confused wether they have or not permissions (or we have a different visual representation that shows which one takes precedence). I guess the denial will always take precedence (in EOS we do, at least), but users don't necessarily know that.

@exalate-issue-sync exalate-issue-sync bot added p3-medium Type:Epic Epic is the parent of user stories and removed Type:Story User Story labels Jul 12, 2022
@exalate-issue-sync exalate-issue-sync bot added the Type:Story User Story label Apr 21, 2023
@exalate-issue-sync exalate-issue-sync bot changed the title Deny access for specific user or group (break inheritance / denial ACL) [web] Deny access for specific user or group (break inheritance / denial ACL) Apr 21, 2023
@exalate-issue-sync exalate-issue-sync bot removed the Type:Epic Epic is the parent of user stories label Apr 21, 2023
@exalate-issue-sync exalate-issue-sync bot changed the title [web] Deny access for specific user or group (break inheritance / denial ACL) [web] Deny access Apr 21, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants