diff --git a/lib/auth/apiserver.go b/lib/auth/apiserver.go index 9c04eb12624b8..94ea591f74165 100644 --- a/lib/auth/apiserver.go +++ b/lib/auth/apiserver.go @@ -516,7 +516,7 @@ func (s *APIServer) upsertUser(auth *ServerWithRoles, w http.ResponseWriter, r * return nil, trace.Wrap(err) } - err = auth.UpsertUser(user) + err = auth.UpsertUser(r.Context(), user) if err != nil { return nil, trace.Wrap(err) } diff --git a/lib/auth/auth_test.go b/lib/auth/auth_test.go index cd846b079e67e..2ad3ed6225d1a 100644 --- a/lib/auth/auth_test.go +++ b/lib/auth/auth_test.go @@ -830,52 +830,6 @@ func TestUpdateConfig(t *testing.T) { }})) } -func TestCreateAndUpdateUserEventsEmitted(t *testing.T) { - t.Parallel() - s := newAuthSuite(t) - - user, err := types.NewUser("some-user") - require.NoError(t, err) - - ctx := context.Background() - - // test create user, happy path - user.SetCreatedBy(types.CreatedBy{ - User: types.UserRef{Name: "some-auth-user"}, - }) - err = s.a.CreateUser(ctx, user) - require.NoError(t, err) - require.Equal(t, s.mockEmitter.LastEvent().GetType(), events.UserCreateEvent) - require.Equal(t, s.mockEmitter.LastEvent().(*apievents.UserCreate).User, "some-auth-user") - s.mockEmitter.Reset() - - // test create user with existing user - err = s.a.CreateUser(ctx, user) - require.True(t, trace.IsAlreadyExists(err)) - require.Nil(t, s.mockEmitter.LastEvent()) - - // test createdBy gets set to default - user2, err := types.NewUser("some-other-user") - require.NoError(t, err) - err = s.a.CreateUser(ctx, user2) - require.NoError(t, err) - require.Equal(t, s.mockEmitter.LastEvent().(*apievents.UserCreate).User, teleport.UserSystem) - s.mockEmitter.Reset() - - // test update on non-existent user - user3, err := types.NewUser("non-existent-user") - require.NoError(t, err) - err = s.a.UpdateUser(ctx, user3) - require.True(t, trace.IsNotFound(err)) - require.Nil(t, s.mockEmitter.LastEvent()) - - // test update user - err = s.a.UpdateUser(ctx, user) - require.NoError(t, err) - require.Equal(t, s.mockEmitter.LastEvent().GetType(), events.UserUpdatedEvent) - require.Equal(t, s.mockEmitter.LastEvent().(*apievents.UserCreate).User, teleport.UserSystem) -} - func TestTrustedClusterCRUDEventEmitted(t *testing.T) { t.Parallel() s := newAuthSuite(t) diff --git a/lib/auth/auth_with_roles.go b/lib/auth/auth_with_roles.go index bec14057533ad..12cf0329b29d1 100644 --- a/lib/auth/auth_with_roles.go +++ b/lib/auth/auth_with_roles.go @@ -3012,7 +3012,7 @@ func (a *ServerWithRoles) UpdateUser(ctx context.Context, user types.User) error return a.authServer.UpdateUser(ctx, user) } -func (a *ServerWithRoles) UpsertUser(u types.User) error { +func (a *ServerWithRoles) UpsertUser(ctx context.Context, u types.User) error { if err := a.action(apidefaults.Namespace, types.KindUser, types.VerbCreate, types.VerbUpdate); err != nil { return trace.Wrap(err) } @@ -3023,7 +3023,37 @@ func (a *ServerWithRoles) UpsertUser(u types.User) error { User: types.UserRef{Name: a.context.User.GetName()}, }) } - return a.authServer.UpsertUser(u) + + if err := a.authServer.UpsertUser(u); err != nil { + return trace.Wrap(err) + } + + // Emitting the audit event is done here and not within [Server.UpsertUser] because + // the UserMetadata for the event cannot be derived without the [context.Context]. + var connectorName string + if u.GetCreatedBy().Connector == nil { + connectorName = constants.LocalConnector + } else { + connectorName = u.GetCreatedBy().Connector.ID + } + + if err := a.authServer.emitter.EmitAuditEvent(ctx, &apievents.UserCreate{ + Metadata: apievents.Metadata{ + Type: events.UserCreateEvent, + Code: events.UserCreateCode, + }, + UserMetadata: authz.ClientUserMetadata(ctx), + ResourceMetadata: apievents.ResourceMetadata{ + Name: u.GetName(), + Expires: u.Expiry(), + }, + Connector: connectorName, + Roles: u.GetRoles(), + }); err != nil { + log.WithError(err).Warn("Failed to emit user upsert event.") + } + + return nil } // UpdateAndSwapUser exists on [ServerWithRoles] only for compatibility with diff --git a/lib/auth/auth_with_roles_test.go b/lib/auth/auth_with_roles_test.go index f0730085ae557..93e67441d51c6 100644 --- a/lib/auth/auth_with_roles_test.go +++ b/lib/auth/auth_with_roles_test.go @@ -4916,6 +4916,82 @@ func TestUpdateHeadlessAuthenticationState(t *testing.T) { } } +type fakeAuditLog struct { + Emitter *eventstest.MockEmitter + events.IAuditLog +} + +func (f fakeAuditLog) EmitAuditEvent(ctx context.Context, e apievents.AuditEvent) error { + return f.Emitter.EmitAuditEvent(ctx, e) +} + +func TestCreateAndUpdateUserEventsEmitted(t *testing.T) { + t.Parallel() + ctx := context.Background() + m := &fakeAuditLog{Emitter: &eventstest.MockEmitter{}} + srv, err := NewTestAuthServer(TestAuthServerConfig{Dir: t.TempDir(), AuditLog: m}) + require.NoError(t, err) + + // Server used to create users and roles. + setupAuthContext, err := srv.Authorizer.Authorize(authz.ContextWithUser(ctx, TestAdmin().I)) + require.NoError(t, err) + setupServer := &ServerWithRoles{ + authServer: srv.AuthServer, + alog: srv.AuditLog, + context: *setupAuthContext, + } + + user, err := types.NewUser("some-user") + require.NoError(t, err) + + localUser := authz.LocalUser{Username: "test", Identity: tlsca.Identity{Username: "test"}} + + // test create user, happy path + err = setupServer.CreateUser(authz.ContextWithUser(ctx, localUser), user) + require.NoError(t, err) + require.Equal(t, events.UserCreateEvent, m.Emitter.LastEvent().GetType()) + require.Equal(t, "test", m.Emitter.LastEvent().(*apievents.UserCreate).User) + m.Emitter.Reset() + + // test create user with existing user + err = setupServer.CreateUser(ctx, user) + require.True(t, trace.IsAlreadyExists(err)) + require.Nil(t, m.Emitter.LastEvent()) + + // test createdBy gets set to default + user2, err := types.NewUser("some-other-user") + require.NoError(t, err) + err = setupServer.CreateUser(ctx, user2) + require.NoError(t, err) + require.Equal(t, teleport.UserSystem, m.Emitter.LastEvent().(*apievents.UserCreate).User) + m.Emitter.Reset() + + // test update on non-existent user + user3, err := types.NewUser("non-existent-user") + require.NoError(t, err) + err = setupServer.UpdateUser(ctx, user3) + require.True(t, trace.IsNotFound(err)) + require.Nil(t, m.Emitter.LastEvent()) + + // test update user + err = setupServer.UpdateUser(authz.ContextWithUser(ctx, localUser), user) + require.NoError(t, err) + require.Equal(t, events.UserUpdatedEvent, m.Emitter.LastEvent().GetType()) + require.Equal(t, "test", m.Emitter.LastEvent().(*apievents.UserCreate).User) + + err = setupServer.UpsertUser(authz.ContextWithUser(ctx, localUser), user) + require.NoError(t, err) + require.Equal(t, events.UserCreateEvent, m.Emitter.LastEvent().GetType()) + require.Equal(t, "test", m.Emitter.LastEvent().(*apievents.UserCreate).User) + m.Emitter.Reset() + + err = setupServer.UpsertUser(ctx, user) + require.NoError(t, err) + require.Equal(t, events.UserCreateEvent, m.Emitter.LastEvent().GetType()) + require.Equal(t, teleport.UserSystem, m.Emitter.LastEvent().(*apievents.UserCreate).User) + m.Emitter.Reset() +} + func TestCreateSnowflakeSession(t *testing.T) { t.Parallel() srv := newTestTLSServer(t) diff --git a/lib/auth/user.go b/lib/auth/user.go index a92454137548c..30089335315b5 100644 --- a/lib/auth/user.go +++ b/lib/auth/user.go @@ -63,7 +63,7 @@ func (a *Server) CreateUser(ctx context.Context, user types.User) error { Type: events.UserCreateEvent, Code: events.UserCreateCode, }, - UserMetadata: authz.ClientUserMetadataWithUser(ctx, user.GetCreatedBy().User.Name), + UserMetadata: authz.ClientUserMetadata(ctx), ResourceMetadata: apievents.ResourceMetadata{ Name: user.GetName(), Expires: user.Expiry(), @@ -116,31 +116,6 @@ func (a *Server) UpsertUser(user types.User) error { return trace.Wrap(err) } - var connectorName string - if user.GetCreatedBy().Connector == nil { - connectorName = constants.LocalConnector - } else { - connectorName = user.GetCreatedBy().Connector.ID - } - - if err := a.emitter.EmitAuditEvent(a.closeCtx, &apievents.UserCreate{ - Metadata: apievents.Metadata{ - Type: events.UserCreateEvent, - Code: events.UserCreateCode, - }, - UserMetadata: apievents.UserMetadata{ - User: user.GetName(), - }, - ResourceMetadata: apievents.ResourceMetadata{ - Name: user.GetName(), - Expires: user.Expiry(), - }, - Connector: connectorName, - Roles: user.GetRoles(), - }); err != nil { - log.WithError(err).Warn("Failed to emit user upsert event.") - } - return nil }