Add state param validation for GitHub OAuth flow

app/controllers/import/github_controller.rb

@@ -28,8 +28,14 @@ class Import::GithubController < Import::BaseController
   end
 
   def callback
-    session[access_token_key] = get_token(params[:code])
-    redirect_to status_import_url
+    auth_state = session[auth_state_key]
+    session[auth_state_key] = nil
+    if auth_state.blank? || !ActiveSupport::SecurityUtils.secure_compare(auth_state, params[:state])
+      provider_unauthorized
+    else
+      session[access_token_key] = get_token(params[:code])
+      redirect_to status_import_url
+    end
   end
 
   def personal_access_token
@@ -154,13 +160,16 @@ class Import::GithubController < Import::BaseController
   end
 
   def authorize_url
+    state = SecureRandom.base64(64)
+    session[auth_state_key] = state
     if Feature.enabled?(:remove_legacy_github_client)
       oauth_client.auth_code.authorize_url(
         redirect_uri: callback_import_url,
-        scope: 'repo, user, user:email'
+        scope: 'repo, user, user:email',
+        state: state
       )
     else
-      client.authorize_url(callback_import_url)
+      client.authorize_url(callback_import_url, state)
     end
   end
 
@@ -219,6 +228,10 @@ class Import::GithubController < Import::BaseController
       alert: _('Missing OAuth configuration for GitHub.')
   end
 
+  def auth_state_key
+    :"#{provider_name}_auth_state_key"
+  end
+
   def access_token_key
     :"#{provider_name}_access_token"
   end

lib/gitlab/legacy_github_import/client.rb

@@ -48,10 +48,11 @@ module Gitlab
         )
       end
 
-      def authorize_url(redirect_uri)
+      def authorize_url(redirect_uri, state = nil)
         client.auth_code.authorize_url({
           redirect_uri: redirect_uri,
-          scope: "repo, user, user:email"
+          scope: "repo, user, user:email",
+          state: state
         })
       end
 

spec/controllers/import/github_controller_spec.rb

@@ -6,6 +6,7 @@ RSpec.describe Import::GithubController do
   include ImportSpecHelper
 
   let(:provider) { :github }
+  let(:new_import_url) { public_send("new_import_#{provider}_url") }
 
   include_context 'a GitHub-ish import controller'
 
@@ -50,13 +51,37 @@ RSpec.describe Import::GithubController do
       stub_omniauth_provider('github')
     end
 
-    it "updates access token" do
-      token = "asdasd12345"
+    context "when auth state param is missing from session" do
+      it "reports an error" do
+        get :callback
 
-      get :callback
+        expect(controller).to redirect_to(new_import_url)
+        expect(flash[:alert]).to eq('Access denied to your GitHub account.')
+      end
+    end
+
+    context "when auth state param is present in session" do
+      let(:valid_auth_state) { "secret-state" }
+
+      before do
+        session[:github_auth_state_key] = valid_auth_state
+      end
 
-      expect(session[:github_access_token]).to eq(token)
-      expect(controller).to redirect_to(status_import_github_url)
+      it "updates access token if state param is valid" do
+        token = "asdasd12345"
+
+        get :callback, params: { state: valid_auth_state }
+
+        expect(session[:github_access_token]).to eq(token)
+        expect(controller).to redirect_to(status_import_github_url)
+      end
+
+      it "reports an error if state param is invalid" do
+        get :callback, params: { state: "different-state" }
+
+        expect(controller).to redirect_to(new_import_url)
+        expect(flash[:alert]).to eq('Access denied to your GitHub account.')
+      end
     end
   end
 
@@ -71,8 +96,6 @@ RSpec.describe Import::GithubController do
       end
 
       context 'when OAuth config is missing' do
-        let(:new_import_url) { public_send("new_import_#{provider}_url") }
-
         before do
           allow(controller).to receive(:oauth_config).and_return(nil)
         end
@@ -108,6 +131,16 @@ RSpec.describe Import::GithubController do
 
         get :status
       end
+
+      it 'gets authorization url using legacy client' do
+        allow(controller).to receive(:logged_in_with_provider?).and_return(true)
+        expect(controller).to receive(:go_to_provider_for_permissions).and_call_original
+        expect_next_instance_of(Gitlab::LegacyGithubImport::Client) do |client|
+          expect(client).to receive(:authorize_url).and_call_original
+        end
+
+        get :new
+      end
     end
 
     context 'when feature remove_legacy_github_client is enabled' do
@@ -130,6 +163,16 @@ RSpec.describe Import::GithubController do
         get :status
       end
 
+      it 'gets authorization url using oauth client' do
+        allow(controller).to receive(:logged_in_with_provider?).and_return(true)
+        expect(controller).to receive(:go_to_provider_for_permissions).and_call_original
+        expect_next_instance_of(OAuth2::Client) do |client|
+          expect(client.auth_code).to receive(:authorize_url).and_call_original
+        end
+
+        get :new
+      end
+
       context 'pagination' do
         context 'when no page is specified' do
           it 'requests first page' do