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

Refactor finding rpId for login #2751

Merged
merged 3 commits into from
Dec 17, 2024
Merged

Refactor finding rpId for login #2751

merged 3 commits into from
Dec 17, 2024

Conversation

lmuntaner
Copy link
Collaborator

@lmuntaner lmuntaner commented Dec 17, 2024

Motivation

There are scenarios where the RP ID selected is not the correct one for the device the user is in. When that happens, we want to tell the user to try again, but this time, we will select another RP ID.

To be able to select the right RP ID the second time, we need to remember which RP ID was selected in the first place (or second place). We can't store that in the MultiWebAuthnIdentity instance, because there is a new instance every time the user tries to log in.

Instead, I want to store this info in the Connection instance, which is the same across tries.

In this PR, I move the logic of selecting the RP ID to the Connection.fromWebauthnCredentials.

Changes

  • Move logic of selecting RP ID to Connection.fromWebauthnCredentials.
  • Add rpId parameter to MultiWebAuthnIdentity constructors.

Tests

  • Add tests for the login method of Connection class to check that rpID is passed as expected.

🟡 Some screens were changed

@lmuntaner lmuntaner requested review from sea-snake and LXIF December 17, 2024 10:17
@lmuntaner lmuntaner marked this pull request as ready for review December 17, 2024 10:18
@lmuntaner
Copy link
Collaborator Author

@LXIF @sea-snake please review

Copy link
Contributor

@LXIF LXIF left a comment

Choose a reason for hiding this comment

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

LGTM :)

@lmuntaner lmuntaner added this pull request to the merge queue Dec 17, 2024
Merged via the queue into main with commit 5e77adc Dec 17, 2024
68 checks passed
@lmuntaner lmuntaner deleted the lm-add-second-third-try branch December 17, 2024 11:50
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