From e56e3cdc62f96541b9bd8b7814204e92f1909253 Mon Sep 17 00:00:00 2001 From: Felipe Artur Date: Mon, 9 May 2016 19:35:37 -0300 Subject: Fix api leaking notes when user is not authorized to read noteable --- lib/api/notes.rb | 31 ++++++++++++++++++------------- 1 file changed, 18 insertions(+), 13 deletions(-) (limited to 'lib/api/notes.rb') diff --git a/lib/api/notes.rb b/lib/api/notes.rb index 71a53e6f0d6..4ac08a3e8c4 100644 --- a/lib/api/notes.rb +++ b/lib/api/notes.rb @@ -20,19 +20,24 @@ module API # GET /projects/:id/snippets/:noteable_id/notes get ":id/#{noteables_str}/:#{noteable_id_str}/notes" do @noteable = user_project.send(:"#{noteables_str}").find(params[:"#{noteable_id_str}"]) - - # We exclude notes that are cross-references and that cannot be viewed - # by the current user. By doing this exclusion at this level and not - # at the DB query level (which we cannot in that case), the current - # page can have less elements than :per_page even if - # there's more than one page. - notes = - # paginate() only works with a relation. This could lead to a - # mismatch between the pagination headers info and the actual notes - # array returned, but this is really a edge-case. - paginate(@noteable.notes). - reject { |n| n.cross_reference_not_visible_for?(current_user) } - present notes, with: Entities::Note + read_ability_name = "read_#{@noteable.class.to_s.underscore.downcase}".to_sym + + if can?(current_user, read_ability_name, @noteable) + # We exclude notes that are cross-references and that cannot be viewed + # by the current user. By doing this exclusion at this level and not + # at the DB query level (which we cannot in that case), the current + # page can have less elements than :per_page even if + # there's more than one page. + notes = + # paginate() only works with a relation. This could lead to a + # mismatch between the pagination headers info and the actual notes + # array returned, but this is really a edge-case. + paginate(@noteable.notes). + reject { |n| n.cross_reference_not_visible_for?(current_user) } + present notes, with: Entities::Note + else + render_api_error!("Not found.", 404) + end end # Get a single +noteable+ note -- cgit v1.2.1 From 93ca5c9964a26fbf31fcc794348b30193f4dff9f Mon Sep 17 00:00:00 2001 From: Felipe Artur Date: Tue, 10 May 2016 16:06:02 -0300 Subject: Fix notes API calls symbol convertions --- lib/api/notes.rb | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) (limited to 'lib/api/notes.rb') diff --git a/lib/api/notes.rb b/lib/api/notes.rb index 4ac08a3e8c4..f0116acd90f 100644 --- a/lib/api/notes.rb +++ b/lib/api/notes.rb @@ -19,7 +19,7 @@ module API # GET /projects/:id/issues/:noteable_id/notes # GET /projects/:id/snippets/:noteable_id/notes get ":id/#{noteables_str}/:#{noteable_id_str}/notes" do - @noteable = user_project.send(:"#{noteables_str}").find(params[:"#{noteable_id_str}"]) + @noteable = user_project.send(noteables_str.to_sym).find(params[noteable_id_str.to_sym]) read_ability_name = "read_#{@noteable.class.to_s.underscore.downcase}".to_sym if can?(current_user, read_ability_name, @noteable) @@ -36,7 +36,7 @@ module API reject { |n| n.cross_reference_not_visible_for?(current_user) } present notes, with: Entities::Note else - render_api_error!("Not found.", 404) + not_found!("Notes") end end @@ -50,7 +50,7 @@ module API # GET /projects/:id/issues/:noteable_id/notes/:note_id # GET /projects/:id/snippets/:noteable_id/notes/:note_id get ":id/#{noteables_str}/:#{noteable_id_str}/notes/:note_id" do - @noteable = user_project.send(:"#{noteables_str}").find(params[:"#{noteable_id_str}"]) + @noteable = user_project.send(noteables_str.to_sym).find(params[noteable_id_str.to_sym]) @note = @noteable.notes.find(params[:note_id]) if @note.cross_reference_not_visible_for?(current_user) -- cgit v1.2.1 From c9be74e24797c1dab5b443728349bb0c5ce969c3 Mon Sep 17 00:00:00 2001 From: Felipe Artur Date: Mon, 16 May 2016 16:43:19 -0300 Subject: Fix single note api request --- lib/api/notes.rb | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) (limited to 'lib/api/notes.rb') diff --git a/lib/api/notes.rb b/lib/api/notes.rb index f0116acd90f..c49b107d1d9 100644 --- a/lib/api/notes.rb +++ b/lib/api/notes.rb @@ -20,9 +20,8 @@ module API # GET /projects/:id/snippets/:noteable_id/notes get ":id/#{noteables_str}/:#{noteable_id_str}/notes" do @noteable = user_project.send(noteables_str.to_sym).find(params[noteable_id_str.to_sym]) - read_ability_name = "read_#{@noteable.class.to_s.underscore.downcase}".to_sym - if can?(current_user, read_ability_name, @noteable) + if can?(current_user, noteable_ability_name(@noteable), @noteable) # We exclude notes that are cross-references and that cannot be viewed # by the current user. By doing this exclusion at this level and not # at the DB query level (which we cannot in that case), the current @@ -52,11 +51,12 @@ module API get ":id/#{noteables_str}/:#{noteable_id_str}/notes/:note_id" do @noteable = user_project.send(noteables_str.to_sym).find(params[noteable_id_str.to_sym]) @note = @noteable.notes.find(params[:note_id]) + can_read_note = can?(current_user, noteable_ability_name(@noteable), @noteable) && !@note.cross_reference_not_visible_for?(current_user) - if @note.cross_reference_not_visible_for?(current_user) - not_found!("Note") - else + if can_read_note present @note, with: Entities::Note + else + not_found!("Note") end end -- cgit v1.2.1 From 5bf49bb63d88b1cce2d9a44716b54acfa63ea657 Mon Sep 17 00:00:00 2001 From: Felipe Artur Date: Tue, 17 May 2016 21:41:53 -0500 Subject: Move note helper method to notes entity file --- lib/api/notes.rb | 10 ++++++++-- 1 file changed, 8 insertions(+), 2 deletions(-) (limited to 'lib/api/notes.rb') diff --git a/lib/api/notes.rb b/lib/api/notes.rb index c49b107d1d9..d4fcfd3d4d3 100644 --- a/lib/api/notes.rb +++ b/lib/api/notes.rb @@ -21,7 +21,7 @@ module API get ":id/#{noteables_str}/:#{noteable_id_str}/notes" do @noteable = user_project.send(noteables_str.to_sym).find(params[noteable_id_str.to_sym]) - if can?(current_user, noteable_ability_name(@noteable), @noteable) + if can?(current_user, noteable_read_ability_name(@noteable), @noteable) # We exclude notes that are cross-references and that cannot be viewed # by the current user. By doing this exclusion at this level and not # at the DB query level (which we cannot in that case), the current @@ -51,7 +51,7 @@ module API get ":id/#{noteables_str}/:#{noteable_id_str}/notes/:note_id" do @noteable = user_project.send(noteables_str.to_sym).find(params[noteable_id_str.to_sym]) @note = @noteable.notes.find(params[:note_id]) - can_read_note = can?(current_user, noteable_ability_name(@noteable), @noteable) && !@note.cross_reference_not_visible_for?(current_user) + can_read_note = can?(current_user, noteable_read_ability_name(@noteable), @noteable) && !@note.cross_reference_not_visible_for?(current_user) if can_read_note present @note, with: Entities::Note @@ -141,5 +141,11 @@ module API end end end + + helpers do + def noteable_read_ability_name(noteable) + "read_#{noteable.class.to_s.underscore.downcase}".to_sym + end + end end end -- cgit v1.2.1