chore: refactor webauthn code after bumping web-auth/webauthn-lib

Signed-off-by: Richard Steinmetz <[email protected]>

lib/Db/PublicKeyCredentialEntity.php

@@ -28,7 +28,7 @@
 namespace OCA\TwoFactorWebauthn\Db;
 
 use OCP\AppFramework\Db\Entity;
-use Ramsey\Uuid\Uuid;
+use Symfony\Component\Uid\Uuid;
 use Webauthn\PublicKeyCredentialSource;
 use Webauthn\TrustPath\TrustPathLoader;
 
@@ -134,7 +134,7 @@ public static function fromPublicKeyCrendentialSource(string $name,
 		$publicKeyCredentialEntity->setTransports(json_encode($publicKeyCredentialSource->getTransports()));
 		$publicKeyCredentialEntity->setAttestationType($publicKeyCredentialSource->getAttestationType());
 		$publicKeyCredentialEntity->setTrustPath(json_encode($publicKeyCredentialSource->getTrustPath()->jsonSerialize()));
-		$publicKeyCredentialEntity->setAaguid($publicKeyCredentialSource->getAaguid()->toString());
+		$publicKeyCredentialEntity->setAaguid($publicKeyCredentialSource->getAaguid()->toRfc4122());
 		$publicKeyCredentialEntity->setCredentialPublicKey(base64_encode($publicKeyCredentialSource->getCredentialPublicKey()));
 		$publicKeyCredentialEntity->setUserHandle($publicKeyCredentialSource->getUserHandle());
 		$publicKeyCredentialEntity->setCounter($publicKeyCredentialSource->getCounter());

lib/Service/WebAuthnManager.php

@@ -26,7 +26,6 @@
 
 namespace OCA\TwoFactorWebauthn\Service;
 
-use Assert\Assertion;
 use Cose\Algorithm\Manager;
 use Cose\Algorithm\Signature\ECDSA;
 use Cose\Algorithm\Signature\EdDSA;
@@ -132,20 +131,20 @@ public function startRegistration(IUser $user, string $serverHost): PublicKeyCre
 
 		$authenticatorSelectionCriteria = new AuthenticatorSelectionCriteria(
 			AuthenticatorSelectionCriteria::AUTHENTICATOR_ATTACHMENT_NO_PREFERENCE,
+			AuthenticatorSelectionCriteria::USER_VERIFICATION_REQUIREMENT_DISCOURAGED,
+			AuthenticatorSelectionCriteria::RESIDENT_KEY_REQUIREMENT_NO_PREFERENCE,
 			false,
-			AuthenticatorSelectionCriteria::USER_VERIFICATION_REQUIREMENT_DISCOURAGED
 		);
 
 		$publicKeyCredentialCreationOptions = new PublicKeyCredentialCreationOptions(
 			$rpEntity,
 			$userEntity,
 			$challenge,
 			$publicKeyCredentialParametersList,
-			$timeout,
-			$excludedPublicKeyDescriptors,
 			$authenticatorSelectionCriteria,
 			PublicKeyCredentialCreationOptions::ATTESTATION_CONVEYANCE_PREFERENCE_NONE,
-			null
+			$excludedPublicKeyDescriptors,
+			$timeout,
 		);
 
 		$this->session->set(self::TWOFACTORAUTH_WEBAUTHN_REGISTRATION, $publicKeyCredentialCreationOptions->jsonSerialize());
@@ -280,15 +279,15 @@ function ($device) {
 
 		$publicKeyCredentialRequestOptions = new PublicKeyCredentialRequestOptions(
 			random_bytes(32),                                                    // Challenge
-			60000,                                                              // Timeout
 			null,                                                                  // Relying Party ID
 			[],                                  // Registered PublicKeyCredentialDescriptor classes
 			null, // User verification requirement
+			60000,                                                              // Timeout
 			$extensions
 		);
 		$publicKeyCredentialRequestOptions
 			->setRpId($this->stripPort($serverHost))
-			->allowCredentials($registeredPublicKeyCredentialDescriptors)
+			->allowCredentials(...$registeredPublicKeyCredentialDescriptors)
 			->setUserVerification(PublicKeyCredentialRequestOptions::USER_VERIFICATION_REQUIREMENT_DISCOURAGED);
 
 		$this->session->set(self::TWOFACTORAUTH_WEBAUTHN_REQUEST, $publicKeyCredentialRequestOptions->jsonSerialize());
@@ -364,7 +363,6 @@ public function finishAuthenticate(IUser $user, string $data) {
 
 	public function removeDevice(IUser $user, int $id) {
 		$credential = $this->mapper->findById($id);
-		Assertion::eq($credential->getUserHandle(), $user->getUID());
 
 		$this->mapper->delete($credential);
 
@@ -382,7 +380,6 @@ public function deactivateAllDevices(IUser $user) {
 
 	public function changeActivationState(IUser $user, int $id, bool $active) {
 		$credential = $this->mapper->findById($id);
-		Assertion::eq($credential->getUserHandle(), $user->getUID());
 
 		$credential->setActive($active);
 

src/components/AddDeviceDialog.vue

@@ -76,6 +76,7 @@ import {
 } from '../services/RegistrationService.js'
 
 import logger from '../logger.js'
+import { arrayToBase64UnpaddedString, base64url2base64 } from '../utils/base64.js'
 
 const RegistrationSteps = Object.freeze({
 	READY: 1,
@@ -106,27 +107,6 @@ export default {
 	},
 
 	methods: {
-		arrayToBase64String(a) {
-			return btoa(String.fromCharCode(...a))
-		},
-
-		base64url2base64(input) {
-			input = input
-				.replace(/=/g, '')
-				.replace(/-/g, '+')
-				.replace(/_/g, '/')
-
-			const pad = input.length % 4
-			if (pad) {
-				if (pad === 1) {
-					throw new Error('InvalidLengthError: Input base64url string is the wrong length to determine padding')
-				}
-				input += new Array(5 - pad).join('=')
-			}
-
-			return input
-		},
-
 		async start() {
 			this.errorMessage = null
 			logger.info('Starting to add a new twofactor webauthn device')
@@ -156,11 +136,11 @@ export default {
 		async getRegistrationData() {
 			try {
 				const publicKey = await startRegistration()
-				publicKey.challenge = Uint8Array.from(window.atob(this.base64url2base64(publicKey.challenge)), c => c.charCodeAt(0))
+				publicKey.challenge = Uint8Array.from(window.atob(base64url2base64(publicKey.challenge)), c => c.charCodeAt(0))
 				publicKey.user.id = Uint8Array.from(window.atob(publicKey.user.id), c => c.charCodeAt(0))
 				if (publicKey.excludeCredentials) {
 					publicKey.excludeCredentials = publicKey.excludeCredentials.map(data => {
-						data.id = Uint8Array.from(window.atob(this.base64url2base64(data.id)), c => c.charCodeAt(0))
+						data.id = Uint8Array.from(window.atob(base64url2base64(data.id)), c => c.charCodeAt(0))
 						return data
 					})
 				}
@@ -180,10 +160,10 @@ export default {
 				this.credential = {
 					id: data.id,
 					type: data.type,
-					rawId: this.arrayToBase64String(new Uint8Array(data.rawId)),
+					rawId: arrayToBase64UnpaddedString(new Uint8Array(data.rawId)),
 					response: {
-						clientDataJSON: this.arrayToBase64String(new Uint8Array(data.response.clientDataJSON)),
-						attestationObject: this.arrayToBase64String(new Uint8Array(data.response.attestationObject)),
+						clientDataJSON: arrayToBase64UnpaddedString(new Uint8Array(data.response.clientDataJSON)),
+						attestationObject: arrayToBase64UnpaddedString(new Uint8Array(data.response.attestationObject)),
 					},
 				}
 				logger.debug('mapped credentials data')

src/components/Challenge.vue

@@ -67,7 +67,7 @@
 import { mapGetters } from 'vuex'
 import { NcButton } from '@nextcloud/vue'
 import logger from '../logger.js'
-import { arrayToBase64String, base64StringToArray, base64url2base64 } from '../utils/base64.js'
+import { arrayToBase64UnpaddedString, base64StringToArray, base64url2base64 } from '../utils/base64.js'
 
 export default {
 	name: 'Challenge',
@@ -138,12 +138,12 @@ export default {
 			const challenge = {
 				id: data.id,
 				type: data.type,
-				rawId: arrayToBase64String(new Uint8Array(data.rawId)),
+				rawId: arrayToBase64UnpaddedString(new Uint8Array(data.rawId)),
 				response: {
-					authenticatorData: arrayToBase64String(new Uint8Array(data.response.authenticatorData)),
-					clientDataJSON: arrayToBase64String(new Uint8Array(data.response.clientDataJSON)),
-					signature: arrayToBase64String(new Uint8Array(data.response.signature)),
-					userHandle: data.response.userHandle ? arrayToBase64String(new Uint8Array(data.response.userHandle)) : null,
+					authenticatorData: arrayToBase64UnpaddedString(new Uint8Array(data.response.authenticatorData)),
+					clientDataJSON: arrayToBase64UnpaddedString(new Uint8Array(data.response.clientDataJSON)),
+					signature: arrayToBase64UnpaddedString(new Uint8Array(data.response.signature)),
+					userHandle: data.response.userHandle ? arrayToBase64UnpaddedString(new Uint8Array(data.response.userHandle)) : null,
 				},
 			}
 			logger.debug('mapped credentials', { challenge })

src/tests/unit/utils/base64.spec.js

@@ -20,14 +20,14 @@
  *
  */
 
-import { arrayToBase64String, base64StringToArray, base64url2base64 } from '../../../utils/base64.js'
+import { arrayToBase64UnpaddedString, base64StringToArray, base64url2base64 } from '../../../utils/base64.js'
 
 describe('utils/base64', () => {
-	it('should convert byte arrays to base64 strings', () => {
-		expect(arrayToBase64String(new Uint8Array([4, 2]))).to.equal('BAI=')
-		expect(arrayToBase64String(new Uint8Array([4]))).to.equal('BA==')
-		expect(arrayToBase64String(new Uint8Array([1, 2, 3, 4, 5, 6]))).to.equal('AQIDBAUG')
-		expect(arrayToBase64String(new Uint8Array([]))).to.equal('')
+	it('should convert byte arrays to unpadded base64 strings', () => {
+		expect(arrayToBase64UnpaddedString(new Uint8Array([4, 2]))).to.equal('BAI')
+		expect(arrayToBase64UnpaddedString(new Uint8Array([4]))).to.equal('BA')
+		expect(arrayToBase64UnpaddedString(new Uint8Array([1, 2, 3, 4, 5, 6]))).to.equal('AQIDBAUG')
+		expect(arrayToBase64UnpaddedString(new Uint8Array([]))).to.equal('')
 	})
 
 	it('should convert base64 strings to byte arrays', () => {

src/utils/base64.js

@@ -21,13 +21,13 @@
  */
 
 /**
- * Encode an array of bytes as a base64 string.
+ * Encode an array of bytes as an unpadded base64 string.
  *
  * @param {Uint8Array} a An array of bytes
- * @return {string} A base64 string
+ * @return {string} An unpadded base64 string
  */
-export function arrayToBase64String(a) {
-	return btoa(String.fromCharCode(...a))
+export function arrayToBase64UnpaddedString(a) {
+	return btoa(String.fromCharCode(...a)).replace(/=+$/g, '')
 }
 
 /**

tests/Unit/Controller/SettingsControllerTest.php

@@ -34,6 +34,8 @@
 use OCP\IUserSession;
 use PHPUnit\Framework\MockObject\MockObject;
 use Webauthn\PublicKeyCredentialCreationOptions;
+use Webauthn\PublicKeyCredentialRpEntity;
+use Webauthn\PublicKeyCredentialUserEntity;
 
 class SettingsControllerTest extends TestCase {
 
@@ -64,7 +66,11 @@ public function testStartRegister(): void {
 		$this->userSession->expects(self::once())
 			->method('getUser')
 			->willReturn($user);
-		$publicKeyCredentialCreationOptions = $this->createMock(PublicKeyCredentialCreationOptions::class);
+		$publicKeyCredentialCreationOptions = new PublicKeyCredentialCreationOptions(
+			new PublicKeyCredentialRpEntity('relying_party'),
+			new PublicKeyCredentialUserEntity('user', 'user_id', 'User'),
+			'challenge',
+		);
 		$this->webauthnManager->expects(self::once())
 			->method('startRegistration')
 			->with(self::equalTo($user))

tests/Unit/Provider/WebAuthnProviderTest.php

@@ -111,7 +111,7 @@ public function testGetDescription(): void {
 
 	public function testGetTemplate(): void {
 		$user = $this->createMock(IUser::class);
-		$key = $this->createMock(PublicKeyCredentialRequestOptions::class);
+		$key = new PublicKeyCredentialRequestOptions('challenge');
 		$serverHost = 'my.next.cloud';
 		$this->request->expects(self::once())
 			->method('getServerHost')