Merge pull request #1065 from publify/fix-admin-article-access-control

Fix admin article access control

publify_core/app/controllers/admin/content_controller.rb

@@ -58,9 +58,9 @@ def create
   end
 
   def update
-    return unless access_granted?(params[:id])
+    id = params[:id]
+    return unless access_granted?(id)
 
-    id = params[:article][:id] || params[:id]
     @article = Article.find(id)
 
     if params[:article][:draft]
@@ -101,6 +101,7 @@ def autosave
     return false unless request.xhr?
 
     id = params[:article][:id] || params[:id]
+    return if id && !access_granted?(id)
 
     article_factory = Article::Factory.new(this_blog, current_user)
     @article = article_factory.get_or_build_from(id)

publify_core/spec/controllers/admin/content_controller_spec.rb

@@ -71,7 +71,7 @@
       sign_in publisher
     end
 
-    context "first time save" do
+    context "when saving a draft article for the first time" do
       it "creates a new draft Article" do
         expect do
           post :autosave, xhr: true, params: { article: attributes_for(:article) }
@@ -86,8 +86,8 @@
       end
     end
 
-    context "second call to save" do
-      let!(:draft) { create(:article, state: "draft") }
+    context "when updating your own existing draft article" do
+      let!(:draft) { create(:article, state: "draft", user: publisher) }
 
       it "does not create an extra draft" do
         expect do
@@ -96,9 +96,27 @@
                                                body_and_extended: "new body" } }
         end.not_to change(Article, :count)
       end
+
+      it "updates the existing draft" do
+        post :autosave,
+             xhr: true, params: { article: { id: draft.id,
+                                             body_and_extended: "new body" } }
+        expect(draft.reload.body).to eq "new body"
+      end
+    end
+
+    context "when updating another user's existing draft article" do
+      let!(:draft) { create(:article, state: "draft") }
+
+      it "does not update the existing draft" do
+        post :autosave,
+             xhr: true, params: { article: { id: draft.id,
+                                             body_and_extended: "new body" } }
+        expect(draft.reload.body).not_to eq "new body"
+      end
     end
 
-    context "with an other existing draft" do
+    context "with saving a new draft article and an existing other existing draft" do
       let!(:draft) { create(:article, state: "draft", body: "existing body") }
 
       it "creates a new draft Article" do
@@ -527,6 +545,26 @@ def base_article(options = {})
         it { expect(article.reload.text_filter.name).to eq("textile") }
         it { expect(article.reload.body).to eq(body) }
       end
+
+      context "with an owned article and another user's article" do
+        let(:article) { create(:article, body: "another *textile* test", user: publisher) }
+        let(:other_article) { create(:article, body: "other article") }
+        let(:body) { "not the *same* text" }
+
+        before do
+          put :update,
+              params: { id: article.id,
+                        article: { id: other_article.id, body: body } }
+        end
+
+        it "ignores the extra id passed in the article parameters" do
+          aggregate_failures do
+            expect(response).to redirect_to(action: "index")
+            expect(article.reload.body).to eq(body)
+            expect(other_article.reload.body).not_to eq(body)
+          end
+        end
+      end
     end
   end