DEV: Convert min_trust_level_for_user_api_key to groups (#25299)

We're changing the implementation of trust levels to use groups. Part of this is to have site settings that reference trust levels use groups instead. It converts the min_trust_level_for_user_api_key  site setting to user_api_key_allowed_groups.

This isn't used by any of our plugins or themes, so very little fallout.

app/controllers/user_api_keys_controller.rb

@@ -183,7 +183,7 @@ class UserApiKeysController < ApplicationController
   end
 
   def meets_tl?
-    current_user.staff? || current_user.trust_level >= SiteSetting.min_trust_level_for_user_api_key
+    current_user.staff? || current_user.in_any_groups?(SiteSetting.user_api_key_allowed_groups_map)
   end
 
   def one_time_password(public_key, username)

config/locales/server.en.yml

@@ -2425,6 +2425,9 @@ en:
     min_trust_level_for_user_api_key: |
       Trust level required for generation of user API keys.<br>
       <b>WARNING</b>: Changing the trust level will prevent users with a lower trust level from logging in via Discourse Hub
+    user_api_key_allowed_groups: |
+      Group membership required for generation of user API keys.<br>
+      <b>WARNING</b>: Changing the trust level will prevent users with a lower trust level from logging in via Discourse Hub
     allowed_user_api_auth_redirects: "Allowed URL for authentication redirect for user API keys. Wildcard symbol * can be used to match any part of it (e.g. www.example.com/*)."
     allowed_user_api_push_urls: "Allowed URLs for server push to user API"
     revoke_user_api_keys_unused_days: "Number of days since a user API key was last used before it is automatically revoked (0 for never)"
@@ -2588,6 +2591,7 @@ en:
       send_email_messages_allowed_groups: "min_trust_to_send_email_messages"
       skip_review_media_groups: "review_media_unless_trust_level"
       post_links_allowed_groups: "min_trust_to_post_links"
+      user_api_key_allowed_groups: "min_trust_level_for_user_api_key"
 
     placeholder:
       discourse_connect_provider_secrets:

config/site_settings.yml

@@ -2985,6 +2985,13 @@ user_api:
   min_trust_level_for_user_api_key:
     default: 0
     enum: "TrustLevelSetting"
+    hidden: true
+  user_api_key_allowed_groups:
+    default: "10"
+    type: group_list
+    allow_any: false
+    refresh: true
+    validator: "AtLeastOneGroupValidator"
   allowed_user_api_push_urls:
     default: ""
     type: list

db/migrate/20240117093148_fill_user_api_key_allowed_groups_based_on_deprecated_settings.rb

@@ -0,0 +1,27 @@
+# frozen_string_literal: true
+
+class FillUserApiKeyAllowedGroupsBasedOnDeprecatedSettings < ActiveRecord::Migration[7.0]
+  def up
+    configured_trust_level =
+      DB.query_single(
+        "SELECT value FROM site_settings WHERE name = 'min_trust_level_for_user_api_key' LIMIT 1",
+      ).first
+
+    # Default for old setting is TL0, we only need to do anything if it's been changed in the DB.
+    if configured_trust_level.present?
+      # Matches Group::AUTO_GROUPS to the trust levels.
+      corresponding_group = "1#{configured_trust_level}"
+
+      # Data_type 20 is group_list.
+      DB.exec(
+        "INSERT INTO site_settings(name, value, data_type, created_at, updated_at)
+        VALUES('user_api_key_allowed_groups', :setting, '20', NOW(), NOW())",
+        setting: corresponding_group,
+      )
+    end
+  end
+
+  def down
+    raise ActiveRecord::IrreversibleMigration
+  end
+end

lib/site_settings/deprecated_settings.rb

@@ -38,6 +38,7 @@ module SiteSettings::DeprecatedSettings
     ["min_trust_to_send_email_messages", "send_email_messages_allowed_groups", false, "3.3"],
     ["review_media_unless_trust_level", "skip_review_media_groups", false, "3.3"],
     ["min_trust_to_post_links", "post_links_allowed_groups", false, "3.3"],
+    ["min_trust_level_for_user_api_key", "user_api_key_allowed_groups", false, "3.3"],
   ]
 
   OVERRIDE_TL_GROUP_SETTINGS = %w[
@@ -60,6 +61,7 @@ module SiteSettings::DeprecatedSettings
     min_trust_to_send_email_messages
     review_media_unless_trust_level
     min_trust_to_post_links
+    min_trust_level_for_user_api_key
   ]
 
   def group_to_tl(old_setting, new_setting)

spec/requests/user_api_keys_controller_spec.rb

@@ -64,10 +64,10 @@ RSpec.describe UserApiKeysController do
     end
 
     it "will allow tokens for staff without TL" do
-      SiteSetting.min_trust_level_for_user_api_key = 2
+      SiteSetting.user_api_key_allowed_groups = Group::AUTO_GROUPS[:trust_level_2]
       SiteSetting.allowed_user_api_auth_redirects = args[:auth_redirect]
 
-      user = Fabricate(:user, trust_level: 1, moderator: true)
+      user = Fabricate(:user, trust_level: 1, moderator: true, refresh_auto_groups: true)
 
       sign_in(user)
 
@@ -76,10 +76,10 @@ RSpec.describe UserApiKeysController do
     end
 
     it "will not create token unless TL is met" do
-      SiteSetting.min_trust_level_for_user_api_key = 2
+      SiteSetting.user_api_key_allowed_groups = Group::AUTO_GROUPS[:trust_level_2]
       SiteSetting.allowed_user_api_auth_redirects = args[:auth_redirect]
 
-      user = Fabricate(:user, trust_level: 1)
+      user = Fabricate(:user, trust_level: 1, refresh_auto_groups: true)
       sign_in(user)
 
       post "/user-api-key.json", params: args
@@ -87,11 +87,11 @@ RSpec.describe UserApiKeysController do
     end
 
     it "will deny access if requesting more rights than allowed" do
-      SiteSetting.min_trust_level_for_user_api_key = 0
+      SiteSetting.user_api_key_allowed_groups = Group::AUTO_GROUPS[:trust_level_0]
       SiteSetting.allowed_user_api_auth_redirects = args[:auth_redirect]
       SiteSetting.allow_user_api_key_scopes = "write"
 
-      user = Fabricate(:user, trust_level: 0)
+      user = Fabricate(:user, trust_level: 0, refresh_auto_groups: true)
       sign_in(user)
 
       post "/user-api-key.json", params: args
@@ -150,13 +150,13 @@ RSpec.describe UserApiKeysController do
     end
 
     it "will not return p access if not yet configured" do
-      SiteSetting.min_trust_level_for_user_api_key = 0
+      SiteSetting.user_api_key_allowed_groups = Group::AUTO_GROUPS[:trust_level_0]
       SiteSetting.allowed_user_api_auth_redirects = args[:auth_redirect]
 
       args[:scopes] = "push,read"
       args[:push_url] = "https://push.it/here"
 
-      user = Fabricate(:user, trust_level: 0)
+      user = Fabricate(:user, trust_level: 0, refresh_auto_groups: true)
       sign_in(user)
 
       post "/user-api-key.json", params: args
@@ -182,14 +182,14 @@ RSpec.describe UserApiKeysController do
     end
 
     it "will redirect correctly with valid token" do
-      SiteSetting.min_trust_level_for_user_api_key = 0
+      SiteSetting.user_api_key_allowed_groups = Group::AUTO_GROUPS[:trust_level_0]
       SiteSetting.allowed_user_api_auth_redirects = args[:auth_redirect]
       SiteSetting.allowed_user_api_push_urls = "https://push.it/here"
 
       args[:scopes] = "push,notifications,message_bus,session_info,one_time_password"
       args[:push_url] = "https://push.it/here"
 
-      user = Fabricate(:user, trust_level: 0)
+      user = Fabricate(:user, trust_level: 0, refresh_auto_groups: true)
       sign_in(user)
 
       post "/user-api-key.json", params: args
@@ -235,12 +235,12 @@ RSpec.describe UserApiKeysController do
     end
 
     it "will just show the payload if no redirect" do
-      user = Fabricate(:user, trust_level: 0)
+      user = Fabricate(:user, trust_level: 0, refresh_auto_groups: true)
       sign_in(user)
 
       args.delete(:auth_redirect)
 
-      SiteSetting.min_trust_level_for_user_api_key = 0
+      SiteSetting.user_api_key_allowed_groups = Group::AUTO_GROUPS[:trust_level_0]
       post "/user-api-key", params: args
       expect(response.status).not_to eq(302)
       payload = Nokogiri.HTML5(response.body).at("code").content
@@ -252,12 +252,12 @@ RSpec.describe UserApiKeysController do
     end
 
     it "will just show the JSON payload if no redirect" do
-      user = Fabricate(:user, trust_level: 0)
+      user = Fabricate(:user, trust_level: 0, refresh_auto_groups: true)
       sign_in(user)
 
       args.delete(:auth_redirect)
 
-      SiteSetting.min_trust_level_for_user_api_key = 0
+      SiteSetting.user_api_key_allowed_groups = Group::AUTO_GROUPS[:trust_level_0]
       post "/user-api-key.json", params: args
       expect(response.status).not_to eq(302)
       payload = response.parsed_body["payload"]
@@ -272,17 +272,17 @@ RSpec.describe UserApiKeysController do
       SiteSetting.allowed_user_api_auth_redirects = args[:auth_redirect] + "/*"
       args[:auth_redirect] = args[:auth_redirect] + "/bluebirds/fly"
 
-      sign_in(Fabricate(:user))
+      sign_in(Fabricate(:user, refresh_auto_groups: true))
 
       post "/user-api-key.json", params: args
       expect(response.status).to eq(302)
     end
 
     it "will keep query_params added in auth_redirect" do
-      SiteSetting.min_trust_level_for_user_api_key = 0
+      SiteSetting.user_api_key_allowed_groups = Group::AUTO_GROUPS[:trust_level_0]
       SiteSetting.allowed_user_api_auth_redirects = args[:auth_redirect] + "/*"
 
-      user = Fabricate(:user, trust_level: 0)
+      user = Fabricate(:user, trust_level: 0, refresh_auto_groups: true)
       sign_in(user)
 
       query_str = "/?param1=val1"
@@ -317,10 +317,10 @@ RSpec.describe UserApiKeysController do
     end
 
     it "will allow one-time-password for staff without TL" do
-      SiteSetting.min_trust_level_for_user_api_key = 2
+      SiteSetting.user_api_key_allowed_groups = Group::AUTO_GROUPS[:trust_level_2]
       SiteSetting.allowed_user_api_auth_redirects = otp_args[:auth_redirect]
 
-      user = Fabricate(:user, trust_level: 1, moderator: true)
+      user = Fabricate(:user, trust_level: 1, moderator: true, refresh_auto_groups: true)
 
       sign_in(user)
 
@@ -329,10 +329,10 @@ RSpec.describe UserApiKeysController do
     end
 
     it "will not allow one-time-password unless TL is met" do
-      SiteSetting.min_trust_level_for_user_api_key = 2
+      SiteSetting.user_api_key_allowed_groups = Group::AUTO_GROUPS[:trust_level_2]
       SiteSetting.allowed_user_api_auth_redirects = otp_args[:auth_redirect]
 
-      user = Fabricate(:user, trust_level: 1)
+      user = Fabricate(:user, trust_level: 1, refresh_auto_groups: true)
       sign_in(user)
 
       post "/user-api-key/otp", params: otp_args
@@ -351,7 +351,7 @@ RSpec.describe UserApiKeysController do
 
     it "will return one-time-password when args are valid" do
       SiteSetting.allowed_user_api_auth_redirects = otp_args[:auth_redirect]
-      user = Fabricate(:user)
+      user = Fabricate(:user, refresh_auto_groups: true)
       sign_in(user)
 
       post "/user-api-key/otp", params: otp_args