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

[decryptedResponse[@"success"] boolValue] returns 'nil' in MSIDDefaultBrokerResponseHandler #2528

Open
ryanmsmith-v opened this issue Feb 24, 2025 · 3 comments
Assignees

Comments

@ryanmsmith-v
Copy link

MSAL 1.7.0
Intune SDK 19.7.9
Xcode 15.4

I'm integrating the Intune SDK into our company's app and have run into a bug in MSAL.

On an unmanaged device or on an Intune MDM-managed device with MS Authenticator installed, Intune user enrollment is successful. However, on an unmanaged device that has MS Authenticator installed, completing the enrollment flow results in a -50000 error upon returning to the app.

Digging into the MSAL source code, I found that [decryptedResponse[@"success"] boolValue] in line 141 of MSIDDefaultBrokerResponseHandler is being evaluated as nil. The "success" value is a string, @"1", not a number. This causes the failure.

As a short-term solution, I have rewritten this to explicitly treat decryptedResponse[@"success"] as a string:

    // decryptedResponse[@"success"] is @"1"
    // [decryptedResponse[@"success"] boolValue] was returning `nil`, resulting in a failure
    // This instead explicitly converts the value to an NSNumber to get the boolean value
    BOOL success = NO;
    if (decryptedResponse[@"success"]) {
        // Make it an NSString, just in case it's something else
        NSString *successString = [NSString stringWithFormat:@"%@", decryptedResponse[@"success"]];
        if (![NSString msidIsStringNilOrBlank:successString]) {
            NSNumberFormatter *formatter = [[NSNumberFormatter alloc] init];
            NSNumber *successNumber = [formatter numberFromString:successString];
            if (successNumber) {
                success = [successNumber boolValue];
            }
        }
    }
    // Successful case
    if ([NSString msidIsStringNilOrBlank:decryptedResponse[@"broker_error_domain"]]
        && success)
    {
@antonioalwan antonioalwan self-assigned this Feb 28, 2025
@antonioalwan
Copy link
Contributor

Thanks, @ryanmsmith-v, for submitting this issue. I will investigate further and update you with the outcome.

@antonioalwan
Copy link
Contributor

Hi @ryanmsmith-v,

I didn't find any instance where "success" is set to a non-NSString value. In the function msidDictionaryFromWWWFormURLEncodedString, each entry is added to the dictionary with the value always converted to a string:

NSString *value = @"";
if (queryElements.count == 2)
{
    value = isFormEncoded ? [queryElements[1] msidTrimmedString].msidWWWFormURLDecode : [queryElements[1] msidTrimmedString].msidURLDecode;
}
[queryDict setValue:value forKey:key];

Additionally, I adjusted some tests to convert the value of "success" to int or double, and boolValue didn't fail in converting to boolean. Please let me know if you have an example you can share with me regarding the value.

@ryanmsmith-v
Copy link
Author

Thanks. The only example I have is that I shared originally. The value of "success" was @"1" when checked in the debugger, but the boolValue of that resulted in nil, not YES. Because the value of "success" is a string that is "1" (and not "y", "Y", "t", "T"), boolValue is not going to result in true. I'm surprised it is resulting in nil instead of false, though.

https://developer.apple.com/documentation/foundation/nsstring/1409420-boolvalue?language=objc

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants