-
Notifications
You must be signed in to change notification settings - Fork 2.4k
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
Add support for time limits for Web API permissions in brave-core #14126
Comments
Did some research, a possible solution looks like this:
❔ It looks like a request bubble can show many permission requests at once. The current delegate implementation assumes we accept all or decline all. The default timer implementation will also be applied to all listed permissions. Can we mention that in the issue description? I guess we can technically support different timers for different permissions requested simultaneously in the one UI bubble, but is it actually needed? I haven't look at ephemeral storage-like behavior yet, but I assume it should not be that hard to implement knowing that the ephemeral storage feature already has some working machinery for a tab tracking which can be connected with this feature. |
That all sounds great @goodov! Also wondering if your digging gave you a sense of how difficult it would be to partition permissions (e.g., whether google maps embedded on site A is independent of whether google maps embedded on site B, both of which are independent of what permissions https://www.google.com/maps has, etc). Its possible upstream is already considering this (as part of the overall NetworkIsolationKey work) but if they haven't / aren't planning to, we should at least. If that would be "easy" to do as part of this issue, I'll add and update the description above. But if it seems like a significant amount of additional work, i'll create another issue for it, and we can sequence however you both think would be best. |
@pes10k Let's do step by step, having an additional issue would be fine |
Extension API permissions logic uses another mechanism, not connected with Entry point for extension API permissions is |
@goodov would your current approach also work for an "allow until end of session" option? |
@pes10k There are currently two approaches now that I'm considering.
The Chromium-based option is very appealing, but it can take a lot of time, and the final result may not be as flexible as we want to, so we discussed it with Ivan and decided to implement Brave-specific variant with possible Chromium reintegration in future.
the current approach with our service will allow me to connect it with |
Here is a discussion with Chromium folks about improving their permission constraints implementation so we can use it directly at some point. Looks like they're happy to accept my recommended changes and we can actually do this. Just need some time to implement it and make a CL. https://bugs.chromium.org/p/chromium/issues/detail?id=1147918#c4 |
That sounds terrific @goodov! Would we need to wait for upstream before we can enable for Brave users then? Or can we enable in Brave while the upstream work is being sorted out? |
We will enable our implementation first (currently in review) and will migrate to upstream implementation when it will be ready (may take few months to get required Chromium changes in our codebase). |
There are a list of some permission requests that are not handled with a general implementation. Some of them use own storage (not
Technically it's possible to add required logic for each of this requests, but I don't think we should do this. It's very likely that each one of them will introduce some conflict to Chromium (patches, etc), it's possible that revoke of some of them can be harmful for a runtime logic, and it's possible that some of them are very rare. From my POV, we can/should support |
I think that all sounds great @goodov ! And fwiw, I think |
@goodov Regarding to |
We have a testplan in-hand (via Slack), but I'm leaving |
Removed |
Verified
NOTES (especially for the rest of the @brave/legacy_qa team):
Time-based cases20-seconds, Allow permission (
20-seconds, Allow permission (
Origin-based casesOrigin-based behavior, multiple tabs (
Origin-based behavior, leave tab open, restart (
Origin-based behavior, close tab, open new tab (
24-hour / 1-week cases24-hours, negative test (25 hours) (
24-hours positive test (23 hours) (
1-week, negative test (8 days) (
1-week, positive test (
Allow/Block forever casesAllow forever (
Block forever (
Consecutive-revocation casesConsecutive-revocation permissions (
Negative/current-parity casesBlock (
Allow (
Allow (
Migration (stored prefs)Block (
Allow (
Verification passed on
Verified selected test case from above test plan 20-seconds, Allow permission (camera + microphone )Verified permission prompt was triggered Origin-based behavior, multiple tabs (location)Verified visiting browserleaks.com/geo triggers permission prompt Allow forever (location)Verified visiting the page triggers permission prompt 24-hours positive test (23 hours) (microphone)Verified permission prompt was shown and was able to set 24h Consecutive-revocation permissions (location)Verified visiting Upgrade test from 1.23.xVerified that in 1.23.x, Verification passed on
Verified selected test case from above test plan 20-seconds, Allow permission_Normal Tab (camera + microphone )Verified permission prompt was triggered 20-seconds, Allow permission_PrivateTab (camera + microphone )Verified permission prompt was triggered 20-seconds, Allow permission_TorTab (camera + microphone )Verified permission prompt was triggered Origin-based behavior, multiple tabs (location)
Allow forever (location)
24-hours positive test (23 hours) (microphone)Verified permission prompt was shown and was able to set 24h Verified permission was allowed in brave://settings/content Verified microphone permissions are Allowed for the site after 23h : Consecutive-revocation permissions (location)Verified visiting
|
Currently, when requesting a permission, Chromium allows users to say yes or no, w/o an intermediate option.
For example, i can give google maps access to my location forever (or deny), or i can give the zoom web client access to my camera and mic forever (or deny), but there is no option to say "you can use my camera until i close the page" or "you can use my location for 24 hours".
Options
This issue is to create the ability to grant a page access to a permissioned API for the following options:
arbitrary amount of time
For 4, we likely wouldn't want to expose this option to users (I imagine we'd want so preset "one week" or similar options), but the implementation should allow for arbitrary lengths.
Current UI
Currently Brave uses the upstream Chromium options (and UI), which give the user only a permanent Yes or No option.
Other Browsers
Fiirefox
Firefox currently has a version of this for for some permissions. By default, "allow" and "deny" apply for the page, and "Remember this decision" makes the choice permanent.
Safari
Safari similarly by default grants permission for the page, with options to make the choice permeant.
Out-of-Scope and Other Considerations
The text was updated successfully, but these errors were encountered: