Browse Source

Fix leak of arbitrary statuses through unfavourite action in REST API (#13161)

Eugen Rochko 1 month ago
parent
commit
0c28a505dd
No account linked to committer's email address

+ 10
- 17
app/controllers/api/v1/statuses/bookmarks_controller.rb View File

@@ -5,35 +5,28 @@ class Api::V1::Statuses::BookmarksController < Api::BaseController
5 5
 
6 6
   before_action -> { doorkeeper_authorize! :write, :'write:bookmarks' }
7 7
   before_action :require_user!
8
+  before_action :set_status
8 9
 
9 10
   respond_to :json
10 11
 
11 12
   def create
12
-    @status = bookmarked_status
13
+    current_account.bookmarks.find_or_create_by!(account: current_account, status: @status)
13 14
     render json: @status, serializer: REST::StatusSerializer
14 15
   end
15 16
 
16 17
   def destroy
17
-    @status = requested_status
18
-    @bookmarks_map = { @status.id => false }
18
+    bookmark = current_account.bookmarks.find_by(status: @status)
19
+    bookmark&.destroy!
19 20
 
20
-    bookmark = Bookmark.find_by!(account: current_user.account, status: @status)
21
-    bookmark.destroy!
22
-
23
-    render json: @status, serializer: REST::StatusSerializer, relationships: StatusRelationshipsPresenter.new([@status], current_user&.account_id, bookmarks_map: @bookmarks_map)
21
+    render json: @status, serializer: REST::StatusSerializer, relationships: StatusRelationshipsPresenter.new([@status], current_account.id, bookmarks_map: { @status.id => false })
24 22
   end
25 23
 
26 24
   private
27 25
 
28
-  def bookmarked_status
29
-    authorize_with current_user.account, requested_status, :show?
30
-
31
-    bookmark = Bookmark.find_or_create_by!(account: current_user.account, status: requested_status)
32
-
33
-    bookmark.status.reload
34
-  end
35
-
36
-  def requested_status
37
-    Status.find(params[:status_id])
26
+  def set_status
27
+    @status = Status.find(params[:status_id])
28
+    authorize @status, :show?
29
+  rescue Mastodon::NotPermittedError
30
+    not_found
38 31
   end
39 32
 end

+ 1
- 2
app/controllers/api/v1/statuses/favourited_by_accounts_controller.rb View File

@@ -69,8 +69,7 @@ class Api::V1::Statuses::FavouritedByAccountsController < Api::BaseController
69 69
     @status = Status.find(params[:status_id])
70 70
     authorize @status, :show?
71 71
   rescue Mastodon::NotPermittedError
72
-    # Reraise in order to get a 404 instead of a 403 error code
73
-    raise ActiveRecord::RecordNotFound
72
+    not_found
74 73
   end
75 74
 
76 75
   def pagination_params(core_params)

+ 9
- 17
app/controllers/api/v1/statuses/favourites_controller.rb View File

@@ -5,34 +5,26 @@ class Api::V1::Statuses::FavouritesController < Api::BaseController
5 5
 
6 6
   before_action -> { doorkeeper_authorize! :write, :'write:favourites' }
7 7
   before_action :require_user!
8
+  before_action :set_status
8 9
 
9 10
   respond_to :json
10 11
 
11 12
   def create
12
-    @status = favourited_status
13
+    FavouriteService.new.call(current_account, @status)
13 14
     render json: @status, serializer: REST::StatusSerializer
14 15
   end
15 16
 
16 17
   def destroy
17
-    @status = requested_status
18
-    @favourites_map = { @status.id => false }
19
-
20
-    UnfavouriteWorker.perform_async(current_user.account_id, @status.id)
21
-
22
-    render json: @status, serializer: REST::StatusSerializer, relationships: StatusRelationshipsPresenter.new([@status], current_user&.account_id, favourites_map: @favourites_map)
18
+    UnfavouriteWorker.perform_async(current_account.id, @status.id)
19
+    render json: @status, serializer: REST::StatusSerializer, relationships: StatusRelationshipsPresenter.new([@status], current_account.id, favourites_map: { @status.id => false })
23 20
   end
24 21
 
25 22
   private
26 23
 
27
-  def favourited_status
28
-    service_result.status.reload
29
-  end
30
-
31
-  def service_result
32
-    FavouriteService.new.call(current_user.account, requested_status)
33
-  end
34
-
35
-  def requested_status
36
-    Status.find(params[:status_id])
24
+  def set_status
25
+    @status = Status.find(params[:status_id])
26
+    authorize @status, :show?
27
+  rescue Mastodon::NotPermittedError
28
+    not_found
37 29
   end
38 30
 end

+ 1
- 2
app/controllers/api/v1/statuses/reblogged_by_accounts_controller.rb View File

@@ -66,8 +66,7 @@ class Api::V1::Statuses::RebloggedByAccountsController < Api::BaseController
66 66
     @status = Status.find(params[:status_id])
67 67
     authorize @status, :show?
68 68
   rescue Mastodon::NotPermittedError
69
-    # Reraise in order to get a 404 instead of a 403 error code
70
-    raise ActiveRecord::RecordNotFound
69
+    not_found
71 70
   end
72 71
 
73 72
   def pagination_params(core_params)

+ 14
- 13
app/controllers/api/v1/statuses/reblogs_controller.rb View File

@@ -5,33 +5,34 @@ class Api::V1::Statuses::ReblogsController < Api::BaseController
5 5
 
6 6
   before_action -> { doorkeeper_authorize! :write, :'write:statuses' }
7 7
   before_action :require_user!
8
+  before_action :set_reblog
8 9
 
9 10
   respond_to :json
10 11
 
11 12
   def create
12
-    @status = ReblogService.new.call(current_user.account, status_for_reblog, reblog_params)
13
+    @status = ReblogService.new.call(current_account, @reblog, reblog_params)
13 14
     render json: @status, serializer: REST::StatusSerializer
14 15
   end
15 16
 
16 17
   def destroy
17
-    @status = status_for_destroy.reblog
18
-    @reblogs_map = { @status.id => false }
18
+    @status = current_account.statuses.find_by(reblog_of_id: @reblog.id)
19 19
 
20
-    authorize status_for_destroy, :unreblog?
21
-    status_for_destroy.discard
22
-    RemovalWorker.perform_async(status_for_destroy.id)
20
+    if @status
21
+      authorize @status, :unreblog?
22
+      @status.discard
23
+      RemovalWorker.perform_async(@status.id)
24
+    end
23 25
 
24
-    render json: @status, serializer: REST::StatusSerializer, relationships: StatusRelationshipsPresenter.new([@status], current_user&.account_id, reblogs_map: @reblogs_map)
26
+    render json: @reblog, serializer: REST::StatusSerializer, relationships: StatusRelationshipsPresenter.new([@status], current_account.id, reblogs_map: { @reblog.id => false })
25 27
   end
26 28
 
27 29
   private
28 30
 
29
-  def status_for_reblog
30
-    Status.find params[:status_id]
31
-  end
32
-
33
-  def status_for_destroy
34
-    @status_for_destroy ||= current_user.account.statuses.where(reblog_of_id: params[:status_id]).first!
31
+  def set_reblog
32
+    @reblog = Status.find(params[:status_id])
33
+    authorize @reblog, :show?
34
+  rescue Mastodon::NotPermittedError
35
+    not_found
35 36
   end
36 37
 
37 38
   def reblog_params

+ 49
- 18
spec/controllers/api/v1/statuses/bookmarks_controller_spec.rb View File

@@ -21,36 +21,67 @@ describe Api::V1::Statuses::BookmarksController do
21 21
         post :create, params: { status_id: status.id }
22 22
       end
23 23
 
24
-      it 'returns http success' do
25
-        expect(response).to have_http_status(:success)
26
-      end
24
+      context 'with public status' do
25
+        it 'returns http success' do
26
+          expect(response).to have_http_status(:success)
27
+        end
28
+
29
+        it 'updates the bookmarked attribute' do
30
+          expect(user.account.bookmarked?(status)).to be true
31
+        end
32
+
33
+        it 'returns json with updated attributes' do
34
+          hash_body = body_as_json
27 35
 
28
-      it 'updates the bookmarked attribute' do
29
-        expect(user.account.bookmarked?(status)).to be true
36
+          expect(hash_body[:id]).to eq status.id.to_s
37
+          expect(hash_body[:bookmarked]).to be true
38
+        end
30 39
       end
31 40
 
32
-      it 'return json with updated attributes' do
33
-        hash_body = body_as_json
41
+      context 'with private status of not-followed account' do
42
+        let(:status) { Fabricate(:status, visibility: :private) }
34 43
 
35
-        expect(hash_body[:id]).to eq status.id.to_s
36
-        expect(hash_body[:bookmarked]).to be true
44
+        it 'returns http not found' do
45
+          expect(response).to have_http_status(404)
46
+        end
37 47
       end
38 48
     end
39 49
 
40 50
     describe 'POST #destroy' do
41
-      let(:status) { Fabricate(:status, account: user.account) }
51
+      context 'with public status' do
52
+        let(:status) { Fabricate(:status, account: user.account) }
42 53
 
43
-      before do
44
-        Bookmark.find_or_create_by!(account: user.account, status: status)
45
-        post :destroy, params: { status_id: status.id }
46
-      end
54
+        before do
55
+          Bookmark.find_or_create_by!(account: user.account, status: status)
56
+          post :destroy, params: { status_id: status.id }
57
+        end
47 58
 
48
-      it 'returns http success' do
49
-        expect(response).to have_http_status(:success)
59
+        it 'returns http success' do
60
+          expect(response).to have_http_status(:success)
61
+        end
62
+
63
+        it 'updates the bookmarked attribute' do
64
+          expect(user.account.bookmarked?(status)).to be false
65
+        end
66
+
67
+        it 'returns json with updated attributes' do
68
+          hash_body = body_as_json
69
+
70
+          expect(hash_body[:id]).to eq status.id.to_s
71
+          expect(hash_body[:bookmarked]).to be false
72
+        end
50 73
       end
51 74
 
52
-      it 'updates the bookmarked attribute' do
53
-        expect(user.account.bookmarked?(status)).to be false
75
+      context 'with private status that was not bookmarked' do
76
+        let(:status) { Fabricate(:status, visibility: :private) }
77
+
78
+        before do
79
+          post :destroy, params: { status_id: status.id }
80
+        end
81
+
82
+        it 'returns http not found' do
83
+          expect(response).to have_http_status(404)
84
+        end
54 85
       end
55 86
     end
56 87
   end

+ 59
- 27
spec/controllers/api/v1/statuses/favourites_controller_spec.rb View File

@@ -21,45 +21,77 @@ describe Api::V1::Statuses::FavouritesController do
21 21
         post :create, params: { status_id: status.id }
22 22
       end
23 23
 
24
-      it 'returns http success' do
25
-        expect(response).to have_http_status(200)
24
+      context 'with public status' do
25
+        it 'returns http success' do
26
+          expect(response).to have_http_status(200)
27
+        end
28
+
29
+        it 'updates the favourites count' do
30
+          expect(status.favourites.count).to eq 1
31
+        end
32
+
33
+        it 'updates the favourited attribute' do
34
+          expect(user.account.favourited?(status)).to be true
35
+        end
36
+
37
+        it 'returns json with updated attributes' do
38
+          hash_body = body_as_json
39
+
40
+          expect(hash_body[:id]).to eq status.id.to_s
41
+          expect(hash_body[:favourites_count]).to eq 1
42
+          expect(hash_body[:favourited]).to be true
43
+        end
26 44
       end
27 45
 
28
-      it 'updates the favourites count' do
29
-        expect(status.favourites.count).to eq 1
30
-      end
31
-
32
-      it 'updates the favourited attribute' do
33
-        expect(user.account.favourited?(status)).to be true
34
-      end
35
-
36
-      it 'return json with updated attributes' do
37
-        hash_body = body_as_json
46
+      context 'with private status of not-followed account' do
47
+        let(:status) { Fabricate(:status, visibility: :private) }
38 48
 
39
-        expect(hash_body[:id]).to eq status.id.to_s
40
-        expect(hash_body[:favourites_count]).to eq 1
41
-        expect(hash_body[:favourited]).to be true
49
+        it 'returns http not found' do
50
+          expect(response).to have_http_status(404)
51
+        end
42 52
       end
43 53
     end
44 54
 
45 55
     describe 'POST #destroy' do
46
-      let(:status) { Fabricate(:status, account: user.account) }
56
+      context 'with public status' do
57
+        let(:status) { Fabricate(:status, account: user.account) }
47 58
 
48
-      before do
49
-        FavouriteService.new.call(user.account, status)
50
-        post :destroy, params: { status_id: status.id }
51
-      end
59
+        before do
60
+          FavouriteService.new.call(user.account, status)
61
+          post :destroy, params: { status_id: status.id }
62
+        end
52 63
 
53
-      it 'returns http success' do
54
-        expect(response).to have_http_status(200)
55
-      end
64
+        it 'returns http success' do
65
+          expect(response).to have_http_status(200)
66
+        end
67
+
68
+        it 'updates the favourites count' do
69
+          expect(status.favourites.count).to eq 0
70
+        end
71
+
72
+        it 'updates the favourited attribute' do
73
+          expect(user.account.favourited?(status)).to be false
74
+        end
56 75
 
57
-      it 'updates the favourites count' do
58
-        expect(status.favourites.count).to eq 0
76
+        it 'returns json with updated attributes' do
77
+          hash_body = body_as_json
78
+
79
+          expect(hash_body[:id]).to eq status.id.to_s
80
+          expect(hash_body[:favourites_count]).to eq 0
81
+          expect(hash_body[:favourited]).to be false
82
+        end
59 83
       end
60 84
 
61
-      it 'updates the favourited attribute' do
62
-        expect(user.account.favourited?(status)).to be false
85
+      context 'with private status that was not favourited' do
86
+        let(:status) { Fabricate(:status, visibility: :private) }
87
+
88
+        before do
89
+          post :destroy, params: { status_id: status.id }
90
+        end
91
+
92
+        it 'returns http not found' do
93
+          expect(response).to have_http_status(404)
94
+        end
63 95
       end
64 96
     end
65 97
   end

+ 59
- 27
spec/controllers/api/v1/statuses/reblogs_controller_spec.rb View File

@@ -21,45 +21,77 @@ describe Api::V1::Statuses::ReblogsController do
21 21
         post :create, params: { status_id: status.id }
22 22
       end
23 23
 
24
-      it 'returns http success' do
25
-        expect(response).to have_http_status(200)
24
+      context 'with public status' do
25
+        it 'returns http success' do
26
+          expect(response).to have_http_status(200)
27
+        end
28
+
29
+        it 'updates the reblogs count' do
30
+          expect(status.reblogs.count).to eq 1
31
+        end
32
+
33
+        it 'updates the reblogged attribute' do
34
+          expect(user.account.reblogged?(status)).to be true
35
+        end
36
+
37
+        it 'returns json with updated attributes' do
38
+          hash_body = body_as_json
39
+
40
+          expect(hash_body[:reblog][:id]).to eq status.id.to_s
41
+          expect(hash_body[:reblog][:reblogs_count]).to eq 1
42
+          expect(hash_body[:reblog][:reblogged]).to be true
43
+        end
26 44
       end
27 45
 
28
-      it 'updates the reblogs count' do
29
-        expect(status.reblogs.count).to eq 1
30
-      end
31
-
32
-      it 'updates the reblogged attribute' do
33
-        expect(user.account.reblogged?(status)).to be true
34
-      end
35
-
36
-      it 'return json with updated attributes' do
37
-        hash_body = body_as_json
46
+      context 'with private status of not-followed account' do
47
+        let(:status) { Fabricate(:status, visibility: :private) }
38 48
 
39
-        expect(hash_body[:reblog][:id]).to eq status.id.to_s
40
-        expect(hash_body[:reblog][:reblogs_count]).to eq 1
41
-        expect(hash_body[:reblog][:reblogged]).to be true
49
+        it 'returns http not found' do
50
+          expect(response).to have_http_status(404)
51
+        end
42 52
       end
43 53
     end
44 54
 
45 55
     describe 'POST #destroy' do
46
-      let(:status) { Fabricate(:status, account: user.account) }
56
+      context 'with public status' do
57
+        let(:status) { Fabricate(:status, account: user.account) }
47 58
 
48
-      before do
49
-        ReblogService.new.call(user.account, status)
50
-        post :destroy, params: { status_id: status.id }
51
-      end
59
+        before do
60
+          ReblogService.new.call(user.account, status)
61
+          post :destroy, params: { status_id: status.id }
62
+        end
52 63
 
53
-      it 'returns http success' do
54
-        expect(response).to have_http_status(200)
55
-      end
64
+        it 'returns http success' do
65
+          expect(response).to have_http_status(200)
66
+        end
67
+
68
+        it 'updates the reblogs count' do
69
+          expect(status.reblogs.count).to eq 0
70
+        end
71
+
72
+        it 'updates the reblogged attribute' do
73
+          expect(user.account.reblogged?(status)).to be false
74
+        end
56 75
 
57
-      it 'updates the reblogs count' do
58
-        expect(status.reblogs.count).to eq 0
76
+        it 'returns json with updated attributes' do
77
+          hash_body = body_as_json
78
+
79
+          expect(hash_body[:id]).to eq status.id.to_s
80
+          expect(hash_body[:reblogs_count]).to eq 0
81
+          expect(hash_body[:reblogged]).to be false
82
+        end
59 83
       end
60 84
 
61
-      it 'updates the reblogged attribute' do
62
-        expect(user.account.reblogged?(status)).to be false
85
+      context 'with private status that was not reblogged' do
86
+        let(:status) { Fabricate(:status, visibility: :private) }
87
+
88
+        before do
89
+          post :destroy, params: { status_id: status.id }
90
+        end
91
+
92
+        it 'returns http not found' do
93
+          expect(response).to have_http_status(404)
94
+        end
63 95
       end
64 96
     end
65 97
   end

Loading…
Cancel
Save