Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

DefaultAccessible seems to have the wrong default value #17582

Closed
kanadaj opened this issue Sep 22, 2023 · 4 comments · Fixed by #20914
Closed

DefaultAccessible seems to have the wrong default value #17582

kanadaj opened this issue Sep 22, 2023 · 4 comments · Fixed by #20914
Labels
area-essentials Essentials: Device, Display, Connectivity, Secure Storage, Sensors, App Info fixed-in-8.0.10 fixed-in-9.0.0-preview.2.10293 migration-compatibility Xamarin.Forms to .NET MAUI Migration, Upgrade Assistant, Try-Convert platform/iOS 🍎 platform/macOS 🍏 macOS / Mac Catalyst s/triaged Issue has been reviewed t/bug Something isn't working
Milestone

Comments

@kanadaj
Copy link

kanadaj commented Sep 22, 2023

Description

For the last few days we've been diagnosing an issue for SecureStorage. It seems that SecureStorage.DefaultAccessible on iOS uses Security.SecAccessible.WhenUnlocked, while the documentation for it says Default value is <see cref="Security.SecAccessible.AfterFirstUnlock"/>.. We couldn't find a location where DefaultAccessible is assigned to a value different to the default enum value in this repo, and the enum has the following values:

  public enum SecAccessible
  {
    Invalid = -1, // 0xFFFFFFFF
    WhenUnlocked = 0,
    AfterFirstUnlock = 1,
    [SupportedOSPlatform("ios"), SupportedOSPlatform("maccatalyst"), SupportedOSPlatform("macos"), SupportedOSPlatform("tvos"), ObsoletedOSPlatform("macos10.14", "Use 'AfterFirstUnlock' or a better suited option instead."), ObsoletedOSPlatform("ios12.0", "Use 'AfterFirstUnlock' or a better suited option instead.")] Always = 2,
    WhenUnlockedThisDeviceOnly = 3,
    AfterFirstUnlockThisDeviceOnly = 4,
    [SupportedOSPlatform("ios"), SupportedOSPlatform("maccatalyst"), SupportedOSPlatform("macos"), SupportedOSPlatform("tvos"), ObsoletedOSPlatform("macos10.14", "Use 'AfterFirstUnlockThisDeviceOnly' or a better suited option instead."), ObsoletedOSPlatform("ios12.0", "Use 'AfterFirstUnlockThisDeviceOnly' or a better suited option instead.")] AlwaysThisDeviceOnly = 5,
    WhenPasscodeSetThisDeviceOnly = 6,
  }

So the default would be WhenUnlocked instead of the documented AfterFirstUnlock. Not sure if this is the intended behaviour and the docs are wrong, or the docs are correct and the value is incorrect.

If this is a spec issue, it'd be good to see the spec fixed and the existence of this platform specific setting highlighted in the docs. If it's an implementation issue, then the default value needs to be assigned in SecureStorageImplementation.

Steps to Reproduce

No response

Link to public reproduction project repository

No response

Version with bug

7.0.92

Is this a regression from previous behavior?

Not sure, did not test other versions

Last version that worked well

Unknown/Other

Affected platforms

iOS, macOS

Affected platform versions

No response

Did you find any workaround?

SecureStorage.DefaultAccessible = SecAccessible.AfterFirstUnlock; in AppDelegate.cs. Not sure if this is a spec issue or a bug, but figured I'd report either way.

Relevant log output

No response

@kanadaj kanadaj added the t/bug Something isn't working label Sep 22, 2023
@cho-trackman
Copy link

I just wondered the same and found this. I recently discovered a similar bug in a native app. In Xamarin.Essentials this was also set to AfterFirstUnlock.

@cho-trackman
Copy link

This will lead to a serious bug if you have background downloads/uploads.

@Eilon Eilon added the area-essentials Essentials: Device, Display, Connectivity, Secure Storage, Sensors, App Info label Nov 8, 2023
@MichaelLHerman
Copy link

MichaelLHerman commented Feb 12, 2024

Just ran into this today, also thought the default was different because I read the documentation.

Suggestion for the workaround - If you are using interfaces and service collection DI, you can set this when configuring your services

#if IOS
        builder.Services.AddSingleton(ctx => (ISecureStorage)ctx.GetService<IPlatformSecureStorage>());
        builder.Services.AddSingleton(_ =>
        {
            var platformSecureStorage = (IPlatformSecureStorage)SecureStorage.Default;
            platformSecureStorage.DefaultAccessible = SecAccessible.AfterFirstUnlockThisDeviceOnly;
            return platformSecureStorage;
        });
#else
        builder.Services.AddSingleton(ctx => SecureStorage.Default);
#endif

@MichaelLHerman
Copy link

Looks like the default value was explicitly set for Xamarin Essentials but that didn't make it over to MAUI.

https://github.com/xamarin/Essentials/blob/1.8.1/Xamarin.Essentials/SecureStorage/SecureStorage.ios.tvos.watchos.macos.cs#L13

I think this should be handled with just a documentation change since the default shouldn't be moved from a more restrictive / secure value (needs to be currently unlocked) to a less restrictive value (had to have been unlocked once) from one MAUI version to another.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
area-essentials Essentials: Device, Display, Connectivity, Secure Storage, Sensors, App Info fixed-in-8.0.10 fixed-in-9.0.0-preview.2.10293 migration-compatibility Xamarin.Forms to .NET MAUI Migration, Upgrade Assistant, Try-Convert platform/iOS 🍎 platform/macOS 🍏 macOS / Mac Catalyst s/triaged Issue has been reviewed t/bug Something isn't working
Projects
None yet
Development

Successfully merging a pull request may close this issue.

7 participants