Repository navigation
Conversation
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Good Sense Reviewer - Round 1Recommendation: AcceptWhat this PR doesThis change lets delegated Expense Agent administrators activate and deactivate the fixed Entra application without unrelated SECURITY or Expense Agent runtime permissions. The remaining checks require effective Configure All Agents permission and Expense Mgmt. Admin. The internal privileged codeunit is limited to the fixed Expense Agent application, its runtime permission set, and the current company, so the change fixes the reported authorization failure without granting general application-management access. Problem-solution fitFit: Strong The removed checks are the direct cause of the reported failure. The remaining authorization checks match the administrative capability required by the scenario and also support equivalent custom permission sets. SuggestionsNone. Risk assessment and necessityRisk: This changes authorization on a privileged application lifecycle path. The risk is contained because callers still need two relevant administrative permissions, the codeunit is internal, and its inherent permissions operate only on the fixed Expense Agent application and current-company runtime grant. The standard Agent - Admin permission set and the existing setup flow use the same effective Configure All Agents permission model. No event publisher or external BaseApp dependency is involved. Necessity: The change is needed because the removed SECURITY and runtime-role requirements grant unrelated capabilities and prevent the intended delegated administrator from completing setup. The scope is narrow and directly addresses that failure.
|
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Good Sense Reviewer - Round 2Recommendation: Request ChangesWhat this PR doesThe latest change restores the This keeps authorization behavior effectively unchanged from the base branch. A user with only Status of previous suggestionsNo previous suggestions. New observations (commits since round 1)S1 (🔴 High): Implement the delegated administrator path Risk assessment and necessityRisk: This code controls a privileged Entra application and its Necessity: The permission-model fix is needed because the intended administrator cannot complete setup without unrelated broad rights. The implementation should stay limited to the fixed client ID, app ID, role ID, and current company.
|
|
|
|
Summary
SECURITY, theExpense Agentruntime permission, andAgent - Adminin addition toExpense Mgmt. Admin.Authorization model
The effective rule remains:
SUPERor (Expense Mgmt. AdminandSECURITYandExpense AgentandAgent - AdminandConfigure All Agents)This branch does not implement the reduced permission model requested by AB#649970.
Validation
git diff --checkpasses.Related to AB#649970