Skip to content

Commit

Permalink
Stop caching preferences instance (#20445)
Browse files Browse the repository at this point in the history
It appears that reusing the shared preferences instance sometimes causes changes to not be properly committed to disk as the instance is not disposed in some cases before the app is torn down (eg: force close).

Caching isn't really necessary here anyways as the underlying android preference manager does its own layer of caching (so we are just paying the interop tax to get a 'new' instance here).

The simplest fix is to stop caching which is the real content of this change.  We don't cache in the regular preferences API where we use shared preferences and the issue does not seem to exist there.
  • Loading branch information
Redth authored Feb 9, 2024
1 parent 5c29eaa commit bbbbfe7
Showing 1 changed file with 17 additions and 16 deletions.
33 changes: 17 additions & 16 deletions src/Essentials/src/SecureStorage/SecureStorage.android.cs
Original file line number Diff line number Diff line change
Expand Up @@ -15,9 +15,11 @@ Task<string> PlatformGetAsync(string key)
{
try
{
ISharedPreferences sharedPreferences = GetEncryptedSharedPreferences();
if (sharedPreferences != null)
return sharedPreferences.GetString(key, null);
var prefs = GetEncryptedSharedPreferences();
if (prefs != null)
{
return prefs.GetString(key, null);
}

// TODO: Use Logger here?
System.Diagnostics.Debug.WriteLine(
Expand Down Expand Up @@ -48,53 +50,52 @@ Task PlatformSetAsync(string key, string data)
{
return Task.Run(() =>
{
using ISharedPreferencesEditor editor = GetEncryptedSharedPreferences()?.Edit();
using var prefs = GetEncryptedSharedPreferences();
using var editor = prefs?.Edit();
if (data is null)
{
editor?.Remove(key);
}
else
{
editor?.PutString(key, data);
}

editor?.Apply();
});
}

bool PlatformRemove(string key)
{
using ISharedPreferencesEditor editor = GetEncryptedSharedPreferences()?.Edit();
using var prefs = GetEncryptedSharedPreferences();
using var editor = prefs?.Edit();
editor?.Remove(key)?.Apply();
return true;
}

void PlatformRemoveAll()
{
using var editor = PreferencesImplementation.GetSharedPreferences(Alias).Edit();
using var prefs = GetEncryptedSharedPreferences();

This comment has been minimized.

Copy link
@gerhartz

gerhartz May 23, 2024

@Redth I think this change here is related to #18230

On dotnet8 doing an uninstall/reinstall causes GetEncryptedSharedPreferences() to throw but then you can't remove the keys anymore because PlatformRemove now calls GetEncryptedSharedPreferences. So we're kind of stuck with a bad secure storage implementation with no workaround.

using var editor = prefs?.Edit();
editor?.Clear()?.Apply();
}

ISharedPreferences _prefs;
ISharedPreferences GetEncryptedSharedPreferences()
{
if (_prefs is not null)
{
return _prefs;
}

try
{
var context = Application.Context;

MasterKey prefsMainKey = new MasterKey.Builder(context, Alias)
var prefsMainKey = new MasterKey.Builder(context, Alias)
.SetKeyScheme(MasterKey.KeyScheme.Aes256Gcm)
.Build();

var sharedPreferences = EncryptedSharedPreferences.Create(
return EncryptedSharedPreferences.Create(
context,
Alias,
prefsMainKey,
EncryptedSharedPreferences.PrefKeyEncryptionScheme.Aes256Siv,
EncryptedSharedPreferences.PrefValueEncryptionScheme.Aes256Gcm);

return _prefs = sharedPreferences;
}
catch (InvalidProtocolBufferException)
{
Expand Down

0 comments on commit bbbbfe7

Please sign in to comment.