diff --git a/cmd/notary/delegations.go b/cmd/notary/delegations.go index d47f87b34a..a61b75cb77 100644 --- a/cmd/notary/delegations.go +++ b/cmd/notary/delegations.go @@ -42,8 +42,8 @@ type delegationCommander struct { configGetter func() (*viper.Viper, error) retriever passphrase.Retriever - paths []string - removeAll, removeYes bool + paths []string + allPaths, removeAll, forceYes bool } func (d *delegationCommander) GetCommand() *cobra.Command { @@ -52,12 +52,13 @@ func (d *delegationCommander) GetCommand() *cobra.Command { cmdRemDelg := cmdDelegationRemoveTemplate.ToCommand(d.delegationRemove) cmdRemDelg.Flags().StringSliceVar(&d.paths, "paths", nil, "List of paths to remove") - cmdRemDelg.Flags().BoolVarP( - &d.removeYes, "yes", "y", false, "Answer yes to the removal question (no confirmation)") + cmdRemDelg.Flags().BoolVarP(&d.forceYes, "yes", "y", false, "Answer yes to the removal question (no confirmation)") + cmdRemDelg.Flags().BoolVar(&d.allPaths, "all-paths", false, "Remove all paths from this delegation") cmd.AddCommand(cmdRemDelg) cmdAddDelg := cmdDelegationAddTemplate.ToCommand(d.delegationAdd) cmdAddDelg.Flags().StringSliceVar(&d.paths, "paths", nil, "List of paths to add") + cmdAddDelg.Flags().BoolVar(&d.allPaths, "all-paths", false, "Add all paths to this delegation") cmd.AddCommand(cmdAddDelg) return cmd } @@ -121,20 +122,21 @@ func (d *delegationCommander) delegationRemove(cmd *cobra.Command, args []string } // If we're only given the gun and the role, attempt to remove all data for this delegation - if len(args) == 2 && d.paths == nil { + if len(args) == 2 && d.paths == nil && !d.allPaths { d.removeAll = true } keyIDs := []string{} - // Change nil paths to empty slice for TUF - if d.paths == nil { - d.paths = []string{} - } if len(args) > 2 { keyIDs = args[2:] } + // If the user passes --all-paths, don't use any of the passed in --paths + if d.allPaths { + d.paths = nil + } + // no online operations are performed by add so the transport argument // should be nil nRepo, err := notaryclient.NewNotaryRepository( @@ -146,7 +148,7 @@ func (d *delegationCommander) delegationRemove(cmd *cobra.Command, args []string if d.removeAll { cmd.Println("\nAre you sure you want to remove all data for this delegation? (yes/no)") // Ask for confirmation before force removing delegation - if !d.removeYes { + if !d.forceYes { confirmed := askConfirm() if !confirmed { fatalf("Aborting action.") @@ -160,6 +162,12 @@ func (d *delegationCommander) delegationRemove(cmd *cobra.Command, args []string return fmt.Errorf("failed to remove delegation: %v", err) } } else { + if d.allPaths { + err = nRepo.ClearDelegationPaths(role) + if err != nil { + return fmt.Errorf("failed to remove delegation: %v", err) + } + } // Remove any keys or paths that we passed in err = nRepo.RemoveDelegationKeysAndPaths(role, keyIDs, d.paths) if err != nil { @@ -171,9 +179,17 @@ func (d *delegationCommander) delegationRemove(cmd *cobra.Command, args []string if d.removeAll { cmd.Printf("Forced removal (including all keys and paths) of delegation role %s to repository \"%s\" staged for next publish.\n", role, gun) } else { - cmd.Printf( - "Removal of delegation role %s with keys %s and paths %s, to repository \"%s\" staged for next publish.\n", - role, keyIDs, d.paths, gun) + removingItems := "" + if len(keyIDs) > 0 { + removingItems = removingItems + fmt.Sprintf("with keys %s, ", keyIDs) + } + if d.allPaths { + removingItems = removingItems + "with all paths, " + } + if d.paths != nil { + removingItems = removingItems + fmt.Sprintf("with paths [%s], ", prettyPrintPaths(d.paths)) + } + cmd.Printf("Removal of delegation role %s %sto repository \"%s\" staged for next publish.\n", role, removingItems, gun) } cmd.Println("") @@ -182,7 +198,8 @@ func (d *delegationCommander) delegationRemove(cmd *cobra.Command, args []string // delegationAdd creates a new delegation by adding a public key from a certificate to a specific role in a GUN func (d *delegationCommander) delegationAdd(cmd *cobra.Command, args []string) error { - if len(args) < 2 || len(args) < 3 && d.paths == nil { + // We must have at least the gun and role name, and at least one key or path (or the --all-paths flag) to add + if len(args) < 2 || len(args) < 3 && d.paths == nil && !d.allPaths { cmd.Usage() return fmt.Errorf("must specify the Global Unique Name and the role of the delegation along with the public key certificate paths and/or a list of paths to add") } @@ -214,6 +231,18 @@ func (d *delegationCommander) delegationAdd(cmd *cobra.Command, args []string) e } } + for _, path := range d.paths { + if path == "" { + d.allPaths = true + break + } + } + + // If the user passes --all-paths (or gave the "" path in --paths), give the "" path + if d.allPaths { + d.paths = []string{""} + } + // no online operations are performed by add so the transport argument // should be nil nRepo, err := notaryclient.NewNotaryRepository( @@ -239,9 +268,16 @@ func (d *delegationCommander) delegationAdd(cmd *cobra.Command, args []string) e } cmd.Println("") + addingItems := "" + if len(pubKeyIDs) > 0 { + addingItems = addingItems + fmt.Sprintf("with keys %s, ", pubKeys) + } + if d.paths != nil || d.allPaths { + addingItems = addingItems + fmt.Sprintf("with paths [%s], ", prettyPrintPaths(d.paths)) + } cmd.Printf( - "Addition of delegation role %s with keys %s and paths %s, to repository \"%s\" staged for next publish.\n", - role, pubKeyIDs, d.paths, gun) + "Addition of delegation role %s %sto repository \"%s\" staged for next publish.\n", + role, addingItems, gun) cmd.Println("") return nil } diff --git a/cmd/notary/integration_test.go b/cmd/notary/integration_test.go index 292b8ce9b9..e176556d7e 100644 --- a/cmd/notary/integration_test.go +++ b/cmd/notary/integration_test.go @@ -195,10 +195,11 @@ func TestClientDelegationsInteraction(t *testing.T) { assert.NoError(t, err) assert.Contains(t, output, "No delegations present in this repository.") - // add new valid delegation with single new cert + // add new valid delegation with single new cert, and no path output, err = runCommand(t, tempDir, "delegation", "add", "gun", "targets/delegation", tempFile.Name()) assert.NoError(t, err) assert.Contains(t, output, "Addition of delegation role") + assert.NotContains(t, output, "path") // check status - see delegation output, err = runCommand(t, tempDir, "status", "gun") @@ -219,11 +220,30 @@ func TestClientDelegationsInteraction(t *testing.T) { assert.NoError(t, err) assert.Contains(t, output, "No unpublished changes for gun") - // list delegations - we should see our added delegation + // list delegations - we should see our added delegation, with no paths output, err = runCommand(t, tempDir, "-s", server.URL, "delegation", "list", "gun") assert.NoError(t, err) assert.Contains(t, output, "targets/delegation") assert.Contains(t, output, keyID) + assert.NotContains(t, output, "\"\"") + + // add all paths to this delegation + output, err = runCommand(t, tempDir, "delegation", "add", "gun", "targets/delegation", "--all-paths") + assert.NoError(t, err) + assert.Contains(t, output, "Addition of delegation role") + assert.Contains(t, output, "\"\"") + assert.Contains(t, output, "") + + // publish repo + _, err = runCommand(t, tempDir, "-s", server.URL, "publish", "gun") + assert.NoError(t, err) + + // list delegations - we should see our added delegation, with no paths + output, err = runCommand(t, tempDir, "-s", server.URL, "delegation", "list", "gun") + assert.NoError(t, err) + assert.Contains(t, output, "targets/delegation") + assert.Contains(t, output, "\"\"") + assert.Contains(t, output, "") // Setup another certificate tempFile2, err := ioutil.TempFile("", "pemfile2") @@ -246,7 +266,7 @@ func TestClientDelegationsInteraction(t *testing.T) { keyID2, err := utils.CanonicalKeyID(parsedPubKey2) assert.NoError(t, err) - // add to the delegation by specifying the same role, this time add another key and path + // add to the delegation by specifying the same role, this time add a scoped path output, err = runCommand(t, tempDir, "delegation", "add", "gun", "targets/delegation", tempFile2.Name(), "--paths", "path") assert.NoError(t, err) assert.Contains(t, output, "Addition of delegation role") @@ -362,6 +382,124 @@ func TestClientDelegationsInteraction(t *testing.T) { assert.Contains(t, output, keyID) assert.Contains(t, output, keyID2) + // Add a bunch of individual paths so we can test a delegation remove --all-paths + output, err = runCommand(t, tempDir, "delegation", "add", "gun", "targets/delegation", "--paths", "abcdef,123456") + assert.NoError(t, err) + + // Add more individual paths so we can test a delegation remove --all-paths + output, err = runCommand(t, tempDir, "delegation", "add", "gun", "targets/delegation", "--paths", "banana/split,apple/crumble/pie,orange.peel,kiwi") + assert.NoError(t, err) + + // publish repo + _, err = runCommand(t, tempDir, "-s", server.URL, "publish", "gun") + assert.NoError(t, err) + + // list delegations - we should see all of our paths + output, err = runCommand(t, tempDir, "-s", server.URL, "delegation", "list", "gun") + assert.NoError(t, err) + assert.Contains(t, output, "abcdef") + assert.Contains(t, output, "123456") + assert.Contains(t, output, "banana/split") + assert.Contains(t, output, "apple/crumble/pie") + assert.Contains(t, output, "orange.peel") + assert.Contains(t, output, "kiwi") + + // Try adding "", and check that adding it with other paths clears out the others + output, err = runCommand(t, tempDir, "delegation", "add", "gun", "targets/delegation", "--paths", "\"\",grapefruit,pomegranate") + assert.NoError(t, err) + + // publish repo + _, err = runCommand(t, tempDir, "-s", server.URL, "publish", "gun") + assert.NoError(t, err) + + // list delegations - we should see all of our old paths, and "" + output, err = runCommand(t, tempDir, "-s", server.URL, "delegation", "list", "gun") + assert.NoError(t, err) + assert.Contains(t, output, "abcdef") + assert.Contains(t, output, "123456") + assert.Contains(t, output, "banana/split") + assert.Contains(t, output, "apple/crumble/pie") + assert.Contains(t, output, "orange.peel") + assert.Contains(t, output, "kiwi") + assert.Contains(t, output, "\"\"") + assert.NotContains(t, output, "grapefruit") + assert.NotContains(t, output, "pomegranate") + + // Try removing just "" + output, err = runCommand(t, tempDir, "delegation", "remove", "gun", "targets/delegation", "--paths", "\"\"") + assert.NoError(t, err) + + // publish repo + _, err = runCommand(t, tempDir, "-s", server.URL, "publish", "gun") + assert.NoError(t, err) + + // list delegations - we should see all of our old paths without "" + output, err = runCommand(t, tempDir, "-s", server.URL, "delegation", "list", "gun") + assert.NoError(t, err) + assert.Contains(t, output, "abcdef") + assert.Contains(t, output, "123456") + assert.Contains(t, output, "banana/split") + assert.Contains(t, output, "apple/crumble/pie") + assert.Contains(t, output, "orange.peel") + assert.Contains(t, output, "kiwi") + assert.NotContains(t, output, "\"\"") + + // Remove --all-paths to clear out all paths from this delegation + output, err = runCommand(t, tempDir, "delegation", "remove", "gun", "targets/delegation", "--all-paths") + assert.NoError(t, err) + + // publish repo + _, err = runCommand(t, tempDir, "-s", server.URL, "publish", "gun") + assert.NoError(t, err) + + // list delegations - we should see all of our paths + output, err = runCommand(t, tempDir, "-s", server.URL, "delegation", "list", "gun") + assert.NoError(t, err) + assert.NotContains(t, output, "abcdef") + assert.NotContains(t, output, "123456") + assert.NotContains(t, output, "banana/split") + assert.NotContains(t, output, "apple/crumble/pie") + assert.NotContains(t, output, "orange.peel") + assert.NotContains(t, output, "kiwi") + + // Check that we ignore other --paths if we pass in --all-paths on an add + output, err = runCommand(t, tempDir, "delegation", "add", "gun", "targets/delegation", "--all-paths", "--paths", "grapefruit,pomegranate") + assert.NoError(t, err) + + // publish repo + _, err = runCommand(t, tempDir, "-s", server.URL, "publish", "gun") + assert.NoError(t, err) + + // list delegations - we should only see "", and not the other paths specified + output, err = runCommand(t, tempDir, "-s", server.URL, "delegation", "list", "gun") + assert.NoError(t, err) + assert.Contains(t, output, "\"\"") + assert.NotContains(t, output, "grapefruit") + assert.NotContains(t, output, "pomegranate") + + // Add those extra paths we ignored to set up the next test + output, err = runCommand(t, tempDir, "delegation", "add", "gun", "targets/delegation", "--paths", "grapefruit,pomegranate") + assert.NoError(t, err) + + // publish repo + _, err = runCommand(t, tempDir, "-s", server.URL, "publish", "gun") + assert.NoError(t, err) + + // Check that we ignore other --paths if we pass in --all-paths on a remove + output, err = runCommand(t, tempDir, "delegation", "remove", "gun", "targets/delegation", "--all-paths", "--paths", "pomegranate") + assert.NoError(t, err) + + // publish repo + _, err = runCommand(t, tempDir, "-s", server.URL, "publish", "gun") + assert.NoError(t, err) + + // list delegations - we should see no paths + output, err = runCommand(t, tempDir, "-s", server.URL, "delegation", "list", "gun") + assert.NoError(t, err) + assert.NotContains(t, output, "\"\"") + assert.NotContains(t, output, "grapefruit") + assert.NotContains(t, output, "pomegranate") + // remove by force to delete the delegation entirely output, err = runCommand(t, tempDir, "delegation", "remove", "gun", "targets/delegation", "-y") assert.NoError(t, err) diff --git a/cmd/notary/prettyprint.go b/cmd/notary/prettyprint.go index 6adc619cf6..a1d917dcca 100644 --- a/cmd/notary/prettyprint.go +++ b/cmd/notary/prettyprint.go @@ -190,7 +190,7 @@ func prettyPrintRoles(rs []*data.Role, writer io.Writer, roleType string) { for _, r := range rs { table.Append([]string{ r.Name, - strings.Join(r.Paths, ","), + prettyPrintPaths(r.Paths), strings.Join(r.KeyIDs, ","), fmt.Sprintf("%v", r.Threshold), }) @@ -198,6 +198,21 @@ func prettyPrintRoles(rs []*data.Role, writer io.Writer, roleType string) { table.Render() } +// Pretty-prints a list of delegation paths, and ensures the empty string is printed as "" in the console +func prettyPrintPaths(paths []string) string { + // sort paths first + sort.Strings(paths) + prettyPaths := []string{} + for _, path := range paths { + // manually escape "" and designate that it is all paths with an extra print + if path == "" { + path = "\"\" " + } + prettyPaths = append(prettyPaths, path) + } + return strings.Join(prettyPaths, ",") +} + // --- pretty printing certs --- // cert by repo name then expiry time. Don't bother sorting by fingerprint. diff --git a/cmd/notary/prettyprint_test.go b/cmd/notary/prettyprint_test.go index 496b4c89c5..b47275b5bb 100644 --- a/cmd/notary/prettyprint_test.go +++ b/cmd/notary/prettyprint_test.go @@ -249,7 +249,7 @@ func TestPrettyPrintSortedRoles(t *testing.T) { {"targets/aardvark/unicorn/pony", "rainbows", "135", "1"}, {"targets/bee", "honey", "246", "1"}, {"targets/bee/wasp", "honey/sting", "246,468", "1"}, - {"targets/zebra", "stripes,black,white", "101", "1"}, + {"targets/zebra", "black,stripes,white", "101", "1"}, } lines := strings.Split(strings.TrimSpace(string(text)), "\n")