[flutter_appauth][flutter_appauth_platform_interface] Use default system browser outside app

onizuka87 commented 2 years ago

Hi,

For one of my clients, I needed to use the default system browser outside of the app, therefore I've added a (bool) defaultSystemBrowser property to AuthorizationParameters and EndSessionRequest which uses [OIDExternalUserAgentIOSCustomBrowser CustomBrowserSafari] in the (id<OIDExternalUserAgent>)userAgentWithViewController method.

This change is only for iOS.

MaikuB commented 2 years ago

Thanks for the PR. I got a couple of questions

Whilst this behaves as expected, is there some documentation on how AppAuth iOS SDK is able to determine the default browser? Asking as I couldn't find any and it looks confusing to me given it makes use of CustomBrowserSafari yet the browser used could actually, say, Chrome if it was set to to be the default browser. It makes me wonder if it's not the intended behaviour
What happens if the user closes the browser i.e. cancels the login request? The demo IdentityServer instance seems to be down but it seems as though with this approach that the future never completes. Is this what you find happening as well? If so, given you seem to have an app making use of this, how would you suggest others deal with this scenario?

The other thing that concerns me is regarding the naming of the parameter and its default value. Suppose I don't have any other browser besides Safari, in this case to me it makes sense that the value of systemDefaultBrowser is true and that it defaults to true as Safari is used behind the scenes.. In this case, IMO it's really more about whether or not the browser is opened outside of the app or not. This to me implies that the parameter could have a better name to describe what it does. I'm not sure what would make for a good name here though but the ones that come to mind would something like opensDefaultBrowserExternally or opensInExternalDefaultBrowser

MaikuB commented 1 year ago

Closing this as there's no further response and the second point I raised would cause an issue with a future that never completes

MaikuB / flutter_appauth

[flutter_appauth][flutter_appauth_platform_interface] Use default system browser outside app #318