Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Merged
merged 9 commits into from
Oct 26, 2020
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -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:

Expand Down
1 change: 1 addition & 0 deletions vault/request_handling.go
Original file line number Diff line number Diff line change
Expand Up @@ -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,
}

Expand Down
143 changes: 143 additions & 0 deletions vault/request_handling_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down Expand Up @@ -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 {
Expand Down
83 changes: 26 additions & 57 deletions vault/token_store_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand All @@ -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)
Expand Down Expand Up @@ -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")
Expand Down Expand Up @@ -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"
Expand Down Expand Up @@ -2431,57 +2449,8 @@ func testTokenStore_HandleRequest_Lookup(t *testing.T, batch bool) {
"entity_id": "",
"type": tokenType,
}

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The diff makes this looks somewhat erroneous, but this deletion is basically to remove the "// Test via POST" step that seems to be happening twice. I suspect that it's a copy/paste duplication.

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 {
Expand Down