Merge pull request #507 from MasterKale/fix/503-consistent-utf8-chall…

…enge-strings fix/503-consistent-utf8-challenge-strings
MasterKale · Jan 20, 2024 · dd387e6 · dd387e6
2 parents 2f4d01b + 65ec8a1
commit dd387e6
Show file tree

Hide file tree

Showing 4 changed files with 38 additions and 9 deletions.
diff --git a/.github/workflows/ciChecks.yml b/.github/workflows/ciChecks.yml
@@ -29,14 +29,16 @@ jobs:
       uses: actions/setup-node@v3
       with:
         node-version: ${{ matrix.node-version }}
-    - run: node -v
+    - name: Confirm installed Node version
+      run: node -v
 
     # Install Deno
     - name: Setup Deno ${{ matrix.deno-version }}
       uses: maximousblk/setup-deno@v2
       with:
         deno-version: ${{ matrix.deno-version }}
-    - run: deno -V
+    - name: Confirm installed Deno version
+      run: deno -V
 
     # Install pnpm w/cache for quicker installs
     # https://github.com/pnpm/action-setup#use-cache-to-reduce-installation-time
@@ -61,9 +63,10 @@ jobs:
     - name: Install dependencies
       run: pnpm install
 
-    # Build and test dnt packages
-    - run: npm run build:types   # browser tests will need this to be built
-    - run: npm run build:server  # dnt will test everything in Node too
-
-    # Test packages
-    - run: npm run test:browser
+    # Build and test packages
+    - name: Build & test @simplewebauthn/typescript-types
+      run: npm run build:types   # browser tests will need this to be built
+    - name: Build & test @simplewebauthn/server
+      run: npm run build:server  # dnt will test everything in Node too
+    - name: Test @simplewebauthn/browser
+      run: npm run test:browser
diff --git a/packages/server/src/authentication/generateAuthenticationOptions.test.ts b/packages/server/src/authentication/generateAuthenticationOptions.test.ts
@@ -132,6 +132,17 @@ Deno.test('should generate a challenge if one is not provided', async () => {
   assert(isoBase64URL.isBase64url(options.challenge));
 });
 
+Deno.test('should treat string challenges as UTF-8 strings', async () => {
+  const options = await generateAuthenticationOptions({
+    challenge: 'こんにちは',
+  });
+
+  assertEquals(
+    options.challenge,
+    '44GT44KT44Gr44Gh44Gv',
+  );
+});
+
 Deno.test('should set rpId if specified', async () => {
   const rpID = 'simplewebauthn.dev';
 

diff --git a/packages/server/src/registration/generateRegistrationOptions.test.ts b/packages/server/src/registration/generateRegistrationOptions.test.ts
@@ -192,6 +192,21 @@ Deno.test('should generate a challenge if one is not provided', async () => {
   mockGenerateChallenge.restore();
 });
 
+Deno.test('should treat string challenges as UTF-8 strings', async () => {
+  const options = await generateRegistrationOptions({
+    rpID: 'not.real',
+    rpName: 'SimpleWebAuthn',
+    userID: '1234',
+    userName: 'usernameHere',
+    challenge: 'こんにちは',
+  });
+
+  assertEquals(
+    options.challenge,
+    '44GT44KT44Gr44Gh44Gv',
+  );
+});
+
 Deno.test('should use custom supported algorithm IDs as-is when provided', async () => {
   const options = await generateRegistrationOptions({
     rpID: 'not.real',

diff --git a/packages/server/src/registration/generateRegistrationOptions.ts b/packages/server/src/registration/generateRegistrationOptions.ts
@@ -157,7 +157,7 @@ export async function generateRegistrationOptions(
    */
   let _challenge = challenge;
   if (typeof _challenge === 'string') {
-    _challenge = isoUint8Array.fromASCIIString(_challenge);
+    _challenge = isoUint8Array.fromUTF8String(_challenge);
   }
 
   return {