From e91d5257e9d18dbe51beee1163090e5a9ef1817d Mon Sep 17 00:00:00 2001 From: lvadlamudi Date: Wed, 2 Sep 2026 18:32:39 +0000 Subject: [PATCH 1/7] feat: validate Entra ID directory roles in Azure preflight Discovery previously checked only ARM RBAC (and skipped even that for Owner/Contributor callers), so a service principal missing Application Administrator or Privileged Role Administrator sailed through preflight with Errors: {} and then failed deployment on the azuread resources. The ARM access token's wids claim already lists the caller's active directory role template IDs, so no Graph call or new dependency is needed. New CheckDirectoryRoles task requires: - Application Administrator (or Cloud Application Administrator or Global Administrator) whenever an Entra ID application is created: always for agentless, and for config/activity log unless an existing AD application is reused - Privileged Role Administrator (or Global Administrator) for config/ activity log, which assign the Directory Readers role to the new app Params.UseExistingAdApplication lets callers waive the config/activity log checks when reusing an existing AD application, exposed on the CLI as lacework preflight azure --existing-ad-application. The human output relabels Admin as Subscription Owner/Contributor and adds the directory role count. --- cli/cmd/preflight_azure.go | 57 +++++++++++++---------- cli/cmd/preflight_test.go | 17 +++---- lwpreflight/azure/azure.go | 33 +++++++------ lwpreflight/azure/caller.go | 28 +++++++---- lwpreflight/azure/caller_test.go | 23 +++++++++ lwpreflight/azure/constants.go | 9 ++++ lwpreflight/azure/directoryrole.go | 62 +++++++++++++++++++++++++ lwpreflight/azure/directoryrole_test.go | 61 ++++++++++++++++++++++++ 8 files changed, 234 insertions(+), 56 deletions(-) create mode 100644 lwpreflight/azure/caller_test.go create mode 100644 lwpreflight/azure/directoryrole.go create mode 100644 lwpreflight/azure/directoryrole_test.go diff --git a/cli/cmd/preflight_azure.go b/cli/cmd/preflight_azure.go index 09a1d4c59..7bbe66c5e 100644 --- a/cli/cmd/preflight_azure.go +++ b/cli/cmd/preflight_azure.go @@ -17,14 +17,15 @@ import ( var ( preflightAzureState struct { - agentless bool - config bool - activityLog bool - subscriptionID string - tenantID string - clientID string - clientSecret string - region string + agentless bool + config bool + activityLog bool + existingAdApplication bool + subscriptionID string + tenantID string + clientID string + clientSecret string + region string } preflightAzureCmd = &cobra.Command{ @@ -51,6 +52,9 @@ func init() { "check permissions for the Config integration") flags.BoolVar(&preflightAzureState.activityLog, "activity-log", false, "check permissions for the Activity Log integration") + flags.BoolVar(&preflightAzureState.existingAdApplication, "existing-ad-application", false, + "reuse an existing Entra ID application for Config/Activity Log instead of creating one, "+ + "which waives the directory roles needed to create it") flags.StringVar(&preflightAzureState.subscriptionID, "subscription-id", "", "Azure subscription ID (required)") flags.StringVar(&preflightAzureState.tenantID, "tenant-id", "", @@ -75,14 +79,15 @@ func runPreflightAzure(_ *cobra.Command, _ []string) error { } params := azure.Params{ - Agentless: s.agentless, - Config: s.config, - ActivityLog: s.activityLog, - SubscriptionID: s.subscriptionID, - TenantID: s.tenantID, - ClientID: s.clientID, - ClientSecret: s.clientSecret, - Region: s.region, + Agentless: s.agentless, + Config: s.config, + ActivityLog: s.activityLog, + UseExistingAdApplication: s.existingAdApplication, + SubscriptionID: s.subscriptionID, + TenantID: s.tenantID, + ClientID: s.clientID, + ClientSecret: s.clientSecret, + Region: s.region, } pf, err := azure.New(params) @@ -117,7 +122,8 @@ func renderAzureHumanResult(result *azure.Result, integrations []string) { cli.OutputHuman(" Object ID: %s\n", result.Caller.ObjectID) cli.OutputHuman(" Display name: %s\n", result.Caller.DisplayName) cli.OutputHuman(" Tenant ID: %s\n", result.Caller.TenantID) - cli.OutputHuman(" Admin: %t\n", result.Caller.IsAdmin) + cli.OutputHuman(" Subscription Owner/Contributor: %t\n", result.Caller.IsAdmin) + cli.OutputHuman(" Directory roles: %d\n", len(result.Caller.DirectoryRoles)) if len(integrations) > 0 { cli.OutputHuman("\nIntegrations checked: %s\n", strings.Join(integrations, ", ")) @@ -132,14 +138,15 @@ func renderAzureHumanResult(result *azure.Result, integrations []string) { } func integrationsRequestedAzure(s struct { - agentless bool - config bool - activityLog bool - subscriptionID string - tenantID string - clientID string - clientSecret string - region string + agentless bool + config bool + activityLog bool + existingAdApplication bool + subscriptionID string + tenantID string + clientID string + clientSecret string + region string }) []string { out := []string{} if s.agentless { diff --git a/cli/cmd/preflight_test.go b/cli/cmd/preflight_test.go index 53901bfdd..298e1b1b0 100644 --- a/cli/cmd/preflight_test.go +++ b/cli/cmd/preflight_test.go @@ -71,14 +71,15 @@ func TestIntegrationsRequestedAws(t *testing.T) { func TestIntegrationsRequestedAzure(t *testing.T) { got := integrationsRequestedAzure(struct { - agentless bool - config bool - activityLog bool - subscriptionID string - tenantID string - clientID string - clientSecret string - region string + agentless bool + config bool + activityLog bool + existingAdApplication bool + subscriptionID string + tenantID string + clientID string + clientSecret string + region string }{config: true, activityLog: true}) assert.Equal(t, []string{"azure_config", "azure_activity_log"}, got) } diff --git a/lwpreflight/azure/azure.go b/lwpreflight/azure/azure.go index 583ebd5b3..1d111ff7e 100644 --- a/lwpreflight/azure/azure.go +++ b/lwpreflight/azure/azure.go @@ -16,11 +16,12 @@ type azureConfig struct { } type Preflight struct { - azureConfig azureConfig - integrationTypes []IntegrationType - tasks []func(p *Preflight) error - permissions map[string]bool - permissionsWithWildcard []string + azureConfig azureConfig + integrationTypes []IntegrationType + tasks []func(p *Preflight) error + permissions map[string]bool + permissionsWithWildcard []string + useExistingAdApplication bool caller Caller details Details @@ -44,12 +45,17 @@ type Params struct { ClientID string ClientSecret string Region string + // Set when config/activity log will reuse an existing Entra ID + // application instead of creating one, which waives the directory-role + // requirements for those integrations + UseExistingAdApplication bool } func New(params Params) (*Preflight, error) { integrationTypes := []IntegrationType{} tasks := []func(p *Preflight) error{ FetchCaller, + CheckDirectoryRoles, FetchPolicies, CheckPermissions, FetchDetails, @@ -95,14 +101,15 @@ func New(params Params) (*Preflight, error) { } preflight := &Preflight{ - azureConfig: cfg, - integrationTypes: integrationTypes, - permissions: map[string]bool{}, - permissionsWithWildcard: []string{}, - tasks: tasks, - details: Details{}, - errors: map[IntegrationType][]string{}, - verboseWriter: verbosewriter.New(), + azureConfig: cfg, + integrationTypes: integrationTypes, + permissions: map[string]bool{}, + permissionsWithWildcard: []string{}, + useExistingAdApplication: params.UseExistingAdApplication, + tasks: tasks, + details: Details{}, + errors: map[IntegrationType][]string{}, + verboseWriter: verbosewriter.New(), } return preflight, nil diff --git a/lwpreflight/azure/caller.go b/lwpreflight/azure/caller.go index 0a36a2f5c..eacaba59b 100644 --- a/lwpreflight/azure/caller.go +++ b/lwpreflight/azure/caller.go @@ -19,7 +19,13 @@ type Caller struct { DisplayName string PrincipalID string TenantID string - IsAdmin bool // true if the caller has Owner or Contributor role + // true if the caller has the Owner or Contributor RBAC role on the + // subscription; says nothing about Entra ID directory roles (see + // DirectoryRoles) + IsAdmin bool + // Entra ID directory role template IDs actively assigned to the caller, + // from the token's wids claim + DirectoryRoles []string } func FetchCaller(p *Preflight) error { @@ -46,11 +52,12 @@ func FetchCaller(p *Preflight) error { } p.caller = Caller{ - ObjectID: claims.ObjectID, - DisplayName: claims.DisplayName, - PrincipalID: claims.PrincipalID, - TenantID: claims.TenantID, - IsAdmin: isAdmin, + ObjectID: claims.ObjectID, + DisplayName: claims.DisplayName, + PrincipalID: claims.PrincipalID, + TenantID: claims.TenantID, + IsAdmin: isAdmin, + DirectoryRoles: claims.Wids, } return nil @@ -97,10 +104,11 @@ func checkAdminRole(cred azcore.TokenCredential, objectID, subscriptionID string } type JWTClaims struct { - ObjectID string `json:"oid"` - DisplayName string `json:"name"` - PrincipalID string `json:"sub"` - TenantID string `json:"tid"` + ObjectID string `json:"oid"` + DisplayName string `json:"name"` + PrincipalID string `json:"sub"` + TenantID string `json:"tid"` + Wids []string `json:"wids"` } func parseJWTClaims(token string) (*JWTClaims, error) { diff --git a/lwpreflight/azure/caller_test.go b/lwpreflight/azure/caller_test.go new file mode 100644 index 000000000..9860eb25a --- /dev/null +++ b/lwpreflight/azure/caller_test.go @@ -0,0 +1,23 @@ +package azure + +import ( + "encoding/base64" + "testing" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +func TestParseJWTClaimsWids(t *testing.T) { + payload := base64.RawURLEncoding.EncodeToString([]byte( + `{"oid":"o","tid":"t","wids":["` + PrivilegedRoleAdministratorRoleID + `"]}`)) + claims, err := parseJWTClaims("h." + payload + ".s") + require.NoError(t, err) + assert.Equal(t, []string{PrivilegedRoleAdministratorRoleID}, claims.Wids) + + // no wids claim: nil slice, so every directory role check fails closed + payload = base64.RawURLEncoding.EncodeToString([]byte(`{"oid":"o"}`)) + claims, err = parseJWTClaims("h." + payload + ".s") + require.NoError(t, err) + assert.Nil(t, claims.Wids) +} diff --git a/lwpreflight/azure/constants.go b/lwpreflight/azure/constants.go index 13a3c898d..66d5dc1d8 100644 --- a/lwpreflight/azure/constants.go +++ b/lwpreflight/azure/constants.go @@ -8,6 +8,15 @@ const ( Agentless IntegrationType = "azure_agentless" ) +// Entra ID directory role template IDs +// https://learn.microsoft.com/en-us/entra/identity/role-based-access-control/permissions-reference +const ( + GlobalAdministratorRoleID = "62e90394-69f5-4237-9190-012177145e10" + ApplicationAdministratorRoleID = "9b895d92-2cd3-44c7-9d02-a6ac2d5ea5c3" + CloudApplicationAdministratorRoleID = "158c047a-c907-4556-b7ef-446551a6b5f7" + PrivilegedRoleAdministratorRoleID = "e8611ab8-c189-46e8-94e1-60213ab1f814" +) + var RequiredPermissions = map[IntegrationType][]string{ Config: { "Microsoft.Authorization/roleAssignments/read", diff --git a/lwpreflight/azure/directoryrole.go b/lwpreflight/azure/directoryrole.go new file mode 100644 index 000000000..bb132d570 --- /dev/null +++ b/lwpreflight/azure/directoryrole.go @@ -0,0 +1,62 @@ +package azure + +import ( + "fmt" + "slices" +) + +// CheckDirectoryRoles validates the Entra ID directory roles that deployment +// needs but that ARM permission checks cannot see. Deployment creates an +// Entra ID application (all integration types when a new AD application is +// created; agentless always creates its own), and config/activity log also +// assign the Directory Readers role to it, which requires Privileged Role +// Administrator. Runs unconditionally: subscription Owner/Contributor +// (IsAdmin) is orthogonal to directory roles. +func CheckDirectoryRoles(p *Preflight) error { + p.verboseWriter.Write("Checking Entra ID directory roles") + + hasAny := func(roleIDs ...string) bool { + for _, id := range roleIDs { + if slices.Contains(p.caller.DirectoryRoles, id) { + return true + } + } + return false + } + + canCreateApp := hasAny( + ApplicationAdministratorRoleID, + CloudApplicationAdministratorRoleID, + GlobalAdministratorRoleID, + ) + canAssignDirectoryRole := hasAny( + PrivilegedRoleAdministratorRoleID, + GlobalAdministratorRoleID, + ) + + for _, integrationType := range p.integrationTypes { + // The agentless module always creates its own Entra ID application; + // config/activity log only do so when not reusing an existing one. + createsApp := integrationType == Agentless || !p.useExistingAdApplication + // Only the ad-application module (config/activity log, new app only) + // assigns the Directory Readers role. + assignsDirectoryRole := integrationType != Agentless && !p.useExistingAdApplication + + if createsApp && !canCreateApp { + p.errors[integrationType] = append(p.errors[integrationType], fmt.Sprintf( + "Required Entra ID directory role missing: Application Administrator "+ + "(or Cloud Application Administrator) to create the Lacework Entra ID "+ + "application for %s. Activate the role in PIM or re-authenticate if it "+ + "was just assigned", integrationType)) + } + if assignsDirectoryRole && !canAssignDirectoryRole { + p.errors[integrationType] = append(p.errors[integrationType], fmt.Sprintf( + "Required Entra ID directory role missing: Privileged Role Administrator "+ + "to assign the Directory Readers role to the Lacework Entra ID "+ + "application for %s. Activate the role in PIM or re-authenticate if it "+ + "was just assigned", integrationType)) + } + } + + return nil +} diff --git a/lwpreflight/azure/directoryrole_test.go b/lwpreflight/azure/directoryrole_test.go new file mode 100644 index 000000000..5005eca45 --- /dev/null +++ b/lwpreflight/azure/directoryrole_test.go @@ -0,0 +1,61 @@ +package azure + +import ( + "testing" + + "github.com/stretchr/testify/assert" + + "github.com/lacework/go-sdk/v2/lwpreflight/verbosewriter" +) + +func preflightWithRoles(roles []string, useExistingAdApp bool, types ...IntegrationType) *Preflight { + return &Preflight{ + integrationTypes: types, + useExistingAdApplication: useExistingAdApp, + caller: Caller{DirectoryRoles: roles}, + errors: map[IntegrationType][]string{}, + verboseWriter: verbosewriter.New(), + } +} + +func TestCheckDirectoryRolesMissingAll(t *testing.T) { + p := preflightWithRoles(nil, false, Config, ActivityLog, Agentless) + assert.NoError(t, CheckDirectoryRoles(p)) + + // config/activity log need app creation + directory role assignment + assert.Len(t, p.errors[Config], 2) + assert.Len(t, p.errors[ActivityLog], 2) + // agentless creates an app but assigns no directory role + assert.Len(t, p.errors[Agentless], 1) + assert.Contains(t, p.errors[Agentless][0], "Application Administrator") +} + +func TestCheckDirectoryRolesMissingPrivilegedRoleAdmin(t *testing.T) { + p := preflightWithRoles( + []string{ApplicationAdministratorRoleID}, false, Config, ActivityLog, Agentless) + assert.NoError(t, CheckDirectoryRoles(p)) + + assert.Len(t, p.errors[Config], 1) + assert.Contains(t, p.errors[Config][0], "Privileged Role Administrator") + assert.Len(t, p.errors[ActivityLog], 1) + // agentless does not need Privileged Role Administrator + assert.Empty(t, p.errors[Agentless]) +} + +func TestCheckDirectoryRolesGlobalAdminSatisfiesAll(t *testing.T) { + p := preflightWithRoles( + []string{GlobalAdministratorRoleID}, false, Config, ActivityLog, Agentless) + assert.NoError(t, CheckDirectoryRoles(p)) + assert.Empty(t, p.errors) +} + +func TestCheckDirectoryRolesExistingAdApplication(t *testing.T) { + p := preflightWithRoles(nil, true, Config, ActivityLog, Agentless) + assert.NoError(t, CheckDirectoryRoles(p)) + + // existing AD app: config/activity log neither create an app nor assign roles + assert.Empty(t, p.errors[Config]) + assert.Empty(t, p.errors[ActivityLog]) + // agentless always creates its own app + assert.Len(t, p.errors[Agentless], 1) +} From b05b475ff0929399a460c254f0234d98b80492de Mon Sep 17 00:00:00 2001 From: lvadlamudi Date: Wed, 2 Sep 2026 18:51:21 +0000 Subject: [PATCH 2/7] test: update preflight azure help snapshot for --existing-ad-application --- .../test_resources/help/preflight_azure | 19 ++++++++++--------- 1 file changed, 10 insertions(+), 9 deletions(-) diff --git a/integration/test_resources/help/preflight_azure b/integration/test_resources/help/preflight_azure index 7766a324e..e17084e55 100644 --- a/integration/test_resources/help/preflight_azure +++ b/integration/test_resources/help/preflight_azure @@ -10,15 +10,16 @@ Usage: lacework preflight azure [flags] Flags: - --activity-log check permissions for the Activity Log integration - --agentless check permissions for the Agentless integration - --client-id string Azure service principal client ID - --client-secret string Azure service principal client secret - --config check permissions for the Config integration - -h, --help help for azure - --region string Azure region to use for region-scoped checks - --subscription-id string Azure subscription ID (required) - --tenant-id string Azure tenant ID (required when using --client-id/--client-secret) + --activity-log check permissions for the Activity Log integration + --agentless check permissions for the Agentless integration + --client-id string Azure service principal client ID + --client-secret string Azure service principal client secret + --config check permissions for the Config integration + --existing-ad-application reuse an existing Entra ID application for Config/Activity Log instead of creating one, which waives the directory roles needed to create it + -h, --help help for azure + --region string Azure region to use for region-scoped checks + --subscription-id string Azure subscription ID (required) + --tenant-id string Azure tenant ID (required when using --client-id/--client-secret) Global Flags: -a, --account string account subdomain of URL (i.e. .lacework.net) From 31c71629bdc7e81ba807cb79abb547b21e82c538 Mon Sep 17 00:00:00 2001 From: lvadlamudi Date: Wed, 2 Sep 2026 18:53:02 +0000 Subject: [PATCH 3/7] fix: shorten --existing-ad-application help text and refresh snapshot --- cli/cmd/preflight_azure.go | 3 +-- integration/test_resources/help/preflight_azure | 2 +- 2 files changed, 2 insertions(+), 3 deletions(-) diff --git a/cli/cmd/preflight_azure.go b/cli/cmd/preflight_azure.go index 7bbe66c5e..2d1016e83 100644 --- a/cli/cmd/preflight_azure.go +++ b/cli/cmd/preflight_azure.go @@ -53,8 +53,7 @@ func init() { flags.BoolVar(&preflightAzureState.activityLog, "activity-log", false, "check permissions for the Activity Log integration") flags.BoolVar(&preflightAzureState.existingAdApplication, "existing-ad-application", false, - "reuse an existing Entra ID application for Config/Activity Log instead of creating one, "+ - "which waives the directory roles needed to create it") + "reuse an existing Entra ID application for Config/Activity Log (skips the directory-role checks)") flags.StringVar(&preflightAzureState.subscriptionID, "subscription-id", "", "Azure subscription ID (required)") flags.StringVar(&preflightAzureState.tenantID, "tenant-id", "", diff --git a/integration/test_resources/help/preflight_azure b/integration/test_resources/help/preflight_azure index e17084e55..487de0895 100644 --- a/integration/test_resources/help/preflight_azure +++ b/integration/test_resources/help/preflight_azure @@ -15,7 +15,7 @@ Flags: --client-id string Azure service principal client ID --client-secret string Azure service principal client secret --config check permissions for the Config integration - --existing-ad-application reuse an existing Entra ID application for Config/Activity Log instead of creating one, which waives the directory roles needed to create it + --existing-ad-application reuse an existing Entra ID application for Config/Activity Log (skips the directory-role checks) -h, --help help for azure --region string Azure region to use for region-scoped checks --subscription-id string Azure subscription ID (required) From 43829242b1d7f383ba6c0d06ee5595f68b5c937a Mon Sep 17 00:00:00 2001 From: lvadlamudi Date: Wed, 2 Sep 2026 20:53:59 +0000 Subject: [PATCH 4/7] fix: scope existing-app waiver per Azure integration --- cli/cmd/preflight_azure.go | 21 +++++++------ lwpreflight/azure/azure.go | 13 ++++---- lwpreflight/azure/directoryrole.go | 6 ++-- lwpreflight/azure/directoryrole_test.go | 41 +++++++++++++++++++++---- 4 files changed, 58 insertions(+), 23 deletions(-) diff --git a/cli/cmd/preflight_azure.go b/cli/cmd/preflight_azure.go index 2d1016e83..dc512fa12 100644 --- a/cli/cmd/preflight_azure.go +++ b/cli/cmd/preflight_azure.go @@ -78,15 +78,18 @@ func runPreflightAzure(_ *cobra.Command, _ []string) error { } params := azure.Params{ - Agentless: s.agentless, - Config: s.config, - ActivityLog: s.activityLog, - UseExistingAdApplication: s.existingAdApplication, - SubscriptionID: s.subscriptionID, - TenantID: s.tenantID, - ClientID: s.clientID, - ClientSecret: s.clientSecret, - Region: s.region, + Agentless: s.agentless, + Config: s.config, + ActivityLog: s.activityLog, + UseExistingAdApplication: map[azure.IntegrationType]bool{ + azure.Config: s.existingAdApplication, + azure.ActivityLog: s.existingAdApplication, + }, + SubscriptionID: s.subscriptionID, + TenantID: s.tenantID, + ClientID: s.clientID, + ClientSecret: s.clientSecret, + Region: s.region, } pf, err := azure.New(params) diff --git a/lwpreflight/azure/azure.go b/lwpreflight/azure/azure.go index 1d111ff7e..cc8f1ac4a 100644 --- a/lwpreflight/azure/azure.go +++ b/lwpreflight/azure/azure.go @@ -2,6 +2,7 @@ package azure import ( "errors" + "maps" "github.com/Azure/azure-sdk-for-go/sdk/azcore" "github.com/Azure/azure-sdk-for-go/sdk/azidentity" @@ -21,7 +22,7 @@ type Preflight struct { tasks []func(p *Preflight) error permissions map[string]bool permissionsWithWildcard []string - useExistingAdApplication bool + useExistingAdApplication map[IntegrationType]bool caller Caller details Details @@ -45,10 +46,10 @@ type Params struct { ClientID string ClientSecret string Region string - // Set when config/activity log will reuse an existing Entra ID - // application instead of creating one, which waives the directory-role - // requirements for those integrations - UseExistingAdApplication bool + // UseExistingAdApplication identifies Config and Activity Log integrations + // that reuse an existing Entra ID application instead of creating one, + // which waives their directory-role requirements. + UseExistingAdApplication map[IntegrationType]bool } func New(params Params) (*Preflight, error) { @@ -105,7 +106,7 @@ func New(params Params) (*Preflight, error) { integrationTypes: integrationTypes, permissions: map[string]bool{}, permissionsWithWildcard: []string{}, - useExistingAdApplication: params.UseExistingAdApplication, + useExistingAdApplication: maps.Clone(params.UseExistingAdApplication), tasks: tasks, details: Details{}, errors: map[IntegrationType][]string{}, diff --git a/lwpreflight/azure/directoryrole.go b/lwpreflight/azure/directoryrole.go index bb132d570..9d2ff5d5c 100644 --- a/lwpreflight/azure/directoryrole.go +++ b/lwpreflight/azure/directoryrole.go @@ -35,12 +35,14 @@ func CheckDirectoryRoles(p *Preflight) error { ) for _, integrationType := range p.integrationTypes { + usesExistingAdApplication := p.useExistingAdApplication[integrationType] + // The agentless module always creates its own Entra ID application; // config/activity log only do so when not reusing an existing one. - createsApp := integrationType == Agentless || !p.useExistingAdApplication + createsApp := integrationType == Agentless || !usesExistingAdApplication // Only the ad-application module (config/activity log, new app only) // assigns the Directory Readers role. - assignsDirectoryRole := integrationType != Agentless && !p.useExistingAdApplication + assignsDirectoryRole := integrationType != Agentless && !usesExistingAdApplication if createsApp && !canCreateApp { p.errors[integrationType] = append(p.errors[integrationType], fmt.Sprintf( diff --git a/lwpreflight/azure/directoryrole_test.go b/lwpreflight/azure/directoryrole_test.go index 5005eca45..8ea71371e 100644 --- a/lwpreflight/azure/directoryrole_test.go +++ b/lwpreflight/azure/directoryrole_test.go @@ -8,10 +8,14 @@ import ( "github.com/lacework/go-sdk/v2/lwpreflight/verbosewriter" ) -func preflightWithRoles(roles []string, useExistingAdApp bool, types ...IntegrationType) *Preflight { +func preflightWithRoles( + roles []string, + useExistingAdApplication map[IntegrationType]bool, + types ...IntegrationType, +) *Preflight { return &Preflight{ integrationTypes: types, - useExistingAdApplication: useExistingAdApp, + useExistingAdApplication: useExistingAdApplication, caller: Caller{DirectoryRoles: roles}, errors: map[IntegrationType][]string{}, verboseWriter: verbosewriter.New(), @@ -19,7 +23,7 @@ func preflightWithRoles(roles []string, useExistingAdApp bool, types ...Integrat } func TestCheckDirectoryRolesMissingAll(t *testing.T) { - p := preflightWithRoles(nil, false, Config, ActivityLog, Agentless) + p := preflightWithRoles(nil, nil, Config, ActivityLog, Agentless) assert.NoError(t, CheckDirectoryRoles(p)) // config/activity log need app creation + directory role assignment @@ -32,7 +36,7 @@ func TestCheckDirectoryRolesMissingAll(t *testing.T) { func TestCheckDirectoryRolesMissingPrivilegedRoleAdmin(t *testing.T) { p := preflightWithRoles( - []string{ApplicationAdministratorRoleID}, false, Config, ActivityLog, Agentless) + []string{ApplicationAdministratorRoleID}, nil, Config, ActivityLog, Agentless) assert.NoError(t, CheckDirectoryRoles(p)) assert.Len(t, p.errors[Config], 1) @@ -44,13 +48,15 @@ func TestCheckDirectoryRolesMissingPrivilegedRoleAdmin(t *testing.T) { func TestCheckDirectoryRolesGlobalAdminSatisfiesAll(t *testing.T) { p := preflightWithRoles( - []string{GlobalAdministratorRoleID}, false, Config, ActivityLog, Agentless) + []string{GlobalAdministratorRoleID}, nil, Config, ActivityLog, Agentless) assert.NoError(t, CheckDirectoryRoles(p)) assert.Empty(t, p.errors) } func TestCheckDirectoryRolesExistingAdApplication(t *testing.T) { - p := preflightWithRoles(nil, true, Config, ActivityLog, Agentless) + p := preflightWithRoles(nil, map[IntegrationType]bool{ + Config: true, ActivityLog: true, + }, Config, ActivityLog, Agentless) assert.NoError(t, CheckDirectoryRoles(p)) // existing AD app: config/activity log neither create an app nor assign roles @@ -59,3 +65,26 @@ func TestCheckDirectoryRolesExistingAdApplication(t *testing.T) { // agentless always creates its own app assert.Len(t, p.errors[Agentless], 1) } + +func TestCheckDirectoryRolesMixedExistingAdApplication(t *testing.T) { + tests := []struct { + name string + existingIntegration IntegrationType + newApplicationType IntegrationType + }{ + {"config reuses an application", Config, ActivityLog}, + {"activity log reuses an application", ActivityLog, Config}, + } + + for _, test := range tests { + t.Run(test.name, func(t *testing.T) { + p := preflightWithRoles(nil, map[IntegrationType]bool{ + test.existingIntegration: true, + }, Config, ActivityLog) + assert.NoError(t, CheckDirectoryRoles(p)) + + assert.Empty(t, p.errors[test.existingIntegration]) + assert.Len(t, p.errors[test.newApplicationType], 2) + }) + } +} From f6c9c59b2613df2ca8917f5e8b4aeace040258d0 Mon Sep 17 00:00:00 2001 From: lvadlamudi Date: Wed, 2 Sep 2026 21:00:46 +0000 Subject: [PATCH 5/7] feat: support mixed Azure existing-app preflight --- cli/cmd/preflight_azure.go | 72 ++++++++++--------- cli/cmd/preflight_test.go | 48 ++++++++++--- .../test_resources/help/preflight_azure | 22 +++--- 3 files changed, 86 insertions(+), 56 deletions(-) diff --git a/cli/cmd/preflight_azure.go b/cli/cmd/preflight_azure.go index dc512fa12..68c9435a5 100644 --- a/cli/cmd/preflight_azure.go +++ b/cli/cmd/preflight_azure.go @@ -15,18 +15,22 @@ import ( "github.com/lacework/go-sdk/v2/lwpreflight/azure" ) +type azurePreflightOptions struct { + agentless bool + config bool + activityLog bool + existingAdApplication bool + configExistingAdApplication bool + activityLogExistingAdApplication bool + subscriptionID string + tenantID string + clientID string + clientSecret string + region string +} + var ( - preflightAzureState struct { - agentless bool - config bool - activityLog bool - existingAdApplication bool - subscriptionID string - tenantID string - clientID string - clientSecret string - region string - } + preflightAzureState azurePreflightOptions preflightAzureCmd = &cobra.Command{ Use: "azure", @@ -53,7 +57,11 @@ func init() { flags.BoolVar(&preflightAzureState.activityLog, "activity-log", false, "check permissions for the Activity Log integration") flags.BoolVar(&preflightAzureState.existingAdApplication, "existing-ad-application", false, - "reuse an existing Entra ID application for Config/Activity Log (skips the directory-role checks)") + "reuse existing Entra ID applications for both Config and Activity Log") + flags.BoolVar(&preflightAzureState.configExistingAdApplication, "config-existing-ad-application", false, + "reuse an existing Entra ID application for Config") + flags.BoolVar(&preflightAzureState.activityLogExistingAdApplication, "activity-log-existing-ad-application", false, + "reuse an existing Entra ID application for Activity Log") flags.StringVar(&preflightAzureState.subscriptionID, "subscription-id", "", "Azure subscription ID (required)") flags.StringVar(&preflightAzureState.tenantID, "tenant-id", "", @@ -78,18 +86,15 @@ func runPreflightAzure(_ *cobra.Command, _ []string) error { } params := azure.Params{ - Agentless: s.agentless, - Config: s.config, - ActivityLog: s.activityLog, - UseExistingAdApplication: map[azure.IntegrationType]bool{ - azure.Config: s.existingAdApplication, - azure.ActivityLog: s.existingAdApplication, - }, - SubscriptionID: s.subscriptionID, - TenantID: s.tenantID, - ClientID: s.clientID, - ClientSecret: s.clientSecret, - Region: s.region, + Agentless: s.agentless, + Config: s.config, + ActivityLog: s.activityLog, + UseExistingAdApplication: existingAdApplicationsAzure(s), + SubscriptionID: s.subscriptionID, + TenantID: s.tenantID, + ClientID: s.clientID, + ClientSecret: s.clientSecret, + Region: s.region, } pf, err := azure.New(params) @@ -139,17 +144,14 @@ func renderAzureHumanResult(result *azure.Result, integrations []string) { } } -func integrationsRequestedAzure(s struct { - agentless bool - config bool - activityLog bool - existingAdApplication bool - subscriptionID string - tenantID string - clientID string - clientSecret string - region string -}) []string { +func existingAdApplicationsAzure(s azurePreflightOptions) map[azure.IntegrationType]bool { + return map[azure.IntegrationType]bool{ + azure.Config: s.existingAdApplication || s.configExistingAdApplication, + azure.ActivityLog: s.existingAdApplication || s.activityLogExistingAdApplication, + } +} + +func integrationsRequestedAzure(s azurePreflightOptions) []string { out := []string{} if s.agentless { out = append(out, string(azure.Agentless)) diff --git a/cli/cmd/preflight_test.go b/cli/cmd/preflight_test.go index 298e1b1b0..225a1b624 100644 --- a/cli/cmd/preflight_test.go +++ b/cli/cmd/preflight_test.go @@ -70,20 +70,46 @@ func TestIntegrationsRequestedAws(t *testing.T) { } func TestIntegrationsRequestedAzure(t *testing.T) { - got := integrationsRequestedAzure(struct { - agentless bool - config bool - activityLog bool - existingAdApplication bool - subscriptionID string - tenantID string - clientID string - clientSecret string - region string - }{config: true, activityLog: true}) + got := integrationsRequestedAzure(azurePreflightOptions{config: true, activityLog: true}) assert.Equal(t, []string{"azure_config", "azure_activity_log"}, got) } +func TestExistingAdApplicationsAzure(t *testing.T) { + tests := []struct { + name string + options azurePreflightOptions + expected map[azure.IntegrationType]bool + }{ + { + name: "both integrations shorthand", + options: azurePreflightOptions{existingAdApplication: true}, + expected: map[azure.IntegrationType]bool{ + azure.Config: true, azure.ActivityLog: true, + }, + }, + { + name: "config only", + options: azurePreflightOptions{configExistingAdApplication: true}, + expected: map[azure.IntegrationType]bool{ + azure.Config: true, azure.ActivityLog: false, + }, + }, + { + name: "activity log only", + options: azurePreflightOptions{activityLogExistingAdApplication: true}, + expected: map[azure.IntegrationType]bool{ + azure.Config: false, azure.ActivityLog: true, + }, + }, + } + + for _, test := range tests { + t.Run(test.name, func(t *testing.T) { + assert.Equal(t, test.expected, existingAdApplicationsAzure(test.options)) + }) + } +} + func TestIntegrationsRequestedGcp(t *testing.T) { got := integrationsRequestedGcp(struct { agentless bool diff --git a/integration/test_resources/help/preflight_azure b/integration/test_resources/help/preflight_azure index 487de0895..30bb8a1ff 100644 --- a/integration/test_resources/help/preflight_azure +++ b/integration/test_resources/help/preflight_azure @@ -10,16 +10,18 @@ Usage: lacework preflight azure [flags] Flags: - --activity-log check permissions for the Activity Log integration - --agentless check permissions for the Agentless integration - --client-id string Azure service principal client ID - --client-secret string Azure service principal client secret - --config check permissions for the Config integration - --existing-ad-application reuse an existing Entra ID application for Config/Activity Log (skips the directory-role checks) - -h, --help help for azure - --region string Azure region to use for region-scoped checks - --subscription-id string Azure subscription ID (required) - --tenant-id string Azure tenant ID (required when using --client-id/--client-secret) + --activity-log check permissions for the Activity Log integration + --activity-log-existing-ad-application reuse an existing Entra ID application for Activity Log + --agentless check permissions for the Agentless integration + --client-id string Azure service principal client ID + --client-secret string Azure service principal client secret + --config check permissions for the Config integration + --config-existing-ad-application reuse an existing Entra ID application for Config + --existing-ad-application reuse existing Entra ID applications for both Config and Activity Log + -h, --help help for azure + --region string Azure region to use for region-scoped checks + --subscription-id string Azure subscription ID (required) + --tenant-id string Azure tenant ID (required when using --client-id/--client-secret) Global Flags: -a, --account string account subdomain of URL (i.e. .lacework.net) From 9e9f2c6643cacb676faa7976039a620d6af76598 Mon Sep 17 00:00:00 2001 From: lvadlamudi Date: Thu, 3 Sep 2026 04:36:38 +0000 Subject: [PATCH 6/7] fix: drop PIM hint from Entra ID directory role errors The trailing sentence made each message ~200 chars and wrapped badly in the deployment wizard's error banner without telling the user anything they act on differently. The missing role name is the actionable part. --- lwpreflight/azure/directoryrole.go | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/lwpreflight/azure/directoryrole.go b/lwpreflight/azure/directoryrole.go index 9d2ff5d5c..47bf4b75a 100644 --- a/lwpreflight/azure/directoryrole.go +++ b/lwpreflight/azure/directoryrole.go @@ -48,15 +48,13 @@ func CheckDirectoryRoles(p *Preflight) error { p.errors[integrationType] = append(p.errors[integrationType], fmt.Sprintf( "Required Entra ID directory role missing: Application Administrator "+ "(or Cloud Application Administrator) to create the Lacework Entra ID "+ - "application for %s. Activate the role in PIM or re-authenticate if it "+ - "was just assigned", integrationType)) + "application for %s", integrationType)) } if assignsDirectoryRole && !canAssignDirectoryRole { p.errors[integrationType] = append(p.errors[integrationType], fmt.Sprintf( "Required Entra ID directory role missing: Privileged Role Administrator "+ "to assign the Directory Readers role to the Lacework Entra ID "+ - "application for %s. Activate the role in PIM or re-authenticate if it "+ - "was just assigned", integrationType)) + "application for %s", integrationType)) } } From c2de68adf2bf2313641bb44cc46aaabd726a3c87 Mon Sep 17 00:00:00 2001 From: lvadlamudi Date: Thu, 3 Sep 2026 18:04:25 +0000 Subject: [PATCH 7/7] feat: accept Graph app permissions in Azure preflight The directory-role check read only the wids claim, which lists directory roles. An app-only principal can hold the equivalent Microsoft Graph application permission instead, so a caller with Application.ReadWrite.OwnedBy and RoleManagement.ReadWrite.Directory was blocked even though deployment would have succeeded. Graph application permissions arrive in the roles claim of a token issued for Graph, never in the ARM token the caller check already decodes, so the credential is asked for a second token. That is a token request, not a Graph API call, and needs no permission of its own. The accepted set stays narrow, and Directory.ReadWrite.All is left out on purpose: a missing entry costs a false failure the caller can fix by assigning a directory role, while a wrong entry waves a caller through to a deployment that then fails. When the Graph token cannot be read at all the check falls back to directory roles and says so in the error, since in silent or JSON output the verbose line goes nowhere. --- cli/cmd/preflight_azure.go | 1 + lwpreflight/azure/azure.go | 3 + lwpreflight/azure/caller.go | 51 ++++++++++-- lwpreflight/azure/caller_test.go | 16 ++++ lwpreflight/azure/constants.go | 19 +++++ lwpreflight/azure/directoryrole.go | 48 +++++++---- lwpreflight/azure/directoryrole_test.go | 101 ++++++++++++++++++++++++ 7 files changed, 219 insertions(+), 20 deletions(-) diff --git a/cli/cmd/preflight_azure.go b/cli/cmd/preflight_azure.go index 68c9435a5..a0a154339 100644 --- a/cli/cmd/preflight_azure.go +++ b/cli/cmd/preflight_azure.go @@ -131,6 +131,7 @@ func renderAzureHumanResult(result *azure.Result, integrations []string) { cli.OutputHuman(" Tenant ID: %s\n", result.Caller.TenantID) cli.OutputHuman(" Subscription Owner/Contributor: %t\n", result.Caller.IsAdmin) cli.OutputHuman(" Directory roles: %d\n", len(result.Caller.DirectoryRoles)) + cli.OutputHuman(" Graph application permissions: %d\n", len(result.Caller.GraphPermissions)) if len(integrations) > 0 { cli.OutputHuman("\nIntegrations checked: %s\n", strings.Join(integrations, ", ")) diff --git a/lwpreflight/azure/azure.go b/lwpreflight/azure/azure.go index cc8f1ac4a..7b1c0b146 100644 --- a/lwpreflight/azure/azure.go +++ b/lwpreflight/azure/azure.go @@ -27,6 +27,9 @@ type Preflight struct { caller Caller details Details errors map[IntegrationType][]string + // set when the caller's Microsoft Graph application permissions could not + // be read, so a directory-role error can say the second path was not seen + graphPermissionsErr error verboseWriter verbosewriter.WriteCloser } diff --git a/lwpreflight/azure/caller.go b/lwpreflight/azure/caller.go index eacaba59b..e429e5e33 100644 --- a/lwpreflight/azure/caller.go +++ b/lwpreflight/azure/caller.go @@ -26,6 +26,11 @@ type Caller struct { // Entra ID directory role template IDs actively assigned to the caller, // from the token's wids claim DirectoryRoles []string + // Microsoft Graph application permissions granted to the caller, from the + // roles claim of a Graph-audience token. Empty for a delegated (user) + // credential, whose effective permission is bounded by its own directory + // roles anyway. + GraphPermissions []string } func FetchCaller(p *Preflight) error { @@ -51,18 +56,51 @@ func FetchCaller(p *Preflight) error { return err } + // Best effort: a caller can hold Graph application permissions instead of a + // directory role. Failing to read them only costs the checks that fall back + // to directory roles alone, so it must not fail preflight. + graphPermissions, err := fetchGraphPermissions(p.azureConfig.cred) + if err != nil { + p.graphPermissionsErr = err + p.verboseWriter.Write(fmt.Sprintf( + "Could not read Microsoft Graph application permissions, "+ + "checking Entra ID directory roles only: %v", err)) + } + p.caller = Caller{ - ObjectID: claims.ObjectID, - DisplayName: claims.DisplayName, - PrincipalID: claims.PrincipalID, - TenantID: claims.TenantID, - IsAdmin: isAdmin, - DirectoryRoles: claims.Wids, + ObjectID: claims.ObjectID, + DisplayName: claims.DisplayName, + PrincipalID: claims.PrincipalID, + TenantID: claims.TenantID, + IsAdmin: isAdmin, + DirectoryRoles: claims.Wids, + GraphPermissions: graphPermissions, } return nil } +// fetchGraphPermissions reads the caller's Microsoft Graph application +// permissions from the roles claim of a Graph-audience token. The ARM token +// cannot carry them: app roles are scoped to the resource the token is for. +// Only the token is requested, never a Graph API call, so this needs no +// permission of its own. +func fetchGraphPermissions(cred azcore.TokenCredential) ([]string, error) { + token, err := cred.GetToken(context.Background(), policy.TokenRequestOptions{ + Scopes: []string{"https://graph.microsoft.com/.default"}, + }) + if err != nil { + return nil, fmt.Errorf("failed to get token: %v", err) + } + + claims, err := parseJWTClaims(token.Token) + if err != nil { + return nil, err + } + + return claims.Roles, nil +} + func checkAdminRole(cred azcore.TokenCredential, objectID, subscriptionID string) (bool, error) { client, err := armauthorization.NewRoleAssignmentsClient(subscriptionID, cred, nil) if err != nil { @@ -109,6 +147,7 @@ type JWTClaims struct { PrincipalID string `json:"sub"` TenantID string `json:"tid"` Wids []string `json:"wids"` + Roles []string `json:"roles"` } func parseJWTClaims(token string) (*JWTClaims, error) { diff --git a/lwpreflight/azure/caller_test.go b/lwpreflight/azure/caller_test.go index 9860eb25a..1ae534fea 100644 --- a/lwpreflight/azure/caller_test.go +++ b/lwpreflight/azure/caller_test.go @@ -21,3 +21,19 @@ func TestParseJWTClaimsWids(t *testing.T) { require.NoError(t, err) assert.Nil(t, claims.Wids) } + +func TestParseJWTClaimsRoles(t *testing.T) { + // Graph application permissions arrive in the roles claim of a + // Graph-audience token, never in the ARM token the caller check decodes + payload := base64.RawURLEncoding.EncodeToString([]byte( + `{"oid":"o","roles":["` + GraphApplicationReadWriteAllPermission + `"]}`)) + claims, err := parseJWTClaims("h." + payload + ".s") + require.NoError(t, err) + assert.Equal(t, []string{GraphApplicationReadWriteAllPermission}, claims.Roles) + + // no roles claim: nil slice, so the check falls back to directory roles + payload = base64.RawURLEncoding.EncodeToString([]byte(`{"oid":"o"}`)) + claims, err = parseJWTClaims("h." + payload + ".s") + require.NoError(t, err) + assert.Nil(t, claims.Roles) +} diff --git a/lwpreflight/azure/constants.go b/lwpreflight/azure/constants.go index 66d5dc1d8..8247c90de 100644 --- a/lwpreflight/azure/constants.go +++ b/lwpreflight/azure/constants.go @@ -17,6 +17,25 @@ const ( PrivilegedRoleAdministratorRoleID = "e8611ab8-c189-46e8-94e1-60213ab1f814" ) +// Microsoft Graph application permissions that grant the same capability as the +// directory roles above. An app-only principal can hold these instead of a +// directory role, in which case its wids claim says nothing about what it can do. +// https://learn.microsoft.com/en-us/graph/permissions-reference +// +// Deliberately limited to the permissions Microsoft documents as sufficient on +// their own. A missing entry costs a false failure, which the caller can fix by +// assigning a directory role; a wrong entry would wave a caller through to a +// deployment that then fails. +const ( + // Create and manage any application registration and service principal. + GraphApplicationReadWriteAllPermission = "Application.ReadWrite.All" + // Create applications, and manage the ones this principal owns, which + // includes every application it creates. + GraphApplicationReadWriteOwnedByPermission = "Application.ReadWrite.OwnedBy" + // Assign a directory role, such as Directory Readers, to a principal. + GraphRoleManagementReadWriteDirectoryPermission = "RoleManagement.ReadWrite.Directory" +) + var RequiredPermissions = map[IntegrationType][]string{ Config: { "Microsoft.Authorization/roleAssignments/read", diff --git a/lwpreflight/azure/directoryrole.go b/lwpreflight/azure/directoryrole.go index 47bf4b75a..2e298cca9 100644 --- a/lwpreflight/azure/directoryrole.go +++ b/lwpreflight/azure/directoryrole.go @@ -5,19 +5,23 @@ import ( "slices" ) -// CheckDirectoryRoles validates the Entra ID directory roles that deployment -// needs but that ARM permission checks cannot see. Deployment creates an -// Entra ID application (all integration types when a new AD application is -// created; agentless always creates its own), and config/activity log also -// assign the Directory Readers role to it, which requires Privileged Role -// Administrator. Runs unconditionally: subscription Owner/Contributor -// (IsAdmin) is orthogonal to directory roles. +// CheckDirectoryRoles validates the Entra ID privileges that deployment needs +// but that ARM permission checks cannot see. Deployment creates an Entra ID +// application (all integration types when a new AD application is created; +// agentless always creates its own), and config/activity log also assign the +// Directory Readers role to it. Either a directory role or the equivalent +// Microsoft Graph application permission satisfies each requirement. Runs +// unconditionally: subscription Owner/Contributor (IsAdmin) is orthogonal to +// both. func CheckDirectoryRoles(p *Preflight) error { p.verboseWriter.Write("Checking Entra ID directory roles") - hasAny := func(roleIDs ...string) bool { - for _, id := range roleIDs { - if slices.Contains(p.caller.DirectoryRoles, id) { + // Directory roles are GUIDs and Graph application permissions are dotted + // names, so the two claim spaces cannot collide in one list. + held := slices.Concat(p.caller.DirectoryRoles, p.caller.GraphPermissions) + hasAny := func(grants ...string) bool { + for _, grant := range grants { + if slices.Contains(held, grant) { return true } } @@ -28,12 +32,24 @@ func CheckDirectoryRoles(p *Preflight) error { ApplicationAdministratorRoleID, CloudApplicationAdministratorRoleID, GlobalAdministratorRoleID, + GraphApplicationReadWriteOwnedByPermission, + GraphApplicationReadWriteAllPermission, ) canAssignDirectoryRole := hasAny( PrivilegedRoleAdministratorRoleID, GlobalAdministratorRoleID, + GraphRoleManagementReadWriteDirectoryPermission, ) + // A caller can hold the Graph permission rather than the directory role, so + // a failure to read those permissions makes either message a guess. + unread := "" + if p.graphPermissionsErr != nil { + unread = fmt.Sprintf( + " (Microsoft Graph application permissions could not be read: %v)", + p.graphPermissionsErr) + } + for _, integrationType := range p.integrationTypes { usesExistingAdApplication := p.useExistingAdApplication[integrationType] @@ -47,14 +63,18 @@ func CheckDirectoryRoles(p *Preflight) error { if createsApp && !canCreateApp { p.errors[integrationType] = append(p.errors[integrationType], fmt.Sprintf( "Required Entra ID directory role missing: Application Administrator "+ - "(or Cloud Application Administrator) to create the Lacework Entra ID "+ - "application for %s", integrationType)) + "(or Cloud Application Administrator, or the "+ + GraphApplicationReadWriteOwnedByPermission+" Microsoft Graph "+ + "permission) to create the Lacework Entra ID application for %s%s", + integrationType, unread)) } if assignsDirectoryRole && !canAssignDirectoryRole { p.errors[integrationType] = append(p.errors[integrationType], fmt.Sprintf( "Required Entra ID directory role missing: Privileged Role Administrator "+ - "to assign the Directory Readers role to the Lacework Entra ID "+ - "application for %s", integrationType)) + "(or the "+GraphRoleManagementReadWriteDirectoryPermission+ + " Microsoft Graph permission) to assign the Directory Readers role "+ + "to the Lacework Entra ID application for %s%s", + integrationType, unread)) } } diff --git a/lwpreflight/azure/directoryrole_test.go b/lwpreflight/azure/directoryrole_test.go index 8ea71371e..abb2c72c5 100644 --- a/lwpreflight/azure/directoryrole_test.go +++ b/lwpreflight/azure/directoryrole_test.go @@ -1,9 +1,11 @@ package azure import ( + "errors" "testing" "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" "github.com/lacework/go-sdk/v2/lwpreflight/verbosewriter" ) @@ -22,6 +24,12 @@ func preflightWithRoles( } } +func preflightWithGraphPermissions(permissions []string, types ...IntegrationType) *Preflight { + p := preflightWithRoles(nil, nil, types...) + p.caller.GraphPermissions = permissions + return p +} + func TestCheckDirectoryRolesMissingAll(t *testing.T) { p := preflightWithRoles(nil, nil, Config, ActivityLog, Agentless) assert.NoError(t, CheckDirectoryRoles(p)) @@ -88,3 +96,96 @@ func TestCheckDirectoryRolesMixedExistingAdApplication(t *testing.T) { }) } } + +func TestCheckDirectoryRolesGraphPermissionsSatisfyAll(t *testing.T) { + // no directory role at all, both capabilities held as Graph app permissions + p := preflightWithGraphPermissions([]string{ + GraphApplicationReadWriteAllPermission, + GraphRoleManagementReadWriteDirectoryPermission, + }, Config, ActivityLog, Agentless) + assert.NoError(t, CheckDirectoryRoles(p)) + assert.Empty(t, p.errors) +} + +func TestCheckDirectoryRolesGraphPermissionsPartial(t *testing.T) { + tests := []struct { + name string + permissions []string + configErrors int + agentlessErrs int + wantError string + }{ + { + name: "app creation only, cannot assign the directory role", + permissions: []string{GraphApplicationReadWriteAllPermission}, + configErrors: 1, + agentlessErrs: 0, + wantError: "Privileged Role Administrator", + }, + { + name: "owned-by variant also creates applications", + permissions: []string{GraphApplicationReadWriteOwnedByPermission}, + configErrors: 1, + agentlessErrs: 0, + wantError: "Privileged Role Administrator", + }, + { + name: "role assignment only, cannot create the application", + permissions: []string{GraphRoleManagementReadWriteDirectoryPermission}, + configErrors: 1, + agentlessErrs: 1, + wantError: "Application Administrator", + }, + { + name: "an unrelated permission satisfies nothing", + permissions: []string{"Directory.Read.All"}, + configErrors: 2, + agentlessErrs: 1, + wantError: "Application Administrator", + }, + } + + for _, test := range tests { + t.Run(test.name, func(t *testing.T) { + p := preflightWithGraphPermissions(test.permissions, Config, Agentless) + assert.NoError(t, CheckDirectoryRoles(p)) + + assert.Len(t, p.errors[Config], test.configErrors) + assert.Len(t, p.errors[Agentless], test.agentlessErrs) + assert.Contains(t, p.errors[Config][0], test.wantError) + }) + } +} + +func TestCheckDirectoryRolesMixedRoleAndGraphPermission(t *testing.T) { + // the case the change exists for: one capability from a directory role, the + // other from a Graph application permission + p := preflightWithRoles([]string{ApplicationAdministratorRoleID}, nil, Config, Agentless) + p.caller.GraphPermissions = []string{GraphRoleManagementReadWriteDirectoryPermission} + assert.NoError(t, CheckDirectoryRoles(p)) + assert.Empty(t, p.errors) + + // and the reverse direction + p = preflightWithRoles([]string{PrivilegedRoleAdministratorRoleID}, nil, Config, Agentless) + p.caller.GraphPermissions = []string{GraphApplicationReadWriteOwnedByPermission} + assert.NoError(t, CheckDirectoryRoles(p)) + assert.Empty(t, p.errors) +} + +func TestCheckDirectoryRolesUnreadableGraphPermissions(t *testing.T) { + p := preflightWithRoles(nil, nil, Agentless) + p.graphPermissionsErr = errors.New("AADSTS900023: tenant not found") + assert.NoError(t, CheckDirectoryRoles(p)) + + require.Len(t, p.errors[Agentless], 1) + // the caller learns the second path was never looked at, so the missing + // directory role is not reported as the whole story + assert.Contains(t, p.errors[Agentless][0], "could not be read") + assert.Contains(t, p.errors[Agentless][0], "AADSTS900023") + + // nothing appended when the permissions were read fine + p = preflightWithRoles(nil, nil, Agentless) + assert.NoError(t, CheckDirectoryRoles(p)) + require.Len(t, p.errors[Agentless], 1) + assert.NotContains(t, p.errors[Agentless][0], "could not be read") +}