Open Bug 1156047 (credman) Opened 9 years ago Updated 2 months ago

Implement the W3C WebAppSec Credential Management API

Categories

(Core :: DOM: Credential Management, enhancement, P3)

enhancement

Tracking

()

People

(Reporter: ignisvulpis, Unassigned)

References

(Depends on 2 open bugs, )

Details

(Keywords: dev-doc-needed, Whiteboard: [domsecurity-backlog])

User Agent: Mozilla/5.0 (X11; Ubuntu; Linux x86_64; rv:37.0) Gecko/20100101 Firefox/37.0
Build ID: 20150403141925

Steps to reproduce:

Implement the W3C WebAppSec Credential Management API
http://w3c.github.io/webappsec/specs/credentialmanagement/

Extend navigator to have navigator.credentials


Actual results:

new api


Expected results:

have navigator.credentials

Initial webidl and implementation is here:
https://github.com/AxelNennker/firefox_credentials/
Status: UNCONFIRMED → NEW
Component: Untriaged → DOM: Security
Ever confirmed: true
Product: Firefox → Core
Note there are a number of open issues with the spec so be prepared to be flexible (or leave the areas with open issues for last):
https://github.com/w3c/webappsec/issues?q=is%3Aissue+CREDENTIAL%3A+is%3Aopen
New push to https://github.com/AxelNennker/firefox_credentials which is a step forward but ...
Next steps: 
- implement GetParentObject (if it is really needed - if yes what is the parent: contentdocument?)
- reintroduce LocallyStoredCredential
- what about NoInterfaceObject on Credential while being a parameter of store?
  http://w3c.github.io/webappsec/specs/credentialmanagement/#interfaces-credential-manager
- set origin on object creation
- UI
- LoginManager?

Help and advise (Push Requests) welcome.

-axel
I absolutely think that we should implement this. I'm sure there are problems to solve and I'm happy to help with that, but need someone that's prepared to step up and implement on at least Fennec and Desktop.
What's needed here https://github.com/AxelNennker/firefox_credentials/ is user consent UI 
Wrapper cache stuff is in the works but I am on vacation now.
Feel free to implement whatever you want next.
Or discuss here if you would like to implement it differently 
Axel
FWIW, I'm not a fan of the LocallyStoredCredential aspect of the spec since it duplicates functionality provided by requestAutocomplete with autocomplete="(username/new-password/current-password)" and didn't seem to be adding value over them last time I looked as the spec (last year). We already have all of the foundation for requestAutocomplete implemented in https://mxr.mozilla.org/mozilla-central/source/toolkit/components/formautofill/

The federated login portions seemed much more interesting as it's something the browser currently doesn't help the user with.
Matthew, could you please file an issue here: https://github.com/w3c/webappsec/issues/new That seems like good feedback to give. Federated login is unfortunately not complete enough to be interesting I think. When I talked with someone from GitHub it sounded like it wasn't really meeting their needs.
(In reply to Anne (:annevk) from comment #6)
> Matthew, could you please file an issue here:
> https://github.com/w3c/webappsec/issues/new That seems like good feedback to
> give.

Done: https://github.com/w3c/webappsec/issues/445
Severity: normal → enhancement
OS: Linux → All
Hardware: x86_64 → All
The current editor's draft moved away from having a method "send" on PasswordCredential but use Fetch instead as described here:
https://w3c.github.io/webappsec-credential-management/#monkey-patching-fetch

That would need (I think) a change to Fetch as described here:
https://w3c.github.io/webappsec-credential-management/#monkey-patching-fetch-4

I am not a member of the W3C's webappsec WG and found no discussion on the public mailing list on this nor in the github issues.
https://lists.w3.org/Archives/Public/public-webappsec/
https://github.com/w3c/webappsec-credential-management/issues

Would you consider implementing this in Fetch? 

Axel

P.s.:

Credentialmanagement is still WIP and I am not sure I like the suggested change:
3.3.4. RequestInit dictionary
Change the RequestInit dictionary’s body member to be of the union type (BodyInit or Credential).
Note: The RequestInit parameter to Request is the only piece of the Fetch infrastructure that accepts a Credential. In particular, neither Response nor XMLHttpRequest's send() method accept Credential objects.

Not everybody needs Credential.type nor Credential.id.
The main feature of this change is to get same-origin protection through Fetch if the body is a Credential and a protection of the password leaking to content javascript and from there to other origins. I would prefer to have a OriginBoundSharedSecret type with fields "secretName" and [[secretValue]]
Flags: needinfo?(dveditz)
Flags: needinfo?(bkelly)
Anne, should this be integrated with the fetch spec better before implementing?  I'm not sure I like the current proposal.

In particular, I'm unsure about adding the opaque flag to Request.  That seems a bit odd to me:

1) Its not observable by script so they have no idea they can't call body consuming methods.
2) For Response with have a wrapping opaque response type, but for Request we would have an opaque flag.  This difference seems inconsistent and a bit confusing.
3) This feels like another attempt to add opaque streams as previously discussed.

Also, I find it quite unexpected that any Request containing a Credentials body ends up bypassing the service worker.

I think I would rather see a system where we can mark the Request to automatically get the credential attached as its body.  This attachment would happen at the same time as other cookie credentials.
Flags: needinfo?(bkelly) → needinfo?(annevk)
Any issues with this specification should really be filed at https://github.com/w3c/webappsec-credential-management/issues/new so Mike can have a look.

1) Script did end up passing the PasswordCredential object, but it might be reasonable to expose this somehow.
2) I'm not sure the same concept works here, but maybe?
3) It's definitely similar to opaque streams I suppose.

They could maybe end up in the service worker I think if we keep the opaque flag. Not sure why the service worker is bypassed.

I suppose your alternative system could work with this being a parameter to fetch() only. That could be an interesting approach, although in that case you'd have to bypass the service worker or keep the opaque flag thingie.
Flags: needinfo?(annevk)
(In reply to Anne (:annevk) from comment #10)
> 1) Script did end up passing the PasswordCredential object, but it might be
> reasonable to expose this somehow.

Someone handed a Request, like library code, has no way to inspect if they can access the body without triggering an exception with the current spec design.

> I suppose your alternative system could work with this being a parameter to
> fetch() only. That could be an interesting approach, although in that case
> you'd have to bypass the service worker or keep the opaque flag thingie.

The idea is its not attached yet, so there is no need to throw on body access.  Its basically just like trying to inspect cookie credentials, they simply don't show up.
I'll file my comments on the github repo.
I have no answers for Axel. webappsec github issues or public-webappsec mail threads are probably the best way to handle it.
Flags: needinfo?(dveditz)
Hi! Glad to see interest in the spec!

@Axel: The conversation that lead to the current design happened mostly on IRC: http://logs.glob.uno/?c=freenode%23whatwg&s=10+Nov+2015&e=12+Nov+2015#c973976. Apologies if that was exclusionary, that wasn't the intent.

@Ben: I've responded to your suggestions https://github.com/w3c/webappsec-credential-management/issues/11. I think we can work something out. I'd appreciate doing so quickly, as we're planning to ship a first pass at the API in Chrome ~51 (https://groups.google.com/a/chromium.org/forum/#!msg/blink-dev/7ouLjWzcjb0/E7YWD1KrDQAJ was the intent to ship, with a few Mozilla folks CC'd). We totally have time to make changes, but we're talking to developers who would like to start using this API, and I'd like to reduce the amount of churn they go through in their experimental deployments.
Whiteboard: [domsecurity-backlog]
Blocks: html5test
Alias: credman
Depends on: 1384331
Priority: -- → P3
Depends on: 1429196
Severity: normal → S3
Component: DOM: Security → DOM: Credential Management
You need to log in before you can comment on or make changes to this bug.