Merge pull request #131 from maxlaverse/prevent_mixing_of_resource_types

prevent mixing of resource types
maxlaverse · Apr 20, 2024 · 28abbd6 · 28abbd6
2 parents 2f6c3cb + 58d5083
commit 28abbd6
Show file tree

Hide file tree

Showing 4 changed files with 29 additions and 28 deletions.
diff --git a/internal/bitwarden/bw/client.go b/internal/bitwarden/bw/client.go
@@ -13,7 +13,7 @@ type Client interface {
 	CreateObject(Object) (*Object, error)
 	EditObject(Object) (*Object, error)
 	GetAttachment(itemId, attachmentId string) ([]byte, error)
-	GetObject(objType, itemId string) (*Object, error)
+	GetObject(objType, itemOrSearch string) (*Object, error)
 	GetSessionKey() string
 	HasSessionKey() bool
 	LoginWithAPIKey(password, clientId, clientSecret string) error
@@ -137,8 +137,8 @@ func (c *client) EditObject(obj Object) (*Object, error) {
 	return &obj, nil
 }
 
-func (c *client) GetObject(objType, itemId string) (*Object, error) {
-	out, err := c.cmdWithSession("get", objType, itemId).Run()
+func (c *client) GetObject(objType, itemOrSearch string) (*Object, error) {
+	out, err := c.cmdWithSession("get", objType, itemOrSearch).Run()
 	if err != nil {
 		return nil, remapError(err)
 	}

diff --git a/internal/provider/data_source_item_login_test.go b/internal/provider/data_source_item_login_test.go
@@ -1,12 +1,10 @@
 package provider
 
 import (
-	"fmt"
 	"regexp"
 	"testing"
 
 	"github.com/hashicorp/terraform-plugin-sdk/v2/helper/resource"
-	"github.com/stretchr/testify/assert"
 )
 
 func TestAccDataSourceItemLoginAttributes(t *testing.T) {
@@ -40,46 +38,39 @@ func TestAccDataSourceItemLoginFailsOnInexistentItem(t *testing.T) {
 	})
 }
 
-func TestAccDataSourceItemLoginDeleted(t *testing.T) {
-	var objectID string
-
+func TestAccDataSourceItemLoginFailsOnWrongResourceType(t *testing.T) {
 	ensureVaultwardenConfigured(t)
 
 	resource.UnitTest(t, resource.TestCase{
 		ProviderFactories: providerFactories,
 		Steps: []resource.TestStep{
 			{
-				Config: tfConfigProvider() + tfConfigResourceItemLoginSmall(),
-				Check:  getObjectID("bitwarden_item_login.foo", &objectID),
+				Config: tfConfigProvider() + tfConfigResourceFolder() + tfConfigResourceItemSecureNote(),
 			},
 			{
-				Config: tfConfigProvider() + tfConfigResourceItemLoginSmall() + tfConfigDataItemLoginWithId(objectID),
-				PreConfig: func() {
-					err := bwTestClient(t).DeleteObject("item", objectID)
-					assert.NoError(t, err)
-				},
-				ExpectError: regexp.MustCompile("Error: object not found"),
+				Config:      tfConfigProvider() + tfConfigResourceFolder() + tfConfigResourceItemSecureNote() + tfConfigDataItemLoginCrossReference(),
+				ExpectError: regexp.MustCompile("Error: returned object type does not match requested object type"),
 			},
 		},
 	})
 }
 
-func tfConfigDataItemLoginWithId(id string) string {
-	return fmt.Sprintf(`
+func tfConfigDataItemLogin() string {
+	return `
 data "bitwarden_item_login" "foo_data" {
 	provider	= bitwarden
 
-	id 			= "%s"
+	id 			= bitwarden_item_login.foo.id
 }
-`, id)
+`
 }
 
-func tfConfigDataItemLogin() string {
+func tfConfigDataItemLoginCrossReference() string {
 	return `
 data "bitwarden_item_login" "foo_data" {
 	provider	= bitwarden
 
-	id 			= bitwarden_item_login.foo.id
+	id 			= bitwarden_item_secure_note.foo.id
 }
 `
 }

diff --git a/internal/provider/object.go b/internal/provider/object.go
@@ -17,12 +17,22 @@ func objectCreate(ctx context.Context, d *schema.ResourceData, meta interface{})
 func objectRead(ctx context.Context, d *schema.ResourceData, meta interface{}) diag.Diagnostics {
 	return diag.FromErr(objectOperation(ctx, d, func(secret bw.Object) (*bw.Object, error) {
 		obj, err := meta.(bw.Client).GetObject(string(secret.Object), secret.ID)
+		if obj != nil {
+			// If the object exists but is marked as soft deleted, we return an error, because relying
+			// on an object in the 'trash' sounds like a bad idea.
+			if obj.DeletedDate != nil {
+				return nil, errors.New("object is soft deleted")
+			}
+
+			if obj.ID != secret.ID {
+				return nil, errors.New("returned object ID does not match requested object ID")
+			}
 
-		// If the object exists but is marked as soft deleted, we return an error, because relying
-		// on an object in the 'trash' sounds like a bad idea.
-		if obj != nil && obj.DeletedDate != nil {
-			return nil, errors.New("object is soft deleted")
+			if obj.Type != secret.Type {
+				return nil, errors.New("returned object type does not match requested object type")
+			}
 		}
+
 		return obj, err
 	}))
 }
@@ -57,7 +67,7 @@ func objectDelete(ctx context.Context, d *schema.ResourceData, meta interface{})
 	}))
 }
 
-func objectOperation(ctx context.Context, d *schema.ResourceData, operation func(secret bw.Object) (*bw.Object, error)) error {
+func objectOperation(_ context.Context, d *schema.ResourceData, operation func(secret bw.Object) (*bw.Object, error)) error {
 	obj, err := operation(objectStructFromData(d))
 	if err != nil {
 		return err

diff --git a/internal/provider/provider.go b/internal/provider/provider.go
@@ -108,7 +108,7 @@ func New(version string) func() *schema.Provider {
 	}
 }
 
-func providerConfigure(version string, p *schema.Provider) func(context.Context, *schema.ResourceData) (interface{}, diag.Diagnostics) {
+func providerConfigure(version string, _ *schema.Provider) func(context.Context, *schema.ResourceData) (interface{}, diag.Diagnostics) {
 	return func(_ context.Context, d *schema.ResourceData) (interface{}, diag.Diagnostics) {
 
 		bwClient, err := newBitwardenClient(d, version)