Commit 6d57b2fd authored by Stan Hu's avatar Stan Hu

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
parent ae216618
...@@ -8,7 +8,7 @@ class Import::BitbucketController < Import::BaseController ...@@ -8,7 +8,7 @@ class Import::BitbucketController < Import::BaseController
rescue_from Bitbucket::Error::Unauthorized, with: :bitbucket_unauthorized rescue_from Bitbucket::Error::Unauthorized, with: :bitbucket_unauthorized
def callback 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_token] = response.token
session[:bitbucket_expires_at] = response.expires_at session[:bitbucket_expires_at] = response.expires_at
...@@ -89,7 +89,7 @@ class Import::BitbucketController < Import::BaseController ...@@ -89,7 +89,7 @@ class Import::BitbucketController < Import::BaseController
end end
def go_to_bitbucket_for_permissions 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 end
def bitbucket_unauthorized def bitbucket_unauthorized
......
...@@ -83,7 +83,7 @@ class Import::GithubController < Import::BaseController ...@@ -83,7 +83,7 @@ class Import::GithubController < Import::BaseController
end end
def callback_import_url 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 end
def provider_unauthorized 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 namespace :import do
resource :github, only: [:create, :new], controller: :github do resource :github, only: [:create, :new], controller: :github do
post :personal_access_token post :personal_access_token
......
...@@ -43,9 +43,13 @@ you to use. ...@@ -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. | | **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. | | **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`. | | **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 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 see an "Invalid redirect_uri" message. For more details, see [the
Bitbucket documentation](https://confluence.atlassian.com/bitbucket/oauth-faq-338365710.html). 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 ...@@ -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. - 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` - Homepage URL: the URL to your GitLab installation. e.g., `https://gitlab.company.com`
- Application description: Fill this in if you wish. - 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) ![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. 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). 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 ...@@ -8,6 +8,7 @@ describe Import::BitbucketController do
let(:secret) { "sekrettt" } let(:secret) { "sekrettt" }
let(:refresh_token) { SecureRandom.hex(15) } let(:refresh_token) { SecureRandom.hex(15) }
let(:access_params) { { token: token, expires_at: nil, expires_in: nil, refresh_token: nil } } let(:access_params) { { token: token, expires_at: nil, expires_in: nil, refresh_token: nil } }
let(:code) { SecureRandom.hex(8) }
def assign_session_tokens def assign_session_tokens
session[:bitbucket_token] = token session[:bitbucket_token] = token
...@@ -32,10 +33,16 @@ describe Import::BitbucketController do ...@@ -32,10 +33,16 @@ describe Import::BitbucketController do
expires_in: expires_in, expires_in: expires_in,
refresh_token: refresh_token) refresh_token: refresh_token)
allow_any_instance_of(OAuth2::Client) 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') stub_omniauth_provider('bitbucket')
get :callback get :callback, params: { code: code }
expect(session[:bitbucket_token]).to eq(token) expect(session[:bitbucket_token]).to eq(token)
expect(session[:bitbucket_refresh_token]).to eq(refresh_token) expect(session[:bitbucket_refresh_token]).to eq(refresh_token)
......
...@@ -12,9 +12,15 @@ describe Import::GithubController do ...@@ -12,9 +12,15 @@ describe Import::GithubController do
it "redirects to GitHub for an access token if logged in with GitHub" 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) 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 get :new
expect(response).to have_http_status(302)
end end
it "prompts for an access token if GitHub not configured" do 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