Check for the case where the engine is not mounted

[ndbroadbent]

Also refactor the code a little bit to remove duplication.

Closes #33

[nicolasrouanne]

Good improvement!

I'll add that it could be interesting that the gem triggers authentication only on the presence on the mounted route, wether or not the lockup_codeword env variable is set.

Indeed, if you 'forget' the environment variable from your staging conf your environment is open to the world. And you might even not notice it since your so used to be automatically logged in 😆

[ndbroadbent]

@nicolasrouanne That's an interesting idea. LOCKUP_CODEWORD works great the first time you set it up, because everyone tests their staging server at least once. However, it's easy to forget if you ever migrate to a different platform. This is especially true if you have a team with multiple engineers.

For example, you're probably going to set up most of your staging environment correctly (because you call Rails.env.staging? or ENV['STAGING'] a few times in your app), but you might forget to set the LOCKUP_CODEWORD on your new servers. In that case, I think it would be much better to show an error page saying "Please set your lockup codeword.", instead of making everything public. You're totally right, you wouldn't notice that for a long time because you rarely need to enter the password.

I think your idea is more future-proof, and lockup should always be enabled if you mount it like this:

mount Lockup::Engine, at: '/lockup' if Rails.env.staging?

I follow Heroku's recommendation to not create a separate staging environment, but I still use a STAGING env variable for things like this:

mount Lockup::Engine, at: '/lockup' if ENV['STAGING']

You can also enforce this behavior in your own app. I've just updated my code in config/routes.rb:

  if ENV['STAGING']
    unless ENV['LOCKUP_CODEWORD'].present?
      raise "You must set the LOCKUP_CODEWORD env variable to password-protect the staging environment."
    end
    mount Lockup::Engine, at: '/lockup'
  end

Thanks a lot for the idea! I'm actually planning to migrate to a new platform soon, and it's very possible that I would have forgotten to set the LOCKUP_CODEWORD variable.

[gblakeman]

I like this check and the refactor is nice. I’ll give it some testing as soon as I have a chance and get it merged into master.

[gblakeman]

@nicolasrouanne @ndbroadbent As far as the discussion related to keeping things locked if a codeword isn’t set:

I’m not saying my mind can’t be changed, but I’ve always really liked the simplicity of tying Lockup to the codeword variable (however you set it).

I often still use Lockup in my production environment right up to launching the site and it’s nice that “launch” is as simple as removing the codeword. I’ll usually followup later, after launch and remove the gem from my production bundle.

I understand it could be easy to forget to set the variable if you move hosting environments, but how often does that happen? Remember that this gem is supposed to be about light security, rather than actual security—it's like a lock on a fence you could hop over.

Personally, I would be more concerned about forgetting that the gem was set up in a production bundle and accidentally locking down a production site.

[gblakeman]

👍👍