Skip to content

Fix role permission authorization checks#1749

Merged
kevinherron merged 2 commits into
eclipse-milo:mainfrom
kevinherron:codex/fix-role-permissions-authorization-issue
May 17, 2026
Merged

Fix role permission authorization checks#1749
kevinherron merged 2 commits into
eclipse-milo:mainfrom
kevinherron:codex/fix-role-permissions-authorization-issue

Conversation

@kevinherron
Copy link
Copy Markdown
Contributor

Motivation

  • The previous authorization logic treated any UserRolePermissions entry with a permission bit as sufficient, ignoring whether the entry’s roleId was assigned to the current session, which allowed role-based authorization bypasses.

Description

  • Require that a UserRolePermissions entry both has the requested PermissionType bit and that its roleId is present in the session’s mapped role IDs by adding a shared hasPermission(...) helper and using it in all affected checks.
  • Replace unchecked Stream.anyMatch(rp -> rp.getPermissions().getX()) checks with hasPermission(roleIds, ..., PermissionType::getX) across read/write RolePermissions, Historizing, browse, call, add reference, delete node, and delete reference paths in DefaultAccessController.
  • Add PermissionType import and a Predicate<PermissionType>-based helper to encapsulate the role-id match plus permission bit test in DefaultAccessController.
  • Add a regression unit test checkRolePermissionAccess_DeniesPermissionsForUnassignedRoles to cover the mismatched-role scenarios across read, write, browse, call, add reference, delete node, and delete reference in DefaultAccessControllerTest.

Codex Task

@kevinherron kevinherron added this to the 1.1.4 milestone May 17, 2026
@kevinherron kevinherron changed the title Codex/fix role permissions authorization issue Fix role permission authorization checks May 17, 2026
@kevinherron kevinherron merged commit 84d3793 into eclipse-milo:main May 17, 2026
3 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants