Aligning Lightweight FedCM with IdP Registration re: 'type'

[ekovac]

As currently written, the explainer describes a new parameter to the IdentityCredential constructor called type, which would serve the same function as the type parameter on the proposed IdentityProvider.register() method.

One issue this raises is that it means that the store() call now needs to prompt the user to prevent malicious or simply ill-behaved IdPs from drive-by registering themselves for a broad type class of RPs for the user.

In the general case when 'type' is NOT supplied during construction of the stored IdentityCredential object, the n.c.store() call shouldn't need user interaction; there's nothing to be gained by a malicious IdP here since reading it back by an RP requires a prompt.

I think the natural choice here is to remove 'type' from the IdentityCredential itself, and if we want IdP Registration type behavior we rely on IdentityProvider.register() .

This makes the behavior more consistent with full FedCM, and eliminates the need to introduce a user prompt for n.c.store().

[ekovac]

If there are no objections I can put together a PR for this to update the explainer.

@bvandersloot-mozilla what are your thoughts?

[bvandersloot-mozilla]

I think the better solution would be to move the store to an internal call by the registration API, assuming the information we need to create a credential is available there.

[ekovac]

So the .store() call would be as-is currently defined, but an IdP registration would happen under the hood?

[bvandersloot-mozilla]

I meant the other way, but I think this might be better. I'm not certain

[samuelgoto]

As currently written, the explainer describes a new parameter to the IdentityCredential constructor called type, which would serve the same function as the type parameter on the proposed IdentityProvider.register() method.

Conceptually speaking, I think that IdentityProvider.register() and the navigator.credentials.store() to be fairly orthogonal and independent concepts: the former allows users to "register identity providers" whereas the latter allows "identity providers to register accounts".

As a concrete example, I think it should be possible for an IdP to be (a) registered and, at the same time, (b) have their user logged-out without any account available. All of the other 4 combinations are also valid: unregistered but logged-in, unregistered and logged-out as well as registered and logged-in.

One issue this raises is that it means that the store() call now needs to prompt the user to prevent malicious or simply ill-behaved IdPs from drive-by registering themselves for a broad type class of RPs for the user.

Yeah, I think that's correct: you need to prompt the user for permission, because otherwise any website could spam the list without any user awareness, as you said, just driving by.

I think the natural choice here is to remove 'type' from the IdentityCredential itself, and if we want IdP Registration type behavior we rely on IdentityProvider.register() .
This makes the behavior more consistent with full FedCM, and eliminates the need to introduce a user prompt for n.c.store().

I think that overall makes sense.

Just to be concrete if I understand this correctly, but roughly speaking, here is what this would look like for an IdP:

// Tells the browser that the user is logged in
// TODO: should this be bundled / inferred with the next call?
navigator.login.setStatus("logged-in");
navigator.credentials.store(new IdentityCredential({
  name: "John Doe",
  email: "foobar@gmail.com"
}));
IdentityProvider.register( /** to we start accepting undefined for lightweight? */ );

Did I get this more or less right?

So the .store() call would be as-is currently defined, but an IdP registration would happen under the hood?

Is the suggestion here to bundle the last two (and maybe the first one too) into the semantics of the store()? For example:

// This is semantically isomorphic syntactic sugar to the code snippet above
navigator.credentials.store(new IdentityCredential({
  type: "indie-auth",
  name: "John Doe",
  email: "foobar@gmail.com"
}));

Did I get this right?

[cbiesinger]

One issue this raises is that it means that the store() call now needs to prompt the user to prevent malicious or simply ill-behaved IdPs from drive-by registering themselves for a broad type class of RPs for the user.

Yeah, I think that's correct: you need to prompt the user for permission, because otherwise any website could spam the list without any user awareness, as you said, just driving by.

Do you think user permission is also needed for store() if the type is moved to a separate register() call? That is, do you think storing just the logged in user data also needs permission?

[ekovac]

Just to be concrete if I understand this correctly, but roughly speaking, here is what this would look like for an IdP:

// Tells the browser that the user is logged in
// TODO: should this be bundled / inferred with the next call?
navigator.login.setStatus("logged-in");
navigator.credentials.store(new IdentityCredential({
  name: "John Doe",
  email: "foobar@gmail.com"
}));
IdentityProvider.register( /** to we start accepting undefined for lightweight? */ );

This is what I had in mind, exactly. And yes, IdentityProvider.register() for a lightweight IdP could just take undefined, we've got the origin from context. I think it'd be harmless to make the login status call implied from the .store() call.

[samuelgoto]

This is what I had in mind, exactly. And yes, IdentityProvider.register() for a lightweight IdP could just take undefined, we've got the origin from context. I think it'd be harmless to make the login status call implied from the .store() call.

Ah, glad that I understood what you had in mind. I just realized that I forgot your actual original point in this thread, using the type from the IdentityProvider.register(), so here is a more complete snippet:

// navigator.login.setStatus() is implied in the `store()`
navigator.credentials.store(new IdentityCredential({
  name: "John Doe",
  email: "foobar@gmail.com"
}));
IdentityProvider.register({
  type: "inde-auth"
});

Did I get this right?

[samuelgoto]

On a related note:

I think it'd be harmless to make the login status call implied from the .store() call.

This reminds me to re-notify you that I think that what you actually want is the reverse: drop the navigator.credentials.store() and extend the navigator.login.setStatus() instead.

I think that what you actually want, is the following:

navigator.login.setStatus("logged-in", {
  accounts: [{
    name: "John Doe",
    email: "foobar@gmail.com"
  }]
}));
IdentityProvider.register({
  type: "inde-auth"
});

For example, what's the reverse of navigator.credentials.store()? Conceptually speaking, I think you want the following to be the reverse of navigator.credentials.store():

navigator.login.setStatus("logged-out");

I think I mentioned this a few times, and it is fine to leave this discussion for later, but as we go along and learn more about the relationship between the parts, it helps to re-test these ideas.

[ekovac]

On a related note:

I think it'd be harmless to make the login status call implied from the .store() call.

This reminds me to re-notify you that I think that what you actually want is the reverse: drop the navigator.credentials.store() and extend the navigator.login.setStatus() instead.

I think that what you actually want, is the following:

navigator.login.setStatus("logged-in", {
  accounts: [{
    name: "John Doe",
    email: "foobar@gmail.com"
  }]
}));
IdentityProvider.register({
  type: "inde-auth"
});

For example, what's the reverse of navigator.credentials.store()? Conceptually speaking, I think you want the following to be the reverse of navigator.credentials.store():

navigator.login.setStatus("logged-out");

I think I mentioned this a few times, and it is fine to leave this discussion for later, but as we go along and learn more about the relationship between the parts, it helps to re-test these ideas.

I'm warming up to this idea, in part because it gets rid of the awkward detail of there being an "input" IdentityCredential and an "output" IdentityCredential that behave distinctly but are the same class in WebIDL. @bvandersloot-mozilla does this make sense to you as well?

[bvandersloot-mozilla]

in part because it gets rid of the awkward detail of there being an "input" IdentityCredential and an "output" IdentityCredential that behave distinctly but are the same class in WebIDL

we could drop the create and constructor and only allow n.c.store() to solve that problem.

IMO if it is all the same it makes more sense to have one function call that does two things and use the state management that already exists in CredMan, rather than the IDP calling two functions to accomplish one task.

[samuelgoto]

we could drop the create and constructor and only allow n.c.store() to solve that problem.

Perhaps, one way to think about this problem, is to ask ourselves: is a "stored" credential still "valid" if the user is logged out? If not, what's the operation that "deletes" the "store"?