Remove API authentication for public statuses (after review) (#1919)

app/controllers/api/v1/statuses_controller.rb

@@ -1,7 +1,7 @@
 # frozen_string_literal: true
 
 class Api::V1::StatusesController < ApiController
-  before_action -> { doorkeeper_authorize! :read }, except: [:create, :destroy, :reblog, :unreblog, :favourite, :unfavourite]
+  before_action :authorize_if_got_token, except:            [:create, :destroy, :reblog, :unreblog, :favourite, :unfavourite]
   before_action -> { doorkeeper_authorize! :write }, only:  [:create, :destroy, :reblog, :unreblog, :favourite, :unfavourite]
   before_action :require_user!, except: [:show, :context, :card, :reblogged_by, :favourited_by]
   before_action :set_status, only:      [:show, :context, :card, :reblogged_by, :favourited_by]
@@ -114,4 +114,9 @@ class Api::V1::StatusesController < ApiController
   def pagination_params(core_params)
     params.permit(:limit).merge(core_params)
   end
+
+  def authorize_if_got_token
+    request_token = Doorkeeper::OAuth::Token.from_request(request, *Doorkeeper.configuration.access_token_methods)
+    doorkeeper_authorize! :read if request_token
+  end
 end

spec/controllers/api/v1/statuses_controller_spec.rb

@@ -7,179 +7,289 @@ RSpec.describe Api::V1::StatusesController, type: :controller do
   let(:app)   { Fabricate(:application, name: 'Test app', website: 'http://testapp.com') }
   let(:token) { double acceptable?: true, resource_owner_id: user.id, application: app }
 
-  before do
-    allow(controller).to receive(:doorkeeper_token) { token }
-  end
+  context 'with an oauth token' do
+    before do
+      allow(controller).to receive(:doorkeeper_token) { token }
+    end
 
-  describe 'GET #show' do
-    let(:status) { Fabricate(:status, account: user.account) }
+    describe 'GET #show' do
+      let(:status) { Fabricate(:status, account: user.account) }
 
-    it 'returns http success' do
-      get :show, params: { id: status.id }
-      expect(response).to have_http_status(:success)
+      it 'returns http success' do
+        get :show, params: { id: status.id }
+        expect(response).to have_http_status(:success)
+      end
     end
-  end
 
-  describe 'GET #context' do
-    let(:status) { Fabricate(:status, account: user.account) }
+    describe 'GET #context' do
+      let(:status) { Fabricate(:status, account: user.account) }
 
-    before do
-      Fabricate(:status, account: user.account, thread: status)
-    end
+      before do
+        Fabricate(:status, account: user.account, thread: status)
+      end
 
-    it 'returns http success' do
-      get :context, params: { id: status.id }
-      expect(response).to have_http_status(:success)
+      it 'returns http success' do
+        get :context, params: { id: status.id }
+        expect(response).to have_http_status(:success)
+      end
     end
-  end
 
-  describe 'GET #reblogged_by' do
-    let(:status) { Fabricate(:status, account: user.account) }
+    describe 'GET #reblogged_by' do
+      let(:status) { Fabricate(:status, account: user.account) }
 
-    before do
-      post :reblog, params: { id: status.id }
-    end
+      before do
+        post :reblog, params: { id: status.id }
+      end
 
-    it 'returns http success' do
-      get :reblogged_by, params: { id: status.id }
-      expect(response).to have_http_status(:success)
+      it 'returns http success' do
+        get :reblogged_by, params: { id: status.id }
+        expect(response).to have_http_status(:success)
+      end
     end
-  end
 
-  describe 'GET #favourited_by' do
-    let(:status) { Fabricate(:status, account: user.account) }
+    describe 'GET #favourited_by' do
+      let(:status) { Fabricate(:status, account: user.account) }
 
-    before do
-      post :favourite, params: { id: status.id }
-    end
+      before do
+        post :favourite, params: { id: status.id }
+      end
 
-    it 'returns http success' do
-      get :favourited_by, params: { id: status.id }
-      expect(response).to have_http_status(:success)
+      it 'returns http success' do
+        get :favourited_by, params: { id: status.id }
+        expect(response).to have_http_status(:success)
+      end
     end
-  end
 
-  describe 'POST #create' do
-    before do
-      post :create, params: { status: 'Hello world' }
-    end
+    describe 'POST #create' do
+      before do
+        post :create, params: { status: 'Hello world' }
+      end
 
-    it 'returns http success' do
-      expect(response).to have_http_status(:success)
+      it 'returns http success' do
+        expect(response).to have_http_status(:success)
+      end
     end
-  end
 
-  describe 'DELETE #destroy' do
-    let(:status) { Fabricate(:status, account: user.account) }
+    describe 'DELETE #destroy' do
+      let(:status) { Fabricate(:status, account: user.account) }
 
-    before do
-      post :destroy, params: { id: status.id }
-    end
+      before do
+        post :destroy, params: { id: status.id }
+      end
 
-    it 'returns http success' do
-      expect(response).to have_http_status(:success)
-    end
+      it 'returns http success' do
+        expect(response).to have_http_status(:success)
+      end
 
-    it 'removes the status' do
-      expect(Status.find_by(id: status.id)).to be nil
+      it 'removes the status' do
+        expect(Status.find_by(id: status.id)).to be nil
+      end
     end
-  end
 
-  describe 'POST #reblog' do
-    let(:status) { Fabricate(:status, account: user.account) }
+    describe 'POST #reblog' do
+      let(:status) { Fabricate(:status, account: user.account) }
 
-    before do
-      post :reblog, params: { id: status.id }
-    end
+      before do
+        post :reblog, params: { id: status.id }
+      end
 
-    it 'returns http success' do
-      expect(response).to have_http_status(:success)
-    end
+      it 'returns http success' do
+        expect(response).to have_http_status(:success)
+      end
 
-    it 'updates the reblogs count' do
-      expect(status.reblogs.count).to eq 1
-    end
+      it 'updates the reblogs count' do
+        expect(status.reblogs.count).to eq 1
+      end
 
-    it 'updates the reblogged attribute' do
-      expect(user.account.reblogged?(status)).to be true
-    end
+      it 'updates the reblogged attribute' do
+        expect(user.account.reblogged?(status)).to be true
+      end
 
-    it 'return json with updated attributes' do
-      hash_body = body_as_json
+      it 'return json with updated attributes' do
+        hash_body = body_as_json
 
-      expect(hash_body[:reblog][:id]).to eq status.id
-      expect(hash_body[:reblog][:reblogs_count]).to eq 1
-      expect(hash_body[:reblog][:reblogged]).to be true
+        expect(hash_body[:reblog][:id]).to eq status.id
+        expect(hash_body[:reblog][:reblogs_count]).to eq 1
+        expect(hash_body[:reblog][:reblogged]).to be true
+      end
     end
-  end
 
-  describe 'POST #unreblog' do
-    let(:status) { Fabricate(:status, account: user.account) }
+    describe 'POST #unreblog' do
+      let(:status) { Fabricate(:status, account: user.account) }
 
-    before do
-      post :reblog,   params: { id: status.id }
-      post :unreblog, params: { id: status.id }
-    end
+      before do
+        post :reblog,   params: { id: status.id }
+        post :unreblog, params: { id: status.id }
+      end
 
-    it 'returns http success' do
-      expect(response).to have_http_status(:success)
-    end
+      it 'returns http success' do
+        expect(response).to have_http_status(:success)
+      end
 
-    it 'updates the reblogs count' do
-      expect(status.reblogs.count).to eq 0
-    end
+      it 'updates the reblogs count' do
+        expect(status.reblogs.count).to eq 0
+      end
 
-    it 'updates the reblogged attribute' do
-      expect(user.account.reblogged?(status)).to be false
+      it 'updates the reblogged attribute' do
+        expect(user.account.reblogged?(status)).to be false
+      end
     end
-  end
 
-  describe 'POST #favourite' do
-    let(:status) { Fabricate(:status, account: user.account) }
+    describe 'POST #favourite' do
+      let(:status) { Fabricate(:status, account: user.account) }
 
-    before do
-      post :favourite, params: { id: status.id }
-    end
+      before do
+        post :favourite, params: { id: status.id }
+      end
 
-    it 'returns http success' do
-      expect(response).to have_http_status(:success)
-    end
+      it 'returns http success' do
+        expect(response).to have_http_status(:success)
+      end
 
-    it 'updates the favourites count' do
-      expect(status.favourites.count).to eq 1
-    end
+      it 'updates the favourites count' do
+        expect(status.favourites.count).to eq 1
+      end
 
-    it 'updates the favourited attribute' do
-      expect(user.account.favourited?(status)).to be true
-    end
+      it 'updates the favourited attribute' do
+        expect(user.account.favourited?(status)).to be true
+      end
 
-    it 'return json with updated attributes' do
-      hash_body = body_as_json
+      it 'return json with updated attributes' do
+        hash_body = body_as_json
 
-      expect(hash_body[:id]).to eq status.id
-      expect(hash_body[:favourites_count]).to eq 1
-      expect(hash_body[:favourited]).to be true
+        expect(hash_body[:id]).to eq status.id
+        expect(hash_body[:favourites_count]).to eq 1
+        expect(hash_body[:favourited]).to be true
+      end
     end
-  end
 
-  describe 'POST #unfavourite' do
-    let(:status) { Fabricate(:status, account: user.account) }
+    describe 'POST #unfavourite' do
+      let(:status) { Fabricate(:status, account: user.account) }
 
-    before do
-      post :favourite,   params: { id: status.id }
-      post :unfavourite, params: { id: status.id }
+      before do
+        post :favourite,   params: { id: status.id }
+        post :unfavourite, params: { id: status.id }
+      end
+
+      it 'returns http success' do
+        expect(response).to have_http_status(:success)
+      end
+
+      it 'updates the favourites count' do
+        expect(status.favourites.count).to eq 0
+      end
+
+      it 'updates the favourited attribute' do
+        expect(user.account.favourited?(status)).to be false
+      end
     end
+  end
 
-    it 'returns http success' do
-      expect(response).to have_http_status(:success)
+  context 'without an oauth token' do
+    before do
+      allow(controller).to receive(:doorkeeper_token) { nil }
     end
 
-    it 'updates the favourites count' do
-      expect(status.favourites.count).to eq 0
+    context 'with a private status' do
+      let(:status) { Fabricate(:status, account: user.account, visibility: :private) }
+
+      describe 'GET #show' do
+        it 'returns http unautharized' do
+          get :show, params: { id: status.id }
+          expect(response).to have_http_status(:missing)
+        end
+      end
+
+      describe 'GET #context' do
+        before do
+          Fabricate(:status, account: user.account, thread: status)
+        end
+
+        it 'returns http unautharized' do
+          get :context, params: { id: status.id }
+          expect(response).to have_http_status(:missing)
+        end
+      end
+
+      describe 'GET #card' do
+        it 'returns http unautharized' do
+          get :card, params: { id: status.id }
+          expect(response).to have_http_status(:missing)
+        end
+      end
+
+      describe 'GET #reblogged_by' do
+        before do
+          post :reblog, params: { id: status.id }
+        end
+
+        it 'returns http unautharized' do
+          get :reblogged_by, params: { id: status.id }
+          expect(response).to have_http_status(:missing)
+        end
+      end
+
+      describe 'GET #favourited_by' do
+        before do
+          post :favourite, params: { id: status.id }
+        end
+
+        it 'returns http unautharized' do
+          get :favourited_by, params: { id: status.id }
+          expect(response).to have_http_status(:missing)
+        end
+      end
     end
 
-    it 'updates the favourited attribute' do
-      expect(user.account.favourited?(status)).to be false
+    context 'with a public status' do
+      let(:status) { Fabricate(:status, account: user.account, visibility: :public) }
+
+      describe 'GET #show' do
+        it 'returns http success' do
+          get :show, params: { id: status.id }
+          expect(response).to have_http_status(:success)
+        end
+      end
+
+      describe 'GET #context' do
+        before do
+          Fabricate(:status, account: user.account, thread: status)
+        end
+
+        it 'returns http success' do
+          get :context, params: { id: status.id }
+          expect(response).to have_http_status(:success)
+        end
+      end
+
+      describe 'GET #card' do
+        it 'returns http success' do
+          get :card, params: { id: status.id }
+          expect(response).to have_http_status(:success)
+        end
+      end
+
+      describe 'GET #reblogged_by' do
+        before do
+          post :reblog, params: { id: status.id }
+        end
+
+        it 'returns http success' do
+          get :reblogged_by, params: { id: status.id }
+          expect(response).to have_http_status(:success)
+        end
+      end
+
+      describe 'GET #favourited_by' do
+        before do
+          post :favourite, params: { id: status.id }
+        end
+
+        it 'returns http success' do
+          get :favourited_by, params: { id: status.id }
+          expect(response).to have_http_status(:success)
+        end
+      end
     end
   end
 end