Add enable_password_config

[amandahla]

Overview

Add configuration to change password_config behavior.

Rationale

This way the administrator can disable password login if the user login via SSO, for example.

Juju Events Changes

Module Changes

Library Changes

Checklist

• The charm style guide was applied
• The contributing guide was applied
• The changes are compliant with ISD054 - Manging Charm Complexity
• The documentation is generated using src-docs
• The documentation for charmhub is updated.
• The PR is tagged with appropriate label (urgent, trivial, complex)

[amandahla]

Fix #62

[merkata]

Looking at the config documentation, password_config is nested, so I'm suggesting to expose password_config and deal with enabling/disabling in the function. I imagine if we want to deal with password configuration in the future, if we go with the current route we would add additional fields, like minimum_length_policy_password_config (to get to password_config.policy.minimum_length and change the min pass length for instance). Another thing to consider is when we render the current password_config to update what we're interested in and the rest should remain unchanged, so if you toggle password_config on and off and you have made changes to the policy (like the min pass length) it does not get lost. It would be clearer to me also that a disable_password_config that takes a password_config object changes the enabled parameter and not enable_password_config. I'm not sure how the function is called though - shouldn't pebble be made aware of that change via the on.config_changed path?

[amandahla]

Looking at the config documentation, password_config is nested, so I'm suggesting to expose password_config and deal with enabling/disabling in the function. I imagine if we want to deal with password configuration in the future, if we go with the current route we would add additional fields, like minimum_length_policy_password_config (to get to password_config.policy.minimum_length and change the min pass length for instance). Another thing to consider is when we render the current password_config to update what we're interested in and the rest should remain unchanged, so if you toggle password_config on and off and you have made changes to the policy (like the min pass length) it does not get lost. It would be clearer to me also that a disable_password_config that takes a password_config object changes the enabled parameter and not enable_password_config. I'm not sure how the function is called though - shouldn't pebble be made aware of that change via the on.config_changed path?

• I considered the scope of the issue as something simple like "disable or enable password". So, that's why other options regarding passwords are left as default. This could be improved in follow-up PRs.
• There is no enable_password_config because the template used by the start.py script already enable password. So the change is just needed in case of disabling it.

[gregory-schiano]

Isn't something missing in this PR ?
I see a new config added to the charm_start but it doesn't seem to be used, and there's a method that disable password config but in an "hardcoded" way

[amandahla]

Isn't something missing in this PR ? I see a new config added to the charm_start but it doesn't seem to be used, and there's a method that disable password config but in an "hardcoded" way

I have no idea what happened, maybe it was lost while merging with previous PRs, but now I added to change_config (pebble) the call to this function.

[cbartz]

Looks good, have one nitpicky comment.

[github-actions]

Test coverage for 08a39ef

Name                            Stmts   Miss Branch BrPart  Cover   Missing
---------------------------------------------------------------------------
src/actions/__init__.py             3      0      0      0   100%
src/actions/register_user.py       22      0      2      0   100%
src/actions/reset_instance.py      21      0      2      0   100%
src/charm.py                      175      8     32      4    94%   124-125, 177-178, 197-198, 239, 253
src/charm_state.py                 64      1     12      1    97%   141
src/charm_types.py                 11      0      0      0   100%
src/database_client.py             53      1     10      3    94%   35, 47->exit, 69->exit
src/database_observer.py           54      4      6      0    93%   70-72, 88
src/exceptions.py                   4      0      0      0   100%
src/mjolnir.py                     76      8     20      2    88%   60-64, 73, 93-96
src/observability.py                9      0      0      0   100%
src/pebble.py                      77     10      6      3    84%   90-91, 93, 95, 109-114
src/saml_observer.py               45      1      8      0    98%   64
src/synapse/__init__.py             3      0      0      0   100%
src/synapse/api.py                161      2     22      2    98%   207, 376
src/synapse/workload.py           200      6     30      5    95%   361-362, 378, 427->430, 476, 478, 483
src/user.py                        24      0      4      0   100%
---------------------------------------------------------------------------
TOTAL                            1002     41    154     20    95%

Static code analysis report

Run started:2023-10-25 17:26:19.525898

Test results:
  No issues identified.

Code scanned:
  Total lines of code: 5054
  Total lines skipped (#nosec): 4
  Total potential issues skipped due to specifically being disabled (e.g., #nosec BXXX): 0

Run metrics:
  Total issues (by severity):
  	Undefined: 0
  	Low: 0
  	Medium: 0
  	High: 0
  Total issues (by confidence):
  	Undefined: 0
  	Low: 0
  	Medium: 0
  	High: 0
Files skipped (0):