[12.0][MIG] password_security by shepilov-vladislav · Pull Request #83 · OCA/server-auth

shepilov-vladislav · 2019-03-31T19:19:53Z

Continue #66

pedrobaeza · 2019-03-31T20:09:29Z

@shepilov-vladislav you have to preserve commit history as told before, and Travis is red in this PR. Please amend both things.

pedrobaeza · 2019-03-31T22:54:07Z

Full commit history is brought this way: https://github.com/OCA/maintainer-tools/wiki/Migration-to-version-12.0#technical-method-to-migrate-a-module-from-110-to-120-branch

shepilov-vladislav · 2019-04-01T18:36:29Z

@pedrobaeza I have a problem with one test. How I can setup environment like on travis? With my environment all works fine.

yajo

I'm reviewing the big diff introduced by this patch and it all seems to come from basically aesthetic styling.

We have a linter to do that job. Please do not "fix" any line that does not fail in a linter (there's a LINT job in travis that you can check).

I commented on 22 of them, but got tired after. Maybe you have a autostyler installed? Please undo all those "fixes" to review again.

Thanks!

shepilov-vladislav · 2019-04-03T07:43:03Z

I'm reviewing the big diff introduced by this patch and it all seems to come from basically aesthetic styling.

We have a linter to do that job. Please do not "fix" any line that does not fail in a linter (there's a LINT job in travis that you can check).

I commented on 22 of them, but got tired after. Maybe you have a autostyler installed? Please undo all those "fixes" to review again.

Thanks!

@yajo I corrected everything you asked for.

yajo

Some minor changes. Mostly code is OK. Thanks!

pedrobaeza · 2019-04-05T12:58:27Z

Travis is red

pedrobaeza

Can you use new README by fragments system?

Also version should be 12.0.1.0.0.

And finally, please stay with only one migration commit.

ernestotejeda

Some change requests

shepilov-vladislav · 2019-04-10T00:15:15Z

Guys, can you help me. I want to fix my one failed test. But I can not reproduce it. Could you give me instructions on how to raise the same environmental locally for tests(like in travis)? Maybe OCA has docker images for it?

pedrobaeza · 2019-04-12T08:26:49Z

Do you mean it's not reproducible in local?

shepilov-vladislav · 2019-04-12T10:17:31Z

Do you mean it's not reproducible in local?

@pedrobaeza Exactly. How I can reproduce environment like used on travis?

pedrobaeza · 2019-04-12T10:19:20Z

You can check the log for seeing how Travis is installing first "base" modules, and then installing the rest with --test-enable flag. You can produce the same using that 2 commands.

shepilov-vladislav · 2019-04-12T10:56:37Z

You can check the log for seeing how Travis is installing first "base" modules, and then installing the rest with --test-enable flag. You can produce the same using that 2 commands.

I understand what you mean, this was my fallback. I thought that OCA has a special docker image for development needs. And I can use it quickly.

pedrobaeza · 2019-04-12T10:58:39Z

No, it uses https://github.com/OCA/maintainer-quality-tools/tree/master/travis. When they were done, there weren't support in Travis for Docker.

* [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 #652

* Switch security to be on correct model to fix #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

shepilov-vladislav · 2019-04-12T21:13:49Z

No, it uses https://github.com/OCA/maintainer-quality-tools/tree/master/travis. When they were done, there weren't support in Travis for Docker.

@pedrobaeza Please put you attention. It is very interesting why the test fail in past.

Explanation:

in test_web_login_expire_pass:

two_days_ago = datetime.now() - timedelta(days=2)
...
user.password_write_date = two_days_ago

in _password_has_expired function:

days = (fields.Datetime.now() - self.password_write_date).days

and after that days == (1 day, 23:59:59.824865).days == 1

I preferred to simply increase the interval to three days in the test. What do you think about that?

Also I added all changes requested by you. So can we merge it now?

pedrobaeza · 2019-04-12T21:30:03Z

OK, if that avoids that edge cases. I remember at least 2 cases in the past with something similar: a test that failed on leap years, and another that fails at midnight when timezone changes the day...

pedrobaeza · 2019-04-12T21:32:16Z

Thanks for the spent time!

shepilov-vladislav · 2019-04-13T06:17:51Z

Thanks for the spent time!

Thank you too for help!

sbidoul · 2019-05-11T11:12:36Z

@shepilov-vladislav FYI I fixed the README at e147125. They must be simple text fragments, the rest is generated by the bot which assembles the fragments and add boilerplate text.

moylop260

👍

shepilov-vladislav · 2019-05-13T10:34:03Z

@shepilov-vladislav FYI I fixed the README at e147125. They must be simple text fragments, the rest is generated by the bot which assembles the fragments and add boilerplate text.

👍

Syncing from upstream OCA/server-auth (11.0)

pedrobaeza added this to the 12.0 milestone Mar 31, 2019

yajo requested changes Apr 2, 2019

View reviewed changes

yajo requested changes Apr 3, 2019

View reviewed changes

yajo approved these changes Apr 3, 2019

View reviewed changes

pedrobaeza mentioned this pull request Apr 5, 2019

[12.0][MIG] password_security #66

Closed

pedrobaeza requested changes Apr 5, 2019

View reviewed changes

ernestotejeda suggested changes Apr 5, 2019

View reviewed changes

Comment thread password_security/models/res_users.py Outdated

Comment thread password_security/models/res_users.py Outdated

lasley and others added 12 commits April 13, 2019 00:01

[MIG] Make modules uninstallable

09946c7

[MIG] Rename manifest files

f9f081d

[MIG] password_security: Migrate to v10

4d6ad3f

* 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

[FIX] password_security: Validate admin pass

2bc3a71

* Add current time as password_write_date for admin user in demo, disabling the reset prompt - fixes #652

[FIX] password_security: Fix history rule

6b4a0a9

* Switch security to be on correct model to fix #674

Improve UX password_match_message()

5b60c15

[FIX] password_security: Fix password stored and token to reset passw…

86aaf46

…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

[FIX] password_security: Force password reset

f605642

* 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

[MIG]password security

95c9cf8

[IMP]change request

b2a60ef

[IMP]Underscore is a special character #1077

393764c

lasley and others added 9 commits April 13, 2019 00:01

[FIX] password_security: Default last write date #1084

09e97ef

[FIX] UTF-8 coding is not necessary

b3032d5

[MIG] auth_totp_password_security: Migration to 11.0

2958135

[I18N] password_security: Translate to Spanish all translatable terms

82e4371

This translates to Spanish all missing translations, 31 in total.

[FIX] password_security: Error 500 when login with bad password (#27)

93f078a

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.

[UPD] Update password_security.pot

fd82a92

[MIG] password_security: Migration to 12.0

d683bf7

pedrobaeza approved these changes Apr 12, 2019

View reviewed changes

pedrobaeza merged commit 26cc605 into OCA:12.0 Apr 12, 2019

OCA-git-bot mentioned this pull request Apr 12, 2019

Migration to version 12.0 #38

Closed

19 tasks

moylop260 reviewed May 11, 2019

View reviewed changes

SiesslPhillip pushed a commit to grueneerde/OCA-server-auth that referenced this pull request Nov 20, 2024

Merge pull request OCA#83 from OCA/11.0

6cb8c08

Syncing from upstream OCA/server-auth (11.0)

Uh oh!

Conversation

shepilov-vladislav commented Mar 31, 2019

Uh oh!

pedrobaeza commented Mar 31, 2019

Uh oh!

pedrobaeza commented Mar 31, 2019

Uh oh!

shepilov-vladislav commented Apr 1, 2019

Uh oh!

yajo left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

shepilov-vladislav commented Apr 3, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

yajo left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

pedrobaeza commented Apr 5, 2019

Uh oh!

pedrobaeza left a comment

Choose a reason for hiding this comment

Uh oh!

ernestotejeda left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

shepilov-vladislav commented Apr 10, 2019

Uh oh!

pedrobaeza commented Apr 12, 2019

Uh oh!

shepilov-vladislav commented Apr 12, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

pedrobaeza commented Apr 12, 2019

Uh oh!

shepilov-vladislav commented Apr 12, 2019

Uh oh!

pedrobaeza commented Apr 12, 2019

Uh oh!

shepilov-vladislav commented Apr 12, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

pedrobaeza commented Apr 12, 2019

Uh oh!

pedrobaeza commented Apr 12, 2019

Uh oh!

shepilov-vladislav commented Apr 13, 2019

Uh oh!

sbidoul commented May 11, 2019

Uh oh!

moylop260 left a comment

Choose a reason for hiding this comment

Uh oh!

shepilov-vladislav commented May 13, 2019

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

shepilov-vladislav commented Apr 3, 2019 •

edited

Loading

shepilov-vladislav commented Apr 12, 2019 •

edited

Loading

shepilov-vladislav commented Apr 12, 2019 •

edited

Loading