Skip to content

[18.0][FIX] password_security: treat an absent policy parameter as disabled#953

Open
kwoychesko wants to merge 1 commit into
OCA:18.0from
bpmi:18.0-fix-password_security-zeros
Open

[18.0][FIX] password_security: treat an absent policy parameter as disabled#953
kwoychesko wants to merge 1 commit into
OCA:18.0from
bpmi:18.0-fix-password_security-zeros

Conversation

@kwoychesko

Copy link
Copy Markdown

Bug

res.config.settings deletes an ir.config_parameter when its integer field is saved as 0. But _get_all_password_params() falls back to non-zero defaults when a parameter is missing:

"minimum_hours": int(params.get_param("password_security.minimum_hours", default=60)),
"lower": int(params.get_param("password_security.lower", default=1)),
...

So setting e.g. Minimum Hours or Uppercase to 0 in the UI silently reverts to the old default — the rule can't actually be turned off, even though the settings page then shows 0.

Fix

Default every lookup to 0, so an absent parameter disables its rule — which is exactly what the settings page already displays. The shipped defaults are still seeded by the post_init hook, so a fresh install behaves the same; this only changes what a deleted / zeroed parameter means.

Adds a regression test.

Refs #865 — this lets an admin actually disable the policy. It deliberately does not change the seeded install defaults; whether password_security should ship with no policy by default is a separate design question.

The settings page deletes an ir.config_parameter when a value is saved as
0, but _get_all_password_params() fell back to non-zero defaults for a
missing parameter -- so setting e.g. Uppercase or Minimum Hours to 0 in the
UI silently reverted to the old default and the rule could not be turned
off. Default every lookup to 0 so that an absent parameter disables its
rule, matching what the settings page already displays.

Refs OCA#865.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

mod:password_security Module password_security series:18.0

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants