digidentity / libsaml

A Ruby gem to easily create SAML 2.0 messages.
MIT License
72 stars 34 forks source link

Saml.provider(entity_id) should handle 2 kind of entity_id when acting as IdP #136

Closed nov closed 7 years ago

nov commented 7 years ago

when handling authn request, the Entity ID given to Saml.provider(entity_id) is of SP (= request issuer). however, when generating response, the Entity ID given to Saml.provider(entity_id) is of IdP (= response issuer).

So, current_provider && current_provider.entity_id == entity_id used in Saml.provider(entity_id) doesn't work well in IdP context.

module Saml
  def self.provider(entity_id)
    if current_provider && current_provider.entity_id == entity_id
      current_provider
    else
      current_store.find_by_entity_id(entity_id) || raise(Saml::Errors::InvalidProvider.new("Cannot find provider with entity_id: #{entity_id}"))
    end
  end
end

Do you have any advice to handle such case?

benoist commented 7 years ago

We are currently using the following in the controller

    extend Saml::Rails::ControllerHelper
    current_provider :current_identity_provider

    private 
    def current_identity_provider
      SamlProvider.find_by_entity_id("idp:entity:id") || Saml.provider("idp:entity:id")
    end
nov commented 7 years ago

Ah, that's probably something different.

I'm currently doing similar in my controller. However, when I call Saml::Bindings::HTTPPost#receive_message, request_or_response.provider is called internally, and it accesses Saml.provider(request_or_response.entity_id).

And it results in calling Saml.provider(sp_entity_id) when parsing request, and Saml.provider(idp_entity_id) when generating response.

Since my Provider model has only one entity_id (and it's SP entity_id in my implementation), it fails entity_id check here, and try to access current_store.find_by_entity_id(entity_id). https://github.com/digidentity/libsaml/blob/master/lib/saml.rb#L258

ps. I attach my current controller code. At this line, I'm modifying the provider instance's entity_id just before generating a response, so that response issuer becomes IDP EntityID, and still response issuer can be SP EntityID. https://gist.github.com/nov/6a6df3c2231baf247ca7ae6d0ee82dff#file-saml_assertions_controller-rb-L26

nov commented 7 years ago

plus, even with this code, request_or_response.provider checks whether request/response issuer is same with provider.entity_id. and it will fail either at request parsing or response generation. https://github.com/digidentity/libsaml/blob/master/lib/saml.rb#L258

extend Saml::Rails::ControllerHelper
current_provider :current_identity_provider

private 
def current_identity_provider
   SamlProvider.find_by_entity_id("idp:entity:id") || Saml.provider("idp:entity:id")
end
benoist commented 7 years ago

Do you mean the provider store you have set up only contains SP's and not your IDP?

benoist commented 7 years ago

We have a provider store that contains both IDP and SP. So Saml.provider(entity_id) may return an SP or an IDP.

With the current_provider method you set the IDP for all actions in the controller. So creating a response will be done as the specified IDP. When receiving messages the provider is retrieved from the provider store, by using the issuer in the message. The both use the Saml.provider lookup method, so the store needs to contain all provider. The reason for this is, that an entity descriptor may contain both IDP and SP descriptors allowing it to act as an IDP and SP. The Saml::Provider module has a type method to retrieve the type and may return "identity_and_service_provider", "identity_provider" or "service_provider".

We have 2 different SAML endpoints in the same application, both with their own store and their own IDP and SP's. This can be specified with the current_provider and current_store before filters.

nov commented 7 years ago

Ah, now I understood the expectation of the gem.

My federation server is multi-tenanted, and SP and IdP are in 1-to-1 relation.

For example,

So one service provider instance represents both a SP and an IdP connected with it at the same time, and the instance has both IdP Entity ID and SP Entity ID as its attributes. However, provider.entity_id represents SP Entity ID.

benoist commented 7 years ago

The entity_id in SAML represents an EntityDescriptor. An EntityDescriptor may have 1 IDP SSO Descriptor and may have 1 SP SSO Descriptor.

You can also create providers on the fly for each request if that fits your needs better.

class AssertionsController < ApplicationController
  extend Saml::Rails::ControllerHelper
  current_provider :current_identity_provider

  def metadata
    render xml: current_service_provider.entity_descriptor.to_xml
  end

  private

  def destination
    provider.single_sign_on_service_url Saml::ProtocolBinding::HTTP_REDIRECT
  end

  def entity_descriptor
    signing_key_descriptor    = Saml::Elements::KeyDescriptor.new(certificate: "PEM", use: 'signing')
    encryption_key_descriptor = Saml::Elements::KeyDescriptor.new(certificate: "PEM", use: 'encryption')

    signle_sign_on_service = Saml::Elements::IDPSSODescriptor::SingleSignOnService.new(
        binding:  Saml::ProtocolBinding::HTTP_POST,
        location: "endpoint"
    )

    idp_sso_descriptor = Saml::Elements::IDPSSODescriptor.new(
        signle_sign_on_services: [signle_sign_on_service],
        key_descriptors:         [signing_key_descriptor, encryption_key_descriptor]
    )

    entity_descriptor                    = Saml::Elements::EntityDescriptor.new(entity_id: "TENANT ENTITY ID")
    entity_descriptor.idp_sso_descriptor = idp_sso_descriptor

    entity_descriptor
  end

  def current_identity_provider
    Saml::BasicProvider.new(entity_descriptor, "PRIVAT KEY PEM", 'identity_provider')
  end
end
nov commented 7 years ago

I see. Switching Entity ID based on whether decoding request or building response as I'm doing here seems fine. https://gist.github.com/nov/6a6df3c2231baf247ca7ae6d0ee82dff#file-saml_assertions_controller-rb-L26

Thanks!