auth: store period value on tokens created via login (#7885) (#10244)

* auth: store period value on tokens created via login * test: reduce potentially flaskiness due to ttl check * test: govet on package declaration * changelog++ * Temporarily remove CL entry * Add back the CL entry Co-authored-by: Vishal Nayak <[email protected]> Co-authored-by: Calvin Leung Huang <[email protected]>
hashicorp · Oct 27, 2020 · 676336c · 676336c
1 parent ae0e6a0
commit 676336c
Show file tree

Hide file tree

Showing 4 changed files with 171 additions and 57 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -3,6 +3,7 @@
 CHANGES:
 
 * agent: Agent now properly returns a non-zero exit code on error, such as one due to template rendering failure. Using `error_on_missing_key` in the template config will cause agent to immediately exit on failure. In order to make agent properly exit due to continuous failure from template rendering errors, the old behavior of indefinitely restarting the template server is now changed to exit once the default retry attempt of 12 times (with exponential backoff) gets exhausted. [[GH-9670](https://github.com/hashicorp/vault/pull/9670)]  
+* token: Periodic tokens generated by auth methods will have the period value stored in its token entry. [[GH-7885](https://github.com/hashicorp/vault/pull/7885)]
 
 FEATURES:
 

diff --git a/vault/request_handling.go b/vault/request_handling.go
@@ -1304,6 +1304,7 @@ func (c *Core) RegisterAuth(ctx context.Context, tokenTTL time.Duration, path st
 		Policies:       auth.TokenPolicies,
 		NamespaceID:    ns.ID,
 		ExplicitMaxTTL: auth.ExplicitMaxTTL,
+		Period:         auth.Period,
 		Type:           auth.TokenType,
 	}
 

diff --git a/vault/request_handling_test.go b/vault/request_handling_test.go
@@ -6,7 +6,9 @@ import (
 	"time"
 
 	"github.com/armon/go-metrics"
+	"github.com/go-test/deep"
 	uuid "github.com/hashicorp/go-uuid"
+	"github.com/hashicorp/vault/builtin/credential/approle"
 	credUserpass "github.com/hashicorp/vault/builtin/credential/userpass"
 	"github.com/hashicorp/vault/helper/namespace"
 	"github.com/hashicorp/vault/sdk/logical"
@@ -147,6 +149,147 @@ func TestRequestHandling_LoginWrapping(t *testing.T) {
 	}
 }
 
+func TestRequestHandling_Login_PeriodicToken(t *testing.T) {
+	core, _, root := TestCoreUnsealed(t)
+
+	if err := core.loadMounts(namespace.RootContext(nil)); err != nil {
+		t.Fatalf("err: %v", err)
+	}
+
+	core.credentialBackends["approle"] = approle.Factory
+
+	// Enable approle
+	req := &logical.Request{
+		Path:        "sys/auth/approle",
+		ClientToken: root,
+		Operation:   logical.UpdateOperation,
+		Data: map[string]interface{}{
+			"type": "approle",
+		},
+		Connection: &logical.Connection{},
+	}
+	resp, err := core.HandleRequest(namespace.RootContext(nil), req)
+	if err != nil {
+		t.Fatalf("err: %v", err)
+	}
+	if resp != nil {
+		t.Fatalf("bad: %#v", resp)
+	}
+
+	// Create role
+	req.Path = "auth/approle/role/role-period"
+	req.Data = map[string]interface{}{
+		"period": "5s",
+	}
+	_, err = core.HandleRequest(namespace.RootContext(nil), req)
+	if err != nil {
+		t.Fatalf("err: %v", err)
+	}
+
+	// Get role ID
+	req.Path = "auth/approle/role/role-period/role-id"
+	req.Operation = logical.ReadOperation
+	req.Data = nil
+	resp, err = core.HandleRequest(namespace.RootContext(nil), req)
+	if err != nil {
+		t.Fatalf("err: %v", err)
+	}
+	if resp == nil || resp.Data == nil {
+		t.Fatalf("bad: %#v", resp)
+	}
+	roleID := resp.Data["role_id"]
+
+	// Get secret ID
+	req.Path = "auth/approle/role/role-period/secret-id"
+	req.Operation = logical.UpdateOperation
+	req.Data = nil
+	resp, err = core.HandleRequest(namespace.RootContext(nil), req)
+	if err != nil {
+		t.Fatalf("err: %v", err)
+	}
+	if resp == nil || resp.Data == nil {
+		t.Fatalf("bad: %#v", resp)
+	}
+	secretID := resp.Data["secret_id"]
+
+	// Perform login
+	req = &logical.Request{
+		Path:      "auth/approle/login",
+		Operation: logical.UpdateOperation,
+		Data: map[string]interface{}{
+			"role_id":   roleID,
+			"secret_id": secretID,
+		},
+		Connection: &logical.Connection{},
+	}
+	resp, err = core.HandleRequest(namespace.RootContext(nil), req)
+	if err != nil {
+		t.Fatalf("err: %v", err)
+	}
+	if resp == nil || resp.Auth == nil {
+		t.Fatalf("bad: %v", resp)
+	}
+	loginToken := resp.Auth.ClientToken
+	entityID := resp.Auth.EntityID
+	accessor := resp.Auth.Accessor
+
+	// Perform token lookup on the generated token
+	req = &logical.Request{
+		Path:        "auth/token/lookup",
+		Operation:   logical.UpdateOperation,
+		ClientToken: root,
+		Data: map[string]interface{}{
+			"token": loginToken,
+		},
+		Connection: &logical.Connection{},
+	}
+	resp, err = core.HandleRequest(namespace.RootContext(nil), req)
+	if err != nil {
+		t.Fatalf("err: %v", err)
+	}
+	if resp == nil {
+		t.Fatalf("bad: %v", resp)
+	}
+	if resp.Data == nil {
+		t.Fatalf("bad: %#v", resp)
+	}
+
+	if resp.Data["creation_time"].(int64) == 0 {
+		t.Fatal("creation time was zero")
+	}
+
+	// Depending on timing of the test this may have ticked down, so reset it
+	// back to the original value as long as it's not expired.
+	if resp.Data["ttl"].(int64) > 0 && resp.Data["ttl"].(int64) < 5 {
+		resp.Data["ttl"] = int64(5)
+	}
+
+	exp := map[string]interface{}{
+		"accessor":         accessor,
+		"creation_time":    resp.Data["creation_time"].(int64),
+		"creation_ttl":     int64(5),
+		"display_name":     "approle",
+		"entity_id":        entityID,
+		"expire_time":      resp.Data["expire_time"].(time.Time),
+		"explicit_max_ttl": int64(0),
+		"id":               loginToken,
+		"issue_time":       resp.Data["issue_time"].(time.Time),
+		"meta":             map[string]string{"role_name": "role-period"},
+		"num_uses":         0,
+		"orphan":           true,
+		"path":             "auth/approle/login",
+		"period":           int64(5),
+		"policies":         []string{"default"},
+		"renewable":        true,
+		"ttl":              int64(5),
+		"type":             "service",
+	}
+
+	if diff := deep.Equal(resp.Data, exp); diff != nil {
+		t.Fatal(diff)
+	}
+}
+
 func labelsMatch(actual, expected map[string]string) bool {
 	for expected_label, expected_val := range expected {
 		if v, ok := actual[expected_label]; ok {

diff --git a/vault/token_store_test.go b/vault/token_store_test.go
@@ -601,10 +601,10 @@ func testMakeTokenDirectly(t testing.TB, ts *TokenStore, te *logical.TokenEntry)
 }
 
 func testMakeServiceTokenViaCore(t testing.TB, c *Core, root, client, ttl string, policy []string) {
-	testMakeTokenViaCore(t, c, root, client, ttl, policy, false, nil)
+	testMakeTokenViaCore(t, c, root, client, ttl, "", policy, false, nil)
 }
 
-func testMakeTokenViaCore(t testing.TB, c *Core, root, client, ttl string, policy []string, batch bool, outAuth *logical.Auth) {
+func testMakeTokenViaCore(t testing.TB, c *Core, root, client, ttl, period string, policy []string, batch bool, outAuth *logical.Auth) {
 	req := logical.TestRequest(t, logical.UpdateOperation, "auth/token/create")
 	req.ClientToken = root
 	if batch {
@@ -615,6 +615,10 @@ func testMakeTokenViaCore(t testing.TB, c *Core, root, client, ttl string, polic
 	req.Data["policies"] = policy
 	req.Data["ttl"] = ttl
 
+	if len(period) != 0 {
+		req.Data["period"] = period
+	}
+
 	resp, err := c.HandleRequest(namespace.RootContext(nil), req)
 	if err != nil || (resp != nil && resp.IsError()) {
 		t.Fatalf("err: %v\nresp: %#v", err, resp)
@@ -2347,11 +2351,20 @@ func TestTokenStore_HandleRequest_RevokeOrphan_NonRoot(t *testing.T) {
 }
 
 func TestTokenStore_HandleRequest_Lookup(t *testing.T) {
-	testTokenStore_HandleRequest_Lookup(t, false)
-	testTokenStore_HandleRequest_Lookup(t, true)
+	t.Run("service_token", func(t *testing.T) {
+		testTokenStoreHandleRequestLookup(t, false, false)
+	})
+
+	t.Run("service_token_periodic", func(t *testing.T) {
+		testTokenStoreHandleRequestLookup(t, false, true)
+	})
+
+	t.Run("batch_token", func(t *testing.T) {
+		testTokenStoreHandleRequestLookup(t, true, false)
+	})
 }
 
-func testTokenStore_HandleRequest_Lookup(t *testing.T, batch bool) {
+func testTokenStoreHandleRequestLookup(t *testing.T, batch, periodic bool) {
 	c, _, root := TestCoreUnsealed(t)
 	ts := c.tokenStore
 	req := logical.TestRequest(t, logical.UpdateOperation, "lookup")
@@ -2392,8 +2405,13 @@ func testTokenStore_HandleRequest_Lookup(t *testing.T, batch bool) {
 		t.Fatalf("bad: expected:%#v\nactual:%#v", exp, resp.Data)
 	}
 
+	period := ""
+	if periodic {
+		period = "3600s"
+	}
+
 	outAuth := new(logical.Auth)
-	testMakeTokenViaCore(t, c, root, "client", "3600s", []string{"foo"}, batch, outAuth)
+	testMakeTokenViaCore(t, c, root, "client", "3600s", period, []string{"foo"}, batch, outAuth)
 
 	tokenType := "service"
 	expID := "client"
@@ -2431,57 +2449,8 @@ func testTokenStore_HandleRequest_Lookup(t *testing.T, batch bool) {
 		"entity_id":        "",
 		"type":             tokenType,
 	}
-
-	if resp.Data["creation_time"].(int64) == 0 {
-		t.Fatalf("creation time was zero")
-	}
-	delete(resp.Data, "creation_time")
-	if resp.Data["issue_time"].(time.Time).IsZero() {
-		t.Fatal("issue time is default time")
-	}
-	delete(resp.Data, "issue_time")
-	if resp.Data["expire_time"].(time.Time).IsZero() {
-		t.Fatal("expire time is default time")
-	}
-	delete(resp.Data, "expire_time")
-
-	// Depending on timing of the test this may have ticked down, so accept 3599
-	if resp.Data["ttl"].(int64) == 3599 {
-		resp.Data["ttl"] = int64(3600)
-	}
-
-	if diff := deep.Equal(resp.Data, exp); diff != nil {
-		t.Fatal(diff)
-	}
-
-	// Test via POST
-	req = logical.TestRequest(t, logical.UpdateOperation, "lookup")
-	req.Data = map[string]interface{}{
-		"token": expID,
-	}
-	resp, err = ts.HandleRequest(namespace.RootContext(nil), req)
-	if err != nil || (resp != nil && resp.IsError()) {
-		t.Fatalf("err: %v\nresp: %#v", err, resp)
-	}
-	if resp == nil {
-		t.Fatalf("bad: %#v", resp)
-	}
-
-	exp = map[string]interface{}{
-		"id":               expID,
-		"accessor":         resp.Data["accessor"],
-		"policies":         []string{"default", "foo"},
-		"path":             "auth/token/create",
-		"meta":             map[string]string(nil),
-		"display_name":     "token",
-		"orphan":           false,
-		"num_uses":         0,
-		"creation_ttl":     int64(3600),
-		"ttl":              int64(3600),
-		"explicit_max_ttl": int64(0),
-		"renewable":        !batch,
-		"entity_id":        "",
-		"type":             tokenType,
+	if periodic {
+		exp["period"] = int64(3600)
 	}
 
 	if resp.Data["creation_time"].(int64) == 0 {