feat: WebAuthN Sign In, Sign Up and Options methods support - NEW #952

victorbojica · 2024-10-17T15:26:36Z

ONLY REVIEW REQUIRED. NOTHING TESTED YET.

Summary of change

Implement WebAuthN support according to:
https://docs.google.com/document/d/1G7tO9_dSNi8wur3ajGg4pq-wiHatKDbHv2sBt-uSbQg/edit#heading=h.olee876uqu8a

Test Plan

No testing at the moment.

Documentation changes

Will have to add WebAuthN recipe documentation

Checklist for important updates

Remaining TODOs for this PR

lib/ts/recipe/multifactorauth/types.ts

lib/ts/recipe/webauthn/types.ts

lib/ts/recipe/multifactorauth/types.ts

lib/ts/recipe/webauthn/types.ts

cloudflare-workers-and-pages · 2024-10-18T13:01:09Z

Deploying supertokens-node-pr-check-for-edge-function-compat with Cloudflare Pages

Latest commit:	`ce371d1`
Status:	✅ Deploy successful!
Preview URL:	https://fa49a36e.supertokens-node-b95.pages.dev

View logs

lib/ts/recipe/webauthn/types.ts

lib/ts/recipe/multifactorauth/types.ts

lib/ts/recipe/webauthn/types.ts

lib/ts/recipe/multitenancy/types.ts

lib/ts/recipe/multifactorauth/types.ts

lib/ts/recipe/webauthn/types.ts

rishabhpoddar · 2024-10-28T07:10:59Z

lib/ts/recipe/webauthn/types.ts

+                  options: APIOptions;
+                  userContext: UserContext;
+              } & ({ email: string } | { recoverAccountToken: string })
+          ) => Promise<


How does webauthn flow work for native mobile apps?

Yes, through WebViews starting from iOS 16 and Android 9 (and 14 in some cases): https://passkeys.dev/docs/reference/android/ and https://passkeys.dev/docs/reference/ios/.

lib/ts/recipe/webauthn/types.ts

rishabhpoddar · 2024-10-28T16:47:12Z

lib/ts/recipe/webauthn/types.ts

              relyingPartyId: string;
-              relyingPartyName: string;
              origin: string;
-              userName: string;
-              userDisplayName: string;
+              email: string;
              timeout: string;
-              attestation: string;
              challenge: string;


why did we remove relyingPartyName, userName, userDisplayName and attestation?

I checked the schema and we don't store them in the generated options. userName is the email and i renamed it to be more simple.

So ideally, the output of registerOptions function should be similar to the output of this function. For example, in the registerOptions output, we have:

user: { id: string; name: string; // user email displayName: string; //user email };

Now in the output of getGeneratedOptions, it's totally different. So this is strange.

Because we only store what is needed to verify the credentials. See the schema here (webauthn_user_generated_options) https://docs.google.com/document/d/1G7tO9_dSNi8wur3ajGg4pq-wiHatKDbHv2sBt-uSbQg/edit?tab=t.0#heading=h.td4zxgo1675k

victorbojica · 2024-11-05T10:47:52Z

lib/ts/user.ts

@@ -90,6 +92,9 @@ export class User implements UserType {
        id: string;
        userId: string;
    }[];
+    public readonly webauthn: {
+        credentialIds: string[];
+    }[];


should use webauthnCredentialId because there is no need to have a list of objects

lib/ts/recipe/webauthn/error.ts

porcellus · 2024-11-08T10:32:10Z

lib/ts/recipe/accountlinking/types.ts

@@ -192,10 +192,13 @@ export type AccountInfo = {
        id: string;
        userId: string;
    };
+    webauthn?: {
+        credentialIds: string[];


This is also used by listUsersByAccount info. We may have to split types, or are there any cases where we'd want to search by multiple credentialIds? How does that function, do we only get a match if all credentialIds are present? Is there a use-case for that?

I haven't taken that into consideration. But as far as ai can tell, no. there is no use like that. There might the possibllity to search by one of the credentialIds. What do you think?

I think searching by a single credentialId makes sense.

lib/ts/recipe/webauthn/api/emailExists.ts

lib/ts/recipe/webauthn/api/recoverAccount.ts

lib/ts/recipe/webauthn/api/utils.ts

porcellus · 2024-11-08T12:19:17Z

lib/ts/recipe/webauthn/api/implementation.ts

+                LINKING_TO_SESSION_USER_FAILED: {
+                    EMAIL_VERIFICATION_REQUIRED:
+                        "Cannot sign in / up due to security reasons. Please contact support. (ERR_CODE_013)",
+                    RECIPE_USER_ID_ALREADY_LINKED_WITH_ANOTHER_PRIMARY_USER_ID_ERROR:
+                        "Cannot sign in / up due to security reasons. Please contact support. (ERR_CODE_014)",
+                    ACCOUNT_INFO_ALREADY_ASSOCIATED_WITH_ANOTHER_PRIMARY_USER_ID_ERROR:
+                        "Cannot sign in / up due to security reasons. Please contact support. (ERR_CODE_015)",
+                    SESSION_USER_ACCOUNT_INFO_ALREADY_ASSOCIATED_WITH_ANOTHER_PRIMARY_USER_ID_ERROR:
+                        "Cannot sign in / up due to security reasons. Please contact support. (ERR_CODE_016)",
+                },


I think the error codes will have to be updated. You can add a TODO here and wherever an update like this is pending.

What should be the update? Or am i missing something ?

well the error codes are login method specific, so we'll have to have new ones I think.

done. added to do.

porcellus · 2024-11-08T12:20:43Z

lib/ts/recipe/webauthn/api/implementation.ts

+            }
+
+            const preAuthCheckRes = await AuthUtils.preAuthChecks({
+                authenticatingAccountInfo: {


shouldn't this have the credentialId? or is that something we only get later?

I don't think so. We are identifying the user by email. And since we are signing up, the credential isn't stored yet.

so in no cases would this be relevant to this or other linking related functions? if so then the types may require some cleanup

will check if we need to remove webauthn credential ids from the type - it is not used atm

porcellus · 2024-11-08T12:22:49Z

lib/ts/recipe/webauthn/api/implementation.ts

+            const verifyCredentialsResponse = await options.recipeImplementation.verifyCredentials({
+                credential,
+                webauthnGeneratedOptionsId,
+                tenantId,
+                userContext,
+            });
+            const checkCredentialsOnTenant = async () => {
+                return verifyCredentialsResponse.status === "OK";
+            };


Is this correct? is credential validation independent of the tenant?

Done.

Was doing it like this, because at the moment of implementation, we hadn't decided on having a method for retrieving the webauthGeneratedOptions (which contained the email) and i had to retrieve the email before calling AuthUtils.getAuthenticatingUserAndAddToCurrentTenantIfRequired.
In the last few chats with Rishabh we deciede on adding the new method, but didn't update the API implmentation yet.

As a side note:
The credential can be shared across tenants and is not linked to a tenant.

I see. So the point of checkCredentialsOnTenant is to find out if the user can use the current set of credentials to log in on the given tenantId (it can take it as a first parameter). This is tenant independent for pwless and thirdparty, but not for emailpassword for example.

reverted to previous version because we don't need to check in the scope of the tenant.

porcellus · 2024-11-08T12:26:17Z

lib/ts/recipe/webauthn/api/implementation.ts

+                };
+            }
+
+            const authenticatingUser = await AuthUtils.getAuthenticatingUserAndAddToCurrentTenantIfRequired({


this function also needs to be updated to take the new account info prop into account (plus I think you'll need to check all places where we use the account info)

maybe we could have a chat about this.

same as previous comments. also need to check method to see if ti works with webauthn

porcellus · 2024-11-08T12:40:34Z

lib/ts/recipe/webauthn/types.ts

+          }
+        // | SignUpErrorResponse
+        | { status: "EMAIL_ALREADY_EXISTS_ERROR" }
+        | { status: "WRONG_CREDENTIALS_ERROR" }


This feels weird on a signUp endpoint, maybe specify which param is wrong/how?

The status is a generic one, for whatever error happens because of the Webauthn credentials flow. If we'd need to be more specific here (not in the API implementation - we should keep it generic there), then we'd need to do the actual core implementation, as there might lots of stuff that we can only find out when we start implementing.

There is another type of status "INVALID_AUTHENTICATOR_ERROR" (along with the reason) which means that the credential has been generated by an "unsupported" authenticator (this could happen by overriding the registerOptions method and overriding the default values for attestation, requireResidentKey, residentKey, userVerification, supportedAlgorithmIds, etc.).

What do you think ?

The INVALID_AUTHENTICATOR_ERROR sounds good. My main issue with WRONG_CREDENTIALS_ERROR is that it overlaps with the error status we have for people submitting wrong passwords I guess.

same as previous error discussion

niftyvictor added 5 commits October 10, 2024 15:47

add initial passkey types

c57b645

passkey types cleanup

56422f3

added untested support for options, sign in and sign up methods

f102128

Merge branch 'feat/webauthn/base' into feat/webauthn/basic-methods

de9fb88

updated types based on pr changes

40bd6e9

victorbojica changed the title ~~Feat/webauthn/basic methods~~ feat: WebAuthN Sign In, Sign Up and Options methods support - NEW Oct 17, 2024

victorbojica commented Oct 18, 2024

View reviewed changes

rishabhpoddar suggested changes Oct 18, 2024

View reviewed changes

pr changes. removed incorrect errors and added missing ones

e150fc2

added missing user type

8c8d711

rishabhpoddar suggested changes Oct 18, 2024

View reviewed changes

lib/ts/recipe/webauthn/types.ts Show resolved Hide resolved

added webauthn details to user object

ced57b1

rishabhpoddar suggested changes Oct 21, 2024

View reviewed changes

pr fixes. centralized error types and added crud for credentials

fbbed56

rishabhpoddar reviewed Oct 21, 2024

View reviewed changes

rishabhpoddar suggested changes Oct 21, 2024

View reviewed changes

lib/ts/recipe/webauthn/types.ts Outdated Show resolved Hide resolved

lib/ts/recipe/webauthn/types.ts Outdated Show resolved Hide resolved

niftyvictor added 3 commits October 21, 2024 16:26

pr fixes

64a4a6b

pr fixes and added decode method

3186aa5

added types implementation and minor fixes

3fa31ce

rishabhpoddar suggested changes Oct 28, 2024

View reviewed changes

lib/ts/recipe/webauthn/types.ts Outdated Show resolved Hide resolved

pr fixes

177b580

rishabhpoddar suggested changes Oct 28, 2024

View reviewed changes

pr fixes and cleanup

5d1363e

rishabhpoddar suggested changes Oct 28, 2024

View reviewed changes

pr fixes

57e210d

rishabhpoddar suggested changes Oct 28, 2024

View reviewed changes

updated initial recipe implementation

ea42bd1

victorbojica commented Nov 5, 2024

View reviewed changes

fixed implementation

b8cffc1

niftyvictor added 2 commits November 8, 2024 09:57

added basic build

bf9e00f

added basic build exports

b44752f

porcellus requested changes Nov 8, 2024

View reviewed changes

niftyvictor added 7 commits November 8, 2024 17:58

pr fixes

dba5cda

pr fixes

c84a76a

pr fixes

3649b45

pr fixes

3bb7234

pr fixes

ce371d1

added missing default email delivery implementation

d356701

added basic tests and mock

e743046

victorbojica merged commit 6a2e53f into feat/webauthn/base Nov 25, 2024
16 of 18 checks passed

victorbojica deleted the feat/webauthn/basic-methods branch November 25, 2024 11:48

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat: WebAuthN Sign In, Sign Up and Options methods support - NEW #952

feat: WebAuthN Sign In, Sign Up and Options methods support - NEW #952

victorbojica commented Oct 17, 2024 •

edited by rishabhpoddar

Loading

cloudflare-workers-and-pages bot commented Oct 18, 2024 •

edited

Loading

rishabhpoddar Oct 28, 2024

victorbojica Oct 29, 2024

victorbojica Oct 29, 2024

rishabhpoddar Oct 28, 2024

victorbojica Oct 29, 2024

rishabhpoddar Oct 30, 2024

victorbojica Nov 4, 2024

victorbojica Nov 5, 2024 •

edited

Loading

porcellus Nov 8, 2024

victorbojica Nov 8, 2024

porcellus Nov 11, 2024

porcellus Nov 8, 2024

victorbojica Nov 8, 2024

porcellus Nov 11, 2024

victorbojica Nov 12, 2024

porcellus Nov 8, 2024

victorbojica Nov 8, 2024

porcellus Nov 11, 2024

victorbojica Nov 12, 2024

porcellus Nov 8, 2024

victorbojica Nov 8, 2024

porcellus Nov 11, 2024

victorbojica Nov 12, 2024

victorbojica Nov 12, 2024

porcellus Nov 8, 2024

victorbojica Nov 8, 2024

victorbojica Nov 12, 2024

porcellus Nov 8, 2024

victorbojica Nov 8, 2024

porcellus Nov 11, 2024

victorbojica Nov 12, 2024

victorbojica Nov 12, 2024

feat: WebAuthN Sign In, Sign Up and Options methods support - NEW #952

feat: WebAuthN Sign In, Sign Up and Options methods support - NEW #952

Conversation

victorbojica commented Oct 17, 2024 • edited by rishabhpoddar Loading

Summary of change

Related

Test Plan

Documentation changes

Checklist for important updates

Remaining TODOs for this PR

cloudflare-workers-and-pages bot commented Oct 18, 2024 • edited Loading

Deploying supertokens-node-pr-check-for-edge-function-compat with Cloudflare Pages

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

victorbojica Nov 5, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

victorbojica commented Oct 17, 2024 •

edited by rishabhpoddar

Loading

cloudflare-workers-and-pages bot commented Oct 18, 2024 •

edited

Loading

victorbojica Nov 5, 2024 •

edited

Loading