Merge pull request #528 from docker/delegation-api

Break down client API for delegations

client/changelist/change.go

@@ -88,6 +88,7 @@ type TufDelegation struct {
 	RemoveKeys             []string     `json:"remove_keys,omitempty"`
 	AddPaths               []string     `json:"add_paths,omitempty"`
 	RemovePaths            []string     `json:"remove_paths,omitempty"`
+	ClearAllPaths          bool         `json:"clear_paths,omitempty"`
 	AddPathHashPrefixes    []string     `json:"add_prefixes,omitempty"`
 	RemovePathHashPrefixes []string     `json:"remove_prefixes,omitempty"`
 }

client/client.go

@@ -24,7 +24,6 @@ import (
 	"github.com/docker/notary/tuf/keys"
 	"github.com/docker/notary/tuf/signed"
 	"github.com/docker/notary/tuf/store"
-	"github.com/docker/notary/tuf/utils"
 )
 
 func init() {
@@ -302,96 +301,6 @@ func addChange(cl *changelist.FileChangelist, c changelist.Change, roles ...stri
 	return nil
 }
 
-// AddDelegation creates a new changelist entry to add a delegation to the repository
-// when the changelist gets applied at publish time.  This does not do any validation
-// other than checking the name of the delegation to add - all that will happen
-// at publish time.
-func (r *NotaryRepository) AddDelegation(name string, threshold int,
-	delegationKeys []data.PublicKey, paths []string) error {
-
-	if !data.IsDelegation(name) {
-		return data.ErrInvalidRole{Role: name, Reason: "invalid delegation role name"}
-	}
-
-	cl, err := changelist.NewFileChangelist(filepath.Join(r.tufRepoPath, "changelist"))
-	if err != nil {
-		return err
-	}
-	defer cl.Close()
-
-	logrus.Debugf(`Adding delegation "%s" with threshold %d, and %d keys\n`,
-		name, threshold, len(delegationKeys))
-
-	tdJSON, err := json.Marshal(&changelist.TufDelegation{
-		NewThreshold: threshold,
-		AddKeys:      data.KeyList(delegationKeys),
-		AddPaths:     paths,
-	})
-	if err != nil {
-		return err
-	}
-
-	template := changelist.NewTufChange(
-		changelist.ActionCreate,
-		name,
-		changelist.TypeTargetsDelegation,
-		"", // no path
-		tdJSON,
-	)
-
-	return addChange(cl, template, name)
-}
-
-// RemoveDelegation creates a new changelist entry to remove a delegation from
-// the repository when the changelist gets applied at publish time.
-// This does not validate that the delegation exists, since one might exist
-// after applying all changes.
-func (r *NotaryRepository) RemoveDelegation(name string, keyIDs, paths []string, removeAll bool) error {
-
-	if !data.IsDelegation(name) {
-		return data.ErrInvalidRole{Role: name, Reason: "invalid delegation role name"}
-	}
-
-	cl, err := changelist.NewFileChangelist(filepath.Join(r.tufRepoPath, "changelist"))
-	if err != nil {
-		return err
-	}
-	defer cl.Close()
-
-	logrus.Debugf(`Removing delegation "%s"\n`, name)
-	var template *changelist.TufChange
-
-	// We use the Delete action only for force removal, Update is used for removing individual keys and paths
-	if removeAll {
-		template = changelist.NewTufChange(
-			changelist.ActionDelete,
-			name,
-			changelist.TypeTargetsDelegation,
-			"",  // no path
-			nil, // deleting role, no data needed
-		)
-
-	} else {
-		tdJSON, err := json.Marshal(&changelist.TufDelegation{
-			RemoveKeys:  keyIDs,
-			RemovePaths: paths,
-		})
-		if err != nil {
-			return err
-		}
-
-		template = changelist.NewTufChange(
-			changelist.ActionUpdate,
-			name,
-			changelist.TypeTargetsDelegation,
-			"", // no path
-			tdJSON,
-		)
-	}
-
-	return addChange(cl, template, name)
-}
-
 // AddTarget creates new changelist entries to add a target to the given roles
 // in the repository when the changelist gets applied at publish time.
 // If roles are unspecified, the default role is "targets".
@@ -529,79 +438,6 @@ func (r *NotaryRepository) GetChangelist() (changelist.Changelist, error) {
 	return cl, nil
 }
 
-// GetDelegationRoles returns the keys and roles of the repository's delegations
-// Also converts key IDs to canonical key IDs to keep consistent with signing prompts
-func (r *NotaryRepository) GetDelegationRoles() ([]*data.Role, error) {
-	// Update state of the repo to latest
-	if _, err := r.Update(false); err != nil {
-		return nil, err
-	}
-
-	// All top level delegations (ex: targets/level1) are stored exclusively in targets.json
-	targets, ok := r.tufRepo.Targets[data.CanonicalTargetsRole]
-	if !ok {
-		return nil, store.ErrMetaNotFound{Resource: data.CanonicalTargetsRole}
-	}
-
-	// make a copy of top-level Delegations and only show canonical key IDs
-	allDelegations, err := translateDelegationsToCanonicalIDs(targets.Signed.Delegations)
-	if err != nil {
-		return nil, err
-	}
-
-	// make a copy for traversing nested delegations
-	delegationsList := make([]*data.Role, len(allDelegations))
-	copy(delegationsList, allDelegations)
-
-	// Now traverse to lower level delegations (ex: targets/level1/level2)
-	for len(delegationsList) > 0 {
-		// Pop off first delegation to traverse
-		delegation := delegationsList[0]
-		delegationsList = delegationsList[1:]
-
-		// Get metadata
-		delegationMeta, ok := r.tufRepo.Targets[delegation.Name]
-		// If we get an error, don't try to traverse further into this subtree because it doesn't exist or is malformed
-		if !ok {
-			continue
-		}
-
-		// For the return list, update with a copy that includes canonicalKeyIDs
-		canonicalDelegations, err := translateDelegationsToCanonicalIDs(delegationMeta.Signed.Delegations)
-		if err != nil {
-			return nil, err
-		}
-		allDelegations = append(allDelegations, canonicalDelegations...)
-		// Add nested delegations to the exploration list
-		delegationsList = append(delegationsList, delegationMeta.Signed.Delegations.Roles...)
-	}
-
-	// Convert all key IDs to canonical IDs:
-	return allDelegations, nil
-}
-
-func translateDelegationsToCanonicalIDs(delegationInfo data.Delegations) ([]*data.Role, error) {
-	canonicalDelegations := make([]*data.Role, len(delegationInfo.Roles))
-	copy(canonicalDelegations, delegationInfo.Roles)
-	delegationKeys := delegationInfo.Keys
-	for i, delegation := range canonicalDelegations {
-		canonicalKeyIDs := []string{}
-		for _, keyID := range delegation.KeyIDs {
-			pubKey, ok := delegationKeys[keyID]
-			if !ok {
-				return nil, fmt.Errorf("Could not translate canonical key IDs for %s", delegation.Name)
-			}
-			canonicalKeyID, err := utils.CanonicalKeyID(pubKey)
-			if err != nil {
-				return nil, fmt.Errorf("Could not translate canonical key IDs for %s: %v", delegation.Name, err)
-			}
-			canonicalKeyIDs = append(canonicalKeyIDs, canonicalKeyID)
-		}
-		canonicalDelegations[i].KeyIDs = canonicalKeyIDs
-	}
-	return canonicalDelegations, nil
-}
-
 // RoleWithSignatures is a Role with its associated signatures
 type RoleWithSignatures struct {
 	Signatures []data.Signature

client/client_test.go

@@ -1910,10 +1910,10 @@ func testPublishDelegations(t *testing.T, clearCache, x509Keys bool) {
 	// targets/a, because these should execute in order
 	for _, delgName := range []string{"targets/a", "targets/a/b", "targets/c"} {
 		assert.NoError(t,
-			repo1.AddDelegation(delgName, 1, []data.PublicKey{delgKey}, []string{""}),
+			repo1.AddDelegation(delgName, []data.PublicKey{delgKey}, []string{""}),
 			"error creating delegation")
 	}
-	assert.Len(t, getChanges(t, repo1), 3, "wrong number of changelist files found")
+	assert.Len(t, getChanges(t, repo1), 6, "wrong number of changelist files found")
 
 	var rec *passRoleRecorder
 	if clearCache {
@@ -1932,11 +1932,11 @@ func testPublishDelegations(t *testing.T, clearCache, x509Keys bool) {
 
 	// this should not publish, because targets/z doesn't exist
 	assert.NoError(t,
-		repo1.AddDelegation("targets/z/y", 1, []data.PublicKey{delgKey}, []string{""}),
+		repo1.AddDelegation("targets/z/y", []data.PublicKey{delgKey}, []string{""}),
 		"error creating delegation")
-	assert.Len(t, getChanges(t, repo1), 1, "wrong number of changelist files found")
+	assert.Len(t, getChanges(t, repo1), 2, "wrong number of changelist files found")
 	assert.Error(t, repo1.Publish())
-	assert.Len(t, getChanges(t, repo1), 1, "wrong number of changelist files found")
+	assert.Len(t, getChanges(t, repo1), 2, "wrong number of changelist files found")
 
 	if clearCache {
 		rec.assertAsked(t, nil)
@@ -2035,7 +2035,7 @@ func testPublishTargetsDelgationScopeNoFallbackIfNoKeys(t *testing.T, clearCache
 	}
 
 	// ensure that the role exists
-	assert.NoError(t, repo.AddDelegation("targets/a", 1, []data.PublicKey{aPubKey}, []string{""}))
+	assert.NoError(t, repo.AddDelegation("targets/a", []data.PublicKey{aPubKey}, []string{""}))
 	assert.NoError(t, repo.Publish())
 
 	if clearCache {
@@ -2076,7 +2076,7 @@ func TestPublishTargetsDelgationSuccessLocallyHasRoles(t *testing.T) {
 	for _, delgName := range []string{"targets/a", "targets/a/b"} {
 		delgKey := createKey(t, repo, delgName, false)
 		assert.NoError(t,
-			repo.AddDelegation(delgName, 1, []data.PublicKey{delgKey}, []string{""}),
+			repo.AddDelegation(delgName, []data.PublicKey{delgKey}, []string{""}),
 			"error creating delegation")
 	}
 
@@ -2106,7 +2106,7 @@ func TestPublishTargetsDelgationNoTargetsKeyNeeded(t *testing.T) {
 	for _, delgName := range []string{"targets/a", "targets/a/b"} {
 		delgKey := createKey(t, repo, delgName, false)
 		assert.NoError(t,
-			repo.AddDelegation(delgName, 1, []data.PublicKey{delgKey}, []string{""}),
+			repo.AddDelegation(delgName, []data.PublicKey{delgKey}, []string{""}),
 			"error creating delegation")
 	}
 
@@ -2170,10 +2170,10 @@ func TestPublishTargetsDelgationSuccessNeedsToDownloadRoles(t *testing.T) {
 
 	// owner creates delegations, adds the delegated key to them, and publishes them
 	assert.NoError(t,
-		ownerRepo.AddDelegation("targets/a", 1, []data.PublicKey{aKey}, []string{""}),
+		ownerRepo.AddDelegation("targets/a", []data.PublicKey{aKey}, []string{""}),
 		"error creating delegation")
 	assert.NoError(t,
-		ownerRepo.AddDelegation("targets/a/b", 1, []data.PublicKey{bKey}, []string{""}),
+		ownerRepo.AddDelegation("targets/a/b", []data.PublicKey{bKey}, []string{""}),
 		"error creating delegation")
 
 	assert.NoError(t, ownerRepo.Publish())
@@ -2212,7 +2212,7 @@ func TestPublishTargetsDelgationFromTwoRepos(t *testing.T) {
 
 	// delegation includes both keys
 	assert.NoError(t,
-		repo1.AddDelegation("targets/a", 1, []data.PublicKey{key1, key2}, []string{""}),
+		repo1.AddDelegation("targets/a", []data.PublicKey{key1, key2}, []string{""}),
 		"error creating delegation")
 
 	assert.NoError(t, repo1.Publish())
@@ -2282,7 +2282,7 @@ func TestPublishRemoveDelgationKeyFromDelegationRole(t *testing.T) {
 
 	// owner creates delegation, adds the delegated key to it, and publishes it
 	assert.NoError(t,
-		ownerRepo.AddDelegation("targets/a", 1, []data.PublicKey{aKey}, []string{""}),
+		ownerRepo.AddDelegation("targets/a", []data.PublicKey{aKey}, []string{""}),
 		"error creating delegation")
 	assert.NoError(t, ownerRepo.Publish())
 
@@ -2340,7 +2340,7 @@ func TestPublishRemoveDelgation(t *testing.T) {
 
 	// owner creates delegation, adds the delegated key to it, and publishes it
 	assert.NoError(t,
-		ownerRepo.AddDelegation("targets/a", 1, []data.PublicKey{aKey}, []string{""}),
+		ownerRepo.AddDelegation("targets/a", []data.PublicKey{aKey}, []string{""}),
 		"error creating delegation")
 	assert.NoError(t, ownerRepo.Publish())
 
@@ -2351,7 +2351,7 @@ func TestPublishRemoveDelgation(t *testing.T) {
 	// owner removes delegation
 	aKeyCanonicalID, err := utils.CanonicalKeyID(aKey)
 	assert.NoError(t, err)
-	assert.NoError(t, ownerRepo.RemoveDelegation("targets/a", []string{aKeyCanonicalID}, []string{}, false))
+	assert.NoError(t, ownerRepo.RemoveDelegationKeys("targets/a", []string{aKeyCanonicalID}))
 	assert.NoError(t, ownerRepo.Publish())
 
 	// delegated repo can now no longer publish to delegated role
@@ -2374,7 +2374,7 @@ func TestPublishSucceedsDespiteDelegationCorrupt(t *testing.T) {
 	assert.NoError(t, err, "error creating delegation key")
 
 	assert.NoError(t,
-		repo.AddDelegation("targets/a", 1, []data.PublicKey{delgKey}, []string{""}),
+		repo.AddDelegation("targets/a", []data.PublicKey{delgKey}, []string{""}),
 		"error creating delegation")
 
 	testPublishBadMetadata(t, "targets/a", repo, false, true)
@@ -2609,22 +2609,25 @@ func TestAddDelegationChangefileValid(t *testing.T) {
 	targetPubKey := repo.CryptoService.GetKey(targetKeyIds[0])
 	assert.NotNil(t, targetPubKey)
 
-	err := repo.AddDelegation("root", 1, []data.PublicKey{targetPubKey}, []string{""})
+	err := repo.AddDelegation("root", []data.PublicKey{targetPubKey}, []string{""})
 	assert.Error(t, err)
 	assert.IsType(t, data.ErrInvalidRole{}, err)
 	assert.Empty(t, getChanges(t, repo))
 
 	// to show that adding does not care about the hierarchy
-	err = repo.AddDelegation("targets/a/b/c", 1, []data.PublicKey{targetPubKey}, []string{""})
+	err = repo.AddDelegation("targets/a/b/c", []data.PublicKey{targetPubKey}, []string{""})
 	assert.NoError(t, err)
 
 	// ensure that the changefiles is correct
 	changes := getChanges(t, repo)
-	assert.Len(t, changes, 1)
+	assert.Len(t, changes, 2)
 	assert.Equal(t, changelist.ActionCreate, changes[0].Action())
 	assert.Equal(t, "targets/a/b/c", changes[0].Scope())
 	assert.Equal(t, changelist.TypeTargetsDelegation, changes[0].Type())
-	assert.Equal(t, "", changes[0].Path())
+	assert.Equal(t, changelist.ActionCreate, changes[1].Action())
+	assert.Equal(t, "targets/a/b/c", changes[1].Scope())
+	assert.Equal(t, changelist.TypeTargetsDelegation, changes[1].Type())
+	assert.Equal(t, "", changes[1].Path())
 	assert.NotEmpty(t, changes[0].Content())
 }
 
@@ -2645,10 +2648,10 @@ func TestAddDelegationChangefileApplicable(t *testing.T) {
 	assert.NotNil(t, targetPubKey)
 
 	// this hierarchy has to be right to be applied
-	err := repo.AddDelegation("targets/a", 1, []data.PublicKey{targetPubKey}, []string{""})
+	err := repo.AddDelegation("targets/a", []data.PublicKey{targetPubKey}, []string{""})
 	assert.NoError(t, err)
 	changes := getChanges(t, repo)
-	assert.Len(t, changes, 1)
+	assert.Len(t, changes, 2)
 
 	// ensure that it can be applied correctly
 	err = applyTargetsChange(repo.tufRepo, changes[0])
@@ -2676,7 +2679,7 @@ func TestAddDelegationErrorWritingChanges(t *testing.T) {
 		targetPubKey := repo.CryptoService.GetKey(targetKeyIds[0])
 		assert.NotNil(t, targetPubKey)
 
-		return repo.AddDelegation("targets/a", 1, []data.PublicKey{targetPubKey}, []string{""})
+		return repo.AddDelegation("targets/a", []data.PublicKey{targetPubKey}, []string{""})
 	})
 }
 
@@ -2693,14 +2696,14 @@ func TestRemoveDelegationChangefileValid(t *testing.T) {
 	rootPubKey := repo.CryptoService.GetKey(rootKeyID)
 	assert.NotNil(t, rootPubKey)
 
-	err := repo.RemoveDelegation("root", []string{rootKeyID}, []string{}, false)
+	err := repo.RemoveDelegationKeys("root", []string{rootKeyID})
 	assert.Error(t, err)
 	assert.IsType(t, data.ErrInvalidRole{}, err)
 	assert.Empty(t, getChanges(t, repo))
 
 	// to demonstrate that so long as the delegation name is valid, the
 	// existence of the delegation doesn't matter
-	assert.NoError(t, repo.RemoveDelegation("targets/a/b/c", []string{rootKeyID}, []string{}, false))
+	assert.NoError(t, repo.RemoveDelegationKeys("targets/a/b/c", []string{rootKeyID}))
 
 	// ensure that the changefile is correct
 	changes := getChanges(t, repo)
@@ -2711,7 +2714,7 @@ func TestRemoveDelegationChangefileValid(t *testing.T) {
 	assert.Equal(t, "", changes[0].Path())
 }
 
-// The changefile produced by RemoveDelegation, when applied, actually removes
+// The changefile produced by RemoveDelegationKeys, when applied, actually removes
 // the delegation from the repo (assuming the repo exists - tests for
 // change application validation are in helpers_test.go)
 func TestRemoveDelegationChangefileApplicable(t *testing.T) {
@@ -2725,10 +2728,11 @@ func TestRemoveDelegationChangefileApplicable(t *testing.T) {
 	assert.NotNil(t, rootPubKey)
 
 	// add a delegation first so it can be removed
-	assert.NoError(t, repo.AddDelegation("targets/a", 1, []data.PublicKey{rootPubKey}, []string{""}))
+	assert.NoError(t, repo.AddDelegation("targets/a", []data.PublicKey{rootPubKey}, []string{""}))
 	changes := getChanges(t, repo)
-	assert.Len(t, changes, 1)
+	assert.Len(t, changes, 2)
 	assert.NoError(t, applyTargetsChange(repo.tufRepo, changes[0]))
+	assert.NoError(t, applyTargetsChange(repo.tufRepo, changes[1]))
 
 	targetRole := repo.tufRepo.Targets[data.CanonicalTargetsRole]
 	assert.Len(t, targetRole.Signed.Delegations.Roles, 1)
@@ -2737,21 +2741,137 @@ func TestRemoveDelegationChangefileApplicable(t *testing.T) {
 	// now remove it
 	rootKeyCanonicalID, err := utils.CanonicalKeyID(rootPubKey)
 	assert.NoError(t, err)
-	assert.NoError(t, repo.RemoveDelegation("targets/a", []string{rootKeyCanonicalID}, []string{}, false))
+	assert.NoError(t, repo.RemoveDelegationKeys("targets/a", []string{rootKeyCanonicalID}))
 	changes = getChanges(t, repo)
-	assert.Len(t, changes, 2)
+	assert.Len(t, changes, 3)
-	assert.NoError(t, applyTargetsChange(repo.tufRepo, changes[1]))
+	assert.NoError(t, applyTargetsChange(repo.tufRepo, changes[2]))
 
 	targetRole = repo.tufRepo.Targets[data.CanonicalTargetsRole]
 	assert.Empty(t, targetRole.Signed.Delegations.Roles)
 	assert.Empty(t, targetRole.Signed.Delegations.Keys)
 }
 
+// The changefile with the ClearAllPaths key set, when applied, actually removes
+// all paths from the specified delegation in the repo (assuming the repo and delegation exist)
+func TestClearAllPathsDelegationChangefileApplicable(t *testing.T) {
+	gun := "docker.com/notary"
+	ts, _, _ := simpleTestServer(t)
+	defer ts.Close()
+
+	repo, rootKeyID := initializeRepo(t, data.ECDSAKey, gun, ts.URL, false)
+	defer os.RemoveAll(repo.baseDir)
+	rootPubKey := repo.CryptoService.GetKey(rootKeyID)
+	assert.NotNil(t, rootPubKey)
+
+	// add a delegation first so it can be removed
+	assert.NoError(t, repo.AddDelegation("targets/a", []data.PublicKey{rootPubKey}, []string{"abc,123,xyz,path"}))
+	changes := getChanges(t, repo)
+	assert.Len(t, changes, 2)
+	assert.NoError(t, applyTargetsChange(repo.tufRepo, changes[0]))
+	assert.NoError(t, applyTargetsChange(repo.tufRepo, changes[1]))
+
+	// now clear paths it
+	assert.NoError(t, repo.ClearDelegationPaths("targets/a"))
+	changes = getChanges(t, repo)
+	assert.Len(t, changes, 3)
+	assert.NoError(t, applyTargetsChange(repo.tufRepo, changes[2]))
+
+	delgRoles := repo.tufRepo.Targets[data.CanonicalTargetsRole].Signed.Delegations.Roles
+	assert.Len(t, delgRoles, 1)
+	assert.Len(t, delgRoles[0].Paths, 0)
+}
+
+// TestFullAddDelegationChangefileApplicable generates a single changelist with AddKeys and AddPaths set,
+// (in the old style of AddDelegation) and tests that all of its changes are reflected on publish
+func TestFullAddDelegationChangefileApplicable(t *testing.T) {
+	gun := "docker.com/notary"
+	ts, _, _ := simpleTestServer(t)
+	defer ts.Close()
+
+	repo, rootKeyID := initializeRepo(t, data.ECDSAKey, gun, ts.URL, false)
+	defer os.RemoveAll(repo.baseDir)
+	rootPubKey := repo.CryptoService.GetKey(rootKeyID)
+	assert.NotNil(t, rootPubKey)
+
+	key2, err := repo.CryptoService.Create("user", data.ECDSAKey)
+	assert.NoError(t, err)
+
+	delegationName := "targets/a"
+
+	// manually create the changelist object to load multiple keys
+	tdJSON, err := json.Marshal(&changelist.TufDelegation{
+		NewThreshold: notary.MinThreshold,
+		AddKeys:      data.KeyList([]data.PublicKey{rootPubKey, key2}),
+		AddPaths:     []string{"abc", "123", "xyz"},
+	})
+	change := newCreateDelegationChange(delegationName, tdJSON)
+	cl, err := changelist.NewFileChangelist(filepath.Join(repo.tufRepoPath, "changelist"))
+	addChange(cl, change, delegationName)
+
+	changes := getChanges(t, repo)
+	assert.Len(t, changes, 1)
+	assert.NoError(t, applyTargetsChange(repo.tufRepo, changes[0]))
+
+	delgRoles := repo.tufRepo.Targets[data.CanonicalTargetsRole].Signed.Delegations.Roles
+	assert.Len(t, delgRoles, 1)
+	assert.Len(t, delgRoles[0].Paths, 3)
+	assert.Len(t, delgRoles[0].KeyIDs, 2)
+	assert.Equal(t, delgRoles[0].Name, delegationName)
+}
+
+// TestFullRemoveDelegationChangefileApplicable generates a single changelist with RemoveKeys and RemovePaths set,
+// (in the old style of RemoveDelegation) and tests that all of its changes are reflected on publish
+func TestFullRemoveDelegationChangefileApplicable(t *testing.T) {
+	gun := "docker.com/notary"
+	ts, _, _ := simpleTestServer(t)
+	defer ts.Close()
+
+	repo, rootKeyID := initializeRepo(t, data.ECDSAKey, gun, ts.URL, false)
+	defer os.RemoveAll(repo.baseDir)
+	rootPubKey := repo.CryptoService.GetKey(rootKeyID)
+	assert.NotNil(t, rootPubKey)
+
+	key2, err := repo.CryptoService.Create("user", data.ECDSAKey)
+	assert.NoError(t, err)
+	key2CanonicalID, err := utils.CanonicalKeyID(key2)
+	assert.NoError(t, err)
+
+	delegationName := "targets/a"
+
+	assert.NoError(t, repo.AddDelegation(delegationName, []data.PublicKey{rootPubKey, key2}, []string{"abc", "123"}))
+	changes := getChanges(t, repo)
+	assert.Len(t, changes, 2)
+	assert.NoError(t, applyTargetsChange(repo.tufRepo, changes[0]))
+	assert.NoError(t, applyTargetsChange(repo.tufRepo, changes[1]))
+
+	targetRole := repo.tufRepo.Targets[data.CanonicalTargetsRole]
+	assert.Len(t, targetRole.Signed.Delegations.Roles, 1)
+	assert.Len(t, targetRole.Signed.Delegations.Keys, 2)
+
+	// manually create the changelist object to load multiple keys
+	tdJSON, err := json.Marshal(&changelist.TufDelegation{
+		RemoveKeys:  []string{key2CanonicalID},
+		RemovePaths: []string{"abc", "123"},
+	})
+	change := newUpdateDelegationChange(delegationName, tdJSON)
+	cl, err := changelist.NewFileChangelist(filepath.Join(repo.tufRepoPath, "changelist"))
+	addChange(cl, change, delegationName)
+
+	changes = getChanges(t, repo)
+	assert.Len(t, changes, 3)
+	assert.NoError(t, applyTargetsChange(repo.tufRepo, changes[2]))
+
+	delgRoles := repo.tufRepo.Targets[data.CanonicalTargetsRole].Signed.Delegations.Roles
+	assert.Len(t, delgRoles, 1)
+	assert.Len(t, delgRoles[0].Paths, 0)
+	assert.Len(t, delgRoles[0].KeyIDs, 1)
+}
+
 // TestRemoveDelegationErrorWritingChanges expects errors writing a change to
 // file to be propagated.
 func TestRemoveDelegationErrorWritingChanges(t *testing.T) {
 	testErrorWritingChangefiles(t, func(repo *NotaryRepository) error {
-		return repo.RemoveDelegation("targets/a", []string{""}, []string{}, false)
+		return repo.RemoveDelegationKeysAndPaths("targets/a", []string{""}, []string{})
 	})
 }
 
@@ -2859,10 +2979,10 @@ func testPublishTargetsDelgationCanUseUserKeyWithArbitraryRole(t *testing.T, x50
 
 	// owner creates delegations, adds the delegated key to them, and publishes them
 	assert.NoError(t,
-		ownerRepo.AddDelegation("targets/a", 1, []data.PublicKey{aKey}, []string{""}),
+		ownerRepo.AddDelegation("targets/a", []data.PublicKey{aKey}, []string{""}),
 		"error creating delegation")
 	assert.NoError(t,
-		ownerRepo.AddDelegation("targets/a/b", 1, []data.PublicKey{bKey}, []string{""}),
+		ownerRepo.AddDelegation("targets/a/b", []data.PublicKey{bKey}, []string{""}),
 		"error creating delegation")
 
 	assert.NoError(t, ownerRepo.Publish())
@@ -3016,7 +3136,7 @@ func TestListRoles(t *testing.T) {
 	// Create a delegation on the top level
 	aKey := createKey(t, repo, "user", true)
 	assert.NoError(t,
-		repo.AddDelegation("targets/a", 1, []data.PublicKey{aKey}, []string{""}),
+		repo.AddDelegation("targets/a", []data.PublicKey{aKey}, []string{""}),
 		"error creating delegation")
 
 	assert.NoError(t, repo.Publish())
@@ -3053,7 +3173,7 @@ func TestListRoles(t *testing.T) {
 	// Create another delegation, one level further
 	bKey := createKey(t, repo, "user", true)
 	assert.NoError(t,
-		repo.AddDelegation("targets/a/b", 1, []data.PublicKey{bKey}, []string{""}),
+		repo.AddDelegation("targets/a/b", []data.PublicKey{bKey}, []string{""}),
 		"error creating delegation")
 
 	assert.NoError(t, repo.Publish())

client/delegations.go

@@ -0,0 +1,310 @@
+package client
+
+import (
+	"encoding/json"
+	"fmt"
+	"path/filepath"
+
+	"github.com/Sirupsen/logrus"
+	"github.com/docker/notary"
+	"github.com/docker/notary/client/changelist"
+	"github.com/docker/notary/tuf/data"
+	"github.com/docker/notary/tuf/store"
+	"github.com/docker/notary/tuf/utils"
+)
+
+// AddDelegation creates changelist entries to add provided delegation public keys and paths.
+// This method composes AddDelegationRoleAndKeys and AddDelegationPaths (each creates one changelist if called).
+func (r *NotaryRepository) AddDelegation(name string, delegationKeys []data.PublicKey, paths []string) error {
+	if len(delegationKeys) > 0 {
+		err := r.AddDelegationRoleAndKeys(name, delegationKeys)
+		if err != nil {
+			return err
+		}
+	}
+	if len(paths) > 0 {
+		err := r.AddDelegationPaths(name, paths)
+		if err != nil {
+			return err
+		}
+	}
+	return nil
+}
+
+// AddDelegationRoleAndKeys creates a changelist entry to add provided delegation public keys.
+// This method is the simplest way to create a new delegation, because the delegation must have at least
+// one key upon creation to be valid since we will reject the changelist while validating the threshold.
+func (r *NotaryRepository) AddDelegationRoleAndKeys(name string, delegationKeys []data.PublicKey) error {
+
+	if !data.IsDelegation(name) {
+		return data.ErrInvalidRole{Role: name, Reason: "invalid delegation role name"}
+	}
+
+	cl, err := changelist.NewFileChangelist(filepath.Join(r.tufRepoPath, "changelist"))
+	if err != nil {
+		return err
+	}
+	defer cl.Close()
+
+	logrus.Debugf(`Adding delegation "%s" with threshold %d, and %d keys\n`,
+		name, notary.MinThreshold, len(delegationKeys))
+
+	// Defaulting to threshold of 1, since we don't allow for larger thresholds at the moment.
+	tdJSON, err := json.Marshal(&changelist.TufDelegation{
+		NewThreshold: notary.MinThreshold,
+		AddKeys:      data.KeyList(delegationKeys),
+	})
+	if err != nil {
+		return err
+	}
+
+	template := newCreateDelegationChange(name, tdJSON)
+	return addChange(cl, template, name)
+}
+
+// AddDelegationPaths creates a changelist entry to add provided paths to an existing delegation.
+// This method cannot create a new delegation itself because the role must meet the key threshold upon creation.
+func (r *NotaryRepository) AddDelegationPaths(name string, paths []string) error {
+
+	if !data.IsDelegation(name) {
+		return data.ErrInvalidRole{Role: name, Reason: "invalid delegation role name"}
+	}
+
+	cl, err := changelist.NewFileChangelist(filepath.Join(r.tufRepoPath, "changelist"))
+	if err != nil {
+		return err
+	}
+	defer cl.Close()
+
+	logrus.Debugf(`Adding %s paths to delegation %s\n`, paths, name)
+
+	tdJSON, err := json.Marshal(&changelist.TufDelegation{
+		AddPaths: paths,
+	})
+	if err != nil {
+		return err
+	}
+
+	template := newCreateDelegationChange(name, tdJSON)
+	return addChange(cl, template, name)
+}
+
+// RemoveDelegationKeysAndPaths creates changelist entries to remove provided delegation key IDs and paths.
+// This method composes RemoveDelegationPaths and RemoveDelegationKeys (each creates one changelist if called).
+func (r *NotaryRepository) RemoveDelegationKeysAndPaths(name string, keyIDs, paths []string) error {
+	if len(paths) > 0 {
+		err := r.RemoveDelegationPaths(name, paths)
+		if err != nil {
+			return err
+		}
+	}
+	if len(keyIDs) > 0 {
+		err := r.RemoveDelegationKeys(name, keyIDs)
+		if err != nil {
+			return err
+		}
+	}
+	return nil
+}
+
+// RemoveDelegationRole creates a changelist to remove all paths and keys from a role, and delete the role in its entirety.
+func (r *NotaryRepository) RemoveDelegationRole(name string) error {
+
+	if !data.IsDelegation(name) {
+		return data.ErrInvalidRole{Role: name, Reason: "invalid delegation role name"}
+	}
+
+	cl, err := changelist.NewFileChangelist(filepath.Join(r.tufRepoPath, "changelist"))
+	if err != nil {
+		return err
+	}
+	defer cl.Close()
+
+	logrus.Debugf(`Removing delegation "%s"\n`, name)
+
+	template := newDeleteDelegationChange(name, nil)
+	return addChange(cl, template, name)
+}
+
+// RemoveDelegationPaths creates a changelist entry to remove provided paths from an existing delegation.
+func (r *NotaryRepository) RemoveDelegationPaths(name string, paths []string) error {
+
+	if !data.IsDelegation(name) {
+		return data.ErrInvalidRole{Role: name, Reason: "invalid delegation role name"}
+	}
+
+	cl, err := changelist.NewFileChangelist(filepath.Join(r.tufRepoPath, "changelist"))
+	if err != nil {
+		return err
+	}
+	defer cl.Close()
+
+	logrus.Debugf(`Removing %s paths from delegation "%s"\n`, paths, name)
+
+	tdJSON, err := json.Marshal(&changelist.TufDelegation{
+		RemovePaths: paths,
+	})
+	if err != nil {
+		return err
+	}
+
+	template := newUpdateDelegationChange(name, tdJSON)
+	return addChange(cl, template, name)
+}
+
+// RemoveDelegationKeys creates a changelist entry to remove provided keys from an existing delegation.
+// When this changelist is applied, if the specified keys are the only keys left in the role,
+// the role itself will be deleted in its entirety.
+func (r *NotaryRepository) RemoveDelegationKeys(name string, keyIDs []string) error {
+
+	if !data.IsDelegation(name) {
+		return data.ErrInvalidRole{Role: name, Reason: "invalid delegation role name"}
+	}
+
+	cl, err := changelist.NewFileChangelist(filepath.Join(r.tufRepoPath, "changelist"))
+	if err != nil {
+		return err
+	}
+	defer cl.Close()
+
+	logrus.Debugf(`Removing %s keys from delegation "%s"\n`, keyIDs, name)
+
+	tdJSON, err := json.Marshal(&changelist.TufDelegation{
+		RemoveKeys: keyIDs,
+	})
+	if err != nil {
+		return err
+	}
+
+	template := newUpdateDelegationChange(name, tdJSON)
+	return addChange(cl, template, name)
+}
+
+// ClearDelegationPaths creates a changelist entry to remove all paths from an existing delegation.
+func (r *NotaryRepository) ClearDelegationPaths(name string) error {
+
+	if !data.IsDelegation(name) {
+		return data.ErrInvalidRole{Role: name, Reason: "invalid delegation role name"}
+	}
+
+	cl, err := changelist.NewFileChangelist(filepath.Join(r.tufRepoPath, "changelist"))
+	if err != nil {
+		return err
+	}
+	defer cl.Close()
+
+	logrus.Debugf(`Removing all paths from delegation "%s"\n`, name)
+
+	tdJSON, err := json.Marshal(&changelist.TufDelegation{
+		ClearAllPaths: true,
+	})
+	if err != nil {
+		return err
+	}
+
+	template := newUpdateDelegationChange(name, tdJSON)
+	return addChange(cl, template, name)
+}
+
+func newUpdateDelegationChange(name string, content []byte) *changelist.TufChange {
+	return changelist.NewTufChange(
+		changelist.ActionUpdate,
+		name,
+		changelist.TypeTargetsDelegation,
+		"", // no path for delegations
+		content,
+	)
+}
+
+func newCreateDelegationChange(name string, content []byte) *changelist.TufChange {
+	return changelist.NewTufChange(
+		changelist.ActionCreate,
+		name,
+		changelist.TypeTargetsDelegation,
+		"", // no path for delegations
+		content,
+	)
+}
+
+func newDeleteDelegationChange(name string, content []byte) *changelist.TufChange {
+	return changelist.NewTufChange(
+		changelist.ActionDelete,
+		name,
+		changelist.TypeTargetsDelegation,
+		"", // no path for delegations
+		content,
+	)
+}
+
+// GetDelegationRoles returns the keys and roles of the repository's delegations
+// Also converts key IDs to canonical key IDs to keep consistent with signing prompts
+func (r *NotaryRepository) GetDelegationRoles() ([]*data.Role, error) {
+	// Update state of the repo to latest
+	if _, err := r.Update(false); err != nil {
+		return nil, err
+	}
+
+	// All top level delegations (ex: targets/level1) are stored exclusively in targets.json
+	targets, ok := r.tufRepo.Targets[data.CanonicalTargetsRole]
+	if !ok {
+		return nil, store.ErrMetaNotFound{Resource: data.CanonicalTargetsRole}
+	}
+
+	// make a copy of top-level Delegations and only show canonical key IDs
+	allDelegations, err := translateDelegationsToCanonicalIDs(targets.Signed.Delegations)
+	if err != nil {
+		return nil, err
+	}
+
+	// make a copy for traversing nested delegations
+	delegationsList := make([]*data.Role, len(allDelegations))
+	copy(delegationsList, allDelegations)
+
+	// Now traverse to lower level delegations (ex: targets/level1/level2)
+	for len(delegationsList) > 0 {
+		// Pop off first delegation to traverse
+		delegation := delegationsList[0]
+		delegationsList = delegationsList[1:]
+
+		// Get metadata
+		delegationMeta, ok := r.tufRepo.Targets[delegation.Name]
+		// If we get an error, don't try to traverse further into this subtree because it doesn't exist or is malformed
+		if !ok {
+			continue
+		}
+
+		// For the return list, update with a copy that includes canonicalKeyIDs
+		canonicalDelegations, err := translateDelegationsToCanonicalIDs(delegationMeta.Signed.Delegations)
+		if err != nil {
+			return nil, err
+		}
+		allDelegations = append(allDelegations, canonicalDelegations...)
+		// Add nested delegations to the exploration list
+		delegationsList = append(delegationsList, delegationMeta.Signed.Delegations.Roles...)
+	}
+
+	// Convert all key IDs to canonical IDs:
+	return allDelegations, nil
+}
+
+func translateDelegationsToCanonicalIDs(delegationInfo data.Delegations) ([]*data.Role, error) {
+	canonicalDelegations := make([]*data.Role, len(delegationInfo.Roles))
+	copy(canonicalDelegations, delegationInfo.Roles)
+	delegationKeys := delegationInfo.Keys
+	for i, delegation := range canonicalDelegations {
+		canonicalKeyIDs := []string{}
+		for _, keyID := range delegation.KeyIDs {
+			pubKey, ok := delegationKeys[keyID]
+			if !ok {
+				return nil, fmt.Errorf("Could not translate canonical key IDs for %s", delegation.Name)
+			}
+			canonicalKeyID, err := utils.CanonicalKeyID(pubKey)
+			if err != nil {
+				return nil, fmt.Errorf("Could not translate canonical key IDs for %s: %v", delegation.Name, err)
+			}
+			canonicalKeyIDs = append(canonicalKeyIDs, canonicalKeyID)
+		}
+		canonicalDelegations[i].KeyIDs = canonicalKeyIDs
+	}
+	return canonicalDelegations, nil
+}

client/helpers.go

@@ -136,8 +136,14 @@ func changeTargetsDelegation(repo *tuf.Repo, c changelist.Change) error {
 		if err := r.AddPathHashPrefixes(td.AddPathHashPrefixes); err != nil {
 			return err
 		}
+
+		// Clear all paths if we're given the flag, else remove specified paths
+		if td.ClearAllPaths {
+			r.RemovePaths(r.Paths)
+		} else {
+			r.RemovePaths(td.RemovePaths)
+		}
 		r.RemoveKeys(removeTUFKeyIDs)
-		r.RemovePaths(td.RemovePaths)
 		r.RemovePathHashPrefixes(td.RemovePathHashPrefixes)
 		return repo.UpdateDelegations(r, td.AddKeys)
 	case changelist.ActionDelete:

cmd/notary/delegations.go

@@ -4,7 +4,6 @@ import (
 	"fmt"
 	"io/ioutil"
 
-	"github.com/docker/notary"
 	notaryclient "github.com/docker/notary/client"
 	"github.com/docker/notary/passphrase"
 	"github.com/docker/notary/trustmanager"
@@ -139,13 +138,19 @@ func (d *delegationCommander) delegationRemove(cmd *cobra.Command, args []string
 		} else {
 			cmd.Println("Confirmed `yes` from flag")
 		}
+		// Delete the entire delegation
+		err = nRepo.RemoveDelegationRole(role)
+		if err != nil {
+			return fmt.Errorf("failed to remove delegation: %v", err)
+		}
+	} else {
+		// Remove any keys or paths that we passed in
+		err = nRepo.RemoveDelegationKeysAndPaths(role, keyIDs, paths)
+		if err != nil {
+			return fmt.Errorf("failed to remove delegation: %v", err)
+		}
 	}
 
-	// Remove the delegation from the repository
-	err = nRepo.RemoveDelegation(role, keyIDs, paths, removeAll)
-	if err != nil {
-		return fmt.Errorf("failed to remove delegation: %v", err)
-	}
 	cmd.Println("")
 	if removeAll {
 		cmd.Printf("Forced removal (including all keys and paths) of delegation role %s to repository \"%s\" staged for next publish.\n", role, gun)
@@ -197,9 +202,7 @@ func (d *delegationCommander) delegationAdd(cmd *cobra.Command, args []string) e
 	}
 
 	// Add the delegation to the repository
-	// Sets threshold to 1 since we only added one key - thresholds are not currently fully supported, though
+	err = nRepo.AddDelegation(role, pubKeys, paths)
-	// one can use additional client-side validation to check for signatures from a quorum of varying delegation roles
-	err = nRepo.AddDelegation(role, notary.MinThreshold, pubKeys, paths)
 	if err != nil {
 		return fmt.Errorf("failed to create delegation: %v", err)
 	}