WIP: Controller logic for RP-initiated logout

[dgommers]

WIP – Relates to issue #132.

Continuing the work that has been done by @luksfarris and @WilliamDenniss on the RP-initiated logout. I've merged master into the dev-logout branch that already contained the EndSession request/response pair and added some controller glue.

This PR is a small step towards completing the RP-initated logout implementation: I would like to discuss my current progress with other contributors.

As of now, I've added the OIDAuthState.presentEndSessionRequest:externalUserAgent:callback: method. This creates a new OIDEndSessionFlowSession and presents it on the OIDExternalUserAgent.

Main things I would like to debate:

1. Any suggestions for a better naming of OIDEndSessionFlowSession? It is quite odd right now.
2. Is the OIDAuthState is the desired location for this? We could make it standalone or integrate it with the OIDAuthorizationService class.
3. What should happen with the OIDAuthState once the logout was successful? As of now it calls the resetState that brings it into an empty state.
4. The OIDEndSessionFlowSession is heavily inspired by the OIDAuthorizationFlowSessionImplementation inside the OIDAuthorizationService. Do we want to get rid of this slight duplication?

No unit tests so far as things are likely to change a lot. I'll sign the contributors agreement a.s.a.p.

Looking forward for your feedback.

[codecov-io]

Codecov Report

Merging #219 into master will increase coverage by 0.3%.
The diff coverage is 43.67%.

@@            Coverage Diff            @@
##           master     #219     +/-   ##
=========================================
+ Coverage   74.05%   74.36%   +0.3%     
=========================================
  Files          58       40     -18     
  Lines        5076     3752   -1324     
  Branches        0       73     +73     
=========================================
- Hits         3759     2790    -969     
+ Misses       1317      960    -357     
- Partials        0        2      +2

Impacted Files	Coverage Δ
Source/iOS/OIDExternalUserAgentIOS.m	0% <ø> (ø)	⬆️
Source/iOS/OIDAuthState+IOS.m	0% <0%> (ø)	⬆️
Source/OIDEndSessionResponse.m	0% <0%> (ø)
Source/OIDEndSessionFlowSession.m	0% <0%> (ø)
Source/OIDServiceDiscovery.m	93.95% <100%> (+0.53%)	⬆️
UnitTests/OIDEndSessionRequestTests.m	100% <100%> (ø)
UnitTests/OIDServiceDiscoveryTests.m	82.27% <100%> (-2.23%)	⬇️
Source/OIDAuthState.m	40.9% <20%> (-5.12%)	⬇️
Source/OIDEndSessionRequest.m	76.47% <76.47%> (ø)
Source/OIDAuthorizationService.m	16.83% <0%> (-46.14%)	⬇️
... and 55 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 d1270ec...b9b6eac. Read the comment docs.

[WilliamDenniss]

Thanks for the contribution. Can you sign the CLA, or link to your signed CLA on that page if you have already?

[a053a]

Is there an ETA when this the travis build error will be fixed and this feature merged with master? Hoping to use this soon. @WilliamDenniss @dgommers

[WilliamDenniss]

@dgommers can you sign the CLA? Otherwise we'll have to abandon this CL.

[dgommers]

@WilliamDenniss Should be done now.

[WilliamDenniss]

Oh cool! I wasn't sure if you were going to, so I made an implementation in #259 yesterday. Looks like I took a slightly different approach, bundling the action logic into OIDAuthorizationSession, whereas you created a separate object OIDEndSessionFlowSession. I was on the fence about that.

[dgommers]

Alright, do you still need anything from me or shall we close it?

[WilliamDenniss]

It's a pity that we overlapped on this one.

However, looking at your changes there's still a few that are not in the logout development branch. Can you checkout the dev-logout branch so we're working on the same tree, and add your work to the AuthState+* files in a fresh commit, making a PR to that branch?

I noticed you change the nullability of the default iOS controller, perhaps that change could be a separate PR (on the main branch as it seems unrelated).

Also, have you tried out logout against a real server? I have not, so I'm just working off the spec right now, so would really appreciate someone testing the dev-logout branch and letting me know if it actually works =)

[dgommers]

Would you like to cherrypick the lines into the files that you've created or just get my files into the dev-logout? I'm not aware of your latest work on that branch as I've not been working with OAuth for a few months now. Manually transferring the code sounds a bit more work so I might not have time for that on the short term.

Yes! I've tested it and it works good with a real Keycloak server. Actually our product is already using it since March without issues :).

[WilliamDenniss]

If you can cherry-pick those lines (i.e. changes to AuthState+* and anything else missing that we should include) onto the current dev-logout branch that would be great!

The nullability change could be a separate PR.

[WilliamDenniss]

Closing in favor of #259