From d17ae1563dd46ded3b358a257a62cff9cc8ce14f Mon Sep 17 00:00:00 2001 From: Ted Johansson Date: Fri, 19 Jan 2024 11:25:24 +0800 Subject: [PATCH] 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 | 2 +- config/locales/server.en.yml | 4 ++ config/site_settings.yml | 7 +++ ...wed_groups_based_on_deprecated_settings.rb | 27 ++++++++++++ lib/site_settings/deprecated_settings.rb | 2 + .../requests/user_api_keys_controller_spec.rb | 44 +++++++++---------- 6 files changed, 63 insertions(+), 23 deletions(-) create mode 100644 db/migrate/20240117093148_fill_user_api_key_allowed_groups_based_on_deprecated_settings.rb diff --git a/app/controllers/user_api_keys_controller.rb b/app/controllers/user_api_keys_controller.rb index 2c38f7e8fd2..64b07dc96fe 100644 --- a/app/controllers/user_api_keys_controller.rb +++ b/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) diff --git a/config/locales/server.en.yml b/config/locales/server.en.yml index c3ea082227f..b8d20629f61 100644 --- a/config/locales/server.en.yml +++ b/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.
WARNING: 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.
+ WARNING: 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: diff --git a/config/site_settings.yml b/config/site_settings.yml index 33e41c33799..a78e973bf8c 100644 --- a/config/site_settings.yml +++ b/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 diff --git a/db/migrate/20240117093148_fill_user_api_key_allowed_groups_based_on_deprecated_settings.rb b/db/migrate/20240117093148_fill_user_api_key_allowed_groups_based_on_deprecated_settings.rb new file mode 100644 index 00000000000..ca7bf5f06b7 --- /dev/null +++ b/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 diff --git a/lib/site_settings/deprecated_settings.rb b/lib/site_settings/deprecated_settings.rb index e4b13808bb6..c83e42a0459 100644 --- a/lib/site_settings/deprecated_settings.rb +++ b/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) diff --git a/spec/requests/user_api_keys_controller_spec.rb b/spec/requests/user_api_keys_controller_spec.rb index 39dc6942e8b..32fb8527afe 100644 --- a/spec/requests/user_api_keys_controller_spec.rb +++ b/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