Skip to content

Commit

Permalink
feat: Synchronous decoupled token store
Browse files Browse the repository at this point in the history
  • Loading branch information
PhilippeWeidmann committed Jul 26, 2023
1 parent 175449a commit a8ed0fd
Show file tree
Hide file tree
Showing 9 changed files with 137 additions and 87 deletions.
7 changes: 5 additions & 2 deletions Mail/AppDelegate.swift
Original file line number Diff line number Diff line change
Expand Up @@ -42,13 +42,16 @@ class AppDelegate: UIResponder, UIApplicationDelegate {

func application(_ application: UIApplication, didRegisterForRemoteNotificationsWithDeviceToken deviceToken: Data) {
@InjectService var notificationService: InfomaniakNotifications
@InjectService var tokenStore: TokenStore

for account in AccountManager.instance.accounts {
guard let token = account.token else { continue }
let userApiFetcher = AccountManager.instance.getApiFetcher(for: token.userId, token: token)
Task {
/* Because of a backend issue we can't register the notification token directly after the creation or refresh of
an API token. We wait at least 15 seconds before trying to register. */
try? await Task.sleep(nanoseconds: 15_000_000_000)

guard let token = tokenStore.tokenFor(userId: account.userId) else { return }
let userApiFetcher = AccountManager.instance.getApiFetcher(for: token.userId, token: token)
await notificationService.updateRemoteNotificationsToken(tokenData: deviceToken,
userApiFetcher: userApiFetcher,
updatePolicy: .always)
Expand Down
4 changes: 4 additions & 0 deletions Mail/MailApp.swift
Original file line number Diff line number Diff line change
Expand Up @@ -45,6 +45,9 @@ public struct EarlyDIHook {
let keychainHelper = Factory(type: KeychainHelper.self) { _, _ in
KeychainHelper(accessGroup: AccountManager.accessGroup)
}
let tokenStore = Factory(type: TokenStore.self) { _, _ in
TokenStore()
}
let notificationService = Factory(type: InfomaniakNotifications.self) { _, _ in
InfomaniakNotifications(appGroup: AccountManager.appGroup)
}
Expand All @@ -71,6 +74,7 @@ public struct EarlyDIHook {
SimpleResolver.sharedResolver.store(factory: loginService)
SimpleResolver.sharedResolver.store(factory: notificationService)
SimpleResolver.sharedResolver.store(factory: keychainHelper)
SimpleResolver.sharedResolver.store(factory: tokenStore)
SimpleResolver.sharedResolver.store(factory: appLockHelper)
SimpleResolver.sharedResolver.store(factory: bugTracker)
SimpleResolver.sharedResolver.store(factory: matomoUtils)
Expand Down
11 changes: 6 additions & 5 deletions Mail/Views/Alerts/LogoutConfirmationView.swift
Original file line number Diff line number Diff line change
Expand Up @@ -43,12 +43,13 @@ struct LogoutConfirmationView: View {
Task {
@InjectService var notificationService: InfomaniakNotifications
await notificationService.removeStoredTokenFor(userId: account.userId)

AccountManager.instance.removeTokenAndAccount(account: account)
if let nextAccount = AccountManager.instance.accounts.first {
AccountManager.instance.switchAccount(newAccount: nextAccount)
}
AccountManager.instance.saveAccounts()
}
AccountManager.instance.removeTokenAndAccount(token: account.token)
if let nextAccount = AccountManager.instance.accounts.first {
AccountManager.instance.switchAccount(newAccount: nextAccount)
}
AccountManager.instance.saveAccounts()
}
}

Expand Down
33 changes: 21 additions & 12 deletions Mail/Views/Switch User/AccountView.swift
Original file line number Diff line number Diff line change
Expand Up @@ -27,13 +27,15 @@ import SwiftUI

class AccountViewDelegate: DeleteAccountDelegate {
@MainActor func didCompleteDeleteAccount() {
guard let account = AccountManager.instance.getCurrentAccount() else { return }
AccountManager.instance.removeTokenAndAccount(token: account.token)
if let nextAccount = AccountManager.instance.accounts.first {
AccountManager.instance.switchAccount(newAccount: nextAccount)
IKSnackBar.showSnackBar(message: "Account deleted")
Task {
guard let account = AccountManager.instance.getCurrentAccount() else { return }
AccountManager.instance.removeTokenAndAccount(account: account)
if let nextAccount = AccountManager.instance.accounts.first {
AccountManager.instance.switchAccount(newAccount: nextAccount)
IKSnackBar.showSnackBar(message: "Account deleted")
}
AccountManager.instance.saveAccounts()
}
AccountManager.instance.saveAccounts()
}

@MainActor func didFailDeleteAccount(error: InfomaniakLoginError) {
Expand All @@ -43,15 +45,16 @@ class AccountViewDelegate: DeleteAccountDelegate {
}

struct AccountView: View {
@LazyInjectService private var matomo: MatomoUtils
@LazyInjectService private var tokenStore: TokenStore

@Environment(\.dismiss) private var dismiss

@EnvironmentObject private var mailboxManager: MailboxManager
@AppStorage(UserDefaults.shared.key(.accentColor)) private var accentColor = DefaultPreferences.accentColor

@LazyInjectService private var matomo: MatomoUtils

@State private var isShowingLogoutAlert = false
@State private var isShowingDeleteAccount = false
@State private var presentedDeletedToken: ApiToken?
@State private var delegate = AccountViewDelegate()

let account: Account
Expand Down Expand Up @@ -94,16 +97,16 @@ struct AccountView: View {
.padding(.bottom, 24)
MailButton(label: MailResourcesStrings.Localizable.buttonAccountDelete) {
matomo.track(eventWithCategory: .account, name: "deleteAccount")
isShowingDeleteAccount.toggle()
presentedDeletedToken = tokenStore.removeTokenFor(account: account)
}
.mailButtonStyle(.destructive)
.padding(.bottom, 24)
}
.padding(.horizontal, 16)
.navigationBarTitle(MailResourcesStrings.Localizable.titleMyAccount, displayMode: .inline)
.backButtonDisplayMode(.minimal)
.sheet(isPresented: $isShowingDeleteAccount) {
DeleteAccountView(account: account, delegate: delegate)
.sheet(item: $presentedDeletedToken) { deletedToken in
DeleteAccountView(token: deletedToken, delegate: delegate)
}
.customAlert(isPresented: $isShowingLogoutAlert) {
LogoutConfirmationView(account: account)
Expand All @@ -113,6 +116,12 @@ struct AccountView: View {
}
}

extension ApiToken: Identifiable {
public var id: String {
return "\(userId)\(accessToken)"
}
}

struct AccountView_Previews: PreviewProvider {
static var previews: some View {
AccountView(account: PreviewHelper.sampleAccount)
Expand Down
6 changes: 3 additions & 3 deletions Mail/Views/Switch User/DeleteAccountView.swift
Original file line number Diff line number Diff line change
Expand Up @@ -22,13 +22,13 @@ import MailCore
import SwiftUI

struct DeleteAccountView: UIViewControllerRepresentable {
var account: Account
var delegate: DeleteAccountDelegate
let token: ApiToken
let delegate: DeleteAccountDelegate

func makeUIViewController(context: Context) -> UINavigationController {
return DeleteAccountViewController.instantiateInViewController(
delegate: delegate,
accessToken: account.token.accessToken,
accessToken: token.accessToken,
navBarColor: nil,
navBarButtonColor: nil
)
Expand Down
4 changes: 2 additions & 2 deletions MailCore/API/MailApiFetcher.swift
Original file line number Diff line number Diff line change
Expand Up @@ -382,6 +382,7 @@ class SyncedAuthenticator: OAuthAuthenticator {
) {
AccountManager.instance.refreshTokenLockedQueue.async {
@InjectService var keychainHelper: KeychainHelper
@InjectService var tokenStore: TokenStore
@InjectService var networkLoginService: InfomaniakNetworkLoginable

SentrySDK
Expand All @@ -395,8 +396,7 @@ class SyncedAuthenticator: OAuthAuthenticator {
}

// Maybe someone else refreshed our token
AccountManager.instance.reloadTokensAndAccounts()
if let token = AccountManager.instance.getTokenForUserId(credential.userId),
if let token = tokenStore.tokenFor(userId: credential.userId, fetchLocation: .keychain),
token.expirationDate > credential.expirationDate {
SentrySDK
.addBreadcrumb(token.generateBreadcrumb(level: .info, message: "Refreshing token - Success with local"))
Expand Down
87 changes: 24 additions & 63 deletions MailCore/Cache/AccountManager.swift
Original file line number Diff line number Diff line change
Expand Up @@ -59,17 +59,9 @@ public extension InfomaniakNetworkLoginable {
}
}

@globalActor actor AccountActor: GlobalActor {
static let shared = AccountActor()

public static func run<T>(resultType: T.Type = T.self, body: @AccountActor @Sendable () throws -> T) async rethrows -> T {
try await body()
}
}

public class AccountManager: RefreshTokenDelegate, ObservableObject {
@LazyInjectService var networkLoginService: InfomaniakNetworkLoginable
@LazyInjectService var keychainHelper: KeychainHelper
@LazyInjectService var tokenStore: TokenStore
@LazyInjectService var bugTracker: BugTracker
@LazyInjectService var notificationService: InfomaniakNotifications
@LazyInjectService var matomo: MatomoUtils
Expand All @@ -82,7 +74,6 @@ public class AccountManager: RefreshTokenDelegate, ObservableObject {

private var currentAccount: Account?
public var accounts = SendableArray<Account>()
public var tokens = SendableArray<ApiToken>()
public let refreshTokenLockedQueue = DispatchQueue(label: "com.infomaniak.mail.refreshtoken")
public weak var delegate: AccountManagerDelegate?
public var currentUserId: Int {
Expand Down Expand Up @@ -149,11 +140,6 @@ public class AccountManager: RefreshTokenDelegate, ObservableObject {
let newAccounts = loadAccounts()
accounts.append(contentsOf: newAccounts)

if !accounts.isEmpty {
tokens.removeAll()
tokens.append(contentsOf: keychainHelper.loadTokens())
}

// Also update current account reference to prevent mismatch
if let account = accounts.values.first(where: { $0.userId == currentAccount?.userId }) {
currentAccount = account
Expand All @@ -163,15 +149,6 @@ public class AccountManager: RefreshTokenDelegate, ObservableObject {
for account in accounts where account.user == nil {
removeAccount(toDeleteAccount: account)
}

for token in tokens {
if let account = account(for: token.userId) {
account.token = token
} else {
// remove token with no account
removeTokenAndAccount(token: token)
}
}
}

public func getMailboxManager(for mailbox: Mailbox) -> MailboxManager? {
Expand All @@ -184,7 +161,7 @@ public class AccountManager: RefreshTokenDelegate, ObservableObject {
if let mailboxManager = mailboxManagers[objectId] {
return mailboxManager
} else if let account = account(for: userId),
let token = account.token,
let token = tokenStore.tokenFor(userId: userId),
let mailbox = MailboxInfosManager.instance.getMailbox(id: mailboxId, userId: userId) {
let apiFetcher = getApiFetcher(for: userId, token: token)
let contactManager = getContactManager(for: userId, apiFetcher: apiFetcher)
Expand Down Expand Up @@ -218,12 +195,8 @@ public class AccountManager: RefreshTokenDelegate, ObservableObject {
}
}

public func getTokenForUserId(_ id: Int) -> ApiToken? {
return account(for: id)?.token
}

public func didUpdateToken(newToken: ApiToken, oldToken: ApiToken) {
updateToken(newToken: newToken, oldToken: oldToken)
tokenStore.addToken(newToken: newToken)
}

public func didFailRefreshToken(_ token: ApiToken) {
Expand All @@ -233,14 +206,11 @@ public class AccountManager: RefreshTokenDelegate, ObservableObject {
key: "Token Infos"
)
}
tokens.removeAll { $0.userId == token.userId }
keychainHelper.deleteToken(for: token.userId)
if let account = account(for: token.userId) {
account.token = nil
if account.userId == currentUserId {
delegate?.currentAccountNeedsAuthentication()
NotificationsHelper.sendDisconnectedNotification()
}
tokenStore.removeTokenFor(userId: token.userId)
if let account = account(for: token.userId),
account.userId == currentUserId {
delegate?.currentAccountNeedsAuthentication()
NotificationsHelper.sendDisconnectedNotification()
}
}

Expand All @@ -265,7 +235,7 @@ public class AccountManager: RefreshTokenDelegate, ObservableObject {

let newAccount = Account(apiToken: token)
newAccount.user = user
addAccount(account: newAccount)
addAccount(account: newAccount, token: token)
setCurrentAccount(account: newAccount)

for mailbox in mailboxesResponse {
Expand All @@ -290,13 +260,12 @@ public class AccountManager: RefreshTokenDelegate, ObservableObject {
}

public func updateUser(for account: Account?) async throws {
guard let account, account.isConnected else {
guard let account,
let token = tokenStore.tokenFor(userId: account.userId) else {
throw MailError.noToken
}

let apiFetcher = await AccountActor.run {
getApiFetcher(for: account.userId, token: account.token)
}
let apiFetcher = getApiFetcher(for: account.userId, token: token)
let user = try await apiFetcher.userProfile(dateFormat: .iso8601)
account.user = user

Expand Down Expand Up @@ -457,12 +426,12 @@ public class AccountManager: RefreshTokenDelegate, ObservableObject {
_ = getMailboxManager(for: mailbox)
}

public func addAccount(account: Account) {
public func addAccount(account: Account, token: ApiToken) {
if accounts.values.contains(account) {
removeAccount(toDeleteAccount: account)
}
accounts.append(account)
keychainHelper.storeToken(account.token)
tokenStore.addToken(newToken: token)
saveAccounts()
}

Expand All @@ -481,13 +450,12 @@ public class AccountManager: RefreshTokenDelegate, ObservableObject {
accounts.removeAll { $0 == toDeleteAccount }
}

public func removeTokenAndAccount(token: ApiToken) {
tokens.removeAll { $0.userId == token.userId }
keychainHelper.deleteToken(for: token.userId)
if let account = account(for: token) {
removeAccount(toDeleteAccount: account)
}
networkLoginService.deleteApiToken(token: token) { error in
public func removeTokenAndAccount(account: Account) {
let removedToken = tokenStore.removeTokenFor(userId: account.userId)
removeAccount(toDeleteAccount: account)

guard let removedToken else { return }
networkLoginService.deleteApiToken(token: removedToken) { error in
DDLogError("Failed to delete api token: \(error.localizedDescription)")
}
}
Expand All @@ -500,19 +468,12 @@ public class AccountManager: RefreshTokenDelegate, ObservableObject {
return accounts.values.first { $0.userId == userId }
}

public func updateToken(newToken: ApiToken, oldToken: ApiToken) {
keychainHelper.storeToken(newToken)
for account in accounts where oldToken.userId == account.userId {
account.token = newToken
}
tokens.removeAll { $0.userId == oldToken.userId }
tokens.append(newToken)
}

public func enableBugTrackerIfAvailable() {
if let currentAccount, currentAccount.user?.isStaff == true {
if let currentAccount,
let token = tokenStore.tokenFor(userId: currentAccount.userId),
currentAccount.user?.isStaff == true {
bugTracker.activateOnScreenshot()
let apiFetcher = getApiFetcher(for: currentAccount.userId, token: currentAccount.token)
let apiFetcher = getApiFetcher(for: currentAccount.userId, token: token)
bugTracker.configure(with: apiFetcher)
} else {
bugTracker.stopActivatingOnScreenshot()
Expand Down
Loading

0 comments on commit a8ed0fd

Please sign in to comment.