From c53b12a9f8aa5c1352cb3a91b31247f9bb24c7bd Mon Sep 17 00:00:00 2001 From: Konyaka1 <47553039+Konyaka1@users.noreply.github.com> Date: Wed, 8 Jul 2026 23:37:36 -0400 Subject: [PATCH 01/15] Add access management: grants, groups, and org data sources New org package: splitsecure_grant (per-resource tier grants, upsert semantics, PermissionDenied retry on create to absorb the creator-grant race), splitsecure_group (locally-managed groups with an authoritative member set), and data sources splitsecure_organization (Everyone-group S2R), splitsecure_org_member (by email), splitsecure_group (by name). Wires an OrgService client, extends examples/full with an access section, and regenerates docs. go.mod carries a dev-only replace to the sibling apis working tree; swap it for a pinned apis version before merge. Co-Authored-By: Claude Fable 5 --- README.md | 4 + docs/data-sources/group.md | 35 ++ docs/data-sources/org_member.md | 34 ++ docs/data-sources/organization.md | 29 ++ docs/index.md | 29 ++ docs/resources/grant.md | 46 ++ docs/resources/group.md | 47 ++ .../splitsecure_group/data-source.tf | 7 + .../splitsecure_org_member/data-source.tf | 6 + .../splitsecure_organization/data-source.tf | 4 + examples/full/main.tf | 29 ++ .../resources/splitsecure_grant/resource.tf | 21 + .../resources/splitsecure_group/resource.tf | 15 + go.mod | 2 + go.sum | 2 - splitsecure/client/client.go | 3 + splitsecure/provider/provider.go | 9 +- .../services/org/data_internal_test.go | 146 ++++++ splitsecure/services/org/doc.go | 5 + splitsecure/services/org/grant.go | 342 +++++++++++++ .../services/org/grant_internal_test.go | 222 ++++++++ splitsecure/services/org/group.go | 477 ++++++++++++++++++ splitsecure/services/org/group_data.go | 166 ++++++ .../services/org/group_internal_test.go | 252 +++++++++ splitsecure/services/org/member_data.go | 137 +++++ splitsecure/services/org/organization_data.go | 102 ++++ 26 files changed, 2168 insertions(+), 3 deletions(-) create mode 100644 docs/data-sources/group.md create mode 100644 docs/data-sources/org_member.md create mode 100644 docs/data-sources/organization.md create mode 100644 docs/resources/grant.md create mode 100644 docs/resources/group.md create mode 100644 examples/data-sources/splitsecure_group/data-source.tf create mode 100644 examples/data-sources/splitsecure_org_member/data-source.tf create mode 100644 examples/data-sources/splitsecure_organization/data-source.tf create mode 100644 examples/resources/splitsecure_grant/resource.tf create mode 100644 examples/resources/splitsecure_group/resource.tf create mode 100644 splitsecure/services/org/data_internal_test.go create mode 100644 splitsecure/services/org/doc.go create mode 100644 splitsecure/services/org/grant.go create mode 100644 splitsecure/services/org/grant_internal_test.go create mode 100644 splitsecure/services/org/group.go create mode 100644 splitsecure/services/org/group_data.go create mode 100644 splitsecure/services/org/group_internal_test.go create mode 100644 splitsecure/services/org/member_data.go create mode 100644 splitsecure/services/org/organization_data.go diff --git a/README.md b/README.md index 0d6634d..058094a 100644 --- a/README.md +++ b/README.md @@ -6,6 +6,10 @@ Terraform provider for managing SplitSecure SAML2 resources via service-account - **`splitsecure_saml2_identity_provider`** — SAML IdP on a SplitSecure team. Computed `metadata_xml` is suitable as the `saml_metadata_document` input for `aws_iam_saml_provider`. - **`splitsecure_saml2_service_provider`** — SAML SP bound to an IdP, supporting all 17 saml2v2 integration variants (AWS, Cloudflare, Okta, GCP, etc.). +- **`splitsecure_grant`** — per-resource permission grant `(resource, grantee) → view/use/edit` so the resources above are visible/usable to org members, not just their creator. Plain RPCs, not proposal-gated. Tiers control visibility and permission management; resource mutations stay proposal/voter-gated regardless of tier. +- **`splitsecure_group`** — locally-managed principal group with an authoritative member list, used as a grant target. Group mutations require the provider's service account to hold the org **admin** role; grants alone work with the default member role (the creating service account receives `edit` on resources it creates). + +Data sources: **`splitsecure_organization`** (org info incl. the "Everyone" group S2R for org-wide grants), **`splitsecure_org_member`** (user S2R by email), **`splitsecure_group`** (existing group by name, e.g. SCIM-synced). Generated reference docs live in [`docs/`](./docs); per-resource attribute tables, validators, and example blocks are kept in sync via `tfplugindocs` (`make docs`). diff --git a/docs/data-sources/group.md b/docs/data-sources/group.md new file mode 100644 index 0000000..2118607 --- /dev/null +++ b/docs/data-sources/group.md @@ -0,0 +1,35 @@ +--- +# generated by https://github.com/hashicorp/terraform-plugin-docs +page_title: "splitsecure_group Data Source - SplitSecure" +subcategory: "" +description: |- + Looks up a single org group by exact name. Errors if no group or more than one group matches (org group names are not unique server-side). The system Everyone group is not listed; read everyone_group_s2r from the splitsecure_organization data source instead. +--- + +# splitsecure_group (Data Source) + +Looks up a single org group by exact name. Errors if no group or more than one group matches (org group names are not unique server-side). The system Everyone group is not listed; read everyone_group_s2r from the splitsecure_organization data source instead. + +## Example Usage + +```terraform +# Look up an existing group (e.g. SCIM-synced) by name. Names are not +# unique server-side; the lookup errors on zero or multiple matches. +# The system "Everyone" group is not listed — use the +# splitsecure_organization data source for it. +data "splitsecure_group" "sre" { + name = "SRE" +} +``` + + +## Schema + +### Required + +- `name` (String) Group name to look up. Matched exactly (case-sensitive). + +### Read-Only + +- `group_s2r` (String) Group s2r URI. Usable as a grant grantee. +- `source` (String) Where the group is managed: "local", "scim", or "system". diff --git a/docs/data-sources/org_member.md b/docs/data-sources/org_member.md new file mode 100644 index 0000000..5ad8cb9 --- /dev/null +++ b/docs/data-sources/org_member.md @@ -0,0 +1,34 @@ +--- +# generated by https://github.com/hashicorp/terraform-plugin-docs +page_title: "splitsecure_org_member Data Source - SplitSecure" +subcategory: "" +description: |- + Looks up a single org member by email (case-insensitive). Errors if no member or more than one member matches. +--- + +# splitsecure_org_member (Data Source) + +Looks up a single org member by email (case-insensitive). Errors if no member or more than one member matches. + +## Example Usage + +```terraform +# Resolve an org member by email to the user S2R used as a grant or +# group-member target. Errors if the email matches zero or multiple +# members. +data "splitsecure_org_member" "alice" { + email = "alice@example.com" +} +``` + + +## Schema + +### Required + +- `email` (String) Email address of the member to look up. Matched case-insensitively. + +### Read-Only + +- `display_name` (String) Human-readable display name of the member. +- `user_s2r` (String) User s2r URI of the member. Usable as a grant grantee or a group member principal. diff --git a/docs/data-sources/organization.md b/docs/data-sources/organization.md new file mode 100644 index 0000000..b6cb823 --- /dev/null +++ b/docs/data-sources/organization.md @@ -0,0 +1,29 @@ +--- +# generated by https://github.com/hashicorp/terraform-plugin-docs +page_title: "splitsecure_organization Data Source - SplitSecure" +subcategory: "" +description: |- + The organization the provider is configured against. Takes no arguments; the org comes from the provider's org_s2r. +--- + +# splitsecure_organization (Data Source) + +The organization the provider is configured against. Takes no arguments; the org comes from the provider's org_s2r. + +## Example Usage + +```terraform +# The provider-configured org. everyone_group_s2r is the grantee for +# org-wide grants; the "Everyone" group is a system group and is not +# returned by group listings. +data "splitsecure_organization" "current" {} +``` + + +## Schema + +### Read-Only + +- `everyone_group_s2r` (String) S2R of the org's system Everyone group -- the grantee to use for org-wide grants. System groups are NOT returned by group listings (including the splitsecure_group data source); this attribute is the way to obtain it. +- `id` (String) Org s2r URI. +- `name` (String) Human-readable organization name. diff --git a/docs/index.md b/docs/index.md index f594420..e6cdece 100644 --- a/docs/index.md +++ b/docs/index.md @@ -203,4 +203,33 @@ output "aws_readonly_role_arn" { value = aws_iam_role.readonly.arn description = "ARN of the readonly role users assume via SAML." } + +# --- Access ------------------------------------------------------- +# Terraform-managed permissions on the SP. Without grants, only org +# owners/admins (and the creating service account) can see it. + +data "splitsecure_organization" "current" {} + +variable "operator_s2rs" { + type = list(string) + default = [] + description = "User / service-account S2Rs allowed to operate the AWS federation SP." +} + +resource "splitsecure_group" "operators" { + name = "aws-federation-operators-${local.account_id}" + members = var.operator_s2rs +} + +resource "splitsecure_grant" "operators_use" { + resource_s2r = splitsecure_saml2_service_provider.main.id + grantee_s2r = splitsecure_group.operators.group_s2r + tier = "use" +} + +resource "splitsecure_grant" "org_view" { + resource_s2r = splitsecure_saml2_service_provider.main.id + grantee_s2r = data.splitsecure_organization.current.everyone_group_s2r + tier = "view" +} ``` diff --git a/docs/resources/grant.md b/docs/resources/grant.md new file mode 100644 index 0000000..63a17be --- /dev/null +++ b/docs/resources/grant.md @@ -0,0 +1,46 @@ +--- +# generated by https://github.com/hashicorp/terraform-plugin-docs +page_title: "splitsecure_grant Resource - SplitSecure" +subcategory: "" +description: |- + Permission grant giving a principal (user, service account, or group) a tier on a resource. Keyed by (resource, grantee) within the provider-configured org; only the tier can change in place. +--- + +# splitsecure_grant (Resource) + +Permission grant giving a principal (user, service account, or group) a tier on a resource. Keyed by (resource, grantee) within the provider-configured org; only the tier can change in place. + +## Example Usage + +```terraform +# Grants authorize a principal (user, service account, or group) to act +# on one resource at a tier: view < use < edit. PutGrant semantics are +# upsert, so changing tier updates the grant in place; changing the +# resource or grantee replaces it. +# +# Org owners and admins hold the edit tier on every resource implicitly; +# grants matter for plain members. + +# Grant a group access to a SAML2 service provider. +resource "splitsecure_grant" "sre_use" { + resource_s2r = splitsecure_saml2_service_provider.main.id + grantee_s2r = data.splitsecure_group.sre.group_s2r + tier = "use" +} + +# Grant every org member visibility via the system "Everyone" group. +resource "splitsecure_grant" "org_view" { + resource_s2r = splitsecure_saml2_service_provider.main.id + grantee_s2r = data.splitsecure_organization.current.everyone_group_s2r + tier = "view" +} +``` + + +## Schema + +### Required + +- `grantee_s2r` (String) s2r URI of the principal receiving access: a user, service account, or group. +- `resource_s2r` (String) s2r URI of the resource being shared. +- `tier` (String) Access tier. One of: view, use, edit. diff --git a/docs/resources/group.md b/docs/resources/group.md new file mode 100644 index 0000000..b203930 --- /dev/null +++ b/docs/resources/group.md @@ -0,0 +1,47 @@ +--- +# generated by https://github.com/hashicorp/terraform-plugin-docs +page_title: "splitsecure_group Resource - SplitSecure" +subcategory: "" +description: |- + Locally-managed principal group in the provider-configured organization. Terraform is authoritative over the member set; SCIM- and system-managed groups cannot be managed by this resource. +--- + +# splitsecure_group (Resource) + +Locally-managed principal group in the provider-configured organization. Terraform is authoritative over the member set; SCIM- and system-managed groups cannot be managed by this resource. + +## Example Usage + +```terraform +# Groups are named sets of principals used as grant targets. Terraform +# manages locally-sourced groups only: SCIM groups are owned by the +# IdP and the system "Everyone" group is implicit. The members list is +# authoritative — principals added out of band are removed on the next +# apply. +# +# Group mutations require the service account behind the provider to +# hold the org admin role. +resource "splitsecure_group" "sre" { + name = "SRE" + members = [ + data.splitsecure_org_member.alice.user_s2r, + "s2r:us:sa:2qX9mK4pLw8vN3rT", + ] +} +``` + + +## Schema + +### Required + +- `name` (String) Human-readable group name. Changing it updates the group in place. + +### Optional + +- `members` (Set of String) Authoritative set of member principal s2r URIs (users or service accounts). Principals not listed here are removed on apply. Leave unset for an empty group. + +### Read-Only + +- `group_s2r` (String) Group s2r URI. Stable identifier; also the import ID. +- `source` (String) Where the group is managed from: "local", "scim", or "system". Always "local" for Terraform-managed groups. diff --git a/examples/data-sources/splitsecure_group/data-source.tf b/examples/data-sources/splitsecure_group/data-source.tf new file mode 100644 index 0000000..be50db9 --- /dev/null +++ b/examples/data-sources/splitsecure_group/data-source.tf @@ -0,0 +1,7 @@ +# Look up an existing group (e.g. SCIM-synced) by name. Names are not +# unique server-side; the lookup errors on zero or multiple matches. +# The system "Everyone" group is not listed — use the +# splitsecure_organization data source for it. +data "splitsecure_group" "sre" { + name = "SRE" +} diff --git a/examples/data-sources/splitsecure_org_member/data-source.tf b/examples/data-sources/splitsecure_org_member/data-source.tf new file mode 100644 index 0000000..184dffe --- /dev/null +++ b/examples/data-sources/splitsecure_org_member/data-source.tf @@ -0,0 +1,6 @@ +# Resolve an org member by email to the user S2R used as a grant or +# group-member target. Errors if the email matches zero or multiple +# members. +data "splitsecure_org_member" "alice" { + email = "alice@example.com" +} diff --git a/examples/data-sources/splitsecure_organization/data-source.tf b/examples/data-sources/splitsecure_organization/data-source.tf new file mode 100644 index 0000000..388884f --- /dev/null +++ b/examples/data-sources/splitsecure_organization/data-source.tf @@ -0,0 +1,4 @@ +# The provider-configured org. everyone_group_s2r is the grantee for +# org-wide grants; the "Everyone" group is a system group and is not +# returned by group listings. +data "splitsecure_organization" "current" {} diff --git a/examples/full/main.tf b/examples/full/main.tf index 2c1087b..627cd0b 100644 --- a/examples/full/main.tf +++ b/examples/full/main.tf @@ -159,3 +159,32 @@ output "aws_readonly_role_arn" { value = aws_iam_role.readonly.arn description = "ARN of the readonly role users assume via SAML." } + +# --- Access ------------------------------------------------------- +# Terraform-managed permissions on the SP. Without grants, only org +# owners/admins (and the creating service account) can see it. + +data "splitsecure_organization" "current" {} + +variable "operator_s2rs" { + type = list(string) + default = [] + description = "User / service-account S2Rs allowed to operate the AWS federation SP." +} + +resource "splitsecure_group" "operators" { + name = "aws-federation-operators-${local.account_id}" + members = var.operator_s2rs +} + +resource "splitsecure_grant" "operators_use" { + resource_s2r = splitsecure_saml2_service_provider.main.id + grantee_s2r = splitsecure_group.operators.group_s2r + tier = "use" +} + +resource "splitsecure_grant" "org_view" { + resource_s2r = splitsecure_saml2_service_provider.main.id + grantee_s2r = data.splitsecure_organization.current.everyone_group_s2r + tier = "view" +} diff --git a/examples/resources/splitsecure_grant/resource.tf b/examples/resources/splitsecure_grant/resource.tf new file mode 100644 index 0000000..6b079fe --- /dev/null +++ b/examples/resources/splitsecure_grant/resource.tf @@ -0,0 +1,21 @@ +# Grants authorize a principal (user, service account, or group) to act +# on one resource at a tier: view < use < edit. PutGrant semantics are +# upsert, so changing tier updates the grant in place; changing the +# resource or grantee replaces it. +# +# Org owners and admins hold the edit tier on every resource implicitly; +# grants matter for plain members. + +# Grant a group access to a SAML2 service provider. +resource "splitsecure_grant" "sre_use" { + resource_s2r = splitsecure_saml2_service_provider.main.id + grantee_s2r = data.splitsecure_group.sre.group_s2r + tier = "use" +} + +# Grant every org member visibility via the system "Everyone" group. +resource "splitsecure_grant" "org_view" { + resource_s2r = splitsecure_saml2_service_provider.main.id + grantee_s2r = data.splitsecure_organization.current.everyone_group_s2r + tier = "view" +} diff --git a/examples/resources/splitsecure_group/resource.tf b/examples/resources/splitsecure_group/resource.tf new file mode 100644 index 0000000..a3a2f3b --- /dev/null +++ b/examples/resources/splitsecure_group/resource.tf @@ -0,0 +1,15 @@ +# Groups are named sets of principals used as grant targets. Terraform +# manages locally-sourced groups only: SCIM groups are owned by the +# IdP and the system "Everyone" group is implicit. The members list is +# authoritative — principals added out of band are removed on the next +# apply. +# +# Group mutations require the service account behind the provider to +# hold the org admin role. +resource "splitsecure_group" "sre" { + name = "SRE" + members = [ + data.splitsecure_org_member.alice.user_s2r, + "s2r:us:sa:2qX9mK4pLw8vN3rT", + ] +} diff --git a/go.mod b/go.mod index 4660cbc..9ad63a8 100644 --- a/go.mod +++ b/go.mod @@ -75,3 +75,5 @@ require ( gopkg.in/yaml.v2 v2.3.0 // indirect gopkg.in/yaml.v3 v3.0.1 // indirect ) + +replace github.com/splitsecure/apis => ../apis diff --git a/go.sum b/go.sum index 5812e3a..5b02a07 100644 --- a/go.sum +++ b/go.sum @@ -168,8 +168,6 @@ github.com/skeema/knownhosts v1.3.1/go.mod h1:r7KTdC8l4uxWRyK2TpQZ/1o5HaSzh06ePQ github.com/spf13/cast v1.3.1/go.mod h1:Qx5cxh0v+4UWYiBimWS+eyWzqEqokIECu5etghLkUJE= github.com/spf13/cast v1.5.0 h1:rj3WzYc11XZaIZMPKmwP96zkFEnnAmV8s6XbB2aY32w= github.com/spf13/cast v1.5.0/go.mod h1:SpXXQ5YoyJw6s3/6cMTQuxvgRl3PCJiyaX9p6b155UU= -github.com/splitsecure/apis v0.0.0-20260514001223-f52c82fb5959 h1:qKSnoDvD7Dc6AKBIKjHrLtGyB6aeOu84VUqlCJnFq/E= -github.com/splitsecure/apis v0.0.0-20260514001223-f52c82fb5959/go.mod h1:wUlW3JTT8bCwxhF+EvEazUtdofaoPjAFRCvYWh49sr4= github.com/stretchr/objx v0.1.0/go.mod h1:HFkY916IF+rwdDfMAkV7OtwuqBVzrE8GR6GFx+wExME= github.com/stretchr/testify v1.2.2/go.mod h1:a8OnRcib4nhh0OaRAV+Yts87kKdq0PP7pXfy6kDkUVs= github.com/stretchr/testify v1.4.0/go.mod h1:j7eGeouHqKxXV5pUuKE4zz7dFj8WfuZ+81PSLYec5m4= diff --git a/splitsecure/client/client.go b/splitsecure/client/client.go index 8f43d0f..5d3201a 100644 --- a/splitsecure/client/client.go +++ b/splitsecure/client/client.go @@ -10,6 +10,7 @@ import ( conveniencestorev1connect "github.com/splitsecure/apis/gen/go/proto/splitsecure/conveniencestore/v1/conveniencestorev1connect" enclaveroundtripv1connect "github.com/splitsecure/apis/gen/go/proto/splitsecure/enclaveroundtrip/v1/enclaveroundtripv1connect" + orgsvcv1connect "github.com/splitsecure/apis/gen/go/proto/splitsecure/orgsvc/v1/orgsvcv1connect" proposalsv1connect "github.com/splitsecure/apis/gen/go/proto/splitsecure/proposals/v1/proposalsv1connect" ) @@ -23,6 +24,7 @@ type Client struct { ConvenienceStoreService conveniencestorev1connect.ConvenienceStoreServiceClient EnclaveRoundtripService enclaveroundtripv1connect.EnclaveRoundtripServiceClient ProposalsService proposalsv1connect.ProposalsServiceClient + OrgService orgsvcv1connect.OrgServiceClient OrgS2R string } @@ -48,6 +50,7 @@ func New(endpoint, bearerToken, orgS2R, version string) *Client { ConvenienceStoreService: conveniencestorev1connect.NewConvenienceStoreServiceClient(httpClient, endpoint), EnclaveRoundtripService: enclaveroundtripv1connect.NewEnclaveRoundtripServiceClient(httpClient, endpoint), ProposalsService: proposalsv1connect.NewProposalsServiceClient(httpClient, endpoint), + OrgService: orgsvcv1connect.NewOrgServiceClient(httpClient, endpoint), OrgS2R: orgS2R, } } diff --git a/splitsecure/provider/provider.go b/splitsecure/provider/provider.go index c823f90..331fc3d 100644 --- a/splitsecure/provider/provider.go +++ b/splitsecure/provider/provider.go @@ -12,6 +12,7 @@ import ( "github.com/hashicorp/terraform-plugin-framework/types" "github.com/splitsecure/terraform-provider-splitsecure/splitsecure/client" + "github.com/splitsecure/terraform-provider-splitsecure/splitsecure/services/org" "github.com/splitsecure/terraform-provider-splitsecure/splitsecure/services/saml2" ) @@ -109,9 +110,15 @@ func (p *splitsecureProvider) Resources(_ context.Context) []func() resource.Res return []func() resource.Resource{ saml2.NewIdentityProvider, saml2.NewServiceProvider, + org.NewGrant, + org.NewGroup, } } func (p *splitsecureProvider) DataSources(_ context.Context) []func() datasource.DataSource { - return nil + return []func() datasource.DataSource{ + org.NewOrganizationDataSource, + org.NewMemberDataSource, + org.NewGroupDataSource, + } } diff --git a/splitsecure/services/org/data_internal_test.go b/splitsecure/services/org/data_internal_test.go new file mode 100644 index 0000000..3bf9a5b --- /dev/null +++ b/splitsecure/services/org/data_internal_test.go @@ -0,0 +1,146 @@ +package org + +import ( + "strings" + "testing" + + orgsvcv1 "github.com/splitsecure/apis/gen/go/proto/splitsecure/orgsvc/v1" +) + +func TestMatchMemberByEmail(t *testing.T) { + t.Parallel() + + members := []*orgsvcv1.Member{ + {UserId: "s2r:us:usr:alice", Email: "alice@example.com", DisplayName: "Alice"}, + {UserId: "s2r:us:usr:bob", Email: "Bob@Example.COM", DisplayName: "Bob"}, + {UserId: "s2r:us:usr:carol1", Email: "carol@example.com", DisplayName: "Carol One"}, + {UserId: "s2r:us:usr:carol2", Email: "CAROL@example.com", DisplayName: "Carol Two"}, + } + + cases := []struct { + name string + email string + wantUserID string + wantErrPart string // empty means the lookup must succeed + }{ + {name: "exact match", email: "alice@example.com", wantUserID: "s2r:us:usr:alice"}, + {name: "case-insensitive match", email: "bob@example.com", wantUserID: "s2r:us:usr:bob"}, + {name: "mixed-case query matches stored lowercase", email: "ALICE@EXAMPLE.COM", wantUserID: "s2r:us:usr:alice"}, + {name: "ambiguous email returns error listing matches", email: "carol@example.com", wantErrPart: "s2r:us:usr:carol2"}, + {name: "absent email returns error naming the email", email: "dave@example.com", wantErrPart: "dave@example.com"}, + {name: "empty member list returns error", email: "alice@example.com", wantErrPart: "alice@example.com"}, + } + + for _, tc := range cases { + t.Run(tc.name, func(t *testing.T) { + t.Parallel() + + in := members + if tc.name == "empty member list returns error" { + in = nil + } + + got, err := matchMemberByEmail(in, tc.email) + if tc.wantErrPart != "" { + if err == nil { + t.Fatalf("matchMemberByEmail(%q) = %+v, want error", tc.email, got) + } + if !strings.Contains(err.Error(), tc.wantErrPart) { + t.Fatalf("error %q does not contain %q", err.Error(), tc.wantErrPart) + } + + return + } + if err != nil { + t.Fatalf("matchMemberByEmail(%q): %v", tc.email, err) + } + if got.GetUserId() != tc.wantUserID { + t.Fatalf("got user %q, want %q", got.GetUserId(), tc.wantUserID) + } + }) + } +} + +func TestMatchGroupByName(t *testing.T) { + t.Parallel() + + groups := []*orgsvcv1.Group{ + {GroupS2R: "s2r:us:grp:eng", Name: "Engineering"}, + {GroupS2R: "s2r:us:grp:ops1", Name: "Ops"}, + {GroupS2R: "s2r:us:grp:ops2", Name: "Ops"}, + } + + cases := []struct { + name string + groupName string + wantS2R string + wantErrPart string // empty means the lookup must succeed + }{ + {name: "exact match", groupName: "Engineering", wantS2R: "s2r:us:grp:eng"}, + {name: "different case does not match", groupName: "engineering", wantErrPart: `"engineering"`}, + {name: "absent name error points at splitsecure_organization for Everyone", groupName: "Everyone", wantErrPart: "splitsecure_organization"}, + {name: "duplicate name returns ambiguity error listing matches", groupName: "Ops", wantErrPart: "s2r:us:grp:ops2"}, + } + + for _, tc := range cases { + t.Run(tc.name, func(t *testing.T) { + t.Parallel() + + got, err := matchGroupByName(groups, tc.groupName) + if tc.wantErrPart != "" { + if err == nil { + t.Fatalf("matchGroupByName(%q) = %+v, want error", tc.groupName, got) + } + if !strings.Contains(err.Error(), tc.wantErrPart) { + t.Fatalf("error %q does not contain %q", err.Error(), tc.wantErrPart) + } + + return + } + if err != nil { + t.Fatalf("matchGroupByName(%q): %v", tc.groupName, err) + } + if got.GetGroupS2R() != tc.wantS2R { + t.Fatalf("got group %q, want %q", got.GetGroupS2R(), tc.wantS2R) + } + }) + } +} + +func TestGroupSourceToString(t *testing.T) { + t.Parallel() + + cases := []struct { + name string + source orgsvcv1.GroupSource + want string + wantError bool + }{ + {name: "local", source: orgsvcv1.GroupSource_GROUP_SOURCE_LOCAL, want: "local"}, + {name: "scim", source: orgsvcv1.GroupSource_GROUP_SOURCE_SCIM, want: "scim"}, + {name: "system", source: orgsvcv1.GroupSource_GROUP_SOURCE_SYSTEM, want: "system"}, + {name: "unspecified returns error", source: orgsvcv1.GroupSource_GROUP_SOURCE_UNSPECIFIED, wantError: true}, + {name: "out-of-range value returns error", source: orgsvcv1.GroupSource(99), wantError: true}, + } + + for _, tc := range cases { + t.Run(tc.name, func(t *testing.T) { + t.Parallel() + + got, err := groupSourceToString(tc.source) + if tc.wantError { + if err == nil { + t.Fatalf("groupSourceToString(%v) = %q, want error", tc.source, got) + } + + return + } + if err != nil { + t.Fatalf("groupSourceToString(%v): %v", tc.source, err) + } + if got != tc.want { + t.Fatalf("got %q, want %q", got, tc.want) + } + }) + } +} diff --git a/splitsecure/services/org/doc.go b/splitsecure/services/org/doc.go new file mode 100644 index 0000000..1117a06 --- /dev/null +++ b/splitsecure/services/org/doc.go @@ -0,0 +1,5 @@ +// Package org implements org-scoped Terraform resources and data +// sources: per-resource permission grants, principal groups, and +// directory lookups (organization, members, groups). All of them talk +// to OrgService and operate within the provider-configured org. +package org diff --git a/splitsecure/services/org/grant.go b/splitsecure/services/org/grant.go new file mode 100644 index 0000000..b3563a0 --- /dev/null +++ b/splitsecure/services/org/grant.go @@ -0,0 +1,342 @@ +package org + +import ( + "context" + "errors" + "fmt" + "strings" + "time" + + "connectrpc.com/connect" + "github.com/hashicorp/terraform-plugin-framework-validators/stringvalidator" + "github.com/hashicorp/terraform-plugin-framework/diag" + "github.com/hashicorp/terraform-plugin-framework/path" + "github.com/hashicorp/terraform-plugin-framework/resource" + "github.com/hashicorp/terraform-plugin-framework/resource/schema" + "github.com/hashicorp/terraform-plugin-framework/resource/schema/planmodifier" + "github.com/hashicorp/terraform-plugin-framework/resource/schema/stringplanmodifier" + "github.com/hashicorp/terraform-plugin-framework/schema/validator" + "github.com/hashicorp/terraform-plugin-framework/types" + + authzv1 "github.com/splitsecure/apis/gen/go/proto/splitsecure/authz/v1" + orgsvcv1 "github.com/splitsecure/apis/gen/go/proto/splitsecure/orgsvc/v1" + "github.com/splitsecure/terraform-provider-splitsecure/splitsecure/client" +) + +var ( + _ resource.Resource = (*grantResource)(nil) + _ resource.ResourceWithImportState = (*grantResource)(nil) +) + +type grantResource struct { + client *client.Client +} + +type grantModel struct { + ResourceS2R types.String `tfsdk:"resource_s2r"` + GranteeS2R types.String `tfsdk:"grantee_s2r"` + Tier types.String `tfsdk:"tier"` +} + +// NewGrant returns a factory for the per-resource permission grant +// resource. +func NewGrant() resource.Resource { + return &grantResource{} +} + +func (r *grantResource) Metadata(_ context.Context, req resource.MetadataRequest, resp *resource.MetadataResponse) { + resp.TypeName = req.ProviderTypeName + "_grant" +} + +func (r *grantResource) Configure(_ context.Context, req resource.ConfigureRequest, resp *resource.ConfigureResponse) { + if req.ProviderData == nil { + return + } + c, ok := req.ProviderData.(*client.Client) + if !ok { + resp.Diagnostics.AddError( + "Unexpected provider data type", + fmt.Sprintf("expected *client.Client, got %T", req.ProviderData), + ) + + return + } + r.client = c +} + +func (r *grantResource) Schema(_ context.Context, _ resource.SchemaRequest, resp *resource.SchemaResponse) { + resp.Schema = schema.Schema{ + Description: "Permission grant giving a principal (user, service account, or group) a tier on a resource. " + + "Keyed by (resource, grantee) within the provider-configured org; only the tier can change in place.", + Attributes: map[string]schema.Attribute{ + "resource_s2r": schema.StringAttribute{ + Required: true, + Description: "s2r URI of the resource being shared.", + PlanModifiers: []planmodifier.String{stringplanmodifier.RequiresReplace()}, + Validators: []validator.String{ + stringvalidator.LengthAtLeast(1), + }, + }, + "grantee_s2r": schema.StringAttribute{ + Required: true, + Description: "s2r URI of the principal receiving access: a user, service account, or group.", + PlanModifiers: []planmodifier.String{stringplanmodifier.RequiresReplace()}, + Validators: []validator.String{ + stringvalidator.LengthAtLeast(1), + }, + }, + "tier": schema.StringAttribute{ + Required: true, + Description: "Access tier. One of: " + strings.Join(tierValues(), ", ") + ".", + Validators: []validator.String{ + stringvalidator.OneOf(tierValues()...), + }, + }, + }, + } +} + +func (r *grantResource) Create(ctx context.Context, req resource.CreateRequest, resp *resource.CreateResponse) { + var plan grantModel + resp.Diagnostics.Append(req.Plan.Get(ctx, &plan)...) + if resp.Diagnostics.HasError() { + return + } + + tier, err := tierFromString(plan.Tier.ValueString()) + if err != nil { + resp.Diagnostics.AddError("Invalid tier", err.Error()) + + return + } + + g, err := r.putGrantRetryingAuthz(ctx, &orgsvcv1.PutGrantRequest{ + OrgS2R: r.client.OrgS2R, + ResourceS2R: plan.ResourceS2R.ValueString(), + GranteeS2R: plan.GranteeS2R.ValueString(), + Tier: tier, + }) + if err != nil { + resp.Diagnostics.AddError("Creating grant", err.Error()) + + return + } + + resp.Diagnostics.Append(populateGrantModel(&plan, g)...) + if resp.Diagnostics.HasError() { + return + } + resp.Diagnostics.Append(resp.State.Set(ctx, &plan)...) +} + +func (r *grantResource) Read(ctx context.Context, req resource.ReadRequest, resp *resource.ReadResponse) { + var state grantModel + resp.Diagnostics.Append(req.State.Get(ctx, &state)...) + if resp.Diagnostics.HasError() { + return + } + + getResp, err := r.client.OrgService.GetGrant(ctx, connect.NewRequest(&orgsvcv1.GetGrantRequest{ + OrgS2R: r.client.OrgS2R, + ResourceS2R: state.ResourceS2R.ValueString(), + GranteeS2R: state.GranteeS2R.ValueString(), + })) + if err != nil { + if connect.CodeOf(err) == connect.CodeNotFound { + resp.State.RemoveResource(ctx) + + return + } + resp.Diagnostics.AddError("Reading grant", err.Error()) + + return + } + + resp.Diagnostics.Append(populateGrantModel(&state, getResp.Msg.GetGrant())...) + if resp.Diagnostics.HasError() { + return + } + resp.Diagnostics.Append(resp.State.Set(ctx, &state)...) +} + +func (r *grantResource) Update(ctx context.Context, req resource.UpdateRequest, resp *resource.UpdateResponse) { + var plan grantModel + resp.Diagnostics.Append(req.Plan.Get(ctx, &plan)...) + if resp.Diagnostics.HasError() { + return + } + + tier, err := tierFromString(plan.Tier.ValueString()) + if err != nil { + resp.Diagnostics.AddError("Invalid tier", err.Error()) + + return + } + + putResp, err := r.client.OrgService.PutGrant(ctx, connect.NewRequest(&orgsvcv1.PutGrantRequest{ + OrgS2R: r.client.OrgS2R, + ResourceS2R: plan.ResourceS2R.ValueString(), + GranteeS2R: plan.GranteeS2R.ValueString(), + Tier: tier, + })) + if err != nil { + resp.Diagnostics.AddError("Updating grant", err.Error()) + + return + } + + resp.Diagnostics.Append(populateGrantModel(&plan, putResp.Msg.GetGrant())...) + if resp.Diagnostics.HasError() { + return + } + resp.Diagnostics.Append(resp.State.Set(ctx, &plan)...) +} + +func (r *grantResource) Delete(ctx context.Context, req resource.DeleteRequest, resp *resource.DeleteResponse) { + var state grantModel + resp.Diagnostics.Append(req.State.Get(ctx, &state)...) + if resp.Diagnostics.HasError() { + return + } + + _, err := r.client.OrgService.DeleteGrant(ctx, connect.NewRequest(&orgsvcv1.DeleteGrantRequest{ + OrgS2R: r.client.OrgS2R, + ResourceS2R: state.ResourceS2R.ValueString(), + GranteeS2R: state.GranteeS2R.ValueString(), + })) + if err != nil && connect.CodeOf(err) != connect.CodeNotFound { + resp.Diagnostics.AddError("Deleting grant", err.Error()) + } +} + +func (r *grantResource) ImportState(ctx context.Context, req resource.ImportStateRequest, resp *resource.ImportStateResponse) { + resourceS2R, granteeS2R, err := parseGrantImportID(req.ID) + if err != nil { + resp.Diagnostics.AddError("Invalid import ID", err.Error()) + + return + } + resp.Diagnostics.Append(resp.State.SetAttribute(ctx, path.Root("resource_s2r"), resourceS2R)...) + resp.Diagnostics.Append(resp.State.SetAttribute(ctx, path.Root("grantee_s2r"), granteeS2R)...) +} + +// putGrantRetryingAuthz is the Create-path PutGrant. It retries +// PermissionDenied for ~30s: a grant against a freshly +// proposal-created resource can race the server-side creator-grant +// write that authorizes this caller. Every other code fails +// immediately. +func (r *grantResource) putGrantRetryingAuthz(ctx context.Context, req *orgsvcv1.PutGrantRequest) (*orgsvcv1.Grant, error) { + waits := []time.Duration{ + 1 * time.Second, + 2 * time.Second, + 4 * time.Second, + 8 * time.Second, + 8 * time.Second, + } + for attempt := 0; ; attempt++ { + putResp, err := r.client.OrgService.PutGrant(ctx, connect.NewRequest(req)) + if err == nil { + return putResp.Msg.GetGrant(), nil + } + if connect.CodeOf(err) != connect.CodePermissionDenied { + return nil, fmt.Errorf("PutGrant: %w", err) + } + if attempt >= len(waits) { + return nil, fmt.Errorf( + "PutGrant still permission-denied after %d attempts: creating a grant on %s requires the calling principal to hold the %q tier on that resource (an org admin can grant it): %w", + attempt+1, req.GetResourceS2R(), "edit", err, + ) + } + + t := time.NewTimer(waits[attempt]) + select { + case <-ctx.Done(): + t.Stop() + + return nil, ctx.Err() + case <-t.C: + } + } +} + +func populateGrantModel(m *grantModel, g *orgsvcv1.Grant) diag.Diagnostics { + var d diag.Diagnostics + if g == nil { + d.AddError("Decoding grant", "response contains no grant") + + return d + } + tierStr, err := tierToString(g.GetTier()) + if err != nil { + d.AddError("Decoding grant", err.Error()) + + return d + } + + m.ResourceS2R = types.StringValue(g.GetResourceS2R()) + m.GranteeS2R = types.StringValue(g.GetGranteeS2R()) + m.Tier = types.StringValue(tierStr) + + return nil +} + +var ( + errUnknownTier = errors.New("unknown tier") + errUnmappableTier = errors.New("unmappable tier enum value") + errMalformedGrantImportID = errors.New("malformed grant import ID") +) + +type tierMapping struct { + name string + tier authzv1.Tier +} + +// tierMappings is the single source of truth for the Terraform-string +// <-> authz enum correspondence; the schema validator, both converters, +// and the docs string all derive from it. +func tierMappings() []tierMapping { + return []tierMapping{ + {name: "view", tier: authzv1.Tier_TIER_VIEW}, + {name: "use", tier: authzv1.Tier_TIER_USE}, + {name: "edit", tier: authzv1.Tier_TIER_EDIT}, + } +} + +func tierValues() []string { + mappings := tierMappings() + names := make([]string, 0, len(mappings)) + for _, m := range mappings { + names = append(names, m.name) + } + + return names +} + +func tierFromString(s string) (authzv1.Tier, error) { + for _, m := range tierMappings() { + if m.name == s { + return m.tier, nil + } + } + + return authzv1.Tier_TIER_UNSPECIFIED, fmt.Errorf("%w: %q (expected one of: %s)", errUnknownTier, s, strings.Join(tierValues(), ", ")) +} + +func tierToString(t authzv1.Tier) (string, error) { + for _, m := range tierMappings() { + if m.tier == t { + return m.name, nil + } + } + + return "", fmt.Errorf("%w: %d (%s)", errUnmappableTier, t, t) +} + +func parseGrantImportID(id string) (string, string, error) { + resourceS2R, granteeS2R, found := strings.Cut(id, ",") + if !found || resourceS2R == "" || granteeS2R == "" || strings.Contains(granteeS2R, ",") { + return "", "", fmt.Errorf("%w: %q (expected \",\", exactly one comma)", errMalformedGrantImportID, id) + } + + return resourceS2R, granteeS2R, nil +} diff --git a/splitsecure/services/org/grant_internal_test.go b/splitsecure/services/org/grant_internal_test.go new file mode 100644 index 0000000..3043eb9 --- /dev/null +++ b/splitsecure/services/org/grant_internal_test.go @@ -0,0 +1,222 @@ +package org + +import ( + "context" + "testing" + + "github.com/hashicorp/terraform-plugin-framework/resource" + rschema "github.com/hashicorp/terraform-plugin-framework/resource/schema" + + authzv1 "github.com/splitsecure/apis/gen/go/proto/splitsecure/authz/v1" +) + +// grantTestSchema pulls the schema out of the grant resource by +// calling its Schema(...) method directly, avoiding framework +// plumbing. +func grantTestSchema(t *testing.T) rschema.Schema { + t.Helper() + + resp := &resource.SchemaResponse{} + (&grantResource{}).Schema(context.Background(), resource.SchemaRequest{}, resp) + if resp.Diagnostics.HasError() { + t.Fatalf("schema diagnostics: %v", resp.Diagnostics) + } + + return resp.Schema +} + +func TestGrantSchema_RequiredAttributes(t *testing.T) { + t.Parallel() + + s := grantTestSchema(t) + + for _, name := range []string{"resource_s2r", "grantee_s2r", "tier"} { + attr, ok := s.Attributes[name] + if !ok { + t.Fatalf("required attribute %q missing from grant schema", name) + } + if !attr.IsRequired() { + t.Errorf("attribute %q should be Required", name) + } + } +} + +// TestGrantSchema_ReplaceSemantics asserts the grant's key attributes +// force replacement while tier stays updatable in place. +func TestGrantSchema_ReplaceSemantics(t *testing.T) { + t.Parallel() + + s := grantTestSchema(t) + + for _, name := range []string{"resource_s2r", "grantee_s2r"} { + attr, ok := s.Attributes[name].(rschema.StringAttribute) + if !ok { + t.Fatalf("attribute %q has wrong type: %T", name, s.Attributes[name]) + } + if len(attr.PlanModifiers) == 0 { + t.Errorf("attribute %q should have a RequiresReplace plan modifier", name) + } + } + + tier, ok := s.Attributes["tier"].(rschema.StringAttribute) + if !ok { + t.Fatalf("tier attribute has wrong type: %T", s.Attributes["tier"]) + } + if len(tier.PlanModifiers) != 0 { + t.Error("tier should have no plan modifiers; it must be updatable in place") + } +} + +func TestGrantSchema_TierHasOneOfValidator(t *testing.T) { + t.Parallel() + + s := grantTestSchema(t) + + tier, ok := s.Attributes["tier"].(rschema.StringAttribute) + if !ok { + t.Fatalf("tier attribute has wrong type: %T", s.Attributes["tier"]) + } + if len(tier.Validators) == 0 { + t.Fatal("tier should have at least one validator (OneOf)") + } +} + +func TestTierToString(t *testing.T) { + t.Parallel() + + tests := []struct { + name string + tier authzv1.Tier + want string + wantErr bool + }{ + {name: "view", tier: authzv1.Tier_TIER_VIEW, want: "view"}, + {name: "use", tier: authzv1.Tier_TIER_USE, want: "use"}, + {name: "edit", tier: authzv1.Tier_TIER_EDIT, want: "edit"}, + {name: "errors on unspecified", tier: authzv1.Tier_TIER_UNSPECIFIED, wantErr: true}, + {name: "errors on out-of-range value", tier: authzv1.Tier(42), wantErr: true}, + } + for _, tc := range tests { + t.Run(tc.name, func(t *testing.T) { + t.Parallel() + + got, err := tierToString(tc.tier) + if tc.wantErr { + if err == nil { + t.Fatalf("tierToString(%v) = %q, want error", tc.tier, got) + } + + return + } + if err != nil { + t.Fatalf("tierToString(%v): %v", tc.tier, err) + } + if got != tc.want { + t.Errorf("tierToString(%v) = %q, want %q", tc.tier, got, tc.want) + } + }) + } +} + +func TestTierFromString(t *testing.T) { + t.Parallel() + + tests := []struct { + name string + in string + want authzv1.Tier + wantErr bool + }{ + {name: "view", in: "view", want: authzv1.Tier_TIER_VIEW}, + {name: "use", in: "use", want: authzv1.Tier_TIER_USE}, + {name: "edit", in: "edit", want: authzv1.Tier_TIER_EDIT}, + {name: "rejects empty string", in: "", wantErr: true}, + {name: "rejects wrong case", in: "VIEW", wantErr: true}, + {name: "rejects unknown value", in: "admin", wantErr: true}, + } + for _, tc := range tests { + t.Run(tc.name, func(t *testing.T) { + t.Parallel() + + got, err := tierFromString(tc.in) + if tc.wantErr { + if err == nil { + t.Fatalf("tierFromString(%q) = %v, want error", tc.in, got) + } + + return + } + if err != nil { + t.Fatalf("tierFromString(%q): %v", tc.in, err) + } + if got != tc.want { + t.Errorf("tierFromString(%q) = %v, want %v", tc.in, got, tc.want) + } + }) + } +} + +// TestTierRoundTrip asserts every schema-accepted tier string survives +// string -> enum -> string unchanged, so the validator list and the +// converters can't drift apart. +func TestTierRoundTrip(t *testing.T) { + t.Parallel() + + for _, name := range tierValues() { + tier, err := tierFromString(name) + if err != nil { + t.Fatalf("tierFromString(%q): %v", name, err) + } + back, err := tierToString(tier) + if err != nil { + t.Fatalf("tierToString(%v): %v", tier, err) + } + if back != name { + t.Errorf("round trip of %q produced %q", name, back) + } + } +} + +func TestParseGrantImportID(t *testing.T) { + t.Parallel() + + tests := []struct { + name string + id string + wantResource string + wantGrantee string + wantErr bool + }{ + { + name: "resource and grantee", + id: "s2r:prod:saml2idp:team1/res1,s2r:prod:user:u1", + wantResource: "s2r:prod:saml2idp:team1/res1", + wantGrantee: "s2r:prod:user:u1", + }, + {name: "rejects missing comma", id: "s2r:prod:saml2idp:team1/res1", wantErr: true}, + {name: "rejects empty string", id: "", wantErr: true}, + {name: "rejects empty resource part", id: ",s2r:prod:user:u1", wantErr: true}, + {name: "rejects empty grantee part", id: "s2r:prod:saml2idp:team1/res1,", wantErr: true}, + {name: "rejects extra comma", id: "a,b,c", wantErr: true}, + } + for _, tc := range tests { + t.Run(tc.name, func(t *testing.T) { + t.Parallel() + + gotResource, gotGrantee, err := parseGrantImportID(tc.id) + if tc.wantErr { + if err == nil { + t.Fatalf("parseGrantImportID(%q) = (%q, %q), want error", tc.id, gotResource, gotGrantee) + } + + return + } + if err != nil { + t.Fatalf("parseGrantImportID(%q): %v", tc.id, err) + } + if gotResource != tc.wantResource || gotGrantee != tc.wantGrantee { + t.Errorf("parseGrantImportID(%q) = (%q, %q), want (%q, %q)", tc.id, gotResource, gotGrantee, tc.wantResource, tc.wantGrantee) + } + }) + } +} diff --git a/splitsecure/services/org/group.go b/splitsecure/services/org/group.go new file mode 100644 index 0000000..fd392f4 --- /dev/null +++ b/splitsecure/services/org/group.go @@ -0,0 +1,477 @@ +package org + +import ( + "context" + "errors" + "fmt" + "sort" + "strings" + + "connectrpc.com/connect" + "github.com/hashicorp/terraform-plugin-framework-validators/stringvalidator" + "github.com/hashicorp/terraform-plugin-framework/diag" + "github.com/hashicorp/terraform-plugin-framework/path" + "github.com/hashicorp/terraform-plugin-framework/resource" + "github.com/hashicorp/terraform-plugin-framework/resource/schema" + "github.com/hashicorp/terraform-plugin-framework/resource/schema/planmodifier" + "github.com/hashicorp/terraform-plugin-framework/resource/schema/stringplanmodifier" + "github.com/hashicorp/terraform-plugin-framework/schema/validator" + "github.com/hashicorp/terraform-plugin-framework/types" + + orgsvcv1 "github.com/splitsecure/apis/gen/go/proto/splitsecure/orgsvc/v1" + "github.com/splitsecure/terraform-provider-splitsecure/splitsecure/client" +) + +var ( + _ resource.Resource = (*groupResource)(nil) + _ resource.ResourceWithImportState = (*groupResource)(nil) +) + +const ( + titleCreatingGroup = "Creating group" + titleAddingMembers = "Adding group members" + titleReadingGroup = "Reading group" +) + +var ( + errEmptyGroup = errors.New("CreateGroup returned no group_s2r; the group may exist server-side but cannot be tracked in state") + errMembersNotAdded = errors.New("some members could not be added") +) + +type groupResource struct { + client *client.Client +} + +type groupResourceModel struct { + GroupS2R types.String `tfsdk:"group_s2r"` + Name types.String `tfsdk:"name"` + Members types.Set `tfsdk:"members"` + Source types.String `tfsdk:"source"` +} + +// NewGroup returns a factory for the org group resource. +func NewGroup() resource.Resource { + return &groupResource{} +} + +func (r *groupResource) Metadata(_ context.Context, req resource.MetadataRequest, resp *resource.MetadataResponse) { + resp.TypeName = req.ProviderTypeName + "_group" +} + +func (r *groupResource) Configure(_ context.Context, req resource.ConfigureRequest, resp *resource.ConfigureResponse) { + if req.ProviderData == nil { + return + } + c, ok := req.ProviderData.(*client.Client) + if !ok { + resp.Diagnostics.AddError( + "Unexpected provider data type", + fmt.Sprintf("expected *client.Client, got %T", req.ProviderData), + ) + + return + } + r.client = c +} + +func (r *groupResource) Schema(_ context.Context, _ resource.SchemaRequest, resp *resource.SchemaResponse) { + resp.Schema = schema.Schema{ + Description: "Locally-managed principal group in the provider-configured organization. " + + "Terraform is authoritative over the member set; SCIM- and system-managed groups cannot be managed by this resource.", + Attributes: map[string]schema.Attribute{ + "group_s2r": schema.StringAttribute{ + Computed: true, + Description: "Group s2r URI. Stable identifier; also the import ID.", + PlanModifiers: []planmodifier.String{stringplanmodifier.UseStateForUnknown()}, + }, + "name": schema.StringAttribute{ + Required: true, + Description: "Human-readable group name. Changing it updates the group in place.", + Validators: []validator.String{ + stringvalidator.LengthAtLeast(1), + }, + }, + "members": schema.SetAttribute{ + ElementType: types.StringType, + Optional: true, + Description: "Authoritative set of member principal s2r URIs (users or service accounts). " + + "Principals not listed here are removed on apply. Leave unset for an empty group.", + }, + "source": schema.StringAttribute{ + Computed: true, + Description: `Where the group is managed from: "local", "scim", or "system". Always "local" for Terraform-managed groups.`, + PlanModifiers: []planmodifier.String{stringplanmodifier.UseStateForUnknown()}, + }, + }, + } +} + +func (r *groupResource) Create(ctx context.Context, req resource.CreateRequest, resp *resource.CreateResponse) { + var plan groupResourceModel + resp.Diagnostics.Append(req.Plan.Get(ctx, &plan)...) + if resp.Diagnostics.HasError() { + return + } + + configured := membersFromSet(ctx, plan.Members, &resp.Diagnostics) + if resp.Diagnostics.HasError() { + return + } + + groupS2R, src, err := r.createGroupShell(ctx, plan.Name.ValueString()) + if err != nil { + if groupS2R != "" { + // The group exists; record it so it is not orphaned from state. + plan.GroupS2R = types.StringValue(groupS2R) + plan.Source = types.StringNull() + if len(configured) > 0 { + plan.Members = types.SetNull(types.StringType) + } + resp.Diagnostics.Append(resp.State.Set(ctx, &plan)...) + } + resp.Diagnostics.AddError(titleCreatingGroup, err.Error()) + + return + } + plan.GroupS2R = types.StringValue(groupS2R) + plan.Source = types.StringValue(src) + + if len(configured) == 0 { + resp.Diagnostics.Append(resp.State.Set(ctx, &plan)...) + + return + } + + addResp, err := r.client.OrgService.AddGroupMembers(ctx, connect.NewRequest(&orgsvcv1.AddGroupMembersRequest{ + GroupS2R: groupS2R, + PrincipalS2Rs: configured, + })) + if err != nil { + // Membership is unknown after a wholesale RPC failure; record + // the group itself so it is not orphaned from state. + plan.Members = types.SetNull(types.StringType) + resp.Diagnostics.Append(resp.State.Set(ctx, &plan)...) + resp.Diagnostics.AddError(titleAddingMembers, err.Error()) + + return + } + + landed, failures := partitionAddResults(addResp.Msg.GetResults()) + if len(failures) > 0 { + landedSet, d := types.SetValueFrom(ctx, types.StringType, landed) + resp.Diagnostics.Append(d...) + plan.Members = landedSet + resp.Diagnostics.Append(resp.State.Set(ctx, &plan)...) + resp.Diagnostics.AddError( + titleAddingMembers, + fmt.Sprintf("group %s was created and recorded in state, but some members could not be added:\n%s", + groupS2R, strings.Join(failures, "\n")), + ) + + return + } + + resp.Diagnostics.Append(resp.State.Set(ctx, &plan)...) +} + +func (r *groupResource) Read(ctx context.Context, req resource.ReadRequest, resp *resource.ReadResponse) { + var state groupResourceModel + resp.Diagnostics.Append(req.State.Get(ctx, &state)...) + if resp.Diagnostics.HasError() { + return + } + + g, src := r.fetchLocalGroup(ctx, state.GroupS2R.ValueString(), &resp.Diagnostics) + if resp.Diagnostics.HasError() { + return + } + if g == nil { + resp.State.RemoveResource(ctx) + + return + } + + principals, err := r.listMemberPrincipals(ctx, g.GetGroupS2R()) + if err != nil { + resp.Diagnostics.AddError("Listing group members", err.Error()) + + return + } + + state.Name = types.StringValue(g.GetName()) + state.Source = types.StringValue(src) + // An unset members attribute means empty membership; keep it null + // when the group is in fact empty so imports and member-less + // configs stay diff-free. + if len(principals) > 0 || !state.Members.IsNull() { + membersVal, d := types.SetValueFrom(ctx, types.StringType, principals) + resp.Diagnostics.Append(d...) + if resp.Diagnostics.HasError() { + return + } + state.Members = membersVal + } + resp.Diagnostics.Append(resp.State.Set(ctx, &state)...) +} + +func (r *groupResource) Update(ctx context.Context, req resource.UpdateRequest, resp *resource.UpdateResponse) { + var plan, state groupResourceModel + resp.Diagnostics.Append(req.Plan.Get(ctx, &plan)...) + resp.Diagnostics.Append(req.State.Get(ctx, &state)...) + if resp.Diagnostics.HasError() { + return + } + + groupS2R := state.GroupS2R.ValueString() + planMembers := membersFromSet(ctx, plan.Members, &resp.Diagnostics) + stateMembers := membersFromSet(ctx, state.Members, &resp.Diagnostics) + if resp.Diagnostics.HasError() { + return + } + + // applied tracks what has actually been persisted server-side so + // far; error paths write it to state because the framework + // pre-populates resp.State with the plan, which would otherwise + // record changes that never happened. + applied := state + applied.GroupS2R = types.StringValue(groupS2R) + + if !plan.Name.Equal(state.Name) { + _, err := r.client.OrgService.UpdateGroup(ctx, connect.NewRequest(&orgsvcv1.UpdateGroupRequest{ + GroupS2R: groupS2R, + Name: plan.Name.ValueString(), + })) + if err != nil { + resp.Diagnostics.Append(resp.State.Set(ctx, &applied)...) + resp.Diagnostics.AddError("Updating group name", err.Error()) + + return + } + applied.Name = plan.Name + } + + current, title, err := r.reconcileMembers(ctx, groupS2R, stateMembers, planMembers) + if err != nil { + membersVal, d := types.SetValueFrom(ctx, types.StringType, current) + resp.Diagnostics.Append(d...) + applied.Members = membersVal + resp.Diagnostics.Append(resp.State.Set(ctx, &applied)...) + resp.Diagnostics.AddError(title, err.Error()) + + return + } + + plan.GroupS2R = types.StringValue(groupS2R) + if plan.Source.IsUnknown() { + plan.Source = state.Source + } + resp.Diagnostics.Append(resp.State.Set(ctx, &plan)...) +} + +func (r *groupResource) Delete(ctx context.Context, req resource.DeleteRequest, resp *resource.DeleteResponse) { + var state groupResourceModel + resp.Diagnostics.Append(req.State.Get(ctx, &state)...) + if resp.Diagnostics.HasError() { + return + } + + _, err := r.client.OrgService.DeleteGroup(ctx, connect.NewRequest(&orgsvcv1.DeleteGroupRequest{ + GroupS2R: state.GroupS2R.ValueString(), + })) + if err != nil && connect.CodeOf(err) != connect.CodeNotFound { + resp.Diagnostics.AddError("Deleting group", err.Error()) + } +} + +func (r *groupResource) ImportState(ctx context.Context, req resource.ImportStateRequest, resp *resource.ImportStateResponse) { + resource.ImportStatePassthroughID(ctx, path.Root("group_s2r"), req, resp) +} + +// createGroupShell creates the empty local group. A non-empty groupS2R +// alongside a non-nil error means the group exists server-side but its +// response was unusable; the caller must still record it in state. +func (r *groupResource) createGroupShell(ctx context.Context, name string) (string, string, error) { + createResp, err := r.client.OrgService.CreateGroup(ctx, connect.NewRequest(&orgsvcv1.CreateGroupRequest{ + OrgS2R: r.client.OrgS2R, + Name: name, + Source: orgsvcv1.GroupSource_GROUP_SOURCE_LOCAL, + })) + if err != nil { + return "", "", err + } + + g := createResp.Msg.GetGroup() + if g.GetGroupS2R() == "" { + return "", "", errEmptyGroup + } + + src, err := groupSourceToString(g.GetSource()) + if err != nil { + return g.GetGroupS2R(), "", err + } + + return g.GetGroupS2R(), src, nil +} + +// fetchLocalGroup returns the group and its source string when it +// exists and is locally managed. A nil group with no appended +// diagnostics means the group is gone (NotFound). +func (r *groupResource) fetchLocalGroup(ctx context.Context, groupS2R string, diags *diag.Diagnostics) (*orgsvcv1.Group, string) { + getResp, err := r.client.OrgService.GetGroup(ctx, connect.NewRequest(&orgsvcv1.GetGroupRequest{ + GroupS2R: groupS2R, + })) + if err != nil { + if connect.CodeOf(err) != connect.CodeNotFound { + diags.AddError(titleReadingGroup, err.Error()) + } + + return nil, "" + } + + g := getResp.Msg.GetGroup() + if g.GetGroupS2R() == "" { + diags.AddError(titleReadingGroup, "GetGroup returned an empty group for "+groupS2R) + + return nil, "" + } + if g.GetSource() != orgsvcv1.GroupSource_GROUP_SOURCE_LOCAL { + diags.AddError( + "Group is not locally managed", + fmt.Sprintf("%s has source %s; only locally-managed groups can be managed by Terraform. "+ + "SCIM groups are owned by the identity provider, and the system Everyone group is implicit.", + g.GetGroupS2R(), g.GetSource()), + ) + + return nil, "" + } + src, err := groupSourceToString(g.GetSource()) + if err != nil { + diags.AddError(titleReadingGroup, err.Error()) + + return nil, "" + } + + return g, src +} + +func (r *groupResource) listMemberPrincipals(ctx context.Context, groupS2R string) ([]string, error) { + membersResp, err := r.client.OrgService.ListGroupMembers(ctx, connect.NewRequest(&orgsvcv1.ListGroupMembersRequest{ + GroupS2R: groupS2R, + })) + if err != nil { + return nil, err + } + principals := make([]string, 0, len(membersResp.Msg.GetMembers())) + for _, m := range membersResp.Msg.GetMembers() { + principals = append(principals, m.GetPrincipalS2R()) + } + + return principals, nil +} + +// reconcileMembers applies the membership diff from state to plan: +// additions first (per-principal results, fail closed), then removals. +// The returned slice always reflects the principals that are members +// after the calls that actually succeeded, so error paths can persist +// an accurate state. +func (r *groupResource) reconcileMembers(ctx context.Context, groupS2R string, stateMembers, planMembers []string) ([]string, string, error) { + toAdd, toRemove := diffMembers(stateMembers, planMembers) + + current := make([]string, 0, len(stateMembers)+len(toAdd)) + current = append(current, stateMembers...) + + if len(toAdd) > 0 { + addResp, err := r.client.OrgService.AddGroupMembers(ctx, connect.NewRequest(&orgsvcv1.AddGroupMembersRequest{ + GroupS2R: groupS2R, + PrincipalS2Rs: toAdd, + })) + if err != nil { + return current, titleAddingMembers, err + } + landed, failures := partitionAddResults(addResp.Msg.GetResults()) + current = append(current, landed...) + if len(failures) > 0 { + return current, titleAddingMembers, + fmt.Errorf("%w to group %s:\n%s", errMembersNotAdded, groupS2R, strings.Join(failures, "\n")) + } + } + + for _, principal := range toRemove { + _, err := r.client.OrgService.RemoveGroupMember(ctx, connect.NewRequest(&orgsvcv1.RemoveGroupMemberRequest{ + GroupS2R: groupS2R, + PrincipalS2R: principal, + })) + if err != nil { + return current, "Removing group member", + fmt.Errorf("removing %s from group %s: %w", principal, groupS2R, err) + } + remaining := current[:0] + for _, m := range current { + if m != principal { + remaining = append(remaining, m) + } + } + current = remaining + } + + return current, "", nil +} + +// membersFromSet extracts the principal list from a members set; null +// and unknown sets mean no members. +func membersFromSet(ctx context.Context, s types.Set, diags *diag.Diagnostics) []string { + if s.IsNull() || s.IsUnknown() { + return nil + } + var out []string + diags.Append(s.ElementsAs(ctx, &out, false)...) + + return out +} + +// diffMembers computes the membership changes needed to go from state +// to plan. Output is sorted so API call order is deterministic. +func diffMembers(state, plan []string) ([]string, []string) { + inState := make(map[string]struct{}, len(state)) + for _, p := range state { + inState[p] = struct{}{} + } + inPlan := make(map[string]struct{}, len(plan)) + for _, p := range plan { + inPlan[p] = struct{}{} + } + + var toAdd, toRemove []string + for p := range inPlan { + if _, ok := inState[p]; !ok { + toAdd = append(toAdd, p) + } + } + for p := range inState { + if _, ok := inPlan[p]; !ok { + toRemove = append(toRemove, p) + } + } + sort.Strings(toAdd) + sort.Strings(toRemove) + + return toAdd, toRemove +} + +// partitionAddResults splits AddGroupMembers results into principals +// that are now members (AlreadyMember counts as success) and +// per-principal failure descriptions. +func partitionAddResults(results []*orgsvcv1.AddGroupMembersResponse_Result) ([]string, []string) { + added := make([]string, 0, len(results)) + var failures []string + for _, res := range results { + if res.GetError() != "" { + failures = append(failures, fmt.Sprintf("%s: %s", res.GetPrincipalS2R(), res.GetError())) + + continue + } + added = append(added, res.GetPrincipalS2R()) + } + + return added, failures +} diff --git a/splitsecure/services/org/group_data.go b/splitsecure/services/org/group_data.go new file mode 100644 index 0000000..0f7721d --- /dev/null +++ b/splitsecure/services/org/group_data.go @@ -0,0 +1,166 @@ +package org + +import ( + "context" + "errors" + "fmt" + "strings" + + "connectrpc.com/connect" + "github.com/hashicorp/terraform-plugin-framework-validators/stringvalidator" + "github.com/hashicorp/terraform-plugin-framework/datasource" + "github.com/hashicorp/terraform-plugin-framework/datasource/schema" + "github.com/hashicorp/terraform-plugin-framework/schema/validator" + "github.com/hashicorp/terraform-plugin-framework/types" + + orgsvcv1 "github.com/splitsecure/apis/gen/go/proto/splitsecure/orgsvc/v1" + "github.com/splitsecure/terraform-provider-splitsecure/splitsecure/client" +) + +var _ datasource.DataSource = (*groupDataSource)(nil) + +var ( + errGroupNotFound = errors.New("group not found") + errAmbiguousGroupName = errors.New("ambiguous group name") + errUnknownGroupSource = errors.New("unknown group source") +) + +type groupDataSource struct { + client *client.Client +} + +type groupDataSourceModel struct { + Name types.String `tfsdk:"name"` + GroupS2R types.String `tfsdk:"group_s2r"` + Source types.String `tfsdk:"source"` +} + +// NewGroupDataSource returns a factory for the splitsecure_group data +// source. +func NewGroupDataSource() datasource.DataSource { + return &groupDataSource{} +} + +func (d *groupDataSource) Metadata(_ context.Context, req datasource.MetadataRequest, resp *datasource.MetadataResponse) { + resp.TypeName = req.ProviderTypeName + "_group" +} + +func (d *groupDataSource) Configure(_ context.Context, req datasource.ConfigureRequest, resp *datasource.ConfigureResponse) { + if req.ProviderData == nil { + return + } + c, ok := req.ProviderData.(*client.Client) + if !ok { + resp.Diagnostics.AddError( + "Unexpected provider data type", + fmt.Sprintf("expected *client.Client, got %T", req.ProviderData), + ) + + return + } + d.client = c +} + +func (d *groupDataSource) Schema(_ context.Context, _ datasource.SchemaRequest, resp *datasource.SchemaResponse) { + resp.Schema = schema.Schema{ + Description: "Looks up a single org group by exact name. Errors if no group or more than one group matches " + + "(org group names are not unique server-side). The system Everyone group is not listed; " + + "read everyone_group_s2r from the splitsecure_organization data source instead.", + Attributes: map[string]schema.Attribute{ + "name": schema.StringAttribute{ + Required: true, + Description: "Group name to look up. Matched exactly (case-sensitive).", + Validators: []validator.String{ + stringvalidator.LengthAtLeast(1), + }, + }, + "group_s2r": schema.StringAttribute{ + Computed: true, + Description: "Group s2r URI. Usable as a grant grantee.", + }, + "source": schema.StringAttribute{ + Computed: true, + Description: `Where the group is managed: "local", "scim", or "system".`, + }, + }, + } +} + +func (d *groupDataSource) Read(ctx context.Context, req datasource.ReadRequest, resp *datasource.ReadResponse) { + var config groupDataSourceModel + resp.Diagnostics.Append(req.Config.Get(ctx, &config)...) + if resp.Diagnostics.HasError() { + return + } + + listResp, err := d.client.OrgService.ListGroups(ctx, connect.NewRequest(&orgsvcv1.ListGroupsRequest{ + OrgS2R: d.client.OrgS2R, + })) + if err != nil { + resp.Diagnostics.AddError("ListGroups", err.Error()) + + return + } + + group, err := matchGroupByName(listResp.Msg.GetGroups(), config.Name.ValueString()) + if err != nil { + resp.Diagnostics.AddError("Looking up group", err.Error()) + + return + } + + source, err := groupSourceToString(group.GetSource()) + if err != nil { + resp.Diagnostics.AddError("Decoding group source", err.Error()) + + return + } + + config.GroupS2R = types.StringValue(group.GetGroupS2R()) + config.Source = types.StringValue(source) + resp.Diagnostics.Append(resp.State.Set(ctx, &config)...) +} + +// matchGroupByName returns the single group whose name equals the given +// name exactly (case-sensitive). Zero or multiple matches are errors. +func matchGroupByName(groups []*orgsvcv1.Group, name string) (*orgsvcv1.Group, error) { + var matches []*orgsvcv1.Group + for _, g := range groups { + if g.GetName() == name { + matches = append(matches, g) + } + } + + switch len(matches) { + case 0: + return nil, fmt.Errorf( + "%w: no group named %q; note the system Everyone group is not returned by group listings -- "+ + "read everyone_group_s2r from the splitsecure_organization data source instead", + errGroupNotFound, name) + case 1: + return matches[0], nil + default: + s2rs := make([]string, len(matches)) + for i, g := range matches { + s2rs[i] = g.GetGroupS2R() + } + + return nil, fmt.Errorf("%w: %q matches %d groups (org group names are not unique): %s", + errAmbiguousGroupName, name, len(matches), strings.Join(s2rs, ", ")) + } +} + +func groupSourceToString(s orgsvcv1.GroupSource) (string, error) { + switch s { + case orgsvcv1.GroupSource_GROUP_SOURCE_LOCAL: + return "local", nil + case orgsvcv1.GroupSource_GROUP_SOURCE_SCIM: + return "scim", nil + case orgsvcv1.GroupSource_GROUP_SOURCE_SYSTEM: + return "system", nil + case orgsvcv1.GroupSource_GROUP_SOURCE_UNSPECIFIED: + return "", fmt.Errorf("%w: %s", errUnknownGroupSource, s) + default: + return "", fmt.Errorf("%w: %s", errUnknownGroupSource, s) + } +} diff --git a/splitsecure/services/org/group_internal_test.go b/splitsecure/services/org/group_internal_test.go new file mode 100644 index 0000000..493376e --- /dev/null +++ b/splitsecure/services/org/group_internal_test.go @@ -0,0 +1,252 @@ +package org + +import ( + "context" + "slices" + "strings" + "testing" + + "github.com/hashicorp/terraform-plugin-framework/resource" + rschema "github.com/hashicorp/terraform-plugin-framework/resource/schema" + "github.com/hashicorp/terraform-plugin-framework/types" + + orgsvcv1 "github.com/splitsecure/apis/gen/go/proto/splitsecure/orgsvc/v1" +) + +func groupResourceSchema(t *testing.T) rschema.Schema { + t.Helper() + + resp := &resource.SchemaResponse{} + (&groupResource{}).Schema(context.Background(), resource.SchemaRequest{}, resp) + if resp.Diagnostics.HasError() { + t.Fatalf("schema diagnostics: %v", resp.Diagnostics) + } + + return resp.Schema +} + +func TestGroupSchema_AttributeModes(t *testing.T) { + t.Parallel() + + s := groupResourceSchema(t) + + name, ok := s.Attributes["name"] + if !ok { + t.Fatal("name attribute missing from group schema") + } + if !name.IsRequired() { + t.Error("name should be Required") + } + + members, ok := s.Attributes["members"] + if !ok { + t.Fatal("members attribute missing from group schema") + } + if !members.IsOptional() || members.IsRequired() || members.IsComputed() { + t.Error("members should be Optional only (authoritative config-owned set)") + } + + for _, attrName := range []string{"group_s2r", "source"} { + attr, ok := s.Attributes[attrName] + if !ok { + t.Fatalf("computed attribute %q missing from group schema", attrName) + } + if !attr.IsComputed() { + t.Errorf("attribute %q should be Computed", attrName) + } + } +} + +func TestGroupSchema_MembersIsStringSet(t *testing.T) { + t.Parallel() + + s := groupResourceSchema(t) + + members, ok := s.Attributes["members"].(rschema.SetAttribute) + if !ok { + t.Fatalf("members is %T, want SetAttribute", s.Attributes["members"]) + } + if !members.ElementType.Equal(types.StringType) { + t.Errorf("members element type is %s, want string", members.ElementType) + } +} + +// TestGroupSchema_GroupS2RKeepsState catches removal of the +// UseStateForUnknown modifier, which would flip group_s2r to "known +// after apply" on every update. +func TestGroupSchema_GroupS2RKeepsState(t *testing.T) { + t.Parallel() + + s := groupResourceSchema(t) + + attr, ok := s.Attributes["group_s2r"].(rschema.StringAttribute) + if !ok { + t.Fatalf("group_s2r is %T, want StringAttribute", s.Attributes["group_s2r"]) + } + if len(attr.PlanModifiers) == 0 { + t.Error("group_s2r should carry the UseStateForUnknown plan modifier") + } +} + +func TestGroupSourceMapping(t *testing.T) { + t.Parallel() + + cases := []struct { + name string + source orgsvcv1.GroupSource + want string + wantError bool + }{ + {name: "local", source: orgsvcv1.GroupSource_GROUP_SOURCE_LOCAL, want: "local"}, + {name: "scim", source: orgsvcv1.GroupSource_GROUP_SOURCE_SCIM, want: "scim"}, + {name: "system", source: orgsvcv1.GroupSource_GROUP_SOURCE_SYSTEM, want: "system"}, + {name: "unspecified returns error", source: orgsvcv1.GroupSource_GROUP_SOURCE_UNSPECIFIED, wantError: true}, + {name: "unknown enum value returns error", source: orgsvcv1.GroupSource(42), wantError: true}, + } + + for _, tc := range cases { + t.Run(tc.name, func(t *testing.T) { + t.Parallel() + + got, err := groupSourceToString(tc.source) + if tc.wantError { + if err == nil { + t.Fatalf("groupSourceToString(%v) = %q, want error", tc.source, got) + } + + return + } + if err != nil { + t.Fatalf("groupSourceToString(%v): %v", tc.source, err) + } + if got != tc.want { + t.Fatalf("groupSourceToString(%v) = %q, want %q", tc.source, got, tc.want) + } + }) + } +} + +func TestDiffMembers(t *testing.T) { + t.Parallel() + + cases := []struct { + name string + state []string + plan []string + wantAdd []string + wantRemove []string + }{ + { + name: "no change", + state: []string{"s2r:us:user:a", "s2r:us:user:b"}, + plan: []string{"s2r:us:user:b", "s2r:us:user:a"}, + }, + { + name: "pure additions", + state: []string{"s2r:us:user:a"}, + plan: []string{"s2r:us:user:a", "s2r:us:user:c", "s2r:us:user:b"}, + wantAdd: []string{"s2r:us:user:b", "s2r:us:user:c"}, + }, + { + name: "pure removals", + state: []string{"s2r:us:user:a", "s2r:us:user:b"}, + plan: []string{"s2r:us:user:a"}, + wantRemove: []string{"s2r:us:user:b"}, + }, + { + name: "mixed add and remove", + state: []string{"s2r:us:user:a", "s2r:us:user:b"}, + plan: []string{"s2r:us:user:b", "s2r:us:sa:x"}, + wantAdd: []string{"s2r:us:sa:x"}, + wantRemove: []string{"s2r:us:user:a"}, + }, + { + name: "from empty", + plan: []string{"s2r:us:user:a"}, + wantAdd: []string{"s2r:us:user:a"}, + }, + { + name: "to empty", + state: []string{"s2r:us:user:a"}, + wantRemove: []string{"s2r:us:user:a"}, + }, + { + name: "both empty", + }, + } + + for _, tc := range cases { + t.Run(tc.name, func(t *testing.T) { + t.Parallel() + + gotAdd, gotRemove := diffMembers(tc.state, tc.plan) + if !slices.Equal(gotAdd, tc.wantAdd) { + t.Errorf("toAdd = %v, want %v", gotAdd, tc.wantAdd) + } + if !slices.Equal(gotRemove, tc.wantRemove) { + t.Errorf("toRemove = %v, want %v", gotRemove, tc.wantRemove) + } + }) + } +} + +func TestPartitionAddResults(t *testing.T) { + t.Parallel() + + results := []*orgsvcv1.AddGroupMembersResponse_Result{ + {PrincipalS2R: "s2r:us:user:ok"}, + {PrincipalS2R: "s2r:us:user:dupe", AlreadyMember: true}, + {PrincipalS2R: "s2r:us:user:bad", Error: "principal not found"}, + {PrincipalS2R: "s2r:us:sa:worse", Error: "not in org"}, + } + + added, failures := partitionAddResults(results) + + wantAdded := []string{"s2r:us:user:ok", "s2r:us:user:dupe"} + if !slices.Equal(added, wantAdded) { + t.Errorf("added = %v, want %v", added, wantAdded) + } + + if len(failures) != 2 { + t.Fatalf("got %d failures, want 2: %v", len(failures), failures) + } + for i, want := range []struct{ principal, reason string }{ + {"s2r:us:user:bad", "principal not found"}, + {"s2r:us:sa:worse", "not in org"}, + } { + if !strings.Contains(failures[i], want.principal) || !strings.Contains(failures[i], want.reason) { + t.Errorf("failures[%d] = %q, want it to mention %q and %q", i, failures[i], want.principal, want.reason) + } + } +} + +func TestPartitionAddResults_AllSucceed(t *testing.T) { + t.Parallel() + + added, failures := partitionAddResults([]*orgsvcv1.AddGroupMembersResponse_Result{ + {PrincipalS2R: "s2r:us:user:a"}, + {PrincipalS2R: "s2r:us:user:b", AlreadyMember: true}, + }) + + if !slices.Equal(added, []string{"s2r:us:user:a", "s2r:us:user:b"}) { + t.Errorf("added = %v, want both principals", added) + } + if len(failures) != 0 { + t.Errorf("failures = %v, want none", failures) + } +} + +func TestPartitionAddResults_EmptyInputYieldsNonNilAdded(t *testing.T) { + t.Parallel() + + added, failures := partitionAddResults(nil) + + // A non-nil slice matters: types.SetValueFrom turns nil into a + // null set, which would corrupt the partial-failure state write. + if added == nil { + t.Error("added should be non-nil for empty input") + } + if len(added) != 0 || len(failures) != 0 { + t.Errorf("added = %v, failures = %v, want both empty", added, failures) + } +} diff --git a/splitsecure/services/org/member_data.go b/splitsecure/services/org/member_data.go new file mode 100644 index 0000000..7c078cd --- /dev/null +++ b/splitsecure/services/org/member_data.go @@ -0,0 +1,137 @@ +package org + +import ( + "context" + "errors" + "fmt" + "strings" + + "connectrpc.com/connect" + "github.com/hashicorp/terraform-plugin-framework-validators/stringvalidator" + "github.com/hashicorp/terraform-plugin-framework/datasource" + "github.com/hashicorp/terraform-plugin-framework/datasource/schema" + "github.com/hashicorp/terraform-plugin-framework/schema/validator" + "github.com/hashicorp/terraform-plugin-framework/types" + + orgsvcv1 "github.com/splitsecure/apis/gen/go/proto/splitsecure/orgsvc/v1" + "github.com/splitsecure/terraform-provider-splitsecure/splitsecure/client" +) + +var _ datasource.DataSource = (*memberDataSource)(nil) + +var ( + errNoOrgMember = errors.New("no org member") + errAmbiguousMemberEmail = errors.New("ambiguous member email") +) + +type memberDataSource struct { + client *client.Client +} + +type memberDataSourceModel struct { + Email types.String `tfsdk:"email"` + UserS2R types.String `tfsdk:"user_s2r"` + DisplayName types.String `tfsdk:"display_name"` +} + +// NewMemberDataSource returns a factory for the splitsecure_org_member +// data source. +func NewMemberDataSource() datasource.DataSource { + return &memberDataSource{} +} + +func (d *memberDataSource) Metadata(_ context.Context, req datasource.MetadataRequest, resp *datasource.MetadataResponse) { + resp.TypeName = req.ProviderTypeName + "_org_member" +} + +func (d *memberDataSource) Configure(_ context.Context, req datasource.ConfigureRequest, resp *datasource.ConfigureResponse) { + if req.ProviderData == nil { + return + } + c, ok := req.ProviderData.(*client.Client) + if !ok { + resp.Diagnostics.AddError( + "Unexpected provider data type", + fmt.Sprintf("expected *client.Client, got %T", req.ProviderData), + ) + + return + } + d.client = c +} + +func (d *memberDataSource) Schema(_ context.Context, _ datasource.SchemaRequest, resp *datasource.SchemaResponse) { + resp.Schema = schema.Schema{ + Description: "Looks up a single org member by email (case-insensitive). Errors if no member or more than one member matches.", + Attributes: map[string]schema.Attribute{ + "email": schema.StringAttribute{ + Required: true, + Description: "Email address of the member to look up. Matched case-insensitively.", + Validators: []validator.String{ + stringvalidator.LengthAtLeast(1), + }, + }, + "user_s2r": schema.StringAttribute{ + Computed: true, + Description: "User s2r URI of the member. Usable as a grant grantee or a group member principal.", + }, + "display_name": schema.StringAttribute{ + Computed: true, + Description: "Human-readable display name of the member.", + }, + }, + } +} + +func (d *memberDataSource) Read(ctx context.Context, req datasource.ReadRequest, resp *datasource.ReadResponse) { + var config memberDataSourceModel + resp.Diagnostics.Append(req.Config.Get(ctx, &config)...) + if resp.Diagnostics.HasError() { + return + } + + listResp, err := d.client.OrgService.ListMembers(ctx, connect.NewRequest(&orgsvcv1.ListMembersRequest{ + Base: &orgsvcv1.ListMembersRequest_Base{OrganizationId: d.client.OrgS2R}, + })) + if err != nil { + resp.Diagnostics.AddError("ListMembers", err.Error()) + + return + } + + member, err := matchMemberByEmail(listResp.Msg.GetMembers(), config.Email.ValueString()) + if err != nil { + resp.Diagnostics.AddError("Looking up org member", err.Error()) + + return + } + + config.UserS2R = types.StringValue(member.GetUserId()) + config.DisplayName = types.StringValue(member.GetDisplayName()) + resp.Diagnostics.Append(resp.State.Set(ctx, &config)...) +} + +// matchMemberByEmail returns the single member whose email equals the +// given email case-insensitively. Zero or multiple matches are errors. +func matchMemberByEmail(members []*orgsvcv1.Member, email string) (*orgsvcv1.Member, error) { + var matches []*orgsvcv1.Member + for _, m := range members { + if strings.EqualFold(m.GetEmail(), email) { + matches = append(matches, m) + } + } + + switch len(matches) { + case 0: + return nil, fmt.Errorf("%w with email %q", errNoOrgMember, email) + case 1: + return matches[0], nil + default: + ids := make([]string, len(matches)) + for i, m := range matches { + ids[i] = m.GetUserId() + } + + return nil, fmt.Errorf("%w: %q matches %d members: %s", errAmbiguousMemberEmail, email, len(matches), strings.Join(ids, ", ")) + } +} diff --git a/splitsecure/services/org/organization_data.go b/splitsecure/services/org/organization_data.go new file mode 100644 index 0000000..744aaf6 --- /dev/null +++ b/splitsecure/services/org/organization_data.go @@ -0,0 +1,102 @@ +package org + +import ( + "context" + "errors" + "fmt" + + "connectrpc.com/connect" + "github.com/hashicorp/terraform-plugin-framework/datasource" + "github.com/hashicorp/terraform-plugin-framework/datasource/schema" + "github.com/hashicorp/terraform-plugin-framework/types" + + orgsvcv1 "github.com/splitsecure/apis/gen/go/proto/splitsecure/orgsvc/v1" + "github.com/splitsecure/terraform-provider-splitsecure/splitsecure/client" +) + +var _ datasource.DataSource = (*organizationDataSource)(nil) + +var errEmptyOrganization = errors.New("GetOrganization returned an empty organization") + +type organizationDataSource struct { + client *client.Client +} + +type organizationDataSourceModel struct { + ID types.String `tfsdk:"id"` + Name types.String `tfsdk:"name"` + EveryoneGroupS2R types.String `tfsdk:"everyone_group_s2r"` +} + +// NewOrganizationDataSource returns a factory for the +// splitsecure_organization data source. +func NewOrganizationDataSource() datasource.DataSource { + return &organizationDataSource{} +} + +func (d *organizationDataSource) Metadata(_ context.Context, req datasource.MetadataRequest, resp *datasource.MetadataResponse) { + resp.TypeName = req.ProviderTypeName + "_organization" +} + +func (d *organizationDataSource) Configure(_ context.Context, req datasource.ConfigureRequest, resp *datasource.ConfigureResponse) { + if req.ProviderData == nil { + return + } + c, ok := req.ProviderData.(*client.Client) + if !ok { + resp.Diagnostics.AddError( + "Unexpected provider data type", + fmt.Sprintf("expected *client.Client, got %T", req.ProviderData), + ) + + return + } + d.client = c +} + +func (d *organizationDataSource) Schema(_ context.Context, _ datasource.SchemaRequest, resp *datasource.SchemaResponse) { + resp.Schema = schema.Schema{ + Description: "The organization the provider is configured against. Takes no arguments; the org comes from the provider's org_s2r.", + Attributes: map[string]schema.Attribute{ + "id": schema.StringAttribute{ + Computed: true, + Description: "Org s2r URI.", + }, + "name": schema.StringAttribute{ + Computed: true, + Description: "Human-readable organization name.", + }, + "everyone_group_s2r": schema.StringAttribute{ + Computed: true, + Description: "S2R of the org's system Everyone group -- the grantee to use for org-wide grants. " + + "System groups are NOT returned by group listings (including the splitsecure_group data source); " + + "this attribute is the way to obtain it.", + }, + }, + } +} + +func (d *organizationDataSource) Read(ctx context.Context, _ datasource.ReadRequest, resp *datasource.ReadResponse) { + getResp, err := d.client.OrgService.GetOrganization(ctx, connect.NewRequest(&orgsvcv1.GetOrganizationRequest{ + Base: &orgsvcv1.GetOrganizationRequest_Base{OrganizationId: d.client.OrgS2R}, + })) + if err != nil { + resp.Diagnostics.AddError("GetOrganization", err.Error()) + + return + } + + o := getResp.Msg.GetOrganization() + if o == nil { + resp.Diagnostics.AddError("Reading organization", fmt.Errorf("%w for %s", errEmptyOrganization, d.client.OrgS2R).Error()) + + return + } + + state := organizationDataSourceModel{ + ID: types.StringValue(o.GetId()), + Name: types.StringValue(o.GetName()), + EveryoneGroupS2R: types.StringValue(o.GetEveryoneGroupS2R()), + } + resp.Diagnostics.Append(resp.State.Set(ctx, &state)...) +} From b6374db3b3a153f194ae229aaea8d7021adc9f18 Mon Sep 17 00:00:00 2001 From: Konyaka1 <47553039+Konyaka1@users.noreply.github.com> Date: Fri, 10 Jul 2026 12:10:19 -0400 Subject: [PATCH 02/15] org: address self-review findings on grants and groups - grant: tolerate no server-echo of RequiresReplace attrs by writing state from the plan on Create/Update (avoids a post-apply consistency failure if the server canonicalizes an s2r); extract the shared upsertGrant helper so Create/Update differ only by the auth retry. - group: tolerate NotFound when removing a member (matches DeleteGroup / DeleteGrant) so an already-gone principal doesn't wedge apply; drop dead UseStateForUnknown guards in Update. - extract a package-local clientFromProviderData helper used by all five resources/data sources' Configure. - extract the context-aware sleep into internal/wait and reuse it from the grant retry loop and the saml2 proposal poller. - tests: clientFromProviderData; member-removal NotFound tolerance and the non-NotFound failure path (via a minimal fake OrgService client). Co-Authored-By: Claude Fable 5 --- splitsecure/internal/wait/wait.go | 21 ++++ splitsecure/services/org/configure.go | 30 +++++ .../services/org/configure_internal_test.go | 32 ++++++ splitsecure/services/org/grant.go | 108 ++++++++---------- splitsecure/services/org/group.go | 23 +--- splitsecure/services/org/group_data.go | 14 +-- .../services/org/group_internal_test.go | 63 ++++++++++ splitsecure/services/org/member_data.go | 14 +-- splitsecure/services/org/organization_data.go | 14 +-- splitsecure/services/saml2/proposal.go | 19 +-- 10 files changed, 206 insertions(+), 132 deletions(-) create mode 100644 splitsecure/internal/wait/wait.go create mode 100644 splitsecure/services/org/configure.go create mode 100644 splitsecure/services/org/configure_internal_test.go diff --git a/splitsecure/internal/wait/wait.go b/splitsecure/internal/wait/wait.go new file mode 100644 index 0000000..b36822d --- /dev/null +++ b/splitsecure/internal/wait/wait.go @@ -0,0 +1,21 @@ +// Package wait holds small context-aware timing helpers shared across +// the provider's polling and retry loops. +package wait + +import ( + "context" + "time" +) + +// Sleep blocks for d, returning early with the context's error if it is +// cancelled or times out first. +func Sleep(ctx context.Context, d time.Duration) error { + t := time.NewTimer(d) + defer t.Stop() + select { + case <-ctx.Done(): + return ctx.Err() + case <-t.C: + return nil + } +} diff --git a/splitsecure/services/org/configure.go b/splitsecure/services/org/configure.go new file mode 100644 index 0000000..8d406c4 --- /dev/null +++ b/splitsecure/services/org/configure.go @@ -0,0 +1,30 @@ +package org + +import ( + "fmt" + + "github.com/hashicorp/terraform-plugin-framework/diag" + + "github.com/splitsecure/terraform-provider-splitsecure/splitsecure/client" +) + +// clientFromProviderData extracts the shared *client.Client the provider +// stashes in ProviderData. It returns nil when ProviderData is unset — +// the framework calls Configure with nil before the provider is +// configured — and records a diagnostic on a type mismatch. +func clientFromProviderData(providerData any, diags *diag.Diagnostics) *client.Client { + if providerData == nil { + return nil + } + c, ok := providerData.(*client.Client) + if !ok { + diags.AddError( + "Unexpected provider data type", + fmt.Sprintf("expected *client.Client, got %T", providerData), + ) + + return nil + } + + return c +} diff --git a/splitsecure/services/org/configure_internal_test.go b/splitsecure/services/org/configure_internal_test.go new file mode 100644 index 0000000..c7c08c8 --- /dev/null +++ b/splitsecure/services/org/configure_internal_test.go @@ -0,0 +1,32 @@ +package org + +import ( + "testing" + + "github.com/hashicorp/terraform-plugin-framework/diag" + + "github.com/splitsecure/terraform-provider-splitsecure/splitsecure/client" +) + +func TestClientFromProviderData(t *testing.T) { + t.Parallel() + + // nil ProviderData (the framework's early Configure call) -> nil, no error. + var d diag.Diagnostics + if got := clientFromProviderData(nil, &d); got != nil || d.HasError() { + t.Fatalf("nil provider data: got %v, diags %v", got, d) + } + + // Wrong type -> nil and a diagnostic. + d = diag.Diagnostics{} + if got := clientFromProviderData("not a client", &d); got != nil || !d.HasError() { + t.Fatalf("wrong type: got %v, diags %v", got, d) + } + + // Correct type -> returned unchanged, no error. + d = diag.Diagnostics{} + c := &client.Client{OrgS2R: "s2r:test:org:x"} + if got := clientFromProviderData(c, &d); got != c || d.HasError() { + t.Fatalf("correct type: got %v, diags %v", got, d) + } +} diff --git a/splitsecure/services/org/grant.go b/splitsecure/services/org/grant.go index b3563a0..a2b6bd1 100644 --- a/splitsecure/services/org/grant.go +++ b/splitsecure/services/org/grant.go @@ -21,6 +21,7 @@ import ( authzv1 "github.com/splitsecure/apis/gen/go/proto/splitsecure/authz/v1" orgsvcv1 "github.com/splitsecure/apis/gen/go/proto/splitsecure/orgsvc/v1" "github.com/splitsecure/terraform-provider-splitsecure/splitsecure/client" + "github.com/splitsecure/terraform-provider-splitsecure/splitsecure/internal/wait" ) var ( @@ -49,19 +50,7 @@ func (r *grantResource) Metadata(_ context.Context, req resource.MetadataRequest } func (r *grantResource) Configure(_ context.Context, req resource.ConfigureRequest, resp *resource.ConfigureResponse) { - if req.ProviderData == nil { - return - } - c, ok := req.ProviderData.(*client.Client) - if !ok { - resp.Diagnostics.AddError( - "Unexpected provider data type", - fmt.Sprintf("expected *client.Client, got %T", req.ProviderData), - ) - - return - } - r.client = c + r.client = clientFromProviderData(req.ProviderData, &resp.Diagnostics) } func (r *grantResource) Schema(_ context.Context, _ resource.SchemaRequest, resp *resource.SchemaResponse) { @@ -103,26 +92,7 @@ func (r *grantResource) Create(ctx context.Context, req resource.CreateRequest, return } - tier, err := tierFromString(plan.Tier.ValueString()) - if err != nil { - resp.Diagnostics.AddError("Invalid tier", err.Error()) - - return - } - - g, err := r.putGrantRetryingAuthz(ctx, &orgsvcv1.PutGrantRequest{ - OrgS2R: r.client.OrgS2R, - ResourceS2R: plan.ResourceS2R.ValueString(), - GranteeS2R: plan.GranteeS2R.ValueString(), - Tier: tier, - }) - if err != nil { - resp.Diagnostics.AddError("Creating grant", err.Error()) - - return - } - - resp.Diagnostics.Append(populateGrantModel(&plan, g)...) + resp.Diagnostics.Append(r.upsertGrant(ctx, plan, true)...) if resp.Diagnostics.HasError() { return } @@ -166,26 +136,7 @@ func (r *grantResource) Update(ctx context.Context, req resource.UpdateRequest, return } - tier, err := tierFromString(plan.Tier.ValueString()) - if err != nil { - resp.Diagnostics.AddError("Invalid tier", err.Error()) - - return - } - - putResp, err := r.client.OrgService.PutGrant(ctx, connect.NewRequest(&orgsvcv1.PutGrantRequest{ - OrgS2R: r.client.OrgS2R, - ResourceS2R: plan.ResourceS2R.ValueString(), - GranteeS2R: plan.GranteeS2R.ValueString(), - Tier: tier, - })) - if err != nil { - resp.Diagnostics.AddError("Updating grant", err.Error()) - - return - } - - resp.Diagnostics.Append(populateGrantModel(&plan, putResp.Msg.GetGrant())...) + resp.Diagnostics.Append(r.upsertGrant(ctx, plan, false)...) if resp.Diagnostics.HasError() { return } @@ -220,6 +171,47 @@ func (r *grantResource) ImportState(ctx context.Context, req resource.ImportStat resp.Diagnostics.Append(resp.State.SetAttribute(ctx, path.Root("grantee_s2r"), granteeS2R)...) } +// upsertGrant writes the grant via PutGrant (an upsert). retryAuthz +// retries PermissionDenied for ~30s on the create path to absorb the +// creator-grant write race. State is set by the caller from the plan: +// resource_s2r, grantee_s2r, and tier are all config-owned, so the +// server echo is intentionally not read back — writing a server- +// canonicalized value into a RequiresReplace attribute would trip the +// framework's post-apply consistency check. +func (r *grantResource) upsertGrant(ctx context.Context, plan grantModel, retryAuthz bool) diag.Diagnostics { + var d diag.Diagnostics + + tier, err := tierFromString(plan.Tier.ValueString()) + if err != nil { + d.AddError("Invalid tier", err.Error()) + + return d + } + + req := &orgsvcv1.PutGrantRequest{ + OrgS2R: r.client.OrgS2R, + ResourceS2R: plan.ResourceS2R.ValueString(), + GranteeS2R: plan.GranteeS2R.ValueString(), + Tier: tier, + } + + if retryAuthz { + _, err = r.putGrantRetryingAuthz(ctx, req) + if err != nil { + d.AddError("Creating grant", err.Error()) + } + + return d + } + + _, err = r.client.OrgService.PutGrant(ctx, connect.NewRequest(req)) + if err != nil { + d.AddError("Updating grant", err.Error()) + } + + return d +} + // putGrantRetryingAuthz is the Create-path PutGrant. It retries // PermissionDenied for ~30s: a grant against a freshly // proposal-created resource can race the server-side creator-grant @@ -248,13 +240,9 @@ func (r *grantResource) putGrantRetryingAuthz(ctx context.Context, req *orgsvcv1 ) } - t := time.NewTimer(waits[attempt]) - select { - case <-ctx.Done(): - t.Stop() - - return nil, ctx.Err() - case <-t.C: + werr := wait.Sleep(ctx, waits[attempt]) + if werr != nil { + return nil, werr } } } diff --git a/splitsecure/services/org/group.go b/splitsecure/services/org/group.go index fd392f4..c3a5324 100644 --- a/splitsecure/services/org/group.go +++ b/splitsecure/services/org/group.go @@ -59,19 +59,7 @@ func (r *groupResource) Metadata(_ context.Context, req resource.MetadataRequest } func (r *groupResource) Configure(_ context.Context, req resource.ConfigureRequest, resp *resource.ConfigureResponse) { - if req.ProviderData == nil { - return - } - c, ok := req.ProviderData.(*client.Client) - if !ok { - resp.Diagnostics.AddError( - "Unexpected provider data type", - fmt.Sprintf("expected *client.Client, got %T", req.ProviderData), - ) - - return - } - r.client = c + r.client = clientFromProviderData(req.ProviderData, &resp.Diagnostics) } func (r *groupResource) Schema(_ context.Context, _ resource.SchemaRequest, resp *resource.SchemaResponse) { @@ -261,10 +249,6 @@ func (r *groupResource) Update(ctx context.Context, req resource.UpdateRequest, return } - plan.GroupS2R = types.StringValue(groupS2R) - if plan.Source.IsUnknown() { - plan.Source = state.Source - } resp.Diagnostics.Append(resp.State.Set(ctx, &plan)...) } @@ -401,7 +385,10 @@ func (r *groupResource) reconcileMembers(ctx context.Context, groupS2R string, s GroupS2R: groupS2R, PrincipalS2R: principal, })) - if err != nil { + // NotFound means the principal is already gone (deleted from the + // org, or removed out of band) — the desired end state is met, so + // treat it as a successful removal rather than failing the apply. + if err != nil && connect.CodeOf(err) != connect.CodeNotFound { return current, "Removing group member", fmt.Errorf("removing %s from group %s: %w", principal, groupS2R, err) } diff --git a/splitsecure/services/org/group_data.go b/splitsecure/services/org/group_data.go index 0f7721d..5e80db8 100644 --- a/splitsecure/services/org/group_data.go +++ b/splitsecure/services/org/group_data.go @@ -46,19 +46,7 @@ func (d *groupDataSource) Metadata(_ context.Context, req datasource.MetadataReq } func (d *groupDataSource) Configure(_ context.Context, req datasource.ConfigureRequest, resp *datasource.ConfigureResponse) { - if req.ProviderData == nil { - return - } - c, ok := req.ProviderData.(*client.Client) - if !ok { - resp.Diagnostics.AddError( - "Unexpected provider data type", - fmt.Sprintf("expected *client.Client, got %T", req.ProviderData), - ) - - return - } - d.client = c + d.client = clientFromProviderData(req.ProviderData, &resp.Diagnostics) } func (d *groupDataSource) Schema(_ context.Context, _ datasource.SchemaRequest, resp *datasource.SchemaResponse) { diff --git a/splitsecure/services/org/group_internal_test.go b/splitsecure/services/org/group_internal_test.go index 493376e..3ebdd21 100644 --- a/splitsecure/services/org/group_internal_test.go +++ b/splitsecure/services/org/group_internal_test.go @@ -2,17 +2,80 @@ package org import ( "context" + "errors" "slices" "strings" "testing" + "connectrpc.com/connect" "github.com/hashicorp/terraform-plugin-framework/resource" rschema "github.com/hashicorp/terraform-plugin-framework/resource/schema" "github.com/hashicorp/terraform-plugin-framework/types" orgsvcv1 "github.com/splitsecure/apis/gen/go/proto/splitsecure/orgsvc/v1" + "github.com/splitsecure/apis/gen/go/proto/splitsecure/orgsvc/v1/orgsvcv1connect" + "github.com/splitsecure/terraform-provider-splitsecure/splitsecure/client" ) +var errStub = errors.New("stub rpc error") + +// fakeOrgClient embeds the OrgService client so only the methods a test +// exercises need overriding; any other call panics on the nil embed. +type fakeOrgClient struct { + orgsvcv1connect.OrgServiceClient + + removeErr error +} + +func (f *fakeOrgClient) RemoveGroupMember( + _ context.Context, _ *connect.Request[orgsvcv1.RemoveGroupMemberRequest], +) (*connect.Response[orgsvcv1.RemoveGroupMemberResponse], error) { + if f.removeErr != nil { + return nil, f.removeErr + } + + return connect.NewResponse(&orgsvcv1.RemoveGroupMemberResponse{}), nil +} + +func reconcileWithRemoveErr(t *testing.T, removeErr error) ([]string, string, error) { + t.Helper() + + r := &groupResource{client: &client.Client{ + OrgService: &fakeOrgClient{removeErr: removeErr}, + OrgS2R: "s2r:test:org:x", + }} + + // State lists one principal, plan lists none -> the member is removed. + return r.reconcileMembers(context.Background(), "s2r:test:group:x/y", []string{"s2r:test:usr:a"}, nil) +} + +func TestReconcileMembers_RemoveToleratesNotFound(t *testing.T) { + t.Parallel() + + current, title, err := reconcileWithRemoveErr(t, connect.NewError(connect.CodeNotFound, errStub)) + if err != nil { + t.Fatalf("NotFound removal should be tolerated, got error %v (%s)", err, title) + } + if len(current) != 0 { + t.Fatalf("principal should be dropped from current, got %v", current) + } +} + +func TestReconcileMembers_RemoveOtherErrorFails(t *testing.T) { + t.Parallel() + + current, title, err := reconcileWithRemoveErr(t, connect.NewError(connect.CodeInternal, errStub)) + if err == nil { + t.Fatal("non-NotFound removal error should fail the reconcile") + } + if title != "Removing group member" { + t.Fatalf("unexpected error title %q", title) + } + if !slices.Equal(current, []string{"s2r:test:usr:a"}) { + t.Fatalf("failed removal should leave the principal in current, got %v", current) + } +} + func groupResourceSchema(t *testing.T) rschema.Schema { t.Helper() diff --git a/splitsecure/services/org/member_data.go b/splitsecure/services/org/member_data.go index 7c078cd..1528f9f 100644 --- a/splitsecure/services/org/member_data.go +++ b/splitsecure/services/org/member_data.go @@ -45,19 +45,7 @@ func (d *memberDataSource) Metadata(_ context.Context, req datasource.MetadataRe } func (d *memberDataSource) Configure(_ context.Context, req datasource.ConfigureRequest, resp *datasource.ConfigureResponse) { - if req.ProviderData == nil { - return - } - c, ok := req.ProviderData.(*client.Client) - if !ok { - resp.Diagnostics.AddError( - "Unexpected provider data type", - fmt.Sprintf("expected *client.Client, got %T", req.ProviderData), - ) - - return - } - d.client = c + d.client = clientFromProviderData(req.ProviderData, &resp.Diagnostics) } func (d *memberDataSource) Schema(_ context.Context, _ datasource.SchemaRequest, resp *datasource.SchemaResponse) { diff --git a/splitsecure/services/org/organization_data.go b/splitsecure/services/org/organization_data.go index 744aaf6..b98bcdd 100644 --- a/splitsecure/services/org/organization_data.go +++ b/splitsecure/services/org/organization_data.go @@ -39,19 +39,7 @@ func (d *organizationDataSource) Metadata(_ context.Context, req datasource.Meta } func (d *organizationDataSource) Configure(_ context.Context, req datasource.ConfigureRequest, resp *datasource.ConfigureResponse) { - if req.ProviderData == nil { - return - } - c, ok := req.ProviderData.(*client.Client) - if !ok { - resp.Diagnostics.AddError( - "Unexpected provider data type", - fmt.Sprintf("expected *client.Client, got %T", req.ProviderData), - ) - - return - } - d.client = c + d.client = clientFromProviderData(req.ProviderData, &resp.Diagnostics) } func (d *organizationDataSource) Schema(_ context.Context, _ datasource.SchemaRequest, resp *datasource.SchemaResponse) { diff --git a/splitsecure/services/saml2/proposal.go b/splitsecure/services/saml2/proposal.go index 881113d..1fa2c36 100644 --- a/splitsecure/services/saml2/proposal.go +++ b/splitsecure/services/saml2/proposal.go @@ -14,6 +14,7 @@ import ( enclaveroundtripv1 "github.com/splitsecure/apis/gen/go/proto/splitsecure/enclaveroundtrip/v1" proposalsv1 "github.com/splitsecure/apis/gen/go/proto/splitsecure/proposals/v1" "github.com/splitsecure/terraform-provider-splitsecure/splitsecure/client" + "github.com/splitsecure/terraform-provider-splitsecure/splitsecure/internal/wait" ) // proposalTimeout is the default wall-clock limit for waiting on a @@ -165,7 +166,7 @@ func awaitProposalID(ctx context.Context, c *client.Client, requestID []byte) ([ return pid, nil } - err = sleep(ctx, pollInterval) + err = wait.Sleep(ctx, pollInterval) if err != nil { return nil, err } @@ -194,7 +195,7 @@ func awaitProposalTerminal(ctx context.Context, c *client.Client, proposalID []b // Keep polling. } - err = sleep(ctx, pollInterval) + err = wait.Sleep(ctx, pollInterval) if err != nil { return err } @@ -220,7 +221,7 @@ func awaitProposalResource(ctx context.Context, c *client.Client, proposalID []b return s, nil } - err = sleep(ctx, pollInterval) + err = wait.Sleep(ctx, pollInterval) if err != nil { return "", err } @@ -242,15 +243,3 @@ func fetchSAML2Record(ctx context.Context, c *client.Client, resourceS2R string) return resp.Msg.GetResources()[resourceS2R], nil } - -// sleep is a context-aware sleep that returns early on cancel/timeout. -func sleep(ctx context.Context, d time.Duration) error { - t := time.NewTimer(d) - defer t.Stop() - select { - case <-ctx.Done(): - return ctx.Err() - case <-t.C: - return nil - } -} From f57a50fd5d8c25f1422ffef003182501deebd614 Mon Sep 17 00:00:00 2001 From: Konyaka1 <47553039+Konyaka1@users.noreply.github.com> Date: Fri, 10 Jul 2026 14:08:36 -0400 Subject: [PATCH 03/15] Revert the internal/wait extraction A dedicated package for one context-aware sleep was more indirection than the small duplication warranted. Restore the inline timer/select in the grant retry loop and saml2's local sleep helper; drop internal/wait. Co-Authored-By: Claude Fable 5 --- splitsecure/internal/wait/wait.go | 21 --------------------- splitsecure/services/org/grant.go | 11 +++++++---- splitsecure/services/saml2/proposal.go | 19 +++++++++++++++---- 3 files changed, 22 insertions(+), 29 deletions(-) delete mode 100644 splitsecure/internal/wait/wait.go diff --git a/splitsecure/internal/wait/wait.go b/splitsecure/internal/wait/wait.go deleted file mode 100644 index b36822d..0000000 --- a/splitsecure/internal/wait/wait.go +++ /dev/null @@ -1,21 +0,0 @@ -// Package wait holds small context-aware timing helpers shared across -// the provider's polling and retry loops. -package wait - -import ( - "context" - "time" -) - -// Sleep blocks for d, returning early with the context's error if it is -// cancelled or times out first. -func Sleep(ctx context.Context, d time.Duration) error { - t := time.NewTimer(d) - defer t.Stop() - select { - case <-ctx.Done(): - return ctx.Err() - case <-t.C: - return nil - } -} diff --git a/splitsecure/services/org/grant.go b/splitsecure/services/org/grant.go index a2b6bd1..2c8b4bc 100644 --- a/splitsecure/services/org/grant.go +++ b/splitsecure/services/org/grant.go @@ -21,7 +21,6 @@ import ( authzv1 "github.com/splitsecure/apis/gen/go/proto/splitsecure/authz/v1" orgsvcv1 "github.com/splitsecure/apis/gen/go/proto/splitsecure/orgsvc/v1" "github.com/splitsecure/terraform-provider-splitsecure/splitsecure/client" - "github.com/splitsecure/terraform-provider-splitsecure/splitsecure/internal/wait" ) var ( @@ -240,9 +239,13 @@ func (r *grantResource) putGrantRetryingAuthz(ctx context.Context, req *orgsvcv1 ) } - werr := wait.Sleep(ctx, waits[attempt]) - if werr != nil { - return nil, werr + t := time.NewTimer(waits[attempt]) + select { + case <-ctx.Done(): + t.Stop() + + return nil, ctx.Err() + case <-t.C: } } } diff --git a/splitsecure/services/saml2/proposal.go b/splitsecure/services/saml2/proposal.go index 1fa2c36..881113d 100644 --- a/splitsecure/services/saml2/proposal.go +++ b/splitsecure/services/saml2/proposal.go @@ -14,7 +14,6 @@ import ( enclaveroundtripv1 "github.com/splitsecure/apis/gen/go/proto/splitsecure/enclaveroundtrip/v1" proposalsv1 "github.com/splitsecure/apis/gen/go/proto/splitsecure/proposals/v1" "github.com/splitsecure/terraform-provider-splitsecure/splitsecure/client" - "github.com/splitsecure/terraform-provider-splitsecure/splitsecure/internal/wait" ) // proposalTimeout is the default wall-clock limit for waiting on a @@ -166,7 +165,7 @@ func awaitProposalID(ctx context.Context, c *client.Client, requestID []byte) ([ return pid, nil } - err = wait.Sleep(ctx, pollInterval) + err = sleep(ctx, pollInterval) if err != nil { return nil, err } @@ -195,7 +194,7 @@ func awaitProposalTerminal(ctx context.Context, c *client.Client, proposalID []b // Keep polling. } - err = wait.Sleep(ctx, pollInterval) + err = sleep(ctx, pollInterval) if err != nil { return err } @@ -221,7 +220,7 @@ func awaitProposalResource(ctx context.Context, c *client.Client, proposalID []b return s, nil } - err = wait.Sleep(ctx, pollInterval) + err = sleep(ctx, pollInterval) if err != nil { return "", err } @@ -243,3 +242,15 @@ func fetchSAML2Record(ctx context.Context, c *client.Client, resourceS2R string) return resp.Msg.GetResources()[resourceS2R], nil } + +// sleep is a context-aware sleep that returns early on cancel/timeout. +func sleep(ctx context.Context, d time.Duration) error { + t := time.NewTimer(d) + defer t.Stop() + select { + case <-ctx.Done(): + return ctx.Err() + case <-t.C: + return nil + } +} From f1157e8aa1d4102d6928db37fab42aa331bc9ddb Mon Sep 17 00:00:00 2001 From: Konyaka1 <47553039+Konyaka1@users.noreply.github.com> Date: Mon, 13 Jul 2026 17:31:08 -0400 Subject: [PATCH 04/15] org: group_s2r-only group data source + review hardening Look up splitsecure_group strictly by group_s2r; drop the mutable, non-unique name path (the ListGroups scan and its ambiguity handling). Address the final code review: - fail closed on empty user_s2r, principal_s2r, and everyone_group_s2r instead of silently emitting an empty s2r into downstream sinks - grant Read refreshes only tier, preserving the config-owned RequiresReplace keys (consistent with Create/Update) - test fetchLocalGroup source rejection, the reconcileMembers add path, and the groupResource.Read null-vs-empty member branch - remove a duplicate source-mapping test; drop putGrantRetryingAuthz's unused return value Co-Authored-By: Claude Opus 4.8 --- docs/data-sources/group.md | 19 +- .../splitsecure_group/data-source.tf | 11 +- .../services/org/data_internal_test.go | 46 -- splitsecure/services/org/grant.go | 20 +- splitsecure/services/org/group.go | 11 +- splitsecure/services/org/group_data.go | 80 ++-- .../services/org/group_internal_test.go | 443 ++++++++++++++++-- splitsecure/services/org/member_data.go | 6 + splitsecure/services/org/organization_data.go | 10 +- 9 files changed, 487 insertions(+), 159 deletions(-) diff --git a/docs/data-sources/group.md b/docs/data-sources/group.md index 2118607..ba1b3dd 100644 --- a/docs/data-sources/group.md +++ b/docs/data-sources/group.md @@ -3,22 +3,23 @@ page_title: "splitsecure_group Data Source - SplitSecure" subcategory: "" description: |- - Looks up a single org group by exact name. Errors if no group or more than one group matches (org group names are not unique server-side). The system Everyone group is not listed; read everyone_group_s2r from the splitsecure_organization data source instead. + Resolves an org group by its stable group_s2r, exposing its current name and source. Lookup is by group_s2r only: a group's name is mutable and not unique server-side, so it is not a stable key. The system Everyone group is not a regular group; read everyone_group_s2r from the splitsecure_organization data source instead. --- # splitsecure_group (Data Source) -Looks up a single org group by exact name. Errors if no group or more than one group matches (org group names are not unique server-side). The system Everyone group is not listed; read everyone_group_s2r from the splitsecure_organization data source instead. +Resolves an org group by its stable group_s2r, exposing its current name and source. Lookup is by group_s2r only: a group's name is mutable and not unique server-side, so it is not a stable key. The system Everyone group is not a regular group; read everyone_group_s2r from the splitsecure_organization data source instead. ## Example Usage ```terraform -# Look up an existing group (e.g. SCIM-synced) by name. Names are not -# unique server-side; the lookup errors on zero or multiple matches. -# The system "Everyone" group is not listed — use the -# splitsecure_organization data source for it. +# Resolve a group by its stable group_s2r, exposing its current name and +# source (e.g. to assert its source before granting on it). A group's +# name is mutable and not unique server-side, so group_s2r is the only +# lookup key. To use a group as a grant grantee you can also reference +# its s2r directly, without this data source. data "splitsecure_group" "sre" { - name = "SRE" + group_s2r = "s2r:us:group:01HX.../01HY..." } ``` @@ -27,9 +28,9 @@ data "splitsecure_group" "sre" { ### Required -- `name` (String) Group name to look up. Matched exactly (case-sensitive). +- `group_s2r` (String) Group s2r URI to resolve. Also usable directly as a grant grantee. ### Read-Only -- `group_s2r` (String) Group s2r URI. Usable as a grant grantee. +- `name` (String) Current group name. Mutable server-side, so do not treat it as an identifier. - `source` (String) Where the group is managed: "local", "scim", or "system". diff --git a/examples/data-sources/splitsecure_group/data-source.tf b/examples/data-sources/splitsecure_group/data-source.tf index be50db9..da6a774 100644 --- a/examples/data-sources/splitsecure_group/data-source.tf +++ b/examples/data-sources/splitsecure_group/data-source.tf @@ -1,7 +1,8 @@ -# Look up an existing group (e.g. SCIM-synced) by name. Names are not -# unique server-side; the lookup errors on zero or multiple matches. -# The system "Everyone" group is not listed — use the -# splitsecure_organization data source for it. +# Resolve a group by its stable group_s2r, exposing its current name and +# source (e.g. to assert its source before granting on it). A group's +# name is mutable and not unique server-side, so group_s2r is the only +# lookup key. To use a group as a grant grantee you can also reference +# its s2r directly, without this data source. data "splitsecure_group" "sre" { - name = "SRE" + group_s2r = "s2r:us:group:01HX.../01HY..." } diff --git a/splitsecure/services/org/data_internal_test.go b/splitsecure/services/org/data_internal_test.go index 3bf9a5b..261ed3a 100644 --- a/splitsecure/services/org/data_internal_test.go +++ b/splitsecure/services/org/data_internal_test.go @@ -61,52 +61,6 @@ func TestMatchMemberByEmail(t *testing.T) { } } -func TestMatchGroupByName(t *testing.T) { - t.Parallel() - - groups := []*orgsvcv1.Group{ - {GroupS2R: "s2r:us:grp:eng", Name: "Engineering"}, - {GroupS2R: "s2r:us:grp:ops1", Name: "Ops"}, - {GroupS2R: "s2r:us:grp:ops2", Name: "Ops"}, - } - - cases := []struct { - name string - groupName string - wantS2R string - wantErrPart string // empty means the lookup must succeed - }{ - {name: "exact match", groupName: "Engineering", wantS2R: "s2r:us:grp:eng"}, - {name: "different case does not match", groupName: "engineering", wantErrPart: `"engineering"`}, - {name: "absent name error points at splitsecure_organization for Everyone", groupName: "Everyone", wantErrPart: "splitsecure_organization"}, - {name: "duplicate name returns ambiguity error listing matches", groupName: "Ops", wantErrPart: "s2r:us:grp:ops2"}, - } - - for _, tc := range cases { - t.Run(tc.name, func(t *testing.T) { - t.Parallel() - - got, err := matchGroupByName(groups, tc.groupName) - if tc.wantErrPart != "" { - if err == nil { - t.Fatalf("matchGroupByName(%q) = %+v, want error", tc.groupName, got) - } - if !strings.Contains(err.Error(), tc.wantErrPart) { - t.Fatalf("error %q does not contain %q", err.Error(), tc.wantErrPart) - } - - return - } - if err != nil { - t.Fatalf("matchGroupByName(%q): %v", tc.groupName, err) - } - if got.GetGroupS2R() != tc.wantS2R { - t.Fatalf("got group %q, want %q", got.GetGroupS2R(), tc.wantS2R) - } - }) - } -} - func TestGroupSourceToString(t *testing.T) { t.Parallel() diff --git a/splitsecure/services/org/grant.go b/splitsecure/services/org/grant.go index 2c8b4bc..613ca0f 100644 --- a/splitsecure/services/org/grant.go +++ b/splitsecure/services/org/grant.go @@ -195,7 +195,7 @@ func (r *grantResource) upsertGrant(ctx context.Context, plan grantModel, retryA } if retryAuthz { - _, err = r.putGrantRetryingAuthz(ctx, req) + err = r.putGrantRetryingAuthz(ctx, req) if err != nil { d.AddError("Creating grant", err.Error()) } @@ -216,7 +216,7 @@ func (r *grantResource) upsertGrant(ctx context.Context, plan grantModel, retryA // proposal-created resource can race the server-side creator-grant // write that authorizes this caller. Every other code fails // immediately. -func (r *grantResource) putGrantRetryingAuthz(ctx context.Context, req *orgsvcv1.PutGrantRequest) (*orgsvcv1.Grant, error) { +func (r *grantResource) putGrantRetryingAuthz(ctx context.Context, req *orgsvcv1.PutGrantRequest) error { waits := []time.Duration{ 1 * time.Second, 2 * time.Second, @@ -225,15 +225,15 @@ func (r *grantResource) putGrantRetryingAuthz(ctx context.Context, req *orgsvcv1 8 * time.Second, } for attempt := 0; ; attempt++ { - putResp, err := r.client.OrgService.PutGrant(ctx, connect.NewRequest(req)) + _, err := r.client.OrgService.PutGrant(ctx, connect.NewRequest(req)) if err == nil { - return putResp.Msg.GetGrant(), nil + return nil } if connect.CodeOf(err) != connect.CodePermissionDenied { - return nil, fmt.Errorf("PutGrant: %w", err) + return fmt.Errorf("PutGrant: %w", err) } if attempt >= len(waits) { - return nil, fmt.Errorf( + return fmt.Errorf( "PutGrant still permission-denied after %d attempts: creating a grant on %s requires the calling principal to hold the %q tier on that resource (an org admin can grant it): %w", attempt+1, req.GetResourceS2R(), "edit", err, ) @@ -244,7 +244,7 @@ func (r *grantResource) putGrantRetryingAuthz(ctx context.Context, req *orgsvcv1 case <-ctx.Done(): t.Stop() - return nil, ctx.Err() + return ctx.Err() case <-t.C: } } @@ -264,8 +264,10 @@ func populateGrantModel(m *grantModel, g *orgsvcv1.Grant) diag.Diagnostics { return d } - m.ResourceS2R = types.StringValue(g.GetResourceS2R()) - m.GranteeS2R = types.StringValue(g.GetGranteeS2R()) + // Refresh only tier. resource_s2r and grantee_s2r are config-owned + // RequiresReplace keys preserved from prior state; echoing the server's + // (possibly canonicalized) value back would trip the framework's post- + // apply consistency check -- same rationale as upsertGrant. m.Tier = types.StringValue(tierStr) return nil diff --git a/splitsecure/services/org/group.go b/splitsecure/services/org/group.go index c3a5324..94e9e58 100644 --- a/splitsecure/services/org/group.go +++ b/splitsecure/services/org/group.go @@ -34,8 +34,9 @@ const ( ) var ( - errEmptyGroup = errors.New("CreateGroup returned no group_s2r; the group may exist server-side but cannot be tracked in state") - errMembersNotAdded = errors.New("some members could not be added") + errEmptyGroup = errors.New("CreateGroup returned no group_s2r; the group may exist server-side but cannot be tracked in state") + errMembersNotAdded = errors.New("some members could not be added") + errEmptyMemberPrincipal = errors.New("ListGroupMembers returned a member with an empty principal_s2r") ) type groupResource struct { @@ -347,7 +348,11 @@ func (r *groupResource) listMemberPrincipals(ctx context.Context, groupS2R strin } principals := make([]string, 0, len(membersResp.Msg.GetMembers())) for _, m := range membersResp.Msg.GetMembers() { - principals = append(principals, m.GetPrincipalS2R()) + p := m.GetPrincipalS2R() + if p == "" { + return nil, fmt.Errorf("%w for group %s", errEmptyMemberPrincipal, groupS2R) + } + principals = append(principals, p) } return principals, nil diff --git a/splitsecure/services/org/group_data.go b/splitsecure/services/org/group_data.go index 5e80db8..26031fe 100644 --- a/splitsecure/services/org/group_data.go +++ b/splitsecure/services/org/group_data.go @@ -4,12 +4,12 @@ import ( "context" "errors" "fmt" - "strings" "connectrpc.com/connect" "github.com/hashicorp/terraform-plugin-framework-validators/stringvalidator" "github.com/hashicorp/terraform-plugin-framework/datasource" "github.com/hashicorp/terraform-plugin-framework/datasource/schema" + "github.com/hashicorp/terraform-plugin-framework/diag" "github.com/hashicorp/terraform-plugin-framework/schema/validator" "github.com/hashicorp/terraform-plugin-framework/types" @@ -21,8 +21,8 @@ var _ datasource.DataSource = (*groupDataSource)(nil) var ( errGroupNotFound = errors.New("group not found") - errAmbiguousGroupName = errors.New("ambiguous group name") errUnknownGroupSource = errors.New("unknown group source") + errEmptyGroupResp = errors.New("server returned an empty group") ) type groupDataSource struct { @@ -30,8 +30,8 @@ type groupDataSource struct { } type groupDataSourceModel struct { - Name types.String `tfsdk:"name"` GroupS2R types.String `tfsdk:"group_s2r"` + Name types.String `tfsdk:"name"` Source types.String `tfsdk:"source"` } @@ -51,20 +51,21 @@ func (d *groupDataSource) Configure(_ context.Context, req datasource.ConfigureR func (d *groupDataSource) Schema(_ context.Context, _ datasource.SchemaRequest, resp *datasource.SchemaResponse) { resp.Schema = schema.Schema{ - Description: "Looks up a single org group by exact name. Errors if no group or more than one group matches " + - "(org group names are not unique server-side). The system Everyone group is not listed; " + - "read everyone_group_s2r from the splitsecure_organization data source instead.", + Description: "Resolves an org group by its stable group_s2r, exposing its current name and source. " + + "Lookup is by group_s2r only: a group's name is mutable and not unique server-side, so it is not a " + + "stable key. The system Everyone group is not a regular group; read everyone_group_s2r from the " + + "splitsecure_organization data source instead.", Attributes: map[string]schema.Attribute{ - "name": schema.StringAttribute{ + "group_s2r": schema.StringAttribute{ Required: true, - Description: "Group name to look up. Matched exactly (case-sensitive).", + Description: "Group s2r URI to resolve. Also usable directly as a grant grantee.", Validators: []validator.String{ stringvalidator.LengthAtLeast(1), }, }, - "group_s2r": schema.StringAttribute{ + "name": schema.StringAttribute{ Computed: true, - Description: "Group s2r URI. Usable as a grant grantee.", + Description: "Current group name. Mutable server-side, so do not treat it as an identifier.", }, "source": schema.StringAttribute{ Computed: true, @@ -81,19 +82,8 @@ func (d *groupDataSource) Read(ctx context.Context, req datasource.ReadRequest, return } - listResp, err := d.client.OrgService.ListGroups(ctx, connect.NewRequest(&orgsvcv1.ListGroupsRequest{ - OrgS2R: d.client.OrgS2R, - })) - if err != nil { - resp.Diagnostics.AddError("ListGroups", err.Error()) - - return - } - - group, err := matchGroupByName(listResp.Msg.GetGroups(), config.Name.ValueString()) - if err != nil { - resp.Diagnostics.AddError("Looking up group", err.Error()) - + group := d.getByS2R(ctx, config.GroupS2R.ValueString(), &resp.Diagnostics) + if resp.Diagnostics.HasError() { return } @@ -104,38 +94,34 @@ func (d *groupDataSource) Read(ctx context.Context, req datasource.ReadRequest, return } - config.GroupS2R = types.StringValue(group.GetGroupS2R()) + config.Name = types.StringValue(group.GetName()) config.Source = types.StringValue(source) resp.Diagnostics.Append(resp.State.Set(ctx, &config)...) } -// matchGroupByName returns the single group whose name equals the given -// name exactly (case-sensitive). Zero or multiple matches are errors. -func matchGroupByName(groups []*orgsvcv1.Group, name string) (*orgsvcv1.Group, error) { - var matches []*orgsvcv1.Group - for _, g := range groups { - if g.GetName() == name { - matches = append(matches, g) - } - } +// getByS2R resolves a group directly by its stable s2r. +func (d *groupDataSource) getByS2R(ctx context.Context, groupS2R string, diags *diag.Diagnostics) *orgsvcv1.Group { + getResp, err := d.client.OrgService.GetGroup(ctx, connect.NewRequest(&orgsvcv1.GetGroupRequest{ + GroupS2R: groupS2R, + })) + if err != nil { + if connect.CodeOf(err) == connect.CodeNotFound { + diags.AddError("Looking up group", fmt.Sprintf("%s: %s", errGroupNotFound, groupS2R)) - switch len(matches) { - case 0: - return nil, fmt.Errorf( - "%w: no group named %q; note the system Everyone group is not returned by group listings -- "+ - "read everyone_group_s2r from the splitsecure_organization data source instead", - errGroupNotFound, name) - case 1: - return matches[0], nil - default: - s2rs := make([]string, len(matches)) - for i, g := range matches { - s2rs[i] = g.GetGroupS2R() + return nil } + diags.AddError("GetGroup", err.Error()) - return nil, fmt.Errorf("%w: %q matches %d groups (org group names are not unique): %s", - errAmbiguousGroupName, name, len(matches), strings.Join(s2rs, ", ")) + return nil } + g := getResp.Msg.GetGroup() + if g.GetGroupS2R() == "" { + diags.AddError("Looking up group", fmt.Sprintf("%s for %s", errEmptyGroupResp, groupS2R)) + + return nil + } + + return g } func groupSourceToString(s orgsvcv1.GroupSource) (string, error) { diff --git a/splitsecure/services/org/group_internal_test.go b/splitsecure/services/org/group_internal_test.go index 3ebdd21..a453f87 100644 --- a/splitsecure/services/org/group_internal_test.go +++ b/splitsecure/services/org/group_internal_test.go @@ -8,8 +8,10 @@ import ( "testing" "connectrpc.com/connect" + "github.com/hashicorp/terraform-plugin-framework/diag" "github.com/hashicorp/terraform-plugin-framework/resource" rschema "github.com/hashicorp/terraform-plugin-framework/resource/schema" + "github.com/hashicorp/terraform-plugin-framework/tfsdk" "github.com/hashicorp/terraform-plugin-framework/types" orgsvcv1 "github.com/splitsecure/apis/gen/go/proto/splitsecure/orgsvc/v1" @@ -24,7 +26,13 @@ var errStub = errors.New("stub rpc error") type fakeOrgClient struct { orgsvcv1connect.OrgServiceClient - removeErr error + removeErr error + getGroup *orgsvcv1.Group + getGroupErr error + addResults []*orgsvcv1.AddGroupMembersResponse_Result + addErr error + listMembers []*orgsvcv1.GroupMember + listErr error } func (f *fakeOrgClient) RemoveGroupMember( @@ -37,6 +45,73 @@ func (f *fakeOrgClient) RemoveGroupMember( return connect.NewResponse(&orgsvcv1.RemoveGroupMemberResponse{}), nil } +func (f *fakeOrgClient) GetGroup( + _ context.Context, _ *connect.Request[orgsvcv1.GetGroupRequest], +) (*connect.Response[orgsvcv1.GetGroupResponse], error) { + if f.getGroupErr != nil { + return nil, f.getGroupErr + } + + return connect.NewResponse(&orgsvcv1.GetGroupResponse{Group: f.getGroup}), nil +} + +func (f *fakeOrgClient) AddGroupMembers( + _ context.Context, _ *connect.Request[orgsvcv1.AddGroupMembersRequest], +) (*connect.Response[orgsvcv1.AddGroupMembersResponse], error) { + if f.addErr != nil { + return nil, f.addErr + } + + return connect.NewResponse(&orgsvcv1.AddGroupMembersResponse{Results: f.addResults}), nil +} + +func (f *fakeOrgClient) ListGroupMembers( + _ context.Context, _ *connect.Request[orgsvcv1.ListGroupMembersRequest], +) (*connect.Response[orgsvcv1.ListGroupMembersResponse], error) { + if f.listErr != nil { + return nil, f.listErr + } + + return connect.NewResponse(&orgsvcv1.ListGroupMembersResponse{Members: f.listMembers}), nil +} + +func TestGroupDataSource_GetByS2R(t *testing.T) { + t.Parallel() + + d := &groupDataSource{client: &client.Client{ + OrgService: &fakeOrgClient{getGroup: &orgsvcv1.Group{ + GroupS2R: "s2r:test:group:x/y", + Name: "SRE", + Source: orgsvcv1.GroupSource_GROUP_SOURCE_LOCAL, + }}, + }} + + var diags diag.Diagnostics + g := d.getByS2R(context.Background(), "s2r:test:group:x/y", &diags) + if diags.HasError() { + t.Fatalf("unexpected diagnostics: %v", diags) + } + if g.GetName() != "SRE" || g.GetGroupS2R() != "s2r:test:group:x/y" { + t.Fatalf("unexpected group: %+v", g) + } +} + +func TestGroupDataSource_GetByS2R_NotFound(t *testing.T) { + t.Parallel() + + d := &groupDataSource{client: &client.Client{ + OrgService: &fakeOrgClient{getGroupErr: connect.NewError(connect.CodeNotFound, errStub)}, + }} + + var diags diag.Diagnostics + if g := d.getByS2R(context.Background(), "s2r:test:group:x/y", &diags); g != nil { + t.Fatalf("expected nil group on NotFound, got %+v", g) + } + if !diags.HasError() { + t.Fatal("expected a diagnostic on NotFound") + } +} + func reconcileWithRemoveErr(t *testing.T, removeErr error) ([]string, string, error) { t.Helper() @@ -151,44 +226,6 @@ func TestGroupSchema_GroupS2RKeepsState(t *testing.T) { } } -func TestGroupSourceMapping(t *testing.T) { - t.Parallel() - - cases := []struct { - name string - source orgsvcv1.GroupSource - want string - wantError bool - }{ - {name: "local", source: orgsvcv1.GroupSource_GROUP_SOURCE_LOCAL, want: "local"}, - {name: "scim", source: orgsvcv1.GroupSource_GROUP_SOURCE_SCIM, want: "scim"}, - {name: "system", source: orgsvcv1.GroupSource_GROUP_SOURCE_SYSTEM, want: "system"}, - {name: "unspecified returns error", source: orgsvcv1.GroupSource_GROUP_SOURCE_UNSPECIFIED, wantError: true}, - {name: "unknown enum value returns error", source: orgsvcv1.GroupSource(42), wantError: true}, - } - - for _, tc := range cases { - t.Run(tc.name, func(t *testing.T) { - t.Parallel() - - got, err := groupSourceToString(tc.source) - if tc.wantError { - if err == nil { - t.Fatalf("groupSourceToString(%v) = %q, want error", tc.source, got) - } - - return - } - if err != nil { - t.Fatalf("groupSourceToString(%v): %v", tc.source, err) - } - if got != tc.want { - t.Fatalf("groupSourceToString(%v) = %q, want %q", tc.source, got, tc.want) - } - }) - } -} - func TestDiffMembers(t *testing.T) { t.Parallel() @@ -313,3 +350,331 @@ func TestPartitionAddResults_EmptyInputYieldsNonNilAdded(t *testing.T) { t.Errorf("added = %v, failures = %v, want both empty", added, failures) } } + +func TestFetchLocalGroup(t *testing.T) { + t.Parallel() + + const groupS2R = "s2r:test:group:x/y" + cases := []struct { + name string + group *orgsvcv1.Group + getErr error + wantGroup bool + wantSource string + wantErr bool + }{ + { + name: "local group resolves", + group: &orgsvcv1.Group{GroupS2R: groupS2R, Name: "SRE", Source: orgsvcv1.GroupSource_GROUP_SOURCE_LOCAL}, + wantGroup: true, + wantSource: "local", + }, + { + name: "scim group is rejected", + group: &orgsvcv1.Group{GroupS2R: groupS2R, Source: orgsvcv1.GroupSource_GROUP_SOURCE_SCIM}, + wantErr: true, + }, + { + name: "system group is rejected", + group: &orgsvcv1.Group{GroupS2R: groupS2R, Source: orgsvcv1.GroupSource_GROUP_SOURCE_SYSTEM}, + wantErr: true, + }, + { + name: "empty group is rejected", + group: &orgsvcv1.Group{}, + wantErr: true, + }, + { + // A nil group with no diagnostic is the contract Read relies on + // to RemoveResource; a surfaced error would abort the read instead. + name: "not found returns nil without a diagnostic", + getErr: connect.NewError(connect.CodeNotFound, errStub), + }, + { + name: "other rpc error is surfaced", + getErr: connect.NewError(connect.CodeInternal, errStub), + wantErr: true, + }, + } + + for _, tc := range cases { + t.Run(tc.name, func(t *testing.T) { + t.Parallel() + + r := &groupResource{client: &client.Client{ + OrgService: &fakeOrgClient{getGroup: tc.group, getGroupErr: tc.getErr}, + }} + + var diags diag.Diagnostics + g, src := r.fetchLocalGroup(context.Background(), groupS2R, &diags) + if tc.wantErr { + if !diags.HasError() { + t.Fatalf("expected an error diagnostic, got group %+v", g) + } + + return + } + if diags.HasError() { + t.Fatalf("unexpected diagnostics: %v", diags) + } + if tc.wantGroup { + if g == nil { + t.Fatal("expected a group") + } + if src != tc.wantSource { + t.Fatalf("source = %q, want %q", src, tc.wantSource) + } + + return + } + if g != nil { + t.Fatalf("expected nil group, got %+v", g) + } + }) + } +} + +func TestListMemberPrincipals_RejectsEmptyPrincipal(t *testing.T) { + t.Parallel() + + r := &groupResource{client: &client.Client{ + OrgService: &fakeOrgClient{listMembers: []*orgsvcv1.GroupMember{ + {PrincipalS2R: "s2r:test:usr:a"}, + {PrincipalS2R: ""}, + }}, + }} + + _, err := r.listMemberPrincipals(context.Background(), "s2r:test:group:x/y") + if !errors.Is(err, errEmptyMemberPrincipal) { + t.Fatalf("err = %v, want errEmptyMemberPrincipal", err) + } +} + +func TestReconcileMembers_AddPath(t *testing.T) { + t.Parallel() + + const groupS2R = "s2r:test:group:x/y" + + t.Run("all additions land", func(t *testing.T) { + t.Parallel() + + r := &groupResource{client: &client.Client{OrgService: &fakeOrgClient{ + addResults: []*orgsvcv1.AddGroupMembersResponse_Result{ + {PrincipalS2R: "s2r:test:usr:a"}, + {PrincipalS2R: "s2r:test:usr:b"}, + }, + }}} + + current, title, err := r.reconcileMembers(context.Background(), groupS2R, nil, []string{"s2r:test:usr:a", "s2r:test:usr:b"}) + if err != nil { + t.Fatalf("unexpected error: %v (%s)", err, title) + } + slices.Sort(current) + if !slices.Equal(current, []string{"s2r:test:usr:a", "s2r:test:usr:b"}) { + t.Fatalf("current = %v, want both principals", current) + } + }) + + t.Run("partial server rejection returns an error and keeps only landed", func(t *testing.T) { + t.Parallel() + + r := &groupResource{client: &client.Client{OrgService: &fakeOrgClient{ + addResults: []*orgsvcv1.AddGroupMembersResponse_Result{ + {PrincipalS2R: "s2r:test:usr:a"}, + {PrincipalS2R: "s2r:test:usr:b", Error: "not in org"}, + }, + }}} + + current, title, err := r.reconcileMembers(context.Background(), groupS2R, nil, []string{"s2r:test:usr:a", "s2r:test:usr:b"}) + if !errors.Is(err, errMembersNotAdded) { + t.Fatalf("err = %v, want errMembersNotAdded", err) + } + if title != titleAddingMembers { + t.Fatalf("title = %q, want %q", title, titleAddingMembers) + } + if !slices.Contains(current, "s2r:test:usr:a") { + t.Fatalf("landed member should be in current: %v", current) + } + if slices.Contains(current, "s2r:test:usr:b") { + t.Fatalf("failed member should not be in current: %v", current) + } + }) + + t.Run("add rpc error preserves prior members", func(t *testing.T) { + t.Parallel() + + r := &groupResource{client: &client.Client{OrgService: &fakeOrgClient{ + addErr: connect.NewError(connect.CodeInternal, errStub), + }}} + + current, title, err := r.reconcileMembers(context.Background(), groupS2R, []string{"s2r:test:usr:x"}, []string{"s2r:test:usr:x", "s2r:test:usr:a"}) + if err == nil { + t.Fatal("expected the add rpc error to fail the reconcile") + } + if title != titleAddingMembers { + t.Fatalf("title = %q, want %q", title, titleAddingMembers) + } + if !slices.Equal(current, []string{"s2r:test:usr:x"}) { + t.Fatalf("prior members should be preserved on add failure, got %v", current) + } + }) +} + +// TestGroupRead_MemberState covers the null-vs-empty member-set branch +// that keeps imported and member-less groups diff-free. +func TestGroupRead_MemberState(t *testing.T) { + t.Parallel() + + s := groupResourceSchema(t) + local := &orgsvcv1.Group{GroupS2R: "s2r:test:group:x/y", Name: "SRE", Source: orgsvcv1.GroupSource_GROUP_SOURCE_LOCAL} + + cases := []struct { + name string + listMembers []*orgsvcv1.GroupMember + priorMembers types.Set + wantNull bool + wantMembers []string + }{ + { + name: "populated membership is refreshed from the server", + listMembers: []*orgsvcv1.GroupMember{{PrincipalS2R: "s2r:test:usr:a"}, {PrincipalS2R: "s2r:test:usr:b"}}, + priorMembers: types.SetNull(types.StringType), + wantMembers: []string{"s2r:test:usr:a", "s2r:test:usr:b"}, + }, + { + name: "empty server membership stays null when prior state was null", + priorMembers: types.SetNull(types.StringType), + wantNull: true, + }, + { + name: "empty server membership becomes an empty set when prior state had members", + priorMembers: mustMemberSet(t, "s2r:test:usr:a"), + wantMembers: []string{}, + }, + } + + for _, tc := range cases { + t.Run(tc.name, func(t *testing.T) { + t.Parallel() + + r := &groupResource{client: &client.Client{ + OrgService: &fakeOrgClient{getGroup: local, listMembers: tc.listMembers}, + }} + req := resource.ReadRequest{State: groupStateForRead(t, s, groupResourceModel{ + GroupS2R: types.StringValue(local.GetGroupS2R()), + Name: types.StringValue("stale name"), + Members: tc.priorMembers, + Source: types.StringValue("local"), + })} + resp := &resource.ReadResponse{State: tfsdk.State{Schema: s}} + + r.Read(context.Background(), req, resp) + if resp.Diagnostics.HasError() { + t.Fatalf("unexpected diagnostics: %v", resp.Diagnostics) + } + + var got groupResourceModel + if diags := resp.State.Get(context.Background(), &got); diags.HasError() { + t.Fatalf("reading result state: %v", diags) + } + if got.Name.ValueString() != "SRE" { + t.Errorf("name = %q, want it refreshed to %q", got.Name.ValueString(), "SRE") + } + if tc.wantNull { + if !got.Members.IsNull() { + t.Fatalf("members should stay null, got %v", got.Members) + } + + return + } + if got.Members.IsNull() { + t.Fatal("members should be a non-null set") + } + var members []string + if diags := got.Members.ElementsAs(context.Background(), &members, false); diags.HasError() { + t.Fatalf("reading members: %v", diags) + } + slices.Sort(members) + if !slices.Equal(members, tc.wantMembers) { + t.Fatalf("members = %v, want %v", members, tc.wantMembers) + } + }) + } +} + +func TestGroupRead_RemovesOnNotFound(t *testing.T) { + t.Parallel() + + s := groupResourceSchema(t) + model := groupResourceModel{ + GroupS2R: types.StringValue("s2r:test:group:x/y"), + Name: types.StringValue("SRE"), + Members: types.SetNull(types.StringType), + Source: types.StringValue("local"), + } + r := &groupResource{client: &client.Client{ + OrgService: &fakeOrgClient{getGroupErr: connect.NewError(connect.CodeNotFound, errStub)}, + }} + req := resource.ReadRequest{State: groupStateForRead(t, s, model)} + // Seed the response with the prior state so a missing RemoveResource + // would leave it non-null and fail the assertion. + resp := &resource.ReadResponse{State: groupStateForRead(t, s, model)} + + r.Read(context.Background(), req, resp) + if resp.Diagnostics.HasError() { + t.Fatalf("NotFound should not surface a diagnostic: %v", resp.Diagnostics) + } + if !resp.State.Raw.IsNull() { + t.Fatal("state should be removed on NotFound") + } +} + +func TestGroupRead_ErrorsWithoutRemovingOnNonLocalSource(t *testing.T) { + t.Parallel() + + s := groupResourceSchema(t) + model := groupResourceModel{ + GroupS2R: types.StringValue("s2r:test:group:x/y"), + Name: types.StringValue("SRE"), + Members: types.SetNull(types.StringType), + Source: types.StringValue("local"), + } + r := &groupResource{client: &client.Client{ + OrgService: &fakeOrgClient{getGroup: &orgsvcv1.Group{ + GroupS2R: "s2r:test:group:x/y", + Source: orgsvcv1.GroupSource_GROUP_SOURCE_SCIM, + }}, + }} + req := resource.ReadRequest{State: groupStateForRead(t, s, model)} + resp := &resource.ReadResponse{State: groupStateForRead(t, s, model)} + + r.Read(context.Background(), req, resp) + if !resp.Diagnostics.HasError() { + t.Fatal("a SCIM-sourced group should fail the read") + } + if resp.State.Raw.IsNull() { + t.Fatal("state must not be removed when the read errors") + } +} + +func mustMemberSet(t *testing.T, principals ...string) types.Set { + t.Helper() + + set, diags := types.SetValueFrom(context.Background(), types.StringType, principals) + if diags.HasError() { + t.Fatalf("building member set: %v", diags) + } + + return set +} + +func groupStateForRead(t *testing.T, s rschema.Schema, model groupResourceModel) tfsdk.State { + t.Helper() + + state := tfsdk.State{Schema: s} + if diags := state.Set(context.Background(), &model); diags.HasError() { + t.Fatalf("building state: %v", diags) + } + + return state +} diff --git a/splitsecure/services/org/member_data.go b/splitsecure/services/org/member_data.go index 1528f9f..9a7b109 100644 --- a/splitsecure/services/org/member_data.go +++ b/splitsecure/services/org/member_data.go @@ -22,6 +22,7 @@ var _ datasource.DataSource = (*memberDataSource)(nil) var ( errNoOrgMember = errors.New("no org member") errAmbiguousMemberEmail = errors.New("ambiguous member email") + errEmptyMemberUserID = errors.New("member resolved to an empty user_s2r") ) type memberDataSource struct { @@ -93,6 +94,11 @@ func (d *memberDataSource) Read(ctx context.Context, req datasource.ReadRequest, return } + if member.GetUserId() == "" { + resp.Diagnostics.AddError("Looking up org member", fmt.Sprintf("%s: %s", errEmptyMemberUserID, config.Email.ValueString())) + + return + } config.UserS2R = types.StringValue(member.GetUserId()) config.DisplayName = types.StringValue(member.GetDisplayName()) diff --git a/splitsecure/services/org/organization_data.go b/splitsecure/services/org/organization_data.go index b98bcdd..6d48072 100644 --- a/splitsecure/services/org/organization_data.go +++ b/splitsecure/services/org/organization_data.go @@ -16,7 +16,10 @@ import ( var _ datasource.DataSource = (*organizationDataSource)(nil) -var errEmptyOrganization = errors.New("GetOrganization returned an empty organization") +var ( + errEmptyOrganization = errors.New("GetOrganization returned an empty organization") + errEmptyEveryoneGroup = errors.New("GetOrganization returned an empty everyone_group_s2r") +) type organizationDataSource struct { client *client.Client @@ -80,6 +83,11 @@ func (d *organizationDataSource) Read(ctx context.Context, _ datasource.ReadRequ return } + if o.GetEveryoneGroupS2R() == "" { + resp.Diagnostics.AddError("Reading organization", fmt.Errorf("%w for %s", errEmptyEveryoneGroup, d.client.OrgS2R).Error()) + + return + } state := organizationDataSourceModel{ ID: types.StringValue(o.GetId()), From e96bf8e30c5d269d28823e67f0323abc7a56a9f5 Mon Sep 17 00:00:00 2001 From: Konyaka1 <47553039+Konyaka1@users.noreply.github.com> Date: Tue, 14 Jul 2026 12:25:32 -0400 Subject: [PATCH 05/15] deps: pin merged apis, drop dev replace apis#14 merged (3a98224); swap the local `replace => ../apis` for the pinned pseudo-version so CI resolves the real public module instead of the sibling checkout. Co-Authored-By: Claude Opus 4.8 --- go.mod | 4 +--- go.sum | 2 ++ 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/go.mod b/go.mod index 9ad63a8..94e1fe4 100644 --- a/go.mod +++ b/go.mod @@ -10,7 +10,7 @@ require ( github.com/hashicorp/terraform-plugin-framework-validators v0.19.0 github.com/hashicorp/terraform-plugin-go v0.31.0 github.com/hashicorp/terraform-plugin-log v0.10.0 - github.com/splitsecure/apis v0.0.0-20260514001223-f52c82fb5959 + github.com/splitsecure/apis v0.0.0-20260714162203-3a98224a8eaa google.golang.org/protobuf v1.36.11 ) @@ -75,5 +75,3 @@ require ( gopkg.in/yaml.v2 v2.3.0 // indirect gopkg.in/yaml.v3 v3.0.1 // indirect ) - -replace github.com/splitsecure/apis => ../apis diff --git a/go.sum b/go.sum index 5b02a07..e42103f 100644 --- a/go.sum +++ b/go.sum @@ -168,6 +168,8 @@ github.com/skeema/knownhosts v1.3.1/go.mod h1:r7KTdC8l4uxWRyK2TpQZ/1o5HaSzh06ePQ github.com/spf13/cast v1.3.1/go.mod h1:Qx5cxh0v+4UWYiBimWS+eyWzqEqokIECu5etghLkUJE= github.com/spf13/cast v1.5.0 h1:rj3WzYc11XZaIZMPKmwP96zkFEnnAmV8s6XbB2aY32w= github.com/spf13/cast v1.5.0/go.mod h1:SpXXQ5YoyJw6s3/6cMTQuxvgRl3PCJiyaX9p6b155UU= +github.com/splitsecure/apis v0.0.0-20260714162203-3a98224a8eaa h1:cO6SepwnYKhe1w2G5tKVHtewU6G72A76kP+9hTqG/H8= +github.com/splitsecure/apis v0.0.0-20260714162203-3a98224a8eaa/go.mod h1:cwf2NxvyDjqjzNkwvQts1gp2i+CuyGXoZCN+d1+Hvuw= github.com/stretchr/objx v0.1.0/go.mod h1:HFkY916IF+rwdDfMAkV7OtwuqBVzrE8GR6GFx+wExME= github.com/stretchr/testify v1.2.2/go.mod h1:a8OnRcib4nhh0OaRAV+Yts87kKdq0PP7pXfy6kDkUVs= github.com/stretchr/testify v1.4.0/go.mod h1:j7eGeouHqKxXV5pUuKE4zz7dFj8WfuZ+81PSLYec5m4= From 6a39807c61076f27fbc1c4892e417f4b28620c1a Mon Sep 17 00:00:00 2001 From: Konyaka1 <47553039+Konyaka1@users.noreply.github.com> Date: Wed, 15 Jul 2026 17:07:34 -0400 Subject: [PATCH 06/15] Add splitsecure_principal data source (email -> s2r for users and SAs) Resolves an org principal to its s2r by the email shown in the console, covering both users (via the member directory) and service accounts (the SA email's local part is its sa: s2r id, validated against GetServiceAccounts). No new backend RPC. The s2r is usable as a group member or grant grantee. examples/full now takes operator_emails and resolves them through the new data source instead of requiring raw s2rs. Co-Authored-By: Claude Opus 4.8 (1M context) --- README.md | 2 +- docs/data-sources/principal.md | 43 +++++ docs/index.md | 13 +- .../splitsecure_principal/data-source.tf | 14 ++ examples/full/main.tf | 13 +- splitsecure/provider/provider.go | 1 + splitsecure/services/org/principal_data.go | 171 ++++++++++++++++++ 7 files changed, 250 insertions(+), 7 deletions(-) create mode 100644 docs/data-sources/principal.md create mode 100644 examples/data-sources/splitsecure_principal/data-source.tf create mode 100644 splitsecure/services/org/principal_data.go diff --git a/README.md b/README.md index 058094a..e832110 100644 --- a/README.md +++ b/README.md @@ -9,7 +9,7 @@ Terraform provider for managing SplitSecure SAML2 resources via service-account - **`splitsecure_grant`** — per-resource permission grant `(resource, grantee) → view/use/edit` so the resources above are visible/usable to org members, not just their creator. Plain RPCs, not proposal-gated. Tiers control visibility and permission management; resource mutations stay proposal/voter-gated regardless of tier. - **`splitsecure_group`** — locally-managed principal group with an authoritative member list, used as a grant target. Group mutations require the provider's service account to hold the org **admin** role; grants alone work with the default member role (the creating service account receives `edit` on resources it creates). -Data sources: **`splitsecure_organization`** (org info incl. the "Everyone" group S2R for org-wide grants), **`splitsecure_org_member`** (user S2R by email), **`splitsecure_group`** (existing group by name, e.g. SCIM-synced). +Data sources: **`splitsecure_organization`** (org info incl. the "Everyone" group S2R for org-wide grants), **`splitsecure_principal`** (principal S2R by console email — resolves both users and service accounts), **`splitsecure_org_member`** (user S2R by email), **`splitsecure_group`** (existing group by its `group_s2r`). Generated reference docs live in [`docs/`](./docs); per-resource attribute tables, validators, and example blocks are kept in sync via `tfplugindocs` (`make docs`). diff --git a/docs/data-sources/principal.md b/docs/data-sources/principal.md new file mode 100644 index 0000000..eeb99db --- /dev/null +++ b/docs/data-sources/principal.md @@ -0,0 +1,43 @@ +--- +# generated by https://github.com/hashicorp/terraform-plugin-docs +page_title: "splitsecure_principal Data Source - SplitSecure" +subcategory: "" +description: |- + Resolves an org principal (user or service account) to its s2r by the email shown in the console. User emails resolve via the member directory; service-account emails parse directly to their sa: s2r. Use the s2r as a group member or grant grantee. +--- + +# splitsecure_principal (Data Source) + +Resolves an org principal (user or service account) to its s2r by the email shown in the console. User emails resolve via the member directory; service-account emails parse directly to their sa: s2r. Use the s2r as a group member or grant grantee. + +## Example Usage + +```terraform +# Resolve an org principal to its s2r by the email shown in the console. +# Works for both users and service accounts: user emails resolve via the +# member directory; a service-account email parses directly to its sa: +# s2r. Use the s2r as a group member or grant grantee. + +# A human user. +data "splitsecure_principal" "alice" { + email = "alice@example.com" +} + +# A service account (email is the one displayed in the console). +data "splitsecure_principal" "ci_bot" { + email = "kQ7...@abc123.serviceaccount.us.splitsecure.com" +} +``` + + +## Schema + +### Required + +- `email` (String) Email of the principal, copy-pasted from the console. Matched case-insensitively. + +### Read-Only + +- `display_name` (String) Display name (user) or name (service account). +- `kind` (String) Principal kind: "user" or "service_account". +- `s2r` (String) Principal s2r URI (usr: for users, sa: for service accounts). Usable as a group member or grant grantee. diff --git a/docs/index.md b/docs/index.md index e6cdece..e794ae7 100644 --- a/docs/index.md +++ b/docs/index.md @@ -210,15 +210,22 @@ output "aws_readonly_role_arn" { data "splitsecure_organization" "current" {} -variable "operator_s2rs" { +variable "operator_emails" { type = list(string) default = [] - description = "User / service-account S2Rs allowed to operate the AWS federation SP." + description = "Emails (as shown in the console) of users / service accounts allowed to operate the AWS federation SP." +} + +# Resolve each console email to its principal s2r (users and service +# accounts alike), so callers paste emails rather than raw s2rs. +data "splitsecure_principal" "operators" { + for_each = toset(var.operator_emails) + email = each.value } resource "splitsecure_group" "operators" { name = "aws-federation-operators-${local.account_id}" - members = var.operator_s2rs + members = [for p in data.splitsecure_principal.operators : p.s2r] } resource "splitsecure_grant" "operators_use" { diff --git a/examples/data-sources/splitsecure_principal/data-source.tf b/examples/data-sources/splitsecure_principal/data-source.tf new file mode 100644 index 0000000..32fca52 --- /dev/null +++ b/examples/data-sources/splitsecure_principal/data-source.tf @@ -0,0 +1,14 @@ +# Resolve an org principal to its s2r by the email shown in the console. +# Works for both users and service accounts: user emails resolve via the +# member directory; a service-account email parses directly to its sa: +# s2r. Use the s2r as a group member or grant grantee. + +# A human user. +data "splitsecure_principal" "alice" { + email = "alice@example.com" +} + +# A service account (email is the one displayed in the console). +data "splitsecure_principal" "ci_bot" { + email = "kQ7...@abc123.serviceaccount.us.splitsecure.com" +} diff --git a/examples/full/main.tf b/examples/full/main.tf index 627cd0b..a65fb72 100644 --- a/examples/full/main.tf +++ b/examples/full/main.tf @@ -166,15 +166,22 @@ output "aws_readonly_role_arn" { data "splitsecure_organization" "current" {} -variable "operator_s2rs" { +variable "operator_emails" { type = list(string) default = [] - description = "User / service-account S2Rs allowed to operate the AWS federation SP." + description = "Emails (as shown in the console) of users / service accounts allowed to operate the AWS federation SP." +} + +# Resolve each console email to its principal s2r (users and service +# accounts alike), so callers paste emails rather than raw s2rs. +data "splitsecure_principal" "operators" { + for_each = toset(var.operator_emails) + email = each.value } resource "splitsecure_group" "operators" { name = "aws-federation-operators-${local.account_id}" - members = var.operator_s2rs + members = [for p in data.splitsecure_principal.operators : p.s2r] } resource "splitsecure_grant" "operators_use" { diff --git a/splitsecure/provider/provider.go b/splitsecure/provider/provider.go index 331fc3d..43bdbc5 100644 --- a/splitsecure/provider/provider.go +++ b/splitsecure/provider/provider.go @@ -119,6 +119,7 @@ func (p *splitsecureProvider) DataSources(_ context.Context) []func() datasource return []func() datasource.DataSource{ org.NewOrganizationDataSource, org.NewMemberDataSource, + org.NewPrincipalDataSource, org.NewGroupDataSource, } } diff --git a/splitsecure/services/org/principal_data.go b/splitsecure/services/org/principal_data.go new file mode 100644 index 0000000..d3f0916 --- /dev/null +++ b/splitsecure/services/org/principal_data.go @@ -0,0 +1,171 @@ +package org + +import ( + "context" + "errors" + "fmt" + "strings" + + "connectrpc.com/connect" + "github.com/hashicorp/terraform-plugin-framework-validators/stringvalidator" + "github.com/hashicorp/terraform-plugin-framework/datasource" + "github.com/hashicorp/terraform-plugin-framework/datasource/schema" + "github.com/hashicorp/terraform-plugin-framework/schema/validator" + "github.com/hashicorp/terraform-plugin-framework/types" + + orgsvcv1 "github.com/splitsecure/apis/gen/go/proto/splitsecure/orgsvc/v1" + "github.com/splitsecure/terraform-provider-splitsecure/splitsecure/client" +) + +var _ datasource.DataSource = (*principalDataSource)(nil) + +var ( + errNoPrincipal = errors.New("no principal") + errBadOrgS2R = errors.New("provider org_s2r is not a valid s2r") + errSAEmailMismatch = errors.New("resolved service account email does not match") +) + +// serviceAccountEmailMarker distinguishes a service-account email +// (@.serviceaccount..splitsecure.com) from a +// human user email. The local part is the SA's sa: s2r id, so an SA email +// resolves to its s2r by parsing — no directory lookup. +const serviceAccountEmailMarker = ".serviceaccount." + +type principalDataSource struct { + client *client.Client +} + +type principalDataSourceModel struct { + Email types.String `tfsdk:"email"` + S2R types.String `tfsdk:"s2r"` + Kind types.String `tfsdk:"kind"` + DisplayName types.String `tfsdk:"display_name"` +} + +// NewPrincipalDataSource returns a factory for the splitsecure_principal +// data source. +func NewPrincipalDataSource() datasource.DataSource { + return &principalDataSource{} +} + +func (d *principalDataSource) Metadata(_ context.Context, req datasource.MetadataRequest, resp *datasource.MetadataResponse) { + resp.TypeName = req.ProviderTypeName + "_principal" +} + +func (d *principalDataSource) Configure(_ context.Context, req datasource.ConfigureRequest, resp *datasource.ConfigureResponse) { + d.client = clientFromProviderData(req.ProviderData, &resp.Diagnostics) +} + +func (d *principalDataSource) Schema(_ context.Context, _ datasource.SchemaRequest, resp *datasource.SchemaResponse) { + resp.Schema = schema.Schema{ + Description: "Resolves an org principal (user or service account) to its s2r by the email shown in the console. " + + "User emails resolve via the member directory; service-account emails parse directly to their sa: s2r. " + + "Use the s2r as a group member or grant grantee.", + Attributes: map[string]schema.Attribute{ + "email": schema.StringAttribute{ + Required: true, + Description: "Email of the principal, copy-pasted from the console. Matched case-insensitively.", + Validators: []validator.String{stringvalidator.LengthAtLeast(1)}, + }, + "s2r": schema.StringAttribute{ + Computed: true, + Description: "Principal s2r URI (usr: for users, sa: for service accounts). Usable as a group member or grant grantee.", + }, + "kind": schema.StringAttribute{ + Computed: true, + Description: `Principal kind: "user" or "service_account".`, + }, + "display_name": schema.StringAttribute{ + Computed: true, + Description: "Display name (user) or name (service account).", + }, + }, + } +} + +func (d *principalDataSource) Read(ctx context.Context, req datasource.ReadRequest, resp *datasource.ReadResponse) { + var config principalDataSourceModel + resp.Diagnostics.Append(req.Config.Get(ctx, &config)...) + if resp.Diagnostics.HasError() { + return + } + email := config.Email.ValueString() + + // A service-account email carries the sa: s2r id in its local part, so + // it resolves without a directory lookup; anything else is a user email. + if _, domain, ok := strings.Cut(email, "@"); ok && strings.Contains(domain, serviceAccountEmailMarker) { + d.resolveServiceAccount(ctx, &config, resp) + return + } + d.resolveUser(ctx, &config, resp) +} + +func (d *principalDataSource) resolveUser(ctx context.Context, config *principalDataSourceModel, resp *datasource.ReadResponse) { + listResp, err := d.client.OrgService.ListMembers(ctx, connect.NewRequest(&orgsvcv1.ListMembersRequest{ + Base: &orgsvcv1.ListMembersRequest_Base{OrganizationId: d.client.OrgS2R}, + })) + if err != nil { + resp.Diagnostics.AddError("ListMembers", err.Error()) + return + } + + member, err := matchMemberByEmail(listResp.Msg.GetMembers(), config.Email.ValueString()) + if err != nil { + resp.Diagnostics.AddError("Looking up principal", err.Error()) + return + } + + config.S2R = types.StringValue(member.GetUserId()) + config.Kind = types.StringValue("user") + config.DisplayName = types.StringValue(member.GetDisplayName()) + resp.Diagnostics.Append(resp.State.Set(ctx, config)...) +} + +func (d *principalDataSource) resolveServiceAccount(ctx context.Context, config *principalDataSourceModel, resp *datasource.ReadResponse) { + deployment, err := deploymentFromS2R(d.client.OrgS2R) + if err != nil { + resp.Diagnostics.AddError("Looking up principal", err.Error()) + return + } + email := config.Email.ValueString() + localPart, _, _ := strings.Cut(email, "@") + saS2R := fmt.Sprintf("s2r:%s:sa:%s", deployment, localPart) + + // Confirm the parsed s2r is a real SA in this org and its email matches + // what was pasted — guards against a typo pointing at another SA. + getResp, err := d.client.OrgService.GetServiceAccounts(ctx, connect.NewRequest(&orgsvcv1.GetServiceAccountsRequest{ + Base: &orgsvcv1.GetServiceAccountsRequest_Base{ + OrganizationId: d.client.OrgS2R, + ServiceAccountIds: []string{saS2R}, + }, + })) + if err != nil { + resp.Diagnostics.AddError("GetServiceAccounts", err.Error()) + return + } + sas := getResp.Msg.GetServiceAccounts() + if len(sas) == 0 { + resp.Diagnostics.AddError("Looking up principal", fmt.Sprintf("%s with email %q", errNoPrincipal, email)) + return + } + sa := sas[0] + if !strings.EqualFold(sa.GetEmail(), email) { + resp.Diagnostics.AddError("Looking up principal", fmt.Sprintf("%s: %q -> %q (%s)", errSAEmailMismatch, email, sa.GetEmail(), sa.GetId())) + return + } + + config.S2R = types.StringValue(sa.GetId()) + config.Kind = types.StringValue("service_account") + config.DisplayName = types.StringValue(sa.GetName()) + resp.Diagnostics.Append(resp.State.Set(ctx, config)...) +} + +// deploymentFromS2R extracts the deployment segment from an s2r URI +// (s2r:{deployment}:{kind}:{id}). +func deploymentFromS2R(s2r string) (string, error) { + parts := strings.SplitN(s2r, ":", 4) + if len(parts) < 4 || parts[0] != "s2r" || parts[1] == "" { + return "", fmt.Errorf("%w: %q", errBadOrgS2R, s2r) + } + return parts[1], nil +} From 51394079b401198888cc956ef259058ed911b760 Mon Sep 17 00:00:00 2001 From: Konyaka1 <47553039+Konyaka1@users.noreply.github.com> Date: Wed, 15 Jul 2026 17:11:22 -0400 Subject: [PATCH 07/15] Satisfy nlreturn in splitsecure_principal data source Blank line before returns; fixes the CI lint failure. Co-Authored-By: Claude Opus 4.8 (1M context) --- splitsecure/services/org/principal_data.go | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/splitsecure/services/org/principal_data.go b/splitsecure/services/org/principal_data.go index d3f0916..70e8470 100644 --- a/splitsecure/services/org/principal_data.go +++ b/splitsecure/services/org/principal_data.go @@ -95,6 +95,7 @@ func (d *principalDataSource) Read(ctx context.Context, req datasource.ReadReque // it resolves without a directory lookup; anything else is a user email. if _, domain, ok := strings.Cut(email, "@"); ok && strings.Contains(domain, serviceAccountEmailMarker) { d.resolveServiceAccount(ctx, &config, resp) + return } d.resolveUser(ctx, &config, resp) @@ -106,12 +107,14 @@ func (d *principalDataSource) resolveUser(ctx context.Context, config *principal })) if err != nil { resp.Diagnostics.AddError("ListMembers", err.Error()) + return } member, err := matchMemberByEmail(listResp.Msg.GetMembers(), config.Email.ValueString()) if err != nil { resp.Diagnostics.AddError("Looking up principal", err.Error()) + return } @@ -125,6 +128,7 @@ func (d *principalDataSource) resolveServiceAccount(ctx context.Context, config deployment, err := deploymentFromS2R(d.client.OrgS2R) if err != nil { resp.Diagnostics.AddError("Looking up principal", err.Error()) + return } email := config.Email.ValueString() @@ -141,16 +145,19 @@ func (d *principalDataSource) resolveServiceAccount(ctx context.Context, config })) if err != nil { resp.Diagnostics.AddError("GetServiceAccounts", err.Error()) + return } sas := getResp.Msg.GetServiceAccounts() if len(sas) == 0 { resp.Diagnostics.AddError("Looking up principal", fmt.Sprintf("%s with email %q", errNoPrincipal, email)) + return } sa := sas[0] if !strings.EqualFold(sa.GetEmail(), email) { resp.Diagnostics.AddError("Looking up principal", fmt.Sprintf("%s: %q -> %q (%s)", errSAEmailMismatch, email, sa.GetEmail(), sa.GetId())) + return } @@ -167,5 +174,6 @@ func deploymentFromS2R(s2r string) (string, error) { if len(parts) < 4 || parts[0] != "s2r" || parts[1] == "" { return "", fmt.Errorf("%w: %q", errBadOrgS2R, s2r) } + return parts[1], nil } From f0b67a3450022fffa6e2168c2629938a8f134a0f Mon Sep 17 00:00:00 2001 From: Konyaka1 <47553039+Konyaka1@users.noreply.github.com> Date: Wed, 15 Jul 2026 17:19:46 -0400 Subject: [PATCH 08/15] examples: move full-example vars to variables.tf; principal by email in group example Addresses review feedback: - examples/full: variable declarations (org_s2r, team_s2r, operator_emails) moved out of main.tf into variables.tf; terraform.tfvars.example gains an operator_emails value; index doc embeds variables.tf so it stays complete. - examples/resources/splitsecure_group: reference members by console email via splitsecure_principal (user and service account) instead of raw s2rs. - Regenerated docs. Co-Authored-By: Claude Opus 4.8 (1M context) --- docs/index.md | 36 ++++++++++--------- docs/resources/group.md | 8 +++-- examples/full/main.tf | 16 --------- examples/full/terraform.tfvars.example | 8 ++++- examples/full/variables.tf | 15 ++++++++ .../resources/splitsecure_group/resource.tf | 8 +++-- templates/index.md.tmpl | 4 ++- 7 files changed, 56 insertions(+), 39 deletions(-) create mode 100644 examples/full/variables.tf diff --git a/docs/index.md b/docs/index.md index e794ae7..4a6492f 100644 --- a/docs/index.md +++ b/docs/index.md @@ -39,7 +39,25 @@ provider "splitsecure" { ## Full Example -End-to-end wiring: a SplitSecure team with a SAML2 IdP, mirrored as `aws_iam_saml_provider` on the AWS side, with admin and readonly IAM roles and a single SP allowing federation into both. Lives at [`examples/full/main.tf`](https://github.com/splitsecure/terraform-provider-splitsecure/tree/main/examples/full) in the repo. +End-to-end wiring: a SplitSecure team with a SAML2 IdP, mirrored as `aws_iam_saml_provider` on the AWS side, with admin and readonly IAM roles and a single SP allowing federation into both. Lives at [`examples/full`](https://github.com/splitsecure/terraform-provider-splitsecure/tree/main/examples/full) in the repo. + +```terraform +variable "org_s2r" { + type = string + description = "Org s2r URI hosting the team below. Used by the provider to spawn the proposal-scoped managed enclave on every Create / Delete." +} + +variable "team_s2r" { + type = string + description = "Team s2r URI that owns the IdP and SP. Voters on this team approve every Create / Delete proposal." +} + +variable "operator_emails" { + type = list(string) + default = [] + description = "Emails (as shown in the console) of users / service accounts allowed to operate the AWS federation SP." +} +``` ```terraform terraform { @@ -60,16 +78,6 @@ provider "splitsecure" { provider "aws" {} -variable "org_s2r" { - type = string - description = "Org s2r URI hosting the team below. Used by the provider to spawn the proposal-scoped managed enclave on every Create / Delete." -} - -variable "team_s2r" { - type = string - description = "Team s2r URI that owns the IdP and SP. Voters on this team approve every Create / Delete proposal." -} - data "aws_caller_identity" "current" {} locals { @@ -210,12 +218,6 @@ output "aws_readonly_role_arn" { data "splitsecure_organization" "current" {} -variable "operator_emails" { - type = list(string) - default = [] - description = "Emails (as shown in the console) of users / service accounts allowed to operate the AWS federation SP." -} - # Resolve each console email to its principal s2r (users and service # accounts alike), so callers paste emails rather than raw s2rs. data "splitsecure_principal" "operators" { diff --git a/docs/resources/group.md b/docs/resources/group.md index b203930..961061d 100644 --- a/docs/resources/group.md +++ b/docs/resources/group.md @@ -21,11 +21,15 @@ Locally-managed principal group in the provider-configured organization. Terrafo # # Group mutations require the service account behind the provider to # hold the org admin role. +# +# Members are referenced by the email shown in the console; the +# splitsecure_principal data source resolves each to its s2r (users and +# service accounts alike). resource "splitsecure_group" "sre" { name = "SRE" members = [ - data.splitsecure_org_member.alice.user_s2r, - "s2r:us:sa:2qX9mK4pLw8vN3rT", + data.splitsecure_principal.alice.s2r, # a user + data.splitsecure_principal.ci_bot.s2r, # a service account ] } ``` diff --git a/examples/full/main.tf b/examples/full/main.tf index a65fb72..7178bc5 100644 --- a/examples/full/main.tf +++ b/examples/full/main.tf @@ -16,16 +16,6 @@ provider "splitsecure" { provider "aws" {} -variable "org_s2r" { - type = string - description = "Org s2r URI hosting the team below. Used by the provider to spawn the proposal-scoped managed enclave on every Create / Delete." -} - -variable "team_s2r" { - type = string - description = "Team s2r URI that owns the IdP and SP. Voters on this team approve every Create / Delete proposal." -} - data "aws_caller_identity" "current" {} locals { @@ -166,12 +156,6 @@ output "aws_readonly_role_arn" { data "splitsecure_organization" "current" {} -variable "operator_emails" { - type = list(string) - default = [] - description = "Emails (as shown in the console) of users / service accounts allowed to operate the AWS federation SP." -} - # Resolve each console email to its principal s2r (users and service # accounts alike), so callers paste emails rather than raw s2rs. data "splitsecure_principal" "operators" { diff --git a/examples/full/terraform.tfvars.example b/examples/full/terraform.tfvars.example index 39d1dee..37b0cfa 100644 --- a/examples/full/terraform.tfvars.example +++ b/examples/full/terraform.tfvars.example @@ -1,4 +1,10 @@ -# Copy to terraform.tfvars (gitignored) and fill in the s2r URIs. +# Copy to terraform.tfvars (gitignored) and fill in the values. org_s2r = "s2r:us:org:..." team_s2r = "s2r:us:team:..." + +# Console emails of the users / service accounts that may operate the SP. +operator_emails = [ + "alice@example.com", + "ci-bot@abc123.serviceaccount.us.splitsecure.com", +] diff --git a/examples/full/variables.tf b/examples/full/variables.tf new file mode 100644 index 0000000..80e1bf4 --- /dev/null +++ b/examples/full/variables.tf @@ -0,0 +1,15 @@ +variable "org_s2r" { + type = string + description = "Org s2r URI hosting the team below. Used by the provider to spawn the proposal-scoped managed enclave on every Create / Delete." +} + +variable "team_s2r" { + type = string + description = "Team s2r URI that owns the IdP and SP. Voters on this team approve every Create / Delete proposal." +} + +variable "operator_emails" { + type = list(string) + default = [] + description = "Emails (as shown in the console) of users / service accounts allowed to operate the AWS federation SP." +} diff --git a/examples/resources/splitsecure_group/resource.tf b/examples/resources/splitsecure_group/resource.tf index a3a2f3b..ad25a0c 100644 --- a/examples/resources/splitsecure_group/resource.tf +++ b/examples/resources/splitsecure_group/resource.tf @@ -6,10 +6,14 @@ # # Group mutations require the service account behind the provider to # hold the org admin role. +# +# Members are referenced by the email shown in the console; the +# splitsecure_principal data source resolves each to its s2r (users and +# service accounts alike). resource "splitsecure_group" "sre" { name = "SRE" members = [ - data.splitsecure_org_member.alice.user_s2r, - "s2r:us:sa:2qX9mK4pLw8vN3rT", + data.splitsecure_principal.alice.s2r, # a user + data.splitsecure_principal.ci_bot.s2r, # a service account ] } diff --git a/templates/index.md.tmpl b/templates/index.md.tmpl index 598f615..431dc4c 100644 --- a/templates/index.md.tmpl +++ b/templates/index.md.tmpl @@ -19,6 +19,8 @@ description: |- ## Full Example -End-to-end wiring: a SplitSecure team with a SAML2 IdP, mirrored as `aws_iam_saml_provider` on the AWS side, with admin and readonly IAM roles and a single SP allowing federation into both. Lives at [`examples/full/main.tf`](https://github.com/splitsecure/terraform-provider-splitsecure/tree/main/examples/full) in the repo. +End-to-end wiring: a SplitSecure team with a SAML2 IdP, mirrored as `aws_iam_saml_provider` on the AWS side, with admin and readonly IAM roles and a single SP allowing federation into both. Lives at [`examples/full`](https://github.com/splitsecure/terraform-provider-splitsecure/tree/main/examples/full) in the repo. + +{{tffile "examples/full/variables.tf"}} {{tffile "examples/full/main.tf"}} From 750d1c3412e2a33e4b0f0f6dd88183edc82beaab Mon Sep 17 00:00:00 2001 From: Konyaka1 <47553039+Konyaka1@users.noreply.github.com> Date: Thu, 16 Jul 2026 19:42:20 -0400 Subject: [PATCH 09/15] org: resolve splitsecure_principal via GetMembersByEmail Swap the principal data source from a ListMembers roster scan plus client-side match (and client-side service-account email parsing) to a single server-side GetMembersByEmail call, which resolves users and service accounts alike. Bump the apis dep to the commit that adds the RPC. Co-Authored-By: Claude Opus 4.8 (1M context) --- go.mod | 2 +- go.sum | 4 +- splitsecure/services/org/principal_data.go | 111 ++++++------------ .../org/principal_data_internal_test.go | 42 +++++++ 4 files changed, 82 insertions(+), 77 deletions(-) create mode 100644 splitsecure/services/org/principal_data_internal_test.go diff --git a/go.mod b/go.mod index 94e1fe4..b2f940d 100644 --- a/go.mod +++ b/go.mod @@ -10,7 +10,7 @@ require ( github.com/hashicorp/terraform-plugin-framework-validators v0.19.0 github.com/hashicorp/terraform-plugin-go v0.31.0 github.com/hashicorp/terraform-plugin-log v0.10.0 - github.com/splitsecure/apis v0.0.0-20260714162203-3a98224a8eaa + github.com/splitsecure/apis v0.0.0-20260716233351-872044c52916 google.golang.org/protobuf v1.36.11 ) diff --git a/go.sum b/go.sum index e42103f..449e310 100644 --- a/go.sum +++ b/go.sum @@ -168,8 +168,8 @@ github.com/skeema/knownhosts v1.3.1/go.mod h1:r7KTdC8l4uxWRyK2TpQZ/1o5HaSzh06ePQ github.com/spf13/cast v1.3.1/go.mod h1:Qx5cxh0v+4UWYiBimWS+eyWzqEqokIECu5etghLkUJE= github.com/spf13/cast v1.5.0 h1:rj3WzYc11XZaIZMPKmwP96zkFEnnAmV8s6XbB2aY32w= github.com/spf13/cast v1.5.0/go.mod h1:SpXXQ5YoyJw6s3/6cMTQuxvgRl3PCJiyaX9p6b155UU= -github.com/splitsecure/apis v0.0.0-20260714162203-3a98224a8eaa h1:cO6SepwnYKhe1w2G5tKVHtewU6G72A76kP+9hTqG/H8= -github.com/splitsecure/apis v0.0.0-20260714162203-3a98224a8eaa/go.mod h1:cwf2NxvyDjqjzNkwvQts1gp2i+CuyGXoZCN+d1+Hvuw= +github.com/splitsecure/apis v0.0.0-20260716233351-872044c52916 h1:DEsVrfRnjLr3LLsJzwR80Uh8FTecA8KeID7fJ/jwrjk= +github.com/splitsecure/apis v0.0.0-20260716233351-872044c52916/go.mod h1:cwf2NxvyDjqjzNkwvQts1gp2i+CuyGXoZCN+d1+Hvuw= github.com/stretchr/objx v0.1.0/go.mod h1:HFkY916IF+rwdDfMAkV7OtwuqBVzrE8GR6GFx+wExME= github.com/stretchr/testify v1.2.2/go.mod h1:a8OnRcib4nhh0OaRAV+Yts87kKdq0PP7pXfy6kDkUVs= github.com/stretchr/testify v1.4.0/go.mod h1:j7eGeouHqKxXV5pUuKE4zz7dFj8WfuZ+81PSLYec5m4= diff --git a/splitsecure/services/org/principal_data.go b/splitsecure/services/org/principal_data.go index 70e8470..2eef8ed 100644 --- a/splitsecure/services/org/principal_data.go +++ b/splitsecure/services/org/principal_data.go @@ -20,17 +20,11 @@ import ( var _ datasource.DataSource = (*principalDataSource)(nil) var ( - errNoPrincipal = errors.New("no principal") - errBadOrgS2R = errors.New("provider org_s2r is not a valid s2r") - errSAEmailMismatch = errors.New("resolved service account email does not match") + errNoPrincipal = errors.New("no principal") + errAmbiguousPrincipal = errors.New("email resolves to multiple principals") + errBadPrincipalS2R = errors.New("resolved principal s2r is malformed") ) -// serviceAccountEmailMarker distinguishes a service-account email -// (@.serviceaccount..splitsecure.com) from a -// human user email. The local part is the SA's sa: s2r id, so an SA email -// resolves to its s2r by parsing — no directory lookup. -const serviceAccountEmailMarker = ".serviceaccount." - type principalDataSource struct { client *client.Client } @@ -59,7 +53,6 @@ func (d *principalDataSource) Configure(_ context.Context, req datasource.Config func (d *principalDataSource) Schema(_ context.Context, _ datasource.SchemaRequest, resp *datasource.SchemaResponse) { resp.Schema = schema.Schema{ Description: "Resolves an org principal (user or service account) to its s2r by the email shown in the console. " + - "User emails resolve via the member directory; service-account emails parse directly to their sa: s2r. " + "Use the s2r as a group member or grant grantee.", Attributes: map[string]schema.Attribute{ "email": schema.StringAttribute{ @@ -91,89 +84,59 @@ func (d *principalDataSource) Read(ctx context.Context, req datasource.ReadReque } email := config.Email.ValueString() - // A service-account email carries the sa: s2r id in its local part, so - // it resolves without a directory lookup; anything else is a user email. - if _, domain, ok := strings.Cut(email, "@"); ok && strings.Contains(domain, serviceAccountEmailMarker) { - d.resolveServiceAccount(ctx, &config, resp) - - return - } - d.resolveUser(ctx, &config, resp) -} - -func (d *principalDataSource) resolveUser(ctx context.Context, config *principalDataSourceModel, resp *datasource.ReadResponse) { - listResp, err := d.client.OrgService.ListMembers(ctx, connect.NewRequest(&orgsvcv1.ListMembersRequest{ - Base: &orgsvcv1.ListMembersRequest_Base{OrganizationId: d.client.OrgS2R}, + // GetMembersByEmail resolves users and service accounts alike, server-side — + // no member roster scan and no client-side service-account parsing. + membersResp, err := d.client.OrgService.GetMembersByEmail(ctx, connect.NewRequest(&orgsvcv1.GetMembersByEmailRequest{ + Base: &orgsvcv1.GetMembersByEmailRequest_Base{ + OrganizationId: d.client.OrgS2R, + Emails: []string{email}, + }, })) if err != nil { - resp.Diagnostics.AddError("ListMembers", err.Error()) - - return - } - - member, err := matchMemberByEmail(listResp.Msg.GetMembers(), config.Email.ValueString()) - if err != nil { - resp.Diagnostics.AddError("Looking up principal", err.Error()) + resp.Diagnostics.AddError("GetMembersByEmail", err.Error()) return } - config.S2R = types.StringValue(member.GetUserId()) - config.Kind = types.StringValue("user") - config.DisplayName = types.StringValue(member.GetDisplayName()) - resp.Diagnostics.Append(resp.State.Set(ctx, config)...) -} - -func (d *principalDataSource) resolveServiceAccount(ctx context.Context, config *principalDataSourceModel, resp *datasource.ReadResponse) { - deployment, err := deploymentFromS2R(d.client.OrgS2R) - if err != nil { - resp.Diagnostics.AddError("Looking up principal", err.Error()) + members := membersResp.Msg.GetMembers() + switch { + case len(members) == 0: + resp.Diagnostics.AddError("Looking up principal", fmt.Sprintf("%s with email %q", errNoPrincipal, email)) return - } - email := config.Email.ValueString() - localPart, _, _ := strings.Cut(email, "@") - saS2R := fmt.Sprintf("s2r:%s:sa:%s", deployment, localPart) - - // Confirm the parsed s2r is a real SA in this org and its email matches - // what was pasted — guards against a typo pointing at another SA. - getResp, err := d.client.OrgService.GetServiceAccounts(ctx, connect.NewRequest(&orgsvcv1.GetServiceAccountsRequest{ - Base: &orgsvcv1.GetServiceAccountsRequest_Base{ - OrganizationId: d.client.OrgS2R, - ServiceAccountIds: []string{saS2R}, - }, - })) - if err != nil { - resp.Diagnostics.AddError("GetServiceAccounts", err.Error()) + case len(members) > 1: + resp.Diagnostics.AddError("Looking up principal", fmt.Sprintf("%s: %q -> %d principals", errAmbiguousPrincipal, email, len(members))) return } - sas := getResp.Msg.GetServiceAccounts() - if len(sas) == 0 { - resp.Diagnostics.AddError("Looking up principal", fmt.Sprintf("%s with email %q", errNoPrincipal, email)) - return - } - sa := sas[0] - if !strings.EqualFold(sa.GetEmail(), email) { - resp.Diagnostics.AddError("Looking up principal", fmt.Sprintf("%s: %q -> %q (%s)", errSAEmailMismatch, email, sa.GetEmail(), sa.GetId())) + member := members[0] + kind, err := principalKindFromS2R(member.GetUserId()) + if err != nil { + resp.Diagnostics.AddError("Looking up principal", err.Error()) return } - config.S2R = types.StringValue(sa.GetId()) - config.Kind = types.StringValue("service_account") - config.DisplayName = types.StringValue(sa.GetName()) + config.S2R = types.StringValue(member.GetUserId()) + config.Kind = types.StringValue(kind) + config.DisplayName = types.StringValue(member.GetDisplayName()) resp.Diagnostics.Append(resp.State.Set(ctx, config)...) } -// deploymentFromS2R extracts the deployment segment from an s2r URI -// (s2r:{deployment}:{kind}:{id}). -func deploymentFromS2R(s2r string) (string, error) { +// principalKindFromS2R maps the kind segment of a principal s2r +// (s2r:{deployment}:{kind}:{id}) to the data source's kind value. +func principalKindFromS2R(s2r string) (string, error) { parts := strings.SplitN(s2r, ":", 4) - if len(parts) < 4 || parts[0] != "s2r" || parts[1] == "" { - return "", fmt.Errorf("%w: %q", errBadOrgS2R, s2r) + if len(parts) < 4 || parts[0] != "s2r" { + return "", fmt.Errorf("%w: %q", errBadPrincipalS2R, s2r) + } + switch parts[2] { + case "usr": + return "user", nil + case "sa": + return "service_account", nil + default: + return "", fmt.Errorf("%w: %q (unexpected kind %q)", errBadPrincipalS2R, s2r, parts[2]) } - - return parts[1], nil } diff --git a/splitsecure/services/org/principal_data_internal_test.go b/splitsecure/services/org/principal_data_internal_test.go new file mode 100644 index 0000000..54338c3 --- /dev/null +++ b/splitsecure/services/org/principal_data_internal_test.go @@ -0,0 +1,42 @@ +package org + +import "testing" + +func TestPrincipalKindFromS2R(t *testing.T) { + t.Parallel() + + cases := []struct { + name string + s2r string + want string + wantError bool + }{ + {name: "user", s2r: "s2r:local-aliaksei:usr:abc123", want: "user"}, + {name: "service account", s2r: "s2r:local-aliaksei:sa:abc123", want: "service_account"}, + {name: "unexpected kind", s2r: "s2r:us:group:abc123", wantError: true}, + {name: "too few segments", s2r: "s2r:us:usr", wantError: true}, + {name: "not an s2r", s2r: "usr:abc123", wantError: true}, + {name: "empty", s2r: "", wantError: true}, + } + + for _, tc := range cases { + t.Run(tc.name, func(t *testing.T) { + t.Parallel() + + got, err := principalKindFromS2R(tc.s2r) + if tc.wantError { + if err == nil { + t.Fatalf("principalKindFromS2R(%q) = %q, want error", tc.s2r, got) + } + + return + } + if err != nil { + t.Fatalf("principalKindFromS2R(%q): %v", tc.s2r, err) + } + if got != tc.want { + t.Fatalf("got %q, want %q", got, tc.want) + } + }) + } +} From 57392f5eb17489daf78d95afb07a360c0260a725 Mon Sep 17 00:00:00 2001 From: Konyaka1 <47553039+Konyaka1@users.noreply.github.com> Date: Fri, 17 Jul 2026 11:43:10 -0400 Subject: [PATCH 10/15] org: read GetMembersByEmail's per-email Result shape The RPC now returns one Result per requested email (email + its matches) instead of a flat member list; splitsecure_principal reads results[0] for its single-email lookup. Bump the apis dep to the merged commit. Co-Authored-By: Claude Opus 4.8 (1M context) --- go.mod | 2 +- go.sum | 4 ++-- splitsecure/services/org/principal_data.go | 8 +++++++- 3 files changed, 10 insertions(+), 4 deletions(-) diff --git a/go.mod b/go.mod index b2f940d..c8f5b50 100644 --- a/go.mod +++ b/go.mod @@ -10,7 +10,7 @@ require ( github.com/hashicorp/terraform-plugin-framework-validators v0.19.0 github.com/hashicorp/terraform-plugin-go v0.31.0 github.com/hashicorp/terraform-plugin-log v0.10.0 - github.com/splitsecure/apis v0.0.0-20260716233351-872044c52916 + github.com/splitsecure/apis v0.0.0-20260717151105-8a4c7579b056 google.golang.org/protobuf v1.36.11 ) diff --git a/go.sum b/go.sum index 449e310..ad2976c 100644 --- a/go.sum +++ b/go.sum @@ -168,8 +168,8 @@ github.com/skeema/knownhosts v1.3.1/go.mod h1:r7KTdC8l4uxWRyK2TpQZ/1o5HaSzh06ePQ github.com/spf13/cast v1.3.1/go.mod h1:Qx5cxh0v+4UWYiBimWS+eyWzqEqokIECu5etghLkUJE= github.com/spf13/cast v1.5.0 h1:rj3WzYc11XZaIZMPKmwP96zkFEnnAmV8s6XbB2aY32w= github.com/spf13/cast v1.5.0/go.mod h1:SpXXQ5YoyJw6s3/6cMTQuxvgRl3PCJiyaX9p6b155UU= -github.com/splitsecure/apis v0.0.0-20260716233351-872044c52916 h1:DEsVrfRnjLr3LLsJzwR80Uh8FTecA8KeID7fJ/jwrjk= -github.com/splitsecure/apis v0.0.0-20260716233351-872044c52916/go.mod h1:cwf2NxvyDjqjzNkwvQts1gp2i+CuyGXoZCN+d1+Hvuw= +github.com/splitsecure/apis v0.0.0-20260717151105-8a4c7579b056 h1:SxkwJ0gJkSRT4fmEcfVmkL67bURlFecobes23X3lkms= +github.com/splitsecure/apis v0.0.0-20260717151105-8a4c7579b056/go.mod h1:cwf2NxvyDjqjzNkwvQts1gp2i+CuyGXoZCN+d1+Hvuw= github.com/stretchr/objx v0.1.0/go.mod h1:HFkY916IF+rwdDfMAkV7OtwuqBVzrE8GR6GFx+wExME= github.com/stretchr/testify v1.2.2/go.mod h1:a8OnRcib4nhh0OaRAV+Yts87kKdq0PP7pXfy6kDkUVs= github.com/stretchr/testify v1.4.0/go.mod h1:j7eGeouHqKxXV5pUuKE4zz7dFj8WfuZ+81PSLYec5m4= diff --git a/splitsecure/services/org/principal_data.go b/splitsecure/services/org/principal_data.go index 2eef8ed..d5d82b6 100644 --- a/splitsecure/services/org/principal_data.go +++ b/splitsecure/services/org/principal_data.go @@ -98,7 +98,13 @@ func (d *principalDataSource) Read(ctx context.Context, req datasource.ReadReque return } - members := membersResp.Msg.GetMembers() + // One email in, one result out (results are per requested email); its members + // are the matches for that email. + results := membersResp.Msg.GetResults() + var members []*orgsvcv1.Member + if len(results) > 0 { + members = results[0].GetMembers() + } switch { case len(members) == 0: resp.Diagnostics.AddError("Looking up principal", fmt.Sprintf("%s with email %q", errNoPrincipal, email)) From 747f90ea790ca1bd1ef77f6eaa7bc26bdbfcb2c2 Mon Sep 17 00:00:00 2001 From: Konyaka1 <47553039+Konyaka1@users.noreply.github.com> Date: Fri, 17 Jul 2026 11:58:16 -0400 Subject: [PATCH 11/15] org: address review on splitsecure_principal - principalKindFromS2R rejects empty deployment / id segments. - Regenerate docs + example to the simplified data-source description (drop the stale "member directory / parse directly" wording). Co-Authored-By: Claude Opus 4.8 (1M context) --- docs/data-sources/principal.md | 10 ++++------ .../data-sources/splitsecure_principal/data-source.tf | 6 ++---- splitsecure/services/org/principal_data.go | 2 +- .../services/org/principal_data_internal_test.go | 2 ++ 4 files changed, 9 insertions(+), 11 deletions(-) diff --git a/docs/data-sources/principal.md b/docs/data-sources/principal.md index eeb99db..b3e0dd1 100644 --- a/docs/data-sources/principal.md +++ b/docs/data-sources/principal.md @@ -3,20 +3,18 @@ page_title: "splitsecure_principal Data Source - SplitSecure" subcategory: "" description: |- - Resolves an org principal (user or service account) to its s2r by the email shown in the console. User emails resolve via the member directory; service-account emails parse directly to their sa: s2r. Use the s2r as a group member or grant grantee. + Resolves an org principal (user or service account) to its s2r by the email shown in the console. Use the s2r as a group member or grant grantee. --- # splitsecure_principal (Data Source) -Resolves an org principal (user or service account) to its s2r by the email shown in the console. User emails resolve via the member directory; service-account emails parse directly to their sa: s2r. Use the s2r as a group member or grant grantee. +Resolves an org principal (user or service account) to its s2r by the email shown in the console. Use the s2r as a group member or grant grantee. ## Example Usage ```terraform -# Resolve an org principal to its s2r by the email shown in the console. -# Works for both users and service accounts: user emails resolve via the -# member directory; a service-account email parses directly to its sa: -# s2r. Use the s2r as a group member or grant grantee. +# Resolve an org principal (user or service account) to its s2r by the email +# shown in the console. Use the s2r as a group member or grant grantee. # A human user. data "splitsecure_principal" "alice" { diff --git a/examples/data-sources/splitsecure_principal/data-source.tf b/examples/data-sources/splitsecure_principal/data-source.tf index 32fca52..62b317e 100644 --- a/examples/data-sources/splitsecure_principal/data-source.tf +++ b/examples/data-sources/splitsecure_principal/data-source.tf @@ -1,7 +1,5 @@ -# Resolve an org principal to its s2r by the email shown in the console. -# Works for both users and service accounts: user emails resolve via the -# member directory; a service-account email parses directly to its sa: -# s2r. Use the s2r as a group member or grant grantee. +# Resolve an org principal (user or service account) to its s2r by the email +# shown in the console. Use the s2r as a group member or grant grantee. # A human user. data "splitsecure_principal" "alice" { diff --git a/splitsecure/services/org/principal_data.go b/splitsecure/services/org/principal_data.go index d5d82b6..dcb8406 100644 --- a/splitsecure/services/org/principal_data.go +++ b/splitsecure/services/org/principal_data.go @@ -134,7 +134,7 @@ func (d *principalDataSource) Read(ctx context.Context, req datasource.ReadReque // (s2r:{deployment}:{kind}:{id}) to the data source's kind value. func principalKindFromS2R(s2r string) (string, error) { parts := strings.SplitN(s2r, ":", 4) - if len(parts) < 4 || parts[0] != "s2r" { + if len(parts) < 4 || parts[0] != "s2r" || parts[1] == "" || parts[3] == "" { return "", fmt.Errorf("%w: %q", errBadPrincipalS2R, s2r) } switch parts[2] { diff --git a/splitsecure/services/org/principal_data_internal_test.go b/splitsecure/services/org/principal_data_internal_test.go index 54338c3..2c419da 100644 --- a/splitsecure/services/org/principal_data_internal_test.go +++ b/splitsecure/services/org/principal_data_internal_test.go @@ -15,6 +15,8 @@ func TestPrincipalKindFromS2R(t *testing.T) { {name: "service account", s2r: "s2r:local-aliaksei:sa:abc123", want: "service_account"}, {name: "unexpected kind", s2r: "s2r:us:group:abc123", wantError: true}, {name: "too few segments", s2r: "s2r:us:usr", wantError: true}, + {name: "empty deployment", s2r: "s2r::usr:abc123", wantError: true}, + {name: "empty id", s2r: "s2r:us:usr:", wantError: true}, {name: "not an s2r", s2r: "usr:abc123", wantError: true}, {name: "empty", s2r: "", wantError: true}, } From a9e849090d7677b84d806c6422d5805d52ff1873 Mon Sep 17 00:00:00 2001 From: Konyaka1 <47553039+Konyaka1@users.noreply.github.com> Date: Fri, 17 Jul 2026 12:16:15 -0400 Subject: [PATCH 12/15] org: address grants review (retries, pagination, schema tests) - client: give OrgService a non-retrying HTTP client so a lost response never replays a non-idempotent mutation (CreateGroup / UpdateGroup / DeleteGroup / PutGrant / DeleteGrant); keep retries for the enclave flow. - group: listMemberPrincipals now consumes every ListGroupMembers page via next_cursor (was truncated at the first page) + a two-page test. - grant schema tests: execute the tier validators (accept real tiers, reject others) and check the key attributes carry requires-replace semantics. Co-Authored-By: Claude Opus 4.8 (1M context) --- splitsecure/client/client.go | 23 +++++++--- .../services/org/grant_internal_test.go | 46 +++++++++++++++++-- splitsecure/services/org/group.go | 34 ++++++++------ .../services/org/group_internal_test.go | 36 ++++++++++++++- 4 files changed, 114 insertions(+), 25 deletions(-) diff --git a/splitsecure/client/client.go b/splitsecure/client/client.go index 5d3201a..c07c232 100644 --- a/splitsecure/client/client.go +++ b/splitsecure/client/client.go @@ -34,23 +34,34 @@ type Client struct { // individual RPC calls see unary polls but the wrapper keeps the // connection pool happy across retries. func New(endpoint, bearerToken, orgS2R, version string) *Client { + userAgent := "terraform-provider-splitsecure/" + version + withAuth := func(wrapped http.RoundTripper) http.RoundTripper { + return &loggingTransport{token: bearerToken, userAgent: userAgent, wrapped: wrapped} + } + + // Retrying client for the long-running enclave / proposal flow. retryClient := retryablehttp.NewClient() retryClient.RetryMax = 3 retryClient.Logger = nil - httpClient := retryClient.StandardClient() httpClient.Timeout = 1 * time.Minute - httpClient.Transport = &loggingTransport{ - token: bearerToken, - userAgent: "terraform-provider-splitsecure/" + version, - wrapped: httpClient.Transport, + httpClient.Transport = withAuth(httpClient.Transport) + + // OrgService carries non-idempotent mutations (CreateGroup / UpdateGroup / + // DeleteGroup / PutGrant / DeleteGrant). Every Connect RPC is a POST, so + // retryablehttp can't scope retries by method and would replay a committed + // write if the response is lost. Give it a non-retrying client; resource + // create paths that need it do their own bounded, condition-scoped retries. + orgHTTPClient := &http.Client{ + Timeout: 1 * time.Minute, + Transport: withAuth(http.DefaultTransport), } return &Client{ ConvenienceStoreService: conveniencestorev1connect.NewConvenienceStoreServiceClient(httpClient, endpoint), EnclaveRoundtripService: enclaveroundtripv1connect.NewEnclaveRoundtripServiceClient(httpClient, endpoint), ProposalsService: proposalsv1connect.NewProposalsServiceClient(httpClient, endpoint), - OrgService: orgsvcv1connect.NewOrgServiceClient(httpClient, endpoint), + OrgService: orgsvcv1connect.NewOrgServiceClient(orgHTTPClient, endpoint), OrgS2R: orgS2R, } } diff --git a/splitsecure/services/org/grant_internal_test.go b/splitsecure/services/org/grant_internal_test.go index 3043eb9..e863531 100644 --- a/splitsecure/services/org/grant_internal_test.go +++ b/splitsecure/services/org/grant_internal_test.go @@ -2,10 +2,15 @@ package org import ( "context" + "strings" "testing" + "github.com/hashicorp/terraform-plugin-framework/diag" + "github.com/hashicorp/terraform-plugin-framework/path" "github.com/hashicorp/terraform-plugin-framework/resource" rschema "github.com/hashicorp/terraform-plugin-framework/resource/schema" + "github.com/hashicorp/terraform-plugin-framework/schema/validator" + "github.com/hashicorp/terraform-plugin-framework/types" authzv1 "github.com/splitsecure/apis/gen/go/proto/splitsecure/authz/v1" ) @@ -48,13 +53,25 @@ func TestGrantSchema_ReplaceSemantics(t *testing.T) { s := grantTestSchema(t) + ctx := context.Background() for _, name := range []string{"resource_s2r", "grantee_s2r"} { attr, ok := s.Attributes[name].(rschema.StringAttribute) if !ok { t.Fatalf("attribute %q has wrong type: %T", name, s.Attributes[name]) } if len(attr.PlanModifiers) == 0 { - t.Errorf("attribute %q should have a RequiresReplace plan modifier", name) + t.Fatalf("attribute %q has no plan modifiers", name) + } + // Not just any modifier: at least one must carry requires-replace + // semantics (RequiresReplace's description says the resource is recreated). + replaces := false + for _, mod := range attr.PlanModifiers { + if strings.Contains(strings.ToLower(mod.Description(ctx)), "recreate") { + replaces = true + } + } + if !replaces { + t.Errorf("attribute %q should force replacement on change", name) } } @@ -67,17 +84,38 @@ func TestGrantSchema_ReplaceSemantics(t *testing.T) { } } -func TestGrantSchema_TierHasOneOfValidator(t *testing.T) { +// TestGrantSchema_TierValidatorAcceptsTiers executes the tier validators: every +// real tier passes and an unknown value is rejected. +func TestGrantSchema_TierValidatorAcceptsTiers(t *testing.T) { t.Parallel() s := grantTestSchema(t) + ctx := context.Background() tier, ok := s.Attributes["tier"].(rschema.StringAttribute) if !ok { t.Fatalf("tier attribute has wrong type: %T", s.Attributes["tier"]) } - if len(tier.Validators) == 0 { - t.Fatal("tier should have at least one validator (OneOf)") + + validate := func(value string) diag.Diagnostics { + var resp validator.StringResponse + for _, v := range tier.Validators { + v.ValidateString(ctx, validator.StringRequest{ + Path: path.Root("tier"), + ConfigValue: types.StringValue(value), + }, &resp) + } + + return resp.Diagnostics + } + + for _, v := range tierValues() { + if d := validate(v); d.HasError() { + t.Errorf("tier %q should be accepted, got: %v", v, d) + } + } + if d := validate("bogus-tier"); !d.HasError() { + t.Error(`tier "bogus-tier" should be rejected`) } } diff --git a/splitsecure/services/org/group.go b/splitsecure/services/org/group.go index 94e9e58..2e38e96 100644 --- a/splitsecure/services/org/group.go +++ b/splitsecure/services/org/group.go @@ -340,22 +340,28 @@ func (r *groupResource) fetchLocalGroup(ctx context.Context, groupS2R string, di } func (r *groupResource) listMemberPrincipals(ctx context.Context, groupS2R string) ([]string, error) { - membersResp, err := r.client.OrgService.ListGroupMembers(ctx, connect.NewRequest(&orgsvcv1.ListGroupMembersRequest{ - GroupS2R: groupS2R, - })) - if err != nil { - return nil, err - } - principals := make([]string, 0, len(membersResp.Msg.GetMembers())) - for _, m := range membersResp.Msg.GetMembers() { - p := m.GetPrincipalS2R() - if p == "" { - return nil, fmt.Errorf("%w for group %s", errEmptyMemberPrincipal, groupS2R) + principals := []string{} // non-nil: an empty group is an empty set, not null + cursor := "" + for { + membersResp, err := r.client.OrgService.ListGroupMembers(ctx, connect.NewRequest(&orgsvcv1.ListGroupMembersRequest{ + GroupS2R: groupS2R, + Cursor: cursor, + })) + if err != nil { + return nil, err + } + for _, m := range membersResp.Msg.GetMembers() { + p := m.GetPrincipalS2R() + if p == "" { + return nil, fmt.Errorf("%w for group %s", errEmptyMemberPrincipal, groupS2R) + } + principals = append(principals, p) + } + cursor = membersResp.Msg.GetNextCursor() + if cursor == "" { + return principals, nil } - principals = append(principals, p) } - - return principals, nil } // reconcileMembers applies the membership diff from state to plan: diff --git a/splitsecure/services/org/group_internal_test.go b/splitsecure/services/org/group_internal_test.go index a453f87..89ea287 100644 --- a/splitsecure/services/org/group_internal_test.go +++ b/splitsecure/services/org/group_internal_test.go @@ -4,6 +4,7 @@ import ( "context" "errors" "slices" + "strconv" "strings" "testing" @@ -32,6 +33,7 @@ type fakeOrgClient struct { addResults []*orgsvcv1.AddGroupMembersResponse_Result addErr error listMembers []*orgsvcv1.GroupMember + listPages [][]*orgsvcv1.GroupMember // when set, ListGroupMembers serves these pages via cursor listErr error } @@ -66,11 +68,23 @@ func (f *fakeOrgClient) AddGroupMembers( } func (f *fakeOrgClient) ListGroupMembers( - _ context.Context, _ *connect.Request[orgsvcv1.ListGroupMembersRequest], + _ context.Context, req *connect.Request[orgsvcv1.ListGroupMembersRequest], ) (*connect.Response[orgsvcv1.ListGroupMembersResponse], error) { if f.listErr != nil { return nil, f.listErr } + if f.listPages != nil { // paginated mode: the cursor is the page index + i := 0 + if c := req.Msg.GetCursor(); c != "" { + i, _ = strconv.Atoi(c) + } + resp := &orgsvcv1.ListGroupMembersResponse{Members: f.listPages[i]} + if i+1 < len(f.listPages) { + resp.NextCursor = strconv.Itoa(i + 1) + } + + return connect.NewResponse(resp), nil + } return connect.NewResponse(&orgsvcv1.ListGroupMembersResponse{Members: f.listMembers}), nil } @@ -450,6 +464,26 @@ func TestListMemberPrincipals_RejectsEmptyPrincipal(t *testing.T) { } } +func TestListMemberPrincipals_ConsumesAllPages(t *testing.T) { + t.Parallel() + + r := &groupResource{client: &client.Client{OrgService: &fakeOrgClient{ + listPages: [][]*orgsvcv1.GroupMember{ + {{PrincipalS2R: "s2r:test:usr:a"}, {PrincipalS2R: "s2r:test:usr:b"}}, + {{PrincipalS2R: "s2r:test:usr:c"}}, + }, + }}} + + got, err := r.listMemberPrincipals(context.Background(), "s2r:test:group:x/y") + if err != nil { + t.Fatalf("listMemberPrincipals: %v", err) + } + want := []string{"s2r:test:usr:a", "s2r:test:usr:b", "s2r:test:usr:c"} + if !slices.Equal(got, want) { + t.Fatalf("got %v, want %v (both pages must be consumed)", got, want) + } +} + func TestReconcileMembers_AddPath(t *testing.T) { t.Parallel() From 5428b229867c170de6d157b055f9e357a362c5bc Mon Sep 17 00:00:00 2001 From: Konyaka1 <47553039+Konyaka1@users.noreply.github.com> Date: Fri, 17 Jul 2026 16:10:21 -0400 Subject: [PATCH 13/15] deps: bump apis to merged main (teamresource lineage_id #18) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Re-pin github.com/splitsecure/apis from the 8a4c757 pseudo-version to the merged apis-main tip 1efd2ff, so the provider sits on the same apis-main commit as priv. No source changes — GetMembersByEmail is unchanged. Co-Authored-By: Claude Opus 4.8 (1M context) --- go.mod | 2 +- go.sum | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/go.mod b/go.mod index c8f5b50..20360d0 100644 --- a/go.mod +++ b/go.mod @@ -10,7 +10,7 @@ require ( github.com/hashicorp/terraform-plugin-framework-validators v0.19.0 github.com/hashicorp/terraform-plugin-go v0.31.0 github.com/hashicorp/terraform-plugin-log v0.10.0 - github.com/splitsecure/apis v0.0.0-20260717151105-8a4c7579b056 + github.com/splitsecure/apis v0.0.0-20260717200245-1efd2ff90e84 google.golang.org/protobuf v1.36.11 ) diff --git a/go.sum b/go.sum index ad2976c..fd97524 100644 --- a/go.sum +++ b/go.sum @@ -168,8 +168,8 @@ github.com/skeema/knownhosts v1.3.1/go.mod h1:r7KTdC8l4uxWRyK2TpQZ/1o5HaSzh06ePQ github.com/spf13/cast v1.3.1/go.mod h1:Qx5cxh0v+4UWYiBimWS+eyWzqEqokIECu5etghLkUJE= github.com/spf13/cast v1.5.0 h1:rj3WzYc11XZaIZMPKmwP96zkFEnnAmV8s6XbB2aY32w= github.com/spf13/cast v1.5.0/go.mod h1:SpXXQ5YoyJw6s3/6cMTQuxvgRl3PCJiyaX9p6b155UU= -github.com/splitsecure/apis v0.0.0-20260717151105-8a4c7579b056 h1:SxkwJ0gJkSRT4fmEcfVmkL67bURlFecobes23X3lkms= -github.com/splitsecure/apis v0.0.0-20260717151105-8a4c7579b056/go.mod h1:cwf2NxvyDjqjzNkwvQts1gp2i+CuyGXoZCN+d1+Hvuw= +github.com/splitsecure/apis v0.0.0-20260717200245-1efd2ff90e84 h1:q0XPABgIhkvC8UjVI7eSH5+vZ5WLOKocknPkeh2yC+o= +github.com/splitsecure/apis v0.0.0-20260717200245-1efd2ff90e84/go.mod h1:cwf2NxvyDjqjzNkwvQts1gp2i+CuyGXoZCN+d1+Hvuw= github.com/stretchr/objx v0.1.0/go.mod h1:HFkY916IF+rwdDfMAkV7OtwuqBVzrE8GR6GFx+wExME= github.com/stretchr/testify v1.2.2/go.mod h1:a8OnRcib4nhh0OaRAV+Yts87kKdq0PP7pXfy6kDkUVs= github.com/stretchr/testify v1.4.0/go.mod h1:j7eGeouHqKxXV5pUuKE4zz7dFj8WfuZ+81PSLYec5m4= From 7958fe82fc0190dc690895f9c60d054f6afe22ec Mon Sep 17 00:00:00 2001 From: Konyaka1 <47553039+Konyaka1@users.noreply.github.com> Date: Fri, 17 Jul 2026 18:20:14 -0400 Subject: [PATCH 14/15] org_member data source: resolve via GetMembersByEmail RPC Replace the full-directory ListMembers pull + client-side EqualFold scan with the server-side GetMembersByEmail lookup built for this. Besides the efficiency win (one indexed keyed lookup vs. streaming the whole member directory on every plan), this fixes three correctness gaps: - pagination: the old path read only the first ListMembers page, so a member past page one resolved as "no org member"; - canonicalization: matching now happens server-side (+tag/IDNA/etc.) instead of a naive case-insensitive compare; - service accounts: SA emails now resolve (saemail), which an EqualFold scan over Members could never do. matchMemberByEmail -> singleMember: the client now only extracts the one member from the per-email Result (0 = not found, >1 = ambiguous); the matching itself is the server's job. Test migrated to the narrowed contract. Co-Authored-By: Claude Opus 4.8 (1M context) --- .../services/org/data_internal_test.go | 38 ++++++++--------- splitsecure/services/org/member_data.go | 42 +++++++++++-------- 2 files changed, 43 insertions(+), 37 deletions(-) diff --git a/splitsecure/services/org/data_internal_test.go b/splitsecure/services/org/data_internal_test.go index 261ed3a..9bfff4e 100644 --- a/splitsecure/services/org/data_internal_test.go +++ b/splitsecure/services/org/data_internal_test.go @@ -7,43 +7,41 @@ import ( orgsvcv1 "github.com/splitsecure/apis/gen/go/proto/splitsecure/orgsvc/v1" ) -func TestMatchMemberByEmail(t *testing.T) { +func TestSingleMember(t *testing.T) { t.Parallel() - members := []*orgsvcv1.Member{ - {UserId: "s2r:us:usr:alice", Email: "alice@example.com", DisplayName: "Alice"}, - {UserId: "s2r:us:usr:bob", Email: "Bob@Example.COM", DisplayName: "Bob"}, - {UserId: "s2r:us:usr:carol1", Email: "carol@example.com", DisplayName: "Carol One"}, - {UserId: "s2r:us:usr:carol2", Email: "CAROL@example.com", DisplayName: "Carol Two"}, + // The server resolves emails to members (canonicalization, ambiguity); the + // client only extracts the single member from the per-email Result. Matching + // is by the request email string the server echoes back. + result := func(email string, members ...*orgsvcv1.Member) *orgsvcv1.GetMembersByEmailResponse_Result { + return &orgsvcv1.GetMembersByEmailResponse_Result{Email: email, Members: members} } + alice := &orgsvcv1.Member{UserId: "s2r:us:usr:alice", Email: "alice@example.com", DisplayName: "Alice"} + carol1 := &orgsvcv1.Member{UserId: "s2r:us:usr:carol1", Email: "carol@example.com", DisplayName: "Carol One"} + carol2 := &orgsvcv1.Member{UserId: "s2r:us:usr:carol2", Email: "carol@example.com", DisplayName: "Carol Two"} cases := []struct { name string email string + results []*orgsvcv1.GetMembersByEmailResponse_Result wantUserID string wantErrPart string // empty means the lookup must succeed }{ - {name: "exact match", email: "alice@example.com", wantUserID: "s2r:us:usr:alice"}, - {name: "case-insensitive match", email: "bob@example.com", wantUserID: "s2r:us:usr:bob"}, - {name: "mixed-case query matches stored lowercase", email: "ALICE@EXAMPLE.COM", wantUserID: "s2r:us:usr:alice"}, - {name: "ambiguous email returns error listing matches", email: "carol@example.com", wantErrPart: "s2r:us:usr:carol2"}, - {name: "absent email returns error naming the email", email: "dave@example.com", wantErrPart: "dave@example.com"}, - {name: "empty member list returns error", email: "alice@example.com", wantErrPart: "alice@example.com"}, + {name: "single member resolves", email: "alice@example.com", results: []*orgsvcv1.GetMembersByEmailResponse_Result{result("alice@example.com", alice)}, wantUserID: "s2r:us:usr:alice"}, + {name: "ambiguous email returns error listing matches", email: "carol@example.com", results: []*orgsvcv1.GetMembersByEmailResponse_Result{result("carol@example.com", carol1, carol2)}, wantErrPart: "s2r:us:usr:carol2"}, + {name: "empty members returns error naming the email", email: "dave@example.com", results: []*orgsvcv1.GetMembersByEmailResponse_Result{result("dave@example.com")}, wantErrPart: "dave@example.com"}, + {name: "no matching result returns error naming the email", email: "erin@example.com", results: []*orgsvcv1.GetMembersByEmailResponse_Result{result("alice@example.com", alice)}, wantErrPart: "erin@example.com"}, + {name: "empty results returns error", email: "alice@example.com", results: nil, wantErrPart: "alice@example.com"}, } for _, tc := range cases { t.Run(tc.name, func(t *testing.T) { t.Parallel() - in := members - if tc.name == "empty member list returns error" { - in = nil - } - - got, err := matchMemberByEmail(in, tc.email) + got, err := singleMember(tc.results, tc.email) if tc.wantErrPart != "" { if err == nil { - t.Fatalf("matchMemberByEmail(%q) = %+v, want error", tc.email, got) + t.Fatalf("singleMember(%q) = %+v, want error", tc.email, got) } if !strings.Contains(err.Error(), tc.wantErrPart) { t.Fatalf("error %q does not contain %q", err.Error(), tc.wantErrPart) @@ -52,7 +50,7 @@ func TestMatchMemberByEmail(t *testing.T) { return } if err != nil { - t.Fatalf("matchMemberByEmail(%q): %v", tc.email, err) + t.Fatalf("singleMember(%q): %v", tc.email, err) } if got.GetUserId() != tc.wantUserID { t.Fatalf("got user %q, want %q", got.GetUserId(), tc.wantUserID) diff --git a/splitsecure/services/org/member_data.go b/splitsecure/services/org/member_data.go index 9a7b109..a000a9b 100644 --- a/splitsecure/services/org/member_data.go +++ b/splitsecure/services/org/member_data.go @@ -79,23 +79,27 @@ func (d *memberDataSource) Read(ctx context.Context, req datasource.ReadRequest, return } - listResp, err := d.client.OrgService.ListMembers(ctx, connect.NewRequest(&orgsvcv1.ListMembersRequest{ - Base: &orgsvcv1.ListMembersRequest_Base{OrganizationId: d.client.OrgS2R}, + email := config.Email.ValueString() + membersResp, err := d.client.OrgService.GetMembersByEmail(ctx, connect.NewRequest(&orgsvcv1.GetMembersByEmailRequest{ + Base: &orgsvcv1.GetMembersByEmailRequest_Base{ + OrganizationId: d.client.OrgS2R, + Emails: []string{email}, + }, })) if err != nil { - resp.Diagnostics.AddError("ListMembers", err.Error()) + resp.Diagnostics.AddError("GetMembersByEmail", err.Error()) return } - member, err := matchMemberByEmail(listResp.Msg.GetMembers(), config.Email.ValueString()) + member, err := singleMember(membersResp.Msg.GetResults(), email) if err != nil { resp.Diagnostics.AddError("Looking up org member", err.Error()) return } if member.GetUserId() == "" { - resp.Diagnostics.AddError("Looking up org member", fmt.Sprintf("%s: %s", errEmptyMemberUserID, config.Email.ValueString())) + resp.Diagnostics.AddError("Looking up org member", fmt.Sprintf("%s: %s", errEmptyMemberUserID, email)) return } @@ -105,27 +109,31 @@ func (d *memberDataSource) Read(ctx context.Context, req datasource.ReadRequest, resp.Diagnostics.Append(resp.State.Set(ctx, &config)...) } -// matchMemberByEmail returns the single member whose email equals the -// given email case-insensitively. Zero or multiple matches are errors. -func matchMemberByEmail(members []*orgsvcv1.Member, email string) (*orgsvcv1.Member, error) { - var matches []*orgsvcv1.Member - for _, m := range members { - if strings.EqualFold(m.GetEmail(), email) { - matches = append(matches, m) +// singleMember extracts the one member the server resolved for email. The RPC +// returns one Result per requested email (in request order) with the matching +// already done server-side; here zero members means no such member and multiple +// means the address is ambiguous — both errors. +func singleMember(results []*orgsvcv1.GetMembersByEmailResponse_Result, email string) (*orgsvcv1.Member, error) { + var members []*orgsvcv1.Member + for _, r := range results { + if r.GetEmail() == email { + members = r.GetMembers() + + break } } - switch len(matches) { + switch len(members) { case 0: return nil, fmt.Errorf("%w with email %q", errNoOrgMember, email) case 1: - return matches[0], nil + return members[0], nil default: - ids := make([]string, len(matches)) - for i, m := range matches { + ids := make([]string, len(members)) + for i, m := range members { ids[i] = m.GetUserId() } - return nil, fmt.Errorf("%w: %q matches %d members: %s", errAmbiguousMemberEmail, email, len(matches), strings.Join(ids, ", ")) + return nil, fmt.Errorf("%w: %q matches %d members: %s", errAmbiguousMemberEmail, email, len(members), strings.Join(ids, ", ")) } } From 58c40a66aff02b9f925447e46bfe5a94e88a8507 Mon Sep 17 00:00:00 2001 From: Konyaka1 <47553039+Konyaka1@users.noreply.github.com> Date: Fri, 17 Jul 2026 18:25:00 -0400 Subject: [PATCH 15/15] test: wrap long TestSingleMember table lines to satisfy lll Return the per-email Result slice from a oneResult helper so the table-driven cases fit under the 200-char line limit. Co-Authored-By: Claude Opus 4.8 (1M context) --- splitsecure/services/org/data_internal_test.go | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/splitsecure/services/org/data_internal_test.go b/splitsecure/services/org/data_internal_test.go index 9bfff4e..bab5e66 100644 --- a/splitsecure/services/org/data_internal_test.go +++ b/splitsecure/services/org/data_internal_test.go @@ -13,8 +13,8 @@ func TestSingleMember(t *testing.T) { // The server resolves emails to members (canonicalization, ambiguity); the // client only extracts the single member from the per-email Result. Matching // is by the request email string the server echoes back. - result := func(email string, members ...*orgsvcv1.Member) *orgsvcv1.GetMembersByEmailResponse_Result { - return &orgsvcv1.GetMembersByEmailResponse_Result{Email: email, Members: members} + oneResult := func(email string, members ...*orgsvcv1.Member) []*orgsvcv1.GetMembersByEmailResponse_Result { + return []*orgsvcv1.GetMembersByEmailResponse_Result{{Email: email, Members: members}} } alice := &orgsvcv1.Member{UserId: "s2r:us:usr:alice", Email: "alice@example.com", DisplayName: "Alice"} carol1 := &orgsvcv1.Member{UserId: "s2r:us:usr:carol1", Email: "carol@example.com", DisplayName: "Carol One"} @@ -27,10 +27,10 @@ func TestSingleMember(t *testing.T) { wantUserID string wantErrPart string // empty means the lookup must succeed }{ - {name: "single member resolves", email: "alice@example.com", results: []*orgsvcv1.GetMembersByEmailResponse_Result{result("alice@example.com", alice)}, wantUserID: "s2r:us:usr:alice"}, - {name: "ambiguous email returns error listing matches", email: "carol@example.com", results: []*orgsvcv1.GetMembersByEmailResponse_Result{result("carol@example.com", carol1, carol2)}, wantErrPart: "s2r:us:usr:carol2"}, - {name: "empty members returns error naming the email", email: "dave@example.com", results: []*orgsvcv1.GetMembersByEmailResponse_Result{result("dave@example.com")}, wantErrPart: "dave@example.com"}, - {name: "no matching result returns error naming the email", email: "erin@example.com", results: []*orgsvcv1.GetMembersByEmailResponse_Result{result("alice@example.com", alice)}, wantErrPart: "erin@example.com"}, + {name: "single member resolves", email: "alice@example.com", results: oneResult("alice@example.com", alice), wantUserID: "s2r:us:usr:alice"}, + {name: "ambiguous email returns error listing matches", email: "carol@example.com", results: oneResult("carol@example.com", carol1, carol2), wantErrPart: "s2r:us:usr:carol2"}, + {name: "empty members returns error naming the email", email: "dave@example.com", results: oneResult("dave@example.com"), wantErrPart: "dave@example.com"}, + {name: "no matching result returns error naming the email", email: "erin@example.com", results: oneResult("alice@example.com", alice), wantErrPart: "erin@example.com"}, {name: "empty results returns error", email: "alice@example.com", results: nil, wantErrPart: "alice@example.com"}, }