Skip to content

[local_auth] Convert to Pigeon #117912

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

Closed
3 tasks done
Tracked by #117844
stuartmorgan-g opened this issue Jan 3, 2023 · 1 comment · Fixed by flutter/packages#3974
Closed
3 tasks done
Tracked by #117844

[local_auth] Convert to Pigeon #117912

stuartmorgan-g opened this issue Jan 3, 2023 · 1 comment · Fixed by flutter/packages#3974
Assignees
Labels
c: contributor-productivity Team-specific productivity, code health, technical debt. p: local_auth Plugin for local authentification P2 Important issues not at the top of the work list package flutter/packages repository. See also p: labels.

Comments

@stuartmorgan-g
Copy link
Contributor

stuartmorgan-g commented Jan 3, 2023

Part of #117844

@stuartmorgan-g stuartmorgan-g added c: contributor-productivity Team-specific productivity, code health, technical debt. plugin p: local_auth Plugin for local authentification P2 Important issues not at the top of the work list labels Jan 3, 2023
stuartmorgan-g added a commit to flutter/plugins that referenced this issue Jan 24, 2023
Updates `local_auth_windows` to use Pigeon, and moves to a more platform-tailored and Dart-centric implementation (rather than keeping the previous cross-platform method channel interface that the current implementation was duplicated from):
- Eliminates `deviceSupportsBiometrics` from the platform interface, since it's always the same as `isDeviceSupported`, in favor of doing that mapping in Dart.
- Eliminates `getEnrolledBiometrics` from the platform interface, since it was the same implementation as `isDeviceSupported` just with a different return value, in favor of doing that logic in Dart.
- Moves throwing for the `biometricOnly` option to the Dart side, simplifying the native logic.

Related changes:
- Adds a significant amount of previously-missing Dart unit test coverage.
- Removes the `biometricOnly` UI from the example app, since it will always fail.

Part of flutter/flutter#117912
mauricioluz pushed a commit to mauricioluz/plugins that referenced this issue Jan 26, 2023
Updates `local_auth_windows` to use Pigeon, and moves to a more platform-tailored and Dart-centric implementation (rather than keeping the previous cross-platform method channel interface that the current implementation was duplicated from):
- Eliminates `deviceSupportsBiometrics` from the platform interface, since it's always the same as `isDeviceSupported`, in favor of doing that mapping in Dart.
- Eliminates `getEnrolledBiometrics` from the platform interface, since it was the same implementation as `isDeviceSupported` just with a different return value, in favor of doing that logic in Dart.
- Moves throwing for the `biometricOnly` option to the Dart side, simplifying the native logic.

Related changes:
- Adds a significant amount of previously-missing Dart unit test coverage.
- Removes the `biometricOnly` UI from the example app, since it will always fail.

Part of flutter/flutter#117912
engine-flutter-autoroll pushed a commit to engine-flutter-autoroll/packages that referenced this issue Feb 22, 2023
Updates `local_auth_windows` to use Pigeon, and moves to a more platform-tailored and Dart-centric implementation (rather than keeping the previous cross-platform method channel interface that the current implementation was duplicated from):
- Eliminates `deviceSupportsBiometrics` from the platform interface, since it's always the same as `isDeviceSupported`, in favor of doing that mapping in Dart.
- Eliminates `getEnrolledBiometrics` from the platform interface, since it was the same implementation as `isDeviceSupported` just with a different return value, in favor of doing that logic in Dart.
- Moves throwing for the `biometricOnly` option to the Dart side, simplifying the native logic.

Related changes:
- Adds a significant amount of previously-missing Dart unit test coverage.
- Removes the `biometricOnly` UI from the example app, since it will always fail.

Part of flutter/flutter#117912
auto-submit bot pushed a commit to flutter/packages that referenced this issue Apr 18, 2023
Updates the platform communication to use Pigeon. This includes some changes to the API boundary:
- Collected authentication options into an options object; this avoids having a lot of positional boolean arguments (since Pigeon doesn't currently support named arguments).
- Collected strings into a strings object, since having them as individual parameters would have been extremely messy.
- Changes the `authenticate` return from a bool+exceptions to an enum that encompasses all of the explicitly known failure modes. To avoid a breaking change for clients, the Dart code creates `PlatformException`s that match the old ones, but this will make it much easier to someday make a (cross-platform) breaking change to replace them with better errors per the best practices we have documented on the wiki. Using an enum rather than throwing errors avoids the need to do string matching when we want to eventually translate them into something other than `PlatformException`.

This removes all Java warnings from the baseline file; the remaining issues in code I wasn't already changing were easy enough to fix opportunistically. There are still XML-based warnings, but fixing those was well out of scope so I left them.

Part of flutter/flutter#117912
@stuartmorgan-g stuartmorgan-g self-assigned this May 12, 2023
auto-submit bot pushed a commit to flutter/packages that referenced this issue May 19, 2023
Updates the platform communication to use Pigeon. This includes some changes to the API boundary:
- Collected authentication options into an options object; this avoids having a lot of positional boolean arguments (since Pigeon doesn't currently support named arguments).
- Collected strings into a strings object, since having them as individual parameters would have been extremely messy.
- Changes the `authenticate` return from a bool+exceptions to an enum that encompasses all of the explicitly known failure modes. To avoid a breaking change for clients, the Dart code creates `PlatformException`s that match the old ones, but this will make it much easier to someday make a (cross-platform) breaking change to replace them with better errors per the best practices we have documented on the wiki. Using an enum rather than throwing errors avoids the need to do string matching when we want to eventually translate them into something other than `PlatformException`.

The Pigeon file, Dart implementation, and rewritten tests are all based very heavily on the recent Android migration: #3748

I noted several bugs that I noticed in the existing implementation during the migration. I intentionally didn't fix them so this isn't changing behavior at the same time that it's changing so much of the code and tests, but I marked them with TODO comments.

Fixes flutter/flutter#117912
@github-actions
Copy link

github-actions bot commented Jun 2, 2023

This thread has been automatically locked since there has not been any recent activity after it was closed. If you are still experiencing a similar issue, please open a new bug, including the output of flutter doctor -v and a minimal reproduction of the issue.

@github-actions github-actions bot locked as resolved and limited conversation to collaborators Jun 2, 2023
@flutter-triage-bot flutter-triage-bot bot added the package flutter/packages repository. See also p: labels. label Jul 5, 2023
nploi pushed a commit to nploi/packages that referenced this issue Jul 16, 2023
Updates the platform communication to use Pigeon. This includes some changes to the API boundary:
- Collected authentication options into an options object; this avoids having a lot of positional boolean arguments (since Pigeon doesn't currently support named arguments).
- Collected strings into a strings object, since having them as individual parameters would have been extremely messy.
- Changes the `authenticate` return from a bool+exceptions to an enum that encompasses all of the explicitly known failure modes. To avoid a breaking change for clients, the Dart code creates `PlatformException`s that match the old ones, but this will make it much easier to someday make a (cross-platform) breaking change to replace them with better errors per the best practices we have documented on the wiki. Using an enum rather than throwing errors avoids the need to do string matching when we want to eventually translate them into something other than `PlatformException`.

The Pigeon file, Dart implementation, and rewritten tests are all based very heavily on the recent Android migration: flutter#3748

I noted several bugs that I noticed in the existing implementation during the migration. I intentionally didn't fix them so this isn't changing behavior at the same time that it's changing so much of the code and tests, but I marked them with TODO comments.

Fixes flutter/flutter#117912
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
c: contributor-productivity Team-specific productivity, code health, technical debt. p: local_auth Plugin for local authentification P2 Important issues not at the top of the work list package flutter/packages repository. See also p: labels.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

1 participant