[webview_flutter_android] Set support for web authentication#11681
[webview_flutter_android] Set support for web authentication#11681creatorpiyush wants to merge 6 commits into
Conversation
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request implements Web Authentication support for the Android WebView, including the necessary Pigeon interface updates, native implementation, and example app integration. Feedback includes addressing a potential compatibility issue with Math.toIntExact on older Android APIs, renaming variables in documentation for clarity, and potentially using an enum to expose all support levels instead of a boolean toggle.
| @SuppressLint("RequiresFeature") | ||
| @Override | ||
| public void setWebAuthenticationSupport(@NonNull WebSettings webSettings, long support) { | ||
| final int supportValue = Math.toIntExact(support); |
There was a problem hiding this comment.
Math.toIntExact was introduced in Android API level 24. Since this package supports API level 21, this call may cause a NoSuchMethodError on devices running API 21-23 unless Java 8 desugaring is guaranteed to be enabled and cover this specific method. Consider using a manual cast with an overflow check for broader compatibility, especially since the unit tests explicitly verify overflow behavior.
final int supportValue = (int) support;
if (supportValue != support) {
throw new ArithmeticException("integer overflow");
}There was a problem hiding this comment.
minSdk in build.gradle.kts is 24
| Future<void> setWebAuthenticationEnabled(bool enabled) async { | ||
| if (!await isWebViewFeatureSupported( | ||
| WebViewFeatureType.webAuthentication, | ||
| )) { | ||
| return; | ||
| } | ||
|
|
||
| final int supportLevel = enabled | ||
| ? WebAuthenticationSupportConstants.forApp | ||
| : WebAuthenticationSupportConstants.none; | ||
|
|
||
| await android_webview.WebSettingsCompat.setWebAuthenticationSupport( | ||
| _webView.settings, | ||
| supportLevel, | ||
| ); | ||
| } |
There was a problem hiding this comment.
The setWebAuthenticationEnabled(bool enabled) method simplifies the API to a boolean toggle, which only covers the none and forApp support levels. Since the underlying Android API and the constants already defined in android_webkit_constants.dart also support forBrowser (intended for browser-like applications), consider using an enum to expose the full range of functionality provided by the platform.
creatorpiyush
force-pushed
the
webview_flutter_android_set-Web-Authentication-Support
branch
from
9a2593d to
886d8da
Compare
Thanks for the contribution! You’ve checked boxes in the PR checklist above that are not reflected in this PR, so I’m assuming this is a work in progress and am marking it as a Draft. Please review the checklist, updating the PR as appropriate, and when the state of the PR as posted reflects the checklist please feel free to mark it as ready for review. |
|
Hello @stuartmorgan-g, PR is up for review; that part was missed to be pushed. |
There was a problem hiding this comment.
Code Review
This pull request introduces support for configuring Web Authentication (WebAuthn) in the AndroidWebViewController via the setWebAuthenticationSupport method. The changes include updates to the Pigeon interface, Android native implementation using WebSettingsCompat, and the Dart API with a new WebAuthenticationSupport enum. A high-severity issue was identified regarding the use of Math.toIntExact, which requires API level 24 and would cause crashes on devices running Android 5.0 to 7.0 (API 21-23) supported by this plugin. Additionally, the WebAuthenticationSupportConstants class should use the @Deprecated annotation instead of just a doc comment for proper IDE and compiler support.
| /** | ||
| * This method should only be called if {@link WebViewFeatureProxyApi#isFeatureSupported(String)} | ||
| * with WEB_AUTHENTICATION returns true. | ||
| * | ||
| * <p>The {@code support} parameter is a {@code long} to accommodate Dart's integer type, but is | ||
| * safely converted to {@code int} for the underlying Android API call. {@link | ||
| * Math#toIntExact(long)} is used to verify the value fits in the {@code int} range and throw | ||
| * {@link ArithmeticException} if it overflows. This is safe because the valid support levels are | ||
| * constants (0, 1, 2) that well within the integer range. | ||
| * | ||
| * <p>Note: {@link Math#toIntExact(long)} requires API level 24 or higher. This is compatible with | ||
| * this plugin's minimum SDK version. | ||
| * | ||
| * @param webSettings the WebSettings instance | ||
| * @param support the WebAuthentication support level (0, 1, or 2) | ||
| * @throws ArithmeticException if {@code support} exceeds {@link Integer#MAX_VALUE} | ||
| */ | ||
| @SuppressLint("RequiresFeature") | ||
| @Override | ||
| public void setWebAuthenticationSupport(@NonNull WebSettings webSettings, long support) { | ||
| final int supportValue = Math.toIntExact(support); | ||
| WebSettingsCompat.setWebAuthenticationSupport(webSettings, supportValue); | ||
| } |
There was a problem hiding this comment.
The use of Math.toIntExact requires Android API level 24 or higher. Since webview_flutter_android supports API levels as low as 21, using this method will result in a NoSuchMethodError on devices running Android 5.0 to 7.0 (API 21-23) unless core library desugaring is enabled.
Given that the support values are expected to be small constants (0, 1, or 2) defined by the plugin, a simple cast to int is safer for compatibility. If overflow protection is desired, a manual check can be used to maintain compatibility with all supported API levels.
/**
* This method should only be called if {@link WebViewFeatureProxyApi#isFeatureSupported(String)}
* with WEB_AUTHENTICATION returns true.
*
* <p>The {@code support} parameter is a {@code long} to accommodate Dart's integer type, but is
* safely converted to {@code int} for the underlying Android API call. This is safe because the
* valid support levels are constants (0, 1, 2) that are well within the integer range.
*
* @param webSettings the WebSettings instance
* @param support the WebAuthentication support level (0, 1, or 2)
*/
@SuppressLint("RequiresFeature")
@Override
public void setWebAuthenticationSupport(@NonNull WebSettings webSettings, long support) {
if (support < Integer.MIN_VALUE || support > Integer.MAX_VALUE) {
throw new ArithmeticException("integer overflow");
}
final int supportValue = (int) support;
WebSettingsCompat.setWebAuthenticationSupport(webSettings, supportValue);
}There was a problem hiding this comment.
minSdk in build.gradle.kts is 24, so I guess this comment is not valid for the current solution
mboetger
requested review from
a team and
gmackall
and removed request for
a team
2 participants
This PR adds Android support for configuring WebView web authentication behavior through WebSettingsCompat.setWebAuthenticationSupport.
working-example.mp4
List which issues are fixed by this PR. You must list at least one issue.
flutter/flutter#186312
Pre-Review Checklist
[shared_preferences]///).If you need help, consider asking for advice on the #hackers-new channel on Discord.
Note: The Flutter team is currently trialing the use of Gemini Code Assist for GitHub. Comments from the
gemini-code-assistbot should not be taken as authoritative feedback from the Flutter team. If you find its comments useful you can update your code accordingly, but if you are unsure or disagree with the feedback, please feel free to wait for a Flutter team member's review for guidance on which automated comments should be addressed.Footnotes
Regular contributors who have demonstrated familiarity with the repository guidelines only need to comment if the PR is not auto-exempted by repo tooling. ↩ ↩2