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

Add global user session timeout configuration #14343

Merged
merged 31 commits into from Jan 23, 2023
Merged

Conversation

patrickmann
Copy link
Contributor

Resolves #11379

A new parameter global_session_timeout_interval is added to server.conf.
When this is present, you can no longer configure a timeout per user; and it overrides any previously configured user-specific timeout value!

Re-enable individual session timeouts by deleting the global setting from server.conf.

@bernd
Copy link
Member

bernd commented Jan 3, 2023

@patrickmann @boosty Is there a reason that this setting is a server.conf setting instead of a global runtime setting in the database?

Server config file settings have the drawback that they need to be in sync on all Graylog nodes and that changing the setting requires a server restart. The global user session timeout is a good candidate for a runtime rather than a server config file setting.

@patrickmann
Copy link
Contributor Author

The global user session timeout is a good candidate for a runtime rather than a server config file setting

Makes sense - I'll fix that

@bernd
Copy link
Member

bernd commented Jan 3, 2023

The global user session timeout is a good candidate for a runtime rather than a server config file setting

Makes sense - I'll fix that

Thank you! 🙏

Please talk with a frontend developer about where users would set the global session timeout. I probably fits somewhere in "System / Users and Teams".

@gally47
Copy link
Contributor

gally47 commented Jan 6, 2023

Frontend is ready for review. in System/Configuration I added a panel for User Configuration to enable the feature and assign a timeout value, in System/Users and Teams I made sure the user session timeout is disabled when the global timeout is enabled.

@gally47 gally47 requested a review from ousmaneo January 6, 2023 14:30
@patrickmann
Copy link
Contributor Author

@gally47 No input fields are visible in the system config form when there is no existing configuration entry in the cluster_config DB collection.
I would expect it to always be present - just disabled and with no / default value when it has never been set.

@patrickmann patrickmann marked this pull request as ready for review January 10, 2023 09:09
Copy link
Contributor

@ousmaneo ousmaneo left a comment

Choose a reason for hiding this comment

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

I tested it and the feature works ! I made some comments and suggestion about some findings

@gally47 gally47 requested a review from ousmaneo January 18, 2023 14:57
Copy link
Contributor

@ousmaneo ousmaneo left a comment

Choose a reason for hiding this comment

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

The frontend looks good! I added a small comment.

@gally47 gally47 requested a review from ousmaneo January 20, 2023 09:47
Copy link
Contributor

@ousmaneo ousmaneo left a comment

Choose a reason for hiding this comment

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

LGTM !
I noticed that we display the global timeout on the user overview page. We should also indicate that this is set globally.

user-overview

@gally47 gally47 requested a review from ousmaneo January 23, 2023 10:28
Copy link
Contributor

@ousmaneo ousmaneo left a comment

Choose a reason for hiding this comment

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

LGTM! Thank you!

@patrickmann patrickmann merged commit 23d116d into master Jan 23, 2023
@patrickmann patrickmann deleted the globalTimeout branch January 23, 2023 14:10
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Add configurable global user timeout configuration setting
4 participants