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

feat: Add passkeys support #942

Closed
wants to merge 1 commit into from
Closed

feat: Add passkeys support #942

wants to merge 1 commit into from

Conversation

victorbojica
Copy link

Summary of change

Add passkeys support

Related issues

Test Plan

  • Mock core API calls
  • Add tests for each method individually

More details here: https://docs.google.com/document/d/1G7tO9_dSNi8wur3ajGg4pq-wiHatKDbHv2sBt-uSbQg/edit

@victorbojica victorbojica changed the title Add passkeys support feat: Add passkeys support Oct 10, 2024
Comment on lines +461 to +473
// todo update this ???
export type TypeEmailPasswordPasswordResetEmailDeliveryInput = {
type: "PASSWORD_RESET";
user: {
id: string;
recipeUserId: RecipeUserId | undefined;
email: string;
};
passwordResetLink: string;
tenantId: string;
};

export type TypeEmailPasswordEmailDeliveryInput = TypeEmailPasswordPasswordResetEmailDeliveryInput;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

needs updating

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

done

Comment on lines +27 to +28
// default implementation for the TypeInput
// todo update this ???
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
// default implementation for the TypeInput
// todo update this ???

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

done

export type RecipeInterface = {
registerPasskeyOptions(input: {
email: string;
password: string;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
password: string;

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

done

registerPasskeyOptions(input: {
email: string;
password: string;
session: SessionContainerInterface | undefined;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
session: SessionContainerInterface | undefined;

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

done

tenantId: string;
userContext: UserContext;
}): Promise<{
status: "OK";
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Are there any failure cases in this API?

status: "OK";
passkeyGeneratedOptionsId: string;
rp: {
id: string;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

where are we getting the id from?

Comment on lines +75 to +76
name: string;
displayName: string;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

where are these two coming from?

user: User;
recipeUserId: RecipeUserId;
}
| { status: "EMAIL_ALREADY_EXISTS_ERROR" }
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

not sure if this is the best way, cause if this error comes, but the user has already added a passkey in their browser, so it would get confusing

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

So instead, we should have this type of error in the part where we are generating a passkeyoption for the purpose of sign up?

userContext: UserContext;
}): Promise<
| { status: "OK"; user: User; recipeUserId: RecipeUserId }
| { status: "WRONG_CREDENTIALS_ERROR" }
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should we call this WRONG_CREDENTIALS_ERROR or something else?

userContext: UserContext;
}): Promise<{ status: "OK"; token: string } | { status: "UNKNOWN_USER_ID_ERROR" }>;

consumeRecoverAccountToken(input: {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

how will this work? LIke the user has to first generate a passkey options right and then call this function after?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants