W3C home > Mailing lists > Public > public-webappsec@w3.org > June 2015

[credential-management] initial feedback

From: timeless <timeless@gmail.com>
Date: Tue, 23 Jun 2015 11:47:02 -0400
Message-ID: <CACsW8eEiRuEc1WWyv9rkWZ_Nf9mF1DaPLJpzA9w56+xWoo5zUA@mail.gmail.com>
To: "public-webappsec@w3.org" <public-webappsec@w3.org>
https://w3c.github.io/webappsec/specs/credentialmanagement/

> and define the persistance [sic] behavior necessary to store the new type.


> The user agent is in a unique position to improve the experience in a number of ways, and most modern user agents have recognized this by providing some measure of credential management natively in the browser.

`user agent`, `user agents`, `browser`. One of these is not like the
other. You can drop `in the browser`.

> Users can save usernames and passwords for websites, and those credentials are autofilled into sign-in forms with varying degrees of success.

autofilled, or filled when indicated by the user...

> These use cases are explored in more detail in §1.1 Use Cases and in Credential Management: Use Cases and Requirements; this specification attempts to address many of the requirements that document outlines by defining a Credential Manager API which a website can use to request credentials for a user, and to ask the user agent to persist credentials when a user signs in successfully.


1. Oh, grandma, what a very long sentence you have there. (All the
better to eat you with, my dear.)
2. `that document` is problematic since there are two previous things
(1.1 UC and CM) - I'd replace it with `the requirements document` or
something.


> Broadly speaking, credential is an assertion about an entity which enables a trust decision.

_a_ credential is /an/ ...

> iconURL, of type USVString, readonly

why USVString instead of DOMString? (the webidl spec says you should
have a good reason, if you have a good reason, you should include it
in the document.)

> The user agent MUST execute the algorithm described in `§4.2.4 Send passwordCredential to url. `when this method is executed.

This is a rare instance where you included a `.` in a section heading,
it makes the sentence unreadable, and you've also underlined the
trailing space, which is just wrong. Please undo both.

> PasswordCredential objects' options matching algorithm always returns Match.

pattern:
I think you could write `object's` (possibly inserting the word `The`
at the beginning of the sentence).

> If this value is null, then the protocol can be inferred from the provider.

would `should` work in place of `can`? Also, who is doing the inferring?

> FederatedCredential objects' options matching algorithm is as follows.

I expected a `:` at the end of this sentence, but there wasn't one.

> Given a FederatedCredential (credential) and a CredentialRequestOptions (options):

There is one here, which you could probably move to earlier if you
made this a heading instead of including it in the `paragraph`...

> Let federated be the value of options’s federated property.

pattern:
options'

It's really annoying to read `options's`, I believe I've convinced
anne to write `options'`, please do the same.

(Note that you write `settings'` later.)

> For each provider in federated' [sic] providers list:

pattern:
federated's

--- You can't just randomly throw a `'` at the end of a word and
expect it to be treated as possessive, that only applies for plural
words ending in `s`, and this word ends in `d`. Please add the `s`.

> When get() is called, the user agent MUST execute the algorithm defined in `§4.1.1 Request a Credential `on types and options.

pattern:
Please don't underline spaces after the last linked word (for that
matter, don't underline whitespace before a link...)

> For consistency, federations passed into the APIs defined in this document (e.g. FederatedCredentialRequestOptions's providers array, or FederatedCredential's provider property) MUST be identified by the ASCII serialization of the origin the provider uses for sign in.

What does this mean for an IDN IRI?

> Switch on type, and execute the associated steps:
> LocallyStoredCredential

One of the other specs has a nice arrow for switch case entries, could
you please use it?

> Given an origin (origin), and a sequence of DOMString (types), and an CredentialRequestOptions dictionary (options),

pattern:
I don't think `dictionary` here adds value.

> This requirement only applies if we would show UI to the user.

I'm pretty sure you're changing person. (This is first person plural,
specs are rarely written in this person.)

> Let credential be the Credential the user chose, or null if the user chose not to share a credential with origin.

For passwords and federations, this step should allow UAs to let the
user enter credentials/select a federation that they haven't yet
stored -- so that it appears as though it was stored to the requesting
site.


> If the user agent’s credential manager does not contain a FederatedCredential storedCredential whose id attribute is credential’s id and whose provider attribute is credential’s provider, then:

This `if` ... `and` sequence is too complicated.
I think you're trying to say that "if there's an exact match of
storedCredential credential-id provider-provider, do nothing",
otherwise ...

> If origin A’s scheme is origin B’s scheme, and origin A’s host's registerable domain is origin B’s host's registerable domain, then return Fuzzy Match.
> Return No Match

you're missing a period (`.`)

> For example, the user agent could display a "Save this password?" dialog box to the user in response to each call to store().

`dialog boxes` have thankfully gone out of style. door hangers are the
current thing, but it should be possible to use language that is more
agnostic to the ui implementation. Also, you should encourage the UA
to indicate the login-name or federation name.

> User consent MAY be requested every time a credential is stored or updated,

Here voice has reverted to third person passive (for lack of a better
description)

> the user agent MAY request a more persistent grant of consent which would apply to some or all subsequent API operations.

Here it's third person active.

pattern:
Avoid passive voice. (i.e. favor The user agent may request user
consent) -- alternatively, minimally: Avoid mixing passive and active
voices, if only within a sentence, but preferably avoid mixing it
within a paragraph, and ideally, please avoid mixing it within the
document.

This section doesn't provide enough warning about sites abusing the feature.

I fully expect "trusted sites" (i.e. banks) to helpfully "update" my
password to an invalid value to ensure that I'm not saving my
password.

Text should be included to encourage browsers to detect when sites
misbehave as such and to encourage them to suggest blocking read/write
access to the store when they seem to be replacing the user's value.

Example: If a user logs in as login:"joe" password:"6p@ck", and the
site updates it to login:"joe" password:"invalid" and then the user
updates it again a few days later to login:"joe" password:"6p@ck", and
the site again changes it to login:"joe" password:"blanked", UAs
should be encouraged to detect this, and inform the user that the site
seems to be fighting w/ the user to update the password and encourage
the user to block the site from performing writes.

Similarly, UAs should be encouraged to indicate to a user when a read
has happened with an option to mark "this site is annoying me by
chastising me about using a password manager" which would result in
"disable read access to this api" and "would you like to publicly
shame {stupid bank} for its stupid behavior?"

> If a [sic] an origin’s requires user mediation flag is set to false in the user agent’s credential store,

> The chooser interface SHOULD include an indication of the origin which is requesting credentials.

what's the mediation for a
really-long-origin-that-is-hard-for-users-to-distinguish? and what
about IDN?

> (via FederatedCredentialRequestOptions's providers property),

drop the `s`
Received on Tuesday, 23 June 2015 15:47:30 UTC

This archive was generated by hypermail 2.3.1 : Monday, 23 October 2017 14:54:13 UTC