-
Notifications
You must be signed in to change notification settings - Fork 55
feat(sso): add sysdig_sso_openid and sysdig_sso_saml resources #688
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: master
Are you sure you want to change the base?
Conversation
Add new Terraform resource to manage OpenID Connect SSO configurations via /api/v1/sso-settings/ endpoint. Refs: ET-553
Add SAML SSO configuration resource for Sysdig, allowing users to configure SAML-based Single Sign-On via Terraform. Features: - Support for metadata_url or metadata_xml (mutually exclusive) - SAML security settings (signature validation, signed assertions, destination verification, encryption support) - Common SSO fields (group mapping, single logout, auto user creation) - Full CRUD operations with optimistic locking via version field
- Change endpoint from /api/v1/sso-settings to /platform/v1/sso-settings - Move disable-before-delete logic to resource layer (API requires client_secret) - Add ForceNew to integration_name (cannot be updated via API) - Preserve additional_scopes from state when API returns null - Skip WithMetadata test due to Platform API bug (returns 500 for manual metadata)
| * `is_single_logout_enabled` - (Optional) Whether SAML Single Logout (SLO) is enabled. Default: `false`. | ||
| * `is_group_mapping_enabled` - (Optional) Whether group mapping from SAML attributes is enabled. Default: `false`. | ||
| * `group_mapping_attribute_name` - (Optional) The SAML attribute name that contains group membership information. Default: `groups`. | ||
| * `integration_name` - (Optional) A custom display name for this SSO integration. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
nit: the integration name, for both saml and openid, is not only affecting used to display a mnemonic name, but it should be used mostly to distinguish different IdP settings so that a customer can properly set then up, and select them in the login page, so I suggest to reword the description to something more generic like "A name to distinguish different SSO integrations".
| Default: true, | ||
| Description: "Whether to use automatic metadata discovery from the issuer URL", | ||
| }, | ||
| "group_attribute_name": { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
i think we have an issue with the current public api: GroupAttributeName is redundant and ignored on updates, while the correct field to use is groupMappingAttributeName (same as saml). So, we should also remove group_attribute_name here and only have group_mapping_attribute_name.
dbonf
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
related to sso settings we also have group mappings (/platform/v1/group-mappings, to be used when we define group mapping in this resource) and global sso settings (/platform/v1/global-sso-settings/, to enable / disable password login), are we planning also to include them?
Summary
sysdig_sso_openidresource for OpenID Connect SSO configurationsysdig_sso_samlresource for SAML SSO configurationBoth resources use the
/platform/v1/sso-settings/endpoint with differenttypediscriminators.sysdig_sso_openid features:
sysdig_sso_saml features:
Common SSO features (both resources):
Test plan
go test ./sysdig -v -run TestAccSSOOpenID -tags=tf_acc_sysdig_secure -timeout 120mgo test ./sysdig -v -run TestAccSSOSaml -tags=tf_acc_sysdig_secure -timeout 120m