Skip to content
This repository has been archived by the owner on Jun 20, 2023. It is now read-only.

New consent Google API (EXPOSUREAPP-4635) #2325

Merged
merged 63 commits into from
Feb 15, 2021

Conversation

mtwalli
Copy link
Contributor

@mtwalli mtwalli commented Feb 11, 2021

Google play services version that is required by this PR: 18210214000

  • Trigger Google consent Dialog when needed to share keys
  • Fallback if unrecoverable error happened is to use older Api for getting the keys
  • Old consent boolean flag in the App is not affected by this change regardless of user selection 😒
  • Ticket is addressing the consent only before scanning QR-Code, while I show when it is (needed and possible) in other places such as warn others.
  • In case of disallowance, this can be revoked from Google settings -> switch on/off

Note:

requestPreAuthorizedTemporaryExposureKeyHistory and requestPreAuthorizedTemporaryExposureKeyRelease
are used always in combination. i.e:
1- requestPreAuthorizedTemporaryExposureKeyRelease can not succeed unless requestPreAuthorizedTemporaryExposureKeyHistory is called before.
2- requestPreAuthorizedTemporaryExposureKeyHistory is triggering the consent dialog once as long as requestPreAuthorizedTemporaryExposureKeyRelease was not called at least once in the next 5 days.

Testing: test the submission flow as before. This change require Google play services Update. Old versions should still work and use older Apis

Important notes:

1- Resetting the App -> does not revoke the consent
2- Consent is completely independent from Test results (positive - pending - negative) -> therefore if you saw it once that is OK (you will be asked when ever this possible)

TAN scenarios:

1- if you were about to use QR code scan and you saw the consent dialog and took an action that is enough for any other case that requires consent such as TAN -> we can only show the consent once using the new Api
2- if you have not seen the consent before ,you should see it when following TAN process , later after you enter it and proceed to warn others

Some scenarios:

Case 1: Happy path user allows the consent and proceed to sharing the keys.
https://user-images.githubusercontent.com/25054729/107620898-b8993380-6c55-11eb-9447-b1927f3796a1.mp4

Case 2: User dis-allow form the consent ,we proceed anyway to the keys sharing using the old api
https://user-images.githubusercontent.com/25054729/107625156-32342000-6c5c-11eb-88e6-e0dba01f40db.mp4

@harambasicluka
Copy link
Contributor

harambasicluka commented Feb 12, 2021

Did some on device testing, everything worked as expected:

Xiaomi Mi 9: Android 9; ENF 18210214000

Pixel 3: Android 11; ENF 18205016000

  • Old ENF version didn't try to use the new consent dialog, the old one is still there:
pixel3_old_enf_version.mp4

@ralfgehrer
Copy link
Contributor

On a Pixel (Android 10) with Play service 18210214000
I tested all edge cases with QR codes. Even scanning a positive code, resetting the app, scanning a positive test, ... worked as expected.

Afterward, I used the Tele Tan, reset the app, and was about to use the QR Code: before scanning the code, I was not prompted to share keys BEFORE scanning the code - as I would have expected. Is this a bug or a wrong assumption of mine?

@mtwalli
Copy link
Contributor Author

mtwalli commented Feb 12, 2021

@ralfgehrer , Thanks, could you please record a video of the scenario that you think it is problematic? then I can investigate it properly

@ralfgehrer ralfgehrer self-assigned this Feb 12, 2021
@ralfgehrer
Copy link
Contributor

@ralfgehrer , Thanks, could you please record a video of the scenario that you think it is problematic? then I can investigate it properly

I tried to reproduce the bug again and record a video, but was not able to. Maybe I did not pay super close attention earlier in the last run. But on the bright side, all seems to work very nicely. Ran about 10 more tests ;)

ralfgehrer
ralfgehrer previously approved these changes Feb 12, 2021
ralfgehrer
ralfgehrer previously approved these changes Feb 12, 2021
Copy link
Contributor

@ralfgehrer ralfgehrer left a comment

Choose a reason for hiding this comment

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

Thanks for the extra set of tests!

@jurajkusnier jurajkusnier self-assigned this Feb 15, 2021
Copy link
Contributor

@jurajkusnier jurajkusnier left a comment

Choose a reason for hiding this comment

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

LGTM

@sonarcloud
Copy link

sonarcloud bot commented Feb 15, 2021

Kudos, SonarCloud Quality Gate passed!

Bug A 0 Bugs
Vulnerability A 0 Vulnerabilities
Security Hotspot A 0 Security Hotspots
Code Smell A 0 Code Smells

73.1% 73.1% Coverage
0.0% 0.0% Duplication

@mtwalli mtwalli merged commit 97f376a into release/1.13.x Feb 15, 2021
@mtwalli mtwalli deleted the feature/4635-new-constent-google-api branch February 15, 2021 14:39
@mar-v-in
Copy link

According to the first message in the PR, this feature requires Google play services version 18,210,214,000
The latest Google play services version according to apkmirror.com is 210,214,000 released in Februar 2021.

So I guess you're not referring to Google play services version but something else?

@mtwalli
Copy link
Contributor Author

mtwalli commented Feb 22, 2021

According to the first message in the PR, this feature requires Google play services version 18,210,214,000
The latest Google play services version according to apkmirror.com is 210,214,000 released in Februar 2021.

So I guess you're not referring to Google play services version but something else?

To be precise : Exposure Notifications Version, You can check that by going to:
Device Settings -> Google Settings -> COVID-19 Exposure Notifications -> Scroll to the bottom

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
enhancement Improvement of an existing feature maintainers Tag pull requests created by maintainers prio PRs to review first.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants