Set default access request TTL to 1 week. (#35799)

The TTL for a request now defaults to 1 week. This will allow reviewers more
time to review an access request before it disappears.

lib/services/access_request.go

@@ -36,20 +36,25 @@ import (
 	apidefaults "github.com/gravitational/teleport/api/defaults"
 	"github.com/gravitational/teleport/api/types"
 	apiutils "github.com/gravitational/teleport/api/utils"
-	"github.com/gravitational/teleport/lib/defaults"
 	"github.com/gravitational/teleport/lib/tlsca"
 	"github.com/gravitational/teleport/lib/utils"
 	"github.com/gravitational/teleport/lib/utils/parse"
 )
 
-const maxAccessRequestReasonSize = 4096
+const (
+	maxAccessRequestReasonSize = 4096
 
-// A day is sometimes 23 hours, sometimes 25 hours, usually 24 hours.
-const day = 24 * time.Hour
+	// A day is sometimes 23 hours, sometimes 25 hours, usually 24 hours.
+	day = 24 * time.Hour
 
-// maxAccessDuration is the maximum duration that an access request can be
-// granted for.
-const maxAccessDuration = 14 * day
+	// maxAccessDuration is the maximum duration that an access request can be
+	// granted for.
+	maxAccessDuration = 14 * day
+
+	// requestTTL is the the TTL for an access request, i.e. the amount of time that
+	// the access request can be reviewed. Defaults to 1 week.
+	requestTTL = 7 * day
+)
 
 // ValidateAccessRequest validates the AccessRequest and sets default values
 func ValidateAccessRequest(ar types.AccessRequest) error {
@@ -1270,27 +1275,22 @@ func (m *RequestValidator) requestTTL(ctx context.Context, identity tlsca.Identi
 	// If no expiration provided, use default.
 	expiry := r.Expiry()
 	if expiry.IsZero() {
-		expiry = m.clock.Now().UTC().Add(defaults.PendingAccessDuration)
+		expiry = m.clock.Now().UTC().Add(requestTTL)
 	}
 
 	if expiry.Before(m.clock.Now().UTC()) {
 		return 0, trace.BadParameter("invalid request TTL: Access Request can not be created in the past")
 	}
 
-	ttl, err := m.truncateTTL(ctx, identity, expiry, r.GetRoles())
-	if err != nil {
-		return 0, trace.BadParameter("invalid request TTL: %v", err)
-	}
-
 	// Before returning the TTL, validate that the value requested was smaller
 	// than the maximum value allowed. Used to return a sensible error to the
 	// user.
 	requestedTTL := expiry.Sub(m.clock.Now().UTC())
-	if !r.Expiry().IsZero() && requestedTTL > ttl {
-		return 0, trace.BadParameter("invalid request TTL: %v greater than maximum allowed (%v)", requestedTTL.Round(time.Minute), ttl.Round(time.Minute))
+	if !r.Expiry().IsZero() && requestedTTL > requestTTL {
+		return 0, trace.BadParameter("invalid request TTL: %v greater than maximum allowed (%v)", requestedTTL.Round(time.Minute), requestTTL.Round(time.Minute))
 	}
 
-	return ttl, nil
+	return requestedTTL, nil
 }
 
 // sessionTTL calculates the TTL of the elevated certificate that will be issued

lib/services/access_request_test.go

@@ -1608,100 +1608,6 @@ func TestPruneRequestRoles(t *testing.T) {
 	}
 }
 
-// TestRequestTTL verifies that the TTL for the Access Request gets reduced by
-// requested access time and lifetime of the requesting certificate.
-func TestRequestTTL(t *testing.T) {
-	clock := clockwork.NewFakeClock()
-	now := clock.Now().UTC()
-
-	tests := []struct {
-		desc          string
-		expiry        time.Time
-		identity      tlsca.Identity
-		maxSessionTTL time.Duration
-		expectedTTL   time.Duration
-		assertion     require.ErrorAssertionFunc
-	}{
-		{
-			desc:          "access request with ttl, below limit",
-			expiry:        now.Add(8 * time.Hour),
-			identity:      tlsca.Identity{Expires: now.Add(10 * time.Hour)},
-			maxSessionTTL: 10 * time.Hour,
-			expectedTTL:   8 * time.Hour,
-			assertion:     require.NoError,
-		},
-		{
-			desc:          "access request with ttl, above limit",
-			expiry:        now.Add(11 * time.Hour),
-			identity:      tlsca.Identity{Expires: now.Add(10 * time.Hour)},
-			maxSessionTTL: 10 * time.Hour,
-			assertion:     require.Error,
-		},
-		{
-			desc:          "access request without ttl (default ttl)",
-			expiry:        time.Time{},
-			identity:      tlsca.Identity{Expires: now.Add(10 * time.Hour)},
-			maxSessionTTL: 10 * time.Hour,
-			expectedTTL:   defaults.PendingAccessDuration,
-			assertion:     require.NoError,
-		},
-		{
-			desc:          "access request without ttl (default ttl), truncation by identity expiration",
-			expiry:        time.Time{},
-			identity:      tlsca.Identity{Expires: now.Add(12 * time.Minute)},
-			maxSessionTTL: 13 * time.Minute,
-			expectedTTL:   12 * time.Minute,
-			assertion:     require.NoError,
-		},
-		{
-			desc:          "access request without ttl (default ttl), truncation by role max session ttl",
-			expiry:        time.Time{},
-			identity:      tlsca.Identity{Expires: now.Add(14 * time.Hour)},
-			maxSessionTTL: 13 * time.Minute,
-			expectedTTL:   13 * time.Minute,
-			assertion:     require.NoError,
-		},
-	}
-
-	for _, tt := range tests {
-		t.Run(tt.desc, func(t *testing.T) {
-			// Setup test user "foo" and "bar" and the mock auth server that
-			// will return users and roles.
-			uls, err := userloginstate.New(header.Metadata{
-				Name: "foo",
-			}, userloginstate.Spec{
-				Roles: []string{"bar"},
-			})
-			require.NoError(t, err)
-
-			role, err := types.NewRole("bar", types.RoleSpecV6{
-				Options: types.RoleOptions{
-					MaxSessionTTL: types.NewDuration(tt.maxSessionTTL),
-				},
-			})
-			require.NoError(t, err)
-
-			getter := &mockGetter{
-				userStates: map[string]*userloginstate.UserLoginState{"foo": uls},
-				roles:      map[string]types.Role{"bar": role},
-			}
-
-			validator, err := NewRequestValidator(context.Background(), clock, getter, "foo", ExpandVars(true))
-			require.NoError(t, err)
-
-			request, err := types.NewAccessRequest("some-id", "foo", "bar")
-			request.SetExpiry(tt.expiry)
-			require.NoError(t, err)
-
-			ttl, err := validator.requestTTL(context.Background(), tt.identity, request)
-			tt.assertion(t, err)
-			if err == nil {
-				require.Equal(t, tt.expectedTTL, ttl)
-			}
-		})
-	}
-}
-
 // TestSessionTTL verifies that the TTL for elevated access gets reduced by
 // requested access time, lifetime of certificate, and strictest session TTL on
 // any role.

web/packages/teleport/src/AccessRequests/utils.ts

@@ -119,10 +119,10 @@ export function middleValues(
   }));
 }
 
-// Generate a list of middle values between now and the session TTL.
+// Generate a list of middle values between now and the request TTL.
 export function requestTtlMiddleValues(
   created: Date,
-  sessionTTL: Date
+  requestTtl: Date
 ): TimeDuration[] {
   const getInterval = (d: Date) =>
     roundToNearestTenMinutes(
@@ -132,22 +132,22 @@ export function requestTtlMiddleValues(
       })
     );
 
-  if (isAfter(addHours(created, 1), sessionTTL)) {
+  if (isAfter(addHours(created, 1), requestTtl)) {
     return [
       {
-        timestamp: sessionTTL.getTime(),
-        duration: getInterval(sessionTTL),
+        timestamp: requestTtl.getTime(),
+        duration: getInterval(requestTtl),
       },
     ];
   }
 
   const points: Date[] = [];
-  // Staggered hour options, up to the maximum possible session TTL.
-  const hourOptions = [1, 2, 3, 4, 6, 8, 12, 18, 24, 30];
+  // Staggered hour options, up to 1 week.
+  const hourOptions = [1, 2, 3, 4, 6, 8, 12, 18, 24, 30, 168];
 
   for (const h of hourOptions) {
     const t = addHours(created, h);
-    if (isAfter(t, sessionTTL)) {
+    if (isAfter(t, requestTtl)) {
       break;
     }
     points.push(t);