From eacc53659d60e34f1e9090d7744dd4c469ccb19d Mon Sep 17 00:00:00 2001 From: Olav Morken Date: Thu, 15 Dec 2016 15:33:44 +0100 Subject: [PATCH] feat: Add `uid_attribute` option to control the attribute used for the user id. Some SAML 2.0 IdPs use the transient name identifier format. In that case the name identifier changes for each login, which makes the name identifier unsuitable as a user identifier. This patch introduces the `uid_attribute` option, that allows us to use a SAML 2.0 attribute as the user identifier instead of the name identifier. Fixes issue #120. --- README.md | 2 ++ lib/omniauth/strategies/saml.rb | 13 ++++++++++++- spec/omniauth/strategies/saml_spec.rb | 25 +++++++++++++++++++++++++ 3 files changed, 39 insertions(+), 1 deletion(-) diff --git a/README.md b/README.md index 9f27f7e..fad3f88 100644 --- a/README.md +++ b/README.md @@ -135,6 +135,8 @@ The service provider metadata used to ease configuration of the SAML SP in the I *Note*: All attributes can also be found in an array under `auth_hash[:extra][:raw_info]`, so this setting should only be used to map attributes that are part of the OmniAuth info hash schema. +* `:uid_attribute` - Attribute that uniquely identifies the user. If unset, the name identifier returned by the IdP is used. + * See the `OneLogin::RubySaml::Settings` class in the [Ruby SAML gem](https://github.com/onelogin/ruby-saml) for additional supported options. ## Devise Integration diff --git a/lib/omniauth/strategies/saml.rb b/lib/omniauth/strategies/saml.rb index d2e5ac7..0913ccb 100644 --- a/lib/omniauth/strategies/saml.rb +++ b/lib/omniauth/strategies/saml.rb @@ -28,6 +28,7 @@ def self.inherited(subclass) last_name: ["last_name", "lastname", "lastName"] } option :slo_default_relay_state + option :uid_attribute def request_phase options[:assertion_consumer_service_url] ||= callback_url @@ -136,7 +137,17 @@ def other_phase end end - uid { @name_id } + uid do + if options.uid_attribute + ret = find_attribute_by([options.uid_attribute]) + if ret.nil? + raise OmniAuth::Strategies::SAML::ValidationError.new("SAML response missing '#{options.uid_attribute}' attribute") + end + ret + else + @name_id + end + end info do found_attributes = options.attribute_statements.map do |key, values| diff --git a/spec/omniauth/strategies/saml_spec.rb b/spec/omniauth/strategies/saml_spec.rb index 85eeffb..d731b3b 100644 --- a/spec/omniauth/strategies/saml_spec.rb +++ b/spec/omniauth/strategies/saml_spec.rb @@ -207,6 +207,31 @@ def post_xml(xml=:example_response, opts = {}) end end + context "when using custom user id attribute" do + before :each do + saml_options[:idp_cert_fingerprint] = "3B:82:F1:F5:54:FC:A8:FF:12:B8:4B:B8:16:61:1D:E4:8E:9B:E2:3C" + saml_options[:uid_attribute] = "http://schemas.xmlsoap.org/ws/2005/05/identity/claims/emailaddress" + post_xml :custom_attributes + end + + it "should return user id attribute" do + expect(auth_hash[:uid]).to eq("user@example.com") + end + end + + context "when using custom user id attribute, but it is missing" do + before :each do + saml_options[:uid_attribute] = "missing_attribute" + post_xml + end + + it "should fail to authenticate" do + should fail_with(:invalid_ticket) + expect(last_request.env['omniauth.error']).to be_instance_of(OmniAuth::Strategies::SAML::ValidationError) + expect(last_request.env['omniauth.error'].message).to eq("SAML response missing 'missing_attribute' attribute") + end + end + context "when response is a logout response" do before :each do saml_options[:issuer] = "https://idp.sso.example.com/metadata/29490"