[12.0][MIG] password_security

[pnajman-modoolar]

Hi guys,

I've migrated this module from 11.0 to 12.0 and I've integrated it with the new Odoo modules auth_password_policy and auth_password_policy_signup.

Also Password Policy configuration section is moved from company level to the Configuration -> General Settings.

Cheers,
Petar

[pedrobaeza]

Hi @pnajman-modoolar, do you think this one is still needed with new improvements in core?

Any way, you should preserve commit history following https://github.com/OCA/maintainer-tools/wiki/Migration-to-version-12.0#technical-method-to-migrate-a-module-from-110-to-120-branch, and doing a migration normally doesn't grant you co-authorship of the module, but it depends on the needed changes.

[pnajman-modoolar]

Hi @pedrobaeza ,

IMHO, yes, i think it's needed because in the new core you can only control password length and nothing else. You can not control password expiry and password history is not kept as well. So, yes, I think it's needed.

Thanks for pointing out migration document, I wasn't aware of it.

Regarding this migration, I didn't simply migrate this module I have also integrated it with the new core modules auth_password_policy and auth_password_policy_signup and due to that integration i've added myself as an co-author of this 12.0 module. If this is not enough I'll kindly change it back.

[pedrobaeza]

OK, then please respect commit history and I will review it.

[pnajman-modoolar]

I'll do that, thanks.

[ddufresne]

@pnajman-modoolar thank you for migrating this module. Tested with a local odoo instance. Works as expected.

Do you plan on restablishing the commit history as asked by @pedrobaeza ?

Thanks

[pedrobaeza]

It's a requirement for merging it, as we should preserve the attribution of things.

[pnajman-modoolar]

hi guys, yes, i know that's a requirement I just didn't had much time lately. I'll do it properly in the upcoming days. Could you point me in the right direction, do I create another proper pull request or ...?

[pedrobaeza]

What you can do is:

• Save current files with your migrated module in any place.
• Delete the branch locally.
• Create a new branch from current 12.0 with the same name.
• Follow the technical steps in https://github.com/OCA/maintainer-tools/wiki/Migration-to-version-12.0.
• Copy the migrated module in the directory once done, and commit for having only the diff to get this to v12.
• Push with -f for forcing the overwrite to you remote, and it will be reflected here in the same PR.

[pedrobaeza]

Check also why Travis is red before pushing for having all done. Let me know if any other doubt.

[pedrobaeza]

Any news about this?

[shepilov-vladislav]

@pnajman-modoolar test test_web_login_expire_pass was failing and also you didn't convert fields.Date/Datetime.from_string to fields.Date.to_date/to_datetime (https://github.com/OCA/maintainer-tools/wiki/Migration-to-version-12.0)

correct test placed here:

    def test_web_login_expire_pass(self, redirect_mock, *args):
        """It should expire password if necessary"""
        two_days_ago = datetime.now() - timedelta(days=2)
        with self.cursor() as cr:
            env = self.env(cr)
            user = env['res.users'].search([('login', '=', 'admin')])
            user.password_write_date = two_days_ago
            user.company_id.password_expiration = 1
        response = self.url_open(
            "/web/login",
            {"login": "admin", "password": "admin"}
        )
        # Password has expired, I'm redirected to reset it
        all_urls = [call[0][0] for call in redirect_mock.call_args_list
                    if isinstance(call[0][0], str)]
        self.assertTrue(all_urls)
        start = response.url.replace("/login", "/reset_password?").replace("127.0.0.1:8069", "localhost:80")
        self.assertTrue(any(url.startswith(start) for url in all_urls))
        self.assertEqual(response.text, "redirected")

[shepilov-vladislav]

@pedrobaeza can I create new one pull request and complete this work?

[pedrobaeza]

Let's wait at least a couple of days to see if @pnajman-modoolar answers.

[shepilov-vladislav]

@pedrobaeza Seems like @pnajman-modoolar doesn't answer. So can I fix this migration with new pull request?

[pedrobaeza]

OK, please go ahead.

[pedrobaeza]

Replaced by #83