w3c / webappsec-credential-management

WebAppSec Credential Management
https://w3c.github.io/webappsec-credential-management/
Other
49 stars 37 forks source link

storeCredentials() method on HTMLFormElement #13

Closed mnoorenberghe closed 8 years ago

mnoorenberghe commented 8 years ago

One of my concerns with this API is that it may encourage behaviour which isn't backwards-compatible with UAs who don't implement it even though there are compatible ways to do the same thing. The spec doesn't even provide any examples of using .store() with a <form> so authors are going to end up not using a form submit event with preventDefault() when they want to use this API to store credentials meaning existing UAs will need to use heuristics to detect the "submission".

One of the benefits that this spec provides is an imperative API to store a credential. I think such an API should (instead) be on <form> to encourage best practices since <form> has benefits for accessibility and UX. When this method (e.g. storeCredentials()) is called, the UA would store credential-related fields (determined based on the autocomplete attribute, no heuristics) similar to 4.1.2. Store a Credential. Ideally this would be the only way to store compatible credentials to ensure this backwards-compatible API is used. As I mentioned before, I think this could work for federated credentials with rAc too.

If wanted, additional restrictions can be placed on this new method e.g. requiring that rAc was successful on it and/or that the <form> submit event has occurred.

This combined with #2 would remove the need for the LocalCredential interface.

Example:

<script>
document.forms[0].addEventListener("submit", function(evt) {
  evt.preventDefault();
  var formData = new FormData(evt.target);
  fetch("https://example.com/loginEndpoint", { method: "POST", body: formData })
    .then(function (response) {
      if (evt.target.storeCredentials) {
        evt.target.storeCredentials();
      }
      // Notify the user that signin succeeded! Do amazing, signed-in things!
    });
});
</script>
<form>
  <input name="un" autocomplete="username">
  <input name="pw" autocomplete="current-password">
  <input type="submit" value="Login">
</form>
mnoorenberghe commented 8 years ago

An alternative would be to have a store method somewhere else but allow an HTMLFormElement as an argument (instead of a credential).

mikewest commented 8 years ago

Thank you for spelling this out!

I like the idea of basing the creation of credentials on a <form>. That seems like a nice way of encouraging backwards compatibility, and if we go the navigator.credentials route I think we could actually accomplish the core of what you're suggesting here by adding a new constructor to PasswordCredential that accepts an HTMLFormElement (and, I suppose, a Content-Type enum?)^1 (it's basically the same code):

theForm.addEventListener("submit", e => {
    e.preventDefault();
    var c = new PasswordCredential(e.target);
    var init = { method: "POST", credentials: c };
    fetch("https://example.com/endpoint", init).then(r => {
      if (/* `r` is a "successful" response */ && navigator.credentials)
        navigator.credentials.store(c);
    });
});

I'm a little wary of putting something like storeCredentials on HTMLFormElement, as it doesn't seem like the right layering for the feature. I think of the form itself like a pretty generic data container. It makes sense to me to separate the container from the specific ways in which the contained data might be used by the UA. If we want to treat the data contained in the form as a credential, it makes sense to me to be explicit about that by parsing it into an object that has distinct meaning, and to do something explicit with that object.

I'm also a little skeptical that this <form>-based model makes sense for anything other than username/password pairs. That is, if I decide to sign in to SpeakerDeck using GitHub, I click a button (which might be part of a form, but isn't in this case) which navigates me through ~3 redirects before sending me back to Speakerdeck with a token, or giving me a sign-in form on GitHub. In either event, by the time I'm actually signed into SpeakerDeck, there's no form available to use as a storage vector. In that case, it seems significantly simpler to allow the developer to synthesize a credential rather than forcing them to construct a form with various attributes and trigger a submit that will be cancelled in order to get into a state that allows storeCredentials. Registering a FIDO credential seems even less conducive to this model, given the interaction between the site and the device.

[^1]: LinkedIn is a good example of a site that, for whatever reason, has an ancient auth system that doesn't understand multipart POSTs. To support sites like that, we need to give fetch() something it will serialize as application/x-www-form-urlencoded.

/ccing interested folks: @annevk @AxelNennker @bifurcation @martinthomson @wanderview

AxelNennker commented 8 years ago

The current store method allows the site to call it only after a successful login.

This is not achieved by this proposal and not by Mike's amendment because I feel that intercepting "submit" to POST the form using fetch is a hack and not an API. I don't see a way to add "store"-functionality to HTMLFormElement without mangling the semantic of form. The form's -URL would be unused. The site could just return a page that stores a new credential, right?

<script>
var credential = new PasswordCredential({
  "id": "JaneDoe98",
  "password": "MfPeRQq5P3yVry68Q4KZMMhB"
});
navigator.credentials.store(credential);
</script>
<p>Welcome Jane</p><br/>You have successfully logged-in!

Which exposes the password to two pages. Bummer. I don't see a simple way to attach store to Form without the hack.

Axel

mikewest commented 8 years ago

The current store method allows the site to call it only after a successful login.

I don't think there's any special handling in the spec for when store() can be called. We require that it be user-mediated so that the user is always involved, but I don't think there's a requirement we could write in the spec that would only allow it to be called if a user really signed in. That would bring us right back to the status quo problem with heuristics that the API is attempting to mitigate.

I don't see a way to add "store"-functionality to HTMLFormElement without mangling the semantic of form.

I think "mangling" is strong, but I agree that store doesn't seem well-suited to the HTMLFormElement interface.

I feel that intercepting "submit" to POST the form using fetch is a hack and not an API.

I don't think either variant is using submit as the sole trigger. That is, both attempt to verify that the sign-in actually happened by looking at the Response object and doing whatever application-specific logic seems relevant. It seems reasonable to assume, however, that the site is going to be storing a credential after some sort of user interaction with a form (since we couldn't hand over a credential, the user is likely typing something in manually and clicking on a button, right?). Hooking that form's submit event to trigger an async sign-in seems like a good idea.

I agree with you that it would be terrible if the site wrote the password into the DOM after a navigation in order to support storage; this is one reason that the whole thing is slanted towards asynchronous authentication (the other, of course, being a better user experience).

mnoorenberghe commented 8 years ago

I like the idea of basing the creation of credentials on a

. That seems like a nice way of encouraging backwards compatibility, and if we go the navigator.credentials route I think we could actually accomplish the core of what you're suggesting here by adding a new constructor to PasswordCredential that accepts an HTMLFormElement (and, I suppose, a Content-Type enum?)^1 (it's basically the same code):

OK but I would prefer to not expose LocalCredential/PasswordCredential constructors to ensure a backwards-compatible implementation with a <form>.

I'm a little wary of putting something like storeCredentials on HTMLFormElement, as it doesn't seem like the right layering for the feature. I think of the form itself like a pretty generic data container. It makes sense to me to separate the container from the specific ways in which the contained data might be used by the UA.

Yeah, pretend I never suggested a method on the <form>. I realized my mistake after commenting and then replied about putting it somewhere else.

If we want to treat the data contained in the form as a credential, it makes sense to me to be explicit about that by parsing it into an object that has distinct meaning, and to do something explicit with that object.

I think saying "store the credentials in this form" (e.g. navigator.credentials.store(form)) doesn't seem bad.

I'm also a little skeptical that this -based model makes sense for anything other than username/password pairs.

Let's start with focusing on local/password credentials then. The specs idea of federated credentials aren't even "credentials" IMO as they aren't enough information to grant a user access to anything and as others mentioned the incremental value of storing a login origin + username in a new API over localStorage/cookies is small and not really interesting.

That is, if I decide to sign in to SpeakerDeck using GitHub, I click a button (which might be part of a form, but isn't in this case) which navigates me through ~3 redirects before sending me back to Speakerdeck with a token, or giving me a sign-in form on GitHub. In either event, by the time I'm actually signed into SpeakerDeck, there's no form available to use as a storage vector. In that case, it seems significantly simpler to allow the developer to synthesize a credential rather than forcing them to construct a form with various attributes and trigger a submit that will be cancelled in order to get into a state that allows storeCredentials.

We could allow calling store() with a HTMLFormElement without submitting if that works from a security perspective (I haven't thought through all of the cases). The last step of the ~3 redirects could also be a hidden form which submits the federated credential information.

Registering a FIDO credential seems even less conducive to this model, given the interaction between the site and the device.

Would you be fine with only allowing store for local/password credentials via an HTMLFormElement then? Leave FIDO/Federated alone for now since, as has been argued in other threads, they are less interesting to Mozilla.

[^1]: LinkedIn is a good example of a site that, for whatever reason, has an ancient auth system that doesn't understand multipart POSTs. To support sites like that, we need to give fetch() something it will serialize as application/x-www-form-urlencoded.

<form> has an enctype attribute that could/would have this information accessible.

mnoorenberghe commented 8 years ago

The current store method allows the site to call it only after a successful login.

Where does it enforce that? It seems like I can run the snippet of code like you have below at any point, not necessarily on success. I think perhaps you didn't mean to include "only"?

This is not achieved by this proposal and not by Mike's amendment because I feel that intercepting "submit" to POST the form using fetch is a hack and not an API.

It's not a hack. Existing sites that are using <form> (aka. doing the right thing) already for password credentials wouldn't use any new credential management API to store credentials as there's no benefit. UAs can already handle this fine.

preventDefault is the correct way to prevent the default form submission and do a fetch if that's what a site already needs to do. This isn't a hack, it's what preventDefault on a <form> is for.

I don't see a way to add "store"-functionality to HTMLFormElement without mangling the semantic of form. The form's -URL would be unused.

Firefox may still use it for the formSubmitURL field. This doesn't seem like a blocker though.

The site could just return a page that stores a new credential, right?

No site would need to do this though since the UA should have already captured the login.

AxelNennker commented 8 years ago

Oops: "The current store method allows the site to call it only after a successful login." is clearly wrong. What I wanted to say is that the current CM spec allows the site to call store after a successful login. Thus getting rid of heuristics in the browser's login manager. The site is in control.

Hooking on the submit handler still feels wrong to me. Maybe the store-after-success scheme is valuable for forms in general too. Consider a typo in a credit card number so that the Luhn-algorithm fails. The form data handler would store the wrong cc-number because the browser does not know about this. Maybe navigator.autocomplete.store(form) is the right way to think about this?!

Sorry about "mangling". Both issues are due to me not being a native speaker.

mnoorenberghe commented 8 years ago

Oops: "The current store method allows the site to call it only after a successful login." is clearly wrong. What I wanted to say is that the current CM spec allows the site to call store after a successful login. Thus getting rid of heuristics in the browser's login manager. The site is in control.

The Firefox team (more recently defined) view on the password manager is that the user is who should be in control so we don't want to add cases where the site can somehow opt-out of having a credential stored that a user "used". If a site used my proposed rAc flow, we wouldn't need to activate capture based on any heuristics on that page (e.g. via pushState, XHR, etc.) as we would know the user just filled username and/or password fields and we could potentially be less aggressive about password save/change UI (perhaps only showing the prompt anchor icon [with the panel dismissed by default]) if we spec that sites need to manually store the form data after preventDefault which follows rAc.

Hooking on the submit handler still feels wrong to me. Maybe the store-after-success scheme is valuable for forms in general too. Consider a typo in a credit card number so that the Luhn-algorithm fails. The form data handler would store the wrong cc-number because the browser does not know about this. Maybe navigator.autocomplete.store(form) is the right way to think about this?!

Sure though many types of data validation can be done client-side with the HTML form validation API already (e.g. setCustomValidity) and browsers probably shouldn't save if the field is in the invalid state. I also wondered about storing of other values but wish there was an existing object it made sense on. FWIW I still prefer this over non-form ways of storing local/password credentials.

Sorry about "mangling". Both issues are due to me not being a native speaker.

No worries.

mikewest commented 8 years ago

Before diving into the line-by-line: you're giving me an inch with navigator.credentials.store(), I'd suggest that having that method would make it both reasonable and expected to have navigator.credentials.get(). Why use a completely different API to retrieve the stored information?

OK but I would prefer to not expose LocalCredential/PasswordCredential constructors to ensure a backwards-compatible implementation with a <form>.

I sympathize with the desire to ensure that folks use forms and autocomplete attributes correctly, but forcing them to put metadata (like "friendly name" or "icon") into the form itself seems limiting.

If we want to treat the data contained in the form as a credential, it makes sense to me to be explicit about that by parsing it into an object that has distinct meaning, and to do something explicit with that object.

I think saying "store the credentials in this form" (e.g. navigator.credentials.store(form)) doesn't seem bad.

I don't think it's bad. I think it's less explicit about what it means. I'm thinking in particular of sign-up forms that encompass things beyond usernames and passwords. Even really trivial sign-up forms like the one on GitHub's homepage (https://github.com/) have extra data (email address, in this case). What does "store" do with that information? I guess we'd implicitly run it through the PasswordCredential constructor, but I'd suggest that being explicit about that lossy conversion would be a good thing.

I'm also a little skeptical that this -based model makes sense for anything other than username/password pairs.

Let's start with focusing on local/password credentials then. The specs idea of federated credentials aren't even "credentials" IMO as they aren't enough information to grant a user access to anything and as others mentioned the incremental value of storing a login origin + username in a new API over localStorage/cookies is small and not really interesting.

I think I completely disagree with you. :) Today. the browser can't help at all with federated sign-in. Eventually, the browser should totally start grabbing tokens on a user's behalf. That shouldn't stop us from taking the baby step of helping users remember which federation they chose. I personally have created literally three StackOverflow accounts because I'm a big dummy. That site does a good job letting me join the accounts together to recover; most sites don't, nor will they ever be bothered to do so.

We could allow calling store() with a HTMLFormElement without submitting if that works from a security perspective (I haven't thought through all of the cases). The last step of the ~3 redirects could also be a hidden form which submits the federated credential information.

It could be lots of things. It isn't. I think it's unlikely that folks are going to reinvent their OAuth implementations to push data into a form, both because it's a bad user experience (yet another request), and because they generally don't want to touch and risk breaking auth code. I want to provide a mechanism that can be layered on top of an existing system without fundamental changes to the way it works.

Registering a FIDO credential seems even less conducive to this model, given the interaction between the site and the device.

Would you be fine with only allowing store for local/password credentials via an HTMLFormElement then? Leave FIDO/Federated alone for now since, as has been argued in other threads, they are less interesting to Mozilla.

It would surprise me a lot if FIDO wasn't interesting to Mozilla. I mean, if nothing else, @bifurcation is the chair of the Working Group and Mozilla explicitly supported its charter. I also think it would be fairly short-sighted to avoid FIDO and federations just because they don't fit into the same mold as passwords. They're different ways of authenticating. It seems fine to invent a generic (but new) way of dealing with them.

<form> has an enctype attribute that could/would have this information accessible.

So far as I can tell, that isn't exposed to Fetch at all. I would love to have a simpler mechanism to choose an encoding and serialize a body for an asynchronous request... for the moment, URLSearchParams and FormData seem to be all that's available. +@annevk

(aka. doing the right thing) already for password credentials wouldn't use any new credential management API to store credentials as there's no benefit. UAs can already handle this fine.

Part of the value here lies in a difference in philosophy between UA's credential managers. Chrome attempts to ensure that a sign-in was successful before popping up a prompt to store a credential. We have a number of heuristics to that end that are getting better at dealing with async login and other weirdnesses, but we've explicitly chosen not to fire on submit. For Chrome, it's super helpful to have an explicit signal to augment our heuristics. I imagine that might be the case for other UAs as well.

annevk commented 8 years ago

@mikewest we could maybe support new URLSearchParams(form). Not sure. (Agreed that with regards to FIDO Mozilla is interested. Have not heard otherwise. And personally I think we should care about improving the status quo of federation credentials too. They are widely used, after all.)

mikewest commented 8 years ago

I've added a first pass at the new PasswordCredential(HTMLFormElement form) constructor to https://w3c.github.io/webappsec-credential-management/#passwordcredential-form-constructor. WDYT?

AxelNennker commented 8 years ago

Regarding https://w3c.github.io/webappsec-credential-management/#passwordcredential-form-constructor

In the example I would only add the event-handler if "navigator.credentials" is defined by moving the "if"-clause before the addEventListener

mikewest commented 8 years ago

Why is the tree-order needed?

The ordering is important as we have to have a sane answer for what to do if more than one element matches the autocomplete attribute we're looking for: we go with the last one in tree order, which is the same thing that FormData does.

I would reorder the steps so that they algorithms fails fast. That is: move step 4 before the loop

Step 4 can only be checked after the loop. The idea is that we walk through all the elements in the form in order, and set the values in the PasswordCredentialData dictionary based on the elements that we find. After we've walked through all the elements, we can examine the dictionary to see if we found any data. If we didn't find an id or a password, we can throw, but we can't know that until we've looked through the data.

In the example I would only add the event-handler if "navigator.credentials" is defined by moving the "if"-clause before the addEventListener

Maybe? I mean, a more complete implementation would actually capture the submit event and send credentials asynchronously regardless, because that's generally a better user experience than forcing a full-page navigation.

AxelNennker commented 8 years ago

Some sites don't want users to store credentials but browser vendors think that the user should be in control and used the LoginManager even when the site said not to do it. See: The autocomplete attribute and login fields Now with this proposal what should the browser do when the site never calls navigator.credentials.store() ?

mikewest commented 8 years ago

Fall back on heuristics, doing whatever it does today? store() shouldn't be the only mechanism for getting data into the UA's credential manager.

AxelNennker commented 8 years ago

What happened to PasswordCredential's additionalData field? Should the constructor PasswordCredential(form) fill it?

mikewest commented 8 years ago

Great point! I forgot to add those pieces in. Addressed in https://github.com/w3c/webappsec-credential-management/commit/90f9cff809c8e1874e8b33921336942c20847ca2

AxelNennker commented 8 years ago

If "the Web" is moving away from full-page navigation because of the better UX then maybe HTMLForm should get a "then"/onResponse attribute? <form onResponse="r => {if (/* |r| is a "successful" Response */) navigator.credentials.store(c);}"/>

/* slightly off-topic */

mikewest commented 8 years ago

/* slightly off-topic */

Why don't we debate the merits of that proposal once we're not arguing about this API. :)

mikewest commented 8 years ago

Ping?

mnoorenberghe commented 8 years ago

Apologies for the delay. I'm quite busy at least until the end of the month.

Before diving into the line-by-line: you're giving me an inch with navigator.credentials.store(), I'd suggest that having that method would make it both reasonable and expected to have navigator.credentials.get(). Why use a completely different API to retrieve the stored information?

I don't think there should be an API to retrieve local credentials at all. The only value I see in this spec for local credentials is a way to tell the browser to store what's in a form and to provide a more secure way to fill in existing forms which can get submitted as they currently do.

OK but I would prefer to not expose LocalCredential/PasswordCredential constructors to ensure a backwards-compatible implementation with a <form>.

I sympathize with the desire to ensure that folks use forms and autocomplete attributes correctly, but forcing them to put metadata (like "friendly name" or "icon") into the form itself seems limiting.

If you're suggesting that only sites that use this JS API will get friendly names and icons then this just adds to my argument that this API will encourage sites to not use a <form> if they want those features. Note that there is already autocomplete values of photo, name and nickname.

    If we want to treat the data contained in the form as a credential, it makes sense to me to be explicit about that by parsing it into an object that has distinct meaning, and to do something explicit with that object.

I think saying "store the credentials in this form" (e.g. navigator.credentials.store(form)) doesn't seem bad.

I don't think it's bad. I think it's less explicit about what it means. I'm thinking in particular of sign-up forms that encompass things beyond usernames and passwords. Even really trivial sign-up forms like the one on GitHub's homepage (https://github.com/) have extra data (email address, in this case). What does "store" do with that information? I guess we'd implicitly run it through the PasswordCredential constructor, but I'd suggest that being explicit about that lossy conversion would be a good thing.

It's a credential API so it only stores credential-related data. You can say the same thing about login mangers already though I realize it's not an explicit API. Why don't they store the email address associated with my account?

    I'm also a little skeptical that this -based model makes sense for anything other than username/password pairs.

Let's start with focusing on local/password credentials then. The specs idea of federated credentials aren't even "credentials" IMO as they aren't enough information to grant a user access to anything and as others mentioned the incremental value of storing a login origin + username in a new API over localStorage/cookies is small and not really interesting.

I think I completely disagree with you. :) Today. the browser can't help at all with federated sign-in. Eventually, the browser should totally start grabbing tokens on a user's behalf. That shouldn't stop us from taking the baby step of helping users remember which federation they chose. I personally have created literally three StackOverflow accounts because I'm a big dummy. That site does a good job letting me join the accounts together to recover; most sites don't, nor will they ever be bothered to do so.

You don't even agree with 'The specs idea of federated credentials aren't even "credentials" IMO as they aren't enough information to grant a user access to anything'?

We could allow calling store() with a HTMLFormElement without submitting if that works from a security perspective (I haven't thought through all of the cases). The last step of the ~3 redirects could also be a hidden form which submits the federated credential information.

It could be lots of things. It isn't. I think it's unlikely that folks are going to reinvent their OAuth implementations to push data into a form, both because it's a bad user experience (yet another request), and because they generally don't want to touch and risk breaking auth code. I want to provide a mechanism that can be layered on top of an existing system without fundamental changes to the way it works.

    Registering a FIDO credential seems even less conducive to this model, given the interaction between the site and the device.

Would you be fine with only allowing store for local/password credentials via an HTMLFormElement then? Leave FIDO/Federated alone for now since, as has been argued in other threads, they are less interesting to Mozilla.

It would surprise me a lot if FIDO wasn't interesting to Mozilla. I mean, if nothing else, @bifurcation is the chair of the Working Group and Mozilla explicitly supported its charter. I also think it would be fairly short-sighted to avoid FIDO and federations just because they don't fit into the same mold as passwords. They're different ways of authenticating. It seems fine to invent a generic (but new) way of dealing with them.

I was talking about this spec's potential interaction with FIDO. FIDO is already doing its own thing in its spec and doesn't need this spec/API AFAICT.

(aka. doing the right thing) already for password credentials wouldn't use any new credential management API to store credentials as there's no benefit. UAs can already handle this fine.

Part of the value here lies in a difference in philosophy between UA's credential managers. Chrome attempts to ensure that a sign-in was successful before popping up a prompt to store a credential. We have a number of heuristics to that end that are getting better at dealing with async login and other weirdnesses, but we've explicitly chosen not to fire on submit. For Chrome, it's super helpful to have an explicit signal to augment our heuristics. I imagine that might be the case for other UAs as well.

I think we agree that an API to store local credentials from a <form> is useful for UAs.

mikewest commented 8 years ago

Apologies for the delay. I'm quite busy at least until the end of the month.

Thanks for spending some time on a response. I understand that you're busy, and I appreciate your feedback.

I don't think there should be an API to retrieve local credentials at all.

For clarity, the API allows the retrieval of an opaque Credentials object, which is submitted via fetch(). We're not handing passwords to JavaScript.

The only value I see in this spec for local credentials is a way to tell the browser to store what's in a form ... I think we agree that an API to store local credentials from a <form> is useful for UAs.

Good. We're not on completely different pages, then, and I think the proposal for how to do this (new constructor on PasswordCredential) is pretty reasonable. Thanks for the suggestion!

forcing them to put metadata (like "friendly name" or "icon") into the form itself seems limiting.

If you're suggesting that only sites that use this JS API will get friendly names and icons then this just

With the changes you've suggested in https://w3c.github.io/webappsec-credential-management/#passwordcredential-form-constructor, we can provide this for sites just using forms. My concern is with "forcing" that to be the only mechanism, as I think you'd agree that most sites don't include either in sign-in forms.

You don't even agree with 'The specs idea of federated credentials aren't even "credentials" IMO as they aren't enough information to grant a user access to anything'?

I don't. It gives the site enough information to confirm that the user is or is not User X. It does so remotely via an OAuth flow as opposed to a local database lookup, but that seems immaterial. We can of course improve that flow in the future by doing the confirmation on the site's behalf, but making it possible at all is both valuable and sufficiently credential-like that I don't think a distinction is necessary or helpful.

But assume I did agree with you that this is a partial credential or something along those lines. It still would have the same value, and still wouldn't be supported in today's world. There's plenty of value in doing so, so let's do it. :)

I was talking about this spec's potential interaction with FIDO. FIDO is already doing its own thing in its spec and doesn't need this spec/API AFAICT.

If you're agreeing that FIDO needs an API, it's not clear to me why it wouldn't/couldn't be this one?

mnoorenberghe commented 8 years ago

Apologies for the delay. I'm quite busy at least until the end of the month.

Thanks for spending some time on a response. I understand that you're busy, and I appreciate your feedback.

I don't think there should be an API to retrieve local credentials at all.

For clarity, the API allows the retrieval of an opaque Credentials object, which is submitted via fetch(). We're not handing passwords to JavaScript.

Understood, but I don't think it's necessary when we have the autocomplete attributes on fields.

The only value I see in this spec for local credentials is a way to tell the browser to store what's in a form
...
I think we agree that an API to store local credentials from a <form> is useful for UAs.

Good. We're not on completely different pages, then, and I think the proposal for how to do this (new constructor on PasswordCredential) is pretty reasonable. Thanks for the suggestion!

Sure, but I don't support having other means to do this (e.g. the other constructor) which aren't backwards compatible.

    forcing them to put metadata (like "friendly name" or "icon") into the form itself seems limiting.

If you're suggesting that only sites that use this JS API will get friendly names and icons then this just

With the changes you've suggested in https://w3c.github.io/webappsec-credential-management/#passwordcredential-form-constructor, we can provide this for sites just using forms. My concern is with "forcing" that to be the only mechanism, as I think you'd agree that most sites don't include either in sign-in forms.

Well they're going to need to have the photo URL in their login page to use your other constructor anyways so why not have it in a hidden input then? They can't use your other local credential constructor from pages that don't have the plaintext password.

You don't even agree with 'The specs idea of federated credentials aren't even "credentials" IMO as they aren't enough information to grant a user access to anything'?

I don't. It gives the site enough information to confirm that the user is or is not User X. It does so remotely via an OAuth flow as opposed to a local database lookup, but that seems immaterial. We can of course improve that flow in the future by doing the confirmation on the site's behalf, but making it possible at all is both valuable and sufficiently credential-like that I don't think a distinction is necessary or helpful.

You're talking about abilities outside of the spec where the UA could keep an OAuth token for an account, right?

I did say "enough information" and you're saying they're enough if you add OAuth…

But assume I did agree with you that this is a partial credential or something along those lines. It still would have the same value, and still wouldn't be supported in today's world. There's plenty of value in doing so, so let's do it. :)

Sure, it's a matter of whether this is the right API for it or not and therefore where the place to do it is. I still think requestAutocomplete is a better place.

I was talking about this spec's potential interaction with FIDO. FIDO is already doing its own thing in its spec and doesn't need this spec/API AFAICT.

If you're agreeing that FIDO needs an API, it's not clear to me why it wouldn't/couldn't be this one?

My understanding is that FIDO doesn't need storage in the browser, it's more of a pass-through API to authenticators so is different enough than local credentials (similar to my comment on Federated ones)

Regardless I'm still not convinced that requestAutocomplete isn't a better API/UX for this (#2).

mikewest commented 8 years ago

Sure, but I don't support having other means to do this (e.g. the other constructor) which aren't backwards compatible. ... Well they're going to need to have the photo URL in their login page to use your other constructor anyways so why not have it in a hidden input then? They can't use your other local credential constructor from pages that don't have the plaintext password.

I was imagining scenarios in which data like photos come in as a result of signing in. That is:

var c = new PasswordCredential(theForm);
fetch("https://endpoint", { credentials: c, method: "POST" })
    .then(r => r.json())
    .then(j => {
      c.iconURL = j.avatarURL;
      c.name = j.friendlyName;
      navigator.credentials.store(c);
    });

But you're right that that scenario wouldn't require a broken-up constructor as long as we have setters on the object. I'll go back to the developers we're talking with to see how they're using the method. shrug Maybe you're right that we can get rid of it.

You're talking about abilities outside of the spec where the UA could keep an OAuth token for an account, right?

I'm talking about The Beautiful And Bright Future(tm) when we all have time to add things like native OAuth support to the browser, either holding a refresh token itself on a site's behalf, or allowing a site to pass in a refresh token that the browser could go out and redeem for it.

This is baby step 1. Something like that would be a reasonable step 2.

I did say "enough information" and you're saying they're enough if you add OAuth…

Sure. Usernames and passwords are only "enough" if you add a database call to verify. I don't see a difference in kind between checking an internal database and checking an external HTTP endpoint. Either way, you're using the credential object to authenticate a user.

Regardless I'm still not convinced that requestAutocomplete isn't a better API/UX for this (#2).

That does seem to be the point of contention at the moment.

From my perspective, again:

I think the imperative API is the right way to go. You don't. Shall we flip a coin? :)

AxelNennker commented 8 years ago

I think that there should be a 'get' and support for rAc through the new constructor as it is in the current version of the spec. Coming back to the topic of this issue: I think there shouldn't be a theForm.storeCredential.

@mnoorenberghe Are you requesting that 'get' be removed from this API?

Axel

Also: sorry for the delay (business travel, school holidays, family visits until end of the month)

mikewest commented 8 years ago

As another argument that we haven't explored yet, some folks pointed out to me that this model of the API is much more in keeping with the extensible web tenent that "browser vendors should provide new low-level capabilities that expose the possibilities of the underlying platform as closely as possible". Building out objects with store and get methods feels like it gets us closer to the idea of exposing primitives than a high-level declarative mechanism would. I see that as another good reason to prefer this structure.

mnoorenberghe commented 8 years ago

But you're right that that scenario wouldn't require a broken-up constructor as long as we have setters on the object. I'll go back to the developers we're talking with to see how they're using the method. shrug Maybe you're right that we can get rid of it.

Removing support for the PasswordCredential constructor taking a PasswordCredentialData would make be feel better about this API as it wouldn't encourage usage of non-form login experiences (keeping it backwards compatible) or the server putting the plaintext password back into the login POST page just to call the constructor (which would be bad for security).

We can add it in a later version if the carrot of the API ends up not being enough for adoption.

mnoorenberghe commented 8 years ago

I'd be okay with .get for PasswordCredentials if there was an API to fill the credentials into a <form> (ideally where the autocomplete=current-password has @writeonly).

https://w3c.github.io/webappsec-credential-management/#examples-password-signin is currently clunky for sites that already use <form>. The description says "A website that supports only username/password pairs can request credentials, and use them in existing sign-in forms" yet the credential doesn't get used in an "existing sign-in form".

mnoorenberghe commented 8 years ago

I think we can mark this as fixed by 0880aaf (and follow-ups) and move other topics to separate issues for easier tracking.