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

Allow idleTimeout/lifespan larger than 32-bit signed integer. #79858

Merged
merged 1 commit into from
Oct 7, 2020

Conversation

azasypkin
Copy link
Member

@azasypkin azasypkin commented Oct 7, 2020

In this PR we add a client-side startTimer function that allows us to workaround a maximum allowed JavaScript timeout (32-bit signed integer or 2,147,483,647 ms or 24.8 days). The main idea here is that in case of large timeout values this method recursively calls setTimeout keeping track of current timeout ID so that it can be cleared at any time.

Unblocks: #68885
Fixes: #22374


Release note: Kibana can now properly handle values for xpack.security.session.idleTimeout and xpack.security.session.lifespan that are larger than ~24 days.

@azasypkin azasypkin added release_note:fix Team:Security Team focused on: Auth, Users, Roles, Spaces, Audit Logging, and more! Feature:Security/Authentication Platform Security - Authentication v8.0.0 v7.10.0 labels Oct 7, 2020
*/
private startTimer(updater: (timeoutID: number) => void, callback: () => void, timeout: number) {
// Max timeout is the largest possible 32-bit signed integer or 2,147,483,647 or 0x7fffffff.
const maxTimeout = 0x7fffffff;
Copy link
Member Author

Choose a reason for hiding this comment

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

note: didn't want to create a dedicated top level const for this purely "technical" thing....

@azasypkin azasypkin marked this pull request as ready for review October 7, 2020 16:22
@azasypkin azasypkin requested a review from a team as a code owner October 7, 2020 16:22
@elasticmachine
Copy link
Contributor

Pinging @elastic/kibana-security (Team:Security)

Copy link
Contributor

@jportner jportner left a comment

Choose a reason for hiding this comment

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

Tested locally with small and large timeout values. LGTM! 🎉

test(`stop works properly for large timeouts`, async () => {
http.fetch.mockResolvedValue({
...defaultSessionInfo,
idleTimeoutExpiration: now + 5_000_000_000,
Copy link
Contributor

Choose a reason for hiding this comment

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

TIL you can use underscores in JS numbers 😁 pretty cool

Copy link
Member Author

Choose a reason for hiding this comment

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

Yeah, I like it as well. Unfortunately we cannot use it in a non-test code yet (at least something was complaining about that during the build) 🙁 Should probably be easy to tweak our build/transpilation pipeline though, I see it's Stage 4 proposal already.

@azasypkin
Copy link
Member Author

7.10/7.10.0: aa8f83a
7.x/7.11.0: ab26307

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
backported Feature:Security/Authentication Platform Security - Authentication release_note:fix Team:Security Team focused on: Auth, Users, Roles, Spaces, Audit Logging, and more! v7.10.0 v7.11.0 v8.0.0
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Large session timeout number expires session almost instantaneously
4 participants