DEV: by default disable anon impersonation in dev environments

DEV: by default disable anon impersonation in dev environments

The impersonate any user by anonymous feature in dev should require a deliberate opt-in. This way developers are better aware of the security implications of this development only feature.

diff --git a/app/controllers/session_controller.rb b/app/controllers/session_controller.rb
index 5ea61f2..3ea48ad 100644
--- a/app/controllers/session_controller.rb
+++ b/app/controllers/session_controller.rb
@@ -103,7 +103,21 @@ class SessionController < ApplicationController
     skip_before_action :check_xhr, only: [:become]
 
     def become
+
       raise Discourse::InvalidAccess if Rails.env.production?
+
+      if ENV['DISCOURSE_DEV_ALLOW_ANON_TO_IMPERSONATE'] != "1"
+        render(content_type: 'text/plain', inline: <<~TEXT)
+          To enable impersonating any user without typing passwords set the following ENV var
+
+          export DISCOURSE_DEV_ALLOW_ANON_TO_IMPERSONATE=1
+
+          You can do that in your bashrc of bash profile file or the script you use to launch the web server
+        TEXT
+
+        return
+      end
+
       user = User.find_by_username(params[:session_id])
       raise "User #{params[:session_id]} not found" if user.blank?

GitHub sha: e302c0af

I’m not sure I understand this commit. What are the security implications of this in dev?

If you don’t read the label and go and deploy Discourse in development mode, your state will be terrible, but not horrendous horrible.

I find anon can impersonate anyone is a feature we HAVE to opt in for, not a good default.

In dev

/session/sam/become will log on anonymous as sam

This is the root cause of the problem IMO and I don’t think we should fix development code when this is a deployment error.

The same rules already hold for better_errors which by default requires you whitelist IPs.

I am not trying to fix deployment of dev environment in production here. What I am trying to do is make sure that people take a deliberate action even in development when they want to turn on the: security 100% off feature.

But why does it require a deliberate action when security 100% off in development is totally safe?

Not following all:

Are you saying?

I see zero point in introducing even rudimentary security measures in development mode?

You can be running a dev environment on a local machine in an internet cafe, suddenly everyone in your network can get admin your Discourse instance, default out-of-the-box.

I don’t like that default.

Even if someone decides to expose their local environment over a network, the solution here doesn’t solve the problem if they always have the env set in their bash profile. I just don’t see how this is going to prevent anything when devs are more likely to throw this in their bash profile and forget that the env even exists.

Those scenarios that you described are extremely rare IMO and I think it should be a conscious effort on the developer’s part to know that he is exposing his instance over the network and to manage the risks involved.

I am 100% open to a solution that instead simply junks this route completely and we use something else:

rake user:login[sam]

paste this into your web site

127.0.0.1/MAGICTOKEN
1 Like

That increases the friction in development significantly though. Perhaps we can just check for the IP of the server and only allow the same IPs through? Not sure?

I created the /become route and use it constantly when developing locally. I would be very much against a rake task every time I wanted to log in.

Having said that sam’s solution of requiring an ENV variable is not bad. I could easily add that and go on my way.

I don’t think it’s a serious problem though. If you deploy your site in development mode to production you have HUGE problems, and by default a local rails server should not be visible on the network. You have to explicitly bind it to a public IP which requires extra steps.

2 Likes