Merge pull request #259 from MasterKale/fix/improve-resident-key-registration-options

Fix/improve resident key registration options

2 files changed, 114 insertions, 7 deletions

diff --git a/packages/server/src/registration/generateRegistrationOptions.test.ts b/packages/server/src/registration/generateRegistrationOptions.test.ts
index db4e11e..7b64434 100644
--- a/packages/server/src/registration/generateRegistrationOptions.test.ts
+++ b/packages/server/src/registration/generateRegistrationOptions.test.ts
@@ -166,3 +166,90 @@ test('should use custom supported algorithm IDs as-is when provided', () => {
     { alg: -65535, type: 'public-key' },
   ]);
 });
+
+test('should require resident key if residentKey option is absent but requireResidentKey is set to true', () => {
+  const options = generateRegistrationOptions({
+    rpID: 'not.real',
+    rpName: 'SimpleWebAuthn',
+    userID: '1234',
+    userName: 'usernameHere',
+    authenticatorSelection: {
+      requireResidentKey: true,
+    }
+  });
+
+  expect(options.authenticatorSelection?.requireResidentKey).toEqual(true);
+  expect(options.authenticatorSelection?.residentKey).toEqual('required');
+});
+
+test('should discourage resident key if residentKey option is absent but requireResidentKey is set to false', () => {
+  const options = generateRegistrationOptions({
+    rpID: 'not.real',
+    rpName: 'SimpleWebAuthn',
+    userID: '1234',
+    userName: 'usernameHere',
+    authenticatorSelection: {
+      requireResidentKey: false,
+    }
+  });
+
+  expect(options.authenticatorSelection?.requireResidentKey).toEqual(false);
+  expect(options.authenticatorSelection?.residentKey).toBeUndefined();
+});
+
+test('should not set resident key if both residentKey and requireResidentKey options are absent', () => {
+  const options = generateRegistrationOptions({
+    rpID: 'not.real',
+    rpName: 'SimpleWebAuthn',
+    userID: '1234',
+    userName: 'usernameHere',
+  });
+
+  expect(options.authenticatorSelection?.requireResidentKey).toEqual(false);
+  expect(options.authenticatorSelection?.residentKey).toBeUndefined();
+});
+
+test('should set requireResidentKey to true if residentKey if set to required', () => {
+  const options = generateRegistrationOptions({
+    rpID: 'not.real',
+    rpName: 'SimpleWebAuthn',
+    userID: '1234',
+    userName: 'usernameHere',
+    authenticatorSelection: {
+      residentKey: 'required',
+    },
+  });
+
+  expect(options.authenticatorSelection?.requireResidentKey).toEqual(true);
+  expect(options.authenticatorSelection?.residentKey).toEqual('required');
+});
+
+test('should set requireResidentKey to false if residentKey if set to preferred', () => {
+  const options = generateRegistrationOptions({
+    rpID: 'not.real',
+    rpName: 'SimpleWebAuthn',
+    userID: '1234',
+    userName: 'usernameHere',
+    authenticatorSelection: {
+      residentKey: 'preferred',
+    },
+  });
+
+  expect(options.authenticatorSelection?.requireResidentKey).toEqual(false);
+  expect(options.authenticatorSelection?.residentKey).toEqual('preferred');
+});
+
+test('should set requireResidentKey to false if residentKey if set to discouraged', () => {
+  const options = generateRegistrationOptions({
+    rpID: 'not.real',
+    rpName: 'SimpleWebAuthn',
+    userID: '1234',
+    userName: 'usernameHere',
+    authenticatorSelection: {
+      residentKey: 'discouraged',
+    },
+  });
+
+  expect(options.authenticatorSelection?.requireResidentKey).toEqual(false);
+  expect(options.authenticatorSelection?.residentKey).toEqual('discouraged');
+});
diff --git a/packages/server/src/registration/generateRegistrationOptions.ts b/packages/server/src/registration/generateRegistrationOptions.ts
index d8d0ab5..0f281f2 100644
--- a/packages/server/src/registration/generateRegistrationOptions.ts
+++ b/packages/server/src/registration/generateRegistrationOptions.ts
@@ -120,15 +120,35 @@ export function generateRegistrationOptions(
   }));
 
   /**
-   * "Relying Parties SHOULD set [requireResidentKey] to true if, and only if, residentKey is set
-   * to "required""
-   *
-   * See https://www.w3.org/TR/webauthn-2/#dom-authenticatorselectioncriteria-requireresidentkey
+   * Capture some of the nuances of how `residentKey` and `requireResidentKey` how either is set
+   * depending on when either is defined in the options
    */
-  if (authenticatorSelection.residentKey === 'required') {
-    authenticatorSelection.requireResidentKey = true;
+  if (authenticatorSelection.residentKey === undefined) {
+    /**
+     * `residentKey`: "If no value is given then the effective value is `required` if
+     * requireResidentKey is true or `discouraged` if it is false or absent."
+     *
+     * See https://www.w3.org/TR/webauthn-2/#dom-authenticatorselectioncriteria-residentkey
+     */
+    if (authenticatorSelection.requireResidentKey) {
+      authenticatorSelection.residentKey = 'required';
+    } else {
+      /**
+       * FIDO Conformance v1.7.2 fails the first test if we do this, even though this is
+       * technically compatible with the WebAuthn L2 spec...
+       */
+      // authenticatorSelection.residentKey = 'discouraged';
+    }
   } else {
-    authenticatorSelection.requireResidentKey = false;
+    /**
+     * `requireResidentKey`: "Relying Parties SHOULD set it to true if, and only if, residentKey is
+     * set to "required""
+     *
+     * Spec says this property defaults to `false` so we should still be okay to assign `false` too
+     *
+     * See https://www.w3.org/TR/webauthn-2/#dom-authenticatorselectioncriteria-requireresidentkey
+     */
+    authenticatorSelection.requireResidentKey = authenticatorSelection.residentKey === 'required';
   }
 
   return {