Skip to content

Commit 43288c7

Browse files
author
Tim Taubert
committed
Bug 1407093 - Web Authentication - WD-07 updates for user handles r=jcj,smaug
Reviewers: jcj, smaug Reviewed By: jcj, smaug Bug #: 1407093 Differential Revision: https://phabricator.services.mozilla.com/D328
1 parent 71f4a21 commit 43288c7

8 files changed

+115
-38
lines changed

‎dom/webauthn/AuthenticatorAssertionResponse.cpp‎

Lines changed: 13 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -15,13 +15,15 @@ NS_IMPL_CYCLE_COLLECTION_UNLINK_BEGIN_INHERITED(AuthenticatorAssertionResponse,
1515
AuthenticatorResponse)
1616
tmp->mAuthenticatorDataCachedObj = nullptr;
1717
tmp->mSignatureCachedObj = nullptr;
18+
tmp->mUserHandleCachedObj = nullptr;
1819
NS_IMPL_CYCLE_COLLECTION_UNLINK_END
1920

2021
NS_IMPL_CYCLE_COLLECTION_TRACE_BEGIN_INHERITED(AuthenticatorAssertionResponse,
2122
AuthenticatorResponse)
2223
NS_IMPL_CYCLE_COLLECTION_TRACE_PRESERVED_WRAPPER
2324
NS_IMPL_CYCLE_COLLECTION_TRACE_JS_MEMBER_CALLBACK(mAuthenticatorDataCachedObj)
2425
NS_IMPL_CYCLE_COLLECTION_TRACE_JS_MEMBER_CALLBACK(mSignatureCachedObj)
26+
NS_IMPL_CYCLE_COLLECTION_TRACE_JS_MEMBER_CALLBACK(mUserHandleCachedObj)
2527
NS_IMPL_CYCLE_COLLECTION_TRACE_END
2628

2729
NS_IMPL_CYCLE_COLLECTION_TRAVERSE_BEGIN_INHERITED(AuthenticatorAssertionResponse,
@@ -38,6 +40,7 @@ AuthenticatorAssertionResponse::AuthenticatorAssertionResponse(nsPIDOMWindowInne
3840
: AuthenticatorResponse(aParent)
3941
, mAuthenticatorDataCachedObj(nullptr)
4042
, mSignatureCachedObj(nullptr)
43+
, mUserHandleCachedObj(nullptr)
4144
{
4245
mozilla::HoldJSObjects(this);
4346
}
@@ -93,17 +96,21 @@ AuthenticatorAssertionResponse::SetSignature(CryptoBuffer& aBuffer)
9396
}
9497

9598
void
96-
AuthenticatorAssertionResponse::GetUserId(DOMString& aRetVal)
99+
AuthenticatorAssertionResponse::GetUserHandle(JSContext* aCx,
100+
JS::MutableHandle<JSObject*> aRetVal)
97101
{
98-
// This requires mUserId to not be re-set for the life of the caller's in-var.
99-
aRetVal.SetOwnedString(mUserId);
102+
if (!mUserHandleCachedObj) {
103+
mUserHandleCachedObj = mUserHandle.ToArrayBuffer(aCx);
104+
}
105+
aRetVal.set(mUserHandleCachedObj);
100106
}
101107

102108
nsresult
103-
AuthenticatorAssertionResponse::SetUserId(const nsAString& aUserId)
109+
AuthenticatorAssertionResponse::SetUserHandle(CryptoBuffer& aBuffer)
104110
{
105-
MOZ_ASSERT(mUserId.IsEmpty(), "We already have a UserID?");
106-
mUserId.Assign(aUserId);
111+
if (NS_WARN_IF(!mUserHandle.Assign(aBuffer))) {
112+
return NS_ERROR_OUT_OF_MEMORY;
113+
}
107114
return NS_OK;
108115
}
109116

‎dom/webauthn/AuthenticatorAssertionResponse.h‎

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -48,17 +48,18 @@ class AuthenticatorAssertionResponse final : public AuthenticatorResponse
4848
SetSignature(CryptoBuffer& aBuffer);
4949

5050
void
51-
GetUserId(DOMString& aRetVal);
51+
GetUserHandle(JSContext* aCx, JS::MutableHandle<JSObject*> aRetVal);
5252

5353
nsresult
54-
SetUserId(const nsAString& aUserId);
54+
SetUserHandle(CryptoBuffer& aUserHandle);
5555

5656
private:
5757
CryptoBuffer mAuthenticatorData;
5858
JS::Heap<JSObject*> mAuthenticatorDataCachedObj;
5959
CryptoBuffer mSignature;
6060
JS::Heap<JSObject*> mSignatureCachedObj;
61-
nsString mUserId;
61+
CryptoBuffer mUserHandle;
62+
JS::Heap<JSObject*> mUserHandleCachedObj;
6263
};
6364

6465
} // namespace dom

‎dom/webauthn/WebAuthnManager.cpp‎

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -225,12 +225,12 @@ WebAuthnManager::MakeCredential(const MakePublicKeyCredentialOptions& aOptions,
225225
return promise.forget();
226226
}
227227

228-
// Enforce 4.4.3 User Account Parameters for Credential Generation
229-
if (aOptions.mUser.mId.WasPassed()) {
230-
// When we add UX, we'll want to do more with this value, but for now
231-
// we just have to verify its correctness.
228+
// Enforce 5.4.3 User Account Parameters for Credential Generation
229+
// When we add UX, we'll want to do more with this value, but for now
230+
// we just have to verify its correctness.
231+
{
232232
CryptoBuffer userId;
233-
userId.Assign(aOptions.mUser.mId.Value());
233+
userId.Assign(aOptions.mUser.mId);
234234
if (userId.Length() > 64) {
235235
promise->MaybeReject(NS_ERROR_DOM_TYPE_ERR);
236236
return promise.forget();

‎dom/webauthn/tests/test_webauthn_loopback.html‎

Lines changed: 7 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -107,8 +107,12 @@ <h1>Full-run test for MakeCredential/GetAssertion for W3C Web Authentication</h1
107107
is(aAssertion.id, bytesToBase64UrlSafe(new Uint8Array(aAssertion.rawId)), "Encoded Key ID and Raw Key ID match");
108108

109109
ok(aAssertion.response.authenticatorData === aAssertion.response.authenticatorData, "AuthenticatorAssertionResponse.AuthenticatorData is SameObject");
110+
ok(aAssertion.response.authenticatorData instanceof ArrayBuffer, "AuthenticatorAssertionResponse.AuthenticatorData is an ArrayBuffer");
110111
ok(aAssertion.response.signature === aAssertion.response.signature, "AuthenticatorAssertionResponse.Signature is SameObject");
111-
isnot(aAssertion.response.userId, undefined, "AuthenticatorAssertionResponse.UserId is defined")
112+
ok(aAssertion.response.signature instanceof ArrayBuffer, "AuthenticatorAssertionResponse.Signature is an ArrayBuffer");
113+
ok(aAssertion.response.userHandle === aAssertion.response.userHandle, "AuthenticatorAssertionResponse.UserHandle is SameObject");
114+
ok(aAssertion.response.userHandle instanceof ArrayBuffer, "AuthenticatorAssertionResponse.UserHandle is an ArrayBuffer");
115+
is(aAssertion.response.userHandle.byteLength, 0, "AuthenticatorAssertionResponse.UserHandle is emtpy");
112116

113117
ok(aAssertion.response.authenticatorData.byteLength > 0, "Authenticator data exists");
114118
let clientData = JSON.parse(buffer2string(aAssertion.response.clientDataJSON));
@@ -137,7 +141,7 @@ <h1>Full-run test for MakeCredential/GetAssertion for W3C Web Authentication</h1
137141

138142
function testMakeCredential() {
139143
let rp = {id: document.domain, name: "none", icon: "none"};
140-
let user = {name: "none", icon: "none", displayName: "none"};
144+
let user = {id: new Uint8Array(), name: "none", icon: "none", displayName: "none"};
141145
let param = {type: "public-key", alg: cose_alg_ECDSA_w_SHA256};
142146
let makeCredentialOptions = {
143147
rp: rp,
@@ -156,7 +160,7 @@ <h1>Full-run test for MakeCredential/GetAssertion for W3C Web Authentication</h1
156160

157161
function testMakeDuplicate(aCredInfo) {
158162
let rp = {id: document.domain, name: "none", icon: "none"};
159-
let user = {name: "none", icon: "none", displayName: "none"};
163+
let user = {id: new Uint8Array(), name: "none", icon: "none", displayName: "none"};
160164
let param = {type: "public-key", alg: cose_alg_ECDSA_w_SHA256};
161165
let makeCredentialOptions = {
162166
rp: rp,

‎dom/webauthn/tests/test_webauthn_make_credential.html‎

Lines changed: 55 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -87,6 +87,61 @@ <h1>Test for MakeCredential for W3C Web Authentication</h1>
8787
.catch(expectTypeError);
8888
},
8989

90+
// Test without rp.name
91+
function() {
92+
let rp = {id: document.domain, icon: "none"};
93+
let makeCredentialOptions = {
94+
rp: rp, user: user, challenge: gCredentialChallenge, pubKeyCredParams: [param]
95+
};
96+
return credm.create({publicKey: makeCredentialOptions})
97+
.then(arrivingHereIsBad)
98+
.catch(expectTypeError);
99+
},
100+
101+
// Test without user.id
102+
function() {
103+
let user = {name: "none", icon: "none", displayName: "none"};
104+
let makeCredentialOptions = {
105+
rp: rp, user: user, challenge: gCredentialChallenge, pubKeyCredParams: [param]
106+
};
107+
return credm.create({publicKey: makeCredentialOptions})
108+
.then(arrivingHereIsBad)
109+
.catch(expectTypeError);
110+
},
111+
112+
// Test without user.name
113+
function() {
114+
let user = {id: new Uint8Array(64), icon: "none", displayName: "none"};
115+
let makeCredentialOptions = {
116+
rp: rp, user: user, challenge: gCredentialChallenge, pubKeyCredParams: [param]
117+
};
118+
return credm.create({publicKey: makeCredentialOptions})
119+
.then(arrivingHereIsBad)
120+
.catch(expectTypeError);
121+
},
122+
123+
// Test without user.displayName
124+
function() {
125+
let user = {id: new Uint8Array(64), name: "none", icon: "none"};
126+
let makeCredentialOptions = {
127+
rp: rp, user: user, challenge: gCredentialChallenge, pubKeyCredParams: [param]
128+
};
129+
return credm.create({publicKey: makeCredentialOptions})
130+
.then(arrivingHereIsBad)
131+
.catch(expectTypeError);
132+
},
133+
134+
// Test with a user handle that exceeds the max length
135+
function() {
136+
let user = {id: new Uint8Array(65), name: "none", icon: "none", displayName: "none"};
137+
let makeCredentialOptions = {
138+
rp: rp, user: user, challenge: gCredentialChallenge, pubKeyCredParams: [param]
139+
};
140+
return credm.create({publicKey: makeCredentialOptions})
141+
.then(arrivingHereIsBad)
142+
.catch(expectTypeError);
143+
},
144+
90145
// Test without any parameters; this is acceptable meaning the RP ID is
91146
// happy to take any credential type
92147
function() {

‎dom/webauthn/tests/test_webauthn_no_token.html‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -44,7 +44,7 @@ <h1>Test for W3C Web Authentication with no token</h1>
4444

4545
function testMakeCredential() {
4646
let rp = {id: document.domain, name: "none", icon: "none"};
47-
let user = {name: "none", icon: "none", displayName: "none"};
47+
let user = {id: new Uint8Array(), name: "none", icon: "none", displayName: "none"};
4848
let param = {type: "public-key", alg: cose_alg_ECDSA_w_SHA256};
4949
let makeCredentialOptions = {
5050
rp: rp, user: user, challenge: credentialChallenge, pubKeyCredParams: [param]

‎dom/webauthn/tests/test_webauthn_sameorigin.html‎

Lines changed: 22 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -25,17 +25,18 @@ <h1>Test Same Origin Policy for W3C Web Authentication</h1>
2525

2626
function arrivingHereIsGood(aResult) {
2727
ok(true, "Good result! Received a: " + aResult);
28-
return Promise.resolve();
2928
}
3029

3130
function arrivingHereIsBad(aResult) {
3231
ok(false, "Bad result! Received a: " + aResult);
33-
return Promise.resolve();
3432
}
3533

3634
function expectSecurityError(aResult) {
3735
ok(aResult.toString().startsWith("SecurityError"), "Expecting a SecurityError");
38-
return Promise.resolve();
36+
}
37+
38+
function expectTypeError(aResult) {
39+
ok(aResult.toString().startsWith("TypeError"), "Expecting a TypeError");
3940
}
4041

4142
function keepThisPublicKeyCredential(aIdentifier) {
@@ -66,7 +67,7 @@ <h1>Test Same Origin Policy for W3C Web Authentication</h1>
6667
var testFuncs = [
6768
function() {
6869
// Test basic good call
69-
let rp = {id: document.domain};
70+
let rp = {id: document.domain, name: "none"};
7071
let makeCredentialOptions = {
7172
rp: rp, user: user, challenge: chall, pubKeyCredParams: [param]
7273
};
@@ -78,15 +79,24 @@ <h1>Test Same Origin Policy for W3C Web Authentication</h1>
7879
function() {
7980
// Test rp.id being unset
8081
let makeCredentialOptions = {
81-
rp: {}, user: user, challenge: chall, pubKeyCredParams: [param]
82+
rp: {name: "none"}, user: user, challenge: chall, pubKeyCredParams: [param]
8283
};
8384
return credm.create({publicKey: makeCredentialOptions})
8485
.then(arrivingHereIsGood)
8586
.catch(arrivingHereIsBad);
8687
},
88+
function() {
89+
// Test rp.name being unset
90+
let makeCredentialOptions = {
91+
rp: {id: document.domain}, user: user, challenge: chall, pubKeyCredParams: [param]
92+
};
93+
return credm.create({publicKey: makeCredentialOptions})
94+
.then(arrivingHereIsBad)
95+
.catch(expectTypeError);
96+
},
8797
function() {
8898
// Test this origin with optional fields
89-
let rp = {id: "user:pass@" + document.domain + ":8888"};
99+
let rp = {id: "user:pass@" + document.domain + ":8888", name: "none"};
90100
let makeCredentialOptions = {
91101
rp: rp, user: user, challenge: chall, pubKeyCredParams: [param]
92102
};
@@ -96,7 +106,7 @@ <h1>Test Same Origin Policy for W3C Web Authentication</h1>
96106
},
97107
function() {
98108
// Test blank rp.id
99-
let rp = {id: ""};
109+
let rp = {id: "", name: "none"};
100110
let makeCredentialOptions = {
101111
rp: rp, user: user, challenge: chall, pubKeyCredParams: [param]
102112
};
@@ -106,7 +116,7 @@ <h1>Test Same Origin Policy for W3C Web Authentication</h1>
106116
},
107117
function() {
108118
// Test subdomain of this origin
109-
let rp = {id: "subdomain." + document.domain};
119+
let rp = {id: "subdomain." + document.domain, name: "none"};
110120
let makeCredentialOptions = {
111121
rp: rp, user: user, challenge: chall, pubKeyCredParams: [param]
112122
};
@@ -116,7 +126,7 @@ <h1>Test Same Origin Policy for W3C Web Authentication</h1>
116126
},
117127
function() {
118128
// Test the same origin
119-
let rp = {id: "example.com"};
129+
let rp = {id: "example.com", name: "none"};
120130
let makeCredentialOptions = {
121131
rp: rp, user: user, challenge: chall, pubKeyCredParams: [param]
122132
};
@@ -126,7 +136,7 @@ <h1>Test Same Origin Policy for W3C Web Authentication</h1>
126136
},
127137
function() {
128138
// Test the eTLD
129-
let rp = {id: "com"};
139+
let rp = {id: "com", name: "none"};
130140
let makeCredentialOptions = {
131141
rp: rp, user: user, challenge: chall, pubKeyCredParams: [param]
132142
};
@@ -136,7 +146,7 @@ <h1>Test Same Origin Policy for W3C Web Authentication</h1>
136146
},
137147
function () {
138148
// Test a different domain within the same TLD
139-
let rp = {id: "alt.test"};
149+
let rp = {id: "alt.test", name: "none"};
140150
let makeCredentialOptions = {
141151
rp: rp, user: user, challenge: chall, pubKeyCredParams: [param]
142152
};
@@ -233,7 +243,7 @@ <h1>Test Same Origin Policy for W3C Web Authentication</h1>
233243
},
234244
function () {
235245
// Test basic good Create call but using an origin (Bug 1380421)
236-
let rp = {id: window.origin};
246+
let rp = {id: window.origin, name: "none"};
237247
let makeCredentialOptions = {
238248
rp: rp, user: user, challenge: chall, pubKeyCredParams: [param]
239249
};

‎dom/webidl/WebAuthentication.webidl‎

Lines changed: 8 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -36,7 +36,7 @@ interface AuthenticatorAttestationResponse : AuthenticatorResponse {
3636
interface AuthenticatorAssertionResponse : AuthenticatorResponse {
3737
[SameObject] readonly attribute ArrayBuffer authenticatorData;
3838
[SameObject] readonly attribute ArrayBuffer signature;
39-
readonly attribute DOMString userId;
39+
[SameObject] readonly attribute ArrayBuffer userHandle;
4040
};
4141

4242
dictionary PublicKeyCredentialParameters {
@@ -59,17 +59,17 @@ dictionary MakePublicKeyCredentialOptions {
5959
};
6060

6161
dictionary PublicKeyCredentialEntity {
62-
DOMString name;
63-
USVString icon;
62+
required DOMString name;
63+
USVString icon;
6464
};
6565

6666
dictionary PublicKeyCredentialRpEntity : PublicKeyCredentialEntity {
6767
DOMString id;
6868
};
6969

7070
dictionary PublicKeyCredentialUserEntity : PublicKeyCredentialEntity {
71-
BufferSource id;
72-
DOMString displayName;
71+
required BufferSource id;
72+
required DOMString displayName;
7373
};
7474

7575
dictionary AuthenticatorSelectionCriteria {
@@ -115,9 +115,9 @@ enum PublicKeyCredentialType {
115115
};
116116

117117
dictionary PublicKeyCredentialDescriptor {
118-
required PublicKeyCredentialType type;
119-
required BufferSource id;
120-
sequence<AuthenticatorTransport> transports;
118+
required PublicKeyCredentialType type;
119+
required BufferSource id;
120+
sequence<AuthenticatorTransport> transports;
121121
};
122122

123123
enum AuthenticatorTransport {

0 commit comments

Comments
 (0)