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

Techdebt/remove auth prompt #256

Merged
merged 5 commits into from
Nov 30, 2024
Merged

Techdebt/remove auth prompt #256

merged 5 commits into from
Nov 30, 2024

Conversation

kcw-grunt
Copy link
Contributor

@kcw-grunt kcw-grunt commented Nov 1, 2024

Overview

The original code was a fork from a private project where they planned on having many features for their original app (Breadwallet). One of the ideas was extra layers of authorization.

In Litewallet, one of the many complaints of our users is they can forget their device PIN and lose access to their Litecoin. In this case, the user must remember that and the app PIN.

This work is to remove that extra layer.

Bug Report

#241

Definition of Done

@kcw-grunt kcw-grunt force-pushed the techdebt/remove-auth-prompt branch from c87e10c to 08f5f83 Compare November 1, 2024 21:48
@kcw-grunt kcw-grunt added 🐛 bug Something isn't working as expected 🦺 tech debt Actions that help maintain the repo and application labels Nov 5, 2024
@kcw-grunt kcw-grunt requested a review from josikie November 5, 2024 09:35
@kcw-grunt kcw-grunt assigned josikie and unassigned josikie Nov 5, 2024
@kcw-grunt kcw-grunt marked this pull request as ready for review November 5, 2024 09:36
@kcw-grunt kcw-grunt linked an issue Nov 5, 2024 that may be closed by this pull request
@andhikayuana
Copy link
Contributor

andhikayuana commented Nov 7, 2024

Hi @kcw-grunt do we have a plan to use biometric/fingerprint again?

I have experience implementing passkeys in my previous company, did you know about passkeys?
with passkeys, this is will not happen

In Litewallet, one of the many complaints of our users is they can forget their device PIN and lose access to their
Litecoin. In this case, the user must remember that and the app PIN.

because it will use password/pin/pattern/face/fingerprint from the device / OS

@andhikayuana
Copy link
Contributor

also I have an assumption that this crash maybe about not using Fragment from androidx, because app.fragment deprecated

app/build.gradle Outdated Show resolved Hide resolved
@kcw-grunt
Copy link
Contributor Author

Hi @kcw-grunt do we have a plan to use biometric/fingerprint again?

I have experience implementing passkeys in my previous company, did you know about passkeys? with passkeys, this is will not happen

Not until the UI refresh next year.

In Litewallet, one of the many complaints of our users is they can forget their device PIN and lose access to their
Litecoin. In this case, the user must remember that and the app PIN.

because it will use password/pin/pattern/face/fingerprint from the device / OS

Yes, so people are used to using a PIN or Biometrics to access part of an app. For some reason, this design added pin / biometric then another PIN...everyone hates it. Its unnecessary.

We may add Biometrics later but this has been a pain for some time....also we want parity with Litewallet iOS where the user uses app PIN or Biometrics

Copy link
Contributor

@andhikayuana andhikayuana left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@kcw-grunt
Copy link
Contributor Author

also I have an assumption that this crash maybe about not using Fragment from androidx, because app.fragment deprecated

Yes, I think part it is we never managed the feature and it is 7 years old. So, that could be one problem.

@kcw-grunt
Copy link
Contributor Author

also I have an assumption that this crash maybe about not using Fragment from androidx, because app.fragment deprecated
More work for v2.13.X! : #261

@kcw-grunt kcw-grunt force-pushed the techdebt/remove-auth-prompt branch from 08f5f83 to 593b0b1 Compare November 8, 2024 13:03
Copy link
Contributor

@josikie josikie left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Awesome. Thank you! @kcw-grunt

josikie and others added 5 commits November 30, 2024 23:19
Signed-off-by: kcw-grunt <mrkerrywashington@icloud.com>
Signed-off-by: kcw-grunt <mrkerrywashington@icloud.com>
- Removed the fingerprint xml
- Removed the Fingerprint Fragment and java files
- Removed the enum referencing the fingerprint prompt
- Removed check of fingerprint enrollment and checks
- Removed biometrics and fingerprint permissions

Signed-off-by: kcw-grunt <mrkerrywashington@icloud.com>
code and version bump

Signed-off-by: kcw-grunt <mrkerrywashington@icloud.com>
@andhikayuana andhikayuana force-pushed the techdebt/remove-auth-prompt branch from bc7afe0 to d2f4823 Compare November 30, 2024 16:22
@kcw-grunt
Copy link
Contributor Author

Hi @kcw-grunt do we have a plan to use biometric/fingerprint again?

I have experience implementing passkeys in my previous company, did you know about passkeys? with passkeys, this is will not happen

In Litewallet, one of the many complaints of our users is they can forget their device PIN and lose access to their
Litecoin. In this case, the user must remember that and the app PIN.

because it will use password/pin/pattern/face/fingerprint from the device / OS
No. But, when we are in the refactor / feature phases...we should discuss @andhikayuana

@kcw-grunt kcw-grunt merged commit 1407df0 into develop Nov 30, 2024
2 checks passed
@kcw-grunt kcw-grunt deleted the techdebt/remove-auth-prompt branch November 30, 2024 20:02
kcw-grunt added a commit that referenced this pull request Dec 19, 2024
* 🚀[Release v2.11.1 071024] Merge into Develop (#245)

* replaced the control in PeerManager.c

Signed-off-by: kcw-grunt <mrkerrywashington@icloud.com>

* reverted BRBitcoinAmount

Signed-off-by: kcw-grunt <mrkerrywashington@icloud.com>

* Cleaning up the files

Signed-off-by: kcw-grunt <mrkerrywashington@icloud.com>

* Removed Pusher remnants

Signed-off-by: kcw-grunt <mrkerrywashington@icloud.com>

---------

Signed-off-by: kcw-grunt <mrkerrywashington@icloud.com>

* Added Fiat feature, and FCM for push notifications (#247)

* Added fiat feature based on language user chose

* added fcm for push notifications

* Update PushNotificationService.kt

* Added new Icon, removed duplicate fcm library, added in app messaging

* Added new Icon, removed duplicate fcm library, added in app messaging

* Tech debt/add af sdk (#248)

* Recent newline

Signed-off-by: kcw-grunt <mrkerrywashington@icloud.com>

* AF added

Refactored cruft

- AF working

Signed-off-by: kcw-grunt <mrkerrywashington@icloud.com>

---------

Signed-off-by: kcw-grunt <mrkerrywashington@icloud.com>

* Techdebt/refactor brevents syncmarkers (#254)

* Tech debt/add af sdk (#248)

- AF working
- Changed requiredActivity
- Added analytics error report
- test this.Activity is null or not
- Bugfix - Phrase Reminder crash
- added an exception handler for UserNotAuthenticatedException.
		 - note: this should allow for the system to display the native authorization UI when needed
		 - fixes issue - https://console.firebase.google.com/u/0/project/litewallet-beta/crashlytics/app/android:com.loafwallet/issues/09dac17241309f0e823ef597a9a82cd4
- Added dev note
- remove calls to BREventManager
- removed BREventManager
- renamed error message
- fixed Firebase Analytics event error

Signed-off-by: kcw-grunt <mrkerrywashington@icloud.com>

* version bump

update gitignore

Signed-off-by: kcw-grunt <mrkerrywashington@icloud.com>

---------

Signed-off-by: kcw-grunt <mrkerrywashington@icloud.com>

* Techdebt/ fixed send issue add syncing measurements (#255)

* removed unused BRSharedPreferences

Signed-off-by: kcw-grunt <mrkerrywashington@icloud.com>

* started commenting out more Timber

Signed-off-by: kcw-grunt <mrkerrywashington@icloud.com>

* Added more shared preferences for syncing

Signed-off-by: kcw-grunt <mrkerrywashington@icloud.com>

* Commented out many verbose Timber logs

Signed-off-by: kcw-grunt <mrkerrywashington@icloud.com>

* WIP

Still trying to figure out the eplased time and last and start timestamps…???
I dunno

Signed-off-by: kcw-grunt <mrkerrywashington@icloud.com>

* Moved sync measurment into the method calls

Signed-off-by: kcw-grunt <mrkerrywashington@icloud.com>

* Debugged the install issue with send

- Moved all the measurements in new methods
- recalculated measure points
- Added analytics
- event did_complete_sync was shown!
- adding a dummy file to make sure file system is set
||||WIP still verifying the JSON is present
- added a blockheight label
-  changed the sleep time (a hack to allow the device time to refresh the view) to 100 ms from 500ms!!
-WIP Why is the bundle not showing on time
Signed-off-by: kcw-grunt <mrkerrywashington@icloud.com>

* replaces afID to first location

Signed-off-by: kcw-grunt <mrkerrywashington@icloud.com>

* Removed redundant sync measurements

Signed-off-by: kcw-grunt <mrkerrywashington@icloud.com>

* code bump

Signed-off-by: kcw-grunt <mrkerrywashington@icloud.com>

---------

Signed-off-by: kcw-grunt <mrkerrywashington@icloud.com>

* Techdebt/test refactor cruft removal (#250)

* Removed non english seed words

-Refactor tests
- Reset AnalyticsTests

Signed-off-by: kcw-grunt <mrkerrywashington@icloud.com>

* Rename Bitcoinletter to Litecoinletters

-added tests
testLitecoinSymbolConstants
testAppExternalURLConstants
testFirebaseAnalyticsConstants

Signed-off-by: kcw-grunt <mrkerrywashington@icloud.com>

---------

Signed-off-by: kcw-grunt <mrkerrywashington@icloud.com>

* fix: [#152] make sure using Fragment, FragmentManager and FragmentTransaction from androidx (#260)

* fix: [#126] the issue came from FragmentBalanceSeedReminder.fetchSeedPhrase, so we just wrap with runCatching for now to avoid crash (#263)

* fix: [#264] add null checking for BRKeyStore.removeAliasAndFiles (#270)

* fix: [#265] add null checking, migrate viewpage… (#271)

* fix: [WIP][#265] work in progress add null checking, migrate viewpager to viewpager2 and more

* fix: [#265] finalize migration some deprecated class and implement null checking at FragmentTransactionItem

* fix: dix dismiss outside FragmentTransactionDetails

* 🚀[Release v2.12.2 20241118] Merge into Develop (#272)

* 🚀[Release v2.11.1 071024] Merge into Main (#246)

* replaced the control in PeerManager.c

Signed-off-by: kcw-grunt <mrkerrywashington@icloud.com>

* reverted BRBitcoinAmount

Signed-off-by: kcw-grunt <mrkerrywashington@icloud.com>

* Cleaning up the files

Signed-off-by: kcw-grunt <mrkerrywashington@icloud.com>

* Removed Pusher remnants

Signed-off-by: kcw-grunt <mrkerrywashington@icloud.com>

---------

Signed-off-by: kcw-grunt <mrkerrywashington@icloud.com>

* version bump

Signed-off-by: kcw-grunt <mrkerrywashington@icloud.com>

* Removed donation button

removed xml
removed fragments
removed references of the donation
removed analytical events

Signed-off-by: kcw-grunt <mrkerrywashington@icloud.com>

* fix: prevent activity close

* code bump

Signed-off-by: kcw-grunt <mrkerrywashington@icloud.com>

* enabled for Debug

Signed-off-by: kcw-grunt <mrkerrywashington@icloud.com>

* fix: [#264] add null checking for BRKeyStore.removeAliasAndFiles (#270)

* fix: [#265] add null checking, migrate viewpage… (#271)

* fix: [WIP][#265] work in progress add null checking, migrate viewpager to viewpager2 and more

* fix: [#265] finalize migration some deprecated class and implement null checking at FragmentTransactionItem

* fix: dix dismiss outside FragmentTransactionDetails

* code bump

Signed-off-by: kcw-grunt <mrkerrywashington@icloud.com>

* version bump

Signed-off-by: kcw-grunt <mrkerrywashington@icloud.com>

---------

Signed-off-by: kcw-grunt <mrkerrywashington@icloud.com>
Co-authored-by: andhikayuana <andhikayuana@gmail.com>

* revert minSDkVersion to 29 (#276)

Signed-off-by: kcw-grunt <mrkerrywashington@icloud.com>

* fix: [#258] avoid OOM with cache the result of opsAll, previously called multiple times at onBindViewHolder (#277)

* fix: [#258] avoid OOM with cache the result of opsAll, previously called multiple times at onBindViewHolder

* chore: [#258] cleanup comment

* fix: [#266] add null checking and default value based on iOS since getCurrency can produce null (#278)

* fix: [#266] add null checking and default value based on iOS since getCurrency can produce null

* chore: [#266] cleanup comment

* fix: [#274] fixing locale on android 14 and setup test using junit & mockk (#282)

* chore: [#274] initial setup test using JUnit & MockK

* chore: [#274] add more test case at LocaleHelperTest

* fix: [#274] fix default language from shared preferences

* fix: [#274] disable language split on AAB

* chore: [#274] rename method

* chore: [#126] simplify exception handling at BRKeyStore._getData and record exception to Firebase Crashlytics (#283)

* Techdebt/remove auth prompt (#256)

* Check reenter pin freeze after the auth prompt deleted.

Signed-off-by: kcw-grunt <mrkerrywashington@icloud.com>

* cleaned up deletion

Signed-off-by: kcw-grunt <mrkerrywashington@icloud.com>

* Many removals

- Removed the fingerprint xml
- Removed the Fingerprint Fragment and java files
- Removed the enum referencing the fingerprint prompt
- Removed check of fingerprint enrollment and checks
- Removed biometrics and fingerprint permissions

Signed-off-by: kcw-grunt <mrkerrywashington@icloud.com>

* removed fingerprint image

code and version bump

Signed-off-by: kcw-grunt <mrkerrywashington@icloud.com>

* chore: remove FragmentFingerprint.java since, not used and failed to build

---------

Signed-off-by: kcw-grunt <mrkerrywashington@icloud.com>
Co-authored-by: josikie <kiejosi12@gmail.com>
Co-authored-by: andhikayuana <andhikayuana@gmail.com>

* 🚀[Release v2.12.4 20241130] Merge into Develop (#286)

* 🚀[Release v2.11.1 071024] Merge into Main (#246)

* replaced the control in PeerManager.c

Signed-off-by: kcw-grunt <mrkerrywashington@icloud.com>

* reverted BRBitcoinAmount

Signed-off-by: kcw-grunt <mrkerrywashington@icloud.com>

* Cleaning up the files

Signed-off-by: kcw-grunt <mrkerrywashington@icloud.com>

* Removed Pusher remnants

Signed-off-by: kcw-grunt <mrkerrywashington@icloud.com>

---------

Signed-off-by: kcw-grunt <mrkerrywashington@icloud.com>

* 🚀[Release v2.12.2 20241118] Merge into Main (#273)

* 🚀[Release v2.11.1 071024] Merge into Develop (#245)

* replaced the control in PeerManager.c

Signed-off-by: kcw-grunt <mrkerrywashington@icloud.com>

* reverted BRBitcoinAmount

Signed-off-by: kcw-grunt <mrkerrywashington@icloud.com>

* Cleaning up the files

Signed-off-by: kcw-grunt <mrkerrywashington@icloud.com>

* Removed Pusher remnants

Signed-off-by: kcw-grunt <mrkerrywashington@icloud.com>

---------

Signed-off-by: kcw-grunt <mrkerrywashington@icloud.com>

* Added Fiat feature, and FCM for push notifications (#247)

* Added fiat feature based on language user chose

* added fcm for push notifications

* Update PushNotificationService.kt

* Added new Icon, removed duplicate fcm library, added in app messaging

* Added new Icon, removed duplicate fcm library, added in app messaging

* Tech debt/add af sdk (#248)

* Recent newline

Signed-off-by: kcw-grunt <mrkerrywashington@icloud.com>

* AF added

Refactored cruft

- AF working

Signed-off-by: kcw-grunt <mrkerrywashington@icloud.com>

---------

Signed-off-by: kcw-grunt <mrkerrywashington@icloud.com>

* Techdebt/refactor brevents syncmarkers (#254)

* Tech debt/add af sdk (#248)

- AF working
- Changed requiredActivity
- Added analytics error report
- test this.Activity is null or not
- Bugfix - Phrase Reminder crash
- added an exception handler for UserNotAuthenticatedException.
		 - note: this should allow for the system to display the native authorization UI when needed
		 - fixes issue - https://console.firebase.google.com/u/0/project/litewallet-beta/crashlytics/app/android:com.loafwallet/issues/09dac17241309f0e823ef597a9a82cd4
- Added dev note
- remove calls to BREventManager
- removed BREventManager
- renamed error message
- fixed Firebase Analytics event error

Signed-off-by: kcw-grunt <mrkerrywashington@icloud.com>

* version bump

update gitignore

Signed-off-by: kcw-grunt <mrkerrywashington@icloud.com>

---------

Signed-off-by: kcw-grunt <mrkerrywashington@icloud.com>

* Techdebt/ fixed send issue add syncing measurements (#255)

* removed unused BRSharedPreferences

Signed-off-by: kcw-grunt <mrkerrywashington@icloud.com>

* started commenting out more Timber

Signed-off-by: kcw-grunt <mrkerrywashington@icloud.com>

* Added more shared preferences for syncing

Signed-off-by: kcw-grunt <mrkerrywashington@icloud.com>

* Commented out many verbose Timber logs

Signed-off-by: kcw-grunt <mrkerrywashington@icloud.com>

* WIP

Still trying to figure out the eplased time and last and start timestamps…???
I dunno

Signed-off-by: kcw-grunt <mrkerrywashington@icloud.com>

* Moved sync measurment into the method calls

Signed-off-by: kcw-grunt <mrkerrywashington@icloud.com>

* Debugged the install issue with send

- Moved all the measurements in new methods
- recalculated measure points
- Added analytics
- event did_complete_sync was shown!
- adding a dummy file to make sure file system is set
||||WIP still verifying the JSON is present
- added a blockheight label
-  changed the sleep time (a hack to allow the device time to refresh the view) to 100 ms from 500ms!!
-WIP Why is the bundle not showing on time
Signed-off-by: kcw-grunt <mrkerrywashington@icloud.com>

* replaces afID to first location

Signed-off-by: kcw-grunt <mrkerrywashington@icloud.com>

* Removed redundant sync measurements

Signed-off-by: kcw-grunt <mrkerrywashington@icloud.com>

* code bump

Signed-off-by: kcw-grunt <mrkerrywashington@icloud.com>

---------

Signed-off-by: kcw-grunt <mrkerrywashington@icloud.com>

* Techdebt/test refactor cruft removal (#250)

* Removed non english seed words

-Refactor tests
- Reset AnalyticsTests

Signed-off-by: kcw-grunt <mrkerrywashington@icloud.com>

* Rename Bitcoinletter to Litecoinletters

-added tests
testLitecoinSymbolConstants
testAppExternalURLConstants
testFirebaseAnalyticsConstants

Signed-off-by: kcw-grunt <mrkerrywashington@icloud.com>

---------

Signed-off-by: kcw-grunt <mrkerrywashington@icloud.com>

* fix: [#152] make sure using Fragment, FragmentManager and FragmentTransaction from androidx (#260)

* fix: [#126] the issue came from FragmentBalanceSeedReminder.fetchSeedPhrase, so we just wrap with runCatching for now to avoid crash (#263)

* version bump

Signed-off-by: kcw-grunt <mrkerrywashington@icloud.com>

* Removed donation button

removed xml
removed fragments
removed references of the donation
removed analytical events

Signed-off-by: kcw-grunt <mrkerrywashington@icloud.com>

* fix: prevent activity close

* code bump

Signed-off-by: kcw-grunt <mrkerrywashington@icloud.com>

* enabled for Debug

Signed-off-by: kcw-grunt <mrkerrywashington@icloud.com>

* fix: [#264] add null checking for BRKeyStore.removeAliasAndFiles (#270)

* fix: [#265] add null checking, migrate viewpage… (#271)

* fix: [WIP][#265] work in progress add null checking, migrate viewpager to viewpager2 and more

* fix: [#265] finalize migration some deprecated class and implement null checking at FragmentTransactionItem

* fix: dix dismiss outside FragmentTransactionDetails

* code bump

Signed-off-by: kcw-grunt <mrkerrywashington@icloud.com>

* version bump

Signed-off-by: kcw-grunt <mrkerrywashington@icloud.com>

---------

Signed-off-by: kcw-grunt <mrkerrywashington@icloud.com>
Co-authored-by: Josi Kie <54074780+josikie@users.noreply.github.com>
Co-authored-by: Andhika Yuana <andhikayuana@gmail.com>

* bumped version and code

* compiled the env (#284)

please check @andhikayuana @josikie  that you are able to compile

* Users complained that recyclerView was setting to the wrong language

- Polished to index to a language
- bump code

* chore: [#126] simplify exception handling at BRKeyStore._getData and record exception to Firebase Crashlytics (#283)

---------

Signed-off-by: kcw-grunt <mrkerrywashington@icloud.com>
Co-authored-by: Josi Kie <54074780+josikie@users.noreply.github.com>
Co-authored-by: Andhika Yuana <andhikayuana@gmail.com>

* Added abilities to copy transaction id by click the transaction id (#290)

* Added abilities to copy transaction id by click the transaction id

* Update build.gradle

* version and code bump

* Unit Test(s) Migration (#287)

* chore: [test] add BRConstantsTest for unitTest and deprecated ConstantsTests inside androidTest

* chore: [test] remove unused test files

* chore: [test] add todo to fill the tests and change package name

* chore: [test] add basic example

* chore: [test] integrate with ci/cd

* chore: [test][ci] fix Cannot find a definition for command named android/restore-gradle-cache

* chore: [test][ci] fix Cannot find a definition for command named android/run-tests

* chore: [test][ci] fix Unexpected argument(s): test-command

* chore: [test][ci] fix Cannot find a definition for executor named android/android-docker

* chore: [test][ci] add default for gradle.properties

* chore: [test][ci] avoid breaking in build.gradle

* chore: [test][ci] enable androidx and jetifier

* chore: [test][ci] init submodule

* chore: [test][ci] just using run command for execute unit test step

* chore: [test][ci] add env for google-services.json

* chore: [test][ci] add env for google-services.json

* chore: [test][ci] add env for google-services.json

* Major refactor of code to move classes to the ideal location

- Removed cruft
- Moved all tests to the unitTest directory
- Remove poorly placed tests into (AndroidTest)
- Commented  old code into the new Kotlin class(es)
- working in the first test of currency
- Removed the paymentRequest method
- Renamed Classes

* chore: resolve CurrencyTests

* Added notes

---------

Co-authored-by: Kerry Washington <mrkerrywashington@icloud.com>

* feat: remote config feature toggle (#293)

* feat: [remote-config] wip for remote config feature toggle

* feat: [remote-config] implement base url toggle for prod and dev

* feat: [test] add BRApiManagerTest

* fix: fix getBaseUrlProd for FragmentBuy and add test

* chore: change BRApiManager.fetchRates using getBaseUrlProd

* version and code bump

* Update .gitignore

---------

Signed-off-by: kcw-grunt <mrkerrywashington@icloud.com>
Co-authored-by: Josi Kie <54074780+josikie@users.noreply.github.com>
Co-authored-by: Andhika Yuana <andhikayuana@gmail.com>
Co-authored-by: josikie <kiejosi12@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
🐛 bug Something isn't working as expected 🦺 tech debt Actions that help maintain the repo and application
Projects
None yet
Development

Successfully merging this pull request may close these issues.

🐛[Bug Report]: AuthPrompt is the source of many crashes
3 participants