enhancement
major
minor
defect
critical
major
minor
minor
#30269
ModelAccessRights.isAllowed(Person, ...): Checks whether another person is granted bypass or admin status in the current context
Background
An application creates notifications for other users during a commit (ModificationListener). Each notification should only be generated if the recipient is authorized to read the affected object. To achieve this, the engine provides ModelAccessRights.isReadAllowed(Person person, TLObject instance) and isAllowed(Person, TLObject, BoundCommandGroup): “Checks whether the given person can perform the given command group on the given instance.”
Problem
SecurityConfigurationService.isAllowed(Person, TLObject, BoundCommandGroup) calls BoundChecker.isAllowedBypass(person, commandGroup) before performing the role check. This returns true if
- the current context bypasses the security check (ThreadContext.isSecurityBypassed(), e.g., system context, background task), or
- the user of the current context is an administrator (ThreadContext.isAdmin()).
Both of these are properties of the current context, not of the person being checked. Thus, if an administrator or a system process modifies an object, any person is considered to have read access. The Javadoc for ThreadContext.isAdmin() itself states: “Whether some given user is an administrator is a property of that user, not of the current context.”
Version: 8.0.0-alpha12 (com.top_logic.model.security.SecurityConfigurationService, com.top_logic.tool.boundsec.BoundChecker).
Reproduction
- User p without a role on object o.
- Evaluate in the system context (or logged in as root): ModelAccessRights.getInstance().isReadAllowed(p, o).
- Result: true. Expected: false.
Expected Behavior
The variant with the person parameter makes a decision based on the person passed in: Bypass and admin status of the current context apply only to the variants without a person (current user). If the person passed in is an administrator, they are, of course, allowed to do everything.
Bypass
Prime (com.top_logic.prime.notify.Responsibles#canRead) sets the person being checked as the context’s user (TLContext.captureIdentity()/setPerson) for the check and enables security checks (ThreadContext.withSecurityChecks(true, ...)); after which the identity is restored.
Solution
- New check TLContext.hasAdministrativeRights(Person): A person has administrative rights if they are an administrator (Person.isAdmin(Person)) or if they are the person of the current context and the security check is bypassed (ThreadContext.isSecurityBypassed()).
- BoundChecker.isAllowedBypass(Person, BoundCommandGroup) and SimpleBoundCommandGroup.isAllowedCommandGroup(Person, BoundCommandGroup) determine the outcome of this check based on the state of the current context. This correction applies to all checks performed by the SecurityConfigurationService (ModelAccessRights), to BoundChecker.allow(...), and to the access checks in React views (ModelAccessPolicy).
- The variants without a person (current user, including in the system context) remain unchanged. A check for another person is determined based on that person’s roles, administrator flags, and restrictions (restricted user).
- The access declaration (EffectiveAccess) no longer requires switching to the context of the declared person; this step is omitted.
- Workarounds in applications (changing identity before the check) are no longer necessary.