FINERACT-2081: Update password policy #4141
Conversation
leksinomi
force-pushed
the
bugfix/FINERACT-2081/strengthen_password_policy
branch
4 times, most recently
from
713748b
to
6b1e967
Compare
leksinomi
changed the title
leksinomi
force-pushed
the
bugfix/FINERACT-2081/strengthen_password_policy
branch
3 times, most recently
from
78083d0
to
c5c77af
Compare
| @@ -32,6 32,8 @@ public class ApiProperties { | |||
| private String username; | |||
| @Value("${fineract-test.api.password}") | |||
| private String password; | |||
| @Value("${fineract-test.api.valid-password}") | |||
There was a problem hiding this comment.
What is "valid password" means? Why do we need this one? why the "password" field is changed to this everywhere?
There was a problem hiding this comment.
E2E tests create users for testing purpose. After I switched password validation policy the "password" value is no longer valid when creating new users or changing the passwords.
| <column name="regex" value="^(?!.*(.)\1)(?!.*\s)(?=.*\d)(?=.*[a-z])(?=.*[A-Z])(?=.*[^\w\s]).{12,50}$"/> | ||
| <column name="description" value="Password must be 12 to 50 characters long, containing at least one uppercase letter, one lowercase letter, one numeric digit, and one special character, with no spaces or consecutive repeating characters"/> | ||
| <column name="active" value="1"/> | ||
| <where>id = 2</where> |
There was a problem hiding this comment.
I think we should not override existing password policy, rather introduce new and set that as active
| @@ -20,6 20,7 @@ | |||
| fineract-test.api.base-url=${BASE_URL:https://localhost:8443} | |||
| fineract-test.api.username=${TEST_USERNAME:mifos} | |||
| fineract-test.api.password=${TEST_PASSWORD:password} | |||
| fineract-test.api.valid-password=${TEST_VALID_PASSWORD:A1b2c3d4e5f$} | |||
There was a problem hiding this comment.
Would you mind to rename it to "strong password"? This valid password is misleading to me..
leksinomi
force-pushed
the
bugfix/FINERACT-2081/strengthen_password_policy
branch
15 times, most recently
from
7ec3929
to
f441779
Compare
|
@adamsaghy I'd like to check this myself. Please do not merge yet. |
| @@ -37,6 37,7 @@ jobs: | |||
| BASE_URL: https://localhost:8443 | |||
| TEST_USERNAME: mifos | |||
| TEST_PASSWORD: password | |||
There was a problem hiding this comment.
What's the reason for keeping the original password here?
There was a problem hiding this comment.
The "mifos/password" credentials have a "Super user" role. They are used for all necessary preparations required for testing.
| <changeSet author="fineract" id="1"> | ||
| <update tableName="m_password_validation_policy"> | ||
| <column name="active" valueBoolean="false"/> | ||
| <where>active=true</where> | ||
| </update> | ||
| </changeSet> |
There was a problem hiding this comment.
This would disable any policy which is set to active. That means if somebody already created a separate password policy for their use, this change could break their functionality.
I'd say only disable the one Fineract is shipped with. Nothing more.
There was a problem hiding this comment.
I will fix that in a next commit, thanks!
leksinomi
force-pushed
the
bugfix/FINERACT-2081/strengthen_password_policy
branch
from
f441779
to
e5520f8
Compare
leksinomi
force-pushed
the
bugfix/FINERACT-2081/strengthen_password_policy
branch
from
e5520f8
to
3728a13
Compare
|
Hi @galovics, I've updated the PR. Please review. |
leksinomi
deleted the
bugfix/FINERACT-2081/strengthen_password_policy
branch
Description
Update password validation to ensure consistency across application layers.
Ignore if these details are present on the associated Apache Fineract JIRA ticket.
Checklist
Please make sure these boxes are checked before submitting your pull request - thanks!
Write the commit message as per https://github.com/apache/fineract/#pull-requests
Acknowledge that we will not review PRs that are not passing the build ("green") - it is your responsibility to get a proposed PR to pass the build, not primarily the project's maintainers.
Create/update unit or integration tests for verifying the changes made.
Follow coding conventions at https://cwiki.apache.org/confluence/display/FINERACT/Coding Conventions.
Add required Swagger annotation and update API documentation at fineract-provider/src/main/resources/static/legacy-docs/apiLive.htm with details of any API changes
Submission is not a "code dump". (Large changes can be made "in repository" via a branch. Ask on the developer mailing list for guidance, if required.)
FYI our guidelines for code reviews are at https://cwiki.apache.org/confluence/display/FINERACT/Code Review Guide.