From d241c6d057798dfbdb6d69e74f82e1ad1848805b Mon Sep 17 00:00:00 2001 From: Stan Hu Date: Thu, 13 Aug 2015 06:35:42 -0700 Subject: Restrict users API endpoints to use integer IDs Closes #2267 --- CHANGELOG | 1 + lib/api/users.rb | 2 +- spec/requests/api/users_spec.rb | 56 +++++++++++++++++++++++++++++++++++++++++ 3 files changed, 58 insertions(+), 1 deletion(-) diff --git a/CHANGELOG b/CHANGELOG index 44ffb56d5fd..cf38606396d 100644 --- a/CHANGELOG +++ b/CHANGELOG @@ -1,6 +1,7 @@ Please view this file on the master branch, on stable branches it's out of date. v 8.0.0 (unreleased) + - Restrict users API endpoints to use integer IDs (Stan Hu) - Only show recent push event if the branch still exists or a recent merge request has not been created (Stan Hu) - Remove satellites - Better performance for web editor (switched from satellites to rugged) diff --git a/lib/api/users.rb b/lib/api/users.rb index ee29f952246..813cc379e43 100644 --- a/lib/api/users.rb +++ b/lib/api/users.rb @@ -3,7 +3,7 @@ module API class Users < Grape::API before { authenticate! } - resource :users do + resource :users, requirements: { uid: /[0-9]*/, id: /[0-9]*/ } do # Get a users list # # Example Request: diff --git a/spec/requests/api/users_spec.rb b/spec/requests/api/users_spec.rb index f2aa369985e..f9bc63680ba 100644 --- a/spec/requests/api/users_spec.rb +++ b/spec/requests/api/users_spec.rb @@ -58,6 +58,11 @@ describe API::API, api: true do expect(response.status).to eq(404) expect(json_response['message']).to eq('404 Not found') end + + it "should return a 404 if invalid ID" do + get api("/users/1ASDF", user) + expect(response.status).to eq(404) + end end describe "POST /users" do @@ -257,6 +262,10 @@ describe API::API, api: true do expect(json_response['message']).to eq('404 Not found') end + it "should raise error for invalid ID" do + expect{put api("/users/ASDF", admin) }.to raise_error(ActionController::RoutingError) + end + it 'should return 400 error if user does not validate' do put api("/users/#{user.id}", admin), password: 'pass', @@ -319,6 +328,10 @@ describe API::API, api: true do post api("/users/#{user.id}/keys", admin), key_attrs end.to change{ user.keys.count }.by(1) end + + it "should raise error for invalid ID" do + expect{post api("/users/ASDF/keys", admin) }.to raise_error(ActionController::RoutingError) + end end describe 'GET /user/:uid/keys' do @@ -346,6 +359,11 @@ describe API::API, api: true do expect(json_response).to be_an Array expect(json_response.first['title']).to eq(key.title) end + + it "should return 404 for invalid ID" do + get api("/users/ASDF/keys", admin) + expect(response.status).to eq(404) + end end end @@ -400,6 +418,10 @@ describe API::API, api: true do post api("/users/#{user.id}/emails", admin), email_attrs end.to change{ user.emails.count }.by(1) end + + it "should raise error for invalid ID" do + expect{post api("/users/ASDF/emails", admin) }.to raise_error(ActionController::RoutingError) + end end describe 'GET /user/:uid/emails' do @@ -427,6 +449,10 @@ describe API::API, api: true do expect(json_response).to be_an Array expect(json_response.first['email']).to eq(email.email) end + + it "should raise error for invalid ID" do + expect{put api("/users/ASDF/emails", admin) }.to raise_error(ActionController::RoutingError) + end end end @@ -463,6 +489,10 @@ describe API::API, api: true do expect(response.status).to eq(404) expect(json_response['message']).to eq('404 Email Not Found') end + + it "should raise error for invalid ID" do + expect{delete api("/users/ASDF/emails/bar", admin) }.to raise_error(ActionController::RoutingError) + end end end @@ -491,6 +521,10 @@ describe API::API, api: true do expect(response.status).to eq(404) expect(json_response['message']).to eq('404 User Not Found') end + + it "should raise error for invalid ID" do + expect{delete api("/users/ASDF", admin) }.to raise_error(ActionController::RoutingError) + end end describe "GET /user" do @@ -553,6 +587,11 @@ describe API::API, api: true do expect(response.status).to eq(404) expect(json_response['message']).to eq('404 Not found') end + + it "should return 404 for invalid ID" do + get api("/users/keys/ASDF", admin) + expect(response.status).to eq(404) + end end describe "POST /user/keys" do @@ -608,6 +647,10 @@ describe API::API, api: true do delete api("/user/keys/#{key.id}") expect(response.status).to eq(401) end + + it "should raise error for invalid ID" do + expect{delete api("/users/keys/ASDF", admin) }.to raise_error(ActionController::RoutingError) + end end describe "GET /user/emails" do @@ -653,6 +696,11 @@ describe API::API, api: true do expect(response.status).to eq(404) expect(json_response['message']).to eq('404 Not found') end + + it "should return 404 for invalid ID" do + get api("/users/emails/ASDF", admin) + expect(response.status).to eq(404) + end end describe "POST /user/emails" do @@ -697,6 +745,10 @@ describe API::API, api: true do delete api("/user/emails/#{email.id}") expect(response.status).to eq(401) end + + it "should raise error for invalid ID" do + expect{delete api("/users/emails/ASDF", admin) }.to raise_error(ActionController::RoutingError) + end end describe 'PUT /user/:id/block' do @@ -748,5 +800,9 @@ describe API::API, api: true do expect(response.status).to eq(404) expect(json_response['message']).to eq('404 User Not Found') end + + it "should raise error for invalid ID" do + expect{put api("/users/ASDF/block", admin) }.to raise_error(ActionController::RoutingError) + end end end -- cgit v1.2.1