Redirect to web client on insufficient oauth scopes

lib/travis/api/app/endpoint/authorization.rb

@@ -202,6 +202,14 @@ class Travis::Api::App
         class UserManager < Struct.new(:data, :token, :drop_token)
           include User::Renaming
 
+          attr_accessor :user
+
+          def initialize(*)
+            super
+
+            @user = ::User.find_by_github_id(data['id'])
+          end
+
           def info(attributes = {})
             info = data.to_hash.slice('name', 'login', 'gravatar_id')
             info.merge! attributes.stringify_keys
@@ -209,9 +217,12 @@ class Travis::Api::App
             info
           end
 
+          def user_exists?
+            user
+          end
+
           def fetch
             retried ||= false
-            user   = ::User.find_by_github_id(data['id'])
             info   = drop_token ? self.info : self.info(github_oauth_token: token)
 
             ActiveRecord::Base.transaction do
@@ -219,7 +230,7 @@ class Travis::Api::App
                 rename_repos_owner(user.login, info['login'])
                 user.update_attributes info
               else
-                user = ::User.create! info
+                self.user = ::User.create! info
               end
 
               nullify_logins(user.github_id, user.login)
@@ -235,11 +246,20 @@ class Travis::Api::App
         end
 
         def user_for_github_token(token, drop_token = false)
-          data   = GH.with(token: token.to_s, client_id: nil) { GH['user'] }
-          scopes = parse_scopes data.headers['x-oauth-scopes']
-          halt 403, 'insufficient access: %p' unless acceptable? scopes
+          data    = GH.with(token: token.to_s, client_id: nil) { GH['user'] }
+          scopes  = parse_scopes data.headers['x-oauth-scopes']
+          manager = UserManager.new(data, token, drop_token)
 
-          user   = UserManager.new(data, token, drop_token).fetch
+          unless acceptable? scopes
+            # TODO: we should probably only redirect if this is a web
+            #      oauth request, are there any other possibilities to
+            #      consider?
+            url =  Travis.config.oauth2.insufficient_access_redirect_url
+            url += "#existing-user" if manager.user_exists?
+            redirect to(url)
+          end
+
+          user   = manager.fetch
           halt 403, 'not a Travis user' if user.nil?
           user
         end

spec/unit/endpoint/authorization/user_manager_spec.rb

@@ -30,11 +30,11 @@ describe Travis::Api::App::Endpoint::Authorization::UserManager do
     }
 
     it 'drops the token when drop_token is set to true' do
-      manager = described_class.new(data, 'abc123', true)
-
       user = stub('user', login: 'drogus', github_id: 456)
       User.expects(:find_by_github_id).with(456).returns(user)
 
+      manager = described_class.new(data, 'abc123', true)
+
       attributes = { login: 'drogus', github_id: 456 }.stringify_keys
 
       user.expects(:update_attributes).with(attributes)

spec/unit/endpoint/authorization_spec.rb

@@ -23,6 +23,62 @@ describe Travis::Api::App::Endpoint::Authorization do
     pending "not yet implemented"
   end
 
+  describe "GET /auth/handshake" do
+    describe 'with insufficient oauth permissions' do
+      before do
+        # TODO: this is a first try of writing tests for authorization, it does not look pretty
+        #       but I wanted to have something to start with, later on I would like to extract
+        #       this flow to a class, which would be easier to test.
+        @original_config = Travis.config.oauth2
+        Travis.config.oauth2 = {
+          authorization_server: 'https://foobar.com',
+          access_token_path: '/access_token_path',
+          client_id: 'client-id',
+          client_secret: 'client-secret',
+          scope: 'public_repo,user:email,new_scope',
+          insufficient_access_redirect_url: 'https://travis-ci.org/insufficient_access'
+        }
+        Travis.redis.sadd('github:states', 'github-state')
+
+        response = mock('response')
+        response.expects(:body).returns('access_token=foobarbaz-token')
+        Faraday.expects(:post).with('https://foobar.com/access_token_path',
+                                    client_id: 'client-id',
+                                    client_secret: 'client-secret',
+                                    scope: 'public_repo,user:email,new_scope',
+                                    redirect_uri: 'http://example.org/auth/handshake',
+                                    state: 'github-state',
+                                    code: 'oauth-code').returns(response)
+
+        data = { 'id' => 111 }
+        data.expects(:headers).returns('x-oauth-scopes' => 'public_repo,user:email')
+        GH.expects(:with).with(token: 'foobarbaz-token', client_id: nil).returns(data)
+      end
+
+      after do
+        Travis.config.oauth2 = @original_config
+        Travis.redis.srem('github:states', 'github-state')
+        # this is cached after first run, so if we change scopes, it will stay
+        # like that for the rest of the run
+        User::Oauth.instance_variable_set("@wanted_scopes", nil)
+      end
+
+      it 'redirects to insufficient access page' do
+        response = get '/auth/handshake?state=github-state&code=oauth-code'
+        response.should redirect_to('https://travis-ci.org/insufficient_access')
+      end
+
+      it 'redirects to insufficient access page for existing user' do
+        user = mock('user')
+        User.expects(:find_by_github_id).with(111).returns(user)
+        expect {
+          response = get '/auth/handshake?state=github-state&code=oauth-code'
+          response.should redirect_to('https://travis-ci.org/insufficient_access#existing-user')
+        }.to_not change { User.count }
+      end
+    end
+  end
+
   describe 'POST /auth/github' do
     before do
       data = { 'id' => user.github_id, 'name' => user.name, 'login' => user.login, 'gravatar_id' => user.gravatar_id }