Add Delete method to accessors (#21)

Author: chlowell

cache/accessor/accessor.go

@@ -7,6 +7,7 @@ import "context"
 
 // Accessor accesses data storage.
 type Accessor interface {
+	Delete(context.Context) error
 	Read(context.Context) ([]byte, error)
 	Write(context.Context, []byte) error
 }

cache/accessor/darwin.go

@@ -43,6 +43,15 @@ func New(servName string, opts ...option) (*Storage, error) {
 	return &s, nil
 }
 
+// Delete deletes the stored data, if any exists.
+func (s *Storage) Delete(context.Context) error {
+	err := keychain.DeleteGenericPasswordItem(s.service, s.account)
+	if errors.Is(err, keychain.ErrorItemNotFound) || errors.Is(err, keychain.ErrorNoSuchKeychain) {
+		return nil
+	}
+	return err
+}
+
 // Read returns data stored on the keychain or, if the keychain item doesn't exist, a nil slice and nil error.
 func (s *Storage) Read(context.Context) ([]byte, error) {
 	data, err := keychain.GetGenericPassword(s.service, s.account, "", "")
@@ -70,3 +79,5 @@ func (s *Storage) Write(_ context.Context, data []byte) error {
 	}
 	return err
 }
+
+var _ Accessor = (*Storage)(nil)

cache/accessor/darwin_test.go

@@ -13,6 +13,9 @@ import (
 )
 
 func TestWithAccount(t *testing.T) {
+	if !manualTests {
+		t.Skipf("set %s to run this test", msalextManualTest)
+	}
 	account := "account"
 	a, err := New(t.Name(), WithAccount(account))
 	require.NoError(t, err)
@@ -24,4 +27,6 @@ func TestWithAccount(t *testing.T) {
 	actual, err := a.Read(ctx)
 	require.NoError(t, err)
 	require.Equal(t, expected, actual)
+
+	require.NoError(t, a.Delete(ctx))
 }

cache/accessor/file/file.go

@@ -24,6 +24,17 @@ func New(p string) (*Storage, error) {
 	return &Storage{m: &sync.RWMutex{}, p: p}, nil
 }
 
+// Delete deletes the file, if it exists.
+func (s *Storage) Delete(context.Context) error {
+	s.m.Lock()
+	defer s.m.Unlock()
+	err := os.Remove(s.p)
+	if errors.Is(err, os.ErrNotExist) {
+		return nil
+	}
+	return err
+}
+
 // Read returns the file's content or, if the file doesn't exist, a nil slice and error.
 func (s *Storage) Read(context.Context) ([]byte, error) {
 	s.m.RLock()

cache/accessor/file/file_test.go

@@ -14,57 +14,43 @@ import (
 
 var ctx = context.Background()
 
-func TestRead(t *testing.T) {
-	p := filepath.Join(t.TempDir(), t.Name())
-	a, err := New(p)
-	require.NoError(t, err)
-
-	expected := []byte("expected")
-	require.NoError(t, os.WriteFile(p, expected, 0600))
-
-	actual, err := a.Read(ctx)
-	require.NoError(t, err)
-	require.Equal(t, expected, actual)
-}
-
-func TestRoundTrip(t *testing.T) {
-	p := filepath.Join(t.TempDir(), "nonexistent", t.Name())
-	a, err := New(p)
-	require.NoError(t, err)
-
-	var expected []byte
-	for i := 0; i < 4; i++ {
-		actual, err := a.Read(ctx)
-		require.NoError(t, err)
-		require.Equal(t, expected, actual)
-
-		expected = append(expected, byte(i))
-		require.NoError(t, a.Write(ctx, expected))
-	}
-}
-
-func TestWrite(t *testing.T) {
-	p := filepath.Join(t.TempDir(), t.Name())
-	for _, create := range []bool{true, false} {
-		name := "file exists"
-		if create {
-			name = "new file"
-		}
-		t.Run(name, func(t *testing.T) {
-			if create {
-				f, err := os.OpenFile(p, os.O_CREATE|os.O_EXCL, 0600)
+func TestReadWriteDelete(t *testing.T) {
+	for _, test := range []struct {
+		desc              string
+		initialData, want []byte
+	}{
+		{desc: "Test when the file exists", initialData: []byte("data"), want: []byte("want")},
+		{desc: "Test when the file doesn't exist", want: []byte("want")},
+	} {
+		t.Run(test.desc, func(t *testing.T) {
+			p := filepath.Join(t.TempDir(), t.Name())
+			if test.initialData != nil {
+				require.NoError(t, os.MkdirAll(filepath.Dir(p), 0700))
+				f, err := os.OpenFile(p, os.O_CREATE|os.O_EXCL|os.O_RDWR, 0600)
+				require.NoError(t, err)
+				_, err = f.Write(test.initialData)
 				require.NoError(t, err)
 				require.NoError(t, f.Close())
 			}
 			a, err := New(p)
 			require.NoError(t, err)
 
-			expected := []byte("expected")
-			require.NoError(t, a.Write(ctx, expected))
+			if test.initialData != nil {
+				actual, err := a.Read(ctx)
+				require.NoError(t, err)
+				require.Equal(t, test.initialData, actual)
+			}
 
-			actual, err := os.ReadFile(p)
+			cp := make([]byte, len(test.want))
+			copy(cp, test.want)
+			err = a.Write(ctx, cp)
 			require.NoError(t, err)
-			require.Equal(t, expected, actual)
+
+			actual, err := a.Read(ctx)
+			require.NoError(t, err)
+			require.Equal(t, test.want, actual)
+
+			require.NoError(t, a.Delete(context.Background()))
 		})
 	}
 }

cache/accessor/linux.go

@@ -67,6 +67,16 @@ void free_g_error(void *f, gError *err)
     fn(err);
 }
 
+// clear (delete) a secret. f must be a pointer to secret_password_clear_sync
+// https://gnome.pages.gitlab.gnome.org/libsecret/func.password_clear_sync.html
+int clear(void *f, schema *sch, void* cancellable, gError **err, char *key1, char *value1, char *key2, char *value2)
+{
+    int (*fn)(schema *sch, void* cancellable, gError **err, char *key1, char *value1, char *key2, char *value2, ...);
+    fn = (int (*)(schema *sch, void* cancellable, gError **err, char *key1, char *value1, char *key2, char *value2, ...))f;
+    int r = fn(sch, cancellable, err, key1, value1, key2, value2, NULL);
+    return r;
+}
+
 // lookup a password. f must be a pointer to secret_password_lookup_sync
 // https://gnome.pages.gitlab.gnome.org/libsecret/func.password_lookup_sync.html
 char *lookup(void *f, schema *sch, void* cancellable, gError **err, char *key1, char *value1, char *key2, char *value2)
@@ -137,8 +147,8 @@ type Storage struct {
 	handle unsafe.Pointer
 	// label of the secret schema
 	label string
-	// freeError, lookup and store are the addresses of libsecret functions
-	freeError, lookup, store unsafe.Pointer
+	// clear, freeError, lookup and store are the addresses of libsecret functions
+	clear, freeError, lookup, store unsafe.Pointer
 	// schema identifies the cached data in the secret service
 	schema *C.schema
 }
@@ -179,6 +189,10 @@ func New(name string, opts ...option) (*Storage, error) {
 		}
 	})
 
+	clear, err := s.symbol("secret_password_clear_sync")
+	if err != nil {
+		return nil, err
+	}
 	freeError, err := s.symbol("g_error_free")
 	if err != nil {
 		return nil, err
@@ -191,6 +205,7 @@ func New(name string, opts ...option) (*Storage, error) {
 	if err != nil {
 		return nil, err
 	}
+	s.clear = clear
 	s.freeError = freeError
 	s.lookup = lookup
 	s.store = store
@@ -205,6 +220,27 @@ func New(name string, opts ...option) (*Storage, error) {
 	return &s, nil
 }
 
+// Delete deletes the stored data, if any exists.
+func (s *Storage) Delete(context.Context) error {
+	// the first nil terminates the list and libsecret ignores any extras
+	attrs := []*C.char{nil, nil, nil, nil}
+	for i, attr := range s.attributes {
+		name := C.CString(attr.name)
+		defer C.free(unsafe.Pointer(name))
+		value := C.CString(attr.value)
+		defer C.free(unsafe.Pointer(value))
+		attrs[i*2] = name
+		attrs[(i*2)+1] = value
+	}
+	var e *C.gError
+	_ = C.clear(s.clear, s.schema, nil, &e, attrs[0], attrs[1], attrs[2], attrs[3])
+	if e != nil {
+		defer C.free_g_error(s.freeError, e)
+		return fmt.Errorf("couldn't delete cache data: %q", C.GoString(e.message))
+	}
+	return nil
+}
+
 // Read returns data stored according to the secret schema or, if no such data exists, a nil slice and nil error.
 func (s *Storage) Read(context.Context) ([]byte, error) {
 	// the first nil terminates the list and libsecret ignores any extras
@@ -274,3 +310,5 @@ func (s *Storage) symbol(name string) (unsafe.Pointer, error) {
 	}
 	return fp, nil
 }
+
+var _ Accessor = (*Storage)(nil)

cache/accessor/storage_test.go

@@ -25,7 +25,7 @@ var (
 	manualTests = runtime.GOOS == "windows" || os.Getenv(msalextManualTest) != ""
 )
 
-func TestReadWrite(t *testing.T) {
+func TestReadWriteDelete(t *testing.T) {
 	if !manualTests {
 		t.Skipf("set %s to run this test", msalextManualTest)
 	}
@@ -51,6 +51,8 @@ func TestReadWrite(t *testing.T) {
 			actual, err := a.Read(ctx)
 			require.NoError(t, err)
 			require.Equal(t, test.want, actual)
+
+			require.NoError(t, a.Delete(context.Background()))
 		})
 	}
 }

cache/accessor/windows.go

@@ -28,6 +28,17 @@ func New(p string) (*Storage, error) {
 	return &Storage{m: &sync.RWMutex{}, p: p}, nil
 }
 
+// Delete deletes the file, if it exists.
+func (s *Storage) Delete(context.Context) error {
+	s.m.Lock()
+	defer s.m.Unlock()
+	err := os.Remove(s.p)
+	if errors.Is(err, os.ErrNotExist) {
+		return nil
+	}
+	return err
+}
+
 // Read returns data from the file. If the file doesn't exist, Read returns a nil slice and error.
 func (s *Storage) Read(context.Context) ([]byte, error) {
 	s.m.RLock()
@@ -100,3 +111,5 @@ func dpapi(op operation, data []byte) (result []byte, err error) {
 	}
 	return result, err
 }
+
+var _ Accessor = (*Storage)(nil)

cache/cache_test.go

@@ -26,6 +26,11 @@ type fakeExternalCache struct {
 	readCallback, writeCallback func() error
 }
 
+func (c *fakeExternalCache) Delete(context.Context) error {
+	c.data = nil
+	return nil
+}
+
 func (a *fakeExternalCache) Read(context.Context) ([]byte, error) {
 	var err error
 	if a.readCallback != nil {