Essentials icon indicating copy to clipboard operation
Essentials copied to clipboard

Specify iOS Keychain Access Group for SecureStorage

Open tedrog36 opened this issue 6 years ago • 34 comments

For iOS, I need the ability to specify the keychain access group for SecureStorage so that the information stored there can be accessed by the main host app as well as app extensions. This is primary reason I am not using SecureStorage.

This could be implemented similar to Preferences/Xam.Settings where a shareName can be specified.

tedrog36 avatar Feb 21 '19 16:02 tedrog36

This would be a good addition. I think that on Android we could pass this in and store it in specific preferences and pass it down. We can have a default which most people will use and then this as an optional parameter. Thanks for the feature request.

jamesmontemagno avatar Feb 21 '19 22:02 jamesmontemagno

No problem. Let me know if you need anything for the implementation since I have already done this in my own KeyChain class.

tedrog36 avatar Feb 22 '19 12:02 tedrog36

If you want to send down a PR that would be cool ;)

jamesmontemagno avatar Feb 22 '19 18:02 jamesmontemagno

If the issue is still open, can I give a try to add this enhancement?

How do we like to pass down the value of KeyChain Access Group value to be used in SecRecord? Should we add constructor that takes this value?

sasivishnu avatar May 07 '19 19:05 sasivishnu

I haven't looked at the Keychain object in SecureStorage in a while but I can tell you that is how I handled it in my Keychain classs.

public Keychain(string serviceName, string accessGroup)
{
	_serviceName = serviceName;
	_accessGroup = accessGroup;
}

tedrog36 avatar May 08 '19 11:05 tedrog36

I created a local implementation last night.

I didn't go with the accessGroup in the constructor approach, as I figured someone might want to change it/not use it between calls. i.e. something keychain items should be sharable and others shouldn't.

That being said, my knowledge is somewhat limited to how the keychain works, as I remember reading somewhere that it if you don't specify an accessGroup it picks the first one from the list (not sure how accurate this is).

Johan-dutoit avatar May 10 '19 09:05 Johan-dutoit

PRs are accepted :)

jamesmontemagno avatar May 14 '19 21:05 jamesmontemagno

@jamesmontemagno I was about to submit a PR, however it's for iOS only and would put the other platforms in an unsupported state. Any ideas on how that should be implemented?

Android we could pass this in and store it in specific preferences and pass it down

When you said we can pass it in and store it in a specific preference, do you mean prefixing the key with the accessGroup value?

Johan-dutoit avatar May 15 '19 07:05 Johan-dutoit

I think that they are just ignored on Android/UWP, we can just document it.

jamesmontemagno avatar Dec 20 '19 00:12 jamesmontemagno

I might be misunderstanding what this issue is about but wasn't this feature supported in Xamarin.Auth?

kenny128 avatar Jan 09 '20 00:01 kenny128

Is this issue still pending to be solved?

Andrec-Dxs avatar May 29 '20 09:05 Andrec-Dxs

any news @jamesmontemagno ?

GMCristiano avatar Jun 15 '20 13:06 GMCristiano

+1

I'm already invested in using Xamarin Essential's SecureStorage in my project and have just hit the snag of needing to share keychain data with an iOS extension that is bundled with the app.

@jamesmontemagno @Johan-dutoit - Is there any progress updates with this feature request? Contemplating whether to wait for this or pivot to another solution, which I'd rather not do since the library has been great for my needs thus far!

Thanks guys.

UPDATE: I ended up forking the repo and made the required changes for my project. I'll try tidy this up and do a pull request for the benefit of everyone else. Unless one of you guys have already done the work and wish to officially submit the PR? I'll leave this open for a little bit and check back in a week or so.

mshuf avatar Aug 06 '20 00:08 mshuf

I ended up forking the repo and made the required changes for my project. I'll try tidy this up and do a pull request for the benefit of everyone else. Unless one of you guys have already done the work and wish to officially submit the PR? I'll leave this open for a little bit and check back in a week or so.

@mshuf : Would love to know when you commit to your fork (even if you don't get to the pull request). I'm trying to get an extension to share SecureStorage with it's parent app and ran into this snag as well. Would be nice to test it out and see if your solution works with my use case.

kiddailey avatar Aug 17 '20 08:08 kiddailey

Would probably look something like this -> https://github.com/xamarin/Essentials/pull/1351

jamesmontemagno avatar Aug 17 '20 23:08 jamesmontemagno

Awesome jamesmontemagno, thanks!

As noted in my post on #1351 , this doesn't seem to work. I'm testing with a container app and extension and neither can access values stored by the other with the same access group applied.

kiddailey avatar Aug 18 '20 18:08 kiddailey

Any progress on this issue @kiddailey @jamesmontemagno

Andrec-Dxs avatar Oct 06 '20 10:10 Andrec-Dxs

Sorry for the slow response. I unfortunately haven't gotten around to looking at this more, but I hope to in the coming week or so. In a nutshell, it doesn't seem to work as-is and there is an outstanding question (posted on #1351) about how to treat this feature in the case of Android.

The app/extension I am developing requires this functionality, so I'm planning to either figure out how to make Essentials work or develop my own implementation. Will post when I can.

kiddailey avatar Oct 22 '20 08:10 kiddailey

Ok ok. @kiddailey I don't know if this might help but I don't think you'll have an issue while doing this for Android because in iOS in order to access a app extension and have the stored information from the main application you need to have the apps on the same app group. On android this doesn't exist so I don't believe that will be an issue. I found this project on Android that tries to handle the secure storage for Android devices. Take a look if that might help you....

https://github.com/adorsys/secure-storage-android/issues

Andrec-Dxs avatar Oct 22 '20 08:10 Andrec-Dxs

@kiddailey any news?

Andrec-Dxs avatar Nov 09 '20 09:11 Andrec-Dxs

@kiddailey any news?

Yes, the apps have to be part of the same app group on iOS. That's the part that doesn't appear to be working for whatever reason. Not sure if it's user error on my part or something else.

I'm going to spend some time today/tomorrow getting everything set up again (because new versions). Will let you know if I discover anything.

Edit: Finally got Essentials compiling/building/packaging on my Mac and starting to get back into the swing. Having an issue getting the Extension project in my test app to accept the new package, but will get there eventually. More to come.

Edit: Got the extension project working with my Essentials build but having same results in that the extension always gets no value back. Slowly working my way through things to see if I can find out anything new...

kiddailey avatar Dec 06 '20 20:12 kiddailey

Had my first successful test! I implemented jamesmontemagno's changes into Essentials, packaged and created a new test application consisting of a container app and a single action extension (apparently, I made some changes that fixed the issues, see below).

After (much) playing around with all the entitlements, it works. I was able to set a key in the container app and access it both in the container AND the extension!

I need to figure out the specifics about the entitlement requirements, because I had to setup keychain access and app groups despite what the documentation seems to say and the app prefix isn't required.

Next step -- figure out those specifics and then try this in a much more sophisticated app and see if the results are the same. Will post when I have that complete. <crossing fingers>

Edit: I should note that this was in the iPhone simulator. I haven't confirmed it's the same on an actual device, but will definitely test that as well.

Edit: In the iPhone simulator, I updated a far more complicated app with a different extension type to use my custom Essentials build and went through the same motions of making sure entitlements are correct, and I have success in the iPhone simulator! Will update again when I test on actual device.

Edit: Actual device working as well! I also realize that I may have made some minor updates to jamesmontemango's original changes, so I need to review those to make sure I didn't accidentally fix things myself :D

Edit: Yes, I made two key changes that I'm researching to determine which, if any, is the reason it is working now

kiddailey avatar Dec 18 '20 20:12 kiddailey

I've figured out the issue for why this wasn't working across apps and/or extensions on iOS and gave a detailed explanation and the required code changes on @jamesmontemagno 's pull request referenced above: https://github.com/xamarin/Essentials/pull/1351#issuecomment-748457139

kiddailey avatar Dec 19 '20 18:12 kiddailey

@kiddailey any update on this PR?

gsgou avatar Feb 07 '21 18:02 gsgou

I submitted a pull request a few days ago to @jamesmontemagno 's original pull request that fixes the issues with iOS (I haven't yet looked at Android). It works well and I've been testing it for some time now with a container app and extension without any issues. Hopefully he'll have some free time to review and incorporate it, but I'm not sure when/if it'll make it into the mainline.

kiddailey avatar Feb 07 '21 20:02 kiddailey

Oh I could so use this right now! Any word on it being released soon? Thanks for all your hard work!

mnxamdev avatar Mar 08 '21 15:03 mnxamdev

@jamesmontemagno Is there any status update on the possibility of your xamarin:secure-storage-group branch being merged into main?

I've been using it with the changes you merged back in February in development and testing on iOS for a number of months now without issue. It works great, but I'm trying to decide if I'm going to wait for Essentials to have this merged into main, or if I should just roll my solution.

If there's something I can do to help, let me know. Thanks!

kiddailey avatar Aug 14 '21 07:08 kiddailey

Hi there, just wanted to leave a +1 on this! Had to implement this in my own layer recently in order to be able to share keychain items between main app and action extension. Would be really great if we could extend Xamarin Essentials to support this scenario. Thank you!

yevgeniy-seleznev avatar Nov 15 '21 23:11 yevgeniy-seleznev

Any word on this pull request?

mnxamdev avatar Jan 17 '22 15:01 mnxamdev

Is this ever going to be merged?

Giorgi avatar Mar 15 '22 17:03 Giorgi