Hook up httpstore RemoveAll to delete handler

Signed-off-by: Riyaz Faizullabhoy <[email protected]>

client/client.go

@@ -953,11 +953,23 @@ func (r *NotaryRepository) rootFileKeyChange(cl changelist.Changelist, role, act
 }
 
 // DeleteTrustData removes the trust data stored for this repo in the TUF cache on the client side
-func (r *NotaryRepository) DeleteTrustData() error {
-	// Clear TUF files and cache
+// Note that we will not delete any private key material from local storage
+func (r *NotaryRepository) DeleteTrustData(deleteRemote bool) error {
+	// Clear local TUF files and cache
 	if err := r.fileStore.RemoveAll(); err != nil {
 		return fmt.Errorf("error clearing TUF repo data: %v", err)
 	}
 	r.tufRepo = tuf.NewRepo(nil)
+
+	// Note that this will require admin permission in this NotaryRepository's roundtripper
+	if deleteRemote {
+		remote, err := getRemoteStore(r.baseURL, r.gun, r.roundTrip)
+		if err != nil {
+			return err
+		}
+		if err := remote.RemoveAll(); err != nil {
+			return err
+		}
+	}
 	return nil
 }

client/client_test.go

@@ -3244,7 +3244,7 @@ func testPublishTargetsDelegationCanUseUserKeyWithArbitraryRole(t *testing.T, x5
 	delgRec.requireAsked(t, []string{"targets/a/b"})
 }
 
-// TestDeleteRepo tests that local repo data, certificate, and keys are deleted from the client library call
+// TestDeleteRepo tests that local repo data is deleted from the client library call
 func TestDeleteRepo(t *testing.T) {
 	gun := "docker.com/notary"
 
@@ -3260,8 +3260,8 @@ func TestDeleteRepo(t *testing.T) {
 	requireRepoHasExpectedMetadata(t, repo, data.CanonicalTargetsRole, true)
 	requireRepoHasExpectedMetadata(t, repo, data.CanonicalSnapshotRole, true)
 
-	// Delete all client trust data for repo
-	err := repo.DeleteTrustData()
+	// Delete all local trust data for repo
+	err := repo.DeleteTrustData(false)
 	require.NoError(t, err)
 
 	// Assert no metadata for this repo exists locally
@@ -3274,6 +3274,85 @@ func TestDeleteRepo(t *testing.T) {
 	requireRepoHasExpectedKeys(t, repo, rootKeyID, true)
 }
 
+// TestDeleteRemoteRepo tests that local and remote repo data is deleted from the client library call
+func TestDeleteRemoteRepo(t *testing.T) {
+	gun := "docker.com/notary"
+
+	ts := fullTestServer(t)
+	defer ts.Close()
+
+	// Create and publish a repo to delete
+	repo, rootKeyID := initializeRepo(t, data.ECDSAKey, gun, ts.URL, false)
+	defer os.RemoveAll(repo.baseDir)
+
+	require.NoError(t, repo.Publish())
+
+	// Create another repo to ensure it stays intact
+	livingGun := "stayingAlive"
+	longLivingRepo, _ := initializeRepo(t, data.ECDSAKey, livingGun, ts.URL, false)
+	defer os.RemoveAll(longLivingRepo.baseDir)
+
+	require.NoError(t, longLivingRepo.Publish())
+
+	// Assert initialization was successful before we delete
+	requireRepoHasExpectedKeys(t, repo, rootKeyID, true)
+	requireRepoHasExpectedMetadata(t, repo, data.CanonicalRootRole, true)
+	requireRepoHasExpectedMetadata(t, repo, data.CanonicalTargetsRole, true)
+	requireRepoHasExpectedMetadata(t, repo, data.CanonicalSnapshotRole, true)
+
+	// Delete all local and remote trust data for one repo
+	err := repo.DeleteTrustData(true)
+	require.NoError(t, err)
+
+	// Assert no metadata for that repo exists locally
+	requireRepoHasExpectedMetadata(t, repo, data.CanonicalRootRole, false)
+	requireRepoHasExpectedMetadata(t, repo, data.CanonicalTargetsRole, false)
+	requireRepoHasExpectedMetadata(t, repo, data.CanonicalSnapshotRole, false)
+	requireRepoHasExpectedMetadata(t, repo, data.CanonicalTimestampRole, false)
+
+	// Assert keys for this repo still exist locally
+	requireRepoHasExpectedKeys(t, repo, rootKeyID, true)
+
+	// Try connecting to the remote store directly and make sure that no metadata exists for this gun
+	remoteStore, err := getRemoteStore(repo.baseURL, repo.gun, repo.roundTrip)
+	require.NoError(t, err)
+	meta, err := remoteStore.GetMeta(data.CanonicalRootRole, store.NoSizeLimit)
+	require.Error(t, err)
+	require.Nil(t, meta)
+	meta, err = remoteStore.GetMeta(data.CanonicalTargetsRole, store.NoSizeLimit)
+	require.Error(t, err)
+	require.Nil(t, meta)
+	meta, err = remoteStore.GetMeta(data.CanonicalSnapshotRole, store.NoSizeLimit)
+	require.Error(t, err)
+	require.Nil(t, meta)
+	meta, err = remoteStore.GetMeta(data.CanonicalTimestampRole, store.NoSizeLimit)
+	require.Error(t, err)
+	require.Nil(t, meta)
+
+	// Check that the other repo was unaffected
+	requireRepoHasExpectedMetadata(t, longLivingRepo, data.CanonicalRootRole, true)
+	requireRepoHasExpectedMetadata(t, longLivingRepo, data.CanonicalTargetsRole, true)
+	requireRepoHasExpectedMetadata(t, longLivingRepo, data.CanonicalSnapshotRole, true)
+	remoteStore, err = getRemoteStore(longLivingRepo.baseURL, longLivingRepo.gun, longLivingRepo.roundTrip)
+	require.NoError(t, err)
+	meta, err = remoteStore.GetMeta(data.CanonicalRootRole, store.NoSizeLimit)
+	require.NoError(t, err)
+	require.NotNil(t, meta)
+	meta, err = remoteStore.GetMeta(data.CanonicalTargetsRole, store.NoSizeLimit)
+	require.NoError(t, err)
+	require.NotNil(t, meta)
+	meta, err = remoteStore.GetMeta(data.CanonicalSnapshotRole, store.NoSizeLimit)
+	require.NoError(t, err)
+	require.NotNil(t, meta)
+	meta, err = remoteStore.GetMeta(data.CanonicalTimestampRole, store.NoSizeLimit)
+	require.NoError(t, err)
+	require.NotNil(t, meta)
+
+	// Try deleting again with an invalid server URL
+	repo.baseURL = "invalid"
+	require.Error(t, repo.DeleteTrustData(true))
+}
+
 type brokenRemoveFilestore struct {
 	store.MetadataStore
 }
@@ -3301,8 +3380,8 @@ func TestDeleteRepoBadFilestore(t *testing.T) {
 	// Make the filestore faulty on remove
 	repo.fileStore = &brokenRemoveFilestore{repo.fileStore}
 
-	// Delete all client trust data for repo, require an error on the filestore removal
-	err := repo.DeleteTrustData()
+	// Delete all local trust data for repo, require an error on the filestore removal
+	err := repo.DeleteTrustData(false)
 	require.Error(t, err)
 }
 

tuf/store/httpstore.go

@@ -193,9 +193,9 @@ func (s HTTPStore) SetMeta(name string, blob []byte) error {
 }
 
 // RemoveMeta always fails, because we should never be able to delete metadata
-// remotely
+// for individual TUF metadata remotely
 func (s HTTPStore) RemoveMeta(name string) error {
-	return ErrInvalidOperation{msg: "cannot delete metadata"}
+	return ErrInvalidOperation{msg: "cannot delete individual metadata files"}
 }
 
 // NewMultiPartMetaRequest builds a request with the provided metadata updates
@@ -243,9 +243,22 @@ func (s HTTPStore) SetMultiMeta(metas map[string][]byte) error {
 	return translateStatusToError(resp, "POST metadata endpoint")
 }
 
-// RemoveAll in the interface is not supported, admins should use the DeleteHandler endpoint directly to delete remote data for a GUN
+// RemoveAll will attempt to delete all TUF metadata for a GUN
 func (s HTTPStore) RemoveAll() error {
-	return errors.New("remove all functionality not supported for HTTPStore")
+	url, err := s.buildMetaURL("")
+	if err != nil {
+		return err
+	}
+	req, err := http.NewRequest("DELETE", url.String(), nil)
+	if err != nil {
+		return err
+	}
+	resp, err := s.roundTrip.RoundTrip(req)
+	if err != nil {
+		return err
+	}
+	defer resp.Body.Close()
+	return translateStatusToError(resp, "DELETE metadata for GUN endpoint")
 }
 
 func (s HTTPStore) buildMetaURL(name string) (*url.URL, error) {

tuf/store/httpstore_test.go

@@ -292,19 +292,15 @@ func TestTranslateErrorsWhenCannotParse400(t *testing.T) {
 }
 
 func TestHTTPStoreRemoveAll(t *testing.T) {
-	// Set up a simple handler and server for our store
-	handler := func(w http.ResponseWriter, r *http.Request) {
-		w.Write([]byte(testRoot))
-	}
+	// Set up a simple handler and server for our store, just check that a non-error response back is fine
+	handler := func(w http.ResponseWriter, r *http.Request) {}
 	server := httptest.NewServer(http.HandlerFunc(handler))
 	defer server.Close()
 	store, err := NewHTTPStore(server.URL, "metadata", "json", "key", http.DefaultTransport)
 	require.NoError(t, err)
 
-	// currently unsupported since there is no use case
-	// check for the error
 	err = store.RemoveAll()
-	require.Error(t, err)
+	require.NoError(t, err)
 }
 
 func TestHTTPOffline(t *testing.T) {