omniauth / omniauth-saml

A generic SAML strategy for OmniAuth
https://github.com/omniauth/omniauth-saml
Other
331 stars 205 forks source link

eIDAS SAML samlp:Extensions to AuthRequest option #180

Open smarek opened 4 years ago

smarek commented 4 years ago

This PR depends on https://github.com/onelogin/ruby-saml/pull/520 being merged and will require binding of omniauth-saml to new version of ruby-saml gem, so I'm pushing the PR for reference and discussion, but don't expect it to be merged before ruby-saml#520 is released

This PR intends to provide samlp:Extensions as per EC eIDAS references ( see https://ec.europa.eu/cefdigital/wiki/display/CEFDIGITAL/eIDAS+eID+Profile )

Basically AuthRequest must contain Extensions with definition of eidas:SPType (ServiceProviderType) and eidas:RequestedAttributes (something that saml-core provides only in SeP metadata)

New option (by default disabled) :auth_request_include_request_attributes allows user to configure sending required Extensions in AuthRequest, and uses options :sptype and :request_attributes to fill the RubySaml::Settings with necessary info

Points for discussion, because this is my first time with your library, and I'm not sure if the implementation follows your expectations/guidelines

Usage should be simple

Rails.application.config.middleware.use OmniAuth::Builder do
  provider :saml,
    :assertion_consumer_service_url     => "consumer_service_url",
    :issuer                             => "rails-application",
    :idp_sso_target_url                 => "idp_sso_target_url",
    :idp_sso_target_url_runtime_params  => {:original_request_param => :mapped_idp_param},
    :idp_cert                           => "-----BEGIN CERTIFICATE-----\n...-----END CERTIFICATE-----",
    :idp_cert_fingerprint               => "E7:91:B2:E1:...",
    :idp_cert_fingerprint_validator     => lambda { |fingerprint| fingerprint },
    :name_identifier_format             => "urn:oasis:names:tc:SAML:1.1:nameid-format:emailAddress",
    # PR relevant configuration options
    :auth_request_include_request_attributes => true,
    :sptype => 'public',
    :request_attributes => [
        {:name => 'http://eidas.europa.eu/attributes/naturalperson/CurrentGivenName', :name_format => 'urn:oasis:names:tc:SAML:2.0:attrname-format:uri', :friendly_name => 'FullName'}
    ]
end

which will result in AuthRequest having this XML snippet included:

  <samlp:Extensions xmlns:eidas="http://eidas.europa.eu/saml-extensions">
    <eidas:SPType xmlns:eidas="http://eidas.europa.eu/saml-extensions">public</eidas:SPType>
    <eidas:RequestedAttributes xmlns:eidas="http://eidas.europa.eu/saml-extensions">
      <eidas:RequestedAttribute Name="http://eidas.europa.eu/attributes/naturalperson/CurrentGivenName" isRequired="false" NameFormat="urn:oasis:names:tc:SAML:2.0:attrname-format:uri" FriendlyName="FullName"/>
    </eidas:RequestedAttributes>
  </samlp:Extensions>

Anyway excuse my ruby skills, i've just started, hope my PR is readable to you

coveralls commented 4 years ago

Coverage Status

Coverage decreased (-4.0%) to 96.009% when pulling ebe95540a74a1ae35599fd2e96285b5401ffacdc on smarek:eidas-saml-extensions into a0eedd6e2daf9fe6285622d4d635aca165358dce on omniauth:master.

smarek commented 4 years ago

Travis CI tests fail from obvious reasons, but locally the whole thing can be tested using Bundler git/github repository feature

Step.1: require updated ruby-saml gem from github via Bundler instead of gemspec (see repo diff lower) Step.2: bundle exec rspec

> git diff
diff --git a/Gemfile b/Gemfile
index 3cb5947..71e5285 100644
--- a/Gemfile
+++ b/Gemfile
@@ -11,5 +11,6 @@ group :test do
 end

 gem 'appraisal'
+gem 'ruby-saml', :github => 'smarek/ruby-saml', :branch => 'eidas-saml-extensions'

 gemspec
diff --git a/omniauth-saml.gemspec b/omniauth-saml.gemspec
index 796796f..bc8e989 100644
--- a/omniauth-saml.gemspec
+++ b/omniauth-saml.gemspec
@@ -14,7 +14,7 @@ Gem::Specification.new do |gem|
   gem.required_ruby_version = '>= 2.1'

   gem.add_runtime_dependency 'omniauth', '~> 1.3', '>= 1.3.2'
-  gem.add_runtime_dependency 'ruby-saml', '~> 1.8'
+#  gem.add_runtime_dependency 'ruby-saml', '~> 1.11.1'

   gem.add_development_dependency 'rake', '>= 10', '< 12'
   gem.add_development_dependency 'rspec', '~>3.4'

For me the tests passes

omniauth-saml #
> bundle exec rspec
....................................

Finished in 0.49922 seconds (files took 0.21124 seconds to load)
36 examples, 0 failures

Coverage report generated for RSpec to omniauth-saml/coverage. 177 / 177 LOC (100.0%) covered.
omniauth-saml #
> bundle install --path vendor/bundle
Using rake 11.3.0
Using bundler 2.0.2
Using thor 0.20.3
Using appraisal 2.2.0
Using conventional-changelog 1.3.0
Using json 1.8.6
Using docile 1.3.2
Using simplecov-html 0.10.2
Using simplecov 0.16.1
Using tins 1.6.0
Using term-ansicolor 1.7.1
Using coveralls 0.8.23
Using diff-lcs 1.3
Using hashie 3.6.0
Using mime-types 2.99.3
Using mini_portile2 2.4.0
Using nokogiri 1.10.5
Using rack 2.0.7
Using omniauth 1.9.0
Using omniauth-saml 1.10.1 from source at `.`
Using rack-test 0.8.3
Using rspec-support 3.9.0
Using rspec-core 3.9.0
Using rspec-expectations 3.9.0
Using rspec-mocks 3.9.0
Using rspec 3.9.0
Using ruby-saml 1.11.0 from https://github.com/smarek/ruby-saml.git (at eidas-saml-extensions@4980fcf)
Bundle complete! 12 Gemfile dependencies, 27 gems now installed.
Bundled gems are installed into `./vendor/bundle`