feat(sso): Enhance SAML connection support with additional fields and tests - #41
Open
jaymesC wants to merge 8 commits into
Open
feat(sso): Enhance SAML connection support with additional fields and tests#41jaymesC wants to merge 8 commits into
jaymesC wants to merge 8 commits into
Conversation
jaymesC
commented
Jul 30, 2026
jaymesC
commented
Contributor
- Added new fields to ssoConnectionDoc for SAML configuration: EnvID, IDPMetadataXML, IDPSSOURL, IDPCertificate, EntityID, ACSURL, SPCertificate, SPPrivateKey, SignRequests, and AttributeMappings.
- Updated ssoDocToConnection and ssoConnectionToDoc functions to handle new fields.
- Modified UpdateConnection method to include new SAML fields in MongoDB updates.
- Enhanced SsoAdminCreateConnectionRequest and SsoAdminUpdateConnectionRequest structs to support new SAML fields.
- Updated SDK specification to reflect new SAML fields in the Connection schema.
- Implemented tests for org membership management in org_membership_test.go.
- Added comprehensive tests for SAML provider functionality in saml_test.go, including validation, metadata handling, and end-to-end assertion processing.
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
… tests - Added new fields to ssoConnectionDoc for SAML configuration: EnvID, IDPMetadataXML, IDPSSOURL, IDPCertificate, EntityID, ACSURL, SPCertificate, SPPrivateKey, SignRequests, and AttributeMappings. - Updated ssoDocToConnection and ssoConnectionToDoc functions to handle new fields. - Modified UpdateConnection method to include new SAML fields in MongoDB updates. - Enhanced SsoAdminCreateConnectionRequest and SsoAdminUpdateConnectionRequest structs to support new SAML fields. - Updated SDK specification to reflect new SAML fields in the Connection schema. - Implemented tests for org membership management in org_membership_test.go. - Added comprehensive tests for SAML provider functionality in saml_test.go, including validation, metadata handling, and end-to-end assertion processing.
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
The otcPayload marshal in mintOTC serializes freshly-minted runtime session and refresh tokens, not hardcoded credentials, so gosec's secret-pattern heuristic (G101) is a false positive. Annotate with #nosec matching the repo convention (token_encryption.go, captcha.go).
Factor the connection-provisioning core out of handleAdminCreateConnection into an exported CreateConnection(ctx, CreateConnectionInput) and add an SSOStore() getter. Host apps that embed the plugin (e.g. Kineta) can then offer self-serve, tenant-scoped SSO management behind their own authorization instead of the platform-admin HTTP route, while still reusing the env resolution, SAML SP keypair generation, and ACS/EntityID derivation. The admin HTTP handler now delegates to CreateConnection; behavior is unchanged.
DB-managed OIDC connections generated a login URL with an empty redirect_uri (connectionToProvider never set one), which real IdPs reject. Derive the redirect_uri from PublicBaseURL (oidcRedirectURLFor) and add a GET /:provider/callback landing (handleOIDCRedirect) that exchanges the code, mints a one-time code, and 302s to the frontend return URL — mirroring the SAML ACS handoff, so one protocol-agnostic frontend callback + /exchange serves both SAML and OIDC. Add a route-registration test guarding the GET/POST co-location on /:provider/callback.
| } | ||
|
|
||
| fail := func(reason string) error { | ||
| http.Redirect(ctx.Response(), r, p.errorRedirect(returnURL, reason), http.StatusFound) |
Wrap the SSO connection store in an EncryptedStore that encrypts the OIDC client secret and the SAML SP private key using the engine token encryptor (AUTHSOME_TOKEN_ENCRYPTION_KEY — the same key that protects social OAuth tokens). AES-GCM output is version-prefixed + base64 (TEXT-safe) and Decrypt passes un-prefixed values through, so existing plaintext rows keep working and re-saving a connection upgrades it in place. No key configured → no-op.
The OTC-marshal false positive is gosec G117 (marshaled struct field matches secret pattern), not G101 — the annotation now lists both, so the Security scan actually suppresses it. Also fix the golangci-lint findings in the SAML provider/tests: - name the multi-value results of LoginURLWithRequestID / Metadata (gocritic unnamedResult) - reuse the outer err instead of shadowing it in saml_test (govet shadow) - httptest.NewRequestWithContext in saml_test (noctx) No behavior change.
goxmldsig v1.4.0 has a loop-variable-capture signature-bypass (GO-2026-4753) reachable from the SAML assertion-validation path (ParseXMLResponse). Bump to v1.6.0, the fixed release. Pulls beevik/etree v1.6.0 + clockwork v0.5.0. Build + SAML tests green.
juicycleff
requested changes
Aug 13, 2026
juicycleff
left a comment
juicycleff
left a comment
Contributor
There was a problem hiding this comment.
Needs to be addressed
| @@ -37,19 +38,30 @@ var _ Store = (*MongoStore)(nil) | |||
| // ────────────────────────────────────────────────── | |||
|
|
|||
| type ssoConnectionDoc struct { | |||
Contributor
There was a problem hiding this comment.
Shouldn't there be a relevant PostgreSQL store implementation and also there's no tests for the store
Contributor
Author
There was a problem hiding this comment.
Good catches 🙏
Store tests: added a conformance suite (712a0e7) covering the full sso.Store, CRUD, SAML/OIDC field round-trip, active-only domain/provider lookup, app-scoped list, not-found. Runs on memory + embedded SQLite.
Postgres store: it's already there, store_postgres.go + store_models.go (fromConnection/toConnection map all columns, SAML fields included). Verified field parity with the Mongo doc. SQLite uses the same mapping, so the new tests cover the Postgres path too.
Address review: the connection store had no CRUD tests. Add a shared conformance suite exercising the full sso.Store contract — SAML/OIDC field round-trip (including attribute_mappings JSON), active-only lookup by domain/provider, app-scoped listing, update, delete, and not-found. Runs against the in-memory store and an embedded SQLite backend. SQLite shares fromConnection/toConnection + ssoConnectionModel with the Postgres store, so it validates the same SQL column mapping without needing Docker.
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
3 participants
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.