mirror of
https://github.com/microsoft/BCQuality.git
synced 2026-08-06 09:26:52 +01:00
Correct security and privacy guidance
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: 9c2eebc4-dcd5-4b85-8113-90772d818900
This commit is contained in:
Jesper Schulz-Wedde
parent
7a678d1aff
commit
ec8f891954
34 changed files with 189 additions and 150 deletions
|
|
@ -2,10 +2,16 @@ codeunit 50207 "Privacy Sample StrSubstNo Bad"
|
|||
{
|
||||
procedure ReportFailure(var Customer: Record Customer)
|
||||
var
|
||||
ErrorMsg: Text;
|
||||
CustomerInvalidErr: Label 'Customer %1 has invalid data.', Comment = '%1 = Customer No.';
|
||||
begin
|
||||
ErrorMsg := StrSubstNo('Customer %1 (%2) at %3 has invalid data',
|
||||
Customer.Name, Customer."E-Mail", Customer.Address);
|
||||
Error(ErrorMsg);
|
||||
Error(StrSubstNo(CustomerInvalidErr, Customer."No."));
|
||||
end;
|
||||
|
||||
procedure ReportCombinedFailure()
|
||||
var
|
||||
HeaderErr: Label 'Customer validation failed. ';
|
||||
DetailErr: Label 'Correct the customer card and try again.';
|
||||
begin
|
||||
Error(HeaderErr + DetailErr);
|
||||
end;
|
||||
}
|
||||
|
|
|
|||
|
|
@ -1,5 +1,5 @@
|
|||
---
|
||||
bc-version: [all]
|
||||
bc-version: [20..]
|
||||
domain: privacy
|
||||
keywords: [strsubstno, error, telemetry, pii, prebuild, text-variable]
|
||||
technologies: [al]
|
||||
|
|
@ -7,20 +7,20 @@ countries: [w1]
|
|||
application-area: [all]
|
||||
---
|
||||
|
||||
# Do not pre-build an error string with `StrSubstNo` before calling `Error()`
|
||||
# Pass a Label directly as the first Error argument
|
||||
|
||||
## Description
|
||||
|
||||
`StrSubstNo` returns a plain `Text` value with the substitutions already performed. When that result is then passed to `Error()`, the platform sees a single plain-text parameter with no field references left to inspect, so it cannot apply `DataClassification` to anything inside it. Whatever PII the `StrSubstNo` call interpolated — customer name, e-mail, address, error text — is logged verbatim to telemetry. This is the canonical way to accidentally leak customer data through error telemetry, and it is the only `Error()` shape that needs to be flagged.
|
||||
Error method trace telemetry includes the AL error string only when the first `Error` argument is a `Label` or `TextConst`. Wrapping a label in `StrSubstNo`, or concatenating labels or text, produces a dynamic `Text` first argument. In that case the actual string is not emitted as the telemetry message; the platform emits its generic guidance instead. CodeCop AA0231 flags both shapes because the label identity and data-classification context are lost.
|
||||
|
||||
## Best Practice
|
||||
|
||||
Call `Error()` directly with the format string and the substitution parameters. The platform classifies each parameter individually and handles telemetry correctly even when the parameters are PII fields (see `error-direct-substitution-safe-for-telemetry.md`). If the message text needs to be a `Label`, pass the `Label` and the parameters to `Error()` — do not pre-render via `StrSubstNo`.
|
||||
Declare the complete message as a `Label` or `TextConst` and pass it directly to `Error`, followed by substitution values. The client receives the formatted message while telemetry can retain the static message template without using the dynamic values as its message. See `error-direct-substitution-safe-for-telemetry.md`.
|
||||
|
||||
See sample: `avoid-strsubstno-prebuild-before-error.good.al`.
|
||||
|
||||
## Anti Pattern
|
||||
|
||||
Assigning `StrSubstNo('Customer %1 (%2) ...', Customer.Name, Customer."E-Mail")` to a `Text` variable and then calling `Error(ErrorMsg)`. The platform has nothing to classify by the time `Error` runs — the PII is baked into the string and goes straight to telemetry. Detection signal for a reviewer: any `Text` variable assigned from `StrSubstNo` and later passed as the *only* parameter to `Error()`.
|
||||
`Error(StrSubstNo(CustomerInvalidErr, Customer."No."))` and `Error(HeaderErr + DetailErr)` both make the first argument dynamic. They reduce error telemetry quality; they do not cause that composed string to be logged verbatim as the telemetry message.
|
||||
|
||||
See sample: `avoid-strsubstno-prebuild-before-error.bad.al`.
|
||||
|
|
|
|||
|
|
@ -1,5 +1,5 @@
|
|||
---
|
||||
bc-version: [all]
|
||||
bc-version: [20..]
|
||||
domain: privacy
|
||||
keywords: [error, strsubstno, direct-substitution, telemetry, classification, label]
|
||||
technologies: [al]
|
||||
|
|
@ -7,18 +7,18 @@ countries: [w1]
|
|||
application-area: [all]
|
||||
---
|
||||
|
||||
# `Error()` with direct substitution parameters is always safe for telemetry
|
||||
# Use a Label or TextConst for the Error telemetry message
|
||||
|
||||
## Description
|
||||
|
||||
When `Error()` is called with a format string and direct substitution parameters (`%1`, `%2`, …), the BC platform intercepts the call, inspects each parameter individually, and applies the `DataClassification` of the source field — stripping or masking sensitive data before writing the message to telemetry. This is true regardless of whether a parameter is a record field reference, a local variable, a function return value, or any other expression. Patterns such as `Error('Invalid email: %1', Customer."E-Mail")` are therefore safe even when the parameter is PII: the platform sees `Customer."E-Mail"` as a `CustomerContent` field reference and handles it correctly.
|
||||
For Error method trace telemetry, the platform includes the AL error string only when `Error` receives a `Label` or `TextConst` as its first argument. Substitution values format the client message, but the static label supplies the telemetry message and preserves its classification context. A string literal, local `Text`, `StrSubstNo` result, or concatenation is not equivalent: telemetry substitutes generic guidance instead of that dynamic string.
|
||||
|
||||
## Best Practice
|
||||
|
||||
Pass values to `Error()` as direct substitution parameters — either inline or via a `Label` with `Comment = '%1 = …'` placeholders. Let the platform do the per-parameter classification. This works equally well for record fields, local text variables, and document IDs.
|
||||
Define the complete error template as a `Label` with placeholder comments, pass the label directly as the first argument, and pass values separately. Independently review whether those values are appropriate to show to the current user.
|
||||
|
||||
See sample: `error-direct-substitution-safe-for-telemetry.good.al`.
|
||||
|
||||
## Anti Pattern
|
||||
|
||||
Treating any `Error()` call that mentions PII as a leak. A review skill that flags `Error('Invalid email: %1', EmailAddress)` is wrong; the platform handles that pattern correctly. The only `Error()` shape that genuinely leaks PII to telemetry is the pre-built `StrSubstNo` form covered in `avoid-strsubstno-prebuild-before-error.md`.
|
||||
Assuming that any direct format string is telemetry-safe, or that a `StrSubstNo`/concatenated first argument is logged verbatim. The required telemetry shape is specifically a directly supplied `Label` or `TextConst`; see `avoid-strsubstno-prebuild-before-error.md`.
|
||||
|
|
|
|||
|
|
@ -1,5 +1,5 @@
|
|||
---
|
||||
bc-version: [all]
|
||||
bc-version: [20..]
|
||||
domain: privacy
|
||||
keywords: [error, message, confirm, notification, telemetry, logging, ui-dialog]
|
||||
technologies: [al]
|
||||
|
|
@ -7,16 +7,16 @@ countries: [w1]
|
|||
application-area: [all]
|
||||
---
|
||||
|
||||
# Only `Error()` is logged to telemetry — `Message`, `Confirm`, `Notification` are not
|
||||
# Error dialogs emit Error method trace telemetry
|
||||
|
||||
## Description
|
||||
|
||||
The privacy concern with dialog APIs is not what the signed-in user sees on the screen — it is what the platform writes to telemetry. The BC platform automatically captures `Error()` invocations in the telemetry stream; it does not capture `Message()`, `Confirm()` or `Notification` calls. That asymmetry is the reason privacy review focuses on `Error()` text and ignores the other dialog APIs: a `Message` that shows a customer's email to the signed-in user reveals nothing they were not already entitled to see, while an `Error` carrying the same email leaks it to a separate, longer-lived telemetry destination.
|
||||
When `Error` displays a dialog, Business Central emits the RT0030 Error method trace telemetry signal. `Message`, `Confirm`, and `Notification` do not emit that Error method trace signal. For RT0030, the actual AL error string is included only when the first `Error` argument is a `Label` or `TextConst`; other first-argument types produce generic guidance instead of the dynamic string.
|
||||
|
||||
## Best Practice
|
||||
|
||||
Treat `Error()` as a telemetry surface, not just a UI surface — review the message text and parameters with the same scrutiny you apply to `Session.LogMessage`. Treat `Message()`, `Confirm()`, and `Notification` as pure UI: showing business data the user is permissioned for is normal functionality.
|
||||
Use a `Label` or `TextConst` as the direct first argument to `Error` so telemetry contains a stable, classified message. Review user-facing substitution values for UI appropriateness. Do not treat `Message`, `Confirm`, or `Notification` content as though it were automatically copied into RT0030.
|
||||
|
||||
## Anti Pattern
|
||||
|
||||
Flagging `Message`/`Confirm`/`Notification` calls for "showing PII" — they are not logged to telemetry, and the user already has permission to the underlying data. The inverse anti-pattern is treating `Error()` as harmless because the user sees only a dialog: the message is also written verbatim to telemetry.
|
||||
Claiming that every rendered `Error` string is written verbatim to telemetry, or that `Message`, `Confirm`, and `Notification` automatically feed the Error method trace. Both overstate the platform behavior.
|
||||
|
|
|
|||
|
|
@ -2,12 +2,18 @@ codeunit 50209 "Privacy Sample GetLastError Bad"
|
|||
{
|
||||
procedure AddAttachment()
|
||||
var
|
||||
ErrorMsg: Text;
|
||||
AttachmentFailedErr: Label 'Attachment failed: %1', Comment = '%1 = underlying error';
|
||||
begin
|
||||
if not TryAddAttachment() then begin
|
||||
ErrorMsg := StrSubstNo('Attachment failed: %1', GetLastErrorText(true));
|
||||
Error(ErrorMsg);
|
||||
end;
|
||||
if not TryAddAttachment() then
|
||||
Error(StrSubstNo(AttachmentFailedErr, GetLastErrorText(true)));
|
||||
end;
|
||||
|
||||
procedure AddAttachmentWithConcatenation()
|
||||
var
|
||||
AttachmentFailedErr: Label 'Attachment failed: ';
|
||||
begin
|
||||
if not TryAddAttachment() then
|
||||
Error(AttachmentFailedErr + GetLastErrorText(true));
|
||||
end;
|
||||
|
||||
[TryFunction]
|
||||
|
|
|
|||
|
|
@ -2,15 +2,15 @@ codeunit 50208 "Privacy Sample GetLastError Good"
|
|||
{
|
||||
procedure AddAttachmentSafely()
|
||||
var
|
||||
AttachmentFailedErr: Label 'Failed to add email attachment. Please try again.';
|
||||
AttachmentFailedErr: Label 'Failed to add the attachment: %1', Comment = '%1 = underlying error shown to the user';
|
||||
begin
|
||||
if not TryAddAttachment() then
|
||||
Error(AttachmentFailedErr);
|
||||
Error(AttachmentFailedErr, GetLastErrorText(true));
|
||||
end;
|
||||
|
||||
[TryFunction]
|
||||
local procedure TryAddAttachment()
|
||||
begin
|
||||
// ... attachment logic that may fail with a customer-data-bearing error ...
|
||||
// Attachment logic that can fail with a customer-data-bearing error.
|
||||
end;
|
||||
}
|
||||
|
|
|
|||
|
|
@ -1,5 +1,5 @@
|
|||
---
|
||||
bc-version: [all]
|
||||
bc-version: [20..]
|
||||
domain: privacy
|
||||
keywords: [getlasterrortext, error, strsubstno, telemetry, customer-data, attachment]
|
||||
technologies: [al]
|
||||
|
|
@ -11,16 +11,16 @@ application-area: [all]
|
|||
|
||||
## Description
|
||||
|
||||
`GetLastErrorText()` returns the text of the last error that occurred in the context where it is called. That text routinely contains customer content — field values that triggered the validation, record keys, customer names, file names from upload failures, and similar fragments lifted from the failing operation. Re-emitting it through `StrSubstNo` into `Error()` bakes that customer data into a single plain-text parameter that the platform can no longer classify, so it is logged verbatim to telemetry (the same problem as any other `StrSubstNo`-pre-built error — see `avoid-strsubstno-prebuild-before-error.md`).
|
||||
`GetLastErrorText()` can contain customer content such as field values, record keys, and file names. When it is passed as a substitution value to an `Error` whose first argument is a `Label` or `TextConst`, the label supplies the Error method trace telemetry message. If `StrSubstNo` or concatenation makes `GetLastErrorText()` part of the first argument, the actual dynamic string is not emitted as that telemetry message; telemetry uses generic guidance instead.
|
||||
|
||||
## Best Practice
|
||||
|
||||
When the goal is to surface a recoverable failure to the user, raise a generic message that does not embed `GetLastErrorText()` content, and log technical detail separately via `Session.LogMessage` with the correct `DataClassification`. If you must propagate the inner error verbatim, re-raise it as a direct parameter of `Error()` (e.g., `Error('%1', GetLastErrorText())`) rather than concatenating with `StrSubstNo` so the platform can apply its own handling.
|
||||
Use a generic label when the user does not need the underlying detail. If showing the detail is appropriate, put `%1` in a label and pass `GetLastErrorText()` as a separate argument. This preserves a useful static telemetry message while keeping the dynamic value out of the telemetry message field.
|
||||
|
||||
See sample: `getlasterrortext-customer-content-in-errors.good.al`.
|
||||
|
||||
## Anti Pattern
|
||||
|
||||
`ErrorMsg := StrSubstNo('Attachment failed: %1', GetLastErrorText(true)); Error(ErrorMsg);` — the inner error text may carry filenames or record values, and `StrSubstNo` strips the platform's ability to filter them before they hit telemetry.
|
||||
`Error(StrSubstNo(AttachmentFailedErr, GetLastErrorText(true)))` or `Error(AttachmentPrefixErr + GetLastErrorText(true))`. Both lose the static first argument and trigger AA0231; neither causes the composed text to be logged verbatim as the Error telemetry message.
|
||||
|
||||
See sample: `getlasterrortext-customer-content-in-errors.bad.al`.
|
||||
|
|
|
|||
|
|
@ -4,10 +4,14 @@ codeunit 50217 "Privacy Sample Consent Bad"
|
|||
var
|
||||
HttpClient: HttpClient;
|
||||
Content: HttpContent;
|
||||
Payload: JsonObject;
|
||||
PayloadText: Text;
|
||||
Response: HttpResponseMessage;
|
||||
begin
|
||||
Content.WriteFrom(StrSubstNo('{"email":"%1","name":"%2"}',
|
||||
Customer."E-Mail", Customer.Name));
|
||||
Payload.Add('email', Customer."E-Mail");
|
||||
Payload.Add('name', Customer.Name);
|
||||
Payload.WriteTo(PayloadText);
|
||||
Content.WriteFrom(PayloadText);
|
||||
HttpClient.Post('https://api.externalservice.com/sync', Content, Response);
|
||||
end;
|
||||
}
|
||||
|
|
|
|||
|
|
@ -1,22 +1,35 @@
|
|||
codeunit 50216 "Privacy Sample Consent Good"
|
||||
{
|
||||
var
|
||||
ExternalSyncNoticeIdLbl: Label 'CONTOSO-EXTERNAL-SYNC', Locked = true;
|
||||
ExternalSyncNameLbl: Label 'Contoso External Sync', Locked = true;
|
||||
PrivacyTermsUrlLbl: Label 'https://contoso.example/privacy', Locked = true;
|
||||
|
||||
internal procedure RegisterPrivacyNotice()
|
||||
var
|
||||
PrivacyNotice: Codeunit "Privacy Notice";
|
||||
begin
|
||||
PrivacyNotice.CreatePrivacyNotice(
|
||||
ExternalSyncNoticeIdLbl, ExternalSyncNameLbl, PrivacyTermsUrlLbl);
|
||||
end;
|
||||
|
||||
procedure SendDataToExternalService(Customer: Record Customer)
|
||||
var
|
||||
PrivacyNotice: Codeunit "Privacy Notice";
|
||||
PrivacyNoticeRegistrations: Codeunit "Privacy Notice Registrations";
|
||||
HttpClient: HttpClient;
|
||||
Content: HttpContent;
|
||||
Payload: JsonObject;
|
||||
PayloadText: Text;
|
||||
Response: HttpResponseMessage;
|
||||
PrivacyConsentRequiredErr: Label 'Privacy notice consent is required for this integration.';
|
||||
begin
|
||||
if PrivacyNotice.GetPrivacyNoticeApprovalState(
|
||||
PrivacyNoticeRegistrations.GetExchangePrivacyNoticeId())
|
||||
<> "Privacy Notice Approval State"::Agreed
|
||||
then
|
||||
if not PrivacyNotice.ConfirmPrivacyNoticeApproval(ExternalSyncNoticeIdLbl) then
|
||||
Error(PrivacyConsentRequiredErr);
|
||||
|
||||
Content.WriteFrom(StrSubstNo('{"email":"%1","name":"%2"}',
|
||||
Customer."E-Mail", Customer.Name));
|
||||
Payload.Add('email', Customer."E-Mail");
|
||||
Payload.Add('name', Customer.Name);
|
||||
Payload.WriteTo(PayloadText);
|
||||
Content.WriteFrom(PayloadText);
|
||||
HttpClient.Post('https://api.externalservice.com/sync', Content, Response);
|
||||
end;
|
||||
}
|
||||
|
|
|
|||
|
|
@ -1,26 +1,26 @@
|
|||
---
|
||||
bc-version: [all]
|
||||
domain: privacy
|
||||
keywords: [privacy-notice, consent, http-client, outgoing-request, external-service, getprivacynoticeapprovalstate]
|
||||
keywords: [privacy-notice, consent, http-client, outgoing-request, external-service, confirmprivacynoticeapproval]
|
||||
technologies: [al]
|
||||
countries: [w1]
|
||||
application-area: [all]
|
||||
---
|
||||
|
||||
# Outgoing requests to external services require a Privacy Notice consent check
|
||||
# Check the custom Privacy Notice before external data transfer
|
||||
|
||||
## Description
|
||||
|
||||
Business Central ships a built-in Privacy Notice framework that the admin uses to grant or withhold per-integration consent for sending data to external services. The relevant API surface is `Codeunit "Privacy Notice"` (consent checks via `GetPrivacyNoticeApprovalState()`), `Codeunit "Privacy Notice Registrations"` (well-known notice IDs for integrations such as Exchange, OneDrive, Teams), and the `Enum "Privacy Notice Approval State"` with values `Agreed`, `Disagreed`, and `Not Set`. The admin UI is the **Privacy Notices Status** page. The compliance concern in code review is therefore not that personal data is included in an outgoing HTTP body — that is normal business functionality — but that the code path issuing the request contains no `PrivacyNotice.GetPrivacyNoticeApprovalState(...)` check.
|
||||
Business Central's `Codeunit "Privacy Notice"` creates notices and records per-integration approval. A custom integration needs its own stable notice ID; it must not borrow the Exchange or another built-in service's consent. `ConfirmPrivacyNoticeApproval` shows the notice when needed and returns whether the request is approved. `GetPrivacyNoticeApprovalState` checks an existing notice without showing UI.
|
||||
|
||||
## Best Practice
|
||||
|
||||
Before issuing an outgoing HTTP request to an external service, verify `PrivacyNotice.GetPrivacyNoticeApprovalState(<integration id>) = "Privacy Notice Approval State"::Agreed`. The check does not have to live next to the `HttpClient.Post` call — it can sit anywhere upstream in the same code path (for example in the page's `OnOpenPage`, in a wizard step, or in a setup action) as long as no execution path reaches the request without passing through it.
|
||||
Register the custom notice with `CreatePrivacyNotice` during setup or through `OnRegisterPrivacyNotices`. Before sending data, call `ConfirmPrivacyNoticeApproval(<custom id>)` outside a write transaction, or check `GetPrivacyNoticeApprovalState(<custom id>)` when the flow must not show UI. No path should issue the request without approval.
|
||||
|
||||
See sample: `privacy-notice-consent-for-external-data-transfer.good.al`.
|
||||
|
||||
## Anti Pattern
|
||||
|
||||
A `procedure SendDataToExternalService(...)` that posts customer data to an external endpoint with no `PrivacyNotice.GetPrivacyNoticeApprovalState` anywhere upstream. The same anti-pattern applies in reverse: removing an existing privacy-notice check from code that still issues the external call.
|
||||
A custom integration that posts data without checking its own notice, or that gates the call with a built-in ID such as the Exchange privacy notice ID. Consent for one service does not authorize another.
|
||||
|
||||
See sample: `privacy-notice-consent-for-external-data-transfer.bad.al`.
|
||||
|
|
|
|||
|
|
@ -1,11 +1,17 @@
|
|||
codeunit 50218 "Privacy Sample Register Integration"
|
||||
{
|
||||
[EventSubscriber(ObjectType::Codeunit, Codeunit::"Privacy Notice Registrations", 'OnRegisterPrivacyNotices', '', false, false)]
|
||||
local procedure OnRegisterPrivacyNotices(var TempPrivacyNotice: Record "Privacy Notice" temporary)
|
||||
var
|
||||
PrivacyNotice: Codeunit "Privacy Notice";
|
||||
ExternalSyncNoticeIdLbl: Label 'CONTOSO-EXTERNAL-SYNC', Locked = true;
|
||||
ExternalSyncNameLbl: Label 'Contoso External Sync', Locked = true;
|
||||
PrivacyTermsUrlLbl: Label 'https://contoso.example/privacy', Locked = true;
|
||||
|
||||
[EventSubscriber(ObjectType::Codeunit, Codeunit::"Privacy Notice", 'OnRegisterPrivacyNotices', '', false, false)]
|
||||
local procedure OnRegisterPrivacyNotices(var TempPrivacyNotice: Record "Privacy Notice" temporary)
|
||||
begin
|
||||
PrivacyNotice.CreatePrivacyNoticeForIntegration(
|
||||
'My External Sync', 'External Customer Sync Service');
|
||||
TempPrivacyNotice.Init();
|
||||
TempPrivacyNotice.ID := ExternalSyncNoticeIdLbl;
|
||||
TempPrivacyNotice."Integration Service Name" := ExternalSyncNameLbl;
|
||||
TempPrivacyNotice.Link := PrivacyTermsUrlLbl;
|
||||
if not TempPrivacyNotice.Insert() then;
|
||||
end;
|
||||
}
|
||||
|
|
|
|||
|
|
@ -1,24 +1,24 @@
|
|||
---
|
||||
bc-version: [all]
|
||||
domain: privacy
|
||||
keywords: [privacy-notice-registrations, integration, register, exchange, onedrive, teams, notice-id]
|
||||
keywords: [privacy-notice, integration, register, onregisterprivacynotices, notice-id]
|
||||
technologies: [al]
|
||||
countries: [w1]
|
||||
application-area: [all]
|
||||
---
|
||||
|
||||
# Register every new external integration with `Privacy Notice Registrations`
|
||||
# Register custom integrations with Codeunit Privacy Notice
|
||||
|
||||
## Description
|
||||
|
||||
`Codeunit "Privacy Notice Registrations"` is the registry of integrations whose consent state the platform tracks. Built-in integrations such as Exchange, OneDrive and Teams already have notice IDs exposed via accessor methods on this codeunit (`GetExchangePrivacyNoticeId`, etc.); a new integration introduced by an extension must add itself to the registry so that the admin can grant or withhold consent on the **Privacy Notices Status** page. Without registration, there is nothing for `Codeunit "Privacy Notice"` to return an approval state for — the call cannot meaningfully gate the outbound request.
|
||||
The current extension point is `Codeunit "Privacy Notice"`. Extensions can subscribe to its `OnRegisterPrivacyNotices` event and add a dedicated notice ID, integration name, and link to the temporary `Privacy Notice` record. For explicit creation outside the default-registration flow, the same codeunit exposes `CreatePrivacyNotice`. `Codeunit "Privacy Notice Registrations"` contains IDs for built-in integrations and is not the registration API for a custom service.
|
||||
|
||||
## Best Practice
|
||||
|
||||
When introducing a new outbound integration: pick a stable notice ID, register it via `Privacy Notice Registrations`, and then gate every outbound call with `PrivacyNotice.GetPrivacyNoticeApprovalState(<that id>)` as described in `privacy-notice-consent-for-external-data-transfer.md`.
|
||||
Choose a stable ID owned by the extension. Register it through `OnRegisterPrivacyNotices`, or call `PrivacyNotice.CreatePrivacyNotice` during an intentional setup or upgrade path. Use that same ID for consent checks described in `privacy-notice-consent-for-external-data-transfer.md`.
|
||||
|
||||
See sample: `register-integration-in-privacy-notice-registrations.good.al`.
|
||||
|
||||
## Anti Pattern
|
||||
|
||||
Shipping a new outbound integration without registering it. Even if the code calls `GetPrivacyNoticeApprovalState`, the admin has no surface to express consent — the integration is effectively unmanaged from a privacy-notice standpoint.
|
||||
Reusing the Exchange or another built-in notice ID for a custom integration, subscribing to `Privacy Notice Registrations`, or calling the nonexistent `CreatePrivacyNoticeForIntegration` method. These shapes attach consent to the wrong service or do not compile.
|
||||
|
|
|
|||
|
|
@ -1,12 +1,19 @@
|
|||
table 50202 "System Configuration Log"
|
||||
{
|
||||
DataClassification = SystemMetadata;
|
||||
|
||||
fields
|
||||
{
|
||||
field(1; "Entry No."; Integer) { }
|
||||
field(2; "Changed By"; Code[50]) { }
|
||||
field(3; "Change Description"; Text[250]) { }
|
||||
field(1; "Entry No."; Integer)
|
||||
{
|
||||
DataClassification = SystemMetadata;
|
||||
}
|
||||
field(2; "Changed By"; Code[50])
|
||||
{
|
||||
DataClassification = EndUserIdentifiableInformation;
|
||||
}
|
||||
field(3; "Change Description"; Text[250])
|
||||
{
|
||||
DataClassification = CustomerContent;
|
||||
}
|
||||
}
|
||||
|
||||
keys
|
||||
|
|
|
|||
|
|
@ -1,24 +1,24 @@
|
|||
---
|
||||
bc-version: [all]
|
||||
domain: privacy
|
||||
keywords: [data-classification, table-level, inheritance, override, cascading]
|
||||
keywords: [data-classification, table-level, normal-field, appsourcecop, as0016]
|
||||
technologies: [al]
|
||||
countries: [w1]
|
||||
application-area: [all]
|
||||
---
|
||||
|
||||
# Table-level DataClassification cascades to every field unless overridden
|
||||
# Set DataClassification on every Normal table field
|
||||
|
||||
## Description
|
||||
|
||||
`DataClassification` may be set at the table level. When it is, every field in the table inherits that classification and individual fields do not need their own `DataClassification` property. The cascade is the platform's intended way of classifying tables whose fields are homogeneous — for example, a system configuration log whose every column is `SystemMetadata`. A field only needs its own classification when its content genuinely differs from the table's default and the inherited value would be wrong.
|
||||
AppSourceCop AS0016 requires every field whose `FieldClass` is `Normal` to declare `DataClassification` and use a value other than `ToBeClassified`. A table-level `DataClassification` property does not satisfy that field-level requirement. FlowFields and FlowFilters are handled separately by the platform and are covered by `flowfield-flowfilter-classification-systemmetadata.md`.
|
||||
|
||||
## Best Practice
|
||||
|
||||
Set `DataClassification` once at the table level whenever every field in the table shares the same classification. Omit field-level `DataClassification` properties in that case. Override only on the specific fields whose data class differs from the table's — for example, a `SystemMetadata` audit table that nonetheless captures a `CustomerContent` value somewhere.
|
||||
Classify each Normal field according to the data it stores, even when every field in the table has the same classification. Repeat the property explicitly so AS0016 can verify every field.
|
||||
|
||||
See sample: `table-level-data-classification-cascades.good.al`.
|
||||
|
||||
## Anti Pattern
|
||||
|
||||
Flagging individual fields for "missing `DataClassification`" when the table declares one — the inheritance is the correct, intentional pattern. The mirror anti-pattern is repeating the same `DataClassification` on every field of a table that already declares it at the table level; the property is redundant and adds nothing the platform did not already know.
|
||||
Relying on `DataClassification` at table scope and leaving Normal fields unclassified. The table property does not cascade in the way AS0016 requires, so the fields still fail AppSourceCop validation.
|
||||
|
|
|
|||
|
|
@ -8,7 +8,7 @@ codeunit 50215 "Sec Sample IsoStorage Good"
|
|||
exit(true);
|
||||
end;
|
||||
|
||||
internal procedure SetApiKey(NewKey: Text)
|
||||
internal procedure SetApiKey(NewKey: SecretText)
|
||||
begin
|
||||
IsolatedStorage.SetEncrypted('ApiKey', NewKey, DataScope::Module);
|
||||
end;
|
||||
|
|
|
|||
|
|
@ -1,5 +1,5 @@
|
|||
---
|
||||
bc-version: [all]
|
||||
bc-version: [24..]
|
||||
domain: security
|
||||
keywords: [isolatedstorage, local, internal, public, getter, setter, encapsulation]
|
||||
technologies: [al]
|
||||
|
|
|
|||
|
|
@ -1,6 +1,6 @@
|
|||
codeunit 50220 "Sec Sample DataScope Bad"
|
||||
{
|
||||
internal procedure StoreCompanyWebhook(WebhookUrl: Text)
|
||||
internal procedure StoreCompanyWebhook(WebhookUrl: SecretText)
|
||||
begin
|
||||
IsolatedStorage.SetEncrypted('WebhookUrl', WebhookUrl, DataScope::Module);
|
||||
end;
|
||||
|
|
|
|||
|
|
@ -1,11 +1,11 @@
|
|||
codeunit 50219 "Sec Sample DataScope Good"
|
||||
{
|
||||
internal procedure StoreTenantApiKey(ApiKey: Text)
|
||||
internal procedure StoreTenantApiKey(ApiKey: SecretText)
|
||||
begin
|
||||
IsolatedStorage.SetEncrypted('TenantApiKey', ApiKey, DataScope::Module);
|
||||
end;
|
||||
|
||||
internal procedure StoreCompanyWebhook(WebhookUrl: Text)
|
||||
internal procedure StoreCompanyWebhook(WebhookUrl: SecretText)
|
||||
begin
|
||||
IsolatedStorage.SetEncrypted('WebhookUrl', WebhookUrl, DataScope::Company);
|
||||
end;
|
||||
|
|
|
|||
|
|
@ -1,5 +1,5 @@
|
|||
---
|
||||
bc-version: [all]
|
||||
bc-version: [24..]
|
||||
domain: security
|
||||
keywords: [isolatedstorage, datascope, module, company, user, scope]
|
||||
technologies: [al]
|
||||
|
|
|
|||
|
|
@ -1,10 +1,11 @@
|
|||
codeunit 50217 "Sec Sample SetEncrypted Good"
|
||||
{
|
||||
internal procedure StoreApiKey(ApiKeyValue: Text)
|
||||
internal procedure StoreApiKey(ApiKeyValue: SecretText)
|
||||
var
|
||||
StoreApiKeyFailedErr: Label 'The API key could not be stored.';
|
||||
begin
|
||||
if StrLen(ApiKeyValue) > 200 then
|
||||
Error('API key too long for encrypted storage');
|
||||
IsolatedStorage.SetEncrypted('ApiKey', ApiKeyValue, DataScope::Module);
|
||||
if not IsolatedStorage.SetEncrypted('ApiKey', ApiKeyValue, DataScope::Module) then
|
||||
Error(StoreApiKeyFailedErr);
|
||||
end;
|
||||
|
||||
local procedure ReadApiKey(var ApiKey: SecretText): Boolean
|
||||
|
|
|
|||
|
|
@ -1,5 +1,5 @@
|
|||
---
|
||||
bc-version: [all]
|
||||
bc-version: [24..]
|
||||
domain: security
|
||||
keywords: [isolatedstorage, setencrypted, encryption, secret, storage]
|
||||
technologies: [al]
|
||||
|
|
@ -15,7 +15,7 @@ application-area: [all]
|
|||
|
||||
## Best Practice
|
||||
|
||||
Use `IsolatedStorage.SetEncrypted` for every value that meets the definition of a secret. Pair it with the matching retrieval pattern: `IsolatedStorage.Contains` to test for presence and `IsolatedStorage.Get` (preferably with a `SecretText` destination) to read. Constrain the input length before storing — long values can exceed the encrypted-storage size limit and the write will fail at runtime. See sample: `isolatedstorage-setencrypted-for-sensitive-values.good.al`.
|
||||
Use the `SecretText` overloads of `IsolatedStorage.SetEncrypted` and `IsolatedStorage.Get` for values that meet the definition of a secret. Check the optional Boolean result when storage failure needs a controlled error; encrypted values are subject to the documented storage-size limit. See sample: `isolatedstorage-setencrypted-for-sensitive-values.good.al`.
|
||||
|
||||
## Anti Pattern
|
||||
|
||||
|
|
|
|||
|
|
@ -1,19 +1,14 @@
|
|||
codeunit 50214 "Sec Sample NonDebug Bad"
|
||||
{
|
||||
procedure BuildConnectionString(ApiKey: SecretText): Text
|
||||
procedure CallLegacyOnPremisesConsumer(ApiKey: SecretText)
|
||||
var
|
||||
PlainApiKey: Text;
|
||||
begin
|
||||
exit('Server=db.example.com;Key=' + ApiKey.Unwrap());
|
||||
PlainApiKey := ApiKey.Unwrap();
|
||||
InvokeLegacyConsumer(PlainApiKey);
|
||||
end;
|
||||
|
||||
procedure ParseSessionToken(Response: HttpResponseMessage; var SessionToken: SecretText)
|
||||
var
|
||||
ResponseText: Text;
|
||||
JsonObject: JsonObject;
|
||||
JsonToken: JsonToken;
|
||||
local procedure InvokeLegacyConsumer(ApiKey: Text)
|
||||
begin
|
||||
Response.Content.ReadAs(ResponseText);
|
||||
JsonObject.ReadFrom(ResponseText);
|
||||
JsonObject.Get('access_token', JsonToken);
|
||||
SessionToken := JsonToken.AsValue().AsText();
|
||||
end;
|
||||
}
|
||||
|
|
|
|||
|
|
@ -1,21 +1,17 @@
|
|||
codeunit 50213 "Sec Sample NonDebug Good"
|
||||
{
|
||||
[NonDebuggable]
|
||||
procedure BuildConnectionString(ApiKey: SecretText): Text
|
||||
procedure CallLegacyOnPremisesConsumer(ApiKey: SecretText)
|
||||
var
|
||||
PlainApiKey: Text;
|
||||
begin
|
||||
exit('Server=db.example.com;Key=' + ApiKey.Unwrap());
|
||||
PlainApiKey := ApiKey.Unwrap();
|
||||
InvokeLegacyConsumer(PlainApiKey);
|
||||
end;
|
||||
|
||||
[NonDebuggable]
|
||||
procedure ParseSessionToken(Response: HttpResponseMessage; var SessionToken: SecretText)
|
||||
var
|
||||
ResponseText: Text;
|
||||
JsonObject: JsonObject;
|
||||
JsonToken: JsonToken;
|
||||
local procedure InvokeLegacyConsumer(ApiKey: Text)
|
||||
begin
|
||||
Response.Content.ReadAs(ResponseText);
|
||||
JsonObject.ReadFrom(ResponseText);
|
||||
JsonObject.Get('access_token', JsonToken);
|
||||
SessionToken := JsonToken.AsValue().AsText();
|
||||
// The on-premises legacy consumer accepts only Text.
|
||||
end;
|
||||
}
|
||||
|
|
|
|||
|
|
@ -1,5 +1,5 @@
|
|||
---
|
||||
bc-version: [all]
|
||||
bc-version: [23..]
|
||||
domain: security
|
||||
keywords: [nondebuggable, attribute, secrettext, unwrap, debugger]
|
||||
technologies: [al]
|
||||
|
|
@ -7,16 +7,16 @@ countries: [w1]
|
|||
application-area: [all]
|
||||
---
|
||||
|
||||
# Mark procedures that call SecretText.Unwrap() as [NonDebuggable]
|
||||
# On-premises only: protect unavoidable SecretText.Unwrap calls
|
||||
|
||||
## Description
|
||||
|
||||
`SecretText` transit — assignment, parameter passing, and return values — is auto-protected: the debugger sees a redacted placeholder, not the value. The protection ends the moment code calls `.Unwrap()`, which converts the `SecretText` back to plain `Text`. From that point on, the local variable holding the result is visible in the debugger like any other `Text`. The `[NonDebuggable]` attribute marks a procedure so that none of its locals or parameters are visible to the debugger during execution, which is exactly what is needed for any procedure that performs an `Unwrap()` or that otherwise materializes a secret as `Text` (for example, while parsing a JSON response to extract an access token).
|
||||
`SecretText.Unwrap()` is supported only for Business Central on-premises and exists for compatibility. It converts a protected value to plain `Text`, where debugger redaction no longer applies. `[NonDebuggable]` prevents the debugger from inspecting a procedure's parameters and locals, but it does not make the resulting `Text` safe to return, log, or pass through debuggable code.
|
||||
|
||||
## Best Practice
|
||||
|
||||
Apply `[NonDebuggable]` to any procedure whose body calls `.Unwrap()` on a `SecretText`, and to any procedure that constructs a `SecretText` from a `Text` source (such as a procedure that reads a JSON response body and converts the resulting `Text` into a `SecretText` for the caller). Keep the unwrap window as small as possible — ideally a single one-line helper that hands the unwrapped value straight to the consuming API. See sample: `nondebuggable-required-when-unwrapping-secrettext.good.al`.
|
||||
In SaaS, keep the value as `SecretText` and use secret-aware APIs instead of unwrapping. For an unavoidable on-premises legacy API that accepts only `Text`, keep the plain-text path as short as possible and mark every procedure in that path `[NonDebuggable]`. Do not return the unwrapped value. See sample: `nondebuggable-required-when-unwrapping-secrettext.good.al`.
|
||||
|
||||
## Anti Pattern
|
||||
|
||||
Calling `ApiKey.Unwrap()` inside a procedure that is not marked `[NonDebuggable]`. The unwrapped value is now an ordinary `Text` local and the debugger will display it, defeating the purpose of using `SecretText` in the first place. Reviewers should flag any `Unwrap()` call in a procedure that lacks the attribute, and any procedure that parses a credential out of a response (`access_token`, `id_token`, `client_secret`) without the attribute. See sample: `nondebuggable-required-when-unwrapping-secrettext.bad.al`.
|
||||
Calling `Unwrap()` in cloud-targeted code, or calling it in an on-premises procedure that is debuggable or returns the resulting `Text`. Both defeat the protection that `SecretText` provides. See sample: `nondebuggable-required-when-unwrapping-secrettext.bad.al`.
|
||||
|
|
|
|||
|
|
@ -1,12 +1,17 @@
|
|||
codeunit 50212 "Sec Sample SecretSubst Bad"
|
||||
{
|
||||
procedure BuildAuthHeader(Token: SecretText): Text
|
||||
procedure BuildAuthHeader(Token: Text): Text
|
||||
begin
|
||||
exit(StrSubstNo('Bearer %1', Token.Unwrap()));
|
||||
exit(StrSubstNo('Bearer %1', Token));
|
||||
end;
|
||||
|
||||
procedure BuildSecretUri(BaseUrl: Text; ApiKey: SecretText): Text
|
||||
procedure BuildSecretUri(ApiKey: Text): Text
|
||||
begin
|
||||
exit(BaseUrl + '?key=' + ApiKey.Unwrap());
|
||||
exit(StrSubstNo('https://api.example.com/data?key=%1', ApiKey));
|
||||
end;
|
||||
|
||||
procedure BuildBrokenAuthHeader(Token: SecretText): SecretText
|
||||
begin
|
||||
exit(SecretStrSubstNo('Bearer', Token));
|
||||
end;
|
||||
}
|
||||
|
|
|
|||
|
|
@ -5,8 +5,8 @@ codeunit 50211 "Sec Sample SecretSubst Good"
|
|||
exit(SecretStrSubstNo('Bearer %1', Token));
|
||||
end;
|
||||
|
||||
procedure BuildSecretUri(BaseUrl: Text; ApiKey: SecretText): SecretText
|
||||
procedure BuildSecretUri(ApiKey: SecretText): SecretText
|
||||
begin
|
||||
exit(SecretStrSubstNo('%1?key=%2', BaseUrl, ApiKey));
|
||||
exit(SecretStrSubstNo('https://api.example.com/data?key=%1', ApiKey));
|
||||
end;
|
||||
}
|
||||
|
|
|
|||
|
|
@ -1,5 +1,5 @@
|
|||
---
|
||||
bc-version: [all]
|
||||
bc-version: [23..]
|
||||
domain: security
|
||||
keywords: [secretstrsubstno, secrettext, strsubstno, format, compose]
|
||||
technologies: [al]
|
||||
|
|
@ -11,12 +11,12 @@ application-area: [all]
|
|||
|
||||
## Description
|
||||
|
||||
`SecretStrSubstNo` is the secret-preserving counterpart of `StrSubstNo`. It accepts a format string and arguments (any of which may be `SecretText`) and returns a `SecretText` — the substitution happens without ever materializing the result as plain `Text`. It is the right tool whenever a secret needs to be embedded in a larger string: an `Authorization: Bearer <token>` header value, a URI that includes an API key as a query parameter, or any other interpolation that combines a `SecretText` with surrounding context.
|
||||
`SecretStrSubstNo` is the secret-preserving counterpart of `StrSubstNo`. It inserts `SecretText` arguments into `%1`, `%2`, and similar placeholders and returns `SecretText` without materializing the result as plain text. It is the right tool for values such as a `Bearer %1` authorization header or a URI with an API key placeholder.
|
||||
|
||||
## Best Practice
|
||||
|
||||
Compose every secret-bearing string through `SecretStrSubstNo` and keep the result as `SecretText` end-to-end. Pass the result to the `SecretText` overload of the consumer — `HttpClient.SetSecretRequestUri`, `HttpHeaders.Add`, or `HttpContent.WriteFrom`. See sample: `secretstrsubstno-for-composing-secrets.good.al`.
|
||||
Compose every secret-bearing string through `SecretStrSubstNo`, ensure the format contains a placeholder for each secret, and keep the result as `SecretText`. Pass it to `HttpRequestMessage.SetSecretRequestUri`, `HttpHeaders.Add`, or `HttpContent.WriteFrom`. See sample: `secretstrsubstno-for-composing-secrets.good.al`.
|
||||
|
||||
## Anti Pattern
|
||||
|
||||
Calling `StrSubstNo('Bearer %1', Token.Unwrap())` to build the header value, or concatenating `'Bearer ' + Token.Unwrap()`. Both produce a plain `Text` containing the secret, which is then visible in the debugger and in any subsequent log or trace. Reviewers should flag any `Unwrap()` whose result is fed into `StrSubstNo` or used in `+` concatenation — `SecretStrSubstNo` removes the need for either. See sample: `secretstrsubstno-for-composing-secrets.bad.al`.
|
||||
Keeping a credential in `Text` and inserting it with `StrSubstNo`, or calling `SecretStrSubstNo` with a format that has no placeholder for the secret. The first exposes the value as plain text; the second silently omits it. See sample: `secretstrsubstno-for-composing-secrets.bad.al`.
|
||||
|
|
|
|||
|
|
@ -1,14 +1,11 @@
|
|||
codeunit 50207 "Sec Sample SecretText Good"
|
||||
{
|
||||
procedure CallExternalApi()
|
||||
procedure CallExternalApi(ApiKey: SecretText)
|
||||
var
|
||||
ApiKey: SecretText;
|
||||
HttpClient: HttpClient;
|
||||
Response: HttpResponseMessage;
|
||||
Headers: HttpHeaders;
|
||||
begin
|
||||
if IsolatedStorage.Contains('ApiKey', DataScope::Module) then
|
||||
IsolatedStorage.Get('ApiKey', DataScope::Module, ApiKey);
|
||||
Headers := HttpClient.DefaultRequestHeaders();
|
||||
Headers.Add('X-Api-Key', ApiKey);
|
||||
HttpClient.Get('https://api.example.com/data', Response);
|
||||
|
|
|
|||
|
|
@ -1,5 +1,5 @@
|
|||
---
|
||||
bc-version: [all]
|
||||
bc-version: [23..]
|
||||
domain: security
|
||||
keywords: [secrettext, credentials, api-key, token, debugger, unwrap]
|
||||
technologies: [al]
|
||||
|
|
@ -15,7 +15,7 @@ application-area: [all]
|
|||
|
||||
## Best Practice
|
||||
|
||||
Declare credential-carrying parameters and variables as `SecretText` from the call site that retrieves the secret all the way to the call site that consumes it (typically an `HttpClient` header or URI). Never round-trip through `Text` — every conversion is a potential exposure point. Retrieve secrets from `IsolatedStorage` with the `SecretText` overload of `Get` rather than the `Text` overload. See sample: `secrettext-for-credentials.good.al`.
|
||||
Declare credential-carrying parameters and variables as `SecretText` from the call site that retrieves the secret all the way to the call site that consumes it (typically an HTTP header or URI). Never round-trip through `Text`. On BC 24 and later, use the `SecretText` overload of `IsolatedStorage.Get` when retrieving stored secrets. See sample: `secrettext-for-credentials.good.al`.
|
||||
|
||||
## Anti Pattern
|
||||
|
||||
|
|
|
|||
|
|
@ -1,23 +1,23 @@
|
|||
codeunit 50210 "Sec Sample SecretHttp Bad"
|
||||
{
|
||||
procedure CallApiWithSecretInUri(ApiKey: SecretText)
|
||||
procedure CallApiWithSecretInUri(ApiKey: Text)
|
||||
var
|
||||
HttpClient: HttpClient;
|
||||
Response: HttpResponseMessage;
|
||||
RequestUri: Text;
|
||||
begin
|
||||
RequestUri := 'https://api.example.com/data?key=' + ApiKey.Unwrap();
|
||||
RequestUri := StrSubstNo('https://api.example.com/data?key=%1', ApiKey);
|
||||
HttpClient.Get(RequestUri, Response);
|
||||
end;
|
||||
|
||||
procedure CallApiWithBearer(BearerToken: SecretText)
|
||||
procedure CallApiWithBearer(BearerToken: Text)
|
||||
var
|
||||
HttpClient: HttpClient;
|
||||
Response: HttpResponseMessage;
|
||||
Headers: HttpHeaders;
|
||||
begin
|
||||
Headers := HttpClient.DefaultRequestHeaders();
|
||||
Headers.Add('Authorization', 'Bearer ' + BearerToken.Unwrap());
|
||||
Headers.Add('Authorization', StrSubstNo('Bearer %1', BearerToken));
|
||||
HttpClient.Get('https://api.example.com/data', Response);
|
||||
end;
|
||||
}
|
||||
|
|
|
|||
|
|
@ -3,26 +3,32 @@ codeunit 50209 "Sec Sample SecretHttp Good"
|
|||
procedure CallApiWithSecretUri(ApiKey: SecretText)
|
||||
var
|
||||
HttpClient: HttpClient;
|
||||
Request: HttpRequestMessage;
|
||||
Response: HttpResponseMessage;
|
||||
SecretUri: SecretText;
|
||||
begin
|
||||
SecretUri := SecretStrSubstNo('https://api.example.com/data?key=%1', ApiKey);
|
||||
HttpClient.SetSecretRequestUri(SecretUri);
|
||||
HttpClient.Get('', Response);
|
||||
Request.Method := 'GET';
|
||||
Request.SetSecretRequestUri(SecretUri);
|
||||
HttpClient.Send(Request, Response);
|
||||
end;
|
||||
|
||||
procedure CallApiWithBearer(BearerToken: SecretText)
|
||||
var
|
||||
HttpClient: HttpClient;
|
||||
Request: HttpRequestMessage;
|
||||
Response: HttpResponseMessage;
|
||||
Headers: HttpHeaders;
|
||||
AuthHeader: SecretText;
|
||||
AuthorizationHeaderMissingErr: Label 'Authorization header missing.';
|
||||
begin
|
||||
Request.Method := 'GET';
|
||||
Request.SetRequestUri('https://api.example.com/data');
|
||||
Request.GetHeaders(Headers);
|
||||
AuthHeader := SecretStrSubstNo('Bearer %1', BearerToken);
|
||||
Headers := HttpClient.DefaultRequestHeaders();
|
||||
Headers.Add('Authorization', AuthHeader);
|
||||
if not Headers.ContainsSecret('Authorization') then
|
||||
Error('Authorization header missing');
|
||||
HttpClient.Get('https://api.example.com/data', Response);
|
||||
Error(AuthorizationHeaderMissingErr);
|
||||
HttpClient.Send(Request, Response);
|
||||
end;
|
||||
}
|
||||
|
|
|
|||
|
|
@ -1,5 +1,5 @@
|
|||
---
|
||||
bc-version: [all]
|
||||
bc-version: [23..]
|
||||
domain: security
|
||||
keywords: [secrettext, httpclient, setsecretrequesturi, containssecret, headers, http]
|
||||
technologies: [al]
|
||||
|
|
@ -7,16 +7,16 @@ countries: [w1]
|
|||
application-area: [all]
|
||||
---
|
||||
|
||||
# Use the SecretText-aware HttpClient surface for secrets in requests
|
||||
# Set secret request URIs on HttpRequestMessage
|
||||
|
||||
## Description
|
||||
|
||||
`HttpClient` and its companion types expose a parallel surface that accepts `SecretText` instead of `Text`, so that secret URIs, secret headers, and secret request bodies never round-trip through plain text. The key entry points are: `HttpClient.SetSecretRequestUri()` for URIs that contain secrets (the subsequent `Get`/`Post` is then called with an empty string); `HttpHeaders.Add()` overload that accepts a `SecretText` value for authorization headers; `HttpHeaders.ContainsSecret()` to test whether a secret header is present (the plain `Contains()` returns false for secret headers); `HttpContent.WriteFrom()` and `HttpContent.ReadAs()` overloads that accept and produce `SecretText` for request and response bodies that carry credentials.
|
||||
The secret URI API belongs to `HttpRequestMessage`, not `HttpClient`. `HttpRequestMessage.SetSecretRequestUri(SecretText)` keeps a credential-bearing URI protected, and the prepared request is sent with `HttpClient.Send`. Companion APIs also accept `SecretText`, including `HttpHeaders.Add` for authorization headers and `HttpContent.WriteFrom` for secret request bodies.
|
||||
|
||||
## Best Practice
|
||||
|
||||
When the URI contains a secret query parameter, compose it as `SecretText` (see `secretstrsubstno-for-composing-secrets.md`), pass it to `SetSecretRequestUri`, and call `Get('', Response)` with an empty string as the URI argument. When the credential is an authorization header, build the header value as `SecretText` and pass it to `Headers.Add`. Use `ContainsSecret` rather than `Contains` to check for the presence of a secret header. See sample: `secrettext-with-httpclient.good.al`.
|
||||
Compose a secret URI with `SecretStrSubstNo`, call `Request.SetSecretRequestUri(SecretUri)`, set the request method, and send the request with `HttpClient.Send(Request, Response)`. For authorization, get the request headers, add a `SecretText` value, and use `ContainsSecret` when checking for that header. See sample: `secrettext-with-httpclient.good.al`.
|
||||
|
||||
## Anti Pattern
|
||||
|
||||
Calling `ApiKey.Unwrap()` to build a URI or header string and passing the resulting `Text` to `HttpClient.Get` or `Headers.Add`. The unwrapped secret is now visible in the debugger, in any HTTP trace that captures the request URI, and in any error that includes the URI. Reviewers should flag any `Unwrap()` call whose result flows into an `HttpClient` argument; the `SecretText` overload exists precisely so the unwrap is not needed. See sample: `secrettext-with-httpclient.bad.al`.
|
||||
Holding a credential in `Text`, interpolating it with `StrSubstNo` or concatenation, and passing that plain text to `HttpClient.Get` or `HttpHeaders.Add`. The secret-aware request and header APIs remove the need to materialize the value as `Text`. See sample: `secrettext-with-httpclient.bad.al`.
|
||||
|
|
|
|||
|
|
@ -2,24 +2,21 @@ tableextension 50223 "Sec Sample VTR Good" extends Customer
|
|||
{
|
||||
fields
|
||||
{
|
||||
field(50223; "System Batch ID"; Code[20])
|
||||
{
|
||||
TableRelation = "Sales Header"."No.";
|
||||
ValidateTableRelation = false;
|
||||
Editable = false;
|
||||
}
|
||||
field(50224; "External Customer Ref"; Code[50])
|
||||
field(50223; "External Customer Ref"; Code[50])
|
||||
{
|
||||
TableRelation = Customer."No.";
|
||||
ValidateTableRelation = false;
|
||||
TestTableRelation = false;
|
||||
|
||||
trigger OnValidate()
|
||||
var
|
||||
Customer: Record Customer;
|
||||
InvalidExternalReferenceErr: Label 'The external customer reference must not contain spaces.';
|
||||
begin
|
||||
if "External Customer Ref" = '' then
|
||||
exit;
|
||||
if not Customer.Get("External Customer Ref") then
|
||||
Error('External customer reference %1 does not exist.', "External Customer Ref");
|
||||
"External Customer Ref" := CopyStr(
|
||||
UpperCase(DelChr("External Customer Ref", '<>', ' ')),
|
||||
1, MaxStrLen("External Customer Ref"));
|
||||
if StrPos("External Customer Ref", ' ') > 0 then
|
||||
Error(InvalidExternalReferenceErr);
|
||||
end;
|
||||
}
|
||||
}
|
||||
|
|
|
|||
|
|
@ -7,16 +7,16 @@ countries: [w1]
|
|||
application-area: [all]
|
||||
---
|
||||
|
||||
# Do not set ValidateTableRelation = false on user-editable fields
|
||||
# Handle free-form input when ValidateTableRelation is false
|
||||
|
||||
## Description
|
||||
|
||||
`TableRelation` on a field declares that the field's value must exist in another table; the platform validates the value on entry and on `Validate`. Setting `ValidateTableRelation = false` keeps the relation as metadata (used by lookups, by Edit-in-Excel, by APIs) but turns off the runtime check. On a system-controlled, non-editable field that is populated only by the platform or by a posting routine, that is acceptable. On a user-editable field, it is dangerous: users can type any value, and downstream code that assumes the relation holds will read a `Customer` record that does not exist, post to an account that was deleted, or join against missing rows.
|
||||
`ValidateTableRelation = false` intentionally lets a user keep free-form input even when it does not match `TableRelation`. This is supported for scenarios such as accepting a new vendor name and handling it in `OnValidate`. The risk is not the property itself; it is leaving downstream code to assume that every value identifies an existing related record.
|
||||
|
||||
## Best Practice
|
||||
|
||||
Leave `ValidateTableRelation` at its default (true) on any field a user can edit. If there is a legitimate reason to turn it off — typically because the relation is not on the primary key, or because the relation is computed — replace it with an `OnValidate` trigger that performs the equivalent check (`if FieldValue <> '' then VerifyExternalReferenceExists(FieldValue)`). Combine `ValidateTableRelation = false` with `Editable = false` for system-controlled fields, so the metadata is correct and the field is unreachable from the UI. See sample: `validatetablerelation-false-on-user-input.good.al`.
|
||||
Keep the default validation when values must exist in the related table. When free-form values are intentional, set both `ValidateTableRelation = false` and `TestTableRelation = false`, then add compensating `OnValidate` logic that normalizes, validates, creates, or otherwise handles unmatched input. Document that downstream code must not assume the relation exists. See sample: `validatetablerelation-false-on-user-input.good.al`.
|
||||
|
||||
## Anti Pattern
|
||||
|
||||
`ValidateTableRelation = false` on a user-facing input field (a `Customer No.` typed by a sales user) with no alternative validation. Reviewers should flag the combination of `ValidateTableRelation = false` and any of: `Editable = true` (the default), an `OnValidate` trigger that does not perform the relation check, or a page that surfaces the field as input. See sample: `validatetablerelation-false-on-user-input.bad.al`.
|
||||
`ValidateTableRelation = false` on a user-facing field with no intentional handling for unmatched values, or leaving `TestTableRelation = true` so database relation tests reject values the UI deliberately accepts. See sample: `validatetablerelation-false-on-user-input.bad.al`.
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue