Skip to content

Add SCIM tests: principal-search dedup, uniqueness conflicts, and missing UserAttribute recovery#764

Open
dasarinaidu wants to merge 1 commit into
rancher:mainfrom
dasarinaidu:scim-missing-attr-tests
Open

Add SCIM tests: principal-search dedup, uniqueness conflicts, and missing UserAttribute recovery#764
dasarinaidu wants to merge 1 commit into
rancher:mainfrom
dasarinaidu:scim-missing-attr-tests

Conversation

@dasarinaidu

Copy link
Copy Markdown
Contributor

Problem / Motivation

Adds OpenLDAP SCIM validation coverage for three recently-fixed auth behaviors:

  • rancher/rancher #55820 (issue #54022): /v3/principals search returned SCIM-provisioned users/groups as local alongside their external principal (duplicate in cluster-member search).
  • rancher/rancher #56081 (issue #54022): SCIM GET/PUT/PATCH and group add-member returned 500 when a user's UserAttribute was missing (created on first login/refresh).
  • rancher/rancher #55220 (issue #54024): uniqueness now enforced on UpdateUser/PatchUser — changing externalId/userName to a value held by another user returns 409.

Changes

Tests (validation/auth/scim/scim_openldap_test.go)

  • TestSCIMPrincipalSearchUserNotLocal, TestSCIMPrincipalSearchGroupNotLocal — SCIM user/group not returned as local.
  • TestSCIMPrincipalSearchLocalUserIsLocal — a genuine local user still surfaces as local (dedup fix does not over-exclude).
  • TestSCIMGetUserMissingUserAttributeReturns200, TestSCIMPutUserMissingUserAttributeReturns200, TestSCIMPatchUserMissingUserAttributeReturns200, TestSCIMGroupAddMemberMissingUserAttribute — handlers recover from a missing UserAttribute (200 / membership recorded) instead of 500.
  • TestSCIMPatchExternalIDConflictReturns409, TestSCIMPutUserNameConflictReturns409 — identity-field collisions return 409.

Action helpers (actions/kubeapi/scim/)

  • VerifyPrincipalNotLocal / VerifyPrincipalIsLocal + shared searchPrincipals (verify.go) — /v3/principals search assertions.
  • ProvisionSCIMUserWithoutAttribute (scim.go) — creates a SCIM user and removes its UserAttribute to reproduce the not-yet-logged-in state.

Testing

Full TestSCIMOpenLDAPSuite run green against a build containing #56081. Build tag: validation.

🤖 Generated with Claude Code

@github-actions

Copy link
Copy Markdown

SCIMOpenLDAPTestSuite TestSCIMPatchDeactivateAdminReturns409
SCIMOpenLDAPTestSuite TestSCIMPatchExternalIDConflictReturns409
SCIMOpenLDAPTestSuite TestSCIMPutUserNameConflictReturns409
SCIMOpenLDAPTestSuite TestSCIMPrincipalSearchUserNotLocal
SCIMOpenLDAPTestSuite TestSCIMPrincipalSearchGroupNotLocal
SCIMOpenLDAPTestSuite TestSCIMGetUserMissingUserAttributeReturns200
SCIMOpenLDAPTestSuite TestSCIMPutUserMissingUserAttributeReturns200
SCIMOpenLDAPTestSuite TestSCIMPatchUserMissingUserAttributeReturns200
SCIMOpenLDAPTestSuite TestSCIMGroupAddMemberMissingUserAttribute
SCIMOpenLDAPTestSuite TestSCIMPrincipalSearchLocalUserIsLocal

TestSuites above were modified. TestSuites below use modified code from this PR.

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Pull request overview

Adds OpenLDAP SCIM validation coverage for recent Rancher SCIM/auth fixes: principal search dedup (no “local” duplicates for SCIM identities), recovery when a user’s UserAttribute is missing, and 409 conflicts on identity-field uniqueness collisions.

Changes:

  • Added new SCIM OpenLDAP suite tests covering /v3/principals dedup behavior, missing-UserAttribute recovery paths, and 409 conflict enforcement.
  • Introduced /v3/principals search verification helpers (VerifyPrincipalNotLocal / VerifyPrincipalIsLocal) in actions/kubeapi/scim.
  • Added an action helper to provision a SCIM user and remove its UserAttribute to reproduce the “never logged in” state.

Reviewed changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated 1 comment.

File Description
validation/auth/scim/scim_openldap_test.go Adds new SCIM OpenLDAP validation tests for dedup, missing-attribute recovery, and uniqueness conflicts.
actions/kubeapi/scim/verify.go Adds helper functions to assert /v3/principals search results are/aren’t local.
actions/kubeapi/scim/scim.go Adds helper to provision a SCIM user and delete its UserAttribute for recovery-path testing.

Comment on lines +34 to +58
_ = kwait.PollUntilContextTimeout(
context.Background(),
defaults.FiveSecondTimeout,
defaults.OneMinuteTimeout,
true,
func(ctx context.Context) (bool, error) {
principals, err := searchPrincipals(client, name)
if err != nil {
callErr = err
return false, err
}
for i := range principals {
if principals[i].Provider == shepherdauth.LocalAuth.String() {
localErr = fmt.Errorf("principal search for %q returned a local principal (id=%s, type=%s); externally-provisioned principals must not appear as local", name, principals[i].ID, principals[i].PrincipalType)
return false, localErr
}
}
return len(principals) > 0, nil
},
)
if localErr != nil {
return localErr
}
return callErr
}
…missing UserAttribute recovery

Covers rancher/rancher #54022 / #55820 / #55220 against OpenLDAP:
- /v3/principals no longer returns SCIM-provisioned users/groups as local,
  and a genuine local user still surfaces as local
- PATCH externalId / PUT userName changes that collide with another user return 409
- GET, PUT, PATCH, and group add-member recover from a missing UserAttribute
  instead of returning 500

Adds action helpers VerifyPrincipalNotLocal / VerifyPrincipalIsLocal and
ProvisionSCIMUserWithoutAttribute in actions/kubeapi/scim.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
@dasarinaidu
dasarinaidu force-pushed the scim-missing-attr-tests branch from 37c8e36 to 18fac50 Compare July 15, 2026 19:03
@github-actions

Copy link
Copy Markdown

SCIMOpenLDAPTestSuite TestSCIMPatchDeactivateAdminReturns409
SCIMOpenLDAPTestSuite TestSCIMPatchExternalIDConflictReturns409
SCIMOpenLDAPTestSuite TestSCIMPutUserNameConflictReturns409
SCIMOpenLDAPTestSuite TestSCIMPrincipalSearchUserNotLocal
SCIMOpenLDAPTestSuite TestSCIMPrincipalSearchGroupNotLocal
SCIMOpenLDAPTestSuite TestSCIMGetUserMissingUserAttributeReturns200
SCIMOpenLDAPTestSuite TestSCIMPutUserMissingUserAttributeReturns200
SCIMOpenLDAPTestSuite TestSCIMPatchUserMissingUserAttributeReturns200
SCIMOpenLDAPTestSuite TestSCIMGroupAddMemberMissingUserAttribute
SCIMOpenLDAPTestSuite TestSCIMPrincipalSearchLocalUserIsLocal

TestSuites above were modified. TestSuites below use modified code from this PR.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants