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

[FIX] 12.0 password_security module #85

Closed
wants to merge 1 commit into from
Closed

[FIX] 12.0 password_security module #85

wants to merge 1 commit into from

Conversation

shepilov-vladislav
Copy link
Contributor

@shepilov-vladislav shepilov-vladislav commented Apr 15, 2019

frontend meter/badgets/popup when using password_security module on signup/reset-password fronted pages.

Problems was solved by this PR:

  1. Meter on frontend pages did works incorrectly.
  2. Original meter from auth_password_policy_signup conflicts with meter from password_security. I did disable meter from auth_password_policy_signup.
  3. Recommendations didn't worked correctly.

It's regression after migration to 12.0 and adding new features.

@shepilov-vladislav
Copy link
Contributor Author

Guys who has experience with testing qcontext? Maybe anyone provide me a link with example?

@pedrobaeza
Copy link
Member

I'm afraid not

@shepilov-vladislav
Copy link
Contributor Author

I'm afraid not

So, and how I can increase test coverage? Maybe it will be merged?

@pedrobaeza
Copy link
Member

Yes, codecov is not mandatory, so this has chances to be merged when reviewed.

@BT-ojossen
Copy link

Hi, what's the state here?

@shepilov-vladislav
Copy link
Contributor Author

Hi, what's the state here?

I found some more strange moments of behavior of this module.

  1. password_minimum options doesn't affect on user' ability of password changing via reset password link

  2. password_history value works with errors - old password quantity which are checked is -1 (if I set 1 - the system allows to set any password including the current, if I set 2 - the system doesn't allow to set only the current password)

  3. password_length and all others are displayed as 1 in hints if 0 value was saved
    pass11

  4. Reset password token expires at once after attempt to use old password (if History > 1)

  5. Password Policy rules are checked firstly at password changing via user preferences. This is a slight vulnerability

Tell me, is it worth fixing them? Or I do not understand how this should work?

@shepilov-vladislav
Copy link
Contributor Author

@pedrobaeza Please suggest.

@pedrobaeza
Copy link
Member

Well, the problems you have found seems legit, so go ahead if you want to fix them, but if not, simply document them in known issues section

password_security/models/res_users.py Outdated Show resolved Hide resolved
password_security/models/res_users.py Outdated Show resolved Hide resolved
@shepilov-vladislav
Copy link
Contributor Author

So old

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.

5 participants