Skip to content

fix(permissions): reload the users list on an unknown id or a denied access [PRD-1404] - #398

Merged
matthv merged 2 commits into
mainfrom
fix/prd-1404-reload-users-on-unknown-id
Oct 1, 2026
Merged

matthv merged 2 commits into
mainfrom
fix/prd-1404-reload-users-on-unknown-id

Conversation

@matthv

@matthv matthv commented Sep 30, 2026 •

Copy link
Copy Markdown
Member

fixes PRD-1404

Why

A user or service account created after the agent started stayed unknown to it until the users cache expired (permission_expiration, 900 s by default): get_user_data never reloaded the list on an unknown id, so can? raised ForbiddenError.

Only the SSE refresh-users event could clear the cache sooner, and instant_cache_refresh defaults to Rails.env.production?. Outside production, or when a proxy buffers the event stream, a service account created to be used right away got 403 for up to 15 min.

A role change had the same delay: a denial only refetched the collections permissions, never the users.

What changes

ForestAdminAgent::Services::Permissions:

  • get_user_data reloads the users list once when the id is absent. can_smart_action?, get_scope and the read permissions go through it, so they benefit too.
  • can? reloads the users on a denial, next to the existing collections refetch, via get_user_data(caller.id, reload: true).
  • fetch_read_permissions does the same when a read denial refetches the collections, so a related collection a new role reads is no longer redacted until the cache expires.
  • Both reloads share a throttle: at most one per USERS_RELOAD_INTERVAL_IN_SECONDS (60 s) per process, so a bogus or removed id cannot trigger a burst of calls to the server. A TTL expiry or an SSE invalidation is not throttled.
  • invalidate_cache with no key also resets the throttle.

The throttle is per process while the FileCache is shared by the workers of a machine: N workers can reload up to N times per minute.

Tests

permissions_spec.rb, /liana/v4/permissions/users stubbed with sequential responses:

  • an unknown id reloads once and returns the user from the second response;
  • a known id does not reload;
  • an id that stays unknown reloads once per interval, then again after it;
  • a denial reloads the users and allows a role granted since the last load;
  • a denial within the interval stays denied without reloading.

related_read_permissions_spec.rb: a read denial reloads the caller, and a role changed since the last load reads the related collection at once.

Five of the six fail without the fix; the known-id one guards against a regression. Package suite: 1264 examples, 0 failures. RuboCop clean.

🤖 Generated with Claude Code

Note

fixes PRD-1404

Why

A user or service account created after the agent started stayed unknown to it until the users cache expired (permission_expiration, 900 s by default): get_user_data never reloaded the list on an unknown id, so can? raised ForbiddenError.

Only the SSE refresh-users event could clear the cache sooner, and instant_cache_refresh defaults to Rails.env.production?. Outside production, or when a proxy buffers the event stream, a service account created to be used right away got 403 for up to 15 min.

A role change had the same delay: a denial only refetched the collections permissions, never the users.

What changes

ForestAdminAgent::Services::Permissions:

  • get_user_data reloads the users list once when the id is absent. can_smart_action?, get_scope and the read permissions go through it, so they benefit too.
  • can? reloads the users on a denial, next to the existing collections refetch, via get_user_data(caller.id, reload: true).
  • fetch_read_permissions does the same when a read denial refetches the collections, so a related collection a new role reads is no longer redacted until the cache expires.
  • Both reloads share a throttle: at most one per USERS_RELOAD_INTERVAL_IN_SECONDS (60 s) per process, so a bogus or removed id cannot trigger a burst of calls to the server. A TTL expiry or an SSE invalidation is not throttled.
  • invalidate_cache with no key also resets the throttle.

The throttle is per process while the FileCache is shared by the workers of a machine: N workers can reload up to N times per minute.

Tests

permissions_spec.rb, /liana/v4/permissions/users stubbed with sequential responses:

  • an unknown id reloads once and returns the user from the second response;
  • a known id does not reload;
  • an id that stays unknown reloads once per interval, then again after it;
  • a denial reloads the users and allows a role granted since the last load;
  • a denial within the interval stays denied without reloading.

related_read_permissions_spec.rb: a read denial reloads the caller, and a role changed since the last load reads the related collection at once.

Five of the six fail without the fix; the known-id one guards against a regression. Package suite: 1264 examples, 0 failures. RuboCop clean.

🤖 Generated with Claude Code

Changes since #398 opened

  • Modified ForestAdminAgent::Services::Permissions.fetch_read_permissions to reload caller user data when a read permission denial triggers a forced collections permissions refetch [ef28c4d]
  • Added RSpec example validating that ForestAdminAgent::Services::Permissions reloads caller data on denial-triggered refetch [ef28c4d]

…access [PRD-1404]

A user or service account created after the agent started was unknown to it
until the users cache expired (15 min by default), since nothing reloaded the
list on an unknown id. Without SSE (the default outside production), or when
the refresh-users event is lost, every call it made got a 403 meanwhile.

- get_user_data reloads the users list once when the id is absent.
- can? reloads the users on a denial, not only the collections permissions,
  so a role change applies right away.
- Both reloads share a throttle: at most one per minute per process.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@linear-code

linear-code Bot commented Sep 30, 2026

Copy link
Copy Markdown

PRD-1404

@qltysh

qltysh Bot commented Sep 30, 2026 •

Copy link
Copy Markdown

Qlty


Coverage Impact

This PR will not change total coverage.

Modified Files with Diff Coverage (1)

RatingFile% DiffUncovered Line #s
Coverage rating: A Coverage rating: A
...est_admin_agent/lib/forest_admin_agent/services/permissions.rb100.0%
Total100.0%
🚦 See full report on Qlty Cloud »

🛟 Help
  • Diff Coverage: Coverage for added or modified lines of code (excludes deleted files). Learn more.

  • Total Coverage: Coverage for the whole repository, calculated as the sum of all File Coverage. Learn more.

  • File Coverage: Covered Lines divided by Covered Lines plus Missed Lines. (Excludes non-executable lines including blank lines and comments.)

    • Indirect Changes: Changes to File Coverage for files that were not modified in this PR. Learn more.

@Scra3 Scra3 left a comment

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.

Spec (PRD-1404): conforms. An unknown id and a can? denial both reload the users under one 60 s throttle per process, and the four required test cases are present.

…collections [PRD-1404]

A user moved to a role that reads a related collection kept its fields
redacted until the cache expired: the refetch reused the user loaded
before it.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

@Scra3 Scra3 left a comment

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.

Approved after /verify-fixes run 398-20260930120805-Scra3-verify: 1 findings of 398-20260930090500-Scra3 closed or accepted, none reopened.

@matthv
matthv merged commit d375cb6 into main Oct 1, 2026
33 checks passed
@matthv
matthv deleted the fix/prd-1404-reload-users-on-unknown-id branch October 1, 2026 15:58
forest-bot added a commit that referenced this pull request Oct 1, 2026
## [1.44.4](v1.44.3...v1.44.4) (2026-10-01)

### Bug Fixes

* **permissions:** reload the users list on an unknown id or a denied access [PRD-1404] ([#398](#398)) ([d375cb6](d375cb6))
@forest-bot

Copy link
Copy Markdown
Member

🎉 This PR is included in version 1.44.4 🎉

The release is available on GitHub release

Your semantic-release bot 📦🚀

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants