Commit e9d3b9659525c23a1d8c3b755c792040a5b41148
1 parent
ed3f4408
Exists in
master
and in
4 other branches
API: fixes visibility of project hook
When a user is not authorized to see the list of hooks for a project, he is still able to access the hooks separately. For example if access to `GET /projects/:id/hooks` fails and returns a `403 Unauthorized` error it is still possible to access a hook directly via `GET /projects/:id/hooks/:hook_id`. Fixes access, also added tests to check access and status codes of hooks.
Showing
2 changed files
with
33 additions
and
10 deletions
Show diff stats
lib/api/projects.rb
| @@ -155,6 +155,7 @@ module Gitlab | @@ -155,6 +155,7 @@ module Gitlab | ||
| 155 | # Example Request: | 155 | # Example Request: |
| 156 | # GET /projects/:id/hooks/:hook_id | 156 | # GET /projects/:id/hooks/:hook_id |
| 157 | get ":id/hooks/:hook_id" do | 157 | get ":id/hooks/:hook_id" do |
| 158 | + authorize! :admin_project, user_project | ||
| 158 | @hook = user_project.hooks.find(params[:hook_id]) | 159 | @hook = user_project.hooks.find(params[:hook_id]) |
| 159 | present @hook, with: Entities::Hook | 160 | present @hook, with: Entities::Hook |
| 160 | end | 161 | end |
spec/requests/api/projects_spec.rb
| @@ -196,22 +196,44 @@ describe Gitlab::API do | @@ -196,22 +196,44 @@ describe Gitlab::API do | ||
| 196 | end | 196 | end |
| 197 | 197 | ||
| 198 | describe "GET /projects/:id/hooks" do | 198 | describe "GET /projects/:id/hooks" do |
| 199 | - it "should return project hooks" do | ||
| 200 | - get api("/projects/#{project.id}/hooks", user) | 199 | + context "authorized user" do |
| 200 | + it "should return project hooks" do | ||
| 201 | + get api("/projects/#{project.id}/hooks", user) | ||
| 202 | + response.status.should == 200 | ||
| 201 | 203 | ||
| 202 | - response.status.should == 200 | 204 | + json_response.should be_an Array |
| 205 | + json_response.count.should == 1 | ||
| 206 | + json_response.first['url'].should == "http://example.com" | ||
| 207 | + end | ||
| 208 | + end | ||
| 203 | 209 | ||
| 204 | - json_response.should be_an Array | ||
| 205 | - json_response.count.should == 1 | ||
| 206 | - json_response.first['url'].should == "http://example.com" | 210 | + context "unauthorized user" do |
| 211 | + it "should not access project hooks" do | ||
| 212 | + get api("/projects/#{project.id}/hooks", user3) | ||
| 213 | + response.status.should == 403 | ||
| 214 | + end | ||
| 207 | end | 215 | end |
| 208 | end | 216 | end |
| 209 | 217 | ||
| 210 | describe "GET /projects/:id/hooks/:hook_id" do | 218 | describe "GET /projects/:id/hooks/:hook_id" do |
| 211 | - it "should return a project hook" do | ||
| 212 | - get api("/projects/#{project.id}/hooks/#{hook.id}", user) | ||
| 213 | - response.status.should == 200 | ||
| 214 | - json_response['url'].should == hook.url | 219 | + context "authorized user" do |
| 220 | + it "should return a project hook" do | ||
| 221 | + get api("/projects/#{project.id}/hooks/#{hook.id}", user) | ||
| 222 | + response.status.should == 200 | ||
| 223 | + json_response['url'].should == hook.url | ||
| 224 | + end | ||
| 225 | + | ||
| 226 | + it "should return a 404 error if hook id is not available" do | ||
| 227 | + get api("/projects/#{project.id}/hooks/1234", user) | ||
| 228 | + response.status.should == 404 | ||
| 229 | + end | ||
| 230 | + end | ||
| 231 | + | ||
| 232 | + context "unauthorized user" do | ||
| 233 | + it "should not access an existing hook" do | ||
| 234 | + get api("/projects/#{project.id}/hooks/#{hook.id}", user3) | ||
| 235 | + response.status.should == 403 | ||
| 236 | + end | ||
| 215 | end | 237 | end |
| 216 | end | 238 | end |
| 217 | 239 |