diff options
author | Yorick Peterse <yorickpeterse@gmail.com> | 2019-01-24 16:26:32 +0000 |
---|---|---|
committer | Yorick Peterse <yorickpeterse@gmail.com> | 2019-01-24 16:26:55 +0000 |
commit | 9244acfd63e4acb27c469e84c30c8ee3767cefaf (patch) | |
tree | cf0b43b1bb84344a53be8eacab279dfb078380cf | |
parent | 7f47e86ed37e0055500dccd314a05a72228ec2bf (diff) | |
download | gitlab-ce-9244acfd63e4acb27c469e84c30c8ee3767cefaf.tar.gz |
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
-rw-r--r-- | app/controllers/import/bitbucket_controller.rb | 4 | ||||
-rw-r--r-- | app/controllers/import/github_controller.rb | 2 | ||||
-rw-r--r-- | changelogs/unreleased/sh-fix-issue-56663-11-5.yml | 5 | ||||
-rw-r--r-- | config/routes/import.rb | 9 | ||||
-rw-r--r-- | doc/integration/bitbucket.md | 6 | ||||
-rw-r--r-- | doc/integration/github.md | 11 | ||||
-rw-r--r-- | spec/controllers/import/bitbucket_controller_spec.rb | 11 | ||||
-rw-r--r-- | spec/controllers/import/github_controller_spec.rb | 8 |
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 |