fix SSF-434

8 files changed, 110 insertions, 14 deletions

diff --git a/server/sonar-auth-github/src/main/java/org/sonar/auth/github/GitHubSettings.java b/server/sonar-auth-github/src/main/java/org/sonar/auth/github/GitHubSettings.java
index 12e01aa4d9b..5bd6f48caab 100644
--- a/server/sonar-auth-github/src/main/java/org/sonar/auth/github/GitHubSettings.java
+++ b/server/sonar-auth-github/src/main/java/org/sonar/auth/github/GitHubSettings.java
@@ -158,8 +158,8 @@ public class GitHubSettings {
         .build(),
       PropertyDefinition.builder(ORGANIZATIONS)
         .name("Organizations")
-        .description("Only members of these organizations will be able to authenticate to the server. " +
-          "If a user is a member of any of the organizations listed they will be authenticated.")
+        .description("Only members of these organizations will be able to authenticate to the server. "
+                     + "⚠ if not set, any GitHub user will be able to authenticate to the server.")
         .multiValues(true)
         .category(CATEGORY)
         .subCategory(SUBCATEGORY)
diff --git a/server/sonar-web/src/main/js/api/mocks/AuthenticationServiceMock.ts b/server/sonar-web/src/main/js/api/mocks/AuthenticationServiceMock.ts
index 8aba451dd6e..3cc0d045089 100644
--- a/server/sonar-web/src/main/js/api/mocks/AuthenticationServiceMock.ts
+++ b/server/sonar-web/src/main/js/api/mocks/AuthenticationServiceMock.ts
@@ -30,6 +30,7 @@ export default class AuthenticationServiceMock {
     mockSettingValue({ key: 'test2', value: 'test2' }),
     mockSettingValue({ key: 'sonar.auth.saml.certificate.secured' }),
     mockSettingValue({ key: 'sonar.auth.saml.enabled', value: 'false' }),
+    mockSettingValue({ key: 'sonar.auth.github.enabled', value: 'true' }),
   ];
 
   constructor() {
diff --git a/server/sonar-web/src/main/js/apps/settings/components/CategoryDefinitionsList.tsx b/server/sonar-web/src/main/js/apps/settings/components/CategoryDefinitionsList.tsx
index e18db13cf06..87e3e439083 100644
--- a/server/sonar-web/src/main/js/apps/settings/components/CategoryDefinitionsList.tsx
+++ b/server/sonar-web/src/main/js/apps/settings/components/CategoryDefinitionsList.tsx
@@ -19,7 +19,11 @@
  */
 import { keyBy } from 'lodash';
 import * as React from 'react';
+import { FormattedMessage } from 'react-intl';
 import { getValues } from '../../../api/settings';
+import DocLink from '../../../components/common/DocLink';
+import { Alert } from '../../../components/ui/Alert';
+import { translate } from '../../../helpers/l10n';
 import {
   ExtendedSettingDefinition,
   SettingDefinitionAndValue,
@@ -38,11 +42,12 @@ interface Props {
 
 interface State {
   settings: SettingDefinitionAndValue[];
+  displayGithubOrganizationWarning: boolean;
 }
 
 export default class CategoryDefinitionsList extends React.PureComponent<Props, State> {
   mounted = false;
-  state: State = { settings: [] };
+  state: State = { settings: [], displayGithubOrganizationWarning: false };
 
   componentDidMount() {
     this.mounted = true;
@@ -60,7 +65,25 @@ export default class CategoryDefinitionsList extends React.PureComponent<Props,
     this.mounted = false;
   }
 
-  async loadSettingValues() {
+  shouldDisplayGithubWarning = (settings: SettingDefinitionAndValue[]) => {
+    const { category, subCategory } = this.props;
+    if (category !== 'authentication' || subCategory !== 'github') {
+      return false;
+    }
+    const isGithubEnabled = settings.find((s) => s.definition.key === 'sonar.auth.github.enabled');
+    const organizationsSetting = settings.find(
+      (s) => s.definition.key === 'sonar.auth.github.organizations'
+    );
+    if (
+      isGithubEnabled?.settingValue?.value === 'true' &&
+      organizationsSetting?.settingValue === undefined
+    ) {
+      return true;
+    }
+    return false;
+  };
+
+  loadSettingValues = async () => {
     const { category, component, definitions } = this.props;
 
     const categoryDefinitions = definitions.filter(
@@ -83,21 +106,41 @@ export default class CategoryDefinitionsList extends React.PureComponent<Props,
       };
     });
 
-    this.setState({ settings });
-  }
+    const displayGithubOrganizationWarning = this.shouldDisplayGithubWarning(settings);
+
+    this.setState({ settings, displayGithubOrganizationWarning });
+  };
 
   render() {
     const { category, component, subCategory, displaySubCategoryTitle } = this.props;
-    const { settings } = this.state;
+    const { settings, displayGithubOrganizationWarning } = this.state;
 
     return (
-      <SubCategoryDefinitionsList
-        category={category}
-        component={component}
-        settings={settings}
-        subCategory={subCategory}
-        displaySubCategoryTitle={displaySubCategoryTitle}
-      />
+      <>
+        {displayGithubOrganizationWarning && (
+          <Alert variant="error">
+            <FormattedMessage
+              id="settings.authentication.github.organization.warning"
+              defaultMessage={translate('settings.authentication.github.organization.warning')}
+              values={{
+                learn_more: (
+                  <DocLink to="/instance-administration/authentication/github/#setting-your-authentication-settings-in-sonarqube">
+                    {translate('settings.authentication.github.organization.warning.learn_more')}
+                  </DocLink>
+                ),
+              }}
+            />
+          </Alert>
+        )}
+        <SubCategoryDefinitionsList
+          category={category}
+          component={component}
+          settings={settings}
+          subCategory={subCategory}
+          displaySubCategoryTitle={displaySubCategoryTitle}
+          onUpdate={this.loadSettingValues}
+        />
+      </>
     );
   }
 }
diff --git a/server/sonar-web/src/main/js/apps/settings/components/Definition.tsx b/server/sonar-web/src/main/js/apps/settings/components/Definition.tsx
index c0167864efc..81ed3dc2cb8 100644
--- a/server/sonar-web/src/main/js/apps/settings/components/Definition.tsx
+++ b/server/sonar-web/src/main/js/apps/settings/components/Definition.tsx
@@ -30,6 +30,7 @@ interface Props {
   component?: Component;
   definition: ExtendedSettingDefinition;
   initialSettingValue?: SettingValue;
+  onUpdate?: () => void;
 }
 
 interface State {
@@ -82,6 +83,10 @@ export default class Definition extends React.PureComponent<Props, State> {
       await resetSettingValue({ keys: definition.key, component: component?.key });
       const settingValue = await getValue({ key: definition.key, component: component?.key });
 
+      if (this.props.onUpdate) {
+        this.props.onUpdate();
+      }
+
       this.setState({
         changedValue: undefined,
         loading: false,
@@ -171,6 +176,10 @@ export default class Definition extends React.PureComponent<Props, State> {
         await setSettingValue(definition, changedValue, component?.key);
         const settingValue = await getValue({ key: definition.key, component: component?.key });
 
+        if (this.props.onUpdate) {
+          this.props.onUpdate();
+        }
+
         this.setState({
           changedValue: undefined,
           isEditing: false,
diff --git a/server/sonar-web/src/main/js/apps/settings/components/DefinitionsList.tsx b/server/sonar-web/src/main/js/apps/settings/components/DefinitionsList.tsx
index 45c04a0978a..46f627d7b05 100644
--- a/server/sonar-web/src/main/js/apps/settings/components/DefinitionsList.tsx
+++ b/server/sonar-web/src/main/js/apps/settings/components/DefinitionsList.tsx
@@ -26,6 +26,7 @@ interface Props {
   component?: Component;
   scrollToDefinition: (element: HTMLLIElement) => void;
   settings: SettingDefinitionAndValue[];
+  onUpdate?: () => void;
 }
 
 export default function DefinitionsList(props: Props) {
@@ -42,6 +43,7 @@ export default function DefinitionsList(props: Props) {
             component={component}
             definition={setting.definition}
             initialSettingValue={setting.settingValue}
+            onUpdate={props.onUpdate}
           />
         </li>
       ))}
diff --git a/server/sonar-web/src/main/js/apps/settings/components/SubCategoryDefinitionsList.tsx b/server/sonar-web/src/main/js/apps/settings/components/SubCategoryDefinitionsList.tsx
index b904584590f..33103b38481 100644
--- a/server/sonar-web/src/main/js/apps/settings/components/SubCategoryDefinitionsList.tsx
+++ b/server/sonar-web/src/main/js/apps/settings/components/SubCategoryDefinitionsList.tsx
@@ -34,6 +34,7 @@ export interface SubCategoryDefinitionsListProps {
   settings: Array<SettingDefinitionAndValue>;
   subCategory?: string;
   displaySubCategoryTitle?: boolean;
+  onUpdate?: () => void;
 }
 
 export class SubCategoryDefinitionsList extends React.PureComponent<SubCategoryDefinitionsListProps> {
@@ -103,6 +104,7 @@ export class SubCategoryDefinitionsList extends React.PureComponent<SubCategoryD
               component={component}
               scrollToDefinition={this.scrollToSubCategoryOrDefinition}
               settings={bySubCategory[subCategory.key]}
+              onUpdate={this.props.onUpdate}
             />
             {this.renderEmailForm(subCategory.key)}
           </li>
diff --git a/server/sonar-web/src/main/js/apps/settings/components/authentication/__tests__/Authentication-it.tsx b/server/sonar-web/src/main/js/apps/settings/components/authentication/__tests__/Authentication-it.tsx
index 1070420b649..f34136ca02f 100644
--- a/server/sonar-web/src/main/js/apps/settings/components/authentication/__tests__/Authentication-it.tsx
+++ b/server/sonar-web/src/main/js/apps/settings/components/authentication/__tests__/Authentication-it.tsx
@@ -79,6 +79,8 @@ const ui = {
   testButton: byText('settings.authentication.saml.form.test'),
   textbox1: byRole('textbox', { name: 'test1' }),
   textbox2: byRole('textbox', { name: 'test2' }),
+  githubTab: byRole('tab', { name: 'github GitHub' }),
+  githubOrganizationWarning: byText('settings.authentication.github.organization.warning'),
 };
 
 it('should render tabs and allow navigation', async () => {
@@ -209,6 +211,41 @@ describe('SAML tab', () => {
   });
 });
 
+describe('GitHub tab', () => {
+  it('should display a warning if github authentication is enabled but no organizations are whitelisted', async () => {
+    const user = userEvent.setup();
+
+    const definitions = [
+      mockDefinition({
+        key: 'sonar.auth.github.enabled',
+        category: 'authentication',
+        subCategory: 'github',
+        name: '"Enabled"',
+        description:
+          'Enable GitHub users to login. Value is ignored if client ID and secret are not defined.',
+        type: SettingType.BOOLEAN,
+      }),
+      mockDefinition({
+        key: 'sonar.auth.github.organizations',
+        category: 'authentication',
+        subCategory: 'github',
+        name: 'Organizations',
+        description:
+          'Only members of these organizations will be able to authenticate to the server. If a user is a member of any of the organizations listed they will be authenticated.',
+        type: SettingType.BOOLEAN,
+        fields: [],
+        multiValues: true,
+        options: [],
+      }),
+    ];
+
+    renderAuthentication(definitions);
+
+    await user.click(await ui.githubTab.find());
+    expect(ui.githubOrganizationWarning.get()).toBeInTheDocument();
+  });
+});
+
 function renderAuthentication(definitions: ExtendedSettingDefinition[], features: Feature[] = []) {
   renderComponent(
     <AvailableFeaturesContext.Provider value={features}>
diff --git a/sonar-core/src/main/resources/org/sonar/l10n/core.properties b/sonar-core/src/main/resources/org/sonar/l10n/core.properties
index 9f78437bb71..8f399e90dfe 100644
--- a/sonar-core/src/main/resources/org/sonar/l10n/core.properties
+++ b/sonar-core/src/main/resources/org/sonar/l10n/core.properties
@@ -1284,6 +1284,8 @@ settings.authentication.saml.form.save_success=Saved successfully
 settings.authentication.saml.form.save_partial=Saved partially
 settings.authentication.saml.form.save_warn=Please check the error messages in the form above, saving failed for {0} field(s).
 settings.authentication.saml.tooltip.required_fields=Please provide a value for the following required field(s): {0}
+settings.authentication.github.organization.warning=GitHub authentication is activated but no allowed organization was provided. Please review your settings to make sure the integration is secure. {learn_more}.
+settings.authentication.github.organization.warning.learn_more=Learn more
 
 settings.pr_decoration.binding.category=DevOps Platform Integration
 settings.pr_decoration.binding.no_bindings=A system administrator needs to enable this feature in the global settings.