From b592a3472e793342a9b9e614582856805a40c261 Mon Sep 17 00:00:00 2001
From: Zach Margolis <zachary.margolis@gsa.gov>
Date: Mon, 10 Apr 2023 12:56:35 -0700
Subject: [PATCH 1/3] Add safer parsing of dynamic SAML urls (LG-8837)

- Pulls ideas from #8105

changelog: Internal, Source code, Add stricter checking of URLs
---
 app/controllers/saml_post_controller.rb | 16 +++++++++++++---
 1 file changed, 13 insertions(+), 3 deletions(-)

diff --git a/app/controllers/saml_post_controller.rb b/app/controllers/saml_post_controller.rb
index 386dcfe0afd..ba2f25d2544 100644
--- a/app/controllers/saml_post_controller.rb
+++ b/app/controllers/saml_post_controller.rb
@@ -3,13 +3,23 @@ class SamlPostController < ApplicationController
   skip_before_action :verify_authenticity_token
 
   def auth
-    path_year = request.path[-4..-1]
-    path_method = "api_saml_authpost#{path_year}_url"
-    action_url = Rails.application.routes.url_helpers.send(path_method)
+    action_url = build_action_url(request.path)
 
     form_params = params.permit(:SAMLRequest, :RelayState, :SigAlg, :Signature)
 
     render 'shared/saml_post_form', locals: { action_url: action_url, form_params: form_params },
                                     layout: false
   end
+
+  private
+
+  def build_action_url(path)
+    path_year = path[-4..-1]
+    path = "/api/saml/authpost#{path_year}"
+    recognized_path = Rails.application.routes.recognize_path(path, method: :post)
+
+    if recognized_path[:controller] == 'saml_idp' && recognized_path[:action] == 'auth'
+      Rails.application.routes.url_for(**recognized_path)
+    end
+  end
 end

From a0f7d9e066cfdb4f22f16a5597dd310e79cedadf Mon Sep 17 00:00:00 2001
From: Zach Margolis <zachary.margolis@gsa.gov>
Date: Mon, 10 Apr 2023 15:01:44 -0700
Subject: [PATCH 2/3] Add additional spec coverage

---
 app/controllers/saml_post_controller.rb       |  1 +
 .../saml_completion_controller_spec.rb        | 17 ++++++++++++++
 spec/controllers/saml_post_controller_spec.rb | 23 +++++++++++++++++++
 3 files changed, 41 insertions(+)

diff --git a/app/controllers/saml_post_controller.rb b/app/controllers/saml_post_controller.rb
index ba2f25d2544..786a793907c 100644
--- a/app/controllers/saml_post_controller.rb
+++ b/app/controllers/saml_post_controller.rb
@@ -4,6 +4,7 @@ class SamlPostController < ApplicationController
 
   def auth
     action_url = build_action_url(request.path)
+    render_not_found and return if !action_url
 
     form_params = params.permit(:SAMLRequest, :RelayState, :SigAlg, :Signature)
 
diff --git a/spec/controllers/saml_completion_controller_spec.rb b/spec/controllers/saml_completion_controller_spec.rb
index fd6cbb8105c..06d5777ae7e 100644
--- a/spec/controllers/saml_completion_controller_spec.rb
+++ b/spec/controllers/saml_completion_controller_spec.rb
@@ -63,6 +63,23 @@
       end
     end
 
+    context 'with an invalid year in the path' do
+      let(:path_year) { SamlEndpoint.suffixes.last.to_i + 1 }
+
+      before do
+        expect(controller).to receive(:sp_session).at_least(:once).and_return(
+          {
+            request_url: "https://example.gov/api/saml/finalauthpost#{path_year}",
+          },
+        )
+      end
+
+      it 'renders 404 not found' do
+        get :index
+        expect(response).to be_not_found
+      end
+    end
+
     context 'with a nil service provider request session' do
       before { expect(controller).to receive(:sp_from_sp_session).and_return nil }
 
diff --git a/spec/controllers/saml_post_controller_spec.rb b/spec/controllers/saml_post_controller_spec.rb
index 8ae21b62562..6a38bd4daa8 100644
--- a/spec/controllers/saml_post_controller_spec.rb
+++ b/spec/controllers/saml_post_controller_spec.rb
@@ -31,5 +31,28 @@
 
       expect(response.body).not_to match(hidden_field_tag('Foo', 'bar'))
     end
+
+    context 'with an invalid year in the path' do
+      let(:path_year) { SamlEndpoint.suffixes.last.to_i + 2 }
+
+      before do
+        allow(controller).to receive(:request).and_wrap_original do |impl|
+          req = impl.call
+          req.path = "https://example.gov/api/saml/auth#{path_year}"
+          req
+        end
+      end
+
+      it 'renders 404 not found' do
+        post :auth, params: {
+          'SAMLRequest' => saml_request,
+          'RelayState' => relay_state,
+          'SigAlg' => sig_alg,
+          'Signature' => signature,
+        }
+
+        expect(response).to be_not_found
+      end
+    end
   end
 end

From b0e632f4f0f8cd740f1f7c04d0b231e960925240 Mon Sep 17 00:00:00 2001
From: Zach Margolis <zachary.margolis@gsa.gov>
Date: Mon, 10 Apr 2023 15:16:47 -0700
Subject: [PATCH 3/3] remove sneaky "and" keyword

---
 app/controllers/saml_post_controller.rb | 5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/app/controllers/saml_post_controller.rb b/app/controllers/saml_post_controller.rb
index 786a793907c..b25b2f65203 100644
--- a/app/controllers/saml_post_controller.rb
+++ b/app/controllers/saml_post_controller.rb
@@ -4,7 +4,10 @@ class SamlPostController < ApplicationController
 
   def auth
     action_url = build_action_url(request.path)
-    render_not_found and return if !action_url
+    if !action_url
+      render_not_found
+      return
+    end
 
     form_params = params.permit(:SAMLRequest, :RelayState, :SigAlg, :Signature)