LGTM3

On Monday, January 8, 2024 at 9:48:00 PM UTC+1 Nicolás Peña wrote:

Hey Alex,

Thanks for the clarification. We will keep bundling intents that we 
consider small or non-controversial in the future.

Regarding the naming of `disconnect`, we did have the TAG mention no 
concerns on the API, albeit under the original name of `revoke`. We changed 
the name to `disconnect` because FedId CG members felt that `revoke` was 
confusing and `disconnect` seemed clearer 
<https://github.com/fedidcg/FedCM/issues/496#issuecomment-1818903120>. I 
can comment on the TAG review in case they want to chime in, but given that 
they did not mention the precedence in the initial review, I do not 
anticipate there being a strong feeling about it.

It is true we did not consider `forget`, but we think this is not ideal for 
our API. First, the Web Bluetooth and Web HID are about physical devices, 
which manifests itself in the fact they have `getDevices` methods. So the 
precedence from that side is weak, since these are very different APIs. 
Second, it is arguable that `disconnect` has more precedence in the space 
we are operating on, in choices like the OpenID Connect 
<https://openid.net/specs/openid-connect-core-1_0.html> standard, FedCM’s 
specification’s Connected Accounts Set 
<https://fedidcg.github.io/FedCM/#browser-connected-accounts-set>, and the 
language used in documentation for developers (e.g. in google sign-in 
<https://developers.google.com/identity/sign-in/android/disconnect>) and 
for users (e.g. in facebook connect 
<https://www.facebook.com/business/help/connect-instagram-to-page>). We 
think this industry-specific precedence is stronger, and one that will be 
more relatable to the developers that use the API. Third, if we adopted 
`forget`, it would be unclear what the endpoint should be named as, since 
something like `forget_endpoint` seems off and confusing.

Based on the above, while we are open to reconsidering, we think the naming 
`disconnect` strikes the right balance and remains ready for API OWNERS to 
review. Thanks for taking the time to look into it! Let us know what you 
think.


Thanks for weighing through the advantages and disadvantages of renaming.
Alex - if you still feel strongly about this, maybe the best path is to 
file an issue?
 



On Monday, January 8, 2024 at 1:03:58 AM UTC-5 Alex Russell wrote:

hey Nicholas,

Apologies for the slow follow up here. As a general matter I don't have a 
strong opinion about bundling of intents, assuming they're all 
non-controversial and don't need additional review. That suggests as 
case-by-case review, which also seems fine here. 

I'm happy for the domain hint to sail along under this intent, but I'm 
wondering if you'd considered chatting with the TAG about the naming of 
`disconnect()`. We have precedent in Web Bluetooth 
<https://chromestatus.com/feature/4797798639730688> and in Web HID 
<https://chromestatus.com/feature/5723581527883776>to use `forget()` 
instead, and so if we're going to go with something else here, it probably 
needs some rationalisation. If it seems like that will take more time than 
you'd like, I'd suggest leaving this intent for hints and starting a new 
intent to break out `disconnect()`/`forget()`.

Best,

Alex

On Wednesday, January 3, 2024 at 8:59:25 AM UTC-8 n...@google.com wrote:

Hey Alex,

You are right that they are only related in that they are extensions of the 
FedCM API. We bundled them up together in the same intent since we 
considered them small enough such that a single intent containing both is 
still manageable, and this reduces the burocratic overhead for shipping 
these features in our team. We have done this before too, like here 
<https://groups.google.com/a/chromium.org/g/blink-dev/c/UUCvZ27-gaI/m/F36Tq4hmBQAJ>
 
and here 
<https://groups.google.com/a/chromium.org/g/blink-dev/c/nKGUbzcVXt8/m/wE0REV8_AQAJ>.
 
Are you ok with us continuing to bundle small extensions of a single API 
into a single intent, even if not very related?

On Wednesday, January 3, 2024 at 11:43:34 AM UTC-5 sligh...@chromium.org 
wrote:

Hey Nicolas:

Sorry for not fully understanding the impact of these features. A (very) 
quick read on my end suggests that they might not be related? Is there a 
code example or explainer that highlights why they belong in the same 
Intent?

Thanks,

Alex

On Tuesday, January 2, 2024 at 8:42:38 AM UTC-8 Nicolás Peña wrote:

Brief update: both spec PRs are now merged. Hoping to ship in M122.

On Friday, December 15, 2023 at 2:29:58 PM UTC-5 Nicolás Peña wrote:

Contact emails

n...@chromium.org


Explainer

Domain Hint (formerly hosted domain): https://github.com/fedidcg/Fed
CM/issues/427

Disconnect (formerly revoke): https://github.com/fedidcg/FedCM/issues/496

(Note: in the FedCM team, we have explainers in the form of GitHub issues 
per feedback 
<https://github.com/fedidcg/FedCM/issues/431#issuecomment-1425025469> from 
FedID CG. The issue and the first comment are the explainers in each case.)

Specification

Domain Hint: https://github.com/fedidcg/FedCM/pull/512

Disconnect: https://fedidcg.github.io/FedCM/#browser-api-identity-creden
tial-disconnect 

Note on spec PR merging policy (to answer the question “why has the first 
not been merged”): in the FedID CG, we have agreed that non-editorial spec 
PRs require review from two implementations. Disconnect has been approved, 
while domainHint is still under review. Both features have been discussed 
thoroughly in the FedID CG and the feedback there has been incorporated.

Summary

Allows showing only accounts matching a given domain hint in the FedCM 
account chooser, and allows disconnecting a federated login account via the 
relying party website. With domain hint, developers may provide a better UX 
by only showing the federated login accounts from the domain that they 
accept. With the disconnect API, a relying party (RP) may notify the 
identity provider (IdP) that an IdP account previously used via FedCM in an 
RP is now disconnected, and hence using that account again via federated 
login would require treating it as a new account.

Blink component

Blink>Identity>FedCM 
<https://bugs.chromium.org/p/chromium/issues/list?q=component:Blink%3EIdentity%3EFedCM>

TAG review

https://github.com/w3ctag/design-reviews/issues/893

TAG review status

Issues addressed

Risks

Interoperability and Compatibility

These are small additions to the FedCM API, which has (general) support 
from WebKit and Mozilla. They haven't shipped FedCM yet, but it would not 
be a lot more work to add these features. If a user agent did not have 
domain hint support, this would mean it would show more accounts in the 
chooser compared to user agents which do have domain hint support. Not 
adding disconnect would mean that this feature of allowing RPs to 
disconnect accounts would not be available in the browser, but it would not 
impact the FedCM API otherwise.


Gecko: Positive for disconnect and no signal yet for domainHint. Firefox 
asked us to not send standards positions requests for small FedCM 
additions, and to instead rely on pull requests. See 
https://github.com/fedidcg/FedCM/pull/512 and https://github.com/fedidcg/Fed
CM/pull/515.

WebKit: No signal (https://github.com/WebKit/standards-positions/issues/249)

Web developers: Positive. This is a feature requested by developers to 
satisfy existing flows which break once third-party cookies are removed.

Other signals:

Ergonomics

It will be often used within the FedCM API. We do not see ergonomics risks.


Activation

Domain hint can be polyfilled via login hint but it would be pretty 
cumbersome to do so. The disconnect API would be hard to polyfill, but 
could perhaps be done in a non-user friendly way via popups. This would 
still be imperfect since the browser knowledge about the connection would 
not be cleared, only the IdP-side disconnection would occur.


Security

The Disconnect endpoint will use CORS. An RP may not impact the connection 
status of accounts not belonging to that RP origin.


WebView application risks

Does this intent deprecate or change behavior of existing APIs, such that 
it has potentially high risk for Android WebView-based applications?

N/A (FedCM does not work on WebViews)


Debuggability

Console errors and DevTools issues will be used to highlight any issues 
with the disconnect call.


Will this feature be supported on all six Blink platforms (Windows, Mac, 
Linux, Chrome OS, Android, and Android WebView)?

FedCM is not supported on Android WebView.


Is this feature fully tested by web-platform-tests 
<https://chromium.googlesource.com/chromium/src/+/main/docs/testing/web_platform_tests.md>
?

Yes. Look for domainhint and disconnect in https://wpt.fyi/results/creden
tial-management?label=experimental&label=master&aligned.


Flag name on chrome://flags

FedCmDomainHint, FedCmDisconnect

Finch feature name

FedCmDomainHint, FedCmDisconnect

Requires code in //chrome?

True

Tracking bug

https://bugs.chromium.org/p/chromium/issues/detail?id=1473135 (follow 
implementations in the two BlockedOn bugs)

Launch bug

https://launch.corp.google.com/launch/4273848

Estimated milestones

No milestones specified


Anticipated spec changes

Open questions about a feature may be a source of future web compat or 
interop issues. Please list open issues (e.g. links to known github issues 
in the project for the feature specification) whose resolution may 
introduce web compat/interop risk (e.g., changing to naming or structure of 
the API in a non-backward-compatible way).

None

Link to entry on the Chrome Platform Status

https://chromestatus.com/feature/5202286040580096

This intent message was generated by Chrome Platform Status 
<https://chromestatus.com/>.

-- 
You received this message because you are subscribed to the Google Groups 
"blink-dev" group.
To unsubscribe from this group and stop receiving emails from it, send an email 
to blink-dev+unsubscr...@chromium.org.
To view this discussion on the web visit 
https://groups.google.com/a/chromium.org/d/msgid/blink-dev/4cd7aaae-4833-4715-9ff3-7b5023b624b1n%40chromium.org.

Reply via email to