[IMP] Allow the administator to forbid passwords that contain the login.

[daramousk]

@NL66278

[NL66278]

👍 Tested and LGTM

[pedrobaeza]

Why Travis is red?

[NL66278]

It seems there are now some tests that fail that assume login's could be the same. Ais is true of course by default for admin and demo in demo. Probably some additional code is needed that suspends these checks for the relevant users, but of course only when testing (not to re-introduce the security hole, this PR intents to stop).

[yajo]

You could add a demo data file that disables that check in demo envs.

[NL66278]

@daramousk I think the most easy thing to do is either:

1. Make False the default for the option to check passwords, and enable it for the test
2. Add demo data (I think that was meant by @yajo) to set the option to False for the main company. (And then enable it in your test method). That seems actually the most "clean" method.

[yajo]

Yes, I meant that. I think option 2 is better because True seems saner as a default value.

[daramousk]

@NL66278 Ok I tried figuring out whether this module breaks the tests on auth_brute_force. I installed and run the auth_brute_force in an empty database and that test still fails. Am I missing something here?

[yajo]

Odoo broke that test, and possibly lots of others, with odoo/odoo#30768. I'm investigating its fix.

[NL66278]

@daramousk the module auth_brute_force tests with passwords that contain the login. That should be rectified to be fully compatible with password_security. I had made a fix that I wanted to submit as a PR on your branch, but unfortunately I can not make PR's on your repo anymore. See patch.

diff --git a/auth_brute_force/tests/test_brute_force.py b/auth_brute_force/tests/test_brute_force.py
index 2e21b53..a3deb15 100644
--- a/auth_brute_force/tests/test_brute_force.py
+++ b/auth_brute_force/tests/test_brute_force.py
@@ -66,10 +66,10 @@ class BruteForceCase(HttpCase):
         except AttributeError:
             pass
         # Complex password to avoid conflicts with `password_security`
-        self.good_password = "Admin$%02584"
+        self.good_password = "A_dmin$%02584"
         self.data_demo = {
             "login": "demo",
-            "password": "Demo%&/(908409**",
+            "password": "D_emo%&/(908409**",
         }
         with self.cursor() as cr:
             env = self.env(cr)

[daramousk]

@hbrunn Maybe I could get a review here so that this can be merged as well.

[OCA-git-bot]

This PR has the approved label and has been created more than 5 days ago. It should therefore be ready to merge by a maintainer (or a PSC member if the concerned addon has no declared maintainer). 🤖

[yajo]

I think that the fix is wrong. Instead of changing all cls for self, you should change setUp for setUpClass.

This benefits from SavepointCase and speeds up tests.

Another option would be to switch for a TransactionCase instead.

[sebalix]

Suggested change

	<field name="password_no_login">False</field>
	<field name="password_no_login" eval="False"/>

[daramousk]

@NL66278 Yes indeed, I have reverted that

[daramousk]

@hbrunn Ping for this one that has not been merged yet.

[daramousk]

hello @pedrobaeza ,
Perhaps this can be merged?

[pedrobaeza]

Please check comment from @yajo

[github-actions]

There hasn't been any activity on this pull request in the past 4 months, so it has been marked as stale and it will be closed automatically if no further activity occurs in the next 30 days.
If you want this PR to never become stale, please ask a PSC member to apply the "no stale" label.