-
Notifications
You must be signed in to change notification settings - Fork 100
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(webauthn): allow userName in register handler options #275
feat(webauthn): allow userName in register handler options #275
Conversation
commit: |
Old responseI think it is better to just do the check inside the handler already like you did before? That way the dev doesn't need to implement it themselves. (it will add new behaviour..)Something like this let user: T;
try {
// If user is logged in, use the current user's username
const { user } = await requireUserSession(event);
user = { userName: user.email };
}
catch {
// Else do the currently implemented checks
if (!body.user?.userName)
throw createError({
message: 'Invalid request, missing userName or verify property',
statusCode: 400,
})
user = body.user
if (validateUser) {
user = await validateUserData(body.user, validateUser)
}
}
// rest of registration event handler.. Just realized that we don't know what shape |
Other idea: if (body.verify === undefined || !body.user?.userName)
throw createError({
message: 'Invalid request, missing userName or verify property',
statusCode: 400,
}) the default |
Didn't quite get this, How would a said api method look like?
Another ideas was to have a separate method for this called |
@Gerbuuun Updated this to check if the user is already logged in, this way is a user is already logged in, then we pick the |
Allow passing userName directly to defineWebAuthnRegisterEventHandler to support linking credentials to existing users without requiring userName in request body.
No breaking change as it maintains backward compatibility while adding new functionality.
How it works?