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

Better support for operationPrompt #18

Open
hfossli opened this issue Dec 20, 2017 · 4 comments
Open

Better support for operationPrompt #18

hfossli opened this issue Dec 20, 2017 · 4 comments

Comments

@hfossli
Copy link
Contributor

hfossli commented Dec 20, 2017

Continuing the discussion from #17.

I think it is a good idea, but I am still conflicted. I don't want to make the API and code more complicated than it is.

I would love to just use the LAContext under the hood, but sadly the localizedReason property was introduced on iOS 11... Thus forcing us to instead send operationPrompt parameter all the way down to the query.

Example of what's needed to change.

Current code

public final class Manager {

    ...1
        
    public func privateKey(context: LAContext? = nil) throws -> PrivateKey {
        ...2
    }
    enum Prompt {
        case context(LAContext)
        case string(String)
    }

    public final class Manager {

        ...1
            
        public func privateKey(context: LAContext? = nil) throws -> PrivateKey {    
            return privateKey(prompt: .context(context))
        }

        public func privateKey(operationPrompt: String) throws -> PrivateKey {    
            return privateKey(prompt: .string(operationPrompt))
        }

        private func privateKey(prompt: Prompt) throws -> PrivateKey {    
            ...2 
            - let key = try helper.getPrivateKey(context: context)
            + let key = try helper.getPrivateKey(prompt: prompt)
        }
@wuf810
Copy link

wuf810 commented Dec 20, 2017

I've had to do the same in my own implementation i.e. use operationPrompt pre iOS 11.x

I don't think you currently have any other choice and although it's not so nice, it is a necessary evil for the moment.

@hfossli
Copy link
Contributor Author

hfossli commented Dec 20, 2017

Yep. Thanks for chiming in.

One additional constraint is that we don’t want to query the private key excessively for devices that doesnt support the access control flag «privateKeyUsage».

@alkalim
Copy link
Contributor

alkalim commented Dec 20, 2017

Looks like a reasonable solution. I'm not sure if you implied this or not but this enum could also be utilized at the API level?

let config = EllipticCurveKeyPair.Config(
...
                operationPrompt: EllipticCurveKeyPair.Prompt.string("Message"),
                // or EllipticCurveKeyPair.Prompt.context(context)
...)

@hfossli
Copy link
Contributor Author

hfossli commented Dec 21, 2017

Yes, it could. Users could also write the shorter version (omitting EllipticCurveKeyPair.Prompt)

let config = EllipticCurveKeyPair.Config(
...
                operationPrompt: .string("Message"),
...)

since the type is known.

I was looking for non-breaking changes, but it seems like a good idea to do a breaking change in order to get this right.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants