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

[17.0][MIG] password_security + use ir.config_parameters #731

Open
wants to merge 75 commits into
base: 17.0
Choose a base branch
from

Conversation

alexis-via
Copy link
Contributor

Replaces migration PR #610

Migration to v17 + move config parameters from res.company to ir.config_parameter. That way, a user that has access to several companies cannot switch to the company with weakest password security params before changing its password, which was a kind of security hole.

By the way, the password security param of auth_password_policy was already stored in ir.config_parameter (cf https://github.com/odoo/odoo/blob/17.0/addons/auth_password_policy/models/res_config_settings.py#L8), so it's coherent to do the same here.

lasley and others added 30 commits November 21, 2024 23:37
* [ADD] res_users_password_security: New module
* Create new module to lock down user passwords

* [REF] res_users_password_security: PR Review fixes
* Also add beta pass history rule

* [ADD] res_users_password_security: Pass history and min time
* Add pass history memory and threshold
* Add minimum time for pass resets through web reset

* Begin controller tests

* Fix copyright, wrong year for new file

* Add tests for password_security_home
* Left to do web_auth_reset_password

* Fix minimum reset threshold and finish tests

* Bug fixes per review

* [REF] password_security: PR review improvements
* Change tech name to password_security
* Use new except format
* Limit 1 & new api
* Cascade deletion for pass history

* [REF] password_security: Fix travis + style
* Fix travis errors
* self to cls
* Better variable names in tests

* [FIX] password_security: Fix travis errors
* Bump versions
* Installable to True
* Add Usage section to ReadMe w/ Runbot link
* `_crypt_context` now directly exposes the `CryptContext`
* Change all instances of openerp to odoo
* Add current time as password_write_date for admin user in demo, disabling the reset prompt - fixes OCA#652
* Switch security to be on correct model to fix OCA#674
…ord invalid (#859)

* [FIX] password_security: Fix password stored

* [REF] password_security: use a unified check_password private method to validate rules and history password
* Add logic to overloaded web_login action to log out users with expired
passwords, preventing the password reset from being ignored
* Add unit test for new logic
This translates to Spanish all missing translations, 31 in total.
Since some implementation details are changed, I had to change some tests that were actually testing the implementation instead of the desired result of the method.
In a normal Odoo deployment, somebody in group *Administration / Access Rights* should be able to create users; but if this addon is installed, it gets this error:

    The requested operation cannot be completed due to security restrictions. Please contact your system administrator.

    (Document type: Res Users Password History, Operation: create)

This is now tested and fixed.
[The `website` addon returns an aditional redirection][1] that makes these tests fail if ran after installing `website`.

The tests were checking the returned value in a funky way anyways.

Now, instead of checking the final returned value, we check directly the parameters sent to the redirection method.

[1]: https://github.com/odoo/odoo/blob/3b85900fafc9469dca6e7c01fca6dac4f55d20f5/addons/website/controllers/main.py#L85-L89
Avoided requiring the module twice in JS.
Currently translated at 57.9% (22 of 38 strings)

Translation: server-auth-12.0/server-auth-12.0-password_security
Translate-URL: https://translation.odoo-community.org/projects/server-auth-12-0/server-auth-12-0-password_security/hr/
dsolanki-initos and others added 24 commits November 21, 2024 23:37
Updated by "Update PO files to match POT (msgmerge)" hook in Weblate.

Translation: server-auth-16.0/server-auth-16.0-password_security
Translate-URL: https://translation.odoo-community.org/projects/server-auth-16-0/server-auth-16-0-password_security/
Currently translated at 24.0% (12 of 50 strings)

Translation: server-auth-16.0/server-auth-16.0-password_security
Translate-URL: https://translation.odoo-community.org/projects/server-auth-16-0/server-auth-16-0-password_security/it/
Currently translated at 24.0% (12 of 50 strings)

Translation: server-auth-16.0/server-auth-16.0-password_security
Translate-URL: https://translation.odoo-community.org/projects/server-auth-16-0/server-auth-16-0-password_security/it/
Currently translated at 100.0% (50 of 50 strings)

Translation: server-auth-16.0/server-auth-16.0-password_security
Translate-URL: https://translation.odoo-community.org/projects/server-auth-16-0/server-auth-16-0-password_security/es/
Currently translated at 100.0% (50 of 50 strings)

Translation: server-auth-16.0/server-auth-16.0-password_security
Translate-URL: https://translation.odoo-community.org/projects/server-auth-16-0/server-auth-16-0-password_security/it/
Replace fields on res.company by ir.config_parameter
Remove dead test for v16 migration script
Copy link

@amh-mw amh-mw left a comment

Choose a reason for hiding this comment

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

Though I agree with the intent of the improvement, this is the second migration pull request in a row for password_security where improvements are being done in the migration pull request. I think it will be more coherent to migrate between versions as usual and then review the improvement in its own pull request.

@alexis-via
Copy link
Contributor Author

The move to ir.config_parameter is in a separate commit.

@etobella
Copy link
Member

@amh-mw the usual flow in this kind of changes is as @alexis-via suggested (at least is what I found in a lot of cases in the OCA)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.