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

enh: add backend check for download permission for cloud attachements #9592

Open
wants to merge 2 commits into
base: main
Choose a base branch
from
Open

enh: add backend check for download permission for cloud attachements #9592

wants to merge 2 commits into from

Conversation

hamza221
Copy link
Contributor

@hamza221 hamza221 commented Apr 22, 2024 •
edited

For now it silently Fails on draft/outbox save/update

  • Find a way to reflect in the front end for versions prior to 3.6 if needed (not needed)

@hamza221 hamza221 self-assigned this Apr 22, 2024
@hamza221 hamza221 added 2. developing 3. to review enhancement skill:backend Issues and PRs that require backend development skills and removed 2. developing labels Apr 22, 2024
@hamza221 hamza221 marked this pull request as ready for review May 2, 2024 10:41
@hamza221 hamza221 force-pushed the enh/backend-check-for-download-permission branch from 3985509 to 0b875b4 Compare May 8, 2024 17:09
@hamza221 hamza221 requested review from st3iny and kesselb May 8, 2024 17:09
st3iny
st3iny reviewed May 10, 2024
View reviewed changes
Copy link
Member

@st3iny st3iny left a comment

Choose a reason for hiding this comment

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

The backend side works. When sending the mail a warning is logged, the attachment is still shown in the frontend (composer) but not sent.

I wonder why the frontend let's me pick a file without download permissions. Shouldn't the file picker filter incoming shares without download permissions?

lib/Service/Attachment/AttachmentService.php Outdated Show resolved Hide resolved
@hamza221
Copy link
Contributor Author

hamza221 commented May 13, 2024 •
edited

they're supposed to be filtered out, it's a regression, I'm investigating
update: 0958ec3

@hamza221
enh: add backend check for download permission for cloud attachements
6648bce 
Signed-off-by: Hamza Mahjoubi <hamzamahjoubi221@gmail.com>
@hamza221 hamza221 force-pushed the enh/backend-check-for-download-permission branch from 0b875b4 to 6648bce Compare May 27, 2024 14:15
@hamza221
Copy link
Contributor Author

hamza221 commented May 28, 2024 •
edited

I'm still investigating the frontend regression, my doubt i that it's a filepicker regression, since share-attributes are not shown anymore, I'm checking with other colleagues

@st3iny
Copy link
Member

st3iny commented May 28, 2024

I'm still investigating the frontend regression, my doubt i that it's a filepicker regression, since share-attributes are not shown anymore, I'm checking with other colleagues

Sounds good. I'd also say that we can move forward with this PR and look at the frontend separately.

@hamza221
fixup! enh: add backend check for download permission for cloud attac…
0e71d88 
…hements

Signed-off-by: Hamza Mahjoubi <hamzamahjoubi221@gmail.com>
@hamza221
Copy link
Contributor Author

hamza221 commented Jun 3, 2024

@st3iny fyi frontend is fixed here #9697

st3iny
st3iny approved these changes Jun 4, 2024
View reviewed changes
Copy link
Member

@st3iny st3iny left a comment

Choose a reason for hiding this comment

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

Tested and works

@st3iny st3iny mentioned this pull request Jun 4, 2024
Merged
@st3iny st3iny added this to the v3.8.0 milestone Jun 4, 2024
@st3iny
Copy link
Member

st3iny commented Jun 4, 2024

/backport to stable3.7

@st3iny
Copy link
Member

st3iny commented Jun 4, 2024

/backport to stable3.6

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@st3iny st3iny st3iny approved these changes

@kesselb kesselb Awaiting requested review from kesselb

@ChristophWurst ChristophWurst Awaiting requested review from ChristophWurst ChristophWurst is a code owner

Assignees

@hamza221 hamza221

Labels
3. to review backport-request enhancement skill:backend Issues and PRs that require backend development skills
Projects
None yet
Milestone
v3.8.0
Development

Successfully merging this pull request may close these issues.

None yet
3 participants
@hamza221 @st3iny @ChristophWurst