From 99de53d374aa7e07ab3bce6f322e322ea18739b5 Mon Sep 17 00:00:00 2001 From: James Montemagno Date: Thu, 3 May 2018 14:42:53 -0700 Subject: [PATCH 1/4] Address #221, use AfterFirstUnlockThisDeviceOnly to match other platforms. Although allow a platform specific override to set accessible state. --- .../SecureStorage/SecureStorage.ios.cs | 37 +++++++++++++++---- 1 file changed, 29 insertions(+), 8 deletions(-) diff --git a/Xamarin.Essentials/SecureStorage/SecureStorage.ios.cs b/Xamarin.Essentials/SecureStorage/SecureStorage.ios.cs index 62f04a995..e544bade1 100644 --- a/Xamarin.Essentials/SecureStorage/SecureStorage.ios.cs +++ b/Xamarin.Essentials/SecureStorage/SecureStorage.ios.cs @@ -8,39 +8,59 @@ namespace Xamarin.Essentials { public static partial class SecureStorage { - static Task PlatformGetAsync(string key) + public static Task GetAsync(string key, SecAccessible accessible) { - var kc = new KeyChain(); + if (string.IsNullOrWhiteSpace(key)) + throw new ArgumentNullException(nameof(key)); + + var kc = new KeyChain(accessible); return Task.FromResult(kc.ValueForKey(key, Alias)); } - static Task PlatformSetAsync(string key, string data) + public static Task SetAsync(string key, string value, SecAccessible accessible) { - var kc = new KeyChain(); - kc.SetValueForKey(data, key, Alias); + if (string.IsNullOrWhiteSpace(key)) + throw new ArgumentNullException(nameof(key)); + + if (value == null) + throw new ArgumentNullException(nameof(value)); + + var kc = new KeyChain(accessible); + kc.SetValueForKey(value, key, Alias); return Task.CompletedTask; } + + static Task PlatformGetAsync(string key) => + GetAsync(key, SecAccessible.AfterFirstUnlockThisDeviceOnly); + + static Task PlatformSetAsync(string key, string data) => + SetAsync(key, data, SecAccessible.AfterFirstUnlockThisDeviceOnly); } class KeyChain { - static SecRecord ExistingRecordForKey(string key, string service) + SecAccessible accessible; + + internal KeyChain(SecAccessible accessible) => + this.accessible = accessible; + + SecRecord ExistingRecordForKey(string key, string service) { return new SecRecord(SecKind.GenericPassword) { Account = key, Service = service, Label = key, + Accessible = accessible }; } internal string ValueForKey(string key, string service) { var record = ExistingRecordForKey(key, service); - SecStatusCode resultCode; - var match = SecKeyChain.QueryAsRecord(record, out resultCode); + var match = SecKeyChain.QueryAsRecord(record, out var resultCode); if (resultCode == SecStatusCode.Success) return NSString.FromData(match.ValueData, NSStringEncoding.UTF8); @@ -75,6 +95,7 @@ SecRecord CreateRecordForNewKeyValue(string key, string value, string service) Account = key, Service = service, Label = key, + Accessible = accessible, ValueData = NSData.FromString(value, NSStringEncoding.UTF8), }; } From 46eefa6641807025e5202f20264898cf2550171c Mon Sep 17 00:00:00 2001 From: James Montemagno Date: Thu, 3 May 2018 14:46:03 -0700 Subject: [PATCH 2/4] Add tests --- DeviceTests/DeviceTests.Shared/SecureStorage_Tests.cs | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/DeviceTests/DeviceTests.Shared/SecureStorage_Tests.cs b/DeviceTests/DeviceTests.Shared/SecureStorage_Tests.cs index 8ed7d618b..375e7d6fd 100644 --- a/DeviceTests/DeviceTests.Shared/SecureStorage_Tests.cs +++ b/DeviceTests/DeviceTests.Shared/SecureStorage_Tests.cs @@ -23,6 +23,13 @@ public async Task Saves_And_Loads(string key, string data, bool emulatePreApi23) // TODO: we don't know how to write iOS apps, it appears, so just skip for now if (DeviceInfo.DeviceType == DeviceType.Virtual) return; + + // Try the new platform specific api + await SecureStorage.SetAsync(key, data, Security.SecAccessible.AfterFirstUnlock); + + var b = await SecureStorage.GetAsync(key, Security.SecAccessible.AfterFirstUnlock); + + Assert.Equal(data, b); #endif #if __ANDROID__ From 2fd15f207621d39461365a906b815da6db055d1a Mon Sep 17 00:00:00 2001 From: Redth Date: Fri, 4 May 2018 12:49:00 -0400 Subject: [PATCH 3/4] Added iOS specific prop to set SecAccessible --- Xamarin.Essentials/SecureStorage/SecureStorage.ios.cs | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/Xamarin.Essentials/SecureStorage/SecureStorage.ios.cs b/Xamarin.Essentials/SecureStorage/SecureStorage.ios.cs index e544bade1..779889cb5 100644 --- a/Xamarin.Essentials/SecureStorage/SecureStorage.ios.cs +++ b/Xamarin.Essentials/SecureStorage/SecureStorage.ios.cs @@ -8,6 +8,9 @@ namespace Xamarin.Essentials { public static partial class SecureStorage { + public static SecAccessible DefaultAccessible { get; set; } = + SecAccessible.AfterFirstUnlockThisDeviceOnly; + public static Task GetAsync(string key, SecAccessible accessible) { if (string.IsNullOrWhiteSpace(key)) @@ -33,10 +36,10 @@ public static Task SetAsync(string key, string value, SecAccessible accessible) } static Task PlatformGetAsync(string key) => - GetAsync(key, SecAccessible.AfterFirstUnlockThisDeviceOnly); + GetAsync(key, DefaultAccessible); static Task PlatformSetAsync(string key, string data) => - SetAsync(key, data, SecAccessible.AfterFirstUnlockThisDeviceOnly); + SetAsync(key, data, DefaultAccessible); } class KeyChain From e330271142a29a6dcf18e923a7d3717a50044fce Mon Sep 17 00:00:00 2001 From: Redth Date: Fri, 4 May 2018 14:37:08 -0400 Subject: [PATCH 4/4] Default to AfterFirstUnlock on iOS SecureStorage --- Xamarin.Essentials/SecureStorage/SecureStorage.ios.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Xamarin.Essentials/SecureStorage/SecureStorage.ios.cs b/Xamarin.Essentials/SecureStorage/SecureStorage.ios.cs index 779889cb5..ab8c259fb 100644 --- a/Xamarin.Essentials/SecureStorage/SecureStorage.ios.cs +++ b/Xamarin.Essentials/SecureStorage/SecureStorage.ios.cs @@ -9,7 +9,7 @@ namespace Xamarin.Essentials public static partial class SecureStorage { public static SecAccessible DefaultAccessible { get; set; } = - SecAccessible.AfterFirstUnlockThisDeviceOnly; + SecAccessible.AfterFirstUnlock; public static Task GetAsync(string key, SecAccessible accessible) {