FIX: Validate type when picking an avatar. (#11602)

This change improves the "UsersController#pick_avatar" validations to raise an error when "allow_uploaded_avatars" is disabled.

app/assets/javascripts/discourse/app/templates/modal/avatar-selector.hbs

@@ -12,6 +12,7 @@
       {{radio-button id="system-avatar" name="avatar" value="system" selection=selected}}
       <label class="radio" for="system-avatar">{{bound-avatar-template user.system_avatar_template "large"}} {{html-safe (i18n "user.change_avatar.letter_based")}}</label>
     </div>
+    {{#if allowAvatarUpload}}
       <div class="avatar-choice">
         {{radio-button id="gravatar" name="avatar" value="gravatar" selection=selected}}
         <label class="radio" for="gravatar">{{bound-avatar-template user.gravatar_avatar_template "large"}} <span>{{html-safe (i18n "user.change_avatar.gravatar" gravatarName=gravatarName gravatarBaseUrl=gravatarBaseUrl gravatarLoginUrl=gravatarLoginUrl)}} {{user.email}}</span></label>
@@ -26,7 +27,6 @@
           <p class="error">{{I18n "user.change_avatar.gravatar_failed" gravatarName=gravatarName}}</p>
         {{/if}}
       </div>
-    {{#if allowAvatarUpload}}
       <div class="avatar-choice">
         {{radio-button id="uploaded-avatar" name="avatar" value="uploaded" selection=selected}}
         <label class="radio" for="uploaded-avatar">

app/controllers/users_controller.rb

@@ -1108,33 +1108,37 @@ class UsersController < ApplicationController
     user = fetch_user_from_params
     guardian.ensure_can_edit!(user)
 
-    type = params[:type]
-    upload_id = params[:upload_id]
-
     if SiteSetting.sso_overrides_avatar
       return render json: failed_json, status: 422
     end
 
-    if !SiteSetting.allow_uploaded_avatars
+    type = params[:type]
-      if type == "uploaded" || type == "custom"
+
+    invalid_type = type.present? && !AVATAR_TYPES_WITH_UPLOAD.include?(type) && type != 'system'
+    if invalid_type
       return render json: failed_json, status: 422
     end
+
+    if type.blank? || type == 'system'
+      upload_id = nil
+    else
+      if !SiteSetting.allow_uploaded_avatars
+        return render json: failed_json, status: 422
       end
 
+      upload_id = params[:upload_id]
       upload = Upload.find_by(id: upload_id)
 
+      if upload.nil?
+        return render_json_error I18n.t('avatar.missing')
+      end
+
       # old safeguard
       user.create_user_avatar unless user.user_avatar
 
       guardian.ensure_can_pick_avatar!(user.user_avatar, upload)
 
-    if AVATAR_TYPES_WITH_UPLOAD.include?(type)
+      if type == 'gravatar'
-
-      if !upload
-        return render_json_error I18n.t("avatar.missing")
-      end
-
-      if type == "gravatar"
         user.user_avatar.gravatar_upload_id = upload_id
       else
         user.user_avatar.custom_upload_id = upload_id

spec/requests/users_controller_spec.rb

@@ -2309,6 +2309,29 @@ describe UsersController do
         expect(response.status).to eq(422)
       end
 
+      it 'ignores the upload if picking a system avatar' do
+        SiteSetting.allow_uploaded_avatars = false
+        another_upload = Fabricate(:upload)
+
+        put "/u/#{user.username}/preferences/avatar/pick.json", params: {
+          upload_id: another_upload.id, type: "system"
+        }
+
+        expect(response.status).to eq(200)
+        expect(user.reload.uploaded_avatar_id).to eq(nil)
+      end
+
+      it 'raises an error if the type is invalid' do
+        SiteSetting.allow_uploaded_avatars = false
+        another_upload = Fabricate(:upload)
+
+        put "/u/#{user.username}/preferences/avatar/pick.json", params: {
+          upload_id: another_upload.id, type: "x"
+        }
+
+        expect(response.status).to eq(422)
+      end
+
       it 'can successfully pick the system avatar' do
         put "/u/#{user.username}/preferences/avatar/pick.json"