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

Break up private functions & search menu bugfix #152

Open
wants to merge 69 commits into
base: v2.7.11
Choose a base branch
from

Conversation

jworkmanjc
Copy link
Contributor

@jworkmanjc jworkmanjc commented Jan 13, 2025

Issues

  • CUT-4101 - Break up the private functions
  • CUT-4541 - Search menu Re-Registration

What does this solve?

  • Breaking up all the functions defined in the start-migration.ps1 file into private and public functions
  • Update the build process into a single function/ file build.ps1
  • Build a new template file used to generate the EXE
  • Resolves an issue where migrated user accounts no longer can access the search menu
  • Adds an additional option to select a different MTP organization after initially selecting one in the selection form.
  • Update the selection form to validate and prevent migrations where local users without a valid SID exist

Is there anything particularly tricky?

How should this be tested?

Additional tests have been written to validate the changes to the selection form. There are three manual test files to check in this release. A number of items have changed in this release we need to validate those changes with the manual test files:

  • Tests/manualTests/ProgressForm.feature - these tests validate the expected behavior of the progress form (during migration)
  • Tests/manualTests/SelectionForm.feature - these tests validate that the migrateProfile button can be selected given various input (the selection form UI)
  • Tests/manualTests/uwp.feature - these tests validate that the uwp app functions as expected post-migration

Screenshots

@jworkmanjc jworkmanjc added ADMU ADMU Module Release patch Patch version release labels Jan 13, 2025
@jworkmanjc jworkmanjc marked this pull request as ready for review January 27, 2025 16:18
@jworkmanjc jworkmanjc requested a review from a team as a code owner January 27, 2025 16:18
@jworkmanjc jworkmanjc changed the base branch from master to v2.7.11 January 28, 2025 19:13
@kmaranionjc kmaranionjc self-requested a review January 28, 2025 20:57
@manual
Feature: Selection form allows user migration

Scenario: Allows "Migrate Profile" Button with domain profile, username and password
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

After selecting the profile and inputting the Username, Migrate Profile was enabled even with password remain unchanged and error icon is still on.
image

This may confuse some users? Should we set it to the green checkmark as default?

When a domain profile is selected
And the "Install JCAgent" checkbox is selected
And a valid "ConnectKey" is pasted in the text field
And the "Install JCAgent" checkbox is selected
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Duplicate. Should be Autobind and valid API key

Given The selection form is opened
And a domain (local AD or EntraID) user has logged into the device
When a domain profile is selected
And a JumpCloud username text "" is specified in the text box
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
And a JumpCloud username text "" is specified in the text box
And a null/blank JumpCloud username in the text box

And a password is specified
Then the "Migrate Profile" button should NOT become active and be greyed-out

Scenario: Disallows "Migrate Profile" Button for JumpCloud Usernames that match the hostname
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Username textbox should have the error icon if the hostname is inputted
image


Feature: JumpCloud Username & AutoBind User Validation

Scenario: Disallows migration for usernames that are not found in the JumpCloud organization
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can we not just disable the Migrate Profile button?
image

And the "Migrate Profile" button is pressed
Then a windows form window should appear and warn: The JumpCloud agent is not installed and to also enter your connectKey

Scenario: Disallows migration for usernames that are not found in the JumpCloud organization
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Username inputted is not a JCUser. Got same error message as the test above:
image

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

disregard, form shows the correct message

And a JumpCloud username that does exist in the JumpCloud Organization is specified in the username text box
And a valid password is specified in the password text box
And the "Migrate Profile" button is pressed
Then a windows form window should appear and note that the specified user has a local account username/ can select OK or Cancel to return to selection form
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Form window seems to not show the right message as seen with the last 2 tests:
image

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

disregard, form shows the correct message

And a JumpCloud username who has a localUsername field specified (that matches the local user created for this test) that does exist in the JumpCloud Organization is specified in the username text box
And a valid password is specified in the password text box
And the "Migrate Profile" button is pressed
Then a windows form window should appear and note migration can not continue when the specified user has a localUsername that matches a user that exists on the device
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Same as above tests:
image

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

disregard, form shows the correct message

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Tests looks good here
image

And the new local account's default .txt editor should be set to the same application prior to migration
And the new local account should have the installed Microsoft Store applications prior to migration available
And the new local account's start menu/ search application (other UWP) applications should be able to be run
And the logs for the UWP application should be available in the `C:\Users\someNewUsername\AppData\Local\JumpCloudADMU` directory, including the list of FTA/PTA/UWP apps to set and the log recording the actions the UWP app attempted.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Test looks good
image

@kmaranionjc kmaranionjc self-requested a review January 29, 2025 04:28
Copy link
Contributor

@kmaranionjc kmaranionjc left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

  • Invalid API key, migrate button still gets enabled
image - Too much empty space with the MTP form window image

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
ADMU ADMU Module Release patch Patch version release
Development

Successfully merging this pull request may close these issues.

2 participants