Skip to content
This repository was archived by the owner on May 15, 2024. It is now read-only.

Conversation

jamesmontemagno
Copy link
Collaborator

Description of Change

Default iOS to AfterFirstUnlockThisDeviceOnly for the cross-platform API, but allow custom platform specific methods to pass in the SecAccessible

Bugs Fixed

Provide links to issues here. Ensure that a GitHub issue was created for your feature or bug fix before sending PR.

API Changes

List all API changes here (or just put None), example:

Added:

  • Task GetAsync(string key, SecAccessible accessible)
  • Task SetAsync(string key, string value, SecAccessible accessible)

Behavioral Changes

Nothing for iOS as this was the default before and what should be exposed.

PR Checklist

  • Has tests (if omitted, state reason in description)
  • Has samples (if omitted, state reason in description)
  • Rebased on top of master at time of PR
  • Changes adhere to coding standard
  • Updated documentation (see walkthrough)

…orms. Although allow a platform specific override to set accessible state.
@jamesmontemagno jamesmontemagno requested a review from Redth May 3, 2018 21:46
@dend
Copy link
Contributor

dend commented May 3, 2018

✅ Validation status: passed

File Status Preview URL Details
DeviceTests/DeviceTests.Shared/SecureStorage_Tests.cs ✅Succeeded
Xamarin.Essentials/SecureStorage/SecureStorage.ios.cs ✅Succeeded

For more details, please refer to the build report.

Note: If you changed an existing file name or deleted a file, broken links in other files to the deleted or renamed file are listed only in the full build report.

@jamesmontemagno
Copy link
Collaborator Author

@migueldeicaza to address the SecAccessible there is no real equivalent on Android/UWP to add to the cross platform API. This PR addresses setting it to the recommended default of AfterFirstUnlockThisDeviceOnly which matches other operating systems. At the same time offers up a platform specific overload to pass in the SecAccessible option to use.

@Redth
Copy link
Member

Redth commented May 4, 2018

I'm thinking we may want to provide an api to set the default on iOS. I think users will generally want to set this once per application and forget about it and continue using GetAsync and SetAsync via their shared code.

Maybe we should do something like:

public partial class SecureStorage
{
    public static Security.SecAccessible DefaultAccessibility { get; set; } = 
        SecAccessible.AfterFirstUnlockThisDeviceOnly;
}

What do you think?

@jamesmontemagno
Copy link
Collaborator Author

I guess my current implementation it is on a key by key value compared to a global... I am not sure what iOS app devs would want really...

@dend
Copy link
Contributor

dend commented May 4, 2018

✅ Validation status: passed

File Status Preview URL Details
DeviceTests/DeviceTests.Shared/SecureStorage_Tests.cs ✅Succeeded
Xamarin.Essentials/SecureStorage/SecureStorage.ios.cs ✅Succeeded

For more details, please refer to the build report.

Note: If you changed an existing file name or deleted a file, broken links in other files to the deleted or renamed file are listed only in the full build report.

@mattleibow
Copy link
Contributor

@Redth I see there are some public platform-specific methods here... I left a few for Android Platform, but we should maybe discuss this and see what we want to do in these cases.

@dend
Copy link
Contributor

dend commented May 4, 2018

✅ Validation status: passed

File Status Preview URL Details
DeviceTests/DeviceTests.Shared/SecureStorage_Tests.cs ✅Succeeded
Xamarin.Essentials/SecureStorage/SecureStorage.ios.cs ✅Succeeded

For more details, please refer to the build report.

Note: If you changed an existing file name or deleted a file, broken links in other files to the deleted or renamed file are listed only in the full build report.

@migueldeicaza
Copy link

This is a good step, but I wonder if we should support even as no-ops the other capabilities.

The other bit to think about is whether we should by default allow the key to be backed up when the user has encrypted backups. That sounds like a good default.

@dend
Copy link
Contributor

dend commented May 4, 2018

✅ Validation status: passed

File Status Preview URL Details
DeviceTests/DeviceTests.Shared/SecureStorage_Tests.cs ✅Succeeded
Xamarin.Essentials/SecureStorage/SecureStorage.ios.cs ✅Succeeded

For more details, please refer to the build report.

Note: If you changed an existing file name or deleted a file, broken links in other files to the deleted or renamed file are listed only in the full build report.

@Redth Redth merged commit 9295ce3 into master May 4, 2018
@Redth Redth deleted the feature/ios-secaccessible branch May 4, 2018 18:58
jamesmontemagno added a commit that referenced this pull request Jun 21, 2018
* GH-221: Add iOS SecAccessible properties (#223)

* Address #221, use AfterFirstUnlockThisDeviceOnly to match other platforms. Although allow a platform specific override to set accessible state.

* Add tests

* Added iOS specific prop to set SecAccessible

* Default to AfterFirstUnlock on iOS SecureStorage

* A few fixes for the release. (#228)

* Setting the correct flags for Chrome Tabs. Fixes #225

* Make sure that there is data to decrypt before starting. Fixes #226

* Fixes for TTS. Fixes #227

* A bit of the fixes for emails on iOS. Relates to #224

* Update the email bit for iOS #224

* Cleaning up the code #224

* Make sure to set the email properties. Fixes #229

* Update SDK Extras

This fixes a lot of references that were required on android that aren't.

* Update Readme with Installation information. (#237)

* Update Readme with Installation information.

* Update README.md

* Update README.md

* Adding the initial work to get Tizen started. #23

* Revert "Adding the initial work to get Tizen started. #23"

This reverts commit 58b6041.

* Update the docs with the latest APIs (#233)

Add docs for SecAccessible

* GH-245: Add Orientation Sensor (#249)

* Add Orientation Sensor

* Add Orientation Sensor

* Remove .csproj clutter

* "orientationsensor" --> "orientation sensor" or just "orientation"

* GH-192: Added DateTime to Preferences (#232)

* Added DateTime to preferences. Fixes #192
 - also properly using overloads

* Added some unit tests

* Added the docs for the new Preferences APIs

* Update Readme with Device Display Information

Added link to Current Features for Device Display Information

* Fixes #258 and Fixes #255 (#259)

* Fixes #258
* put vectors nuget everywhere.
* NuGet is broken - https://developercommunity.visualstudio.com/content/problem/232996/warning-nu1603-runtimenativesystemiocompression-41.html
* The NuGet doesn't add the assembly references from the GAC
* We need to use a later version of the iOS SDK

* Fix typo (#274)

* Fix up null checks when getting lask known location.

* Additional Null checks

* GH-240 Add IsMainThread detection API (#277)

* Add IsMainThread detection API

* Add platform tests for main thread

* Update docs and add more platform tests

* iOS secure storage simulator tests (#247) (#278)

* iOS secure storage simulator tests (#247)

* Remove line that skips test on virtual devices

* Adding keychain-access-groups in Entitlements.plist

* Setting CodeSign entitlements for Debug configuration

* Making same entitlement.plist changes for iOS sample

* Removing specific code sign key

* Removing physical device trait for iOS SecureStorageTests

* Update DeviceTests.iOS.csproj

* Update Samples.iOS.csproj

* Add configs for codesign on release

* Fixes #181 Allowing setting of calibration for heading. (#282)

* GH-254 Update Support Packages, Forms, and Reference Vectors (#279)

* Update Support Packages, Forms, and Reference Vectors

* Bump targetsdk

* Fix android build :)

* Fix build
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants