From fbdb60821abbe7a1187c4afc9a4f29a9490b7775 Mon Sep 17 00:00:00 2001 From: Riyaz Faizullabhoy Date: Thu, 7 Jul 2016 13:59:08 -0700 Subject: [PATCH] Hook up httpstore RemoveAll to delete handler Signed-off-by: Riyaz Faizullabhoy --- client/client.go | 16 ++++++- client/client_test.go | 85 ++++++++++++++++++++++++++++++++++--- tuf/store/httpstore.go | 21 +++++++-- tuf/store/httpstore_test.go | 10 ++--- 4 files changed, 114 insertions(+), 18 deletions(-) diff --git a/client/client.go b/client/client.go index eababee600..76008e0900 100644 --- a/client/client.go +++ b/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 } diff --git a/client/client_test.go b/client/client_test.go index 6daffaafa2..7bcd31d552 100644 --- a/client/client_test.go +++ b/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,81 @@ 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) +} + type brokenRemoveFilestore struct { store.MetadataStore } @@ -3301,8 +3376,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) } diff --git a/tuf/store/httpstore.go b/tuf/store/httpstore.go index a925786616..3fbddfc034 100644 --- a/tuf/store/httpstore.go +++ b/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) { diff --git a/tuf/store/httpstore_test.go b/tuf/store/httpstore_test.go index 705a6a09cc..1a3d58a286 100644 --- a/tuf/store/httpstore_test.go +++ b/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) {