DEV: Require at least one scope for API key granular mode (#31253)

Currently, if creating an API key in "granular" mode, and not selecting any scopes, a globally scoped API key is created. This can be surprising and is not ideal. Having a key with no scopes isn't useful in the first place, so this PR adds client- and server side validations to check that at least one scope is selected if using "granular" mode.

app/assets/javascripts/admin/addon/components/admin-config-areas/api-keys-new.gjs

@@ -10,6 +10,7 @@ import DButton from "discourse/components/d-button";
 import Form from "discourse/components/form";
 import { ajax } from "discourse/lib/ajax";
 import { popupAjaxError } from "discourse/lib/ajax-error";
+import { bind } from "discourse/lib/decorators";
 import { i18n } from "discourse-i18n";
 import ApiKeyUrlsModal from "admin/components/modal/api-key-urls";
 import EmailGroupUserChooser from "select-kit/components/email-group-user-chooser";
@@ -79,7 +80,10 @@ export default class AdminConfigAreasApiKeysNew extends Component {
 
   @action
   async save(data) {
-    const payload = { description: data.description };
+    const payload = {
+      description: data.description,
+      scope_mode: data.scope_mode,
+    };
 
     if (data.user_mode === "single") {
       payload.username = data.user;
@@ -123,6 +127,21 @@ export default class AdminConfigAreasApiKeysNew extends Component {
     return enabledScopes.flat();
   }
 
+  @bind
+  atLeastOneGranularScope(data, { addError, removeError }) {
+    removeError("scopes");
+
+    if (
+      data.scope_mode === "granular" &&
+      this.#selectedScopes(data.scopes).length === 0
+    ) {
+      addError("scopes", {
+        title: i18n("admin.api.scopes.title"),
+        message: i18n("admin.api.scopes.one_or_more"),
+      });
+    }
+  }
+
   @action
   async showURLs(urls) {
     await this.modal.show(ApiKeyUrlsModal, {
@@ -165,6 +184,7 @@ export default class AdminConfigAreasApiKeysNew extends Component {
               <Form
                 @onSubmit={{this.save}}
                 @data={{this.formData}}
+                @validate={{this.atLeastOneGranularScope}}
                 as |form transientData|
               >
                 <form.Field

app/assets/javascripts/admin/addon/models/api-key.js

@@ -55,7 +55,12 @@ export default class ApiKey extends RestModel {
   }
 
   createProperties() {
-    return this.getProperties("description", "username", "scopes");
+    return this.getProperties(
+      "description",
+      "username",
+      "scopes",
+      "scope_mode"
+    );
   }
 
   @discourseComputed()

app/controllers/admin/api_controller.rb

@@ -75,6 +75,7 @@ class Admin::ApiController < Admin::AdminController
     ApiKey.transaction do
       api_key.created_by = current_user
       api_key.api_key_scopes = build_scopes
+      api_key.scope_mode = params.dig(:key, :scope_mode)
       if username = params.require(:key).permit(:username)[:username].presence
         api_key.user = User.find_by_username(username)
         raise Discourse::NotFound unless api_key.user

app/models/api_key.rb

@@ -4,6 +4,8 @@ class ApiKey < ActiveRecord::Base
   class KeyAccessError < StandardError
   end
 
+  attr_accessor :scope_mode
+
   has_many :api_key_scopes
   belongs_to :user
   belongs_to :created_by, class_name: "User"
@@ -18,6 +20,7 @@ class ApiKey < ActiveRecord::Base
         end
 
   validates :description, length: { maximum: 255 }
+  validate :at_least_one_granular_scope
 
   after_initialize :generate_key
 
@@ -114,6 +117,17 @@ class ApiKey < ActiveRecord::Base
     # using update_column to avoid the AR transaction
     update_column(:last_used_at, now)
   end
+
+  private
+
+  def at_least_one_granular_scope
+    if scope_mode == "granular" && api_key_scopes.empty?
+      errors.add(
+        :api_key_scopes,
+        I18n.t("activerecord.errors.models.api_key.base.at_least_one_granular_scope"),
+      )
+    end
+  end
 end
 
 # == Schema Information

config/locales/client.en.yml

@@ -5478,6 +5478,7 @@ en:
             When using scopes, you can restrict an API key to a specific set of endpoints.
             You can also define which parameters will be allowed. Use commas to separate multiple values.
           title: Scopes
+          one_or_more: At least one scope must be selected.
           granular: Granular
           read_only: Read-only
           global: Global

config/locales/server.en.yml

@@ -847,6 +847,9 @@ en:
           attributes:
             linkable_type:
               invalid: "is not valid"
+        api_key:
+          base:
+            at_least_one_granular_scope: "at least one must be selected"
 
   uncategorized_category_name: "Uncategorized"
 

spec/models/api_key_spec.rb

@@ -8,6 +8,15 @@ RSpec.describe ApiKey do
   it { is_expected.to belong_to :created_by }
   it { is_expected.to validate_length_of(:description).is_at_most(255) }
 
+  it "validates at least one scope for granular mode" do
+    api_key = ApiKey.new
+    api_key.scope_mode = "granular"
+
+    api_key.validate
+
+    expect(api_key.errors).to contain_exactly("Api key scopes at least one must be selected")
+  end
+
   it "generates a key when saving" do
     api_key = ApiKey.new
     api_key.save!