FIX: Allow plugins to correctly extend API key scopes. (#12113)

Adding a scope from a plugin was broken. This commit fixes it and adds a test.

It also documents the instance method and renames the serialized "id" attribute to "scope_id" to avoid a conflict when the scope also has a parameter with the same name.

app/controllers/admin/api_controller.rb

@@ -27,7 +27,7 @@ class Admin::ApiController < Admin::AdminController
       memo.tap do |m|
         m[resource] = actions.map do |k, v|
           {
-            id: "#{resource}:#{k}",
+            scope_id: "#{resource}:#{k}",
             key: k,
             name: k.to_s.gsub('_', ' '),
             params: v[:params],
@@ -99,7 +99,7 @@ class Admin::ApiController < Admin::AdminController
 
   def build_scopes
     params.require(:key)[:scopes].to_a.map do |scope_params|
-      resource, action = scope_params[:id].split(':')
+      resource, action = scope_params[:scope_id].split(':')
 
       mapping = ApiKeyScope.scope_mappings.dig(resource.to_sym, action.to_sym)
       raise Discourse::InvalidParameters if mapping.nil? # invalid mapping

app/models/api_key_scope.rb

@@ -36,7 +36,7 @@ class ApiKeyScope < ActiveRecord::Base
         users: {
           bookmarks: { actions: %w[users#bookmarks], params: %i[username] },
           sync_sso: { actions: %w[admin/users#sync_sso], params: %i[sso sig] },
-          show: { actions: %w[users#show], params: %i[username external_id] },
+          show: { actions: %w[users#show], params: %i[username external_id external_provider] },
           check_emails: { actions: %w[users#check_emails], params: %i[username] },
           update: { actions: %w[users#update], params: %i[username] },
           log_out: { actions: %w[admin/users#log_out] },
@@ -61,10 +61,15 @@ class ApiKeyScope < ActiveRecord::Base
       plugin_mappings = DiscoursePluginRegistry.api_key_scope_mappings
 
       default_mappings.tap do |mappings|
-        plugin_mappings.each do |mapping|
-          mapping[:urls] = find_urls(mapping[:actions])
+        plugin_mappings.each do |resource|
 
-          mappings.deep_merge!(mapping)
+          resource.each_value do |resource_actions|
+            resource_actions.each_value do |action_data|
+              action_data[:urls] = find_urls(action_data[:actions])
+            end
+          end
+
+          mappings.deep_merge!(resource)
         end
       end
     end

lib/plugin/instance.rb

@@ -791,6 +791,13 @@ class Plugin::Instance
     end
   end
 
+  # Register a new API key scope.
+  #
+  # Example:
+  # add_api_key_scope(:groups, { delete: { actions: %w[groups#add_members], params: %i[id] } })
+  #
+  # This scope lets you add members to a group. Additionally, you can specify which group ids are allowed.
+  # The delete action is added to the groups resource.
   def add_api_key_scope(resource, action)
     DiscoursePluginRegistry.register_api_key_scope_mapping({ resource => action }, self)
   end

spec/components/plugin/instance_spec.rb

@@ -593,4 +593,13 @@ describe Plugin::Instance do
       expect(ReviewableScore.types.values.max).to eq(highest_flag_id + 2)
     end
   end
+
+  describe '#add_api_key_scope' do
+    it 'adds a custom api key scope' do
+      actions = %w[admin/groups#create]
+      subject.add_api_key_scope(:groups, create: { actions: actions })
+
+      expect(ApiKeyScope.scope_mappings.dig(:groups, :create, :actions)).to contain_exactly(*actions)
+    end
+  end
 end

spec/requests/admin/api_controller_spec.rb

@@ -137,7 +137,7 @@ describe Admin::ApiController do
           post "/admin/api/keys.json", params: {
             key: {
               description: "master key description",
-              scopes: [{ id: 'topics:write', topic_id: '55' }]
+              scopes: [{ scope_id: 'topics:write', topic_id: '55' }]
             }
           }
           expect(response.status).to eq(200)
@@ -154,7 +154,7 @@ describe Admin::ApiController do
           post "/admin/api/keys.json", params: {
             key: {
               description: "master key description",
-              scopes: [{ id: 'topics:write', topic_id: '55,33' }]
+              scopes: [{ scope_id: 'topics:write', topic_id: '55,33' }]
             }
           }
           expect(response.status).to eq(200)
@@ -170,7 +170,7 @@ describe Admin::ApiController do
         post "/admin/api/keys.json", params: {
           key: {
             description: "master key description",
-            scopes: [{ id: 'topics:write', fake_id: '55' }]
+            scopes: [{ scope_id: 'topics:write', fake_id: '55' }]
           }
         }
 
@@ -186,7 +186,7 @@ describe Admin::ApiController do
         post "/admin/api/keys.json", params: {
           key: {
             description: "master key description",
-            scopes: [{ id: 'something:else' }]
+            scopes: [{ scope_id: 'something:else' }]
           }
         }