Skip to content

fix: log audit log failures instead of dropping them #1769

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
AmanGIT07 wants to merge 3 commits into
base: main
Choose a base branch
from
Open

fix: log audit log failures instead of dropping them #1769

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
cc91053
fix(connect): log audit failures instead of dropping them
AmanGIT07 Jul 17, 2026
65c65f1
fix(core): log audit log failures instead of dropping them
AmanGIT07 Jul 17, 2026
0a4c843
test(connect): assert audit log failure does not fail the request
AmanGIT07 Jul 17, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 4 additions & 1 deletion core/deleter/service.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ import (
"context"
"errors"
"fmt"
"log/slog"

"github.com/raystack/frontier/core/audit"

Expand Down Expand Up @@ -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
}

Expand Down
36 changes: 24 additions & 12 deletions core/membership/service.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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
}
Expand Down Expand Up @@ -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) {
Expand All @@ -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) {
Expand Down Expand Up @@ -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) {
Expand All @@ -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) {
Expand All @@ -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.
Expand Down
5 changes: 4 additions & 1 deletion core/organization/service.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ import (
"context"
"errors"
"fmt"
"log/slog"

"github.com/raystack/frontier/core/audit"

Expand Down Expand Up @@ -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
}
Expand Down
8 changes: 6 additions & 2 deletions internal/api/v1beta1connect/authorize.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down
7 changes: 5 additions & 2 deletions internal/api/v1beta1connect/billing_customer.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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())
Expand Down
14 changes: 12 additions & 2 deletions internal/api/v1beta1connect/group.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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)
}
Expand Down Expand Up @@ -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
}

Expand Down Expand Up @@ -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)
}
Expand Down Expand Up @@ -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
}

Expand Down
8 changes: 6 additions & 2 deletions internal/api/v1beta1connect/kyc.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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(),
Expand All @@ -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
}
Expand Down
54 changes: 46 additions & 8 deletions internal/api/v1beta1connect/kyc_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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),
Expand Down Expand Up @@ -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 {
Expand All @@ -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)
Expand Down
14 changes: 11 additions & 3 deletions internal/api/v1beta1connect/organization.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down Expand Up @@ -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)
}
Expand Down Expand Up @@ -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
}

Expand Down
10 changes: 8 additions & 2 deletions internal/api/v1beta1connect/permission_check.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ package v1beta1connect
import (
"context"
"fmt"
"log/slog"

"connectrpc.com/connect"
"github.com/raystack/frontier/core/audit"
Expand All @@ -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) {
Expand Down
Loading
Loading