eIDAS SAML extensions including SPType and RequestedAttribute/RequestedAttributes

[smarek]

I'm very new to Ruby programming, so I'm sorry for any default mistakes I could've made.

This proposal should provide ability of ruby-saml package to provide samlp:Extensions element as of eidas saml extensions xsd (included in PR) together with SPType (ServiceProviderType) indication and RequestedAttributes collection, directly in AuthRequest

This is based on EC eIDAS eID Profile specification (https://ec.europa.eu/cefdigital/wiki/display/CEFDIGITAL/eIDAS+eID+Profile), specifically eIDAS Message Format v1.2.pdf sections 2.3.2 and 4.1

To explain changes:

• .gitignore - local vendor bundle should be ignored imo in ruby projects
• formatting of code (proper indentation of lines and missing whitespaces) in authrequest.rb, settings.rb and request_test.rb ( i haven't found any coding style guidelines with the project, so i've applied default lint recommendations from RuboCop in my RubyMine IDE)
• requested_attribute.rb - helper class to work with RequestedAttribute element, and provided that to REXML::Element.add_element
• new param of Settings.initialize, to allow combination of keeping none/one-of/both security and extensions attributes
• one test to validate, that code adds expected elements properly (this i plan to expand)
• one test to validate, that settings provide :extensions symbol accessor

While I'm sorry for including changes, not directly relevant to matters, it would be very painful for me to format all my code manually, so please excuse that

[coveralls]

Coverage increased (+0.09%) to 97.985% when pulling 33472e1 on smarek:eidas-saml-extensions into 449dd6b on onelogin:master.

[coveralls]

Coverage increased (+0.03%) to 97.925% when pulling a0833fb on smarek:eidas-saml-extensions into 449dd6b on onelogin:master.

[smarek]

Only Travis job I consider possibly my fault is this: https://travis-ci.org/onelogin/ruby-saml/jobs/617734280
Could anyone please confirm that for me?

[dub357]

@smarek
Please don't take this the wrong way as I know that this isn't my codebase and I'm not a reviewer, but I don't really think that federation-specific extensions belong in a generic SAML library. The code you are trying to introduce is better suited for a custom AuthnRequest subclass that overrides the 'create_xml_document' method and custom Settings subclass that adds an attr_accessor for your 'extensions' and overrides the initialize method to keep (or put back) the security settings. Also, repo owners don't like to see so many changes in a PR that are unrelated to the need being addressed. While the whitespace 'fixes' may be nice to see, they seem kind of unnecessary and really only create more work for reviewers as there are a lot more things to look over.
That being said, I completely understand your struggles here as I underwent the exact same frustrations. The omniauth-saml gem actually instantiates the ruby-saml objects and calls them and doesn't give any additional options or extension points to the underlying ruby-saml library. Could you perhaps sublass OmniAuth::Strategies::SAML and override the 'request_phase' and possibly 'with_settings' method(s) so that it will use your custom AuthnRequest class and Settings objects? Or even monkey-patch it in an initializer? Having to deal with another gem as a middleman makes it very difficult to do custom stuff with ruby-saml which is why I ended up moving away from omniauth-saml to a completely custom middleware stack...

[smarek]

@dub357 whole point of this is to allow login via eIDAS IdP in Gitlab, using just modified configuration of gitlab.yml, since I don't know any better way to override gitlab omniauth-saml/ruby-saml dependencies with custom ones, i've done it this way. I'm open to any suggestions on how to do it better, but in the mentioned context (Gitlab) i'm not sure, there is a better way

[dub357]

Ouch I see...its a little more tricky then since you are dealing with an off-the-shelf product with its own configuration that uses omniauth-saml, which then uses ruby-saml. I've found that most products that claim to support SAML only support the basics. Many dont' even support the full spec. So you have a real challenge here.
Can you set up a SAML proxy and use that for your Gitlab instance(s)? Something like SimpleSAMLPHP can act as both an IdP to 'vanilla' Gitlab and then as an SP to your federation. You can then tweak your SimpleSAMLPHP install (since its open source) to provide the functionality you need. This library may even help with that:

https://github.com/otevrenamesta/simplesamlphp-saml2-eidas

[smarek]

@dub357 cheers, linked library was created by me, so that will definitely help, but not really, unless this ticket is solved simplesamlphp/saml2#211

anyway, with this PR, relevant PR on omniauth-saml and some configuration of gitlab.yml, we're running against eIDAS IdP successfully, so I'm not pushed by my peers, to do anything to these proposals.

and last, yes, wrapping the whole Gitlab with custom SSO proxy, could be solution, but not systematic, and instead of changing the gitlab dependencies (used libraries), i'd have to change the deploy/run model, and that is far more difficult, than what I've already done

[pitbulk]

@dub357 already gave you arguments why this PR can't be accepted.

I think you missed the AttributeService class, which already take care of the attributes that the SP gonna need, so I don't think you need this new requested_attribute class, and use instead the attribute_service which is already used by the metadata class.

As @dub357 suggested, you better fork the authnrequest class reusing the settings.attribute_consuming_service parameter, drop all those new xsd files. Maybe omniauth-saml give you more flexibility or you will need to also extend it

I will leave it here in case someone else requires it.

[marcinbukowczan64-prog]

bitwarden_export_20260207190840.json