Closed bom-bakbang closed 1 month ago
@bom-bakbang thank you for providing detailed info and your contribution, I requested some small changes. Please, check them
@bom-bakbang thank you for providing detailed info and your contribution, I requested some small changes. Please, check them
Thanks for detail code review. Sorry for late correction changes, I got accident so I couldn't focus 😅. I changed all your comments, please check again :)
Hi! I'm Lukas, a South Korean Android Developer. I tried using KMPAuth and find it very useful! Thanks for your work.
As far as I know, I could find there exists only one option for GoogleAuthCredentials:
serverId
, which is used for buildingGetGoogleIdOption
while other options are fixed in your android implementation like below.https://github.com/mirzemehdi/KMPAuth/blob/2d8c62be7ea991aba021b306039f3f1a260f95eb/kmpauth-google/src/androidMain/kotlin/com/mmk/kmpauth/google/GoogleAuthUiProviderImpl.kt#L79-L84
However, there exists an issue for crash in
Credentials
when you set other options like above: https://stackoverflow.com/questions/78538579/new-google-credential-manager-is-throwing-a-transactiontoolargeexceptionI also got similar issue related to
TransactionTooLargeException
, because I already had SAMSUNG Credentials. So I could solve the problem by set.setFilterByAuthorizedAccounts(true)
first for filtering out SAMSUNG Credentials.Also, when you see official recommendation at https://developer.android.com/identity/sign-in/credential-manager-siwg#enable-sign-up, they guide to check whether there exist previously used accounts by set
.setFilterByAuthorizedAccounts(true)
first. If there is no available accounts, it will throwNoCredentialException
.So I suggest adding flow for checking previously used accounts. This is my first time to participate open source, so please let me know if there exist more steps I should know.
Sincerly, Lukas.