From e1c2edde6ab045ef3793c7f0c7b8777295b7dcf1 Mon Sep 17 00:00:00 2001
From: Julia Allen <julia.l.allen@gsa.gov>
Date: Wed, 31 Aug 2022 23:29:13 -0700
Subject: [PATCH] Update SAML SP request flow to POST internally instead of GET

---
 app/controllers/application_controller.rb     | 14 +++++---
 app/controllers/saml_completion_controller.rb | 23 ++++++++++++
 app/controllers/saml_post_controller.rb       |  2 +-
 .../saml_post_form.html.erb}                  |  0
 config/routes.rb                              |  1 +
 .../saml_completion_controller_spec.rb        | 35 +++++++++++++++++++
 6 files changed, 70 insertions(+), 5 deletions(-)
 create mode 100644 app/controllers/saml_completion_controller.rb
 rename app/views/{saml_post/auth.html.erb => shared/saml_post_form.html.erb} (100%)
 create mode 100644 spec/controllers/saml_completion_controller_spec.rb

diff --git a/app/controllers/application_controller.rb b/app/controllers/application_controller.rb
index 915ed4c9fa0..d7c718124a6 100644
--- a/app/controllers/application_controller.rb
+++ b/app/controllers/application_controller.rb
@@ -437,10 +437,16 @@ def sp_session
   end
 
   def sp_session_request_url_with_updated_params
-    # Login.gov redirects to the orginal request_url after a user authenticates
-    # replace prompt=login with prompt=select_account to prevent sign_out
-    # which should only every occur once when the user lands on Login.gov with prompt=login
-    url = sp_session[:request_url]&.gsub('prompt=login', 'prompt=select_account')
+    return unless sp_session[:request_url].present?
+    request_url = URI(sp_session[:request_url])
+    url = if request_url.path.match?('saml')
+      complete_saml_url
+    else
+      # Login.gov redirects to the orginal request_url after a user authenticates
+      # replace prompt=login with prompt=select_account to prevent sign_out
+      # which should only every occur once when the user lands on Login.gov with prompt=login
+      sp_session[:request_url]&.gsub('prompt=login', 'prompt=select_account')
+    end
 
     # If the user has changed the locale, we should preserve that as well
     if url && locale_url_param && UriService.params(url)[:locale] != locale_url_param
diff --git a/app/controllers/saml_completion_controller.rb b/app/controllers/saml_completion_controller.rb
new file mode 100644
index 00000000000..8c52e68d2c7
--- /dev/null
+++ b/app/controllers/saml_completion_controller.rb
@@ -0,0 +1,23 @@
+# Handles the INTERNAL redirect which happens when a service provider authentication request
+# is passed through the IDP's authentication flow (sign-in, MFA, etc.)
+# The original request was saved to the sp_session object, so we retrieve it to pass on the
+# original request url in the form of a POST request to the SamlIdpController#auth method
+
+class SamlCompletionController < ApplicationController
+
+  # Pass the original service provider request to the main SamlIdpController#auth method
+  # via a POST with form parameters replacing the url query parameters
+  def index
+    request_url = URI(sp_session[:request_url])
+    path_year = request_url.path[-4..-1]
+    path_method = "api_saml_authpost#{path_year}_url"
+    action_url = Rails.application.routes.url_helpers.send(path_method)
+
+    # Takes the query params which were set internally in the sp_session (so they should always be valid).
+    # A bad request that originated outside of the IDP would have already responded with a 400 status before
+    # reaching this point.
+    form_params = UriService.params(request_url)
+
+    render 'shared/saml_post_form', locals: { action_url: action_url, form_params: form_params }, layout: false
+  end
+end
diff --git a/app/controllers/saml_post_controller.rb b/app/controllers/saml_post_controller.rb
index a9c2d389e9e..1154e570f95 100644
--- a/app/controllers/saml_post_controller.rb
+++ b/app/controllers/saml_post_controller.rb
@@ -9,6 +9,6 @@ def auth
 
     form_params = params.permit(:SAMLRequest, :RelayState, :SigAlg, :Signature)
 
-    render locals: { action_url: action_url, form_params: form_params }, layout: false
+    render 'shared/saml_post_form', locals: { action_url: action_url, form_params: form_params }, layout: false
   end
 end
diff --git a/app/views/saml_post/auth.html.erb b/app/views/shared/saml_post_form.html.erb
similarity index 100%
rename from app/views/saml_post/auth.html.erb
rename to app/views/shared/saml_post_form.html.erb
diff --git a/config/routes.rb b/config/routes.rb
index e86e1aa5325..ba0e880ff46 100644
--- a/config/routes.rb
+++ b/config/routes.rb
@@ -24,6 +24,7 @@
     post "/api/saml/authpost#{suffix}" => 'saml_idp#auth'
     get "/api/saml/auth#{suffix}" => 'saml_idp#auth'
   end
+  get "/api/saml/complete" => 'saml_completion#index', as: :complete_saml
 
   post '/api/service_provider' => 'service_provider#update'
   post '/api/verify/images' => 'idv/image_uploads#create'
diff --git a/spec/controllers/saml_completion_controller_spec.rb b/spec/controllers/saml_completion_controller_spec.rb
new file mode 100644
index 00000000000..debb4c086d1
--- /dev/null
+++ b/spec/controllers/saml_completion_controller_spec.rb
@@ -0,0 +1,35 @@
+require 'rails_helper'
+
+describe SamlCompletionController do
+  describe 'GET #index' do
+    render_views
+    include ActionView::Helpers::FormTagHelper
+
+    let(:form_action_regex) { /<form.+action=".+\/api\/saml\/authpost\d{4}.+"/ }
+    let(:saml_request) { 'abc123' }
+    let(:relay_state) { 'def456' }
+    let(:sig_alg) { 'aes256' }
+    let(:signature) { 'xyz789' }
+
+    context 'with SAML protocol params passed in appropriately via an internal redirect' do
+      let(:get_params) { "?SAMLRequest=#{saml_request}&RelayState=#{relay_state}&SigAlg=#{sig_alg}&Signature=#{signature}" }
+      let(:sp_session_request_url) { 'http://example.gov/api/saml/auth2022' }
+
+      before do
+        expect(controller).to receive(:sp_session).at_least(:once).and_return(
+          request_url: sp_session_request_url + get_params
+        )
+      end
+
+      it 'renders the appropriate form' do
+        get :index
+
+        expect(response.body).to match(form_action_regex)
+        expect(response.body).to match(hidden_field_tag('SAMLRequest', saml_request))
+        expect(response.body).to match(hidden_field_tag('RelayState', relay_state))
+        expect(response.body).to match(hidden_field_tag('SigAlg', sig_alg))
+        expect(response.body).to match(hidden_field_tag('Signature', signature))
+      end
+    end
+  end
+end