Making the FlowSessions and UICoordinators generic

[luksfarris]

So they can be used for more than just authorization purposes. Relates to issue #195

[WilliamDenniss]

This looks good, nice work!

Can we settle on a different naming convention though? I find "UserAgentUI" and "UserAgentFlowSession" slightly odd. How about we go with "external user-agent" names to match the RFC 8252 terminology. So that would mean:

OIDExternalUserAgentSession
OIDExternalUserAgentCoordinator
OIDExternalUserAgentCoordinatorMac
dismissExternalUserAgentAnimated:

[codecov-io]

Codecov Report

Merging #196 into master will increase coverage by 0.53%.
The diff coverage is 13.55%.

@@            Coverage Diff             @@
##           master     #196      +/-   ##
==========================================
+ Coverage   66.51%   67.04%   +0.53%     
==========================================
  Files          41       41              
  Lines        3993     4003      +10     
  Branches       73       73              
==========================================
+ Hits         2656     2684      +28     
+ Misses       1335     1317      -18     
  Partials        2        2

Impacted Files	Coverage Δ
Source/iOS/OIDAuthorizationService+IOS.m	0% <0%> (ø)	⬆️
Source/OIDAuthState.m	42.27% <0%> (ø)	⬆️
Source/macOS/OIDAuthState+Mac.m	0% <0%> (ø)	⬆️
...S/OIDExternalUserAgentUICoordinatorCustomBrowser.m	0% <0%> (ø)
Source/iOS/OIDExternalUserAgentUICoordinatorIOS.m	0% <0%> (ø)
...ource/macOS/OIDExternalUserAgentUICoordinatorMac.m	0% <0%> (ø)
Source/macOS/OIDAuthorizationService+Mac.m	0% <0%> (ø)	⬆️
Source/macOS/OIDRedirectHTTPHandler.m	0% <0%> (ø)	⬆️
Source/OIDAuthorizationService.m	16.83% <0%> (-0.12%)	⬇️
Source/iOS/OIDAuthState+IOS.m	0% <0%> (ø)	⬆️
... and 3 more

---

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 135f99d...c3cf1c7. Read the comment docs.

[luksfarris]

Good point! I've renamed the classes, methods, and docs to use the external terminology!

[WilliamDenniss]

Code itself looks good, but all of the examples are broken with this PR currently. The reason is that they use the OIDAuthorizationFlowSession protocol which was removed.

Thinking more broadly than the examples, literally everyone will be impacted by this change. To avoid that, I think we should do the following:

Keep the old OIDAuthorizationFlowSession protocol (and keep the new OIDExternalUserAgentSession). Existing implementations of OIDAuthorizationFlowSession should implement both OIDAuthorizationFlowSession and OIDExternalUserAgentSession (resumeAuthorizationFlowWithURL can simply call resumeExternalUserAgentFlowWithURL in the implementation). New flows like logout can implement just OIDExternalUserAgentSession.

OIDAuthorizationService.m will need to be updated too like so:

+ (id<OIDExternalUserAgentFlowSession,OIDAuthorizationFlowSession>) presentAuthorizationRequest:(OIDAuthorizationRequest *)request UICoordinator:(id<OIDExternalUserAgentUICoordinator>)UICoordinator callback:(OIDAuthorizationCallback)callback {

That way we can avoid most impact of these changes.

[luksfarris]

Very good idea @WilliamDenniss . I've just finish implementing it and it works in the iOS example project. I'm making sure it won't break the other ones, and I'll update the PR and rebase it to resolve these conflicts.
Thanks again

edit: it still broke the Mac version. I've decided to keep the flow session property with the old name currentAuthorizationFlow so it won't break anyone's code. Is this ok for you?

edit2: awesome work on the custom browser!! I hope you don't mind I've renamed it to match the naming convention of the other classes in this PR

[dgommers]

This method is calling itself right now, causing an infinite loop. It should call the new [self failExternalUserAgentFlowWithError:error] instead.

[jaishankar]

@WilliamDenniss @luksfarris Please let us know if this will be reviewed and merged. Also can you confirm on the invoking user agent flow for RP Initiated logout - #191 (comment)

[luksfarris]

@dgommers I had missed that. Just fixed and rebased
@jaishankar I'll reply your comment on issue #191 soon

[WilliamDenniss]

LGTM.

I kept your commit, but applied some additional refactoring on the names (dropping the "UICoordinator" since it's a bit superfluous). Merged as part of #212.

[luksfarris]

Awesome! Thank you!