Handles leading and trailing slashes correctly on return_to path

parent cded1917
...@@ -27,6 +27,8 @@ module Oauth2 ...@@ -27,6 +27,8 @@ module Oauth2
if logout_token&.is_utf8? if logout_token&.is_utf8?
Doorkeeper::AccessToken.by_token(logout_token) Doorkeeper::AccessToken.by_token(logout_token)
else
nil
end end
end end
end end
...@@ -37,7 +39,7 @@ module Oauth2 ...@@ -37,7 +39,7 @@ module Oauth2
def user_return_to def user_return_to
full_path = oauth_session.get_oauth_state_return_to_full_path full_path = oauth_session.get_oauth_state_return_to_full_path
URI.join(geo_node_url, full_path).to_s Gitlab::Utils.append_path(geo_node_url, full_path)
end end
def geo_node_url def geo_node_url
......
...@@ -132,7 +132,13 @@ module Gitlab ...@@ -132,7 +132,13 @@ module Gitlab
def full_path def full_path
uri = parse_uri(location) uri = parse_uri(location)
full_path_for_uri(uri) if uri
if uri
path = remove_domain_from_uri(uri)
path = add_fragment_back_to_path(uri, path)
path
end
end end
private private
...@@ -140,14 +146,17 @@ module Gitlab ...@@ -140,14 +146,17 @@ module Gitlab
attr_reader :location attr_reader :location
def parse_uri(location) def parse_uri(location)
location && URI.parse(location) location && URI.parse(location.sub(/\A\/\/+/, '/'))
rescue URI::InvalidURIError rescue URI::InvalidURIError
nil nil
end end
def full_path_for_uri(uri) def remove_domain_from_uri(uri)
path_with_query = [uri.path, uri.query].compact.join('?') [uri.path.sub(/\A\/+/, '/'), uri.query].compact.join('?')
[path_with_query, uri.fragment].compact.join("#") end
def add_fragment_back_to_path(uri, path)
[path, uri.fragment].compact.join('#')
end end
end end
......
...@@ -70,7 +70,7 @@ describe Oauth2::LogoutTokenValidationService do ...@@ -70,7 +70,7 @@ describe Oauth2::LogoutTokenValidationService do
it 'returns the fullpath to the Geo node to redirect the user back' do it 'returns the fullpath to the Geo node to redirect the user back' do
result = described_class.new(user, state: logout_state).execute result = described_class.new(user, state: logout_state).execute
expect(result).to include(return_to: URI.join(node.url, oauth_return_to).to_s) expect(result).to include(return_to: "#{node.url.chomp('/')}/project/test")
end end
it 'replaces the host with the Geo node associated with OAuth application' do it 'replaces the host with the Geo node associated with OAuth application' do
...@@ -80,7 +80,17 @@ describe Oauth2::LogoutTokenValidationService do ...@@ -80,7 +80,17 @@ describe Oauth2::LogoutTokenValidationService do
result = described_class.new(user, state: logout_state).execute result = described_class.new(user, state: logout_state).execute
expect(result).to include(return_to: URI.join(node.url, '/project/test').to_s) expect(result).to include(return_to: "#{node.url.chomp('/')}/project/test")
end
it 'handles leading and trailing slashes correctly on return_to path' do
oauth_return_to = '//project/test'
oauth_session = Gitlab::Geo::OauthSession.new(access_token: access_token.token, return_to: oauth_return_to)
logout_state = oauth_session.generate_logout_state
result = described_class.new(user, state: logout_state).execute
expect(result).to include(return_to: "#{node.url.chomp('/')}/project/test")
end end
end end
end end
......
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