Auth-flow observations: same refresh-token bugs as iOS SDK, plus .gitignore typo and dupe-key on session removal

[pimfeltkamp]

Discovered during a fresh-eyes audit, mirror of cryptohopper-ios-sdk#1. The Android SDK shares the iOS SDK's auth-flow bug chain (likely because they're sister-ports of the same design) and adds two Android-specific issues. Flagging in case any of these are unintentional.

1. HopperAPIAuthenticationResponse only decodes access_token

HopperAPIAuthenticationResponse.kt:

data class HopperAPIAuthenticationResponse (
        @SerializedName("access_token") val accessToken: String?
)

The OAuth2 /oauth2/token response normally carries access_token, refresh_token, expires_in, and token_type. This data class silently discards refresh_token and expires_in, so the rest of the refresh-token plumbing in the SDK has no real data to work with for the SDK's own auth flow.

2. Setter and handleAuthResponse both store the access token under the RefreshToken key

The setter at HopperAPISessionManager.kt:38-42:

this?.putString("$clientId#AccessToken", newValue.accessToken)
this?.putString("$clientId#RefreshToken", newValue.accessToken)  // ← stores accessToken in RefreshToken key

The setter ignores newValue.refreshToken entirely. So even if a future fix to #1 starts decoding refresh_token, the setter still silently drops it.

handleAuthResponse at HopperAPISessionManager.kt:90-94:

this.session = HopperAPISession(
    accessToken = response.accessToken ?: "",
    refreshToken = response.accessToken ?: "",  // ← same bug
    accessTokenExpiresAt = nextYear
)

updateRefreshTokenIfNeeded() at line 71-87 then calls HopperAPIRefreshTokenRequest(refreshToken) with the access token — likely 401-ing from the server.

3. Expiration hard-coded to 1 year regardless of server response

HopperAPISessionManager.kt:88-89:

val today = Date()
val nextYear = Date(today.time + (1000 * 60 * 60 * 24 * 365))

The server's expires_in (typically 3600s) isn't decoded (#1), so the SDK can't honour it. The 1-year fallback means updateRefreshTokenIfNeeded() won't try to refresh until 1 year after first login — by which time the actual access token has long since expired and every API call returns 401.

Combined effect on a typical user flow

1. User logs in via CryptohopperAuth.login(...) or loginWithCode(...).
2. SDK stores the real access token with a 1-year fake expiration.
3. After ~1 hour the access token expires server-side. Every API call returns 401.
4. updateRefreshTokenIfNeeded() looks at the fake 1-year expiration, sees currentDate < refreshDate, and does nothing.
5. User is stuck until they manually re-authenticate.

4. .gitignore typo: secret.properties (singular) vs the actual secrets.properties (plural)

.gitignore line for secret:

secret.properties
/secret.properties

The actual file is secrets.properties (plural). The repo currently ships an empty template (all values are \"\") so nothing leaks today. But if a contributor ever fills in real API_KEY/API_PASSWORD/etc. into secrets.properties for local testing, those credentials will be committed because the gitignore protects the wrong filename.

Suggested fix: make the gitignore match the actual filename.

-secret.properties
-/secret.properties
+secrets.properties
+/secrets.properties

(And reset the template to a stub like secrets.properties.example so the build instructions are explicit about what to copy.)

5. Session-removal block has a duplicated key, RefreshToken never cleared

HopperAPISessionManager.kt:43-47:

} else {
    this?.remove("$clientId#AccessToken")
    this?.remove("$clientId#AccessTokenExpiresAt")
    this?.remove("$clientId#AccessTokenExpiresAt")  // ← duplicate; should be RefreshToken
}

When clearing a session (logout), RefreshToken is never removed from SharedPreferences. Practical impact today is limited (it gets overwritten on next login because of #2), but it's a residue-on-logout issue and the duplicate line is clearly a copy-paste typo.

What I'd suggest (in priority order)

1. Decode the missing fields:

   data class HopperAPIAuthenticationResponse (
       @SerializedName(\"access_token\") val accessToken: String?,
       @SerializedName(\"refresh_token\") val refreshToken: String?,
       @SerializedName(\"expires_in\") val expiresIn: Long?,
   )

2. Fix the setter and handleAuthResponse to use refreshToken and expiresIn:

   this?.putString(\"$clientId#RefreshToken\", newValue.refreshToken)
   // ...
   val expiresAt = response.expiresIn?.let { Date(System.currentTimeMillis() + it * 1000 - 60_000) } ?: nextYear

   (-60_000 gives a 60-second safety margin so the SDK refreshes just before expiration.)
3. Fix the .gitignore typo.
4. Fix the duplicate-key removal: replace one of the AccessTokenExpiresAt removals with RefreshToken.

I deliberately didn't open a PR — same reasoning as the iOS issue: 5-year-old SDK with real users, the maintainer should sign off on the design before any code lands. Happy to follow up with a PR once you confirm direction.

Cross-reference

• iOS sister bugs: cryptohopper-ios-sdk#1