]> source.dussan.org Git - sonarqube.git/commitdiff
SONAR-21088 Show warning if GitHub config is insecure
authorViktor Vorona <viktor.vorona@sonarsource.com>
Tue, 30 Jan 2024 16:30:51 +0000 (17:30 +0100)
committersonartech <sonartech@sonarsource.com>
Wed, 31 Jan 2024 20:03:38 +0000 (20:03 +0000)
server/sonar-web/src/main/js/apps/settings/components/authentication/ConfigurationForm.tsx
server/sonar-web/src/main/js/apps/settings/components/authentication/GitHubConfirmModal.tsx [new file with mode: 0644]
server/sonar-web/src/main/js/apps/settings/components/authentication/GithubAuthenticationTab.tsx
server/sonar-web/src/main/js/apps/settings/components/authentication/__tests__/Authentication-Github-it.tsx
server/sonar-web/src/main/js/apps/settings/components/authentication/hook/useGithubConfiguration.ts
sonar-core/src/main/resources/org/sonar/l10n/core.properties

index fdd9db6cf0ea2d136cc7c3bc7077425dc6dac3e0..9a7d6b545f89a3b91b55d984da26d1ec3b597e94 100644 (file)
@@ -24,10 +24,14 @@ import { FormattedMessage } from 'react-intl';
 import DocumentationLink from '../../../../components/common/DocumentationLink';
 import { translate } from '../../../../helpers/l10n';
 import { useSaveValuesMutation } from '../../../../queries/settings';
+import { AlmKeys } from '../../../../types/alm-settings';
+import { ProvisioningType } from '../../../../types/provisioning';
 import { Dict } from '../../../../types/types';
 import { AuthenticationTabs, DOCUMENTATION_LINK_SUFFIXES } from './Authentication';
 import AuthenticationFormField from './AuthenticationFormField';
+import GitHubConfirmModal from './GitHubConfirmModal';
 import { SettingValue } from './hook/useConfiguration';
+import { isAllowToSignUpEnabled, isOrganizationListEmpty } from './hook/useGithubConfiguration';
 
 interface Props {
   create: boolean;
@@ -39,6 +43,7 @@ interface Props {
   tab: AuthenticationTabs;
   excludedField: string[];
   hasLegacyConfiguration?: boolean;
+  provisioningStatus?: ProvisioningType;
 }
 
 interface ErrorValue {
@@ -46,7 +51,7 @@ interface ErrorValue {
   message: string;
 }
 
-export default function ConfigurationForm(props: Props) {
+export default function ConfigurationForm(props: Readonly<Props>) {
   const {
     create,
     loading,
@@ -56,8 +61,10 @@ export default function ConfigurationForm(props: Props) {
     tab,
     excludedField,
     hasLegacyConfiguration,
+    provisioningStatus,
   } = props;
   const [errors, setErrors] = React.useState<Dict<ErrorValue>>({});
+  const [showConfirmModal, setShowConfirmModal] = React.useState(false);
 
   const { mutateAsync: changeConfig } = useSaveValuesMutation();
 
@@ -67,15 +74,14 @@ export default function ConfigurationForm(props: Props) {
     event.preventDefault();
 
     if (canBeSave) {
-      const data = await changeConfig(Object.values(values));
-      const errors = data
-        .filter(({ success }) => !success)
-        .map(({ key }) => ({ key, message: translate('default_save_field_error_message') }));
-
-      setErrors(keyBy(errors, 'key'));
-
-      if (errors.length === 0) {
-        props.onClose();
+      if (
+        tab === AlmKeys.GitHub &&
+        isOrganizationListEmpty(values) &&
+        (provisioningStatus === ProvisioningType.auto || isAllowToSignUpEnabled(values))
+      ) {
+        setShowConfirmModal(true);
+      } else {
+        await onSave();
       }
     } else {
       const errors = Object.values(values)
@@ -85,6 +91,19 @@ export default function ConfigurationForm(props: Props) {
     }
   };
 
+  const onSave = async () => {
+    const data = await changeConfig(Object.values(values));
+    const errors = data
+      .filter(({ success }) => !success)
+      .map(({ key }) => ({ key, message: translate('default_save_field_error_message') }));
+
+    setErrors(keyBy(errors, 'key'));
+
+    if (errors.length === 0) {
+      props.onClose();
+    }
+  };
+
   const helpMessage = hasLegacyConfiguration ? `legacy_help.${tab}` : 'help';
 
   const FORM_ID = 'configuration-form';
@@ -136,17 +155,27 @@ export default function ConfigurationForm(props: Props) {
   );
 
   return (
-    <Modal
-      headerTitle={header}
-      isScrollable
-      onClose={props.onClose}
-      body={formBody}
-      primaryButton={
-        <ButtonPrimary form={FORM_ID} type="submit" autoFocus disabled={!canBeSave}>
-          {translate('settings.almintegration.form.save')}
-          <Spinner className="sw-ml-2" loading={loading} />
-        </ButtonPrimary>
-      }
-    />
+    <>
+      <Modal
+        headerTitle={header}
+        isScrollable
+        onClose={props.onClose}
+        body={formBody}
+        primaryButton={
+          <ButtonPrimary form={FORM_ID} type="submit" autoFocus disabled={!canBeSave}>
+            {translate('settings.almintegration.form.save')}
+            <Spinner className="sw-ml-2" loading={loading} />
+          </ButtonPrimary>
+        }
+      />
+      {showConfirmModal && (
+        <GitHubConfirmModal
+          onConfirm={onSave}
+          onClose={() => setShowConfirmModal(false)}
+          values={values}
+          provisioningStatus={provisioningStatus ?? ProvisioningType.jit}
+        />
+      )}
+    </>
   );
 }
diff --git a/server/sonar-web/src/main/js/apps/settings/components/authentication/GitHubConfirmModal.tsx b/server/sonar-web/src/main/js/apps/settings/components/authentication/GitHubConfirmModal.tsx
new file mode 100644 (file)
index 0000000..41a55f6
--- /dev/null
@@ -0,0 +1,64 @@
+/*
+ * SonarQube
+ * Copyright (C) 2009-2024 SonarSource SA
+ * mailto:info AT sonarsource DOT com
+ *
+ * This program is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU Lesser General Public
+ * License as published by the Free Software Foundation; either
+ * version 3 of the License, or (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
+ * Lesser General Public License for more details.
+ *
+ * You should have received a copy of the GNU Lesser General Public License
+ * along with this program; if not, write to the Free Software Foundation,
+ * Inc., 51 Franklin Street, Fifth Floor, Boston, MA  02110-1301, USA.
+ */
+import { FlagMessage } from 'design-system';
+import React from 'react';
+import ConfirmModal from '../../../../components/controls/ConfirmModal';
+import { translate } from '../../../../helpers/l10n';
+import { ProvisioningType } from '../../../../types/provisioning';
+import { SettingValue } from './hook/useConfiguration';
+import { isAllowToSignUpEnabled, isOrganizationListEmpty } from './hook/useGithubConfiguration';
+
+interface GithubAuthenticationProps {
+  onConfirm: () => void;
+  onClose: () => void;
+  values: Record<string, SettingValue>;
+  hasGithubProvisioningTypeChange?: boolean;
+  provisioningStatus: ProvisioningType;
+}
+
+export default function GitHubConfirmModal(props: Readonly<GithubAuthenticationProps>) {
+  const { onConfirm, onClose, hasGithubProvisioningTypeChange, provisioningStatus, values } = props;
+
+  const organizationListIsEmpty = isOrganizationListEmpty(values);
+  const allowToSignUpEnabled = isAllowToSignUpEnabled(values);
+
+  return (
+    <ConfirmModal
+      onConfirm={onConfirm}
+      header={translate(
+        'settings.authentication.github.confirm',
+        hasGithubProvisioningTypeChange ? provisioningStatus : 'insecure',
+      )}
+      onClose={onClose}
+      confirmButtonText={translate(
+        'settings.authentication.github.provisioning_change.confirm_changes',
+      )}
+    >
+      {hasGithubProvisioningTypeChange &&
+        translate('settings.authentication.github.confirm', provisioningStatus, 'description')}
+      {(provisioningStatus === ProvisioningType.auto || allowToSignUpEnabled) &&
+        organizationListIsEmpty && (
+          <FlagMessage className="sw-mt-2" variant="warning">
+            {translate('settings.authentication.github.provisioning_change.insecure_config')}
+          </FlagMessage>
+        )}
+    </ConfirmModal>
+  );
+}
index 287ef88a7e6dd9a4d0cf70547b4691d70a24ea22..160285095023ac18348878315ec76b9b8579b5a9 100644 (file)
@@ -22,7 +22,6 @@ import React, { FormEvent, useState } from 'react';
 import { FormattedMessage } from 'react-intl';
 import GitHubSynchronisationWarning from '../../../../app/components/GitHubSynchronisationWarning';
 import DocumentationLink from '../../../../components/common/DocumentationLink';
-import ConfirmModal from '../../../../components/controls/ConfirmModal';
 import { translate, translateWithParameters } from '../../../../helpers/l10n';
 import { useIdentityProviderQuery } from '../../../../queries/identity-provider/common';
 import {
@@ -39,6 +38,7 @@ import AutoProvisioningConsent from './AutoProvisionningConsent';
 import ConfigurationDetails from './ConfigurationDetails';
 import ConfigurationForm from './ConfigurationForm';
 import GitHubConfigurationValidity from './GitHubConfigurationValidity';
+import GitHubConfirmModal from './GitHubConfirmModal';
 import GitHubMappingModal from './GitHubMappingModal';
 import ProvisioningSection from './ProvisioningSection';
 import TabHeader from './TabHeader';
@@ -46,6 +46,8 @@ import useGithubConfiguration, {
   GITHUB_ADDITIONAL_FIELDS,
   GITHUB_JIT_FIELDS,
   GITHUB_PROVISIONING_FIELDS,
+  isAllowToSignUpEnabled,
+  isOrganizationListEmpty,
 } from './hook/useGithubConfiguration';
 
 interface GithubAuthenticationProps {
@@ -87,6 +89,11 @@ export default function GithubAuthenticationTab(props: GithubAuthenticationProps
     deleteMutation: { isLoading: isDeleting, mutate: deleteConfiguration },
   } = useGithubConfiguration(definitions);
 
+  const provisioningStatus =
+    newGithubProvisioningStatus ?? githubProvisioningStatus
+      ? ProvisioningType.auto
+      : ProvisioningType.jit;
+
   const hasDifferentProvider = data?.provider !== undefined && data.provider !== Provider.Github;
   const { canSyncNow, synchronizeNow } = useSyncWithGitHubNow();
   const { refetch } = useCheckGitHubConfigQuery(enabled);
@@ -102,7 +109,11 @@ export default function GithubAuthenticationTab(props: GithubAuthenticationProps
 
   const handleSubmit = (e: FormEvent) => {
     e.preventDefault();
-    if (hasGithubProvisioningTypeChange) {
+    if (
+      hasGithubProvisioningTypeChange ||
+      (isOrganizationListEmpty(values) &&
+        (isAllowToSignUpEnabled(values) || provisioningStatus === ProvisioningType.auto))
+    ) {
       setShowConfirmProvisioningModal(true);
     } else {
       applyAdditionalOptions();
@@ -298,23 +309,13 @@ export default function GithubAuthenticationTab(props: GithubAuthenticationProps
               }
             />
             {showConfirmProvisioningModal && (
-              <ConfirmModal
+              <GitHubConfirmModal
                 onConfirm={() => changeProvisioning()}
-                header={translate(
-                  'settings.authentication.github.confirm',
-                  newGithubProvisioningStatus ? 'auto' : 'jit',
-                )}
                 onClose={() => setShowConfirmProvisioningModal(false)}
-                confirmButtonText={translate(
-                  'settings.authentication.github.provisioning_change.confirm_changes',
-                )}
-              >
-                {translate(
-                  'settings.authentication.github.confirm',
-                  newGithubProvisioningStatus ? 'auto' : 'jit',
-                  'description',
-                )}
-              </ConfirmModal>
+                values={values}
+                hasGithubProvisioningTypeChange={hasGithubProvisioningTypeChange}
+                provisioningStatus={provisioningStatus}
+              />
             )}
             {showMappingModal && (
               <GitHubMappingModal
@@ -337,6 +338,7 @@ export default function GithubAuthenticationTab(props: GithubAuthenticationProps
             onClose={handleCloseConfiguration}
             create={!hasConfiguration}
             hasLegacyConfiguration={hasLegacyConfiguration}
+            provisioningStatus={provisioningStatus}
           />
         )}
 
index 759a8d3c8647765f5c5d9e3cf8ab8cc385441775..0c113f2346e1af83a1da0776f65129994fde24a7 100644 (file)
@@ -64,6 +64,7 @@ const ui = {
   textbox1: byRole('textbox', { name: 'test1' }),
   textbox2: byRole('textbox', { name: 'test2' }),
   tab: byRole('tab', { name: 'github GitHub' }),
+  cancelDialogButton: byRole('dialog').byRole('button', { name: 'cancel' }),
   noGithubConfiguration: byText('settings.authentication.github.form.not_configured'),
   createConfigButton: ghContainer.byRole('button', {
     name: 'settings.authentication.form.create',
@@ -83,6 +84,9 @@ const ui = {
   allowUsersToSignUp: byRole('switch', {
     name: 'property.sonar.auth.github.allowUsersToSignUp.name',
   }),
+  projectVisibility: byRole('switch', {
+    name: 'property.provisioning.github.project.visibility.enabled.name',
+  }),
   organizations: byRole('textbox', {
     name: 'property.sonar.auth.github.organizations.name',
   }),
@@ -137,6 +141,9 @@ const ui = {
       name: `settings.definition.delete_value.property.sonar.auth.github.organizations.name.${org}`,
     }),
   enableFirstMessage: ghContainer.byText('settings.authentication.github.enable_first'),
+  insecureConfigWarning: byRole('dialog').byText(
+    'settings.authentication.github.provisioning_change.insecure_config',
+  ),
   jitProvisioningButton: ghContainer.byRole('radio', {
     name: /settings.authentication.form.provisioning_at_login/,
   }),
@@ -838,6 +845,7 @@ describe('Github tab', () => {
       expect(await ui.saveGithubProvisioning.find()).toBeEnabled();
 
       await user.click(ui.saveGithubProvisioning.get());
+      await user.click(ui.confirmProvisioningButton.get());
 
       // Clean local mapping state
       await user.click(ui.jitProvisioningButton.get());
@@ -917,6 +925,8 @@ describe('Github tab', () => {
       expect(await ui.saveGithubProvisioning.find()).toBeEnabled();
       await user.click(ui.saveGithubProvisioning.get());
 
+      await user.click(ui.confirmProvisioningButton.get());
+
       // Clean local mapping state
       await user.click(ui.jitProvisioningButton.get());
       await user.click(ui.githubProvisioningButton.get());
@@ -945,6 +955,60 @@ describe('Github tab', () => {
       expect(custom3Checkboxes[5]).not.toBeChecked();
       await user.click(ui.mappingDialogClose.get());
     });
+
+    it('should should show insecure config warning', async () => {
+      const user = userEvent.setup();
+      settingsHandler.presetGithubAutoProvisioning();
+      renderAuthentication([Feature.GithubProvisioning]);
+      await user.click(await ui.tab.find());
+
+      expect(ui.allowUsersToSignUp.get()).toBeChecked();
+      await user.click(ui.allowUsersToSignUp.get());
+      await user.click(ui.saveGithubProvisioning.get());
+
+      expect(ui.insecureConfigWarning.query()).not.toBeInTheDocument();
+
+      await user.click(ui.allowUsersToSignUp.get());
+      await user.click(ui.saveGithubProvisioning.get());
+
+      expect(ui.insecureConfigWarning.get()).toBeInTheDocument();
+      await user.click(ui.confirmProvisioningButton.get());
+
+      await user.click(ui.githubProvisioningButton.get());
+      await user.click(ui.saveGithubProvisioning.get());
+
+      expect(ui.insecureConfigWarning.get()).toBeInTheDocument();
+      await user.click(ui.confirmProvisioningButton.get());
+
+      await user.click(ui.projectVisibility.get());
+      await user.click(ui.saveGithubProvisioning.get());
+
+      expect(ui.insecureConfigWarning.get()).toBeInTheDocument();
+      await user.click(ui.confirmProvisioningButton.get());
+
+      await user.click(ui.editConfigButton.get());
+      await user.click(ui.saveConfigButton.get());
+
+      expect(ui.insecureConfigWarning.get()).toBeInTheDocument();
+      await user.click(ui.cancelDialogButton.get());
+      await user.type(ui.organizations.get(), '123');
+      await user.click(ui.saveConfigButton.get());
+      expect(ui.insecureConfigWarning.query()).not.toBeInTheDocument();
+
+      await user.click(ui.projectVisibility.get());
+      await user.click(ui.saveGithubProvisioning.get());
+      expect(ui.insecureConfigWarning.query()).not.toBeInTheDocument();
+
+      await user.click(ui.jitProvisioningButton.get());
+      await user.click(ui.saveGithubProvisioning.get());
+      expect(ui.confirmProvisioningButton.get()).toBeInTheDocument();
+      expect(ui.insecureConfigWarning.query()).not.toBeInTheDocument();
+      await user.click(ui.confirmProvisioningButton.get());
+
+      await user.click(ui.allowUsersToSignUp.get());
+      await user.click(ui.saveGithubProvisioning.get());
+      expect(ui.insecureConfigWarning.query()).not.toBeInTheDocument();
+    });
   });
 });
 
index 080197b154e0ec296391187eb1faae786a023ee1..6890c9c8e0c38939b71c4dc4c0e78c578dddf9c5 100644 (file)
@@ -29,20 +29,22 @@ import { useSaveValueMutation, useSaveValuesMutation } from '../../../../../quer
 import { Feature } from '../../../../../types/features';
 import { GitHubMapping } from '../../../../../types/provisioning';
 import { ExtendedSettingDefinition } from '../../../../../types/settings';
-import useConfiguration from './useConfiguration';
+import useConfiguration, { SettingValue } from './useConfiguration';
 
 export const GITHUB_ENABLED_FIELD = 'sonar.auth.github.enabled';
 export const GITHUB_APP_ID_FIELD = 'sonar.auth.github.appId';
 export const GITHUB_API_URL_FIELD = 'sonar.auth.github.apiUrl';
 export const GITHUB_CLIENT_ID_FIELD = 'sonar.auth.github.clientId.secured';
-export const GITHUB_JIT_FIELDS = ['sonar.auth.github.allowUsersToSignUp'];
+export const GITHUB_ALLOW_TO_SIGN_UP_FIELD = 'sonar.auth.github.allowUsersToSignUp';
+export const GITHUB_ORGANIZATIONS_LIST = 'sonar.auth.github.organizations';
+export const GITHUB_JIT_FIELDS = [GITHUB_ALLOW_TO_SIGN_UP_FIELD];
 export const GITHUB_PROVISIONING_FIELDS = ['provisioning.github.project.visibility.enabled'];
 
 export const GITHUB_ADDITIONAL_FIELDS = [...GITHUB_JIT_FIELDS, ...GITHUB_PROVISIONING_FIELDS];
 export const OPTIONAL_FIELDS = [
   GITHUB_ENABLED_FIELD,
   ...GITHUB_ADDITIONAL_FIELDS,
-  'sonar.auth.github.organizations',
+  GITHUB_ORGANIZATIONS_LIST,
   'sonar.auth.github.groupsSync',
 ];
 
@@ -55,6 +57,16 @@ export interface SamlSettingValue {
   definition: ExtendedSettingDefinition;
 }
 
+export const isOrganizationListEmpty = (values: Record<string, SettingValue>) =>
+  values[GITHUB_ORGANIZATIONS_LIST]?.newValue
+    ? (values[GITHUB_ORGANIZATIONS_LIST]?.newValue as string[]).length === 0
+    : !values[GITHUB_ORGANIZATIONS_LIST]?.value ||
+      values[GITHUB_ORGANIZATIONS_LIST]?.value.length === 0;
+export const isAllowToSignUpEnabled = (values: Record<string, SettingValue>) =>
+  values[GITHUB_ALLOW_TO_SIGN_UP_FIELD]?.value === 'true'
+    ? values[GITHUB_ALLOW_TO_SIGN_UP_FIELD]?.newValue === undefined
+    : values[GITHUB_ALLOW_TO_SIGN_UP_FIELD]?.newValue === true;
+
 export default function useGithubConfiguration(definitions: ExtendedSettingDefinition[]) {
   const config = useConfiguration(definitions, OPTIONAL_FIELDS);
   const { values, isValueChange, setNewValue } = config;
index cfb318747f81ffdec1af7f35aa110f8d418be72e..945ad0079781fab148a5bb7dc82b557d8f8c138d 100644 (file)
@@ -1521,17 +1521,19 @@ settings.authentication.form.settings.save_success=Settings saved successfully.
 settings.authentication.form.create.github=New GitHub Configuration
 settings.authentication.form.edit.github=Edit GitHub Configuration
 settings.authentication.github.appid_x=App ID: {0}
-settings.authentication.github.confirm.auto=Switch to automatic provisioning
-settings.authentication.github.confirm.jit=Switch to Just-in-Time provisioning
-settings.authentication.github.confirm.auto.description=Once you transition to automatic provisioning, groups, users, group memberships, and permissions on GitHub projects will be inherited from GitHub. You will no longer have the ability to edit them within SonarQube. Do you want to proceed with this change?
-settings.authentication.github.confirm.jit.description=Switching to Just-in-Time provisioning removes the automatic synchronization of users, groups, and group memberships. Users are provisioned and group memberships are updated only at user login. Are you sure?
+settings.authentication.github.confirm.AUTO_PROVISIONING=Switch to automatic provisioning
+settings.authentication.github.confirm.JIT=Switch to Just-in-Time provisioning
+settings.authentication.github.confirm.insecure=Potentially insecure configuration
+settings.authentication.github.confirm.AUTO_PROVISIONING.description=Once you transition to automatic provisioning, groups, users, group memberships, and permissions on GitHub projects will be inherited from GitHub. You will no longer have the ability to edit them within SonarQube. Do you want to proceed with this change?
+settings.authentication.github.confirm.JIT.description=Switching to Just-in-Time provisioning removes the automatic synchronization of users, groups, and group memberships. Users are provisioned and group memberships are updated only at user login. Are you sure?
 settings.authentication.github.confirm_auto_provisioning.header=Confirm the provisioning method
 settings.authentication.github.confirm_auto_provisioning.description1=Automatic user and group provisioning is currently suspended.
 settings.authentication.github.confirm_auto_provisioning.description2=This provisioning method has been enhanced. It now includes the synchronization of user permissions and project visibility from GitHub. For more details, please refer to the {documentation}.
 settings.authentication.github.confirm_auto_provisioning.question=Which provisioning method would you like to use?
 settings.authentication.github.confirm_auto_provisioning.continue=Automatic user, group, and permission provisioning
 settings.authentication.github.confirm_auto_provisioning.switch_jit=Just-in-Time user and group provisioning
-settings.authentication.github.provisioning_change.confirm_changes=Confirm Changes
+settings.authentication.github.provisioning_change.confirm_changes=Confirm changes
+settings.authentication.github.provisioning_change.insecure_config=Please be aware that your configuration is potentially insecure because you didn't add any organization to the allowlist. If your GitHub App is public, anyone can install it and gain access to your instance.
 settings.authentication.github.configuration=GitHub Configuration
 settings.authentication.github.form.not_configured=GitHub App is not configured
 settings.authentication.github.form.legacy_configured=Compatibility with GitHub OAuth Apps is deprecated and will be removed in a future release. As such, your current configuration will continue to work but some features will no longer be available. We recommend that you create a new GitHub App configuration. This will automatically replace your current GitHub OAuth App configuration. {documentation}