diff --git a/Frameworks/Account/Account.swift b/Frameworks/Account/Account.swift index 3f2a51dc9..d7a02667c 100644 --- a/Frameworks/Account/Account.swift +++ b/Frameworks/Account/Account.swift @@ -274,68 +274,31 @@ public final class Account: DisplayNameProvider, UnreadCountProvider, Container, // MARK: - API public func storeCredentials(_ credentials: Credentials) throws { + username = credentials.username guard let server = delegate.server else { - throw CredentialsError.incompleteCredentials + assertionFailure() + return } - - switch credentials { - case .basic(let username, _): - self.username = username - case .readerAPIBasicLogin(let username, _): - self.username = username - case .readerAPIAuthLogin(let username, _): - self.username = username - case .oauthAccessToken(let username, _): - self.username = username - case .oauthRefreshToken(let username, _): - self.username = username - } - try CredentialsManager.storeCredentials(credentials, server: server) - delegate.credentials = credentials } - public func retrieveCredentials() throws -> Credentials? { - switch type { - case .feedbin: - guard let username = self.username, let server = delegate.server else { - return nil - } - return try CredentialsManager.retrieveBasicCredentials(server: server, username: username) - case .freshRSS: - guard let username = self.username, let server = delegate.server else { - return nil - } - return try CredentialsManager.retrieveReaderAPIAuthCredentials(server: server, username: username) - default: + public func retrieveCredentials(type: CredentialsType) throws -> Credentials? { + guard let username = self.username, let server = delegate.server else { return nil } + return try CredentialsManager.retrieveCredentials(type: type, server: server, username: username) } - public func removeCredentials() throws { - switch type { - case .feedbin: - guard let username = self.username, let server = delegate.server else { - return - } - try CredentialsManager.removeBasicCredentials(server: server, username: username) - self.username = nil - case .freshRSS: - guard let username = self.username, let server = delegate.server else { - return - } - try CredentialsManager.removeReaderAPIAuthCredentials(server: server, username: username) - self.username = nil - default: - break + public func removeCredentials(type: CredentialsType) throws { + guard let username = self.username, let server = delegate.server else { + return } + try CredentialsManager.removeCredentials(type: type, server: server, username: username) } public static func validateCredentials(transport: Transport = URLSession.webserviceTransport(), type: AccountType, credentials: Credentials, endpoint: URL? = nil, completion: @escaping (Result) -> Void) { switch type { - case .onMyMac: - LocalAccountDelegate.validateCredentials(transport: transport, credentials: credentials, completion: completion) case .feedbin: FeedbinAccountDelegate.validateCredentials(transport: transport, credentials: credentials, completion: completion) case .freshRSS: diff --git a/Frameworks/Account/Credentials/Credentials.swift b/Frameworks/Account/Credentials/Credentials.swift index 9ce2c6651..22a99e8a3 100644 --- a/Frameworks/Account/Credentials/Credentials.swift +++ b/Frameworks/Account/Credentials/Credentials.swift @@ -13,11 +13,22 @@ public enum CredentialsError: Error { case unhandledError(status: OSStatus) } -public enum Credentials { - case basic(username: String, password: String) - case readerAPIBasicLogin(username: String, password: String) - case readerAPIAuthLogin(username: String, apiKey: String) - case oauthAccessToken(username: String, token: String) - case oauthRefreshToken(username: String, token: String) +public enum CredentialsType: String { + case basic = "password" + case readerBasic = "readerBasic" + case readerAPIKey = "readerAPIKey" + case oauthAccessToken = "oauthAccessToken" + case oauthRefreshToken = "oauthRefreshToken" } +public struct Credentials { + public let type: CredentialsType + public let username: String + public let secret: String + + public init(type: CredentialsType, username: String, secret: String) { + self.type = type + self.username = username + self.secret = secret + } +} diff --git a/Frameworks/Account/Credentials/CredentialsManager.swift b/Frameworks/Account/Credentials/CredentialsManager.swift index 136aef279..6ac291cca 100644 --- a/Frameworks/Account/Credentials/CredentialsManager.swift +++ b/Frameworks/Account/Credentials/CredentialsManager.swift @@ -12,127 +12,17 @@ public struct CredentialsManager { public static func storeCredentials(_ credentials: Credentials, server: String) throws { - switch credentials { - case .basic(let username, let password): - try storeBasicCredentials(server: server, username: username, password: password) - case .readerAPIBasicLogin(let username, let password): - try storeBasicCredentials(server: server, username: username, password: password) - case .readerAPIAuthLogin(let username, let apiKey): - try storeBasicCredentials(server: server, username: username, password: apiKey) - case .oauthAccessToken(let username, let token): - try storeBasicCredentials(server: server, username: username, password: token) - case .oauthRefreshToken(let username, let token): - try storeBasicCredentials(server: server, username: username, password: token) - } - - } - - public static func retrieveBasicCredentials(server: String, username: String) throws -> Credentials? { - - let query: [String: Any] = [kSecClass as String: kSecClassInternetPassword, - kSecAttrAccount as String: username, - kSecAttrServer as String: server, - kSecMatchLimit as String: kSecMatchLimitOne, - kSecReturnAttributes as String: true, - kSecReturnData as String: true] - - var item: CFTypeRef? - let status = SecItemCopyMatching(query as CFDictionary, &item) - - guard status != errSecItemNotFound else { - return nil - } - - guard status == errSecSuccess else { - throw CredentialsError.unhandledError(status: status) - } - - guard let existingItem = item as? [String : Any], - let passwordData = existingItem[kSecValueData as String] as? Data, - let password = String(data: passwordData, encoding: String.Encoding.utf8) else { - return nil - } - - return Credentials.basic(username: username, password: password) - - } - - public static func removeBasicCredentials(server: String, username: String) throws { - - let query: [String: Any] = [kSecClass as String: kSecClassInternetPassword, - kSecAttrAccount as String: username, - kSecAttrServer as String: server, - kSecMatchLimit as String: kSecMatchLimitOne, - kSecReturnAttributes as String: true, - kSecReturnData as String: true] - - let status = SecItemDelete(query as CFDictionary) - guard status == errSecSuccess || status == errSecItemNotFound else { - throw CredentialsError.unhandledError(status: status) - } - - } - - public static func retrieveReaderAPIAuthCredentials(server: String, username: String) throws -> Credentials? { - - let query: [String: Any] = [kSecClass as String: kSecClassInternetPassword, - kSecAttrAccount as String: username, - kSecAttrServer as String: server, - kSecMatchLimit as String: kSecMatchLimitOne, - kSecReturnAttributes as String: true, - kSecReturnData as String: true] - - var item: CFTypeRef? - let status = SecItemCopyMatching(query as CFDictionary, &item) - - guard status != errSecItemNotFound else { - return nil - } - - guard status == errSecSuccess else { - throw CredentialsError.unhandledError(status: status) - } - - guard let existingItem = item as? [String : Any], - let passwordData = existingItem[kSecValueData as String] as? Data, - let password = String(data: passwordData, encoding: String.Encoding.utf8) else { - return nil - } - - return Credentials.readerAPIAuthLogin(username: username, apiKey: password) - - } - - public static func removeReaderAPIAuthCredentials(server: String, username: String) throws { - - let query: [String: Any] = [kSecClass as String: kSecClassInternetPassword, - kSecAttrAccount as String: username, - kSecAttrServer as String: server, - kSecMatchLimit as String: kSecMatchLimitOne, - kSecReturnAttributes as String: true, - kSecReturnData as String: true] - - let status = SecItemDelete(query as CFDictionary) - guard status == errSecSuccess || status == errSecItemNotFound else { - throw CredentialsError.unhandledError(status: status) - } - - } -} - -// MARK: Private - -extension CredentialsManager { - - static func storeBasicCredentials(server: String, username: String, password: String) throws { - - let passwordData = password.data(using: String.Encoding.utf8)! - - let updateQuery: [String: Any] = [kSecClass as String: kSecClassInternetPassword, - kSecAttrAccount as String: username, + var query: [String: Any] = [kSecClass as String: kSecClassInternetPassword, + kSecAttrAccount as String: credentials.username, kSecAttrServer as String: server] - let attributes: [String: Any] = [kSecValueData as String: passwordData] - let status = SecItemUpdate(updateQuery as CFDictionary, attributes as CFDictionary) + + if credentials.type != .basic { + query[kSecAttrSecurityDomain as String] = credentials.type.rawValue + } + + let secretData = credentials.secret.data(using: String.Encoding.utf8)! + let attributes: [String: Any] = [kSecValueData as String: secretData] + let status = SecItemUpdate(query as CFDictionary, attributes as CFDictionary) switch status { case errSecSuccess: @@ -147,15 +37,67 @@ extension CredentialsManager { return } - let addQuery: [String: Any] = [kSecClass as String: kSecClassInternetPassword, - kSecAttrAccount as String: username, - kSecAttrServer as String: server, - kSecValueData as String: passwordData] - let addStatus = SecItemAdd(addQuery as CFDictionary, nil) + query[kSecValueData as String] = secretData + + let addStatus = SecItemAdd(query as CFDictionary, nil) if addStatus != errSecSuccess { throw CredentialsError.unhandledError(status: status) } + + } + + public static func retrieveCredentials(type: CredentialsType, server: String, username: String) throws -> Credentials? { + + var query: [String: Any] = [kSecClass as String: kSecClassInternetPassword, + kSecAttrAccount as String: username, + kSecAttrServer as String: server, + kSecMatchLimit as String: kSecMatchLimitOne, + kSecReturnAttributes as String: true, + kSecReturnData as String: true] + + if type != .basic { + query[kSecAttrSecurityDomain as String] = type.rawValue + } + + var item: CFTypeRef? + let status = SecItemCopyMatching(query as CFDictionary, &item) + + guard status != errSecItemNotFound else { + return nil + } + + guard status == errSecSuccess else { + throw CredentialsError.unhandledError(status: status) + } + + guard let existingItem = item as? [String : Any], + let secretData = existingItem[kSecValueData as String] as? Data, + let secret = String(data: secretData, encoding: String.Encoding.utf8) else { + return nil + } + + return Credentials(type: type, username: username, secret: secret) + + } + + public static func removeCredentials(type: CredentialsType, server: String, username: String) throws { + + var query: [String: Any] = [kSecClass as String: kSecClassInternetPassword, + kSecAttrAccount as String: username, + kSecAttrServer as String: server, + kSecMatchLimit as String: kSecMatchLimitOne, + kSecReturnAttributes as String: true, + kSecReturnData as String: true] + + if type != .basic { + query[kSecAttrSecurityDomain as String] = type.rawValue + } + + let status = SecItemDelete(query as CFDictionary) + guard status == errSecSuccess || status == errSecItemNotFound else { + throw CredentialsError.unhandledError(status: status) + } } - + } diff --git a/Frameworks/Account/Credentials/URLRequest+RSWeb.swift b/Frameworks/Account/Credentials/URLRequest+RSWeb.swift index 534b05e7c..4374f285b 100755 --- a/Frameworks/Account/Credentials/URLRequest+RSWeb.swift +++ b/Frameworks/Account/Credentials/URLRequest+RSWeb.swift @@ -19,29 +19,28 @@ public extension URLRequest { return } - switch credentials { - case .basic(let username, let password): - let data = "\(username):\(password)".data(using: .utf8) + switch credentials.type { + case .basic: + let data = "\(credentials.username):\(credentials.secret)".data(using: .utf8) let base64 = data?.base64EncodedString() let auth = "Basic \(base64 ?? "")" setValue(auth, forHTTPHeaderField: HTTPRequestHeader.authorization) - case .readerAPIBasicLogin(let username, let password): + case .readerBasic: setValue("application/x-www-form-urlencoded", forHTTPHeaderField: "Content-Type") httpMethod = "POST" - let postData = "Email=\(username)&Passwd=\(password)" + let postData = "Email=\(credentials.username)&Passwd=\(credentials.secret)" httpBody = postData.data(using: String.Encoding.utf8) - case .readerAPIAuthLogin(_, let apiKey): - let auth = "GoogleLogin auth=\(apiKey)" + case .readerAPIKey: + let auth = "GoogleLogin auth=\(credentials.secret)" setValue(auth, forHTTPHeaderField: HTTPRequestHeader.authorization) - case .oauthAccessToken(_, let token): - let auth = "OAuth \(token)" + case .oauthAccessToken: + let auth = "OAuth \(credentials.secret)" setValue(auth, forHTTPHeaderField: "Authorization") case .oauthRefreshToken: // While both access and refresh tokens are credentials, it seems the `Credentials` cases // enumerates how the identity of the user can be proved rather than // credentials-in-general, such as in this refresh token case, // the authority to prove an identity. - // TODO: Refactor as usage becomes clearer. assertionFailure("Refresh tokens are used to replace expired access tokens. Did you mean to use `accessToken` instead?") break } diff --git a/Frameworks/Account/Feedbin/FeedbinAccountDelegate.swift b/Frameworks/Account/Feedbin/FeedbinAccountDelegate.swift index 1226473c8..cc9020df3 100644 --- a/Frameworks/Account/Feedbin/FeedbinAccountDelegate.swift +++ b/Frameworks/Account/Feedbin/FeedbinAccountDelegate.swift @@ -525,7 +525,7 @@ final class FeedbinAccountDelegate: AccountDelegate { } func accountDidInitialize(_ account: Account) { - credentials = try? account.retrieveCredentials() + credentials = try? account.retrieveCredentials(type: .basic) accountMetadata = account.metadata } @@ -1229,7 +1229,7 @@ private extension FeedbinAccountDelegate { func retrieveCredentialsIfNecessary(_ account: Account) { if credentials == nil { - credentials = try? account.retrieveCredentials() + credentials = try? account.retrieveCredentials(type: .basic) } } diff --git a/Frameworks/Account/ReaderAPI/ReaderAPIAccountDelegate.swift b/Frameworks/Account/ReaderAPI/ReaderAPIAccountDelegate.swift index 1d5e376a8..ca6631b20 100644 --- a/Frameworks/Account/ReaderAPI/ReaderAPIAccountDelegate.swift +++ b/Frameworks/Account/ReaderAPI/ReaderAPIAccountDelegate.swift @@ -414,7 +414,7 @@ final class ReaderAPIAccountDelegate: AccountDelegate { func accountDidInitialize(_ account: Account) { accountMetadata = account.metadata - credentials = try? account.retrieveCredentials() + credentials = try? account.retrieveCredentials(type: .readerAPIKey) } static func validateCredentials(transport: Transport, credentials: Credentials, endpoint: URL?, completion: @escaping (Result) -> Void) { diff --git a/Frameworks/Account/ReaderAPI/ReaderAPICaller.swift b/Frameworks/Account/ReaderAPI/ReaderAPICaller.swift index 6d92e84f7..23499a9c0 100644 --- a/Frameworks/Account/ReaderAPI/ReaderAPICaller.swift +++ b/Frameworks/Account/ReaderAPI/ReaderAPICaller.swift @@ -84,11 +84,6 @@ final class ReaderAPICaller: NSObject { return } - guard case .readerAPIBasicLogin(let username, _) = credentials else { - completion(.failure(CredentialsError.incompleteCredentials)) - return - } - let request = URLRequest(url: endpoint.appendingPathComponent(ReaderAPIEndpoints.login.rawValue), credentials: credentials) transport.send(request: request) { result in @@ -117,7 +112,7 @@ final class ReaderAPICaller: NSObject { } // Save Auth Token for later use - self.credentials = .readerAPIAuthLogin(username: username, apiKey: authString) + self.credentials = Credentials(type: .readerAPIKey, username: credentials.username, secret: authString) completion(.success(self.credentials)) case .failure(let error): diff --git a/Mac/Preferences/Accounts/AccountsFeedbinWindowController.swift b/Mac/Preferences/Accounts/AccountsFeedbinWindowController.swift index a1e0af2cf..d097151e1 100644 --- a/Mac/Preferences/Accounts/AccountsFeedbinWindowController.swift +++ b/Mac/Preferences/Accounts/AccountsFeedbinWindowController.swift @@ -27,10 +27,8 @@ class AccountsFeedbinWindowController: NSWindowController { } override func windowDidLoad() { - if let account = account, let credentials = try? account.retrieveCredentials() { - if case .basic(let username, _) = credentials { - usernameTextField.stringValue = username - } + if let account = account, let credentials = try? account.retrieveCredentials(type: .basic) { + usernameTextField.stringValue = credentials.username actionButton.title = NSLocalizedString("Update", comment: "Update") } else { actionButton.title = NSLocalizedString("Create", comment: "Create") @@ -63,7 +61,7 @@ class AccountsFeedbinWindowController: NSWindowController { progressIndicator.isHidden = false progressIndicator.startAnimation(self) - let credentials = Credentials.basic(username: usernameTextField.stringValue, password: passwordTextField.stringValue) + let credentials = Credentials(type: .basic, username: usernameTextField.stringValue, secret: passwordTextField.stringValue) Account.validateCredentials(type: .feedbin, credentials: credentials) { [weak self] result in guard let self = self else { return } @@ -79,6 +77,7 @@ class AccountsFeedbinWindowController: NSWindowController { self.errorMessageLabel.stringValue = NSLocalizedString("Invalid email/password combination.", comment: "Credentials Error") return } + var newAccount = false if self.account == nil { self.account = AccountManager.shared.createAccount(type: .feedbin) @@ -86,7 +85,7 @@ class AccountsFeedbinWindowController: NSWindowController { } do { - try self.account?.removeCredentials() + try self.account?.removeCredentials(type: .basic) try self.account?.storeCredentials(validatedCredentials) if newAccount { self.account?.refreshAll() { result in diff --git a/Mac/Preferences/Accounts/AccountsReaderAPIWindowController.swift b/Mac/Preferences/Accounts/AccountsReaderAPIWindowController.swift index d4a986339..1035a4f53 100644 --- a/Mac/Preferences/Accounts/AccountsReaderAPIWindowController.swift +++ b/Mac/Preferences/Accounts/AccountsReaderAPIWindowController.swift @@ -42,10 +42,9 @@ class AccountsReaderAPIWindowController: NSWindowController { } } - if let account = account, let credentials = try? account.retrieveCredentials() { - if case .basic(let username, _) = credentials { - usernameTextField.stringValue = username - } + if let account = account, let credentials = try? account.retrieveCredentials(type: .readerBasic) { + usernameTextField.stringValue = credentials.username + apiURLTextField.stringValue = account.endpointURL?.absoluteString ?? "" actionButton.title = NSLocalizedString("Update", comment: "Update") } else { actionButton.title = NSLocalizedString("Create", comment: "Create") @@ -83,7 +82,7 @@ class AccountsReaderAPIWindowController: NSWindowController { return } - let credentials = Credentials.readerAPIBasicLogin(username: usernameTextField.stringValue, password: passwordTextField.stringValue) + let credentials = Credentials(type: .readerBasic, username: usernameTextField.stringValue, secret: passwordTextField.stringValue) Account.validateCredentials(type: accountType!, credentials: credentials, endpoint: apiURL) { [weak self] result in guard let self = self else { return } @@ -109,7 +108,9 @@ class AccountsReaderAPIWindowController: NSWindowController { do { self.account?.endpointURL = apiURL - try self.account?.removeCredentials() + try self.account?.removeCredentials(type: .readerBasic) + try self.account?.removeCredentials(type: .readerAPIKey) + try self.account?.storeCredentials(credentials) try self.account?.storeCredentials(validatedCredentials) if newAccount { diff --git a/iOS/Settings/Account/SettingsFeedbinAccountView.swift b/iOS/Settings/Account/SettingsFeedbinAccountView.swift index 82543bf00..cd862ae89 100644 --- a/iOS/Settings/Account/SettingsFeedbinAccountView.swift +++ b/iOS/Settings/Account/SettingsFeedbinAccountView.swift @@ -62,7 +62,7 @@ struct SettingsFeedbinAccountView : View { error = "" let emailAddress = viewModel.email.trimmingCharacters(in: .whitespaces) - let credentials = Credentials.basic(username: emailAddress, password: viewModel.password) + let credentials = Credentials(type: .basic, username: emailAddress, secret: viewModel.password) Account.validateCredentials(type: .feedbin, credentials: credentials) { result in @@ -85,7 +85,7 @@ struct SettingsFeedbinAccountView : View { do { do { - try workAccount.removeCredentials() + try workAccount.removeCredentials(type: .basic) } catch {} try workAccount.storeCredentials(credentials) @@ -125,9 +125,8 @@ struct SettingsFeedbinAccountView : View { init(account: Account) { self.account = account - if case .basic(let username, let password) = try? account.retrieveCredentials() { - self.email = username - self.password = password + if let credentials = try? account.retrieveCredentials(type: .basic) { + self.email = credentials.username } } diff --git a/iOS/Settings/Account/SettingsReaderAPIAccountView.swift b/iOS/Settings/Account/SettingsReaderAPIAccountView.swift index a63ff2ba4..a20255d67 100644 --- a/iOS/Settings/Account/SettingsReaderAPIAccountView.swift +++ b/iOS/Settings/Account/SettingsReaderAPIAccountView.swift @@ -64,7 +64,7 @@ struct SettingsReaderAPIAccountView : View { error = "" let emailAddress = viewModel.email.trimmingCharacters(in: .whitespaces) - let credentials = Credentials.readerAPIBasicLogin(username: emailAddress, password: viewModel.password) + let credentials = Credentials(type: .readerBasic, username: emailAddress, secret: viewModel.password) guard let apiURL = URL(string: viewModel.apiURL) else { self.error = "Invalid API URL." return @@ -94,11 +94,13 @@ struct SettingsReaderAPIAccountView : View { do { do { - try workAccount.removeCredentials() + try workAccount.removeCredentials(type: .readerBasic) + try workAccount.removeCredentials(type: .readerAPIKey) } catch {} workAccount.endpointURL = apiURL + try workAccount.storeCredentials(credentials) try workAccount.storeCredentials(validatedCredentials) if newAccount { @@ -136,9 +138,8 @@ struct SettingsReaderAPIAccountView : View { init(account: Account) { self.account = account self.accountType = account.type - if case .readerAPIBasicLogin(let username, let password) = try? account.retrieveCredentials() { - self.email = username - self.password = password + if let credentials = try? account.retrieveCredentials(type: .readerBasic) { + self.email = credentials.username self.apiURL = account.endpointURL?.absoluteString ?? "" } }