Merge branch '4459-redirect-users-back-to-secondary-after-logout-login' into 'master'

Geo - Redirect user back to the secondary after a logout & re-login via the primary

Closes #4459

See merge request gitlab-org/gitlab-ee!8157

ee/app/controllers/ee/sessions_controller.rb

@@ -26,7 +26,11 @@ module EE
     def gitlab_geo_logout
       return unless ::Gitlab::Geo.secondary?
 
-      oauth = ::Gitlab::Geo::OauthSession.new(access_token: session[:access_token])
+      oauth = ::Gitlab::Geo::OauthSession.new(
+        access_token: session[:access_token],
+        return_to: safe_redirect_path_for_url(request.referer)
+      )
+
       @geo_logout_state = oauth.generate_logout_state # rubocop:disable Gitlab/ModuleWithInstanceVariables
     end
 

ee/app/controllers/oauth/geo_auth_controller.rb

@@ -3,7 +3,6 @@ class Oauth::GeoAuthController < ActionController::Base
   rescue_from OAuth2::Error, with: :auth
 
   def auth
-    oauth = Gitlab::Geo::OauthSession.new(state: params[:state])
     unless oauth.oauth_state_valid?
       redirect_to root_url
       return
@@ -12,9 +11,7 @@ class Oauth::GeoAuthController < ActionController::Base
     redirect_to oauth.authorize_url(redirect_uri: oauth_geo_callback_url, state: params[:state])
   end
 
-  # rubocop: disable CodeReuse/ActiveRecord
   def callback
-    oauth = Gitlab::Geo::OauthSession.new(state: params[:state])
     unless oauth.oauth_state_valid?
       redirect_to new_user_session_path
       return
@@ -22,23 +19,22 @@ class Oauth::GeoAuthController < ActionController::Base
 
     token = oauth.get_token(params[:code], redirect_uri: oauth_geo_callback_url)
     remote_user = oauth.authenticate_with_gitlab(token)
+    user = UserFinder.new(remote_user['id']).find_by_id
 
-    user = User.find_by(id: remote_user['id'])
-
-    if user && sign_in(user, bypass: true)
+    if user && bypass_sign_in(user)
       after_sign_in_with_gitlab(token, oauth.get_oauth_state_return_to)
     else
       invalid_credentials
     end
   end
-  # rubocop: enable CodeReuse/ActiveRecord
 
   def logout
     logout = Oauth2::LogoutTokenValidationService.new(current_user, params)
     result = logout.execute
+
     if result[:status] == :success
       sign_out current_user
-      redirect_to root_path
+      after_sign_out_with_gitlab(result[:return_to])
     else
       access_token_error(result[:message])
     end
@@ -46,11 +42,24 @@ class Oauth::GeoAuthController < ActionController::Base
 
   private
 
+  def oauth
+    @oauth ||= Gitlab::Geo::OauthSession.new(state: params[:state])
+  end
+
   def after_sign_in_with_gitlab(token, return_to)
     session[:access_token] = token
+
+    # Prevent alert from popping up on the first page shown after authentication.
+    flash[:alert] = nil
+
     redirect_to(return_to || root_path)
   end
 
+  def after_sign_out_with_gitlab(return_to)
+    session[:user_return_to] = return_to
+    redirect_to(root_path)
+  end
+
   def invalid_credentials
     @error = 'Cannot find user to login. Your account may have been deleted.'
     render :error, layout: 'errors'

ee/app/models/geo_node.rb

@@ -89,6 +89,11 @@ class GeoNode < ActiveRecord::Base
       left_join_status.minimum(:cursor_last_event_id)
     end
 
+    # Tries to find a GeoNode by oauth_application_id, returning nil if none could be found.
+    def find_by_oauth_application_id(oauth_application_id)
+      where(oauth_application_id: oauth_application_id).take
+    end
+
     private
 
     def left_join_status

ee/app/services/oauth2/logout_token_validation_service.rb

 module Oauth2
   class LogoutTokenValidationService < ::BaseService
-    attr_reader :status
+    include Gitlab::Utils::StrongMemoize
+
+    attr_reader :state
 
     def initialize(user, params = {})
-      @params = params
       @current_user = user
+      @state = params[:state]
     end
 
     def execute
-      return error('access token not found') unless access_token
+      return error('Access token could not be found') unless access_token.present?
 
       status = AccessTokenValidationService.new(access_token).validate
+      return error(status) unless status == AccessTokenValidationService::VALID
 
-      if status == AccessTokenValidationService::VALID
-        user = User.find(access_token.resource_owner_id)
+      user = User.find(access_token.resource_owner_id)
 
-        if current_user == user
-          success
-        end
+      if user && user == current_user
+        success(return_to: user_return_to)
       else
-        error(status)
+        error('User could not be found')
       end
     end
 
-    def access_token
-      @access_token ||= begin
-        return unless  params[:state] && !params[:state].empty?
-
-        oauth_session = Gitlab::Geo::OauthSession.new(state: params[:state])
+    private
 
+    def access_token
+      strong_memoize(:access_token) do
         logout_token = oauth_session.extract_logout_token
-        return unless logout_token && logout_token.is_utf8?
 
-        Doorkeeper::AccessToken.by_token(logout_token)
+        if logout_token&.is_utf8?
+          Doorkeeper::AccessToken.by_token(logout_token)
+        else
+          nil
+        end
       end
     end
+
+    def oauth_session
+      @oauth_session ||= Gitlab::Geo::OauthSession.new(state: state)
+    end
+
+    def user_return_to
+      full_path = oauth_session.get_oauth_state_return_to_full_path
+      Gitlab::Utils.append_path(geo_node_url, full_path)
+    end
+
+    def geo_node_url
+      GeoNode.find_by_oauth_application_id(access_token.application_id)&.url
+    end
   end
 end

ee/changelogs/unreleased/4459-redirect-users-back-to-secondary-after-logout-login.yml

+---
+title: Geo - Redirect user back to the secondary after a logout & re-login via the primary
+merge_request: 8157
+author:
+type: fixed

ee/lib/gitlab/geo/oauth_session.rb

@@ -8,46 +8,31 @@ module Gitlab
       attr_accessor :return_to
 
       def oauth_state_valid?
-        return false unless state
-
-        salt, hmac, return_to = state.split(':', 3)
-
-        return false unless return_to
-
-        hmac == generate_oauth_hmac(salt, return_to)
+        salt, hmac, return_to = state.to_s.split(':', 3)
+        LoginState.new(salt, return_to).valid?(hmac)
       end
 
       def generate_oauth_state
-        return unless return_to
-
-        hmac = generate_oauth_hmac(oauth_salt, return_to)
-        self.state = "#{oauth_salt}:#{hmac}:#{return_to}"
+        self.state = LoginState.new(oauth_salt, return_to).encode
       end
 
       def generate_logout_state
-        return unless access_token
-
-        cipher = logout_token_cipher(oauth_salt, :encrypt)
-        encrypted = cipher.update(access_token) + cipher.final
-        self.state = "#{oauth_salt}:#{Base64.urlsafe_encode64(encrypted)}"
-      rescue OpenSSL::OpenSSLError
-        false
+        self.state = LogoutState.new(oauth_salt, access_token, return_to).encode
       end
 
       def extract_logout_token
-        return unless state
-
-        salt, encrypted = state.split(':', 2)
-        decipher = logout_token_cipher(salt, :decrypt)
-        decipher.update(Base64.urlsafe_decode64(encrypted)) + decipher.final
-      rescue OpenSSL::OpenSSLError
-        false
+        salt, encrypted, return_to = state.to_s.split(':', 3)
+        LogoutState.new(salt, encrypted, return_to).decode
       end
 
       def get_oauth_state_return_to
         state.split(':', 3)[2] if state
       end
 
+      def get_oauth_state_return_to_full_path
+        ReturnToLocation.new(get_oauth_state_return_to).full_path
+      end
+
       def authorize_url(params = {})
         oauth_client.auth_code.authorize_url(params)
       end
@@ -65,28 +50,122 @@ module Gitlab
 
       private
 
-      def generate_oauth_hmac(salt, return_to)
-        return false unless return_to
+      class LoginState
+        def initialize(salt, return_to)
+          @salt      = salt
+          @return_to = return_to
+        end
+
+        def valid?(hmac)
+          return false unless salt && return_to
+
+          hmac == generate_hmac
+        end
+
+        def encode
+          return unless salt && return_to
+
+          "#{salt}:#{generate_hmac}:#{return_to}"
+        end
+
+        private
+
+        attr_reader :salt, :return_to
+
+        def generate_hmac
+          digest = OpenSSL::Digest.new('sha256')
+          key    = Gitlab::Application.secrets.secret_key_base + salt
+
+          OpenSSL::HMAC.hexdigest(digest, key, return_to)
+        end
+      end
+
+      class LogoutState
+        def initialize(salt, token, return_to)
+          @salt      = salt
+          @token     = token
+          @return_to = return_to
+        end
+
+        def decode
+          return unless salt && token
 
-        digest = OpenSSL::Digest.new('sha256')
-        key = Gitlab::Application.secrets.secret_key_base + salt
-        OpenSSL::HMAC.hexdigest(digest, key, return_to)
+          decrypt = cipher(salt, :decrypt)
+          decrypt.update(Base64.urlsafe_decode64(token)) + decrypt.final
+        rescue OpenSSL::OpenSSLError
+          nil
+        end
+
+        def encode
+          return unless token
+
+          encrypt   = cipher(salt, :encrypt)
+          encrypted = encrypt.update(token) + encrypt.final
+          encoded   = Base64.urlsafe_encode64(encrypted)
+
+          "#{salt}:#{encoded}:#{full_path}"
+        rescue OpenSSL::OpenSSLError
+          nil
+        end
+
+        private
+
+        attr_reader :salt, :token, :return_to
+
+        def cipher(salt, operation)
+          cipher = OpenSSL::Cipher::AES.new(128, :CBC)
+          cipher.__send__(operation) # rubocop:disable GitlabSecurity/PublicSend
+          cipher.iv = salt
+          cipher.key = Settings.attr_encrypted_db_key_base[0..15]
+          cipher
+        end
+
+        def full_path
+          ReturnToLocation.new(return_to).full_path
+        end
       end
 
-      def logout_token_cipher(salt, operation)
-        cipher = OpenSSL::Cipher::AES.new(128, :CBC)
-        cipher.__send__(operation) # rubocop:disable GitlabSecurity/PublicSend
-        cipher.iv = salt
-        cipher.key = Settings.attr_encrypted_db_key_base[0..15]
-        cipher
+      class ReturnToLocation
+        def initialize(location)
+          @location = location
+        end
+
+        def full_path
+          uri = parse_uri(location)
+
+          if uri
+            path = remove_domain_from_uri(uri)
+            path = add_fragment_back_to_path(uri, path)
+
+            path
+          end
+        end
+
+        private
+
+        attr_reader :location
+
+        def parse_uri(location)
+          location && URI.parse(location.sub(%r{\A\/\/+}, '/'))
+        rescue URI::InvalidURIError
+          nil
+        end
+
+        def remove_domain_from_uri(uri)
+          [uri.path.sub(%r{\A\/+}, '/'), uri.query].compact.join('?')
+        end
+
+        def add_fragment_back_to_path(uri, path)
+          [path, uri.fragment].compact.join('#')
+        end
       end
 
       def oauth_salt
-        @salt ||= SecureRandom.hex(8)
+        @oauth_salt ||= SecureRandom.hex(8)
       end
 
       def oauth_client
-        @client ||= begin
+        @oauth_client ||= begin
           ::OAuth2::Client.new(
             oauth_app.uid,
             oauth_app.secret,

ee/spec/controllers/oauth/geo_auth_controller_spec.rb

@@ -2,9 +2,11 @@ require 'spec_helper'
 
 describe Oauth::GeoAuthController do
   let(:user) { create(:user) }
-  let(:oauth_app) { create(:doorkeeper_application) }
-  let(:access_token) { create(:doorkeeper_access_token, resource_owner_id: user.id).token }
-  let(:auth_state) { Gitlab::Geo::OauthSession.new(access_token: access_token, return_to: projects_url).generate_oauth_state }
+  let(:node) { create(:geo_node) }
+  let(:oauth_app) { node.oauth_application }
+  let(:access_token) { create(:doorkeeper_access_token, resource_owner_id: user.id, application: oauth_app) }
+  let(:oauth_session) { Gitlab::Geo::OauthSession.new(access_token: access_token.token, return_to: projects_url) }
+  let(:auth_state) { oauth_session.generate_oauth_state }
   let(:primary_node_url) { 'http://localhost:3001/' }
 
   before do
@@ -30,7 +32,8 @@ describe Oauth::GeoAuthController do
   end
 
   describe 'GET callback' do
-    let(:callback_state) { Gitlab::Geo::OauthSession.new(access_token: access_token, return_to: projects_url).generate_oauth_state }
+    let(:oauth_session) { Gitlab::Geo::OauthSession.new(access_token: access_token.token, return_to: projects_url) }
+    let(:callback_state) { oauth_session.generate_oauth_state }
     let(:primary_node_oauth_endpoint) { Gitlab::Geo::OauthSession.new.authorize_url(redirect_uri: oauth_geo_callback_url, state: callback_state) }
 
     context 'redirection' do
@@ -51,6 +54,12 @@ describe Oauth::GeoAuthController do
 
         expect(response).to redirect_to(projects_url)
       end
+
+      it 'does not display a flash message if state is valid' do
+        get :callback, state: callback_state
+
+        expect(controller).to set_flash[:alert].to(nil)
+      end
     end
 
     context 'invalid credentials' do
@@ -58,7 +67,7 @@ describe Oauth::GeoAuthController do
       let(:oauth_error) { OAuth2::Error.new(OAuth2::Response.new(fake_response)) }
 
       before do
-        expect_any_instance_of(Gitlab::Geo::OauthSession).to receive(:get_token) { access_token }
+        expect_any_instance_of(Gitlab::Geo::OauthSession).to receive(:get_token) { access_token.token }
         expect_any_instance_of(Gitlab::Geo::OauthSession).to receive(:authenticate_with_gitlab).and_raise(oauth_error)
       end
 
@@ -87,23 +96,27 @@ describe Oauth::GeoAuthController do
   end
 
   describe 'GET logout' do
-    let(:logout_state) { Gitlab::Geo::OauthSession.new(access_token: access_token).generate_logout_state }
+    let(:oauth_session) { Gitlab::Geo::OauthSession.new(access_token: access_token.token) }
+    let(:logout_state) { oauth_session.generate_logout_state }
 
-    context 'access_token error' do
-      render_views
+    render_views
 
-      before do
-        sign_in(user)
-      end
+    before do
+      sign_in(user)
+    end
 
-      it 'logs out when correct access_token is informed' do
+    context 'when access_token is valid' do
+      it 'logs out and redirects to the root_url' do
         get :logout, state: logout_state
 
         expect(response).to redirect_to root_url
       end
+    end
 
+    context 'when access_token is invalid' do
       it 'handles access token problems' do
         allow_any_instance_of(Oauth2::LogoutTokenValidationService).to receive(:execute) { { status: :error, message: :expired } }
+
         get :logout, state: logout_state
 
         expect(response.body).to include("There is a problem with the OAuth access_token: expired")

ee/spec/lib/gitlab/geo/oauth_session_spec.rb

@@ -52,31 +52,62 @@ describe Gitlab::Geo::OauthSession do
   end
 
   describe '#get_oauth_state_return_to' do
-    subject { described_class.new(state: valid_state) }
-
     it 'returns return_to value' do
+      subject = described_class.new(state: valid_state)
+
       expect(subject.get_oauth_state_return_to).to eq(oauth_return_to)
     end
   end
 
+  describe '#get_oauth_state_return_to_full_path' do
+    it 'removes the domain from return_to value' do
+      subject = described_class.new(state: valid_state)
+
+      expect(subject.get_oauth_state_return_to_full_path).to eq('/oauth/geo/callback')
+    end
+  end
+
   describe '#generate_logout_state' do
     it 'returns nil when access_token is not defined' do
       expect(described_class.new.generate_logout_state).to be_nil
     end
 
     it 'returns false when encryptation fails' do
-      allow_any_instance_of(OpenSSL::Cipher::AES).to receive(:final) { raise OpenSSL::OpenSSLError }
+      allow_any_instance_of(OpenSSL::Cipher::AES)
+        .to receive(:final) { raise OpenSSL::OpenSSLError }
+
       expect(subject.generate_logout_state).to be_falsey
     end
 
-    it 'returns a string with salt and encrypted access token colon separated' do
-      state = described_class.new(access_token: access_token).generate_logout_state
+    it 'returns a string with salt, encrypted access token, and return_to colon separated' do
+      subject = described_class.new(access_token: access_token, return_to: oauth_return_to)
+
+      state = subject.generate_logout_state
       expect(state).to be_a String
       expect(state).not_to be_blank
 
-      salt, encrypted = state.split(':', 2)
+      salt, encrypted, return_to = state.split(':', 3)
       expect(salt).not_to be_blank
       expect(encrypted).not_to be_blank
+      expect(return_to).not_to be_blank
+    end
+
+    it 'include a empty value for return_to into state when return_to param is not defined' do
+      subject = described_class.new(access_token: access_token)
+
+      state = subject.generate_logout_state
+      _, _, return_to = state.split(':', 3)
+
+      expect(return_to).to eq ''
+    end
+
+    it 'does not include the host from return_to param into into the state' do
+      subject = described_class.new(access_token: access_token, return_to: oauth_return_to)
+
+      state = subject.generate_logout_state
+      _, _, return_to = state.split(':', 3)
+
+      expect(return_to).to eq '/oauth/geo/callback'
     end
   end
 
@@ -87,9 +118,15 @@ describe Gitlab::Geo::OauthSession do
       expect(subject.extract_logout_token).to be_nil
     end
 
+    it 'returns nil when state is empty' do
+      subject.state = ''
+
+      expect(subject.extract_logout_token).to be_nil
+    end
+
     it 'returns false when decryptation fails' do
-      subject.generate_logout_state
-      allow_any_instance_of(OpenSSL::Cipher::AES).to receive(:final) { raise OpenSSL::OpenSSLError }
+      allow_any_instance_of(OpenSSL::Cipher::AES)
+        .to receive(:final) { raise OpenSSL::OpenSSLError }
 
       expect(subject.extract_logout_token).to be_falsey
     end

ee/spec/models/geo_node_spec.rb

@@ -158,6 +158,24 @@ describe GeoNode, type: :model do
     end
   end
 
+  describe '.find_by_oauth_application_id' do
+    context 'when the Geo node exists' do
+      it 'returns the Geo node' do
+        found = described_class.find_by_oauth_application_id(node.oauth_application_id)
+
+        expect(found).to eq(node)
+      end
+    end
+
+    context 'when the Geo node does not exist' do
+      it 'returns nil' do
+        found = described_class.find_by_oauth_application_id(-1)
+
+        expect(found).to be_nil
+      end
+    end
+  end
+
   describe '#repair' do
     it 'creates an oauth application for a Geo secondary node' do
       stub_current_geo_node(node)

ee/spec/services/oauth2/logout_token_validation_service_spec.rb

 require 'spec_helper'
 
 describe Oauth2::LogoutTokenValidationService do
-  let(:user) { FactoryBot.create(:user) }
-  let(:access_token) { FactoryBot.create(:doorkeeper_access_token, resource_owner_id: user.id).token }
-  let(:logout_state) { Gitlab::Geo::OauthSession.new(access_token: access_token).generate_logout_state }
+  let(:user) { create(:user) }
+  let(:node) { create(:geo_node) }
+  let(:access_token) { create(:doorkeeper_access_token, resource_owner_id: user.id, application_id: node.oauth_application_id) }
+  let(:oauth_return_to) { '/project/test' }
+  let(:oauth_session) { Gitlab::Geo::OauthSession.new(access_token: access_token.token, return_to: oauth_return_to) }
+  let(:logout_state) { oauth_session.generate_logout_state }
 
   context '#execute' do
     it 'return error when params are empty' do
       result = described_class.new(user, {}).execute
+
       expect(result[:status]).to eq(:error)
     end
 
-    it 'returns error when state param is empty' do
-      result = described_class.new(user, { state: nil }).execute
+    it 'returns error when state param is nil' do
+      result = described_class.new(user, state: nil).execute
+
       expect(result[:status]).to eq(:error)
+    end
+
+    it 'returns error when state param is empty' do
+      result = described_class.new(user, state: '').execute
 
-      result = described_class.new(user, { state: '' }).execute
       expect(result[:status]).to eq(:error)
     end
 
-    it 'returns error when incorrect encoding' do
-      invalid_token = "\xD800\xD801\xD802"
-      allow_any_instance_of(Gitlab::Geo::OauthSession).to receive(:extract_logout_token) { invalid_token }
+    it 'returns error when token has incorrect encoding' do
+      allow_any_instance_of(Gitlab::Geo::OauthSession)
+        .to receive(:extract_logout_token)
+        .and_return("\xD800\xD801\xD802")
+
+      result = described_class.new(user, state: logout_state).execute
 
-      result = described_class.new(user, { state: logout_state }).execute
       expect(result[:status]).to eq(:error)
     end
 
-    it 'returns true when token is valid' do
-      result = described_class.new(user, { state: logout_state }).execute
-      expect(result[:status]).to eq(:success)
+    it 'returns error when current user is nil' do
+      result = described_class.new(nil, state: logout_state).execute
+
+      expect(result).to eq(status: :error, message: 'User could not be found')
+    end
+
+    it 'returns error when token owner could not be found' do
+      allow(User).to receive(:find).with(user.id).and_return(nil)
+
+      result = described_class.new(user, state: logout_state).execute
+
+      expect(result).to eq(status: :error, message: 'User could not be found')
+    end
+
+    it 'returns error when token does not belong to the current user' do
+      result = described_class.new(create(:user), state: logout_state).execute
+
+      expect(result).to eq(status: :error, message: 'User could not be found')
+    end
+
+    context 'when token is valid' do
+      it 'returns success' do
+        result = described_class.new(user, state: logout_state).execute
+
+        expect(result).to include(status: :success)
+      end
+
+      context 'when OAuth session return_to param is nil' do
+        it 'returns the Geo node URL associated with OAuth application to redirect the user back' do
+          oauth_session = Gitlab::Geo::OauthSession.new(access_token: access_token.token, return_to: nil)
+          logout_state = oauth_session.generate_logout_state
+
+          result = described_class.new(user, state: logout_state).execute
+
+          expect(result).to include(return_to: node.url)
+        end
+      end
+
+      context 'when OAuth session return_to param is empty' do
+        it 'returns the Geo node URL associated with OAuth application to redirect the user back' do
+          oauth_session = Gitlab::Geo::OauthSession.new(access_token: access_token.token, 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)
+        end
+      end
+
+      context 'when OAuth session return_to param is set' do
+        it 'returns the fullpath to the Geo node to redirect the user back' do
+          result = described_class.new(user, state: logout_state).execute
+
+          expect(result).to include(return_to: "#{node.url.chomp('/')}/project/test")
+        end
+
+        it 'replaces the host with the Geo node associated with OAuth application' do
+          oauth_return_to = 'http://fake-secondary/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
+
+        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

lib/gitlab/utils.rb

@@ -16,6 +16,11 @@ module Gitlab
       str.force_encoding(Encoding::UTF_8)
     end
 
+    # Append path to host, making sure there's one single / in between
+    def append_path(host, path)
+      "#{host.to_s.sub(%r{\/+$}, '')}/#{path.to_s.sub(%r{^\/+}, '')}"
+    end
+
     # A slugified version of the string, suitable for inclusion in URLs and
     # domain names. Rules:
     #

spec/lib/gitlab/utils_spec.rb

@@ -2,7 +2,7 @@ require 'spec_helper'
 
 describe Gitlab::Utils do
   delegate :to_boolean, :boolean_to_yes_no, :slugify, :random_string, :which, :ensure_array_from_string,
-   :bytes_to_megabytes, to: :described_class
+   :bytes_to_megabytes, :append_path, to: :described_class
 
   describe '.slugify' do
     {
@@ -106,4 +106,25 @@ describe Gitlab::Utils do
       expect(bytes_to_megabytes(bytes)).to eq(1)
     end
   end
+
+  describe '.append_path' do
+    using RSpec::Parameterized::TableSyntax
+
+    where(:host, :path, :result) do
+      'http://test/'  | '/foo/bar'  |  'http://test/foo/bar'
+      'http://test/'  | '//foo/bar' |  'http://test/foo/bar'
+      'http://test//' | '/foo/bar'  |  'http://test/foo/bar'
+      'http://test'   | 'foo/bar'   |  'http://test/foo/bar'
+      'http://test//' | ''          |  'http://test/'
+      'http://test//' | nil         |  'http://test/'
+      ''              | '/foo/bar'  |  '/foo/bar'
+      nil             | '/foo/bar'  |  '/foo/bar'
+    end
+
+    with_them do
+      it 'makes sure there is only one slash as path separator' do
+        expect(append_path(host, path)).to eq(result)
+      end
+    end
+  end
 end