Skip to content

Commit

Permalink
Merge pull request #1194 from Infomaniak/401LogOut
Browse files Browse the repository at this point in the history 
fix: Logout on API token no longer availlable remotely
  • Loading branch information
@adrien-coye
adrien-coye authored Jun 7, 2024
2 parents ce9f354 + f4c7a8b commit 8b1fa9a
Show file tree
Hide file tree
Showing 5 changed files with 79 additions and 69 deletions.
8 changes: 0 additions & 8 deletions kDrive/AppDelegate.swift
View file
Original file line number Diff line number Diff line change
Expand Up @@ -460,14 +460,6 @@ final class AppDelegate: UIResponder, UIApplicationDelegate, AccountManagerDeleg
}
}

// MARK: - Account manager delegate

func currentAccountNeedsAuthentication() {
Task { @MainActor in
setRootViewController(SwitchUserViewController.instantiateInNavigationController())
}
}

// MARK: - State restoration

func application(_ application: UIApplication, shouldSaveApplicationState coder: NSCoder) -> Bool {
Expand Down
15 changes: 1 addition & 14 deletions kDrive/UI/Controller/Menu/MenuViewController.swift
View file
Original file line number Diff line number Diff line change
Expand Up @@ -286,20 +286,7 @@ extension MenuViewController {
.alertRemoveUserDescription(currentAccount?.user.displayName ?? ""),
action: KDriveResourcesStrings.Localizable.buttonConfirm,
destructive: true) {
if let currentAccount = self.accountManager.currentAccount {
self.accountManager.removeTokenAndAccount(account: currentAccount)
}

let appDelegate = (UIApplication.shared.delegate as? AppDelegate)

if let nextAccount = self.accountManager.accounts.first {
self.accountManager.switchAccount(newAccount: nextAccount)
appDelegate?.refreshCacheScanLibraryAndUpload(preload: true, isSwitching: true)
} else {
SentrySDK.setUser(nil)
}
self.accountManager.saveAccounts()
appDelegate?.updateRootViewControllerState()
self.accountManager.logoutCurrentAccountAndSwitchToNextIfPossible()
}
present(alert, animated: true)
case .help:
Expand Down
80 changes: 37 additions & 43 deletions kDriveCore/Data/Api/DriveApiFetcher.swift
View file
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@ import InfomaniakCore
import InfomaniakDI
import InfomaniakLogin
import Kingfisher
import Sentry
import UIKit

public extension ApiFetcher {
Expand Down Expand Up @@ -475,6 +476,37 @@ class SyncedAuthenticator: OAuthAuthenticator {
@LazyInjectService var appContextService: AppContextServiceable
@LazyInjectService var keychainHelper: KeychainHelper

func handleFailedRefreshingToken(oldToken: ApiToken, error: Error?) -> Result<OAuthAuthenticator.Credential, Error> {
guard let error else {
// Couldn't refresh the token, keep the old token and fetch it later. Maybe because of bad network ?
SentrySDK
.addBreadcrumb(oldToken.generateBreadcrumb(level: .error,
message: "Refreshing token failed - Other \(error.debugDescription)"))
return .failure(DriveError.unknownError)
}

if case .noRefreshToken = (error as? InfomaniakLoginError) {
// Couldn't refresh the token because we don't have a refresh token
SentrySDK
.addBreadcrumb(oldToken.generateBreadcrumb(level: .error,
message: "Refreshing token failed - Cannot refresh infinite token"))
refreshTokenDelegate?.didFailRefreshToken(oldToken)
return .failure(error)
}

if (error as NSError).domain == "invalid_grant" {
// Couldn't refresh the token, API says it's invalid
SentrySDK
.addBreadcrumb(oldToken.generateBreadcrumb(level: .error,
message: "Refreshing token failed - Invalid grant"))
refreshTokenDelegate?.didFailRefreshToken(oldToken)
return .failure(error)
}

// Something else happened
return .failure(error)
}

override func refresh(
_ credential: OAuthAuthenticator.Credential,
for session: Session,
Expand Down Expand Up @@ -513,29 +545,9 @@ class SyncedAuthenticator: OAuthAuthenticator {
}
}

let group = DispatchGroup()
group.enter()
var taskIdentifier: UIBackgroundTaskIdentifier = .invalid
if !self.appContextService.isExtension {
// It is absolutely necessary that the app stays awake while we refresh the token
taskIdentifier = UIApplication.shared.beginBackgroundTask(withName: "Refresh token") {
let message = "Refreshing token failed - Background task expired"
SentryDebug.addBreadcrumb(message: message, category: .apiToken, level: .error, metadata: metadata)

// If we didn't fetch the new token in the given time there is not much we can do apart from hoping that it
// wasn't revoked
if taskIdentifier != .invalid {
UIApplication.shared.endBackgroundTask(taskIdentifier)
taskIdentifier = .invalid
}
}

if taskIdentifier == .invalid {
// We couldn't request additional time to refresh token maybe try later...
completion(.failure(DriveError.refreshToken))
return
}
}
// It is necessary that the app stays awake while we refresh the token
let expiringActivity = ExpiringActivity()
expiringActivity.start()
self.tokenable.refreshToken(token: credential) { token, error in
// New token has been fetched correctly
if let token {
Expand All @@ -545,28 +557,10 @@ class SyncedAuthenticator: OAuthAuthenticator {
self.refreshTokenDelegate?.didUpdateToken(newToken: token, oldToken: credential)
completion(.success(token))
} else {
// Couldn't refresh the token, API says it's invalid
if let error = error as NSError?, error.domain == "invalid_grant" {
let message = "Refreshing token failed - Invalid grant"
SentryDebug.addBreadcrumb(message: message, category: .apiToken, level: .error, metadata: metadata)

self.refreshTokenDelegate?.didFailRefreshToken(credential)
completion(.failure(error))
} else {
// Couldn't refresh the token, keep the old token and fetch it later. Maybe because of bad network ?
let message = "Refreshing token failed - Other \(error.debugDescription)"
SentryDebug.addBreadcrumb(message: message, category: .apiToken, level: .error, metadata: metadata)

completion(.success(credential))
}
}
if taskIdentifier != .invalid {
UIApplication.shared.endBackgroundTask(taskIdentifier)
taskIdentifier = .invalid
completion(self.handleFailedRefreshingToken(oldToken: credential, error: error))
}
group.leave()
expiringActivity.endAll()
}
group.wait()
}
}
}
Expand Down
43 changes: 39 additions & 4 deletions kDriveCore/Data/Cache/AccountManager.swift
View file
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,8 @@ public protocol UpdateAccountDelegate: AnyObject {
}

public protocol AccountManagerDelegate: AnyObject {
func currentAccountNeedsAuthentication()
func refreshCacheScanLibraryAndUpload(preload: Bool, isSwitching: Bool)
func updateRootViewControllerState()
}

public extension InfomaniakLogin {
Expand Down Expand Up @@ -81,6 +82,7 @@ public protocol AccountManageable: AnyObject {
func removeTokenAndAccount(account: Account)
func account(for token: ApiToken) -> Account?
func account(for userId: Int) -> Account?
func logoutCurrentAccountAndSwitchToNextIfPossible()
}

public class AccountManager: RefreshTokenDelegate, AccountManageable {
Expand Down Expand Up @@ -246,11 +248,27 @@ public class AccountManager: RefreshTokenDelegate, AccountManageable {
"Expiration date": token.expirationDate?.timeIntervalSince1970 ?? "Infinite"] as [String: Any]
SentryDebug.capture(message: "Failed refreshing token", context: context, contextKey: "Token Infos")

let tokenUserId = token.userId
tokenStore.removeTokenFor(userId: token.userId)
if let account = account(for: token),
account.userId == currentUserId {
delegate?.currentAccountNeedsAuthentication()

// Remove matching account
guard let accountToDelete = accounts.first(where: { account in
account.userId == tokenUserId
}) else {
SentryDebug.capture(message: "Failed matching failed token to account \(tokenUserId)",
context: context,
contextKey: "Token Infos")
DDLogError("Failed matching failed token to account \(tokenUserId)")
return
}

if accountToDelete == currentAccount {
DDLogInfo("matched \(currentAccount) to \(accountToDelete), removing current account")
notificationHelper.sendDisconnectedNotification()
logoutCurrentAccountAndSwitchToNextIfPossible()
} else {
DDLogInfo("user with token error \(accountToDelete) do not match current account, doing nothing")
removeAccount(toDeleteAccount: accountToDelete)
}
}

Expand Down Expand Up @@ -434,4 +452,21 @@ public class AccountManager: RefreshTokenDelegate, AccountManageable {
public func account(for userId: Int) -> Account? {
return accounts.first { $0.userId == userId }
}

public func logoutCurrentAccountAndSwitchToNextIfPossible() {
Task { @MainActor in
if let currentAccount {
removeTokenAndAccount(account: currentAccount)
}

if let nextAccount = accounts.first {
switchAccount(newAccount: nextAccount)
delegate?.refreshCacheScanLibraryAndUpload(preload: true, isSwitching: true)
} else {
SentrySDK.setUser(nil)
}
saveAccounts()
delegate?.updateRootViewControllerState()
}
}
}
2 changes: 2 additions & 0 deletions kDriveTests/kDrive/Launch/MockAccountManager.swift
View file
Original file line number Diff line number Diff line change
Expand Up @@ -86,4 +86,6 @@ class MockAccountManager: AccountManageable, RefreshTokenDelegate {
func account(for userId: Int) -> Account? { fatalError("Not implemented") }

func updateToken(newToken: ApiToken, oldToken: ApiToken) {}

func logoutCurrentAccountAndSwitchToNextIfPossible() { fatalError("Not implemented") }
}

0 comments on commit 8b1fa9a

Please sign in to comment.