From 5b7bc77b93b270c7d320ac2a304d85b2b63475ec Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Mislav=20Marohnic=CC=81?= Date: Sat, 3 Dec 2011 16:44:30 +0100 Subject: [PATCH] authenticate all API requests, helps dealing with private repos GET requests that not necessarily require authentication will be performed unauthenticated if GitHub user/token aren't present. Fixes #111 --- lib/hub/commands.rb | 29 +++++++++++++++++++---------- test/hub_test.rb | 32 +++++++++++++++++++++++++------- 2 files changed, 44 insertions(+), 17 deletions(-) diff --git a/lib/hub/commands.rb b/lib/hub/commands.rb index bdd441c0..06d6eefa 100644 --- a/lib/hub/commands.rb +++ b/lib/hub/commands.rb @@ -34,10 +34,10 @@ module Hub # provides git interrogation methods extend Context - API_REPO = 'http://github.com/api/v2/yaml/repos/show/%s/%s' + API_REPO = 'https://github.com/api/v2/yaml/repos/show/%s/%s' API_FORK = 'https://github.com/api/v2/yaml/repos/fork/%s/%s' API_CREATE = 'https://github.com/api/v2/yaml/repos/create' - API_PULL = 'http://github.com/api/v2/json/pulls/%s' + API_PULL = 'https://github.com/api/v2/json/pulls/%s' API_PULLREQUEST = 'https://github.com/api/v2/yaml/pulls/%s/%s' NAME_RE = /[\w.-]+/ @@ -307,7 +307,8 @@ module Hub owner, repo, pull_id = $1, $2, $3 load_net_http - pull_body = Net::HTTP.get URI(API_PULL % File.join(owner, repo, pull_id)) + response = http_request(API_PULL % File.join(owner, repo, pull_id)) + pull_body = response.body user, branch = pull_body.match(/"label":\s*"(.+?)"/)[1].split(':', 2) new_branch_name = args[2] || "#{user}-#{branch}" @@ -837,8 +838,7 @@ help # Determines whether a user has a fork of the current repo on GitHub. def repo_exists?(user, repo = repo_name) load_net_http - url = API_REPO % [user, repo] - Net::HTTPSuccess === Net::HTTP.get_response(URI(url)) + Net::HTTPSuccess === http_request(API_REPO % [user, repo]) end # Forks the current repo using the GitHub API. @@ -929,11 +929,12 @@ help end end - def http_post(url, params = nil) + def http_request(url, type = :Get) url = URI(url) - post = Net::HTTP::Post.new(url.request_uri) - post.basic_auth "#{github_user}/token", github_token - post.set_form_data params if params + user, token = github_user(type != :Get), github_token(type != :Get) + + req = Net::HTTP.const_get(type).new(url.request_uri) + req.basic_auth "#{user}/token", token if user and token port = url.port if use_ssl = 'https' == url.scheme and not use_ssl? @@ -947,7 +948,15 @@ help # TODO: SSL peer verification http.verify_mode = OpenSSL::SSL::VERIFY_NONE end - http.start { http.request(post) } + + yield req if block_given? + http.start { http.request(req) } + end + + def http_post(url, params = nil) + http_request(url, :Post) do |req| + req.set_form_data params if params + end end def load_net_http diff --git a/test/hub_test.rb b/test/hub_test.rb index 2285d7a0..8125fccd 100644 --- a/test/hub_test.rb +++ b/test/hub_test.rb @@ -316,6 +316,19 @@ class HubTest < Test::Unit::TestCase "fetch xoebus" end + def test_fetch_no_auth + stub_github_user nil + stub_github_token nil + stub_remotes_group('xoebus', nil) + # stub_existing_fork('xoebus') + stub_request(:get, "https://github.com/api/v2/yaml/repos/show/xoebus/hub"). + to_return(:status => 200) + + assert_commands "git remote add xoebus git://github.com/xoebus/hub.git", + "git fetch xoebus", + "fetch xoebus" + end + def test_fetch_new_remote_with_options stub_remotes_group('xoebus', nil) stub_existing_fork('xoebus') @@ -557,7 +570,10 @@ class HubTest < Test::Unit::TestCase def test_create_no_openssl stub_no_remotes - stub_nonexisting_fork('tpw') + # stub_nonexisting_fork('tpw') + stub_request(:get, "http://#{auth}github.com/api/v2/yaml/repos/show/tpw/hub"). + to_return(:status => 404) + stub_request(:post, "http://#{auth}github.com/api/v2/yaml/repos/create"). with(:body => { 'name' => 'hub' }) @@ -584,13 +600,15 @@ class HubTest < Test::Unit::TestCase def test_create_with_env_authentication stub_no_remotes - stub_nonexisting_fork('mojombo') - old_user = ENV['GITHUB_USER'] old_token = ENV['GITHUB_TOKEN'] ENV['GITHUB_USER'] = 'mojombo' ENV['GITHUB_TOKEN'] = '123abc' + # stub_nonexisting_fork('mojombo') + stub_request(:get, "https://#{auth('mojombo', '123abc')}github.com/api/v2/yaml/repos/show/mojombo/hub"). + to_return(:status => 404) + stub_request(:post, "https://#{auth('mojombo', '123abc')}github.com/api/v2/yaml/repos/create"). with(:body => { 'name' => 'hub' }) @@ -834,7 +852,7 @@ class HubTest < Test::Unit::TestCase end def test_checkout_pullrequest - stub_request(:get, "http://github.com/api/v2/json/pulls/defunkt/hub/73"). + stub_request(:get, "https://#{auth}github.com/api/v2/json/pulls/defunkt/hub/73"). to_return(:body => mock_pull_response('blueyed:feature')) assert_commands 'git remote add -f -t feature blueyed git://github.com/blueyed/hub.git', @@ -843,7 +861,7 @@ class HubTest < Test::Unit::TestCase end def test_checkout_pullrequest_custom_branch - stub_request(:get, "http://github.com/api/v2/json/pulls/defunkt/hub/73"). + stub_request(:get, "https://#{auth}github.com/api/v2/json/pulls/defunkt/hub/73"). to_return(:body => mock_pull_response('blueyed:feature')) assert_commands 'git remote add -f -t feature blueyed git://github.com/blueyed/hub.git', @@ -854,7 +872,7 @@ class HubTest < Test::Unit::TestCase def test_checkout_pullrequest_existing_remote stub_command_output 'remote', "origin\nblueyed" - stub_request(:get, "http://github.com/api/v2/json/pulls/defunkt/hub/73"). + stub_request(:get, "https://#{auth}github.com/api/v2/json/pulls/defunkt/hub/73"). to_return(:body => mock_pull_response('blueyed:feature')) assert_commands 'git remote set-branches --add blueyed feature', @@ -1173,7 +1191,7 @@ config end def stub_fork(user, repo, status) - stub_request(:get, "github.com/api/v2/yaml/repos/show/#{user}/#{repo}"). + stub_request(:get, "https://#{auth}github.com/api/v2/yaml/repos/show/#{user}/#{repo}"). to_return(:status => status) end -- GitLab