Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add ability to sign metadata. #207

Closed
wants to merge 4 commits into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
9 changes: 6 additions & 3 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -336,14 +336,17 @@ In order to be able to sign we need first to define the private key and the publ
The settings related to sign are stored in the `security` attribute of the settings:

```ruby
settings.security[:authn_requests_signed] = true # Enable or not signature on AuthNRequest
settings.security[:logout_requests_signed] = true # Enable or not signature on Logout Request
settings.security[:authn_requests_signed] = true # Enable or not signature on AuthNRequest
settings.security[:logout_requests_signed] = true # Enable or not signature on Logout Request
settings.security[:logout_responses_signed] = true # Enable or not signature on Logout Response
settings.security[:metadata_signed] = true # Enable or not signature on Metadata

settings.security[:digest_method] = XMLSecurity::Document::SHA1
settings.security[:signature_method] = XMLSecurity::Document::SHA1

settings.security[:embed_sign] = false # Embeded signature or HTTP GET parameter Signature
# Embeded signature or HTTP GET parameter signature
# Note that metadata signature is always embedded regardless of this value.
settings.security[:embed_sign] = false
```


Expand Down
2 changes: 1 addition & 1 deletion lib/onelogin/ruby-saml/authrequest.rb
Original file line number Diff line number Diff line change
Expand Up @@ -111,7 +111,7 @@ def create_authentication_xml_doc(settings)
end
end

# embebed sign
# embed signature
if settings.security[:authn_requests_signed] && settings.private_key && settings.certificate && settings.security[:embed_sign]
private_key = settings.get_sp_key()
cert = settings.get_sp_cert()
Expand Down
2 changes: 1 addition & 1 deletion lib/onelogin/ruby-saml/logoutrequest.rb
Original file line number Diff line number Diff line change
Expand Up @@ -88,7 +88,7 @@ def create_logout_request_xml_doc(settings)
sessionindex.text = settings.sessionindex
end

# embebed sign
# embed signature
if settings.security[:logout_requests_signed] && settings.private_key && settings.certificate && settings.security[:embed_sign]
private_key = settings.get_sp_key()
cert = settings.get_sp_cert()
Expand Down
12 changes: 8 additions & 4 deletions lib/onelogin/ruby-saml/metadata.rb
Original file line number Diff line number Diff line change
@@ -1,5 +1,3 @@
require "rexml/document"
require "rexml/xpath"
require "uri"

require "onelogin/ruby-saml/logging"
Expand All @@ -10,10 +8,9 @@
# will be updated automatically
module OneLogin
module RubySaml
include REXML
class Metadata
def generate(settings)
meta_doc = REXML::Document.new
meta_doc = XMLSecurity::Document.new
root = meta_doc.add_element "md:EntityDescriptor", {
"xmlns:md" => "urn:oasis:names:tc:SAML:2.0:metadata"
}
Expand Down Expand Up @@ -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
private_key = settings.get_sp_key()
meta_doc.sign_document(private_key, cert, settings.security[:signature_method], settings.security[:digest_method])
end

ret = ""
# pretty print the XML so IdP administrators can easily see what the SP supports
meta_doc.write(ret, 1)
Expand Down
2 changes: 1 addition & 1 deletion lib/onelogin/ruby-saml/slo_logoutresponse.rb
Original file line number Diff line number Diff line change
Expand Up @@ -86,7 +86,7 @@ def create_logout_response_xml_doc(settings, request_id = nil, logout_message =
issuer.text = settings.issuer
end

# embebed sign
# embed signature
if settings.security[:logout_responses_signed] && settings.private_key && settings.certificate && settings.security[:embed_sign]
private_key = settings.get_sp_key()
cert = settings.get_sp_cert()
Expand Down
2 changes: 1 addition & 1 deletion test/logoutrequest_test.rb
Original file line number Diff line number Diff line change
Expand Up @@ -88,7 +88,7 @@ class RequestTest < Minitest::Test
end
end

describe "when the settings indicate to sign (embebed) the logout request" do
describe "when the settings indicate to sign (embedded) the logout request" do
it "created a signed logout request" do
settings = OneLogin::RubySaml::Settings.new
settings.idp_slo_target_url = "http://example.com?field=value"
Expand Down
133 changes: 76 additions & 57 deletions test/metadata_test.rb
Original file line number Diff line number Diff line change
Expand Up @@ -3,86 +3,105 @@
class MetadataTest < Minitest::Test

describe 'Metadata' do
def setup
@settings = OneLogin::RubySaml::Settings.new
@settings.issuer = "https://example.com"
@settings.name_identifier_format = "urn:oasis:names:tc:SAML:1.1:nameid-format:emailAddress"
@settings.assertion_consumer_service_url = "https://foo.example/saml/consume"
@settings.security[:authn_requests_signed] = false
end

it "generates Service Provider Metadata with X509Certificate" do
@settings.security[:authn_requests_signed] = true
@settings.certificate = ruby_saml_cert_text

xml_text = OneLogin::RubySaml::Metadata.new.generate(@settings)

# assert xml_text can be parsed into an xml doc
xml_doc = REXML::Document.new(xml_text)
let(:settings) { OneLogin::RubySaml::Settings.new }
let(:xml_text) { OneLogin::RubySaml::Metadata.new.generate(settings) }
let(:xml_doc) { REXML::Document.new(xml_text) }
let(:spsso_descriptor) { REXML::XPath.first(xml_doc, "//md:SPSSODescriptor") }
let(:acs) { REXML::XPath.first(xml_doc, "//md:AssertionConsumerService") }

spsso_descriptor = REXML::XPath.first(xml_doc, "//md:SPSSODescriptor")
assert_equal "true", spsso_descriptor.attribute("AuthnRequestsSigned").value

cert_node = REXML::XPath.first(xml_doc, "//md:KeyDescriptor/ds:KeyInfo/ds:X509Data/ds:X509Certificate", {
"md" => "urn:oasis:names:tc:SAML:2.0:metadata",
"ds" => "http://www.w3.org/2000/09/xmldsig#"
})
cert_text = cert_node.text
cert = OpenSSL::X509::Certificate.new(Base64.decode64(cert_text))
assert_equal ruby_saml_cert.to_der, cert.to_der
end

it "generates Service Provider Metadata" do
settings = OneLogin::RubySaml::Settings.new
before do
settings.issuer = "https://example.com"
settings.name_identifier_format = "urn:oasis:names:tc:SAML:1.1:nameid-format:emailAddress"
settings.assertion_consumer_service_url = "https://foo.example/saml/consume"
settings.security[:authn_requests_signed] = false

xml_text = OneLogin::RubySaml::Metadata.new.generate(settings)
end

it "generates Service Provider Metadata" do
# assert correct xml declaration
start = "<?xml version='1.0' encoding='UTF-8'?>\n<md:EntityDescriptor"
assert xml_text[0..start.length-1] == start

# assert xml_text can be parsed into an xml doc
xml_doc = REXML::Document.new(xml_text)

assert_equal "https://example.com", REXML::XPath.first(xml_doc, "//md:EntityDescriptor").attribute("entityID").value

spsso_descriptor = REXML::XPath.first(xml_doc, "//md:SPSSODescriptor")
assert_equal "urn:oasis:names:tc:SAML:2.0:protocol", spsso_descriptor.attribute("protocolSupportEnumeration").value
assert_equal "false", spsso_descriptor.attribute("AuthnRequestsSigned").value
assert_equal "false", spsso_descriptor.attribute("WantAssertionsSigned").value

assert_equal "urn:oasis:names:tc:SAML:1.1:nameid-format:emailAddress", REXML::XPath.first(xml_doc, "//md:NameIDFormat").text.strip

acs = REXML::XPath.first(xml_doc, "//md:AssertionConsumerService")
assert_equal "urn:oasis:names:tc:SAML:2.0:bindings:HTTP-POST", acs.attribute("Binding").value
assert_equal "https://foo.example/saml/consume", acs.attribute("Location").value
end

it "generates attribute service if configured" do
settings = OneLogin::RubySaml::Settings.new
settings.issuer = "https://example.com"
settings.name_identifier_format = "urn:oasis:names:tc:SAML:1.1:nameid-format:emailAddress"
settings.assertion_consumer_service_url = "https://foo.example/saml/consume"
settings.attribute_consuming_service.configure do
service_name "Test Service"
add_attribute(:name => "Name", :name_format => "Name Format", :friendly_name => "Friendly Name", :attribute_value => "Attribute Value")
describe "when auth requests are signed" do
let(:cert_node) do
REXML::XPath.first(
xml_doc,
"//md:KeyDescriptor/ds:KeyInfo/ds:X509Data/ds:X509Certificate",
"md" => "urn:oasis:names:tc:SAML:2.0:metadata",
"ds" => "http://www.w3.org/2000/09/xmldsig#"
)
end
let(:cert) { OpenSSL::X509::Certificate.new(Base64.decode64(cert_node.text)) }

before do
settings.security[:authn_requests_signed] = true
settings.certificate = ruby_saml_cert_text
end

it "generates Service Provider Metadata with X509Certificate" do
assert_equal "true", spsso_descriptor.attribute("AuthnRequestsSigned").value
assert_equal ruby_saml_cert.to_der, cert.to_der
end
end

describe "when attribute service is configured" do
let(:attr_svc) { REXML::XPath.first(xml_doc, "//md:AttributeConsumingService") }
let(:req_attr) { REXML::XPath.first(xml_doc, "//md:RequestedAttribute") }

before do
settings.attribute_consuming_service.configure do
service_name "Test Service"
add_attribute(:name => "Name", :name_format => "Name Format", :friendly_name => "Friendly Name", :attribute_value => "Attribute Value")
end
end

it "generates attribute service" do
assert_equal "true", attr_svc.attribute("isDefault").value
assert_equal "1", attr_svc.attribute("index").value
assert_equal REXML::XPath.first(xml_doc, "//md:ServiceName").text.strip, "Test Service"

assert_equal "Name", req_attr.attribute("Name").value
assert_equal "Name Format", req_attr.attribute("NameFormat").value
assert_equal "Friendly Name", req_attr.attribute("FriendlyName").value
assert_equal "Attribute Value", REXML::XPath.first(xml_doc, "//md:AttributeValue").text.strip
end
end

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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
end

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@Umofomia If you've got the energy to do so, sure! That'd be wonderful! :)

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@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.

describe "when the settings indicate to sign (embedded) the metadata" do
before do
settings.security[:metadata_signed] = true
settings.certificate = ruby_saml_cert_text
settings.private_key = ruby_saml_key_text
end

it "creates a signed metadata" do
assert_match %r[<ds:SignatureValue>\s*([a-zA-Z0-9/+=]+)\s*</ds:SignatureValue>]m, xml_text
assert_match %r[<ds:SignatureMethod Algorithm='http://www.w3.org/2000/09/xmldsig#rsa-sha1'/>], xml_text
assert_match %r[<ds:DigestMethod Algorithm='http://www.w3.org/2000/09/xmldsig#rsa-sha1'/>], xml_text
end

xml_text = OneLogin::RubySaml::Metadata.new.generate(settings)
xml_doc = REXML::Document.new(xml_text)
acs = REXML::XPath.first(xml_doc, "//md:AttributeConsumingService")
assert_equal "true", acs.attribute("isDefault").value
assert_equal "1", acs.attribute("index").value
assert_equal REXML::XPath.first(xml_doc, "//md:ServiceName").text.strip, "Test Service"
req_attr = REXML::XPath.first(xml_doc, "//md:RequestedAttribute")
assert_equal "Name", req_attr.attribute("Name").value
assert_equal "Name Format", req_attr.attribute("NameFormat").value
assert_equal "Friendly Name", req_attr.attribute("FriendlyName").value
assert_equal "Attribute Value", REXML::XPath.first(xml_doc, "//md:AttributeValue").text.strip
describe "when digest and signature methods are specified" do
before do
settings.security[:signature_method] = XMLSecurity::Document::SHA256
settings.security[:digest_method] = XMLSecurity::Document::SHA512
end

it "creates a signed metadata with specified digest and signature methods" do
assert_match %r[<ds:SignatureValue>\s*([a-zA-Z0-9/+=]+)\s*</ds:SignatureValue>]m, xml_text
assert_match %r[<ds:SignatureMethod Algorithm='http://www.w3.org/2001/04/xmldsig-more#rsa-sha256'/>], xml_text
assert_match %r[<ds:DigestMethod Algorithm='http://www.w3.org/2001/04/xmldsig-more#rsa-sha512'/>], xml_text
end
end
end
end
end
2 changes: 1 addition & 1 deletion test/request_test.rb
Original file line number Diff line number Diff line change
Expand Up @@ -143,7 +143,7 @@ class RequestTest < Minitest::Test
end
end

describe "when the settings indicate to sign (embebed) the request" do
describe "when the settings indicate to sign (embedded) the request" do
it "create a signed request" do
settings = OneLogin::RubySaml::Settings.new
settings.compress_request = false
Expand Down
2 changes: 1 addition & 1 deletion test/slo_logoutresponse_test.rb
Original file line number Diff line number Diff line change
Expand Up @@ -65,7 +65,7 @@ class SloLogoutresponseTest < Minitest::Test
assert_match /<samlp:StatusMessage>Custom Logout Message<\/samlp:StatusMessage>/, inflated
end

describe "when the settings indicate to sign (embebed) the logout response" do
describe "when the settings indicate to sign (embedded) the logout response" do
it "create a signed logout response" do
settings = OneLogin::RubySaml::Settings.new
settings.compress_response = false
Expand Down