Skip to content

Audit of classes in firebase app distribution to minimize user access #3196

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

Merged
merged 19 commits into from
Dec 10, 2021
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
Original file line number Diff line number Diff line change
Expand Up @@ -27,77 +27,77 @@
* href="https://github.com/google/auto/tree/master/value">https://github.com/google/auto/tree/master/value</a>
*/
@AutoValue
public abstract class AppDistributionReleaseInternal {
abstract class AppDistributionReleaseInternal {

@NonNull
public static Builder builder() {
static Builder builder() {
return new AutoValue_AppDistributionReleaseInternal.Builder();
}

/** The short bundle version of this build (example 1.0.0) */
@NonNull
public abstract String getDisplayVersion();
abstract String getDisplayVersion();

/** The bundle version of this build (example: 123) */
@NonNull
public abstract String getBuildVersion();
abstract String getBuildVersion();

/** The release notes for this build */
@Nullable
public abstract String getReleaseNotes();
abstract String getReleaseNotes();

/** The binary type for this build */
@NonNull
public abstract BinaryType getBinaryType();
abstract BinaryType getBinaryType();

/** Hash of binary of an Android app */
@Nullable
public abstract String getCodeHash();
abstract String getCodeHash();

/** Efficient hash of an Android apk. Used to identify a release */
@Nullable
public abstract String getApkHash();
abstract String getApkHash();

/**
* IAS artifact id. This value is inserted into the manifest of APK's installed via Used to map a
* release to an APK installed via an app bundle
*/
@Nullable
public abstract String getIasArtifactId();
abstract String getIasArtifactId();

/** Short-lived download URL */
@Nullable
public abstract String getDownloadUrl();
abstract String getDownloadUrl();

/** Builder for {@link AppDistributionReleaseInternal}. */
@AutoValue.Builder
public abstract static class Builder {
abstract static class Builder {

@NonNull
public abstract Builder setDisplayVersion(@NonNull String value);
abstract Builder setDisplayVersion(@NonNull String value);

@NonNull
public abstract Builder setBuildVersion(@NonNull String value);
abstract Builder setBuildVersion(@NonNull String value);

@NonNull
public abstract Builder setReleaseNotes(@Nullable String value);
abstract Builder setReleaseNotes(@Nullable String value);

@NonNull
public abstract Builder setBinaryType(@NonNull BinaryType value);
abstract Builder setBinaryType(@NonNull BinaryType value);

@NonNull
public abstract Builder setCodeHash(@NonNull String value);
abstract Builder setCodeHash(@NonNull String value);

@NonNull
public abstract Builder setApkHash(@NonNull String value);
abstract Builder setApkHash(@NonNull String value);

@NonNull
public abstract Builder setIasArtifactId(@NonNull String value);
abstract Builder setIasArtifactId(@NonNull String value);

@NonNull
public abstract Builder setDownloadUrl(@NonNull String value);
abstract Builder setDownloadUrl(@NonNull String value);

@NonNull
public abstract AppDistributionReleaseInternal build();
abstract AppDistributionReleaseInternal build();
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,8 @@
import java.io.File;

/**
* Activity opened during installation in {@link UpdateApkClient} after APK download is finished.
* Activity opened during installation in {@link FirebaseAppDistribution} after APK download is
* finished.
*/
public class InstallActivity extends AppCompatActivity {
private static final String TAG = "InstallActivity: ";
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@
import androidx.annotation.NonNull;

/** Wrapper that handles Android logcat logging. */
public class LogWrapper {
class LogWrapper {

private static final String LOG_TAG = "FirebaseAppDistribution";
private static LogWrapper instance;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -29,12 +29,12 @@
import java.util.zip.ZipEntry;
import java.util.zip.ZipFile;

public final class ReleaseIdentificationUtils {
final class ReleaseIdentificationUtils {
private static final String TAG = "ReleaseIdentification";
private static final int BYTES_IN_LONG = 8;

Choose a reason for hiding this comment

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

I think the methods in this class should also be package-private

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Good call. I thought since the class is package-private it was fine but its better to be consistent.

@Nullable
public static String extractInternalAppSharingArtifactId(@NonNull Context appContext) {
static String extractInternalAppSharingArtifactId(@NonNull Context appContext) {
try {
PackageInfo packageInfo =
appContext
Expand All @@ -51,7 +51,7 @@ public static String extractInternalAppSharingArtifactId(@NonNull Context appCon
}

@Nullable
public static String calculateApkHash(@NonNull File file) {
static String calculateApkHash(@NonNull File file) {
LogWrapper.getInstance().v(TAG + "Calculating release id for " + file.getPath());
LogWrapper.getInstance().v(TAG + "File size: " + file.length());

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,9 @@
import java.util.concurrent.Executors;
import javax.net.ssl.HttpsURLConnection;

/** Client class that handles updateApp functionality for AABs in {@link UpdateAppClient}. */
/**
* Client class that handles updateApp functionality for AABs in {@link FirebaseAppDistribution}.
*/
class UpdateAabClient {
private static final String TAG = "UpdateAabClient:";

Expand Down Expand Up @@ -68,7 +70,7 @@ void onActivityStarted(Activity activity) {
this.tryCancelAabUpdateTask();
}

public UpdateTaskImpl updateAab(@NonNull AppDistributionReleaseInternal newRelease) {
UpdateTaskImpl updateAab(@NonNull AppDistributionReleaseInternal newRelease) {
synchronized (updateAabLock) {
if (cachedUpdateTask != null && !cachedUpdateTask.isComplete()) {
return cachedUpdateTask;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,9 @@
import java.util.jar.JarFile;
import javax.net.ssl.HttpsURLConnection;

/** Client class that handles updateApp functionality for APKs in {@link UpdateAppClient}. */
/**
* Client class that handles updateApp functionality for APKs in {@link FirebaseAppDistribution}.
*/
class UpdateApkClient {
private static final int UPDATE_INTERVAL_MS = 250;
private static final String TAG = "UpdateApkClient:";
Expand Down Expand Up @@ -68,7 +70,7 @@ public UpdateApkClient(
this.installApkClient = installApkClient;
}

public UpdateTaskImpl updateApk(
UpdateTaskImpl updateApk(
@NonNull AppDistributionReleaseInternal newRelease, boolean showDownloadNotificationManager) {
synchronized (updateTaskLock) {
if (cachedUpdateTask != null && !cachedUpdateTask.isComplete()) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -22,9 +22,7 @@

import android.app.Activity;
import android.net.Uri;
import androidx.test.core.app.ApplicationProvider;
import com.google.firebase.FirebaseApp;
import com.google.firebase.FirebaseOptions;
import java.io.ByteArrayInputStream;
import java.io.IOException;
import java.util.ArrayList;
Expand All @@ -45,10 +43,6 @@

@RunWith(RobolectricTestRunner.class)
public class UpdateAabClientTest {

private static final String TEST_API_KEY = "AIzaSyabcdefghijklmnopqrstuvwxyz1234567";
private static final String TEST_APP_ID_1 = "1:123456789:android:abcdef";
private static final String TEST_PROJECT_ID = "777777777777";
private static final String TEST_URL = "https://test-url";
private static final String REDIRECT_TO_PLAY = "https://redirect-to-play-url";
private static final Executor testExecutor = Executors.newSingleThreadExecutor();
Expand Down Expand Up @@ -76,15 +70,6 @@ public void setup() {

FirebaseApp.clearInstancesForTest();

FirebaseApp firebaseApp =
FirebaseApp.initializeApp(
ApplicationProvider.getApplicationContext(),
new FirebaseOptions.Builder()
.setApplicationId(TEST_APP_ID_1)
.setProjectId(TEST_PROJECT_ID)
.setApiKey(TEST_API_KEY)
.build());

activity =
Robolectric.buildActivity(FirebaseAppDistributionTest.TestActivity.class).create().get();
shadowActivity = shadowOf(activity);
Expand Down