From d81452a92c0f7e3e7ffed0cd5cf5f1df8667e9b9 Mon Sep 17 00:00:00 2001 From: brent s Date: Mon, 13 Dec 2021 00:15:38 -0500 Subject: [PATCH] =?UTF-8?q?v1=20complete=20and=20tests=20complete=20(for?= =?UTF-8?q?=20now.=20=F0=9F=98=89).?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- README.adoc | 10 +-- collection_funcs.go | 23 +++++- collection_funcs_test.go | 102 +++++++++++++++++++++++--- consts.go | 7 ++ doc.go | 6 +- item_funcs.go | 13 +--- item_funcs_test.go | 150 +++++++++++++++++++++++++++++++++++++++ service_funcs_test.go | 49 +++++++------ types.go | 8 +-- 9 files changed, 310 insertions(+), 58 deletions(-) create mode 100644 item_funcs_test.go diff --git a/README.adoc b/README.adoc index 1b1c7c5..b4ea98c 100644 --- a/README.adoc +++ b/README.adoc @@ -99,12 +99,10 @@ Service And so on. -In *practice*, however, most users will only have two Session types: +In *practice*, however, most users will only have two ``Collection``s: -* a default "system" one, and -* a temporary one that may or may not exist, running in memory for the current login session - -and a single Collection, named `login` (and aliased to `default`, usually). +* a default "system" one named `login` (usually unlocked upon login), and +* a temporary one that may or may not exist, running in memory for the current login session named `session` == Usage @@ -235,3 +233,5 @@ Many functions are consolidated into a single test due to how dependent certain Obviously since this library interacts directly with Dbus (and I don't want to spend the time to mock up an entire Dbus-like interface to test), all tests are integration tests rather than unit tests. Therefore in the event of a failed run, you will need to open e.g. Seahorse or d-feet or some other Dbus/SecretService browser and manually delete the created Secret Service collection. It/they should be easily identified; they use a generated UUID4 string as the collection name and it is highly unlikely that you will see any other collections named as such. If running `go test` with the verbose flag (`-v`), the name and path of the collection will be printed out. If all tests pass, the test collection should be removed automatically. The same UUID is used for all tests in a test run. + +You may be prompted during a test run for a password; you can simply use a blank password for this as it is the password used to protect a collection. This prompt pops up during the creation of a Collection. diff --git a/collection_funcs.go b/collection_funcs.go index c35acfb..7f7cc31 100644 --- a/collection_funcs.go +++ b/collection_funcs.go @@ -41,16 +41,30 @@ func NewCollection(service *Service, path dbus.ObjectPath) (coll *Collection, er return } -// CreateItem returns a pointer to an Item based on a label, some attributes, a Secret, and whether any existing secret with the same label should be replaced or not. -func (c *Collection) CreateItem(label string, attrs map[string]string, secret *Secret, replace bool) (item *Item, err error) { +/* + CreateItem returns a pointer to an Item based on a label, some attributes, a Secret, + whether any existing secret with the same label should be replaced or not, and the optional itemType. + + itemType is optional; if specified, it should be a Dbus interface (only the first element is used). + If not specified, the default DbusDefaultItemType will be used. +*/ +func (c *Collection) CreateItem(label string, attrs map[string]string, secret *Secret, replace bool, itemType ...string) (item *Item, err error) { var prompt *Prompt var path dbus.ObjectPath var promptPath dbus.ObjectPath var variant *dbus.Variant var props map[string]dbus.Variant = make(map[string]dbus.Variant) + var typeString string + + if itemType != nil && len(itemType) > 0 { + typeString = itemType[0] + } else { + typeString = DbusDefaultItemType + } props[DbusItemLabel] = dbus.MakeVariant(label) + props[DbusItemType] = dbus.MakeVariant(typeString) props[DbusItemAttributes] = dbus.MakeVariant(attrs) if err = c.Dbus.Call( @@ -177,8 +191,11 @@ func (c *Collection) Relabel(newLabel string) (err error) { /* SearchItems searches a Collection for a matching profile string. - It's mostly a carry-over from go-libsecret, and is here for convenience. + It's mostly a carry-over from go-libsecret, and is here for convenience. IT MAY BE REMOVED IN THE FUTURE. + I promise it's not useful for any other implementation/storage of SecretService whatsoever. + + Deprecated: Use Service.SearchItems instead. */ func (c *Collection) SearchItems(profile string) (items []*Item, err error) { diff --git a/collection_funcs_test.go b/collection_funcs_test.go index 5666d44..7145c20 100644 --- a/collection_funcs_test.go +++ b/collection_funcs_test.go @@ -66,12 +66,13 @@ func TestCollection_Items(t *testing.T) { } if collection, err = svc.GetCollection(defaultCollection); err != nil { - if err = svc.Close(); err != nil { - t.Errorf("could not close Service.Session: %v", err.Error()) - } - t.Fatalf("failed when fetching collection '%v': %v", + t.Errorf("failed when fetching collection '%v': %v", defaultCollection, err.Error(), ) + if err = svc.Close(); err != nil { + t.Fatalf("could not close Service.Session: %v", err.Error()) + } + return } if items, err = collection.Items(); err != nil { @@ -83,6 +84,22 @@ func TestCollection_Items(t *testing.T) { t.Logf("found %v items in collection '%v' at '%v'", len(items), defaultCollection, string(collection.Dbus.Path())) } + /* This is almost always going to trigger the warning. See Item.idx for details why. + var label string + for idx, i := range items { + if label, err = i.Label(); err != nil { + t.Errorf("failed to get label of item '%v' in collection '%v': %v", string(i.Dbus.Path()), collectionName.String(), err.Error()) + continue + } + if i.idx != idx { + t.Logf( + "WARN: item '%v' ('%v') in collection '%v' internal IDX ('%v') does NOT match native slice IDX ('%v')", + string(i.Dbus.Path()), label, collectionName.String(), i.idx, idx, + ) + } + } + */ + secret = NewSecret(svc.Session, []byte{}, []byte(testSecretContent), "text/plain") if item, err = collection.CreateItem(testItemLabel, itemAttrs, secret, false); err != nil { @@ -136,10 +153,10 @@ func TestCollection_Label(t *testing.T) { "failed when fetching collection '%v': %v", defaultCollectionLabel, err.Error(), ) - err = nil if err = svc.Close(); err != nil { - t.Errorf("could not close Service.Session: %v", err.Error()) + t.Fatalf("could not close Service.Session: %v", err.Error()) } + return } if collLabel, err = collection.Label(); err != nil { @@ -147,6 +164,7 @@ func TestCollection_Label(t *testing.T) { if err = svc.Close(); err != nil { t.Fatalf("could not close Service.Session: %v", err.Error()) } + return } if defaultCollectionLabel != collLabel { @@ -182,10 +200,10 @@ func TestCollection_Locked(t *testing.T) { "failed when fetching collection '%v': %v", defaultCollectionLabel, err.Error(), ) - err = nil if err = svc.Close(); err != nil { - t.Errorf("could not close Service.Session: %v", err.Error()) + t.Fatalf("could not close Service.Session: %v", err.Error()) } + return } if isLocked, err = collection.Locked(); err != nil { @@ -198,3 +216,71 @@ func TestCollection_Locked(t *testing.T) { t.Errorf("could not close Service.Session: %v", err.Error()) } } + +/* + TestCollection_Relabel tests the following internal functions/methods via nested calls: + + (all calls in TestNewCollection) + Service.CreateCollection + Collection.Relabel + +*/ +func TestCollection_Relabel(t *testing.T) { + + var svc *Service + var collection *Collection + var collLabel string + var newCollLabel string = collectionAlias.String() + var err error + + if svc, err = NewService(); err != nil { + t.Fatalf("NewService failed: %v", err.Error()) + } + + if collection, err = svc.CreateCollection(collectionName.String()); err != nil { + t.Errorf("could not create collection '%v': %v", collectionName.String(), err.Error()) + if err = svc.Close(); err != nil { + t.Fatalf("could not close Service.Session: %v", err.Error()) + } + return + } else { + t.Logf("created collection '%v' at path '%v' successfully", collectionName.String(), string(collection.Dbus.Path())) + } + + if collLabel, err = collection.Label(); err != nil { + t.Errorf("could not fetch label for collection '%v': %v", string(collection.Dbus.Path()), err.Error()) + if err = svc.Close(); err != nil { + t.Fatalf("could not close Service.Session: %v", err.Error()) + } + return + } + + if err = collection.Relabel(newCollLabel); err != nil { + t.Errorf("failed to relabel collection '%v' to '%v': %v", collLabel, newCollLabel, err.Error()) + } else { + t.Logf("relabeled collection '%v' to '%v'", collLabel, newCollLabel) + } + + if collLabel, err = collection.Label(); err != nil { + t.Errorf("could not fetch label for collection '%v': %v", string(collection.Dbus.Path()), err.Error()) + if err = collection.Delete(); err != nil { + t.Errorf("failed to delete collection '%v': %v", string(collection.Dbus.Path()), err.Error()) + } + if err = svc.Close(); err != nil { + t.Fatalf("could not close Service.Session: %v", err.Error()) + } + return + } else { + if collLabel != newCollLabel { + t.Errorf("collection did not relabel; new label '%v', actual label '%v'", newCollLabel, collLabel) + } + } + + if err = collection.Delete(); err != nil { + t.Errorf("failed to delete collection '%v': %v", string(collection.Dbus.Path()), err.Error()) + } + + if err = svc.Close(); err != nil { + t.Errorf("could not close Service.Session: %v", err.Error()) + } +} diff --git a/consts.go b/consts.go index a01ba95..f0b6939 100644 --- a/consts.go +++ b/consts.go @@ -18,6 +18,13 @@ const ( DbusServiceBase string = "org.freedesktop.Secret" // DbusPrompterInterface is an interface for issuing a Prompt. Yes, it should be doubled up like that. DbusPrompterInterface string = DbusServiceBase + ".Prompt.Prompt" + /* + DbusDefaultItemType is the default type to use for Item.Type. + I've only ever seen "org.gnome.keyring.NetworkPassword" in the wild + aside from the below. It may be legacy (gnome-keyring is obsoleted by SecretService). + If in doubt, the below is considered the "proper" interface. + */ + DbusDefaultItemType string = DbusServiceBase + ".Generic" ) // Service interface. diff --git a/doc.go b/doc.go index e7bc876..31915c6 100644 --- a/doc.go +++ b/doc.go @@ -76,9 +76,9 @@ So the object hierarchy in THEORY looks kind of like this: └─ Secret "B_2_b" And so on. -In PRACTICE, however, most users will only have two Session items -(a default "system" one and a temporary one that may or may not exist, running in memory for the current login session) -and a single Collection, named "login" (and aliased to "default", usually). +In PRACTICE, however, most users will only have two Collection items +(a default "system" one named "login", which usually is unlocked upon login, +and a temporary one that may or may not exist, running in memory for the current login session named `session`). Usage diff --git a/item_funcs.go b/item_funcs.go index 2afd645..74d375f 100644 --- a/item_funcs.go +++ b/item_funcs.go @@ -173,15 +173,9 @@ func (i *Item) Relabel(newLabel string) (err error) { // ReplaceAttributes replaces the Item's attributes in Dbus. func (i *Item) ReplaceAttributes(newAttrs map[string]string) (err error) { - var label string - var props map[string]dbus.Variant = make(map[string]dbus.Variant, 0) + var props dbus.Variant - if label, err = i.Label(); err != nil { - return - } - - props[DbusItemLabel] = dbus.MakeVariant(label) - props[DbusItemAttributes] = dbus.MakeVariant(newAttrs) + props = dbus.MakeVariant(newAttrs) if err = i.Dbus.SetProperty(DbusItemAttributes, props); err != nil { return @@ -217,8 +211,7 @@ func (i *Item) Type() (itemType string, err error) { return } - i.ItemType = variant.Value().(string) - itemType = i.ItemType + itemType = variant.Value().(string) return } diff --git a/item_funcs_test.go b/item_funcs_test.go new file mode 100644 index 0000000..af3c0d0 --- /dev/null +++ b/item_funcs_test.go @@ -0,0 +1,150 @@ +package gosecret + +import ( + `reflect` + `testing` +) + +// Some functions are covered in the Service tests and Collection tests. + +/* + TestItem tests all remaining Item funcs (see Service and Collection funcs for the other tests. + +*/ +func TestItem(t *testing.T) { + + var svc *Service + var collection *Collection + var item *Item + var secret *Secret + var newItemLabel string + var testLabel string + var attrs map[string]string + var modAttrs map[string]string + var newAttrs map[string]string + var newAttrsGnome map[string]string + var typeString string + var err error + + // Setup. + if svc, err = NewService(); err != nil { + t.Fatalf("NewService failed: %v", err.Error()) + } + + if collection, err = svc.CreateCollection(collectionName.String()); err != nil { + t.Errorf("could not create collection '%v': %v", collectionName.String(), err.Error()) + if err = svc.Close(); err != nil { + t.Fatalf("could not close Service.Session: %v", err.Error()) + } + return + } else { + t.Logf("created collection '%v' at path '%v' successfully", collectionName.String(), string(collection.Dbus.Path())) + } + + // Create an Item/Secret. + secret = NewSecret(svc.Session, []byte{}, []byte(testSecretContent), "text/plain") + + 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 = svc.Close(); err != nil { + t.Fatalf("could not close Service.Session: %v", err.Error()) + } + return + } + + // Fetch attributes + if attrs, err = item.Attributes(); err != nil { + t.Errorf("failed to fetch attributes for item %v in collection '%v': %v", testItemLabel, collectionName.String(), err.Error()) + } else { + t.Logf( + "Fetch result; original attributes: %#v, fetched attributes: %#v for item '%v' in '%v'", + itemAttrs, attrs, testItemLabel, collectionName.String(), + ) + } + + newAttrs = map[string]string{ + "foo": "bar", + "bar": "baz", + "baz": "quux", + } + + // Replace attributes. + if err = item.ReplaceAttributes(newAttrs); err != nil { + t.Errorf("could not replace attributes for item '%v' in collection '%v': %v", testItemLabel, collectionName.String(), err.Error()) + } else { + // Modify attributes. + // "flat" modification. + modAttrs = map[string]string{ + "foo": "quux", + } + if err = item.ModifyAttributes(modAttrs); err != nil { + t.Errorf( + "could not modify attributes for item '%v' in collection '%v' (%#v => %#v): %v", + testItemLabel, collectionName.String(), newAttrs, modAttrs, err.Error(), + ) + } + // "delete" modification. + newAttrs = map[string]string{ + "foo": "quux", + "bar": "baz", + } + newAttrsGnome = make(map[string]string, 0) + for k, v := range newAttrs { + newAttrsGnome[k] = v + } + // Added via SecretService automatically? Seahorse? It appears sometimes and others it does not. Cause is unknown. + newAttrsGnome["xdg:schema"] = DbusDefaultItemType + modAttrs = map[string]string{ + "baz": ExplicitAttrEmptyValue, + } + + if err = item.ModifyAttributes(modAttrs); err != nil { + t.Errorf( + "could not modify (with deletion) attributes for item '%v' in collection '%v' (%#v => %#v): %v", + testItemLabel, collectionName.String(), newAttrs, modAttrs, err.Error(), + ) + } else { + if attrs, err = item.Attributes(); err != nil { + t.Errorf("failed to fetch attributes for item %v in collection '%v': %v", testItemLabel, collectionName.String(), err.Error()) + } + if !reflect.DeepEqual(attrs, newAttrs) && !reflect.DeepEqual(attrs, newAttrsGnome) { + t.Errorf("newly-modified attributes (%#v) do not match expected attributes (%#v)", attrs, newAttrs) + } else { + t.Logf("modified attributes (%#v) match expected attributes (%#v)", attrs, newAttrs) + } + } + } + + // Item.Relabel + newItemLabel = testItemLabel + "_RELABELED" + if err = item.Relabel(newItemLabel); err != nil { + t.Errorf("failed to relabel item '%v' to '%v': %v", testItemLabel, newItemLabel, err.Error()) + } + if testLabel, err = item.Label(); err != nil { + t.Errorf("failed to fetch label for '%v': %v", string(item.Dbus.Path()), err.Error()) + } + if newItemLabel != testLabel { + t.Errorf("new item label post-relabeling ('%v') does not match explicitly set label ('%v')", testLabel, newItemLabel) + } + + // And Item.Type. + if typeString, err = item.Type(); err != nil { + t.Errorf("failed to get Item.Type for '%v': %v", string(item.Dbus.Path()), err.Error()) + } else { + if typeString != DbusDefaultItemType { + t.Errorf("Item.Type mismatch for '%v': '%v' (should be '%v')", string(item.Dbus.Path()), typeString, DbusDefaultItemType) + } + t.Logf("item type for '%v': %v", string(item.Dbus.Path()), typeString) + } + + // Teardown. + if err = item.Delete(); err != nil { + t.Errorf("failed to delete item '%v': %v", string(item.Dbus.Path()), err.Error()) + } + if err = collection.Delete(); err != nil { + t.Errorf("failed to delete collection '%v': %v", collectionName.String(), err.Error()) + } + if err = svc.Close(); err != nil { + t.Errorf("could not close Service.Session: %v", err.Error()) + } +} diff --git a/service_funcs_test.go b/service_funcs_test.go index de76b3d..f3817f6 100644 --- a/service_funcs_test.go +++ b/service_funcs_test.go @@ -63,30 +63,31 @@ func TestService_Collections(t *testing.T) { t.Errorf("could not get Service.Collections: %v", err.Error()) } else { t.Logf("found %v collections via Service.Collections", len(colls)) - } - for idx, c := range colls { - if collLabel, err = c.Label(); err != nil { - t.Errorf( - "failed to get label for collection '%v': %v", - string(c.Dbus.Path()), err.Error(), + + for idx, c := range colls { + if collLabel, err = c.Label(); err != nil { + t.Errorf( + "failed to get label for collection '%v': %v", + string(c.Dbus.Path()), err.Error(), + ) + } + if created, err = c.Created(); err != nil { + t.Errorf( + "failed to get created time for collection '%v': %v", + string(c.Dbus.Path()), err.Error(), + ) + } + if modified, _, err = c.Modified(); err != nil { + t.Errorf( + "failed to get modified time for collection '%v': %v", + string(c.Dbus.Path()), err.Error(), + ) + } + t.Logf( + "collection #%v (name '%v', label '%v'): created %v, last modified %v", + idx, c.PathName(), collLabel, created, modified, ) } - if created, err = c.Created(); err != nil { - t.Errorf( - "failed to get created time for collection '%v': %v", - string(c.Dbus.Path()), err.Error(), - ) - } - if modified, _, err = c.Modified(); err != nil { - t.Errorf( - "failed to get modified time for collection '%v': %v", - string(c.Dbus.Path()), err.Error(), - ) - } - t.Logf( - "collection #%v (name '%v', label '%v'): created %v, last modified %v", - idx, c.PathName(), collLabel, created, modified, - ) } if err = svc.Close(); err != nil { @@ -158,7 +159,7 @@ func TestService_CreateAliasedCollection(t *testing.T) { Service.GetCollection NewCollection Collection.Modified - Service.ReadAlias + Service.ReadAlias The default collection (login) is fetched instead of creating one as this collection should exist, and thus this function tests fetching existing collections instead of newly-created ones. @@ -228,6 +229,7 @@ func TestService_Secrets(t *testing.T) { if err = svc.Close(); err != nil { t.Fatalf("could not close Service.Session: %v", err.Error()) } + return } else { t.Logf("created collection '%v' at path '%v' successfully", collectionName.String(), string(collection.Dbus.Path())) } @@ -256,6 +258,7 @@ func TestService_Secrets(t *testing.T) { if err = svc.Close(); err != nil { t.Fatalf("could not close Service.Session: %v", err.Error()) } + return } if itemName, err = testItem.Label(); err != nil { diff --git a/types.go b/types.go index 3c02b9f..61612f9 100644 --- a/types.go +++ b/types.go @@ -107,18 +107,14 @@ type Item struct { *DbusObject // Secret is the corresponding Secret object. Secret *Secret `json:"secret"` - /* - ItemType is the type of this Item as a Dbus interface name. - e.g. org.gnome.keyring.NetworkPassword, org.freedesktop.Secret.Generic, org.remmina.Password, etc. - */ - ItemType string `json:"dbus_type"` // lastModified is unexported because it's important that API users don't change it; it's used by Collection.Modified. lastModified time.Time // lastModifiedSet is unexported; it's only used to determine if this is a first-initialization of the modification time or not. lastModifiedSet bool /* idx is the index identifier of the Item. - It SHOULD correlate to indices in Collection.Items, but don't rely on this. + It is almost guaranteed to not match the index in Collection.Items (unless you have like, only one item) + as those indices are static and do not determine the order that Dbus returns the list of item paths. */ idx int // collection tracks the Collection this Item is in.