Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Demo for Failed to execute 'atob' on 'Window': The string to be decoded is not correctly encoded. #22654

Closed
wants to merge 2 commits into from

Conversation

wxiaoguang
Copy link
Contributor

Before:

  • Console error: Uncaught DOMException: Failed to execute 'atob' on 'Window': The string to be decoded is not correctly encoded.
  • The webauthn dialog can not be displayed.

After:

  • No console error.
  • The dialog can be displayed.

image

@delvh
Copy link
Member

delvh commented Jan 29, 2023

While your changes make more sense regarding the error message, they don't explain why it was previously working and then just… stopped working?

@GiteaBot GiteaBot added the lgtm/need 2 This PR needs two approvals by maintainers to be considered for merging. label Jan 29, 2023
@zeripath
Copy link
Contributor

I was definitely experiencing something where the atob function appeared to be not present.

I equally don't understand as to why this is all of a sudden required when it was not previously.

@zeripath
Copy link
Contributor

zeripath commented Jan 29, 2023

ah I think I understand ... yup this should work!

The decode function was missing that we're urlencoding the base64 and not de-urlencoding properly.

@GiteaBot GiteaBot added lgtm/need 1 This PR needs approval from one additional maintainer to be merged. and removed lgtm/need 2 This PR needs two approvals by maintainers to be considered for merging. labels Jan 29, 2023
zeripath added a commit to zeripath/gitea that referenced this pull request Jan 29, 2023
Signed-off-by: Andrew Thornton <art27@cantab.net>
@zeripath
Copy link
Contributor

We need to change every decode to do the same thing.

patch
diff --git a/web_src/js/features/user-auth-webauthn.js b/web_src/js/features/user-auth-webauthn.js
index f11a49864..ed210c843 100644
--- 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: bufferURLEncodedBase64(rawId),
       type: assertedCredential.type,
       clientExtensionResults: assertedCredential.getClientExtensionResults(),
       response: {
-        authenticatorData: bufferEncode(authData),
-        clientDataJSON: bufferEncode(clientDataJSON),
-        signature: bufferEncode(sig),
-        userHandle: bufferEncode(userHandle),
+        authenticatorData: bufferURLEncodedBase64(authData),
+        clientDataJSON: bufferURLEncodedBase64(clientDataJSON),
+        signature: bufferURLEncodedBase64(sig),
+        userHandle: bufferURLEncodedBase64(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 bufferURLEncodedBase64(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: bufferURLEncodedBase64(rawId),
       type: newCredential.type,
       response: {
-        attestationObject: bufferEncode(attestationObject),
-        clientDataJSON: bufferEncode(clientDataJSON),
+        attestationObject: bufferURLEncodedBase64(attestationObject),
+        clientDataJSON: bufferURLEncodedBase64(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);
       }
     }
 

@wxiaoguang
Copy link
Contributor Author

wxiaoguang commented Jan 30, 2023

While your changes make more sense regarding the error message, they don't explain why it was previously working and then just… stopped working?

Because maybe the upgraded webauthn (go lib) changed their chanllenge id to URL base64 encoding, which may be required for webauthn standard. Old webauthn lib outputed standard base64 encoding.

Please see my previous comment:

image

@wxiaoguang
Copy link
Contributor Author

And it seems that only challenge id is using URL encoding at the moment, I am not sure whether attestationObject and others need it or not. And I am not using webauthn now nor I have read the webauthn standard about the encodings, so this PR is only a demo, I will close this one.

@wxiaoguang wxiaoguang closed this Jan 30, 2023
@wxiaoguang wxiaoguang deleted the demo branch January 30, 2023 01:14
@go-gitea go-gitea locked and limited conversation to collaborators May 3, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
lgtm/need 1 This PR needs approval from one additional maintainer to be merged.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants