From 0767e9c0c15c7fc9ecca6595d07cfc238cf01407 Mon Sep 17 00:00:00 2001 From: brent s Date: Mon, 13 Dec 2021 05:34:53 -0500 Subject: [PATCH] fixing various race conditions and errors after refactoring --- collection_funcs.go | 65 ++++++++++++++++--------------------------- consts.go | 9 ++++++ item_funcs.go | 50 ++++++++------------------------- item_funcs_test.go | 3 ++ service_funcs.go | 31 +++++++++++++-------- service_funcs_test.go | 8 +++++- 6 files changed, 75 insertions(+), 91 deletions(-) diff --git a/collection_funcs.go b/collection_funcs.go index 2444745..b9a740d 100644 --- a/collection_funcs.go +++ b/collection_funcs.go @@ -81,15 +81,14 @@ func (c *Collection) CreateItem(label string, attrs map[string]string, secret *S props[DbusItemLabel] = dbus.MakeVariant(label) props[DbusItemType] = dbus.MakeVariant(typeString) props[DbusItemAttributes] = dbus.MakeVariant(attrs) + props[DbusItemCreated] = dbus.MakeVariant(uint64(time.Now().Unix())) + // props[DbusItemModified] = dbus.MakeVariant(uint64(time.Now().Unix())) if err = c.Dbus.Call( DbusCollectionCreateItem, 0, props, secret, replace, ).Store(&path, &promptPath); err != nil { return } - if err = c.setModify(); err != nil { - return - } if isPrompt(promptPath) { prompt = NewPrompt(c.Conn, promptPath) @@ -102,9 +101,6 @@ func (c *Collection) CreateItem(label string, attrs map[string]string, secret *S } item, err = NewItem(c, path) - if err = item.setCreate(); err != nil { - return - } return } @@ -196,6 +192,10 @@ func (c *Collection) Lock() (err error) { } c.IsLocked = true + if _, _, err = c.Modified(); err != nil { + return + } + return } @@ -225,6 +225,10 @@ func (c *Collection) Relabel(newLabel string) (err error) { } c.LabelName = newLabel + if _, _, err = c.Modified(); err != nil { + return + } + return } @@ -273,7 +277,15 @@ func (c *Collection) SetAlias(alias string) (err error) { DbusServiceSetAlias, 0, alias, c.Dbus.Path(), ) - err = call.Err + if err = call.Err; err != nil { + return + } + + c.Alias = alias + + if _, _, err = c.Modified(); err != nil { + return + } return } @@ -293,6 +305,10 @@ func (c *Collection) Unlock() (err error) { } c.IsLocked = false + if _, _, err = c.Modified(); err != nil { + return + } + return } @@ -309,6 +325,7 @@ func (c *Collection) Created() (created time.Time, err error) { timeInt = variant.Value().(uint64) created = time.Unix(int64(timeInt), 0) + c.CreatedAt = created return } @@ -346,40 +363,6 @@ func (c *Collection) Modified() (modified time.Time, isChanged bool, err error) return } -/* - setCreate updates the Collection's creation time (as specified by Collection.Created). - It seems that this does not generate automatically. -*/ -func (c *Collection) setCreate() (err error) { - - var t time.Time = time.Now() - - if err = c.Dbus.SetProperty(DbusCollectionCreated, uint64(t.Unix())); err != nil { - return - } - c.CreatedAt = t - - if err = c.setModify(); err != nil { - return - } - - return -} - -/* - setModify updates the Collection's modification time (as specified by Collection.Modified). - It seems that this does not update automatically. -*/ -func (c *Collection) setModify() (err error) { - - var t time.Time = time.Now() - - err = c.Dbus.SetProperty(DbusCollectionModified, uint64(t.Unix())) - c.LastModified = t - - return -} - // path is a *very* thin wrapper around Collection.Dbus.Path(). It is needed for LockableObject interface membership. func (c *Collection) path() (dbusPath dbus.ObjectPath) { diff --git a/consts.go b/consts.go index 61a7a66..9fc11a8 100644 --- a/consts.go +++ b/consts.go @@ -1,5 +1,9 @@ package gosecret +import ( + `github.com/godbus/dbus/v5` +) + // Constants for use with gosecret. const ( /* @@ -24,6 +28,11 @@ const ( DbusDefaultItemType string = DbusServiceBase + ".Generic" ) +// Libsecret/SecretService special values. +var ( + DbusRemoveAliasPath dbus.ObjectPath = dbus.ObjectPath("/") +) + // Service interface. const ( /* diff --git a/item_funcs.go b/item_funcs.go index 0532618..143b5c0 100644 --- a/item_funcs.go +++ b/item_funcs.go @@ -96,7 +96,7 @@ func (i *Item) ChangeItemType(newItemType string) (err error) { } i.SecretType = newItemType - if err = i.setModify(); err != nil { + if _, _, err = i.Modified(); err != nil { return } @@ -216,7 +216,7 @@ func (i *Item) Relabel(newLabel string) (err error) { } i.LabelName = newLabel - if err = i.setModify(); err != nil { + if _, _, err = i.Modified(); err != nil { return } @@ -235,7 +235,7 @@ func (i *Item) ReplaceAttributes(newAttrs map[string]string) (err error) { } i.Attrs = newAttrs - if err = i.setModify(); err != nil { + if _, _, err = i.Modified(); err != nil { return } @@ -256,7 +256,7 @@ func (i *Item) SetSecret(secret *Secret) (err error) { } i.Secret = secret - if err = i.setModify(); err != nil { + if _, _, err = i.Modified(); err != nil { return } @@ -293,6 +293,10 @@ func (i *Item) Lock() (err error) { } i.IsLocked = true + if _, _, err = i.Modified(); err != nil { + return + } + return } @@ -327,6 +331,10 @@ func (i *Item) Unlock() (err error) { } i.IsLocked = false + if _, _, err = i.Modified(); err != nil { + return + } + return } @@ -381,40 +389,6 @@ func (i *Item) Modified() (modified time.Time, isChanged bool, err error) { return } -/* - setCreate updates the Item's creation time (as specified by Item.Created). - It seems that this does not generate automatically. -*/ -func (i *Item) setCreate() (err error) { - - var t time.Time = time.Now() - - if err = i.Dbus.SetProperty(DbusItemCreated, uint64(t.Unix())); err != nil { - return - } - i.CreatedAt = t - - if err = i.setModify(); err != nil { - return - } - - return -} - -/* - setModify updates the Item's modification time (as specified by Item.Modified). - It seems that this does not update automatically. -*/ -func (i *Item) setModify() (err error) { - - var t time.Time = time.Now() - - err = i.Dbus.SetProperty(DbusItemModified, uint64(t.Unix())) - i.LastModified = t - - return -} - // path is a *very* thin wrapper around Item.Dbus.Path(). It is needed for LockableObject membership. func (i *Item) path() (dbusPath dbus.ObjectPath) { diff --git a/item_funcs_test.go b/item_funcs_test.go index af3c0d0..dc042af 100644 --- a/item_funcs_test.go +++ b/item_funcs_test.go @@ -46,6 +46,9 @@ func TestItem(t *testing.T) { if item, err = collection.CreateItem(testItemLabel, itemAttrs, secret, true); err != nil { t.Errorf("could not create item %v in collection '%v': %v", testItemLabel, collectionName.String(), err.Error()) + if err = collection.Delete(); err != nil { + t.Errorf("could not delete collection '%v': %v", collectionName.String(), err.Error()) + } if err = svc.Close(); err != nil { t.Fatalf("could not close Service.Session: %v", err.Error()) } diff --git a/service_funcs.go b/service_funcs.go index d28cecd..85c986c 100644 --- a/service_funcs.go +++ b/service_funcs.go @@ -5,6 +5,7 @@ import ( "fmt" "path/filepath" "strings" + `time` "github.com/godbus/dbus/v5" ) @@ -85,6 +86,8 @@ func (s *Service) CreateAliasedCollection(label, alias string) (collection *Coll var props map[string]dbus.Variant = make(map[string]dbus.Variant) props[DbusCollectionLabel] = dbus.MakeVariant(label) + props[DbusCollectionCreated] = dbus.MakeVariant(uint64(time.Now().Unix())) + props[DbusCollectionModified] = dbus.MakeVariant(uint64(time.Now().Unix())) if err = s.Dbus.Call( DbusServiceCreateCollection, 0, props, alias, @@ -103,9 +106,6 @@ func (s *Service) CreateAliasedCollection(label, alias string) (collection *Coll } collection, err = NewCollection(s, path) - if err = collection.setCreate(); err != nil { - return - } return } @@ -235,7 +235,6 @@ func (s *Service) GetSession() (ssn *Session, err error) { // Lock locks an Unlocked Collection or Item (LockableObject). func (s *Service) Lock(objects ...LockableObject) (err error) { - var variant dbus.Variant var toLock []dbus.ObjectPath // We only use these as destinations. var locked []dbus.ObjectPath @@ -252,10 +251,9 @@ func (s *Service) Lock(objects ...LockableObject) (err error) { for idx, o := range objects { toLock[idx] = o.path() } - variant = dbus.MakeVariant(toLock) if err = s.Dbus.Call( - DbusServiceLock, 0, variant, + DbusServiceLock, 0, toLock, ).Store(&locked, &promptPath); err != nil { return } @@ -347,7 +345,7 @@ func (s *Service) ReadAlias(alias string) (collection *Collection, err error) { // RemoveAlias is a thin wrapper around Service.SetAlias using the removal method specified there. func (s *Service) RemoveAlias(alias string) (err error) { - if err = s.SetAlias(alias, dbus.ObjectPath("/")); err != nil { + if err = s.SetAlias(alias, DbusRemoveAliasPath); err != nil { return } @@ -447,12 +445,25 @@ func (s *Service) SearchItems(attributes map[string]string) (unlockedItems []*It func (s *Service) SetAlias(alias string, objectPath dbus.ObjectPath) (err error) { var c *dbus.Call + var collection *Collection + + if collection, err = s.GetCollection(alias); err != nil { + return + } c = s.Dbus.Call( DbusServiceSetAlias, 0, alias, objectPath, ) - err = c.Err + if err = c.Err; err != nil { + return + } + + if objectPath == DbusRemoveAliasPath { + collection.Alias = "" + } else { + collection.Alias = alias + } return } @@ -460,7 +471,6 @@ func (s *Service) SetAlias(alias string, objectPath dbus.ObjectPath) (err error) // Unlock unlocks a locked Collection or Item (LockableObject). func (s *Service) Unlock(objects ...LockableObject) (err error) { - var variant dbus.Variant var toUnlock []dbus.ObjectPath // We only use these as destinations. var unlocked []dbus.ObjectPath @@ -477,10 +487,9 @@ func (s *Service) Unlock(objects ...LockableObject) (err error) { for idx, o := range objects { toUnlock[idx] = o.path() } - variant = dbus.MakeVariant(toUnlock) if err = s.Dbus.Call( - DbusServiceUnlock, 0, variant, + DbusServiceUnlock, 0, toUnlock, ).Store(&unlocked, &resultPath); err != nil { return } diff --git a/service_funcs_test.go b/service_funcs_test.go index e0067c8..d064517 100644 --- a/service_funcs_test.go +++ b/service_funcs_test.go @@ -277,7 +277,13 @@ func TestService_Secrets(t *testing.T) { t.Errorf("at least one locked item in collection '%v'", collectionName.String()) } if len(itemResultsUnlocked) != 1 { - t.Errorf("number of unlocked items in collection '%v' is not equal to 1", collectionName.String()) + t.Errorf( + "number of unlocked items in collection '%v' (%v) is not equal to 1; items dump pending...", + collectionName.String(), len(itemResultsUnlocked), + ) + for idx, i := range itemResultsUnlocked { + t.Logf("ITEM #%v IN COLLECTION %v: %v ('%v')", idx, collectionName.String(), i.LabelName, string(i.Dbus.Path())) + } } if resultItemName, err = itemResultsUnlocked[0].Label(); err != nil { t.Errorf("cannot fetch test Item name from collection '%v' in SearchItems: %v", collectionName.String(), err.Error())