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.

Sign up for GitHub

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

Android 14 update #2894

Merged
merged 17 commits into from
Dec 10, 2024
Merged

Android 14 update #2894

Show file tree
Hide file tree
Changes from 12 commits
Commits
Show all changes
17 commits
Select commit Hold shift + click to select a range
241b9a8
Bump compile and target SDK to version 34
avazirna Sep 4, 2024
b93b72e
Bump AGP version to version 8.1.1
avazirna Oct 29, 2024
000fef6
Set foreground service type
avazirna Nov 14, 2024
e8879bc
Set exported attribute to receivers
avazirna Nov 14, 2024
5f83b76
Merge branch 'master' into android-14-update
avazirna Nov 14, 2024
88760e3
Set exported attribute to dynamic broadcast receivers
avazirna Nov 15, 2024
569476d
Migrate from Google Play core
avazirna Nov 15, 2024
124f9f1
Migrate to configuration avoindance APIs
avazirna Nov 27, 2024
65a8b2a
Remove package attribute from manifest files
avazirna Nov 27, 2024
476f0dd
Revert "Set foreground service type"
avazirna Nov 28, 2024
ecee832
Set CommCareSessionService foreground service type
avazirna Nov 28, 2024
ef25be8
Fix pendingIntent mutability flag
avazirna Nov 28, 2024
641857f
update test libs and work manager
shubham1g5 Nov 29, 2024
0e86fb6
Bump Support Library compile SDK to 35 and target SDK to 34
avazirna Dec 2, 2024
38a4dc9
Update broacast receivers exported attribute
avazirna Dec 9, 2024
26eb7c3
Merge pull request #2905 from dimagi/standardLibsUpdate
avazirna Dec 9, 2024
86ef226
Downgrade Espresso to 3.4
avazirna Dec 9, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
15 changes: 10 additions & 5 deletions app/AndroidManifest.xml
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,6 +1,5 @@
<?xml version="1.0" encoding="utf-8"?>
<manifest package="org.commcare.dalvik"
xmlns:android="http://schemas.android.com/apk/res/android"
<manifest xmlns:android="http://schemas.android.com/apk/res/android"
xmlns:tools="http://schemas.android.com/tools"
android:versionCode="106"
android:versionName="2.55">
Expand Down Expand Up @@ -38,6 +37,7 @@
<uses-permission android:name="android.permission.NEARBY_WIFI_DEVICES"
android:usesPermissionFlags="neverForLocation"/>
<uses-permission android:name="android.permission.POST_NOTIFICATIONS" />
<uses-permission android:name="android.permission.FOREGROUND_SERVICE_SPECIAL_USE" />

<uses-feature
android:name="android.hardware.telephony"
Expand Down Expand Up @@ -304,12 +304,14 @@
android:name="org.commcare.activities.FormRecordListActivity"
android:windowSoftInputMode="adjustResize">
</activity>

<service
android:enabled="true"
android:foregroundServiceType="specialUse"
android:name="org.commcare.services.CommCareSessionService">
<property
android:name="android.app.PROPERTY_SPECIAL_USE_FGS_SUBTYPE"
android:value="Service that maintains an encryption key in memory to securely submit data to the server" />
</service>

<activity
android:launchMode="singleTop"
android:name="org.commcare.activities.FormEntryActivity"
Expand Down Expand Up @@ -372,7 +374,9 @@
<activity android:name="org.commcare.activities.MessageActivity">
</activity>

<receiver android:name="org.commcare.views.notifications.NotificationClearReceiver">
<receiver
android:exported="false"
android:name="org.commcare.views.notifications.NotificationClearReceiver">
Comment on lines +377 to +379
Copy link
Contributor

Choose a reason for hiding this comment

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

we should either add a qa note to test clearning of notifications or test it out ourselves and mention it in safety story of the PR

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I tested this and it worked fine, I will add a note to the PR.

</receiver>

<activity
Expand Down Expand Up @@ -560,6 +564,7 @@
Below code removes this receiver's entry from Manifest.
-->
<receiver android:name="io.ona.kujaku.receivers.KujakuNetworkChangeReceiver"
android:exported="false"
tools:node="remove"/>

<!-- The zebra-print-android library is not yet targeting Android 12 and at least one
Expand Down
15 changes: 8 additions & 7 deletions app/build.gradle
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -124,7 +124,7 @@ dependencies {
implementation 'androidx.work:work-runtime:2.7.1'
implementation 'androidx.work:work-runtime-ktx:2.7.1'

implementation 'com.google.android.play:core:1.10.3'
implementation 'com.google.android.play:app-update:2.1.0'
implementation 'android.arch.lifecycle:common-java8:1.1.1'


Expand Down Expand Up @@ -184,7 +184,7 @@ afterEvaluate {
* https://discuss.gradle.org/t/gradle-7-0-seems-to-take-an-overzealous-approach-to-inter-task-dependencies/39656/2
* Gradle 7.0 treats any copy tasks as having an implicit dependencies with each other.
*/
task injectPropertiesIntoFirebaseConfigFile {
tasks.register('injectPropertiesIntoFirebaseConfigFile') {
description = 'Injects properties into the google-services.json file at runtime'

copy {
Expand Down Expand Up @@ -222,7 +222,7 @@ static def getDate() {

android {
namespace 'org.commcare.dalvik'
compileSdk 33
compileSdk 34

lintOptions {
abortOnError false
Expand All @@ -249,7 +249,8 @@ android {

defaultConfig {
minSdkVersion 21
targetSdkVersion 33
targetSdkVersion 34

applicationId 'org.commcare.dalvik'
testNamespace 'org.commcare.dalvik.test'

Expand Down Expand Up @@ -491,7 +492,7 @@ android {
* Download and unpack commcare app associated with 'cc_app_id' into assets
* folder
*/
task downloadCCApp(type: Exec)
tasks.register('downloadCCApp', Exec)

// task configuration phase
downloadCCApp {
Expand Down Expand Up @@ -521,7 +522,7 @@ downloadCCApp {
}


task downloadRestoreFile(type: Exec)
tasks.register('downloadRestoreFile', Exec)

// task configuration phase
downloadRestoreFile {
Expand Down Expand Up @@ -551,7 +552,7 @@ downloadRestoreFile {
}

// dynamically inject commcare app download into standalone build process
tasks.whenTaskAdded { task ->
tasks.configureEach { task ->
if ((task.name == 'processStandaloneDebugResources' ||
task.name == 'processStandaloneReleaseResources') && 'true' == runDownloadScripts) {
task.dependsOn downloadCCApp
Expand Down
10 changes: 9 additions & 1 deletion app/src/org/commcare/activities/CommCareActivity.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@
import android.content.DialogInterface;
import android.content.IntentFilter;
import android.graphics.Rect;
import android.os.Build;
import android.os.Bundle;
import android.text.Spannable;
import android.util.DisplayMetrics;
Expand Down Expand Up @@ -302,7 +303,14 @@ protected void onResume() {

if (shouldListenToSyncComplete() && isBackgroundSyncEnabled()) {
dataSyncCompleteBroadcastReceiver = new DataSyncCompleteBroadcastReceiver();
registerReceiver(dataSyncCompleteBroadcastReceiver, new IntentFilter(COMMCARE_DATA_UPDATE_ACTION));
if (Build.VERSION.SDK_INT >= Build.VERSION_CODES.O) {
registerReceiver(
dataSyncCompleteBroadcastReceiver,
new IntentFilter(COMMCARE_DATA_UPDATE_ACTION), Context.RECEIVER_NOT_EXPORTED);
} else {
registerReceiver(
dataSyncCompleteBroadcastReceiver, new IntentFilter(COMMCARE_DATA_UPDATE_ACTION));
}
}
}

Expand Down
12 changes: 10 additions & 2 deletions app/src/org/commcare/activities/FormEntryActivity.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -297,7 +297,11 @@ public void onReceive(Context context, Intent intent) {
IntentFilter filter = new IntentFilter();
filter.addAction(PollSensorAction.XPATH_ERROR_ACTION);
filter.addAction(GeoUtils.ACTION_LOCATION_ERROR);
registerReceiver(mLocationServiceIssueReceiver, filter);
if (Build.VERSION.SDK_INT >= Build.VERSION_CODES.O) {
registerReceiver(mLocationServiceIssueReceiver, filter, Context.RECEIVER_NOT_EXPORTED);
} else {
registerReceiver(mLocationServiceIssueReceiver, filter);
}
}

@Override
Expand Down Expand Up @@ -997,7 +1001,11 @@ public void onResumeSessionSafe() {
reportVideoUsageIfAny();

IntentFilter intentFilter = new IntentFilter(PENGING_SYNC_ALERT_ACTION);
registerReceiver(pendingSyncAlertBroadcastReceiver, intentFilter);
if (Build.VERSION.SDK_INT >= Build.VERSION_CODES.O) {
registerReceiver(pendingSyncAlertBroadcastReceiver, intentFilter, Context.RECEIVER_NOT_EXPORTED);
} else {
registerReceiver(pendingSyncAlertBroadcastReceiver, intentFilter);
}

// Flag that a background sync shouldn't be triggered when this activity is in the foreground
CommCareApplication.instance().setBackgroundSyncSafe(false);
Expand Down
10 changes: 5 additions & 5 deletions app/src/org/commcare/services/CommCareFirebaseMessagingService.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -92,11 +92,11 @@ private void showNotification(RemoteMessage.Notification notification) {
i.setAction(Intent.ACTION_MAIN);
i.addCategory(Intent.CATEGORY_LAUNCHER);

PendingIntent contentIntent;
if (Build.VERSION.SDK_INT >= Build.VERSION_CODES.M)
contentIntent = PendingIntent.getActivity(this, 0, i, PendingIntent.FLAG_IMMUTABLE);
else
contentIntent = PendingIntent.getActivity(this, 0, i, 0);
int pendingIntentFlags = PendingIntent.FLAG_UPDATE_CURRENT;
if (Build.VERSION.SDK_INT >= Build.VERSION_CODES.M) {
pendingIntentFlags = pendingIntentFlags | PendingIntent.FLAG_IMMUTABLE;
shubham1g5 marked this conversation as resolved.
Show resolved Hide resolved
}
PendingIntent contentIntent = PendingIntent.getActivity(this, 0, i, pendingIntentFlags);

NotificationCompat.Builder fcmNotification = new NotificationCompat.Builder(this,
CommCareNoficationManager.NOTIFICATION_CHANNEL_PUSH_NOTIFICATIONS_ID)
Expand Down
19 changes: 11 additions & 8 deletions app/src/org/commcare/services/CommCareSessionService.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@
import android.app.Service;
import android.content.Intent;
import android.content.SharedPreferences;
import android.content.pm.ServiceInfo;
import android.os.Binder;
import android.os.Build;
import android.os.IBinder;
Expand Down Expand Up @@ -208,11 +209,14 @@ public IBinder onBind(Intent intent) {
/**
* Show a notification while this service is running.
*/
@SuppressLint("UnspecifiedImmutableFlag")
public void showLoggedInNotification(@Nullable User user) {
// Send the notification. This will cause error messages if CommCare doesn't have
// permission to post notifications
this.startForeground(NOTIFICATION, createSessionNotification());
if (Build.VERSION.SDK_INT >= Build.VERSION_CODES.UPSIDE_DOWN_CAKE) {
this.startForeground(NOTIFICATION, createSessionNotification(), ServiceInfo.FOREGROUND_SERVICE_TYPE_SPECIAL_USE);
} else {
this.startForeground(NOTIFICATION, createSessionNotification());
}
}

/**
Expand Down Expand Up @@ -702,12 +706,11 @@ private Notification createSessionNotification(){
callable.setAction("android.intent.action.MAIN");
callable.addCategory("android.intent.category.LAUNCHER");

// The PendingIntent to launch our activity if the user selects this notification
PendingIntent contentIntent;
if (Build.VERSION.SDK_INT >= Build.VERSION_CODES.M)
contentIntent = PendingIntent.getActivity(this, 0, callable, PendingIntent.FLAG_IMMUTABLE);
else
contentIntent = PendingIntent.getActivity(this, 0, callable, 0);
int pendingIntentFlags = PendingIntent.FLAG_UPDATE_CURRENT;
if (Build.VERSION.SDK_INT >= Build.VERSION_CODES.M) {
pendingIntentFlags = pendingIntentFlags | PendingIntent.FLAG_IMMUTABLE;
}
PendingIntent contentIntent = PendingIntent.getActivity(this, 0, callable, pendingIntentFlags);

String notificationText;
if (AppUtils.getInstalledAppRecords().size() > 1) {
Expand Down
8 changes: 7 additions & 1 deletion app/src/org/commcare/utils/SessionRegistrationHelper.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,8 @@
import android.content.Intent;
import android.content.IntentFilter;
import androidx.appcompat.app.AppCompatActivity;

import android.os.Build;
import android.util.Log;

import org.commcare.activities.DispatchActivity;
Expand Down Expand Up @@ -52,7 +54,11 @@ public static boolean handleSessionExpiration(AppCompatActivity activity) {
}

public static void registerSessionExpirationReceiver(AppCompatActivity activity) {
activity.registerReceiver(userSessionExpiredReceiver, expirationFilter);
if (Build.VERSION.SDK_INT >= Build.VERSION_CODES.O) {
activity.registerReceiver(userSessionExpiredReceiver, expirationFilter, Context.RECEIVER_NOT_EXPORTED);
} else {
activity.registerReceiver(userSessionExpiredReceiver, expirationFilter);
}
Copy link

@coderabbitai coderabbitai bot Dec 9, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

💡 Codebase verification

Broadcast receiver security improvements needed in several files

The verification revealed additional broadcast receiver registrations that need security improvements:

  • CommCareProviderLocationController.kt: Missing SDK version check and RECEIVER_NOT_EXPORTED flag for provider location changes
  • CommCareWiFiDirectActivity.java: Missing SDK version check and RECEIVER_NOT_EXPORTED flag for WiFi Direct receiver
  • Test files can be excluded as they use LocalBroadcastManager which is already secure
🔗 Analysis chain

LGTM! Secure broadcast receiver registration implemented correctly.

The conditional registration of the session expiration receiver with RECEIVER_NOT_EXPORTED flag for Android Oreo and above follows security best practices.

Let's verify that all broadcast receiver registrations in the codebase follow this pattern:

🏁 Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Description: Check for potentially insecure broadcast receiver registrations
# Look for registerReceiver calls that might need the RECEIVER_NOT_EXPORTED flag

# Search for registerReceiver calls without RECEIVER_NOT_EXPORTED
rg "registerReceiver\([^,]+,[^,]+" -A 2

Length of output: 2983

Copy link
Contributor

Choose a reason for hiding this comment

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

@avazirna can you verify this please.

Copy link

Choose a reason for hiding this comment

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

Seems like the humans are having a chat. I'll hop back into my burrow for now. If you need me again, just tag @coderabbitai in a new comment, and I'll come hopping out!

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I pushed a fix in commit 38a4dc9, please review it.

}

/**
Expand Down
2 changes: 1 addition & 1 deletion build.gradle
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@ buildscript {
}

dependencies {
classpath 'com.android.tools.build:gradle:8.0.1'
classpath 'com.android.tools.build:gradle:8.1.1'
classpath 'com.google.gms:google-services:4.3.14'
classpath 'com.google.firebase:firebase-crashlytics-gradle:2.9.2'
classpath "org.jetbrains.kotlin:kotlin-gradle-plugin:$kotlin_version"
Expand Down
2 changes: 1 addition & 1 deletion commcare-support-library/src/main/AndroidManifest.xml
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,2 +1,2 @@
<?xml version="1.0" encoding="utf-8"?>
<manifest package="org.commcare.commcaresupportlibrary" />
Copy link
Contributor

Choose a reason for hiding this comment

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

Think we should be bumping the Android sdk version in support lib build.gradle as well to go along with this change.

<manifest />