FEATURE: Allow drafts to be deleted via the API (#21148)

This PR adds the ability to destroy drafts for a passed user via the API. This was not possible before as this action was reserved for only your personal drafts.

If a user is an admin and calls the `#destroy` action from the API they are able to destroy a draft for a passed user. A user can be targeted by passed either their:
- username
- external_id (for SSO) 

to the request.

In the case you attempt to destroy a non-personal draft and
- You are not an admin
- You do not access the `#destroy` action via the API

you will raise a `Discourse::InvalidAccess` (403) and will not succeed in destroying the draft.

app/controllers/drafts_controller.rb

@@ -87,11 +87,25 @@ class DraftsController < ApplicationController
   end
 
   def destroy
+    user =
+      if is_api?
+        if @guardian.is_admin?
+          fetch_user_from_params
+        else
+          raise Discourse::InvalidAccess
+        end
+      else
+        current_user
+      end
+
     begin
-      Draft.clear(current_user, params[:id], params[:sequence].to_i)
-    rescue Draft::OutOfSequence
-      # nothing really we can do here, if try clearing a draft that is not ours, just skip it.
+      Draft.clear(user, params[:id], params[:sequence].to_i)
+    rescue Draft::OutOfSequence => e
+      return render json: failed_json.merge(errors: e), status: 404
+    rescue StandardError => e
+      return render json: failed_json.merge(errors: e), status: 401
     end
+
     render json: success_json
   end
 end

app/models/draft.rb

@@ -134,12 +134,14 @@ class Draft < ActiveRecord::Base
   end
 
   def self.clear(user, key, sequence)
-    return if !user || !user.id || !User.human_user_id?(user.id)
+    if !user || !user.id || !User.human_user_id?(user.id)
+      raise StandardError.new("user not present")
+    end
 
     current_sequence = DraftSequence.current(user, key)
 
     # bad caller is a reason to complain
-    raise Draft::OutOfSequence if sequence != current_sequence
+    raise Draft::OutOfSequence.new("bad draft sequence") if sequence != current_sequence
 
     # corrupt data is not a reason not to leave data
     Draft.where(user_id: user.id, draft_key: key).destroy_all

spec/requests/drafts_controller_spec.rb

@@ -233,8 +233,88 @@ RSpec.describe DraftsController do
       user = sign_in(Fabricate(:user))
       Draft.set(user, "xxx", 0, "hi")
       delete "/drafts/xxx.json", params: { sequence: 0 }
+
       expect(response.status).to eq(200)
       expect(Draft.get(user, "xxx", 0)).to eq(nil)
     end
+
+    it "denies attempts to destroy unowned draft" do
+      sign_in(Fabricate(:admin))
+      user = Fabricate(:user)
+      Draft.set(user, "xxx", 0, "hi")
+      delete "/drafts/xxx.json", params: { sequence: 0, username: user.username }
+
+      # Draft is not deleted because request is not via API
+      expect(Draft.get(user, "xxx", 0)).to be_present
+    end
+
+    it "raises bad sequence" do
+      user = sign_in(Fabricate(:user))
+      delete "/drafts/xxx.json", params: { sequence: 1 }
+
+      expect(response.status).to eq(404)
+      expect(response.parsed_body["errors"]).to eq("bad draft sequence")
+    end
+
+    shared_examples "for a passed user" do
+      it "deletes draft" do
+        api_key = Fabricate(:api_key).key
+        Draft.set(recipient, "xxx", 0, "hi")
+
+        delete "/drafts/xxx.json",
+               params: {
+                 sequence: 0,
+                 username: recipient.username,
+               },
+               headers: {
+                 HTTP_API_USERNAME: caller.username,
+                 HTTP_API_KEY: api_key,
+               }
+
+        expect(response.status).to eq(response_code)
+
+        if draft_deleted
+          expect(Draft.get(recipient, "xxx", 0)).to eq(nil)
+        else
+          expect(Draft.get(recipient, "xxx", 0)).to be_present
+        end
+      end
+    end
+
+    describe "api called by admin" do
+      include_examples "for a passed user" do
+        let(:caller) { Fabricate(:admin) }
+        let(:recipient) { Fabricate(:user) }
+        let(:response_code) { 200 }
+        let(:draft_deleted) { true }
+      end
+    end
+
+    describe "api called by tl4 user" do
+      include_examples "for a passed user" do
+        let(:caller) { Fabricate(:trust_level_4) }
+        let(:recipient) { Fabricate(:user) }
+        let(:response_code) { 403 }
+        let(:draft_deleted) { false }
+      end
+    end
+
+    describe "api called by regular user" do
+      include_examples "for a passed user" do
+        let(:caller) { Fabricate(:user) }
+        let(:recipient) { Fabricate(:user) }
+        let(:response_code) { 403 }
+        let(:draft_deleted) { false }
+      end
+    end
+
+    describe "api called by admin for another admin" do
+      include_examples "for a passed user" do
+        let(:caller) { Fabricate(:admin) }
+        let(:recipient) { Fabricate(:admin) }
+        let(:response_code) { 200 }
+        let(:draft_deleted) { true }
+      end
+    end
   end
 end