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

Add commands to rekey device and create packages #509

Merged
merged 30 commits into from
Mar 12, 2024
Merged

Conversation

fumer-fubotv
Copy link
Collaborator

@fumer-fubotv fumer-fubotv commented Oct 13, 2023

Added support for the following commands:

  1. Rekey Device: Rekey specified device based on the signing password ("brightscript.remoteControl.signingPassword") and package file ("brightscript.remoteControl.signedPackagePath") provided.

  2. Create Package: This will present user a list of available launch configs to choose from. Once selected it will create a .zip file and .pkg file based on the config and will save them to the ${workspacefolder}/out

  3. Rekey Device and Create Package: This will first rekey device and then create package for the selected launch config

Here's a demo of the "Rekey Device" command:

rekey.mp4

1. Added support to show preview of screenshot
2. Fixed a linter issue
Added support for the following commands:

1. Rekey Device: Rekey specified device based on the signing password ("brightscript.remoteControl.signingPassword") and package file ("brightscript.remoteControl.signedPackagePath") provided.

2. Create Package: This will present user a list of available launch configs to choose from. Once selected it will create a .zip file and .pkg file based on the config and will save them  to the ${workspacefolder}/out

3. Rekey Device and Create Package: This will first rekey device and then create package for the selected launch config
Copy link
Member

@TwitchBronBron TwitchBronBron left a comment

Choose a reason for hiding this comment

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

Hey, after I started looking over this, I realized we need to tweak the design a bit. We should sync up about this so I can explain in more detail. But we shouldn't use the "last used device" stuff like we did with the screenshot logic. Here's a rough overview of how the flow should work:

rekey: 
a) "How do you want to enter your stuff?"
 - "Pick from json file"
 - "Enter manually"
b) prompt for host (prepopulated from json above if possible)
c) Prompt for password (prepopulated from json above if possible)
d) Prompt for signed package path (prepopulated from json above if available)

package: 
a) "What should we package?"
 - "Pick a folder"
 - "Pick a zip"
 - "Pick from a launch.json"
    - show a launch.json option picker
 - "Pick a rokudeploy.json"
	- 

b) prompt for host (prepopulated from json above if possible)
c) Prompt for password (prepopulated from json above if possible)
d) Prompt for signed package path (prepopulated from json above if available)
e) Show a summary of stuff, ask to click "Yes, do it" or "No, something's wrong, cancel".

rekeyAndPackage: same as "package" but do the rekey first, no new input necessary

await this.getRemoteHost();
await this.getRemotePassword();
await this.getSigningPassword();
await this.getSignedPackagePath();
Copy link
Member

Choose a reason for hiding this comment

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

We should show a file picker for this.

Updates based on below reqs:

rekey:
a) "How do you want to enter your stuff?"
 - "Pick from json file"
 - "Enter manually"
b) prompt for host (prepopulated from json above if possible)
c) Prompt for password (prepopulated from json above if possible)
d) Prompt for signed package path (prepopulated from json above if available)

package:
a) "What should we package?"
 - "Pick a folder"
 - "Pick from a launch.json"
    - show a launch.json option picker
 - "Pick a rokudeploy.json"
	-

b) prompt for host (prepopulated from json above if possible)
c) Prompt for password (prepopulated from json above if possible)
d) Prompt for signed package path (prepopulated from json above if available)
e) Show a summary of stuff, ask to click "Yes, do it" or "No, something's wrong, cancel".
Copy link
Member

@TwitchBronBron TwitchBronBron left a comment

Choose a reason for hiding this comment

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

I have a few suggestions:

  1. This stuff in this file getting a bit large, so let's move all this new logic out of BrightScriptCommands.ts into a separate src/commands/RekeyAndPackageCommands.ts file.

  2. There seem to be a few bugs around picking a json file for the "Rekey device" flow. When I pick a rokudeploy.json with only rekeySignedPackage, devId, signingPassword, convertToSquashFS, it fails with an error "A system error occurred (getaddrinfo ENOTFOUND :80). I'm assuming it's because the logic currently assumes that everything was supposed to be included in the json. That's not how it should work. We should load any values we find in the json, but then still walk through the manual prompts to allow the user to confirm those are the values they want, and fill in any missing values.

  3. When running "Rekey Device", and choosing "enter manually". after I enter my signing password, it just pops up a new file picker dialog, but there was no messaging around WHAT I'm supposed to pick. I think we need another intermediary dropdown with instructions saying "pick your pkg" so they know why the file picker appears.

  4. After running the manual rekey for our internal app, I get this error at the end.
    image

src/BrightScriptCommands.ts Outdated Show resolved Hide resolved
@fumer-fubotv
Copy link
Collaborator Author

I have a few suggestions:

  1. This stuff in this file getting a bit large, so let's move all this new logic out of BrightScriptCommands.ts into a separate src/commands/RekeyAndPackageCommands.ts file.
  2. There seem to be a few bugs around picking a json file for the "Rekey device" flow. When I pick a rokudeploy.json with only rekeySignedPackage, devId, signingPassword, convertToSquashFS, it fails with an error "A system error occurred (getaddrinfo ENOTFOUND :80). I'm assuming it's because the logic currently assumes that everything was supposed to be included in the json. That's not how it should work. We should load any values we find in the json, but then still walk through the manual prompts to allow the user to confirm those are the values they want, and fill in any missing values.
  3. When running "Rekey Device", and choosing "enter manually". after I enter my signing password, it just pops up a new file picker dialog, but there was no messaging around WHAT I'm supposed to pick. I think we need another intermediary dropdown with instructions saying "pick your pkg" so they know why the file picker appears.
  4. After running the manual rekey for our internal app, I get this error at the end.
    image

Most of the suggestions are addressed in the update. However i was not able to reproduce the error in #4 you mentioned

@TwitchBronBron
Copy link
Member

TwitchBronBron commented Jan 29, 2024

@fumer-fubotv I finally got a chance to look over this. I've pushed some changes to optimize the flow a bit....somehow I had these sitting in an uncommited branch since November!

The biggest change here is that we really need to prompt the user for all the details every time (either from json or from input boxes). We can't use values like the last password or last host used, because that might not be the device they want to rekey.

One thing we need to tweak, is for the file picker modal, we should add information showing the currently selected zip (if there is one from rokudeploy.json), and add another button in that case saying "I want to change it". I see it having 2 flows:

  1. there is no package picked yet, so they have "Pick a file" and "cancel".
  2. There is already a default package set, so they have "Use the current file", "Pick a different file", and "cancel"
    image

It would be great if you could run through this workflow a few times to test it out, and iron out any rough edges. Then It should probably be good to go.

TwitchBronBron and others added 3 commits January 29, 2024 16:14
1. Added support to show existing pkg file for rekey confirmation
2. Updated create package flow to confirm all the entries with user and ti update - if needed
Copy link
Member

@TwitchBronBron TwitchBronBron left a comment

Choose a reason for hiding this comment

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

Here are a few things to change:

  • when choosing "I want to change something", it's not showing the previously-selected host in the host picker. That userInputManager.promptForHost() function accepts a default value. We should pass in rekeyConfig?host ?? defaultValues?.host
    image

  • the "create package" flow doesn't remember the previously selected "what do you want to package" option when I choose "I want to change something".
    image

  • the "rekey device and create package" command feels quirky. It seems like it just runs the rekey command and then the create package command next. It should collect all user input first (for rekey AND packaging inputs), and THEN show a summary at the end including all rekey inputs AND package/sign inputs. At that point, run both actions.

src/commands/RekeyAndPackageCommand.ts Show resolved Hide resolved
Added support to maintain status of previously selected host and config
@fumer-fubotv
Copy link
Collaborator Author

fumer-fubotv commented Feb 8, 2024

Host and create package flows have been updated as per feedback

Copy link
Member

@TwitchBronBron TwitchBronBron left a comment

Choose a reason for hiding this comment

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

Found a few more issues.

  1. For "Create package" and choosing "use folder", it expects there to be a zip. But my folder has the source files, not a zip. We need to zip that folder as part of this flow.
    image

  2. For "Create Package", when I choose "Use a rokudeploy.json" file, and if that file doesn't have rootDir specified, the packaging fails.
    image
    image

  3. For "Create package", when choosing "use launch.json", it appears to grab all the right values, but then the flow fails
    image
    image

updated rekey and create package flow
@fumer-fubotv
Copy link
Collaborator Author

fumer-fubotv commented Feb 13, 2024

I tried to reproduce the scenarios mentioned above and below are the findings:

  1. Create package using "use folder" works successfully if the specified folder has a manifest in it. Otherwise it will throw an error (Just fyi- Current implementation already zip the source folder user selects)
  2. Create package using "use a rokudeploy.json" expects source to be specified.
  3. Create Package using launch.json has been working fine for me. Lets connect to reproduce/resolve this issue

1. updated outfile name to be the source folder name only
2. Fixed a bug for outdir that only supported mac paths
3. Added title to all inputboxes
4. Added a prompt for rootDir incase it is missing during create package
5. Added outfile path in dialog upon successful packaging
@TwitchBronBron TwitchBronBron changed the title Feature request: add ability to rekey/sign apps Add commands to rekey device and create packages Feb 14, 2024
@TwitchBronBron TwitchBronBron merged commit b05f3a3 into master Mar 12, 2024
5 checks passed
@TwitchBronBron TwitchBronBron deleted the TBD-126 branch March 12, 2024 23:12
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.

2 participants