-
Notifications
You must be signed in to change notification settings - Fork 55
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
[Family Safety API] Official API Review #3036
base: main
Are you sure you want to change the base?
Conversation
Family Safety API Draft
/// Please see https://aka.ms/EdgeFamilySafetyContentFiltering for more details. | ||
/// | ||
/// The `uris` parameter value are list of uris that is a wildcard string matched aganist the | ||
/// navigation uri. This is a glob style wildcard string in which a `*` matches zero or more characters and |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Do we use this syntax anywhere else? Is this following some kind of existing practice?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Same format is used by custom scheme registration AllowedOrigins and AddWebResourceRequestedFilter. The latter has a big fancy table.
Documentation should probably say "using the same format as AddWebResourceRequestedFilter" and link to that page, so developers can reach the big fancy table.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I will open an ADO item to make a new doc concepts page for this matching syntax that we can link to from here.
Before then, we can have this documentation just do a brief one sentence summary and link to the AddWebResourceRequestedFilter page and say to follow the guidance for its syntax details.
|
||
/// `GetFamilySafetyAllowedUris` and `SetFamilySafetyAllowedUris` allow developers to get and set | ||
/// the list of URIs that will be allowed by the Family Safety filter even when in Block All mode. | ||
/// Family Safety provides web filtering control in two modes: Allow-all and Block-all. In |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
How do the allow-all and block-all modes relate to this API? Is block-all another way of saying IsFamilySafetyEnabled==true?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Here's my interpretation:
IsFamilySafetyEnabled | Block mode | Result |
---|---|---|
False | * | All URIs are allowed |
True | Allow-all | Parent provides list of block URIs; anything unknown is allowed |
True | Block-all | Parent provides list of allowed URIs; anything unknown is blocked |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is a good table we should include in our documentation:
| IsFamilySafetyEnabled | Family Safety Default Block Mode | Result |
| --- | --- | --- |
| `False` | * | All URIs are allowed |
| `True` | Allow by default | Parent provides list of block URIs; anything unknown is allowed |
| `True` | Block by default | Parent provides list of allowed URIs; App provides FamilySafetyAdditionalAllowedUris list of additional allowed URIs; anything unknown is blocked |
Also, please ensure this documentation to describe in words what the table says esp including how FamilySafetyAdditionalAllowedUris interacts with it.
/// the list of URIs that will be allowed by the Family Safety filter even when in Block All mode. | ||
/// Family Safety provides web filtering control in two modes: Allow-all and Block-all. In | ||
/// Allow-all mode, only sites that are blocked by the parents will be blocked. In | ||
/// blocked-all mode, only allowed sites that are allowed by the parents are allowed. In this |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
/// blocked-all mode, only allowed sites that are allowed by the parents are allowed. In this | |
/// block-all mode, only allowed sites that are allowed by the parents are allowed. In this |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please fix
/// `GetFamilySafetyAllowedUris` and `SetFamilySafetyAllowedUris` allow developers to get and set | ||
/// the list of URIs that will be allowed by the Family Safety filter even when in Block All mode. | ||
/// Family Safety provides web filtering control in two modes: Allow-all and Block-all. In | ||
/// Allow-all mode, only sites that are blocked by the parents will be blocked. In |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Where's the API to block a site?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Out of scope for this feature.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please ensure documentation makes it clear that this is about enabling family safety to respect OS family safety settings and not to configure family safety specific to your app, other than the additional allowed URI list which is just to ensure the app can work with family safety.
options->put_IsFamilySafetyEnabled(TRUE); | ||
// appassets.example is used as an example app content. All subdomain are added to the allow list | ||
const WCHAR* allowedUris[1] = {L"https://appassets.example/*"}; | ||
options->SetFamilySafetyAllowedUris(1, allowedUris); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
When family safety is enabled, isn't there a list of sites to allow/block in the family account already? Is Get returning that cloud information, or is this a local-override that's merged with the cloud values?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
My interpretation is that this list is merged with the cloud-controlled values. Maybe call this FamilySafetyAdditionalAllowedUris? The word "Additional" already is in use in this class for a similar concept (AdditionalBrowserArguments).
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Local only. There is no developer access to the family safety's cloud list. Parent explicit block list wins over this local allow list.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Let's rename: FamilySafetyAdditionalAllowedUris
=== | ||
|
||
# Background | ||
Our customers have asked for a new API to toggle the Family Safety feature on and off. The Family |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Since the value is locked once the browser process starts, I think "toggling" is overselling the feature. You can't toggle it. You can merely choose whether to enable it or not.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please fix. Replace with 'enable'
# Examples | ||
## WinRT and .NET | ||
```c# | ||
void CreateEnvrionmentWithOption() |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
TYPO
void CreateEnvrionmentWithOption() | |
void CreateEnvironmentWithOption() |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, please fix.
// by parents. | ||
CoreWebView2EnvironmentOptions options = new CoreWebView2EnvironmentOptions(); | ||
options.IsFamilySafetyEnabled = true; | ||
// appassets.example is used as an example app content. All subdomain are added to the allow list |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Comment here does not match comment in IDL. Which is correct?
Comment here implies that https://subdomain.appassets.example/
is allowed (subdomain). But the comment in IDL does not mention subdomains. It says that *
is a wildcard, which therefore matches paths under the appassets.example
domain, but not subdomains.
To clarify, given the pattern https://appassets.example/*
, which of the following URIs are allowed?
URI | Allowed? |
---|---|
https://subdomain.appassets.example/ |
|
https://subdomain.appassets.example/path |
|
https://appassets.example/ |
|
https://appassets.example/path |
|
http://appassets.example/ |
|
http://appassets.example/path |
If the comment in the IDL is correct, then there doesn't appear to be a way to securely match subdomains, because the *
can match slashes too: https://*.appassets.example/
will inadvertently match https://hackersite.example/attack.appassets.example/
.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This comment is out of date. There is no good way to match subdomains with this.
options->put_IsFamilySafetyEnabled(TRUE); | ||
// appassets.example is used as an example app content. All subdomain are added to the allow list | ||
const WCHAR* allowedUris[1] = {L"https://appassets.example/*"}; | ||
options->SetFamilySafetyAllowedUris(1, allowedUris); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
My interpretation is that this list is merged with the cloud-controlled values. Maybe call this FamilySafetyAdditionalAllowedUris? The word "Additional" already is in use in this class for a similar concept (AdditionalBrowserArguments).
/// When `IsFamilySafetyEnabled` is `TRUE` WebView2 provides the same Family Safety | ||
/// functionality as the Edge browser. | ||
/// Family Safety is a set of features available on Windows for managing children Internet | ||
/// priviliges. Microsoft account may be linked to have a family relationship, with adults |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
/// priviliges. Microsoft account may be linked to have a family relationship, with adults | |
/// privileges. Microsoft account may be linked to have a family relationship, with adults |
Overall, this comment block could do with an editing pass.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please fix typo.
Please revise paragraph or ask for PM to help rephrase.
|
||
/// `GetFamilySafetyAllowedUris` and `SetFamilySafetyAllowedUris` allow developers to get and set | ||
/// the list of URIs that will be allowed by the Family Safety filter even when in Block All mode. | ||
/// Family Safety provides web filtering control in two modes: Allow-all and Block-all. In |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Here's my interpretation:
IsFamilySafetyEnabled | Block mode | Result |
---|---|---|
False | * | All URIs are allowed |
True | Allow-all | Parent provides list of block URIs; anything unknown is allowed |
True | Block-all | Parent provides list of allowed URIs; anything unknown is blocked |
/// Family Safety provides web filtering control in two modes: Allow-all and Block-all. In | ||
/// Allow-all mode, only sites that are blocked by the parents will be blocked. In | ||
/// blocked-all mode, only allowed sites that are allowed by the parents are allowed. In this | ||
/// scenario, apps using WebView2 will have their content blocked if enabled Family Safety in WebView2. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
"In this scenario" - which scenario are we talking about? The allow-all scenario or the block-all scenario?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please revise this paragraph.
/// Allow-all mode, only sites that are blocked by the parents will be blocked. In | ||
/// blocked-all mode, only allowed sites that are allowed by the parents are allowed. In this | ||
/// scenario, apps using WebView2 will have their content blocked if enabled Family Safety in WebView2. | ||
/// Please see https://aka.ms/EdgeFamilySafetyContentFiltering for more details. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The comment text implies that this list of allowed URIs is ignored in allow-all mode.
What happens if Family Safety is in allow-mode, parent has blocked https://example.com/
, and the app adds https://example.com/
to the FamilySafetyAllowedUris? Who wins - the app or the parent? The comment above implies that the parent wins, and the URIs are blocked. Is there a way for the app to detect this case, so it knows that the app will not be functional and can provide a better error message? Or does the app just have to navigate to its home page, observe the navigation error, and try to guess whether the navigation failure was due to Family Safety (as opposed to a network or server error).
If the app wins, then maybe call this FamilySafetyAlwaysAllowedUris (the "always" implying that this overrides parent settings).
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The comment is correct: the parent list wins.
Additionally, we need to document what happens when Family Safety blocks a website (auto navigated to the Family Safety site), and recommendations on Family Safety websites that need to be allowed in order for this all to work.
/// Please see https://aka.ms/EdgeFamilySafetyContentFiltering for more details. | ||
/// | ||
/// The `uris` parameter value are list of uris that is a wildcard string matched aganist the | ||
/// navigation uri. This is a glob style wildcard string in which a `*` matches zero or more characters and |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Same format is used by custom scheme registration AllowedOrigins and AddWebResourceRequestedFilter. The latter has a big fancy table.
Documentation should probably say "using the same format as AddWebResourceRequestedFilter" and link to that page, so developers can reach the big fancy table.
=== | ||
|
||
# Background | ||
Our customers have asked for a new API to toggle the Family Safety feature on and off. The Family |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please fix. Replace with 'enable'
# Examples | ||
## WinRT and .NET | ||
```c# | ||
void CreateEnvrionmentWithOption() |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, please fix.
// by parents. | ||
CoreWebView2EnvironmentOptions options = new CoreWebView2EnvironmentOptions(); | ||
options.IsFamilySafetyEnabled = true; | ||
// appassets.example is used as an example app content. All subdomain are added to the allow list |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This comment is out of date. There is no good way to match subdomains with this.
/// Please see https://aka.ms/EdgeFamilySafetyContentFiltering for more details. | ||
/// | ||
/// The `uris` parameter value are list of uris that is a wildcard string matched aganist the | ||
/// navigation uri. This is a glob style wildcard string in which a `*` matches zero or more characters and |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I will open an ADO item to make a new doc concepts page for this matching syntax that we can link to from here.
Before then, we can have this documentation just do a brief one sentence summary and link to the AddWebResourceRequestedFilter page and say to follow the guidance for its syntax details.
options->put_IsFamilySafetyEnabled(TRUE); | ||
// appassets.example is used as an example app content. All subdomain are added to the allow list | ||
const WCHAR* allowedUris[1] = {L"https://appassets.example/*"}; | ||
options->SetFamilySafetyAllowedUris(1, allowedUris); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Local only. There is no developer access to the family safety's cloud list. Parent explicit block list wins over this local allow list.
/// `GetFamilySafetyAllowedUris` and `SetFamilySafetyAllowedUris` allow developers to get and set | ||
/// the list of URIs that will be allowed by the Family Safety filter even when in Block All mode. | ||
/// Family Safety provides web filtering control in two modes: Allow-all and Block-all. In | ||
/// Allow-all mode, only sites that are blocked by the parents will be blocked. In |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please ensure documentation makes it clear that this is about enabling family safety to respect OS family safety settings and not to configure family safety specific to your app, other than the additional allowed URI list which is just to ensure the app can work with family safety.
/// the list of URIs that will be allowed by the Family Safety filter even when in Block All mode. | ||
/// Family Safety provides web filtering control in two modes: Allow-all and Block-all. In | ||
/// Allow-all mode, only sites that are blocked by the parents will be blocked. In | ||
/// blocked-all mode, only allowed sites that are allowed by the parents are allowed. In this |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please fix
/// Family Safety provides web filtering control in two modes: Allow-all and Block-all. In | ||
/// Allow-all mode, only sites that are blocked by the parents will be blocked. In | ||
/// blocked-all mode, only allowed sites that are allowed by the parents are allowed. In this | ||
/// scenario, apps using WebView2 will have their content blocked if enabled Family Safety in WebView2. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please revise this paragraph.
/// Allow-all mode, only sites that are blocked by the parents will be blocked. In | ||
/// blocked-all mode, only allowed sites that are allowed by the parents are allowed. In this | ||
/// scenario, apps using WebView2 will have their content blocked if enabled Family Safety in WebView2. | ||
/// Please see https://aka.ms/EdgeFamilySafetyContentFiltering for more details. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The comment is correct: the parent list wins.
Additionally, we need to document what happens when Family Safety blocks a website (auto navigated to the Family Safety site), and recommendations on Family Safety websites that need to be allowed in order for this all to work.
[interface_name("Microsoft.Web.WebView2.Core.ICoreWebView2EnvironmentOptions3")] | ||
{ | ||
// ICoreWebView2EnvironmentOptions3 members | ||
Boolean IsFamilySafetyEnabled { get; set; }; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
For both APIs we want to remove brand names and use generic term "Parental Controls":
IsFamilySafetyEnabled becomes:
- AreParentControlsEnabled
FamilySafetyAllowedUris becomes:
- ParentalControlsAdditionalAllowedUris
But continue to use Family Safety in the documentation.
Family Safety API Review