-
Notifications
You must be signed in to change notification settings - Fork 4
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
Would this work with the Multi-IdP API? The Mode API? The Continuation API? The IdP Registration API? #48
Comments
I'd like that. I think it shouldn't be a problem and is explicitly a target for e.g. the Mode API. How do you feel about the Registration API? Are these two APIs similar enough that there's a point in convergence / better integration? |
Yeah, that's a good question, but I do think that this aligns well with the IdP Registration API. I think you are right in that there is something awkward about them, but this seems like a resolvable issue (i.e. where to put the parameters of the // No prompts, tells the browser that the user is logged in
navigator.login.setStatus("logged-in");
// Doesn't prompt the user, caches the user's metadata
navigator.credentials.store({
identity: {
name: "John Doe",
email: "[email protected]"
// .. oof, is it awkward that we could store the accounts / token endpoint here or at the IdP Registration API?
}
});
// Prompts the user to register the origin as an IdP
IdentityProvider.register({
// IdP Registration typically gets a configURL as a parameter here, but for lightweight, maybe there won't be one?
// Do I specify them here? Or in the store?
// no id_assertion_endpoint, no accounts_endpoint, etc
}); |
Since lightweight FedCM does not use ID assertion endpoints, continuation API (and params API) does not really work with it as I understand it. |
I've been operating under the assumption during implementation that this should work alongside Multi-IDP and the Mode API since there's no reason not to. The registration API is interesting; it seems that the separate registration step shouldn't be necessary for a lightweight credential provider. Why would the user register an IdP they don't have an account with? Presumably if they are on a lightweight FedCM IdP page and opting to register the IdP, they've also had an account stored? I think the registration API, as I understand it, is still useful for "full" FedCM IdPs, but lightweight FedCM doesn't really benefit from it. |
IdP registration could still work, right? Instead of just storing the accounts, the IdP registers as being available for a certain 'type', and then the RP can request IdPs of a given type. |
n.c.store(IdentityCredential({...}) as defined in the explainer already supports a 'type' on the IdentityCredential, so it already covers this case without needing to invoke the registration API. |
That means that IdP registration is supported. That is literally what IdP registration API is. |
IdP Registration shows a user prompt though, right? Just wanted to make sure we're on the same page there. I agree that registration feels like something that could be frequently used with LW. I'm curious if we could make Continuation API work without the endpoints since it sounds potentially useful? I would need to read up more on Continuation API :) |
I'm going to open a separate issue specifically about whether the 'type' parameter should remain as part of the IdentityCredential passed to the n.c.store() call (in which case we will need to add a user prompt at store time) or if we should require using the registration API for that functionality, for consistency. (This came up in a discussion with @npm1) See: #49 |
It might be possible to call the store internally from the IDP registration API, if all of the arguments are there. I think it is worth considering putting the credential store operation behind a dialog regardless! |
I think this is the overall goal, but just filing it here to make sure this doesn't fall through the cracks, but would it be fair to work under the assumption that we would like this to work with the different extensions to FedCM?
The text was updated successfully, but these errors were encountered: