diff options
| -rw-r--r-- | app/controllers/admin_censor_rule_controller.rb | 4 | ||||
| -rw-r--r-- | app/controllers/admin_controller.rb | 31 | ||||
| -rw-r--r-- | app/controllers/admin_general_controller.rb | 2 | ||||
| -rw-r--r-- | app/controllers/admin_public_body_controller.rb | 8 | ||||
| -rw-r--r-- | app/controllers/admin_request_controller.rb | 18 | ||||
| -rw-r--r-- | app/controllers/application_controller.rb | 14 | ||||
| -rw-r--r-- | app/controllers/user_controller.rb | 5 | ||||
| -rw-r--r-- | app/views/admin_general/debug.rhtml | 2 | ||||
| -rw-r--r-- | spec/controllers/admin_public_body_controller_spec.rb | 51 |
9 files changed, 86 insertions, 49 deletions
diff --git a/app/controllers/admin_censor_rule_controller.rb b/app/controllers/admin_censor_rule_controller.rb index dca312b8b..5381921bf 100644 --- a/app/controllers/admin_censor_rule_controller.rb +++ b/app/controllers/admin_censor_rule_controller.rb @@ -15,7 +15,7 @@ class AdminCensorRuleController < AdminController end def create - params[:censor_rule][:last_edit_editor] = admin_http_auth_user() + params[:censor_rule][:last_edit_editor] = admin_current_user() @censor_rule = CensorRule.new(params[:censor_rule]) if @censor_rule.save if !@censor_rule.info_request.nil? @@ -42,7 +42,7 @@ class AdminCensorRuleController < AdminController end def update - params[:censor_rule][:last_edit_editor] = admin_http_auth_user() + params[:censor_rule][:last_edit_editor] = admin_current_user() @censor_rule = CensorRule.find(params[:id]) if @censor_rule.update_attributes(params[:censor_rule]) if !@censor_rule.info_request.nil? diff --git a/app/controllers/admin_controller.rb b/app/controllers/admin_controller.rb index dd966c4af..d7933b212 100644 --- a/app/controllers/admin_controller.rb +++ b/app/controllers/admin_controller.rb @@ -45,12 +45,36 @@ class AdminController < ApplicationController end end + # For administration interface, return display name of authenticated user + def admin_current_user + if Configuration::skip_admin_auth + admin_http_auth_user + else + session[:admin_name] + end + end + + # If we're skipping Alaveteli admin authentication, assume that the environment + # will give us an authenticated user name + def admin_http_auth_user + # This needs special magic in mongrel: http://www.ruby-forum.com/topic/83067 + # Hence the second clause which reads X-Forwarded-User header if available. + # See the rewrite rules in conf/httpd.conf which set X-Forwarded-User + if request.env["REMOTE_USER"] + return request.env["REMOTE_USER"] + elsif request.env["HTTP_X_FORWARDED_USER"] + return request.env["HTTP_X_FORWARDED_USER"] + else + return "*unknown*"; + end + end + def authenticate if Configuration::skip_admin_auth session[:using_admin] = 1 return else - if session[:using_admin].nil? + if session[:using_admin].nil? || session[:admin_name].nil? if params[:emergency].nil? if authenticated?( :web => _("To log into the administrative interface"), @@ -59,11 +83,12 @@ class AdminController < ApplicationController :user_name => "a superuser") if !@user.nil? && @user.admin_level == "super" session[:using_admin] = 1 - request.env['REMOTE_USER'] = @user.url_name + session[:admin_name] = @user.url_name else session[:using_admin] = nil session[:user_id] = nil + session[:admin_name] = nil self.authenticate end end @@ -71,7 +96,7 @@ class AdminController < ApplicationController authenticate_or_request_with_http_basic do |user_name, password| if user_name == Configuration::admin_username && password == Configuration::admin_password session[:using_admin] = 1 - request.env['REMOTE_USER'] = user_name + session[:admin_name] = user_name else request_http_basic_authentication end diff --git a/app/controllers/admin_general_controller.rb b/app/controllers/admin_general_controller.rb index 5176eb8db..9f4c398c1 100644 --- a/app/controllers/admin_general_controller.rb +++ b/app/controllers/admin_general_controller.rb @@ -120,7 +120,7 @@ class AdminGeneralController < AdminController end def debug - @http_auth_user = admin_http_auth_user + @admin_current_user = admin_current_user @current_commit = `git log -1 --format="%H"` @current_branch = `git branch | perl -ne 'print $1 if /^\\* (.*)/'` @current_version = `git describe --always --tags` diff --git a/app/controllers/admin_public_body_controller.rb b/app/controllers/admin_public_body_controller.rb index e64925bde..ac12e97b2 100644 --- a/app/controllers/admin_public_body_controller.rb +++ b/app/controllers/admin_public_body_controller.rb @@ -88,7 +88,7 @@ class AdminPublicBodyController < AdminController def create PublicBody.with_locale(I18n.default_locale) do - params[:public_body][:last_edit_editor] = admin_http_auth_user() + params[:public_body][:last_edit_editor] = admin_current_user() @public_body = PublicBody.new(params[:public_body]) if @public_body.save flash[:notice] = 'PublicBody was successfully created.' @@ -107,7 +107,7 @@ class AdminPublicBodyController < AdminController def update PublicBody.with_locale(I18n.default_locale) do - params[:public_body][:last_edit_editor] = admin_http_auth_user() + params[:public_body][:last_edit_editor] = admin_current_user() @public_body = PublicBody.find(params[:id]) if @public_body.update_attributes(params[:public_body]) flash[:notice] = 'PublicBody was successfully updated.' @@ -157,7 +157,7 @@ class AdminPublicBodyController < AdminController params[:tag], params[:tag_behaviour], true, - admin_http_auth_user(), + admin_current_user(), I18n.available_locales) if errors.size == 0 @@ -171,7 +171,7 @@ class AdminPublicBodyController < AdminController params[:tag], params[:tag_behaviour], false, - admin_http_auth_user(), + admin_current_user(), I18n.available_locales) if errors.size != 0 raise "dry run mismatched real run" diff --git a/app/controllers/admin_request_controller.rb b/app/controllers/admin_request_controller.rb index c97ef7f1e..3e574b10f 100644 --- a/app/controllers/admin_request_controller.rb +++ b/app/controllers/admin_request_controller.rb @@ -85,7 +85,7 @@ class AdminRequestController < AdminController if @info_request.valid? @info_request.save! @info_request.log_event("edit", - { :editor => admin_http_auth_user(), + { :editor => admin_current_user(), :old_title => old_title, :title => @info_request.title, :old_prominence => old_prominence, :prominence => @info_request.prominence, :old_described_state => old_described_state, :described_state => @info_request.described_state, @@ -128,7 +128,7 @@ class AdminRequestController < AdminController @outgoing_message.fully_destroy @outgoing_message.info_request.log_event("destroy_outgoing", - { :editor => admin_http_auth_user(), :deleted_outgoing_message_id => outgoing_message_id }) + { :editor => admin_current_user(), :deleted_outgoing_message_id => outgoing_message_id }) flash[:notice] = 'Outgoing message successfully destroyed.' redirect_to request_admin_url(@info_request) @@ -141,7 +141,7 @@ class AdminRequestController < AdminController if @outgoing_message.update_attributes(params[:outgoing_message]) @outgoing_message.info_request.log_event("edit_outgoing", - { :outgoing_message_id => @outgoing_message.id, :editor => admin_http_auth_user(), + { :outgoing_message_id => @outgoing_message.id, :editor => admin_current_user(), :old_body => old_body, :body => @outgoing_message.body }) flash[:notice] = 'Outgoing message successfully updated.' redirect_to request_admin_url(@outgoing_message.info_request) @@ -163,7 +163,7 @@ class AdminRequestController < AdminController if @comment.update_attributes(params[:comment]) @comment.info_request.log_event("edit_comment", - { :comment_id => @comment.id, :editor => admin_http_auth_user(), + { :comment_id => @comment.id, :editor => admin_current_user(), :old_body => old_body, :body => @comment.body, :old_visible => old_visible, :visible => @comment.visible, }) @@ -182,7 +182,7 @@ class AdminRequestController < AdminController @incoming_message.fully_destroy @incoming_message.info_request.log_event("destroy_incoming", - { :editor => admin_http_auth_user(), :deleted_incoming_message_id => incoming_message_id }) + { :editor => admin_current_user(), :deleted_incoming_message_id => incoming_message_id }) # expire cached files expire_for_request(@info_request) flash[:notice] = 'Incoming message successfully destroyed.' @@ -213,7 +213,7 @@ class AdminRequestController < AdminController incoming_message_id = incoming_message.id incoming_message.info_request.log_event("redeliver_incoming", { - :editor => admin_http_auth_user(), + :editor => admin_current_user(), :destination_request => destination_request.id, :deleted_incoming_message_id => incoming_message_id }) @@ -239,7 +239,7 @@ class AdminRequestController < AdminController info_request.user = destination_user info_request.save! info_request.log_event("move_request", { - :editor => admin_http_auth_user(), + :editor => admin_current_user(), :old_user_url_name => old_user.url_name, :user_url_name => destination_user.url_name }) @@ -257,7 +257,7 @@ class AdminRequestController < AdminController info_request.public_body = destination_public_body info_request.save! info_request.log_event("move_request", { - :editor => admin_http_auth_user(), + :editor => admin_current_user(), :old_public_body_url_name => old_public_body.url_name, :public_body_url_name => destination_public_body.url_name }) @@ -367,7 +367,7 @@ class AdminRequestController < AdminController info_request.prominence = "requester_only" info_request.log_event("hide", { - :editor => admin_http_auth_user(), + :editor => admin_current_user(), :reason => params[:reason], :subject => subject, :explanation => explanation diff --git a/app/controllers/application_controller.rb b/app/controllers/application_controller.rb index f29015c63..3f3c169ae 100644 --- a/app/controllers/application_controller.rb +++ b/app/controllers/application_controller.rb @@ -340,20 +340,6 @@ class ApplicationController < ActionController::Base end - # For administration interface, return display name of authenticated user - def admin_http_auth_user - # This needs special magic in mongrel: http://www.ruby-forum.com/topic/83067 - # Hence the second clause which reads X-Forwarded-User header if available. - # See the rewrite rules in conf/httpd.conf which set X-Forwarded-User - if request.env["REMOTE_USER"] - return request.env["REMOTE_USER"] - elsif request.env["HTTP_X_FORWARDED_USER"] - return request.env["HTTP_X_FORWARDED_USER"] - else - return "*unknown*"; - end - end - # Convert URL name for sort by order, to Xapian query def order_to_sort_by(sortby) if sortby.nil? diff --git a/app/controllers/user_controller.rb b/app/controllers/user_controller.rb index 810b3321e..4ee527bae 100644 --- a/app/controllers/user_controller.rb +++ b/app/controllers/user_controller.rb @@ -119,13 +119,13 @@ class UserController < ApplicationController @track_things = TrackThing.find(:all, :conditions => ["tracking_user_id = ? and track_medium = ?", @display_user.id, 'email_daily'], :order => 'created_at desc') for track_thing in @track_things # XXX factor out of track_mailer.rb - xapian_object = InfoRequest.full_search([InfoRequestEvent], track_thing.track_query, 'described_at', true, nil, 20, 1) + xapian_object = InfoRequest.full_search([InfoRequestEvent], track_thing.track_query, 'described_at', true, nil, 20, 1) feed_results += xapian_object.results.map {|x| x[:model]} end end @feed_results = Array(feed_results).sort {|x,y| y.created_at <=> x.created_at}.first(20) - + respond_to do |format| format.html { @has_json = true } format.json { render :json => @display_user.json_for_api } @@ -244,6 +244,7 @@ class UserController < ApplicationController session[:user_circumstance] = nil session[:remember_me] = false session[:using_admin] = nil + session[:admin_name] = nil end def signout self._do_signout diff --git a/app/views/admin_general/debug.rhtml b/app/views/admin_general/debug.rhtml index d7bf1c6da..99488ba0c 100644 --- a/app/views/admin_general/debug.rhtml +++ b/app/views/admin_general/debug.rhtml @@ -2,7 +2,7 @@ <h1><%=@title%></h1> -<p>You are <%= h @http_auth_user %></p> +<p>You are <%= h @admin_current_user %></p> <h2>Version numbers</h2> diff --git a/spec/controllers/admin_public_body_controller_spec.rb b/spec/controllers/admin_public_body_controller_spec.rb index be33802c5..504ddc5cc 100644 --- a/spec/controllers/admin_public_body_controller_spec.rb +++ b/spec/controllers/admin_public_body_controller_spec.rb @@ -166,6 +166,13 @@ describe AdminPublicBodyController, "when administering public bodies and paying config['SKIP_ADMIN_AUTH'] = true end + def setup_emergency_credentials(username, password) + config = MySociety::Config.load_default() + config['SKIP_ADMIN_AUTH'] = false + config['ADMIN_USERNAME'] = username + config['ADMIN_PASSWORD'] = password + @request.env["HTTP_AUTHORIZATION"] = "" + end it "disallows non-authenticated users to do anything" do @request.env["HTTP_AUTHORIZATION"] = "" @@ -180,19 +187,14 @@ describe AdminPublicBodyController, "when administering public bodies and paying config = MySociety::Config.load_default() config['SKIP_ADMIN_AUTH'] = true @request.env["HTTP_AUTHORIZATION"] = "" - n = PublicBody.count post :destroy, { :id => public_bodies(:forlorn_public_body).id } PublicBody.count.should == n - 1 session[:using_admin].should == 1 end - it "doesn't let people with bad credentials log in" do - config = MySociety::Config.load_default() - config['SKIP_ADMIN_AUTH'] = false - config['ADMIN_USERNAME'] = 'biz' - config['ADMIN_PASSWORD'] = 'fuz' - @request.env["HTTP_AUTHORIZATION"] = "" + it "doesn't let people with bad emergency account credentials log in" do + setup_emergency_credentials('biz', 'fuz') n = PublicBody.count basic_auth_login(@request, "baduser", "badpassword") post :destroy, { :id => public_bodies(:forlorn_public_body).id } @@ -201,12 +203,8 @@ describe AdminPublicBodyController, "when administering public bodies and paying session[:using_admin].should == nil end - it "allows people with good credentials log in using HTTP Basic Auth" do - config = MySociety::Config.load_default() - config['SKIP_ADMIN_AUTH'] = false - config['ADMIN_USERNAME'] = 'biz' - config['ADMIN_PASSWORD'] = 'fuz' - @request.env["HTTP_AUTHORIZATION"] = "" + it "allows people with good emergency account credentials log in using HTTP Basic Auth" do + setup_emergency_credentials('biz', 'fuz') n = PublicBody.count basic_auth_login(@request, "biz", "fuz") post :show, { :id => public_bodies(:humpadink_public_body).id, :emergency => 1} @@ -235,6 +233,33 @@ describe AdminPublicBodyController, "when administering public bodies and paying PublicBody.count.should == n session[:using_admin].should == nil end + + describe 'when asked for the admin current user' do + + it 'returns the emergency account name for someone who logged in with the emergency account' do + setup_emergency_credentials('biz', 'fuz') + basic_auth_login(@request, "biz", "fuz") + post :show, { :id => public_bodies(:humpadink_public_body).id, :emergency => 1 } + controller.send(:admin_current_user).should == 'biz' + end + + it 'returns the current user url_name for a superuser' do + session[:user_id] = users(:admin_user).id + @request.env["HTTP_AUTHORIZATION"] = "" + post :show, { :id => public_bodies(:humpadink_public_body).id } + controller.send(:admin_current_user).should == users(:admin_user).url_name + end + + it 'returns the REMOTE_USER value from the request environment when skipping admin auth' do + config = MySociety::Config.load_default() + config['SKIP_ADMIN_AUTH'] = true + @request.env["HTTP_AUTHORIZATION"] = "" + @request.env["REMOTE_USER"] = "i_am_admin" + post :show, { :id => public_bodies(:humpadink_public_body).id } + controller.send(:admin_current_user).should == "i_am_admin" + end + + end end describe AdminPublicBodyController, "when administering public bodies with i18n" do |