-
Notifications
You must be signed in to change notification settings - Fork 73
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
Update IDP signin status API explainer #505
Conversation
Per discussion at TPAC.
@samuelgoto @martinthomson @bvandersloot-mozilla -- could you sanitycheck that this represents what we discussed? |
Also should I rename this to "IDP Login Status API" now, since we changed signin to login in the API? |
As described in https://github.com/fedidcg/login-status and w3c-fedid/FedCM#505 [email protected] Bug: 1451396 Change-Id: I10b4a7b93e72184f86d5116b2708d3918a68573c Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/4874093 Reviewed-by: Zachary Tan <[email protected]> Commit-Queue: Zachary Tan <[email protected]> Auto-Submit: Christian Biesinger <[email protected]> Cr-Commit-Position: refs/heads/main@{#1199147}
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.
If we want to keep this updated, I think there are some more changes needed:
- A few places have "signed-in" or "signed-out" where it should use "logged-in" or "logged-out"
- Should the state naming also change?
- The alternatives considered seciton also needs to be updated. In particular, it still mentions "action=signin".
Just wondering, since there is a lot of duplication between the two explainer, why wouldn't we just point people to this explainer instead [1] for the specifics, and this explainer can go over how FedCM intends to use the signal ? |
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 addressed the comments.
I added a link to the login status explainer but wanted to keep the two sections in this document so readers don't need to read two documents to understand the proposal.
SHA: aafc66f Reason: push, by samuelgoto Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
SHA: aafc66f Reason: push, by pull[bot] Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
* Update IDP signin status API explainer Per discussion at TPAC. * restrict to secure context * s/signed/logged/g * remove action * ted comments * better describe parsing of the header * review comments
Per discussion at TPAC.