Improve redirect uri state and fix all remaining tests

app/controllers/google_api/authorizations_controller.rb

@@ -9,16 +9,21 @@ module GoogleApi
       session[GoogleApi::CloudPlatform::Client.session_key_for_expires_at] =
         expires_at.to_s
 
-      key, _ = GoogleApi::CloudPlatform::Client
-        .session_key_for_second_redirect_uri(secure: params[:state])
+      state_redirect_uri = redirect_uri_from_session_key(params[:state])
 
-      second_redirect_uri = session[key]
-
-      if second_redirect_uri.present?
-        redirect_to second_redirect_uri
+      if state_redirect_uri
+        redirect_to state_redirect_uri
       else
         redirect_to root_path
       end
     end
+
+    private
+
+    def redirect_uri_from_session_key(state)
+      key = GoogleApi::CloudPlatform::Client
+        .session_key_for_redirect_uri(params[:state])
+      session[key] if key
+    end
   end
 end

app/controllers/projects/clusters_controller.rb

@@ -16,13 +16,11 @@ class Projects::ClustersController < Projects::ApplicationController
 
   def login
     begin
-      GoogleApi::CloudPlatform::Client.session_key_for_second_redirect_uri.tap do |key, secure|
-        session[key] = namespace_project_clusters_url.to_s
+      state = generate_session_key_redirect(namespace_project_clusters_url.to_s)
 
-        @authorize_url = GoogleApi::CloudPlatform::Client.new(
-          nil, callback_google_api_auth_url,
-          state: secure).authorize_url
-      end
+      @authorize_url = GoogleApi::CloudPlatform::Client.new(
+        nil, callback_google_api_auth_url,
+        state: state).authorize_url
     rescue GoogleApi::Auth::ConfigMissingError
       # no-op
     end
@@ -122,6 +120,12 @@ class Projects::ClustersController < Projects::ApplicationController
       session[GoogleApi::CloudPlatform::Client.session_key_for_expires_at]
   end
 
+  def generate_session_key_redirect(uri)
+    GoogleApi::CloudPlatform::Client.new_session_key_for_redirect_uri do |key|
+      session[key] = uri
+    end
+  end
+
   def authorize_update_cluster!
     access_denied! unless can?(current_user, :update_cluster, cluster)
   end

lib/google_api/cloud_platform/client.rb

@@ -16,9 +16,14 @@ module GoogleApi
           :cloud_platform_expires_at
         end
 
-        def session_key_for_second_redirect_uri(secure: nil)
-          secure = SecureRandom.hex unless secure
-          return "cloud_platform_second_redirect_uri_#{secure}", secure
+        def new_session_key_for_redirect_uri
+          SecureRandom.hex.tap do |state|
+            yield session_key_for_redirect_uri(state)
+          end
+        end
+
+        def session_key_for_redirect_uri(state)
+          "cloud_platform_second_redirect_uri_#{state}"
         end
       end
 

spec/controllers/google_api/authorizations_controller_spec.rb

@@ -24,20 +24,19 @@ describe GoogleApi::AuthorizationsController do
         .to eq(expires_at)
     end
 
-    context 'when second redirection url key is stored in state' do
+    context 'when redirect uri key is stored in state' do
       set(:project) { create(:project) }
-      let(:second_redirect_uri) { project_clusters_url(project).to_s }
+      let(:redirect_uri) { project_clusters_url(project).to_s }
 
       before do
-        GoogleApi::CloudPlatform::Client
-          .session_key_for_second_redirect_uri.tap do |key, secure|
-          @state = secure
-          session[key] = second_redirect_uri
+        @state = GoogleApi::CloudPlatform::Client
+          .new_session_key_for_redirect_uri do |key|
+          session[key] = redirect_uri
         end
       end
 
       it 'redirects to the URL stored in state param' do
-        expect(subject).to redirect_to(second_redirect_uri)
+        expect(subject).to redirect_to(redirect_uri)
       end
     end
 

spec/controllers/projects/clusters_controller_spec.rb

@@ -190,7 +190,7 @@ describe Projects::ClustersController do
         subject
 
         expect(response).to have_http_status(:ok)
-        expect(response.body).to include("Save changes")
+        expect(response.body).to include("Save")
       end
 
       it "allows remove integration" do

spec/features/projects/clusters_spec.rb

@@ -69,14 +69,14 @@ feature 'Clusters', :js do
       end
 
       it 'user sees an cluster details page' do
-        expect(page).to have_button('Save changes')
+        expect(page).to have_button('Save')
         expect(page.find(:css, '.cluster-name').value).to eq(cluster.gcp_cluster_name)
       end
 
       context 'when user disables the cluster' do
         before do
           page.find(:css, '.js-toggle-cluster').click
-          click_button 'Save changes'
+          click_button 'Save'
         end
 
         it 'user sees the succeccful message' do

spec/lib/google_api/cloud_platform/client_spec.rb

@@ -4,26 +4,20 @@ describe GoogleApi::CloudPlatform::Client do
   let(:token) { 'token' }
   let(:client) { described_class.new(token, nil) }
 
-  describe '.session_key_for_second_redirect_uri' do
-    subject { described_class.session_key_for_second_redirect_uri(secure: secure) }
+  describe '.session_key_for_redirect_uri' do
+    let(:state) { 'random_string' }
 
-    context 'when pass a postfix' do
-      let(:secure) { SecureRandom.hex }
+    subject { described_class.session_key_for_redirect_uri(state) }
 
-      it 'creates a required session key' do
-        key, _ = described_class.session_key_for_second_redirect_uri(secure: secure)
-        expect(key).to eq("cloud_platform_second_redirect_uri_#{secure}")
-      end
+    it 'creates a new session key' do
+      is_expected.to eq('cloud_platform_second_redirect_uri_random_string')
     end
+  end
 
-    context 'when pass a postfix' do
-      let(:secure) { nil }
-
-      it 'creates a new session key' do
-        key, secure = described_class.session_key_for_second_redirect_uri
-        expect(key).to include('cloud_platform_second_redirect_uri_')
-        expect(secure).not_to be_nil
-      end
+  describe '.new_session_key_for_redirect_uri' do
+    it 'generates a new session key' do
+      expect { |b| described_class.new_session_key_for_redirect_uri(&b) }
+        .to yield_with_args(String)
     end
   end