Skip to content
This repository has been archived by the owner on May 27, 2022. It is now read-only.

Commit

Permalink
FEATURE: Make allow_uploaded_avatars accept TL (discourse#14091)
Browse files Browse the repository at this point in the history
This gives admins more control over who can upload custom profile
pictures.
  • Loading branch information
nbianca authored Aug 24, 2021
1 parent eb6d66f commit ff367e2
Show file tree
Hide file tree
Showing 12 changed files with 122 additions and 14 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -83,10 +83,23 @@ export default Controller.extend(ModalFunctionality, {
}
},

@discourseComputed()
allowAvatarUpload() {
siteSettingMatches(value, user) {
switch (value) {
case "disabled":
return false;
case "staff":
return user.staff;
case "admin":
return user.admin;
default:
return user.trust_level >= parseInt(value, 10) || user.staff;
}
},

@discourseComputed("siteSettings.allow_uploaded_avatars")
allowAvatarUpload(allowUploadedAvatars) {
return (
this.siteSettings.allow_uploaded_avatars &&
this.siteSettingMatches(allowUploadedAvatars, this.currentUser) &&
allowsImages(this.currentUser.staff, this.siteSettings)
);
},
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -62,7 +62,7 @@ const ORIGINAL_SETTINGS = {
max_image_width: 690,
max_image_height: 500,
allow_profile_backgrounds: true,
allow_uploaded_avatars: true,
allow_uploaded_avatars: "0",
tl1_requires_read_posts: 30,
enable_long_polling: true,
polling_interval: 3000,
Expand Down
2 changes: 1 addition & 1 deletion app/controllers/uploads_controller.rb
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,7 @@ def create
# 50 characters ought to be enough for the upload type
type = (params[:upload_type].presence || params[:type].presence).parameterize(separator: "_")[0..50]

if type == "avatar" && !me.admin? && (SiteSetting.discourse_connect_overrides_avatar || !SiteSetting.allow_uploaded_avatars)
if type == "avatar" && !me.admin? && (SiteSetting.discourse_connect_overrides_avatar || !TrustLevelAndStaffAndDisabledSetting.matches?(SiteSetting.allow_uploaded_avatars, me))
return render json: failed_json, status: 422
end

Expand Down
2 changes: 1 addition & 1 deletion app/controllers/users_controller.rb
Original file line number Diff line number Diff line change
Expand Up @@ -1138,7 +1138,7 @@ def pick_avatar

if type.blank? || type == 'system'
upload_id = nil
elsif !SiteSetting.allow_uploaded_avatars
elsif !TrustLevelAndStaffAndDisabledSetting.matches?(SiteSetting.allow_uploaded_avatars, user)
return render json: failed_json, status: 422
else
upload_id = params[:upload_id]
Expand Down
32 changes: 32 additions & 0 deletions app/models/trust_level_and_staff_and_disabled_setting.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,32 @@
# frozen_string_literal: true

class TrustLevelAndStaffAndDisabledSetting < TrustLevelAndStaffSetting
def self.valid_value?(val)
valid_values.include?(val) || (val.to_i.to_s == val.to_s && valid_values.include?(val.to_i))
end

def self.valid_values
['disabled'] + TrustLevel.valid_range.to_a + special_groups
end

def self.translation(value)
if value == 'disabled'
I18n.t('site_settings.disabled')
else
super
end
end

def self.matches?(value, user)
case value
when 'disabled'
false
when 'staff'
user.staff?
when 'admin'
user.admin?
else
user.has_trust_level?(value.to_i) || user.staff?
end
end
end
1 change: 1 addition & 0 deletions config/locales/server.en.yml
Original file line number Diff line number Diff line change
Expand Up @@ -1450,6 +1450,7 @@ en:
watched_word_regexp_error: "The regular expression for '%{action}' watched words is invalid. Please check your <a href='%{base_path}/admin/customize/watched_words'>Watched Word settings</a>, or disable the 'watched words regular expressions' site setting."

site_settings:
disabled: "disabled"
display_local_time_in_user_card: "Display the local time based on a user's timezone when their user card is opened."
censored_words: "Words that will be automatically replaced with &#9632;&#9632;&#9632;&#9632;"
delete_old_hidden_posts: "Auto-delete any hidden posts that stay hidden for more than 30 days."
Expand Down
3 changes: 2 additions & 1 deletion config/site_settings.yml
Original file line number Diff line number Diff line change
Expand Up @@ -1365,7 +1365,8 @@ files:
automatically_download_gravatars: true
allow_uploaded_avatars:
client: true
default: true
default: "0"
enum: "TrustLevelAndStaffAndDisabledSetting"
default_avatars:
default: ""
type: url_list
Expand Down
19 changes: 19 additions & 0 deletions db/migrate/20210819152920_change_allow_uploaded_avatars.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,19 @@
# frozen_string_literal: true

class ChangeAllowUploadedAvatars < ActiveRecord::Migration[6.1]
def up
execute <<~SQL
UPDATE site_settings
SET data_type = 7, value = (CASE WHEN value = 'f' THEN 'disabled' ELSE '0' END)
WHERE name = 'allow_uploaded_avatars'
SQL
end

def down
execute <<~SQL
UPDATE site_settings
SET data_type = 5, value = (CASE WHEN value = 'disabled' THEN 'f' ELSE 't' END)
WHERE name = 'allow_uploaded_avatars'
SQL
end
end
2 changes: 1 addition & 1 deletion lib/composer_messages_finder.rb
Original file line number Diff line number Diff line change
Expand Up @@ -78,7 +78,7 @@ def check_avatar_notification
# - "disable avatar education message" is enabled
# - "sso overrides avatar" is enabled
# - "allow uploaded avatars" is disabled
return if SiteSetting.disable_avatar_education_message || SiteSetting.discourse_connect_overrides_avatar || !SiteSetting.allow_uploaded_avatars
return if SiteSetting.disable_avatar_education_message || SiteSetting.discourse_connect_overrides_avatar || !TrustLevelAndStaffAndDisabledSetting.matches?(SiteSetting.allow_uploaded_avatars, @user)

# If we got this far, log that we've nagged them about the avatar
UserHistory.create!(action: UserHistory.actions[:notified_about_avatar], target_user_id: @user.id)
Expand Down
2 changes: 1 addition & 1 deletion spec/components/composer_messages_finder_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -144,7 +144,7 @@
end

it "doesn't notify users if 'allow_uploaded_avatars' setting is disabled" do
SiteSetting.allow_uploaded_avatars = false
SiteSetting.allow_uploaded_avatars = 'disabled'
expect(finder.check_avatar_notification).to be_blank
end
end
Expand Down
4 changes: 2 additions & 2 deletions spec/requests/uploads_controller_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -126,7 +126,7 @@
end

it 'ensures allow_uploaded_avatars is enabled when uploading an avatar' do
SiteSetting.allow_uploaded_avatars = false
SiteSetting.allow_uploaded_avatars = 'disabled'
post "/uploads.json", params: { file: logo, type: "avatar" }
expect(response.status).to eq(422)
end
Expand All @@ -139,7 +139,7 @@

it 'always allows admins to upload avatars' do
sign_in(Fabricate(:admin))
SiteSetting.allow_uploaded_avatars = false
SiteSetting.allow_uploaded_avatars = 'disabled'

post "/uploads.json", params: { file: logo, type: "avatar" }
expect(response.status).to eq(200)
Expand Down
48 changes: 45 additions & 3 deletions spec/requests/users_controller_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -2483,16 +2483,58 @@ def post_user(extra_params = {})
end

it "raises an error when selecting the custom/uploaded avatar and allow_uploaded_avatars is disabled" do
SiteSetting.allow_uploaded_avatars = false
SiteSetting.allow_uploaded_avatars = 'disabled'
put "/u/#{user.username}/preferences/avatar/pick.json", params: {
upload_id: upload.id, type: "custom"
}

expect(response.status).to eq(422)
end

it "raises an error when selecting the custom/uploaded avatar and allow_uploaded_avatars is admin" do
SiteSetting.allow_uploaded_avatars = 'admin'
put "/u/#{user.username}/preferences/avatar/pick.json", params: {
upload_id: upload.id, type: "custom"
}
expect(response.status).to eq(422)

user.update!(admin: true)
put "/u/#{user.username}/preferences/avatar/pick.json", params: {
upload_id: upload.id, type: "custom"
}
expect(response.status).to eq(200)
end

it "raises an error when selecting the custom/uploaded avatar and allow_uploaded_avatars is staff" do
SiteSetting.allow_uploaded_avatars = 'staff'
put "/u/#{user.username}/preferences/avatar/pick.json", params: {
upload_id: upload.id, type: "custom"
}
expect(response.status).to eq(422)

user.update!(moderator: true)
put "/u/#{user.username}/preferences/avatar/pick.json", params: {
upload_id: upload.id, type: "custom"
}
expect(response.status).to eq(200)
end

it "raises an error when selecting the custom/uploaded avatar and allow_uploaded_avatars is a trust level" do
SiteSetting.allow_uploaded_avatars = '3'
put "/u/#{user.username}/preferences/avatar/pick.json", params: {
upload_id: upload.id, type: "custom"
}
expect(response.status).to eq(422)

user.update!(trust_level: 3)
put "/u/#{user.username}/preferences/avatar/pick.json", params: {
upload_id: upload.id, type: "custom"
}
expect(response.status).to eq(200)
end

it 'ignores the upload if picking a system avatar' do
SiteSetting.allow_uploaded_avatars = false
SiteSetting.allow_uploaded_avatars = 'disabled'
another_upload = Fabricate(:upload)

put "/u/#{user.username}/preferences/avatar/pick.json", params: {
Expand All @@ -2504,7 +2546,7 @@ def post_user(extra_params = {})
end

it 'raises an error if the type is invalid' do
SiteSetting.allow_uploaded_avatars = false
SiteSetting.allow_uploaded_avatars = 'disabled'
another_upload = Fabricate(:upload)

put "/u/#{user.username}/preferences/avatar/pick.json", params: {
Expand Down

0 comments on commit ff367e2

Please sign in to comment.