Fix bugs with WebAuthn preventing sign in and registration. (#22651)

This PR fixes two bugs with Webauthn support: * There was a longstanding bug within webauthn due to the backend using URLEncodedBase64 but the javascript using decoding using plain base64. This causes intermittent issues with users reporting decoding errors. * Following the recent upgrade to webauthn there was a change in the way the library expects RPOrigins to be configured. This leads to the Relying Party Origin not being configured and prevents registration. Fix #22507 Signed-off-by: Andrew Thornton <art27@cantab.net> Co-authored-by: wxiaoguang <wxiaoguang@gmail.com>
go-gitea · Feb 1, 2023 · 19d5b2f · 19d5b2f
1 parent 2871ea0
commit 19d5b2f
Show file tree

Hide file tree

Showing 3 changed files with 25 additions and 18 deletions.
diff --git a/modules/auth/webauthn/webauthn.go b/modules/auth/webauthn/webauthn.go
@@ -28,7 +28,7 @@ func Init() {
 		Config: &webauthn.Config{
 			RPDisplayName: setting.AppName,
 			RPID:          setting.Domain,
-			RPOrigin:      appURL,
+			RPOrigins:     []string{appURL},
 			AuthenticatorSelection: protocol.AuthenticatorSelection{
 				UserVerification: "discouraged",
 			},

diff --git a/modules/auth/webauthn/webauthn_test.go b/modules/auth/webauthn/webauthn_test.go
@@ -15,11 +15,11 @@ func TestInit(t *testing.T) {
 	setting.Domain = "domain"
 	setting.AppName = "AppName"
 	setting.AppURL = "https://domain/"
-	rpOrigin := "https://domain"
+	rpOrigin := []string{"https://domain"}
 
 	Init()
 
 	assert.Equal(t, setting.Domain, WebAuthn.Config.RPID)
 	assert.Equal(t, setting.AppName, WebAuthn.Config.RPDisplayName)
-	assert.Equal(t, rpOrigin, WebAuthn.Config.RPOrigin)
+	assert.Equal(t, rpOrigin, WebAuthn.Config.RPOrigins)
 }
diff --git a/web_src/js/features/user-auth-webauthn.js b/web_src/js/features/user-auth-webauthn.js
@@ -14,9 +14,9 @@ export function initUserAuthWebAuthn() {
 
   $.getJSON(`${appSubUrl}/user/webauthn/assertion`, {})
     .done((makeAssertionOptions) => {
-      makeAssertionOptions.publicKey.challenge = decode(makeAssertionOptions.publicKey.challenge);
+      makeAssertionOptions.publicKey.challenge = decodeURLEncodedBase64(makeAssertionOptions.publicKey.challenge);
       for (let i = 0; i < makeAssertionOptions.publicKey.allowCredentials.length; i++) {
-        makeAssertionOptions.publicKey.allowCredentials[i].id = decode(makeAssertionOptions.publicKey.allowCredentials[i].id);
+        makeAssertionOptions.publicKey.allowCredentials[i].id = decodeURLEncodedBase64(makeAssertionOptions.publicKey.allowCredentials[i].id);
       }
       navigator.credentials.get({
         publicKey: makeAssertionOptions.publicKey
@@ -56,14 +56,14 @@ function verifyAssertion(assertedCredential) {
     type: 'POST',
     data: JSON.stringify({
       id: assertedCredential.id,
-      rawId: bufferEncode(rawId),
+      rawId: encodeURLEncodedBase64(rawId),
       type: assertedCredential.type,
       clientExtensionResults: assertedCredential.getClientExtensionResults(),
       response: {
-        authenticatorData: bufferEncode(authData),
-        clientDataJSON: bufferEncode(clientDataJSON),
-        signature: bufferEncode(sig),
-        userHandle: bufferEncode(userHandle),
+        authenticatorData: encodeURLEncodedBase64(authData),
+        clientDataJSON: encodeURLEncodedBase64(clientDataJSON),
+        signature: encodeURLEncodedBase64(sig),
+        userHandle: encodeURLEncodedBase64(userHandle),
       },
     }),
     contentType: 'application/json; charset=utf-8',
@@ -85,14 +85,21 @@ function verifyAssertion(assertedCredential) {
   });
 }
 
-// Encode an ArrayBuffer into a base64 string.
-function bufferEncode(value) {
+// Encode an ArrayBuffer into a URLEncoded base64 string.
+function encodeURLEncodedBase64(value) {
   return encode(value)
     .replace(/\+/g, '-')
     .replace(/\//g, '_')
     .replace(/=/g, '');
 }
 
+// Dccode a URLEncoded base64 to an ArrayBuffer string.
+function decodeURLEncodedBase64(value) {
+  return decode(value
+    .replace(/_/g, '/')
+    .replace(/-/g, '+'));
+}
+
 function webauthnRegistered(newCredential) {
   const attestationObject = new Uint8Array(newCredential.response.attestationObject);
   const clientDataJSON = new Uint8Array(newCredential.response.clientDataJSON);
@@ -104,11 +111,11 @@ function webauthnRegistered(newCredential) {
     headers: {'X-Csrf-Token': csrfToken},
     data: JSON.stringify({
       id: newCredential.id,
-      rawId: bufferEncode(rawId),
+      rawId: encodeURLEncodedBase64(rawId),
       type: newCredential.type,
       response: {
-        attestationObject: bufferEncode(attestationObject),
-        clientDataJSON: bufferEncode(clientDataJSON),
+        attestationObject: encodeURLEncodedBase64(attestationObject),
+        clientDataJSON: encodeURLEncodedBase64(clientDataJSON),
       },
     }),
     dataType: 'json',
@@ -184,11 +191,11 @@ function webAuthnRegisterRequest() {
   }).done((makeCredentialOptions) => {
     $('#nickname').closest('div.field').removeClass('error');
 
-    makeCredentialOptions.publicKey.challenge = decode(makeCredentialOptions.publicKey.challenge);
-    makeCredentialOptions.publicKey.user.id = decode(makeCredentialOptions.publicKey.user.id);
+    makeCredentialOptions.publicKey.challenge = decodeURLEncodedBase64(makeCredentialOptions.publicKey.challenge);
+    makeCredentialOptions.publicKey.user.id = decodeURLEncodedBase64(makeCredentialOptions.publicKey.user.id);
     if (makeCredentialOptions.publicKey.excludeCredentials) {
       for (let i = 0; i < makeCredentialOptions.publicKey.excludeCredentials.length; i++) {
-        makeCredentialOptions.publicKey.excludeCredentials[i].id = decode(makeCredentialOptions.publicKey.excludeCredentials[i].id);
+        makeCredentialOptions.publicKey.excludeCredentials[i].id = decodeURLEncodedBase64(makeCredentialOptions.publicKey.excludeCredentials[i].id);
       }
     }