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

feat(app-check)!: update activate() to be able to choose android app attest provider. Support Play Integrity provider. Deprecate Safety Net provider #9646

Merged
merged 12 commits into from
Oct 25, 2022

Conversation

russellwheatley
Copy link
Member

@russellwheatley russellwheatley commented Sep 30, 2022

Description

Breaking change. I've changed the API so that the android Play Integrity provider is the default. There's now an AndroidProvider enum to select the specific provider you wish in the activate() API. API now looks like this:

// Use play integrity provider (default):
await FirebaseAppCheck.instance.activate();

// Use default debug provider:
await FirebaseAppCheck.instance.activate(
    androidProvider: AndroidProvider.debug,
);

AndroidProvider options:

  1. Safety net (This was default but it is deprecated now)
  2. Debug
  3. Play integrity (Recommended app attest provider)

Edit: I've updated so now Play Integrity is the default provider.

Here's a screenshot whilst using the new API. The "unverified: invalid requests" are from using the Play Integrity provider (we don't have an app in the Play Store so this was the next best thing 😓). I also tested the debug provider which you can see from the "verified requests":
Screenshot 2022-09-30 at 16 04 06

I've also updated the App Check example to use "flutterfire-e2e-tests" Firebase projects across all platforms,and updated this issue:
#9643

Related Issues

closes #9178

Checklist

Before you create this PR confirm that it meets all requirements listed below by checking the relevant checkboxes ([x]).
This will ensure a smooth and quick review process. Updating the pubspec.yaml and changelogs is not required.

  • I read the Contributor Guide and followed the process outlined there for submitting PRs.
  • My PR includes unit or integration tests for all changed/updated/fixed behaviors (See Contributor Guide).
  • All existing and new tests are passing.
  • I updated/added relevant documentation (doc comments with ///).
  • The analyzer (melos run analyze) does not report any problems on my PR.
  • I read and followed the Flutter Style Guide.
  • I signed the CLA.
  • I am willing to follow-up on review comments in a timely manner.

Breaking Change

Does your PR require plugin users to manually update their apps to accommodate your change?

  • Yes, this is a breaking change.
  • No, this is not a breaking change.

private final String TAG = "FLTAppCheckPlugin";

private final String debugProvider = "debug";
private final String safetyNetProvider = "safetyNet";
Copy link
Contributor

Choose a reason for hiding this comment

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

If it's deprecated, shouldn't we just remove it from this release? If a developer still wants to use it, they'll keep using the previous version?
So we can keep the previous API (debug or not) and just make breaking the type of provider?

Copy link
Member Author

Choose a reason for hiding this comment

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

Yeah, that might be an idea. Although, it's not removed completely for another 2 years though so I feel users might complain if they get stuck on an old version 🤔. I can go either way but might be worth flagging with the team.
Screenshot 2022-10-06 at 09 11 23

Copy link

Choose a reason for hiding this comment

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

Would be great to have this. Current recommandations from firebase communication team is to ship new apps with this and safety net is marked as deprecated. Doesn't make sense to not have it. If app check is available on flutter we must have PlayIntegrity. Well TBH i'll launch a brand new app soon and I'd like to have it. :D

@Lyokone
Copy link
Contributor

Lyokone commented Oct 11, 2022

Should also close #9050

@russellwheatley russellwheatley changed the title feat(app-check)!: update activate() to be able to choose android app attest provider. Now includes Play Integrity provider. feat(app-check)!: update activate() to be able to choose android app attest provider. Now includes Play Integrity provider. Deprecate Safety Net provider Oct 20, 2022
@russellwheatley russellwheatley changed the title feat(app-check)!: update activate() to be able to choose android app attest provider. Now includes Play Integrity provider. Deprecate Safety Net provider feat(app-check)!: update activate() to be able to choose android app attest provider. Support Play Integrity provider. Deprecate Safety Net provider Oct 20, 2022
@yevgeniaronov
Copy link

when can we expect this to be released?

@Lyokone Lyokone requested a review from Salakar October 24, 2022 06:34
Comment on lines 53 to 55
// TODO: Since App Check is a beta SDK it's not available in the Firebase Android BoM so we need to specify an exact version here.
implementation 'com.google.firebase:firebase-appcheck-safetynet:16.0.2'
implementation 'com.google.firebase:firebase-appcheck-debug:16.0.2'
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
// TODO: Since App Check is a beta SDK it's not available in the Firebase Android BoM so we need to specify an exact version here.
implementation 'com.google.firebase:firebase-appcheck-safetynet:16.0.2'
implementation 'com.google.firebase:firebase-appcheck-debug:16.0.2'
// TODO: Since App Check is a beta SDK it's not available in the Firebase Android BoM so we need to specify an exact version here.
implementation 'com.google.firebase:firebase-appcheck-safetynet:16.0.2'
implementation 'com.google.firebase:firebase-appcheck-debug:16.0.2'

Is this still needed with the TODO, are the versions in the BoM now - can address separate PR if need be, just checking :D

Copy link
Member Author

Choose a reason for hiding this comment

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

Looks like they're part of the BOM now as of 30.0.0. I'll remove them from PR.
Screenshot 2022-10-24 at 14 50 44

Copy link
Member Author

Choose a reason for hiding this comment

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

Compared to the proceeding BOM before 30.0.0:
Screenshot 2022-10-24 at 14 51 30

Copy link
Member Author

Choose a reason for hiding this comment

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

I guess they are still needed as build fails due to missing symbols from those packages: https://github.com/firebase/flutterfire/actions/runs/3313899595/jobs/5472489288

@mzkai
Copy link

mzkai commented Nov 11, 2022

Testing the new version of the app_check package using the Play Integrity API as default, we could see that quite a few more devices would be blocked than if we used the Safety Net provider. This is probably a good thing in the longer run for security, but might lead to lots of angry users if the Play Integrity API verdicts are not also supported.

To make sure users get the right feedback, I think it would be important that the flutter app_check SDK allows for listening to the optional Integrity API device_recognition_verdit responses (https://developer.android.com/google/play/integrity/verdict):

  • MEETS_DEVICE_INTEGRITY
  • MEETS_BASIC_INTEGRITY
  • MEETS_STRONG_INTEGRITY

This will make it possible to provide users with the right information on why the app they have been using is suddenly not working anymore.

@firebase firebase locked and limited conversation to collaborators Nov 25, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[📚] [firebase_app_check] Play Integrity Support
6 participants