Skip to content

fix(iam): audit super-admin and grant-escalation denials - #47

Merged
zaxovaiko merged 6 commits into
devfrom
fix/audit-unauthorized-access-attempts
Aug 5, 2026
Merged

fix(iam): audit super-admin and grant-escalation denials#47
zaxovaiko merged 6 commits into
devfrom
fix/audit-unauthorized-access-attempts

Conversation

@klaudia-blazyczek-blurify

@klaudia-blazyczek-blurify klaudia-blazyczek-blurify commented Jul 31, 2026

Copy link
Copy Markdown
Collaborator

Summary

IamService.assertSuperAdmin and the no-escalation check in setRolePermissions now emit identity.user.unauthorized_access before throwing, so these service-level denials are audited the same way AdminGuard denials always have been. BF-412.

Why

Two service-level FORBIDDEN paths (NotSuperAdminError, GrantEscalationError) throw before ever reaching AdminGuard.assert(), so they bypassed the guard's audit emission and produced no identity.user.unauthorized_access row - even though the request genuinely reached the backend and was genuinely denied.

Alternatives considered

An earlier version of this PR also added AdminGuard.recordDeniedAccess / an iam reportAccessDenied route so a frontend page guard could self-report a denial after blocking navigation client-side. Dropped per review: a client-side redirect never sends the underlying request, so it isn't a reliable audit signal - only a real backend request that hits AdminGuard.assert() (or one of the two service-level checks above) is. The frontend continues to redirect + show a toast for inaccessible pages, without a separate audit entry.

Risks

None. Changeset downgraded to patch - no new public API, pure bug fix.

agniev-a-hub
agniev-a-hub previously approved these changes Jul 31, 2026

@agniev-a-hub agniev-a-hub left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

As per slack

Comment on lines +173 to +180
this.emitUnauthorized(
caller.userId,
caller.role,
resource,
missingAction,
caller.ip,
caller.userAgent,
);

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I strongly suggest to use named parameters

if (this.rateLimiter) {
const key = makeRateLimitKey(
RATE_LIMIT_KEYS.ACCESS_DENIED_REPORT,
`${caller.userId}:${resource}:${level}`,

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Let's extract the key to some function (as we did before)

Comment thread packages/core/src/iam/contract/index.ts Outdated
.route({ method: 'GET', path: '/iam/my-permissions' })
.output(EffectivePermissionsSchema),

reportAccessDenied: oc

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What is the purpose of this endpoint?

zaxovaiko
zaxovaiko previously approved these changes Jul 31, 2026
@zaxovaiko
zaxovaiko dismissed stale reviews from agniev-a-hub, marek-chmielowski-blurify, and themself via a217e18 July 31, 2026 12:40
@zaxovaiko
zaxovaiko marked this pull request as draft July 31, 2026 22:38
@zaxovaiko zaxovaiko self-assigned this Jul 31, 2026
…-access-attempts

# Conflicts:
#	.rulesync/rules/conventions.md
@zaxovaiko zaxovaiko changed the title feat(iam): audit authorisation denials reported by client-side guards fix(iam): audit super-admin and grant-escalation denials Aug 5, 2026
@zaxovaiko
zaxovaiko marked this pull request as ready for review August 5, 2026 17:50
@zaxovaiko
zaxovaiko merged commit 6ca7135 into dev Aug 5, 2026
2 checks passed
@zaxovaiko
zaxovaiko deleted the fix/audit-unauthorized-access-attempts branch August 5, 2026 17:50
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants