Skip to content

fix: rename devnet and reduce scope for Google Drive #1418

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

Merged
HashEngineering merged 4 commits into from
Aug 6, 2025
Merged

fix: rename devnet and reduce scope for Google Drive #1418

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
dc6d3b6
refactor: move schnapps to devnet (flavor)
HashEngineering Jul 25, 2025
5c19ba9
fix: use DRIVE_FILES scope
HashEngineering Jul 26, 2025
122540e
fix: don't cache google tokens, ask everytime
HashEngineering Jul 29, 2025
23717a9
Revert "fix: don't cache google tokens, ask everytime"
HashEngineering Jul 29, 2025
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
2 changes: 1 addition & 1 deletion wallet/.gitignore
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,4 +4,4 @@ res/drawable-*/stat_notify_received.png
prod/release
_testNet3/release
staging/release
schnapps/release
devnet/release
11 changes: 8 additions & 3 deletions wallet/build.gradle
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -293,7 +293,7 @@ android {
res.srcDirs = ["staging/res"]
}
devnet {
res.srcDirs = ["schnapps/res"]
res.srcDirs = ["devnet/res"]
}
}
flavorDimensions "default"
Expand Down Expand Up @@ -363,7 +363,7 @@ android {
buildConfigField("String", "ZENLEDGER_CLIENT_SECRET", "\"\"")
}
devnet {
applicationId = "org.dash.dashpay.schnapps"
applicationId = "org.dash.dashpay.devnet"
def imgurClientId = props.getProperty("UPHOLD_CLIENT_ID_SANDBOX")
def imgurClientSecret = props.getProperty("UPHOLD_CLIENT_SECRET_SANDBOX")
if (imgurClientId == null) {
Expand All @@ -372,9 +372,14 @@ android {
if (imgurClientSecret == null) {
imgurClientSecret = "\"UPHOLD_CLIENT_SECRET\""
}
applicationId = "org.dash.wallet.devnet"
buildConfigField("String", "UPHOLD_CLIENT_ID", "\"UPHOLD_CLIENT_ID\"")
buildConfigField("String", "UPHOLD_CLIENT_SECRET", "\"UPHOLD_CLIENT_SECRET\"")
def topperKeyId = props.getProperty("TOPPER_KEY_ID_SANDBOX", "\"TOPPER_KEY_ID\"")
def topperWidgetId = props.getProperty("TOPPER_WIDGET_ID_SANDBOX", "\"TOPPER_WIDGET_ID\"")
def topperPrivateKey = props.getProperty("TOPPER_PRIVATE_KEY_SANDBOX", "\"TOPPER_PRIVATE_KEY\"")
buildConfigField("String", "TOPPER_KEY_ID", topperKeyId)
buildConfigField("String", "TOPPER_WIDGET_ID", topperWidgetId)
buildConfigField("String", "TOPPER_PRIVATE_KEY", topperPrivateKey)
buildConfigField("String", "ZENLEDGER_CLIENT_ID", "\"\"")
Comment on lines +377 to 383

@coderabbitai coderabbitai Bot Aug 5, 2025 •
edited
Loading

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.

🛠️ Refactor suggestion

Secrets exposure & property fallback consistency

Great to see sandbox-specific TOPPER_* fields added. Two minor points:

  1. These keys end up in the generated BuildConfig.java. Even for devnet, avoid putting real secrets in plain text; double-check that service.properties is git-ignored and that CI injects them via secure variables.

  2. Immediately above (Lines 367-376) the flavour fetches sandbox Uphold credentials into imgurClientId/Secret, but then discards them and hard-codes "UPHOLD_CLIENT_ID" / "UPHOLD_CLIENT_SECRET". That looks like a stale copy-paste.
    If you actually want the sandbox values, wire them into buildConfigField similarly to what you just did for the TOPPER_* keys.

-    buildConfigField("String", "UPHOLD_CLIENT_ID", "\"UPHOLD_CLIENT_ID\"")
-    buildConfigField("String", "UPHOLD_CLIENT_SECRET", "\"UPHOLD_CLIENT_SECRET\"")
+    buildConfigField("String", "UPHOLD_CLIENT_ID", imgurClientId)
+    buildConfigField("String", "UPHOLD_CLIENT_SECRET", imgurClientSecret)

Committable suggestion skipped: line range outside the PR's diff.

🤖 Prompt for AI Agents 
In wallet/build.gradle around lines 377 to 383, avoid exposing real secrets in
BuildConfig by ensuring service.properties is git-ignored and secrets are
injected securely in CI. Also, fix the stale hard-coded Uphold sandbox
credentials above these lines by replacing the fixed strings with properties
fetched from service.properties, similar to how TOPPER_* sandbox keys are
handled, to maintain consistency and proper secret management.

@HashEngineering HashEngineering Aug 5, 2025

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

What is the alternative? How can I avoid plain text secrets in BuildConfig? @coderabbitai

@coderabbitai coderabbitai Bot Aug 5, 2025

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.

@HashEngineering Great question! Here are several alternatives to avoid plain text secrets in BuildConfig:

1. Encrypted SharedPreferences + Runtime Injection

Store secrets encrypted and decrypt at runtime:

// Instead of BuildConfig.TOPPER_KEY_ID
class SecretManager @Inject constructor(
    @ApplicationContext private val context: Context
) {
    fun getTopperKeyId(): String {
        return EncryptedSharedPreferences.create(...)
            .getString("topper_key_id", "") ?: ""
    }
}

2. Android Keystore + Server Injection

Most secure approach - fetch secrets from your backend at runtime:

// App requests secrets from your server using device/user authentication
class ApiKeyRepository {
    suspend fun getTopperCredentials(): TopperCredentials {
        return api.getCredentials(deviceId, userToken)
    }
}

3. ProGuard Obfuscation (Minimal Protection)

Better than nothing, but still reversible:

buildTypes {
    release {
        minifyEnabled true
        // Obfuscates field names but values still visible
    }
}

4. Native Library Storage

Store in C++ with some obfuscation:

// secrets.cpp
extern "C" JNIEXPORT jstring JNICALL
Java_com_yourapp_SecretManager_getTopperKey(JNIEnv *env, jobject) {
    // Still not 100% secure but harder to extract
    return env->NewStringUTF(decrypt("encrypted_key_here"));
}

5. Hybrid Approach (Recommended)

  • Development/Staging: Use BuildConfig for convenience
  • Production: Runtime injection from secure server
  • Sandbox keys: Can stay in BuildConfig (less sensitive)
class ConfigManager {
    fun getTopperKeyId(): String {
        return if (BuildConfig.DEBUG) {
            BuildConfig.TOPPER_KEY_ID // Dev convenience
        } else {
            secureStorage.getTopperKey() // Production security
        }
    }
}

Reality check: Client-side apps inherently expose some data. Focus on:

  • Keep production secrets server-side when possible
  • Use sandbox/test keys in builds
  • Implement key rotation
  • Monitor for API key abuse

For your devnet flavor, using sandbox keys in BuildConfig is probably acceptable since they're not production credentials.

🧠 Learnings used 
Learnt from: HashEngineering
PR: dashpay/dash-wallet#1410
File: wallet/src/de/schildbach/wallet/data/InvitationLinkData.kt:79-84
Timestamp: 2025-07-12T07:12:04.769Z
Learning: HashEngineering prefers to handle defensive validation improvements for URI parsing in follow-up PRs rather than including them in the current PR when the main focus is on replacing Firebase with AppsFlyer.

@HashEngineering HashEngineering Aug 6, 2025

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

Thank you. This will be considered in a future story.

@coderabbitai coderabbitai Bot Aug 6, 2025

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.

@HashEngineering You're welcome! That makes perfect sense - keeping this PR focused on the devnet renaming and Google Drive scope changes is the right approach. The security improvements can definitely be addressed in a dedicated story where they can get proper attention and testing.

Thanks for considering the suggestions for future implementation!

🧠 Learnings used 
Learnt from: HashEngineering
PR: dashpay/dash-wallet#1410
File: wallet/src/de/schildbach/wallet/data/InvitationLinkData.kt:79-84
Timestamp: 2025-07-12T07:12:04.769Z
Learning: HashEngineering prefers to handle defensive validation improvements for URI parsing in follow-up PRs rather than including them in the current PR when the main focus is on replacing Firebase with AppsFlyer.

buildConfigField("String", "ZENLEDGER_CLIENT_SECRET", "\"\"")
}
Expand Down
0 ...s/drawable-xhdpi/currency_symbol_dash.png → ...s/drawable-xhdpi/currency_symbol_dash.png
View file
Open in desktop
File renamed without changes
0 .../drawable-xhdpi/currency_symbol_mdash.png → .../drawable-xhdpi/currency_symbol_mdash.png
View file
Open in desktop
File renamed without changes
0 .../drawable-xhdpi/currency_symbol_udash.png → .../drawable-xhdpi/currency_symbol_udash.png
View file
Open in desktop
File renamed without changes
0 .../schnapps/res/mipmap-hdpi/ic_launcher.png → ...et/devnet/res/mipmap-hdpi/ic_launcher.png
View file
Open in desktop
File renamed without changes
0 .../schnapps/res/mipmap-mdpi/ic_launcher.png → ...et/devnet/res/mipmap-mdpi/ic_launcher.png
View file
Open in desktop
File renamed without changes
0 ...schnapps/res/mipmap-xhdpi/ic_launcher.png → ...t/devnet/res/mipmap-xhdpi/ic_launcher.png
View file
Open in desktop
File renamed without changes
0 ...chnapps/res/mipmap-xxhdpi/ic_launcher.png → .../devnet/res/mipmap-xxhdpi/ic_launcher.png
View file
Open in desktop
File renamed without changes
0 ...hnapps/res/mipmap-xxxhdpi/ic_launcher.png → ...devnet/res/mipmap-xxxhdpi/ic_launcher.png
View file
Open in desktop
File renamed without changes
0 ...pps/res/values/ic_launcher_background.xml → ...net/res/values/ic_launcher_background.xml
View file
Open in desktop
File renamed without changes.
0 wallet/schnapps/res/values/strings-extra.xml → wallet/devnet/res/values/strings-extra.xml
View file
Open in desktop
File renamed without changes.
0 wallet/schnapps/res/values/values.xml → wallet/devnet/res/values/values.xml
View file
Open in desktop
File renamed without changes.
1 change: 0 additions & 1 deletion wallet/src/de/schildbach/wallet/Constants.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -125,7 +125,6 @@ public final class Constants {
break;
}
case "devnet": {
// Schnapps Devnet
BIP44_PATH = DeterministicKeyChain.BIP44_ACCOUNT_ZERO_PATH_TESTNET;
NETWORK_PARAMETERS = OuzoDevNetParams.get();
String devNetName = ((DevNetParams)NETWORK_PARAMETERS).getDevNetName();
Expand Down
2 changes: 1 addition & 1 deletion wallet/src/de/schildbach/wallet/payments/RequestWalletBalanceTask.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -340,7 +340,7 @@ boolean requestWalletBalanceFromBlockExplorers(Address address) {
blockExplorers.push(new Pair<>("https://insight.dash.org/insight-api/addr/", UnspentAPI.Insight));
} else if (BuildConfig.FLAVOR.equals("_testNet3") || BuildConfig.FLAVOR.equals("staging")) {
blockExplorers.push(new Pair<>("https://insight.testnet.networks.dash.org:3002/insight-api/addr/", UnspentAPI.Insight));
} else if (BuildConfig.FLAVOR.equals("schnapps")) {
} else if (BuildConfig.FLAVOR.equals("devnet")) {
blockExplorers.push(new Pair<>(String.format("http://insight.%s.networks.dash.org:3002/insight-api/addr/", Constants.NETWORK_PARAMETERS.getDevNetName().substring("devnet-".length())), UnspentAPI.Insight));
}

Expand Down
8 changes: 2 additions & 6 deletions wallet/src/de/schildbach/wallet/ui/EditProfileActivity.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -46,7 +46,6 @@ import androidx.lifecycle.lifecycleScope
import com.amulyakhare.textdrawable.TextDrawable
import com.bumptech.glide.Glide
import com.bumptech.glide.signature.ObjectKey
import com.google.android.gms.auth.api.identity.AuthorizationRequest
import com.google.android.gms.auth.api.identity.AuthorizationResult
import com.google.android.gms.auth.api.identity.Identity
import com.google.android.gms.common.api.Scope
Expand All @@ -57,6 +56,7 @@ import de.schildbach.wallet.Constants
import de.schildbach.wallet.database.entity.DashPayProfile
import de.schildbach.wallet.livedata.Status
import de.schildbach.wallet.ui.dashpay.*
import de.schildbach.wallet.ui.dashpay.utils.GoogleDriveService
import de.schildbach.wallet.ui.dashpay.utils.display
import de.schildbach.wallet.ui.dashpay.work.UpdateProfileError
import de.schildbach.wallet.ui.send.SendCoinsActivity
Expand Down Expand Up @@ -337,11 +337,7 @@ class EditProfileActivity : LockScreenActivity() {
}

private fun authorizeGoogleDrive() {
val authorizationRequest = AuthorizationRequest
.builder()
.setRequestedScopes(
listOf(Scope(DriveScopes.DRIVE))
).build()
val authorizationRequest = editProfileViewModel.getGoogleDriveAuthRequest()

Identity.getAuthorizationClient(this@EditProfileActivity)
.authorize(authorizationRequest)
Expand Down
5 changes: 5 additions & 0 deletions wallet/src/de/schildbach/wallet/ui/dashpay/EditProfileViewModel.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@ import android.os.Environment
import android.provider.Settings
import androidx.lifecycle.MutableLiveData
import androidx.lifecycle.viewModelScope
import com.google.android.gms.auth.api.identity.AuthorizationRequest
import com.google.api.client.googleapis.auth.oauth2.GoogleCredential
import com.google.api.client.http.HttpRequestInitializer
import dagger.hilt.android.lifecycle.HiltViewModel
Expand Down Expand Up @@ -284,4 +285,8 @@ class EditProfileViewModel @Inject constructor(
suspend fun hasEnoughCredits(): CreditBalanceInfo {
return platformRepo.getIdentityBalance()
}

fun getGoogleDriveAuthRequest(): AuthorizationRequest {
return googleDriveService.getAuthRequest()
}
}
9 changes: 9 additions & 0 deletions wallet/src/de/schildbach/wallet/ui/dashpay/utils/GoogleDriveService.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,11 +1,14 @@
package de.schildbach.wallet.ui.dashpay.utils

import android.content.Context
import com.google.android.gms.auth.api.identity.AuthorizationRequest
import com.google.android.gms.common.api.Scope
import com.google.api.client.http.ByteArrayContent
import com.google.api.client.http.HttpRequestInitializer
import com.google.api.client.http.javanet.NetHttpTransport
import com.google.api.client.json.gson.GsonFactory
import com.google.api.services.drive.Drive
import com.google.api.services.drive.DriveScopes
import com.google.api.services.drive.model.File
import com.google.api.services.drive.model.Permission
import dagger.hilt.android.qualifiers.ApplicationContext
Expand Down Expand Up @@ -91,4 +94,10 @@ class GoogleDriveService(
}
}
}

fun getAuthRequest() = AuthorizationRequest
.builder()
.setRequestedScopes(
listOf(Scope(DriveScopes.DRIVE_FILE))
).build()
Comment on lines +97 to +102

@HashEngineering HashEngineering Aug 5, 2025

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

Scopes reduced from DRIVE (all functions) to DRIVE_FILES (only files created by this app)

}