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

Add customProperties to register/authenticate #83

Merged
merged 6 commits into from
Jan 27, 2025

Conversation

Paul-Taiwo
Copy link
Contributor

This PR introduces the excludeCredentials property to the passkey creation flow. This feature enhances the security and usability of the passkey creation process by preventing duplicate registrations of credentials associated with the same user.

@dagnelies
Copy link
Collaborator

Hi @Paul-Taiwo . Thank you very much for this PR. Although it's good, I slightly hesitate to accept it for various small reasons.

  1. I think it's a better practice to just set the user ID in order to avoid multiple passkeys for the same user. It's also simpler than passing lists of credential IDs around. It also has the advantage of overwriting the existing credential (if there is any) which often provides better UX (overwrite instead of error) and may also help overwrite a problematic credential.
  2. Less to document, understand, test, ...
  3. Allowing multiple accounts, like one for work and a personal one, or for multiple users on the same device, sounds more sensible to me.

Actually, I'm more inclined to revert the random user.id generation (it was a hash of the username before) or add an option to add arbitrary properties.

@Paul-Taiwo
Copy link
Contributor Author

Hi @dagnelies

Thank you for your feedback on my earlier PR. I’ve taken your suggestions into account and have made the following updates:

  1. Removed excludeCredentials
  2. Added customProperties Option:
    To retain flexibility for advanced use cases, I’ve introduced an optional customProperties field. This allows developers to add arbitrary properties to the creationOptions object without altering the default behavior.

@dagnelies
Copy link
Collaborator

Sounds great 👍

What do you think about adding the custom properties to both the registration and authentication? I think it would be more versatile and intuitive that way. Also, the docs must be updated so that people are aware this even exists.

Besides this nitpicking, it looks good and meaningful. Thanks for the contribution

@Paul-Taiwo
Copy link
Contributor Author

Hi @dagnelies,
I’ve added custom properties to the authentication and updated the documentation accordingly. Please let me know if there are any other changes you'd like me to make.

@dagnelies dagnelies merged commit be10451 into passwordless-id:main Jan 27, 2025
@dagnelies dagnelies changed the title Add excludeCredentials to Passkey Creation Process Add customProperties to register/authenticate Jan 27, 2025
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.

2 participants