From d3245472f4d2c278779d1e736c30e0eb40924a14 Mon Sep 17 00:00:00 2001 From: Brian Flad Date: Tue, 6 Sep 2022 08:48:44 -0400 Subject: [PATCH] resource/random_password: Fix bcrypt_hash generation (#308) Reference: https://github.com/hashicorp/terraform-provider-random/issues/307 This change fixes the source of the `bcrypt_hash` generation to being the result of the random password generation. The issue was introduced in v3.4.0. Previously: ``` --- FAIL: TestAccResourcePassword_BcryptHash (0.63s) /Users/bflad/src/github.com/hashicorp/terraform-provider-random/internal/provider/resource_password_test.go:107: Step 1/1 error: Check failed: Check 3/3 error: crypto/bcrypt: hashedPassword is not the hash of the given password ``` Suggested CHANGELOG: ``` NOTES: * resource/random_password: If the resource was created between versions 3.4.0 and 3.4.2, the `bcrypt_hash` value would not correctly verify against the `result` value. Affected resources will automatically regenerate a valid `bcrypt_hash` after this upgrade. BUG FIXES: * resource/random_password: Fixed incorrect `bcrypt_hash` generation since version 3.4.0 ``` --- internal/provider/provider_test.go | 9 + internal/provider/resource_password.go | 318 +++++++++++- internal/provider/resource_password_test.go | 504 +++++++++++++++++++- internal/provider/tftypes_test.go | 23 + 4 files changed, 826 insertions(+), 28 deletions(-) create mode 100644 internal/provider/tftypes_test.go diff --git a/internal/provider/provider_test.go b/internal/provider/provider_test.go index 579db5c4..aa22c51d 100644 --- a/internal/provider/provider_test.go +++ b/internal/provider/provider_test.go @@ -39,3 +39,12 @@ func providerVersion332() map[string]resource.ExternalProvider { }, } } + +func providerVersion342() map[string]resource.ExternalProvider { + return map[string]resource.ExternalProvider{ + "random": { + VersionConstraint: "3.4.2", + Source: "hashicorp/random", + }, + } +} diff --git a/internal/provider/resource_password.go b/internal/provider/resource_password.go index 4d4abb3a..a06d7c4c 100644 --- a/internal/provider/resource_password.go +++ b/internal/provider/resource_password.go @@ -2,6 +2,7 @@ package provider import ( "context" + "errors" "github.com/hashicorp/terraform-plugin-framework-validators/int64validator" "github.com/hashicorp/terraform-plugin-framework/diag" @@ -22,7 +23,7 @@ var _ provider.ResourceType = (*passwordResourceType)(nil) type passwordResourceType struct{} func (r *passwordResourceType) GetSchema(context.Context) (tfsdk.Schema, diag.Diagnostics) { - return passwordSchemaV2(), nil + return passwordSchemaV3(), nil } func (r *passwordResourceType) NewResource(_ context.Context, _ provider.Provider) (resource.Resource, diag.Diagnostics) { @@ -38,7 +39,7 @@ var ( type passwordResource struct{} func (r *passwordResource) Create(ctx context.Context, req resource.CreateRequest, resp *resource.CreateResponse) { - var plan passwordModelV2 + var plan passwordModelV3 diags := req.Plan.Get(ctx, &plan) resp.Diagnostics.Append(diags...) @@ -65,7 +66,7 @@ func (r *passwordResource) Create(ctx context.Context, req resource.CreateReques return } - state := passwordModelV2{ + state := passwordModelV3{ ID: types.String{Value: "none"}, Keepers: plan.Keepers, Length: types.Int64{Value: plan.Length.Value}, @@ -82,7 +83,7 @@ func (r *passwordResource) Create(ctx context.Context, req resource.CreateReques Result: types.String{Value: string(result)}, } - hash, err := generateHash(plan.Result.Value) + hash, err := generateHash(string(result)) if err != nil { resp.Diagnostics.Append(diagnostics.HashGenerationError(err.Error())...) } @@ -102,7 +103,7 @@ func (r *passwordResource) Read(ctx context.Context, req resource.ReadRequest, r // Update ensures the plan value is copied to the state to complete the update. func (r *passwordResource) Update(ctx context.Context, req resource.UpdateRequest, resp *resource.UpdateResponse) { - var model passwordModelV2 + var model passwordModelV3 resp.Diagnostics.Append(req.Plan.Get(ctx, &model)...) @@ -121,7 +122,7 @@ func (r *passwordResource) Delete(ctx context.Context, req resource.DeleteReques func (r *passwordResource) ImportState(ctx context.Context, req resource.ImportStateRequest, resp *resource.ImportStateResponse) { id := req.ID - state := passwordModelV2{ + state := passwordModelV3{ ID: types.String{Value: "none"}, Result: types.String{Value: id}, Length: types.Int64{Value: int64(len(id))}, @@ -155,20 +156,25 @@ func (r *passwordResource) ImportState(ctx context.Context, req resource.ImportS func (r *passwordResource) UpgradeState(context.Context) map[int64]resource.StateUpgrader { schemaV0 := passwordSchemaV0() schemaV1 := passwordSchemaV1() + schemaV2 := passwordSchemaV2() return map[int64]resource.StateUpgrader{ 0: { PriorSchema: &schemaV0, - StateUpgrader: upgradePasswordStateV0toV2, + StateUpgrader: upgradePasswordStateV0toV3, }, 1: { PriorSchema: &schemaV1, - StateUpgrader: upgradePasswordStateV1toV2, + StateUpgrader: upgradePasswordStateV1toV3, + }, + 2: { + PriorSchema: &schemaV2, + StateUpgrader: upgradePasswordStateV2toV3, }, } } -func upgradePasswordStateV0toV2(ctx context.Context, req resource.UpgradeStateRequest, resp *resource.UpgradeStateResponse) { +func upgradePasswordStateV0toV3(ctx context.Context, req resource.UpgradeStateRequest, resp *resource.UpgradeStateResponse) { type modelV0 struct { ID types.String `tfsdk:"id"` Keepers types.Map `tfsdk:"keepers"` @@ -192,7 +198,7 @@ func upgradePasswordStateV0toV2(ctx context.Context, req resource.UpgradeStateRe return } - passwordDataV2 := passwordModelV2{ + passwordDataV3 := passwordModelV3{ Keepers: passwordDataV0.Keepers, Length: passwordDataV0.Length, Special: passwordDataV0.Special, @@ -209,19 +215,19 @@ func upgradePasswordStateV0toV2(ctx context.Context, req resource.UpgradeStateRe ID: passwordDataV0.ID, } - hash, err := generateHash(passwordDataV2.Result.Value) + hash, err := generateHash(passwordDataV3.Result.Value) if err != nil { resp.Diagnostics.Append(diagnostics.HashGenerationError(err.Error())...) return } - passwordDataV2.BcryptHash.Value = hash + passwordDataV3.BcryptHash.Value = hash - diags := resp.State.Set(ctx, passwordDataV2) + diags := resp.State.Set(ctx, passwordDataV3) resp.Diagnostics.Append(diags...) } -func upgradePasswordStateV1toV2(ctx context.Context, req resource.UpgradeStateRequest, resp *resource.UpgradeStateResponse) { +func upgradePasswordStateV1toV3(ctx context.Context, req resource.UpgradeStateRequest, resp *resource.UpgradeStateResponse) { type modelV1 struct { ID types.String `tfsdk:"id"` Keepers types.Map `tfsdk:"keepers"` @@ -246,7 +252,7 @@ func upgradePasswordStateV1toV2(ctx context.Context, req resource.UpgradeStateRe return } - passwordDataV2 := passwordModelV2{ + passwordDataV3 := passwordModelV3{ Keepers: passwordDataV1.Keepers, Length: passwordDataV1.Length, Special: passwordDataV1.Special, @@ -264,16 +270,278 @@ func upgradePasswordStateV1toV2(ctx context.Context, req resource.UpgradeStateRe ID: passwordDataV1.ID, } - diags := resp.State.Set(ctx, passwordDataV2) + diags := resp.State.Set(ctx, passwordDataV3) resp.Diagnostics.Append(diags...) } +func upgradePasswordStateV2toV3(ctx context.Context, req resource.UpgradeStateRequest, resp *resource.UpgradeStateResponse) { + var passwordDataV2 passwordModelV2 + + resp.Diagnostics.Append(req.State.Get(ctx, &passwordDataV2)...) + + if resp.Diagnostics.HasError() { + return + } + + // Schema version 2 to schema version 3 is a duplicate of the data, + // however the BcryptHash value may have been incorrectly generated. + + //nolint:gosimple // V3 model will expand over time so all fields are written out to help future code changes. + passwordDataV3 := passwordModelV3{ + BcryptHash: passwordDataV2.BcryptHash, + ID: passwordDataV2.ID, + Keepers: passwordDataV2.Keepers, + Length: passwordDataV2.Length, + Lower: passwordDataV2.Lower, + MinLower: passwordDataV2.MinLower, + MinNumeric: passwordDataV2.MinNumeric, + MinSpecial: passwordDataV2.MinSpecial, + MinUpper: passwordDataV2.MinUpper, + Number: passwordDataV2.Number, + Numeric: passwordDataV2.Numeric, + OverrideSpecial: passwordDataV2.OverrideSpecial, + Result: passwordDataV2.Result, + Special: passwordDataV2.Special, + Upper: passwordDataV2.Upper, + } + + // Set the duplicated data now so we can easily return early below. + // The BcryptHash value will be adjusted later if it is incorrect. + resp.Diagnostics.Append(resp.State.Set(ctx, passwordDataV3)...) + + if resp.Diagnostics.HasError() { + return + } + + // If the BcryptHash value does not correctly verify against the Result + // value we should regenerate it. + err := bcrypt.CompareHashAndPassword([]byte(passwordDataV2.BcryptHash.Value), []byte(passwordDataV2.Result.Value)) + + // If the hash matched the password, there is nothing to do. + if err == nil { + return + } + + if !errors.Is(err, bcrypt.ErrMismatchedHashAndPassword) { + resp.Diagnostics.AddError( + "Version 3 State Upgrade Error", + "An unexpected error occurred when comparing the state version 2 password and bcrypt hash. "+ + "This is always an issue in the provider and should be reported to the provider developers.\n\n"+ + "Original Error: "+err.Error(), + ) + return + } + + // Regenerate the BcryptHash value. + newBcryptHash, err := bcrypt.GenerateFromPassword([]byte(passwordDataV2.Result.Value), bcrypt.DefaultCost) + + if err != nil { + resp.Diagnostics.AddError( + "Version 3 State Upgrade Error", + "An unexpected error occurred when generating a new password bcrypt hash. "+ + "Check the error below and ensure the system executing Terraform can properly generate randomness.\n\n"+ + "Original Error: "+err.Error(), + ) + return + } + + passwordDataV3.BcryptHash.Value = string(newBcryptHash) + + resp.Diagnostics.Append(resp.State.Set(ctx, passwordDataV3)...) +} + func generateHash(toHash string) (string, error) { hash, err := bcrypt.GenerateFromPassword([]byte(toHash), bcrypt.DefaultCost) return string(hash), err } +func passwordSchemaV3() tfsdk.Schema { + return tfsdk.Schema{ + Version: 3, + Description: "Identical to [random_string](string.html) with the exception that the result is " + + "treated as sensitive and, thus, _not_ displayed in console output. Read more about sensitive " + + "data handling in the " + + "[Terraform documentation](https://www.terraform.io/docs/language/state/sensitive-data.html).\n\n" + + "This resource *does* use a cryptographic random number generator.", + Attributes: map[string]tfsdk.Attribute{ + "keepers": { + Description: "Arbitrary map of values that, when changed, will trigger recreation of " + + "resource. See [the main provider documentation](../index.html) for more information.", + Type: types.MapType{ + ElemType: types.StringType, + }, + Optional: true, + PlanModifiers: []tfsdk.AttributePlanModifier{ + planmodifiers.RequiresReplaceIfValuesNotNull(), + }, + }, + + "length": { + Description: "The length of the string desired. The minimum value for length is 1 and, length " + + "must also be >= (`min_upper` + `min_lower` + `min_numeric` + `min_special`).", + Type: types.Int64Type, + Required: true, + PlanModifiers: []tfsdk.AttributePlanModifier{ + resource.RequiresReplace(), + }, + Validators: []tfsdk.AttributeValidator{ + int64validator.AtLeast(1), + int64validator.AtLeastSumOf( + path.MatchRoot("min_upper"), + path.MatchRoot("min_lower"), + path.MatchRoot("min_numeric"), + path.MatchRoot("min_special"), + ), + }, + }, + + "special": { + Description: "Include special characters in the result. These are `!@#$%&*()-_=+[]{}<>:?`. Default value is `true`.", + Type: types.BoolType, + Optional: true, + Computed: true, + PlanModifiers: []tfsdk.AttributePlanModifier{ + planmodifiers.DefaultValue(types.Bool{Value: true}), + planmodifiers.RequiresReplace(), + }, + }, + + "upper": { + Description: "Include uppercase alphabet characters in the result. Default value is `true`.", + Type: types.BoolType, + Optional: true, + Computed: true, + PlanModifiers: []tfsdk.AttributePlanModifier{ + planmodifiers.DefaultValue(types.Bool{Value: true}), + planmodifiers.RequiresReplace(), + }, + }, + + "lower": { + Description: "Include lowercase alphabet characters in the result. Default value is `true`.", + Type: types.BoolType, + Optional: true, + Computed: true, + PlanModifiers: []tfsdk.AttributePlanModifier{ + planmodifiers.DefaultValue(types.Bool{Value: true}), + planmodifiers.RequiresReplace(), + }, + }, + + "number": { + Description: "Include numeric characters in the result. Default value is `true`. " + + "**NOTE**: This is deprecated, use `numeric` instead.", + Type: types.BoolType, + Optional: true, + Computed: true, + PlanModifiers: []tfsdk.AttributePlanModifier{ + planmodifiers.NumberNumericAttributePlanModifier(), + planmodifiers.RequiresReplace(), + }, + DeprecationMessage: "**NOTE**: This is deprecated, use `numeric` instead.", + }, + + "numeric": { + Description: "Include numeric characters in the result. Default value is `true`.", + Type: types.BoolType, + Optional: true, + Computed: true, + PlanModifiers: []tfsdk.AttributePlanModifier{ + planmodifiers.NumberNumericAttributePlanModifier(), + planmodifiers.RequiresReplace(), + }, + }, + + "min_numeric": { + Description: "Minimum number of numeric characters in the result. Default value is `0`.", + Type: types.Int64Type, + Optional: true, + Computed: true, + PlanModifiers: []tfsdk.AttributePlanModifier{ + planmodifiers.DefaultValue(types.Int64{Value: 0}), + planmodifiers.RequiresReplace(), + }, + }, + + "min_upper": { + Description: "Minimum number of uppercase alphabet characters in the result. Default value is `0`.", + Type: types.Int64Type, + Optional: true, + Computed: true, + PlanModifiers: []tfsdk.AttributePlanModifier{ + planmodifiers.DefaultValue(types.Int64{Value: 0}), + planmodifiers.RequiresReplace(), + }, + }, + + "min_lower": { + Description: "Minimum number of lowercase alphabet characters in the result. Default value is `0`.", + Type: types.Int64Type, + Optional: true, + Computed: true, + PlanModifiers: []tfsdk.AttributePlanModifier{ + planmodifiers.DefaultValue(types.Int64{Value: 0}), + planmodifiers.RequiresReplace(), + }, + }, + + "min_special": { + Description: "Minimum number of special characters in the result. Default value is `0`.", + Type: types.Int64Type, + Optional: true, + Computed: true, + PlanModifiers: []tfsdk.AttributePlanModifier{ + planmodifiers.DefaultValue(types.Int64{Value: 0}), + planmodifiers.RequiresReplace(), + }, + }, + + "override_special": { + Description: "Supply your own list of special characters to use for string generation. This " + + "overrides the default character list in the special argument. The `special` argument must " + + "still be set to true for any overwritten characters to be used in generation.", + Type: types.StringType, + Optional: true, + Computed: true, + PlanModifiers: []tfsdk.AttributePlanModifier{ + planmodifiers.DefaultValue(types.String{Value: ""}), + resource.RequiresReplace(), + }, + }, + + "result": { + Description: "The generated random string.", + Type: types.StringType, + Computed: true, + Sensitive: true, + PlanModifiers: []tfsdk.AttributePlanModifier{ + resource.UseStateForUnknown(), + }, + }, + + "bcrypt_hash": { + Description: "A bcrypt hash of the generated random string.", + Type: types.StringType, + Computed: true, + Sensitive: true, + PlanModifiers: []tfsdk.AttributePlanModifier{ + resource.UseStateForUnknown(), + }, + }, + + "id": { + Description: "A static value used internally by Terraform, this should not be referenced in configurations.", + Computed: true, + Type: types.StringType, + PlanModifiers: []tfsdk.AttributePlanModifier{ + resource.UseStateForUnknown(), + }, + }, + }, + } +} + func passwordSchemaV2() tfsdk.Schema { return tfsdk.Schema{ Version: 2, @@ -772,6 +1040,24 @@ func passwordSchemaV0() tfsdk.Schema { } } +type passwordModelV3 struct { + ID types.String `tfsdk:"id"` + Keepers types.Map `tfsdk:"keepers"` + Length types.Int64 `tfsdk:"length"` + Special types.Bool `tfsdk:"special"` + Upper types.Bool `tfsdk:"upper"` + Lower types.Bool `tfsdk:"lower"` + Number types.Bool `tfsdk:"number"` + Numeric types.Bool `tfsdk:"numeric"` + MinNumeric types.Int64 `tfsdk:"min_numeric"` + MinUpper types.Int64 `tfsdk:"min_upper"` + MinLower types.Int64 `tfsdk:"min_lower"` + MinSpecial types.Int64 `tfsdk:"min_special"` + OverrideSpecial types.String `tfsdk:"override_special"` + Result types.String `tfsdk:"result"` + BcryptHash types.String `tfsdk:"bcrypt_hash"` +} + type passwordModelV2 struct { ID types.String `tfsdk:"id"` Keepers types.Map `tfsdk:"keepers"` diff --git a/internal/provider/resource_password_test.go b/internal/provider/resource_password_test.go index 62636d06..c880f47b 100644 --- a/internal/provider/resource_password_test.go +++ b/internal/provider/resource_password_test.go @@ -2,6 +2,7 @@ package provider import ( "context" + "errors" "fmt" "regexp" "runtime" @@ -14,9 +15,54 @@ import ( "github.com/hashicorp/terraform-plugin-go/tftypes" "github.com/hashicorp/terraform-plugin-sdk/v2/helper/resource" "github.com/hashicorp/terraform-plugin-sdk/v2/terraform" + "github.com/terraform-providers/terraform-provider-random/internal/random" "golang.org/x/crypto/bcrypt" ) +func TestGenerateHash(t *testing.T) { + t.Parallel() + + testCases := map[string]struct { + input random.StringParams + }{ + "defaults": { + input: random.StringParams{ + Length: 32, // Required + Lower: true, + Numeric: true, + Special: true, + Upper: true, + }, + }, + } + + for name, testCase := range testCases { + name, testCase := name, testCase + + t.Run(name, func(t *testing.T) { + t.Parallel() + + randomBytes, err := random.CreateString(testCase.input) + + if err != nil { + t.Fatalf("unexpected random.CreateString error: %s", err) + } + + hash, err := generateHash(string(randomBytes)) + + if err != nil { + t.Fatalf("unexpected generateHash error: %s", err) + } + + err = bcrypt.CompareHashAndPassword([]byte(hash), randomBytes) + + if err != nil { + t.Fatalf("unexpected bcrypt.CompareHashAndPassword error: %s", err) + } + }) + } +} + func TestAccResourcePassword(t *testing.T) { resource.UnitTest(t, resource.TestCase{ ProtoV5ProviderFactories: protoV5ProviderFactories(), @@ -54,6 +100,96 @@ func TestAccResourcePassword(t *testing.T) { }) } +func TestAccResourcePassword_BcryptHash(t *testing.T) { + t.Parallel() + + var result, bcryptHash string + + resource.UnitTest(t, resource.TestCase{ + ProtoV5ProviderFactories: protoV5ProviderFactories(), + Steps: []resource.TestStep{ + { + Config: `resource "random_password" "test" { + length = 12 + }`, + Check: resource.ComposeTestCheckFunc( + testExtractResourceAttr("random_password.test", "bcrypt_hash", &bcryptHash), + testExtractResourceAttr("random_password.test", "result", &result), + testBcryptHashValid(&bcryptHash, &result), + ), + }, + }, + }) +} + +// TestAccResourcePassword_BcryptHash_FromVersion3_3_2 verifies behaviour when +// upgrading state from schema V2 to V3 without a bcrypt_hash update. +func TestAccResourcePassword_BcryptHash_FromVersion3_3_2(t *testing.T) { + var bcryptHash1, bcryptHash2, result1, result2 string + + resource.ParallelTest(t, resource.TestCase{ + Steps: []resource.TestStep{ + { + ExternalProviders: providerVersion332(), + Config: `resource "random_password" "test" { + length = 12 + }`, + Check: resource.ComposeTestCheckFunc( + testExtractResourceAttr("random_password.test", "bcrypt_hash", &bcryptHash1), + testExtractResourceAttr("random_password.test", "result", &result1), + testBcryptHashValid(&bcryptHash1, &result1), + ), + }, + { + ProtoV5ProviderFactories: protoV5ProviderFactories(), + Config: `resource "random_password" "test" { + length = 12 + }`, + Check: resource.ComposeTestCheckFunc( + testExtractResourceAttr("random_password.test", "bcrypt_hash", &bcryptHash2), + testExtractResourceAttr("random_password.test", "result", &result2), + testCheckAttributeValuesEqual(&bcryptHash1, &bcryptHash2), + testBcryptHashValid(&bcryptHash2, &result2), + ), + }, + }, + }) +} + +// TestAccResourcePassword_BcryptHash_FromVersion3_4_2 verifies behaviour when +// upgrading state from schema V2 to V3 with an expected bcrypt_hash update. +func TestAccResourcePassword_BcryptHash_FromVersion3_4_2(t *testing.T) { + var bcryptHash1, bcryptHash2, result1, result2 string + + resource.ParallelTest(t, resource.TestCase{ + Steps: []resource.TestStep{ + { + ExternalProviders: providerVersion342(), + Config: `resource "random_password" "test" { + length = 12 + }`, + Check: resource.ComposeTestCheckFunc( + testExtractResourceAttr("random_password.test", "bcrypt_hash", &bcryptHash1), + testExtractResourceAttr("random_password.test", "result", &result1), + testBcryptHashInvalid(&bcryptHash1, &result1), + ), + }, + { + ProtoV5ProviderFactories: protoV5ProviderFactories(), + Config: `resource "random_password" "test" { + length = 12 + }`, + Check: resource.ComposeTestCheckFunc( + testExtractResourceAttr("random_password.test", "bcrypt_hash", &bcryptHash2), + testExtractResourceAttr("random_password.test", "result", &result2), + testCheckAttributeValuesDiffer(&bcryptHash1, &bcryptHash2), + testBcryptHashValid(&bcryptHash2, &result2), + ), + }, + }, + }) +} + func TestAccResourcePassword_Override(t *testing.T) { resource.UnitTest(t, resource.TestCase{ ProtoV5ProviderFactories: protoV5ProviderFactories(), @@ -507,7 +643,7 @@ func TestAccResourcePassword_Min(t *testing.T) { }) } -// TestAccResourcePassword_UpgradeFromVersion2_2_1 verifies behaviour when upgrading state from schema V0 to V2. +// TestAccResourcePassword_UpgradeFromVersion2_2_1 verifies behaviour when upgrading state from schema V0 to V3. func TestAccResourcePassword_UpgradeFromVersion2_2_1(t *testing.T) { resource.Test(t, resource.TestCase{ PreCheck: func() { @@ -586,7 +722,7 @@ func TestAccResourcePassword_UpgradeFromVersion2_2_1(t *testing.T) { }) } -// TestAccResourcePassword_UpgradeFromVersion3_2_0 verifies behaviour when upgrading state from schema V1 to V2. +// TestAccResourcePassword_UpgradeFromVersion3_2_0 verifies behaviour when upgrading state from schema V1 to V3. func TestAccResourcePassword_UpgradeFromVersion3_2_0(t *testing.T) { resource.Test(t, resource.TestCase{ Steps: []resource.TestStep{ @@ -737,7 +873,7 @@ func TestAccResourcePassword_UpgradeFromVersion3_3_2(t *testing.T) { }) } -func TestUpgradePasswordStateV0toV2(t *testing.T) { +func TestUpgradePasswordStateV0toV3(t *testing.T) { raw := tftypes.NewValue(tftypes.Object{}, map[string]tftypes.Value{ "id": tftypes.NewValue(tftypes.String, "none"), "keepers": tftypes.NewValue(tftypes.Map{ElementType: tftypes.String}, nil), @@ -763,13 +899,13 @@ func TestUpgradePasswordStateV0toV2(t *testing.T) { resp := &res.UpgradeStateResponse{ State: tfsdk.State{ - Schema: passwordSchemaV2(), + Schema: passwordSchemaV3(), }, } - upgradePasswordStateV0toV2(context.Background(), req, resp) + upgradePasswordStateV0toV3(context.Background(), req, resp) - expected := passwordModelV2{ + expected := passwordModelV3{ ID: types.String{Value: "none"}, Keepers: types.Map{Null: true, ElemType: types.StringType}, Length: types.Int64{Value: 16}, @@ -786,7 +922,7 @@ func TestUpgradePasswordStateV0toV2(t *testing.T) { Result: types.String{Value: "DZy_3*tnonj%Q%Yx"}, } - actual := passwordModelV2{} + actual := passwordModelV3{} diags := resp.State.Get(context.Background(), &actual) if diags.HasError() { t.Errorf("error getting state: %v", diags) @@ -805,7 +941,7 @@ func TestUpgradePasswordStateV0toV2(t *testing.T) { } } -func TestUpgradePasswordStateV1toV2(t *testing.T) { +func TestUpgradePasswordStateV1toV3(t *testing.T) { raw := tftypes.NewValue(tftypes.Object{}, map[string]tftypes.Value{ "id": tftypes.NewValue(tftypes.String, "none"), "keepers": tftypes.NewValue(tftypes.Map{ElementType: tftypes.String}, nil), @@ -832,13 +968,13 @@ func TestUpgradePasswordStateV1toV2(t *testing.T) { resp := &res.UpgradeStateResponse{ State: tfsdk.State{ - Schema: passwordSchemaV2(), + Schema: passwordSchemaV3(), }, } - upgradePasswordStateV1toV2(context.Background(), req, resp) + upgradePasswordStateV1toV3(context.Background(), req, resp) - expected := passwordModelV2{ + expected := passwordModelV3{ ID: types.String{Value: "none"}, Keepers: types.Map{Null: true, ElemType: types.StringType}, Length: types.Int64{Value: 16}, @@ -856,7 +992,7 @@ func TestUpgradePasswordStateV1toV2(t *testing.T) { Result: types.String{Value: "DZy_3*tnonj%Q%Yx"}, } - actual := passwordModelV2{} + actual := passwordModelV3{} diags := resp.State.Get(context.Background(), &actual) if diags.HasError() { t.Errorf("error getting state: %v", diags) @@ -867,6 +1003,316 @@ func TestUpgradePasswordStateV1toV2(t *testing.T) { } } +func TestUpgradePasswordStateV2toV3(t *testing.T) { + t.Parallel() + + testCases := map[string]struct { + request res.UpgradeStateRequest + expected *res.UpgradeStateResponse + }{ + "valid-hash": { + request: res.UpgradeStateRequest{ + State: &tfsdk.State{ + Raw: tftypes.NewValue(tftypes.Object{ + AttributeTypes: map[string]tftypes.Type{ + "bcrypt_hash": tftypes.String, + "id": tftypes.String, + "keepers": tftypes.Map{ElementType: tftypes.String}, + "length": tftypes.Number, + "lower": tftypes.Bool, + "min_lower": tftypes.Number, + "min_numeric": tftypes.Number, + "min_special": tftypes.Number, + "min_upper": tftypes.Number, + "number": tftypes.Bool, + "numeric": tftypes.Bool, + "override_special": tftypes.String, + "result": tftypes.String, + "special": tftypes.Bool, + "upper": tftypes.Bool, + }, + }, map[string]tftypes.Value{ + "bcrypt_hash": tftypes.NewValue(tftypes.String, "$2a$10$d9zhEkVg.O1jZ6fEIMRlRuu/vMa0/4UIzeK5joaTBhZJlYiIPhWWa"), + "id": tftypes.NewValue(tftypes.String, "none"), + "keepers": tftypes.NewValue(tftypes.Map{ElementType: tftypes.String}, nil), + "length": tftypes.NewValue(tftypes.Number, 20), + "lower": tftypes.NewValue(tftypes.Bool, true), + "min_lower": tftypes.NewValue(tftypes.Number, 0), + "min_numeric": tftypes.NewValue(tftypes.Number, 0), + "min_special": tftypes.NewValue(tftypes.Number, 0), + "min_upper": tftypes.NewValue(tftypes.Number, 0), + "number": tftypes.NewValue(tftypes.Bool, true), + "numeric": tftypes.NewValue(tftypes.Bool, true), + "override_special": tftypes.NewValue(tftypes.String, ""), + "result": tftypes.NewValue(tftypes.String, "n:um[a9kO&x!L=9og[EM"), + "special": tftypes.NewValue(tftypes.Bool, true), + "upper": tftypes.NewValue(tftypes.Bool, true), + }), + Schema: passwordSchemaV2(), + }, + }, + expected: &res.UpgradeStateResponse{ + State: tfsdk.State{ + Raw: tftypes.NewValue(tftypes.Object{ + AttributeTypes: map[string]tftypes.Type{ + "bcrypt_hash": tftypes.String, + "id": tftypes.String, + "keepers": tftypes.Map{ElementType: tftypes.String}, + "length": tftypes.Number, + "lower": tftypes.Bool, + "min_lower": tftypes.Number, + "min_numeric": tftypes.Number, + "min_special": tftypes.Number, + "min_upper": tftypes.Number, + "number": tftypes.Bool, + "numeric": tftypes.Bool, + "override_special": tftypes.String, + "result": tftypes.String, + "special": tftypes.Bool, + "upper": tftypes.Bool, + }, + }, map[string]tftypes.Value{ + // The difference checking should compare this actual + // value since it should not be updated. + "bcrypt_hash": tftypes.NewValue(tftypes.String, "$2a$10$d9zhEkVg.O1jZ6fEIMRlRuu/vMa0/4UIzeK5joaTBhZJlYiIPhWWa"), + "id": tftypes.NewValue(tftypes.String, "none"), + "keepers": tftypes.NewValue(tftypes.Map{ElementType: tftypes.String}, nil), + "length": tftypes.NewValue(tftypes.Number, 20), + "lower": tftypes.NewValue(tftypes.Bool, true), + "min_lower": tftypes.NewValue(tftypes.Number, 0), + "min_numeric": tftypes.NewValue(tftypes.Number, 0), + "min_special": tftypes.NewValue(tftypes.Number, 0), + "min_upper": tftypes.NewValue(tftypes.Number, 0), + "number": tftypes.NewValue(tftypes.Bool, true), + "numeric": tftypes.NewValue(tftypes.Bool, true), + "override_special": tftypes.NewValue(tftypes.String, ""), + "result": tftypes.NewValue(tftypes.String, "n:um[a9kO&x!L=9og[EM"), + "special": tftypes.NewValue(tftypes.Bool, true), + "upper": tftypes.NewValue(tftypes.Bool, true), + }), + Schema: passwordSchemaV3(), + }, + }, + }, + "invalid-hash": { + request: res.UpgradeStateRequest{ + State: &tfsdk.State{ + Raw: tftypes.NewValue(tftypes.Object{ + AttributeTypes: map[string]tftypes.Type{ + "bcrypt_hash": tftypes.String, + "id": tftypes.String, + "keepers": tftypes.Map{ElementType: tftypes.String}, + "length": tftypes.Number, + "lower": tftypes.Bool, + "min_lower": tftypes.Number, + "min_numeric": tftypes.Number, + "min_special": tftypes.Number, + "min_upper": tftypes.Number, + "number": tftypes.Bool, + "numeric": tftypes.Bool, + "override_special": tftypes.String, + "result": tftypes.String, + "special": tftypes.Bool, + "upper": tftypes.Bool, + }, + }, map[string]tftypes.Value{ + "bcrypt_hash": tftypes.NewValue(tftypes.String, "$2a$10$bPOZGBpGe4XIgbpVaWNya.dz/HsU1GDLjuEposH2wf.vUO5rA1wXe"), + "id": tftypes.NewValue(tftypes.String, "none"), + "keepers": tftypes.NewValue(tftypes.Map{ElementType: tftypes.String}, nil), + "length": tftypes.NewValue(tftypes.Number, 20), + "lower": tftypes.NewValue(tftypes.Bool, true), + "min_lower": tftypes.NewValue(tftypes.Number, 0), + "min_numeric": tftypes.NewValue(tftypes.Number, 0), + "min_special": tftypes.NewValue(tftypes.Number, 0), + "min_upper": tftypes.NewValue(tftypes.Number, 0), + "number": tftypes.NewValue(tftypes.Bool, true), + "numeric": tftypes.NewValue(tftypes.Bool, true), + "override_special": tftypes.NewValue(tftypes.String, ""), + "result": tftypes.NewValue(tftypes.String, "$7r>NiN4Z%uAxpU]:DuB"), + "special": tftypes.NewValue(tftypes.Bool, true), + "upper": tftypes.NewValue(tftypes.Bool, true), + }), + Schema: passwordSchemaV2(), + }, + }, + expected: &res.UpgradeStateResponse{ + State: tfsdk.State{ + Raw: tftypes.NewValue(tftypes.Object{ + AttributeTypes: map[string]tftypes.Type{ + "bcrypt_hash": tftypes.String, + "id": tftypes.String, + "keepers": tftypes.Map{ElementType: tftypes.String}, + "length": tftypes.Number, + "lower": tftypes.Bool, + "min_lower": tftypes.Number, + "min_numeric": tftypes.Number, + "min_special": tftypes.Number, + "min_upper": tftypes.Number, + "number": tftypes.Bool, + "numeric": tftypes.Bool, + "override_special": tftypes.String, + "result": tftypes.String, + "special": tftypes.Bool, + "upper": tftypes.Bool, + }, + }, map[string]tftypes.Value{ + // bcrypt_hash is randomly generated, so the difference checking + // will ignore this value. + "bcrypt_hash": tftypes.NewValue(tftypes.String, nil), + "id": tftypes.NewValue(tftypes.String, "none"), + "keepers": tftypes.NewValue(tftypes.Map{ElementType: tftypes.String}, nil), + "length": tftypes.NewValue(tftypes.Number, 20), + "lower": tftypes.NewValue(tftypes.Bool, true), + "min_lower": tftypes.NewValue(tftypes.Number, 0), + "min_numeric": tftypes.NewValue(tftypes.Number, 0), + "min_special": tftypes.NewValue(tftypes.Number, 0), + "min_upper": tftypes.NewValue(tftypes.Number, 0), + "number": tftypes.NewValue(tftypes.Bool, true), + "numeric": tftypes.NewValue(tftypes.Bool, true), + "override_special": tftypes.NewValue(tftypes.String, ""), + "result": tftypes.NewValue(tftypes.String, "$7r>NiN4Z%uAxpU]:DuB"), + "special": tftypes.NewValue(tftypes.Bool, true), + "upper": tftypes.NewValue(tftypes.Bool, true), + }), + Schema: passwordSchemaV3(), + }, + }, + }, + } + + for name, testCase := range testCases { + name, testCase := name, testCase + + t.Run(name, func(t *testing.T) { + t.Parallel() + + got := res.UpgradeStateResponse{ + State: tfsdk.State{ + Schema: testCase.expected.State.Schema, + }, + } + + upgradePasswordStateV2toV3(context.Background(), testCase.request, &got) + + // Since bcrypt_hash is generated, this test is very involved to + // ensure the test case is set up properly and the generated + // value is removed to prevent false positive differences. + var err error + var requestBcryptHash, requestResult, expectedBcryptHash, gotBcryptHash, gotResult string + + bcryptHashPath := tftypes.NewAttributePath().WithAttributeName("bcrypt_hash") + resultPath := tftypes.NewAttributePath().WithAttributeName("result") + + requestBcryptHashValue, err := testTftypesValueAtPath(testCase.request.State.Raw, bcryptHashPath) + + if err != nil { + t.Fatalf("unexpected error getting request bcrypt_hash value: %s", err) + } + + if err := requestBcryptHashValue.As(&requestBcryptHash); err != nil { + t.Fatalf("unexpected error converting request bcrypt_hash to string: %s", err) + } + + requestResultValue, err := testTftypesValueAtPath(testCase.request.State.Raw, resultPath) + + if err != nil { + t.Fatalf("unexpected error getting request result value: %s", err) + } + + if err := requestResultValue.As(&requestResult); err != nil { + t.Fatalf("unexpected error converting request result to string: %s", err) + } + + expectedBcryptHashValue, err := testTftypesValueAtPath(testCase.expected.State.Raw, bcryptHashPath) + + if err != nil { + t.Fatalf("unexpected error getting expected bcrypt_hash value: %s", err) + } + + if err := expectedBcryptHashValue.As(&expectedBcryptHash); err != nil { + t.Fatalf("unexpected error converting expected bcrypt_hash to string: %s", err) + } + + gotBcryptHashValue, err := testTftypesValueAtPath(got.State.Raw, bcryptHashPath) + + if err != nil { + t.Fatalf("unexpected error getting got bcrypt_hash value: %s", err) + } + + if err := gotBcryptHashValue.As(&gotBcryptHash); err != nil { + t.Fatalf("unexpected error converting got bcrypt_hash to string: %s", err) + } + + gotResultValue, err := testTftypesValueAtPath(got.State.Raw, resultPath) + + if err != nil { + t.Fatalf("unexpected error getting got result value: %s", err) + } + + if err := gotResultValue.As(&gotResult); err != nil { + t.Fatalf("unexpected error converting got result to string: %s", err) + } + + err = bcrypt.CompareHashAndPassword([]byte(requestBcryptHash), []byte(requestResult)) + + // If the request bcrypt_hash was valid, it should be in expected + // and got. Otherwise, it should be regenerated which will be a + // random value which must be stripped to prevent false positives. + if err == nil { + // Ensure the test case is valid. + if !requestBcryptHashValue.Equal(expectedBcryptHashValue) { + t.Fatal("expected request bcrypt_hash in expected") + } + + // Ensure the request bcrypt_hash was not modified. + if !requestBcryptHashValue.Equal(gotBcryptHashValue) { + t.Fatal("expected request bcrypt_hash in got") + } + } else { + // If we got a different error than mismatched hash, then the + // test case might not be valid. + if !errors.Is(err, bcrypt.ErrMismatchedHashAndPassword) { + t.Fatalf("unexpected request bcrypt_hash error: %s", err) + } + + // Ensure the test case has null values on both sides as a + // regenerated bcrypt_hash cannot be equality compared. + if !expectedBcryptHashValue.IsNull() { + t.Fatal("expected null bcrypt_hash in expected") + } + + // Prevent differences from the got bcrypt_path being randomly + // generated. + got.State.Raw, err = tftypes.Transform( + got.State.Raw, + func(path *tftypes.AttributePath, value tftypes.Value) (tftypes.Value, error) { + // Purposefully set bcrypt_hash value to nil. + if path.Equal(bcryptHashPath) { + return tftypes.NewValue(tftypes.String, nil), nil + } + + return value, nil + }, + ) + + if err != nil { + t.Fatalf("unexpected error transforming got: %s", err) + } + } + + // The got bcrypt_hash should always be valid. + if err := bcrypt.CompareHashAndPassword([]byte(gotBcryptHash), []byte(gotResult)); err != nil { + t.Errorf("unexpected error comparing got bcrypt_hash and result: %s", err) + } + + // Ensure all state values are checked. + if diff := cmp.Diff(*testCase.expected, got); diff != "" { + t.Errorf("unexpected difference: %s", diff) + } + }) + } +} + func TestAccResourcePassword_NumberNumericErrors(t *testing.T) { resource.UnitTest(t, resource.TestCase{ ProtoV5ProviderFactories: protoV5ProviderFactories(), @@ -1625,3 +2071,37 @@ func TestAccResourcePassword_Keepers_FrameworkMigration_NullMapValueToValue(t *t }, }) } + +func testBcryptHashInvalid(hash *string, password *string) resource.TestCheckFunc { + return func(_ *terraform.State) error { + if hash == nil || *hash == "" { + return fmt.Errorf("expected hash value") + } + + if password == nil || *password == "" { + return fmt.Errorf("expected password value") + } + + err := bcrypt.CompareHashAndPassword([]byte(*hash), []byte(*password)) + + if !errors.Is(err, bcrypt.ErrMismatchedHashAndPassword) { + return fmt.Errorf("unexpected error: %s", err) + } + + return nil + } +} + +func testBcryptHashValid(hash *string, password *string) resource.TestCheckFunc { + return func(_ *terraform.State) error { + if hash == nil || *hash == "" { + return fmt.Errorf("expected hash value") + } + + if password == nil || *password == "" { + return fmt.Errorf("expected password value") + } + + return bcrypt.CompareHashAndPassword([]byte(*hash), []byte(*password)) + } +} diff --git a/internal/provider/tftypes_test.go b/internal/provider/tftypes_test.go new file mode 100644 index 00000000..3913249c --- /dev/null +++ b/internal/provider/tftypes_test.go @@ -0,0 +1,23 @@ +package provider + +import ( + "fmt" + + "github.com/hashicorp/terraform-plugin-go/tftypes" +) + +func testTftypesValueAtPath(value tftypes.Value, path *tftypes.AttributePath) (tftypes.Value, error) { + valueAtPathRaw, remaining, err := tftypes.WalkAttributePath(value, path) + + if err != nil { + return tftypes.Value{}, fmt.Errorf("unexpected error getting %s: %s (%s remaining)", path, err, remaining) + } + + valueAtPath, ok := valueAtPathRaw.(tftypes.Value) + + if !ok { + return tftypes.Value{}, fmt.Errorf("unexpected type converting %s to tftypes.Value, got: %T", path, valueAtPathRaw) + } + + return valueAtPath, nil +}