Merge pull request #159 from maxlaverse/small_improvements

Small improvements

GNUmakefile

@@ -13,4 +13,16 @@ docs:
 	find docs -type f -name '*.md' -exec sed -i '' '/INTERNAL USE/d' {} \;
 
 clean:
-	rm internal/provider/.bitwarden/data.json || true
+	rm internal/provider/.bitwarden/data.json || true
+
+server:
+	docker run -ti \
+	-e I_REALLY_WANT_VOLATILE_STORAGE=true \
+	-e DISABLE_ICON_DOWNLOAD=true \
+	-e ADMIN_TOKEN=test1234 \
+	-e LOGIN_RATELIMIT_SECONDS=1 \
+	-e LOGIN_RATELIMIT_MAX_BURST=1000000 \
+	-e ADMIN_RATELIMIT_SECONDS=1 \
+	-e ADMIN_RATELIMIT_MAX_BURST=1000000 \
+	--mount type=tmpfs,destination=/data \
+	-p 8080:80 vaultwarden/server

README.md

@@ -78,7 +78,9 @@ Currently, the Terraform SDK doesn't offer a way to remove the encrypted Vault o
 The issue [hashicorp/terraform-plugin-sdk#63] tracks discussions for adding such a feature.
 
 If you want find out more about this file, you can read [Terraform's documentation on Data Storage].
-Please note that this file is stored at `<your-project>/.bitwarden/` by default, in order to not interfer with your local Vaults.
+Please note that this file is stored at `<your-project>/.bitwarden/` by default, in order to not interfere with your local Vaults.
+
+NOTE: This whole paragraph doesn't apply to the experimental client, as nothing is stored on disk.
 
 ## Developing the Provider
 
@@ -90,16 +92,7 @@ To generate or update documentation, run `go generate`.
 
 In order to run the full suite of Acceptance tests, start a Vaultwarden server:
 ```sh
-$ docker run -ti \
-  -e I_REALLY_WANT_VOLATILE_STORAGE=true \
-  -e DISABLE_ICON_DOWNLOAD=true \
-  -e ADMIN_TOKEN=test1234 \
-  -e LOGIN_RATELIMIT_SECONDS=1 \
-  -e LOGIN_RATELIMIT_MAX_BURST=1000000 \
-  -e ADMIN_RATELIMIT_SECONDS=1 \
-  -e ADMIN_RATELIMIT_MAX_BURST=1000000 \
-  --mount type=tmpfs,destination=/data \
-  -p 8080:80 vaultwarden/server
+$ make server
 ```
 
 Then run `make testacc`.

internal/bitwarden/webapi/client.go

@@ -19,7 +19,7 @@ import (
 )
 
 const (
-	deviceName = "Terraform Provider Bitwarden"
+	deviceName = "Bitwarden Terraform Provider"
 )
 
 type Client interface {
@@ -333,6 +333,9 @@ func (c *client) LoginWithAPIKey(ctx context.Context, clientId, clientSecret str
 	}
 
 	tokenResp, err := doRequest[TokenResponse](ctx, c.httpClient, httpReq)
+	if err != nil {
+		return nil, err
+	}
 	c.sessionAccessToken = tokenResp.AccessToken
 	return tokenResp, err
 }

internal/provider/provider.go

@@ -3,8 +3,10 @@ package provider
 import (
 	"context"
 	"fmt"
+	"os"
 	"os/exec"
 	"path/filepath"
+	"strings"
 
 	"github.com/hashicorp/terraform-plugin-log/tflog"
 	"github.com/hashicorp/terraform-plugin-sdk/v2/diag"
@@ -108,14 +110,6 @@ func New(version string) func() *schema.Provider {
 						},
 					},
 				},
-
-				// Internal
-				attributeDeviceIdentifier: {
-					Description: descriptionInternal,
-					Type:        schema.TypeString,
-					Computed:    true,
-					Optional:    true,
-				},
 			},
 			DataSourcesMap: map[string]*schema.Resource{
 				"bitwarden_attachment":       dataSourceAttachment(),
@@ -139,7 +133,7 @@ func New(version string) func() *schema.Provider {
 	}
 }
 
-func experimentalEmbeddedClient(d *schema.ResourceData) bool {
+func useExperimentalEmbeddedClient(d *schema.ResourceData) bool {
 	experimentalFeatures, hasExperimentalFeatures := d.GetOk(attributeExperimental)
 	if hasExperimentalFeatures {
 		if experimentalFeatures.(*schema.Set).Len() > 0 {
@@ -154,7 +148,11 @@ func experimentalEmbeddedClient(d *schema.ResourceData) bool {
 func providerConfigure(version string, _ *schema.Provider) func(context.Context, *schema.ResourceData) (interface{}, diag.Diagnostics) {
 	return func(ctx context.Context, d *schema.ResourceData) (interface{}, diag.Diagnostics) {
 
-		if experimentalEmbeddedClient(d) {
+		if useExperimentalEmbeddedClient(d) {
+			if _, hasSessionKey := d.GetOk(attributeSessionKey); hasSessionKey {
+				return nil, diag.Errorf("session key is not supported with the embedded client")
+			}
+
 			bwClient, err := newBitwardenEmbeddedClient(ctx, d, version)
 			if err != nil {
 				return nil, diag.FromErr(err)
@@ -311,7 +309,7 @@ func newBitwardenClient(d *schema.ResourceData, version string) (bwcli.CLIClient
 }
 
 func newBitwardenEmbeddedClient(ctx context.Context, d *schema.ResourceData, version string) (bitwarden.Client, error) {
-	deviceId, err := getOrGenerateDeviceIdentifier(ctx, d)
+	deviceId, err := getOrGenerateDeviceIdentifier(ctx)
 	if err != nil {
 		return nil, err
 	}
@@ -329,18 +327,28 @@ func newBitwardenEmbeddedClient(ctx context.Context, d *schema.ResourceData, ver
 	return embedded.NewWebAPIVault(serverURL, opts...), nil
 }
 
-func getOrGenerateDeviceIdentifier(ctx context.Context, d *schema.ResourceData) (string, error) {
-	deviceId, hasDeviceID := d.GetOk(attributeDeviceIdentifier)
-	if hasDeviceID {
-		return deviceId.(string), nil
+func getOrGenerateDeviceIdentifier(ctx context.Context) (string, error) {
+	deviceIdBytes, err := os.ReadFile(".bitwarden/device_identifier")
+	if err == nil {
+		deviceId := string(deviceIdBytes)
+		tflog.Info(ctx, "Read device identifier from disk", map[string]interface{}{"device_id": deviceId})
+		return strings.TrimSpace(deviceId), nil
 	}
-	deviceId = embedded.NewDeviceIdentifier()
-	err := d.Set(attributeDeviceIdentifier, embedded.NewDeviceIdentifier())
+
+	deviceId := embedded.NewDeviceIdentifier()
+	err = os.Mkdir(".bitwarden", 0700)
 	if err != nil {
+		tflog.Error(ctx, "Failed to create .bitwarden directory", map[string]interface{}{"error": err})
 		return "", err
 	}
+	err = os.WriteFile(".bitwarden/device_identifier", []byte(deviceId), 0600)
+	if err != nil {
+		tflog.Error(ctx, "Failed to store device identifier", map[string]interface{}{"error": err})
+		return "", err
+	}
+
 	tflog.Info(ctx, "Generated device identifier", map[string]interface{}{"device_id": deviceId})
-	return deviceId.(string), nil
+	return deviceId, nil
 }
 
 func ensureLoggedInEmbedded(ctx context.Context, d *schema.ResourceData, bwClient bitwarden.Client) error {

internal/provider/schema_attributes.go

@@ -92,7 +92,6 @@ const (
 	attributeExtraCACertsPath           = "extra_ca_certs"
 	attributeExperimental               = "experimental"
 	attributeExperimentalEmbeddedClient = "embedded_client"
-	attributeDeviceIdentifier           = "device_id"
 
 	// Provider field descriptions
 	descriptionClientSecret               = "Client Secret (env: `BW_CLIENTSECRET`). Do not commit this information in Git unless you know what you're doing. Prefer using a Terraform `variable {}` in order to inject this value from the environment."