KM-13631 Fix logout when activating DIP for some users

[kp-diego-trevisan]

Summary

This fixes a loop where App and VPN authentication tokens were never cleared from the storage if they reached an expired state.

In those cases a new refresh attempt would happen, it would fail, log out the user, the tokens would remain in the storage and the loop would start again once they tried to activate DIP again.

This PR fixes/implements the following bugs/features

• Bug 1
• Bug 2
• Feature 1
• Feature 2
• Breaking changes

Test plan (required)

Code formatting

Closing issues

[claude]

Claude finished @kp-diego-trevisan's task —— View job

---

Code Review Complete ✅

Todo List:

• Read CLAUDE.md for repo-specific guidelines
• Analyze ClientErrorMapper.swift changes
• Review DefaultAccountProvider.swift changes
• Examine RefreshAPITokenUseCase.swift changes
• Review RefreshVpnTokenUseCase.swift changes
• Analyze NetworkRequestClient.swift changes
• Evaluate overall authentication flow and bug fix
• Check for code quality, security, and threading issues
• Provide comprehensive review feedback

---

📋 Review Summary

This PR effectively addresses the authentication token loop issue by implementing proper token cleanup on refresh failures. The changes are well-targeted and follow good defensive programming practices.

✅ Strengths

1. Robust Error Handling

• RefreshAPITokenUseCase.swift:27-29, 35-37, 49-52, 61-63: Excellent defensive approach - tokens are cleared on every failure scenario (network errors, no response, no data, save failures)
• RefreshVpnTokenUseCase.swift:26-29, 33-36, 49-52, 59-62: Consistent token clearing logic mirrors API token handling
• This prevents the exact loop described in the PR: expired tokens → refresh fails → tokens remain → user logs out → loop repeats

2. Enhanced Observability

• RefreshAPITokenUseCase.swift:4, 28, 35, 50, 61: Added comprehensive logging with PIALogger for each token clearing scenario
• RefreshVpnTokenUseCase.swift:4, 27, 34, 49, 60: Symmetric logging approach helps with debugging token-related issues
• Aligns with CLAUDE.md debugging requirements for VPN applications

3. Consistent Cleanup Strategy

• DefaultAccountProvider.swift:604-606: cleanDatabase() now properly clears both token types, ensuring complete logout cleanup
• RefreshAPITokenUseCase.swift:45: Fixed completion callback type from incorrect RefreshVpnTokenUseCaseType to correct RefreshAPITokenUseCaseType

4. Memory Safety

• RefreshAPITokenUseCase.swift:25, 32: Proper weak self usage prevents retain cycles
• RefreshVpnTokenUseCase.swift:24, 31: Consistent memory management approach

🔍 Code Quality Assessment

Threading ✅

• All completion callbacks are handled appropriately
• Async operations properly managed with weak self references
• No blocking operations on main thread detected

Security ✅

• Token clearing prevents credential persistence after failures
• Follows CLAUDE.md security guidelines: "Credentials → Keychain only"
• No hardcoded credentials or token logging (only error descriptions)
• Proper isolation between API and VPN token handling

Memory Management ✅

• Consistent use of weak self captures
• No retain cycles introduced
• Proper cleanup in error paths

Performance ✅

• No unnecessary operations added
• Logging is informational level (not debug spam)
• Token clearing is lightweight operation

🎯 Bug Fix Validation

The fix directly addresses the described issue:

1. Before: Expired tokens remained in storage after refresh failures → endless retry loops
2. After: All token refresh failures immediately clear stored tokens → breaks the loop

The comprehensive approach ensures tokens are cleared in all failure scenarios:

• Network errors
• HTTP error responses
• Missing response data
• Token parsing/saving failures
• Complete connection failures

🧪 Test Coverage Recommendations

Consider adding unit tests for:

• Token clearing behavior in each error scenario
• cleanDatabase() token cleanup verification
• Mock providers testing refresh failure paths

Tests should use PIA VPNTests (preferred per CLAUDE.md) or PIALibrary mock providers.

✨ Minor Improvements

Code Consistency

• RefreshAPITokenUseCase.swift:22: Removed unnecessary blank lines (good)
• RefreshVpnTokenUseCase.swift:22: Consistent formatting improvements

Error Handling

All error paths now include explanatory comments and logging - excellent for maintainability.

---

🎉 Overall Assessment

APPROVED ✅

This is a well-crafted fix that:

• ✅ Solves the specific DIP activation logout loop
• ✅ Follows VPN security best practices
• ✅ Maintains code quality standards
• ✅ Includes proper error handling and logging
• ✅ Uses defensive programming principles

The implementation is comprehensive without being over-engineered. Great work on identifying and fixing this authentication loop issue!