Settings API: domain_{black,white}list should be arrays

As in documentation.

Fixes: #58180.

Also remove the requirement between domain_blacklist_enabled and domain_blacklist.

app/controllers/admin/application_settings_controller.rb

@@ -85,6 +85,7 @@ class Admin::ApplicationSettingsController < Admin::ApplicationController
 
     params[:application_setting][:import_sources]&.delete("")
     params[:application_setting][:restricted_visibility_levels]&.delete("")
+    # TODO Remove domain_blacklist_raw in APIv5 (See https://gitlab.com/gitlab-org/gitlab-ce/issues/67204)
     params.delete(:domain_blacklist_raw) if params[:domain_blacklist_file]
     params.delete(:domain_blacklist_raw) if params[:domain_blacklist]
     params.delete(:domain_whitelist_raw) if params[:domain_whitelist]

app/helpers/application_settings_helper.rb

@@ -182,9 +182,11 @@ module ApplicationSettingsHelper
       :disabled_oauth_sign_in_sources,
       :domain_blacklist,
       :domain_blacklist_enabled,
-      :domain_blacklist_raw, # TODO: remove in API v5
+      # TODO Remove domain_blacklist_raw in APIv5 (See https://gitlab.com/gitlab-org/gitlab-ce/issues/67204)
+      :domain_blacklist_raw,
       :domain_whitelist,
-      :domain_whitelist_raw, # TODO: remove in API v5
+      # TODO Remove domain_whitelist_raw in APIv5 (See https://gitlab.com/gitlab-org/gitlab-ce/issues/67204)
+      :domain_whitelist_raw,
       :outbound_local_requests_whitelist_raw,
       :dsa_key_restriction,
       :ecdsa_key_restriction,

doc/api/settings.md

@@ -210,7 +210,7 @@ are listed in the descriptions of the relevant settings.
 | `diff_max_patch_bytes`                   | integer          | no                                   | Maximum diff patch size (Bytes). |
 | `disabled_oauth_sign_in_sources`         | array of strings | no                                   | Disabled OAuth sign-in sources. |
 | `dns_rebinding_protection_enabled`       | boolean          | no                                   | Enforce DNS rebinding attack protection. |
-| `domain_blacklist`                       | array of strings | required by: `domain_blacklist_enabled` | Users with e-mail addresses that match these domain(s) will NOT be able to sign-up. Wildcards allowed. Use separate lines for multiple entries. Ex: `domain.com`, `*.domain.com`. |
+| `domain_blacklist`                       | array of strings | no                                   | Users with e-mail addresses that match these domain(s) will NOT be able to sign-up. Wildcards allowed. Use separate lines for multiple entries. Ex: `domain.com`, `*.domain.com`. |
 | `domain_blacklist_enabled`               | boolean          | no                                   | (**If enabled, requires:** `domain_blacklist`) Allows blocking sign-ups from emails from specific domains. |
 | `domain_whitelist`                       | array of strings | no                                   | Force people to use only corporate emails for sign-up. Default is `null`, meaning there is no restriction. |
 | `dsa_key_restriction`                    | integer          | no                                   | The minimum allowed bit length of an uploaded DSA key. Default is `0` (no restriction). `-1` disables DSA keys. |

lib/api/settings.rb

@@ -50,10 +50,8 @@ module API
       optional :default_snippet_visibility, type: String, values: Gitlab::VisibilityLevel.string_values, desc: 'The default snippet visibility'
       optional :disabled_oauth_sign_in_sources, type: Array[String], desc: 'Disable certain OAuth sign-in sources'
       optional :domain_blacklist_enabled, type: Boolean, desc: 'Enable domain blacklist for sign ups'
-      given domain_blacklist_enabled: ->(val) { val } do
-        requires :domain_blacklist, type: String, desc: 'Users with e-mail addresses that match these domain(s) will NOT be able to sign-up. Wildcards allowed. Use separate lines for multiple entries. Ex: domain.com, *.domain.com'
-      end
-      optional :domain_whitelist, type: String, desc: 'ONLY users with e-mail addresses that match these domain(s) will be able to sign-up. Wildcards allowed. Use separate lines for multiple entries. Ex: domain.com, *.domain.com'
+      optional :domain_blacklist, type: Array[String], coerce_with: Validations::Types::CommaSeparatedToArray.coerce, desc: 'Users with e-mail addresses that match these domain(s) will NOT be able to sign-up. Wildcards allowed. Use separate lines for multiple entries. Ex: domain.com, *.domain.com'
+      optional :domain_whitelist, type: Array[String], coerce_with: Validations::Types::CommaSeparatedToArray.coerce, desc: 'ONLY users with e-mail addresses that match these domain(s) will be able to sign-up. Wildcards allowed. Use separate lines for multiple entries. Ex: domain.com, *.domain.com'
       optional :email_author_in_body, type: Boolean, desc: 'Some email servers do not support overriding the email sender name. Enable this option to include the name of the author of the issue, merge request or comment in the email body instead.'
       optional :enabled_git_access_protocol, type: String, values: %w[ssh http nil], desc: 'Allow only the selected protocols to be used for Git access.'
       optional :gitaly_timeout_default, type: Integer, desc: 'Default Gitaly timeout, in seconds. Set to 0 to disable timeouts.'

spec/requests/api/settings_spec.rb

@@ -252,5 +252,43 @@ describe API::Settings, 'Settings' do
         expect(json_response['asset_proxy_whitelist']).to eq(['example.com', '*.example.com', 'localhost'])
       end
     end
+
+    context 'domain_blacklist settings' do
+      it 'rejects domain_blacklist_enabled when domain_blacklist is empty' do
+        put api('/application/settings', admin),
+          params: {
+            domain_blacklist_enabled: true,
+            domain_blacklist: []
+          }
+
+        expect(response).to have_gitlab_http_status(400)
+        message = json_response["message"]
+        expect(message["domain_blacklist"]).to eq(["Domain blacklist cannot be empty if Blacklist is enabled."])
+      end
+
+      it 'allows array for domain_blacklist' do
+        put api('/application/settings', admin),
+          params: {
+            domain_blacklist_enabled: true,
+            domain_blacklist: ['domain1.com', 'domain2.com']
+          }
+
+        expect(response).to have_gitlab_http_status(200)
+        expect(json_response['domain_blacklist_enabled']).to be(true)
+        expect(json_response['domain_blacklist']).to eq(['domain1.com', 'domain2.com'])
+      end
+
+      it 'allows a string for domain_blacklist' do
+        put api('/application/settings', admin),
+          params: {
+            domain_blacklist_enabled: true,
+            domain_blacklist: 'domain3.com, *.domain4.com'
+          }
+
+        expect(response).to have_gitlab_http_status(200)
+        expect(json_response['domain_blacklist_enabled']).to be(true)
+        expect(json_response['domain_blacklist']).to eq(['domain3.com', '*.domain4.com'])
+      end
+    end
   end
 end