Commit b915e71e authored by Yorick Peterse's avatar Yorick Peterse

Merge branch 'sh-fix-import-redirect-vulnerability' into 'master'

[master] Alias GitHub and Bitbucket OAuth2 callback URLs

See merge request gitlab/gitlabhq!2840
parents 577b6ba7 6d57b2fd
......@@ -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
......
......@@ -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
......
---
title: Alias GitHub and BitBucket OAuth2 callback URLs
merge_request:
author:
type: security
# 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
......
......@@ -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).
......
......@@ -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).
......
......@@ -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)
......
......@@ -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
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment