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

Privacy/Security: Make Discord Integration Optional #1179

Open
feldoth opened this issue Oct 12, 2024 · 8 comments
Open

Privacy/Security: Make Discord Integration Optional #1179

feldoth opened this issue Oct 12, 2024 · 8 comments

Comments

@feldoth
Copy link

feldoth commented Oct 12, 2024

Hello,

The new Discord integration requires permissions to be granted to the extension on discord.com, for someone that never intends to use the Discord roll20 activity, this is an unnecessary and unwanted permission. I'm not sure why this has been included in the core permissions (for Firefox at least), when integration with multiple VTTs are under the optional permissions. Personally I consider this a privacy/security issue, and will not use Beyond20 with a browser logged into Discord until it is resolved.

Thanks!

@kakaroto
Copy link
Owner

Hi,
The permissions requirement is explained in the release notes here: https://beyond20.here-for-more.info/support#v296-september-22nd-2024

Unfortunately, Firefox does not support optional permissions and the same permission request API that Chrome supports, making it impossible to have that feature without also having Firefox request those permissions at the time of the update. On Chrome, that's exactly how it works, the user has to request the permissions on discord and gets prompted at that point to accept or not just those additional permissions.

What you could do is use the developer mode installation instructions from here: https://github.com/kakaroto/Beyond20#developer-mode-installation
And edit the "manifest.json" file to remove the two lines requesting discord permissions (line 14 and 15, and you'll need to remove the trailing comma on line 13)

"*://discord.com/*",
"*://1199271093882589195.discordsays.com/*"

I don't believe this to be a privacy or security issue as the code is open source and auditable by anyone who wishes to do so, and no data is retrieved from discord in any way, the permission only allows the integration with roll20 discord activity. If you were to review the code, you'd see that no code runs on the discord page itself, but the permission is necessary in order to run the extension inside the roll20 activity (https://1199271093882589195.discordsays.com/) as it is an iframe inside the discord website.

@feldoth
Copy link
Author

feldoth commented Oct 13, 2024

Hi, thanks for responding. I am a bit confused by your answer though, as it seems like there are optional permissions in Firefox. This is what I'm referring to:

image

Is there a reason the Discord permissions cannot be implemented like the Forge and Astral VTT permissions are?

Edit: Also, to clarify on the security issue - while we can audit the code, that's not something people are likely to do, or even capable of in many cases. Discord is, in my opinion, a very high value target for attackers - it's a way to impersonate someone to their friends and family. I trust you, but what happens if your access to the plugin repository is compromised and a fraudulent version is uploaded? Now suddenly we have a plugin that was legitimately granted access to sensitive service in a way that is potentially capable of bypassing authentication (including MFA). It doesn't need to request additional permissions, no red flags are set off for most users. This makes Beyond20 itself (and you, by extension) a high value target for attackers. If the permissions were instead opt-in, this would not be a problem.

Things like roll20 and D&D Beyond do not have the same threat profile as something like Discord, which is a full social media platform at this point. I'm fine with authorizing a plugin to have access to those types of sites - they are single purpose and contain no sensitive data.

@kakaroto
Copy link
Owner

Hi, thanks for responding. I am a bit confused by your answer though, as it seems like there are optional permissions in Firefox.

Checking the docs, yes, it says it's supported, and I can set optional permissions in the extension manifest, but I have not been able to request the permissions dynamically. I've tried this more than once and it just doesn't seem to support it.

The Forge is a mandatory permission, as well as the roll20 and beyond20 website. The astral tabletop one was also mandatory but it's been removed now since we dropped support for it. I don't know why it's showing all those as optional in your screenshot.

I'll give this another look in the coming days, maybe 4th time's the charm... 🤷‍♂️

@feldoth
Copy link
Author

feldoth commented Oct 16, 2024

I appreciate you looking into it.

If I had to guess, it's possible that I've had Beyond20 installed for so long in this browser that I have previously granted it the Astral tabletop permissions, and it simply remembers that because I have never revoked it, even though you no longer request it. Also, I'm not sure if it matters, but I am using the developer version of Firefox, so it's possible that its options are different from the standard version.

@avanheuvelen
Copy link

I am running regular Firefox and I also have these optional permissions. The only difference is that Astral is not an option for me.

image

@avanheuvelen
Copy link

I've just tested building the extension with the following removed from permissions and added to optional_permissions in manifest_ff.json:

               "*://discord.com/*",
               "*://1199271093882589195.discordsays.com/*",

The permissions now show up as:

image

://1199271093882589195.discordsays.com/ is still included in the 'Required permissions' list. I think this is because you have defined the content_scripts for the domain in the manifest.json file. If I remove the matches for 1199271093882589195.discordsays.com it will remove the domain from 'Required permissions':

image

However, this will obviously break the Discord integration. I think the way to fix this is to:

  • Add some sort of 'Enable discord' toggle, that requests permission for the discord domains when enabled.
  • Listen for this permissions with an event listener
  • Register the content scripts in this event listener.

However, I have to add the disclaimer that I have not tested this, nor do I have experience with developing Firefox addons. This is just what I found out while messing with this for a bit. I hope it helps.

@kakaroto
Copy link
Owner

Hi,
Unfortunately, I haven't had time to dig back into this until now.
I just poked at it, trying to figure out what I missed and finally found the issue that's been eluding me all this time, and I think I can get it all fixed 🥳
I can now properly request optional permissions and Firefox will prompt you for it!
Example on https://demo.foundryvtt.com clicking the toolbar icon and clicking the request permissions blue entry (which failed to appear before)
{E0DC206A-BCE2-4F84-80F6-72B15C558034}

Turns out, when I use chrome.permissions, it's misbehaving and causing all sorts of problems, basically making the optional permissions system simply not work!
The issue is that I'm using chrome.permissions API (I use chrome. APIs everywhere, since on Firefox, chrome object is on purpose made to be the same as browser object for cross compatibility). Only, it looks like Firefox has always had this bug where the specific APIs from chrome.permissions are misbehaving.
You can see the output below:
{E08CFBAC-6178-4A1F-881D-580BDED17A69}

The APIs were returning undefined (and apparently didn't do anything either) instead of returning a Promise, which is why they never worked.
I've now changed the code to use browser instead of chrome when we're on Firefox and it solved all of the issues I had before with this.

Re-opening the issue until I can do full QA on this and commit it.
Thanks for making me take a second (well, a fourth) look at the premissions problem with Firefox! 😁

@kakaroto kakaroto reopened this Oct 22, 2024
@feldoth
Copy link
Author

feldoth commented Oct 22, 2024

Fantastic news! Thank you again for taking the time to improve this. It is very appreciated.

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

No branches or pull requests

3 participants