Open vabr-g opened 8 years ago
To be more specific about the proposed change to the spec:
@mikewest , some questions:
Am I reinventing the wheel with the template definition?
Let me repeat the proposal back to you to see if I understand it: I think that what you're suggesting is that we change the additionalData
attribute to accept a flat string with some predefined placeholders for the username and password fields. Something like:
var credential = await navigator.credentials.get({ "password": true });
if (credential) {
credential.additionalData = "{ name: '${USERNAME}', password: '${PASSWORD}', csrf: 'abcdefg' }";
fetch("/endpoint", { credentials: credential });
}
I'm not sure whether that's more or less elegant than the approach I was considering, which would copy an object into additionalData
, populate fields corresponding to idName
and passwordName
, and then serialize it on the wire. That is:
var credential = await navigator.credentials.get({ "password": true });
if (credential) {
credential.additionalData = { csrf: 'abcdefg' };
credential.idName = "name";
credential.passwordName = "password";
fetch("/endpoint", { credentials: credential });
}
I have the feeling that it's going to be more complicated to construct a string than to construct an object (though I suppose they devolve to ~same thing if you build an object and then use JSON.stringify()
). Building the string has the advantage of not being tied to JSON, which isn't nothing. And it can support more deeply nested structures than what I was thinking. Perhaps we could ask developers whether they like the mechanism, or whether the flexibility is too expensive in terms of construction overhead.
How would you support defining the content-type? We currently use the extraction mechanism to define that based on the data type we pass in, but it's not clear what we'd want to define for a string. Limiting things to JSON means we could pick application/json
and be done with it.
Also: What do you do when the password/username contains characters that mean something for the encoding into which they're being inserted? Quotes and newlines need to be escaped in JSON, for instance, but not in binary formats like CBOR or protobuf. Those formats generally need to encode the string lengths, though. How flexible do you want the replacements to be?
Also also: You'll notice that the JSON blob I posted above isn't actually JSON, because it's using single-quotes rather than double-quotes (because I started the string with double-quotes and didn't want to go back and fix it, or deal with escaping in the string). Developers are probably going to be just as lazy as I; if we can lean on the system to do the right thing for us (via standardized mechanisms like JSON.stringify()
), I think we have a better chance of producing valid output.
Also also also: Do we need to care about character encodings? I'll honestly need to check to see whether or not FormData
carries the accept-charset
along with it or not; if it does, it's another behavior we'd need to somehow replicate (as I don't think you'd have a good time trying to convince JavaScript to treat a string as a non-UTF-8 encoding.
Thanks for sharing your thoughts, Mike!
I understand your first question as asking whether we should let the developer do the serialisation as they want and afterwards let the user agent put the passwords in (=what I suggested with the USVString), or whether we let them specify the payload in a structured object, and let the user agent first insert the password and then serialise (=what happens in both the current FormData approach of the spec, and also what you described here).
I acknowledge the issues with the verbatim password value possibly breaking the developer-provided encoding. Not sure if I understand the deep nesting issue -- would it be sufficient to just force the user agent to do a complete (nested) search of the passed object for the idName and passwordName properties ? The bigger issue with not letting the developer to serialise the request is losing the flexibility of the format.
The reason to do USVString is to allow the developer to avoid server-side changes, just keeping the request in the way the server used to receive them. This, however, ties our hands with some safe encoding of the password value. We cannot just let it break the request message, and we cannot encode it properly if we don't know what the server expects. So to me this looks like a dead end.
So perhaps let's go with JSON, and I can loop @agektmr to help me find out if other common formats are used?
As for character encoding, for FormData the spec forces utf-8 in §3.3.2. We could keep this for serialising into JSON as well.
I heard from @agektmr in an e-mail, that so far the only custom format requested by web developers was JSON.
So the proposal would be adding the possibility for additionalData
to be an arbitrary object, and then serialising all objects which are not FormData by (1) replacing idName
and passwordName
with the credential values, and (2) calling JSON.stringify()
on the result.
Additionally, we can consider whether it makes sense to look for idName
and passwordName
arbitrarily deep in the data object.
Any thoughts before I start drafting the spec change for this?
So the proposal would be adding the possibility for
additionalData
to be an arbitrary object, and then serialising all objects which are notFormData
Or URLSearchParams
.
by (1) replacing
idName
andpasswordName
with the credential values
As noted above, I don't even think replacement is necessary. I'd just set a property on the object with a name of the credential object's idName
, and a value of the credential's id
(and likewise for the password). Overwrite the property if it exists on the object, add the property if not.
Note that you'll need to be careful to copy the object, and to ensure that the setter for the property named passwordName
doesn't have any side-effects. Now that I'm thinking about it, it might be simpler to simply require that the object not have a property named idName
or passwordName
(which I suppose would mean throwing on assignment of idName
, passwordName
, or additionalData
).
and (2) calling
JSON.stringify()
on the result.
And returning application/json
as the content type.
Additionally, we can consider whether it makes sense to look for idName and passwordName arbitrarily deep in the data object.
As noted above, this would add some complexity (in terms of ensuring that the property setters aren't attacker-created traps), and it's not clear how we'd specify this (we'd need to deal with cyclical references, for instance (var a = { yay: { password: null } }; a.boo = a;
). That's certainly doable (JSON.stringify
does it by holding a stack of objects (see https://tc39.github.io/ecma262/#sec-serializejsonobject)), but seems like more work than it's worth doing.
Currently, to post the request with credential for authentication, the site needs to initialise the request with the opaque PasswordCredential object. The goal is to avoid leaking the credential to the potentially compromised JavaScript on the site.
Some frameworks might require a specific format of the request, e.g., JSON. That cannot currently be achieved, because the CM API implementation formats the body as multipart/form-data.
To allow more flexibility, while preserving the leak protection, could we make it an option to create the request with a JSON template with a placeholder, which the CM API implementation would replace with the serialisation of the credential upon sending the request off?