Azure-Samples / active-directory-b2c-ios-swift-native-msal

An iOS sample in Swift that authenticates Azure AD B2C users and calls an API using OAuth 2.0
https://aka.ms/aadb2c
MIT License
46 stars 22 forks source link

Documentation and sample not matching #49

Closed ekscrypto closed 3 years ago

ekscrypto commented 3 years ago

The documentation in the ReadMe file does not match the code of the sample application. While there are similitudes the sample has quite a number of extra steps. Would be great to have coherence between the docs & sample.

The code in the documentation and in the sample are also apparently deprecated, we should be using MSALWebviewParameters(authPresentationViewController: …) instead of MSALWebviewParameters(parentViewController: …)

mipetriu commented 3 years ago

@ekscrypto thanks for pointing that out.

mipetriu commented 3 years ago

@ekscrypto when I went to update the documentation as you mentioned, I didn't notice the deprecated code in this iOS B2C sample, but I did notice it in the general quickstart iOS sample here: https://github.com/Azure-Samples/ms-identity-mobile-apple-swift-objc. And I noticed the deprecated code in the ReadMe of the MSAL library: https://github.com/AzureAD/microsoft-authentication-library-for-objc.

Do you mind confirming those are the samples and docs you were looking at? If so, I'll close this issue in the B2C sample and make the other changes.

ekscrypto commented 3 years ago

The issue is in the ReadMe of this project here: https://github.com/Azure-Samples/active-directory-b2c-ios-swift-native-msal

As mentioned the issue is the ReadMe file recommends using

let webViewParameters = MSALWebviewParameters(parentViewController: viewController)

The header file in the MSAL framework contains this:

/**
    Creates an instance of MSALWebviewParameters with a provided parentViewController.
    @param parentViewController The view controller to present authorization UI from.
    @note parentViewController is mandatory on iOS 13+. It is strongly recommended on macOS 10.15+ to allow correct presentation of ASWebAuthenticationSession. If parentViewController is not provided on macOS 10.15+, MSAL will use application's keyWindow for presentation
 */
- (nonnull instancetype)initWithParentViewController:(MSALViewController *)parentViewController DEPRECATED_MSG_ATTRIBUTE("Use -initWithAuthPresentationViewController: instead.");;
Which as the function signature indicates, is now deprecated.

As can be seen by the function signature, it is now deprecated. The ReadMe should be updated to recommend using:

/**
   Creates an instance of MSALWebviewParameters with a provided parentViewController.
   @param parentViewController The view controller to present authorization UI from.
   @note parentViewController is mandatory on iOS 13+. It is strongly recommended on macOS 10.15+ to allow correct presentation of ASWebAuthenticationSession. If parentViewController is not provided on macOS 10.15+, MSAL will use application's keyWindow for presentation
*/
- (nonnull instancetype)initWithAuthPresentationViewController:(MSALViewController *)parentViewController;
ameyapat commented 3 years ago

Code fix has been committed and to be released soon