diff --git a/README.md b/README.md index 6b4cdfd..067dee9 100644 --- a/README.md +++ b/README.md @@ -163,6 +163,40 @@ spec: name: azure-account-creds ``` +#### Only Active (Enabled) Accounts + +By default a user is validated on existence alone, so a disabled (for example +offboarded) account still passes. Set `activeAccount: true` to additionally +require the Entra ID `accountEnabled` attribute to be true, omitting disabled +users from the result: + +```yaml + queryType: UserValidation + users: + - "user1@yourdomain.com" + - "user2@yourdomain.com" + target: "status.validatedUsers" + activeAccount: true +``` + +Every validated user carries its `accountEnabled` value in the result, +regardless of whether `activeAccount` is set, so the account state is visible to +downstream consumers either way. A user for which Microsoft Graph returns no +`accountEnabled` value is treated as disabled: when the account state cannot be +confirmed, `activeAccount: true` excludes the user. + +> Note: `activeAccount` only applies to the `UserValidation` query type, and it +> filters the result of a query. It does not retroactively purge users from a +> target that is no longer being refreshed, so combining it with +> `skipQueryWhenTargetHasData: true` on an existing XR leaves any already-stored +> disabled users in place. Clear the target once (or drop the skip for one +> reconcile) when enabling the flag on an existing deployment. `queryInterval` +> has the same effect, bounded by the interval. + +> Note: consumers whose XRD constrains the target with a structural item schema +> (rather than `x-kubernetes-preserve-unknown-fields`) need to add +> `accountEnabled` to that schema, otherwise the API server silently prunes it. + ### Get Group Membership ```yaml @@ -276,7 +310,8 @@ spec: | `target` | string | Required. Where to store the query results. Can be `status.` or `context.` | | `skipQueryWhenTargetHasData` | bool | Optional. When true, will skip the query if the target already has data | | `queryInterval` | string | Optional. Minimum interval between queries as a Go duration string (e.g. `10m`, `1h`, `90s`). Skips querying Microsoft Graph until the interval has elapsed since the last successful query, independent of reconcile frequency. Only effective in Composition mode with a `status.` target. | -| `FailOnEmpty` | bool | Optional. When true, the function will fail if the `users`, `groups`, or `servicePrincipals` lists are empty, or if their respective reference fields are empty lists. | +| `failOnEmpty` | bool | Optional. When true, the function will fail if the `users`, `groups`, or `servicePrincipals` lists are empty, or if their respective reference fields are empty lists. | +| `activeAccount` | bool | Optional. `UserValidation` only. When true, only users whose Entra ID `accountEnabled` attribute is true are stored at the target; disabled users, and users whose account state Graph does not report, are omitted. | | `identity.type` | string | Optional. Type of identity credentials to use. Valid values: `AzureServicePrincipalCredentials`, `AzureWorkloadIdentityCredentials`. Default is `AzureServicePrincipalCredentials` | ## Result Targets @@ -336,6 +371,7 @@ status: displayName: "Jane Doe" userPrincipalName: "jane@example.com" mail: "jane@example.com" + accountEnabled: true lastQueryTimestamps: # separate metadata, keyed by target validatedUsers: "2026-07-16T10:00:00Z" ``` diff --git a/example/README.md b/example/README.md index 70f8657..2c81b37 100644 --- a/example/README.md +++ b/example/README.md @@ -141,3 +141,19 @@ crossplane render xr-with-last-query-time.yaml user-validation-example-query-int > `xr-with-last-query-time.yaml` uses a far-future timestamp in `status.lastQueryTimestamps` so the skip is deterministic; set it to a real recent time to test the natural elapsed boundary. The credentials are not used on the skip path (no Graph call is made), so they need not be valid for this command. > **macOS note:** these commands use `-r` (function results) rather than `-rc`. The `-c`/`--include-context` flag makes `crossplane render` v2.x run an internal context-extraction step over a unix socket bind-mounted into its Docker helper container, which Docker Desktop for macOS does not support (`connect: operation not supported`) — the render then hangs with no output. Since the query-interval results are written to a `status.` target, `-c` is unnecessary here. This is a known CLI bug ([crossplane/cli#161](https://github.com/crossplane/cli/issues/161)), fixed by [#163](https://github.com/crossplane/cli/pull/163) (context function now listens on TCP) but not yet in a tagged release as of CLI v2.4.0. Until then, drop `-c` on macOS, or run `crossplane render` on Linux / a CLI built from `main` if you need context output. + +### 6. Active Account Filtering + +Restrict `UserValidation` results to enabled accounts with `activeAccount: true`. Users whose Entra ID `accountEnabled` attribute is not true are omitted from the target. Every result carries its `accountEnabled` value regardless of the flag, so the two renders below can be compared directly (both write to a `status.` target, so `-r` is sufficient, see the macOS note above): + +```shell +crossplane render xr.yaml user-validation-example-active-account.yaml functions.yaml --function-credentials=./secrets/azure-creds.yaml -r +``` + +Disabled users are absent from `status.validatedUsers`, and every remaining entry has `accountEnabled: true`. Render the plain example for the unfiltered baseline, where a disabled user is still listed, with `accountEnabled: false`: + +```shell +crossplane render xr.yaml user-validation-example.yaml functions.yaml --function-credentials=./secrets/azure-creds.yaml -r +``` + +> The example intentionally omits `skipQueryWhenTargetHasData`. Enabling `activeAccount` on an XR whose target already holds disabled users does not purge them while the query is being skipped, so clear the target once (or drop the skip for one reconcile) when adopting the flag on an existing deployment. diff --git a/example/e2e/README.md b/example/e2e/README.md index 20228ad..ef86214 100644 --- a/example/e2e/README.md +++ b/example/e2e/README.md @@ -1,22 +1,24 @@ -# queryInterval e2e (Composition mode) +# function-msgraph e2e (Composition mode) -Manifests to exercise the `queryInterval` throttling feature end-to-end on a real -cluster, where the time-based skip/refresh loop can actually be observed across -reconciles (something `crossplane render` cannot show). +Manifests to exercise features end-to-end on a real cluster, where behaviour +across reconciles can actually be observed (something `crossplane render` cannot +show). ## Files | File | Purpose | |------|---------| -| `function.yaml` | Installs `function-msgraph` (pin the tag you want to test) | +| `function.yaml` | Installs `function-msgraph` (pin the tag you want to test) with `--debug` enabled | | `composition.yaml` | UserValidation → `status.validatedUsers`, `queryInterval: "2m"` | -| `xr.yaml` | A composite resource instance | +| `xr.yaml` | A composite resource instance for the `queryInterval` scenario | +| `composition-active-account.yaml` | Two UserValidation Compositions, identical except for `activeAccount: true` | +| `xr-active-account.yaml` | One composite resource instance per `activeAccount` Composition | Reuses `../definition.yaml` (XRD) and the `azure-account-creds` secret built from `../secrets/azure-creds.yaml` — see [Update Credentials](../README.md#update-credentials). Populate it locally; keep real values out of commits. -## Run +## Cluster ```shell kind create cluster --name msgraph-e2e @@ -27,12 +29,15 @@ kubectl apply -f example/e2e/function.yaml kubectl wait function.pkg.crossplane.io/function-msgraph --for=condition=Healthy --timeout=180s kubectl apply -f example/definition.yaml -kubectl apply -f example/e2e/composition.yaml kubectl apply -f example/secrets/azure-creds.yaml # populate locally; keep real values out of commits -kubectl apply -f example/e2e/xr.yaml ``` -## Verify +## Scenario: queryInterval + +```shell +kubectl apply -f example/e2e/composition.yaml +kubectl apply -f example/e2e/xr.yaml +``` ```shell # the result list stays a clean list of results @@ -54,9 +59,40 @@ Force reconciles with `kubectl annotate xr msgraph-query-interval-e2e poke=$(dat Poking repeatedly inside the 2m window leaves the query count flat; after 2m elapses the next reconcile re-queries and `status.lastQueryTimestamps.validatedUsers` advances. +## Scenario: activeAccount + +Edit `composition-active-account.yaml` and `xr-active-account.yaml` first: replace +the placeholder UPNs with one enabled and one deliberately disabled account from +your directory, otherwise the filter has nothing to exclude. + +```shell +kubectl apply -f example/e2e/composition-active-account.yaml +kubectl apply -f example/e2e/xr-active-account.yaml +``` + +```shell +# baseline: the disabled account passes validation, reported as accountEnabled false +kubectl get xr msgraph-baseline-e2e -o jsonpath='{.status.validatedUsers}' | jq + +# activeAccount: only the enabled account is stored +kubectl get xr msgraph-active-account-e2e -o jsonpath='{.status.validatedUsers}' | jq + +# the exclusion is logged per user +POD=$(kubectl get pods -n crossplane-system -o name | grep msgraph) +kubectl logs -n crossplane-system "$POD" | grep 'Skipping user with disabled account' +``` + +Both XRs should report `SYNCED=True READY=True` with a `FunctionSuccess/Success` +condition. Poke either XR to confirm the filtering holds across reconciles rather +than being a first-pass artefact. + ## Notes - `queryInterval` is effective only in Composition mode with a `status.` target. +- `activeAccount` filters the result of a query, it does not purge a target that + is no longer being refreshed. Combining it with `skipQueryWhenTargetHasData` or + `queryInterval` leaves already-stored disabled users in place, which is why the + `activeAccount` Composition sets neither. - The example XRD resolves to `LegacyCluster` scope under Crossplane v2, so the XR is cluster-scoped (no namespace). - Teardown: `kind delete cluster --name msgraph-e2e`. diff --git a/example/e2e/composition-active-account.yaml b/example/e2e/composition-active-account.yaml new file mode 100644 index 0000000..f0dd38e --- /dev/null +++ b/example/e2e/composition-active-account.yaml @@ -0,0 +1,65 @@ +--- +# Baseline: existence check only. A disabled account still passes validation, +# which is the behaviour activeAccount exists to change. Kept alongside the +# filtered Composition so the two can be compared in the same cluster. +apiVersion: apiextensions.crossplane.io/v1 +kind: Composition +metadata: + name: user-validation-baseline-e2e +spec: + compositeTypeRef: + apiVersion: example.crossplane.io/v1 + kind: XR + mode: Pipeline + pipeline: + - step: validate-user + functionRef: + name: function-msgraph + input: + apiVersion: msgraph.fn.crossplane.io/v1alpha1 + kind: Input + queryType: UserValidation + # Replace with real accounts from your directory: one enabled, one + # deliberately disabled, otherwise there is nothing for the filter to do. + users: + - "user@example.onmicrosoft.com" + - "disabled-user@example.onmicrosoft.com" + target: "status.validatedUsers" + credentials: + - name: azure-creds + source: Secret + secretRef: + namespace: crossplane-system + name: azure-account-creds +--- +# Same query, restricted to enabled accounts. Note that neither +# skipQueryWhenTargetHasData nor queryInterval is set: both suppress the query, +# so an already-populated target would keep its disabled entries. +apiVersion: apiextensions.crossplane.io/v1 +kind: Composition +metadata: + name: user-validation-active-account-e2e +spec: + compositeTypeRef: + apiVersion: example.crossplane.io/v1 + kind: XR + mode: Pipeline + pipeline: + - step: validate-user + functionRef: + name: function-msgraph + input: + apiVersion: msgraph.fn.crossplane.io/v1alpha1 + kind: Input + queryType: UserValidation + users: + - "user@example.onmicrosoft.com" + - "disabled-user@example.onmicrosoft.com" + target: "status.validatedUsers" + activeAccount: true + credentials: + - name: azure-creds + source: Secret + secretRef: + namespace: crossplane-system + name: azure-account-creds diff --git a/example/e2e/function.yaml b/example/e2e/function.yaml index 9b9844c..f1d5648 100644 --- a/example/e2e/function.yaml +++ b/example/e2e/function.yaml @@ -1,7 +1,27 @@ --- +# Run the function with --debug so per-user decisions are visible in the pod +# logs (for example the accounts excluded by activeAccount, and the queryInterval +# skips). +apiVersion: pkg.crossplane.io/v1beta1 +kind: DeploymentRuntimeConfig +metadata: + name: msgraph-debug +spec: + deploymentTemplate: + spec: + selector: {} + template: + spec: + containers: + - name: package-runtime + args: + - --debug +--- apiVersion: pkg.crossplane.io/v1 kind: Function metadata: name: function-msgraph spec: - package: xpkg.upbound.io/upbound/function-msgraph:v0.7.0-rc4 + package: xpkg.upbound.io/upbound/function-msgraph:v0.8.0-rc2 + runtimeConfigRef: + name: msgraph-debug diff --git a/example/e2e/xr-active-account.yaml b/example/e2e/xr-active-account.yaml new file mode 100644 index 0000000..8611f1c --- /dev/null +++ b/example/e2e/xr-active-account.yaml @@ -0,0 +1,24 @@ +--- +apiVersion: example.crossplane.io/v1 +kind: XR +metadata: + name: msgraph-baseline-e2e +spec: + compositionRef: + name: user-validation-baseline-e2e + userAccess: + emails: + - "user@example.onmicrosoft.com" + - "disabled-user@example.onmicrosoft.com" +--- +apiVersion: example.crossplane.io/v1 +kind: XR +metadata: + name: msgraph-active-account-e2e +spec: + compositionRef: + name: user-validation-active-account-e2e + userAccess: + emails: + - "user@example.onmicrosoft.com" + - "disabled-user@example.onmicrosoft.com" diff --git a/example/user-validation-example-active-account.yaml b/example/user-validation-example-active-account.yaml new file mode 100644 index 0000000..9c8fa1b --- /dev/null +++ b/example/user-validation-example-active-account.yaml @@ -0,0 +1,40 @@ +apiVersion: apiextensions.crossplane.io/v1 +kind: Composition +metadata: + name: user-validation-example-active-account +# Important: This function example requires an Azure AD app registration with Microsoft Graph API permissions: +# - User.Read.All +# - Directory.Read.All +spec: + compositeTypeRef: + apiVersion: example.crossplane.io/v1 + kind: XR + mode: Pipeline + pipeline: + - step: validate-user + functionRef: + name: function-msgraph + input: + apiVersion: msgraph.fn.crossplane.io/v1alpha1 + kind: Input + queryType: UserValidation + # Replace these with actual users in your directory. To see the filter + # take effect, include at least one account that is disabled in Entra ID. + users: + - "admin@example.onmicrosoft.com" + - "user@example.onmicrosoft.com" + - "yury@upbound.io" + target: "status.validatedUsers" + # Only users whose accountEnabled attribute is true are stored at the + # target. Disabled users, and users whose account state Microsoft Graph + # does not report, are omitted. + activeAccount: true + # Note: skipQueryWhenTargetHasData is deliberately not set here, so every + # render re-queries Graph. With the skip enabled, a target that already + # holds disabled users is never refreshed and they would persist. + credentials: + - name: azure-creds + source: Secret + secretRef: + namespace: crossplane-system + name: azure-account-creds diff --git a/fn.go b/fn.go index df9a5ed..e6401ff 100644 --- a/fn.go +++ b/fn.go @@ -72,6 +72,7 @@ const ( fieldMail = "mail" fieldUserPrincipalName = "userPrincipalName" fieldType = "type" + fieldAccountEnabled = "accountEnabled" ) const ( @@ -528,6 +529,7 @@ func (g *GraphQuery) validateUsers(ctx context.Context, client *msgraphsdk.Graph } results := make([]interface{}, 0) + requireActiveAccount := ptr.Deref(in.ActiveAccount, false) for _, userPrincipalName := range in.Users { if userPrincipalName == nil { @@ -543,8 +545,9 @@ func (g *GraphQuery) validateUsers(ctx context.Context, client *msgraphsdk.Graph filterValue := fmt.Sprintf("userPrincipalName eq '%s'", *userPrincipalName) requestConfig.QueryParameters.Filter = &filterValue - // Use standard fields for user validation - requestConfig.QueryParameters.Select = []string{"id", fieldDisplayName, fieldUserPrincipalName, fieldMail} + // Use standard fields for user validation. accountEnabled is returned by + // Microsoft Graph only when it is explicitly selected. + requestConfig.QueryParameters.Select = []string{"id", fieldDisplayName, fieldUserPrincipalName, fieldMail, fieldAccountEnabled} // Execute the query result, err := client.Users().Get(ctx, requestConfig) @@ -553,22 +556,44 @@ func (g *GraphQuery) validateUsers(ctx context.Context, client *msgraphsdk.Graph } // Process results - if result.GetValue() != nil { - for _, user := range result.GetValue() { - userMap := map[string]interface{}{ - "id": ptr.Deref(user.GetId(), ""), - fieldDisplayName: ptr.Deref(user.GetDisplayName(), ""), - fieldUserPrincipalName: ptr.Deref(user.GetUserPrincipalName(), ""), - fieldMail: ptr.Deref(user.GetMail(), ""), - } - results = append(results, userMap) - } - } + results = append(results, g.buildUserResults(result.GetValue(), requireActiveAccount)...) } return results, nil } +// buildUserResults converts the users returned by a single UserValidation lookup +// into result maps. When requireActiveAccount is set, users whose accountEnabled +// attribute is not true are omitted. A nil attribute is treated as disabled, +// because Microsoft Graph returns accountEnabled only when it is explicitly +// selected, so an absent value means the account state is unconfirmed. +func (g *GraphQuery) buildUserResults(graphUsers []models.Userable, requireActiveAccount bool) []interface{} { + results := make([]interface{}, 0, len(graphUsers)) + + for _, user := range graphUsers { + if user == nil { + continue + } + + accountEnabled := ptr.Deref(user.GetAccountEnabled(), false) + if requireActiveAccount && !accountEnabled { + g.log.Debug("Skipping user with disabled account", + fieldUserPrincipalName, ptr.Deref(user.GetUserPrincipalName(), "")) + continue + } + + results = append(results, map[string]interface{}{ + "id": ptr.Deref(user.GetId(), ""), + fieldDisplayName: ptr.Deref(user.GetDisplayName(), ""), + fieldUserPrincipalName: ptr.Deref(user.GetUserPrincipalName(), ""), + fieldMail: ptr.Deref(user.GetMail(), ""), + fieldAccountEnabled: accountEnabled, + }) + } + + return results +} + // findGroupByName finds a group by its display name and returns its ID func (g *GraphQuery) findGroupByName(ctx context.Context, client *msgraphsdk.GraphServiceClient, groupName string) (*string, error) { // Create filter by displayName diff --git a/fn_test.go b/fn_test.go index 9b53a3e..27c6cd8 100644 --- a/fn_test.go +++ b/fn_test.go @@ -26,6 +26,8 @@ const ( testCredentialsKey = "credentials" testAzureCredsName = "azure-creds" testSPID1 = "sp-id-1" + testUserID1 = "user-id-1" + testUserDisplay = "Test User" testUser1Email = "user1@example.com" testUser2Email = "user2@example.com" watchedResourceKey = "ops.crossplane.io/watched-resource" @@ -987,7 +989,7 @@ func TestResolveGroupRef(t *testing.T) { } return []interface{}{ map[string]interface{}{ - "id": "user-id-1", + "id": testUserID1, fieldDisplayName: "Test User 1", fieldMail: testUser1Email, fieldUserPrincipalName: testUser1Email, @@ -1586,7 +1588,7 @@ func TestResolveUsersRef(t *testing.T) { // Generate different test data based on user principal name switch *user { case testUser1Email: - userID = "user-id-1" + userID = testUserID1 displayName = "User 1" case testUser2Email: userID = "user-id-2" @@ -1596,7 +1598,7 @@ func TestResolveUsersRef(t *testing.T) { displayName = "Admin User" default: userID = "test-user-id" - displayName = "Test User" + displayName = testUserDisplay } userMap := map[string]interface{}{ @@ -2488,6 +2490,77 @@ func TestRunFunction(t *testing.T) { }, }, }, + "UserValidationActiveAccount": { + reason: "The Function should pass activeAccount through to the query and store only the enabled users", + args: args{ + ctx: context.Background(), + req: &fnv1.RunFunctionRequest{ + Meta: &fnv1.RequestMeta{Tag: testRequestTag}, + Input: resource.MustStructJSON(`{ + "apiVersion": "msgraph.fn.crossplane.io/v1alpha1", + "kind": "Input", + "queryType": "UserValidation", + "users": ["user@example.com", "disabled@example.com"], + "activeAccount": true, + "target": "status.validatedUsers" + }`), + Observed: &fnv1.State{ + Composite: &fnv1.Resource{ + Resource: resource.MustStructJSON(xr), + }, + }, + Credentials: map[string]*fnv1.Credentials{ + testAzureCredsName: { + Source: &fnv1.Credentials_CredentialData{CredentialData: creds}, + }, + }, + }, + }, + want: want{ + rsp: &fnv1.RunFunctionResponse{ + Meta: &fnv1.ResponseMeta{Tag: testRequestTag, Ttl: durationpb.New(response.DefaultTTL)}, + Conditions: []*fnv1.Condition{ + { + Type: condTypeFunctionSuccess, + Status: fnv1.Status_STATUS_CONDITION_TRUE, + Reason: condReasonSuccess, + Target: fnv1.Target_TARGET_COMPOSITE_AND_CLAIM.Enum(), + }, + }, + Results: []*fnv1.Result{ + { + Severity: fnv1.Severity_SEVERITY_NORMAL, + Message: msgUserValidationQueryType, + Target: fnv1.Target_TARGET_COMPOSITE.Enum(), + }, + }, + Desired: &fnv1.State{ + Composite: &fnv1.Resource{ + Resource: resource.MustStructJSON(`{ + "apiVersion": "example.org/v1", + "kind": "XR", + "metadata": { + "name": "cool-xr" + }, + "spec": { + "count": 2 + }, + "status": { + "validatedUsers": [ + { + "id": "test-user-id", + "displayName": "Test User", + "userPrincipalName": "user@example.com", + "mail": "user@example.com", + "accountEnabled": true + } + ] + }}`), + }, + }, + }, + }, + }, "GroupMembershipMissingGroup": { reason: "The Function should handle GroupMembership with missing group", args: args{ @@ -3912,10 +3985,23 @@ func TestRunFunction(t *testing.T) { if len(in.Users) == 0 { return nil, errors.New("no users provided for validation") } + if ptr.Deref(in.ActiveAccount, false) { + // Mirrors buildUserResults: the disabled user is dropped + // and the remaining one carries accountEnabled. + return []interface{}{ + map[string]interface{}{ + "id": "test-user-id", + fieldDisplayName: testUserDisplay, + fieldUserPrincipalName: "user@example.com", + fieldMail: "user@example.com", + fieldAccountEnabled: true, + }, + }, nil + } return []interface{}{ map[string]interface{}{ "id": "test-user-id", - fieldDisplayName: "Test User", + fieldDisplayName: testUserDisplay, fieldUserPrincipalName: "user@example.com", fieldMail: "user@example.com", }, @@ -3926,7 +4012,7 @@ func TestRunFunction(t *testing.T) { } return []interface{}{ map[string]interface{}{ - "id": "user-id-1", + "id": testUserID1, fieldDisplayName: "Test User 1", fieldMail: testUser1Email, fieldUserPrincipalName: testUser1Email, @@ -4213,7 +4299,7 @@ func TestIdentityType(t *testing.T) { // in additionalData). func newTestUser() models.DirectoryObjectable { user := models.NewUser() - user.SetId(ptr.To("user-id-1")) + user.SetId(ptr.To(testUserID1)) user.SetDisplayName(ptr.To("Test User 1")) user.SetMail(ptr.To(testUser1Email)) user.SetUserPrincipalName(ptr.To(testUser1Email)) @@ -4254,6 +4340,158 @@ func newTestDirectoryObject() models.DirectoryObjectable { return do } +// newTestGraphUser builds a typed Graph user as returned by a UserValidation +// lookup, with the accountEnabled attribute set to the given value. +func newTestGraphUser(id, upn string, accountEnabled bool) models.Userable { + user := models.NewUser() + user.SetId(ptr.To(id)) + user.SetDisplayName(ptr.To(testUserDisplay)) + user.SetMail(ptr.To(upn)) + user.SetUserPrincipalName(ptr.To(upn)) + user.SetAccountEnabled(ptr.To(accountEnabled)) + return user +} + +// newTestGraphUserWithoutAccountEnabled builds a typed Graph user whose +// accountEnabled attribute is absent, as happens when the property is not +// projected via $select or is withheld from the caller. +func newTestGraphUserWithoutAccountEnabled() models.Userable { + user := models.NewUser() + user.SetId(ptr.To("user-id-unknown")) + user.SetDisplayName(ptr.To(testUserDisplay)) + user.SetMail(ptr.To("unknown@example.com")) + user.SetUserPrincipalName(ptr.To("unknown@example.com")) + // accountEnabled intentionally left unset. + return user +} + +// newTestGraphUserMinimal builds a typed Graph user carrying only an id and an +// accountEnabled attribute, exercising the empty-string defaults. +func newTestGraphUserMinimal() models.Userable { + user := models.NewUser() + user.SetId(ptr.To("user-id-minimal")) + user.SetAccountEnabled(ptr.To(true)) + return user +} + +// TestBuildUserResults exercises the real UserValidation result-building path +// (bypassed by the mocked graphQuery in the RunFunction tests), including the +// accountEnabled filtering enabled by the activeAccount input. +func TestBuildUserResults(t *testing.T) { + cases := map[string]struct { + reason string + graphUsers []models.Userable + requireActiveAccount bool + want []interface{} + }{ + "FlagOffKeepsDisabledUsers": { + reason: "Without activeAccount a disabled user is kept and reported as accountEnabled false", + graphUsers: []models.Userable{ + newTestGraphUser(testUserID1, testUser1Email, true), + newTestGraphUser("user-id-2", testUser2Email, false), + }, + requireActiveAccount: false, + want: []interface{}{ + map[string]interface{}{ + "id": testUserID1, + fieldDisplayName: testUserDisplay, + fieldUserPrincipalName: testUser1Email, + fieldMail: testUser1Email, + fieldAccountEnabled: true, + }, + map[string]interface{}{ + "id": "user-id-2", + fieldDisplayName: testUserDisplay, + fieldUserPrincipalName: testUser2Email, + fieldMail: testUser2Email, + fieldAccountEnabled: false, + }, + }, + }, + "FlagOnDropsDisabledUsers": { + reason: "With activeAccount only the enabled user is returned", + graphUsers: []models.Userable{ + newTestGraphUser(testUserID1, testUser1Email, true), + newTestGraphUser("user-id-2", testUser2Email, false), + }, + requireActiveAccount: true, + want: []interface{}{ + map[string]interface{}{ + "id": testUserID1, + fieldDisplayName: testUserDisplay, + fieldUserPrincipalName: testUser1Email, + fieldMail: testUser1Email, + fieldAccountEnabled: true, + }, + }, + }, + "FlagOnNilAccountEnabledExcluded": { + reason: "With activeAccount an unconfirmed account state must fail closed and be excluded", + graphUsers: []models.Userable{newTestGraphUserWithoutAccountEnabled()}, + requireActiveAccount: true, + want: []interface{}{}, + }, + "FlagOffNilAccountEnabledReportedFalse": { + reason: "Without activeAccount an absent accountEnabled attribute is reported as false", + graphUsers: []models.Userable{newTestGraphUserWithoutAccountEnabled()}, + requireActiveAccount: false, + want: []interface{}{ + map[string]interface{}{ + "id": "user-id-unknown", + fieldDisplayName: testUserDisplay, + fieldUserPrincipalName: "unknown@example.com", + fieldMail: "unknown@example.com", + fieldAccountEnabled: false, + }, + }, + }, + "EmptyInputReturnsEmptySlice": { + reason: "A lookup that matched no user returns an empty list rather than nil", + graphUsers: nil, + requireActiveAccount: true, + want: []interface{}{}, + }, + "NilUserSkipped": { + reason: "A nil entry in the returned collection is skipped", + graphUsers: []models.Userable{nil, newTestGraphUser(testUserID1, testUser1Email, true)}, + requireActiveAccount: true, + want: []interface{}{ + map[string]interface{}{ + "id": testUserID1, + fieldDisplayName: testUserDisplay, + fieldUserPrincipalName: testUser1Email, + fieldMail: testUser1Email, + fieldAccountEnabled: true, + }, + }, + }, + "MissingOptionalFieldsDefaultToEmptyString": { + reason: "Absent displayName and mail attributes are reported as empty strings", + graphUsers: []models.Userable{newTestGraphUserMinimal()}, + requireActiveAccount: true, + want: []interface{}{ + map[string]interface{}{ + "id": "user-id-minimal", + fieldDisplayName: "", + fieldUserPrincipalName: "", + fieldMail: "", + fieldAccountEnabled: true, + }, + }, + }, + } + + for name, tc := range cases { + t.Run(name, func(t *testing.T) { + g := &GraphQuery{log: logging.NewNopLogger()} + got := g.buildUserResults(tc.graphUsers, tc.requireActiveAccount) + if diff := cmp.Diff(tc.want, got); diff != "" { + t.Errorf("%s\nbuildUserResults(...): -want, +got:\n%s", tc.reason, diff) + } + }) + } +} + // TestProcessMember exercises the real member-extraction path (bypassed by the // mocked graphQuery in the RunFunction tests). It is the regression test for // issue #115: typed user members must expose mail and userPrincipalName. @@ -4267,7 +4505,7 @@ func TestProcessMember(t *testing.T) { reason: "A typed user member should expose mail and userPrincipalName from the typed getters", member: newTestUser(), want: map[string]interface{}{ - "id": "user-id-1", + "id": testUserID1, fieldDisplayName: "Test User 1", fieldType: userType, fieldMail: testUser1Email, diff --git a/input/v1beta1/input.go b/input/v1beta1/input.go index e290b13..252be57 100644 --- a/input/v1beta1/input.go +++ b/input/v1beta1/input.go @@ -86,6 +86,15 @@ type Input struct { // +optional FailOnEmpty *bool `json:"failOnEmpty,omitempty"` + // ActiveAccount restricts UserValidation results to users whose Entra ID + // accountEnabled attribute is true. Disabled users, and users for which + // Microsoft Graph returns no accountEnabled value, are omitted from the + // result written to the target. + // If false or unset, users are validated on existence alone. + // Only applies to the UserValidation query type. + // +optional + ActiveAccount *bool `json:"activeAccount,omitempty"` + // Identity defines the type of identity used for authentication to the Microsoft Graph API. Identity *Identity `json:"identity,omitempty"` } diff --git a/input/v1beta1/zz_generated.deepcopy.go b/input/v1beta1/zz_generated.deepcopy.go index 0031c7b..603e7ab 100644 --- a/input/v1beta1/zz_generated.deepcopy.go +++ b/input/v1beta1/zz_generated.deepcopy.go @@ -101,6 +101,11 @@ func (in *Input) DeepCopyInto(out *Input) { *out = new(bool) **out = **in } + if in.ActiveAccount != nil { + in, out := &in.ActiveAccount, &out.ActiveAccount + *out = new(bool) + **out = **in + } if in.Identity != nil { in, out := &in.Identity, &out.Identity *out = new(Identity) diff --git a/package/input/msgraph.fn.crossplane.io_inputs.yaml b/package/input/msgraph.fn.crossplane.io_inputs.yaml index c1c25f5..2eae9e7 100644 --- a/package/input/msgraph.fn.crossplane.io_inputs.yaml +++ b/package/input/msgraph.fn.crossplane.io_inputs.yaml @@ -21,6 +21,15 @@ spec: openAPIV3Schema: description: Input can be used to provide input to this Function. properties: + activeAccount: + description: |- + ActiveAccount restricts UserValidation results to users whose Entra ID + accountEnabled attribute is true. Disabled users, and users for which + Microsoft Graph returns no accountEnabled value, are omitted from the + result written to the target. + If false or unset, users are validated on existence alone. + Only applies to the UserValidation query type. + type: boolean apiVersion: description: |- APIVersion defines the versioned schema of this representation of an object.