Skip to content

Auth-flow observations: refresh_token/expires_in not decoded, 1-year hard-coded expiration #1

@pimfeltkamp

Description

@pimfeltkamp

Discovered during a fresh-eyes audit of the auth flow. Three observations on the same code path, flagging them in case they're unintentional. Happy to close as wontfix if the design is deliberate (e.g. a Cryptohopper-OAuth quirk I'm not aware of).

1. HopperAPIAuthenticationResponse only decodes access_token

HopperAPIAuthenticationResponse.swift:11-17:

class HopperAPIAuthenticationResponse: Codable {
    var accessToken: String?
    private enum CodingKeys: String, CodingKey {
        case accessToken = "access_token"
    }
}

The OAuth2 /oauth2/token response normally contains access_token, refresh_token, expires_in, and token_type. This struct silently drops refresh_token and expires_in. That makes the rest of the refresh-token plumbing in the SDK structurally unable to work for the SDK's own auth flow (it works for injectSession(...) callers who pass tokens externally).

2. handleAuthResponse stores accessToken in both fields

HopperAPISessionManager.swift:80:

self.session = HopperAPISession(
    accessToken: response.accessToken ?? "",
    refreshToken: response.accessToken ?? "",  // ← refreshToken set to the access token
    accessTokenExpiresAt: nextDate ?? Date()
)

Combined with #1, this is forced — the response struct doesn't have a refreshToken field to use. But the result is that updateRefreshTokenIfNeeded() on HopperAPISessionManager.swift:64-71 calls HopperAPIRefreshTokenRequest(refreshToken:) with the access token — likely returning 401 from the server.

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

HopperAPISessionManager.swift:74-80:

var dayComponent = DateComponents()
dayComponent.year = 1
let theCalendar = Calendar.current
let nextDate = theCalendar.date(byAdding: dayComponent, to: Date())
// …
accessTokenExpiresAt: nextDate ?? Date()

The server-provided expires_in (typically 3600s) isn't decoded (#1), so the SDK can't honour it. The 1-year fallback means updateRefreshTokenIfNeeded() won't even 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 with no recovery path.

Combined effect on a typical user flow

  1. User logs in via CryptohopperAuth.login(...).
  2. SDK gets a real access token (valid ~1 hour) but stores it 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.

This is consistent with what some users on the Cryptohopper community forum have reported as "logged out after a while" issues — though I haven't dug to confirm those are the same root cause.

Bonus typo

CryptohopperAuth.swift:143:

public static func loginWithAutCode(authCode: String) {

Aut should be Auth. Renaming would be a public-API breaking change, so probably best left until the next major version bump.

What I'd suggest (in priority order)

  1. Add refresh_token and expires_in to HopperAPIAuthenticationResponse so the data is actually captured.
  2. Have handleAuthResponse use those fields:
    self.session = HopperAPISession(
        accessToken: response.accessToken ?? "",
        refreshToken: response.refreshToken ?? "",
        accessTokenExpiresAt: Date(timeIntervalSinceNow: TimeInterval(response.expiresIn ?? 3600) - 60)
    )
    The -60 gives a 60-second safety margin so the SDK refreshes just before expiration, not exactly at it.
  3. Keep the 1-year fallback only for the case where expires_in is missing.
  4. Backwards-compat note: existing apps that depend on the broken 1-year behaviour will start refreshing tokens correctly after the fix. That's a behaviour change but should be invisible if the rest of the refresh flow works.

I deliberately didn't open a PR — this is a 5-year-old SDK with real users, and the maintainer should decide whether the design is intentional before any code lands. Happy to follow up with a PR if you confirm the fix direction.

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions