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

Fix preferImmediatelyAvailableCredentials usage in iOS package #83

Merged
merged 1 commit into from
Jan 10, 2025

Conversation

ice-tychon
Copy link
Contributor

What does this PR do?

This PR fixes issue with preferImmediatelyAvailableCredentials.

  • if we pass non-empty AuthenticateRequestType.allowCredentials and AuthenticateRequestType.preferImmediatelyAvailableCredentials: false to the PasskeyAuthenticator().authenticate, but current device doesn't have any of the allowCredentials the bevior:
    • current: we get "no credentials found" exception
    • expected: the system should show a dialog with "other account" (like Chrome) and other device (QR code)

ice-tychon added a commit to ice-blockchain/ion-app that referenced this pull request Dec 13, 2024
## Description
This PR adds a fix for iOS to be able to use another device to login.

## Additional Notes
Changes in the passkeys_ios:
corbado/flutter-passkeys#83

## Type of Change
- [x] Bug fix
- [ ] New feature
- [ ] Breaking change
- [ ] Refactoring
- [ ] Documentation
- [ ] Chore

## Screenshots
<img width="300" alt="image"
src="[image_url_here](https://github.com/user-attachments/assets/103e26f1-870e-49d7-b7ab-3b33e077fce5)">
@Dopeamin
Copy link
Collaborator

Hey Thank you for pointing out the issue. After some trial from my side! It seems that iOS's ASAuthorizationController.performRequests(options:) is treating the presence of the .preferImmediatelyAvailableCredentials option as an indication to enable the behavior, regardless of the value passed. As such i like the approach you went with but i would also add a comment there so it explains to anyone checking the code why you had to do this check .

@Dopeamin
Copy link
Collaborator

Dopeamin commented Jan 10, 2025

I will be merging this PR , also i opened a new one where i added a comment to better explain the reasoning here . Thank you again for suggestion.

@Dopeamin Dopeamin closed this Jan 10, 2025
@Dopeamin Dopeamin reopened this Jan 10, 2025
@Dopeamin Dopeamin merged commit 2fffd84 into corbado:main Jan 10, 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