diff --git a/cli/cmd/preflight_azure.go b/cli/cmd/preflight_azure.go index 09a1d4c59..a0a154339 100644 --- a/cli/cmd/preflight_azure.go +++ b/cli/cmd/preflight_azure.go @@ -15,17 +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 - subscriptionID string - tenantID string - clientID string - clientSecret string - region string - } + preflightAzureState azurePreflightOptions preflightAzureCmd = &cobra.Command{ Use: "azure", @@ -51,6 +56,12 @@ 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 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", "", @@ -75,14 +86,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: existingAdApplicationsAzure(s), + SubscriptionID: s.subscriptionID, + TenantID: s.tenantID, + ClientID: s.clientID, + ClientSecret: s.clientSecret, + Region: s.region, } pf, err := azure.New(params) @@ -117,7 +129,9 @@ 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)) + cli.OutputHuman(" Graph application permissions: %d\n", len(result.Caller.GraphPermissions)) if len(integrations) > 0 { cli.OutputHuman("\nIntegrations checked: %s\n", strings.Join(integrations, ", ")) @@ -131,16 +145,14 @@ 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 -}) []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 53901bfdd..225a1b624 100644 --- a/cli/cmd/preflight_test.go +++ b/cli/cmd/preflight_test.go @@ -70,19 +70,46 @@ 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 - }{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 7766a324e..30bb8a1ff 100644 --- a/integration/test_resources/help/preflight_azure +++ b/integration/test_resources/help/preflight_azure @@ -10,15 +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 - -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) diff --git a/lwpreflight/azure/azure.go b/lwpreflight/azure/azure.go index 583ebd5b3..7b1c0b146 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" @@ -16,15 +17,19 @@ 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 map[IntegrationType]bool 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 } @@ -44,12 +49,17 @@ type Params struct { ClientID string ClientSecret string Region string + // 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) { integrationTypes := []IntegrationType{} tasks := []func(p *Preflight) error{ FetchCaller, + CheckDirectoryRoles, FetchPolicies, CheckPermissions, FetchDetails, @@ -95,14 +105,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: maps.Clone(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..e429e5e33 100644 --- a/lwpreflight/azure/caller.go +++ b/lwpreflight/azure/caller.go @@ -19,7 +19,18 @@ 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 + // 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 { @@ -45,17 +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, + 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 { @@ -97,10 +142,12 @@ 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"` + Roles []string `json:"roles"` } 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..1ae534fea --- /dev/null +++ b/lwpreflight/azure/caller_test.go @@ -0,0 +1,39 @@ +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) +} + +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 13a3c898d..8247c90de 100644 --- a/lwpreflight/azure/constants.go +++ b/lwpreflight/azure/constants.go @@ -8,6 +8,34 @@ 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" +) + +// 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 new file mode 100644 index 000000000..2e298cca9 --- /dev/null +++ b/lwpreflight/azure/directoryrole.go @@ -0,0 +1,82 @@ +package azure + +import ( + "fmt" + "slices" +) + +// 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") + + // 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 + } + } + return false + } + + canCreateApp := hasAny( + 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] + + // 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 || !usesExistingAdApplication + // Only the ad-application module (config/activity log, new app only) + // assigns the Directory Readers role. + assignsDirectoryRole := integrationType != Agentless && !usesExistingAdApplication + + if createsApp && !canCreateApp { + p.errors[integrationType] = append(p.errors[integrationType], fmt.Sprintf( + "Required Entra ID directory role missing: Application Administrator "+ + "(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 "+ + "(or the "+GraphRoleManagementReadWriteDirectoryPermission+ + " Microsoft Graph permission) to assign the Directory Readers role "+ + "to the Lacework Entra ID application for %s%s", + integrationType, unread)) + } + } + + return nil +} diff --git a/lwpreflight/azure/directoryrole_test.go b/lwpreflight/azure/directoryrole_test.go new file mode 100644 index 000000000..abb2c72c5 --- /dev/null +++ b/lwpreflight/azure/directoryrole_test.go @@ -0,0 +1,191 @@ +package azure + +import ( + "errors" + "testing" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" + + "github.com/lacework/go-sdk/v2/lwpreflight/verbosewriter" +) + +func preflightWithRoles( + roles []string, + useExistingAdApplication map[IntegrationType]bool, + types ...IntegrationType, +) *Preflight { + return &Preflight{ + integrationTypes: types, + useExistingAdApplication: useExistingAdApplication, + caller: Caller{DirectoryRoles: roles}, + errors: map[IntegrationType][]string{}, + verboseWriter: verbosewriter.New(), + } +} + +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)) + + // 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}, nil, 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}, nil, Config, ActivityLog, Agentless) + assert.NoError(t, CheckDirectoryRoles(p)) + assert.Empty(t, p.errors) +} + +func TestCheckDirectoryRolesExistingAdApplication(t *testing.T) { + 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 + 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) +} + +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) + }) + } +} + +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") +}