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

[MM-55054] Consider a matching origin for a media request as a trusted URL when checking permissions #2893

Merged
merged 1 commit into from
Nov 2, 2023

Conversation

devinbinnie
Copy link
Member

Summary

With the new permission changes, we were stopping servers with subpaths from being able to grant the media permission, due to being too strict about which URLs we were considering trusted.

The media permission generally populates the securityOrigin field when doing a permission check, so when checking for that permission if the URL can be trusted, we just need to match the two origins.

This PR makes that exception in the permissions handler.

Ticket Link

https://mattermost.atlassian.net/browse/MM-55054
Closes #2881

Fixed an issue where servers on a subpath could not grant the `media` permission

@devinbinnie devinbinnie added 2: Dev Review Requires review by a core committer 3: Security Review Review requested from Security Team labels Oct 24, 2023
@marianunez
Copy link
Member

@devinbinnie could we add steps in the tickets on how we will do QA validation on this ?

@mattermost mattermost deleted a comment from marianunez Oct 24, 2023
@devinbinnie devinbinnie removed the 2: Dev Review Requires review by a core committer label Oct 24, 2023
@amyblais amyblais added this to the v5.6.0 milestone Oct 25, 2023
@esarafianou
Copy link
Contributor

@devinbinnie if a domain e.g. example.com hosts 2 Mattermost servers: one with URL example.com/mm1 and one with URL example.com/mm2, will after this change these 2 Mattermost servers need to ask for media permission separately or if example.com/mm1 is granted media permission, example.com/mm2 will inherit that permission without asking?

@devinbinnie
Copy link
Member Author

@devinbinnie if a domain e.g. example.com hosts 2 Mattermost servers: one with URL example.com/mm1 and one with URL example.com/mm2, will after this change these 2 Mattermost servers need to ask for media permission separately or if example.com/mm1 is granted media permission, example.com/mm2 will inherit that permission without asking?

Correct, because we are using the securityOrigin field which is what I assume is the correct field to use for the media permission. It's not provided for the others. Not sure if this is a problem or not.

Copy link
Contributor

@esarafianou esarafianou left a comment

Choose a reason for hiding this comment

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

LGTM!

@devinbinnie devinbinnie added 4: Reviews Complete All reviewers have approved the pull request and removed 3: Security Review Review requested from Security Team labels Nov 2, 2023
@devinbinnie devinbinnie merged commit 9faaa79 into mattermost:master Nov 2, 2023
15 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
4: Reviews Complete All reviewers have approved the pull request release-note
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[Bug]: Permission for audio not allowed
6 participants