Make webview parameters optional in MSALSignoutParameters

[oldalton]

Proposed changes

Since most of developers call MSAL signout without requesting interactive signout from browser, it is tedious to require to provide MSALWebViewParameters in those cases.

This PR makes MSAL a bit "smarter" and makes MSALWebViewParameters mandatory only when signoutFromBrowser is requested.

Type of change

• Feature work
• Bug fix
• Documentation
• Engineering change
• Test
• Logging/Telemetry

Risk

• High – Errors could cause MAJOR regression of many scenarios. (Example: new large features or high level infrastructure changes)
• Medium – Errors could cause regression of 1 or more scenarios. (Example: somewhat complex bug fixes, small new features)
• Small – No issues are expected. (Example: Very small bug fixes, string changes, or configuration settings changes)

Additional information

[mipetriu]

nit: Not immediately sure what's being filled in with the account. Maybe "setAccountIdentifier"?

[oldalton]

Addressed

[mipetriu]

Just curious, should we include a test case for _whenSignoutFromBrowserFalse_andNilWebViewParameters_shouldremoveAccount?

[oldalton]

Great idea, added :)

[aherciya]

This isn't related to your changes Olga but I detected some unused/deprecated methods/fields in MSALPublicClientApplication.m:
->unused method(line 714): acquireTokenForScopes
->unused field(line 114): _defaultKeychainGroup
->deprecated method(lines 164 and 176): both use self initWithClientId:clientId
keychainGroup:MSALCacheConfig.defaultKeychainSharingGroup
authority:authority
redirectUri:nil
error:error]. But should use (instancetype)initWithConfiguration:(MSALPublicClientApplicationConfig *)config
error:(NSError **)error instead.

[oldalton]

This isn't related to your changes Olga but I detected some unused/deprecated methods/fields in MSALPublicClientApplication.m:
->unused method(line 714): acquireTokenForScopes
->unused field(line 114): _defaultKeychainGroup
->deprecated method(lines 164 and 176): both use self initWithClientId:clientId
keychainGroup:MSALCacheConfig.defaultKeychainSharingGroup
authority:authority
redirectUri:nil
error:error]. But should use (instancetype)initWithConfiguration:(MSALPublicClientApplicationConfig *)config
error:(NSError **)error instead.

Thanks, I think acquireTokenForScopes on line 714 is actually public. It is deprecated, but we cannot remove it without updating major MSAL version. We should be able to remove all deprecated methods when we release MSAL 2.x.

_defaultKeychainGroup - good catch. Please submit a separate PR to fix this.

deprecated method(lines 164 and 176): both use self initWithClientId:clientId - I think it is the same as acquireTokenForScopes. We need to keep it for backward compatibility since those are public APIs. Let me know if that explains.

[aherciya]

Ok. I'll create a separate PR for removing _defaultKeychainGroup. As for the deprecated/unused methods, does a list of these exist somewhere for when MSAL 2.x happens? If not I can add start a list our knowledgeable. (Unless we have some script that we are planning on running before MSAL 2.x that will catch all of these).

[oldalton]

Ok. I'll create a separate PR for removing _defaultKeychainGroup. As for the deprecated/unused methods, does a list of these exist somewhere for when MSAL 2.x happens? If not I can add start a list our knowledgeable. (Unless we have some script that we are planning on running before MSAL 2.x that will catch all of these).

Thanks. For the deprecated APIs, you can just see all of them in the MSALPublicClientApplication.h file. They're annotated with DEPRECATED_MSG_ATTRIBUTE.

[aherciya]

Ok. I'll create a separate PR for removing _defaultKeychainGroup. As for the deprecated/unused methods, does a list of these exist somewhere for when MSAL 2.x happens? If not I can add start a list our knowledgeable. (Unless we have some script that we are planning on running before MSAL 2.x that will catch all of these).

Thanks. For the deprecated APIs, you can just see all of them in the MSALPublicClientApplication.h file. They're annotated with DEPRECATED_MSG_ATTRIBUTE.

Got it..

[aherciya]

Looks good to me.