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.

Sign up for GitHub

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

Jump to bottom

Don't call LeaseExtend on login renewal paths when period is provided #3803

Merged
merged 15 commits into from
Jan 18, 2018
Merged

Don't call LeaseExtend on login renewal paths when period is provided #3803

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
15 commits
Select commit Hold shift + click to select a range
3e0ae3f
Don't call LeaseExtend on login renewal paths when period is provided
calvn Jan 16, 2018
261cd10
WIP tests
calvn Jan 16, 2018
e9e274b
NoopBackend accept backend ttl values
calvn Jan 16, 2018
7f053de
Test period value on credentials backend
calvn Jan 16, 2018
89baaf8
Use t.Fatalf instead
calvn Jan 16, 2018
fe5b7a3
Remove mockCoreExpiration
calvn Jan 16, 2018
6cdf90f
Add login renewal test for approle backend
calvn Jan 17, 2018
a6812f6
Add resp.Auth.Period check on aws and cert backend tests
calvn Jan 17, 2018
1a85591
Pass in approle's period via role's period
calvn Jan 17, 2018
87bff1f
Correctly set period in valid-role's role
calvn Jan 17, 2018
419c4a6
Add period renewal test using TestCluster and approle backend
calvn Jan 17, 2018
68784f1
Check for ttl values after renewals on test
calvn Jan 18, 2018
8d6d7b1
Merge branch 'master' into auth-login-renewal-period
calvn Jan 18, 2018
34dfecd
Merge branch 'master' into auth-login-renewal-period
calvn Jan 18, 2018
6428ce4
Merge branch 'master' into auth-login-renewal-period
calvn Jan 18, 2018
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
16 changes: 11 additions & 5 deletions builtin/credential/approle/path_login.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ import (
"context"
"fmt"
"strings"
"time"

"github.com/hashicorp/vault/logical"
"github.com/hashicorp/vault/logical/framework"
Expand Down Expand Up @@ -100,12 +101,17 @@ func (b *backend) pathLoginRenew(ctx context.Context, req *logical.Request, data
return nil, fmt.Errorf("role %s does not exist during renewal", roleName)
}

resp, err := framework.LeaseExtend(role.TokenTTL, role.TokenMaxTTL, b.System())(ctx, req, data)
if err != nil {
return nil, err
// If a period is provided, set that as part of resp.Auth.Period and return a
// response immediately. Let expiration manager handle renewal from thereon.
Copy link
Member

Choose a reason for hiding this comment

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

s/thereon/there on

if role.Period > time.Duration(0) {
resp := &logical.Response{
Auth: req.Auth,
}
resp.Auth.Period = role.Period
return resp, nil
}
resp.Auth.Period = role.Period
return resp, nil

return framework.LeaseExtend(role.TokenTTL, role.TokenMaxTTL, b.System())(ctx, req, data)
}

const pathLoginHelpSys = "Issue a token based on the credentials supplied"
Expand Down
30 changes: 20 additions & 10 deletions builtin/credential/aws/path_login.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -975,12 +975,17 @@ func (b *backend) pathLoginRenewIam(ctx context.Context, req *logical.Request, d
}
}

resp, err := framework.LeaseExtend(roleEntry.TTL, roleEntry.MaxTTL, b.System())(ctx, req, data)
if err != nil {
return nil, err
// If a period is provided, set that as part of resp.Auth.Period and return a
// response immediately. Let expiration manager handle renewal from thereon.
if roleEntry.Period > time.Duration(0) {
resp := &logical.Response{
Auth: req.Auth,
}
resp.Auth.Period = roleEntry.Period
return resp, nil
}
resp.Auth.Period = roleEntry.Period
return resp, nil

return framework.LeaseExtend(roleEntry.TTL, roleEntry.MaxTTL, b.System())(ctx, req, data)
}

func (b *backend) pathLoginRenewEc2(ctx context.Context, req *logical.Request, data *framework.FieldData) (*logical.Response, error) {
Expand Down Expand Up @@ -1060,12 +1065,17 @@ func (b *backend) pathLoginRenewEc2(ctx context.Context, req *logical.Request, d
return nil, err
}

resp, err := framework.LeaseExtend(roleEntry.TTL, shortestMaxTTL, b.System())(ctx, req, data)
if err != nil {
return nil, err
// If a period is provided, set that as part of resp.Auth.Period and return a
// response immediately. Let expiration manager handle renewal from thereon.
if roleEntry.Period > time.Duration(0) {
resp := &logical.Response{
Auth: req.Auth,
}
resp.Auth.Period = roleEntry.Period
return resp, nil
}
resp.Auth.Period = roleEntry.Period
return resp, nil

return framework.LeaseExtend(roleEntry.TTL, shortestMaxTTL, b.System())(ctx, req, data)
}

func (b *backend) pathLoginUpdateIam(ctx context.Context, req *logical.Request, data *framework.FieldData) (*logical.Response, error) {
Expand Down
15 changes: 10 additions & 5 deletions builtin/credential/cert/path_login.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -175,12 +175,17 @@ func (b *backend) pathLoginRenew(ctx context.Context, req *logical.Request, d *f
return nil, fmt.Errorf("policies have changed, not renewing")
}

resp, err := framework.LeaseExtend(cert.TTL, cert.MaxTTL, b.System())(ctx, req, d)
if err != nil {
return nil, err
// If a period is provided, set that as part of resp.Auth.Period and return a
// response immediately. Let expiration manager handle renewal from thereon.
if cert.Period > time.Duration(0) {
resp := &logical.Response{
Auth: req.Auth,
}
resp.Auth.Period = cert.Period
return resp, nil
}
resp.Auth.Period = cert.Period
return resp, nil

return framework.LeaseExtend(cert.TTL, cert.MaxTTL, b.System())(ctx, req, d)
}

func (b *backend) verifyCredentials(req *logical.Request, d *framework.FieldData) (*ParsedCert, *logical.Response, error) {
Expand Down