Ability to set attribute_map via config setting (#162)

I added a config hook to set the attribute map dynamically based on the SAML response. By default, it will maintain the existing behavior: look up the `attribute-map.yml` file in the Rails root.

Other changes:

- The tests started failing on Travis despite working locally—I finally determined that bundler was looking at this repository's Gemfile instead of the test app's Gemfile, so I added a temporary working directory to put the test apps in. That seems to have worked.
- Updated the test matrix for new Ruby versions

Fixes #65

Co-authored-by: Kevin Trowbridge <[email protected]>

Author: adamstegman

.gitignore

@@ -13,6 +13,4 @@ lib/bundler/man
 pkg
 rdoc
 spec/reports
-spec/support/idp/
-spec/support/sp/
 tmp

.travis.yml

@@ -4,10 +4,10 @@ rvm:
   - "2.1.10"
   - "2.2.10"
   - "2.3.8"
-  - "2.4.9"
-  - "2.5.7"
-  - "2.6.5"
-  - "2.7.0"
+  - "2.4.10"
+  - "2.5.8"
+  - "2.6.6"
+  - "2.7.1"
 gemfile:
   - Gemfile
   - spec/support/Gemfile.rails5.2
@@ -38,11 +38,11 @@ matrix:
       gemfile: spec/support/Gemfile.rails5.2
     - rvm: "2.3.8"
       gemfile: Gemfile
-    - rvm: "2.4.9"
+    - rvm: "2.4.10"
       gemfile: Gemfile
-    - rvm: "2.6.3"
+    - rvm: "2.6.6"
       gemfile: spec/support/Gemfile.rails4
-    - rvm: "2.7.0"
+    - rvm: "2.7.1"
       gemfile: spec/support/Gemfile.rails4
 
 before_install:

README.md

@@ -130,7 +130,26 @@ In `config/initializers/devise.rb`:
   end
 ```
 
-In the config directory, create a YAML file (`attribute-map.yml`) that maps SAML attributes with your model's fields:
+#### Attributes
+
+There are two ways to map SAML attributes to User attributes:
+
+- [initializer](#attribute-map-initializer)
+- [config file](#attribute-map-config-file)
+
+The attribute mappings are very dependent on the way the IdP encodes the attributes.
+In these examples the attributes are given in URN style.
+Other IdPs might provide them as OID's, or by other means.
+
+You are now ready to test it against an IdP.
+
+When the user visits `/users/saml/sign_in` they will be redirected to the login page of the IdP.
+
+Upon successful login the user is redirected to the Devise `user_root_path`.
+
+##### Attribute map config file
+
+Create a YAML file (`config/attribute-map.yml`) that maps SAML attributes with your model's fields:
 
 ```yaml
   # attribute-map.yml
@@ -141,15 +160,39 @@ In the config directory, create a YAML file (`attribute-map.yml`) that maps SAML
   "urn:mace:dir:attribute-def:givenName": "name"
 ```
 
-The attribute mappings are very dependent on the way the IdP encodes the attributes.
-In this example the attributes are given in URN style.
-Other IdPs might provide them as OID's, or by other means.
+##### Attribute map initializer
 
-You are now ready to test it against an IdP.
+In `config/initializers/devise.rb` (see above), add an attribute map resolver.
+The resolver gets the [SAML response from the IdP](https://github.com/onelogin/ruby-saml/blob/master/lib/onelogin/ruby-saml/response.rb) so it can decide which attribute map to load.
+If you only have one IdP, you can use the config file above, or just return a single hash.
 
-When the user visits `/users/saml/sign_in` they will be redirected to the login page of the IdP.
+```ruby
+  # config/initializers/devise.rb
+  Devise.setup do |config|
+    ...
+    # ==> Configuration for :saml_authenticatable
 
-Upon successful login the user is redirected to the Devise `user_root_path`.
+    config.saml_attribute_map_resolver = MyAttributeMapResolver
+  end
+```
+
+```ruby
+  # app/lib/my_attribute_map_resolver
+  class MyAttributeMapResolver < DeviseSamlAuthenticatable::DefaultAttributeMapResolver
+    def attribute_map
+      issuer = saml_response.issuers.first
+      case issuer
+      when "idp_entity_id"
+        {
+          "urn:mace:dir:attribute-def:uid" => "user_name",
+          "urn:mace:dir:attribute-def:email" => "email",
+          "urn:mace:dir:attribute-def:name" => "last_name",
+          "urn:mace:dir:attribute-def:givenName" => "name",
+        }
+      end
+    end
+  end
+```
 
 ## Supporting Multiple IdPs
 

app/controllers/devise/saml_sessions_controller.rb

@@ -79,6 +79,12 @@ def after_sign_out_path_for(_)
   end
 
   def generate_idp_logout_response(saml_config, logout_request_id)
-    OneLogin::RubySaml::SloLogoutresponse.new.create(saml_config, logout_request_id, nil)
+
+    params = {}
+    if relay_state
+      params[:RelayState] = relay_state
+    end
+
+    OneLogin::RubySaml::SloLogoutresponse.new.create(saml_config, logout_request_id, nil, params)
   end
 end

lib/devise_saml_authenticatable.rb

@@ -5,6 +5,7 @@
 require "devise_saml_authenticatable/logger"
 require "devise_saml_authenticatable/routes"
 require "devise_saml_authenticatable/saml_config"
+require "devise_saml_authenticatable/default_attribute_map_resolver"
 require "devise_saml_authenticatable/default_idp_entity_id_reader"
 
 begin
@@ -66,6 +67,10 @@ module Devise
   mattr_accessor :saml_relay_state
   @@saml_relay_state
 
+  # Instead of storing the attribute_map in attribute-map.yml, store it in the database, or set it programatically
+  mattr_accessor :saml_attribute_map_resolver
+  @@saml_attribute_map_resolver ||= ::DeviseSamlAuthenticatable::DefaultAttributeMapResolver
+
   # Implements a #validate method that takes the retrieved resource and response right after retrieval,
   # and returns true if it's valid.  False will cause authentication to fail.
   # Only one of saml_resource_validator and saml_resource_validator_hook may be used.

lib/devise_saml_authenticatable/default_attribute_map_resolver.rb

@@ -0,0 +1,26 @@
+module DeviseSamlAuthenticatable
+  class DefaultAttributeMapResolver
+    def initialize(saml_response)
+      @saml_response = saml_response
+    end
+
+    def attribute_map
+      return {} unless File.exist?(attribute_map_path)
+
+      attribute_map = YAML.load(File.read(attribute_map_path))
+      if attribute_map.key?(Rails.env)
+        attribute_map[Rails.env]
+      else
+        attribute_map
+      end
+    end
+
+    private
+
+    attr_reader :saml_response
+
+    def attribute_map_path
+      Rails.root.join("config", "attribute-map.yml")
+    end
+  end
+end

lib/devise_saml_authenticatable/model.rb

@@ -33,9 +33,9 @@ def authenticate_with_saml(saml_response, relay_state)
           key = Devise.saml_default_user_key
           decorated_response = ::SamlAuthenticatable::SamlResponse.new(
             saml_response,
-            attribute_map
+            Devise.saml_attribute_map_resolver.new(saml_response).attribute_map,
           )
-          if (Devise.saml_use_subject)
+          if Devise.saml_use_subject
             auth_value = saml_response.name_id
           else
             auth_value = decorated_response.attribute_value_by_resource_key(key)
@@ -80,21 +80,6 @@ def reset_session_key_for(name_id)
         def find_for_shibb_authentication(conditions)
           find_for_authentication(conditions)
         end
-
-        def attribute_map
-          @attribute_map ||= attribute_map_for_environment
-        end
-
-        private
-
-        def attribute_map_for_environment
-          attribute_map = YAML.load(File.read("#{Rails.root}/config/attribute-map.yml"))
-          if attribute_map.has_key?(Rails.env)
-            attribute_map[Rails.env]
-          else
-            attribute_map
-          end
-        end
       end
     end
   end

lib/devise_saml_authenticatable/strategy.rb

@@ -8,7 +8,7 @@ def valid?
         if params[:SAMLResponse]
           OneLogin::RubySaml::Response.new(
             params[:SAMLResponse],
-            settings: Devise.saml_config,
+            settings: saml_config(get_idp_entity_id(params)),
             allowed_clock_drift: Devise.allowed_clock_drift_in_seconds,
           )
         else

spec/controllers/devise/saml_sessions_controller_spec.rb

@@ -333,6 +333,16 @@ def self.entity_id(params)
         end
       end
 
+      context "with a relay_state lambda defined" do
+        let(:relay_state) { ->(request) { "123" } }
+
+        it "includes the RelayState param in the request to the IdP" do
+          expect(Devise).to receive(:saml_relay_state).at_least(:once).and_return(relay_state)
+          do_post
+          expect(saml_response).to have_received(:create).with(Devise.saml_config, saml_request.id, nil, {RelayState: "123"})
+        end
+      end
+
       context 'when saml_session_index_key is not configured' do
         before do
           Devise.saml_session_index_key = nil

spec/devise_saml_authenticatable/default_attribute_map_resolver_spec.rb

@@ -0,0 +1,58 @@
+require "spec_helper"
+require "devise_saml_authenticatable/default_attribute_map_resolver"
+
+describe DeviseSamlAuthenticatable::DefaultAttributeMapResolver do
+  let!(:rails) { class_double("Rails", env: "test", logger: logger, root: rails_root).as_stubbed_const }
+  let(:logger) { instance_double("Logger", info: nil) }
+  let(:rails_root) { Pathname.new("tmp") }
+
+  let(:saml_response) { instance_double("OneLogin::RubySaml::Response") }
+  let(:file_contents) {
+    <<YAML
+---
+firstname: first_name
+lastname: last_name
+YAML
+  }
+  before do
+    allow(File).to receive(:exist?).and_return(true)
+    allow(File).to receive(:read).and_return(file_contents)
+  end
+
+  describe "#attribute_map" do
+    it "reads the attribute map from the config file" do
+      expect(described_class.new(saml_response).attribute_map).to eq(
+        "firstname" => "first_name",
+        "lastname" => "last_name",
+      )
+      expect(File).to have_received(:read).with(Pathname.new("tmp").join("config", "attribute-map.yml"))
+    end
+
+    context "when the attribute map is broken down by environment" do
+      let(:file_contents) {
+        <<YAML
+---
+test:
+  first: first_name
+  last: last_name
+YAML
+      }
+      it "reads the attribute map from the environment key" do
+        expect(described_class.new(saml_response).attribute_map).to eq(
+          "first" => "first_name",
+          "last" => "last_name",
+        )
+      end
+    end
+
+    context "when the config file does not exist" do
+      before do
+        allow(File).to receive(:exist?).and_return(false)
+      end
+
+      it "is an empty hash" do
+        expect(described_class.new(saml_response).attribute_map).to eq({})
+      end
+    end
+  end
+end