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

Remove Link Password based on permission #7538

Closed
4 tasks
tbsbdr opened this issue Oct 19, 2023 · 7 comments · Fixed by owncloud/web#9857
Closed
4 tasks

Remove Link Password based on permission #7538

tbsbdr opened this issue Oct 19, 2023 · 7 comments · Fixed by owncloud/web#9857
Assignees
Labels
Category:Enhancement Add new functionality Type:Story User Story
Milestone

Comments

@tbsbdr
Copy link

tbsbdr commented Oct 19, 2023

Description

User Stories 🗣️

  • As a CISO I want that users "opt out" from setting a password for publicly shared links so that not setting a password for a link is a deliberate decision.

Value 💵

Empower security by default and make insecure links a conscious user decision

Userflow 💻

  1. The User creates public link with "Viewer" permission
    1697744314298

  2. The user must enter a password (pw follows the policy)

1697743894226

  1. Link is created successfully

1697744395533

  1. The user can remove the password if he has the permission to opt out

1697744638691

  1. The user can not remove the password if he has no permission to opt out

1697744766428

Acceptance Criteria

  • New permission ReadOnlyPublicLinkPassword.Delete exists in the settings service
    • Assigned to "Admin"
    • Assigned to "Space Manager"
  • Users with the opt-out permission can delete the password on read-only public links (OCS)

API Request as Einstein (has no opt out permission)

curl -L -X PUT 'https://localhost:9200/ocs/v2.php/apps/files_sharing/api/v1/shares/PjiUGbavRqtGNlO' \
-H 'Authorization: Basic ZWluc3RlaW46cmVsYXRpdml0eQ==' \
-F 'permissions="1"' \
-F 'name="Link"' \
-F 'password=""' \
-F 'expireDate=""'

Response 403 - Forbidden

<?xml version="1.0" encoding="UTF-8"?>
<ocs>
    <meta>
        <status>error</status>
        <statuscode>104</statuscode>
        <message>user is not allowed to delete the password from the public link</message>
    </meta>
</ocs>

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
    [ ] code 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
  • Non-functional requirements
    [ ] no sonar cloud issues
@micbar
Copy link
Contributor

micbar commented Oct 20, 2023

@janackermann I am implementing the backend already. Needs tests. See linked ocis PR.

@AlexAndBear
Copy link
Contributor

@micbar thx, please let me know as soon as those open prs are merged I will continue on that

@kulmann
Copy link
Member

kulmann commented Oct 23, 2023

I'd like to propose that a user with the permission can opt out in the form field instead of setting a password in the first place and then deleting it afterwards. That would still be a conscious decision.. maybe ui wise even with "danger zone" checks like github does it in repo settings.

@micbar
Copy link
Contributor

micbar commented Oct 23, 2023

backend implementation is assuming that the "CREATE" always needs to enforce a password. Only the "UPDATE" can leave the password empty.

IMO this is the flow which was originally designed.

@micbar
Copy link
Contributor

micbar commented Oct 23, 2023

More arguments. If you implement it like this, we have no enforcement at all on the server side.

@micbar
Copy link
Contributor

micbar commented Oct 23, 2023

@janackermann PR in ocis is merged.

"Admin" and "SpaceAdmin" have the permission ReadOnlyPublicLinkPassword.Delete.
The PATCH allows an empty passwords parameter when this permission is set.

You need to set OCIS_SHARING_PUBLIC_SHARE_MUST_HAVE_PASSWORD=true to test this behavior.

@AlexAndBear
Copy link
Contributor

Updated story points to 8, because lots of issues in web....

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Category:Enhancement Add new functionality Type:Story User Story
Projects
Status: Done
4 participants