Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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
Keep HOWTO-chrome.md up to date #506
Keep HOWTO-chrome.md up to date #506
Changes from 2 commits
c665d3b
7807b03
35db075
671b116
013a93a
3ccbb07
8a6684e
a0f208f
File filter
Filter by extension
Conversations
Jump to
There are no files selected for viewing
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.
Thinking this might be more consistent language here, but not certain.
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.
While it's true that API is called on the RP site, the API caller could be the RP itself or the IdP (via SDK) or 3P library that uses FedCM API.
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.
providing this information seems like it would be unnecessary. What's the motivation for indicating this to the RP/IDP rather than just making this a client side only piece of information? Say for example, that the user doesn't auto select an account could that reveal information about whether or not it's a temp account versus a primary permanent account based on statistical analysis?
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.
Could you elaborate a bit on the attacking vector here? I'm not sure if I understand what you meant by temp account v.s. primary account. The boolean can only be true for "returning" account which means that the user has used that account in this browser client in the past.
For motivation and privacy & security considerations, please see #497
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.
Thanks, I'll have to give a read into this a bit further. I wasn't aware of the linked issue before this, but just came across it. Do you guys have a glitch.me demo of this feature built by chance? I'd like to play around with this feature a bit more to make sure I'm understanding the goals and am able to assess the potential impact correctly.
FWIW, don't block this on my review but my hunch does think this is probably oversharing information. If I come up with a practical attack then it could always be reverted later too.
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.
You could try https://webid-fcm.glitch.me after enabling the flag. The boolean will be displayed in the log section.
Note: in Chrome, after auto re-authen is triggered, it will be suppressed for 10 mins (or until a user explicitly signs 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.
Hey Kyle, ok with us merging this howto change or do you have any further comment?
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.
Yeah fine to merge as is. I'll take a look into this post merge when I pick up some time to see if I spot any concerns with it. If I do find something I'll open an issue with follow up suggested changes.