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

Only approve web API permission requests for permissions that FreeTube needs #5022

Merged
merged 1 commit into from
Apr 27, 2024

Conversation

absidue
Copy link
Member

@absidue absidue commented Apr 25, 2024

Only approve web API permission requests for permissions that FreeTube needs

Pull Request Type

  • Other - Security Improvement

Description

In a web browser when a webpage wants to use an API that could provide access to sensitive information or allows potentially unwanted actions, the web browser prompts the user for permission, Electron however defaults to approving all permission requests. This pull request implements Electron security guideline 5: "Handle session permission requests from remote content" by only approving permissions requests for the two permissions that FreeTube actually uses, other permission requests get rejected. I also decided to reject all permission requests for non-FreeTube URLs, that's a future proofing measure in case we ever show a remote page in an iframe or something.

FreeTube only needs the following permissions:

  • fullscreen: So that the video player can enter full screen
  • clipboard-sanitized-write: To allow the user to copy video URLs and error messages

Testing

Tests to check that functionality that relies on permissions still works

  • Test that copying a link from the 3 dots menu on a video in a video still works (that the link gets written to your clipboard, you can just paste into a text editor or something right afterwards to confirm it)
  • Test that the video player can still be put into fullscreen

Test to check that unwanted permission requests are rejected

  • Run await navigator.mediaDevices.getUserMedia({ audio: true }) (requesting an audio stream from the microphone) in the dev tools, it should throw Uncaught DOMException: Permission denied

Desktop

  • OS: Windows
  • OS Version: 10
  • FreeTube version: 0.20.0

@FreeTubeBot FreeTubeBot enabled auto-merge (squash) April 25, 2024 19:25
@github-actions github-actions bot added the PR: waiting for review For PRs that are complete, tested, and ready for review label Apr 25, 2024
@kommunarr kommunarr mentioned this pull request Apr 25, 2024
1 task
@kommunarr
Copy link
Collaborator

question: This is more my ignorance and not an actual concern, but why does opening external links work when openExternal is not configured?

@absidue
Copy link
Member Author

absidue commented Apr 27, 2024

FreeTube intercepts all clicks on a elements, if it's a YouTube link it gets opened in FreeTube, otherwise depending on the user's settings either nothing happens, the user gets prompted or it gets opened by passing it to the main process via IPC call, which opens the link.

https://github.com/FreeTubeApp/FreeTube/blob/development/src/renderer/App.js#L345-L390

https://github.com/FreeTubeApp/FreeTube/blob/development/src/renderer/helpers/utils.js#L328-L339

@FreeTubeBot FreeTubeBot merged commit 84572f8 into FreeTubeApp:development Apr 27, 2024
5 checks passed
@github-actions github-actions bot removed the PR: waiting for review For PRs that are complete, tested, and ready for review label Apr 27, 2024
@absidue absidue deleted the permission-handler branch April 27, 2024 12:34
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants