From d475169a5130b938fb5a57027fa05f241ea5be9d Mon Sep 17 00:00:00 2001 From: Ellie Date: Mon, 14 Aug 2023 08:45:30 -0500 Subject: [PATCH] prevent overwriting of schema and password_policy values on update of config (#75) --- path_config.go | 14 +++++++++ path_config_test.go | 72 +++++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 86 insertions(+) diff --git a/path_config.go b/path_config.go index 1f8583b..ee36c30 100644 --- a/path_config.go +++ b/path_config.go @@ -118,6 +118,13 @@ func (b *backend) configCreateUpdateOperation(ctx context.Context, req *logical. passLength := rawPassLength.(int) schema := fieldData.Get("schema").(string) + _, schemaChanged := fieldData.Raw["schema"] + + // if update operation and schema not updated in raw payload, keep existing schema + if existing != nil && !schemaChanged { + schema = conf.LDAP.Schema + } + if schema == "" { return nil, errors.New("schema is required") } @@ -135,6 +142,13 @@ func (b *backend) configCreateUpdateOperation(ctx context.Context, req *logical. } passPolicy := fieldData.Get("password_policy").(string) + _, passPolicyChanged := fieldData.Raw["password_policy"] + + // if update operation and password_policy not updated in raw payload, keep existing password_policy + if existing != nil && !passPolicyChanged { + passPolicy = conf.PasswordPolicy + } + if passPolicy != "" && hasPassLen { // If both a password policy and a password length are set, we can't figure out what to do return nil, fmt.Errorf("cannot set both 'password_policy' and 'length'") diff --git a/path_config_test.go b/path_config_test.go index 0d192aa..c0e19c3 100644 --- a/path_config_test.go +++ b/path_config_test.go @@ -375,6 +375,78 @@ func TestConfig_Update(t *testing.T) { validCertificate, resp.Data["certificate"]) } }) + + t.Run("update retains prior schema and password_policy values in storage", func(t *testing.T) { + b, storage := getBackend(false) + defer b.Cleanup(context.Background()) + + initialSchema := "ad" + initialPasswordPolicy := "test_policy" + + data := map[string]interface{}{ + "binddn": "tester", + "schema": initialSchema, + "password_policy": initialPasswordPolicy, + "bindpass": "pa$$w0rd", + "url": "ldap://138.91.247.105", + "certificate": validCertificate, + } + + req := &logical.Request{ + Operation: logical.CreateOperation, + Path: configPath, + Storage: storage, + Data: data, + } + + resp, err := b.HandleRequest(context.Background(), req) + if err != nil || (resp != nil && resp.IsError()) { + t.Fatalf("err:%s resp:%#v\n", err, resp) + } + + // schema and password_policy are intentionally omitted for the update in order + // to test that their values set at creation time is retained. + data = map[string]interface{}{ + "binddn": "newtester", + "bindpass": "pa$$w0rd", + "url": "ldap://138.91.247.105", + } + + req = &logical.Request{ + Operation: logical.UpdateOperation, + Path: configPath, + Storage: storage, + Data: data, + } + + resp, err = b.HandleRequest(context.Background(), req) + if err != nil || (resp != nil && resp.IsError()) { + t.Fatalf("err:%s resp:%#v\n", err, resp) + } + + req = &logical.Request{ + Operation: logical.ReadOperation, + Path: configPath, + Storage: storage, + Data: nil, + } + + resp, err = b.HandleRequest(context.Background(), req) + if err != nil || (resp != nil && resp.IsError()) { + t.Fatalf("err:%s resp:%#v\n", err, resp) + } + + // Assert that schema and password policy are retained in storage after the update + if resp.Data["schema"] != initialSchema { + t.Fatalf("expected schema to be %q after update, got %q", + initialSchema, resp.Data["schema"]) + } + + if resp.Data["password_policy"] != initialPasswordPolicy { + t.Fatalf("expected password_policy to be %q after update, got %q", + initialPasswordPolicy, resp.Data["password_policy"]) + } + }) } func TestConfig_Delete(t *testing.T) {