Closed Bug 1407829 Opened 6 years ago Closed 6 years ago

Web Authentication - Implement CredMan's Store method

Categories

(Core :: DOM: Device Interfaces, enhancement, P2)

Product:
Core
Component:
DOM: Device Interfaces
Type:
enhancement

Tracking

()

Status:
RESOLVED FIXED
Milestone:
mozilla58
Tracking Flags:
Tracking Status
firefox58 --- fixed

People

(Reporter: jcj, Assigned: jcj)

References

(
URL
)

Details

(Whiteboard: [webauthn][webauthn-wd07])

Attachments

(1 file)

Bug 1407829 - WebAuthn: Implement CredMan's Store method
59 bytes, text/x-review-board-request
qdot
: review+
ttaubert
: review+
Details 
Credential Management defines a Store operation, which isn't quite in WebAuthn's WD-07 yet but will be shortly.

We need to implement that -- it just returns an error immediately.
Assignee: nobody → jjones
Status: NEW → ASSIGNED
Comment on attachment 8918062 [details]
Bug 1407829 - WebAuthn: Implement CredMan's Store method

https://reviewboard.mozilla.org/r/188958/#review194416

::: dom/webauthn/WebAuthnManager.cpp:694
(Diff revision 1)
>  
> +already_AddRefed<Promise>
> +WebAuthnManager::Store(nsPIDOMWindowInner* aParent,
> +                       const Credential& aCredential)
> +{
> +  MOZ_ASSERT(aParent);

nit: `MOZ_ASSERT(NS_IsMainThread())`

::: dom/webauthn/WebAuthnManager.cpp:702
(Diff revision 1)
> +
> +  nsCOMPtr<nsIGlobalObject> global = do_QueryInterface(aParent);
> +
> +  ErrorResult rv;
> +  RefPtr<Promise> promise = Promise::Create(global, rv);
> +  if(rv.Failed()) {

nit: add a space after if

::: dom/webauthn/tests/test_webauthn_store_credential.html:37
(Diff revision 1)
> +      isnot(navigator.credentials, undefined, "Credential Management API endpoint must exist");
> +      isnot(navigator.credentials.create, undefined, "CredentialManagement create API endpoint must exist");
> +      isnot(navigator.credentials.get, undefined, "CredentialManagement get API endpoint must exist");
> +      isnot(navigator.credentials.store, undefined, "CredentialManagement store API endpoint must exist");
> +
> +      let gCredentialChallenge = new Uint8Array(16);

nit: The variable name makes it look like that's a global.
Attachment #8918062 - Flags: review?(ttaubert) → review+
Comment on attachment 8918062 [details]
Bug 1407829 - WebAuthn: Implement CredMan's Store method

https://reviewboard.mozilla.org/r/188958/#review194416

Thanks, Tim. Updated.
Comment on attachment 8918062 [details]
Bug 1407829 - WebAuthn: Implement CredMan's Store method

https://reviewboard.mozilla.org/r/188958/#review195140

::: dom/webauthn/WebAuthnManager.h:94
(Diff revision 2)
>    GetAssertion(nsPIDOMWindowInner* aParent,
>                 const PublicKeyCredentialRequestOptions& aOptions);
>  
> +  already_AddRefed<Promise>
> +  Store(nsPIDOMWindowInner* aParent,
> +               const Credential& aCredential);

nit: identation
Attachment #8918062 - Flags: review?(kyle) → review+
Comment on attachment 8918062 [details]
Bug 1407829 - WebAuthn: Implement CredMan's Store method

https://reviewboard.mozilla.org/r/188958/#review195140

Thanks for the reviews!
Keywords: checkin-needed
Target Milestone: Future → mozilla58
Pushed by ryanvm@gmail.com:
https://hg.mozilla.org/integration/autoland/rev/8ebe3f571ab8
WebAuthn: Implement CredMan's Store method r=qdot,ttaubert
Keywords: checkin-needed
https://hg.mozilla.org/mozilla-central/rev/8ebe3f571ab8
Status: ASSIGNED → RESOLVED
Closed: 6 years ago
status-firefox58: --- → fixed
Resolution: --- → FIXED
Pushed by ttaubert@mozilla.com:
https://hg.mozilla.org/integration/mozilla-inbound/rev/d99dbb659ecc
Fix merge bustage in WebAuthnManager.h on a CLOSED TREE r=me
You need to log in before you can comment on or make changes to this bug.