changing callback_path to callback_url to account for relative root url #16

Open
wants to merge 1 commit into
from

Conversation

Projects
None yet
4 participants

When an app is deployed to a subdirectory of the webroot, so that the root url is like http://host/subdirectory, the authentication callback fails, since callback_path is passed as the action to the form, which causes the user to be directed to http://host/auth/ldap/callback.

This fix passes callback_url as the action to the form so that the form action is the full url: http://host/subdirectory/auth/ldap/callback for applications deployed to a subdirectory, or http://host/auth/ldap/callback for applications deployed in the host's webroot.

Collaborator

pyu10055 commented Dec 13, 2013

I believe you can set custom :callback_path option to /subdirectory/auth/ldap/callback

:callback_path and :request_path are part of omniauth base strategy, which ldap has included

aldanor commented Mar 10, 2015

Has anyone figured how to resolve this?

Including the sub URI in the :callback_path seems wrong, because then on_callback_path? would fail (as it matches against it). So the correct solution would indeed be to use callback_url.

aldanor commented Mar 10, 2015

TL;DR: this PR does look like a correct solution, are there are any reasons not to merge it in?

jmccann commented Mar 18, 2015

I just ran into this. Tried to work around it by forcing the callback_path but env['omniauth.auth'] is nil. Seems like this is a known issue when you set :callback_path as is suggested as the workaround for this issue.

intridea/omniauth#767

aldanor commented Mar 18, 2015

@jmccann I've ran into the same issue -- the omniauth.auth is empty because is_on_*_path methods start behaving wrong (they are just matching substrings).

I was able to make it work by doing something like this:

module OmniAuthLDAPExt
    def request_phase
        @callback_path = nil  # latest version caches callback path
        path = options[:callback_path]
        options[:callback_path] = callback_url
        form = super
        options[:callback_path] = path
        form
    end
end

module OmniAuth
    module Strategies
        class LDAP
            prepend OmniAuthLDAPExt
        end
    end
end

jmccann commented Mar 18, 2015

@aldanor Thanks! I'm testing monkeypatching in the fix right now doing the following as part of my setup:

module OmniAuth
  module Strategies
    class LDAP
      def request_phase
        OmniAuth::LDAP::Adaptor.validate @options
        f = OmniAuth::Form.new(:title => (options[:title] || "LDAP Authentication"), :url => callback_url)
        f.text_field 'Login', 'username'
        f.password_field 'Password', 'password'
        f.button "Sign In"
        f.to_response
      end
    end
  end
end

aldanor commented Mar 18, 2015

@jmccann it's pretty much the same, I just wanted to avoid copying/pasting all the form code, hence using prepend / super.

jonmbake referenced this pull request in jonmbake/discourse-ldap-auth Jun 17, 2016

Closed

problem when using /forum to access discourse #8

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment