Redesign the implementation of "Invite Members" search
Resolving this issue should unblock the solution for https://gitlab.com/gitlab-org/gitlab/-/issues/424505.
Why? was initially described in !147732 (closed).
When we limited search result on Invite Members
to users with group's SAML or SCIM identity for groups with SSO enforcement is enabled, it caused https://gitlab.com/gitlab-org/gitlab/-/issues/424505. We need to unblock the solution to resolve that issue.
The policy which users can and cannot be added to members, as per the group settings, is subject to change along with developing GitLab application. The existing implementation is not designed for its extension. Any extension of the existing implementation we have done is a hack and could lead to bugs and even breaking change since we use API... Let me rationale below.
Since introducing Service Accounts, we changed policy to allow service account users to be added to members if SSO enforcement is enabled for the group. We allowed that on the backend side. However, to allow users to do so from UI, we had to change the search implementation. We needed to do this change quickly to allow customers that use SSO enforcement to use service accounts. But there was no any possible way to do this change correctly so we did a hack: We changed User.limit_to_saml_provider(saml_provider_id)
method and /api/v4/users?saml_provider_id=ID
API endpoint to return group's service accounts. This method name and API endpoint with saml_provider_id
emphasize that it should return only users with the group's SAML identity, however it now returns the group's service accounts users which has no relation to SAML at all. We should change that since it could lead to bugs in the future.
Then we discovered https://gitlab.com/gitlab-org/gitlab/-/issues/424505. To keep the existing UX for Invite Members
and to unblock the solution to resolve that issue we are about to add new API endpoints !134570 (merged) and !144796 (closed). At first glance they looked promising, but:
- Since the policy which users can and cannot be added to members, as per the group settings, is subject to change along with developing GitLab application, those endpoints are subject to change too. It breaks promises that any endpoint changes under
/api/v4/
should follow REST API deprecations and removals process if there is no security reason for that. I know we mentioned that those endpoints are for internal purposes in the docs, but It won't stop some users to start using it. And in case if any of this endpoint is changed in few month or years, we will break those customers workflows and it will lead to an incident because any change to an API endpoint is a breaking change. - Those endpoints looked promising and useful for users and for evolving, however after the following change !141040 (comment 1773138463), we made those endpoint available only when SSO enforcement is enabled. As a user, I find it very confusing that I cannot use neither
GET /groups/:id/users
norGET /projects/:id/users
endpoints for my groups that do not have SSO enforcement enabled. Those endpoints has no value for group that do not use SSO enforcement despite those endpoints names do not emphasize that they are supposed to work for SSO enforced groups only. I don't see any reason why those endpoints should only be allowed for use when SSO enforcement is enabled, at least we should change that.
Implementation plan
-
Close Draft: Add project's group users endpoint (!144796 - closed) and Draft: Use group/project ID for invite user modal (!144507 - closed) -
Revert Use group ID instead of root group ID for query (!142920 - merged) and Replace query to get SAML users and service acc... (!138075 - merged) - Reverted by !149815 (merged) and !150978 (merged)
-
Remove group_user_saml
FF: #434464 (closed)/chatops run feature delete group_user_saml --dev --pre --staging --staging-ref --production
-
Revert Allow subgroup owner to query SAML users (!141040 - merged) -
Revert Add params and ordering to group users API (!140241 - merged) -
Revert Add an API endpoint for Groups to get users rel... (!134570 - merged) -
Add internal endpoints, not in /api/v4/
namespace, but just as a simple Rails action/endpoint that is designed specifically for purposesInvite Members
search to keep the existing UX. In that endpoint check whether and how search result should be limited. That endpoint should be accessible to those users that can admin the resource members. We will be able to change this endpoint anytime we need along with changes to the the policy which users can and cannot be added to members and not be concerned that it could cause breaking change, like it was done for/api/v4/users?saml_provider_id=ID
API endpoint, or lead to ambiguous code and hacks like inUser.limit_to_saml_provider(saml_provider_id)
method. -
Partially revert Allow Service Account to be added to SSO-enforc... (!126059 - merged) -
Revert Filter suggested users by saml provider in invi... (!63565 - merged) that was related to https://gitlab.com/gitlab-org/gitlab/-/issues/333625