Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Make the number of days since last login before making a user inactive a config #8416

Conversation

cmadjar
Copy link
Collaborator

@cmadjar cmadjar commented Feb 28, 2023

Brief summary of changes

This adds a new Config in the config module to allow projects to configure the number of days since a user last logged in before making their account inactive.

Testing instructions

  1. Run the SQL patch SQL/New_patches/2023-02-28_create_max_days_inactive_config_for_users.sql
  2. Create a new user if needed and login as that user
  3. For that new user, update the last login to 3 years ago UPDATE user_login_history SET Login_timestamp='2020-02-28 19:28:35' WHERE userID=<new user id>
  4. Try logging in again with that user. You should see the following:
    Screen Shot 2023-02-28 at 2 32 22 PM

@cmadjar cmadjar added Feature PR or issue introducing/requiring at least one new feature SQL PR contains SQL modifications such as schema changes and new SQL scripts labels Feb 28, 2023
@cmadjar cmadjar added this to the 25.0.0 milestone Feb 28, 2023
@cmadjar cmadjar changed the title make the number of days since last login before making a user inactive Make the number of days since last login before making a user inactive a config Feb 28, 2023
@cmadjar cmadjar closed this Mar 1, 2023
@cmadjar cmadjar reopened this Mar 1, 2023
@@ -356,7 +361,8 @@ class SinglePointLogin
}

if ($row['Active'] == 'N'
|| $this->disabledDueToInactivity($username, 365)
|| ($maxDaysInactive
Copy link
Collaborator

Choose a reason for hiding this comment

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

because of the not-so-predictable behaviour of intval(), I would instead do a more explicit check like (!empty($maxDaysInactive) && is_numeric($maxDaysInactive))

intval:

Return Values ¶
The integer value of value on success, or 0 on failure. Empty arrays return 0, non-empty arrays return 1.

The maximum value depends on the system. 32 bit systems have a maximum signed integer range of -2147483648 to 2147483647. So for example on such a system, intval('1000000000000') will return 2147483647. The maximum signed integer value for 64 bit systems is 9223372036854775807.

Strings will most likely return 0 although this depends on the leftmost characters of the string. The common rules of integer casting apply

@cmadjar
Copy link
Collaborator Author

cmadjar commented Mar 1, 2023

@ridz1208 Thanks for the quick review! Your comments should be addressed and I added a note to the roadmap call to discuss whether it is OK to have maxDaysUserInactive not set for LORIS.

@driusan
Copy link
Collaborator

driusan commented Mar 7, 2023

@cmadjar needs rebase, conflicts.

add default value to config
fix phrasing of the config to something hopefully clearer
modify RB to add the new config
@cmadjar cmadjar force-pushed the make_nb_of_days_inactive_before_making_user_inactive_a_config branch from 6549aef to 4368970 Compare March 7, 2023 18:06
@driusan
Copy link
Collaborator

driusan commented Mar 8, 2023

@cmadjar seems like there's a problem with the tests on this PR, can you look into it when you get a chance?

@cmadjar cmadjar closed this Mar 9, 2023
@cmadjar cmadjar reopened this Mar 9, 2023
@cmadjar
Copy link
Collaborator Author

cmadjar commented Mar 10, 2023

@driusan this passes the tests now.

@driusan driusan merged commit 396cad2 into aces:main Mar 10, 2023
# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
Feature PR or issue introducing/requiring at least one new feature SQL PR contains SQL modifications such as schema changes and new SQL scripts
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants