Merge branch 'branch/v17' into bernard/backport-49375-branch/v17

api/client/credentials_test.go

@@ -471,6 +471,7 @@ func TestDynamicIdentityFileCreds(t *testing.T) {
 	require.NoError(t, err)
 	wantTLSCert, err := tls.X509KeyPair(tlsCert, keyPEM)
 	require.NoError(t, err)
+	wantTLSCert.Leaf = nil
 	require.Equal(t, wantTLSCert, *gotTLSCert)
 
 	expiry, ok := cred.Expiry()
@@ -529,6 +530,7 @@ func TestDynamicIdentityFileCreds(t *testing.T) {
 	require.NoError(t, err)
 	wantTLSCert, err = tls.X509KeyPair(secondTLSCertPem, keyPEM)
 	require.NoError(t, err)
+	wantTLSCert.Leaf = nil
 	require.Equal(t, wantTLSCert, *gotTLSCert)
 
 	expiry, ok = cred.Expiry()

constants.go

@@ -410,6 +410,10 @@ const (
 
 	// MinimumEtcdVersion is the minimum version of etcd supported by Teleport
 	MinimumEtcdVersion = "3.3.0"
+
+	// EnvVarAllowNoSecondFactor is used to allow disabling second factor auth
+	// todo(tross): DELETE WHEN ABLE TO
+	EnvVarAllowNoSecondFactor = "TELEPORT_ALLOW_NO_SECOND_FACTOR"
 )
 
 const (

docs/pages/includes/role-spec.mdx

@@ -366,6 +366,14 @@ spec:
       kubernetes_resources:
         - kind: "namespace"
 
+      # 'reason' defines settings for the reason for the access provided by the user.
+      reason:
+        # 'mode' can be either "required" or "optional". Empty string is treated as
+        # "optional". If a role has the request reason mode set to "required", then reason
+        # is required for all Access Requests requesting roles or resources allowed by
+        # this role. It applies only to users who have this role assigned.
+        mode: "optional"
+
       # thresholds specifies minimum amount of approvers and deniers,
       # defaults to 1 for both (enterprise-only)
       thresholds:

lib/auth/init.go

@@ -26,6 +26,7 @@ import (
 	"errors"
 	"fmt"
 	"log/slog"
+	"os"
 	"slices"
 	"strings"
 	"sync"
@@ -780,14 +781,15 @@ func initializeAuthPreference(ctx context.Context, asrv *Server, newAuthPref typ
 		}
 
 		if !shouldReplace {
-			if err := modules.ValidateResource(storedAuthPref); err != nil {
+			if os.Getenv(teleport.EnvVarAllowNoSecondFactor) != "true" {
+				err := modules.ValidateResource(storedAuthPref)
 				if errors.Is(err, modules.ErrCannotDisableSecondFactor) {
 					return trace.Wrap(err, secondFactorUpgradeInstructions)
 				}
-
-				return trace.Wrap(err)
+				if err != nil {
+					return trace.Wrap(err)
+				}
 			}
-
 			return nil
 		}
 

lib/modules/modules.go

@@ -25,6 +25,7 @@ import (
 	"crypto"
 	"errors"
 	"fmt"
+	"os"
 	"runtime"
 	"sync"
 	"time"
@@ -332,7 +333,10 @@ var ErrCannotDisableSecondFactor = errors.New("cannot disable multi-factor authe
 
 // ValidateResource performs additional resource checks.
 func ValidateResource(res types.Resource) error {
-	if GetModules().Features().Cloud || !IsInsecureTestMode() {
+	// todo(tross): DELETE WHEN ABLE TO [remove env var, leave insecure test mode]
+	if GetModules().Features().Cloud ||
+		(os.Getenv(teleport.EnvVarAllowNoSecondFactor) != "yes" && !IsInsecureTestMode()) {
+
 		switch r := res.(type) {
 		case types.AuthPreference:
 			if !r.IsSecondFactorEnforced() {

lib/srv/db/postgres/users.go

@@ -75,7 +75,7 @@ func (e *Engine) ActivateUser(ctx context.Context, sessionCtx *common.Session) e
 	// bookkeeping group or stored procedures get deleted or changed offband.
 	logger := e.Log.With("user", sessionCtx.DatabaseUser)
 	err = withRetry(ctx, logger, func() error {
-		return trace.Wrap(e.updateAutoUsersRole(ctx, conn))
+		return trace.Wrap(e.updateAutoUsersRole(ctx, conn, sessionCtx.Database.GetAdminUser().Name))
 	})
 	if err != nil {
 		return trace.Wrap(err)
@@ -401,17 +401,45 @@ func (e *Engine) deleteUserRedshift(ctx context.Context, sessionCtx *common.Sess
 
 // updateAutoUsersRole ensures the bookkeeping role for auto-provisioned users
 // is present.
-func (e *Engine) updateAutoUsersRole(ctx context.Context, conn *pgx.Conn) error {
+func (e *Engine) updateAutoUsersRole(ctx context.Context, conn *pgx.Conn, adminUser string) error {
 	_, err := conn.Exec(ctx, fmt.Sprintf("create role %q", teleportAutoUserRole))
 	if err != nil {
 		if !strings.Contains(err.Error(), "already exists") {
 			return trace.Wrap(err)
 		}
-		e.Log.DebugContext(ctx, "PostgreSQL role already exists.", "role", teleportAutoUserRole)
+		e.Log.DebugContext(ctx, "PostgreSQL role already exists", "role", teleportAutoUserRole)
 	} else {
-		e.Log.DebugContext(ctx, "Created PostgreSQL role.", "role", teleportAutoUserRole)
+		e.Log.DebugContext(ctx, "Created PostgreSQL role", "role", teleportAutoUserRole)
+	}
+
+	// v16 Postgres changed the role grant permissions model such that you can
+	// no longer grant non-superuser role membership just by having the
+	// CREATEROLE attribute.
+	// On v16 Postgres, when a role is created the creator is automatically
+	// granted that role with "INHERIT FALSE, SET FALSE, ADMIN OPTION" options.
+	// Prior to v16 Postgres that grant is not automatically made, because
+	// the CREATEROLE attribute alone was sufficient to grant the role to
+	// others.
+	// This is the only role that is created and granted to others by the
+	// Teleport database admin.
+	// It grants the auto user role to every role it provisions.
+	// To avoid breaking user auto-provisioning for customers who upgrade from
+	// v15 postgres to v16, we should grant this role with the admin option to
+	// ourselves after creating it.
+	// Also note that the grant syntax in v15 postgres and below does not
+	// support WITH INHERIT FALSE or WITH SET FALSE syntax, so we only specify
+	// WITH ADMIN OPTION.
+	// See: https://www.postgresql.org/docs/16/release-16.html
+	stmt := fmt.Sprintf("grant role %q to %q WITH ADMIN OPTION", teleportAutoUserRole, adminUser)
+	_, err = conn.Exec(ctx, stmt)
+	if err != nil {
+		if !strings.Contains(err.Error(), "cannot be granted back") && !strings.Contains(err.Error(), "already") {
+			e.Log.DebugContext(ctx, "Failed to grant required role to the Teleport database admin, user auto-provisioning may not work until the database admin is granted the role by a superuser",
+				"role", teleportAutoUserRole,
+				"database_admin", adminUser,
+			)
+		}
 	}
-
 	return nil
 }
 

tool/tctl/common/bots_command.go

@@ -114,10 +114,10 @@ func (c *BotsCommand) Initialize(app *kingpin.Application, config *servicecfg.Co
 
 	c.botsInstances = bots.Command("instances", "Manage bot instances.").Alias("instance")
 
-	c.botsInstancesShow = c.botsInstances.Command("show", "Shows information about a specific bot instance").Alias("get").Alias("describe")
+	c.botsInstancesShow = c.botsInstances.Command("show", "Shows information about a specific bot instance.").Alias("get").Alias("describe")
 	c.botsInstancesShow.Arg("id", "The full ID of the bot instance, in the form of [bot name]/[uuid]").Required().StringVar(&c.instanceID)
 
-	c.botsInstancesList = c.botsInstances.Command("list", "List bot instances").Alias("ls")
+	c.botsInstancesList = c.botsInstances.Command("list", "List bot instances.").Alias("ls")
 	c.botsInstancesList.Arg("name", "The name of the bot from which to list instances. If unset, lists instances from all bots.").StringVar(&c.botName)
 
 	c.botsInstancesAdd = c.botsInstances.Command("add", "Join a new instance onto an existing bot.").Alias("join")

tool/tctl/common/resource_command.go

@@ -1573,6 +1573,8 @@ func (rc *ResourceCommand) Delete(ctx context.Context, client *authclient.Client
 		types.KindInstaller,
 		types.KindUIConfig,
 		types.KindNetworkRestrictions,
+		types.KindAutoUpdateConfig,
+		types.KindAutoUpdateVersion,
 	}
 	if !slices.Contains(singletonResources, rc.ref.Kind) && (rc.ref.Kind == "" || rc.ref.Name == "") {
 		return trace.BadParameter("provide a full resource name to delete, for example:\n$ tctl rm cluster/east\n")
@@ -1972,6 +1974,16 @@ func (rc *ResourceCommand) Delete(ctx context.Context, client *authclient.Client
 			return trace.Wrap(err)
 		}
 		fmt.Printf("static host user %q has been deleted\n", rc.ref.Name)
+	case types.KindAutoUpdateConfig:
+		if err := client.DeleteAutoUpdateConfig(ctx); err != nil {
+			return trace.Wrap(err)
+		}
+		fmt.Printf("AutoUpdateConfig has been deleted\n")
+	case types.KindAutoUpdateVersion:
+		if err := client.DeleteAutoUpdateVersion(ctx); err != nil {
+			return trace.Wrap(err)
+		}
+		fmt.Printf("AutoUpdateVersion has been deleted\n")
 	default:
 		return trace.BadParameter("deleting resources of type %q is not supported", rc.ref.Kind)
 	}

tool/tctl/common/resource_command_test.go

@@ -2325,6 +2325,12 @@ version: v1
 		protocmp.IgnoreFields(&headerv1.Metadata{}, "revision"),
 		protocmp.Transform(),
 	))
+
+	// Delete the resource
+	_, err = runResourceCommand(t, clt, []string{"rm", types.KindAutoUpdateConfig})
+	require.NoError(t, err)
+	_, err = runResourceCommand(t, clt, []string{"get", types.KindAutoUpdateConfig})
+	require.ErrorContains(t, err, "autoupdate_config \"autoupdate-config\" doesn't exist")
 }
 
 func testCreateAutoUpdateVersion(t *testing.T, clt *authclient.Client) {
@@ -2361,6 +2367,12 @@ version: v1
 		protocmp.IgnoreFields(&headerv1.Metadata{}, "revision"),
 		protocmp.Transform(),
 	))
+
+	// Delete the resource
+	_, err = runResourceCommand(t, clt, []string{"rm", types.KindAutoUpdateVersion})
+	require.NoError(t, err)
+	_, err = runResourceCommand(t, clt, []string{"get", types.KindAutoUpdateVersion})
+	require.ErrorContains(t, err, "autoupdate_version \"autoupdate-version\" doesn't exist")
 }
 
 func testCreateDynamicWindowsDesktop(t *testing.T, clt *authclient.Client) {