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

Implement RP-initiated logout #132

Closed
zachmargolis opened this issue Jul 19, 2017 · 11 comments
Closed

Implement RP-initiated logout #132

zachmargolis opened this issue Jul 19, 2017 · 11 comments

Comments

@zachmargolis
Copy link

tl;dr is it OK to submit a PR to add RP-initiated logout to this repo?

Hi there,

For our IDP, we implemented RP-initiated logout (http://openid.net/specs/openid-connect-session-1_0.html#RPLogout). We'd like to encourage our clients to use this library but it doesn't currently support this feature.

As an outline, we could add another method to the OIDAuthorizationService like this:

+ (id<OIDAuthorizationFlowSession>)
    presentAuthorizationRequest:(OIDLogoutRequest *)request
       presentingViewController:(UIViewController *)presentingViewController
                       callback:(OIDAuthorizationCallback)callback;

and add the necessary OIDLogoutRequest and such as needed, and expand OIDAuthorizationFlowSession to check for logout URLs to continue with as well as authorize URLs.

@WilliamDenniss
Copy link
Member

Yes, this is in scope for the library. Would be happy to review a PR.

@zachmargolis
Copy link
Author

@WilliamDenniss great! Here's a PR adding some of the basic objects needed for serialization: #138

Things get tricky for me when trying to figure out a right way to add on to OIDAuthorizationService

Which would be preferred?

  1. Store a separate OIDLogoutRequest on OIDAuthorizationFlowSessionImplementation
  2. Store only one request on the flow, and update both OIDLogoutRequest and OIDAuthorizeRequestto conform to some protocol ("OIDCallbackRequest" or something) so that they can both have authorizationRequestURL and be associated with a callback? They are very similarly "shaped" objects

@jaishankar
Copy link

@zachmargolis / @WilliamDenniss , Any update please on this Pullrequest, I am looking for this capability too!

@zachmargolis
Copy link
Author

@jaishankar Hi, thanks for pinging. I am no longer working on an OIDC project so I'm not working to update this PR right now, but please feel free to pick ip #138 and address the feedback if you'd like!

@luksfarris
Copy link

I created PR #191 based on Zach's suggestion

@lordcodes
Copy link

I need to implement logout into my application, which is using AppAuth-iOS and an OIDC IdentityServer.

I understand building this into AppAuth-iOS has been partially worked on. From reading this and the different PRs-relating to it.

Are there any updates on when this might be introduced to the library? Happy to help, although not really that big on Objective-C knowledge (only Swift).

Alternatively if it is a while off, does anyone have any ideas of how I would go about implementing this into the app without AppAuth-iOS doing it all for me?

Currently, once you login, you are completely unable to logout, even after re-installing the app, due to Safari persisting the data.

@luksfarris
Copy link

Hi @andrewlord1990 regarding your question:

Alternatively if it is a while off, does anyone have any ideas of how I would go about implementing this into the app without AppAuth-iOS doing it all for me?

You just need to check the logout url in the discovery document and call it in the browser passing the correct parameters. Here's an example https://github.com/okta/okta-sdk-appauth-ios/pull/30/files

@lordcodes
Copy link

@luksfarris Thanks very much, that's awesome!

@dgommers
Copy link

dgommers commented Mar 2, 2018

Recently our project switched to AppAuth with the outlook on the new logout feature, which we did have in our previous library. Unfortunately, it has been hanging here for quite a while now.

Our team has enough time to finish it up next week. However, the current status is not very clear to me. For example, what is pending for further processing of #196? Not sure if it should point directly to master either.

@luksfarris and @WilliamDenniss, maybe you could indicate what work items are left so we can finish it for next week? Just trying to unload you and unlock ourselves.

@luksfarris
Copy link

I've separated the work in 3 pull requests:

1 - PR #191 fixed the previously opened PR (#138) that contained the RP-Initiated logout Request/Response classes. Those were merged to the branch openid:dev-logout.
2 - PR #196 changes the implementation to treat requests that need opening the browser in a generic way (previously only the authentication request used the browser). Just like you, @dgommers @andrewlord1990 and @jaishankar , I'm also waiting for this to be merged.
3 - The final pull request would be to just implement the logout controllers. This is a small amount of work, but I didn't want to do it before #196 was merged.

On my personal project I'm just calling the logout url in the browser myself, without going through AppAuth-iOS, so you shouldn't be blocked by this as well if you're under a deadline like I was

@lordcodes
Copy link

Thanks for all of this information and the effort to get this feature implemented. This will be really useful when I come to implement log out soon! Appreciate it 🙏

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

7 participants