Skip to content

remove "singleTask" launch mode on Android 11+#1673

Merged
jlf0dev merged 3 commits into
masterfrom
bug/a11y-draw-over-build-mode
Dec 6, 2021
Merged

remove "singleTask" launch mode on Android 11+#1673
jlf0dev merged 3 commits into
masterfrom
bug/a11y-draw-over-build-mode

Conversation

@jlf0dev

@jlf0dev jlf0dev commented Dec 6, 2021

Copy link
Copy Markdown
Member

Type of change

  • Bug fix
  • New feature development
  • Tech debt (refactoring, code cleanup, dependency upgrades, etc)
  • Build/deploy pipeline (DevOps)
  • Other

Objective

This bug fix has multiple parts:

  1. Android 11 has introduced new lifecycle changes and fixes that break our Accessibility Service. When our app is open in the background, the Accessibility Service uses that instance and runs through OnNewIntent. Although OnNewIntent is not new, we haven't seen this behavior, thus we had no code yet in OnNewIntent to handle accessibility interactions.

  2. The second problem is that even though we now handle accessibility calls in OnNewIntent, the autofill page does not return our login information to our Accessibility Service. Our Accessibility Service uses a StartActivityForResult call that opens the main app and loads the autofill page. StartActivityForResult is incompatible with LaunchMode.SingleTask because it pulls the existing app to the front, that app is in a different Task. Tasks cannot communicate, meaning no data was being passed back to the Accessibility Service. We require LaunchMode.SingleTask to mitigate Strandhogg vulnerabilities in earlier versions of Android, but it can be removed in Android 11+.

tl;dr
The Android Accessibility Service on Android 11+ should now correctly show the autofill page and autofill credentials when selected.

Asana Task: https://app.asana.com/0/1169444489336079/1201373745150061/f
Closes #1645
Might impact #1389

Code changes

  • src/Android/MainActivity.cs: Remove [Activity] and [IntentFilter] attributes and declare manually in AndroidManifest.xml so we can use a resource string for LaunchMode. Handle Accessibility Service calls in OnNewIntent.
  • src/App/App.xaml.cs: Show Autofill Page if Accessibility Service comes through OnNewIntent.
  • src/Android/Properties/AndroidManifest.xml: Create <activity> and <intent-filter> manually so we can use a resource string for LaunchMode.
  • src/Android/Resources/values-v31/manifest.xml: Set LaunchMode to LaunchMode.Multiple if Android 11+.
  • src/Android/Resources/values/manifest.xml: Set LaunchMode to LaunchMode.SingleTask if Android 10-.
  • src/Android/Android.csproj: New files

Testing requirements

  • Android 10- Accessibility Service should still work correctly.
    • With/without app open in background.
  • Android 11+ Accessibility Service should now work correctly.
    • With/without app open in background.
  • Testing around the Android lifecycle to make sure the new LaunchMode in Android 11+ didn't make any unintentional changes.
    • Two Factor Authentication workflow.
    • Returning from application in background.
    • Autofill Service.
    • Main app launcher still works correctly.

Before you submit

  • I have added unit tests where it makes sense to do so (encouraged but not required)
  • This change requires a documentation update (notify the documentation team)
  • This change has particular deployment requirements (notify the DevOps team)

@jlf0dev jlf0dev marked this pull request as draft December 6, 2021 16:06
@jlf0dev jlf0dev marked this pull request as ready for review December 6, 2021 16:21
@jlf0dev jlf0dev requested review from fedemkr and mpbw2 December 6, 2021 16:22
Comment thread src/Android/MainActivity.cs Outdated
@"text/*"
})]
// Activity and IntentFilter declarations have been moved to Properties/AndroidManifest.xml
// They have been hardcoded so we can use the default LaunchMode on Android 12+

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Can you add a note about making sure any future changes are made in both manifest files? (I can see forgetting about the values-v31 version)

Comment thread src/Android/Android.csproj Outdated
<AndroidResource Include="Resources\values\styles.xml" />
<AndroidResource Include="Resources\values\colors.xml" />
<AndroidResource Include="Resources\values\manifest.xml">
<SubType></SubType>

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Remove the empty SubType and Generator tags for both manifests


<!-- Support for LG "Dual Window" mode (for Android < 7.0 users) -->
<meta-data android:name="com.lge.support.SPLIT_WINDOW" android:value="true" />
<activity android:name="com.x8bit.bitwarden.MainActivity" android:configChanges="keyboard|keyboardHidden|navigation|orientation|screenSize|uiMode" android:exported="true" android:icon="@mipmap/ic_launcher" android:label="Bitwarden" android:launchMode="@integer/launchModeAPIlevel" android:theme="@style/LaunchTheme">

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Formatting - can you break this line up

Comment thread src/Android/MainActivity.cs Outdated
Comment on lines +145 to +146
var uri = intent?.GetStringExtra("uri");
if (uri != null)

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

I think we could make it one line (not sure if there is an standard that is against this):

if(intent?.GetStringExtra("uri") is string uri)
{
    ....
}

@jlf0dev jlf0dev changed the title remove "singleTask" launch mode on Android 12+ remove "singleTask" launch mode on Android 11+ Dec 6, 2021
@jlf0dev jlf0dev merged commit 05bcc10 into master Dec 6, 2021
@jlf0dev jlf0dev deleted the bug/a11y-draw-over-build-mode branch December 6, 2021 19:17
fedemkr added a commit that referenced this pull request Dec 9, 2021
* master:
  Bumped version to 2.15.0 (#1676)
  Autosync the updated translations (#1661)
  move splash screen logic to OnResignActivation (#1674)
  remove "singleTask" launch mode on Android 11+ (#1673)
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.

Android 12 change in lifecycle autofill bug with Draw Over/Accessibility Quick Tile

3 participants