logto-io / js

🤓 Logto JS SDKs.
https://docs.logto.io/quick-starts/
MIT License
67 stars 40 forks source link

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

Open michelerenzullo opened 4 months ago

michelerenzullo commented 4 months ago

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 https://github.com/openid/AppAuth-Android/issues/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:

Possible solution:

Screenshot 2024-07-07 at 03 12 25
michelerenzullo commented 4 months ago

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

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 commented 4 months ago

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

michelerenzullo commented 4 months ago

Is there any update about?

wangsijie commented 4 months ago

@michelerenzullo I'll take a look soon.

wangsijie commented 4 months ago

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 commented 4 months ago

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 commented 4 months ago

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 commented 4 months ago

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 commented 3 months ago

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

xiaoyijun commented 3 months ago

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

https://github.com/user-attachments/assets/079fe5a9-0c0f-49dd-b785-42a4f7cf47e2

michelerenzullo commented 3 months ago

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 commented 3 months ago

@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

https://github.com/user-attachments/assets/075e61ed-fcb9-417d-931e-738780563b13

michelerenzullo commented 3 months ago

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

wangsijie commented 3 months ago

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

simeng-li commented 3 months ago

@michelerenzullo are you using Logto Flutter SDK?

michelerenzullo commented 3 months ago

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 commented 3 months ago

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 commented 3 months ago

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 commented 3 months ago

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