Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Introduce a convenience property list initializer, to align with OktaOAuth2 flows #110

Merged
merged 2 commits into from
Aug 5, 2022

Conversation

mikenachbaur-okta
Copy link
Contributor

I discovered, while documenting the authentication flows, that InteractionCodeFlow didn't consume the default Okta.plist file, as in other authentication flows.

Copy link

@emmanuelogunmuyiwa-okta emmanuelogunmuyiwa-okta left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good!

@emmanuelogunmuyiwa-okta
Copy link

emmanuelogunmuyiwa-okta commented Aug 5, 2022

Since using the default plist now is an option, I think it might be beneficial to add a default/shared instance of InteractionCodeFlow and maybe we can update the Create the flow documentation as well to reflect this. Any thoughts on this ?

@mikenachbaur-okta
Copy link
Contributor Author

The flows themselves are meant to be explicitly retained. The reason is that I want to avoid overuse / abuse of singletons. The WebAuthentication.shared singleton is an exception, since it's a common use-case. But using the flows directly is a more advanced use-case that is reasonable to assume a developer would be able to manage their memory properly.

@mikenachbaur-okta
Copy link
Contributor Author

I like your point about updating docs to reflect this convenience. I'll do that before merging this PR.

@mikenachbaur-okta mikenachbaur-okta merged commit 4cfa3c2 into master Aug 5, 2022
@mikenachbaur-okta mikenachbaur-okta deleted the man-ConveniencePListInitializer branch August 5, 2022 17:40
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants