Skip to content

[Quality Management] Enforce rules and restrictions with implicit permissions#8556

Draft
JakovljevicDusan wants to merge 1 commit into
mainfrom
bugs/QM-EnforcingQMRulesAndRestrictionWithImplicitPermissions
Draft

[Quality Management] Enforce rules and restrictions with implicit permissions#8556
JakovljevicDusan wants to merge 1 commit into
mainfrom
bugs/QM-EnforcingQMRulesAndRestrictionWithImplicitPermissions

Conversation

@JakovljevicDusan

@JakovljevicDusan JakovljevicDusan commented Jun 9, 2026

Copy link
Copy Markdown
Contributor

What & why

Implement InherentPermissions so users without QM permissions would not silently skip mandatory QI creation or transaction restrictions.

Linked work

Fixes AB#626330

@JakovljevicDusan JakovljevicDusan requested a review from a team as a code owner June 9, 2026 21:10
@github-actions github-actions Bot added the AL: Apps (W1) Add-on apps for W1 label Jun 9, 2026
@github-actions github-actions Bot added this to the Version 29.0 milestone Jun 9, 2026
@github-actions

github-actions Bot commented Jun 9, 2026

Copy link
Copy Markdown
Contributor

$\textbf{🟠\ High\ Severity\ —\ Security} \quad \color{gray}{\texttt{\small Iteration\ 1}}$

ReadPermission guard also removed from warehouse activity check

The same ReadPermission() removal issue exists in CommonCheckWarehouseActivityLineIsAllowed: QltyInspectionHeader.ReadPermission() and QltyInspectionResult.ReadPermission() guards were dropped, so quality-blocking logic now runs for all users performing warehouse activity line operations, even on tenants that have not licensed or configured Quality Management.

Recommendation:

  • Apply the same fix as HandleOnAfterCheckItemTrackingInformation: add an explicit check that Quality Management is enabled before executing quality inspection enforcement logic.
if not QltyManagementSetup.GetSetupRecord() then
    exit;
if not QltyManagementSetup.IsEnabled() then
    exit;

Line mapping was unavailable, so this was posted as an issue comment.

👍 useful · ❤️ especially valuable · 👎 wrong - reply with why

@github-actions

github-actions Bot commented Jun 9, 2026

Copy link
Copy Markdown
Contributor

$\textbf{🟡\ Medium\ Severity\ —\ Security} \quad \color{gray}{\texttt{\small Iteration\ 1}}$

Error message loses actionable remediation guidance

The FinishBeforeReinspectionErr label had its actionable guidance removed: the sentence directing users to ask their administrator to grant finish-inspection permission via the Quality Inspection Permissions page was deleted. Users encountering this error will now receive a dead-end message with no path to resolution.

Recommendation:

  • Restore the administrator guidance sentence, or replace it with a more specific and up-to-date instruction (e.g. a link or page reference) so that users can self-serve or escalate effectively.
FinishBeforeReinspectionErr: Label 'An inspection must be finished before a Re-inspection can be made. This is done automatically, but you do not have permission to finish an inspection. Ask your administrator to grant you the Finish Inspection permission in the Quality Inspection Permissions page.';

Line mapping was unavailable, so this was posted as an issue comment.

👍 useful · ❤️ especially valuable · 👎 wrong - reply with why

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hold for additional check

@alexei-dobriansky

Copy link
Copy Markdown
Contributor

Agentic PR Review - Round 1

Recommendation: Request Changes

What this PR does

This PR adds procedure-level implicit permissions to Quality Management integration subscribers so automatic inspection creation and item-tracking/warehouse restrictions run even when the current user lacks direct Quality Management permissions. That is the right enforcement direction for the described issue, and the tracking change intentionally removes the old ReadPermission() short-circuit. However, the diff still leaves at least two production source-update subscribers dependent on direct setup read permission, and there are no changed tests for a security/permission behavior change.

Suggestions

S1 - Add permission regression coverage
Add tests under src\Apps\W1\Quality Management\test\src that run the changed posting/restriction paths with restrictive or lowered permissions and assert both blocked and allowed behavior. This PR changes permission enforcement but modifies no test file, so there is no proof that the implicit permissions cover the intended paths without over-granting.

S2 - Cover production source update subscribers
QltyManufacturIntegration.Codeunit.al still calls QltyManagementSetup.GetSetupRecord() in HandleOnAfterToProdOrderLineModify and HandleOnAfterToProdOrderRtngLineInsert before reaching helper methods with implicit header permissions. Add the same implicit setup/header permissions, or otherwise remove the direct-permission dependency, so production line/routing source updates are not silently skipped for users without Quality Management read access.

S3 - Restore actionable reinspection remediation
QltyInspectionHeader.Table.al shortens FinishBeforeReinspectionErr while the code still errors when CanFinishInspection() is false before automatically finishing an inspection for re-inspection. Keep or replace the administrator guidance so users know which Quality Inspection permission to request when re-inspection creation is blocked.

Risk assessment and necessity

Risk: The changed subscribers sit on purchase, sales return, transfer, assembly, manufacturing, warehouse posting/registering, and item-tracking validation paths. An incomplete permission fix can either continue bypassing mandatory inspections/restrictions or unexpectedly block operational transactions; the unannotated manufacturing line/routing callbacks are a concrete remaining gap.

Necessity: The scenario is important because users without direct Quality Management permissions should not bypass mandatory inspection creation or transaction restrictions. AB#626330 is linked, but the work item was not independently verified; the PR body and diff still show a valid enforcement need.


[AI-PR-REVIEW] version=1 system=github pr=8556 round=1 by=alexei-dobriansky at=2026-06-10T15:49:07Z lastSha=794939cd7756d1912ce0d0edb76b7fbe323c5074 suggestions=S1,S2,S3

@alexei-dobriansky alexei-dobriansky left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@JakovljevicDusan, please check if the suggestions are valid.

@JakovljevicDusan JakovljevicDusan marked this pull request as draft June 11, 2026 20:37
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

AL: Apps (W1) Add-on apps for W1

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants