xamarin / Xamarin.Auth

Xamarin.Auth
Apache License 2.0
541 stars 351 forks source link

Crash on Android AccountStore Migration #291

Closed gwelshons closed 5 years ago

gwelshons commented 6 years ago

Application crashes when trying to migrate default password to new user set password in AccountStore.

The reason is because the hard coded password is actually empty when it is used. So the old password is not actually the hardcoded one but actually "System.Char[]".

reense commented 6 years ago

Is this the exception you're getting?


Java.IO.IOException: KeyStore integrity check failed.

If yes, we're in the same boat.

gwelshons commented 6 years ago

Yeah. The temporary fix is to pass in "System.Char[]" as your password. See this xamarin forum post for a little more detail. https://forums.xamarin.com/discussion/comment/328887#Comment_328887

reense commented 6 years ago

@gwelshons that worked. Thanks! Any idea what the long term solution would be?

ChristopherStephan commented 6 years ago

Passing "System.Char[]" as the password did not prevent this error in my case (using Xamarin.Auth version 1.6.0.2).

mfj commented 6 years ago

This also affects Xamarin.Forms version of Auth, only workaround seems to be to downgrade to 1.5.x.

PaulVrugt commented 6 years ago

The problem is that in 1.6.0.2, the default password was changed from "System.Char[]" to the actual system password. This prevents accountstores to be migrated as of 1.6.0.2.

@moljac We currently have xamarin.forms apps that still use the default password. However, the xamarin.forms "AccountStore.Create" method doesn't have an overload that supports a password. So how can we ever migrate?

RandallCoy commented 6 years ago

We are facing the same conundrum as @PaulVrugt and are trying to understand a way forward.

ansydor commented 6 years ago

Solution is there https://github.com/xamarin/Xamarin.Auth/blob/master/source/Core/Xamarin.Auth.XamarinAndroid/AccountStore/AndroidAccountStore.cs#L100

PaulVrugt commented 6 years ago

@ansydor please elaborate.

RandallCoy commented 6 years ago

Yes, @ansydor we need more info. Here is more details about our situation: We have a .NET Standard 2.0 project with Xamarin.Auth in it and which uses Xamarin.Auth like this: var accountStore = AccountStore.Create(); notice there are no parameters. We also then have our Xamarin Android project which uses the .NET Standard project (but does not have Xamarin.Auth specific code). The .NET Standard 2.0 project does not complain that Xamarin.Auth.AccountStore.Create() is obsolete. If I put Xamarin.Auth.AccountStore.Create() into our Xamarin Android project (for testing) then it complains that the method without parameters is obsolete. So something is not lining up (possibly with the Nuget packages??) but I do not know where. Any ideas? Let me know if you need more info on our projects.

bulentb commented 6 years ago

I have changed my app name and related things then clean and build solved.

ansydor commented 6 years ago

U can run on a new AVD or uninstall application fully, and then install it again, but if u cant get an access u need to try the password "3295043EA18CA264B2C40E0B72051DEF2D07AD2B4593F43DDDE1515A7EC32617" But u cant uninstall on real devices, some kind of migration from old password to new is needed

PaulVrugt commented 6 years ago

@ansydor Of course we cannot uninstall on real devices, since our application is installed at thousands of end users. We cannot ask them all to reinstall the app.

We first need a change that gives xamarin.forms users access to the overload of the accountstore with a password

PaulVrugt commented 6 years ago

@moljac It would be nice to get some comment from you on this issue

moljac commented 6 years ago

@PaulVrugt Sorry for delay, but I was moved to AndroidSupportLibraries and GooglePlayServices, because we are outnumbered and new Google stuff is huge, so I work on Xamarin.Auth with reduced priority and performance.

This issue is my No. issue to be fixed. When? Not sure. I hope next week.

TamasRusvai commented 6 years ago

@moljac Hello, Any progress with this bug?

HappyTreesDev commented 6 years ago

Can we somehow change the 1.6.0.2 nuget package to beta or remove it. I recently forgot about this issue and updated the package only to have to downgrade it again because this is still not fixed but It makes me look like I need an update. Very frustrating!

PaulVrugt commented 6 years ago

@moljac

It's been two months now without any progress. Any update?

CNouws commented 6 years ago

@moljac Any update on this issue?

alexshikov commented 6 years ago

Oh man, this is an unexpected surprise of the today :)

Issue reason

Let's summarize the issue. The breaking change from the 1.5.x to 1.6.x in terms of the current issue is on the line https://github.com/xamarin/Xamarin.Auth/blob/d592c51a6f91aa073f0c1be8a768c464aa6e62a7/source/Core/Xamarin.Auth.XamarinAndroid/AccountStore/AndroidAccountStore.cs#L53

The change happened from

public AndroidAccountStore(Context context)
            : this(context, PasswordHardCoded.ToString())
{}

to

public AndroidAccountStore(Context context)
            : this(context, new string(PasswordHardCoded))
{}

As we see, PasswordHardCoded.ToString() actually returns System.Char[], that's why the proposed fix from the forum https://forums.xamarin.com/discussion/comment/328887/#Comment_328887 works for the most cases.

Workaround

Though, the problem is lots of people including me use the plugin in the PCL project where AccountStore.Create(string password) is not available.

As a workaround, a new factory method should be used with an injection function.

public static class AccountStoreFix
{
    // Replace default construction with Android fix in the MainActivity.cs  
    public static Func<AccountStore> CreateFunc = AccountStore.Create;

    public static AccountStore Create ()
    {
        return CreateFunc ();
    }
}

In the MainActivity inject new create function

AccountStoreFix.CreateFunc = () => Xamarin.Auth.AccountStore.Create ("System.Char[]");

Now in the core project AccountStoreFix.Create(); should be used instead of AccountStore.Create();

A bit more detailed example can be found here

PaulVrugt commented 6 years ago

@moljac

Although @alexshikov 's workaround is valid. The issue is a bit more complex. The actual fix should include a way in the plc (standard) project to set the password for the keystore, because we should not be using the default password. However, this will break all implementations, because the migratekeystore method that moves the keystore to a new keystore with the new password uses the ACTUAL default password as a source for the migration instead of "System.Char[]" that was used because of the bug.

This means that the final fix for this issue should also include a change in the MigrateKeyStore method to also attempt to migrate the keystore with "System.Char[]" as password.

alexshikov commented 6 years ago

Agree. I also think if there should be options to deal with migration failure:

So that we have an opportunity to continue the application because right now the app just stuck in a situation when the AccountStore can't be used at all.

gwelshons commented 6 years ago

How I have temporarily fixed it was to pull down my own copy of the Droid Xamarin.Auth. Then in AndroidAccountStore I added a second passwordHardCoded.

static readonly char[] PasswordHardCoded = "System.Char[]".ToCharArray();
static readonly char[] PasswordHardCoded2 = "3295043EA18CA264B2C40E0B72051DEF2D07AD2B4593F43DDDE1515A7EC32617".ToCharArray();

Then in the code that does the migration if failed I added another try catch to try again if the first hardcoded password fails.

if (ex.Message == "KeyStore integrity check failed.") {
     // Migration scenario: this exception means that the keystore could not be opened
     // with the app provided password, so there is probably an existing keystore
     // that was encoded with the old hard coded password, which was deprecated.
     // We'll try to open the keystore with the old password, and migrate the contents
     // to a new one that will be encoded with the new password.
     try {
          MigrateKeyStore( context, PasswordHardCoded );
     } catch {
          MigrateKeyStore( context, PasswordHardCoded2 );
     }
}
tharanagaraj commented 6 years ago

Is downgrading Xamarin.Auth to 1.5.x a temporary solution to this? Does that version cause the 'KeyStore was not initialized' and/or 'Uninitialized keystore' issue?

newky2k commented 5 years ago

Xamarin.Auth 1.6.0.3 has now been published to Nuget

mauritsb commented 5 years ago

As @PaulVrugt mentioned before, the "AccountStore.Create" method should have an overload that changes the default password. This issue should remain open.