Skip to content

Set MaxAge for cookie based on WebSession expiry#58091

Merged
avatus merged 1 commit intomasterfrom
avatus/web_auth_max_age
Aug 22, 2025
Merged

Set MaxAge for cookie based on WebSession expiry#58091
avatus merged 1 commit intomasterfrom
avatus/web_auth_max_age

Conversation

@avatus
Copy link
Copy Markdown
Contributor

@avatus avatus commented Aug 20, 2025

Currently our web session cookies are literally session cookies, which means "generally" they are removed the browser is closed. (i say generally because browsers all choose to do their own whacky things with these cookies)

To alleviate this and get a more consistent behavior, we can set the max-age of our session cookies so that they persist after a browser is closed. We will set the max-age in seconds based on the time left until the underlying WebSession expires (this matches the sessionTTL, default is 12 hours for example). this wont completely fix "frequent reauth" but its a first step.

Because the backend already assumed these tokens didn't have an expiration or max-age, we aren't giving up on any security from our end here (we didnt check expiration of the cookie itself). The cookies are still validates against the WebSession expiration, this just allows the users web session to exist after closing the browser. (happy to do an update to make this behavior opt-in as well, but I don't think its necessary). Nothing would be different here if the user just left their browser open for 12 hours, so expiration behavior will not change.

there will be an e pr for this as well when setting web session cookies from some sso stuff (2 lines).

im also ok with waiting on this and implementing it with a "remember me" button. i would bring in the UX team for that one tho

@avatus avatus added the no-changelog Indicates that a PR does not require a changelog entry label Aug 20, 2025
@avatus avatus requested review from kimlisa and ryanclark August 20, 2025 00:39
@github-actions github-actions bot requested review from aadc-dev and r0mant August 20, 2025 00:39
@avatus avatus removed request for aadc-dev and r0mant August 20, 2025 00:39
@avatus avatus force-pushed the avatus/web_auth_max_age branch from a60644f to 9f52dd0 Compare August 20, 2025 00:51
@avatus avatus requested a review from rudream August 20, 2025 00:59
Copy link
Copy Markdown
Contributor

@kimlisa kimlisa left a comment

Choose a reason for hiding this comment

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

im also ok with waiting on this and implementing it with a "remember me" button. i would bring in the UX team for that one tho

+1 implementing it as an opt in feature, if someone else disagrees, i won't object

@zmb3
Copy link
Copy Markdown
Collaborator

zmb3 commented Aug 22, 2025

I say ship it as is. Less config options are better IMO.

@avatus avatus requested a review from kimlisa August 22, 2025 15:04
@public-teleport-github-review-bot public-teleport-github-review-bot bot removed the request for review from ryanclark August 22, 2025 20:06
@avatus
Copy link
Copy Markdown
Contributor Author

avatus commented Aug 22, 2025

Gonna let this one sit in master for a few weeks before backporting.

@avatus avatus enabled auto-merge August 22, 2025 20:07
@avatus avatus force-pushed the avatus/web_auth_max_age branch from 9f52dd0 to 5cb8608 Compare August 22, 2025 20:56
@avatus avatus added this pull request to the merge queue Aug 22, 2025
Merged via the queue into master with commit 98ff655 Aug 22, 2025
41 checks passed
@avatus avatus deleted the avatus/web_auth_max_age branch August 22, 2025 21:42
github-merge-queue bot pushed a commit that referenced this pull request Nov 10, 2025
* Set MaxAge for cookie based on WebSession expiry (#58091)

* Set MaxAge in web cookie if time until expiry is > 0 (#58293)

This updates the logic to only setting max-age if the provided expiry is
in the future rather than any non-zero value.

* Respect WebIdleTimeout in bearerTokenTTL (#59645)

The webUI will log a user out due to invalid token if they haven't
pinged the server within the idle time, which defaulted to the bearer
token default (10 minutes). Instead, we will only default to 10 minutes
if the web_idle_timeout is not configured in the cluster config
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

no-changelog Indicates that a PR does not require a changelog entry size/sm

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants