search

xamarin / Xamarin.Auth

Xamarin.Auth
Apache License 2.0
542 stars 352 forks source link

FindAccountsForService throws on iOS #370

Closed Kukkimonsuta closed 5 years ago

Kukkimonsuta commented 5 years ago

Version

Steps to reproduce

var accountStore = AccountStore.Create();
var accounts = accountStore.FindAccountsForService("someid");

Expected behaviour

Not throw.

Actual behaviour

Exception is thrown:

Search/Find FindAccountsForServiceAsync Object reference not set to an instance of an object
  at Xamarin.Auth.KeyChainAccountStore.FindAccountsForServiceAsync (System.String serviceId) [0x0007f] in <85539ee049364f3e8d64880a4a42b589>:0 
  at Xamarin.Auth.KeyChainAccountStore.FindAccountsForService (System.String serviceId) [0x00000] in <85539ee049364f3e8d64880a4a42b589>:0 
  at ShopApp.iOS.Platform.IdentityStorage.Delete () [0x00007] in D:\Projects\shopapp-mobile\src\ShopApp.iOS\Platform\IdentityStorage.cs:117 
  at ShopApp.iOS.AppDelegate.CheckApplicationFirstLaunch (ShopApp.App application) [0x0001a] in D:\Projects\shopapp-mobile\src\ShopApp.iOS\AppDelegate.cs:111 
  at ShopApp.iOS.AppDelegate.FinishedLaunching (UIKit.UIApplication app, Foundation.NSDictionary options) [0x00034] in D:\Projects\shopapp-mobile\src\ShopApp.iOS\AppDelegate.cs:34

Inner exception:

Object reference not set to an instance of an object
  at Xamarin.Auth.KeyChainAccountStore+<>c.<.cctor>b__12_2 () [0x00042] in <85539ee049364f3e8d64880a4a42b589>:0 
  at System.Lazy`1[T].ViaFactory (System.Threading.LazyThreadSafetyMode mode) [0x00043] in <56d44aabfc8d4a488316d98c7b91b0b0>:0 
  at System.Lazy`1[T].ExecutionAndPublication (System.LazyHelper executionAndPublication, System.Boolean useDefaultConstructor) [0x00022] in <56d44aabfc8d4a488316d98c7b91b0b0>:0 
  at System.Lazy`1[T].CreateValue () [0x00074] in <56d44aabfc8d4a488316d98c7b91b0b0>:0 
  at System.Lazy`1[T].get_Value () [0x0000a] in <56d44aabfc8d4a488316d98c7b91b0b0>:0 
  at Xamarin.Auth.KeyChainAccountStore.FindAccountsForServiceAsync (System.String serviceId) [0x00030] in <85539ee049364f3e8d64880a4a42b589>:0

I've tracked the issue down to https://github.com/xamarin/Xamarin.Auth/blob/b4fb1bb5c409e281c6bffad74ea8caba87805ea8/source/Core/Xamarin.Auth.XamarinIOS/AccountStore/KeyChainAccountStore.Aync.cs#L63-L69 where GetField("True") returns null.

VS bug #838824

ghost commented 5 years ago

+1 to this. Seems like an issue with Xamarin.iOS SDK version 12.6 (preview). Downgrading Xamarin.iOS to version 12.2 seems to work.

cklenk commented 5 years ago

I got this same error today using VS 2019 Preview v4.1. I will try downgrading the iOS sdk to 12.2.

Thanks!

berhir commented 5 years ago

Same issue here with VS 2019 Preview 4.1 SVC1 and Xamarin.iOS 12.6.0.23. Is there any workaround besides downgrading?

dk-mushiyoke commented 5 years ago

I am able to reproduce this on the official VS 2019 release that came out earlier today (version 8.0 build 3001) with Xamarin iOS 12.6.0.25, I am using the latest Xamarin.Auth version which is 1.6.0.4.

Exception log: System.AggregateException: One or more errors occurred. (Search/Find FindAccountsForServiceAsync Object reference not set to an instance of an object) ---> Xamarin.Auth.AccountStoreException: Search/Find FindAccountsForServiceAsync Object reference not set to an instance of an object ---> System.NullReferenceException: Object reference not set to an instance of an object at Xamarin.Auth.KeyChainAccountStore+<>c.<.cctor>b__12_2 () [0x00042] in <85539ee049364f3e8d64880a4a42b589>:0 at System.Lazy1[T].ViaFactory (System.Threading.LazyThreadSafetyMode mode) [0x0001c] in :0 --- End of stack trace from previous location where exception was thrown ---

at System.LazyHelper.ThrowException () [0x00000] in /Library/Frameworks/Xamarin.iOS.framework/Versions/12.6.0.25/src/Xamarin.iOS/external/corefx/src/Common/src/CoreLib/System/Lazy.cs:97 at System.Lazy1[T].CreateValue () [0x0007e] in <f6fd5b6dc94346cb9547cf7dbf9aa8b2>:0 at System.Lazy1[T].get_Value () [0x0000a] in :0 at Xamarin.Auth.KeyChainAccountStore.FindAccountsForServiceAsync (System.String serviceId) [0x00030] in <85539ee049364f3e8d64880a4a42b589>:0 --- End of inner exception stack trace --- at Xamarin.Auth.KeyChainAccountStore.FindAccountsForServiceAsync (System.String serviceId) [0x0007f] in <85539ee049364f3e8d64880a4a42b589>:0 at Xamarin.Auth.KeyChainAccountStore.FindAccountsForService (System.String serviceId) [0x00000] in <85539ee049364f3e8d64880a4a42b589>:0 `

libin85 commented 5 years ago

having the same issue with the latest visual studio 2019 release

ali-h2010 commented 5 years ago

I started getting the issue after Visual Studio 2019 Update. Xcode and Xamarin are all up to date.

santihcc commented 5 years ago

+1 having the same issue since this morning. In the meantime, any help where I can find the installer to downgrade Xamarin.iOS to version 12.2?

sbajtl commented 5 years ago

I have same issue this morning. One solution is to downgrade vs to 2017, I think you can't downgrade only Xamarin.iOS, if someone success, please share solution. Another solution is to use SecureStorage from Xamarin.Essentials instead Xamarin.Auth account store.

Redth commented 5 years ago

Folks, we'll take a look at this one, but the correct way forward is to switch to SecureStorage from Xamarin.Essentials as we have deprecated the AccountStore in Xamarin.Auth.

Redth commented 5 years ago

Also here's a migration guide to switch to SecureStorage in Xamarin.Essentials: https://github.com/xamarin/Xamarin.Auth/wiki/Migrating-from-AccountStore-to-Xamarin.Essentials-SecureStorage

motoko89 commented 5 years ago

@Kukkimonsuta Did you manage to fix that line? If yes, could you share your repo?

wayousuf commented 5 years ago

@someone1984 don't need to downgrade the VS2019, just downgrade the Xamarin.IOS version to 12.2.1.16, here is the link https://dl.xamarin.com/MonoTouch/Mac/xamarin.ios-12.2.1.16.pkg

sbajtl commented 5 years ago

@wayousuf thanks for your suggestion, I tried it right now and it's working. But anyway maybe in future, best solution is replace Xamarin.Auth AccountStore with SecureStorage.

sillerjp commented 5 years ago

Hi @Redth

I'm migrating to secure storage, in Android, I'm already overriding OnRequestPermissionsResult for certain activities using custom request codes. Does this matter when setting Xamarin.Essentials.Platform.OnRequestPermissionsResult(requestCode, permissions, grantResults);

Apologies if this is not the right spot to ask this but thought it was good for anyone that wants to migrate too instead of downgrading.

motoko89 commented 5 years ago

Folks, we'll take a look at this one, but the correct way forward is to switch to SecureStorage from Xamarin.Essentials as we have deprecated the AccountStore in Xamarin.Auth.

@Redth Does SecureStorage use iOS keychain and Android Keystore? Why's there the need for migration? Also I don't think we can even migrate because this function is crashing

aleksandrsmyk commented 5 years ago

I have the same issue. When it will be fixed?

ali-h2010 commented 5 years ago

I moved to Xamarin Essentials. The only concern was that there are no Account class. You just create a dictionary of keys and strings. But you can always create your own handlers.

jcampana commented 5 years ago

Hey folks, how do you migrate to SecureStorage if AccountStore.Create().FindAccountsForService(); throws exception? I am trying to migrate but with this crash is impossible...

ali-h2010 commented 5 years ago

@jcampana, I didn't migrate. I am actually not sure which scenario will require you to transfer the old keys to a new app. Usually, you ship a new clean app without any old info.

What i did is install the plugin and follow the api to store, retrieve, and delete the keys.

moljac commented 5 years ago

This code was fix/workaround for old bug in iOS. When will it be fixed?

I'm working on it. Currently trying to resurrect old projects so I can build the code.

Regarding AccountStore OAuth does not specify anything about storing tokens. Storing access_token is BAD security practice, so the vNext version of Xamarin.Auth will not have AccountStore for sure. Maybe in something like Xamarin.Auth.Extensions (or Extras) and it will use for sure Essentials.

ShravanJ commented 5 years ago

Downgrading to Xamarin.iOS 12.2 worked for me as per @wayousuf’s suggestion but I agree, this is just a temporary fix.

sillerjp commented 5 years ago

Is it only AccountStore that is being deprecated or the whole Xamarin.Auth library? It seems SecureStorage uses Account class from Xamarin.Auth so that was confusing. From where to where are the accounts migrated?

moljac commented 5 years ago

@JPSiller SecureStorage does not use anything from Xamarin.Auth.

The design goal of Essentials was to have 0 or close to 0 dependencies. Account class is there, because Essentials drew few ideas from older libraries.

Xamarin.Auth was designed and implemented in 2012/2013. Reworked in 2016. It will be minimally maintained (bugfixin), but that is almost all.

sillerjp commented 5 years ago

Hi @moljac

Then does Xamarin.Essentials has it's own Account class?

From the migration guide https://github.com/xamarin/Xamarin.Auth/wiki/Migrating-from-AccountStore-to-Xamarin.Essentials-SecureStorage.

From the migration guide: "Now, instead, using the helper class you'd write:

await SecureStorageAccountStore.SaveAsync(account, "Facebook");"

That method uses the Xamarin.Auth account class

moljac commented 5 years ago

SecureStorageAccountStore in the doc is wrapper for both Xamarin.Auth.Account and Xamarin.Essentials.SecureStorage. Check the snippet in the doc. Xamarin.Auth.AccountStore from

https://github.com/xamarin/Xamarin.Auth/tree/master/source/Core/Xamarin.Auth.Common.LinkSource/AccountStore

is used only to migrate existing Xamarin.Auth accounts to new storage in Essentials. Essentials use SetAsync(string, string), so the trick is to serialze Account to JSON.

sillerjp commented 5 years ago

@moljac Ok got it, thanks for the explanation so helper class is completely for migration.

After that you can use the SecureStorage directly. To migrate in iOS we will need to wait for the fix for this thread right?

After that, once all our users migrate to a version that utilizes secure storage we can safely remove Xamarin.Auth completely in the our next app release. Is that correct?

moljac commented 5 years ago

No problems at all. That's the reason we are here.

To migrate in iOS we will need to wait for the fix for this thread right?

I think so. But good new. I dusted Xamarin.Auth off, upgraded nugets and everything works. I was a bit afraid of this step. I'm testing right now, so patch/fix might be out soon

sillerjp commented 5 years ago

@moljac Awesome, thanks a lot.

ngalicoco commented 5 years ago

@moljac So if the fix for Xamarin.Auth is out, do we still need to use Secure Storage with migration helper class, or we can do it later when we upgrade our solution to Xamarin.Essentials secure storage ?

tipa commented 5 years ago

I would also appreciate a quick hotfix for this issue as it currently blocks me from releasing an app update. Would love to migrate to SecureStorage but as previously mentioned, it won't work with the current version

markradacz commented 5 years ago

+1 for now downgrading the Xamarin.IOS version to 12.2.1.16 worked

sschmidTU commented 5 years ago

@tipa you can hotfix the issue by downgrading the IOS SDK from 12.6 to 12.2, as others wrote before. Just download and execute (install) this: https://dl.xamarin.com/MonoTouch/Mac/xamarin.ios-12.2.1.16.pkg (578 MB) (it's not necessary to downgrade anything else, like Visual Studio 2019, i tested this.)

@moljac thanks for working on a fix! even though Xamarin.Auth is deprecated and migrating to SecureStorage is the better long term fix, fixing Auth could give some QOL for existing projects.

hvaughan3 commented 5 years ago

@JPSiller You will want to keep Xamarin.Auth migration code until you can be sure that some majority of users is off of any of the versions that previously used AccountStore.

If a user skips the one version that has the AccountStore to Essentials migration code and instead updates to the version after that, the migration code would be gone and the app would no longer work for that user.

sillerjp commented 5 years ago

@hvaughan3 That's a good point, I think we'll have to leave it for a while until our analytics tell us 99% of our users are using a secure storage version as you suggested. Thanks.

Hope the fix is available soon

grantkiwi commented 5 years ago

Do we have an ETA for a fix here? How can I downgrade the iOS from 12.6 to 12.2?

grantkiwi commented 5 years ago

I installed the hotfix: https://dl.xamarin.com/MonoTouch/Mac/xamarin.ios-12.2.1.16.pkg

then I got an error about a System.Buffers Filenotfound exception

I'm using VS 2019 on PC (SSH to mac mini)

Vijay4657 commented 5 years ago

I downgraded the iOS version to 12.0 and 9.0 as well. Still facing the same issue. Any help would be really great.?

softsan commented 5 years ago

@rickyaare - so when will the new nuget package out? any ETA? thanks!

rickykaare commented 5 years ago

@softsan, I think we need someone from the team approving the pull-request and do the release.

I was facing the issue with FindAccountsForService in an old project, and found the suggested fix after some debugging.

While we are waiting for a release of Xamarin.Auth you can build the cfboolean-truehandle branch yourself and reference this instead of the Xamarin.Auth NuGet package.

rickykaare commented 5 years ago

ping @moljac

softsan commented 5 years ago

@rickykaare - Thanks.

grantkiwi commented 5 years ago

I have just taken easy way and gone back to VS 2017 and working fine

ajaybhandiwad commented 5 years ago

@moljac - Thanks a lot.

moljac commented 5 years ago

Dear community I beg you for a little bit patience, due to the fact that all artifacts (nugets) must be published via CI system (rules).

And of course due to our other tasks and obligations Xamarin.Auth code is a bit stale, so I need to fix Cake scripts and create Azure DevOps Pipelines. I will add support for VS2019 later on.

Thanks for understanding.

lewixlabs commented 5 years ago

@someone1984 don't need to downgrade the VS2019, just downgrade the Xamarin.IOS version to 12.2.1.16, here is the link https://dl.xamarin.com/MonoTouch/Mac/xamarin.ios-12.2.1.16.pkg

@wayousuf's solution is the best, for those (like me) having in apps in production already, and cannot risk implementing last minute (great for the future) solution proposed by @Redth https://github.com/xamarin/Xamarin.Auth/issues/370#issuecomment-479491900

Please Xamarin team fix this issue on iOS SDK stable channel.

Hi! Lewix

sillerjp commented 5 years ago

@lewixlabs Hi! The solution has already been implemented, this is not a Xamarin.iOS issue but rather the Xamarin.Auth library needed to be upgraded to be compatible with the latest Xamarin.iOS.

Right now, the only thing left to do is for the Xamarin.Auth nuget package to be released so you can easily install it into your projects but per the author some scripts need to be updated so it may take a little bit more to be available.

If you really need this fix you can checkout the branch and compile the library yourself and you would be able to add the dll manually.

You can see what was implemented for the fix in here: https://github.com/xamarin/Xamarin.Auth/pull/376

lewixlabs commented 5 years ago

Thank you @JPSiller !

mjoconnor1985 commented 5 years ago

I'm using Xamarin.iOS 12.2.1.12 and I'm still having the problem.

gayko1976 commented 5 years ago

Problem still exists with latest SDK, Forms and Auth... Please fix asap. These kind of bugs are annoying...

taublast commented 5 years ago

Having 2 apps to be published that are now just crashing, call this annoying..