Skip to content

Add access management: grants, groups, and org data sources - #11

Merged
Konyaka1 merged 15 commits into
mainfrom
aliaksei/grants
Jul 21, 2026
Merged

Add access management: grants, groups, and org data sources#11
Konyaka1 merged 15 commits into
mainfrom
aliaksei/grants

Conversation

@Konyaka1

@Konyaka1 Konyaka1 commented Jul 9, 2026 •
edited
Loading

Copy link
Copy Markdown
Contributor

Summary

Adds access-management resources to the provider so a Terraform config can grant users, service accounts, and groups access to SplitSecure resources — closing the gap where a resource created via Terraform was invisible to everyone but its creator until an admin fixed it up by hand. New resources splitsecure_grant and splitsecure_group, plus data sources to resolve principals and the org's directory.

Stacked on splitsecure/apis#14 and priv#1068 (merge those first). go.mod currently carries a dev-only replace github.com/splitsecure/apis => ../apis; before this is un-drafted the replace is dropped and the require is pinned to the merged apis version. CI is red until then.

Provider

  • splitsecure_grant — per-resource grant (resource, grantee) → view|use|edit. PutGrant upsert semantics, so changing tier updates in place; changing resource/grantee replaces. Create retries PermissionDenied briefly to absorb the server-side creator-grant write race after a resource's create-proposal lands. Import by resource_s2r,grantee_s2r.
  • splitsecure_group — locally-managed principal group with an authoritative members set. Refuses SCIM/system groups. Group mutations require the provider's service account to hold the org admin role (grants alone work with the default member role, via the creator grant).
  • Data sources — splitsecure_organization (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).
  • Wires an OrgService client; regenerated docs; unit tests for tier/import-ID/member-diff/matching helpers.

Example usage

data "splitsecure_organization" "current" {}

data "splitsecure_org_member" "oncall_lead" { email = "lead@example.com" }
data "splitsecure_org_member" "contractor"  { email = "contractor@example.com" }

# Existing SCIM-synced group — referenced, not managed.
data "splitsecure_group" "engineering" { name = "Engineering" }

resource "splitsecure_saml2_identity_provider" "corp" {
  team_s2r      = var.team_s2r
  name          = "corp-idp"
  justification = "Company IdP for workforce federation."
}

resource "splitsecure_saml2_service_provider" "aws_prod" {
  team_s2r         = var.team_s2r
  idp_resource_s2r = splitsecure_saml2_identity_provider.corp.id
  name             = "aws-prod"
  entity_id        = "urn:amazon:webservices"
  acs_url          = "https://signin.aws.amazon.com/saml"
  justification    = "Admin federation into prod AWS."
  account {
    kind = "aws"
    aws {
      saml_provider_arn = "arn:aws:iam::111111111111:saml-provider/splitsecure"
      allowed_role_arns = ["arn:aws:iam::111111111111:role/SplitSecureAdmin"]
    }
  }
}

# A Terraform-owned group (authoritative membership).
resource "splitsecure_group" "platform_oncall" {
  name = "platform-oncall"
  members = [
    data.splitsecure_org_member.oncall_lead.user_s2r,
    data.splitsecure_org_member.contractor.user_s2r,
  ]
}

# The access matrix: who can do what, on which resource.
locals {
  grants = {
    "prod/oncall-use" = { resource = splitsecure_saml2_service_provider.aws_prod.id, grantee = splitsecure_group.platform_oncall.group_s2r,             tier = "use" }
    "prod/eng-view"   = { resource = splitsecure_saml2_service_provider.aws_prod.id, grantee = data.splitsecure_group.engineering.group_s2r,            tier = "view" }
    "prod/org-view"   = { resource = splitsecure_saml2_service_provider.aws_prod.id, grantee = data.splitsecure_organization.current.everyone_group_s2r, tier = "view" }
  }
}

resource "splitsecure_grant" "matrix" {
  for_each     = local.grants
  resource_s2r = each.value.resource
  grantee_s2r  = each.value.grantee
  tier         = each.value.tier
}

Terraform's dependency graph orders each grant after the resource it references (whose create blocks through voter approval), and destroy deletes grants before the resource. Tiers control who can see and administer a resource; resource mutations remain proposal/voter-gated regardless of tier.

Self-review

A workflow-backed review (xhigh) surfaced 9 findings; the load-bearing ones are fixed in this branch:

  • Member removal now tolerates NotFound — matches DeleteGroup/DeleteGrant; an already-gone principal no longer wedges apply.
  • Grant Create/Update write state from the plan, not the server echo, so a server-side s2r canonicalization can't trip the framework's post-apply consistency check on the RequiresReplace attributes.
  • DRY: shared upsertGrant (Create/Update differ only by the auth retry); a package-local clientFromProviderData for all five Configure methods.
  • Dropped dead UseStateForUnknown guards in group Update.
  • Added tests: clientFromProviderData, and member-removal NotFound-tolerance plus the non-NotFound failure path (via a minimal fake OrgService client).

Findings intentionally not actioned: the group Read non-LOCAL guard (a group's source is immutable server-side — UpdateGroup only writes name, SCIM groups are a separate store — so it's unreachable; terraform state rm remains the escape hatch), and empty-everyone_group_s2r / unset-CreateGroup-source handling (old orgs are out of scope; the server echoing source is the contract).

Test plan

  • After apis#14 + priv#1068 merge: drop the replace, pin the apis require, go build/go test/golangci-lint, make check-docs.
  • go build ./..., go test ./... (incl. new grant/group unit tests), golangci-lint run (only finding is the temporary local replace).
  • Local end-to-end against a real org: member-role SA denied grant/group mutations with actionable errors; admin-role SA succeeds; grants at mixed tiers verified in the web UI and audit log; idempotent re-plan; import; destroy.

Konyaka1 and others added 3 commits July 8, 2026 23:37
@Konyaka1 @claude
Add access management: grants, groups, and org data sources
c53b12a 
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 <noreply@anthropic.com>
@Konyaka1 @claude
org: address self-review findings on grants and groups
b6374db 
- 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 <noreply@anthropic.com>
@Konyaka1 @claude
Revert the internal/wait extraction
f57a50f 
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 <noreply@anthropic.com>
@Konyaka1
Konyaka1 requested a review from geffrak July 10, 2026 18:41
@Konyaka1
Konyaka1 marked this pull request as ready for review July 10, 2026 18:41
@Konyaka1

Konyaka1 commented Jul 10, 2026

Copy link
Copy Markdown
Contributor Author

pipe is failing because apis#14 is not merged yet

Konyaka1 and others added 4 commits July 13, 2026 17:31
@Konyaka1 @claude
org: group_s2r-only group data source + review hardening
f1157e8 
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 <noreply@anthropic.com>
@Konyaka1 @claude
deps: pin merged apis, drop dev replace
e96bf8e 
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 <noreply@anthropic.com>
@Konyaka1 @claude
Add splitsecure_principal data source (email -> s2r for users and SAs)
6a39807 
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) <noreply@anthropic.com>
@Konyaka1 @claude
Satisfy nlreturn in splitsecure_principal data source
5139407 
Blank line before returns; fixes the CI lint failure.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
geffrak
geffrak reviewed Jul 15, 2026
View reviewed changes
Comment thread examples/full/main.tf Outdated
Konyaka1 and others added 3 commits July 15, 2026 17:19
@Konyaka1 @claude
examples: move full-example vars to variables.tf; principal by email …
f0b67a3 
…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) <noreply@anthropic.com>
@Konyaka1 @claude
org: resolve splitsecure_principal via GetMembersByEmail
750d1c3 
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) <noreply@anthropic.com>
@Konyaka1 @claude
org: read GetMembersByEmail's per-email Result shape
57392f5 
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) <noreply@anthropic.com>
@coderabbitai

coderabbitai Bot commented Jul 17, 2026 •
edited
Loading

Copy link
Copy Markdown

Review Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: ASSERTIVE

Plan: Pro Plus

Run ID: 89a38849-3973-46d8-8b68-365e9829063e

📥 Commits

Reviewing files that changed from the base of the PR and between 7958fe8 and 58c40a6.

📒 Files selected for processing (1)
  • splitsecure/services/org/data_internal_test.go
🔗 Linked repositories identified

CodeRabbit considers these linked repositories for cross-repo context during reviews:

  • splitsecure/apis (auto-detected)
📜 Recent review details
⏰ Context from checks skipped due to timeout. (3)
  • GitHub Check: Analyze (go)
  • GitHub Check: Analyze (actions)
  • GitHub Check: Analyze (go)
🧰 Additional context used
🔀 Multi-repo context

Linked repositories findings

No additional cross-repository context beyond the previously identified API contract findings was necessary.

🔇 Additional comments (1)
splitsecure/services/org/data_internal_test.go (1)

10-60: LGTM!

Walkthrough

Adds OrgService-backed Terraform resources for grants and local groups, organization-related data sources, provider registration, client wiring, tests, examples, and generated documentation.

Changes

Organization capabilities

Layer / File(s) Summary
OrgService client and provider registration
go.mod, splitsecure/client/client.go, splitsecure/provider/provider.go, splitsecure/services/org/...		 Adds the OrgService client, registers organization resources and data sources, and validates shared provider client data.
Organization and principal data sources
splitsecure/services/org/*_data.go, splitsecure/services/org/*_internal_test.go		 Implements organization, member, principal, and group lookups with schema validation, RPC error handling, and Terraform state updates.
Permission grant lifecycle
splitsecure/services/org/grant.go, splitsecure/services/org/grant_internal_test.go		 Adds grant lifecycle operations, import parsing, tier conversion, authorization retry behavior, and validation tests.
Local group lifecycle
splitsecure/services/org/group.go, splitsecure/services/org/group_data.go, splitsecure/services/org/group_internal_test.go		 Adds local group management, authoritative membership reconciliation, import support, and partial-result handling.
Examples and generated documentation
docs/*, examples/*, README.md, templates/index.md.tmpl		 Documents the new capabilities and updates examples for organization lookups, operator groups, and service-provider grants.

Estimated code review effort: 4 (Complex) | ~60 minutes

Sequence Diagram(s)

sequenceDiagram
  participant Terraform
  participant Provider
  participant OrgService
  participant OrganizationAPI
  Terraform->>Provider: Configure organization resources and data sources
  Provider->>OrgService: Create OrgService client
  Terraform->>OrgService: Resolve organization, principals, and groups
  OrgService->>OrganizationAPI: Execute organization RPCs
  OrganizationAPI-->>OrgService: Return organization data
  OrgService-->>Terraform: Return Terraform state
  Terraform->>OrgService: Apply groups and permission grants
  OrgService-->>Terraform: Return applied resource state
Loading
🚥 Pre-merge checks | ✅ 4
✅ Passed checks (4 passed)
Check name Status Explanation
Title check ✅ Passed The title clearly summarizes the main change: access-management grants, groups, and org data sources.
Description check ✅ Passed The description matches the changeset and accurately describes the new resources, data sources, examples, and constraints.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.

Comment @coderabbitai help to get the list of available commands.

coderabbitai[bot]
coderabbitai Bot reviewed Jul 17, 2026
View reviewed changes

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 5

🤖 Prompt for all review comments with AI agents 
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@docs/data-sources/principal.md`:
- Around line 5-11: Align principal descriptions with GetMembersByEmail: in
docs/data-sources/principal.md lines 5-11, regenerate the page from the schema
description and remove the direct service-account parsing claim from both
duplicated descriptions; in
examples/data-sources/splitsecure_principal/data-source.tf lines 1-4, state that
service-account emails are resolved through the member directory.

In `@splitsecure/client/client.go`:
- Line 53: Update the client setup around OrgService and its NewOrgServiceClient
construction to disable transport retries for mutation RPCs, including
CreateGroup, UpdateGroup, DeleteGroup, PutGrant, and DeleteGrant. Use a separate
non-retrying HTTP client or an RPC-specific CheckRetry configuration, while
preserving existing retry behavior for safe OrgService operations.

In `@splitsecure/services/org/grant_internal_test.go`:
- Around line 51-81: Strengthen TestGrantSchema_RequiresReplace and
TestGrantSchema_TierHasOneOfValidator by executing the configured plan modifiers
and validators instead of checking only slice lengths. Verify resource_s2r and
grantee_s2r require replacement, and verify tier accepts supported tier values
while rejecting an invalid value.

In `@splitsecure/services/org/group.go`:
- Around line 342-358: The listMemberPrincipals method must consume all
ListGroupMembers pages instead of returning only the first page. Loop while the
response provides next_cursor, requesting each subsequent page with the same
groupS2R and cursor, and append every member principal while preserving existing
empty-principal errors. Add coverage for a two-page response to verify
principals from both pages are returned.

In `@splitsecure/services/org/principal_data.go`:
- Around line 135-147: Update principalKindFromS2R to reject empty required S2R
segments, including the deployment segment and identifier segment, before
evaluating parts[2]. Preserve the existing prefix, segment-count, and
supported-kind validation, and add test cases covering malformed inputs such as
"s2r::usr:" and other empty required segments.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: ASSERTIVE

Plan: Pro Plus

Run ID: d6248a2c-a927-47d8-a06f-374311338599

📥 Commits

Reviewing files that changed from the base of the PR and between 0c92209 and 57392f5.

⛔ Files ignored due to path filters (1)
  • go.sum is excluded by !**/*.sum
📒 Files selected for processing (34)
  • README.md
  • docs/data-sources/group.md
  • docs/data-sources/org_member.md
  • docs/data-sources/organization.md
  • docs/data-sources/principal.md
  • docs/index.md
  • docs/resources/grant.md
  • docs/resources/group.md
  • examples/data-sources/splitsecure_group/data-source.tf
  • examples/data-sources/splitsecure_org_member/data-source.tf
  • examples/data-sources/splitsecure_organization/data-source.tf
  • examples/data-sources/splitsecure_principal/data-source.tf
  • examples/full/main.tf
  • examples/full/terraform.tfvars.example
  • examples/full/variables.tf
  • examples/resources/splitsecure_grant/resource.tf
  • examples/resources/splitsecure_group/resource.tf
  • go.mod
  • splitsecure/client/client.go
  • splitsecure/provider/provider.go
  • splitsecure/services/org/configure.go
  • splitsecure/services/org/configure_internal_test.go
  • splitsecure/services/org/data_internal_test.go
  • splitsecure/services/org/doc.go
  • splitsecure/services/org/grant.go
  • splitsecure/services/org/grant_internal_test.go
  • splitsecure/services/org/group.go
  • splitsecure/services/org/group_data.go
  • splitsecure/services/org/group_internal_test.go
  • splitsecure/services/org/member_data.go
  • splitsecure/services/org/organization_data.go
  • splitsecure/services/org/principal_data.go
  • splitsecure/services/org/principal_data_internal_test.go
  • templates/index.md.tmpl
📜 Review details
⏰ Context from checks skipped due to timeout. (2)
  • GitHub Check: Analyze (go)
  • GitHub Check: Analyze (go)
⚠️ CI failures not shown inline (2)

GitHub Actions: CI / Check Generated Docs: Add access management: grants, groups, and org data sources

Conclusion: failure

View job details

##[group]Run make check-docs
 �[36;1mmake check-docs�[0m
 shell: /usr/bin/bash -e {0}
 env:
   GOPRIVATE: github.com/splitsecure/
   GOTOOLCHAIN: local
 ##[endgroup]
 go generate ./...
 go: downloading github.com/hashicorp/go-retryablehttp v0.7.8
 go: downloading github.com/splitsecure/apis v0.0.0-20260717151105-8a4c7579b056
 go: downloading github.com/hashicorp/terraform-plugin-log v0.10.0
 go: downloading connectrpc.com/connect v1.20.0
 go: downloading github.com/hashicorp/terraform-plugin-framework-validators v0.19.0
 go: downloading github.com/hashicorp/terraform-plugin-framework v1.19.0
 go: downloading google.golang.org/protobuf v1.36.11
 go: downloading github.com/hashicorp/go-cleanhttp v0.5.2
 go: downloading github.com/hashicorp/go-hclog v1.6.3
 go: downloading github.com/hashicorp/terraform-plugin-go v0.31.0
 go: downloading github.com/fatih/color v1.18.0
 go: downloading github.com/mattn/go-isatty v0.0.20
 go: downloading github.com/vmihailenco/msgpack/v5 v5.4.1
 go: downloading github.com/hashicorp/go-uuid v1.0.3
 go: downloading github.com/hashicorp/go-plugin v1.7.0
 go: downloading github.com/mitchellh/go-testing-interface v1.14.1
 go: downloading google.golang.org/grpc v1.79.2
 go: downloading github.com/mattn/go-colorable v0.1.14
 go: downloading golang.org/x/sys v0.42.0
 go: downloading github.com/vmihailenco/tagparser/v2 v2.0.0
 go: downloading github.com/hashicorp/terraform-registry-address v0.4.0
 go: downloading github.com/golang/protobuf v1.5.4
 go: downloading github.com/hashicorp/yamux v0.1.2
 go: downloading github.com/oklog/run v1.1.0
 go: downloading golang.org/x/net v0.52.0
 go: downloading google.golang.org/genproto/googleapis/rpc v0.0.0-20251202230838-ff82c1b0f217
 go: downloading github.com/hashicorp/terraform-svchost v0.1.1
 go: downloading golang.org/x/text v0.36.0
 go: downloading github.com/hashicorp/terraform-plugin-docs v0.25.0
 go: downloading github.com/hashicorp/cli v1.1.7
 go: downloading github.com/Kunde21/markdownfmt/v3 ...

GitHub Actions: CI / 2_Check Generated Docs.txt: Add access management: grants, groups, and org data sources

Conclusion: failure

View job details

##[group]Run make check-docs
 �[36;1mmake check-docs�[0m
 shell: /usr/bin/bash -e {0}
 env:
   GOPRIVATE: github.com/splitsecure/
   GOTOOLCHAIN: local
 ##[endgroup]
 go generate ./...
 go: downloading github.com/hashicorp/go-retryablehttp v0.7.8
 go: downloading github.com/splitsecure/apis v0.0.0-20260717151105-8a4c7579b056
 go: downloading github.com/hashicorp/terraform-plugin-log v0.10.0
 go: downloading connectrpc.com/connect v1.20.0
 go: downloading github.com/hashicorp/terraform-plugin-framework-validators v0.19.0
 go: downloading github.com/hashicorp/terraform-plugin-framework v1.19.0
 go: downloading google.golang.org/protobuf v1.36.11
 go: downloading github.com/hashicorp/go-cleanhttp v0.5.2
 go: downloading github.com/hashicorp/go-hclog v1.6.3
 go: downloading github.com/hashicorp/terraform-plugin-go v0.31.0
 go: downloading github.com/fatih/color v1.18.0
 go: downloading github.com/mattn/go-isatty v0.0.20
 go: downloading github.com/vmihailenco/msgpack/v5 v5.4.1
 go: downloading github.com/hashicorp/go-uuid v1.0.3
 go: downloading github.com/hashicorp/go-plugin v1.7.0
 go: downloading github.com/mitchellh/go-testing-interface v1.14.1
 go: downloading google.golang.org/grpc v1.79.2
 go: downloading github.com/mattn/go-colorable v0.1.14
 go: downloading golang.org/x/sys v0.42.0
 go: downloading github.com/vmihailenco/tagparser/v2 v2.0.0
 go: downloading github.com/hashicorp/terraform-registry-address v0.4.0
 go: downloading github.com/golang/protobuf v1.5.4
 go: downloading github.com/hashicorp/yamux v0.1.2
 go: downloading github.com/oklog/run v1.1.0
 go: downloading golang.org/x/net v0.52.0
 go: downloading google.golang.org/genproto/googleapis/rpc v0.0.0-20251202230838-ff82c1b0f217
 go: downloading github.com/hashicorp/terraform-svchost v0.1.1
 go: downloading golang.org/x/text v0.36.0
 go: downloading github.com/hashicorp/terraform-plugin-docs v0.25.0
 go: downloading github.com/hashicorp/cli v1.1.7
 go: downloading github.com/Kunde21/markdownfmt/v3 ...
🧰 Additional context used
🪛 GitHub Actions: CI / 2_Check Generated Docs.txt
docs/data-sources/principal.md

[error] 1-1: Documentation content mismatch detected by check-docs (principal data source description changed).

🔇 Additional comments (29)
splitsecure/services/org/grant.go (1)

1-335: LGTM!

splitsecure/services/org/grant_internal_test.go (1)

1-50: LGTM!

Also applies to: 84-222

README.md (1)

9-12: LGTM!

docs/data-sources/group.md (1)

1-36: LGTM!

docs/data-sources/org_member.md (1)

1-34: LGTM!

docs/data-sources/organization.md (1)

1-29: LGTM!

examples/full/terraform.tfvars.example (1)

1-10: LGTM!

examples/full/variables.tf (1)

1-15: LGTM!

examples/resources/splitsecure_grant/resource.tf (1)

10-20: 🎯 Functional Correctness

Verify standalone wiring for the resource examples.

Both examples reference Terraform objects declared outside their own files. If these snippets are presented as independently copyable examples, they will fail during planning unless the prerequisite declarations are included or the documentation clearly identifies the required companion configuration.

  • examples/resources/splitsecure_grant/resource.tf#L10-L20: provide or document splitsecure_saml2_service_provider.main, data.splitsecure_group.sre, and data.splitsecure_organization.current.
  • examples/resources/splitsecure_group/resource.tf#L13-L18: provide or document data.splitsecure_principal.alice and data.splitsecure_principal.ci_bot.
templates/index.md.tmpl (1)

22-24: LGTM!

docs/index.md (1)

42-60: LGTM!

Also applies to: 79-80, 214-243

docs/resources/grant.md (1)

1-47: LGTM!

docs/resources/group.md (1)

1-52: LGTM!

examples/data-sources/splitsecure_group/data-source.tf (1)

1-8: LGTM!

examples/data-sources/splitsecure_org_member/data-source.tf (1)

1-6: LGTM!

examples/data-sources/splitsecure_organization/data-source.tf (1)

1-4: LGTM!

examples/full/main.tf (1)

18-19: LGTM!

Also applies to: 152-181

go.mod (1)

13-13: LGTM!

splitsecure/client/client.go (1)

13-13: LGTM!

Also applies to: 27-27

splitsecure/provider/provider.go (1)

15-15: LGTM!

Also applies to: 113-124

splitsecure/services/org/configure.go (1)

1-30: LGTM!

splitsecure/services/org/configure_internal_test.go (1)

1-32: LGTM!

splitsecure/services/org/doc.go (1)

1-5: LGTM!

splitsecure/services/org/organization_data.go (1)

1-98: LGTM!

splitsecure/services/org/member_data.go (1)

1-131: LGTM!

splitsecure/services/org/principal_data.go (1)

1-134: LGTM!

splitsecure/services/org/group_data.go (1)

1-140: LGTM!

splitsecure/services/org/data_internal_test.go (1)

1-100: LGTM!

splitsecure/services/org/principal_data_internal_test.go (1)

1-42: LGTM!

Comment thread docs/data-sources/principal.md Outdated
Comment thread splitsecure/client/client.go Outdated
Comment thread splitsecure/services/org/grant_internal_test.go
Comment thread splitsecure/services/org/group.go Outdated
Comment thread splitsecure/services/org/principal_data.go
@Konyaka1 @claude
org: address review on splitsecure_principal
747f90e 
- 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) <noreply@anthropic.com>
coderabbitai[bot]
coderabbitai Bot reviewed Jul 17, 2026
View reviewed changes

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (1)
splitsecure/services/org/principal_data.go (1)

89-92: 🗄️ Data Integrity & Integration | 🟠 Major | 🏗️ Heavy lift

Pass the organization ID here, not d.client.OrgS2R (splitsecure/services/org/principal_data.go:89-92).
GetMembersByEmailRequest.Base.OrganizationId expects the org ID; d.client.OrgS2R is the configured org S2R URI. Thread the resolved org ID into this path before calling GetMembersByEmail, or principal lookups will fail.

🤖 Prompt for AI Agents 
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@splitsecure/services/org/principal_data.go` around lines 89 - 92, Update the
principal lookup flow around GetMembersByEmail so
GetMembersByEmailRequest.Base.OrganizationId receives the resolved organization
ID rather than d.client.OrgS2R, which is the configured S2R URI. Thread that
resolved ID into this path before calling d.client.OrgService.GetMembersByEmail,
preserving the existing email lookup behavior.

Source: Linked repositories

🤖 Prompt for all review comments with AI agents 
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Outside diff comments:
In `@splitsecure/services/org/principal_data.go`:
- Around line 89-92: Update the principal lookup flow around GetMembersByEmail
so GetMembersByEmailRequest.Base.OrganizationId receives the resolved
organization ID rather than d.client.OrgS2R, which is the configured S2R URI.
Thread that resolved ID into this path before calling
d.client.OrgService.GetMembersByEmail, preserving the existing email lookup
behavior.
ℹ️ Review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: ASSERTIVE

Plan: Pro Plus

Run ID: c9c0a400-a029-4f31-9673-af8a0eef415d

📥 Commits

Reviewing files that changed from the base of the PR and between 57392f5 and 747f90e.

📒 Files selected for processing (4)
  • docs/data-sources/principal.md
  • examples/data-sources/splitsecure_principal/data-source.tf
  • splitsecure/services/org/principal_data.go
  • splitsecure/services/org/principal_data_internal_test.go
🔗 Linked repositories identified

CodeRabbit considers these linked repositories for cross-repo context during reviews:

  • splitsecure/apis (auto-detected)
📜 Review details
⏰ Context from checks skipped due to timeout. (2)
  • GitHub Check: Analyze (go)
  • GitHub Check: Analyze (go)
🧰 Additional context used
🔀 Multi-repo context splitsecure/apis

Linked repositories findings

splitsecure/apis

  • Org scoping differs across RPCs. Grant requests explicitly require org_s2r (put_grant.proto:8-13, get_grant.proto:7-11, delete_grant.proto:5-9), while GetMembersByEmailRequest requires base.organization_id (get_members_by_email.proto:19-27). The provider’s principal data source should ensure it sends the organization ID, not the configured org_s2r. [::splitsecure/apis::]
  • Group-member listing is cursor-paginated. ListGroupMembersRequest exposes limit and cursor, and the response returns next_cursor (list_group_members.proto:7-15). The provider’s group read/reconciliation logic must follow next_cursor to avoid managing only the first page of membership. [::splitsecure/apis::]
  • Group mutations accept only user/service-account principals. AddGroupMembersRequest explicitly rejects other S2R kinds (add_group_members.proto:5-9), consistent with the provider’s documented members restriction. [::splitsecure/apis::]
  • Everyone is a system group and is intentionally excluded from group listings. Its S2R must come from Organization.everyone_group_s2r; ListGroupMembers rejects it (orgsvc_service.proto:35-40, organization_resource.proto:8-11). This matches the new organization data-source documentation and example usage. [::splitsecure/apis::]
  • Grant mutations require edit authorization. PutGrant/delete operations require the caller to hold edit tier on the resource, with org owners/admins implicit (orgsvc_service.proto:50-56). The provider’s retry handling for permission races does not replace the required authorization. [::splitsecure/apis::]
🔇 Additional comments (4)
splitsecure/services/org/principal_data.go (1)

135-148: LGTM!

splitsecure/services/org/principal_data_internal_test.go (1)

18-19: LGTM!

docs/data-sources/principal.md (1)

5-41: LGTM!

examples/data-sources/splitsecure_principal/data-source.tf (1)

1-12: LGTM!

@Konyaka1 @claude
org: address grants review (retries, pagination, schema tests)
a9e8490 
- 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) <noreply@anthropic.com>
coderabbitai[bot]
coderabbitai Bot reviewed Jul 17, 2026
View reviewed changes

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 1

🤖 Prompt for all review comments with AI agents 
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@splitsecure/services/org/grant_internal_test.go`:
- Around line 87-119: Update the local validate function in
TestGrantSchema_TierValidatorAcceptsTiers by inserting a blank line immediately
before return resp.Diagnostics to satisfy the nlreturn linter.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: ASSERTIVE

Plan: Pro Plus

Run ID: 9aaa103f-9e48-4096-ba06-3c8814c87782

📥 Commits

Reviewing files that changed from the base of the PR and between 747f90e and d4f6966.

📒 Files selected for processing (4)
  • splitsecure/client/client.go
  • splitsecure/services/org/grant_internal_test.go
  • splitsecure/services/org/group.go
  • splitsecure/services/org/group_internal_test.go
🔗 Linked repositories identified

CodeRabbit considers these linked repositories for cross-repo context during reviews:

  • splitsecure/apis (auto-detected)
📜 Review details
⏰ Context from checks skipped due to timeout. (2)
  • GitHub Check: Analyze (go)
  • GitHub Check: Analyze (go)
🧰 Additional context used
🪛 GitHub Actions: CI / 1_Lint.txt
splitsecure/services/org/grant_internal_test.go

[error] 108-108: golangci-lint: nlreturn violation. 'return' statement missing a blank line before it.

🪛 GitHub Actions: CI / Lint
splitsecure/services/org/grant_internal_test.go

[error] 108-108: golangci-lint: (nlreturn) return statement must be preceded by a blank line.

🪛 GitHub Check: Lint
splitsecure/services/org/grant_internal_test.go

[failure] 108-108:
return with no blank line before (nlreturn)

🔀 Multi-repo context splitsecure/apis

Linked repositories findings

splitsecure/apis

  • Org-scoped grant RPCs require org_s2r, while member lookup uses base.organization_id; callers must not substitute the provider’s organization S2R for the latter. [::splitsecure/apis::]
  • Group-member listing is cursor-paginated via limit, cursor, and next_cursor; provider reads must consume all pages. [::splitsecure/apis::]
  • Group mutations accept only user/service-account principals, matching the provider’s documented membership restriction. [::splitsecure/apis::]
  • The Everyone group is system-managed and excluded from normal group listings; its S2R comes from Organization.everyone_group_s2r. [::splitsecure/apis::]
  • Grant mutations require edit authorization on the resource, with org owners/admins implicit; retry logic does not replace authorization. [::splitsecure/apis::]
🔇 Additional comments (4)
splitsecure/services/org/group.go (1)

343-362: LGTM!

splitsecure/services/org/group_internal_test.go (1)

7-7: LGTM!

Also applies to: 36-36, 70-86, 467-484

splitsecure/client/client.go (1)

37-64: LGTM!

splitsecure/services/org/grant_internal_test.go (1)

49-85: LGTM!

Also applies to: 121-259

Comment thread splitsecure/services/org/grant_internal_test.go
Konyaka1 and others added 2 commits July 17, 2026 16:10
@Konyaka1 @claude
deps: bump apis to merged main (teamresource lineage_id #18)
5428b22 
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) <noreply@anthropic.com>
@Konyaka1 @claude
org_member data source: resolve via GetMembersByEmail RPC
7958fe8 
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) <noreply@anthropic.com>
coderabbitai[bot]
coderabbitai Bot reviewed Jul 17, 2026
View reviewed changes

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 1

🤖 Prompt for all review comments with AI agents 
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@splitsecure/services/org/data_internal_test.go`:
- Around line 30-33: Reformat the overlong table literals in the test cases
“single member resolves” and “no matching result returns error naming the email”
into multiple lines so each line stays within the configured 200-character
limit, without changing their values or assertions.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: ASSERTIVE

Plan: Pro Plus

Run ID: 548bacc2-6e61-42e5-966b-a41b170ea38e

📥 Commits

Reviewing files that changed from the base of the PR and between 5428b22 and 7958fe8.

📒 Files selected for processing (2)
  • splitsecure/services/org/data_internal_test.go
  • splitsecure/services/org/member_data.go
🔗 Linked repositories identified

CodeRabbit considers these linked repositories for cross-repo context during reviews:

  • splitsecure/apis (auto-detected)
📜 Review details
⏰ Context from checks skipped due to timeout. (2)
  • GitHub Check: Analyze (go)
  • GitHub Check: Analyze (go)
🧰 Additional context used
🪛 GitHub Actions: CI / 0_Lint.txt
splitsecure/services/org/data_internal_test.go

[error] 31-31: golangci-lint: The line is 220 characters long, which exceeds the maximum of 200 characters. (lll)

🪛 GitHub Actions: CI / Lint
splitsecure/services/org/data_internal_test.go

[error] 31-31: golangci-lint (lll): The line is 220 characters long, which exceeds the maximum of 200 characters.

🪛 GitHub Check: Lint
splitsecure/services/org/data_internal_test.go

[failure] 33-33:
The line is 213 characters long, which exceeds the maximum of 200 characters. (lll)

[failure] 31-31:
The line is 220 characters long, which exceeds the maximum of 200 characters. (lll)

🔀 Multi-repo context

Linked repositories findings

No additional cross-repository context beyond the previously identified API contract findings was necessary.

🔇 Additional comments (1)
splitsecure/services/org/member_data.go (1)

116-124: 🎯 Functional Correctness

Drop this finding. singleMember intentionally matches the echoed request email, and the tests codify that contract.

			> Likely an incorrect or invalid review comment.

Comment thread splitsecure/services/org/data_internal_test.go Outdated
@Konyaka1 @claude
test: wrap long TestSingleMember table lines to satisfy lll
58c40a6 
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) <noreply@anthropic.com>
@Konyaka1
Konyaka1 requested a review from geffrak July 20, 2026 19:26
@Konyaka1
Konyaka1 merged commit 1d0e7b7 into main Jul 21, 2026
9 checks passed
@Konyaka1
Konyaka1 deleted the aliaksei/grants branch July 21, 2026 17:24
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@coderabbitai coderabbitai[bot] coderabbitai[bot] left review comments
@geffrak geffrak geffrak approved these changes

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

2 participants

@Konyaka1 @geffrak