add favicon support

[Sneezry]

This PR adds favicon support.

1. This PR adds favicon support for MV2
2. Firefox doesn't support to fetch favicon in MV2, and this PR disable this feature for Firefox
3. When we migrate to MV3, chrome://favicon permission is not needed, and we need to remove it from permission list and CSP.

We read website url from issuer field. The user need to add ::<website-domain> in end of the issuer to show the favicon correctly. If the user doesn't add such an extension in the issuer, a default icon is shown.

There will be a warning to ask the user to grant extension permission to read the website favicon when enable this feature at the first time.

[mymindstorm]

Just FYI, not going to have time to look at this until around the 28th.

…

-------- Original message -------- From: Zhe Li ***@***.***> Date: 10/17/23 3:01 AM (GMT-06:00) To: Authenticator-Extension/Authenticator ***@***.***> Cc: Brendan Early ***@***.***>, Review requested ***@***.***> Subject: Re: [Authenticator-Extension/Authenticator] add favicon support (PR #1118) @Sneezry<https://github.com/Sneezry> requested your review on: #1118<#1118> add favicon support. — Reply to this email directly, view it on GitHub<#1118 (comment)>, or unsubscribe<https://github.com/notifications/unsubscribe-auth/AGUAT3RAWUFSBGFHDIRN4KDX7Y3N7AVCNFSM6AAAAAA6DMQTWKVHI2DSMVQWIX3LMV45UABCJFZXG5LFIV3GK3TUJZXXI2LGNFRWC5DJN5XDWMJQGY3TINZTGEYDQNQ>. You are receiving this because your review was requested.Message ID: ***@***.***>

[jaredzimmerman]

Is requesting permission to read favicons absolutely necessary? it seems really unnecessary from a UX perspective, even having a setting at all seems somewhat unneeded.

[Sneezry]

Is requesting permission to read favicons absolutely necessary? it seems really unnecessary from a UX perspective, even having a setting at all seems somewhat unneeded.

I do not want to embed favicons in the extension. That will increase the size of the package. For MV3, favicon permission is required, and chrome://favicon permission is required for MV2.

I add these new permissions as optional permission, only when this feature is enabled the extension will ask the permission.

Because this feature requires new permissions, we must have a new option to let the users disable it.

[jaredzimmerman]

How many 2FA will users have couple 100s max? even high res/svg favicons stored in local storage the size will be trivial.

[Sneezry]

How many 2FA will users have couple 100s max? even high res/svg favicons stored in local storage the size will be trivial.

The question is which favicons should we embed in the package? Google, Microsoft, Twitter, Facebook, GitHub... and what? And we can expect that there will be issues for missing favicons.

"Hey, please add xxx's favicon in the extension..."

We don't want to have such an issue.

[jaredzimmerman]

No, don't pre-install any favicons, get them when the user adds a site for a new code gen and store it in LocalStorage, you don't need to pre-install anything.

[Sneezry]

I got your point. LocalStorage has a 5MB limit, we can't store too much icons in that place. Most favicons are PNGs and ICOs, and if we want to store them in LocalStorage, we need to convert them to Base64 strings first. Again, base64 strings are ~1/3 bigger than the raw binary. Also, it can't be synced.

[jaredzimmerman]

I'm not sure that's accurate, also you don't need to sync the actual assets across installs, just the pointer, after sync on a new machine just have the local extension redownload from the synced pointers per code item. You can just grab a single variant of the favicon rather than trying to store full icos

[Sneezry]

you don't need to sync the actual assets across installs, just the pointer

chrome://favicon/xxx is the pointer, and it requires a new permission to access to.

[jaredzimmerman]

Use the URL matching function you already have to get the TLD for each item, on new install of the extension run the sync and redownload the favicons to that new machine, you're already storing matching URLS for each item.

[Sneezry]

redownload the favicons

Where should the extension download the favicons? If it downloads from chrome://favicon/xxx, this PR meets your expectation. If it downloads from <TLD>/favicon.ico, it will require <all_urls> permission, which is not acceptable.

[jaredzimmerman]

Ah! understood, my extension has <all_urls> so that's what I was comparing to. I understand what you're saying now, is there not metadata in chrome://favicon/xxx that allows you to match against the URL that your using for your site matching function? I'm not super familiar with how chrome stores favicons locally since I haven't needed to use that function.

[Sneezry]

For MV2, we can access to the favicons with this URL: chrome://favicon/<TLD>, for example, chrome://favicon/https://www.google.com. This requires chrome://favicon permssion.

For MV3, we can access to the favicons with this URL: <extension-url>/_favicon?pageUrl=<TLD>&size=16, for example, chrome-extension://bhghoamapcdpbohphigoooaddinpkbai/_favicon?pageUrl=https%3A%2F%2Fwww.google.com&size=16. This requires favicon permission.

[mymindstorm]

Have not forgotten about this. Next available time to work on this is Wednesday .