Skip to content

@W-22701996: [Android] Improve error handling at token refresh#4

Closed
JohnsonEricAtSalesforce wants to merge 23 commits into
devfrom
feature/w-22701996_android-improve-error-handling-at-token-refresh
Closed

@W-22701996: [Android] Improve error handling at token refresh#4
JohnsonEricAtSalesforce wants to merge 23 commits into
devfrom
feature/w-22701996_android-improve-error-handling-at-token-refresh

Conversation

@JohnsonEricAtSalesforce

Copy link
Copy Markdown
Owner

Summary

Surfaces specific token endpoint error types (user_blocked, user_blocked_retry, invalid_grant) to apps via broadcast intent extras during token refresh, and handles user_blocked_retry without logging out the user.

Work Item: W-22701996 — [Android] Improve error handling at token refresh

Problem

Previously, all token endpoint errors (HTTP 400/401/403) during refresh were treated identically — the SDK logged out the user and broadcast ACCESS_TOKEN_REVOKE_INTENT with no error details. For App Attestation, the server returns two distinct error types:

  • user_blocked (terminal — device/app failed integrity checks)
  • user_blocked_retry (retriable — attestation couldn't be verified but the refresh token is still valid)

Both incorrectly triggered logout. Apps had no way to distinguish error types to show appropriate UI.

Changes

  • OAuth2.java: Added USER_BLOCKED_ERROR and USER_BLOCKED_RETRY_ERROR constants. Added USER_BLOCKED to LogoutReason enum.
  • ClientManager.java: Added EXTRA_TOKEN_ERROR and EXTRA_TOKEN_ERROR_DESCRIPTION broadcast intent extra constants. Restructured getNewAuthToken() to catch OAuthFailedException and branch on error type — user_blocked_retry skips logout while terminal errors use the appropriate LogoutReason. All OAuth failures broadcast error details in intent extras. Modified refreshStaleToken() to rethrow OAuthFailedException instead of swallowing it.
  • AuthenticatorService.java: Added early check for user_blocked_retry to return an error bundle without redirecting to login, before the isRefreshTokenInvalid() check that would otherwise treat it as terminal.

Tests Added

  • AuthenticatorServiceTest.kt (new file, 3 tests):

    • user_blocked_retry → returns error bundle (no login redirect)
    • user_blocked → redirects to login
    • invalid_grant → redirects to login
  • ClientManagerMockTest.kt (3 new tests):

    • user_blocked → logout with USER_BLOCKED reason, extras in broadcast
    • user_blocked_retry → NO logout, null token returned, extras in broadcast
    • invalid_grant → logout with REFRESH_TOKEN_EXPIRED, extras in broadcast

How Tests Validate Requirements

Requirement Test Validation
1. Surface specific error type All 6 tests verify the error type propagates through the respective path
2. Include error + error_description in broadcast extras ClientManager tests assert EXTRA_TOKEN_ERROR and EXTRA_TOKEN_ERROR_DESCRIPTION on captured broadcast intents
3. user_blocked → logout with specific reason testGetNewAuthToken_UserBlocked verifies LogoutReason.USER_BLOCKED passed to logout()
4. user_blocked_retry → no logout, fail waiting requests testGetNewAuthToken_UserBlockedRetry verifies logout() never called, null token returned

Test plan

  • All existing ClientManagerMockTest tests pass (11 pre-existing + 3 new)
  • All AuthenticatorServiceTest tests pass (3 new)
  • Firebase Test Lab CI passes
  • No new lint warnings introduced
  • Codecov confirms line coverage for new code paths

…ce token error type and handle user_blocked_retry without logout)
…Review: Extract inline strings to constants)
…Review: Use static imports for error constants)
…Review: Add final modifier to local variables)
…Review: Restore Javadoc to ATTESTATION constant)
…Review: Add doc comments to broadcast extra constants)
…Review: Remove redundant assignment flagged by inspector)
…Review: Consolidate duplicate error bundle creation)
…Review: Hoist accounts/matchingAccount to method scope to eliminate duplicate lookup)
…Review: Guard invalidateToken against null after user_blocked_retry)
…uthenticatorService tests for error-specific token refresh behavior)
…Review: Rename EXTRA_TOKEN_ERROR_TYPE to EXTRA_TOKEN_ERROR)
…Review: Restore revoke API comment on logout call)
…Review: Extract test helper to reduce boilerplate in token error tests)
…Review: Remove unused imports in AuthenticatorServiceTest)
…Review: Extract test helper to reduce boilerplate in AuthenticatorServiceTest)
public static final String ACCESS_TOKEN_REFRESH_INTENT = "access_token_refeshed";
public static final String INSTANCE_URL_UPDATE_INTENT = "instance_url_updated";
/** Intent extra: the {@code error} value from the token endpoint response (e.g. "user_blocked", "invalid_grant"). */
public static final String EXTRA_TOKEN_ERROR = "token_error";

Copy link
Copy Markdown
Owner Author

Choose a reason for hiding this comment

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

Requirement #1 & #2: These constants surface the token endpoint's error and error_description fields in the broadcast intent extras, allowing apps to distinguish between failure modes (e.g. show "app blocked" vs "session expired").

ofe.getHttpStatusCode() + ")", ofe);
}
return null;
} catch (OAuthFailedException ofe) {

Copy link
Copy Markdown
Owner Author

Choose a reason for hiding this comment

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

Requirement #1 (enabler): Previously this method caught OAuthFailedException, logged, and returned null — discarding all error context. Now it rethrows so that getNewAuthToken() can inspect the specific error type and branch accordingly.

final String errorType = tokenError != null ? tokenError.error : null;
final String errorDesc = tokenError != null ? tokenError.errorDescription : null;

if (!USER_BLOCKED_RETRY_ERROR.equals(errorType)) {

Copy link
Copy Markdown
Owner Author

Choose a reason for hiding this comment

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

Requirement #4: user_blocked_retry is retriable — the server couldn't verify attestation but the refresh token is still valid. We skip logout here so the app can re-attest and retry. All other errors (including user_blocked) are terminal and trigger logout.

Requirement #3: user_blocked uses LogoutReason.USER_BLOCKED instead of REFRESH_TOKEN_EXPIRED so apps/analytics can distinguish "blocked by attestation" from "token expired".

SalesforceSDKLogger.i(TAG, "Token endpoint error: (Error: " + ofe.response.error + ", Status Code: " + ofe.httpStatusCode + ")", ofe);

// Terminal errors (except retriable attestation) redirect to login.
if (!USER_BLOCKED_RETRY_ERROR.equals(ofe.response.error) && ofe.isRefreshTokenInvalid()) {

Copy link
Copy Markdown
Owner Author

Choose a reason for hiding this comment

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

Requirement #4: For the AuthenticatorService path, user_blocked_retry must NOT redirect to login. Since isRefreshTokenInvalid() only checks HTTP status codes (400/401/403) and user_blocked_retry returns 400, we check the error type first. The combined condition ensures retriable errors return an error bundle while terminal errors still redirect to login.

@@ -236,6 +242,7 @@ public enum LogoutReason {
UNEXPECTED, // Unexpected error or crash
UNEXPECTED_RESPONSE, // Unexpected response from server
UNKNOWN, // Unknown

Copy link
Copy Markdown
Owner Author

Choose a reason for hiding this comment

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

Requirement #3: USER_BLOCKED provides a distinct logout reason for attestation failures, differentiating from REFRESH_TOKEN_EXPIRED. Available to SalesforceSDKManager.logout() callers and analytics/telemetry.

@github-actions

github-actions Bot commented Jun 4, 2026

Copy link
Copy Markdown
1 Warning
⚠️ libs/SalesforceSDK/src/com/salesforce/androidsdk/auth/AuthenticatorService.java#L58 - Do not place Android context classes in static fields (static reference to Authenticator which has field context pointing to Context); this is a memory leak

Generated by 🚫 Danger

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant