Skip to content
This repository has been archived by the owner on Jan 12, 2024. It is now read-only.

Metadata route does not work #3

Closed
rnubel opened this issue Mar 14, 2016 · 5 comments
Closed

Metadata route does not work #3

rnubel opened this issue Mar 14, 2016 · 5 comments

Comments

@rnubel
Copy link

rnubel commented Mar 14, 2016

The implementation of other_phase within omniauth-saml (see here) prevents this gem from having any sane way to modify that functionality and get the service-provider metadata generation feature to work. However, I've opened a PR (omniauth/omniauth-saml#92) which should let us override just that path-comparison logic to work as expected. Any thoughts?

In the meantime, this very hacky monkeypatch in an initializer gets around the issue:

OmniAuth::Strategies::SAML.class_eval do
  def on_path?(path)
    if path =~ /metadata$/i
      current_path =~ /metadata$/i
    else
      super(path)
    end
  end
end

Coupled with a new route, to actually hit the "other" phase of Omniauth:

match '/auth/saml/:provider/metadata',
  via: [:get, :post],
  to: 'authorization_callbacks#saml',
  as: 'user_omniauth_metadata'
@jturkel
Copy link
Member

jturkel commented Aug 19, 2016

@rnubel - Sorry for the delay in following up on this. It looks like omniauth/omniauth-saml#92 would allow us to monkey patch OmniAuth::Strategies::SAML#on_metadata_path? but I'd prefer to avoid monkey patching if possible. Currently this gem relies on the omniauth setup phase to setup the request_path and callback_path options for a given SAML identity provider for a given request. Unfortunately the setup phase isn't run until after on_metadata_path? is called so none of those dynamically computed paths will have been set yet.

Perhaps we could instead make on_metadata_path? work like OmniAuth::Strategy#on_request_path? which would allow this gem to pass a proc for the metadata_path that could properly take dynamic path segments into account? The implementation of this method would look something like:

def on_metadata_path?
  if options[:metadata_path].respond_to?(:call)
    options[:metadata_path].call(env)
  else
    metadata_path = options[:metadata_path] || "#{request_path}/metadata"
    on_path?(metadata_path)
  end
end

What do you think?

@rnubel
Copy link
Author

rnubel commented Aug 19, 2016

@jturkel that looks like a good approach to me, as it's consistent with the other path matchers.

@jturkel
Copy link
Member

jturkel commented Aug 23, 2016

@rnubel - Any interest in putting together a PR against omniauth-saml for the on_metadata_path? change? I won't have time to get to this for a few weeks but that could accelerate my timetable.

cbeckr added a commit to mes/omniauth-multi-provider-saml that referenced this issue Oct 23, 2016
cbeckr added a commit to mes/omniauth-saml that referenced this issue Oct 23, 2016
@rimshakhalid
Copy link

Is there any update on this issue? I need to show metadata for each provider.

@jturkel
Copy link
Member

jturkel commented Dec 2, 2019

@rimshakhalid - This gem is deprecated in favor of the more generic https://github.com/salsify/omniauth-multi-provider gem. Checkout salsify/omniauth-multi-provider#4 for more details on this issue.

@rnubel rnubel closed this as completed Jan 10, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants