MaikuB / flutter_appauth

A Flutter wrapper for AppAuth iOS and Android SDKs
270 stars 243 forks source link

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

Closed ziegler-daniel closed 2 years ago

ziegler-daniel commented 2 years ago

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 commented 2 years ago

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

https://user-images.githubusercontent.com/25263378/179745973-9b621bb5-d2ed-4589-b712-e705e2e64f0f.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

https://user-images.githubusercontent.com/25263378/179746253-2546f778-6aab-479a-9a3e-400507270859.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 commented 2 years ago

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 commented 2 years ago

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

dconlisk commented 2 years ago

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

ziegler-daniel commented 2 years ago

@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 commented 2 years ago

@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 commented 2 years ago

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

ziegler-daniel commented 2 years ago

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.

fbernaly commented 2 years ago

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