Reading from keychain, need static method to instantiate object

[matthewpalmer]

Lots of people have questions about the utility of having password in the examples in the README. One day, I'd like this to be improved.

If you have the same question, this issue has some relevant discussion that might help.

If you notice any other confusing aspects of the documentation, please leave a comment here too and I'll fix it up when I eventually do the rewrite.

[matthewpalmer]

A quick code dump that might be helpful—I was exploring a couple of different approaches that make sense in a documentation sense.

import UIKit
import Locksmith

enum SoundCloudAccountError: ErrorType {
    case TokenNotInitialized
}

///
/// There are many ways you can represent an account using Locksmith. This is a great part of the flexibility that protocols offer. Here are two of the more common ways it could be used to hold credentials for an audio file host.
///

///
/// The first option is to conform to all of Locksmith's protocols on a type with mutable state.
///
struct SoundCloudAccount: CreateableSecureStorable, ReadableSecureStorable, GenericPasswordSecureStorable, DeleteableSecureStorable {
    let service = "SoundCloud"
    var account: String { return userID }
    let userID: String
    var token: String?

    init(userID: String, token: String? = nil) {
        self.userID = userID
        self.token = token
    }

    func createInSecureStore() throws {
        guard token != nil else { throw SoundCloudAccountError.TokenNotInitialized }
        try (self as CreateableSecureStorable).createInSecureStore()
    }

    var data: [String: AnyObject] {
        guard let token = token else { fatalError("Calling `createInSecureStore` without token is invalid.") }
        return ["token": token]
    }

    func authenticate(completion: (token: String) -> ()) {
        // Get an access token from the server
        let token = NSUUID().UUIDString
        completion(token: token)
    }
}

///
/// Another option is to have two types representing the two states: one type for unauthenticated users, and one for those that have logged in with the app previously.
///

protocol LibsynAccount {
    var username: String { get }
}

extension LibsynAccount {
    var service: String { return "Libsyn" }
    var account: String { return username }
    var KeychainDataKey: String { return "password" }
}

struct UnauthenticatedLibsynAccount: LibsynAccount, CreateableSecureStorable, GenericPasswordSecureStorable {
    let username: String
    let password: String

    var data: [String: AnyObject] {
        return [KeychainDataKey: password]
    }
}

struct AuthenticatedLibsynAccount: LibsynAccount, ReadableSecureStorable, GenericPasswordSecureStorable, DeleteableSecureStorable {
    let username: String
    private var password: String?

    init?(username: String) {
        self.username = username
        guard let result = self.readFromSecureStore() else { return nil }
        guard let keychainData = result.data else { return nil }
        guard let password = keychainData[KeychainDataKey] as? String else { return nil }
        self.password = password
    }

    func upload() -> String {
        return "Upload successful"
    }
}

class ViewController: UIViewController {
    override func viewDidLoad() {
        super.viewDidLoad()
        // Do any additional setup after loading the view, typically from a nib.

        // Two examples: SoundCloud (mutable) and Libsyn (two types).

         SoundCloud()

        // Libsyn()

    }

    func SoundCloud() {
        // Get a user's credentials via the UI
        let userID = "user_13843"

        // Some time later...
        func someTimeLater(user: SoundCloudAccount) {
            // user.upload()

            // Clean up for demo purposes
            try! user.deleteFromSecureStore()
        }

        var account = SoundCloudAccount(userID: userID)
        account.authenticate { (token) -> () in
            account.token = token
            try! account.createInSecureStore()
            someTimeLater(account)
        }
    }

    func Libsyn() {
        // Get a user's credentials via the UI
        let username = "matthewpalmer"
        let password = "my_password"

        let unauthenticatedAccount = UnauthenticatedLibsynAccount(username: username, password: password)
        // Authenticate the account and then...
        try! unauthenticatedAccount.createInSecureStore()

        // Some time later...
        let authenticatedAccount = AuthenticatedLibsynAccount(username: username)
        authenticatedAccount?.upload()

        // Clean up for demo purposes
        try! authenticatedAccount?.deleteFromSecureStore()
    }
}

[stepanhruda]

I think you want a static method for the reads instead

[stepanhruda]

This is my current solution in the spirit of the readme. It would be great to get rid of the nested type.

struct TwitterCredentials: CreateableSecureStorable, GenericPasswordSecureStorable {
    let username: String
    let password: String

    static func readFromKeychainForUsername(username: String) -> TwitterCredentials? {
        let access = Access(username: username)
        let result = access.readFromSecureStore()
        guard let data = result?.data, password = data[passwordKey] as? String else { return nil }
        return TwitterCredentials(username: username, password: password)
    }

    static func deleteFromKeychainForUsername(username: String) throws {
        let access = Access(username: username)
        try access.deleteFromSecureStore()
    }

    // MARK: GenericPasswordSecureStorable

    var service: String { return "Twitter" }
    var account: String { return username }

    // MARK: CreateableSecureStorable

    var data: [String: AnyObject] {
        return [
            passwordKey: password,
        ]
    }

    private struct Access: ReadableSecureStorable, DeleteableSecureStorable, GenericPasswordSecureStorable {

        private let username: String

        var service: String { return "Twitter" }
        var account: String { return username }
    }
}

private let passwordKey = "password"

[matthewpalmer]

Was exploring the idea of using a factory to create the objects. The main problem with using a static func is that it'd be really hard to test, and it'd be tricky to get all of the necessary meta-attributes for keychain items in as arguments.

I was playing around with something like this (which didn't work because Swift doesn't allow structs nested in generic functions :-1: ).

protocol SomeFactoryType {
  typealias Produces
  typealias KeychainResultType: SecureStorableResultType
  typealias Source: CreateableSecureStorable
  func make(source: Source, ((KeychainResultType) -> Produces)) -> Produces
}

extension SomeFactoryType {
  func make(source: Source, construct: ((KeychainResultType) -> Produces)) -> Produces {
    // Fails because we can't do this nested struct
    struct MinimalReadableType: ReadableSecureStorable {
       let service: String
       let account: String
    }

    let read = MinimalReadableType(service: source.service, account: source.account)
    return construct(read.readFromSecureStore)
  }
}

[matthewpalmer]

Just dumping out some thoughts here...

The main concerns in developing this are:

• we need to be able to access the service, account, etc. etc. of an already existing type
• the library can't know precisely how to construct an object that conforms to ReadableSecureStorable since it'll vary so much
• we might be able to add a requirement for ReadableSecureStorable such that it can be init-d with a SecureStorableResultType, i.e. init(result: SecureStorableResultType). Hmm...

struct TwitterAccount: Createable, Readable, GenericPassword {
  let username: String
  let password: String

  var account: String = { username }
  let service: String = "Twitter"
  var data: [String: AnyObject] = { ["password": password] }

  init(username: String, password: String) {
    // Regular old init method (e.g. when the user first enters their username and password)
    //...
  }

  init(username: String, result: SecureStorableResultType) {
    self.username = username
    self.password = result.data["password"] as! String
    // (or maybe we make this a failable init so no need to force cast)
  }
}

which would then let us create a factory method on Createable

extension Createable where Self : Readable {
    func makeFromKeychain() -> Self {
        // again we hit the problems of the inner struct
    }
}

hmm don't think that's going to work

[matthewpalmer]

I quite like @stepanhruda's solution (more than any I've come up with), but I don't think we can introduce it at the library level. As mentioned, it'd be great to remove the nested type.

(Tangent: I think Swift 3 is expanding how protocols and generics work together, so I'll be interested in seeing how that turns out. I'd love to make the saved and retrieved data from the keychain generic, and hopefully some of the improvements in the language will help us with this problem.)

[skington]

The thing that really confused me for quite a while, as a new-to-Swift developer but an experience Perl (classic and Moose) OO programmer, was this line:

// ReadableSecureStorable lets us read the account from the keychain
let result = account.readFromSecureStore()

Why would I already have a TwitterAccount object? The whole point is that I'm trying to fetch one!

I tried messing about with calling e.g. TwitterAccount.readFromSecureStore() or calling readFromSecureStore inside an initializer, and eventually settled on the less-intuitive approach of creating a blank object and adding a fetch method. I only really stumbled across that approach by finding this issue, and similar ones. It would be useful to explain what's going on in the documentation.

[matthewpalmer]

Have been playing with something like this (which works but is still very experimental)

// In Locksmith.swift
public protocol ReadableSecureStorable_2: SecureStorable {
    static var service: String { get }

    init?(account: String, keychainResult: SecureStorableResultType)
}

struct BlandType: GenericPasswordSecureStorable, ReadableSecureStorable {
    let service: String
    let account: String
}

public extension ReadableSecureStorable_2 {
    static func constructAccountFromKeychain(account: String) -> Self? {
        let t = BlandType(service: Self.service, account: account)

        guard let result: SecureStorableResultType = t.readFromSecureStore() else { return nil }
        return self.init(account: account, keychainResult: result)
    }
}

struct TwitterAccount: GenericPasswordSecureStorable, CreateableSecureStorable, ReadableSecureStorable_2, DeleteableSecureStorable {
    static let service = "Twitter"

    var account: String { return username }
    var service: String = TwitterAccount.service

    let username: String
    let password: String

    var data: [String: AnyObject] {
        return ["password": password]
    }

    init(username: String, password: String) {
        self.username = username
        self.password = password
    }

    init?(account: String, keychainResult: SecureStorableResultType) {
        self.username = account
        guard let data = keychainResult.data, let password = data["password"] as? String else { return nil }

        self.password = password
    }
}

let t = TwitterAccount(username: "yeah", password: "okay")
//        try! t.deleteFromSecureStore()
//        try! t.createInSecureStore()

// Some time later...
let t2 = TwitterAccount.constructAccountFromKeychain("yeah")
print(t2!.password)

[dltlr]

Any progress on this approach?

Something like this let t2 = TwitterAccount.constructAccountFromKeychain("yeah") that you have above seems reasonable.

[matthewpalmer]

@dltlr no real updates, unfortunately. Haven't really had time to flesh this out fully

[neokree]

I have another implementation that seems to work.

class TwitterAccount : ReadableSecureStorable, CreateableSecureStorable, DeleteableSecureStorable, GenericPasswordSecureStorable {
  var username: String
  var password: String

  var account: String = { return username }
  let service: String = "Twitter"
  var data: [String: AnyObject] = { return ["password": password] }

  init(username: String) {
     self.username = username
     password = "" // default value 
  }
}

Then in your controller/AppDelegate or other

let account = TwitterAccount(username: "myCurrentUsername")
let values = account.readFromSecureStore()

It's the first time i use keychain so maybe I'm missing something, but here you are using service variable for identify the object and account for know the account. Tell me if i'm breaking something and I should not use this code please.

[gavinwilliams]

I agree with a lot of this. If I had the Account username and password, why would I need to persist it to the keychain. There should either be a method to get the Struct by passing the account name. If you use structs, you can't retrieve your account using loadDataForUserAccount. Just spent an hour trying to work this out.