Merge pull request #738 from github/no-token-reuse

Improve authentication process

features/authentication.feature

@@ -5,12 +5,15 @@ Feature: OAuth authentication
   Scenario: Ask for username & password, create authorization
     Given the GitHub API server:
       """
-      require 'rack/auth/basic'
-      get('/authorizations') { '[]' }
+      require 'socket'
+      require 'etc'
+      machine_id = "#{Etc.getlogin}@#{Socket.gethostname}"
+
       post('/authorizations') {
-        auth = Rack::Auth::Basic::Request.new(env)
-        halt 401 unless auth.credentials == %w[mislav kitty]
-        assert :scopes => ['repo']
+        assert_basic_auth 'mislav', 'kitty'
+        assert :scopes => ['repo'],
+               :note => "hub for #{machine_id}",
+               :note_url => 'http://hub.github.com/'
         json :token => 'OTOKEN'
       }
       get('/user') {
@@ -32,50 +35,29 @@ Feature: OAuth authentication
     And the file "../home/.config/hub" should contain "oauth_token: OTOKEN"
     And the file "../home/.config/hub" should have mode "0600"
 
-  Scenario: Ask for username & password, re-use existing authorization
+  Scenario: Rename & retry creating authorization if there's a token name collision
     Given the GitHub API server:
       """
-      require 'rack/auth/basic'
-      get('/authorizations') {
-        auth = Rack::Auth::Basic::Request.new(env)
-        halt 401 unless auth.credentials == %w[mislav kitty]
-        json [
-          {:token => 'SKIPPD', :note_url => 'http://example.com'},
-          {:token => 'OTOKEN', :note_url => 'http://hub.github.com/'}
-        ]
-      }
-      get('/user') {
-        json :login => 'mislav'
-      }
-      post('/user/repos') {
-        json :full_name => 'mislav/dotfiles'
-      }
-      """
-    When I run `hub create` interactively
-    When I type "mislav"
-    And I type "kitty"
-    Then the output should contain "github.com password for mislav (never stored):"
-    And the exit status should be 0
-    And the file "../home/.config/hub" should contain "oauth_token: OTOKEN"
+      require 'socket'
+      require 'etc'
+      machine_id = "#{Etc.getlogin}@#{Socket.gethostname}"
 
-  Scenario: Re-use existing authorization with an old URL
-    Given the GitHub API server:
-      """
-      require 'rack/auth/basic'
-      get('/authorizations') {
-        auth = Rack::Auth::Basic::Request.new(env)
-        halt 401 unless auth.credentials == %w[mislav kitty]
-        json [
-          {:token => 'OTOKEN', :note => 'hub', :note_url => 'http://defunkt.io/hub/'}
-        ]
-      }
       post('/authorizations') {
-        status 422
-        json :message => "Validation Failed",
-          :errors => [{:resource => "OauthAccess", :code => "already_exists", :field => "description"}]
+        assert_basic_auth 'mislav', 'kitty'
+        if params[:note] == "hub for #{machine_id} 3"
+          json :token => 'OTOKEN'
+        else
+          status 422
+          json :message => 'Validation Failed',
+               :errors => [{
+                 :resource => 'OauthAccess',
+                 :code => 'already_exists',
+                 :field => 'description'
+               }]
+        end
       }
       get('/user') {
-        json :login => 'mislav'
+        json :login => 'MiSlAv'
       }
       post('/user/repos') {
         json :full_name => 'mislav/dotfiles'
@@ -84,54 +66,43 @@ Feature: OAuth authentication
     When I run `hub create` interactively
     When I type "mislav"
     And I type "kitty"
-    Then the output should contain "github.com password for mislav (never stored):"
+    Then the output should contain "github.com username:"
     And the exit status should be 0
     And the file "../home/.config/hub" should contain "oauth_token: OTOKEN"
 
-  Scenario: Re-use existing authorization found on page 3
+  Scenario: Avoid getting caught up in infinite recursion while retrying token names
     Given the GitHub API server:
       """
-      get('/authorizations') {
-        assert_basic_auth 'mislav', 'kitty'
-        page = (params[:page] || 1).to_i
-        if page < 3
-          response.headers['Link'] = %(<#{url}?page=#{page+1}>; rel="next")
-          json []
-        else
-          json [
-            {:token => 'OTOKEN', :note => 'hub', :note_url => 'http://hub.github.com/'}
-          ]
-        end
-      }
+      tries = 0
       post('/authorizations') {
+        tries += 1
+        halt 400, json(:message => "too many tries") if tries >= 10
         status 422
-        json :message => "Validation Failed",
-          :errors => [{:resource => "OauthAccess", :code => "already_exists", :field => "description"}]
-      }
-      get('/user') {
-        json :login => 'mislav'
-      }
-      post('/user/repos') {
-        json :full_name => 'mislav/dotfiles'
+        json :message => 'Validation Failed',
+             :errors => [{
+               :resource => 'OauthAccess',
+               :code => 'already_exists',
+               :field => 'description'
+             }]
       }
       """
     When I run `hub create` interactively
     When I type "mislav"
     And I type "kitty"
-    Then the output should contain "github.com password for mislav (never stored):"
-    And the exit status should be 0
-    And the file "../home/.config/hub" should contain "oauth_token: OTOKEN"
+    Then the output should contain:
+      """
+      Error creating repository: Unprocessable Entity (HTTP 422)
+      Duplicate value for "description"
+      """
+    And the exit status should be 1
+    And the file "../home/.config/hub" should not exist
 
   Scenario: Credentials from GITHUB_USER & GITHUB_PASSWORD
     Given the GitHub API server:
       """
-      require 'rack/auth/basic'
-      get('/authorizations') {
-        auth = Rack::Auth::Basic::Request.new(env)
-        halt 401 unless auth.credentials == %w[mislav kitty]
-        json [
-          {:token => 'OTOKEN', :note_url => 'http://hub.github.com/'}
-        ]
+      post('/authorizations') {
+        assert_basic_auth 'mislav', 'kitty'
+        json :token => 'OTOKEN'
       }
       get('/user') {
         json :login => 'mislav'
@@ -149,41 +120,54 @@ Feature: OAuth authentication
   Scenario: Wrong password
     Given the GitHub API server:
       """
-      require 'rack/auth/basic'
-      get('/authorizations') {
-        auth = Rack::Auth::Basic::Request.new(env)
-        halt 401 unless auth.credentials == %w[mislav kitty]
+      post('/authorizations') {
+        assert_basic_auth 'mislav', 'kitty'
       }
       """
     When I run `hub create` interactively
     When I type "mislav"
     And I type "WRONG"
-    Then the stderr should contain "Error creating repository: Unauthorized (HTTP 401)"
+    Then the stderr should contain exactly:
+      """
+      Error creating repository: Unauthorized (HTTP 401)
+      Bad credentials
+
+      """
     And the exit status should be 1
     And the file "../home/.config/hub" should not exist
 
-  Scenario: Two-factor authentication, create authorization
+  Scenario: Personal access token used instead of password
     Given the GitHub API server:
       """
-      require 'rack/auth/basic'
-      get('/authorizations') {
-        auth = Rack::Auth::Basic::Request.new(env)
-        halt 401 unless auth.credentials == %w[mislav kitty]
-        if request.env['HTTP_X_GITHUB_OTP'] != "112233"
-          response.headers['X-GitHub-OTP'] = "required;application"
-          halt 401
-        end
-        json [ ]
+      post('/authorizations') {
+        status 403
+        json :message => "This API can only be accessed with username and password Basic Auth"
       }
+      """
+    When I run `hub create` interactively
+    When I type "mislav"
+    And I type "PERSONALACCESSTOKEN"
+    Then the stderr should contain exactly:
+      """
+      Error creating repository: Forbidden (HTTP 403)
+      This API can only be accessed with username and password Basic Auth
+
+      """
+    And the exit status should be 1
+    And the file "../home/.config/hub" should not exist
+
+  Scenario: Two-factor authentication, create authorization
+    Given the GitHub API server:
+      """
       post('/authorizations') {
-        auth = Rack::Auth::Basic::Request.new(env)
-        halt 401 unless auth.credentials == %w[mislav kitty]
-        halt 412 unless params[:scopes]
-        if request.env['HTTP_X_GITHUB_OTP'] != "112233"
-          response.headers['X-GitHub-OTP'] = "required;application"
-          halt 401
+        assert_basic_auth 'mislav', 'kitty'
+        if request.env['HTTP_X_GITHUB_OTP'] == '112233'
+          json :token => 'OTOKEN'
+        else
+          response.headers['X-GitHub-OTP'] = 'required; app'
+          status 401
+          json :message => "Must specify two-factor authentication OTP code."
         end
-        json :token => 'OTOKEN'
       }
       get('/user') {
         json :login => 'mislav'
@@ -198,28 +182,25 @@ Feature: OAuth authentication
     And I type "112233"
     Then the output should contain "github.com password for mislav (never stored):"
     Then the output should contain "two-factor authentication code:"
+    And the output should not contain "warning: invalid two-factor code"
     And the exit status should be 0
     And the file "../home/.config/hub" should contain "oauth_token: OTOKEN"
 
-  Scenario: Two-factor authentication, re-use existing authorization
+  Scenario: Retry entering two-factor authentication code
     Given the GitHub API server:
       """
-      token = 'OTOKEN'
+      previous_otp_code = nil
       post('/authorizations') {
         assert_basic_auth 'mislav', 'kitty'
-        token << 'SMS'
-        status 412
-      }
-      get('/authorizations') {
-        assert_basic_auth 'mislav', 'kitty'
-        if request.env['HTTP_X_GITHUB_OTP'] != "112233"
-          response.headers['X-GitHub-OTP'] = "required;application"
-          halt 401
+        if request.env['HTTP_X_GITHUB_OTP'] == '112233'
+          halt 400 unless '666' == previous_otp_code
+          json :token => 'OTOKEN'
+        else
+          previous_otp_code = request.env['HTTP_X_GITHUB_OTP']
+          response.headers['X-GitHub-OTP'] = 'required; app'
+          status 401
+          json :message => "Must specify two-factor authentication OTP code."
         end
-        json [ {
-          :token => token,
-          :note_url => 'http://hub.github.com/'
-          } ]
       }
       get('/user') {
         json :login => 'mislav'
@@ -231,16 +212,15 @@ Feature: OAuth authentication
     When I run `hub create` interactively
     When I type "mislav"
     And I type "kitty"
+    And I type "666"
     And I type "112233"
-    Then the output should contain "github.com password for mislav (never stored):"
-    Then the output should contain "two-factor authentication code:"
+    Then the output should contain "warning: invalid two-factor code"
     And the exit status should be 0
-    And the file "../home/.config/hub" should contain "oauth_token: OTOKENSMS"
+    And the file "../home/.config/hub" should contain "oauth_token: OTOKEN"
 
   Scenario: Special characters in username & password
     Given the GitHub API server:
       """
-      get('/authorizations') { '[]' }
       post('/authorizations') {
         assert_basic_auth 'mislav@example.com', 'my pass@phrase ok?'
         json :token => 'OTOKEN'

features/support/local_server.rb

@@ -70,11 +70,7 @@ module Hub
           require 'rack/auth/basic'
           auth = Rack::Auth::Basic::Request.new(env)
           if auth.credentials != expected
-            halt 401, json(
-              :message => "expected %p; got %p" % [
-                expected, auth.credentials
-              ]
-            )
+            halt 401, json(:message => "Bad credentials")
           end
         end
 

lib/hub/commands.rb

@@ -1157,8 +1157,7 @@ help
 
     def display_api_exception(action, response)
       $stderr.puts "Error #{action}: #{response.message.strip} (HTTP #{response.status})"
-      if 422 == response.status and response.error_message?
-        # display validation errors
+      if [401, 403, 422].include?(response.status) && response.error_message?
         msg = response.error_message
         msg = msg.join("\n") if msg.respond_to? :join
         warn msg

lib/hub/github_api.rb

@@ -333,36 +333,49 @@ module Hub
         end
       end
 
-      def obtain_oauth_token host, user, two_factor_code = nil
+      def obtain_oauth_token host, user
         auth_url = URI.parse("https://%s@%s/authorizations" % [CGI.escape(user), host])
+        auth_params = {
+          :scopes => ['repo'],
+          :note => "hub for #{local_user}@#{local_hostname}",
+          :note_url => oauth_app_url
+        }
+        res = nil
+        two_factor_code = nil
 
-        # dummy request to trigger a 2FA SMS since a HTTP GET won't do it
-        post(auth_url) if !two_factor_code
+        loop do
+          res = post(auth_url, auth_params) do |req|
+            req['X-GitHub-OTP'] = two_factor_code if two_factor_code
+          end
 
-        # first try to fetch existing authorization
-        res = get_all(auth_url) do |req|
-          req['X-GitHub-OTP'] = two_factor_code if two_factor_code
-        end
-        unless res.success?
-          if !two_factor_code && res['X-GitHub-OTP'].to_s.include?('required')
+          if res.success?
+            break
+          elsif res.status == 401 && res['X-GitHub-OTP'].to_s.include?('required')
+            $stderr.puts "warning: invalid two-factor code" if two_factor_code
             two_factor_code = config.prompt_auth_code
-            return obtain_oauth_token(host, user, two_factor_code)
+          elsif res.status == 422 && 'already_exists' == res.data['errors'][0]['code']
+            if auth_params[:note] =~ / (\d+)$/
+              res.error! if $1.to_i >= 9
+              auth_params[:note].succ!
+            else
+              auth_params[:note] += ' 2'
+            end
           else
             res.error!
           end
         end
 
-        if found = res.data.find {|auth| auth['note'] == 'hub' || auth['note_url'] == oauth_app_url }
-          found['token']
-        else
-          # create a new authorization
-          res = post auth_url,
-            :scopes => %w[repo], :note => 'hub', :note_url => oauth_app_url do |req|
-              req['X-GitHub-OTP'] = two_factor_code if two_factor_code
-            end
-          res.error! unless res.success?
-          res.data['token']
-        end
+        res.data['token']
+      end
+
+      def local_user
+        require 'etc'
+        Etc.getlogin
+      end
+
+      def local_hostname
+        require 'socket'
+        Socket.gethostname
       end
     end