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

Only show Auto Log Out Time if you have the User Management role #4071

Closed
wants to merge 1 commit into from
Closed

Only show Auto Log Out Time if you have the User Management role #4071

wants to merge 1 commit into from

Conversation

MarcBanyard
Copy link
Contributor

Changed the new feature in Pull Request #4064 to only show if the user has the User Management role set on their account.
This was requested by @TheWitness

If a User needs to be able to set their own Auto Log Out Time they need to have the Maintain Custom Graph and User Settings enabled under Configuration > Users > User Account under the General tab
image
and have the Update Profile and User Management enabled under Configuration > Users > User Account under the Permissions tab
image
Then the user will be able to see the Auto Log Out Time option when they edit their profile
image

If an Administrator needs to set this for other users they must have the Console Access and User Management enabled under Configuration > Users > User Account under the Permissions tab
image

Then when they are editing a user account the additional option called Auto Log Out Time can be found within the User Settings tab.
image

The default behaviour has not been changed and it is set to be Never as default so you have to enable this on per user if required.

@MarcBanyard
Copy link
Contributor Author

I know in the last comment you made in Pull Request #4064 that a session cookie may be a better way to go, but I have used the existing way that a session is logged out after the specified time and it works very well.

Bearing in mind that enabling the Support Authentication Cookies under Configuration > Settings > Authentication enables the system to allow users to stay logged in for 30 days before being automatically logged out.
image

I think this modification adds that little bit extra control for people who need it and utilises the same code that is used when the Support Authentication Cookies option is enabled.

@TheWitness
Copy link
Member

Let's slow this down just a smidge. Does auto-refresh still work? I need to review the design to make sure it does not add additional confusion.

@TheWitness
Copy link
Member

TheWitness commented Jan 22, 2021

So, what I'm okay with is the following. At the administration level and NOT the user level, I would be okay with changing the auto-logout time to be greater than the "session.gc_maxlifetime" to something to be something greater, but only at the Administrative level and globally.

It's true that if a user does not use their session within "session.gc_maxlifetime" seconds that the session file is subject to be removed (aka the user is logged off anyway). But there is no guarantee that it'll be deleted immediately. Apache and Nginx will only delete the session file during garbage cleanup.

So, last note. The right way to extend auto-logout in a reliable way is to increase the "session.gc_maxlifetime", but if you want to make that setting 'viewable' and configurable from the GUI, having a GLOBAL setting that is less than the "session.gc_maxlifetime" would be fine.

To do this right though, the admin should change the gc_maxlifetime and the setting should always be less, and of course, the dropdown you created would be great, but keep that max value less than the variable setting.

This also needs better documentation. Maybe @bmfmancini can knock something out. So, no user setting.

@bmfmancini
Copy link
Member

Sounds good ill add it to the ticket as a reminder

@MarcBanyard
Copy link
Contributor Author

Let's slow this down just a smidge. Does auto-refresh still work? I need to review the design to make sure it does not add additional confusion.

Yes the Auto-Refresh still works fine.

@MarcBanyard
Copy link
Contributor Author

MarcBanyard commented Jan 22, 2021

So, what I'm okay with is the following. At the administration level and NOT the user level, I would be okay with changing the auto-logout time to be greater than the "session.gc_maxlifetime" to something to be something greater, but only at the Administrative level and globally.

It is at the Administration Level unless the user has the User Management assigned to them, but this would then make them have user administration rights.

I can move this to the Configuration > Users > User Account > General tab if required, but that will require an update to the database as a new column would need to be added to the user_auth table which I was trying to avoid as I didn't want to mess with the core code too much.

It is never visible or settable to a user unless they have the User Management role assigned to their user account in addition to the following

  • Console Access role (which would make you an administrator
  • Maintain Custom Graph and User Settings which would enable a user to edit settings in their own account.

It's true that if a user does not use their session within "session.gc_maxlifetime" seconds that the session file is subject to be removed (aka the user is logged off anyway). But there is no guarantee that it'll be deleted immediately. Apache and Nginx will only delete the session file during garbage cleanup.

If the Auto Log Out Time is not set then nothing has changed.

So, last note. The right way to extend auto-logout in a reliable way is to increase the "session.gc_maxlifetime", but if you want to make that setting 'viewable' and configurable from the GUI, having a GLOBAL setting that is less than the "session.gc_maxlifetime" would be fine.

To do this right though, the admin should change the gc_maxlifetime and the setting should always be less, and of course, the dropdown you created would be great, but keep that max value less than the variable setting.

Essentially all of the core cacti code is exactly the same and the only thing that has been done is an option has been put in the Configuration > Users > User Account > User Settings tab that only shows if you have the User Management permission set on your account.
Then to get this to work the existing code to automatically log people out has been added to in the way of an additional if statement at the top to check if the Auto Log Out Time has been set.

This also needs better documentation. Maybe @bmfmancini can knock something out. So, no user setting.

I've documented how it works in this pull request as well as explaining under what conditions the option is available, but if it is never set then it doesn't change anything in Cacti.

@MarcBanyard MarcBanyard closed this Feb 2, 2021
@github-actions github-actions bot locked and limited conversation to collaborators May 4, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants