PublicClientApplication constructor throws exception when iPhone screen is locked

[tipa]

Which Version of MSAL are you using ?
MSAL 2.1.0-preview

Which platform has the issue?
Xamarin.iOS

What authentication flow has the issue?
Mobile

Repro

PCA = new PublicClientApplication(ClientID) { RedirectUri = $"msal{App.ClientID}://auth"; };

Expected behavior
The constructor does not crash

Actual behavior
An NullReferenceException is thrown in method GetTeamId(). More specifically, the variable match is null.
This is because the Accessible parameter isn't specified - it defaults to WhenUnlocked: https://developer.apple.com/documentation/security/ksecattraccessiblewhenunlocked

Possible Solution
Adding Accessible = SecAccessible.Always (or AfterFirstUnlockThisDeviceOnly) to this line:
https://github.com/AzureAD/azure-activedirectory-library-for-dotnet/blob/1d3eb8c49be9039d99c1bf0057e98506c256b6de/core/src/Platforms/iOS/TokenCacheAccessor.cs#L89

On a different note: Isn't there any better, less-hacky way to get the TeamId?

[jennyf19]

@tipa - Can you please provide detailed repro steps? Thank you.

[tipa]

I am using a geofence in my iOS app. When a user enters or leaves a geofence, the phone screen might be locked when the app is invoked in the background. I then want to read or write from or to OneDrive.
My code looks a bit like this:

[Register("AppDelegate")]
public class AppDelegate : UIApplicationDelegate, ICLLocationManagerDelegate
{
    public static CLLocationManager LocationManager;

    public override bool FinishedLaunching(UIApplication application, NSDictionary launchOptions)
    {
        LocationManager = new CLLocationManager() { Delegate = this };
        return true;
    }

    [Export("locationManager:didEnterRegion:")]
    public async void RegionEntered(CLLocationManager locationManager, CLRegion region)
    {
        await OneDriveHelper.Sync();
        await DatabaseHelper.DoSomeChange();
        await OneDriveHelper.Sync();
    }
}

[jennyf19]

@tipa - I wasn't able to repro this issue.
Are you creating the UIParent in the AppDelegate?
Also, have you tried w/Msal2.2.0-preview?

[tipa]

I am not creating a UIParent because the PublicClientApplication constructor already throws the exception.
I have not tried Msal2.2.0-preview yet but I don't see why any of the changes relate to this bug report. The GetTeamId()-method would still crash on locked devices because of the missing Accessible-parameter for SecRecord.

[jmprieur]

To be sure, I understand, @tipa : your app is started with the screen unlocked, and the screen get locked, and the app is re-invoked in the backgroud, which does call the constructor again? (sorry for my lack of knowledge of iOS)

[tipa]

Yes, I create a new instance of PublicClientApplication every time my "sync"-procedure is invoked. That might also happen when the phone screen is locked

[jmprieur]

Thanks for the quick update, @tipa
So I guess we'd need to have some configuration to enable your scenario.
@jennyf19 @henrik-me : let's discuss about that.

[tipa]

Thanks! As I wrote in my initial message, I would simply add Accessible = SecAccessible.Always (or Accessible = _defaultAccessiblityPolicy (which is set to AfterFirstUnlockThisDeviceOnly)) to this line:
https://github.com/AzureAD/azure-activedirectory-library-for-dotnet/blob/1d3eb8c49be9039d99c1bf0057e98506c256b6de/core/src/Platforms/iOS/TokenCacheAccessor.cs#L89

[jmprieur]

@oldalton : would you have any guidance on this issue ?
cc: @jennyf19

[henrik-me]

@jennyf19 @jmprieur : to me what @tipa says make perfect sense. only question I would have is whether we should allow it to roam to another device (my first reaction is no). Thus we should rather be using AlwaysThisDeviceOnly. @tipa would this setting work for you? (perhaps the suggestion you had with AfterFirstUnlockThisDeviceOnly is the better choice, though I would even say AlwaysThisDeviceOnly would be fine)

[oldalton]

TeamId can be always accessible, as there's no private information in it.
Tokens have to be AfterFirstUnlockThisDeviceOnly to prevent roaming.

[magnusnorberg]

@tipa @jennyf19 @jmprieur @oldalton
I am desperately trying to understand under what conditions the GetTeamId() is throwing this null reference exception.

I get this behavior on Xamarin.iOS with ADAL 4.1.0-preview and 4.0.0-preview (and I cannot change to MSAL since we use Brokered SSO in some scenarios). (ref issue AzureAD/azure-activedirectory-library-for-dotnet#1206 even though that thread is mixing two different problems)

We get this error when we try to get a new access token by the refresh token. And when this error has occurred, there is no way to get it to work again - the user may tap on "try again" but every time it fails; the only way to get it to work again is to force close the application. Could anyone explain that?

I can not really see how it would be related to the lock screen in my case. It is true that the user may receive a notification and tap on the notification on the lock screen, and the first thing that happens when the app opens upp, if the old access token has expired, is that we are using AuthenticationContext to get a new one. BUT, this does not happen every time, not even often. And then, how to explain, that once it has failed, the only remedy is to force close and then when open up the app again, everything works fine.

I understand that this description is not possible for anyone to reproduce, but I would very much appreciate if anyone could shed some light on what mechanisms in iOS + ADAL that may cause this.

There are a few issues for ADAL and MSAL related to GetTeamId(). It would be great if the mechanism to get the team id would be more robust, maybe with retry logic. And the maybe worst part of all, that the user get stuck, if he or she does not force close the app in our case.

Any help appreciated.
@tipa, you seem to have investigated this quite a bit, do you have any ideas?

Thank you

[magnusnorberg]

@jennyf19
I have looked at the latest source code and I see that you have added improved error handling to GetTeamId() with the error message CannotAccessPublisherKeyChain. BUT, the problem for me is that I have done everything according to https://aka.ms/msal-net-enable-keychain-access and although the code is successfully most of the time, at some occasions we still get the null reference exception from GetTeamId (ADAL 4.1.0-preview), and then the only way to get it to work again is to force close the app. Then everything works fine again. Se this error being intermittent, and force-closing the app makes it work again, proves it is not a configuration issue as suggested in https://aka.ms/msal-net-enable-keychain-access. What is written there is of course perfectly valid, but it does not explain my observed behavior. (In two weeks, 233 out of 455 users have got this error intermittently, and after force close, it starts working again).

It's difficult to see what way to proceed.
-We need ADAL, not MSAL, since we need Brokered SSO for some users.
-We abandoned ADAL 3.x since we intermittently get the timeout error that has been discussed in different threads (for example AzureAD/azure-activedirectory-library-for-dotnet#1206)
-We get this null reference error in ADAL 4.x intermittently, without ability to retry gracefully since force close of the app is needed.

Do you have any suggestion going forward?

Thank you

[magnusnorberg]

@jennyf19 It would be interesting to inspect SecStatusCode when our app is failing, to help us understand under what circumstances it's impossible to read from keychain until force refresh of the app.

SecRecord match = SecKeyChain.QueryAsRecord(queryRecord, out SecStatusCode resultCode);

If I want to instrument this myself, is it the dev branch I should use for my testing?

Thank you

[jmprieur]

@magnusnorberg : yes this is the dev branch, but of the ADAL.NET repo: See https://github.com/AzureAD/microsoft-authentication-library-for-dotnet#where-is-the-source-code

To build MSAL.NET, see https://github.com/AzureAD/azure-activedirectory-library-for-dotnet/wiki/ADAL-.NET-Build-&-Run