Alias GitHub and BitBucket OAuth2 callback URLs

To prevent an OAuth2 covert redirect vulnerability, this commit adds and
uses an alias for the GitHub and BitBucket OAuth2 callback URLs to the
following paths:

GitHub: /users/auth/-/import/github
Bitbucket: /users/auth/-/import/bitbucket

This allows admins to put a more restrictive callback URL in the OAuth2
configuration settings. Instead of https://example.com, admins can now use:

https://example.com/users/auth

It's possible but not trivial to change Devise and OmniAuth to use a
different prefix for callback URLs instead of /users/auth. For now,
aliasing the import URLs under the /users/auth namespace should suffice.

Closes https://gitlab.com/gitlab-org/gitlab-ce/issues/56663

8 files changed, 43 insertions, 8 deletions

diff --git a/app/controllers/import/bitbucket_controller.rb b/app/controllers/import/bitbucket_controller.rb
index 1b30b4dda36..2b1395f364f 100644
--- a/app/controllers/import/bitbucket_controller.rb
+++ b/app/controllers/import/bitbucket_controller.rb
@@ -8,7 +8,7 @@ class Import::BitbucketController < Import::BaseController
   rescue_from Bitbucket::Error::Unauthorized, with: :bitbucket_unauthorized
 
   def callback
-    response = client.auth_code.get_token(params[:code], redirect_uri: callback_import_bitbucket_url)
+    response = client.auth_code.get_token(params[:code], redirect_uri: users_import_bitbucket_callback_url)
 
     session[:bitbucket_token]         = response.token
     session[:bitbucket_expires_at]    = response.expires_at
@@ -89,7 +89,7 @@ class Import::BitbucketController < Import::BaseController
   end
 
   def go_to_bitbucket_for_permissions
-    redirect_to client.auth_code.authorize_url(redirect_uri: callback_import_bitbucket_url)
+    redirect_to client.auth_code.authorize_url(redirect_uri: users_import_bitbucket_callback_url)
   end
 
   def bitbucket_unauthorized
diff --git a/app/controllers/import/github_controller.rb b/app/controllers/import/github_controller.rb
index 34c7dbdc2fe..3fbc0817e95 100644
--- a/app/controllers/import/github_controller.rb
+++ b/app/controllers/import/github_controller.rb
@@ -83,7 +83,7 @@ class Import::GithubController < Import::BaseController
   end
 
   def callback_import_url
-    public_send("callback_import_#{provider}_url", extra_import_params) # rubocop:disable GitlabSecurity/PublicSend
+    public_send("users_import_#{provider}_callback_url", extra_import_params) # rubocop:disable GitlabSecurity/PublicSend
   end
 
   def provider_unauthorized
diff --git a/changelogs/unreleased/sh-fix-import-redirect-vulnerability.yml b/changelogs/unreleased/sh-fix-import-redirect-vulnerability.yml
new file mode 100644
index 00000000000..addf327b69d
--- /dev/null
+++ b/changelogs/unreleased/sh-fix-import-redirect-vulnerability.yml
@@ -0,0 +1,5 @@
+---
+title: Alias GitHub and BitBucket OAuth2 callback URLs
+merge_request:
+author:
+type: security
diff --git a/config/routes/import.rb b/config/routes/import.rb
index 3998d977c81..69df82611f2 100644
--- a/config/routes/import.rb
+++ b/config/routes/import.rb
@@ -1,3 +1,12 @@
+# Alias import callbacks under the /users/auth endpoint so that
+# the OAuth2 callback URL can be restricted under http://example.com/users/auth
+# instead of http://example.com.
+Devise.omniauth_providers.each do |provider|
+  next if provider == 'ldapmain'
+
+  get "/users/auth/-/import/#{provider}/callback", to: "import/#{provider}#callback", as: "users_import_#{provider}_callback"
+end
+
 namespace :import do
   resource :github, only: [:create, :new], controller: :github do
     post :personal_access_token
diff --git a/doc/integration/bitbucket.md b/doc/integration/bitbucket.md
index a69db1d1a6e..68ec8c4b5c2 100644
--- a/doc/integration/bitbucket.md
+++ b/doc/integration/bitbucket.md
@@ -43,9 +43,13 @@ you to use.
     | :--- | :---------- |
     | **Name** | This can be anything. Consider something like `<Organization>'s GitLab` or `<Your Name>'s GitLab` or something else descriptive. |
     | **Application description** | Fill this in if you wish. |
-    | **Callback URL** | The URL to your GitLab installation, e.g., `https://gitlab.example.com`. |
+    | **Callback URL** | The URL to your GitLab installation, e.g., `https://gitlab.example.com/users/auth`. |
     | **URL** | The URL to your GitLab installation, e.g., `https://gitlab.example.com`. |
 
+    NOTE: Be sure to append `/users/auth` to the end of the callback URL
+    to prevent a [OAuth2 convert
+    redirect](http://tetraph.com/covert_redirect/) vulnerability.
+
     NOTE: Starting in GitLab 8.15, you MUST specify a callback URL, or you will
     see an "Invalid redirect_uri" message. For more details, see [the
     Bitbucket documentation](https://confluence.atlassian.com/bitbucket/oauth-faq-338365710.html).
diff --git a/doc/integration/github.md b/doc/integration/github.md
index b8156b2b593..eca9aa16499 100644
--- a/doc/integration/github.md
+++ b/doc/integration/github.md
@@ -21,9 +21,13 @@ To get the credentials (a pair of Client ID and Client Secret), you must registe
     - Application name: This can be anything. Consider something like `<Organization>'s GitLab` or `<Your Name>'s GitLab` or something else descriptive.
     - Homepage URL: the URL to your GitLab installation. e.g., `https://gitlab.company.com`
     - Application description: Fill this in if you wish.
-    - Authorization callback URL: `http(s)://${YOUR_DOMAIN}`. Please make sure the port is included if your GitLab instance is not configured on default port.
+    - Authorization callback URL: `http(s)://${YOUR_DOMAIN}/users/auth`. Please make sure the port is included if your GitLab instance is not configured on default port.
     ![Register OAuth App](img/github_register_app.png)
 
+    NOTE: Be sure to append `/users/auth` to the end of the callback URL
+    to prevent a [OAuth2 convert
+    redirect](http://tetraph.com/covert_redirect/) vulnerability.
+
 1.  Select **Register application**.
 
 1.  You should now see a pair of **Client ID** and **Client Secret** near the top right of the page (see screenshot).
diff --git a/spec/controllers/import/bitbucket_controller_spec.rb b/spec/controllers/import/bitbucket_controller_spec.rb
index 51793f2c048..0bc09c86939 100644
--- a/spec/controllers/import/bitbucket_controller_spec.rb
+++ b/spec/controllers/import/bitbucket_controller_spec.rb
@@ -8,6 +8,7 @@ describe Import::BitbucketController do
   let(:secret) { "sekrettt" }
   let(:refresh_token) { SecureRandom.hex(15) }
   let(:access_params) { { token: token, expires_at: nil, expires_in: nil, refresh_token: nil } }
+  let(:code) { SecureRandom.hex(8) }
 
   def assign_session_tokens
     session[:bitbucket_token] = token
@@ -32,10 +33,16 @@ describe Import::BitbucketController do
                             expires_in: expires_in,
                             refresh_token: refresh_token)
       allow_any_instance_of(OAuth2::Client)
-        .to receive(:get_token).and_return(access_token)
+        .to receive(:get_token)
+        .with(hash_including(
+                'grant_type' => 'authorization_code',
+                'code' => code,
+                redirect_uri: users_import_bitbucket_callback_url),
+              {})
+        .and_return(access_token)
       stub_omniauth_provider('bitbucket')
 
-      get :callback
+      get :callback, params: { code: code }
 
       expect(session[:bitbucket_token]).to eq(token)
       expect(session[:bitbucket_refresh_token]).to eq(refresh_token)
diff --git a/spec/controllers/import/github_controller_spec.rb b/spec/controllers/import/github_controller_spec.rb
index 780e49f7b93..bca5f3f6589 100644
--- a/spec/controllers/import/github_controller_spec.rb
+++ b/spec/controllers/import/github_controller_spec.rb
@@ -12,9 +12,15 @@ describe Import::GithubController do
 
     it "redirects to GitHub for an access token if logged in with GitHub" do
       allow(controller).to receive(:logged_in_with_provider?).and_return(true)
-      expect(controller).to receive(:go_to_provider_for_permissions)
+      expect(controller).to receive(:go_to_provider_for_permissions).and_call_original
+      allow_any_instance_of(Gitlab::LegacyGithubImport::Client)
+        .to receive(:authorize_url)
+        .with(users_import_github_callback_url)
+        .and_call_original
 
       get :new
+
+      expect(response).to have_http_status(302)
     end
 
     it "prompts for an access token if GitHub not configured" do