make saml entity use the account's domain

AccountAuthorizationConfigs now require the entity_id to
be set for SAML configs. It will be set to the domain of
the account. For existing SAML configs, the entity_id will
be set to what is in the SAML config file if there is one.

This commit also allows SAML meta data to show up for all
domains even if they're not configured for SAML. This helps
admins set up initial SAML configurations.

Test Plan:
 * Create a saml configuration for an account
 * Look at the metadata, the entity_id should be set to the domain/saml2 and not what is in the saml config file
 * Try loading the meta data for an account with no SAML config. It should load and have the proper entity_id

closes #6713

Change-Id: Ia98543c996285d9b1febd788c3f3ec072b672b12
Reviewed-on: https://gerrit.instructure.com/7967
Reviewed-by: Cody Cutrer <cody@instructure.com>
Tested-by: Hudson <hudson@instructure.com>

Author: bracken

app/controllers/accounts_controller.rb

@@ -387,12 +387,8 @@ def saml_meta_data
     # This needs to be publicly available since external SAML
     # servers need to be able to access it without being authenticated.
     # It is used to disclose our SAML configuration settings.
-    if @domain_root_account.account_authorization_config and @domain_root_account.account_authorization_config.auth_type == 'saml'
-      settings = @domain_root_account.account_authorization_config.saml_settings(request.env['canvas.account_domain'])
-      render :xml => Onelogin::Saml::MetaData.create(settings)
-    else
-      render :xml => ""
-    end
+    settings = AccountAuthorizationConfig.saml_settings_for_account(@domain_root_account, request.env['canvas.account_domain'])
+    render :xml => Onelogin::Saml::MetaData.create(settings)
   end
 
   # TODO Refactor add_account_user and remove_account_user actions into

app/models/account_authorization_config.rb

@@ -27,7 +27,9 @@ class AccountAuthorizationConfig < ActiveRecord::Base
                   :certificate_fingerprint, :entity_id, :change_password_url,
                   :login_handle_name, :ldap_filter, :auth_filter
 
+  before_validation :set_saml_entity_id, :if => Proc.new { |aac| aac.saml_authentication? }
   validates_presence_of :account_id
+  validates_presence_of :entity_id, :if => Proc.new{|aac| aac.saml_authentication?}
   after_save :disable_open_registration_if_delegated
 
   def ldap_connection
@@ -41,6 +43,10 @@ def ldap_connection
     ldap
   end
 
+  def set_saml_entity_id
+    self.entity_id ||= saml_default_entity_id
+  end
+  
   def sanitized_ldap_login(login)
     [ [ '\\', '\5c' ], [ '*', '\2a' ], [ '(', '\28' ], [ ')', '\29' ], [ "\00", '\00' ] ].each do |re|
       login.gsub!(re[0], re[1])
@@ -79,56 +85,75 @@ def auth_decrypted_password
     return nil unless self.auth_password_salt && self.auth_crypted_password
     Canvas::Security.decrypt_password(self.auth_crypted_password, self.auth_password_salt, 'instructure_auth')
   end
+  
+  def self.saml_default_entity_id_for_account(account)
+    "http://#{HostUrl.context_host(account)}/saml2"
+  end
+  
+  def saml_default_entity_id
+    AccountAuthorizationConfig.saml_default_entity_id_for_account(self.account)
+  end
 
   def saml_settings(preferred_account_domain=nil)
     return nil unless self.auth_type == 'saml'
-    app_config = Setting.from_config('saml')
-    raise "This Canvas instance isn't configured for SAML" unless app_config
 
     unless @saml_settings
-      domain = HostUrl.context_host(self.account, preferred_account_domain)
-      @saml_settings = Onelogin::Saml::Settings.new
+      @saml_settings = AccountAuthorizationConfig.saml_settings_for_account(self.account, preferred_account_domain)
 
-      @saml_settings.issuer = self.entity_id || app_config[:entity_id]
       @saml_settings.idp_sso_target_url = self.log_in_url
       @saml_settings.idp_slo_target_url = self.log_out_url
       @saml_settings.idp_cert_fingerprint = self.certificate_fingerprint
       @saml_settings.name_identifier_format = self.identifier_format
-      if ENV['RAILS_ENV'] == 'development'
-        # if you set the domain to go to your local box in /etc/hosts you can test saml
-        @saml_settings.assertion_consumer_service_url = "http://#{domain}/saml_consume"
-        @saml_settings.sp_slo_url = "http://#{domain}/saml_logout"
-      else
-        @saml_settings.assertion_consumer_service_url = "https://#{domain}/saml_consume"
-        @saml_settings.sp_slo_url = "https://#{domain}/saml_logout"
-      end
-      @saml_settings.tech_contact_name = app_config[:tech_contact_name] || 'Webmaster'
-      @saml_settings.tech_contact_email = app_config[:tech_contact_email]
-
-      encryption = app_config[:encryption]
-      if encryption.is_a?(Hash) && File.exists?(encryption[:xmlsec_binary])
-        resolve_path = lambda {|path|
-          if path.nil?
-            nil
-          elsif path[0,1] == '/'
-            path
-          else
-            File.join(Rails.root, 'config', path)
-          end
-        }
-
-        private_key_path = resolve_path.call(encryption[:private_key])
-        certificate_path = resolve_path.call(encryption[:certificate])
-
-        if File.exists?(private_key_path) && File.exists?(certificate_path)
-          @saml_settings.xmlsec1_path = encryption[:xmlsec_binary]
-          @saml_settings.xmlsec_certificate = certificate_path
-          @saml_settings.xmlsec_privatekey = private_key_path
+    end
+    
+    @saml_settings
+  end
+  
+  def self.saml_settings_for_account(account, preferred_account_domain=nil)
+    app_config = Setting.from_config('saml') || {}
+    domain = HostUrl.context_host(account, preferred_account_domain)
+    
+    settings = Onelogin::Saml::Settings.new
+    if ENV['RAILS_ENV'] == 'development'
+      # if you set the domain to go to your local box in /etc/hosts you can test saml
+      settings.assertion_consumer_service_url = "http://#{domain}/saml_consume"
+      settings.sp_slo_url = "http://#{domain}/saml_logout"
+    else
+      settings.assertion_consumer_service_url = "https://#{domain}/saml_consume"
+      settings.sp_slo_url = "https://#{domain}/saml_logout"
+    end
+    settings.tech_contact_name = app_config[:tech_contact_name] || 'Webmaster'
+    settings.tech_contact_email = app_config[:tech_contact_email] || ''
+    
+    if account.saml_authentication?
+      settings.issuer = account.account_authorization_config.entity_id 
+    else
+      settings.issuer = saml_default_entity_id_for_account(account)
+    end
+    
+    encryption = app_config[:encryption]
+    if encryption.is_a?(Hash) && File.exists?(encryption[:xmlsec_binary])
+      resolve_path = lambda { |path|
+        if path.nil?
+          nil
+        elsif path[0, 1] == '/'
+          path
+        else
+          File.join(Rails.root, 'config', path)
         end
+      }
+
+      private_key_path = resolve_path.call(encryption[:private_key])
+      certificate_path = resolve_path.call(encryption[:certificate])
+
+      if File.exists?(private_key_path) && File.exists?(certificate_path)
+        settings.xmlsec1_path = encryption[:xmlsec_binary]
+        settings.xmlsec_certificate = certificate_path
+        settings.xmlsec_privatekey = private_key_path
       end
     end
 
-    @saml_settings
+    settings
   end
 
   def email_identifier?

db/migrate/20120106220543_set_saml_entity_id.rb

@@ -0,0 +1,23 @@
+class SetSamlEntityId < ActiveRecord::Migration
+
+  # Sets all existing SAML configs to be what is currently in the config so they won't break
+  # If there is no config use the host of the account
+  # All future new SAML configs will use the host of the account
+  def self.up
+    old_default_domain = nil
+    if app_config = Setting.from_config('saml')
+      old_default_domain = app_config[:entity_id]
+    end
+    
+    AccountAuthorizationConfig.find_all_by_auth_type("saml").each do |aac|
+      if aac.entity_id.blank?
+        aac.entity_id = old_default_domain || aac.saml_default_entity_id
+        aac.save!
+      end
+    end
+  end
+
+  def self.down
+    #raise ActiveRecord::IrreversibleMigration
+  end
+end

spec/integration/account_spec.rb

@@ -0,0 +1,50 @@
+#
+# Copyright (C) 2012 Instructure, Inc.
+#
+# This file is part of Canvas.
+#
+# Canvas is free software: you can redistribute it and/or modify it under
+# the terms of the GNU Affero General Public License as published by the Free
+# Software Foundation, version 3 of the License.
+#
+# Canvas is distributed in the hope that it will be useful, but WITHOUT ANY
+# WARRANTY; without even the implied warranty of MERCHANTABILITY or FITNESS FOR
+# A PARTICULAR PURPOSE. See the GNU Affero General Public License for more
+# details.
+#
+# You should have received a copy of the GNU Affero General Public License along
+# with this program. If not, see <http://www.gnu.org/licenses/>.
+#
+
+require File.expand_path(File.dirname(__FILE__) + '/../spec_helper')
+
+describe AccountsController do
+
+  context "SAML meta data" do
+    before(:each) do
+      Setting.set_config('saml', {
+              :tech_contact_name => nil,
+              :tech_contact_email => nil
+      })
+      @account = Account.create!(:name => "test")
+    end
+
+    it 'should render for non SAML configured accounts' do
+      get "/saml_meta_data"
+      response.should be_success
+      response.body.should_not == ""
+    end
+    
+    it "should use the correct entity_id" do
+      HostUrl.stubs(:default_host).returns('bob.cody.instructure.com')
+      @aac = @account.account_authorization_configs.create!(:auth_type => "saml")
+      
+      get "/saml_meta_data"
+      response.should be_success
+      doc = Nokogiri::XML(response.body)
+      doc.at_css("EntityDescriptor")['entityID'].should == "http://bob.cody.instructure.com/saml2"
+    end
+
+  end
+
+end

spec/migrations/set_saml_entity_id_spec.rb

@@ -0,0 +1,62 @@
+#
+# Copyright (C) 2012 Instructure, Inc.
+#
+# This file is part of Canvas.
+#
+# Canvas is free software: you can redistribute it and/or modify it under
+# the terms of the GNU Affero General Public License as published by the Free
+# Software Foundation, version 3 of the License.
+#
+# Canvas is distributed in the hope that it will be useful, but WITHOUT ANY
+# WARRANTY; without even the implied warranty of MERCHANTABILITY or FITNESS FOR
+# A PARTICULAR PURPOSE. See the GNU Affero General Public License for more
+# details.
+#
+# You should have received a copy of the GNU Affero General Public License along
+# with this program. If not, see <http://www.gnu.org/licenses/>.
+#
+
+require File.expand_path(File.dirname(__FILE__) + '/../spec_helper.rb')
+require 'db/migrate/20120106220543_set_saml_entity_id'
+
+describe SetSamlEntityId do
+  before(:each) do
+    Setting.set_config('saml', {
+            :entity_id => "http://watup_fool.com/saml2"
+    })
+    HostUrl.stubs(:default_host).returns('bob.cody.instructure.com')
+    @account = Account.new
+    @account.save
+    @aac = @account.account_authorization_configs.create!(:auth_type => "saml")
+    AccountAuthorizationConfig.update_all(:entity_id => nil, :id => [@aac.id])
+  end
+  
+  it "should set the entity_id to the current setting if none is set" do
+    SetSamlEntityId.up
+    @aac.reload
+    @aac.entity_id.should == "http://watup_fool.com/saml2"
+  end
+  
+  it "should leave the entity_id the same if already set" do
+    @aac.entity_id = "haha"
+    @aac.save
+
+    SetSamlEntityId.up
+    
+    @aac.reload
+    @aac.entity_id.should == "haha"
+  end
+  
+  it "should use the account's domain if no config is set" do
+    Setting.set_config('saml', {
+            :entity_id => nil
+    })
+
+    SetSamlEntityId.up
+
+    @aac.reload
+    @aac.entity_id.should == "http://bob.cody.instructure.com/saml2"
+  end
+  
+  
+end

spec/models/account_authorization_config_spec.rb

@@ -35,6 +35,10 @@
   end
 
   context "SAML settings" do
+    before(:each) do
+      @account = Account.create!(:name => "account")
+    end
+    
     it "should load encryption settings" do
       file_that_exists = File.expand_path(__FILE__)
       Setting.set_config('saml', {
@@ -48,12 +52,22 @@
         }
       })
 
-      @account = Account.new
       config = @account.account_authorization_configs.build(:auth_type => 'saml')
 
       s = config.saml_settings
       s.encryption_configured?.should be_true
     end
+    
+    it "should set the entity_id with the current domain" do
+      HostUrl.stubs(:default_host).returns('bob.cody.instructure.com')
+      @aac = @account.account_authorization_configs.create!(:auth_type => "saml")
+      @aac.entity_id.should == "http://bob.cody.instructure.com/saml2"
+    end
+    
+    it "should not overwrite a specific entity_id" do
+      @aac = @account.account_authorization_configs.create!(:auth_type => "saml", :entity_id => "http://wtb.instructure.com/saml2")
+      @aac.entity_id.should == "http://wtb.instructure.com/saml2"
+    end
   end
 
   context "password" do