Present ExternalUserAgent without passing UIViewController for iOS

[kevinjohnason]

Hi @WilliamDenniss

@iainmcgin and I work in the same technology group and he encourages me to contribute.

While I was working on integrating our iOS SDK with AppAuth, I struggled asking our developers to pass UIViewController in order to sign in.

I understand that devices below iOS 11 require a UIViewController to present SafariViewController, however, an API that depends on UI code seems to violate the concept of clean architecture. I ended up minimizing the problem by marking iOS version with attribute

@available(iOS, deprecated: 11, message: "use signIn()")
public func signIn(from viewController: UIViewController) {}

@available(iOS 11, *)
public func signIn() {
  signIn(from: UIViewController(nibName: nil, bundle: nil))
}

This solve my immediate problem because the apps we're going to support will soon stop supporting iOS 10. But this also makes me wonder if SFAuthenticationSession on iOS 11 and ASWebAuthenticationSession on iOS 12 simply just present their web views on the top most view controller, maybe it is not such a bad idea to do the same for iOS 9 and 10 for the sake of consistency and cleanness.

Please let me know how you think.

Thanks

[WilliamDenniss]

Hi Kevin,

I'd definitely support the addition of new methods available starting iOS 11 that don't require a UIViewController.

I don't think we should deprecate the current one in 11 though, as this is still the best practice for anyone building with iOS 11+, but targeting <= iOS 10. To me it seems like you don't really need to deprecate anything to achieve your goal here.

I don't really like the idea of just grabbing the top most view controller. Anything relying on what is essentially a global state like that doesn't sit well with me, I think being explicit is better. Again, I don't think such a change is needed, if you add your new methods then your issue is solved and the old API can remain for users who need it.

[markusfassbender]

@WilliamDenniss you can deprecate for iOS 11, because Xcode recognizes that you Target earlier versions. You just get a Compiler warning if you target 11+.

[WilliamDenniss]

I'm not convinced it's desirable to have a compiler warning for people targeting 11 & below. Is this warning actionable for them? I believe that all error/warning messages should be actionable.

[markusfassbender]

I guess its a missunderstanding. I did a small test in one of my projects and deprecated a variable with iOS 10.

Build target iOS 9

Build target iOS 10 |

Users who target a smaller version do not see any warning. So it is totally fine to deprecate the function with UIViewController for iOS 11 and everything is fine. Thats just what i've meant.

[WilliamDenniss]

In that case, this is a great solution :) thanks for testing this.

I left some comments in #340 which still stand: we just need a new constructor to OIDExternalUserAgentIOS that doesn't require the UIViewController. Once that's fixed up, can get it merged.