[v18] write operations on integration resource now require MFA as administrative actions (#69521)

* write operations on integration resource now require MFA as
administrative actions

* fix admin MFA for update integration

* authorizerForAdminUser

* move test cases to their own buckets

* fix lint

lib/auth/integration/integrationv1/awsoidc_test.go

@@ -68,14 +68,14 @@ func TestGenerateAWSOIDCToken(t *testing.T) {
 	_, err = localClient.CreateIntegration(ctx, ig)
 	require.NoError(t, err)
 
-	ctx = authorizerForDummyUser(t, ctx, types.RoleSpecV6{
+	ctx = authorizerForAdminUser(t, ctx, types.RoleSpecV6{
 		Allow: types.RoleConditions{Rules: []types.Rule{
 			{Resources: []string{types.KindIntegration}, Verbs: []string{types.VerbUse}},
 		}},
 	}, localClient)
 
 	t.Run("requesting with an user should return access denied", func(t *testing.T) {
-		ctx = authorizerForDummyUser(t, ctx, types.RoleSpecV6{
+		ctx = authorizerForAdminUser(t, ctx, types.RoleSpecV6{
 			Allow: types.RoleConditions{Rules: []types.Rule{
 				{Resources: []string{types.KindIntegration}, Verbs: []string{types.VerbUse}},
 			}},
@@ -240,7 +240,7 @@ func TestRBAC(t *testing.T) {
 			}}},
 		}
 
-		userCtx := authorizerForDummyUser(t, ctx, role, localClient)
+		userCtx := authorizerForAdminUser(t, ctx, role, localClient)
 
 		for _, tt := range []endpointSubtest{
 			{
@@ -335,7 +335,7 @@ func TestRBAC(t *testing.T) {
 			}}},
 		}
 
-		userCtx := authorizerForDummyUser(t, ctx, role, localClient)
+		userCtx := authorizerForAdminUser(t, ctx, role, localClient)
 
 		for _, tt := range []endpointSubtest{
 			{

lib/auth/integration/integrationv1/awsra_test.go

@@ -55,14 +55,14 @@ func TestGenerateAWSRACredentials(t *testing.T) {
 	_, err = localClient.CreateIntegration(ctx, ig)
 	require.NoError(t, err)
 
-	ctx = authorizerForDummyUser(t, ctx, types.RoleSpecV6{
+	ctx = authorizerForAdminUser(t, ctx, types.RoleSpecV6{
 		Allow: types.RoleConditions{Rules: []types.Rule{
 			{Resources: []string{types.KindIntegration}, Verbs: []string{types.VerbUse}},
 		}},
 	}, localClient)
 
 	t.Run("requesting with an user should return access denied", func(t *testing.T) {
-		ctx = authorizerForDummyUser(t, ctx, types.RoleSpecV6{
+		ctx = authorizerForAdminUser(t, ctx, types.RoleSpecV6{
 			Allow: types.RoleConditions{Rules: []types.Rule{
 				{Resources: []string{types.KindIntegration}, Verbs: []string{types.VerbUse}},
 			}},
@@ -108,7 +108,7 @@ func TestAWSRolesAnywhereProfileSyncFilters(t *testing.T) {
 	ca := newCertAuthority(t, types.AWSRACA, clusterName)
 	ctx, localClient, resourceSvc := initSvc(t, ca, clusterName, proxyPublicAddr)
 
-	ctx = authorizerForDummyUser(t, ctx, types.RoleSpecV6{
+	ctx = authorizerForAdminUser(t, ctx, types.RoleSpecV6{
 		Allow: types.RoleConditions{Rules: []types.Rule{
 			{Resources: []string{types.KindIntegration}, Verbs: []string{types.VerbCreate, types.VerbUpdate}},
 		}},
@@ -188,7 +188,7 @@ func TestAWSRolesAnywherePing(t *testing.T) {
 	_, err = localClient.CreateIntegration(ctx, ig)
 	require.NoError(t, err)
 
-	ctx = authorizerForDummyUser(t, ctx, types.RoleSpecV6{
+	ctx = authorizerForAdminUser(t, ctx, types.RoleSpecV6{
 		Allow: types.RoleConditions{Rules: []types.Rule{
 			{Resources: []string{types.KindIntegration}, Verbs: []string{types.VerbUse}},
 		}},

lib/auth/integration/integrationv1/azureoidc_test.go

@@ -56,7 +56,7 @@ func TestGenerateAzureOIDCToken(t *testing.T) {
 
 	t.Run("only Auth, Discovery, and Proxy roles should be able to generate Azure tokens", func(t *testing.T) {
 		// A dummy user should not be able to generate Azure OIDC tokens
-		ctx = authorizerForDummyUser(t, ctx, types.RoleSpecV6{
+		ctx = authorizerForAdminUser(t, ctx, types.RoleSpecV6{
 			Allow: types.RoleConditions{Rules: []types.Rule{
 				{Resources: []string{types.KindIntegration}, Verbs: []string{types.VerbUse}},
 			}},

lib/auth/integration/integrationv1/credentials_test.go

@@ -95,7 +95,7 @@ func TestExportIntegrationCertAuthorities(t *testing.T) {
 		{
 			name:        "not allowed",
 			integration: githubIntegration.GetName(),
-			identity:    authorizerForDummyUser(t, ctx, types.RoleSpecV6{}, localClient),
+			identity:    authorizerForAdminUser(t, ctx, types.RoleSpecV6{}, localClient),
 			check: func(t *testing.T, resp *integrationpb.ExportIntegrationCertAuthoritiesResponse, err error) {
 				t.Helper()
 				require.Nil(t, resp)

lib/auth/integration/integrationv1/service.go

@@ -253,6 +253,9 @@ func (s *Service) CreateIntegration(ctx context.Context, req *integrationpb.Crea
 	if err := authCtx.CheckAccessToKind(types.KindIntegration, types.VerbCreate); err != nil {
 		return nil, trace.Wrap(err)
 	}
+	if err := authCtx.AuthorizeAdminActionAllowReusedMFA(); err != nil {
+		return nil, trace.Wrap(err)
+	}
 
 	switch req.Integration.GetSubKind() {
 	case types.IntegrationSubKindGitHub:
@@ -317,6 +320,9 @@ func (s *Service) UpdateIntegration(ctx context.Context, req *integrationpb.Upda
 	if err := authCtx.CheckAccessToKind(types.KindIntegration, types.VerbUpdate); err != nil {
 		return nil, trace.Wrap(err)
 	}
+	if err := authCtx.AuthorizeAdminActionAllowReusedMFA(); err != nil {
+		return nil, trace.Wrap(err)
+	}
 
 	if err := validateAWSRolesAnywhereProfileFilters(req.Integration); err != nil {
 		return nil, trace.Wrap(err)
@@ -399,6 +405,9 @@ func (s *Service) DeleteIntegration(ctx context.Context, req *integrationpb.Dele
 	if err := authCtx.CheckAccessToKind(types.KindIntegration, types.VerbDelete); err != nil {
 		return nil, trace.Wrap(err)
 	}
+	if err := authCtx.AuthorizeAdminActionAllowReusedMFA(); err != nil {
+		return nil, trace.Wrap(err)
+	}
 
 	ig, err := s.cache.GetIntegration(ctx, req.GetName())
 	if err != nil {

lib/auth/integration/integrationv1/service_test.go

@@ -30,9 +30,13 @@ import (
 	"github.com/gravitational/trace"
 	"github.com/stretchr/testify/assert"
 	"github.com/stretchr/testify/require"
+	"google.golang.org/grpc/metadata"
 
+	proto "github.com/gravitational/teleport/api/client/proto"
 	"github.com/gravitational/teleport/api/defaults"
 	integrationpb "github.com/gravitational/teleport/api/gen/proto/go/teleport/integration/v1"
+	mfav1 "github.com/gravitational/teleport/api/gen/proto/go/teleport/mfa/v1"
+	"github.com/gravitational/teleport/api/mfa"
 	"github.com/gravitational/teleport/api/types"
 	"github.com/gravitational/teleport/api/types/discoveryconfig"
 	"github.com/gravitational/teleport/api/types/externalauditstorage"
@@ -86,6 +90,7 @@ func TestIntegrationCRUD(t *testing.T) {
 		Name            string
 		Role            types.RoleSpecV6
 		IntegrationName string
+		DummyUser       bool
 		Setup           func(t *testing.T, igName string)
 		Test            func(ctx context.Context, resourceSvc *Service, igName string) error
 		Validate        func(t *testing.T, igName string)
@@ -94,7 +99,8 @@ func TestIntegrationCRUD(t *testing.T) {
 	}{
 		// Read
 		{
-			Name: "allowed read access to integrations",
+			Name:      "allowed read access to integrations",
+			DummyUser: true,
 			Role: types.RoleSpecV6{
 				Allow: types.RoleConditions{Rules: []types.Rule{{
 					Resources: []string{types.KindIntegration},
@@ -249,6 +255,26 @@ func TestIntegrationCRUD(t *testing.T) {
 			ErrAssertion: noError,
 		},
 
+		{
+			Name:      "create fails without admin MFA",
+			DummyUser: true,
+			Role: types.RoleSpecV6{
+				Allow: types.RoleConditions{Rules: []types.Rule{{
+					Resources: []string{types.KindIntegration},
+					Verbs:     []string{types.VerbCreate},
+				}}},
+			},
+			Test: func(ctx context.Context, resourceSvc *Service, igName string) error {
+				ig := sampleIntegrationFn(t, igName)
+				_, err := resourceSvc.CreateIntegration(
+					ctx,
+					integrationpb.CreateIntegrationRequest_builder{Integration: ig.(*types.IntegrationV1)}.Build(),
+				)
+				return err
+			},
+			ErrAssertion: trace.IsAccessDenied,
+		},
+
 		// Update
 		{
 			Name: "no access to update integration",
@@ -397,6 +423,30 @@ func TestIntegrationCRUD(t *testing.T) {
 			ErrAssertion: trace.IsBadParameter,
 		},
 
+		{
+			Name:      "update fails without admin MFA",
+			DummyUser: true,
+			Role: types.RoleSpecV6{
+				Allow: types.RoleConditions{Rules: []types.Rule{{
+					Resources: []string{types.KindIntegration},
+					Verbs:     []string{types.VerbUpdate, types.VerbCreate},
+				}}},
+			},
+			Setup: func(t *testing.T, igName string) {
+				_, err := localClient.CreateIntegration(ctx, sampleIntegrationFn(t, igName))
+				require.NoError(t, err)
+			},
+			Test: func(ctx context.Context, resourceSvc *Service, igName string) error {
+				ig := sampleIntegrationFn(t, igName)
+				_, err := resourceSvc.UpdateIntegration(
+					ctx,
+					integrationpb.UpdateIntegrationRequest_builder{Integration: ig.(*types.IntegrationV1)}.Build(),
+				)
+				return err
+			},
+			ErrAssertion: trace.IsAccessDenied,
+		},
+
 		// Delete
 		{
 			Name: "no access to delete integration",
@@ -772,6 +822,29 @@ func TestIntegrationCRUD(t *testing.T) {
 			ErrAssertion: noError,
 		},
 
+		{
+			Name:      "delete fails without admin MFA",
+			DummyUser: true,
+			Role: types.RoleSpecV6{
+				Allow: types.RoleConditions{Rules: []types.Rule{{
+					Resources: []string{types.KindIntegration},
+					Verbs:     []string{types.VerbDelete, types.VerbCreate},
+				}}},
+			},
+			Setup: func(t *testing.T, igName string) {
+				_, err := localClient.CreateIntegration(ctx, sampleIntegrationFn(t, igName))
+				require.NoError(t, err)
+			},
+			Test: func(ctx context.Context, resourceSvc *Service, igName string) error {
+				_, err := resourceSvc.DeleteIntegration(
+					ctx,
+					integrationpb.DeleteIntegrationRequest_builder{Name: igName}.Build(),
+				)
+				return err
+			},
+			ErrAssertion: trace.IsAccessDenied,
+		},
+
 		// Delete all
 		{
 			Name: "delete all integrations fails",
@@ -793,7 +866,12 @@ func TestIntegrationCRUD(t *testing.T) {
 	for _, tc := range tt {
 		tc := tc
 		t.Run(tc.Name, func(t *testing.T) {
-			localCtx := authorizerForDummyUser(t, ctx, tc.Role, localClient)
+			var localCtx context.Context
+			if tc.DummyUser {
+				localCtx = authorizerForDummyUser(t, ctx, tc.Role, localClient)
+			} else {
+				localCtx = authorizerForAdminUser(t, ctx, tc.Role, localClient)
+			}
 			igName := cmp.Or(tc.IntegrationName, uuid.NewString())
 			if tc.Setup != nil {
 				tc.Setup(t, igName)
@@ -814,7 +892,9 @@ func TestIntegrationCRUD(t *testing.T) {
 	}
 }
 
+// authorizerForDummyUser creates a user context without admin MFA verification.
 func authorizerForDummyUser(t *testing.T, ctx context.Context, roleSpec types.RoleSpecV6, localClient localClient) context.Context {
+	t.Helper()
 	// Create role
 	roleName := "role-" + uuid.NewString()
 	role, err := types.NewRole(roleName, roleSpec)
@@ -839,6 +919,26 @@ func authorizerForDummyUser(t *testing.T, ctx context.Context, roleSpec types.Ro
 	})
 }
 
+func authorizerForAdminUser(t *testing.T, ctx context.Context, roleSpec types.RoleSpecV6, localClient localClient) context.Context {
+	t.Helper()
+	ctx = authorizerForDummyUser(t, ctx, roleSpec, localClient)
+	encoded, err := mfa.EncodeMFAChallengeResponseCredentials(&proto.MFAAuthenticateResponse{
+		Response: &proto.MFAAuthenticateResponse_TOTP{
+			TOTP: &proto.TOTPResponse{Code: "mock"},
+		},
+	})
+	require.NoError(t, err)
+	return metadata.NewIncomingContext(ctx, metadata.MD{
+		mfa.ResponseMetadataKey: {encoded},
+	})
+}
+
+type fakeMFAAuthenticator struct{}
+
+func (f *fakeMFAAuthenticator) ValidateMFAAuthResponse(_ context.Context, _ *proto.MFAAuthenticateResponse, _ string, _ *mfav1.ChallengeExtensions) (*authz.MFAAuthData, error) {
+	return &authz.MFAAuthData{}, nil
+}
+
 type localClient interface {
 	CreateUser(ctx context.Context, user types.User) (types.User, error)
 	CreateRole(ctx context.Context, role types.Role) (types.Role, error)
@@ -934,7 +1034,13 @@ func initSvc(t *testing.T, ca types.CertAuthority, clusterName string, proxyPubl
 	easSvc := local.NewExternalAuditStorageService(backend)
 	pluginSvc := local.NewPluginsService(backend)
 
-	_, err = clusterConfigSvc.UpsertAuthPreference(ctx, types.DefaultAuthPreference())
+	authPref, err := types.NewAuthPreference(types.AuthPreferenceSpecV2{
+		Type:         "local",
+		SecondFactor: "webauthn",
+		Webauthn:     &types.Webauthn{RPID: "localhost"},
+	})
+	require.NoError(t, err)
+	_, err = clusterConfigSvc.UpsertAuthPreference(ctx, authPref)
 	require.NoError(t, err)
 	require.NoError(t, clusterConfigSvc.SetClusterAuditConfig(ctx, types.DefaultClusterAuditConfig()))
 	_, err = clusterConfigSvc.UpsertClusterNetworkingConfig(ctx, types.DefaultClusterNetworkingConfig())
@@ -965,9 +1071,10 @@ func initSvc(t *testing.T, ca types.CertAuthority, clusterName string, proxyPubl
 	require.NoError(t, err)
 
 	authorizer, err := authz.NewAuthorizer(authz.AuthorizerOpts{
-		ClusterName: clusterName,
-		AccessPoint: accessPoint,
-		LockWatcher: lockWatcher,
+		ClusterName:      clusterName,
+		AccessPoint:      accessPoint,
+		LockWatcher:      lockWatcher,
+		MFAAuthenticator: &fakeMFAAuthenticator{},
 	})
 	require.NoError(t, err)
 

lib/web/integrations.go

@@ -34,6 +34,7 @@ import (
 	integrationv1 "github.com/gravitational/teleport/api/gen/proto/go/teleport/integration/v1"
 	pluginspb "github.com/gravitational/teleport/api/gen/proto/go/teleport/plugins/v1"
 	usertasksv1 "github.com/gravitational/teleport/api/gen/proto/go/teleport/usertasks/v1"
+	"github.com/gravitational/teleport/api/mfa"
 	"github.com/gravitational/teleport/api/types"
 	"github.com/gravitational/teleport/api/types/discoveryconfig"
 	"github.com/gravitational/teleport/api/types/usertasks"
@@ -168,7 +169,10 @@ func (h *Handler) integrationsUpdate(w http.ResponseWriter, r *http.Request, p h
 		return nil, trace.Wrap(err)
 	}
 
-	integration, err := clt.GetIntegration(r.Context(), integrationName)
+	// Strip MFA from context for the read call so the MFA response is not
+	// consumed before the UpdateIntegration call that actually needs it.
+	getCtx := mfa.ContextWithMFAResponse(r.Context(), nil)
+	integration, err := clt.GetIntegration(getCtx, integrationName)
 	if err != nil {
 		return nil, trace.Wrap(err)
 	}

tool/tctl/common/resources/integration.go

@@ -78,9 +78,8 @@ func integrationHandler() Handler {
 		updateHandler: updateIntegration,
 		deleteHandler: deleteIntegration,
 		singleton:     false,
-		// TODO(greedy52) add admin action MFA for integrations.
-		mfaRequired: false,
-		description: "An integration with an external service such as AWS, GitHub, or Azure.",
+		mfaRequired:   true,
+		description:   "An integration with an external service such as AWS, GitHub, or Azure.",
 	}
 }
 

tool/tctl/common/resources/resource_test.go

@@ -77,7 +77,7 @@ func TestHandlers(t *testing.T) {
 				})
 				return ig
 			},
-			checkMFARequired: require.False,
+			checkMFARequired: require.True,
 		},
 	}