groups: select only id when listing members and owners - #1911
Open
orgads wants to merge 1 commit into
Open
Conversation
The group data source and the group/group_member/group_without_members resources read members and owners purely for their object IDs, but ask Graph for the full directory object projection. Request $select=id instead. For a group of 39 users that takes the response from 819KB, at 79 properties per member, down to 694 bytes -- nearly all of which the callers discarded, since each keeps only the id. It applies to every read of a group's members and owners, resource refreshes included. The unfiltered projection was also what exposed the provider to a Graph beta serialization bug (hashicorp#1910): for app-only callers lacking User.Read.All, every user property came back null, including the non-nullable isProvisionedToOnPremises of onPremisesProvisioningState, so Graph aborted mid-object and appended an error to the bytes it had already sent, leaving a body that failed to unmarshal. Microsoft have since fixed that, so this is no longer a fix for hashicorp#1910 -- but not asking for properties the provider never reads is what would have kept it clear of a server-side bug in a projection it has no use for. Verified against two security groups (40 and 7 members) with an app-only token: members and owners come back identical to the unfiltered request. @odata.type is still returned under $select=id, so the polymorphic directory object unmarshalling is unaffected. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
orgads
force-pushed
the
fix/group-members-select
branch
from
August 7, 2026 10:12
b006a16 to
a56c771
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Community Note
Description
The group data source and the group / group_member / group_without_members resources read members
and owners purely for their object IDs — every consumer is
pointer.From(object.DirectoryObject().Id)— but they ask Graph for the full directory objectprojection. This requests
$select=idinstead, through three small helpers next to the existinggroupGetMemberininternal/services/groups/groups.go.The payload difference is substantial. For a group of 39 users:
/beta/groups/{id}/members/beta/groups/{id}/members?$select=idThat is ~1000x less to transfer and unmarshal, for a data source that then discards all of it
except one field per member. It applies on every read of a group's
membersandowners,including refreshes of
azuread_groupresources.How I got here (#1910): the full projection was also what tripped a Microsoft Graph beta
serialization bug for app-only callers lacking
User.Read.All. Every user property in thatrestricted projection comes back
null, including the non-nullableisProvisionedToOnPremisesofmicrosoft.graph.onPremisesProvisioningState, so Graph aborted mid-object and appended an error tothe bytes it had already sent, which the provider surfaced as:
Microsoft have since fixed that serialization failure, so this PR is no longer needed to fix
#1910 — I have confirmed the failure no longer reproduces and said so on the issue. I still think
the change is worth taking on the two remaining grounds: the payload saving above, and not asking
Graph for properties the provider never reads, which is what exposed it to a server-side bug in a
projection it had no use for.
@odata.typeis still returned under$select=id, so the polymorphic directory-objectunmarshalling is unaffected — responses come back as
{"@odata.type": "...", "id": "..."}.Changes to existing Resource / Data Source
members/ownersvalues this code path produces. Happy to add coverage if you can suggest a shape for asserting$selectthat fits the suite.Testing
Unit tests, build and vet pass:
I do not have a tenant I can point the acceptance tests at. Instead I verified against a real
tenant with a
dev_overridesbuild, reading two security groups (40 and 7 members) with anapp-only service principal, both with
include_transitive_members = falseand via theownerspath:
The member and owner IDs are identical to what the unfiltered request returns for the same groups,
i.e. narrowing the projection loses nothing the data source exposes. Note this is no longer a
before/after comparison: when I first ran it, these two reads failed on released 3.9.0 with the
error quoted above and succeeded with this change — but with Microsoft's fix in place, stock 3.9.0
now reads them successfully too.
Change Log
azuread_group- request only theidproperty when reading members and owners, greatly reducing the response size [data.azuread_group: members read fails with "invalid character '{' after object key:value pair" (Graph beta returns a truncated body for app-only tokens without User.Read.All) #1910]azuread_group_member- request only theidproperty when listing existing members [data.azuread_group: members read fails with "invalid character '{' after object key:value pair" (Graph beta returns a truncated body for app-only tokens without User.Read.All) #1910]azuread_group_without_members- request only theidproperty when reading owners [data.azuread_group: members read fails with "invalid character '{' after object key:value pair" (Graph beta returns a truncated body for app-only tokens without User.Read.All) #1910]This is a (please select all that apply):
Related Issue(s)
Relates to #1910
Rollback Plan
If a change needs to be reverted, we will publish an updated version of the provider.
Changes to Security Controls
No changes to security controls. The requests are narrowed, not widened: the provider now asks
Graph for strictly fewer properties than before, and no permission requirements change.