Skip to content
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

The UA should not implicitly link different RPs and should rather rely upon user consent for each origin #315

Open
kdenhartog opened this issue Jul 14, 2022 · 14 comments

Comments

@kdenhartog
Copy link
Contributor

kdenhartog commented Jul 14, 2022

Today, one of the more annoying things that can occur is that default accounts that are signed into on one origin can get re-used as the same default account at a separate origin. For example, I maintain multiple accounts, which the first one I sign into is the one I default to. This default is then set and shared across different origins such as rpBrand1.example, rpBrand2.example, and subdomain.rpBrand3.example. While for many users this may be incredible convenient for them (especially people with a single account) it can also be an inconvenience and a privacy concern where the user is being automatically linked on an origin even if they don't intend to login to one of the origins.

For example, one of my accounts is managed under an enterprise policy which doesn't allow for me to take certain actions on a service offered, but if I use my secondary, non-default account I do have access to that service which I use. In this case, because the RPs are being linked and shared across origins it presents issues where my browser is being automatically configured in a way that I cannot change and I'm left to consistently account switch depending on the service I'm trying to access.

Instead, this spec should limit cross-origin linkage such that the user has to explicitly set the accounts and re-login to each origin so that they maintain maximum control over which accounts are being shared with which RPs.

@npm1
Copy link
Collaborator

npm1 commented Jul 15, 2022

This spec cannot control how origins within the same domain share cookies. The spec allows idtoken from the IDP origin to the RP origin, but it is up to the RP to choose whether the sign in state of the user should be stored in a cookie locked to that single origin or a cookie shared with origins within the same domain.

@kdenhartog
Copy link
Contributor Author

kdenhartog commented Jul 16, 2022

Couldn't the approved_clients list in section 5.2 be defined as required and become a list of origins that have registered for a particular account?

This could be done during the navigation.credentials.get() call where if the origin has not been seen when the user selects their account during the account chooser step the approved_clients list is updated in the browser state. Then on the re-auth flow the browser can detect which accounts are registered with this origin and can display only that list of accounts plus an "add new account" option.

@kdenhartog
Copy link
Contributor Author

This spec cannot control how origins within the same domain share cookies.

Also, just realized there may have been a bit of miscommunication. This isn't concerned with origins of the same domain, it's origins with different domains. E.g. rpBrand1.example, rpBrand2.example, and subdomain.rpBrand3.example are all different domains so shouldn't they also be different origins?

The practical example I'm thinking of here is that youtube.com and mail.google.com and gmail.com, but I see gmail.com is just a redirect to mail.google.com, so that doesn't seem to be a cross origin example like I originally thought. Another example of this would be facebook.com, instagram.com, and messenger.com. In the FB case though it's behaving as expected and not auto connecting the domains like Google's domains are. The point of this issue is to enforce this boundary via the API rather than letting the RPs automatically connect.

@npm1
Copy link
Collaborator

npm1 commented Jul 18, 2022

Couldn't the approved_clients list in section 5.2 be defined as required and become a list of origins that have registered for a particular account?

This could be done during the navigation.credentials.get() call where if the origin has not been seen when the user selects their account during the account chooser step the approved_clients list is updated in the browser state. Then on the re-auth flow the browser can detect which accounts are registered with this origin and can display only that list of accounts plus an "add new account" option.

This seems like something the user agent can do in their UI if they choose to do so (i.e. instead of listing all accounts, only listing the ones that are returning and more clearly separate the list of new accounts). What do you think?

@kdenhartog
Copy link
Contributor Author

This seems like something the user agent can do in their UI if they choose to do so (i.e. instead of listing all accounts, only listing the ones that are returning and more clearly separate the list of new accounts). What do you think?

Yeah that makes sense and my interpretation was that's the intended route of the spec today by making this attribute optional. Is that correct? The only additional requirement I hand in mind would be that this becomes a required attribute that the RP registers during the initial call so that the browser can enforce the boundary at the origin rather than allowing this to be potentially collapsed like it does in the youtube.com case today. With it being optional I'm thinking it potentially makes it not possible for the UA to be able to detect this and enforce on the origin boundary properly.

Speaking of which, I still haven't figured out how that's occurring but it is cross-browser so it must be something behind the scenes that YT as an RP was able to coordinate with the Google IDP since they are one of the few cases that can be coordinated in this way. This definitely isn't a common case (if it was I'd guess FB would have taken a similar route and not many consumer companies are also IDPs - enterprise/EDU is a different story), but it's something that I don't necessarily want to see encouraged by the spec without explicit user consent being involved at least.

@npm1
Copy link
Collaborator

npm1 commented Jul 19, 2022

Yeah that makes sense and my interpretation was that's the intended route of the spec today by making this attribute optional. Is that correct?

We currently show all accounts together in our UI, although I would argue that this is not ideal and having some indication about which account(s) have been used before is better.

The only additional requirement I hand in mind would be that this becomes a required attribute that the RP registers during the initial call so that the browser can enforce the boundary at the origin rather than allowing this to be potentially collapsed like it does in the youtube.com case today. With it being optional I'm thinking it potentially makes it not possible for the UA to be able to detect this and enforce on the origin boundary properly.

I don't follow the concern here. I will note that we already do our checks at the origin level. So for example RP1 getting sign in permission from IDP will not imply anything about RP2 in our API, even if RP1 and RP2 are same domain. That said, RP1 could create domain level cookies.

Speaking of which, I still haven't figured out how that's occurring but it is cross-browser so it must be something behind the scenes that YT as an RP was able to coordinate with the Google IDP since they are one of the few cases that can be coordinated in this way. This definitely isn't a common case (if it was I'd guess FB would have taken a similar route and not many consumer companies are also IDPs - enterprise/EDU is a different story), but it's something that I don't necessarily want to see encouraged by the spec without explicit user consent being involved at least.

Can you elaborate on what you are seeing?

@kdenhartog
Copy link
Contributor Author

kdenhartog commented Jul 19, 2022

I don't follow the concern here.

The primary concern here is cross domain login being allowed. A secondary concern is cross subdomain tracking. The reason this concerns me is for two reasons:

A.) The users login credentials (primarily username) can be used as an identifier for cross domain tracking which goes against the default value I'd like to see.
B.) By letting the user explicitly login on each domain (and preferably subdomain as well although domain cookies would subvert this) it allows the user to configure the accounts to each separate RP independently rather than encountering side effects from cross-subdomain account management such as when logged into multiple accounts where the first selected is a default then the same account is set by default cross domain.

I think the Google/YT example exemplifies the trouble I'm encountering here and am hoping would be blocked by this spec. Here's a video to show what I mean.

signin.cross.domain.mp4

See how there's cross domain login flows right now? This is confirmed in both Brave (chromium based) and Safari. I'd like to see this not be allowed.

@timcappalli
Copy link
Member

timcappalli commented Jul 20, 2022

This is same party federation vs third party federation. One goal of same party federation is single sign on. Why shouldn't this be allowed?

Should you have to sign in explicitly to both Google Sheets and Google Slides when interacting with Google Drive?

@cbiesinger
Copy link
Collaborator

I'm not sure what FedCM could do to prevent that anyway? Once we give the token to the RP, there's nothing stopping them from setting domain cookies. I'm not 100% sure how YouTube does it but it probably (?) involves a redirect through *.google.com.

@npm1
Copy link
Collaborator

npm1 commented Jul 20, 2022

To recap, FedCM is already using per-origin controls in its state machine, but it seems that there are ways to obtain cross-domain cookies (probably via redirects) which are being used by YouTube to sync user login journeys. This is not really actionable on this spec, can this one be closed?

@kdenhartog
Copy link
Contributor Author

Why shouldn't this be allowed?

I'm pretty sure that would cause a conflict with the per-origin controls being defined because the subdomain produces a different origin so this would make it cross-origin. As @cbiesinger pointed out this is extensible to achieve those types of flows.

but it seems that there are ways to obtain cross-domain cookies (probably via redirects) which are being used by YouTube to sync user login journeys

Wouldn't that make it a different RP with a different client_id since it's going across the origin boundaries set in the state machine? Or is it acceptable for many different origins to register as the same RP? That seems like it would be bypassing some token validation steps since the IDP is issuing the token to a specific client_id.

@kdenhartog
Copy link
Contributor Author

kdenhartog commented Jul 21, 2022

This is not really actionable on this spec, can this one be closed?

I agree that it's not fully actionable by this spec as the RP/IDP can subvert them (sometimes for valid reasons such as better UX. I definitely think this is extremely convienent), but I still think it's a good thing to discuss as to where we want the boundaries to lie here. Primarily because I think it will lead to some editorial clarifications as to the way some things are being defined and also some security and privacy considerations potentially to talk more about cross domain tracking and other such considerations like re-use of the client_id.

@samuelgoto
Copy link
Collaborator

samuelgoto commented Aug 18, 2022

I agree that it's not fully actionable by this spec as the RP/IDP can subvert them.

I think this is true (through subdomain cookie sharing and top-level redirects) and worth establishing: this isn't actionable by the FedCM spec.

I still think it's a good thing to discuss as to where we want the boundaries to lie here.

This seems like a worthy discussion, but one that I think should happen at a different layer (as you suggested, not actionable by the FedCM spec specifically).

Specifically, one of the places that I think would be most constructive to have this discussion is Privacy Principles Definition of "partition" which, as noted, there is not a shared / interoperable definition of across user agents.

I think the stance that FedCM can take is to allow each user agent to define the "privacy boundary" (e.g. for example with regards to First Party Sets) and to be an API that allows identity to cross that boundary with a user's consent.

So, may I suggest that we close this issue and follow on this discussion in this spec instead?

@kdenhartog
Copy link
Contributor Author

kdenhartog commented Aug 28, 2022

This is same party federation vs third party federation. One goal of same party federation is single sign on. Why shouldn't this be allowed?

I'm not against SSO, I'm just trying to make it clear that as it currently stands today the origin is where the boundary has been set and is where it will generally lie unless the user opts in to a FPS. So for example, if a user wishes to sign in once for an set of first party origins they can opt in to, but it shouldn't be implicitly handled by the UA without their knowledge in my opinion.

Should you have to sign in explicitly to both Google Sheets and Google Slides when interacting with Google Drive?

In my opinion the safer answer here is yes. This is because as is stated in 2.1.1.1 of the privacy principles

A privacy harm occurs if a person reasonably expects that they'll be using a different identity on a site, but the site discovers and uses the fact that the two or more visits probably came from the same person anyway.

Below that it states the following:

User agents can't, in general, determine exactly where intra-site context boundaries are, or how a site allows a person to express that they intend to change identities

So, if the UA is unable to determine the site boundaries (Google Sheets is a wholly separate service from gmail for example), and can't infer the users wishes of whether they intend to change identities, shouldn't it be required to prompt the user for their wishes to determine whether they want to share across boundaries whether that be cross origin or same-site? This to me seems to be the safer option that's more aligned with the privacy principles today even though it goes against the current state of the art.

I'm not sure what FedCM could do to prevent that anyway? Once we give the token to the RP, there's nothing stopping them from setting domain cookies.

Yes, and are these the types of behaviors we want to discourage without consent correct since it may go against the users wishes? That's the direction I'm hoping to land towards with this issue.

So this is why I put forward this possible solution. #315 (comment)

This seems to tread the balance between we want to discourage this by requiring RPs to explicitly retrieve credentials themselves via the APIs defined here rather than by passing credentials around either server side or via the domain level cookies. Assuming the refresh tokens need to be retrieved via this API as well then this would make it so the secondary RPs wouldn't be able to retrieve refresh tokens as well since it would require them redirecting back to the original RP who requested the credentials in order to refresh the token and re-store it in the top level domain which would be far more complicated then the second RP just requesting it's own credentials or the first RP requesting authorization for the secondary origins such that the UA can know they now share the same account and any one of the approved_clients could then request an updated credential.

I recognize that this isn't a perfect solution, however I'm trying to create patterns that are aligned with what some users may expect to behave more like an antipattern in order to discourage their usage while also creating a legitimate patterns that utilize user consent to offer a legitimate solution to the problem still.

That seems actionable to me. WDYT?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
5 participants