WIPextract_gcp_from_controller

6 files changed, 104 insertions, 90 deletions

diff --git a/app/controllers/concerns/gcp_session.rb b/app/controllers/concerns/gcp_session.rb
index 75909aedf4c..00552cb38be 100644
--- a/app/controllers/concerns/gcp_session.rb
+++ b/app/controllers/concerns/gcp_session.rb
@@ -4,11 +4,11 @@ module GcpSession
   extend ActiveSupport::Concern
 
   included do
+    helper_method :gcp_authorize_url
     helper_method :token_in_session
+    helper_method :validated_gcp_token
   end
 
-  private
-
   def gcp_authorize_url(redirect_url)
     state = generate_session_key_redirect(redirect_url.to_s)
 
@@ -19,15 +19,16 @@ module GcpSession
     # no-op
   end
 
+  def token_in_session
+    session[GoogleApi::CloudPlatform::Client.session_key_for_token]
+  end
+
   def validated_gcp_token
-    GoogleApi::CloudPlatform::Client.new(token_in_session, nil)
+    @validated_gcp_token ||= GoogleApi::CloudPlatform::Client.new(token_in_session, nil)
       .validate_token(expires_at_in_session)
   end
 
-  def expires_at_in_session
-    @expires_at_in_session ||=
-      session[GoogleApi::CloudPlatform::Client.session_key_for_expires_at]
-  end
+  private
 
   def generate_session_key_redirect(uri)
     GoogleApi::CloudPlatform::Client.new_session_key_for_redirect_uri do |key|
@@ -35,7 +36,8 @@ module GcpSession
     end
   end
 
-  def token_in_session
-    session[GoogleApi::CloudPlatform::Client.session_key_for_token]
+  def expires_at_in_session
+    @expires_at_in_session ||=
+      session[GoogleApi::CloudPlatform::Client.session_key_for_expires_at]
   end
 end
diff --git a/app/controllers/projects/clusters_controller.rb b/app/controllers/projects/clusters_controller.rb
index 6e063fce296..d1b26ac63a4 100644
--- a/app/controllers/projects/clusters_controller.rb
+++ b/app/controllers/projects/clusters_controller.rb
@@ -5,8 +5,7 @@ class Projects::ClustersController < Projects::ApplicationController
 
   before_action :cluster, except: [:index, :new, :create_gcp, :create_user]
   before_action :authorize_read_cluster!
-  before_action :generate_gcp_authorize_url, only: [:new]
-  before_action :validate_gcp_token, only: [:new]
+  before_action :generate_redirect_after_google_authorize_url, only: [:new, :create_gcp, :create_user]
   before_action :gcp_cluster, only: [:new]
   before_action :user_cluster, only: [:new]
   before_action :authorize_create_cluster!, only: [:new]
@@ -80,8 +79,6 @@ class Projects::ClustersController < Projects::ApplicationController
     if @gcp_cluster.persisted?
       redirect_to project_cluster_path(project, @gcp_cluster)
     else
-      generate_gcp_authorize_url
-      validate_gcp_token
       user_cluster
 
       render :new, locals: { active_tab: 'gcp' }
@@ -96,8 +93,6 @@ class Projects::ClustersController < Projects::ApplicationController
     if @user_cluster.persisted?
       redirect_to project_cluster_path(project, @user_cluster)
     else
-      generate_gcp_authorize_url
-      validate_gcp_token
       gcp_cluster
 
       render :new, locals: { active_tab: 'user' }
@@ -169,8 +164,8 @@ class Projects::ClustersController < Projects::ApplicationController
       )
   end
 
-  def generate_gcp_authorize_url
-    @authorize_url = gcp_authorize_url(new_project_cluster_path(@project))
+  def generate_redirect_after_google_authorize_url
+    @redirect_after_authorize_url = new_project_cluster_path(@project)
   end
 
   def gcp_cluster
@@ -185,10 +180,6 @@ class Projects::ClustersController < Projects::ApplicationController
     end
   end
 
-  def validate_gcp_token
-    @valid_gcp_token = validated_gcp_token
-  end
-
   def authorize_update_cluster!
     access_denied! unless can?(current_user, :update_cluster, cluster)
   end
diff --git a/app/views/projects/clusters/gcp/_form.html.haml b/app/views/projects/clusters/gcp/_form.html.haml
index 171ceeceb68..725679519fb 100644
--- a/app/views/projects/clusters/gcp/_form.html.haml
+++ b/app/views/projects/clusters/gcp/_form.html.haml
@@ -10,7 +10,7 @@
   - link_to_help_page = link_to(s_('ClusterIntegration|help page'), help_page_path('user/project/clusters/index'), target: '_blank', rel: 'noopener noreferrer')
   = s_('ClusterIntegration|Read our %{link_to_help_page} on Kubernetes cluster integration.').html_safe % { link_to_help_page: link_to_help_page}
 
-%p= link_to('Select a different Google account', @authorize_url)
+%p= link_to('Select a different Google account', gcp_authorize_url(@redirect_after_authorize_url))
 
 = form_for @gcp_cluster, html: { class: 'js-gke-cluster-creation prepend-top-20', data: { token: token_in_session } }, url:  create_gcp_namespace_project_clusters_path(@project.namespace, @project), as: :cluster do |field|
   = form_errors(@gcp_cluster)
diff --git a/app/views/projects/clusters/new.html.haml b/app/views/projects/clusters/new.html.haml
index a38003f5750..8729e5e80d9 100644
--- a/app/views/projects/clusters/new.html.haml
+++ b/app/views/projects/clusters/new.html.haml
@@ -20,11 +20,11 @@
     .tab-content.gitlab-tab-content
       .tab-pane{ id: 'create-gcp-cluster-pane', class: active_when(active_tab == 'gcp'), role: 'tabpanel' }
         = render 'projects/clusters/gcp/header'
-        - if @valid_gcp_token
+        - if validated_gcp_token
           = render 'projects/clusters/gcp/form'
-        - elsif @authorize_url
+        - elsif gcp_authorize_url(@redirect_after_authorize_url)
           .signin-with-google
-            = link_to(image_tag('auth_buttons/signin_with_google.png', width: '191px'), @authorize_url)
+            = link_to(image_tag('auth_buttons/signin_with_google.png', width: '191px'), gcp_authorize_url(@redirect_after_authorize_url))
             = _('or')
             = link_to('create a new Google account', 'https://accounts.google.com/SignUpWithoutGmail?service=cloudconsole&continue=https%3A%2F%2Fconsole.cloud.google.com%2Ffreetrial%3Futm_campaign%3D2018_cpanel%26utm_source%3Dgitlab%26utm_medium%3Dreferral', target: '_blank', rel: 'noopener noreferrer')
         - else
diff --git a/spec/controllers/concerns/gcp_session_spec.rb b/spec/controllers/concerns/gcp_session_spec.rb
new file mode 100644
index 00000000000..8b57a404092
--- /dev/null
+++ b/spec/controllers/concerns/gcp_session_spec.rb
@@ -0,0 +1,68 @@
+# frozen_string_literal: true
+
+# extract new and create_gcp, create_user ?
+
+require 'spec_helper'
+
+describe GcpSession do
+  controller(ApplicationController) do
+    # `described_class` is not available in this context
+    include GcpSession # rubocop:disable RSpec/DescribedClass
+  end
+
+  describe '#token_in_session' do
+    subject { controller_class.new.token_in_session }
+
+    it 'runs' do
+      subject
+    end
+  end
+
+  describe '#validated_gcp_token' do
+    context 'when access token is expired' do
+      before do
+        stub_google_api_expired_token
+      end
+
+      it { expect(@valid_gcp_token).to be_falsey }
+    end
+
+    context 'when access token is not stored in session' do
+      it { expect(@valid_gcp_token).to be_falsey }
+    end
+  end
+
+  describe '#gcp_authorize_url' do
+    context 'when omniauth has been configured' do
+      let(:key) { 'secret-key' }
+      let(:session_key_for_redirect_uri) do
+        GoogleApi::CloudPlatform::Client.session_key_for_redirect_uri(key)
+      end
+
+      before do
+        allow(SecureRandom).to receive(:hex).and_return(key)
+      end
+
+      it 'has authorize_url' do
+        go
+
+        expect(assigns(:authorize_url)).to include(key)
+        expect(session[session_key_for_redirect_uri]).to eq(new_project_cluster_path(project))
+      end
+    end
+
+    context 'when omniauth has not configured' do
+      before do
+        stub_omniauth_setting(providers: [])
+      end
+
+      it 'does not have authorize_url' do
+        go
+
+        expect(assigns(:authorize_url)).to be_nil
+      end
+    end
+  end
+
+
+end
diff --git a/spec/controllers/projects/clusters_controller_spec.rb b/spec/controllers/projects/clusters_controller_spec.rb
index 3773b29e5cd..deaa48b36f2 100644
--- a/spec/controllers/projects/clusters_controller_spec.rb
+++ b/spec/controllers/projects/clusters_controller_spec.rb
@@ -83,58 +83,16 @@ describe Projects::ClustersController do
         sign_in(user)
       end
 
-      context 'when omniauth has been configured' do
-        let(:key) { 'secret-key' }
-        let(:session_key_for_redirect_uri) do
-          GoogleApi::CloudPlatform::Client.session_key_for_redirect_uri(key)
-        end
-
-        before do
-          allow(SecureRandom).to receive(:hex).and_return(key)
-        end
-
-        it 'has authorize_url' do
-          go
-
-          expect(assigns(:authorize_url)).to include(key)
-          expect(session[session_key_for_redirect_uri]).to eq(new_project_cluster_path(project))
-        end
-      end
-
-      context 'when omniauth has not configured' do
-        before do
-          stub_omniauth_setting(providers: [])
-        end
-
-        it 'does not have authorize_url' do
-          go
+      it 'has new object' do
+        go
 
-          expect(assigns(:authorize_url)).to be_nil
-        end
+        expect(assigns(:gcp_cluster)).to be_an_instance_of(Clusters::Cluster)
       end
 
-      context 'when access token is valid' do
-        before do
-          stub_google_api_validate_token
-        end
-
-        it 'has new object' do
-          go
-
-          expect(assigns(:gcp_cluster)).to be_an_instance_of(Clusters::Cluster)
-        end
-      end
-
-      context 'when access token is expired' do
-        before do
-          stub_google_api_expired_token
-        end
-
-        it { expect(@valid_gcp_token).to be_falsey }
-      end
+      it 'sets the redirect url after google authorization' do
+        go
 
-      context 'when access token is not stored in session' do
-        it { expect(@valid_gcp_token).to be_falsey }
+        expect(assigns(:redirect_after_authorize_url)).to eq new_project_cluster_path(project)
       end
     end
 
@@ -192,10 +150,6 @@ describe Projects::ClustersController do
       end
 
       context 'when access token is valid' do
-        before do
-          stub_google_api_validate_token
-        end
-
         it 'creates a new cluster' do
           expect(ClusterProvisionWorker).to receive(:perform_async)
           expect { go }.to change { Clusters::Cluster.count }
@@ -228,19 +182,13 @@ describe Projects::ClustersController do
             expect(response).to have_gitlab_http_status(:ok)
             expect(response).to render_template(:new)
           end
-        end
-      end
-
-      context 'when access token is expired' do
-        before do
-          stub_google_api_expired_token
-        end
 
-        it { expect(@valid_gcp_token).to be_falsey }
-      end
+          it 'sets the redirect url after google authorization' do
+            go
 
-      context 'when access token is not stored in session' do
-        it { expect(@valid_gcp_token).to be_falsey }
+            expect(assigns(:redirect_after_authorize_url)).to eq new_project_cluster_path(project)
+          end
+        end
       end
     end
 
@@ -248,8 +196,7 @@ describe Projects::ClustersController do
       before do
         allow_any_instance_of(described_class)
         .to receive(:token_in_session).and_return('token')
-        allow_any_instance_of(described_class)
-        .to receive(:expires_at_in_session).and_return(1.hour.since.to_i.to_s)
+
         allow_any_instance_of(GoogleApi::CloudPlatform::Client)
           .to receive(:projects_zones_clusters_create) do
           OpenStruct.new(
@@ -323,6 +270,12 @@ describe Projects::ClustersController do
           expect(response).to have_gitlab_http_status(:ok)
           expect(response).to render_template(:new)
         end
+
+        it 'sets the redirect url after google authorization' do
+          go
+
+          expect(assigns(:redirect_after_authorize_url)).to eq new_project_cluster_path(project)
+        end
       end
 
       context 'when creates a RBAC-enabled cluster' do