Redirect to login if guest visits wizard

Sponsor: @Economicurtis

Description

If a guest visits a wizard url currently they see this

This task is to redirect guests trying to visit a wizard to forum.domain.com/login, then redirect them back to the wizard upon login completion.

Notes

  • This task involves adding another modification to the ApplicationController. The existing customisations this plugin makes are here: discourse-custom-wizard/plugin.rb at master · paviliondev/discourse-custom-wizard · GitHub

  • The basic structure will be along the lines of:

    1. If the request url is a wizard url (see existing modficiation), and there is no current user, redirect and store original wizard url

    2. On completion of sign-in, redirect to stored wizard url.

1 Like

@pacharanero I think this would be a good one for you.

Note:

Screenshot%20at%20Oct%2003%2013-49-46

2 Likes

Looks good to me. I’m available tomorrow for those kind of hours.

Love the way hours/rate/total displays in the Topic BTW. A neat refinement would be more detailed explanation of what the numbers/icons mean on hover (I realise that by suggesting this I am probably assigning myself to make it happen, too)

3 Likes

@pacharanero Great, let me know if you have any issues. As mentioned above, this is for @Economicurtis, who runs https://community.rstudio.com.

2 Likes

@pacharanero How did you go with this?

@angus Good initial progress but then hit some issues.

I got set up with a suitable test instance with the plugin installed and imported an example wizard from Pavilion.io.

Initially I started adding a preceding before_action in the ApplicationController (which is meant to be valid Rails), but this cause the server to crash.

I wondered if this was maybe if Discourse is perhaps doing something ‘nonstandard Rails’ when it loads the plugin, meaning that multiple before_actions wouldn’t work. So I tried a different approach, using a single before action, and putting the conditional logic about whether or not there is a current_user in there.

Current code is trying various things around this kind of theme:

require_dependency 'application_controller'
  class ::ApplicationController
    before_action :redirect_to_wizard_or_login_if_required

    def redirect_to_wizard_or_login_if_required
      if current_user
        wizard_id = current_user.custom_fields['redirect_to_wizard']
        @excluded_routes ||= SiteSetting.wizard_redirect_exclude_paths.split('|') + ['/w/']
        url = request.referer || request.original_url

        if request.format === 'text/html' && !@excluded_routes.any? {|str| /#{str}/ =~ url} && wizard_id
          if request.referer !~ /\/w\// && request.referer !~ /\/invites\//
            CustomWizard::Wizard.set_submission_redirect(current_user, wizard_id, request.referer)
          end

          if CustomWizard::Wizard.exists?(wizard_id)
            redirect_to "/w/#{wizard_id.dasherize}"
          end
        end
      else #there is no current_user, so they need to log in first
        # store requested URL somewhere
        redirect_to "/login"
      end
    end
  end

This current code fails with ‘Too many redirects’ suggesting there’s a circular redirect perhaps.

I would suggest to replace redirect calls with puts and see if redirection call is happening more than once.

1 Like

@pacharanero multiple before_action calls should work. I just gave this shot on my local and it seems to be working:

require_dependency 'application_controller'
class ::ApplicationController
  before_action :redirect_to_wizard_if_required, if: :current_user
  before_action :redirect_to_login_if_wizard_guest, unless: :current_user 

  ... // redirect_to_wizard_if_required method

  def redirect_to_login_if_wizard_guest
    if request.referer =~ /\/w\//
        puts "HELLO"
    end
  end
end

When visiting a wizard route as a guest, I see

Processing by CustomWizard::WizardController#index as JSON
Parameters: {"_"=>"1570441080052", "wizard_id"=>"test"}
HELLO

One thing that could cause the redirects here is that multiple requests are made to the server on any page load. Looking through the server log, there are multiple "HELLO"s. So you may want to make it specific to the wizard controller, i.e.

controller.controller_name == CustomWizard::WizardController

Or something like that.

OK the multiple requests thing is fixed by making it only respond when controller_path == "custom_wizard/wizard" (I couldn’t access a controller object in that context but controller_path seems to work)

Would we save the destination URL in a cookie or in the session? Elsewhere in Discourse it is saved in a cookie unless SSO is enabled (I don’t understand the reason for that).

2 things I would like to mention.

  1. How about monkey patching the CustomWizardController itself instead of ApplicationController ?

If you want to redirect back to the wizard, you can use a url param i.e., your_url?redirect-to=‘wizard_url’, then when login succeeds, just fetch the param from the url and redirect there…

As I’m learning too, it would be helpful for me to understand those details.

OK I’ve pushed a version of non-working code to a feature branch

When this runs I get an apparent success in the Rails log but no change in URL and the page is blank. Clearly I’m not doing something right, I suspect in the Ember side of things.

Processing by CustomWizard::WizardController#index as JSON
  Parameters: {"_"=>"1570460238651", "wizard_id"=>"feature-request"}
Redirected to http://localhost:9292/login?destination_wizard='http://localhost:9292/w/feature-request?_=1570460238651'
Filter chain halted as :redirect_to_login_if_wizard_guest rendered or redirected
Completed 302 Found in 4ms (ActiveRecord: 0.0ms | Allocations: 556)


Started GET "/login?destination_wizard=%27http://localhost:9292/w/feature-request?_=1570460238651%27" for 172.17.0.1 at 2019-10-07 14:57:19 +0000
Processing by StaticController#show as JSON
  Parameters: {"destination_wizard"=>"'http://localhost:9292/w/feature-request?_=1570460238651'", "id"=>"login"}
  Rendering static/login.html.erb
  Rendered static/login.html.erb (Duration: 0.4ms | Allocations: 225)
Completed 200 OK in 5ms (Views: 1.6ms | ActiveRecord: 0.0ms | Allocations: 1064)

I’m not sure what’s not working here but it seems as though Rails completed the redirect and renders login.html/erb (even though really we just need it to change the URL so that Ember knows something has changed). In the browser the page is blank and the Dev Tools show that Custom Wizard page content is still loaded.

Here’s a similar redirect in Discourse’s own code, but following this pattern it is still not working.

Any thoughts?

Indeed, this task is a little tricker than it first seems. I think I sent you down the wrong path in the first place, as what you’re struggling with here is the disconnect between the server and the client.

I’ve pushed a working branch to the repository, and made PR so you can have a look / test before we merge it. This is still your job. You spent time on this and I misdirected you at the start, so you’re still getting the payment for this.

What’s happening here is that Rails is running both the controller hooks, and rendering the wizard layout, which is set in the wizard controller, overriding the standard application layout.

The wizard layout is what actually loads the Ember client, specifically the wizard-custom-start.js script. If you look at that script in javascripts/wizard you’ll see that it requires the file with the wizard ember app in it.

After testing this a bit, I’ve found the most efficient way to perform this redirect is to actually do it on the client, by loading a different script, i.e. other than wizard-custom-start.js if there is no user server-side. This new script simply sends the client to the login route.

1 Like

Thanks @angus I’ve had a look at the PR and I see how it works now. Thanks for merging that PR.

I have tested the new version which does redirect to /login however it doesn’t seem to remember the originally requested wizard URL and forward me to that after successful login.

The Ruby code in wizard.rb is not getting run, because the redirect happens before that stage. If I manually set the cookie in the Javascript console like this:

document.cookie='destination_url=http://localhost:9292/w/feature-request'

the cookie is set and Discourse does its own redirect to the right URL after login.

However when I’ve added this to wizard-custom-guest.js it doesn’t work, possibly there’s something about the context of that bit of code I’m not getting right…?

(function() {
  document.cookie='destination_url=http://localhost:9292/w/feature-request';
  window.location.href = "/login";
})();

I presume we just need to put this code in the correct bit in the JS and grab the current URL from the window. We can remove the Ruby before_action.

1 Like

@pacharanero Cool, can you make a PR to custom-wizard with that fix? I’ve deployed the latest to thepavilion.io and the initial redirect is working (but the redirect back isn’t, as you say)

1 Like
1 Like

@angus could you take a look at this? I’ve drawn a blank trying to work out why the login redirect works but the line immediately above does not appear to run, or even error.

I know backtick string template syntax is new-ish but I think it’s fairly uniformly implemented now isn’t it? source

I can’t seem even to get the debugger; to invoke at this stage! Time to take a break from it

@Economicurtis This was merged last week. Closing this.

1 Like