Merge pull request #993 from hashicorp/lease-extend-systemview

Rejig leases

api/auth_token_test.go

@@ -130,13 +130,13 @@ func TestAuthTokenRenew(t *testing.T) {
 	client.SetToken(secret.Auth.ClientToken)
 
 	// Now attempt a renew with the new token
-	secret, err = client.Auth().Token().Renew(secret.Auth.ClientToken, 0)
+	secret, err = client.Auth().Token().Renew(secret.Auth.ClientToken, 3600)
 	if err != nil {
 		t.Fatal(err)
 	}
 
 	if secret.Auth.LeaseDuration != 3600 {
-		t.Errorf("expected 1h, got %q", secret.Auth.LeaseDuration)
+		t.Errorf("expected 1h, got %v", secret.Auth.LeaseDuration)
 	}
 
 	if secret.Auth.Renewable != true {
@@ -144,13 +144,13 @@ func TestAuthTokenRenew(t *testing.T) {
 	}
 
 	// Do the same thing with the self variant
-	secret, err = client.Auth().Token().RenewSelf(0)
+	secret, err = client.Auth().Token().RenewSelf(3600)
 	if err != nil {
 		t.Fatal(err)
 	}
 
 	if secret.Auth.LeaseDuration != 3600 {
-		t.Errorf("expected 1h, got %q", secret.Auth.LeaseDuration)
+		t.Errorf("expected 1h, got %v", secret.Auth.LeaseDuration)
 	}
 
 	if secret.Auth.Renewable != true {

http/logical_test.go

@@ -168,7 +168,7 @@ func TestLogical_CreateToken(t *testing.T) {
 			"policies":       []interface{}{"root"},
 			"metadata":       nil,
 			"lease_duration": float64(0),
-			"renewable":      false,
+			"renewable":      true,
 		},
 		"warnings": nilWarnings,
 	}

logical/framework/backend_test.go

@@ -248,9 +248,14 @@ func TestBackendHandleRequest_renew(t *testing.T) {
 }
 
 func TestBackendHandleRequest_renewExtend(t *testing.T) {
+	sysView := logical.StaticSystemView{
+		DefaultLeaseTTLVal: 5 * time.Minute,
+		MaxLeaseTTLVal:     30 * time.Hour,
+	}
+
 	secret := &Secret{
 		Type:            "foo",
-		Renew:           LeaseExtend(0, 0, false),
+		Renew:           LeaseExtend(0, 0, sysView),
 		DefaultDuration: 5 * time.Minute,
 	}
 	b := &Backend{
@@ -268,7 +273,7 @@ func TestBackendHandleRequest_renewExtend(t *testing.T) {
 		t.Fatal("should have secret")
 	}
 
-	if resp.Secret.TTL < 60*time.Minute || resp.Secret.TTL > 70*time.Minute {
+	if resp.Secret.TTL < 59*time.Minute || resp.Secret.TTL > 61*time.Minute {
 		t.Fatalf("bad: %s", resp.Secret.TTL)
 	}
 }

logical/framework/lease.go

@@ -7,77 +7,79 @@ import (
 	"github.com/hashicorp/vault/logical"
 )
 
-// LeaseExtend returns an OperationFunc that can be used to simply extend
+// LeaseExtend returns an OperationFunc that can be used to simply extend the
-// the lease of the auth/secret for the duration that was requested. Max
+// lease of the auth/secret for the duration that was requested.
-// is the max time past the _current_ time that a lease can be extended. i.e.
-// setting it to 2 hours forces a renewal within the next 2 hours again.
 //
-// maxSession is the maximum session length allowed since the original
+// backendIncrement is the backend's requested increment -- perhaps from a user
-// issue time. If this is zero, it is ignored.
+// request, perhaps from a role/config value. If not set, uses the mount/system
+// value.
 //
-// maxFromLease controls if the maximum renewal period comes from the existing
+// backendMax is the backend's requested increment -- this can be more
-// lease. This means the value of `max` will be replaced with the existing
+// restrictive than the mount/system value but not less.
-// lease duration.
+//
-func LeaseExtend(max, maxSession time.Duration, maxFromLease bool) OperationFunc {
+// systemView is the system view from the calling backend, used to determine
+// and/or correct default/max times.
+func LeaseExtend(backendIncrement, backendMax time.Duration, systemView logical.SystemView) OperationFunc {
 	return func(req *logical.Request, data *FieldData) (*logical.Response, error) {
-		leaseOpts := detectLease(req)
+		var leaseOpts *logical.LeaseOptions
-		if leaseOpts == nil {
+		switch {
+		case req.Auth != nil:
+			leaseOpts = &req.Auth.LeaseOptions
+		case req.Secret != nil:
+			leaseOpts = &req.Secret.LeaseOptions
+		default:
 			return nil, fmt.Errorf("no lease options for request")
 		}
 
-		// Check if we should limit max
+		// Use the mount's configured max unless the backend specifies
-		if maxFromLease {
+		// something more restrictive (perhaps from a role configuration
-			max = leaseOpts.TTL
+		// parameter)
+		max := systemView.MaxLeaseTTL()
+		if backendMax > 0 && backendMax < max {
+			max = backendMax
 		}
 
-		// Sanity check the desired increment
+		// Should never happen, but guard anyways
-		switch {
+		if max < 0 {
-		// Protect against negative leases
+			return nil, fmt.Errorf("max TTL is negative")
-		case leaseOpts.Increment < 0:
-			return logical.ErrorResponse(
-				"increment must be greater than 0"), logical.ErrInvalidRequest
-
-			// If no lease increment, or too large of an increment, use the max
-		case max > 0 && leaseOpts.Increment == 0, max > 0 && leaseOpts.Increment > max:
-			leaseOpts.Increment = max
 		}
 
+		// We cannot go past this time
+		maxValidTime := leaseOpts.IssueTime.UTC().Add(max)
+
 		// Get the current time
 		now := time.Now().UTC()
 
-		// Check if we're passed the issue limit
+		// If we are past the max TTL, we shouldn't be in this function...but
-		var maxSessionTime time.Time
+		// fast path out if we are
-		if maxSession > 0 {
+		if maxValidTime.Before(now) {
-			maxSessionTime = leaseOpts.IssueTime.Add(maxSession)
+			return nil, fmt.Errorf("past the max TTL, cannot renew")
-			if maxSessionTime.Before(now) {
+		}
-				return logical.ErrorResponse(fmt.Sprintf(
+
-					"lease can only be renewed up to %s past original issue",
+		// Basic max safety checks have passed, now let's figure out our
-					maxSession)), logical.ErrInvalidRequest
+		// increment. We'll use the user-supplied value first, then backend-provided default if possible, or the
+		// mount/system default if not.
+		increment := leaseOpts.Increment
+		if increment <= 0 {
+			if backendIncrement > 0 {
+				increment = backendIncrement
+			} else {
+				increment = systemView.DefaultLeaseTTL()
 			}
 		}
 
-		// The new lease is the minimum of the requested Increment
+		// We are proposing a time of the current time plus the increment
-		// or the maxSessionTime
+		proposedExpiration := now.Add(increment)
-		requestedLease := now.Add(leaseOpts.Increment)
+
-		if !maxSessionTime.IsZero() && requestedLease.After(maxSessionTime) {
+		// If the proposed expiration is after the maximum TTL of the lease,
-			requestedLease = maxSessionTime
+		// cap the increment to whatever is left
+		if maxValidTime.Before(proposedExpiration) {
+			increment = maxValidTime.Sub(now)
 		}
 
-		// Determine the requested lease
-		newLeaseDuration := requestedLease.Sub(now)
-
 		// Set the lease
-		leaseOpts.TTL = newLeaseDuration
+		leaseOpts.TTL = increment
 
 		return &logical.Response{Auth: req.Auth, Secret: req.Secret}, nil
 	}
 }
-
-func detectLease(req *logical.Request) *logical.LeaseOptions {
-	if req.Auth != nil {
-		return &req.Auth.LeaseOptions
-	} else if req.Secret != nil {
-		return &req.Secret.LeaseOptions
-	}
-	return nil
-}

logical/framework/lease_test.go

@@ -8,66 +8,80 @@ import (
 )
 
 func TestLeaseExtend(t *testing.T) {
+
+	testSysView := logical.StaticSystemView{
+		DefaultLeaseTTLVal: 5 * time.Hour,
+		MaxLeaseTTLVal:     30 * time.Hour,
+	}
+
 	now := time.Now().UTC().Round(time.Hour)
 
 	cases := map[string]struct {
-		Max          time.Duration
+		BackendDefault time.Duration
-		MaxSession   time.Duration
+		BackendMax     time.Duration
-		Request      time.Duration
+		Increment      time.Duration
-		Result       time.Duration
+		Result         time.Duration
-		MaxFromLease bool
+		Error          bool
-		Error        bool
 	}{
-		"valid request, good bounds": {
+		"valid request, good bounds, increment is preferred": {
-			Max:     30 * time.Hour,
+			BackendDefault: 30 * time.Hour,
-			Request: 1 * time.Hour,
+			Increment:      1 * time.Hour,
-			Result:  1 * time.Hour,
+			Result:         1 * time.Hour,
 		},
 
-		"valid request, zero max": {
+		"valid request, zero backend default, uses increment": {
-			Max:     0,
+			BackendDefault: 0,
-			Request: 1 * time.Hour,
+			Increment:      1 * time.Hour,
-			Result:  1 * time.Hour,
+			Result:         1 * time.Hour,
 		},
 
-		"request is zero": {
+		"lease increment is zero, uses backend default": {
-			Max:     30 * time.Hour,
+			BackendDefault: 30 * time.Hour,
-			Request: 0,
+			Increment:      0,
-			Result:  30 * time.Hour,
+			Result:         30 * time.Hour,
 		},
 
-		"request is too long": {
+		"lease increment and default are zero, uses systemview": {
-			Max:     3 * time.Hour,
+			BackendDefault: 0,
-			Request: 7 * time.Hour,
+			Increment:      0,
-			Result:  3 * time.Hour,
+			Result:         5 * time.Hour,
 		},
 
-		"request would go past max session": {
+		"backend max and associated request are too long": {
-			Max:        9 * time.Hour,
+			BackendDefault: 40 * time.Hour,
-			MaxSession: 5 * time.Hour,
+			BackendMax:     45 * time.Hour,
-			Request:    7 * time.Hour,
+			Result:         30 * time.Hour,
-			Result:     5 * time.Hour,
 		},
 
-		"request within max session": {
+		"all request values are larger than the system view, so the system view limits": {
-			Max:        9 * time.Hour,
+			BackendDefault: 40 * time.Hour,
-			MaxSession: 5 * time.Hour,
+			BackendMax:     50 * time.Hour,
-			Request:    4 * time.Hour,
+			Increment:      40 * time.Hour,
-			Result:     4 * time.Hour,
+			Result:         30 * time.Hour,
 		},
 
-		// Don't think core will allow this, but let's protect against
+		"request within backend max": {
-		// it at multiple layers anyways.
+			BackendDefault: 9 * time.Hour,
-		"request is negative": {
+			BackendMax:     5 * time.Hour,
-			Max:     3 * time.Hour,
+			Increment:      4 * time.Hour,
-			Request: -7 * time.Hour,
+			Result:         4 * time.Hour,
-			Error:   true,
 		},
 
-		"max form lease, request too large": {
+		"request outside backend max": {
-			Request:      10 * time.Hour,
+			BackendDefault: 9 * time.Hour,
-			MaxFromLease: true,
+			BackendMax:     4 * time.Hour,
-			Result:       time.Hour,
+			Increment:      5 * time.Hour,
+			Result:         4 * time.Hour,
+		},
+
+		"request is negative, no backend default, use sysview": {
+			Increment: -7 * time.Hour,
+			Result:    5 * time.Hour,
+		},
+
+		"lease increment too large": {
+			Increment: 40 * time.Hour,
+			Result:    30 * time.Hour,
 		},
 	}
 
@@ -77,12 +91,12 @@ func TestLeaseExtend(t *testing.T) {
 				LeaseOptions: logical.LeaseOptions{
 					TTL:       1 * time.Hour,
 					IssueTime: now,
-					Increment: tc.Request,
+					Increment: tc.Increment,
 				},
 			},
 		}
 
-		callback := LeaseExtend(tc.Max, tc.MaxSession, tc.MaxFromLease)
+		callback := LeaseExtend(tc.BackendDefault, tc.BackendMax, testSysView)
 		resp, err := callback(req, nil)
 		if (err != nil) != tc.Error {
 			t.Fatalf("bad: %s\nerr: %s", name, err)