Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat: Synchronous decoupled token store #897

Merged
merged 1 commit into from
Jul 26, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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 {
PhilippeWeidmann marked this conversation as resolved.
Show resolved Hide resolved
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
Loading