search

okta / okta-oidc-xamarin

Okta OIDC SDK for Xamarin
https://github.com/okta/okta-oidc-xamarin
Apache License 2.0
10 stars 11 forks source link

Don't subclass MainActivity and AppDelegate #55

Closed alexanderdibenedetto closed 2 years ago

alexanderdibenedetto commented 3 years ago

Subclassing MainActivity and AppDelegate is a treacherous anti-pattern for a third party integration. Third party integrations should not assume the responsibility of initializing an entire Xamarin Forms application by overriding its AppDelegate or MainActivity classes. What if every third party integration in an application required subclassing as a means of implementing this SDK? A developer would only get to pick one....

User story

"As a Xamarin Forms developer, I want complete freedom over what, if any, subclasses are used for the MainActivity and AppDelegate, since you can only subclass a main activity or app delegate with one third party integration."

Proposed solution

Xamarin Essentials provides a good example of how to do this correctly by providing a required method, static void Init() for each Platform.

You could create the same thing! Make an OktaPlatform class with a void Init() method for iOS and static void Init(ApplicationContext context) method for Android (one for each platform). That method would then do this (hiding away all this abstraction):

iOS

OktaContext.Init(new iOsOidcClient(Window.RootViewController, iOsOktaConfig.LoadFromPList("OktaConfig.plist")));

Android

OktaContext.Init(new AndroidOidcClient(this, AndroidOktaConfig.LoadFromXmlStream(Assets.Open("OktaConfig.xml"))));

For iOS, you would additionally require the OpenUrl method be overridden to add the OktaService.OpenUrl(application, url, sourceApplication, annotation) method that would do this:

iOsOidcClient.IsOktaCallback(application, url, sourceApplication, annotation);

For apps I write, I have quite a few OpenUrl methods (including my own deep links) to check here. Okta would just be one of many such integrations.

Another issue is that in iOS, you make the user subscribe to SignInCompleted and SignOutCompleted, but in Android you do it for us then provide a virtual override method that we hook into. Instead, consider leaving the SignInCompleted and SignOutCompleted subscriptions up to the individual developers to do.

Alternatives considered

The alternative is to copy and paste the code you have in your custom main activity and app delegate code into my application. Which is a pain to maintain as changes may/may not be made here.

Additional information

Feel free to reach out to me with any questions!

bryanapellanes-okta commented 3 years ago

@alexanderdibenedetto, Thank you so much for your input! It is great to receive community feedback like this so we can make the improvements that our users want and need. I've opened an internal issue for tracking OKTA-416549. We'll review your notes more closely to be included in a future release.

If you were so inclined and had the time, please feel free to submit a pull request with your proposed implementation and we can work together to build the solution that is best for everyone.