Skip to content

Commit

Permalink
prevent overwriting of schema and password_policy values on update of…
Browse files Browse the repository at this point in the history
… config (#75)
  • Loading branch information
elliesterner committed Aug 14, 2023
1 parent e253c92 commit d475169
Show file tree
Hide file tree
Showing 2 changed files with 86 additions and 0 deletions.
14 changes: 14 additions & 0 deletions path_config.go
Original file line number Diff line number Diff line change
Expand Up @@ -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")
}
Expand All @@ -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'")
Expand Down
72 changes: 72 additions & 0 deletions path_config_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Expand Down

0 comments on commit d475169

Please sign in to comment.