Updating password rules #2842

muralibasani · 2025-03-17T13:41:39Z

Linked issue

Resolves: #xxxxx

What kind of change does this PR introduce?

What is the current behavior?

Describe the state of the application before this PR. Illustrations appreciated (videos, gifs, screenshots).

What is the new behavior?

Describe the state of the application after this PR. Illustrations appreciated (videos, gifs, screenshots).

Other information

Additional changes, explanations of the approach taken, unresolved issues, necessary follow ups, etc.

Requirements (all must be checked before review)

The pull request title follows our guidelines
Tests for the changes have been added (if relevant)
The latest changes from the main branch have been pulled
pnpm lint has been run successfully

core/src/main/resources/templates/addUser.html

coral/src/app/features/user-information/change-password/changePasswordFormSchema.ts

coral/types/api.d.ts

core/src/main/java/io/aiven/klaw/model/requests/ChangePasswordRequestModel.java

core/src/main/resources/static/js/manageUsers.js

core/src/main/resources/templates/addUser.html

core/src/test/java/io/aiven/klaw/EnvsClustersTenantsControllerIT.java

core/src/test/java/io/aiven/klaw/controller/UiConfigControllerTest.java

core/src/test/java/io/aiven/klaw/service/UsersTeamsControllerServiceTest.java

Signed-off-by: Muralidhar Basani <[email protected]>

openapi.yaml

Signed-off-by: Muralidhar Basani <[email protected]>

aindriu-aiven

A few comments for consideration.

aindriu-aiven · 2025-03-19T12:44:06Z

docker-scripts/docker-compose-klaw.yaml

@@ -8,7 +8,7 @@ services:
      SPRING_DATASOURCE_URL: "jdbc:h2:file:/klaw/klawprodb;DB_CLOSE_ON_EXIT=FALSE;DB_CLOSE_DELAY=-1;MODE=MySQL;CASE_INSENSITIVE_IDENTIFIERS=TRUE;"
      KLAW_UIAPI_SERVERS: "http://klaw-core:9097"
      KLAW_CLUSTERAPI_URL: "http://klaw-cluster-api:9343"
-      KLAW_SUPERADMIN_DEFAULT_PASSWORD: 'welcometoklaw'
+      KLAW_SUPERADMIN_DEFAULT_PASSWORD: 'WelcomeToKlaw321@'


We will need to make an update to klaw docs to let folks know we have updated this.

Just so we can merge one and then immediately merge the other

aindriu-aiven · 2025-03-19T12:46:46Z

core/src/test/java/io/aiven/klaw/service/UsersTeamsControllerServiceTest.java

+    violations.forEach(
+        vio ->
+            assertThat(vio.getMessage())
+                .contains("Password must be at least 8 characters long and include at least"));


Should just check this and line 909 against the constant PASSWORD_REGEX_VALIDATION_STR

aindriu-aiven · 2025-03-19T12:48:48Z

core/src/test/java/io/aiven/klaw/service/UsersTeamsControllerServiceTest.java

@@ -889,6 +897,31 @@ public void changePwd() throws KlawException {
    changePwdEncodedParams(updatePwdUserDetails, changePwdRequestModel);
  }

+  @Test


You could also @ParamterizedTest this and add a CSVSource with "password,violationsCount" and fire in a number of passwords, it should simplify the test and show xactly what passwords are and are not allowed.

aindriu-aiven · 2025-03-19T12:50:24Z

core/src/test/java/io/aiven/klaw/model/UserInfoModelTest.java

+    violations.forEach(
+        vio ->
+            assertThat(vio.getMessage())
+                .contains("Password must be at least 8 characters long and include at least"));


Can check against the constant here as well.

aindriu-aiven · 2025-03-19T13:05:56Z

core/src/main/java/io/aiven/klaw/model/requests/ChangePasswordRequestModel.java

  @NotNull
+  @Pattern(regexp = PASSWORD_REGEX, message = PASSWORD_REGEX_VALIDATION_STR)


It might be worth splitting the @pattern into 3 or 4 things to give concise feedback about the issue with the password wdyt?

e.g.
@pattern(regex = "(?=.[a-z])" message = "must contain at least 1 lower case letter")
@pattern(regex = "(?=.[A-Z])" message = "must contain at least 1 upper case letter")
@pattern(regex = "(?=.[0-9])" message = "must contain at least 1 number")
@pattern(regex = "(?=.["!@#$%&*()'+,-./:;<=>?[]^_`{|}"])" message = "must contain at least 1 special character")
@SiZe(min = 8)

Note have not tested above regex

We have the same pattern check in front end.
This pattern is invoked only when someone is invoking BE apis manually, and we have not enabled them yet.

aindriu-aiven · 2025-03-19T13:07:52Z

core/src/main/resources/static/js/manageUsers.js

@@ -336,7 +342,7 @@ app.controller("manageUsersCtrl", function($scope, $http, $location, $window) {

            if(!$scope.addNewUser.pwd)
            {
-                $scope.alertnote = "Please enter a password.";
+                $scope.alertnote = "Please enter a suggested password.";


i dont think you need to add suggested here, as they should be entering their password?

aindriu-aiven · 2025-03-19T13:08:23Z

core/src/main/resources/static/js/registerUsers.js

@@ -389,7 +389,7 @@ app.controller("registerUsersCtrl", function($scope, $http, $location, $window)

          if(!$scope.registerUser.pwd)
          {
-              $scope.alertnote = "Please enter a password.";
+              $scope.alertnote = "Please enter a suggested password.";


same I dont think you need to change it to suggested here

core/src/main/resources/application.properties

Signed-off-by: Muralidhar Basani <[email protected]>

muralibasani · 2025-03-20T08:09:47Z

@aindriu-aiven can you take a look again ?

aindriu-aiven

Tested changing password through old and new UI and tested creating a new user, having admin reset password as well.

The one area I fond that wasnt covered by the improved password policy was the password reset page.
Here any password can be added without validation.

UserTeamsControllerService and i think forgotPassword.js need to be updated

aindriu-aiven · 2025-03-20T08:26:59Z

Tested changing password through old and new UI and tested creating a new user, having admin reset password as well.

The one area I fond that wasnt covered by the improved password policy was the password reset page. Here any password can be added without validation.

UserTeamsControllerService and i think forgotPassword.js need to be updated

@muralibasani you can just make the change on the API to check it, the error should be propagated back to the UI.

muralibasani · 2025-03-20T10:03:13Z

Tested changing password through old and new UI and tested creating a new user, having admin reset password as well.

The one area I fond that wasnt covered by the improved password policy was the password reset page. Here any password can be added without validation.

UserTeamsControllerService and i think forgotPassword.js need to be updated

Right, updated and added a test.

aindriu-aiven

LGTM thank you

muralibasani requested a review from programmiri March 17, 2025 13:41

muralibasani marked this pull request as ready for review March 17, 2025 13:55

muralibasani requested a review from nosahama March 17, 2025 14:01

ArtturiSipila reviewed Mar 17, 2025

View reviewed changes

core/src/main/resources/templates/addUser.html Outdated Show resolved Hide resolved

muralibasani mentioned this pull request Mar 18, 2025

Delete default superadmin pwd #2834

Merged

9 tasks

muralibasani force-pushed the pwd-policy branch from 5185f83 to e1854b7 Compare March 18, 2025 09:14

nosahama reviewed Mar 18, 2025

View reviewed changes

coral/src/app/features/user-information/change-password/changePasswordFormSchema.ts Show resolved Hide resolved

nosahama reviewed Mar 18, 2025

View reviewed changes

coral/types/api.d.ts Outdated Show resolved Hide resolved

nosahama reviewed Mar 18, 2025

View reviewed changes

coral/types/api.d.ts Outdated Show resolved Hide resolved

nosahama reviewed Mar 18, 2025

View reviewed changes

core/src/main/java/io/aiven/klaw/model/requests/ChangePasswordRequestModel.java Outdated Show resolved Hide resolved

nosahama reviewed Mar 18, 2025

View reviewed changes

core/src/main/resources/static/js/manageUsers.js Show resolved Hide resolved

nosahama reviewed Mar 18, 2025

View reviewed changes

core/src/main/resources/templates/addUser.html Outdated Show resolved Hide resolved

nosahama reviewed Mar 18, 2025

View reviewed changes

core/src/test/java/io/aiven/klaw/EnvsClustersTenantsControllerIT.java Show resolved Hide resolved

nosahama reviewed Mar 18, 2025

View reviewed changes

core/src/test/java/io/aiven/klaw/controller/UiConfigControllerTest.java Show resolved Hide resolved

nosahama reviewed Mar 18, 2025

View reviewed changes

core/src/test/java/io/aiven/klaw/service/UsersTeamsControllerServiceTest.java Outdated Show resolved Hide resolved

nosahama previously approved these changes Mar 18, 2025

View reviewed changes

muralibasani and others added 4 commits March 18, 2025 16:11

Updating password rules

6265445

Updating password rules

8e48ec7

Updating password regex

47b11d5

Signed-off-by: Muralidhar Basani <[email protected]>

🤖 Auto-update API types for TypeScript usage

5ff877a

muralibasani force-pushed the pwd-policy branch from e1854b7 to 5ff877a Compare March 18, 2025 15:11

Update null checks

f3cf90f

muralibasani dismissed nosahama’s stale review via f3cf90f March 18, 2025 15:36

nosahama reviewed Mar 18, 2025

View reviewed changes

openapi.yaml Show resolved Hide resolved

Fix password in Unit test

92880e9

Signed-off-by: Muralidhar Basani <[email protected]>

aindriu-aiven reviewed Mar 19, 2025

View reviewed changes

core/src/main/resources/application.properties Outdated Show resolved Hide resolved

From review fix

edc74c1

Signed-off-by: Muralidhar Basani <[email protected]>

muralibasani requested review from nosahama and aindriu-aiven March 19, 2025 14:22

nosahama previously approved these changes Mar 19, 2025

View reviewed changes

aindriu-aiven requested changes Mar 20, 2025

View reviewed changes

Update forgotpwd with pattern checks

5c5c9f0

muralibasani dismissed nosahama’s stale review via 5c5c9f0 March 20, 2025 09:58

Update forgotpwd with pattern checks

e149059

aindriu-aiven approved these changes Mar 20, 2025

View reviewed changes

muralibasani merged commit ba9baaa into main Mar 20, 2025
30 checks passed

muralibasani deleted the pwd-policy branch March 20, 2025 11:33

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Updating password rules #2842

Updating password rules #2842

muralibasani commented Mar 17, 2025

aindriu-aiven left a comment

aindriu-aiven Mar 19, 2025

aindriu-aiven Mar 19, 2025

aindriu-aiven Mar 19, 2025

aindriu-aiven Mar 19, 2025

aindriu-aiven Mar 19, 2025

aindriu-aiven Mar 19, 2025

muralibasani Mar 19, 2025

aindriu-aiven Mar 19, 2025

aindriu-aiven Mar 19, 2025

muralibasani commented Mar 20, 2025

aindriu-aiven left a comment

aindriu-aiven commented Mar 20, 2025

muralibasani commented Mar 20, 2025

aindriu-aiven left a comment

		@NotNull
		@Pattern(regexp = PASSWORD_REGEX, message = PASSWORD_REGEX_VALIDATION_STR)

Updating password rules #2842

Updating password rules #2842

Conversation

muralibasani commented Mar 17, 2025

Linked issue

What kind of change does this PR introduce?

What is the current behavior?

What is the new behavior?

Other information

Requirements (all must be checked before review)

aindriu-aiven left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

muralibasani commented Mar 20, 2025

aindriu-aiven left a comment

Choose a reason for hiding this comment

aindriu-aiven commented Mar 20, 2025

muralibasani commented Mar 20, 2025

aindriu-aiven left a comment

Choose a reason for hiding this comment