Add ability to sign metadata. #207
Conversation
| @@ -84,5 +84,42 @@ def setup | |||
| assert_equal "Friendly Name", req_attr.attribute("FriendlyName").value | |||
| assert_equal "Attribute Value", REXML::XPath.first(xml_doc, "//md:AttributeValue").text.strip | |||
| end | |||
|
|
|||
There was a problem hiding this comment.
it might be nice for cleanliness to instantiate the settings object in a let block, and do the setup for these tests in a before block
describe "when the settings indicate to sign (embedded) the metadata" do
let(:settings) { OneLogin::RubySaml::Settings.new }
let(:xml_text) { OneLogin::RubySaml::Metadata.new.generate(settings) }
before do
settings.issuer = "https://example.com"
# ... etc
end
it "creates a signed metadata" do
# your tests
end
endThere was a problem hiding this comment.
I was following the convention of the other tests in this file, but I can refactor this with the cleaner format. Should I refactor the existing tests as well?
There was a problem hiding this comment.
@Umofomia If you've got the energy to do so, sure! That'd be wonderful! :)
There was a problem hiding this comment.
@Lordnibbler Done. I only refactored the metadata tests since I noticed that @pitbulk still has #197 open and I didn't want to cause too many merge conflicts for him to resolve.
| @@ -85,6 +82,13 @@ def generate(settings) | |||
| # <md:XACMLAuthzDecisionQueryDescriptor WantAssertionsSigned="false" protocolSupportEnumeration="urn:oasis:names:tc:SAML:2.0:protocol"/> | |||
|
|
|||
| meta_doc << REXML::XMLDecl.new("1.0", "UTF-8") | |||
|
|
|||
| # embed signature | |||
| if settings.security[:metadata_signed] && settings.private_key && settings.certificate && settings.security[:embed_sign] | |||
There was a problem hiding this comment.
The Signature on the Metadata element must be always embed (there is not other way) so we may do that independent of what value has the settings.security[:embed_sign].
There was a problem hiding this comment.
Good point. I just pushed up a change that removes the check for security[:embed_sign].
|
Hmm... strange... TravisCI is failing, but the tests pass just fine on my machine. I'll try to see what's going on. |
|
@pitbulk Thanks, I just came to the same conclusion as well. |
…int optional on metadata generator, signature position fixed). Fix algorithm calculator
|
Changes Unknown when pulling 60dd414 on Umofomia:sign-metadata into * on onelogin:master*. |
I also fixed the "embebed" typo that was copy-pasted across many of the comments.