CXH-1473: Add source_type fields to GroupTrait and AppTrait#848
Open
al-conductorone wants to merge 6 commits into
Open
CXH-1473: Add source_type fields to GroupTrait and AppTrait#848al-conductorone wants to merge 6 commits into
al-conductorone wants to merge 6 commits into
Conversation
Add typed source_type metadata to two traits so IDP-supplied group/app kind information can flow through the SDK instead of being dropped or stuffed into the loose profile struct. GroupTrait gets group_source_type (field 3) plus raw_group_source_type (field 4); AppTrait gets app_source_type (field 6) and raw_app_source_type (field 7). All four are additive optional string fields with ignore_empty validation, so existing connectors keep working unchanged. For GroupTrait, expose a typed-string GroupSourceType vocabulary in pkg/types/resource matching the RFC table (native, app_imported, built_in, directory_synced, dynamic, distribution) plus the matching WithGroupSourceType / WithRawGroupSourceType functional options. For AppTrait, ship WithAppSourceType / WithRawAppSourceType as free-form strings; a normalized app vocabulary is out of scope for this slice. Tests lock the wire-format values of the group vocabulary and round-trip both option pairs.
Replace the tautological TestGroupSourceTypeVocabulary (which asserted
GroupSourceType("native") == GroupSourceTypeNative) with
TestGroupSourceTypeWireFormat: marshal each constant through a real
proto Marshal/Unmarshal and assert the deserialized GroupTrait carries
the documented literal string. A constant rename or typo now trips the
test; same-value aliases still pass.
Add per-trait validation tests covering the four new fields against the
proto validate.rules (max_bytes 64 on normalized, 256 on raw,
ignore_empty=true). Empty strings pass; 65-byte normalized and 257-byte
raw values surface a GroupTraitValidationError / AppTraitValidationError
with the expected Field() string.
Add annotation round-trip tests for both traits: pack the trait into a
v2.Resource via annotations.New, read it back through GetGroupTrait /
GetAppTrait, and confirm the new source_type fields survive the
anypb.UnmarshalTo path.
On the proto/Go side, expand the AppTrait.app_source_type comment with
example values ("saml", "oidc", "scim") instead of the unanchored
"follow-up RFC" pointer, and document that WithAppSourceType is
deliberately free-form (unlike WithGroupSourceType) so the next reader
doesn't read the asymmetry as an oversight.
Drop the inline group source-type vocabulary list from the proto comment
(it duplicates the Go source of truth at pkg/types/resource and creates
a two-places-to-update sync risk). Replace with a single-line pointer to
GroupSourceType.
Drop the raw_group_source_type / raw_app_source_type proto comments
entirely. Both restated the field name; the typed/raw pairing is already
explained on GroupSourceType's doc comment in pkg/types/resource.
Trim the AppTrait.app_source_type comment to one line. Examples ("saml",
"oidc", "scim") were borderline normative without being constraints, so
they invited misuse.
Trim TestGroupSourceTypeWireFormat's header to a single line that says
what the test catches (constant rename or typo) rather than restating
what the body does.
ghost
reviewed
May 21, 2026
Contributor
General PR Review: CXH-1473: Add source_type fields to GroupTrait and AppTraitBlocking Issues: 0 | Suggestions: 0 | Threads Resolved: 0 Review SummaryThe new commits add an Security IssuesNone found. Correctness IssuesNone found. SuggestionsNone. |
FeliLucero1
reviewed
May 22, 2026
Introduce AppSourceType string alias so callers can no longer pass a raw value to WithAppSourceType (or vice versa) without a compiler error. No constants yet, no app source-type vocab is defined. Also drop the stray blank lines in pkg/sdk/version.go.
luisina-santos
approved these changes
May 25, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Connectors can now report what kind of group or app each entry is (security group, distribution list, dynamic, etc.) so C1 stops dropping that classification when it pulls data from identity providers.