diff --git a/actions/kubeapi/scim/scim.go b/actions/kubeapi/scim/scim.go index 497ea593f2..e5488748cb 100644 --- a/actions/kubeapi/scim/scim.go +++ b/actions/kubeapi/scim/scim.go @@ -223,6 +223,35 @@ func CreateSCIMUser(scimClient *scimclient.Client, ts *session.Session, external return name, id, nil } +// ProvisionSCIMUserWithoutAttribute creates an active SCIM user with the given externalID, deletes its +// UserAttribute, and waits until the attribute is absent, returning the SCIM userName and Rancher user +// ID. It reproduces the state of a user provisioned but not yet logged in (no UserAttribute), used to +// exercise the SCIM handlers' missing-attribute recovery path. +func ProvisionSCIMUserWithoutAttribute(client *rancher.Client, scimClient *scimclient.Client, ts *session.Session, externalID string) (string, string, error) { + userName, userID, err := CreateSCIMUser(scimClient, ts, externalID, true) + if err != nil { + return "", "", err + } + + err = client.WranglerContext.Mgmt.UserAttribute().Delete(userID, &metav1.DeleteOptions{}) + if err != nil && !apierrors.IsNotFound(err) { + return "", "", fmt.Errorf("failed to delete UserAttribute for user %s: %w", userID, err) + } + + err = WaitForSCIMResourceDeletion(func() (int, error) { + _, getErr := client.WranglerContext.Mgmt.UserAttribute().Get(userID, metav1.GetOptions{}) + if apierrors.IsNotFound(getErr) { + return http.StatusNotFound, nil + } + return http.StatusOK, nil + }) + if err != nil { + return "", "", fmt.Errorf("UserAttribute for user %s was not deleted: %w", userID, err) + } + + return userName, userID, nil +} + // CreateSCIMGroup builds a SCIM group with a random displayName and the given test-specific options func CreateSCIMGroup(scimClient *scimclient.Client, ts *session.Session, externalID string) (string, string, error) { groupName := namegen.AppendRandomString("scim-group") diff --git a/actions/kubeapi/scim/verify.go b/actions/kubeapi/scim/verify.go new file mode 100644 index 0000000000..65f96d64de --- /dev/null +++ b/actions/kubeapi/scim/verify.go @@ -0,0 +1,73 @@ +package scim + +import ( + "context" + "fmt" + + "github.com/rancher/norman/types" + "github.com/rancher/shepherd/clients/rancher" + shepherdauth "github.com/rancher/shepherd/clients/rancher/auth" + v3 "github.com/rancher/shepherd/clients/rancher/generated/management/v3" + "github.com/rancher/shepherd/extensions/defaults" + kwait "k8s.io/apimachinery/pkg/util/wait" +) + +// searchPrincipals runs the /v3/principals search for the given name. The principal collection is +// listed first so the search collection action link is populated before invoking it. +func searchPrincipals(client *rancher.Client, name string) ([]v3.Principal, error) { + collection, err := client.Management.Principal.List(&types.ListOpts{}) + if err != nil { + return nil, fmt.Errorf("failed to list principals: %w", err) + } + result, err := client.Management.Principal.CollectionActionSearch(collection, &v3.SearchPrincipalsInput{Name: name}) + if err != nil { + return nil, fmt.Errorf("failed to search principals for %q: %w", name, err) + } + return result.Data, nil +} + +// VerifyPrincipalNotLocal polls the /v3/principals search for the given name and returns an error if +// any returned principal is from the local provider. Externally-provisioned users and groups (SCIM, +// SAML, OAuth) have no local login and must not be surfaced as local principals +func VerifyPrincipalNotLocal(client *rancher.Client, name string) error { + var localErr, callErr error + _ = kwait.PollUntilContextTimeout( + context.Background(), + defaults.FiveSecondTimeout, + defaults.TenSecondTimeout, + 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 +} + +// VerifyPrincipalIsLocal runs the /v3/principals search for the given name and returns an error if no +// local principal is returned. A genuine local user must still surface as local after external-user dedup. +func VerifyPrincipalIsLocal(client *rancher.Client, name string) error { + principals, err := searchPrincipals(client, name) + if err != nil { + return err + } + for i := range principals { + if principals[i].Provider == shepherdauth.LocalAuth.String() { + return nil + } + } + return fmt.Errorf("principal search for %q returned no local principal; a local user must still surface as local", name) +} diff --git a/validation/auth/scim/scim_openldap_test.go b/validation/auth/scim/scim_openldap_test.go index e4744e29b5..022d8e3699 100644 --- a/validation/auth/scim/scim_openldap_test.go +++ b/validation/auth/scim/scim_openldap_test.go @@ -2014,6 +2014,161 @@ func (s *SCIMOpenLDAPTestSuite) TestSCIMPatchDeactivateAdminReturns409() { require.NoError(s.T(), scimactions.CheckStatus(resp, http.StatusConflict, "PATCH active=false on default admin should return 409")) } +func (s *SCIMOpenLDAPTestSuite) TestSCIMPatchExternalIDConflictReturns409() { + subSession := s.session.NewSession() + defer subSession.Cleanup() + + logrus.Info("Verifying PATCH externalId to a value already held by another user returns 409 (userName mode, where externalId is mutable)") + + defer func() { require.NoError(s.T(), scimactions.RestoreSCIMBaseline(s.client, s.scimClient, authactions.OpenLdap)) }() + require.NoError(s.T(), scimactions.SetSCIMConfigMapAndWaitCreateReady(s.client, s.scimClient, authactions.OpenLdap, scimactions.BaselineSCIMConfigMap())) + + externalIDA := namegen.AppendRandomString("ext-conflict-a") + _, _, err := scimactions.CreateSCIMUser(s.scimClient, subSession, externalIDA, true) + require.NoError(s.T(), err) + + _, userIDB, err := scimactions.CreateSCIMUser(s.scimClient, subSession, namegen.AppendRandomString("ext-conflict-b"), true) + require.NoError(s.T(), err) + + resp, err := scimactions.PatchUser(s.scimClient, userIDB, "replace", "externalId", externalIDA) + require.NoError(s.T(), err) + require.NoError(s.T(), scimactions.CheckStatus(resp, http.StatusConflict, "PATCH externalId to another user's value should return 409")) +} + +func (s *SCIMOpenLDAPTestSuite) TestSCIMPutUserNameConflictReturns409() { + subSession := s.session.NewSession() + defer subSession.Cleanup() + + logrus.Info("Verifying PUT userName to a value already held by another user returns 409") + + defer func() { require.NoError(s.T(), scimactions.RestoreSCIMBaseline(s.client, s.scimClient, authactions.OpenLdap)) }() + require.NoError(s.T(), scimactions.SetSCIMConfigMapAndWaitCreateReady(s.client, s.scimClient, authactions.OpenLdap, map[string]string{"enabled": "true", "userIdAttribute": "externalId"})) + + userNameA, _, err := scimactions.CreateSCIMUser(s.scimClient, subSession, namegen.AppendRandomString("un-conflict-a-ext"), true) + require.NoError(s.T(), err) + + externalIDB := namegen.AppendRandomString("un-conflict-b-ext") + _, userIDB, err := scimactions.CreateSCIMUser(s.scimClient, subSession, externalIDB, true) + require.NoError(s.T(), err) + + resp, err := s.scimClient.Users().Update(userIDB, scimclient.User{ + Schemas: []string{scimclient.SCIMSchemaUser}, + UserName: userNameA, + ExternalID: externalIDB, + Active: scimclient.BoolPtr(true), + }) + require.NoError(s.T(), err) + require.NoError(s.T(), scimactions.CheckStatus(resp, http.StatusConflict, "PUT userName to another user's value should return 409")) +} + +func (s *SCIMOpenLDAPTestSuite) TestSCIMPrincipalSearchUserNotLocal() { + subSession := s.session.NewSession() + defer subSession.Cleanup() + + logrus.Info("Verifying a SCIM-provisioned user is not returned as a local principal by /v3/principals search") + + userName, _, err := scimactions.CreateSCIMUser(s.scimClient, subSession, "", true) + require.NoError(s.T(), err) + + require.NoError(s.T(), scimactions.VerifyPrincipalNotLocal(s.client, userName)) +} + +func (s *SCIMOpenLDAPTestSuite) TestSCIMPrincipalSearchGroupNotLocal() { + subSession := s.session.NewSession() + defer subSession.Cleanup() + + logrus.Info("Verifying a SCIM-provisioned group is not returned as a local principal by /v3/principals search") + + groupName, _, err := scimactions.CreateSCIMGroup(s.scimClient, subSession, "") + require.NoError(s.T(), err) + + require.NoError(s.T(), scimactions.VerifyPrincipalNotLocal(s.client, groupName)) +} + +func (s *SCIMOpenLDAPTestSuite) TestSCIMGetUserMissingUserAttributeReturns200() { + subSession := s.session.NewSession() + defer subSession.Cleanup() + + logrus.Info("Verifying GET /Users/{id} returns 200 for a SCIM user whose UserAttribute is absent (never logged in)") + + _, userID, err := scimactions.ProvisionSCIMUserWithoutAttribute(s.client, s.scimClient, subSession, "") + require.NoError(s.T(), err) + + resp, err := s.scimClient.Users().ByID(userID) + require.NoError(s.T(), err) + require.NoError(s.T(), scimactions.CheckStatus(resp, http.StatusOK, "GET /Users/{id} should return 200 even when the user has no UserAttribute")) +} + +func (s *SCIMOpenLDAPTestSuite) TestSCIMPutUserMissingUserAttributeReturns200() { + subSession := s.session.NewSession() + defer subSession.Cleanup() + + logrus.Info("Verifying PUT /Users/{id} returns 200 for a SCIM user whose UserAttribute is absent (externalId mode, userName mutable)") + + defer func() { require.NoError(s.T(), scimactions.RestoreSCIMBaseline(s.client, s.scimClient, authactions.OpenLdap)) }() + require.NoError(s.T(), scimactions.SetSCIMConfigMapAndWaitCreateReady(s.client, s.scimClient, authactions.OpenLdap, map[string]string{"enabled": "true", "userIdAttribute": "externalId"})) + + externalID := namegen.AppendRandomString("ext-put-recover") + _, userID, err := scimactions.ProvisionSCIMUserWithoutAttribute(s.client, s.scimClient, subSession, externalID) + require.NoError(s.T(), err) + + resp, err := s.scimClient.Users().Update(userID, scimclient.User{ + Schemas: []string{scimclient.SCIMSchemaUser}, + UserName: namegen.AppendRandomString("put-recovered"), + Active: scimclient.BoolPtr(true), + }) + require.NoError(s.T(), err) + require.NoError(s.T(), scimactions.CheckStatus(resp, http.StatusOK, "PUT /Users/{id} should return 200 even when the user has no UserAttribute")) +} + +func (s *SCIMOpenLDAPTestSuite) TestSCIMPatchUserMissingUserAttributeReturns200() { + subSession := s.session.NewSession() + defer subSession.Cleanup() + + logrus.Info("Verifying PATCH /Users/{id} returns 200 for a SCIM user whose UserAttribute is absent") + + _, userID, err := scimactions.ProvisionSCIMUserWithoutAttribute(s.client, s.scimClient, subSession, "") + require.NoError(s.T(), err) + + resp, err := scimactions.PatchUser(s.scimClient, userID, "replace", "displayName", namegen.AppendRandomString("patched")) + require.NoError(s.T(), err) + require.NoError(s.T(), scimactions.CheckStatus(resp, http.StatusOK, "PATCH /Users/{id} should return 200 even when the user has no UserAttribute")) +} + +func (s *SCIMOpenLDAPTestSuite) TestSCIMGroupAddMemberMissingUserAttribute() { + subSession := s.session.NewSession() + defer subSession.Cleanup() + + logrus.Info("Verifying PATCH group add member records membership when the member's UserAttribute is absent") + + _, userID, err := scimactions.ProvisionSCIMUserWithoutAttribute(s.client, s.scimClient, subSession, "") + require.NoError(s.T(), err) + + _, groupID, err := scimactions.CreateSCIMGroup(s.scimClient, subSession, "") + require.NoError(s.T(), err) + + patchResp, err := scimactions.PatchGroup(s.scimClient, groupID, "add", "members", []scimclient.Member{{Value: userID}}) + require.NoError(s.T(), err) + require.NoError(s.T(), scimactions.CheckStatus(patchResp, http.StatusOK, "PATCH group add member should return 200 when the member has no UserAttribute")) + + require.NoError(s.T(), scimactions.WaitForGroupMemberCount(s.scimClient, groupID, 1)) +} + +func (s *SCIMOpenLDAPTestSuite) TestSCIMPrincipalSearchLocalUserIsLocal() { + subSession := s.session.NewSession() + defer subSession.Cleanup() + + logrus.Info("Verifying a genuine local user is still returned as a local principal by /v3/principals search") + + adminUsers, err := s.client.Management.User.List(&types.ListOpts{ + Filters: map[string]interface{}{"username": "admin"}, + }) + require.NoError(s.T(), err) + require.NotEmpty(s.T(), adminUsers.Data, "default admin user should exist") + + require.NoError(s.T(), scimactions.VerifyPrincipalIsLocal(s.client, adminUsers.Data[0].Username)) +} + func TestSCIMOpenLDAPSuite(t *testing.T) { suite.Run(t, new(SCIMOpenLDAPTestSuite)) }