Commit 5c8fd09c authored by Valery Sizov's avatar Valery Sizov Committed by Imre Farkas

Geo: Fix OAuth authentication with relative URLs

Secondaries settings should not have any effect on
primary URL calculation
parent 70039994
---
title: 'Geo: Fix OAuth authentication with relative URL used on secondary'
merge_request: 37083
author:
type: fixed
......@@ -8,11 +8,12 @@ module Gitlab
include Gitlab::Utils::StrongMemoize
include GrapePathHelpers::NamedRouteMatcher
# We don't use oauth_token_path helper because its output depends
# We don't use oauth_*_path helpers because their outputs depends
# on secondary configuration (ex., relative URL) while we really need
# it for a primary. This is why we're building it ourselves using
# primary node configuration and this static URL
# primary node configuration and these static URLs
TOKEN_PATH = '/oauth/token'
AUTHORIZATION_PATH = '/oauth/authorize'
def authorize_url(params = {})
oauth_client.auth_code.authorize_url(params)
......@@ -41,7 +42,7 @@ module Gitlab
oauth_application&.uid,
oauth_application&.secret,
site: Gitlab::Geo.primary_node.url,
authorize_url: oauth_authorization_path,
authorize_url: oauth_authorization_url,
token_url: token_url
)
end
......@@ -54,6 +55,10 @@ module Gitlab
def token_url
Gitlab::Utils.append_path(Gitlab::Geo.primary_node.internal_url, TOKEN_PATH)
end
def oauth_authorization_url
Gitlab::Utils.append_path(Gitlab::Geo.primary_node.internal_url, AUTHORIZATION_PATH)
end
end
end
end
......
......@@ -19,6 +19,23 @@ RSpec.describe Gitlab::Geo::Oauth::Session, :geo do
it 'returns a valid url to the primary node' do
expect(subject.authorize_url).to start_with(primary_node.internal_url)
end
context 'secondary is configured with relative URL' do
def stub_relative_url(host, script_name)
url_options = { host: host, protocol: "http", port: nil, script_name: script_name }
allow(Rails.application.routes).to receive(:default_url_options).and_return(url_options)
end
it 'does not include secondary relative URL path' do
secondary_url = 'http://secondary.host/relative-path/'
stub_config_setting(url: secondary_url, https: false)
stub_relative_url('secondary.host', '/relative-path')
expect(subject.authorize_url).not_to include('relative-path')
end
end
end
describe '#authenticate' 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