mirror of
https://github.com/microsoft/BCQuality.git
synced 2026-08-06 09:26:52 +01:00
Promote security knowledge from community to Microsoft layer (#49)
* Promote security knowledge from community to Microsoft layer Pure git-mv relocation of the SECURITY domain from the community layer to the Microsoft layer. No content changes. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * Address review feedback on security knowledge promotion - do-not-grant-rights-beyond-a-users-entitlement.md: drop the See sample reference to a .good.al file that does not exist - Remove the 'Contributions welcome' boilerplate line from compose-permission-sets, prefer-oauth2, and protect-sensitive-data - protect-sensitive-data-in-temporary-tables: remove the pointless DeleteAll on the locally scoped temp buffer in the good sample and reword Best Practice to note local buffers are cleaned up automatically - Drop guard-bulk-operations-with-istemporary from the promotion; it stays in the community layer pending a decision on whether it is security Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> * Remove Contributions welcome boilerplate from do-not-grant article for consistency Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> --------- Co-authored-by: Jesper Schulz-Wedde <jesper.schulzwedde@microsoft.com> Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
This commit is contained in:
Jesper Schulz-Wedde
GitHub
parent
7a678d1aff
commit
bfda67a95a
13 changed files with 1 additions and 12 deletions
|
|
@ -1,28 +0,0 @@
|
|||
table 50100 "Customer Feedback"
|
||||
{
|
||||
fields
|
||||
{
|
||||
field(1; "Feedback No."; Code[20])
|
||||
{
|
||||
// No DataClassification declared. Defaults to ToBeClassified.
|
||||
}
|
||||
field(2; "Contact Name"; Text[100])
|
||||
{
|
||||
DataClassification = ToBeClassified;
|
||||
}
|
||||
field(3; "Email"; Text[80])
|
||||
{
|
||||
// Personal data classified as CustomerContent understates privacy impact.
|
||||
DataClassification = CustomerContent;
|
||||
}
|
||||
field(4; "Feedback Text"; Text[2048])
|
||||
{
|
||||
DataClassification = ToBeClassified;
|
||||
}
|
||||
}
|
||||
|
||||
keys
|
||||
{
|
||||
key(PK; "Feedback No.") { Clustered = true; }
|
||||
}
|
||||
}
|
||||
|
|
@ -1,36 +0,0 @@
|
|||
table 50100 "Customer Feedback"
|
||||
{
|
||||
fields
|
||||
{
|
||||
field(1; "Feedback No."; Code[20])
|
||||
{
|
||||
DataClassification = SystemMetadata;
|
||||
}
|
||||
field(2; "Contact Name"; Text[100])
|
||||
{
|
||||
DataClassification = EndUserIdentifiableInformation;
|
||||
}
|
||||
field(3; "Email"; Text[80])
|
||||
{
|
||||
DataClassification = EndUserIdentifiableInformation;
|
||||
}
|
||||
field(4; "Product Code"; Code[20])
|
||||
{
|
||||
DataClassification = CustomerContent;
|
||||
}
|
||||
field(5; "Feedback Text"; Text[2048])
|
||||
{
|
||||
// When uncertain between CustomerContent and EUII, prefer the stronger protection.
|
||||
DataClassification = EndUserIdentifiableInformation;
|
||||
}
|
||||
field(6; "Submitted DateTime"; DateTime)
|
||||
{
|
||||
DataClassification = SystemMetadata;
|
||||
}
|
||||
}
|
||||
|
||||
keys
|
||||
{
|
||||
key(PK; "Feedback No.") { Clustered = true; }
|
||||
}
|
||||
}
|
||||
|
|
@ -1,26 +0,0 @@
|
|||
---
|
||||
bc-version: [all]
|
||||
domain: security
|
||||
keywords: [dataclassification, gdpr, privacy, euii, compliance]
|
||||
technologies: [al]
|
||||
countries: [w1]
|
||||
application-area: [all]
|
||||
---
|
||||
|
||||
# Classify every field with DataClassification
|
||||
|
||||
## Description
|
||||
|
||||
Every field on every AL table and table extension must have a resolved `DataClassification` value, either declared directly on the field or inherited from a table-level default. The value drives GDPR tooling, data-subject requests, retention policies, and audit reporting — all of which rely on the field metadata to know what data to include, anonymize, or delete. A field with no field-level property and no table-level default resolves to `ToBeClassified`, which is a compliance gap, not a neutral state.
|
||||
|
||||
## Best Practice
|
||||
|
||||
Choose the narrowest value that accurately describes the field's content: `EndUserIdentifiableInformation` for data that directly identifies a person, `EndUserPseudonymousIdentifiers` for indirect identifiers, `CustomerContent` for business operational data, `SystemMetadata` for system-generated housekeeping, `AccountData` for tenant/billing, `OrganizationIdentifiableInformation` for organization-level identifiers. Use a table-level default for homogeneous tables, and override individual fields whose content differs from that default. When uncertain between two values, pick the stronger protection.
|
||||
|
||||
See sample: `classify-every-field-with-dataclassification.good.al`.
|
||||
|
||||
## Anti Pattern
|
||||
|
||||
Leaving `DataClassification = ToBeClassified` on a field, omitting classification when the table has no default, or relying on a table-level default that understates a field's actual content. Code in this state fails compliance audits and breaks the subject-access-request and retention tooling that depends on the property being set correctly.
|
||||
|
||||
See sample: `classify-every-field-with-dataclassification.bad.al`.
|
||||
|
|
@ -1,21 +0,0 @@
|
|||
// Two role-shaped sets, each re-enumerating the same objects. Adding a new
|
||||
// Sales table means editing both sets by hand; forgetting one creates a
|
||||
// subtle authorization bug where one role was updated and its sibling was not.
|
||||
|
||||
permissionset 50110 "Sales Order Processor"
|
||||
{
|
||||
Assignable = true;
|
||||
Permissions =
|
||||
tabledata Customer = IM,
|
||||
tabledata "Sales Header" = IMD,
|
||||
tabledata "Sales Line" = IMD;
|
||||
}
|
||||
|
||||
permissionset 50111 "Sales Viewer"
|
||||
{
|
||||
Assignable = true;
|
||||
Permissions =
|
||||
tabledata Customer = R,
|
||||
tabledata "Sales Header" = R,
|
||||
tabledata "Sales Line" = R;
|
||||
}
|
||||
|
|
@ -1,34 +0,0 @@
|
|||
// Building blocks: focused per-concern, marked Assignable = false so administrators
|
||||
// do not accidentally assign a fragment.
|
||||
permissionset 50100 "Sales Tables - Read"
|
||||
{
|
||||
Assignable = false;
|
||||
Permissions =
|
||||
tabledata Customer = R,
|
||||
tabledata "Sales Header" = R,
|
||||
tabledata "Sales Line" = R;
|
||||
}
|
||||
|
||||
permissionset 50101 "Sales Tables - Edit"
|
||||
{
|
||||
Assignable = false;
|
||||
IncludedPermissionSets = "Sales Tables - Read";
|
||||
Permissions =
|
||||
tabledata Customer = IM,
|
||||
tabledata "Sales Header" = IMD,
|
||||
tabledata "Sales Line" = IMD;
|
||||
}
|
||||
|
||||
// Role-shaped, Assignable = true, composed from building blocks.
|
||||
// Adding a new Sales table means editing one building block; both roles inherit the change.
|
||||
permissionset 50110 "Sales Order Processor"
|
||||
{
|
||||
Assignable = true;
|
||||
IncludedPermissionSets = "Sales Tables - Edit";
|
||||
}
|
||||
|
||||
permissionset 50111 "Sales Viewer"
|
||||
{
|
||||
Assignable = true;
|
||||
IncludedPermissionSets = "Sales Tables - Read";
|
||||
}
|
||||
|
|
@ -1,28 +0,0 @@
|
|||
---
|
||||
bc-version: [all]
|
||||
domain: security
|
||||
keywords: [permissionset, includedpermissionsets, assignable, composition, role]
|
||||
technologies: [al]
|
||||
countries: [w1]
|
||||
application-area: [all]
|
||||
---
|
||||
|
||||
# Compose permission sets with IncludedPermissionSets
|
||||
|
||||
> Contributions welcome — open a PR to refine or extend this article.
|
||||
|
||||
## Description
|
||||
|
||||
The `IncludedPermissionSets` property lets one AL permission set reference another, composing rights out of smaller building blocks. Combined with `Assignable = false` on the building blocks, an extension can ship focused per-module units (a table-data cluster, an API-access cluster) and assemble role-shaped sets that include them. Adding an object updates one building block, and every role-shaped set that includes it inherits the change automatically — instead of drifting apart across duplicated definitions.
|
||||
|
||||
## Best Practice
|
||||
|
||||
Break permission grants into small, focused building blocks, one per cohesive concern. Mark the building blocks `Assignable = false` so administrators do not accidentally assign a fragment. Build role-shaped, `Assignable = true` sets that reference the relevant building blocks through `IncludedPermissionSets`. When the extension grows, the structure absorbs the growth without duplicated edits.
|
||||
|
||||
See sample: `compose-permission-sets-with-included-sets.good.al`.
|
||||
|
||||
## Anti Pattern
|
||||
|
||||
Declaring several role-shaped permission sets that each re-enumerate the same object lists. Adding a new table means touching every set by hand; the sets drift apart over time, and subtle authorization bugs appear where one role was updated and a sibling role was not.
|
||||
|
||||
See sample: `compose-permission-sets-with-included-sets.bad.al`.
|
||||
|
|
@ -1,24 +0,0 @@
|
|||
---
|
||||
bc-version: [all]
|
||||
domain: security
|
||||
keywords: [entitlement, permissionset, license, clipping, sandbox-drift]
|
||||
technologies: [al]
|
||||
countries: [w1]
|
||||
application-area: [all]
|
||||
---
|
||||
|
||||
# Do not grant rights beyond a user's entitlement
|
||||
|
||||
> Contributions welcome — open a PR to refine or extend this article.
|
||||
|
||||
## Description
|
||||
|
||||
Entitlements are license-level caps on what a user can access, derived automatically from the BC license tier. Permission sets are application-level grants administered on top of the entitlement. A permission set can only grant within the entitlement's boundaries; grants beyond those boundaries are silently clipped at runtime. This means a permission set authored and validated in a developer sandbox (with a broad license) can appear to work correctly there and fail silently in a customer tenant where users hold a narrower entitlement.
|
||||
|
||||
## Best Practice
|
||||
|
||||
When designing a permission set that ships with an extension, consult the entitlement model for the target user population before finalizing the grants. Every object and tabledata right the set expects to grant should be reachable within the intended entitlement tier; if it is not, the set needs to be scoped to licenses that permit it, or the feature needs a different access path.
|
||||
|
||||
## Anti Pattern
|
||||
|
||||
Authoring permission sets in a sandbox with full-license context and shipping them without verifying which entitlement tier customer users actually hold. The sets look complete in test; on a real customer they silently lose rights at runtime and the symptom is "the feature does not work for some users" with no obvious authorization error.
|
||||
|
|
@ -1,29 +0,0 @@
|
|||
codeunit 50100 "Partner API Client"
|
||||
{
|
||||
procedure FetchOrders(var Response: Text): Boolean
|
||||
var
|
||||
HttpClient: HttpClient;
|
||||
HttpRequest: HttpRequestMessage;
|
||||
HttpResponse: HttpResponseMessage;
|
||||
ApiKey: Text;
|
||||
begin
|
||||
// API key stored as plain Text in a setup table - not IsolatedStorage,
|
||||
// not SecretText. Rotation means the admin editing a Text field;
|
||||
// a single disclosure exposes every tenant running this extension.
|
||||
ApiKey := GetApiKeyFromSetupTable();
|
||||
|
||||
HttpRequest.SetRequestUri('https://partner.example.com/orders');
|
||||
HttpRequest.Method('GET');
|
||||
HttpRequest.GetHeaders().Add('X-API-Key', ApiKey);
|
||||
|
||||
if not HttpClient.Send(HttpRequest, HttpResponse) then
|
||||
exit(false);
|
||||
|
||||
HttpResponse.Content.ReadAs(Response);
|
||||
exit(HttpResponse.IsSuccessStatusCode);
|
||||
end;
|
||||
|
||||
local procedure GetApiKeyFromSetupTable(): Text
|
||||
begin
|
||||
end;
|
||||
}
|
||||
|
|
@ -1,44 +0,0 @@
|
|||
codeunit 50100 "Partner API Client"
|
||||
{
|
||||
procedure FetchOrders(var Response: Text): Boolean
|
||||
var
|
||||
OAuth2: Codeunit OAuth2;
|
||||
HttpClient: HttpClient;
|
||||
HttpRequest: HttpRequestMessage;
|
||||
HttpResponse: HttpResponseMessage;
|
||||
AccessToken: SecretText;
|
||||
Scopes: List of [Text];
|
||||
begin
|
||||
Scopes.Add('https://partner.example.com/.default');
|
||||
|
||||
// Client-credentials flow for service-to-service. Tokens expire and rotate
|
||||
// on their own schedule; secret and client id are retrieved from IsolatedStorage.
|
||||
if not OAuth2.AcquireTokenWithClientCredentials(
|
||||
GetClientIdFromIsolatedStorage(),
|
||||
GetClientSecretFromIsolatedStorage(),
|
||||
'https://login.example.com/tenantid/oauth2/v2.0/token',
|
||||
'',
|
||||
Scopes,
|
||||
AccessToken)
|
||||
then
|
||||
exit(false);
|
||||
|
||||
HttpRequest.SetRequestUri('https://partner.example.com/orders');
|
||||
HttpRequest.Method('GET');
|
||||
HttpRequest.GetHeaders().Add('Authorization', SecretStrSubstNo('Bearer %1', AccessToken));
|
||||
|
||||
if not HttpClient.Send(HttpRequest, HttpResponse) then
|
||||
exit(false);
|
||||
|
||||
HttpResponse.Content.ReadAs(Response);
|
||||
exit(HttpResponse.IsSuccessStatusCode);
|
||||
end;
|
||||
|
||||
local procedure GetClientIdFromIsolatedStorage(): Text
|
||||
begin
|
||||
end;
|
||||
|
||||
local procedure GetClientSecretFromIsolatedStorage(): SecretText
|
||||
begin
|
||||
end;
|
||||
}
|
||||
|
|
@ -1,28 +0,0 @@
|
|||
---
|
||||
bc-version: [all]
|
||||
domain: security
|
||||
keywords: [oauth2, api-key, authentication, httpclient, token-refresh]
|
||||
technologies: [al]
|
||||
countries: [w1]
|
||||
application-area: [all]
|
||||
---
|
||||
|
||||
# Prefer OAuth2 over API keys for external HTTP calls
|
||||
|
||||
> Contributions welcome — open a PR to refine or extend this article.
|
||||
|
||||
## Description
|
||||
|
||||
External HTTP integrations from AL can authenticate using OAuth 2.0 (client-credentials for service-to-service, authorization-code for user-delegated), API keys, basic authentication, or credentials in URLs. The mechanisms differ substantially in the blast radius of a leaked secret and in how cleanly tokens can be rotated. OAuth-issued tokens expire on their own schedule and rotate cleanly; API keys and basic-auth passwords typically have to be rotated manually and usually live unencrypted in a configuration table. When the partner supports OAuth, the difference is a material security improvement, not a stylistic preference.
|
||||
|
||||
## Best Practice
|
||||
|
||||
When the partner supports OAuth, use the platform `OAuth2` codeunit (`AcquireTokenWithClientCredentials` for service-to-service, `AcquireAuthorizationCodeTokenFromCache` for user-delegated flows) rather than hand-rolled token acquisition. Carry tokens and client secrets as `SecretText`, persist them only in IsolatedStorage, and refresh tokens proactively — on a buffer before the documented expiry — so routine calls never block on a token refresh.
|
||||
|
||||
See sample: `prefer-oauth2-over-api-keys-for-external-http-calls.good.al`.
|
||||
|
||||
## Anti Pattern
|
||||
|
||||
Accepting an API-key or basic-auth integration because it is the first option documented, even when the partner supports OAuth. The shared secret usually ends up in a setup-table `Text` field, rotation becomes a manual operation that rarely happens, and a single disclosure exposes every tenant using the extension.
|
||||
|
||||
See sample: `prefer-oauth2-over-api-keys-for-external-http-calls.bad.al`.
|
||||
|
|
@ -1,27 +0,0 @@
|
|||
codeunit 50100 "Customer Temp Processor"
|
||||
{
|
||||
// Global temporary buffer - survives across procedure calls, carries values
|
||||
// to unrelated callers that may have no right to see them.
|
||||
var
|
||||
GlobalTempCustomer: Record Customer temporary;
|
||||
|
||||
procedure LoadCustomersForExport(FilterText: Text)
|
||||
var
|
||||
Customer: Record Customer;
|
||||
begin
|
||||
// No ReadPermission check before populating.
|
||||
Customer.SetFilter("No.", FilterText);
|
||||
if Customer.FindSet() then
|
||||
repeat
|
||||
GlobalTempCustomer := Customer;
|
||||
GlobalTempCustomer.Insert();
|
||||
until Customer.Next() = 0;
|
||||
|
||||
ExportBuffer();
|
||||
// No DeleteAll. Data remains in the global for the lifetime of the codeunit.
|
||||
end;
|
||||
|
||||
local procedure ExportBuffer()
|
||||
begin
|
||||
end;
|
||||
}
|
||||
|
|
@ -1,32 +0,0 @@
|
|||
codeunit 50100 "Customer Temp Processor"
|
||||
{
|
||||
procedure BuildScopedCustomerBuffer(CustomerNoFilter: Text): Boolean
|
||||
var
|
||||
Customer: Record Customer;
|
||||
TempCustomer: Record Customer temporary;
|
||||
begin
|
||||
// Validate the caller's permission before copying sensitive rows.
|
||||
if not Customer.ReadPermission() then
|
||||
exit(false);
|
||||
|
||||
Customer.SetFilter("No.", CustomerNoFilter);
|
||||
if not Customer.FindSet() then
|
||||
exit(true);
|
||||
|
||||
repeat
|
||||
TempCustomer := Customer;
|
||||
TempCustomer.Insert();
|
||||
until Customer.Next() = 0;
|
||||
|
||||
ProcessCustomerBuffer(TempCustomer);
|
||||
|
||||
// Explicit cleanup on the normal exit path.
|
||||
TempCustomer.DeleteAll();
|
||||
exit(true);
|
||||
end;
|
||||
|
||||
local procedure ProcessCustomerBuffer(var TempCustomer: Record Customer temporary)
|
||||
begin
|
||||
// Use the buffer in-place; do not persist values elsewhere.
|
||||
end;
|
||||
}
|
||||
|
|
@ -1,28 +0,0 @@
|
|||
---
|
||||
bc-version: [all]
|
||||
domain: security
|
||||
keywords: [temporary-table, data-protection, permission, cleanup]
|
||||
technologies: [al]
|
||||
countries: [w1]
|
||||
application-area: [all]
|
||||
---
|
||||
|
||||
# Protect sensitive data in temporary tables
|
||||
|
||||
> Contributions welcome — open a PR to refine or extend this article.
|
||||
|
||||
## Description
|
||||
|
||||
A temporary record copies data out of the source table into session memory. The platform does not automatically enforce the source table's permission model on the copy, and a value written to a temporary buffer can outlive the procedure that put it there if the buffer is a global or is passed upward. Code that places sensitive rows into a temporary table is therefore responsible for the checks and cleanup the source table would otherwise provide.
|
||||
|
||||
## Best Practice
|
||||
|
||||
Validate the caller's read permission on the source table before populating the temporary buffer. Keep the buffer's lifetime as short as the work requires, and delete its contents on every exit path — including error paths — so sensitive values do not linger. Prefer local temporary variables over globals for anything carrying sensitive data.
|
||||
|
||||
See sample: `protect-sensitive-data-in-temporary-tables.good.al`.
|
||||
|
||||
## Anti Pattern
|
||||
|
||||
Copying records into a temporary buffer without a preceding permission check, and relying on procedure-exit to clean up. An exception before the explicit cleanup leaves the data in the buffer; a global or var-parameter buffer carries the data back to callers that may have no right to see it.
|
||||
|
||||
See sample: `protect-sensitive-data-in-temporary-tables.bad.al`.
|
||||
Loading…
Add table
Add a link
Reference in a new issue