SONAR-21413 Fix SSF-530 - Add Groups in jit and auto cards

server/sonar-web/src/main/js/api/gitlab-provisioning.ts

@@ -44,6 +44,7 @@ export function createGitLabConfiguration(
   return axios.post(GITLAB_CONFIGURATIONS, {
     ...data,
     provisioningType: ProvisioningType.jit,
+    allowedGroups: [],
     allowUsersToSignUp: false,
     enabled: true,
   });

server/sonar-web/src/main/js/apps/settings/components/authentication/AuthenticationFormField.tsx

@@ -37,10 +37,11 @@ interface Props {
   onFieldChange: (key: string, value: string | boolean | string[]) => void;
   isNotSet: boolean;
   error?: string;
+  className?: string;
 }
 
 export default function AuthenticationFormField(props: Readonly<Props>) {
-  const { mandatory = false, definition, settingValue, isNotSet, error } = props;
+  const { mandatory = false, definition, settingValue, isNotSet, error, className } = props;
 
   const intl = useIntl();
 
@@ -75,6 +76,7 @@ export default function AuthenticationFormField(props: Readonly<Props>) {
 
   return (
     <FormField
+      className={className}
       htmlFor={definition.key}
       ariaLabel={name}
       label={name}

server/sonar-web/src/main/js/apps/settings/components/authentication/GitLabAuthenticationTab.tsx

@@ -50,22 +50,36 @@ interface ChangesForm {
   provisioningType?: GitLabConfigurationUpdateBody['provisioningType'];
   allowUsersToSignUp?: GitLabConfigurationUpdateBody['allowUsersToSignUp'];
   provisioningToken?: GitLabConfigurationUpdateBody['provisioningToken'];
+  allowedGroups?: GitLabConfigurationUpdateBody['allowedGroups'];
 }
 
-const definitions: Record<keyof Omit<ChangesForm, 'provisioningType'>, DefinitionV2> = {
-  allowUsersToSignUp: {
-    name: translate('settings.authentication.gitlab.form.allowUsersToSignUp.name'),
-    secured: false,
-    key: 'allowUsersToSignUp',
-    description: translate('settings.authentication.gitlab.form.allowUsersToSignUp.description'),
-    type: SettingType.BOOLEAN,
-  },
-  provisioningToken: {
-    name: translate('settings.authentication.gitlab.form.provisioningToken.name'),
-    secured: true,
-    key: 'provisioningToken',
-    description: translate('settings.authentication.gitlab.form.provisioningToken.description'),
-  },
+const getDefinitions = (
+  provisioningType: ProvisioningType,
+): Record<keyof Omit<ChangesForm, 'provisioningType'>, DefinitionV2> => {
+  return {
+    allowUsersToSignUp: {
+      name: translate('settings.authentication.gitlab.form.allowUsersToSignUp.name'),
+      secured: false,
+      key: 'allowUsersToSignUp',
+      description: translate('settings.authentication.gitlab.form.allowUsersToSignUp.description'),
+      type: SettingType.BOOLEAN,
+    },
+    provisioningToken: {
+      name: translate('settings.authentication.gitlab.form.provisioningToken.name'),
+      secured: true,
+      key: 'provisioningToken',
+      description: translate('settings.authentication.gitlab.form.provisioningToken.description'),
+    },
+    allowedGroups: {
+      name: translate('settings.authentication.gitlab.form.allowedGroups.name'),
+      secured: false,
+      key: 'allowedGroups',
+      description: translate(
+        `settings.authentication.gitlab.form.allowedGroups.description.${provisioningType}`,
+      ),
+      multiValues: true,
+    },
+  };
 };
 
 export default function GitLabAuthenticationTab() {
@@ -92,6 +106,9 @@ export default function GitLabAuthenticationTab() {
   const { mutate: updateConfig, isLoading: isUpdating } = useUpdateGitLabConfigurationMutation();
   const { mutate: deleteConfig, isLoading: isDeleting } = useDeleteGitLabConfigurationMutation();
 
+  const definitions = getDefinitions(
+    changes?.provisioningType ?? configuration?.provisioningType ?? ProvisioningType.jit,
+  );
   const toggleEnable = () => {
     if (!configuration) {
       return;
@@ -134,22 +151,26 @@ export default function GitLabAuthenticationTab() {
   const setJIT = () =>
     setChangesWithCheck({
       provisioningType: ProvisioningType.jit,
+      allowedGroups: changes?.allowedGroups,
       provisioningToken: undefined,
     });
 
   const setAuto = () =>
     setChangesWithCheck({
       provisioningType: ProvisioningType.auto,
+      allowedGroups: changes?.allowedGroups,
       allowUsersToSignUp: undefined,
     });
 
   const hasDifferentProvider =
     identityProvider?.provider !== undefined && identityProvider.provider !== Provider.Gitlab;
   const allowUsersToSignUpDefinition = definitions.allowUsersToSignUp;
+  const allowedGroupsDefinition = definitions.allowedGroups;
   const provisioningTokenDefinition = definitions.provisioningToken;
 
   const provisioningType = changes?.provisioningType ?? configuration?.provisioningType;
   const allowUsersToSignUp = changes?.allowUsersToSignUp ?? configuration?.allowUsersToSignUp;
+  const allowedGroups = changes?.allowedGroups ?? configuration?.allowedGroups;
   const provisioningToken = changes?.provisioningToken;
 
   const canSave = () => {
@@ -158,7 +179,12 @@ export default function GitLabAuthenticationTab() {
     }
     const type = changes.provisioningType ?? configuration.provisioningType;
     if (type === ProvisioningType.auto) {
-      return configuration.isProvisioningTokenSet || !!changes.provisioningToken;
+      const areGroupsDefined =
+        changes.allowedGroups?.some((val) => val !== '') ??
+        configuration.allowedGroups?.some((val) => val !== '');
+      return (
+        (configuration.isProvisioningTokenSet || !!changes.provisioningToken) && areGroupsDefined
+      );
     }
     return true;
   };
@@ -173,6 +199,10 @@ export default function GitLabAuthenticationTab() {
         configuration?.allowUsersToSignUp === newChanges.allowUsersToSignUp
           ? undefined
           : newChanges.allowUsersToSignUp,
+      allowedGroups:
+        configuration?.allowedGroups === newChanges.allowedGroups
+          ? undefined
+          : newChanges.allowedGroups,
       provisioningToken: newChanges.provisioningToken,
     };
     if (Object.values(newValue).some((v) => v !== undefined)) {
@@ -220,6 +250,7 @@ export default function GitLabAuthenticationTab() {
               onToggle={toggleEnable}
             />
             <ProvisioningSection
+              isLoading={isUpdating}
               provisioningType={provisioningType ?? ProvisioningType.jit}
               onChangeProvisioningType={(val: ProvisioningType) =>
                 val === ProvisioningType.auto ? setAuto() : setJIT()
@@ -251,18 +282,32 @@ export default function GitLabAuthenticationTab() {
                 />
               }
               jitSettings={
-                <AuthenticationFormField
-                  settingValue={allowUsersToSignUp}
-                  definition={allowUsersToSignUpDefinition}
-                  mandatory
-                  onFieldChange={(_, value) =>
-                    setChangesWithCheck({
-                      ...changes,
-                      allowUsersToSignUp: value as boolean,
-                    })
-                  }
-                  isNotSet={configuration.provisioningType !== ProvisioningType.auto}
-                />
+                <>
+                  <AuthenticationFormField
+                    settingValue={allowUsersToSignUp}
+                    definition={allowUsersToSignUpDefinition}
+                    mandatory
+                    onFieldChange={(_, value) =>
+                      setChangesWithCheck({
+                        ...changes,
+                        allowUsersToSignUp: value as boolean,
+                      })
+                    }
+                    isNotSet={configuration.provisioningType !== ProvisioningType.auto}
+                  />
+                  <AuthenticationFormField
+                    className="sw-mt-8"
+                    settingValue={allowedGroups}
+                    definition={allowedGroupsDefinition}
+                    onFieldChange={(_, values) =>
+                      setChangesWithCheck({
+                        ...changes,
+                        allowedGroups: values as string[],
+                      })
+                    }
+                    isNotSet={configuration.provisioningType !== ProvisioningType.auto}
+                  />
+                </>
               }
               autoTitle={translate('settings.authentication.gitlab.form.provisioning_with_gitlab')}
               hasDifferentProvider={hasDifferentProvider}
@@ -302,19 +347,34 @@ export default function GitLabAuthenticationTab() {
               canSync={canSyncNow}
               synchronizationDetails={<GitLabSynchronisationWarning />}
               autoSettings={
-                <AuthenticationFormField
-                  settingValue={provisioningToken}
-                  key={tokenKey}
-                  definition={provisioningTokenDefinition}
-                  mandatory
-                  onFieldChange={(_, value) =>
-                    setChangesWithCheck({
-                      ...changes,
-                      provisioningToken: value as string,
-                    })
-                  }
-                  isNotSet={!configuration.isProvisioningTokenSet}
-                />
+                <>
+                  <AuthenticationFormField
+                    settingValue={provisioningToken}
+                    key={tokenKey}
+                    definition={provisioningTokenDefinition}
+                    mandatory
+                    onFieldChange={(_, value) =>
+                      setChangesWithCheck({
+                        ...changes,
+                        provisioningToken: value as string,
+                      })
+                    }
+                    isNotSet={!configuration.isProvisioningTokenSet}
+                  />
+                  <AuthenticationFormField
+                    className="sw-mt-8"
+                    settingValue={allowedGroups}
+                    definition={allowedGroupsDefinition}
+                    mandatory
+                    onFieldChange={(_, values) =>
+                      setChangesWithCheck({
+                        ...changes,
+                        allowedGroups: values as string[],
+                      })
+                    }
+                    isNotSet={configuration.provisioningType !== ProvisioningType.auto}
+                  />
+                </>
               }
             />
           </>

server/sonar-web/src/main/js/apps/settings/components/authentication/GitLabConfigurationForm.tsx

@@ -19,7 +19,7 @@
  */
 
 import { ButtonPrimary, FlagMessage, Modal, Spinner } from 'design-system';
-import { isArray, keyBy } from 'lodash';
+import { keyBy } from 'lodash';
 import * as React from 'react';
 import { FormattedMessage } from 'react-intl';
 import DocumentationLink from '../../../../components/common/DocumentationLink';
@@ -102,27 +102,12 @@ export default function GitLabConfigurationForm(props: Readonly<Props>) {
         type: SettingType.BOOLEAN,
       },
     },
-    allowedGroups: {
-      value: data?.allowedGroups ?? [],
-      required: true,
-      definition: {
-        name: translate('settings.authentication.gitlab.form.allowedGroups.name'),
-        secured: false,
-        key: 'allowedGroups',
-        description: translate('settings.authentication.gitlab.form.allowedGroups.description'),
-        multiValues: true,
-      },
-    },
   });
 
   const header = translate('settings.authentication.gitlab.form', isCreate ? 'create' : 'edit');
 
   const canBeSaved = Object.values(formData).every(({ definition, required, value }) => {
-    return (
-      (!isCreate && definition.secured) ||
-      !required ||
-      (isArray(value) ? value.some((val) => val !== '') : value !== '')
-    );
+    return (!isCreate && definition.secured) || !required || value !== '';
   });
 
   const handleSubmit = (event: React.SyntheticEvent<HTMLFormElement>) => {

server/sonar-web/src/main/js/apps/settings/components/authentication/ProvisioningSection.tsx

@@ -23,6 +23,7 @@ import {
   ButtonSecondary,
   FlagMessage,
   RadioButton,
+  Spinner,
   SubHeading,
 } from 'design-system';
 import React, { FormEvent, ReactElement } from 'react';
@@ -30,6 +31,7 @@ import { translate } from '../../../../helpers/l10n';
 import { ProvisioningType } from '../../../../types/provisioning';
 
 interface Props {
+  isLoading?: boolean;
   provisioningType: ProvisioningType;
   onChangeProvisioningType: (val: ProvisioningType) => void;
   disabledConfigText: string;
@@ -54,6 +56,7 @@ interface Props {
 
 export default function ProvisioningSection(props: Readonly<Props>) {
   const {
+    isLoading,
     provisioningType,
     jitTitle,
     jitDescription,
@@ -151,8 +154,10 @@ export default function ProvisioningSection(props: Readonly<Props>) {
               <ButtonSecondary onClick={onCancel} disabled={!hasUnsavedChanges}>
                 {translate('cancel')}
               </ButtonSecondary>
+              <Spinner loading={!!isLoading} />
               <FlagMessage variant="warning" className="sw-mb-0">
                 {hasUnsavedChanges &&
+                  !isLoading &&
                   translate('settings.authentication.github.configuration.unsaved_changes')}
               </FlagMessage>
             </div>

server/sonar-web/src/main/js/apps/settings/components/authentication/__tests__/Authentication-Gitlab-it.tsx

@@ -154,8 +154,6 @@ it('should create a Gitlab configuration and disable it with proper validation',
   expect(ui.saveConfigButton.get()).toBeDisabled();
   await user.type(ui.url.get(), 'https://company.ui.com');
   await user.type(ui.secret.get(), '123');
-  expect(ui.saveConfigButton.get()).toBeDisabled();
-  await user.type(ui.groups.get(), 'NWA');
   expect(ui.saveConfigButton.get()).toBeEnabled();
   await user.click(ui.synchronizeGroups.get());
   await user.click(ui.saveConfigButton.get());
@@ -185,7 +183,6 @@ it('should edit a configuration with proper validation and delete it', async ()
   expect(ui.url.get()).toHaveValue('URL');
   expect(ui.applicationId.get()).toBeInTheDocument();
   expect(ui.secret.query()).not.toBeInTheDocument();
-  expect(ui.groups.get()).toHaveValue('Cypress Hill');
   expect(ui.synchronizeGroups.get()).toBeChecked();
 
   expect(ui.applicationId.get()).toBeInTheDocument();
@@ -199,15 +196,6 @@ it('should edit a configuration with proper validation and delete it', async ()
   expect(ui.saveConfigButton.get()).toBeDisabled();
   await user.type(ui.url.get(), 'www.internet.com');
   expect(ui.saveConfigButton.get()).toBeEnabled();
-
-  expect(ui.groups.get()).toHaveValue('Cypress Hill');
-  await user.click(ui.groups.get());
-  await user.click(ui.deleteGroupButton.get());
-  expect(ui.groups.get()).not.toHaveValue('Cypress Hill');
-  expect(ui.saveConfigButton.get()).toBeDisabled();
-  await user.click(ui.groups.get());
-  await user.type(ui.groups.get(), 'Run DMC');
-  expect(ui.saveConfigButton.get()).toBeEnabled();
   await user.click(ui.saveConfigButton.get());
 
   expect(glContainer.get()).not.toHaveTextContent('URL');
@@ -222,18 +210,68 @@ it('should edit a configuration with proper validation and delete it', async ()
   expect(ui.editConfigButton.query()).not.toBeInTheDocument();
 });
 
-it('should change from just-in-time to Auto Provisioning if auto was never set', async () => {
+it('should be able to save just-in-time with no organizations', async () => {
+  const user = userEvent.setup();
+  renderAuthentication([Feature.GitlabProvisioning]);
+
+  expect(await ui.jitProvisioningRadioButton.find()).toBeChecked();
+
+  expect(ui.groups.get()).toHaveValue('Cypress Hill');
+  expect(await ui.saveProvisioning.find()).toBeDisabled();
+  await user.click(ui.deleteGroupButton.get());
+  expect(await ui.saveProvisioning.find()).toBeEnabled();
+});
+
+it('should not be able to save Auto provisioning with no organizations', async () => {
+  const user = userEvent.setup();
+  handler.setGitlabConfigurations([
+    mockGitlabConfiguration({
+      allowUsersToSignUp: false,
+      enabled: true,
+      provisioningType: ProvisioningType.auto,
+      allowedGroups: ['D12'],
+      isProvisioningTokenSet: true,
+    }),
+  ]);
+  renderAuthentication([Feature.GitlabProvisioning]);
+
+  expect(await ui.autoProvisioningRadioButton.find()).toBeChecked();
+
+  expect(ui.groups.get()).toHaveValue('D12');
+  expect(ui.saveProvisioning.get()).toBeDisabled();
+  await user.click(ui.deleteGroupButton.get());
+  expect(await ui.saveProvisioning.find()).toBeDisabled();
+});
+
+it('should change from just-in-time to Auto Provisioning if auto was never set before', async () => {
   const user = userEvent.setup();
+  handler.setGitlabConfigurations([
+    mockGitlabConfiguration({
+      allowUsersToSignUp: false,
+      enabled: true,
+      provisioningType: ProvisioningType.jit,
+      allowedGroups: [],
+      isProvisioningTokenSet: false,
+    }),
+  ]);
   renderAuthentication([Feature.GitlabProvisioning]);
 
   expect(await ui.editConfigButton.find()).toBeInTheDocument();
   expect(ui.jitProvisioningRadioButton.get()).toBeChecked();
 
-  user.click(ui.autoProvisioningRadioButton.get());
+  await user.click(ui.autoProvisioningRadioButton.get());
   expect(await ui.autoProvisioningRadioButton.find()).toBeEnabled();
   expect(ui.saveProvisioning.get()).toBeDisabled();
 
   await user.type(ui.autoProvisioningToken.get(), 'JRR Tolkien');
+  expect(await ui.saveProvisioning.find()).toBeDisabled();
+
+  await user.type(ui.groups.get(), 'Run DMC');
+  expect(await ui.saveProvisioning.find()).toBeEnabled();
+  await user.click(ui.deleteGroupButton.get());
+  expect(await ui.saveProvisioning.find()).toBeDisabled();
+
+  await user.type(ui.groups.get(), 'Public Enemy');
   expect(await ui.saveProvisioning.find()).toBeEnabled();
 });
 
@@ -255,6 +293,13 @@ it('should change from just-in-time to Auto Provisioning if auto was set before'
 
   user.click(ui.autoProvisioningRadioButton.get());
   expect(await ui.autoProvisioningRadioButton.find()).toBeEnabled();
+  expect(await ui.saveProvisioning.find()).toBeEnabled();
+
+  expect(ui.groups.get()).toHaveValue('D12');
+  await user.click(ui.deleteGroupButton.get());
+  expect(await ui.saveProvisioning.find()).toBeDisabled();
+  await user.type(ui.groups.get(), 'Wu Tang Clan');
+
   expect(ui.saveProvisioning.get()).toBeEnabled();
 });
 

server/sonar-web/src/main/js/types/provisioning.ts

@@ -97,7 +97,6 @@ export interface GitLabConfigurationCreateBody {
   url: string;
   secret: string;
   synchronizeGroups: boolean;
-  allowedGroups: string[];
 }
 
 export type GitLabConfigurationUpdateBody = {

sonar-core/src/main/resources/org/sonar/l10n/core.properties

@@ -1592,7 +1592,8 @@ settings.authentication.gitlab.form.secret.description=Secret provided by GitLab
 settings.authentication.gitlab.form.synchronizeGroups.name=Synchronize user groups
 settings.authentication.gitlab.form.synchronizeGroups.description=For each GitLab group they belong to, the user will be associated to a group with the same name (if it exists) in SonarQube. If enabled, the GitLab OAuth 2 application will need to provide the api scope.
 settings.authentication.gitlab.form.allowedGroups.name=Allowed groups
-settings.authentication.gitlab.form.allowedGroups.description=Only members of these groups (and sub-groups) will be allowed to authenticate. Please enter the group slug as it appears in the GitLab URL, for instance `my-gitlab-group`. If you use Auto-provisioning, only members of these groups (and sub-groups) will be provisioned.
+settings.authentication.gitlab.form.allowedGroups.description.JIT=Only members of these groups (and sub-groups) will be allowed to authenticate. Please enter the group slug as it appears in the GitLab URL, for instance `my-gitlab-group`. ⚠︎ if not set and `Allow users to sign up` is enabled, any user from GitLab will be able to login to this SonarQube instance.
+settings.authentication.gitlab.form.allowedGroups.description.AUTO_PROVISIONING=Only members of these groups (and sub-groups) will be provisioned. Please enter the group slug as it appears in the GitLab URL, for instance `my-gitlab-group`.
 settings.authentication.gitlab.form.allowUsersToSignUp.name=Allow users to sign up
 settings.authentication.gitlab.form.allowUsersToSignUp.description=Allow new users to authenticate. When set to disabled, only existing users will be able to authenticate to the server.
 settings.authentication.gitlab.form.provisioningToken.name=Provisioning token