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

[flutter_appauth][flutter_appauth_platform_interface] Add preferEphemeralSession option to logout #349

Merged

Conversation

ziegler-daniel
Copy link
Contributor

Hi,

at the moment it is possible to pass the option preferEphemeralSession: true to the sign in call, but not to the end session call. Therefore, before every logout a pop-up like this is shown to the user:
image

If no pop-up was displayed before the login, it would be nicer if there is also none before the logout. So, I added the functionality to set the preferEphemeralSession option also for the end session call.

It would be nice if this feature could be included into the library.

@MaikuB
Copy link
Owner

MaikuB commented Jul 19, 2022

Whilst the popup is removed, using an ephemeral session means a "private" browser is used. Users won't actually get signed out as a result. The following is a recording from sigining in and signing out normally using the example

regular.end.session.mp4

At the end I attempt to sign in again to show that the browser no longer knows I've been signed in as I'm prompted to sign in again. Now let's repeat this with the end session request where preferEphemeralSession is set to true

ephemeral.sign.out.mp4

After signing out, I've tried to sign in again and still recognises that I'm signed in. This effectively means the end session request hasn't really done anything. With this is in mind and do let me know if I've missed something, I don't see a reason why this feature added by the PR since the user's session is being kept.

Edit: reuploaded videos as .mp4 files instead of .mov

@dconlisk
Copy link

Ah, that's a shame that it doesn't work. @MaikuB do you know if it's possible to update the text in the popup from "Sign In" to "Sign Out" when ending the session?

@MaikuB
Copy link
Owner

MaikuB commented Jul 19, 2022

@dconlisk no as the prompt comes from Apple's authorisation APIs

@dconlisk
Copy link

@MaikuB I was afraid that might be the case - thanks!

@ziegler-daniel
Copy link
Contributor Author

@MaikuB thank you for your response. In my opinion the option preferEphemeralSession = true must only be used for the logout call, if it was also used for the sign in call. According to the apple documentation no data is stored in an ephemeral session. So, if you use preferEphemeralSession = true for sign in, you won't get the described problem. Using different values for preferEphemeralSession for sign in and end session call would be like using two different browsers for the two calls.

The end session call invalidates the current refresh token regardless of the preferEphemeralSession option.

I can add this to the documentation to prevent misunderstandings.

@MaikuB
Copy link
Owner

MaikuB commented Jul 20, 2022

@ziegler-daniel ok gotcha. Updating the documentation around using this makes sense to me. Could you do this as part of the PR?

@ziegler-daniel
Copy link
Contributor Author

Sure, I added some documentation at the EndSessionRequest and in the readme file. Is that okay with you?

Copy link
Owner

@MaikuB MaikuB left a comment

Choose a reason for hiding this comment

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

Yes placing the documentation like you have is fine though I left comments on the usage of commas :)

flutter_appauth/README.md Outdated Show resolved Hide resolved
flutter_appauth/README.md Outdated Show resolved Hide resolved
@ziegler-daniel
Copy link
Contributor Author

Yes placing the documentation like you have is fine though I left comments on the usage of commas :)

Thank you for you feedback. I adjusted it.

@MaikuB MaikuB merged commit 6ea276b into MaikuB:master Jul 22, 2022
@fbernaly
Copy link

@ziegler-daniel: Thanks for this PR. This is what I needed for my app.

rvdsoft pushed a commit to digitalrmdy/flutter_appauth that referenced this pull request Mar 23, 2023
…eralSession option to logout (MaikuB#349)

* feat: Add ephemeral option to logout

* Make preferEphemeralSession non-nullable

* Add documentation

* Adjust documentation

Co-authored-by: Daniel Ziegler <daniel.ziegler@senacor.com>
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.

4 participants