[FEAT] Support managing github_membership and lookups by stable user_id (#524)

[niklasbeinghaus]

Summary

• Adds optional user_id (TypeInt) to github_membership resource and data source, and to github_user data source — mutually exclusive with username via ExactlyOneOf.
• Memberships managed by user_id are now resilient to GitHub username renames: Read resolves the live login via GET /user/{id} and silently updates the username attribute in state, producing no diff.
• Resource ID for new memberships is org:user_id; legacy org:username IDs are migrated lazily on the next Read. Imports accept both shapes.

Closes #524.

Why

The GitHub id is globally unique and immutable; username is not. Today, when a user renames their account, modules using github_membership drift and trigger recreates. Issue #524 asks for the stable ID to be a valid lookup key. This PR adds that, while keeping the existing username-keyed workflow fully backward-compatible.

Behavior

data "github_user"

• New optional input user_id (mutually exclusive with username).
• Computed user_id attribute always populated.
• Resolves via Users.GetByID when user_id is set.

data "github_membership"

• New optional input user_id (mutually exclusive with username).
• Resolves user_id → login first (GitHub's org membership endpoints only accept logins), then queries the membership.
• Computed user_id attribute always populated.

resource "github_membership"

• New optional input user_id (TypeInt, ForceNew, exactly-one-of with username).
• Create resolves both halves of the (login, numeric_id) pair so state always carries both.
• ID format is org:user_id for new resources.
• Read auto-detects the ID shape: digits → numeric path (GetByID); non-digits → legacy username path, and rewrites the ID in place to org:user_id on first refresh.
• Delete uses the live username from state, which Read keeps fresh — so deletion still works correctly after a rename.

Tests

Acceptance tests (require TF_ACC=1 plus the env vars in CONTRIBUTING.md):

• TestAccGithubUserDataSource — added subtests for user_id happy path + error paths (not-found, mutual exclusion).
• TestAccGithubMembershipDataSource — same coverage for the data source.
• TestAccGithubMembership — added subtest for user_id-keyed create + mutual-exclusion error paths.
• TestAccGithubMembershipRenameResilience — end-to-end live-rename test. Creates a membership by user_id, renames the external user via PATCH /user with their own PAT (GH_TEST_EXTERNAL_USER_TOKEN), refreshes, asserts the username attribute updated with no plan diff, then restores the original login in cleanup. Skips automatically when GH_TEST_EXTERNAL_USER_TOKEN is unset.

Local validation:

go build ./...   # OK
go vet ./...     # OK
go test ./github/... -timeout=120s -parallel=4 -skip '^TestAcc' -count=1   # OK (5.0s)

Docs

• templates/data-sources/user.md.tmpl, templates/data-sources/membership.md.tmpl, templates/resources/membership.md.tmpl updated with user_id arguments, import notes, and rename-resilience guidance.
• New example files: examples/data-sources/user/example_2.tf, examples/data-sources/membership/example_2.tf, examples/resources/membership/example_2.tf.
• docs/* regenerated to match templates (please re-run make generatedocs on a machine with terraform on PATH to confirm bit-for-bit match — my dev machine lacked it; I synced docs to templates manually).

Backward compatibility

• Existing modules using username continue to work unchanged.
• Existing state with org:username IDs migrates lazily on next Read (single extra Users.GetByID round-trip, one-shot).
• terraform import org:username and terraform import org:12345 both supported.

Notes for reviewers

• data "github_user" exposes the stable ID as user_id rather than id (the issue's wording) because the Terraform SDKv2 reserves the top-level id attribute as the resource ID. Using user_id is also more consistent with the rest of this provider (e.g. organization_role_users).
• Read-side lazy migration was chosen over a SchemaVersion bump because StateUpgraders are supposed to be pure, and resolving username → user_id needs a network call. Read-side handles the migration self-healingly and incurs no upfront cost on terraform plan.

Commits

• [FEAT] Support looking up github_user data source by user_id
• [FEAT] Support looking up github_membership data source by user_id
• [FEAT] Support managing github_membership by stable user_id

[github-actions]

👋 Hi! Thank you for this contribution! Just to let you know, our GitHub SDK team does a round of issue and PR reviews twice a week, every Monday and Friday! We have a process in place for prioritizing and responding to your input. Because you are a part of this community please feel free to comment, add to, or pick up any issues/PRs that are labeled with Status: Up for grabs. You & others like you are the reason all of this works! So thank you & happy coding! 🚀

[niklasbeinghaus]

Security implications worth calling out

Beyond fixing the state-drift annoyance from #524, this PR meaningfully reduces a real, exploitable class of attack against github_membership. Flagging here so reviewers can weigh it explicitly.

1. Username-rename squatting against organizations

GitHub releases a username for reuse shortly after the original owner renames their account. Combined with a username-keyed github_membership, this enables a concrete attack:

1. An org member (call them alice) renames themselves to alice-new.
2. The old login alice becomes claimable by anyone on GitHub.
3. An attacker registers alice.
4. On the next terraform apply, the provider sees alice is no longer in the org and reconciles toward the declared config — re-inviting alice into the organization. The attacker accepts and is now a member (or admin, depending on the resource's role) of the org, fully via the org owner's own Terraform pipeline.

This isn't theoretical — it's the standard GitHub-username-squat pattern applied to IaC. The org owner has no signal anything is wrong; their drift detection passes because the config still says alice = member.

With this PR, when the membership is keyed by user_id (the immutable numeric ID), step 4 doesn't happen: the resource follows the renamed human, not the released string. The squatter inherits nothing.

This is an opt-in improvement — users who keep using username remain exposed. We may want to (in a follow-up) recommend user_id in the docs more strongly, or even mark username as soft-deprecated for the resource. Not in this PR; flagging for discussion.

2. Admin-role continuity through rename

Same mechanism, sharper edge: an org admin renaming themselves currently causes the username-keyed resource to drift to "not a member". Depending on downgrade_on_destroy and apply ordering, this can produce confusing state — Terraform thinks the admin is gone while GitHub still has them as admin under the new login. user_id-keyed resources keep role enforcement tied to the human, so privilege drift stays observable.

3. New attack surface this PR adds (honest accounting)

• Lazy ID migration in Read: one extra Users.GetByID request per legacy resource on first refresh. Same TLS, same configured token, same audit trail. No new trust boundary.
• TestAccGithubMembershipRenameResilience: the new live-rename test performs PATCH /user against the account whose PAT is supplied as GH_TEST_EXTERNAL_USER_TOKEN. Operationally important: this token must belong to a dedicated throwaway test account, never a real contributor's personal GitHub account, because the test mutates the authenticated user's login. The test skips automatically when the env var is unset (so CI without it is safe). Worth a line in CONTRIBUTING.md if maintainers want me to add it — happy to do so in this PR or a follow-up.

4. What this does NOT do

• Doesn't fix existing username-keyed state retroactively until users explicitly migrate their HCL to user_id. The ID-format migration in Read only converts state representation; it doesn't change the identification semantics chosen in HCL.
• Doesn't apply to github_team_membership (out of scope for Feature Request > ability to use github unique ID for github_membership #524, but the same threat model applies there — possible follow-up).

Happy to add a ## Security section to the resource docs spelling this out for end users if reviewers think it belongs in this PR rather than a follow-up.

[mczaplinski]

Security implications worth calling out

1. An org member (call them alice) renames themselves to alice-new.
2. The old login alice becomes claimable by anyone on GitHub.
3. An attacker registers alice.
4. On the next terraform apply, the provider sees alice is no longer in the org and reconciles toward the declared config — re-inviting alice into the organization. The attacker accepts and is now a member (or admin, depending on the resource's role) of the org, fully via the org owner's own Terraform pipeline.

That's why I'm not using this provider for user management as of now. Mapping members by mutable usernames is just too risky for me. I hope this gets merged!