From cc9105350b185860bebfc778df79e1bec33dadc4 Mon Sep 17 00:00:00 2001 From: aman Date: Fri, 17 Jul 2026 13:47:58 +0530 Subject: [PATCH 1/3] fix(connect): log audit failures instead of dropping them Audit log writes in the connect handlers discarded their error. Check the error and record it through the existing error logger. Requests still succeed when the audit write fails. Co-Authored-By: Claude Fable 5 --- internal/api/v1beta1connect/authorize.go | 8 +++- .../api/v1beta1connect/billing_customer.go | 7 +++- internal/api/v1beta1connect/group.go | 14 ++++++- internal/api/v1beta1connect/kyc.go | 8 +++- internal/api/v1beta1connect/organization.go | 14 +++++-- .../api/v1beta1connect/permission_check.go | 10 ++++- internal/api/v1beta1connect/policy.go | 17 +++++++-- internal/api/v1beta1connect/project.go | 26 ++++++++++--- internal/api/v1beta1connect/resource.go | 21 +++++++--- internal/api/v1beta1connect/role.go | 38 ++++++++++++++----- internal/api/v1beta1connect/serviceuser.go | 16 ++++++-- internal/api/v1beta1connect/user.go | 32 ++++++++++++---- 12 files changed, 160 insertions(+), 51 deletions(-) diff --git a/internal/api/v1beta1connect/authorize.go b/internal/api/v1beta1connect/authorize.go index 7b16762a1..3eafd43a4 100644 --- a/internal/api/v1beta1connect/authorize.go +++ b/internal/api/v1beta1connect/authorize.go @@ -162,10 +162,14 @@ func (h *ConnectHandler) CheckPlanEntitlement(ctx context.Context, obj relation. return err } for _, customr := range customers { - audit.GetAuditor(ctx, orgID).LogWithAttrs(audit.BillingEntitlementCheckedEvent, audit.Target{ + if err := audit.GetAuditor(ctx, orgID).LogWithAttrs(audit.BillingEntitlementCheckedEvent, audit.Target{ ID: customr.ID, Type: "billing_account", - }, map[string]string{}) + }, map[string]string{}); err != nil { + errorLogger.LogServiceError(ctx, request, "CheckPlanEntitlement.AuditLog", err, + "customer_id", customr.ID, + "org_id", orgID) + } if err := h.entitlementService.CheckPlanEligibility(ctx, customr.ID); err != nil { errorLogger.LogUnexpectedError(ctx, request, "CheckPlanEntitlement", err, "customer_id", customr.ID, diff --git a/internal/api/v1beta1connect/billing_customer.go b/internal/api/v1beta1connect/billing_customer.go index 76352a93d..e43c93e73 100644 --- a/internal/api/v1beta1connect/billing_customer.go +++ b/internal/api/v1beta1connect/billing_customer.go @@ -373,13 +373,16 @@ func (h *ConnectHandler) UpdateBillingAccountDetails(ctx context.Context, reques // Add audit log - infer org_id from billing account customerOb, err := h.customerService.GetByID(ctx, request.Msg.GetId()) if err == nil { - audit.GetAuditor(ctx, customerOb.OrgID).LogWithAttrs(audit.BillingAccountDetailsUpdatedEvent, audit.Target{ + if err := audit.GetAuditor(ctx, customerOb.OrgID).LogWithAttrs(audit.BillingAccountDetailsUpdatedEvent, audit.Target{ ID: request.Msg.GetId(), Type: "billing_account", }, map[string]string{ "credit_min": fmt.Sprintf("%d", details.CreditMin), "due_in_days": fmt.Sprintf("%d", details.DueInDays), - }) + }); err != nil { + errorLogger.LogServiceError(ctx, request, "UpdateBillingAccountDetails.AuditLog", err, + "customer_id", request.Msg.GetId()) + } } else { errorLogger.LogServiceError(ctx, request, "UpdateBillingAccountDetails.GetByID", err, "customer_id", request.Msg.GetId()) diff --git a/internal/api/v1beta1connect/group.go b/internal/api/v1beta1connect/group.go index dadc5b6bd..d832d56de 100644 --- a/internal/api/v1beta1connect/group.go +++ b/internal/api/v1beta1connect/group.go @@ -148,6 +148,7 @@ func (h *ConnectHandler) listGroupUsers(ctx context.Context, groupID, roleID str } func (h *ConnectHandler) CreateGroup(ctx context.Context, request *connect.Request[frontierv1beta1.CreateGroupRequest]) (*connect.Response[frontierv1beta1.CreateGroupResponse], error) { + errorLogger := NewErrorLogger() if request.Msg.GetBody() == nil { return nil, connect.NewError(connect.CodeInvalidArgument, ErrBadRequest) } @@ -201,7 +202,11 @@ func (h *ConnectHandler) CreateGroup(ctx context.Context, request *connect.Reque return nil, connect.NewError(connect.CodeInternal, fmt.Errorf("CreateGroup: entity_id=%s: %w", newGroup.ID, err)) } - audit.GetAuditor(ctx, request.Msg.GetOrgId()).Log(audit.GroupCreatedEvent, audit.GroupTarget(newGroup.ID)) + if err := audit.GetAuditor(ctx, request.Msg.GetOrgId()).Log(audit.GroupCreatedEvent, audit.GroupTarget(newGroup.ID)); err != nil { + errorLogger.LogServiceError(ctx, request, "CreateGroup.AuditLog", err, + "group_id", newGroup.ID, + "org_id", request.Msg.GetOrgId()) + } return connect.NewResponse(&frontierv1beta1.CreateGroupResponse{Group: &groupPB}), nil } @@ -241,6 +246,7 @@ func (h *ConnectHandler) GetGroup(ctx context.Context, request *connect.Request[ } func (h *ConnectHandler) UpdateGroup(ctx context.Context, request *connect.Request[frontierv1beta1.UpdateGroupRequest]) (*connect.Response[frontierv1beta1.UpdateGroupResponse], error) { + errorLogger := NewErrorLogger() if request.Msg.GetBody() == nil { return nil, connect.NewError(connect.CodeInvalidArgument, ErrBadRequest) } @@ -279,7 +285,11 @@ func (h *ConnectHandler) UpdateGroup(ctx context.Context, request *connect.Reque return nil, connect.NewError(connect.CodeInternal, fmt.Errorf("UpdateGroup: entity_id=%s: %w", updatedGroup.ID, err)) } - audit.GetAuditor(ctx, updatedGroup.OrganizationID).Log(audit.GroupUpdatedEvent, audit.GroupTarget(updatedGroup.ID)) + if err := audit.GetAuditor(ctx, updatedGroup.OrganizationID).Log(audit.GroupUpdatedEvent, audit.GroupTarget(updatedGroup.ID)); err != nil { + errorLogger.LogServiceError(ctx, request, "UpdateGroup.AuditLog", err, + "group_id", updatedGroup.ID, + "org_id", updatedGroup.OrganizationID) + } return connect.NewResponse(&frontierv1beta1.UpdateGroupResponse{Group: &groupPB}), nil } diff --git a/internal/api/v1beta1connect/kyc.go b/internal/api/v1beta1connect/kyc.go index 1865d1e96..03bcd4874 100644 --- a/internal/api/v1beta1connect/kyc.go +++ b/internal/api/v1beta1connect/kyc.go @@ -14,6 +14,7 @@ import ( ) func (h *ConnectHandler) SetOrganizationKyc(ctx context.Context, request *connect.Request[frontierv1beta1.SetOrganizationKycRequest]) (*connect.Response[frontierv1beta1.SetOrganizationKycResponse], error) { + errorLogger := NewErrorLogger() orgKyc, err := h.orgKycService.SetKyc(ctx, kyc.KYC{ OrgID: request.Msg.GetOrgId(), Status: request.Msg.GetStatus(), @@ -33,11 +34,14 @@ func (h *ConnectHandler) SetOrganizationKyc(ctx context.Context, request *connec } // Add audit log - audit.GetAuditor(ctx, orgKyc.OrgID). + if err := audit.GetAuditor(ctx, orgKyc.OrgID). LogWithAttrs(audit.OrgKycUpdatedEvent, audit.OrgTarget(orgKyc.OrgID), map[string]string{ "status": strconv.FormatBool(orgKyc.Status), "link": orgKyc.Link, - }) + }); err != nil { + errorLogger.LogServiceError(ctx, request, "SetOrganizationKyc.AuditLog", err, + "org_id", orgKyc.OrgID) + } return connect.NewResponse(&frontierv1beta1.SetOrganizationKycResponse{OrganizationKyc: transformOrgKycToPB(orgKyc)}), nil } diff --git a/internal/api/v1beta1connect/organization.go b/internal/api/v1beta1connect/organization.go index 3f87ee008..8be417bc4 100644 --- a/internal/api/v1beta1connect/organization.go +++ b/internal/api/v1beta1connect/organization.go @@ -151,6 +151,7 @@ func (h *ConnectHandler) CreateOrganization(ctx context.Context, request *connec } func (h *ConnectHandler) AdminCreateOrganization(ctx context.Context, request *connect.Request[frontierv1beta1.AdminCreateOrganizationRequest]) (*connect.Response[frontierv1beta1.AdminCreateOrganizationResponse], error) { + errorLogger := NewErrorLogger() metaDataMap := metadata.Build(request.Msg.GetBody().GetMetadata().AsMap()) if err := h.metaSchemaService.Validate(metaDataMap, orgMetaSchema); err != nil { @@ -181,14 +182,18 @@ func (h *ConnectHandler) AdminCreateOrganization(ctx context.Context, request *c return nil, connect.NewError(connect.CodeInternal, fmt.Errorf("AdminCreateOrganization: entity_id=%s: %w", newOrg.ID, err)) } - audit.GetAuditor(ctx, newOrg.ID).LogWithAttrs(audit.OrgCreatedEvent, audit.OrgTarget(newOrg.ID), map[string]string{ + if err := audit.GetAuditor(ctx, newOrg.ID).LogWithAttrs(audit.OrgCreatedEvent, audit.OrgTarget(newOrg.ID), map[string]string{ "title": newOrg.Title, "name": newOrg.Name, - }) + }); err != nil { + errorLogger.LogServiceError(ctx, request, "AdminCreateOrganization.AuditLog", err, + "org_id", newOrg.ID) + } return connect.NewResponse(&frontierv1beta1.AdminCreateOrganizationResponse{Organization: orgPB}), nil } func (h *ConnectHandler) UpdateOrganization(ctx context.Context, request *connect.Request[frontierv1beta1.UpdateOrganizationRequest]) (*connect.Response[frontierv1beta1.UpdateOrganizationResponse], error) { + errorLogger := NewErrorLogger() if request.Msg.GetBody() == nil { return nil, connect.NewError(connect.CodeInvalidArgument, ErrBadRequest) } @@ -233,7 +238,10 @@ func (h *ConnectHandler) UpdateOrganization(ctx context.Context, request *connec return nil, connect.NewError(connect.CodeInternal, fmt.Errorf("UpdateOrganization: entity_id=%s: %w", updatedOrg.ID, err)) } - audit.GetAuditor(ctx, updatedOrg.ID).Log(audit.OrgUpdatedEvent, audit.OrgTarget(updatedOrg.ID)) + if err := audit.GetAuditor(ctx, updatedOrg.ID).Log(audit.OrgUpdatedEvent, audit.OrgTarget(updatedOrg.ID)); err != nil { + errorLogger.LogServiceError(ctx, request, "UpdateOrganization.AuditLog", err, + "org_id", updatedOrg.ID) + } return connect.NewResponse(&frontierv1beta1.UpdateOrganizationResponse{Organization: orgPB}), nil } diff --git a/internal/api/v1beta1connect/permission_check.go b/internal/api/v1beta1connect/permission_check.go index 3fc2ea8d1..51cbb2647 100644 --- a/internal/api/v1beta1connect/permission_check.go +++ b/internal/api/v1beta1connect/permission_check.go @@ -3,6 +3,7 @@ package v1beta1connect import ( "context" "fmt" + "log/slog" "connectrpc.com/connect" "github.com/raystack/frontier/core/audit" @@ -20,12 +21,17 @@ func logAuditForCheck(ctx context.Context, result bool, objectID string, objectN if !result { auditStatus = "failure" } - audit.GetAuditor(ctx, schema.PlatformOrgID.String()).LogWithAttrs(audit.PermissionCheckedEvent, audit.Target{ + if err := audit.GetAuditor(ctx, schema.PlatformOrgID.String()).LogWithAttrs(audit.PermissionCheckedEvent, audit.Target{ ID: objectID, Type: objectNamespace, }, map[string]string{ "status": auditStatus, - }) + }); err != nil { + slog.ErrorContext(ctx, "PermissionCheck.AuditLog operation failed", + "error", err, + "object_id", objectID, + "object_namespace", objectNamespace) + } } func (h *ConnectHandler) getPermissionName(ctx context.Context, ns, name string) (string, error) { diff --git a/internal/api/v1beta1connect/policy.go b/internal/api/v1beta1connect/policy.go index 7dd6e551d..0ee3e6dde 100644 --- a/internal/api/v1beta1connect/policy.go +++ b/internal/api/v1beta1connect/policy.go @@ -4,6 +4,7 @@ import ( "context" "errors" "fmt" + "log/slog" "connectrpc.com/connect" "github.com/raystack/frontier/core/audit" @@ -121,10 +122,13 @@ func (h *ConnectHandler) DeletePolicy(ctx context.Context, request *connect.Requ } } - audit.GetAuditor(ctx, schema.PlatformOrgID.String()).Log(audit.PolicyDeletedEvent, audit.Target{ + if err := audit.GetAuditor(ctx, schema.PlatformOrgID.String()).Log(audit.PolicyDeletedEvent, audit.Target{ ID: policyID, Type: "app/policy", - }) + }); err != nil { + errorLogger.LogServiceError(ctx, request, "DeletePolicy.AuditLog", err, + "policy_id", policyID) + } return connect.NewResponse(&frontierv1beta1.DeletePolicyResponse{}), nil } @@ -235,7 +239,7 @@ func transformPolicyToPB(policy policy.Policy) (*frontierv1beta1.Policy, error) } func auditPolicyCreationEvent(ctx context.Context, policyCreated policy.Policy) { - audit.GetAuditor(ctx, schema.PlatformOrgID.String()). + if err := audit.GetAuditor(ctx, schema.PlatformOrgID.String()). LogWithAttrs(audit.PolicyCreatedEvent, audit.Target{ ID: policyCreated.ResourceID, Type: policyCreated.ResourceType, @@ -243,7 +247,12 @@ func auditPolicyCreationEvent(ctx context.Context, policyCreated policy.Policy) "role_id": policyCreated.RoleID, "principal_id": policyCreated.PrincipalID, "principal_type": policyCreated.PrincipalType, - }) + }); err != nil { + slog.ErrorContext(ctx, "PolicyCreation.AuditLog operation failed", + "error", err, + "policy_id", policyCreated.ID, + "resource_id", policyCreated.ResourceID) + } } // resolveFilter resolves the filter from the request and returns a policy filter diff --git a/internal/api/v1beta1connect/project.go b/internal/api/v1beta1connect/project.go index eb7048c50..dbd162567 100644 --- a/internal/api/v1beta1connect/project.go +++ b/internal/api/v1beta1connect/project.go @@ -72,7 +72,10 @@ func (h *ConnectHandler) CreateProject(ctx context.Context, request *connect.Req if err != nil { return nil, connect.NewError(connect.CodeInternal, fmt.Errorf("CreateProject: entity_id=%s: %w", newProject.ID, err)) } - auditor.Log(audit.ProjectCreatedEvent, audit.ProjectTarget(newProject.ID)) + if err := auditor.Log(audit.ProjectCreatedEvent, audit.ProjectTarget(newProject.ID)); err != nil { + errorLogger.LogServiceError(ctx, request, "CreateProject.AuditLog", err, + "project_id", newProject.ID) + } return connect.NewResponse(&frontierv1beta1.CreateProjectResponse{Project: projectPB}), nil } @@ -122,7 +125,10 @@ func (h *ConnectHandler) UpdateProject(ctx context.Context, request *connect.Req return nil, connect.NewError(connect.CodeInternal, fmt.Errorf("UpdateProject: entity_id=%s: %w", updatedProject.ID, err)) } - audit.GetAuditor(ctx, updatedProject.Organization.ID).Log(audit.ProjectUpdatedEvent, audit.ProjectTarget(updatedProject.ID)) + if err := audit.GetAuditor(ctx, updatedProject.Organization.ID).Log(audit.ProjectUpdatedEvent, audit.ProjectTarget(updatedProject.ID)); err != nil { + errorLogger.LogServiceError(ctx, request, "UpdateProject.AuditLog", err, + "project_id", updatedProject.ID) + } return connect.NewResponse(&frontierv1beta1.UpdateProjectResponse{Project: projectPB}), nil } @@ -415,11 +421,15 @@ func (h *ConnectHandler) SetProjectMemberRole(ctx context.Context, request *conn } } - audit.GetAuditor(ctx, "").LogWithAttrs(audit.ProjectMemberRoleChangedEvent, audit.ProjectTarget(projectID), map[string]string{ + if err := audit.GetAuditor(ctx, "").LogWithAttrs(audit.ProjectMemberRoleChangedEvent, audit.ProjectTarget(projectID), map[string]string{ "principal_id": principalID, "principal_type": principalType, "role_id": roleID, - }) + }); err != nil { + errorLogger.LogServiceError(ctx, request, "SetProjectMemberRole.AuditLog", err, + "project_id", projectID, + "principal_id", principalID) + } return connect.NewResponse(&frontierv1beta1.SetProjectMemberRoleResponse{}), nil } @@ -448,10 +458,14 @@ func (h *ConnectHandler) RemoveProjectMember(ctx context.Context, request *conne } } - audit.GetAuditor(ctx, "").LogWithAttrs(audit.ProjectMemberRemovedEvent, audit.ProjectTarget(projectID), map[string]string{ + if err := audit.GetAuditor(ctx, "").LogWithAttrs(audit.ProjectMemberRemovedEvent, audit.ProjectTarget(projectID), map[string]string{ "principal_id": principalID, "principal_type": principalType, - }) + }); err != nil { + errorLogger.LogServiceError(ctx, request, "RemoveProjectMember.AuditLog", err, + "project_id", projectID, + "principal_id", principalID) + } return connect.NewResponse(&frontierv1beta1.RemoveProjectMemberResponse{}), nil } diff --git a/internal/api/v1beta1connect/resource.go b/internal/api/v1beta1connect/resource.go index d3de6cab4..a4d9e6901 100644 --- a/internal/api/v1beta1connect/resource.go +++ b/internal/api/v1beta1connect/resource.go @@ -125,10 +125,13 @@ func (h *ConnectHandler) CreateProjectResource(ctx context.Context, request *con return nil, connect.NewError(connect.CodeInternal, fmt.Errorf("CreateProjectResource: entity_id=%s: %w", newResource.ID, err)) } - audit.GetAuditor(ctx, parentProject.Organization.ID).Log(audit.ResourceCreatedEvent, audit.Target{ + if err := audit.GetAuditor(ctx, parentProject.Organization.ID).Log(audit.ResourceCreatedEvent, audit.Target{ ID: newResource.ID, Type: newResource.NamespaceID, - }) + }); err != nil { + errorLogger.LogServiceError(ctx, request, "CreateProjectResource.AuditLog", err, + "resource_id", newResource.ID) + } return connect.NewResponse(&frontierv1beta1.CreateProjectResourceResponse{ Resource: resourcePB, }), nil @@ -225,10 +228,13 @@ func (h *ConnectHandler) UpdateProjectResource(ctx context.Context, request *con return nil, connect.NewError(connect.CodeInternal, fmt.Errorf("UpdateProjectResource: entity_id=%s: %w", updatedResource.ID, err)) } - audit.GetAuditor(ctx, parentProject.Organization.ID).Log(audit.ResourceUpdatedEvent, audit.Target{ + if err := audit.GetAuditor(ctx, parentProject.Organization.ID).Log(audit.ResourceUpdatedEvent, audit.Target{ ID: updatedResource.ID, Type: updatedResource.NamespaceID, - }) + }); err != nil { + errorLogger.LogServiceError(ctx, request, "UpdateProjectResource.AuditLog", err, + "resource_id", updatedResource.ID) + } return connect.NewResponse(&frontierv1beta1.UpdateProjectResourceResponse{ Resource: resourcePB, }), nil @@ -263,10 +269,13 @@ func (h *ConnectHandler) DeleteProjectResource(ctx context.Context, request *con return nil, connect.NewError(connect.CodeInternal, fmt.Errorf("DeleteProjectResource: resource_id=%s project_id=%s namespace=%s: %w", resourceID, resourceToDel.ProjectID, resourceToDel.NamespaceID, err)) } - audit.GetAuditor(ctx, parentProject.Organization.ID).Log(audit.ResourceDeletedEvent, audit.Target{ + if err := audit.GetAuditor(ctx, parentProject.Organization.ID).Log(audit.ResourceDeletedEvent, audit.Target{ ID: resourceID, Type: resourceToDel.NamespaceID, - }) + }); err != nil { + errorLogger.LogServiceError(ctx, request, "DeleteProjectResource.AuditLog", err, + "resource_id", resourceID) + } return connect.NewResponse(&frontierv1beta1.DeleteProjectResourceResponse{}), nil } diff --git a/internal/api/v1beta1connect/role.go b/internal/api/v1beta1connect/role.go index 545c3ae6c..271d2dd3a 100644 --- a/internal/api/v1beta1connect/role.go +++ b/internal/api/v1beta1connect/role.go @@ -73,11 +73,14 @@ func (h *ConnectHandler) CreateRole(ctx context.Context, request *connect.Reques return nil, connect.NewError(connect.CodeInternal, fmt.Errorf("CreateRole: entity_id=%s: %w", newRole.ID, err)) } - audit.GetAuditor(ctx, schema.PlatformOrgID.String()).Log(audit.RoleCreatedEvent, audit.Target{ + if err := audit.GetAuditor(ctx, schema.PlatformOrgID.String()).Log(audit.RoleCreatedEvent, audit.Target{ ID: newRole.ID, Type: schema.RoleNamespace, Name: newRole.Name, - }) + }); err != nil { + errorLogger.LogServiceError(ctx, request, "CreateRole.AuditLog", err, + "role_id", newRole.ID) + } return connect.NewResponse(&frontierv1beta1.CreateRoleResponse{Role: &rolePB}), nil } @@ -129,11 +132,14 @@ func (h *ConnectHandler) UpdateRole(ctx context.Context, request *connect.Reques return nil, connect.NewError(connect.CodeInternal, fmt.Errorf("UpdateRole: entity_id=%s: %w", updatedRole.ID, err)) } - audit.GetAuditor(ctx, schema.PlatformOrgID.String()).Log(audit.RoleUpdatedEvent, audit.Target{ + if err := audit.GetAuditor(ctx, schema.PlatformOrgID.String()).Log(audit.RoleUpdatedEvent, audit.Target{ ID: updatedRole.ID, Type: schema.RoleNamespace, Name: updatedRole.Name, - }) + }); err != nil { + errorLogger.LogServiceError(ctx, request, "UpdateRole.AuditLog", err, + "role_id", updatedRole.ID) + } return connect.NewResponse(&frontierv1beta1.UpdateRoleResponse{Role: &rolePB}), nil } @@ -257,11 +263,15 @@ func (h *ConnectHandler) CreateOrganizationRole(ctx context.Context, request *co return nil, connect.NewError(connect.CodeInternal, fmt.Errorf("CreateOrganizationRole: entity_id=%s: %w", newRole.ID, err)) } - audit.GetAuditor(ctx, request.Msg.GetOrgId()).Log(audit.RoleCreatedEvent, audit.Target{ + if err := audit.GetAuditor(ctx, request.Msg.GetOrgId()).Log(audit.RoleCreatedEvent, audit.Target{ ID: newRole.ID, Type: schema.RoleNamespace, Name: newRole.Name, - }) + }); err != nil { + errorLogger.LogServiceError(ctx, request, "CreateOrganizationRole.AuditLog", err, + "role_id", newRole.ID, + "org_id", request.Msg.GetOrgId()) + } return connect.NewResponse(&frontierv1beta1.CreateOrganizationRoleResponse{Role: &rolePB}), nil } @@ -349,11 +359,15 @@ func (h *ConnectHandler) UpdateOrganizationRole(ctx context.Context, request *co return nil, connect.NewError(connect.CodeInternal, fmt.Errorf("UpdateOrganizationRole: entity_id=%s: %w", updatedRole.ID, err)) } - audit.GetAuditor(ctx, request.Msg.GetOrgId()).Log(audit.RoleUpdatedEvent, audit.Target{ + if err := audit.GetAuditor(ctx, request.Msg.GetOrgId()).Log(audit.RoleUpdatedEvent, audit.Target{ ID: updatedRole.ID, Type: schema.RoleNamespace, Name: updatedRole.Name, - }) + }); err != nil { + errorLogger.LogServiceError(ctx, request, "UpdateOrganizationRole.AuditLog", err, + "role_id", updatedRole.ID, + "org_id", request.Msg.GetOrgId()) + } return connect.NewResponse(&frontierv1beta1.UpdateOrganizationRoleResponse{Role: &rolePB}), nil } @@ -383,10 +397,14 @@ func (h *ConnectHandler) DeleteOrganizationRole(ctx context.Context, request *co } } - audit.GetAuditor(ctx, orgID).Log(audit.RoleDeletedEvent, audit.Target{ + if err := audit.GetAuditor(ctx, orgID).Log(audit.RoleDeletedEvent, audit.Target{ ID: roleID, Type: schema.RoleNamespace, - }) + }); err != nil { + errorLogger.LogServiceError(ctx, request, "DeleteOrganizationRole.AuditLog", err, + "role_id", roleID, + "org_id", orgID) + } return connect.NewResponse(&frontierv1beta1.DeleteOrganizationRoleResponse{}), nil } diff --git a/internal/api/v1beta1connect/serviceuser.go b/internal/api/v1beta1connect/serviceuser.go index 5a1d64cbe..c2d1093b5 100644 --- a/internal/api/v1beta1connect/serviceuser.go +++ b/internal/api/v1beta1connect/serviceuser.go @@ -159,10 +159,14 @@ func (h *ConnectHandler) CreateServiceUser(ctx context.Context, request *connect return nil, connect.NewError(connect.CodeInternal, fmt.Errorf("CreateServiceUser: entity_id=%s: %w", svUser.ID, err)) } - audit.GetAuditor(ctx, request.Msg.GetOrgId()). + if err := audit.GetAuditor(ctx, request.Msg.GetOrgId()). LogWithAttrs(audit.ServiceUserCreatedEvent, audit.ServiceUserTarget(svUser.ID), map[string]string{ "title": svUser.Title, - }) + }); err != nil { + errorLogger.LogServiceError(ctx, request, "CreateServiceUser.AuditLog", err, + "service_user_id", svUser.ID, + "org_id", request.Msg.GetOrgId()) + } return connect.NewResponse(&frontierv1beta1.CreateServiceUserResponse{ Serviceuser: svUserPb, @@ -205,8 +209,12 @@ func (h *ConnectHandler) DeleteServiceUser(ctx context.Context, request *connect } } - audit.GetAuditor(ctx, orgID). - Log(audit.ServiceUserDeletedEvent, audit.ServiceUserTarget(serviceUserID)) + if err := audit.GetAuditor(ctx, orgID). + Log(audit.ServiceUserDeletedEvent, audit.ServiceUserTarget(serviceUserID)); err != nil { + errorLogger.LogServiceError(ctx, request, "DeleteServiceUser.AuditLog", err, + "service_user_id", serviceUserID, + "org_id", orgID) + } return connect.NewResponse(&frontierv1beta1.DeleteServiceUserResponse{}), nil } diff --git a/internal/api/v1beta1connect/user.go b/internal/api/v1beta1connect/user.go index 94d281d37..6d9886c21 100644 --- a/internal/api/v1beta1connect/user.go +++ b/internal/api/v1beta1connect/user.go @@ -148,13 +148,16 @@ func (h *ConnectHandler) CreateUser(ctx context.Context, request *connect.Reques if err != nil { return nil, connect.NewError(connect.CodeInternal, fmt.Errorf("CreateUser: entity_id=%s: %w", newUser.ID, err)) } - audit.GetAuditor(ctx, schema.PlatformOrgID.String()). + if err := audit.GetAuditor(ctx, schema.PlatformOrgID.String()). LogWithAttrs(audit.UserCreatedEvent, audit.UserTarget(newUser.ID), map[string]string{ "email": newUser.Email, "name": newUser.Name, "title": newUser.Title, "avatar": newUser.Avatar, - }) + }); err != nil { + errorLogger.LogServiceError(ctx, request, "CreateUser.AuditLog", err, + "user_id", newUser.ID) + } return connect.NewResponse(&frontierv1beta1.CreateUserResponse{User: transformedUser}), nil } @@ -286,12 +289,15 @@ func (h *ConnectHandler) UpdateUser(ctx context.Context, request *connect.Reques return nil, connect.NewError(connect.CodeInternal, fmt.Errorf("UpdateUser: entity_id=%s: %w", updatedUser.ID, err)) } - auditor.LogWithAttrs(audit.UserUpdatedEvent, audit.UserTarget(updatedUser.ID), map[string]string{ + if err := auditor.LogWithAttrs(audit.UserUpdatedEvent, audit.UserTarget(updatedUser.ID), map[string]string{ "email": updatedUser.Email, "name": updatedUser.Name, "title": updatedUser.Title, "avatar": updatedUser.Avatar, - }) + }); err != nil { + errorLogger.LogServiceError(ctx, request, "UpdateUser.AuditLog", err, + "user_id", updatedUser.ID) + } return connect.NewResponse(&frontierv1beta1.UpdateUserResponse{User: userPB}), nil } @@ -349,12 +355,15 @@ func (h *ConnectHandler) UpdateCurrentUser(ctx context.Context, request *connect return nil, connect.NewError(connect.CodeInternal, fmt.Errorf("UpdateCurrentUser: entity_id=%s: %w", updatedUser.ID, err)) } - auditor.LogWithAttrs(audit.UserUpdatedEvent, audit.UserTarget(updatedUser.ID), map[string]string{ + if err := auditor.LogWithAttrs(audit.UserUpdatedEvent, audit.UserTarget(updatedUser.ID), map[string]string{ "email": updatedUser.Email, "name": updatedUser.Name, "title": updatedUser.Title, "avatar": updatedUser.Avatar, - }) + }); err != nil { + errorLogger.LogServiceError(ctx, request, "UpdateCurrentUser.AuditLog", err, + "user_id", updatedUser.ID) + } return connect.NewResponse(&frontierv1beta1.UpdateCurrentUserResponse{User: userPB}), nil } @@ -416,7 +425,10 @@ func (h *ConnectHandler) DeleteUser(ctx context.Context, request *connect.Reques } } - audit.GetAuditor(ctx, schema.PlatformOrgID.String()).Log(audit.UserDeletedEvent, audit.UserTarget(request.Msg.GetId())) + if err := audit.GetAuditor(ctx, schema.PlatformOrgID.String()).Log(audit.UserDeletedEvent, audit.UserTarget(request.Msg.GetId())); err != nil { + errorLogger.LogServiceError(ctx, request, "DeleteUser.AuditLog", err, + "user_id", userID) + } return connect.NewResponse(&frontierv1beta1.DeleteUserResponse{}), nil } @@ -525,6 +537,7 @@ func (h *ConnectHandler) ListCurrentUserGroups(ctx context.Context, request *con } func (h *ConnectHandler) ListUsers(ctx context.Context, request *connect.Request[frontierv1beta1.ListUsersRequest]) (*connect.Response[frontierv1beta1.ListUsersResponse], error) { + errorLogger := NewErrorLogger() auditor := audit.GetAuditor(ctx, request.Msg.GetOrgId()) var users []*frontierv1beta1.User @@ -555,7 +568,10 @@ func (h *ConnectHandler) ListUsers(ctx context.Context, request *connect.Request users = append(users, userPB) } - auditor.Log(audit.UserListedEvent, audit.OrgTarget(request.Msg.GetOrgId())) + if err := auditor.Log(audit.UserListedEvent, audit.OrgTarget(request.Msg.GetOrgId())); err != nil { + errorLogger.LogServiceError(ctx, request, "ListUsers.AuditLog", err, + "org_id", request.Msg.GetOrgId()) + } return connect.NewResponse(&frontierv1beta1.ListUsersResponse{ Count: int32(len(users)), Users: users, From 65c65f11ffc3fa771b4cd36fa616d3e1c07a68ae Mon Sep 17 00:00:00 2001 From: aman Date: Fri, 17 Jul 2026 14:47:54 +0530 Subject: [PATCH 2/3] fix(core): log audit log failures instead of dropping them Legacy audit log writes in the membership, deleter, and organization services discarded their error. Log a warning when the write fails. The calling operation still succeeds. Co-Authored-By: Claude Fable 5 --- core/deleter/service.go | 5 ++++- core/membership/service.go | 36 ++++++++++++++++++++++++------------ core/organization/service.go | 5 ++++- 3 files changed, 32 insertions(+), 14 deletions(-) diff --git a/core/deleter/service.go b/core/deleter/service.go index 7b7dc58b5..efd7e0e38 100644 --- a/core/deleter/service.go +++ b/core/deleter/service.go @@ -4,6 +4,7 @@ import ( "context" "errors" "fmt" + "log/slog" "github.com/raystack/frontier/core/audit" @@ -276,7 +277,9 @@ func (d Service) DeleteOrganization(ctx context.Context, id string) error { return err } - audit.NewLogger(ctx, id).Log(audit.OrgDeletedEvent, audit.OrgTarget(id)) + if err := audit.NewLogger(ctx, id).Log(audit.OrgDeletedEvent, audit.OrgTarget(id)); err != nil { + slog.WarnContext(ctx, "failed to write audit log", "error", err, "event", audit.OrgDeletedEvent) + } return nil } diff --git a/core/membership/service.go b/core/membership/service.go index 4f0de1c05..6e871b531 100644 --- a/core/membership/service.go +++ b/core/membership/service.go @@ -442,10 +442,12 @@ func (s *Service) removeOrganizationMember(ctx context.Context, orgID, principal } s.auditOrgMemberRemoved(ctx, org, principalID, targetAuditType) - audit.GetAuditor(ctx, org.ID).Log(audit.OrgMemberDeletedEvent, audit.Target{ + if err := audit.GetAuditor(ctx, org.ID).Log(audit.OrgMemberDeletedEvent, audit.Target{ ID: principalID, Type: principalType, - }) + }); err != nil { + s.log.WarnContext(ctx, "failed to write audit log", "error", err, "event", audit.OrgMemberDeletedEvent) + } return nil } @@ -825,12 +827,14 @@ func (s *Service) auditOrgMemberRoleChanged(ctx context.Context, org organizatio OccurredAt: time.Now(), }) - audit.GetAuditor(ctx, org.ID).LogWithAttrs(audit.OrgMemberRoleChangedEvent, audit.Target{ + if err := audit.GetAuditor(ctx, org.ID).LogWithAttrs(audit.OrgMemberRoleChangedEvent, audit.Target{ ID: p.ID, Type: p.Type, }, map[string]string{ "role_id": roleID, - }) + }); err != nil { + s.log.WarnContext(ctx, "failed to write audit log", "error", err, "event", audit.OrgMemberRoleChangedEvent) + } } func (s *Service) auditOrgMemberAdded(ctx context.Context, org organization.Organization, p principalInfo, roleID string) { @@ -857,12 +861,14 @@ func (s *Service) auditOrgMemberAdded(ctx context.Context, org organization.Orga OccurredAt: time.Now(), }) - audit.GetAuditor(ctx, org.ID).LogWithAttrs(audit.OrgMemberCreatedEvent, audit.Target{ + if err := audit.GetAuditor(ctx, org.ID).LogWithAttrs(audit.OrgMemberCreatedEvent, audit.Target{ ID: p.ID, Type: p.Type, }, map[string]string{ "role_id": roleID, - }) + }); err != nil { + s.log.WarnContext(ctx, "failed to write audit log", "error", err, "event", audit.OrgMemberCreatedEvent) + } } func (s *Service) auditOrgMemberRemoved(ctx context.Context, org organization.Organization, targetID string, targetType pkgAuditRecord.EntityType) { @@ -1725,13 +1731,15 @@ func (s *Service) auditGroupMemberAdded(ctx context.Context, grp group.Group, p OccurredAt: time.Now(), }) - audit.GetAuditor(ctx, grp.OrganizationID).LogWithAttrs(audit.GroupMemberCreatedEvent, audit.Target{ + if err := audit.GetAuditor(ctx, grp.OrganizationID).LogWithAttrs(audit.GroupMemberCreatedEvent, audit.Target{ ID: p.ID, Type: p.Type, }, map[string]string{ "role_id": roleID, "group_id": grp.ID, - }) + }); err != nil { + s.log.WarnContext(ctx, "failed to write audit log", "error", err, "event", audit.GroupMemberCreatedEvent) + } } func (s *Service) auditGroupMemberRoleChanged(ctx context.Context, grp group.Group, p principalInfo, roleID string) { @@ -1758,13 +1766,15 @@ func (s *Service) auditGroupMemberRoleChanged(ctx context.Context, grp group.Gro OccurredAt: time.Now(), }) - audit.GetAuditor(ctx, grp.OrganizationID).LogWithAttrs(audit.GroupMemberRoleChangedEvent, audit.Target{ + if err := audit.GetAuditor(ctx, grp.OrganizationID).LogWithAttrs(audit.GroupMemberRoleChangedEvent, audit.Target{ ID: p.ID, Type: p.Type, }, map[string]string{ "role_id": roleID, "group_id": grp.ID, - }) + }); err != nil { + s.log.WarnContext(ctx, "failed to write audit log", "error", err, "event", audit.GroupMemberRoleChangedEvent) + } } func (s *Service) auditGroupMemberRemoved(ctx context.Context, grp group.Group, p principalInfo) { @@ -1791,12 +1801,14 @@ func (s *Service) auditGroupMemberRemoved(ctx context.Context, grp group.Group, OccurredAt: time.Now(), }) - audit.GetAuditor(ctx, grp.OrganizationID).LogWithAttrs(audit.GroupMemberRemovedEvent, audit.Target{ + if err := audit.GetAuditor(ctx, grp.OrganizationID).LogWithAttrs(audit.GroupMemberRemovedEvent, audit.Target{ ID: p.ID, Type: p.Type, }, map[string]string{ "group_id": grp.ID, - }) + }); err != nil { + s.log.WarnContext(ctx, "failed to write audit log", "error", err, "event", audit.GroupMemberRemovedEvent) + } } // ResourceFilter narrows the results of ListResourcesByPrincipal. diff --git a/core/organization/service.go b/core/organization/service.go index 45d80ab1e..ac55369af 100644 --- a/core/organization/service.go +++ b/core/organization/service.go @@ -4,6 +4,7 @@ import ( "context" "errors" "fmt" + "log/slog" "github.com/raystack/frontier/core/audit" @@ -259,7 +260,9 @@ func (s Service) Enable(ctx context.Context, id string) error { func (s Service) Disable(ctx context.Context, id string) error { err := s.repository.SetState(ctx, id, Disabled) if err == nil { - audit.GetAuditor(ctx, id).Log(audit.OrgDisabledEvent, audit.OrgTarget(id)) + if auditErr := audit.GetAuditor(ctx, id).Log(audit.OrgDisabledEvent, audit.OrgTarget(id)); auditErr != nil { + slog.WarnContext(ctx, "failed to write audit log", "error", auditErr, "event", audit.OrgDisabledEvent) + } } return err } From 0a4c8435c283bf7224e6e033672cf14a5e71eb9d Mon Sep 17 00:00:00 2001 From: aman Date: Fri, 17 Jul 2026 15:02:29 +0530 Subject: [PATCH 3/3] test(connect): assert audit log failure does not fail the request Inject a failing audit repository into the handler context and verify SetOrganizationKyc still succeeds. Co-Authored-By: Claude Fable 5 --- internal/api/v1beta1connect/kyc_test.go | 54 +++++++++++++++++++++---- 1 file changed, 46 insertions(+), 8 deletions(-) diff --git a/internal/api/v1beta1connect/kyc_test.go b/internal/api/v1beta1connect/kyc_test.go index 725211370..a2023edbc 100644 --- a/internal/api/v1beta1connect/kyc_test.go +++ b/internal/api/v1beta1connect/kyc_test.go @@ -15,15 +15,32 @@ import ( "github.com/stretchr/testify/mock" ) +// failingAuditRepository makes every audit log write fail. Handlers must not +// fail the request because of it. +type failingAuditRepository struct{} + +func (failingAuditRepository) Create(context.Context, *audit.Log) error { + return errors.New("audit write failed") +} + +func (failingAuditRepository) List(context.Context, audit.Filter) ([]audit.Log, error) { + return nil, nil +} + +func (failingAuditRepository) GetByID(context.Context, string) (audit.Log, error) { + return audit.Log{}, nil +} + func TestSetOrganizationKyc(t *testing.T) { tests := []struct { - mockService *mocks.KycService - name string - request *connect.Request[frontierv1beta1.SetOrganizationKycRequest] - mockResponse kyc.KYC - mockError error - expectError bool - expectedError error + mockService *mocks.KycService + name string + request *connect.Request[frontierv1beta1.SetOrganizationKycRequest] + mockResponse kyc.KYC + mockError error + auditRepository audit.Repository + expectError bool + expectedError error }{ { mockService: mocks.NewKycService(t), @@ -89,6 +106,23 @@ func TestSetOrganizationKyc(t *testing.T) { expectError: true, expectedError: connect.NewError(connect.CodeInternal, fmt.Errorf("SetOrganizationKyc.SetKyc: org_id=%s status=%v: %w", "valid-org-id", true, errors.New("internal error"))), }, + { + mockService: mocks.NewKycService(t), + name: "audit log failure does not fail the request", + request: connect.NewRequest(&frontierv1beta1.SetOrganizationKycRequest{ + OrgId: "valid-org-id", + Status: true, + Link: "http://kyc-link.com", + }), + mockResponse: kyc.KYC{ + OrgID: "valid-org-id", + Status: true, + Link: "http://kyc-link.com", + }, + auditRepository: failingAuditRepository{}, + mockError: nil, + expectError: false, + }, } for _, tt := range tests { @@ -106,8 +140,12 @@ func TestSetOrganizationKyc(t *testing.T) { } // Create context with audit service + auditRepository := tt.auditRepository + if auditRepository == nil { + auditRepository = audit.NewNoopRepository() + } ctx := context.Background() - ctx = audit.SetContextWithService(ctx, audit.NewService("test", audit.NewNoopRepository(), audit.NewNoopWebhookService())) + ctx = audit.SetContextWithService(ctx, audit.NewService("test", auditRepository, audit.NewNoopWebhookService())) // Call the handler method response, err := handler.SetOrganizationKyc(ctx, tt.request)