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

A not-yet logged in IDP has no route to success with this flow #442

Open
bvandersloot-mozilla opened this issue Feb 16, 2023 · 45 comments
Open

Comments

@bvandersloot-mozilla
Copy link
Collaborator

Enabling an option in which the IDP can provide a way to be logged in (via a controlled call to window.open) and then immediately be dropped back into the account chooser would be nice! I discussed something related to this in fedidcg/proposals#3 but it could be adapted here. I'll add a comment with a general shape of the proposal.

@cbiesinger
Copy link
Collaborator

Yes, this will be needed, especially if an RP wants to use FedCM for a button flow (as opposed to triggering it onload, where silent failure is probably acceptable)

#436 adds a signin_url field, which is part of what is needed here.

@bvandersloot-mozilla
Copy link
Collaborator Author

#436 adds a signin_url field, which is part of what is needed here.

Agreed. A key piece will be having the proposed dialog that shows that link show up when a user isn't yet logged in. In addition to that, there are two options:

  1. require the IDP support something like a solution to The IDP has to support additional infrastructure to support FedCM #441 so it can complete the flow in the opened link
  2. Have the sign-in status API signal to pending login requests that have already "chosen" that IDP that they should re-issue the account-list request. If that fails it should display a failure popup.

I think the best way forward is the latter.

@cbiesinger
Copy link
Collaborator

Yes option 2 sounds good to me.

But I think we need to figure out how to trigger this behavior. Maybe add a flag somewhere saying that if there are no accounts found, that we should show a signin prompt instead of returning failure silently?

And/or have the presence of transient user activation trigger this behavior?

@achimschloss
Copy link
Contributor

But I think we need to figure out how to trigger this behavior. Maybe add a flag somewhere saying that if there are no accounts found, that we should show a signin prompt instead of returning failure silently?

I think this is also another instance where the RP would want a flag to influence this behavior especially with the introduction of multi-idp support - see #438 (comment) - there could be a IDP specific flag where the RP could indicate to be wanting to enable a IDP sign-in from its site.

@Bojhan
Copy link

Bojhan commented Mar 24, 2023

This would be quite relevant to Seamless Access. We expect that users will move between many sites, wanting to access for example scientific content.

They will be shown the option, but it needs to be clear that they are signed in or still need to

@samuelgoto
Copy link
Collaborator

samuelgoto commented Aug 11, 2023

Enabling an option in which the IDP can provide a way to be logged in (via a controlled call to window.open) and then immediately be dropped back into the account chooser would be nice! I

#436 adds a signin_url field, which is part of what is needed here.

Agreed.

+1 that the signin_url can be the url that we point to in the window as @bvandersloot-mozilla described above.

But I think we need to figure out how to trigger this behavior. Maybe add a flag somewhere saying that if there are no accounts found, that we should show a signin prompt instead of returning failure silently?

Yeah, maybe an extra attribute like signedOut: signin || dismiss (we currently dismiss, so maybe that can be the default so that we can keep backwards compatibility?) that controls the create identity credential algorithm to open a window with the signin_url when the user is not logged in to the IdP is that we want?

signedout feels a bit awkward of a flag name to me, so happy to take suggestions on better names ...
maybe when_signedout, or signedout_mode, or ux_mode, or signin_when_signedout: true or
signedout_fallback or if_signedout or on_signedout?

await navigator.credentials.get({
  identity: {
    providers: [{
      // ...
      signedOut: "signin" // defaults to "dismiss"
    }]
  }
});

@bvandersloot-mozilla does this sound more or less right to you? any intuition on where this should hang?

There is also a related discussion on allowing users to "add/change accounts" when users are already signed-in, which we would expect not all IdPs would necessarily want to support (e.g. not all IdPs have the concept of "multiple accounts"), so maybe we need another flag like allowNewSessions: true?

await navigator.credentials.get({
  identity: {
    providers: [{
      // ...
      signedOut: "signin", // defaults to "dismiss"
      allowNewSession: true // defaults to false
    }]
  }
});

Another challenge here is (IdPs) supporting the combinatorial explosion of flags here, so another option here is to hard code a few known supported flows rather than individual flags with an enumeration of ux_modes = button | widget. For example:

await navigator.credentials.get({
  identity: {
    providers: [{
      // ...
      mode: "button" || "widget"
    }]
  }
});

So maybe we can start with the coarse mode and then introduce individual granular flags if/when needed?

Another related discussion is that of user activation. Outside of user activation, maybe we can assume mode=widget and inside user activation, maybe (probably not?) we can assume mode=button? So, maybe, we can just use user activation to decide which mode to use?

Another consideration is whether this should be a configuration per-IdP or at the IdentityCredential level. That is, when more than IdP is passed, should they have differently? So, maybe, this should actually be a parameter that is outside of proividers?

await navigator.credentials.get({
  identity: {
    mode: "button" || "widget"
    providers: [{
      // ...
    }]
  }
});
@cbiesinger
Copy link
Collaborator

hm, I think RPs may want different behavior at different times. For example, they may want to show an account chooser onload but only if the user is logged in; but they may have a "sign in" button which should not fail silently.

@samuelgoto
Copy link
Collaborator

hm, I think RPs may want different behavior at different times. For example, they may want to show an account chooser onload but only if the user is logged in; but they may have a "sign in" button which should not fail silently.

Yeah, I was thinking that IdPs would have the ability to have more than one configURL that they can expose for different products that behave one way (e.g. account choosers onload) or another (button flows).

So, RPs can still choose between two different configURLs, but IdPs also have the controls to expose or not one/both/either option.

@samuelgoto samuelgoto added the agenda+ Regular CG meeting agenda items label Aug 11, 2023
@bvandersloot-mozilla
Copy link
Collaborator Author

I think that enumerating a set of supported flows may be painting ourselves into a corner.

How does either proposal interact with a multiple-IDP use case?

@achimschloss
Copy link
Contributor

I think revisiting the discussion points in the multi-IDP PR #438 should help here as well as the general issue files on this (as this is largely about what an RP would want to happen on their site) - #348

aarongable pushed a commit to chromium/chromium that referenced this issue Oct 2, 2023
In this CL, we expose - behind a flag - a "mode" parameter in the
FedCM API and plumb it to the browser process through the mojom
interface.

This starts allowing the FedCM API to degrade more gracefully when the
user is *not* logged-in to the IdP.

Specific feature request here from Mozilla:

fedidcg/FedCM#442 (comment)

The "mode" (={button, widget}) parameter controls the internal
algorithms of FedCM to handle logged-out users. In mode=button, the
browser opens a pop-up window when users are logged-out, whereas in
mode=widget (the default) the prompt is just automatically dismissed.

Part (pre-requisite) for this larger feature:

fedidcg/FedCM#477 (comment)

Privacy and Security Early Reviews:

https://docs.google.com/document/d/1WtiG-JwlcuCJPI0EZdo61vxg6sjTAuxFFm5I2Jk4x6s/edit

The browser implementation, that will actually change based on this
flag, is being implemented in a separate CL (see gerrit CL dependency).

Bug: 1429083
Change-Id: If9f6b453cc5e16c479b89a16191d010892bd0377
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/4779439
Commit-Queue: Sam Goto <goto@chromium.org>
Reviewed-by: Yi Gu <yigu@chromium.org>
Reviewed-by: Emily Stark <estark@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1204181}
aarongable pushed a commit to chromium/chromium that referenced this issue Oct 6, 2023
In this CL, behind a flag, we make the algorithm degrade gracefully
for logged-out users when in "button" mode by opening a pop-up window
that allows the user to sign-in to the IdP (as opposed to dismissing the
request, when in "widget" mode)).

This allows the FedCM API to support button flows where the user
expects a fallback, as opposed to a dismissal.

fedidcg/FedCM#442 (comment)

Change-Id: Id1492f8f3e10fbd9e400b809caf64e90ac2806ca
Bug: 1490588
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/4790926
Commit-Queue: Sam Goto <goto@chromium.org>
Reviewed-by: Yi Gu <yigu@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1206673}
@philsmart
Copy link
Contributor

I've tried this with the multi-idp mode. As it is a global property, it seems to pop up the signin_url window for every IdP that you've not signed into before offering the account chooser. I am not sure how sensible that is.

@npm1
Copy link
Collaborator

npm1 commented Oct 12, 2023

If that comment is about the Chrome flag, we haven't worked on that in a while so it is likely not working correctly at this point. In multi-idp mode, I'd expect the user to pick the IDP they want to sign in to, not for one popup per IDP to open.

@philsmart
Copy link
Contributor

That's it (chrome flag), thanks. The behavior I observed was a popup per signed-out IdP. But I realize the multi-idp support is a WiP, so just adding a data point from testing.

@achimschloss
Copy link
Contributor

The behavior I observed was a popup per signed-out IdP. But I realize the multi-idp support is a WiP, so just adding a data point from testing.

Interesting, thats really not what you'd want for a final version. As noted in the CG call I think the RP/Authentication Intermediary should be able top open a pop-up in this case to let the user choose how to proceed (not a browser mediated UI, or at least both options must be available)

See also the implementation bug in Chrome to track this: https://bugs.chromium.org/p/chromium/issues/detail?id=1487268

@achimschloss
Copy link
Contributor

@samuelgoto seems this should be a proposal under https://github.com/fedidcg/FedCM/tree/main/proposals ?

@timcappalli
Copy link
Member

2023-11-28 call: prototype exists in Chrome Canary, please test

@samuelgoto
Copy link
Collaborator

Just a small update on this: we built a prototype in chrome canaries with the following formulation:

await navigator.credentials.get({
  identity: {
    mode: "button" || "widget"
    providers: [{
      // ...
    }]
  }
});

You can try this by flipping this flag in chrome canaries: chrome://flags/#fedcm-authz.

What it does is that, on mode=button, when the user is logged-out (based on what the browser knows about the Login Status API), it opens a pop-up window pointing the user to the login_url found in the configURL passed.

Any chance we have anyone around interested in giving this a try and letting us know if this behaves the way that they'd expect this to behave?

@achimschloss
Copy link
Contributor

Any chance we have anyone around interested in giving this a try and letting us know if this behaves the way that they'd expect this to behave?

FWIW - this is also already supported as an option in the demonstrator implementations (mode = button) on the RP side.

@philsmart
Copy link
Contributor

philsmart commented Dec 5, 2023

Yes. This looks like the implementation I tested with multi-idp support above. I uploaded a demo video of how it acts in that case, for reference.

@philsmart
Copy link
Contributor

I should add, that it behaves in the way described for a single IdP.

@philsmart
Copy link
Contributor

philsmart commented Dec 5, 2023

I have no good suggestions of how to fix that for multi-idps, even if you moved the mode option into the provider's array (i.e., per-provider) I am not sure how the RP would know to set it differently per IdP (or even why it would do that), and would probably just set it the same for each provider anyway (so I think it makes sense to set globally as it is). I guess, as already suggested in this thread, you might need the user to select which IdP to use first before trying to retrieve accounts from it, and therefore before deciding if the sign-in popup is needed.

@cbiesinger
Copy link
Collaborator

Yeah, we'd probably have to have an "IDP selection" dialog if the user is not signed in to any of the IDPs, and if the user is signed in to at least one IDP the dialog should still let the user sign in to one of the others

@hlflanagan
Copy link
Contributor

Don't forget that SeamlessAccess has a lot of experience with that IdP choosing process. Lessons to be learned there (and possibly code to use).

@timcappalli
Copy link
Member

timcappalli commented Apr 10, 2024

@yi-gu re: feature detection, we could consider something like getClientCapabilities() which we recently added to WebAuthn L3. This could be useful for both FedCM and the Digital Credentials API.

@yi-gu
Copy link
Collaborator

yi-gu commented Apr 10, 2024

Thanks @timcappalli for the suggestion! That idea was brought up late 2023 when it was un-merged. Will take a closer look and we can discuss about it in the CG.

@yi-gu
Copy link
Collaborator

yi-gu commented Apr 18, 2024

Plan to share some update from the Chrome team on this topic in the next CG call. In addition, we could discuss about Tim's suggestion on feature detection.

@hlflanagan
Copy link
Contributor

Question for @bvandersloot-mozilla - is the button mode as currently being tested in origin trials sufficient to resolve this issue?

@RByers
Copy link

RByers commented Jun 20, 2024

For context, here are some details on Chrome's FedCM button flow origin trial.

From group discussion, we think IDP login for multi-IDP is a separate (harder) problem, as it requires the browser to take on the "nascar flag" UI.

@bvandersloot-mozilla
Copy link
Collaborator Author

Yeah, the button flow is sufficient for our concern. We agree that multiple cold IDPs is a much harder problem, and may not be desirable for the browser to solve.

@yi-gu
Copy link
Collaborator

yi-gu commented Jul 3, 2024

We added some mocks for Android. They are not final and we will keep folks updated as we are getting closer to running Origin Trials in Chrome.

@wseltzer wseltzer added the FPWD label Jul 9, 2024
@yi-gu yi-gu added the agenda+ Regular CG meeting agenda items label Jul 11, 2024
@garykac
Copy link

garykac commented Jul 12, 2024

There's not a lot of information in the Explainer post about the proposed Use Other Account API/extension here.

Is it using an existing account chooser? If so, why is it called out as a new proposal?

If it is different from (or a modification to) an existing account chooser, then there needs to be more info in order to perform security and privacy reviews. How is it different and how does it ensure that available accounts and current logged-in status is not leaked?

@yi-gu
Copy link
Collaborator

yi-gu commented Jul 12, 2024

Hi garykac@,
From API's perspective, "Use Other Account" introduces a new way for IdPs to express that whether they allow using different account sessions and that's it. From privacy and security's perspective, the "log in to IdP" flow reuses the existing pattern from the LoginStatus API.
From UI's perspective, this feature may be presented differently to users across user agents. In Chrome's proposal, we add a "Use a different account" button to the account chooser (see mocks here). Upon user clicking that button, we trigger the same UI affordance for users to sign in to the IdP with the login_url from the LoginStatus API.

@garykac
Copy link

garykac commented Jul 12, 2024

OK, thank you for the clarification. I hoped that was the case, but ChromeStatus describes this as a proposed API, which makes it seem like a larger change than it actually is.

@yi-gu
Copy link
Collaborator

yi-gu commented Jul 12, 2024

Sorry for the confusion. It is indeed a new API like

// IdP's config file
{
  "accounts_endpoint" : ...,
  "modes: {
    "button": {
      "supports_use_other_account": true|false,
    },
    "widget": {
      "supports_use_other_account": true|false,
    }
  }
}

but as you mentioned it was not a big API change with new privacy/security implications.

@samuelgoto samuelgoto removed the agenda+ Regular CG meeting agenda items label Aug 2, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment