Merge branch 'sh-fix-issue-56663-11-5' into 'security-11-5'

[11.5] Alias GitHub and BitBucket OAuth2 callback URLs

See merge request gitlab/gitlabhq!2847

(cherry picked from commit c038dc73735e9b0b933ab6417ca6630c3793e14c)

9eb5c6f3 Alias GitHub and BitBucket OAuth2 callback URLs

8 files changed, 46 insertions, 10 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 58565aaf8c9..01a3e5738bf 100644
--- a/app/controllers/import/github_controller.rb
+++ b/app/controllers/import/github_controller.rb
@@ -86,7 +86,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-issue-56663-11-5.yml b/changelogs/unreleased/sh-fix-issue-56663-11-5.yml
new file mode 100644
index 00000000000..addf327b69d
--- /dev/null
+++ b/changelogs/unreleased/sh-fix-issue-56663-11-5.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 bf587b5b296..5259abcbf3b 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 7a83b8e4b35..1b9d8830aba 100644
--- a/doc/integration/github.md
+++ b/doc/integration/github.md
@@ -19,10 +19,15 @@ GitHub will generate an application ID and secret key for you to use.
     - 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. 'https://gitlab.company.com'
     - Application description: Fill this in if you wish.
-    - Authorization callback URL is 'http(s)://${YOUR_DOMAIN}'. Please make sure the port is included if your GitLab instance is not configured on default port.
-1.  Select "Register application".
+    - 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.
 
-1.  You should now see a Client ID and Client Secret near the top right of the page (see screenshot).
+    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).
     Keep this page open as you continue configuration.
     ![GitHub app](img/github_app.png)
 
diff --git a/spec/controllers/import/bitbucket_controller_spec.rb b/spec/controllers/import/bitbucket_controller_spec.rb
index be49b92d23f..6081c5458b5 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, 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 9bbd97ec305..e6070fc429d 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
   end