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 25, 2023
1 parent 175449a commit 6c82b08
Show file tree
Hide file tree
Showing 9 changed files with 139 additions and 72 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
17 changes: 12 additions & 5 deletions Mail/Views/Alerts/LogoutConfirmationView.swift
Original file line number Diff line number Diff line change
Expand Up @@ -43,12 +43,19 @@ 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()
}
}

extension ApiToken: Identifiable {
public var id: Int {
return userId
}
}

Expand Down
25 changes: 15 additions & 10 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 @@ -51,7 +53,7 @@ struct AccountView: View {
@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 +96,19 @@ struct AccountView: View {
.padding(.bottom, 24)
MailButton(label: MailResourcesStrings.Localizable.buttonAccountDelete) {
matomo.track(eventWithCategory: .account, name: "deleteAccount")
isShowingDeleteAccount.toggle()
Task {
@InjectService var tokenStore: TokenStore
presentedDeletedToken = await 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 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
70 changes: 20 additions & 50 deletions MailCore/Cache/AccountManager.swift
Original file line number Diff line number Diff line change
Expand Up @@ -69,7 +69,7 @@ public extension InfomaniakNetworkLoginable {

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 +82,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 +148,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 +157,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 +169,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 +203,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.updateToken(newToken: newToken, oldToken: oldToken)
}

public func didFailRefreshToken(_ token: ApiToken) {
Expand All @@ -233,10 +214,8 @@ public class AccountManager: RefreshTokenDelegate, ObservableObject {
key: "Token Infos"
)
}
tokens.removeAll { $0.userId == token.userId }
keychainHelper.deleteToken(for: token.userId)
tokenStore.removeTokenFor(userId: token.userId)
if let account = account(for: token.userId) {
account.token = nil
if account.userId == currentUserId {
delegate?.currentAccountNeedsAuthentication()
NotificationsHelper.sendDisconnectedNotification()
Expand Down Expand Up @@ -265,7 +244,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 +269,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 +435,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 +459,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 +477,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
74 changes: 74 additions & 0 deletions MailCore/Cache/TokenStore.swift
Original file line number Diff line number Diff line change
@@ -0,0 +1,74 @@
/*
Infomaniak Mail - iOS App
Copyright (C) 2022 Infomaniak Network SA
This program is free software: you can redistribute it and/or modify
it under the terms of the GNU General Public License as published by
the Free Software Foundation, either version 3 of the License, or
(at your option) any later version.
This program is distributed in the hope that it will be useful,
but WITHOUT ANY WARRANTY; without even the implied warranty of
MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
GNU General Public License for more details.
You should have received a copy of the GNU General Public License
along with this program. If not, see <http://www.gnu.org/licenses/>.
*/

import Foundation
import InfomaniakCore
import InfomaniakDI

public class TokenStore {
public enum TokenStoreFetchLocation {
case cache
case keychain
}

public typealias UserId = Int
@LazyInjectService private var keychainHelper: KeychainHelper
private let tokens = SendableDictionary<UserId, ApiToken>()

public init() {
let keychainTokens = keychainHelper.loadTokens()
for token in keychainTokens {
tokens[token.userId] = token
}
}

@discardableResult
public func removeTokenFor(userId: UserId) -> ApiToken? {
let removedToken = tokens.removeValue(forKey: userId)
keychainHelper.deleteToken(for: userId)

return removedToken
}

@discardableResult
public func removeTokenFor(account: Account) -> ApiToken? {
return removeTokenFor(userId: account.userId)
}

public func updateToken(newToken: ApiToken, oldToken: ApiToken) {
keychainHelper.storeToken(newToken)
removeTokenFor(userId: oldToken.userId)
tokens[newToken.userId] = newToken
}

public func tokenFor(userId: UserId, fetchLocation: TokenStoreFetchLocation = .cache) -> ApiToken? {
if fetchLocation == .keychain {
let keychainTokens = keychainHelper.loadTokens()
for token in keychainTokens {
tokens[token.userId] = token
}
}

return tokens[userId]
}

public func addToken(newToken: ApiToken) {
keychainHelper.storeToken(newToken)
tokens[newToken.userId] = newToken
}
}
Loading

0 comments on commit 6c82b08

Please sign in to comment.