Skip to content

Conversation

@bungle
Copy link
Owner

@bungle bungle commented Sep 25, 2025

Summary

There are use cases where immediate deletion of remember cookies causes issues. Those use cases mostly happen in client browser based JS apps, single page apps, where the login semantics is not clearly separated from the app.

More info https://konghq.atlassian.net/browse/KAG-7803

@bungle bungle force-pushed the fix/remember-concurrency branch 2 times, most recently from a83693b to 0cd4c24 Compare September 25, 2025 09:39
### Summary

There are use cases where immediate deletion of remember cookies
causes issues. Those use cases mostly happen in client browser
based JS apps, single page apps, where the login semantics is not
clearly separated from the app.
@findns94
Copy link

findns94 commented Sep 29, 2025

Looks good! But I still have some questions about the remember_session behavior.

  1. The customer wants to let multiple browser clients with the same remember_session get refreshed at the same time, that means, multiple HTTP request with the same old remember_session will be sent to Kong within a short time window. My question is, can this PR let all of the HTTP request get the same refreshed remember_session and return successful response code instead of current behavior, letting one of the HTTP request get refreshed remember_session successfully, other requests will get 401.
  2. Do we need a configuration to allow remember_session deletion timeout configurable? If the schema need to modify, it will require migration in databases.
  3. If we set remember_cookie from deletion to timeout, will it cause more pressure to Redis? Because some keys may remain in Redis for some time, this will make Redis overload if too many requests want to refresh their remember_session in some extreme circumstances.

@findns94
Copy link

findns94 commented Sep 30, 2025

I tested this PR and the result is good. After new remember_session created, the old remember_session exists for 10 seconds as default stale_ttl is 10 seconds.

Copy link

@findns94 findns94 left a comment

Choose a reason for hiding this comment

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

Great job! I will let Yankun approve this PR if the customer's response is positive.

@bungle bungle merged commit 556e999 into master Oct 28, 2025
2 checks passed
@bungle bungle deleted the fix/remember-concurrency branch October 28, 2025 14:37
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.

3 participants