Skip to content

[Subscription Billing] Remove internal modifiers and make enums extensible for partner extension support #8387

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
Copilot wants to merge 2 commits into
base: main
Choose a base branch
from
Open

[Subscription Billing] Remove internal modifiers and make enums extensible for partner extension support #8387

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
40a93bd
Initial plan
Copilot Jun 1, 2026
fcfacac
Remove internal modifiers and make enums extensible for partner exten…
Copilot Jun 1, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 2 additions & 3 deletions src/Apps/W1/Subscription Billing/App/Base/Codeunits/PersonalizationDataMgmt.Codeunit.al
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,9 +4,8 @@ using System.Environment.Configuration;

codeunit 8020 "Personalization Data Mgmt."
{
Access = Internal;

internal procedure SetDataPagePersonalization(ObjectType: Option ,,,Report,,,XMLport,,Page; ObjectID: Text; ValueName: Code[40]; Value: Text)
procedure SetDataPagePersonalization(ObjectType: Option ,,,Report,,,XMLport,,Page; ObjectID: Text; ValueName: Code[40]; Value: Text)

@github-actions github-actions Bot Jun 2, 2026

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.

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

Personalization write API exposed without InherentPermissions

Removing Access = Internal makes SetDataPagePersonalization callable by any partner extension. The method directly inserts or modifies rows in the Page Data Personalization system table. Without an [InherentPermissions] annotation, there is no explicit permission contract for callers, and the ObjectID text is parsed via CopyStr+Evaluate without error handling—a non-numeric suffix causes a runtime error.

Recommendation:

  • Add [InherentPermissions(PermissionObjectType::TableData, Database::"Page Data Personalization", 'IMD')] and wrap the Evaluate(ObjectNo, ObjectID) call in an if not Evaluate(...) guard to prevent unhandled runtime errors on malformed input.
Suggested change
procedure SetDataPagePersonalization(ObjectType: Option ,,,Report,,,XMLport,,Page; ObjectID: Text; ValueName: Code[40]; Value: Text)
[InherentPermissions(PermissionObjectType::TableData, Database::"Page Data Personalization", 'IMD')]
procedure SetDataPagePersonalization(ObjectType: Option ,,,Report,,,XMLport,,Page; ObjectID: Text; ValueName: Code[40]; Value: Text)
var
...
ObjectNo: Integer;
begin
ObjectID := CopyStr(ObjectID, StrPos(ObjectID, ' ') + 1);
if not Evaluate(ObjectNo, ObjectID) then
exit;
...

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

var
PageDataPersonalization: Record "Page Data Personalization";
BigText: BigText;
Expand All @@ -27,7 +26,7 @@ codeunit 8020 "Personalization Data Mgmt."
PageDataPersonalization.Modify(false);
end;

internal procedure GetDataPagePersonalization(ObjectType: Option ,,,Report,,,XMLport,,Page; ObjectID: Text; ValueName: Code[40]; var Value: Text): Boolean
procedure GetDataPagePersonalization(ObjectType: Option ,,,Report,,,XMLport,,Page; ObjectID: Text; ValueName: Code[40]; var Value: Text): Boolean

@github-actions github-actions Bot Jun 2, 2026

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.

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

User personalization read API exposed to partners

Making GetDataPagePersonalization public allows partner extensions to read arbitrary personalization values stored for the current user (UserSecurityId()). Values stored via SetDataPagePersonalization are free-form text blobs with no DataClassification tag and no audit trail, so partner code could silently read or infer user preferences, saved state, or other user-associated data.

Recommendation:

  • If this API must remain public, add a DataClassification annotation to the Page Data Personalization value field (or document that callers must only write DataClassification::SystemMetadata data), and expose read access through a narrower, purpose-specific method rather than a generic key-value getter.
Suggested change
procedure GetDataPagePersonalization(ObjectType: Option ,,,Report,,,XMLport,,Page; ObjectID: Text; ValueName: Code[40]; var Value: Text): Boolean
[InherentPermissions(PermissionObjectType::TableData, Database::"Page Data Personalization", 'R')]
procedure GetDataPagePersonalization(ObjectType: Option ,,,Report,,,XMLport,,Page; ObjectID: Text; ValueName: Code[40]; var Value: Text): Boolean

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

var
PageDataPersonalization: Record "Page Data Personalization";
BigText: BigText;
Expand Down
2 changes: 1 addition & 1 deletion src/Apps/W1/Subscription Billing/App/Billing/Enums/ContractInvoiceTextType.Enum.al
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@ namespace Microsoft.SubscriptionBilling;

enum 8001 "Contract Invoice Text Type"
{
Extensible = false;
Extensible = true;

value(0; " ")
{
Expand Down
2 changes: 1 addition & 1 deletion src/Apps/W1/Subscription Billing/App/Billing/Enums/CustomerRecBillingGrouping.Enum.al
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@ namespace Microsoft.SubscriptionBilling;

enum 8057 "Customer Rec. Billing Grouping"
{
Extensible = false;

@github-actions github-actions Bot Jun 2, 2026

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.

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

Extensible enum breaks billing document creation

CreateBillingDocuments.Codeunit.al contains two case CustomerRecurringBillingGrouping of … end; blocks (lines 58–64 and 737–742) with no else clause. When a partner adds a new grouping value, the outer case at line 58 silently creates no billing documents, and the inner case at line 737 leaves PartnerNo empty, which would create billing documents with a blank customer.

Recommendation:

  • Before merging, add else Error(...) or an OnBeforeProcessBillingLines integration event handler in both case blocks in CreateBillingDocuments.Codeunit.al so that unrecognised grouping values fail fast rather than silently misbehaving.
Suggested change
Extensible = false;
case CustomerRecurringBillingGrouping of
CustomerRecurringBillingGrouping::Contract:
CreateSalesDocumentsPerContract();
CustomerRecurringBillingGrouping::"Sell-to Customer No.",
CustomerRecurringBillingGrouping::"Bill-to Customer No.":
CreateSalesDocumentsPerCustomer();
else
Error(UnhandledBillingGroupingErr, CustomerRecurringBillingGrouping);
end;

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

Extensible = true;

value(0; "Contract")
{
Expand Down
2 changes: 1 addition & 1 deletion src/Apps/W1/Subscription Billing/App/Billing/Enums/RecBillingDocumentType.Enum.al
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@ namespace Microsoft.SubscriptionBilling;

enum 8054 "Rec. Billing Document Type"
{
Extensible = false;
Extensible = true;
value(0; None)
{
Caption = ' ', Locked = true;
Expand Down
2 changes: 1 addition & 1 deletion src/Apps/W1/Subscription Billing/App/Billing/Enums/VendorRecBillingGrouping.Enum.al
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@ namespace Microsoft.SubscriptionBilling;

enum 8058 "Vendor Rec. Billing Grouping"
{
Extensible = false;

@github-actions github-actions Bot Jun 2, 2026

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.

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

Extensible enum breaks vendor billing creation

CreateBillingDocuments.Codeunit.al has two case VendorRecurringBillingGrouping of … end; blocks (lines 66–72 and 748–752) with no else clause. A partner-added grouping value will cause the first block to create no purchase documents and the second block to leave PartnerNo empty, silently producing billing documents with a blank vendor.

Recommendation:

  • Add else Error(...) to both case blocks in CreateBillingDocuments.Codeunit.al for VendorRecurringBillingGrouping, mirroring the existing OnGetAdditionalLineTextElseCase pattern used for ContractInvoiceTextType.
Suggested change
Extensible = false;
case VendorRecurringBillingGrouping of
VendorRecurringBillingGrouping::Contract:
CreatePurchaseDocumentsPerContract();
VendorRecurringBillingGrouping::"Pay-to Vendor No.",
VendorRecurringBillingGrouping::"Buy-from Vendor No.":
CreatePurchaseDocumentsPerVendor();
else
Error(UnhandledBillingGroupingErr, VendorRecurringBillingGrouping);
end;

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

Extensible = true;

value(0; "Contract")
{
Expand Down
4 changes: 2 additions & 2 deletions src/Apps/W1/Subscription Billing/App/Customer Contracts/Pages/ExtendContract.Page.al
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -630,7 +630,7 @@ page 8002 "Extend Contract"
TempServiceCommitmentPackage.SetRange(Selected);
end;

internal procedure SetParameters(NewCustomerNo: Code[20]; NewCustomerContractNo: Code[20]; NewProvisionStartDate: Date; NewExtendCustomerContract: Boolean)
procedure SetParameters(NewCustomerNo: Code[20]; NewCustomerContractNo: Code[20]; NewProvisionStartDate: Date; NewExtendCustomerContract: Boolean)
begin
SellToCustomerNoParam := NewCustomerNo;
CustomerContractNoParam := NewCustomerContractNo;
Expand Down Expand Up @@ -661,7 +661,7 @@ page 8002 "Extend Contract"
end;
end;

internal procedure SetUsageBasedParameters(SupplierNo: Code[20]; NewSubscriptionEntryNo: Integer)
procedure SetUsageBasedParameters(SupplierNo: Code[20]; NewSubscriptionEntryNo: Integer)
begin
UsageDataSupplierNoParam := SupplierNo;
SubscriptionEntryNoParam := NewSubscriptionEntryNo;
Expand Down
2 changes: 1 addition & 1 deletion ...ion Billing/App/Sales Service Commitments/Codeunits/SalesSubscriptionLineMgmt.Codeunit.al
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,7 @@ codeunit 8069 "Sales Subscription Line Mgmt."
AddSalesServiceCommitmentsForSalesLine(Rec, false);
end;

internal procedure AddSalesServiceCommitmentsForSalesLine(var SalesLine: Record "Sales Line"; SkipAddAdditionalSalesServComm: Boolean)

@github-actions github-actions Bot Jun 2, 2026

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.

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

Subscription line attachment bypasses sales-line triggers

Making AddSalesServiceCommitmentsForSalesLine public allows partner extensions to attach subscription package commitments to any Sales Line record outside the normal OnAfterInsert/OnAfterModify trigger flow. This can attach commitments to lines that should not carry subscriptions, or double-add commitments if called together with the event-driven path.

Recommendation:

  • Add an [InherentPermissions] attribute and consider exposing an OnBeforeAddSalesServiceCommitmentsForSalesLine integration event so that partners can influence the existing trigger-driven call rather than bypassing it entirely.
Suggested change
internal procedure AddSalesServiceCommitmentsForSalesLine(var SalesLine: Record "Sales Line"; SkipAddAdditionalSalesServComm: Boolean)
[InherentPermissions(PermissionObjectType::TableData, Database::"Sales Subscription Line", 'IM')]
procedure AddSalesServiceCommitmentsForSalesLine(var SalesLine: Record "Sales Line"; SkipAddAdditionalSalesServComm: Boolean)

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

procedure AddSalesServiceCommitmentsForSalesLine(var SalesLine: Record "Sales Line"; SkipAddAdditionalSalesServComm: Boolean)
var
ItemServCommitmentPackage: Record "Item Subscription Package";
SalesHeader: Record "Sales Header";
Expand Down
2 changes: 1 addition & 1 deletion ...Subscription Billing/App/Sales Service Commitments/Table Extensions/SalesLine.TableExt.al
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -377,7 +377,7 @@ tableextension 8054 "Sales Line" extends "Sales Line"
exit(Rec.IsContractRenewal());
end;

internal procedure IsContractRenewal(): Boolean
procedure IsContractRenewal(): Boolean
var
SalesServiceCommitment: Record "Sales Subscription Line";
begin
Expand Down
6 changes: 3 additions & 3 deletions ...pps/W1/Subscription Billing/App/Service Commitments/Pages/ServiceCommPackageLines.Page.al
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -188,17 +188,17 @@ page 8058 "Service Comm. Package Lines"

Bold: Boolean;

internal procedure SetItemNo(NewItemNo: Code[20])
procedure SetItemNo(NewItemNo: Code[20])
begin
ItemNo := NewItemNo;
end;

internal procedure SetShowAllPackageLines(NewShowAllPackageLines: Boolean)
procedure SetShowAllPackageLines(NewShowAllPackageLines: Boolean)
begin
ShowAllPackageLines := NewShowAllPackageLines;
end;

internal procedure SetPackageCode(NewPackageCode: Code[20])
procedure SetPackageCode(NewPackageCode: Code[20])
begin
PackageCode := NewPackageCode;
SetDefaultFilters();
Expand Down
8 changes: 4 additions & 4 deletions ...s/W1/Subscription Billing/App/Service Commitments/Tables/ItemSubscriptionPackage.Table.al
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -88,12 +88,12 @@ table 8058 "Item Subscription Package"
PackageFilter := GetPackageFilterForItem(ItemNo, '');
end;

internal procedure GetPackageFilterForItem(ItemNo: Code[20]; ServiceObjectNo: Code[20]) PackageFilter: Text
procedure GetPackageFilterForItem(ItemNo: Code[20]; ServiceObjectNo: Code[20]) PackageFilter: Text
begin
PackageFilter := GetPackageFilterForItem(ItemNo, ServiceObjectNo, false);
end;

internal procedure GetPackageFilterForItem(ItemNo: Code[20]; ServiceObjectNo: Code[20]; OnlyNonStandardPackage: Boolean) PackageFilter: Text
procedure GetPackageFilterForItem(ItemNo: Code[20]; ServiceObjectNo: Code[20]; OnlyNonStandardPackage: Boolean) PackageFilter: Text
var
ItemServCommitmentPackage: Record "Item Subscription Package";
TextManagement: Codeunit "Text Management";
Expand Down Expand Up @@ -122,7 +122,7 @@ table 8058 "Item Subscription Package"
exit(not ServiceCommitment.IsEmpty());
end;

internal procedure GetPackageFilterForItem(SalesLine: Record "Sales Line"; RemoveExistingPackageFromFilter: Boolean) PackageFilter: Text
procedure GetPackageFilterForItem(SalesLine: Record "Sales Line"; RemoveExistingPackageFromFilter: Boolean) PackageFilter: Text
var
ItemServCommitmentPackage: Record "Item Subscription Package";
TextManagement: Codeunit "Text Management";
Expand Down Expand Up @@ -150,7 +150,7 @@ table 8058 "Item Subscription Package"
exit(not SalesServiceCommitment.IsEmpty());
end;

internal procedure GetAllStandardPackageFilterForItem(ItemNo: Code[20]; CustomerPriceGroup: Code[10]) PackageFilter: Text
procedure GetAllStandardPackageFilterForItem(ItemNo: Code[20]; CustomerPriceGroup: Code[10]) PackageFilter: Text
var
ItemServCommitmentPackage: Record "Item Subscription Package";
TextManagement: Codeunit "Text Management";
Expand Down
4 changes: 2 additions & 2 deletions src/Apps/W1/Subscription Billing/App/Service Commitments/Tables/SubscriptionPackage.Table.al
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -135,7 +135,7 @@ table 8055 "Subscription Package"
exit(Input in ['0' .. '9']);
end;

internal procedure FilterCodeOnPackageFilter(PackageFilter: Text)
procedure FilterCodeOnPackageFilter(PackageFilter: Text)
begin
if PackageFilter = '' then
Rec.SetRange(Code, '')
Expand All @@ -153,7 +153,7 @@ table 8055 "Subscription Package"
exit(not SubscriptionPackageLine.IsEmpty());
end;

internal procedure ServCommPackageLineExists(): Boolean
procedure ServCommPackageLineExists(): Boolean
var
SubscriptionPackageLine: Record "Subscription Package Line";
begin
Expand Down
4 changes: 2 additions & 2 deletions src/Apps/W1/Subscription Billing/App/Service Objects/Tables/SubscriptionHeader.Table.al
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2247,14 +2247,14 @@ table 8057 "Subscription Header"
OnAfterInsertFromItemNoAndCustomerContract(ServiceObject, CustomerContract);
end;

internal procedure SetUnitPriceAndUnitCostFromExtendContract(NewUnitPrice: Decimal; NewUnitCost: Decimal)

@github-actions github-actions Bot Jun 2, 2026

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.

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

Public pricing override bypasses price calculation

Making SetUnitPriceAndUnitCostFromExtendContract public allows any partner extension to set CalledFromExtendContract := true with arbitrary prices. Once set, all subscription lines created from this table instance use the caller-supplied prices instead of the normal price calculation logic (lines 1947–1958 in the same file), with no guard ensuring ResetCalledFromExtendContract is called afterward.

Recommendation:

  • Add an [InherentPermissions] attribute or expose only through a dedicated integration event. If the public API is intentional, document that callers must always call ResetCalledFromExtendContract() in a try-finally block to avoid pricing state leakage.
Suggested change
internal procedure SetUnitPriceAndUnitCostFromExtendContract(NewUnitPrice: Decimal; NewUnitCost: Decimal)
[InherentPermissions(PermissionObjectType::TableData, Database::"Subscription Header", 'M')]
procedure SetUnitPriceAndUnitCostFromExtendContract(NewUnitPrice: Decimal; NewUnitCost: Decimal)
begin
CalledFromExtendContract := true;
UnitPrice := NewUnitPrice;
UnitCost := NewUnitCost;
end;

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

procedure SetUnitPriceAndUnitCostFromExtendContract(NewUnitPrice: Decimal; NewUnitCost: Decimal)
begin
CalledFromExtendContract := true;
UnitPrice := NewUnitPrice;
UnitCost := NewUnitCost;
end;

internal procedure ResetCalledFromExtendContract()

@github-actions github-actions Bot Jun 2, 2026

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.

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

ResetCalledFromExtendContract lacks pairing contract

Exposing ResetCalledFromExtendContract publicly alongside SetUnitPriceAndUnitCostFromExtendContract without documenting a call-pair contract means partners could call Reset at arbitrary points, unexpectedly zeroing UnitPrice/UnitCost mid-flow when another caller has legitimately set them. There is no re-entrancy guard or usage counter protecting the state.

Recommendation:

  • Document or enforce that these methods must be used as a matched set. Consider returning a disposable context object or using a try-finally guard pattern to ensure the reset is always paired with the set.
Suggested change
internal procedure ResetCalledFromExtendContract()
/// <summary>Must always be called in a try-finally block after SetUnitPriceAndUnitCostFromExtendContract.</summary>
procedure ResetCalledFromExtendContract()
begin
CalledFromExtendContract := false;
UnitPrice := 0;
UnitCost := 0;
end;

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

procedure ResetCalledFromExtendContract()
begin
CalledFromExtendContract := false;
UnitPrice := 0;
Expand Down
3 changes: 1 addition & 2 deletions ...cription Billing/App/Usage Based Billing/Codeunits/UsageBasedContrSubscribers.Codeunit.al
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,6 @@ using Microsoft.Sales.Posting;

codeunit 8028 "Usage Based Contr. Subscribers"
{
Access = Internal;

var
UsageBasedDocTypeConv: Codeunit "Usage Based Doc. Type Conv.";
Expand Down Expand Up @@ -81,7 +80,7 @@ codeunit 8028 "Usage Based Contr. Subscribers"
exit(not UsageDataBilling.IsEmpty());
end;

internal procedure CreateContractInvoicesFromUsageDataImport(ServicePartner: Enum "Service Partner"; ContractNoFilter: Text; ContractLineFilter: Text; BillingRhytmFilter: Text)
procedure CreateContractInvoicesFromUsageDataImport(ServicePartner: Enum "Service Partner"; ContractNoFilter: Text; ContractLineFilter: Text; BillingRhytmFilter: Text)

@github-actions github-actions Bot Jun 2, 2026

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.

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

Financial billing API exposed without permission guard

Removing Access = Internal and the internal modifier from CreateContractInvoicesFromUsageDataImport allows any partner extension to invoke invoice creation for arbitrary contract/line filters. The method has no [InherentPermissions] attribute, so there is no explicit declaration of what permissions callers require; a misconfigured extension could trigger billing runs it should not be allowed to start.

Recommendation:

  • Add an [InherentPermissions] attribute that declares the minimum tabledata permissions needed, or keep the codeunit Access = Internal and instead expose the functionality via a dedicated integration event.
Suggested change
procedure CreateContractInvoicesFromUsageDataImport(ServicePartner: Enum "Service Partner"; ContractNoFilter: Text; ContractLineFilter: Text; BillingRhytmFilter: Text)
[InherentPermissions(PermissionObjectType::TableData, Database::"Billing Line", 'R')]
procedure CreateContractInvoicesFromUsageDataImport(ServicePartner: Enum "Service Partner"; ContractNoFilter: Text; ContractLineFilter: Text; BillingRhytmFilter: Text)

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

begin
case ServicePartner of
ServicePartner::Customer:
Expand Down
2 changes: 1 addition & 1 deletion ...ps/W1/Subscription Billing/App/Usage Based Billing/Enums/UsageBasedBillingDocType.Enum.al
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@ namespace Microsoft.SubscriptionBilling;

enum 8008 "Usage Based Billing Doc. Type"
{
Extensible = false;

@github-actions github-actions Bot Jun 2, 2026

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.

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

Extensible enum leaves cleanup logic with silent gaps

Document cleanup logic in UsageBasedContrSubscribers.Codeunit.al (lines 158–164 and 186–192) uses if … = ::Invoice … else if … = ::"Credit Memo" chains. When a partner adds a new UsageBasedBillingDocType value, deleted sales/purchase header records will not trigger any cleanup of UsageDataBilling rows, leaving stale billing references that corrupt usage reconciliation.

Recommendation:

  • Expose an OnAfterCleanupUsageDataBillingForDocumentType integration event in the document-cleanup local procedures so that partners extending the enum can implement the corresponding cleanup logic.
Suggested change
Extensible = false;
[IntegrationEvent(false, false)]
local procedure OnAfterCleanupUsageDataBillingForDocumentType(ServicePartner: Enum "Service Partner"; UsageBasedBillingDocType: Enum "Usage Based Billing Doc. Type"; DocumentNo: Code[20]; var IsHandled: Boolean)
begin
end;

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

Extensible = true;
value(0; None)
{
Caption = ' ', Locked = true;
Expand Down
2 changes: 1 addition & 1 deletion src/Apps/W1/Subscription Billing/App/Usage Based Billing/Tables/UsageDataImport.Table.al
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -295,7 +295,7 @@ table 8013 "Usage Data Import"
CreateCustomerInvoices(CustomerContractFilter, CustomerContractLineFilter);
end;

internal procedure CollectVendorContractsAndCreateInvoices(var UsageDataImport: Record "Usage Data Import")

@github-actions github-actions Bot Jun 2, 2026

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.

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

Vendor invoice creation publicly callable without guards

Making CollectVendorContractsAndCreateInvoices public allows partner code to initiate vendor contract invoice creation for any UsageDataImport record set. No [InherentPermissions] attribute documents the required access level, and there are no pre-conditions validating the caller's intent or the state of the import records.

Recommendation:

  • Annotate with [InherentPermissions] declaring the minimum permissions needed (e.g., indirect Modify on billing tables), or restrict to an integration event pattern so that partner logic can hook in without taking full control of the invoicing flow.
Suggested change
internal procedure CollectVendorContractsAndCreateInvoices(var UsageDataImport: Record "Usage Data Import")
[InherentPermissions(PermissionObjectType::TableData, Database::"Usage Data Import", 'RM')]
procedure CollectVendorContractsAndCreateInvoices(var UsageDataImport: Record "Usage Data Import")

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

procedure CollectVendorContractsAndCreateInvoices(var UsageDataImport: Record "Usage Data Import")
var
VendorContractFilter: Text;
VendorContractLineFilter: Text;
Expand Down
2 changes: 1 addition & 1 deletion ...1/Subscription Billing/App/Usage Based Billing/Tables/UsageDataSupplierReference.Table.al
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -73,7 +73,7 @@ table 8015 "Usage Data Supplier Reference"
Rec.Reset();
end;

internal procedure FindSupplierReference(SupplierNo: Code[20]; SupplierReference: Text[80]; ReferenceType: Enum "Usage Data Reference Type"): Boolean
procedure FindSupplierReference(SupplierNo: Code[20]; SupplierReference: Text[80]; ReferenceType: Enum "Usage Data Reference Type"): Boolean
begin
Rec.FilterUsageDataSupplierReference(SupplierNo, SupplierReference, ReferenceType);
exit(Rec.FindFirst());
Expand Down
Loading