Feat custom user #1978
Merged
Feat custom user #1978
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Add custom user ID to a user. This custom user ID is only used for verifying assertions from imported passkeys.
Change the DB schema to restrict a user handle to only one user.
bjoern-m
reviewed
Dec 5, 2024
| @@ -354,3 +350,39 @@ func (s *webauthnService) VerifyAttestationResponse(p VerifyAttestationResponseP | |||
|
|
|||
| return credential, nil | |||
| } | |||
|
|
|||
| func (s *webauthnService) GetWebAuthnUser(tx *pop.Connection, credential models.WebauthnCredential, userID uuid.UUID) (webauthn.User, *models.User, error) { | |||
| var customUserHandle *string = nil | |||
There was a problem hiding this comment.
It seems that it is only possible to use (custom) user handles that are not UUID's. I would have expected that UserHandle.Handle is used when assigned to the related credential, the User.ID otherwise, so that the logic would look something like this:
var webAuthnID []bytes
if credentialModel.UserHandle != nil {
webAuthnID = []byte(credentialModel.UserHandle.Handle)
} else {
webAuthnID = credentialModel.User.ID.Bytes()
}
| @@ -90,7 +90,7 @@ func NewManager(jwkManager hankoJwk.Manager, config config.Config) (Manager, err | |||
| } | |||
|
|
|||
| // GenerateJWT creates a new session JWT for the given user | |||
| func (m *manager) GenerateJWT(userId uuid.UUID, email *dto.EmailJwt) (string, jwt.Token, error) { | |||
| func (m *manager) GenerateJWT(userId uuid.UUID, email *dto.EmailJwt, opts ...JWTOptions) (string, jwt.Token, error) { | |||
There was a problem hiding this comment.
Wouldn't it make sense to set the email parameter via a JWTOption too?
There was a problem hiding this comment.
Yes, that would make sense. This is just leftover from the previous implementation where a user would have a custom userID. And I forgot that its still there. Maybe we can do it in a separate PR?
backend/persistence/migrations/20241118114500_change_webauthn_credentials.up.fizz
Outdated
Show resolved
Hide resolved
backend/persistence/migrations/20241118114500_change_webauthn_credentials.up.fizz
Outdated
Show resolved
Hide resolved
Co-authored-by: bjoern-m <[email protected]>
Co-authored-by: bjoern-m <[email protected]>
bjoern-m
approved these changes
Dec 5, 2024
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Description
Add custom user handle to a webauthn credential. This custom user handle is only used for verifying assertions from imported passkeys.
New passkeys are created with the user id (uuid) from Hanko.
Implementation
Added a new column to the webauthn credentials table. Check in
WebauthnService.VerifyAssertionResponseif the userHandle in the response is a uuid, if not treat it as a custom user handle. Get the user based on the credential ID verify the response against the user credentialsTests
Keep in mind that the webauthn credential must be created and used with the same RP ID.
Todos
Additional context
This is needed when a relying party already has implemented passkeys, but wants to migrate to Hanko and does not use uuids as user identifiers.