Commit c4fa437

@@ -2,18 +2,13 @@ module Saml
   module Kit
     class AuthenticationRequest < Document
       include Requestable
-      validates_presence_of :acs_url, if: :expected_type?
 
       def initialize(xml)
         super(xml, name: "AuthnRequest")
       end
 
       def acs_url
-        #if signed? && trusted?
-          to_h[name]['AssertionConsumerServiceURL'] || registered_acs_url(binding: :post)
-        #else
-          #registered_acs_url
-        #end
+        to_h[name]['AssertionConsumerServiceURL']
       end
 
       def name_id_format
@@ -26,11 +21,6 @@ module Saml
 
       private
 
-      def registered_acs_url(binding:)
-        return if provider.nil?
-        provider.assertion_consumer_service_for(binding: binding).try(:location)
-      end
-
       class Builder
         attr_accessor :id, :now, :issuer, :acs_url, :name_id_format, :sign, :destination
         attr_accessor :version

@@ -88,7 +88,7 @@ module Saml
           @version = "2.0"
           @status_code = Namespaces::SUCCESS
           @issuer = configuration.issuer
-          @destination = request.acs_url
+          @destination = destination_for(request)
           @sign = want_assertions_signed
         end
 
@@ -146,6 +146,14 @@ module Saml
 
         private
 
+        def destination_for(request)
+          if request.signed? && request.trusted?
+            request.acs_url || request.provider.assertion_consumer_service_for(binding: :post).try(:location)
+          else
+            request.provider.assertion_consumer_service_for(binding: :post).try(:location)
+          end
+        end
+
         def configuration
           Saml::Kit.configuration
         end

@@ -102,35 +102,6 @@ RSpec.describe Saml::Kit::AuthenticationRequest do
       expect(subject.errors[:provider]).to be_present
     end
 
-    it 'is invalid when an assertion consumer service url is not provided' do
-      allow(metadata).to receive(:matches?).and_return(true)
-      allow(metadata).to receive(:assertion_consumer_service_for).and_return(nil)
-
-      builder = described_class::Builder.new
-      builder.acs_url = nil
-      xml = builder.to_xml
-
-      subject = described_class.new(xml)
-      expect(subject).to be_invalid
-      expect(subject.errors[:acs_url]).to be_present
-    end
-
-    it 'is valid when an the ACS is available via the registry' do
-      allow(registry).to receive(:metadata_for).with(issuer)
-        .and_return(metadata)
-      allow(metadata).to receive(:matches?).and_return(true)
-      allow(metadata).to receive(:assertion_consumer_service_for).and_return(
-        Saml::Kit::HttpPostBinding.new(location: acs_url)
-      )
-
-      builder = described_class::Builder.new
-      builder.issuer = issuer
-      builder.acs_url = nil
-      xml = builder.to_xml
-
-      expect(described_class.new(xml)).to be_valid
-    end
-
     it 'validates the schema of the request' do
       xml = ::Builder::XmlMarkup.new
       id = SecureRandom.uuid
@@ -175,19 +146,13 @@ XML
       expect(subject.acs_url).to eql(acs_url)
     end
 
-    it 'returns the registered ACS url' do
+    it 'returns nil' do
       builder = described_class::Builder.new
       builder.issuer = issuer
       builder.acs_url = nil
       subject = builder.build
 
-      allow(Saml::Kit.configuration).to receive(:registry).and_return(registry)
-      allow(registry).to receive(:metadata_for).and_return(metadata)
-      allow(registry).to receive(:metadata_for).with(issuer).and_return(metadata)
-      allow(metadata).to receive(:assertion_consumer_service_for).and_return(
-        Saml::Kit::HttpPostBinding.new(location: acs_url)
-      )
-      expect(subject.acs_url).to eql(acs_url)
+      expect(subject.acs_url).to be_nil
     end
   end
 end

@@ -76,7 +76,7 @@ RSpec.describe Saml::Kit::HttpPostBinding do
 
     it 'deserializes to a Response' do
       user = double(:user, name_id_for: SecureRandom.uuid, assertion_attributes_for: [])
-      request = double(:request, id: SecureRandom.uuid, provider: nil, acs_url: FFaker::Internet.http_url, name_id_format: Saml::Kit::Namespaces::PERSISTENT, issuer: FFaker::Internet.http_url)
+      request = double(:request, id: SecureRandom.uuid, provider: nil, acs_url: FFaker::Internet.http_url, name_id_format: Saml::Kit::Namespaces::PERSISTENT, issuer: FFaker::Internet.http_url, signed?: true, trusted?: true)
       builder = Saml::Kit::Response::Builder.new(user, request)
       _, params = subject.serialize(builder)
       result = subject.deserialize(params)

@@ -47,7 +47,7 @@ RSpec.describe Saml::Kit::HttpRedirectBinding do
 
     it 'deserializes the SAMLResponse to a Response' do
       user = double(:user, name_id_for: SecureRandom.uuid, assertion_attributes_for: [])
-      request = double(:request, id: SecureRandom.uuid, provider: nil, acs_url: FFaker::Internet.http_url, name_id_format: Saml::Kit::Namespaces::PERSISTENT, issuer: issuer)
+      request = double(:request, id: SecureRandom.uuid, provider: nil, acs_url: FFaker::Internet.http_url, name_id_format: Saml::Kit::Namespaces::PERSISTENT, issuer: issuer, signed?: true, trusted?: true)
       url, _ = subject.serialize(Saml::Kit::Response::Builder.new(user, request))
       result = subject.deserialize(query_params_from(url))
       expect(result).to be_instance_of(Saml::Kit::Response)

@@ -3,19 +3,33 @@ require 'spec_helper'
 RSpec.describe Saml::Kit::Response do
   describe "#destination" do
     let(:acs_url) { "https://#{FFaker::Internet.domain_name}/acs" }
-    let(:user) { double(:user, name_id_for: SecureRandom.uuid, assertion_attributes_for: { }) }
-    let(:request) { instance_double(Saml::Kit::AuthenticationRequest, id: SecureRandom.uuid, acs_url: acs_url, issuer: FFaker::Movie.title, name_id_format: Saml::Kit::Namespaces::EMAIL_ADDRESS, provider: nil) }
+    let(:user) { double(:user, name_id_for: SecureRandom.uuid, assertion_attributes_for: []) }
     subject { described_class::Builder.new(user, request).build }
 
-    it 'returns the acs_url' do
-      expect(subject.destination).to eql(acs_url)
+    describe "when the request is signed and trusted" do
+      let(:request) { instance_double(Saml::Kit::AuthenticationRequest, id: SecureRandom.uuid, acs_url: acs_url, issuer: FFaker::Movie.title, name_id_format: Saml::Kit::Namespaces::EMAIL_ADDRESS, provider: nil, signed?: true, trusted?: true) }
+
+      it 'returns the ACS embedded in the request' do
+        expect(subject.destination).to eql(acs_url)
+      end
+    end
+
+    describe "when the request is not trusted" do
+      let(:registered_acs_url) { FFaker::Internet.uri("https") }
+      let(:request) { instance_double(Saml::Kit::AuthenticationRequest, id: SecureRandom.uuid, acs_url: acs_url, issuer: FFaker::Movie.title, name_id_format: Saml::Kit::Namespaces::EMAIL_ADDRESS, provider: provider, signed?: true, trusted?: false) }
+      let(:provider) { instance_double(Saml::Kit::ServiceProviderMetadata, want_assertions_signed: false) }
+
+      it 'returns the registered ACS embedded in the metadata' do
+        allow(provider).to receive(:assertion_consumer_service_for).and_return(double(location: registered_acs_url))
+        expect(subject.destination).to eql(registered_acs_url)
+      end
     end
   end
 
   describe "#to_xml" do
     subject { described_class::Builder.new(user, request) }
     let(:user) { double(:user, name_id_for: SecureRandom.uuid, assertion_attributes_for: { email: email, created_at: Time.now.utc.iso8601 }) }
-    let(:request) { double(id: SecureRandom.uuid, acs_url: acs_url, issuer: FFaker::Movie.title, name_id_format: Saml::Kit::Namespaces::EMAIL_ADDRESS, provider: nil) }
+    let(:request) { double(id: SecureRandom.uuid, acs_url: acs_url, issuer: FFaker::Movie.title, name_id_format: Saml::Kit::Namespaces::EMAIL_ADDRESS, provider: nil, trusted?: true, signed?: true) }
     let(:acs_url) { "https://#{FFaker::Internet.domain_name}/acs" }
     let(:issuer) { FFaker::Movie.title }
     let(:email) { FFaker::Internet.email }
@@ -76,7 +90,7 @@ RSpec.describe Saml::Kit::Response do
   end
 
   describe "#valid?" do
-    let(:request) { instance_double(Saml::Kit::AuthenticationRequest, id: "_#{SecureRandom.uuid}", issuer: FFaker::Internet.http_url, acs_url: FFaker::Internet.http_url, name_id_format: Saml::Kit::Namespaces::PERSISTENT, provider: nil) }
+    let(:request) { instance_double(Saml::Kit::AuthenticationRequest, id: "_#{SecureRandom.uuid}", issuer: FFaker::Internet.http_url, acs_url: FFaker::Internet.http_url, name_id_format: Saml::Kit::Namespaces::PERSISTENT, provider: nil, signed?: true, trusted?: true) }
     let(:user) { double(:user, name_id_for: SecureRandom.uuid, assertion_attributes_for: { id: SecureRandom.uuid }) }
     let(:builder) { described_class::Builder.new(user, request) }
     let(:registry) { instance_double(Saml::Kit::DefaultRegistry) }
@@ -232,7 +246,7 @@ RSpec.describe Saml::Kit::Response do
   describe described_class::Builder do
     subject { described_class.new(user, request) }
     let(:user) { double(:user, name_id_for: SecureRandom.uuid, assertion_attributes_for: []) }
-    let(:request) { double(:request, id: SecureRandom.uuid, acs_url: FFaker::Internet.http_url, provider: nil, name_id_format: Saml::Kit::Namespaces::PERSISTENT, issuer: FFaker::Internet.http_url) }
+    let(:request) { double(:request, id: SecureRandom.uuid, acs_url: FFaker::Internet.http_url, provider: nil, name_id_format: Saml::Kit::Namespaces::PERSISTENT, issuer: FFaker::Internet.http_url, signed?: true, trusted?: true) }
 
     describe "#build" do
       it 'builds a response with the request_id' do

@@ -1,11 +1,11 @@
 require 'spec_helper'
 
 RSpec.describe Saml::Kit::ServiceProviderMetadata do
-  let(:entity_id) { FFaker::Internet.http_url }
-  let(:acs_post_url) { FFaker::Internet.http_url }
-  let(:acs_redirect_url) { FFaker::Internet.http_url }
-  let(:logout_post_url) { FFaker::Internet.http_url }
-  let(:logout_redirect_url) { FFaker::Internet.http_url }
+  let(:entity_id) { FFaker::Internet.uri("https") }
+  let(:acs_post_url) { FFaker::Internet.uri("https") }
+  let(:acs_redirect_url) { FFaker::Internet.uri("https") }
+  let(:logout_post_url) { FFaker::Internet.uri("https") }
+  let(:logout_redirect_url) { FFaker::Internet.uri("https") }
 
   describe described_class::Builder do
     let(:acs_url) { FFaker::Internet.http_url }
@@ -149,6 +149,19 @@ RSpec.describe Saml::Kit::ServiceProviderMetadata do
       expect(subject).to be_invalid
       expect(subject.errors[:base]).to include("invalid signature.")
     end
+
+    it 'is invalid when 0 ACS endpoints are specified' do
+      xml = <<-XML
+<?xml version="1.0" encoding="UTF-8"?>
+<EntityDescriptor xmlns="#{Saml::Kit::Namespaces::METADATA}" ID="_#{SecureRandom.uuid}" entityID="#{entity_id}">
+  <SPSSODescriptor AuthnRequestsSigned="false" WantAssertionsSigned="true" protocolSupportEnumeration="#{Saml::Kit::Namespaces::PROTOCOL}">
+    <SingleLogoutService Binding="#{Saml::Kit::Namespaces::HTTP_POST}" Location="#{FFaker::Internet.uri("https")}"/>
+    <NameIDFormat>#{Saml::Kit::Namespaces::PERSISTENT}</NameIDFormat>
+  </SPSSODescriptor>
+</EntityDescriptor>
+      XML
+      expect(described_class.new(xml)).to be_invalid
+    end
   end
 
   describe "#matches?" do