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

enabling "invite people" for password-protected folder/file #9931

Merged
merged 5 commits into from
Nov 16, 2023
Merged

enabling "invite people" for password-protected folder/file #9931

Changes from 1 commit
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
0f99d66
enabling invite people for password-protected folder/files
jacob-nv Nov 10, 2023
c41dde3
removed hasPassword from LinkShareRoles
jacob-nv Nov 15, 2023
f7694e6
Merge branch 'master' into password-link-bugfix
jacob-nv Nov 15, 2023
9cc3ae6
Merge branch 'master' into password-link-bugfix
jacob-nv Nov 15, 2023
1a1052d
added invite people for password-protected folder changelog
jacob-nv Nov 15, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
13 changes: 10 additions & 3 deletions packages/web-app-files/src/components/SideBar/Shares/FileLinks.vue
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -443,7 +443,14 @@ export default defineComponent({
},

checkLinkToUpdate({ link }) {
const params = this.getParamsForLink(link)
let params = this.getParamsForLink(link)
if (link.permissions === 0) {
params = {
...params,
password: '',
expireDate: ''
}
}

if (!link.password && !this.canDeletePublicLinkPassword(link)) {
showQuickLinkPasswordModal(
Expand Down Expand Up @@ -629,7 +636,7 @@ export default defineComponent({
this.hasPublicLinkEditing,
this.hasPublicLinkContribute,
this.hasPublicLinkAliasSupport,
!!link.password,
false,
this.canCreatePublicLinks
)
}
Expand All @@ -639,7 +646,7 @@ export default defineComponent({
this.hasPublicLinkEditing,
this.hasPublicLinkContribute,
this.hasPublicLinkAliasSupport,
!!link.password,
false,
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

does this always work? for example if you have password enforced for editable links only and switch permissions from existing link that only has view permissions to editable permissions, will the password required still pop up as it did before?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@janackermann Could you please navigate me more in detail on how to recreate this use case?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sure, please remove all the other env settings and set

SHARING_PUBLIC_WRITEABLE_SHARE_MUST_HAVE_PASSWORD
SHARING_PUBLIC_WRITEABLE_SHARE_MUST_HAVE_PASSWORD

To true

https://owncloud.dev/services/sharing/configuration/

This will let you create a standard readonly link but as soon you want to change to edit permissions the popup opens and asks you to set a password

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Got popup asking to setup password in case of "can upload", "can edit" and "secret file drop". Still able to change to "invited people" which will drop password. After dropping password, I'm only able to set to "can view" without popup

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If I read the code correctly you can remove the hasPassword param from the LinkShareRoles.list and LinkShareRoles.filterByBitmask functions entirely, right? If that's correct then please do so.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@kulmann
I was attempting to remove it from the start, but wasn't successful, quickest fix was setting false for it in specific instances.
If I would to test again removing it completely, how would this condition would be resolved?
...(hasAliasLinks && !hasPassword ? [linkRoleInternalFile, linkRoleInternalFolder] : []),
packages/web-client/src/helpers/share/role.ts LinkShareRoles.list

Don't have enough understanding of what and how alias link works and what it could affect

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[...] how would this condition would be resolved? ...(hasAliasLinks && !hasPassword ? [linkRoleInternalFile, linkRoleInternalFolder] : []), packages/web-client/src/helpers/share/role.ts LinkShareRoles.list

Alias links are what we now call only invited people or internal links in the ui. They don't hold permissions on their own, they just serve as a pointer to a file or folder (but in the same data structure like a public link). Meaning: the one who clicks on such a link needs something else which grants permission to the file/folder, i.e. a share or a space membership with certain privileges.

That being said, a password doesn't make sense at all for an alias link, because the receiver of the link needs to hold permissions to access the linked file/folder anyway. Don't know why we would ever hide the alias link role when a password has been set before. Removing the && !hasPassword in fact seems to be the underlying bugfix, that you addressed by always setting it to false. Just remove the && !hasPassword condition entirely please. And with that there are no other references to the hasPassword param anymore => you can get rid of the param.

this.canCreatePublicLinks
)
}
Expand Down