From 38b2919570d0ab99ad609d593c12e4fa8dbd0766 Mon Sep 17 00:00:00 2001 From: joerger Date: Fri, 17 Nov 2023 16:49:53 -0800 Subject: [PATCH 1/2] Require admin action MFA for SAML Idp service provider CRUD. --- lib/auth/auth_with_roles.go | 207 ++++++++++++++++++++---------------- 1 file changed, 118 insertions(+), 89 deletions(-) diff --git a/lib/auth/auth_with_roles.go b/lib/auth/auth_with_roles.go index 516ca314d72f6..5b7964d3d5a68 100644 --- a/lib/auth/auth_with_roles.go +++ b/lib/auth/auth_with_roles.go @@ -6373,128 +6373,157 @@ func (a *ServerWithRoles) GetSAMLIdPServiceProvider(ctx context.Context, name st } // CreateSAMLIdPServiceProvider creates a new SAML IdP service provider resource. -func (a *ServerWithRoles) CreateSAMLIdPServiceProvider(ctx context.Context, sp types.SAMLIdPServiceProvider) error { - code := events.SAMLIdPServiceProviderCreateFailureCode - var err error - if err = a.action(apidefaults.Namespace, types.KindSAMLIdPServiceProvider, types.VerbCreate); err == nil { - err = a.authServer.CreateSAMLIdPServiceProvider(ctx, sp) - if err == nil { - code = events.SAMLIdPServiceProviderCreateCode +func (a *ServerWithRoles) CreateSAMLIdPServiceProvider(ctx context.Context, sp types.SAMLIdPServiceProvider) (err error) { + defer func() { + code := events.SAMLIdPServiceProviderCreateCode + if err != nil { + code = events.SAMLIdPServiceProviderCreateFailureCode } + + if emitErr := a.authServer.emitter.EmitAuditEvent(a.authServer.closeCtx, &apievents.SAMLIdPServiceProviderCreate{ + Metadata: apievents.Metadata{ + Type: events.SAMLIdPServiceProviderCreateEvent, + Code: code, + }, + ResourceMetadata: apievents.ResourceMetadata{ + Name: sp.GetName(), + UpdatedBy: authz.ClientUsername(ctx), + }, + SAMLIdPServiceProviderMetadata: apievents.SAMLIdPServiceProviderMetadata{ + ServiceProviderEntityID: sp.GetEntityID(), + }, + }); emitErr != nil { + log.WithError(trace.NewAggregate(emitErr, err)).Warn("Failed to emit SAML IdP service provider created event.") + } + }() + + if err = a.action(apidefaults.Namespace, types.KindSAMLIdPServiceProvider, types.VerbCreate); err != nil { + return trace.Wrap(err) } - if emitErr := a.authServer.emitter.EmitAuditEvent(a.authServer.closeCtx, &apievents.SAMLIdPServiceProviderCreate{ - Metadata: apievents.Metadata{ - Type: events.SAMLIdPServiceProviderCreateEvent, - Code: code, - }, - ResourceMetadata: apievents.ResourceMetadata{ - Name: sp.GetName(), - UpdatedBy: authz.ClientUsername(ctx), - }, - SAMLIdPServiceProviderMetadata: apievents.SAMLIdPServiceProviderMetadata{ - ServiceProviderEntityID: sp.GetEntityID(), - }, - }); emitErr != nil { - log.WithError(trace.NewAggregate(emitErr, err)).Warn("Failed to emit SAML IdP service provider created event.") + if err = authz.AuthorizeAdminAction(ctx, &a.context); err != nil { + return trace.Wrap(err) } + err = a.authServer.CreateSAMLIdPServiceProvider(ctx, sp) return trace.Wrap(err) } // UpdateSAMLIdPServiceProvider updates an existing SAML IdP service provider resource. -func (a *ServerWithRoles) UpdateSAMLIdPServiceProvider(ctx context.Context, sp types.SAMLIdPServiceProvider) error { - code := events.SAMLIdPServiceProviderUpdateFailureCode - var err error - if err = a.action(apidefaults.Namespace, types.KindSAMLIdPServiceProvider, types.VerbUpdate); err == nil { - err = a.authServer.UpdateSAMLIdPServiceProvider(ctx, sp) - if err == nil { - code = events.SAMLIdPServiceProviderUpdateCode +func (a *ServerWithRoles) UpdateSAMLIdPServiceProvider(ctx context.Context, sp types.SAMLIdPServiceProvider) (err error) { + defer func() { + code := events.SAMLIdPServiceProviderUpdateCode + if err != nil { + code = events.SAMLIdPServiceProviderUpdateFailureCode + } + + if emitErr := a.authServer.emitter.EmitAuditEvent(a.authServer.closeCtx, &apievents.SAMLIdPServiceProviderUpdate{ + Metadata: apievents.Metadata{ + Type: events.SAMLIdPServiceProviderUpdateEvent, + Code: code, + }, + ResourceMetadata: apievents.ResourceMetadata{ + Name: sp.GetName(), + UpdatedBy: authz.ClientUsername(ctx), + }, + SAMLIdPServiceProviderMetadata: apievents.SAMLIdPServiceProviderMetadata{ + ServiceProviderEntityID: sp.GetEntityID(), + }, + }); emitErr != nil { + log.WithError(trace.NewAggregate(emitErr, err)).Warn("Failed to emit SAML IdP service provider updated event.") } + }() + + if err := a.action(apidefaults.Namespace, types.KindSAMLIdPServiceProvider, types.VerbUpdate); err != nil { + return trace.Wrap(err) } - if emitErr := a.authServer.emitter.EmitAuditEvent(a.authServer.closeCtx, &apievents.SAMLIdPServiceProviderUpdate{ - Metadata: apievents.Metadata{ - Type: events.SAMLIdPServiceProviderUpdateEvent, - Code: code, - }, - ResourceMetadata: apievents.ResourceMetadata{ - Name: sp.GetName(), - UpdatedBy: authz.ClientUsername(ctx), - }, - SAMLIdPServiceProviderMetadata: apievents.SAMLIdPServiceProviderMetadata{ - ServiceProviderEntityID: sp.GetEntityID(), - }, - }); emitErr != nil { - log.WithError(trace.NewAggregate(emitErr, err)).Warn("Failed to emit SAML IdP service provider updated event.") + if err := authz.AuthorizeAdminAction(ctx, &a.context); err != nil { + return trace.Wrap(err) } + err = a.authServer.UpdateSAMLIdPServiceProvider(ctx, sp) return trace.Wrap(err) } // DeleteSAMLIdPServiceProvider removes the specified SAML IdP service provider resource. -func (a *ServerWithRoles) DeleteSAMLIdPServiceProvider(ctx context.Context, name string) error { +func (a *ServerWithRoles) DeleteSAMLIdPServiceProvider(ctx context.Context, name string) (err error) { var entityID string - code := events.SAMLIdPServiceProviderDeleteFailureCode - var err error - if err = a.action(apidefaults.Namespace, types.KindSAMLIdPServiceProvider, types.VerbDelete); err == nil { - var sp types.SAMLIdPServiceProvider - // Get the service provider so we can emit its entity ID later. - sp, err = a.authServer.GetSAMLIdPServiceProvider(ctx, name) - if err == nil { - name = sp.GetName() - entityID = sp.GetEntityID() - - // Delete the actual service provider. - err = a.authServer.DeleteSAMLIdPServiceProvider(ctx, name) - if err == nil { - code = events.SAMLIdPServiceProviderDeleteCode - } + defer func() { + code := events.SAMLIdPServiceProviderDeleteCode + if err != nil { + code = events.SAMLIdPServiceProviderDeleteFailureCode } + + if emitErr := a.authServer.emitter.EmitAuditEvent(a.authServer.closeCtx, &apievents.SAMLIdPServiceProviderDelete{ + Metadata: apievents.Metadata{ + Type: events.SAMLIdPServiceProviderDeleteEvent, + Code: code, + }, + ResourceMetadata: apievents.ResourceMetadata{ + Name: name, + UpdatedBy: authz.ClientUsername(ctx), + }, + SAMLIdPServiceProviderMetadata: apievents.SAMLIdPServiceProviderMetadata{ + ServiceProviderEntityID: entityID, + }, + }); emitErr != nil { + log.WithError(trace.NewAggregate(emitErr, err)).Warn("Failed to emit SAML IdP service provider deleted event.") + } + }() + + if err := a.action(apidefaults.Namespace, types.KindSAMLIdPServiceProvider, types.VerbDelete); err != nil { + return trace.Wrap(err) } - if emitErr := a.authServer.emitter.EmitAuditEvent(a.authServer.closeCtx, &apievents.SAMLIdPServiceProviderDelete{ - Metadata: apievents.Metadata{ - Type: events.SAMLIdPServiceProviderDeleteEvent, - Code: code, - }, - ResourceMetadata: apievents.ResourceMetadata{ - Name: name, - UpdatedBy: authz.ClientUsername(ctx), - }, - SAMLIdPServiceProviderMetadata: apievents.SAMLIdPServiceProviderMetadata{ - ServiceProviderEntityID: entityID, - }, - }); emitErr != nil { - log.WithError(trace.NewAggregate(emitErr, err)).Warn("Failed to emit SAML IdP service provider deleted event.") + if err := authz.AuthorizeAdminAction(ctx, &a.context); err != nil { + return trace.Wrap(err) } + // Get the service provider so we can emit its entity ID later. + sp, err := a.authServer.GetSAMLIdPServiceProvider(ctx, name) + if err != nil { + return trace.Wrap(err) + } + + name = sp.GetName() + entityID = sp.GetEntityID() + + // Delete the actual service provider. + err = a.authServer.DeleteSAMLIdPServiceProvider(ctx, name) return trace.Wrap(err) } // DeleteAllSAMLIdPServiceProviders removes all SAML IdP service providers. -func (a *ServerWithRoles) DeleteAllSAMLIdPServiceProviders(ctx context.Context) error { - code := events.SAMLIdPServiceProviderDeleteAllFailureCode - var err error - if err = a.action(apidefaults.Namespace, types.KindSAMLIdPServiceProvider, types.VerbDelete); err == nil { - err = a.authServer.DeleteAllSAMLIdPServiceProviders(ctx) - if err == nil { - code = events.SAMLIdPServiceProviderDeleteAllCode +func (a *ServerWithRoles) DeleteAllSAMLIdPServiceProviders(ctx context.Context) (err error) { + defer func() { + code := events.SAMLIdPServiceProviderDeleteAllCode + if err != nil { + code = events.SAMLIdPServiceProviderDeleteAllFailureCode + } + + if emitErr := a.authServer.emitter.EmitAuditEvent(a.authServer.closeCtx, &apievents.SAMLIdPServiceProviderDeleteAll{ + Metadata: apievents.Metadata{ + Type: events.SAMLIdPServiceProviderDeleteAllEvent, + Code: code, + }, + ResourceMetadata: apievents.ResourceMetadata{ + UpdatedBy: authz.ClientUsername(ctx), + }, + }); emitErr != nil { + log.WithError(trace.NewAggregate(emitErr, err)).Warn("Failed to emit SAML IdP service provider deleted all event.") } + }() + + if err := a.action(apidefaults.Namespace, types.KindSAMLIdPServiceProvider, types.VerbDelete); err != nil { + return trace.Wrap(err) } - if emitErr := a.authServer.emitter.EmitAuditEvent(a.authServer.closeCtx, &apievents.SAMLIdPServiceProviderDeleteAll{ - Metadata: apievents.Metadata{ - Type: events.SAMLIdPServiceProviderDeleteAllEvent, - Code: code, - }, - ResourceMetadata: apievents.ResourceMetadata{ - UpdatedBy: authz.ClientUsername(ctx), - }, - }); emitErr != nil { - log.WithError(trace.NewAggregate(emitErr, err)).Warn("Failed to emit SAML IdP service provider deleted all event.") + if err := authz.AuthorizeAdminAction(ctx, &a.context); err != nil { + return trace.Wrap(err) } + err = a.authServer.DeleteAllSAMLIdPServiceProviders(ctx) return trace.Wrap(err) } From 3f1b86cb3175878a5531399da9298ee12c3f140b Mon Sep 17 00:00:00 2001 From: joerger Date: Mon, 11 Dec 2023 11:32:26 -0800 Subject: [PATCH 2/2] Add tctl test. --- tool/tctl/common/admin_action_test.go | 50 +++++++++++++++++++++++++++ 1 file changed, 50 insertions(+) diff --git a/tool/tctl/common/admin_action_test.go b/tool/tctl/common/admin_action_test.go index e5f20b886616c..81d9ec799dbd7 100644 --- a/tool/tctl/common/admin_action_test.go +++ b/tool/tctl/common/admin_action_test.go @@ -62,6 +62,7 @@ func TestAdminActionMFA(t *testing.T) { t.Run("OIDCConnector", s.testOIDCConnector) t.Run("SAMLConnector", s.testSAMLConnector) t.Run("GithubConnector", s.testGithubConnector) + t.Run("SAMLIdpServiceProvider", s.testSAMLIdpServiceProvider) } func (s *adminActionTestSuite) testUsers(t *testing.T) { @@ -409,6 +410,55 @@ func (s *adminActionTestSuite) testGithubConnector(t *testing.T) { }) } +func (s *adminActionTestSuite) testSAMLIdpServiceProvider(t *testing.T) { + ctx := context.Background() + + sp, err := types.NewSAMLIdPServiceProvider(types.Metadata{ + Name: "test-saml-app", + }, types.SAMLIdPServiceProviderSpecV1{ + // A test entity descriptor from https://sptest.iamshowcase.com/testsp_metadata.xml. + EntityDescriptor: ` + + + urn:oasis:names:tc:SAML:1.1:nameid-format:unspecified + urn:oasis:names:tc:SAML:1.1:nameid-format:emailAddress + + + `, + EntityID: "test-saml-app", + }) + require.NoError(t, err) + + CreateSAMLIdPServiceProvider := func() error { + return s.authServer.CreateSAMLIdPServiceProvider(ctx, sp) + } + + getSAMLIdPServiceProvider := func() (types.Resource, error) { + return s.authServer.GetSAMLIdPServiceProvider(ctx, sp.GetName()) + } + + deleteSAMLIdPServiceProvider := func() error { + return s.authServer.DeleteSAMLIdPServiceProvider(ctx, sp.GetName()) + } + + t.Run("ResourceCommands", func(t *testing.T) { + s.testResourceCommand(t, ctx, resourceCommandTestCase{ + resource: sp, + resourceCreate: CreateSAMLIdPServiceProvider, + resourceDelete: deleteSAMLIdPServiceProvider, + }) + }) + + t.Run("EditCommand", func(t *testing.T) { + s.testEditCommand(t, ctx, editCommandTestCase{ + resourceRef: getResourceRef(sp), + resourceCreate: CreateSAMLIdPServiceProvider, + resourceGet: getSAMLIdPServiceProvider, + resourceDelete: deleteSAMLIdPServiceProvider, + }) + }) +} + type resourceCommandTestCase struct { resource types.Resource resourceCreate func() error