From b363e14a5970cc4d669ec38a1397a33aeacd7af4 Mon Sep 17 00:00:00 2001
From: Aubrey Holland <aubreyholland@gmail.com>
Date: Sat, 5 Nov 2016 20:06:59 -0400
Subject: [PATCH] stop using a singleton for SAML::SettingsService (#447)

* stop using a singleton for SAML::SettingsService

* do the whole SSL thing

* fix spec
---
 app/controllers/application_controller.rb     |  2 +-
 lib/saml/settings_service.rb                  | 20 +++++++++++++------
 .../v0/sessions_controller_spec.rb            |  3 ++-
 spec/lib/saml/saml_settings_service_spec.rb   | 14 +++++--------
 4 files changed, 22 insertions(+), 17 deletions(-)

diff --git a/app/controllers/application_controller.rb b/app/controllers/application_controller.rb
index 7ed073b9036..4ae05bf8f43 100644
--- a/app/controllers/application_controller.rb
+++ b/app/controllers/application_controller.rb
@@ -86,7 +86,7 @@ def render_unauthorized
   end
 
   def saml_settings
-    settings = SAML::SettingsService.instance.saml_settings
+    settings = SAML::SettingsService.new.saml_settings
     # TODO: 'level' should be its own class with proper validation
     level = LOA::MAPPING.invert[params[:level]&.to_i]
     settings.authn_context = level || LOA::MAPPING.invert[1]
diff --git a/lib/saml/settings_service.rb b/lib/saml/settings_service.rb
index fc5d080e235..af7445848ee 100644
--- a/lib/saml/settings_service.rb
+++ b/lib/saml/settings_service.rb
@@ -3,14 +3,22 @@ module SAML
   # This class is responsible for putting together a complete ruby-saml
   # SETTINGS object, meaning, our static SP settings + the IDP settings
   # which must be fetched once and only once via IDP metadata.
-  require 'singleton'
   class SettingsService
-    include Singleton
-
     attr_reader :saml_settings
 
+    METADATA_URI = URI(SAML_CONFIG['metadata_url'])
+
     def initialize
-      @saml_settings ||= fetch_idp_metadata
+      @saml_settings = create_settings_from_metadata
+    end
+
+    def self.metadata
+      return @metadata if defined?(@metadata)
+      http = Net::HTTP.new(METADATA_URI.host, METADATA_URI.port)
+      http.use_ssl = true
+      http.verify_mode = OpenSSL::SSL::VERIFY_PEER
+      get = Net::HTTP::Get.new(METADATA_URI.request_uri)
+      @metadata = http.request(get).body
     end
 
     private
@@ -31,9 +39,9 @@ def settings
     end
 
     ## Makes an external web call to get IDP metadata and populates SETTINGS
-    def fetch_idp_metadata
+    def create_settings_from_metadata
       parser = OneLogin::RubySaml::IdpMetadataParser.new
-      parser.parse_remote(SAML_CONFIG['metadata_url'], true, settings: settings)
+      parser.parse(self.class.metadata, settings: settings)
     end
   end
 end
diff --git a/spec/controllers/v0/sessions_controller_spec.rb b/spec/controllers/v0/sessions_controller_spec.rb
index 89d5893cdfc..3f840778d6a 100644
--- a/spec/controllers/v0/sessions_controller_spec.rb
+++ b/spec/controllers/v0/sessions_controller_spec.rb
@@ -24,8 +24,9 @@
   let(:query_token) { Rack::Utils.parse_query(URI.parse(response.location).query)['token'] }
 
   before(:each) do
+    stub_request(:get, SAML_CONFIG['metadata_url']).to_return(body: 'abc')
     allow_any_instance_of(Decorators::MviUserDecorator).to receive(:create).and_return(mvi_user)
-    allow(settings_service).to receive_message_chain(:instance, :saml_settings).and_return(settings_no_context)
+    allow_any_instance_of(SAML::SettingsService).to receive(:saml_settings).and_return(settings_no_context)
   end
 
   context 'when not logged in' do
diff --git a/spec/lib/saml/saml_settings_service_spec.rb b/spec/lib/saml/saml_settings_service_spec.rb
index 585140fc7d9..671baad355c 100644
--- a/spec/lib/saml/saml_settings_service_spec.rb
+++ b/spec/lib/saml/saml_settings_service_spec.rb
@@ -3,15 +3,11 @@
 require 'saml/settings_service'
 
 RSpec.describe SAML::SettingsService do
-  # testing singletons is tricky since it may be initialized in a previous spec example,
-  # use this this instance variable for testing instead
-  before(:each) { @service_instance = SAML::SettingsService.clone }
-
   it 'should only ever make 1 external web call' do
-    stub_request(:get, SAML_CONFIG['metadata_url'])
-    @service_instance.instance.saml_settings
-    @service_instance.instance.saml_settings
-    @service_instance.instance.saml_settings
-    expect(a_request(:get, SAML_CONFIG['metadata_url'])).to have_been_made.times(1)
+    stub_request(:get, SAML_CONFIG['metadata_url']).to_return(body: 'abc')
+    SAML::SettingsService.new.saml_settings
+    SAML::SettingsService.new.saml_settings
+    SAML::SettingsService.new.saml_settings
+    expect(a_request(:get, SAML_CONFIG['metadata_url'])).to have_been_made.at_most_once
   end
 end