refactor how Credentials work

This commit is contained in:
Maurice Parker 2019-05-05 03:25:21 -05:00
parent 261e2a951a
commit fc7b6f2c6b
11 changed files with 102 additions and 158 deletions

View File

@ -113,7 +113,7 @@ public final class Account: DisplayNameProvider, UnreadCountProvider, Container,
let dataFolder: String let dataFolder: String
let database: ArticlesDatabase let database: ArticlesDatabase
let delegate: AccountDelegate var delegate: AccountDelegate
static let saveQueue = CoalescingQueue(name: "Account Save Queue", interval: 1.0) static let saveQueue = CoalescingQueue(name: "Account Save Queue", interval: 1.0)
private var unreadCounts = [String: Int]() // [feedID: Int] private var unreadCounts = [String: Int]() // [feedID: Int]
@ -228,6 +228,8 @@ public final class Account: DisplayNameProvider, UnreadCountProvider, Container,
NotificationCenter.default.addObserver(self, selector: #selector(displayNameDidChange(_:)), name: .DisplayNameDidChange, object: nil) NotificationCenter.default.addObserver(self, selector: #selector(displayNameDidChange(_:)), name: .DisplayNameDidChange, object: nil)
NotificationCenter.default.addObserver(self, selector: #selector(childrenDidChange(_:)), name: .ChildrenDidChange, object: nil) NotificationCenter.default.addObserver(self, selector: #selector(childrenDidChange(_:)), name: .ChildrenDidChange, object: nil)
delegate.credentials = try? retrieveBasicCredentials()
pullObjectsFromDisk() pullObjectsFromDisk()
DispatchQueue.main.async { DispatchQueue.main.async {
@ -242,99 +244,32 @@ public final class Account: DisplayNameProvider, UnreadCountProvider, Container,
// MARK: - API // MARK: - API
public func storeCredentials(_ credentials: Credentials) throws { public func storeCredentials(_ credentials: Credentials) throws {
guard let server = delegate.server else {
guard let username = credentials.username, let password = credentials.password, let server = delegate.server else {
throw CredentialsError.incompleteCredentials throw CredentialsError.incompleteCredentials
} }
self.username = username switch credentials {
case .basic(let username, _):
let passwordData = password.data(using: String.Encoding.utf8)! self.username = username
let updateQuery: [String: Any] = [kSecClass as String: kSecClassInternetPassword,
kSecAttrAccount as String: username,
kSecAttrServer as String: server]
let attributes: [String: Any] = [kSecValueData as String: passwordData]
let status = SecItemUpdate(updateQuery as CFDictionary, attributes as CFDictionary)
switch status {
case errSecSuccess:
return
case errSecItemNotFound:
break
default:
throw CredentialsError.unhandledError(status: status)
} }
guard status == errSecItemNotFound else { try CredentialsManager.storeCredentials(credentials, server: server)
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)
if addStatus != errSecSuccess {
throw CredentialsError.unhandledError(status: status)
}
} }
public func retrieveCredentials() throws -> Credentials? { public func retrieveBasicCredentials() throws -> Credentials? {
guard let username = self.username, let server = delegate.server else { guard let username = self.username, let server = delegate.server else {
return nil return nil
} }
return try CredentialsManager.retrieveBasicCredentials(server: server, username: username)
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 BasicCredentials(username: username, password: password)
} }
public func removeCredentials() throws { public func removeBasicCredentials() throws {
guard let username = self.username, let server = delegate.server else { guard let username = self.username, let server = delegate.server else {
return return
} }
try CredentialsManager.removeBasicCredentials(server: server, username: username)
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)
}
self.username = nil self.username = nil
} }
public static func validateCredentials(transport: Transport = URLSession.webserviceTransport(), type: AccountType, credentials: Credentials, completionHandler handler: @escaping (Result<Bool, Error>) -> Void) { public static func validateCredentials(transport: Transport = URLSession.webserviceTransport(), type: AccountType, credentials: Credentials, completionHandler handler: @escaping (Result<Bool, Error>) -> Void) {

View File

@ -214,26 +214,26 @@
848934EC1F62484F00CEBD24 = { 848934EC1F62484F00CEBD24 = {
isa = PBXGroup; isa = PBXGroup;
children = ( children = (
846E77531F6F00E300A165E2 /* AccountManager.swift */,
848935101F62486800CEBD24 /* Account.swift */, 848935101F62486800CEBD24 /* Account.swift */,
84AF4EA3222CFDD100F6A800 /* AccountSettings.swift */,
841974241F6DDCE4006346C4 /* AccountDelegate.swift */, 841974241F6DDCE4006346C4 /* AccountDelegate.swift */,
841974001F6DD1EC006346C4 /* Folder.swift */, 846E77531F6F00E300A165E2 /* AccountManager.swift */,
84AF4EA3222CFDD100F6A800 /* AccountSettings.swift */,
84F73CF0202788D80000BCEF /* ArticleFetcher.swift */,
84C365491F899F3B001EC85C /* CombinedRefreshProgress.swift */,
8419740D1F6DD25F006346C4 /* Container.swift */,
84B99C9E1FAE8D3200ECDEDB /* ContainerPath.swift */,
84C8B3F31F89DE430053CCA6 /* DataExtensions.swift */,
844B297C2106C7EC004020B3 /* Feed.swift */, 844B297C2106C7EC004020B3 /* Feed.swift */,
84B2D4CE2238C13D00498ADA /* FeedMetadata.swift */, 84B2D4CE2238C13D00498ADA /* FeedMetadata.swift */,
841974001F6DD1EC006346C4 /* Folder.swift */,
844B297E210CE37E004020B3 /* UnreadCountProvider.swift */, 844B297E210CE37E004020B3 /* UnreadCountProvider.swift */,
84B99C9E1FAE8D3200ECDEDB /* ContainerPath.swift */, 848935031F62484F00CEBD24 /* AccountTests */,
84C365491F899F3B001EC85C /* CombinedRefreshProgress.swift */,
84C8B3F31F89DE430053CCA6 /* DataExtensions.swift */,
84F73CF0202788D80000BCEF /* ArticleFetcher.swift */,
8419740D1F6DD25F006346C4 /* Container.swift */,
8419742B1F6DDE84006346C4 /* LocalAccount */,
84245C7D1FDDD2580074AFBB /* Feedbin */, 84245C7D1FDDD2580074AFBB /* Feedbin */,
8469F80F1F6DC3C10084783E /* Frameworks */, 8469F80F1F6DC3C10084783E /* Frameworks */,
848934FA1F62484F00CEBD24 /* Info.plist */, 8419742B1F6DDE84006346C4 /* LocalAccount */,
848935031F62484F00CEBD24 /* AccountTests */,
848934F71F62484F00CEBD24 /* Products */, 848934F71F62484F00CEBD24 /* Products */,
D511EEB4202422BB00712EC3 /* xcconfig */, D511EEB4202422BB00712EC3 /* xcconfig */,
848934FA1F62484F00CEBD24 /* Info.plist */,
); );
sourceTree = "<group>"; sourceTree = "<group>";
usesTabs = 1; usesTabs = 1;

View File

@ -14,8 +14,7 @@ public protocol AccountDelegate {
// Local account does not; some synced accounts might. // Local account does not; some synced accounts might.
var supportsSubFolders: Bool { get } var supportsSubFolders: Bool { get }
var server: String? { get } var server: String? { get }
var credentials: Credentials? { get set }
static func validateCredentials(transport: Transport, credentials: Credentials, completionHandler handler: @escaping (Result<Bool, Error>) -> Void)
var refreshProgress: DownloadProgress { get } var refreshProgress: DownloadProgress { get }
@ -34,4 +33,7 @@ public protocol AccountDelegate {
// Saved to disk with other Account data. Could be called at any time. // Saved to disk with other Account data. Could be called at any time.
// And called many times. // And called many times.
func userInfo(for: Account) -> NSDictionary? func userInfo(for: Account) -> NSDictionary?
static func validateCredentials(transport: Transport, credentials: Credentials, completionHandler handler: @escaping (Result<Bool, Error>) -> Void)
} }

View File

@ -26,12 +26,12 @@ class AccountCredentialsTest: XCTestCase {
// Make sure any left over from failed tests are gone // Make sure any left over from failed tests are gone
do { do {
try account.removeCredentials() try account.removeBasicCredentials()
} catch { } catch {
XCTFail(error.localizedDescription) XCTFail(error.localizedDescription)
} }
var credentials: Credentials? = BasicCredentials(username: "maurice", password: "hardpasswd") var credentials: Credentials? = Credentials.basic(username: "maurice", password: "hardpasswd")
// Store the credentials // Store the credentials
do { do {
@ -43,15 +43,19 @@ class AccountCredentialsTest: XCTestCase {
// Retrieve them // Retrieve them
credentials = nil credentials = nil
do { do {
credentials = try account.retrieveCredentials() credentials = try account.retrieveBasicCredentials()
} catch { } catch {
XCTFail(error.localizedDescription) XCTFail(error.localizedDescription)
} }
XCTAssertEqual("maurice", credentials!.username)
XCTAssertEqual("hardpasswd", credentials!.password) switch credentials! {
case .basic(let username, let password):
XCTAssertEqual("maurice", username)
XCTAssertEqual("hardpasswd", password)
}
// Update them // Update them
credentials?.password = "easypasswd" credentials = Credentials.basic(username: "maurice", password: "easypasswd")
do { do {
try account.storeCredentials(credentials!) try account.storeCredentials(credentials!)
} catch { } catch {
@ -61,23 +65,27 @@ class AccountCredentialsTest: XCTestCase {
// Retrieve them again // Retrieve them again
credentials = nil credentials = nil
do { do {
credentials = try account.retrieveCredentials() credentials = try account.retrieveBasicCredentials()
} catch { } catch {
XCTFail(error.localizedDescription) XCTFail(error.localizedDescription)
} }
XCTAssertEqual("maurice", credentials!.username)
XCTAssertEqual("easypasswd", credentials!.password) switch credentials! {
case .basic(let username, let password):
XCTAssertEqual("maurice", username)
XCTAssertEqual("easypasswd", password)
}
// Delete them // Delete them
do { do {
try account.removeCredentials() try account.removeBasicCredentials()
} catch { } catch {
XCTFail(error.localizedDescription) XCTFail(error.localizedDescription)
} }
// Make sure they are gone // Make sure they are gone
do { do {
try credentials = account.retrieveCredentials() try credentials = account.retrieveBasicCredentials()
} catch { } catch {
XCTFail(error.localizedDescription) XCTFail(error.localizedDescription)
} }

View File

@ -11,8 +11,11 @@ import RSWeb
struct NilTransport: Transport { struct NilTransport: Transport {
func send(request: URLRequest, completion: @escaping (Result<Data, Error>) -> Void) { func send<T>(request: URLRequest, resultType: T.Type, completion: @escaping (Result<(HTTPHeaders, T), Error>) -> Void) where T : Decodable, T : Encodable {
completion(.success(Data())) }
func send(request: URLRequest, completion: @escaping (Result<(HTTPHeaders, Data), Error>) -> Void) {
} }
} }

View File

@ -13,13 +13,14 @@ final class FeedbinAPICaller: NSObject {
private let feedbinBaseURL = URL(string: "https://api.feedbin.com/v2/")! private let feedbinBaseURL = URL(string: "https://api.feedbin.com/v2/")!
private var transport: Transport! private var transport: Transport!
var credentials: Credentials?
init(transport: Transport) { init(transport: Transport) {
super.init() super.init()
self.transport = transport self.transport = transport
} }
func validateCredentials(credentials: Credentials, completionHandler handler: @escaping (Result<Bool, Error>) -> Void) { func validateCredentials(completionHandler completion: @escaping (Result<Bool, Error>) -> Void) {
let callURL = feedbinBaseURL.appendingPathComponent("authentication.json") let callURL = feedbinBaseURL.appendingPathComponent("authentication.json")
let request = URLRequest(url: callURL, credentials: credentials) let request = URLRequest(url: callURL, credentials: credentials)
@ -27,20 +28,38 @@ final class FeedbinAPICaller: NSObject {
transport.send(request: request) { result in transport.send(request: request) { result in
switch result { switch result {
case .success: case .success:
handler(.success(true)) completion(.success(true))
case .failure(let error): case .failure(let error):
switch error { switch error {
case TransportError.httpError(let status): case TransportError.httpError(let status):
if status == 401 { if status == 401 {
handler(.success(false)) completion(.success(false))
} else { } else {
handler(.failure(error)) completion(.failure(error))
} }
default: default:
handler(.failure(error)) completion(.failure(error))
} }
} }
} }
} }
func retrieveSubscriptions(completionHandler completion: @escaping (Result<[FeedbinFeed], Error>) -> Void) {
let callURL = feedbinBaseURL.appendingPathComponent("subscriptions.json")
let request = URLRequest(url: callURL, credentials: credentials)
transport.send(request: request, resultType: [FeedbinFeed].self) { result in
switch result {
case .success(let (headers, feeds)):
break // TODO: put pageing implementation here
case .failure(let error):
completion(.failure(error))
}
}
}
} }

View File

@ -15,17 +15,23 @@ final class FeedbinAccountDelegate: AccountDelegate {
let server: String? = "api.feedbin.com" let server: String? = "api.feedbin.com"
private let caller: FeedbinAPICaller private let caller: FeedbinAPICaller
var credentials: Credentials? {
didSet {
caller.credentials = credentials
}
}
init(transport: Transport) { init(transport: Transport) {
caller = FeedbinAPICaller(transport: transport) caller = FeedbinAPICaller(transport: transport)
} }
var refreshProgress = DownloadProgress(numberOfTasks: 0) var refreshProgress = DownloadProgress(numberOfTasks: 0)
static func validateCredentials(transport: Transport, credentials: Credentials, completionHandler handler: @escaping (Result<Bool, Error>) -> Void) { static func validateCredentials(transport: Transport, credentials: Credentials, completionHandler handler: @escaping (Result<Bool, Error>) -> Void) {
let caller = FeedbinAPICaller(transport: transport) let caller = FeedbinAPICaller(transport: transport)
caller.validateCredentials(credentials: credentials) { result in caller.credentials = credentials
caller.validateCredentials() { result in
handler(result) handler(result)
} }

View File

@ -10,7 +10,7 @@ import Foundation
import RSCore import RSCore
import RSParser import RSParser
struct FeedbinFeed { struct FeedbinFeed: Codable {
// https://github.com/feedbin/feedbin-api/blob/master/content/feeds.md // https://github.com/feedbin/feedbin-api/blob/master/content/feeds.md
// //
@ -28,45 +28,13 @@ struct FeedbinFeed {
let url: String let url: String
let homePageURL: String? let homePageURL: String?
struct Key { enum CodingKeys: String, CodingKey {
static let subscriptionID = "id" case subscriptionID = "id"
static let feedID = "feed_id" case feedID = "feed_id"
static let creationDate = "created_at" case creationDate = "created_at"
static let name = "title" case name = "title"
static let url = "feed_url" case url = "feed_url"
static let homePageURL = "site_url" case homePageURL = "site_url"
} }
init?(dictionary: JSONDictionary) {
guard let subscriptionID = dictionary[Key.subscriptionID] as? Int else {
return nil
}
guard let feedID = dictionary[Key.feedID] as? Int else {
return nil
}
guard let url = dictionary[Key.url] as? String else {
return nil
}
self.subscriptionID = subscriptionID
self.feedID = feedID
self.url = url
if let creationDateString = dictionary[Key.creationDate] as? String {
self.creationDate = RSDateWithString(creationDateString)
}
else {
self.creationDate = nil
}
self.name = dictionary[Key.name] as? String
self.homePageURL = dictionary[Key.homePageURL] as? String
}
static func feeds(with array: JSONArray) -> [FeedbinFeed]? {
let subs = array.compactMap { FeedbinFeed(dictionary: $0) }
return subs.isEmpty ? nil : subs
}
} }

View File

@ -13,6 +13,7 @@ final class LocalAccountDelegate: AccountDelegate {
let supportsSubFolders = false let supportsSubFolders = false
let server: String? = nil let server: String? = nil
var credentials: Credentials?
private let refresher = LocalAccountRefresher() private let refresher = LocalAccountRefresher()

View File

@ -27,9 +27,11 @@ class AccountsFeedbinWindowController: NSWindowController {
} }
override func windowDidLoad() { override func windowDidLoad() {
if let account = account, let credentials = try? account.retrieveCredentials() { if let account = account, let credentials = try? account.retrieveBasicCredentials() {
usernameTextField.stringValue = credentials.username ?? "" if case .basic(let username, let password) = credentials {
passwordTextField.stringValue = credentials.password ?? "" usernameTextField.stringValue = username
passwordTextField.stringValue = password
}
actionButton.title = NSLocalizedString("Update", comment: "Update") actionButton.title = NSLocalizedString("Update", comment: "Update")
} else { } else {
actionButton.title = NSLocalizedString("Create", comment: "Create") actionButton.title = NSLocalizedString("Create", comment: "Create")
@ -62,7 +64,7 @@ class AccountsFeedbinWindowController: NSWindowController {
progressIndicator.isHidden = false progressIndicator.isHidden = false
progressIndicator.startAnimation(self) progressIndicator.startAnimation(self)
let credentials = BasicCredentials(username: usernameTextField.stringValue, password: passwordTextField.stringValue) let credentials = Credentials.basic(username: usernameTextField.stringValue, password: passwordTextField.stringValue)
Account.validateCredentials(type: .feedbin, credentials: credentials) { [weak self] result in Account.validateCredentials(type: .feedbin, credentials: credentials) { [weak self] result in
guard let self = self else { return } guard let self = self else { return }
@ -81,7 +83,7 @@ class AccountsFeedbinWindowController: NSWindowController {
} }
do { do {
try self.account?.removeCredentials() try self.account?.removeBasicCredentials()
try self.account?.storeCredentials(credentials) try self.account?.storeCredentials(credentials)
self.hostWindow?.endSheet(self.window!, returnCode: NSApplication.ModalResponse.OK) self.hostWindow?.endSheet(self.window!, returnCode: NSApplication.ModalResponse.OK)
} catch { } catch {

@ -1 +1 @@
Subproject commit 731711fff487f923d5be8ea1f4a9c19a58f059c3 Subproject commit 64732f6b09b56374059bc3feb1ba1afb859dd0fa