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

bug: _session cookie of deleted user not invalidated, feature: cookie max-age #765

Open
michelerenzullo opened this issue Jul 7, 2024 · 19 comments
Assignees
Labels
bug Something isn't working

Comments

@michelerenzullo
Copy link

michelerenzullo commented Jul 7, 2024

Problem:
There is a specific circumstance where it's impossible to sign-in anymore till the deletion of _session cookie in Chrome(Android) or Safari(iOS), even if the prompt=login.
Related issue here openid/AppAuth-Android#874 and how it is solved

Steps to reproduce the issue, assuming you have a signin with auto code exchange and prompt=login, i.e should always force a new login:

  1. You log-in in your app
  2. You clear the data / uninstall app
  3. From LogTo, you must delete the account where you logged in at step 1
  4. Open the app and try to login: you get always "invalid-grant"
    PlatformException(authorize_and_exchange_code_failed, Failed to authorize: [error: invalid_grant, description: grant request is invalid], null, null)

What would be the correct flow:

  1. //
  2. //
  3. //
  4. You should be able to login again, because prompt=login should be a guarantee that any (corrupted or not corrupted)_session cookie previously stored in the browser won't prevent you to login again

Explanations:

  • Seems that somehow is read the _session cookie that points to "dead" user information (deleted). The user will not be able to login anymore till he manually delete the _session cookie in Chrome.

Possible solution:

  • Find a way to set max_age: 0 along with prompt=login, so that the _session cookie auto-expire. In the specs there is actually an OpenID param called max-age or max-auth-age but I'm not sure LogTo implement it.
    You can notice, in the photo below that _session is valid for 2 weeks from login, it should be always 0 when prompt=login is used
Screenshot 2024-07-07 at 03 12 25
@michelerenzullo michelerenzullo added the bug Something isn't working label Jul 7, 2024
@michelerenzullo
Copy link
Author

michelerenzullo commented Jul 7, 2024

In order to don't break the user sign-in flow after a user deletion, we should either:

  • Workaround / Feature "maxAge": this will tweak the _session cookie so that is not always 1_209_600 but is maxAge dependant.
  • I think that, when prompt=login, _session cookie should never be parsed/validated, because everything related the previous session can be safely ignored.
  • If still, for some reason, we want to validate the _session cookie even if prompt=login, we should definitely find a way to don't throw an exception when the user info associated to the _session cookie doesn't exist anymore

on iOS a temp workaround to this issue is to set ephimeralSession true, but is not possible on Android and also is not something nice, since the browser have stored the connectors account (like AppleID / Gmail) to be easily tapped

@michelerenzullo
Copy link
Author

I tried to pass "max-age":0 or "max_age":0 but is still setting _session cookie expiration after 2 weeks

@michelerenzullo
Copy link
Author

Is there any update about?

@wangsijie
Copy link
Contributor

@michelerenzullo I'll take a look soon.

@wangsijie
Copy link
Contributor

Hi @michelerenzullo,

I attempted to reproduce the issue but was unsuccessful. Here's my process:

  1. Signed in as user1 and redirected to the application
  2. Deleted user1 via the admin console
  3. Cleared the application's local data (specifically, the localStorage)
  4. Reloaded the application

Result: I was redirected to the sign-in page. The password form appeared without any errors. This behavior is expected because the server automatically invalidates and clears the session for the deleted user.

Let me know if you need any clarification or if you'd like me to try a different approach.

@michelerenzullo
Copy link
Author

Hi @wangsijie ,

I believe there is something different in your environment, for app I mean any android or iOS application . The "_session" cookie is stored in Cookie Tab of Chrome or Safari (you can use Dev tools and remote debugging from your laptop). Even if you delete local storage, the cookie is still present and is erased only when doing an endSession on the endpoint of LogTo. Let me know how can I help you to reproduce, I will think something more advanced in a bit.

@michelerenzullo
Copy link
Author

The _session cookie is still holding the userID information has an hash, because this is verified in LogTo and it throws an error. The _session cookie can't be directly "deleted" because LogTo has no access to Chrome or Safari, only if executing an end session request this is erased, but this option is excluded since (point 2) the user deleted the app

@wangsijie
Copy link
Contributor

I can confirm that _session exists in my browser, and it was sent to the server.

I'll do another test in native application.

@michelerenzullo
Copy link
Author

Hi, is there any update about this issue? Thank you

@xiaoyijun
Copy link
Contributor

Hi @michelerenzullo , I just have a test, but I can't reproduce.

session-test.mp4

@michelerenzullo
Copy link
Author

That s strange, have you checked if there is the _session cookie? If there isn't this explain why your app is still able to sign in again. I tested with Flutter. I will do a screen record.

@michelerenzullo
Copy link
Author

michelerenzullo commented Jul 30, 2024

@xiaoyijun I created a similar test to yours, using an android emulator and a test app (flutter based), the cookie is kept stored in WebView Shell, so the bug persist in the same way as I described initially.

In your test you are using native layer Android, so perhaps you are not launching a Tab in an external browser (doesn't matter if is WebView Shell / Chrome / Firefox / Safari iOS...), so somehow the _session cookie is not stored in your scenario and therefore you not having the same issue.

In the video the "browser" is WebView Shell, but I can create the same test using Chrome and showing the same behaviour, where only deleting the cookie _session solve the issue (or generally speaking all data).

The issue is not actually the _session cookie, is that, LogTo backend try to parse the info inside this cookie, when it should not parse (since 'login' prompt), or it should safely "ignore" since are pointing to a deleted user, rather than reject, returning an "invalid_grant" error

2024-07-30.20.00.36.mp4

@michelerenzullo
Copy link
Author

Is there any update about? I can try to provide more info if can be helpful to you

@wangsijie
Copy link
Contributor

We are still unable to reproduce this in native SDKs, we are now trying to do it again with flutter SDK.

@simeng-li
Copy link
Contributor

@michelerenzullo are you using Logto Flutter SDK?

@michelerenzullo
Copy link
Author

I'm using flutter_appauth plugin, but I don't think makes any difference as long as it is flutter and not native SDKs. I can try to replicate with LogTo Flutter SDK as well to confirm.

@simeng-li
Copy link
Contributor

simeng-li commented Aug 8, 2024

No worries, just want to confirm the native environment you have. As for all Logto native SDKs (including Flutter), we do not store login session cookies to the WebView. E.g. Swift SDK we have the ephimeralSession set to true.

That is probably why @wangsijie and @xiaoyijun couldn't reproduce this issue.

@michelerenzullo
Copy link
Author

while this might be true for Swift SDK, using under the hood by default ephimeralSession, this is not true for LogTo Dart SDK, I just verified, LogTo Dart SDK stores for 14 days the _session cookie, I used chrome://inspect/#devices. so if is flutter_appauth or is LogTo Dart SDK the bug is still here

@michelerenzullo
Copy link
Author

michelerenzullo commented Aug 10, 2024

I want to let you know that I found a workaround for my situation that does the job but I strongly believe that a more good / elegant approach should be done from your side, in the backend, my solution is to force an endSession when invalid_grant is detected during signIn or during refresh tokens (well actually both are the same thing under the hood). Reference here

end session wipe out _session cookie but is not the best approach in my opinion

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Development

No branches or pull requests

4 participants