From aa41f190cfbe5ba40d18d3c3d0545f93c1594313 Mon Sep 17 00:00:00 2001 From: Calvin Leung Huang Date: Tue, 12 Dec 2017 00:01:35 -0500 Subject: [PATCH 1/8] Add logic for using Auth.Period when handling auth login/renew requests --- builtin/credential/approle/path_login.go | 24 +++++------------- logical/framework/lease.go | 3 ++- vault/expiration.go | 13 ++++++++++ vault/request_handling.go | 31 +++++++++++++++++------- 4 files changed, 43 insertions(+), 28 deletions(-) diff --git a/builtin/credential/approle/path_login.go b/builtin/credential/approle/path_login.go index 300ee9409f89..3dd829a8463d 100644 --- a/builtin/credential/approle/path_login.go +++ b/builtin/credential/approle/path_login.go @@ -3,7 +3,6 @@ package approle import ( "fmt" "strings" - "time" "github.com/hashicorp/vault/logical" "github.com/hashicorp/vault/logical/framework" @@ -68,20 +67,13 @@ func (b *backend) pathLoginUpdate(req *logical.Request, data *framework.FieldDat Policies: role.Policies, LeaseOptions: logical.LeaseOptions{ Renewable: true, + TTL: role.TokenTTL, }, Alias: &logical.Alias{ Name: role.RoleID, }, } - // If 'Period' is set, use the value of 'Period' as the TTL. - // Otherwise, set the normal TokenTTL. - if role.Period > time.Duration(0) { - auth.TTL = role.Period - } else { - auth.TTL = role.TokenTTL - } - return &logical.Response{ Auth: auth, }, nil @@ -107,16 +99,12 @@ func (b *backend) pathLoginRenew(req *logical.Request, data *framework.FieldData return nil, fmt.Errorf("role %s does not exist during renewal", roleName) } - // If 'Period' is set on the Role, the token should never expire. - // Replenish the TTL with 'Period's value. - if role.Period > time.Duration(0) { - // If 'Period' was updated after the token was issued, - // token will bear the updated 'Period' value as its TTL. - req.Auth.TTL = role.Period - return &logical.Response{Auth: req.Auth}, nil - } else { - return framework.LeaseExtend(role.TokenTTL, role.TokenMaxTTL, b.System())(req, data) + resp, err := framework.LeaseExtend(role.TokenTTL, role.TokenMaxTTL, b.System())(req, data) + if err != nil { + return nil, err } + resp.Auth.Period = role.Period + return resp, nil } const pathLoginHelpSys = "Issue a token based on the credentials supplied" diff --git a/logical/framework/lease.go b/logical/framework/lease.go index 4fd2ac902c66..d2678f7120e8 100644 --- a/logical/framework/lease.go +++ b/logical/framework/lease.go @@ -8,7 +8,8 @@ import ( ) // LeaseExtend returns an OperationFunc that can be used to simply extend the -// lease of the auth/secret for the duration that was requested. +// lease of the auth/secret for the duration that was requested. The parameters +// provided are used to determine the lease's new TTL value. // // backendIncrement is the backend's requested increment -- perhaps from a user // request, perhaps from a role/config value. If not set, uses the mount/system diff --git a/vault/expiration.go b/vault/expiration.go index 23176ae4453f..683035d49d1b 100644 --- a/vault/expiration.go +++ b/vault/expiration.go @@ -750,6 +750,13 @@ func (m *ExpirationManager) RenewToken(req *logical.Request, source string, toke }, nil } + // If it resp.Period is non-zero, use that as the TTL and override backend's + // call on TTL modification, such as the TTL determined by + // framework.LeaseExtend call against the request. + if resp.Auth.Period > time.Duration(0) { + resp.Auth.TTL = resp.Auth.Period + } + // Attach the ClientToken resp.Auth.ClientToken = token resp.Auth.Increment = 0 @@ -866,6 +873,12 @@ func (m *ExpirationManager) RegisterAuth(source string, auth *logical.Auth) erro return err } + // If it resp.Period is non-zero, override the TTL value determined + // by the backend. + if auth.Period > time.Duration(0) { + auth.TTL = auth.Period + } + // Create a lease entry le := leaseEntry{ LeaseID: path.Join(source, saltedID), diff --git a/vault/request_handling.go b/vault/request_handling.go index 7fb7ea1d3241..43f3775851ea 100644 --- a/vault/request_handling.go +++ b/vault/request_handling.go @@ -477,14 +477,27 @@ func (c *Core) handleLoginRequest(req *logical.Request) (retResp *logical.Respon return nil, nil, ErrInternalError } - // Set the default lease if not provided - if auth.TTL == 0 { - auth.TTL = sysView.DefaultLeaseTTL() - } - - // Limit the lease duration - if auth.TTL > sysView.MaxLeaseTTL() { - auth.TTL = sysView.MaxLeaseTTL() + // Start off with the sys default value, and update according to period/TTL + // from resp.Auth + tokenTTL := sysView.DefaultLeaseTTL() + + switch { + case auth.Period > time.Duration(0): + // Cap the period value to the sys max_ttl value. The auth backend should + // have checked for it on its login path, but we check here again for + // sanity. + if auth.Period > sysView.MaxLeaseTTL() { + auth.Period = sysView.MaxLeaseTTL() + } + tokenTTL = auth.Period + case auth.TTL > time.Duration(0): + // Cap the TTL value. The auth backend should have checked for it on its + // login path (e.g. a call to b.SanitizeTTL), but we check here again for + // sanity. + if auth.TTL > sysView.MaxLeaseTTL() { + auth.TTL = sysView.MaxLeaseTTL() + } + tokenTTL = auth.TTL } // Generate a token @@ -494,7 +507,7 @@ func (c *Core) handleLoginRequest(req *logical.Request) (retResp *logical.Respon Meta: auth.Metadata, DisplayName: auth.DisplayName, CreationTime: time.Now().Unix(), - TTL: auth.TTL, + TTL: tokenTTL, NumUses: auth.NumUses, EntityID: auth.EntityID, } From 8d0e9ef5f11d1abd202b009e91789cb9c67b955d Mon Sep 17 00:00:00 2001 From: Calvin Leung Huang Date: Tue, 12 Dec 2017 12:44:34 -0500 Subject: [PATCH 2/8] Set auth.TTL if not set in handleLoginRequest --- builtin/credential/app-id/backend_test.go | 4 ++-- vault/request_handling.go | 5 +++++ 2 files changed, 7 insertions(+), 2 deletions(-) diff --git a/builtin/credential/app-id/backend_test.go b/builtin/credential/app-id/backend_test.go index 4ae5d3e1cfec..e5d335b4f781 100644 --- a/builtin/credential/app-id/backend_test.go +++ b/builtin/credential/app-id/backend_test.go @@ -141,7 +141,7 @@ func testAccStepMapUserIdCidr(t *testing.T, cidr string) logicaltest.TestStep { func testAccLogin(t *testing.T, display string) logicaltest.TestStep { checkTTL := func(resp *logical.Response) error { if resp.Auth.LeaseOptions.TTL.String() != "768h0m0s" { - return fmt.Errorf("invalid TTL") + return fmt.Errorf("invalid TTL: got %s", resp.Auth.LeaseOptions.TTL) } return nil } @@ -165,7 +165,7 @@ func testAccLogin(t *testing.T, display string) logicaltest.TestStep { func testAccLoginAppIDInPath(t *testing.T, display string) logicaltest.TestStep { checkTTL := func(resp *logical.Response) error { if resp.Auth.LeaseOptions.TTL.String() != "768h0m0s" { - return fmt.Errorf("invalid TTL") + return fmt.Errorf("invalid TTL: got %s", resp.Auth.LeaseOptions.TTL) } return nil } diff --git a/vault/request_handling.go b/vault/request_handling.go index 43f3775851ea..9c2dd33a58cf 100644 --- a/vault/request_handling.go +++ b/vault/request_handling.go @@ -531,6 +531,11 @@ func (c *Core) handleLoginRequest(req *logical.Request) (retResp *logical.Respon auth.Accessor = te.Accessor auth.Policies = te.Policies + // Set auth.TTL if not set, used below in RegisterAuth + if auth.TTL == 0 { + auth.TTL = tokenTTL + } + // Register with the expiration manager if err := c.expiration.RegisterAuth(te.Path, auth); err != nil { c.tokenStore.Revoke(te.ID) From f0d60873b61229b0cbeebe5545b6feff348f3d53 Mon Sep 17 00:00:00 2001 From: Calvin Leung Huang Date: Tue, 12 Dec 2017 14:56:15 -0500 Subject: [PATCH 3/8] Always set auth.TTL = te.TTL on handleLoginRequest, check TTL and period against sys values on RenewToken --- vault/expiration.go | 15 ++++++++++++++- vault/expiration_test.go | 14 ++++++++++++-- vault/request_handling.go | 9 ++------- 3 files changed, 28 insertions(+), 10 deletions(-) diff --git a/vault/expiration.go b/vault/expiration.go index 683035d49d1b..202afd8767c7 100644 --- a/vault/expiration.go +++ b/vault/expiration.go @@ -750,10 +750,23 @@ func (m *ExpirationManager) RenewToken(req *logical.Request, source string, toke }, nil } + sysView := m.router.MatchingSystemView(req.Path) + if sysView == nil { + return nil, fmt.Errorf("expiration: unable to retrieve system view from router") + } + + // Cap TTL value to the sys/mount max value + if resp.Auth.TTL > sysView.MaxLeaseTTL() { + resp.Auth.TTL = sysView.MaxLeaseTTL() + } + // If it resp.Period is non-zero, use that as the TTL and override backend's - // call on TTL modification, such as the TTL determined by + // call on TTL modification, such as a TTL value determined by // framework.LeaseExtend call against the request. if resp.Auth.Period > time.Duration(0) { + if resp.Auth.Period > sysView.MaxLeaseTTL() { + resp.Auth.Period = sysView.MaxLeaseTTL() + } resp.Auth.TTL = resp.Auth.Period } diff --git a/vault/expiration_test.go b/vault/expiration_test.go index 144bd16b045f..58bcbf470341 100644 --- a/vault/expiration_test.go +++ b/vault/expiration_test.go @@ -782,8 +782,13 @@ func TestExpiration_RenewToken(t *testing.T) { t.Fatalf("err: %v", err) } + req := &logical.Request{ + Operation: logical.UpdateOperation, + Path: "auth/token/renew", + } + // Renew the token - out, err := exp.RenewToken(&logical.Request{}, "auth/token/login", root.ID, 0) + out, err := exp.RenewToken(req, "auth/token/login", root.ID, 0) if err != nil { t.Fatalf("err: %v", err) } @@ -813,8 +818,13 @@ func TestExpiration_RenewToken_NotRenewable(t *testing.T) { t.Fatalf("err: %v", err) } + req := &logical.Request{ + Operation: logical.UpdateOperation, + Path: "auth/token/renew", + } + // Attempt to renew the token - resp, err := exp.RenewToken(&logical.Request{}, "auth/github/login", root.ID, 0) + resp, err := exp.RenewToken(req, "auth/github/login", root.ID, 0) if err != nil && (err != logical.ErrInvalidRequest || (resp != nil && resp.IsError() && resp.Error().Error() != "lease is not renewable")) { t.Fatalf("bad: err:%v resp:%#v", err, resp) } diff --git a/vault/request_handling.go b/vault/request_handling.go index 9c2dd33a58cf..09207914b57a 100644 --- a/vault/request_handling.go +++ b/vault/request_handling.go @@ -526,15 +526,10 @@ func (c *Core) handleLoginRequest(req *logical.Request) (retResp *logical.Respon return nil, auth, ErrInternalError } - // Populate the client token and accessor + // Populate the client token, accessor, and TTL auth.ClientToken = te.ID auth.Accessor = te.Accessor - auth.Policies = te.Policies - - // Set auth.TTL if not set, used below in RegisterAuth - if auth.TTL == 0 { - auth.TTL = tokenTTL - } + auth.TTL = te.TTL // Register with the expiration manager if err := c.expiration.RegisterAuth(te.Path, auth); err != nil { From 7ebb840a3880995655dc297eebe83cfe73062e73 Mon Sep 17 00:00:00 2001 From: Calvin Leung Huang Date: Tue, 12 Dec 2017 16:34:05 -0500 Subject: [PATCH 4/8] Get sysView from le.Path, revert tests --- vault/expiration.go | 2 +- vault/expiration_test.go | 14 ++------------ 2 files changed, 3 insertions(+), 13 deletions(-) diff --git a/vault/expiration.go b/vault/expiration.go index 202afd8767c7..58dc6c3bb72a 100644 --- a/vault/expiration.go +++ b/vault/expiration.go @@ -750,7 +750,7 @@ func (m *ExpirationManager) RenewToken(req *logical.Request, source string, toke }, nil } - sysView := m.router.MatchingSystemView(req.Path) + sysView := m.router.MatchingSystemView(le.Path) if sysView == nil { return nil, fmt.Errorf("expiration: unable to retrieve system view from router") } diff --git a/vault/expiration_test.go b/vault/expiration_test.go index 58bcbf470341..144bd16b045f 100644 --- a/vault/expiration_test.go +++ b/vault/expiration_test.go @@ -782,13 +782,8 @@ func TestExpiration_RenewToken(t *testing.T) { t.Fatalf("err: %v", err) } - req := &logical.Request{ - Operation: logical.UpdateOperation, - Path: "auth/token/renew", - } - // Renew the token - out, err := exp.RenewToken(req, "auth/token/login", root.ID, 0) + out, err := exp.RenewToken(&logical.Request{}, "auth/token/login", root.ID, 0) if err != nil { t.Fatalf("err: %v", err) } @@ -818,13 +813,8 @@ func TestExpiration_RenewToken_NotRenewable(t *testing.T) { t.Fatalf("err: %v", err) } - req := &logical.Request{ - Operation: logical.UpdateOperation, - Path: "auth/token/renew", - } - // Attempt to renew the token - resp, err := exp.RenewToken(req, "auth/github/login", root.ID, 0) + resp, err := exp.RenewToken(&logical.Request{}, "auth/github/login", root.ID, 0) if err != nil && (err != logical.ErrInvalidRequest || (resp != nil && resp.IsError() && resp.Error().Error() != "lease is not renewable")) { t.Fatalf("bad: err:%v resp:%#v", err, resp) } From b458265b5c996080e959ed4d5dda9af7c0f37ba4 Mon Sep 17 00:00:00 2001 From: Calvin Leung Huang Date: Thu, 14 Dec 2017 01:52:17 -0500 Subject: [PATCH 5/8] Add back auth.Policies --- vault/request_handling.go | 1 + 1 file changed, 1 insertion(+) diff --git a/vault/request_handling.go b/vault/request_handling.go index 09207914b57a..3453ff1b0ec2 100644 --- a/vault/request_handling.go +++ b/vault/request_handling.go @@ -529,6 +529,7 @@ func (c *Core) handleLoginRequest(req *logical.Request) (retResp *logical.Respon // Populate the client token, accessor, and TTL auth.ClientToken = te.ID auth.Accessor = te.Accessor + auth.Policies = te.Policies auth.TTL = te.TTL // Register with the expiration manager From dd90688c8f81d70fe79579cb7718d4b14ca5f93c Mon Sep 17 00:00:00 2001 From: Calvin Leung Huang Date: Thu, 14 Dec 2017 14:52:25 -0500 Subject: [PATCH 6/8] Fix TokenStore tests, add resp warning when capping values --- vault/expiration.go | 9 +++-- vault/token_store_test.go | 69 +++++++++++++++++++++------------------ 2 files changed, 43 insertions(+), 35 deletions(-) diff --git a/vault/expiration.go b/vault/expiration.go index 58dc6c3bb72a..7cb2f0622b87 100644 --- a/vault/expiration.go +++ b/vault/expiration.go @@ -755,8 +755,10 @@ func (m *ExpirationManager) RenewToken(req *logical.Request, source string, toke return nil, fmt.Errorf("expiration: unable to retrieve system view from router") } + retResp := &logical.Response{} // Cap TTL value to the sys/mount max value if resp.Auth.TTL > sysView.MaxLeaseTTL() { + retResp.AddWarning(fmt.Sprintf("TTL of %d seconds is greater than current mount/system default of %d seconds, value will be truncated.", resp.Auth.TTL, sysView.MaxLeaseTTL())) resp.Auth.TTL = sysView.MaxLeaseTTL() } @@ -765,6 +767,7 @@ func (m *ExpirationManager) RenewToken(req *logical.Request, source string, toke // framework.LeaseExtend call against the request. if resp.Auth.Period > time.Duration(0) { if resp.Auth.Period > sysView.MaxLeaseTTL() { + retResp.AddWarning(fmt.Sprintf("Period of %d seconds is greater than current mount/system default of %d seconds, value will be truncated.", resp.Auth.TTL, sysView.MaxLeaseTTL())) resp.Auth.Period = sysView.MaxLeaseTTL() } resp.Auth.TTL = resp.Auth.Period @@ -784,9 +787,9 @@ func (m *ExpirationManager) RenewToken(req *logical.Request, source string, toke // Update the expiration time m.updatePending(le, resp.Auth.LeaseTotal()) - return &logical.Response{ - Auth: resp.Auth, - }, nil + + retResp.Auth = resp.Auth + return retResp, nil } // Register is used to take a request and response with an associated diff --git a/vault/token_store_test.go b/vault/token_store_test.go index 1b3d7c346c16..db8cf4816666 100644 --- a/vault/token_store_test.go +++ b/vault/token_store_test.go @@ -2312,7 +2312,7 @@ func TestTokenStore_RolePeriod(t *testing.T) { req := logical.TestRequest(t, logical.UpdateOperation, "auth/token/roles/test") req.ClientToken = root req.Data = map[string]interface{}{ - "period": 300, + "period": 5, } resp, err := core.HandleRequest(req) @@ -2425,8 +2425,8 @@ func TestTokenStore_RolePeriod(t *testing.T) { t.Fatalf("err: %v", err) } ttl := resp.Data["ttl"].(int64) - if ttl < 299 { - t.Fatalf("TTL too small (expected %d, got %d", 299, ttl) + if ttl > 5 { + t.Fatalf("TTL too large (expected %d, got %d", 5, ttl) } // Let the TTL go down a bit to 3 seconds @@ -2449,8 +2449,8 @@ func TestTokenStore_RolePeriod(t *testing.T) { t.Fatalf("err: %v", err) } ttl = resp.Data["ttl"].(int64) - if ttl < 299 { - t.Fatalf("TTL too small (expected %d, got %d", 299, ttl) + if ttl > 5 { + t.Fatalf("TTL too large (expected %d, got %d", 5, ttl) } } } @@ -2677,7 +2677,7 @@ func TestTokenStore_Periodic(t *testing.T) { req := logical.TestRequest(t, logical.UpdateOperation, "auth/token/roles/test") req.ClientToken = root req.Data = map[string]interface{}{ - "period": 300, + "period": 5, } resp, err := core.HandleRequest(req) @@ -2715,8 +2715,8 @@ func TestTokenStore_Periodic(t *testing.T) { t.Fatalf("err: %v", err) } ttl := resp.Data["ttl"].(int64) - if ttl < 299 { - t.Fatalf("TTL too small (expected %d, got %d)", 299, ttl) + if ttl > 5 { + t.Fatalf("TTL too large (expected %d, got %d)", 5, ttl) } // Let the TTL go down a bit @@ -2739,8 +2739,8 @@ func TestTokenStore_Periodic(t *testing.T) { t.Fatalf("err: %v", err) } ttl = resp.Data["ttl"].(int64) - if ttl < 299 { - t.Fatalf("TTL too small (expected %d, got %d)", 299, ttl) + if ttl > 5 { + t.Fatalf("TTL too large (expected %d, got %d)", 5, ttl) } } @@ -2750,8 +2750,8 @@ func TestTokenStore_Periodic(t *testing.T) { req.Operation = logical.UpdateOperation req.Path = "auth/token/create" req.Data = map[string]interface{}{ - "period": 300, - "explicit_max_ttl": 150, + "period": 5, + "explicit_max_ttl": 4, } resp, err = core.HandleRequest(req) if err != nil { @@ -2775,8 +2775,8 @@ func TestTokenStore_Periodic(t *testing.T) { t.Fatalf("err: %v", err) } ttl := resp.Data["ttl"].(int64) - if ttl < 149 || ttl > 150 { - t.Fatalf("TTL bad (expected %d, got %d)", 149, ttl) + if ttl < 3 || ttl > 4 { + t.Fatalf("TTL bad (expected %d, got %d)", 3, ttl) } // Let the TTL go down a bit @@ -2799,8 +2799,8 @@ func TestTokenStore_Periodic(t *testing.T) { t.Fatalf("err: %v", err) } ttl = resp.Data["ttl"].(int64) - if ttl < 140 || ttl > 150 { - t.Fatalf("TTL bad (expected around %d, got %d)", 145, ttl) + if ttl > 2 { + t.Fatalf("TTL bad (expected less than %d, got %d)", 2, ttl) } } @@ -2812,7 +2812,7 @@ func TestTokenStore_Periodic(t *testing.T) { req.Operation = logical.UpdateOperation req.Path = "auth/token/create/test" req.Data = map[string]interface{}{ - "period": 150, + "period": 5, } resp, err = core.HandleRequest(req) if err != nil { @@ -2836,8 +2836,8 @@ func TestTokenStore_Periodic(t *testing.T) { t.Fatalf("err: %v", err) } ttl := resp.Data["ttl"].(int64) - if ttl < 149 || ttl > 150 { - t.Fatalf("TTL bad (expected %d, got %d)", 149, ttl) + if ttl < 4 || ttl > 5 { + t.Fatalf("TTL bad (expected %d, got %d)", 4, ttl) } // Let the TTL go down a bit @@ -2860,8 +2860,8 @@ func TestTokenStore_Periodic(t *testing.T) { t.Fatalf("err: %v", err) } ttl = resp.Data["ttl"].(int64) - if ttl < 149 { - t.Fatalf("TTL bad (expected %d, got %d)", 149, ttl) + if ttl > 5 { + t.Fatalf("TTL bad (expected less than %d, got %d)", 5, ttl) } } @@ -2869,18 +2869,23 @@ func TestTokenStore_Periodic(t *testing.T) { { req.Path = "auth/token/roles/test" req.ClientToken = root + req.Operation = logical.UpdateOperation req.Data = map[string]interface{}{ - "period": 300, - "explicit_max_ttl": 150, + "period": 5, + "explicit_max_ttl": 4, + } + + resp, err := core.HandleRequest(req) + if err != nil { + t.Fatalf("err: %v %v", err, resp) + } + if resp != nil { + t.Fatalf("expected a nil response") } req.ClientToken = root req.Operation = logical.UpdateOperation req.Path = "auth/token/create/test" - req.Data = map[string]interface{}{ - "period": 150, - "explicit_max_ttl": 130, - } resp, err = core.HandleRequest(req) if err != nil { t.Fatalf("err: %v %v", err, resp) @@ -2903,12 +2908,12 @@ func TestTokenStore_Periodic(t *testing.T) { t.Fatalf("err: %v", err) } ttl := resp.Data["ttl"].(int64) - if ttl < 129 || ttl > 130 { - t.Fatalf("TTL bad (expected %d, got %d)", 129, ttl) + if ttl < 3 || ttl > 4 { + t.Fatalf("TTL bad (expected %d, got %d)", 3, ttl) } // Let the TTL go down a bit - time.Sleep(4 * time.Second) + time.Sleep(2 * time.Second) req.Operation = logical.UpdateOperation req.Path = "auth/token/renew-self" @@ -2927,8 +2932,8 @@ func TestTokenStore_Periodic(t *testing.T) { t.Fatalf("err: %v", err) } ttl = resp.Data["ttl"].(int64) - if ttl > 127 { - t.Fatalf("TTL bad (expected < %d, got %d)", 128, ttl) + if ttl > 2 { + t.Fatalf("TTL bad (expected less than %d, got %d)", 2, ttl) } } } From 465506ad6c99256a38739249da89dc63ea841015 Mon Sep 17 00:00:00 2001 From: Calvin Leung Huang Date: Thu, 14 Dec 2017 16:44:00 -0500 Subject: [PATCH 7/8] Use switch for ttl/period check on RenewToken --- vault/expiration.go | 15 ++++++++------- 1 file changed, 8 insertions(+), 7 deletions(-) diff --git a/vault/expiration.go b/vault/expiration.go index 7cb2f0622b87..09d55ae7c49b 100644 --- a/vault/expiration.go +++ b/vault/expiration.go @@ -756,21 +756,22 @@ func (m *ExpirationManager) RenewToken(req *logical.Request, source string, toke } retResp := &logical.Response{} - // Cap TTL value to the sys/mount max value - if resp.Auth.TTL > sysView.MaxLeaseTTL() { - retResp.AddWarning(fmt.Sprintf("TTL of %d seconds is greater than current mount/system default of %d seconds, value will be truncated.", resp.Auth.TTL, sysView.MaxLeaseTTL())) - resp.Auth.TTL = sysView.MaxLeaseTTL() - } - + switch { // If it resp.Period is non-zero, use that as the TTL and override backend's // call on TTL modification, such as a TTL value determined by // framework.LeaseExtend call against the request. - if resp.Auth.Period > time.Duration(0) { + case resp.Auth.Period > time.Duration(0): if resp.Auth.Period > sysView.MaxLeaseTTL() { retResp.AddWarning(fmt.Sprintf("Period of %d seconds is greater than current mount/system default of %d seconds, value will be truncated.", resp.Auth.TTL, sysView.MaxLeaseTTL())) resp.Auth.Period = sysView.MaxLeaseTTL() } resp.Auth.TTL = resp.Auth.Period + // Cap TTL value to the sys/mount max value + case resp.Auth.TTL > time.Duration(0): + if resp.Auth.TTL > sysView.MaxLeaseTTL() { + retResp.AddWarning(fmt.Sprintf("TTL of %d seconds is greater than current mount/system default of %d seconds, value will be truncated.", resp.Auth.TTL, sysView.MaxLeaseTTL())) + resp.Auth.TTL = sysView.MaxLeaseTTL() + } } // Attach the ClientToken From 532487152dfac3397471b7fd0b79002e398cf3aa Mon Sep 17 00:00:00 2001 From: Calvin Leung Huang Date: Thu, 14 Dec 2017 16:48:03 -0500 Subject: [PATCH 8/8] Move comments around --- vault/expiration.go | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/vault/expiration.go b/vault/expiration.go index 09d55ae7c49b..62ec07d52dc1 100644 --- a/vault/expiration.go +++ b/vault/expiration.go @@ -757,17 +757,18 @@ func (m *ExpirationManager) RenewToken(req *logical.Request, source string, toke retResp := &logical.Response{} switch { - // If it resp.Period is non-zero, use that as the TTL and override backend's - // call on TTL modification, such as a TTL value determined by - // framework.LeaseExtend call against the request. case resp.Auth.Period > time.Duration(0): + // If it resp.Period is non-zero, use that as the TTL and override backend's + // call on TTL modification, such as a TTL value determined by + // framework.LeaseExtend call against the request. Also, cap period value to + // the sys/mount max value. if resp.Auth.Period > sysView.MaxLeaseTTL() { retResp.AddWarning(fmt.Sprintf("Period of %d seconds is greater than current mount/system default of %d seconds, value will be truncated.", resp.Auth.TTL, sysView.MaxLeaseTTL())) resp.Auth.Period = sysView.MaxLeaseTTL() } resp.Auth.TTL = resp.Auth.Period - // Cap TTL value to the sys/mount max value case resp.Auth.TTL > time.Duration(0): + // Cap TTL value to the sys/mount max value if resp.Auth.TTL > sysView.MaxLeaseTTL() { retResp.AddWarning(fmt.Sprintf("TTL of %d seconds is greater than current mount/system default of %d seconds, value will be truncated.", resp.Auth.TTL, sysView.MaxLeaseTTL())) resp.Auth.TTL = sysView.MaxLeaseTTL()