Block unauthorized Sales Order Agent setup access - #10232
Conversation
| if not Evaluate(UserSecurityID, UserSecurityIDFilter) then | ||
| Clear(UserSecurityID); | ||
|
|
||
| if not AgentSystemPermissions.CurrentUserCanManageAgent(UserSecurityID) then |
There was a problem hiding this comment.
Adding Error(NotAuthorizedToConfigureAgentErr) in OnOpenPage means users who previously could open the Sales Order Agent setup page can no longer do so unless they hold agent-management permission (Configure All Agents, or Can Curr. User Configure Agent on the resolved Agent record). For a page reached only via first-time setup (gated separately by AllowCreateNewSOAgent requiring Configure All Agents) this is consistent, but for the edit path any previously-permitted viewer is now blocked at open time rather than at a modify/configure action, which is a behavior change worth calling out explicitly in the PR description.
👍 useful · ❤️ especially valuable · 👎 wrong - reply with why · AL review agent v1.32.4
| if not Evaluate(UserSecurityID, UserSecurityIDFilter) then | ||
| Clear(UserSecurityID); | ||
|
|
||
| if not AgentSystemPermissions.CurrentUserCanManageAgent(UserSecurityID) then |
There was a problem hiding this comment.
The new permission-denied branch throws Error(NotAuthorizedToConfigureAgentErr) before the existing FeatureTelemetry.LogUptake(...Discovered) call executes, so a user who opens the Sales Order Agent setup page but lacks permission to manage the agent no longer records the feature-discovery telemetry event. Per the feature-uptake lifecycle guidance, Discovered should be logged when the user encounters the feature even if the flow cannot continue past that point. Move the LogUptake(Discovered) call (or an equivalent call) ahead of the new permission check, or emit it inside the error branch.
Suggested fix (apply manually — could not be anchored as a one-click suggestion):
if not AgentSystemPermissions.CurrentUserCanManageAgent(UserSecurityID) then begin
FeatureTelemetry.LogUptake('0000QIK', SOASetupCU.GetFeatureName(), Enum::"Feature Uptake Status"::Discovered);
Error(NotAuthorizedToConfigureAgentErr);
end;Knowledge:
👍 useful · ❤️ especially valuable · 👎 wrong - reply with why · AL review agent v1.32.4
Agentic PR Review - Round 1Recommendation: Accept with SuggestionsWhat this PR doesThis PR adds a permission check when page 4400 SOA Setup opens. The check uses the resolved User Security ID, so an unfiltered direct URL requires Configure All Agents, while a filtered existing agent can still be opened by a user who can configure that agent. The fix addresses the root cause at the page entry boundary, before setup data is loaded or changed. It also matches the existing Agent Card pattern, which uses Agent System Permissions.CurrentUserCanManageAgent for the same agent-management decision. SuggestionsS1 - Add a permission regression test Risk assessment and necessityRisk: This is an authorization boundary for Sales Order Agent setup. A wrong check could either reopen the direct-access bypass or block valid setup users, but the code uses the existing agent permission API and leaves the update flow unchanged. Necessity: The work item and PR description describe a concrete unauthorized direct-navigation bug. The scope is targeted: it blocks access immediately after resolving the target agent user security ID and before setup state can be changed.
|
|
Problem
A user without Sales Order Agent configuration rights can navigate directly to page 4400 and activate or reconfigure the agent, bypassing the authorization applied by the normal Agents pages.
Root cause
The SOA Setup page trusts its caller. Its
OnOpenPagetrigger resolves the target agent but does not verify that the current user can manage that agent.Fix
Validate the resolved agent user security ID with
Agent System Permissions.CurrentUserCanManageAgent. This preserves per-agent Can Configure access and requires Configure All Agents for first-time setup or unfiltered direct navigation.Validation
git diff --check: cleanCurrentUserCanManageAgentguard and authorization errorContact List.OnBeforeFindRecordevent, so GitHub validation remains authoritative for the complete current-main app buildRisk
Low. The change is limited to the setup page entry boundary and uses the existing agent authorization API.
Fixes AB#640825