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.