From 55ebfeba5ae6fd192c996cab1d6c55f3227698c4 Mon Sep 17 00:00:00 2001 From: Calvin Leung Huang Date: Mon, 23 Dec 2019 11:58:38 -0800 Subject: [PATCH 1/3] ldap, okta: fix renewal when login policies are empty --- builtin/credential/ldap/path_login.go | 3 ++- builtin/credential/okta/path_login.go | 2 +- 2 files changed, 3 insertions(+), 2 deletions(-) diff --git a/builtin/credential/ldap/path_login.go b/builtin/credential/ldap/path_login.go index a798a1c2497e..18123323e04f 100644 --- a/builtin/credential/ldap/path_login.go +++ b/builtin/credential/ldap/path_login.go @@ -133,9 +133,10 @@ func (b *backend) pathLoginRenew(ctx context.Context, req *logical.Request, d *f password := req.Auth.InternalData["password"].(string) loginPolicies, resp, groupNames, err := b.Login(ctx, req, username, password) - if len(loginPolicies) == 0 { + if err != nil || (resp != nil && resp.IsError()) { return resp, err } + finalPolicies := cfg.TokenPolicies if len(loginPolicies) > 0 { finalPolicies = append(finalPolicies, loginPolicies...) diff --git a/builtin/credential/okta/path_login.go b/builtin/credential/okta/path_login.go index 8538a7a3c957..b1fb6b857414 100644 --- a/builtin/credential/okta/path_login.go +++ b/builtin/credential/okta/path_login.go @@ -118,7 +118,7 @@ func (b *backend) pathLoginRenew(ctx context.Context, req *logical.Request, d *f } loginPolicies, resp, groupNames, err := b.Login(ctx, req, username, password) - if len(loginPolicies) == 0 { + if err != nil || (resp != nil && resp.IsError()) { return resp, err } From 6f42b5094e99c0430a2105c87264cf2c5f0931ec Mon Sep 17 00:00:00 2001 From: Calvin Leung Huang Date: Wed, 15 Jan 2020 13:45:56 -0800 Subject: [PATCH 2/3] test/policy: add test for login renewal without configured policy --- .../external_tests/policy/no_default_test.go | 94 ++++++++++++++++++- 1 file changed, 89 insertions(+), 5 deletions(-) diff --git a/vault/external_tests/policy/no_default_test.go b/vault/external_tests/policy/no_default_test.go index a296d1364b1a..d4587547e011 100644 --- a/vault/external_tests/policy/no_default_test.go +++ b/vault/external_tests/policy/no_default_test.go @@ -1,10 +1,8 @@ package policy -// This is TODO once tokenhelper is added to ldaputil -/* - import ( "testing" + "time" "github.com/go-test/deep" "github.com/hashicorp/go-hclog" @@ -15,7 +13,7 @@ import ( "github.com/hashicorp/vault/vault" ) -func TestNoDefaultPolicy(t *testing.T) { +func TestPolicy_NoDefaultPolicy(t *testing.T) { var err error coreConfig := &vault.CoreConfig{ DisableMlock: true, @@ -86,4 +84,90 @@ func TestNoDefaultPolicy(t *testing.T) { t.Fatal(diff) } } -*/ + +func TestPolicy_NoConfiguredPolicy(t *testing.T) { + var err error + coreConfig := &vault.CoreConfig{ + DisableMlock: true, + DisableCache: true, + Logger: hclog.NewNullLogger(), + CredentialBackends: map[string]logical.Factory{ + "ldap": ldap.Factory, + }, + } + + cluster := vault.NewTestCluster(t, coreConfig, &vault.TestClusterOptions{ + HandlerFunc: vaulthttp.Handler, + }) + + cluster.Start() + defer cluster.Cleanup() + + cores := cluster.Cores + + vault.TestWaitActive(t, cores[0].Core) + + client := cores[0].Client + + err = client.Sys().EnableAuthWithOptions("ldap", &api.EnableAuthOptions{ + Type: "ldap", + }) + if err != nil { + t.Fatal(err) + } + + // Configure LDAP auth backend + secret, err := client.Logical().Write("auth/ldap/config", map[string]interface{}{ + "url": "ldap://ldap.forumsys.com", + "userattr": "uid", + "userdn": "dc=example,dc=com", + "groupdn": "dc=example,dc=com", + "binddn": "cn=read-only-admin,dc=example,dc=com", + "token_ttl": "24h", + }) + if err != nil { + t.Fatal(err) + } + + // Create a local user in LDAP without any policies configured + secret, err = client.Logical().Write("auth/ldap/users/tesla", map[string]interface{}{}) + if err != nil { + t.Fatal(err) + } + + // Login with LDAP and create a token + secret, err = client.Logical().Write("auth/ldap/login/tesla", map[string]interface{}{ + "password": "password", + }) + if err != nil { + t.Fatal(err) + } + token := secret.Auth.ClientToken + + // Lookup the token to get the entity ID + secret, err = client.Auth().Token().Lookup(token) + if err != nil { + t.Fatal(err) + } + + if diff := deep.Equal(secret.Data["policies"], []interface{}{"default"}); diff != nil { + t.Fatal(diff) + } + + // Sleep a bit to let the lease elapse + time.Sleep(3 * time.Second) + + // Renew the token + secret, err = client.Logical().Write("auth/token/renew", map[string]interface{}{ + "token": token, + }) + if err != nil { + t.Fatal(err) + } + + // Verify that the lease renewal extended the duration properly. We give it a one + // second leeway to prevent test failure in case the response is delayed. + if secret.Auth.LeaseDuration <= 86399 { + t.Fatalf("failed to renew lease, got: %v", secret.Auth.LeaseDuration) + } +} From 95e7bea4cf599841faa358375facc52b42d46162 Mon Sep 17 00:00:00 2001 From: Calvin Leung Huang Date: Wed, 15 Jan 2020 14:43:47 -0800 Subject: [PATCH 3/3] test/policy: remove external dependency on tests, refactor lease duration check --- .../external_tests/policy/no_default_test.go | 62 +++++++++++-------- 1 file changed, 36 insertions(+), 26 deletions(-) diff --git a/vault/external_tests/policy/no_default_test.go b/vault/external_tests/policy/no_default_test.go index d4587547e011..fcba0d011028 100644 --- a/vault/external_tests/policy/no_default_test.go +++ b/vault/external_tests/policy/no_default_test.go @@ -8,6 +8,7 @@ import ( "github.com/hashicorp/go-hclog" "github.com/hashicorp/vault/api" "github.com/hashicorp/vault/builtin/credential/ldap" + ldaphelper "github.com/hashicorp/vault/helper/testhelpers/ldap" vaulthttp "github.com/hashicorp/vault/http" "github.com/hashicorp/vault/sdk/logical" "github.com/hashicorp/vault/vault" @@ -45,12 +46,17 @@ func TestPolicy_NoDefaultPolicy(t *testing.T) { } // Configure LDAP auth backend - secret, err := client.Logical().Write("auth/ldap/config", map[string]interface{}{ - "url": "ldap://ldap.forumsys.com", - "userattr": "uid", - "userdn": "dc=example,dc=com", - "groupdn": "dc=example,dc=com", - "binddn": "cn=read-only-admin,dc=example,dc=com", + cleanup, cfg := ldaphelper.PrepareTestContainer(t, "latest") + defer cleanup() + + _, err = client.Logical().Write("auth/ldap/config", map[string]interface{}{ + "url": cfg.Url, + "userattr": cfg.UserAttr, + "userdn": cfg.UserDN, + "groupdn": cfg.GroupDN, + "groupattr": cfg.GroupAttr, + "binddn": cfg.BindDN, + "bindpass": cfg.BindPassword, "token_no_default_policy": true, }) if err != nil { @@ -58,7 +64,7 @@ func TestPolicy_NoDefaultPolicy(t *testing.T) { } // Create a local user in LDAP - secret, err = client.Logical().Write("auth/ldap/users/tesla", map[string]interface{}{ + secret, err := client.Logical().Write("auth/ldap/users/hermes conrad", map[string]interface{}{ "policies": "foo", }) if err != nil { @@ -66,8 +72,8 @@ func TestPolicy_NoDefaultPolicy(t *testing.T) { } // Login with LDAP and create a token - secret, err = client.Logical().Write("auth/ldap/login/tesla", map[string]interface{}{ - "password": "password", + secret, err = client.Logical().Write("auth/ldap/login/hermes conrad", map[string]interface{}{ + "password": "hermes", }) if err != nil { t.Fatal(err) @@ -117,12 +123,17 @@ func TestPolicy_NoConfiguredPolicy(t *testing.T) { } // Configure LDAP auth backend - secret, err := client.Logical().Write("auth/ldap/config", map[string]interface{}{ - "url": "ldap://ldap.forumsys.com", - "userattr": "uid", - "userdn": "dc=example,dc=com", - "groupdn": "dc=example,dc=com", - "binddn": "cn=read-only-admin,dc=example,dc=com", + cleanup, cfg := ldaphelper.PrepareTestContainer(t, "latest") + defer cleanup() + + _, err = client.Logical().Write("auth/ldap/config", map[string]interface{}{ + "url": cfg.Url, + "userattr": cfg.UserAttr, + "userdn": cfg.UserDN, + "groupdn": cfg.GroupDN, + "groupattr": cfg.GroupAttr, + "binddn": cfg.BindDN, + "bindpass": cfg.BindPassword, "token_ttl": "24h", }) if err != nil { @@ -130,14 +141,14 @@ func TestPolicy_NoConfiguredPolicy(t *testing.T) { } // Create a local user in LDAP without any policies configured - secret, err = client.Logical().Write("auth/ldap/users/tesla", map[string]interface{}{}) + secret, err := client.Logical().Write("auth/ldap/users/hermes conrad", map[string]interface{}{}) if err != nil { t.Fatal(err) } // Login with LDAP and create a token - secret, err = client.Logical().Write("auth/ldap/login/tesla", map[string]interface{}{ - "password": "password", + secret, err = client.Logical().Write("auth/ldap/login/hermes conrad", map[string]interface{}{ + "password": "hermes", }) if err != nil { t.Fatal(err) @@ -154,20 +165,19 @@ func TestPolicy_NoConfiguredPolicy(t *testing.T) { t.Fatal(diff) } - // Sleep a bit to let the lease elapse - time.Sleep(3 * time.Second) - - // Renew the token + // Renew the token with an increment of 2 hours to ensure that lease renewal + // occurred and can be checked against the default lease duration with a + // big enough delta. secret, err = client.Logical().Write("auth/token/renew", map[string]interface{}{ - "token": token, + "token": token, + "increment": "2h", }) if err != nil { t.Fatal(err) } - // Verify that the lease renewal extended the duration properly. We give it a one - // second leeway to prevent test failure in case the response is delayed. - if secret.Auth.LeaseDuration <= 86399 { + // Verify that the lease renewal extended the duration properly. + if float64(secret.Auth.LeaseDuration) < (1 * time.Hour).Seconds() { t.Fatalf("failed to renew lease, got: %v", secret.Auth.LeaseDuration) } }