Additions for more versatile use. Also includes view with black background now as default. #50
Conversation
.optionalEnterPassword is same as .enterPassword but with cancellation possibility.
- Possibility to hide touchID button from view. - Possibility to dismiss view automatically on too many wrong attempts. Requires state with possibility to cancel.
Use new features and add new button to test authentication manually. Also test new callbacks by logging.
Preparing for future changes
Add list of possible callbacks
Correct GitHub repository for Podfile example
Fix typos
Update Podfile example
Include dark version of view
oskarirauta
changed the title
- Reset passcode attempts every time view comes up. - Let animation display to the end, before dismissing on too many passcode attempts.
Fix a bad idea by reverting to older....
|
@oskarirauta can you explain with more details what this PR is for? |
|
I made changes, and wanted you to let know about 'em. Maybe you get some ideas.
- more callbacks
- new type (cancellable version of enter passcode)
- secondary default ui with black background
- more example uses
- possibility to dismiss a cancellable view automaticly after attempts exceed maximum
- bug fix: reset attempts when view comes up
- possibility to use touchid when passcode is requested, but hide button used for that
Lähetetty iPhonesta
… On 7 Jun 2017, at 21.54, Chris Ziogas ***@***.***> wrote:
@oskarirauta can you explain with more details what this PR is for?
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub, or mute the thread.
|
|
Nice ideas.
Might be nice to also follow @yankodimitrov way and instead of callbacks do the same my posting a notification. What do you think about this?
Can you describe the use case of this? What I think you want from a security perspective is to lock the user out of your App/View. I already see partly your use case, as you can say that this view is cancelable. Although, will it be better UX wise to handle this case in your code and let the user know why she got out of the flow he was trying to reach? So your code will here for a notification and in this case will a) show an alert or something to the user, b) close the Passcode view.
In the App we are using this library, this is not what we want, so might be easier that the App itself handles this case. Although I would like to know your use case, and if it fits better to what most need, then it might be better for this library. :)
Which button, the one in the PasscodeViewController? This would mean that once the user puts your App in background mode, and again back in foreground, there will be no way for the user to open TouchID. Of course, you can still open it for him programmatically. Again, what most people need from this library. Thanks for your contribution @oskarirauta and for putting the above on discussion from all of us. It's all about setting this library defaults, as the each implementor can switch them in their App implementation, so let's discuss altogether what the library defaults we want to be. |
|
On 9 Jun 2017, at 16:52, Chris Ziogas ***@***.***> wrote:
Nice ideas.
more callbacks
Might be nice to also follow @yankodimitrov <https://github.com/yankodimitrov> way and instead of callbacks do the same my posting a notification. What do you think about this?
I am developing a application which has multiple lockable view controllers, I thought notification approach, but decided that this way it’s easier to point notifications to correct view controller. My application also has multiple points that are locked in same view controller, therefore, it’s way easier to give each section their own callback instead of thinking which part is supposed to receive a notification.
But one could easily provide both, it won’t grow heap size too much to include these as well.
possibility to dismiss a cancellable view automatically after attempts exceed maximum
Can you describe the use case of this? What I think you want from a security perspective is to lock the user out of your App/View. I already see partly your use case, as you can say that this view is cancelable. Although, will it be better UX wise to handle this case in your code and let the user know why she got out of the flow he was trying to reach? So your code will here for a notification and in this case will a) show an alert or something to the user, b) close the Passcode view.
It more or less follows the “protocol” set by Apple. When you want to open keypad lock, you can use either your finger or enter code - also, if you don’t have the code - or decide so, you may cancel. In application that I am developing, I have sections that are described as public and secure, public sections are available without authentication, but secure ones must be authenticated for viewing, therefore, without cancellation, user who wouldn’t have credentials, would be forced to restart the application when he reaches point where authentication is required. I wanted a way for going back.
bug fix: reset attempts when view comes up
In the App we are using this library, this is not what we want, so might be easier that the App itself handles this case. Although I would like to know your use case, and if it fits better to what most need, then it might be better for this library. :)
Yes, but still - I don’t want to authenticate when starting application - I want to authenticate inside it, depending on user’s configuration - multiple times, user might want to give another use access to some parts, but not all, so he authenticates these parts and leaves other parts locked.. Making app handle this- would cause unnecessary replication of code.
possibility to use touchid when passcode is requested, but hide button used for that
Which button, the one in the PasscodeViewController? This would mean that once the user puts your App in background mode, and again back in foreground, there will be no way for the user to open TouchID. Of course, you can still open it for him programmatically. Again, what most people need from this library.
True. But in my approach I don’t use it like this at all. One using it in the way you and example describes it - this shouldn’t be enabled.
Idea behind this is simple: we use touchID and passcode is a fallback, and there’s no going back after. Thought I saw this approach somewhere else… And liked it :)
But your mileage may vary, that’s why it’s optional.
Thanks for your contribution @oskarirauta <https://github.com/oskarirauta> and for putting the above on discussion from all of us. It's all about setting this library defaults, as the each implementor can switch them in their App implementation, so let's discuss altogether what the library defaults we want to be.
I think defaults are good as they were, I just wanted to give something more, which can be toggled when necessary. What I contributed… Is not finished/polished, but it works, so it could be considered as a stub - or a concept. But too many times in this world, we decide to re-invent the wheel, so I decided to make my contribution public and tell my ideas out loud :)
For sure, you could join my pull request as is, but one who wants features I brought, can easily use my fork - I just think you have great potential, so I shared my ideas :)
With best regards, Oskari Rauta.
|
I see your point to this one.
So, it is indeed good to offer this easily to implementors. DOes it makes sense to change this line Then you can remove the whole
I still can't see how this is a problem in any case. I will try to run some tests in XCode in order I see it in action.
Fair enough, do you think that Thanks a lot @oskarirauta and I think some from ideas above make sense. I will try to test them and will come back with feedback if needed. @yankodimitrov & @velikanov are you willing to give back your feedback, as you know the lib better than me :) |
|
On 10 Jun 2017, at 19:32, Chris Ziogas ***@***.***> wrote:
more callbacks
I see your point to this one.
possibility to dismiss a cancellable view automatically after attempts exceed maximum
So, it is indeed good to offer this easily to implementors. DOes it makes sense to change this line case .enterOptionalPasscode: return EnterOptionalPasscodeState() to case .enterOptionalPasscode: return EnterPasscodeState(allowCancellation: true)?
Then you can remove the whole EnterOptionalPasscodeState implementation.
I wasn't able to check the diff in my work laptop, so I am checking the files in GitHub only, let me know if more changes are made.
Yes, this indeed is a better solution. No need to invent wheel twice ;)
bug fix: reset attempts when view comes up
I still can't see how this is a problem in any case. I will try to run some tests in XCode in order I see it in action.
While testing this, without resetting attempts, and when using dismiss option after maximum attempts, sometimes, it dismissed after one or two attempts, even though maximum attempts was set to 3.
possibility to use touchid when passcode is requested, but hide button used for that
Fair enough, do you think that hideTouchIDButton makes more sense as the option name instead of shouldDisableTouchIDButton?
I indeed do think, it makes more sense.
… Thanks a lot @oskarirauta <https://github.com/oskarirauta> and I think some from ideas above make sense.
I will try to test them and will come back with feedback if needed.
@yankodimitrov <https://github.com/yankodimitrov> & @velikanov <https://github.com/velikanov> are you willing to give back your feedback, as you know the lib better than me :)
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub <#50 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/ABRSHt6Vq51MXRXpH4BP8O0_lxFO2uAaks5sCsUngaJpZM4Nwpwt>.
|
|
Hi @oskarirauta. p.s.: it's really cool, that you are willing to contribute to this library 👍 |
|
Previous changes were a bug fix. What is your apple ID? I will let you beta test my app, so you can see how this performs.
Lähetetty iPhonesta
… On 11 Jul 2017, at 13.37, Chris Ziogas ***@***.***> wrote:
Hi @oskarirauta.
This PR becomes very big, making it harder to review and merge it. I will try to check it again, but it will be helpful if you separate your future work in smaller, more pointed PRs. :)
p.s.: it's really cool, that you are willing to contribute to this library 👍
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub, or mute the thread.
|
|
I can't find your email, in order I send my ID in a private channel. Can you try to email me? |
|
It is [email protected] <mailto:[email protected]>
Software although is in a beta stage, but partially gives idea, why I need authentication multiple times. (And partially hiding screen when backgrounding app does not work, but this is not because of SwiftPasscodeLock)
… On 11 Jul 2017, at 17:38, Chris Ziogas ***@***.***> wrote:
I can't find your email, in order I send my ID in a private channel. Can you try to email me?
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub <#50 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/ABRSHnPoOgFu7WEPKwSz-qUWqbhLppkKks5sM4kAgaJpZM4Nwpwt>.
|
changed execution order of callbacks..
Swift 5 syntax
Nothing that my commits do - was impossible before I made these commits, but now it's just easier...