wizard: add user information step (HMS-4903) #2551

mgold1234 · 2024-11-03T20:20:09Z

wizard: add user information step (HMS-4903)
This commit introduces the user information step with the following fields:
() userName field with validation according to Red Hat guidelines: https://access.redhat.com/solutions/30164
() password field with validation using the pam_pwquality module, which enforces strict password policies for Linux systems.
This module enhances security by enforcing minimum length, complexity, and avoiding weak passwords.
More info: https://access.redhat.com/solutions/6979714
Validation is configured based on the requirements defined for our systems:
https://github.com/osbuild/osbuild-composer/tree/main/test/data/manifests
https://github.com/osbuild/images/pkg/distro/rhel/rhel10
() confirm password field that check if confirm password equal to password
() ssh key field - implement the same validation as we have in edge.
[x] [wip> unit tests- in progress because I want to make sure that all new fields are in a good shape.

avitova · 2024-11-21T15:45:55Z

There is a bug now - if you fill in the user information, click next, visit the Users step again, and change some of the fields, all of the contents of the other fields get deleted.

Screencast.From.2024-11-21.17-44-57.mp4

Other than that, it looks good, I think. :)

avitova · 2024-11-26T09:54:09Z

There is a bug now - if you fill in the user information, click next, visit the Users step again, and change some of the fields, all of the contents of the other fields get deleted.

Screencast.From.2024-11-21.17-44-57.mp4
Other than that, it looks good, I think. :)

This still happens in case that I:

Fill in the user info.
Go to the next steps.
Return back, and try to change any of the fields.

regexowl

Added some questions, the step is looking really good ✨

src/test/fixtures/editMode.ts

regexowl · 2024-11-26T10:02:12Z

src/Components/CreateImageWizard/steps/Users/component/UserInfo.tsx

+        <FormHelperText>
+          <HelperText>
+            <HelperTextItem>
+              Can contain at least 3 different character classes (uppercase


I guess we require three different character classes, I'd change the wording a bit.

Suggested change

Can contain at least 3 different character classes (uppercase

Must contain at least 3 different character classes (uppercase

regexowl · 2024-11-26T10:05:54Z

src/Components/CreateImageWizard/steps/Users/component/UserInfo.tsx

+    if (additionalLogic) {
+      additionalLogic(updatedUser);
+    }
+    // Determine if user exists by index, not name


Why not name? Shouldn't the user name be unique? 🤔

I think keeping track of the user via an id makes sense to me.

Imagine an edge case where you have a user called user. Then you want to add a second user, user2. As you are typing in user2, there will be a brief moment in time where you have two users with the same name (user).

We will still want to enforce uniqueness in the validation, of course, but using an id to keep track of the users would help us avoid problems like the one above.

regexowl · 2024-11-26T10:07:53Z

src/Components/CreateImageWizard/steps/Users/component/Empty.tsx

  return (
    <EmptyState variant={EmptyStateVariant.lg}>
      <EmptyStateHeader
        icon={<EmptyStateIcon icon={UserIcon} />}
        headingLevel="h4"
      />
      <EmptyStateFooter>
-        <Button variant="secondary" onClick={() => {}}>
+        <Button variant="secondary" onClick={onAddUserClick}>


Switching between the empty and user creation states could be handled in a similar way like FSC I think, we could introduce a helper into the users structure called userMode or smth like that and then switch what renders on the step based on a store value.

But this might also be a follow up.

regexowl · 2024-11-26T10:10:16Z

src/store/wizardSlice.ts

+  userState: {
+    users: UserWithAdditionalInfo[];
+    userInfo: UserWithAdditionalInfo;
+  };


What blocks us from doing just:

Suggested change

userState: {

users: UserWithAdditionalInfo[];

userInfo: UserWithAdditionalInfo;

};

users: UserWithAdditionalInfo[];

and accessing the users by their name value?

This way we have a complex structure that includes an array and then just one item of the array that is stored outside of it

it becomes very complicated to implement the change of every field. and like in this way, the verifaition of every filed stay simple

mgold1234 · 2024-11-26T10:20:39Z

@avitova very strange, it is not happend to me, are you looking into the last commit?

Screen.Recording.2024-11-26.at.12.16.32.mov

avitova · 2024-11-26T10:39:38Z

@mgold1234 Yes, your are right, then dismiss my comment. You fixed it, I accidentally tested different version.

This commit introduces the user information step with the following fields: (*) `userName` field with validation according to Red Hat guidelines: https://access.redhat.com/solutions/30164 (*) `password` field with validation using the pam_pwquality module, which enforces strict password policies for Linux systems. This module enhances security by enforcing minimum length, complexity, and avoiding weak passwords. More info: https://access.redhat.com/solutions/6979714 Validation is configured based on the requirements defined for our systems: https://github.com/osbuild/osbuild-composer/tree/main/test/data/manifests https://github.com/osbuild/images/pkg/distro/rhel/rhel10 (*) `confirm password` field that check if confirm password equal to password (*) `ssh key` field - implement the same validation as we have in edge. [x] [wip> unit tests- in progress because I want to make sure that all new fields are in a good shape.

…k with users[] object

lucasgarfield · 2024-11-25T17:28:54Z

src/Components/CreateImageWizard/CreateImageWizard.tsx

+                      !(
+                        userName === '' &&
+                        userPassword === '' &&
+                        userSshKey === ''


I think this should be either password OR SSH key.

lucasgarfield · 2024-11-26T16:33:25Z

src/Components/CreateImageWizard/steps/Users/component/UserInfo.tsx

+    if (additionalLogic) {
+      additionalLogic(updatedUser);
+    }
+    // Determine if user exists by index, not name


I think keeping track of the user via an id makes sense to me.

Imagine an edge case where you have a user called user. Then you want to add a second user, user2. As you are typing in user2, there will be a brief moment in time where you have two users with the same name (user).

We will still want to enforce uniqueness in the validation, of course, but using an id to keep track of the users would help us avoid problems like the one above.

mgold1234 marked this pull request as draft November 3, 2024 20:20

mgold1234 force-pushed the user_info branch from 0ee5742 to e30b32d Compare November 4, 2024 10:32

lucasgarfield changed the title ~~<WIP> User info~~ <WIP> User info (HMS-4903) Nov 4, 2024

mgold1234 force-pushed the user_info branch 27 times, most recently from 0d89bee to fe5208c Compare November 12, 2024 10:30

mgold1234 force-pushed the user_info branch 7 times, most recently from 9b3fe35 to ef687e3 Compare November 25, 2024 09:03

mgold1234 requested review from regexowl, avitova and lucasgarfield November 25, 2024 09:03

mgold1234 force-pushed the user_info branch 7 times, most recently from dd38a0c to 2654a27 Compare November 25, 2024 16:17

lucasgarfield force-pushed the user_info branch from 2654a27 to 3383ce0 Compare November 25, 2024 17:20

mgold1234 force-pushed the user_info branch from 3383ce0 to c5ac229 Compare November 26, 2024 09:09

mgold1234 force-pushed the user_info branch from c5ac229 to 9a4b637 Compare November 26, 2024 10:09

regexowl reviewed Nov 26, 2024

View reviewed changes

mgold1234 force-pushed the user_info branch from 9a4b637 to 936c15f Compare November 26, 2024 11:27

mgold1234 added 2 commits November 26, 2024 17:09

remove general function

0a44d3d

<wip> - this commit doesnt work, I am trying to figure out how to wor…

881aeb9

…k with users[] object

lucasgarfield reviewed Nov 26, 2024

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

wizard: add user information step (HMS-4903) #2551

wizard: add user information step (HMS-4903) #2551

mgold1234 commented Nov 3, 2024 •

edited

Loading

avitova commented Nov 21, 2024

avitova commented Nov 26, 2024

regexowl left a comment

regexowl Nov 26, 2024

regexowl Nov 26, 2024

lucasgarfield Nov 26, 2024

regexowl Nov 26, 2024

regexowl Nov 26, 2024

mgold1234 Nov 26, 2024

mgold1234 commented Nov 26, 2024

avitova commented Nov 26, 2024

lucasgarfield Nov 25, 2024

lucasgarfield Nov 26, 2024

	Can contain at least 3 different character classes (uppercase
	Must contain at least 3 different character classes (uppercase

wizard: add user information step (HMS-4903) #2551

Are you sure you want to change the base?

wizard: add user information step (HMS-4903) #2551

Conversation

mgold1234 commented Nov 3, 2024 • edited Loading

avitova commented Nov 21, 2024

avitova commented Nov 26, 2024

regexowl 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

mgold1234 commented Nov 26, 2024

avitova commented Nov 26, 2024

Choose a reason for hiding this comment

Choose a reason for hiding this comment

mgold1234 commented Nov 3, 2024 •

edited

Loading