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

builtin/credential/approle/path_login.go

@@ -4,6 +4,7 @@ import (
 	"context"
 	"fmt"
 	"strings"
+	"time"
 
 	"github.com/hashicorp/vault/logical"
 	"github.com/hashicorp/vault/logical/framework"
@@ -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 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"

builtin/credential/aws/path_login.go

@@ -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 there on.
+	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) {
@@ -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 there on.
+	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) {

builtin/credential/cert/path_login.go

@@ -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 there on.
+	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) {

vault/expiration_test.go

@@ -29,6 +29,11 @@ func mockExpiration(t testing.TB) *ExpirationManager {
 	return ts.expiration
 }
 
+func mockCoreExpiration(t testing.TB) (*Core, *ExpirationManager) {
+	c, ts, _, _ := TestCoreWithTokenStore(t)
+	return c, ts.expiration
+}
+
 func mockBackendExpiration(t testing.TB, backend physical.Backend) (*Core, *ExpirationManager) {
 	c, ts, _, _ := TestCoreWithBackendTokenStore(t, backend)
 	return c, ts.expiration
@@ -790,8 +795,109 @@ func TestExpiration_RenewToken(t *testing.T) {
 	}
 
 	if auth.ClientToken != out.Auth.ClientToken {
-		t.Fatalf("Bad: %#v", out)
+		t.Fatalf("bad: %#v", out)
+	}
+}
+
+func TestExpiration_RenewToken_period(t *testing.T) {
+	exp := mockExpiration(t)
+	root, err := exp.tokenStore.rootToken()
+	if err != nil {
+		t.Fatalf("err: %v", err)
+	}
+
+	// Register a token
+	auth := &logical.Auth{
+		ClientToken: root.ID,
+		LeaseOptions: logical.LeaseOptions{
+			TTL:       time.Hour,
+			Renewable: true,
+		},
+		Period: time.Minute,
+	}
+	err = exp.RegisterAuth("auth/token/login", auth)
+	if err != nil {
+		t.Fatalf("err: %v", err)
+	}
+
+	// Renew the token
+	out, err := exp.RenewToken(&logical.Request{}, "auth/token/login", root.ID, 0)
+	if err != nil {
+		t.Fatalf("err: %v", err)
+	}
+
+	if auth.ClientToken != out.Auth.ClientToken {
+		t.Fatalf("bad: %#v", out)
+	}
+
+	if out.Auth.TTL > time.Minute {
+		t.Fatalf("expected TTL to be less than 1 minute, got: %s", out.Auth.TTL)
+	}
+}
+
+func TestExpiration_RenewToken_period_backend(t *testing.T) {
+	exp := mockExpiration(t)
+	root, err := exp.tokenStore.rootToken()
+	if err != nil {
+		t.Fatalf("err: %v", err)
 	}
+
+	// Mount a noop backend
+	noop := &NoopBackend{
+		Response: &logical.Response{
+			Auth: &logical.Auth{
+				LeaseOptions: logical.LeaseOptions{
+					TTL:       5 * time.Second,
+					Renewable: true,
+				},
+			},
+		},
+		DefaultLeaseTTL: 5 * time.Second,
+		MaxLeaseTTL:     5 * time.Second,
+	}
+
+	_, barrier, _ := mockBarrier(t)
+	view := NewBarrierView(barrier, credentialBarrierPrefix)
+	meUUID, err := uuid.GenerateUUID()
+	if err != nil {
+		t.Fatal(err)
+	}
+	err = exp.router.Mount(noop, "auth/foo/", &MountEntry{Path: "auth/foo/", Type: "noop", UUID: meUUID, Accessor: "noop-accessor"}, view)
+	if err != nil {
+		t.Fatal(err)
+	}
+
+	// Register a token
+	auth := &logical.Auth{
+		ClientToken: root.ID,
+		LeaseOptions: logical.LeaseOptions{
+			TTL:       5 * time.Second,
+			Renewable: true,
+		},
+		// Period: 5 * time.Second,
+	}
+
+	err = exp.RegisterAuth("auth/foo/login", auth)
+	if err != nil {
+		t.Fatalf("err: %v", err)
+	}
+
+	// Wait 3 seconds
+	// time.Sleep(3 * time.Second)
+	out, err := exp.RenewToken(&logical.Request{}, "auth/foo/login", root.ID, 0)
+	if err != nil {
+		t.Fatalf("err: %v", err)
+	}
+
+	fmt.Println(out.Auth)
+
+	// time.Sleep(3 * time.Second)
+	out, err = exp.RenewToken(&logical.Request{}, "auth/foo/login", root.ID, 0)
+	if err != nil {
+		t.Fatalf("err: %v", err)
+	}
+
+	fmt.Println(out.Auth)
 }
 
 func TestExpiration_RenewToken_NotRenewable(t *testing.T) {

vault/router_test.go

@@ -19,12 +19,14 @@ import (
 type NoopBackend struct {
 	sync.Mutex
 
-	Root          []string
-	Login         []string
-	Paths         []string
-	Requests      []*logical.Request
-	Response      *logical.Response
-	Invalidations []string
+	Root            []string
+	Login           []string
+	Paths           []string
+	Requests        []*logical.Request
+	Response        *logical.Response
+	Invalidations   []string
+	DefaultLeaseTTL time.Duration
+	MaxLeaseTTL     time.Duration
 }
 
 func (n *NoopBackend) HandleRequest(ctx context.Context, req *logical.Request) (*logical.Response, error) {
@@ -53,9 +55,19 @@ func (n *NoopBackend) SpecialPaths() *logical.Paths {
 }
 
 func (n *NoopBackend) System() logical.SystemView {
+	defaultLeaseTTLVal := time.Hour * 24
+	maxLeaseTTLVal := time.Hour * 24 * 32
+	if n.DefaultLeaseTTL > 0 {
+		defaultLeaseTTLVal = n.DefaultLeaseTTL
+	}
+
+	if n.MaxLeaseTTL > 0 {
+		maxLeaseTTLVal = n.MaxLeaseTTL
+	}
+
 	return logical.StaticSystemView{
-		DefaultLeaseTTLVal: time.Hour * 24,
-		MaxLeaseTTLVal:     time.Hour * 24 * 32,
+		DefaultLeaseTTLVal: defaultLeaseTTLVal,
+		MaxLeaseTTLVal:     maxLeaseTTLVal,
 	}
 }