From c77fd1d1cfaacda5051f9c8e831dca9a365abf84 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Jarka=20Ko=C5=A1anov=C3=A1?= <jarka@gitlab.com>
Date: Tue, 12 Feb 2019 09:35:34 +0100
Subject: [PATCH 01/20] Check if labels are available for target issuable

- labels have to be in the same project/group
as an issuable
---
 app/models/label.rb                           |  4 +
 app/services/issuable_base_service.rb         | 20 +++--
 .../labels/available_labels_service.rb        | 60 +++++++++++++
 ee/app/services/ee/boards/base_service.rb     | 21 ++---
 .../unreleased/security-milestone-labels.yml  |  5 ++
 .../projects/boards_controller_spec.rb        |  2 +-
 .../services/boards/update_service_spec.rb    | 21 ++++-
 .../labels/available_labels_service_spec.rb   | 86 +++++++++++++++++++
 8 files changed, 193 insertions(+), 26 deletions(-)
 create mode 100644 app/services/labels/available_labels_service.rb
 create mode 100644 ee/changelogs/unreleased/security-milestone-labels.yml
 create mode 100644 spec/services/labels/available_labels_service_spec.rb

diff --git a/app/models/label.rb b/app/models/label.rb
index 96bdb7f17c5482..e13a2322c6b0ed 100644
--- a/app/models/label.rb
+++ b/app/models/label.rb
@@ -133,6 +133,10 @@ def self.min_chars_for_partial_matching
     1
   end
 
+  def self.by_ids(ids)
+    where(id: ids)
+  end
+
   def open_issues_count(user = nil)
     issues_count(user, state: 'opened')
   end
diff --git a/app/services/issuable_base_service.rb b/app/services/issuable_base_service.rb
index e18dc04b8d78bb..042f0e0cd71493 100644
--- a/app/services/issuable_base_service.rb
+++ b/app/services/issuable_base_service.rb
@@ -70,10 +70,14 @@ def filter_milestone
   end
 
   def filter_labels
-    filter_labels_in_param(:add_label_ids)
-    filter_labels_in_param(:remove_label_ids)
-    filter_labels_in_param(:label_ids)
-    find_or_create_label_ids
+    params[:add_label_ids] = labels_service.filter_labels_ids_in_param(:add_label_ids) if params[:add_label_ids]
+    params[:remove_label_ids] = labels_service.filter_labels_ids_in_param(:remove_label_ids) if params[:remove_label_ids]
+
+    if params[:label_ids]
+      params[:label_ids] = labels_service.filter_labels_ids_in_param(:label_ids)
+    elsif params[:labels]
+      params[:label_ids] = labels_service.find_or_create_by_titles.map(&:id)
+    end
   end
 
   # rubocop: disable CodeReuse/ActiveRecord
@@ -101,6 +105,10 @@ def find_or_create_label_ids
     end.compact
   end
 
+  def labels_service
+    @labels_service ||= ::Labels::AvailableLabelsService.new(current_user, parent, params)
+  end
+
   def process_label_ids(attributes, existing_label_ids: nil)
     label_ids = attributes.delete(:label_ids)
     add_label_ids = attributes.delete(:add_label_ids)
@@ -118,10 +126,6 @@ def process_label_ids(attributes, existing_label_ids: nil)
     new_label_ids
   end
 
-  def available_labels
-    @available_labels ||= LabelsFinder.new(current_user, project_id: @project.id, include_ancestor_groups: true).execute
-  end
-
   def handle_quick_actions_on_create(issuable)
     merge_quick_actions_into_params!(issuable)
   end
diff --git a/app/services/labels/available_labels_service.rb b/app/services/labels/available_labels_service.rb
new file mode 100644
index 00000000000000..fe477d96970edc
--- /dev/null
+++ b/app/services/labels/available_labels_service.rb
@@ -0,0 +1,60 @@
+# frozen_string_literal: true
+module Labels
+  class AvailableLabelsService
+    attr_reader :current_user, :parent, :params
+
+    def initialize(current_user, parent, params)
+      @current_user = current_user
+      @parent = parent
+      @params = params
+    end
+
+    def find_or_create_by_titles
+      labels = params.delete(:labels)
+
+      return [] unless labels
+
+      labels = labels.split(',') if labels.is_a?(String)
+
+      labels.map do |label_name|
+        label = Labels::FindOrCreateService.new(
+          current_user,
+          parent,
+          include_ancestor_groups: true,
+          title: label_name.strip,
+          available_labels: available_labels
+        ).execute
+
+        label
+      end.compact
+    end
+
+    def filter_labels_ids_in_param(key)
+      return [] if params[key].to_a.empty?
+
+      # rubocop:disable CodeReuse/ActiveRecord
+      available_labels.by_ids(params[key]).pluck(:id)
+      # rubocop:enable CodeReuse/ActiveRecord
+    end
+
+    private
+
+    def available_labels
+      @available_labels ||= LabelsFinder.new(current_user, finder_params).execute
+    end
+
+    def finder_params
+      params = { include_ancestor_groups: true }
+
+      case parent
+      when Group
+        params[:group_id] = parent.id
+        params[:only_group_labels] = true
+      when Project
+        params[:project_id] = parent.id
+      end
+
+      params
+    end
+  end
+end
diff --git a/ee/app/services/ee/boards/base_service.rb b/ee/app/services/ee/boards/base_service.rb
index 591bebf85e74dd..4670aa3d533383 100644
--- a/ee/app/services/ee/boards/base_service.rb
+++ b/ee/app/services/ee/boards/base_service.rb
@@ -35,20 +35,15 @@ def set_milestone
       # rubocop: enable CodeReuse/ActiveRecord
 
       def set_labels
-        labels = params.delete(:labels)
-
-        return unless labels
-
-        params[:label_ids] = labels.split(",").map do |label_name|
-          label = Labels::FindOrCreateService.new(
-            current_user,
-            parent,
-            title: label_name.strip,
-            include_ancestor_groups: true
-          ).execute
+        if params[:label_ids]
+          params[:label_ids] = labels_service.filter_labels_ids_in_param(:label_ids)
+        elsif params[:labels]
+          params[:label_ids] = labels_service.find_or_create_by_titles.map(&:id)
+        end
+      end
 
-          label.try(:id)
-        end.compact
+      def labels_service
+        @labels_service ||= ::Labels::AvailableLabelsService.new(current_user, parent, params)
       end
     end
   end
diff --git a/ee/changelogs/unreleased/security-milestone-labels.yml b/ee/changelogs/unreleased/security-milestone-labels.yml
new file mode 100644
index 00000000000000..4f8abcbc8bee09
--- /dev/null
+++ b/ee/changelogs/unreleased/security-milestone-labels.yml
@@ -0,0 +1,5 @@
+---
+title: Check label_ids parent when updating issue board
+merge_request:
+author:
+type: security
diff --git a/ee/spec/controllers/projects/boards_controller_spec.rb b/ee/spec/controllers/projects/boards_controller_spec.rb
index 08079f06b38ef3..df4bbdde147c69 100644
--- a/ee/spec/controllers/projects/boards_controller_spec.rb
+++ b/ee/spec/controllers/projects/boards_controller_spec.rb
@@ -141,7 +141,7 @@ def create_board(board_params)
     let(:board) { create(:board, project: project, name: 'Backend') }
     let(:user) { create(:user) }
     let(:milestone) { create(:milestone, project: project) }
-    let(:label) { create(:label) }
+    let(:label) { create(:label, project: project) }
 
     let(:update_params) do
       { name: 'Frontend',
diff --git a/ee/spec/services/boards/update_service_spec.rb b/ee/spec/services/boards/update_service_spec.rb
index 4324611fd71553..1833d931385be1 100644
--- a/ee/spec/services/boards/update_service_spec.rb
+++ b/ee/spec/services/boards/update_service_spec.rb
@@ -17,7 +17,7 @@
   end
 
   describe '#execute' do
-    let(:project) { create(:project) }
+    let(:project) { create(:project, group: group) }
     let(:group) { create(:group) }
     let!(:board)  { create(:board, group: group, name: 'Backend') }
 
@@ -44,10 +44,11 @@
     it 'updates the configuration params when scoped issue board is enabled' do
       stub_licensed_features(scoped_issue_board: true)
       assignee = create(:user)
-      milestone = create(:milestone, project: project)
-      label = create(:label, project: project)
+      milestone = create(:milestone, group: group)
+      label = create(:group_label, group: board.group)
+      user = create(:user)
 
-      service = described_class.new(project, double,
+      service = described_class.new(group, user,
                                     milestone_id: milestone.id,
                                     assignee_id: assignee.id,
                                     label_ids: [label.id])
@@ -213,6 +214,18 @@ def expect_label_assigned(user, board, input_labels, expected_labels)
             expect_label_assigned(user, board, %w{group_label project_label new_label}, %w{group_label project_label})
           end
         end
+
+        context 'when label_ids param is provided' do
+          it 'updates using only labels accessible by the project board' do
+            other_project_label = create(:label, title: 'other_project_label')
+            other_group_label = create(:group_label, title: 'other_group_label')
+            label_ids = [group_label.id, label.id, other_project_label.id, other_group_label.id]
+
+            described_class.new(board.parent, user, label_ids: label_ids).execute(board)
+
+            expect(board.reload.labels).to contain_exactly(group_label, label)
+          end
+        end
       end
     end
   end
diff --git a/spec/services/labels/available_labels_service_spec.rb b/spec/services/labels/available_labels_service_spec.rb
new file mode 100644
index 00000000000000..4d5c87ecc53671
--- /dev/null
+++ b/spec/services/labels/available_labels_service_spec.rb
@@ -0,0 +1,86 @@
+# frozen_string_literal: true
+require 'spec_helper'
+
+describe Labels::AvailableLabelsService do
+  let(:user) { create(:user) }
+  let(:project) { create(:project, :public, group: group) }
+  let(:group) { create(:group) }
+
+  let(:project_label) { create(:label, project: project) }
+  let(:other_project_label) { create(:label) }
+  let(:group_label) { create(:group_label, group: group) }
+  let(:other_group_label) { create(:group_label) }
+  let(:labels) { [project_label, other_project_label, group_label, other_group_label] }
+
+  context '#find_or_create_by_titles' do
+    let(:label_titles) { labels.map(&:title).push('non existing title') }
+
+    context 'when parent is a project' do
+      context 'when a user is not a project member' do
+        it 'returns only relevant label ids' do
+          result = described_class.new(user, project, labels: label_titles).find_or_create_by_titles
+
+          expect(result).to match_array([project_label, group_label])
+        end
+      end
+
+      context 'when a user is a project member' do
+        before do
+          project.add_developer(user)
+        end
+
+        it 'creates new labels for not found titles' do
+          result = described_class.new(user, project, labels: label_titles).find_or_create_by_titles
+
+          expect(result.count).to eq(5)
+          expect(result).to include(project_label, group_label)
+          expect(result).not_to include(other_project_label, other_group_label)
+        end
+      end
+    end
+
+    context 'when parent is a group' do
+      context 'when a user is not a group member' do
+        it 'returns only relevant label ids' do
+          result = described_class.new(user, group, labels: label_titles).find_or_create_by_titles
+
+          expect(result).to match_array([group_label])
+        end
+      end
+
+      context 'when a user is a group member' do
+        before do
+          group.add_developer(user)
+        end
+
+        it 'creates new labels for not found titles' do
+          result = described_class.new(user, group, labels: label_titles).find_or_create_by_titles
+
+          expect(result.count).to eq(5)
+          expect(result).to include(group_label)
+          expect(result).not_to include(project_label, other_project_label, other_group_label)
+        end
+      end
+    end
+  end
+
+  context '#filter_labels_ids_in_param' do
+    let(:label_ids) { labels.map(&:id).push(99999) }
+
+    context 'when parent is a project' do
+      it 'returns only relevant label ids' do
+        result = described_class.new(user, project, ids: label_ids).filter_labels_ids_in_param(:ids)
+
+        expect(result).to match_array([project_label.id, group_label.id])
+      end
+    end
+
+    context 'when parent is a group' do
+      it 'returns only relevant label ids' do
+        result = described_class.new(user, group, ids: label_ids).filter_labels_ids_in_param(:ids)
+
+        expect(result).to match_array([group_label.id])
+      end
+    end
+  end
+end
-- 
GitLab


From 5b71f0808993efc84c51b86cad7b0e7f532ee5b8 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Ma=C5=82gorzata=20Ksionek?= <meksionek@gmail.com>
Date: Wed, 20 Mar 2019 14:39:07 +0100
Subject: [PATCH 02/20] Disallow changing namespace of a project in update
 method

---
 app/controllers/projects_controller.rb          | 13 ++++++++-----
 ...curity-mass-assignment-on-project-update.yml |  5 +++++
 spec/controllers/projects_controller_spec.rb    | 17 +++++++++++++++++
 3 files changed, 30 insertions(+), 5 deletions(-)
 create mode 100644 changelogs/unreleased/security-mass-assignment-on-project-update.yml

diff --git a/app/controllers/projects_controller.rb b/app/controllers/projects_controller.rb
index 8d851bb5733e55..0b453d117082ae 100644
--- a/app/controllers/projects_controller.rb
+++ b/app/controllers/projects_controller.rb
@@ -47,7 +47,7 @@ def edit
   end
 
   def create
-    @project = ::Projects::CreateService.new(current_user, project_params).execute
+    @project = ::Projects::CreateService.new(current_user, project_params(attributes: project_params_create_attributes)).execute
 
     if @project.saved?
       cookies[:issue_board_welcome_hidden] = { path: project_path(@project), value: nil, expires: Time.at(0) }
@@ -328,9 +328,9 @@ def load_events
   end
   # rubocop: enable CodeReuse/ActiveRecord
 
-  def project_params
+  def project_params(attributes: project_params_attributes)
     params.require(:project)
-      .permit(project_params_attributes)
+      .permit(attributes)
   end
 
   def project_params_attributes
@@ -349,11 +349,10 @@ def project_params_attributes
       :last_activity_at,
       :lfs_enabled,
       :name,
-      :namespace_id,
       :only_allow_merge_if_all_discussions_are_resolved,
       :only_allow_merge_if_pipeline_succeeds,
-      :printing_merge_request_link_enabled,
       :path,
+      :printing_merge_request_link_enabled,
       :public_builds,
       :request_access_enabled,
       :runners_token,
@@ -375,6 +374,10 @@ def project_params_attributes
     ]
   end
 
+  def project_params_create_attributes
+    project_params_attributes << :namespace_id
+  end
+
   def custom_import_params
     {}
   end
diff --git a/changelogs/unreleased/security-mass-assignment-on-project-update.yml b/changelogs/unreleased/security-mass-assignment-on-project-update.yml
new file mode 100644
index 00000000000000..8657dcdd135a98
--- /dev/null
+++ b/changelogs/unreleased/security-mass-assignment-on-project-update.yml
@@ -0,0 +1,5 @@
+---
+title: Disallow updating namespace during updating project
+merge_request:
+author:
+type: security
diff --git a/spec/controllers/projects_controller_spec.rb b/spec/controllers/projects_controller_spec.rb
index 578b6d81461847..6527049562c210 100644
--- a/spec/controllers/projects_controller_spec.rb
+++ b/spec/controllers/projects_controller_spec.rb
@@ -392,6 +392,23 @@
         end
       end
 
+      it 'does not update namespace' do
+        controller.instance_variable_set(:@project, project)
+
+        params = {
+          namespace_id: 'test'
+        }
+
+        expect do
+          put :update,
+            params: {
+              namespace_id: project.namespace,
+              id: project.id,
+              project: params
+            }
+        end.not_to change {project.namespace}
+      end
+
       def update_project(**parameters)
         put :update,
             params: {
-- 
GitLab


From 1b10edf2ffda18d2a1e513008c8b17104e86ce00 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Ma=C5=82gorzata=20Ksionek?= <meksionek@gmail.com>
Date: Mon, 25 Mar 2019 11:08:20 +0100
Subject: [PATCH 03/20] Add cr remarks

---
 app/controllers/projects_controller.rb                      | 6 +++---
 .../security-mass-assignment-on-project-update.yml          | 2 +-
 spec/controllers/projects_controller_spec.rb                | 2 +-
 3 files changed, 5 insertions(+), 5 deletions(-)

diff --git a/app/controllers/projects_controller.rb b/app/controllers/projects_controller.rb
index 0b453d117082ae..e460917316d9af 100644
--- a/app/controllers/projects_controller.rb
+++ b/app/controllers/projects_controller.rb
@@ -328,9 +328,9 @@ def load_events
   end
   # rubocop: enable CodeReuse/ActiveRecord
 
-  def project_params(attributes: project_params_attributes)
+  def project_params(attributes: [])
     params.require(:project)
-      .permit(attributes)
+      .permit(project_params_attributes + attributes)
   end
 
   def project_params_attributes
@@ -375,7 +375,7 @@ def project_params_attributes
   end
 
   def project_params_create_attributes
-    project_params_attributes << :namespace_id
+    [:namespace_id]
   end
 
   def custom_import_params
diff --git a/changelogs/unreleased/security-mass-assignment-on-project-update.yml b/changelogs/unreleased/security-mass-assignment-on-project-update.yml
index 8657dcdd135a98..93561cd91b3a1a 100644
--- a/changelogs/unreleased/security-mass-assignment-on-project-update.yml
+++ b/changelogs/unreleased/security-mass-assignment-on-project-update.yml
@@ -1,5 +1,5 @@
 ---
-title: Disallow updating namespace during updating project
+title: Disallow updating namespace when updating a project
 merge_request:
 author:
 type: security
diff --git a/spec/controllers/projects_controller_spec.rb b/spec/controllers/projects_controller_spec.rb
index 6527049562c210..ad7d3969b44af7 100644
--- a/spec/controllers/projects_controller_spec.rb
+++ b/spec/controllers/projects_controller_spec.rb
@@ -406,7 +406,7 @@
               id: project.id,
               project: params
             }
-        end.not_to change {project.namespace}
+        end.not_to change {project.reload.namespace}
       end
 
       def update_project(**parameters)
-- 
GitLab


From 8577679632d9bcaf9ad60bf4b444f89533d0dcc5 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Ma=C5=82gorzata=20Ksionek?= <meksionek@gmail.com>
Date: Tue, 26 Mar 2019 09:30:16 +0100
Subject: [PATCH 04/20] Refactor specs according to the code review

---
 spec/controllers/projects_controller_spec.rb | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/spec/controllers/projects_controller_spec.rb b/spec/controllers/projects_controller_spec.rb
index ad7d3969b44af7..d2e6a58a71df67 100644
--- a/spec/controllers/projects_controller_spec.rb
+++ b/spec/controllers/projects_controller_spec.rb
@@ -406,7 +406,7 @@
               id: project.id,
               project: params
             }
-        end.not_to change {project.reload.namespace}
+        end.not_to change { project.namespace.reload }
       end
 
       def update_project(**parameters)
-- 
GitLab


From 1fb58b1535f8b81232de582b48eff7769928d355 Mon Sep 17 00:00:00 2001
From: Douglas Barbosa Alexandre <dbalexandre@gmail.com>
Date: Thu, 14 Feb 2019 17:21:48 -0200
Subject: [PATCH 05/20] Use secure_compare to verify the validity of the
 provided HMAC

---
 ee/lib/gitlab/geo/oauth/login_state.rb | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/ee/lib/gitlab/geo/oauth/login_state.rb b/ee/lib/gitlab/geo/oauth/login_state.rb
index c855f7a381f1d8..d83a75be35a7b9 100644
--- a/ee/lib/gitlab/geo/oauth/login_state.rb
+++ b/ee/lib/gitlab/geo/oauth/login_state.rb
@@ -20,7 +20,7 @@ def initialize(return_to:, salt: nil, hmac: nil)
         def valid?
           return false unless salt.present? && hmac.present?
 
-          hmac == generate_hmac
+          ActiveSupport::SecurityUtils.secure_compare(hmac, generate_hmac)
         end
 
         def encode
-- 
GitLab


From c6bea8e072c00c6dd126e81ed7f1ce71ee8d0869 Mon Sep 17 00:00:00 2001
From: Douglas Barbosa Alexandre <dbalexandre@gmail.com>
Date: Thu, 14 Feb 2019 18:08:56 -0200
Subject: [PATCH 06/20] Use a key derivation mechanism to generate the HMAC key

---
 ee/lib/gitlab/geo/oauth/login_state.rb           | 9 ++++++---
 ee/spec/lib/gitlab/geo/oauth/login_state_spec.rb | 4 ++--
 2 files changed, 8 insertions(+), 5 deletions(-)

diff --git a/ee/lib/gitlab/geo/oauth/login_state.rb b/ee/lib/gitlab/geo/oauth/login_state.rb
index d83a75be35a7b9..e3cfbc4795e41a 100644
--- a/ee/lib/gitlab/geo/oauth/login_state.rb
+++ b/ee/lib/gitlab/geo/oauth/login_state.rb
@@ -32,10 +32,13 @@ def encode
         attr_reader :hmac
 
         def generate_hmac
-          digest = OpenSSL::Digest::SHA256.new
-          key = Gitlab::Application.secrets.secret_key_base + salt
+          OpenSSL::HMAC.hexdigest(OpenSSL::Digest::SHA256.new, key, return_to.to_s)
+        end
 
-          OpenSSL::HMAC.hexdigest(digest, key, return_to.to_s)
+        def key
+          ActiveSupport::KeyGenerator
+            .new(Gitlab::Application.secrets.secret_key_base)
+            .generate_key(salt)
         end
 
         def salt
diff --git a/ee/spec/lib/gitlab/geo/oauth/login_state_spec.rb b/ee/spec/lib/gitlab/geo/oauth/login_state_spec.rb
index e3c4fd2a80b21c..f4b894c9626c8b 100644
--- a/ee/spec/lib/gitlab/geo/oauth/login_state_spec.rb
+++ b/ee/spec/lib/gitlab/geo/oauth/login_state_spec.rb
@@ -3,8 +3,8 @@
 require 'spec_helper'
 
 describe Gitlab::Geo::Oauth::LoginState do
-  let(:salt) { '100d8cbd1750a2bb' }
-  let(:hmac) { '62fdcface89baab582f33de6672f10499974c28b5cc269795c4830b8b3ab06be' }
+  let(:salt) { 'b9653b6aa2ff6b54' }
+  let(:hmac) { '908844004aa6ba7237be5cd394499a79e64c054e9b8021bd9b43ff7dc508320b' }
   let(:oauth_return_to) { 'http://fake-secondary.com:3000/project/test' }
 
   before do
-- 
GitLab


From cedac0c66ca5ca4a5f59ae38c8b8465ebe3885d9 Mon Sep 17 00:00:00 2001
From: Douglas Barbosa Alexandre <dbalexandre@gmail.com>
Date: Thu, 14 Feb 2019 21:10:16 -0200
Subject: [PATCH 07/20] Prevent open redirect attack with tampered headers

---
 ee/app/controllers/ee/sessions_controller.rb  |  4 +-
 ee/lib/gitlab/geo/oauth/login_state.rb        |  2 +-
 .../ee/sessions_controller_spec.rb            | 58 +++++++++++++++++++
 .../oauth/geo_auth_controller_spec.rb         |  2 +-
 .../lib/gitlab/geo/oauth/login_state_spec.rb  | 40 +++++++++----
 5 files changed, 92 insertions(+), 14 deletions(-)
 create mode 100644 ee/spec/controllers/ee/sessions_controller_spec.rb

diff --git a/ee/app/controllers/ee/sessions_controller.rb b/ee/app/controllers/ee/sessions_controller.rb
index e6a6e31cdd02dc..86904c54e77a97 100644
--- a/ee/app/controllers/ee/sessions_controller.rb
+++ b/ee/app/controllers/ee/sessions_controller.rb
@@ -14,7 +14,7 @@ def new
       return super if signed_in?
 
       if ::Gitlab::Geo.secondary_with_primary?
-        redirect_to oauth_geo_auth_url(state: geo_login_state.encode)
+        redirect_to oauth_geo_auth_url(host: ::Gitlab::Geo.current_node.url, state: geo_login_state.encode)
       else
         super
       end
@@ -41,7 +41,7 @@ def geo_logout_state
     end
 
     def geo_return_to_after_login
-      stored_redirect_uri || ::Gitlab::Utils.append_path(root_url, session[:user_return_to].to_s)
+      ::Gitlab::Utils.append_path(root_url, sanitize_redirect(session[:user_return_to].to_s))
     end
 
     def geo_return_to_after_logout
diff --git a/ee/lib/gitlab/geo/oauth/login_state.rb b/ee/lib/gitlab/geo/oauth/login_state.rb
index e3cfbc4795e41a..37a6cf4aff97ae 100644
--- a/ee/lib/gitlab/geo/oauth/login_state.rb
+++ b/ee/lib/gitlab/geo/oauth/login_state.rb
@@ -12,7 +12,7 @@ def self.from_state(state)
         end
 
         def initialize(return_to:, salt: nil, hmac: nil)
-          @return_to = return_to
+          @return_to = Gitlab::ReturnToLocation.new(return_to).full_path
           @salt = salt
           @hmac = hmac
         end
diff --git a/ee/spec/controllers/ee/sessions_controller_spec.rb b/ee/spec/controllers/ee/sessions_controller_spec.rb
new file mode 100644
index 00000000000000..9851d0fa348a41
--- /dev/null
+++ b/ee/spec/controllers/ee/sessions_controller_spec.rb
@@ -0,0 +1,58 @@
+# frozen_string_literal: true
+
+require 'spec_helper'
+
+describe SessionsController do
+  include DeviseHelpers
+  include EE::GeoHelpers
+
+  describe '#new' do
+    before do
+      set_devise_mapping(context: @request)
+    end
+
+    context 'on a Geo secondary node' do
+      set(:primary_node) { create(:geo_node, :primary) }
+      set(:secondary_node) { create(:geo_node) }
+
+      let(:salt) { 'MTAwZDhjYmQxNzUw' }
+      let(:login_state) { Gitlab::Geo::Oauth::LoginState.new(salt: salt, return_to: '/') }
+
+      before do
+        stub_current_geo_node(secondary_node)
+        allow(SecureRandom).to receive(:hex).and_return(salt)
+      end
+
+      context 'with a tampered HOST header' do
+        it 'prevents open redirect attack' do
+          request.headers['HOST'] = 'http://this.is.not.my.host'
+
+          get(:new)
+
+          expect(response).to have_gitlab_http_status(302)
+          expect(response).to redirect_to(oauth_geo_auth_url(host: secondary_node.url, state: login_state.encode))
+        end
+      end
+
+      context 'with a tampered X-Forwarded-Host header' do
+        it 'prevents open redirect attack' do
+          request.headers['X-Forwarded-Host'] = 'http://this.is.not.my.host'
+
+          get(:new)
+
+          expect(response).to have_gitlab_http_status(302)
+          expect(response).to redirect_to(oauth_geo_auth_url(host: secondary_node.url, state: login_state.encode))
+        end
+      end
+
+      context 'without a tampered header' do
+        it 'redirects to oauth_geo_auth_url' do
+          get(:new)
+
+          expect(response).to have_gitlab_http_status(302)
+          expect(response).to redirect_to(oauth_geo_auth_url(host: secondary_node.url, state: login_state.encode))
+        end
+      end
+    end
+  end
+end
diff --git a/ee/spec/controllers/oauth/geo_auth_controller_spec.rb b/ee/spec/controllers/oauth/geo_auth_controller_spec.rb
index 6a262365dc0538..cbfcda5edaf32e 100644
--- a/ee/spec/controllers/oauth/geo_auth_controller_spec.rb
+++ b/ee/spec/controllers/oauth/geo_auth_controller_spec.rb
@@ -58,7 +58,7 @@
       it 'redirects to redirect_url if state is valid' do
         get :callback, params: { state: login_state }
 
-        expect(response).to redirect_to(secondary_node.url)
+        expect(response).to redirect_to('/')
       end
 
       it 'does not display a flash message if state is valid' do
diff --git a/ee/spec/lib/gitlab/geo/oauth/login_state_spec.rb b/ee/spec/lib/gitlab/geo/oauth/login_state_spec.rb
index f4b894c9626c8b..1b2c4a93acfb9d 100644
--- a/ee/spec/lib/gitlab/geo/oauth/login_state_spec.rb
+++ b/ee/spec/lib/gitlab/geo/oauth/login_state_spec.rb
@@ -4,8 +4,8 @@
 
 describe Gitlab::Geo::Oauth::LoginState do
   let(:salt) { 'b9653b6aa2ff6b54' }
-  let(:hmac) { '908844004aa6ba7237be5cd394499a79e64c054e9b8021bd9b43ff7dc508320b' }
-  let(:oauth_return_to) { 'http://fake-secondary.com:3000/project/test' }
+  let(:hmac) { 'd75afcc6faa0fd5133c4512080c42ae579e9d9691bd1731475c287f394a35208' }
+  let(:return_to) { 'http://fake-secondary.com:3000/project/test?foo=bar#zoo' }
 
   before do
     allow(Gitlab::Application.secrets).to receive(:secret_key_base)
@@ -22,7 +22,7 @@
     end
 
     it 'returns a valid instance when state is valid' do
-      expect(described_class.from_state("#{salt}:#{hmac}:#{oauth_return_to}")).to be_valid
+      expect(described_class.from_state("#{salt}:#{hmac}:#{return_to}")).to be_valid
     end
   end
 
@@ -40,31 +40,31 @@
     end
 
     it 'returns false when hmac is nil' do
-      subject = described_class.new(return_to: oauth_return_to, salt: salt, hmac: nil)
+      subject = described_class.new(return_to: return_to, salt: salt, hmac: nil)
 
       expect(subject.valid?).to eq false
     end
 
     it 'returns false when hmac is empty' do
-      subject = described_class.new(return_to: oauth_return_to, salt: salt, hmac: '')
+      subject = described_class.new(return_to: return_to, salt: salt, hmac: '')
 
       expect(subject.valid?).to eq false
     end
 
     it 'returns false when salt not match' do
-      subject = described_class.new(return_to: oauth_return_to, salt: 'salt', hmac: hmac)
+      subject = described_class.new(return_to: return_to, salt: 'salt', hmac: hmac)
 
       expect(subject.valid?).to eq(false)
     end
 
     it 'returns false when hmac does not match' do
-      subject = described_class.new(return_to: oauth_return_to, salt: salt, hmac: 'hmac')
+      subject = described_class.new(return_to: return_to, salt: salt, hmac: 'hmac')
 
       expect(subject.valid?).to eq(false)
     end
 
     it 'returns true when hmac matches' do
-      subject = described_class.new(return_to: oauth_return_to, salt: salt, hmac: hmac)
+      subject = described_class.new(return_to: return_to, salt: salt, hmac: hmac)
 
       expect(subject.valid?).to eq(true)
     end
@@ -78,13 +78,33 @@
     end
 
     it 'returns a string with salt, hmac, and return_to colon separated' do
-      subject = described_class.new(return_to: oauth_return_to)
+      subject = described_class.new(return_to: return_to)
 
       salt, hmac, return_to = subject.encode.split(':', 3)
 
       expect(salt).not_to be_blank
       expect(hmac).not_to be_blank
-      expect(return_to).to eq oauth_return_to
+      expect(return_to).to eq return_to
+    end
+  end
+
+  describe '#return_to' do
+    it 'returns nil when return_to is nil' do
+      subject = described_class.new(return_to: nil)
+
+      expect(subject.return_to).to be_nil
+    end
+
+    it 'returns an empty string when return_to is empty' do
+      subject = described_class.new(return_to: '')
+
+      expect(subject.return_to).to eq('')
+    end
+
+    it 'returns the full path of the return_to URL' do
+      subject = described_class.new(return_to: return_to)
+
+      expect(subject.return_to).to eq('/project/test?foo=bar#zoo')
     end
   end
 end
-- 
GitLab


From 2a081fdb957b8ef730f785e064a0793967db808a Mon Sep 17 00:00:00 2001
From: Douglas Barbosa Alexandre <dbalexandre@gmail.com>
Date: Mon, 18 Feb 2019 16:25:45 -0300
Subject: [PATCH 08/20] Add CHANGELOG entry

---
 .../unreleased/security-geo-2019-q1-recurity-assessment.yml | 6 ++++++
 1 file changed, 6 insertions(+)
 create mode 100644 ee/changelogs/unreleased/security-geo-2019-q1-recurity-assessment.yml

diff --git a/ee/changelogs/unreleased/security-geo-2019-q1-recurity-assessment.yml b/ee/changelogs/unreleased/security-geo-2019-q1-recurity-assessment.yml
new file mode 100644
index 00000000000000..a29b43c3097025
--- /dev/null
+++ b/ee/changelogs/unreleased/security-geo-2019-q1-recurity-assessment.yml
@@ -0,0 +1,6 @@
+---
+title: Geo - Improve security while redirecting user back to the secondary after a
+  logout & re-login via the primary
+merge_request:
+author:
+type: security
-- 
GitLab


From 808c2872b436eb66e034df11eecf498f35701ffb Mon Sep 17 00:00:00 2001
From: Douglas Barbosa Alexandre <dbalexandre@gmail.com>
Date: Tue, 19 Feb 2019 16:00:48 -0300
Subject: [PATCH 09/20] Use JSONWebToken::HMACToken to encode state into
 Geo::Oauth::LoginState

This change is necessary to prevent replay attacks.
---
 ee/lib/gitlab/geo/oauth/login_state.rb        | 40 +++++++++++------
 .../ee/sessions_controller_spec.rb            | 40 ++++++++---------
 .../lib/gitlab/geo/oauth/login_state_spec.rb  | 43 +++++++++++++------
 3 files changed, 75 insertions(+), 48 deletions(-)

diff --git a/ee/lib/gitlab/geo/oauth/login_state.rb b/ee/lib/gitlab/geo/oauth/login_state.rb
index 37a6cf4aff97ae..18be2038be57e5 100644
--- a/ee/lib/gitlab/geo/oauth/login_state.rb
+++ b/ee/lib/gitlab/geo/oauth/login_state.rb
@@ -7,32 +7,42 @@ class LoginState
         attr_reader :return_to
 
         def self.from_state(state)
-          salt, hmac, return_to = state.to_s.split(':', 3)
-          self.new(salt: salt, hmac: hmac, return_to: return_to)
+          salt, token, return_to = state.to_s.split(':', 3)
+          self.new(salt: salt, token: token, return_to: return_to)
         end
 
-        def initialize(return_to:, salt: nil, hmac: nil)
+        def initialize(return_to:, salt: nil, token: nil)
           @return_to = Gitlab::ReturnToLocation.new(return_to).full_path
           @salt = salt
-          @hmac = hmac
+          @token = token
         end
 
-        def valid?
-          return false unless salt.present? && hmac.present?
-
-          ActiveSupport::SecurityUtils.secure_compare(hmac, generate_hmac)
+        def encode
+          "#{salt}:#{hmac_token}:#{return_to}"
         end
 
-        def encode
-          "#{salt}:#{generate_hmac}:#{return_to}"
+        def valid?
+          return false unless salt.present? && token.present?
+
+          decoded_token = JSONWebToken::HMACToken.decode(token, key).first
+          secure_compare(decoded_token.dig('data', 'return_to'))
+        rescue JWT::DecodeError
+          false
         end
 
         private
 
-        attr_reader :hmac
+        attr_reader :token
+
+        def expiration_time
+          1.minute
+        end
 
-        def generate_hmac
-          OpenSSL::HMAC.hexdigest(OpenSSL::Digest::SHA256.new, key, return_to.to_s)
+        def hmac_token
+          hmac_token = JSONWebToken::HMACToken.new(key)
+          hmac_token.expire_time = Time.now + expiration_time
+          hmac_token[:data] = { return_to: return_to.to_s }
+          hmac_token.encoded
         end
 
         def key
@@ -44,6 +54,10 @@ def key
         def salt
           @salt ||= SecureRandom.hex(8)
         end
+
+        def secure_compare(value)
+          ActiveSupport::SecurityUtils.secure_compare(return_to.to_s, value)
+        end
       end
     end
   end
diff --git a/ee/spec/controllers/ee/sessions_controller_spec.rb b/ee/spec/controllers/ee/sessions_controller_spec.rb
index 9851d0fa348a41..58eecc32faba4e 100644
--- a/ee/spec/controllers/ee/sessions_controller_spec.rb
+++ b/ee/spec/controllers/ee/sessions_controller_spec.rb
@@ -15,43 +15,41 @@
       set(:primary_node) { create(:geo_node, :primary) }
       set(:secondary_node) { create(:geo_node) }
 
-      let(:salt) { 'MTAwZDhjYmQxNzUw' }
-      let(:login_state) { Gitlab::Geo::Oauth::LoginState.new(salt: salt, return_to: '/') }
-
       before do
         stub_current_geo_node(secondary_node)
-        allow(SecureRandom).to receive(:hex).and_return(salt)
       end
 
-      context 'with a tampered HOST header' do
-        it 'prevents open redirect attack' do
-          request.headers['HOST'] = 'http://this.is.not.my.host'
-
+      shared_examples 'a valid oauth authentication redirect' do
+        it 'redirects to the correct oauth_geo_auth_url' do
           get(:new)
 
+          redirect_uri = URI.parse(response.location)
+          redirect_params = CGI.parse(redirect_uri.query)
+
           expect(response).to have_gitlab_http_status(302)
-          expect(response).to redirect_to(oauth_geo_auth_url(host: secondary_node.url, state: login_state.encode))
+          expect(response).to redirect_to %r(\A#{primary_node.url}oauth/geo/auth)
+          expect(redirect_params['state'].first).to end_with(':/')
         end
       end
 
-      context 'with a tampered X-Forwarded-Host header' do
-        it 'prevents open redirect attack' do
-          request.headers['X-Forwarded-Host'] = 'http://this.is.not.my.host'
+      context 'with a tampered HOST header' do
+        before do
+          request.headers['HOST'] = 'http://this.is.not.my.host'
+        end
 
-          get(:new)
+        it_behaves_like 'a valid oauth authentication redirect'
+      end
 
-          expect(response).to have_gitlab_http_status(302)
-          expect(response).to redirect_to(oauth_geo_auth_url(host: secondary_node.url, state: login_state.encode))
+      context 'with a tampered X-Forwarded-Host header' do
+        before do
+          request.headers['X-Forwarded-Host'] = 'http://this.is.not.my.host'
         end
+
+        it_behaves_like 'a valid oauth authentication redirect'
       end
 
       context 'without a tampered header' do
-        it 'redirects to oauth_geo_auth_url' do
-          get(:new)
-
-          expect(response).to have_gitlab_http_status(302)
-          expect(response).to redirect_to(oauth_geo_auth_url(host: secondary_node.url, state: login_state.encode))
-        end
+        it_behaves_like 'a valid oauth authentication redirect'
       end
     end
   end
diff --git a/ee/spec/lib/gitlab/geo/oauth/login_state_spec.rb b/ee/spec/lib/gitlab/geo/oauth/login_state_spec.rb
index 1b2c4a93acfb9d..b7e3e612a9c918 100644
--- a/ee/spec/lib/gitlab/geo/oauth/login_state_spec.rb
+++ b/ee/spec/lib/gitlab/geo/oauth/login_state_spec.rb
@@ -4,8 +4,13 @@
 
 describe Gitlab::Geo::Oauth::LoginState do
   let(:salt) { 'b9653b6aa2ff6b54' }
-  let(:hmac) { 'd75afcc6faa0fd5133c4512080c42ae579e9d9691bd1731475c287f394a35208' }
+  let(:token) { 'eyJhbGciOiJIUzI1NiIsInR5cCI6IkpXVCJ9.eyJkYXRhIjp7InJldHVybl90byI6Ii9wcm9qZWN0L3Rlc3Q_Zm9vPWJhciN6b28ifSwianRpIjoiODdjZDQ2M2MtOTgyNC00ZjliLWI5NDMtOGFkMjJmY2E2MmZhIiwiaWF0IjoxNTQ5ODI1MjAwLCJuYmYiOjE1NDk4MjUxOTUsImV4cCI6MTU0OTgyNTI2MH0.qZE6kuoeW6BK1URuIl8l8MiCfGjtTTXixVdMCE80gVA' }
   let(:return_to) { 'http://fake-secondary.com:3000/project/test?foo=bar#zoo' }
+  let(:timestamp) { Time.utc(2019, 2, 10, 19, 0, 0) }
+
+  around do |example|
+    Timecop.freeze(timestamp) { example.run }
+  end
 
   before do
     allow(Gitlab::Application.secrets).to receive(:secret_key_base)
@@ -22,7 +27,7 @@
     end
 
     it 'returns a valid instance when state is valid' do
-      expect(described_class.from_state("#{salt}:#{hmac}:#{return_to}")).to be_valid
+      expect(described_class.from_state("#{salt}:#{token}:#{return_to}")).to be_valid
     end
   end
 
@@ -39,32 +44,42 @@
       expect(subject.valid?).to eq false
     end
 
-    it 'returns false when hmac is nil' do
-      subject = described_class.new(return_to: return_to, salt: salt, hmac: nil)
+    it 'returns false when token is nil' do
+      subject = described_class.new(return_to: return_to, salt: salt, token: nil)
 
       expect(subject.valid?).to eq false
     end
 
-    it 'returns false when hmac is empty' do
-      subject = described_class.new(return_to: return_to, salt: salt, hmac: '')
+    it 'returns false when token is empty' do
+      subject = described_class.new(return_to: return_to, salt: salt, token: '')
 
       expect(subject.valid?).to eq false
     end
 
     it 'returns false when salt not match' do
-      subject = described_class.new(return_to: return_to, salt: 'salt', hmac: hmac)
+      subject = described_class.new(return_to: return_to, salt: 'invalid-salt', token: token)
 
       expect(subject.valid?).to eq(false)
     end
 
-    it 'returns false when hmac does not match' do
-      subject = described_class.new(return_to: return_to, salt: salt, hmac: 'hmac')
+    it 'returns false when token does not match' do
+      subject = described_class.new(return_to: return_to, salt: salt, token: 'invalid-token')
 
       expect(subject.valid?).to eq(false)
     end
 
-    it 'returns true when hmac matches' do
-      subject = described_class.new(return_to: return_to, salt: salt, hmac: hmac)
+    it "returns false when token's expired" do
+      subject = described_class.new(return_to: return_to, salt: salt, token: token)
+
+      # Needs to be at least 120 seconds, because the default expiry is
+      # 60 seconds with an additional 60 second leeway.
+      Timecop.freeze(timestamp + 125) do
+        expect(subject.valid?).to eq(false)
+      end
+    end
+
+    it 'returns true when token matches' do
+      subject = described_class.new(return_to: return_to, salt: salt, token: token)
 
       expect(subject.valid?).to eq(true)
     end
@@ -77,13 +92,13 @@
       expect { subject.encode }.not_to raise_error
     end
 
-    it 'returns a string with salt, hmac, and return_to colon separated' do
+    it 'returns a string with salt, token, and return_to colon separated' do
       subject = described_class.new(return_to: return_to)
 
-      salt, hmac, return_to = subject.encode.split(':', 3)
+      salt, token, return_to = subject.encode.split(':', 3)
 
       expect(salt).not_to be_blank
-      expect(hmac).not_to be_blank
+      expect(token).not_to be_blank
       expect(return_to).to eq return_to
     end
   end
-- 
GitLab


From 91ec7343571e07bfd8334c083560b35864cb0d3b Mon Sep 17 00:00:00 2001
From: Douglas Barbosa Alexandre <dbalexandre@gmail.com>
Date: Mon, 18 Mar 2019 15:26:15 -0300
Subject: [PATCH 10/20] Sanitize the entire geo_return_to_after_login url

---
 ee/app/controllers/ee/sessions_controller.rb       | 2 +-
 ee/spec/controllers/ee/sessions_controller_spec.rb | 2 +-
 2 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/ee/app/controllers/ee/sessions_controller.rb b/ee/app/controllers/ee/sessions_controller.rb
index 86904c54e77a97..9f471db0da45f0 100644
--- a/ee/app/controllers/ee/sessions_controller.rb
+++ b/ee/app/controllers/ee/sessions_controller.rb
@@ -41,7 +41,7 @@ def geo_logout_state
     end
 
     def geo_return_to_after_login
-      ::Gitlab::Utils.append_path(root_url, sanitize_redirect(session[:user_return_to].to_s))
+      sanitize_redirect(::Gitlab::Utils.append_path(root_url, session[:user_return_to].to_s))
     end
 
     def geo_return_to_after_logout
diff --git a/ee/spec/controllers/ee/sessions_controller_spec.rb b/ee/spec/controllers/ee/sessions_controller_spec.rb
index 58eecc32faba4e..eed553c720a026 100644
--- a/ee/spec/controllers/ee/sessions_controller_spec.rb
+++ b/ee/spec/controllers/ee/sessions_controller_spec.rb
@@ -28,7 +28,7 @@
 
           expect(response).to have_gitlab_http_status(302)
           expect(response).to redirect_to %r(\A#{primary_node.url}oauth/geo/auth)
-          expect(redirect_params['state'].first).to end_with(':/')
+          expect(redirect_params['state'].first).to end_with(':')
         end
       end
 
-- 
GitLab


From d042751cbb8cb38a2a34b18fcb444df6ca2b667c Mon Sep 17 00:00:00 2001
From: Douglas Barbosa Alexandre <dbalexandre@gmail.com>
Date: Mon, 18 Mar 2019 15:58:20 -0300
Subject: [PATCH 11/20] Improve test coverage for Oauth::GeoAuthController

---
 .../oauth/geo_auth_controller_spec.rb         | 68 ++++++++++++++++---
 1 file changed, 57 insertions(+), 11 deletions(-)

diff --git a/ee/spec/controllers/oauth/geo_auth_controller_spec.rb b/ee/spec/controllers/oauth/geo_auth_controller_spec.rb
index cbfcda5edaf32e..8e23b27057e621 100644
--- a/ee/spec/controllers/oauth/geo_auth_controller_spec.rb
+++ b/ee/spec/controllers/oauth/geo_auth_controller_spec.rb
@@ -27,12 +27,34 @@
       expect(response).to redirect_to(root_url)
     end
 
-    it "redirects to primary node's oauth endpoint" do
-      oauth_endpoint = Gitlab::Geo::Oauth::Session.new.authorize_url(redirect_uri: oauth_geo_callback_url, state: login_state)
+    shared_examples "a valid redirect to to primary node's oauth endpoint" do
+      it "redirects to primary node's oauth endpoint" do
+        oauth_endpoint = Gitlab::Geo::Oauth::Session.new.authorize_url(redirect_uri: oauth_geo_callback_url, state: login_state)
 
-      get :auth, params: { state: login_state }
+        get :auth, params: { state: login_state }
+
+        expect(response).to redirect_to(oauth_endpoint)
+      end
+    end
+
+    context 'without a tampered header' do
+      it_behaves_like "a valid redirect to to primary node's oauth endpoint"
+    end
+
+    context 'with a tampered HOST header' do
+      before do
+        request.headers['HOST'] = 'http://this.is.not.my.host'
+      end
+
+      it_behaves_like "a valid redirect to to primary node's oauth endpoint"
+    end
+
+    context 'with a tampered X-Forwarded-Host header' do
+      before do
+        request.headers['X-Forwarded-Host'] = 'http://this.is.not.my.host'
+      end
 
-      expect(response).to redirect_to(oauth_endpoint)
+      it_behaves_like "a valid redirect to to primary node's oauth endpoint"
     end
   end
 
@@ -55,16 +77,40 @@
         expect(response).to redirect_to(new_user_session_path)
       end
 
-      it 'redirects to redirect_url if state is valid' do
-        get :callback, params: { state: login_state }
+      context 'with a valid state' do
+        shared_examples 'a valid redirect to redirect_url' do
+          it "redirects to primary node's oauth endpoint" do
+            get :callback, params: { state: login_state }
 
-        expect(response).to redirect_to('/')
-      end
+            expect(response).to redirect_to('/')
+          end
+        end
 
-      it 'does not display a flash message if state is valid' do
-        get :callback, params: { state: login_state }
+        context 'without a tampered header' do
+          it_behaves_like 'a valid redirect to redirect_url'
+        end
+
+        context 'with a tampered HOST header' do
+          before do
+            request.headers['HOST'] = 'http://this.is.not.my.host'
+          end
+
+          it_behaves_like 'a valid redirect to redirect_url'
+        end
+
+        context 'with a tampered X-Forwarded-Host header' do
+          before do
+            request.headers['X-Forwarded-Host'] = 'http://this.is.not.my.host'
+          end
+
+          it_behaves_like 'a valid redirect to redirect_url'
+        end
+
+        it 'does not display a flash message' do
+          get :callback, params: { state: login_state }
 
-        expect(controller).to set_flash[:alert].to(nil)
+          expect(controller).to set_flash[:alert].to(nil)
+        end
       end
     end
 
-- 
GitLab


From 19356c729c849f42030e52fbb1bd0aa7e40a9af5 Mon Sep 17 00:00:00 2001
From: Douglas Barbosa Alexandre <dbalexandre@gmail.com>
Date: Tue, 19 Mar 2019 11:56:57 -0300
Subject: [PATCH 12/20] Fix SessionsController#geo_return_to_after_login

---
 ee/app/controllers/ee/sessions_controller.rb | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/ee/app/controllers/ee/sessions_controller.rb b/ee/app/controllers/ee/sessions_controller.rb
index 9f471db0da45f0..11e92e664912a4 100644
--- a/ee/app/controllers/ee/sessions_controller.rb
+++ b/ee/app/controllers/ee/sessions_controller.rb
@@ -33,7 +33,7 @@ def gitlab_geo_logout
     end
 
     def geo_login_state
-      ::Gitlab::Geo::Oauth::LoginState.new(return_to: geo_return_to_after_login)
+      ::Gitlab::Geo::Oauth::LoginState.new(return_to: sanitize_redirect(geo_return_to_after_login))
     end
 
     def geo_logout_state
@@ -41,7 +41,7 @@ def geo_logout_state
     end
 
     def geo_return_to_after_login
-      sanitize_redirect(::Gitlab::Utils.append_path(root_url, session[:user_return_to].to_s))
+      stored_redirect_uri || ::Gitlab::Utils.append_path(root_url, session[:user_return_to].to_s)
     end
 
     def geo_return_to_after_logout
-- 
GitLab


From 4bcd3ae8ba5c88fc50402686995348b350ffb1f6 Mon Sep 17 00:00:00 2001
From: GitLab Release Tools Bot <robert+release-tools@gitlab.com>
Date: Tue, 26 Mar 2019 21:35:48 +0000
Subject: [PATCH 13/20] Update CHANGELOG-EE.md for 11.9.2-ee

[ci skip]
---
 CHANGELOG-EE.md | 8 ++++++++
 1 file changed, 8 insertions(+)

diff --git a/CHANGELOG-EE.md b/CHANGELOG-EE.md
index b0c839b6c805ae..52788729576aee 100644
--- a/CHANGELOG-EE.md
+++ b/CHANGELOG-EE.md
@@ -1,5 +1,13 @@
 Please view this file on the master branch, on stable branches it's out of date.
 
+## 11.9.2 (2019-03-26)
+
+### Security (2 changes)
+
+- Geo - Improve security while redirecting user back to the secondary after a logout & re-login via the primary.
+- Check label_ids parent when updating issue board.
+
+
 ## 11.9.1 (2019-03-25)
 
 ### Fixed (1 change)
-- 
GitLab


From 7d7a9c9e656cedb85c85a12883065a786d697245 Mon Sep 17 00:00:00 2001
From: GitLab Release Tools Bot <robert+release-tools@gitlab.com>
Date: Tue, 26 Mar 2019 21:35:52 +0000
Subject: [PATCH 14/20] Update CHANGELOG.md for 11.9.2

[ci skip]
---
 CHANGELOG.md | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/CHANGELOG.md b/CHANGELOG.md
index a4c783d8809c02..781155e8effc78 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -2,6 +2,10 @@
 documentation](doc/development/changelog.md) for instructions on adding your own
 entry.
 
+## 11.9.2 (2019-03-26)
+
+- No changes.
+
 ## 11.9.1 (2019-03-25)
 
 ### Fixed (7 changes)
-- 
GitLab


From 93ba67b77384d094daa2f098d92807d72b045024 Mon Sep 17 00:00:00 2001
From: GitLab Release Tools Bot <robert+release-tools@gitlab.com>
Date: Tue, 26 Mar 2019 21:42:45 +0000
Subject: [PATCH 15/20] Update CHANGELOG-EE.md for 11.7.8-ee

[ci skip]
---
 CHANGELOG-EE.md | 8 ++++++++
 1 file changed, 8 insertions(+)

diff --git a/CHANGELOG-EE.md b/CHANGELOG-EE.md
index 52788729576aee..dabf53058c9657 100644
--- a/CHANGELOG-EE.md
+++ b/CHANGELOG-EE.md
@@ -230,6 +230,14 @@ Please view this file on the master branch, on stable branches it's out of date.
 - Creates an EE component for the pipeline graph.
 
 
+## 11.7.8 (2019-03-26)
+
+### Security (2 changes)
+
+- Geo - Improve security while redirecting user back to the secondary after a logout & re-login via the primary.
+- Check label_ids parent when updating issue board.
+
+
 ## 11.7.7 (2019-03-19)
 
 - No changes.
-- 
GitLab


From 55ead2a121f225c0bf3790784c328704832eef31 Mon Sep 17 00:00:00 2001
From: GitLab Release Tools Bot <robert+release-tools@gitlab.com>
Date: Tue, 26 Mar 2019 21:42:50 +0000
Subject: [PATCH 16/20] Update CHANGELOG.md for 11.7.8

[ci skip]
---
 CHANGELOG.md | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/CHANGELOG.md b/CHANGELOG.md
index 781155e8effc78..c784b9d0d36715 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -552,6 +552,10 @@ entry.
 - Creates mixin to reduce code duplication between CE and EE in graph component.
 
 
+## 11.7.8 (2019-03-26)
+
+- No changes.
+
 ## 11.7.7 (2019-03-19)
 
 ### Security (2 changes)
-- 
GitLab


From d7f922f81963c928eb52058415fbc72d64f43674 Mon Sep 17 00:00:00 2001
From: GitLab Release Tools Bot <robert+release-tools@gitlab.com>
Date: Wed, 27 Mar 2019 16:05:51 +0000
Subject: [PATCH 17/20] Update CHANGELOG-EE.md for 11.9.3-ee

[ci skip]
---
 CHANGELOG-EE.md | 7 +++++++
 1 file changed, 7 insertions(+)

diff --git a/CHANGELOG-EE.md b/CHANGELOG-EE.md
index dabf53058c9657..fc1c145bc2ca2f 100644
--- a/CHANGELOG-EE.md
+++ b/CHANGELOG-EE.md
@@ -1,5 +1,12 @@
 Please view this file on the master branch, on stable branches it's out of date.
 
+## 11.9.3 (2019-03-27)
+
+### Security (1 change)
+
+- Check label_ids parent when updating issue board.
+
+
 ## 11.9.2 (2019-03-26)
 
 ### Security (2 changes)
-- 
GitLab


From 83ab9c0b72469a0df1d51438d2a6686430e8400e Mon Sep 17 00:00:00 2001
From: GitLab Release Tools Bot <robert+release-tools@gitlab.com>
Date: Wed, 27 Mar 2019 16:05:54 +0000
Subject: [PATCH 18/20] Update CHANGELOG.md for 11.9.3

[ci skip]
---
 CHANGELOG.md | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/CHANGELOG.md b/CHANGELOG.md
index c784b9d0d36715..5f7cecf09ff09f 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -2,6 +2,10 @@
 documentation](doc/development/changelog.md) for instructions on adding your own
 entry.
 
+## 11.9.3 (2019-03-27)
+
+- No changes.
+
 ## 11.9.2 (2019-03-26)
 
 - No changes.
-- 
GitLab


From 3a710e37b95dfaacbeda517fe12ac4ac11a3b02c Mon Sep 17 00:00:00 2001
From: GitLab Release Tools Bot <robert+release-tools@gitlab.com>
Date: Thu, 28 Mar 2019 15:15:22 +0000
Subject: [PATCH 19/20] Update CHANGELOG-EE.md for 11.7.10-ee

[ci skip]
---
 CHANGELOG-EE.md | 7 +++++++
 1 file changed, 7 insertions(+)

diff --git a/CHANGELOG-EE.md b/CHANGELOG-EE.md
index fc1c145bc2ca2f..66b196c3b084c7 100644
--- a/CHANGELOG-EE.md
+++ b/CHANGELOG-EE.md
@@ -237,6 +237,13 @@ Please view this file on the master branch, on stable branches it's out of date.
 - Creates an EE component for the pipeline graph.
 
 
+## 11.7.10 (2019-03-28)
+
+### Security (1 change)
+
+- Check label_ids parent when updating issue board.
+
+
 ## 11.7.8 (2019-03-26)
 
 ### Security (2 changes)
-- 
GitLab


From 5708636b948c913e21ac16957efd1dce05ad372c Mon Sep 17 00:00:00 2001
From: GitLab Release Tools Bot <robert+release-tools@gitlab.com>
Date: Thu, 28 Mar 2019 15:15:27 +0000
Subject: [PATCH 20/20] Update CHANGELOG.md for 11.7.10

[ci skip]
---
 CHANGELOG.md | 13 +++++++++++++
 1 file changed, 13 insertions(+)

diff --git a/CHANGELOG.md b/CHANGELOG.md
index 5f7cecf09ff09f..7f3704729fc97e 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -556,6 +556,19 @@ entry.
 - Creates mixin to reduce code duplication between CE and EE in graph component.
 
 
+## 11.7.10 (2019-03-28)
+
+### Security (7 changes)
+
+- Disallow guest users from accessing Releases.
+- Fix PDF.js vulnerability.
+- Hide "related branches" when user does not have permission.
+- Fix XSS in resolve conflicts form.
+- Added rake task for removing EXIF data from existing uploads.
+- Disallow updating namespace when updating a project.
+- Use UntrustedRegexp for matching refs policy.
+
+
 ## 11.7.8 (2019-03-26)
 
 - No changes.
-- 
GitLab