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

Use LAPolicyDeviceOwnerAuthentication when fallback is enabled #82

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

azgron
Copy link

@azgron azgron commented May 19, 2019

'Enter Password' doesn't working. It because the policy should be LAPolicyDeviceOwnerAuthentication when fallback is enabled.

Screen Shot 2019-05-19 at 9 55 33

@phillbaker
Copy link
Collaborator

Thanks @azgron - to clarify, what's the behavior you're expecting to see when "Enter Password" is used?

To clarify - the fallback doesn't bring up the system fallback (e.g. password), but the app's fallback.

@azgron
Copy link
Author

azgron commented May 20, 2019

@phillbaker Hah, I didn't understand it. I thought it should fallback to device passcode :)

Do you think the passcode could be useful for the library?

Simulator Screen Shot - iPhone 8 - 2019-05-20 at 08 03 43

@phillbaker
Copy link
Collaborator

Do you think the passcode could be useful for the library?

I don't understand the question - do you mean this PR? Can you explain a bit?

@azgron
Copy link
Author

azgron commented May 20, 2019

Yes.
This pull request was because in the beginning I thought that the functionality of clicking 'Enter Password' button, should present the 'ios passcode screen' (system fallback):
Simulator Screen Shot - iPhone 8 - 2019-05-20 at 08 03 43

Now, I do understand that the application should implement it's own functionality of fallback.

To clarify - the fallback doesn't bring up the system fallback (e.g. password), but the app's fallback.

Do you think this library should support system fallback as default?
If yes, How do you think it should be implemented?

@phillbaker
Copy link
Collaborator

Do you think this library should support system fallback as default?
If yes, How do you think it should be implemented?

As I understand it, the system password is not available for use by applications - it's only available to the device itself. Are there any docs for exposing the system password for use in apps?

@azgron
Copy link
Author

azgron commented May 22, 2019

@phillbaker

My changes in this pull request do this functionality (Read about LAPolicyDeviceOwnerAuthentication vs LAPolicyDeviceOwnerAuthenticationWithBiometrics)

It brings the passcode device screen.

Please check my changes and run it locally (Try to press ‘Enter passcode’).

I have attached a gif:
IMG_7107 TRIM

@azgron
Copy link
Author

azgron commented May 26, 2019

@phillbaker

What do you think? Would you like to change the way I implemented the system fallback?

@phillbaker
Copy link
Collaborator

phillbaker commented May 27, 2019 via email

@MarianBe
Copy link

MarianBe commented Jul 4, 2019

will this pr be merged? i really like the approach

@ReemaVinodGangdev
Copy link

@azgron
I really want to implement device password fallback in my app. Thanks much for this PR. Is this pr be merged?

@mikehardy
Copy link
Collaborator

There were questions above @ReemaVinodGangdev

  • only supports iOS10+, what does react-native support? iOS moved to 10+ - https://github.com/facebook/react-native/blob/master/template/ios/Podfile#L4 - as of react-native 0.63 which is release candidate now but will release soon. It seems this may need IFDEFs for iOS9 so it is safe on them as the old react-native's should be supported for more than "the upcoming release candidate has it"

  • is it cross-compatible with android - no idea on that one, and @azgron did not answer

If you @ReemaVinodGangdev were at least able to pull the PR and test it to report success that would probably also help the process. Usually PRs need a few people to collaborate in order to review, pull+test+report-success etc, so if you need it, help push it forward and that's how it gets done :-)

@marcosrdz
Copy link

is it cross-compatible with android - no idea on that one, and @azgron did not answer

iOS only. Given that only 1 file was modified.

If you @ReemaVinodGangdev were at least able to pull the PR and test it to report success that would probably also help the process. Usually PRs need a few people to collaborate in order to review, pull+test+report-success etc, so if you need it, help push it forward and that's how it gets done :-)

I tested it. Worked great.

@rayansys
Copy link

rayansys commented Aug 3, 2021

please merge & update.
worked for me

@Vatsal4GVM
Copy link

Please Merge this PR.

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

Successfully merging this pull request may close these issues.

8 participants